From 9788b167f26127b5df66e39cbef60338172e261c Mon Sep 17 00:00:00 2001 From: Daniel Date: Wed, 9 Sep 2026 23:35:06 +0200 Subject: [PATCH] refactor: retrieval is text-only; admins can add model ids discovery never returns Multimodal removal The multimodal path called nc_multimodal_search against a second hardcoded collection whose embedding service (multimodal-embeddings:7999) was never deployed and ENABLE_MULTIMODAL_RAG has always been false, so it only ever logged "multimodal search skipped". Removed rather than left as dead weight: - clinicalRetrieval: normalizeMcpMultimodalResponse, isVisualSourceQuery, isRadiologyQuery, buildMultimodalSearchQuery, classifyAndRerankMultimodalResults, selectMultimodalResults, visualIntent, visualMetadataScore, shouldRejectVisualSource, allowsFrontMatterQuery, looksLikeFrontMatterPage, looksLikeTextOnlyPage and MULTIMODAL_CANDIDATE_LIMIT (~140 lines). - clinicalMcpClient: multimodalSearch. - The route's visual/text slot split is gone; the whole search limit is text. - The "[visual PDF page match]" prompt label and the "visual PDF page" source badge are gone with it. Adding models Model availability could only be ticked from what the gateway advertised, so an admin could never offer a model discovery did not list. Each list now has a text field: a typed id joins the same checkbox list, is enabled by default, is de-duplicated, and persists through the normal allowed_models save. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BkfrkQwA4YGrGw9LZSpeAq --- public/components/admin.html | 9 ++ public/js/admin/clinicalAssistant.js | 32 ++++ public/js/assistant/sources.js | 1 - src/routes/clinicalAssistant.js | 39 ++--- src/utils/clinicalMcpClient.js | 9 -- src/utils/clinicalRetrieval.js | 150 +------------------ test/admin-clinical-assistant-wiring.test.js | 34 +++++ test/clinical-mcp-session-lifecycle.test.js | 4 +- test/clinical-retrieval-title.test.js | 8 +- 9 files changed, 92 insertions(+), 194 deletions(-) diff --git a/public/components/admin.html b/public/components/admin.html index 68c3dae8..e8356bf6 100644 --- a/public/components/admin.html +++ b/public/components/admin.html @@ -299,11 +299,20 @@
Chat models
+
+ + +
Image models
+
+ + +
+

Discovery only lists what the gateway advertises. Add any id your gateway routes — it is checked on first use, and stays in the list once saved.

diff --git a/public/js/admin/clinicalAssistant.js b/public/js/admin/clinicalAssistant.js index 59070dbd..33deafa1 100644 --- a/public/js/admin/clinicalAssistant.js +++ b/public/js/admin/clinicalAssistant.js @@ -51,6 +51,38 @@ export function initClinicalAssistantAdmin(adminEscapeHtml) { container.appendChild(row); }); } + // Discovery only returns what the gateway advertises, so an admin could never + // offer a model it does not list. Typed ids join the same checkbox list and + // persist through the normal allowed_models save. + function addAssistantModel(containerId, id) { + var container = document.getElementById(containerId); + id = String(id || '').trim(); + if (!container || !id) return false; + var existing = Array.prototype.map.call(container.querySelectorAll('input[type="checkbox"]'), function(b) { return b.value; }); + if (existing.indexOf(id) !== -1) { + var already = container.querySelector('input[value="' + (window.CSS && CSS.escape ? CSS.escape(id) : id) + '"]'); + if (already) already.checked = true; + return true; + } + var roster = containerId === 'assistant-allowed-chat-models' ? chatRoster : imageRoster; + roster.push(id); + renderAssistantCheckboxList(containerId, roster, existing.filter(function(v) { + var box = container.querySelector('input[value="' + (window.CSS && CSS.escape ? CSS.escape(v) : v) + '"]'); + return box && box.checked; + }).concat([id])); + return true; + } + + document.addEventListener('click', function(event) { + var button = event.target.closest && event.target.closest('[data-assistant-add-model]'); + if (!button) return; + var containerId = button.getAttribute('data-assistant-add-model'); + var field = document.getElementById(containerId === 'assistant-allowed-chat-models' ? 'assistant-add-chat-model' : 'assistant-add-image-model'); + if (!field) return; + if (addAssistantModel(containerId, field.value)) field.value = ''; + else if (typeof window.showToast === 'function') window.showToast('Enter a model id first', 'error'); + }); + function checkedAssistantModels(containerId) { var container = document.getElementById(containerId); if (!container) return []; diff --git a/public/js/assistant/sources.js b/public/js/assistant/sources.js index 4e9df303..801a5327 100644 --- a/public/js/assistant/sources.js +++ b/public/js/assistant/sources.js @@ -7,7 +7,6 @@ export function renderSourcesList(sources) { var page = s.page || s.page_number || s.pageNumber; var meta = []; if (page) meta.push('page ' + page); - if (s.source_type === 'multimodal_page') meta.push('visual PDF page'); if (s.visual_caption_source) meta.push('caption: ' + s.visual_caption_source); if (s.visual_kind || s.source_priority) meta.push(s.visual_kind || s.source_priority); if (s.category) meta.push(s.category); diff --git a/src/routes/clinicalAssistant.js b/src/routes/clinicalAssistant.js index 982ac9b4..c55d345d 100644 --- a/src/routes/clinicalAssistant.js +++ b/src/routes/clinicalAssistant.js @@ -20,7 +20,6 @@ var redisCache = require('../utils/redis'); var { createClinicalPromptPool } = require('../utils/clinicalPromptPool'); var { semanticSearch, - multimodalSearch, indexedTopicSuggestions, getMcpHealth, warmMcpSession @@ -28,11 +27,7 @@ var { var { cleanSourceExcerpt, normalizeMcpSearchResponse, - normalizeMcpMultimodalResponse, dedupeSources, - isVisualSourceQuery, - buildMultimodalSearchQuery, - classifyAndRerankMultimodalResults } = require('../utils/clinicalRetrieval'); var { buildSystemPrompt, @@ -552,28 +547,14 @@ async function prepareAssistantChat(body) { includeContext: includeContext, contextChars: contextChars }); - var visualQuery = isVisualSourceQuery(message) || isVisualSourceQuery(searchQuery); - var multimodalResponse = visualQuery ? await multimodalSearch(buildMultimodalSearchQuery(searchQuery), { limit: 8 }).catch(function(e) { - console.warn('[clinical-assistant] multimodal search skipped:', e.message); - return null; - }) : null; - var rawTextResults = normalizeMcpSearchResponse(searchResponse); - var rawMultimodalResults = await classifyAndRerankMultimodalResults( - message + ' ' + searchQuery, - normalizeMcpMultimodalResponse(multimodalResponse) - ); - var rawResults = rawTextResults.concat(rawMultimodalResults); - console.info('[clinical-assistant] retrieval counts:', { - text: rawTextResults.length, - multimodal: rawMultimodalResults.length - }); - var visualSlots = rawMultimodalResults.length ? Math.min(2, Math.max(1, Math.floor(searchLimit / 4))) : 0; - var textSlots = searchLimit - visualSlots; - var sources = dedupeSources( - dedupeSources(rawTextResults).slice(0, textSlots).concat( - dedupeSources(rawMultimodalResults).slice(0, visualSlots) - ) - ).slice(0, searchLimit); + // Retrieval is text-only. The multimodal path called nc_multimodal_search + // against a second hardcoded collection with an embedding service that was + // never deployed, so it only ever logged "multimodal search skipped". + var rawResults = normalizeMcpSearchResponse(searchResponse); + console.info('[clinical-assistant] retrieval count:', rawResults.length); + // No visual/text slot split any more: every result is text, so the whole + // search limit goes to it. + var sources = dedupeSources(rawResults).slice(0, searchLimit); if (sources.length === 0) { return { direct: { @@ -601,7 +582,6 @@ async function prepareAssistantChat(body) { ], search: { totalFound: rawResults.length, - multimodalFound: rawMultimodalResults.length, query: searchQuery, rewritten: searchQuery !== message, verifiedChunkCount: searchResponse.verified_chunk_count || searchResponse.verifiedChunkCount || 0, @@ -646,8 +626,7 @@ function needsContextualRewrite(message) { function formatSourcesForPrompt(sources) { return sources.map(function(s) { - var label = s.source_type === 'multimodal_page' ? ' [visual PDF page match]' : ''; - return '[' + s.number + '] ' + s.title + (s.page ? ', page ' + s.page : '') + label + '\n' + cleanSourceExcerpt(s.excerpt); + return '[' + s.number + '] ' + s.title + (s.page ? ', page ' + s.page : '') + '\n' + cleanSourceExcerpt(s.excerpt); }).join('\n\n---\n\n'); } diff --git a/src/utils/clinicalMcpClient.js b/src/utils/clinicalMcpClient.js index 8555eaba..3fae5326 100644 --- a/src/utils/clinicalMcpClient.js +++ b/src/utils/clinicalMcpClient.js @@ -84,14 +84,6 @@ async function semanticSearch(query, opts) { }); } -async function multimodalSearch(query, opts) { - opts = opts || {}; - return callMcpTool('nc_multimodal_search', { - query: query, - limit: opts.limit - }); -} - async function indexedTopicSuggestions(limit) { return callMcpTool('nc_indexed_topic_suggestions', { limit: limit || 12, @@ -342,7 +334,6 @@ function parseMcpResponse(body) { module.exports = { semanticSearch: semanticSearch, - multimodalSearch: multimodalSearch, indexedTopicSuggestions: indexedTopicSuggestions, getMcpHealth: getMcpHealth, warmMcpSession: warmMcpSession, diff --git a/src/utils/clinicalRetrieval.js b/src/utils/clinicalRetrieval.js index 89a480ab..5699d408 100644 --- a/src/utils/clinicalRetrieval.js +++ b/src/utils/clinicalRetrieval.js @@ -1,5 +1,4 @@ var sourceMarkdown = require('markdown-it')({ html: false }); -var MULTIMODAL_CANDIDATE_LIMIT = 6; var INTERNAL_TITLE_FALLBACKS = { '1586022': 'Respiratory Disease', 'FABK010-fm[i-xiv].qxd': 'Respiratory Disease' @@ -42,54 +41,6 @@ function normalizeMcpSearchResponse(result) { }).filter(function(r) { return r.excerpt; }); } -function normalizeMcpMultimodalResponse(result) { - var data = result && (result.structuredContent || result.data || result); - if ((!data || !Array.isArray(data.results)) && result && Array.isArray(result.content)) { - for (var i = 0; i < result.content.length; i++) { - var c = result.content[i]; - if (c && c.type === 'text' && c.text) { - try { - var parsed = JSON.parse(c.text); - if (parsed && Array.isArray(parsed.results)) data = parsed; - } catch (e) {} - } - } - } - if (!data || !Array.isArray(data.results)) return []; - return data.results.map(function(r, idx) { - var nearby = clip(cleanSourceExcerpt(r.nearby_text || ''), 1400); - var caption = clip(cleanSourceExcerpt(r.visual_caption || ''), 900); - var labels = Array.isArray(r.visual_labels) ? r.visual_labels.filter(Boolean).slice(0, 20) : []; - var excerptParts = []; - if (caption) excerptParts.push('[Visual caption] ' + caption); - if (labels.length) excerptParts.push('[Visual labels] ' + labels.join(', ')); - if (nearby) excerptParts.push('[Page text] ' + nearby); - return { - number: idx + 1, - id: r.id, - doc_type: r.doc_type || 'file', - source_type: 'multimodal_page', - title: displayTitleForSource(r, 'PDF page image'), - file_path: r.file_path || '', - category: r.category || '', - subcategory: r.subcategory || '', - category_path: r.category_path || '', - page: r.page_number || r.pageNumber || null, - page_count: r.page_count || r.pageCount || null, - visual_caption: caption, - visual_labels: labels, - visual_caption_source: r.visual_caption_source || '', - is_cover_page: Boolean(r.is_cover_page || r.isCoverPage), - is_title_page: Boolean(r.is_title_page || r.isTitlePage), - reject_for_visual_search: Boolean(r.reject_for_visual_search || r.rejectForVisualSearch), - excerpt: excerptParts.length ? '[Page-image match] ' + excerptParts.join('\n') : '[Page-image match] Rendered PDF page matched the visual/text query.', - score: r.score, - chunk_index: 'page-image-' + (r.page_number || idx + 1), - total_chunks: r.page_count || null - }; - }).filter(function(r) { return r.page; }); -} - function dedupeSources(results) { var seen = new Map(); var out = []; @@ -125,100 +76,6 @@ function mergeDuplicateSource(existing, duplicate) { return merged; } -function isVisualSourceQuery(query) { - return visualIntent(query).wanted.length > 0; -} - -function isRadiologyQuery(query) { - return visualIntent(query).wanted.indexOf('radiology') !== -1; -} - -function buildMultimodalSearchQuery(query) { - query = String(query || '').trim(); - var intent = visualIntent(query); - if (intent.wanted.indexOf('radiology') !== -1) return query + ' radiograph x-ray chest imaging radiology'; - if (intent.wanted.indexOf('diagram') !== -1) return query + ' figure diagram pathway flowchart algorithm illustration'; - if (intent.wanted.indexOf('photo') !== -1) return query + ' clinical photograph image skin lesion pathology microscopy'; - return query; -} - -async function classifyAndRerankMultimodalResults(query, results) { - results = Array.isArray(results) ? results : []; - if (!results.length) return []; - var candidates = results.slice(0, MULTIMODAL_CANDIDATE_LIMIT); - return selectMultimodalResults(query, candidates); -} - -function selectMultimodalResults(query, results) { - results = Array.isArray(results) ? results : []; - if (!results.length) return []; - results = results.filter(function(r) { return !shouldRejectVisualSource(query, r); }); - if (!results.length) return []; - if (!isRadiologyQuery(query)) return results.filter(function(r) { return !looksLikeTextOnlyPage(r); }); - var radiologyTerms = /\b(radiology|radiologic|radiograph|x-?ray|cxr|chest\s*(film|x-?ray|radiograph)|imaging|ultrasound|ct|mri|scan|film|pneumonia|bronchiolitis|lung|airway|hyperinflation|infiltrate|consolidation|atelectasis)\b/i; - return results.filter(function(r) { - return radiologyTerms.test([r.title, r.excerpt, r.file_path, r.category, r.subcategory, r.category_path].filter(Boolean).join(' ')); - }); -} - -function visualIntent(query) { - var text = String(query || '').toLowerCase(); - var wanted = []; - if (/\b(radiology|radiologic|radiograph|x-?ray|cxr|chest\s*(film|x-?ray|radiograph)|imaging|ultrasound|ct|mri|scan|film)\b/i.test(text)) { - wanted.push('radiology'); - } - if (/\b(algorithm|flow\s*chart|flowchart|pathway|diagram|graph|chart|figure|figures|illustration|infographic|schema|schematic|table)\b/i.test(text)) { - wanted.push('diagram'); - } - if (/\b(photo|photograph|picture|skin|rash|lesion|wound|gross|microscopy|histology|pathology image|clinical image)\b/i.test(text)) { - wanted.push('photo'); - } - if (!wanted.length && /\b(visual|image|images|show|find|display|preview)\b/i.test(text)) { - wanted.push('diagram', 'photo'); - } - return { wanted: Array.from(new Set(wanted)) }; -} - -function visualMetadataScore(query, source) { - var haystack = [source.title, source.excerpt, source.visual_caption, (source.visual_labels || []).join(' '), source.file_path, source.category, source.subcategory, source.category_path].filter(Boolean).join(' '); - var score = 0; - var intent = visualIntent(query); - if (isRadiologyQuery(query) && /\b(radiology|radiograph|x-?ray|cxr|imaging|ct|mri|ultrasound|film)\b/i.test(haystack)) score += 0.12; - if (intent.wanted.indexOf('diagram') !== -1 && /\b(figure|diagram|algorithm|flowchart|pathway|chart|illustration|infographic)\b/i.test(haystack)) score += 0.12; - if (intent.wanted.indexOf('photo') !== -1 && /\b(photo|photograph|skin|rash|lesion|wound|gross|microscopy|histology)\b/i.test(haystack)) score += 0.12; - if (/\b(pneumonia|bronchiolitis|lung|airway|hyperinflation|infiltrate|consolidation|atelectasis)\b/i.test(haystack)) score += 0.06; - if (looksLikeFrontMatterPage(source)) score -= 0.5; - if (looksLikeTextOnlyPage(source)) score -= 0.18; - return score; -} - -function shouldRejectVisualSource(query, source) { - if (!source) return false; - if (allowsFrontMatterQuery(query)) return false; - return Boolean(source.reject_for_visual_search || source.is_cover_page || source.is_title_page || looksLikeFrontMatterPage(source)); -} - -function allowsFrontMatterQuery(query) { - return /\b(cover|front cover|title page|copyright|table of contents|contents|preface|foreword|publisher|isbn)\b/i.test(String(query || '')); -} - -function looksLikeFrontMatterPage(source) { - var page = Number(source && source.page) || 0; - var pageCount = Number(source && source.page_count) || 0; - var earlyLimit = Math.min(8, Math.max(2, pageCount || 8)); - if (!page || page > earlyLimit) return false; - var haystack = [source.title, source.excerpt, source.visual_caption, (source.visual_labels || []).join(' '), source.file_path].filter(Boolean).join(' '); - var hasVisualSignal = /\b(figure|fig\.?|radiograph|x-?ray|cxr|ultrasound|ct|mri|photograph|photo|image|diagram|flowchart|algorithm|histology|microscopy|gross pathology)\b/i.test(haystack); - if (hasVisualSignal) return false; - var frontMatterSignal = /\b(cover|front cover|title page|copyright|all rights reserved|isbn|library of congress|table of contents|contents|preface|foreword|dedication|contributors?|textbook of|manual of|review of|board review|edition|authors?|editor|publisher)\b/i.test(haystack); - return frontMatterSignal || page <= 2; -} - -function looksLikeTextOnlyPage(source) { - var text = String(source && source.excerpt || ''); - return text.length > 900 && !/\b(figure|image|radiograph|x-?ray|cxr|ultrasound|ct|mri|scan|film|diagram|illustration)\b/i.test(text); -} - function clip(s, n) { return String(s || '').replace(/\s+/g, ' ').trim().substring(0, n); } function displayTitleForSource(source, fallback) { @@ -349,10 +206,5 @@ module.exports = { cleanSourceExcerpt: cleanSourceExcerpt, clipSourceExcerpt: clipSourceExcerpt, normalizeMcpSearchResponse: normalizeMcpSearchResponse, - normalizeMcpMultimodalResponse: normalizeMcpMultimodalResponse, - dedupeSources: dedupeSources, - isVisualSourceQuery: isVisualSourceQuery, - buildMultimodalSearchQuery: buildMultimodalSearchQuery, - classifyAndRerankMultimodalResults: classifyAndRerankMultimodalResults, - visualMetadataScore: visualMetadataScore + dedupeSources: dedupeSources }; diff --git a/test/admin-clinical-assistant-wiring.test.js b/test/admin-clinical-assistant-wiring.test.js index e4e4ee67..b5577762 100644 --- a/test/admin-clinical-assistant-wiring.test.js +++ b/test/admin-clinical-assistant-wiring.test.js @@ -125,3 +125,37 @@ test('real extracted initializer displays the server default only when returned assert.match(document.getElementById('assistant-conversation-budget-meta').textContent, /built-in default/, "source 'default' must not be reported as coming from the environment variable"); }); + +test('an admin can add a model id discovery never returned', async t => { + const dom = new JSDOM(read('public/components/admin.html')); + browserGlobals(t, dom, async () => ({ ok: true, json: async () => ({ + success: true, config: [], models: [{ id: 'discovered-model', name: 'Discovered' }], + conversationBudget: { limit: 120000, source: 'default' } + }) }), []); + const { initClinicalAssistantAdmin } = await import(pathToFileURL(path.join(root, 'public/js/admin/clinicalAssistant.js')).href); + initClinicalAssistantAdmin(value => value); + document.dispatchEvent(new dom.window.CustomEvent('tabChanged', { detail: { tab: 'admin' } })); + await tick(); + + const list = document.getElementById('assistant-allowed-chat-models'); + const field = document.getElementById('assistant-add-chat-model'); + assert.ok(field, 'there is somewhere to type a model id'); + const before = list.querySelectorAll('input[type="checkbox"]').length; + + field.value = 'openrouter-deepseek-v4-pro'; + document.querySelector('[data-assistant-add-model="assistant-allowed-chat-models"]').click(); + await tick(); + + const values = Array.from(list.querySelectorAll('input[type="checkbox"]')).map(b => b.value); + assert.ok(values.includes('openrouter-deepseek-v4-pro'), 'the typed id joins the list'); + assert.equal(values.length, before + 1); + const added = list.querySelector('input[value="openrouter-deepseek-v4-pro"]'); + assert.equal(added.checked, true, 'and is enabled, since that is why it was added'); + assert.equal(field.value, '', 'the field clears for the next one'); + + // Adding the same id twice must not duplicate the row. + field.value = 'openrouter-deepseek-v4-pro'; + document.querySelector('[data-assistant-add-model="assistant-allowed-chat-models"]').click(); + await tick(); + assert.equal(list.querySelectorAll('input[value="openrouter-deepseek-v4-pro"]').length, 1); +}); diff --git a/test/clinical-mcp-session-lifecycle.test.js b/test/clinical-mcp-session-lifecycle.test.js index 6dabf91e..d0bbb5d2 100644 --- a/test/clinical-mcp-session-lifecycle.test.js +++ b/test/clinical-mcp-session-lifecycle.test.js @@ -57,12 +57,12 @@ test('live session reuse, real initialized notification and serialized tool call } }); const first = client.semanticSearch('one', { limit: 4 }); await started.promise; - const second = client.multimodalSearch('two', { limit: 8 }); + const second = client.indexedTopicSuggestions(8); // any second tool call proves serialization await turn(); assert.equal(tools, 1); held.resolve(); await Promise.all([first, second]); - await client.indexedTopicSuggestions(12); + await client.semanticSearch('three', { limit: 4 }); assert.deepEqual(calls.filter(c => c.method === 'POST').map(method), ['initialize', 'notifications/initialized', 'tools/call', 'tools/call', 'tools/call']); const notice = calls[1]; assert.equal(notice.payload.jsonrpc, '2.0'); diff --git a/test/clinical-retrieval-title.test.js b/test/clinical-retrieval-title.test.js index a90f36a5..30cd0b1c 100644 --- a/test/clinical-retrieval-title.test.js +++ b/test/clinical-retrieval-title.test.js @@ -1,7 +1,7 @@ const { test } = require('node:test'); const assert = require('node:assert/strict'); -const { normalizeMcpSearchResponse, normalizeMcpMultimodalResponse } = require('../src/utils/clinicalRetrieval'); +const { normalizeMcpSearchResponse } = require('../src/utils/clinicalRetrieval'); test('clinical retrieval replaces known internal PDF title with document title', () => { const results = normalizeMcpSearchResponse({ @@ -19,13 +19,15 @@ test('clinical retrieval replaces known internal PDF title with document title', }); test('clinical retrieval prefers file basename for internal PDF production titles', () => { - const results = normalizeMcpMultimodalResponse({ + // Retrieval is text-only now; this used to run through the multimodal + // normalizer, but the title cleanup it checks belongs to the shared path. + const results = normalizeMcpSearchResponse({ results: [{ id: 99, doc_type: 'file', title: 'chapter-layout.qxd', file_path: 'Medical Library/Critical%20care%20%26%20Respiratory/Respiratory%20Disease.pdf', - visual_caption: 'Airway diagram', + excerpt: 'Airway anatomy and management in the critically ill child.', page_number: 12 }] });