pal-e-docs

Review: Validate: basketball-api (2 PRs, test failure)

review-514-2026-03-28 Doc

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Task
  • [x] Lineage -- "Validation audit -- session 2026-03-28 pipeline gap"
  • [ ] Repo -- MISSING (template requires explicit ### Repo section)
  • [x] User Story -- present, clear "who wants what and why"
  • [x] Context -- embedded in Scope section (Task type uses Scope instead of Context + File Targets)
  • [x] File Targets -- N/A for Task type; Scope section present instead
  • [x] Acceptance Criteria -- 7 items present
  • [x] Test Expectations -- present
  • [x] Constraints -- present, includes hands-off convention reminder
  • [ ] Checklist -- MISSING (template requires ### Checklist section)
  • [x] Related -- present

Traceability

  • [x] story:WS-S5 -- platform operational validation story
  • [x] arch:basketball-api -- correct target component
  • [x] Forgejo issue -- forgejo_admin/basketball-api#201, open

File Targets

N/A -- Task type uses Scope section instead of File Targets. Correct per template-issue convention.

Repo Placement

OK. Issue filed on basketball-api, all work is within basketball-api. No multi-repo concerns.

Dependencies

  • [x] Board item #468 ("Add downstream consumer check to migration workflow") -- done. This is PR #199 itself. No conflict.
  • [x] PR #203 ("fix: update test assertion to use settings.frontend_url") -- merged. This already fixed the root cause test failure.
  • [x] Pipeline #211 on main -- green (614 tests passing). No outstanding failures.
  • No basketball-api items currently in in_progress, next_up, or qa columns. No blockers.

Acceptance Criteria

7 AC items. Assessment:
  • AC 1 ("Pipeline failures diagnosed -- root cause documented") -- Already done. Root cause: PR #196 changed settings.base_url to settings.frontend_url in tryouts.py but test assertion at tests/test_tryouts.py:419 was not updated. PR #203 fixed it. Testable: yes (check comment history).
  • AC 2 ("Determined whether docs-only PRs should skip build") -- Partially answered. Pipeline at .woodpecker.yaml only excludes k8s/.argocd-source-*. Docs-only PRs trigger full build+test. The test failure was pre-existing, not caused by docs changes. Path filtering for docs/ is a separate optimization. Testable: yes.
  • AC 3 ("Verified no regression in basketball-api functionality") -- Testable: pipeline #211 green (614 passed, 0 failed).
  • AC 4 ("API endpoints still responding correctly") -- Testable: curl health endpoint.
  • AC 5 ("Pipeline verified") -- NOT SPECIFIC. Should say: "Pipeline #211 or later green on main." Ambiguous "verified" is not agent-executable.
  • AC 6 ("Deployment confirmed") -- NOT SPECIFIC. Should say: "ArgoCD sync shows current image SHA from latest green pipeline." No way to verify "confirmed" without a concrete check.
  • AC 7 ("Features validated") -- NOT SPECIFIC. Should say: "docs/migrations.md and docs/auth-architecture.md present in repo on main." What "features" means for docs-only PRs is unclear.

Blast Radius

  • settings.base_url still referenced in 7 locations across basketball-api: routes/admin.py, routes/tryouts.py, services/coach_onboarding.py, scripts/send_registration_emails.py, and 3 test files. PR #196 introduced frontend_url but did not fully migrate. Discovered scope -- not in scope for this validation ticket but should become its own board item.
  • No downstream consumers affected by the docs-only PRs #199 and #200.
  • Rollback: not applicable -- docs-only changes with no behavioral impact.

Decomposition Assessment

7 AC items across 1 repo. Normally 7 AC triggers the decomposition threshold (>5). However:
  • Most work is already complete: root cause diagnosed (issue comment), fix merged (PR #203), pipeline green (#211).
  • Remaining work is pure confirmation: check pipeline status, check API health, update AC checkboxes, close issue.
  • Estimated agent time: under 5 minutes.
  • No independent subtasks that would benefit from parallelization.
No decomposition needed.

Recommendation

  • [BODY] Add explicit ### Repo section: forgejo_admin/basketball-api
  • [BODY] Add ### Checklist section per template-issue
  • [BODY] Replace AC 5 with: "Pipeline #211 or later green on main"
  • [BODY] Replace AC 6 with: "ArgoCD sync shows current image SHA from latest green pipeline"
  • [BODY] Replace AC 7 with: "docs/migrations.md and docs/auth-architecture.md present in repo on main"
  • [BODY] Update Scope section to note that root cause is already diagnosed and fixed (PR #203). Remaining work is confirmation only.
  • [SCOPE] Discovered scope: settings.base_url still used in 7 locations after PR #196 introduced frontend_url. Should become its own board item for full migration.