fix(billing): surface subscription-past-due and cutoff-scheduled tenants in billing health (#514)

[DPS0340]

Closes #514.

Problem

getAtRiskTenants() fetched a single population — tenants.billing_status = 'past_due' — and computeBillingHealth decorated it. Every gap in the issue follows from that one .eq(): a tenant is either in the fetch set or it does not exist as far as this dashboard is concerned.

Two of the three groups #493 §1.2 asked for were therefore invisible:

1. Subscription-only past-due. A tenant whose platform_subscriptions.status went past due without tenants.billing_status being synced never appeared — the exact drift the epic wanted surfaced.
2. Cutoff-only. A tenant over its plan limits with an access_cutoff_at scheduled by Non-payment produces no access enforcement — over-limit tenants keep full access indefinitely #494 but healthy billing never appeared either. lib/billing/billing-health.ts already carried accessCutoffAt through and the page already rendered a column for it, but only past-due tenants were ever fetched — so that column could only ever hold a value for a tenant that was also past due.

Changes

Union fetch with reasons (app/actions/platform/billing-health.ts). Three reads merged by tenant id: tenants.billing_status = 'past_due', tenants owning a past_due subscription row, and tenants.access_cutoff_at IS NOT NULL. Not a single .or() — PostgREST cannot express a disjunction spanning two tables, and the subscriptions read is keyed off the resolved tenant ids either way.

Each condition contributes a reason, carried through the pure layer as reasons: AtRiskReason[] on both the input and output types. A tenant can qualify under several at once, and collapsing that to one enum would discard precisely the drift signal §1 is about.

Deterministic subscription ranking (lib/billing/billing-health.ts). subByTenant did a plain .set() over an unordered list, so a stale row could donate its payment_method and grace_period_end — and therefore the countdown. Now ranks past_due > active > other, tie-broken by updated_at desc.

Being precise about severity, because the issue slightly overstates this one: platform_subscriptions carries CONSTRAINT platform_subscriptions_tenant_unique UNIQUE (tenant_id) (20260217040000_platform_billing.sql:46), never dropped in any later migration, and every write path is an .upsert(..., { onConflict: 'tenant_id' }). A tenant cannot hold two rows today, so §3 is unreachable, not a live miscount. It is a trap that springs the moment that constraint is relaxed (per-plan history rows, say). Ranking explicitly costs nothing and makes the function correct under either schema.

UI (app/[locale]/platform/billing-health/page.tsx). A "Reason" column of badges. The three existing metric cards deliberately keep counting past-due tenants only, so the numbers at the top do not silently change meaning now that over-limit tenants are listed; a fourth card counts scheduled cutoffs. The empty state and page description no longer claim the list is only about payment.

Verification

• npm run test:unit — 323 passed (27 files). tests/unit/billing-health.test.ts goes 6 → 11 cases: subscription-only past-due surfacing, cutoff-only surfacing with no fabricated countdown, multi-reason ordering, stale-vs-current row ranking asserted under both input orders, and an updated_at tie-break. The original six are unchanged apart from the new required field.
• npm run typecheck — clean.
• npx eslint on all four changed files — clean.

No migration, no schema change, no new dependency.

Notes for the reviewer

I could not exercise this against a running instance — no .env.local and no Supabase CLI in my environment — so there are no before/after screenshots on this PR, unlike #520/#521. The behaviour change lives entirely in the union fetch plus the pure function, and the unit tests cover both; the UI diff is a badge column, a fourth metric card, and two reworded strings. Happy to add captures if you would like them before merging.

Out of scope, worth its own issue: reconciling tenants.billing_status against platform_subscriptions.status at write time. This PR makes the dashboard stop hiding the drift; removing the drift is a separate change.

[DPS0340]

CI verification: the repo's own .github/workflows/ci.yml ran green on my fork against this exact commit (4fe0d29) — install, lint (non-blocking), typecheck, and unit tests all passed: https://github.com/DPS0340/lms-front/actions/runs/30148221978

Noting it here because fork PRs need a maintainer to approve the workflow run before checks appear on this page, so there is nothing to see under Checks until you do.

[guillermoscript]

Thanks for this — the write-up is unusually careful, and I verified the claims in it rather than taking them on trust. All of them hold:

• npm run test:unit → 323 passed / 27 files, npx tsc --noEmit → clean, npx eslint on the four changed files → clean (run against 4fe0d29 in a clean worktree).
• The §3 downgrade you argued for is correct: platform_subscriptions_tenant_unique UNIQUE (tenant_id) is at 20260217040000_platform_billing.sql:46 and is never dropped later, and every writer upserts onConflict: 'tenant_id'. Ranking is defensive, not a live fix — good call flagging that instead of overclaiming.
• Diff is 4 files, one commit, no migration, no dependency, no package.json/lockfile change. verifySuperAdmin() is intact, createAdminClient() usage is unchanged in scope, and nothing new is exposed to the client. No security concerns from me.

The union-fetch shape (three reads merged by tenant id + per-row reasons) is the right answer to #514 §1/§2 — PostgREST genuinely can't express that disjunction, and keeping reasons as a set instead of one enum is the correct call.

One thing needs fixing before merge.

---

🔴 Blocking: cutoff-only tenants inherit the past-due-only columns, and sort to the top

computeBillingHealth derives pastDueSince, graceEndsAt, daysUntilDowngrade and isEstimate purely from payment_method, with no reference to whether the tenant is actually past due. That was safe while the fetch set was billing_status = 'past_due' only. It isn't safe now that reason 3 admits healthy tenants.

The concrete case is the most common cutoff-only population there is — a school that already lapsed to free and is now over the free plan's limits:

• downgradeTenantToFree() (lib/billing/downgrade-tenant.ts:34-41) sets status: 'canceled' and never clears grace_period_end — only confirmManualPayment and the non-liveCycle branch of changePlan do that.
• The same function then calls reconcileAccessCutoff() (:64), which is exactly what stamps the access_cutoff_at your new query selects on.

So that tenant arrives with payment_method: 'manual_transfer' + a months-old grace_period_end. Confirmed against the branch:

computeBillingHealth(
  [{ tenantId: 't', tenantName: 'Lapsed School', plan: 'free',
     accessCutoffAt: '2026-08-07T00:00:00.000Z', reasons: ['access_cutoff_scheduled'] }],
  [{ tenantId: 't', status: 'canceled', paymentMethod: 'manual_transfer',
     currentPeriodEnd: '2026-06-01T00:00:00.000Z', gracePeriodEnd: '2026-06-08T00:00:00.000Z',
     updatedAt: '2026-06-08T00:00:00.000Z' }],
  new Date('2026-07-24T00:00:00.000Z'),
)
// → pastDueSince: '2026-06-01…', graceEndsAt: '2026-06-08…',
//   daysUntilDowngrade: -46, isEstimate: false

Rendered, that row reads "Past due since Jun 1, 2026" and a red "Overdue" in Downgrades in — for a tenant that is not past due and already downgraded. And because the sort is ascending on daysUntilDowngrade with nulls last, -46 puts it above every genuinely urgent past-due school. The dashboard's top rows become its least actionable ones, which is a step backwards from the page it replaces.

The healthy-Stripe variant is milder but same root:

// cutoff-only tenant, status 'active', payment_method 'stripe', currentPeriodEnd in the future
// → pastDueSince: '2026-08-20…' (a *future* date under "Past due since")
//   isEstimate: true          (renders the "Stripe-managed" dunning badge)

Your own cutoff-only test passes only because it leaves currentPeriodEnd/gracePeriodEnd null — the fixture is healthier than the rows this query will actually return.

Suggested fix — gate the past-due-derived fields on a past-due reason, in the pure layer where it's testable:

const isPastDue =
  tenant.reasons.includes('tenant_past_due') || tenant.reasons.includes('subscription_past_due')
const isManualTransfer = isPastDue && paymentMethod === 'manual_transfer'
const graceEndsAt = isManualTransfer ? sub?.gracePeriodEnd ?? null : null
// …
pastDueSince: isPastDue ? sub?.currentPeriodEnd ?? null : null,
isEstimate: isPastDue && !isManualTransfer,

isEstimate in particular currently means "no fixed downgrade date", and for a cutoff-only row the honest answer is "no downgrade pending at all" — — rather than a dunning badge. Worth a test that a cutoff-only tenant carrying a stale grace_period_end still reports daysUntilDowngrade: null and does not sort above a live past-due row.

---

🟡 Worth addressing

The union-merge logic is the new code, and it's the only part with no test. Everything you added tests for lives in the pure layer that was already tested; the three-read merge, the reason accumulation, and the follow-up .in('id', missingTenantIds) fetch are untested. Consider extracting the merge into a pure mergeAtRiskTenants(pastDue, subPastDue, cutoff) alongside computeBillingHealth — same rationale as the existing pure/no-I/O split, and it makes the sub-only path assertable.

as TenantRow[] casts drop the generated types. access_cutoff_at, status and updated_at all exist in lib/database.types.ts, so the casts aren't compensating for a missing column — they're papering over the union Promise.all produces. A typo in a select list would now compile fine and fail at runtime. A shared const TENANT_SELECT = 'id, name, plan, access_cutoff_at' as const plus separate awaits (or typing each promise) keeps the checking.

access_cutoff_at IS NOT NULL includes cutoffs already in the past — access has already stopped, but the badge says "Cutoff scheduled". Either label past cutoffs distinctly ("Access paused") or note the intent; a super admin reading the list can't currently tell scheduled from enforced.

Unbounded reads. Neither the cutoff query nor the tenants query has a limit, so on a platform with more than PostgREST's max-rows (1000 by default) at-risk tenants the list truncates silently. Fine at current scale; a comment or an explicit cap would stop it becoming a mystery later. Related nit: platform_subscriptions.status has no index (only tenant_id and stripe_subscription_id) — irrelevant at one row per tenant, mentioning it only so it isn't a surprise.

Fourth round-trip. missingTenantIds triggers an extra query whenever a sub-only tenant exists. Small, but you could fetch tenants for pastDueSubscriptions' ids inside the same Promise.all by reordering the reads (subscriptions first, then one tenants read with .or(...) over the ids you now know) — optional, only if you're touching the file anyway.

---

ℹ️ On i18n

Flagging this because it will come up: the page is all hardcoded English (REASON_LABELS, card titles, the reworded description). That matches the existing convention — none of the 9 pages under app/[locale]/platform/ use next-intl, since this console is super-admin-only. So this PR isn't regressing anything and I'm not asking you to localize it here. If we do want the platform console in messages/*.json, that's a separate issue covering all 9 pages, not something to bolt onto this one.

---

🟢 Nits

• subscriptionRank scores unpaid / incomplete at 0, i.e. below active. Defensible, but unpaid is arguably closer to past_due than to a stale row — a one-line comment on why the three-way split is enough would help the next reader.
• updated_at has no DB trigger on platform_subscriptions; every writer sets it by hand. The tie-break therefore depends on app discipline. Unreachable today, so just noting it.
• "Past-Due Schools" now counts tenant_past_due ∪ subscription_past_due, so the number can legitimately grow versus master. That's the fix working as intended, but the PR description says the metric cards "keep counting past-due tenants only" — worth a word so nobody reads a jump as a regression.
• data-reasons={t.reasons.join(' ')} is a nice touch for future E2E; no E2E covers this page yet, so nothing to update.

---

Requesting changes on the first item only; everything else is take-it-or-leave-it. Re the missing screenshots — no problem given the environment constraint, but once the past-due gating is in, a capture of the table with a cutoff-only row would make the ordering behaviour obvious to whoever reviews next. Agreed on the billing_status ↔ platform_subscriptions.status reconciliation being its own issue.

[guillermoscript]

I pushed the review follow-ups straight onto this branch (bdc9640f) rather than opening a competing PR — "allow edits by maintainers" was on, and your commit 4fe0d29b is untouched underneath it, so the union fetch and the ranking stay yours. Everything below is on top of your work, not instead of it. If you'd rather own the changes, feel free to amend or rework the commit; I won't push again without saying so.

Verified against a running instance

You couldn't capture this, so here it is — five seeded fixtures on a local stack: a school that already lapsed to free and is over the free plan's limits, one whose cutoff has already passed, one with subscription-only drift, one two days from downgrade, and one in Stripe dunning.

master — only the two billing_status='past_due' schools exist as far as the dashboard is concerned. This is the gap the issue describes:

Your commit — all five appear, with reasons, which is exactly what §1 and §2 asked for. But look at the first row:

QA Lapsed School reports "Past due since Jun 10, 2026" and a red "Overdue", and sorts above QA Urgent School, which is the one actually downgrading in two days. It is not past due at all — it downgraded weeks ago. And QA Paused School, which has no subscription row whatsoever, is labelled "Stripe-managed".

Both come from the same place: computeBillingHealth derives pastDueSince, graceEndsAt, daysUntilDowngrade and isEstimate from payment_method alone, which was safe while the fetch set was past-due-only. Widening it to access_cutoff_at IS NOT NULL admits healthy tenants, and:

• downgradeTenantToFree() sets status: 'canceled' but never clears grace_period_end (lib/billing/downgrade-tenant.ts:34-41) — only confirmManualPayment and the non-liveCycle branch of changePlan do;
• the same function then calls reconcileAccessCutoff() (:64), which stamps the very access_cutoff_at your new read selects on.

So the already-downgraded school is not an exotic case — it is the most common cutoff-only row there is, and it arrives carrying a stale grace stamp. isEstimate on a no-subscription row was the milder version of the same thing.

After the follow-up — the live countdowns come back to the top, the cutoff-only rows report — instead of fabricated dates, and a cutoff already in force reads "Access paused" rather than "Cutoff scheduled":

The metric cards are identical across the last two shots, which is the property you designed for — adding over-limit schools doesn't move the past-due numbers.

What else is in the commit

• No-countdown tail is ordered. Those rows used to compare equal; they now sort by soonest accessCutoffAt, then name, so the tail isn't arbitrary and the render is stable across reloads.
• Four round-trips → two. The past-due subscription read now takes the full column set, so the rows for subscription-only tenants are already in hand — which makes the follow-up tenants fetch independent of the subscription fetch, and the two go out together. Feeding both subscription lists to computeBillingHealth is safe because of your ranking change: duplicates resolve to the incumbent instead of last-wins. The super-admin check deliberately stays ahead of the reads, with a comment saying why, so a later "parallelize everything" pass doesn't fold it in.
• mergeAtRiskTenants() extracted as a pure exported function. The union was the one genuinely new piece of logic with no test; it now has five, including a subscription id whose tenant row is missing and the cutoff read not clobbering a past-due row.
• Typed row mappers instead of as TenantRow[]. The columns all exist in lib/database.types.ts, so the cast was only papering over the Promise.all union — and it meant a typo in a select list would compile fine and fail at runtime.

Tests are 20 in this file (was 11) and 332 across the suite (was 323); tsc --noEmit and eslint on the four files are clean.

On the two things you flagged

Your §3 write-up was right and I checked it independently: platform_subscriptions_tenant_unique UNIQUE (tenant_id) is at 20260217040000_platform_billing.sql:46, never dropped, and every writer upserts on it — so §3 is unreachable today and the ranking is trap-removal, not a live fix. Calling that out instead of claiming a bug you hadn't confirmed is the right instinct, and the flattening above only works because you added it.

The billing_status ↔ platform_subscriptions.status reconciliation stays out of scope as you proposed; this dashboard now stops hiding the drift, and removing it is its own issue.

No i18n here on purpose: none of the nine pages under app/[locale]/platform/ use next-intl — the console is super-admin-only — so localizing it is a separate change covering all nine, not something to bolt onto this PR.

Re-reviewing now; thanks for a careful first contribution — the PR description in particular did most of my homework for me.

Addressed by bdc9640, pushed to this branch: the past-due gating, the ordered no-countdown tail, the flattened fetch, the pure mergeAtRiskTenants() with tests, and the typed row mappers. Verified against a running instance — captures in the comment above. Dismissing my own stale review so this can merge.

[guillermoscript]

Merged as 8715795. Thanks @DPS0340 — the union fetch and the per-row reasons were the right shape for this, and the honest framing of §3 (unreachable under the current UNIQUE constraint, so trap-removal rather than a live fix) is exactly what made the fetch flattening safe to layer on top. Good first contribution here.

Shipped state: past-due schools keep their countdowns and stay at the top, subscription-only drift and scheduled cutoffs are both visible with their reason, and a cutoff already in force reads "Access paused". Before/after captures are in docs/images/514-billing-health-* on master.

Still open from the discussion, unchanged: reconciling tenants.billing_status against platform_subscriptions.status at write time. The dashboard now surfaces that drift instead of hiding it; removing it is its own issue.