mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(config): canonicalize model.name/model.model to model.default (#34500)
A custom_providers config that names the model under model.name (or model.model) resolved to an empty model, so the API request went out with model= — HTTP 400 from OpenAI-compatible backends. Display paths (hermes status/dump) already read model.name and showed the model, making the failure silent. The model id was read via 'default or model' at ~14 independent sites (cli, gateway, cron, curator, oneshot, fallback, profiles, ...), none of which honored 'name'. Rather than patch every site, canonicalize at the single load/save chokepoint: _normalize_root_model_keys() now promotes model.model/model.name -> model.default (precedence default > model > name) and drops the stale alias, so every reader — present and future — sees a populated default and config.yaml is migrated canonical on next save. The gateway, which bypasses load_config(), replays the same normalization in _load_gateway_config(). Co-authored-by: Bartok9 <danielrpike9@gmail.com> Credit: root-cause analysis and fix direction from @Bartok9 (#34502, first) and @v86861062 (#34527).
This commit is contained in:
parent
2ecb6f7fe6
commit
64972b6403
4 changed files with 127 additions and 3 deletions
|
|
@ -2173,7 +2173,20 @@ def _load_gateway_config() -> dict:
|
|||
raw = managed_scope.apply_managed_overlay(raw if isinstance(raw, dict) else {})
|
||||
except Exception:
|
||||
pass
|
||||
return raw if isinstance(raw, dict) else {}
|
||||
if not isinstance(raw, dict):
|
||||
return {}
|
||||
# Canonicalize model-id aliases (model.name / model.model → model.default)
|
||||
# and migrate stale root-level provider/base_url into the model section.
|
||||
# The gateway bypasses load_config() (it reads raw YAML for speed), so the
|
||||
# normalization that load_config() applies must be replayed here or the
|
||||
# gateway would resolve an empty model for ``model: {name: <id>}`` configs
|
||||
# while the CLI resolves it correctly. See issue #34500. Fail-open.
|
||||
try:
|
||||
from hermes_cli.config import _normalize_root_model_keys
|
||||
raw = _normalize_root_model_keys(raw)
|
||||
except Exception:
|
||||
pass
|
||||
return raw
|
||||
|
||||
|
||||
def _load_gateway_runtime_config() -> dict:
|
||||
|
|
|
|||
|
|
@ -5812,14 +5812,35 @@ def _normalize_root_model_keys(config: Dict[str, Any]) -> Dict[str, Any]:
|
|||
``model.base_url``), causing requests to fall back to OpenRouter. We migrate
|
||||
the alias to the canonical key (fallback-only — never override an explicit
|
||||
``base_url``) and drop the alias so it can't confuse later loads.
|
||||
|
||||
Finally, canonicalizes the model-id key to ``model.default`` (issue #34500).
|
||||
The runtime resolver and ~14 other readers select the chat model via
|
||||
``model.default``; ``model.model`` was already aliased inline at some sites
|
||||
but ``model.name`` was not, so a custom-provider config like
|
||||
``model: {name: <id>, provider: <custom>}`` resolved to an empty model and
|
||||
the API request went out with ``model=`` (HTTP 400 from OpenAI-compatible
|
||||
backends) — while display paths (``hermes status``/``dump``) read ``name``
|
||||
and *showed* the model, making the failure silent. Normalizing here (the
|
||||
single load/save chokepoint) means every reader, present and future, sees a
|
||||
populated ``default`` and the stale alias is migrated out of config.yaml on
|
||||
the next save. Precedence: ``default`` > ``model`` > ``name`` (never
|
||||
overrides an explicit ``default``, so existing configs are unaffected).
|
||||
"""
|
||||
# Only act if there are root-level keys (or an api_base alias) to migrate
|
||||
# Only act if there's something to migrate: root-level keys, an api_base
|
||||
# alias, or a model dict whose id lives under a non-canonical key.
|
||||
model_in = config.get("model")
|
||||
model_has_alias = isinstance(model_in, dict) and model_in.get("api_base")
|
||||
# A model dict needs canonicalization if its id lives under a non-canonical
|
||||
# key (``model``/``name``) — either because ``default`` is empty (we must
|
||||
# promote the alias) or because ``default`` is set but a stale alias still
|
||||
# lingers (we must drop it so config.yaml ends up canonical).
|
||||
model_needs_canon = isinstance(model_in, dict) and (
|
||||
model_in.get("model") or model_in.get("name")
|
||||
)
|
||||
has_root = any(
|
||||
config.get(k) for k in ("provider", "base_url", "context_length", "api_base")
|
||||
)
|
||||
if not has_root and not model_has_alias:
|
||||
if not has_root and not model_has_alias and not model_needs_canon:
|
||||
return config
|
||||
|
||||
config = dict(config)
|
||||
|
|
@ -5843,6 +5864,18 @@ def _normalize_root_model_keys(config: Dict[str, Any]) -> Dict[str, Any]:
|
|||
config.pop("api_base", None)
|
||||
model.pop("api_base", None)
|
||||
|
||||
# Canonicalize the model id to ``default``. ``model`` and ``name`` are
|
||||
# last-resort aliases (in that order) — only consulted when ``default`` is
|
||||
# empty, then dropped so later loads/saves can't reintroduce the ambiguity.
|
||||
if not (model.get("default") or ""):
|
||||
alias = model.get("model") or model.get("name")
|
||||
if alias:
|
||||
model["default"] = alias
|
||||
if model.get("default"):
|
||||
# Drop the now-redundant aliases so config.yaml ends up canonical.
|
||||
model.pop("model", None)
|
||||
model.pop("name", None)
|
||||
|
||||
return config
|
||||
|
||||
|
||||
|
|
|
|||
BIN
infographic/model-name-canon/infographic.png
Normal file
BIN
infographic/model-name-canon/infographic.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 1.6 MiB |
|
|
@ -731,6 +731,84 @@ class TestRootLevelProviderOverride:
|
|||
assert result["model"]["context_length"] == 128000
|
||||
assert "context_length" not in result
|
||||
|
||||
# --- model-id alias canonicalization (issue #34500) -------------------
|
||||
# ``model.name`` / ``model.model`` must canonicalize to ``model.default``
|
||||
# so the runtime resolver (and ~14 other readers) never sends an empty
|
||||
# ``model=`` to the backend. Precedence: default > model > name.
|
||||
|
||||
def test_normalize_model_name_aliases_to_default(self):
|
||||
"""model.name (custom-provider repro) becomes model.default (#34500)."""
|
||||
from hermes_cli.config import _normalize_root_model_keys
|
||||
|
||||
config = {
|
||||
"model": {"name": "claude-sonnet-4-20250514", "provider": "my-litellm"},
|
||||
}
|
||||
result = _normalize_root_model_keys(config)
|
||||
assert result["model"]["default"] == "claude-sonnet-4-20250514"
|
||||
assert "name" not in result["model"] # stale alias dropped
|
||||
|
||||
def test_normalize_model_alias_to_default(self):
|
||||
"""model.model becomes model.default."""
|
||||
from hermes_cli.config import _normalize_root_model_keys
|
||||
|
||||
result = _normalize_root_model_keys({"model": {"model": "via-model-key"}})
|
||||
assert result["model"]["default"] == "via-model-key"
|
||||
assert "model" not in result["model"]
|
||||
|
||||
def test_normalize_explicit_default_wins_over_name(self):
|
||||
"""An explicit model.default is never overridden, and a stale alias is dropped."""
|
||||
from hermes_cli.config import _normalize_root_model_keys
|
||||
|
||||
result = _normalize_root_model_keys(
|
||||
{"model": {"default": "real-model", "name": "ignored"}}
|
||||
)
|
||||
assert result["model"]["default"] == "real-model"
|
||||
assert "name" not in result["model"]
|
||||
|
||||
def test_normalize_explicit_default_wins_over_model(self):
|
||||
from hermes_cli.config import _normalize_root_model_keys
|
||||
|
||||
result = _normalize_root_model_keys(
|
||||
{"model": {"default": "real-model", "model": "ignored"}}
|
||||
)
|
||||
assert result["model"]["default"] == "real-model"
|
||||
assert "model" not in result["model"]
|
||||
|
||||
def test_normalize_model_wins_over_name(self):
|
||||
"""Precedence: model > name when both are aliases and default is empty."""
|
||||
from hermes_cli.config import _normalize_root_model_keys
|
||||
|
||||
result = _normalize_root_model_keys({"model": {"model": "m-key", "name": "n-key"}})
|
||||
assert result["model"]["default"] == "m-key"
|
||||
assert "model" not in result["model"] and "name" not in result["model"]
|
||||
|
||||
def test_normalize_empty_model_dict_stays_empty(self):
|
||||
"""No id key anywhere → default stays empty (no fabricated value)."""
|
||||
from hermes_cli.config import _normalize_root_model_keys
|
||||
|
||||
result = _normalize_root_model_keys({"model": {"provider": "my-litellm"}})
|
||||
assert (result["model"].get("default") or "") == ""
|
||||
|
||||
def test_normalize_model_name_save_roundtrip_migrates_key(self, tmp_path, monkeypatch):
|
||||
"""A model.name config is permanently migrated to model.default on save."""
|
||||
import hermes_cli.config as cfgmod
|
||||
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
cfg_path = home / "config.yaml"
|
||||
cfg_path.write_text("model:\n name: claude-sonnet-4\n provider: my-litellm\n")
|
||||
# bust the mtime cache
|
||||
cfgmod._RAW_CONFIG_CACHE.clear()
|
||||
|
||||
loaded = cfgmod.load_config()
|
||||
assert loaded["model"]["default"] == "claude-sonnet-4"
|
||||
cfgmod.save_config(loaded)
|
||||
|
||||
raw = cfg_path.read_text()
|
||||
assert "name:" not in raw # stale alias gone from the file
|
||||
assert "default: claude-sonnet-4" in raw
|
||||
|
||||
|
||||
class TestProviderResolution:
|
||||
def test_api_key_is_string_or_none(self):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue