mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(tui): /model picker surfaces curated list, matching classic CLI (#12671)
model.options unconditionally overwrote each provider's curated model list with provider_model_ids() (live /models catalog), so TUI users saw non-agentic models that classic CLI /model and `hermes model` filter out via the curated _PROVIDER_MODELS source. On Nous specifically the live endpoint returns ~380 IDs including TTS, embeddings, rerankers, and image/video generators — the TUI picker showed all of them. Classic CLI picker showed the curated 30-model list. Drop the overwrite. list_authenticated_providers() already populates provider['models'] with the curated list (same source as classic CLI at cli.py:4792), sliced to max_models=50. Honor that. Added regression test that fails if the handler ever re-introduces a provider_model_ids() call over the curated list.
This commit is contained in:
parent
d393104bad
commit
ddd28329ff
2 changed files with 82 additions and 9 deletions
|
|
@ -1108,3 +1108,79 @@ def test_session_create_no_race_keeps_worker_alive(monkeypatch):
|
|||
|
||||
# Cleanup
|
||||
server._sessions.pop(sid, None)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# model.options — curated-list parity with `hermes model` and classic /model
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_model_options_does_not_overwrite_curated_models(monkeypatch):
|
||||
"""The TUI model.options handler must surface the same curated model
|
||||
list as `hermes model` and the classic CLI /model picker.
|
||||
|
||||
Regression: earlier versions of this handler unconditionally replaced
|
||||
each provider's curated ``models`` field with ``provider_model_ids()``
|
||||
(live /models catalog). That pulled in hundreds of non-agentic models
|
||||
for providers like Nous whose /models endpoint returns image/video
|
||||
generators, rerankers, embeddings, and TTS models alongside chat models.
|
||||
"""
|
||||
curated_providers = [
|
||||
{
|
||||
"slug": "nous",
|
||||
"name": "Nous",
|
||||
"models": ["moonshotai/kimi-k2.5", "anthropic/claude-opus-4.7"],
|
||||
"total_models": 30,
|
||||
"source": "built-in",
|
||||
"is_current": False,
|
||||
"is_user_defined": False,
|
||||
},
|
||||
]
|
||||
|
||||
monkeypatch.setattr(
|
||||
server,
|
||||
"_load_cfg",
|
||||
lambda: {"providers": {}, "custom_providers": []},
|
||||
)
|
||||
|
||||
with patch(
|
||||
"hermes_cli.model_switch.list_authenticated_providers",
|
||||
return_value=curated_providers,
|
||||
) as listing:
|
||||
# If provider_model_ids gets called at all, the handler is still
|
||||
# overwriting curated with live — that's the regression we're
|
||||
# guarding against.
|
||||
with patch("hermes_cli.models.provider_model_ids") as live_fetch:
|
||||
resp = server._methods["model.options"](99, {"session_id": ""})
|
||||
|
||||
assert "result" in resp, resp
|
||||
providers = resp["result"]["providers"]
|
||||
nous = next((p for p in providers if p.get("slug") == "nous"), None)
|
||||
assert nous is not None
|
||||
assert nous["models"] == [
|
||||
"moonshotai/kimi-k2.5",
|
||||
"anthropic/claude-opus-4.7",
|
||||
]
|
||||
assert nous["total_models"] == 30
|
||||
# Handler must not consult the live catalog — curated is the truth.
|
||||
live_fetch.assert_not_called()
|
||||
# list_authenticated_providers is the single source.
|
||||
assert listing.call_count == 1
|
||||
|
||||
|
||||
def test_model_options_propagates_list_exception(monkeypatch):
|
||||
"""If list_authenticated_providers itself raises, surface as an RPC
|
||||
error rather than swallowing to a blank picker."""
|
||||
monkeypatch.setattr(
|
||||
server,
|
||||
"_load_cfg",
|
||||
lambda: {"providers": {}, "custom_providers": []},
|
||||
)
|
||||
with patch(
|
||||
"hermes_cli.model_switch.list_authenticated_providers",
|
||||
side_effect=RuntimeError("catalog blew up"),
|
||||
):
|
||||
resp = server._methods["model.options"](77, {"session_id": ""})
|
||||
assert "error" in resp
|
||||
assert resp["error"]["code"] == 5033
|
||||
assert "catalog blew up" in resp["error"]["message"]
|
||||
|
|
|
|||
|
|
@ -2499,27 +2499,24 @@ def _(rid, params: dict) -> dict:
|
|||
def _(rid, params: dict) -> dict:
|
||||
try:
|
||||
from hermes_cli.model_switch import list_authenticated_providers
|
||||
from hermes_cli.models import provider_model_ids
|
||||
|
||||
session = _sessions.get(params.get("session_id", ""))
|
||||
agent = session.get("agent") if session else None
|
||||
cfg = _load_cfg()
|
||||
current_provider = getattr(agent, "provider", "") or ""
|
||||
current_model = getattr(agent, "model", "") or _resolve_model()
|
||||
# list_authenticated_providers already populates each provider's
|
||||
# "models" with the curated list (same source as `hermes model` and
|
||||
# classic CLI's /model picker). Do NOT overwrite with live
|
||||
# provider_model_ids() — that bypasses curation and pulls in
|
||||
# non-agentic models (e.g. Nous /models returns ~400 IDs including
|
||||
# TTS, embeddings, rerankers, image/video generators).
|
||||
providers = list_authenticated_providers(
|
||||
current_provider=current_provider,
|
||||
user_providers=cfg.get("providers") if isinstance(cfg.get("providers"), dict) else {},
|
||||
custom_providers=cfg.get("custom_providers") if isinstance(cfg.get("custom_providers"), list) else [],
|
||||
max_models=50,
|
||||
)
|
||||
for provider in providers:
|
||||
try:
|
||||
models = provider_model_ids(provider.get("slug"))
|
||||
if models:
|
||||
provider["models"] = models
|
||||
provider["total_models"] = len(models)
|
||||
except Exception as e:
|
||||
provider["warning"] = f"model catalog unavailable: {e}"
|
||||
return _ok(rid, {"providers": providers, "model": current_model, "provider": current_provider})
|
||||
except Exception as e:
|
||||
return _err(rid, 5033, str(e))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue