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'); +});