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
161 lines
8.3 KiB
JavaScript
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\)/);
|
|
});
|