From 8a0547a8a07f0d4b5faed5625ee441d789579273 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 15:42:15 +0200 Subject: [PATCH 01/18] feat(pricing): add is_usable_rate, one definition of a billable rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rate is usable only when it is finite and non-negative. Zero is usable — "free" is a real price — but NaN, ±inf and negatives are not prices at all. Python's truthiness cannot answer this question: inf and NaN are both truthy, and NaN > 0, NaN < 0 and NaN == 0 are all False, so a malformed rate passes every `if rate:` and every comparison-based guard. Stating the test once means every caller answers it identically. Co-Authored-By: Claude Opus 5 --- routstr/payment/models.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/routstr/payment/models.py b/routstr/payment/models.py index 3deedb5a..3cd4de83 100644 --- a/routstr/payment/models.py +++ b/routstr/payment/models.py @@ -1,5 +1,6 @@ import asyncio import json +import math import random import httpx @@ -58,6 +59,29 @@ class Pricing(BaseModel): max_cost: float = 0.0 # in sats not msats +def is_usable_rate(rate: float) -> bool: + """True if a single billable rate is a number a request could be billed on. + + The one definition of a usable rate, so every guard that asks the question + answers it identically. A rate qualifies only when it is finite and + non-negative; zero is usable (it means "free", which is a real price) but + ``NaN``, ``±inf`` and negatives are not prices at all. + + Non-finite: ``inf > 0`` is True, so an infinite rate reads as chargeable and + would be served, routed and billed as ``inf``; ``NaN`` poisons every total it + enters and defeats ordinary comparisons, since ``NaN > 0``, ``NaN < 0`` and + ``NaN == 0`` are all False. Negative: a negative rate produces a negative + cost, which the settlement path subtracts from the balance — it pays the + caller to make requests. + + Both reach a stored row from upstream catalogs as well as the admin edge + (``json.loads`` accepts the bare ``NaN``/``Infinity`` literals and overflows + ``1e999`` to ``inf``), so the check belongs in one shared place rather than + at each writer. + """ + return math.isfinite(rate) and rate >= 0.0 + + class TopProvider(BaseModel): context_length: int | None = None max_completion_tokens: int | None = None From dbb9df13f205c854596bb06e68baf00c6c64ef5b Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 15:42:15 +0200 Subject: [PATCH 02/18] fix(pricing): keep an unusable rate out of the money math Prices reach the node from upstream catalogs, an operator's admin edit, a legacy database row and the BTC/USD feed. json.loads accepts the bare NaN and Infinity literals and overflows 1e999 to inf, so any of those sources can deliver a value that is not a price. Three guards let one through: - The token-rate gate tested truthiness, so NaN and inf reached the token math and raised ValueError/OverflowError *after* the response was served. The streaming handlers swallow that, so the request went unbilled. A negative rate produced a negative charge, which settlement subtracts from the balance. - An upstream-reported cost component was clamped with max(0.0, ...), which passes inf and NaN through. A non-finite component poisoned the proportional split in _calculate_from_usd_cost (inf / inf is NaN); the exception was absorbed by the broad handler around the USD path, so a request whose total was perfectly valid fell through to token estimation and was billed a fraction of what the upstream charged. Each spelling is now coerced before the fallback chooses between them, so a malformed first field cannot win the `or` and hide the usable figure beside it. - An exchange quote that was zero, negative or non-finite was accepted as the node's BTC/USD price, repricing every model on the node. Each now declines to price rather than billing a nonsensical amount. Co-Authored-By: Claude Opus 5 --- routstr/payment/cost_calculation.py | 58 +++- routstr/payment/price.py | 72 ++++- tests/unit/test_pricing_rate_validation.py | 317 +++++++++++++++++++++ 3 files changed, 424 insertions(+), 23 deletions(-) create mode 100644 tests/unit/test_pricing_rate_validation.py diff --git a/routstr/payment/cost_calculation.py b/routstr/payment/cost_calculation.py index 86495728..a51f633d 100644 --- a/routstr/payment/cost_calculation.py +++ b/routstr/payment/cost_calculation.py @@ -201,13 +201,14 @@ async def calculate_cost( cost_details = usage_data.get("cost_details", {}) if not isinstance(cost_details, dict): cost_details = {} - input_usd = _coerce_usd( - cost_details.get("input_cost") - or cost_details.get("upstream_inference_prompt_cost") + # Coerce each spelling before choosing between them: `inf` and `NaN` + # are truthy, so a malformed first field would otherwise win the + # fallback and the usable figure beside it would never be read. + input_usd = _coerce_usd(cost_details.get("input_cost")) or _coerce_usd( + cost_details.get("upstream_inference_prompt_cost") ) - output_usd = _coerce_usd( - cost_details.get("output_cost") - or cost_details.get("upstream_inference_completions_cost") + output_usd = _coerce_usd(cost_details.get("output_cost")) or _coerce_usd( + cost_details.get("upstream_inference_completions_cost") ) cache_pricing_rates: tuple[float, float, float, float] | None = None if cache_read_tokens > 0 or cache_creation_tokens > 0: @@ -267,9 +268,22 @@ async def calculate_cost( else: input_rate, output_rate, cache_read_rate, cache_creation_rate = pricing_rates - if not (input_rate and output_rate): + # Local import mirrors this module's existing lazy pricing imports. + from .models import is_usable_rate + + # An unusable rate is not "no pricing" to Python's truthiness: `NaN` and a + # negative float are both truthy, so they sailed past this gate — the one + # guard meant to catch a rate that cannot be billed on — and reached the + # token math, which raises `ValueError` on `NaN` and `OverflowError` on + # `inf` *after* the response was served (the streaming handlers swallow + # that, so the request goes unbilled), while a negative produced a negative + # charge. Ask whether each rate is usable rather than whether it is truthy. + rates = (input_rate, output_rate, cache_read_rate, cache_creation_rate) + if not all(is_usable_rate(rate) for rate in rates) or not ( + input_rate and output_rate + ): logger.warning( - "No token pricing configured — billing at flat MaxCostData. " + "No usable token pricing — billing at flat MaxCostData. " "Token counts %s in the upstream response but cannot be " "priced; the request will appear in dashboards with the " "raw counts and a fixed max-cost charge.", @@ -279,6 +293,8 @@ async def calculate_cost( "model": response_data.get("model", "unknown"), "input_tokens": input_tokens, "output_tokens": output_tokens, + "input_rate": input_rate, + "output_rate": output_rate, }, ) return MaxCostData( @@ -313,15 +329,35 @@ async def calculate_cost( def _coerce_usd(value: object) -> float: - """Coerce a value to USD float, handling various formats safely.""" + """Coerce an upstream-reported USD figure to a usable amount, else ``0.0``. + + These values come straight off the upstream response, where ``json.loads`` + accepts the bare ``NaN``/``Infinity`` literals and overflows ``1e999`` to + ``inf``. A non-finite figure is not a cost, and letting one through poisoned + the proportional split in ``_calculate_from_usd_cost`` (``inf / inf`` is + ``NaN``): the resulting exception was absorbed by the broad handler around + the USD path, so a request whose *total* cost was perfectly valid fell + through to token-estimated pricing and was billed a fraction of what the + upstream charged. + + ``0.0`` means "no usable figure" to every caller, which is the same thing an + absent field means, so the caller's existing ``> 0`` checks handle it. + """ + # Local import mirrors this module's existing lazy pricing imports. + from .models import is_usable_rate + if value is None or isinstance(value, bool): return 0.0 if not isinstance(value, (int, float, str)): return 0.0 try: - return max(0.0, float(value)) - except (TypeError, ValueError): + # An oversized integer raises OverflowError, not ValueError. + amount = float(value) + except (TypeError, ValueError, OverflowError): return 0.0 + # `is_usable_rate` also rejects negatives, which the previous `max(0.0, …)` + # clamped to zero — same outcome, stated once instead of inline. + return amount if is_usable_rate(amount) else 0.0 def _resolve_usd_cost(usage_data: dict, response_data: dict) -> float: diff --git a/routstr/payment/price.py b/routstr/payment/price.py index ad614322..93ed68e9 100644 --- a/routstr/payment/price.py +++ b/routstr/payment/price.py @@ -12,16 +12,68 @@ BTC_USD_PRICE: float | None = None SATS_USD_PRICE: float | None = None +def _parse_quote(raw: object, exchange: str) -> float | None: + """Coerce an exchange quote to a price, or ``None`` if it is not one. + + Every quote passes through here because the aggregator takes the ``min()`` + of what it collects: an unusable quote does not merely join the sample, it + *wins* it, and the result is the rate every model and every request on the + node is priced at. A zero divides by zero on the USD cost path, ``NaN`` + raises out of the integer conversion in settlement, and a negative rate + produces a negative charge that is credited back to the caller. + + ``is_usable_rate`` is the same predicate the billable-rate guards use, so + "finite and non-negative" has one definition; a quote is stricter still and + must be positive, since a BTC price of zero is a broken feed, not free money. + Imported lazily because ``payment.models`` imports this module. + """ + from .models import is_usable_rate + + # A boolean is a shape change, not a price: `float(True)` is a finite, + # positive 1.0 that passes every numeric guard below and then wins the + # `min()`, pricing the node at one dollar per bitcoin. + if isinstance(raw, bool): + logger.warning( + "Non-numeric price quote — ignoring this exchange", + extra={"exchange": exchange, "quote": repr(raw)}, + ) + return None + + try: + price = float(raw) # type: ignore[arg-type] + except (TypeError, ValueError, OverflowError) as e: + logger.warning( + "Unparseable price quote — ignoring this exchange", + extra={ + "error": str(e), + "error_type": type(e).__name__, + "exchange": exchange, + "quote": repr(raw), + }, + ) + return None + + if not is_usable_rate(price) or price <= 0: + logger.warning( + "Unusable price quote — ignoring this exchange", + extra={"exchange": exchange, "quote": price}, + ) + return None + + return price + + async def _kraken_btc_usd(client: httpx.AsyncClient) -> float | None: """Fetch BTC/USD price from Kraken API.""" api = "https://api.kraken.com/0/public/Ticker?pair=XBTUSD" try: response = await client.get(api) price_data = response.json() - price = float(price_data["result"]["XXBTZUSD"]["c"][0]) - - return price - except (httpx.RequestError, KeyError) as e: + return _parse_quote(price_data["result"]["XXBTZUSD"]["c"][0], "kraken") + except (httpx.RequestError, KeyError, IndexError, TypeError, ValueError) as e: + # A payload whose *shape* changed raises IndexError/TypeError, and a + # non-JSON body raises ValueError; unhandled, one exchange's bad day + # aborted the whole aggregation instead of dropping a single quote. logger.warning( "Kraken API error", extra={ @@ -39,10 +91,8 @@ async def _coinbase_btc_usd(client: httpx.AsyncClient) -> float | None: try: response = await client.get(api) price_data = response.json() - price = float(price_data["data"]["amount"]) - - return price - except (httpx.RequestError, KeyError) as e: + return _parse_quote(price_data["data"]["amount"], "coinbase") + except (httpx.RequestError, KeyError, IndexError, TypeError, ValueError) as e: logger.warning( "Coinbase API error", extra={ @@ -60,10 +110,8 @@ async def _binance_btc_usdt(client: httpx.AsyncClient) -> float | None: try: response = await client.get(api) price_data = response.json() - price = float(price_data["price"]) - - return price - except (httpx.RequestError, KeyError) as e: + return _parse_quote(price_data["price"], "binance") + except (httpx.RequestError, KeyError, IndexError, TypeError, ValueError) as e: logger.warning( "Binance API error", extra={ diff --git a/tests/unit/test_pricing_rate_validation.py b/tests/unit/test_pricing_rate_validation.py new file mode 100644 index 00000000..db2f5100 --- /dev/null +++ b/tests/unit/test_pricing_rate_validation.py @@ -0,0 +1,317 @@ +"""Tests that an unusable rate never reaches the money math. + +A billable rate is usable only when it is finite and non-negative. Prices reach +the node from upstream catalogs, an operator's admin edit, a legacy database row +and the BTC/USD feed, and each of those can deliver ``NaN``, ``±inf`` or a +negative — ``json.loads`` accepts the bare ``NaN``/``Infinity`` literals and +overflows ``1e999`` to ``inf``. + +These tests cover the guards between such a value and a charge: the token-rate +gate that decides a model cannot be priced, the upstream-reported USD cost, the +exchange-rate feed, and the stored-row read path. They assert the node declines +to price the request rather than billing a nonsensical amount or raising after +the response has already been served. +""" + +from __future__ import annotations + +import math +from collections.abc import Iterator +from typing import Any +from unittest.mock import patch + +import pytest + +from routstr.payment.cost_calculation import ( + CostData, + MaxCostData, + calculate_cost, +) +from routstr.payment.models import ( + Architecture, + Model, + Pricing, +) + + +@pytest.fixture(autouse=True) +def patch_sats_usd_price() -> Iterator[None]: + """Pin the exchange rate; these tests are about the rates, not the feed.""" + with patch("routstr.payment.cost_calculation.sats_usd_price", return_value=5.0e-5): + yield + + +def _architecture() -> Architecture: + return Architecture( + modality="text", + input_modalities=["text"], + output_modalities=["text"], + tokenizer="unknown", + instruct_type=None, + ) + + +def _model(sats_pricing: Pricing) -> Model: + return Model( + id="m", + name="m", + created=0, + description="d", + context_length=8192, + architecture=_architecture(), + pricing=Pricing(prompt=1e-06, completion=2e-06), + sats_pricing=sats_pricing, + ) + + +def _usage_response() -> dict[str, Any]: + return {"model": "m", "usage": {"prompt_tokens": 1000, "completion_tokens": 500}} + + +@pytest.mark.parametrize( + "bad_rate", + [float("nan"), float("inf"), -5.0], + ids=["nan", "inf", "negative"], +) +@pytest.mark.asyncio +async def test_unusable_token_rate_falls_back_to_max_cost(bad_rate: float) -> None: + """An unusable configured rate must not be billed on. + + The "no token pricing configured" gate is a truthiness test, and ``NaN`` and + negative floats are both truthy, so an unusable rate passes the guard that + exists to catch it. It then reaches the integer conversion in the token math, + which raises ``ValueError`` for ``NaN`` and ``OverflowError`` for ``inf`` — + after the upstream response has already been served, where the streaming + handlers swallow it and the request goes unbilled. + """ + model = _model(Pricing(prompt=bad_rate, completion=1.0)) + + cost = await calculate_cost(_usage_response(), max_cost=1234, model_obj=model) + + assert isinstance(cost, MaxCostData) + assert cost.total_msats == 1234 + + +@pytest.mark.parametrize( + "junk", [float("inf"), float("nan"), "Infinity"], ids=["inf", "nan", "inf-string"] +) +@pytest.mark.asyncio +async def test_junk_cost_component_still_bills_the_reported_total(junk: Any) -> None: + """A malformed component must not discard the upstream's real total cost. + + ``cost_details`` only splits the total across input and output; the total is + the authoritative billed amount. A non-finite component poisons the + proportional allocation (``inf / inf`` is ``NaN``), which raised out of the + USD path and was swallowed by the broad handler around it — so the request + silently fell through to token-estimated pricing and was billed at a small + fraction of what the upstream actually charged. + """ + model = _model(Pricing(prompt=1e-06, completion=2e-06)) + response = { + "model": "m", + "usage": { + "prompt_tokens": 1000, + "completion_tokens": 500, + "cost": 0.01, + "cost_details": {"input_cost": junk, "output_cost": 0.004}, + }, + } + + cost = await calculate_cost(response, max_cost=9999, model_obj=model) + + assert isinstance(cost, CostData) + # $0.01 at 5.0e-5 USD/sat = 200 sats = 200_000 msats. + assert cost.total_msats == 200000 + assert cost.total_usd == pytest.approx(0.01) + + +@pytest.mark.asyncio +async def test_junk_cost_component_falls_back_to_its_alternate_field() -> None: + """A malformed component must not shadow the field that would have replaced it. + + Each side of the split has two spellings and the second is a fallback for a + missing first. ``inf`` and ``NaN`` are both truthy, so a malformed + ``input_cost`` won that choice before anything checked whether it was a + number, and the usable figure beside it was never read — the input side was + then billed at nothing and the whole total landed on output. + """ + model = _model(Pricing(prompt=1e-06, completion=2e-06)) + response = { + "model": "m", + "usage": { + "prompt_tokens": 1000, + "completion_tokens": 500, + "cost": 0.01, + "cost_details": { + "input_cost": float("inf"), + "upstream_inference_prompt_cost": 0.006, + "output_cost": 0.004, + }, + }, + } + + cost = await calculate_cost(response, max_cost=9999, model_obj=model) + + assert isinstance(cost, CostData) + # $0.01 at 5.0e-5 USD/sat = 200_000 msats, split 0.006 : 0.004. + assert cost.total_msats == 200000 + assert (cost.input_msats, cost.output_msats) == (120000, 80000) + + +@pytest.mark.asyncio +async def test_non_finite_reported_cost_is_not_a_cost() -> None: + """An upstream-reported ``Infinity`` cost is junk, not an infinite charge. + + ``json.loads`` accepts the bare ``Infinity`` literal, so a compromised or + buggy upstream can put one in ``usage.cost``. It must not be treated as a + positive USD cost at all — the request falls through to the node's own token + pricing instead. + """ + model = _model(Pricing(prompt=1e-06, completion=2e-06)) + response = { + "model": "m", + "usage": { + "prompt_tokens": 1000, + "completion_tokens": 500, + "cost": float("inf"), + }, + } + + cost = await calculate_cost(response, max_cost=9999, model_obj=model) + + assert isinstance(cost, CostData) + assert math.isfinite(cost.total_usd) + assert cost.total_msats == 2 + + +class _ExchangeResponse: + def __init__(self, payload: dict[str, Any]) -> None: + self._payload = payload + + def json(self) -> dict[str, Any]: + return self._payload + + +class _ExchangeClient: + """Answers each exchange endpoint with a caller-supplied quote.""" + + def __init__(self, quotes: dict[str, Any]) -> None: + self._quotes = quotes + + async def get(self, url: str) -> _ExchangeResponse: + if "kraken" in url: + return _ExchangeResponse( + {"result": {"XXBTZUSD": {"c": [self._quotes["kraken"]]}}} + ) + if "coinbase" in url: + return _ExchangeResponse({"data": {"amount": self._quotes["coinbase"]}}) + return _ExchangeResponse({"price": self._quotes["binance"]}) + + +class _AsyncCtx: + def __init__(self, client: _ExchangeClient) -> None: + self._client = client + + async def __aenter__(self) -> _ExchangeClient: + return self._client + + async def __aexit__(self, *exc: object) -> bool: + return False + + +@pytest.fixture +def refresh_price_with() -> Iterator[Any]: + """Refresh the node's BTC/USD price from caller-supplied exchange quotes. + + Restores the module's cached price afterwards so one test cannot set the + rate another one bills at. + """ + import routstr.payment.price as price_module + + previous = (price_module.BTC_USD_PRICE, price_module.SATS_USD_PRICE) + + async def _run(quotes: dict[str, Any], last_good: float | None = None) -> None: + price_module.BTC_USD_PRICE = last_good + price_module.SATS_USD_PRICE = ( + None if last_good is None else last_good / 100_000_000 + ) + with patch.object( + price_module.httpx, + "AsyncClient", + lambda *a, **k: _AsyncCtx(_ExchangeClient(quotes)), + ): + await price_module._update_prices() + + yield _run + + price_module.BTC_USD_PRICE, price_module.SATS_USD_PRICE = previous + + +@pytest.mark.parametrize( + "bad_quote", + ["0", "0.00000000", "-1", "NaN", "Infinity", "N/A"], + ids=["zero", "zero-padded", "negative", "nan", "infinity", "non-numeric"], +) +@pytest.mark.asyncio +async def test_unusable_exchange_quote_does_not_set_the_node_price( + bad_quote: str, refresh_price_with: Any +) -> None: + """One exchange returning junk must not set the price the node bills at. + + The feed takes the ``min()`` of the quotes it collects, so an unusable quote + does not merely join the sample — it *wins*, and poisons the rate every model + and every request is priced at until the next refresh. Zero then divides by + zero on the USD path, ``NaN`` raises out of the integer conversion, and a + negative rate produces a negative charge that settlement credits back to the + caller. + + The two healthy quotes must still price the node. + """ + from routstr.payment.price import btc_usd_price + + await refresh_price_with( + {"kraken": bad_quote, "coinbase": "100000.0", "binance": "100000.0"} + ) + + assert btc_usd_price() == pytest.approx(100000.0) + + +@pytest.mark.asyncio +async def test_boolean_exchange_quote_does_not_set_the_node_price( + refresh_price_with: Any, +) -> None: + """A boolean in the price field is a shape change, not a $1 bitcoin. + + ``float(True)`` is ``1.0``, which is finite and positive, so a payload whose + price field turned into a boolean passes every numeric guard — and then + *wins* the ``min()``, pricing the whole node at one dollar per bitcoin. The + node's other coercions all reject ``bool`` before the numeric check for this + reason; this one is the exception. + """ + from routstr.payment.price import btc_usd_price + + await refresh_price_with( + {"kraken": True, "coinbase": "100000.0", "binance": "100000.0"} + ) + + assert btc_usd_price() == pytest.approx(100000.0) + + +@pytest.mark.asyncio +async def test_all_quotes_unusable_keeps_the_last_good_price( + refresh_price_with: Any, +) -> None: + """When every quote is junk the node keeps the last price it trusted. + + Adopting ``0`` or ``NaN`` because it was the only thing on offer would take + out billing for every model at once; skipping the update degrades to a stale + rate, which is the safe direction and what an unreachable exchange already + does. + """ + from routstr.payment.price import btc_usd_price + + await refresh_price_with( + {"kraken": "0", "coinbase": "NaN", "binance": "-3"}, last_good=90000.0 + ) + + assert btc_usd_price() == pytest.approx(90000.0) From 89e6acef4d99bddf2ef128a5d0adbd5da96463ee Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 16:16:15 +0200 Subject: [PATCH 03/18] fix(pricing): treat a non-finite rate as no price wherever pricing is ingested MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `inf > 0` is True, so an infinite rate read as a real price: it was imported, served, routed, and would bill an infinite amount. `NaN` is worse — every comparison with it is False, so it slipped past both the negative and the both-zero guards. Rates arrive from upstream catalogs as well as the admin edge, and `json.loads` accepts the bare `NaN`/`Infinity` literals and overflows `1e999` to `inf`, so the finiteness check goes in the catalog import filter and in each resolver rung that reports a resolved price rather than at one entry point. An oversized integer raises `OverflowError` instead of `ValueError`, so coercion catches that too — unhandled it unwound the whole fetch and cost the node an entire upstream catalog over one junk entry. A malformed *cache* rate costs only the cache rate: it is dropped and billing falls back to the full input rate, which is what a missing cache rate means. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- routstr/payment/models.py | 12 ++- routstr/upstream/pricing_resolver.py | 20 +++- tests/unit/test_pricing_rate_validation.py | 95 ++++++++++++++++++ tests/unit/test_upstream_generic.py | 107 +++++++++++++++++++++ 4 files changed, 228 insertions(+), 6 deletions(-) diff --git a/routstr/payment/models.py b/routstr/payment/models.py index 3cd4de83..dad9bbb5 100644 --- a/routstr/payment/models.py +++ b/routstr/payment/models.py @@ -168,7 +168,7 @@ def backfill_cache_pricing(model_id: str, pricing: Pricing) -> Pricing: def _has_valid_pricing(model: dict) -> bool: - """Check if model has valid pricing (not free, no negative values).""" + """Check if model has valid pricing (usable rates, and not free).""" pricing = model.get("pricing", {}) if not pricing: return False @@ -176,10 +176,16 @@ def _has_valid_pricing(model: dict) -> bool: try: prompt = float(pricing.get("prompt", 0)) completion = float(pricing.get("completion", 0)) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): + # An integer too large for a float raises OverflowError, not + # ValueError, so it escaped this coercion guard and unwound the whole + # fetch — one junk entry cost the node the entire upstream catalog. return False - if prompt < 0 or completion < 0: + # `NaN`/`±inf` are not prices, and neither is caught by the checks below: + # every comparison with `NaN` is False, and `inf` reads as a large positive + # rate that would be advertised and billed on. + if not is_usable_rate(prompt) or not is_usable_rate(completion): return False if prompt == 0 and completion == 0: diff --git a/routstr/upstream/pricing_resolver.py b/routstr/upstream/pricing_resolver.py index 3d009fdc..3064ba57 100644 --- a/routstr/upstream/pricing_resolver.py +++ b/routstr/upstream/pricing_resolver.py @@ -15,6 +15,7 @@ it into the base provider unchanged. from __future__ import annotations +import math from dataclasses import dataclass, field @@ -65,11 +66,19 @@ def estimate_context_length(model_id: str) -> int: def _as_float(value: object) -> float | None: - """OpenRouter reports prices as strings; coerce, ``None`` if unparseable.""" + """OpenRouter reports prices as strings; coerce, ``None`` if not a real number. + + Non-finite values are rejected as unparseable: ``float("Infinity")`` and + ``float("NaN")`` parse happily from a feed string, and ``json.loads`` + accepts the bare literals and overflows ``1e999`` to ``inf``. An oversized + integer raises ``OverflowError`` rather than ``ValueError``, so that is + caught too. + """ try: - return float(value) # type: ignore[arg-type] - except (TypeError, ValueError): + parsed = float(value) # type: ignore[arg-type] + except (TypeError, ValueError, OverflowError): return None + return parsed if math.isfinite(parsed) else None def _as_int(value: object) -> int | None: @@ -94,6 +103,11 @@ def _from_litellm(model_id: str) -> ResolvedPricing | None: # moderation/rerank tiers do this) — treating 0/0 as resolved would serve # the model for free. Reject it (and any negative) so the caller falls # through, mirroring async_fetch_openrouter_models' _has_valid_pricing. + # A non-finite entry is junk, not a price: `inf` would bill an infinite + # amount and `NaN` poisons every total it enters (and defeats the `< 0` and + # `== 0` guards below, since both comparisons are False for `NaN`). + if not math.isfinite(prompt) or not math.isfinite(completion): + return None if prompt < 0 or completion < 0 or (prompt == 0 and completion == 0): return None diff --git a/tests/unit/test_pricing_rate_validation.py b/tests/unit/test_pricing_rate_validation.py index db2f5100..9e4a5d6e 100644 --- a/tests/unit/test_pricing_rate_validation.py +++ b/tests/unit/test_pricing_rate_validation.py @@ -315,3 +315,98 @@ async def test_all_quotes_unusable_keeps_the_last_good_price( ) assert btc_usd_price() == pytest.approx(90000.0) + + +# --------------------------------------------------------------------------- +# Catalog ingest — a malformed rate must never become a stored price +# --------------------------------------------------------------------------- + + +class _CatalogResponse: + def __init__(self, payload: dict[str, Any]) -> None: + self._payload = payload + + def raise_for_status(self) -> None: + return None + + def json(self) -> dict[str, Any]: + return self._payload + + +class _CatalogClient: + """Stands in for ``httpx.AsyncClient`` against the OpenRouter catalog.""" + + def __init__(self, models: list[dict[str, Any]]) -> None: + self._models = models + + async def __aenter__(self) -> "_CatalogClient": + return self + + async def __aexit__(self, *exc: object) -> bool: + return False + + async def get(self, url: str, timeout: int | None = None) -> _CatalogResponse: + if url.endswith("/embeddings/models"): + return _CatalogResponse({"data": []}) + return _CatalogResponse({"data": self._models}) + + +def _catalog_entry(model_id: str, pricing: dict[str, Any]) -> dict[str, Any]: + return {"id": model_id, "name": model_id, "pricing": pricing} + + +def _patch_openrouter_catalog(models: list[dict[str, Any]]) -> Any: + return patch( + "routstr.payment.models.httpx.AsyncClient", + lambda *args, **kwargs: _CatalogClient(models), + ) + + +@pytest.mark.parametrize( + "bad_rate", + [float("nan"), float("inf"), float("-inf")], + ids=["nan", "inf", "negative-inf"], +) +@pytest.mark.asyncio +async def test_non_finite_catalog_rate_is_not_imported(bad_rate: float) -> None: + """A non-finite rate in the upstream catalog is junk, not a price. + + ``json.loads`` accepts the bare ``NaN``/``Infinity`` literals and overflows + ``1e999`` to ``inf``, so an upstream feed can deliver one. The import filter + rejects a negative and a both-zero price, but every comparison with ``NaN`` + is False and ``inf`` reads as a large positive, so both sailed through and + became a stored price the node would advertise and bill on. + """ + with _patch_openrouter_catalog( + [ + _catalog_entry("bad", {"prompt": bad_rate, "completion": "0.000002"}), + _catalog_entry("good", {"prompt": "0.000001", "completion": "0.000002"}), + ] + ): + from routstr.payment.models import async_fetch_openrouter_models + + models = await async_fetch_openrouter_models() + + assert [m["id"] for m in models] == ["good"] + + +@pytest.mark.asyncio +async def test_oversized_catalog_rate_does_not_empty_the_catalog() -> None: + """An integer too large to be a float must cost one model, not all of them. + + ``float()`` raises ``OverflowError`` — not ``ValueError`` — for such a + value, so the coercion guard in the import filter did not catch it and the + exception unwound the whole fetch. The node then imported nothing at all + from an upstream whose catalog was fine apart from one entry. + """ + with _patch_openrouter_catalog( + [ + _catalog_entry("bad", {"prompt": 10**400, "completion": 2}), + _catalog_entry("good", {"prompt": "0.000001", "completion": "0.000002"}), + ] + ): + from routstr.payment.models import async_fetch_openrouter_models + + models = await async_fetch_openrouter_models() + + assert [m["id"] for m in models] == ["good"] diff --git a/tests/unit/test_upstream_generic.py b/tests/unit/test_upstream_generic.py index 39daff5c..bf5316a1 100644 --- a/tests/unit/test_upstream_generic.py +++ b/tests/unit/test_upstream_generic.py @@ -522,3 +522,110 @@ async def test_unresolvable_model_fails_closed( for rec in caplog.records if rec.levelno >= logging.WARNING ) + + +# --------------------------------------------------------------------------- +# rate validation — a malformed rate is not a resolved price, at any rung +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_non_finite_litellm_rate_is_not_a_resolved_price() -> None: + """A non-finite entry in the cost map must not answer the resolution chain. + + The litellm rung rejects negatives and a both-zero entry, but every + comparison with ``NaN`` is False and ``inf`` reads as a large positive, so + either would be reported as a resolved price — enabling the model at a rate + the node cannot bill on. Fail closed instead: the model imports disabled, + which is what "no source knows this price" already means here. + """ + payload = { + "data": [ + {"id": "nan-priced-model", "object": "model", "owned_by": "mystery"}, + ] + } + cost_entry = { + "input_cost_per_token": float("nan"), + "output_cost_per_token": float("inf"), + "max_input_tokens": 8192, + } + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=[]) + with patch("routstr.payment.models.litellm_cost_entry", lambda _id: cost_entry): + with patch("routstr.payment.models.async_fetch_openrouter_models", or_feed): + models = await GenericUpstreamProvider( + base_url="http://x" + ).fetch_models() + + model = _model_by_id(models, "nan-priced-model") + assert model.enabled is False + assert model.pricing.prompt == 0.0 + assert model.pricing.completion == 0.0 + + +@pytest.mark.asyncio +async def test_non_finite_openrouter_rate_is_not_a_resolved_price() -> None: + """``float("Infinity")`` parses happily from a feed string, so the + OpenRouter rung's coercion accepted it and reported an infinite rate as a + resolved price. It is not a price; the model must import disabled.""" + payload = { + "data": [ + {"id": "or-nonfinite-xyz", "object": "model", "owned_by": "mystery"}, + ] + } + feed = [ + { + "id": "or-nonfinite-xyz", + "pricing": {"prompt": "Infinity", "completion": "0.000002"}, + "context_length": 8192, + } + ] + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=feed) + with patch("routstr.payment.models.async_fetch_openrouter_models", or_feed): + models = await GenericUpstreamProvider(base_url="http://x").fetch_models() + + model = _model_by_id(models, "or-nonfinite-xyz") + assert model.enabled is False + assert model.pricing.prompt == 0.0 + assert model.pricing.completion == 0.0 + + +@pytest.mark.asyncio +async def test_non_finite_openrouter_cache_rate_is_dropped_not_carried() -> None: + """A malformed *cache* rate must cost the cache rate, not the model. + + The catalog import filter only inspects prompt and completion, so an entry + with two sound token rates and an unusable ``input_cache_read`` reaches the + resolver intact. Carrying that rate through would price every cached input + token at ``inf``; dropping it falls back to the full input rate, which is + what a missing cache rate already means. + """ + payload = { + "data": [ + {"id": "or-badcache-xyz", "object": "model", "owned_by": "mystery"}, + ] + } + feed = [ + { + "id": "or-badcache-xyz", + "pricing": { + "prompt": "0.000001", + "completion": "0.000002", + "input_cache_read": "Infinity", + }, + "context_length": 8192, + } + ] + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=feed) + with patch("routstr.payment.models.async_fetch_openrouter_models", or_feed): + models = await GenericUpstreamProvider(base_url="http://x").fetch_models() + + model = _model_by_id(models, "or-badcache-xyz") + assert model.enabled is True + assert model.pricing.prompt == pytest.approx(1e-06) + assert model.pricing.input_cache_read == 0.0 From 754f5d1a77353cea2b312684fb6bd151d4c537c9 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 16:21:33 +0200 Subject: [PATCH 04/18] fix(admin): reject a malformed pricing rate at the write edge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The model write endpoints accepted any rate a client sent. A non-numeric string coerced to $0 on the read path, producing an unpriced-looking row that cannot be told from a deliberate free price; a negative or non-finite rate is truthy, so it read back as a real price and the model could be enabled and bill a nonsensical amount. Validate the pricing payload on the carrier all three write endpoints share and answer any present billable rate that is non-numeric, non-finite or negative with a 422. An oversized integer raises OverflowError, which pydantic does not convert into a validation error, so it is caught explicitly — it was reaching the row-to-model read-back and escaping as a 500 after the row had already been written. Numeric strings stay valid: the stored JSON accepts them and the UI round-trips rates through text fields. `BILLABLE_PRICING_FIELDS` names the rates a request can bill on, so the edge and the read-path guards cover the same set. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- routstr/core/admin.py | 39 +++- routstr/payment/models.py | 15 ++ .../test_admin_pricing_rate_validation.py | 211 ++++++++++++++++++ 3 files changed, 263 insertions(+), 2 deletions(-) create mode 100644 tests/integration/test_admin_pricing_rate_validation.py diff --git a/routstr/core/admin.py b/routstr/core/admin.py index 2da3baf1..be58972c 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -6,12 +6,17 @@ from datetime import datetime, timezone from pathlib import Path from fastapi import APIRouter, Depends, HTTPException, Query, Request -from pydantic import BaseModel, RootModel +from pydantic import BaseModel, RootModel, field_validator from pydantic.v1 import ValidationError as PydanticValidationError from sqlmodel import select from sqlmodel.ext.asyncio.session import AsyncSession -from ..payment.models import _row_to_model, list_models +from ..payment.models import ( + BILLABLE_PRICING_FIELDS, + _row_to_model, + is_usable_rate, + list_models, +) from ..proxy import refresh_model_maps, reinitialize_upstreams from ..wallet import fetch_all_balances, send_token, token_mint_url from . import vault @@ -520,6 +525,36 @@ class ModelCreate(BaseModel): enabled: bool = True forwarded_model_id: str | None = None + @field_validator("pricing") + @classmethod + def _validate_pricing(cls, value: dict[str, object]) -> dict[str, object]: + """Reject a malformed, non-finite or negative billable rate at the edge. + + A present-but-invalid rate would otherwise slip through: a non-numeric + string coerces to $0 on the read path (an unpriced-looking row), while a + negative or ``NaN``/``inf`` value is truthy and reads back as a real + price, so the model could be enabled and bill a nonsensical amount. + Surfacing a 422 reports the client bug as a client bug instead of + persisting it. Absent rates and numeric strings (``"0.000005"``) stay + valid — the stored JSON accepts both. + """ + for field in BILLABLE_PRICING_FIELDS: + raw = value.get(field) + if raw is None: + continue + if isinstance(raw, bool) or not isinstance(raw, (int, float, str)): + raise ValueError(f"{field} must be a non-negative number") + try: + rate = float(raw) + except (ValueError, OverflowError): + # An integer too large for a float raises OverflowError, which + # pydantic does not convert into a validation error — unhandled + # it escapes as a 500 for what is still a bad client value. + raise ValueError(f"{field} must be a number, got {raw!r}") + if not is_usable_rate(rate): + raise ValueError(f"{field} must be a finite, non-negative number") + return value + def _normalize_forwarded_model_id(value: str | None) -> str | None: if value is None: diff --git a/routstr/payment/models.py b/routstr/payment/models.py index dad9bbb5..6595d684 100644 --- a/routstr/payment/models.py +++ b/routstr/payment/models.py @@ -59,6 +59,21 @@ class Pricing(BaseModel): max_cost: float = 0.0 # in sats not msats +# The rates a request can bill on. Derived fields (``max_*_cost``) are excluded +# — they are computed carriers, not charged rates. One definition, shared by the +# admin write edge and the served/routed guards, so they all cover the same set. +BILLABLE_PRICING_FIELDS = ( + "prompt", + "completion", + "request", + "image", + "web_search", + "internal_reasoning", + "input_cache_read", + "input_cache_write", +) + + def is_usable_rate(rate: float) -> bool: """True if a single billable rate is a number a request could be billed on. diff --git a/tests/integration/test_admin_pricing_rate_validation.py b/tests/integration/test_admin_pricing_rate_validation.py new file mode 100644 index 00000000..376d2ca0 --- /dev/null +++ b/tests/integration/test_admin_pricing_rate_validation.py @@ -0,0 +1,211 @@ +"""Admin write edge: a rate that is not a number never becomes a stored price. + +A billable rate is usable only when it is finite and non-negative. The admin +model endpoints are an entry point for rates the node will later bill on, and +they accept whatever a client sends: ``json`` parses the bare ``NaN``/ +``Infinity`` literals into real floats and overflows ``1e999`` to ``inf``, a +non-numeric string coerced silently to ``$0``, and a negative rate is truthy so +it read back as a chargeable price that bills a negative amount. + +These tests assert the edge answers a malformed rate with a 422 — a client bug +reported as a client bug — rather than persisting it or failing as a 500, and +that the operator can still open the listing that shows the row needing repair. +""" + +from __future__ import annotations + +import json +from datetime import datetime, timedelta, timezone + +import pytest +from httpx import AsyncClient +from sqlmodel.ext.asyncio.session import AsyncSession + +from routstr.core.admin import admin_sessions +from routstr.core.db import ModelRow, UpstreamProviderRow +from routstr.proxy import reinitialize_upstreams + + +def _admin_headers() -> dict[str, str]: + token = "test-admin-rate-validation-token" + admin_sessions[token] = int( + (datetime.now(timezone.utc) + timedelta(minutes=5)).timestamp() + ) + return {"Authorization": f"Bearer {token}"} + + +def _pricing(**overrides: object) -> dict[str, object]: + pricing: dict[str, object] = { + "prompt": 1.4e-7, + "completion": 2.8e-7, + "request": 0.0, + "image": 0.0, + "web_search": 0.0, + "internal_reasoning": 0.0, + "input_cache_read": 0.0, + "input_cache_write": 0.0, + } + pricing.update(overrides) + return pricing + + +def _payload( + provider_id: int, + *, + model_id: str = "rate-model", + pricing: dict[str, object] | None = None, +) -> dict[str, object]: + return { + "id": model_id, + "name": "Rate Model", + "description": "d", + "created": 0, + "context_length": 128000, + "architecture": { + "modality": "text", + "input_modalities": ["text"], + "output_modalities": ["text"], + "tokenizer": "unknown", + "instruct_type": None, + }, + "pricing": pricing if pricing is not None else _pricing(), + "per_request_limits": None, + "top_provider": None, + "upstream_provider_id": provider_id, + "canonical_slug": None, + "alias_ids": [], + "enabled": True, + "forwarded_model_id": model_id, + } + + +async def _make_provider(session: AsyncSession) -> int: + provider = UpstreamProviderRow( + provider_type="generic", + base_url="https://rate-upstream.example/v1", + api_key="test-key", + provider_fee=1.0, + ) + session.add(provider) + await session.commit() + await session.refresh(provider) + await reinitialize_upstreams() + assert provider.id is not None + return provider.id + + +def _raw_model_body(provider_id: int, model_id: str, prompt_literal: str) -> str: + """A request body built as text, so it can carry a literal ``json`` accepts + but Python's own encoder would refuse to produce.""" + return ( + f'{{"id": "{model_id}", "name": "raw", "description": "d", "created": 0,' + ' "context_length": 8192, "architecture": {"modality": "text"},' + f' "pricing": {{"prompt": {prompt_literal}, "completion": 2.8e-7}},' + f' "upstream_provider_id": {provider_id}}}' + ) + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_negative_price_is_rejected( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """A negative rate is not a valid price — accepting it would persist a row + that bills a negative amount, which settlement subtracts from the balance. + Being truthy, it also reads back as a chargeable price. Reject at the edge + rather than silently storing it.""" + provider_id = await _make_provider(integration_session) + + resp = await integration_client.post( + f"/admin/api/upstream-providers/{provider_id}/models", + headers=_admin_headers(), + json=_payload(provider_id, model_id="neg-price", pricing=_pricing(prompt=-1.0)), + ) + + assert resp.status_code == 422 + assert await integration_session.get(ModelRow, ("neg-price", provider_id)) is None + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_malformed_price_string_is_rejected( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """A present non-numeric rate is a client bug: it coerces to ``$0`` on the + read path, producing an unpriced-looking row indistinguishable from a + deliberate free price. Surface it as a 422 instead of accepting it.""" + provider_id = await _make_provider(integration_session) + + resp = await integration_client.post( + f"/admin/api/upstream-providers/{provider_id}/models", + headers=_admin_headers(), + json=_payload( + provider_id, model_id="bad-price", pricing=_pricing(prompt="oops") + ), + ) + + assert resp.status_code == 422 + assert await integration_session.get(ModelRow, ("bad-price", provider_id)) is None + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_numeric_string_price_is_still_accepted( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """The stored pricing JSON has always accepted numeric strings, and the UI + round-trips rates through text fields. Rejecting a *malformed* rate must not + also reject a well-formed one that arrives spelled as a string.""" + provider_id = await _make_provider(integration_session) + + resp = await integration_client.post( + f"/admin/api/upstream-providers/{provider_id}/models", + headers=_admin_headers(), + json=_payload( + provider_id, model_id="string-price", pricing=_pricing(prompt="0.000005") + ), + ) + + assert resp.status_code == 200 + row = await integration_session.get(ModelRow, ("string-price", provider_id)) + assert row is not None + assert json.loads(row.pricing)["prompt"] == "0.000005" + + +def test_non_finite_price_is_rejected_by_the_write_model() -> None: + """``NaN``/``±inf`` are not billable rates: the carrier every write endpoint + shares must reject them before they can be persisted and read back as a + chargeable price.""" + from pydantic import ValidationError + + from routstr.core.admin import ModelCreate + + for bad in (float("nan"), float("inf"), float("-inf")): + with pytest.raises(ValidationError): + ModelCreate.model_validate( + _payload(1, model_id="nonfinite", pricing=_pricing(prompt=bad)) + ) + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_oversized_integer_price_is_rejected( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """A JSON integer too large for a float is a client bug, not a server fault. + + ``float()`` raises ``OverflowError`` for it, and pydantic converts only + ``ValueError``/``AssertionError`` into validation errors, so it escaped the + edge as a 500. It must be answered with the same 422 as every other + unusable rate. + """ + provider_id = await _make_provider(integration_session) + + resp = await integration_client.post( + f"/admin/api/upstream-providers/{provider_id}/models", + headers={**_admin_headers(), "Content-Type": "application/json"}, + content=_raw_model_body(provider_id, "huge-price", "9" * 400), + ) + + assert resp.status_code == 422 + assert await integration_session.get(ModelRow, ("huge-price", provider_id)) is None From 09e356b64247e6624ed8ca4031df7ae9a474137d Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 16:23:31 +0200 Subject: [PATCH 05/18] fix(api): answer a request-validation failure with a 422 that serializes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `json` parses the bare `Infinity`/`NaN` literals into real floats, so the pricing edge rejects them correctly — but pydantic echoes the offending value back in the error's `input` field, and JSONResponse encodes with `allow_nan=False`. Serializing the 422 then raised "Out of range float values are not JSON compliant" and the reply escaped as a 500, reporting a client's bad rate as a server fault. The request already failed closed, so no row was ever written. Handle RequestValidationError explicitly and render non-finite floats as text, which covers every endpoint rather than the pricing edge alone. The integration app now copies the main app's exception handlers so a failing request fails the way it does in production. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- routstr/core/exceptions.py | 43 ++++++++++++++++ routstr/core/main.py | 8 ++- tests/integration/conftest.py | 4 ++ .../test_admin_pricing_rate_validation.py | 50 +++++++++++++++++++ 4 files changed, 104 insertions(+), 1 deletion(-) diff --git a/routstr/core/exceptions.py b/routstr/core/exceptions.py index 9e4f2ae6..fd3cfefa 100644 --- a/routstr/core/exceptions.py +++ b/routstr/core/exceptions.py @@ -1,4 +1,8 @@ +import math + from fastapi import Request +from fastapi.encoders import jsonable_encoder +from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse from .logging import get_logger @@ -61,6 +65,45 @@ async def http_exception_handler(request: Request, exc: Exception) -> JSONRespon return JSONResponse(status_code=status_code, content=content) +def json_compliant(value: object) -> object: + """Render non-finite floats as text so a reply carrying them can serialize. + + ``json`` parses the bare ``NaN``/``Infinity``/``-Infinity`` literals into + real floats, so a request body — and a stored row written from one — may + hold one anywhere. ``JSONResponse`` encodes with ``allow_nan=False`` and + raises on them, which would turn a reply that merely *quotes* the offending + value into a 500. + """ + if isinstance(value, float) and not math.isfinite(value): + return repr(value) + if isinstance(value, dict): + return {key: json_compliant(item) for key, item in value.items()} + if isinstance(value, (list, tuple)): + return [json_compliant(item) for item in value] + return value + + +async def validation_exception_handler( + request: Request, exc: Exception +) -> JSONResponse: + """Answer a request-validation failure with a 422 that always serializes. + + Pydantic echoes the rejected value back in each error's ``input`` field. A + non-finite float there breaks the encoder, so the 422 escapes as a 500 and + reports a client's bad rate as a server fault. + """ + request_id = getattr(request.state, "request_id", "unknown") + errors = exc.errors() if isinstance(exc, RequestValidationError) else [] + + return JSONResponse( + status_code=422, + content={ + "detail": json_compliant(jsonable_encoder(errors)), + "request_id": request_id, + }, + ) + + async def general_exception_handler(request: Request, exc: Exception) -> JSONResponse: """Handle general exceptions and include request ID in response.""" request_id = getattr(request.state, "request_id", "unknown") diff --git a/routstr/core/main.py b/routstr/core/main.py index 979f5cdb..b89fa64d 100644 --- a/routstr/core/main.py +++ b/routstr/core/main.py @@ -4,6 +4,7 @@ from pathlib import Path from typing import AsyncGenerator from fastapi import FastAPI +from fastapi.exceptions import RequestValidationError from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse, RedirectResponse from fastapi.staticfiles import StaticFiles @@ -33,7 +34,11 @@ from ..upstream.litellm_routing import configure_litellm from ..wallet import periodic_payout, periodic_refund_sweep, periodic_routstr_fee_payout from .admin import admin_router from .db import create_session, init_db, run_migrations -from .exceptions import general_exception_handler, http_exception_handler +from .exceptions import ( + general_exception_handler, + http_exception_handler, + validation_exception_handler, +) from .logging import get_logger, setup_logging from .middleware import LoggingMiddleware from .not_found import _NOT_FOUND_HTML, not_found_catch_all # noqa: F401 @@ -289,6 +294,7 @@ app.add_middleware(LoggingMiddleware) # Add exception handlers app.add_exception_handler(HTTPException, http_exception_handler) # type: ignore +app.add_exception_handler(RequestValidationError, validation_exception_handler) app.add_exception_handler(Exception, general_exception_handler) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index aa10a81c..265c73c9 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -508,6 +508,10 @@ async def integration_app( # Copy all routes from the main app test_app.router = app.router + # ...and its exception handlers, so a request that fails here fails the way + # it would in production rather than escaping as a bare exception. + test_app.exception_handlers.update(app.exception_handlers) + # Override the get_session dependency async def override_get_session() -> AsyncGenerator[AsyncSession, None]: yield integration_session diff --git a/tests/integration/test_admin_pricing_rate_validation.py b/tests/integration/test_admin_pricing_rate_validation.py index 376d2ca0..ed609051 100644 --- a/tests/integration/test_admin_pricing_rate_validation.py +++ b/tests/integration/test_admin_pricing_rate_validation.py @@ -209,3 +209,53 @@ async def test_oversized_integer_price_is_rejected( assert resp.status_code == 422 assert await integration_session.get(ModelRow, ("huge-price", provider_id)) is None + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_non_finite_literal_price_is_rejected( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """A bare ``Infinity``/``NaN`` literal gets the same 422 as any other rate. + + ``json`` accepts both literals, so the edge sees a real float and rejects + it — but pydantic echoes the offending value back in the error's ``input`` + field, and the response encoder runs with ``allow_nan=False``. Serializing + that reply raised "Out of range float values are not JSON compliant", so the + 422 escaped as a 500 and reported a client's bad rate as a server fault. + """ + provider_id = await _make_provider(integration_session) + + for literal in ("Infinity", "-Infinity", "NaN"): + resp = await integration_client.post( + f"/admin/api/upstream-providers/{provider_id}/models", + headers={**_admin_headers(), "Content-Type": "application/json"}, + content=_raw_model_body(provider_id, "odd-price", literal), + ) + + assert resp.status_code == 422, literal + assert ( + await integration_session.get(ModelRow, ("odd-price", provider_id)) is None + ) + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_non_finite_literal_price_is_rejected_in_batch_override( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """The batch path shares the same carrier, so it must answer 422 too.""" + provider_id = await _make_provider(integration_session) + + resp = await integration_client.post( + f"/admin/api/upstream-providers/{provider_id}/batch-override", + headers={**_admin_headers(), "Content-Type": "application/json"}, + content=( + '{"models": [' + + _raw_model_body(provider_id, "odd-batch", "Infinity") + + "]}" + ), + ) + + assert resp.status_code == 422 + assert await integration_session.get(ModelRow, ("odd-batch", provider_id)) is None From e202435ca0e7cb5da93ee630bf1e4338475b5d8c Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 16:28:59 +0200 Subject: [PATCH 06/18] fix(pricing): never serve or route a price built on an unusable rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A stored rate that is negative or non-finite is not a price, but every guard on the read path tested truthiness, and `NaN`, `inf` and a negative float are all truthy. Such a row was advertised in the catalog and built into the routing map, and the cost calculation cannot bill on it: every request on the model fell through to the flat maximum reservation, or — for a negative rate — was billed an amount settlement credits back to the caller. `has_usable_pricing` asks the question once, across every billable rate, so the served-catalog and routing backstops answer it identically. One unusable rate disqualifies the whole price: a positive completion rate must not hide a negative prompt rate. Zero is untouched — a free model is a real price. The admin listing (`include_disabled`) is deliberately exempt, or the operator would lose sight of the row that needs repairing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- routstr/algorithm.py | 19 ++- routstr/payment/models.py | 49 +++++- .../test_served_catalog_rate_backstop.py | 157 ++++++++++++++++++ tests/unit/test_algorithm.py | 64 +++++++ 4 files changed, 279 insertions(+), 10 deletions(-) create mode 100644 tests/integration/test_served_catalog_rate_backstop.py diff --git a/routstr/algorithm.py b/routstr/algorithm.py index cced61f7..68406876 100644 --- a/routstr/algorithm.py +++ b/routstr/algorithm.py @@ -135,9 +135,21 @@ def create_model_mappings( Returns: Tuple of (model_instances, provider_map, unique_models) """ - from .payment.models import _row_to_model + from .payment.models import _row_to_model, has_usable_pricing from .upstream.helpers import resolve_model_alias + def _unusable_price(model: "Model") -> bool: + """A candidate may only route on rates a request can be billed against. + + Mirrors the served-catalog backstop in ``list_models``: a negative or + non-finite rate is not a price, and the cost calculation cannot bill on + one, so every request on the model would be charged the full maximum + reservation instead. Applies to provider-discovered models as well as + persisted overrides — no override row need exist for a malformed price + to be built into the candidate map. + """ + return not has_usable_pricing(model.pricing) + candidates: dict[str, list[tuple["Model", "BaseUpstreamProvider"]]] = {} unique_models: dict[str, "Model"] = {} unique_model_keys: dict[str, str] = {} @@ -231,6 +243,9 @@ def create_model_mappings( else: model_to_use = model + if _unusable_price(model_to_use): + continue + forwarded_model_id = get_effective_forwarded_model_id(model_to_use) # Get all aliases for this model @@ -297,6 +312,8 @@ def create_model_mappings( continue if not model_to_use.enabled: continue + if _unusable_price(model_to_use): + continue forwarded_model_id = get_effective_forwarded_model_id(model_to_use) diff --git a/routstr/payment/models.py b/routstr/payment/models.py index 6595d684..23c7ab6f 100644 --- a/routstr/payment/models.py +++ b/routstr/payment/models.py @@ -97,6 +97,19 @@ def is_usable_rate(rate: float) -> bool: return math.isfinite(rate) and rate >= 0.0 +def has_usable_pricing(pricing: Pricing) -> bool: + """True if every billable rate is a number a request could be billed on. + + Free is usable — a rate of zero is a real price. This asks only whether the + price is well-formed. One unusable rate disqualifies the whole price even + alongside a valid one, since a request can bill on the bad field: a positive + ``completion`` must not hide a negative ``prompt``. + """ + return all( + is_usable_rate(getattr(pricing, field)) for field in BILLABLE_PRICING_FIELDS + ) + + class TopProvider(BaseModel): context_length: int | None = None max_completion_tokens: int | None = None @@ -354,21 +367,39 @@ async def list_models( rows = (await session.exec(query)).all() # type: ignore provider_result = await session.exec(select(UpstreamProviderRow)) providers_by_id = {p.id: p for p in provider_result.all()} - return [ - _row_to_model( + + models: list[Model] = [] + for r in rows: + if not include_disabled and not ( + r.upstream_provider_id in providers_by_id + and providers_by_id[r.upstream_provider_id].enabled + ): + continue + model = _row_to_model( r, apply_provider_fee=apply_fees, provider_fee=providers_by_id[r.upstream_provider_id].provider_fee if r.upstream_provider_id in providers_by_id else 1.01, ) - for r in rows - if include_disabled - or ( - r.upstream_provider_id in providers_by_id - and providers_by_id[r.upstream_provider_id].enabled - ) - ] + # Served-map backstop for legacy rows and writers that bypass the admin + # edge: a negative or non-finite rate is not a price. Serving one + # advertises a rate the cost calculation cannot bill on, so the request + # falls through to the flat maximum reservation — or, if the rate is + # negative, bills an amount settlement credits back to the caller. + # ``include_disabled`` is the operator's listing, which must keep showing + # the row so it can be repaired. + if not include_disabled and not has_usable_pricing(model.pricing): + logger.warning( + "Withholding model with an unusable stored rate from the catalog", + extra={ + "model_id": r.id, + "upstream_provider_id": r.upstream_provider_id, + }, + ) + continue + models.append(model) + return models def _calculate_usd_max_costs(model: Model) -> tuple[float, float, float]: diff --git a/tests/integration/test_served_catalog_rate_backstop.py b/tests/integration/test_served_catalog_rate_backstop.py new file mode 100644 index 00000000..96455554 --- /dev/null +++ b/tests/integration/test_served_catalog_rate_backstop.py @@ -0,0 +1,157 @@ +"""The served catalog is the last guard between a stored row and a charge. + +Stored pricing is JSON written by whatever produced the row — an upstream +import, an operator, a legacy migration, or a foreign writer that never passed +the admin edge. So the read path cannot assume a stored rate is a number: it +must decline to serve a row it cannot bill on, and it must survive a row it +cannot read at all rather than taking the whole catalog down with it. + +The admin listing is deliberately exempt: it includes disabled models and is the +one view that still shows the operator the row that needs repair. +""" + +from __future__ import annotations + +import json + +import pytest +from sqlmodel.ext.asyncio.session import AsyncSession + +from routstr.core.db import ModelRow, UpstreamProviderRow +from routstr.payment.models import list_models +from routstr.proxy import reinitialize_upstreams + +_ARCHITECTURE = json.dumps( + { + "modality": "text", + "input_modalities": ["text"], + "output_modalities": ["text"], + "tokenizer": "unknown", + "instruct_type": None, + } +) + + +async def _make_provider(session: AsyncSession) -> int: + provider = UpstreamProviderRow( + provider_type="generic", + base_url="https://served-upstream.example/v1", + api_key="test-key", + provider_fee=1.0, + ) + session.add(provider) + await session.commit() + await session.refresh(provider) + await reinitialize_upstreams() + assert provider.id is not None + return provider.id + + +async def _insert_row( + session: AsyncSession, + provider_id: int, + *, + model_id: str, + pricing: dict[str, object], +) -> None: + session.add( + ModelRow( + id=model_id, + name=model_id, + description="d", + created=0, + context_length=8192, + architecture=_ARCHITECTURE, + pricing=json.dumps(pricing), + upstream_provider_id=provider_id, + enabled=True, + forwarded_model_id=model_id, + ) + ) + await session.commit() + + +@pytest.mark.integration +@pytest.mark.asyncio +@pytest.mark.parametrize( + "bad_rate", + [float("nan"), float("inf"), -1.0], + ids=["nan", "inf", "negative"], +) +async def test_served_catalog_excludes_a_malformed_stored_rate( + integration_session: AsyncSession, bad_rate: float +) -> None: + """A stored rate that is not a number must not be advertised. + + Zero is a real price and a free model is servable, but a negative or + non-finite rate is not a price at all: serving it advertises a rate the cost + calculation cannot bill on, so every request falls through to the flat + maximum reservation — or, for a negative rate, bills an amount settlement + credits back to the caller. + """ + provider_id = await _make_provider(integration_session) + await _insert_row( + integration_session, + provider_id, + model_id="good", + pricing={"prompt": 1e-06, "completion": 2e-06}, + ) + await _insert_row( + integration_session, + provider_id, + model_id="bad-rate", + pricing={"prompt": bad_rate, "completion": 2e-06}, + ) + + served = {m.id for m in await list_models(integration_session, provider_id)} + + assert served == {"good"} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_free_stored_price_is_still_served( + integration_session: AsyncSession, +) -> None: + """Zero is a real price. Rejecting malformed rates must not also drop a row + priced at zero, which is a free model and not a broken one.""" + provider_id = await _make_provider(integration_session) + await _insert_row( + integration_session, + provider_id, + model_id="free", + pricing={"prompt": 0.0, "completion": 0.0}, + ) + + served = {m.id for m in await list_models(integration_session, provider_id)} + + assert served == {"free"} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_admin_listing_still_shows_a_malformed_stored_rate( + integration_session: AsyncSession, +) -> None: + """The operator has to be able to see the row that needs fixing. + + The backstop keeps a malformed row out of the *served* catalog. The listing + that includes disabled models is the one view where the row must still + appear, or the operator loses the ability to repair it. + """ + provider_id = await _make_provider(integration_session) + await _insert_row( + integration_session, + provider_id, + model_id="bad-rate", + pricing={"prompt": -1.0, "completion": 2e-06}, + ) + + listed = { + m.id + for m in await list_models( + integration_session, provider_id, include_disabled=True + ) + } + + assert listed == {"bad-rate"} diff --git a/tests/unit/test_algorithm.py b/tests/unit/test_algorithm.py index 5d31af2e..62d3ad0d 100644 --- a/tests/unit/test_algorithm.py +++ b/tests/unit/test_algorithm.py @@ -954,3 +954,67 @@ def test_create_model_mappings_uppercase_prefixed_base_keeps_top_tier() -> None: assert provider_map["qwen2.5-72b"][0] == (prefixed_cheap, prefixed_provider) assert unique_models["qwen2.5-72b"].upstream_provider_id == "prefixed" + + +def test_create_model_mappings_excludes_a_malformed_price() -> None: + """A rate that is not a number must not be routable. + + A negative or non-finite rate reads as a real price to every truthiness + check, so the candidate was built into the map and served. The cost + calculation cannot price on such a rate, so every request on the model fell + through to the flat maximum reservation — or, for a negative rate, billed a + negative amount that settlement credits back to the caller. + """ + healthy = create_test_model("healthy-model") + for bad_rate in (float("nan"), float("inf"), -1.0): + broken = create_test_model("broken-model", prompt_price=bad_rate) + provider = create_test_provider( + "custom", + "https://custom.example/v1", + db_id=1, + models=[broken, healthy], + ) + + _, provider_map, unique_models = create_model_mappings( + upstreams=[provider], + overrides_by_key={}, + disabled_model_keys=set(), + ) + + assert "broken-model" not in provider_map, bad_rate + assert "broken-model" not in unique_models, bad_rate + # One unroutable candidate must not cost the provider its other models. + assert "healthy-model" in provider_map, bad_rate + + +def test_create_model_mappings_excludes_an_override_with_a_malformed_price( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An override row carrying a malformed rate is unroutable too. + + An override replaces the discovered model's price, so a provider whose + catalog is sound still routes at whatever the row says. The guard has to sit + after the override is applied, not before it. + """ + discovered = create_test_model("shared-model") + provider = create_test_provider( + "custom", "https://custom.example/v1", db_id=3, models=[discovered] + ) + override_model = create_test_model("shared-model", prompt_price=float("-inf")) + + monkeypatch.setattr( + "routstr.payment.models._row_to_model", + lambda *args, **kwargs: override_model, + ) + override_row = SimpleNamespace( + id="shared-model", upstream_provider_id=3, enabled=True + ) + + _, provider_map, unique_models = create_model_mappings( + upstreams=[provider], + overrides_by_key={("shared-model", 3): (override_row, 1.0)}, + disabled_model_keys=set(), + ) + + assert "shared-model" not in provider_map + assert "shared-model" not in unique_models From 41fed7413adf33ebb2f6c3c1b3abc44bf280cd99 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 16:33:44 +0200 Subject: [PATCH 07/18] fix(pricing): keep one unreadable model row from taking the node with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stored pricing is JSON from whatever wrote the row, so a legacy import or foreign writer can leave a field that will not parse. Both places that convert a row do it inside a loop, so one such row raised out of the whole operation: the catalog listing returned nothing and the node advertised no models at all, and the routing map came up empty at boot — on a later refresh the map it already had went permanently stale instead. Drop the row that cannot be read, log it by id, and keep serving and routing the rest. It is unservable either way. The sibling loop over override-only rows in the mapping builder already did this; the two now match. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- routstr/algorithm.py | 24 +++++++++-- routstr/payment/models.py | 32 +++++++++++---- .../test_served_catalog_rate_backstop.py | 32 +++++++++++++++ tests/unit/test_algorithm.py | 40 +++++++++++++++++++ 4 files changed, 118 insertions(+), 10 deletions(-) diff --git a/routstr/algorithm.py b/routstr/algorithm.py index 68406876..6a896665 100644 --- a/routstr/algorithm.py +++ b/routstr/algorithm.py @@ -237,9 +237,27 @@ def create_model_mappings( # Apply overrides only for this provider's model row. if model_key is not None and model_key in overrides_by_key: override_row, provider_fee = overrides_by_key[model_key] - model_to_use = _row_to_model( - override_row, apply_provider_fee=True, provider_fee=provider_fee - ) + try: + model_to_use = _row_to_model( + override_row, apply_provider_fee=True, provider_fee=provider_fee + ) + except Exception as exc: + # Stored pricing is JSON from whatever wrote the row, so + # converting it can raise. Doing that inside this loop let + # one such row unwind the whole map build: at boot the node + # came up routing nothing, and on a later refresh the map it + # already had went permanently stale. The sibling loop over + # override-only rows already skips and logs such a row. + logger.warning( + "Skipping invalid model override while building model mappings", + extra={ + "model_id": model.id, + "upstream_provider_id": upstream_db_id, + "error": str(exc), + "error_type": type(exc).__name__, + }, + ) + continue else: model_to_use = model diff --git a/routstr/payment/models.py b/routstr/payment/models.py index 23c7ab6f..7f3a20f3 100644 --- a/routstr/payment/models.py +++ b/routstr/payment/models.py @@ -375,13 +375,31 @@ async def list_models( and providers_by_id[r.upstream_provider_id].enabled ): continue - model = _row_to_model( - r, - apply_provider_fee=apply_fees, - provider_fee=providers_by_id[r.upstream_provider_id].provider_fee - if r.upstream_provider_id in providers_by_id - else 1.01, - ) + try: + model = _row_to_model( + r, + apply_provider_fee=apply_fees, + provider_fee=providers_by_id[r.upstream_provider_id].provider_fee + if r.upstream_provider_id in providers_by_id + else 1.01, + ) + except Exception as e: + # Stored pricing/architecture is JSON from whatever wrote the row, so + # a legacy import or foreign writer can leave a field that will not + # parse. Converting inside this loop meant one such row raised out of + # the whole listing and the node advertised nothing at all. Drop the + # row we cannot read — it is unservable either way — and keep serving + # the rest. + logger.warning( + "Skipping model row that could not be read", + extra={ + "model_id": r.id, + "upstream_provider_id": r.upstream_provider_id, + "error": str(e), + "error_type": type(e).__name__, + }, + ) + continue # Served-map backstop for legacy rows and writers that bypass the admin # edge: a negative or non-finite rate is not a price. Serving one # advertises a rate the cost calculation cannot bill on, so the request diff --git a/tests/integration/test_served_catalog_rate_backstop.py b/tests/integration/test_served_catalog_rate_backstop.py index 96455554..02829bae 100644 --- a/tests/integration/test_served_catalog_rate_backstop.py +++ b/tests/integration/test_served_catalog_rate_backstop.py @@ -155,3 +155,35 @@ async def test_admin_listing_still_shows_a_malformed_stored_rate( } assert listed == {"bad-rate"} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_one_unreadable_stored_price_does_not_blank_the_catalog( + integration_session: AsyncSession, +) -> None: + """A single unparseable row must cost that row, not every model on the node. + + Stored pricing is JSON written by whatever produced the row, so a + non-numeric rate is reachable from a legacy import or a foreign writer. + Parsing it raises out of the row-to-model conversion, and because the + conversion ran inside the catalog loop the exception took the whole listing + with it — one bad row and the node advertised nothing at all. + """ + provider_id = await _make_provider(integration_session) + await _insert_row( + integration_session, + provider_id, + model_id="good", + pricing={"prompt": 1e-06, "completion": 2e-06}, + ) + await _insert_row( + integration_session, + provider_id, + model_id="unreadable", + pricing={"prompt": "not-a-number", "completion": 2e-06}, + ) + + served = {m.id for m in await list_models(integration_session, provider_id)} + + assert served == {"good"} diff --git a/tests/unit/test_algorithm.py b/tests/unit/test_algorithm.py index 62d3ad0d..cefc4f90 100644 --- a/tests/unit/test_algorithm.py +++ b/tests/unit/test_algorithm.py @@ -1018,3 +1018,43 @@ def test_create_model_mappings_excludes_an_override_with_a_malformed_price( assert "shared-model" not in provider_map assert "shared-model" not in unique_models + + +def test_create_model_mappings_survives_an_unreadable_override_row( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """One row that cannot be read must not empty the whole routing map. + + Stored pricing is JSON from whatever wrote the row, so converting it can + raise. Converting an override while walking a provider's catalog let that + exception unwind the entire map build: at boot the node came up routing + nothing, and on a later refresh the map it already had went permanently + stale. The sibling loop over override-only rows already skips and logs such + a row. + """ + broken = create_test_model("broken-model") + healthy = create_test_model("healthy-model") + provider = create_test_provider( + "custom", + "https://custom.example/v1", + db_id=5, + models=[broken, healthy], + ) + + def raising_row_to_model(row: Any, *args: Any, **kwargs: Any) -> Model: + raise ValueError("value is not a valid float") + + monkeypatch.setattr("routstr.payment.models._row_to_model", raising_row_to_model) + override_row = SimpleNamespace( + id="broken-model", upstream_provider_id=5, enabled=True + ) + + _, provider_map, unique_models = create_model_mappings( + upstreams=[provider], + overrides_by_key={("broken-model", 5): (override_row, 1.0)}, + disabled_model_keys=set(), + ) + + assert "broken-model" not in provider_map + assert "healthy-model" in provider_map + assert "healthy-model" in unique_models From 6e5a388ee4c4cd72fb1a2858b88932d46fcf1fd4 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 16:37:33 +0200 Subject: [PATCH 08/18] fix(admin): show the operator the rate that needs fixing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admin listing includes disabled models, so it is the one view still carrying a row the served-catalog backstop holds back — including one whose stored rate is not a usable number. The encoder reported that rate as `null`, indistinguishable from a missing one, so the operator could see the row but not the reason it was withheld. Reuses the non-finite renderer the 422 handler already uses, promoted to a shared name now that it has a second caller. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- routstr/core/admin.py | 10 +++- .../test_admin_pricing_rate_validation.py | 49 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/routstr/core/admin.py b/routstr/core/admin.py index be58972c..0e8b1158 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -35,6 +35,7 @@ from .db import ( from .db import ( store_cashu_transaction_with_retry as store_cashu_transaction, ) +from .exceptions import json_compliant from .log_manager import log_manager from .logging import get_logger from .provider_slugs import allocate_unique_provider_slug @@ -1273,8 +1274,13 @@ async def get_provider_models(provider_id: str) -> dict[str, object]: "provider_type": provider.provider_type, "base_url": provider.base_url, }, - "db_models": [m.dict() for m in db_models], - "remote_models": [m.dict() for m in filtered_remote_models], + # This listing includes disabled models, so it is the one view that + # still carries a row the served-catalog backstop holds back — + # including one whose stored rate is not a usable number. The + # encoder would report that rate as `null`, indistinguishable from a + # missing one; show the operator the value that needs fixing. + "db_models": [json_compliant(m.dict()) for m in db_models], + "remote_models": [json_compliant(m.dict()) for m in filtered_remote_models], } diff --git a/tests/integration/test_admin_pricing_rate_validation.py b/tests/integration/test_admin_pricing_rate_validation.py index ed609051..78614a8b 100644 --- a/tests/integration/test_admin_pricing_rate_validation.py +++ b/tests/integration/test_admin_pricing_rate_validation.py @@ -259,3 +259,52 @@ async def test_non_finite_literal_price_is_rejected_in_batch_override( assert resp.status_code == 422 assert await integration_session.get(ModelRow, ("odd-batch", provider_id)) is None + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_admin_model_listing_shows_a_non_finite_stored_rate( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """The operator must be able to see the rate that needs fixing. + + The admin listing deliberately includes disabled models, so it is the one + view that still carries a row the served-catalog backstop holds back. + FastAPI's encoder rendered a stored ``Infinity`` rate as ``null``, which is + indistinguishable from a rate the row never carried — the operator could see + the row but not the reason it was withheld. Render the offending value as + text instead, as the 422 handler already does. + """ + provider_id = await _make_provider(integration_session) + integration_session.add( + ModelRow( + id="inf-rate", + name="inf-rate", + description="d", + created=0, + context_length=8192, + architecture=json.dumps( + { + "modality": "text", + "input_modalities": ["text"], + "output_modalities": ["text"], + "tokenizer": "unknown", + "instruct_type": None, + } + ), + pricing=json.dumps({"prompt": float("inf"), "completion": 2e-06}), + upstream_provider_id=provider_id, + enabled=True, + forwarded_model_id="inf-rate", + ) + ) + await integration_session.commit() + + resp = await integration_client.get( + f"/admin/api/upstream-providers/{provider_id}/models", + headers=_admin_headers(), + ) + + assert resp.status_code == 200 + listed = {m["id"]: m for m in resp.json()["db_models"]} + assert listed["inf-rate"]["pricing"]["prompt"] == "inf" From 86be12f4be85bc1a3d82967f8c9e2d918c7b80ea Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 20:28:16 +0200 Subject: [PATCH 09/18] fix(admin): show the rate that needs fixing in the single-model view too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The listing already renders a stored non-finite rate as text rather than letting the encoder report it as `null`, indistinguishable from a rate the row never carried. The single-model view is the other view of that same row — the one an operator opens from the listing to repair it — and still answered `null`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- routstr/core/admin.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/routstr/core/admin.py b/routstr/core/admin.py index 0e8b1158..f018b6a5 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -682,9 +682,13 @@ async def get_provider_model(provider_id: str, model_id: str) -> dict[str, objec raise HTTPException( status_code=404, detail="Model not found for this provider" ) - return _row_to_model( - row, apply_provider_fee=False, provider_fee=provider.provider_fee - ).dict() # type: ignore + # Same duty as the listing this view is opened from: a stored rate that + # is not a usable number must be shown as it is, not encoded as `null`. + return json_compliant( # type: ignore[return-value] + _row_to_model( + row, apply_provider_fee=False, provider_fee=provider.provider_fee + ).dict() + ) @admin_router.delete( From b68086c5923a77b2bd218a813e7c0eed876cdc4e Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 20:28:16 +0200 Subject: [PATCH 10/18] test(pricing): pin rate validation to every billable rate, not the token pair MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The write-edge validator and the served-catalog backstop both iterate the full billable-rate tuple, but every test drove them through `prompt` alone — the tuple could be narrowed to the two token rates and the suite stayed green, while a malformed image/search/reasoning/cache rate walked in. Covers each remaining rate at both surfaces. `request` is deliberately absent from the catalog list and pinned by its own test instead: the row-to-model conversion clamps a negative stored `request` to zero before the price is built, so the backstop never sees one. Also covers the exchange-quote reader's widened error handling, which had no test: a body that never yields JSON raises before any number is read, and unhandled it abandoned the whole aggregation with two healthy quotes in hand. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- .../test_admin_pricing_rate_validation.py | 82 +++++++++++++++++++ .../test_served_catalog_rate_backstop.py | 68 +++++++++++++++ tests/unit/test_pricing_rate_validation.py | 38 ++++++++- 3 files changed, 185 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_admin_pricing_rate_validation.py b/tests/integration/test_admin_pricing_rate_validation.py index 78614a8b..5b53b02a 100644 --- a/tests/integration/test_admin_pricing_rate_validation.py +++ b/tests/integration/test_admin_pricing_rate_validation.py @@ -308,3 +308,85 @@ async def test_admin_model_listing_shows_a_non_finite_stored_rate( assert resp.status_code == 200 listed = {m["id"]: m for m in resp.json()["db_models"]} assert listed["inf-rate"]["pricing"]["prompt"] == "inf" + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_malformed_auxiliary_rate_is_rejected( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """Validation spans every billable rate, not just the token rates. + + ``prompt``/``completion`` are the rates most prices are built from, but the + request, image, search, reasoning and cache rates are billed too. A negative + or non-finite value in any of them is the same defect and must be answered + the same way. + """ + provider_id = await _make_provider(integration_session) + + for field, bad in ( + ("request", -1.0), + ("image", -0.5), + ("web_search", float("inf")), + ("internal_reasoning", float("nan")), + ("input_cache_read", -1e-06), + ("input_cache_write", float("-inf")), + ("completion", -1.0), + ): + resp = await integration_client.post( + f"/admin/api/upstream-providers/{provider_id}/models", + headers=_admin_headers(), + json=_payload( + provider_id, model_id="aux-rate", pricing=_pricing(**{field: bad}) + ), + ) + + assert resp.status_code == 422, field + assert ( + await integration_session.get(ModelRow, ("aux-rate", provider_id)) is None + ), field + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_admin_single_model_shows_a_non_finite_stored_rate( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """The single-model view answers like the listing it is opened from. + + It is the other view of a row the served-catalog backstop holds back, so it + has the same duty to name the rate that needs fixing rather than rendering + it as ``null``. + """ + provider_id = await _make_provider(integration_session) + integration_session.add( + ModelRow( + id="inf-one", + name="inf-one", + description="d", + created=0, + context_length=8192, + architecture=json.dumps( + { + "modality": "text", + "input_modalities": ["text"], + "output_modalities": ["text"], + "tokenizer": "unknown", + "instruct_type": None, + } + ), + pricing=json.dumps({"prompt": float("inf"), "completion": 2e-06}), + upstream_provider_id=provider_id, + enabled=True, + forwarded_model_id="inf-one", + ) + ) + await integration_session.commit() + + resp = await integration_client.get( + f"/admin/api/upstream-providers/{provider_id}/models/inf-one", + headers=_admin_headers(), + ) + + assert resp.status_code == 200 + assert resp.json()["pricing"]["prompt"] == "inf" diff --git a/tests/integration/test_served_catalog_rate_backstop.py b/tests/integration/test_served_catalog_rate_backstop.py index 02829bae..7819c017 100644 --- a/tests/integration/test_served_catalog_rate_backstop.py +++ b/tests/integration/test_served_catalog_rate_backstop.py @@ -187,3 +187,71 @@ async def test_one_unreadable_stored_price_does_not_blank_the_catalog( served = {m.id for m in await list_models(integration_session, provider_id)} assert served == {"good"} + + +@pytest.mark.integration +@pytest.mark.asyncio +@pytest.mark.parametrize( + "field", + [ + "image", + "web_search", + "internal_reasoning", + "input_cache_read", + "input_cache_write", + ], +) +async def test_served_catalog_excludes_a_malformed_auxiliary_rate( + integration_session: AsyncSession, field: str +) -> None: + """The backstop covers every billable rate, not only the token rates. + + A price whose ``prompt``/``completion`` are sound can still carry a + malformed request, image, search, reasoning or cache rate — the catalog + import filter never inspects those — and the request that hits one is billed + against it just the same. + """ + provider_id = await _make_provider(integration_session) + await _insert_row( + integration_session, + provider_id, + model_id="good", + pricing={"prompt": 1e-06, "completion": 2e-06}, + ) + await _insert_row( + integration_session, + provider_id, + model_id="bad-aux", + pricing={"prompt": 1e-06, "completion": 2e-06, field: -1.0}, + ) + + served = {m.id for m in await list_models(integration_session, provider_id)} + + assert served == {"good"} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_a_negative_request_rate_is_clamped_on_read_and_still_served( + integration_session: AsyncSession, +) -> None: + """``request`` is the one billable rate the row-to-model conversion repairs. + + It clamps a negative stored ``request`` to zero before the price is built, + so the backstop never sees one and the row is served at a zero request rate + — money-safe, and the reason ``request`` is absent from the list of rates + above. Pinned here so that if the clamp goes, this rate joins that list + rather than quietly becoming the one unguarded field. + """ + provider_id = await _make_provider(integration_session) + await _insert_row( + integration_session, + provider_id, + model_id="neg-request", + pricing={"prompt": 1e-06, "completion": 2e-06, "request": -1.0}, + ) + + served = await list_models(integration_session, provider_id) + + assert [m.id for m in served] == ["neg-request"] + assert served[0].pricing.request == 0.0 diff --git a/tests/unit/test_pricing_rate_validation.py b/tests/unit/test_pricing_rate_validation.py index 9e4a5d6e..d3656d25 100644 --- a/tests/unit/test_pricing_rate_validation.py +++ b/tests/unit/test_pricing_rate_validation.py @@ -189,6 +189,12 @@ class _ExchangeResponse: self._payload = payload def json(self) -> dict[str, Any]: + # A quote given as an exception stands for a response body that never + # produced one: an exchange answering with an HTML error page raises + # out of `.json()` before any price is read. + quote = next(iter(self._payload.values())) + if isinstance(quote, BaseException): + raise quote return self._payload @@ -200,9 +206,10 @@ class _ExchangeClient: async def get(self, url: str) -> _ExchangeResponse: if "kraken" in url: - return _ExchangeResponse( - {"result": {"XXBTZUSD": {"c": [self._quotes["kraken"]]}}} - ) + quote = self._quotes["kraken"] + if isinstance(quote, BaseException): + return _ExchangeResponse({"error": quote}) + return _ExchangeResponse({"result": {"XXBTZUSD": {"c": [quote]}}}) if "coinbase" in url: return _ExchangeResponse({"data": {"amount": self._quotes["coinbase"]}}) return _ExchangeResponse({"price": self._quotes["binance"]}) @@ -297,6 +304,31 @@ async def test_boolean_exchange_quote_does_not_set_the_node_price( assert btc_usd_price() == pytest.approx(100000.0) +@pytest.mark.asyncio +async def test_an_unreadable_exchange_response_drops_only_that_quote( + refresh_price_with: Any, +) -> None: + """An exchange whose response never yields a quote costs one quote. + + The price is aggregated across three exchanges precisely so that one of them + having a bad day is survivable. A body that is not JSON, or whose shape moved + under the reader, raises before any number is seen; unhandled, it aborted the + whole aggregation and left the node on a stale rate even though two healthy + quotes were already in hand. + """ + from routstr.payment.price import btc_usd_price + + await refresh_price_with( + { + "kraken": ValueError("Expecting value: line 1 column 1 (char 0)"), + "coinbase": "100000.0", + "binance": "100000.0", + } + ) + + assert btc_usd_price() == pytest.approx(100000.0) + + @pytest.mark.asyncio async def test_all_quotes_unusable_keeps_the_last_good_price( refresh_price_with: Any, From fec229e1f4e147e7012f6527409e9a60f0f9d6dd Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 20:47:48 +0200 Subject: [PATCH 11/18] fix(pricing): ask the shared rate question when reading a feed price The feed-price coercion rejected `NaN`/`inf` but returned a negative unchanged, and the catalog import filter that would have caught one inspects only prompt and completion. A negative cache rate was the one malformed value still reaching a stored price, where it prices cached input tokens at a credit. Both resolver rungs now ask `is_usable_rate` rather than spelling the rule out again, so a feed price is held to the same standard as one arriving at the admin edge. The both-zero rejection stays: that is a free price, not a malformed one, and it is a separate question. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- routstr/upstream/pricing_resolver.py | 32 +++++++++++++---------- tests/unit/test_upstream_generic.py | 38 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/routstr/upstream/pricing_resolver.py b/routstr/upstream/pricing_resolver.py index 3064ba57..ef7e544c 100644 --- a/routstr/upstream/pricing_resolver.py +++ b/routstr/upstream/pricing_resolver.py @@ -15,7 +15,6 @@ it into the base provider unchanged. from __future__ import annotations -import math from dataclasses import dataclass, field @@ -66,19 +65,23 @@ def estimate_context_length(model_id: str) -> int: def _as_float(value: object) -> float | None: - """OpenRouter reports prices as strings; coerce, ``None`` if not a real number. + """OpenRouter reports prices as strings; coerce, ``None`` if not a real rate. - Non-finite values are rejected as unparseable: ``float("Infinity")`` and - ``float("NaN")`` parse happily from a feed string, and ``json.loads`` - accepts the bare literals and overflows ``1e999`` to ``inf``. An oversized - integer raises ``OverflowError`` rather than ``ValueError``, so that is - caught too. + Every caller reads a *price* out of a feed, so this asks the shared + billable-rate question rather than merely parsing: ``float("Infinity")`` and + ``float("NaN")`` parse happily from a feed string, ``json.loads`` accepts the + bare literals and overflows ``1e999`` to ``inf``, and a negative parses + cleanly into a rate that credits the caller. An oversized integer raises + ``OverflowError`` rather than ``ValueError``, so that is caught too. """ + # Lazy, like the litellm lookup below: the resolver stays import-light. + from ..payment.models import is_usable_rate + try: parsed = float(value) # type: ignore[arg-type] except (TypeError, ValueError, OverflowError): return None - return parsed if math.isfinite(parsed) else None + return parsed if is_usable_rate(parsed) else None def _as_int(value: object) -> int | None: @@ -89,7 +92,7 @@ def _as_int(value: object) -> int | None: def _from_litellm(model_id: str) -> ResolvedPricing | None: # Lazy import so the resolver stays import-light and shares the exact # lookup semantics used by cache-rate backfill. - from ..payment.models import litellm_cost_entry + from ..payment.models import is_usable_rate, litellm_cost_entry info = litellm_cost_entry(model_id) if info is None: @@ -103,12 +106,13 @@ def _from_litellm(model_id: str) -> ResolvedPricing | None: # moderation/rerank tiers do this) — treating 0/0 as resolved would serve # the model for free. Reject it (and any negative) so the caller falls # through, mirroring async_fetch_openrouter_models' _has_valid_pricing. - # A non-finite entry is junk, not a price: `inf` would bill an infinite - # amount and `NaN` poisons every total it enters (and defeats the `< 0` and - # `== 0` guards below, since both comparisons are False for `NaN`). - if not math.isfinite(prompt) or not math.isfinite(completion): + # A malformed entry is junk, not a price: `inf` would bill an infinite + # amount, `NaN` poisons every total it enters, and a negative credits the + # caller. `NaN` also defeats the both-zero guard below on its own, since + # every comparison against it is False. + if not is_usable_rate(prompt) or not is_usable_rate(completion): return None - if prompt < 0 or completion < 0 or (prompt == 0 and completion == 0): + if prompt == 0 and completion == 0: return None input_modalities = ["text"] diff --git a/tests/unit/test_upstream_generic.py b/tests/unit/test_upstream_generic.py index bf5316a1..04470a76 100644 --- a/tests/unit/test_upstream_generic.py +++ b/tests/unit/test_upstream_generic.py @@ -629,3 +629,41 @@ async def test_non_finite_openrouter_cache_rate_is_dropped_not_carried() -> None assert model.enabled is True assert model.pricing.prompt == pytest.approx(1e-06) assert model.pricing.input_cache_read == 0.0 + + +@pytest.mark.asyncio +async def test_negative_openrouter_cache_rate_is_dropped_not_carried() -> None: + """A negative rate is as unusable as a non-finite one, and arrives the same way. + + The coercion that reads a feed price rejected ``NaN``/``inf`` but returned a + negative unchanged, and the catalog import filter that would have caught one + inspects only prompt and completion. So a negative cache rate was the single + malformed value that still reached a stored price — where it prices cached + input tokens at a credit rather than a charge. + """ + payload = { + "data": [ + {"id": "or-negcache-xyz", "object": "model", "owned_by": "mystery"}, + ] + } + feed = [ + { + "id": "or-negcache-xyz", + "pricing": { + "prompt": "0.000001", + "completion": "0.000002", + "input_cache_read": "-0.0000005", + }, + "context_length": 8192, + } + ] + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=feed) + with patch("routstr.payment.models.async_fetch_openrouter_models", or_feed): + models = await GenericUpstreamProvider(base_url="http://x").fetch_models() + + model = _model_by_id(models, "or-negcache-xyz") + assert model.enabled is True + assert model.pricing.prompt == pytest.approx(1e-06) + assert model.pricing.input_cache_read == 0.0 From 568a8b1205ebc53856d9449e74d7ce0d58c6e2c8 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 21:24:29 +0200 Subject: [PATCH 12/18] test(pricing): read the catalog through the test session only The provider helper called reinitialize_upstreams(), which re-reads the provider table through the global engine rather than the session the test was handed. On a developer machine that engine finds keys.db and the call is invisible; on a clean checkout there is no such database and every test in the file fails with "no such table: upstream_providers". Nothing here needs it: these tests call list_models() with the session directly and never route a request, so the proxy's cached upstreams are not part of what is under test. Dropping the call also stops the file mutating that global for whatever runs next. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM --- tests/integration/test_served_catalog_rate_backstop.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration/test_served_catalog_rate_backstop.py b/tests/integration/test_served_catalog_rate_backstop.py index 7819c017..4acee442 100644 --- a/tests/integration/test_served_catalog_rate_backstop.py +++ b/tests/integration/test_served_catalog_rate_backstop.py @@ -19,7 +19,6 @@ from sqlmodel.ext.asyncio.session import AsyncSession from routstr.core.db import ModelRow, UpstreamProviderRow from routstr.payment.models import list_models -from routstr.proxy import reinitialize_upstreams _ARCHITECTURE = json.dumps( { @@ -42,7 +41,6 @@ async def _make_provider(session: AsyncSession) -> int: session.add(provider) await session.commit() await session.refresh(provider) - await reinitialize_upstreams() assert provider.id is not None return provider.id From 6318a3bdbb58f68c152b2714f1eb1080645ad1bb Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Wed, 26 Aug 2026 15:29:04 +0200 Subject: [PATCH 13/18] refactor(pricing): give the usable-rate question a module of its own `is_usable_rate` lived in `payment/models.py`, which pulls in httpx, fastapi, the DB session and settings. Every consumer therefore reached it through a deferred import to avoid the cycle: five of them, in four files. Move the predicate and the billable-rate tuple to `payment/rates.py`, which imports only `math`, and let the call sites import it normally. The rationale now lives in that one docstring, so each guard says what it does with the answer instead of repeating why the answer matters. No behaviour change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014X8RZzzbAuQCbavhFjTvJ4 --- routstr/core/admin.py | 3 +- routstr/payment/cost_calculation.py | 19 +++-------- routstr/payment/models.py | 40 +---------------------- routstr/payment/price.py | 14 +++----- routstr/payment/rates.py | 48 ++++++++++++++++++++++++++++ routstr/upstream/pricing_resolver.py | 25 ++++++--------- 6 files changed, 69 insertions(+), 80 deletions(-) create mode 100644 routstr/payment/rates.py diff --git a/routstr/core/admin.py b/routstr/core/admin.py index f018b6a5..1af941d0 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -12,11 +12,10 @@ from sqlmodel import select from sqlmodel.ext.asyncio.session import AsyncSession from ..payment.models import ( - BILLABLE_PRICING_FIELDS, _row_to_model, - is_usable_rate, list_models, ) +from ..payment.rates import BILLABLE_PRICING_FIELDS, is_usable_rate from ..proxy import refresh_model_maps, reinitialize_upstreams from ..wallet import fetch_all_balances, send_token, token_mint_url from . import vault diff --git a/routstr/payment/cost_calculation.py b/routstr/payment/cost_calculation.py index a51f633d..ea45abe6 100644 --- a/routstr/payment/cost_calculation.py +++ b/routstr/payment/cost_calculation.py @@ -6,6 +6,7 @@ from pydantic.v1 import BaseModel from ..core import get_logger from ..core.settings import settings from .price import sats_usd_price +from .rates import is_usable_rate from .usage import normalize_usage, parse_token_count if TYPE_CHECKING: @@ -268,16 +269,8 @@ async def calculate_cost( else: input_rate, output_rate, cache_read_rate, cache_creation_rate = pricing_rates - # Local import mirrors this module's existing lazy pricing imports. - from .models import is_usable_rate - - # An unusable rate is not "no pricing" to Python's truthiness: `NaN` and a - # negative float are both truthy, so they sailed past this gate — the one - # guard meant to catch a rate that cannot be billed on — and reached the - # token math, which raises `ValueError` on `NaN` and `OverflowError` on - # `inf` *after* the response was served (the streaming handlers swallow - # that, so the request goes unbilled), while a negative produced a negative - # charge. Ask whether each rate is usable rather than whether it is truthy. + # Truthiness is not the question: `NaN` and a negative rate are both truthy + # and sailed past this gate into the token math. rates = (input_rate, output_rate, cache_read_rate, cache_creation_rate) if not all(is_usable_rate(rate) for rate in rates) or not ( input_rate and output_rate @@ -343,9 +336,6 @@ def _coerce_usd(value: object) -> float: ``0.0`` means "no usable figure" to every caller, which is the same thing an absent field means, so the caller's existing ``> 0`` checks handle it. """ - # Local import mirrors this module's existing lazy pricing imports. - from .models import is_usable_rate - if value is None or isinstance(value, bool): return 0.0 if not isinstance(value, (int, float, str)): @@ -355,8 +345,7 @@ def _coerce_usd(value: object) -> float: amount = float(value) except (TypeError, ValueError, OverflowError): return 0.0 - # `is_usable_rate` also rejects negatives, which the previous `max(0.0, …)` - # clamped to zero — same outcome, stated once instead of inline. + # A negative is rejected here, where the previous `max(0.0, …)` clamped it. return amount if is_usable_rate(amount) else 0.0 diff --git a/routstr/payment/models.py b/routstr/payment/models.py index 7f3a20f3..62773371 100644 --- a/routstr/payment/models.py +++ b/routstr/payment/models.py @@ -1,6 +1,5 @@ import asyncio import json -import math import random import httpx @@ -13,6 +12,7 @@ from ..core.db import ModelRow, UpstreamProviderRow, get_session from ..core.logging import get_logger from ..core.settings import settings from .price import sats_usd_price +from .rates import BILLABLE_PRICING_FIELDS, is_usable_rate logger = get_logger(__name__) @@ -59,44 +59,6 @@ class Pricing(BaseModel): max_cost: float = 0.0 # in sats not msats -# The rates a request can bill on. Derived fields (``max_*_cost``) are excluded -# — they are computed carriers, not charged rates. One definition, shared by the -# admin write edge and the served/routed guards, so they all cover the same set. -BILLABLE_PRICING_FIELDS = ( - "prompt", - "completion", - "request", - "image", - "web_search", - "internal_reasoning", - "input_cache_read", - "input_cache_write", -) - - -def is_usable_rate(rate: float) -> bool: - """True if a single billable rate is a number a request could be billed on. - - The one definition of a usable rate, so every guard that asks the question - answers it identically. A rate qualifies only when it is finite and - non-negative; zero is usable (it means "free", which is a real price) but - ``NaN``, ``±inf`` and negatives are not prices at all. - - Non-finite: ``inf > 0`` is True, so an infinite rate reads as chargeable and - would be served, routed and billed as ``inf``; ``NaN`` poisons every total it - enters and defeats ordinary comparisons, since ``NaN > 0``, ``NaN < 0`` and - ``NaN == 0`` are all False. Negative: a negative rate produces a negative - cost, which the settlement path subtracts from the balance — it pays the - caller to make requests. - - Both reach a stored row from upstream catalogs as well as the admin edge - (``json.loads`` accepts the bare ``NaN``/``Infinity`` literals and overflows - ``1e999`` to ``inf``), so the check belongs in one shared place rather than - at each writer. - """ - return math.isfinite(rate) and rate >= 0.0 - - def has_usable_pricing(pricing: Pricing) -> bool: """True if every billable rate is a number a request could be billed on. diff --git a/routstr/payment/price.py b/routstr/payment/price.py index 93ed68e9..48fc5b65 100644 --- a/routstr/payment/price.py +++ b/routstr/payment/price.py @@ -5,6 +5,7 @@ import httpx from ..core import get_logger from ..core.settings import settings +from .rates import is_usable_rate logger = get_logger(__name__) @@ -22,16 +23,11 @@ def _parse_quote(raw: object, exchange: str) -> float | None: raises out of the integer conversion in settlement, and a negative rate produces a negative charge that is credited back to the caller. - ``is_usable_rate`` is the same predicate the billable-rate guards use, so - "finite and non-negative" has one definition; a quote is stricter still and - must be positive, since a BTC price of zero is a broken feed, not free money. - Imported lazily because ``payment.models`` imports this module. + A quote is stricter than a billable rate: it must be positive, since a BTC + price of zero is a broken feed, not free money. """ - from .models import is_usable_rate - - # A boolean is a shape change, not a price: `float(True)` is a finite, - # positive 1.0 that passes every numeric guard below and then wins the - # `min()`, pricing the node at one dollar per bitcoin. + # `float(True)` is a finite, positive 1.0 that passes every guard below and + # would then win the `min()`, pricing the node at one dollar per bitcoin. if isinstance(raw, bool): logger.warning( "Non-numeric price quote — ignoring this exchange", diff --git a/routstr/payment/rates.py b/routstr/payment/rates.py new file mode 100644 index 00000000..4ac74587 --- /dev/null +++ b/routstr/payment/rates.py @@ -0,0 +1,48 @@ +"""The one definition of a billable rate, with no dependencies of its own. + +A rate reaches the node from an upstream catalog, the LiteLLM cost map, an +operator's admin edit, a legacy database row and the BTC/USD feed. Each of those +readers needs the same two questions answered — is this value a rate at all, and +is it a rate a request can be billed on — so both answers live here, in a module +that imports nothing from the package. Every guard then shares one definition +instead of drifting, and no caller needs a deferred import to reach it. +""" + +import math + +# The rates a request can bill on. Derived fields (``max_*_cost``) are excluded +# — they are computed carriers, not charged rates. One definition, shared by the +# admin write edge and the served/routed guards, so they all cover the same set. +BILLABLE_PRICING_FIELDS = ( + "prompt", + "completion", + "request", + "image", + "web_search", + "internal_reasoning", + "input_cache_read", + "input_cache_write", +) + + +def is_usable_rate(rate: float) -> bool: + """True if a single billable rate is a number a request could be billed on. + + The one definition of a usable rate, so every guard that asks the question + answers it identically. A rate qualifies only when it is finite and + non-negative; zero is usable (it means "free", which is a real price) but + ``NaN``, ``±inf`` and negatives are not prices at all. + + Non-finite: ``inf > 0`` is True, so an infinite rate reads as chargeable and + would be served, routed and billed as ``inf``; ``NaN`` poisons every total it + enters and defeats ordinary comparisons, since ``NaN > 0``, ``NaN < 0`` and + ``NaN == 0`` are all False. Negative: a negative rate produces a negative + cost, which the settlement path subtracts from the balance — it pays the + caller to make requests. Both reach a stored row from upstream catalogs as + well as the admin edge (``json.loads`` accepts the bare ``NaN``/``Infinity`` + literals and overflows ``1e999`` to ``inf``). + + This is the rationale for every guard that calls it; the call sites say what + they do with the answer, not why the answer matters. + """ + return math.isfinite(rate) and rate >= 0.0 diff --git a/routstr/upstream/pricing_resolver.py b/routstr/upstream/pricing_resolver.py index ef7e544c..b12a7f1e 100644 --- a/routstr/upstream/pricing_resolver.py +++ b/routstr/upstream/pricing_resolver.py @@ -17,6 +17,8 @@ from __future__ import annotations from dataclasses import dataclass, field +from ..payment.rates import is_usable_rate + @dataclass class ResolvedPricing: @@ -68,15 +70,10 @@ def _as_float(value: object) -> float | None: """OpenRouter reports prices as strings; coerce, ``None`` if not a real rate. Every caller reads a *price* out of a feed, so this asks the shared - billable-rate question rather than merely parsing: ``float("Infinity")`` and - ``float("NaN")`` parse happily from a feed string, ``json.loads`` accepts the - bare literals and overflows ``1e999`` to ``inf``, and a negative parses - cleanly into a rate that credits the caller. An oversized integer raises - ``OverflowError`` rather than ``ValueError``, so that is caught too. + billable-rate question rather than merely parsing: ``float("Infinity")``, + ``float("NaN")`` and a negative all parse cleanly from a feed string. An + oversized integer raises ``OverflowError`` rather than ``ValueError``. """ - # Lazy, like the litellm lookup below: the resolver stays import-light. - from ..payment.models import is_usable_rate - try: parsed = float(value) # type: ignore[arg-type] except (TypeError, ValueError, OverflowError): @@ -90,9 +87,9 @@ def _as_int(value: object) -> int | None: def _from_litellm(model_id: str) -> ResolvedPricing | None: - # Lazy import so the resolver stays import-light and shares the exact - # lookup semantics used by cache-rate backfill. - from ..payment.models import is_usable_rate, litellm_cost_entry + # Lazy import so the resolver shares the exact lookup semantics used by + # cache-rate backfill without importing the models module at load time. + from ..payment.models import litellm_cost_entry info = litellm_cost_entry(model_id) if info is None: @@ -106,10 +103,8 @@ def _from_litellm(model_id: str) -> ResolvedPricing | None: # moderation/rerank tiers do this) — treating 0/0 as resolved would serve # the model for free. Reject it (and any negative) so the caller falls # through, mirroring async_fetch_openrouter_models' _has_valid_pricing. - # A malformed entry is junk, not a price: `inf` would bill an infinite - # amount, `NaN` poisons every total it enters, and a negative credits the - # caller. `NaN` also defeats the both-zero guard below on its own, since - # every comparison against it is False. + # Checked before the both-zero guard below: `NaN` defeats that guard on its + # own, since every comparison against it is False. if not is_usable_rate(prompt) or not is_usable_rate(completion): return None if prompt == 0 and completion == 0: From dff164b98031e257f228b6f84a2459449f7b0250 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Wed, 26 Aug 2026 15:44:29 +0200 Subject: [PATCH 14/18] fix(pricing): a rate spelled as a boolean is not a rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `isinstance(True, int)` is True and `float(True)` is `1.0`, so a JSON `true` in a catalog was a finite, positive rate that passed every numeric guard: a dollar per token. It reached a stored price through the OpenRouter feed filter, the LiteLLM cost-map rung and the OpenRouter resolver rung. The write edge and the exchange feed already rejected booleans explicitly, which is the shape of the real problem: four readers were each parsing a rate for themselves and disagreeing about what a rate is. Give them one coercion — `coerce_rate` — and let it answer for all of them. A numeric string stays a rate, because feeds report prices as strings; that now holds on the LiteLLM rung too, which previously required a float outright. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014X8RZzzbAuQCbavhFjTvJ4 --- routstr/core/admin.py | 20 +++---- routstr/payment/cost_calculation.py | 18 ++---- routstr/payment/models.py | 21 +++---- routstr/payment/price.py | 38 ++----------- routstr/payment/rates.py | 23 ++++++++ routstr/upstream/pricing_resolver.py | 35 ++++-------- .../test_admin_pricing_rate_validation.py | 22 +++++++ tests/unit/test_pricing_rate_validation.py | 20 +++++++ tests/unit/test_upstream_generic.py | 57 +++++++++++++++++++ 9 files changed, 160 insertions(+), 94 deletions(-) diff --git a/routstr/core/admin.py b/routstr/core/admin.py index 1af941d0..f02ec441 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -15,7 +15,7 @@ from ..payment.models import ( _row_to_model, list_models, ) -from ..payment.rates import BILLABLE_PRICING_FIELDS, is_usable_rate +from ..payment.rates import BILLABLE_PRICING_FIELDS, coerce_rate from ..proxy import refresh_model_maps, reinitialize_upstreams from ..wallet import fetch_all_balances, send_token, token_mint_url from . import vault @@ -542,17 +542,13 @@ class ModelCreate(BaseModel): raw = value.get(field) if raw is None: continue - if isinstance(raw, bool) or not isinstance(raw, (int, float, str)): - raise ValueError(f"{field} must be a non-negative number") - try: - rate = float(raw) - except (ValueError, OverflowError): - # An integer too large for a float raises OverflowError, which - # pydantic does not convert into a validation error — unhandled - # it escapes as a 500 for what is still a bad client value. - raise ValueError(f"{field} must be a number, got {raw!r}") - if not is_usable_rate(rate): - raise ValueError(f"{field} must be a finite, non-negative number") + # The shared coercion also absorbs the OverflowError an oversized + # integer raises, which pydantic does not convert into a validation + # error — unhandled it escaped as a 500 for a bad client value. + if coerce_rate(raw) is None: + raise ValueError( + f"{field} must be a finite, non-negative number, got {raw!r}" + ) return value diff --git a/routstr/payment/cost_calculation.py b/routstr/payment/cost_calculation.py index ea45abe6..e7cfdb87 100644 --- a/routstr/payment/cost_calculation.py +++ b/routstr/payment/cost_calculation.py @@ -6,7 +6,7 @@ from pydantic.v1 import BaseModel from ..core import get_logger from ..core.settings import settings from .price import sats_usd_price -from .rates import is_usable_rate +from .rates import coerce_rate, is_usable_rate from .usage import normalize_usage, parse_token_count if TYPE_CHECKING: @@ -336,17 +336,11 @@ def _coerce_usd(value: object) -> float: ``0.0`` means "no usable figure" to every caller, which is the same thing an absent field means, so the caller's existing ``> 0`` checks handle it. """ - if value is None or isinstance(value, bool): - return 0.0 - if not isinstance(value, (int, float, str)): - return 0.0 - try: - # An oversized integer raises OverflowError, not ValueError. - amount = float(value) - except (TypeError, ValueError, OverflowError): - return 0.0 - # A negative is rejected here, where the previous `max(0.0, …)` clamped it. - return amount if is_usable_rate(amount) else 0.0 + # A cost figure is coerced exactly like a rate; only the way an unusable one + # is reported differs. A negative is rejected here, where the previous + # `max(0.0, …)` clamped it. + amount = coerce_rate(value) + return amount if amount is not None else 0.0 def _resolve_usd_cost(usage_data: dict, response_data: dict) -> float: diff --git a/routstr/payment/models.py b/routstr/payment/models.py index 62773371..49ca89b1 100644 --- a/routstr/payment/models.py +++ b/routstr/payment/models.py @@ -12,7 +12,7 @@ from ..core.db import ModelRow, UpstreamProviderRow, get_session from ..core.logging import get_logger from ..core.settings import settings from .price import sats_usd_price -from .rates import BILLABLE_PRICING_FIELDS, is_usable_rate +from .rates import BILLABLE_PRICING_FIELDS, coerce_rate, is_usable_rate logger = get_logger(__name__) @@ -163,19 +163,12 @@ def _has_valid_pricing(model: dict) -> bool: if not pricing: return False - try: - prompt = float(pricing.get("prompt", 0)) - completion = float(pricing.get("completion", 0)) - except (ValueError, TypeError, OverflowError): - # An integer too large for a float raises OverflowError, not - # ValueError, so it escaped this coercion guard and unwound the whole - # fetch — one junk entry cost the node the entire upstream catalog. - return False - - # `NaN`/`±inf` are not prices, and neither is caught by the checks below: - # every comparison with `NaN` is False, and `inf` reads as a large positive - # rate that would be advertised and billed on. - if not is_usable_rate(prompt) or not is_usable_rate(completion): + # Coercion runs before the both-zero test below, which `NaN` would defeat + # on its own — and one entry the coercion chokes on must not unwind the + # whole fetch, which once cost the node an entire upstream catalog. + prompt = coerce_rate(pricing.get("prompt", 0)) + completion = coerce_rate(pricing.get("completion", 0)) + if prompt is None or completion is None: return False if prompt == 0 and completion == 0: diff --git a/routstr/payment/price.py b/routstr/payment/price.py index 48fc5b65..ae162398 100644 --- a/routstr/payment/price.py +++ b/routstr/payment/price.py @@ -5,7 +5,7 @@ import httpx from ..core import get_logger from ..core.settings import settings -from .rates import is_usable_rate +from .rates import coerce_rate logger = get_logger(__name__) @@ -19,40 +19,14 @@ def _parse_quote(raw: object, exchange: str) -> float | None: Every quote passes through here because the aggregator takes the ``min()`` of what it collects: an unusable quote does not merely join the sample, it *wins* it, and the result is the rate every model and every request on the - node is priced at. A zero divides by zero on the USD cost path, ``NaN`` - raises out of the integer conversion in settlement, and a negative rate - produces a negative charge that is credited back to the caller. - - A quote is stricter than a billable rate: it must be positive, since a BTC - price of zero is a broken feed, not free money. + node is priced at. A quote is stricter than a billable rate — it must be + positive, since a BTC price of zero is a broken feed, not free money. """ - # `float(True)` is a finite, positive 1.0 that passes every guard below and - # would then win the `min()`, pricing the node at one dollar per bitcoin. - if isinstance(raw, bool): - logger.warning( - "Non-numeric price quote — ignoring this exchange", - extra={"exchange": exchange, "quote": repr(raw)}, - ) - return None - - try: - price = float(raw) # type: ignore[arg-type] - except (TypeError, ValueError, OverflowError) as e: - logger.warning( - "Unparseable price quote — ignoring this exchange", - extra={ - "error": str(e), - "error_type": type(e).__name__, - "exchange": exchange, - "quote": repr(raw), - }, - ) - return None - - if not is_usable_rate(price) or price <= 0: + price = coerce_rate(raw) + if price is None or price <= 0: logger.warning( "Unusable price quote — ignoring this exchange", - extra={"exchange": exchange, "quote": price}, + extra={"exchange": exchange, "quote": repr(raw)}, ) return None diff --git a/routstr/payment/rates.py b/routstr/payment/rates.py index 4ac74587..998f3ca5 100644 --- a/routstr/payment/rates.py +++ b/routstr/payment/rates.py @@ -46,3 +46,26 @@ def is_usable_rate(rate: float) -> bool: they do with the answer, not why the answer matters. """ return math.isfinite(rate) and rate >= 0.0 + + +def coerce_rate(value: object) -> float | None: + """Coerce a value from outside the node to a usable rate, or ``None``. + + The one coercion, shared by every reader of a rate the node did not compute + itself: an upstream catalog, the LiteLLM cost map, the exchange feed and the + admin write edge. Each of them was parsing for itself, and they disagreed — + which is how a boolean became a price on some paths and not others. + + A boolean is rejected outright: it is a change of shape, not a rate, and + Python would make ``True`` a finite, positive ``1.0`` that passes every + numeric guard downstream — a dollar per token. A numeric string is accepted, + because feeds report prices as strings. An oversized integer raises + ``OverflowError`` rather than ``ValueError``, so that is caught too. + """ + if isinstance(value, bool) or not isinstance(value, (int, float, str)): + return None + try: + rate = float(value) + except (TypeError, ValueError, OverflowError): + return None + return rate if is_usable_rate(rate) else None diff --git a/routstr/upstream/pricing_resolver.py b/routstr/upstream/pricing_resolver.py index b12a7f1e..8b58dddb 100644 --- a/routstr/upstream/pricing_resolver.py +++ b/routstr/upstream/pricing_resolver.py @@ -17,7 +17,7 @@ from __future__ import annotations from dataclasses import dataclass, field -from ..payment.rates import is_usable_rate +from ..payment.rates import coerce_rate @dataclass @@ -67,18 +67,8 @@ def estimate_context_length(model_id: str) -> int: def _as_float(value: object) -> float | None: - """OpenRouter reports prices as strings; coerce, ``None`` if not a real rate. - - Every caller reads a *price* out of a feed, so this asks the shared - billable-rate question rather than merely parsing: ``float("Infinity")``, - ``float("NaN")`` and a negative all parse cleanly from a feed string. An - oversized integer raises ``OverflowError`` rather than ``ValueError``. - """ - try: - parsed = float(value) # type: ignore[arg-type] - except (TypeError, ValueError, OverflowError): - return None - return parsed if is_usable_rate(parsed) else None + """OpenRouter reports prices as strings; coerce, ``None`` if not a real rate.""" + return coerce_rate(value) def _as_int(value: object) -> int | None: @@ -95,18 +85,15 @@ def _from_litellm(model_id: str) -> ResolvedPricing | None: if info is None: return None - prompt = info.get("input_cost_per_token") - completion = info.get("output_cost_per_token") - if not isinstance(prompt, (int, float)) or not isinstance(completion, (int, float)): + prompt = coerce_rate(info.get("input_cost_per_token")) + completion = coerce_rate(info.get("output_cost_per_token")) + if prompt is None or completion is None: return None # A both-zero entry is litellm listing a model without a real price (free # moderation/rerank tiers do this) — treating 0/0 as resolved would serve - # the model for free. Reject it (and any negative) so the caller falls - # through, mirroring async_fetch_openrouter_models' _has_valid_pricing. - # Checked before the both-zero guard below: `NaN` defeats that guard on its - # own, since every comparison against it is False. - if not is_usable_rate(prompt) or not is_usable_rate(completion): - return None + # the model for free. Reject it so the caller falls through, mirroring + # async_fetch_openrouter_models' _has_valid_pricing. Coercion runs first: + # `NaN` would defeat this guard on its own, every comparison being False. if prompt == 0 and completion == 0: return None @@ -115,8 +102,8 @@ def _from_litellm(model_id: str) -> ResolvedPricing | None: input_modalities.append("image") return ResolvedPricing( - prompt=float(prompt), - completion=float(completion), + prompt=prompt, + completion=completion, # max_input_tokens is the context window; max_tokens is litellm's # completion cap (it tracks max_output_tokens for ~94% of models), so # it is never a context source. A missing window falls to the id-based diff --git a/tests/integration/test_admin_pricing_rate_validation.py b/tests/integration/test_admin_pricing_rate_validation.py index 5b53b02a..5a3d06c4 100644 --- a/tests/integration/test_admin_pricing_rate_validation.py +++ b/tests/integration/test_admin_pricing_rate_validation.py @@ -148,6 +148,28 @@ async def test_malformed_price_string_is_rejected( assert await integration_session.get(ModelRow, ("bad-price", provider_id)) is None +@pytest.mark.integration +@pytest.mark.asyncio +async def test_boolean_price_is_rejected( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """A JSON ``true`` coerces to a finite, positive ``1.0`` — a dollar per + token — so it passes every numeric guard. The write edge asks the same + coercion the catalog readers do, and answers a 422.""" + provider_id = await _make_provider(integration_session) + + resp = await integration_client.post( + f"/admin/api/upstream-providers/{provider_id}/models", + headers=_admin_headers(), + json=_payload( + provider_id, model_id="bool-price", pricing=_pricing(prompt=True) + ), + ) + + assert resp.status_code == 422 + assert await integration_session.get(ModelRow, ("bool-price", provider_id)) is None + + @pytest.mark.integration @pytest.mark.asyncio async def test_numeric_string_price_is_still_accepted( diff --git a/tests/unit/test_pricing_rate_validation.py b/tests/unit/test_pricing_rate_validation.py index d3656d25..fab4e6d5 100644 --- a/tests/unit/test_pricing_rate_validation.py +++ b/tests/unit/test_pricing_rate_validation.py @@ -442,3 +442,23 @@ async def test_oversized_catalog_rate_does_not_empty_the_catalog() -> None: models = await async_fetch_openrouter_models() assert [m["id"] for m in models] == ["good"] + + +@pytest.mark.asyncio +async def test_boolean_catalog_rate_is_not_imported() -> None: + """A JSON ``true`` is a change of shape, not a price. + + Python coerces it to a finite, positive ``1.0`` — a dollar per token — so it + passes every numeric guard and must be rejected before coercion. + """ + with _patch_openrouter_catalog( + [ + _catalog_entry("bad", {"prompt": True, "completion": "0.000002"}), + _catalog_entry("good", {"prompt": "0.000001", "completion": "0.000002"}), + ] + ): + from routstr.payment.models import async_fetch_openrouter_models + + models = await async_fetch_openrouter_models() + + assert [m["id"] for m in models] == ["good"] diff --git a/tests/unit/test_upstream_generic.py b/tests/unit/test_upstream_generic.py index 04470a76..2e6fd56e 100644 --- a/tests/unit/test_upstream_generic.py +++ b/tests/unit/test_upstream_generic.py @@ -667,3 +667,60 @@ async def test_negative_openrouter_cache_rate_is_dropped_not_carried() -> None: assert model.enabled is True assert model.pricing.prompt == pytest.approx(1e-06) assert model.pricing.input_cache_read == 0.0 + + +@pytest.mark.asyncio +async def test_boolean_litellm_rate_is_not_a_resolved_price() -> None: + """``isinstance(True, int)`` is True, so a boolean passed the cost map's own + numeric check and resolved as a rate of ``1.0`` — a dollar per token.""" + payload = { + "data": [ + {"id": "bool-priced-model", "object": "model", "owned_by": "mystery"}, + ] + } + cost_entry = { + "input_cost_per_token": True, + "output_cost_per_token": 2e-06, + "max_input_tokens": 8192, + } + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=[]) + with patch("routstr.payment.models.litellm_cost_entry", lambda _id: cost_entry): + with patch("routstr.payment.models.async_fetch_openrouter_models", or_feed): + models = await GenericUpstreamProvider( + base_url="http://x" + ).fetch_models() + + model = _model_by_id(models, "bool-priced-model") + assert model.enabled is False + assert model.pricing.prompt == 0.0 + assert model.pricing.completion == 0.0 + + +@pytest.mark.asyncio +async def test_boolean_openrouter_rate_is_not_a_resolved_price() -> None: + """The same coercion reads a feed's ``true`` as a rate of ``1.0``; the model + must import disabled rather than priced at a dollar per token.""" + payload = { + "data": [ + {"id": "or-bool-xyz", "object": "model", "owned_by": "mystery"}, + ] + } + feed = [ + { + "id": "or-bool-xyz", + "pricing": {"prompt": True, "completion": "0.000002"}, + "context_length": 8192, + } + ] + + with _patch_models_endpoint(payload): + or_feed = AsyncMock(return_value=feed) + with patch("routstr.payment.models.async_fetch_openrouter_models", or_feed): + models = await GenericUpstreamProvider(base_url="http://x").fetch_models() + + model = _model_by_id(models, "or-bool-xyz") + assert model.enabled is False + assert model.pricing.prompt == 0.0 + assert model.pricing.completion == 0.0 From 337be4895eef2191e215e0e8701c34aef0b015ec Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Wed, 26 Aug 2026 15:47:44 +0200 Subject: [PATCH 15/18] fix(billing): a rate of zero is a price, not a missing price MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate that decides a model cannot be priced on tokens asked `input_rate and output_rate`, so a rate of zero read as an absent one and the request was billed the whole reservation. The catalog serves such a model and the router routes it — this PR says so in as many words — and then billing charged it as if it had no price at all. For a model that is free on both sides the reservation is the minimum-request floor, so the overcharge is a msat. The bite is a model free on one side only: the reservation there is the context window priced at the paid rate, charged flat on every request no matter how few tokens it used. The catalog import filter rejects only a both-zero price, so those models are ingested, served and routed today. Ask only whether each rate is usable. A zero rate now bills as what it is. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014X8RZzzbAuQCbavhFjTvJ4 --- routstr/payment/cost_calculation.py | 8 ++++---- tests/unit/test_pricing_rate_validation.py | 24 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/routstr/payment/cost_calculation.py b/routstr/payment/cost_calculation.py index e7cfdb87..a4ded317 100644 --- a/routstr/payment/cost_calculation.py +++ b/routstr/payment/cost_calculation.py @@ -270,11 +270,11 @@ async def calculate_cost( input_rate, output_rate, cache_read_rate, cache_creation_rate = pricing_rates # Truthiness is not the question: `NaN` and a negative rate are both truthy - # and sailed past this gate into the token math. + # and sailed past this gate into the token math, while a rate of zero is a + # price — free — and reading it as a missing one charged the whole + # reservation for a request the model serves for nothing. rates = (input_rate, output_rate, cache_read_rate, cache_creation_rate) - if not all(is_usable_rate(rate) for rate in rates) or not ( - input_rate and output_rate - ): + if not all(is_usable_rate(rate) for rate in rates): logger.warning( "No usable token pricing — billing at flat MaxCostData. " "Token counts %s in the upstream response but cannot be " diff --git a/tests/unit/test_pricing_rate_validation.py b/tests/unit/test_pricing_rate_validation.py index fab4e6d5..f0390c9b 100644 --- a/tests/unit/test_pricing_rate_validation.py +++ b/tests/unit/test_pricing_rate_validation.py @@ -92,6 +92,30 @@ async def test_unusable_token_rate_falls_back_to_max_cost(bad_rate: float) -> No assert cost.total_msats == 1234 +@pytest.mark.parametrize( + ("prompt", "completion", "expected_msats"), + [(0.0, 0.0, 0), (0.0, 2e-06, 1), (1e-06, 0.0, 1)], + ids=["free", "free-input", "free-output"], +) +@pytest.mark.asyncio +async def test_a_rate_of_zero_is_billed_as_free_not_as_missing( + prompt: float, completion: float, expected_msats: int +) -> None: + """Zero is a price, and the request must be billed on it. + + The gate that decides a model has no token pricing was a truthiness test, so + a free rate read as an absent one and the request was charged the whole + reservation instead — on a model priced at zero for that side, which is a + price the catalog serves and the router routes. + """ + model = _model(Pricing(prompt=prompt, completion=completion)) + + cost = await calculate_cost(_usage_response(), max_cost=1234, model_obj=model) + + assert isinstance(cost, CostData) + assert cost.total_msats == expected_msats + + @pytest.mark.parametrize( "junk", [float("inf"), float("nan"), "Infinity"], ids=["inf", "nan", "inf-string"] ) From 027bb3da31d39a32d9e79374a2eb383377c8f12b Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Wed, 26 Aug 2026 15:50:26 +0200 Subject: [PATCH 16/18] fix(pricing): reject an exchange quote that underflows the sats price MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A quote of `1e-320` is finite and positive, so it passed the feed's guards and then won the `min()` — but the node prices in sats, and `1e-320 / 100_000_000` is `0.0`. A zero sats price divides by zero on every model's rate, so one malformed feed could take the node's pricing down while two healthy quotes stood beside it. Require the quote to survive the conversion it is going to be put through, and name the divisor while it has two call sites. Also drops a line from the boolean-quote test that said this coercion was the only one rejecting booleans; they all share one now. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014X8RZzzbAuQCbavhFjTvJ4 --- routstr/payment/price.py | 10 +++++++--- tests/unit/test_pricing_rate_validation.py | 23 +++++++++++++++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/routstr/payment/price.py b/routstr/payment/price.py index ae162398..e63d6754 100644 --- a/routstr/payment/price.py +++ b/routstr/payment/price.py @@ -12,6 +12,8 @@ logger = get_logger(__name__) BTC_USD_PRICE: float | None = None SATS_USD_PRICE: float | None = None +SATS_PER_BTC = 100_000_000 + def _parse_quote(raw: object, exchange: str) -> float | None: """Coerce an exchange quote to a price, or ``None`` if it is not one. @@ -20,10 +22,12 @@ def _parse_quote(raw: object, exchange: str) -> float | None: of what it collects: an unusable quote does not merely join the sample, it *wins* it, and the result is the rate every model and every request on the node is priced at. A quote is stricter than a billable rate — it must be - positive, since a BTC price of zero is a broken feed, not free money. + positive, and positive *after* the sats conversion the node prices in: a + subnormal quote survives every guard here and still underflows to a zero + sats price, which then divides by zero on every model's rate. """ price = coerce_rate(raw) - if price is None or price <= 0: + if price is None or price <= 0 or price / SATS_PER_BTC <= 0: logger.warning( "Unusable price quote — ignoring this exchange", extra={"exchange": exchange, "quote": repr(raw)}, @@ -141,7 +145,7 @@ async def _update_prices() -> None: ) return BTC_USD_PRICE = btc_price - SATS_USD_PRICE = btc_price / 100_000_000 + SATS_USD_PRICE = btc_price / SATS_PER_BTC def btc_usd_price() -> float: diff --git a/tests/unit/test_pricing_rate_validation.py b/tests/unit/test_pricing_rate_validation.py index f0390c9b..895fa06e 100644 --- a/tests/unit/test_pricing_rate_validation.py +++ b/tests/unit/test_pricing_rate_validation.py @@ -315,9 +315,7 @@ async def test_boolean_exchange_quote_does_not_set_the_node_price( ``float(True)`` is ``1.0``, which is finite and positive, so a payload whose price field turned into a boolean passes every numeric guard — and then - *wins* the ``min()``, pricing the whole node at one dollar per bitcoin. The - node's other coercions all reject ``bool`` before the numeric check for this - reason; this one is the exception. + *wins* the ``min()``, pricing the whole node at one dollar per bitcoin. """ from routstr.payment.price import btc_usd_price @@ -328,6 +326,25 @@ async def test_boolean_exchange_quote_does_not_set_the_node_price( assert btc_usd_price() == pytest.approx(100000.0) +@pytest.mark.asyncio +async def test_an_underflowing_exchange_quote_does_not_set_the_node_price( + refresh_price_with: Any, +) -> None: + """A quote too small to survive the sats conversion is not a price. + + ``1e-320`` is positive, so it passes the guards and wins the ``min()``, but + the node prices in sats and ``1e-320 / 100_000_000`` underflows to ``0.0`` + — a zero sats price divides by zero on every model's rate. + """ + from routstr.payment.price import btc_usd_price + + await refresh_price_with( + {"kraken": "1e-320", "coinbase": "100000.0", "binance": "100000.0"} + ) + + assert btc_usd_price() == pytest.approx(100000.0) + + @pytest.mark.asyncio async def test_an_unreadable_exchange_response_drops_only_that_quote( refresh_price_with: Any, From 59bf8f4c9010ae2ed178b12b6cbbb08522f24f71 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Wed, 26 Aug 2026 15:54:49 +0200 Subject: [PATCH 17/18] fix(admin): a rate given as null is not an absent rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The write edge read each rate through `dict.get`, which returns `None` for an absent key and for an explicit `null` alike, and skipped both. `Pricing` declares `prompt` and `completion` without a default and every rate as a float, so neither is a row the read path can parse: the write was accepted, the row committed, and the response that reads it back raised — a 500 for a request the edge had already acted on, leaving a model that cannot be served. Ask whether the key is there, then whether its value is a rate. Which rates are required is derived from `Pricing` itself rather than restated, so the two cannot drift; an omitted auxiliary rate is still accepted, since those have defaults. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014X8RZzzbAuQCbavhFjTvJ4 --- routstr/core/admin.py | 21 ++++--- routstr/payment/models.py | 7 +++ .../test_admin_pricing_rate_validation.py | 61 +++++++++++++++++++ 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/routstr/core/admin.py b/routstr/core/admin.py index f02ec441..63fb44f6 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -12,6 +12,7 @@ from sqlmodel import select from sqlmodel.ext.asyncio.session import AsyncSession from ..payment.models import ( + REQUIRED_PRICING_FIELDS, _row_to_model, list_models, ) @@ -528,26 +529,32 @@ class ModelCreate(BaseModel): @field_validator("pricing") @classmethod def _validate_pricing(cls, value: dict[str, object]) -> dict[str, object]: - """Reject a malformed, non-finite or negative billable rate at the edge. + """Reject a rate that is malformed, non-finite, negative or not there. A present-but-invalid rate would otherwise slip through: a non-numeric string coerces to $0 on the read path (an unpriced-looking row), while a negative or ``NaN``/``inf`` value is truthy and reads back as a real price, so the model could be enabled and bill a nonsensical amount. Surfacing a 422 reports the client bug as a client bug instead of - persisting it. Absent rates and numeric strings (``"0.000005"``) stay - valid — the stored JSON accepts both. + persisting it. Numeric strings (``"0.000005"``) stay valid, and so does + an omitted auxiliary rate — the stored JSON accepts both. """ for field in BILLABLE_PRICING_FIELDS: - raw = value.get(field) - if raw is None: + if field not in value: + # ``dict.get`` cannot tell this from an explicit ``null``, so + # both were skipped and a row that ``Pricing`` cannot parse was + # written — and then raised out of the response that reads it + # back, after the row had been committed. + if field in REQUIRED_PRICING_FIELDS: + raise ValueError(f"{field} is required") continue # The shared coercion also absorbs the OverflowError an oversized # integer raises, which pydantic does not convert into a validation # error — unhandled it escaped as a 500 for a bad client value. - if coerce_rate(raw) is None: + if coerce_rate(value[field]) is None: raise ValueError( - f"{field} must be a finite, non-negative number, got {raw!r}" + f"{field} must be a finite, non-negative number, " + f"got {value[field]!r}" ) return value diff --git a/routstr/payment/models.py b/routstr/payment/models.py index 49ca89b1..e7b160fb 100644 --- a/routstr/payment/models.py +++ b/routstr/payment/models.py @@ -59,6 +59,13 @@ class Pricing(BaseModel): max_cost: float = 0.0 # in sats not msats +# The rates ``Pricing`` declares without a default, derived from the model so the +# two cannot drift. A payload that omits one writes a row that will not parse. +REQUIRED_PRICING_FIELDS = tuple( + name for name, field in Pricing.__fields__.items() if field.required +) + + def has_usable_pricing(pricing: Pricing) -> bool: """True if every billable rate is a number a request could be billed on. diff --git a/tests/integration/test_admin_pricing_rate_validation.py b/tests/integration/test_admin_pricing_rate_validation.py index 5a3d06c4..86754544 100644 --- a/tests/integration/test_admin_pricing_rate_validation.py +++ b/tests/integration/test_admin_pricing_rate_validation.py @@ -170,6 +170,67 @@ async def test_boolean_price_is_rejected( assert await integration_session.get(ModelRow, ("bool-price", provider_id)) is None +@pytest.mark.integration +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("model_id", "pricing"), + [ + ("null-prompt", _pricing(prompt=None)), + ("null-aux-rate", _pricing(image=None)), + ("no-prompt", {k: v for k, v in _pricing().items() if k != "prompt"}), + ], + ids=["null-required", "null-auxiliary", "absent-required"], +) +async def test_a_rate_that_is_not_there_is_rejected( + model_id: str, + pricing: dict[str, object], + integration_client: AsyncClient, + integration_session: AsyncSession, +) -> None: + """A rate given as ``null``, or a required rate left out, is not a price. + + The validator read both through ``dict.get``, which cannot tell an absent + key from an explicit ``null``, and skipped both. ``Pricing`` declares + ``prompt`` and ``completion`` without a default and every rate as a float, + so such a row is written with a 200 and then fails to parse on read — and + a row that will not parse is withheld from the catalog, leaving the operator + a model that was accepted and is nowhere to be seen. + """ + provider_id = await _make_provider(integration_session) + + resp = await integration_client.post( + f"/admin/api/upstream-providers/{provider_id}/models", + headers=_admin_headers(), + json=_payload(provider_id, model_id=model_id, pricing=pricing), + ) + + assert resp.status_code == 422 + assert await integration_session.get(ModelRow, (model_id, provider_id)) is None + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_an_absent_auxiliary_rate_is_still_accepted( + integration_client: AsyncClient, integration_session: AsyncSession +) -> None: + """Only ``prompt`` and ``completion`` are required; the rest carry defaults, + and a payload that omits them must still be accepted.""" + provider_id = await _make_provider(integration_session) + + resp = await integration_client.post( + f"/admin/api/upstream-providers/{provider_id}/models", + headers=_admin_headers(), + json=_payload( + provider_id, + model_id="lean-price", + pricing={"prompt": 1.4e-7, "completion": 2.8e-7}, + ), + ) + + assert resp.status_code == 200 + assert await integration_session.get(ModelRow, ("lean-price", provider_id)) + + @pytest.mark.integration @pytest.mark.asyncio async def test_numeric_string_price_is_still_accepted( From 76d7b8fceb37f7a642a26d0246c34b727ebe16bc Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Wed, 26 Aug 2026 15:56:10 +0200 Subject: [PATCH 18/18] test(pricing): say once what a rate-validation test pins The failure history these tests were written against now lives in one place, `payment/rates.py`. Cut the longest test docstrings down to what the test itself pins, rather than restating that history at every boundary. No test changes behaviour. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014X8RZzzbAuQCbavhFjTvJ4 --- .../test_admin_pricing_rate_validation.py | 18 ++++------ tests/unit/test_algorithm.py | 9 ++--- tests/unit/test_pricing_rate_validation.py | 34 ++++++------------- 3 files changed, 19 insertions(+), 42 deletions(-) diff --git a/tests/integration/test_admin_pricing_rate_validation.py b/tests/integration/test_admin_pricing_rate_validation.py index 86754544..2e9a45b9 100644 --- a/tests/integration/test_admin_pricing_rate_validation.py +++ b/tests/integration/test_admin_pricing_rate_validation.py @@ -189,12 +189,9 @@ async def test_a_rate_that_is_not_there_is_rejected( ) -> None: """A rate given as ``null``, or a required rate left out, is not a price. - The validator read both through ``dict.get``, which cannot tell an absent - key from an explicit ``null``, and skipped both. ``Pricing`` declares - ``prompt`` and ``completion`` without a default and every rate as a float, - so such a row is written with a 200 and then fails to parse on read — and - a row that will not parse is withheld from the catalog, leaving the operator - a model that was accepted and is nowhere to be seen. + ``dict.get`` cannot tell the two apart and skipped both, so a row + ``Pricing`` cannot parse was committed and the response that reads it back + raised. """ provider_id = await _make_provider(integration_session) @@ -351,12 +348,9 @@ async def test_admin_model_listing_shows_a_non_finite_stored_rate( ) -> None: """The operator must be able to see the rate that needs fixing. - The admin listing deliberately includes disabled models, so it is the one - view that still carries a row the served-catalog backstop holds back. - FastAPI's encoder rendered a stored ``Infinity`` rate as ``null``, which is - indistinguishable from a rate the row never carried — the operator could see - the row but not the reason it was withheld. Render the offending value as - text instead, as the 422 handler already does. + The admin listing is the one view that still carries a row the served + catalog holds back, and its encoder rendered a stored ``Infinity`` as + ``null`` — indistinguishable from a rate the row never carried. """ provider_id = await _make_provider(integration_session) integration_session.add( diff --git a/tests/unit/test_algorithm.py b/tests/unit/test_algorithm.py index cefc4f90..f5c875f9 100644 --- a/tests/unit/test_algorithm.py +++ b/tests/unit/test_algorithm.py @@ -1025,12 +1025,9 @@ def test_create_model_mappings_survives_an_unreadable_override_row( ) -> None: """One row that cannot be read must not empty the whole routing map. - Stored pricing is JSON from whatever wrote the row, so converting it can - raise. Converting an override while walking a provider's catalog let that - exception unwind the entire map build: at boot the node came up routing - nothing, and on a later refresh the map it already had went permanently - stale. The sibling loop over override-only rows already skips and logs such - a row. + Converting an override while walking a provider's catalog let the exception + unwind the entire map build: the node came up routing nothing. The sibling + loop over override-only rows already skips and logs such a row. """ broken = create_test_model("broken-model") healthy = create_test_model("healthy-model") diff --git a/tests/unit/test_pricing_rate_validation.py b/tests/unit/test_pricing_rate_validation.py index 895fa06e..b3e538c7 100644 --- a/tests/unit/test_pricing_rate_validation.py +++ b/tests/unit/test_pricing_rate_validation.py @@ -77,12 +77,9 @@ def _usage_response() -> dict[str, Any]: async def test_unusable_token_rate_falls_back_to_max_cost(bad_rate: float) -> None: """An unusable configured rate must not be billed on. - The "no token pricing configured" gate is a truthiness test, and ``NaN`` and - negative floats are both truthy, so an unusable rate passes the guard that - exists to catch it. It then reaches the integer conversion in the token math, - which raises ``ValueError`` for ``NaN`` and ``OverflowError`` for ``inf`` — - after the upstream response has already been served, where the streaming - handlers swallow it and the request goes unbilled. + It reached the token math, which raises after the response was already + served — where the streaming handlers swallow it and the request goes + unbilled. """ model = _model(Pricing(prompt=bad_rate, completion=1.0)) @@ -124,11 +121,8 @@ async def test_junk_cost_component_still_bills_the_reported_total(junk: Any) -> """A malformed component must not discard the upstream's real total cost. ``cost_details`` only splits the total across input and output; the total is - the authoritative billed amount. A non-finite component poisons the - proportional allocation (``inf / inf`` is ``NaN``), which raised out of the - USD path and was swallowed by the broad handler around it — so the request - silently fell through to token-estimated pricing and was billed at a small - fraction of what the upstream actually charged. + the authoritative billed amount. A non-finite component poisoned the split, + and the request fell through to token estimation for a fraction of it. """ model = _model(Pricing(prompt=1e-06, completion=2e-06)) response = { @@ -289,14 +283,9 @@ async def test_unusable_exchange_quote_does_not_set_the_node_price( ) -> None: """One exchange returning junk must not set the price the node bills at. - The feed takes the ``min()`` of the quotes it collects, so an unusable quote - does not merely join the sample — it *wins*, and poisons the rate every model - and every request is priced at until the next refresh. Zero then divides by - zero on the USD path, ``NaN`` raises out of the integer conversion, and a - negative rate produces a negative charge that settlement credits back to the - caller. - - The two healthy quotes must still price the node. + The feed takes the ``min()`` of what it collects, so an unusable quote does + not merely join the sample — it *wins*. The two healthy quotes must still + price the node. """ from routstr.payment.price import btc_usd_price @@ -351,11 +340,8 @@ async def test_an_unreadable_exchange_response_drops_only_that_quote( ) -> None: """An exchange whose response never yields a quote costs one quote. - The price is aggregated across three exchanges precisely so that one of them - having a bad day is survivable. A body that is not JSON, or whose shape moved - under the reader, raises before any number is seen; unhandled, it aborted the - whole aggregation and left the node on a stale rate even though two healthy - quotes were already in hand. + The price is aggregated across three exchanges so that one of them having a + bad day is survivable; unhandled, the raise aborted the whole aggregation. """ from routstr.payment.price import btc_usd_price