Review: Repo restructure: Nemo agent + modular tool registry

review-683-2026-03-28 Doc

review needs-refinement

Project

[x] story:read-ops label — "Admin Read Operations via GroupMe"
[x] story note verified — found in project-westside-ai-assistant user-stories section (key: read-ops, note: story-westside-ai-assistant-read-ops)
[x] arch:A2 label — AI Engine component
[ ] arch note MISSING — [SCOPE] No note named arch-A2 exists in pal-e-docs. The project page references arch-domain-westside-ai-assistant, arch-dataflow-westside-ai-assistant, arch-deployment-westside-ai-assistant, but none of these notes exist yet either (search returned empty). The A2 component ID is referenced in the story note but has no dedicated architecture note.
[x] Forgejo issue — forgejo_admin/westside-ai-assistant#18, open

[x] app/ai.py — verified: exists (618 lines), contains inline TOOLS list with 14 tool definitions and _execute_read_tool dispatch function. Ticket correctly identifies this for modification.
[x] app/basketball.py — verified: exists, correctly listed as do-not-touch
[x] app/config.py — verified: exists, correctly listed as no-changes-needed
[x] app/groupme.py — verified: exists, correctly listed as do-not-touch
[x] app/confirmation.py — verified: exists, correctly listed as dormant/do-not-touch
[x] New files to create (agents/nemo.md, tools/*/TOOL.md, tools/*/handler.py, app/tool_registry.py, prompts/system.md) — parent directories do not yet exist, creation is valid
[ ] ISSUE: Ticket says "4 tool categories" with 7 read tools, but ai.py has 14 tools (7 read + 7 write). The write tools (update_player, assign_player_to_team, remove_player_from_team, toggle_player_visibility, create_team, checkin_player, bulk_assign_tryout_numbers) are not addressed in the file targets. If read-only V1 is intentional, the ticket should explicitly state that write tool categories are deferred and the registry should only discover read tools. Otherwise the registry will silently drop 7 tools.

OK — issue filed on forgejo_admin/westside-ai-assistant, all file targets are within that repo. Single-repo change.

[ ] Board item #684 (Interaction logging to Postgres) — depends on ai.py internals. This restructure should land first. Not explicitly documented as a dependency. Status: backlog.
[ ] Board item #685 (Read-only lockdown) — directly affected by this restructure since it modifies the tool set. Should land after this. Not explicitly documented. Status: backlog.
[ ] Board item #647 (Model provider switch: Anthropic/Ollama) — touches ai.py Anthropic integration. Should be aware of registry pattern. Not explicitly documented. Status: backlog.
No items currently in_progress that would conflict. No blocking dependencies — this ticket can proceed independently.

[x] AC 1 (agents/nemo.md defines personality) — testable: file existence + content check
[x] AC 2 (4 tool categories with TOOL.md + handler.py) — testable: directory/file existence
[x] AC 3 (tool_registry.py auto-discovers and generates definitions) — testable: unit test
[x] AC 4 (ai.py uses registry) — testable: import check + behavior test
[x] AC 5 (new tool = new directory, no ai.py changes) — testable: verify no dispatch in ai.py
[ ] AC 6 (all existing read-tool tests still pass) — testable via pytest, BUT test_ai.py (593 lines) imports TOOLS, _TOOL_META, _execute_read_tool directly from app.ai. After restructure, these must still be importable from the same location OR tests must be updated. The ticket does not mention test file modifications.
[x] AC 7 (system prompt from prompts/system.md + nemo.md) — testable: content assembly check

test_ai.py (593 lines) imports TOOLS, _TOOL_META, _execute_read_tool, and other internals from app.ai. Restructuring these requires either (a) preserving re-exports in ai.py or (b) updating test imports. The ticket does not address this.
.woodpecker.yaml CI pipeline — should not be affected if pytest command stays the same.
No other repos consume this code (single-service, no SDK). Rollback is straightforward (revert PR).

Apply the three-thing limit and five-minute rule:

File count: 12+ new files to create, 1 file to modify (ai.py), potentially 1 more (test_ai.py). All in one repo.
AC count: 7 acceptance criteria + 4 test expectations = 11 checkpoints.
Discrete changes: 4 — (1) create tool directory structure with TOOL.md + handler.py, (2) create tool_registry.py, (3) refactor ai.py to use registry, (4) create agents/nemo.md + prompts/system.md. Exceeds three-thing limit.
Estimated agent time: 5-8 minutes. The file creation is mechanical (move definitions from inline to modular files), but the registry + ai.py integration requires careful wiring.
Assessment: Borderline. The work is tightly coupled (registry must exist before ai.py can use it, tools must exist before registry can discover them). Parallelization is limited. A single agent pass is feasible but tight. If refinement adds write tools to scope, decomposition would be needed.

[BODY] Clarify write tool handling — add explicit statement to Context and File Targets: "Write tools (update_player, assign_player_to_team, remove_player_from_team, toggle_player_visibility, create_team, checkin_player, bulk_assign_tryout_numbers) remain inline in ai.py for now. The registry discovers read tools only. Write tool modularization is deferred to a future ticket." OR include write tool categories in the file targets (which would trigger decomposition).
[BODY] Add tests/test_ai.py to "Files the agent should modify" — test imports reference TOOLS, _TOOL_META, _execute_read_tool directly from app.ai. Either document that re-exports must be preserved in ai.py, or list test_ai.py as a modification target for import updates.
[SCOPE] Create architecture notes — the project page references arch-domain-westside-ai-assistant, arch-dataflow-westside-ai-assistant, arch-deployment-westside-ai-assistant but none exist in pal-e-docs. The arch:A2 label has no backing note. Create at minimum arch-domain-westside-ai-assistant with component IDs (A1=GroupMe Webhook, A2=AI Engine, A3=Basketball Client, A4=Infrastructure).

Verdict: NEEDS_REFINEMENT