diff --git a/CHANGELOG.md b/CHANGELOG.md index df74633..f0a9328 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Hermes Web UI -- Changelog +## [v0.50.135] — 2026-04-22 + +### Fixed +- **BYOK/custom provider models now appear in the WebUI model dropdown** — three root causes fixed. (1) Provider aliases like `z.ai`, `x.ai`, `google`, `grok`, `claude`, `aws-bedrock`, `dashscope`, and ~25 others were not normalized to their internal catalog slugs, causing the provider to miss `_PROVIDER_MODELS` lookup and show an empty dropdown while the TUI worked. (2) The fix works even without `hermes-agent` on `sys.path` (CI, minimal installs) via an inlined `_PROVIDER_ALIASES` table in `api/config.py` — the previous `try/except ImportError` was silently swallowing the failure. (3) `custom_providers` entries now appear in the live model enrichment path. `provider_id` on every group makes optgroup matching deterministic. Closes #815. (#817) + ## [v0.50.134] — 2026-04-21 ### Fixed diff --git a/api/config.py b/api/config.py index 3bf5ba9..ddeef7c 100644 --- a/api/config.py +++ b/api/config.py @@ -512,6 +512,73 @@ _PROVIDER_DISPLAY = { "x-ai": "xAI", } +# Provider alias → canonical slug. Users configure providers using the +# dotted/hyphenated form they see on the provider website (``z.ai``, +# ``x.ai``, ``google``) but the internal catalog (``_PROVIDER_MODELS``) +# uses slugs without punctuation (``zai``, ``xai``, ``gemini``). Without +# normalisation the provider lands in the ``else`` branch of the group +# builder and no models are returned — the bug behind #815. +# +# This table is authoritative for the WebUI. When ``hermes_cli.models`` +# is importable we also merge its ``_PROVIDER_ALIASES`` on top so any +# new aliases added to the agent automatically apply. Keeping the local +# copy means the fix works even in environments where the agent tree is +# not on ``sys.path`` (CI, installs without hermes-agent cloned +# alongside the WebUI). +_PROVIDER_ALIASES = { + "glm": "zai", + "z-ai": "zai", + "z.ai": "zai", + "zhipu": "zai", + "github": "copilot", + "github-copilot": "copilot", + "github-models": "copilot", + "github-model": "copilot", + "google": "gemini", + "google-gemini": "gemini", + "google-ai-studio": "gemini", + "kimi": "kimi-coding", + "moonshot": "kimi-coding", + "claude": "anthropic", + "claude-code": "anthropic", + "deep-seek": "deepseek", + "opencode": "opencode-zen", + "grok": "xai", + "x-ai": "xai", + "x.ai": "xai", + "aws": "bedrock", + "aws-bedrock": "bedrock", + "amazon": "bedrock", + "amazon-bedrock": "bedrock", + "qwen": "alibaba", + "aliyun": "alibaba", + "dashscope": "alibaba", + "alibaba-cloud": "alibaba", +} + + +def _resolve_provider_alias(name: str) -> str: + """Return the canonical provider slug for *name*. + + Applies the WebUI's local alias table first, then merges any + additional aliases the agent provides (when hermes_cli is on + sys.path). Lookup is case-insensitive and whitespace-trimmed. + Unknown names pass through unchanged. + """ + if not name: + return name + raw = str(name).strip().lower() + # Prefer the agent's table when available so new aliases added there + # work automatically; otherwise fall through to our local copy. + try: + from hermes_cli.models import _PROVIDER_ALIASES as _agent_aliases + if raw in _agent_aliases: + return _agent_aliases[raw] + except Exception: + pass + return _PROVIDER_ALIASES.get(raw, name) + + # Well-known models per provider (used to populate dropdown for direct API providers) _PROVIDER_MODELS = { "anthropic": [ @@ -974,6 +1041,13 @@ def get_available_models() -> dict: if cfg_default: default_model = cfg_default + # Normalize active_provider to its canonical key so it matches the + # _PROVIDER_MODELS lookup below (e.g. 'z.ai' -> 'zai', 'x.ai' -> 'xai', + # 'google' -> 'gemini'). Works even when hermes_cli is not on sys.path + # because the WebUI ships its own _PROVIDER_ALIASES table. + if active_provider: + active_provider = _resolve_provider_alias(active_provider) + # 2. Try to read auth store for active provider (if hermes is installed) if not active_provider: try: @@ -1289,7 +1363,7 @@ def get_available_models() -> dict: # Named custom provider — use the stored display name and its own model list _nc_display, _nc_models = _named_custom_groups[pid] if _nc_models: - groups.append({"provider": _nc_display, "models": _nc_models}) + groups.append({"provider": _nc_display, "provider_id": pid, "models": _nc_models}) continue provider_name = _PROVIDER_DISPLAY.get(pid, pid.title()) if pid == "openrouter": @@ -1297,6 +1371,7 @@ def get_available_models() -> dict: groups.append( { "provider": "OpenRouter", + "provider_id": "openrouter", "models": [ {"id": m["id"], "label": m["label"]} for m in _FALLBACK_MODELS @@ -1334,6 +1409,7 @@ def get_available_models() -> dict: groups.append( { "provider": provider_name, + "provider_id": pid, "models": models, } ) @@ -1346,6 +1422,7 @@ def get_available_models() -> dict: groups.append( { "provider": provider_name, + "provider_id": pid, "models": auto_detected_models, } ) @@ -1356,7 +1433,7 @@ def get_available_models() -> dict: if default_model: label = default_model.split("/")[-1] if "/" in default_model else default_model groups.append( - {"provider": "Default", "models": [{"id": default_model, "label": label}]} + {"provider": "Default", "provider_id": "default", "models": [{"id": default_model, "label": label}]} ) # Ensure the user's configured default_model always appears in the dropdown. @@ -1396,6 +1473,7 @@ def get_available_models() -> dict: groups.append( { "provider": "Default", + "provider_id": "default", "models": [{"id": default_model, "label": label}], } ) @@ -1403,6 +1481,7 @@ def get_available_models() -> dict: groups.append( { "provider": active_provider or "Default", + "provider_id": active_provider or "default", "models": [{"id": default_model, "label": label}], } ) diff --git a/api/routes.py b/api/routes.py index 95a46b5..8db9b9b 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2028,6 +2028,14 @@ def _handle_live_models(handler, parsed): if not provider: return j(handler, {"error": "no_provider", "models": []}) + # Normalize provider alias so 'z.ai' -> 'zai', 'x.ai' -> 'xai', etc. + # The browser sends whatever active_provider the static endpoint returned; + # without normalization, provider_model_ids() misses the alias and returns []. + # Uses the WebUI-owned table (api/config._resolve_provider_alias) which + # works even when hermes_cli is not on sys.path. + from api.config import _resolve_provider_alias + provider = _resolve_provider_alias(provider) + # Delegate to the agent's live-fetch + fallback resolver. # provider_model_ids() tries live endpoints first and falls back to # the static _PROVIDER_MODELS list — it never raises. @@ -2048,7 +2056,23 @@ def _handle_live_models(handler, parsed): ids = [m["id"] for m in _pm.get(provider, [])] if not ids: - return j(handler, {"provider": provider, "models": [], "count": 0}) + # For 'custom' provider, provider_model_ids() returns [] because + # 'custom' isn't a real endpoint. Fall back to the custom_providers + # entries from config.yaml so the live-model enrichment step can + # add any models that weren't already in the static list. + if provider == "custom": + try: + _cp_entries = cfg.get("custom_providers", []) + if isinstance(_cp_entries, list): + ids = [ + _cp.get("model", "") + for _cp in _cp_entries + if isinstance(_cp, dict) and _cp.get("model", "") + ] + except Exception: + pass + if not ids: + return j(handler, {"provider": provider, "models": [], "count": 0}) # Normalise to {id, label} — provider_model_ids() returns plain string IDs def _make_label(mid): diff --git a/static/ui.js b/static/ui.js index f407ac5..da0c941 100644 --- a/static/ui.js +++ b/static/ui.js @@ -90,6 +90,7 @@ async function populateModelDropdown(){ for(const g of data.groups){ const og=document.createElement('optgroup'); og.label=g.provider; + if(g.provider_id) og.dataset.provider=g.provider_id; for(const m of g.models){ const opt=document.createElement('option'); opt.value=m.id; @@ -134,6 +135,12 @@ async function _fetchLiveModels(provider, sel){ // Keep other providers' optgroups intact let providerGroup=null; for(const og of sel.querySelectorAll('optgroup')){ + // Prefer exact data-provider match (set from provider_id in API response) + // over substring label match — avoids false positives like 'zai' not matching + // 'Z.AI / GLM' and vice versa. + if(og.dataset.provider&&og.dataset.provider===provider){ + providerGroup=og; break; + } if(og.label&&og.label.toLowerCase().includes(provider.toLowerCase())){ providerGroup=og; break; } diff --git a/tests/test_byok_model_dropdown.py b/tests/test_byok_model_dropdown.py new file mode 100644 index 0000000..882a3e8 --- /dev/null +++ b/tests/test_byok_model_dropdown.py @@ -0,0 +1,378 @@ +"""Tests for #815 — BYOK/custom provider models missing from WebUI model dropdown. + +Root causes fixed: + 1. active_provider alias not normalized in get_available_models() + ('z.ai' -> 'zai', 'x.ai' -> 'xai', 'google' -> 'gemini', etc.) + causing the provider to fall to the 'else/unknown' branch with no models. + + 2. /api/models/live didn't normalize the provider query param, so + provider_model_ids() received the un-aliased form and returned []. + + 3. /api/models/live returned empty for provider='custom' even when + custom_providers entries exist in config.yaml — the live enrichment + step never added those models. +""" +import pathlib +import re +import sys +import unittest.mock as mock + +import pytest + +REPO = pathlib.Path(__file__).parent.parent +sys.path.insert(0, str(REPO)) +sys.path.insert(0, str(REPO.parent / ".hermes" / "hermes-agent")) + + +def read(rel): + return (REPO / rel).read_text(encoding="utf-8") + + +@pytest.fixture(autouse=True) +def _isolate_models_cache(): + """Invalidate the TTL model cache before AND after every test. + + ``get_available_models()`` caches its result keyed on config.yaml mtime. + Tests in this file repoint ``_get_config_path`` to a tmp_path, populate + the cache there, then let monkeypatch restore the original path. The + cache, keyed on the tmp_path's mtime, then poisons downstream tests + (e.g. test_model_resolver) which see stale data and never hit their + mocks. Clearing the cache around each test breaks that linkage. + """ + import api.config as c + try: + c.invalidate_models_cache() + except Exception: + pass + yield + try: + c.invalidate_models_cache() + except Exception: + pass + + +# ── api/config.py — active_provider normalization ───────────────────────────── + +class TestActiveProviderNormalization: + """get_available_models() must normalize active_provider aliases before lookup.""" + + def _run(self, tmp_path, provider_str, monkeypatch): + """Return get_available_models() output for a given provider string.""" + import api.config as c + + cfgfile = tmp_path / "config.yaml" + cfgfile.write_text( + f"model:\n provider: {provider_str}\n default: test-model\n", + encoding="utf-8", + ) + monkeypatch.setattr(c, "_get_config_path", lambda: cfgfile) + c.reload_config() + # Patch list_available_providers to avoid real network calls + fake_prov = mock.MagicMock() + fake_prov.return_value = [] + try: + import hermes_cli.models as hm + monkeypatch.setattr(hm, "list_available_providers", fake_prov) + except Exception: + pass + result = c.get_available_models() + c.reload_config() + return result + + def test_z_dot_ai_normalized_to_zai(self, tmp_path, monkeypatch): + result = self._run(tmp_path, "z.ai", monkeypatch) + # active_provider returned to browser must be canonical 'zai' or + # at minimum must not be 'z.ai' (which would miss the _PROVIDER_MODELS lookup) + ap = result.get("active_provider", "") + assert ap in ("zai", ""), f"active_provider should be 'zai', got {ap!r}" + + def test_x_dot_ai_normalized_to_xai(self, tmp_path, monkeypatch): + result = self._run(tmp_path, "x.ai", monkeypatch) + ap = result.get("active_provider", "") + assert ap in ("xai", ""), f"active_provider should be 'xai', got {ap!r}" + + def test_google_normalized_to_gemini(self, tmp_path, monkeypatch): + result = self._run(tmp_path, "google", monkeypatch) + ap = result.get("active_provider", "") + assert ap in ("gemini", ""), f"active_provider should be 'gemini', got {ap!r}" + + def test_normalization_code_present(self): + """Source-level check: config.py must call _PROVIDER_ALIASES for active_provider.""" + src = read("api/config.py") + # Must alias-normalize active_provider before the group-builder runs + assert "_PROVIDER_ALIASES" in src, ( + "api/config.py must import _PROVIDER_ALIASES to normalize active_provider" + ) + # The normalization must happen before the group builder loop + alias_pos = src.index("_PROVIDER_ALIASES") + group_builder_pos = src.index("for pid in sorted(detected_providers)") + assert alias_pos < group_builder_pos, ( + "active_provider normalization must occur before the group-builder loop" + ) + + +# ── api/routes.py — /api/models/live provider normalization ─────────────────── + +class TestLiveModelsProviderNormalization: + """_handle_live_models must normalize the provider query param.""" + + def test_live_models_normalizes_provider_alias(self): + src = read("api/routes.py") + # Find _handle_live_models function + m = re.search( + r"def _handle_live_models\(.*?\ndef ", + src, + re.DOTALL, + ) + assert m, "_handle_live_models not found" + fn = m.group(0) + assert "_resolve_provider_alias" in fn, ( + "_handle_live_models must normalize provider via " + "api.config._resolve_provider_alias so 'z.ai' -> 'zai' " + "before calling provider_model_ids()" + ) + + def test_live_models_normalization_before_provider_model_ids(self): + """Normalization call must appear before the provider_model_ids call site.""" + src = read("api/routes.py") + alias_match = re.search( + r"provider\s*=\s*_resolve_provider_alias\(provider\)", + src, + ) + pmi_call_match = re.search( + r"ids\s*=\s*_pmi\(provider\)", + src, + ) + assert alias_match, "_resolve_provider_alias call not found in routes.py" + assert pmi_call_match, "ids = _pmi(provider) call not found" + assert alias_match.start() < pmi_call_match.start(), ( + "alias normalization must occur before ids = _pmi(provider)" + ) + + def test_alias_resolver_works_without_hermes_cli(self): + """Normalization must work even when hermes_cli is not importable — + CI and installs without the agent cloned alongside the WebUI. + The WebUI ships its own _PROVIDER_ALIASES table; the agent's table + is merged only when available.""" + import api.config as c + # Core CLI aliases from #815's bug report + assert c._resolve_provider_alias('z.ai') == 'zai' + assert c._resolve_provider_alias('x.ai') == 'xai' + assert c._resolve_provider_alias('google') == 'gemini' + assert c._resolve_provider_alias('grok') == 'xai' + # Case / whitespace insensitive + assert c._resolve_provider_alias(' Z.AI ') == 'zai' + # Canonical names pass through unchanged + assert c._resolve_provider_alias('openrouter') == 'openrouter' + assert c._resolve_provider_alias('anthropic') == 'anthropic' + assert c._resolve_provider_alias('custom') == 'custom' + # Empty / None pass through + assert c._resolve_provider_alias('') == '' + assert c._resolve_provider_alias(None) is None + + +# ── api/routes.py — /api/models/live custom_providers fallback ──────────────── + +class TestLiveModelsCustomProviderFallback: + """When provider='custom' and provider_model_ids() returns [], + /api/models/live must fall back to custom_providers entries from config.yaml.""" + + def test_custom_fallback_code_present(self): + src = read("api/routes.py") + m = re.search( + r"def _handle_live_models\(.*?\ndef ", + src, + re.DOTALL, + ) + assert m, "_handle_live_models not found" + fn = m.group(0) + assert "custom_providers" in fn, ( + "_handle_live_models must read custom_providers from config " + "as fallback when provider='custom' and provider_model_ids() returns []" + ) + assert 'provider == "custom"' in fn or "provider=='custom'" in fn, ( + "_handle_live_models must check provider == 'custom' before fallback" + ) + + def test_custom_fallback_returns_configured_models(self, tmp_path, monkeypatch): + """End-to-end: /api/models/live?provider=custom returns custom_providers models.""" + import api.config as c + import api.routes as r + + cfgfile = tmp_path / "config.yaml" + cfgfile.write_text( + "model:\n provider: custom\n default: my-byok-model\n" + "custom_providers:\n" + " - model: my-byok-model\n" + " api_base: https://my-llm.example.com/v1\n" + " api_key: sk-test\n", + encoding="utf-8", + ) + monkeypatch.setattr(c, "_get_config_path", lambda: cfgfile) + c.reload_config() + + # Mock handler and parsed URL + handler = mock.MagicMock() + responses = [] + def fake_j(h, data, **kw): + responses.append(data) + return True + monkeypatch.setattr(r, "j", fake_j) + + from urllib.parse import urlparse + parsed = mock.MagicMock() + parsed.query = "provider=custom" + + # Mock provider_model_ids to return [] (simulating no live endpoint) + try: + import hermes_cli.models as hm + monkeypatch.setattr(hm, "provider_model_ids", lambda p: []) + except Exception: + pass + + r._handle_live_models(handler, parsed) + + assert responses, "handler must produce a response" + resp = responses[-1] + assert "models" in resp + model_ids = [m["id"] for m in resp.get("models", [])] + assert "my-byok-model" in model_ids, ( + f"custom_providers model 'my-byok-model' must appear in live response; " + f"got {model_ids}" + ) + + +# ── Regression: known-good providers still work ─────────────────────────────── + +class TestKnownProvidersUnaffected: + """Normalization must not break providers whose names are already canonical.""" + + def test_openrouter_unaffected(self): + src = read("api/config.py") + # _PROVIDER_ALIASES lookup: 'openrouter' -> 'openrouter' (no change) + assert "openrouter" in src, "openrouter must still exist in config" + + def test_anthropic_unaffected(self): + src = read("api/config.py") + assert "anthropic" in src + + def test_custom_unaffected(self): + """'custom' is not in _PROVIDER_ALIASES so normalization is a no-op.""" + try: + from hermes_cli.models import _PROVIDER_ALIASES + assert "custom" not in _PROVIDER_ALIASES, ( + "'custom' must not be aliased to anything — it's a special sentinel" + ) + except ImportError: + pass # hermes-agent not available in this env — skip + + +# ── Source-level: active_provider returned to browser is canonical ───────────── + +class TestProviderIdInGroupResponse: + """get_available_models() must include provider_id on every group so the JS + _fetchLiveModels can match optgroups exactly rather than by substring.""" + + def test_groups_include_provider_id(self, tmp_path, monkeypatch): + import api.config as c + + cfgfile = tmp_path / "config.yaml" + cfgfile.write_text( + "model:\n provider: zai\n default: glm-5\n", + encoding="utf-8", + ) + monkeypatch.setattr(c, "_get_config_path", lambda: cfgfile) + c.reload_config() + try: + import hermes_cli.models as hm + monkeypatch.setattr(hm, "list_available_providers", lambda: [ + {"id": "zai", "authenticated": True} + ]) + import hermes_cli.auth as ha + monkeypatch.setattr(ha, "get_auth_status", lambda p: {"key_source": "env"}) + except Exception: + pass + result = c.get_available_models() + c.reload_config() + for g in result.get("groups", []): + assert "provider_id" in g, ( + f"group {g.get('provider')!r} missing provider_id — " + "JS _fetchLiveModels needs it to match optgroups exactly" + ) + + def test_provider_id_in_static_ui_js_optgroup(self): + src = read("static/ui.js") + assert "og.dataset.provider" in src, ( + "populateModelDropdown must set og.dataset.provider from g.provider_id " + "so _fetchLiveModels can match by exact provider_id" + ) + + def test_fetch_live_models_prefers_data_provider_match(self): + src = read("static/ui.js") + m = re.search(r'function _fetchLiveModels\b.*?\n\}', src, re.DOTALL) + assert m, "_fetchLiveModels not found" + fn = m.group(0) + assert 'og.dataset.provider' in fn, ( + "_fetchLiveModels must check og.dataset.provider===provider before " + "falling back to label substring match" + ) + # The data-provider check must come before the label.includes check + dp_pos = fn.index('og.dataset.provider') + label_pos = fn.index('og.label') + assert dp_pos < label_pos, ( + "data-provider exact match must be attempted before label substring match" + ) + + +# ── Opus-identified edge case: 'ollama' normalizes to 'custom' ──────────────── + +class TestOllamaAliasEdgeCase: + """Opus review found: 'ollama' -> 'custom' via _PROVIDER_ALIASES. + This is better behaviour (custom_providers fallback catches it) but worth + documenting and not regressing.""" + + def test_ollama_not_in_provider_aliases_as_ollama(self): + """'ollama' maps to 'custom' in _PROVIDER_ALIASES — verify this is the + intended behavior post-normalization (not a silent breakage).""" + try: + from hermes_cli.models import _PROVIDER_ALIASES + # 'ollama' -> 'custom' means ollama users hit the custom_providers path + # This is fine — ollama models appear via base_url auto-detection (step 3) + # in get_available_models, not via _PROVIDER_MODELS lookup. + ollama_target = _PROVIDER_ALIASES.get("ollama", "ollama") + # Acceptable outcomes: either unchanged (not in aliases) or 'custom'/'ollama-cloud' + assert ollama_target in ("ollama", "custom", "ollama-cloud"), ( + f"Unexpected ollama alias: {ollama_target}" + ) + except ImportError: + pass # hermes-agent not available + + +class TestGetAvailableModelsReturnsCanonicalProvider: + """get_available_models() must return normalized active_provider in its response + so the browser sends the right value to /api/models/live.""" + + def test_active_provider_in_response_is_normalized(self, tmp_path, monkeypatch): + import api.config as c + + cfgfile = tmp_path / "config.yaml" + cfgfile.write_text( + "model:\n provider: z.ai\n default: glm-5\n", + encoding="utf-8", + ) + monkeypatch.setattr(c, "_get_config_path", lambda: cfgfile) + c.reload_config() + try: + import hermes_cli.models as hm + monkeypatch.setattr(hm, "list_available_providers", lambda: []) + except Exception: + pass + result = c.get_available_models() + c.reload_config() + ap = result.get("active_provider", "") + # The browser will pass this value to /api/models/live?provider= + # It must be 'zai' so optgroup matching works in _fetchLiveModels + assert ap != "z.ai", ( + "active_provider 'z.ai' must be normalized to 'zai' before being " + "returned to the browser (browser passes it back to /api/models/live)" + )