From 658ac1d866569fbf7b35cbf57a1352ef21f0f373 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Tue, 16 Jun 2026 23:13:33 +0530 Subject: [PATCH] fix(models): keep curated-first ordering in live+curated merge; use pure-catalog helper in validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- hermes_cli/models.py | 46 +++++++------- .../test_models_dev_preferred_merge.py | 4 +- .../test_provider_live_curated_merge.py | 60 +++++++++++++++---- 3 files changed, 71 insertions(+), 39 deletions(-) diff --git a/hermes_cli/models.py b/hermes_cli/models.py index 3432f1ae4a1..7be0547799a 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -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, diff --git a/tests/hermes_cli/test_models_dev_preferred_merge.py b/tests/hermes_cli/test_models_dev_preferred_merge.py index 0eadbbb17d9..dfa25d1bb2e 100644 --- a/tests/hermes_cli/test_models_dev_preferred_merge.py +++ b/tests/hermes_cli/test_models_dev_preferred_merge.py @@ -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.""" diff --git a/tests/hermes_cli/test_provider_live_curated_merge.py b/tests/hermes_cli/test_provider_live_curated_merge.py index 28f35439af7..184d410542e 100644 --- a/tests/hermes_cli/test_provider_live_curated_merge.py +++ b/tests/hermes_cli/test_provider_live_curated_merge.py @@ -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." + ) +