fix(models): deduplicate model IDs across provider groups (#1228)
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
This commit is contained in:
bergeouss
Hermes Agent
parent
0741a2ab9f
commit
a8101d98f7
4 changed files with 280 additions and 2 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
217
tests/test_issue1228_model_picker_duplicate_ids.py
Normal file
217
tests/test_issue1228_model_picker_duplicate_ids.py
Normal file
|
|
@ -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()}"
|
||||
|
|
@ -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 -----------------------
|
||||
|
|
|
|||
Loading…
Reference in a new issue