pal-e-docs

Review: T2 basketball-api migration 031 jersey_public_orders

review-947-2026-04-10 Doc

review approved
Project

First Pass Verdict: NEEDS_REFINEMENT (2026-04-10 22:09)

Board item #947 on board-westside-basketball — Forgejo issue forgejo_admin/basketball-api#429. Scope is schema-sound and the story is valid, but two concrete facts in the issue body are wrong: the migration number and the model file path. Both will cause dev to either collide with existing files or be forced to improvise repo convention. Fix in the body and re-submit.

Template Completeness

  • [x] Type (Feature)
  • [x] Lineage
  • [x] Repo
  • [x] User Story (WS-S31 stated narratively)
  • [x] Context
  • [x] File Targets
  • [x] Schema (exact SQL)
  • [x] Acceptance Criteria
  • [x] Test Expectations (with pytest -k command)
  • [x] Constraints
  • [x] Checklist
  • [x] Related

First Pass Findings

  • [BODY] Migration number wrong: said 014, actual head is 030, next is 031.
  • [BODY] Model path wrong: said src/basketball_api/models/jersey_public_order.py, actual is flat src/basketball_api/models.py.
  • [BODY] Missing CREATE EXTENSION IF NOT EXISTS pgcrypto for gen_random_uuid().
  • [BODY] Hands-off list should cover migrations 013-030 and existing models.py classes.
  • [SCOPE] arch-jersey-intake note flagged as missing — CORRECTION in re-review below.

Re-Review Verdict: APPROVED (2026-04-10)

Re-reviewer: Dottie. All four [BODY] fixes from the first pass have landed in issue #429. Ticket is cleared to advance from todo to next_up.

Fix Verification

  • [x] Title — now reads "Migration 031: jersey_public_orders table". Correct.
  • [x] Migration file target — File Targets section now specifies alembic/versions/031_add_jersey_public_orders.py with revision = "031", down_revision = "030". Correct against verified head (030_add_registration_type_to_registrations.py — I re-verified the versions directory: 013 through 030 all present, 031 is next).
  • [x] Model target — File Targets now says "append a new JerseyPublicOrder SQLAlchemy class at the end of the file" targeting src/basketball_api/models.py. No new submodule file. Verified against repo: src/basketball_api/models.py is flat, no models/ directory exists.
  • [x] pgcrypto extension — Schema block opens with CREATE EXTENSION IF NOT EXISTS pgcrypto; and the surrounding prose says "In upgrade() — FIRST ensure pgcrypto is available for gen_random_uuid()". Correct.
  • [x] pgcrypto downgrade guard — Explicit line after the SQL block: "In downgrade(): DROP TABLE jersey_public_orders. Do NOT drop the pgcrypto extension — other tables may rely on it." Correct.
  • [x] Hands-off list — "Files the agent should NOT touch" now enumerates "Any existing migration in alembic/versions/ (013 through 030)" and "Any existing class in src/basketball_api/models.py — append-only". Correct.
  • [x] Discovery guard in AC — Acceptance Criteria includes "No existing class in models.py is modified (verify with git diff main -- src/basketball_api/models.py — only additions below the final pre-existing class)". Checklist also includes the same git diff check. Correct.
  • [x] Context paragraph — Rewritten to state current head is 030_add_registration_type_to_registrations.py and next revision is 031, with the "migrations 014 through 030 already exist" warning. Correct.

arch-jersey-intake Correction

First pass flagged arch-jersey-intake as MISSING based on search_notes returning zero results. That was a stale vector index response — the Ollama embedding service was down earlier today. Verified via direct get_note(slug="arch-jersey-intake"): the note EXISTS (note id 1406, created 2026-04-10 21:50, updated 22:08, tags: architecture, active, arch:jersey-intake, project: westside-basketball). It documents all three intake systems (A roster, B public intake, C generic checkout), the System B data flow, and the no-merge decision. Traceability triangle is fully intact. No scoping ticket needed — dropping that [SCOPE] item from the first pass.

Traceability

  • [x] story:WS-S31 — verified.
  • [x] arch:jersey-intake — label present and backing arch note confirmed to exist.
  • [x] Forgejo issue forgejo_admin/basketball-api#429 — open, body updated.
  • [x] type:feature, scope:production, wave:1 labels present.

Unchanged Strengths (carried from first pass)

  • Schema sanity: CHECK constraints, FK types match Parent/Player integer PKs, ON DELETE SET NULL correct, index set reasonable, no inappropriate UNIQUEs.
  • Blast radius: zero existing references to jersey_public_order. Purely additive.
  • Decomposition: single-repo, single-agent, under the 5-minute rule.
  • AC covers upgrade/downgrade round-trip, CHECK constraints, NOT NULL, model importability.

Recommendation

APPROVED. Ticket #947 / basketball-api#429 is ready to move from todo to next_up. Dispatch to dev when capacity allows. No further refinement needed.