Conversation
📝 WalkthroughWalkthroughAdds a new customer charges API surface (models, operations, index, and endpoint wiring), introduces billing totals and shared enums/properties, adds subscription reference and proration/settlement types, adds an unimplemented Go route, and removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as API Gateway
participant Server as Server (routes)
participant Spec as CustomerChargesOperations
participant Service as Billing Service
participant DB as Database
Client->>API: GET /openmeter/customers/{id}/charges?...
API->>Server: forward request
Server->>Spec: ListCustomerCharges(params)
Spec->>Service: fetch charges (pagination, filter, expand)
Service->>DB: query charges and totals
DB-->>Service: charge rows, totals
Service-->>Spec: compose PagePaginatedResponse<Charge>
Spec-->>Server: return response (or NotFound/Error)
Server-->>API: HTTP response
API-->>Client: 200/404/4xx
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 |
95a5465 to
4b8e1e8
Compare
ac8d2b3 to
c9151e6
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
api/spec/packages/aip/src/shared/properties.tsp (1)
169-180: Use a non-empty interval in the examplesNice addition overall. Tiny docs nit: Line 170 and Line 179 use the same instant, which models an empty period. Using a later
toexample would make the intended semantics clearer.Suggested tweak
`@summary`("End") - `@example`(DateTime.fromISO("2023-01-01T01:01:01.001Z")) + `@example`(DateTime.fromISO("2023-02-01T01:01:01.001Z")) to: DateTime;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/spec/packages/aip/src/shared/properties.tsp` around lines 169 - 180, The examples for the DateTime interval use the same instant making the period empty; update the `@example` for the to property (and/or the from property) so to is a later instant than from (e.g., change DateTime.fromISO("2023-01-01T01:01:01.001Z") for to to a later timestamp) to demonstrate a non-empty interval for the from and to fields.
🤖 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/spec/packages/aip/src/customers/charges/charges.tsp`:
- Around line 193-197: The doc comment above the model ChargeTotals incorrectly
describes it as “A usage-based charge for a customer.” Update the comment to
accurately describe this type as billing totals (e.g., "Billing totals for a
customer" or "Aggregated charge totals for a customer") so the JSDoc/description
matches the model name ChargeTotals; edit the comment block immediately
preceding model ChargeTotals to reflect this wording change.
In `@api/spec/packages/aip/src/customers/charges/operations.tsp`:
- Around line 23-30: Update the API docs to match the current charge status set
by removing the obsolete `settled` entry from the supported statuses list in the
customers/charges operations documentation block; ensure the list now shows only
`created`, `active`, `final`, and `deleted` and that the following sentence ("If
omitted, all statuses are returned except for `deleted`.") remains accurate or
is adjusted if the implementation changed.
In `@api/spec/packages/aip/src/productcatalog/ratecard.tsp`:
- Around line 12-21: The SettlementMode enum is missing the backend-supported
value "invoice_only"; update the enum definition (SettlementMode) to add a
corresponding member (e.g., InvoiceOnly: "invoice_only") and extend the
comment/docs list to include the `invoice_only` description ("Usage is invoiced
only") so the API matches openmeter/productcatalog/settlementmode.go and
conversion logic remains consistent.
---
Nitpick comments:
In `@api/spec/packages/aip/src/shared/properties.tsp`:
- Around line 169-180: The examples for the DateTime interval use the same
instant making the period empty; update the `@example` for the to property (and/or
the from property) so to is a later instant than from (e.g., change
DateTime.fromISO("2023-01-01T01:01:01.001Z") for to to a later timestamp) to
demonstrate a non-empty interval for the from and to fields.
🪄 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: 232e0945-9582-4015-a8bd-08290f68918f
⛔ Files ignored due to path filters (19)
api/client/python/openmeter/_generated/_client.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/_configuration.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/_utils/model_base.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/_client.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/_configuration.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/operations/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/operations/_operations.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_enums.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_models.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/operations/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_types.pyis excluded by!api/client/**api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/chargecreditpurchase/chargecreditpurchase.gois excluded by!**/ent/db/**openmeter/ent/db/chargeflatfee/chargeflatfee.gois excluded by!**/ent/db/**openmeter/ent/db/chargessearchv1/chargessearchv1.gois excluded by!**/ent/db/**openmeter/ent/db/chargeusagebased/chargeusagebased.gois excluded by!**/ent/db/**openmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**
📒 Files selected for processing (17)
api/spec/packages/aip/src/billing/index.tspapi/spec/packages/aip/src/billing/totals.tspapi/spec/packages/aip/src/customers/charges/charges.tspapi/spec/packages/aip/src/customers/charges/index.tspapi/spec/packages/aip/src/customers/charges/operations.tspapi/spec/packages/aip/src/customers/index.tspapi/spec/packages/aip/src/konnect.tspapi/spec/packages/aip/src/openmeter.tspapi/spec/packages/aip/src/productcatalog/ratecard.tspapi/spec/packages/aip/src/shared/enums.tspapi/spec/packages/aip/src/shared/index.tspapi/spec/packages/aip/src/shared/properties.tspapi/spec/packages/aip/src/subscriptions/index.tspapi/spec/packages/aip/src/subscriptions/reference.tspapi/v3/api.gen.goapi/v3/server/routes.goopenmeter/billing/charges/meta/charge.go
💤 Files with no reviewable changes (1)
- openmeter/billing/charges/meta/charge.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/spec/packages/aip/src/customers/charges/charges.tsp`:
- Around line 193-215: Fix the two doc issues in the ChargeTotals model: correct
the typo in the top comment from "change" to "charge" and update the expand
reference from `realtime_usage` to match the enum name `real_time_usage` (as
defined in the ChargesExpand enum) so the doc and enum stay consistent; locate
the model `ChargeTotals` and update its comment text and the `realtime` field's
doc string accordingly.
🪄 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: a0e2987c-8179-4240-93af-92a7649bddbe
📒 Files selected for processing (2)
api/spec/packages/aip/src/customers/charges/charges.tspapi/spec/packages/aip/src/customers/charges/operations.tsp
🚧 Files skipped from review as they are similar to previous changes (1)
- api/spec/packages/aip/src/customers/charges/operations.tsp
| /** | ||
| * The totals of a change. | ||
| * | ||
| * RealTime is only expanded when the `real_time_usage` expand is used. | ||
| */ | ||
| @friendlyName("BillingChargeTotals") | ||
| model ChargeTotals { | ||
| /** | ||
| * The amount of the charge already booked to the internal accounting system. | ||
| */ | ||
| @visibility(Lifecycle.Read) | ||
| @summary("Booked") | ||
| booked: Billing.BillingTotals; | ||
|
|
||
| /** | ||
| * The realtime amount of the charge. | ||
| * | ||
| * Requires the `realtime_usage` expand. | ||
| */ | ||
| @visibility(Lifecycle.Read) | ||
| @summary("Realtime totals") | ||
| realtime?: Billing.BillingTotals; | ||
| } |
There was a problem hiding this comment.
Typo and expand name mismatch in ChargeTotals.
A couple small issues here:
- Line 194: "The totals of a change" should be "The totals of a charge"
- Line 210: The doc references
realtime_usagebut theChargesExpandenum on line 274 defines it asreal_time_usage(with an underscore betweenrealandtime)
Suggested fix
/**
- * The totals of a change.
+ * The totals of a charge.
*
* RealTime is only expanded when the `real_time_usage` expand is used.
*/
`@friendlyName`("BillingChargeTotals")
model ChargeTotals {
/**
* The amount of the charge already booked to the internal accounting system.
*/
`@visibility`(Lifecycle.Read)
`@summary`("Booked")
booked: Billing.BillingTotals;
/**
* The realtime amount of the charge.
*
- * Requires the `realtime_usage` expand.
+ * Requires the `real_time_usage` expand.
*/
`@visibility`(Lifecycle.Read)
`@summary`("Realtime totals")
realtime?: Billing.BillingTotals;
}📝 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.
| /** | |
| * The totals of a change. | |
| * | |
| * RealTime is only expanded when the `real_time_usage` expand is used. | |
| */ | |
| @friendlyName("BillingChargeTotals") | |
| model ChargeTotals { | |
| /** | |
| * The amount of the charge already booked to the internal accounting system. | |
| */ | |
| @visibility(Lifecycle.Read) | |
| @summary("Booked") | |
| booked: Billing.BillingTotals; | |
| /** | |
| * The realtime amount of the charge. | |
| * | |
| * Requires the `realtime_usage` expand. | |
| */ | |
| @visibility(Lifecycle.Read) | |
| @summary("Realtime totals") | |
| realtime?: Billing.BillingTotals; | |
| } | |
| /** | |
| * The totals of a charge. | |
| * | |
| * RealTime is only expanded when the `real_time_usage` expand is used. | |
| */ | |
| `@friendlyName`("BillingChargeTotals") | |
| model ChargeTotals { | |
| /** | |
| * The amount of the charge already booked to the internal accounting system. | |
| */ | |
| `@visibility`(Lifecycle.Read) | |
| `@summary`("Booked") | |
| booked: Billing.BillingTotals; | |
| /** | |
| * The realtime amount of the charge. | |
| * | |
| * Requires the `real_time_usage` expand. | |
| */ | |
| `@visibility`(Lifecycle.Read) | |
| `@summary`("Realtime totals") | |
| realtime?: Billing.BillingTotals; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/spec/packages/aip/src/customers/charges/charges.tsp` around lines 193 -
215, Fix the two doc issues in the ChargeTotals model: correct the typo in the
top comment from "change" to "charge" and update the expand reference from
`realtime_usage` to match the enum name `real_time_usage` (as defined in the
ChargesExpand enum) so the doc and enum stay consistent; locate the model
`ChargeTotals` and update its comment text and the `realtime` field's doc string
accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
api/spec/packages/aip/src/shared/properties.tsp (1)
162-181:ClosedPeriodnaming is a bit misleading for[from, to)semantics.Nice addition overall, but Line 162 names this as
ClosedPeriodwhile Lines 167 and 176 document a half-open range (inclusive start, exclusive end). That can confuse SDK users. Consider renaming to something likeHalfOpenPeriod(or making both ends inclusive if you want “closed” semantics).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/spec/packages/aip/src/shared/properties.tsp` around lines 162 - 181, The model name ClosedPeriod conflicts with the documented semantics ([from, to) = inclusive start, exclusive end); rename the type to a clear name like HalfOpenPeriod (or alternatively change the JSDoc/comments to describe a truly closed range) and update all references/usages and any generated exports accordingly; specifically edit the model declaration ClosedPeriod and its fields from and to, update the `@friendlyName` and any imports/usages across the codebase and SDK generation configs so consumers see the accurate API surface and examples.
🤖 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/spec/packages/aip/src/customers/charges/operations.tsp`:
- Around line 31-33: The status filter's equality field currently uses a
free-form string; change the type of filter.status.oeq from string to the
ChargeStatus enum so callers are constrained to the valid values (created,
active, final, deleted). Update the declaration where status?: { oeq: string }
appears to use oeq: ChargeStatus and ensure any related imports or references to
ChargeStatus in the same file remain correct (e.g., the ChargeStatus enum
definition and any serialization/validation logic that depends on it).
---
Nitpick comments:
In `@api/spec/packages/aip/src/shared/properties.tsp`:
- Around line 162-181: The model name ClosedPeriod conflicts with the documented
semantics ([from, to) = inclusive start, exclusive end); rename the type to a
clear name like HalfOpenPeriod (or alternatively change the JSDoc/comments to
describe a truly closed range) and update all references/usages and any
generated exports accordingly; specifically edit the model declaration
ClosedPeriod and its fields from and to, update the `@friendlyName` and any
imports/usages across the codebase and SDK generation configs so consumers see
the accurate API surface and examples.
🪄 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: c1541681-fd2b-4dee-a537-f0e905baafe4
⛔ Files ignored due to path filters (20)
api/client/javascript/src/zod/index.tsis excluded by!api/client/**api/client/python/openmeter/_generated/_client.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/_configuration.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/_utils/model_base.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/_client.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/_configuration.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/operations/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/operations/_operations.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_enums.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_models.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/operations/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_types.pyis excluded by!api/client/**api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/chargecreditpurchase/chargecreditpurchase.gois excluded by!**/ent/db/**openmeter/ent/db/chargeflatfee/chargeflatfee.gois excluded by!**/ent/db/**openmeter/ent/db/chargessearchv1/chargessearchv1.gois excluded by!**/ent/db/**openmeter/ent/db/chargeusagebased/chargeusagebased.gois excluded by!**/ent/db/**openmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**
📒 Files selected for processing (18)
api/spec/packages/aip/src/billing/index.tspapi/spec/packages/aip/src/billing/totals.tspapi/spec/packages/aip/src/customers/charges/charges.tspapi/spec/packages/aip/src/customers/charges/index.tspapi/spec/packages/aip/src/customers/charges/operations.tspapi/spec/packages/aip/src/customers/index.tspapi/spec/packages/aip/src/konnect.tspapi/spec/packages/aip/src/openmeter.tspapi/spec/packages/aip/src/productcatalog/ratecard.tspapi/spec/packages/aip/src/shared/enums.tspapi/spec/packages/aip/src/shared/index.tspapi/spec/packages/aip/src/shared/properties.tspapi/spec/packages/aip/src/subscriptions/index.tspapi/spec/packages/aip/src/subscriptions/reference.tspapi/v3/api.gen.goapi/v3/handlers/customers/credits/convert.goapi/v3/server/routes.goopenmeter/billing/charges/meta/charge.go
💤 Files with no reviewable changes (1)
- openmeter/billing/charges/meta/charge.go
✅ Files skipped from review due to trivial changes (8)
- api/spec/packages/aip/src/shared/index.tsp
- api/spec/packages/aip/src/customers/index.tsp
- api/spec/packages/aip/src/shared/enums.tsp
- api/spec/packages/aip/src/subscriptions/index.tsp
- api/spec/packages/aip/src/customers/charges/index.tsp
- api/spec/packages/aip/src/subscriptions/reference.tsp
- api/spec/packages/aip/src/productcatalog/ratecard.tsp
- api/spec/packages/aip/src/billing/totals.tsp
🚧 Files skipped from review as they are similar to previous changes (4)
- api/spec/packages/aip/src/billing/index.tsp
- api/v3/handlers/customers/credits/convert.go
- api/spec/packages/aip/src/openmeter.tsp
- api/spec/packages/aip/src/customers/charges/charges.tsp
| status?: { | ||
| oeq: string; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify generated API params typing for customer charges filter/status
rg -n -C4 'type ListCustomerChargesParams' api/v3/api.gen.go
rg -n -C4 'status|oeq|Filter' api/v3/api.gen.goRepository: openmeterio/openmeter
Length of output: 33655
🏁 Script executed:
cat -n api/spec/packages/aip/src/customers/charges/operations.tsp | head -80Repository: openmeterio/openmeter
Length of output: 2337
🏁 Script executed:
# Check what ChargeStatus is defined as
rg -n "ChargeStatus" api/spec/packages/aip/src/customers/charges/Repository: openmeterio/openmeter
Length of output: 322
🏁 Script executed:
# Also check the charges.tsp file mentioned in the review comment
cat -n api/spec/packages/aip/src/customers/charges/charges.tsp | head -100Repository: openmeterio/openmeter
Length of output: 3044
Use ChargeStatus enum for filter.status.oeq instead of free-form string.
The filter currently accepts any string, but the ChargeStatus enum (defined in the same file) provides a closed set of valid statuses (created, active, final, deleted). This makes generated SDKs looser than they should be—they'll happily accept invalid status values instead of constraining to the enum. Switch oeq: string to oeq: ChargeStatus to match the documentation and keep the API contract tight.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/spec/packages/aip/src/customers/charges/operations.tsp` around lines 31 -
33, The status filter's equality field currently uses a free-form string; change
the type of filter.status.oeq from string to the ChargeStatus enum so callers
are constrained to the valid values (created, active, final, deleted). Update
the declaration where status?: { oeq: string } appears to use oeq: ChargeStatus
and ensure any related imports or references to ChargeStatus in the same file
remain correct (e.g., the ChargeStatus enum definition and any
serialization/validation logic that depends on it).
Overview
Add charges list endpint.
Notes for reviewer
Summary by CodeRabbit