From 59bf8f4c9010ae2ed178b12b6cbbb08522f24f71 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Wed, 26 Aug 2026 15:54:49 +0200 Subject: [PATCH] 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(