b3nw
d06776a4c8
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)
89 lines
3.7 KiB
Python
89 lines
3.7 KiB
Python
"""Opus pre-release follow-up for stage-267:
|
|
|
|
#1454/#1474 split(':')[-1] trailing-empty guard — when a malformed configured model id
|
|
has a trailing colon (e.g. `@custom:foo:bar:`), the new normalization would collapse
|
|
two distinct ids to the empty string. Defensive `parts[-1] or s` falls back to the
|
|
original input so distinct ids stay distinct in the configured-model badge filter.
|
|
|
|
Mirrors:
|
|
api/config.py _norm_model_id
|
|
static/ui.js _normalizeConfiguredModelKey
|
|
"""
|
|
from pathlib import Path
|
|
|
|
REPO_ROOT = Path(__file__).parent.parent
|
|
CONFIG_PY = (REPO_ROOT / "api" / "config.py").read_text(encoding="utf-8")
|
|
UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8")
|
|
|
|
|
|
def _exec_norm():
|
|
"""Re-execute the _norm_model_id closure body via a synthetic def, returning the function."""
|
|
# Extract source between `def _norm_model_id(model_id: str) -> str:` and the next `def _build_configured_model_badges`
|
|
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]
|
|
# Dedent (it's nested 8 spaces inside a function)
|
|
lines = body.splitlines()
|
|
# Find first non-blank line indent
|
|
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"]
|
|
|
|
|
|
def test_norm_model_id_trailing_colon_keeps_original():
|
|
"""Malformed @provider: ids with trailing colon must not collapse to empty."""
|
|
norm = _exec_norm()
|
|
# Trailing colon — last split segment is empty, must fall back to original
|
|
out = norm("@custom:foo:bar:")
|
|
assert out, f"trailing-colon collapsed to empty: {out!r}"
|
|
|
|
|
|
def test_norm_model_id_clean_multi_segment_strips_correctly():
|
|
"""Clean @custom:vendor:model still strips to last segment (the new fix's purpose)."""
|
|
norm = _exec_norm()
|
|
assert norm("@custom:jingdong:GLM-5") == "glm.5"
|
|
|
|
|
|
def test_norm_model_id_trailing_slash_keeps_original():
|
|
"""Same guard on the / branch — trailing slash must not collapse to empty."""
|
|
norm = _exec_norm()
|
|
out = norm("custom/jingdong/")
|
|
assert out, f"trailing-slash collapsed to empty: {out!r}"
|
|
|
|
|
|
def test_norm_model_id_simple_inputs_unchanged():
|
|
"""Sanity: simple inputs round-trip as before."""
|
|
norm = _exec_norm()
|
|
assert norm("gpt-4") == "gpt.4"
|
|
assert norm("provider/model-name") == "model.name"
|
|
assert norm("") == ""
|
|
assert norm(None) == ""
|
|
|
|
|
|
def test_ui_js_mirror_has_trailing_empty_guard():
|
|
"""Frontend _normalizeConfiguredModelKey must mirror the backend guard."""
|
|
# 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)"
|
|
)
|
|
|