pal-e-docs

Review: Replace 7 read tools with generic read-only api_get tool (re-review)

review-721-2026-03-30-r2 Review

review ready
Project

Verdict: READY

Re-review of board item #721 after refinement. Previous review: review-721-2026-03-30 (NEEDS_REFINEMENT). All three issues addressed.

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Standalone, simplifies Nemo's read architecture
  • [x] Repo -- forgejo_admin/westside-ai-assistant
  • [x] User Story -- clear who/what/why for Marcus (admin)
  • [x] Context -- explains 7-tool problem, generic replacement rationale, safety constraint
  • [x] File Targets -- 2 files to modify, 3 files not to touch, all specific
  • [x] Acceptance Criteria -- 7 testable criteria
  • [x] Test Expectations -- unit + integration tests with run command
  • [x] Constraints -- GET-only, keep writes, update system prompt
  • [x] Checklist -- PR/tests/no-unrelated
  • [x] Related -- correctly references project-westside-basketball and upstream issue
All template-issue-feature sections present and correct.

Traceability

  • [x] story:WS-S5 label -- Admin WS-S5 is "view registration stats on a dashboard." This ticket's generic api_get tool is a superset enabler: it gives Nemo access to all admin endpoints including dashboard stats. Superset mapping accepted.
  • [x] story note verified -- WS-S5 found in project-westside-basketball user-stories section (both Superadmin and Admin tiers)
  • [x] arch:basketball-api label -- present on board item (changed from arch:westside-ai-assistant per refinement)
  • [x] arch note verified -- arch:basketball-api is a registered component in arch-deployment-westside-basketball service inventory table
  • [x] Forgejo issue -- forgejo_admin/westside-ai-assistant#23, open

File Targets

  • [x] app/ai.py -- verified: contains exactly 7 read tool definitions (get_dashboard, list_players, get_player, list_teams, get_roster, get_subscriptions_overview, list_subscriptions) + 7 write tools. Contains _execute_read_tool dispatch function. TOOLS list and dispatch logic confirmed.
  • [x] app/basketball.py -- verified: contains 7 matching read methods and 7 write methods. Uses _request(method, path) helper. No generic get(path) method exists yet. Ready for modification.
  • [x] app/confirmation.py -- verified exists, correctly listed as NOT to touch.
  • [x] app/groupme.py -- verified exists, correctly listed as NOT to touch.
All file targets verified in repo. Paths and code claims are accurate.

Repo Placement

OK. Issue filed on forgejo_admin/westside-ai-assistant, work targets the same repo. Single-repo change.

Dependencies

  • [x] basketball-api endpoints -- satisfied. The 7 existing endpoints are live. New endpoints (jerseys, schedules) are a downstream benefit, not a blocker.
  • [x] Keycloak service account -- satisfied. Already has admin access per issue context.
  • [x] No blocking items on the board. Related items (#702 QA nits, #722 court rental) are independent.
Note: Forgejo issue #20 (read-only lockdown) in the same repo overlaps. Consider closing #20 as superseded after #23 merges.

Acceptance Criteria

7 criteria, all testable by an agent:
  • "Single api_get tool replaces all 7 read tools" -- verify tool count in TOOLS list. Testable.
  • "api_get enforces GET method at code level" -- verify hardcoded method="GET". Testable.
  • "No request body is ever sent" -- verify no json= parameter in get method. Testable.
  • "System prompt lists example endpoints" -- verify SYSTEM_PROMPT content. Testable.
  • "Existing write tools + confirmation flow unchanged" -- verify write tool count and confirmation.py untouched. Testable.
  • "How many players? still works" -- regression test with mock. Testable.
  • "Who ordered jerseys? works if endpoint exists" -- conditional, testable with mock. Testable.
All criteria are specific and verifiable.

Blast Radius

Low. Changes confined to 2 files in a single service. Write tools and confirmation flow explicitly preserved. No downstream consumers of internal tool definitions. Rollback = revert single PR.

Decomposition Assessment

2 file targets in 1 repo. 7 AC but tightly coupled -- one atomic refactor (remove 7 read tools, add 1 generic tool, add 1 method to basketball.py). Decomposing would create artificial splits with merge-order dependencies. Estimated agent work: 3-4 minutes. No decomposition needed.

Refinement Resolution

  • story:WS-S5 label -- RESOLVED. Superset justification accepted: generic api_get enables all admin reads including dashboard stats (WS-S5 scope).
  • arch label -- RESOLVED. Changed to arch:basketball-api, which is a registered component in arch-deployment-westside-basketball.
  • Related section -- RESOLVED. Updated from project-westside-ai-assistant to project-westside-basketball in the issue body.

Recommendation

No action needed. All three refinement issues resolved. Ticket is ready for dispatch.