fix: model picker snaps to wrong model with multi-slash IDs (#3360)

When a custom/proxy provider serves models whose IDs share the same base
name across vendor prefixes (e.g. vendor_a/deepseek/deepseek-v4-pro vs
vendor_b/deepseek/deepseek-v4-pro), several normalization functions use
split('/').pop() (or split('/')[-1]) which discards all segments except
the last.  This causes three user-facing symptoms: (1) clicking one
model selects a different colliding model, (2) configured-model badges
attach to the wrong dropdown entry, and (3) the model-chip label in the
composer bar is truncated to just the base model name.

Root cause: all three callers take only the last slash-segment instead
of stripping only the first (provider) segment and preserving the
remaining vendor hierarchy.

Fix 1 — _findModelInDropdown (static/ui.js): Move the exact string match
before the provider-aware normalized match.  Previously, when all models
share the same provider ID (common with LLM proxy setups), the normalized
match returned whichever colliding option appeared first in DOM order,
even though an exact match existed.

Fix 2 — _normalizeConfiguredModelKey (static/ui.js) and _norm_model_id
(api/config.py): Replace split('/').pop() / split('/')[-1] with a first-
segment-only strip (regex on frontend, split('/',1) on backend), matching
the strategy already used by _findModelInDropdown's norm lambda.  This
prevents multi-slash IDs from colliding in badge assignment and the
configured-entry dedup set.  Additionally, strip colon-qualified provider
prefixes (e.g. custom:name/) before the slash strip so badge-key variants
like 'custom:llm-proxy/opencode_go/model' merge correctly with the bare
'opencode_go/model' in the configured section dedup.

Fix 3 — getModelLabel (static/ui.js) and _get_label_for_model
(api/config.py): Same split('/').pop() to first-segment-strip change so
the composer-bar model chip and backend label preserve vendor context
(e.g. shows 'opencode_go/deepseek-v4-pro' instead of 'deepseek-v4-pro').

Verification: 9 new regression tests (test_issue3360) covering exact-
match priority, multi-slash normalization, and backend/frontend parity.
Updated 1 existing test (test_norm_model_id_trailing_empty_guard) that
asserted the old split('/').pop() pattern.  All 25 related tests pass.

AI Usage: Gemini (gemini-2.5-pro), via Antigravity IDE, pair-programmed.

(cherry picked from commit a454fecd2b3a83f7da34473c883be069171aeac9)
This commit is contained in:
b3nw
2026-06-01 20:46:13 +00:00
committed by nesquena-hermes
parent 866969161e
commit d06776a4c8
4 changed files with 322 additions and 20 deletions

View File

@@ -3076,9 +3076,9 @@ def _get_label_for_model(model_id: str, existing_groups: list) -> str:
if m.get("label") and _norm(str(m.get("id", ""))) == norm_lookup:
return m["label"]
# Fall back: capitalize each hyphen-separated word, preserve dots in version numbers.
# The catalog lookup above handles well-known models; this only fires for unlisted IDs.
bare = lookup_id.split("/")[-1] if "/" in lookup_id else lookup_id
# Fall back: strip only the first slash-segment (provider prefix),
# preserving vendor hierarchy for multi-slash IDs (#3360).
bare = lookup_id.split("/", 1)[1] if "/" in lookup_id else lookup_id
return " ".join(
w.upper() if (len(w) <= 3 and w.replace(".", "").isalnum() and not w.isdigit()) else w.capitalize()
for w in bare.replace("_", "-").split("-")
@@ -3231,11 +3231,14 @@ def get_available_models() -> dict:
if s.startswith("@") and ":" in s:
parts = s.split(":")
s = parts[-1] or s
# Strip provider/model prefix (e.g., custom:jingdong/GLM-5 -> GLM-5).
# Same trailing-empty guard.
# Strip only the first slash-segment (provider prefix), preserving
# any remaining vendor hierarchy. Using parts[-1] here previously
# discarded ALL segments except the last, collapsing distinct
# multi-slash IDs like 'vendor_a/deepseek-v4-pro' and
# 'vendor_b/deepseek/deepseek-v4-pro' to the same key (#3360).
if "/" in s:
parts = s.split("/")
s = parts[-1] or s
stripped = s.split("/", 1)[1]
s = stripped or s
return s.replace("-", ".")
def _build_configured_model_badges() -> dict[str, dict[str, str]]:

View File

@@ -1060,6 +1060,20 @@ function _findModelInDropdown(modelId, sel, preferredProviderId){
if(!modelId||!sel) return null;
const options=Array.from(sel.options);
const opts=options.map(o=>o.value);
// 0. Exact match — highest priority when it doesn't conflict with a
// cross-provider preference (#3360, guarded for #1228/#1313).
// When all models share the same provider (e.g. a custom proxy),
// normalization can collapse distinct multi-slash IDs to the same key
// and options.find() returns whichever appears first in the DOM instead
// of the exact value. But when the exact option belongs to a *different*
// provider than the preferred one, we must fall through to the provider-
// aware match so rehydration doesn't snap to the wrong provider row.
if(opts.includes(modelId)){
const exactOpt=options.find(o=>o.value===modelId);
const exactProv=exactOpt?_getOptionProviderId(exactOpt).toLowerCase():'';
const pref=String(preferredProviderId||'').toLowerCase();
if(!pref || !exactProv || exactProv===pref) return modelId;
}
// 1. Normalize: lowercase, strip namespace prefix, replace hyphens→dots.
// Also strip @provider: prefix from deduplicated model IDs (#1228, #1313).
const norm=s=>s.toLowerCase().replace(/^[^/]+\//,'').replace(/^@([^:]+:)+/,'').replace(/-/g,'.');
@@ -1074,8 +1088,7 @@ function _findModelInDropdown(modelId, sel, preferredProviderId){
const providerMatch=options.find(o=>norm(o.value)===target && _getOptionProviderId(o).toLowerCase()===preferred);
if(providerMatch) return providerMatch.value;
}
// 2. Exact match
if(opts.includes(modelId)) return modelId;
// 2. Normalized match
const exact=opts.find(o=>norm(o)===target);
if(exact) return exact;
// If the request is provider-qualified (either explicit @provider:model or
@@ -1418,7 +1431,21 @@ function _normalizeConfiguredModelKey(modelId){
// Defensive: trailing-colon / trailing-slash falls back to the original key
// so malformed configs don't collapse distinct ids to '' (matches backend _norm_model_id).
if(s.startsWith('@')&&s.includes(':')){const last=s.split(':').pop();s=last||s;}
if(s.includes('/')){const last=s.split('/').pop();s=last||s;}
// Strip provider-qualified prefixes that contain colons before the first
// slash (e.g. 'custom:llm-proxy/model' → 'model'). Without this, badge-
// key variants like 'custom:llm-proxy/opencode_go/deepseek-v4-pro' and the
// bare 'opencode_go/deepseek-v4-pro' produce different normalized keys and
// aren't deduped in the configured section (#3360).
if(s.includes('/')&&s.indexOf(':')!==-1&&s.indexOf(':')<s.indexOf('/')){
s=s.slice(s.indexOf('/')+1)||s;
}
// Strip only the first slash-segment (provider prefix), preserving any
// remaining vendor hierarchy. Using split('/').pop() here previously
// discarded ALL segments except the last, collapsing distinct multi-slash
// IDs like 'vendor_a/deepseek-v4-pro' and 'vendor_b/deepseek/deepseek-v4-pro'
// to the same key, causing badge misattribution and configured-entry
// suppression (#3360).
if(s.includes('/')) s=s.replace(/^[^/]+\//, '')||s;
return s.replace(/-/g,'.');
}
@@ -2860,7 +2887,7 @@ function getModelLabel(modelId){
if(rawId.startsWith('@custom:')){
const rest=rawId.slice('@custom:'.length);
if(rest.includes(':')) return rest.slice(rest.lastIndexOf(':')+1)||rawId;
if(rest.includes('/')) return rest.split('/').pop()||rawId;
if(rest.includes('/')) return rest.slice(rest.indexOf('/')+1)||rawId;
return rest||rawId;
}
// Check dynamic labels first, then fall back to splitting the ID
@@ -2868,8 +2895,9 @@ function getModelLabel(modelId){
// Static fallback for common models
const STATIC_LABELS={'openai/gpt-5.4-mini':'GPT-5.4 Mini','openai/gpt-4o':'GPT-4o','openai/o3':'o3','openai/o4-mini':'o4-mini','anthropic/claude-sonnet-4.6':'Sonnet 4.6','anthropic/claude-sonnet-4-5':'Sonnet 4.5','anthropic/claude-haiku-3-5':'Haiku 3.5','google/gemini-3.1-pro-preview':'Gemini 3.1 Pro','google/gemini-3-flash-preview':'Gemini 3 Flash','google/gemini-3.1-flash-lite-preview':'Gemini 3.1 Flash Lite','google/gemini-2.5-pro':'Gemini 2.5 Pro','google/gemini-2.5-flash':'Gemini 2.5 Flash','deepseek/deepseek-v4-flash':'DeepSeek V4 Flash','deepseek/deepseek-v4-pro':'DeepSeek V4 Pro','deepseek/deepseek-chat-v3-0324':'DeepSeek V3 (legacy)','meta-llama/llama-4-scout':'Llama 4 Scout'};
if(STATIC_LABELS[modelId]) return STATIC_LABELS[modelId];
// Safe Ollama-tag fallback formatter before generic split('/').pop()
let _last = modelId.split('/').pop() || modelId;
// Safe Ollama-tag fallback: strip only the first slash-segment (provider
// prefix) so multi-slash IDs preserve their vendor hierarchy (#3360).
let _last = modelId.includes('/') ? (modelId.slice(modelId.indexOf('/')+1) || modelId) : modelId;
// Strip @provider: prefix if present (e.g. @ollama-cloud:kimi-k2.6)
if (_last.startsWith('@') && _last.includes(':')) _last = _last.split(':').slice(1).join(':');
const looksLikeOllamaTag = /^[a-z0-9][\w.-]*:[\w.-]+$/i.test(_last);

View File

@@ -0,0 +1,263 @@
"""
Regression tests for #3360 — multi-slash model ID collisions in the
model picker.
Two bugs:
1. ``_findModelInDropdown`` provider-aware match (L1074) runs BEFORE the
exact match (L1078). When multiple options from the same proxy provider
normalize identically (e.g. ``vendor_a/deepseek/deepseek-v4-pro`` and
``vendor_b/deepseek/deepseek-v4-pro`` both → ``deepseek/deepseek.v4.pro``),
``options.find()`` returns whichever appears first in DOM order. Fix:
move the exact match to the top of the function.
2. ``_normalizeConfiguredModelKey`` uses ``split('/').pop()`` which takes
only the last segment, collapsing multi-slash IDs to the same key as
single-slash configured models. Fix: strip only the first segment via
``replace(/^[^/]+\\//, '')``. Backend ``_norm_model_id`` mirrors the
same fix.
Tests run the live JS functions via Node and the live Python function via
exec, so drift between the test and the real code is caught immediately.
"""
import json
import shutil
import subprocess
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).parent.parent.resolve()
UI_JS_PATH = REPO_ROOT / "static" / "ui.js"
CONFIG_PY = (REPO_ROOT / "api" / "config.py").read_text(encoding="utf-8")
NODE = shutil.which("node")
pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH")
# ── JS driver for _findModelInDropdown ──────────────────────────────────────
_FIND_MODEL_DRIVER = r"""
const fs = require('fs');
const ui = fs.readFileSync(process.argv[2], 'utf8');
function extractFunc(name) {
const re = new RegExp('function\\s+' + name + '\\s*\\(');
const start = ui.search(re);
if (start < 0) throw new Error(name + ' not found');
let i = ui.indexOf('{', start); let depth = 1; i++;
while (depth > 0 && i < ui.length) { if (ui[i]==='{') depth++; else if (ui[i]==='}') depth--; i++; }
return ui.slice(start, i);
}
function _getOptionProviderId(opt) {
if (!opt) return '';
if (opt.dataset && opt.dataset.provider) return opt.dataset.provider;
const group = opt.parentElement;
if (group && group.tagName === 'OPTGROUP' && group.dataset && group.dataset.provider) return group.dataset.provider;
const value = String(opt.value || '');
if (value.startsWith('@') && value.includes(':')) return value.slice(1, value.lastIndexOf(':'));
return '';
}
eval(extractFunc('_findModelInDropdown'));
const args = JSON.parse(process.argv[3]);
const sel = {
options: args.options.map(v => {
const opt = {value: v.value || v, dataset: {}};
if (v.provider) opt.dataset.provider = v.provider;
// Simulate optgroup parent for provider detection
if (v.provider) {
opt.parentElement = {tagName: 'OPTGROUP', dataset: {provider: v.provider}};
}
return opt;
})
};
const got = _findModelInDropdown(args.modelId, sel, args.preferredProvider || undefined);
process.stdout.write(JSON.stringify(got));
"""
# ── JS driver for _normalizeConfiguredModelKey ──────────────────────────────
_NORM_KEY_DRIVER = r"""
const fs = require('fs');
const ui = fs.readFileSync(process.argv[2], 'utf8');
function extractFunc(name) {
const re = new RegExp('function\\s+' + name + '\\s*\\(');
const start = ui.search(re);
if (start < 0) throw new Error(name + ' not found');
let i = ui.indexOf('{', start); let depth = 1; i++;
while (depth > 0 && i < ui.length) { if (ui[i]==='{') depth++; else if (ui[i]==='}') depth--; i++; }
return ui.slice(start, i);
}
eval(extractFunc('_normalizeConfiguredModelKey'));
const ids = JSON.parse(process.argv[3]);
const result = {};
for (const id of ids) { result[id] = _normalizeConfiguredModelKey(id); }
process.stdout.write(JSON.stringify(result));
"""
@pytest.fixture(scope="module")
def find_driver(tmp_path_factory):
p = tmp_path_factory.mktemp("find_driver") / "driver.js"
p.write_text(_FIND_MODEL_DRIVER, encoding="utf-8")
return str(p)
@pytest.fixture(scope="module")
def norm_driver(tmp_path_factory):
p = tmp_path_factory.mktemp("norm_driver") / "driver.js"
p.write_text(_NORM_KEY_DRIVER, encoding="utf-8")
return str(p)
def _find(driver_path, model_id, options, preferred=None):
result = subprocess.run(
[NODE, driver_path, str(UI_JS_PATH),
json.dumps({"modelId": model_id, "options": options, "preferredProvider": preferred})],
capture_output=True, text=True, timeout=10,
)
if result.returncode != 0:
raise RuntimeError(f"node driver failed: {result.stderr}")
return json.loads(result.stdout)
def _norm_keys(driver_path, ids):
result = subprocess.run(
[NODE, driver_path, str(UI_JS_PATH), json.dumps(ids)],
capture_output=True, text=True, timeout=10,
)
if result.returncode != 0:
raise RuntimeError(f"node driver failed: {result.stderr}")
return json.loads(result.stdout)
def _backend_norm():
"""Extract and exec the backend _norm_model_id function."""
start_marker = "def _norm_model_id(model_id: str) -> str:"
end_marker = "def _build_configured_model_badges"
s = CONFIG_PY.find(start_marker)
e = CONFIG_PY.find(end_marker, s)
assert s != -1 and e != -1
body = CONFIG_PY[s:e]
lines = body.splitlines()
indent = None
for ln in lines:
if ln.strip():
indent = len(ln) - len(ln.lstrip())
break
dedented = "\n".join(ln[indent:] if len(ln) >= indent else ln for ln in lines)
ns = {}
exec(dedented, ns)
return ns["_norm_model_id"]
# ═══════════════════════════════════════════════════════════════════════════
# Fix 1: _findModelInDropdown — exact match must beat provider-aware match
# ═══════════════════════════════════════════════════════════════════════════
class TestFindModelExactMatchPriority:
"""When the exact model ID exists as an option value, return it
regardless of normalization collisions with other options."""
def test_exact_match_beats_normalized_collision_same_provider(self, find_driver):
"""Core #3360 regression: two multi-slash IDs from the same proxy
provider normalize identically. The clicked value must be returned."""
options = [
{"value": "nanogpt/deepseek/deepseek-v4-pro", "provider": "llm-proxy"},
{"value": "command/deepseek/deepseek-v4-pro", "provider": "llm-proxy"},
]
got = _find(find_driver, "command/deepseek/deepseek-v4-pro", options, "llm-proxy")
assert got == "command/deepseek/deepseek-v4-pro", (
f"Expected exact match for command/deepseek/deepseek-v4-pro, got {got!r}"
)
def test_exact_match_beats_dom_order(self, find_driver):
"""Even when the clicked option is NOT first in DOM order, the
exact match must still win."""
options = [
{"value": "alpha/deepseek/deepseek-v4-pro", "provider": "proxy"},
{"value": "beta/deepseek/deepseek-v4-pro", "provider": "proxy"},
{"value": "gamma/deepseek/deepseek-v4-pro", "provider": "proxy"},
]
# Click the last one
got = _find(find_driver, "gamma/deepseek/deepseek-v4-pro", options, "proxy")
assert got == "gamma/deepseek/deepseek-v4-pro"
def test_exact_match_single_slash_still_works(self, find_driver):
"""Single-slash IDs that exist as options must still resolve."""
options = [
{"value": "openai/gpt-5.5", "provider": "openai"},
{"value": "openai/gpt-5.4-mini", "provider": "openai"},
]
got = _find(find_driver, "openai/gpt-5.5", options, "openai")
assert got == "openai/gpt-5.5"
# ═══════════════════════════════════════════════════════════════════════════
# Fix 2: _normalizeConfiguredModelKey — multi-slash IDs must not collide
# ═══════════════════════════════════════════════════════════════════════════
class TestNormalizeConfiguredModelKeyMultiSlash:
"""After the fix, multi-slash IDs preserve vendor hierarchy and do
not collide with single-slash or bare IDs."""
def test_multi_slash_preserves_vendor_segment(self, norm_driver):
keys = _norm_keys(norm_driver, [
"vendor_a/deepseek-v4-pro",
"vendor_b/deepseek/deepseek-v4-pro",
])
assert keys["vendor_a/deepseek-v4-pro"] == "deepseek.v4.pro"
assert keys["vendor_b/deepseek/deepseek-v4-pro"] == "deepseek/deepseek.v4.pro"
assert keys["vendor_a/deepseek-v4-pro"] != keys["vendor_b/deepseek/deepseek-v4-pro"], (
"Single-slash and multi-slash IDs must not collide"
)
def test_single_slash_behavior_unchanged(self, norm_driver):
keys = _norm_keys(norm_driver, [
"openai/gpt-5.5",
"anthropic/claude-opus-4.6",
])
assert keys["openai/gpt-5.5"] == "gpt.5.5"
assert keys["anthropic/claude-opus-4.6"] == "claude.opus.4.6"
def test_bare_model_unchanged(self, norm_driver):
keys = _norm_keys(norm_driver, ["deepseek-v4-pro"])
assert keys["deepseek-v4-pro"] == "deepseek.v4.pro"
def test_at_provider_prefix_still_stripped(self, norm_driver):
keys = _norm_keys(norm_driver, ["@custom:jingdong:GLM-5"])
assert keys["@custom:jingdong:GLM-5"] == "glm.5"
def test_trailing_slash_fallback(self, norm_driver):
"""A trailing slash (malformed) must not collapse to empty."""
keys = _norm_keys(norm_driver, ["provider/"])
assert keys["provider/"] != "", "Trailing slash collapsed to empty string"
# ═══════════════════════════════════════════════════════════════════════════
# Backend / frontend parity
# ═══════════════════════════════════════════════════════════════════════════
class TestBackendFrontendNormParity:
"""The Python _norm_model_id must produce the same output as the
JS _normalizeConfiguredModelKey for identical inputs."""
def test_parity_multi_slash(self, norm_driver):
ids = [
"deepseek-v4-pro",
"vendor_a/deepseek-v4-pro",
"vendor_b/deepseek/deepseek-v4-pro",
"@custom:jingdong:GLM-5",
]
js_keys = _norm_keys(norm_driver, ids)
py_norm = _backend_norm()
for model_id in ids:
py_result = py_norm(model_id)
js_result = js_keys[model_id]
assert py_result == js_result, (
f"Parity mismatch for {model_id!r}: "
f"Python={py_result!r}, JS={js_result!r}"
)

View File

@@ -71,10 +71,18 @@ def test_norm_model_id_simple_inputs_unchanged():
def test_ui_js_mirror_has_trailing_empty_guard():
"""Frontend _normalizeConfiguredModelKey must mirror the backend guard."""
# The new pattern uses `const last=s.split(':').pop();s=last||s;`
assert "s.split(':').pop()" in UI_JS, "ui.js no longer uses split-pop pattern"
# Look for the `||s` fallback specifically
snippet = UI_JS[UI_JS.find("function _normalizeConfiguredModelKey"):UI_JS.find("function _normalizeConfiguredModelKey") + 600]
assert "last||s" in snippet, "ui.js missing trailing-empty guard `||s` fallback"
# And mirror on / branch
assert snippet.count("last||s") >= 2, "ui.js trailing-empty guard not mirrored on slash branch"
# The colon branch still uses `const last=s.split(':').pop();s=last||s;`
assert "s.split(':').pop()" in UI_JS, "ui.js no longer uses split-pop pattern for colon branch"
# Look for the `||s` fallback on the colon branch
snippet = UI_JS[UI_JS.find("function _normalizeConfiguredModelKey"):UI_JS.find("function _normalizeConfiguredModelKey") + 1800]
assert "last||s" in snippet, "ui.js missing trailing-empty guard `||s` fallback on colon branch"
# The slash branch now uses replace(/^[^/]+\//, '') instead of split('/').pop()
# to preserve multi-slash vendor hierarchy (#3360). Verify the new pattern
# and its trailing-empty guard (the `||s` suffix).
assert "replace(/^[^/]+\\/" in snippet, (
"ui.js slash branch should use replace(/^[^/]+\\//) pattern (#3360)"
)
assert "'')||s" in snippet, (
"ui.js slash branch should have ||s trailing-empty guard (#3360)"
)