feat: a text-only model cannot be set as the slide reviewer
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 49s
Forgejo Docker Build / Root app tests (push) Successful in 48s
Forgejo Android APK / Build signed APK (push) Successful in 2m6s
Forgejo Docker Build / Build Docker image (push) Successful in 14s
Forgejo Docker Build / Deploy to the host (push) Failing after 2s

The reviewer is shown rendered images of the deck, so a text-only model there
fails on every generation — at request time, long after the one moment an
administrator could have picked differently. Nothing checked it.

Saving my_resources.review_model now asks the gateway what it reports for that
model and refuses only an explicit supports_vision === false.

Three answers, not two. Most of this roster carries no supports_vision at all
(every openrouter-* entry here), and refusing unknowns would block the reviewer
this deployment already runs on. An unreachable gateway is not evidence about a
model either, so it never blocks the save. Empty means review is off and skips
the lookup entirely.

The capability is read from the gateway, never inferred from the model id.

Verified against a mutation: relaxing `canSee === false` to `canSee !== true`
fails the two tests that say unknown must stay allowed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
Daniel 2026-09-12 06:42:44 +02:00
parent 491a2b0811
commit 973f9d554f
3 changed files with 141 additions and 0 deletions

View file

@ -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

View file

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

View file

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