mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-26 03:30:36 +00:00
458cf38ac9
Reporter (Deor, Discord #report-bugs, May 03 2026 14:19 PT, relayed by @AvidFuturist) saw the Settings → Default Model dropdown rendering the OpenCode Go provider as TWO separate optgroups: "OpenCode Go" (the canonical one with all 14 catalog models) and "Opencode_Go" (a phantom group containing one self-referential entry). Three structural causes, all in api/config.py:_build_available_models_uncached: 1. **Detection-path id leakage.** The detection block at line ~1980 reads cfg["providers"] keys verbatim. If the user's config has ``providers.opencode_go.api_key`` (underscore variant) AND another path adds the canonical ``opencode-go`` (e.g. via active_provider), both end up in detected_providers and the build loop creates two distinct provider groups with the second labelled via the ``pid.title()`` fallback as ``"Opencode_Go"``. 2. **Injection-block rogue model.** The default-model injection block at line ~2598 puts ANY ``model.default`` string into the picker as a fake option. A stray ``model.default: opencode_go`` (provider id mistakenly used as a model id) surfaces as a phantom model labelled ``"Opencode GO"``. 3. **Empty-group bleed.** When a non-canonical provider id makes it into detected_providers but has no entry in _PROVIDER_MODELS, the build loop creates an optgroup with zero models — pure UI noise. This PR addresses all three: - **New `_canonicalise_provider_id()` helper** that folds underscores to hyphens, lowercases, and applies alias resolution only when the alias target is itself a canonical id in `_PROVIDER_DISPLAY`. The last constraint avoids round-tripping ``x-ai`` (canonical) through the alias table to ``xai`` (which the WebUI doesn't index by). - **Detection-path canonicalisation.** The cfg["providers"] scan applies the helper before adding to detected_providers. Same treatment in the only_show_configured intersection so that mode doesn't accidentally exclude the canonical id when configured_providers only contains the underscore-variant key. - **Post-collection dedup pass** that re-canonicalises every entry in detected_providers — belt-and-braces against future regressions in any of the ~25 ``detected_providers.add(...)`` callsites without auditing each one. Idempotent for already-canonical ids. - **Provider-id guard on the model.default injection block.** When the injected value matches a known provider display name or alias (after underscore/case normalisation), skip the injection and emit a `logger.warning` instead. Real unknown model ids (newly released models, custom endpoints) still get injected — only provider-shaped values are rejected. - **Empty-group filter at end of build.** Drop optgroups with zero models. Custom: groups (`provider_id` starts with `custom:`) are exempt — users may want an empty card visible as a reminder. Tests ----- `tests/test_issue1568_duplicate_provider_groups.py` (17 tests): - TestCanonicaliseProviderId (8): unit tests pinning helper behaviour — canonical preserved, underscore folded, case folded, aliases resolved, x-ai not round-tripped, empty input, unknown ids normalised, idempotence - TestProviderGroupDedup (4): end-to-end picker behaviour — underscored providers-key produces ONE group not two (Deor's case), uppercase providers-key collapsed, aliased keys (z-ai → zai) collapsed, happy path unchanged - TestDefaultModelProviderIdGuard (3): provider id as model.default doesn't inject phantom + WARNING logged; alias as model.default also caught; legitimate unknown model IDs (forward-compat) still injected - TestEmptyGroupFilter (2): empty optgroups dropped from picker; custom: providers exempted from filter Plus one structural test fix in `tests/test_issue604_all_providers_model_picker.py:test_cfg_providers_only_adds_known` — widened the regex window from 500 to 1500 chars so the new documentation comment block doesn't push `_PROVIDER_MODELS` past the substring slice. Pre-existing brittle window pattern, not a new issue. Verification ------------ Live on port 8789 with Deor's exact reproduction config (`providers.opencode_go.api_key` + `model.provider: opencode-go`): /api/models groups: 1 (was 2) Browser <select> optgroups: 1 (was 2) Total options under "OpenCode Go": 14 (was 14 in real group + 0 in phantom group) Five-scenario sweep all collapse to ONE provider group: | Config shape | Pre-fix | Post-fix | |---|---|---| | Hyphenated provider + underscored providers-key (Deor's case) | 2 groups | 1 group ✅ | | Hyphenated provider + UPPERCASE providers-key | 2 groups | 1 group ✅ | | Aliased providers-key (z-ai resolved to zai) | 2 groups | 1 group ✅ | | model.default = provider-id (orig #1568 scenario) | 15 models with phantom | 14 models, no phantom ✅ | | Happy path (canonical-only) | 1 group | 1 group ✅ | 4070 pytest passed (was 4053 → 4070, +17 from this PR). 3 CI runs to follow on push. QA harness 11/11 passed. JS unaffected — pure backend fix. Reporter: Deor (Discord #report-bugs, May 03 2026 14:19 PT) Relayed by: @AvidFuturist
113 lines
4.1 KiB
Python
113 lines
4.1 KiB
Python
"""Tests for #604 — model picker shows all configured providers."""
|
|
import re
|
|
|
|
|
|
def _src() -> str:
|
|
with open("api/config.py") as f:
|
|
return f.read()
|
|
|
|
|
|
def _get_provider_models_keys() -> set:
|
|
"""Extract top-level provider keys from _PROVIDER_MODELS dict."""
|
|
with open("api/config.py") as f:
|
|
lines = f.readlines()
|
|
keys = []
|
|
in_dict = False
|
|
for line in lines:
|
|
if "_PROVIDER_MODELS = {" in line:
|
|
in_dict = True
|
|
continue
|
|
if in_dict:
|
|
m = re.match(r'^ "([^"]+)":\s*\[', line)
|
|
if m:
|
|
keys.append(m.group(1))
|
|
if re.match(r'^\}', line):
|
|
break
|
|
return set(keys)
|
|
|
|
|
|
_PROVIDER_MODELS_KEYS = _get_provider_models_keys()
|
|
|
|
|
|
class TestProviderDetectionEnvVars:
|
|
"""All known env vars should map to valid provider IDs."""
|
|
|
|
# Providers that exist but aren't in _PROVIDER_MODELS (use special handling)
|
|
_SPECIAL_PROVIDERS = {"openrouter", "ollama-cloud", "custom", "ollama", "lmstudio", "local"}
|
|
|
|
def test_xai_env_maps_to_xai_provider(self):
|
|
"""XAI_API_KEY should add 'x-ai' (not 'xai')."""
|
|
src = _src()
|
|
assert re.search(r'XAI_API_KEY.*?add\("x-ai"\)', src, re.DOTALL), \
|
|
"XAI_API_KEY must map to provider 'x-ai'"
|
|
|
|
def test_mistral_env_maps_to_mistralai_provider(self):
|
|
"""MISTRAL_API_KEY should add 'mistralai' (not 'mistral')."""
|
|
src = _src()
|
|
assert re.search(r'MISTRAL_API_KEY.*?add\("mistralai"\)', src, re.DOTALL), \
|
|
"MISTRAL_API_KEY must map to provider 'mistralai'"
|
|
|
|
def test_all_provider_env_vars_map_to_known_providers(self):
|
|
"""Every detected_provider.add() call should reference a known provider."""
|
|
src = _src()
|
|
fn = re.search(r'def _build_available_models_uncached', src)
|
|
fn_block = src[fn.start():fn.start() + 10000]
|
|
adds = re.findall(r'detected_providers\.add\("([^"]+)"\)', fn_block)
|
|
unknown = [p for p in adds if p not in _PROVIDER_MODELS_KEYS and p not in self._SPECIAL_PROVIDERS]
|
|
assert not unknown, \
|
|
f"Unknown provider IDs in env var detection: {unknown}"
|
|
|
|
|
|
class TestConfigProvidersDetection:
|
|
"""Providers listed in config.yaml providers section should be detected."""
|
|
|
|
def test_cfg_providers_detection_exists(self):
|
|
"""_build_available_models must scan cfg['providers'] for known providers."""
|
|
src = _src()
|
|
assert "cfg.get(\"providers\", {})" in src, \
|
|
"Must read cfg['providers']"
|
|
assert "_cfg_providers" in src, \
|
|
"Must use _cfg_providers variable"
|
|
|
|
def test_cfg_providers_only_adds_known(self):
|
|
"""Only providers in _PROVIDER_MODELS should be added from config."""
|
|
src = _src()
|
|
# Find the config providers detection block
|
|
m = re.search(r'Also detect providers explicitly listed', src)
|
|
assert m, "Comment about config.yaml providers detection must exist"
|
|
# 1500-char window absorbs documentation expansion (e.g. the
|
|
# _canonicalise_provider_id discussion added in #1568) without
|
|
# losing the structural-assertion intent.
|
|
block = src[m.start():m.start() + 1500]
|
|
assert "_PROVIDER_MODELS" in block, \
|
|
"Config providers detection must check against _PROVIDER_MODELS"
|
|
|
|
|
|
class TestProviderModelsCompleteness:
|
|
"""Verify _PROVIDER_MODELS has expected providers."""
|
|
|
|
def test_has_anthropic(self):
|
|
assert "anthropic" in _PROVIDER_MODELS_KEYS
|
|
|
|
def test_has_openai(self):
|
|
assert "openai" in _PROVIDER_MODELS_KEYS
|
|
|
|
def test_has_google(self):
|
|
assert "google" in _PROVIDER_MODELS_KEYS
|
|
|
|
def test_has_deepseek(self):
|
|
assert "deepseek" in _PROVIDER_MODELS_KEYS
|
|
|
|
def test_has_xai(self):
|
|
assert "x-ai" in _PROVIDER_MODELS_KEYS
|
|
|
|
def test_has_mistralai(self):
|
|
assert "mistralai" in _PROVIDER_MODELS_KEYS
|
|
|
|
def test_has_openrouter(self):
|
|
# openrouter uses _FALLBACK_MODELS, not _PROVIDER_MODELS
|
|
pass # intentionally no assertion
|
|
|
|
def test_has_minimax_cn(self):
|
|
assert "minimax-cn" in _PROVIDER_MODELS_KEYS
|