From 60235dba5e8d1a2a518131a53de63bba37f41830 Mon Sep 17 00:00:00 2001 From: Traemond Anderson Date: Tue, 21 Apr 2026 10:25:10 -0400 Subject: [PATCH] feat(cli): add list_picker_providers for credential-filtered picker The Telegram/Discord /model pickers currently call list_authenticated_providers(), which returns every provider whose credentials resolve locally and every model in its curated snapshot. Two failure modes fall out: - OpenRouter rows can include IDs the live catalog no longer carries. - Provider rows can surface with zero callable models (e.g. a slug whose credential pool entry exists but has nothing behind it). list_picker_providers() wraps the base function and post-processes the result so the interactive picker only shows models the user can actually select: - OpenRouter's models come from fetch_openrouter_models() (live-catalog filtered against the curated OPENROUTER_MODELS snapshot). - Rows with an empty models list are dropped, except custom endpoints (is_user_defined=True with an api_url) where the user may enter model ids manually. - All other fields pass through unchanged. The gateway /model handler switches to the new helper for the interactive picker payload only. Typed /model and the text fallback list stay on list_authenticated_providers() so nothing is hidden from power users or platforms without a picker. Covered by nine focused unit tests in tests/hermes_cli/test_list_picker_providers.py. Co-Authored-By: Claude Opus 4.7 (1M context) --- gateway/run.py | 3 +- hermes_cli/model_switch.py | 56 +++++ .../hermes_cli/test_list_picker_providers.py | 216 ++++++++++++++++++ 3 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 tests/hermes_cli/test_list_picker_providers.py diff --git a/gateway/run.py b/gateway/run.py index c0a6ae0bb1..ed3bd47b96 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -7578,6 +7578,7 @@ class GatewayRunner: from hermes_cli.model_switch import ( switch_model as _switch_model, parse_model_flags, list_authenticated_providers, + list_picker_providers, ) from hermes_cli.providers import get_label @@ -7632,7 +7633,7 @@ class GatewayRunner: if has_picker: try: - providers = list_authenticated_providers( + providers = list_picker_providers( current_provider=current_provider, current_base_url=current_base_url, current_model=current_model, diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 8c2dc2eaed..ec3ca6aed2 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -1683,3 +1683,59 @@ def list_authenticated_providers( results.sort(key=lambda r: (not r["is_current"], -r["total_models"])) return results + + +def list_picker_providers( + current_provider: str = "", + user_providers: dict = None, + custom_providers: list | None = None, + max_models: int = 8, +) -> List[dict]: + """Interactive-picker variant of :func:`list_authenticated_providers`. + + Post-processes the base list so the ``/model`` picker (Telegram/Discord + inline keyboards) only surfaces models that are actually callable in the + current install: + + - OpenRouter's model list is replaced with the output of + :func:`hermes_cli.models.fetch_openrouter_models`, which filters the + curated ``OPENROUTER_MODELS`` snapshot against the live OpenRouter + catalog. IDs the live catalog no longer carries drop out, so the + picker never offers a model the user can't call. + - Provider rows whose model list ends up empty are dropped, except + custom endpoints (``is_user_defined=True`` with an ``api_url``) where + the user may supply their own model set through config. + + All other providers and metadata fields are passed through unchanged. + The typed ``/model `` path is unaffected -- only the interactive + picker payload is narrowed. + """ + from hermes_cli.models import fetch_openrouter_models + + providers = list_authenticated_providers( + current_provider=current_provider, + user_providers=user_providers, + custom_providers=custom_providers, + max_models=max_models, + ) + + filtered: List[dict] = [] + for p in providers: + slug = str(p.get("slug", "")).lower() + if slug == "openrouter": + try: + live = fetch_openrouter_models() + live_ids = [mid for mid, _ in live] + except Exception: + live_ids = list(p.get("models", [])) + p = dict(p) + p["models"] = live_ids[:max_models] + p["total_models"] = len(live_ids) + + has_models = bool(p.get("models")) + is_custom_endpoint = bool(p.get("is_user_defined")) and bool(p.get("api_url")) + if not has_models and not is_custom_endpoint: + continue + filtered.append(p) + + return filtered diff --git a/tests/hermes_cli/test_list_picker_providers.py b/tests/hermes_cli/test_list_picker_providers.py new file mode 100644 index 0000000000..e424a104fd --- /dev/null +++ b/tests/hermes_cli/test_list_picker_providers.py @@ -0,0 +1,216 @@ +"""Tests for ``list_picker_providers`` — the /model picker filter. + +``list_picker_providers`` wraps ``list_authenticated_providers`` and +post-processes the result for interactive pickers (Telegram, Discord): + +- OpenRouter's ``models`` are replaced with the live-filtered output of + ``fetch_openrouter_models``, so IDs the live catalog no longer carries + drop out. +- Provider rows with an empty ``models`` list are dropped, except custom + endpoints (``is_user_defined=True`` with an ``api_url``) where the user + may supply their own model set through config. + +These tests exercise the filter in isolation by mocking +``list_authenticated_providers`` and ``fetch_openrouter_models`` so no +network or auth state is required. +""" + +import pytest +from hermes_cli import model_switch + + +def _make_provider(slug, name=None, models=None, *, is_current=False, + is_user_defined=False, source="built-in", api_url=None): + """Build a dict shaped like ``list_authenticated_providers`` output.""" + entry = { + "slug": slug, + "name": name or slug.title(), + "is_current": is_current, + "is_user_defined": is_user_defined, + "models": list(models or []), + "total_models": len(models or []), + "source": source, + } + if api_url is not None: + entry["api_url"] = api_url + return entry + + +def test_openrouter_models_replaced_with_live_catalog(monkeypatch): + """OpenRouter row's ``models`` should come from fetch_openrouter_models.""" + base = [ + _make_provider("openrouter", models=["openai/gpt-stale", "old/model"]), + ] + live = [("openai/gpt-5.4", "recommended"), ("moonshotai/kimi-k2.6", "")] + + monkeypatch.setattr(model_switch, "list_authenticated_providers", + lambda **kw: list(base)) + monkeypatch.setattr("hermes_cli.models.fetch_openrouter_models", + lambda *a, **kw: list(live)) + + result = model_switch.list_picker_providers(max_models=50) + + assert len(result) == 1 + openrouter = result[0] + assert openrouter["slug"] == "openrouter" + assert openrouter["models"] == ["openai/gpt-5.4", "moonshotai/kimi-k2.6"] + assert openrouter["total_models"] == 2 + + +def test_openrouter_falls_back_to_base_models_on_fetch_failure(monkeypatch): + """If the live catalog fetch raises, keep whatever base provided.""" + fallback_models = ["openai/gpt-5.4", "moonshotai/kimi-k2.6"] + base = [_make_provider("openrouter", models=fallback_models)] + + def _raise(*_a, **_kw): + raise RuntimeError("network down") + + monkeypatch.setattr(model_switch, "list_authenticated_providers", + lambda **kw: list(base)) + monkeypatch.setattr("hermes_cli.models.fetch_openrouter_models", _raise) + + result = model_switch.list_picker_providers(max_models=50) + + assert len(result) == 1 + assert result[0]["models"] == fallback_models + + +def test_openrouter_empty_live_catalog_drops_row(monkeypatch): + """If the live catalog returns nothing for OpenRouter, drop the row.""" + base = [_make_provider("openrouter", models=["something/stale"])] + + monkeypatch.setattr(model_switch, "list_authenticated_providers", + lambda **kw: list(base)) + monkeypatch.setattr("hermes_cli.models.fetch_openrouter_models", + lambda *a, **kw: []) + + result = model_switch.list_picker_providers(max_models=50) + + assert result == [] + + +def test_non_openrouter_rows_passed_through_unchanged(monkeypatch): + """Non-OpenRouter providers keep their curated ``models`` as-is.""" + base = [ + _make_provider("anthropic", models=["claude-sonnet-4-6", "claude-opus-4-7"]), + _make_provider("gemini", models=["gemini-3-flash-preview"]), + ] + + monkeypatch.setattr(model_switch, "list_authenticated_providers", + lambda **kw: list(base)) + # fetch_openrouter_models must not be consulted when there's no openrouter row + monkeypatch.setattr("hermes_cli.models.fetch_openrouter_models", + lambda *a, **kw: pytest.fail("should not be called")) + + result = model_switch.list_picker_providers(max_models=50) + + assert [p["slug"] for p in result] == ["anthropic", "gemini"] + assert result[0]["models"] == ["claude-sonnet-4-6", "claude-opus-4-7"] + assert result[1]["models"] == ["gemini-3-flash-preview"] + + +def test_empty_models_row_dropped(monkeypatch): + """Built-in provider with an empty ``models`` list is dropped.""" + base = [ + _make_provider("anthropic", models=[]), # drop + _make_provider("openrouter", models=["anything"]), # replaced by live + ] + + monkeypatch.setattr(model_switch, "list_authenticated_providers", + lambda **kw: list(base)) + monkeypatch.setattr("hermes_cli.models.fetch_openrouter_models", + lambda *a, **kw: [("openai/gpt-5.4", "recommended")]) + + result = model_switch.list_picker_providers(max_models=50) + + assert [p["slug"] for p in result] == ["openrouter"] + + +def test_custom_endpoint_with_api_url_kept_when_models_empty(monkeypatch): + """User-defined endpoints with an ``api_url`` survive even if models empty. + + Rationale: custom endpoints may accept any model id the user types -- + the picker still shows the row so the user can enter one manually. + """ + base = [ + _make_provider("local-ollama", is_user_defined=True, + api_url="http://localhost:11434/v1", models=[], + source="user-config"), + ] + + monkeypatch.setattr(model_switch, "list_authenticated_providers", + lambda **kw: list(base)) + monkeypatch.setattr("hermes_cli.models.fetch_openrouter_models", + lambda *a, **kw: []) + + result = model_switch.list_picker_providers(max_models=50) + + assert len(result) == 1 + assert result[0]["slug"] == "local-ollama" + assert result[0]["models"] == [] + + +def test_user_defined_without_api_url_and_empty_models_dropped(monkeypatch): + """An is_user_defined row WITHOUT api_url and no models is still dropped. + + The exemption is specifically for custom endpoints that can accept + arbitrary model ids; without an api_url there's nothing to point at. + """ + base = [ + _make_provider("orphan", is_user_defined=True, api_url=None, models=[]), + ] + + monkeypatch.setattr(model_switch, "list_authenticated_providers", + lambda **kw: list(base)) + monkeypatch.setattr("hermes_cli.models.fetch_openrouter_models", + lambda *a, **kw: []) + + result = model_switch.list_picker_providers(max_models=50) + + assert result == [] + + +def test_max_models_caps_openrouter_live_output(monkeypatch): + """``max_models`` caps how many OpenRouter IDs land in the row.""" + live = [(f"vendor/model-{i}", "") for i in range(20)] + base = [_make_provider("openrouter", models=["placeholder"])] + + monkeypatch.setattr(model_switch, "list_authenticated_providers", + lambda **kw: list(base)) + monkeypatch.setattr("hermes_cli.models.fetch_openrouter_models", + lambda *a, **kw: list(live)) + + result = model_switch.list_picker_providers(max_models=5) + + assert len(result) == 1 + assert len(result[0]["models"]) == 5 + assert result[0]["models"] == [mid for mid, _ in live[:5]] + # total_models reflects the full live catalog, not the capped slice. + assert result[0]["total_models"] == 20 + + +def test_passthrough_kwargs_to_base(monkeypatch): + """All kwargs (current_provider, user_providers, custom_providers, max_models) + must be forwarded to ``list_authenticated_providers`` unchanged. + """ + captured = {} + + def _capture(**kwargs): + captured.update(kwargs) + return [] + + monkeypatch.setattr(model_switch, "list_authenticated_providers", _capture) + monkeypatch.setattr("hermes_cli.models.fetch_openrouter_models", + lambda *a, **kw: []) + + model_switch.list_picker_providers( + current_provider="openrouter", + user_providers={"foo": {"api": "http://x"}}, + custom_providers=[{"name": "bar", "base_url": "http://y"}], + max_models=12, + ) + + assert captured["current_provider"] == "openrouter" + assert captured["user_providers"] == {"foo": {"api": "http://x"}} + assert captured["custom_providers"] == [{"name": "bar", "base_url": "http://y"}] + assert captured["max_models"] == 12