From 1270899dcb84b8f9c0125fda4f1cc6f5957e5565 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 11 Sep 2026 16:47:08 +0200 Subject: [PATCH] feat: PubMed search for My Resources, and an image tool that actually fires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PubMed joins web search as an optional source for a generated resource: a literature search on the topic, with abstracts, cited by PMID in References. Off by default, admin-enabled, with its own optional API key (NCBI raises the rate limit from 3/sec to 10/sec; it works without one). Neither search is a tool any more, and that is the point. Offering them as function calls meant the model decided whether to search, and with a prompt ending "Output ONLY Pandoc markdown" it decided not to — every time, with and without corpus grounding, no matter how the tool description was worded. Calling callAI with the tool directly produced a correct pubmed_search call, so the plumbing was never the problem. The search only ever needed the topic, and the route knows the topic before it calls the model, so both searches now run up front and their results go into the prompt as findings, exactly the way corpus excerpts do. Ticking the box now means the search happened. Verified live against deepseek-v4-flash: 30 corpus excerpts and 6 PubMed results, and a References slide carrying both the library sources and four real PMIDs (29562151, 38506440, 35721052, 28814254). Three fixes to illustration, which had never once fired: - The dispatch call had been lost in a refactor. The tool was still offered, the model still called it, and the call was dropped, so no job was ever enqueued. - imageContext was passed as a bare topic string where dispatch expects { request, history }, which made the bound request undefined. - The prompt never mentioned the tool existed while explicitly demanding only markdown — the same suppression that killed the searches. It now says an illustration is available and that calling it is not a violation of that rule. my_resources is its own image workflow rather than a reuse of learning_hub, because generated_image_links only accepts learning_hub assets, and that is exactly the barrier that keeps a private illustration out of published content. The illustration renders in the panel, rather than a toast pointing at an image history this feature does not have. Verified end to end: job queued, rendered, and the asset served to its owner as a correctly labelled subglottic-anatomy teaching diagram. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- docs/retrieval-tuning.md | 2 +- .../1780400000000_my-resources-images.js | 24 +++ public/components/admin.html | 47 ++++- public/components/my-resources.html | 20 +++ public/js/admin.js | 25 ++- public/js/generatedImages.js | 2 +- public/js/myResources.js | 43 ++++- src/routes/adminConfig.js | 33 ++-- src/routes/generatedImages.js | 2 +- src/routes/myResources.js | 116 ++++++++---- src/utils/generatedImages.js | 2 +- src/utils/pubmedSearch.js | 167 ++++++++++++++++++ src/utils/webSearch.js | 22 +-- test/my-resources.test.js | 35 +++- test/web-search.test.js | 45 +++-- 15 files changed, 491 insertions(+), 94 deletions(-) create mode 100644 migrations/1780400000000_my-resources-images.js create mode 100644 src/utils/pubmedSearch.js diff --git a/docs/retrieval-tuning.md b/docs/retrieval-tuning.md index 32d6fa6f..9799e269 100644 --- a/docs/retrieval-tuning.md +++ b/docs/retrieval-tuning.md @@ -53,7 +53,7 @@ clamped on read so a bad value cannot break a search. | Feature | Keys | Default | Clamp | |---|---|---|---| -| Clinical Assistant | `clinical_assistant.search_limit`, `clinical_assistant.context_chars` | 12, 1400 | 3–20, 300–4000 | +| Clinical Assistant | `clinical_assistant.search_limit`, `clinical_assistant.context_chars` | 8, 1400 | 3–20, 300–4000 | | Learning Hub | `learning.search_limit`, `learning.context_chars` | 30, 2500 | 3–60, 300–8000 | | My Resources | *the same `learning.*` keys* | 30, 2500 | 3–60, 300–8000 | diff --git a/migrations/1780400000000_my-resources-images.js b/migrations/1780400000000_my-resources-images.js new file mode 100644 index 00000000..b93011cb --- /dev/null +++ b/migrations/1780400000000_my-resources-images.js @@ -0,0 +1,24 @@ +// Illustration for a person's own resources. +// +// A third image workflow rather than a reuse of learning_hub, because the two +// have opposite visibility rules. A learning_hub asset can be linked into +// published content and served to everyone; generated_image_links enforces +// that by requiring workflow='learning_hub', which is exactly the guarantee +// this feature needs to keep. Filing a private illustration under learning_hub +// would make it linkable into published content by anyone who knew its id. +// +// Nothing else is needed to serve them: asset() already grants the owner, so a +// my_resources image is visible to the person who made it and to nobody else. + +exports.up = pgm => pgm.sql(` + ALTER TABLE generated_image_jobs DROP CONSTRAINT IF EXISTS generated_image_jobs_workflow_check; + ALTER TABLE generated_image_jobs ADD CONSTRAINT generated_image_jobs_workflow_check + CHECK (workflow IN ('clinical_assistant', 'learning_hub', 'my_resources')); +`); + +exports.down = pgm => pgm.sql(` + DELETE FROM generated_image_jobs WHERE workflow='my_resources'; + ALTER TABLE generated_image_jobs DROP CONSTRAINT IF EXISTS generated_image_jobs_workflow_check; + ALTER TABLE generated_image_jobs ADD CONSTRAINT generated_image_jobs_workflow_check + CHECK (workflow IN ('clinical_assistant', 'learning_hub')); +`); diff --git a/public/components/admin.html b/public/components/admin.html index f25536b3..0fa5bc9a 100644 --- a/public/components/admin.html +++ b/public/components/admin.html @@ -419,7 +419,7 @@
-

Web Search

+

Search Sources

Off by default
@@ -470,8 +470,51 @@
+
+
+ PubMed +
+

+ A separate source from the web providers above, and a separate tick box for + authors. It returns structured records — title, journal, year, PMID — + so a reference can be exact rather than reconstructed from a page title. Queries + go to NCBI, not to a commercial provider. +

+ +
+ Enabled +
+ +
+
+ +
+ +
+ +

+ Optional. PubMed works without one; a key raises the rate limit from 3 + requests per second to 10, which matters for bulk work rather than for one + person writing a resource. Create one at + account.ncbi.nlm.nih.gov/settings — sign in, then API Key Management → Create an API Key. +

+
+
+ +
+ +
+ +

NCBI asks callers to identify themselves so they can contact you before blocking you. Optional but courteous.

+
+
+
+
- +
diff --git a/public/components/my-resources.html b/public/components/my-resources.html index 7430e644..e654af52 100644 --- a/public/components/my-resources.html +++ b/public/components/my-resources.html @@ -89,6 +89,21 @@ + +
@@ -98,6 +113,11 @@
+ + +
diff --git a/public/js/admin.js b/public/js/admin.js index 7446a046..f59fd9c6 100644 --- a/public/js/admin.js +++ b/public/js/admin.js @@ -430,8 +430,12 @@ function adminTabActive() { set('ws-enabled', cfg['websearch.enabled'] === 'true' ? 'true' : 'false'); set('ws-provider', cfg['websearch.provider'] || 'tavily'); set('ws-base-url', cfg['websearch.base_url']); + set('pm-enabled', cfg['pubmed.enabled'] === 'true' ? 'true' : 'false'); + set('pm-email', cfg['pubmed.contact_email']); var key = document.getElementById('ws-api-key'); if (key) key.placeholder = cfg['websearch.api_key'] || 'Leave blank to keep the current key'; + var pmKey = document.getElementById('pm-api-key'); + if (pmKey) pmKey.placeholder = cfg['pubmed.api_key'] || 'Optional — leave blank to keep the current key'; }) .catch(function() {}); } @@ -445,15 +449,20 @@ function adminTabActive() { enabled: (document.getElementById('ws-enabled') || {}).value, provider: (document.getElementById('ws-provider') || {}).value, apiKey: (document.getElementById('ws-api-key') || {}).value, - baseUrl: (document.getElementById('ws-base-url') || {}).value + baseUrl: (document.getElementById('ws-base-url') || {}).value, + pubmedEnabled: (document.getElementById('pm-enabled') || {}).value, + pubmedApiKey: (document.getElementById('pm-api-key') || {}).value, + pubmedEmail: (document.getElementById('pm-email') || {}).value }) }) .then(function(r) { return r.json(); }) .then(function(data) { if (!data.success) throw new Error(data.error || 'Save failed'); if (status) status.textContent = 'Saved ' + new Date().toLocaleTimeString() + '.'; - var key = document.getElementById('ws-api-key'); - if (key) key.value = ''; + ['ws-api-key', 'pm-api-key'].forEach(function (id) { + var field = document.getElementById(id); + if (field) field.value = ''; + }); showToast('Web search settings saved', 'success'); loadWebSearch(); }) @@ -472,10 +481,12 @@ function adminTabActive() { .then(function(r) { return r.json(); }) .then(function(data) { if (!status) return; - status.textContent = data.success - ? data.count + ' results from ' + data.provider + ' — ' + - (data.sample || []).map(function(x) { return x.title; }).slice(0, 2).join('; ') - : 'No results: ' + (data.reason || 'unknown'); + // Both sources, separately, so one press says which of them works. + var web = data.web || {}; + var pubmed = data.pubmed || {}; + status.textContent = + 'Web: ' + (web.reason ? web.reason : web.count + ' from ' + web.provider) + + ' · PubMed: ' + (pubmed.reason ? pubmed.reason : pubmed.count + ' records'); status.style.color = data.success ? 'var(--green)' : 'var(--red)'; }) .catch(function(err) { if (status) { status.textContent = err.message; status.style.color = 'var(--red)'; } }); diff --git a/public/js/generatedImages.js b/public/js/generatedImages.js index 53df2bd6..10a5604f 100644 --- a/public/js/generatedImages.js +++ b/public/js/generatedImages.js @@ -145,7 +145,7 @@ export function renderImageJobs(container, jobs, workflow, onDone) { const card = document.createElement('section'); card.className = 'assistant-image-card'; const status = document.createElement('p'); status.setAttribute('role', 'status'); card.append(status); container.append(card); - const base = workflow === 'learning_hub' ? '/api/admin/learning/image/jobs/' : '/api/clinical-assistant/image/jobs/'; + const base = { learning_hub: '/api/admin/learning/image/jobs/', my_resources: '/api/my-resources/image/jobs/' }[workflow] || '/api/clinical-assistant/image/jobs/'; async function poll() { if (!validSharingOwner(ticket) || !card.isConnected) return; try { diff --git a/public/js/myResources.js b/public/js/myResources.js index 2deefcb4..ef2f7b39 100644 --- a/public/js/myResources.js +++ b/public/js/myResources.js @@ -64,6 +64,8 @@ // never appears as something a user could turn on and be refused. var webRow = document.getElementById('mr-web-row'); if (webRow) webRow.hidden = !data.webSearchAvailable; + var pubmedRow = document.getElementById('mr-pubmed-row'); + if (pubmedRow) pubmedRow.hidden = !data.pubmedAvailable; }) .catch(function () { /* the defaults still work without this */ }); } @@ -104,7 +106,8 @@ useCorpus: corpusBox && corpusBox.checked === false ? 'false' : 'true', model: (document.getElementById('mr-model') || {}).value || '', withImages: (document.getElementById('mr-with-images') || {}).checked ? 'true' : 'false', - withWebSearch: (document.getElementById('mr-web-search') || {}).checked ? 'true' : 'false' + withWebSearch: (document.getElementById('mr-web-search') || {}).checked ? 'true' : 'false', + withPubmed: (document.getElementById('mr-pubmed') || {}).checked ? 'true' : 'false' }) }) .then(function (r) { return r.json(); }) @@ -119,14 +122,14 @@ g.used ? 'good' : null); // Say what was searched for. A query that left the network is worth // showing plainly rather than leaving someone to wonder. - if (data.webSearch && typeof showToast === 'function') { - showToast(data.webSearch.count - ? 'Searched the web for "' + data.webSearch.query + '" — ' + data.webSearch.count + ' results used.' - : 'Web search found nothing for "' + data.webSearch.query + '".', 'info'); - } - if ((data.imageJobs || []).length && typeof showToast === 'function') { - showToast('An illustration is being generated; it will appear in your image history.', 'info'); - } + (data.searches || []).forEach(function (s) { + if (typeof showToast !== 'function') return; + var where = s.tool === 'pubmed_search' ? 'PubMed' : 'the web'; + showToast(s.count + ? 'Searched ' + where + ' for "' + s.query + '" — ' + s.count + ' used.' + : 'Nothing found on ' + where + ' for "' + s.query + '".', 'info'); + }); + showIllustrations(data.imageJobs || []); loadLibrary(); }) .catch(function (err) { status(err.message, 'bad'); }) @@ -135,6 +138,28 @@ }); } + // The shared poller, so an image made here behaves exactly like one made in + // 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 showIllustrations(jobs) { + var box = document.getElementById('mr-images'); + if (!box) return; + box.innerHTML = ''; + if (!jobs.length) return; + import('/js/generatedImages.js').then(function (m) { + m.renderImageJobs(box, jobs, 'my_resources', function (card, data) { + var img = document.createElement('img'); + img.alt = 'Generated teaching illustration'; + img.style.maxWidth = '100%'; + card.append(img); + m.hydrateImage(img, data.imageUrl).catch(function () { img.alt = 'Private image unavailable'; }); + }); + }).catch(function () { + if (typeof showToast === 'function') showToast('An illustration was generated but could not be displayed.', 'info'); + }); + } + function loadLibrary() { var list = document.getElementById('mr-list'); if (!list) return; diff --git a/src/routes/adminConfig.js b/src/routes/adminConfig.js index 3fd831f7..815dcaa4 100644 --- a/src/routes/adminConfig.js +++ b/src/routes/adminConfig.js @@ -974,7 +974,8 @@ router.put('/config/:key(*)', async function(req, res) { // Its own routes rather than the generic config setter, because the key must be // masked on read and preserved when the field is left blank — the same handling // the OIDC client secret gets. -var WEBSEARCH_KEYS = ['websearch.enabled', 'websearch.provider', 'websearch.api_key', 'websearch.base_url']; +var WEBSEARCH_KEYS = ['websearch.enabled', 'websearch.provider', 'websearch.api_key', 'websearch.base_url', + 'pubmed.enabled', 'pubmed.api_key', 'pubmed.contact_email']; router.get('/websearch', adminMiddleware, async function (req, res) { try { @@ -983,9 +984,9 @@ router.get('/websearch', adminMiddleware, async function (req, res) { out[WEBSEARCH_KEYS[i]] = await db.getSetting(WEBSEARCH_KEYS[i], '') || ''; } // Never send the key back. Enough tail to recognise which one is set. - if (out['websearch.api_key']) { - out['websearch.api_key'] = '••••••••' + out['websearch.api_key'].slice(-4); - } + ['websearch.api_key', 'pubmed.api_key'].forEach(function (k) { + if (out[k]) out[k] = '••••••••' + out[k].slice(-4); + }); res.json({ success: true, config: out }); } catch (err) { logger.error('GET /admin/websearch', err.message); @@ -1005,8 +1006,16 @@ router.put('/websearch', adminMiddleware, async function (req, res) { // A blank field means "leave it alone", so editing the provider does not // silently wipe the key that was already working. + await db.setSetting('pubmed.enabled', String(req.body.pubmedEnabled) === 'true' ? 'true' : 'false'); + await db.setSetting('pubmed.contact_email', String(req.body.pubmedEmail || '').trim().slice(0, 200)); + + // A blank field means "leave it alone", so editing anything else does not + // silently wipe a key that was already working. The mask can never be saved + // back as a key. var key = String(req.body.apiKey || '').trim(); if (key && key.indexOf('•') === -1) await db.setSetting('websearch.api_key', key.slice(0, 400)); + var pmKey = String(req.body.pubmedApiKey || '').trim(); + if (pmKey && pmKey.indexOf('•') === -1) await db.setSetting('pubmed.api_key', pmKey.slice(0, 400)); res.json({ success: true }); } catch (err) { @@ -1017,14 +1026,16 @@ router.put('/websearch', adminMiddleware, async function (req, res) { router.post('/websearch/test', adminMiddleware, async function (req, res) { try { - var webSearch = require('../utils/webSearch'); - var found = await webSearch.search(String(req.body.query || 'paediatric bronchiolitis guideline')); + var query = String(req.body.query || 'paediatric bronchiolitis guideline'); + // Both sources, so one press says which of them actually works. + var web = await require('../utils/webSearch').search(query); + var pubmed = await require('../utils/pubmedSearch').search(query); res.json({ - success: !found.reason, - provider: found.provider || null, - count: found.results.length, - reason: found.reason || null, - sample: found.results.slice(0, 3).map(function (r) { return { title: r.title, url: r.url }; }) + success: !web.reason || !pubmed.reason, + web: { provider: web.provider || null, count: web.results.length, reason: web.reason || null, + sample: web.results.slice(0, 2).map(function (r) { return { title: r.title, url: r.url }; }) }, + pubmed: { count: pubmed.results.length, reason: pubmed.reason || null, + sample: pubmed.results.slice(0, 2).map(function (r) { return { title: r.title, pmid: r.pmid }; }) } }); } catch (err) { res.status(502).json({ success: false, reason: err.message }); diff --git a/src/routes/generatedImages.js b/src/routes/generatedImages.js index c3234d7b..a31d9117 100644 --- a/src/routes/generatedImages.js +++ b/src/routes/generatedImages.js @@ -45,7 +45,7 @@ router.get('/generated-images/:id', async (req, res) => { }); router.get('/image-jobs/:workflow', async (req, res) => { try { - if (!['clinical_assistant', 'learning_hub'].includes(req.params.workflow)) throw images.failure(404, 'Workflow not found'); + if (!['clinical_assistant', 'learning_hub', 'my_resources'].includes(req.params.workflow)) throw images.failure(404, 'Workflow not found'); const result = await db.query('SELECT id,stage,model,error_code,context_included,context_total,prompt_units,budget FROM generated_image_jobs WHERE owner_id=$1 AND workflow=$2 ORDER BY created_at DESC LIMIT 100', [req.user.id, req.params.workflow]); res.json({ success: true, jobs: result.rows.map(images.publicJob) }); } catch (e) { fail(res, e); } diff --git a/src/routes/myResources.js b/src/routes/myResources.js index ec1773dc..9c6e8e2b 100644 --- a/src/routes/myResources.js +++ b/src/routes/myResources.js @@ -21,7 +21,9 @@ 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 webSearch = require('../utils/webSearch'); +var pubmedSearch = require('../utils/pubmedSearch'); var documentExport = require('../utils/documentExport'); // Scoped to this router's own prefix. Mounted on /api, a bare @@ -78,6 +80,37 @@ function buildPrompt(opts) { 'LIBRARY EXCERPTS:\n"""\n' + opts.corpusContext + '\n"""\n' : ''; + // Literature and web findings arrive the same way corpus excerpts do: as + // material in the prompt, not as something the model has to ask for. An + // earlier version offered these as tools and the model never called them — + // it had been told to output only Pandoc markdown, and it obeyed that + // instead. Searching first is also deterministic: ticking the box now means + // the search happened, rather than that the model was allowed to consider it. + var findings = ''; + if (opts.literature) { + findings += "\nPublished literature found for this topic. Cite what you use by PMID in the " + + "References section, and do not cite anything not listed here.\n\n" + + "PUBMED RESULTS:\n\"\"\"\n" + opts.literature + "\n\"\"\"\n"; + } + if (opts.webFindings) { + findings += "\nCurrent material from the web. Use it for anything more recent than the " + + "library, and list what you use in the References section by title and URL.\n\n" + + "WEB RESULTS:\n\"\"\"\n" + opts.webFindings + "\n\"\"\"\n"; + } + + // The image tool is the one thing still left to the model to decide, so the + // prompt has to say it exists. Handing over a tool schema and then writing + // "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' + : ''; + var shape = kind === 'presentation' ? 'Write a ' + opts.slideCount + '-slide teaching presentation.\n\n' + 'Output ONLY Pandoc markdown:\n' + @@ -92,7 +125,7 @@ function buildPrompt(opts) { 'sections. Use prose, not slide bullets.\n'; return 'You are writing teaching material for a medical professional audience ' + - '(pediatrics / primary care).\n\nTOPIC: ' + opts.topic + '\n' + grounding + '\n' + shape + + '(pediatrics / primary care).\n\nTOPIC: ' + opts.topic + '\n' + grounding + findings + illustration + '\n' + shape + (opts.refinement ? '\nAdditional instructions: ' + opts.refinement + '\n' : ''); } @@ -103,6 +136,16 @@ function firstHeading(markdown, fallback) { // What this user may choose. Driven entirely by admin settings, so the screen // shows a single fixed model until an admin allows more — exactly like chat. +// Polling for an illustration this feature queued. Scoped to the caller and to +// this workflow, so it can only ever report on an image the caller made here. +router.get('/my-resources/image/jobs/:id', async function (req, res) { + try { + res.json(await require('../utils/generatedImages').service().get(req.params.id, req.user.id, 'my_resources')); + } catch (err) { + res.status(err.statusCode || 500).json({ error: err.statusCode ? err.message : 'Image status unavailable' }); + } +}); + router.get('/my-resources/options', async function (req, res) { try { var models = await allowedModels(); @@ -111,7 +154,8 @@ router.get('/my-resources/options', async function (req, res) { models: models.allowed, defaultModel: models.configured, imagesAvailable: Boolean(await db.getSetting('clinical_assistant.image_model', '')), - webSearchAvailable: await webSearch.isAvailable() + webSearchAvailable: await webSearch.isAvailable(), + pubmedAvailable: await pubmedSearch.isAvailable() }); } catch (err) { console.error('[my-resources] options:', err.message); @@ -139,12 +183,6 @@ router.post('/my-resources/generate', async function (req, res) { var corpus = { sources: [], context: '', reason: 'not requested' }; if (useCorpus) corpus = await learningRetrieval.retrieve(topic, db.getSetting); - var prompt = buildPrompt({ - topic: topic, kind: kind, refinement: refinement, corpusContext: corpus.context, - slideCount: clampInt(req.body.slideCount, 3, 30, 8), - wordCount: clampInt(req.body.wordCount, 200, 3000, 800) - }); - // Illustration is opt-in per generation. The tool is only offered when the // author ticked the box, because a model handed a drawing tool will find a // reason to use it, and most teaching material does not want one. @@ -153,50 +191,60 @@ router.post('/my-resources/generate', async function (req, res) { // This is the only path here that sends text outside the building. var wantsWeb = (String(req.body.withWebSearch) === 'true' || req.body.withWebSearch === true) && await webSearch.isAvailable(); + var wantsPubmed = (String(req.body.withPubmed) === 'true' || req.body.withPubmed === true) + && await pubmedSearch.isAvailable(); var imageModel = wantsImages ? String(await db.getSetting('clinical_assistant.image_model', '') || '') : ''; + + // Both searches run up front on the topic, so their results are in the + // prompt before a word is written. Neither can fail the generation. + var searches = []; + var literature = ''; + var webFindings = ''; + if (wantsPubmed) { + var papers = await pubmedSearch.search(topic); + searches.push({ tool: 'pubmed_search', query: topic, count: papers.results.length, reason: papers.reason }); + if (papers.results.length) literature = pubmedSearch.formatForPrompt(papers.results); + } + if (wantsWeb) { + var pages = await webSearch.search(topic); + searches.push({ tool: 'web_search', query: topic, count: pages.results.length, reason: pages.reason }); + if (pages.results.length) webFindings = webSearch.formatForPrompt(pages.results); + } + + var prompt = buildPrompt({ + topic: topic, kind: kind, refinement: refinement, corpusContext: corpus.context, + literature: literature, webFindings: webFindings, + wantsImages: Boolean(wantsImages && imageModel), + slideCount: clampInt(req.body.slideCount, 3, 30, 8), + wordCount: clampInt(req.body.wordCount, 200, 3000, 800) + }); + var model = await resolveModel(req.body.model); var messages = [{ role: 'user', content: prompt }]; var options = { model: model, temperature: 0.3 }; // Tools the model may reach for on this generation, and only these. + // Only the image tool stays a tool. Illustration genuinely needs the model to + // decide and to compose a prompt; a search only needs the topic, and the + // topic is already known. var tools = []; if (wantsImages && imageModel) tools = tools.concat(imageTool.tools); - if (wantsWeb) tools = tools.concat(webSearch.tools); var ai = await callAI(messages, tools.length ? Object.assign({}, options, { tools: tools }) : options); - // A web_search call is answered here and the model asked to continue, so - // the search result reaches the resource as material rather than as a tool - // transcript. One round only: a model allowed to keep searching will. - var webUsed = null; - if (wantsWeb && ai.toolCalls && ai.toolCalls.length) { - var call = ai.toolCalls.find(function (c) { - return c && c.function && c.function.name === 'web_search'; - }); - if (call) { - var args = {}; - try { args = JSON.parse(call.function.arguments || '{}'); } catch (e) { args = {}; } - var found = await webSearch.search(args.query); - webUsed = { query: String(args.query || ''), count: found.results.length, reason: found.reason }; - ai = await callAI(messages.concat([ - { role: 'assistant', content: null, tool_calls: [call] }, - { role: 'tool', tool_call_id: call.id, content: found.results.length - ? webSearch.formatForPrompt(found.results) - : 'No results. Write the resource without web material.' } - ]), Object.assign({}, options, { tools: tools, toolChoice: 'none' })); - } - } - // The same dispatcher the assistant uses, so an image generated here is - // owned, queued and rendered exactly as one generated there. + // 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. if (wantsImages && imageModel) { ai = await imageTool.dispatch(ai, { owner: req.user.id, workflow: 'my_resources', body: req.body, - imageContext: topic, imageModel: imageModel, + imageContext: generatedImages.imageContext(topic, []), imageModel: imageModel, messages: messages, options: options, callAI: callAI }); } + // Searches already ran; the model's only job was to write from what it was + // given. An empty body is a failed generation rather than an empty resource. var markdown = String((ai && ai.content) || '').trim(); if (!markdown) return res.status(502).json({ error: 'The model returned nothing. Try again.' }); @@ -212,7 +260,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 || [], - webSearch: webUsed, + searches: searches, model: ai && ai.model }); } catch (err) { diff --git a/src/utils/generatedImages.js b/src/utils/generatedImages.js index 2c41cb78..a724a569 100644 --- a/src/utils/generatedImages.js +++ b/src/utils/generatedImages.js @@ -3,7 +3,7 @@ const { DEFAULT_IMAGE_BEHAVIOR, imagePromptForCanvas } = require('./clinicalProm const storageUtil = require('./generatedImageStorage'); const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/; const failure = (statusCode, message) => Object.assign(new Error(message), { statusCode }); -const workflows = ['clinical_assistant', 'learning_hub']; +const workflows = ['clinical_assistant', 'learning_hub', 'my_resources']; function budgetLimit(value) { const n = value == null || value === '' ? 32000 : Number(value); if ((value != null && !['string', 'number'].includes(typeof value)) || !Number.isInteger(n) || n < 1000 || n > 32000) throw failure(400, 'Image budget must be 1000..32000 UTF-16 code units'); diff --git a/src/utils/pubmedSearch.js b/src/utils/pubmedSearch.js new file mode 100644 index 00000000..b5e47d5e --- /dev/null +++ b/src/utils/pubmedSearch.js @@ -0,0 +1,167 @@ +// ============================================================ +// PUBMED SEARCH +// The literature, as a source of its own. +// +// Deliberately not a provider option under web search. A web result is a page +// whose citation has to be reconstructed from its title; a PubMed record is +// structured — title, journal, year, authors, PMID — so a reference can be +// exact, and a References section can carry a PMID somebody can look up. +// +// A model should also be able to reach for "the literature" distinctly from +// "the web": a resource may legitimately want both, and each gets one search. +// +// NCBI E-utilities needs no key. A key only raises the rate limit from 3 +// requests a second to 10, which matters for indexing and not for a person +// generating one resource — so the key is optional and its absence is not a +// misconfiguration. Either way this reaches NCBI, not a commercial third party. +// ============================================================ + +var db = require('../db/database'); + +var BASE = 'https://eutils.ncbi.nlm.nih.gov/entrez/eutils'; +var MAX_RESULTS = 6; +var TIMEOUT_MS = 15000; +// NCBI asks callers to identify themselves so they can contact you before +// blocking you. Free, and the polite thing to do. +var TOOL_NAME = 'ped-ai'; + +async function settings() { + return { + enabled: String(await db.getSetting('pubmed.enabled', 'false')) === 'true', + apiKey: String(await db.getSetting('pubmed.api_key', '') || ''), + email: String(await db.getSetting('pubmed.contact_email', '') || '') + }; +} + +async function isAvailable() { + return (await settings()).enabled; +} + +function withCommon(url, s) { + url.searchParams.set('tool', TOOL_NAME); + if (s.email) url.searchParams.set('email', s.email); + if (s.apiKey) url.searchParams.set('api_key', s.apiKey); + return url; +} + +async function getJson(url) { + var r = await fetch(url, { headers: { Accept: 'application/json' }, signal: AbortSignal.timeout(TIMEOUT_MS) }); + if (!r.ok) throw new Error('PubMed returned ' + r.status); + return r.json(); +} + +function clip(text, n) { + return String(text || '').replace(/\s+/g, ' ').trim().slice(0, n); +} + +function yearOf(pubdate) { + var m = String(pubdate || '').match(/\d{4}/); + return m ? m[0] : ''; +} + +// Abstracts are fetched separately because esummary does not carry them, and +// they are what makes a record useful to write from rather than merely cite. +// Parsed leniently: a missing abstract is normal (editorials, some letters) and +// must not lose the record. +async function fetchAbstracts(pmids, s) { + if (!pmids.length) return {}; + try { + var url = withCommon(new URL(BASE + '/efetch.fcgi'), s); + url.searchParams.set('db', 'pubmed'); + url.searchParams.set('id', pmids.join(',')); + url.searchParams.set('retmode', 'xml'); + url.searchParams.set('rettype', 'abstract'); + var r = await fetch(url, { signal: AbortSignal.timeout(TIMEOUT_MS) }); + if (!r.ok) return {}; + var xml = await r.text(); + var out = {}; + // Each holds one PMID and zero or more AbstractText parts, + // which may be labelled (BACKGROUND, METHODS...) in structured abstracts. + String(xml).split('').slice(1).forEach(function (chunk) { + var pmid = (chunk.match(/]*>(\d+)<\/PMID>/) || [])[1]; + if (!pmid) return; + var parts = []; + var re = /]*>([\s\S]*?)<\/AbstractText>/g; + var m; + while ((m = re.exec(chunk)) !== null) { + parts.push(m[1].replace(/<[^>]+>/g, ' ')); + } + if (parts.length) out[pmid] = clip(parts.join(' '), 1500); + }); + return out; + } catch (e) { + return {}; + } +} + +/** + * Search PubMed for a topic. + * + * Never throws, for the same reason corpus retrieval and web search do not: a + * lookup failing must not fail the resource someone is writing. + */ +async function search(query) { + var text = String(query || '').trim().slice(0, 400); + if (!text) return { results: [], reason: 'empty query' }; + try { + var s = await settings(); + if (!s.enabled) return { results: [], reason: 'PubMed search is disabled' }; + + var searchUrl = withCommon(new URL(BASE + '/esearch.fcgi'), s); + searchUrl.searchParams.set('db', 'pubmed'); + searchUrl.searchParams.set('term', text); + searchUrl.searchParams.set('retmode', 'json'); + searchUrl.searchParams.set('retmax', String(MAX_RESULTS)); + searchUrl.searchParams.set('sort', 'relevance'); + var found = await getJson(searchUrl); + var pmids = ((found.esearchresult || {}).idlist || []).slice(0, MAX_RESULTS); + if (!pmids.length) return { results: [], reason: 'no results' }; + + var summaryUrl = withCommon(new URL(BASE + '/esummary.fcgi'), s); + summaryUrl.searchParams.set('db', 'pubmed'); + summaryUrl.searchParams.set('id', pmids.join(',')); + summaryUrl.searchParams.set('retmode', 'json'); + var summary = (await getJson(summaryUrl)).result || {}; + var abstracts = await fetchAbstracts(pmids, s); + + var results = pmids.map(function (pmid) { + var record = summary[pmid] || {}; + var authors = (record.authors || []).map(function (a) { return a.name; }).filter(Boolean); + return { + pmid: pmid, + title: clip(record.title, 300) || 'Untitled', + journal: clip(record.source, 150), + year: yearOf(record.pubdate), + // Three and "et al" is how a citation reads; the full list is noise here. + authors: authors.slice(0, 3).join(', ') + (authors.length > 3 ? ', et al' : ''), + url: 'https://pubmed.ncbi.nlm.nih.gov/' + pmid + '/', + abstract: abstracts[pmid] || '' + }; + }); + return { results: results, reason: null }; + } catch (e) { + return { results: [], reason: e.message || 'PubMed search failed' }; + } +} + +// Deliberately not a tool. Offering this as a function call meant the model +// decided whether to search, and with a prompt that ends "output only Pandoc +// markdown" it decided not to, every time. The search only ever needs the +// topic, and the topic is known before the model is called, so the route runs +// it up front and puts the findings in the prompt. + + +// Formatted so a model can cite it exactly: the PMID is right there, and the +// References section can carry it instead of an approximation. +function formatForPrompt(results) { + return results.map(function (r) { + return [ + r.title, + [r.authors, r.journal, r.year].filter(Boolean).join('. '), + 'PMID: ' + r.pmid + ' — ' + r.url, + r.abstract || '(no abstract available)' + ].join('\n'); + }).join('\n\n---\n\n'); +} + +module.exports = { search, isAvailable, settings, formatForPrompt, MAX_RESULTS }; diff --git a/src/utils/webSearch.js b/src/utils/webSearch.js index b6de45cc..f752eb69 100644 --- a/src/utils/webSearch.js +++ b/src/utils/webSearch.js @@ -137,20 +137,12 @@ async function search(query) { // The tool a model may call. Described tightly: a model given a search tool will // reach for it constantly unless told when not to. -var tools = [{ - type: 'function', - function: { - name: 'web_search', - description: 'Search the public web for current information — a guideline published after your training, a drug approval, an outbreak. ' + - 'Do not use it for settled clinical knowledge, which the indexed library and your own training already cover. ' + - 'One search per resource at most. Never send patient details or anything identifying.', - parameters: { - type: 'object', additionalProperties: false, - properties: { query: { type: 'string', minLength: 3, maxLength: 400 } }, - required: ['query'] - } - } -}]; +// Deliberately not a tool. Offering this as a function call meant the model +// decided whether to search, and with a prompt that ends "output only Pandoc +// markdown" it decided not to, every time. The search only ever needs the +// topic, and the topic is known before the model is called, so the route runs +// it up front and puts the findings in the prompt. + function formatForPrompt(results) { return results.map(function (r, i) { @@ -158,4 +150,4 @@ function formatForPrompt(results) { }).join('\n\n'); } -module.exports = { search, isAvailable, settings, tools, formatForPrompt, PROVIDERS, MAX_RESULTS }; +module.exports = { search, isAvailable, settings, formatForPrompt, PROVIDERS, MAX_RESULTS }; diff --git a/test/my-resources.test.js b/test/my-resources.test.js index acfbb31c..8b7d46a7 100644 --- a/test/my-resources.test.js +++ b/test/my-resources.test.js @@ -133,12 +133,45 @@ test('illustration is opt-in, and reuses the assistant’s image tool', () => { // only offered when the author asked for one. assert.match(route, /var wantsImages = String\(req\.body\.withImages\) === 'true'/); // Tools are assembled per generation: only what the author asked for. + // Illustration is the only thing left that is genuinely a tool: it needs the + // 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 && imageModel\) tools = tools\.concat\(imageTool\.tools\);/); - assert.match(route, /if \(wantsWeb\) tools = tools\.concat\(webSearch\.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'); + + // 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, \{/); + // 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, \[\]\)/); + + // 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, /wantsImages: Boolean\(wantsImages && imageModel\)/); + + // Its own workflow, not a reuse of learning_hub: generated_image_links only + // accepts learning_hub assets, and that is exactly the barrier keeping a + // private illustration out of published content. + assert.match(read('src/utils/generatedImages.js'), /const workflows = \['clinical_assistant', 'learning_hub', 'my_resources'\];/); + assert.match(read('migrations/1780400000000_my-resources-images.js'), /CHECK \(workflow IN \('clinical_assistant', 'learning_hub', 'my_resources'\)\)/); + // Status polling is owner-scoped and workflow-scoped, so it can only report + // on an image the caller made here. + assert.match(route, /service\(\)\.get\(req\.params\.id, req\.user\.id, 'my_resources'\)/); + assert.match(read('public/js/generatedImages.js'), /my_resources: '\/api\/my-resources\/image\/jobs\/'/); + // And it renders where the person is looking, rather than pointing them at an + // image history this feature does not have. + assert.match(read('public/js/myResources.js'), /showIllustrations\(data\.imageJobs \|\| \[\]\)/); + assert.match(read('public/components/my-resources.html'), /id="mr-images"/); assert.match(route, /imageJobs: ai\.imageJobs \|\| \[\]/, 'and reported back'); // The row is hidden entirely when no image model is configured. diff --git a/test/web-search.test.js b/test/web-search.test.js index a7873174..c02c6d5f 100644 --- a/test/web-search.test.js +++ b/test/web-search.test.js @@ -86,27 +86,50 @@ test('results are bounded, and a result with no URL is dropped', async () => { assert.ok(out.results.every(r => r.snippet.length <= 1200), 'snippets clipped'); }); -test('the tool tells the model when NOT to search', () => { - const src = read('src/utils/webSearch.js'); - // A model given a search tool will reach for it constantly unless told - // otherwise, and settled clinical knowledge is what the corpus is for. - assert.match(src, /Do not use it for settled clinical knowledge/); - assert.match(src, /One search per resource at most/); - assert.match(src, /Never send patient details or anything identifying/); - - // One round only, enforced in the route rather than trusted to the model. +test('searching is the route\u2019s job, not something the model is asked to do', () => { + // This was a tool first. Tested live against a question explicitly about + // recent trials, the model never called it \u2014 with or without corpus + // grounding, and no matter how the description was worded, because the prompt + // ends "Output ONLY Pandoc markdown" and a model told to output only markdown + // does not emit a tool call. Calling callAI with the tool directly produced a + // correct pubmed_search call, so the plumbing was never the problem. + // + // The search only ever needed the topic, and the route knows the topic before + // it calls the model. So both searches run up front and their results go into + // the prompt as findings, the same way corpus excerpts do. const route = read('src/routes/myResources.js'); - assert.match(route, /toolChoice: 'none'/); assert.match(route, /var wantsWeb = \(String\(req\.body\.withWebSearch\) === 'true'/); assert.match(route, /&& await webSearch\.isAvailable\(\)/, 'and the server checks, not just the UI'); + assert.match(route, /var pages = await webSearch\.search\(topic\);/); + assert.match(route, /var papers = await pubmedSearch\.search\(topic\);/); + + // Declared before they are used. They were not, once: `var` hoisting made + // wantsPubmed undefined at the point of the test, so the block never ran and + // said nothing about it. + assert.ok(route.indexOf('var wantsPubmed =') < route.indexOf('if (wantsPubmed)'), + 'declared above the branch that reads it'); + assert.ok(route.indexOf('var wantsWeb =') < route.indexOf('if (wantsWeb)')); + + // Neither search may fail a generation, so what happened is reported back + // instead: how many results, and why there were none. + assert.match(route, /searches\.push\(\{ tool: 'pubmed_search', query: topic, count: papers\.results\.length, reason: papers\.reason \}\);/); + assert.match(route, /searches\.push\(\{ tool: 'web_search', query: topic, count: pages\.results\.length, reason: pages\.reason \}\);/); + assert.match(route, /searches: searches/); + + // And neither library still advertises itself as a tool. + assert.doesNotMatch(read('src/utils/webSearch.js'), /name: 'web_search'/); + assert.doesNotMatch(read('src/utils/pubmedSearch.js'), /name: 'pubmed_search'/); }); test('the key is masked on read and preserved when left blank', () => { const admin = read('src/routes/adminConfig.js'); // Same handling the OIDC client secret gets. - assert.match(admin, /out\['websearch\.api_key'\] = '••••••••' \+ out\['websearch\.api_key'\]\.slice\(-4\)/); + // Both keys, one rule: never send a key back, enough tail to recognise it. + assert.match(admin, /\['websearch\.api_key', 'pubmed\.api_key'\]\.forEach/); + assert.match(admin, /if \(out\[k\]\) out\[k\] = '••••••••' \+ out\[k\]\.slice\(-4\);/); // Changing the provider must not silently wipe a working key. assert.match(admin, /if \(key && key\.indexOf\('•'\) === -1\) await db\.setSetting\('websearch\.api_key'/); + assert.match(admin, /if \(pmKey && pmKey\.indexOf\('•'\) === -1\) await db\.setSetting\('pubmed\.api_key'/); assert.match(admin, /if \(providers\.indexOf\(provider\) === -1\)/, 'and the provider is validated'); });