From 4683a4a0d0a201c6f60f49ec0f1912a52ad2ba41 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 30 Apr 2026 15:24:35 +0000 Subject: [PATCH] fix(models): default model rehydration when providers share slash-qualified IDs (#1313) From PR #1326. Co-authored-by: hacker2005 --- api/config.py | 65 ++++++++++--------- static/panels.js | 4 +- static/ui.js | 58 ++++++++++++----- ...st_issue1228_model_picker_duplicate_ids.py | 56 ++++++++++++++-- tests/test_model_picker_badges.py | 50 +++++++++++++- 5 files changed, 180 insertions(+), 53 deletions(-) diff --git a/api/config.py b/api/config.py index 406261e2..e49ba23a 100644 --- a/api/config.py +++ b/api/config.py @@ -880,23 +880,23 @@ def _apply_provider_prefix( def _deduplicate_model_ids(groups: list[dict]) -> None: """Ensure every model ID across groups is globally unique. - When multiple providers expose the same bare model ID (e.g. two - custom providers both listing ``gpt-5.4``), the dropdown cannot - distinguish them. This post-process detects such collisions and - prefixes colliding entries with ``@provider_id:`` so the frontend - can treat them as distinct options. + When multiple providers expose the same model ID (either bare names like + ``gpt-5.4`` or slash-qualified IDs like ``google/gemma-4-27b``), the + dropdown cannot distinguish them. This post-process detects such + collisions and prefixes colliding entries with ``@provider_id:`` so the + frontend can treat them as distinct options. - The first occurrence (in group order) is left bare for backward - compatibility with sessions that already store the bare model name. - If that provider is later removed from the config, the next cache - rebuild re-runs dedup — the remaining provider becomes the sole - occurrence and is left bare, so the session still matches. + The first occurrence (in provider-id order) is left unchanged for backward + compatibility with sessions that already store the original bare/slash + model name. If that provider is later removed from the config, the next + cache rebuild re-runs dedup — the remaining provider becomes the sole + occurrence and is left unchanged, so the session still matches. .. note:: - The "first occurrence wins" rule means the bare ID is not stable + The "first occurrence wins" rule means the unchanged ID is not stable across config changes (adding, removing, or reordering providers). This is acceptable because the dedup runs on every cache rebuild, - so sessions always resolve to the current canonical bare ID. + so sessions always resolve to the current canonical unchanged ID. The ``@provider_id:model`` format is consistent with the existing ``_apply_provider_prefix()`` function and is handled by @@ -908,8 +908,8 @@ def _deduplicate_model_ids(groups: list[dict]) -> None: if not groups: return - # Collect {bare_id: [(group_idx, model_idx), ...]} in alphabetical - # provider_id order so that the "first occurrence stays bare" rule is + # Collect {model_id: [(group_idx, model_idx), ...]} in alphabetical + # provider_id order so that the "first occurrence stays unchanged" rule is # deterministic across config edits (adding/removing/reordering providers). sorted_group_indices = sorted( range(len(groups)), @@ -918,34 +918,29 @@ def _deduplicate_model_ids(groups: list[dict]) -> None: id_map: dict[str, list[tuple[int, int]]] = {} for gi in sorted_group_indices: group = groups[gi] - pid = group.get("provider_id", "") for mi, model in enumerate(group.get("models", [])): - mid = model.get("id", "") - # Skip IDs that are already provider-qualified - if mid.startswith("@") or "/" in mid: + mid = str(model.get("id", "") or "").strip() + # Skip IDs that are already provider-qualified. + if not mid or mid.startswith("@"): continue id_map.setdefault(mid, []).append((gi, mi)) - # For any bare ID appearing in 2+ groups, prefix all but the first - # occurrence. The first stays bare for backward compat; the rest - # get ``@provider_id:id`` and a disambiguated label. + # For any ID appearing in 2+ groups, prefix all but the first occurrence. # This handles N>2 providers correctly: the loop iterates over all # occurrences after the first, prefixing each with its own provider_id. - for bare_id, locations in id_map.items(): + for original_id, locations in id_map.items(): if len(locations) < 2: continue - # Prefix all occurrences after the first for gi, mi in locations[1:]: group = groups[gi] model = group["models"][mi] pid = group.get("provider_id", "") - model["id"] = f"@{pid}:{bare_id}" + model["id"] = f"@{pid}:{original_id}" provider_name = group.get("provider", pid) - # Update label to show provider for clarity - if model.get("label") != bare_id: + if model.get("label") != original_id: model["label"] = f"{model['label']} ({provider_name})" else: - model["label"] = f"{bare_id} ({provider_name})" + model["label"] = f"{original_id} ({provider_name})" def resolve_model_provider(model_id: str) -> tuple: @@ -1475,6 +1470,12 @@ def get_available_models() -> dict: option_ids = [m.get("id", "") for g in groups for m in g.get("models", []) if m.get("id")] option_lookup = {str(opt_id): str(opt_id) for opt_id in option_ids} + option_provider_lookup = { + str(m.get("id")): str(g.get("provider_id") or "") + for g in groups + for m in g.get("models", []) + if m.get("id") + } norm_lookup: dict[str, list[str]] = {} for opt_id in option_ids: norm_lookup.setdefault(_norm_model_id(opt_id), []).append(opt_id) @@ -1493,8 +1494,9 @@ def get_available_models() -> dict: raw_candidates.append(candidate) match_id = None + exact_match = next((option_lookup[c] for c in raw_candidates if c in option_lookup), None) for candidate in raw_candidates: - if candidate in option_lookup: + if candidate in option_lookup and option_provider_lookup.get(candidate) == provider: match_id = option_lookup[candidate] break if match_id is None: @@ -1504,15 +1506,18 @@ def get_available_models() -> dict: if not matches: continue provider_match = next( - (m for m in matches if m.startswith(f"@{provider}:") or m.startswith(f"{provider}/")), + (m for m in matches if option_provider_lookup.get(m) == provider), None, ) - match_id = provider_match or matches[0] + match_id = provider_match or exact_match or matches[0] if match_id: break badge_payload = {"role": entry["role"], "label": entry["label"], "provider": provider} for candidate in raw_candidates: + candidate_provider = option_provider_lookup.get(candidate) + if candidate_provider and candidate_provider != provider: + continue badges[candidate] = badge_payload if match_id: badges[match_id] = badge_payload diff --git a/static/panels.js b/static/panels.js index aee02104..67865b3c 100644 --- a/static/panels.js +++ b/static/panels.js @@ -2244,7 +2244,7 @@ async function switchToProfile(name) { // ── Apply model ──────────────────────────────────────────────────────── if (data.default_model) { const sel = $('modelSelect'); - const resolved = _applyModelToDropdown(data.default_model, sel); + const resolved = _applyModelToDropdown(data.default_model, sel, window._activeProvider||null); const modelToUse = resolved || data.default_model; S._pendingProfileModel = modelToUse; // Only patch the in-memory session model if we're NOT about to replace the session @@ -2753,7 +2753,7 @@ async function loadSettingsPanel(){ // picker renders blank for any user whose default was persisted without the // @-prefix — CLI-first users, legacy installs, etc. if(typeof _applyModelToDropdown==='function'){ - _applyModelToDropdown(_settingsHermesDefaultModelOnOpen, modelSel); + _applyModelToDropdown(_settingsHermesDefaultModelOnOpen, modelSel, (models&&models.active_provider)||window._activeProvider||null); }else{ modelSel.value=_settingsHermesDefaultModelOnOpen; } diff --git a/static/ui.js b/static/ui.js index edfb6250..cffc8a31 100644 --- a/static/ui.js +++ b/static/ui.js @@ -192,16 +192,34 @@ window._configuredModelBadges=window._configuredModelBadges||{}; // ── Smart model resolver ──────────────────────────────────────────────────── // Finds the best matching option value in a