Skip to main content

Fable Review: Invoicing System — Findings Report

Date: 2026-07-06 · Reviewer: Claude (Fable 5) adversarial review pass · Scope: billing-basis/overrides, coverage gaps, end-to-end process, AR/reminders, UX. Analysis only — no code changed.

Scope correction (read first)​

The brief's named artifacts do not exist: there is no INVOICING-HANDOFF.md, and none of the four mockup HTML files (invoicing-tab-weekly.html, generate-from-time-flow.html, review-and-send.html, hours-override.html) exist in the working tree or anywhere in git history. What exists is the shipped implementation, so that was reviewed as the source of truth: src/server/actions/invoicing.ts, the services in src/server/services/invoice/, the SQL RPCs (migrations 0045→0070→0074→0087→0088), and the live UI in src/components/admin/invoicing/. Two of the brief's premises are also false in the code:

  • There is no billing-basis or override panel. ADR-0036 explicitly rejected billing_basis/override_reason/per-diem/retainers. The only basis is hourly, billing min(logged, contracted) with an approved overage lifting the cap (src/lib/invoice/index.ts:82-86). The real "override" surfaces are: owner manual time entry (LogWeekHoursForm.tsx), overage approval/waiver (approval_requests), under-contract hours_reason, draft recompute, and correctInvoice.
  • "No aging view" is wrong. An AR aging widget and a cockpit AR KPI already exist (Part D).

Part A/B answers below are mapped onto the mechanisms that actually exist.

PART A — Billing-basis & override edge cases​

A1. REDO — Mid-week assignment change silently drops hours and re-prices the whole week. pickLatestPerContractor picks exactly one assignment per contractor for the week (src/server/services/invoice/generate.ts:125-141), then sums time entries filtered to only the chosen assignments (generate.ts:261-267). A mid-week rate or contracted-hours change made by inserting a new assignment row (the versioning model, effective_date/end_date) means: (a) the new terms apply retroactively to the entire Mon–Sun week — no proration exists anywhere on the billing side (proration lives only in the pay engine, src/lib/pay/index.ts); and (b) time entries recorded against the superseded assignment id vanish from actualHours. The SQL run (0088:83-84) and the shelf mirror the same pick, so preview, cockpit, and invoice all agree — consistently wrong, so nothing flags it. Severity: wrong invoice, silent (under-billing via dropped hours, or mis-priced week via retroactive rate). Status: EXISTS (mechanism), fix is NET-NEW. Patch won't hold at the picker; the correct shape is summing a contractor's entries across all their in-window assignments at the facility, priced per-assignment (a two-segment week bills as two segments).

A2. KEEP — FX is a non-issue on the invoice side. Invoices bill USD hourly_rate_cents frozen on the assignment; PHP exists only in payroll (Wise). There is no exchange-rate assumption in the invoice path to get wrong. Don't "add FX support" here.

A3. KEEP — Over-contract billing is structurally gated, not procedurally. You cannot bill above contracted without an approved approval_requests row: the cap is applied inside the engine at generation (lib/invoice/index.ts:82-86), re-applied by the send-time staleness recompute (send.ts:420-497), the create_facility_invoice RPC is service-role-only behind a preview recompute, concurrent duplicates are blocked by a GiST exclusion constraint (0087:24-38), and concurrent sends serialize on FOR UPDATE (0045:262-269). Two open shelves racing produce an error, not a double bill. This is the best-engineered part of the system — don't loosen it.

A4. IMPROVE — correctInvoice is an engine bypass with too wide a blast radius. It rewrites number, dates, and total on any non-void invoice — including sent and paid ones (invoiceManage.ts:258-273, the only guard is .neq('status','void')), re-spreading lines proportionally and de-itemizing them. It exists for backfilled/historical invoices, but nothing restricts it to origin='backfill', and it performs no approval-request or engine check. One line closes most of it: refuse origin='app' invoices that aren't drafts (point those at Correct/reissue). Severity: human-catchable today (owner-only, deliberate act), but it's the one path where a total can diverge from the engine with no recompute tie-back.

A5. IMPROVE — Acknowledged-unapproved hours are billed but never locked. send_invoices locks only approved entries (0045:319-327). When the owner acknowledges the soft gate and sends a bill containing submitted hours, those hours were billed but remain editable — the guard_locked_time_entry trigger protects only locked rows, so the sent invoice's reconciliation basis can drift afterwards. The drift only surfaces if someone opens that facility's shelf later (InvoiceShelf.tsx:259-267). Severity: silent divergence between billed and recorded hours. Minimal fix: on an acknowledged send, lock all PAYABLE entries in the window (or auto-approve-then-lock), not just approved ones.

A6. IMPROVE — Batch-wide acknowledge in Review & send. One checked unapproved draft sets acknowledgeUnapproved: true for the whole batch (ReviewAndSendDialog.tsx:105-112), and the RPC applies it to every invoice. An invoice whose hours became unapproved after the dialog snapshot rides through without its own acknowledgment. The staleness guard catches total changes but approval-status changes don't move totals. Fix: pass per-invoice ack ids instead of one boolean. Severity: human-catchable, narrow window.

A7. IMPROVE — Two operators, same week: last-write-wins at the time-entry layer. Owner manual entries land directly approved under a unique (contractor, assignment, entry_date) key; there's no conflict surfacing between an HR correction and an operator correction beyond the invoice-layer guards (expectedTotalCents, staleness hold). Acceptable at one-operator scale — but note it in the runbook rather than pretending it's handled. Severity: cosmetic today (single operator), latent if a second admin arrives.

A8. IMPROVE — Due date is frozen at generation, not send. invoiceDueDate(todayUtc(), terms) runs at draft creation (generate.ts:491-492). A draft generated Monday and sent Friday gives the facility Net-30-minus-4. Severity: cosmetic/fairness, but it also skews overdue math. One-line fix: stamp due_date in send_invoices from now() + terms.

PART B — Coverage-gap edge cases​

B1. IMPROVE — Gap detection cannot distinguish data-entry error from a real no-show. is_gap is binary: zero payable hours on an active in-window assignment, no approved leave (0088:74-82). An unmapped Hubstaff member (silently skipped during sync), hours logged against the wrong facility, and a genuine no-show all render identically. Worse, one gapped assignment bool_ors the whole facility into gap (0088:100), which excludes it from billableCents (run.ts:155) and removes the Generate checkbox (InvoicingCockpit.tsx:33-39) — so one bad mapping can hold up an entire facility's otherwise-clean bill. Severity: blocks/delays billing silently at the facility grain (human sees "gap", not "why"). Minimal change: annotate the gap row with cause hints (entries exist at other facilities this week? Hubstaff sync skipped members?) and stop letting one assignment's gap block generating for the covered lines.

B2. KEEP (pre-send) / REDO (post-send) — Stale-gap handling. Pre-send is right: detection is recomputed live in SQL on every load — no cache, so a late-entered timesheet clears a gap immediately. But after send, derived_status = 'sent' short-circuits everything (0088:125-136): hours that arrive after the invoice went out never re-flag anywhere. The under-billing is visible only in the shelf's drift footnote for a row nobody has a reason to reopen. Late Friday-night hours entered Tuesday after a Monday send = money silently never billed. Severity: wrong invoice, silent, recurring. Fix shape: a "sent-week drift" check — same recompute the staleness guard already does, run against sent invoices for the trailing N weeks, surfaced as a cockpit banner ("INV-1042 now reconciles to $X more — reissue?"). The recompute machinery already exists; only the trigger/surface is missing (EXTEND).

B3. KEEP — Boundary shifts don't double-count. An overnight shift is one entry attributed wholly to its punch-in business day (facility-tz, timeEntries.ts:262); it can't be double-detected across weeks. Legacy Sun–Sat vs Mon–Sun overlap can't double-bill (GiST + overlap-aware dedup, 0087). Two residual notes, both minor: manual overnight pairs are impossible to enter (schema requires to > from same-day), and the 24h/day clamp truncates silently.

B4. IMPROVE — Expected non-coverage creates weekly re-review with no memory. The pause window (0088) covers on-hold facilities well, and approved leave silences legitimately absent contractors. The unhandled case is the standing low-utilization assignment (a floater who works one week in six): it re-raises gap every idle week, and the red row + danger alert train the operator to rubber-stamp. There is no "gapped N consecutive weeks" signal and no "expected zero this week" ack. Severity: alert-fatigue → real gaps get skimmed past (process risk, not direct money). Minimal: a per-assignment gap-streak count on the row, and/or an "end-date this assignment?" nudge after 3+ consecutive gap weeks.

B5. IMPROVE — Leave silences by any-overlap, not window coverage. Confirmed exactly as suspected: the leave test is week-overlap (0088:78-81) — a one-day approved leave suppresses the gap flag for a five-day zero week. Note the financial exposure is nil in this billing model (zero hours bills $0 either way; leave never changes billed amounts) — the cost is diagnostic: a 4-day no-show hides behind a 1-day leave. Severity: non-financial / operational. Fix: flag when leave days < weekdays-with-zero-hours instead of binary silencing.

PART C — End-to-end process (pipeline stage table)​

Context that frames everything: every money action is owner-only, there is one operator (the owner), one tenant, and no written weekly runbook — PRODUCT.md/README.md contain no operating rhythm, and the handoff document the brief assumed exists… does not exist. Also load-bearing: all six cron jobs are code + a manual post-deploy SQL template (docs/cron-setup.md); no migration schedules them, and audit/05-gaps.md flags them as built-but-not-wired. Whether anything runs on a schedule in prod is unverifiable from the repo — check select * from cron.job in prod.

StageCurrent stateSPOF?Recommended forcing function
Time capture (Hubstaff pull / CSV / self-entry / owner manual)Manual-only — owner clicks sync; unmapped members silently skippedY — ownerScheduled Hubstaff pull + a "skipped members" surfaced count (non-zero = red)
Approval (owner bulk-approve; overage magic-link)Manual, no forcing function — unapproved hours still bill (capped); unapproved overage is never billed, ever; approval links die after 14 days with no re-notify; /admin/overage list has no age columnY — owner + facility contactAge column + auto re-send of expiring approval links + weekly "unbilled approved-pending overage $" KPI
Generation (cockpit, per closed week)Manual-only — no cron, no nudge when a closed week has ungenerated ready rows; operator sick Monday = billing simply doesn't happen and nothing says soY — owner (only role that can generate)Either scheduled draft generation for ready rows, or minimally a dashboard nag: "week of X closed N days ago, M facilities not invoiced"
Review → sendManual with good guardrails — recipients, due date, anomaly flag, staleness hold, held/stale never silentY — ownerSound design; fix A6 (per-invoice ack). Rubber-stamp risk is mitigated because the dialog surfaces per-row facts, not just a total
Send → deliveryExists and sound — outbox, email_status, resend, failure badges; but if RESEND_API_KEY unset, sends are recorded with email skipped (banner warns)N (system)Verify the env is actually set in prod; the audit flags the seam as no-op by default
Payment intake (checks)Exists and sound — OCR packet import, dedup, owner-confirmed atomic apply, flips paid/partialY — owner uploads/confirmsFine as-is at this scale
Overdue → chaseNo process — pure hope — detection is derived-on-view only; reminders NET-NEW; escalation NET-NEW; interest accrual code exists but its cron wiring is a manual step of unknown prod statusY — owner must lookSee Part D
Credit / write-offNo process — no credit memo for overpayment, no un-reconcile, no write-off state—NET-NEW; needed the first time a check bounces or overpays

Single-point-of-failure summary: the entire pipeline is one person at every non-automated stage, is_admin() is "has a profile row" rather than a real role model (audit D2), and there is no handoff document. The minimum viable mitigation is not software: write the weekly runbook (Monday: sync → approve → generate → review & send; daily: dashboard AR glance; on check arrival: upload → reconcile) and commit it to docs/.

PART D — AR reconciliation & overdue reminders​

PieceVerdictEvidence
Overdue detectionEXISTS (derived), EXTEND (stored)Nothing ever writes status='overdue' — by design (ADR-0022). Display, list filter, KPI, and aging all derive it from due_date at read time (src/lib/invoice/status.ts:24-39, invoice_ar_summary in 0089). Consistent and correct as long as every consumer remembers to derive — any future raw SQL filter on status silently misses overdue invoices.
ReconciliationEXISTSCheck upload → OCR → payer targeting → owner-confirmed atomic apply; partials handled oldest-first; invoices.status flips paid/partial in the RPC (0018_check_reconciliation.sql). Gaps inside it: overpayment leftover is displayed but not recorded (no credit ledger — NET-NEW, per ADR-0023), no reversal path for a mis-applied confirmed check (NET-NEW), and the unmatched check status is a dead enum value nothing writes.
Reminder cadenceNET-NEWfacilities.reminder_schedule, email_template_reminder, email_template_overdue have zero readers and zero writers — pure dead schema (0002:140-143; grep hits only db/types.ts). ADR-0024 explicitly deferred delivery. The outbox seam (ADR-0035) was built to carry these — wiring a /api/cron/invoice-reminders handler over existing rails is the EXTEND move; otherwise drop the columns.
Interest & escalationEXISTS (code) / EXTEND (wiring + collection)Real math (src/lib/interest/index.ts:44-48), idempotent nightly job (cron.ts:75-150), owner waiver, per-invoice manual accrue. Two holes: cron wiring is manual/unverified (above), and accrued interest is never billed — it sits in interest_accrued_cents, shown in the Manage panel, and no invoice, email, or statement ever carries it to the facility. Escalation beyond email: nothing (NET-NEW), and there is no reminder email in the first place.
VisibilityEXISTSAging widget Current/1–30/31–60/60+ (finance.tsx:96-150), cockpit "Outstanding AR" KPI with overdue cents (KpiRow.tsx:19-29), Needs-attention links to ?filter=overdue. The "no aging view" premise is false. Missing only the 61–90/90+ split (trivial) — and note the cockpit run table hides overdue: derived_status collapses sent/paid/partial/overdue into sent (0088:126).

The actual test — tracing the most-likely-overdue facility: invoice sent, Net-30 passes unpaid. What happens automatically: it renders as overdue in three places, and (if the cron is truly wired) interest accrues nightly onto a column no one bills. What happens only if a human acts: (1) noticing — the owner must open the dashboard or invoices tab; no notification of any kind fires; (2) reminding — no reminder ever sends; the owner writes a manual email outside any cadence; (3) escalating — no state beyond "still overdue"; three ignored reminders look identical to zero reminders; (4) collecting interest — the accrued figure must be manually communicated and manually reconciled; (5) resolution — payment has a real path (checks pipeline); non-payment has none: no write-off, no dispute state, the invoice sits overdue forever and the aging widget's 60+ bucket becomes a graveyard nobody is forced to look at. Every step from due-date-passed to cash depends on the owner remembering, with zero system nudges.

PART E — UX/UI pass (on the shipped screens)​

KEEP

  • The shared SlideOver shelf host and the single weekly_billing_run read model — every surface (grid, shelf, generator) computes from the same SQL, so numbers agree across screens by construction.
  • Review & send's per-row disclosure of actual recipients, due date, and the amount-anomaly baseline (ReviewAndSendDialog.tsx:330-356) — the correct anti-rubber-stamp design: it makes the irreversible step show its consequences inline. Same for held/stale results never being silently dropped.
  • The two-step create-drafts-then-send flow. Creating is reversible, sending is not; the split is doing real safety work. Common-path cost is low: select-all → Create → Review & send → confirm covers a clean week of 9 facilities in ~5 interactions.
  • PeriodBar's refusal to advance past the last closed week.

IMPROVE

  • The page header lies about the billing policy. "Invoices bill contracted hours; logged hours are reconciled…" (src/app/admin/invoices/page.tsx:522-528) has been false since migration 0070 switched to lesser-of. The shelf says the true rule two clicks later. An operator (or successor) trusting the header will mis-explain bills to facilities. Severity: actively misleads. One-sentence fix.
  • LogWeekHoursForm day-of-week labels are off by one. DOW = ['Sun','Mon',…] indexed over a Monday-anchored week (LogWeekHoursForm.tsx:19, :101-105): the first column says "Sun" over the Monday date. An operator keying by weekday writes hours to the next day's date. Weekly totals (and thus invoice amounts) survive, but per-day records are wrong — which matters exactly when disputes/boundaries matter. Severity: actively causes mistakes. Fix: derive the label from the date. (Same stale "Sun–Sat" ghosts in the PeriodBar and LogWeekHoursForm doc comments.)
  • A gap row can't reach its own fix. openShelf routes gap to the facility shelf (InvoicingCockpit.tsx:212-215), and the invoice-cell link renders — for gap rows (:338-357) — but the "Log hours" form that resolves a gap lives only in the invoice shelf. From a coverage-gap row there is no path to it; the operator detours through Time & Activity. The status points at a shelf that can't fix it. Severity: slows the exact case the red alert exists for.
  • The shelf's Generate dead-ends on under-contract confirmation. generateDraftInvoice from the shelf returns needsConfirm as a bare error string ("Add a reason for billing X 0h of 40h…") with no confirm affordance (InvoiceShelf.tsx:67-78); only the cockpit batch path has the confirmDanger dialog (InvoicingCockpit.tsx:188-201). Two entry points, one mental model, different capabilities.
  • sent hides the AR truth in the run table. The badge says "Sent" whether the invoice is paid, partial, or 45 days overdue — existingInvoiceStatus is already on the row; render "Sent · Paid" / "Sent · Overdue" and the weekly view stops implying that sent = done. (The UI face of Part D's visibility gap.)
  • Six statuses, four operator actions. Ready→generate; review/gap→investigate; paused/none→nothing; sent→monitor. Do not merge gap into review (different investigations), but paused/none could share a "Nothing to bill" tab, and the real miss is that no tab means "money at risk" — that lives on the dashboard instead. Severity: inconsistent-but-not-harmful.
  • Redundant judgment on unapproved hours — flagged in the grid (review), again in the shelf alert, again in the dialog (unchecked by default), with no memory between them. Three prompts is defensible for a money gate, but triage doesn't persist: un-checking a draft in the dialog leaves no trace of why for next week. Cosmetic-to-modest.

REDO — nothing in the UI warrants a structural redo. The shelf/dialog architecture is sound; the failures are copy, routing, and one label bug.

Ranked: what's most likely already costing money​

  1. Unapproved overage is silently never billed, and nothing ages it (Parts A/C — 0088:92-95, 14-day token TTL in overage.ts, no reminder, no aging view). The contractor gets paid for the hours; the facility is never charged; the cap re-applies forever once the link expires. Given real over-contract weeks already happen (migration 0084 exists because overage billing broke once before), this is the top standing leak. Fix: age/expiry visibility + auto re-request + an "unbilled overage $" KPI.
  2. Post-send hour changes never resurface (B2 + A5). Late-entered hours after a Monday send are permanently unbilled unless the owner happens to reopen that shelf, and acknowledged-unapproved billed hours stay editable after send. The staleness-recompute machinery already exists — pointing it at sent invoices for the trailing weeks and surfacing drift as a cockpit banner closes both.
  3. The overdue-to-cash chase has no forcing function, and its one automated piece may not even be running (Part D). Reminders are dead schema, escalation doesn't exist, interest is accrued-but-never-billed, and cron wiring is an unverified manual step. First actions are cheap: verify cron.job in prod, then wire one /api/cron/invoice-reminders handler over the existing outbox + the dead template columns.

Runner-up worth scheduling: A1 (mid-week assignment change drops hours) — lower frequency than the top three, but it's the one that produces a confidently wrong invoice with zero signals anywhere.