feat/ledger-mrr
needs attentioneaa8b95 · incrementalpre-PRreviewed 2026-08-06 17:53 UTC0H · 1M · 2L- Purpose
- Fill the MRR measurement gap for accounts that are invisible to the Stripe-driven path: off-Stripe enterprise deals (no Stripe customer) and overage-heavy orgs that show only their base subscription amount.
- Goal
- Emit a separate revenue.usage_mrr_snapshot PostHog event per active ledger org, derived from the credit ledger (overage credits × overage rate), disjoint from the Stripe base so base + variable = total MRR with no double-count.
- Sub-goals
- SG-1: Pure computeVariableMrrMxn function in packages/billing/src/mrr.ts (no I/O, unit-tested)
- SG-2: Section 9 in mrr-sync.handler.ts reads active ledger rows and emits per-org usage MRR events
- SG-3: PostHog insight in infra/posthog/insights.tf to visualize the new metric
- SG-4: analytics event type in packages/analytics/src/events.ts
- What
- Code-review address: import path moved from barrel @batu/billing to subpath @batu/billing/mrr (avoids Stripe re-export, enables real-function testing); per-row inner try/catch added so one org's PostHog failure doesn't abort the rest; segment null coalesced to undefined; verbose section 9 comment condensed pointing to source module doc; test mock routing hardened from innerJoin flag to fromTable identity (vi.hoisted); removed mock for pure function to use real implementation.
- Why
- The Stripe-only MRR path systematically underreported revenue for usage-priced accounts. The credit ledger already tracks the ground truth; deriving variable MRR from it fills the gap without any new DB writes.
- Areas
- packages/billing+142−0packages/database/src/schema+2−2services/billing+186−15infra/posthog+45−0packages/analytics/src+24−0
- Blast
- 403 lines added, 17 removed across 9 files; billing-service Lambda + PostHog analytics only — no DB writes, no API surface changes, no user-facing impact.
Findings · 3
tests1
Per-org inner trackServer failure path not tested
services/billing/src/__tests__/mrr-sync.handler.test.ts
The handler's inner try/catch (section 9, lines ~394–417) isolates per-org PostHog failures so one org's failure doesn't skip the rest. The new outer-catch test covers the ledger query itself throwing, but no test covers: ledger resolves with multiple rows AND trackServer rejects for one — verifying that the remaining orgs still get processed and usageSnapshots reflects only successes. A regression that re-throws inside the inner catch or double-counts errors would go undetected.
improvement2
segment ?? undefined is a no-op; comment contradicts schema
services/billing/src/handlers/mrr-sync.handler.ts
organizations.segment is .notNull().default('providers') in the Drizzle schema, so led.segment is typed string, never null. The ?? undefined guard has no runtime effect but the inline comment says 'segment is nullable', contradicting the schema and misleading future readers. Sections 1–8 pass org.segment directly without a guard. Remove the guard and comment (trust the schema), or reconcile by investigating whether any DB rows have null segment and add a migration.
Per-org PostHog failures not surfaced in SyncResult.errors
services/billing/src/handlers/mrr-sync.handler.ts
Inner-catch PostHog failures are only console.error'd; they don't push to result.errors or increment any counter. Sections 1–8 per-org failures DO push to result.errors. The final log's '${result.errors.length} errors' undercounts — a run with 0 usage snapshots due to PostHog being down looks identical to a run with no active ledgers. Consider adding a postHogFailures counter to SyncResult or pushing tagged entries to result.errors (matching sections 1–8's pattern).