fix(models): let the titler actually see a provider's model catalog
The fast-model picker reads /v1/models to find the small model a provider currently serves, and it asked anonymously. Most of those endpoints need a key, so the fetch 401'd and the empty result read as "this provider has no small model" — the picker fell back to its curated list and never noticed. Worse, a failed fetch cached its empty result forever, so one bad moment during startup disabled live model discovery for the life of the process, and the processes that read this run for weeks. Give the failure an expiry and pass the provider's credentials. The bare family rungs (-mini, -flash, haiku) also picked whichever id sorted first, which is the oldest generation a provider still serves: gpt-3.5-mini over gpt-5.4-mini, claude-3-haiku over claude-haiku-4.5. Compare the digit runs as numbers so the rung meant to keep us current does.
This commit is contained in:
parent
34577fcb03
commit
071eab821b
|
|
@ -741,33 +741,76 @@ _FAST_MODEL_EXCLUDE: tuple = (
|
|||
)
|
||||
|
||||
|
||||
_VERSION_CHUNK_RE = re.compile(r"(\d+(?:\.\d+)?)")
|
||||
|
||||
|
||||
def _model_recency_key(model_id: str) -> tuple:
|
||||
"""Sort key that puts a family's newest release first (descending).
|
||||
|
||||
The rungs at the bottom of ``_FAST_MODEL_FAMILIES`` are bare family names —
|
||||
``-mini``, ``-flash``, ``haiku`` — and a provider serves every generation of
|
||||
those it hasn't retired. Compared as plain strings, the oldest wins:
|
||||
``gpt-3.5-mini`` sorts before ``gpt-5.4-mini``, and ``claude-3-haiku`` before
|
||||
``claude-haiku-4.5``. So the rung meant to keep us current on a provider's
|
||||
small tier was pinning us to its most obsolete member.
|
||||
|
||||
Splitting digit runs out and comparing them as numbers fixes both the
|
||||
generation order and the 9-vs-10 cliff a string sort walks off.
|
||||
"""
|
||||
chunks = []
|
||||
for index, part in enumerate(_VERSION_CHUNK_RE.split(model_id.lower())):
|
||||
if not part:
|
||||
continue
|
||||
# re.split with one capturing group alternates text, number, text, …
|
||||
chunks.append((1, float(part), "") if index % 2 else (0, 0.0, part))
|
||||
return tuple(chunks)
|
||||
|
||||
|
||||
def _fast_model_from_catalog(provider_id: str) -> str:
|
||||
"""Pick the fastest small model the provider ACTUALLY serves right now.
|
||||
|
||||
Reads the provider's live (cached) ``/v1/models`` catalog and returns the
|
||||
first ``_FAST_MODEL_FAMILIES`` match. Returns "" when the catalog is
|
||||
newest ``_FAST_MODEL_FAMILIES`` match. Returns "" when the catalog is
|
||||
unavailable or holds no small model, so the caller falls through to the
|
||||
provider's curated default. Never raises and never blocks on a cold
|
||||
network path — the underlying fetch is memory+disk cached with a
|
||||
last-known-good fallback.
|
||||
"""
|
||||
try:
|
||||
from hermes_cli.auth import resolve_api_key_provider_credentials
|
||||
from hermes_cli.models import fetch_models_with_pricing
|
||||
from providers import get_provider_profile
|
||||
|
||||
profile = get_provider_profile(provider_id)
|
||||
base_url = str(getattr(profile, "base_url", "") or "").rstrip("/")
|
||||
# The provider's own credentials, because most ``/v1/models`` endpoints
|
||||
# are authenticated: fetched anonymously they 401, and the caller reads
|
||||
# that as "this provider serves no small model" and quietly falls back
|
||||
# to the curated default forever.
|
||||
api_key, base_url = "", ""
|
||||
try:
|
||||
creds = resolve_api_key_provider_credentials(provider_id) or {}
|
||||
api_key = str(creds.get("api_key", "")).strip()
|
||||
base_url = str(creds.get("base_url", "")).strip()
|
||||
except Exception:
|
||||
# Not an API-key provider, or nothing configured yet. The anonymous
|
||||
# fetch below still works for the catalogs that allow it.
|
||||
logger.debug("No credentials for %s catalog", provider_id, exc_info=True)
|
||||
|
||||
if not base_url:
|
||||
base_url = str(getattr(get_provider_profile(provider_id), "base_url", "") or "")
|
||||
base_url = base_url.rstrip("/")
|
||||
if not base_url:
|
||||
return ""
|
||||
# fetch_models_with_pricing appends its own /v1/models.
|
||||
if base_url.endswith("/v1"):
|
||||
base_url = base_url[:-3]
|
||||
catalog = fetch_models_with_pricing(base_url=base_url, timeout=3.0) or {}
|
||||
catalog = fetch_models_with_pricing(
|
||||
api_key=api_key or None, base_url=base_url, timeout=3.0
|
||||
) or {}
|
||||
except Exception:
|
||||
logger.debug("Fast-model catalog lookup failed for %s", provider_id, exc_info=True)
|
||||
return ""
|
||||
|
||||
ids = sorted(str(m) for m in catalog)
|
||||
ids = sorted((str(m) for m in catalog), key=_model_recency_key, reverse=True)
|
||||
for family in _FAST_MODEL_FAMILIES:
|
||||
for model_id in ids:
|
||||
lowered = model_id.lower()
|
||||
|
|
|
|||
|
|
@ -1695,6 +1695,43 @@ def ai_gateway_model_ids(*, force_refresh: bool = False) -> list[str]:
|
|||
# Cache: maps model_id → {"prompt": str, "completion": str} per endpoint
|
||||
_pricing_cache: dict[str, dict[str, dict[str, str]]] = {}
|
||||
|
||||
# A failed fetch caches its empty result too, so an unreachable endpoint isn't
|
||||
# re-dialed on every call — but only until this deadline. Cached forever, one
|
||||
# bad moment (a blip during startup, a key that hadn't been written yet) turns
|
||||
# into no live model discovery for the life of the process, and the processes
|
||||
# that read this most are the ones that run for weeks: the gateway, the desktop
|
||||
# backend. Every caller falls back to a curated list meanwhile, so the cost of
|
||||
# the stale entry is silent and invisible.
|
||||
_FAILED_CATALOG_TTL_SECONDS = 120.0
|
||||
_pricing_cache_retry_after: dict[str, float] = {}
|
||||
|
||||
|
||||
def _cached_catalog(cache_key: str) -> Optional[dict[str, dict[str, Any]]]:
|
||||
"""The cached catalog for *cache_key*, or None to go fetch it."""
|
||||
cached = _pricing_cache.get(cache_key)
|
||||
if cached is None:
|
||||
return None
|
||||
retry_after = _pricing_cache_retry_after.get(cache_key)
|
||||
if retry_after is not None and time.monotonic() >= retry_after:
|
||||
_pricing_cache.pop(cache_key, None)
|
||||
_pricing_cache_retry_after.pop(cache_key, None)
|
||||
return None
|
||||
return cached
|
||||
|
||||
|
||||
def _cache_catalog(
|
||||
cache_key: str, result: dict[str, dict[str, Any]]
|
||||
) -> dict[str, dict[str, Any]]:
|
||||
"""Cache a catalog result, giving an empty one an expiry."""
|
||||
_pricing_cache[cache_key] = result
|
||||
if result:
|
||||
_pricing_cache_retry_after.pop(cache_key, None)
|
||||
else:
|
||||
_pricing_cache_retry_after[cache_key] = (
|
||||
time.monotonic() + _FAILED_CATALOG_TTL_SECONDS
|
||||
)
|
||||
return result
|
||||
|
||||
|
||||
def _format_price_per_mtok(per_token_str: str) -> str:
|
||||
"""Convert a per-token price string to a human-friendly $/Mtok string.
|
||||
|
|
@ -1831,8 +1868,10 @@ def fetch_models_with_pricing(
|
|||
``original``.
|
||||
"""
|
||||
cache_key = (base_url or "").rstrip("/")
|
||||
if not force_refresh and cache_key in _pricing_cache:
|
||||
return _pricing_cache[cache_key]
|
||||
if not force_refresh:
|
||||
cached = _cached_catalog(cache_key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
|
||||
url = cache_key + "/v1/models"
|
||||
headers: dict[str, str] = {
|
||||
|
|
@ -1847,8 +1886,7 @@ def fetch_models_with_pricing(
|
|||
with _urlopen_model_catalog_request(req, timeout=timeout) as resp:
|
||||
payload = json.loads(resp.read().decode())
|
||||
except Exception:
|
||||
_pricing_cache[cache_key] = {}
|
||||
return {}
|
||||
return _cache_catalog(cache_key, {})
|
||||
|
||||
result: dict[str, dict[str, Any]] = {}
|
||||
for item in payload.get("data", []):
|
||||
|
|
@ -1881,8 +1919,7 @@ def fetch_models_with_pricing(
|
|||
entry["original"] = orig_entry
|
||||
result[mid] = entry
|
||||
|
||||
_pricing_cache[cache_key] = result
|
||||
return result
|
||||
return _cache_catalog(cache_key, result)
|
||||
|
||||
|
||||
def fetch_ai_gateway_pricing(
|
||||
|
|
@ -1899,8 +1936,10 @@ def fetch_ai_gateway_pricing(
|
|||
from hermes_constants import AI_GATEWAY_BASE_URL
|
||||
|
||||
cache_key = AI_GATEWAY_BASE_URL.rstrip("/")
|
||||
if not force_refresh and cache_key in _pricing_cache:
|
||||
return _pricing_cache[cache_key]
|
||||
if not force_refresh:
|
||||
cached = _cached_catalog(cache_key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
|
||||
try:
|
||||
req = urllib.request.Request(
|
||||
|
|
@ -1910,8 +1949,7 @@ def fetch_ai_gateway_pricing(
|
|||
with urllib.request.urlopen(req, timeout=timeout) as resp:
|
||||
payload = json.loads(resp.read().decode())
|
||||
except Exception:
|
||||
_pricing_cache[cache_key] = {}
|
||||
return {}
|
||||
return _cache_catalog(cache_key, {})
|
||||
|
||||
result: dict[str, dict[str, str]] = {}
|
||||
for item in payload.get("data", []):
|
||||
|
|
@ -1931,8 +1969,7 @@ def fetch_ai_gateway_pricing(
|
|||
entry["input_cache_write"] = str(pricing["input_cache_write"])
|
||||
result[mid] = entry
|
||||
|
||||
_pricing_cache[cache_key] = result
|
||||
return result
|
||||
return _cache_catalog(cache_key, result)
|
||||
|
||||
|
||||
def _resolve_openrouter_api_key() -> str:
|
||||
|
|
@ -2039,8 +2076,10 @@ def _fireworks_pricing_from_models_dev(
|
|||
pricing formatter expects per-token strings, so divide by 1M.
|
||||
"""
|
||||
cache_key = "models.dev/fireworks"
|
||||
if not force_refresh and cache_key in _pricing_cache:
|
||||
return _pricing_cache[cache_key]
|
||||
if not force_refresh:
|
||||
cached = _cached_catalog(cache_key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
|
||||
result: dict[str, dict[str, str]] = {}
|
||||
try:
|
||||
|
|
@ -2068,8 +2107,7 @@ def _fireworks_pricing_from_models_dev(
|
|||
except Exception:
|
||||
result = {}
|
||||
|
||||
_pricing_cache[cache_key] = result
|
||||
return result
|
||||
return _cache_catalog(cache_key, result)
|
||||
|
||||
|
||||
def _fetch_novita_pricing(
|
||||
|
|
@ -2093,8 +2131,10 @@ def _fetch_novita_pricing(
|
|||
|
||||
base_url = os.getenv("NOVITA_BASE_URL", "").strip() or "https://api.novita.ai/openai/v1"
|
||||
cache_key = base_url.rstrip("/")
|
||||
if not force_refresh and cache_key in _pricing_cache:
|
||||
return _pricing_cache[cache_key]
|
||||
if not force_refresh:
|
||||
cached = _cached_catalog(cache_key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
|
||||
url = cache_key + "/models"
|
||||
headers = {
|
||||
|
|
@ -2108,8 +2148,7 @@ def _fetch_novita_pricing(
|
|||
with _urlopen_model_catalog_request(req, timeout=timeout) as resp:
|
||||
payload = json.loads(resp.read().decode())
|
||||
except Exception:
|
||||
_pricing_cache[cache_key] = {}
|
||||
return {}
|
||||
return _cache_catalog(cache_key, {})
|
||||
|
||||
result: dict[str, dict[str, str]] = {}
|
||||
for item in payload.get("data", []):
|
||||
|
|
@ -2127,8 +2166,7 @@ def _fetch_novita_pricing(
|
|||
"completion": str(float(out or 0) / 10_000 / 1_000_000),
|
||||
}
|
||||
|
||||
_pricing_cache[cache_key] = result
|
||||
return result
|
||||
return _cache_catalog(cache_key, result)
|
||||
|
||||
|
||||
# All provider IDs and aliases that are valid for the provider:model syntax.
|
||||
|
|
|
|||
|
|
@ -4519,6 +4519,42 @@ class TestFastModelTier:
|
|||
with patch("hermes_cli.models.fetch_models_with_pricing", return_value=catalog):
|
||||
assert ac._fast_model_from_catalog("nous") == "google/gemini-3.6-flash"
|
||||
|
||||
def test_catalog_match_takes_the_newest_of_a_family(self):
|
||||
"""The bare family rungs must land on the current generation.
|
||||
|
||||
A provider serves every generation of its small tier it hasn't retired,
|
||||
and compared as strings the oldest sorts first — so the rung meant to
|
||||
keep the titler current was pinning it to the most obsolete member.
|
||||
"""
|
||||
from agent import auxiliary_client as ac
|
||||
|
||||
catalog = {
|
||||
"openai/gpt-3.5-mini": {},
|
||||
"openai/gpt-9-mini": {},
|
||||
"openai/gpt-10-mini": {},
|
||||
}
|
||||
with patch("hermes_cli.models.fetch_models_with_pricing", return_value=catalog):
|
||||
assert ac._fast_model_from_catalog("nous") == "openai/gpt-10-mini"
|
||||
|
||||
def test_catalog_fetch_is_authenticated(self):
|
||||
"""Most /v1/models endpoints need a key; anonymously they 401.
|
||||
|
||||
A 401 reads as "this provider serves no small model", so the titler
|
||||
would fall back to the curated default and never notice.
|
||||
"""
|
||||
from agent import auxiliary_client as ac
|
||||
|
||||
with patch(
|
||||
"hermes_cli.auth.resolve_api_key_provider_credentials",
|
||||
return_value={"api_key": "sk-test", "base_url": "https://api.example.com/v1"},
|
||||
), patch(
|
||||
"hermes_cli.models.fetch_models_with_pricing", return_value={}
|
||||
) as fetch:
|
||||
ac._fast_model_from_catalog("openai")
|
||||
|
||||
assert fetch.call_args.kwargs["api_key"] == "sk-test"
|
||||
assert fetch.call_args.kwargs["base_url"] == "https://api.example.com"
|
||||
|
||||
def test_falls_back_to_curated_default_when_catalog_unavailable(self):
|
||||
"""An offline catalog degrades to the provider's pinned default."""
|
||||
from agent import auxiliary_client as ac
|
||||
|
|
|
|||
|
|
@ -91,3 +91,66 @@ def test_resolve_nous_pricing_credentials_honors_inference_env_override(monkeypa
|
|||
assert base_url == "https://stg-inference-api.nousresearch.com/v1"
|
||||
|
||||
|
||||
def test_a_failed_catalog_fetch_is_not_cached_forever(monkeypatch):
|
||||
"""A blip must not disable live model discovery for the whole process.
|
||||
|
||||
The empty result is cached so a dead endpoint isn't re-dialed on every
|
||||
call, but it expires — the processes that read this run for weeks, and
|
||||
every caller silently falls back to a curated list meanwhile.
|
||||
"""
|
||||
models_mod._pricing_cache.clear()
|
||||
models_mod._pricing_cache_retry_after.clear()
|
||||
|
||||
calls = []
|
||||
|
||||
def _fail(req, timeout=8.0):
|
||||
calls.append(req)
|
||||
raise OSError("connection refused")
|
||||
|
||||
monkeypatch.setattr(models_mod, "_urlopen_model_catalog_request", _fail)
|
||||
|
||||
assert fetch_models_with_pricing(base_url="https://example.test") == {}
|
||||
# Inside the window the failure is cached: no second dial.
|
||||
assert fetch_models_with_pricing(base_url="https://example.test") == {}
|
||||
assert len(calls) == 1
|
||||
|
||||
now = models_mod.time.monotonic()
|
||||
monkeypatch.setattr(
|
||||
models_mod.time,
|
||||
"monotonic",
|
||||
lambda: now + models_mod._FAILED_CATALOG_TTL_SECONDS + 1,
|
||||
)
|
||||
assert fetch_models_with_pricing(base_url="https://example.test") == {}
|
||||
assert len(calls) == 2
|
||||
|
||||
|
||||
def test_a_successful_catalog_fetch_stays_cached(monkeypatch):
|
||||
"""Only the failures expire; a real catalog is still fetched once."""
|
||||
models_mod._pricing_cache.clear()
|
||||
models_mod._pricing_cache_retry_after.clear()
|
||||
|
||||
calls = []
|
||||
body = json.dumps(
|
||||
{"data": [{"id": "a/b", "pricing": {"prompt": "1", "completion": "2"}}]}
|
||||
).encode()
|
||||
resp = MagicMock()
|
||||
resp.read.return_value = body
|
||||
resp.__enter__ = lambda self: self
|
||||
resp.__exit__ = lambda *a: False
|
||||
|
||||
def _ok(req, timeout=8.0):
|
||||
calls.append(req)
|
||||
return resp
|
||||
|
||||
monkeypatch.setattr(models_mod, "_urlopen_model_catalog_request", _ok)
|
||||
|
||||
assert "a/b" in fetch_models_with_pricing(base_url="https://example.test")
|
||||
now = models_mod.time.monotonic()
|
||||
monkeypatch.setattr(
|
||||
models_mod.time,
|
||||
"monotonic",
|
||||
lambda: now + models_mod._FAILED_CATALOG_TTL_SECONDS + 1,
|
||||
)
|
||||
assert "a/b" in fetch_models_with_pricing(base_url="https://example.test")
|
||||
assert len(calls) == 1
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue