feat: modifying a deck can see it
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 1m0s
Forgejo Docker Build / Root app tests (push) Successful in 47s
Forgejo Android APK / Build signed APK (push) Successful in 2m13s
Forgejo Docker Build / Build Docker image (push) Successful in 24s
Forgejo Docker Build / Deploy to the host (push) Failing after 0s

The model editing a deck could not see it, which made most of what people
actually ask for unanswerable: "that slide is too crowded", "the diagram is in
the wrong place", "this one looks empty" are facts about the rendered page, not
about the JSON.

When a vision model is configured, modifying now renders the current deck —
with its figures, unlike the review pass, which runs while they are still being
drawn — and hands the model one image per slide alongside the JSON. Same
pipeline as review, reused rather than reimplemented: pptx, Gotenberg, PDF,
pdftoppm, capped at MAX_SLIDES.

The vision model then does the editing, which is a second and separately
measured benefit. On a real 20-slide deck, ds-deepseek-v4-flash returned the
deck unchanged for "make it better" — the echo reported yesterday — while
openrouter-gemini-3.8-flash applied it. So the stronger model fixes the echo
even without sight. A model the author picks explicitly still wins over both.

The result is rendered and reviewed again. Generation-only was the old rule, on
the reasoning that refining is a text edit; it is not. The edit is made against
how the deck looked before it, so a slide that gains two bullets only overflows
once it is rendered again. The reviewer may reposition but is held to the same
words, so a verification pass cannot quietly undo what was just asked for.

Whether an instruction achieved anything is judged on the model's edit, before
the reviewer runs, or a reviewer nudging a slide into two columns would mask an
instruction that did nothing.

Sight is an upgrade, never a dependency: no vision model, Gotenberg down, a
render that fails — each falls through to editing blind, and a test covers each
of those paths. Verified against two mutations: keeping the text model when
images are attached, and dropping the verification pass, each fail exactly one
test.

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:55:16 +02:00
parent 973f9d554f
commit e244ee5240
7 changed files with 234 additions and 18 deletions

View file

@ -262,8 +262,29 @@ slide and a nine-item list that wants two columns are invisible to it. With a
reviewer configured, each generated deck is rendered to PDF through Gotenberg,
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.
One pass per change — on generation, and again on the result of a modification.
Modifying was excluded at first on the reasoning that refining is a text edit.
It is not: an edit is made against how the deck looked *before* it, so a slide
that gains two bullets only overflows once it is rendered again, which is
exactly what the reviewer exists to catch.
### Modifying can see the deck too
When a vision model is configured, modifying renders the current deck — with
its figures — and hands the model one image per slide alongside the JSON. Most
of what people ask for while modifying is about the rendered page: "that slide
is crowded", "the diagram is in the wrong place", "this one looks empty". None
of it is answerable from the JSON.
The vision model then does the editing, which is a second benefit measured
before this was built: on a real 20-slide deck, `ds-deepseek-v4-flash` returned
the deck unchanged for "make it better" and `openrouter-gemini-3.8-flash` did
not. A model the author picks explicitly still wins over both.
Sight is an upgrade, never a dependency. No vision model configured, Gotenberg
down, a render that fails — each falls through to editing the JSON blind, which
is what this did before it could see at all, and none of them may cost someone
their modification.
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`

View file

@ -378,7 +378,12 @@
say('The model returned it unchanged — nothing was modified. ' +
'Try naming the slide or section to change, and what to change about it.', 'bad');
} else {
say('Applied' + (g.used ? ', using ' + g.count + ' library excerpt' + (g.count === 1 ? '' : 's') : '') +
// Whether it could see the slides is worth saying: it is the
// difference between "slide 4 looks crowded" being actionable and
// being guesswork, and it explains why this took longer.
say('Applied' +
(data.saw ? ', after looking at all ' + data.saw + ' slides' : '') +
(g.used ? ', using ' + g.count + ' library excerpt' + (g.count === 1 ? '' : 's') : '') +
'. Download it to see the result.', 'good');
}
reportSearches(data.searches);

View file

@ -535,7 +535,7 @@ router.post('/my-resources/:id/refine', async function (req, res) {
if (!instructions) return res.status(400).json({ error: 'Say what to change' });
var existing = await db.get(
'SELECT id, kind, topic, markdown, deck FROM user_resources WHERE id = ? AND user_id = ?',
'SELECT id, kind, topic, markdown, deck, image_ids FROM user_resources WHERE id = ? AND user_id = ?',
[parseInt(req.params.id, 10), req.user.id]
);
if (!existing) return res.status(404).json({ error: 'Not found' });
@ -595,12 +595,31 @@ router.post('/my-resources/:id/refine', async function (req, res) {
'never a real patient. Returning the markdown is still required; a tool call is not a ' +
'substitute for it, and no image tag or URL goes into the markdown.';
// Let it look at the deck it is about to edit, when a vision model is
// configured. Measured before building this: the text model returned the
// deck unchanged for "make it better", and a stronger model fixed that
// without seeing anything — but sight is what makes the visual half of the
// instructions answerable at all, and that is most of what people ask for
// while modifying.
var visionModel = existingDeck
? String(await db.getSetting('my_resources.review_model', '') || '') : '';
var slideViews = visionModel
? await renderDeckForSight(existingDeck, existing.image_ids, req.user)
: [];
var sight = !slideViews.length ? '' :
'\n\nYou can see the deck as it renders now: ' + slideViews.length + ' image' +
(slideViews.length === 1 ? '' : 's') + ', one per slide, in order. The first image is ' +
'slide index 0 in the JSON below. Use them for anything the instruction says about how ' +
'a slide looks — crowded, empty, a figure in the wrong place, text running off the ' +
'bottom. The JSON is what you edit; the images only tell you what it currently ' +
'produces.';
var messages = [{ role: 'user', content: existingDeck
? 'Revise the following slide deck according to the instruction. Return ONLY the ' +
'complete revised JSON object, in exactly the same shape, no commentary and no code ' +
'fences. Keep every slide that the instruction does not ask you to change, including ' +
'its layout and its "image_job" values, and keep any References slide last.\n\n' +
'INSTRUCTION: ' + instructions + illustration + material +
'INSTRUCTION: ' + instructions + illustration + material + sight +
'\n\nThe layouts available are:\n' + deckSchema.instructions(
(existingDeck.slides || []).length, 0) +
'\n\nDECK JSON:\n' + JSON.stringify({ slides: existingDeck.slides })
@ -610,7 +629,18 @@ router.post('/my-resources/:id/refine', async function (req, res) {
'References section at the end.\n\nINSTRUCTION: ' + instructions + illustration + material +
'\n\nMARKDOWN:\n"""\n' + existing.markdown + '\n"""' }];
// The reply restates the whole resource, so it needs room for one.
var options = { model: await resolveModel(req.body.model), temperature: 0.2, maxTokens: 16000 };
//
// The vision model does the editing when there is one — both because it is
// the only one that can act on what it sees, and because it follows a vague
// instruction better: measured on a real 20-slide deck, the text model
// echoed "make it better" back unchanged and the vision model did not.
// An explicit choice by the author still wins over both.
var options = {
model: req.body.model ? await resolveModel(req.body.model)
: (visionModel && slideViews.length ? visionModel : await resolveModel('')),
temperature: 0.2, maxTokens: 16000
};
if (slideViews.length) options.images = slideViews;
// Deck mode declares its figures; only the markdown path needs the tool.
var tools = sources.wantsImages && !existingDeck ? resourceImages.tools : [];
var callOptions = tools.length ? Object.assign({}, options, { tools: tools }) : options;
@ -647,6 +677,31 @@ router.post('/my-resources/:id/refine', async function (req, res) {
}
}
// Look at the result. The edit was made against how the deck looked before
// it; a slide that gained two bullets now overflows, and only rendering it
// again shows that. Same reviewer as generation, which may reposition but
// is held to the same words — so a verification pass cannot quietly undo
// the change that was just asked for.
// Judged before the reviewer touches it. The question "did my instruction do
// anything" is about the model's edit; a reviewer that nudged a slide into
// two columns would otherwise mask an instruction that achieved nothing.
var echoedBack = revisedDeck
? JSON.stringify(existingDeck.slides) === JSON.stringify(revisedDeck.slides)
: false;
var verified = { reviewed: false, reason: 'not attempted' };
if (revisedDeck && visionModel) {
verified = await deckReview.review(revisedDeck, {
model: visionModel,
callAI: callAI,
extractJson: deckBuild.extractJson,
gotenberg: documentExport.GOTENBERG,
mime: documentExport.FORMATS.pptx.mime,
pptx: await documentExport.renderDeck(revisedDeck, [], figureIdList(existing.image_ids))
});
revisedDeck = verified.deck;
}
var revised = revisedDeck ? deckSchema.toMarkdown(revisedDeck)
: String((ai && ai.content) || '').trim();
if (!revised) return res.status(502).json({ error: 'The model returned nothing. Try again.' });
@ -668,8 +723,7 @@ router.post('/my-resources/:id/refine', async function (req, res) {
// while answering "Applied" left the one person who could do something about
// it — the person who asked for the change — reading a success message next
// to an identical file.
var unchanged = revisedDeck
? JSON.stringify(existingDeck.slides) === JSON.stringify(revisedDeck.slides)
var unchanged = revisedDeck ? echoedBack
: revised.trim() === String(existing.markdown || '').trim();
logRefine({
id: existing.id,
@ -685,6 +739,8 @@ router.post('/my-resources/:id/refine', async function (req, res) {
res.json({
success: true, resource: row, markdown: revised, unchanged: unchanged,
saw: slideViews.length,
review: { applied: verified.reviewed, reason: verified.reason },
grounding: { used: Boolean(sources.corpus.context), count: sources.corpus.sources.length,
reason: sources.corpus.reason || null },
searches: sources.searches,
@ -698,6 +754,43 @@ router.post('/my-resources/:id/refine', async function (req, res) {
}
});
// ── Showing the deck to the model that is about to change it ────────────────
// The model that writes a deck never sees it, and that is just as true when it
// is editing one. Most of what people ask for while modifying is about the
// rendered page — "that slide is too crowded", "the diagram is in the wrong
// place", "this one looks empty" — and none of it is answerable from the JSON.
//
// So the deck is rendered the same way a download is, turned into one image per
// slide, and handed to the model along with the JSON. Same pipeline as the
// review pass, reused rather than reimplemented: pptx -> Gotenberg -> PDF ->
// pdftoppm -> PNG, capped at deckReview.MAX_SLIDES.
//
// Never fatal. A gateway that is down, a render that fails, a deck too big —
// all of them fall through to editing the JSON blind, which is what this did
// before it could see at all.
async function renderDeckForSight(deck, figureIds, user) {
var fsp = require('fs/promises');
var os = require('os');
var pathMod = require('path');
var dir = null;
try {
dir = await fsp.mkdtemp(pathMod.join(os.tmpdir(), 'sight-'));
// With its figures, unlike the review pass: a review runs while they are
// still being drawn, but by the time someone is modifying they exist, and
// "move the picture" is unanswerable without seeing the picture.
var figures = await collectFigures(figureIds, user, dir);
var pptx = await documentExport.renderDeck(deck, figures, figureIdList(figureIds));
return await deckReview.slideImages(pptx, documentExport.GOTENBERG,
documentExport.FORMATS.pptx.mime);
} catch (e) {
logger.warn('[my-resources] could not render the deck to look at it; editing blind',
{ error: e.message });
return [];
} finally {
if (dir) await require('fs/promises').rm(dir, { recursive: true, force: true }).catch(function () {});
}
}
// What a modification actually did, in one line, so "it spat out the same thing"
// can be checked rather than guessed at. The unchanged case is the one worth
// watching: the response says success either way, and the download is identical.

View file

@ -266,4 +266,7 @@ async function review(deck, options) {
}
}
module.exports = { review, fingerprint, movedOnly, applyChanges, instructions, MAX_SLIDES, MAX_CHANGES, RENDER_DPI };
// slideImages is exported because modifying a deck wants the same picture of it
// that reviewing does — same render path, same DPI, same per-slide cap — and two
// copies of this would drift.
module.exports = { review, slideImages, fingerprint, movedOnly, applyChanges, instructions, MAX_SLIDES, MAX_CHANGES, RENDER_DPI };

View file

@ -137,14 +137,22 @@ test('nothing about the review can fail a generation', async () => {
assert.match(long.reason, /too long/);
});
test('the reviewer is admin-chosen, off by default, and one pass on generation only', () => {
test('the reviewer is admin-chosen, off by default, and runs once per change', () => {
const route = read('src/routes/myResources.js');
assert.match(route, /db\.getSetting\('my_resources\.review_model', ''\)/);
assert.match(route, /if \(reviewModel\) \{/, 'nothing happens without one');
// Generation only: refining a deck is a text edit, and re-reviewing costs as
// much as the first pass while fixing far less.
// Modifying reviews too, which generation-only used to forbid. The reasoning
// changed with the evidence: an edit is made against how the deck looked
// *before* it, so a slide that gains two bullets only overflows once it is
// rendered again — exactly the class of fault the reviewer exists for. The
// old rule assumed refining was a text edit; it is a layout edit as often as
// not.
const refine = route.slice(route.indexOf("router.post('/my-resources/:id/refine'"));
assert.doesNotMatch(refine, /deckReview/);
assert.match(refine, /deckReview\.review\(revisedDeck/);
assert.match(refine, /if \(revisedDeck && visionModel\)/, 'and only when one is configured');
// Still one pass. The verification runs on the result, never in a loop.
assert.equal((refine.match(/deckReview\.review\(/g) || []).length, 1);
// The key has to be writable, or saving it silently does nothing.
assert.match(read('src/routes/adminConfig.js'), /'clinical_assistant\.', 'my_resources\.'\]/);
// And the image can actually rasterise a deck.

View file

@ -27,6 +27,7 @@ function router(t, overrides = {}) {
const module = { exports: {} };
const aiCalls = [];
const replies = overrides.replies ? overrides.replies.slice() : null;
const reviewCalls = [];
const updates = [];
const row = Object.assign({
id: 5, user_id: 7, title: 'Croup', kind: 'presentation', topic: 'croup',
@ -49,7 +50,11 @@ function router(t, overrides = {}) {
},
all: async () => [],
run: async () => ({}),
getSetting: async key => (key === 'clinical_assistant.image_model' ? 'synthetic-image' : '')
getSetting: async key => {
if (key === 'clinical_assistant.image_model') return 'synthetic-image';
if (key === 'my_resources.review_model') return overrides.visionModel || '';
return '';
}
},
'../middleware/auth': { authMiddleware: (req, res, next) => next() },
'../utils/ai': {
@ -64,8 +69,12 @@ function router(t, overrides = {}) {
},
'../utils/deckBuild': deckBuild,
'../utils/deckSchema': require('../src/utils/deckSchema'),
'../utils/deckReview': { enabled: async () => false, review: async d => d },
'../utils/documentExport': { FORMATS: {}, isSupported: () => false, filename: () => 'x', mimeFor: () => '', render: async () => ({}), renderDeck: async () => ({}) },
'../utils/deckReview': {
slideImages: async () => (overrides.slideImages || []),
review: async (deck) => { reviewCalls.push(deck); return { deck: deck, reviewed: true, reason: 'ok' }; },
MAX_SLIDES: 20
},
'../utils/documentExport': { FORMATS: { pptx: { mime: 'application/vnd.openxmlformats-officedocument.presentationml.presentation' } }, GOTENBERG: 'http://gotenberg:3000', isSupported: () => false, filename: () => 'x', mimeFor: () => '', render: async () => ({}), renderDeck: async () => Buffer.from('pptx') },
'../utils/generatedImages': { service: () => ({ get: async () => null, asset: async () => null }), workflows: ['my_resources'] },
'../utils/learningRetrieval': { retrieve: async () => ({ context: '', sources: [], reason: null }) },
'../utils/logger': quiet,
@ -95,7 +104,7 @@ function router(t, overrides = {}) {
await handler({ body, params: { id: '5' }, query: {}, user: { id: 7 } }, res);
return res;
}
return { request, aiCalls, updates };
return { request, aiCalls, updates, reviewCalls };
}
test('modifying a deck asks for a deck, even when illustration is on', async () => {
@ -231,3 +240,80 @@ test('a deck that parses first time is never asked for twice', async () => {
assert.equal(r.aiCalls.length, 1, 'the retry costs a call and must only happen on failure');
assert.equal(res.body.deckFallback, null);
});
// ── Modifying with sight ───────────────────────────────────────────────────
const TWO_PNGS = [
{ mimeType: 'image/png', dataBase64: 'aW1hZ2Ux' },
{ mimeType: 'image/png', dataBase64: 'aW1hZ2Uy' }
];
const EDITED = JSON.stringify({
slides: [
{ type: 'title', title: 'Croup' },
{ type: 'bullets', title: 'Features', bullets: ['Barking cough', 'Stridor', 'Hoarse voice'] }
]
});
test('modifying a deck shows the model what the deck currently looks like', async () => {
// Most of what people ask for while modifying is about the rendered page —
// "that slide is crowded", "the diagram is in the wrong place" — and none of
// it is answerable from the JSON alone.
const r = router(null, { visionModel: 'seeing-model', slideImages: TWO_PNGS, replies: [EDITED] });
const res = await r.request('post', '/my-resources/:id/refine', {
instructions: 'slide 2 looks crowded, split it'
});
assert.equal(res.statusCode, 200);
assert.equal(r.aiCalls[0].options.images, TWO_PNGS, 'the rendered slides are attached');
assert.equal(r.aiCalls[0].options.model, 'seeing-model', 'and the model that can see does the editing');
assert.match(r.aiCalls[0].messages[0].content, /You can see the deck as it renders now: 2 images/);
assert.match(r.aiCalls[0].messages[0].content, /first image is slide index 0/,
'the images must be anchored to the JSON or a slide reference means nothing');
assert.equal(res.body.saw, 2);
});
test('the edited deck is rendered again and checked', async () => {
// The edit was made against how the deck looked *before* it. A slide that
// gained two bullets only overflows once it is rendered again.
const r = router(null, { visionModel: 'seeing-model', slideImages: TWO_PNGS, replies: [EDITED] });
const res = await r.request('post', '/my-resources/:id/refine', { instructions: 'add a feature' });
assert.equal(r.reviewCalls.length, 1, 'the result goes back past the reviewer');
assert.equal(res.body.review.applied, true);
});
test('with no vision model configured, modify still works and never renders', async () => {
// Sight is an upgrade, not a dependency. Nothing here may become a new way
// for a modification to fail.
const r = router(null, { replies: [EDITED] });
const res = await r.request('post', '/my-resources/:id/refine', { instructions: 'add a feature' });
assert.equal(res.statusCode, 200);
assert.equal(res.body.saw, 0);
assert.equal(r.aiCalls[0].options.images, undefined);
assert.equal(r.reviewCalls.length, 0, 'and nothing is rendered or reviewed');
});
test('a render that fails falls through to editing blind rather than failing', async () => {
// Gotenberg down, LibreOffice wedged, a deck too big: none of them may cost
// the author their modification.
const r = router(null, { visionModel: 'seeing-model', slideImages: [], replies: [EDITED] });
const res = await r.request('post', '/my-resources/:id/refine', { instructions: 'add a feature' });
assert.equal(res.statusCode, 200);
assert.equal(res.body.saw, 0);
assert.equal(r.aiCalls[0].options.images, undefined);
assert.notEqual(r.aiCalls[0].options.model, 'seeing-model',
'no images means no reason to pay for the vision model');
});
test('an echo is still reported as an echo, even when the reviewer moved something', async () => {
// The reviewer may reposition, which would otherwise make an instruction that
// achieved nothing look like it had worked.
const r = router(null, { visionModel: 'seeing-model', slideImages: TWO_PNGS,
replies: [JSON.stringify({ slides: DECK.slides })] });
const res = await r.request('post', '/my-resources/:id/refine', { instructions: 'make it better' });
assert.equal(res.body.unchanged, true, 'judged on the model edit, before the reviewer ran');
});

View file

@ -317,7 +317,7 @@ test('modifying a presentation edits the deck, not only its markdown', () => {
// and left the deck alone, so a modification reported success, updated the
// library row, and produced a byte-identical download. Measured: markdown
// gained the new slide, the deck did not, and the exported pptx did not.
assert.match(route, /SELECT id, kind, topic, markdown, deck FROM user_resources/);
assert.match(route, /SELECT id, kind, topic, markdown, deck, image_ids FROM user_resources/);
assert.match(route, /var revisedDeck = existingDeck \? deckBuild\.parse\(ai && ai\.content\) : null;/);
assert.match(route, /deck = COALESCE\(\?::jsonb, deck\)/, 'and the deck is written back');