Skip to main content

Invoicing Fix Plan — from the 2026-07-06 Fable review

Source: docs/reviews/invoicing-fable-review.md. Status: APPROVED 2026-07-06 — all decisions (D1–D7) resolved; ready to execute Step 0 → PR-A. Execution model: one PR per work package, sequential, conventional commits, branch type/short-description, CI green before merge. Any diff touching src/db/migrations/, src/lib/invoice/, or src/server/services/invoice/ gets a Fable-level review before commit (money path rule).

Ordering rationale​

Ranked by (money at risk × likelihood it's happening now) ÷ effort. Step 0 is ops, not code, and can happen today. PR-A is trivial and clears the "actively misleading" items immediately. PR-C and PR-B close the two active leaks. PR-D builds the AR chase. PR-E is the highest-severity code defect but lowest frequency, so it goes after the standing leaks. PR-F/PR-G are quality hardening. The runbook closes the single-person risk.

Step 0 — Prod verification (no code, Oliver + assistant, TODAY)​

  1. select jobname, schedule, active from cron.job; against prod — confirm the six pg_cron jobs from docs/cron-setup.md are actually scheduled (especially accrue-overdue-interest). If missing, run the SQL template from that doc.
  2. Confirm RESEND_API_KEY + EMAIL_FROM are set in the prod deployment (else every "send" records email skipped).
  3. Commit the review report + this plan to main (docs-only commit).

Deliverable: a yes/no per item recorded at the bottom of this file.

PR-A — fix/invoicing-copy-and-labels (model: sonnet; haiku-simple but one item has logic)​

Small, zero-risk-to-money, ships first.

  1. Billing-policy header copy — src/app/admin/invoices/page.tsx:522-528: replace "Invoices bill contracted hours…" with the true lesser-of-logged-and-contracted description (mirror the InvoiceShelf wording).
  2. DOW label off-by-one — src/components/admin/invoicing/LogWeekHoursForm.tsx:19,101-105: delete the hardcoded DOW array; derive the weekday label from each date (fromISO(d) → weekday). Also fix the stale "(Sun–Sat)" doc comments here and in PeriodBar.tsx:17-20.
  3. Sent · <substatus> badge — InvoicingCockpit.tsx status cell: when r.status === 'sent', append the underlying existingInvoiceStatus (paid / partial / overdue — derive overdue via effectiveInvoiceStatus with the row's due date if available, else show stored status). Data is already on the row; display-only change.
  4. Tests: one Vitest case for the weekday-label helper (Monday-anchored week renders Mon..Sun).

Review: sonnet self-review + CI. No Fable pass needed (no money path).

PR-B — feat/overage-aging-and-rerequest (model: opus; Fable review on the SQL)​

Closes ranked leak #1 (unapproved overage silently never billed).

  1. Age + expiry columns on /admin/overage (src/app/admin/overage/page.tsx): per pending group show days-pending and the approval link's expires_at state (active / expiring ≤3d / expired). Sort oldest first.
  2. One-click re-request for expired/expiring links: re-use requestOverageApproval (src/server/actions/overage.ts:122) to mint a fresh token + email. Owner-triggered (see Decision D1 — not auto-send by default).
  3. "Unbilled overage" KPI: extend needs_attention_counts (or add a sibling aggregate in a new migration) to return Σ(overage hours × assignment rate) for pending groups in the trailing 4 weeks; render as a dashboard Needs-attention line + link to /admin/overage.
  4. Tests: aggregate math (SQL or via a query-layer test) + expiry-bucket logic.

Fable review: yes (new SQL aggregate touching billing figures).

PR-C — fix/post-send-drift-and-lock (model: Fable — money-critical RPC changes)​

Closes ranked leak #2 (sent invoices silently diverge from recorded hours).

  1. Lock acknowledged-unapproved entries at send — new migration (009x_send_lock_all_payable.sql) revising send_invoices: on an acknowledged send, flip submitted/pending_approval entries in the window to locked too (currently only approved, 0045:319-327). Subtlety: void_invoice unlocks to approved — that would launder never-approved hours into approved. Record prior status (new column time_entries.status_before_lock or lock-audit row) and restore it on void.
  2. Sent-week drift surfacing — cheapest correct version: extend weekly_billing_run to also return existing_invoice_total_cents; the cockpit then badges a sent row whose live billable_amount_cents ≠ billed total ("Hours changed since billing — review/reissue"). Trailing-weeks coverage: add a drifted-sent-invoices count (trailing 4 weeks) to needs_attention_counts so it surfaces even when nobody opens that week.
  3. Tests: RPC-level tests for lock/unlock round-trip incl. void restore; drift-flag fixture (late entry after send flips the badge).

Fable review: it is Fable work. Second-opinion pass by opus on the migration for regressions.

PR-D — feat/invoice-reminders (model: opus; Fable review on send-side)​

Closes ranked leak #3 (no AR chase). Builds on rails that already exist (outbox seam ADR-0035, dead columns facilities.reminder_schedule / email_template_reminder / email_template_overdue). Amended per D5: the email is a consolidated per-facility statement, not one email per invoice.

  1. Define reminder_schedule shape: { "days_after_due": [3, 10, 21] } (company default in settings, per-facility override in the existing column). Migration seeds the default.
  2. New cron route /api/cron/invoice-reminders (mirror accrue-interest route pattern + CRON_SHARED_SECRET). Overdue is always DERIVED from due_date + unpaid balance — never trust the stored status.
  3. Consolidated statement email, one per facility per tick that has newly-due stages — two sections:
    • Section 1 — Overdue, payment requested: table of overdue invoices (number, period, amount, balance, days late) + total due now, worded as a payment request. Interest warning included ONLY when facilities.interest_enabled ("per the contract, N%/mo late interest applies; accrued to date: $X" from interest_accrued_cents). No interest sentence at all for facilities without contractual interest.
    • Section 2 — Coming due (unpaid, not yet due): number, amount, due date — "how much and when," so one email is the facility's full open-balance picture. Early stages (due+3) use email_template_reminder tone; the final stage (due+21) uses email_template_overdue.
  4. Idempotency stays per invoice per stage (invoice_reminders(invoice_id, stage, sent_at) table): a cron tick collects all invoices hitting an unfired stage, groups by facility, sends ONE email per facility covering everything open, then marks all triggered stages fired. Same invoice-stage never fires twice; consolidation never suppresses a stage.
  5. Recipients: billing contacts (else admin), CC cc_on_invoice contacts — mirror the invoice send path. Record in outbound_emails with kind: 'reminder'.
  6. Facility settings UI: expose the two templates + schedule override on the facility form.
  7. Wire the cron: add the cron.schedule statement to docs/cron-setup.md and run it in prod (per Step 0 learnings).
  8. Tests: stage idempotency, consolidation grouping (2 overdue + 1 coming-due = one email, both sections), interest sentence gated on interest_enabled, overdue derivation, token fills.

Fable review: yes (outbound money-adjacent email + new table).

PR-E — fix/mid-week-assignment-hours (model: Fable — REDO of the core picker)​

Fixes A1 (mid-week assignment change silently drops hours / re-prices the week). Scope per Decision D3; recommended: minimal version:

  1. Keep pickLatestPerContractor for rate/contracted-hours terms, but sum time_entries across all of the contractor's in-window assignment ids at that facility (not just the picked one) in all four sites: generate.ts:261-267, weeklyRun.ts shelf query, weekly_billing_run SQL (entry_hours join), and the facility-shelf activity map. Hours can no longer vanish; the latest terms still price the whole week (visible, policy-consistent, and what the current code intends).
  2. Migration for the SQL function; TS changes mirrored; the audit's note that pickLatestPerContractor is reimplemented 4× — consolidate to one shared helper while touching it (src/lib/invoice/ pure function + SQL stays SQL).
  3. Tests: fixture with an assignment superseded mid-week (entries on both ids) — invoice bills all hours; rate change mid-week prices at latest rate; no-change weeks byte-identical output (regression guard).
  4. Full per-segment pricing (each entry billed at its own assignment's rate) only if D3 chooses it — bigger blast radius: line model, PDF description, variance basis.

Fable review: it is Fable work + opus second-opinion on the migration.

PR-F — feat/gap-diagnostics (model: opus)​

B1/B4/B5 — make the gap flag diagnostic instead of binary.

  1. Cause hints: when a row is gap, annotate why-candidates — contractor has entries at another facility this week (wrong-facility likely), last Hubstaff sync reported skipped members (mapping likely), else true zero. Extend the run RPC or do a cheap follow-up query server-side.
  2. Unblock covered lines (Decision D4): allow Generate for a gap facility — the engine already handles $0 lines with an under-contract reason; the block is purely the UI canGenerate (InvoicingCockpit.tsx:33-39). Gate behind the same confirmDanger flow with the gap named explicitly.
  3. Gap streak: per-assignment consecutive-gap-week count (SQL window over prior runs or a computed query); after 3+, nudge "end-date this assignment?".
  4. Leave-window honesty (B5): replace binary leave silencing with a coverage comparison — leave days overlapping the week vs 0-hour weekdays; partial coverage renders "leave (partial)" instead of silencing.
  5. Tests: each hint path, streak counting, partial-leave fixture.

Fable review: yes for the weekly_billing_run migration portion.

PR-G — fix/correction-guards (model: sonnet; Fable review on the RPC param)​

A4 + A6 + A8 + E-routing/dead-end — small hardening batch.

  1. correctInvoice guard — invoiceManage.ts:258-273: refuse origin='app' invoices that aren't draft (point at Correct/reissue). One conditional + test.
  2. Per-invoice acknowledge — send_invoices takes p_acknowledged_ids uuid[] instead of one boolean; ReviewAndSendDialog passes exactly the checked-with-unapproved ids. Migration + action + dialog change.
  3. Due date at send — in send_invoices, re-stamp due_date = now() + facility terms when flipping to sent; then best-effort re-render the PDF post-send in performSend before the email attach when the due date moved (machinery exists: renderAndStoreInvoicePdf). Decision D6 confirms.
  4. Gap-row routing + shelf confirm — openShelf lets a gap row reach the invoice shelf (Log hours lives there), and InvoiceShelf.generate handles needsConfirm with the same confirmDanger dialog the cockpit uses instead of a dead-end error.
  5. Tests: guard rejection, per-invoice ack matrix (checked/unchecked × approved/unapproved), due-date restamp.

Fable review: yes (send RPC + due-date changes are money path).

PR-H — feat/facility-invoice-history (model: sonnet; Fable review — touches weeklyRun service)​

New requirement 2026-07-06: "a section in invoicing showing all invoices sent to a selected facility with a status."

Where it lives (recommendation): two connected surfaces, no new page —

  1. Facility shelf gains an "Invoices" section (FacilityShelf.tsx, below Contract): the facility's last 6 invoices — number, period, amount, and a status pill using the DERIVED status (effectiveInvoiceStatus, so overdue shows as overdue, not "sent") plus balance when partial. Header line: "Outstanding: $X across N open invoices." Data: extend fetchFacilityShelf with one query over invoices for the facility (no new RPC needed).
  2. "All invoices for this facility →" link at the bottom of that section, deep-linking to the All-invoices tab with a new ?facility=<id> filter. Implementation: add facility to the existing in-memory filter chain in page.tsx (same pattern as ?filter=overdue) + a facility dropdown in the FilterBox row.

Why here and not a new page: the facility shelf is already the "who is this facility" panel one click from every cockpit row; AR history is facility master-data. The All-invoices tab already has the full table, filters, and per-invoice actions — it just can't be scoped to one facility yet. This adds zero new navigation concepts.

How it looks (shelf section sketch):

INVOICES                          Outstanding: $12,400 · 3 open
INV-1041  Jun 23–29   $4,200   ● Overdue (12d)
INV-1044  Jun 30–Jul 6 $4,100  ● Sent · due Jul 21
INV-1038  Jun 16–22   $4,100   ● Paid
INV-1035  Jun 9–15    $4,100   ● Paid
                     All invoices for this facility →

Tests: derived-status pill mapping, facility filter param.

PR-I — refactor/single-generate-path (model: sonnet) — D7 resolved: delete​

New requirement 2026-07-06: review the Create-Invoice ("Generate an invoice") design to minimize confusion and reduce friction.

Findings on the current form (src/components/admin/InvoicePreviewAndGenerate.tsx, "All invoices" tab):

  • It fully duplicates the cockpit path (facility + week → preview → generate) with no unique capability: the cockpit reaches any week via the PeriodBar/?week=, the engine applies the same rules, and the expectedTotalCents lock is matched in safety by the cockpit's immediate-generate-from-live-data.
  • It is actively stale: user-facing "Week (Sun–Sat)" label (:123) and "this Sun–Sat week" copy (:222) on a Monday-anchored week; success message says "Refresh to see it listed below, then download and send it" (:302-304) — manual refresh + send guidance pointing away from Review & send.
  • Friction: 6+ interactions for one facility vs. the cockpit's batch flow; two parallel mental models for the same act ("which one do I use?").

Resolved (D7): delete it. Remove the "Generate an invoice" section from the All-invoices tab; replace with one line: "Create invoices from the Weekly run tab →" (link that switches tab, preserving ?week=). Delete InvoicePreviewAndGenerate.tsx and prune its now-unused server actions (previewInvoice/generateInvoice in src/server/actions/invoices.ts) IF nothing else imports them — grep first; keep them if shared. One generation path = one mental model, and the deletion removes the stale "Sun–Sat" copy for free. The per-line-reason rigor it had is preserved by the cockpit's confirm-with-audit-reason flow (and PR-F's gap work).

Tests: tab link preserves week param; no dangling imports after deletion.

Runbook — docs/runbook-weekly-billing.md (model: opus drafts, Oliver edits)​

The Part C single-person mitigation. Contents: the weekly rhythm (Mon: Hubstaff sync → approve → generate → Review & send; daily: dashboard AR glance; on check: upload → reconcile), the escalation ladder once PR-D ships, prod access notes (SSM/env/cron verification commands), and "if Oliver is unavailable" minimum steps. Docs-only PR, no review gate beyond Oliver reading it.

Decision log (resolved 2026-07-06, except D7)​

  • D1 (PR-B): ✅ Owner-click re-request (no auto-email to facilities).
  • D2 (PR-D): ✅ Default cadence due+3 / due+10 / due+21 accepted.
  • D3 (PR-E): ✅ Minimal scope — never drop hours; latest terms price the week.
  • D4 (PR-F): ✅ Allowed — generate with the gapped line at $0 + recorded reason.
  • D5 (PR-D): ✅ AMENDED — interest mentioned in reminders; overdue stage is a payment-request email with the interest warning ONLY when contractual (interest_enabled); email includes a separate "coming due" section (unpaid, not yet due: amount + date). No formal interest billing for now.
  • D6 (PR-G): ✅ Confirmed — due date stamps at send; PDF re-rendered if it moved.
  • D7 (PR-I): ✅ Delete the duplicate "Generate an invoice" form; replace with a "Create invoices from the Weekly run tab →" link (preserving ?week=).

Model assignment summary​

WorkModelWhy
Copy/labels/badges (PR-A)sonnetMechanical UI, no money path
Overage aging + KPI (PR-B)opusServer actions + SQL aggregate; judgment needed
Send-lock + drift (PR-C)FableRewrites the send RPC — the money-atomic core
Reminders cron (PR-D)opusNew subsystem on existing rails
Assignment picker (PR-E)FableREDO of core billing math across 4 sites + migration
Gap diagnostics (PR-F)opusRun-RPC changes + UI, moderate
Correction guards (PR-G)sonnetSmall diffs; Fable reviews the RPC param change
Facility invoice history (PR-H)sonnetDisplay + one query; Fable review (touches weeklyRun service)
Single generate path (PR-I)sonnetDeletion + link (pending D7)
RunbookopusWriting, not code

Standing rule: every PR touching src/db/migrations/, src/lib/invoice/, or src/server/services/invoice/ gets a Fable review pass before merge, regardless of who wrote it.

Sequencing & effort ballpark​

Step 0 (today, ~15 min ops) → PR-A → PR-I (small, pairs with A once D7 lands) → PR-C → PR-B → PR-D → PR-H (natural follow-on to D: the AR view for the AR chase) → PR-E → PR-F → PR-G → Runbook. Each PR is independently shippable; C/B/D are the money-recovery core. Rough shape: A/G/H/I small, B/D/F medium, C/E large-careful.

Step 0 results (fill in)​

  • cron.job verified: …
  • RESEND env verified: …
  • review + plan committed: …