fix: a model added in Admin now appears everywhere models are chosen
Some checks failed
Forgejo Docker Build / Build Docker image (push) Blocked by required conditions
Forgejo Docker Build / Deploy to the host (push) Blocked by required conditions
Forgejo Android APK / Root app tests (push) Successful in 47s
Forgejo Docker Build / Root app tests (push) Successful in 46s
Forgejo Android APK / Build signed APK (push) Has been cancelled
Some checks failed
Forgejo Docker Build / Build Docker image (push) Blocked by required conditions
Forgejo Docker Build / Deploy to the host (push) Blocked by required conditions
Forgejo Android APK / Root app tests (push) Successful in 47s
Forgejo Docker Build / Root app tests (push) Successful in 46s
Forgejo Android APK / Build signed APK (push) Has been cancelled
Adding a discovered model refreshed exactly one dropdown — the default-model one, in the same card. Every other picker had been filled when the admin tab loaded, behind a guard that makes its loader run once per visit, so the Clinical Assistant chat model, the allowed-models list, the slide reviewer and the image-model selects all kept the roster they were given. The model was genuinely added; it simply could not be selected until the page was reloaded, which reads as the add having failed. Every mutation of the roster — add, remove, clear-all, enable/disable — now dispatches `models-changed`, and the cards that list models listen and refetch. The event carries no payload: a listener re-reads the list itself, so there is one source of truth rather than a copy to keep in step. Same pattern as the existing `assistant-image-roster` event. Each listener clears its own guard before re-running, and returns early if a load is already in flight. The assistant loader keeps unsaved drafts, so re-running it costs nothing but a refreshed set of options. The add toast said "now select it as default and click Set Default" — advice that only made sense when the default dropdown was the one thing that updated. Verified against a mutation: removing the guard reset fails the propagation test, because the listener then fires into a loader that returns early. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
parent
ff2b2bc9d3
commit
31e634e0ce
4 changed files with 90 additions and 1 deletions
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
}
|
||||
|
|
|
|||
54
test/models-changed-propagation.test.js
Normal file
54
test/models-changed-propagation.test.js
Normal file
|
|
@ -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/);
|
||||
});
|
||||
Loading…
Reference in a new issue