fix(models): corrupt-at cache rows degrade to live fetch in cached_provider_model_ids

Surfaced during the post-merge review pass on our own #81113 follow-up:
cached_fetch_api_models gained _cache_entry_valid (numeric-'at'
validation) but its sibling cached_provider_model_ids still did
float(entry.get('at', 0)), which raises ValueError/TypeError on a
hand-edited or corrupted provider_models_cache.json row and propagates
uncaught into the /model picker call sites. Same fix, same helper:
corrupt rows are now a cache miss (live fetch), never an exception.
Both wrappers now share the identical validity predicate, closing the
divergence the 'mirrors' docstring promised away.

Also two test nits from the same review: unused OrderedDict import
dropped and the drain-order assertion strengthened to pin LRU-first
FIFO order in tests/gateway/test_agent_cache_pressure.py.

Mutation-checked: restoring the raising float() form makes the new
corrupt-at tests fail.
This commit is contained in:
kshitij 2026-08-07 21:34:04 +05:30
parent fa1a5c0485
commit c015663b21
3 changed files with 23 additions and 17 deletions

View File

@ -3287,14 +3287,8 @@ def cached_provider_model_ids(
entry = cache.get(normalized)
now = time.time()
if (
not force_refresh
and isinstance(entry, dict)
and entry.get("fp") == fp
and isinstance(entry.get("models"), list)
and entry["models"]
):
age = now - float(entry.get("at", 0))
if not force_refresh and _cache_entry_valid(entry, fp):
age = now - entry["at"]
if age < ttl_seconds:
return list(entry["models"])
if age < _PROVIDER_MODELS_STALE_SERVE_MAX:
@ -3318,12 +3312,7 @@ def cached_provider_model_ids(
# Live fetch returned nothing. If we have a stale entry with the
# SAME fingerprint, prefer it over an empty result — stale data
# beats no data when the network is flaky.
if (
isinstance(entry, dict)
and entry.get("fp") == fp
and isinstance(entry.get("models"), list)
and entry["models"]
):
if _cache_entry_valid(entry, fp):
return list(entry["models"])
return list(live or [])

View File

@ -433,8 +433,6 @@ class TestSalvageFollowups:
reference pins the evicted agents during gc.collect + malloc_trim
(otherwise the in-pass trim frees nothing and the next tick
over-evicts another batch)."""
from collections import OrderedDict as _OD
from gateway.run import GatewayRunner
runner = GatewayRunner.__new__(GatewayRunner)
@ -455,7 +453,7 @@ class TestSalvageFollowups:
runner._release_pressure_batch(plan)
assert sorted(released) == ["s0", "s1", "s2"]
assert released == ["s0", "s1", "s2"], "LRU-first (FIFO) release order"
assert plan_len_at_trim["len"] == 0, (
"plan still held agent references when trim_memory ran"
)

View File

@ -183,3 +183,22 @@ class TestCatalogSWR:
assert out == manifest
fetch.assert_called_once()
spawn.assert_not_called()
class TestCorruptCacheRowDegradation:
"""A corrupted 'at' in the user-editable provider_models_cache.json must
degrade cached_provider_model_ids to a cache miss (live fetch), never
raise through the picker (which has no try/except at its call sites)."""
@pytest.mark.parametrize("bad_at", ["yesterday", None, True])
def test_corrupt_at_falls_back_to_live_fetch(self, bad_at):
import hermes_cli.models as mod
cache = {"openrouter": {"fp": "fp", "at": bad_at, "models": ["corrupt-row"]}}
with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
patch.object(mod, "_credential_fingerprint", return_value="fp"), \
patch.object(mod, "_save_provider_models_cache"), \
patch.object(mod, "provider_model_ids", return_value=["live-model"]) as live:
out = mod.cached_provider_model_ids("openrouter")
assert out == ["live-model"]
live.assert_called_once()