fix(ui): restore "Selected" row for bare custom model ids (#6884)
After the send path emits the bare model id (e.g. "model-a:free"), the restore/refresh path compared the qualified catalog row value (@custom:hetmer.net:model-a:free) against the bare state model, so no row rendered as active/"Selected". Normalize both sides through _modelPickerOptionIdentity (the same identity used for picker dedup) and keep the provider equality gate, so exactly one row is restored. Adds a round-trip regression test (send -> persist -> restore) with a colon-bearing custom model id asserting exactly one "Selected" row with the model intact, and updates the #5748 source-assertions to the normalized comparison.
This commit is contained in:
webtecnica committed
1 parent
c82c6a542a
commit
cef2521f2a
3 files changed
+299
-4
No files matched your search
+17
-1
@@ -4431,7 +4431,23 @@ function renderModelDropdown(){
|
||||
const _provider=String((m&&m.providerId)||(m&&m.badge&&m.badge.provider)||((typeof _providerFromModelValue==='function')?_providerFromModelValue(m&&m.value):'')||'').trim();
|
||||
return (_provider&&_provider!=='default')?_provider:null;
|
||||
};
|
||||
const _isSelectedModelRow=(m)=>String((m&&m.value)||'')===String((_selectedModelState&&_selectedModelState.model)||(sel&&sel.value)||'')&&String(_modelProviderForSelectedBadge(m)||'')===String((_selectedModelState&&_selectedModelState.model_provider)||'');
|
||||
const _isSelectedModelRow=(m)=>{
|
||||
const _rowModel=String((m&&m.value)||'');
|
||||
const _rowProvider=String(_modelProviderForSelectedBadge(m)||'');
|
||||
const _stateModel=String((_selectedModelState&&_selectedModelState.model)||(sel&&sel.value)||'');
|
||||
const _stateProvider=String((_selectedModelState&&_selectedModelState.model_provider)||'');
|
||||
// Normalize both sides to the same model/provider identity. Catalog rows
|
||||
// carry the qualified @custom:<slug>:<model> value while the outgoing
|
||||
// state model is bare (#6884) — a raw string comparison would leave no
|
||||
// row marked active/"Selected" after a restore. _modelPickerOptionIdentity
|
||||
// is the same identity used for picker dedup, so the row that survives is
|
||||
// exactly the one the send path resolves.
|
||||
const _norm=(model,provider)=>typeof _modelPickerOptionIdentity==='function'
|
||||
?_modelPickerOptionIdentity(model,provider)
|
||||
:String(model||'');
|
||||
return _norm(_rowModel,_rowProvider)===_norm(_stateModel,_stateProvider)
|
||||
&&_rowProvider===_stateProvider;
|
||||
};
|
||||
const _selectedModelBadge=(m)=>_isSelectedModelRow(m)
|
||||
?`<span class="model-opt-badge model-opt-badge--selected">${esc(t('model_badge_selected')||'Selected')}</span>`
|
||||
:'';
|
||||
|
||||
@@ -278,6 +278,275 @@ def test_non_default_named_custom_provider_model_strips_qualified_prefix():
|
||||
}
|
||||
|
||||
|
||||
# Round-trip: send -> persist -> restore for a colon-bearing named
|
||||
# custom-provider model. The send path emits the bare id ("model-a:free");
|
||||
# on restore, _findModelInDropdown reverse-matches the bare id back to the
|
||||
# qualified catalog option ("@custom:hetmer.net:model-a:free") and the
|
||||
# dropdown re-renders. _isSelectedModelRow must normalize both sides to the
|
||||
# same model/provider identity so exactly ONE row renders as active /
|
||||
# "Selected" — with the model id intact, not truncated.
|
||||
_ROUND_TRIP_DRIVER = r"""
|
||||
const fs = require('fs');
|
||||
const ui = fs.readFileSync(process.argv[2], 'utf8');
|
||||
|
||||
function extractFunc(name) {
|
||||
const re = new RegExp('(?:async\\s+)?function\\s+' + name + '\\s*\\(');
|
||||
const start = ui.search(re);
|
||||
if (start < 0) throw new Error(name + ' not found');
|
||||
let openParen = ui.indexOf('(', start);
|
||||
let i = openParen + 1;
|
||||
let parenDepth = 1;
|
||||
while (parenDepth > 0 && i < ui.length) {
|
||||
if (ui[i] === '(') parenDepth++;
|
||||
else if (ui[i] === ')') parenDepth--;
|
||||
i++;
|
||||
}
|
||||
i = ui.indexOf('{', i);
|
||||
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 makeClassList(initial) {
|
||||
const set = new Set(initial || []);
|
||||
return {
|
||||
_set: set,
|
||||
add(cls) { set.add(cls); },
|
||||
remove(cls) { set.delete(cls); },
|
||||
contains(cls) { return set.has(cls); },
|
||||
toggle(cls, force) {
|
||||
if (force === true) { set.add(cls); return true; }
|
||||
if (force === false) { set.delete(cls); return false; }
|
||||
if (set.has(cls)) { set.delete(cls); return false; }
|
||||
set.add(cls);
|
||||
return true;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function defineClassName(node) {
|
||||
Object.defineProperty(node, 'className', {
|
||||
get() { return [...node.classList._set].join(' '); },
|
||||
set(v) { node.classList = makeClassList(String(v || '').split(/\s+/).filter(Boolean)); },
|
||||
});
|
||||
}
|
||||
|
||||
function makeNode(tag) {
|
||||
const node = {
|
||||
tagName: String(tag || '').toUpperCase(),
|
||||
children: [],
|
||||
dataset: {},
|
||||
style: {},
|
||||
parentElement: null,
|
||||
textContent: '',
|
||||
value: '',
|
||||
tabIndex: 0,
|
||||
onclick: null,
|
||||
_listeners: {},
|
||||
_innerHTML: '',
|
||||
appendChild(child) {
|
||||
child.parentElement = this;
|
||||
this.children.push(child);
|
||||
if (this.tagName === 'OPTGROUP' && this._ownerSelect && child.tagName === 'OPTION') {
|
||||
this._ownerSelect.options.push(child);
|
||||
}
|
||||
return child;
|
||||
},
|
||||
addEventListener(type, handler) { this._listeners[type] = handler; },
|
||||
querySelector(selector) { return this._qs ? this._qs[selector] || null : null; },
|
||||
setAttribute(name, value) { this[name] = value; },
|
||||
focus() { this._focused = true; },
|
||||
};
|
||||
node.classList = makeClassList();
|
||||
defineClassName(node);
|
||||
Object.defineProperty(node, 'innerHTML', {
|
||||
get() { return this._innerHTML; },
|
||||
set(v) {
|
||||
this._innerHTML = String(v || '');
|
||||
this.children = [];
|
||||
this._qs = {};
|
||||
if (this.tagName === 'DIV' && this._innerHTML.includes('model-search-input')) {
|
||||
const input = makeNode('input');
|
||||
input.className = 'model-search-input';
|
||||
const clear = makeNode('button');
|
||||
clear.className = 'model-search-clear';
|
||||
this._qs['.model-search-input'] = input;
|
||||
this._qs['.model-search-clear'] = clear;
|
||||
} else if (this.tagName === 'DIV' && this._innerHTML.includes('model-custom-input')) {
|
||||
const input = makeNode('input');
|
||||
input.className = 'model-custom-input';
|
||||
const btn = makeNode('button');
|
||||
btn.className = 'model-custom-btn';
|
||||
this._qs['.model-custom-input'] = input;
|
||||
this._qs['.model-custom-btn'] = btn;
|
||||
}
|
||||
},
|
||||
});
|
||||
return node;
|
||||
}
|
||||
|
||||
function makeOption(value, label, parent) {
|
||||
const opt = makeNode('option');
|
||||
opt.value = value;
|
||||
opt.textContent = label || value;
|
||||
opt.parentElement = parent || null;
|
||||
return opt;
|
||||
}
|
||||
|
||||
function makeSelect(groups, selectedValue) {
|
||||
const sel = { id: 'modelSelect', children: [], options: [], _value: selectedValue || '' };
|
||||
Object.defineProperty(sel, 'value', {get(){return sel._value;}, set(v){sel._value=String(v||'');}});
|
||||
Object.defineProperty(sel, 'selectedOptions', {get(){const o=sel.options.find(x=>x.value===sel._value);return o?[o]:[];}});
|
||||
sel.appendChild=function(option){option.parentElement=null;sel.options.push(option);};
|
||||
sel.querySelectorAll=function(){return [];};
|
||||
for (const group of groups || []) {
|
||||
const og = makeNode('optgroup');
|
||||
og.label = group.provider || '';
|
||||
og.dataset.provider = group.provider_id || '';
|
||||
og._ownerSelect = sel;
|
||||
for (const model of group.models || []) og.appendChild(makeOption(model.id, model.label || model.id, og));
|
||||
sel.children.push(og);
|
||||
sel.options.push(...og.children);
|
||||
}
|
||||
return sel;
|
||||
}
|
||||
|
||||
function snapshot(dd) {
|
||||
const out = [];
|
||||
const walk = (node) => {
|
||||
for (const child of (node.children || [])) {
|
||||
out.push({
|
||||
className: child.className,
|
||||
textContent: child.textContent,
|
||||
html: child._innerHTML || '',
|
||||
});
|
||||
if (child.children && child.children.length) walk(child);
|
||||
}
|
||||
};
|
||||
walk(dd);
|
||||
return out;
|
||||
}
|
||||
|
||||
const payload = JSON.parse(process.argv[3]);
|
||||
const dropdown = makeNode('div');
|
||||
dropdown.classList.add('open');
|
||||
const modelSelect = makeSelect(payload.groups, payload.selectedValue || payload.groups[0].models[0].id);
|
||||
|
||||
function $(id) {
|
||||
if (id === 'composerModelDropdown') return dropdown;
|
||||
if (id === 'modelSelect') return modelSelect;
|
||||
return null;
|
||||
}
|
||||
const window = { _configuredModelBadges: payload.configuredBadges || {} };
|
||||
const document = { createElement(tag) { return makeNode(tag); } };
|
||||
function esc(v) { return String(v || ''); }
|
||||
function t(key, ...args) {
|
||||
if (key === 'model_show_all_models') return `Show all ${args[0]} models`;
|
||||
return key;
|
||||
}
|
||||
function li() { return 'x'; }
|
||||
function getModelLabel(v) { return String(v || ''); }
|
||||
function _providerFromModelValue(v) {
|
||||
const value = String(v || '');
|
||||
if (value.startsWith('@') && value.includes(':')) return value.slice(1, value.lastIndexOf(':'));
|
||||
return '';
|
||||
}
|
||||
function _normalizeConfiguredModelKey(v) { return String(v || '').toLowerCase(); }
|
||||
function _getConfiguredModelBadge(value, badgeMap) { return badgeMap[value] || null; }
|
||||
function closeModelDropdown() {}
|
||||
function syncModelChip() {}
|
||||
function _refreshOpenModelDropdown() {}
|
||||
function _deduplicateModelPickerOptions() { return 0; }
|
||||
async function selectModelFromDropdown(value, provider) {
|
||||
_ensureModelOptionInDropdown(value, modelSelect, provider);
|
||||
window.__picked=_modelStateForSelect(modelSelect,modelSelect.value);
|
||||
}
|
||||
|
||||
for (const name of [
|
||||
'_modelPickerOptionIdentity',
|
||||
'_readModelOverflowData',
|
||||
'_appendOverflowOptionsToGroup',
|
||||
'_isEquivalentConfiguredModelEntry',
|
||||
'_getOptionProviderId',
|
||||
'_modelStateForSelect',
|
||||
'_findModelInDropdown',
|
||||
'_applyModelToDropdown',
|
||||
'_ensureModelOptionInDropdown',
|
||||
'renderModelDropdown',
|
||||
]) {
|
||||
eval(extractFunc(name));
|
||||
}
|
||||
|
||||
// Send path: the picked state carries the bare, intact model id.
|
||||
const sent = _modelStateForSelect(modelSelect, '@custom:hetmer.net:model-a:free');
|
||||
// Restore path: _findModelInDropdown reverse-matches the bare id to the
|
||||
// qualified catalog option, the picker restores sel.value to it and
|
||||
// re-renders the dropdown.
|
||||
const restoreValue = _findModelInDropdown(sent.model, modelSelect, sent.model_provider);
|
||||
modelSelect.value = restoreValue;
|
||||
renderModelDropdown();
|
||||
const rows = snapshot(dropdown).filter(n => String(n.className || '').includes('model-opt'));
|
||||
const activeRows = rows.filter(n => String(n.className || '').includes(' active'));
|
||||
process.stdout.write(JSON.stringify({
|
||||
sent,
|
||||
restoreValue,
|
||||
activeCount: activeRows.length,
|
||||
activeRow: activeRows.length ? {
|
||||
className: activeRows[0].className,
|
||||
text: activeRows[0].textContent,
|
||||
html: activeRows[0].html,
|
||||
} : null,
|
||||
}));
|
||||
"""
|
||||
|
||||
|
||||
@pytest.mark.skipif(NODE is None, reason="node not on PATH")
|
||||
def test_colon_bearing_custom_provider_round_trip_restores_selected_row(tmp_path):
|
||||
driver = tmp_path / "round_trip_driver.js"
|
||||
driver.write_text(_ROUND_TRIP_DRIVER, encoding="utf-8")
|
||||
payload = {
|
||||
"groups": [
|
||||
{
|
||||
"provider": "hetmer.net",
|
||||
"provider_id": "custom:hetmer.net",
|
||||
"models": [
|
||||
{"id": "luna", "label": "luna"},
|
||||
{"id": "@custom:hetmer.net:model-a:free", "label": "model-a:free"},
|
||||
],
|
||||
}
|
||||
],
|
||||
"configuredBadges": {},
|
||||
"selectedValue": "luna",
|
||||
}
|
||||
assert NODE is not None
|
||||
result = subprocess.run(
|
||||
[NODE, str(driver), str(UI_JS), json.dumps(payload)],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=30,
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
actual = json.loads(result.stdout)
|
||||
|
||||
# Send path: bare model id, intact (NOT truncated to "free").
|
||||
assert actual["sent"] == {
|
||||
"model": "model-a:free",
|
||||
"model_provider": "custom:hetmer.net",
|
||||
}
|
||||
# Restore path: the bare id maps back to the qualified catalog option.
|
||||
assert actual["restoreValue"] == "@custom:hetmer.net:model-a:free"
|
||||
# Exactly one row renders as active/"Selected", with the intact model id.
|
||||
assert actual["activeCount"] == 1, actual
|
||||
assert "model-opt-badge--selected" in actual["activeRow"]["html"]
|
||||
assert "model-a:free" in actual["activeRow"]["html"]
|
||||
assert ">free<" not in actual["activeRow"]["html"].replace("model-a:free", "")
|
||||
|
||||
|
||||
_RENDERED_CLICK_DRIVER = r"""
|
||||
|
||||
const fs = require('fs');
|
||||
|
||||
@@ -34,15 +34,25 @@ def test_model_picker_renders_selected_badge_without_replacing_configured_badge(
|
||||
|
||||
|
||||
def test_selected_badge_is_keyed_to_current_model_value():
|
||||
assert "String((m&&m.value)||'')===String((_selectedModelState&&_selectedModelState.model)||(sel&&sel.value)||'')" in UI_JS
|
||||
# The selected badge must key on the resolved model identity, not a raw
|
||||
# string of the dropdown value: catalog rows carry the qualified
|
||||
# @custom:<slug>:<model> value while the outgoing state model is bare
|
||||
# (#6884), so both sides are normalized via _modelPickerOptionIdentity
|
||||
# before comparison.
|
||||
assert "const _norm=(model,provider)=>typeof _modelPickerOptionIdentity==='function'" in UI_JS
|
||||
assert "_norm(_rowModel,_rowProvider)===_norm(_stateModel,_stateProvider)" in UI_JS
|
||||
|
||||
|
||||
def test_selected_badge_is_keyed_to_current_model_provider():
|
||||
assert "const _selectedModelState=(typeof _modelStateForSelect==='function')?_modelStateForSelect(sel,sel.value)" in UI_JS
|
||||
assert "const _modelProviderForSelectedBadge=(m)=>" in UI_JS
|
||||
assert "return (_provider&&_provider!=='default')?_provider:null;" in UI_JS
|
||||
assert "String(_modelProviderForSelectedBadge(m)||'')===String((_selectedModelState&&_selectedModelState.model_provider)||'')" in UI_JS
|
||||
assert "const _isSelectedModelRow=(m)=>" in UI_JS
|
||||
# Provider identity still gates the row match — the normalized model
|
||||
# comparison alone would collapse same-id rows across provider groups.
|
||||
assert "_rowProvider=String(_modelProviderForSelectedBadge(m)||'')" in UI_JS
|
||||
assert "_stateProvider=String((_selectedModelState&&_selectedModelState.model_provider)||'')" in UI_JS
|
||||
assert "_rowProvider===_stateProvider" in UI_JS
|
||||
assert "const _isSelectedModelRow=(m)=>{" in UI_JS
|
||||
assert "row.className='model-opt'+(_isSelectedModelRow(m)?' active':'');" in UI_JS
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user