From b2d2cf4e248c68796c14ebc62ee3fd489e5cb6d7 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 19 Sep 2026 00:59:40 +0530 Subject: [PATCH] fix(cli): the startup route decodes custom: from the caller's providers, not a second config read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings folded: (1) `parse_model_input` resolved the configured `custom:` ids through its own `load_config()`, so the `user_providers`/ `custom_providers` the three startup callers pass were ignored for this branch — two config sources for one decision. It now takes `custom_ids` and the route builds them from its arguments with `custom_provider_slug`, the same identity `providers:` entries carry everywhere else. (2) The "typo" guard was wrong: it fired on legitimate bare-custom tags (`custom:qwen3.5:4b`) and its `None` fell straight back into the default-provider egress the fix exists to prevent. A bare `custom` route with an unknown id now 404s on the user's own endpoint, matching `/model`. Docstring lists the `provider:model` form. --- hermes_cli/model_switch.py | 18 +++++++++--------- hermes_cli/models.py | 12 ++++++++---- tests/hermes_cli/test_startup_model_routing.py | 6 +++++- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 5c7318186e..1df8d9c65c 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -357,7 +357,7 @@ def resolve_startup_model_route( raw_model: str, *, explicit_provider: str = "", current_provider: str = "", user_providers: Optional[dict] = None, custom_providers: Optional[list] = None) -> Optional[StartupModelRoute]: - """Resolve aliases and configured ``provider/model`` input at startup. + """Resolve aliases, ``provider:model`` and configured ``provider/model`` input at startup. ``HermesCLI`` is constructed before the interactive ``/model`` pipeline runs; resolving here keeps startup from attaching the configured default provider to an explicitly requested @@ -389,15 +389,15 @@ def resolve_startup_model_route( return None # ``custom::`` / ``:`` — the same qualified form ``/model`` # accepts. Left undecoded, the configured default provider receives the unsplit string as - # the model name and the whole prompt goes to its endpoint before it 404s (#73943). + # the model name and the whole prompt goes to its endpoint before it 404s (#73943). The + # configured ids come from the caller's config, the same source the ``/`` branch below uses. from hermes_cli.models import parse_model_input - qualified_provider, qualified_model = parse_model_input(raw, "") - if qualified_provider == "custom" and ":" in qualified_model: - # ``custom::`` — no such named provider; a bare-custom request with a garbage - # model id would be as silent as the default-provider egress this decode prevents. - logger.warning("No providers.%s entry configured; ignoring provider prefix in %r", - qualified_model.split(":", 1)[0], raw) - return None + from hermes_cli.providers import custom_provider_slug + custom_ids = {custom_provider_slug(str(entry.get("name") or key), str(key)) + for key, entry in (user_providers or {}).items() if isinstance(entry, dict)} + custom_ids.update(custom_provider_slug(str(entry.get("name") or "")) + for entry in (custom_providers or []) if isinstance(entry, dict) and _clean(entry.get("name"))) + qualified_provider, qualified_model = parse_model_input(raw, "", custom_ids=custom_ids) if qualified_provider: return StartupModelRoute(model=qualified_model, provider=qualified_provider) if "/" not in raw: diff --git a/hermes_cli/models.py b/hermes_cli/models.py index 61ca444e6f..b04a475b5e 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -728,9 +728,12 @@ def list_available_providers() -> list[dict[str, str]]: for pid in [p.slug for p in CANONICAL_PROVIDERS] + ["custom"]] -def parse_model_input(raw: str, current_provider: str) -> tuple[str, str]: +def parse_model_input( + raw: str, current_provider: str, *, custom_ids: Optional[set[str]] = None) -> tuple[str, str]: """Parse ``/model`` input into ``(provider, model)``. The colon is a provider delimiter only when - the left side is a known provider/alias, so ``anthropic/claude-3.5-sonnet:beta`` stays a model.""" + the left side is a known provider/alias, so ``anthropic/claude-3.5-sonnet:beta`` stays a model. + ``custom_ids`` is the caller's already-loaded set of configured ``custom:`` ids (default: + read from config) so one decision never consults two config sources.""" stripped = raw.strip() colon = stripped.find(":") if colon > 0: @@ -738,9 +741,10 @@ def parse_model_input(raw: str, current_provider: str) -> tuple[str, str]: model_part = stripped[colon + 1:].strip() if provider_part and model_part and provider_part in _KNOWN_PROVIDER_NAMES: if provider_part == "custom": + configured = _configured_custom_provider_ids() if custom_ids is None else custom_ids # Longest configured ``custom:`` id that prefixes the input wins. lowered = stripped.lower() - for custom_id in sorted(_configured_custom_provider_ids() - {"custom"}, key=len, reverse=True): + for custom_id in sorted(configured - {"custom"}, key=len, reverse=True): if lowered.startswith(f"{custom_id.lower()}:"): return custom_id, stripped[len(custom_id) + 1 :].strip() # ``custom:local:qwen`` → ("custom:local", "qwen") for a configured named provider; @@ -748,7 +752,7 @@ def parse_model_input(raw: str, current_provider: str) -> tuple[str, str]: if ":" in model_part: custom_name, actual_model = (part.strip() for part in model_part.split(":", 1)) if custom_name and actual_model: - if f"custom:{custom_name.lower()}" in _configured_custom_provider_ids(): + if f"custom:{custom_name.lower()}" in configured: return (f"custom:{custom_name.lower()}", actual_model) return ("custom", model_part) return (normalize_provider(provider_part), model_part) diff --git a/tests/hermes_cli/test_startup_model_routing.py b/tests/hermes_cli/test_startup_model_routing.py index 363ac4980f..d814086409 100644 --- a/tests/hermes_cli/test_startup_model_routing.py +++ b/tests/hermes_cli/test_startup_model_routing.py @@ -176,6 +176,10 @@ def test_startup_route_decodes_custom_colon_qualified_model(tmp_path, monkeypatc "custom:jetson-vllm:nemotron-nano-30b", current_provider="anthropic", user_providers=cfg.get("providers")) assert route == model_switch.StartupModelRoute("nemotron-nano-30b", "custom:jetson-vllm", "") + # The caller's providers are the only source: without the entry the prefix is bare ``custom``. + assert model_switch.resolve_startup_model_route( + "custom:jetson-vllm:nemotron-nano-30b", current_provider="anthropic", user_providers={} + ).provider == "custom" # A colon inside a plain model id is not a provider delimiter. assert model_switch.resolve_startup_model_route( "anthropic/claude-3.5-sonnet:beta", current_provider="anthropic", @@ -187,9 +191,9 @@ def test_oneshot_and_tui_qualified_model_never_reaches_default_provider(tmp_path startup owner, so provider auto-detection never hands the qualified string to the configured default (#73943).""" from hermes_cli.oneshot import _resolve_model_and_provider - from tui_gateway import server as tui_server cfg = _write_named_provider(tmp_path, monkeypatch) + from tui_gateway import server as tui_server # binds the config path at import: after HERMES_HOME monkeypatch.delenv("HERMES_INFERENCE_PROVIDER", raising=False) monkeypatch.delenv("HERMES_TUI_PROVIDER", raising=False) monkeypatch.setattr(