From b1e039d834b30df930e1f88dd3dadd09de419b80 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 12 Sep 2026 17:46:18 +0200 Subject: [PATCH] fix: a newly added model reaches every picker, including the user-facing ones The first pass at this covered the two admin cards. It missed the pickers that matter most: the per-tab model selectors in app.js, which every clinical tab uses, and the My Resources model dropdown. Both were filled once at page load, so a model added in Admin was still invisible where people actually choose one. app.js's boot fetch is now a named loadModelList() that also runs on models-changed; My Resources re-runs loadOptions(), which is the same call that decides whether the model row is shown at all. Both rebuilds keep a choice already made. These selects can be rebuilt while someone is halfway through a form, and silently moving them off the model they picked would be worse than not refreshing. Verified against a mutation: removing the app.js listener fails the test that says every picker listens. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- public/js/app.js | 18 ++++++++++++++++-- public/js/myResources.js | 12 ++++++++++++ test/models-changed-propagation.test.js | 21 +++++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/public/js/app.js b/public/js/app.js index 6c6d6cbb..54647b9d 100644 --- a/public/js/app.js +++ b/public/js/app.js @@ -572,7 +572,11 @@ document.addEventListener('DOMContentLoaded', function() { window._currentProvider = 'openrouter'; window._defaultModelId = ''; - fetch('/api/models') + // Re-read on demand, so a model an administrator adds reaches every per-tab + // selector without a page reload. The same call runs at boot and on + // models-changed; it is idempotent and rebuilds whatever selectors exist now. + function loadModelList() { + return fetch('/api/models') .then(function(r) { return r.json(); }) .then(function(data) { window._currentModels = data.models || []; @@ -593,12 +597,22 @@ document.addEventListener('DOMContentLoaded', function() { var defaultModelId = data.defaultModel || (window._currentModels.length > 0 ? window._currentModels[0].id : ''); window._defaultModelId = defaultModelId; - // Populate all per-tab model selectors already in DOM + // Populate all per-tab model selectors already in DOM. A selector that + // already holds a valid choice keeps it: rebuilding must not silently + // move someone off the model they picked. document.querySelectorAll('.tab-model-select').forEach(function(sel) { + var chosen = sel.value; window._buildModelOptions(sel); + if (chosen && Array.prototype.some.call(sel.options, function(o) { return o.value === chosen; })) { + sel.value = chosen; + } }); }) .catch(function(err) { console.warn('Models load failed:', err); }); + } + + loadModelList(); + document.addEventListener('models-changed', function() { loadModelList(); }); console.log('✅ App.js DOM ready'); diff --git a/public/js/myResources.js b/public/js/myResources.js index d63d5612..ad725d16 100644 --- a/public/js/myResources.js +++ b/public/js/myResources.js @@ -11,6 +11,13 @@ (function () { var inited = false; + // A model an administrator adds has to show up here too, without a reload. + // loadOptions also decides whether the model row is shown at all, so this is + // the same call rather than a partial refresh of the select. + document.addEventListener('models-changed', function () { + if (inited) loadOptions(); + }); + document.addEventListener('tabChanged', function (e) { if (!e.detail || e.detail.tab !== 'myresources') return; if (!inited) { init(); inited = true; } @@ -121,6 +128,10 @@ var modelRow = document.getElementById('mr-model-row'); var models = data.models || []; if (select) { + // Rebuilt, but a choice already made is kept: this also runs when an + // administrator adds a model, and moving someone off the model they + // picked mid-form would be worse than not refreshing at all. + var chosen = select.value; select.textContent = ''; models.forEach(function (id) { var option = document.createElement('option'); @@ -129,6 +140,7 @@ if (id === data.defaultModel) option.selected = true; select.appendChild(option); }); + if (chosen && models.indexOf(chosen) !== -1) select.value = chosen; } if (modelRow) modelRow.hidden = models.length < 2; diff --git a/test/models-changed-propagation.test.js b/test/models-changed-propagation.test.js index dbc95379..d680db96 100644 --- a/test/models-changed-propagation.test.js +++ b/test/models-changed-propagation.test.js @@ -45,6 +45,27 @@ test('the pickers listen, and re-run a loader that is otherwise once-per-visit', assert.match(images, /if \(loading\) return;/, 'no reload mid-flight'); }); +test('every model picker listens, not only the two in the admin cards', () => { + // The point is that a model added in Admin is selectable wherever models are + // chosen — the per-tab selectors and My Resources included, not just the + // admin's own dropdowns. + const app = read('public/js/app.js'); + assert.match(app, /document\.addEventListener\('models-changed'/); + assert.match(app, /function loadModelList\(\)/, 'the boot fetch is reusable, not inline'); + + const resources = read('public/js/myResources.js'); + assert.match(resources, /document\.addEventListener\('models-changed'/); + assert.match(resources, /if \(inited\) loadOptions\(\);/, 'nothing to refresh before the tab has initialised'); +}); + +test('a refresh never moves someone off the model they picked', () => { + // These selects are rebuilt while a form may be half filled in. + const app = read('public/js/app.js'); + assert.match(app, /var chosen = sel\.value;[\s\S]{0,260}sel\.value = chosen;/); + const resources = read('public/js/myResources.js'); + assert.match(resources, /var chosen = select\.value;[\s\S]{0,420}select\.value = chosen;/); +}); + test('the toast no longer tells people to go and set a default', () => { // It used to say "now select it as default and click Set Default", which was // advice for the one dropdown that did refresh.