mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix: preserve static custom provider models
This commit is contained in:
parent
020a71678c
commit
fc18d15f40
4 changed files with 229 additions and 54 deletions
|
|
@ -4611,13 +4611,29 @@ def _normalize_custom_provider_entry(
|
|||
if isinstance(models, dict) and models:
|
||||
normalized["models"] = models
|
||||
elif isinstance(models, list) and models:
|
||||
# Hand-edited configs (and older Hermes versions) write ``models`` as
|
||||
# a plain list of model ids. Preserve them by converting to the dict
|
||||
# shape downstream code expects; otherwise normalize silently drops
|
||||
# the list and /model shows the provider with (0) models.
|
||||
normalized["models"] = {
|
||||
str(m): {} for m in models if isinstance(m, str) and m.strip()
|
||||
}
|
||||
# Hand-edited configs (and older Hermes versions) may write
|
||||
# ``models`` as a plain list of ids or as ``[{id: ...}]`` rows.
|
||||
# Preserve both by converting to the dict shape downstream code
|
||||
# expects; otherwise normalize silently drops the list and /model
|
||||
# shows the provider with (0) models.
|
||||
normalized_models: Dict[str, Any] = {}
|
||||
for item in models:
|
||||
if isinstance(item, str) and item.strip():
|
||||
normalized_models[item.strip()] = {}
|
||||
continue
|
||||
if not isinstance(item, dict):
|
||||
continue
|
||||
model_id = item.get("id")
|
||||
if not isinstance(model_id, str) or not model_id.strip():
|
||||
model_id = item.get("name")
|
||||
if not isinstance(model_id, str) or not model_id.strip():
|
||||
continue
|
||||
model_meta = {
|
||||
k: v for k, v in item.items() if k not in {"id", "name"}
|
||||
}
|
||||
normalized_models[model_id.strip()] = model_meta
|
||||
if normalized_models:
|
||||
normalized["models"] = normalized_models
|
||||
|
||||
context_length = entry.get("context_length")
|
||||
if isinstance(context_length, int) and context_length > 0:
|
||||
|
|
|
|||
|
|
@ -52,6 +52,54 @@ _UNCAPPED_PICKER_PROVIDERS: frozenset[str] = frozenset({"opencode-zen", "opencod
|
|||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _declared_model_ids(value: Any) -> list[str]:
|
||||
"""Return configured model IDs from supported config shapes.
|
||||
|
||||
Accepts:
|
||||
- ``{"model-id": {...}}``
|
||||
- ``["model-a", "model-b"]``
|
||||
- ``[{"id": "model-a"}, {"name": "model-b"}]``
|
||||
- ``"model-a"``
|
||||
"""
|
||||
ids: list[str] = []
|
||||
seen: set[str] = set()
|
||||
|
||||
def _add(candidate: Any) -> None:
|
||||
if not isinstance(candidate, str):
|
||||
return
|
||||
model_id = candidate.strip()
|
||||
if not model_id:
|
||||
return
|
||||
lowered = model_id.lower()
|
||||
if lowered in seen:
|
||||
return
|
||||
seen.add(lowered)
|
||||
ids.append(model_id)
|
||||
|
||||
if isinstance(value, str):
|
||||
_add(value)
|
||||
return ids
|
||||
|
||||
if isinstance(value, dict):
|
||||
for model_id in value:
|
||||
_add(model_id)
|
||||
return ids
|
||||
|
||||
if isinstance(value, (list, tuple)):
|
||||
for item in value:
|
||||
if isinstance(item, str):
|
||||
_add(item)
|
||||
continue
|
||||
if isinstance(item, dict):
|
||||
model_id = item.get("id")
|
||||
if not isinstance(model_id, str) or not model_id.strip():
|
||||
model_id = item.get("name")
|
||||
_add(model_id)
|
||||
return ids
|
||||
|
||||
return ids
|
||||
|
||||
|
||||
def _bare_custom_provider_def(current_base_url: str) -> Optional[ProviderDef]:
|
||||
"""ProviderDef for a direct ``model.provider: custom`` endpoint."""
|
||||
base_url = str(current_base_url or "").strip()
|
||||
|
|
@ -700,22 +748,9 @@ def _configured_provider_matches(
|
|||
def _match(value) -> Optional[str]:
|
||||
"""Canonical id if ``value`` (a model collection or scalar) declares
|
||||
``target``, else None."""
|
||||
if isinstance(value, str):
|
||||
return value if value.strip().lower() == target else None
|
||||
if isinstance(value, dict):
|
||||
for mid in value:
|
||||
if isinstance(mid, str) and mid.strip().lower() == target:
|
||||
return mid
|
||||
return None
|
||||
if isinstance(value, (list, tuple)):
|
||||
for item in value:
|
||||
if isinstance(item, str) and item.strip().lower() == target:
|
||||
return item
|
||||
if isinstance(item, dict):
|
||||
name = item.get("name")
|
||||
if isinstance(name, str) and name.strip().lower() == target:
|
||||
return name
|
||||
return None
|
||||
for model_id in _declared_model_ids(value):
|
||||
if model_id.lower() == target:
|
||||
return model_id
|
||||
return None
|
||||
|
||||
matches: dict[str, str] = {}
|
||||
|
|
@ -1243,16 +1278,9 @@ def switch_model(
|
|||
# user_providers is a dict: {provider_slug: config_dict}
|
||||
for slug, cfg in user_providers.items():
|
||||
if slug == target_provider:
|
||||
cfg_models = cfg.get("models", {})
|
||||
# Direct membership works for dict (keys) and list (strings)
|
||||
if new_model in cfg_models:
|
||||
if new_model in _declared_model_ids(cfg.get("models", {})):
|
||||
override = True
|
||||
break
|
||||
# Also accept if models is a list of dicts with 'name' field
|
||||
if isinstance(cfg_models, list):
|
||||
if any(m.get("name") == new_model for m in cfg_models if isinstance(m, dict)):
|
||||
override = True
|
||||
break
|
||||
# Also check custom_providers list — models declared there should be accepted
|
||||
# even if the remote /v1/models endpoint doesn't list them.
|
||||
if not override and custom_providers and isinstance(custom_providers, list):
|
||||
|
|
@ -1270,7 +1298,7 @@ def switch_model(
|
|||
if new_model == entry_model:
|
||||
override = True
|
||||
break
|
||||
if isinstance(entry_models, dict) and new_model in entry_models:
|
||||
if new_model in _declared_model_ids(entry_models):
|
||||
override = True
|
||||
break
|
||||
if override:
|
||||
|
|
@ -1961,18 +1989,11 @@ def list_authenticated_providers(
|
|||
if default_model:
|
||||
models_list.append(default_model)
|
||||
# Also include the full models list from config.
|
||||
# Hermes writes ``models:`` as a dict keyed by model id
|
||||
# (see hermes_cli/main.py::_save_custom_provider); older
|
||||
# configs or hand-edited files may still use a list.
|
||||
cfg_models = ep_cfg.get("models", [])
|
||||
if isinstance(cfg_models, dict):
|
||||
for m in cfg_models:
|
||||
if m and m not in models_list:
|
||||
models_list.append(m)
|
||||
elif isinstance(cfg_models, list):
|
||||
for m in cfg_models:
|
||||
if m and m not in models_list:
|
||||
models_list.append(m)
|
||||
# Hermes writes ``models:`` as a dict keyed by model id, but older
|
||||
# or hand-edited configs may use strings or ``[{id: ...}]`` rows.
|
||||
for model_id in _declared_model_ids(ep_cfg.get("models", [])):
|
||||
if model_id not in models_list:
|
||||
models_list.append(model_id)
|
||||
|
||||
# Official OpenAI API rows in providers: often have base_url but no
|
||||
# explicit models: dict — avoid a misleading zero count in /model.
|
||||
|
|
@ -2177,15 +2198,9 @@ def list_authenticated_providers(
|
|||
if default_model and default_model not in groups[group_key]["models"]:
|
||||
groups[group_key]["models"].append(default_model)
|
||||
|
||||
cfg_models = entry.get("models", {})
|
||||
if isinstance(cfg_models, dict):
|
||||
for m in cfg_models:
|
||||
if m and m not in groups[group_key]["models"]:
|
||||
groups[group_key]["models"].append(m)
|
||||
elif isinstance(cfg_models, list):
|
||||
for m in cfg_models:
|
||||
if m and m not in groups[group_key]["models"]:
|
||||
groups[group_key]["models"].append(m)
|
||||
for model_id in _declared_model_ids(entry.get("models", {})):
|
||||
if model_id not in groups[group_key]["models"]:
|
||||
groups[group_key]["models"].append(model_id)
|
||||
|
||||
_section4_emitted_slugs: set = set()
|
||||
_current_base_url_norm = str(current_base_url or "").strip().rstrip("/").lower()
|
||||
|
|
|
|||
|
|
@ -62,6 +62,32 @@ def test_load_picker_context_full_dict():
|
|||
assert isinstance(ctx.custom_providers, list)
|
||||
|
||||
|
||||
def test_load_picker_context_normalizes_list_of_dict_models():
|
||||
cfg = _cfg(
|
||||
providers={
|
||||
"static-gateway": {
|
||||
"name": "Static Gateway",
|
||||
"api": "https://router.example.com/v1",
|
||||
"default_model": "claude-3-7-sonnet",
|
||||
"models": [
|
||||
{"id": "claude-3-7-sonnet"},
|
||||
{"id": "claude-sonnet-4", "context_length": 200000},
|
||||
],
|
||||
"discover_models": False,
|
||||
}
|
||||
},
|
||||
)
|
||||
with patch("hermes_cli.config.load_config", return_value=cfg):
|
||||
ctx = load_picker_context()
|
||||
|
||||
assert len(ctx.custom_providers) == 1
|
||||
assert ctx.custom_providers[0]["models"] == {
|
||||
"claude-3-7-sonnet": {},
|
||||
"claude-sonnet-4": {"context_length": 200000},
|
||||
}
|
||||
assert ctx.custom_providers[0]["discover_models"] is False
|
||||
|
||||
|
||||
def test_load_picker_context_falls_back_to_name_when_default_missing():
|
||||
cfg = _cfg(model={"name": "gpt-5.4", "provider": "openai"})
|
||||
with patch("hermes_cli.config.load_config", return_value=cfg):
|
||||
|
|
@ -716,6 +742,52 @@ def test_two_custom_providers_with_overlap_both_survive():
|
|||
assert b_row["total_models"] == 2
|
||||
|
||||
|
||||
def test_build_models_payload_keeps_static_provider_models_from_providers_dict():
|
||||
"""The inventory payload must keep configured static models from a
|
||||
``providers:`` entry even when the same endpoint also appears via the
|
||||
compatibility ``custom_providers`` view and live discovery would fail."""
|
||||
cfg = _cfg(
|
||||
model={
|
||||
"provider": "static-gateway",
|
||||
"default": "claude-3-7-sonnet",
|
||||
},
|
||||
providers={
|
||||
"static-gateway": {
|
||||
"name": "Static Gateway",
|
||||
"api": "https://router.example.com/v1",
|
||||
"api_key": "sk-test",
|
||||
"default_model": "claude-3-7-sonnet",
|
||||
"models": [
|
||||
{"id": "claude-3-7-sonnet"},
|
||||
{"id": "claude-sonnet-4"},
|
||||
],
|
||||
"discover_models": False,
|
||||
}
|
||||
},
|
||||
)
|
||||
with (
|
||||
patch("hermes_cli.config.load_config", return_value=cfg),
|
||||
patch("agent.models_dev.fetch_models_dev", return_value={}),
|
||||
patch("hermes_cli.providers.HERMES_OVERLAYS", {}),
|
||||
patch(
|
||||
"hermes_cli.models.fetch_api_models",
|
||||
side_effect=AssertionError("fetch_api_models must not be called"),
|
||||
),
|
||||
):
|
||||
ctx = load_picker_context()
|
||||
payload = build_models_payload(ctx)
|
||||
|
||||
rows = [
|
||||
row
|
||||
for row in payload["providers"]
|
||||
if row.get("api_url") == "https://router.example.com/v1"
|
||||
]
|
||||
assert len(rows) == 1
|
||||
assert rows[0]["slug"] == "static-gateway"
|
||||
assert rows[0]["models"] == ["claude-3-7-sonnet", "claude-sonnet-4"]
|
||||
assert rows[0]["total_models"] == 2
|
||||
|
||||
|
||||
def test_build_models_payload_no_max_models_returns_full_list():
|
||||
"""When max_models is not passed (None), build_models_payload must
|
||||
return the full model list — not truncate to the old default of 50.
|
||||
|
|
@ -779,4 +851,3 @@ def test_list_authenticated_providers_refresh_busts_cache():
|
|||
assert clear.call_count == 0
|
||||
model_switch.list_authenticated_providers(refresh=True)
|
||||
assert clear.call_count == 1
|
||||
|
||||
|
|
|
|||
|
|
@ -927,6 +927,79 @@ def test_custom_providers_discover_models_false_string_is_normalised(monkeypatch
|
|||
assert gateway_prov["models"] == ["only-model"]
|
||||
|
||||
|
||||
def test_custom_providers_discover_models_false_list_of_dict_ids(monkeypatch):
|
||||
"""List-of-dicts ``models: [{id: ...}]`` must be preserved as configured
|
||||
model IDs when discovery is disabled."""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
|
||||
|
||||
calls = []
|
||||
|
||||
def fake_fetch_api_models(api_key, base_url, **kwargs):
|
||||
calls.append((api_key, base_url, kwargs))
|
||||
return ["live-a", "live-b"]
|
||||
|
||||
monkeypatch.setattr("hermes_cli.models.fetch_api_models", fake_fetch_api_models)
|
||||
|
||||
custom_providers = [
|
||||
{
|
||||
"name": "static-gateway",
|
||||
"api_key": "***",
|
||||
"base_url": "https://router.example.com/v1",
|
||||
"discover_models": False,
|
||||
"model": "claude-3-7-sonnet",
|
||||
"models": [
|
||||
{"id": "claude-3-7-sonnet"},
|
||||
{"id": "claude-sonnet-4"},
|
||||
],
|
||||
}
|
||||
]
|
||||
|
||||
providers = list_authenticated_providers(
|
||||
current_provider="openrouter",
|
||||
current_base_url="https://openrouter.ai/api/v1",
|
||||
custom_providers=custom_providers,
|
||||
max_models=50,
|
||||
)
|
||||
|
||||
gateway_prov = next(
|
||||
(p for p in providers if p.get("api_url") == "https://router.example.com/v1"),
|
||||
None,
|
||||
)
|
||||
|
||||
assert gateway_prov is not None
|
||||
assert calls == [], "discover_models: false must skip live discovery"
|
||||
assert gateway_prov["models"] == ["claude-3-7-sonnet", "claude-sonnet-4"]
|
||||
assert gateway_prov["total_models"] == 2
|
||||
|
||||
|
||||
def test_list_of_dict_models_prefers_id_over_label(monkeypatch):
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
|
||||
|
||||
providers = list_authenticated_providers(
|
||||
current_provider="openrouter",
|
||||
current_base_url="https://openrouter.ai/api/v1",
|
||||
custom_providers=[
|
||||
{
|
||||
"name": "static-gateway",
|
||||
"base_url": "https://router.example.com/v1",
|
||||
"discover_models": False,
|
||||
"models": [{"id": "real-model-id", "name": "Friendly Label"}],
|
||||
}
|
||||
],
|
||||
max_models=50,
|
||||
)
|
||||
|
||||
gateway_prov = next(
|
||||
(p for p in providers if p.get("api_url") == "https://router.example.com/v1"),
|
||||
None,
|
||||
)
|
||||
|
||||
assert gateway_prov is not None
|
||||
assert gateway_prov["models"] == ["real-model-id"]
|
||||
|
||||
|
||||
def test_resolve_custom_provider_passes_key_env():
|
||||
"""resolve_custom_provider should propagate key_env into api_key_env_vars.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue