diff --git a/public/js/myResources.js b/public/js/myResources.js index cf1c71ca..67bca54f 100644 --- a/public/js/myResources.js +++ b/public/js/myResources.js @@ -317,7 +317,8 @@ var option = document.createElement('option'); option.value = String(row.id); option.textContent = (row.title || 'Untitled') + - ' \u2014 ' + (row.kind === 'article' ? 'article' : 'presentation'); + ' \u2014 ' + (row.kind === 'article' ? 'article' + : row.has_deck === false ? 'presentation, plain text' : 'presentation'); select.appendChild(option); }); if (previous && library.some(function (row) { return String(row.id) === previous; })) { @@ -361,12 +362,20 @@ // Same reporting as generating: what it was written from, what was // searched, and any figure that came back. var g = data.grounding || {}; - say('Applied' + (g.used ? ', using ' + g.count + ' library excerpt' + (g.count === 1 ? '' : 's') : '') + - '. Download it to see the result.', 'good'); + // The model can return the document back unchanged. That is a failed + // modification, and saying "Applied" for it sent people off to download + // an identical file and conclude the feature was broken. + if (data.unchanged) { + 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') : '') + + '. Download it to see the result.', 'good'); + } reportSearches(data.searches); showIllustrations(data.imageJobs || []); reportImageFailures(data.imageFailures); - if (box) box.value = ''; + if (box && !data.unchanged) box.value = ''; loadLibrary(); }) .catch(function (err) { say(err.message, 'bad'); }) @@ -390,11 +399,21 @@ title.style.cssText = 'font-weight:600;font-size:13px;'; title.textContent = row.title || 'Untitled'; + // The modified time, when there is one. Showing only the creation time meant + // a modification that did apply still looked like nothing had happened, so + // the timestamp was the thing people used to conclude modify was broken. + var created = new Date(row.created_at); + var updated = row.updated_at ? new Date(row.updated_at) : created; + var edited = updated - created > 2000; + var meta = document.createElement('div'); meta.style.cssText = 'font-size:11px;color:var(--g500);'; meta.textContent = (row.kind === 'article' ? 'Article' : 'Presentation') + - ' · ' + new Date(row.created_at).toLocaleString() + - (row.grounded_count ? ' · ' + row.grounded_count + ' library excerpts' : ' · not grounded'); + ' · ' + (edited ? 'modified ' + updated.toLocaleString() + : created.toLocaleString()) + + (row.grounded_count ? ' · ' + row.grounded_count + ' library excerpts' : ' · not grounded') + + // Only worth saying when it is the weaker kind. A deck is the normal case. + (row.kind !== 'article' && row.has_deck === false ? ' · plain text, no slide layout' : ''); body.appendChild(title); body.appendChild(meta); diff --git a/src/routes/myResources.js b/src/routes/myResources.js index dbefc3e5..ac4fe5c2 100644 --- a/src/routes/myResources.js +++ b/src/routes/myResources.js @@ -439,7 +439,12 @@ router.post('/my-resources/generate', async function (req, res) { router.get('/my-resources', async function (req, res) { try { var rows = await db.all( - 'SELECT id, title, kind, topic, grounded_count, created_at, updated_at ' + + // has_deck, not the deck itself: the list does not need the slides, but a + // presentation without one behaves differently enough — flat layout, a + // weaker modification path — that the owner should be able to see which + // kind they have. + 'SELECT id, title, kind, topic, grounded_count, created_at, updated_at, ' + + "(deck IS NOT NULL AND jsonb_array_length(COALESCE(deck->'slides', '[]'::jsonb)) > 0) AS has_deck " + 'FROM user_resources WHERE user_id = ? ORDER BY created_at DESC LIMIT ?', [req.user.id, MAX_PER_USER] ); @@ -520,6 +525,19 @@ router.post('/my-resources/:id/refine', async function (req, res) { 'a URL to fill the gap: leave the References section as it is.'; } + // A presentation with a stored deck is edited as a deck. Editing its + // markdown instead changed only the markdown: export renders from the deck, + // so a modification succeeded, said so, and produced an identical download. + // + // Declared here, above its first reader, and not further down where it used + // to sit. `var` hoisted it, so the illustration branch below read it as + // undefined and composed the markdown instruction for a deck: the model was + // told to return markdown and deck JSON in the same reply, the reply parsed + // as neither, and modifying a deck with illustration on failed outright. + var existingDeck = existing.deck + ? (typeof existing.deck === 'string' ? JSON.parse(existing.deck) : existing.deck) : null; + if (existingDeck && !(existingDeck.slides || []).length) existingDeck = null; + // How a figure is asked for depends on which thing is being edited. A deck // places figures by declaring them on a slide; markdown has nowhere to put // one, so it uses the tool. Offering the tool while editing a deck queued a @@ -536,13 +554,6 @@ 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.'; - // A presentation with a stored deck is edited as a deck. Editing its - // markdown instead changed only the markdown: export renders from the deck, - // so a modification succeeded, said so, and produced an identical download. - var existingDeck = existing.deck - ? (typeof existing.deck === 'string' ? JSON.parse(existing.deck) : existing.deck) : null; - if (existingDeck && !(existingDeck.slides || []).length) existingDeck = null; - 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 ' + @@ -612,22 +623,27 @@ router.post('/my-resources/:id/refine', async function (req, res) { // Said out loud, every time. A modification that changes nothing is the // failure worth catching, and it is invisible from the response: the row // updates, the title updates, and the file is identical. + // Computed once, because the caller is told this too. Logging it server-side + // 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) + : revised.trim() === String(existing.markdown || '').trim(); logRefine({ id: existing.id, path: revisedDeck ? 'deck' : 'markdown', outcome: 'applied', before: existingDeck ? (existingDeck.slides || []).length : existing.markdown.length, after: revisedDeck ? (revisedDeck.slides || []).length : revised.length, - unchanged: revisedDeck - ? JSON.stringify(existingDeck.slides) === JSON.stringify(revisedDeck.slides) - : revised.trim() === String(existing.markdown || '').trim(), + unchanged: unchanged, figures: added.length, model: ai && ai.model, instructions: instructions }); res.json({ - success: true, resource: row, markdown: revised, + success: true, resource: row, markdown: revised, unchanged: unchanged, grounding: { used: Boolean(sources.corpus.context), count: sources.corpus.sources.length, reason: sources.corpus.reason || null }, searches: sources.searches, diff --git a/test/my-resources-refine.test.js b/test/my-resources-refine.test.js new file mode 100644 index 00000000..06b6108b --- /dev/null +++ b/test/my-resources-refine.test.js @@ -0,0 +1,169 @@ +// Behavioural cover for modifying a resource. The rest of my-resources.test.js +// reads the source; these run the handler, because the three faults they pin +// were all invisible to source reading — the code said the right thing and did +// the wrong one. +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'); + +// Normalised, because that is the shape the route stores. Comparing a +// hand-written deck with a parsed reply would compare normalisation, not content. +const deckBuild = require('../src/utils/deckBuild'); +const DECK = Object.assign( + { title: 'Croup', subtitle: 'Teaching deck', date: '2026-01-01' }, + deckBuild.parse(JSON.stringify({ + slides: [ + { type: 'title', title: 'Croup' }, + { type: 'bullets', title: 'Features', bullets: ['Barking cough', 'Stridor'] } + ] + })) +); + +function router(t, overrides = {}) { + const module = { exports: {} }; + const aiCalls = []; + const updates = []; + const row = Object.assign({ + id: 5, user_id: 7, title: 'Croup', kind: 'presentation', topic: 'croup', + markdown: '# Croup\n\n- Barking cough\n- Stridor\n', deck: JSON.stringify(DECK) + }, overrides.row || {}); + const quiet = { log() {}, error() {}, warn() {}, info() {} }; + + const mocks = { + express: require('express'), + os: require('os'), + path: require('path'), + 'fs/promises': require('fs/promises'), + '../db/database': { + get: async (sql, params) => { + if (/^UPDATE user_resources/.test(sql)) { + updates.push({ sql, params }); + return { id: row.id, title: row.title, updated_at: '2026-01-02T00:00:00Z' }; + } + return row; + }, + all: async () => [], + run: async () => ({}), + getSetting: async key => (key === 'clinical_assistant.image_model' ? 'synthetic-image' : '') + }, + '../middleware/auth': { authMiddleware: (req, res, next) => next() }, + '../utils/ai': { + callAI: async (messages, options) => { + aiCalls.push({ messages, options }); + return { content: overrides.reply, model: 'synthetic' }; + }, + discoverModels: async () => [] + }, + '../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/generatedImages': { service: () => ({ get: async () => null, asset: async () => null }), workflows: ['my_resources'] }, + '../utils/learningRetrieval': { retrieve: async () => ({ context: '', sources: [], reason: null }) }, + '../utils/metrics': { resourceRefines: { inc() {} }, resourceVocabularyGaps: { inc() {} } }, + '../utils/pubmedSearch': { isAvailable: async () => false, search: async () => ({ results: [] }), formatForPrompt: () => '' }, + '../utils/webSearch': { isAvailable: async () => false, search: async () => ({ results: [] }), formatForPrompt: () => '' }, + '../utils/resourceImages': { tools: [{ name: 'draw' }], dispatch: async x => x, guidance: () => 'GUIDANCE.', requestedCount: () => 0, MAX_IMAGES: 6 } + }; + + vm.runInNewContext(read('src/routes/myResources.js'), { + module, exports: module.exports, console: quiet, Buffer, JSON, Date, Math, + process: { env: {} }, setTimeout() {}, + require(name) { assert.ok(Object.hasOwn(mocks, name), 'Unexpected import: ' + name); return mocks[name]; } + }); + + async function request(method, routePath, body) { + const layer = module.exports.stack.find(l => l.route && l.route.path === routePath && l.route.methods[method]); + assert.ok(layer, 'route missing: ' + method + ' ' + routePath); + const stack = layer.route.stack; + const handler = stack[stack.length - 1].handle; + const res = { + statusCode: 200, body: null, + status(code) { this.statusCode = code; return this; }, + json(payload) { this.body = payload; return this; }, + setHeader() {}, send() { return this; } + }; + await handler({ body, params: { id: '5' }, query: {}, user: { id: 7 } }, res); + return res; + } + return { request, aiCalls, updates }; +} + +test('modifying a deck asks for a deck, even when illustration is on', async () => { + // The fault: existingDeck was declared below the branch that read it, so var + // hoisting made it undefined there. Illustration on a deck therefore appended + // the markdown instruction — "Returning the markdown is still required" — + // to a prompt whose body asked for deck JSON. The model was told to produce + // two different artifacts at once and the reply parsed as neither, so every + // modification of a presentation with illustration ticked failed with 502. + const r = router(null, { + reply: JSON.stringify({ slides: [DECK.slides[0], { type: 'bullets', title: 'Features', bullets: ['Barking cough', 'Stridor', 'Hoarse voice'] }] }) + }); + const res = await r.request('post', '/my-resources/:id/refine', { + instructions: 'add a third feature', withImages: 'true' + }); + + assert.equal(res.statusCode, 200, 'a deck modification with images on must not 502'); + const prompt = r.aiCalls[0].messages[0].content; + assert.match(prompt, /Revise the following slide deck/); + assert.match(prompt, /Add "image_prompt" to the slides/, 'the deck illustration instruction'); + assert.doesNotMatch(prompt, /Returning the markdown is still required/, + 'the markdown instruction must never reach a deck prompt'); + assert.doesNotMatch(prompt, /An illustration tool is available/, + 'a deck declares its figures; it is not given the tool'); + assert.equal(r.aiCalls[0].options.tools, undefined, 'and no tool schema is attached'); +}); + +test('a modification the model returned unchanged says so instead of claiming success', async () => { + // Answering "Applied" for a reply identical to the original sent people off + // to download the same file and conclude the feature was broken. It was + // logged server-side, where the person who could reword the instruction + // could not see it. + const r = router(null, { reply: JSON.stringify({ slides: DECK.slides }) }); + const res = await r.request('post', '/my-resources/:id/refine', { instructions: 'make it better' }); + + assert.equal(res.statusCode, 200); + assert.equal(res.body.unchanged, true, 'the caller is told the deck came back identical'); +}); + +test('a modification that did change the deck reports itself as changed', async () => { + const r = router(null, { + reply: JSON.stringify({ slides: [DECK.slides[0], { type: 'bullets', title: 'Features', bullets: ['Barking cough', 'Stridor', 'Hoarse voice'] }] }) + }); + const res = await r.request('post', '/my-resources/:id/refine', { instructions: 'add a third feature' }); + + assert.equal(res.body.unchanged, false); + assert.equal(r.updates.length, 1, 'and the row is written'); + assert.match(r.updates[0].sql, /updated_at = NOW\(\)/); +}); + +test('a presentation with no deck still modifies, through the markdown path', async () => { + const r = router(null, { row: { deck: null }, reply: '# Croup\n\n- Barking cough\n- Stridor\n- Hoarse voice\n' }); + const res = await r.request('post', '/my-resources/:id/refine', { + instructions: 'add a third feature', withImages: 'true' + }); + + assert.equal(res.statusCode, 200); + assert.equal(res.body.unchanged, false); + const prompt = r.aiCalls[0].messages[0].content; + assert.match(prompt, /Revise the following Pandoc markdown/); + assert.match(prompt, /An illustration tool is available/, 'markdown has nowhere to declare a figure'); +}); + +test('the library says which presentations carry a deck, and the row shows when it was modified', () => { + // Both halves of "modify does nothing": the list read created_at, so a + // modification that did apply left the visible timestamp untouched, and + // nothing distinguished a real deck from a flat one. + const route = read('src/routes/myResources.js'); + assert.match(route, /AS has_deck/); + + const ui = read('public/js/myResources.js'); + assert.match(ui, /row\.updated_at \? new Date\(row\.updated_at\)/); + assert.match(ui, /edited \? 'modified '/); + assert.match(ui, /has_deck === false \? ' · plain text, no slide layout' : ''/); + assert.match(ui, /if \(data\.unchanged\) \{/); +});