From 96a6a353fc339f13295f42a093c40d2ce4964a6b Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 11 Sep 2026 04:04:31 +0200 Subject: [PATCH] fix: the assistant settings page says what saves what MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The card holds more than one Save button and nothing said so. "Save image settings" is injected directly above "Save model & retrieval settings", with no indication of where one stops and the other starts, and the page saves nothing automatically. It now states that up front, and the bottom button says which settings it applies. "Retry loading settings" sat beside Save looking like an ordinary control, because it did: a bare button with the hidden attribute, which the browser's own [hidden] rule could not hide once .btn-sm set a display. It is now inside an error message that exists only on failure, says what failed, and says that nothing typed has been lost. The status line used to read "Settings ready." forever, which answers a question nobody asks. It now reports the thing an admin actually wants to know when they come back: whether the last save went through, and at what time. A toast is gone in three seconds; this stays on the page. The signed-out preview moves to Feature Flags, where it belongs. It was a second checkbox under a row labelled "Sources", followed by two paragraphs, the first about preview and the second about citations — so neither paragraph clearly belonged to either checkbox. It is stored as feature.assistant_preview now, with the old clinical_assistant.preview_enabled still honoured when the new key has never been written. That also means an ordinary admin can toggle it under ADMIN_LOCKDOWN: clinical_assistant.* is locked, and putting a day-to-day switch behind host access was never the intent. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- public/components/admin.html | 44 ++++++++++++++------ public/js/admin.js | 22 +++++++++- public/js/admin/clinicalAssistant.js | 28 +++++++------ src/routes/clinicalAssistant.js | 11 ++++- test/admin-clinical-assistant-wiring.test.js | 8 +++- test/clinical-release-integration.test.js | 3 +- test/frontend-prompt-env.test.js | 19 +++++++-- 7 files changed, 100 insertions(+), 35 deletions(-) diff --git a/public/components/admin.html b/public/components/admin.html index 9a13858c..d44c8906 100644 --- a/public/components/admin.html +++ b/public/components/admin.html @@ -119,8 +119,17 @@ +
+ + +
+

Lets a visitor who is not signed in try the Clinical Assistant. They can ask questions and read the answer; everything else asks them to sign in. Nothing is saved to any account, because a preview visitor has no account.

- + +
@@ -316,9 +325,15 @@

Clinical Assistant / Learning

- Model and prompt settings + Model, retrieval and prompt settings
+

+ Nothing on this page saves by itself. Each Save button applies only the + settings above it, up to the previous Save. This card has two: image settings have their + own, and everything else is saved by Save model & retrieval settings + at the bottom. +

@@ -362,18 +377,13 @@
- Sources + Citations
- -

Preview is read-only: visitors can try the assistant, and anything else asks them to sign in or create an account.

-

Display only. The AI prompt, retrieval and the stored answer are byte-for-byte identical either way, so turning this off cannot change what the model says. Citations are hidden, not removed — turning it back on restores them.

+

Display only. The AI prompt, retrieval and the stored answer are byte-for-byte identical either way, so turning this off cannot change what the model says. Citations are hidden, not removed — turning it back on restores them.

@@ -398,10 +408,18 @@
Loading Learning prompts...
-
- - - + +
+ + Applies everything in this card except the image settings above. +
diff --git a/public/js/admin.js b/public/js/admin.js index 05d9d220..c410bf66 100644 --- a/public/js/admin.js +++ b/public/js/admin.js @@ -378,6 +378,12 @@ function adminTabActive() { var flagNextcloud = document.getElementById('cms-flag-nextcloud'); if (flagReadAloud) flagReadAloud.value = cfg['feature.read_aloud'] !== undefined ? cfg['feature.read_aloud'] : 'true'; if (flagNextcloud) flagNextcloud.value = cfg['feature.nextcloud'] !== undefined ? cfg['feature.nextcloud'] : 'true'; + var flagPreview = document.getElementById('cms-flag-assistant-preview'); + // Falls back to the key this setting used to live under, so the switch + // shows the real state on an installation that set it before the move. + if (flagPreview) flagPreview.value = + cfg['feature.assistant_preview'] !== undefined ? cfg['feature.assistant_preview'] : + (cfg['clinical_assistant.preview_enabled'] !== undefined ? cfg['clinical_assistant.preview_enabled'] : 'false'); // Auto-delete setting var autoDeleteDays = cfg['site.auto_delete_days'] || '7'; @@ -419,12 +425,24 @@ function adminTabActive() { var readAloud = document.getElementById('cms-flag-read-aloud').value; var nextcloud = document.getElementById('cms-flag-nextcloud').value; + var preview = (document.getElementById('cms-flag-assistant-preview') || {}).value; + var status = document.getElementById('cms-flags-status'); + if (status) status.textContent = 'Saving...'; + Promise.all([ putConfig('feature.read_aloud', readAloud), - putConfig('feature.nextcloud', nextcloud) + putConfig('feature.nextcloud', nextcloud), + // A feature flag, stored under feature.* like the others. The assistant + // still reads the old clinical_assistant.preview_enabled key when this one + // has never been set, so existing installations keep their setting. + putConfig('feature.assistant_preview', preview === 'true' ? 'true' : 'false') ]).then(function() { + if (status) status.textContent = 'Saved.'; showToast('Feature flags saved', 'success'); - }).catch(function() { showToast('Save failed', 'error'); }); + }).catch(function() { + if (status) status.textContent = 'Not saved.'; + showToast('Save failed', 'error'); + }); } // ---- EMAIL TEMPLATES ---- diff --git a/public/js/admin/clinicalAssistant.js b/public/js/admin/clinicalAssistant.js index 76f6a8be..e561c7ad 100644 --- a/public/js/admin/clinicalAssistant.js +++ b/public/js/admin/clinicalAssistant.js @@ -91,13 +91,18 @@ export function initClinicalAssistantAdmin(adminEscapeHtml) { function updateAssistantLoadState() { var save = document.getElementById('btn-save-assistant-config'); if (save) save.disabled = configState !== 'ready'; - var retry = document.getElementById('btn-retry-assistant-config'); - if (retry) retry.hidden = configState !== 'failed'; + // The retry lives inside an error message rather than sitting beside Save + // looking like an ordinary control, which is how it read before. + var errorBox = document.getElementById('assistant-config-error'); + if (errorBox) errorBox.hidden = configState !== 'failed'; var status = document.getElementById('assistant-admin-status'); - if (status) status.textContent = configState === 'failed' ? - 'Settings load failed. Drafts are unchanged. Retry loading settings or revisit the Admin tab.' : - configState !== 'ready' ? 'Loading assistant settings...' : - 'Settings ready.'; + // The failure case is spelled out in the error box above, so repeating it + // here would only be noise. + if (status && configState !== 'ready') { + status.textContent = configState === 'failed' ? '' : 'Loading settings...'; + } else if (status && !/^Saved |^Not saved/.test(status.textContent)) { + status.textContent = 'Settings loaded. Unsaved changes are kept until you press Save.'; + } } function loadAssistantAdmin() { @@ -170,8 +175,6 @@ export function initClinicalAssistantAdmin(adminEscapeHtml) { if (saved === undefined || saved === '') saved = cfg['clinical_assistant.citations_enabled']; // legacy key sourcesBox.checked = String(saved) !== 'false'; } - var previewBox = document.getElementById('assistant-preview-enabled'); - if (previewBox) previewBox.checked = String(cfg['clinical_assistant.preview_enabled']) === 'true'; var budgetInput = document.getElementById('assistant-conversation-budget'); var cfgBudget = cfg['clinical_assistant.conversation_chars']; if (budgetInput) { @@ -322,16 +325,17 @@ export function initClinicalAssistantAdmin(adminEscapeHtml) { putAssistantConfig('clinical_assistant.translate_provider', getValue('assistant-translate-provider') || 'libretranslate'), putAssistantConfig('clinical_assistant.show_sources', (document.getElementById('assistant-show-sources') || {}).checked === false ? 'false' : 'true'), - putAssistantConfig('clinical_assistant.preview_enabled', - (document.getElementById('assistant-preview-enabled') || {}).checked === true ? 'true' : 'false'), putAssistantConfig('clinical_assistant.allowed_models', checkedAssistantModels('assistant-allowed-chat-models').join(',')), putAssistantConfig('clinical_assistant.allowed_image_models', checkedAssistantModels('assistant-allowed-image-models').join(',')) ]).then(function() { - if (status) status.textContent = ''; + // A toast is gone in three seconds. Whether these settings are saved is + // exactly the question an admin has when they come back to this page, so + // the answer stays on the page. + if (status) status.textContent = 'Saved ' + new Date().toLocaleTimeString() + '.'; showToast('Assistant settings saved', 'success'); }).catch(function(err) { - if (status) status.textContent = ''; + if (status) status.textContent = 'Not saved. Nothing was changed.'; showToast(err.message || 'Save failed', 'error'); }); } diff --git a/src/routes/clinicalAssistant.js b/src/routes/clinicalAssistant.js index e8636bde..71238970 100644 --- a/src/routes/clinicalAssistant.js +++ b/src/routes/clinicalAssistant.js @@ -59,8 +59,17 @@ var PREVIEW_PATHS = new Set([ ]); var PREVIEW_USER = Object.freeze({ id: null, preview: true, role: 'preview' }); +// It is a feature flag and now lives with the others under feature.*, which also +// means an ordinary admin can still toggle it under ADMIN_LOCKDOWN — the +// clinical_assistant.* prefix is locked, and burying a day-to-day switch behind +// host access was not the intent. +// +// The old key is still honoured when the new one has never been written, so an +// installation that enabled preview before the move keeps it enabled. async function previewEnabled() { - return String(await getSetting('clinical_assistant.preview_enabled', 'false')) === 'true'; + var flag = await getSetting('feature.assistant_preview', ''); + if (String(flag) === '') flag = await getSetting('clinical_assistant.preview_enabled', 'false'); + return String(flag) === 'true'; } router.use(async function(req, res, next) { diff --git a/test/admin-clinical-assistant-wiring.test.js b/test/admin-clinical-assistant-wiring.test.js index 42f6081e..893912da 100644 --- a/test/admin-clinical-assistant-wiring.test.js +++ b/test/admin-clinical-assistant-wiring.test.js @@ -79,9 +79,13 @@ test('native admin initializer preserves lazy navigation, assistant actions and const writes = () => calls.filter(c => c.options.method === 'PUT'); const save = document.getElementById('btn-save-assistant-config'); save.click(); await tick(); - assert.equal(writes().length, 9); + // Eight, not nine: the signed-out preview is a feature flag and is saved by + // the Feature Flags card now, not by this button. + assert.equal(writes().length, 8); + assert.equal(writes().filter(c => /preview/.test(c.url)).length, 0, + 'this button no longer writes the preview flag'); assert.deepEqual(writes().map(c => c.url.split('/').pop()).sort(), [ - 'clinical_assistant.allowed_image_models', 'clinical_assistant.allowed_models', 'clinical_assistant.chat_model', 'clinical_assistant.context_chars', 'clinical_assistant.conversation_chars', 'clinical_assistant.preview_enabled', 'clinical_assistant.search_limit', 'clinical_assistant.show_sources', 'clinical_assistant.translate_provider' + 'clinical_assistant.allowed_image_models', 'clinical_assistant.allowed_models', 'clinical_assistant.chat_model', 'clinical_assistant.context_chars', 'clinical_assistant.conversation_chars', 'clinical_assistant.search_limit', 'clinical_assistant.show_sources', 'clinical_assistant.translate_provider' ]); assert.ok(toasts.some(([message, kind]) => message === 'Assistant settings saved' && kind === 'success')); diff --git a/test/clinical-release-integration.test.js b/test/clinical-release-integration.test.js index c841bdad..4ec8708e 100644 --- a/test/clinical-release-integration.test.js +++ b/test/clinical-release-integration.test.js @@ -85,7 +85,8 @@ test('native admin and assistant modules retain budget, table/source identity an document.getElementById('btn-save-assistant-config').click(); await tick(); assert.equal(limit, 2000); - assert.equal(calls.filter(call => call.options.method === 'PUT').length, 9, 'one native admin initializer; prompts are not generic setting saves'); + // Eight since the signed-out preview moved to the Feature Flags card. + assert.equal(calls.filter(call => call.options.method === 'PUT').length, 8, 'one native admin initializer; prompts are not generic setting saves'); assert.equal(calls.some(call => call.url.endsWith('/config/clinical_assistant.conversation_chars')), true, 'the conversation budget is an admin-settable override'); document.dispatchEvent(new window.CustomEvent('tabChanged', { detail: { tab: 'assistant' } })); await tick(); await tick(); await tick(); diff --git a/test/frontend-prompt-env.test.js b/test/frontend-prompt-env.test.js index a36915a7..7f64363e 100644 --- a/test/frontend-prompt-env.test.js +++ b/test/frontend-prompt-env.test.js @@ -289,7 +289,15 @@ test('assistant config GET503 plus Save makes zero PUTs; failed retry preserves await forceAssistantSave(ui); assert.equal(writes(ui).length, 0, 'failed configuration must never write defaults'); assert.equal(ui.document.getElementById('btn-save-assistant-config').disabled, true); - assert.match(setting(ui, 'admin-status').textContent, /failed|unavailable/i); + // The failure is stated in an error box now rather than in a status line + // beside Save, where a bare 'Retry loading settings' button read like an + // ordinary control that was always there. + const errorBox = ui.document.getElementById('assistant-config-error'); + assert.equal(errorBox.hidden, false, 'a failed load has to say so'); + assert.match(errorBox.textContent, /could not be loaded/i); + assert.match(errorBox.textContent, /nothing you have typed has been lost/i); + assert.ok(ui.document.getElementById('btn-retry-assistant-config'), + 'and the retry has to be reachable from inside it'); assert.equal(setting(ui, 'chat-model').options.length, 0); assert.equal(ui.document.getElementById('workflow-image-settings').children.length, 0, 'image settings load only after config success'); assert.equal(ui.calls.some(c => c.url.endsWith('/image-models/discover')), false); @@ -315,8 +323,11 @@ test('assistant config GET503 plus Save makes zero PUTs; failed retry preserves assert.equal(setting(ui, 'conversation-budget').value, '', 'no saved override, so the field stays empty'); assert.equal(setting(ui, 'conversation-budget').placeholder, '240000', 'environment metadata shows as the placeholder'); assert.equal(setting(ui, 'conversation-budget').readOnly, false); - assert.match(setting(ui, 'admin-status').textContent, /ready/i); - assert.equal(retry.hidden, true); + // Idle no longer claims 'ready'. What an admin actually needs to know is + // whether their change was applied, so the line reports that instead. + assert.match(setting(ui, 'admin-status').textContent, /Settings loaded|Saved /i); + // The error box hides on a successful load; the retry inside it goes with it. + assert.equal(ui.document.getElementById('assistant-config-error').hidden, true); adminVisit(ui); adminVisit(ui); await tick(); assert.equal(ui.calls.filter(c => c.url === '/api/admin/config').length, 3, 'ready revisits neither reload nor add handlers'); setting(ui, 'search-limit').value = '19'; @@ -328,7 +339,7 @@ test('assistant config GET503 plus Save makes zero PUTs; failed retry preserves ['clinical_assistant.conversation_chars', ''], ['clinical_assistant.search_limit', '19'], ['clinical_assistant.context_chars', '2300'], ['clinical_assistant.translate_provider', 'libretranslate'], - ['clinical_assistant.show_sources', 'true'], ['clinical_assistant.preview_enabled', 'false'], + ['clinical_assistant.show_sources', 'true'], ['clinical_assistant.allowed_models', ''], ['clinical_assistant.allowed_image_models', ''] ]); });