improvement(billing): migrate hot path writes away from user_stats#4768
improvement(billing): migrate hot path writes away from user_stats#4768icecrasher321 wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Period totals are read from the ledger via Schema adds Removed side effects: MCP copilot call counter updates, workflow Invoice / subscription lifecycle resets and period-close overage now use Stripe invoice line periods and ledger sums when rolling Reviewed by Cursor Bugbot for commit 05d3bab. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 05d3bab. Configure here.
| eq(usageLog.billingEntityId, billingEntity.id), | ||
| eq(usageLog.billingPeriodStart, periodStart) | ||
| ) | ||
| : undefined |
There was a problem hiding this comment.
Daily refresh excludes pre-migration usage log rows
Medium Severity
The new billingEntityFilter uses eq() on columns (billingEntityType, billingEntityId, billingPeriodStart) that are NULL for all pre-migration usageLog rows. Since SQL equality with NULL returns false, every pre-existing usage log entry is invisible to daily refresh computation. All callers now pass billingEntity, so this filter is always active. During the billing period spanning deployment, days before the migration contribute zero to the refresh calculation, significantly under-deducting refresh credits and inflating effective usage — potentially causing paid users to hit limits prematurely.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 05d3bab. Configure here.
| return { | ||
| percentUsed: 0, | ||
| isWarning: false, | ||
| isExceeded: false, |
There was a problem hiding this comment.
Ledger usage ignored when userStats row missing
Low Severity
When no userStats row exists, checkUsageStatus returns currentUsage: 0 without querying getBillingPeriodUsageCost. Since recordUsage no longer writes to userStats at all, the ledger is now the sole record of post-migration usage. A user with ledger-tracked usage but a missing userStats row would be reported as having zero usage, bypassing usage limits entirely.
Reviewed by Cursor Bugbot for commit 05d3bab. Configure here.
| .catch((error) => { | ||
| logger.error('Failed to track MCP copilot call', { error, userId }) | ||
| }) | ||
| logger.debug('MCP copilot call tracked via request logs', { userId }) |
There was a problem hiding this comment.
Dead no-op function trackMcpCopilotCall still called
Low Severity
trackMcpCopilotCall was gutted to a single debug log but is still invoked at the call site. The function no longer tracks anything meaningful — it's dead code that adds confusion about whether MCP calls are actually being counted anywhere.
Reviewed by Cursor Bugbot for commit 05d3bab. Configure here.
Greptile SummaryThis PR migrates billing writes off the
Confidence Score: 3/5The enterprise usage reset in handleInvoiceFinalized was previously unconditional; the new guard makes it contingent on successfully parsing the billing period from the Stripe webhook, creating a silent failure mode for enterprise customers. A malformed or structurally unexpected invoice would leave enterprise currentPeriodCost accumulating indefinitely across billing periods. The baseline+ledger read pattern is architecturally correct and double-counting has been verified, but the sentinel duplication and index-sensitive eventKey are code-quality risks that warrant a follow-up before the WiP label is removed. apps/sim/lib/billing/webhooks/invoices.ts (enterprise reset regression), apps/sim/lib/billing/core/usage-log.ts (sentinel duplication, eventKey ordering) Important Files Changed
|
| const invoicePeriod = getSubscriptionLinePeriod(invoice, stripeSubscriptionId) | ||
| if (!invoicePeriod) { | ||
| logger.error('Missing subscription line period on subscription cycle invoice', { | ||
| invoiceId: invoice.id, | ||
| stripeSubscriptionId, | ||
| }) | ||
| return | ||
| } |
There was a problem hiding this comment.
Before this PR, the enterprise reset (isEnterprise branch) was unconditional — it always ran on subscription_cycle invoices regardless of whether period bounds were available. The new invoicePeriod guard is placed before that check, so if getSubscriptionLinePeriod returns null for any reason (unexpected Stripe invoice structure, missing stripeSubscriptionId match on any line item), the entire function exits and enterprise usage is never reset. Over time, currentPeriodCost would accumulate across billing periods and enterprise users would be incorrectly blocked or see inflated usage.
Consider separating the guard so enterprise plans can still fall back to a period-less reset:
if (!invoicePeriod) {
logger.error('Missing subscription line period on subscription cycle invoice', { ... })
if (isEnterprise(sub.plan)) {
await resetUsageForSubscription({ plan: sub.plan, referenceId: sub.referenceId })
}
return
}
Summary
Migrate billing away from hot path writes to user_stats table. Source of truth becomes usage logs.
Type of Change
Testing
WiP
Checklist