diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index 7a17fc83943f..58202c7ec194 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -862,10 +862,78 @@ def find_custom_provider_identity(base_url: str) -> Optional[str]: return None +def find_custom_provider_identity_by_model(model: str) -> Optional[str]: + """Map a model id back to the ``custom:`` entry that serves it. + + Returns the ``custom:`` slug of the first ``providers:`` + / ``custom_providers:`` entry whose ``model`` / ``default_model`` matches, + or whose ``models`` catalog (dict or list shape) contains the id. + ``None`` when no entry serves the model. + + Companion to :func:`find_custom_provider_identity` (URL reverse-lookup) + for the persistence paths where no base_url survived the round-trip: the + session row always stores the model name, and a custom endpoint's model + ids (e.g. an in-house SFT checkpoint) virtually never collide with + catalog models on built-in providers, so the model is the last durable + fact that can recover the entry identity. + """ + target = str(model or "").strip().lower() + if not target: + return None + try: + config = load_config() + except Exception: + return None + + def _entry_serves_model(entry: Dict[str, Any]) -> bool: + for key in ("model", "default_model"): + value = entry.get(key) + if isinstance(value, str) and value.strip().lower() == target: + return True + models = entry.get("models") + if isinstance(models, dict): + return any( + str(mid).strip().lower() == target for mid in models.keys() + ) + if isinstance(models, list): + for item in models: + if isinstance(item, str) and item.strip().lower() == target: + return True + if isinstance(item, dict): + mid = item.get("id") or item.get("name") + if isinstance(mid, str) and mid.strip().lower() == target: + return True + return False + + providers = config.get("providers") + if isinstance(providers, dict): + for ep_name, entry in providers.items(): + if not isinstance(entry, dict): + continue + if _entry_serves_model(entry): + return f"custom:{_normalize_custom_provider_name(str(ep_name))}" + + try: + custom_providers = get_compatible_custom_providers(config) + except Exception: + custom_providers = None + for entry in custom_providers or []: + if not isinstance(entry, dict): + continue + name = entry.get("name") + if not isinstance(name, str) or not name.strip(): + continue + if _entry_serves_model(entry): + return f"custom:{_normalize_custom_provider_name(name)}" + + return None + + def canonical_custom_identity( *, base_url: Optional[str] = None, config_provider: Optional[str] = None, + model: Optional[str] = None, ) -> Optional[str]: """Recover a routable ``custom:`` identity for a bare custom provider. @@ -877,17 +945,24 @@ def canonical_custom_identity( Any code path that persists or restores a session's provider override must run the resolved provider through this helper so a bare ``"custom"`` - is upgraded back to its durable ``custom:`` menu key. Two recovery - sources, in priority order: + is upgraded back to its durable ``custom:`` menu key. Three + recovery sources, in priority order: 1. ``base_url`` — reverse-lookup the entry that owns the endpoint URL (the one fact that always survives the persistence round-trip when a URL was recorded). - 2. ``config_provider`` — the active ``config.model.provider`` (or its - ``provider``/``HERMES_INFERENCE_PROVIDER`` equivalent). When the agent - was built without a base_url on the override (the recurring - Desktop/TUI regression vector), the configured provider is the only - durable identity left, so fall back to it when it names a real entry. + 2. ``model`` — reverse-lookup the entry that serves the session's model + (``model``/``default_model``/``models`` catalog). The session row + always stores the model name, so when no base_url survived (the + recurring Desktop/TUI regression vector) the model is the last + session-scoped fact that can recover the entry — and unlike the + config fallback below it stays correct after the user points their + global default at a different provider. + 3. ``config_provider`` — the active ``config.model.provider`` (or its + ``provider``/``HERMES_INFERENCE_PROVIDER`` equivalent). When neither + a base_url nor a model recovered the entry, the configured provider + is the only durable identity left, so fall back to it when it names + a real entry. Returns ``custom:`` when a routable identity is recovered, else ``None`` (caller keeps whatever it had — bare ``"custom"`` only as a last @@ -899,7 +974,13 @@ def canonical_custom_identity( if identity: return identity - # 2. Fall back to the configured provider when it names a real entry. + # 2. Reverse-lookup by the session's model name. + if model: + identity = find_custom_provider_identity_by_model(model) + if identity: + return identity + + # 3. Fall back to the configured provider when it names a real entry. candidate = str(config_provider or "").strip() if not candidate: try: diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 32994cf0be30..889a637d36e8 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -9561,6 +9561,7 @@ def test_model_options_preserves_canonical_custom_row_after_agent_init(monkeypat canonical.assert_called_once_with( base_url="http://127.0.0.1:11434/v1", config_provider="custom:local-ollama", + model="qwen3.6:35b-65k", ) diff --git a/tests/tui_gateway/test_custom_provider_session_persistence.py b/tests/tui_gateway/test_custom_provider_session_persistence.py index f78a5d8c2fa8..0715aa9ee820 100644 --- a/tests/tui_gateway/test_custom_provider_session_persistence.py +++ b/tests/tui_gateway/test_custom_provider_session_persistence.py @@ -352,3 +352,158 @@ class TestBareCustomNoBaseUrlHealsFromConfig: assert persisted.get("provider") == "custom:mimo-v2.5-pro" +# --- Regression: bare "custom" + no base_url + DIFFERENT default provider ---- +# +# The config-provider fallback above only heals when ``config.model.provider`` +# still points at the custom entry. A user whose global default is a built-in +# provider (e.g. Nous) but who switched THIS session to a self-hosted model +# gets no heal: the bare provider is dropped, resume falls back to the default +# provider, and the default provider's endpoint 404s with "Model '' not +# found" (the b200/hermes-ultra-sft report). The stored MODEL NAME is the one +# session-scoped fact that still identifies the entry — these tests lock the +# model-name recovery tier. + +ULTRA_URL = "http://b200-cluster:30090/v1" + +ULTRA_CONFIG = { + # Global default deliberately points at a BUILT-IN provider — the config + # fallback must not fire; only the model lookup can recover the entry. + "model": {"default": "some-nous-model", "provider": "nous"}, + "providers": { + "hermes-ultra": { + "api": ULTRA_URL, + "api_key": "sk-ultra", + "models": ["hermes-ultra-sft"], + } + }, +} + +ULTRA_LEGACY_CONFIG = { + "model": {"default": "some-nous-model", "provider": "nous"}, + "custom_providers": [ + { + "name": "hermes-ultra", + "base_url": ULTRA_URL, + "api_key": "sk-ultra", + "model": "hermes-ultra-sft", + } + ], +} + + +class TestModelNameRecoversEntryIdentity: + def test_identity_by_model_from_providers_dict_models_list(self, monkeypatch): + monkeypatch.setattr(rp, "load_config", lambda: ULTRA_CONFIG) + + assert ( + rp.find_custom_provider_identity_by_model("hermes-ultra-sft") + == "custom:hermes-ultra" + ) + + def test_identity_by_model_from_legacy_default_model(self, monkeypatch): + monkeypatch.setattr(rp, "load_config", lambda: ULTRA_LEGACY_CONFIG) + + assert ( + rp.find_custom_provider_identity_by_model("hermes-ultra-sft") + == "custom:hermes-ultra" + ) + + def test_identity_by_model_unknown_model_returns_none(self, monkeypatch): + monkeypatch.setattr(rp, "load_config", lambda: ULTRA_CONFIG) + + assert rp.find_custom_provider_identity_by_model("gpt-5.5") is None + assert rp.find_custom_provider_identity_by_model("") is None + + def test_canonical_identity_prefers_base_url_over_model(self, monkeypatch): + """URL ownership beats model-name matching when both are present.""" + config = { + "model": {"default": "x", "provider": "nous"}, + "providers": { + "by-url": {"api": ULTRA_URL, "api_key": "k1"}, + "by-model": { + "api": "http://other:9000/v1", + "api_key": "k2", + "models": ["hermes-ultra-sft"], + }, + }, + } + monkeypatch.setattr(rp, "load_config", lambda: config) + + assert ( + rp.canonical_custom_identity( + base_url=ULTRA_URL, model="hermes-ultra-sft" + ) + == "custom:by-url" + ) + + def test_canonical_identity_recovers_from_model_when_config_points_elsewhere( + self, monkeypatch + ): + monkeypatch.setattr(rp, "load_config", lambda: ULTRA_CONFIG) + monkeypatch.setattr(rp, "_get_model_config", lambda: ULTRA_CONFIG["model"]) + + assert ( + rp.canonical_custom_identity(base_url=None, model="hermes-ultra-sft") + == "custom:hermes-ultra" + ) + + def test_restore_heals_bare_custom_row_via_model_name(self, monkeypatch): + """The b200/hermes-ultra-sft report: row has bare custom, no base_url, + and the global default provider is a built-in. Before the model tier, + the bare provider was dropped and resume silently rerouted the + session's model to the default provider (Nous 404: "Model + 'hermes-ultra-sft' not found... OpenRouter catalog").""" + monkeypatch.setattr(rp, "load_config", lambda: ULTRA_CONFIG) + monkeypatch.setattr(rp, "_get_model_config", lambda: ULTRA_CONFIG["model"]) + + from tui_gateway.server import _stored_session_runtime_overrides + + row = { + "model": "hermes-ultra-sft", + "model_config": json.dumps( + {"model": "hermes-ultra-sft", "provider": "custom"} + ), + "billing_provider": "custom", + } + overrides = _stored_session_runtime_overrides(row) + + assert overrides["provider_override"] == "custom:hermes-ultra" + assert overrides["model_override"]["provider"] == "custom:hermes-ultra" + + def test_persist_heals_bare_custom_via_model_when_no_base_url(self, monkeypatch): + monkeypatch.setattr(rp, "load_config", lambda: ULTRA_CONFIG) + monkeypatch.setattr(rp, "_get_model_config", lambda: ULTRA_CONFIG["model"]) + + from tui_gateway.server import _runtime_model_config + + agent = types.SimpleNamespace( + model="hermes-ultra-sft", + provider="custom", + base_url="", + api_mode="chat_completions", + reasoning_config=None, + service_tier=None, + ) + config = _runtime_model_config(agent) + + assert config["provider"] == "custom:hermes-ultra" + + def test_make_agent_heals_via_model_end_to_end(self, monkeypatch): + """resume → _make_agent with bare custom + no base_url + built-in + global default must rebuild against the entry's endpoint + key, not + the default provider.""" + override = { + "model": "hermes-ultra-sft", + "provider": "custom", + "base_url": None, + "api_mode": "chat_completions", + } + + kwargs = _make_agent_with_override( + override, monkeypatch, ULTRA_CONFIG, model_cfg=ULTRA_CONFIG["model"] + ) + + assert kwargs["base_url"] == ULTRA_URL + assert kwargs["api_key"] == "sk-ultra" + + diff --git a/tui_gateway/server.py b/tui_gateway/server.py index cb6bfccf1427..581f287e2c09 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -2119,7 +2119,8 @@ def _ensure_session_db_row(session: dict) -> None: from hermes_cli.runtime_provider import canonical_custom_identity healed = canonical_custom_identity( - base_url=model_config.get("base_url") or None + base_url=model_config.get("base_url") or None, + model=model_config.get("model") or row_model or None, ) if healed: model_config["provider"] = healed @@ -2839,9 +2840,10 @@ def _stored_session_runtime_overrides(row: dict | None) -> dict: # the resolved billing class, not a routable identity — restoring it as the # session's provider override routes the resume to the OpenRouter default # URL with no api_key, surfacing as "No LLM provider configured". Recover - # the durable ``custom:`` menu key from the stored base_url, falling - # back to the configured provider when the row has no base_url (the - # recurring Desktop/TUI regression vector). If neither names a real entry, + # the durable ``custom:`` menu key from the stored base_url, then + # from the entry that serves the stored model, falling back to the + # configured provider when the row has neither (the recurring Desktop/TUI + # regression vector). If none names a real entry, # drop the bare provider entirely so resume falls back to the configured # default rather than the broken OpenRouter route. if provider.strip().lower() == "custom": @@ -2849,7 +2851,9 @@ def _stored_session_runtime_overrides(row: dict | None) -> dict: try: from hermes_cli.runtime_provider import canonical_custom_identity - healed = canonical_custom_identity(base_url=base_url or None) + healed = canonical_custom_identity( + base_url=base_url or None, model=model or None + ) except Exception: logger.debug( "custom provider identity recovery failed", exc_info=True @@ -2913,7 +2917,10 @@ def _runtime_model_config(agent, existing: dict | None = None) -> dict: ) provider = ( - canonical_custom_identity(base_url=base_url) or provider + canonical_custom_identity( + base_url=base_url, model=model or None + ) + or provider ) except Exception: logger.debug( @@ -5449,7 +5456,9 @@ def _make_agent( # (the recurring Desktop/TUI regression vector). from hermes_cli.runtime_provider import canonical_custom_identity - recovered = canonical_custom_identity(base_url=override_base_url or None) + recovered = canonical_custom_identity( + base_url=override_base_url or None, model=model or None + ) if recovered: requested_provider = recovered if override_base_url: @@ -15383,6 +15392,8 @@ def _model_picker_context(agent): canonical_custom_identity( base_url=base_url or None, config_provider=ctx.current_provider, + model=(getattr(agent, "model", "") if agent else "") + or None, ) or provider )