From a8101d98f75b076275817b67e98a698e895fa99a Mon Sep 17 00:00:00 2001 From: bergeouss Date: Tue, 28 Apr 2026 18:42:52 +0000 Subject: [PATCH] fix(models): deduplicate model IDs across provider groups (#1228) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When multiple providers expose the same bare model ID (e.g. two custom providers both listing gpt-5.4), the model picker cannot distinguish them — both rows appear active and clicking the other provider's copy is a no-op. Fix: - Add _deduplicate_model_ids() post-process in api/config.py that detects duplicate bare model IDs across groups and prefixes collisions with @provider_id: so each entry is globally unique - Update norm() regex in static/ui.js to strip @provider: prefixes for fuzzy matching, so existing sessions with bare model IDs still restore correctly - First occurrence stays bare for backward compatibility with sessions that already store the bare model name - Update test_model_resolver to be dedup-aware Closes #1228 --- api/config.py | 59 +++++ static/ui.js | 3 +- ...st_issue1228_model_picker_duplicate_ids.py | 217 ++++++++++++++++++ tests/test_model_resolver.py | 3 +- 4 files changed, 280 insertions(+), 2 deletions(-) create mode 100644 tests/test_issue1228_model_picker_duplicate_ids.py diff --git a/api/config.py b/api/config.py index 9194cf2a..54045450 100644 --- a/api/config.py +++ b/api/config.py @@ -834,6 +834,60 @@ def _apply_provider_prefix( return result +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 the ID with ``@provider_id:`` for every group that + contributes the colliding entry. The *active* provider's copy + (first group matching ``active_provider``) is left bare so that + existing sessions referencing the bare name keep working. + + Operates in-place on *groups*. + """ + if not groups: + return + + # Collect {bare_id: [(group_idx, model_idx), ...]} + id_map: dict[str, list[tuple[int, int]]] = {} + for gi, group in enumerate(groups): + 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: + continue + id_map.setdefault(mid, []).append((gi, mi)) + + # For any bare ID appearing in 2+ groups, prefix all occurrences. + # The first group that matches the provider_id format we see gets a + # bare copy so existing session references still resolve; the rest + # get ``@provider_id:id``. + for bare_id, locations in id_map.items(): + if len(locations) < 2: + continue + # Mark all but the first location for prefixing + for gi, mi in locations[1:]: + group = groups[gi] + model = group["models"][mi] + pid = group.get("provider_id", "") + model["id"] = f"@{pid}:{bare_id}" + provider_name = group.get("provider", pid) + # Update label to show provider for clarity + if model.get("label") != bare_id: + model["label"] = f"{model['label']} ({provider_name})" + else: + model["label"] = f"{bare_id} ({provider_name})" + # Also prefix the first occurrence to keep things consistent — + # otherwise the "active" bare one and a prefixed one both match + # the same underlying model, which defeats the purpose. + # Actually, keep the first one bare for backward compatibility + # with sessions that already store the bare model name. + # If ALL copies are prefixed, old sessions break on model restore. + + def resolve_model_provider(model_id: str) -> tuple: """Resolve model name, provider, and base_url for AIAgent. @@ -1755,6 +1809,11 @@ def get_available_models() -> dict: } ) + # 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) + return { "active_provider": active_provider, "default_model": default_model, diff --git a/static/ui.js b/static/ui.js index 2d891e0e..a10d879b 100644 --- a/static/ui.js +++ b/static/ui.js @@ -102,7 +102,8 @@ function _findModelInDropdown(modelId, sel){ // 1. Exact match if(opts.includes(modelId)) return modelId; // 2. Normalize: lowercase, strip namespace prefix, replace hyphens→dots - const norm=s=>s.toLowerCase().replace(/^[^/]+\//,'').replace(/-/g,'.'); + // Also strip @provider: prefix from deduplicated model IDs (#1228). + const norm=s=>s.toLowerCase().replace(/^[^/]+\//,'').replace(/^@([^:]+:)+/,'').replace(/-/g,'.'); const target=norm(modelId); const exact=opts.find(o=>norm(o)===target); if(exact) return exact; diff --git a/tests/test_issue1228_model_picker_duplicate_ids.py b/tests/test_issue1228_model_picker_duplicate_ids.py new file mode 100644 index 00000000..07036ce8 --- /dev/null +++ b/tests/test_issue1228_model_picker_duplicate_ids.py @@ -0,0 +1,217 @@ +""" +Tests for #1228 — model picker loses provider identity when multiple +providers expose the same model ID. + +Covers: +- _deduplicate_model_ids() post-process in api/config.py +- Frontend norm() regex in ui.js that strips @provider: prefixes +""" +import copy +import unittest + + +class TestDeduplicateModelIds(unittest.TestCase): + """Backend: _deduplicate_model_ids() in api/config.py""" + + def _call(self, groups): + from api.config import _deduplicate_model_ids + groups = copy.deepcopy(groups) + _deduplicate_model_ids(groups) + return groups + + # ── No collision ──────────────────────────────────────────────── + + def test_unique_ids_unchanged(self): + """When all model IDs are unique across groups, nothing changes.""" + groups = [ + {"provider": "Anthropic", "provider_id": "anthropic", "models": [ + {"id": "claude-sonnet-4.6", "label": "Claude Sonnet 4.6"}, + ]}, + {"provider": "OpenAI", "provider_id": "openai-codex", "models": [ + {"id": "gpt-5.4", "label": "GPT-5.4"}, + ]}, + ] + result = self._call(groups) + assert result[0]["models"][0]["id"] == "claude-sonnet-4.6" + assert result[1]["models"][0]["id"] == "gpt-5.4" + + def test_single_group_unchanged(self): + """A single group never triggers deduplication.""" + groups = [ + {"provider": "Anthropic", "provider_id": "anthropic", "models": [ + {"id": "claude-sonnet-4.6", "label": "Claude Sonnet 4.6"}, + {"id": "claude-opus-4.6", "label": "Claude Opus 4.6"}, + ]}, + ] + result = self._call(groups) + ids = [m["id"] for m in result[0]["models"]] + assert "claude-sonnet-4.6" in ids + assert "claude-opus-4.6" in ids + + def test_empty_groups(self): + """Empty groups list is a no-op.""" + result = self._call([]) + assert result == [] + + # ── Collision: two providers, same bare model ID ──────────────── + + def test_two_providers_same_model_prefixes_second(self): + """When two providers share the same bare model ID, the second + gets @provider_id: prefix and a disambiguated label.""" + groups = [ + {"provider": "Edith", "provider_id": "custom:edith", "models": [ + {"id": "gpt-5.4", "label": "GPT-5.4"}, + ]}, + {"provider": "OpenAI Codex", "provider_id": "openai-codex", "models": [ + {"id": "gpt-5.4", "label": "GPT-5.4"}, + ]}, + ] + result = self._call(groups) + # First stays bare for backward compat + assert result[0]["models"][0]["id"] == "gpt-5.4" + assert result[0]["models"][0]["label"] == "GPT-5.4" + # Second gets prefixed + assert result[1]["models"][0]["id"] == "@openai-codex:gpt-5.4" + assert "OpenAI Codex" in result[1]["models"][0]["label"] + + def test_three_providers_same_model(self): + """With three providers sharing the same model, first stays bare, + the other two get prefixed.""" + groups = [ + {"provider": "A", "provider_id": "alpha", "models": [ + {"id": "gpt-5.4", "label": "GPT-5.4"}, + ]}, + {"provider": "B", "provider_id": "beta", "models": [ + {"id": "gpt-5.4", "label": "GPT-5.4"}, + ]}, + {"provider": "C", "provider_id": "gamma", "models": [ + {"id": "gpt-5.4", "label": "GPT-5.4"}, + ]}, + ] + result = self._call(groups) + assert result[0]["models"][0]["id"] == "gpt-5.4" + assert result[1]["models"][0]["id"] == "@beta:gpt-5.4" + assert result[2]["models"][0]["id"] == "@gamma:gpt-5.4" + + # ── Already-prefixed IDs are skipped ─────────────────────────── + + def test_already_prefixed_ids_skipped(self): + """Model IDs already starting with @ or containing / are not + considered for deduplication.""" + groups = [ + {"provider": "Anthropic", "provider_id": "anthropic", "models": [ + {"id": "@anthropic:claude-sonnet-4.6", "label": "Claude Sonnet 4.6"}, + ]}, + {"provider": "OpenRouter", "provider_id": "openrouter", "models": [ + {"id": "anthropic/claude-sonnet-4.6", "label": "Claude Sonnet 4.6 (OR)"}, + ]}, + ] + result = self._call(groups) + # Neither should be modified + assert result[0]["models"][0]["id"] == "@anthropic:claude-sonnet-4.6" + assert result[1]["models"][0]["id"] == "anthropic/claude-sonnet-4.6" + + # ── Mixed: some unique, some colliding ───────────────────────── + + def test_mixed_unique_and_colliding(self): + """Only colliding IDs get prefixed; unique ones stay bare.""" + groups = [ + {"provider": "Edith", "provider_id": "custom:edith", "models": [ + {"id": "gpt-5.4", "label": "GPT-5.4"}, + {"id": "claude-sonnet-4.6", "label": "Claude Sonnet 4.6"}, + ]}, + {"provider": "OpenAI Codex", "provider_id": "openai-codex", "models": [ + {"id": "gpt-5.4", "label": "GPT-5.4"}, + {"id": "o3-pro", "label": "O3 Pro"}, + ]}, + ] + result = self._call(groups) + # gpt-5.4 collides → second gets prefixed + assert result[0]["models"][0]["id"] == "gpt-5.4" + assert result[1]["models"][0]["id"] == "@openai-codex:gpt-5.4" + # claude-sonnet-4.6 is unique → stays bare + assert result[0]["models"][1]["id"] == "claude-sonnet-4.6" + # o3-pro is unique → stays bare + assert result[1]["models"][1]["id"] == "o3-pro" + + # ── Label disambiguation ──────────────────────────────────────── + + def test_label_differs_from_id_when_custom_label(self): + """When the original label differs from the bare ID, the + disambiguated label preserves the custom label + adds provider.""" + groups = [ + {"provider": "Edith", "provider_id": "custom:edith", "models": [ + {"id": "gpt-5.4", "label": "GPT 5.4 Turbo"}, + ]}, + {"provider": "Codex", "provider_id": "openai-codex", "models": [ + {"id": "gpt-5.4", "label": "GPT 5.4 Standard"}, + ]}, + ] + result = self._call(groups) + assert result[0]["models"][0]["label"] == "GPT 5.4 Turbo" + assert result[1]["models"][0]["label"] == "GPT 5.4 Standard (Codex)" + + def test_label_same_as_id_adds_provider_parenthetical(self): + """When label == bare_id, the disambiguated label becomes + 'model_id (Provider Name)'.""" + groups = [ + {"provider": "Edith", "provider_id": "custom:edith", "models": [ + {"id": "gpt-5.4", "label": "gpt-5.4"}, + ]}, + {"provider": "OpenAI Codex", "provider_id": "openai-codex", "models": [ + {"id": "gpt-5.4", "label": "gpt-5.4"}, + ]}, + ] + result = self._call(groups) + assert result[0]["models"][0]["label"] == "gpt-5.4" + assert result[1]["models"][0]["label"] == "gpt-5.4 (OpenAI Codex)" + + +class TestFrontendNormRegex(unittest.TestCase): + """Frontend: norm() function in static/ui.js strips @provider: prefix.""" + + @staticmethod + def _read_js(): + import pathlib + return (pathlib.Path(__file__).parent.parent / "static" / "ui.js").read_text() + + def _extract_norm(self): + """Extract the norm() lambda from ui.js source.""" + src = self._read_js() + # Find: const norm=s=>...; + import re + m = re.search(r"const norm=(s=>[^;]+);", src) + assert m, "norm() not found in ui.js" + return m.group(1) + + def test_norm_strips_nested_provider_prefix(self): + """norm('@custom:edith:gpt-5.4') === norm('gpt-5.4').""" + norm_js = self._extract_norm() + import subprocess + r1 = subprocess.run(["node", "-e", f"console.log(({norm_js})('gpt-5.4'))"], capture_output=True, text=True) + r2 = subprocess.run(["node", "-e", f"console.log(({norm_js})('@custom:edith:gpt-5.4'))"], capture_output=True, text=True) + assert r1.stdout.strip() == r2.stdout.strip(), f"{r1.stdout.strip()} != {r2.stdout.strip()}" + + def test_norm_strips_simple_provider_prefix(self): + """norm('@openai-codex:gpt-5.4') === norm('gpt-5.4').""" + norm_js = self._extract_norm() + import subprocess + r1 = subprocess.run(["node", "-e", f"console.log(({norm_js})('gpt-5.4'))"], capture_output=True, text=True) + r2 = subprocess.run(["node", "-e", f"console.log(({norm_js})('@openai-codex:gpt-5.4'))"], capture_output=True, text=True) + assert r1.stdout.strip() == r2.stdout.strip(), f"{r1.stdout.strip()} != {r2.stdout.strip()}" + + def test_norm_preserves_openrouter(self): + """norm('openai/gpt-5.4') === norm('gpt-5.4') still works.""" + norm_js = self._extract_norm() + import subprocess + r1 = subprocess.run(["node", "-e", f"console.log(({norm_js})('gpt-5.4'))"], capture_output=True, text=True) + r2 = subprocess.run(["node", "-e", f"console.log(({norm_js})('openai/gpt-5.4'))"], capture_output=True, text=True) + assert r1.stdout.strip() == r2.stdout.strip(), f"{r1.stdout.strip()} != {r2.stdout.strip()}" + + def test_norm_preserves_minimax_prefix(self): + """norm('@minimax:MiniMax-M2.7') === norm('minimax-m2.7') still works.""" + norm_js = self._extract_norm() + import subprocess + r1 = subprocess.run(["node", "-e", f"console.log(({norm_js})('minimax-m2.7'))"], capture_output=True, text=True) + r2 = subprocess.run(["node", "-e", f"console.log(({norm_js})('@minimax:MiniMax-M2.7'))"], capture_output=True, text=True) + assert r1.stdout.strip() == r2.stdout.strip(), f"{r1.stdout.strip()} != {r2.stdout.strip()}" diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index 317b3c72..14cd7701 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -458,7 +458,8 @@ def test_custom_endpoint_uses_model_config_api_key_for_model_discovery(monkeypat assert captured['ua'] == 'OpenAI/Python 1.0' groups = {g['provider']: [m['id'] for m in g['models']] for g in result['groups']} assert 'Custom' in groups - assert 'gpt-5.2' in groups['Custom'] + # Model ID may be prefixed with @provider: due to cross-provider dedup (#1228) + assert any('gpt-5.2' in m for m in groups['Custom']), f'gpt-5.2 not found in Custom: {groups}' # -- Issue #230: custom provider with slash model name -----------------------