From 92de761579a9b0e2fa7862cf0e03ecc0a0e2c8b2 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 20 Jun 2026 01:56:25 -0400 Subject: [PATCH] Avoid duplicate picker rebuilds when live models arrive --- static/ui.js | 11 +- ...est_issue1771_session_model_switch_sync.py | 128 ++++++++++++++++-- 2 files changed, 127 insertions(+), 12 deletions(-) diff --git a/static/ui.js b/static/ui.js index d8a7689e4..6fb23a231 100644 --- a/static/ui.js +++ b/static/ui.js @@ -2060,14 +2060,21 @@ function _addLiveModelsToSelect(provider, models, sel){ _dynamicModelLabels[mid]=m.label||m.id; added++; } - const currentProvider=(S.session&&S.session.model_provider)||null; + const currentState=(currentVal&&typeof _modelStateForSelect==='function') + ? _modelStateForSelect(sel, currentVal) + : {model:currentVal||'', model_provider:(S.session&&S.session.model_provider)||null}; + const currentProvider=currentState&¤tState.model_provider||null; if(added>0 && currentVal) _applyModelToDropdown(currentVal, sel, currentProvider, {forceRefresh:true}); // After live models are added, re-apply the session's model in case it was // absent from the static list and syncTopbar() fired before the live fetch // completed (#1169). This ensures the session model wins over any premature // fallback that may have set sel.value to the first available option. if(S.session && S.session.model && sel.id==='modelSelect'){ - const reapplied=_applyModelToDropdown(S.session.model, sel, S.session.model_provider||null, {forceRefresh:added>0}); + const sessionProvider=S.session.model_provider||null; + const sessionAlreadyRefreshed=added>0 && currentVal + && String((currentState&¤tState.model)||'')===String(S.session.model||'') + && String((currentState&¤tState.model_provider)||'')===String(sessionProvider||''); + const reapplied=_applyModelToDropdown(S.session.model, sel, sessionProvider, {forceRefresh:added>0&&!sessionAlreadyRefreshed}); if(reapplied && typeof syncModelChip==='function') syncModelChip(); } return added; diff --git a/tests/test_issue1771_session_model_switch_sync.py b/tests/test_issue1771_session_model_switch_sync.py index 3eeb9226e..7d03d9023 100644 --- a/tests/test_issue1771_session_model_switch_sync.py +++ b/tests/test_issue1771_session_model_switch_sync.py @@ -44,12 +44,39 @@ function extractFunc(name, opts = {}) { } const calls = {syncModelChip: 0, renderModelDropdown: 0, positionModelDropdown: 0, fetches: []}; +const _dynamicModelLabels = {}; let modelSelect; let dropdownOpen = false; const dropdown = {classList: {contains: (name) => name === 'open' && dropdownOpen}}; +function makeOptGroup(provider, label) { + return { + tagName: 'OPTGROUP', + label: label || '', + dataset: {provider: provider || ''}, + children: [], + appendChild(opt) { + opt.parentElement = this; + this.children.push(opt); + if (this._select && !this._select.options.includes(opt)) { + this._select.options.push(opt); + } + } + }; +} + +function makeOption(value, label, provider) { + return { + value, + textContent: label || value, + dataset: provider ? {provider} : {}, + title: '', + parentElement: null, + }; +} + function makeSelect(options, initialValue) { - const sel = {id: 'modelSelect', options: [], selectedIndex: -1, selectedOptions: []}; + const sel = {id: 'modelSelect', options: [], selectedIndex: -1, selectedOptions: [], children: []}; Object.defineProperty(sel, 'value', { get() { return this._value || ''; }, set(v) { @@ -59,11 +86,36 @@ function makeSelect(options, initialValue) { this.selectedOptions = idx >= 0 ? [this.options[idx]] : []; } }); + sel.appendChild = function(node) { + if (!node) return node; + if (node.tagName === 'OPTGROUP') { + node._select = this; + this.children.push(node); + for (const child of node.children || []) { + if (!this.options.includes(child)) this.options.push(child); + } + return node; + } + node.parentElement = node.parentElement || null; + this.options.push(node); + return node; + }; + sel.querySelectorAll = function(selector) { + if (selector === 'optgroup') return this.children.filter(child => child.tagName === 'OPTGROUP'); + return []; + }; sel.querySelector = function(_selector) { return this.options[0] || null; }; + const groups = new Map(); for (const item of options) { - const group = {tagName: 'OPTGROUP', dataset: {provider: item.provider || ''}}; - const opt = {value: item.value, textContent: item.label || item.value, parentElement: group, dataset: {}}; - sel.options.push(opt); + const provider = item.provider || ''; + let group = groups.get(provider); + if (!group) { + group = makeOptGroup(provider, provider); + groups.set(provider, group); + sel.appendChild(group); + } + const opt = makeOption(item.value, item.label || item.value, provider); + group.appendChild(opt); } sel.value = initialValue || ''; return sel; @@ -91,7 +143,12 @@ const _liveModelFetchPending = new Set(); const document = { title: '', baseURI: 'http://127.0.0.1/hermes/', - createElement(tag) { return {tagName: tag.toUpperCase(), className: '', textContent: '', appendChild(){}}; }, + createElement(tag) { + const upper = String(tag || '').toUpperCase(); + if (upper === 'OPTGROUP') return makeOptGroup('', ''); + if (upper === 'OPTION') return makeOption('', '', ''); + return {tagName: upper, className: '', textContent: '', dataset: {}, appendChild(){}}; + }, createTextNode(text) { return {textContent: text}; }, }; const window = { _botName: 'Hermes', _defaultModel: null, _activeProvider: null }; @@ -102,6 +159,7 @@ for (const name of [ '_topbarLoadedMessageCount', '_topbarMessageMetaText', '_getOptionProviderId', '_providerFromModelValue', '_modelStateForSelect', '_findModelInDropdown', '_refreshOpenModelDropdown', '_applyModelToDropdown', + '_addLiveModelsToSelect', '_modelStateFromAppliedDropdown', '_persistSessionModelCorrection', '_applySessionModelFallback', 'syncTopbar' ]) { @@ -131,12 +189,17 @@ var S = { if (args.preapplyModel) { _applyModelToDropdown(args.preapplyModel, modelSelect, args.preapplyProvider || null); } -syncTopbar(); +if (args.liveProvider && Array.isArray(args.liveModels)) { + _addLiveModelsToSelect(args.liveProvider, args.liveModels, modelSelect); +} else { + syncTopbar(); +} process.stdout.write(JSON.stringify({ selectValue: modelSelect.value, sessionModel: S.session.model, sessionProvider: S.session.model_provider, + optionValues: modelSelect.options.map(o => o.value), calls, })); """ @@ -186,6 +249,41 @@ def _run_sync( return json.loads(result.stdout) +def _run_add_live_models( + driver_path, + *, + session_model, + initial_value, + live_provider, + live_models, + session_provider=None, + dropdown_open=False, +): + payload = { + "sessionModel": session_model, + "sessionProvider": session_provider, + "initialValue": initial_value, + "defaultModel": "@safe:gpt-4o-mini", + "activeProvider": live_provider, + "dropdownOpen": dropdown_open, + "liveProvider": live_provider, + "liveModels": live_models, + "options": [ + {"provider": "expensive", "value": "@expensive:gpt-5.5", "label": "GPT-5.5"}, + {"provider": "safe", "value": "@safe:gpt-4o-mini", "label": "GPT-4o mini"}, + ], + } + result = subprocess.run( + [NODE, driver_path, str(UI_JS_PATH), json.dumps(payload)], + capture_output=True, + text=True, + timeout=30, + ) + if result.returncode != 0: + raise RuntimeError(f"node driver failed:\nSTDOUT={result.stdout}\nSTDERR={result.stderr}") + return json.loads(result.stdout) + + def test_sync_topbar_missing_model_falls_back_to_configured_default_not_previous_chat(driver_path): got = _run_sync(driver_path, session_model="") @@ -241,11 +339,21 @@ def test_sync_topbar_rerender_count_remains_one_for_preapply_plus_sync(driver_pa assert got["calls"]["positionModelDropdown"] == 1 -def test_live_model_reapply_paths_force_refresh_after_catalog_growth(): - src = UI_JS_PATH.read_text(encoding="utf-8") +def test_live_model_reapply_refreshes_open_dropdown_after_catalog_growth(driver_path): + got = _run_add_live_models( + driver_path, + session_model="@safe:gpt-4o-mini", + session_provider="safe", + initial_value="@safe:gpt-4o-mini", + live_provider="safe", + live_models=[{"id": "gpt-4.1", "label": "GPT-4.1"}], + dropdown_open=True, + ) - assert "_applyModelToDropdown(currentVal, sel, currentProvider, {forceRefresh:true})" in src - assert "_applyModelToDropdown(S.session.model, sel, S.session.model_provider||null, {forceRefresh:added>0})" in src + assert got["selectValue"] == "@safe:gpt-4o-mini" + assert "@safe:gpt-4.1" in got["optionValues"] + assert got["calls"]["renderModelDropdown"] == 1 + assert got["calls"]["positionModelDropdown"] == 1