You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR-N2 of the Compute Pricing layer (#3533). Builds the attribution + extras balance
services on top of PR-N1 plan versioning. All new code paths are gated behind config.billing.meterMode = false (backward compat absolute — non-compute downstreams
are entirely unaffected).
models/billing.extraBalance.schema.js — Zod mirror with full constraint symmetry.
repositories/billing.extraBalance.repository.js — atomic Mongo ops: getOrCreate, creditPack (idempotent on stripeSessionId), debit (guarded by
cachedBalance >= amount AND refId dedup), addExpirationEntries (idempotent sweep), getBalance (cheap read).
services/billing.extra.service.js — creditPack, debit, expireOldEntries, refundPartial (proportional, balance may go negative = economic reflection), listLedger (paginated, newest-first).
services/billing.meter.service.js — unitsFromCosts (frozen plan ratio lookup,
dollarsToUnitRatio, METER_RUN_BASE floor), attribute (idempotent on history._id).
services/billing.reset.service.js — resetWeek (archive-then-upsert, E11000-safe), resetAllDue (sweeps active subs by currentPeriodStart), isoWeekKey helper.
services/billing.usage.service.js — add currentWeekKey (ISO 8601), incrementMeter
(full flow: quota snapshot, overflow→extras, 80%/100% threshold events deduped per
cycle), getMeter.
services/billing.plan.service.js — add bumpVersionWithRetry (exp backoff
100/300/900ms on E11000, max 3 attempts).
Architecture decisions
Idempotency via atomic filter: both creditPack and debit use $ne guards
inside the findOneAndUpdate filter so the check + update is a single round-trip
(no TOCTOU). No separate "does it exist?" read for the happy path.
Threshold dedup: alertedAt80/alertedAt100 are set atomically via updateOne
with { alertedAt80: null } filter so only one pod wins the event emit per cycle.
resetWeek race safety: $ne weekKey archive pass + E11000 retry on upsert
handles concurrent pod resets gracefully.
refundPartial negative balance: if units already consumed, balance goes negative —
this is the correct economic reflection and matches the design intent.
Tests
79 new tests (739 total, baseline 660). All edge cases from the spec:
@PierreBrisorgueil has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 42 seconds before requesting another review.
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.
⌛ How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.
We recommend that you space out your commits to avoid hitting the rate limit.
🚦 How do rate limits work?
CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
This PR introduces a comprehensive billing subsystem for managing prepaid "extra" meter units across organizations. It adds Mongoose models and Zod schemas for billing extra-balance ledgers, repositories for atomic ledger mutations with idempotency, services for credit packs/debits/refunds/expirations, and related meter/reset/usage service enhancements, alongside extensive unit test coverage.
Changes
Cohort / File(s)
Summary
Data Models & Schemas modules/billing/models/billing.extraBalance.model.mongoose.js, modules/billing/models/billing.extraBalance.schema.js
Defines Mongoose model and Zod schema for BillingExtraBalance with append-only ledger of typed mutations (topup, debit, refund, expiration, adjustment), sparse indexes for idempotency/replay protection, and cached balance tracking.
Extra Balance Repository modules/billing/repositories/billing.extraBalance.repository.js
Atomic MongoDB repository for idempotent ledger operations: getOrCreate, creditPack, debit, addExpirationEntries, refundPartial, and getBalance with replay protection via session/ref IDs and cached-balance semantics.
Extends usage repo with findByWeek, incrementMeter (with idempotency and snapshot), archiveOtherWeeks, upsertWeekSnapshot, and markThreshold; adds subscription repo query findAllDueForReset for weekly meter resets.
Adds extra-balance service with credit/debit/refund/expiration/listing; meter service for USD-to-units conversion; reset service for weekly meter resets; extends usage service with week-based metering; adds retry wrapper for plan version bumps.
Unit Tests modules/billing/tests/billing.extra*.unit.tests.js, modules/billing/tests/billing.meter.service.unit.tests.js, modules/billing/tests/billing.plan.service.bumpretry.unit.tests.js, modules/billing/tests/billing.reset.service.unit.tests.js, modules/billing/tests/billing.usage.*unit.tests.js
Comprehensive Jest test suites validating schema parsing, repository atomicity and idempotency, service delegation and business logic, meter computation, and weekly reset flow.
Sequence Diagram
sequenceDiagram
actor Client
participant ExtraService as BillingExtraService
participant Config
participant Repository as ExtraBalanceRepository
participant Mongoose as Mongoose Model
rect rgba(100, 150, 200, 0.5)
Note over Client,Mongoose: Credit Pack Flow
Client->>ExtraService: creditPack(orgId, packId, stripeSessionId)
ExtraService->>Config: read pack metadata
ExtraService->>ExtraService: compute expiresAt if expiryDays present
ExtraService->>Repository: creditPack(orgId, units, stripeSessionId, expiresAt)
Repository->>Mongoose: findOneAndUpdate with $push ledger + $inc cachedBalance
Mongoose-->>Repository: { doc, applied }
Repository-->>ExtraService: { doc, applied }
ExtraService-->>Client: { applied, doc }
end
rect rgba(200, 100, 150, 0.5)
Note over Client,Mongoose: Debit Flow (with Balance Check)
Client->>ExtraService: debit(orgId, units, refId)
ExtraService->>Repository: debit(orgId, units, refId)
Repository->>Mongoose: findOneAndUpdate with cachedBalance >= amount guard
alt Sufficient Balance & Not Replayed
Mongoose-->>Repository: { doc, applied: true }
else Insufficient Balance or Replayed
Mongoose-->>Repository: { doc, applied: false }
end
Repository-->>ExtraService: { doc, applied }
ExtraService-->>Client: { applied }
end
rect rgba(150, 200, 100, 0.5)
Note over Client,Mongoose: Expiration Sweep Flow
Client->>ExtraService: expireOldEntries(orgId)
ExtraService->>Repository: addExpirationEntries(orgId, now)
Repository->>Mongoose: fetch topups without expire entries
Mongoose-->>Repository: [expired topup docs]
Repository->>Mongoose: findOneAndUpdate $push expiration entries
Mongoose-->>Repository: count added
Repository-->>ExtraService: count
ExtraService-->>Client: count
end
Loading
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related issues
Compute pricing layer (upstream Node) #3533: Directly adds the compute/extra-balance models, schemas, repositories, and services that implement the "Compute pricing layer" plan's prepaid meter unit tracking requirement.
The title clearly and concisely summarizes the main changes: adding meter service, extras balance, and weekly reset functionality as part of PR-N2 of the Compute Pricing layer.
Description check
✅ Passed
The description comprehensively covers all required template sections: detailed Summary with context (PR-N2, gating by config.billing.meterMode), new/modified files with purpose, architecture decisions, tests (79 new), and test plan status.
Docstring Coverage
✅ Passed
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
✅ Passed
Check skipped because no linked issues were found for this pull request.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches🧪 Generate unit tests (beta)
Create PR with unit tests
Commit unit tests in branch feature/meter-service-extras
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.
❌ Patch coverage is 90.49180% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.33%. Comparing base (0f2320d) to head (063a1c9). ⚠️ Report is 1 commits behind head on master.
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds the PR-N2 “compute pricing” building blocks to the billing module: weekly meter attribution, an extras-balance ledger, and reset/idempotency/retry helpers, all gated behind config.billing.meterMode.
Changes:
Introduces meter attribution services (billing.meter.service.js, billing.usage.service.js) including ISO-week metering and 80%/100% threshold marking.
Adds extras balance persistence via a ledger-based Mongoose model + repository + service (BillingExtraBalance).
Adds weekly reset logic (billing.reset.service.js) and E11000-safe version bump retries for plans (bumpVersionWithRetry).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 13 comments.
… edge guards
HIGH: Move all mongoose access out of the 3 target services into repositories.
- extraBalance.repository: add refundPartial() atomic ledger write
- usage.repository: add markThreshold(), archiveOtherWeeks(), upsertWeekSnapshot()
- subscription.repository: add findAllDueForReset()
(also fixes BillingSubscription->Subscription model name bug in resetAllDue)
MEDIUM: refundPartial ambiguous-pack guard — return applied=false+reason
when 0 or >1 packs share same meterUnits; TODO PR-N3 for packId from metadata.
resetAllDue window query limitation documented with TODO PR-N5.
LOW: Remove no-op sparse:true from LedgerEntry subdocument fields.
Add z.refine(n !== 0) on LedgerEntry.amount to reject zero-amount entries.
Tests: 749 passing (+14 new tests for new repo methods + edge cases).
The Subscription model uses 'organization' (not 'organizationId').
Projection + service consumer were both broken, silently making
resetAllDue a no-op (sub.organizationId always undefined).
Caught by critical-review pass 2 on PR-N2.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 18
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modules/billing/models/billing.extraBalance.model.mongoose.js`:
- Around line 38-41: Add a schema-level validator to the Mongoose schema that
rejects zero ledger amounts by updating the amount field definition (the
"amount" property on the ExtraBalance/extraBalance schema in
billing.extraBalance.model.mongoose.js) to include a validator that fails when
value === 0 and returns a clear error message; implement this as a `validate`
function or a custom validator option on the existing amount field so any
attempt to save an amount of 0 is rejected at the schema level.
In `@modules/billing/models/billing.extraBalance.schema.js`:
- Around line 20-40: LedgerEntry currently only rejects zero amounts but allows
wrong signs for different LedgerKind values; update the LedgerEntry schema to
add a refine check on the pair (kind, amount) that enforces sign rules (e.g.,
for kinds representing topup/adjustment require amount > 0, for kinds
representing debit/expiration/refund/refund-clawback require amount < 0) and
return a clear validation message; locate the LedgerEntry z.object and add a
refinement that reads kind and amount and uses the LedgerKind discriminants (the
LedgerEntry schema, the amount field, and the kind value) to validate sign
consistency.
In `@modules/billing/repositories/billing.extraBalance.repository.js`:
- Around line 56-57: Add repository-level guards in creditPack, debit, and
refundPartial to validate numeric amounts and non-empty idempotency/session
keys: at the top of each function (after the existing isValidOrgId check in
creditPack) verify that amount is a finite number > 0 and that the idempotency
key / stripeSessionId parameter is a non-empty string; if a check fails return
the same shape used for invalid orgs (e.g., { doc: null, applied: false }) and
optionally log the invalid input, mirroring the existing early-return style to
prevent any ledger mutation from invalid inputs.
In `@modules/billing/repositories/billing.usage.repository.js`:
- Around line 103-108: The code currently validates breakdown keys but not their
values, so non-numeric values can break the MongoDB $inc update; inside the loop
over Object.entries(breakdown) (see SAFE_KEY_RE, incPayload and meterBreakdown
references) validate each value before adding to incPayload by checking it is a
finite number (e.g., typeof value === 'number' && isFinite(value') or by
coercing with Number(value) and ensuring !Number.isNaN(result)); if the value is
invalid, skip that entry (or optionally log a warning) and only add numeric
values as incPayload[`meterBreakdown.${key}`] = numericValue.
In `@modules/billing/services/billing.extra.service.js`:
- Around line 115-116: The idempotency key refundRefId built as
`refund-${stripeSessionId}-${amountRefundedCents}` can collide for multiple
partial refunds of the same cents; change the key to include a truly unique
component (for example append a UUID or timestamp/sequence) so each refund
attempt is distinct—update the construction of refundRefId (referencing
refundRefId, stripeSessionId, amountRefundedCents) to
`refund-${stripeSessionId}-${amountRefundedCents}-${uniqueSuffix}` where
uniqueSuffix is generated via crypto.randomUUID() or an injected
refundAttemptId/paymentIntent/sequence, and add the necessary import or
parameter to supply that unique value.
- Around line 79-83: The code assumes
BillingExtraBalanceRepository.getOrCreate(orgId) always returns a doc and
dereferences doc.ledger in refundPartial and listLedger, which can crash if
getOrCreate returns null; update both functions to check the returned value (the
local doc variable from getOrCreate) before accessing doc.ledger, e.g., if
(!doc) handle the error/return early (throw a descriptive error or return a safe
default) so subsequent uses of doc.ledger are guarded; reference the call
BillingExtraBalanceRepository.getOrCreate and the variables doc and doc.ledger
in your changes.
In `@modules/billing/services/billing.meter.service.js`:
- Around line 107-113: The code currently sets extrasConsumed based solely on
result.extrasConsumed before calling BillingExtraService.debit, which can
misreport consumption if the debit call fails or returns applied:false; change
the flow so you call const debitResult = await
BillingExtraService.debit(organizationId, result.extrasConsumed, idempotencyKey)
when result.extrasConsumed > 0, then only set extrasConsumed =
result.extrasConsumed if debitResult.applied is true (otherwise leave
extrasConsumed = 0), and wrap the debit call in a try/catch so exceptions don’t
cause extras to be reported as consumed; refer to BillingExtraService.debit and
the local extrasConsumed variable to locate where to apply this logic.
In `@modules/billing/services/billing.plan.service.js`:
- Around line 142-159: Validate and normalize the maxAttempts option at the
start of bumpVersionWithRetry: ensure maxAttempts is a positive integer (e.g.,
coerce via Math.floor and enforce >=1) or throw a clear error if invalid, so the
retry loop always runs at least once; update the function signature handling for
bumpVersionWithRetry to perform this guard before using backoffMs and entering
the for-loop and ensure any downstream logic (delay index into backoffMs) uses a
safe fallback if attempt index exceeds the array.
In `@modules/billing/services/billing.reset.service.js`:
- Around line 40-45: Update the JSDoc for the resetWeek function to reflect that
it can return either the upserted usage object or null when meter mode is off by
changing the `@returns` description/type to include null (e.g.,
Promise<Object|null> or Promise<null|Object>) and update the text to mention the
null return path when config?.billing?.meterMode is falsy; locate the JSDoc
above the resetWeek declaration to make this change.
In `@modules/billing/services/billing.usage.service.js`:
- Around line 161-193: The threshold event is emitted even when
UsageRepository.markThreshold did not actually update the doc (no dedup),
causing duplicate events; modify the logic in the alert branches (the block
using updatedDoc.alertedAt100/alertedAt80 and calling
UsageRepository.markThreshold) to capture the result of markThreshold and only
call billingEvents.emit('meter.threshold_crossed', ...) if the repository call
indicates a successful write (e.g., returns an object with modifiedCount > 0 or
a truthy success flag); keep the existing try/catch for best-effort but skip
emission when the markThreshold result shows no modification.
In `@modules/billing/tests/billing.extra.service.unit.tests.js`:
- Around line 16-22: The helper function makeDoc is a named standalone utility
and needs a JSDoc header: add a JSDoc block above makeDoc that documents the
parameter "overrides" (type Object, optional, describe possible keys like _id,
organization, ledger, cachedBalance) and the return type (Object with those
fields), using `@param` and `@returns` tags and a short description; ensure the
JSDoc clearly states defaults used by makeDoc and that the function returns the
merged document object.
In `@modules/billing/tests/billing.extraBalance.unit.tests.js`:
- Around line 135-143: The helper function makeDoc is missing a JSDoc header;
add a JSDoc block above makeDoc that documents the parameter and return value
(e.g., `@param` {Object} overrides - optional fields to merge into the default doc
and `@returns` {Object} the constructed document with properties like _id,
organization, ledger, cachedBalance, cachedBalanceAt). Ensure the JSDoc
describes the optional nature of overrides and the shape of the returned object
to satisfy the repository rule that every function include `@param` and `@returns`.
In `@modules/billing/tests/billing.meter.service.unit.tests.js`:
- Around line 16-24: Add a JSDoc block above the makePlan function describing
the function, include an `@param` {Object} [overrides] annotation that documents
optional override fields (e.g., _id, planId, version, meterQuota, ratios,
active) and an `@returns` {Object} annotation that describes the returned plan
object shape (same fields as defaults). Keep the JSDoc concise and place it
immediately above the makePlan declaration so linters recognize the
documentation.
In `@modules/billing/tests/billing.plan.service.bumpretry.unit.tests.js`:
- Around line 13-23: The named test helper makeDoc lacks a JSDoc header; add a
JSDoc block above the makeDoc function that documents the overrides parameter
(object, optional) with `@param` and describes the returned document object with
`@returns`, briefly outlining key properties (e.g., _id, planId, version,
meterQuota, ratios, effectiveFrom, effectiveUntil, active) so the helper
complies with the project /**/*.js rule requiring `@param/`@returns on named
functions.
In `@modules/billing/tests/billing.reset.service.unit.tests.js`:
- Around line 18-34: Add JSDoc blocks above the named helper functions makePlan
and makeUsageDoc describing their purpose, the single optional parameter
(overrides: Object) and the structure of the returned object; include `@param`
{Object} overrides - optional properties to override defaults and `@returns`
{Object} describing the returned plan/usage document fields (e.g., planId,
version, meterQuota, active for makePlan; _id, organizationId, weekKey,
meterUsed, meterQuota, planVersion for makeUsageDoc) so the tests comply with
the project's JSDoc requirement.
- Around line 200-203: The test fixtures set organizationId but the code under
test (resetAllDue) reads sub.organization, so update the mocked records returned
by mockMongooseSubscription.findAllDueForReset (and the similar fixtures at
lines ~216-218) to include the organization property (e.g., organization:
'507f1f77bcf86cd799439011' and organization: '507f1f77bcf86cd799439022') so the
test exercises the real wiring and passes the actual organization through to the
resetAllDue call.
In `@modules/billing/tests/billing.usage.repository.unit.tests.js`:
- Around line 16-25: Add a JSDoc block above the named function makeUsageDoc
that documents the parameter and return value: include an `@param` {Object}
overrides description (optional fields merged into the usage document) and an
`@returns` {Object} description (the constructed usage document object with
defaults applied); ensure the JSDoc briefly describes the function purpose to
satisfy the repo's /** */ rule for .js files.
In `@modules/billing/tests/billing.usage.service.unit.tests.js`:
- Around line 17-37: Add full JSDoc blocks above the named test helper functions
makePlan and makeUsageDoc describing their purpose, the optional overrides
parameter (object) and the shape of the returned object (properties like planId,
version, meterQuota, active for makePlan; _id, organizationId, weekKey, month,
meterUsed, meterQuota, planVersion, alertedAt80, alertedAt100,
consumedHistoryIds for makeUsageDoc). Ensure the JSDoc tags (`@param` {Object}
[overrides], `@returns` {Object}) are present and accurate so these named helpers
satisfy the file's JSDoc requirement (anonymous test callbacks remain exempt).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 79962e28-48ef-4f8a-9c2c-5f0a7694b922
📥 Commits
Reviewing files that changed from the base of the PR and between 0f2320d and 6149721.
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
PierreBrisorgueil
dismissed
coderabbitai[bot]’s stale review
Summary
PR-N2 of the Compute Pricing layer (#3533). Builds the attribution + extras balance
services on top of PR-N1 plan versioning. All new code paths are gated behind
config.billing.meterMode = false(backward compat absolute — non-compute downstreamsare entirely unaffected).
New files
models/billing.extraBalance.model.mongoose.js— ledger-based extra balance doc(topup / debit / refund / expiration / adjustment). Idempotency indexes on
ledger.stripeSessionId,ledger.historyId,ledger.expiresAt.models/billing.extraBalance.schema.js— Zod mirror with full constraint symmetry.repositories/billing.extraBalance.repository.js— atomic Mongo ops:getOrCreate,creditPack(idempotent on stripeSessionId),debit(guarded bycachedBalance >= amount AND refId dedup),
addExpirationEntries(idempotent sweep),getBalance(cheap read).services/billing.extra.service.js—creditPack,debit,expireOldEntries,refundPartial(proportional, balance may go negative = economic reflection),listLedger(paginated, newest-first).services/billing.meter.service.js—unitsFromCosts(frozen plan ratio lookup,dollarsToUnitRatio, METER_RUN_BASE floor),
attribute(idempotent on history._id).services/billing.reset.service.js—resetWeek(archive-then-upsert, E11000-safe),resetAllDue(sweeps active subs by currentPeriodStart),isoWeekKeyhelper.Modified files
repositories/billing.usage.repository.js— addfindByWeek(lean read),incrementMeter(atomic upsert with $ne idempotency key, E11000 retry, $setOnInsertsnapshot).
services/billing.usage.service.js— addcurrentWeekKey(ISO 8601),incrementMeter(full flow: quota snapshot, overflow→extras, 80%/100% threshold events deduped per
cycle),
getMeter.services/billing.plan.service.js— addbumpVersionWithRetry(exp backoff100/300/900ms on E11000, max 3 attempts).
Architecture decisions
creditPackanddebituse$neguardsinside the
findOneAndUpdatefilter so the check + update is a single round-trip(no TOCTOU). No separate "does it exist?" read for the happy path.
alertedAt80/alertedAt100are set atomically viaupdateOnewith
{ alertedAt80: null }filter so only one pod wins the event emit per cycle.$ne weekKeyarchive pass + E11000 retry on upserthandles concurrent pod resets gracefully.
this is the correct economic reflection and matches the design intent.
Tests
79 new tests (739 total, baseline 660). All edge cases from the spec:
Test plan
npm run lint— cleannpm run test:unit— 739 passingRefs #3533
Summary by CodeRabbit
Release Notes