mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(model_switch): enumerate dict-format models in /model picker
list_authenticated_providers() builds /model picker rows for CLI, TUI and gateway flows, but fails to enumerate custom provider models stored in dict form: - custom_providers[] entries surface only the singular `model:` field, hiding every other model in the `models:` dict. - providers: dict entries with dict-format `models:` are silently dropped and render as `(0 models)`. Hermes's own writer (main.py::_save_custom_provider) persists configured models as a dict keyed by model id, and most downstream readers (agent/models_dev.py, gateway/run.py, run_agent.py, hermes_cli/config.py) already consume that dict format. The /model picker was the only stale path. Add a dict branch in both sections of list_authenticated_providers(), preferring dict (canonical) and keeping the list branch as fallback for hand-edited / legacy configs. Dedup against the already-added default model so nothing duplicates when the default is also a dict key. Six new regression tests in tests/hermes_cli/ cover: dict models with a default, dict models without a default, and default dedup against a matching dict key. Fixes #11677 Fixes #9148 Related: #11017
This commit is contained in:
parent
13294c2d18
commit
bca03eab20
3 changed files with 234 additions and 2 deletions
|
|
@ -1047,9 +1047,16 @@ def list_authenticated_providers(
|
|||
models_list = []
|
||||
if default_model:
|
||||
models_list.append(default_model)
|
||||
# Also include the full models list from config
|
||||
# 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, list):
|
||||
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)
|
||||
|
|
@ -1100,10 +1107,27 @@ def list_authenticated_providers(
|
|||
"api_url": api_url,
|
||||
"models": [],
|
||||
}
|
||||
# The singular ``model:`` field only holds the currently
|
||||
# active model. Hermes's own writer (main.py::_save_custom_provider)
|
||||
# stores every configured model as a dict under ``models:``;
|
||||
# downstream readers (agent/models_dev.py, gateway/run.py,
|
||||
# run_agent.py, hermes_cli/config.py) already consume that dict.
|
||||
# The /model picker previously ignored it, so multi-model
|
||||
# custom providers appeared to have only the active model.
|
||||
default_model = (entry.get("model") or "").strip()
|
||||
if default_model and default_model not in groups[slug]["models"]:
|
||||
groups[slug]["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[slug]["models"]:
|
||||
groups[slug]["models"].append(m)
|
||||
elif isinstance(cfg_models, list):
|
||||
for m in cfg_models:
|
||||
if m and m not in groups[slug]["models"]:
|
||||
groups[slug]["models"].append(m)
|
||||
|
||||
for slug, grp in groups.items():
|
||||
if slug.lower() in seen_slugs:
|
||||
continue
|
||||
|
|
|
|||
|
|
@ -156,3 +156,100 @@ def test_list_deduplicates_same_model_in_group(monkeypatch):
|
|||
assert len(my_rows) == 1
|
||||
assert my_rows[0]["models"] == ["llama3", "mistral"]
|
||||
assert my_rows[0]["total_models"] == 2
|
||||
|
||||
|
||||
def test_list_enumerates_dict_format_models_alongside_default(monkeypatch):
|
||||
"""custom_providers entry with dict-format ``models:`` plus singular
|
||||
``model:`` should surface the default and every dict key.
|
||||
|
||||
Regression: Hermes's own writer stores configured models as a dict
|
||||
keyed by model id, but the /model picker previously only honored the
|
||||
singular ``model:`` field, so multi-model custom providers appeared
|
||||
to have only the active model.
|
||||
"""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr(providers_mod, "HERMES_OVERLAYS", {})
|
||||
|
||||
providers = list_authenticated_providers(
|
||||
current_provider="openai-codex",
|
||||
user_providers={},
|
||||
custom_providers=[
|
||||
{
|
||||
"name": "DeepSeek",
|
||||
"base_url": "https://api.deepseek.com",
|
||||
"api_mode": "chat_completions",
|
||||
"model": "deepseek-chat",
|
||||
"models": {
|
||||
"deepseek-chat": {"context_length": 128000},
|
||||
"deepseek-reasoner": {"context_length": 128000},
|
||||
},
|
||||
}
|
||||
],
|
||||
max_models=50,
|
||||
)
|
||||
|
||||
ds_rows = [p for p in providers if p["name"] == "DeepSeek"]
|
||||
assert len(ds_rows) == 1
|
||||
assert ds_rows[0]["models"] == ["deepseek-chat", "deepseek-reasoner"]
|
||||
assert ds_rows[0]["total_models"] == 2
|
||||
|
||||
|
||||
def test_list_enumerates_dict_format_models_without_singular_model(monkeypatch):
|
||||
"""Dict-format ``models:`` with no singular ``model:`` should still
|
||||
enumerate every dict key (previously the picker reported 0 models)."""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr(providers_mod, "HERMES_OVERLAYS", {})
|
||||
|
||||
providers = list_authenticated_providers(
|
||||
current_provider="openai-codex",
|
||||
user_providers={},
|
||||
custom_providers=[
|
||||
{
|
||||
"name": "Thor",
|
||||
"base_url": "http://thor.lab:8337/v1",
|
||||
"models": {
|
||||
"gemma-4-26B-A4B-it-MXFP4_MOE": {"context_length": 262144},
|
||||
"Qwen3.5-35B-A3B-MXFP4_MOE": {"context_length": 262144},
|
||||
"gemma-4-31B-it-Q4_K_M": {"context_length": 262144},
|
||||
},
|
||||
}
|
||||
],
|
||||
max_models=50,
|
||||
)
|
||||
|
||||
thor_rows = [p for p in providers if p["name"] == "Thor"]
|
||||
assert len(thor_rows) == 1
|
||||
assert set(thor_rows[0]["models"]) == {
|
||||
"gemma-4-26B-A4B-it-MXFP4_MOE",
|
||||
"Qwen3.5-35B-A3B-MXFP4_MOE",
|
||||
"gemma-4-31B-it-Q4_K_M",
|
||||
}
|
||||
assert thor_rows[0]["total_models"] == 3
|
||||
|
||||
|
||||
def test_list_dedupes_dict_model_matching_singular_default(monkeypatch):
|
||||
"""When the singular ``model:`` is also a key in the ``models:`` dict,
|
||||
it must appear exactly once in the picker."""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr(providers_mod, "HERMES_OVERLAYS", {})
|
||||
|
||||
providers = list_authenticated_providers(
|
||||
current_provider="openai-codex",
|
||||
user_providers={},
|
||||
custom_providers=[
|
||||
{
|
||||
"name": "DeepSeek",
|
||||
"base_url": "https://api.deepseek.com",
|
||||
"model": "deepseek-chat",
|
||||
"models": {
|
||||
"deepseek-chat": {"context_length": 128000},
|
||||
"deepseek-reasoner": {"context_length": 128000},
|
||||
},
|
||||
}
|
||||
],
|
||||
max_models=50,
|
||||
)
|
||||
|
||||
ds_rows = [p for p in providers if p["name"] == "DeepSeek"]
|
||||
assert ds_rows[0]["models"].count("deepseek-chat") == 1
|
||||
assert ds_rows[0]["models"] == ["deepseek-chat", "deepseek-reasoner"]
|
||||
|
|
|
|||
|
|
@ -86,6 +86,117 @@ def test_list_authenticated_providers_dedupes_models_when_default_in_list(monkey
|
|||
assert user_prov["models"].count("model-a") == 1, "model-a should not be duplicated"
|
||||
|
||||
|
||||
def test_list_authenticated_providers_enumerates_dict_format_models(monkeypatch):
|
||||
"""providers: dict entries with ``models:`` as a dict keyed by model id
|
||||
(canonical Hermes write format) should surface every key in the picker.
|
||||
|
||||
Regression: the ``providers:`` dict path previously only accepted
|
||||
list-format ``models:`` and silently dropped dict-format entries,
|
||||
even though Hermes's own writer and downstream readers use dict format.
|
||||
"""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
|
||||
|
||||
user_providers = {
|
||||
"local-ollama": {
|
||||
"name": "Local Ollama",
|
||||
"api": "http://localhost:11434/v1",
|
||||
"default_model": "minimax-m2.7:cloud",
|
||||
"models": {
|
||||
"minimax-m2.7:cloud": {"context_length": 196608},
|
||||
"kimi-k2.5:cloud": {"context_length": 200000},
|
||||
"glm-5.1:cloud": {"context_length": 202752},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
providers = list_authenticated_providers(
|
||||
current_provider="local-ollama",
|
||||
user_providers=user_providers,
|
||||
custom_providers=[],
|
||||
max_models=50,
|
||||
)
|
||||
|
||||
user_prov = next(
|
||||
(p for p in providers if p.get("is_user_defined") and p["slug"] == "local-ollama"),
|
||||
None,
|
||||
)
|
||||
|
||||
assert user_prov is not None
|
||||
assert user_prov["total_models"] == 3
|
||||
assert user_prov["models"] == [
|
||||
"minimax-m2.7:cloud",
|
||||
"kimi-k2.5:cloud",
|
||||
"glm-5.1:cloud",
|
||||
]
|
||||
|
||||
|
||||
def test_list_authenticated_providers_dict_models_without_default_model(monkeypatch):
|
||||
"""Dict-format ``models:`` without a ``default_model`` must still expose
|
||||
every dict key, not collapse to an empty list."""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
|
||||
|
||||
user_providers = {
|
||||
"multimodel": {
|
||||
"api": "http://example.com/v1",
|
||||
"models": {
|
||||
"alpha": {"context_length": 8192},
|
||||
"beta": {"context_length": 16384},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
providers = list_authenticated_providers(
|
||||
current_provider="",
|
||||
user_providers=user_providers,
|
||||
custom_providers=[],
|
||||
)
|
||||
|
||||
user_prov = next(
|
||||
(p for p in providers if p.get("is_user_defined") and p["slug"] == "multimodel"),
|
||||
None,
|
||||
)
|
||||
|
||||
assert user_prov is not None
|
||||
assert user_prov["total_models"] == 2
|
||||
assert set(user_prov["models"]) == {"alpha", "beta"}
|
||||
|
||||
|
||||
def test_list_authenticated_providers_dict_models_dedupe_with_default(monkeypatch):
|
||||
"""When ``default_model`` is also a key in the ``models:`` dict, it must
|
||||
appear exactly once (list already had this for list-format models)."""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
|
||||
|
||||
user_providers = {
|
||||
"my-provider": {
|
||||
"api": "http://example.com/v1",
|
||||
"default_model": "model-a",
|
||||
"models": {
|
||||
"model-a": {"context_length": 8192},
|
||||
"model-b": {"context_length": 16384},
|
||||
"model-c": {"context_length": 32768},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
providers = list_authenticated_providers(
|
||||
current_provider="my-provider",
|
||||
user_providers=user_providers,
|
||||
custom_providers=[],
|
||||
)
|
||||
|
||||
user_prov = next(
|
||||
(p for p in providers if p.get("is_user_defined")),
|
||||
None,
|
||||
)
|
||||
|
||||
assert user_prov is not None
|
||||
assert user_prov["total_models"] == 3
|
||||
assert user_prov["models"].count("model-a") == 1
|
||||
|
||||
|
||||
def test_list_authenticated_providers_fallback_to_default_only(monkeypatch):
|
||||
"""When no models array is provided, should fall back to default_model."""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue