Skip to content

Coaching schema cleanup — sessions under students; coaching invoice as map field on session

Why (owner, 2026-06-20)

Coaching is conceptually student-centric — the coaching page enters by student, then maneuvers within. Owner confirmed Students/{abbr}/Sessions/ is currently EMPTY in tebs-mel, so the migration is additive, not a dual-source cleanup. And the matchable-coaching-payments.ts:103 endpoint already reads studentDoc.ref.collection('Sessions') — currently returning nothing, which means session ordinals on that endpoint are wrong / undefined. Migration accidentally fixes that bug on the way through.

Also: coaching invoices are 1:1 with sessions (verified — writer at app/coaching/invoice/[sessionId]/client.tsx:617-659 derives the invoice number deterministically from (year, studentAbbr, sessionOrdinal) and setDocs to one path; no multi-invoice code path). So coaching invoice doesn't need its own subcollection — it can live as a map field on the session doc.

Target shape (tebs-mel)

Students/{studentAbbr}/Sessions/{sessionId} {
  …existing session fields…,
  invoice: { …the coaching invoice as a map field… }
}
Top-level Sessions/ collection is deleted after migration. The Sessions/{id}/invoice/{docId} subcollection is deleted after migration.

Consumer conversions (~9 sites + writer)

Site Today After
pages/api/accounting/matchable-coaching-invoices.ts (Records tab + matchable list) Sessions.get() + iter .collection('invoice') collectionGroup('Sessions').get() + .invoice map
lib/accounting/derivedJournals.server.ts:1217 (COACHING_ISSUED journals) mel.collectionGroup('invoice').get() collectionGroup('Sessions') + .invoice map
lib/accounting/reports.server.ts:1660 (AR aging) same same
pages/api/accounting/coaching-auto-match.ts:120 (lookup by invoice number) .collectionGroup('invoice').where('invoiceNumber','==',X).limit(1) collectionGroup('Sessions').where('invoice.invoiceNumber','==',X).limit(1) + index
lib/accounting/coachingWopcAuto.ts:89 same same
pages/api/coaching/invoices/[sessionId]/saved.ts:96, pdf.ts:76, lib/share/fetchSharedDoc.server.ts:262 Sessions.doc(id).collection('invoice').limit(1) Sessions.doc(id).get() then read .invoice
app/coaching/invoice/[sessionId]/client.tsx:617-659 (writer) setDoc(Sessions/{id}/invoice/{docId}, …) update(Sessions/{id}, { invoice: {…} })
lib/accounting/transactions.ts:3772-3775 (match references invoice subcollection) path traversal path goes away
2 cross-student session queries Sessions.get() collectionGroup('Sessions').get()
pages/api/accounting/matchable-coaching-payments.ts:103 (latent bug) reads empty Students/.../Sessions/ reads the migrated location — bug self-resolves

Plus a Firestore composite index on invoice.invoiceNumber for the two lookup-by-number queries.

Other ride-along cleanup

  • Drop Sessions/{id}.paid write on match (zero readers; safe per T-075 audit).
  • Migrate the 2 readers of Sessions/{id}/payment/{txId} (compute.ts hasSessPayment, SessionsTab "Pay On" date) to derive from gl.coachingInvoices on the bank tx, then drop that subcollection write.
  • Students/{abbr}/Payments/{txId} stays — appliedAmount/remainingAmount track partial-allocation state that doesn't exist on the bank tx side.

Risks / questions

  • Cross-DB writes (coaching in tebs-mel, bank tx in tebs-erl) can't be atomic. Same trade-off we accept today. The bank-tx-side gl.coachingInvoices remains canonical.
  • The migration touches every existing session + coaching invoice in tebs-mel. Backup the whole Sessions/ subtree first.

T-075 (gl[4001] write — first half of the structural fix) · T-076 (derived-as-SoT — same architectural direction) · T-078 (invoice shape migration — runs first; coaching invoice migrates inside the new detail.* shape).

Log

  • 2026-06-20 created. Owner confirmed Students/.../Sessions/ empty in prod; 1:1 invoice-to-session verified by writer code inspection; Records-page tab confirmed as cross-student consumer. Logged for after T-077 ships.
  • 2026-06-20 PHASE A SHIPPED in PR #765 (claude/coaching-schema-cleanup-yxMLM, base #764). Collapsed Sessions/{id}/invoice/{docId} → Sessions/{id}.invoice map field. Dual-read pattern across 10+ consumer sites via new getCoachingInvoiceForSession + findCoachingInvoiceByNumber helpers (map-field first, legacy subcollection fallback). Writer at app/coaching/invoice/[sessionId]/client.tsx switches to updateDoc on the session; updateLogs move to Sessions/{id}/invoiceUpdateLogs. Match path in lib/accounting/transactions.ts uses the helper for validation; T-078's syncCoachingInvoiceTxTrail detects shape. Migration script scripts/migrate-coaching-invoice-shape.ts hoists every legacy subcollection doc into the session map + moves updateLogs + deletes the legacy. tsc + accounting suite (114) clean.
  • 2026-06-20 PHASE B starting on a new branch off Phase A — move Sessions from top-level Sessions/ → Students/{abbr}/Sessions/{id} in tebs-mel. ~2 cross- student session sweeps convert to collectionGroup('Sessions'). The latent matchable-coaching-payments.ts:103 bug (reads empty Students/.../Sessions/) self- resolves on the way through.
  • 2026-06-20 status flipped to done — Accounting [Infrastructure Development] (housekeeping; the work itself shipped via PR #767 and the T-080 cleanup chain). Source: Accounting [Infrastructure Development] · https://claude.ai/code/session_015P6KzVYsQCLgEmUjR9bMwM (Source line retrofitted 2026-06-21 — the convention postdates this entry.)

2026-06-24 — POSTSCRIPT: invoice-number gap in the migration's done-state, now backfilled

  • ✅ Attestation (Accounting (Infrastructure)): read AGENTS.md; appending to the task I own (kept done) to record a gap found + closed. Branch signal main 🟢 · nightly 🔴 → main only; mirrored to taskboard.
  • Source: Accounting (Infrastructure) · https://claude.ai/code/session_015P6KzVYsQCLgEmUjR9bMwM
  • Gap (surfaced by I-012, owner report 2026-06-24): this migration carried legacy coaching invoice bodies into the invoice map, but those bodies never stored invoice.invoiceNumber (the number was always derived at render time). T-079's own design specced invoice.invoiceNumber as a queryable/indexed field, so the field being absent on all 15 migrated sessions was an incomplete done-state — the Records → Coaching Invoices tab read it and, finding none, showed empty.
  • Closed by: a deterministic backfill (owner chose data-fix over read adaptors) — recomputed the canonical MEL-{year}/{abbr}-{ordinal:03d}(PR) via the same generateRequestNumber the writer uses, wrote it to all 15 legacy sessions in prod tebs-mel. 15/15, backup taken, verified; matched the pre-existing detail.invoice.number byte-for-byte on the sample. Full detail + the committed scripts/backfill-coaching-invoice-number.py are in I-012.
  • Note: the live writer (app/coaching/invoice/[sessionId]/client.tsx) already persists invoice.invoiceNumber for new invoices, so no recurrence — this was a one-time legacy gap.
  • Blast radius: data-only (15 invoice.invoiceNumber fields added in tebs-mel); no code, no deploy, no schema change. Touches the same coaching invoice surface as T-022/T-052 in flight, but is purely additive (a field add) so it doesn't conflict with their restructure.

2026-06-24 — POSTSCRIPT 2: client-side parallel impls deleted (last legacy-subcollection read removed)

  • ✅ Attestation (Accounting (Infrastructure)): read AGENTS.md; appending to my own done task to close the final consumer-conversion gap. Branch signal main 🟢 · nightly 🔴 → main only; mirrored to taskboard.
  • Source: Accounting (Infrastructure) · https://claude.ai/code/session_015P6KzVYsQCLgEmUjR9bMwM
  • Gap (flagged at the bottom of T-112's close-out): lib/accounting/derivedJournals.ts:512 (a client-side mirror of derivedJournals.server.ts) was still reading the legacy Sessions/{id}.collection('invoice') subcollection — the exact shape T-079 collapsed. The T-079 design table listed the server variant for conversion (derivedJournals.server.ts:1217, Phase A shipped that); the client mirror was missed because it's parallel code, never indexed by the design grep.
  • Decision — delete, don't migrate: the client-side modules were a parallel implementation of the report + journal generators. Architecture comment at lib/accounting/reports.server.ts:41-44 explicitly disavows this ("Pull journal entries from the canonical source instead of maintaining a parallel implementation here") — the live wiring everywhere is via the *Server variants (getDerivedJournalEntriesServer in derivedJournals.server.ts, generate{TrialBalance, ProfitAndLoss,BalanceSheet,ARAgingReport}Server in reports.server.ts). The client variants are reachable only through the lib/accounting/index.ts barrel and nothing imports them (verified — barrel grep shows zero callers of getDerivedJournalEntries / getDerivedJournalEntry / generateTrialBalance / generateProfitAndLoss / generateBalanceSheet / generateARAgingReport). Migrating them would be polishing dead code; deleting them is the architecture-aligned move.
  • What landed:
  • git rm lib/accounting/derivedJournals.ts (~735 LoC, including the buggy legacy subcollection iteration at line 512 + the inlined reconstruction fallback I just refactored in T-112).
  • git rm lib/accounting/reports.ts (~500 LoC; the client-side TB/P&L/BS/AR-aging mirror that fed off it).
  • lib/accounting/index.ts — removed the two re-export blocks (getDerivedJournalEntries{,Entry}
    • the four generate*Report exports). No other shape change.
  • Note on T-112: the helper lib/accounting/coachingInvoiceNumber.ts introduced in T-112 is still used — by the LIVE call site lib/accounting/reports.server.ts:1685. Only the derivedJournals.ts client mirror site went away with the file. T-112 is unaffected.
  • Verification: npx tsc --noEmit -p tsconfig.json → same 2 pre-existing lib/wopc.server.ts errors as before; nothing new (i.e. nothing was importing the deleted modules). Filtered grep for the six removed export names across the repo → zero hits outside the deleted files / the barrel edit itself.
  • Blast radius: dead-code removal only; no live behaviour change. The server-side canonical generators remain unchanged. With this, every consumer in T-079's design table is on the post-T-079 shape (invoice map field, no subcollection iteration anywhere in live code).

Commit index (backfilled 2026-07-01, best-effort · Coaching (Diagnostic))

Candidate related commits, auto-backfilled from git on main: commits whose message references this task's UID or a PR number it cites. Not verified — this squash-merged history can't yield a precise per-task list, so rows tagged (mentions only) name the task in passing (may be tangential) and untagged work commits may be missing. Treat as a starting point: verify, prune tangential rows, and append any real ones per the AGENTS.md "record every related SHA" policy.

  • 6dfb0a1 2026-06-24 — fix(records): restore WOPC list after T-089 migration + drop the All tab (mentions only)
  • ab59c65 2026-06-24 — docs(board): T-110 (WOPC list fix + drop All tab) + I-011/I-012 symptom tickets (mentions only)
  • ff14ab6 2026-06-24 — Merge #806: fix(records) — restore WOPC list after T-089 migration + drop the All tab (mentions only)
  • 1eb290a 2026-06-24 — refactor(T-112): harden coaching invoice-number reconstruction fallback (mentions only)
  • 851002f 2026-06-24 — chore(T-079): delete dead client-side parallel impls (derivedJournals.ts + reports.ts)
  • a56f444 2026-06-29 — docs: refresh Accounting (Infrastructure) session ledger + T-130 review note (mentions only)
  • 465fbab 2026-07-01 — fix(coaching): re-route Coaching page onto nested Sessions + re-home orphaned legacy sessions (#830) (mentions only)
  • a5484f5 2026-07-01 — coaching: orphaned session vouchers/history recovery + card-height fix + rules-deploy docs (#831) (mentions only)