diff --git a/Dockerfile b/Dockerfile index b53f972f..9ebfdbbe 100644 --- a/Dockerfile +++ b/Dockerfile @@ -31,6 +31,11 @@ RUN apk add --no-cache ffmpeg curl jq pandoc-cli # py3-lxml and py3-pillow come from apk rather than pip because both are C # extensions and Alpine has no wheels for them; installing from source here # would mean carrying a compiler in the runtime image. Adds ~58MB. +# poppler-utils supplies pdftoppm, which turns a rendered deck into one image +# per slide. That is the only way to let a vision model see what a deck actually +# looks like — Gotenberg converts to PDF and stops there. +RUN apk add --no-cache poppler-utils + RUN apk add --no-cache python3 py3-pip py3-lxml py3-pillow \ && pip install --break-system-packages --no-cache-dir python-pptx==1.0.2 \ && python3 -c 'import pptx' diff --git a/docs/deployment.md b/docs/deployment.md index 8df531f1..70a8f701 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -16,6 +16,7 @@ on. They are in `Dockerfile` and worth knowing about before trimming it: | `pandoc-cli` | Word (`.docx`) export | | `python3`, `py3-lxml`, `py3-pillow` | the slide renderer. Both libraries are C extensions with no Alpine wheels, so they come from apk rather than pip — installing them from source would mean carrying a compiler in the runtime image | | `python-pptx==1.0.2` (pip) | builds the decks. Pinned: unpinned, a rebuild from the same commit could produce different slides | +| `poppler-utils` | `pdftoppm`, which turns a rendered deck into one image per slide so a vision model can see it. Only needed when slide review is switched on | | `ffmpeg`, `curl`, `jq` | audio handling and entrypoint scripting | Roughly 58MB of that is Python. PDF conversion is **not** in the image — it goes diff --git a/docs/my-resources.md b/docs/my-resources.md index 92dfe9ed..e0d51232 100644 --- a/docs/my-resources.md +++ b/docs/my-resources.md @@ -167,6 +167,52 @@ download that works without it. 1.0.2 from pip. Roughly 58MB. Unpinned, a rebuild from the same commit could produce different decks. +## Slide review + +Off unless an administrator names a model, in **Admin → Slide 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. 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. + +### It returns a patch, not a deck + +```json +{"changes":[ + {"slide":2,"action":"two"}, + {"slide":4,"action":"split","after":3,"heading":"Management (continued)"}, + {"slide":6,"action":"compare","at":3,"labels":["MILD","SEVERE"]} +]} +``` + +Asking for the corrected 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, at any output budget the provider would honour. + +The patch is better for a second reason. The reviewer names a slide and an +action; the server moves the text it already has. The words never pass through +the model at all, so a review cannot reword, drop or invent a single bullet — +which is a stronger guarantee than instructing it not to and checking +afterwards. The check still runs: body text must come out the same multiset, +figures the same set, and a heading may only be reused or extended. A +continuation heading is the reviewer's one piece of text, and it is replaced +with `" (continued)"` if it does not continue anything. + +Nothing here can fail a generation. No reviewer, an unreachable one, an +unparseable reply, a deck longer than `MAX_SLIDES`, or a patch that applies to +nothing — each returns the deck that was written. + +### Cost + +One image per slide on every presentation generated. Pick a cheap capable vision +model rather than the best one available; `openrouter-gemini-3.8-flash` is a +reasonable default. Measured on a three-slide deck: three images in, one change +out. + ## Modify `POST /api/my-resources/:id/refine` rewrites a resource in place, keeping its diff --git a/public/components/admin.html b/public/components/admin.html index fdc5c5e6..1c663b16 100644 --- a/public/components/admin.html +++ b/public/components/admin.html @@ -525,6 +525,38 @@ + +
+
+

Slide review

+ Off unless a model is chosen +
+
+

+ The model that writes a deck never sees it, so it cannot tell that a slide + overflowed or that a long list wants two columns. With a reviewer chosen, each + generated deck is rendered, looked at, and its layout corrected once. Wording and + figures are never changed — a review that alters them is discarded. +

+
+ +
+ +

+ Needs to be able to see images. One image per slide is sent on every + presentation generated, so this costs money each time — a cheap capable + vision model is the right choice here, not the best one available. +

+
+
+
+ + +
+
+
+
diff --git a/public/js/admin/clinicalAssistant.js b/public/js/admin/clinicalAssistant.js index e561c7ad..b7cc6e76 100644 --- a/public/js/admin/clinicalAssistant.js +++ b/public/js/admin/clinicalAssistant.js @@ -86,8 +86,57 @@ export function initClinicalAssistantAdmin(adminEscapeHtml) { if (e.target.closest('#btn-test-assistant-chat-model')) testAssistantChatModel(); if (e.target.closest('#btn-regenerate-assistant-prompt-pool')) regenerateAssistantPromptPool(); if (e.target.closest('#btn-restore-assistant-prompt-pool')) restoreAssistantPromptPool(); + if (e.target.closest('#btn-save-review-model')) saveReviewModel(); }); + // ── Slide review ──────────────────────────────────────────────────── + // Its own control rather than part of Save model & retrieval settings: it is + // the one setting that spends money on every generation without a user having + // asked for anything, so turning it on should be a deliberate act. + function saveReviewModel() { + var select = document.getElementById('mr-review-model'); + var status = document.getElementById('mr-review-status'); + if (!select) return; + if (status) { status.textContent = 'Saving...'; status.style.color = 'var(--g500)'; } + putAssistantConfig('my_resources.review_model', select.value || '') + .then(function() { + if (!status) return; + status.textContent = select.value ? 'Decks will be reviewed by ' + select.value : 'Slide review is off'; + status.style.color = 'var(--green)'; + }) + .catch(function(err) { + if (!status) return; + status.textContent = err.message; + status.style.color = 'var(--red)'; + }); + } + + // Any chat model the gateway offers. Whether it can actually see an image is + // not something the model list says, so the choice is the administrator's — + // a deck reviewed by a text-only model is discarded rather than applied. + function renderReviewModel(models, saved) { + var select = document.getElementById('mr-review-model'); + if (!select) return; + select.innerHTML = ''; + var off = document.createElement('option'); + off.value = ''; + off.textContent = 'Off — do not review decks'; + select.appendChild(off); + (Array.isArray(models) ? models : []).filter(function(m) { return m && m.id; }).forEach(function(m) { + var opt = document.createElement('option'); + opt.value = m.id; + opt.textContent = m.name || m.id; + select.appendChild(opt); + }); + if (saved && !Array.prototype.some.call(select.options, function(o) { return o.value === saved; })) { + var kept = document.createElement('option'); + kept.value = saved; + kept.textContent = saved + ' (saved/custom)'; + select.appendChild(kept); + } + select.value = saved || ''; + } + function updateAssistantLoadState() { var save = document.getElementById('btn-save-assistant-config'); if (save) save.disabled = configState !== 'ready'; @@ -155,6 +204,7 @@ export function initClinicalAssistantAdmin(adminEscapeHtml) { } chatSelect.value = savedChatModel; } + renderReviewModel(modelsData.models, cfg['my_resources.review_model'] || ''); window._assistantImageModelValue = cfg['clinical_assistant.image_model'] || ''; savedChatAllowed = parseAssistantList(cfg['clinical_assistant.allowed_models']); savedImageAllowed = parseAssistantList(cfg['clinical_assistant.allowed_image_models']); diff --git a/src/routes/adminConfig.js b/src/routes/adminConfig.js index 7845692b..80676251 100644 --- a/src/routes/adminConfig.js +++ b/src/routes/adminConfig.js @@ -932,7 +932,7 @@ router.put('/config/:key(*)', async function(req, res) { } // Security: only allow known key prefixes - var allowed = ['announcement.', 'feature.', 'email.', 'prompt.', 'registration_enabled', 'registration_invite_only', 'site.', 'smtp.', 'models.', 'tts.', 'stt.', 'embeddings.', 'clinical_assistant.']; + var allowed = ['announcement.', 'feature.', 'email.', 'prompt.', 'registration_enabled', 'registration_invite_only', 'site.', 'smtp.', 'models.', 'tts.', 'stt.', 'embeddings.', 'clinical_assistant.', 'my_resources.']; var isAllowed = allowed.some(function(p) { return key === p || key.startsWith(p); }); if (!isAllowed) { return res.status(400).json({ error: 'Unknown config key' }); diff --git a/src/routes/myResources.js b/src/routes/myResources.js index caeec528..d15d88d6 100644 --- a/src/routes/myResources.js +++ b/src/routes/myResources.js @@ -23,6 +23,7 @@ var learningRetrieval = require('../utils/learningRetrieval'); var resourceImages = require('../utils/resourceImages'); var deckSchema = require('../utils/deckSchema'); var deckBuild = require('../utils/deckBuild'); +var deckReview = require('../utils/deckReview'); var webSearch = require('../utils/webSearch'); var pubmedSearch = require('../utils/pubmedSearch'); var documentExport = require('../utils/documentExport'); @@ -339,6 +340,30 @@ router.post('/my-resources/generate', async function (req, res) { ai = Object.assign({}, ai, { imageJobs: drawn.jobs, imageFailures: drawn.failures }); } + // Look at what was built. The model that wrote the 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. One pass, on generation only, and only when + // an administrator has named a reviewer — a second pass costs as much as + // the first and fixes far less. + var reviewed = { reviewed: false, reason: 'not attempted' }; + if (deck) { + var reviewModel = String(await db.getSetting('my_resources.review_model', '') || ''); + if (reviewModel) { + var figureIds = (ai.imageJobs || []).map(function (job) { return job.jobId; }).filter(Boolean); + reviewed = await deckReview.review(deck, { + model: reviewModel, + callAI: callAI, + extractJson: deckBuild.extractJson, + gotenberg: documentExport.GOTENBERG, + mime: documentExport.FORMATS.pptx.mime, + // Rendered without the figures: they are still being drawn at this + // point, and a reviewer judges layout, not artwork. + pptx: await documentExport.renderDeck(deck, [], figureIds) + }); + deck = reviewed.deck; + } + } + // Markdown is still the readable artifact: it is what Word export renders // and what a text edit edits. In deck mode it is serialised from the deck // rather than written by the model. @@ -347,7 +372,7 @@ router.post('/my-resources/generate', async function (req, res) { // The figures belong to the resource, or an exported deck has no way to // include the pictures the author asked for. - var figureIds = (ai.imageJobs || []).map(function (job) { return job.jobId; }).filter(Boolean); + var savedFigureIds = (ai.imageJobs || []).map(function (job) { return job.jobId; }).filter(Boolean); var row = await db.get( 'INSERT INTO user_resources (user_id, title, kind, markdown, topic, grounded_count, image_ids, deck) ' + 'VALUES (?, ?, ?, ?, ?, ?, ?, ?) RETURNING id, title, kind, topic, grounded_count, created_at', @@ -364,6 +389,7 @@ router.post('/my-resources/generate', async function (req, res) { imageJobs: ai.imageJobs || [], imageFailures: ai.imageFailures || [], searches: searches, + review: { applied: reviewed.reviewed, reason: reviewed.reason }, model: ai && ai.model }); } catch (err) { diff --git a/src/utils/deckReview.js b/src/utils/deckReview.js new file mode 100644 index 00000000..b58697e8 --- /dev/null +++ b/src/utils/deckReview.js @@ -0,0 +1,269 @@ +// ============================================================ +// DECK REVIEW +// ============================================================ +// Render the deck, look at it, and fix what only looking can catch. +// +// The model that writes a deck never sees it. It cannot tell that slide four +// overflowed, that a figure landed on the wrong slide, or that a nine-item list +// would read better as two columns — those are facts about the rendered page, +// not about the text. So the deck is rendered to images and a vision model is +// asked to correct the layout. +// +// It may only move things. Same words, same figures: the reviewer returns a +// deck whose body text is the same multiset it was given, or its answer is +// discarded. That is checked rather than asked for, because 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. +// +// Off unless an administrator names a model. One pass, on generation only: +// a second pass costs as much as the first and fixes much less. + +var fsp = require('fs/promises'); +var os = require('os'); +var pathMod = require('path'); +var { execFile } = require('child_process'); + +var MAX_SLIDES = 20; // a review pass is one image per slide +var RENDER_DPI = 70; // legible to a model, small enough to send +var CONVERT_TIMEOUT = 60000; +// The reply restates the whole deck, so it needs room for one. +var MAX_REPLY_TOKENS = 2000; // a list of fixes, not a deck +var MAX_CHANGES = 12; + +function run(command, args, cwd) { + return new Promise(function (resolve, reject) { + execFile(command, args, { cwd: cwd, timeout: CONVERT_TIMEOUT, maxBuffer: 4 * 1024 * 1024 }, + function (err, stdout, stderr) { + if (err) return reject(new Error(command + ': ' + (stderr || err.message).toString().slice(0, 300))); + resolve(stdout); + }); + }); +} + +/** One PNG per slide, in order. */ +async function slideImages(pptx, gotenbergUrl, mime) { + var dir = await fsp.mkdtemp(pathMod.join(os.tmpdir(), 'review-')); + try { + var form = new FormData(); + form.append('files', new File([pptx], 'deck.pptx', { type: mime })); + var response = await fetch(gotenbergUrl + '/forms/libreoffice/convert', { + method: 'POST', body: form, signal: AbortSignal.timeout(90000) + }); + if (!response.ok) throw new Error('PDF conversion failed (' + response.status + ')'); + await fsp.writeFile(pathMod.join(dir, 'deck.pdf'), Buffer.from(await response.arrayBuffer())); + + await run('pdftoppm', ['-png', '-r', String(RENDER_DPI), 'deck.pdf', 'slide'], dir); + var files = (await fsp.readdir(dir)).filter(function (f) { return /^slide-?\d+\.png$/.test(f); }).sort(); + var images = []; + for (var i = 0; i < files.length && i < MAX_SLIDES; i++) { + images.push({ + mimeType: 'image/png', + dataBase64: (await fsp.readFile(pathMod.join(dir, files[i]))).toString('base64') + }); + } + return images; + } finally { + await fsp.rm(dir, { recursive: true, force: true }).catch(function () {}); + } +} + +// Everything the reviewer is forbidden to change, in a form two decks can be +// compared by. Headings are held separately because splitting an overfull slide +// legitimately repeats one. +function fingerprint(deck) { + var body = []; + var headings = []; + var figures = []; + (deck.slides || []).forEach(function (slide) { + if (slide.heading) headings.push(slide.heading.trim()); + if (slide.image_job) figures.push(slide.image_job); + // Every place a bullet can live: one list, two columns, or two labelled + // columns. Missing one of them makes a legitimate re-layout look like a + // rewrite — which it did, for the two-column case. + [slide.bullets, slide.left, slide.right].forEach(function (list) { + (list || []).forEach(function (b) { body.push(String(b.text || '').trim()); }); + }); + (slide.columns || []).forEach(function (c) { + (c.bullets || []).forEach(function (b) { body.push(String(b.text || '').trim()); }); + }); + (slide.header || []).forEach(function (cell) { if (cell) body.push(String(cell).trim()); }); + (slide.rows || []).forEach(function (row) { + row.forEach(function (cell) { if (cell) body.push(String(cell).trim()); }); + }); + if (slide.text) body.push(slide.text.trim()); + if (slide.caption) body.push(slide.caption.trim()); + }); + return { body: body.sort(), headings: headings, figures: figures.sort() }; +} + +function sameMultiset(a, b) { + return a.length === b.length && a.every(function (value, i) { return value === b[i]; }); +} + +/** + * True when the reviewed deck moved things without changing them. + * + * Body text must be the same multiset — moving a bullet to another slide or into + * a column keeps it, rewording it does not. Figures must be the same set, so a + * review can place them but cannot invent or drop one. A heading may be reused + * or extended, which is what splitting a slide needs, but not invented. + */ +function movedOnly(before, after) { + if (!sameMultiset(before.body, after.body)) return 'the wording changed'; + if (!sameMultiset(before.figures, after.figures)) return 'the figures changed'; + var known = before.headings; + var invented = after.headings.filter(function (heading) { + return !known.some(function (original) { + return heading === original || heading.indexOf(original) === 0; + }); + }); + if (invented.length) return 'a heading was invented: ' + invented[0].slice(0, 60); + return null; +} + +function instructions(slides) { + return [ + 'You are looking at the rendered slides of a teaching deck, in order. Slide 1', + 'in the images is index 0 below. The JSON that produced them follows.', + '', + 'Report layout problems only — things that are wrong about the rendered page,', + 'not about the writing:', + ' - text running off the bottom of a slide, or too small to read', + ' - a slide carrying so much that it should be two', + ' - a long single-column list that would read better in two columns', + ' - two labelled groups that would read better as a side-by-side comparison', + '', + 'Return ONLY a JSON object of changes, nothing else:', + '', + '{"changes":[', + ' {"slide":2,"action":"two"},', + ' {"slide":4,"action":"split","after":3,"heading":"Management (continued)"},', + ' {"slide":6,"action":"compare","at":3,"labels":["MILD","SEVERE"]},', + ' {"slide":1,"action":"one"}', + ']}', + '', + ' "two" — lay this slide\'s bullets out in two columns', + ' "one" — put a two-column slide back into one column', + ' "split" — make a second slide from the bullets after index "after"', + ' (0-based); "heading" must repeat or extend the original', + ' "compare" — two labelled columns, splitting the bullets at index "at";', + ' "labels" are two short column headings', + '', + 'You cannot change any wording, and you are not being asked to: name the slide', + 'and the action, and the server rearranges the text it already has. Slide', + 'indices are 0 to ' + (slides - 1) + '. Return {"changes":[]} if the deck reads well.' + ].join('\n'); +} + +// Apply a reviewer's changes to a copy of the deck. +// +// The reviewer names slides and split points; the text is moved by this code and +// never passes through the model, so a review cannot reword, drop or invent a +// single bullet. That is a stronger guarantee than asking it not to and checking +// afterwards — which is still done, because a bug here would be just as bad. +function applyChanges(deck, changes) { + var slides = (deck.slides || []).map(function (slide) { return JSON.parse(JSON.stringify(slide)); }); + var applied = 0; + + // Highest index first, so splitting one slide does not renumber the next. + var ordered = changes.slice().sort(function (a, b) { return (b.slide || 0) - (a.slide || 0); }); + + ordered.forEach(function (change) { + var index = parseInt(change.slide, 10); + if (!(index >= 0 && index < slides.length)) return; + var slide = slides[index]; + var action = String(change.action || ''); + + if (action === 'two' && slide.type === 'bullets' && (slide.bullets || []).length > 3) { + var half = Math.ceil(slide.bullets.length / 2); + slides[index] = { type: 'two', heading: slide.heading, notes: slide.notes, + left: slide.bullets.slice(0, half), right: slide.bullets.slice(half) }; + applied++; + return; + } + if (action === 'one' && slide.type === 'two') { + slides[index] = { type: 'bullets', heading: slide.heading, notes: slide.notes, + bullets: (slide.left || []).concat(slide.right || []) }; + applied++; + return; + } + if (action === 'compare' && slide.type === 'bullets') { + var at = parseInt(change.at, 10); + var labels = Array.isArray(change.labels) ? change.labels : []; + if (!(at > 0 && at < (slide.bullets || []).length) || labels.length !== 2) return; + slides[index] = { type: 'compare', heading: slide.heading, notes: slide.notes, columns: [ + { label: String(labels[0]).slice(0, 60), bullets: slide.bullets.slice(0, at) }, + { label: String(labels[1]).slice(0, 60), bullets: slide.bullets.slice(at) }] }; + applied++; + return; + } + if (action === 'split' && slide.type === 'bullets') { + var after = parseInt(change.after, 10); + if (!(after >= 0 && after < (slide.bullets || []).length - 1)) return; + // The continuation heading is the reviewer's only piece of text, and it + // has to repeat or extend the original or it is not a continuation. + var heading = String(change.heading || '').trim(); + var original = String(slide.heading || '').trim(); + if (!heading || heading.indexOf(original) !== 0) heading = original + ' (continued)'; + var tail = { type: 'bullets', heading: heading, bullets: slide.bullets.slice(after + 1) }; + slides[index] = Object.assign({}, slide, { bullets: slide.bullets.slice(0, after + 1) }); + slides.splice(index + 1, 0, tail); + applied++; + } + }); + + return { deck: Object.assign({}, deck, { slides: slides }), applied: applied }; +} + +/** + * Review a deck and return a corrected one, or the original. + * + * Never throws and never fails a generation: a deck that could not be reviewed + * is the deck that was written, which is what would have shipped anyway. + */ +async function review(deck, options) { + var reasons = []; + try { + if (!options.model) return { deck: deck, reviewed: false, reason: 'no reviewer configured' }; + var slides = (deck.slides || []).length; + if (!slides) return { deck: deck, reviewed: false, reason: 'nothing to review' }; + if (slides > MAX_SLIDES) return { deck: deck, reviewed: false, reason: 'deck too long to review' }; + + var images = await slideImages(options.pptx, options.gotenberg, options.mime); + if (!images.length) return { deck: deck, reviewed: false, reason: 'could not render the deck' }; + + var ai = await options.callAI( + [{ role: 'user', content: instructions(deck.slides.length) + + '\n\nDECK JSON:\n' + JSON.stringify({ slides: deck.slides }) }], + { model: options.model, temperature: 0.1, images: images, maxTokens: MAX_REPLY_TOKENS } + ); + + // A list of changes, not a deck. Asking for the whole deck back put the + // reply in proportion to the deck rather than to the number of problems, + // and a fourteen-slide deck came back cut off mid-object every time. + var reply = options.extractJson(String((ai && ai.content) || '')); + var changes = reply && Array.isArray(reply.changes) ? reply.changes : null; + if (!changes) return { deck: deck, reviewed: false, reason: 'the reviewer did not return changes' }; + if (!changes.length) return { deck: deck, reviewed: false, reason: 'nothing to fix' }; + + var result = applyChanges(deck, changes.slice(0, MAX_CHANGES)); + if (!result.applied) return { deck: deck, reviewed: false, reason: 'no change was applicable' }; + + // The text never went through the model, so this should always hold. It is + // checked anyway: a bug in applyChanges would be as bad as a model rewriting + // the words, and silently worse for being trusted. + var complaint = movedOnly(fingerprint(deck), fingerprint(result.deck)); + if (complaint) { + console.warn('[deck-review] discarded: ' + complaint); + return { deck: deck, reviewed: false, reason: complaint }; + } + return { deck: result.deck, reviewed: true, reason: null, + slides: images.length, changes: result.applied }; + } catch (err) { + console.warn('[deck-review] skipped:', err.message); + reasons.push(err.message); + return { deck: deck, reviewed: false, reason: reasons[0] }; + } +} + +module.exports = { review, fingerprint, movedOnly, applyChanges, instructions, MAX_SLIDES, MAX_CHANGES, RENDER_DPI }; diff --git a/src/utils/documentExport.js b/src/utils/documentExport.js index 285bb8ac..39603a1d 100644 --- a/src/utils/documentExport.js +++ b/src/utils/documentExport.js @@ -204,4 +204,16 @@ function filename(title, format) { return safe + '.' + (FORMATS[format] || FORMATS.pdf).ext; } -module.exports = { render, filename, mimeFor, isSupported, FORMATS }; +// The deck as bytes, without writing a download. The reviewer renders the deck +// it is about to judge, and that is the same pipeline an export runs. +async function renderDeck(deck, images, figureIds) { + var workdir = await fsp.mkdtemp(pathMod.join(os.tmpdir(), 'deck-')); + try { + await buildDeck('', workdir, images || [], { deck: deck, figureIds: figureIds || [] }); + return await fsp.readFile(pathMod.join(workdir, 'doc.pptx')); + } finally { + await fsp.rm(workdir, { recursive: true, force: true }).catch(function () {}); + } +} + +module.exports = { render, renderDeck, filename, mimeFor, isSupported, FORMATS, GOTENBERG }; diff --git a/test/deck-review.test.js b/test/deck-review.test.js new file mode 100644 index 00000000..fa656cd0 --- /dev/null +++ b/test/deck-review.test.js @@ -0,0 +1,153 @@ +// ============================================================ +// 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 one pass on generation only', () => { + 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. + const refine = route.slice(route.indexOf("router.post('/my-resources/:id/refine'")); + assert.doesNotMatch(refine, /deckReview/); + // 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\)/); +});