← all branches

feat/ledger-mrr

needs attentionviewing older commit
4aabeff · fullpre-PRreviewed 2026-08-06 15:06 UTC0H · 3M · 5L · 7I
The branch
Purpose
Stripe-only MRR reporting misses off-Stripe enterprise accounts and metered-heavy orgs that read as $0 base in the current pipeline
Goal
Emit ledger-derived variable (overage) MRR as a disjoint PostHog event so total MRR = base (mrr_snapshot) + variable (usage_mrr_snapshot), covering all orgs regardless of Stripe presence
Sub-goals
  • computeVariableMrrMxn: pure function, max(0, consumed - included) × rate / 100, fully unit-tested
  • mrr-sync section 9: read active credit ledgers, compute per-org variable MRR, emit revenue.usage_mrr_snapshot
  • New PostHog insight: mrr_variable_usage with same argMax-per-org-per-day dedup pattern as mrr_total
  • New analytics event type: RevenueUsageMrrSnapshotEvent added to AnalyticsEvent union
The changes (whole branch)
What
New pure billing function + export, section 9 in mrr-sync handler, new analytics event type, new PostHog Terraform insight. Analytics side-effect only — no schema migration, no API surface change, no DB writes.
Why
Enables accurate total MRR visibility including Tiendas Neto, Pilgrim's (off-Stripe enterprise) and Energía Real overage without touching the existing Stripe-based mrr_total insight.
Areas
services/billing+753packages/billing+1420packages/analytics+240infra/posthog+450
Blast
7 files, +360/−5 across billing service, billing package, analytics events, PostHog infra. No schema migration, no API change.
analytics-only no-db-write no-stripe-dependency
CI· No PR — no CI datacoderabbit· No .coderabbit.yamltypecheck· Not run locally

Findings · 9

security1

low

console.error logs raw Drizzle error object — may include query context in CloudWatch

services/billing/src/handlers/mrr-sync.handler.ts:417

Drizzle errors can include parameterized query context in the message. No credentials are in params here, but org IDs and plan slugs may appear. Log error.message over the full object for hygiene.

conventions1

medium

DB column overageRateMxn stores centavos but name implies pesos

packages/database/src/schema/plans.ts

Pre-existing: overage_rate_mxn holds centavos (e.g. 2500 = $25). The new interface field overageRateMxnCentavos disambiguates at the boundary, but the mapping could mislead a future reader. Add a unit comment: `// centavos (e.g. 2500 = $25.00 MXN)`.

tests4

medium

innerJoin-flag mock routing breaks if handler adds a second joined query before section 9

services/billing/src/__tests__/mrr-sync.handler.test.ts:29

The joined boolean in the db.select mock routes based on first innerJoin presence. If a future query between sections 1–8 also uses innerJoin, it silently routes to ledgerWhereMock producing wrong data. Discriminate by table name or argument instead.

medium

@batu/billing mock reimplements computeVariableMrrMxn inline — can silently diverge

services/billing/src/__tests__/mrr-sync.handler.test.ts:76

The mock duplicates the pure function's formula. If production gains rounding or cap logic, the handler test continues to pass while the integration is broken. Use vi.importActual or call the real export — it has no I/O and doesn't need mocking.

low

No test for section 9 catch path

services/billing/src/__tests__/mrr-sync.handler.test.ts

The catch block is the key safety property. Add: ledgerWhereMock.mockRejectedValue(new Error('DB down')) → handler returns valid result with usageSnapshots: 0.

low

Snapshot test omits asserting credits_included and segment event properties

services/billing/src/__tests__/mrr-sync.handler.test.ts:429

Minor coverage gap — the test asserts usage_mrr_mxn, plan_slug, credits_consumed but not credits_included or segment.

improvement3

medium

null segment sent to PostHog instead of undefined

services/billing/src/handlers/mrr-sync.handler.ts:411

organizations.segment is nullable. `segment: led.segment` passes null; event type declares `segment?: string`. Some PostHog SDK versions store null as the string 'null'. Fix: `segment: led.segment ?? undefined`.

low

trackServer failure stops all remaining ledger rows — no per-item try/catch

services/billing/src/handlers/mrr-sync.handler.ts:392

A single PostHog 5xx inside the for-loop throws and skips all subsequent orgs. Match section 8 pattern: wrap each trackServer call in its own try/catch, log per-org, continue.

low

Handler section 9 comment block duplicates mrr.ts JSDoc verbatim

services/billing/src/handlers/mrr-sync.handler.ts:354

20-line WHY block restates what mrr.ts already explains. Retain handler-specific context and cross-reference computeVariableMrrMxn docs for business rationale.

History · 2 commits

  1. eaa8b95needs attentionincremental0H · 1M · 2L2026-08-06 17:53
  2. 4aabeffneeds attentionfull0H · 3M · 5L2026-08-06 15:06current