pediatric-ai-scribe-v3/test/deck-review.test.js
Daniel e244ee5240
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
feat: modifying a deck can see it
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
2026-09-12 06:55:16 +02:00

161 lines
8.3 KiB
JavaScript

// ============================================================
// DECK REVIEW
// ============================================================
// The model that writes a deck never sees it, so overflow, a figure on the
// wrong slide and a nine-item list that wants two columns are invisible to it.
// The review renders the deck and looks.
//
// The thing that has to hold: it may move content, never change it.
const test = require('node:test');
const assert = require('node:assert');
const fs = require('fs');
const path = require('path');
const review = require('../src/utils/deckReview');
const read = p => fs.readFileSync(path.join(__dirname, '..', p), 'utf8');
const BEFORE = { slides: [
{ type: 'bullets', heading: 'Management', bullets: [{ text: 'One' }, { text: 'Two' }, { text: 'Three' }, { text: 'Four' }] },
{ type: 'figure', heading: 'Anatomy', bullets: [{ text: 'Five' }], image_job: 'job-a' },
]};
const verdict = after => review.movedOnly(review.fingerprint(BEFORE), review.fingerprint(after));
test('a review may move content between slides and layouts', () => {
assert.equal(verdict(BEFORE), null, 'unchanged');
// Splitting an overfull slide is the most valuable fix there is, and it
// necessarily repeats the heading.
assert.equal(verdict({ slides: [
{ type: 'bullets', heading: 'Management', bullets: [{ text: 'One' }, { text: 'Two' }] },
{ type: 'bullets', heading: 'Management (continued)', bullets: [{ text: 'Three' }, { text: 'Four' }] },
{ type: 'figure', heading: 'Anatomy', bullets: [{ text: 'Five' }], image_job: 'job-a' },
]}), null, 'split into two');
// Regrouping into labelled columns moves text between fields; the words stay.
// Column labels are structure, not content, so they may be new.
assert.equal(verdict({ slides: [
{ type: 'compare', heading: 'Management', columns: [
{ label: 'EARLY', bullets: [{ text: 'One' }, { text: 'Two' }] },
{ label: 'LATE', bullets: [{ text: 'Three' }, { text: 'Four' }] }] },
{ type: 'figure', heading: 'Anatomy', bullets: [{ text: 'Five' }], image_job: 'job-a' },
]}), null, 'bullets to a comparison');
// Reordering slides.
assert.equal(verdict({ slides: [BEFORE.slides[1], BEFORE.slides[0]] }), null, 'reordered');
});
test('a review that changes the words is discarded', () => {
// A model told not to rewrite will still occasionally improve a sentence, and
// a silent edit to clinical text is the one thing this must never introduce.
assert.match(verdict({ slides: [
{ type: 'bullets', heading: 'Management', bullets: [{ text: 'One thing' }, { text: 'Two' }, { text: 'Three' }, { text: 'Four' }] },
BEFORE.slides[1],
]}), /wording changed/);
// Dropping a bullet is a change too, not a layout decision.
assert.match(verdict({ slides: [
{ type: 'bullets', heading: 'Management', bullets: [{ text: 'One' }, { text: 'Two' }] },
BEFORE.slides[1],
]}), /wording changed/);
// Figures are drawn already; a review places them but cannot invent or lose one.
assert.match(verdict({ slides: [BEFORE.slides[0],
{ type: 'bullets', heading: 'Anatomy', bullets: [{ text: 'Five' }] }] }), /figures changed/);
// A heading may be reused or extended, never invented.
assert.match(verdict({ slides: [
{ type: 'bullets', heading: 'Key points', bullets: [{ text: 'One' }, { text: 'Two' }, { text: 'Three' }, { text: 'Four' }] },
BEFORE.slides[1],
]}), /heading was invented/);
});
test('the reviewer names slides, and the server moves the text', () => {
// Asking for the whole deck back put the reply in proportion to the deck
// rather than to the number of problems: a fourteen-slide deck came back cut
// off mid-object every time. A patch is small, and the words never pass
// through the model at all — which is a stronger guarantee than asking it not
// to rewrite them and checking afterwards.
const deck = { title: 'T', slides: [
{ type: 'bullets', heading: 'Management', bullets: [{ text: 'A' }, { text: 'B' }, { text: 'C' }, { text: 'D' }, { text: 'E' }, { text: 'F' }] },
{ type: 'figure', heading: 'Anatomy', bullets: [{ text: 'K' }], image_job: 'job-a' },
]};
const intact = out => review.movedOnly(review.fingerprint(deck), review.fingerprint(out.deck));
const two = review.applyChanges(deck, [{ slide: 0, action: 'two' }]);
assert.equal(two.applied, 1);
assert.equal(two.deck.slides[0].type, 'two');
assert.equal(intact(two), null);
const split = review.applyChanges(deck, [{ slide: 0, action: 'split', after: 2, heading: 'Management (continued)' }]);
assert.equal(split.deck.slides.length, 3);
assert.equal(intact(split), null);
const compare = review.applyChanges(deck, [{ slide: 0, action: 'compare', at: 3, labels: ['EARLY', 'LATE'] }]);
assert.equal(compare.deck.slides[0].columns.length, 2);
assert.equal(intact(compare), null);
// A continuation heading is the reviewer's only piece of text, so it is
// replaced rather than trusted when it does not continue anything.
const invented = review.applyChanges(deck, [{ slide: 0, action: 'split', after: 2, heading: 'Totally New' }]);
assert.equal(invented.deck.slides[1].heading, 'Management (continued)');
assert.equal(intact(invented), null);
// Nonsense is ignored rather than applied badly.
for (const bad of [[{ slide: 9, action: 'two' }],
[{ slide: 0, action: 'split', after: 5 }],
[{ slide: 0, action: 'compare', at: 3, labels: ['only'] }],
[{ slide: 0, action: 'nonsense' }]]) {
assert.equal(review.applyChanges(deck, bad).applied, 0, JSON.stringify(bad));
}
// Two splits at once must not renumber each other.
const both = review.applyChanges(deck, [{ slide: 0, action: 'split', after: 1 }, { slide: 1, action: 'two' }]);
assert.equal(intact(both), null);
});
test('nothing about the review can fail a generation', async () => {
const src = read('src/utils/deckReview.js');
// Off unless an administrator names a reviewer.
const off = await review.review(BEFORE, { model: '' });
assert.equal(off.reviewed, false);
assert.equal(off.deck, BEFORE, 'the deck comes back untouched');
// A reviewer that throws, returns nothing, or returns prose gives the deck back.
const broken = await review.review(BEFORE, {
model: 'some-model', gotenberg: 'http://127.0.0.1:1', mime: 'application/x',
pptx: Buffer.from(''), extractJson: () => null,
callAI: async () => { throw new Error('provider unreachable'); },
});
assert.equal(broken.reviewed, false);
assert.equal(broken.deck, BEFORE);
assert.match(src, /catch \(err\) \{\s*\n\s*console\.warn\('\[deck-review\] skipped:'/);
// A deck longer than the pass can look at is left alone rather than truncated.
const long = await review.review({ slides: new Array(review.MAX_SLIDES + 1).fill({ type: 'section', heading: 'x' }) },
{ model: 'some-model' });
assert.equal(long.reviewed, false);
assert.match(long.reason, /too long/);
});
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');
// 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.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.
assert.match(read('Dockerfile'), /poppler-utils/);
assert.match(read('src/utils/deckReview.js'), /'pdftoppm', \['-png', '-r', String\(RENDER_DPI\)/);
});