add DebugAgent and debug_tools coverage#778
Open
shmbhvi101 wants to merge 2 commits into
Open
Conversation
Contributor
WalkthroughThreads local_mode through API and agent runtime, adds runtime sync, implements many VS Code tunnel-backed debug tools and tunnel routing/formatting, wires tools into ToolService/registry, updates environment/startup, and adds comprehensive unit/integration tests. ChangesDebug Tools and Local Mode Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/modules/intelligence/tools/registry/definitions.py (1)
323-334: ⚡ Quick winDerive
DEBUG_TOOLSfromTOOL_DEFINITIONSto avoid drift.This duplicates the debug tool names already declared above, so future edits can desync the two lists.
♻️ Proposed refactor
-DEBUG_TOOLS: List[str] = [ - "debug_start", - "debug_stop", - "debug_set_breakpoints", - "debug_snapshot", - "debug_step_into", - "debug_step_out", - "debug_step_over", - "debug_continue", - "debug_select_frame", - "debug_list_sessions", -] +DEBUG_TOOLS: List[str] = [ + name for name, meta in TOOL_DEFINITIONS.items() if meta.get("category") == "debug" +]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/modules/intelligence/tools/registry/definitions.py` around lines 323 - 334, The static DEBUG_TOOLS list duplicates definitions and should be derived from TOOL_DEFINITIONS to prevent drift: replace the hardcoded DEBUG_TOOLS with a computed list built from TOOL_DEFINITIONS (e.g., iterate TOOL_DEFINITIONS and collect tool names that identify as debug tools — for example keys or objects whose name starts with "debug_" or whose metadata/category is "debug"), ensuring the new DEBUG_TOOLS variable contains the same string names as before by filtering TOOL_DEFINITIONS and preserving ordering if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/modules/intelligence/tools/registry/definitions.py`:
- Around line 323-334: The static DEBUG_TOOLS list duplicates definitions and
should be derived from TOOL_DEFINITIONS to prevent drift: replace the hardcoded
DEBUG_TOOLS with a computed list built from TOOL_DEFINITIONS (e.g., iterate
TOOL_DEFINITIONS and collect tool names that identify as debug tools — for
example keys or objects whose name starts with "debug_" or whose
metadata/category is "debug"), ensuring the new DEBUG_TOOLS variable contains
the same string names as before by filtering TOOL_DEFINITIONS and preserving
ordering if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b0c3ea0-bb93-4ece-9d3b-23b1f3620fd0
📒 Files selected for processing (28)
app/celery/celery_app.pyapp/modules/conversations/conversation/conversation_controller.pyapp/modules/conversations/conversations_router.pyapp/modules/intelligence/agents/chat_agents/multi_agent/execution_flows.pyapp/modules/intelligence/agents/chat_agents/pydantic_agent.pyapp/modules/intelligence/agents/chat_agents/system_agents/debug_agent.pyapp/modules/intelligence/agents/chat_agents/tool_helpers.pyapp/modules/intelligence/tools/debug_tools/__init__.pyapp/modules/intelligence/tools/debug_tools/debug_continue_tool.pyapp/modules/intelligence/tools/debug_tools/debug_list_sessions_tool.pyapp/modules/intelligence/tools/debug_tools/debug_select_frame_tool.pyapp/modules/intelligence/tools/debug_tools/debug_set_breakpoints_tool.pyapp/modules/intelligence/tools/debug_tools/debug_snapshot_tool.pyapp/modules/intelligence/tools/debug_tools/debug_start_tool.pyapp/modules/intelligence/tools/debug_tools/debug_step_into_tool.pyapp/modules/intelligence/tools/debug_tools/debug_step_out_tool.pyapp/modules/intelligence/tools/debug_tools/debug_step_over_tool.pyapp/modules/intelligence/tools/debug_tools/debug_stop_tool.pyapp/modules/intelligence/tools/debug_tools/debug_tunnel_utils.pyapp/modules/intelligence/tools/local_search_tools/tunnel_utils.pyapp/modules/intelligence/tools/registry/definitions.pyapp/modules/intelligence/tools/registry/population.pyapp/modules/intelligence/tools/registry/schema.pyapp/modules/intelligence/tools/tool_service.pyscripts/start.shtests/unit/intelligence/agents/chat_agents/system_agents/test_debug_agent_integration.pytests/unit/intelligence/tools/debug_tools/test_debug_structured_tools.pytests/unit/intelligence/tools/debug_tools/test_debug_tunnel_utils.py
…ting ones - Introduced new debug tools: `add_watch`, `build_debug_context`, `find_related_tests`, `debug_list_launch_configs`, and `debug_list_adapters`. - Updated `DebugAgent` to utilize new tools and improved task prompts for local debugging. - Enhanced existing tools to support persistent watch expressions and improved context building from debug signals. - Updated dependencies in `pyproject.toml` and `requirements.txt` to include `torch` version 2.12.0 and other necessary packages. - Updated `uv.lock` to reflect changes in package versions and dependencies.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/modules/intelligence/tools/debug_tools/watch_store.py (1)
29-34: ⚡ Quick winAvoid creating store entries when removing from non-existent keys.
Lines 32-34 will create an empty list entry in
_storewhen the key doesn't exist (e.g., callingremove_watchbefore any watches are added). This accumulates empty dictionary entries over time and is inconsistent withlist_watches, which doesn't create entries.♻️ Proposed fix to skip assignment for non-existent keys or cleanup empty results
def remove_watch(user_id: Optional[str], conversation_id: Optional[str], expression: str) -> List[str]: k = _key(user_id, conversation_id) with _lock: + if k not in _store: + return [] watches = _store.get(k, []) _store[k] = [w for w in watches if w != expression] + if not _store[k]: + del _store[k] return list(_store[k])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/modules/intelligence/tools/debug_tools/watch_store.py` around lines 29 - 34, remove_watch currently creates an empty _store entry for non-existent keys; update remove_watch (use _key, _store, and _lock) to first check if k is in _store while holding _lock, return [] immediately if missing, otherwise filter the existing list, and then either assign the filtered list back to _store[k] or delete _store[k] when the filtered result is empty so you don't accumulate empty entries (preserve thread-safety by performing the check, filter, assign/delete inside the same _lock block).app/modules/intelligence/tools/registry/definitions.py (1)
378-399: ⚡ Quick winDerive
DEBUG_TOOLSfromTOOL_DEFINITIONSto avoid drift.This manual list duplicates the debug keys already declared above. A future add/remove can easily desync behavior.
♻️ Proposed change
-DEBUG_TOOLS: List[str] = [ - # DAP / extension tunnel - "debug_start", - "debug_stop", - "debug_set_breakpoints", - "debug_snapshot", - "debug_step_into", - "debug_step_out", - "debug_step_over", - "debug_continue", - "debug_select_frame", - "debug_list_sessions", - "debug_list_launch_configs", - "debug_list_adapters", - # Hypothesis workflow (backend-only) - "parse_debug_signal", - "build_debug_context", - "find_related_tests", - "add_watch", - "remove_watch", - "list_watches", -] +DEBUG_TOOLS: List[str] = [ + name for name, meta in TOOL_DEFINITIONS.items() if meta.get("category") == "debug" +]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/modules/intelligence/tools/registry/definitions.py` around lines 378 - 399, DEBUG_TOOLS is a manually maintained list that duplicates keys already present in TOOL_DEFINITIONS; replace the hardcoded list by deriving it from TOOL_DEFINITIONS to prevent drift by creating DEBUG_TOOLS from the keys/definitions in TOOL_DEFINITIONS (e.g., filter keys that start with "debug_" or match your debug predicate) while preserving the desired ordering and using the same symbol name DEBUG_TOOLS so callers are unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/modules/intelligence/agents/chat_agents/system_agents/debug_agent.py`:
- Around line 130-133: The warning call in DebugAgent's local_mode uses a "{}"
placeholder which doesn't match Python logging interpolation; update the
logger.warning invocation in debug_agent.py (the DebugAgent local_mode warning)
to use consistent logging formatting, e.g. replace the "{}" placeholder with a
printf-style "%s" and pass missing as a second argument
(logger.warning("DebugAgent local_mode: requested tools missing from ToolService
— %s", missing)) or use an f-string (logger.warning(f"DebugAgent local_mode:
requested tools missing from ToolService — {missing}")) so the message
interpolates correctly.
In `@app/modules/intelligence/tools/debug_tools/parse_debug_signal_tool.py`:
- Around line 117-131: _extract_test_info currently only uses Jest regexes
(_JEST_EXPECTED and _JEST_RECEIVED) so pytest-style diffs leave expected/actual
empty; update _extract_test_info to, after attempting the Jest matches, fall
back to the pytest regexes (e.g. _PYTEST_EXPECTED and _PYTEST_ACTUAL or whatever
the pytest-pattern names are) to populate expected and received when the Jest
patterns return None, keeping the same trimming/length limit behavior and
preserving test_path extraction via _TEST_PATH.
---
Nitpick comments:
In `@app/modules/intelligence/tools/debug_tools/watch_store.py`:
- Around line 29-34: remove_watch currently creates an empty _store entry for
non-existent keys; update remove_watch (use _key, _store, and _lock) to first
check if k is in _store while holding _lock, return [] immediately if missing,
otherwise filter the existing list, and then either assign the filtered list
back to _store[k] or delete _store[k] when the filtered result is empty so you
don't accumulate empty entries (preserve thread-safety by performing the check,
filter, assign/delete inside the same _lock block).
In `@app/modules/intelligence/tools/registry/definitions.py`:
- Around line 378-399: DEBUG_TOOLS is a manually maintained list that duplicates
keys already present in TOOL_DEFINITIONS; replace the hardcoded list by deriving
it from TOOL_DEFINITIONS to prevent drift by creating DEBUG_TOOLS from the
keys/definitions in TOOL_DEFINITIONS (e.g., filter keys that start with "debug_"
or match your debug predicate) while preserving the desired ordering and using
the same symbol name DEBUG_TOOLS so callers are unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab4acd48-5468-4610-ba2d-6a580561c91a
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
app/modules/intelligence/agents/chat_agents/system_agents/debug_agent.pyapp/modules/intelligence/agents/chat_agents/tool_helpers.pyapp/modules/intelligence/tools/debug_tools/__init__.pyapp/modules/intelligence/tools/debug_tools/add_watch_tool.pyapp/modules/intelligence/tools/debug_tools/build_debug_context_tool.pyapp/modules/intelligence/tools/debug_tools/debug_list_adapters_tool.pyapp/modules/intelligence/tools/debug_tools/debug_list_launch_configs_tool.pyapp/modules/intelligence/tools/debug_tools/debug_select_frame_tool.pyapp/modules/intelligence/tools/debug_tools/debug_snapshot_tool.pyapp/modules/intelligence/tools/debug_tools/debug_step_into_tool.pyapp/modules/intelligence/tools/debug_tools/debug_step_out_tool.pyapp/modules/intelligence/tools/debug_tools/debug_step_over_tool.pyapp/modules/intelligence/tools/debug_tools/debug_tunnel_utils.pyapp/modules/intelligence/tools/debug_tools/find_related_tests_tool.pyapp/modules/intelligence/tools/debug_tools/list_watches_tool.pyapp/modules/intelligence/tools/debug_tools/parse_debug_signal_tool.pyapp/modules/intelligence/tools/debug_tools/remove_watch_tool.pyapp/modules/intelligence/tools/debug_tools/watch_store.pyapp/modules/intelligence/tools/registry/definitions.pypyproject.tomlrequirements.txt
✅ Files skipped from review due to trivial changes (1)
- pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- app/modules/intelligence/tools/debug_tools/debug_snapshot_tool.py
- app/modules/intelligence/tools/debug_tools/debug_tunnel_utils.py
Comment on lines
+130
to
+133
| logger.warning( | ||
| "DebugAgent local_mode: requested tools missing from ToolService — {}", | ||
| missing, | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect logger factory implementation
fd -i "logger.py" app/modules/utils
rg -n -C3 "def setup_logger|logging.getLogger|loguru|structlog" app/modules/utils
# Inspect local usage in this file
rg -n -C2 "requested tools missing from ToolService|logger\\.warning\\(" app/modules/intelligence/agents/chat_agents/system_agents/debug_agent.pyRepository: potpie-ai/potpie
Length of output: 5587
Normalize warning interpolation to use consistent formatting.
Line 130–133 uses {} placeholder (str.format-style) with a positional argument, which is a format mismatch. The logger expects either f-string formatting or printf-style %s with positional args, not mixed styles.
Suggested fix
if missing:
- logger.warning(
- "DebugAgent local_mode: requested tools missing from ToolService — {}",
- missing,
- )
+ logger.warning(
+ f"DebugAgent local_mode: requested tools missing from ToolService — {missing}"
+ )📝 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.
Suggested change
| logger.warning( | |
| "DebugAgent local_mode: requested tools missing from ToolService — {}", | |
| missing, | |
| ) | |
| logger.warning( | |
| f"DebugAgent local_mode: requested tools missing from ToolService — {missing}" | |
| ) |
🧰 Tools
🪛 Ruff (0.15.12)
[error] 130-130: Too many arguments for logging format string
(PLE1205)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/modules/intelligence/agents/chat_agents/system_agents/debug_agent.py`
around lines 130 - 133, The warning call in DebugAgent's local_mode uses a "{}"
placeholder which doesn't match Python logging interpolation; update the
logger.warning invocation in debug_agent.py (the DebugAgent local_mode warning)
to use consistent logging formatting, e.g. replace the "{}" placeholder with a
printf-style "%s" and pass missing as a second argument
(logger.warning("DebugAgent local_mode: requested tools missing from ToolService
— %s", missing)) or use an f-string (logger.warning(f"DebugAgent local_mode:
requested tools missing from ToolService — {missing}")) so the message
interpolates correctly.
Comment on lines
+117
to
+131
| def _extract_test_info(text: str) -> Dict[str, Optional[str]]: | ||
| expected = received = test_path = None | ||
|
|
||
| m = _TEST_PATH.search(text) | ||
| if m: | ||
| test_path = m.group(1).strip() | ||
|
|
||
| m = _JEST_EXPECTED.search(text) | ||
| if m: | ||
| expected = m.group(1).strip()[:200] | ||
| m = _JEST_RECEIVED.search(text) | ||
| if m: | ||
| received = m.group(1).strip()[:200] | ||
|
|
||
| return {"expected": expected, "actual": received, "test_path": test_path} |
Contributor
There was a problem hiding this comment.
Add pytest fallback for expected/actual extraction.
Line 124–129 only use Jest patterns, so pytest-style diffs won’t populate expected/actual even though regexes already exist. Add fallback matching to avoid dropping key signal data.
💡 Proposed fix
def _extract_test_info(text: str) -> Dict[str, Optional[str]]:
expected = received = test_path = None
@@
m = _JEST_EXPECTED.search(text)
if m:
expected = m.group(1).strip()[:200]
+ else:
+ m = _PYTEST_EXPECTED.search(text)
+ if m:
+ expected = m.group(1).strip()[:200]
+
m = _JEST_RECEIVED.search(text)
if m:
received = m.group(1).strip()[:200]
+ else:
+ m = _PYTEST_RECEIVED.search(text)
+ if m:
+ received = m.group(1).strip()[:200]
return {"expected": expected, "actual": received, "test_path": test_path}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/modules/intelligence/tools/debug_tools/parse_debug_signal_tool.py` around
lines 117 - 131, _extract_test_info currently only uses Jest regexes
(_JEST_EXPECTED and _JEST_RECEIVED) so pytest-style diffs leave expected/actual
empty; update _extract_test_info to, after attempting the Jest matches, fall
back to the pytest regexes (e.g. _PYTEST_EXPECTED and _PYTEST_ACTUAL or whatever
the pytest-pattern names are) to populate expected and received when the Jest
patterns return None, keeping the same trimming/length limit behavior and
preserving test_path extraction via _TEST_PATH.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This change delivers the VS Code–tunnel–backed debug agent work end to end on the backend: 10 high-level debugger tools, registry and ToolService wiring, DebugAgent local_mode behavior and workflow prompt, UI progress strings, and automated tests that cover routing, per-tool behavior, and mock integration with the agent.
What’s included
Debugger tool package :
• 10 StructuredTool modules: debug_start, debug_stop, debug_set_breakpoints, debug_snapshot, debug_step_into, debug_step_out, debug_step_over, debug_continue, debug_select_frame, debug_list_sessions.
• debug_tunnel_utils.py: maps each operation to LocalServer /api/debug/* paths, applies operation-specific timeouts, normalizes { success, result?, error? }, and routes over socket tunnel or legacy HTTP tunnel (and optional local-server HTTP shortcut where the
environment resolves a reachable LocalServer without going through tunnel metadata).
• create_debug_tools() in init.py exposes a single registration surface for ToolService.
Registry & tooling:
• ToolCategory extended with "debug".
• TOOL_DEFINITIONS: entries for all 10 tools with local_mode_only: true (and read_only / idempotent on debug_list_sessions).
• DEBUG_TOOLS constant lists the canonical tool names.
• tool_service.py registers tools from create_debug_tools() alongside existing tunnel-backed helpers.
Debug agent:
• DebugAgent._build_agent(..., local_mode=...) appends debug_tools_prompt_section when local_mode is true.
• When local_mode, get_tools(...) is called again for debugger tools plus execute_terminal_command, search_text, search_files; missing names are logged.
• run / run_stream derive local_mode from ChatContext.
Product / UX copy:
• tool_helpers.py: get_tool_run_message() branches for every debug tool name for streaming/progress UX.
Registry population:
• population.py: debug is handled so annotations come from TOOL_DEFINITIONS (no accidental derivation as terminal/search).
Tests:
• test_debug_tunnel_utils.py: formatting + route_debug_command with mocks (no tunnel parameterized for all operations, socket success/errors, legacy HTTP tunnel).
• test_debug_structured_tools.py: per-tool assertions that payloads and route_debug_command invocation match expectations; tunnel-unavailable and missing-user behaviors for all 10 tools.
• test_debug_agent_integration.py: mock integration checks that local_mode changes the get_tools call shape and documents a start → breakpoints → snapshot sequence at the routing layer under socket stubs.
Summary by CodeRabbit
New Features
Infrastructure
Tests