fix(payments): stop double-subtracting refunds from payout balances (#511) (#521)

`computeOwedBalances` excluded refunded sales from `grossOwed` and then
subtracted their scaled share again as `clawback`, so a school's balance
was understated by the refund. The second subtraction was never needed:
`alreadyPaid` is the all-time payout sum, so it still holds whatever was
paid out for a sale that later flipped to `refunded`. Dropping the sale
from `grossOwed` recovers that overpayment on its own.

`netOwed` is now `max(grossOwed - alreadyPaid, 0)` — one representation,
one subtraction. The issue's worked example (1000 successful + 100
refunded at 80%, 80 paid) goes from 640 to the correct 720.

`clawback` survives as a reporting-only figure and is gated on
plausibility: a refund counts only when a payout in the same currency
settled up to a point at or after the sale, so refunds that were never
paid out no longer reduce anything or appear as recoverable. The dates
come from `transactions.transaction_date` (that table has no
`created_at`) and from `payouts.period_end ?? paid_at ?? created_at` —
`period_end` is already the exact "covered through" the issue asked for,
so no migration is needed.

The four `#498` unit tests encoded the old behavior as intended and are
rewritten with asymmetric amounts, so the `Math.max(…, 0)` floor can no
longer mask a sign error. Adds coverage for undated refunds, undated
payouts, cross-currency payouts, latest-payout-wins, and mixed ISO
offset formats. 26 tests in the file, 313 across the suite.

The `/platform/payouts` clawback banner and column are reworded, since
that number is no longer a deduction the reader should apply themselves.


Claude-Session: https://claude.ai/code/session_012nmbfroRHjSNrkGbRqTozs

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: guillermoscript

app/[locale]/platform/payouts/page.tsx

@@ -140,8 +140,9 @@ export default async function PlatformPayoutsPage() {
           data-testid="payouts-clawback-banner"
         >
           <span className="font-medium">Refund clawback: {formatByCurrency(totalClawbackByCurrency)}.</span>{' '}
-          Some already-paid-out transactions were later refunded — this amount reduces what&apos;s
-          currently owed rather than being paid again.
+          That much of what you already paid out was for sales that have since been refunded. It is
+          already netted out of the balances below, so don&apos;t collect it again — nothing extra to
+          do here.
         </div>
       )}
 
@@ -163,7 +164,7 @@ export default async function PlatformPayoutsPage() {
                     <th className="pb-2 text-right font-medium">Collected</th>
                     <th className="pb-2 text-right font-medium">School %</th>
                     <th className="pb-2 text-right font-medium">Paid so far</th>
-                    <th className="pb-2 text-right font-medium">Clawback</th>
+                    <th className="pb-2 text-right font-medium">Of which refunded</th>
                     <th className="pb-2 text-right font-medium">Owed</th>
                     <th className="pb-2 text-right font-medium"></th>
                   </tr>
@@ -187,8 +188,14 @@ export default async function PlatformPayoutsPage() {
                       </td>
                       <td className="py-2.5 text-right tabular-nums">
                         {r.clawback > 0 ? (
-                          <span className="font-medium text-red-600 dark:text-red-400">
-                            −{money(r.clawback, r.currency)}
+                          // Not a deduction column: this is the slice of "Paid so far"
+                          // that covered sales later refunded. "Owed" already accounts
+                          // for it (#511), so it carries no minus sign.
+                          <span
+                            className="font-medium text-red-600 dark:text-red-400"
+                            title="Part of what was already paid out, for sales later refunded. Already reflected in Owed."
+                          >
+                            {money(r.clawback, r.currency)}
                           </span>
                         ) : (
                           <span className="text-muted-foreground">—</span>

app/actions/platform/payouts.ts

@@ -29,10 +29,14 @@ export async function getPayoutsOwed(): Promise<TenantOwed[]> {
     admin.from('revenue_splits').select('tenant_id, school_percentage'),
     admin
       .from('transactions')
-      .select('tenant_id, payment_provider, amount, currency, school_percentage_snapshot, status')
+      .select('tenant_id, payment_provider, amount, currency, school_percentage_snapshot, status, transaction_date')
       .in('status', ['successful', 'refunded'])
       .in('payment_provider', PLATFORM_SETTLED_PROVIDERS),
-    admin.from('payouts').select('tenant_id, amount, currency').eq('payout_method', 'manual').eq('status', 'paid'),
+    admin
+      .from('payouts')
+      .select('tenant_id, amount, currency, period_end, paid_at, created_at')
+      .eq('payout_method', 'manual')
+      .eq('status', 'paid'),
   ])
 
   const schoolPercentageByTenant = new Map(
@@ -54,8 +58,17 @@ export async function getPayoutsOwed(): Promise<TenantOwed[]> {
         currency: t.currency || 'usd',
         schoolPercentageSnapshot: t.school_percentage_snapshot as number | null,
         status: t.status as 'successful' | 'refunded',
+        transactionDate: t.transaction_date as string | null,
       })),
-    (paid || []).map((p) => ({ tenantId: p.tenant_id, amount: p.amount, currency: p.currency || 'usd' }))
+    // `period_end` is the exact "covered through" when a payout recorded a period;
+    // manually recorded ones leave it null today, so fall back to when the money
+    // actually moved (issue #511).
+    (paid || []).map((p) => ({
+      tenantId: p.tenant_id,
+      amount: p.amount,
+      currency: p.currency || 'usd',
+      coveredThrough: p.period_end ?? p.paid_at ?? p.created_at,
+    }))
   )
 }
 

lib/payments/payouts-owed.ts

@@ -24,13 +24,40 @@
  * `transactions.currency` and `payouts.currency` are trusted as given; this
  * module does no currency conversion, only grouping.
  *
- * Refunds/chargebacks (issue #498): a transaction can flip from `successful`
- * to `refunded` after its payout was already recorded. Callers pass refunded
- * transactions through alongside successful ones (tagged via `status`); this
- * module scales each refund by the same split percentage a sale would've
- * used and surfaces it as a distinct `clawback` figure, subtracted from
- * `netOwed` on the next cycle rather than silently vanishing from
- * `grossCollected`/`grossOwed` with no trace.
+ * Refunds/chargebacks (issues #498, #511): a transaction can flip from
+ * `successful` to `refunded` after its payout was already recorded. Callers
+ * pass refunded transactions through alongside successful ones (tagged via
+ * `status`), and refunded sales are simply left out of
+ * `grossCollected`/`grossOwed`.
+ *
+ * That exclusion is the ONLY place a refund moves `netOwed`, and it is
+ * sufficient on its own: `alreadyPaid` is the all-time sum of recorded
+ * payouts, so it still contains whatever was paid out for a sale that later
+ * flipped to `refunded`. Dropping the sale from `grossOwed` while leaving its
+ * payout inside `alreadyPaid` recovers the overpayment automatically —
+ * 1000 successful plus a refunded 100 at an 80% split with 80 already paid
+ * gives `800 - 80 = 720`, the correct figure. Subtracting the refund a second
+ * time as a `clawback` term (as this module did before #511) understated the
+ * balance and also penalised refunds that had never been paid out at all.
+ *
+ * `clawback` therefore survives as a REPORTING-ONLY figure and is not part of
+ * the `netOwed` arithmetic. It answers "how much of what we already paid this
+ * school was for sales that have since been refunded?", so an operator can
+ * see why a balance dropped. Because it is already netted out of `netOwed` via
+ * `alreadyPaid`, it must never be collected a second time.
+ *
+ * A refund only counts toward `clawback` when a payout plausibly covered it:
+ * its `transactionDate` must fall at or before the latest `coveredThrough` of
+ * that tenant's payouts IN THE SAME CURRENCY. This is a heuristic — the module
+ * has no record of which transactions a given payout settled — so it can
+ * overstate `clawback` when a payout deliberately skipped a sale. That
+ * inaccuracy cannot move `netOwed`; making it exact needs real payout-to-
+ * transaction linkage on `payouts`.
+ *
+ * `netOwed` keeps its `Math.max(…, 0)` floor, so a school overpaid beyond its
+ * outstanding balance reads 0 rather than a negative — only a payout row moves
+ * money, and this view never invents a reverse one. The unrecovered remainder
+ * in that situation is visible through `clawback`.
  */
 
 /** Used when a tenant has no `revenue_splits` row yet (shouldn't normally happen, but keeps callers from dividing by an absent value). */
@@ -52,15 +79,30 @@ export interface PlatformSettledTxn {
   currency: string
   /** revenue_splits.school_percentage in effect when this transaction was created (0–100), or null for pre-#496 rows. */
   schoolPercentageSnapshot: number | null
-  /** 'successful' contributes to grossCollected/grossOwed as normal; 'refunded' contributes its scaled amount to `clawback` instead (issue #498). */
+  /** 'successful' contributes to grossCollected/grossOwed as normal; 'refunded' is left out of both (issues #498, #511). */
   status: 'successful' | 'refunded'
+  /**
+   * `transactions.transaction_date` (ISO 8601) — that table has no `created_at`.
+   * Only read for refunded rows, to decide whether a payout plausibly covered
+   * this sale before it was refunded (issue #511). Null means "unknown", which
+   * is treated as not covered rather than assumed covered.
+   */
+  transactionDate: string | null
 }
 
 export interface ManualPayoutRecord {
   tenantId: string
   amount: number
   /** payouts.currency — the currency this payout was actually recorded in. */
   currency: string
+  /**
+   * The point in time this payout is understood to settle up to (ISO 8601) —
+   * `payouts.period_end` when the payout recorded one, else `paid_at`, else
+   * `created_at`. Refunded sales dated at or before it are counted as having
+   * been paid out already (issue #511). Null means "unknown", and covers
+   * nothing.
+   */
+  coveredThrough: string | null
 }
 
 export interface CurrencyBalance {
@@ -71,9 +113,15 @@ export interface CurrencyBalance {
   grossOwed: number
   /** Sum of manual payouts already recorded as paid in this currency. */
   alreadyPaid: number
-  /** Sum over refunded transactions of amount × (that transaction's snapshotted split, or the tenant's current split) — money already paid out for sales that later got refunded (issue #498). */
+  /**
+   * REPORTING ONLY — not a term in `netOwed` (issue #511). Sum, over refunded
+   * transactions a payout plausibly covered, of amount × (that transaction's
+   * snapshotted split, or the tenant's current split): how much of what was
+   * already paid to this school was for sales that have since been refunded.
+   * `alreadyPaid` already accounts for it, so it must not be recovered again.
+   */
   clawback: number
-  /** max(grossOwed - alreadyPaid - clawback, 0) — what's currently owed in this currency. */
+  /** max(grossOwed - alreadyPaid, 0) — what's currently owed in this currency. */
   netOwed: number
   /** Per-provider breakdown of grossCollected in this currency. */
   byProvider: Record<string, number>
@@ -111,29 +159,66 @@ export function computeOwedBalances(
     return entry
   }
 
+  const paidByTenantCurrency = new Map<string, Map<string, number>>()
+  // tenantId -> currency -> latest `coveredThrough` across that currency's payouts.
+  // Payouts are cumulative and all-time, so the latest one bounds everything any
+  // payout could have settled.
+  const coveredThroughByTenantCurrency = new Map<string, Map<string, number>>()
+  for (const payout of paidPayouts) {
+    let byCurrency = paidByTenantCurrency.get(payout.tenantId)
+    if (!byCurrency) {
+      byCurrency = new Map()
+      paidByTenantCurrency.set(payout.tenantId, byCurrency)
+    }
+    byCurrency.set(payout.currency, (byCurrency.get(payout.currency) ?? 0) + payout.amount)
+
+    if (payout.coveredThrough) {
+      // Parsed rather than string-compared: `paid_at` and `period_end` reach us
+      // as whatever offset format their source used ('…Z' vs '…+00:00'), which
+      // lexicographic comparison gets wrong across formats.
+      const covered = Date.parse(payout.coveredThrough)
+      if (Number.isNaN(covered)) continue
+      let coveredByCurrency = coveredThroughByTenantCurrency.get(payout.tenantId)
+      if (!coveredByCurrency) {
+        coveredByCurrency = new Map()
+        coveredThroughByTenantCurrency.set(payout.tenantId, coveredByCurrency)
+      }
+      const previous = coveredByCurrency.get(payout.currency)
+      if (previous == null || covered > previous) {
+        coveredByCurrency.set(payout.currency, covered)
+      }
+    }
+  }
+
+  /**
+   * True when a payout in the same tenant AND currency settled up to a point at
+   * or after this sale, so its share plausibly went out before the refund. A
+   * missing date on either side answers false — an unjustifiable clawback is
+   * worse than a missing one (issue #511).
+   */
+  function wasPlausiblyPaidOut(txn: PlatformSettledTxn) {
+    if (!txn.transactionDate) return false
+    const soldAt = Date.parse(txn.transactionDate)
+    if (Number.isNaN(soldAt)) return false
+    const coveredThrough = coveredThroughByTenantCurrency.get(txn.tenantId)?.get(txn.currency)
+    return coveredThrough != null && soldAt <= coveredThrough
+  }
+
   for (const txn of txns) {
     const entry = bucket(txn.tenantId, txn.currency)
     const effectivePercentage = txn.schoolPercentageSnapshot ?? schoolPercentageByTenant.get(txn.tenantId) ?? 0
     const scaledAmount = (txn.amount * effectivePercentage) / 100
     if (txn.status === 'refunded') {
-      entry.clawback += scaledAmount
+      // Reporting only. Leaving the sale out of grossOwed below is what actually
+      // corrects the balance; this just names the amount for the operator.
+      if (wasPlausiblyPaidOut(txn)) entry.clawback += scaledAmount
       continue
     }
     entry.grossCollected += txn.amount
     entry.grossOwed += scaledAmount
     entry.byProvider[txn.paymentProvider] = (entry.byProvider[txn.paymentProvider] ?? 0) + txn.amount
   }
 
-  const paidByTenantCurrency = new Map<string, Map<string, number>>()
-  for (const payout of paidPayouts) {
-    let byCurrency = paidByTenantCurrency.get(payout.tenantId)
-    if (!byCurrency) {
-      byCurrency = new Map()
-      paidByTenantCurrency.set(payout.tenantId, byCurrency)
-    }
-    byCurrency.set(payout.currency, (byCurrency.get(payout.currency) ?? 0) + payout.amount)
-  }
-
   return tenants.map((tenant) => {
     const collected = byTenantCurrency.get(tenant.tenantId) ?? new Map()
     const paid = paidByTenantCurrency.get(tenant.tenantId) ?? new Map()
@@ -148,7 +233,11 @@ export function computeOwedBalances(
       const grossOwed = entry?.grossOwed ?? 0
       const clawback = entry?.clawback ?? 0
       const alreadyPaid = paid.get(currency) ?? 0
-      const netOwed = Math.max(grossOwed - alreadyPaid - clawback, 0)
+      // `clawback` is deliberately absent here — refunded sales are already out
+      // of `grossOwed`, and their payouts are still inside `alreadyPaid`, so the
+      // overpayment nets out on its own. Subtracting it again double-counted the
+      // refund (issue #511).
+      const netOwed = Math.max(grossOwed - alreadyPaid, 0)
       return {
         currency,
         grossCollected,