pal-e-docs

Review: Model provider switch: Anthropic / Ollama feature flag

review-647-2026-03-28 Doc

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, scoped from session discussion
  • [x] Repo — forgejo_admin/westside-ai-assistant
  • [x] User Story — As Lucas (platform operator), switch providers via env var
  • [x] Context — Good background on SDK differences, Ollama cluster-internal URL, qwen2.5:7b already pulled
  • [x] File Targets — 5 modify targets + 3 don't-touch targets
  • [x] Acceptance Criteria — 6 items
  • [x] Test Expectations — 3 unit tests + run command
  • [x] Constraints — Minimal provider interface, normalized response, no plugin system
  • [x] Checklist — PR opened, tests pass, no unrelated changes
  • [x] Related — project-westside-ai-assistant, story-westside-ai-assistant-read-ops
All required sections present. Template is complete.

Traceability

  • [x] story:read-ops label — present on board item, listed in project-westside-ai-assistant user-stories table
  • [ ] story:read-ops note MISSING — [SCOPE] Create user story note story-westside-ai-assistant-read-ops (referenced on project page but note does not exist in pal-e-docs)
  • [x] story:write-ops label — present on board item, listed in project-westside-ai-assistant user-stories table
  • [ ] story:write-ops note MISSING — [SCOPE] Create user story note story-westside-ai-assistant-write-ops (referenced on project page but note does not exist in pal-e-docs)
  • [x] arch:A2 label — present on board item
  • [ ] arch note MISSING — [SCOPE] No arch-A2 note exists. Project page references arch-domain-westside-ai-assistant, arch-dataflow-westside-ai-assistant, and arch-deployment-westside-ai-assistant but none of these exist in pal-e-docs. The A2 label-to-named-note mapping is also unclear.
  • [x] Forgejo issue — forgejo_admin/westside-ai-assistant#16, open

File Targets

  • [x] app/ai.py — verified: exists, currently Anthropic-only (imports anthropic, uses anthropic.Anthropic client directly, hardcoded to settings.anthropic_api_key and settings.anthropic_model). Provider abstraction needed as described.
  • [x] app/config.py — verified: exists, currently has only anthropic_api_key and anthropic_model fields. Needs ai_provider, ollama_url, ollama_model additions.
  • [x] app/health.py — verified: exists, readiness check currently hardcoded to bool(settings.anthropic_api_key). Needs provider-aware credential check.
  • [x] requirements.txt — verified: exists, currently has anthropic>=0.52,<1 but no openai. Needs openai dependency added.
  • [x] tests/test_ai.py — verified: exists, 260+ lines of Anthropic-specific tests. Needs dual-provider test paths.
All 5 file targets verified. Paths are accurate and current codebase state matches what the issue describes.

Repo Placement

OK. Issue filed on forgejo_admin/westside-ai-assistant, all file targets are in this single repo. No cross-repo concerns.

Dependencies

  • [x] Board item #642 "Swap Anthropic SDK for Ollama (local qwen2.5:7b)" — done. This ticket logically follows it, restoring dual-provider support.
  • [x] No blocking dependencies. No items blocked by this ticket.
  • Note: Item #683 "Repo restructure: Nemo agent + modular tool registry" (backlog) would touch app/ai.py — ordering matters but not a blocker since #683 is still in backlog.

Acceptance Criteria

6 AC items, all verifiable by an agent:
  • AC 1-2: Environment variable switching — verifiable via unit tests with mocked SDK clients
  • AC 3: Tool definition compatibility — verifiable by running both provider paths through tool_use tests
  • AC 4: Unchanged confirmation flow — verifiable by existing test suite continuing to pass
  • AC 5: Missing credentials error — verifiable via unit test with missing env vars
  • AC 6: Default provider — verifiable by checking config default value
All criteria are concrete and testable. No missing criteria detected.

Blast Radius

Low. Changes are contained within the AI engine layer. basketball.py, confirmation.py, and groupme.py are explicitly excluded. The ProviderResponse normalization ensures downstream consumers are unaffected. No sibling services use the same AI provider pattern. Rollback is straightforward: revert the PR.

Decomposition Assessment

5 file targets in 1 repo. 6 AC + 3 test expectations = 9 verification items. All changes are tightly coupled: config drives provider selection, provider drives ai.py, health checks config, tests verify both paths, requirements list dependencies. The constraint explicitly says "two if/else branches, not a plugin system." A single agent can handle this within the 5-minute window. No decomposition needed.

Recommendation

  • [SCOPE] Create user story note story-westside-ai-assistant-read-ops — referenced on project page but does not exist in pal-e-docs
  • [SCOPE] Create user story note story-westside-ai-assistant-write-ops — referenced on project page but does not exist in pal-e-docs
  • [SCOPE] Create architecture notes for this project — project page references arch-domain-westside-ai-assistant, arch-dataflow-westside-ai-assistant, arch-deployment-westside-ai-assistant but none exist. The arch:A2 label on board items has no clear mapping to these named arch notes.
Note: All three [SCOPE] items are project-level gaps affecting every board item on board-westside-ai-assistant, not specific to this ticket. The issue body itself is well-scoped and complete. Once the backing notes are created, this ticket would be READY.