From 1f06a19007bdf356a1f4a559b77f14caee94135f Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 12 Sep 2026 15:43:36 +0200 Subject: [PATCH] feat: a text-only model can ask a model that can see; and the image regex is gone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **The regex is gone.** The route ran a pattern over the user's message and enqueued an image from the answer text when the model had not called the tool. It was a compatibility path for models without tool calling and it did more harm than good: it decided in English only, it could not see the conversation, and "image summary" fell through it while reading as an obvious image request to the model itself — which was measured, not assumed. A second and worse decision-maker sitting behind the first. Whether a message deserves a picture is now the model's call, made from the tool description, which is the only place it ever belonged. **Lending eyes.** The same shape, for a different capability. When someone attaches a photograph and the chat model cannot accept image input, the attachment was either refused by the provider or silently dropped — an answer about a picture nobody had looked at, which is worse than a refusal. The chat model is now offered look_at_image beside the image tool and decides when to use it. The attachment goes to clinical_assistant.vision_model, whose description comes back as a tool result, and the chat model answers in its own voice with its own sources. Only the seeing is delegated; the clinical reasoning stays with the model an administrator chose. The seeing model is told to report and not to diagnose, because it has a picture and no context and an opinion from it would carry weight it has not earned. Delegation triggers only on an explicit supports_vision: false from the gateway. An unknown is left alone — most of a roster reports nothing, and treating silence as blindness would route good models through a detour. The capability lookup moved to its own module, is cached for five minutes because it runs on exactly the requests that are already slowest, and is never inferred from the model id. liteLLMBaseUrl moved from the admin route to litellm.js, where the other gateway helpers live. The new setting is guarded like the slide reviewer: a model the gateway calls text-only cannot be saved as the one that looks at images. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- docs/clinical-assistant.md | 25 ++++ src/routes/adminConfig.js | 7 +- src/routes/clinicalAssistant.js | 93 ++++++++------- src/utils/litellm.js | 9 +- src/utils/modelVision.js | 52 +++++++++ src/utils/visionTool.js | 117 +++++++++++++++++++ test/assistant-attachment-roundtrip.test.js | 2 + test/assistant-image-attachments.test.js | 2 + test/assistant-image-intent.test.js | 14 ++- test/assistant-translate.test.js | 2 + test/clinical-conversation.test.js | 2 + test/generated-image-tools.test.js | 2 + test/generated-images.integration.js | 2 + test/patient-takehome.test.js | 21 +++- test/review-model-vision-guard.test.js | 6 +- test/vision-tool.test.js | 121 ++++++++++++++++++++ 16 files changed, 419 insertions(+), 58 deletions(-) create mode 100644 src/utils/modelVision.js create mode 100644 src/utils/visionTool.js create mode 100644 test/vision-tool.test.js diff --git a/docs/clinical-assistant.md b/docs/clinical-assistant.md index c037474e..9d507b1c 100644 --- a/docs/clinical-assistant.md +++ b/docs/clinical-assistant.md @@ -124,6 +124,31 @@ default in the right-hand column. deployment, which is the real ceiling on every search. See [retrieval-tuning.md](retrieval-tuning.md). +## Lending eyes to a text-only model + +`clinical_assistant.vision_model`, when set, is the model shown an attachment +that the chat model cannot be shown. + +The chat model is offered a `look_at_image` tool alongside the image tool and +decides when to use it, exactly as it decides about drawing. The attachment is +withheld from its own request — sending an image to a model that cannot accept +one is either refused by the provider or silently dropped, and an answer about a +picture nobody looked at is worse than a refusal. + +Delegation only happens when the gateway reports `supports_vision: false` for +the chat model. An unknown is left alone: most of a roster carries no +`supports_vision` at all, and treating silence as blindness would route +perfectly good models through a detour they do not need. The capability is read +from `/model/info` and cached for five minutes, never inferred from the model id. + +The seeing model is told to report and not to diagnose: it has a picture and no +conversation, no retrieved sources and no system prompt, so an opinion from it +would carry weight it has not earned. Its description returns as a tool result +and the chat model answers in its own voice, from words. + +Saving the setting is refused if the gateway reports that model as text-only — +the same check that guards the slide reviewer. + ## Environment variables Settings above are the normal way to configure the assistant. These environment diff --git a/src/routes/adminConfig.js b/src/routes/adminConfig.js index 85a1aa2b..6eb3a74c 100644 --- a/src/routes/adminConfig.js +++ b/src/routes/adminConfig.js @@ -986,12 +986,13 @@ router.put('/config/:key(*)', async function(req, res) { // moment they could have chosen differently. Refused only when the gateway // states supports_vision === false; an unknown is left to the administrator, // which is how it worked before there was any check at all. - if (key === 'my_resources.review_model' && String(value).trim()) { + if ((key === 'my_resources.review_model' || key === 'clinical_assistant.vision_model') + && String(value).trim()) { var canSee = await liteLLMVisionSupport(String(value).trim()); if (canSee === false) { return res.status(400).json({ - error: String(value).trim() + ' is a text-only model, so it cannot look at ' + - 'rendered slides. Choose a model the gateway reports as vision-capable.' + error: String(value).trim() + ' is a text-only model, so it cannot be shown ' + + 'an image. Choose a model the gateway reports as vision-capable.' }); } } diff --git a/src/routes/clinicalAssistant.js b/src/routes/clinicalAssistant.js index b41186f2..10ac2884 100644 --- a/src/routes/clinicalAssistant.js +++ b/src/routes/clinicalAssistant.js @@ -13,6 +13,8 @@ var { authMiddleware } = require('../middleware/auth'); var { callAI, callAIStream } = require('../utils/ai'); var generatedImages = require('../utils/generatedImages'); var imageTool = require('../utils/imageTool'); +var visionTool = require('../utils/visionTool'); +var modelVision = require('../utils/modelVision'); var imageLinks = require('../utils/generatedImageLinks'); var logger = require('../utils/logger'); var cryptoUtil = require('../utils/crypto'); @@ -324,27 +326,42 @@ function trackCitations(req, question, answer, sources) { tracker.store(req.user.id, question, result, sources); } +// A model that cannot be shown the attachment is given a tool to ask about it +// instead, and the attachment is withheld from its own request — sending it +// would either be refused by the provider or silently dropped, and an answer +// about a picture nobody looked at is worse than a refusal. +function assistantToolset(prepared) { + return prepared.delegateVision + ? { tools: imageTool.tools.concat(visionTool.tools), images: undefined } + : { tools: imageTool.tools, images: prepared.images }; +} + router.post('/clinical-assistant/chat', async function(req, res) { var started = Date.now(); try { var prepared = await prepareAssistantChat(req.body); if (prepared.direct) return res.json(prepared.direct); + var toolset = assistantToolset(prepared); var ai = await callAI(prepared.messages, assistantGenerationOptions({ model: prepared.chatModel || undefined, temperature: 0.15, - tools: imageTool.tools, + tools: toolset.tools, maxTokens: 2600, - images: prepared.images + images: toolset.images })); + ai = await visionTool.dispatch(ai, { + images: prepared.images, visionModel: prepared.visionModel, callAI: callAI, + messages: prepared.messages, + options: assistantGenerationOptions({ model: prepared.chatModel || undefined, temperature: 0.15, maxTokens: 2600 }) + }); ai = await imageTool.dispatch(ai, { owner: req.user.id, workflow: 'clinical_assistant', body: req.body, imageContext: prepared.imageContext, imageModel: prepared.imageModel, messages: prepared.messages, options: assistantGenerationOptions({ model: prepared.chatModel || undefined, temperature: 0.15 }), callAI: callAI }); - ai = await dispatchImageRequestFallback(ai, prepared, req); var finalized = ai.imageToolHandled ? { answer: String(ai.content || ''), ai: ai } : await finalizeAssistantAnswer(ai, { messages: prepared.messages, chatModel: prepared.chatModel, callAI: callAI, - generationOptions: assistantGenerationOptions({ temperature: 0.15, images: prepared.images }) + generationOptions: assistantGenerationOptions({ temperature: 0.15, images: assistantToolset(prepared).images }) }); var answer = finalized.answer; ai = finalized.ai; @@ -404,27 +421,33 @@ router.post('/clinical-assistant/chat/stream', async function(req, res) { sendEvent('sources', { sources: safeSources, search: prepared.search }); sendEvent('status', { message: 'Generating answer...' }); + var toolset = assistantToolset(prepared); + if (prepared.delegateVision) sendEvent('status', { message: 'Looking at your image…' }); var ai = await callAIStream(prepared.messages, assistantGenerationOptions({ model: prepared.chatModel || undefined, temperature: 0.15, - tools: imageTool.tools, + tools: toolset.tools, maxTokens: 2600, - images: prepared.images + images: toolset.images }), function(delta) { sendEvent('token', { token: delta }); }); + ai = await visionTool.dispatch(ai, { + images: prepared.images, visionModel: prepared.visionModel, callAI: callAI, + messages: prepared.messages, + options: assistantGenerationOptions({ model: prepared.chatModel || undefined, temperature: 0.15, maxTokens: 2600 }) + }); { ai = await imageTool.dispatch(ai, { owner: req.user.id, workflow: 'clinical_assistant', body: req.body, imageContext: prepared.imageContext, imageModel: prepared.imageModel, messages: prepared.messages, options: assistantGenerationOptions({ model: prepared.chatModel || undefined, temperature: 0.15 }), callAI: callAI }); - ai = await dispatchImageRequestFallback(ai, prepared, req); } if (ai.imageToolHandled) sendEvent('status', { message: 'Generating image…' }); var finalized = ai.imageToolHandled ? { answer: String(ai.content || ''), ai: ai } : await finalizeAssistantAnswer(ai, { messages: prepared.messages, chatModel: prepared.chatModel, callAI: callAI, - generationOptions: assistantGenerationOptions({ temperature: 0.15, images: prepared.images }), + generationOptions: assistantGenerationOptions({ temperature: 0.15, images: assistantToolset(prepared).images }), streamed: true, onRegenerating: function() { sendEvent('status', { message: 'Completing answer...' }); } }); @@ -461,20 +484,16 @@ router.post('/clinical-assistant/chat/stream', async function(req, res) { }); // Model-agnostic guarantee: when the user explicitly asks for an image and -// the model answered with text instead of invoking the tool, run the job from -// the model's answer text anyway. Deterministic — never depends on tool-call -// support of the serving model. -// Compatibility path only, for a serving model that writes the image prompt as -// text instead of calling the tool. It is gated on the USER's own message, never -// on the model's answer, so it cannot replay a previous turn. Whether an -// acknowledgement deserves an image is the model's decision, made from the -// system prompt and the tool description — not from a pattern here. -const IMAGE_REQUEST_PATTERN = /(create|generate|draw|make|illustrate|render|visualize)\b[^.!?\n]{0,160}\b(image|figure|diagram|poster|infographic|illustration|chart|visual|graphic)/i; - -function isExplicitImageRequest(message) { - return IMAGE_REQUEST_PATTERN.test(String(message || '')); -} - +// Whether a message deserves a picture is the model's decision, made from the +// system prompt and the tool description. There is no pattern here any more. +// +// There used to be one: a regex over the user's message that queued an image +// from the answer text when the model had not called the tool. It was a +// compatibility path for models without tool calling, and it did more harm than +// good — it decided in English only, it could not see the conversation, and +// "image summary" fell through it while reading as an obvious image request to +// the model itself. A second, worse decision-maker sitting behind the first. +// // Typing "Окей" after an image turn produced another image every time: the model // saw its own "I'll generate an educational image…" in the history and replayed // it verbatim. Detecting acknowledgements by vocabulary cannot cover every @@ -485,28 +504,6 @@ function normalizeAnswerForRepeatCheck(text) { return String(text || '').toLowerCase().replace(/\s+/g, ' ').trim(); } -async function dispatchImageRequestFallback(ai, prepared, req) { - if (!ai || ai.imageToolHandled || (ai.toolCalls && ai.toolCalls.length)) return ai; - if (!isExplicitImageRequest(prepared.message)) return ai; - var text = String(ai.content || '').trim(); - if (!text || text.length > 32000) return ai; - try { - var job = await generatedImages.service().enqueue( - req.user.id, 'clinical_assistant', - { prompt: text }, - 'img:' + generatedImages.requestKey({ prompt: text, ts: Date.now() }), - true, - prepared.imageContext, - prepared.imageModel); - ai.imageJobs = [job]; - ai.imageToolHandled = true; - logger.info('[clinical-assistant] image requested via text; queued image job', { jobId: job.jobId, model: job.model }); - } catch (e) { - logger.warn('[clinical-assistant] image fallback enqueue failed', { error: e && e.message }); - } - return ai; -} - async function submitImage(req, res, synchronous) { try { const body = req.body || {}; @@ -568,6 +565,8 @@ async function prepareAssistantChat(body) { var chatModel = await resolveAssistantChatModel(body); var imageModel = await resolveAssistantImageModel(body); + // The model that gets shown an attachment when the chat model cannot be. + var visionModel = String(await getSetting('clinical_assistant.vision_model', '') || ''); var searchLimit = clampInt(await getSetting('clinical_assistant.search_limit', '8'), 3, 20, 8); var contextChars = clampInt(await getSetting('clinical_assistant.context_chars', '1400'), 300, 4000, 1400); var behavior = await getSetting('clinical_assistant.system_behavior', DEFAULT_BEHAVIOR) || DEFAULT_BEHAVIOR; @@ -608,6 +607,12 @@ async function prepareAssistantChat(body) { showSources: showSources, images: images, imageContext: generatedImages.imageContext(message, history), + // Whether this model can be shown the attachment itself. Only an explicit + // "no" from the gateway triggers delegation: a model the gateway says + // nothing about is given the image, which is how it has always worked. + delegateVision: images.length > 0 && visionModel + && (await modelVision.supportsVision(chatModel || await getSetting('models.default', ''))) === false, + visionModel: visionModel, history: history, chatModel: chatModel, imageModel: imageModel, diff --git a/src/utils/litellm.js b/src/utils/litellm.js index 166e2741..b7ab80b9 100644 --- a/src/utils/litellm.js +++ b/src/utils/litellm.js @@ -14,7 +14,14 @@ function getLiteLLMAdminHeaders(contentType) { return headers; } +// The gateway root, with any trailing /v1 stripped: admin endpoints such as +// /model/info sit beside /v1, not under it. +function liteLLMBaseUrl() { + return (process.env.LITELLM_API_BASE || '').replace(/\/+$/, '').replace(/\/v1\/?$/, ''); +} + module.exports = { getLiteLLMHeaders, - getLiteLLMAdminHeaders + getLiteLLMAdminHeaders, + liteLLMBaseUrl }; diff --git a/src/utils/modelVision.js b/src/utils/modelVision.js new file mode 100644 index 00000000..ccefa782 --- /dev/null +++ b/src/utils/modelVision.js @@ -0,0 +1,52 @@ +// ============================================================ +// MODEL VISION SUPPORT +// ============================================================ +// Can this model be shown a picture? +// +// Three answers, not two: true, false, and "the gateway does not say". Only an +// explicit false is actionable. Most of a roster carries no supports_vision at +// all, and treating silence as blindness would route perfectly good models +// through a delegation they do not need. +// +// Asked of the gateway, never inferred from the model id: "gemini-3.8-flash" +// says nothing about whether this deployment's copy of it accepts images, and a +// name-matching rule is the kind of thing that quietly rots. +// +// Cached briefly. This is asked on requests that carry an attachment, and a +// /model/info round trip per request would add latency to exactly the requests +// that are already the slowest. +var CACHE_MS = 5 * 60 * 1000; +var cache = { at: 0, byId: null }; + +async function roster() { + if (cache.byId && Date.now() - cache.at < CACHE_MS) return cache.byId; + var axios = require('axios'); + var { getLiteLLMAdminHeaders, liteLLMBaseUrl } = require('./litellm'); + var response = await axios.get(liteLLMBaseUrl() + '/model/info', + { headers: getLiteLLMAdminHeaders(), timeout: 10000 }); + var byId = Object.create(null); + ((response.data && response.data.data) || []).forEach(function (model) { + var id = model && model.model_name; + if (!id) return; + var info = model.model_info || {}; + byId[id] = typeof info.supports_vision === 'boolean' ? info.supports_vision : null; + }); + cache = { at: Date.now(), byId: byId }; + return byId; +} + +/** true, false, or null when the gateway does not say (or cannot be reached). */ +async function supportsVision(modelId) { + if (!modelId || !process.env.LITELLM_API_BASE) return null; + try { + var byId = await roster(); + return Object.prototype.hasOwnProperty.call(byId, modelId) ? byId[modelId] : null; + } catch (e) { + // The gateway being unreachable is not evidence about the model. + return null; + } +} + +function forget() { cache = { at: 0, byId: null }; } + +module.exports = { supportsVision, forget, CACHE_MS }; diff --git a/src/utils/visionTool.js b/src/utils/visionTool.js new file mode 100644 index 00000000..bdbef305 --- /dev/null +++ b/src/utils/visionTool.js @@ -0,0 +1,117 @@ +// ============================================================ +// VISION TOOL +// ============================================================ +// Lets a text-only model ask a model that can see. +// +// The same shape as the image tool, and for the same reason: the model is handed +// a capability and decides when to use it. Here the capability is eyesight. When +// someone attaches a photograph and the configured chat model cannot accept +// image input, the attachment used to be either refused by the provider or +// silently dropped — the person got an answer about a picture nobody had looked +// at, which is worse than a refusal. +// +// The attachment never reaches the chat model. It goes to the vision model, +// whose description comes back as a tool result, and the chat model answers from +// that. So the clinical reasoning stays with the model an administrator chose +// for it, and only the seeing is delegated. +var MAX_QUESTION = 500; +var MAX_DESCRIPTION_TOKENS = 900; + +var tools = [{ + type: 'function', + function: { + name: 'look_at_image', + description: 'Look at the image the user attached to this conversation. ' + + 'You cannot see images yourself; this asks a model that can and returns ' + + 'what it sees in words. Call it whenever the user\'s message refers to an ' + + 'attached image, and ask for exactly what you need to answer — a general ' + + 'description, or a specific question such as the reading on a device, the ' + + 'distribution of a rash, or the text on a label.', + parameters: { + type: 'object', + additionalProperties: false, + properties: { + question: { + type: 'string', + minLength: 1, + maxLength: MAX_QUESTION, + description: 'What you need to know about the image.' + } + }, + required: ['question'] + } + } +}]; + +function failure(statusCode, message) { + var error = new Error(message); + error.statusCode = statusCode; + return error; +} + +// What the seeing model is told. Deliberately narrow: it reports, it does not +// diagnose. The clinical judgement belongs to the chat model, which has the +// conversation, the retrieved sources and the system prompt; this one has a +// picture and no context, and an opinion from it would carry undeserved weight. +function describePrompt(question) { + return 'Describe what is visible in the attached image so that a clinician who ' + + 'cannot see it can reason about it. Report only what is observable — colours, ' + + 'distribution, morphology, text, readings, scale. Do not diagnose, do not ' + + 'speculate about causes, and say plainly when something is unclear or cut off ' + + 'rather than guessing.\n\nThe question to answer: ' + question; +} + +/** + * Run a look_at_image call and let the model finish its answer. + * Returns `ai` unchanged when the model did not call the tool. + */ +async function dispatch(ai, opts) { + opts = opts || {}; + if (!ai || !ai.toolCalls || !ai.toolCalls.length) return ai; + var call = ai.toolCalls.find(function (c) { + return c && c.function && c.function.name === 'look_at_image'; + }); + if (!call) return ai; + + if (!Array.isArray(opts.images) || !opts.images.length) { + throw failure(400, 'There is no image attached to this conversation.'); + } + if (!opts.visionModel) { + throw failure(503, 'No vision model is configured, so an attached image cannot be read.'); + } + + var question = ''; + try { + var args = JSON.parse(String(call.function.arguments || '{}')); + question = String(args.question || '').slice(0, MAX_QUESTION).trim(); + } catch (e) { + throw failure(400, 'The image question could not be read.'); + } + if (!question) question = 'Describe this image.'; + + var seen = await opts.callAI( + [{ role: 'user', content: describePrompt(question) }], + { model: opts.visionModel, temperature: 0.1, images: opts.images, maxTokens: MAX_DESCRIPTION_TOKENS } + ); + var description = String((seen && seen.content) || '').trim(); + if (!description) throw failure(502, 'The vision model returned no description of the image.'); + + // Handed back as a tool result so the chat model answers in its own voice, + // with its own sources and system prompt, from a description rather than a + // picture. toolChoice none: it has what it asked for and must now answer. + var completed = await opts.callAI( + (opts.messages || []).concat([ + { role: 'assistant', content: null, tool_calls: [call] }, + { role: 'tool', tool_call_id: call.id, content: JSON.stringify({ description: description }) } + ]), + Object.assign({}, opts.options || {}, { tools: tools, toolChoice: 'none' }) + ); + + return Object.assign({}, completed, { + visionHandled: true, + visionModel: opts.visionModel, + visionDescription: description + }); +} + +module.exports = { tools, dispatch, describePrompt, MAX_QUESTION }; diff --git a/test/assistant-attachment-roundtrip.test.js b/test/assistant-attachment-roundtrip.test.js index d0d39e0e..52453a80 100644 --- a/test/assistant-attachment-roundtrip.test.js +++ b/test/assistant-attachment-roundtrip.test.js @@ -48,6 +48,8 @@ function server(options = {}) { '../utils/redis': { async getJson() { return null; }, async setJson() {} }, '../utils/clinicalPromptPool': { createClinicalPromptPool: () => ({}) }, '../utils/generatedImages': { ...require('../src/utils/generatedImages'), service: () => ({ async get() { return { jobId: 'synthetic', status: 'done', success: true }; }, async enqueue() { return { success: true, jobId: 'synthetic', status: 'pending' }; } }) }, '../utils/generatedImageLinks': require('../src/utils/generatedImageLinks'), + '../utils/visionTool': require('../src/utils/visionTool'), + '../utils/modelVision': { supportsVision: async () => null }, '../utils/imageTool': require('../src/utils/imageTool'), '../utils/clinicalPrompts': require('../src/utils/clinicalPrompts'), '../utils/clinicalMcpClient': { diff --git a/test/assistant-image-attachments.test.js b/test/assistant-image-attachments.test.js index dd44031c..c279ca53 100644 --- a/test/assistant-image-attachments.test.js +++ b/test/assistant-image-attachments.test.js @@ -46,6 +46,8 @@ function server(options = {}) { '../utils/redis': { async getJson() { return null; }, async setJson() {} }, '../utils/clinicalPromptPool': { createClinicalPromptPool: () => ({}) }, '../utils/generatedImages': require('../src/utils/generatedImages'), '../utils/generatedImageLinks': require('../src/utils/generatedImageLinks'), + '../utils/visionTool': require('../src/utils/visionTool'), + '../utils/modelVision': { supportsVision: async () => null }, '../utils/imageTool': require('../src/utils/imageTool'), '../utils/clinicalPrompts': require('../src/utils/clinicalPrompts'), '../utils/clinicalMcpClient': { diff --git a/test/assistant-image-intent.test.js b/test/assistant-image-intent.test.js index 0101f1cc..5f5639c3 100644 --- a/test/assistant-image-intent.test.js +++ b/test/assistant-image-intent.test.js @@ -51,8 +51,14 @@ test('no hand-maintained acknowledgement or language list is left in the route', assert.doesNotMatch(src, /GREETING_RE/, 'greeting word list removed — the model decides'); assert.doesNotMatch(src, /IMAGE_NOUN_NON_LATIN|IMAGE_REPEAT_NON_LATIN/, 'no multilingual vocabulary lists'); assert.doesNotMatch(src, /suppressRepeatedImage/, 'no answer-inspection backstop'); - // The one surviving pattern is the compatibility path for models that write - // the prompt as text; it reads the USER's message only. - const fallback = src.slice(src.indexOf('async function dispatchImageRequestFallback')); - assert.match(fallback, /isExplicitImageRequest\(prepared\.message\)/, 'gated on the user, never on the answer'); + // And now no pattern at all. The last one was a compatibility path for models + // without tool calling: it decided in English only, could not see the + // conversation, and let "image summary" through while the model itself read + // that as an obvious image request. Whether a message deserves a picture is + // the model's decision, made from the tool description. + assert.doesNotMatch(src, /IMAGE_REQUEST_PATTERN/); + assert.doesNotMatch(src, /isExplicitImageRequest/); + assert.doesNotMatch(src, /dispatchImageRequestFallback/); + // The tool is still offered — removing the pattern must not remove the path. + assert.match(src, /tools: imageTool\.tools/); }); diff --git a/test/assistant-translate.test.js b/test/assistant-translate.test.js index 814741c1..cfceefc8 100644 --- a/test/assistant-translate.test.js +++ b/test/assistant-translate.test.js @@ -87,6 +87,8 @@ test('translate route is owner-bound, validated and cached; admin default provid '../utils/redis': { async getJson() { return null; }, async setJson() {} }, '../utils/clinicalPromptPool': { createClinicalPromptPool: () => ({}) }, '../utils/generatedImages': { ...require('../src/utils/generatedImages'), service: () => ({ async get() { return {}; }, async enqueue() { return {}; } }) }, '../utils/generatedImageLinks': require('../src/utils/generatedImageLinks'), + '../utils/visionTool': require('../src/utils/visionTool'), + '../utils/modelVision': { supportsVision: async () => null }, '../utils/imageTool': require('../src/utils/imageTool'), '../utils/clinicalPrompts': require('../src/utils/clinicalPrompts'), '../utils/clinicalMcpClient': { async semanticSearch() { return {}; }, async getMcpHealth() { return {}; } }, diff --git a/test/clinical-conversation.test.js b/test/clinical-conversation.test.js index af583e63..36a84c7d 100644 --- a/test/clinical-conversation.test.js +++ b/test/clinical-conversation.test.js @@ -46,6 +46,8 @@ function server(options = {}) { return { success: true, jobId: 'synthetic', status: 'pending' }; } }) }, '../utils/generatedImageLinks': require('../src/utils/generatedImageLinks'), + '../utils/visionTool': require('../src/utils/visionTool'), + '../utils/modelVision': { supportsVision: async () => null }, '../utils/imageTool': require('../src/utils/imageTool'), '../utils/clinicalPrompts': require('../src/utils/clinicalPrompts'), '../utils/clinicalMcpClient': { diff --git a/test/generated-image-tools.test.js b/test/generated-image-tools.test.js index b3f88ae6..77ea3422 100644 --- a/test/generated-image-tools.test.js +++ b/test/generated-image-tools.test.js @@ -90,6 +90,8 @@ function route(file, ai, jobs) { '../utils/patientTakehome':require('../src/utils/patientTakehome'), './auth':{__sendEmail:async()=>false}, '../utils/generatedImages':realImages, '../utils/generatedImageLinks':require('../src/utils/generatedImageLinks'), + '../utils/visionTool': require('../src/utils/visionTool'), + '../utils/modelVision': { supportsVision: async () => null }, '../utils/imageTool':{tools:imageTool.tools,dispatch:(value,options)=>imageTool.dispatch(value,{...options,images:{enqueue:async(...args)=>{jobs.push(args);return {jobId:id,status:'pending'};}}})}, '../utils/clinicalMcpClient':{semanticSearch:async()=>({})}, '../utils/clinicalRetrieval':{normalizeMcpSearchResponse:()=>[{number:3,title:'Synthetic source',page:17,excerpt:'Synthetic reference'}],dedupeSources:s=>s, diff --git a/test/generated-images.integration.js b/test/generated-images.integration.js index 2e890468..d5aac323 100644 --- a/test/generated-images.integration.js +++ b/test/generated-images.integration.js @@ -160,6 +160,8 @@ test('actual authenticated asset/settings and Learning content write routes enfo const clinicalRoutes = load('src/routes/clinicalAssistant.js', { express, axios: {}, crypto: require('crypto'), '../db/database': routeDb, '../middleware/auth': auth, '../utils/ai': {}, '../utils/generatedImages': { ...require('../src/utils/generatedImages'), service: () => jobs }, + '../utils/visionTool': require('../src/utils/visionTool'), + '../utils/modelVision': { supportsVision: async () => null }, '../utils/imageTool': require('../src/utils/imageTool'), '../utils/generatedImageLinks': links, '../utils/logger': { audit() {}, error() {} }, '../utils/crypto': require('../src/utils/crypto'), '../utils/redis': {}, '../utils/clinicalPromptPool': { createClinicalPromptPool: () => ({}) }, '../utils/clinicalMcpClient': {}, '../utils/clinicalRetrieval': {}, diff --git a/test/patient-takehome.test.js b/test/patient-takehome.test.js index 74512a74..f9e8e6df 100644 --- a/test/patient-takehome.test.js +++ b/test/patient-takehome.test.js @@ -35,6 +35,8 @@ function server(t, overrides = {}) { '../middleware/auth': { authMiddleware: (req, res, next) => next() }, '../utils/ai': { callAI: async (messages, options) => { aiCalls.push({ messages, options }); return { content: 'Take home [1] text.', model: 'synthetic-chat' }; }, callAIStream: async () => { throw new Error('unexpected'); }, activeProvider: 'synthetic', discoverModels: async () => [], vertexClient: null, litellmClient: null, applyImageAttachments: x => x }, '../utils/generatedImages': { workflows: ['clinical_assistant'], snapshot: async () => ({}), enqueue: async () => ({}), tick: async () => {}, ready: async () => {} }, + '../utils/visionTool': require('../src/utils/visionTool'), + '../utils/modelVision': { supportsVision: async () => null }, '../utils/imageTool': { tools: [], dispatch: async x => x }, '../utils/generatedImageLinks': { validateChat: () => null }, '../utils/logger': quiet, @@ -112,7 +114,14 @@ test('patient take-home email reports honestly when SMTP is not configured', asy assert.equal(res.body.code, 'SMTP_NOT_CONFIGURED'); }); -test('explicit image requests enqueue a job even when the model only writes text', async t => { +test('a text-only answer no longer conjures an image; the model must call the tool', async t => { + // The route used to run a regex over the user's message and enqueue a job from + // the answer text when the model had not called the tool. It decided in + // English only, could not see the conversation, and let "image summary" + // through while the model itself read that as an obvious image request — a + // second, worse decision-maker sitting behind the first. Removed: whether a + // message deserves a picture is the model's call, made from the tool + // description. const fs = require('node:fs'); const path = require('node:path'); const read = file => fs.readFileSync(path.join(__dirname, '..', file), 'utf8'); const vm = require('node:vm'); @@ -126,6 +135,8 @@ test('explicit image requests enqueue a job even when the model only writes text '../utils/ai': { callAI: async () => ({ content: 'Create a poster showing oxygen delivery for neonates.' }), callAIStream: async () => ({ content: 'Create a poster showing oxygen delivery for neonates.' }) }, '../utils/generatedImages': { service: () => ({ enqueue: async (owner, workflow, input, key, replay, context, model) => { jobs.push({ owner, workflow, input, model }); return { jobId: 'job-x', status: 'pending', imageUrl: null }; } }), imageContext: (r, h) => ({ request: r, history: h }), requestKey: b => 'k' + String(b).length }, '../utils/imageTool': { tools: [], dispatch: async ai => ai }, + '../utils/visionTool': require('../src/utils/visionTool'), + '../utils/modelVision': { supportsVision: async () => null }, '../utils/generatedImageLinks': { validateChat: () => null }, '../utils/logger': { error() {}, audit() {}, warn() {} }, '../utils/crypto': { randomUUID: () => 'u', encryptString: v => v, decryptString: v => v }, @@ -150,9 +161,11 @@ test('explicit image requests enqueue a job even when the model only writes text const layer = module.exports.stack.find(l => l.route && l.route.path === '/clinical-assistant/chat' && l.route.methods.post).route.stack.find(l => l.method === 'post').handle; const res = { statusCode: 200, json(b) { this.body = b; return this; }, status(c) { this.statusCode = c; return this; }, set() {}, setHeader() {}, flushHeaders() {} }; await layer({ body: { message: 'create an image of oxygen delivery' }, user: { id: 7 }, ip: 'x' }, res); - assert.equal(jobs.length, 1, 'a real image job was enqueued despite the text-only answer — route status ' + res.statusCode + ' body ' + JSON.stringify(res.body)); - assert.match(jobs[0].input.prompt, /poster/, 'the model answer text became the image prompt'); - assert.equal(jobs[0].owner, 7, 'owner-bound'); + + assert.equal(jobs.length, 0, 'no job without a tool call, however the message is worded'); + assert.equal(res.statusCode, 200, 'and the answer is still returned'); + const route = read('src/routes/clinicalAssistant.js'); + assert.doesNotMatch(route, /IMAGE_REQUEST_PATTERN|isExplicitImageRequest|dispatchImageRequestFallback/); }); test('patient take-home email sends plain text wrapped in a simple caregiver note', async t => { diff --git a/test/review-model-vision-guard.test.js b/test/review-model-vision-guard.test.js index 0a068838..d1a843bd 100644 --- a/test/review-model-vision-guard.test.js +++ b/test/review-model-vision-guard.test.js @@ -72,7 +72,9 @@ test('a model the gateway reports as text-only is refused for slide review', () assert.match(route, /key === 'my_resources\.review_model'/); assert.match(route, /if \(canSee === false\)/, 'strictly false — not falsy, which would also catch null'); - assert.match(route, /is a text-only model, so it cannot look at/); + assert.match(route, /is a text-only model, so it cannot be shown/); + assert.match(route, /key === 'clinical_assistant\.vision_model'/, + 'the assistant\'s vision model is guarded the same way'); }); test('an unknown model is allowed, because unknown is not proof of blindness', () => { @@ -87,6 +89,6 @@ test('an unknown model is allowed, because unknown is not proof of blindness', ( test('turning slide review off is never blocked by the check', () => { const route = read('src/routes/adminConfig.js'); - assert.match(route, /key === 'my_resources\.review_model' && String\(value\)\.trim\(\)/, + assert.match(route, /\&\& String\(value\)\.trim\(\)\) \{/, 'an empty value means off, and must skip the lookup entirely'); }); diff --git a/test/vision-tool.test.js b/test/vision-tool.test.js new file mode 100644 index 00000000..44ee0832 --- /dev/null +++ b/test/vision-tool.test.js @@ -0,0 +1,121 @@ +// A text-only chat model can be lent a pair of eyes: it calls look_at_image, a +// vision model describes the attachment, and the chat model answers from the +// description. The attachment never reaches the model that cannot read it. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const visionTool = require('../src/utils/visionTool'); + +const read = file => fs.readFileSync(path.join(__dirname, '..', file), 'utf8'); +const IMAGES = [{ mimeType: 'image/png', dataBase64: 'aW1n' }]; +const call = (question) => ({ + id: 'call_1', type: 'function', + function: { name: 'look_at_image', arguments: JSON.stringify({ question }) } +}); + +function recorder(replies) { + const calls = []; + const queue = replies.slice(); + return { + calls, + callAI: async (messages, options) => { + calls.push({ messages, options }); + return queue.length > 1 ? queue.shift() : queue[0]; + } + }; +} + +test('the tool describes itself as eyesight the model does not have', () => { + const fn = visionTool.tools[0].function; + assert.equal(fn.name, 'look_at_image'); + assert.match(fn.description, /You cannot see images yourself/); + assert.equal(fn.parameters.required[0], 'question'); +}); + +test('a call routes the image to the vision model, never to the chat model', async () => { + const r = recorder([{ content: 'A red maculopapular rash over the trunk.' }, { content: 'That pattern fits...' }]); + const out = await visionTool.dispatch({ content: '', toolCalls: [call('describe the rash')] }, { + images: IMAGES, visionModel: 'seeing-model', callAI: r.callAI, + messages: [{ role: 'user', content: 'what is this rash' }], options: { model: 'blind-model' } + }); + + assert.equal(r.calls.length, 2); + // First hop: the vision model, with the picture. + assert.equal(r.calls[0].options.model, 'seeing-model'); + assert.equal(r.calls[0].options.images, IMAGES); + assert.match(r.calls[0].messages[0].content, /describe the rash/); + // Second hop: the chat model, with words and no picture. + assert.equal(r.calls[1].options.model, 'blind-model'); + assert.equal(r.calls[1].options.images, undefined, 'the blind model never receives the image'); + assert.equal(r.calls[1].options.toolChoice, 'none', 'it has its answer; it must now use it'); + const toolMessage = r.calls[1].messages.find(m => m.role === 'tool'); + assert.match(toolMessage.content, /maculopapular/); + + assert.equal(out.visionHandled, true); + assert.equal(out.content, 'That pattern fits...'); +}); + +test('the seeing model is told to report, not to diagnose', () => { + // It has a picture and no conversation, no sources and no system prompt. An + // opinion from it would carry weight it has not earned. + const prompt = visionTool.describePrompt('what is this'); + assert.match(prompt, /Report only what is observable/); + assert.match(prompt, /Do not diagnose/); + assert.match(prompt, /say plainly when something is unclear/); +}); + +test('a reply with no tool call passes straight through', async () => { + const r = recorder([{ content: 'unused' }]); + const ai = { content: 'A direct answer.', toolCalls: [] }; + assert.equal(await visionTool.dispatch(ai, { images: IMAGES, visionModel: 'm', callAI: r.callAI }), ai); + assert.equal(r.calls.length, 0); +}); + +test('another tool’s call is left for that tool', async () => { + const r = recorder([{ content: 'unused' }]); + const ai = { content: '', toolCalls: [{ id: 'x', type: 'function', function: { name: 'generate_image', arguments: '{}' } }] }; + assert.equal(await visionTool.dispatch(ai, { images: IMAGES, visionModel: 'm', callAI: r.callAI }), ai); + assert.equal(r.calls.length, 0); +}); + +test('calling it with nothing attached, or no vision model, says so plainly', async () => { + const r = recorder([{ content: 'x' }]); + await assert.rejects( + () => visionTool.dispatch({ content: '', toolCalls: [call('what is this')] }, + { images: [], visionModel: 'm', callAI: r.callAI }), + err => /no image attached/i.test(err.message) && err.statusCode === 400); + + await assert.rejects( + () => visionTool.dispatch({ content: '', toolCalls: [call('what is this')] }, + { images: IMAGES, visionModel: '', callAI: r.callAI }), + err => /No vision model is configured/.test(err.message) && err.statusCode === 503); +}); + +test('a vision model that says nothing is an error, not an empty description', async () => { + const r = recorder([{ content: ' ' }]); + await assert.rejects( + () => visionTool.dispatch({ content: '', toolCalls: [call('describe')] }, + { images: IMAGES, visionModel: 'seeing', callAI: r.callAI, messages: [] }), + err => /no description/i.test(err.message)); +}); + +test('the route withholds the attachment only from a model the gateway calls blind', () => { + const route = read('src/routes/clinicalAssistant.js'); + assert.match(route, /function assistantToolset\(prepared\)/); + // Delegation is opt-in on an explicit false, never on an unknown. + assert.match(route, /supportsVision\([\s\S]{0,80}\) === false/); + assert.match(route, /tools: imageTool\.tools\.concat\(visionTool\.tools\), images: undefined/); + // Both call sites build the toolset, and both completion paths reuse the same + // decision — a finalize that re-attached the withheld image would undo it. + assert.equal((route.match(/var toolset = assistantToolset\(prepared\);/g) || []).length, 2); + assert.equal((route.match(/images: assistantToolset\(prepared\)\.images/g) || []).length, 2); +}); + +test('vision capability is asked of the gateway and cached, never guessed from the id', () => { + const util = read('src/utils/modelVision.js'); + assert.match(util, /supports_vision === 'boolean' \? info\.supports_vision : null/); + assert.match(util, /never inferred from the model id/); + assert.match(util, /CACHE_MS/); + assert.doesNotMatch(util, /\/gemini|\/gpt-4|test\(modelId\)/, 'no name matching'); +});