feat: grant voiding#4653
Merged
Merged
Conversation
GAlexIHU
added
the
release-note/ignore
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds end-to-end void-credit-grant support through API contracts and clients, v3 routing, transactional billing and ledger services, persisted void records, customer-balance reporting, dependency wiring, migrations, and integration coverage for lifecycle, payment, concurrency, and idempotency behavior. ChangesVoid Credit Grant
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant VoidCreditGrantHandler
participant CreditGrantService
participant CreditVoidService
participant CreditPurchaseService
participant CustomerBalance
Client->>VoidCreditGrantHandler: POST /customers/{customerId}/credits/grants/{creditGrantId}/void
VoidCreditGrantHandler->>CreditGrantService: Void(VoidInput)
CreditGrantService->>CreditVoidService: VoidCreditPurchase
CreditVoidService->>CreditPurchaseService: MarkVoided
CreditGrantService-->>VoidCreditGrantHandler: updated BillingCreditGrant
VoidCreditGrantHandler-->>Client: 200 JSON response
Client->>CustomerBalance: list credit transactions
CustomerBalance->>CreditVoidService: ListVoidedCreditImpacts
CreditVoidService-->>CustomerBalance: voided impacts
CustomerBalance-->>Client: voided transactions
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
Contributor
Greptile SummaryThis PR adds credit grant voiding across the API and billing stack. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "fix: breakage tx listing status mapping ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+8659
to
+8661
| // Void credit grant | ||
| // (POST /openmeter/customers/{customerId}/credits/grants/{creditGrantId}/void) | ||
| VoidCreditGrant(w http.ResponseWriter, r *http.Request, customerId ULID, creditGrantId ULID) |
Contributor
There was a problem hiding this comment.
Adding VoidCreditGrant to api.ServerInterface makes every existing implementation provide this method. The v3 server implements it, but openmeter/server/router/router.go still asserts *Router implements the same interface and has no matching VoidCreditGrant method, so repository builds that include that package fail with a missing-method compile error.
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/v3/api.gen.go
Line: 8659-8661
Comment:
**Server Interface Missing Method**
Adding `VoidCreditGrant` to `api.ServerInterface` makes every existing implementation provide this method. The v3 server implements it, but `openmeter/server/router/router.go` still asserts `*Router` implements the same interface and has no matching `VoidCreditGrant` method, so repository builds that include that package fail with a missing-method compile error.
How can I resolve this? If you propose a fix, please make it concise.
GAlexIHU
force-pushed
the
feat/grant-voiding
branch
2 times, most recently
from
July 8, 2026 15:20
7d3419a to
5fffa68
Compare
GAlexIHU
marked this pull request as ready for review
July 9, 2026 09:42
GAlexIHU
force-pushed
the
feat/grant-voiding
branch
from
July 9, 2026 09:59
b9dd7c7 to
6ae1fbb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
api/spec/packages/aip/src/customers/credits/operations.tsp (1)
93-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
VoidCreditGrantRequestoutside thenamespace Credits.Grantsblock.All other request models in this file (
UpdateCreditGrantExternalSettlementRequestat line 142,ListCreditGrantsParamsFilterat line 21, etc.) are declared in the parentCustomersnamespace, not insideCredits.Grants. The coding guideline says to wrap only the*Operationsinterface in the sub-namespace and leave models in the parent namespace. While@friendlyNamecontrols the OpenAPI schema name, the TypeSpec fully qualified name (Customers.Credits.Grants.VoidCreditGrantRequestvsCustomers.VoidCreditGrantRequest) can affect other code generation toolchains.♻️ Move model to parent namespace
namespace Credits.Grants { - /** - * Request body for voiding a credit grant. - */ - `@friendlyName`("VoidCreditGrantRequest") - model VoidCreditGrantRequest { - /** - * How voiding adjusts payment state related to the grant. - * - * Currently only `none` is supported: voiding does not adjust invoices, payment - * authorization, settlement, payment intents, or external collection state. If - * payment later completes, the original invoiced amount may still be collected. - */ - payment_adjustment?: CreditGrantVoidPaymentAdjustment = CreditGrantVoidPaymentAdjustment.None; - } - interface CustomerCreditGrantVoidOperations { /** * Void a credit grant, forfeiting the remaining unused balance. @@ -134,4 +134,17 @@ } } + +/** + * Request body for voiding a credit grant. + */ +@friendlyName("VoidCreditGrantRequest") +model VoidCreditGrantRequest { + /** + * How voiding adjusts payment state related to the grant. + * + * Currently only `none` is supported: voiding does not adjust invoices, payment + * authorization, settlement, payment intents, or external collection state. If + * payment later completes, the original invoiced amount may still be collected. + */ + payment_adjustment?: CreditGrantVoidPaymentAdjustment = CreditGrantVoidPaymentAdjustment.None; +}🤖 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 `@api/spec/packages/aip/src/customers/credits/operations.tsp` around lines 93 - 107, Move VoidCreditGrantRequest out of the Credits.Grants namespace and declare it in the parent Customers namespace alongside the other request models. Keep the Credits.Grants namespace limited to the operations interface, and preserve the `@friendlyName` on VoidCreditGrantRequest so the OpenAPI name stays the same while the fully qualified TypeSpec name changes to Customers.VoidCreditGrantRequest.Source: Coding guidelines
openmeter/ledger/breakage/breakage_impacts_test.go (1)
66-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
SourceKindassertions to the test.The test sets up records with mixed
SourceKindvalues (SourceKindCreditPurchaseandSourceKindUsage) but never asserts on the outputSourceKind. SinceSourceKindis a new field introduced in this PR, the test should verify it's propagated correctly — especially given the mixed-source scenario.🧪 Suggested assertions
require.Equal(t, float64(-7), amountByChargeID[chargeA]) require.Equal(t, float64(-7), amountByChargeID[chargeB]) +sourceKindByChargeID := make(map[string]SourceKind, len(got.Items)) +for _, item := range got.Items { + chargeID, ok := item.Annotations[ledger.AnnotationChargeID].(string) + require.True(t, ok) + sourceKindByChargeID[chargeID] = item.SourceKind +} + +// Assert the expected SourceKind for each charge group +require.Equal(t, SourceKindCreditPurchase, sourceKindByChargeID[chargeA]) +require.Equal(t, SourceKindCreditPurchase, sourceKindByChargeID[chargeB])🤖 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/ledger/breakage/breakage_impacts_test.go` around lines 66 - 85, The breakage impacts test is missing assertions for the new SourceKind field, so it does not verify propagation for the mixed-source setup. Update breakage_impacts_test.go in the ListExpiredBreakageImpacts test to assert each returned item’s SourceKind alongside the existing collection type and charge ID checks, using the same items loop after calling svc.ListExpiredBreakageImpacts and before the amount assertions. Ensure the test covers both SourceKindCreditPurchase and SourceKindUsage expectations for the corresponding charge records.
🤖 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 `@api/v3/handlers/customers/credits/void_grant_test.go`:
- Around line 66-71: Replace the use of context.Background() in
void_grant_test.go with t.Context() in the test that calls
apierrors.GenericErrorEncoder(), so the test uses the testing context
consistently. Keep the rest of the assertions unchanged and update the local
setup around rec, req, and handled to pass t.Context() wherever the context is
currently created manually.
In `@openmeter/ledger/customerbalance/ledger_loader.go`:
- Around line 23-79: The pagination logic in credit transaction loading is
leaving stale state and over-returning items. In ledger_loader.go within the
loop that calls l.service.Ledger.ListTransactions, reset hasMore based on the
current page instead of retaining a previous true value, and ensure the returned
txs slice is trimmed to input.Limit before calling
creditTransactionsFromLedgerTransactions. Update the
creditTransactionLoaderResult assembly so HasMore reflects the final page state
and Items never exceed the requested limit.
---
Nitpick comments:
In `@api/spec/packages/aip/src/customers/credits/operations.tsp`:
- Around line 93-107: Move VoidCreditGrantRequest out of the Credits.Grants
namespace and declare it in the parent Customers namespace alongside the other
request models. Keep the Credits.Grants namespace limited to the operations
interface, and preserve the `@friendlyName` on VoidCreditGrantRequest so the
OpenAPI name stays the same while the fully qualified TypeSpec name changes to
Customers.VoidCreditGrantRequest.
In `@openmeter/ledger/breakage/breakage_impacts_test.go`:
- Around line 66-85: The breakage impacts test is missing assertions for the new
SourceKind field, so it does not verify propagation for the mixed-source setup.
Update breakage_impacts_test.go in the ListExpiredBreakageImpacts test to assert
each returned item’s SourceKind alongside the existing collection type and
charge ID checks, using the same items loop after calling
svc.ListExpiredBreakageImpacts and before the amount assertions. Ensure the test
covers both SourceKindCreditPurchase and SourceKindUsage expectations for the
corresponding charge records.
🪄 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
Run ID: 90c7f7f0-e131-4f51-81d6-02e6874490c8
⛔ Files ignored due to path filters (27)
api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/chargecreditpurchase.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase/chargecreditpurchase.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase/where.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase_create.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase_update.gois excluded by!**/ent/db/**openmeter/ent/db/client.gois excluded by!**/ent/db/**openmeter/ent/db/cursor.gois excluded by!**/ent/db/**openmeter/ent/db/ent.gois excluded by!**/ent/db/**openmeter/ent/db/entmixinaccessor.gois excluded by!**/ent/db/**openmeter/ent/db/expose.gois excluded by!**/ent/db/**openmeter/ent/db/hook/hook.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord/ledgercreditvoidrecord.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord/where.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_create.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_delete.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_query.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_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/paginate.gois excluded by!**/ent/db/**openmeter/ent/db/predicate/predicate.gois excluded by!**/ent/db/**openmeter/ent/db/runtime.gois excluded by!**/ent/db/**openmeter/ent/db/setorclear.gois excluded by!**/ent/db/**openmeter/ent/db/tx.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (61)
api/spec/packages/aip-client-javascript/README.mdapi/spec/packages/aip-client-javascript/src/funcs/customers.tsapi/spec/packages/aip-client-javascript/src/index.tsapi/spec/packages/aip-client-javascript/src/models/operations/customers.tsapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip-client-javascript/src/sdk/customers.tsapi/spec/packages/aip/src/customers/credits/grant.tspapi/spec/packages/aip/src/customers/credits/ledger.tspapi/spec/packages/aip/src/customers/credits/operations.tspapi/spec/packages/aip/src/konnect.tspapi/spec/packages/aip/src/openmeter.tspapi/v3/api.gen.goapi/v3/handlers/customers/credits/convert.goapi/v3/handlers/customers/credits/convert_test.goapi/v3/handlers/customers/credits/errors.goapi/v3/handlers/customers/credits/handler.goapi/v3/handlers/customers/credits/void_grant.goapi/v3/handlers/customers/credits/void_grant_test.goapi/v3/server/routes.goapp/common/creditgrant.goapp/common/customerbalance.gocmd/server/wire_gen.goe2e/customer_credits_v3_test.goe2e/v3helpers_test.goopenmeter/billing/charges/creditpurchase/adapter.goopenmeter/billing/charges/creditpurchase/adapter/charge.goopenmeter/billing/charges/creditpurchase/adapter/mapper.goopenmeter/billing/charges/creditpurchase/charge.goopenmeter/billing/charges/creditpurchase/service.goopenmeter/billing/charges/creditpurchase/service/get.goopenmeter/billing/creditgrant/noop.goopenmeter/billing/creditgrant/service.goopenmeter/billing/creditgrant/service/service.goopenmeter/billing/creditgrant/service/service_test.goopenmeter/ent/schema/chargescreditpurchase.goopenmeter/ent/schema/ledger_credit_void_record.goopenmeter/ledger/breakage/breakage_impacts.goopenmeter/ledger/breakage/breakage_impacts_test.goopenmeter/ledger/breakage/service.goopenmeter/ledger/breakage/types.goopenmeter/ledger/creditvoid/adapter/adapter.goopenmeter/ledger/creditvoid/adapter/query.goopenmeter/ledger/creditvoid/adapter/record.goopenmeter/ledger/creditvoid/noop.goopenmeter/ledger/creditvoid/service.goopenmeter/ledger/customerbalance/expired_loader.goopenmeter/ledger/customerbalance/ledger_loader.goopenmeter/ledger/customerbalance/loaders.goopenmeter/ledger/customerbalance/service.goopenmeter/ledger/customerbalance/testenv_test.goopenmeter/ledger/customerbalance/transactions.goopenmeter/ledger/customerbalance/voided_loader.gotest/app/stripe/invoice_test.gotest/credits/base.gotest/credits/creditgrant_test.gotest/credits/voidgrant_test.gotools/migrate/migrations/20260706131715_add_credit_purchase_voided_at.down.sqltools/migrate/migrations/20260706131715_add_credit_purchase_voided_at.up.sqltools/migrate/migrations/20260707114110_add_ledger_credit_void_records.down.sqltools/migrate/migrations/20260707114110_add_ledger_credit_void_records.up.sql
GAlexIHU
force-pushed
the
feat/grant-voiding
branch
from
July 9, 2026 11:03
6ae1fbb to
49890a9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@api/spec/packages/aip-client-javascript/README.md`:
- Around line 153-162: The `client.customers.credits.grants.void` documentation
uses non-American English in the description, so update the wording in the
README to American English by changing “afterwards” to “afterward” while keeping
the rest of the void-grant explanation intact. Make the edit in the table entry
for `client.customers.credits.grants.void` so the documentation wording is
consistent with the style guide.
🪄 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
Run ID: a34a1284-fc95-4086-af96-d4feea33802a
⛔ Files ignored due to path filters (27)
api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/chargecreditpurchase.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase/chargecreditpurchase.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase/where.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase_create.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase_update.gois excluded by!**/ent/db/**openmeter/ent/db/client.gois excluded by!**/ent/db/**openmeter/ent/db/cursor.gois excluded by!**/ent/db/**openmeter/ent/db/ent.gois excluded by!**/ent/db/**openmeter/ent/db/entmixinaccessor.gois excluded by!**/ent/db/**openmeter/ent/db/expose.gois excluded by!**/ent/db/**openmeter/ent/db/hook/hook.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord/ledgercreditvoidrecord.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord/where.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_create.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_delete.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_query.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_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/paginate.gois excluded by!**/ent/db/**openmeter/ent/db/predicate/predicate.gois excluded by!**/ent/db/**openmeter/ent/db/runtime.gois excluded by!**/ent/db/**openmeter/ent/db/setorclear.gois excluded by!**/ent/db/**openmeter/ent/db/tx.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (61)
api/spec/packages/aip-client-javascript/README.mdapi/spec/packages/aip-client-javascript/src/funcs/customers.tsapi/spec/packages/aip-client-javascript/src/index.tsapi/spec/packages/aip-client-javascript/src/models/operations/customers.tsapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip-client-javascript/src/sdk/customers.tsapi/spec/packages/aip/src/customers/credits/grant.tspapi/spec/packages/aip/src/customers/credits/ledger.tspapi/spec/packages/aip/src/customers/credits/operations.tspapi/spec/packages/aip/src/konnect.tspapi/spec/packages/aip/src/openmeter.tspapi/v3/api.gen.goapi/v3/handlers/customers/credits/convert.goapi/v3/handlers/customers/credits/convert_test.goapi/v3/handlers/customers/credits/errors.goapi/v3/handlers/customers/credits/handler.goapi/v3/handlers/customers/credits/void_grant.goapi/v3/handlers/customers/credits/void_grant_test.goapi/v3/server/routes.goapp/common/creditgrant.goapp/common/customerbalance.gocmd/server/wire_gen.goe2e/customer_credits_v3_test.goe2e/v3helpers_test.goopenmeter/billing/charges/creditpurchase/adapter.goopenmeter/billing/charges/creditpurchase/adapter/charge.goopenmeter/billing/charges/creditpurchase/adapter/mapper.goopenmeter/billing/charges/creditpurchase/charge.goopenmeter/billing/charges/creditpurchase/service.goopenmeter/billing/charges/creditpurchase/service/get.goopenmeter/billing/creditgrant/noop.goopenmeter/billing/creditgrant/service.goopenmeter/billing/creditgrant/service/service.goopenmeter/billing/creditgrant/service/service_test.goopenmeter/ent/schema/chargescreditpurchase.goopenmeter/ent/schema/ledger_credit_void_record.goopenmeter/ledger/breakage/breakage_impacts.goopenmeter/ledger/breakage/breakage_impacts_test.goopenmeter/ledger/breakage/service.goopenmeter/ledger/breakage/types.goopenmeter/ledger/creditvoid/adapter/adapter.goopenmeter/ledger/creditvoid/adapter/query.goopenmeter/ledger/creditvoid/adapter/record.goopenmeter/ledger/creditvoid/noop.goopenmeter/ledger/creditvoid/service.goopenmeter/ledger/customerbalance/expired_loader.goopenmeter/ledger/customerbalance/ledger_loader.goopenmeter/ledger/customerbalance/loaders.goopenmeter/ledger/customerbalance/service.goopenmeter/ledger/customerbalance/testenv_test.goopenmeter/ledger/customerbalance/transactions.goopenmeter/ledger/customerbalance/voided_loader.gotest/app/stripe/invoice_test.gotest/credits/base.gotest/credits/creditgrant_test.gotest/credits/voidgrant_test.gotools/migrate/migrations/20260709105231_add_credit_purchase_voided_at.down.sqltools/migrate/migrations/20260709105231_add_credit_purchase_voided_at.up.sqltools/migrate/migrations/20260709105759_add_ledger_credit_void_records.down.sqltools/migrate/migrations/20260709105759_add_ledger_credit_void_records.up.sql
✅ Files skipped from review due to trivial changes (5)
- tools/migrate/migrations/20260709105231_add_credit_purchase_voided_at.up.sql
- tools/migrate/migrations/20260709105759_add_ledger_credit_void_records.up.sql
- api/spec/packages/aip-client-javascript/src/index.ts
- api/v3/server/routes.go
- openmeter/ledger/customerbalance/expired_loader.go
🚧 Files skipped from review as they are similar to previous changes (51)
- openmeter/billing/creditgrant/service/service_test.go
- openmeter/billing/charges/creditpurchase/service/get.go
- openmeter/billing/charges/creditpurchase/adapter/mapper.go
- app/common/customerbalance.go
- openmeter/ledger/breakage/breakage_impacts.go
- openmeter/ledger/customerbalance/voided_loader.go
- api/spec/packages/aip-client-javascript/src/models/operations/customers.ts
- api/spec/packages/aip/src/customers/credits/operations.tsp
- api/spec/packages/aip/src/customers/credits/grant.tsp
- openmeter/ledger/creditvoid/adapter/adapter.go
- test/credits/creditgrant_test.go
- test/credits/base.go
- openmeter/billing/charges/creditpurchase/charge.go
- openmeter/ledger/breakage/service.go
- api/spec/packages/aip/src/customers/credits/ledger.tsp
- api/v3/handlers/customers/credits/void_grant.go
- openmeter/ent/schema/ledger_credit_void_record.go
- openmeter/ledger/customerbalance/loaders.go
- api/v3/handlers/customers/credits/errors.go
- cmd/server/wire_gen.go
- test/app/stripe/invoice_test.go
- app/common/creditgrant.go
- openmeter/ledger/creditvoid/adapter/query.go
- openmeter/ent/schema/chargescreditpurchase.go
- openmeter/ledger/breakage/types.go
- api/spec/packages/aip/src/konnect.tsp
- api/spec/packages/aip-client-javascript/src/sdk/customers.ts
- openmeter/ledger/customerbalance/transactions.go
- api/v3/handlers/customers/credits/void_grant_test.go
- openmeter/ledger/customerbalance/testenv_test.go
- openmeter/ledger/creditvoid/noop.go
- openmeter/ledger/breakage/breakage_impacts_test.go
- api/spec/packages/aip-client-javascript/src/models/types.ts
- openmeter/billing/creditgrant/noop.go
- openmeter/billing/creditgrant/service/service.go
- api/v3/handlers/customers/credits/handler.go
- openmeter/ledger/customerbalance/ledger_loader.go
- openmeter/billing/charges/creditpurchase/service.go
- openmeter/billing/charges/creditpurchase/adapter/charge.go
- api/spec/packages/aip/src/openmeter.tsp
- openmeter/ledger/creditvoid/adapter/record.go
- e2e/v3helpers_test.go
- api/v3/handlers/customers/credits/convert_test.go
- openmeter/billing/charges/creditpurchase/adapter.go
- e2e/customer_credits_v3_test.go
- api/spec/packages/aip-client-javascript/src/models/schemas.ts
- openmeter/billing/creditgrant/service.go
- openmeter/ledger/customerbalance/service.go
- openmeter/ledger/creditvoid/service.go
- api/v3/handlers/customers/credits/convert.go
- test/credits/voidgrant_test.go
GAlexIHU
force-pushed
the
feat/grant-voiding
branch
from
July 9, 2026 12:34
44b5e60 to
cbc5ff1
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
api/spec/packages/aip-client-javascript/README.md (1)
153-162: 📐 Maintainability & Code Quality | 🟡 Minor | 💤 Low value"afterwards" still present despite prior fix.
The previous review flagged
afterwards→afterwardand was marked as addressed, but line 158 still readsafterwards. Since this README is generated output, the fix should be applied in the TypeSpec source (likelyapi/spec/packages/aip/src/customers/credits/grant.tsp) and the README regenerated, rather than hand-editing the generated file.Based on learnings, this file is generator output and everything in it must be regenerable from TypeSpec. As per coding guidelines, do not hand-edit
packages/aip-client-javascript.#!/bin/bash # Description: Search for "afterwards" in the TypeSpec source to verify if the fix was applied upstream. rg -rn 'afterwards' api/spec/packages/aip/src/🤖 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 `@api/spec/packages/aip-client-javascript/README.md` around lines 153 - 162, The generated README still contains “afterwards” instead of the corrected wording, so the upstream TypeSpec needs to be fixed and regenerated rather than editing the generated docs directly. Update the relevant copy in the TypeSpec source under the credits grant definitions (for example, the description around the grant void behavior in the grant spec that feeds `client.customers.credits.grants.void`), then regenerate the `aip-client-javascript` README so the text reflects the source change. Avoid manual edits in the generated package output and verify there are no remaining “afterwards” occurrences in the TypeSpec source.Source: Coding guidelines
🧹 Nitpick comments (1)
openmeter/ledger/customerbalance/ledger_loader_test.go (1)
105-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding
// given:,// when:,// then:intent comments.Per coding guidelines, service and lifecycle subtests should start with concise intent comments. Both test functions above would benefit from them for readability, though the descriptive test names partially compensate.
As per coding guidelines: "For service and lifecycle subtests, start each non-trivial subtest body with concise
// given:,// when:,// 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/ledger/customerbalance/ledger_loader_test.go` around lines 105 - 180, Add concise intent comments to the non-trivial subtest bodies in the ledger loader tests so they follow the coding guideline for service and lifecycle subtests. Update the relevant test functions around ledgerLoaderFakeLedger and ledgerLoaderTestTransaction usage to begin each subtest with brief // given:, // when:, and // then: comments, keeping the comments aligned with the setup, action, and assertion sections.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.
Duplicate comments:
In `@api/spec/packages/aip-client-javascript/README.md`:
- Around line 153-162: The generated README still contains “afterwards” instead
of the corrected wording, so the upstream TypeSpec needs to be fixed and
regenerated rather than editing the generated docs directly. Update the relevant
copy in the TypeSpec source under the credits grant definitions (for example,
the description around the grant void behavior in the grant spec that feeds
`client.customers.credits.grants.void`), then regenerate the
`aip-client-javascript` README so the text reflects the source change. Avoid
manual edits in the generated package output and verify there are no remaining
“afterwards” occurrences in the TypeSpec source.
---
Nitpick comments:
In `@openmeter/ledger/customerbalance/ledger_loader_test.go`:
- Around line 105-180: Add concise intent comments to the non-trivial subtest
bodies in the ledger loader tests so they follow the coding guideline for
service and lifecycle subtests. Update the relevant test functions around
ledgerLoaderFakeLedger and ledgerLoaderTestTransaction usage to begin each
subtest with brief // given:, // when:, and // then: comments, keeping the
comments aligned with the setup, action, and assertion sections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5fd46077-add0-4d59-8156-7fc751b2d168
⛔ Files ignored due to path filters (27)
api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/chargecreditpurchase.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase/chargecreditpurchase.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase/where.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase_create.gois excluded by!**/ent/db/**openmeter/ent/db/chargecreditpurchase_update.gois excluded by!**/ent/db/**openmeter/ent/db/client.gois excluded by!**/ent/db/**openmeter/ent/db/cursor.gois excluded by!**/ent/db/**openmeter/ent/db/ent.gois excluded by!**/ent/db/**openmeter/ent/db/entmixinaccessor.gois excluded by!**/ent/db/**openmeter/ent/db/expose.gois excluded by!**/ent/db/**openmeter/ent/db/hook/hook.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord/ledgercreditvoidrecord.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord/where.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_create.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_delete.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_query.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercreditvoidrecord_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/paginate.gois excluded by!**/ent/db/**openmeter/ent/db/predicate/predicate.gois excluded by!**/ent/db/**openmeter/ent/db/runtime.gois excluded by!**/ent/db/**openmeter/ent/db/setorclear.gois excluded by!**/ent/db/**openmeter/ent/db/tx.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (62)
api/spec/packages/aip-client-javascript/README.mdapi/spec/packages/aip-client-javascript/src/funcs/customers.tsapi/spec/packages/aip-client-javascript/src/index.tsapi/spec/packages/aip-client-javascript/src/models/operations/customers.tsapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip-client-javascript/src/sdk/customers.tsapi/spec/packages/aip/src/customers/credits/grant.tspapi/spec/packages/aip/src/customers/credits/ledger.tspapi/spec/packages/aip/src/customers/credits/operations.tspapi/spec/packages/aip/src/konnect.tspapi/spec/packages/aip/src/openmeter.tspapi/v3/api.gen.goapi/v3/handlers/customers/credits/convert.goapi/v3/handlers/customers/credits/convert_test.goapi/v3/handlers/customers/credits/errors.goapi/v3/handlers/customers/credits/handler.goapi/v3/handlers/customers/credits/void_grant.goapi/v3/handlers/customers/credits/void_grant_test.goapi/v3/server/routes.goapp/common/creditgrant.goapp/common/customerbalance.gocmd/server/wire_gen.goe2e/customer_credits_v3_test.goe2e/v3helpers_test.goopenmeter/billing/charges/creditpurchase/adapter.goopenmeter/billing/charges/creditpurchase/adapter/charge.goopenmeter/billing/charges/creditpurchase/adapter/mapper.goopenmeter/billing/charges/creditpurchase/charge.goopenmeter/billing/charges/creditpurchase/service.goopenmeter/billing/charges/creditpurchase/service/get.goopenmeter/billing/creditgrant/noop.goopenmeter/billing/creditgrant/service.goopenmeter/billing/creditgrant/service/service.goopenmeter/billing/creditgrant/service/service_test.goopenmeter/ent/schema/chargescreditpurchase.goopenmeter/ent/schema/ledger_credit_void_record.goopenmeter/ledger/breakage/breakage_impacts.goopenmeter/ledger/breakage/breakage_impacts_test.goopenmeter/ledger/breakage/service.goopenmeter/ledger/breakage/types.goopenmeter/ledger/creditvoid/adapter/adapter.goopenmeter/ledger/creditvoid/adapter/query.goopenmeter/ledger/creditvoid/adapter/record.goopenmeter/ledger/creditvoid/noop.goopenmeter/ledger/creditvoid/service.goopenmeter/ledger/customerbalance/expired_loader.goopenmeter/ledger/customerbalance/ledger_loader.goopenmeter/ledger/customerbalance/ledger_loader_test.goopenmeter/ledger/customerbalance/loaders.goopenmeter/ledger/customerbalance/service.goopenmeter/ledger/customerbalance/testenv_test.goopenmeter/ledger/customerbalance/transactions.goopenmeter/ledger/customerbalance/voided_loader.gotest/app/stripe/invoice_test.gotest/credits/base.gotest/credits/creditgrant_test.gotest/credits/voidgrant_test.gotools/migrate/migrations/20260709132500_add_credit_purchase_voided_at.down.sqltools/migrate/migrations/20260709132500_add_credit_purchase_voided_at.up.sqltools/migrate/migrations/20260709132600_add_ledger_credit_void_records.down.sqltools/migrate/migrations/20260709132600_add_ledger_credit_void_records.up.sql
✅ Files skipped from review due to trivial changes (3)
- tools/migrate/migrations/20260709132600_add_ledger_credit_void_records.up.sql
- api/spec/packages/aip-client-javascript/src/index.ts
- cmd/server/wire_gen.go
🚧 Files skipped from review as they are similar to previous changes (52)
- openmeter/ledger/breakage/breakage_impacts.go
- openmeter/ledger/customerbalance/loaders.go
- openmeter/ledger/customerbalance/testenv_test.go
- openmeter/ent/schema/ledger_credit_void_record.go
- openmeter/billing/charges/creditpurchase/charge.go
- openmeter/ledger/customerbalance/voided_loader.go
- api/spec/packages/aip/src/customers/credits/grant.tsp
- app/common/customerbalance.go
- openmeter/ent/schema/chargescreditpurchase.go
- api/spec/packages/aip/src/openmeter.tsp
- openmeter/billing/charges/creditpurchase/service/get.go
- openmeter/billing/charges/creditpurchase/adapter/charge.go
- api/spec/packages/aip-client-javascript/src/models/operations/customers.ts
- openmeter/billing/charges/creditpurchase/service.go
- test/app/stripe/invoice_test.go
- api/spec/packages/aip/src/customers/credits/ledger.tsp
- api/v3/handlers/customers/credits/errors.go
- openmeter/ledger/creditvoid/noop.go
- openmeter/billing/charges/creditpurchase/adapter/mapper.go
- api/v3/handlers/customers/credits/convert_test.go
- openmeter/ledger/breakage/types.go
- openmeter/ledger/customerbalance/expired_loader.go
- openmeter/billing/creditgrant/noop.go
- openmeter/ledger/customerbalance/transactions.go
- openmeter/ledger/customerbalance/service.go
- openmeter/ledger/creditvoid/adapter/record.go
- openmeter/ledger/breakage/breakage_impacts_test.go
- api/v3/handlers/customers/credits/handler.go
- e2e/v3helpers_test.go
- api/spec/packages/aip/src/konnect.tsp
- openmeter/ledger/breakage/service.go
- openmeter/billing/charges/creditpurchase/adapter.go
- api/spec/packages/aip-client-javascript/src/models/types.ts
- openmeter/billing/creditgrant/service/service_test.go
- test/credits/creditgrant_test.go
- openmeter/ledger/creditvoid/adapter/query.go
- api/spec/packages/aip-client-javascript/src/sdk/customers.ts
- api/v3/server/routes.go
- e2e/customer_credits_v3_test.go
- openmeter/ledger/customerbalance/ledger_loader.go
- api/spec/packages/aip/src/customers/credits/operations.tsp
- api/v3/handlers/customers/credits/void_grant_test.go
- openmeter/billing/creditgrant/service.go
- test/credits/base.go
- api/v3/handlers/customers/credits/void_grant.go
- openmeter/billing/creditgrant/service/service.go
- openmeter/ledger/creditvoid/adapter/adapter.go
- app/common/creditgrant.go
- api/v3/handlers/customers/credits/convert.go
- test/credits/voidgrant_test.go
- api/spec/packages/aip-client-javascript/src/models/schemas.ts
- openmeter/ledger/creditvoid/service.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/ledger/creditvoid/types.go`:
- Line 118: Wrap the accumulated validation errors returned by the relevant
validation function in models.NewNillableGenericValidationError, passing
errors.Join(errs...) as its argument instead of returning the joined errors
directly.
- Around line 74-82: Update CreateRecordsInput.Validate to collect validation
failures from every record in a var errs []error, preserving each record’s index
in its wrapped error, then return
models.NewNillableGenericValidationError(errors.Join(errs...)) instead of
returning immediately; return the same wrapper for the empty-error case as
required by the validation convention.
🪄 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
Run ID: 6b065708-de78-422c-b3ee-b5917641a9fa
📒 Files selected for processing (6)
api/v3/handlers/customers/credits/void_grant_test.goopenmeter/billing/creditgrant/service.goopenmeter/ledger/creditvoid/service.goopenmeter/ledger/creditvoid/types.goopenmeter/ledger/customerbalance/ledger_loader.goopenmeter/ledger/customerbalance/ledger_loader_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- api/v3/handlers/customers/credits/void_grant_test.go
- openmeter/billing/creditgrant/service.go
Comment on lines
+74
to
+82
| func (i CreateRecordsInput) Validate() error { | ||
| for idx, record := range i.Records { | ||
| if err := record.Validate(); err != nil { | ||
| return fmt.Errorf("records[%d]: %w", idx, err) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Collect all record errors and wrap the result.
This one bails on the first bad record instead of collecting them. As per coding guidelines: "For Validate() error methods, collect all validation problems into var errs []error and return models.NewNillableGenericValidationError(errors.Join(errs...)) rather than failing fast on the first invalid field."
♻️ Proposed fix
func (i CreateRecordsInput) Validate() error {
+ var errs []error
for idx, record := range i.Records {
if err := record.Validate(); err != nil {
- return fmt.Errorf("records[%d]: %w", idx, err)
+ errs = append(errs, fmt.Errorf("records[%d]: %w", idx, err))
}
}
-
- return nil
+ return models.NewNillableGenericValidationError(errors.Join(errs...))
}📝 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
| func (i CreateRecordsInput) Validate() error { | |
| for idx, record := range i.Records { | |
| if err := record.Validate(); err != nil { | |
| return fmt.Errorf("records[%d]: %w", idx, err) | |
| } | |
| } | |
| return nil | |
| } | |
| func (i CreateRecordsInput) Validate() error { | |
| var errs []error | |
| for idx, record := range i.Records { | |
| if err := record.Validate(); err != nil { | |
| errs = append(errs, fmt.Errorf("records[%d]: %w", idx, err)) | |
| } | |
| } | |
| return models.NewNillableGenericValidationError(errors.Join(errs...)) | |
| } |
🤖 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/ledger/creditvoid/types.go` around lines 74 - 82, Update
CreateRecordsInput.Validate to collect validation failures from every record in
a var errs []error, preserving each record’s index in its wrapped error, then
return models.NewNillableGenericValidationError(errors.Join(errs...)) instead of
returning immediately; return the same wrapper for the empty-error case as
required by the validation convention.
Source: Coding guidelines
| errs = append(errs, errors.New("receivable sub-account id is required")) | ||
| } | ||
|
|
||
| return errors.Join(errs...) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Wrap the joined errors in NewNillableGenericValidationError.
The error accumulation here is spot on, just missing the standard wrapper. As per coding guidelines: "return models.NewNillableGenericValidationError(errors.Join(errs...))".
♻️ Proposed fix
- return errors.Join(errs...)
+ return models.NewNillableGenericValidationError(errors.Join(errs...))📝 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
| return errors.Join(errs...) | |
| return models.NewNillableGenericValidationError(errors.Join(errs...)) |
🤖 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/ledger/creditvoid/types.go` at line 118, Wrap the accumulated
validation errors returned by the relevant validation function in
models.NewNillableGenericValidationError, passing errors.Join(errs...) as its
argument instead of returning the joined errors directly.
Source: Coding guidelines
turip
reviewed
Jul 10, 2026
| switch metaStatus { | ||
| case meta.ChargeStatusActive, meta.ChargeStatusFinal: | ||
| case meta.ChargeStatusCreated: | ||
| return models.NewGenericConflictError(fmt.Errorf("credit grant %s is pending and cannot be voided", charge.ID)) |
Member
There was a problem hiding this comment.
I think from the UX perspective it would be better if we allow it and treat it as a noop imho (e.g. we don't need to trigger the - non-existing statemachine) in this case.
GAlexIHU
force-pushed
the
feat/grant-voiding
branch
from
July 10, 2026 14:01
53209c9 to
5ad12dc
Compare
turip
previously approved these changes
Jul 10, 2026
GAlexIHU
force-pushed
the
feat/grant-voiding
branch
from
July 13, 2026 11:12
4d462d2 to
90eb5a2
Compare
turip
approved these changes
Jul 14, 2026
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
2 participants
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.
Overview
Adds credit grant voiding for customer credits.
Voiding now:
voided_atledger_credit_void_recordsrow linked to the ledger transaction group so transaction listing can annotate voided credit movementsvoidedgrant and transaction status in the v3 customer credits APIAPI behavior
POST /api/v3/customers/{customerId}/credits/grants/{grantId}/voidvoidedstatus support for grants and credit transactionspayment_adjustmentto the void requestpayment_adjustmentvalue for now isnonepayment_adjustment=noneis explicit API surface for the current payment limitation: voiding does not mutate or cancel payment-side artifacts. If an invoiced grant is already authorized or settled, payment-side receivable balances can remain and are expected to be handled separately later.Notes for reviewer
voided_at.voidedoverexpiredwhen a source charge has both states.Verification
make gen-apimake generateatlas migrate --env local hashPOSTGRES_HOST=127.0.0.1 go test -tags=dynamic ./api/v3/handlers/customers/credits ./openmeter/billing/creditgrant/service ./openmeter/ledger/breakage ./openmeter/ledger/customerbalance ./openmeter/ledger/creditvoid/... ./test/credits -run 'Test(ConvertAPI|ToAPIBillingCreditGrant|ValidateChargeVoidable|ListExpiredBreakageImpacts|ListCreditTransactions|CreditTransaction|GetBalance|FacadeGetBalance|VoidGrantTestSuite)' -count=1POSTGRES_HOST=127.0.0.1 go vet -tags=dynamic ./api/v3/handlers/customers/credits ./openmeter/billing/creditgrant/service ./openmeter/ledger/breakage ./openmeter/ledger/customerbalance ./openmeter/ledger/creditvoid/... ./test/creditsCI=true make lint-api-specTZ=UTC go test -count=1 -tags=dynamic -v -run 'TestV3VoidCreditGrant|TestV3VoidCreditGrantPaymentAdjustmentNone' ./e2e/(compiled locally; skipped becauseOPENMETER_ADDRESSwas unset)go vet -tags=dynamic ./e2eSummary by CodeRabbit
paymentAdjustment, supporting onlynone).voidedlifecycle/state.VoidedAtare derived consistently (including expiry/void precedence) and remain stable across reads.Greptile Summary
This PR adds grant voiding for customer credits. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (15): Last reviewed commit: "fix(credits): address grant voiding revi..." | Re-trigger Greptile
Context used (3)