feat: subscription billing correctness (#1684)

backend/billing/jobs/index.ts

@@ -26,6 +26,7 @@ export {
 export type {
   RevenueSchedule,
   RevenueScheduleEntry,
+  EntryRecognitionProgress,
   RecognitionJournalEntry,
   RevenueRecognitionRepository,
   RecognitionJobMetrics,

backend/billing/jobs/monthlyRevenueRecognitionJob.ts

@@ -19,6 +19,12 @@ export interface RevenueScheduleEntry {
   periodEnd: number; // Unix ms
   recognisedAmount: number;
   isRecognised: boolean;
+  /**
+   * Cumulative amount already recognised for this entry across all prior
+   * runs. Optional so schedules persisted before progress tracking existed
+   * still load; treat a missing value as 0.
+   */
+  recognisedToDate?: number;
 }
 
 export interface RevenueSchedule {
@@ -42,6 +48,12 @@ export interface RecognitionJournalEntry {
   type: 'scheduled' | 'accelerated' | 'partial';
 }
 
+/** Per-entry cumulative recognition progress recorded by a run. */
+export interface EntryRecognitionProgress {
+  periodStart: number;
+  recognisedToDate: number;
+}
+
 export interface RevenueRecognitionRepository {
   /** Return all schedules where at least one entry is not yet recognised. */
   getPendingSchedules(): Promise<RevenueSchedule[]>;
@@ -54,6 +66,19 @@ export interface RevenueRecognitionRepository {
   ): Promise<void>;
   /** Mark individual schedule entries as recognised. */
   markEntriesRecognised(subscriptionId: string, entryPeriodStarts: number[]): Promise<void>;
+  /**
+   * Persist cumulative per-entry recognition progress.
+   *
+   * Optional. Without it a partially-elapsed entry cannot record how much of
+   * itself it has already recognised, and the next run re-recognises the whole
+   * elapsed portion — inflating recognised revenue on every run. When the
+   * method is absent the job degrades to recognising each entry only once,
+   * at its full amount, which is slower but never over-counts.
+   */
+  recordEntryProgress?(
+    subscriptionId: string,
+    progress: EntryRecognitionProgress[]
+  ): Promise<void>;
 }
 
 export interface RecognitionJobMetrics {
@@ -70,6 +95,12 @@ function nextId(): string {
   return `rev-${Date.now().toString(36)}-${(++_seq).toString(36)}`;
 }
 
+/** Rounds to minor units, mapping non-finite input to zero. */
+function roundMoney(amount: number): number {
+  if (!Number.isFinite(amount)) return 0;
+  return Math.round((amount + Number.EPSILON * Math.sign(amount)) * 100) / 100;
+}
+
 /**
  * Determine how much of a single schedule entry to recognise as of `now`.
  * Returns 0 for entries that haven't started yet or amount = 0 (free trial).
@@ -150,13 +181,48 @@ export class MonthlyRevenueRecognitionJob {
     for (const schedule of schedules) {
       localMetrics.schedulesProcessed += 1;
       const recognisedPeriodStarts: number[] = [];
+      const progressUpdates: EntryRecognitionProgress[] = [];
       let totalRecognisedForSchedule = 0;
 
+      // Deferred revenue is what was contractually billed but not yet
+      // recognised, i.e. the schedule total minus everything recognised on
+      // prior runs and earlier in *this* run. Ignoring prior runs produced a
+      // deferred balance that grew without bound.
+      const previouslyRecognised = (schedule.entries ?? []).reduce(
+        (sum, entry) => sum + (entry.recognisedToDate ?? 0),
+        0
+      );
+      let recognisedSoFar = previouslyRecognised;
+
+      const canTrackProgress = typeof this.repo.recordEntryProgress === 'function';
+
       for (const entry of schedule.entries) {
-        const { amount, type } = computeEntryRecognition(entry, now);
-        if (!type || amount <= 0) continue;
+        const { amount: cumulativeTarget, type } = computeEntryRecognition(entry, now);
+        if (!type || cumulativeTarget <= 0) continue;
+
+        // Recognise only the increment since the last run. Recognising the
+        // full elapsed portion again on every run is what compounded the
+        // misstatement day after day.
+        const alreadyRecognised = entry.recognisedToDate ?? 0;
+        const amount = canTrackProgress
+          ? roundMoney(Math.max(cumulativeTarget - alreadyRecognised, 0))
+          : // Without progress tracking, only recognise an entry once its
+            // period has fully elapsed, so it is never recognised twice.
+            type === 'partial'
+            ? 0
+            : roundMoney(cumulativeTarget);
+
+        if (amount <= 0) {
+          if (type === 'scheduled' || type === 'accelerated') {
+            // Already fully recognised on a prior run; close it out.
+            recognisedPeriodStarts.push(entry.periodStart);
+          }
+          continue;
+        }
 
-        const deferredBefore = schedule.totalAmount - totalRecognisedForSchedule;
+        const deferredBefore = roundMoney(
+          Math.max(schedule.totalAmount - recognisedSoFar, 0)
+        );
         const journalEntry: RecognitionJournalEntry = {
           id: nextId(),
           subscriptionId: schedule.subscriptionId,
@@ -165,19 +231,33 @@ export class MonthlyRevenueRecognitionJob {
           periodEnd: entry.periodEnd,
           recognisedAmount: amount,
           deferredBefore,
-          deferredAfter: deferredBefore - amount,
+          // Deferred can never go negative: recognising more than was
+          // deferred would be reported as an amount owed back to the merchant.
+          deferredAfter: roundMoney(Math.max(deferredBefore - amount, 0)),
           createdAt: now,
           type,
         };
 
         try {
           await this.repo.writeJournalEntry(journalEntry);
+          recognisedSoFar += amount;
           totalRecognisedForSchedule += amount;
           localMetrics.entriesRecognised += 1;
           localMetrics.totalAmountRecognised += amount;
 
-          // Only mark fully-elapsed entries as done.
-          if (type === 'scheduled' || type === 'accelerated') {
+          progressUpdates.push({
+            periodStart: entry.periodStart,
+            recognisedToDate: roundMoney(alreadyRecognised + amount),
+          });
+
+          // Only close out entries whose period has fully elapsed AND that
+          // are now fully recognised.
+          const recognisedTotal = roundMoney(alreadyRecognised + amount);
+          if (
+            type === 'scheduled' ||
+            type === 'accelerated' ||
+            recognisedTotal >= roundMoney(entry.recognisedAmount)
+          ) {
             recognisedPeriodStarts.push(entry.periodStart);
           }
         } catch (err) {
@@ -195,6 +275,12 @@ export class MonthlyRevenueRecognitionJob {
             recognisedDelta: totalRecognisedForSchedule,
             deferredDelta: -totalRecognisedForSchedule,
           });
+          if (progressUpdates.length > 0 && canTrackProgress) {
+            await this.repo.recordEntryProgress!(
+              schedule.subscriptionId,
+              progressUpdates
+            );
+          }
           if (recognisedPeriodStarts.length > 0) {
             await this.repo.markEntriesRecognised(
               schedule.subscriptionId,

backend/services/analytics/subscriptionAnalyticsService.ts

@@ -317,9 +317,14 @@ export class SubscriptionAnalyticsService {
   // ── Private helpers ─────────────────────────────────────────────────────────
 
   private _monthlyRevenue(sub: Subscription): number {
-    // Normalise any billing cycle to monthly revenue
-    if (sub.billingCycle === BillingCycle.YEARLY) return sub.price / 12;
-    if (sub.billingCycle === BillingCycle.WEEKLY) return sub.price * 4.345;
+    // Normalise any billing cycle to monthly revenue. `ANNUAL` was missing
+    // here, so an annual subscription's full yearly price was reported as
+    // MRR — inflating MRR/ARR/churn by 12×. `monthlyEquivalent` also keeps
+    // the weekly factor identical to the frontend (52/12, not 4.345).
+    if (sub.billingCycle === BillingCycle.YEARLY || sub.billingCycle === BillingCycle.ANNUAL) {
+      return sub.price / 12;
+    }
+    if (sub.billingCycle === BillingCycle.WEEKLY) return sub.price * (52 / 12);
     return sub.price;
   }
 }

backend/services/batchChargeService.ts

@@ -43,7 +43,7 @@ export interface BatchChargeResult {
 export class BatchChargeService {
   private intervalHandle: ReturnType<of setInterval> | null = null;
   private lastMatchTimestamp = 0;
-  private RunHistory: BatchChargeResult[] = [];
+  private runHistory: BatchChargeResult[] = [];
   private maxHistory = 50;
   private cronExpression = '0 0 * * *';
   private checkIntervalMs = 60_000;

backend/services/billing/groupBilling.ts

@@ -53,10 +53,24 @@ export interface GroupPlanCustomization {
   sharedFeatures: string[];
   memberLimits: Record<string, number>;
   ownerDiscount: number;
+  /** Tax rate in basis points (e.g. 2000 = 20%). Optional; 0 when unset. */
+  taxRateBps?: number;
   createdAt: number;
   updatedAt: number;
 }
 
+/** Rounds to minor units, mapping non-finite input to zero. */
+const roundMoney = (amount: number): number => {
+  if (!Number.isFinite(amount)) return 0;
+  return Math.round((amount + Number.EPSILON * Math.sign(amount)) * 100) / 100;
+};
+
+/** Constrains a basis-point tax rate to a sane `[0, 100%]` range. */
+const clampRate = (bps: number | undefined): number => {
+  if (bps === undefined || !Number.isFinite(bps)) return 0;
+  return Math.min(Math.max(bps, 0), 10_000);
+};
+
 const createId = (prefix: string): string =>
   `${prefix}_${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`;
 
@@ -97,28 +111,40 @@ export class GroupBillingService {
     };
   }
 
-  aggregateCharges(group: SubscriptionGroup, periodDays = 30): GroupBillingLineItem[] {
-    const cutoff = Date.now() - periodDays * 24 * 60 * 60 * 1000;
-    const recentCharges = group.charges.filter(
-      (c) => c.chargedAt.getTime() >= cutoff
-    );
+  aggregateCharges(
+    group: SubscriptionGroup,
+    periodStart: number,
+    periodEnd: number
+  ): GroupBillingLineItem[] {
+    // The window is the requested period, not "the last N days relative to
+    // now". Deriving the cutoff from `Date.now()` made a historical invoice
+    // price a completely different set of charges than the period it claims
+    // to cover, so the line items never summed to the stated period.
+    const start = Math.min(periodStart, periodEnd);
+    const end = Math.max(periodStart, periodEnd);
+    const periodCharges = group.charges.filter((c) => {
+      const chargedAt = c.chargedAt.getTime();
+      return chargedAt >= start && chargedAt <= end;
+    });
 
     const memberCharges = new Map<string, number>();
     for (const member of group.members) {
       memberCharges.set(member.address, 0);
     }
 
-    for (const charge of recentCharges) {
+    for (const charge of periodCharges) {
       for (const item of charge.breakdown) {
         const current = memberCharges.get(item.memberAddress) ?? 0;
-        memberCharges.set(item.memberAddress, current + item.amount);
+        memberCharges.set(item.memberAddress, roundMoney(current + item.amount));
       }
     }
 
+    const periodDays = Math.max(1, Math.ceil((end - start) / (24 * 60 * 60 * 1000)));
+
     return Array.from(memberCharges.entries()).map(([address, amount]) => ({
       memberAddress: address,
       amount,
-      description: `Aggregated charges over last ${periodDays} days`,
+      description: `Aggregated charges over ${periodDays} day(s)`,
     }));
   }
 
@@ -130,15 +156,24 @@ export class GroupBillingService {
     periodEnd: number,
     currency = 'USD'
   ): GroupInvoice {
-    const aggregated = this.aggregateCharges(group, Math.ceil((periodEnd - periodStart) / (24 * 60 * 60 * 1000)));
-    const totalAmount = aggregated.reduce((sum, item) => sum + item.amount, 0);
+    const aggregated = this.aggregateCharges(group, periodStart, periodEnd);
+    const totalAmount = roundMoney(aggregated.reduce((sum, item) => sum + item.amount, 0));
 
     const customization = this.planCustomizations.get(group.groupId);
-    const discountAmount = customization?.ownerDiscount
-      ? (totalAmount * customization.ownerDiscount) / 100
-      : 0;
-    const taxAmount = 0;
-    const finalAmount = totalAmount - discountAmount + taxAmount;
+
+    // `ownerDiscount` is a percentage but arrives from user-editable config.
+    // A negative value used to *increase* the bill and anything above 100
+    // produced a negative payable amount, so both ends are clamped.
+    const rawDiscountPercent = customization?.ownerDiscount ?? 0;
+    const discountPercent = Math.min(Math.max(Number.isFinite(rawDiscountPercent) ? rawDiscountPercent : 0, 0), 100);
+    const discountAmount = roundMoney((totalAmount * discountPercent) / 100);
+
+    // Tax was hard-coded to 0, so group invoices were never taxed even for
+    // entities inside a tax jurisdiction.
+    const taxRate = clampRate(customization?.taxRateBps);
+    const taxableAmount = Math.max(totalAmount - discountAmount, 0);
+    const taxAmount = roundMoney((taxableAmount * taxRate) / 10_000);
+    const finalAmount = roundMoney(Math.max(taxableAmount + taxAmount, 0));
 
     const invoice: GroupInvoice = {
       id: createId('ginv'),

backend/services/billing/interfaces.ts

@@ -222,7 +222,7 @@ export interface IPartnerService {
 
 export interface IGroupBillingService {
   generateBillingSummary(group: any): any;
-  aggregateCharges(group: any, periodDays?: number): any[];
+  aggregateCharges(group: any, periodStart: number, periodEnd: number): any[];
   generateInvoice(group: any, periodStart: number, periodEnd: number, currency?: string): any;
   issueInvoice(invoiceId: string, groupId: string): any | null;
   markInvoicePaid(invoiceId: string, groupId: string): any | null;

backend/services/billing/meteringService.ts

@@ -12,6 +12,12 @@ const SOFT_THRESHOLD_RATIO = 0.8;
 const HARD_THRESHOLD_RATIO = 1.0;
 const MAX_BATCH_SIZE = 500;
 
+/** Monotonic sequence used to keep same-millisecond auto-keys distinct. */
+let _autoSequence = 0;
+function nextAutoSequence(): number {
+  return ++_autoSequence;
+}
+
 export interface UsageMetric {
   userId: string;
   metricType: 'api' | 'compute' | 'storage';
@@ -126,7 +132,14 @@ export class MeteringService {
   }
 
   private async recordOne(metric: UsageMetric): Promise<UsageIngestResult> {
-    const idempotencyKey = metric.idempotencyKey ?? `auto-${meterKey(metric.userId, metric.metricType)}-${metric.timestamp.getTime()}`;
+    // An auto-generated key built purely from the event timestamp collides
+    // for two legitimate events landing in the same millisecond, and the
+    // second one was then reported as a duplicate and silently never rated —
+    // under-billing the customer. A monotonic sequence keeps distinct events
+    // distinct. Callers that need retry-idempotency must pass an explicit key.
+    const idempotencyKey =
+      metric.idempotencyKey ??
+      `auto-${meterKey(metric.userId, metric.metricType)}-${metric.timestamp.getTime()}-${nextAutoSequence()}`;
 
     const cached = this.seenIdempotencyKeys.get(idempotencyKey);
     if (cached) {
@@ -228,12 +241,22 @@ export class MeteringService {
     this.events.delete(meterKey(userId, metricType));
   }
 
-  getUsageByMetric(subscriptionId: string): Record<string, number> {
+  /**
+ * Sums recorded usage per metric for a subscription.
+ *
+ * An optional window is essential: without one this returns the entire
+ * lifetime total, which callers then label as a single billing period.
+ */
+getUsageByMetric(subscriptionId: string, from?: Date, to?: Date): Record<string, number> {
+    const fromMs = from instanceof Date ? from.getTime() : undefined;
+    const toMs = to instanceof Date ? to.getTime() : undefined;
     const usage: Record<string, number> = {};
     for (const entry of this.usageHistory) {
-      if (entry.subscriptionId === subscriptionId) {
-        usage[entry.metric] = (usage[entry.metric] || 0) + entry.value;
-      }
+      if (entry.subscriptionId !== subscriptionId) continue;
+      const entryTime = entry.timestamp.getTime();
+      if (fromMs !== undefined && entryTime < fromMs) continue;
+      if (toMs !== undefined && entryTime > toMs) continue;
+      usage[entry.metric] = (usage[entry.metric] || 0) + entry.value;
     }
     return usage;
   }
@@ -299,13 +322,29 @@ export class MeteringService {
   }
 
   calculateUsageBilling(subscriptionId: string, unitPrice: number, currency: string): UsageBillingIntegration {
-    const metricUsage = this.getUsageByMetric(subscriptionId);
     const now = new Date();
     const periodStart = new Date(now.getFullYear(), now.getMonth(), 1);
-    const periodEnd = new Date(now.getFullYear(), now.getMonth() + 1, 0);
+    // `new Date(y, m + 1, 0)` is midnight *starting* the last day, which
+    // excluded the final day of the month from the billed usage.
+    const periodEnd = new Date(now.getFullYear(), now.getMonth() + 1, 1);
+
+    // Bill only usage inside the period being invoiced. Summing the full
+    // lifetime history while labelling the result "this month" over-billed
+    // every account with any prior history, compounding each cycle.
+    const metricUsage = this.getUsageByMetric(subscriptionId, periodStart, periodEnd);
     let totalMetered = 0;
     for (const usage of Object.values(metricUsage)) totalMetered += usage;
-    return { subscriptionId, meteredAmount: totalMetered, unitPrice, totalAmount: totalMetered * unitPrice, currency, period: { start: periodStart, end: periodEnd } };
+
+    const totalAmount = Number.isFinite(unitPrice) ? totalMetered * unitPrice : 0;
+
+    return {
+      subscriptionId,
+      meteredAmount: totalMetered,
+      unitPrice,
+      totalAmount,
+      currency,
+      period: { start: periodStart, end: periodEnd },
+    };
   }
 }