pal-e-docs

Review: #486 Recover 18 stranded Utah Invitational orders (regen + apology blast)

review-1022-2026-04-17 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Scope is largely solid — the 6-gate approval chain is the spine of the ticket and is cited in three places (HARD STOP block, Phase A AC, Constraints). The 18-order stranded list and 5-order paid-exclusion list reconcile 100% against live DB state. Script and MJML wrapper references are valid. Four fixable issues prevent advance to todo.

Template Completeness

  • [x] Type (Task)
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] Scope
  • [x] Acceptance Criteria (phased A/B/C, 13 gates)
  • [x] Constraints
  • [x] Checklist
  • [x] Related
  • [x] HARD STOP block with 6-gate chain — prominent, unmissable
  • File Targets section N/A — Task type uses Scope instead (per template-issue)

Traceability

  • [x] type:task label — matches issue Type header
  • [ ] story:payment-recovery label — story NOT found in project-westside-basketball user-stories. Closest existing story is WS-S22 ("receive clear email communications with action links so I never miss a deadline") — this ticket arguably fulfills that story.
  • [ ] arch:stripe-checkout label — no arch-stripe-checkout note exists in pal-e-docs (verified via search_notes). Existing arch notes are arch-domain-westside-basketball, arch-dataflow-westside-basketball, arch-deployment-westside-basketball, arch-auth-westside-basketball.
  • [x] Forgejo issue forgejo_admin/basketball-api#486 — open, body well-formed

File / Script Targets

  • [x] scripts/regenerate_tournament_orders.py — verified. Supports --product-ids 5,6,7 --commit exactly as ticket states (argparse at line 184, product-ids at 191-194, --commit at 196-200). Dry-run default, commit flag opt-in.
  • [x] _brand_wrapper MJML helper — verified at src/basketball_api/services/email.py:202. In active use by contract emails and roster emails. Queens branded (_BRAND_RED default).
  • [x] Blast endpoint via basketball-api / gmail-sdk — confirmed via tests/test_tournament_blast.py.

Stranded-18 Verification (DB query against live basketball-api)

Products 5/6/7 have exactly 23 orders. 5 paid, 18 pending — matches ticket counts. Revenue math: 5×$55 + 5×$65 + 8×$55 = $1,040 ✓.
  • [x] 17U Elite (prod 5) pending = 5: orders 51 (rjmob446), 53 (gabrieliuspeciulis08), 54 (derob81), 56 (zacbod1709), 93 (Westside Admin) — all match ticket.
  • [x] 17U Select (prod 6) pending = 5: orders 58 (Roni Webster), 59 (elsonolotu46), 60 (Eric Porter), 62 (Kandis & Jeff Froebe), 94 (Chaoying Fan) — all match.
  • [x] 16U Elite (prod 7) pending = 8: orders 63 (miroakbas), 64 (Mohamed Nur), 65 (ulgenersarp), 66 (yigitulgener), 67 (niyitangadaniel72 = Daniel), 68 (owencebully29), 69 (Artyom Litvinau), 70 (Vince Ifote Ifote) — all match.
  • [x] Paid exclusions (5): orders 52 (Spencer Thorn), 55 (Goudiaby Family), 57 (Marcel), 61 (Heather Jordan), 71 (fatkid816) — all verified status=paid, matches ticket exclusion list exactly.
  • [!] Order 93 concern — parent name is "Westside Admin", email westsidebasktball@gmail.com, player name "Test Kings Player". This appears to be an internal admin test order created Apr 15, not a real parent. Blasting a customer-facing apology to the Westside admin inbox is either intentional (QA check of the template) or a scope bug. Ticket does not acknowledge this. Recipient count of 18 drops to 17 if excluded.

Repo Placement

OK — all work lands in forgejo_admin/basketball-api. Script, MJML wrapper, blast endpoint, DB all in this repo. No cross-repo scope.

Dependencies

Sibling-related to board items #1023 (forgejo #488, 30-day expires_at infra fix) and #1024 (forgejo #487, metric emission). All three on board-westside-basketball in backlog. Ticket correctly notes #488 "ideally merges first" but #486 can proceed with 24h TTL in an emergency — this is the right tradeoff. Checklist item "Runbook updated… move into #488's scope" is slightly awkward — #488 is an infra bug, not docs. Runbook tracking should live in its own board item or explicitly deferred to #488's Epilogue.

Acceptance Criteria

Strong. 13 gates across three phases, each individually verifiable:
  • Phase A (6 approval gates) — binary pass/fail, clear actors (Lucas, Marcus, Ava).
  • Phase B (4 gates) — dry-run count, Lucas confirms, commit, Stripe spot-check. All verifiable via CLI + Stripe dashboard.
  • Phase C (3 gates) — send, webhook flip, post-send report. Verifiable via logs + DB.
  • Gate 5 (Ava reads back) is the mura-reducer — forces recipient count and name echo before blast. Good control.

Blast Radius

Recovery action is narrowly scoped — only touches 18 rows (status=pending, product_id in [5,6,7]). Script re-uses existing tournament_checkout.py helper which already carries the order_id metadata fix from #485 (closed). Parallel Stripe session creation paths exist at 6 sites (checkout.py, admin.py, jersey.py, register.py, tournament_checkout.py, regen script) — each still carries 24h TTL. Those sites are correctly scoped to #488, not this ticket. No bleed.
Email blast radius: 18 recipients. Inadvertent over-send risk mitigated by hard-coded product filter + dry-run gate. Under-send risk (e.g., Eric Porter email bounces) addressed by AC Phase C monitor step.

Decomposition Assessment

5-min rule: 13 AC, 1 repo, 1 script, 1 template. Agent-pass-wise Phase B is <5 min (dry-run + commit). Phases A and C are gated on human approval, not agent work. One agent can own the whole ticket with Lucas/Marcus wait states between phases. No decomposition needed.

Recommendation

  • [SCOPE] Clarify order 93 (Westside Admin test order, westsidebasktball@gmail.com) treatment. Options: (a) exclude it and update recipient count to 17 throughout the ticket + Gate 5 read-back; (b) keep it as intentional QA recipient and note that explicitly in Scope. Currently ambiguous.
  • [SCOPE] Create the story:payment-recovery entry on project-westside-basketball → stories-parent section (e.g. WS-S29: "As a parent whose checkout link expired before I could pay, I receive an apology with a fresh link so I can still complete payment"). Or, if team prefers, relabel board item to story:WS-S22 (existing "clear communications" story).
  • [SCOPE] Create arch-stripe-checkout note covering the 6 Stripe Session creation paths + expires_at/metadata conventions. This backs the arch:stripe-checkout label on this ticket and the sibling tickets #1023 and #1024. Alternatively relabel to arch:dataflow-westside-basketball (existing).
  • [BODY] Reword Checklist item 6 ("Runbook updated with actual recovery procedure (move into #488's scope to avoid PR churn)"). #488 is a Stripe TTL bug, not docs work. Either open a separate tracking item for the runbook update or move the clause to #488's Checklist directly (with confirmation).
Not blocking: 6-gate approval chain placement is exemplary — HARD STOP header, Phase A AC, and Constraints each re-state the rule. A reader skimming the ticket cannot miss it. The three memory references (feedback_email_blast_nuclear_gate, feedback_no_email_without_five_approvals, feedback_one_approval_one_send) are correctly cited. No refinement needed on the approval chain itself.