fix: My Resources uses the image model you chose for it, and says so on save
All checks were successful
Forgejo Docker Build / Root app tests (push) Successful in 48s
Forgejo Docker Build / Build Docker image (push) Successful in 9s
Forgejo Docker Build / End-to-end (browser) (push) Successful in 5s

"The model is chosen per request, where the deck is generated" described
something that happens nowhere. There is no per-request picker, and
asking where it was is what exposed the real bug.

My Resources read clinical_assistant.image_model while the admin screen
saved my_resources.image_model. That setting was stored, returned by the
API and rendered into the form — and never used by anything. Somebody
noticed the field did nothing and disabled it rather than finding out
why, which left a control that could not be changed and a note
explaining a mechanism that does not exist. My note repeating it was
wrong too.

The generator now reads its own setting and falls back to the
Assistant's, so an install that only ever set one model keeps working
untouched, and the field is enabled again with "leave blank to use the
Clinical Assistant's" — which is now true rather than a rationalisation.

Saving also says what it saved. "Saved 8:31:59 PM. Decks will be
reviewed by ..." answered a different question from the one an admin
actually has, which is whether the model they just picked is the one
that will draw. It now names each workflow's model and fallbacks back,
and spells out the blank case rather than leaving a gap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
Daniel 2026-09-13 02:37:57 +02:00
parent 0287fd091b
commit ae602a1852
4 changed files with 74 additions and 21 deletions

View file

@ -408,12 +408,17 @@ export function initClinicalAssistantAdmin(adminEscapeHtml) {
putAssistantConfig('clinical_assistant.allowed_image_models', checkedAssistantModels('assistant-allowed-image-models').join(',')),
putAssistantConfig('my_resources.review_model', getValue('mr-review-model')),
saveImageSettings()
]).then(function() {
]).then(function(results) {
var reviewer = getValue('mr-review-model');
// The image models named back. "Saved" does not tell an administrator
// that the model they chose is the one that will draw — which is the
// whole question, and the reason this card was hard to trust.
var images = Array.isArray(results[results.length - 1]) ? results[results.length - 1] : [];
if (status) {
status.textContent = 'Saved ' + new Date().toLocaleTimeString() + '. ' +
(reviewer ? 'Decks will be reviewed by ' + reviewer + '.' : 'Slide review is off.') +
' New image jobs use these settings; existing jobs are unchanged.';
(images.length ? 'Images — ' + images.join('; ') + '. ' : '') +
(reviewer ? 'Decks reviewed by ' + reviewer + '. ' : 'Slide review is off. ') +
'New image jobs use these settings; existing jobs are unchanged.';
}
showToast('Availability saved', 'success');
}).catch(function(err) {

View file

@ -50,21 +50,18 @@ async function load() {
group.appendChild(legend);
// My Resources picks its model per request from the roster, so only the
// Only the Assistant names a primary here. My Resources takes its model
// from the request that asks for the picture, so a disabled, permanently
// empty dropdown labelled "set per request" was a control that could
// never do anything — it read as broken rather than as not applicable.
// Its fallbacks still belong here: those are what runs when the chosen
// model fails, whoever chose it.
// Both workflows name a model. My Resources' was saved and ignored for a
// while — the generator read the Assistant's setting instead — so the
// field was disabled and labelled "set per request", describing something
// that happens nowhere. It works now, and blank means "use the Clinical
// Assistant's", which is what every install did before it worked.
const primary = makeSelect([saved.model], discovered);
group.appendChild(row('Model', primary));
if (workflow.key === 'my_resources') {
const note = document.createElement('p');
note.style.cssText = 'margin:2px 0 8px;font-size:12px;color:var(--g500);';
note.textContent = 'The model is chosen per request, where the deck is generated. ' +
'The fallbacks below apply to whichever model that request picked.';
note.style.cssText = 'margin:2px 0 10px;font-size:12px;color:var(--g500);';
note.textContent = 'Leave blank to use the Clinical Assistant\'s image model above.';
group.appendChild(note);
} else {
group.appendChild(row('Model', primary));
}
const fallbacks = [];
@ -100,18 +97,26 @@ async function load() {
// settings that were not sent.
export async function saveImageSettings() {
if (!loaded) throw new Error('Image settings were not loaded, so they were not saved.');
// What was written, for the card's confirmation. "Saved" alone does not
// answer the question an admin actually has, which is whether the model they
// just picked is the one that will be used.
const written = [];
for (const workflow of WORKFLOWS) {
const control = controls[workflow.key];
const body = {
budget: Number(control.budget.value),
fallbacks: control.fallbacks.map(s => s.value).filter(Boolean)
};
// Only the workflows that own a primary send one; My Resources would
// be overwriting a per-request choice with a form field.
if (workflow.key !== 'my_resources') body.model = control.primary.value;
// Every workflow sends its model. My Resources' used to be withheld,
// which is why its field could never be set to anything.
body.model = control.primary.value;
await imageJson('/api/admin/image-settings/' + workflow.key,
{ method: 'PUT', body: JSON.stringify(body) });
written.push(workflow.label + ': ' +
(body.model || 'the Clinical Assistant\'s model') +
(body.fallbacks.length ? ', then ' + body.fallbacks.join(', ') : ', no fallback'));
}
return written;
}
function row(labelText, control) {

View file

@ -209,6 +209,24 @@ router.get('/my-resources/image/jobs/:id', async function (req, res) {
}
});
// Which model draws the figures in a resource.
//
// Its own setting, with the Clinical Assistant's as the fallback. This used to
// read the Assistant's and only the Assistant's, while the admin screen saved
// my_resources.image_model — so that field was stored, returned by the API,
// shown in the form, and never used. Somebody noticed the field did nothing and
// disabled it rather than finding out why, which left a control that could not
// be changed and a note saying the model was "chosen per request", which is not
// a thing that happens anywhere.
//
// Blank still means "use the Assistant's", so an install that only ever set one
// model keeps working without anyone touching the screen.
async function resourceImageModel() {
var own = String(await db.getSetting('my_resources.image_model', '') || '');
if (own) return own;
return String(await db.getSetting('clinical_assistant.image_model', '') || '');
}
router.get('/my-resources/options', async function (req, res) {
try {
var models = await allowedModels();
@ -216,7 +234,7 @@ router.get('/my-resources/options', async function (req, res) {
success: true,
models: models.allowed,
defaultModel: models.configured,
imagesAvailable: Boolean(await db.getSetting('clinical_assistant.image_model', '')),
imagesAvailable: Boolean(await resourceImageModel()),
// Whether this person has somewhere to send a file. The button is hidden
// rather than shown-and-refused: an action that always fails is worse
// than an action that is not offered.
@ -275,7 +293,7 @@ async function gatherSources(subject, body, keywords) {
// Asking for an illustration when no image model is configured is not an
// error, it just cannot happen; the caller reports that rather than failing.
var imageModel = wantsImages ? String(await db.getSetting('clinical_assistant.image_model', '') || '') : '';
var imageModel = wantsImages ? await resourceImageModel() : '';
return {
corpus: corpus, searches: searches, literature: literature, webFindings: webFindings,
// A search that was asked for and came back empty is the dangerous case: the

View file

@ -421,9 +421,12 @@ test('image-model dropdowns keep saved selections through discovery failures and
'Learning Hub is gone; saving to it is what broke the card');
assert.deepEqual(puts().find(c => c.url.endsWith('clinical_assistant')).body,
{ model: 'saved-image', budget: 32000, fallbacks: ['saved-backup'] });
// My Resources chooses its model per request, so the form must not send one.
// My Resources sends its model like every other workflow. It used to be
// withheld — the generator read the Clinical Assistant's setting instead, so
// my_resources.image_model was saved and ignored, and the field was disabled
// rather than fixed. Blank is meaningful: it means "use the Assistant's".
assert.deepEqual(puts().find(c => c.url.endsWith('my_resources')).body,
{ budget: 32000, fallbacks: [] });
{ model: '', budget: 32000, fallbacks: [] });
assert.equal(select.value, 'saved-image', 'save never reverts the selection');
});
@ -513,3 +516,25 @@ test('catalogue loading failure always reaches a visible retry state, never an e
assert.doesNotMatch(f.textContent, /Loading/, 'no eternal spinner text remains');
}
});
test('saving names the image models back, so a choice can be seen to have taken', async t => {
// "Saved 8:31:59 PM" answered a different question from the one an admin has,
// which is whether the model they just picked is the one that will draw.
const ui = await browser(t, 'admin', url => {
if (url === '/api/admin/config') return json(assistantConfig());
if (url === '/api/admin/image-settings') return json({ success: true, maxModels: 3, workflows: {
clinical_assistant: { model: 'draws-this', fallbacks: ['then-this'], budget: 32000 },
my_resources: { model: '', fallbacks: [], budget: 32000 } } });
if (url.endsWith('/image-models/discover')) return json({ models: [{ id: 'draws-this' }, { id: 'then-this' }] });
if (url.includes('/api/admin/image-settings/')) return json({ success: true });
});
for (let i = 0; i < 6; i++) await tick();
ui.document.getElementById('btn-save-availability').click();
for (let i = 0; i < 10; i++) await tick();
const said = ui.document.getElementById('assistant-availability-status').textContent;
assert.match(said, /Clinical Assistant: draws-this, then then-this/);
// Blank is meaningful and is spelled out rather than shown as an empty gap.
assert.match(said, /My Resources: the Clinical Assistant's model/);
assert.match(said, /existing jobs are unchanged/);
});