fix(models): keep curated-first ordering in live+curated merge; use pure-catalog helper in validation

The generic live+curated merge (commit 630b438) seeded the merged list
from live results, demoting curated-only models below live ones. That
regressed #46309, which deliberately surfaces the newest curated model
(kimi-k2.7-code) FIRST in the native picker even when the live /models
listing lags. Restore curated-first ordering: curated entries lead (in
catalog order), live-only entries are appended for discovery. This keeps
the #46850 fix (zai glm-5.2 now appears) without the kimi regression.

Also switch the validate_requested_model curated fallback (commit
ee7b8a4) from provider_model_ids() — which triggers a second, uncached
live /models fetch with its own 8s timeout and may resolve different
credentials than the api_key/base_url just probed — to the pure-catalog
helper _model_in_provider_catalog(). Membership is checked against the
shipped catalog only, with no extra network call.

Tests: restore the curated-first assertion in
test_kimi_coding_live_catalog_does_not_hide_curated_k2_7_code; update
the new merge tests to curated-first semantics; de-circularize the
validation fallback tests to patch _PROVIDER_MODELS (the real source)
instead of mocking the function under test.
This commit is contained in:
kshitijk4poor 2026-06-16 23:13:33 +05:30
parent ee7b8a4672
commit 658ac1d866
3 changed files with 71 additions and 39 deletions

View file

@ -2370,16 +2370,18 @@ def provider_model_ids(provider: Optional[str], *, force_refresh: bool = False)
if api_key:
live = _p.fetch_models(api_key=api_key)
if live:
# Merge live API results with static curated list so
# Merge static curated list with live API results so
# models that the live endpoint omits (stale cache,
# partial rollout) still appear in the picker.
# Live entries come first (provider's preferred order),
# then curated-only entries are appended. (#46850)
# Curated entries come first so deliberately-surfaced
# newest models (e.g. kimi-k2.7-code, #46309) stay at
# the top of the picker; live-only entries are appended
# afterwards for discovery. (#46850)
curated = list(_PROVIDER_MODELS.get(normalized, []))
if curated:
merged = list(live)
merged_lower = {m.lower() for m in live}
for m in curated:
merged = list(curated)
merged_lower = {m.lower() for m in curated}
for m in live:
if m.lower() not in merged_lower:
merged.append(m)
merged_lower.add(m.lower())
@ -3942,24 +3944,20 @@ def validate_requested_model(
# Model not in live /v1/models — check the curated catalog
# before rejecting. Providers may omit models from their live
# listing that are still valid (stale cache, partial rollout,
# gated previews). If the curated list has it, accept with a
# note. (#46850)
try:
curated = provider_model_ids(normalized)
except Exception:
curated = []
if curated:
curated_lower = {m.lower(): m for m in curated}
if requested_for_lookup.lower() in curated_lower:
return {
"accepted": True,
"persist": True,
"recognized": True,
"message": (
f"Note: `{requested}` was not found in the live /v1/models listing "
f"but exists in the curated catalog — accepted."
),
}
# gated previews). Use the pure-catalog helper (no extra live
# fetch) so we only accept models Hermes actually ships. (#46850)
if _model_in_provider_catalog(
requested_for_lookup.lower(), _provider_keys(normalized)
):
return {
"accepted": True,
"persist": True,
"recognized": True,
"message": (
f"Note: `{requested}` was not found in the live /v1/models listing "
f"but exists in the curated catalog — accepted."
),
}
return {
"accepted": False,

View file

@ -114,8 +114,8 @@ class TestProviderModelIdsPreferred:
patch("providers.base.ProviderProfile.fetch_models", return_value=["kimi-k2.6"]),
):
out = provider_model_ids("kimi-coding")
# Live-first order; curated-only (k2.7-code) appended after live
assert out[:2] == ["kimi-k2.6", "kimi-k2.7-code"]
# Curated-first order; curated newest (k2.7-code) stays ahead of live.
assert out[:2] == ["kimi-k2.7-code", "kimi-k2.6"]
def test_kimi_setup_flow_uses_same_coding_plan_catalog(self):
"""The setup wizard must not carry a stale duplicate Kimi model list."""

View file

@ -23,7 +23,7 @@ class TestGenericProviderLiveCuratedMerge:
return p
def test_live_models_merged_with_curated(self):
"""Live models come first; curated-only models are appended."""
"""Curated models come first; live-only models are appended."""
live = ["glm-5.2", "glm-5.1", "glm-5"]
curated = _PROVIDER_MODELS["zai"] # includes glm-5.1, glm-5, glm-4.5, etc.
profile = self._make_profile(live)
@ -34,11 +34,14 @@ class TestGenericProviderLiveCuratedMerge:
):
result = provider_model_ids("zai")
# Live entries first (in live order)
# Curated entries first, in catalog order (keeps newest curated models
# like glm-5.2 at the top of the picker — see #46309).
assert result[: len(curated)] == list(curated)
assert result[0] == "glm-5.2"
assert result[1] == "glm-5.1"
assert result[2] == "glm-5"
# Curated-only entries appended (e.g. glm-4.5)
# Models present in both live and curated are not duplicated.
assert result.count("glm-5.2") == 1
assert result.count("glm-5.1") == 1
# Curated-only entries are part of the result (e.g. glm-4.5).
result_lower = [m.lower() for m in result]
assert "glm-4.5" in result_lower
assert "glm-4.5-flash" in result_lower
@ -73,11 +76,8 @@ class TestGenericProviderLiveCuratedMerge:
):
result = provider_model_ids("zai")
# Live casing preserved for duplicates
assert result[0] == "GLM-5.1"
assert result[1] == "glm-5"
# Curated-only appended
assert "glm-4.5" in result
# Curated-first: curated casing wins for models present in both.
assert result == ["glm-5.1", "GLM-5", "glm-4.5"]
def test_empty_curated_returns_live_only(self):
"""When no curated list exists, live is returned as-is."""
@ -118,7 +118,11 @@ class TestValidateRequestedModelCuratedFallback:
def test_model_in_curated_but_not_live_is_accepted(self):
"""When live /v1/models omits a model that exists in the curated
catalog, validate_requested_model should accept it with a note."""
catalog, validate_requested_model should accept it with a note.
Patches the real ``_PROVIDER_MODELS`` source (not the function under
test) so the curated-catalog fallback is genuinely exercised.
"""
from hermes_cli.models import validate_requested_model
# Live API returns only glm-5.1, but curated has glm-5.2
@ -127,7 +131,7 @@ class TestValidateRequestedModelCuratedFallback:
with (
patch("hermes_cli.models.fetch_api_models", return_value=live_models),
patch("hermes_cli.models.provider_model_ids", return_value=curated),
patch.dict("hermes_cli.models._PROVIDER_MODELS", {"zai": curated}),
):
result = validate_requested_model("glm-5.2", "zai", api_key="dummy")
@ -145,7 +149,7 @@ class TestValidateRequestedModelCuratedFallback:
with (
patch("hermes_cli.models.fetch_api_models", return_value=live_models),
patch("hermes_cli.models.provider_model_ids", return_value=curated),
patch.dict("hermes_cli.models._PROVIDER_MODELS", {"zai": curated}),
):
result = validate_requested_model("nonexistent-model", "zai", api_key="dummy")
@ -163,3 +167,33 @@ class TestValidateRequestedModelCuratedFallback:
assert result["accepted"] is True
assert result["recognized"] is True
assert result["message"] is None
def test_curated_fallback_is_scoped_to_the_current_provider(self):
"""The curated fallback must not leak models across providers.
A model that lives in some OTHER provider's catalog (or only on an
aggregator like OpenRouter) must still be rejected when the current
provider neither lists it live nor ships it in its OWN curated
catalog. The fallback keys on ``_provider_keys(normalized)``, so
catalog membership is checked per-provider, never globally.
"""
from hermes_cli.models import validate_requested_model
# `some-other-model` is known to a DIFFERENT provider, not to zai.
# zai's live listing also omits it. It must be rejected.
live_models = ["glm-5.1"]
with (
patch("hermes_cli.models.fetch_api_models", return_value=live_models),
patch.dict(
"hermes_cli.models._PROVIDER_MODELS",
{"zai": ["glm-5.2", "glm-5.1"], "openrouter": ["some-other-model"]},
),
):
result = validate_requested_model("some-other-model", "zai", api_key="dummy")
assert result["accepted"] is False, (
"A model only present in another provider's catalog must not be "
"accepted on this provider via the curated fallback."
)