From fc18d15f404e0d46b49ce12b38871e51e6483f06 Mon Sep 17 00:00:00 2001 From: Lord_dubious Date: Sat, 4 Jul 2026 18:54:10 +0000 Subject: [PATCH] fix: preserve static custom provider models --- hermes_cli/config.py | 30 +++-- hermes_cli/model_switch.py | 107 ++++++++++-------- tests/hermes_cli/test_inventory.py | 73 +++++++++++- .../test_model_switch_custom_providers.py | 73 ++++++++++++ 4 files changed, 229 insertions(+), 54 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 5c27b7bc5d2..f36b788bbf5 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -4611,13 +4611,29 @@ def _normalize_custom_provider_entry( if isinstance(models, dict) and models: normalized["models"] = models elif isinstance(models, list) and models: - # Hand-edited configs (and older Hermes versions) write ``models`` as - # a plain list of model ids. Preserve them by converting to the dict - # shape downstream code expects; otherwise normalize silently drops - # the list and /model shows the provider with (0) models. - normalized["models"] = { - str(m): {} for m in models if isinstance(m, str) and m.strip() - } + # Hand-edited configs (and older Hermes versions) may write + # ``models`` as a plain list of ids or as ``[{id: ...}]`` rows. + # Preserve both by converting to the dict shape downstream code + # expects; otherwise normalize silently drops the list and /model + # shows the provider with (0) models. + normalized_models: Dict[str, Any] = {} + for item in models: + if isinstance(item, str) and item.strip(): + normalized_models[item.strip()] = {} + continue + if not isinstance(item, dict): + continue + model_id = item.get("id") + if not isinstance(model_id, str) or not model_id.strip(): + model_id = item.get("name") + if not isinstance(model_id, str) or not model_id.strip(): + continue + model_meta = { + k: v for k, v in item.items() if k not in {"id", "name"} + } + normalized_models[model_id.strip()] = model_meta + if normalized_models: + normalized["models"] = normalized_models context_length = entry.get("context_length") if isinstance(context_length, int) and context_length > 0: diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index c92d50a7319..638ecf9d50f 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -52,6 +52,54 @@ _UNCAPPED_PICKER_PROVIDERS: frozenset[str] = frozenset({"opencode-zen", "opencod logger = logging.getLogger(__name__) +def _declared_model_ids(value: Any) -> list[str]: + """Return configured model IDs from supported config shapes. + + Accepts: + - ``{"model-id": {...}}`` + - ``["model-a", "model-b"]`` + - ``[{"id": "model-a"}, {"name": "model-b"}]`` + - ``"model-a"`` + """ + ids: list[str] = [] + seen: set[str] = set() + + def _add(candidate: Any) -> None: + if not isinstance(candidate, str): + return + model_id = candidate.strip() + if not model_id: + return + lowered = model_id.lower() + if lowered in seen: + return + seen.add(lowered) + ids.append(model_id) + + if isinstance(value, str): + _add(value) + return ids + + if isinstance(value, dict): + for model_id in value: + _add(model_id) + return ids + + if isinstance(value, (list, tuple)): + for item in value: + if isinstance(item, str): + _add(item) + continue + if isinstance(item, dict): + model_id = item.get("id") + if not isinstance(model_id, str) or not model_id.strip(): + model_id = item.get("name") + _add(model_id) + return ids + + return ids + + def _bare_custom_provider_def(current_base_url: str) -> Optional[ProviderDef]: """ProviderDef for a direct ``model.provider: custom`` endpoint.""" base_url = str(current_base_url or "").strip() @@ -700,22 +748,9 @@ def _configured_provider_matches( def _match(value) -> Optional[str]: """Canonical id if ``value`` (a model collection or scalar) declares ``target``, else None.""" - if isinstance(value, str): - return value if value.strip().lower() == target else None - if isinstance(value, dict): - for mid in value: - if isinstance(mid, str) and mid.strip().lower() == target: - return mid - return None - if isinstance(value, (list, tuple)): - for item in value: - if isinstance(item, str) and item.strip().lower() == target: - return item - if isinstance(item, dict): - name = item.get("name") - if isinstance(name, str) and name.strip().lower() == target: - return name - return None + for model_id in _declared_model_ids(value): + if model_id.lower() == target: + return model_id return None matches: dict[str, str] = {} @@ -1243,16 +1278,9 @@ def switch_model( # user_providers is a dict: {provider_slug: config_dict} for slug, cfg in user_providers.items(): if slug == target_provider: - cfg_models = cfg.get("models", {}) - # Direct membership works for dict (keys) and list (strings) - if new_model in cfg_models: + if new_model in _declared_model_ids(cfg.get("models", {})): override = True break - # Also accept if models is a list of dicts with 'name' field - if isinstance(cfg_models, list): - if any(m.get("name") == new_model for m in cfg_models if isinstance(m, dict)): - override = True - break # Also check custom_providers list — models declared there should be accepted # even if the remote /v1/models endpoint doesn't list them. if not override and custom_providers and isinstance(custom_providers, list): @@ -1270,7 +1298,7 @@ def switch_model( if new_model == entry_model: override = True break - if isinstance(entry_models, dict) and new_model in entry_models: + if new_model in _declared_model_ids(entry_models): override = True break if override: @@ -1961,18 +1989,11 @@ def list_authenticated_providers( if default_model: models_list.append(default_model) # Also include the full models list from config. - # Hermes writes ``models:`` as a dict keyed by model id - # (see hermes_cli/main.py::_save_custom_provider); older - # configs or hand-edited files may still use a list. - cfg_models = ep_cfg.get("models", []) - if isinstance(cfg_models, dict): - for m in cfg_models: - if m and m not in models_list: - models_list.append(m) - elif isinstance(cfg_models, list): - for m in cfg_models: - if m and m not in models_list: - models_list.append(m) + # Hermes writes ``models:`` as a dict keyed by model id, but older + # or hand-edited configs may use strings or ``[{id: ...}]`` rows. + for model_id in _declared_model_ids(ep_cfg.get("models", [])): + if model_id not in models_list: + models_list.append(model_id) # Official OpenAI API rows in providers: often have base_url but no # explicit models: dict — avoid a misleading zero count in /model. @@ -2177,15 +2198,9 @@ def list_authenticated_providers( if default_model and default_model not in groups[group_key]["models"]: groups[group_key]["models"].append(default_model) - cfg_models = entry.get("models", {}) - if isinstance(cfg_models, dict): - for m in cfg_models: - if m and m not in groups[group_key]["models"]: - groups[group_key]["models"].append(m) - elif isinstance(cfg_models, list): - for m in cfg_models: - if m and m not in groups[group_key]["models"]: - groups[group_key]["models"].append(m) + for model_id in _declared_model_ids(entry.get("models", {})): + if model_id not in groups[group_key]["models"]: + groups[group_key]["models"].append(model_id) _section4_emitted_slugs: set = set() _current_base_url_norm = str(current_base_url or "").strip().rstrip("/").lower() diff --git a/tests/hermes_cli/test_inventory.py b/tests/hermes_cli/test_inventory.py index 6bd582d438f..386cda0e0da 100644 --- a/tests/hermes_cli/test_inventory.py +++ b/tests/hermes_cli/test_inventory.py @@ -62,6 +62,32 @@ def test_load_picker_context_full_dict(): assert isinstance(ctx.custom_providers, list) +def test_load_picker_context_normalizes_list_of_dict_models(): + cfg = _cfg( + providers={ + "static-gateway": { + "name": "Static Gateway", + "api": "https://router.example.com/v1", + "default_model": "claude-3-7-sonnet", + "models": [ + {"id": "claude-3-7-sonnet"}, + {"id": "claude-sonnet-4", "context_length": 200000}, + ], + "discover_models": False, + } + }, + ) + with patch("hermes_cli.config.load_config", return_value=cfg): + ctx = load_picker_context() + + assert len(ctx.custom_providers) == 1 + assert ctx.custom_providers[0]["models"] == { + "claude-3-7-sonnet": {}, + "claude-sonnet-4": {"context_length": 200000}, + } + assert ctx.custom_providers[0]["discover_models"] is False + + def test_load_picker_context_falls_back_to_name_when_default_missing(): cfg = _cfg(model={"name": "gpt-5.4", "provider": "openai"}) with patch("hermes_cli.config.load_config", return_value=cfg): @@ -716,6 +742,52 @@ def test_two_custom_providers_with_overlap_both_survive(): assert b_row["total_models"] == 2 +def test_build_models_payload_keeps_static_provider_models_from_providers_dict(): + """The inventory payload must keep configured static models from a + ``providers:`` entry even when the same endpoint also appears via the + compatibility ``custom_providers`` view and live discovery would fail.""" + cfg = _cfg( + model={ + "provider": "static-gateway", + "default": "claude-3-7-sonnet", + }, + providers={ + "static-gateway": { + "name": "Static Gateway", + "api": "https://router.example.com/v1", + "api_key": "sk-test", + "default_model": "claude-3-7-sonnet", + "models": [ + {"id": "claude-3-7-sonnet"}, + {"id": "claude-sonnet-4"}, + ], + "discover_models": False, + } + }, + ) + with ( + patch("hermes_cli.config.load_config", return_value=cfg), + patch("agent.models_dev.fetch_models_dev", return_value={}), + patch("hermes_cli.providers.HERMES_OVERLAYS", {}), + patch( + "hermes_cli.models.fetch_api_models", + side_effect=AssertionError("fetch_api_models must not be called"), + ), + ): + ctx = load_picker_context() + payload = build_models_payload(ctx) + + rows = [ + row + for row in payload["providers"] + if row.get("api_url") == "https://router.example.com/v1" + ] + assert len(rows) == 1 + assert rows[0]["slug"] == "static-gateway" + assert rows[0]["models"] == ["claude-3-7-sonnet", "claude-sonnet-4"] + assert rows[0]["total_models"] == 2 + + def test_build_models_payload_no_max_models_returns_full_list(): """When max_models is not passed (None), build_models_payload must return the full model list — not truncate to the old default of 50. @@ -779,4 +851,3 @@ def test_list_authenticated_providers_refresh_busts_cache(): assert clear.call_count == 0 model_switch.list_authenticated_providers(refresh=True) assert clear.call_count == 1 - diff --git a/tests/hermes_cli/test_model_switch_custom_providers.py b/tests/hermes_cli/test_model_switch_custom_providers.py index 04dbc3f36ed..a1ec07117f2 100644 --- a/tests/hermes_cli/test_model_switch_custom_providers.py +++ b/tests/hermes_cli/test_model_switch_custom_providers.py @@ -927,6 +927,79 @@ def test_custom_providers_discover_models_false_string_is_normalised(monkeypatch assert gateway_prov["models"] == ["only-model"] +def test_custom_providers_discover_models_false_list_of_dict_ids(monkeypatch): + """List-of-dicts ``models: [{id: ...}]`` must be preserved as configured + model IDs when discovery is disabled.""" + monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {}) + monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {}) + + calls = [] + + def fake_fetch_api_models(api_key, base_url, **kwargs): + calls.append((api_key, base_url, kwargs)) + return ["live-a", "live-b"] + + monkeypatch.setattr("hermes_cli.models.fetch_api_models", fake_fetch_api_models) + + custom_providers = [ + { + "name": "static-gateway", + "api_key": "***", + "base_url": "https://router.example.com/v1", + "discover_models": False, + "model": "claude-3-7-sonnet", + "models": [ + {"id": "claude-3-7-sonnet"}, + {"id": "claude-sonnet-4"}, + ], + } + ] + + providers = list_authenticated_providers( + current_provider="openrouter", + current_base_url="https://openrouter.ai/api/v1", + custom_providers=custom_providers, + max_models=50, + ) + + gateway_prov = next( + (p for p in providers if p.get("api_url") == "https://router.example.com/v1"), + None, + ) + + assert gateway_prov is not None + assert calls == [], "discover_models: false must skip live discovery" + assert gateway_prov["models"] == ["claude-3-7-sonnet", "claude-sonnet-4"] + assert gateway_prov["total_models"] == 2 + + +def test_list_of_dict_models_prefers_id_over_label(monkeypatch): + monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {}) + monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {}) + + providers = list_authenticated_providers( + current_provider="openrouter", + current_base_url="https://openrouter.ai/api/v1", + custom_providers=[ + { + "name": "static-gateway", + "base_url": "https://router.example.com/v1", + "discover_models": False, + "models": [{"id": "real-model-id", "name": "Friendly Label"}], + } + ], + max_models=50, + ) + + gateway_prov = next( + (p for p in providers if p.get("api_url") == "https://router.example.com/v1"), + None, + ) + + assert gateway_prov is not None + assert gateway_prov["models"] == ["real-model-id"] + + def test_resolve_custom_provider_passes_key_env(): """resolve_custom_provider should propagate key_env into api_key_env_vars.