fix: reset model picker on session switch

This commit is contained in:
Michael Lam
2026-05-06 19:48:31 -07:00
committed by test
parent 8ed7a7f61c
commit 24f76bcf37
5 changed files with 277 additions and 34 deletions
Binary file not shown.

After

Width:  |  Height:  |  Size: 142 KiB

+77 -29
View File
@@ -516,16 +516,58 @@ function _findModelInDropdown(modelId, sel, preferredProviderId){
// Set the model picker to the best match for modelId.
// Returns the resolved value that was actually set, or null if nothing matched.
function _refreshOpenModelDropdown(){
const dd=$('composerModelDropdown');
if(dd&&dd.classList&&dd.classList.contains('open')&&typeof renderModelDropdown==='function'){
renderModelDropdown();
if(typeof _positionModelDropdown==='function') _positionModelDropdown();
}
}
function _applyModelToDropdown(modelId, sel, preferredProviderId){
if(!modelId||!sel) return null;
const resolved=_findModelInDropdown(modelId,sel,preferredProviderId);
if(resolved){
sel.value=resolved;
if(sel.id==='modelSelect' && typeof syncModelChip==='function') syncModelChip();
if(sel.id==='modelSelect'){
if(typeof syncModelChip==='function') syncModelChip();
_refreshOpenModelDropdown();
}
return resolved;
}
return null;
}
function _modelStateFromAppliedDropdown(sel, modelValue){
const state=(typeof _modelStateForSelect==='function')
? _modelStateForSelect(sel,modelValue)
: {model:modelValue,model_provider:null};
return {model:state.model||modelValue,model_provider:state.model_provider||null};
}
function _persistSessionModelCorrection(model, provider){
if(!S.session) return;
fetch(new URL('api/session/update',document.baseURI||location.href).href,{
method:'POST',credentials:'include',
headers:{'Content-Type':'application/json'},
body:JSON.stringify({session_id:S.session.id||S.session.session_id,model:model,model_provider:provider||null})
}).catch(()=>{});
}
function _applySessionModelFallback(sel){
if(!sel) return null;
const configuredDefault=String(window._defaultModel||'').trim();
if(configuredDefault){
const appliedDefault=_applyModelToDropdown(configuredDefault,sel,window._activeProvider||null);
if(appliedDefault) return _modelStateFromAppliedDropdown(sel,appliedDefault);
}
const first=sel.querySelector('optgroup > option, option');
if(first){
sel.value=first.value;
if(sel.id==='modelSelect'){
if(typeof syncModelChip==='function') syncModelChip();
_refreshOpenModelDropdown();
}
return _modelStateFromAppliedDropdown(sel,first.value);
}
return null;
}
async function populateModelDropdown(){
const sel=$('modelSelect');
@@ -3660,35 +3702,41 @@ function syncTopbar(){
_applyModelToDropdown(modelOverride,$('modelSelect'),providerOverride);
currentModel=modelOverride;
} else {
const applied=_applyModelToDropdown(currentModel,$('modelSelect'),S.session.model_provider||null);
// If the model isn't in the current provider list, silently reset to the
// first available model so stale values don't pollute the picker (#829).
if(!applied && currentModel){
const deferModelCorrection=Boolean(S.session._modelResolutionDeferred);
// Also defer if a live model fetch is still in flight — the model may be
// in the list once the fetch completes. Persisting now would corrupt the
// session with the wrong model before live models arrive (#1169).
const liveStillPending=window._activeProvider&&_liveModelFetchPending.has(window._activeProvider);
if(liveStillPending){
// Live fetch in flight — don't touch sel.value or S.session.model yet.
// _addLiveModelsToSelect() will re-apply S.session.model once done (#1169).
} else {
// Stale session model not in the current provider catalog — reset to the
// first available model rather than injecting an "(unavailable)" option
// that visually appears under the wrong provider group (#829).
const modelSel=$('modelSelect');
const first=modelSel&&modelSel.querySelector('optgroup > option, option');
if(first){
modelSel.value=first.value;
if(!deferModelCorrection){
S.session.model=first.value;
S.session.model_provider=_getOptionProviderId(first)||null;
const modelSel=$('modelSelect');
const rawCurrentModel=String(currentModel||'').trim();
const hasSessionModel=rawCurrentModel&&rawCurrentModel.toLowerCase()!=='unknown';
if(!hasSessionModel){
// Missing/unknown session metadata must not leave the picker on the
// previously viewed chat's model (#1771). Apply the configured default
// first, then the first available option only as an HTML fallback.
const fallback=_applySessionModelFallback(modelSel);
if(fallback){
S.session.model=fallback.model;
S.session.model_provider=fallback.model_provider||null;
currentModel=fallback.model;
_persistSessionModelCorrection(fallback.model,S.session.model_provider||null);
}
} else {
const applied=_applyModelToDropdown(currentModel,modelSel,S.session.model_provider||null);
// If the model isn't in the current provider list, reset to the configured
// default rather than silently retaining the previous chat's selection (#1771).
if(!applied){
const deferModelCorrection=Boolean(S.session._modelResolutionDeferred);
// Also defer if a live model fetch is still in flight — the model may be
// in the list once the fetch completes. Persisting now would corrupt the
// session with the wrong model before live models arrive (#1169).
const liveStillPending=window._activeProvider&&_liveModelFetchPending.has(window._activeProvider);
if(liveStillPending){
// Live fetch in flight — don't touch sel.value or S.session.model yet.
// _addLiveModelsToSelect() will re-apply S.session.model once done (#1169).
} else {
const fallback=_applySessionModelFallback(modelSel);
if(fallback&&!deferModelCorrection){
S.session.model=fallback.model;
S.session.model_provider=fallback.model_provider||null;
currentModel=fallback.model;
// Persist the correction so the session doesn't re-inject on next load.
fetch(new URL('api/session/update',document.baseURI||location.href).href,{
method:'POST',credentials:'include',
headers:{'Content-Type':'application/json'},
body:JSON.stringify({session_id:S.session.id||S.session.session_id,model:first.value,model_provider:S.session.model_provider||null})
}).catch(()=>{});
_persistSessionModelCorrection(fallback.model,S.session.model_provider||null);
}
}
}
@@ -0,0 +1,192 @@
"""
Regression tests for issue #1771: switching sessions with missing/stale model
metadata must not leave the composer model picker on the previously viewed
chat's model.
These tests execute the real static/ui.js syncTopbar() path in Node with a tiny
DOM/select shim so the behavioral contract is protected without needing a full
browser harness.
"""
import json
import shutil
import subprocess
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).resolve().parents[1]
UI_JS_PATH = REPO_ROOT / "static" / "ui.js"
NODE = shutil.which("node")
pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH")
_DRIVER_SRC = r"""
const fs = require('fs');
const ui = fs.readFileSync(process.argv[2], 'utf8');
function extractFunc(name, opts = {}) {
const re = new RegExp('function\\s+' + name + '\\s*\\(');
const start = ui.search(re);
if (start < 0) {
if (opts.optional) return '';
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);
}
const calls = {syncModelChip: 0, renderModelDropdown: 0, positionModelDropdown: 0, fetches: []};
let modelSelect;
let dropdownOpen = false;
const dropdown = {classList: {contains: (name) => name === 'open' && dropdownOpen}};
function makeSelect(options, initialValue) {
const sel = {id: 'modelSelect', options: [], selectedIndex: -1, selectedOptions: []};
Object.defineProperty(sel, 'value', {
get() { return this._value || ''; },
set(v) {
this._value = v;
const idx = this.options.findIndex(o => o.value === v);
this.selectedIndex = idx;
this.selectedOptions = idx >= 0 ? [this.options[idx]] : [];
}
});
sel.querySelector = function(_selector) { return this.options[0] || null; };
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);
}
sel.value = initialValue || '';
return sel;
}
function $(id) {
if (id === 'modelSelect') return modelSelect;
if (id === 'composerModelDropdown') return dropdown;
return {textContent: '', style: {}, classList: {add(){}, remove(){}, toggle(){}, contains(){return false;}}, appendChild(){}, appendChildNode(){}};
}
function t(key) { return key; }
function syncModelChip() { calls.syncModelChip++; }
function renderModelDropdown() { calls.renderModelDropdown++; }
function _positionModelDropdown() { calls.positionModelDropdown++; }
function syncAppTitlebar() {}
function syncWorkspaceDisplays() {}
function syncReasoningChip() {}
function syncToolsetsChip() {}
function syncTerminalButton() {}
function _syncHermesPanelSessionActions() {}
function _latestGatewayRoutingForSession() { return null; }
function getModelLabel(v) { return v; }
function _formatGatewayModelLabel(_v, text) { return text; }
const _liveModelFetchPending = new Set();
const document = {
title: '',
baseURI: 'http://127.0.0.1/hermes/',
createElement(tag) { return {tagName: tag.toUpperCase(), className: '', textContent: '', appendChild(){}}; },
createTextNode(text) { return {textContent: text}; },
};
const window = { _botName: 'Hermes', _defaultModel: null, _activeProvider: null };
function fetch(url, opts) { calls.fetches.push({url: String(url), body: opts && opts.body || ''}); return Promise.resolve({ok: true}); }
for (const name of [
'_getOptionProviderId', '_providerFromModelValue', '_modelStateForSelect',
'_findModelInDropdown', '_refreshOpenModelDropdown', '_applyModelToDropdown',
'_modelStateFromAppliedDropdown', '_persistSessionModelCorrection',
'_applySessionModelFallback', 'syncTopbar'
]) {
const src = extractFunc(name, {optional: name !== 'syncTopbar'});
if (src) eval(src);
}
const args = JSON.parse(process.argv[3]);
modelSelect = makeSelect(args.options, args.initialValue);
dropdownOpen = !!args.dropdownOpen;
window._defaultModel = args.defaultModel || null;
window._activeProvider = args.activeProvider || null;
var S = {
session: {
session_id: 'session-b',
id: 'session-b',
title: 'Session B',
model: args.sessionModel,
model_provider: args.sessionProvider || null,
messages: [],
},
messages: [],
activeProfile: 'default',
};
syncTopbar();
process.stdout.write(JSON.stringify({
selectValue: modelSelect.value,
sessionModel: S.session.model,
sessionProvider: S.session.model_provider,
calls,
}));
"""
@pytest.fixture(scope="module")
def driver_path(tmp_path_factory):
p = tmp_path_factory.mktemp("issue1771_driver") / "driver.js"
p.write_text(_DRIVER_SRC, encoding="utf-8")
return str(p)
def _run_sync(driver_path, *, session_model, initial_value="@expensive:gpt-5.5", default_model="@safe:gpt-4o-mini", dropdown_open=False):
payload = {
"sessionModel": session_model,
"sessionProvider": None,
"initialValue": initial_value,
"defaultModel": default_model,
"activeProvider": "safe",
"dropdownOpen": dropdown_open,
"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=10,
)
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="")
assert got["selectValue"] == "@safe:gpt-4o-mini"
assert got["sessionModel"] == "@safe:gpt-4o-mini"
assert got["sessionProvider"] == "safe"
assert got["selectValue"] != "@expensive:gpt-5.5"
def test_sync_topbar_unknown_model_falls_back_to_configured_default_not_first_option(driver_path):
got = _run_sync(driver_path, session_model="unknown")
assert got["selectValue"] == "@safe:gpt-4o-mini"
assert got["sessionModel"] == "@safe:gpt-4o-mini"
assert got["sessionProvider"] == "safe"
def test_sync_topbar_rerenders_open_visible_model_dropdown_after_session_model_change(driver_path):
got = _run_sync(driver_path, session_model="", dropdown_open=True)
assert got["selectValue"] == "@safe:gpt-4o-mini"
assert got["calls"]["renderModelDropdown"] >= 1
assert got["calls"]["positionModelDropdown"] >= 1
+7 -4
View File
@@ -1344,8 +1344,11 @@ def test_stale_ui_js_does_not_inject_unavailable_option():
"stale models should be silently reset to the first available model (#829)"
)
# The new silent-reset pattern must be present
assert "first.value" in src and "S.session.model=first.value" in src, (
"renderSession() must silently reset S.session.model to the first "
"available option when the session model is not in the dropdown (#829)"
# The reset path remains, but #1771 now prefers the configured default
# before using the first HTML option as a last-resort fallback.
assert "_applySessionModelFallback" in src and "configuredDefault" in src, (
"stale session models should be reset through the safe fallback helper"
)
assert "const first=sel.querySelector('optgroup > option, option');" in src, (
"the first available option should remain only as a fallback when no configured default applies"
)
+1 -1
View File
@@ -39,7 +39,7 @@ def test_session_switch_defers_model_resolution_without_blocking():
assert "messages=0&resolve_model=1" in src
assert "_modelResolutionDeferred=true" in src
assert "deferModelCorrection" in ui
assert "if(!deferModelCorrection)" in ui
assert "if(fallback&&!deferModelCorrection)" in ui
def test_boot_does_not_block_session_restore_on_model_catalog():