fix(payments): report payout overpayments instead of clamping them away (#516)

[DPS0340]

Closes #516.

Two halves to this issue. §1 is real and fixed here; §2 does not reproduce, and I would rather show the evidence than quietly "fix" working code.

§1 — overpayment is invisible and unrecoverable

netOwed = Math.max(grossOwed - alreadyPaid, 0) (lib/payments/payouts-owed.ts:239) clamps the negative away, so once payouts exceed what is owed the balance reads 0 and the size of the overpayment is nowhere on screen. disabled={netOwed <= 0} (mark-payout-paid-dialog.tsx:69) then removes the only control that writes a payouts row.

On the suggested fix. The issue asks for "a negative/adjusting payout row or carry the overpayment forward". The first option is not available: payouts.amount carries CHECK (amount > 0) from 20260216212440_create_revenue_infrastructure.sql:66, and 20260724120000_manual_payouts.sql relaxed period_start/period_end but left that check alone; markPayoutPaid also rejects non-positive amounts at payouts.ts:88. Writing one means a migration plus a sign convention threading through alreadyPaid, clawback, and the school-facing payout history — larger than this issue, and it would silently reinterpret every existing reader of that column.

The second option already works. alreadyPaid is an all-time sum, so the next cycle's grossOwed - alreadyPaid starts in the hole and absorbs the excess with no operator action. The recovery was never missing — the visibility was. A balance sitting at 0 while quietly working off a debt is indistinguishable from a balance that is simply settled.

So: add overpaid (max(alreadyPaid - grossOwed, 0)) beside netOwed, and let the existing arithmetic do the recovery.

• netOwed keeps its floor. It feeds the metric cards, the mismatch guard in markPayoutPaid, and the school-facing view; letting it go negative would ripple into all three. overpaid is purely additive — at most one of the two is ever non-zero.
• Table gains an "Overpaid" column, plus a banner in the same idiom as the existing clawback banner, stating that the excess comes off the next payout automatically.
• The Mark-as-Paid button stays disabled at netOwed <= 0. With nothing owed, enabling it would only let the operator overpay further; the banner points at the carry-forward instead.

§2 — mismatch guard skipped on retry: does not reproduce

mismatch is cleared on every amount change (mark-payout-paid-dialog.tsx:88-91):

onChange={(e) => {
  setAmount(e.target.value)
  setMismatch(null)
}}

git log -S 'setMismatch(null)' -- components/platform/mark-payout-paid-dialog.tsx returns exactly one commit — 3a36763, the one that introduced the dialog in #507. It was never absent and nothing removed it. An edited amount therefore re-submits with confirmMismatch: false and the 10% check re-runs on the new value.

My read is that the report came from the handleConfirm line alone, where confirmMismatch = mismatch !== null does look bypass-prone without the onChange beside it. No code change; I documented the contract at both ends so the next reader sees why the two lines have to stay in step.

I wanted to pin this with a rendering test, but vitest.config.ts runs environment: 'node' with include: ['tests/unit/**/*.test.ts'] and the repo has no @testing-library/react — a component test would mean introducing a DOM environment and a testing-library dependency, which felt out of proportion for a regression guard on a two-line invariant. Say the word if you would like that infrastructure added and I will do it in a separate PR.

Verification

• npm run test:unit — 322 passed (27 files). payouts-owed.test.ts goes 26 → 30: the overpayment reported while netOwed stays floored, zero in the settled and underpaid cases, per-currency isolation (a USD overpayment must not touch the EUR balance), and the carry-forward asserted across three successive states as sales catch up.
• npm run typecheck — clean. npx eslint on all four changed files — clean.
• Repo CI (.github/workflows/ci.yml) green on my fork against this commit: https://github.com/DPS0340/lms-front/actions/runs/30148455504

No migration, no change to netOwed semantics, no new dependency.

Same caveat as #523: no .env.local or Supabase CLI here, so no before/after screenshots. The change is one field in a pure function plus a column and a banner that render from it. Happy to add captures if you want them before merging.

[guillermoscript]

Reviewed by fetching the branch and running it locally, plus re-deriving the claims in the description rather than taking them on trust.

Verification

• npm run test:unit → 322 passed / 27 files. npx tsc --noEmit → clean.
• git diff master... is exactly the 4 files described — no dependency, migration, config or CI changes.
• payouts.amount NUMERIC(10,2) NOT NULL CHECK (amount > 0) confirmed at 20260216212440_create_revenue_infrastructure.sql:66, and 20260724120000_manual_payouts.sql only relaxes period_start/period_end + rewrites payouts_period_order. The constraint claim holds.
• markPayoutPaid does reject non-positive amounts (app/actions/platform/payouts.ts).
• §2 confirmed as not reproducing. git log -S 'setMismatch(null)' -- components/platform/mark-payout-paid-dialog.tsx returns only 3a36763, the commit that introduced the dialog in #507. The onChange has cleared mismatch since day one, so an edited amount re-submits with confirmMismatch: false. The issue text was wrong, not the code — thanks for showing the evidence instead of "fixing" it.

The arithmetic is right, netOwed keeps its floor so no existing consumer moves, overpaid is purely additive, and the tests are the right ones (per-currency isolation and the three-state carry-forward assertion are exactly what would regress silently). Non-blocking overall; the items below are copy/UX and one scope question.

---

1. The banner copy overstates the recovery — please change

It recovers itself — the excess is deducted from their next payout automatically … Nothing to collect, and nothing to record here.

That is only true while the school keeps selling. A school that churns, goes dormant, or is deleted never generates the sales that absorb the excess, and the money is then genuinely unrecovered — while the page instructs the operator to do nothing about it. That is a worse failure than the silent 0 in #516: a confident wrong instruction beats an ambiguous one for causing lost money.

Related: in practice the most common cause of a below-zero balance is a mis-keyed payout row (extra zero, wrong currency — note a payout recorded in a currency with no transactions produces a pure overpaid row via the payouts-only currency union). Those need operator action too.

Suggested rewording:

Overpaid: {total}. These schools have been paid more than they were owed. The excess is deducted from their next payout automatically, so their balance stays at 0 until new sales catch up. If a school has no further sales, it has to be recovered outside the platform.

2. Overpaid and Of which refunded double-report the same money

1000 successful + a covered refunded 100 at 80%, with 880 already paid → clawback 80 and overpaid 80. Same 80, two adjacent columns, and both banners stack, each ending with "nothing to do" — it reads as 160 of anomaly. Worth either naming the overlap in the overpaid banner ("…of which {x} is the refunded portion shown as clawback") or merging the two banners when both are non-zero.

3. Purple is the brand primary hue

Per CLAUDE.md's design context the default primary is ~293 OKLCH (purple), and this page already codes amber = owed, red = refund, emerald = paid. purple-600 for an anomaly state reads as "branded", not "needs attention" — amber or slate would sit better in the existing scale. Also the state is conveyed by colour plus a title tooltip only; title is not keyboard-reachable and is unreliably announced by screen readers. The clawback cell has the same problem, so this is consistency rather than a regression, but an icon or a short visible label would fix both.

4. Scope question — for me, not for you

#516 §1 asked for "a negative/adjusting payout row or carry the overpayment forward". Your reasoning for not writing the negative row is correct and I accept it: the CHECK blocks it, and a sign convention would silently reinterpret every existing reader of payouts.amount. But leaving disabled={netOwed <= 0} untouched keeps the other half of what the issue complained about — there is still no way to record a real payment to a school sitting at 0 (advance, goodwill, out-of-band settlement).

Cheaper middle path, if I decide I want it: let the dialog open at netOwed === 0 and let the existing guard do the work — with netOwed === 0, Math.abs(amount - 0) > 0 * 0.1 is true for any positive amount, so the operator always gets the confirm step. That is exactly the behaviour you'd want for an off-cycle payment. Not asking for it in this PR — I'll open a follow-up.

5. i18n

The new strings are hardcoded English. That is consistent with the file: none of the 22 pages under app/[locale]/platform/ use next-intl, unlike the school-facing /dashboard/admin/payouts, which does. So not a defect here, and I don't want you to translate one page in an untranslated tree. Flagging it so the decision stays explicit — platform-side i18n is its own sweep.

Nits

• The 6-line comment inside the table-cell JSX restates module-level rationale that payouts-owed.ts already carries at length. One line at the render site would read better.
• hasOverpayments runs some(amount > 0) over a map that only ever receives values > 0 — harmless, and it mirrors the clawback code, so fine either way.
• Agreed on skipping the render test: introducing jsdom + testing-library for a two-line invariant is disproportionate. If we ever want it covered, Playwright against a seeded overpaid tenant is the cheaper route.

Fix §1 (and ideally §2) and this is good to merge.

[guillermoscript]

Addendum — ran this past our React/composition guidelines (.claude/skills/vercel-react-best-practices, .claude/skills/vercel-composition-patterns) and revisited the i18n point from my earlier review. Two asks, plus a list of rules I checked that you can safely ignore.

1. i18n — I do want this translated (revising what I said above)

I said "don't translate one page in an untranslated tree". Changing that call: the strings this PR adds are user-facing and I don't want more untranslated surface added, so let's start the namespace here.

The pattern already exists on the school-facing side — app/[locale]/dashboard/admin/payouts/page.tsx uses getTranslations('dashboard.admin.payouts') with 23 keys in messages/en.json. /platform just never got it (messages/en.json has no platform.* namespace at all today, only platformPricing).

Please add a platform.payouts namespace to messages/en.json and messages/es.json and move this page onto getTranslations — the whole file, not only your new strings. It's self-contained (~20 strings) and leaving half the page hardcoded is worse than either end state. The banner needs an interpolated total (t('overpaidBanner', { total })), and Intl.NumberFormat('en-US', …) in money() should take the request locale rather than being pinned to en-US.

On the Spanish: write it literally and I'll adjust the wording — I'm a native speaker and payout terminology here is opinionated. Don't block on getting the es copy perfect.

If you'd rather keep this PR to the fix, say so and I'll take the i18n conversion myself in a follow-up — but then the new strings need to land already keyed, not hardcoded.

2. Composition — the Overpaid cell is a clone of the clawback cell

The new <td> is a near-verbatim copy of the "Of which refunded" cell right above it: colored money span + title tooltip + em-dash fallback, ~17 lines each. That's two copies, and the next reporting-only column makes three.

Per patterns-explicit-variants, the shape I'd like is a shared presentational cell composed by two explicit variants, rather than one cell with a tone/variant string:

function ReportedAmountCell({ amount, currency, className, hint, ...rest }) {
  return (
    <td className="py-2.5 text-right tabular-nums" {...rest}>
      {amount > 0
        ? <span className={`font-medium ${className}`} title={hint}>{money(amount, currency)}</span>
        : <span className="text-muted-foreground">—</span>}
    </td>
  )
}

function ClawbackCell(props) { return <ReportedAmountCell className="text-red-600 dark:text-red-400" hint={t('clawbackHint')} {...props} /> }
function OverpaidCell(props) { return <ReportedAmountCell className="text-purple-600 dark:text-purple-400" hint={t('overpaidHint')} {...props} /> }

Pairs well with the color point from my first review, since the tone then lives in one place. Also folds nicely into the i18n work above — the tooltip strings become keys instead of inline literals.

3. Forward-looking, for the follow-up issue — not this PR

When we enable recording at netOwed === 0, please don't reach for allowZeroBalance / isAdvance booleans on MarkPayoutPaidDialog (architecture-avoid-boolean-props). An explicit second entry point sharing the form internals is what I'd merge.

Rules I checked that do NOT apply — don't go chasing these

• server-serialization — the new overpaid stays server-side: it feeds the table and the banner and is deliberately not added to MarkPayoutPaidDialog's props, so the RSC→client payload is unchanged. Correct as written.
• rendering-conditional-render (prefer ternary over &&) — doesn't bite here: hasOverpayments comes from .some(), so it's a real boolean with no 0/NaN to leak into the DOM.
• rerender-* / rendering-hoist-jsx — the page is an async RSC rendered once per request, and the dialog gains no state (comments only). Nothing to memoize or hoist.
• async-parallel — still the single getPayoutsOwed() await, untouched.
• js-combine-iterations — the new totalOverpaidByCurrency accumulates inside the existing single pass rather than adding another loop. Good.
• Trivial, take it or leave it: hasOverpayments could be Object.keys(totalOverpaidByCurrency).length > 0, since entries are only inserted when > 0 — same for the existing hasClawbacks. Irrelevant to performance in an RSC, purely a readability call.

[guillermoscript]

Rather than send you a list, I pushed the review follow-up onto this branch myself (56814ab) — you enabled maintainer edits, so it lands here instead of in a second PR. Your commit is untouched underneath it; the arithmetic, the tests and the §2 analysis are all yours and unchanged. Pull before you continue.

What I changed

1. Banner copy. "It recovers itself … Nothing to collect, and nothing to record here" is only true while the school keeps selling. A dormant or departed school never generates the sales that absorb the excess, and neither does a mis-keyed payout row — the page was telling the operator to do nothing about money that is genuinely gone. It now ends with "If a school has no further sales, it has to be recovered outside the platform", and the same caveat is in the payouts-owed.ts module doc so the next reader of the pure function sees it too.

2. Clawback / overpaid overlap. With 1000 successful, a covered refunded 100 at 80%, and 880 paid, clawback and overpaid both read 80 — the same money, two adjacent columns, two banners each saying "nothing to do". The overpaid banner now names the overlap, but only when a clawback is also present.

3. i18n. New platform.payouts namespace in messages/en.json and messages/es.json, covering the whole page and the Mark-as-Paid dialog rather than only the new strings. money() now formats in the request locale instead of a hardcoded en-US. tests/unit/platform-messages-parity.test.ts asserts en/es key parity, no empty strings, and matching ICU placeholders for that namespace — a missing Spanish key falls back to English silently, so nothing else would have caught it. Scoped to platform on purpose: the rest of the catalogue has pre-existing drift.

4. Composition + colour. ReportedAmountCell with ClawbackCell / OverpaidCell as explicit variants, so the two cells stop being copies of each other. Each carries an icon, so the columns aren't separated by colour alone (WCAG 1.4.1). Overpaid moved off purple — that's the brand primary hue here — onto sky.

Verified

Local Supabase, seeded so one tenant is overpaid with a covered refund and another is plainly owed:

Default School	collected 100, owed 80, paid 200 → owed $0.00, overpaid $120.00, refunded $40.00
Code Academy Pro	collected 300 → owed $240.00, overpaid —

npm run test:unit 325/325 (28 files, your 30 included), tsc --noEmit clean, eslint clean on all changed files. Both banners, both cells, the ICU plural ("1 school awaiting payout") and the mismatch guard render in both locales; Mark-as-Paid is disabled on the overpaid row and enabled on the owed one.

English

Spanish

Mismatch guard, Spanish — entering 999 against a 240 balance still trips the 10% check after an edit, which is the §2 behaviour you documented:

Still open

• The Spanish copy is mine and needs my own second pass before merge — I'm a native speaker, and I'd rather re-read it cold than ship it the same day I wrote it. Nothing for you to do there.
• The dialog still echoes amounts through toFixed (240.00 USD) while the table formats per locale (240,00 US$). Deliberate for now — those numbers mirror what's in the input — but if it bothers you in review, say so.
• The Close label on the dialog's built-in close button comes from components/ui/dialog and is untranslated everywhere in the app. Out of scope here.
• disabled={netOwed <= 0} still stands, per my earlier comment. Follow-up issue, not this PR.

Thanks for the analysis in the description — the §2 evidence in particular saved a round trip, and the carry-forward tests are the ones that would have caught a regression here.