fix(picker): collapse duplicate provider groups + guard provider-id-as-model.default (closes #1568)
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
This commit is contained in:
nesquena-hermes
153
api/config.py
153
api/config.py
@@ -653,6 +653,48 @@ def _resolve_provider_alias(name: str) -> str:
|
||||
return _PROVIDER_ALIASES.get(raw, name)
|
||||
|
||||
|
||||
def _canonicalise_provider_id(name: object) -> str:
|
||||
"""Normalise a provider id slug into a stable lowercase-hyphenated form.
|
||||
|
||||
Folds underscores to hyphens and lowercases the result, so a user with
|
||||
``providers.opencode_go.api_key`` in ``config.yaml`` and
|
||||
``model.provider: opencode-go`` sees ONE provider group, not two
|
||||
(#1568). Then attempts alias resolution but only if the alias target
|
||||
is itself a known canonical id in ``_PROVIDER_DISPLAY`` — this avoids
|
||||
converting ``x-ai`` (canonical in WebUI's data structures) to ``xai``
|
||||
(the hermes_cli alias target which the WebUI doesn't index by).
|
||||
|
||||
Examples::
|
||||
|
||||
opencode-go -> opencode-go (canonical, no change)
|
||||
opencode_go -> opencode-go (underscore folded)
|
||||
OpenCode-Go -> opencode-go (case folded)
|
||||
OPENCODE_GO -> opencode-go (both folded)
|
||||
z_ai -> zai (alias-resolved — zai is canonical)
|
||||
x-ai -> x-ai (preserved — x-ai is canonical)
|
||||
|
||||
Empty input passes through as the empty string. Unknown ids preserve
|
||||
their normalised form.
|
||||
"""
|
||||
if not name:
|
||||
return ""
|
||||
raw = str(name).strip().lower().replace("_", "-")
|
||||
if not raw:
|
||||
return ""
|
||||
# Already a canonical id known to _PROVIDER_DISPLAY/_PROVIDER_MODELS:
|
||||
# keep as-is to avoid round-tripping through aliases (e.g. x-ai → xai).
|
||||
if raw in _PROVIDER_DISPLAY or raw in _PROVIDER_MODELS:
|
||||
return raw
|
||||
# Try alias resolution. Only accept the result if it's itself a
|
||||
# canonical id in _PROVIDER_DISPLAY — that prevents aliases pointing
|
||||
# at non-canonical strings (legacy, hermes_cli-specific) from leaking
|
||||
# in. Falls back to the normalised input otherwise.
|
||||
resolved = _resolve_provider_alias(raw)
|
||||
if resolved and resolved.lower() in _PROVIDER_DISPLAY:
|
||||
return resolved.lower()
|
||||
return raw
|
||||
|
||||
|
||||
# Well-known models per provider (used to populate dropdown for direct API providers)
|
||||
_PROVIDER_MODELS = {
|
||||
"anthropic": [
|
||||
@@ -1844,11 +1886,21 @@ def get_available_models() -> dict:
|
||||
# Also detect providers explicitly listed in config.yaml providers section.
|
||||
# A user may configure a provider key via config.yaml providers.<name>.api_key
|
||||
# without setting the corresponding env var. (#604)
|
||||
#
|
||||
# Canonicalise the id slug here so a user with ``providers.opencode_go``
|
||||
# (underscore variant) doesn't see TWO provider groups in the picker —
|
||||
# one for the canonical ``opencode-go`` from active_provider detection
|
||||
# and a phantom ``Opencode_Go`` group for the config-key form (#1568).
|
||||
# The same applies to mixed-case ids like ``OpenCode-Go`` and to
|
||||
# legitimate aliases like ``z-ai`` → ``zai``.
|
||||
_cfg_providers = cfg.get("providers", {})
|
||||
if isinstance(_cfg_providers, dict):
|
||||
for _pid_key in _cfg_providers:
|
||||
if _pid_key in _PROVIDER_MODELS or _pid_key in cfg.get("providers", {}):
|
||||
detected_providers.add(_pid_key)
|
||||
_canonical = _canonicalise_provider_id(_pid_key)
|
||||
if not _canonical:
|
||||
continue
|
||||
if _canonical in _PROVIDER_MODELS or _canonical in _cfg_providers or _pid_key in _cfg_providers:
|
||||
detected_providers.add(_canonical)
|
||||
|
||||
def _normalize_base_url_for_match(value: object) -> str:
|
||||
url = str(value or "").strip().rstrip("/")
|
||||
@@ -2115,10 +2167,30 @@ def get_available_models() -> dict:
|
||||
configured_providers.add(active_provider)
|
||||
cfg_providers = cfg.get("providers", {})
|
||||
if isinstance(cfg_providers, dict):
|
||||
configured_providers.update(cfg_providers.keys())
|
||||
# Canonicalise here too — same rationale as #1568 detection
|
||||
# path. Without this, only_show_configured mode could
|
||||
# exclude detected ``opencode-go`` because configured_providers
|
||||
# only has the underscore-variant key from config.yaml.
|
||||
configured_providers.update(
|
||||
_canonicalise_provider_id(k) or k for k in cfg_providers.keys()
|
||||
)
|
||||
# Only show providers that are both detected and configured
|
||||
detected_providers = detected_providers.intersection(configured_providers)
|
||||
|
||||
# Post-collection dedup: re-canonicalise every entry so any path that
|
||||
# added a non-canonical id (mixed-case from auth-store, raw config-key,
|
||||
# legacy alias) gets folded onto the canonical key. Belt-and-braces for
|
||||
# #1568 — protects against future regressions in any of the ~25
|
||||
# `detected_providers.add(...)` callsites without auditing each one.
|
||||
# The fold is idempotent for already-canonical ids, so safe to run
|
||||
# unconditionally.
|
||||
if detected_providers:
|
||||
_canonicalised_detected = set()
|
||||
for _pid in detected_providers:
|
||||
_c = _canonicalise_provider_id(_pid) or _pid
|
||||
_canonicalised_detected.add(_c)
|
||||
detected_providers = _canonicalised_detected
|
||||
|
||||
# 5. Build model groups
|
||||
if detected_providers:
|
||||
for pid in sorted(detected_providers):
|
||||
@@ -2332,34 +2404,69 @@ def get_available_models() -> dict:
|
||||
)
|
||||
|
||||
if default_model:
|
||||
all_ids_norm = {_norm_model_id(m["id"]) for g in groups for m in g.get("models", [])}
|
||||
if _norm_model_id(default_model) not in all_ids_norm:
|
||||
label = _get_label_for_model(default_model, groups)
|
||||
target_display = (
|
||||
_PROVIDER_DISPLAY.get(active_provider, active_provider or "").lower()
|
||||
if active_provider
|
||||
else ""
|
||||
# Guard against provider-id values mistakenly stored in
|
||||
# ``model.default``. The injection logic below puts ANY string
|
||||
# into the picker as a fake option, so a stray provider id
|
||||
# surfaces as a self-referential phantom model labelled e.g.
|
||||
# ``Opencode GO`` — a 15th entry under the OpenCode Go group
|
||||
# (#1568). The user's misconfig is real, but the picker is
|
||||
# the wrong surface to surface it; we'd rather skip injection
|
||||
# and emit a warning so the underlying config issue is logged.
|
||||
_looks_like_provider_id = (
|
||||
str(default_model).strip().lower().replace("_", "-") in _PROVIDER_DISPLAY
|
||||
or _canonicalise_provider_id(default_model) in _PROVIDER_DISPLAY
|
||||
)
|
||||
if _looks_like_provider_id:
|
||||
logger.warning(
|
||||
"Suspicious model.default value %r — looks like a provider id, "
|
||||
"not a model id. Skipping picker injection. Check `model.default` "
|
||||
"in config.yaml.",
|
||||
default_model,
|
||||
)
|
||||
injected = False
|
||||
for g in groups:
|
||||
if target_display and g.get("provider", "").lower() == target_display:
|
||||
g["models"].insert(0, {"id": default_model, "label": label})
|
||||
injected = True
|
||||
break
|
||||
if not injected and groups:
|
||||
groups.append(
|
||||
{
|
||||
"provider": "Default",
|
||||
"provider_id": active_provider or "default",
|
||||
"models": [{"id": default_model, "label": label}],
|
||||
}
|
||||
else:
|
||||
all_ids_norm = {_norm_model_id(m["id"]) for g in groups for m in g.get("models", [])}
|
||||
if _norm_model_id(default_model) not in all_ids_norm:
|
||||
label = _get_label_for_model(default_model, groups)
|
||||
target_display = (
|
||||
_PROVIDER_DISPLAY.get(active_provider, active_provider or "").lower()
|
||||
if active_provider
|
||||
else ""
|
||||
)
|
||||
injected = False
|
||||
for g in groups:
|
||||
if target_display and g.get("provider", "").lower() == target_display:
|
||||
g["models"].insert(0, {"id": default_model, "label": label})
|
||||
injected = True
|
||||
break
|
||||
if not injected and groups:
|
||||
groups.append(
|
||||
{
|
||||
"provider": "Default",
|
||||
"provider_id": active_provider or "default",
|
||||
"models": [{"id": default_model, "label": label}],
|
||||
}
|
||||
)
|
||||
|
||||
# Post-process: ensure model IDs are globally unique across groups.
|
||||
# When multiple providers expose the same bare model ID, prefix
|
||||
# collisions with @provider_id: so the frontend can distinguish them.
|
||||
_deduplicate_model_ids(groups)
|
||||
|
||||
# Defense-in-depth: drop any optgroup that ended up with zero models
|
||||
# — those are pure UI noise. A zero-model group typically means a
|
||||
# detection path added an id that has no static catalog AND the
|
||||
# live-fetch returned empty (#1568 — the user's
|
||||
# ``providers.opencode_go`` config-key path produced an empty
|
||||
# ``Opencode_Go`` group at the end of the picker before this fix).
|
||||
# Custom providers from ``custom_providers`` config are exempt —
|
||||
# they may legitimately render with zero entries when the user
|
||||
# hasn't filled in models yet but wants the card visible.
|
||||
groups = [
|
||||
g for g in groups
|
||||
if g.get("models")
|
||||
or (g.get("provider_id") or "").startswith("custom:")
|
||||
]
|
||||
|
||||
return {
|
||||
"active_provider": active_provider,
|
||||
"default_model": default_model,
|
||||
|
||||
424
tests/test_issue1568_duplicate_provider_groups.py
Normal file
424
tests/test_issue1568_duplicate_provider_groups.py
Normal file
@@ -0,0 +1,424 @@
|
||||
"""Regression tests for #1568 — duplicate provider groups in model picker.
|
||||
|
||||
Reporter (Deor, Discord #report-bugs, May 03 2026 14:19 PT) 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 with one self-referential entry).
|
||||
|
||||
Three structural causes, all in ``api/config.py:_build_available_models_uncached``:
|
||||
|
||||
1. The detection path 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 groups.
|
||||
|
||||
2. The injection block at line ~2598 puts ANY ``model.default`` string into
|
||||
the picker as a fake option, so a stray ``model.default: opencode_go``
|
||||
(provider id mistakenly used as a model id) surfaces as a phantom model
|
||||
labelled ``"Opencode GO"``.
|
||||
|
||||
3. Empty optgroups can leak through 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.
|
||||
|
||||
The fix is a new ``_canonicalise_provider_id`` helper applied at every
|
||||
detection callsite, a post-collection dedup of ``detected_providers``, a
|
||||
provider-id guard on the model.default injection block, and an empty-group
|
||||
filter at the very end of the build.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
|
||||
import api.config as config
|
||||
import api.profiles as profiles
|
||||
|
||||
|
||||
def _install_fake_hermes_cli(monkeypatch):
|
||||
"""Stub hermes_cli so detection is deterministic in tests."""
|
||||
fake_pkg = types.ModuleType("hermes_cli")
|
||||
fake_pkg.__path__ = []
|
||||
|
||||
fake_models = types.ModuleType("hermes_cli.models")
|
||||
fake_models.list_available_providers = lambda: []
|
||||
fake_models.provider_model_ids = lambda pid: []
|
||||
|
||||
fake_auth = types.ModuleType("hermes_cli.auth")
|
||||
fake_auth.get_auth_status = lambda _pid: {}
|
||||
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli", fake_pkg)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.models", fake_models)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.auth", fake_auth)
|
||||
monkeypatch.delitem(sys.modules, "agent.credential_pool", raising=False)
|
||||
monkeypatch.delitem(sys.modules, "agent", raising=False)
|
||||
|
||||
config.invalidate_models_cache()
|
||||
|
||||
|
||||
def _swap_in_test_config(extra_cfg):
|
||||
old_cfg = dict(config.cfg)
|
||||
old_mtime = config._cfg_mtime
|
||||
config.cfg.clear()
|
||||
config.cfg["model"] = {}
|
||||
config.cfg.update(extra_cfg)
|
||||
try:
|
||||
config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime
|
||||
except Exception:
|
||||
config._cfg_mtime = 0.0
|
||||
|
||||
def _restore():
|
||||
config.cfg.clear()
|
||||
config.cfg.update(old_cfg)
|
||||
config._cfg_mtime = old_mtime
|
||||
|
||||
return _restore
|
||||
|
||||
|
||||
def _scrub_provider_env(monkeypatch):
|
||||
for var in (
|
||||
"ANTHROPIC_API_KEY", "OPENAI_API_KEY", "GOOGLE_API_KEY", "GEMINI_API_KEY",
|
||||
"DEEPSEEK_API_KEY", "XAI_API_KEY", "GROQ_API_KEY",
|
||||
"MISTRAL_API_KEY", "OPENROUTER_API_KEY",
|
||||
"OLLAMA_CLOUD_API_KEY", "OLLAMA_API_KEY",
|
||||
"GLM_API_KEY", "KIMI_API_KEY", "MOONSHOT_API_KEY",
|
||||
"MINIMAX_API_KEY", "MINIMAX_CN_API_KEY",
|
||||
"OPENCODE_ZEN_API_KEY", "OPENCODE_GO_API_KEY",
|
||||
"NOUS_API_KEY", "NVIDIA_API_KEY", "LM_API_KEY", "LMSTUDIO_API_KEY",
|
||||
):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 1 — _canonicalise_provider_id helper
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestCanonicaliseProviderId:
|
||||
def test_canonical_id_preserved(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
assert _canonicalise_provider_id("opencode-go") == "opencode-go"
|
||||
assert _canonicalise_provider_id("anthropic") == "anthropic"
|
||||
assert _canonicalise_provider_id("x-ai") == "x-ai"
|
||||
|
||||
def test_underscore_folded_to_hyphen(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
# Deor's exact failure mode — the config-file key uses underscores
|
||||
# but every other code path uses the hyphenated canonical form.
|
||||
assert _canonicalise_provider_id("opencode_go") == "opencode-go"
|
||||
|
||||
def test_case_folded(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
assert _canonicalise_provider_id("OpenCode-Go") == "opencode-go"
|
||||
assert _canonicalise_provider_id("OPENCODE_GO") == "opencode-go"
|
||||
assert _canonicalise_provider_id("Anthropic") == "anthropic"
|
||||
|
||||
def test_alias_resolved_when_target_is_canonical(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
# z-ai is an alias for the canonical zai.
|
||||
assert _canonicalise_provider_id("z-ai") == "zai"
|
||||
assert _canonicalise_provider_id("z_ai") == "zai"
|
||||
assert _canonicalise_provider_id("Z.AI") == "zai" or _canonicalise_provider_id("Z.AI") == "z.ai"
|
||||
|
||||
def test_alias_not_applied_when_input_is_already_canonical(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
# x-ai IS the canonical key in _PROVIDER_DISPLAY/_PROVIDER_MODELS.
|
||||
# _PROVIDER_ALIASES happens to also map x-ai → xai (for hermes_cli
|
||||
# compat), but we must NOT round-trip through that alias because
|
||||
# xai isn't keyed in _PROVIDER_DISPLAY/_PROVIDER_MODELS.
|
||||
assert _canonicalise_provider_id("x-ai") == "x-ai"
|
||||
assert _canonicalise_provider_id("X-AI") == "x-ai"
|
||||
|
||||
def test_empty_input(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
assert _canonicalise_provider_id("") == ""
|
||||
assert _canonicalise_provider_id(None) == ""
|
||||
assert _canonicalise_provider_id(" ") == ""
|
||||
|
||||
def test_unknown_id_normalised_but_preserved(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
# Unknown ids: still get the underscore→hyphen + lowercase fold so
|
||||
# downstream dedup works, but no alias resolution.
|
||||
assert _canonicalise_provider_id("future_provider") == "future-provider"
|
||||
assert _canonicalise_provider_id("CUSTOM_THING") == "custom-thing"
|
||||
|
||||
def test_idempotent(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
for raw in ("opencode_go", "OPENCODE-GO", "z-ai", "anthropic", "future_x"):
|
||||
once = _canonicalise_provider_id(raw)
|
||||
twice = _canonicalise_provider_id(once)
|
||||
assert once == twice, f"helper must be idempotent: {raw!r} -> {once!r} -> {twice!r}"
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 2 — Detection-path dedup (the core #1568 fix)
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestProviderGroupDedup:
|
||||
"""When config.yaml uses a non-canonical providers.<id> key, the picker
|
||||
must still surface ONE provider group, not two."""
|
||||
|
||||
def test_underscored_providers_key_does_not_create_phantom_group(self, monkeypatch, tmp_path):
|
||||
"""Deor's exact reproduction case: ``providers.opencode_go.api_key``
|
||||
(underscored) with ``model.provider: opencode-go`` (hyphenated)."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "opencode-go", "default": "glm-5.1"},
|
||||
"providers": {"opencode_go": {"api_key": "fake-test-key"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
opencode_groups = [
|
||||
g for g in data["groups"]
|
||||
if "opencode" in (g.get("provider_id") or "").lower()
|
||||
or "opencode" in (g.get("provider") or "").lower()
|
||||
]
|
||||
assert len(opencode_groups) == 1, (
|
||||
f"Expected exactly ONE OpenCode Go group, got {len(opencode_groups)}: "
|
||||
f"{[(g['provider'], g['provider_id']) for g in opencode_groups]}. "
|
||||
f"Pre-fix, the underscored providers-key produced a separate "
|
||||
f"'Opencode_Go' provider group at the bottom of the picker (#1568)."
|
||||
)
|
||||
grp = opencode_groups[0]
|
||||
assert grp["provider_id"] == "opencode-go", (
|
||||
f"Group provider_id should be canonical 'opencode-go', got "
|
||||
f"{grp['provider_id']!r}."
|
||||
)
|
||||
assert grp["provider"] == "OpenCode Go", (
|
||||
f"Group display name should be canonical 'OpenCode Go', got "
|
||||
f"{grp['provider']!r}."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_uppercase_providers_key_does_not_create_phantom_group(self, monkeypatch, tmp_path):
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "opencode-go", "default": "glm-5.1"},
|
||||
"providers": {"OPENCODE-GO": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
opencode_groups = [
|
||||
g for g in data["groups"]
|
||||
if (g.get("provider_id") or "").lower().replace("_", "-") == "opencode-go"
|
||||
]
|
||||
assert len(opencode_groups) == 1
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_aliased_providers_key_collapses_to_canonical(self, monkeypatch, tmp_path):
|
||||
"""``z-ai`` is a known alias for canonical ``zai``. A user with
|
||||
``providers.z-ai.api_key`` should still see ONE Z.AI group, not two."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "zai", "default": "glm-5"},
|
||||
"providers": {"z-ai": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
zai_groups = [
|
||||
g for g in data["groups"]
|
||||
if (g.get("provider_id") or "") in ("zai", "z-ai")
|
||||
]
|
||||
assert len(zai_groups) == 1, (
|
||||
f"Expected one Z.AI group, got {len(zai_groups)}: "
|
||||
f"{[(g['provider'], g['provider_id']) for g in zai_groups]}"
|
||||
)
|
||||
assert zai_groups[0]["provider_id"] == "zai"
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_happy_path_unchanged(self, monkeypatch, tmp_path):
|
||||
"""Sanity: when config keys are already canonical, behaviour is unchanged."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "opencode-go", "default": "glm-5.1"},
|
||||
"providers": {"opencode-go": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
opencode_groups = [
|
||||
g for g in data["groups"]
|
||||
if g.get("provider_id") == "opencode-go"
|
||||
]
|
||||
assert len(opencode_groups) == 1
|
||||
assert opencode_groups[0]["provider"] == "OpenCode Go"
|
||||
assert len(opencode_groups[0]["models"]) >= 1
|
||||
finally:
|
||||
restore()
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 3 — model.default provider-id injection guard
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestDefaultModelProviderIdGuard:
|
||||
"""``model.default = <provider id>`` is a common config typo. Pre-fix the
|
||||
picker silently injected the provider id as a phantom model option.
|
||||
Post-fix the injection is skipped + a warning is logged."""
|
||||
|
||||
def test_provider_id_as_default_does_not_inject_phantom(self, monkeypatch, tmp_path, caplog):
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "opencode-go", "default": "opencode_go"},
|
||||
"providers": {"opencode-go": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
with caplog.at_level("WARNING", logger="api.config"):
|
||||
data = config.get_available_models()
|
||||
opencode = next(
|
||||
g for g in data["groups"] if g.get("provider_id") == "opencode-go"
|
||||
)
|
||||
ids = {m["id"] for m in opencode["models"]}
|
||||
for bad in ("opencode_go", "opencode-go", "OpenCode Go"):
|
||||
assert bad not in ids, (
|
||||
f"Phantom model id {bad!r} leaked into picker — the "
|
||||
f"provider-id guard should skip injection. Pre-fix, "
|
||||
f"this surfaced as a self-referential 'Opencode GO' "
|
||||
f"15th entry. (#1568)"
|
||||
)
|
||||
# And we get a logged warning so the misconfig is discoverable.
|
||||
assert any(
|
||||
"model.default" in rec.getMessage().lower()
|
||||
or "provider id" in rec.getMessage().lower()
|
||||
for rec in caplog.records
|
||||
), (
|
||||
"Skipping the injection should emit a WARNING so the user's "
|
||||
"actual config error is discoverable in logs, not just silently "
|
||||
"papered over."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_provider_alias_as_default_does_not_inject_phantom(self, monkeypatch, tmp_path):
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
# Z.AI / GLM has display name "Z.AI / GLM", canonical id "zai",
|
||||
# alias "z-ai". model.default == "z-ai" should be caught.
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "zai", "default": "z-ai"},
|
||||
"providers": {"zai": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
zai = next(g for g in data["groups"] if g.get("provider_id") == "zai")
|
||||
ids = {m["id"] for m in zai["models"]}
|
||||
assert "z-ai" not in ids
|
||||
assert "zai" not in ids
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_real_unknown_model_id_still_injected(self, monkeypatch, tmp_path):
|
||||
"""Forward-compat: a NEW model id not yet in the static catalog
|
||||
(newly released, custom endpoint) should STILL be injected so the
|
||||
user's configured default isn't hidden from them."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "anthropic", "default": "claude-opus-5.0-future"},
|
||||
"providers": {"anthropic": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
all_ids = {m["id"] for g in data["groups"] for m in g["models"]}
|
||||
assert "claude-opus-5.0-future" in all_ids, (
|
||||
"Legitimate unknown model ids must still be injected — "
|
||||
"otherwise newly-released models or custom endpoints "
|
||||
"wouldn't show in the picker until a release with an "
|
||||
"updated _PROVIDER_MODELS catalog. The guard must only "
|
||||
"reject provider ids and known aliases."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 4 — Empty-group filter
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestEmptyGroupFilter:
|
||||
def test_empty_optgroups_dropped(self, monkeypatch, tmp_path):
|
||||
"""Pre-fix, when a non-canonical provider id slipped past the
|
||||
detection guards into _PROVIDER_MODELS lookup (which has no entry
|
||||
for ``opencode_go``), the build loop produced a zero-models
|
||||
optgroup that rendered as a phantom provider entry. The empty-group
|
||||
filter at the end of the build catches this regardless of which
|
||||
detection path leaked the bad id."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "opencode-go", "default": "glm-5.1"},
|
||||
"providers": {"opencode_go": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
empty_groups = [g for g in data["groups"] if not g.get("models")]
|
||||
# Only custom: groups are allowed to be empty (intentional UX).
|
||||
allowed_empty = [
|
||||
g for g in empty_groups
|
||||
if (g.get("provider_id") or "").startswith("custom:")
|
||||
]
|
||||
disallowed = [g for g in empty_groups if g not in allowed_empty]
|
||||
assert not disallowed, (
|
||||
f"Zero-model optgroups should not appear in the picker — "
|
||||
f"they're pure UI noise. Got {len(disallowed)} unexpected "
|
||||
f"empty groups: {[(g['provider'], g['provider_id']) for g in disallowed]}."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_custom_provider_can_still_be_empty(self, monkeypatch, tmp_path):
|
||||
"""Custom providers from ``custom_providers`` config are exempt
|
||||
from the empty-group filter — users may want an empty card visible
|
||||
as a reminder to fill in models."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "custom", "default": "some-model"},
|
||||
"custom_providers": [
|
||||
{"name": "my-empty-provider", "api_key": "fake"},
|
||||
],
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
# The empty-group filter should NOT drop a custom: provider.
|
||||
# (The exact custom group surface depends on other config logic;
|
||||
# this test just pins that custom: groups are exempt from the
|
||||
# filter, not that one is necessarily produced.)
|
||||
for g in data["groups"]:
|
||||
if (g.get("provider_id") or "").startswith("custom:"):
|
||||
# Found at least one custom group — that's enough to
|
||||
# confirm the exempt path doesn't drop them, since
|
||||
# the empty-models case would otherwise be filtered.
|
||||
return
|
||||
finally:
|
||||
restore()
|
||||
@@ -75,7 +75,10 @@ class TestConfigProvidersDetection:
|
||||
# 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"
|
||||
block = src[m.start():m.start() + 500]
|
||||
# 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"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user