feat(customcurrencies): support flat fee credit then invoice - #4794
Conversation
📝 WalkthroughWalkthroughFlat-fee custom currencies now create overage placeholders, convert overage to fiat during line mapping, reconcile mutable runs against intents, accrue fiat transactions through a new callback, and pass fiat amounts through payment authorization and settlement. Integration and adapter tests cover the updated lifecycle. ChangesCustom-currency flat-fee lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Charge
participant FlatFeeLineMapper
participant AccrueInvoiceUsage
participant FlatFeeHandler
Charge->>FlatFeeLineMapper: Convert custom-currency overage to fiat
FlatFeeLineMapper->>AccrueInvoiceUsage: provide mapped fiat overage line
AccrueInvoiceUsage->>FlatFeeHandler: OnCustomCurrencyOverageAccrued
FlatFeeHandler-->>AccrueInvoiceUsage: return fiat total and transaction group
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openmeter/billing/charges/flatfee/service/realizations/credittheninvoice.go (1)
271-279: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
ReconcileRunToIntentskips the custom-currency fiat check thatStartCreditThenInvoiceRunnow does.
StartCreditThenInvoiceRun(Lines 146-154) derivesNoFiatTransactionRequiredfrom the converted fiat amount for custom currencies, but reconciliation still usesrunTotals.Total.IsZero()in the charge currency. After a shrink/extend on a custom-currency charge whose remaining overage converts to zero fiat (the exact "fiat overage rounds to zero" case covered ininvoicable_test.go), the run keepsNoFiatTransactionRequired = falsewhilepopulateCustomCurrencyOverageFromRunmaps the line to a zero fiat total.AccrueInvoiceUsageInput.Validateinrealizations/invoiceaccrued.go(Lines 52-54) then rejects that combination and invoice issuing fails.Worth extracting the derivation so both paths stay in sync.
🐛 Suggested fix
runTotals := detailedLines.SumTotals().RoundToPrecision(currency) + noFiatTransactionRequired, err := noFiatTransactionRequiredForRun(in.Charge, currency, runTotals) + if err != nil { + return ReconcileRunToIntentResult{}, err + } runBase, err := s.adapter.UpdateRealizationRun(ctx, flatfee.UpdateRealizationRunInput{ ID: run.ID, ServicePeriod: mo.Some(rateableIntent.ServicePeriod), AmountAfterProration: mo.Some(rateableIntent.AmountAfterProration), Totals: mo.Some(runTotals), - NoFiatTransactionRequired: mo.Some(runTotals.Total.IsZero()), + NoFiatTransactionRequired: mo.Some(noFiatTransactionRequired), })with the shared helper (also used by
StartCreditThenInvoiceRun):func noFiatTransactionRequiredForRun(charge flatfee.Charge, currency currencies.Currency, runTotals totals.Totals) (bool, error) { if runTotals.Total.IsZero() { return true, nil } if !currency.IsCustom() { return false, nil } fiatOverage, err := charge.ConvertCustomCurrencyOverageToFiat(runTotals) if err != nil { return false, fmt.Errorf("converting custom currency overage to fiat: %w", err) } return fiatOverage.Amount.IsZero(), nil }🤖 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/billing/charges/flatfee/service/realizations/credittheninvoice.go` around lines 271 - 279, Extract the shared no-fiat derivation used by StartCreditThenInvoiceRun into a helper such as noFiatTransactionRequiredForRun, handling zero totals, non-custom currencies, and custom-currency conversion errors. Update both StartCreditThenInvoiceRun and ReconcileRunToIntent to use it, replacing the reconciliation check based only on runTotals.Total.IsZero() while preserving conversion error propagation.
🧹 Nitpick comments (3)
openmeter/billing/charges/flatfee/service/create.go (1)
189-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the overage line naming rule. Both the gathering placeholder and the realized fiat overage line build the display name with the same
"overage"/"%s (overage)"logic. Since the two names must stay identical for the placeholder-to-realization transition to look coherent, a single helper (e.g.overageLineName(name string) stringin the flat-fee service package) keeps them from drifting.
openmeter/billing/charges/flatfee/service/create.go#L189-L192: replace the inline name derivation with a call to the shared helper.openmeter/billing/charges/flatfee/service/linemapper.go#L149-L153: use the same helper fornamebefore assigningstdLine.Nameand the detailed line name.🤖 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/billing/charges/flatfee/service/create.go` around lines 189 - 192, The overage display-name logic is duplicated between the gathering placeholder and realized fiat line. Add a shared overageLineName helper in the flat-fee service package, then replace the inline derivation in openmeter/billing/charges/flatfee/service/create.go lines 189-192 and use the helper for name before assigning stdLine.Name and the detailed line name in openmeter/billing/charges/flatfee/service/linemapper.go lines 149-153.openmeter/billing/charges/flatfee/handler.go (1)
111-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider validating the fiat currency here too.
Validate()checks the custom currency and the resolved cost basis, but notGetFiatCurrency()— yet both the ledger adapter and the test handler call it right afterValidate()succeeds. Folding it in keeps the "input is fully usable after Validate" contract intact.♻️ Suggested addition
if _, err := i.GetCostBasis(); err != nil { errs = append(errs, fmt.Errorf("cost basis: %w", err)) } + + if _, err := i.GetFiatCurrency(); err != nil { + errs = append(errs, fmt.Errorf("fiat currency: %w", err)) + }🤖 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/billing/charges/flatfee/handler.go` around lines 111 - 139, Update OnCustomCurrencyOverageAccruedInput.Validate to also call GetFiatCurrency().Validate(), appending any failure with the “fiat currency” context before returning the validation error, so callers can safely use the resolved fiat currency after validation succeeds.openmeter/billing/charges/service/base_test.go (1)
283-291: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNice helper — could lean on
Annotations.GetStringhere.The other overage assertions in
invoicable_test.go(e.g. lines 323-325) useline.Annotations.GetString(billing.AnnotationKeyReason), which already handles the string extraction. Using it here drops the*stringbranch and keeps the assertions consistent.♻️ Suggested simplification
- switch reason := in.line.Annotations[billing.AnnotationKeyReason].(type) { - case string: - s.Equal(billing.AnnotationValueReasonOverage, reason) - case *string: - s.Require().NotNil(reason) - s.Equal(billing.AnnotationValueReasonOverage, *reason) - default: - s.Fail("overage reason annotation has an unexpected type") - } + reason, ok := in.line.Annotations.GetString(billing.AnnotationKeyReason) + s.Require().True(ok, "overage reason annotation is missing or not a string") + s.Equal(billing.AnnotationValueReasonOverage, reason)🤖 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/billing/charges/service/base_test.go` around lines 283 - 291, In the overage reason assertion, replace the manual type switch on in.line.Annotations with Annotations.GetString(billing.AnnotationKeyReason), then assert the returned value equals billing.AnnotationValueReasonOverage. Keep the assertion behavior consistent with the existing overage checks in invoicable_test.go.
🤖 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.
Outside diff comments:
In `@openmeter/billing/charges/flatfee/service/realizations/credittheninvoice.go`:
- Around line 271-279: Extract the shared no-fiat derivation used by
StartCreditThenInvoiceRun into a helper such as noFiatTransactionRequiredForRun,
handling zero totals, non-custom currencies, and custom-currency conversion
errors. Update both StartCreditThenInvoiceRun and ReconcileRunToIntent to use
it, replacing the reconciliation check based only on runTotals.Total.IsZero()
while preserving conversion error propagation.
---
Nitpick comments:
In `@openmeter/billing/charges/flatfee/handler.go`:
- Around line 111-139: Update OnCustomCurrencyOverageAccruedInput.Validate to
also call GetFiatCurrency().Validate(), appending any failure with the “fiat
currency” context before returning the validation error, so callers can safely
use the resolved fiat currency after validation succeeds.
In `@openmeter/billing/charges/flatfee/service/create.go`:
- Around line 189-192: The overage display-name logic is duplicated between the
gathering placeholder and realized fiat line. Add a shared overageLineName
helper in the flat-fee service package, then replace the inline derivation in
openmeter/billing/charges/flatfee/service/create.go lines 189-192 and use the
helper for name before assigning stdLine.Name and the detailed line name in
openmeter/billing/charges/flatfee/service/linemapper.go lines 149-153.
In `@openmeter/billing/charges/service/base_test.go`:
- Around line 283-291: In the overage reason assertion, replace the manual type
switch on in.line.Annotations with
Annotations.GetString(billing.AnnotationKeyReason), then assert the returned
value equals billing.AnnotationValueReasonOverage. Keep the assertion behavior
consistent with the existing overage checks in invoicable_test.go.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 127df9ab-5b3d-4ee9-bfbb-364067fa81cb
📒 Files selected for processing (18)
openmeter/billing/charges/flatfee/charge.goopenmeter/billing/charges/flatfee/handler.goopenmeter/billing/charges/flatfee/service/create.goopenmeter/billing/charges/flatfee/service/creditheninvoice.goopenmeter/billing/charges/flatfee/service/lineengine.goopenmeter/billing/charges/flatfee/service/linemapper.goopenmeter/billing/charges/flatfee/service/payment.goopenmeter/billing/charges/flatfee/service/payment_test.goopenmeter/billing/charges/flatfee/service/realizations/credittheninvoice.goopenmeter/billing/charges/flatfee/service/realizations/invoiceaccrued.goopenmeter/billing/charges/service/base_test.goopenmeter/billing/charges/service/gathering_preview_test.goopenmeter/billing/charges/service/handlers_test.goopenmeter/billing/charges/service/invoicable_test.goopenmeter/billing/charges/service/usagebased_test.goopenmeter/billing/charges/testutils/handlers.goopenmeter/ledger/chargeadapter/flatfee.goopenmeter/ledger/chargeadapter/flatfee_test.go
💤 Files with no reviewable changes (1)
- openmeter/billing/charges/flatfee/service/payment_test.go
Summary
Add custom-currency support to flat-fee charges using credit-then-invoice settlement.
Flat-fee realization runs and credit allocations remain denominated in the charge currency. Any uncovered value is converted with the charge's persisted cost basis and represented on the invoice as a single fiat overage line. Gathering previews remain zero-value scheduling placeholders until a draft invoice realization exists.
Charge-managed custom-currency lines reject API-originated updates and deletes so invoice edits cannot diverge from the charge lifecycle. Accrual validation is retry-safe: invalid ledger results persist neither accrued usage nor payment, and the invoice retry action completes normally after the ledger recovers.
The real ledger implementation for custom-currency overage booking remains a separate integration concern; this change defines and exercises the charge-side lifecycle through the mocked ledger handler.
Validation
POSTGRES_HOST=127.0.0.1 go test -tags=dynamic -count=1 ./openmeter/billing/charges/servicePOSTGRES_HOST=127.0.0.1 go test -tags=dynamic -count=1 ./openmeter/ledger/chargeadapterPOSTGRES_HOST=127.0.0.1 go test -tags=dynamic -count=1 ./test/creditsmake lint-go-fastSummary by CodeRabbit
Greptile Summary
This PR adds custom-currency support for credit-then-invoice flat-fee charges.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains; the previously reported zero-fiat reconciliation defect is corrected by deriving transaction necessity from the rounded fiat overage, with focused rerating coverage.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Billing participant Charge as Flat-fee charge participant Credits as Credit ledger participant Invoice Billing->>Charge: Start realization Charge->>Credits: Allocate charge-currency credits Credits-->>Charge: Uncovered custom-currency total Charge->>Charge: Convert overage using persisted cost basis Charge->>Invoice: Populate one fiat overage line Invoice->>Charge: Accrue and process payment when fiat total is positiveReviews (2): Last reviewed commit: "fix(flatfee): preserve zero-fiat reconci..." | Re-trigger Greptile