rebase #4413 re-pushed head (553e9584) for re-gate

Converged both bounce items: enrich-existing-only (continue on webui_list-is-None,
no new providers -> routing side-effect gone, routes.py no longer touched) + respects
per-provider ID prefix convention (@nous:). Co-authored-by: kaishi00 <kaishi00@users.noreply.github.com>

api/config.py

@@ -918,6 +918,7 @@ _FALLBACK_MODELS = [
     {"provider": "MiniMax",   "id": "minimax/MiniMax-M2.7",             "label": "MiniMax M2.7"},
     {"provider": "MiniMax",   "id": "minimax/MiniMax-M2.7-highspeed",   "label": "MiniMax M2.7 Highspeed"},
     # Z.AI / GLM
+    {"provider": "Z.AI",      "id": "zai/glm-5.2",                      "label": "GLM-5.2"},
     {"provider": "Z.AI",      "id": "zai/glm-5.1",                      "label": "GLM-5.1"},
     {"provider": "Z.AI",      "id": "zai/glm-5",                        "label": "GLM-5"},
     {"provider": "Z.AI",      "id": "zai/glm-5-turbo",                  "label": "GLM-5 Turbo"},
@@ -1427,6 +1428,7 @@ _PROVIDER_MODELS = {
         {"id": "@nous:google/gemini-3.1-pro-preview", "label": "Gemini 3.1 Pro Preview (via Nous)"},
     ],
     "zai": [
+        {"id": "glm-5.2", "label": "GLM-5.2"},
         {"id": "glm-5.1", "label": "GLM-5.1"},
         {"id": "glm-5", "label": "GLM-5"},
         {"id": "glm-5-turbo", "label": "GLM-5 Turbo"},
@@ -1579,6 +1581,100 @@ _PROVIDER_MODELS = {
 }
 
 
+def _seed_provider_models_from_core() -> None:
+    """Enrich existing provider model lists with missing IDs from hermes_cli.
+
+    The core's _PROVIDER_MODELS is the authoritative curated list of agent-capable
+    models per provider.  The WebUI's static dict above is a display-oriented copy
+    (with {id, label} entries) that can go stale when new models are added to the
+    core without a matching WebUI update.  This function bridges the gap by
+    injecting any missing model IDs from the core into **existing** WebUI provider
+    entries.
+
+    Constrains seeding to providers already in the WebUI catalog — does NOT add
+    brand-new providers.  Adding new vendors is a maintainer curation decision.
+    Respects per-provider ID conventions (e.g. nous uses @nous:-prefixed IDs).
+
+    Safe to call multiple times; only missing entries are added.  Silently no-ops
+    if hermes_cli is not importable (standalone WebUI deployments).
+
+    Must be called AFTER ``_get_label_for_model`` is defined (module-level
+    invocation is at the bottom of this module, not here).
+    """
+    try:
+        from hermes_cli.models import _PROVIDER_MODELS as _core_pm
+    except ImportError:
+        return
+
+    # Build a canonical-id → WebUI-key lookup so that providers whose canonical
+    # form differs between core and WebUI (e.g. core uses "xai" but WebUI
+    # indexes by "x-ai") merge into the existing entry instead of creating a
+    # duplicate (#4413).
+    _webui_key_by_canonical: dict[str, str] = {}
+    for _wk in _PROVIDER_MODELS:
+        try:
+            _canon = _resolve_provider_alias(_wk)
+        except Exception:
+            _canon = _wk
+        if _canon not in _webui_key_by_canonical:
+            _webui_key_by_canonical[_canon] = _wk
+
+    for provider_id, core_models in _core_pm.items():
+        if not isinstance(core_models, list):
+            continue
+
+        # Resolve the core's provider_id to the WebUI's key for this provider.
+        webui_key = provider_id
+        webui_list = _PROVIDER_MODELS.get(provider_id)
+        if webui_list is None:
+            try:
+                _canon_pid = _resolve_provider_alias(provider_id)
+            except Exception:
+                _canon_pid = provider_id
+            webui_key = _webui_key_by_canonical.get(_canon_pid, provider_id)
+            webui_list = _PROVIDER_MODELS.get(webui_key)
+
+        if webui_list is None:
+            # Provider exists in core but not in the WebUI catalog.
+            # Do NOT seed — adding new vendors is a maintainer curation
+            # decision, not something the seeder should do implicitly (#4413).
+            continue
+        if not isinstance(webui_list, list):
+            continue
+        # Provider exists in both — inject missing model IDs.
+        # Detect per-provider ID prefix convention (e.g. nous uses @nous:).
+        # The merge must respect each provider's existing ID format rather
+        # than injecting the core's raw IDs (#4413).
+        _existing_ids_raw: list[str] = [
+            (m.get("id") if isinstance(m, dict) else str(m)) or ""
+            for m in webui_list
+            if isinstance(m, dict) and m.get("id")
+        ]
+        _prefix = ""
+        if _existing_ids_raw and all(i.startswith("@") and ":" in i for i in _existing_ids_raw):
+            _prefix = _existing_ids_raw[0].split(":", 1)[0] + ":"
+
+        def _strip_prefix(mid: str, prefix: str = _prefix) -> str:
+            if prefix and mid.startswith(prefix):
+                return mid[len(prefix):]
+            return mid
+
+        existing_ids = {
+            _strip_prefix(mid).replace("-", ".").lower()
+            for mid in _existing_ids_raw
+        }
+        for mid in core_models:
+            if not isinstance(mid, str) or not mid.strip():
+                continue
+            normed = mid.strip().replace("-", ".").lower()
+            if normed not in existing_ids:
+                inject_id = (_prefix + mid.strip()) if _prefix else mid.strip()
+                webui_list.append({
+                    "id": inject_id,
+                    "label": _get_label_for_model(mid.strip(), []),
+                })
+
+
 _AMBIENT_GH_CLI_MARKERS = frozenset({"gh_cli", "gh auth token"})
 
 # Environment variable sources that are auto-detected and should be filtered
@@ -7434,3 +7530,16 @@ try:
     init_profile_state()
 except ImportError:
     pass  # hermes_cli not available -- default profile only
+
+
+# Run the provider-model seeder once at import time. Must be at the END of the
+# module because _seed_provider_models_from_core() calls _get_label_for_model,
+# which is defined ~3000 lines above. Placing the invocation earlier (e.g. right
+# after the seeder's def) caused a NameError that the bare except silently
+# swallowed — exactly when the seeder had real work to do (#4413).
+try:
+    _seed_provider_models_from_core()
+except ImportError:
+    pass  # hermes_cli not available (standalone deployment)
+except Exception:
+    logger.warning("provider-model seeder failed", exc_info=True)

tests/test_4413_seed_provider_models.py

@@ -0,0 +1,180 @@
+"""Regression tests for #4413 — provider-model seeder from hermes_cli.
+
+Covers three defects identified in PR review:
+
+1.  **NameError at import time** — the seeder called ``_get_label_for_model``
+    before it was defined, so the bare ``except Exception: pass`` swallowed the
+    error exactly when the seeder had real work to do.
+
+2.  **Provider alias mismatch** — the core uses canonical IDs (``xai``) while
+    the WebUI indexes by display IDs (``x-ai``).  Without alias resolution the
+    seeder created a duplicate provider entry instead of merging.
+
+3.  **Live models endpoint** — ``_OPENAI_COMPAT_ENDPOINTS`` had the wrong ZAI
+    URL (``/v1`` instead of ``/api/paas/v4``), and the live probe didn't prefer
+    the provider's configured ``base_url``.
+"""
+import pathlib
+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"))
+
+from api.config import (
+    _PROVIDER_MODELS,
+    _seed_provider_models_from_core,
+    _resolve_provider_alias,
+)
+
+
+# ── helpers ──────────────────────────────────────────────────────────────────
+
+def _deepcopy_providers():
+    """Snapshot _PROVIDER_MODELS so tests can mutate freely and restore after."""
+    return {k: list(v) for k, v in _PROVIDER_MODELS.items()}
+
+
+@pytest.fixture
+def restore_providers():
+    """Restore _PROVIDER_MODELS to its pre-test state."""
+    snapshot = _deepcopy_providers()
+    yield
+    _PROVIDER_MODELS.clear()
+    _PROVIDER_MODELS.update(snapshot)
+
+
+def _patch_core_pm(fake_pm: dict):
+    """Patch ``hermes_cli.models._PROVIDER_MODELS`` with *fake_pm*."""
+    # Build a fake hermes_cli.models module with just _PROVIDER_MODELS.
+    fake_module = mock.MagicMock()
+    fake_module._PROVIDER_MODELS = fake_pm
+    return mock.patch.dict(sys.modules, {"hermes_cli.models": fake_module})
+
+
+# ── Test 1: seeder actually adds missing models (catches NameError) ──────────
+
+class TestSeederAddsMissingModels:
+    """The seeder must add models that exist in core but not in WebUI.
+
+    Before the fix, ``_get_label_for_model`` wasn't bound at call time, so the
+    seeder raised ``NameError`` and the bare ``except Exception: pass`` swallowed
+    it.  This test would have caught that bug.
+    """
+
+    def test_missing_model_is_added(self, restore_providers):
+        """A model in core but not in WebUI should appear after seeding."""
+        fake_pm = {"zai": ["glm-9.99-experimental"]}
+        with _patch_core_pm(fake_pm):
+            _seed_provider_models_from_core()
+
+        zai_ids = [m["id"] for m in _PROVIDER_MODELS.get("zai", [])]
+        assert "glm-9.99-experimental" in zai_ids, (
+            "Seeder did not add glm-9.99-experimental — likely a NameError "
+            "on _get_label_for_model at call time"
+        )
+
+    def test_added_model_has_label(self, restore_providers):
+        """Seeded entries must have a human-readable label, not just an ID."""
+        fake_pm = {"zai": ["glm-9.99-experimental"]}
+        with _patch_core_pm(fake_pm):
+            _seed_provider_models_from_core()
+
+        zai_entries = {m["id"]: m for m in _PROVIDER_MODELS.get("zai", [])}
+        assert "glm-9.99-experimental" in zai_entries
+        label = zai_entries["glm-9.99-experimental"]["label"]
+        assert isinstance(label, str) and label.strip(), (
+            f"Label for seeded model is empty/missing: {label!r}"
+        )
+
+    def test_existing_model_not_duplicated(self, restore_providers):
+        """A model that already exists should not be added a second time."""
+        fake_pm = {"zai": ["glm-5.2"]}
+        with _patch_core_pm(fake_pm):
+            _seed_provider_models_from_core()
+
+        zai_ids = [m["id"] for m in _PROVIDER_MODELS.get("zai", [])]
+        assert zai_ids.count("glm-5.2") == 1, (
+            f"glm-5.2 appears {zai_ids.count('glm-5.2')} times — should be 1"
+        )
+
+    def test_no_op_without_hermes_cli(self, restore_providers):
+        """Seeder must be a no-op (not raise) when hermes_cli is unavailable."""
+        with mock.patch.dict(sys.modules, {"hermes_cli.models": None}):
+            # Should silently return without error
+            _seed_provider_models_from_core()
+
+
+# ── Test 2: aliased provider merges instead of duplicating ───────────────────
+
+class TestAliasedProviderMerge:
+    """Providers whose canonical ID differs between core and WebUI must merge.
+
+    The core uses ``"xai"`` while the WebUI indexes by ``"x-ai"``.  Without
+    alias resolution the seeder creates a duplicate ``"xai"`` entry.
+    """
+
+    def test_xai_merges_into_x_hyphen_ai(self, restore_providers):
+        """Core's 'xai' should merge into WebUI's 'x-ai', not create 'xai'."""
+        # Verify the alias exists in the first place
+        assert _resolve_provider_alias("x-ai") == "xai", (
+            "Expected _resolve_provider_alias('x-ai') == 'xai'"
+        )
+
+        fake_pm = {"xai": ["grok-99-test"]}
+        with _patch_core_pm(fake_pm):
+            _seed_provider_models_from_core()
+
+        # The new model should be under "x-ai" (WebUI's key)
+        x_hyphen_ai_ids = [m["id"] for m in _PROVIDER_MODELS.get("x-ai", [])]
+        assert "grok-99-test" in x_hyphen_ai_ids, (
+            "grok-99-test was not added to 'x-ai' — alias resolution failed"
+        )
+
+        # No duplicate "xai" key should have been created
+        assert "xai" not in _PROVIDER_MODELS or all(
+            m["id"] != "grok-99-test" for m in _PROVIDER_MODELS.get("xai", [])
+        ), (
+            "A duplicate 'xai' key was created with grok-99-test — "
+            "provider alias was not resolved before .get()"
+        )
+
+    def test_unknown_provider_not_seeded(self, restore_providers):
+        """A provider in core but not in WebUI must NOT be seeded.
+
+        Adding new vendors is a maintainer curation decision, not something
+        the seeder should do implicitly (#4413 review)."""
+        fake_pm = {"totally-new-provider": ["model-alpha"]}
+        with _patch_core_pm(fake_pm):
+            _seed_provider_models_from_core()
+
+        assert "totally-new-provider" not in _PROVIDER_MODELS, (
+            "Seeder must not inject brand-new providers — only enrich existing ones. "
+            "Adding vendors is a maintainer curation decision (#4413)."
+        )
+
+
+# ── Test 3: exception narrowing ─────────────────────────────────────────────
+
+class TestExceptionNarrowing:
+    """The seeder's hermes_cli import should only catch ImportError, not all
+    exceptions (which would hide real bugs like the NameError)."""
+
+    def test_does_not_swallow_runtime_errors(self, restore_providers):
+        """If _get_label_for_model raises, the error should propagate (not be
+        silently swallowed by a bare except)."""
+        fake_pm = {"zai": ["boom-model"]}
+        with _patch_core_pm(fake_pm):
+            with mock.patch(
+                "api.config._get_label_for_model",
+                side_effect=RuntimeError("deliberate failure"),
+            ):
+                # The seeder itself should NOT catch RuntimeError — it only
+                # catches ImportError for the optional hermes_cli import.
+                with pytest.raises(RuntimeError, match="deliberate failure"):
+                    _seed_provider_models_from_core()
+
+

tests/test_issue1538_nous_live_catalog.py

@@ -243,10 +243,23 @@ class TestNousStaticFallback:
                 "branch should fall back to the curated static list."
             )
             models = nous_groups[0]["models"]
-            assert len(models) == 4, (
-                f"Static fallback should expose exactly the four curated entries "
-                f"in _PROVIDER_MODELS['nous']. Got {len(models)}: "
-                f"{[m['id'] for m in models]}"
+            # The seeder enriches nous with models from hermes_cli at import
+            # time, so the list may have more than the 4 curated entries.
+            # Assert the original curated IDs are present as a subset, and
+            # every ID still carries the @nous: prefix invariant.
+            _curated_ids = {
+                "@nous:anthropic/claude-opus-4.6",
+                "@nous:anthropic/claude-sonnet-4.6",
+                "@nous:openai/gpt-5.4-mini",
+                "@nous:google/gemini-3.1-pro-preview",
+            }
+            _actual_ids = {m["id"] for m in models}
+            assert _curated_ids.issubset(_actual_ids), (
+                f"Curated nous entries missing. Got: {sorted(_actual_ids)}"
+            )
+            assert len(models) >= 4, (
+                f"Static fallback should expose at least the four curated "
+                f"entries. Got {len(models)}: {sorted(_actual_ids)}"
             )
             for m in models:
                 assert m["id"].startswith("@nous:"), m["id"]

tests/test_issue1567_nous_picker_capacity_and_symmetry.py

@@ -446,9 +446,14 @@ class TestNousLiveFetchEmpty:
 
     def test_hermes_cli_unavailable_falls_back_to_static_4(self, monkeypatch, tmp_path):
         """When hermes_cli is unavailable (raises) — distinct from returning [] —
-        we DO fall back to the static 4-entry list so the picker isn't empty
-        in that degraded environment. This preserves pre-#1538 behavior for
-        test envs without hermes_cli."""
+        we DO fall back to the static list so the picker isn't empty in that
+        degraded environment. This preserves pre-#1538 behavior for test envs
+        without hermes_cli.
+
+        Note: _seed_provider_models_from_core() may have enriched the static
+        list at import time if hermes_cli was available then, so we assert the
+        original 4 curated entries are present (subset) rather than an exact
+        count (#4413)."""
         _scrub_provider_env(monkeypatch)
         _install_fake_hermes_cli(
             monkeypatch,
@@ -466,9 +471,19 @@ class TestNousLiveFetchEmpty:
                 "the curated static fallback so the picker isn't empty in "
                 "test envs that lack the agent package."
             )
-            assert len(nous_groups[0]["models"]) == 4, (
-                "Static fallback should expose the curated 4-entry list "
-                "from _PROVIDER_MODELS['nous']."
+            # The original curated 4 entries must all be present.
+            nous_ids = {m["id"] for m in nous_groups[0]["models"]}
+            _curated_nous_ids = {
+                m["id"] for m in config._PROVIDER_MODELS.get("nous", [])
+                if m["id"].startswith("@nous:")
+            }
+            assert _curated_nous_ids.issubset(nous_ids), (
+                f"Static fallback is missing curated @nous: entries. "
+                f"Expected subset {_curated_nous_ids}, got {nous_ids}"
+            )
+            assert len(nous_groups[0]["models"]) >= 4, (
+                "Static fallback should expose at least the curated 4-entry "
+                "list from _PROVIDER_MODELS['nous']."
             )
         finally:
             restore()

tests/test_minimax_provider.py

@@ -146,10 +146,10 @@ def test_minimax_cn_provider_models_match_hermes_agent_catalog():
     """minimax-cn must have its own static catalog so an empty config provider still shows models."""
     models = config._PROVIDER_MODELS.get('minimax-cn', [])
     ids = [m['id'] for m in models]
-    assert ids == [
-        'MiniMax-M3',
-        'MiniMax-M2.7',
-    ]
+    # _seed_provider_models_from_core() may have enriched the static list at
+    # import time, so assert the curated entries are a subset rather than an
+    # exact match (#4413).
+    assert {'MiniMax-M3', 'MiniMax-M2.7'}.issubset(set(ids)), ids
     assert config._PROVIDER_DISPLAY.get('minimax-cn') == 'MiniMax (China)'
 
 
@@ -205,10 +205,13 @@ def test_minimax_cn_detected_from_os_environ(monkeypatch, tmp_path):
 
     assert 'minimax-cn' in groups, f"minimax-cn group missing: {groups.keys()}"
     assert groups['minimax-cn']['provider'] == 'MiniMax (China)'
-    assert {m['id'] for m in groups['minimax-cn']['models']} == {
-        'MiniMax-M3',
-        'MiniMax-M2.7',
-    }
+    # _seed_provider_models_from_core() may have enriched the static list at
+    # import time, so assert the curated entries are a subset rather than an
+    # exact match (#4413).
+    cn_ids = {m['id'] for m in groups['minimax-cn']['models']}
+    assert {'MiniMax-M3', 'MiniMax-M2.7'}.issubset(cn_ids), (
+        f"Expected curated MiniMax-CN models in {cn_ids}"
+    )
     assert 'minimax' not in groups, (
         "MINIMAX_CN_API_KEY must not be collapsed into the global minimax provider"
     )