Skip to content

Preserve native catalog currencies - #90

Open
VasiliyRad wants to merge 3 commits into
mainfrom
codex/native-currency-costs
Open

VasiliyRad wants to merge 3 commits into
mainfrom
codex/native-currency-costs

Conversation

@VasiliyRad

@VasiliyRad VasiliyRad commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Addresses #65.

Scope

Adds native-currency support to metergraph-core: a price row may declare a three-letter currency code, and core carries the computed native amount and currency through catalog resolution and billing. Non-USD amounts never populate fields named *_usd, and no currency conversion is performed.

This PR does not add end-to-end non-USD cost tracking to the MeterGraph server or dashboard. Server persistence, mixed-currency aggregation, API representation, and UI display remain out of scope. The bundled catalog remains USD-only; native-currency behavior is available to core consumers and custom catalogs.

Compatibility

Existing USD behavior and legacy CostResult construction remain supported. resolve_billing() also accepts the legacy result shape that exposes cost_usd without the new cost and currency attributes, treating that representation as USD.

cost_status == "priced" now means a native-currency amount is available; callers that specifically require dollars must check cost_usd.

Validation

  • env PYTHONPATH=core/src:server/src pytest core/tests server/tests -q (862 passed, 12 skipped)
  • Added regression coverage for legacy billing-result compatibility, USD behavior, non-USD preservation, contradictory cost states, and USD gateway evidence beside a non-USD catalog price.

@dhyabi2 dhyabi2 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for asking. I read the source changes in billing.py, catalog.py and loader.py and the docs; I did not run the suite. The shape is what #65 asked for: the amount keeps its currency, and nothing non-USD lands in a field named *_usd. Four things I would look at before merging, in order of how much they could surprise an existing caller.

1. cost_status == "priced" no longer implies cost_usd is not None. In resolve_billing the catalog branch changed from catalog_result.cost_usd is not None to catalog_result.cost is not None. For a non-USD row that yields cost_status="priced", cost_provenance="catalog" and cost_usd=None. Any caller that today does if d.cost_status == "priced": total += d.cost_usd was correct before this PR and breaks after the first non-USD row is added to a catalog. The bundled catalog has none, so nothing breaks today, but it is the one compatibility change in the PR and it is silent. Either a distinct status for "priced, not in USD", or one sentence in the BillingDecision docstring and the changelog saying the implication no longer holds, would cover it.

2. CostResult accepts contradictory states. __post_init__ mirrors cost_usd into cost/currency only when both are unset. CostResult(..., cost_usd=Decimal("1"), currency="EUR") is accepted as written: cost stays None while cost_usd holds a number labelled EUR. Since the point of the PR is that a native amount can never be read as dollars, I would reject cost_usd is not None and currency not in (None, "USD"), and cost_usd != cost when both are set.

3. Gateway USD beside a non-USD catalog row. The comment says "the two are never compared", but the discrepancy computation is above the hunk and not in the diff. If it still reads catalog_result.cost_usd, it is safe by accident (it is None); if it is ever switched to catalog_result.cost, it compares dollars with another currency. A test with a reported USD cost and a EUR catalog row asserting cost_discrepancy_status is None would pin that.

4. The docs say ISO 4217; the loader checks three letters. _CURRENCY_CODE accepts any three letters, so ZZZ loads. That may be intended (it keeps the loader free of a currency table), in which case I would say "a three-letter code" in docs/prices.md rather than "ISO 4217". Related, and not a blocker: _COST_QUANTUM is eight decimal places, which suits every fiat currency but rounds to zero for a unit with more precision than that.

Smaller: the document-level currency must still be USD, so a catalog that is wholly in another currency remains impossible and every row must say so itself. The docs do state this; I mention it only because it is the first thing someone with a non-USD catalog will try.

@VasiliyRad

Copy link
Copy Markdown
Collaborator Author

Addressed in 7e9122a:

  1. Documented that cost_status == "priced" means a native-currency cost is available and does not guarantee cost_usd; callers requiring dollars must check cost_usd.
  2. Enforced the CostResult representation invariant. USD construction is normalized across the legacy and generic fields, while mismatched USD amounts, a non-USD currency beside cost_usd, and half-specified native amounts are rejected.
  3. The reported-USD plus EUR-catalog case was already covered by test_gateway_usd_wins_over_a_native_catalog_amount_without_comparison, including cost_discrepancy_status is None; that behavior remains unchanged.
  4. Changed the docs and source comment to say "three-letter currency code" rather than claim ISO 4217 validation.

The document-level USD default remains intentional for this additive change; rows may override it individually. Full verification: env PYTHONPATH=core/src:server/src pytest core/tests server/tests -q produced 861 passed, 12 skipped.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants