Updates Features for the Cli from amp#2
Conversation
📝 WalkthroughWalkthroughAdds multiple new CLI modules and commands (threads, tools, mcp, permissions), login/logout and non-interactive execute mode, a minimal LSP server, an HTTP session viewer endpoint, a persistent permission store, enhanced webfetch/websearch logic, and session/thread management utilities; README checkboxes updated accordingly. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
codesm/cli.py (1)
214-254:⚠️ Potential issue | 🟠 MajorEnsure
agent.cleanup()runs even on execute errors.If
agent.chat()raises, cleanup is skipped and resources can leak. Usetry/finallylike the chat path.🛠️ Proposed fix
async def run_execute(): store = CredentialStore() model = store.get_preferred_model() or "anthropic/claude-sonnet-4-20250514" # Initialize agent agent = Agent(directory=Path("."), model=model) # We want to capture the final answer, not the whole stream full_response = "" - async for chunk in agent.chat(execute): - if chunk.type == "text": - full_response += chunk.content - - # Print only the final result - print(full_response) - - await agent.cleanup() + try: + async for chunk in agent.chat(execute): + if chunk.type == "text": + full_response += chunk.content + # Print only the final result + print(full_response) + finally: + await agent.cleanup()
🤖 Fix all issues with AI agents
In `@codesm/cli_threads.py`:
- Around line 70-120: The export_session command currently silently falls back
to markdown for unknown format values; update export_session to validate the
format argument (use format.lower()) against an explicit allowed set (e.g.,
{"json","markdown"}) and if it's not allowed, raise a clear typer.BadParameter
or call typer.Exit with a descriptive error logged to console explaining the
supported formats; reference the format parameter and the export_session
function and ensure the validation occurs before any processing so typos like
"md" produce an immediate error instead of defaulting to markdown.
In `@codesm/server/server.py`:
- Around line 39-92: The view_session handler renders unescaped user data and
must sanitize all outputs: import html and bleach, escape session.title and any
tool_name with html.escape(session.title) / html.escape(tool_name), render
markdown for user/assistant messages then pass the resulting HTML through
bleach.clean(...) using a safe tag/attribute allowlist before inserting into the
template, and escape raw tool_display content with html.escape(content) before
putting it into the <pre>. Update view_session and the loop that uses
session.get_messages_for_display() to apply these changes and add bleach to
project dependencies.
In `@README.md`:
- Around line 298-299: Update the README feature list entry for the thread
export command so the word "Markdown" is capitalized: change the checkbox line
containing `codesm threads markdown` to use `Markdown` (e.g., `codesm threads
Markdown`) while leaving the command text intact and keeping the surrounding
checklist formatting and the `codesm threads replay` line unchanged.
🧹 Nitpick comments (2)
codesm/lsp/server.py (1)
18-27: Silence unusedparamswarning.Rename to
_params(or add a noqa) to avoid the ARG001 lint.♻️ Proposed tweak
-@server.feature(TEXT_DOCUMENT_COMPLETION) -def completions(params: CompletionParams): +@server.feature(TEXT_DOCUMENT_COMPLETION) +def completions(_params: CompletionParams):codesm/cli_tools.py (1)
37-40: Rename unused loop variable for clarity.
nameisn’t used; use_nameto signal intentional unused.♻️ Proposed tweak
- for name, config in servers.items(): + for _name, config in servers.items(): manager.add_server(config)
| @app.command("export") | ||
| def export_session( | ||
| session_id: str = typer.Argument(..., help="Session ID"), | ||
| format: str = typer.Option("markdown", "--format", "-f", help="Output format (markdown, json)"), | ||
| output: Path = typer.Option(None, "--output", "-o", help="Output file path (default: stdout)"), | ||
| ): | ||
| """Export a session to Markdown or JSON""" | ||
| session = Session.load(session_id) | ||
| if not session: | ||
| console.print(f"[red]Session not found: {session_id}[/red]") | ||
| raise typer.Exit(1) | ||
|
|
||
| result = "" | ||
|
|
||
| if format.lower() == "json": | ||
| import json | ||
| # Filter out internal keys starting with _ | ||
| clean_messages = [] | ||
| for msg in session.messages: | ||
| clean_msg = {k: v for k, v in msg.items() if not k.startswith("_")} | ||
| clean_messages.append(clean_msg) | ||
|
|
||
| data = { | ||
| "id": session.id, | ||
| "title": session.title, | ||
| "created_at": session.created_at.isoformat(), | ||
| "messages": clean_messages | ||
| } | ||
| result = json.dumps(data, indent=2) | ||
|
|
||
| else: # markdown | ||
| result = f"# {session.title}\n\n" | ||
| result += f"*Created: {session.created_at}*\n\n" | ||
|
|
||
| for msg in session.get_messages_for_display(): | ||
| role = msg.get("role", "unknown") | ||
| content = msg.get("content", "") | ||
|
|
||
| if role == "user": | ||
| result += f"## User\n\n{content}\n\n" | ||
| elif role == "assistant": | ||
| result += f"## codesm\n\n{content}\n\n" | ||
| elif role == "tool_display": | ||
| result += f"### Tool: {msg.get('tool_name')}\n\n```\n{content}\n```\n\n" | ||
|
|
||
| if output: | ||
| output.write_text(result, encoding="utf-8") | ||
| console.print(f"[green]Exported to {output}[/green]") | ||
| else: | ||
| print(result) | ||
|
|
There was a problem hiding this comment.
Validate export format to avoid silent fallback.
Any unknown format currently defaults to markdown; a typo (e.g., md) won’t error and may confuse users.
✅ Proposed guard
result = ""
- if format.lower() == "json":
+ fmt = format.lower()
+ if fmt not in {"markdown", "json"}:
+ console.print(f"[red]Unsupported format: {format}[/red]")
+ raise typer.Exit(1)
+
+ if fmt == "json":
import json
# Filter out internal keys starting with _
@@
- else: # markdown
+ else: # markdown
result = f"# {session.title}\n\n"🧰 Tools
🪛 Ruff (0.14.14)
[warning] 74-74: Do not perform function call typer.Option in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
In `@codesm/cli_threads.py` around lines 70 - 120, The export_session command
currently silently falls back to markdown for unknown format values; update
export_session to validate the format argument (use format.lower()) against an
explicit allowed set (e.g., {"json","markdown"}) and if it's not allowed, raise
a clear typer.BadParameter or call typer.Exit with a descriptive error logged to
console explaining the supported formats; reference the format parameter and the
export_session function and ensure the validation occurs before any processing
so typos like "md" produce an immediate error instead of defaulting to markdown.
| @app.get("/session/{session_id}") | ||
| async def view_session(session_id: str): | ||
| """"View a session as HTML""" | ||
| from codesm.session.session import Session | ||
| from fastapi.responses import HTMLResponse | ||
| import markdown | ||
|
|
||
| session = Session.load(session_id) | ||
| if not session: | ||
| return HTMLResponse("Session not found", status_code=404) | ||
|
|
||
| html = f""" | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <title>{session.title} - codesm</title> | ||
| <style> | ||
| body {{ font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif; max-width: 800px; margin: 0 auto; padding: 20px; line-height: 1.6; color: #333; }} | ||
| pre {{ background: #f4f4f4; padding: 15px; border-radius: 5px; overflow-x: auto; }} | ||
| code {{ font-family: 'Menlo', 'Monaco', 'Courier New', monospace; font-size: 0.9em; }} | ||
| .message {{ margin-bottom: 20px; padding: 15px; border-radius: 8px; }} | ||
| .user {{ background: #e3f2fd; border-left: 4px solid #2196f3; }} | ||
| .assistant {{ background: #f5f5f5; border-left: 4px solid #757575; }} | ||
| .tool {{ background: #fff8e1; border-left: 4px solid #ffc107; font-size: 0.9em; }} | ||
| .meta {{ font-size: 0.8em; color: #666; margin-bottom: 5px; }} | ||
| h1 {{ border-bottom: 1px solid #eee; padding-bottom: 10px; }} | ||
| </style> | ||
| </head> | ||
| <body> | ||
| <h1>{session.title}</h1> | ||
| <div class="meta">Created: {session.created_at} | ID: {session.id}</div> | ||
|
|
||
| <div id="messages"> | ||
| """ | ||
|
|
||
| for msg in session.get_messages_for_display(): | ||
| role = msg.get("role", "unknown") | ||
| content = msg.get("content", "") | ||
|
|
||
| if role == "user": | ||
| html += f'<div class="message user"><div class="meta">User</div>{markdown.markdown(content)}</div>' | ||
| elif role == "assistant": | ||
| html += f'<div class="message assistant"><div class="meta">codesm</div>{markdown.markdown(content)}</div>' | ||
| elif role == "tool_display": | ||
| tool_name = msg.get("tool_name", "tool") | ||
| html += f'<div class="message tool"><div class="meta">Tool: {tool_name}</div><pre>{content}</pre></div>' | ||
|
|
||
| html += """ | ||
| </div> | ||
| </body> | ||
| </html> | ||
| """ | ||
|
|
||
| return HTMLResponse(html) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the project structure and find the Session class
find . -type f -name "*.py" | grep -E "(session|server)" | head -20Repository: Aditya-PS-05/codesm
Length of output: 492
🏁 Script executed:
# Locate the Session class definition
fd -e py -x grep -l "class Session" {}Repository: Aditya-PS-05/codesm
Length of output: 197
🏁 Script executed:
# Check if bleach is already a dependency
find . -type f \( -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml" -o -name "poetry.lock" \) | head -10Repository: Aditya-PS-05/codesm
Length of output: 79
🏁 Script executed:
# Check dependencies in pyproject.toml
cat pyproject.tomlRepository: Aditya-PS-05/codesm
Length of output: 783
🏁 Script executed:
# Read the Session class implementation to understand data flow
cat -n codesm/session/session.py | head -100Repository: Aditya-PS-05/codesm
Length of output: 4648
🏁 Script executed:
# Check the get_messages_for_display method
grep -n "get_messages_for_display" codesm/session/session.py -A 20Repository: Aditya-PS-05/codesm
Length of output: 965
🏁 Script executed:
# Check where messages are added to sessions
grep -rn "messages.append\|messages +=" codesm/session/ --include="*.py" | head -20Repository: Aditya-PS-05/codesm
Length of output: 268
🏁 Script executed:
# Read the message module to understand message structure
cat -n codesm/session/message.pyRepository: Aditya-PS-05/codesm
Length of output: 2558
🏁 Script executed:
# Check if there's any existing HTML sanitization in the codebase
grep -rn "html.escape\|bleach\|sanitize" codesm/ --include="*.py"Repository: Aditya-PS-05/codesm
Length of output: 625
🏁 Script executed:
# Look at markdown usage in the codebase
grep -rn "markdown" codesm/ --include="*.py"Repository: Aditya-PS-05/codesm
Length of output: 6049
🏁 Script executed:
# Check if the /session endpoint has any authentication/protection
cat -n codesm/server/server.py | head -100Repository: Aditya-PS-05/codesm
Length of output: 4151
🏁 Script executed:
# Check FastAPI app setup for any middleware or security
grep -rn "app.get\|app.post\|@app\|middleware\|security" codesm/server/ --include="*.py" -B2 -A5 | head -60Repository: Aditya-PS-05/codesm
Length of output: 1125
🏁 Script executed:
# Look for how sessions are accessed in the frontend/client
grep -rn "/session/" codesm/ --include="*.py" --include="*.js" --include="*.ts" | head -20Repository: Aditya-PS-05/codesm
Length of output: 207
🌐 Web query:
markdown.markdown() HTML sanitization default behavior Python
💡 Result:
markdown.markdown() (Python-Markdown) does not sanitize HTML by default: any raw HTML in the Markdown input is generally passed through / not altered and will appear in the output HTML. Python-Markdown’s docs explicitly note that “the default behavior is for Markdown to not alter raw HTML.” [2]
Historically, Python-Markdown offered a safe_mode option, but it was deprecated because it was never actually safe and later ignored/removed; the project recommends sanitizing after conversion using a real HTML sanitizer such as Bleach. [1][3]
Minimal safe pattern for untrusted input:
import markdown, bleach
html = bleach.clean(markdown.markdown(text))[3]
Add HTML sanitization to prevent XSS when rendering session content.
The /session/{session_id} endpoint is publicly accessible and renders user-controlled data (session title, chat messages, tool outputs) as HTML. The markdown.markdown() function does not sanitize HTML by default—raw HTML and script tags in message content pass through to the rendered page. Additionally, session.title, tool_name, and raw <pre>{content}</pre> are not escaped, creating multiple XSS vectors.
Sanitize all rendered content:
- Escape
session.titleandtool_namewithhtml.escape() - Pass markdown-rendered HTML through
bleach.clean()with a safe tag allowlist for user and assistant messages - Escape
contentin tool_display<pre>blocks
Note: bleach must be added as a dependency in pyproject.toml.
🤖 Prompt for AI Agents
In `@codesm/server/server.py` around lines 39 - 92, The view_session handler
renders unescaped user data and must sanitize all outputs: import html and
bleach, escape session.title and any tool_name with html.escape(session.title) /
html.escape(tool_name), render markdown for user/assistant messages then pass
the resulting HTML through bleach.clean(...) using a safe tag/attribute
allowlist before inserting into the template, and escape raw tool_display
content with html.escape(content) before putting it into the <pre>. Update
view_session and the loop that uses session.get_messages_for_display() to apply
these changes and add bleach to project dependencies.
| - [x] `codesm threads markdown` - Export thread as markdown (via export command) | ||
| - [ ] `codesm threads replay` - Replay a thread |
There was a problem hiding this comment.
Capitalize “Markdown” in feature list.
The formatting language is a proper noun; use “Markdown”.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~298-~298: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...sm threads markdown- Export thread as markdown (via export command) - [ ]codesm th...
(MARKDOWN_NNP)
🤖 Prompt for AI Agents
In `@README.md` around lines 298 - 299, Update the README feature list entry for
the thread export command so the word "Markdown" is capitalized: change the
checkbox line containing `codesm threads markdown` to use `Markdown` (e.g.,
`codesm threads Markdown`) while leaving the command text intact and keeping the
surrounding checklist formatting and the `codesm threads replay` line unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@codesm/cli.py`:
- Around line 197-203: The file registers the same Typer subcommands twice
causing a runtime error: remove the duplicate early registrations by deleting or
commenting out the two calls that add threads_app and tools_app via
app.add_typer (the calls that reference threads_app and tools_app near the
import block) so that only the later registrations (the ones that include help
text) remain; ensure no other app.add_typer calls reuse the names "threads" or
"tools".
🧹 Nitpick comments (8)
codesm/cli.py (3)
339-354: Remove unused exception variable and consider logging ImportError.The exception variable
eis captured but never used. Either remove it or log it to help diagnose missing optional submodules.♻️ Proposed fix
except ImportError as e: - pass + import logging + logging.getLogger(__name__).debug(f"Optional CLI submodule not available: {e}")Or simply:
-except ImportError as e: +except ImportError: pass
183-184: Remove redundant import oftyper.
typeris already imported at the top of the file (line 3). This inner import is unnecessary.♻️ Proposed fix
from codesm.auth.credentials import CredentialStore - import typer
228-253: Consider adding error handling for the execute mode.The execute mode runs the agent but doesn't handle potential exceptions (e.g., network errors, authentication failures). Unhandled exceptions will produce stack traces instead of user-friendly error messages.
♻️ Proposed fix with error handling
async def run_execute(): store = CredentialStore() model = store.get_preferred_model() or "anthropic/claude-sonnet-4-20250514" # Initialize agent agent = Agent(directory=Path("."), model=model) - # We want to capture the final answer, not the whole stream - full_response = "" - async for chunk in agent.chat(execute): - if chunk.type == "text": - full_response += chunk.content - - # Print only the final result - print(full_response) - - await agent.cleanup() + try: + # We want to capture the final answer, not the whole stream + full_response = "" + async for chunk in agent.chat(execute): + if chunk.type == "text": + full_response += chunk.content + + # Print only the final result + print(full_response) + finally: + await agent.cleanup() asyncio.run(run_execute())codesm/cli_mcp.py (2)
23-28: Consider catching specific JSON exceptions.Catching a blind
Exceptioncan mask unexpected errors. Since this is config parsing, catchingjson.JSONDecodeErrorwould be more precise while still handling corrupt files gracefully.♻️ Proposed fix
def _load_raw_config() -> dict: _ensure_config() try: return json.loads(CONFIG_PATH.read_text()) - except Exception: + except (json.JSONDecodeError, FileNotFoundError): return {"mcpServers": {}}
158-170: Credentials are stored in plaintext JSON.The OAuth credentials (API tokens, secrets) are stored in
~/.config/codesm/mcp.jsonas plaintext. Consider documenting this behavior or using a more secure storage mechanism (e.g., system keychain viakeyringlibrary) for sensitive credentials.codesm/cli_permissions.py (1)
58-63: Consider addingadd_guard/remove_guardmethods toPermissionStore.The handling of guarded paths directly mutates
store.get_rules().guarded_pathsand manually callssave(), while allow/block rules use dedicated methods (add_allow,add_block). For consistency and encapsulation, consider addingadd_guardandremove_guardmethods to thePermissionStoreclass.codesm/permission/store.py (2)
26-35: Consider catching specific exceptions for config parsing.Similar to the MCP config, catching a blind
Exceptioncan mask unexpected errors. Since this handles JSON parsing, catchingjson.JSONDecodeErrorwould be more precise.♻️ Proposed fix
try: data = json.loads(self.path.read_text()) self._rules = PermissionRules( allowlist=data.get("allowlist", []), blocklist=data.get("blocklist", []), guarded_paths=data.get("guarded_paths", []) ) - except Exception: + except json.JSONDecodeError: # Fallback to empty if corrupt self._rules = PermissionRules()
44-62: Consider addingadd_guard/remove_guardmethods for consistency.The store has dedicated methods for allowlist and blocklist management but not for guarded_paths. The CLI currently mutates
get_rules().guarded_pathsdirectly. Adding helper methods would provide a consistent API.♻️ Proposed additions
def remove_block(self, pattern: str): if pattern in self._rules.blocklist: self._rules.blocklist.remove(pattern) self.save() + + def add_guard(self, pattern: str): + if pattern not in self._rules.guarded_paths: + self._rules.guarded_paths.append(pattern) + self.save() + + def remove_guard(self, pattern: str): + if pattern in self._rules.guarded_paths: + self._rules.guarded_paths.remove(pattern) + self.save()
| # Import subcommands | ||
| from codesm.cli_threads import app as threads_app | ||
| from codesm.cli_tools import app as tools_app | ||
|
|
||
| app.add_typer(threads_app, name="threads") | ||
| app.add_typer(tools_app, name="tools") | ||
|
|
There was a problem hiding this comment.
Duplicate subcommand registration will cause runtime error.
threads_app and tools_app are registered twice: once here at lines 198-202 without help text, and again at lines 340-345 with help text. Typer will raise an error when the same subcommand name is registered multiple times. Remove one of these registrations.
🐛 Proposed fix: Remove the first registration
-# Import subcommands
-from codesm.cli_threads import app as threads_app
-from codesm.cli_tools import app as tools_app
-
-app.add_typer(threads_app, name="threads")
-app.add_typer(tools_app, name="tools")
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Import subcommands | |
| from codesm.cli_threads import app as threads_app | |
| from codesm.cli_tools import app as tools_app | |
| app.add_typer(threads_app, name="threads") | |
| app.add_typer(tools_app, name="tools") |
🤖 Prompt for AI Agents
In `@codesm/cli.py` around lines 197 - 203, The file registers the same Typer
subcommands twice causing a runtime error: remove the duplicate early
registrations by deleting or commenting out the two calls that add threads_app
and tools_app via app.add_typer (the calls that reference threads_app and
tools_app near the import block) so that only the later registrations (the ones
that include help text) remain; ensure no other app.add_typer calls reuse the
names "threads" or "tools".
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
codesm/tool/websearch.py (1)
25-28:⚠️ Potential issue | 🟡 MinorDocumentation mismatch: default value differs from constant.
The schema description says "default: 8" but
DEFAULT_NUM_RESULTSis set to 5 on line 15.📝 Proposed fix
"num_results": { "type": "integer", - "description": "Number of search results to return (default: 8)", + "description": "Number of search results to return (default: 5)", },
🤖 Fix all issues with AI agents
In `@codesm/tool/websearch.py`:
- Line 61: Fix the inconsistent leading space before the assignment to results
in the websearch coroutine: locate the line containing "results = await
ddgs.text(query, max_results=num_results)" in function or method handling the
DDGS query and remove the extra leading space so the statement aligns with
surrounding indentation level; ensure the indentation matches the enclosing
block (e.g., same level as other statements in that function) to restore
consistent formatting.
- Around line 127-129: In the except block that catches Exa failures (inside the
method handling Exa searches, e.g., _search_exa), stop silently swallowing the
exception: log the caught exception (use the variable e) via the module logger
or processLogger and include the exception message when falling back to
DuckDuckGo, or prepend/annotate the returned DDG results with a note that Exa
failed and include the error text; ensure the except still returns await
self._search_ddg(query, num_results) after logging/annotating so static analysis
no longer flags the unused variable e.
🧹 Nitpick comments (1)
README.md (1)
280-284: Consider removing duplicate "Web Search" checkbox.Line 280 marks "Web Search tool improvements" as complete with details about DuckDuckGo and Exa AI support. Line 284 has a separate "[x] Web Search" that appears redundant. Consider removing line 284 to avoid confusion.
📝 Proposed fix
- [x] Web Search tool improvements - **DuckDuckGo** (default/free) and **Exa AI** (deep research) support - Enhanced HTML parsing with **BeautifulSoup4** (WebFetch) - [ ] Rust core performance -- [x] Web Search
| try: | ||
| from duckduckgo_search import AsyncDDGS | ||
| async with AsyncDDGS() as ddgs: | ||
| results = await ddgs.text(query, max_results=num_results) |
There was a problem hiding this comment.
Inconsistent indentation on line 61.
There's an extra leading space before results which is inconsistent with the surrounding code. While Python is lenient here, it reduces readability.
🔧 Proposed fix
async with AsyncDDGS() as ddgs:
- results = await ddgs.text(query, max_results=num_results)
+ results = await ddgs.text(query, max_results=num_results)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| results = await ddgs.text(query, max_results=num_results) | |
| results = await ddgs.text(query, max_results=num_results) |
🤖 Prompt for AI Agents
In `@codesm/tool/websearch.py` at line 61, Fix the inconsistent leading space
before the assignment to results in the websearch coroutine: locate the line
containing "results = await ddgs.text(query, max_results=num_results)" in
function or method handling the DDGS query and remove the extra leading space so
the statement aligns with surrounding indentation level; ensure the indentation
matches the enclosing block (e.g., same level as other statements in that
function) to restore consistent formatting.
| except Exception as e: | ||
| return f"Error searching for '{query}': {str(e)}" | ||
| # Fallback to DDG on failure? | ||
| return await self._search_ddg(query, num_results) |
There was a problem hiding this comment.
Silent fallback masks Exa failures.
When Exa fails with an unexpected exception, the code silently falls back to DuckDuckGo. The user won't know Exa failed, and the variable e is unused (flagged by static analysis F841). Consider logging the error or prefixing the DDG results with a note.
🔧 Proposed fix
except Exception as e:
- # Fallback to DDG on failure?
- return await self._search_ddg(query, num_results)
+ # Fallback to DDG on failure
+ ddg_result = await self._search_ddg(query, num_results)
+ return f"(Exa failed: {e!s}, falling back to DuckDuckGo)\n\n{ddg_result}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except Exception as e: | |
| return f"Error searching for '{query}': {str(e)}" | |
| # Fallback to DDG on failure? | |
| return await self._search_ddg(query, num_results) | |
| except Exception as e: | |
| # Fallback to DDG on failure | |
| ddg_result = await self._search_ddg(query, num_results) | |
| return f"(Exa failed: {e!s}, falling back to DuckDuckGo)\n\n{ddg_result}" |
🧰 Tools
🪛 Ruff (0.14.14)
[warning] 127-127: Do not catch blind exception: Exception
(BLE001)
[error] 127-127: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
🤖 Prompt for AI Agents
In `@codesm/tool/websearch.py` around lines 127 - 129, In the except block that
catches Exa failures (inside the method handling Exa searches, e.g.,
_search_exa), stop silently swallowing the exception: log the caught exception
(use the variable e) via the module logger or processLogger and include the
exception message when falling back to DuckDuckGo, or prepend/annotate the
returned DDG results with a note that Exa failed and include the error text;
ensure the except still returns await self._search_ddg(query, num_results) after
logging/annotating so static analysis no longer flags the unused variable e.
|
@Hackb07 , thanks for creating this. I am review it shortly. |
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.