diff --git a/public/components/my-resources.html b/public/components/my-resources.html index 5ac903c3..911961d4 100644 --- a/public/components/my-resources.html +++ b/public/components/my-resources.html @@ -61,7 +61,7 @@ @@ -71,7 +71,7 @@
- +
@@ -123,7 +123,7 @@
diff --git a/public/js/myResources.js b/public/js/myResources.js index c1310e21..cf1c71ca 100644 --- a/public/js/myResources.js +++ b/public/js/myResources.js @@ -188,6 +188,7 @@ g.used ? 'good' : null); reportSearches(data.searches); showIllustrations(data.imageJobs || []); + reportImageFailures(data.imageFailures); loadLibrary(); }) .catch(function (err) { status(err.message, 'bad'); }) @@ -217,6 +218,15 @@ // the assistant: same status line, same durable job, same asset endpoint. It // lives in an ES module and this file is a classic script, hence the dynamic // import — which also means a failure to load it cannot break generation. + function reportImageFailures(failures) { + if (!(failures || []).length || typeof showToast !== 'function') return; + // Fewer pictures than asked for, without a word, would look like the model + // ignoring the request rather than the queue refusing it. + showToast(failures.length === 1 + ? 'One illustration could not be started: ' + failures[0] + : failures.length + ' illustrations could not be started.', 'error'); + } + function showIllustrations(jobs) { var box = document.getElementById('mr-images'); if (!box) return; @@ -355,6 +365,7 @@ '. Download it to see the result.', 'good'); reportSearches(data.searches); showIllustrations(data.imageJobs || []); + reportImageFailures(data.imageFailures); if (box) box.value = ''; loadLibrary(); }) diff --git a/src/routes/myResources.js b/src/routes/myResources.js index da929c11..5cba3487 100644 --- a/src/routes/myResources.js +++ b/src/routes/myResources.js @@ -20,8 +20,7 @@ var db = require('../db/database'); var { authMiddleware } = require('../middleware/auth'); var { callAI } = require('../utils/ai'); var learningRetrieval = require('../utils/learningRetrieval'); -var imageTool = require('../utils/imageTool'); -var generatedImages = require('../utils/generatedImages'); +var resourceImages = require('../utils/resourceImages'); var webSearch = require('../utils/webSearch'); var pubmedSearch = require('../utils/pubmedSearch'); var documentExport = require('../utils/documentExport'); @@ -110,21 +109,19 @@ function buildPrompt(opts) { // "Output ONLY Pandoc markdown" reads as a prohibition: the model returned // prose and never called the tool, exactly as the search tools failed. var illustration = opts.wantsImages - ? '\nAn illustration tool is available and the author has asked for one. If a diagram or ' + - 'picture would genuinely help this topic, call generate_image ONCE before writing, with a ' + - 'description of the single most useful figure. It must be schematic or anatomical teaching ' + - 'artwork, never a depiction of a real patient. The "output only Pandoc markdown" rule below ' + - 'is about the written resource; the tool call is not a violation of it. Do not write an ' + - 'image tag or a URL into the markdown \u2014 the image is attached separately.\n' + + ? '\nAn illustration tool is available and the author has asked for illustration. ' + + resourceImages.guidance(opts.refinement) + ' It must be schematic or anatomical teaching ' + + 'artwork, never a depiction of a real patient. The "output only Pandoc markdown" rule above ' + + 'is about the written resource; a tool call is not a violation of it. Do not write an ' + + 'image tag or a URL into the markdown \u2014 the images are attached separately.\n' + // Otherwise the decision is the model's alone, and an author who wants a // figure of something specific has no way to say so. Instructions are // free text, so this is what makes "illustrate the airway anatomy" or - // "definitely include a diagram" actually reach the illustration choice - // instead of only steering the prose. - 'If the author\'s additional instructions below ask for illustration, or name what the ' + - 'figure should show, follow them: treat that as the decision already made and compose the ' + - 'image description from what they asked for. Exactly one image is produced per ' + - 'generation, so if they ask for several, draw the single most useful one.\n' + // "use three diagrams" actually reach the illustration choice instead of + // only steering the prose. + 'If the author\'s additional instructions above name what a figure should show, follow ' + + 'them: treat that as the decision already made and compose the image description from ' + + 'what they asked for.\n' : ''; var shape = kind === 'presentation' @@ -140,9 +137,15 @@ function buildPrompt(opts) { 'Output ONLY Pandoc markdown: a level-1 heading for the title, then level-2 headings for ' + 'sections. Use prose, not slide bullets.\n'; + // The illustration paragraph goes last, after the output rules and the + // author's instructions. Placed before them it lost: measured with the tool + // offered, the model returned 3297 characters of markdown and zero tool + // calls, while the same tool and the same wording in a shorter prompt + // produced three calls. A long, emphatic "Output ONLY Pandoc markdown" block + // read afterwards is simply the more recent instruction. return 'You are writing teaching material for a medical professional audience ' + - '(pediatrics / primary care).\n\nTOPIC: ' + opts.topic + '\n' + grounding + findings + illustration + '\n' + shape + - (opts.refinement ? '\nAdditional instructions: ' + opts.refinement + '\n' : ''); + '(pediatrics / primary care).\n\nTOPIC: ' + opts.topic + '\n' + grounding + findings + '\n' + shape + + (opts.refinement ? '\nAdditional instructions: ' + opts.refinement + '\n' : '') + illustration; } function firstHeading(markdown, fallback) { @@ -269,17 +272,26 @@ router.post('/my-resources/generate', async function (req, res) { // decide and to compose a prompt; a search only needs the topic, and the // topic is already known. var tools = []; - if (wantsImages) tools = tools.concat(imageTool.tools); + if (wantsImages) tools = tools.concat(resourceImages.tools); - var ai = await callAI(messages, tools.length ? Object.assign({}, options, { tools: tools }) : options); + // When the author names a number — "use 3 diagrams" — the call is required + // rather than merely offered. Measured, deterministically: with the library + // switched off the model made three calls, and with thirty library excerpts + // in the prompt it made none and wrote a longer deck instead. The excerpts + // are not wrong to dominate; the request for figures simply has to survive + // them. With no number given the choice stays the model's. + var callOptions = tools.length ? Object.assign({}, options, { tools: tools }) : options; + if (tools.length && resourceImages.requestedCount(refinement)) callOptions.toolChoice = 'required'; - // The same dispatcher the assistant uses, so an image generated here is - // owned, queued and rendered exactly as one generated there. Without this - // the model's tool call is simply dropped and no job is ever enqueued. + var ai = await callAI(messages, callOptions); + + // My Resources' own dispatcher, not the assistant's: that one permits a + // single image per request, which is right for a chat reply and wrong for a + // deck. Same queue, same storage, same my_resources workflow — only the + // number of figures differs. if (wantsImages) { - ai = await imageTool.dispatch(ai, { - owner: req.user.id, workflow: 'my_resources', body: req.body, - imageContext: generatedImages.imageContext(topic, []), imageModel: imageModel, + ai = await resourceImages.dispatch(ai, { + owner: req.user.id, body: req.body, subject: topic, imageModel: imageModel, messages: messages, options: options, callAI: callAI }); } @@ -301,6 +313,7 @@ router.post('/my-resources/generate', async function (req, res) { markdown: markdown, grounding: { used: Boolean(corpus.context), count: corpus.sources.length, reason: corpus.reason || null }, imageJobs: ai.imageJobs || [], + imageFailures: ai.imageFailures || [], searches: searches, model: ai && ai.model }); @@ -396,11 +409,10 @@ router.post('/my-resources/:id/refine', async function (req, res) { } var illustration = sources.wantsImages - ? '\n\nAn illustration tool is available and the author has asked for one. If the ' + - 'instruction asks for a figure, or one would genuinely help, call generate_image ONCE ' + - 'with a description of it. Schematic or anatomical teaching artwork only, never a real ' + - 'patient. Returning the markdown is still required; the tool call is not a substitute ' + - 'for it, and no image tag or URL goes into the markdown.' + ? '\n\nAn illustration tool is available and the author has asked for illustration. ' + + resourceImages.guidance(instructions) + ' Schematic or anatomical teaching artwork only, ' + + '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.' : ''; // The markdown is the thing being edited, which is the whole reason it is @@ -412,13 +424,14 @@ 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"""' }]; var options = { model: await resolveModel(req.body.model), temperature: 0.2 }; - var tools = sources.wantsImages ? imageTool.tools : []; + var tools = sources.wantsImages ? resourceImages.tools : []; + var callOptions = tools.length ? Object.assign({}, options, { tools: tools }) : options; + if (tools.length && resourceImages.requestedCount(instructions)) callOptions.toolChoice = 'required'; - var ai = await callAI(messages, tools.length ? Object.assign({}, options, { tools: tools }) : options); + var ai = await callAI(messages, callOptions); if (sources.wantsImages) { - ai = await imageTool.dispatch(ai, { - owner: req.user.id, workflow: 'my_resources', body: req.body, - imageContext: generatedImages.imageContext(subject, []), imageModel: sources.imageModel, + ai = await resourceImages.dispatch(ai, { + owner: req.user.id, body: req.body, subject: subject, imageModel: sources.imageModel, messages: messages, options: options, callAI: callAI }); } @@ -437,6 +450,7 @@ router.post('/my-resources/:id/refine', async function (req, res) { reason: sources.corpus.reason || null }, searches: sources.searches, imageJobs: ai.imageJobs || [], + imageFailures: ai.imageFailures || [], model: ai && ai.model }); } catch (err) { diff --git a/src/utils/resourceImages.js b/src/utils/resourceImages.js new file mode 100644 index 00000000..29794fd3 --- /dev/null +++ b/src/utils/resourceImages.js @@ -0,0 +1,159 @@ +// ============================================================ +// RESOURCE ILLUSTRATIONS +// Multiple figures for one generated resource. +// ============================================================ +// Deliberately not imageTool.dispatch. That one is shared with the clinical +// assistant and the Learning Hub and permits exactly one image per request — +// "Only one image tool invocation is permitted per request" — which is the +// right rule for a chat reply and the wrong one for a twelve-slide deck where +// an author has asked for three figures. +// +// Rather than relax a limit three features depend on, this is a separate path +// with its own bound. It reuses the same queue, the same storage, the same +// my_resources workflow and the same asset endpoint, so a figure made here is +// owned, served and deleted exactly like one made anywhere else. Only the +// number of them differs. + +var images = require('./generatedImages'); + +// Enough for a deck with a figure every few slides, few enough that a model +// which has decided everything needs a picture cannot run up a bill. Each image +// is a separate paid request, so this bound is a cost control as much as a +// design one. +var MAX_IMAGES = 6; + +var tools = [{ + type: 'function', + function: { + name: 'generate_image', + description: 'Generate one teaching illustration for this resource — a diagram, ' + + 'a labelled anatomical figure, a flowchart, a comparison table drawn as a graphic. ' + + 'Call it once per figure you want, up to ' + MAX_IMAGES + ' in total: if the author asks ' + + 'for three images, make three separate calls in the same reply. Each prompt must stand ' + + 'on its own and describe a different figure. Schematic or anatomical teaching artwork ' + + 'only, never a depiction of a real patient, and never anything identifying. ' + + 'Write the resource itself as well; the images are attached separately, so put no image ' + + 'tag and no URL in the markdown.', + parameters: { + type: 'object', additionalProperties: false, + properties: { + prompt: { type: 'string', minLength: 1, maxLength: 32000 }, + layout: { type: 'string', enum: ['auto', 'portrait', 'landscape', 'square'] } + }, + required: ['prompt'] + } + } +}]; + +// How many figures the author asked for, if they said. "three images", "3 +// diagrams", "a couple of figures" — the number is a ceiling the model is told +// about, not a quota it must fill. +var WORD_NUMBERS = { one: 1, two: 2, three: 3, four: 4, five: 5, six: 6, a: 1, an: 1, couple: 2, few: 3 }; +var COUNT = /\b(\d+|one|two|three|four|five|six|a|an|couple|few)\s+(?:of\s+)?(?:more\s+|extra\s+|additional\s+)?(?:of\s+)?(images?|diagrams?|figures?|illustrations?|pictures?|charts?|graphics?|visuals?|schematics?)\b/i; + +function requestedCount(text) { + var match = COUNT.exec(String(text || '')); + if (!match) return null; + var token = match[1].toLowerCase(); + var n = /^\d+$/.test(token) ? parseInt(token, 10) : WORD_NUMBERS[token]; + if (!n || n < 1) return null; + return Math.min(n, MAX_IMAGES); +} + +// A sentence for the prompt, so the model knows both the ceiling and that it is +// free to use fewer if fewer would help. +function guidance(instructions) { + var asked = requestedCount(instructions); + if (asked) { + return 'The author asked for ' + asked + ' illustration' + (asked === 1 ? '' : 's') + + '. Call generate_image ' + asked + ' time' + (asked === 1 ? '' : 's') + + ', once per figure, each describing a different one.'; + } + return 'Add an illustration where one genuinely helps — a diagram, a labelled figure, a ' + + 'flowchart. Call generate_image once per figure, at most ' + MAX_IMAGES + ' in total, and ' + + 'only where a picture earns its place.'; +} + +// A resource starts with a Pandoc title block or a heading. Anything shorter +// than a couple of sentences with neither is the model talking, not writing. +function looksLikeResource(content) { + var text = String(content || '').trim(); + if (!text) return false; + return /^%/m.test(text) || /^#{1,2}\s/m.test(text); +} + +function parseCall(call) { + if (!call || call.type !== 'function' || !call.function || call.function.name !== 'generate_image') return null; + if (typeof call.id !== 'string' || call.id.length > 200) return null; + if (typeof call.function.arguments !== 'string' || call.function.arguments.length > 40000) return null; + var input; + try { input = JSON.parse(call.function.arguments); } catch (e) { return null; } + try { return images.args(input); } catch (e) { return null; } +} + +// Every call is queued, then the model is asked to continue once so the resource +// itself still comes back. Nothing here may fail the generation: a figure that +// cannot be queued is reported and the text is kept, because a deck with no +// picture beats no deck. +async function dispatch(ai, opts) { + var calls = (ai && ai.toolCalls) || []; + if (!calls.length) return ai; + + var jobs = []; + var failures = []; + var queue = opts.images || images.service(); + var context = images.imageContext(opts.subject, []); + + for (var i = 0; i < calls.length && jobs.length < MAX_IMAGES; i++) { + var input = parseCall(calls[i]); + if (!input) { failures.push('an illustration request could not be read'); continue; } + try { + // A key per figure, so two figures in one reply are two jobs rather than + // the second being returned as a replay of the first. + var key = ('res:' + images.requestKey(opts.body) + ':' + i).slice(0, 160); + jobs.push(await queue.enqueue(opts.owner, 'my_resources', input, key, true, context, opts.imageModel)); + } catch (err) { + failures.push(err && err.message ? err.message : 'an illustration could not be queued'); + } + } + + // A model that has just made three tool calls often signs off with a sentence + // instead of the resource — measured: "I'll create the presentation and the + // three teaching diagrams." and nothing else, 61 characters, which was then + // saved as the resource. Treating only a completely empty body as missing was + // the mistake; a body with no heading is not a Pandoc document whatever its + // length. + var completed = ai; + if (!looksLikeResource(ai && ai.content)) { + // The model spent its turn on tool calls. Ask once for the resource, with + // the tool withdrawn so it cannot spend a second turn the same way. + var transcript = [{ role: 'assistant', content: null, tool_calls: calls }]; + calls.forEach(function (call, index) { + transcript.push({ + role: 'tool', tool_call_id: call.id, + content: JSON.stringify({ + jobId: jobs[index] ? jobs[index].jobId : null, + status: jobs[index] ? jobs[index].status : 'not queued', + instruction: 'Illustration queued. Now write the resource itself in the requested ' + + 'format. Do not claim the image is finished and do not insert image URLs.' + }) + }); + }); + completed = await opts.callAI(opts.messages.concat(transcript), + Object.assign({}, opts.options, { tools: tools, toolChoice: 'none' })); + // If the continuation is no better, keep whichever of the two actually + // reads like a resource rather than silently saving "I'll create the + // presentation." + if (!looksLikeResource(completed && completed.content) && looksLikeResource(ai && ai.content)) { + completed = ai; + } + } + + return Object.assign({}, completed, { + imageJobs: jobs, + imageFailures: failures, + imageToolHandled: true + }); +} + +module.exports = { tools, dispatch, guidance, requestedCount, MAX_IMAGES }; diff --git a/test/my-resources.test.js b/test/my-resources.test.js index c2471352..a59b7d85 100644 --- a/test/my-resources.test.js +++ b/test/my-resources.test.js @@ -128,7 +128,7 @@ test('users pick from the models an admin already approved, and nothing else', ( assert.match(js, /if \(modelRow\) modelRow\.hidden = models\.length < 2;/); }); -test('illustration is opt-in, and reuses the assistant’s image tool', () => { +test('illustration is opt-in, with its own dispatcher rather than the assistant’s', () => { const route = read('src/routes/myResources.js'); // A model handed a drawing tool will find a reason to use it, so the tool is // only offered when the author asked for one. @@ -142,26 +142,29 @@ test('illustration is opt-in, and reuses the assistant’s image tool', () => { // model to decide there should be a picture and to compose the prompt for it. // Search does not — see web-search.test.js for why both searches were taken // away from the model and run by the route instead. - assert.match(route, /if \(wantsImages\) tools = tools\.concat\(imageTool\.tools\);/); + assert.match(route, /if \(wantsImages\) tools = tools\.concat\(resourceImages\.tools\);/); assert.doesNotMatch(route, /tools\.concat\((?:webSearch|pubmedSearch)\.tools\)/); - // The same dispatcher the assistant uses, so an image made here is owned, - // queued and rendered identically to one made there. - assert.match(route, /imageTool\.dispatch\(ai, \{/); - assert.match(route, /workflow: 'my_resources'/, 'but attributed to this feature'); + // Its own dispatcher. The assistant's permits one image per request, which is + // right for a chat reply and wrong for a deck, and three features depend on + // that rule — so this is a separate path rather than a relaxed shared one. + assert.match(route, /resourceImages\.dispatch\(ai, \{/); + assert.doesNotMatch(route, /imageTool/, 'the shared single-image dispatcher is not used here'); + assert.match(read('src/utils/imageTool.js'), /Only one image tool invocation is permitted per request/, + 'and its limit is left exactly as it was'); + assert.match(read('src/utils/resourceImages.js'), /'my_resources'/, 'but attributed to this feature'); // The dispatch call itself. It was lost once in a refactor: the tool was // still offered, the model still called it, and the call was silently // dropped, so no job was ever enqueued and imageJobs was always empty. - assert.match(route, /ai = await imageTool\.dispatch\(ai, \{/); + assert.match(route, /ai = await resourceImages\.dispatch\(ai, \{/); // dispatch expects { request, history }; a bare topic string made the bound // request undefined and lost the topic entirely. - assert.match(route, /imageContext: generatedImages\.imageContext\(topic, \[\]\)/); + assert.match(read('src/utils/resourceImages.js'), /images\.imageContext\(opts\.subject, \[\]\)/); // A model handed a tool schema and then told to "Output ONLY Pandoc markdown" // obeys the sentence, not the schema — measured: zero tool calls until the // prompt said the tool existed and that calling it was not a violation. - assert.match(route, /call generate_image ONCE before writing/); - assert.match(route, /is about the written resource; the tool call is not a violation of it/); + assert.match(route, /is about the written resource; a tool call is not a violation of it/); assert.match(route, /wantsImages: Boolean\(wantsImages && imageModel\)/); // Its own workflow, not a reuse of learning_hub: generated_image_links only @@ -188,20 +191,33 @@ test('the author can ask for the illustration, not only leave it to the model', // Without this the decision is the model's alone, and someone who wants a // figure of something particular has no way to say so — the instructions // steer the prose and nothing else. - assert.match(route, /If the author\\'s additional instructions below ask for illustration/); - assert.match(route, /image description from what they asked for/); - // And it says what it cannot do, rather than quietly drawing one of three. - assert.match(route, /Exactly one image is produced per/); - // The instructions really do come after this paragraph in the prompt, so - // "below" is accurate rather than a guess. - const prompt = route.slice(route.indexOf('function buildPrompt')); - assert.ok(prompt.indexOf('illustration +') < prompt.indexOf('Additional instructions'), - 'illustration guidance precedes the instructions it refers to'); + assert.match(route, /If the author\\'s additional instructions above name what a figure should show/); + assert.match(route, /compose the image description from/); + + // How many, when the author says. "use 3 images" is as clear an instruction + // as any other and used to be capped at one figure regardless. + const lib = read('src/utils/resourceImages.js'); + assert.match(lib, /function requestedCount\(text\)/); + assert.match(lib, /var MAX_IMAGES = 6;/, 'bounded, because each figure is a paid request'); + assert.match(lib, /jobs\.length < MAX_IMAGES/); + assert.match(lib, /'res:' \+ images\.requestKey\(opts\.body\) \+ ':' \+ i/, + 'a key per figure, or the second is returned as a replay of the first'); + assert.match(route, /resourceImages\.guidance\(opts\.refinement\)/, 'generate'); + assert.match(route, /resourceImages\.guidance\(instructions\)/, 'and modify'); + // A figure that cannot be queued is said out loud; fewer pictures than asked + // for with no explanation reads as the model ignoring the request. + assert.match(lib, /failures\.push/); + assert.match(read('public/js/myResources.js'), /function reportImageFailures/); + // The paragraph now comes last, so it says "above" — checked, because a + // prompt that points the model at the wrong end of itself is worse than one + // that says nothing. + assert.match(route, /additional instructions above name what a figure should show/); + assert.doesNotMatch(route, /instructions below/); // Said once on screen too: the label points at Instructions, and the // Instructions placeholder shows what asking for one looks like. const html = read('public/components/my-resources.html'); - assert.match(html, /include a diagram of the airway/); - assert.match(html, /Add one illustration/, 'and the count is stated where the choice is made'); + assert.match(html, /use 3 diagrams/, 'and the placeholder shows that asking for several works'); + assert.match(html, /Add illustrations — say how many in Instructions/); // Saying it in the instructions is as clear as ticking the box, so the box // follows rather than the request being dropped in silence. @@ -214,6 +230,37 @@ test('the author can ask for the illustration, not only leave it to the model', assert.match(js, /wireImageIntent\('mr-modify-instructions', 'mr-modify-images', 'mr-modify-image-hint'\)/); }); +test('a named number of figures survives a prompt full of library excerpts', () => { + const route = read('src/routes/myResources.js'); + const lib = read('src/utils/resourceImages.js'); + + // Measured, and deterministic on this model: with the library off, "use 3 + // diagrams" produced three tool calls; with thirty excerpts in the prompt it + // produced none and a longer deck instead. The excerpts are not wrong to + // dominate — the request simply has to survive them. + assert.match(route, /if \(tools\.length && resourceImages\.requestedCount\(refinement\)\) callOptions\.toolChoice = 'required';/); + assert.match(route, /if \(tools\.length && resourceImages\.requestedCount\(instructions\)\) callOptions\.toolChoice = 'required';/); + // With no number named the choice stays the model's. + assert.doesNotMatch(route, /toolChoice = 'required';\s*\n\s*var ai = await callAI\(messages, Object/); + + // Placement matters as much as wording: the illustration paragraph goes after + // the output rules and the author's instructions, because read before them it + // lost to a long "Output ONLY Pandoc markdown" block. + const tail = route.slice(route.indexOf("return 'You are writing teaching material")); + assert.ok(tail.indexOf('+ illustration;') > tail.indexOf('Additional instructions'), + 'illustration guidance is the last thing in the prompt'); + + // A model that has just made three tool calls tends to sign off rather than + // write. Measured: "I'll create the presentation and the three teaching + // diagrams." — 61 characters, saved as the resource, because only a + // completely empty body counted as missing. + assert.match(lib, /function looksLikeResource\(content\)/); + assert.match(lib, /if \(!looksLikeResource\(ai && ai\.content\)\)/); + assert.match(lib, /\^%\/m\.test\(text\) \|\| \/\^#\{1,2\}/, 'a title block or a heading, not mere length'); + // And if the continuation is no better, keep whichever actually reads like one. + assert.match(lib, /completed = ai;/); +}); + test('the library is bounded, searchable, and drives the modify picker', () => { const html = read('public/components/my-resources.html'); // Unbounded, a long library pushes everything else off the page.