feat: project-level agent discovery with TUI source display and shadow warnings#187
Open
benkearsley wants to merge 15 commits intompfaffenberger:mainfrom
Open
feat: project-level agent discovery with TUI source display and shadow warnings#187benkearsley wants to merge 15 commits intompfaffenberger:mainfrom
benkearsley wants to merge 15 commits intompfaffenberger:mainfrom
Conversation
added 5 commits
February 18, 2026 11:34
Adds discover_json_agents_with_sources() which merges user-level (~/.code_puppy/agents/) and project-level (.code_puppy/agents/) JSON agents, with project-level taking precedence on name collision. Adds get_agent_source_path() to expose the file path of any registered JSON agent, and updates the public __init__ exports accordingly. Updates README to document both agent directories and the override semantics.
Agent creator now explicitly asks whether to save the new agent to the user-level directory (~/.code_puppy/agents/) or the project-level directory (.code_puppy/agents/) before writing the file, using ask_user_question with arrow selection. Also fixes path expansion — previously ~ was used literally, which created a ~/directory inside the project root instead of the real home directory.
…prompts The agent details panel now shows the source file path for JSON agents. When a project-level agent overrides a user-level agent of the same name, a shadow warning is displayed so the user knows which one is active. Clone operations now prompt for destination (user vs project directory) using arrow_select_async rather than silently cloning alongside the source. Any JSON agent (user or project level) can now be deleted — the previous restriction to clone-only agents has been removed.
…s, TypedDict 🌀 Magic applied with Wibey CLI 🪄 (https://wibey.walmart.com/cli)
…dTuple 🌀 Magic applied with Wibey CLI 🪄 (https://wibey.walmart.com/cli)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds explicit user- and project-level JSON agent directories with override/shadow semantics, returns rich discovery metadata (source + shadowed path), exposes agent source helpers, updates cloning/deletion to choose/enforce target directories, shifts agent-creation to prompt-driven saves, updates CLI to show path/shadow metadata, and adds tests and README edits. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AgentMenu as Agent Menu UI
participant Discovery as Agent Discovery
participant UserDir as User Agent Dir
participant ProjectDir as Project Agent Dir
participant Registry as Agent Registry
User->>AgentMenu: Request agent list
AgentMenu->>Discovery: get_available_agents_with_descriptions()
Discovery->>UserDir: scan ~/.code_puppy/agents/
UserDir-->>Discovery: return user JSON agents
Discovery->>ProjectDir: scan .code_puppy/agents/
ProjectDir-->>Discovery: return project JSON agents
Discovery->>Discovery: merge results, mark source and shadowed_path
Discovery-->>AgentMenu: return AgentEntry list with metadata
AgentMenu-->>User: render menu (shows source/path/shadow)
sequenceDiagram
participant User
participant Menu as Agent Menu
participant Manager as Agent Manager
participant Prompt as Location Prompt
participant TargetDir as Target Directory
participant Filesystem as Filesystem
User->>Menu: initiate clone of agent "X"
Menu->>Manager: clone_agent("X")
Manager->>Prompt: _ask_clone_location() or use target_dir
Prompt->>User: choose Project or User directory
User-->>Prompt: select Project
Prompt-->>Manager: return project path
Manager->>Filesystem: write cloned JSON into project path
Filesystem-->>Manager: created clone file path
Manager-->>Menu: return clone path
Menu-->>User: display success with clone path
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
added 2 commits
February 18, 2026 14:44
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/agents/test_agent_creator_agent.py (1)
59-83:⚠️ Potential issue | 🟡 Minor
get_project_agents_directoryis not mocked — test behavior is environment-dependent.
get_system_prompt()callsget_project_agents_directory()(line 30 ofagent_creator_agent.py). Without a mock, the test invokes the real function, which inspectsos.getcwd()/.code_puppy/agents. If the test runner's working directory contains that path,project_agents_dirwill be a real absolute path rather than the fallback string, making the test non-deterministic.🛡️ Proposed fix
+ monkeypatch.setattr( + "code_puppy.agents.agent_creator_agent.get_project_agents_directory", + lambda: None, + ) monkeypatch.setattr( "code_puppy.agents.agent_creator_agent.get_user_agents_directory", lambda: mock_dir, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/agents/test_agent_creator_agent.py` around lines 59 - 83, The test is not mocking get_project_agents_directory, making behavior environment-dependent; update the test for AgentCreatorAgent.get_system_prompt to monkeypatch "code_puppy.agents.agent_creator_agent.get_project_agents_directory" to return a deterministic path (e.g., the same mock_dir or a known fallback) so prompt composition is stable; ensure the existing mocks (get_available_tool_names, get_user_agents_directory, ModelFactory.load_config) remain and then assert the expected strings in prompt as before.code_puppy/command_line/agent_menu.py (1)
554-557:⚠️ Potential issue | 🟡 MinorStale return-type annotation —
get_current_entry()saysTuple[str, str, str]butentriesis nowList[AgentEntry](5-tuple).
AgentEntryhas five fields (name,display_name,description,source_path,shadowed_path). The annotation was not updated whenAgentEntryreplaced the previous 3-tuple, so type-checkers and readers are misled.🛡️ Proposed fix
- def get_current_entry() -> Optional[Tuple[str, str, str]]: + def get_current_entry() -> Optional[AgentEntry]: if 0 <= selected_idx[0] < len(entries): return entries[selected_idx[0]] return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/agent_menu.py` around lines 554 - 557, The return-type annotation on get_current_entry is stale (it declares Optional[Tuple[str, str, str]] while entries is now List[AgentEntry] with five fields); update get_current_entry's signature to return Optional[AgentEntry] (or Optional[Tuple[str,str,str,str,str]] if you prefer tuples) and adjust any callers expecting a 3-tuple to use AgentEntry.name/display_name/description/source_path/shadowed_path accordingly; ensure AgentEntry is imported or in scope where get_current_entry is defined and update the annotation in the def line for get_current_entry.
🧹 Nitpick comments (8)
code_puppy/agents/json_agent.py (1)
13-16: LGTM —AgentSourceInfoTypedDict is well-typed.Minor style note: in
agent_manager.pyline 511,info.get("shadowed_path")is called on aTypedDictinstance whereshadowed_pathis a declared required field. Since the key is always present,info["shadowed_path"]is more idiomatic and clearer to type checkers (.get()implies optionality at the key level, not just the value level).♻️ Proposed change in
agent_manager.pyline 511- return info.get("shadowed_path") + return info["shadowed_path"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/agents/json_agent.py` around lines 13 - 16, Replace the use of info.get("shadowed_path") with direct key access info["shadowed_path"] because AgentSourceInfo declares shadowed_path as a required field; locate the usage in the agent manager code where AgentSourceInfo instances are read (search for info.get("shadowed_path") in the agent manager/handler that deals with AgentSourceInfo) and change to info["shadowed_path"] to reflect required-key semantics and satisfy type checkers.code_puppy/command_line/agent_menu.py (2)
307-332: Consider cachingdiscover_json_agents_with_sources()to avoid a redundant second filesystem scan per menu refresh.
get_available_agents_with_descriptions()(line 317) already calls_discover_agents()→discover_json_agents()→discover_json_agents_with_sources()internally. Callingdiscover_json_agents_with_sources()again on line 322 means everyrefresh_entries()triggers two full directory scans. The current code correctly documents this ("two filesystem scans instead of three"), but the second scan could be avoided by plumbing the source info through the registry or_discover_agents().This is informational — the two-scan approach is acceptable at current scale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/agent_menu.py` around lines 307 - 332, The current _get_agent_entries calls discover_json_agents_with_sources() a second time causing a redundant filesystem scan; fix by plumbing or caching the source info so the second call is avoided: modify get_available_agents_with_descriptions or _discover_agents to return (or store) the source dict from discover_json_agents_with_sources and have _get_agent_entries consume that value instead of calling discover_json_agents_with_sources() again, or add a simple module-level cache that discover_json_agents_with_sources() populates and _get_agent_entries reads; update refresh_entries to use the new plumbing/cache so only a single discover_json_agents_with_sources() scan runs per refresh.
1-51: Advisory: this file is incode_puppy/command_line/, which the project guidelines flag as off-limits for new functionality.The learned guideline states new functionality should live under
code_puppy/plugins/and hook in viacode_puppy/callbacks.pyrather than editingcode_puppy/command_line/. The UI changes here are tightly coupled to the new discovery feature — if there's an opportunity to extract the shadow-display and location-selection logic into a plugin that hooks into the menu via a callback, that would better align with the architecture.Based on learnings: "Nearly all new functionality should live in a plugin under
code_puppy/plugins/and hook into core via lifecycle callbacks incode_puppy/callbacks.pyrather than editing files incode_puppy/command_line/or touching the core agent loop".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/agent_menu.py` around lines 1 - 51, This file adds UI-level discovery/location-selection logic inside command_line.agent_menu (see AgentEntry, _USER_DIR_CHOICE, _PROJECT_DIR_CHOICE, and PAGE_SIZE) which violates the plugin-guideline; extract the shadow-display and location-selection behavior into a new plugin under code_puppy/plugins (implementing the logic that currently references user/project choices and shadowed_path) and expose a lifecycle callback in code_puppy/callbacks.py that the existing VSplit/agent menu UI in agent_menu.py can call; remove the new discovery/location-selection code from agent_menu.py so agent_menu retains only core UI rendering and invoke the plugin via the new callback (wire up get_available_agents_with_descriptions/clone_agent/delete_clone_agent interactions through the callback interface).code_puppy/agents/agent_manager.py (2)
814-827:delete_clone_agentis a misleading name — the implementation now deletes any JSON agent.The docstring and logic were correctly updated (no longer checks
is_clone_agent_name), but the function name implies clone-only deletion. External callers relying on the name alone may misuse or avoid it. Consider renaming todelete_json_agentand updating the one export in__init__.py.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/agents/agent_manager.py` around lines 814 - 827, The function delete_clone_agent now deletes any JSON agent but its name is misleading; rename the function delete_json_agent (update the def name in agent_manager: delete_clone_agent -> delete_json_agent), update the export/import in the package __init__.py to expose delete_json_agent instead of delete_clone_agent, and update all internal call sites and references (tests, other modules) to use delete_json_agent so names are consistent with the docstring and behavior.
650-667: Pre-existing regex bug in_next_clone_index:(\\d+)in a raw f-string is\\d+(matches literal\d), not\d+(matches digits).The pattern never matches existing clone names, so
indicesis always empty and the next index always starts from 1, relying solely on thewhileloop's file-existence check to skip occupied slots. Functionally correct but degrades linearly with the number of existing clones. The fix is a one-character change.♻️ Proposed fix
- clone_pattern = re.compile(rf"^{re.escape(base_name)}-clone-(\\d+)$") + clone_pattern = re.compile(rf"^{re.escape(base_name)}-clone-(\d+)$")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/agents/agent_manager.py` around lines 650 - 667, The regex in _next_clone_index is incorrectly escaping digits: update the clone_pattern definition (variable clone_pattern) to use a single backslash digit class so it matches digits (e.g., rf"^{re.escape(base_name)}-clone-(\d+)$" or use a raw string r'\d+' in the pattern) instead of the current "(\\d+)" so existing clone names are properly detected and indices populated.tests/agents/test_agent_cloning.py (1)
50-52: Redundant manual_AGENT_REGISTRYmanipulation —clone_agent()calls_discover_agents()internally, which clears and repopulates the registry using the already-monkeypatched config functions.The source agent is discovered automatically via the mocked
get_user_agents_directory. The explicit_AGENT_REGISTRY["source-agent"] = ...at line 52 is overwritten beforeclone_agent()uses it.test_clone_to_user_directorydoes_AGENT_REGISTRY.clear()first (line 106) which is also redundant for the same reason. The tests pass correctly, but the setup code is misleading about how the discovery actually works.Also applies to: 104-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/agents/test_agent_cloning.py` around lines 50 - 52, Remove the redundant manual manipulation of the agent registry: delete assignments to _AGENT_REGISTRY["source-agent"] and any explicit _AGENT_REGISTRY.clear() calls in the tests (e.g., in test_clone_to_user_directory), since clone_agent() calls _discover_agents() which repopulates _AGENT_REGISTRY using the already-monkeypatched get_user_agents_directory; rely on the mocked get_user_agents_directory to expose the source agent instead of setting or clearing _AGENT_REGISTRY manually.tests/command_line/test_agent_menu.py (2)
975-991: Prefer named attribute access for readability.Lines 990–991 use positional index access (
result[0][3],result[0][4]) while the rest of the file (e.g., line 972–973) uses named attributes (result[0].source_path). Mixing styles hurts readability.Suggested diff
- assert result[0][3] == project_path - assert result[0][4] == user_path + assert result[0].source_path == project_path + assert result[0].shadowed_path == user_path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/command_line/test_agent_menu.py` around lines 975 - 991, Update the test_get_agent_entries_includes_shadowed_path assertion to use the named attributes returned by _get_agent_entries instead of positional indexes; replace result[0][3] and result[0][4] with the equivalent attribute access (e.g., result[0].source_path and result[0].shadowed_path or whatever attribute names _get_agent_entries yields) so the test matches the style used elsewhere in the file and improves readability.
1081-1095: Delete guard coverage is one-sided — consider adding a positive-path test.Currently only the "non-clone rejected" path is tested. A test that verifies
_handle_delete_actiondelegates todelete_clone_agentwhenis_clone_agent_namereturnsTruewould increase confidence in the happy path. Not blocking, but worth adding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/command_line/test_agent_menu.py` around lines 1081 - 1095, Add a positive-path unit test in TestDeleteGuard that patches is_clone_agent_name to return True and asserts _handle_delete_action delegates to delete_clone_agent: patch delete_clone_agent and emit_warning, call _handle_delete_action("my-json-agent", current_agent_name="code-puppy"), assert delete_clone_agent was called once with the expected agent name (and any other expected args), assert emit_warning was not called, and assert the return value indicates success (e.g., True or the delete_clone_agent return value).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/agents/agent_creator_agent.py`:
- Around line 402-429: The system prompt JSON in agent_creator_agent.py uses the
wrong camelCase key "multiSelect" — update that key to snake_case "multi_select"
so it matches the Question Pydantic model in
code_puppy/tools/ask_user_question/models.py; locate the JSON passed to
ask_user_question (the prompt string in agent_creator_agent.py that starts with
the {"questions":[{"question":"Where should this agent be saved?",...}]}),
change all "multiSelect" occurrences to "multi_select", and verify the code path
that calls ask_user_question and any references in agent_manager.py still work
with the updated key; ensure the user-facing behavior remains the same and
notify the user of successful creation as before.
In `@code_puppy/agents/agent_manager.py`:
- Around line 480-490: get_agent_source_path currently reads _AGENT_REGISTRY
directly and can return stale None for JSON agents; modify get_agent_source_path
to call _discover_agents() at the start (same pattern used by
get_available_agents / get_agent_descriptions) so the registry is populated
before checking _AGENT_REGISTRY, then keep the existing isinstance(ref, str)
check and return Optional[str] as before; ensure you do not change the public
signature or behavior for Python agents (still return None).
- Around line 697-708: The system prompt in agent_creator_agent.py (around the
code that builds the LLM-generated JSON at line ~409) currently emits
"multiSelect": false (camelCase) but ask_user_question expects "multi_select"
(snake_case); update the prompt/template so the generated payload uses
"multi_select": false instead of "multiSelect" to match the Pydantic model used
by ask_user_question, and verify any other LLM-generated keys follow the
snake_case naming convention to avoid silent validation mismatches.
In `@code_puppy/command_line/agent_menu.py`:
- Around line 206-211: The if-check guarding against a None user_dir in
agent_menu.clone logic is dead because get_user_agents_directory() (in
code_puppy/config.py) always creates and returns a non-empty AGENTS_DIR; remove
the unreachable guard: delete the lines that test "if user_dir is None", the
emit_info("No user agents directory configured — clone cancelled.") call and the
subsequent "return None", leaving the existing assignments project_dir =
get_project_agents_directory() and user_dir = get_user_agents_directory() and
continuing normal flow in the clone function (refer to get_user_agents_directory
and the clone code around where project_dir and user_dir are used).
In `@tests/agents/test_agent_cloning.py`:
- Around line 4-7: Delete the unused imports to satisfy ruff: remove Path, Mock,
and pytest from the top-level import list (the existing "from pathlib import
Path", "from unittest.mock import Mock", and "import pytest" statements) in
tests/agents/test_agent_cloning.py; ensure only the actually used imports remain
and run the test linter to confirm no further unused-import errors.
In `@tests/agents/test_agent_creator_agent.py`:
- Around line 227-269: Mock out ModelFactory.load_config and
get_user_agents_directory in both tests to avoid touching the real filesystem or
reading real configs: in each test call
monkeypatch.setattr("code_puppy.config.ModelFactory.load_config", lambda
self=None: {}) (or return a minimal dict) and
monkeypatch.setattr("code_puppy.agents.agent_creator_agent.get_user_agents_directory",
lambda: None or str(tmp_path / ".code_puppy" / "agents")) so the tests do not
call os.makedirs or read real config files; apply the same two monkeypatches to
both test_system_prompt_contains_expanded_project_path and
test_system_prompt_fallback_when_no_project_directory.
---
Outside diff comments:
In `@code_puppy/command_line/agent_menu.py`:
- Around line 554-557: The return-type annotation on get_current_entry is stale
(it declares Optional[Tuple[str, str, str]] while entries is now
List[AgentEntry] with five fields); update get_current_entry's signature to
return Optional[AgentEntry] (or Optional[Tuple[str,str,str,str,str]] if you
prefer tuples) and adjust any callers expecting a 3-tuple to use
AgentEntry.name/display_name/description/source_path/shadowed_path accordingly;
ensure AgentEntry is imported or in scope where get_current_entry is defined and
update the annotation in the def line for get_current_entry.
In `@tests/agents/test_agent_creator_agent.py`:
- Around line 59-83: The test is not mocking get_project_agents_directory,
making behavior environment-dependent; update the test for
AgentCreatorAgent.get_system_prompt to monkeypatch
"code_puppy.agents.agent_creator_agent.get_project_agents_directory" to return a
deterministic path (e.g., the same mock_dir or a known fallback) so prompt
composition is stable; ensure the existing mocks (get_available_tool_names,
get_user_agents_directory, ModelFactory.load_config) remain and then assert the
expected strings in prompt as before.
---
Nitpick comments:
In `@code_puppy/agents/agent_manager.py`:
- Around line 814-827: The function delete_clone_agent now deletes any JSON
agent but its name is misleading; rename the function delete_json_agent (update
the def name in agent_manager: delete_clone_agent -> delete_json_agent), update
the export/import in the package __init__.py to expose delete_json_agent instead
of delete_clone_agent, and update all internal call sites and references (tests,
other modules) to use delete_json_agent so names are consistent with the
docstring and behavior.
- Around line 650-667: The regex in _next_clone_index is incorrectly escaping
digits: update the clone_pattern definition (variable clone_pattern) to use a
single backslash digit class so it matches digits (e.g.,
rf"^{re.escape(base_name)}-clone-(\d+)$" or use a raw string r'\d+' in the
pattern) instead of the current "(\\d+)" so existing clone names are properly
detected and indices populated.
In `@code_puppy/agents/json_agent.py`:
- Around line 13-16: Replace the use of info.get("shadowed_path") with direct
key access info["shadowed_path"] because AgentSourceInfo declares shadowed_path
as a required field; locate the usage in the agent manager code where
AgentSourceInfo instances are read (search for info.get("shadowed_path") in the
agent manager/handler that deals with AgentSourceInfo) and change to
info["shadowed_path"] to reflect required-key semantics and satisfy type
checkers.
In `@code_puppy/command_line/agent_menu.py`:
- Around line 307-332: The current _get_agent_entries calls
discover_json_agents_with_sources() a second time causing a redundant filesystem
scan; fix by plumbing or caching the source info so the second call is avoided:
modify get_available_agents_with_descriptions or _discover_agents to return (or
store) the source dict from discover_json_agents_with_sources and have
_get_agent_entries consume that value instead of calling
discover_json_agents_with_sources() again, or add a simple module-level cache
that discover_json_agents_with_sources() populates and _get_agent_entries reads;
update refresh_entries to use the new plumbing/cache so only a single
discover_json_agents_with_sources() scan runs per refresh.
- Around line 1-51: This file adds UI-level discovery/location-selection logic
inside command_line.agent_menu (see AgentEntry, _USER_DIR_CHOICE,
_PROJECT_DIR_CHOICE, and PAGE_SIZE) which violates the plugin-guideline; extract
the shadow-display and location-selection behavior into a new plugin under
code_puppy/plugins (implementing the logic that currently references
user/project choices and shadowed_path) and expose a lifecycle callback in
code_puppy/callbacks.py that the existing VSplit/agent menu UI in agent_menu.py
can call; remove the new discovery/location-selection code from agent_menu.py so
agent_menu retains only core UI rendering and invoke the plugin via the new
callback (wire up
get_available_agents_with_descriptions/clone_agent/delete_clone_agent
interactions through the callback interface).
In `@tests/agents/test_agent_cloning.py`:
- Around line 50-52: Remove the redundant manual manipulation of the agent
registry: delete assignments to _AGENT_REGISTRY["source-agent"] and any explicit
_AGENT_REGISTRY.clear() calls in the tests (e.g., in
test_clone_to_user_directory), since clone_agent() calls _discover_agents()
which repopulates _AGENT_REGISTRY using the already-monkeypatched
get_user_agents_directory; rely on the mocked get_user_agents_directory to
expose the source agent instead of setting or clearing _AGENT_REGISTRY manually.
In `@tests/command_line/test_agent_menu.py`:
- Around line 975-991: Update the test_get_agent_entries_includes_shadowed_path
assertion to use the named attributes returned by _get_agent_entries instead of
positional indexes; replace result[0][3] and result[0][4] with the equivalent
attribute access (e.g., result[0].source_path and result[0].shadowed_path or
whatever attribute names _get_agent_entries yields) so the test matches the
style used elsewhere in the file and improves readability.
- Around line 1081-1095: Add a positive-path unit test in TestDeleteGuard that
patches is_clone_agent_name to return True and asserts _handle_delete_action
delegates to delete_clone_agent: patch delete_clone_agent and emit_warning, call
_handle_delete_action("my-json-agent", current_agent_name="code-puppy"), assert
delete_clone_agent was called once with the expected agent name (and any other
expected args), assert emit_warning was not called, and assert the return value
indicates success (e.g., True or the delete_clone_agent return value).
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)
code_puppy/command_line/agent_menu.py (1)
558-561:⚠️ Potential issue | 🟡 MinorStale return-type annotation on
get_current_entry().
entriesisList[AgentEntry](a 5-field NamedTuple), so the annotationOptional[Tuple[str, str, str]]is wrong. Every key-binding handler that callsget_current_entry()will be given incorrect type information.🔧 Proposed fix
- def get_current_entry() -> Optional[Tuple[str, str, str]]: + def get_current_entry() -> Optional[AgentEntry]: if 0 <= selected_idx[0] < len(entries): return entries[selected_idx[0]] return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/agent_menu.py` around lines 558 - 561, The return-type annotation of get_current_entry() is wrong: entries is List[AgentEntry] (a 5-field NamedTuple), so update the function signature to return Optional[AgentEntry] (or the exact AgentEntry type alias) instead of Optional[Tuple[str, str, str]]; locate get_current_entry() and the entries variable in agent_menu.py and change the annotation and any related type imports/usages to reference AgentEntry so callers receive correct type information.
🧹 Nitpick comments (2)
tests/command_line/test_agent_menu.py (1)
1099-1104: Prefer named attribute access onAgentEntryfor consistency.Lines 1103–1104 use positional indexing (
result[0][3],result[0][4]), while the very similar test at lines 1076–1077 uses.source_path/.shadowed_path. Named access is clearer and less fragile if the field order ever changes.Suggested change
- assert result[0][3] == project_path - assert result[0][4] == user_path + assert result[0].source_path == project_path + assert result[0].shadowed_path == user_path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/command_line/test_agent_menu.py` around lines 1099 - 1104, Replace positional tuple indexing with attribute access on the AgentEntry objects returned by _get_agent_entries: instead of asserting result[0][3] and result[0][4], assert result[0].source_path and result[0].shadowed_path equal project_path and user_path respectively; update the two assertions in the test block that currently reference result[0][3] and result[0][4] to use result[0].source_path and result[0].shadowed_path to match the other assertions at lines 1076–1077.code_puppy/agents/agent_manager.py (1)
720-729: Misleadingtarget_dirdocstring.The inline comment says "pass the source agent's parent directory to avoid nested TUI conflicts", but
target_diris the destination directory for the clone (user or project agents dir). The confusion with the source directory could lead future maintainers to pass the wrong value.📝 Proposed fix
- target_dir: Directory to save the clone. If None, asks the user - interactively (only works outside of a TUI context). - When called from the agent menu, pass the source agent's - parent directory to avoid nested TUI conflicts. + target_dir: Destination directory in which to write the clone JSON + file. If None, prompts the user interactively (only valid outside + a TUI context). Pass the directory chosen by _select_clone_location() + when invoking from within the agent menu to avoid nested TUI prompts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/agents/agent_manager.py` around lines 720 - 729, The docstring for clone_agent is misleading about target_dir; update the description to clearly state that target_dir is the destination directory where the cloned agent will be saved (not the source), and clarify the guidance about TUI conflicts by saying: when invoking from the agent menu, pass the destination directory that is the same parent as the source agent's parent (i.e., the source agent's parent directory) only to avoid nested TUI contexts—make this change in the clone_agent function docstring so callers correctly pass a destination path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/agents/agent_manager.py`:
- Around line 710-717: In _ask_clone_location() handle two edge cases: first,
guard access to result.answers[0].selected_options by checking that
result.answers and result.answers[0].selected_options are non-empty before
indexing, returning None if missing; second, avoid calling Path(None) by
ensuring when choice == _PROJECT_DIR_LABEL that project_dir is truthy (e.g., if
not project_dir then fall back to get_user_agents_directory() or return None);
update the selection logic around variables result, selected_options, choice,
_PROJECT_DIR_LABEL, project_dir and get_user_agents_directory() so a valid path
string is always passed to Path().
In `@tests/agents/test_agent_cloning.py`:
- Around line 45-48: The test modifies the global _AGENT_REGISTRY without
clearing it, risking cross-test pollution; update the
test_clone_to_project_directory setup to call _AGENT_REGISTRY.clear() before
assigning _AGENT_REGISTRY["source-agent"] = str(source_agent_path) so the
registry is empty and clone numbering is deterministic; ensure the clear happens
in the same test function (or its fixture) before the assignment to
_AGENT_REGISTRY.
- Around line 71-117: Mock get_project_agents_directory in this test to avoid
non-deterministic discovery: ensure that when clone_agent triggers
_discover_agents → discover_json_agents → discover_json_agents_with_sources it
does not read the real CWD; add a monkeypatch for
code_puppy.config.get_project_agents_directory to either return None or the test
tmp_path (as done in test_clone_to_project_directory) so project agent discovery
is disabled or deterministic for this test.
---
Outside diff comments:
In `@code_puppy/command_line/agent_menu.py`:
- Around line 558-561: The return-type annotation of get_current_entry() is
wrong: entries is List[AgentEntry] (a 5-field NamedTuple), so update the
function signature to return Optional[AgentEntry] (or the exact AgentEntry type
alias) instead of Optional[Tuple[str, str, str]]; locate get_current_entry() and
the entries variable in agent_menu.py and change the annotation and any related
type imports/usages to reference AgentEntry so callers receive correct type
information.
---
Duplicate comments:
In `@code_puppy/agents/agent_creator_agent.py`:
- Around line 402-429: The JSON payload sent to ask_user_question uses the wrong
key "multiSelect" — update the payload in agent_creator_agent.py (the code that
builds the questions object before calling ask_user_question) to use
"multi_select" (snake_case) to match the Question Pydantic model in
code_puppy/tools/ask_user_question/models.py; keep the rest of the structure the
same, ensure the conditional logic that omits the "Project directory" option
when .code_puppy/agents/ doesn't exist still works (use agents_dir and
project_agents_dir variables), then continue using edit_file to write the chosen
path ({agents_dir}/agent-name.json or {project_agents_dir}/agent-name.json) and
notify the user with the full expanded path on success.
In `@code_puppy/agents/agent_manager.py`:
- Around line 480-490: get_agent_source_path reads from _AGENT_REGISTRY but may
be called before agents are discovered; call _discover_agents() (or otherwise
ensure registry is populated) at the start of get_agent_source_path so JSON
agents on disk are found even if discovery hasn't run yet. Update
get_agent_source_path to invoke _discover_agents() (or check a cached/discovered
flag) before accessing _AGENT_REGISTRY, referencing the existing
_discover_agents and _AGENT_REGISTRY symbols so behavior is consistent with
other discovery-triggering functions.
In `@code_puppy/command_line/agent_menu.py`:
- Around line 209-211: The if-check guarding on user_dir being None is dead code
because get_user_agents_directory() always returns a created AGENTS_DIR; remove
the unreachable branch (the "if user_dir is None: emit_info(...); return None")
from the clone flow in agent_menu.py and rely on get_user_agents_directory()
directly (or if you prefer an explicit safety check, raise an AssertionError
referencing get_user_agents_directory and user_dir to signal an unexpected
invariant violation); update any callers/comments to reflect that user_dir is
non-null.
In `@tests/agents/test_agent_creator_agent.py`:
- Around line 227-269: The tests
test_system_prompt_contains_expanded_project_path and
test_system_prompt_fallback_when_no_project_directory cause filesystem
side-effects because ModelFactory.load_config and get_user_agents_directory are
not mocked; update both tests to monkeypatch
"code_puppy.models.ModelFactory.load_config" to a no-op or stubbed return and
"code_puppy.agents.agent_creator_agent.get_user_agents_directory" to a safe
lambda (e.g. returning None or a temp path) so no real ~/.code_puppy/agents is
created during AgentCreatorAgent().get_system_prompt() execution; keep existing
monkeypatches for get_project_agents_directory and get_available_tool_names and
ensure the patched names match the functions ModelFactory.load_config and
get_user_agents_directory used by AgentCreatorAgent.
---
Nitpick comments:
In `@code_puppy/agents/agent_manager.py`:
- Around line 720-729: The docstring for clone_agent is misleading about
target_dir; update the description to clearly state that target_dir is the
destination directory where the cloned agent will be saved (not the source), and
clarify the guidance about TUI conflicts by saying: when invoking from the agent
menu, pass the destination directory that is the same parent as the source
agent's parent (i.e., the source agent's parent directory) only to avoid nested
TUI contexts—make this change in the clone_agent function docstring so callers
correctly pass a destination path.
In `@tests/command_line/test_agent_menu.py`:
- Around line 1099-1104: Replace positional tuple indexing with attribute access
on the AgentEntry objects returned by _get_agent_entries: instead of asserting
result[0][3] and result[0][4], assert result[0].source_path and
result[0].shadowed_path equal project_path and user_path respectively; update
the two assertions in the test block that currently reference result[0][3] and
result[0][4] to use result[0].source_path and result[0].shadowed_path to match
the other assertions at lines 1076–1077.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code_puppy/command_line/agent_menu.py (1)
560-563:⚠️ Potential issue | 🟡 MinorStale return-type annotation — should be
Optional[AgentEntry], notOptional[Tuple[str, str, str]].
entriesisList[AgentEntry](a 5-field NamedTuple), so the return type is wrong. This will mislead type checkers and downstream readers.♻️ Suggested fix
- def get_current_entry() -> Optional[Tuple[str, str, str]]: + def get_current_entry() -> Optional[AgentEntry]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/agent_menu.py` around lines 560 - 563, The return type annotation on get_current_entry is stale: it declares Optional[Tuple[str, str, str]] but entries holds AgentEntry (a 5-field NamedTuple); update the annotation to Optional[AgentEntry] and ensure any necessary imports/reference to AgentEntry are present so the function signature reads get_current_entry() -> Optional[AgentEntry] and accurately reflects the actual entries element type.
🧹 Nitpick comments (3)
code_puppy/command_line/agent_menu.py (1)
329-335: Inconsistent dict access:info["path"]will raiseKeyErrorif the key is ever absent, whileinfo.get("shadowed_path")is safe.Use
.get()consistently for both lookups to be defensive.♻️ Suggested fix
- source_path = info["path"] if info else None + source_path = info.get("path") if info else None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/agent_menu.py` around lines 329 - 335, The code uses direct indexing on info["path"] which can raise KeyError while info.get("shadowed_path") is used safely; update the loop that builds AgentEntry instances (where agents_metadata, source_info, and the local variable info are used) to obtain both source_path and shadowed_path via info.get("path") and info.get("shadowed_path") respectively, so both lookups are defensive before constructing the AgentEntry(name, display_name, description, source_path, shadowed_path).tests/agents/test_agent_cloning.py (1)
162-178: Mock config paths to isolate discovery from the host filesystem.
clone_agent()calls_discover_agents()which scans real user/project directories. While the assertion holds (the agent truly doesn't exist), this leaks host state into the test. Add the same config mocks used by the other tests.🛡️ Proposed fix
def test_clone_nonexistent_agent(self, monkeypatch): """Test cloning a non-existent agent.""" # Mock empty registry from code_puppy.agents.agent_manager import _AGENT_REGISTRY _AGENT_REGISTRY.clear() + monkeypatch.setattr( + "code_puppy.config.get_project_agents_directory", lambda: None + ) + monkeypatch.setattr( + "code_puppy.config.get_user_agents_directory", + lambda: str(monkeypatch.tmp_path if hasattr(monkeypatch, 'tmp_path') else "/tmp/empty"), + ) + # Mock _ask_clone_location (shouldn't be called)A simpler alternative: accept
tmp_pathin the test signature and use it:- def test_clone_nonexistent_agent(self, monkeypatch): + def test_clone_nonexistent_agent(self, tmp_path, monkeypatch): """Test cloning a non-existent agent.""" from code_puppy.agents.agent_manager import _AGENT_REGISTRY _AGENT_REGISTRY.clear() + + monkeypatch.setattr( + "code_puppy.config.get_project_agents_directory", lambda: None + ) + monkeypatch.setattr( + "code_puppy.config.get_user_agents_directory", + lambda: str(tmp_path), + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/agents/test_agent_cloning.py` around lines 162 - 178, The test leaks host filesystem state because clone_agent() invokes _discover_agents(); update the test_clone_nonexistent_agent to mock the config discovery paths the same way other tests do (or accept a tmp_path fixture and point config dirs there) so _discover_agents() scans an isolated temp location. Specifically, before clearing _AGENT_REGISTRY and monkeypatching _ask_clone_location, set the config path globals or functions used by _discover_agents (the same config mocks used in other tests) to point to tmp_path or temporary directories so discovery is isolated when calling clone_agent("nonexistent-agent").code_puppy/agents/agent_manager.py (1)
562-599: Consider delegatingget_available_agents/get_agent_descriptionsto this function to reduce duplication.This new consolidated helper is a good idea, but
get_available_agents()(lines 328-370) andget_agent_descriptions()(lines 517-559) duplicate essentially the same config-check / discover / iterate / instantiate pattern. Each triggers its own_discover_agents()call and repeats the pack/UC filter logic. Over time this is likely to diverge.If backward compatibility is needed, the older functions can thin-wrap this one:
♻️ Possible consolidation
def get_available_agents() -> Dict[str, str]: return {name: dn for name, (dn, _) in get_available_agents_with_descriptions().items()} def get_agent_descriptions() -> Dict[str, str]: return {name: desc for name, (_, desc) in get_available_agents_with_descriptions().items()}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/agents/agent_manager.py` around lines 562 - 599, Replace duplicated discovery/filter/instantiate logic by making get_available_agents() and get_agent_descriptions() thin wrappers around get_available_agents_with_descriptions(): call get_available_agents_with_descriptions(), then map its returned Dict[str, Tuple[str,str]] to the expected Dict[str,str] shape (extracting display_name or description respectively) instead of re-calling _discover_agents() or re-implementing PACK_AGENT_NAMES / UC_AGENT_NAMES checks; keep get_available_agents_with_descriptions() as the single source of truth for discovery and filtering so _discover_agents(), PACK_AGENT_NAMES and UC_AGENT_NAMES logic aren’t duplicated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/agents/test_agent_cloning.py`:
- Around line 123-160: The test is currently exercising the "agent not found"
early exit because clone_agent() calls _discover_agents(), which repopulates
_AGENT_REGISTRY from the real user agents directory; mock
get_user_agents_directory so discovery points at the tmp_path user_agents folder
(same as other tests) before calling clone_agent(), ensuring _AGENT_REGISTRY
retains "source-agent" and the mocked _ask_clone_location cancellation path in
clone_agent() is actually exercised.
---
Outside diff comments:
In `@code_puppy/command_line/agent_menu.py`:
- Around line 560-563: The return type annotation on get_current_entry is stale:
it declares Optional[Tuple[str, str, str]] but entries holds AgentEntry (a
5-field NamedTuple); update the annotation to Optional[AgentEntry] and ensure
any necessary imports/reference to AgentEntry are present so the function
signature reads get_current_entry() -> Optional[AgentEntry] and accurately
reflects the actual entries element type.
---
Nitpick comments:
In `@code_puppy/agents/agent_manager.py`:
- Around line 562-599: Replace duplicated discovery/filter/instantiate logic by
making get_available_agents() and get_agent_descriptions() thin wrappers around
get_available_agents_with_descriptions(): call
get_available_agents_with_descriptions(), then map its returned Dict[str,
Tuple[str,str]] to the expected Dict[str,str] shape (extracting display_name or
description respectively) instead of re-calling _discover_agents() or
re-implementing PACK_AGENT_NAMES / UC_AGENT_NAMES checks; keep
get_available_agents_with_descriptions() as the single source of truth for
discovery and filtering so _discover_agents(), PACK_AGENT_NAMES and
UC_AGENT_NAMES logic aren’t duplicated.
In `@code_puppy/command_line/agent_menu.py`:
- Around line 329-335: The code uses direct indexing on info["path"] which can
raise KeyError while info.get("shadowed_path") is used safely; update the loop
that builds AgentEntry instances (where agents_metadata, source_info, and the
local variable info are used) to obtain both source_path and shadowed_path via
info.get("path") and info.get("shadowed_path") respectively, so both lookups are
defensive before constructing the AgentEntry(name, display_name, description,
source_path, shadowed_path).
In `@tests/agents/test_agent_cloning.py`:
- Around line 162-178: The test leaks host filesystem state because
clone_agent() invokes _discover_agents(); update the
test_clone_nonexistent_agent to mock the config discovery paths the same way
other tests do (or accept a tmp_path fixture and point config dirs there) so
_discover_agents() scans an isolated temp location. Specifically, before
clearing _AGENT_REGISTRY and monkeypatching _ask_clone_location, set the config
path globals or functions used by _discover_agents (the same config mocks used
in other tests) to point to tmp_path or temporary directories so discovery is
isolated when calling clone_agent("nonexistent-agent").
…exist When no project-level agents directory exists, don't prompt users to choose a save location — silently use the user directory instead. - agent_creator_agent: system prompt now fully branches on project dir presence; LLM is explicitly told not to ask when only user dir exists - agent_manager._ask_clone_location: short-circuits to user dir when no project dir, skipping ask_user_question entirely - agent_menu._select_clone_location: short-circuits to user dir when no project dir, skipping arrow_select_async entirely - Tests updated to assert no prompt is shown in the single-option case Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
code_puppy/command_line/agent_menu.py (1)
328-334: Consider using.get("path")for defensive access.Line 330 uses
info["path"]which will raiseKeyErrorif adiscover_json_agents_with_sources()entry ever lacks a"path"key. The adjacent line already uses.get("shadowed_path")defensively. Aligning the access pattern would be more consistent and robust.♻️ Suggested diff
- source_path = info["path"] if info else None + source_path = info.get("path") if info else None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/agent_menu.py` around lines 328 - 334, The loop that builds AgentEntry objects uses info["path"] which can raise KeyError if discover_json_agents_with_sources() produced an entry without "path"; change the access to info.get("path") (similar to shadowed_path) so source_path = info.get("path") if info else None, ensuring AgentEntry(...) receives None when the key is absent and avoiding exceptions in the agent_menu.py loop over agents_metadata and source_info.code_puppy/agents/agent_manager.py (2)
562-599: Good optimization — single discovery pass for display names + descriptions.The filtering and instantiation logic is duplicated across
get_available_agents(),get_agent_descriptions(), and this new function. Consider extracting the shared filter+instantiate loop into a private helper if the pattern grows further.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/agents/agent_manager.py` around lines 562 - 599, Extract the shared filter-and-instantiate loop used in get_available_agents_with_descriptions, get_available_agents, and get_agent_descriptions into a private helper (e.g., _collect_agents_info) that accepts message_group_id, pack_agents_enabled, uc_enabled and a callback/flag to return either names, display_name+description tuples, or descriptions; move the UUID creation and _discover_agents call outside the helper so callers can reuse a single discovery pass; inside the helper iterate over _AGENT_REGISTRY, skip entries in PACK_AGENT_NAMES/UC_AGENT_NAMES when disabled, instantiate via JSONAgent when agent_ref is str or call agent_ref(), and catch exceptions to provide fallback values (use the same fallback logic currently in get_available_agents_with_descriptions) so existing public functions can call this helper to avoid duplication.
496-514:get_agent_shadowed_pathperforms a full filesystem scan on every call.
discover_json_agents_with_sources()walks both agent directories each invocation. This is fine for occasional UI calls but could become a concern if called in a loop (e.g., once per agent in_get_agent_entries). Currently_get_agent_entriesinagent_menu.pycallsdiscover_json_agents_with_sources()directly rather than per-agent through this helper, so there's no immediate problem — just something to be aware of if callers change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/agents/agent_manager.py` around lines 496 - 514, get_agent_shadowed_path currently calls discover_json_agents_with_sources() which walks the filesystem on every invocation; avoid repeated full scans by changing get_agent_shadowed_path(agent_name: str) to accept an optional precomputed sources mapping (e.g., sources: Optional[Dict] = None) or by memoizing discover_json_agents_with_sources with a short-lived cache, then use the passed-in sources or cached result instead of always calling discover_json_agents_with_sources; update callers (like _get_agent_entries in agent_menu.py) to compute discover_json_agents_with_sources() once and pass the mapping into get_agent_shadowed_path to prevent per-agent filesystem walks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@code_puppy/command_line/agent_menu.py`:
- Around line 200-231: The guard in _select_clone_location that checks "if not
user_dir:" is unreachable because get_user_agents_directory() always creates and
returns AGENTS_DIR; remove that conditional block (the emit_info and return
None) and simplify the flow to assume user_dir is present, keeping
KeyboardInterrupt and None handling unchanged; reference
get_user_agents_directory, _select_clone_location, and the user_dir variable
when making the change.
---
Nitpick comments:
In `@code_puppy/agents/agent_manager.py`:
- Around line 562-599: Extract the shared filter-and-instantiate loop used in
get_available_agents_with_descriptions, get_available_agents, and
get_agent_descriptions into a private helper (e.g., _collect_agents_info) that
accepts message_group_id, pack_agents_enabled, uc_enabled and a callback/flag to
return either names, display_name+description tuples, or descriptions; move the
UUID creation and _discover_agents call outside the helper so callers can reuse
a single discovery pass; inside the helper iterate over _AGENT_REGISTRY, skip
entries in PACK_AGENT_NAMES/UC_AGENT_NAMES when disabled, instantiate via
JSONAgent when agent_ref is str or call agent_ref(), and catch exceptions to
provide fallback values (use the same fallback logic currently in
get_available_agents_with_descriptions) so existing public functions can call
this helper to avoid duplication.
- Around line 496-514: get_agent_shadowed_path currently calls
discover_json_agents_with_sources() which walks the filesystem on every
invocation; avoid repeated full scans by changing
get_agent_shadowed_path(agent_name: str) to accept an optional precomputed
sources mapping (e.g., sources: Optional[Dict] = None) or by memoizing
discover_json_agents_with_sources with a short-lived cache, then use the
passed-in sources or cached result instead of always calling
discover_json_agents_with_sources; update callers (like _get_agent_entries in
agent_menu.py) to compute discover_json_agents_with_sources() once and pass the
mapping into get_agent_shadowed_path to prevent per-agent filesystem walks.
In `@code_puppy/command_line/agent_menu.py`:
- Around line 328-334: The loop that builds AgentEntry objects uses info["path"]
which can raise KeyError if discover_json_agents_with_sources() produced an
entry without "path"; change the access to info.get("path") (similar to
shadowed_path) so source_path = info.get("path") if info else None, ensuring
AgentEntry(...) receives None when the key is absent and avoiding exceptions in
the agent_menu.py loop over agents_metadata and source_info.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code_puppy/command_line/agent_menu.py (1)
555-558:⚠️ Potential issue | 🟡 MinorStale return type annotation on
get_current_entry— should beOptional[AgentEntry].The function returns
entries[selected_idx[0]]which is now anAgentEntry, but the annotation still saysOptional[Tuple[str, str, str]]. Any static analysis tool following this annotation will flag attribute accesses likeentry.nameon the result as errors.♻️ Proposed fix
- def get_current_entry() -> Optional[Tuple[str, str, str]]: + def get_current_entry() -> Optional[AgentEntry]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/agent_menu.py` around lines 555 - 558, Update the stale return type on get_current_entry from Optional[Tuple[str, str, str]] to Optional[AgentEntry]; the function returns items from entries (AgentEntry objects), so change the annotation to Optional[AgentEntry] and ensure AgentEntry is imported or in scope where get_current_entry is defined (and adjust any typing imports if necessary).
🧹 Nitpick comments (2)
code_puppy/command_line/agent_menu.py (2)
345-345: Stale docstring — updateentriesdescription toList[AgentEntry].♻️ Proposed fix
- entries: List of (name, display_name, description) tuples + entries: List of AgentEntry descriptors🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/agent_menu.py` at line 345, Update the stale docstring for the parameter/variable named "entries" in agent_menu.py to reflect the correct type: change the description from "List of (name, display_name, description) tuples" to "List[AgentEntry]"; search for the docstring that mentions entries (e.g., in functions or classes around agent_menu.py where entries is documented) and replace the text to reference the AgentEntry type to keep the doc accurate and consistent with the AgentEntry class/type.
572-575: Prefer attribute access over positional unpacking forAgentEntryiteration.
for idx, (name, *_) in enumerate(entries)works becauseNamedTuplesupports both attribute access and positional index access and can be unpacked, but it is inconsistent with the attribute-access style (entry.name) used throughout the rest of this file.♻️ Proposed fix
- for idx, (name, *_) in enumerate(entries): - if name == selected_name: + for idx, entry in enumerate(entries): + if entry.name == selected_name:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/agent_menu.py` around lines 572 - 575, The loop uses positional unpacking for AgentEntry items; replace unpacking with attribute access to be consistent: iterate as "for idx, entry in enumerate(entries):" and compare "entry.name == selected_name", then set "selected_idx[0] = idx" and break. Update the loop that references entries, selected_name, and selected_idx to use entry.name instead of "(name, *_)".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/command_line/agent_menu.py`:
- Around line 222-223: The check "if choice is None: return None" is
dead/unreachable because arrow_select_async either returns a string or raises
KeyboardInterrupt (already caught); remove the unreachable guard in the function
containing the call to arrow_select_async (look for the variable choice
assignment and the surrounding try/except block around arrow_select_async) so
control flow is simplified and no-op returns are eliminated; ensure no other
code expects choice to be None after that try/except.
---
Outside diff comments:
In `@code_puppy/command_line/agent_menu.py`:
- Around line 555-558: Update the stale return type on get_current_entry from
Optional[Tuple[str, str, str]] to Optional[AgentEntry]; the function returns
items from entries (AgentEntry objects), so change the annotation to
Optional[AgentEntry] and ensure AgentEntry is imported or in scope where
get_current_entry is defined (and adjust any typing imports if necessary).
---
Nitpick comments:
In `@code_puppy/command_line/agent_menu.py`:
- Line 345: Update the stale docstring for the parameter/variable named
"entries" in agent_menu.py to reflect the correct type: change the description
from "List of (name, display_name, description) tuples" to "List[AgentEntry]";
search for the docstring that mentions entries (e.g., in functions or classes
around agent_menu.py where entries is documented) and replace the text to
reference the AgentEntry type to keep the doc accurate and consistent with the
AgentEntry class/type.
- Around line 572-575: The loop uses positional unpacking for AgentEntry items;
replace unpacking with attribute access to be consistent: iterate as "for idx,
entry in enumerate(entries):" and compare "entry.name == selected_name", then
set "selected_idx[0] = idx" and break. Update the loop that references entries,
selected_name, and selected_idx to use entry.name instead of "(name, *_)".
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
1 participant
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.
Follow up to #170
Summary
Path:field showing the agent's file path (or "built-in" for Python agents)Changes
json_agent.py: addeddiscover_json_agents_with_sources()returning source metadata and shadow info;discover_json_agents()delegates to itagent_manager.py: extended to exposeget_agent_source_path(),get_agent_shadowed_path(); restored directory-confinement guard indelete_clone_agent()(broadened to cover both user and project dirs); added_ask_clone_location()with sentinel constantsagent_creator_agent.py:ask_save_location()prompts for user vs. project directory; LLM prompt updated to use absolute project pathagent_menu.py:_get_agent_entries()returns 5-tupleAgentEntryNamedTuple withsource_pathandshadowed_path; preview panel rendersPath:field; menu panel renders shadow warning badgeagents/__init__.py: exportsget_agent_source_path,get_agent_shadowed_pathtest_json_agent_sources.py,test_agent_cloning.py; extendedtest_agent_menu.py,test_agent_creator_agent.pyTesting
Summary by CodeRabbit
New Features
Documentation
Bug Fixes / Improvements
Tests