diff --git a/docs/my-resources.md b/docs/my-resources.md index 5525342f..20098be7 100644 --- a/docs/my-resources.md +++ b/docs/my-resources.md @@ -265,6 +265,15 @@ rasterised to one PNG per slide with `pdftoppm`, and shown to a vision model. One pass, on generation only. A second pass costs as much as the first and fixes far less, and refining is a text edit. +The reviewer must be able to see. Saving `my_resources.review_model` asks the +gateway what it reports for that model and refuses one whose `supports_vision` +is explicitly `false` — otherwise the mistake surfaces as a failed request on +every generation, long after the moment an administrator could have chosen +differently. A model the gateway says nothing about is allowed: most of a +roster carries no `supports_vision` at all, and silence is not proof of +blindness. An unreachable gateway is not evidence either, and never blocks the +save. + ### It returns a patch, not a deck ```json diff --git a/src/routes/adminConfig.js b/src/routes/adminConfig.js index 80676251..85a1aa2b 100644 --- a/src/routes/adminConfig.js +++ b/src/routes/adminConfig.js @@ -62,6 +62,30 @@ function liteLLMModelMode(model) { return model && model.model_info && model.model_info.mode ? String(model.model_info.mode) : ''; } +// 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 — a model +// the gateway has no metadata for is not thereby proven blind, and refusing +// those would block most of the roster over missing metadata rather than over +// a real incapability. +async function liteLLMVisionSupport(modelId) { + if (!modelId || !process.env.LITELLM_API_BASE) return null; + try { + var axios = require('axios'); + var resp = await axios.get(liteLLMBaseUrl() + '/model/info', + { headers: getLiteLLMAdminHeaders(), timeout: 10000 }); + var models = (resp.data && resp.data.data) || []; + for (var i = 0; i < models.length; i++) { + if (liteLLMModelId(models[i]) !== modelId) continue; + var info = models[i].model_info || {}; + return typeof info.supports_vision === 'boolean' ? info.supports_vision : null; + } + return null; + } catch (e) { + // The gateway being unreachable is not evidence about the model. + return null; + } +} + async function probeLiteLLMEmbeddingDimensions(modelId) { try { var axios = require('axios'); @@ -956,6 +980,22 @@ router.put('/config/:key(*)', async function(req, res) { return res.status(400).json({ error: 'Conversation budget must be an integer between 1000 and 1000000 UTF-16 code units, or empty to use CLINICAL_ASSISTANT_CONVERSATION_CHARS' }); } } + // The slide reviewer is shown rendered images of the deck. A text-only model + // cannot do that job: it would be sent pictures and fail at request time, on + // every generation, with the administrator having had no warning at the one + // 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()) { + 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.' + }); + } + } + if (key === 'clinical_assistant.preview_enabled' && !['true', 'false'].includes(String(value))) { return res.status(400).json({ error: 'Preview mode must be true or false' }); } diff --git a/test/review-model-vision-guard.test.js b/test/review-model-vision-guard.test.js new file mode 100644 index 00000000..0a068838 --- /dev/null +++ b/test/review-model-vision-guard.test.js @@ -0,0 +1,92 @@ +// The slide reviewer is shown rendered images. Setting a text-only model there +// produces a failure on every generation, at request time, long after the +// administrator could have chosen differently — so the choice is checked when +// it is made. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); + +const read = file => fs.readFileSync(path.join(__dirname, '..', file), 'utf8'); + +// Runs the real liteLLMVisionSupport out of the route file against a mocked +// gateway, so the parsing is exercised rather than read. +function visionSupport(modelInfo, opts = {}) { + const src = read('src/routes/adminConfig.js'); + const start = src.indexOf('async function liteLLMVisionSupport'); + const end = src.indexOf('\n}\n', start) + 3; + assert.ok(start > 0 && end > start, 'liteLLMVisionSupport not found'); + + const sandbox = { + liteLLMBaseUrl: () => 'https://gateway.invalid', + getLiteLLMAdminHeaders: () => ({}), + liteLLMModelId: m => (m && m.model_name) || '', + process: { env: opts.noGateway ? {} : { LITELLM_API_BASE: 'https://gateway.invalid' } }, + require: name => { + assert.equal(name, 'axios'); + return { get: async () => { + if (opts.unreachable) throw new Error('ECONNREFUSED'); + return { data: { data: modelInfo } }; + } }; + }, + module: { exports: {} } + }; + vm.runInNewContext(src.slice(start, end) + '\nmodule.exports = liteLLMVisionSupport;', sandbox); + return sandbox.module.exports; +} + +const ROSTER = [ + { model_name: 'text-only-model', model_info: { mode: 'chat', supports_vision: false } }, + { model_name: 'seeing-model', model_info: { mode: 'chat', supports_vision: true } }, + { model_name: 'unknown-model', model_info: { mode: 'chat' } } +]; + +test('the gateway is read for each of the three answers', async () => { + const ask = visionSupport(ROSTER); + assert.equal(await ask('text-only-model'), false, 'stated blind'); + assert.equal(await ask('seeing-model'), true, 'stated sighted'); + assert.equal(await ask('unknown-model'), null, 'the gateway does not say'); + assert.equal(await ask('not-on-the-roster'), null, 'a model it has never heard of'); +}); + +test('an unreachable gateway is not evidence that a model is blind', async () => { + // Returning false here would refuse a perfectly good model because the + // network blipped while an administrator was clicking Save. + assert.equal(await visionSupport(ROSTER, { unreachable: true })('seeing-model'), null); + assert.equal(await visionSupport(ROSTER, { noGateway: true })('text-only-model'), null); + assert.equal(await visionSupport(ROSTER)(''), null, 'and an empty model means review is off'); +}); + +test('the vision check is asked of the gateway, not guessed from the model name', () => { + const route = read('src/routes/adminConfig.js'); + assert.match(route, /async function liteLLMVisionSupport/); + assert.match(route, /info\.supports_vision === 'boolean' \? info\.supports_vision : null/, + 'three answers: true, false, and "the gateway does not say"'); + assert.doesNotMatch(route, /supports_vision.*test\(|\/vision\/\.test/, + 'never inferred from the model id'); +}); + +test('a model the gateway reports as text-only is refused for slide review', () => { + const route = read('src/routes/adminConfig.js'); + 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/); +}); + +test('an unknown model is allowed, because unknown is not proof of blindness', () => { + // Most of the roster reports no supports_vision at all. Refusing those would + // block the working configuration this deployment already runs on. + const route = read('src/routes/adminConfig.js'); + const guard = route.slice(route.indexOf("key === 'my_resources.review_model'")); + assert.doesNotMatch(guard.slice(0, 600), /canSee !== true/, + 'an unknown must not be treated as a refusal'); + assert.match(route, /The gateway being unreachable is not evidence about the model/); +}); + +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\(\)/, + 'an empty value means off, and must skip the lookup entirely'); +});