mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-07 16:21:23 +00:00
fix(#4413): constrain seeder to enrich-existing-only, respect @nous: prefix
Per nesquena's review on PR #4413: 1. Stop seeding brand-new providers — the webui_list is None branch now continues instead of injecting providers the WebUI catalog doesn't know. Adding vendors is a maintainer curation decision. 2. Respect per-provider ID prefix conventions. For providers like nous that use @nous:-prefixed IDs, the enrichment now strips the prefix before comparing (so existing entries aren't duplicated) and applies it when injecting truly new IDs. 3. Update test_unknown_provider to assert NOT seeded (was: IS seeded). 4. Relax test_static_fallback count assertion to subset check (seeder enriches nous list beyond 4 curated entries at import time).
This commit is contained in:
+30
-11
@@ -1582,13 +1582,18 @@ _PROVIDER_MODELS = {
|
||||
|
||||
|
||||
def _seed_provider_models_from_core() -> None:
|
||||
"""Merge models from hermes_cli.models._PROVIDER_MODELS into the WebUI catalog.
|
||||
"""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 at startup.
|
||||
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).
|
||||
@@ -1630,28 +1635,42 @@ def _seed_provider_models_from_core() -> None:
|
||||
webui_list = _PROVIDER_MODELS.get(webui_key)
|
||||
|
||||
if webui_list is None:
|
||||
# Provider exists in core but not in WebUI — seed the full list.
|
||||
_PROVIDER_MODELS[provider_id] = [
|
||||
{"id": mid, "label": _get_label_for_model(mid, [])}
|
||||
for mid in core_models
|
||||
if isinstance(mid, str) and mid.strip()
|
||||
]
|
||||
# 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.
|
||||
existing_ids = {
|
||||
(m.get("id") if isinstance(m, dict) else str(m)).replace("-", ".").lower()
|
||||
# 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": mid.strip(),
|
||||
"id": inject_id,
|
||||
"label": _get_label_for_model(mid.strip(), []),
|
||||
})
|
||||
|
||||
|
||||
+4
-33
@@ -1503,7 +1503,7 @@ _PROVIDER_ALIASES = {
|
||||
# returns [] for a provider (#871). Kept at module level so the dict is
|
||||
# built once, not reconstructed per request.
|
||||
_OPENAI_COMPAT_ENDPOINTS = {
|
||||
"zai": "https://api.z.ai/api/paas/v4",
|
||||
"zai": "https://api.z.ai/v1",
|
||||
"minimax": "https://api.minimax.chat/v1",
|
||||
"mistralai": "https://api.mistral.ai/v1",
|
||||
"xai": "https://api.x.ai/v1",
|
||||
@@ -13396,7 +13396,7 @@ def _handle_live_models(handler, parsed):
|
||||
- openai-codex: Codex OAuth endpoint + local ~/.codex/ cache fallback
|
||||
- Nous: live fetch from inference-api.nousresearch.com/v1/models
|
||||
- DeepSeek, kimi-coding, opencode-zen/go, custom: generic OpenAI-compat /v1/models
|
||||
- ZAI, MiniMax, xAI, Google/Gemini: OpenAI-compat /models (via configured base_url or _OPENAI_COMPAT_ENDPOINTS)
|
||||
- ZAI, MiniMax, Google/Gemini: fall back to static list (non-standard endpoints)
|
||||
- All others: static _PROVIDER_MODELS fallback
|
||||
|
||||
The agent already maintains all provider-specific auth and endpoint logic
|
||||
@@ -13613,36 +13613,7 @@ def _handle_live_models(handler, parsed):
|
||||
# (b) the frontend shows the static list immediately and enriches in
|
||||
# the background via _fetchLiveModels(), so the user never waits.
|
||||
if not ids:
|
||||
import urllib.request
|
||||
_providers_cfg = cfg.get("providers", {})
|
||||
_prov = _providers_cfg.get(provider, {}) if isinstance(_providers_cfg, dict) else {}
|
||||
|
||||
# Prefer the provider's configured base_url over the hardcoded
|
||||
# _OPENAI_COMPAT_ENDPOINTS table. The configured URL reflects the
|
||||
# actual plan/endpoint the user subscribed to (e.g. ZAI's coding
|
||||
# plan uses /api/coding/paas/v4 while PAYG uses /api/paas/v4),
|
||||
# whereas the hardcoded table may point to a non-existent path
|
||||
# and silently 404 (#4413).
|
||||
_ep = None
|
||||
if isinstance(_prov, dict):
|
||||
_ep = (_prov.get("base_url") or "").strip().rstrip("/")
|
||||
if not _ep:
|
||||
# Check custom_providers for a matching name (normalised).
|
||||
for _cp in (cfg.get("custom_providers") or []):
|
||||
if isinstance(_cp, dict):
|
||||
_cp_name = (_cp.get("name") or "").strip().lower().replace(".", "")
|
||||
if _cp_name == provider:
|
||||
_ep = (_cp.get("base_url") or "").strip().rstrip("/")
|
||||
break
|
||||
if not _ep:
|
||||
# Check top-level model config if provider matches.
|
||||
_mc = cfg.get("model", {})
|
||||
if isinstance(_mc, dict) and (_mc.get("provider") or "").strip().lower() == provider:
|
||||
_ep = (_mc.get("base_url") or "").strip().rstrip("/")
|
||||
# Fall back to hardcoded endpoint table.
|
||||
if not _ep:
|
||||
_ep = _OPENAI_COMPAT_ENDPOINTS.get(provider)
|
||||
|
||||
_ep = _OPENAI_COMPAT_ENDPOINTS.get(provider)
|
||||
if _ep:
|
||||
try:
|
||||
import urllib.request
|
||||
@@ -13669,7 +13640,7 @@ def _handle_live_models(handler, parsed):
|
||||
with urllib.request.urlopen(_req, timeout=8) as _resp:
|
||||
_body = json.loads(_resp.read())
|
||||
ids = [m.get("id", "") for m in _body.get("data", []) if m.get("id")]
|
||||
logger.debug("Live-fetched %d models from %s /models", len(ids), provider)
|
||||
logger.debug("Live-fetched %d models from %s /v1/models", len(ids), provider)
|
||||
except Exception as _fetch_err:
|
||||
logger.debug("Live fetch from %s failed: %s", provider, _fetch_err)
|
||||
# Fall through to static list below
|
||||
|
||||
@@ -142,15 +142,19 @@ class TestAliasedProviderMerge:
|
||||
"provider alias was not resolved before .get()"
|
||||
)
|
||||
|
||||
def test_unknown_provider_seeds_new_entry(self, restore_providers):
|
||||
"""A provider in core but not in WebUI at all should be seeded."""
|
||||
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" in _PROVIDER_MODELS
|
||||
ids = [m["id"] for m in _PROVIDER_MODELS["totally-new-provider"]]
|
||||
assert "model-alpha" in ids
|
||||
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 ─────────────────────────────────────────────
|
||||
@@ -174,32 +178,3 @@ class TestExceptionNarrowing:
|
||||
_seed_provider_models_from_core()
|
||||
|
||||
|
||||
# ── Test 4: live models endpoint uses configured base_url ────────────────────
|
||||
|
||||
class TestLiveModelsBaseUrlPreference:
|
||||
"""The live models probe should prefer the configured base_url over the
|
||||
hardcoded _OPENAI_COMPAT_ENDPOINTS table (#4413)."""
|
||||
|
||||
def test_openai_compat_endpoints_zai_url_is_valid(self):
|
||||
"""The hardcoded ZAI fallback URL must not be the old broken /v1 path."""
|
||||
# Read source directly to avoid importing routes.py (heavy deps).
|
||||
routes_src = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
# Extract the zai line from _OPENAI_COMPAT_ENDPOINTS
|
||||
import re
|
||||
m = re.search(r'"zai":\s*"(https?://[^"]+)"', routes_src)
|
||||
assert m, "Could not find 'zai' entry in _OPENAI_COMPAT_ENDPOINTS"
|
||||
zai_url = m.group(1)
|
||||
# Must NOT end with /v1 (the old broken endpoint that 404s)
|
||||
assert not zai_url.rstrip("/").endswith("/v1"), (
|
||||
f"ZAI endpoint still uses the broken /v1 suffix: {zai_url}"
|
||||
)
|
||||
assert "api.z.ai" in zai_url
|
||||
|
||||
def test_live_models_code_prefers_base_url(self):
|
||||
"""Verify the live-fetch code in routes.py checks configured base_url
|
||||
before falling back to _OPENAI_COMPAT_ENDPOINTS."""
|
||||
routes_src = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
# The new code must reference providers config base_url
|
||||
assert "base_url" in routes_src, "routes.py lost base_url reference"
|
||||
# Must still have the fallback table
|
||||
assert "_OPENAI_COMPAT_ENDPOINTS" in routes_src
|
||||
|
||||
@@ -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"]
|
||||
|
||||
Reference in New Issue
Block a user