fix(cli): the startup route decodes custom:<name> from the caller's providers, not a second config read
Review findings folded: (1) `parse_model_input` resolved the configured `custom:<name>` 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.
This commit is contained in:
@@ -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:<name>:<model>`` / ``<provider>:<model>`` — 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:<typo>:<model>`` — 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:
|
||||
|
||||
@@ -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:<name>`` 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:<name>`` 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)
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user