mirror of
https://github.com/Routstr/routstr-core.git
synced 2026-10-05 12:28:22 +00:00
fix(pricing): a rate spelled as a boolean is not a rate
`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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014X8RZzzbAuQCbavhFjTvJ4
This commit is contained in:
co-authored by
Claude Opus 5
parent
6318a3bdbb
commit
dff164b980
+8
-12
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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"]
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user