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