feat/smarter-mrr
needs attentionviewing older commitaf89f54 · incrementalpre-PRreviewed 2026-08-10 17:36 UTC1H · 2M · 9L · 4I- Purpose
- Fixes correctness issues in the Smarter MRR event-sourced billing feature: UTC was used instead of Mexico-local time for year_month queries, and the FX rate fetch had no timeout or sanity bounds.
- Goal
- Ensure MRR month counts target the correct Mexico-local billing month and FX fetch is robust against hung hosts and garbage rates.
- Sub-goals
- Replace UTC-derived year_month with America/Mexico_City local month in both the manual refresh script and the Lambda handler
- Add 5s AbortSignal timeout + plausibility bounds (5-100 USD/MXN) to FX fetch with primary+fallback
- Add unit tests for fetchUsdMxnRate (6 cases) and computeSmarterMrr/readSmarterUsdRate/currentMexicoYearMonth
- Add isNull(deletedAt) filter to findCalculatedBillCountsByOrg to exclude soft-deleted orgs
- What
- 8 files changed in this increment: bill.queries.ts (input guard + deletedAt filter), fx.ts (timeout + sanity), smarter-mrr.ts (currentMexicoYearMonth), index.ts (export), fx.test.ts (new), smarter-mrr.test.ts (new), smarter-mrr-refresh.ts (UTC→MX, orm dedup), mrr-sync.handler.ts (UTC→MX).
- Why
- Bills store year_month as the Mexico-local billing month. At UTC midnight on the 1st, Mexico is still in the previous month (UTC-6), causing the sync to count zero bills and emit $0 MRR for a real billing month.
- Areas
- packages/billing+293−1domains/utility/src/bill+60−0services/billing/src/handlers+7−1scripts/billing+16−6
- Blast
- 8 files in this increment, +376/−8. Branch total: 15 files +805/−1 across billing, analytics, infra/posthog, and DB seed.
Findings · 16
correctness3
Regex guard accepts semantically invalid months (e.g. 2026-13)
domains/utility/src/bill/bill.queries.ts:2908
YYYY-MM format validated but month range 01-12 not checked; out-of-range months match nothing and read as $0 MRR.
Timezone hardcoded to America/Mexico_City — Baja California assumption unverified
packages/billing/src/smarter-mrr.ts:52
Tijuana still observes DST at a different offset. Confirm bill ingest uses CDMX time universally.
Persistent FX outage invisible in handler result
services/billing/src/handlers/mrr-sync.handler.ts:449
FX timeout (up to 10s) swallowed; result shows smarterSnapshots:0 with no error entry. Only visible via CloudWatch.
security2
User-controlled yearMonth reflected verbatim in thrown error message
domains/utility/src/bill/bill.queries.ts:2908
Raw argv/caller value interpolated into error string. Truncate to 20 chars or omit if errors surface in API responses.
External HTTP body reflected in error (capped at 200 chars)
packages/billing/src/fx.ts:62
Hardcoded URL, not user-controlled. The 200-char cap is correct. No action needed.
conventions2
Domain query layer throws plain Error instead of returning Result<T,E>
domains/utility/src/bill/bill.queries.ts:2906
FCIS requires domain code to return discriminated-union errors (_tag + statusCode). Convert to BillErrors.invalidYearMonth() Result.err, or move the guard to the calling shell.
Pure utility currentMexicoYearMonth exported from infrastructure package
packages/billing/src/smarter-mrr.ts:48
No violation today but importing into domain decisions would invert the dependency direction.
tests6
isSaneRate boundary thresholds not pinned (extreme values only)
packages/billing/src/__tests__/fx.test.ts:40
Tests use 0.0001/9999 but omit near-boundary: 4.99 (rejected), 5.01 (accepted), 100.0 (rejected). A constant change would pass silently.
Fallback fetch network-level throw not tested
packages/billing/src/__tests__/fx.test.ts:1
If fallback fetch itself rejects (AbortError), exception propagates raw — distinct from the !ok branch. Add: primary throws + fallback throws.
Primary ok=true but result!='success' with valid rate not tested
packages/billing/src/__tests__/fx.test.ts:1
open.er-api.com returns result:'error' on quota exhaustion (HTTP 200). The fallback is correct but untested.
currentMexicoYearMonth missing Dec to Jan year rollover test
packages/billing/src/__tests__/smarter-mrr.test.ts:56
2027-01-01T03:00Z should return 2026-12. A formatToParts regression would not be caught by the current suite.
No unit test for findCalculatedBillCountsByOrg input validation guard
domains/utility/src/bill/bill.queries.ts:2906
Guard fires before any DB query — testable without mock. Call with '2026-8', '26-08', '' and assert throws.
fetchedAt field not asserted in any test
packages/billing/src/__tests__/fx.test.ts:16
Part of UsdMxnRate interface recorded in PostHog. Add: expect(r.fetchedAt).toMatch(/^\d{4}-\d{2}-\d{2}T/).
improvement3
en-CA locale eliminates formatToParts verbosity
packages/billing/src/smarter-mrr.ts:48
Intl.DateTimeFormat('en-CA', {...}).format(now).slice(0,7) gives YYYY-MM directly with no nullable .find() calls.
Sanity bounds MIN_RATE=5/MAX_RATE=100 undocumented — silent fallback to stale rate
packages/billing/src/fx.ts:25
On out-of-range live rate, function silently falls back to stale Supabase rate. A comment + log on rejection would make this observable.
orm/db split correctness fix needs a comment to prevent regression
scripts/billing/smarter-mrr-refresh.ts:50
Without explanation, a reader may collapse to two drizzle() calls and re-introduce the duplicate connection bug.