mirror of
https://github.com/Routstr/routstr-core.git
synced 2026-10-05 12:28:22 +00:00
fix(pricing): keep one unreadable model row from taking the node with it
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cKHVF5LA7TR5QuYi6ErLM
This commit is contained in:
co-authored by
Claude Opus 5
parent
e202435ca0
commit
41fed7413a
@@ -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]
|
||||
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
|
||||
|
||||
|
||||
@@ -375,6 +375,7 @@ async def list_models(
|
||||
and providers_by_id[r.upstream_provider_id].enabled
|
||||
):
|
||||
continue
|
||||
try:
|
||||
model = _row_to_model(
|
||||
r,
|
||||
apply_provider_fee=apply_fees,
|
||||
@@ -382,6 +383,23 @@ async def list_models(
|
||||
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
|
||||
|
||||
@@ -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"}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user