feat: make billable usage delivery durable - #1669
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (42)
📝 WalkthroughWalkthrough청구 가능 사용량을 위한 Changes청구 가능 사용량 저널
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-08-01T17:24:11.785Z · Commit: a473174 |
kang-heewon
force-pushed
the
fix/1518-billable-usage-journal
branch
from
August 1, 2026 15:58
044e20d to
4864bca
Compare
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/docs/src/content/docs/api/metering-core/src/interfaces/BillableUsageJournal.md`:
- Around line 8-10: Update the original documentation comment for
BillableUsageJournal in BillableUsageJournal.ts so request-process-independent
persistence is required only when durability is "persistent", while retaining
the atomic-transition requirement for implementations generally; then regenerate
the corresponding API Markdown without editing the generated file directly.
- Line 11: Update the BillableUsageJournal claim contract documentation to state
that a claim is valid only when its lease has not expired, using server time as
the reference, in addition to matching ownerId and a monotonically increasing
fencingToken. Apply the change to the source API comment and regenerate the
generated documentation.
In `@packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md`:
- Around line 5846-5863: Update the `metering/billable-usage-journal-required`
documentation to describe a configuration validation failure from
`MeterRegistry.assertJournalConfigured`, triggered when billing is `"required"`
without a persistent `billableUsageJournal`, including during `loadAll`, lazy
`get`, or `register`. Replace the incorrect upstream-failure and retry-later
guidance with operator/user actions to connect a persistent journal or change
the meter billing contract before retrying.
In `@packages/metering-core/src/index.ts`:
- Around line 165-177: Move the InMemoryBillableUsageJournal and
RedisBillableUsageJournal exports out of the Events section in the index barrel.
Add a dedicated journal/storage section header, keep the value exports together,
and place the related BillableUsage* type export block immediately after them.
In `@packages/metering-core/src/libs/BillableUsageJournal.ts`:
- Around line 197-204: Update the claim return in the surrounding claim method
to explicitly construct a BillableUsageClaim object with state, ownerId,
fencingToken, leaseExpiresAt, and the remaining required entry fields, rather
than returning copyEntry(candidate) with a type assertion. Preserve the existing
field values and remove reliance on the type assertion so missing claim fields
are caught by the compiler.
In `@packages/metering-core/src/libs/decorators/Metered.ts`:
- Around line 174-193: Update the validation around Metered.record so a cached
"unknown" result caused by an expired billing cache does not throw before the
original method executes, especially for billing: "local". Route expired-cache
cases through the existing asynchronous synchronization path used by
MeterRegistry.getOrThrow(), or restrict the unknown check to genuinely
cache-only lookups; preserve rejection for billable meters when the required
service or contract is unavailable.
In `@packages/metering-core/src/libs/MeteringService.ts`:
- Around line 135-151: Extract the duplicated billing-required condition from
recordUsage and persistUsage into a shared private helper method in
MeteringService. Have both flows call the helper using meter and billableMeter,
preserving the existing InvalidUsageEnvelopeProblem validation and persistence
behavior.
In `@packages/metering-core/src/libs/MeterRegistry.ts`:
- Around line 29-33: Update the MeterRegistry DI wiring so billableUsageJournal
is included when constructing MeterRegistry, rather than only passing
repository. Adjust the construction paths in the metering entrypoint and package
smoke setup, or route creation through the existing MeteringService factory,
ensuring billing configuration reflects whether the journal is actually injected
and record() does not require an unavailable persistent journal.
- Around line 91-94: Update the save flow around repository.save and addToCache
in MeterRegistry to validate the returned MeterDefinition with the same
journal-configuration check after saving and before caching or returning it;
preserve the existing validation of options.
In `@packages/metering-core/src/libs/RedisBillableUsageJournal.ts`:
- Around line 70-72: Update the APPEND_SCRIPT and terminal-transition logic in
RedisBillableUsageJournal so accepted and terminal-failed entries receive the
configured retention TTL via the entry key’s EXPIRE operation. Remove the unused
allKey index and its writes, or define equivalent cleanup/TTL behavior if it
must remain; ensure terminal indexes are also bounded according to the retention
policy.
- Around line 1-14: Update the imports in RedisBillableUsageJournal so the
BillableUsage symbols use a dedicated import type declaration, removing the
inline type modifiers; preserve the existing value imports separately.
- Around line 16-27: Update the StoredEntry type by adding deliverableAt to the
Omit key list, so its explicit string | number property overrides the original
Date type and accurately represents the Redis-stored format.
- Around line 63-73: Redis Lua의 append 저장 흐름에서 cjson.decode(ARGV[2])와
cjson.encode(entry)를 제거해 원본 event payload를 재직렬화하지 마십시오. client가 전달한 ARGV[2]를 별도
field 또는 key에 원문 그대로 저장하고, createdAt·createdAtEpochMs·updatedAt 같은 mutable 상태만
별도로 추가하거나 갱신하십시오. RedisBillableUsageJournal의 append 관련 Lua 반환값과 저장 구조가 원본 event
및 eventFingerprint를 그대로 보존하도록 조정하십시오.
In `@packages/metering-core/src/libs/types.ts`:
- Around line 22-24: Update MeterDefinition.aggregation in types.ts to use the
existing MeterAggregation type from MeterRef.ts instead of declaring the inline
"COUNT" | "SUM" union, preserving the current allowed values and eliminating the
duplicate definition.
In `@packages/metering-core/src/tests/BillableUsageJournal.spec.ts`:
- Around line 18-52: Update the InMemoryBillableUsageJournal tests to initialize
a shared journal in beforeEach instead of constructing it inside each it block.
In the conflicting-event test, validate the rejected Problem with
rejects.toThrow while preserving the existing code assertion for
"metering/transition-conflict".
In `@packages/metering-core/src/tests/MeteringService.spec.ts`:
- Around line 460-501: Extract the repeated
InMemoryBillableUsageJournal-to-persistent BillableUsageJournal wrapper into a
shared test helper, preserving all method bindings and overriding durability to
"persistent". Replace the duplicate wrapper construction in both the “should
retain a pending intent after local commit and process loss” test and the
corresponding test around the second occurrence, so future BillableUsageJournal
interface changes require updating only the helper.
In `@packages/metering-core/src/tests/MeterRegistry.spec.ts`:
- Around line 40-54: Extend the MeterRegistry tests near “should reject a
required billing meter without a persistent journal” to cover a
BillableUsageJournal with durability set to “volatile”. Assert that
registry.loadAll() rejects with code “metering/billable-usage-journal-required”,
preserving the existing no-journal and persistent-journal cases.
In `@packages/metering-core/src/tests/RedisBillableUsageJournal.spec.ts`:
- Around line 116-130: Update the RedisBillableUsageJournal diagnostics tests
around getDiagnostics to add coverage for an empty backlog where the Lua result
uses -1 and oldestPendingAgeMs resolves to null. Remove the script-text
assertion expect(script).not.toContain("0, -1"), while preserving the existing
diagnostics and Redis eval argument assertions.
- Around line 111-113: Update the ordering assertion in
RedisBillableUsageJournal.spec.ts to first verify that both SET and ZREM call
strings are present before comparing their positions. Keep the existing order
requirement, but ensure a missing or changed script fragment causes the test to
fail instead of allowing indexOf’s -1 result to pass.
In `@packages/metering-core/src/tests/RedisMetering.integration.spec.ts`:
- Around line 297-307: Replace the fixed 120 ms delay in the competing claim
test with short-interval retries that call journal.claimNext for worker-3 until
the expired lease is reclaimed, using a bounded timeout or retry limit. Preserve
the fencingToken assertion and ensure the retry failure clearly indicates that
the lease was not reclaimed.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7b7b52ee-433c-4968-89e8-276ee991c1dd
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (41)
.changeset/bright-badgers-journal.mddocs/problem-code-registry.jsonpackages/docs/src/content/docs/api/metering-core/src/classes/BillableUsageJournalRequiredProblem.mdpackages/docs/src/content/docs/api/metering-core/src/classes/InMemoryBillableUsageJournal.mdpackages/docs/src/content/docs/api/metering-core/src/classes/MeterRegistry.mdpackages/docs/src/content/docs/api/metering-core/src/classes/MeteringService.mdpackages/docs/src/content/docs/api/metering-core/src/classes/RedisBillableUsageJournal.mdpackages/docs/src/content/docs/api/metering-core/src/interfaces/BillableUsageJournal.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/BillableUsageAppendResult.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/BillableUsageClaim.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/BillableUsageClaimOptions.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/BillableUsageDeliveryState.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/BillableUsageEvent.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/BillableUsageFailure.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/BillableUsageJournalDiagnostics.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/BillableUsageJournalEntry.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/MeterDefinition.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/MeteredMetadata.mdpackages/docs/src/content/docs/api/metering-core/src/type-aliases/MeteredOptions.mdpackages/docs/src/content/docs/api/problems-core/src/classes/Problem.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/metering-core/README.mdpackages/metering-core/src/index.tspackages/metering-core/src/libs/BillableUsageJournal.tspackages/metering-core/src/libs/MeterRegistry.tspackages/metering-core/src/libs/MeteringService.tspackages/metering-core/src/libs/RedisBillableUsageJournal.tspackages/metering-core/src/libs/decorators/Metered.tspackages/metering-core/src/libs/problems/BillableUsageJournalRequiredProblem.tspackages/metering-core/src/libs/redisKey.tspackages/metering-core/src/libs/types.tspackages/metering-core/src/tests/BillableUsageJournal.spec.tspackages/metering-core/src/tests/MeterRegistry.spec.tspackages/metering-core/src/tests/MeteringService.spec.tspackages/metering-core/src/tests/RedisBillableUsageJournal.spec.tspackages/metering-core/src/tests/RedisMetering.integration.spec.tspackages/metering-core/src/tests/decorators/Metered.spec.tspackages/metering-core/src/tests/problems/Problems.spec.tspublic-api-surface.snapshot.jsonscripts/package-entrypoint-smoke.mts
kang-heewon
force-pushed
the
fix/1518-billable-usage-journal
branch
from
August 1, 2026 16:37
4864bca to
13df9bd
Compare
Member
Author
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
kang-heewon
force-pushed
the
fix/1518-billable-usage-journal
branch
from
August 1, 2026 17:19
13df9bd to
eda7498
Compare
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.
Summary
Billable usage is now journaled durably before local acceptance and becomes provider-deliverable only after the local usage or quota decision commits. Persistent Redis claims use server-time leases and fencing tokens so retries, worker loss, and concurrent delivery cannot silently duplicate accepted usage.
Meters declared with
billing: "required"now fail bootstrap or request execution with stable diagnostics when a persistent journal or caller-owned event identity is unavailable. Journal backlog, age, retry, and terminal-failure diagnostics are exposed through the metering service, while@Meteredremains free of provider calls.Verification
pnpm checkpnpm typecheckpnpm testpnpm lintpnpm docs:api:checkCROCO_TEST_REAL_RESOURCES=1 pnpm --dir packages/metering-core exec vitest run src/tests/RedisMetering.integration.spec.tspnpm changeset-required:check -- --base origin/trunk --head HEADFixes #1518
Summary by CodeRabbit
새 기능
오류 개선
문서