fix(models): honor vendor/model prefix and dict model.aliases in provider detection (#87189)
This commit is contained in:
liuhao1024
@@ -562,10 +562,12 @@ def _load_direct_aliases() -> dict[str, DirectAlias]:
|
||||
neither is set the key is resolved from the alias HOST, never from the
|
||||
previously active provider (#83612).
|
||||
|
||||
Also reads ``model.aliases`` (set by ``hermes config set model.aliases.xxx``)
|
||||
and converts simple string entries (``ds-flash: deepseek/deepseek-v4-flash``)
|
||||
into DirectAlias objects. The provider is parsed from the ``provider/``
|
||||
prefix in the value; if no slash, the current provider is used.
|
||||
Also reads ``model.aliases`` (set by ``hermes config set model.aliases.xxx``
|
||||
or hand-written). String entries (``ds-flash: deepseek/deepseek-v4-flash``)
|
||||
are converted into DirectAlias objects with the provider parsed from the
|
||||
``provider/`` prefix in the value; if no slash, the current provider is
|
||||
used. Dict entries use the same shape as ``model_aliases:`` (``model``,
|
||||
``provider``, ``base_url`` keys).
|
||||
"""
|
||||
merged = dict(_BUILTIN_DIRECT_ALIASES)
|
||||
try:
|
||||
@@ -588,18 +590,34 @@ def _load_direct_aliases() -> dict[str, DirectAlias]:
|
||||
key_env=str(entry.get("key_env", "") or "").strip(),
|
||||
)
|
||||
|
||||
# --- model.aliases (string-based format, from config set) ---
|
||||
# --- model.aliases (from config set / hand-written config) ---
|
||||
model_section = cfg.get("model", {})
|
||||
if isinstance(model_section, dict):
|
||||
simple_aliases = model_section.get("aliases")
|
||||
if isinstance(simple_aliases, dict):
|
||||
current_provider = model_section.get("provider", "")
|
||||
for name, value in simple_aliases.items():
|
||||
key = name.strip().lower()
|
||||
if not key or key in merged:
|
||||
continue # don't override explicit model_aliases entries
|
||||
if isinstance(value, dict):
|
||||
# Dict form mirrors the ``model_aliases:`` shape:
|
||||
# localqwen: {model: qwen3.5:4b, provider: custom}.
|
||||
# Hand-written configs already use it; honoring it
|
||||
# here keeps aliases with an explicit provider from
|
||||
# being silently dropped (#87189).
|
||||
model = str(value.get("model") or "").strip()
|
||||
if not model:
|
||||
continue
|
||||
provider = str(value.get("provider") or "").strip()
|
||||
merged[key] = DirectAlias(
|
||||
model=model,
|
||||
provider=provider or current_provider or "custom",
|
||||
base_url=str(value.get("base_url") or "").strip(),
|
||||
)
|
||||
continue
|
||||
if not isinstance(value, str) or not value.strip():
|
||||
continue
|
||||
key = name.strip().lower()
|
||||
if key in merged:
|
||||
continue # don't override explicit model_aliases entries
|
||||
val = value.strip()
|
||||
if "/" in val:
|
||||
provider, model = val.split("/", 1)
|
||||
|
||||
@@ -3614,6 +3614,65 @@ def detect_static_provider_for_model(
|
||||
return None
|
||||
|
||||
|
||||
def _configured_provider_ids() -> set[str]:
|
||||
"""Provider ids defined in the user's config ``providers:`` block.
|
||||
|
||||
Includes both top-level ids (``ollama``, ``nous``) and ``custom:*``
|
||||
profile ids. Returns an empty set when config is unreadable — callers
|
||||
treat that as "no user-defined providers" and fall through to built-in
|
||||
catalogs only.
|
||||
"""
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
cfg = load_config() or {}
|
||||
providers = cfg.get("providers")
|
||||
if not isinstance(providers, dict):
|
||||
return set()
|
||||
ids: set[str] = set()
|
||||
for pid in providers:
|
||||
key = str(pid).strip().lower()
|
||||
if key:
|
||||
ids.add(key)
|
||||
return ids
|
||||
except Exception:
|
||||
return set()
|
||||
|
||||
|
||||
def _resolve_provider_prefix(model_name: str) -> Optional[tuple[str, str]]:
|
||||
"""Resolve an explicit ``vendor/model`` prefix to a known provider.
|
||||
|
||||
``nous/deepseek-v4-pro`` or ``ollama/qwen3.5:4b`` should route to the
|
||||
named provider instead of falling back to the configured default (which
|
||||
silently sends non-default models to the wrong endpoint, #87189). The
|
||||
vendor counts as known when it is a built-in provider id/alias or a key
|
||||
in the user's ``providers:`` config block. The returned model is the
|
||||
suffix with the prefix stripped — the provider's API expects the bare id.
|
||||
"""
|
||||
if "/" not in model_name:
|
||||
return None
|
||||
vendor, model = model_name.split("/", 1)
|
||||
vendor = vendor.strip().lower()
|
||||
model = model.strip()
|
||||
if not vendor or not model:
|
||||
return None
|
||||
configured = _configured_provider_ids()
|
||||
# A provider block the user explicitly named (``ollama:``) wins over the
|
||||
# built-in alias table, which may canonicalize the same name elsewhere
|
||||
# (``ollama`` → ``custom``) and route to the wrong endpoint.
|
||||
if vendor in configured:
|
||||
return (vendor, model)
|
||||
canonical = _PROVIDER_ALIASES.get(vendor, vendor)
|
||||
known = (
|
||||
canonical in _PROVIDER_LABELS
|
||||
or canonical in _PROVIDER_MODELS
|
||||
or canonical in configured
|
||||
)
|
||||
if not known:
|
||||
return None
|
||||
return (canonical, model)
|
||||
|
||||
|
||||
def detect_provider_for_model(
|
||||
model_name: str,
|
||||
current_provider: str,
|
||||
@@ -3650,6 +3709,15 @@ def detect_provider_for_model(
|
||||
return ("openrouter", or_slug)
|
||||
return None # already on openrouter with matching name
|
||||
|
||||
# --- Step 3: explicit ``vendor/model`` prefix naming a provider ---
|
||||
# Checked after the OpenRouter slug lookup so aggregator-native slugs
|
||||
# (e.g. ``deepseek/deepseek-chat``) keep their existing routing; this
|
||||
# step only catches names no catalog serves, which previously fell back
|
||||
# to the configured default provider and 404'd (#87189).
|
||||
prefix_match = _resolve_provider_prefix(name)
|
||||
if prefix_match is not None:
|
||||
return prefix_match
|
||||
|
||||
return None
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
"""Regression tests for vendor-prefix model routing and dict model.aliases (#87189).
|
||||
|
||||
``--model nous/deepseek-v4-pro`` / ``--model ollama/qwen3.5:4b`` used to fall
|
||||
through provider auto-detection and be sent to the configured default provider
|
||||
(api.anthropic.com) with the prefixed name intact, producing HTTP 404. Dict
|
||||
entries under ``model.aliases`` (``localqwen: {model: ..., provider: ...}``)
|
||||
were silently dropped because only string values were parsed.
|
||||
"""
|
||||
|
||||
import hermes_cli.models as models
|
||||
import hermes_cli.model_switch as model_switch
|
||||
|
||||
|
||||
class TestVendorPrefixRouting:
|
||||
"""detect_provider_for_model honors an explicit ``vendor/model`` prefix."""
|
||||
|
||||
def test_builtin_provider_prefix_routes_to_provider(self, monkeypatch):
|
||||
monkeypatch.setattr(models, "_find_openrouter_slug", lambda _name: None)
|
||||
detected = models.detect_provider_for_model("nous/deepseek-v4-pro", "anthropic")
|
||||
assert detected == ("nous", "deepseek-v4-pro")
|
||||
|
||||
def test_configured_provider_prefix_routes_to_provider(self, monkeypatch):
|
||||
monkeypatch.setattr(models, "_find_openrouter_slug", lambda _name: None)
|
||||
monkeypatch.setattr(models, "_configured_provider_ids", lambda: {"ollama"})
|
||||
detected = models.detect_provider_for_model("ollama/qwen3.5:4b", "anthropic")
|
||||
assert detected == ("ollama", "qwen3.5:4b")
|
||||
|
||||
def test_configured_provider_wins_over_alias_canonicalization(self, monkeypatch):
|
||||
"""A user-named ``ollama`` block must not be rewritten to ``custom``."""
|
||||
monkeypatch.setattr(models, "_find_openrouter_slug", lambda _name: None)
|
||||
monkeypatch.setattr(models, "_configured_provider_ids", lambda: {"ollama"})
|
||||
assert models._PROVIDER_ALIASES.get("ollama") == "custom" # precondition
|
||||
detected = models.detect_provider_for_model("ollama/qwen3.5:4b", "anthropic")
|
||||
assert detected == ("ollama", "qwen3.5:4b")
|
||||
|
||||
def test_provider_alias_prefix_canonicalized(self, monkeypatch):
|
||||
monkeypatch.setattr(models, "_find_openrouter_slug", lambda _name: None)
|
||||
monkeypatch.setattr(models, "_configured_provider_ids", lambda: set())
|
||||
detected = models.detect_provider_for_model("glm/glm-4.7", "anthropic")
|
||||
assert detected == ("zai", "glm-4.7")
|
||||
|
||||
def test_unknown_vendor_prefix_still_unmatched(self, monkeypatch):
|
||||
monkeypatch.setattr(models, "_find_openrouter_slug", lambda _name: None)
|
||||
monkeypatch.setattr(models, "_configured_provider_ids", lambda: set())
|
||||
assert models.detect_provider_for_model("notaprovider/foo-model", "anthropic") is None
|
||||
|
||||
def test_openrouter_slug_still_wins_over_prefix_routing(self, monkeypatch):
|
||||
"""Aggregator-native slugs keep their existing OpenRouter routing."""
|
||||
monkeypatch.setattr(
|
||||
models, "_find_openrouter_slug", lambda _name: "deepseek/deepseek-chat"
|
||||
)
|
||||
monkeypatch.setattr(models, "_configured_provider_ids", lambda: set())
|
||||
detected = models.detect_provider_for_model("deepseek/deepseek-chat", "anthropic")
|
||||
assert detected == ("openrouter", "deepseek/deepseek-chat")
|
||||
|
||||
def test_bare_model_detection_unchanged(self, monkeypatch):
|
||||
monkeypatch.setattr(models, "_find_openrouter_slug", lambda _name: None)
|
||||
detected = models.detect_provider_for_model("deepseek-chat", "anthropic")
|
||||
assert detected == ("deepseek", "deepseek-chat")
|
||||
|
||||
|
||||
class TestDictModelAliases:
|
||||
"""``model.aliases`` accepts dict entries with an explicit provider."""
|
||||
|
||||
def _load_with(self, monkeypatch, cfg):
|
||||
monkeypatch.setattr("hermes_cli.config.load_config", lambda: cfg)
|
||||
return model_switch._load_direct_aliases()
|
||||
|
||||
def test_dict_entry_with_explicit_provider(self, monkeypatch):
|
||||
cfg = {
|
||||
"model": {
|
||||
"aliases": {
|
||||
"localqwen": {"model": "qwen3.5:4b", "provider": "custom"},
|
||||
},
|
||||
},
|
||||
}
|
||||
aliases = self._load_with(monkeypatch, cfg)
|
||||
da = aliases["localqwen"]
|
||||
assert (da.model, da.provider) == ("qwen3.5:4b", "custom")
|
||||
|
||||
def test_dict_entry_with_base_url(self, monkeypatch):
|
||||
cfg = {
|
||||
"model": {
|
||||
"aliases": {
|
||||
"qwen": {
|
||||
"model": "qwen3.5:4b",
|
||||
"provider": "ollama",
|
||||
"base_url": "http://localhost:11434/v1",
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
aliases = self._load_with(monkeypatch, cfg)
|
||||
da = aliases["qwen"]
|
||||
assert (da.model, da.provider, da.base_url) == (
|
||||
"qwen3.5:4b", "ollama", "http://localhost:11434/v1",
|
||||
)
|
||||
|
||||
def test_dict_entry_without_provider_uses_model_provider(self, monkeypatch):
|
||||
cfg = {
|
||||
"model": {
|
||||
"provider": "openrouter",
|
||||
"aliases": {"bare": {"model": "some-model"}},
|
||||
},
|
||||
}
|
||||
aliases = self._load_with(monkeypatch, cfg)
|
||||
da = aliases["bare"]
|
||||
assert (da.model, da.provider) == ("some-model", "openrouter")
|
||||
|
||||
def test_string_entries_still_parse(self, monkeypatch):
|
||||
cfg = {
|
||||
"model": {
|
||||
"aliases": {"ds-flash": "deepseek/deepseek-v4-flash"},
|
||||
},
|
||||
}
|
||||
aliases = self._load_with(monkeypatch, cfg)
|
||||
da = aliases["ds-flash"]
|
||||
assert (da.model, da.provider) == ("deepseek-v4-flash", "deepseek")
|
||||
|
||||
def test_model_aliases_block_keeps_priority_over_model_aliases(self, monkeypatch):
|
||||
cfg = {
|
||||
"model_aliases": {
|
||||
"shared": {"model": "from-top-block", "provider": "custom"},
|
||||
},
|
||||
"model": {
|
||||
"aliases": {"shared": {"model": "from-nested", "provider": "ollama"}},
|
||||
},
|
||||
}
|
||||
aliases = self._load_with(monkeypatch, cfg)
|
||||
assert aliases["shared"].model == "from-top-block"
|
||||
Reference in New Issue
Block a user