Conversation
📝 WalkthroughWalkthroughAdds a CreditAdjustment model and POST /openmeter/customers/{customerId}/credits/adjustments endpoint, implements credit-grant domain/service and handlers (create/get/list), removes the void-credit-grant operation and grant timing fields, and wires the CreditGrantService through the app and router. Changes
Sequence DiagramsequenceDiagram
participant Client
participant API_Handler as CreateCreditAdjustment<br/>(Handler)
participant CreditGrantSvc as CreditGrantService
participant BillingRegistry as BillingRegistry<br/>(CreditPurchase)
participant Storage as Charge Storage
Client->>API_Handler: POST /customers/{id}/credits/adjustments (currency, amount,...)
API_Handler->>API_Handler: validate request, resolve namespace
API_Handler->>CreditGrantSvc: Create(CreateInput)
CreditGrantSvc->>CreditGrantSvc: validate input, verify customer
CreditGrantSvc->>BillingRegistry: Create CreditPurchase.Intent
BillingRegistry->>Storage: persist charge
Storage-->>BillingRegistry: stored charge record
BillingRegistry-->>CreditGrantSvc: creditpurchase.Charge
CreditGrantSvc-->>API_Handler: creditpurchase.Charge
API_Handler->>API_Handler: convert to API model
API_Handler-->>Client: 201 Created (BillingCreditAdjustment)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/v3/api.gen.go (1)
3147-3212:⚠️ Potential issue | 🟡 MinorThe grant lifecycle docs are now out of sync with the public schema.
This request no longer exposes timing fields, but the generated
BillingCreditGrantStatusdocs still describepending/active/expiredin terms ofeffective_atandexpires_at. That makes the published contract confusing for SDK consumers. Please fix the source spec comments and regenerate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v3/api.gen.go` around lines 3147 - 3212, The generated docs for BillingCreditGrantStatus are out-of-date because CreateCreditGrantRequest no longer exposes timing fields (effective_at, expires_at); update the source OpenAPI/spec comments for BillingCreditGrantStatus (and any related schema comments) to remove references to effective_at/expires_at and instead describe the new lifecycle semantics (e.g., how pending/active/expired are determined without those fields), then regenerate api.gen.go so the comment block above type BillingCreditGrantStatus and any references in CreateCreditGrantRequest/Purchase/TaxConfig match the current public schema.
🧹 Nitpick comments (5)
openmeter/billing/creditgrant/service.go (1)
97-105: Avoid silently accepting unused purchase termsOn Line 97-Line 105,
Purchaseis allowed even whenFundingMethodisnone, but those fields are later ignored. I’d recommend rejecting that combination to keep API behavior explicit and predictable.Suggested validation tweak
if i.FundingMethod != FundingMethodNone && i.Purchase == nil { errs = append(errs, errors.New("purchase terms are required for funded grants")) } + +if i.FundingMethod == FundingMethodNone && i.Purchase != nil { + errs = append(errs, errors.New("purchase terms must be empty when funding method is none")) +} if i.Purchase != nil { if err := i.Purchase.Currency.Validate(); err != nil { errs = append(errs, fmt.Errorf("purchase currency: %w", err)) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openmeter/billing/creditgrant/service.go` around lines 97 - 105, The validation currently allows i.Purchase when i.FundingMethod == FundingMethodNone and later ignores those fields; update the validation in the same function (where i is validated) to reject the combination by appending an error when i.FundingMethod == FundingMethodNone && i.Purchase != nil (e.g., "purchase terms not allowed for non-funded grants"), and keep the existing currency validation but guard it so it only runs when i.Purchase != nil and i.FundingMethod != FundingMethodNone to avoid validating ignored fields.openmeter/server/router/router.go (1)
138-248: Add an explicit credits-enabled guard forCreditGrantServiceSince
CreditGrantServiceis now part ofConfig, it’d be safer to enforce: if credits are enabled, the service must be non-nil. That makes failures deterministic at startup instead of later in request handling.Based on learnings Ensure
credits.enabledsetting is explicitly guarded at multiple layers: ledger-backed customer credit handlers inapi/v3/server, customer ledger hooks, and namespace/default-account provisioning; these are wired separately and must each stay disabled when credits are offSuggested guard in config validation
func (c Config) Validate() error { + if c.Credits.Enabled && c.CreditGrantService == nil { + return errors.New("credit grant service is required when credits are enabled") + } + if c.NamespaceManager == nil { return errors.New("namespace manager is required") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openmeter/server/router/router.go` around lines 138 - 248, Config.Validate must explicitly guard CreditGrantService when credits are enabled: add a check in Config.Validate that returns an error if the credits.enabled flag is true and c.CreditGrantService is nil. Implement this by reading your config flag (e.g., a boolean field like c.CreditsEnabled or a nested c.Credits.Enabled) and if it is true and CreditGrantService == nil, return errors.New("credit grant service is required when credits are enabled"); keep the check near the other connector/service validations in the Validate() method.api/spec/packages/aip/src/customers/credits/adjustment.tsp (1)
16-23: Clarify adjustment sign semantics in the field docsTiny docs nit: on Line 16 and Line 22, “granted” reads like only positive grants. For an adjustment endpoint, consider wording like “adjusted amount” and (if true) explicitly state positive vs negative behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/spec/packages/aip/src/customers/credits/adjustment.tsp` around lines 16 - 23, Update the field JSDoc to clarify sign semantics: change "The currency of the granted credits." (currency: Currencies.CurrencyCode) to indicate it's the currency for the adjustment (e.g., "The currency of the adjusted credits."), and replace "Granted credit amount." on the adjustment amount field with wording like "Adjusted amount; positive values grant credits, negative values deduct credits." Ensure you update only the documentation comments for the relevant fields (currency and the adjustment amount) in the adjustment.tsp model.api/spec/packages/aip/src/customers/credits/grant.tsp (1)
258-279: Consider removing (not commenting) deprecated contract fields.Keeping full commented-out schema blocks in the TypeSpec source makes the contract harder to read and maintain. A short changelog note/PR reference is usually cleaner than inline commented definitions.
As per coding guidelines: "Review the TypeSpec code for conformity with TypeSpec best practices."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/spec/packages/aip/src/customers/credits/grant.tsp` around lines 258 - 279, Remove the commented-out deprecated schema fields (effective_at, expires_after, expires_at) from the TypeSpec source so the contract is not cluttered with dead code; delete the entire commented blocks that reference those symbols and, if needed, add a short changelog/PR note near the top of the file or in the PR description documenting the removal rather than leaving inline commented definitions.api/spec/packages/aip/src/customers/credits/operations.tsp (1)
81-98: Can we remove the old void operation instead of parking it as comments?Keeping a full removed endpoint in the TypeSpec source gets stale fast and makes the contract a bit harder to trust. I’d just delete it and let git/history preserve the old shape.
As per coding guidelines, "Define API specifications using TypeSpec in
api/spec/as the source of truth for API contracts, then regenerate OpenAPI and SDKs withmake gen-api."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/spec/packages/aip/src/customers/credits/operations.tsp` around lines 81 - 98, Remove the commented-out legacy endpoint block for CustomerCreditGrantVoidOperations (the commented interface and its voidGrant method including `@post/`@operationId/@summary annotations) from the TypeSpec file so the API spec no longer contains stale commented endpoints; rely on git history to retain the previous shape and then run the usual generation step (make gen-api) to update derived artifacts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/v3/handlers/customers/credits/convert.go`:
- Around line 52-60: The convertGrantStatus function currently maps only
"created" and "active" and defaults everything else to
api.BillingCreditGrantStatusActive; update
convertGrantStatus(creditpurchase.Charge) to explicitly map all known domain
statuses (at minimum "active", "created", and "voided") to their corresponding
api.BillingCreditGrantStatus values and stop silently defaulting unknown strings
to Active — change the function to return (api.BillingCreditGrantStatus, error)
(or otherwise return a clearly invalid/unknown enum value) and return an error
for any unrecognized charge.Status so callers fail closed instead of
misreporting unknown states.
- Around line 244-256: The parsed per_unit_cost_basis must be validated as >0;
update validation to reject zero/negative values by adding an IsPositive() check
after alpacadecimal.NewFromString(...) and setting purchase.PerUnitCostBasis (or
centralize it in creditgrant.CreateInput.Validate()); specifically, in the
handler around the block that parses body.Purchase.PerUnitCostBasis ensure you
call costBasis.IsPositive() (or equivalent) and return an error like
"per_unit_cost_basis must be positive" if false, or alternatively add the same
positive check inside creditgrant.CreateInput.Validate() so CreateInput and
purchase.PerUnitCostBasis cannot be zero/negative.
In `@api/v3/handlers/customers/credits/get_grant.go`:
- Around line 17-20: GetCreditGrantParams currently includes CustomerID but that
value isn't used: update the handler that builds creditgrant.GetInput (and
related lookups used by GetCreditGrant handler) to pass args.CustomerID into the
service query or, after fetching the grant via creditgrant.GetInput (which
currently uses Namespace + ChargeID), add an ownership check comparing the
returned grant's CustomerID to args.CustomerID and return a not-found/forbidden
error if they differ; specifically modify where creditgrant.GetInput is
constructed and where the service returns the grant to either include CustomerID
in the input or validate ownership before returning in the GetCreditGrant
handler (also apply the same fix to the other similar handler referenced around
lines 32-35).
In `@api/v3/server/routes.go`:
- Around line 372-374: The CreateCreditAdjustment handler currently delegates to
unimplemented.CreateCreditAdjustment, exposing a 501 for the public API; either
remove/hide the route from the published spec or implement the handler now:
replace the unimplemented call in Server.CreateCreditAdjustment with the real
logic (e.g., parse request body, validate customerId, call your service/repo
method such as s.Service.CreateCreditAdjustment or s.AdjustmentStore.Create,
handle errors and write appropriate HTTP responses), or if staged, remove the
public registration so the contract matches implementation.
- Around line 337-347: The grant route currently only checks
s.customersCreditsHandler but can still panic if the CreditGrantService is nil;
update the guard in ListCreditGrants (and the other grant handlers around the
same area) to require both s.customersCreditsHandler != nil AND
s.CreditGrantService != nil before calling
customersCreditsHandler.ListCreditGrants().With(...).ServeHTTP; if either is
nil, call unimplemented.ListCreditGrants(w, r, customerId, params) (or the
corresponding unimplemented handler) and return. Ensure you apply the same
pattern to the other grant routes referenced (the blocks around lines 349–358
and 360–370) so grant-specific handlers are gated separately from balance
handlers.
In `@openmeter/billing/creditgrant/service/service.go`:
- Around line 126-128: The current call to s.chargesService.ListCharges(ctx,
listInput) ignores the FundingMethod, Status, and Currency filters; add a
fail-fast guard before invoking ListCharges that validates listInput (check the
fields named FundingMethod, Status, and Currency on the request/listInput struct
used by service.go) and return a clear error when any of these filters are set
(non-nil or non-empty) so callers are not silently given unfiltered results;
keep the TODO comment and replace the guard with real filtering logic once
ListCharges supports those filters.
---
Outside diff comments:
In `@api/v3/api.gen.go`:
- Around line 3147-3212: The generated docs for BillingCreditGrantStatus are
out-of-date because CreateCreditGrantRequest no longer exposes timing fields
(effective_at, expires_at); update the source OpenAPI/spec comments for
BillingCreditGrantStatus (and any related schema comments) to remove references
to effective_at/expires_at and instead describe the new lifecycle semantics
(e.g., how pending/active/expired are determined without those fields), then
regenerate api.gen.go so the comment block above type BillingCreditGrantStatus
and any references in CreateCreditGrantRequest/Purchase/TaxConfig match the
current public schema.
---
Nitpick comments:
In `@api/spec/packages/aip/src/customers/credits/adjustment.tsp`:
- Around line 16-23: Update the field JSDoc to clarify sign semantics: change
"The currency of the granted credits." (currency: Currencies.CurrencyCode) to
indicate it's the currency for the adjustment (e.g., "The currency of the
adjusted credits."), and replace "Granted credit amount." on the adjustment
amount field with wording like "Adjusted amount; positive values grant credits,
negative values deduct credits." Ensure you update only the documentation
comments for the relevant fields (currency and the adjustment amount) in the
adjustment.tsp model.
In `@api/spec/packages/aip/src/customers/credits/grant.tsp`:
- Around line 258-279: Remove the commented-out deprecated schema fields
(effective_at, expires_after, expires_at) from the TypeSpec source so the
contract is not cluttered with dead code; delete the entire commented blocks
that reference those symbols and, if needed, add a short changelog/PR note near
the top of the file or in the PR description documenting the removal rather than
leaving inline commented definitions.
In `@api/spec/packages/aip/src/customers/credits/operations.tsp`:
- Around line 81-98: Remove the commented-out legacy endpoint block for
CustomerCreditGrantVoidOperations (the commented interface and its voidGrant
method including `@post/`@operationId/@summary annotations) from the TypeSpec file
so the API spec no longer contains stale commented endpoints; rely on git
history to retain the previous shape and then run the usual generation step
(make gen-api) to update derived artifacts.
In `@openmeter/billing/creditgrant/service.go`:
- Around line 97-105: The validation currently allows i.Purchase when
i.FundingMethod == FundingMethodNone and later ignores those fields; update the
validation in the same function (where i is validated) to reject the combination
by appending an error when i.FundingMethod == FundingMethodNone && i.Purchase !=
nil (e.g., "purchase terms not allowed for non-funded grants"), and keep the
existing currency validation but guard it so it only runs when i.Purchase != nil
and i.FundingMethod != FundingMethodNone to avoid validating ignored fields.
In `@openmeter/server/router/router.go`:
- Around line 138-248: Config.Validate must explicitly guard CreditGrantService
when credits are enabled: add a check in Config.Validate that returns an error
if the credits.enabled flag is true and c.CreditGrantService is nil. Implement
this by reading your config flag (e.g., a boolean field like c.CreditsEnabled or
a nested c.Credits.Enabled) and if it is true and CreditGrantService == nil,
return errors.New("credit grant service is required when credits are enabled");
keep the check near the other connector/service validations in the Validate()
method.
🪄 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: 943aa1d2-1be4-43f8-8813-fcd427e160b4
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (23)
api/spec/packages/aip/src/customers/credits/adjustment.tspapi/spec/packages/aip/src/customers/credits/grant.tspapi/spec/packages/aip/src/customers/credits/operations.tspapi/spec/packages/aip/src/konnect.tspapi/spec/packages/aip/src/openmeter.tspapi/types/entitlement.goapi/v3/api.gen.goapi/v3/handlers/customers/credits/convert.goapi/v3/handlers/customers/credits/create_grant.goapi/v3/handlers/customers/credits/get_balance.goapi/v3/handlers/customers/credits/get_grant.goapi/v3/handlers/customers/credits/handler.goapi/v3/handlers/customers/credits/list_grants.goapi/v3/server/routes.goapi/v3/server/server.goapp/common/creditgrant.gocmd/server/main.gocmd/server/wire.gocmd/server/wire_gen.goopenmeter/billing/creditgrant/service.goopenmeter/billing/creditgrant/service/service.goopenmeter/server/router/router.goopenmeter/server/server.go
💤 Files with no reviewable changes (1)
- api/types/entitlement.go
| GetCreditGrantParams struct { | ||
| CustomerID api.ULID | ||
| CreditGrantID api.ULID | ||
| } |
There was a problem hiding this comment.
Please scope this lookup by customerId.
args.CustomerID is never used, and creditgrant.GetInput only carries Namespace + ChargeID (openmeter/billing/creditgrant/service.go:116-133). Right now the same grant can be fetched through any customer path inside the namespace, so the customer-scoped route isn’t actually enforced. Please either thread CustomerID into the service lookup or verify ownership before returning the grant.
Also applies to: 32-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/v3/handlers/customers/credits/get_grant.go` around lines 17 - 20,
GetCreditGrantParams currently includes CustomerID but that value isn't used:
update the handler that builds creditgrant.GetInput (and related lookups used by
GetCreditGrant handler) to pass args.CustomerID into the service query or, after
fetching the grant via creditgrant.GetInput (which currently uses Namespace +
ChargeID), add an ownership check comparing the returned grant's CustomerID to
args.CustomerID and return a not-found/forbidden error if they differ;
specifically modify where creditgrant.GetInput is constructed and where the
service returns the grant to either include CustomerID in the input or validate
ownership before returning in the GetCreditGrant handler (also apply the same
fix to the other similar handler referenced around lines 32-35).
| func (s *Server) CreateCreditAdjustment(w http.ResponseWriter, r *http.Request, customerId api.ULID) { | ||
| unimplemented.CreateCreditAdjustment(w, r, customerId) | ||
| } |
There was a problem hiding this comment.
CreateCreditAdjustment is publicly exposed but always returns 501.
Line 373 hardwires this endpoint to unimplemented, so the new API contract is advertised without functional behavior. If this is intentionally staged, consider hiding it from the public spec until wiring lands, or wire the handler in this PR.
As per coding guidelines: "The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/v3/server/routes.go` around lines 372 - 374, The CreateCreditAdjustment
handler currently delegates to unimplemented.CreateCreditAdjustment, exposing a
501 for the public API; either remove/hide the route from the published spec or
implement the handler now: replace the unimplemented call in
Server.CreateCreditAdjustment with the real logic (e.g., parse request body,
validate customerId, call your service/repo method such as
s.Service.CreateCreditAdjustment or s.AdjustmentStore.Create, handle errors and
write appropriate HTTP responses), or if staged, remove the public registration
so the contract matches implementation.
| // TODO: apply FundingMethod, Status, Currency filters once the charges list supports them | ||
|
|
||
| result, err := s.chargesService.ListCharges(ctx, listInput) |
There was a problem hiding this comment.
Filters are accepted but ignored in list results
Line 126-Line 128 currently drops FundingMethod, Status, and Currency filters. That means clients can send valid filters and still get unfiltered results, which is a behavior bug.
If upstream filtering support isn’t ready yet, it’s safer to fail fast when any of these filters are set (instead of silently ignoring them).
Fail-fast guard until filtering is wired
func (s *service) List(ctx context.Context, input creditgrant.ListInput) (pagination.Result[creditpurchase.Charge], error) {
if err := input.Validate(); err != nil {
return pagination.Result[creditpurchase.Charge]{}, fmt.Errorf("invalid input: %w", err)
}
+
+ if input.FundingMethod != nil || input.Status != nil || input.Currency != nil {
+ return pagination.Result[creditpurchase.Charge]{}, fmt.Errorf("list filters are not supported yet")
+ }
listInput := charges.ListChargesInput{
Page: input.Page,📝 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.
| // TODO: apply FundingMethod, Status, Currency filters once the charges list supports them | |
| result, err := s.chargesService.ListCharges(ctx, listInput) | |
| func (s *service) List(ctx context.Context, input creditgrant.ListInput) (pagination.Result[creditpurchase.Charge], error) { | |
| if err := input.Validate(); err != nil { | |
| return pagination.Result[creditpurchase.Charge]{}, fmt.Errorf("invalid input: %w", err) | |
| } | |
| if input.FundingMethod != nil || input.Status != nil || input.Currency != nil { | |
| return pagination.Result[creditpurchase.Charge]{}, fmt.Errorf("list filters are not supported yet") | |
| } | |
| // TODO: apply FundingMethod, Status, Currency filters once the charges list supports them | |
| result, err := s.chargesService.ListCharges(ctx, listInput) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openmeter/billing/creditgrant/service/service.go` around lines 126 - 128, The
current call to s.chargesService.ListCharges(ctx, listInput) ignores the
FundingMethod, Status, and Currency filters; add a fail-fast guard before
invoking ListCharges that validates listInput (check the fields named
FundingMethod, Status, and Currency on the request/listInput struct used by
service.go) and return a clear error when any of these filters are set (non-nil
or non-empty) so callers are not silently given unfiltered results; keep the
TODO comment and replace the guard with real filtering logic once ListCharges
supports those filters.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/v3/api.gen.go (1)
310-323:⚠️ Potential issue | 🟡 MinorEnum docs and actual values are out of sync
Nice cleanup on the enum values, but the type docs still mention removed values (
on_authorization,on_settlement) at Line 1809 and Line 1810. That can mislead SDK/API consumers.Please fix this in the OpenAPI/TypeSpec source and regenerate this file (rather than editing this file directly).
As per coding guidelines, all generated files must have
// Code generated by X, DO NOT EDIT.headers at the top; never edit files bearing this header.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v3/api.gen.go` around lines 310 - 323, The generated enum docs for BillingCreditAvailabilityPolicy are stale—remove the references to the removed values ("on_authorization", "on_settlement") from the OpenAPI/TypeSpec source where BillingCreditAvailabilityPolicy is defined, ensure the enum definition only contains the current member (BillingCreditAvailabilityPolicyOnCreation) and update its description accordingly, then regenerate api.gen.go so the docblock and enum values stay in sync and the file contains the required "// Code generated by ..." header; do not edit api.gen.go directly.
🧹 Nitpick comments (2)
api/v3/handlers/customers/credits/convert.go (1)
134-143: Handle allpayment.Statusvalues explicitly for robustness.The code does sensibly default unknown payment statuses to
Pending, but you might want to addpayment.StatusOpen(or similar) to the switch if it exists in the domain. Currently:
Authorized→ AuthorizedSettled→ Settled- Everything else → Pending
If other statuses exist (e.g.,
StatusFailed,StatusOpen), they'd silently becomePending.openmeter/billing/creditgrant/service.go (1)
144-186: ListInput validation looks good, but optional filters are silently accepted.The validation properly checks required fields and validates optional filters when present. However, I noticed from reviewing the service implementation that
FundingMethod,Status, andCurrencyfilters pass validation here but are currently ignored downstream (see the TODO at line 137 inservice/service.go).Since validation passes but the filters have no effect, clients might not realize their filters aren't being applied. Consider either:
- Rejecting unsupported filters in validation until implemented
- Documenting this limitation clearly
This is a behavior quirk rather than a correctness bug, so flagging for awareness.
Optional: Fail fast on unsupported filters
func (i ListInput) Validate() error { var errs []error if i.Namespace == "" { errs = append(errs, errors.New("namespace is required")) } if i.CustomerID == "" { errs = append(errs, errors.New("customer ID is required")) } + // Filters are validated but not yet supported downstream + if i.FundingMethod != nil || i.Status != nil || i.Currency != nil { + errs = append(errs, errors.New("list filters are not yet supported")) + } + - if i.FundingMethod != nil { - if err := i.FundingMethod.Validate(); err != nil { - errs = append(errs, err) - } - } - - if i.Status != nil { - if err := i.Status.Validate(); err != nil { - errs = append(errs, err) - } - } - - if i.Currency != nil { - if err := i.Currency.Validate(); err != nil { - errs = append(errs, fmt.Errorf("currency: %w", err)) - } - } return models.NewNillableGenericValidationError(errors.Join(errs...)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openmeter/billing/creditgrant/service.go` around lines 144 - 186, ListInput.Validate currently accepts optional filters (FundingMethod, Status, Currency) but those filters are ignored downstream; to fail fast, update ListInput.Validate (the Validate method on the ListInput type) to reject unsupported filters by appending validation errors when FundingMethod, Status, or Currency are non-nil (e.g., errors.New("funding method filter not supported"), errors.New("status filter not supported"), errors.New("currency filter not supported")), then return them via models.NewNillableGenericValidationError(errors.Join(errs...)); this ensures callers get a clear validation error instead of silently ignored filters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/v3/api.gen.go`:
- Around line 8830-8832: Add the same guard used by other credit endpoints to
the CreateCreditAdjustment handler: detect if s.customersCreditsHandler == nil
and if so call unimplemented.CreateCreditAdjustment(w, r, customerId) and
return; modify the CreateCreditAdjustment implementation (the handler that
currently unconditionally calls unimplemented.CreateCreditAdjustment) to perform
this nil-check before delegating to s.customersCreditsHandler so the
credits.enabled feature flag gates the endpoint consistently.
---
Outside diff comments:
In `@api/v3/api.gen.go`:
- Around line 310-323: The generated enum docs for
BillingCreditAvailabilityPolicy are stale—remove the references to the removed
values ("on_authorization", "on_settlement") from the OpenAPI/TypeSpec source
where BillingCreditAvailabilityPolicy is defined, ensure the enum definition
only contains the current member (BillingCreditAvailabilityPolicyOnCreation) and
update its description accordingly, then regenerate api.gen.go so the docblock
and enum values stay in sync and the file contains the required "// Code
generated by ..." header; do not edit api.gen.go directly.
---
Nitpick comments:
In `@openmeter/billing/creditgrant/service.go`:
- Around line 144-186: ListInput.Validate currently accepts optional filters
(FundingMethod, Status, Currency) but those filters are ignored downstream; to
fail fast, update ListInput.Validate (the Validate method on the ListInput type)
to reject unsupported filters by appending validation errors when FundingMethod,
Status, or Currency are non-nil (e.g., errors.New("funding method filter not
supported"), errors.New("status filter not supported"), errors.New("currency
filter not supported")), then return them via
models.NewNillableGenericValidationError(errors.Join(errs...)); this ensures
callers get a clear validation error instead of silently ignored filters.
🪄 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: a311604d-4393-4a84-9acf-9febf3eb8a31
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (8)
api/spec/packages/aip/src/customers/credits/grant.tspapi/v3/api.gen.goapi/v3/handlers/customers/credits/convert.goapi/v3/handlers/customers/credits/get_grant.goapi/v3/server/routes.goopenmeter/billing/creditgrant/service.goopenmeter/billing/creditgrant/service/service.goopenmeter/ledger/chargeadapter/creditpurchase.go
🚧 Files skipped from review as they are similar to previous changes (2)
- api/spec/packages/aip/src/customers/credits/grant.tsp
- api/v3/handlers/customers/credits/get_grant.go
| r.Group(func(r chi.Router) { | ||
| r.Post(options.BaseURL+"/openmeter/customers/{customerId}/credits/adjustments", wrapper.CreateCreditAdjustment) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that CreateCreditAdjustment is protected by credits-enabled checks
# in non-generated server/handler layers.
rg -n -C4 --type=go '\bCreateCreditAdjustment\b'
echo "---- searching for credits feature flags/guards ----"
rg -n -C4 --type=go 'credits\.enabled|CreditsEnabled|EnableCredits|credit.*enabled'Repository: openmeterio/openmeter
Length of output: 11784
🏁 Script executed:
rg -n -C6 --type=go '\bGetCustomerCreditBalance\b|\bListCreditGrants\b|\bCreateCreditGrant\b' api/v3/server/routes.goRepository: openmeterio/openmeter
Length of output: 1761
🏁 Script executed:
rg -n -C10 --type=go 'credits.*enabled|Credits.*Enabled' api/v3/server/routes.go | head -50Repository: openmeterio/openmeter
Length of output: 47
🏁 Script executed:
# Look for how credit-related routes are mounted and check for any middleware
rg -n -B5 -A5 --type=go 'openmeter/customers.*credits' api/v3/api.gen.go | head -80Repository: openmeterio/openmeter
Length of output: 4505
🏁 Script executed:
rg -n --type=go 'customersCreditsHandler' api/v3/server/*.go | head -30Repository: openmeterio/openmeter
Length of output: 1445
🏁 Script executed:
rg -n --type=go 'customersCreditsHandler.*=' | grep -v testRepository: openmeterio/openmeter
Length of output: 661
🏁 Script executed:
# Check the server initialization to see how credits handler is wired
fd -e go 'server' api/v3 | xargs grep -l 'NewServer\|customersCreditsHandler' | head -5Repository: openmeterio/openmeter
Length of output: 88
🏁 Script executed:
# Check the condition around customersCreditsHandler initialization
sed -n '215,230p' api/v3/server/server.goRepository: openmeterio/openmeter
Length of output: 2060
🏁 Script executed:
# Check if CreateCreditAdjustment has the nil check that other credit handlers have
sed -n '372,375p' api/v3/server/routes.goRepository: openmeterio/openmeter
Length of output: 226
🏁 Script executed:
# Let's see more context around where customersCreditsHandler is initialized
sed -n '210,260p' api/v3/server/server.goRepository: openmeterio/openmeter
Length of output: 3520
Add the credits.enabled guard to CreateCreditAdjustment like other credit handlers
CreateCreditAdjustment at api/v3/server/routes.go:372 unconditionally calls unimplemented.CreateCreditAdjustment(), unlike other credit endpoints (GetCustomerCreditBalance, ListCreditGrants, etc.) that first check if s.customersCreditsHandler == nil. The customersCreditsHandler is only initialized when config.Credits.Enabled is true, so this nil-check is the defensive guard that gates the feature.
Add the same pattern:
if s.customersCreditsHandler == nil {
unimplemented.CreateCreditAdjustment(w, r, customerId)
return
}This ensures consistency across all credit operations and maintains the multi-layer guarding described in your implementation approach.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/v3/api.gen.go` around lines 8830 - 8832, Add the same guard used by other
credit endpoints to the CreateCreditAdjustment handler: detect if
s.customersCreditsHandler == nil and if so call
unimplemented.CreateCreditAdjustment(w, r, customerId) and return; modify the
CreateCreditAdjustment implementation (the handler that currently
unconditionally calls unimplemented.CreateCreditAdjustment) to perform this
nil-check before delegating to s.customersCreditsHandler so the credits.enabled
feature flag gates the endpoint consistently.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes