pal-e-docs

Review: Add read-only SQL tool with Postgres access to basketball + contracts DBs

review-729-2026-03-30 Doc

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, evolved from api_get (#23)
  • [x] Repo — forgejo_admin/westside-ai-assistant
  • [x] User Story — As Marcus (admin), ask Nemo any question about program data
  • [x] Context — explains why SQL access is more powerful than REST for arbitrary queries
  • [x] File Targets — 2 create, 2 modify, exclusions listed
  • [x] Acceptance Criteria — 8 criteria
  • [x] Test Expectations — 3 test types + run command
  • [x] Constraints — read-only role, schema in prompt, asyncpg/psycopg, k8s secrets
  • [x] Checklist — standard 3-item
  • [x] Related — project + predecessor issue
All required sections for Feature template are present.

Traceability

  • [ ] story:WS-S5 label — MISMATCH. Superadmin WS-S5 = "basketball-api's database on CNPG shared cluster so that player data has daily backups." Admin WS-S5 = "view registration stats on a dashboard." Neither matches the actual user story in this issue: "ask Nemo any question about program data and get an answer." This feature needs its own story ID or a corrected label.
  • [ ] story note MISSING — [SCOPE] Create a new user story entry on project-westside-basketball user-stories section for AI assistant data queries (existing WS-S5 does not cover this scope).
  • [x] arch:westside-ai-assistant label — present on board item
  • [ ] arch note MISSING — search_notes("arch-westside-ai-assistant") returned no results. [SCOPE] Create architecture note arch-westside-ai-assistant for the Nemo AI assistant component.
  • [x] Forgejo issue — forgejo_admin/westside-ai-assistant#27, open

File Targets

  • [x] app/database.py — to create. Confirmed does not exist. Parent directory app/ exists. Correct.
  • [x] app/ai.py — verified exists. TOOLS list at line 49, SYSTEM_PROMPT at line 24. Correct modification targets (add db_query tool, add schema to prompt).
  • [x] app/config.py — verified exists. Uses pydantic_settings, currently no DATABASE_URL. Correct target for adding DB env vars.
  • [x] app/confirmation.py — verified exists. Correctly listed as "do not touch" (read-only queries need no confirmation flow).
  • [ ] requirements.txt — MISSING from File Targets. Currently has no Postgres driver. asyncpg must be added as a new dependency. basketball-api uses psycopg2-binary (sync), but westside-ai-assistant is async FastAPI, so asyncpg is the natural choice.
  • [ ] Infrastructure targets — NOT in this repo. Creating a read-only Postgres role, REVOKE on sensitive columns, statement_timeout, and k8s secrets are infra tasks that live in pal-e-services or pal-e-platform. These are undocumented cross-repo file targets.
  • [ ] pal-e-deployments/overlays/westside-ai-assistant/prod/deployment-patch.yaml — MISSING from File Targets. Needs DATABASE_URL env vars added (currently only has BASKETBALL_API_URL, ANTHROPIC_*, GROUPME_*, KEYCLOAK_*).

Repo Placement

Cross-repo mismatch. The Forgejo issue is filed on forgejo_admin/westside-ai-assistant which is correct for the application code. However, the "Infrastructure" section describes work that must happen in other repos:
  • Creating read-only Postgres role on CNPG cluster — likely pal-e-services or pal-e-platform terraform
  • Adding connection strings to k8s secrets — pal-e-services or pal-e-deployments
  • Network policy changes for Postgres port access — pal-e-platform/terraform/network-policies.tf
The infra work needs its own Forgejo issue(s) in the appropriate repo(s), and this ticket should list those as blocking dependencies.

Dependencies

  • [x] Board item #721 (done) — "Replace 7 read tools with generic read-only api_get tool" — predecessor, completed. No blocker.
  • [x] Board item #730 (backlog) — "Upgrade Nemo model from Haiku 4.5 to Sonnet 4.6" — independent, no dependency.
  • [ ] CNPG read-only Postgres role — must be provisioned before app code can connect. No board item exists for this infra work. Blocking dependency.
  • [ ] Network policies — westside-ai-assistant pods currently cannot reach basketball-api or westside-contracts Postgres (port 5432). Network policy updates required. No board item exists. Blocking dependency.
  • [ ] Deployment config — DATABASE_URL env vars must be wired in pal-e-deployments before the app can use them. No board item exists. Blocking dependency.

Acceptance Criteria

8 acceptance criteria. Assessment of testability:
  • [x] AC1-2 (SELECT against both DBs) — testable if DB access is configured
  • [x] AC3 (INSERT/UPDATE/DELETE rejected at role level) — testable with integration test
  • [x] AC4 (statement_timeout) — testable with slow query
  • [x] AC5 (sensitive tables not queryable) — testable with SELECT on blocked columns
  • [ ] AC6 (schema index in system prompt) — verifiable by code inspection, but which tables/columns are not specified. Agent must discover schemas at implementation time or the issue should enumerate them.
  • [x] AC7-8 (natural language to SQL to response) — integration tests, testable
Missing criteria: No AC for network connectivity (can the pod reach both DBs?). No AC for deployment-patch.yaml env var wiring. No AC for the new asyncpg dependency being installed correctly.

Blast Radius

  • Network policies — westside-ai-assistant currently has ingress rules for basketball-api (HTTP) and keycloak only. Direct Postgres access (port 5432) to basketball-api DB and westside-contracts DB requires new network policy rules. May require changes in pal-e-platform/terraform/network-policies.tf.
  • CNPG role management — creating a read-only role affects the shared CNPG cluster. Other services using the same cluster are unaffected if the role is truly SELECT-only, but misconfiguration could expose data across tenants.
  • No blast to existing tools — the issue correctly states confirmation.py and write tools are unchanged. The existing api_get tool is unaffected.
  • Rollback — straightforward. Remove db_query tool from TOOLS list, revert config.py. Postgres role can remain (harmless). Low rollback risk.

Decomposition Assessment

NEEDS DECOMPOSITION — route to skill-decompose-ticket.
  • 8 acceptance criteria (exceeds 5 threshold)
  • Cross-repo work: app code (westside-ai-assistant) + infra provisioning (pal-e-services/pal-e-platform) + deployment config (pal-e-deployments) — 3+ repos
  • 3+ discrete work streams: (1) Postgres role + secrets provisioning, (2) network policy updates, (3) app code (database.py + ai.py + config.py + requirements.txt + tests)
  • Estimated agent time: well over 5 minutes for combined scope
  • Independent subtasks that could be parallelized: infra provisioning and app code development can happen in parallel once the interface (env var names, connection string format) is agreed upon
Suggested decomposition:
  • Infra: Create read-only Postgres roles + secrets — CNPG role creation, REVOKE on sensitive columns, statement_timeout, k8s secrets with connection strings. Repo: pal-e-services or pal-e-platform.
  • Infra: Network policies — Allow westside-ai-assistant pods to reach basketball-api and westside-contracts Postgres on port 5432. Repo: pal-e-platform.
  • Deploy: Wire DATABASE_URL env vars — Add env vars to deployment-patch.yaml referencing the k8s secrets. Repo: pal-e-deployments.
  • App: database.py + config.py + requirements.txt — async Postgres client, connection management, env vars, asyncpg dependency. Repo: westside-ai-assistant.
  • App: db_query tool + schema prompt + tests — tool definition in ai.py, schema index in SYSTEM_PROMPT, unit + integration tests. Repo: westside-ai-assistant. Depends on sub-ticket 4.

Recommendation

  • [LABEL] Fix story label: story:WS-S5 does not match the user story in this issue. Either create a new story (e.g., "As an admin, I want to ask the AI assistant any data question so that I don't need custom endpoints for every query") and update the label, or reassign to an existing matching story.
  • [SCOPE] Create architecture note arch-westside-ai-assistant for the Nemo AI assistant component in pal-e-docs.
  • [SCOPE] Create user story entry for AI assistant data queries on project-westside-basketball user-stories section (current WS-S5 does not cover this scope).
  • [BODY] Add requirements.txt to File Targets — asyncpg must be added as a new dependency.
  • [BODY] Add pal-e-deployments/overlays/westside-ai-assistant/prod/deployment-patch.yaml to File Targets for DATABASE_URL env var wiring.
  • [BODY] Document network policy dependency — westside-ai-assistant pods need Postgres port 5432 access to basketball-api and westside-contracts namespaces.
  • [BODY] Specify which tables/columns should appear in the schema index (AC6 is not actionable without this).
  • [DECOMPOSE] 8 AC across 3+ repos (westside-ai-assistant, pal-e-platform, pal-e-deployments), 3+ discrete work streams. Route to skill-decompose-ticket for sub-board creation.