Repository navigation
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openmeterio/openmeter/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThis change adds ChangesCredit correction transactions
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant ListCreditTransactions
participant correctionCreditTransactionLoader
participant Ledger
participant correctionCreditTransactions
ListCreditTransactions->>correctionCreditTransactionLoader: load correction rows
correctionCreditTransactionLoader->>Ledger: list correction ledger transactions
correctionCreditTransactionLoader->>correctionCreditTransactions: project transaction groups
correctionCreditTransactions-->>correctionCreditTransactionLoader: return balance-impact rows
correctionCreditTransactionLoader-->>ListCreditTransactions: return paginated rows
Suggested reviewers: Merge Risk: ⚪ Minimal · up to Credit history can now show correction activity when a subscription edit or ledger correction refunds earlier usage. The latest changes add test coverage for a prorated subscription edit and set up the test service to support it. No concrete defect was found, so the change looks ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
| func (l *correctionCreditTransactionLoader) Load(ctx context.Context, input creditTransactionLoaderInput) (creditTransactionLoaderResult, error) { | ||
| items := make([]CreditTransaction, 0, input.Limit+1) | ||
| after, before := input.After, input.Before | ||
| resolved := make(map[string][]CreditTransaction) |
There was a problem hiding this comment.
Unbounded correction group cache When a customer has a long correction history, this cache retains every scanned group until the request ends, even groups with no visible balance impact. A request for a small page can therefore use memory proportional to the customer's full correction history. Please bound or evict cached groups as scanning advances.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openmeter/ledger/customerbalance/correction_loader.go
Line: 26
Comment:
**Unbounded correction group cache** When a customer has a long correction history, this cache retains every scanned group until the request ends, even groups with no visible balance impact. A request for a small page can therefore use memory proportional to the customer's full correction history. Please bound or evict cached groups as scanning advances.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| require.NoError(t, err) | ||
| versions := edited.Phases[0].ItemsByKey[rateCard.Key()] | ||
| require.Len(t, versions, 2) | ||
| require.NotEqual(t, originalItem.ID, versions[0].SubscriptionItem.ID, "the edit must exercise recreated subscription items") |
There was a problem hiding this comment.
Item version assertion fails After the edit, items are sorted by start time, so
versions[0] is still the original item. This assertion compares the original ID with itself and fails before the test reaches its correction-history checks. Compare it with versions[1] instead.
| require.NotEqual(t, originalItem.ID, versions[0].SubscriptionItem.ID, "the edit must exercise recreated subscription items") | |
| require.NotEqual(t, originalItem.ID, versions[1].SubscriptionItem.ID, "the edit must exercise recreated subscription items") |
Prompt To Fix With AI
This is a comment left during a code review.
Path: test/subscription/credit_history_test.go
Line: 157
Comment:
**Item version assertion fails** After the edit, items are sorted by start time, so `versions[0]` is still the original item. This assertion compares the original ID with itself and fails before the test reaches its correction-history checks. Compare it with `versions[1]` instead.
```suggestion
require.NotEqual(t, originalItem.ID, versions[1].SubscriptionItem.ID, "the edit must exercise recreated subscription items")
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Subscription edits can reverse credit usage in the ledger while credit transaction history still shows only the original consumption. Add
correctionactivity for both legacy and origin-tracked corrections, preserving booking times, creation times, charge labels, currency identity, and balance-before/after values.Combine each correction batch's customer balance effects by booked time and currency so advance and backfill reversals do not appear as extra consumption. Exclude correction legs from consumed rows and fix backward pagination to select the nearest activities across types. No links to original usage or funding rows are exposed.
Validation:
test/subscription: create an inline custom subscription with one monthly flat fee, check history, replace the item halfway through the month, and verify proration corrections, recreated item metadata, timestamps, and the final balance. Full subscription suite and focused lint pass.make generate. Fullmake gen-apiwas blocked by a missing legacy Python emitter interpreter; the v3 generators were run directly.Summary by CodeRabbit
The PR is not ready to merge because the new subscription integration test fails at its item-version assertion.
Fix with agent prompt
Summary
The PR adds correction movements to customer credit history, separates them from consumption, updates pagination, and synchronizes the API and SDK transaction types. The latest changes add a subscription integration test and its service wiring.
Reviews (2) · Last reviewed commit: "test(subscription): cover prorated credi..."