pal-e-docs

Review: Implement billing follow-up agent for Westside Basketball

review-1903-2026-07-25-r2 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Re-review after refinement. Prior review: review-1903-2026-07-25. All [BODY] issues from the prior review have been resolved. Remaining issues are [SCOPE] items only -- companion artifacts that need to be created.

Prior Review Resolution

  • [x] [BODY] Resolve data access approach -- RESOLVED. Issue now explicitly commits to JSON API on Rails with 5 named endpoints. Prerequisite documented in "What Must Be Built" section with code blocks, routes, and env var requirements.
  • [x] [BODY] Resolve email sending approach -- RESOLVED. Issue explicitly states "Email sending uses Postmark outbound (not Gmail/ActionMailer). Nemo sends directly via Postmark API, not through the Rails app." Repeated in Constraints.
  • [x] [BODY] Fix ParentMailer#billing_followup reference -- RESOLVED. Issue now correctly lists the 3 public mailer methods (announcement, payment_reminder, quick_send) and explicitly states "There is NO billing_followup method. Nemo does NOT use Rails mailers."
  • [x] [BODY] AC vagueness for edge cases -- IMPROVED. AC #10 now enumerates specific edge cases: "no-show players, disputed charges, players who left program."
  • [ ] [SCOPE] Create project-nemo page -- STILL MISSING (404)
  • [ ] [SCOPE] Create arch-nemo-agent note -- STILL MISSING (404)
  • [x] [DECOMPOSE] Owner explicitly chose to keep as single ticket -- ACKNOWLEDGED, respected

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Standalone, first ticket for Nemo project
  • [x] Repo -- ldraney/nemo
  • [x] User Story -- well-formed (As Marcus, I want an AI agent that emails me billing drafts...)
  • [x] Context -- thorough, includes key architectural decisions and "What Exists Today" inventory
  • [x] File Targets -- 13 files to create, 3 directories excluded with reasons
  • [x] Feature Flag -- none
  • [x] Acceptance Criteria -- 13 criteria
  • [x] Test Expectations -- 5 test items + run command
  • [x] Constraints -- 8 constraints listed
  • [x] Checklist -- 4 items
  • [x] Related -- 6 references
All required template sections present. The issue is notably thorough, with additional sections ("What Exists Today in the Rails App" and "What Must Be Built") that provide excellent context for the implementing agent.

Traceability

  • [x] story:billing-follow-up label -- present on board item
  • [ ] story note MISSING -- project-nemo page does not exist (404). [SCOPE] Create project page project-nemo with user-stories section containing the billing-follow-up story.
  • [x] arch:nemo-agent label -- present on board item
  • [ ] arch note MISSING -- arch-nemo-agent note does not exist (404). [SCOPE] Create architecture note arch-nemo-agent. The nemo repo's docs/architecture.md has detailed system diagrams and component descriptions to derive from.
  • [x] Forgejo issue -- ldraney/nemo#1, state: open

File Targets

  • [x] src/__init__.py -- to create; standard package init
  • [x] src/webhook.py -- to create; repo currently docs-only, valid path
  • [x] src/agent.py -- to create; consistent with architecture doc
  • [x] src/tools.py -- to create; consistent with architecture doc
  • [x] src/config.py -- to create; consistent with architecture doc
  • [x] Dockerfile -- to create; standard containerized service
  • [x] pyproject.toml -- to create; standard Python project
  • [x] .pre-commit-config.yaml -- to create; matches platform convention
  • [x] .woodpecker/ci.yaml -- to create; matches platform CI convention
  • [x] tests/__init__.py -- to create; test package init
  • [x] tests/test_webhook.py -- to create; webhook parsing tests
  • [x] tests/test_approval.py -- to create; approval parser tests
  • [x] tests/test_fixes.py -- to create; standard fixes logic tests
  • [x] docs/ -- excluded, already exists in repo (verified)
  • [x] projects/ -- excluded, workflow.md exists in repo (verified)
  • [x] README.md -- excluded, already exists (verified)
All file targets valid for a greenfield service in a docs-only repo.

Rails App References Verified

The "What Exists Today" section makes specific claims about the westside-basketball codebase. All verified against the actual code:
  • [x] WebhooksController inherits from ActionController::Base -- confirmed
  • [x] No API controllers exist (app/controllers/api/ directory absent) -- confirmed
  • [x] No API_TOKEN env references anywhere in the app -- confirmed
  • [x] Player model: has_many :orders, :payment_links, :player_tournaments; columns stripe_customer_id, subscription_status, monthly_fee, contract_status, parent_id -- all confirmed in schema
  • [x] Parent model: has_many :players; columns name, email, phone -- confirmed
  • [x] Order model: belongs_to :player, :parent, :product; columns amount_cents, status (orderstatus enum) -- confirmed. Minor: ticket says "pending/paid/canceled" but enum also includes "refunded"
  • [x] PaymentLink model: belongs_to :player, :parent, :order (optional); columns description, amount_cents, status, stripe_payment_link_id, stripe_payment_link_url -- confirmed
  • [x] PlayerTournament model: belongs_to :player, :tournament; validates payment_status against PAYMENT_STATUSES (pending/paid_stripe/paid_cash/paid_venmo/waived) -- confirmed
  • [x] ParentMailer: 3 public methods (announcement, payment_reminder, quick_send), no billing_followup -- confirmed
  • [x] jbuilder gem installed but unused -- confirmed
  • [x] No rack-cors gem -- confirmed
  • [x] TENANT_SLUG = "westside-kings-queens" in ApplicationController -- confirmed
  • [x] StripeClient and GmailClient services exist -- confirmed

Repo Placement

Issue filed on ldraney/nemo. All file targets are in the nemo repo. Cross-repo Rails API work is explicitly documented as a prerequisite in the "What Must Be Built" section with full controller code, route definitions, and env var requirements. The Checklist includes "Rails API prerequisite ticket created on westside-basketball repo." This is correct placement.
However, the Rails API prerequisite ticket does NOT yet exist on westside-basketball (checked all 19 open issues -- none match).

Dependencies

  • Only item on board-nemo; no board-level blockers or blocked items.
  • Cross-repo prerequisite (documented but not created): 5 JSON API endpoints + BaseController + API_TOKEN env var on westside-basketball. The issue documents these thoroughly but the actual Forgejo issue has not been created yet.
  • Operational dependencies (documented in Constraints): Postmark account, Anthropic API key, Stripe API key, k8s namespace -- not code dependencies, deployment prerequisites.

Acceptance Criteria

13 acceptance criteria. All are testable:
  • [x] AC 1 (POST /inbound parses Postmark JSON) -- testable via webhook mock
  • [x] AC 2 (Agent pulls billing data via Rails API) -- testable with mock; depends on Rails prerequisite
  • [x] AC 3 (Standard fixes: rename dues, cancel dupes, add missing fees) -- testable with mock data
  • [x] AC 4 (Draft billing email with itemized table) -- testable
  • [x] AC 5 (Send test email to Marcus via Postmark) -- testable
  • [x] AC 6 (Parse Marcus's approval reply) -- testable, approval language explicitly enumerated
  • [x] AC 7 (Send to parent only after explicit approval) -- testable
  • [x] AC 8 (One player at a time, sequential) -- testable
  • [x] AC 9 (Resume from persistent state file) -- testable
  • [x] AC 10 (Flag edge cases) -- testable, specific cases enumerated
  • [x] AC 11 (Postmark error handling with retry/backoff) -- testable
  • [x] AC 12 (Ruff lint + format in CI) -- testable
  • [x] AC 13 (pytest tests pass) -- testable
One minor note: the approval parsing constraint is well-specified -- "send it"/"approved"/naming the recipient count as approval; "Push it"/"let's go"/"ASAP" do NOT. This is a critical safety gate that is clearly testable.

Blast Radius

  • Nemo is a new standalone service; blast radius within its own repo is contained.
  • Cross-repo impact on westside-basketball is limited to adding a new API namespace (no existing code modified).
  • No existing Postmark integration in the infrastructure -- net-new.
  • Documentation inconsistency in nemo repo: projects/westside-billing-agent/workflow.md still references Gmail API, ParentMailer#billing_followup, POST /api/v1/emails/send, and kubectl exec approach. docs/architecture.md diagram has stale "send email" label on the Rails connection and mentions "Gmail" in the intro. These contradict the issue's explicit Postmark approach. An implementing agent reading these docs could get confused.

Decomposition Assessment

Thresholds exceeded but owner chose single ticket:
  • 13 file targets in 1 repo -- exceeds 3+ threshold (but single repo, not >2 repos)
  • 13 acceptance criteria -- exceeds 5 AC threshold
  • Estimated agent work far exceeds 5 minutes -- full greenfield service
  • Owner explicitly chose to keep as single detailed ticket rather than decompose
The issue's level of detail (inline code blocks, exact routes, column-level model descriptions) partially compensates for its size. An agent has enough specificity to implement without ambiguity, even though the scope is large. Respecting owner's decision.

Recommendation

  • [SCOPE] Create project page project-nemo with user-stories section containing the billing-follow-up story entry. (Carried forward from prior review.)
  • [SCOPE] Create architecture note arch-nemo-agent for the agent component. Derive from docs/architecture.md in the nemo repo. (Carried forward from prior review.)
  • [SCOPE] Create the Rails API prerequisite ticket on westside-basketball. The issue body contains complete spec material (controller code, routes, env var) -- this should be extracted into a separate Forgejo issue before work starts.
  • [SCOPE] Update projects/westside-billing-agent/workflow.md and docs/architecture.md in the nemo repo to reflect the Postmark decision. Currently they reference Gmail API, ParentMailer#billing_followup, and POST /api/v1/emails/send, which contradict the issue's Postmark approach. This should happen before an agent starts implementation to avoid confusion.