feat(credit): add usage snapshot schema - #4850
Conversation
📝 WalkthroughWalkthroughThe change adds an optional ChangesUsage snapshot persistence
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openmeter/ent/schema/balance_snapshot.go (1)
40-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a lifecycle comment on
usage_snapshotto match its sibling field.The
usagefield comment (Lines 35-36) explains that it is the legacy representation during the rolling migration. The newusage_snapshotfield is the destination of that migration, but it has no comment stating this role, unlikeusageandunit_config. A short comment here would help future readers understand why the field is nullable now and what fills it in later.📝 Suggested comment addition
+ // usage_snapshot stores the complete usage-period snapshot that will + // replace the legacy `usage` field once the rolling migration + // completes. Nil until populated by the resume/warmup path. field.JSON("usage_snapshot", &balance.UsageSnapshot{}).Immutable().Optional().SchemaType(map[string]string{ dialect.Postgres: "jsonb", }),As per coding guidelines, "Comments and docstrings should explain non-obvious intent, domain constraints, lifecycle state, and failure consequences."
🤖 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/ent/schema/balance_snapshot.go` around lines 40 - 42, Add a concise lifecycle comment immediately before the usage_snapshot field declaration, explaining that it is the destination of the rolling migration from the legacy usage representation, is nullable during migration, and will be populated later. Keep the field definition and its existing schema configuration unchanged.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/ent/schema/balance_snapshot.go`:
- Around line 40-42: Add a concise lifecycle comment immediately before the
usage_snapshot field declaration, explaining that it is the destination of the
rolling migration from the legacy usage representation, is nullable during
migration, and will be populated later. Keep the field definition and its
existing schema configuration unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a856668-8051-45b7-b28c-1ad4a0e151ee
⛔ Files ignored due to path filters (9)
openmeter/ent/db/balancesnapshot.gois excluded by!**/ent/db/**openmeter/ent/db/balancesnapshot/balancesnapshot.gois excluded by!**/ent/db/**openmeter/ent/db/balancesnapshot/where.gois excluded by!**/ent/db/**openmeter/ent/db/balancesnapshot_create.gois excluded by!**/ent/db/**openmeter/ent/db/balancesnapshot_update.gois excluded by!**/ent/db/**openmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**openmeter/ent/db/mutation.gois excluded by!**/ent/db/**openmeter/ent/db/runtime.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (4)
openmeter/credit/balance/balance.goopenmeter/ent/schema/balance_snapshot.gotools/migrate/migrations/20260803135655_add_balance_snapshot_usage_snapshot.down.sqltools/migrate/migrations/20260803135655_add_balance_snapshot_usage_snapshot.up.sql
What
usage_snapshotJSONB field to balance snapshotsWhy
This is the schema-first half of the entitlement balance snapshot fix in #4846. Landing and deploying the additive persistence shape first keeps the follow-up runtime change zero-downtime and leaves room for a snapshot warmup job if needed.
Impact
Additive and inert: this PR does not read, write, or select the new field at runtime.
Validation
git diff --checkSummary by CodeRabbit
New Features
Migration
Greptile Summary
This PR introduces the persistence shape needed for complete usage-period snapshots while leaving runtime behavior unchanged.
UsageSnapshotdomain type and an optional pointer on balance snapshots.usage_snapshotJSONB field to the Ent schema and generated client.Confidence Score: 5/5
The PR appears safe to merge as an additive schema-first change with no active runtime behavior.
The nullable column, generated Ent artifacts, positional schema references, and migration are internally consistent, while existing rows remain compatible through a nil usage snapshot.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR Domain[balance.UsageSnapshot] --> Ent[Ent usage_snapshot field] Ent --> DB[(balance_snapshots.usage_snapshot JSONB)] Runtime[Follow-up runtime integration] -. deferred .-> DomainReviews (1): Last reviewed commit: "feat(credit): add usage snapshot schema" | Re-trigger Greptile
Context used: