Skip to content

fix(entitlement): preserve grant usage across balance snapshots - #4846

Merged
GAlexIHU merged 7 commits into
mainfrom
codex/entitlement-balance-snapshots
Aug 14, 2026
Merged

fix(entitlement): preserve grant usage across balance snapshots#4846
GAlexIHU merged 7 commits into
mainfrom
codex/entitlement-balance-snapshots

Conversation

@GAlexIHU

@GAlexIHU GAlexIHU commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What changed

  • make balance snapshots carry complete usage-period state, including cumulative grant usage
  • anchor burn-down history to a complete starting snapshot and expose reset rollover as an explicit history segment
  • persist and select only resumable snapshots while retaining the legacy usage shape for rolling-deploy compatibility
  • fall back to measurement start when no compatible snapshot exists
  • add regression and continuity coverage for snapshot pruning, insertion, resets, and resumed calculations
  • document entitlement and credit domain semantics, including minute-level time resolution and snapshot lifecycle

Why

totalAvailableGrantAmount was reconstructed from grant usage after the latest persisted snapshot. Grant usage from earlier in the same usage period was therefore lost, allowing API responses where usage exceeded total available grants while balance remained positive and overage stayed zero.

A persisted snapshot must be a complete, independent checkpoint. Its result must not depend on earlier snapshots still being present.

Impact

Entitlement balance reads now preserve the full usage-period grant history when resuming from snapshots. Existing snapshots without the new usage state are ignored and recalculated from measurement start until new checkpoints are written. The legacy field remains written so old and new binaries can overlap during deployment.

Checks

  • full non-e2e Go test suite
  • repository lint
  • focused credit engine and metered entitlement regression tests
  • documentation diff and link checks

Deployment notes

  • One-time recompute: existing snapshots (without usage_snapshot) are ignored after deploy; each owner's first balance read replays from measurement start until a new complete checkpoint lands. Expect a temporary balance-worker load increase and verify ClickHouse retention still covers the oldest active entitlements before deploying.
  • Replay caveat: historical resets replay under the entitlement's current preserveOverageAtReset because per-call reset overrides are not persisted. If explicit overrides were used, recomputed balances may differ from the previously checkpointed values.
  • Rollback ordering: roll back binaries before applying the down migration. New binaries reference usage_snapshot in every snapshot read and write, so dropping the column first breaks entitlement balance APIs.

Summary by CodeRabbit

  • New Features

    • Added comprehensive documentation for credits and entitlements.
    • Improved balance snapshots with persisted usage details, cloning, and complete starting-state support.
    • Added accurate grant usage tracking across periods, resets, rollovers, and split calculations.
    • Preserved usage and unit configuration when resuming or migrating snapshots.
  • Bug Fixes

    • Balance totals now consistently include usage and grant consumption.
    • Snapshot retrieval and persistence now safely handle incomplete usage data.
    • Improved validation for invalid, overlapping, or misaligned snapshots and history.

Greptile Summary

This PR makes balance snapshots independent, resumable checkpoints carrying cumulative usage-period state.

  • Persists and selects the new complete usage snapshot while retaining the legacy representation for rolling deployments.
  • Reconstructs snapshot state from burn-down history, including explicit reset rollover segments.
  • Falls back to replaying from measurement start when no compatible checkpoint exists.
  • Adds continuity, reset, pruning, and resumed-calculation coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
openmeter/credit/adapter/balance_snapshot.go Selects only complete snapshots and persists both new and legacy usage representations for rolling-deployment compatibility.
openmeter/credit/balance/balance.go Adds complete starting snapshots, cumulative usage state, and deep snapshot cloning.
openmeter/credit/engine/history.go Anchors burn-down history to a complete snapshot and reconstructs resumable state at segment boundaries.
openmeter/credit/engine/reset.go Models rollover and preserved-overage consumption as an explicit history transition.
openmeter/credit/engine/run.go Validates complete starting state and carries cumulative usage and grant consumption through engine runs.
openmeter/credit/helper.go Falls back to measurement start when no complete compatible snapshot is available.
openmeter/entitlement/metered/balance.go Derives entitlement usage and total available grants from the complete engine snapshot.

Sequence Diagram

sequenceDiagram
  participant API as Entitlement balance read
  participant Repo as Snapshot repository
  participant Engine as Credit engine
  participant Usage as Metered usage
  API->>Repo: Get latest complete snapshot at query time
  alt Compatible snapshot exists
    Repo-->>API: Balances, overage, usage-period state
  else No compatible snapshot
    API->>API: Build zero snapshot at measurement start
  end
  API->>Engine: Run from selected starting snapshot
  Engine->>Usage: Query incremental usage
  Usage-->>Engine: Usage by burn phase
  Engine->>Engine: Burn grants and process resets
  Engine-->>API: Final snapshot and anchored history
  API->>Repo: Save eligible complete checkpoint
Loading

Reviews (8): Last reviewed commit: "fix(credit): preserve snapshot unit conf..." | Re-trigger Greptile

Context used:

@GAlexIHU GAlexIHU added the release-note/bug-fix Release note: Bug Fixes label Aug 3, 2026
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The credit engine now uses complete usage snapshots across balance creation, history, resets, persistence, and entitlement balance reads. Rollover transitions are explicit, and snapshot state remains independent across runs.

Changes

Credit snapshot contract and persistence

Layer / File(s) Summary
Snapshot contract and persistence
openmeter/credit/balance/*, openmeter/credit/adapter/*, openmeter/credit/balance/service.go, openmeter/credit/helper.go, openmeter/registry/builder/entitlement.go, test/...
Snapshot now carries UsageSnapshot and supports cloning. Persistence requires and stores complete usage snapshots while retaining legacy usage compatibility.
Domain guidance
AGENTS.md, openmeter/credit/README.md, openmeter/subscription/README.md
Credit-domain documentation and Go guidance were added. Subscription documentation was reformatted.

History and rollover transitions

Layer / File(s) Summary
History validation and reconstruction
openmeter/credit/engine/history.go, openmeter/credit/engine/history_test.go
History construction validates snapshot and segment continuity, accumulates grant usage, preserves unit configuration, and reconstructs usage snapshots.
Reset rollover segments
openmeter/credit/engine/reset.go, openmeter/credit/engine/reset_test.go
Resets return zero-duration rollover segments with balances, overage, grant usage, and rollover metadata.

Engine usage propagation

Layer / File(s) Summary
Run and result snapshots
openmeter/credit/engine/engine.go, openmeter/credit/engine/run.go, openmeter/credit/engine/*_test.go
Runs validate and clone starting snapshots, preserve usage across boundaries, append rollover segments, and accumulate usage totals.
Snapshot behavior validation
openmeter/credit/engine/snapshot_test.go
Tests cover invalid totals, rollover accumulation, split-run resumption, and unit configuration preservation.

Entitlement balance integration

Layer / File(s) Summary
Entitlement balance reads and persistence coverage
openmeter/entitlement/README.md, openmeter/entitlement/metered/*
Metered entitlement calculations read UsageSnapshot. Tests cover complete snapshots, migration selection, independent versions, stale snapshots, resets, and service wiring.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant EngineRun
  participant reset
  participant GrantBurnDownHistory
  participant SnapshotAdapter
  participant EntitlementBalance

  EngineRun->>reset: create reset snapshot and rollover segment
  reset-->>EngineRun: return rollover history
  EngineRun->>GrantBurnDownHistory: build history from starting snapshot
  GrantBurnDownHistory-->>EngineRun: return accumulated usage
  EngineRun->>SnapshotAdapter: persist complete usage snapshot
  SnapshotAdapter-->>EntitlementBalance: provide latest valid snapshot
  EntitlementBalance->>EntitlementBalance: calculate usage and available grant amount
Loading

Possibly related PRs

Suggested labels: kind/refactor

Suggested reviewers: chrisgacsal, turip

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving grant usage across entitlement balance snapshots.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/entitlement-balance-snapshots

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@GAlexIHU
GAlexIHU force-pushed the codex/entitlement-balance-snapshots branch from e365ec2 to 59058ea Compare August 3, 2026 16:46
@GAlexIHU
GAlexIHU marked this pull request as ready for review August 3, 2026 16:48
@GAlexIHU
GAlexIHU requested a review from a team as a code owner August 3, 2026 16:48

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
openmeter/credit/engine/history.go (1)

104-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the rollover-ordering comparator into a named helper.

The sort.SliceStable comparator at lines 123-129 encodes a real domain rule: rollover segments must sort before usage segments at the same timestamp. Move this into a named helper, for example segmentsLessForSort or rolloverPrecedesUsageAtSameTime. This keeps the domain rule visible outside the closure and makes it independently testable.

The rest of NewGrantBurnDownHistory (copy, per-segment ordering check, anchor validation, contiguity validation) is correct and matches the test coverage in history_test.go.

Based on learnings, AGENTS.md states: "Do not hide type switching, validation, persistence mapping, or meaningful domain translation inside local closures. Use a named helper; reserve inline callbacks for obvious, tiny logic."

♻️ Proposed extraction
-	sort.SliceStable(s, func(i, j int) bool {
-		if s[i].ClosedPeriod.From.Equal(s[j].ClosedPeriod.From) {
-			return s[i].TerminationReasons.Rollover && !s[j].TerminationReasons.Rollover
-		}
-
-		return s[i].ClosedPeriod.From.Before(s[j].ClosedPeriod.From)
-	})
+	sort.SliceStable(s, func(i, j int) bool {
+		return segmentLess(s[i], s[j])
+	})
+}
+
+// segmentLess orders segments chronologically. Rollover transitions precede
+// regular segments at the same timestamp so the reset transition is applied
+// before new-period usage.
+func segmentLess(a, b GrantBurnDownHistorySegment) bool {
+	if a.ClosedPeriod.From.Equal(b.ClosedPeriod.From) {
+		return a.TerminationReasons.Rollover && !b.TerminationReasons.Rollover
+	}
+
+	return a.ClosedPeriod.From.Before(b.ClosedPeriod.From)
🤖 Prompt for 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.

In `@openmeter/credit/engine/history.go` around lines 104 - 165, Extract the
domain-specific comparator from sort.SliceStable in NewGrantBurnDownHistory into
a named helper such as segmentsLessForSort, preserving chronological ordering
and ensuring rollover segments precede non-rollover segments at equal
timestamps. Pass the helper to sort.SliceStable so the ordering rule is
independently testable.

Source: Coding guidelines

openmeter/credit/balance/balance_test.go (1)

93-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extend TestSnapshotCloneCopiesUsageSnapshot to cover Balances and UnitConfig independence.

This test only checks that mutating the clone's UsageSnapshot does not affect the original. Snapshot.Clone() also clones Balances and UnitConfig independently. Add assertions for those two fields so a future regression in either clone path is caught here.

♻️ Suggested extension
 func TestSnapshotCloneCopiesUsageSnapshot(t *testing.T) {
 	snapshot := balance.Snapshot{
+		Balances: balance.Map{"grant-1": 100},
 		UsageSnapshot: &balance.UsageSnapshot{
 			Usage:           5,
 			TotalGrantUsage: 10,
 		},
 	}

 	cloned := snapshot.Clone()
+	cloned.Balances["grant-1"] = 999
+	assert.Equal(t, 100.0, snapshot.Balances["grant-1"])
 	require.NotNil(t, cloned.UsageSnapshot)
 	...

As per path instructions, "Make sure the tests are comprehensive and cover the changes," and Clone() is new behavior introduced by this PR without full field coverage.

🤖 Prompt for 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.

In `@openmeter/credit/balance/balance_test.go` around lines 93 - 114, Extend
TestSnapshotCloneCopiesUsageSnapshot to initialize representative Balances and
UnitConfig values, then assert the cloned fields match the original values and
are independent. Mutate the clone’s Balances and UnitConfig after cloning and
verify the corresponding fields on the original snapshot remain unchanged,
alongside the existing UsageSnapshot assertions.

Source: Path instructions

🤖 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 `@openmeter/credit/engine/reset.go`:
- Around line 51-86: Clone rolledOver before passing it to burnDownGrants in the
reset flow so in-place mutations cannot alter the original rollover balances.
Keep balances initialized from rolledOver for the reset snapshot, but pass a
cloned map as burnDownGrants’ balance input; preserve BalanceAtStart as the
pre-burn rolled-over state.

In `@openmeter/credit/engine/run.go`:
- Around line 251-277: Preserve the conversion configuration when constructing
the snapshot returned by runBetweenResets: copy
params.StartingSnapshot.UnitConfig into the new balance.Snapshot alongside the
existing fields. Also update reset() to copy UnitConfig into its returned reset
snapshot so subsequent engine segments retain the same configuration.

In `@openmeter/entitlement/README.md`:
- Around line 27-28: Update the sentence in the entitlement documentation so the
singular subject “Entitlement” uses the possessive pronoun “its” instead of
“their,” leaving the rest of the wording unchanged.

---

Nitpick comments:
In `@openmeter/credit/balance/balance_test.go`:
- Around line 93-114: Extend TestSnapshotCloneCopiesUsageSnapshot to initialize
representative Balances and UnitConfig values, then assert the cloned fields
match the original values and are independent. Mutate the clone’s Balances and
UnitConfig after cloning and verify the corresponding fields on the original
snapshot remain unchanged, alongside the existing UsageSnapshot assertions.

In `@openmeter/credit/engine/history.go`:
- Around line 104-165: Extract the domain-specific comparator from
sort.SliceStable in NewGrantBurnDownHistory into a named helper such as
segmentsLessForSort, preserving chronological ordering and ensuring rollover
segments precede non-rollover segments at equal timestamps. Pass the helper to
sort.SliceStable so the ordering rule is independently testable.
🪄 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: CHILL

Plan: Pro Plus

Run ID: d267b431-eec1-4a79-a976-00385739403a

📥 Commits

Reviewing files that changed from the base of the PR and between c348100 and 59058ea.

⛔ Files ignored due to path filters (9)
  • openmeter/ent/db/balancesnapshot.go is excluded by !**/ent/db/**
  • openmeter/ent/db/balancesnapshot/balancesnapshot.go is excluded by !**/ent/db/**
  • openmeter/ent/db/balancesnapshot/where.go is excluded by !**/ent/db/**
  • openmeter/ent/db/balancesnapshot_create.go is excluded by !**/ent/db/**
  • openmeter/ent/db/balancesnapshot_update.go is excluded by !**/ent/db/**
  • openmeter/ent/db/migrate/schema.go is excluded by !**/ent/db/**
  • openmeter/ent/db/mutation.go is excluded by !**/ent/db/**
  • openmeter/ent/db/runtime.go is excluded by !**/ent/db/**
  • tools/migrate/migrations/atlas.sum is excluded by !**/*.sum, !**/*.sum
📒 Files selected for processing (34)
  • AGENTS.md
  • openmeter/credit/README.md
  • openmeter/credit/adapter/balance_snapshot.go
  • openmeter/credit/balance/balance.go
  • openmeter/credit/balance/balance_test.go
  • openmeter/credit/balance/repository.go
  • openmeter/credit/balance/service.go
  • openmeter/credit/balance/service_test.go
  • openmeter/credit/engine/engine.go
  • openmeter/credit/engine/engine_test.go
  • openmeter/credit/engine/history.go
  • openmeter/credit/engine/history_test.go
  • openmeter/credit/engine/reset.go
  • openmeter/credit/engine/reset_test.go
  • openmeter/credit/engine/run.go
  • openmeter/credit/engine/run_test.go
  • openmeter/credit/engine/runresult_test.go
  • openmeter/credit/engine/snapshot_test.go
  • openmeter/credit/helper.go
  • openmeter/ent/schema/balance_snapshot.go
  • openmeter/entitlement/README.md
  • openmeter/entitlement/metered/balance.go
  • openmeter/entitlement/metered/balance_test.go
  • openmeter/entitlement/metered/balance_total_available_test.go
  • openmeter/entitlement/metered/balance_unitconfig_test.go
  • openmeter/entitlement/metered/lateevents_test.go
  • openmeter/entitlement/metered/reset_test.go
  • openmeter/entitlement/metered/utils_test.go
  • openmeter/registry/builder/entitlement.go
  • openmeter/subscription/README.md
  • test/billing/subscription_suite.go
  • test/entitlement/regression/framework_test.go
  • tools/migrate/migrations/20260803135655_add_balance_snapshot_usage_snapshot.down.sql
  • tools/migrate/migrations/20260803135655_add_balance_snapshot_usage_snapshot.up.sql
💤 Files with no reviewable changes (1)
  • openmeter/credit/balance/service_test.go

Comment thread openmeter/credit/engine/reset.go
Comment thread openmeter/credit/engine/run.go
Comment on lines +27 to +28
that materializes and supersedes subscription-managed entitlements.
Entitlement owns their persisted lifecycle and value resolution.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the pronoun-agreement slip.

"Entitlement" is singular, so "their" should be "its" in this sentence.

✏️ Proposed fix
-Entitlement owns their persisted lifecycle and value resolution.
+Entitlement owns its persisted lifecycle and value resolution.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
that materializes and supersedes subscription-managed entitlements.
Entitlement owns their persisted lifecycle and value resolution.
that materializes and supersedes subscription-managed entitlements.
Entitlement owns its persisted lifecycle and value resolution.
🤖 Prompt for 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.

In `@openmeter/entitlement/README.md` around lines 27 - 28, Update the sentence in
the entitlement documentation so the singular subject “Entitlement” uses the
possessive pronoun “its” instead of “their,” leaving the rest of the wording
unchanged.

Source: Path instructions

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
openmeter/credit/engine/snapshot_test.go (1)

327-353: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add lifecycle intent comments to the subtest.

Add concise given, when, and then comments. This test covers both normal execution and reset rollover behavior.

As per coding guidelines, “Begin non-trivial service or lifecycle subtests with concise given, when, and then intent comments.”

🤖 Prompt for 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.

In `@openmeter/credit/engine/snapshot_test.go` around lines 327 - 353, Add concise
given, when, and then intent comments inside the subtest around the starting
snapshot and engine setup, the eng.Run invocation, and the snapshot unit-config
assertions. Ensure the comments describe both normal execution and reset
rollover behavior without changing test logic.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@openmeter/credit/engine/snapshot_test.go`:
- Around line 327-353: Add concise given, when, and then intent comments inside
the subtest around the starting snapshot and engine setup, the eng.Run
invocation, and the snapshot unit-config assertions. Ensure the comments
describe both normal execution and reset rollover behavior without changing test
logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 03dad9eb-620b-4498-8022-f16ffc861c2b

📥 Commits

Reviewing files that changed from the base of the PR and between 59058ea and 83dc569.

📒 Files selected for processing (3)
  • openmeter/credit/engine/reset.go
  • openmeter/credit/engine/run.go
  • openmeter/credit/engine/snapshot_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • openmeter/credit/engine/reset.go
  • openmeter/credit/engine/run.go

chrisgacsal
chrisgacsal previously approved these changes Aug 4, 2026
@GAlexIHU
GAlexIHU force-pushed the codex/entitlement-balance-snapshots branch from 83dc569 to 67823cb Compare August 4, 2026 09:23
@GAlexIHU
GAlexIHU changed the base branch from main to codex/usage-snapshot-schema August 4, 2026 09:24

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
openmeter/credit/engine/history.go (1)

60-71: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

Consider validating UsageSnapshot.Usage too.

validateStartingSnapshot guards TotalGrantUsage against NaN, Inf, and negative values. UsageSnapshot.Usage gets no such guard, and getUsageSnapshotAtStartOfSegment accumulates it into every reconstructed segment snapshot at line 207. A NaN or Inf there silently poisons every downstream usage figure the same way. Same check, same place, cheap to add.

♻️ Optional: extend the finiteness check
+	usage := snapshot.UsageSnapshot.Usage
+	if usage < 0 || math.IsNaN(usage) || math.IsInf(usage, 0) {
+		return fmt.Errorf("starting snapshot usage must be a finite non-negative number, got %v", usage)
+	}
+
 	totalGrantUsage := snapshot.UsageSnapshot.TotalGrantUsage
 	if totalGrantUsage < 0 || math.IsNaN(totalGrantUsage) || math.IsInf(totalGrantUsage, 0) {
 		return fmt.Errorf("starting snapshot total grant usage must be a finite non-negative number, got %v", totalGrantUsage)
 	}
🤖 Prompt for 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.

In `@openmeter/credit/engine/history.go` around lines 60 - 71, The
validateStartingSnapshot function validates
snapshot.UsageSnapshot.TotalGrantUsage for NaN, Inf, and negative values, but
does not validate snapshot.UsageSnapshot.Usage with the same checks. Since Usage
is accumulated into reconstructed segment snapshots downstream, an invalid Usage
value (NaN or Inf) can poison all downstream usage figures. Add the same
finiteness and non-negative validation check for UsageSnapshot.Usage in the
validateStartingSnapshot function that already exists for TotalGrantUsage, using
the same error-reporting pattern to maintain consistency.
🤖 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.

Nitpick comments:
In `@openmeter/credit/engine/history.go`:
- Around line 60-71: The validateStartingSnapshot function validates
snapshot.UsageSnapshot.TotalGrantUsage for NaN, Inf, and negative values, but
does not validate snapshot.UsageSnapshot.Usage with the same checks. Since Usage
is accumulated into reconstructed segment snapshots downstream, an invalid Usage
value (NaN or Inf) can poison all downstream usage figures. Add the same
finiteness and non-negative validation check for UsageSnapshot.Usage in the
validateStartingSnapshot function that already exists for TotalGrantUsage, using
the same error-reporting pattern to maintain consistency.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c1ed8c37-c563-482a-af8a-dadbc62f70dc

📥 Commits

Reviewing files that changed from the base of the PR and between 83dc569 and 67823cb.

⛔ Files ignored due to path filters (9)
  • openmeter/ent/db/balancesnapshot.go is excluded by !**/ent/db/**
  • openmeter/ent/db/balancesnapshot/balancesnapshot.go is excluded by !**/ent/db/**
  • openmeter/ent/db/balancesnapshot/where.go is excluded by !**/ent/db/**
  • openmeter/ent/db/balancesnapshot_create.go is excluded by !**/ent/db/**
  • openmeter/ent/db/balancesnapshot_update.go is excluded by !**/ent/db/**
  • openmeter/ent/db/migrate/schema.go is excluded by !**/ent/db/**
  • openmeter/ent/db/mutation.go is excluded by !**/ent/db/**
  • openmeter/ent/db/runtime.go is excluded by !**/ent/db/**
  • tools/migrate/migrations/atlas.sum is excluded by !**/*.sum, !**/*.sum
📒 Files selected for processing (34)
  • AGENTS.md
  • openmeter/credit/README.md
  • openmeter/credit/adapter/balance_snapshot.go
  • openmeter/credit/balance/balance.go
  • openmeter/credit/balance/balance_test.go
  • openmeter/credit/balance/repository.go
  • openmeter/credit/balance/service.go
  • openmeter/credit/balance/service_test.go
  • openmeter/credit/engine/engine.go
  • openmeter/credit/engine/engine_test.go
  • openmeter/credit/engine/history.go
  • openmeter/credit/engine/history_test.go
  • openmeter/credit/engine/reset.go
  • openmeter/credit/engine/reset_test.go
  • openmeter/credit/engine/run.go
  • openmeter/credit/engine/run_test.go
  • openmeter/credit/engine/runresult_test.go
  • openmeter/credit/engine/snapshot_test.go
  • openmeter/credit/helper.go
  • openmeter/ent/schema/balance_snapshot.go
  • openmeter/entitlement/README.md
  • openmeter/entitlement/metered/balance.go
  • openmeter/entitlement/metered/balance_test.go
  • openmeter/entitlement/metered/balance_total_available_test.go
  • openmeter/entitlement/metered/balance_unitconfig_test.go
  • openmeter/entitlement/metered/lateevents_test.go
  • openmeter/entitlement/metered/reset_test.go
  • openmeter/entitlement/metered/utils_test.go
  • openmeter/registry/builder/entitlement.go
  • openmeter/subscription/README.md
  • test/billing/subscription_suite.go
  • test/entitlement/regression/framework_test.go
  • tools/migrate/migrations/20260803135655_add_balance_snapshot_usage_snapshot.down.sql
  • tools/migrate/migrations/20260803135655_add_balance_snapshot_usage_snapshot.up.sql
💤 Files with no reviewable changes (1)
  • openmeter/credit/balance/service_test.go
🚧 Files skipped from review as they are similar to previous changes (31)
  • AGENTS.md
  • openmeter/subscription/README.md
  • openmeter/credit/engine/engine_test.go
  • openmeter/credit/helper.go
  • openmeter/entitlement/metered/balance_unitconfig_test.go
  • test/billing/subscription_suite.go
  • openmeter/entitlement/metered/balance.go
  • openmeter/credit/balance/repository.go
  • openmeter/credit/README.md
  • test/entitlement/regression/framework_test.go
  • openmeter/credit/engine/engine.go
  • openmeter/credit/adapter/balance_snapshot.go
  • openmeter/entitlement/metered/utils_test.go
  • openmeter/ent/schema/balance_snapshot.go
  • openmeter/credit/balance/service.go
  • openmeter/credit/balance/balance.go
  • openmeter/credit/balance/balance_test.go
  • openmeter/entitlement/README.md
  • openmeter/entitlement/metered/reset_test.go
  • openmeter/entitlement/metered/balance_total_available_test.go
  • openmeter/entitlement/metered/balance_test.go
  • openmeter/credit/engine/snapshot_test.go
  • openmeter/credit/engine/run.go
  • openmeter/credit/engine/history_test.go
  • openmeter/credit/engine/reset_test.go
  • tools/migrate/migrations/20260803135655_add_balance_snapshot_usage_snapshot.up.sql
  • openmeter/entitlement/metered/lateevents_test.go
  • openmeter/credit/engine/reset.go
  • openmeter/credit/engine/run_test.go
  • openmeter/credit/engine/runresult_test.go
  • openmeter/registry/builder/entitlement.go

Base automatically changed from codex/usage-snapshot-schema to main August 4, 2026 09:34
@GAlexIHU
GAlexIHU dismissed chrisgacsal’s stale review August 4, 2026 09:34

The base branch was changed.

@GAlexIHU
GAlexIHU requested a review from chrisgacsal August 4, 2026 09:36
@GAlexIHU
GAlexIHU force-pushed the codex/entitlement-balance-snapshots branch from 2800784 to a8c85f9 Compare August 10, 2026 08:19
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

chrisgacsal
chrisgacsal previously approved these changes Aug 11, 2026

@chrisgacsal chrisgacsal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! :shipit:

@GAlexIHU
GAlexIHU force-pushed the codex/entitlement-balance-snapshots branch from a8c85f9 to 821a2c1 Compare August 14, 2026 10:51
@GAlexIHU
GAlexIHU force-pushed the codex/entitlement-balance-snapshots branch from 821a2c1 to 2a1fbfc Compare August 14, 2026 10:59
@GAlexIHU
GAlexIHU merged commit 842c5fd into main Aug 14, 2026
27 checks passed
@GAlexIHU
GAlexIHU deleted the codex/entitlement-balance-snapshots branch August 14, 2026 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/bug-fix Release note: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants