refactor(billing): use fiat currency for invoices#4774
Conversation
📝 WalkthroughWalkthroughThis PR narrows billing and Stripe currency contracts from ChangesFiat currency contracts
Charge invoice-currency resolution
Currency grouping and validation
Estimated code review effort: 3 (Moderate) | ~25 minutes 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.
Actionable comments posted: 3
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/usagebased/service/create.go (1)
121-142: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftConvert custom-currency amounts before fiat invoice-line construction.
Both builders switch the line currency to the resolved fiat code without converting the existing amount from the source charge currency. That can produce mispriced invoices for custom-currency charges.
openmeter/billing/charges/usagebased/service/create.go#L121-L142: convertintent.Priceusing the cost-basis rate before assigninginvoiceCurrency.openmeter/billing/charges/flatfee/service/create.go#L198-L225: convertamountAfterProrationbefore constructingPricewithinvoiceCurrency.🤖 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/usagebased/service/create.go` around lines 121 - 142, Convert custom-currency amounts to the resolved fiat currency using the cost-basis rate before constructing invoice lines: in openmeter/billing/charges/usagebased/service/create.go:121-142, convert intent.Price before assigning invoiceCurrency in the GatheringLine; in openmeter/billing/charges/flatfee/service/create.go:198-225, convert amountAfterProration before constructing Price with invoiceCurrency. Preserve existing pricing and proration behavior apart from the required currency conversion.
🤖 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/billing/charges/models/costbasis/intent.go`:
- Around line 133-160: Update GetFiatCurrency to validate the FiatCurrency
pointer returned by the ModeDynamic, ModePinned, and ModeManual intent variants
before returning success; return an appropriate validation error when it is nil,
while preserving existing conversion errors and valid non-nil results.
In `@openmeter/billing/charges/service/usagebased_costbasis_test.go`:
- Around line 143-144: Extend the assertions in the usage-based cost-basis tests
around GatheringLineToCreate to verify the converted usage amounts, not only the
USD currency. Cover custom-currency cases using the resolved manual or pinned
cost-basis rate, and define the expected amount for the dynamic-rate case so the
generated fiat lines validate the conversion.
In `@openmeter/billing/charges/usagebased/service/linemapper.go`:
- Around line 134-136: Update the line-mapping flow around AsFiatCurrency to
apply the custom-currency conversion rate to detailed amounts, credits, and
expected totals before WithCreditsApplied and RoundToPrecision; otherwise reject
custom-currency input instead of emitting incorrectly labeled fiat amounts.
---
Outside diff comments:
In `@openmeter/billing/charges/usagebased/service/create.go`:
- Around line 121-142: Convert custom-currency amounts to the resolved fiat
currency using the cost-basis rate before constructing invoice lines: in
openmeter/billing/charges/usagebased/service/create.go:121-142, convert
intent.Price before assigning invoiceCurrency in the GatheringLine; in
openmeter/billing/charges/flatfee/service/create.go:198-225, convert
amountAfterProration before constructing Price with invoiceCurrency. Preserve
existing pricing and proration behavior apart from the required currency
conversion.
🪄 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 Plus
Run ID: c0eed596-327e-4776-ae8b-ea4f11941f20
⛔ Files ignored due to path filters (14)
openmeter/ent/db/billinggatheringinvoiceline.gois excluded by!**/ent/db/**openmeter/ent/db/billinggatheringinvoiceline/where.gois excluded by!**/ent/db/**openmeter/ent/db/billinggatheringinvoiceline_create.gois excluded by!**/ent/db/**openmeter/ent/db/billinginvoice.gois excluded by!**/ent/db/**openmeter/ent/db/billinginvoice/where.gois excluded by!**/ent/db/**openmeter/ent/db/billinginvoice_create.gois excluded by!**/ent/db/**openmeter/ent/db/billinginvoiceline.gois excluded by!**/ent/db/**openmeter/ent/db/billinginvoiceline/where.gois excluded by!**/ent/db/**openmeter/ent/db/billinginvoiceline_create.gois excluded by!**/ent/db/**openmeter/ent/db/billinginvoicesplitlinegroup.gois excluded by!**/ent/db/**openmeter/ent/db/billinginvoicesplitlinegroup/where.gois excluded by!**/ent/db/**openmeter/ent/db/billinginvoicesplitlinegroup_create.gois excluded by!**/ent/db/**openmeter/ent/db/entmixinaccessor.gois excluded by!**/ent/db/**openmeter/ent/db/mutation.gois excluded by!**/ent/db/**
📒 Files selected for processing (61)
openmeter/app/stripe/calculator.goopenmeter/app/stripe/client/invoice.goopenmeter/billing/charges/creditpurchase/service/create.goopenmeter/billing/charges/flatfee/charge.goopenmeter/billing/charges/flatfee/service/create.goopenmeter/billing/charges/flatfee/service/linemapper.goopenmeter/billing/charges/flatfee/service/manualedit.goopenmeter/billing/charges/invoiceupdater/invoiceupdate.goopenmeter/billing/charges/models/costbasis/intent.goopenmeter/billing/charges/service/create.goopenmeter/billing/charges/service/creditpurchase_test.goopenmeter/billing/charges/service/flatfee_costbasis_test.goopenmeter/billing/charges/service/invoicable_test.goopenmeter/billing/charges/service/invoice.goopenmeter/billing/charges/service/pendinglines.goopenmeter/billing/charges/service/pendinglines_test.goopenmeter/billing/charges/service/taxcode_test.goopenmeter/billing/charges/service/usagebased_costbasis_test.goopenmeter/billing/charges/usagebased/charge.goopenmeter/billing/charges/usagebased/service/create.goopenmeter/billing/charges/usagebased/service/lineengine_test.goopenmeter/billing/charges/usagebased/service/linemapper.goopenmeter/billing/charges/usagebased/service/linemapper_test.goopenmeter/billing/charges/usagebased/service/run/payment_test.goopenmeter/billing/gatheringinvoice.goopenmeter/billing/httpdriver/invoice.goopenmeter/billing/httpdriver/invoice_test.goopenmeter/billing/httpdriver/invoiceline.goopenmeter/billing/invoiceline.goopenmeter/billing/invoicelinesplitgroup.goopenmeter/billing/sequence/sequence.goopenmeter/billing/sequence/service/service.goopenmeter/billing/sequence/service/service_test.goopenmeter/billing/service/gatheringinvoicependinglines.goopenmeter/billing/service/invoicecalc/collectionat_test.goopenmeter/billing/service/invoiceupdate.goopenmeter/billing/service/invoiceupdate_test.goopenmeter/billing/service/stdinvoiceline.goopenmeter/billing/stdinvoice.goopenmeter/billing/stdinvoiceline.goopenmeter/billing/stdinvoiceline_test.goopenmeter/billing/worker/subscriptionsync/service/reconciler/invoiceupdater/invoiceupdate.goopenmeter/billing/worker/subscriptionsync/service/targetstate/targetstateitem.goopenmeter/ent/schema/billing.goopenmeter/notification/internal/rule.gotest/app/custominvoicing/invocing_test.gotest/app/stripe/invoice_credits_test.gotest/app/stripe/invoice_test.gotest/billing/adapter_test.gotest/billing/collection_test.gotest/billing/discount_test.gotest/billing/invoice_test.gotest/billing/lineengine_test.gotest/billing/schemamigration_test.gotest/billing/suite.gotest/billing/tax_test.gotest/billing/taxcode_dual_write_test.gotest/billing/unitconfig_legacy_test.gotest/credits/credit_then_invoice_test.gotest/credits/sanity_test.gotest/customer/subject.go
| // GetFiatCurrency returns the fiat currency in which the custom-currency cost | ||
| // basis is expressed, regardless of the selected resolution mode. | ||
| func (i Intent) GetFiatCurrency() (*currencyx.FiatCurrency, error) { | ||
| switch i.kind { | ||
| case ModeDynamic: | ||
| intent, err := i.AsDynamic() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return intent.FiatCurrency, nil | ||
| case ModePinned: | ||
| intent, err := i.AsPinned() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return intent.FiatCurrency, nil | ||
| case ModeManual: | ||
| intent, err := i.AsManual() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return intent.FiatCurrency, nil | ||
| default: | ||
| return nil, models.NewGenericValidationError(fmt.Errorf("invalid intent kind: %s", i.kind)) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching intent/costbasis:"
git ls-files | rg '(^|/)openmeter/billing/charges/models/costbasis/intent\.go$|costbasis' || true
echo
echo "Relevant intent outline:"
ast-grep outline openmeter/billing/charges/models/costbasis/intent.go --view expanded || true
echo
echo "Interest locations in intent.go:"
sed -n '1,240p' openmeter/billing/charges/models/costbasis/intent.go
echo
echo "Search GetFiatCurrency/getFiatCode/get invoicesCurrency:"
rg -n "GetFiatCurrency|GetFiatCode|GetInvoiceCurrency|fiatCurrency|FiatCurrency" openmeter/billing/charges -SRepository: openmeterio/openmeter
Length of output: 23791
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search GetFiatCurrency/getFiatCode/get InvoiceCurrency:"
rg -n "GetFiatCurrency|GetFiatCode|GetInvoiceCurrency|fiatCurrency|FiatCurrency" openmeter/billing/charges -S
echo
echo "Search all repository usages:"
rg -n "GetFiatCurrency|GetFiatCode|GetInvoiceCurrency" . -SRepository: openmeterio/openmeter
Length of output: 18434
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "costbasis model relevant sections:"
sed -n '1,280p' openmeter/billing/charges/models/costbasis/model.go
echo
echo "currencyx fiat validation relevant sections:"
sed -n '1,260p' pkg/currencyx/costbasis.go
sed -n '150,210p' pkg/currencyx/currency.go
echo
echo "flatfee charge invoice path:"
sed -n '75,110p' openmeter/billing/charges/flatfee/charge.go
echo
echo "usagebased charge invoice path:"
sed -n '80,110p' openmeter/billing/charges/usagebased/charge.go
echo
echo "read-only nil-pointer behavior probe:"
cat > /tmp/probe.go <<'GO'
package main
type FiatCode string
type FiatCurrency struct{ code FiatCode }
func (f *FiatCurrency) GetFiatCode() FiatCode { return f.code }
type Mode string
type DynamicIntent struct{ FiatCurrency *FiatCurrency }
type PinnedIntent struct{ FiatCurrency *FiatCurrency }
type ManualIntent struct{ FiatCurrency *FiatCurrency }
type Intent struct{ kind Mode; dynamic *DynamicIntent; pinned *PinnedIntent; manual *ManualIntent }
func (i Intent) AsDynamic() (DynamicIntent, error) {
if i.dynamic == nil { return DynamicIntent{}, nil }
return *i.dynamic, nil
}
func (i Intent) AsPinned() (PinnedIntent, error) {
if i.pinned == nil { return PinnedIntent{}, nil }
return *i.pinned, nil
}
func (i Intent) AsManual() (ManualIntent, error) {
if i.manual == nil { return ManualIntent{}, nil }
return *i.manual, nil
}
func (i Intent) GetFiatCurrency() (*FiatCurrency, error) {
switch i.kind {
case "dynamic":
intent, err := i.AsDynamic()
if err != nil { return nil, err }
return intent.FiatCurrency, nil
case "pinned":
intent, err := i.AsPinned()
if err != nil { return nil, err }
return intent.FiatCurrency, nil
case "manual":
intent, err := i.AsManual()
if err != nil { return nil, err }
return intent.FiatCurrency, nil
default:
return nil, nil
}
}
func GetInvoiceCurrency(i Intent) (FiatCode, string) {
costBasisIntent, err := i.GetFiatCurrency()
if err != nil { return "", "error" }
return costBasisIntent.GetFiatCode(), "ok"
}
func main() {
code, status := GetInvoiceCurrency(Intent{kind: "dynamic"})
println("code=", string(code), "status=", status)
_ = GetInvoiceCurrency(Intent{kind: "dynamic"}) // this could panic
}
GO
run_go="$(command -v go || true)"
if [ -n "$run_go" ]; then
go test /tmp/probe.go
else
cat /tmp/probe.go
fiRepository: openmeterio/openmeter
Length of output: 11902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Intent currency accessor implementations:"
rg -n "func \\(i Intent\\) (GetCurrency|GetCostBasisIntent|GetCode|FiatCurrency|CostBasisIntent)|AsCurrency|costBasisIntent" openmeter/billing/charges/models/costbasis openmeter/billing/charges/currencies -S || true
echo
echo "Models/costbasis remaining outline:"
ast-grep outline openmeter/billing/charges/models/costbasis/*.go --view expanded || true
echo
echo "CostBasisIntent definition usages:"
rg -n "type CostBasisIntent|CostBasisIntent" openmeter/billing/charges/models openmeter/billing/charges/currencies -S || true
echo
echo "Read-only nil-pointer behavior probe:"
cat > /tmp/probe.go <<'GO'
package main
import (
"errors"
"fmt"
)
type FiatCode string
type FiatCurrency struct{ code FiatCode }
type Mode string
func main() {
var ptr *FiatCurrency
fmt.Printf("value %#v error=%v panic=%v\n", nil)
_, err := fmt.Sprint(ptr)
fmt.Println("print err:", err != nil && errors.Is(err, (*runtime.Error)(nil).Error()) || err != nil)
}
GO
run_go="$(command -v go || true)"
if [ -n "$run_go" ]; then
go build -o /tmp/probe /tmp/probe.go && /tmp/probe
else
go doc fmt.Sprint
fiRepository: openmeterio/openmeter
Length of output: 5354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Intent methods/usages beyond current files:"
rg -n "\.GetFiatCurrency\(|GetFiatCurrency\(|FiatCurrency\." openmeter -S --glob '*.go'
echo
rg -n "type Intent|func \(i Intent\)|func \(c ChargeBase\)" openmeter -S --glob 'openmeter/billing/charges/**/*.go' || true
echo
echo "ChargeBase methods in relevant charge files:"
sed -n '1,130p' openmeter/billing/charges/flatfee/charge.go
echo "----"
sed -n '1,130p' openmeter/billing/charges/usagebased/charge.goRepository: openmeterio/openmeter
Length of output: 14491
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Cost model fiat code schema:"
rg -n "FiatCurrency|fiatCurrency|FiatCurrencyValidator" openmeter/ent openmeter/billing/charges -S --glob '*.go' | head -n 220
echo
echo "Schema files:"
for f in $(rg -l "FiatCurrency" openmeter/ent/schema -S --glob '*.go'); do
echo "--- $f"
sed -n '1,220p' "$f"
done
echo
echo "Read-only nil validation behavior probe:"
cat > /tmp/probe.go <<'GO'
package main
import (
"errors"
"fmt"
)
func Validate(f interface{}) error {
if f == nil {
return errors.New("fiat currency is not initialized")
}
return nil
}
func main() {
var ptr *struct{}
if err := Validate(ptr); err != nil {
fmt.Println("nil Validate result:", err)
}
fmt.Println("done")
}
GO
run_go="$(command -v go || true)"
if [ -n "$run_go" ]; then
go run /tmp/probe.go || true
else
cat /tmp/probe.go
fiRepository: openmeterio/openmeter
Length of output: 28755
Guard the fiat-c currency pointer before dereferencing it.
GetFiatCurrency() can return (nil, nil) for malformed intent data, while the invoice-currency paths call fiatCurrency.GetFiatCode() immediately. Either reject a nil fiat currency here in one place or make the non-nil invariant explicit at construction/persistence and guarded at this accessor.
🤖 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/models/costbasis/intent.go` around lines 133 - 160,
Update GetFiatCurrency to validate the FiatCurrency pointer returned by the
ModeDynamic, ModePinned, and ModeManual intent variants before returning
success; return an appropriate validation error when it is nil, while preserving
existing conversion errors and valid non-nil results.
| s.Require().NotNil(result.GatheringLineToCreate, "charge index %d", idx) | ||
| s.Require().Equal(currencyx.FiatCode("USD"), result.GatheringLineToCreate.Currency, "charge index %d", idx) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Also assert the converted usage amount.
These checks prove that the line is denominated in USD, but they would still pass if custom-currency usage were copied into the fiat line without applying the resolved manual or pinned cost-basis rate. Assert the generated amounts for those cases (and define the dynamic case) so this billing regression cannot slip through.
🤖 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/usagebased_costbasis_test.go` around lines
143 - 144, Extend the assertions in the usage-based cost-basis tests around
GatheringLineToCreate to verify the converted usage amounts, not only the USD
currency. Cover custom-currency cases using the resolved manual or pinned
cost-basis rate, and define the expected amount for the dynamic-rate case so the
generated fiat lines validate the conversion.
Source: Path instructions
| cur, err := stdLine.Currency.AsFiatCurrency() | ||
| if err != nil { | ||
| return fmt.Errorf("creating currency calculator: %w", err) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Apply the cost-basis conversion before building the fiat invoice line.
AsFiatCurrency() only resolves the destination currency definition. The later mapping copies detailed-line amounts and Run.Totals without applying a custom-currency conversion rate, so custom-currency usage can be labeled in the resolved fiat currency with incorrect amounts. Convert detailed amounts, credits, and expected totals before WithCreditsApplied and RoundToPrecision, or reject this path until conversion is available.
🤖 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/usagebased/service/linemapper.go` around lines 134
- 136, Update the line-mapping flow around AsFiatCurrency to apply the
custom-currency conversion rate to detailed amounts, credits, and expected
totals before WithCreditsApplied and RoundToPrecision; otherwise reject
custom-currency input instead of emitting incorrectly labeled fiat amounts.
Summary
currencyx.FiatCodeacross billing domain objects and persistenceFiatCodeto a calculator only at calculation boundaries and regenerate Ent codeWhy
Gathering and Standard invoices are settled in fiat currency. Using the broader
currencyx.Codetype allowed custom-currency codes to flow into invoice persistence and calculation even though those paths require fiat semantics.Impact
Invoice and invoice-line callers now get compile-time enforcement that the persisted currency is fiat. Custom-currency charges continue to invoice in the fiat currency selected by their cost basis. The physical database columns remain
varchar(3), so no SQL migration is required.Validation
make lint-go-fastmake test-nocache— all code-related tests passed; the existing notification integration test could not connect to the local Svix service at127.0.0.1:8071git diff --checkGreptile Summary
Refactors billing invoices and invoice lines to carry fiat-only currency codes.
Confidence Score: 3/5
This PR should not merge until custom-currency usage totals are converted with their resolved cost basis before being represented as fiat invoice amounts.
Usage rating retains the custom-currency price and calculator, while the changed gathering line carries a fiat code and the resulting custom-unit totals flow into the standard invoice without rate conversion.
openmeter/billing/charges/usagebased/service/create.go, openmeter/billing/charges/usagebased/rating.go, openmeter/billing/charges/usagebased/service/linemapper.go
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Custom-currency usage charge] --> B[Cost-basis intent] B --> C[Fiat invoice currency] A --> D[Rating with custom price and calculator] C --> E[Gathering invoice] D --> F[Realization run totals] E --> G[Standard invoice line] F --> GPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "refactor(billing): use fiat currency for..." | Re-trigger Greptile
Context used (3)
Summary by CodeRabbit