feat(billing): credit-balance alerts for plans without a weekly quota - #4126
Merged
Merged
Conversation
A one-shot signup-grant plan (meterQuota=0) has no weekly usage doc to alert against — every unit debits BillingExtraBalance.cachedBalance directly, and that balance IS the real limit, so users ran out silently. incrementMeter now detects extras-balance threshold crossings (80/100, stateless — no alertedAtN dedup field, since a lifetime balance would falsely dedup a weekly-scoped field) against the debit's own pre/post balance and emits billing.extras.balance_threshold_crossed. billing.email.js sends a credit-warning or credit-exhausted email off of it, in absolute credits left, never a percentage. Scoped to meterQuota=0 plans with a valid signupGrant only — once a pack tops up the same balance, percent-of-grant is ambiguous (documented in README.md, along with the pack-expiry/refund alerting gap). Refs #3536 Claude-Session: https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3
README documented the level formula as (1 - threshold/100) * signupGrant — the exact form the code comment (previous commit) warns against: it hits float imprecision at common values (500 * (1 - 80/100) = 99.99999999999997, not 100), silently missing an exact boundary crossing. Correct it to signupGrant * (100 - threshold) / 100, and add the ERRORS.md entry. Claude-Session: https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3
Rewrite billing-credit-warning/exhausted template copy to stack-neutral
"usage" wording (was "Runs are paused"/"Runs keep working"), matching the
billing-quota-* templates' register.
Add an integration test driving incrementMeter on a meterQuota=0,
signupGrant plan against the real test DB: a debit crossing the 80%
level emits billing.extras.balance_threshold_crossed once with the
expected {threshold, remaining}; a later debit that stays above zero
emits nothing new.
Claude-Session: https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 23 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: pierreb-devkit/Node/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4126 +/- ##
==========================================
+ Coverage 94.42% 94.44% +0.02%
==========================================
Files 173 173
Lines 6042 6072 +30
Branches 1945 1961 +16
==========================================
+ Hits 5705 5735 +30
Misses 274 274
Partials 63 63
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Contributor
Author
|
@coderabbitai full review |
|
Fallback /critical-review (Opus, CodeRabbit rate-limited) findings: - the "no signupGrant" unit test used numbers that could never cross regardless of the guard (pre=0); reseed it so it WOULD cross without the guard, and add a signupGrant=0 case — the one that actually isolates the `> 0` half of the check (undefined already short-circuits via NaN arithmetic on its own). Verified both fail with the guard removed, then restored it. - add a duplicate_step replay case: a replayed debit (applied=false) must not re-derive a crossing. - drop the second, duplicated SUPPORTED_CREDIT_ALERT_THRESHOLDS set; filter with the existing thresholdFields map instead (single source). - document the real accepted limitation on a debit that throws outright: the outbox/retry-cron this module's own docs describe was removed (see crons/README.md), so an unreconciled failed debit never reaches the crossing check either — not "reconciled by a retry cron" as originally suggested, since that cron no longer exists. - drop the redundant "(0 credits left)" from the exhausted template. Claude-Session: https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3
Codecov patch check flagged 2 uncovered added lines: the catch block
around billingEvents.emit('billing.extras.balance_threshold_crossed', ...)
had no test forcing the emit to throw (mirrors the existing coverage
of the sibling runaway_debit emit-failure path).
Claude-Session: https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3
The comment on the "without a signupGrant" test claimed the missing-signupGrant guard alone kept it silent. Verified experimentally (guard removed, test still passed) that undefined already short-circuits via Number.isFinite/NaN arithmetic on its own — it's the signupGrant=0 case added alongside it that isolates the guard's `> 0` half. Comment was misleading; fix it rather than ship a self-contradicting note. Claude-Session: https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Users on a plan with no weekly quota (a one-shot signup credit grant) now get an email when their credits run low and when they run out.
Why
The 80 % / 100 % alerts only ran for plans with a weekly quota. On a
meterQuota: 0plan every unit is debited from the extras balance, and nothing warned the user before their usage stopped at zero.How
incrementMeter, inside the existing extras-debit branch, stateless crossing detection:post= the debit's own returnedcachedBalance(atomic$inc,returnDocument: 'after'),pre = post + extrasConsumed. LevelL = signupGrant * (100 - T) / 100forTinbilling.alerts.thresholdPercentsfiltered to 80/100; fires whenpre > L && post <= L. Concurrent debits get disjoint(post, pre]intervals, so exactly one caller crosses each level.effectiveQuota === 0andsignupGrantis a positive finite number. Quota plans that overflow into extras are untouched.billing.extras.balance_threshold_crossed{ organizationId, threshold, remaining, planId }, emitted in its own try/catch. Listener inbilling.email.js→ newbilling-credit-warning.html/billing-credit-exhausted.html. Copy states absolute credits left, never a % of the grant (wrong once packs or other grants top up the balance), and never the org name.(1 - T/100) * grant(float drift:500 * 0.2 = 99.99…misses an exact boundary). Noted in ERRORS.md.Tests
Unit: boundary
post === L, adjacent concurrent debits → one crosser, double crossing → one event, re-cross after a credit, quota>0 overflow → none, quota-0 without grant → none, listener → both templates.Integration (real DB):
incrementMeteron a quota-0 plan crosses the low level once, a further debit above zero emits nothing.Pre-push gate: kimi OK with nits (neutral copy + integration test applied; a concurrency concern was checked and rejected — each atomic debit returns its own post-state).
Closes #4117
https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3