Review: C1 — app: Monthly subscription section

review-1071-2026-04-22 Review

review approved

Verdict: APPROVED

Board item: #1071 — C1 — app: Monthly subscription section
Forgejo issue: forgejo_admin/westside-streamlit#14 (open)
Type: Feature
Reviewer: Dottie
Date: 2026-04-22
Round: 3 (informal rounds 1+2 already completed; round 2 was APPROVED after placeholder + enum prose fixes)

Template Completeness

All Feature template sections present:
  • [x] Type — Feature
  • [x] Lineage — standalone, discovered 2026-04-23, blocked-by called out
  • [x] Repo — forgejo_admin/westside-streamlit
  • [x] User Story — full Marcus narrative with role/want/so-that
  • [x] Context — schema source, enum source, GRANTs already in place, mirror pattern called out
  • [x] File Targets — single file (app.py), explicit "files NOT to touch" list
  • [x] Acceptance Criteria — 9 criteria, all observable
  • [x] Test Expectations — ruff check + format, run commands listed
  • [x] Constraints — mirror pattern, single-file, tenant filter, no deps, enum source-of-truth rule
  • [x] Checklist — PR/lint/render/no-unrelated
  • [x] Related — links to story note, project, parent project, hands-off feedback memory

Traceability

  • [x] story:monthly label — Monthly Subscription Visibility
  • [x] story note verified — story-westside-streamlit-monthly (id 1593) exists, tag user-story, status active, project westside-streamlit. Role/want/so-that match issue body.
  • [~] story note NOT yet listed in project-westside-streamlit user-stories table — table currently shows triage / blast / clusters / jerseys / audit / reachable. Missing: monthly row + tournament row (separate ticket #1073's story).
  • [x] arch:streamlit-app label — references the streamlit app component
  • [~] arch note arch-streamlit-app not located via slug lookup. Architecture is described in project-westside-streamlit Architecture section, which is the convention for this small project. Acceptable for this round.
  • [x] Forgejo issue — open, body well-formed
  • [x] type:feature, track:backend, scope:planned labels — all present

File Targets

  • [x] app.py — verified at /home/ldraney/westside-streamlit/app.py, 211 lines. "After line ~211" matches actual EOF.
  • [x] app.py:36-39 q() helper — verified, matches issue claim (cached psycopg + pandas read_sql).
  • [x] app.py:51-112 Offered pool — verified as the mirror pattern (header → query → filters → dataframe → cluster detection → cohort export).
  • [x] app.py:103-109 parent-cluster detection — verified, exact pattern referenced in AC #6.
  • [x] app.py:111-112 email cohort export — verified (st.text_area pattern).
  • [x] ~/basketball-api/src/basketball_api/models.py SubscriptionStatus — verified at lines 101-105. Four values exactly: active, past_due, canceled, none. Constraint to read enum from source (not hardcode) is sound.
  • [x] Migration 044 GRANTs — verified at ~/basketball-api/alembic/versions/044_add_westside_streamlit_ro_role.py. GRANT_TABLES includes players, parents, teams, player_teams. subscription_status + monthly_fee columns live on the already-granted players table — no new GRANT needed. Issue claim is accurate.

Repo Placement

OK. Issue is filed on westside-streamlit; the only file change is app.py in that repo. The basketball-api reference is read-only (consult the enum file, no edits) — explicitly called out in "Files NOT to touch" and consistent with feedback_basketball_hands_off.

Dependencies

Blocked-by chain explicit in issue body:
  • forgejo_admin/pal-e-services#60 — Harbor + ArgoCD Application (open, board item #1069 backlog)
  • forgejo_admin/pal-e-services#61 — Keycloak client (open, board item #1070 backlog, supersedes #10)
Substrate must be live before this dashboard change can deploy. Not a scope problem — correctly noted as a blocker, and #1071 is in backlog (won't enter todo/next_up until substrate clears). Sibling related ticket #1074 (O1 onboarding) is the final downstream consumer.

Acceptance Criteria

All 9 criteria are observable and verifiable by an agent:
  • Header presence — DOM check after Jerseys section
  • 4 metric cards — count + label match enum values
  • Filterable grid columns — column list specified
  • Filter widgets — Program (3-way) + Status (4-way radio)
  • Totals caption — "Showing N of M" pattern matches existing
  • Parent-cluster block — pattern referenced by line range
  • Email cohort textarea — pattern referenced by line range
  • q() usage + read-only — testable by code review (no INSERT/UPDATE/DELETE)
  • No new dependency — testable by git diff requirements.txt
Test commands (ruff check app.py, ruff format --check app.py) are real and run cleanly today.

Blast Radius

Low.
  • Single file, append-only section.
  • Read-only SQL — no writes to basketball-api or its DB.
  • No schema/ACL changes — column already exposed via existing GRANT.
  • No new dependency — uses existing q() helper, existing st.dataframe + st.text_area patterns.
  • Auth gate at app.py:13-31 already in place — Keycloak admin role required before any query runs. New section inherits this gate by virtue of being below it.
  • No similar buggy pattern to grep for — this is additive, not a fix.
  • Downstream: Marcus dashboard view, no API consumers.

Decomposition Assessment

No decomposition needed. 5-minute rule:
  • 1 file target across 1 repo — well under the >3/>2 threshold
  • 9 AC, but they are all features of one section, not independent work — verifiable in one screenshot
  • Estimated agent work: append ~50-60 lines mirroring the Offered pool pattern, ~3-5 minutes
  • Single PR, single review

Recommendation

APPROVED — ready to advance to todo once Lucas confirms.
One discovered scope item (does NOT block this ticket):
  • [SCOPE] Add monthly row to project-westside-streamlit user-stories table linking to story-westside-streamlit-monthly (id 1593). Story note exists and is correct — only the project-page table needs the new row. The tournament story for sibling ticket #1073 has the same gap. Per feedback_discovered_scope_always_tracked, this should become its own backlog item rather than be folded into #1071.
No [BODY], [LABEL], or [DECOMPOSE] recommendations — issue body, board labels, and ticket size are all correct.

Verification Provenance

  • Forgejo API: GET /repos/forgejo_admin/westside-streamlit/issues/14 (open, body matches input)
  • Forgejo API: GET /repos/forgejo_admin/pal-e-services/issues/60 (open) and #61 (open) — blockers confirmed open
  • Filesystem: ~/westside-streamlit/app.py (211 lines, all referenced lines match)
  • Filesystem: ~/basketball-api/src/basketball_api/models.py:101-105 (SubscriptionStatus enum, 4 values)
  • Filesystem: ~/basketball-api/alembic/versions/044_add_westside_streamlit_ro_role.py (GRANT_TABLES verified)
  • pal-e-docs: story-westside-streamlit-monthly id 1593 (exists, project-bound, user-story tag)
  • pal-e-docs: project-westside-streamlit user-stories section (no monthly row — recorded as discovered scope)
  • pal-e-docs: board-westside-streamlit via list_board_items (item #1071 in backlog, dependencies #1069/#1070 also in backlog)