diff --git a/public/js/admin.js b/public/js/admin.js index c83947d1..f7d06011 100644 --- a/public/js/admin.js +++ b/public/js/admin.js @@ -975,6 +975,8 @@ initImageSettings(); .then(function(r) { return r.json(); }) .then(function(data) { if (data.success) { + // Disabling removes a model from every picker, not only this list. + announceModelsChanged(); showToast(modelId + ' ' + (enabled ? 'enabled' : 'disabled'), 'success'); loadAdminModels(); } else { @@ -1028,6 +1030,16 @@ initImageSettings(); }); } + // The model roster changed. Every picker that lists models listens for this, + // because otherwise they keep whatever they were given when the tab loaded: + // adding a model used to refresh the default-model dropdown alone, and the + // Clinical Assistant, review-model and image pickers only caught up on a page + // reload. The detail carries nothing — a listener re-reads the list itself, + // so there is one source of truth rather than a payload to keep in step. + function announceModelsChanged() { + try { document.dispatchEvent(new CustomEvent('models-changed')); } catch (e) {} + } + function addDiscoveredModel(id, name, btn) { fetch('/api/admin/config/models/add-discovered', { method: 'POST', @@ -1037,7 +1049,8 @@ initImageSettings(); .then(function(r) { return r.json(); }) .then(function(data) { if (data.success) { - showToast('Added: ' + name + ' — now select it as default and click Set Default', 'success'); + announceModelsChanged(); + showToast('Added: ' + name + ' — it is now selectable everywhere models are chosen', 'success'); if (btn) { btn.textContent = 'Added'; btn.disabled = true; btn.style.background = 'var(--green)'; } // Refresh model lists, then auto-select the newly added model fetch('/api/admin/config/models', { headers: getAuthHeaders() }) @@ -1105,6 +1118,7 @@ initImageSettings(); .then(function(r) { return r.json(); }) .then(function(data) { if (data.success) { + announceModelsChanged(); showToast('Removed: ' + modelId, 'info'); loadAdminModels(); } else { @@ -1121,6 +1135,7 @@ initImageSettings(); .then(function(r) { return r.json(); }) ]).then(function(results) { if (results[0].success) { + announceModelsChanged(); showToast('All models cleared', 'info'); loadAdminModels(); } else { diff --git a/public/js/admin/clinicalAssistant.js b/public/js/admin/clinicalAssistant.js index b7cc6e76..a70ea96f 100644 --- a/public/js/admin/clinicalAssistant.js +++ b/public/js/admin/clinicalAssistant.js @@ -80,6 +80,18 @@ export function initClinicalAssistantAdmin(adminEscapeHtml) { if (e.detail && e.detail.tab === 'admin') loadAssistantAdmin(); }); + // The roster changed while this card was already on screen. Its loader is + // guarded so it runs once per visit, which is right for a tab change and + // wrong here — without this, a model added in the Models card did not appear + // in the chat, allowed-models or review pickers until the page was reloaded. + document.addEventListener('models-changed', function() { + // The loader keeps unsaved drafts, so re-running it costs nothing but a + // refreshed list of options. + if (configState === 'loading') return; + configState = 'idle'; + loadAssistantAdmin(); + }); + document.addEventListener('click', function(e) { if (e.target.closest('#btn-save-assistant-config')) saveAssistantAdmin(); if (e.target.closest('#btn-retry-assistant-config')) loadAssistantAdmin(); diff --git a/public/js/admin/imageSettings.js b/public/js/admin/imageSettings.js index f3a897ba..4e8f091c 100644 --- a/public/js/admin/imageSettings.js +++ b/public/js/admin/imageSettings.js @@ -145,4 +145,12 @@ function makeBudget(value) { export function initImageSettings() { document.addEventListener('tabChanged', e => { if (e.detail?.tab === 'admin') load(); }); + // A model added in the Models card has to show up in these dropdowns without + // a page reload. `loaded` exists so a tab revisit does not refetch; a roster + // change is the one case where refetching is the point. + document.addEventListener('models-changed', () => { + if (loading) return; + loaded = false; + load(); + }); } diff --git a/test/models-changed-propagation.test.js b/test/models-changed-propagation.test.js new file mode 100644 index 00000000..dbc95379 --- /dev/null +++ b/test/models-changed-propagation.test.js @@ -0,0 +1,54 @@ +// Adding a model in the Models card has to reach every picker that lists models. +// It used to refresh the default-model dropdown alone, so a newly added model +// was invisible in the Clinical Assistant, review-model and image pickers until +// the page was reloaded — which is not obvious, and looks like the add failed. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const read = f => fs.readFileSync(path.join(__dirname, '..', f), 'utf8'); + +test('every mutation of the model roster announces it', () => { + const admin = read('public/js/admin.js'); + assert.match(admin, /function announceModelsChanged\(\)/); + assert.match(admin, /new CustomEvent\('models-changed'\)/); + + // Add, remove, clear-all and enable/disable each change what is selectable. + const announced = (admin.match(/announceModelsChanged\(\);/g) || []).length; + assert.ok(announced >= 4, 'expected an announcement on add, remove, clear and toggle; found ' + announced); + + for (const [label, marker] of [ + ['add-discovered', "config/models/add-discovered"], + ['remove custom', "config/models/custom/"], + ['clear all', "config/models/clear-all"], + ['toggle', "config/models/toggle"] + ]) { + const at = admin.indexOf(marker); + assert.ok(at > -1, label + ' call site missing'); + // The announcement lives inside that call's success branch. + assert.ok(admin.slice(at, at + 900).includes('announceModelsChanged()'), + label + ' does not announce the change'); + } +}); + +test('the pickers listen, and re-run a loader that is otherwise once-per-visit', () => { + const assistant = read('public/js/admin/clinicalAssistant.js'); + assert.match(assistant, /document\.addEventListener\('models-changed'/); + // Its guard must be cleared, or the listener fires and the loader returns early. + assert.match(assistant, /configState = 'idle';\s*\n\s*loadAssistantAdmin\(\);/); + assert.match(assistant, /if \(configState === 'loading'\) return;/, 'no reload mid-flight'); + + const images = read('public/js/admin/imageSettings.js'); + assert.match(images, /document\.addEventListener\('models-changed'/); + assert.match(images, /loaded = false;\s*\n\s*load\(\);/); + assert.match(images, /if \(loading\) return;/, 'no reload mid-flight'); +}); + +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. + const admin = read('public/js/admin.js'); + assert.doesNotMatch(admin, /now select it as default and click Set Default/); + assert.match(admin, /selectable everywhere models are chosen/); +});