feat(api): Add tax_id and tax_behavior to charges list api

[mark-vass-konghq]

@coderabbitai

Summary by CodeRabbit

• New Features

  • Optional tax configuration for charges: users can set tax behavior (inclusive/exclusive) and attach a tax code to flat-fee and usage-based charges. Charges without a configuration will show no tax info.

• Tests

  • Added tests ensuring tax configuration is correctly mapped, converted, persisted, and returned in listings across charge types, including cases with behavior-only, code-only, both, or none.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6e287902-75e2-4985-bf07-a827313d67f6

📥 Commits

Reviewing files that changed from the base of the PR and between 36c02f9 and 72d2bd6.

📒 Files selected for processing (1)

• openmeter/billing/charges/service/taxcode_test.go

---

📝 Walkthrough

Walkthrough

Adds an optional charge-level tax configuration model and surfaces it through the API and conversion layer; updates generated Go types and adds unit/integration tests validating mapping and persistence for flat-fee and usage-based charges.

Changes

Cohort / File(s)	Summary
TypeSpec api/spec/packages/aip/src/customers/charges/charges.tsp	Adds ChargeTaxConfig (friendlyName BillingChargeTaxConfig) and an optional tax_config?: ChargeTaxConfig field on ChargeBase.
Generated API types api/v3/api.gen.go	Adds BillingChargeTaxConfig type and wires optional TaxConfig into BillingFlatFeeCharge and BillingUsageBasedCharge (including Swagger spec updates).
Handler conversion api/v3/handlers/customers/charges/convert.go	Introduces convertTaxCodeConfigToAPI and maps internal tax behavior and tax_code id into the API TaxConfig during charge conversion.
Conversion unit tests api/v3/handlers/customers/charges/convert_test.go	Table-driven tests for convertTaxCodeConfigToAPI: nil/empty handling, enum mapping, and tax code ULID mapping scenarios.
Persistence/integration test openmeter/billing/charges/service/taxcode_test.go	Adds test verifying ListCharges returns tax_config for created charges (flat-fee and usage-based) and preserves nil when absent; uses pagination and ExpandRealizations.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant API_Handler as Handler
  participant Converter
  participant DB
  participant API_Response as Response

  Client->>API_Handler: Request charge(s) list or charge entity
  API_Handler->>DB: Query charge records (include tax config fields)
  DB-->>API_Handler: Return charge records (may include tax config)
  API_Handler->>Converter: convert internal model -> API model (calls convertTaxCodeConfigToAPI)
  Converter-->>API_Handler: API charge objects with optional BillingChargeTaxConfig
  API_Handler-->>Response: Send JSON response to Client
  Response-->>Client: JSON payload includes optional tax_config

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat(charges): add tax code #4237 — Adds and wires charge-level tax_config/tax_code across API and persistence (strong overlap in fields and mapping).
• feat(productcatalog): expose taxCodeId in taxConfig API #4224 — Introduces/updates tax-code identifier (ULID) support and API mapping for charge tax config.
• feat(api): add charges list api http handler #4103 — Related changes to charges conversion/handlers that overlap with the conversion wiring in this PR.

Suggested reviewers

• turip
• tothandras

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title accurately reflects the main change: adding tax configuration fields to the charges API, though it uses slightly different naming (tax_id vs tax_code and tax_behavior vs tax_behavior).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/add-tax_id-and-tax-behaviour-to-charges-list-api

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

openmeter/billing/charges/service/taxcode_test.go (1)

784-819: ⚡ Quick win

Use separate tax config values for each seeded charge.

Both intents currently share the same taxConfig pointer. If any create/backfill path mutates it in place, the two assertions become coupled and this test can hide a bug. Duplicating the tiny struct per intent keeps the fixture isolated.

Small fixture tweak

-	taxConfig := &productcatalog.TaxCodeConfig{
-		Behavior:  lo.ToPtr(productcatalog.InclusiveTaxBehavior),
-		TaxCodeID: &tc.ID,
-	}
+	flatFeeTaxConfig := &productcatalog.TaxCodeConfig{
+		Behavior:  lo.ToPtr(productcatalog.InclusiveTaxBehavior),
+		TaxCodeID: &tc.ID,
+	}
+	usageBasedTaxConfig := &productcatalog.TaxCodeConfig{
+		Behavior:  lo.ToPtr(productcatalog.InclusiveTaxBehavior),
+		TaxCodeID: &tc.ID,
+	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@openmeter/billing/charges/service/taxcode_test.go` around lines 784 - 819,
The test currently reuses the same taxConfig pointer for both mock intents which
couples their state; create a separate productcatalog.TaxCodeConfig instance for
each intent (e.g., taxConfigFlat for "flat-fee-list-taxcode" and taxConfigUsage
for "usage-based-list-taxcode") and pass those distinct pointers into the
respective s.createMockChargeIntent calls so each seeded charge has its own
independent taxConfig.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@openmeter/billing/charges/service/taxcode_test.go`:
- Around line 784-819: The test currently reuses the same taxConfig pointer for
both mock intents which couples their state; create a separate
productcatalog.TaxCodeConfig instance for each intent (e.g., taxConfigFlat for
"flat-fee-list-taxcode" and taxConfigUsage for "usage-based-list-taxcode") and
pass those distinct pointers into the respective s.createMockChargeIntent calls
so each seeded charge has its own independent taxConfig.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 63afa3b9-27bb-4332-8a08-2f83b470c64f

📥 Commits

Reviewing files that changed from the base of the PR and between 0da1145 and 36c02f9.

⛔ Files ignored due to path filters (1)

• api/v3/openapi.yaml is excluded by !**/openapi.yaml

📒 Files selected for processing (5)

• api/spec/packages/aip/src/customers/charges/charges.tsp
• api/v3/api.gen.go
• api/v3/handlers/customers/charges/convert.go
• api/v3/handlers/customers/charges/convert_test.go
• openmeter/billing/charges/service/taxcode_test.go