diff --git a/public/js/assistant/citations.js b/public/js/assistant/citations.js index 9d806432..17837c8b 100644 --- a/public/js/assistant/citations.js +++ b/public/js/assistant/citations.js @@ -102,6 +102,64 @@ function sourceByNumber(sources, n) { return list[n - 1]; } +/** + * Sources in the order the answer cites them, renumbered to match. + * + * They arrive in retrieval order, which is an order the reader never sees and + * has no way to follow: an answer whose first citation is [7] opened a list + * that began at [1], so matching a marker to a source meant hunting. Reference + * lists in published writing are numbered by first appearance for exactly this + * reason. + * + * Cited sources come first, renumbered 1..n by first appearance. Anything + * retrieved and not cited keeps its place after them — it is still evidence of + * what the search returned, which is what the panel is for, and it is no longer + * mixed in among the numbers the answer actually used. + * + * Returns new objects. Renumbering in place would corrupt a stored answer whose + * text still holds the original markers. + */ +export function orderSourcesByCitation(text, sources) { + var list = Array.isArray(sources) ? sources : []; + if (!list.length) return { text: String(text || ''), sources: list }; + + // First appearance wins, and only markers that resolve to a real source + // count — an invented number must not reserve a position in the list. + var order = []; + String(text || '').replace(/\[((?:\d+\s*,\s*)*\d+)\]/g, function (_, cluster) { + cluster.split(',').forEach(function (part) { + var n = Number(part.trim()); + if (!Number.isInteger(n) || n < 1) return; + if (!sourceByNumber(list, n)) return; + if (order.indexOf(n) === -1) order.push(n); + }); + return ''; + }); + if (!order.length) return { text: String(text || ''), sources: list }; + + var renumbered = []; + var mapping = {}; + order.forEach(function (was, i) { + var source = sourceByNumber(list, was); + mapping[was] = i + 1; + renumbered.push(Object.assign({}, source, { number: i + 1 })); + }); + list.forEach(function (source, i) { + var was = Number(source && source.number) || i + 1; + if (mapping[was]) return; // already placed + renumbered.push(Object.assign({}, source, { number: renumbered.length + 1, uncited: true })); + }); + + // Rewrite the markers in one pass. Doing it number by number would renumber + // something twice — 2 becomes 1, then that 1 becomes whatever 1 maps to. + var rewritten = String(text || '').replace(/\[((?:\d+\s*,\s*)*\d+)\]/g, function (match, cluster) { + var nums = cluster.split(',').map(function (p) { return Number(p.trim()); }); + if (nums.some(function (n) { return !mapping[n]; })) return match; // leave anything unresolved alone + return '[' + nums.map(function (n) { return mapping[n]; }).join(', ') + ']'; + }); + return { text: rewritten, sources: renumbered }; +} + export function renderCitationLinks(html, sources, options) { var opts = options || {}; return String(html || '').replace(new RegExp( @@ -115,7 +173,11 @@ export function renderCitationLinks(html, sources, options) { var title = source ? source.title || source.resource || 'Source' : 'Source'; var page = source && (source.page || source.page_number || source.pageNumber); var label = 'Source ' + n + ': ' + title + (page ? ', page ' + page : ''); - var text = opts.citationLabel === 'number' ? String(n) : 'src'; + // The number, not "src". Every marker used to read the same, so the only + // way to tell one citation from another was to hover it — and the list + // below is numbered, which made the numbering useless to match against. + // The export has always shown numbers; the screen now agrees with it. + var text = opts.citationLabel === 'text' ? 'src' : String(n); return '' + text + ''; }).join(' '); }); diff --git a/public/js/assistant/sources.js b/public/js/assistant/sources.js index 801a5327..46a88974 100644 --- a/public/js/assistant/sources.js +++ b/public/js/assistant/sources.js @@ -12,6 +12,10 @@ export function renderSourcesList(sources) { if (s.category) meta.push(s.category); if (s.doc_type || s.type) meta.push(s.doc_type || s.type); if (s.score != null) meta.push('score ' + Number(s.score).toFixed(3)); + // Retrieved, but the answer did not lean on it. Worth showing — the panel + // is also a view of what the search found — but worth distinguishing from + // the numbers the answer actually used. + if (s.uncited) meta.unshift('not cited'); return '
' + '[' + escapeHtml(n) + '] ' + escapeHtml(s.title || s.resource || 'Untitled source') + '' + renderSourceBadges(s) + diff --git a/public/js/clinicalAssistant.js b/public/js/clinicalAssistant.js index 824943f8..a9466a58 100644 --- a/public/js/clinicalAssistant.js +++ b/public/js/clinicalAssistant.js @@ -4,7 +4,7 @@ // server can call native MCP directly without routing through mcpo. // ============================================================ import { EMPTY_PROMPT_SETS } from './assistant/data.js'; -import { escapeAttr, escapeHtml, renderAssistantMarkdown, renderCitationLinks, safeImageUrl, wrapTables } from './assistant/citations.js'; +import { escapeAttr, escapeHtml, orderSourcesByCitation, renderAssistantMarkdown, renderCitationLinks, safeImageUrl, wrapTables } from './assistant/citations.js'; import { renderSourcesList } from './assistant/sources.js'; import { createAssistantExporter } from './assistant/export.js'; import { createAssistantImageStore } from './assistant/images.js'; @@ -598,8 +598,15 @@ import { if (request && request.cancelled) return; - lastAnswer = finalData.answer || finalData.markdown || ''; - lastSources = finalData.sources || finalData.citations || streamSources; + // Renumber here, where the whole answer is finally known. It cannot be + // done while streaming: the order is the order of first citation, and a + // citation that has not arrived yet cannot take its place — numbers would + // shuffle under the reader mid-sentence. + var ordered = orderSourcesByCitation( + finalData.answer || finalData.markdown || '', + finalData.sources || finalData.citations || streamSources); + lastAnswer = ordered.text; + lastSources = ordered.sources; replaceLoadingMessage(loading, lastAnswer, lastSources, finalData.suggestions || []); attachImageJobs(loading, messages[messages.length - 1], finalData.imageJobs || []); renderSources(lastSources); diff --git a/test/assistant-citations.test.js b/test/assistant-citations.test.js index 8ac98c9f..3c660ed4 100644 --- a/test/assistant-citations.test.js +++ b/test/assistant-citations.test.js @@ -28,14 +28,14 @@ test('renders citation clusters as links to matching source cards', async () => assert.match(html, /title="Source 1: Nelson Textbook of Pediatrics"/); assert.match(html, /data-source-number="1"/); assert.match(html, /data-source-number="2"/); - assert.match(html, /]*>src<\/a> ]*>src<\/a>/); + assert.match(html, /]*>\d+<\/a> ]*>\d+<\/a>/); }); test('renders escaped citation tokens as source links, not display math', async () => { const { renderAssistantMarkdown } = await loadCitationModule(); const katex = { renderToString: function () { throw new Error('citation sent to KaTeX'); } }; const html = renderAssistantMarkdown('Acquired hypothyroidism is uncommon. \\[1, 2\\].', sources, { katex }); - assert.match(html, /]*>src<\/a> ]*>src<\/a>/); + assert.match(html, /]*>\d+<\/a> ]*>\d+<\/a>/); assert.doesNotMatch(html, /katex-display/); }); @@ -64,9 +64,9 @@ test('leaves unknown citation clusters untouched rather than guessing', async () test('does not merge separate citations across separate claims', async () => { const { renderAssistantMarkdown } = await loadCitationModule(); const html = renderAssistantMarkdown('Use oxygen [1]. Give bronchodilator [2].', sources); - assert.match(html, /oxygen ]*data-source-number="1"[^>]*>src<\/a>/); - assert.match(html, /bronchodilator ]*data-source-number="2"[^>]*>src<\/a>/); - assert.doesNotMatch(html, /data-source-number="1"[^>]*>src<\/a>]*data-source-number="2"/); + assert.match(html, /oxygen ]*data-source-number="1"[^>]*>\d+<\/a>/); + assert.match(html, /bronchodilator ]*data-source-number="2"[^>]*>\d+<\/a>/); + assert.doesNotMatch(html, /data-source-number="1"[^>]*>\d+<\/a>]*data-source-number="2"/); }); test('sorts adjacent citation tokens into one safe Vancouver cluster', async () => { @@ -74,7 +74,7 @@ test('sorts adjacent citation tokens into one safe Vancouver cluster', async () const html = renderAssistantMarkdown('Deteriorating course [1][4][2][3].', [ { title: 'A' }, { title: 'B' }, { title: 'C' }, { title: 'D' } ]); - assert.match(html, /data-source-number="1"[^>]*>src<\/a> ]*data-source-number="2"[^>]*>src<\/a> ]*data-source-number="3"[^>]*>src<\/a> ]*data-source-number="4"[^>]*>src<\/a>/); + assert.match(html, /data-source-number="1"[^>]*>\d+<\/a> ]*data-source-number="2"[^>]*>\d+<\/a> ]*data-source-number="3"[^>]*>\d+<\/a> ]*data-source-number="4"[^>]*>\d+<\/a>/); assert.doesNotMatch(html, /\]]*>src<\/a> ]*data-source-number="2"[^>]*>src<\/a> ]*data-source-number="3"[^>]*>src<\/a> ]*data-source-number="4"[^>]*>src<\/a>/); + assert.match(html, /data-source-number="1"[^>]*>\d+<\/a> ]*data-source-number="2"[^>]*>\d+<\/a> ]*data-source-number="3"[^>]*>\d+<\/a> ]*data-source-number="4"[^>]*>\d+<\/a>/); }); test('does not normalize adjacent citations if any source number is unknown', async () => { @@ -132,14 +132,14 @@ test('does not turn citation-delimited prose into a list', async () => { test('joins a citation-only paragraph back to its claim', async () => { const { renderAssistantMarkdown } = await loadCitationModule(); const html = renderAssistantMarkdown('Acquired hypothyroidism is uncommon.\n\n[1, 2]\n\n.\n\n- Next point', sources); - assert.match(html, /uncommon\. ]*>src<\/a> ]*>src<\/a>\./); + assert.match(html, /uncommon\. ]*>\d+<\/a> ]*>\d+<\/a>\./); assert.doesNotMatch(html, /

\s* { const { renderAssistantMarkdown } = await loadCitationModule(); const html = renderAssistantMarkdown('Assess for lethargy [1] and dehydration [2].', sources); - assert.match(html, /lethargy ]*>src<\/a> and dehydration ]*>src<\/a>/); + assert.match(html, /lethargy ]*>\d+<\/a> and dehydration ]*>\d+<\/a>/); assert.doesNotMatch(html, /<\/a>
\s*and dehydration/); }); diff --git a/test/assistant-image-attachments.test.js b/test/assistant-image-attachments.test.js index b9c1f78b..896aefcf 100644 --- a/test/assistant-image-attachments.test.js +++ b/test/assistant-image-attachments.test.js @@ -291,6 +291,9 @@ function browserUI(options = {}) { console: quiet, AbortController, TextDecoder, TextEncoder, URL, Blob, crypto: require('node:crypto').webcrypto, FileReader: dom.window.FileReader, File: dom.window.File, setTimeout() {}, showToast(text, kind) { toasts.push([String(text), kind || '']); }, escapeHtml, escapeAttr: escapeHtml, + // Renumbering is exercised in test/citation-ordering.test.js against the + // real implementation; here it only has to exist and pass things through. + orderSourcesByCitation: (text, sources) => ({ text: text, sources: sources || [] }), renderAssistantMarkdown: text => escapeHtml(text), renderSourcesList: () => '', EMPTY_PROMPT_SETS: [[]], createAssistantExporter: () => ({ invalidate() {}, exportAnswerPdf() {} }), createAssistantImageStore: () => ({ renderGeneratedImage: src => '', clear() {} }), diff --git a/test/assistant-saved-tables.test.js b/test/assistant-saved-tables.test.js index 800b299a..8c63ec50 100644 --- a/test/assistant-saved-tables.test.js +++ b/test/assistant-saved-tables.test.js @@ -52,7 +52,9 @@ test('raw v2 actual save/load and SSE/fallback/append/export share all table cel c.fetchAssistantChat = async () => ({ success: true, answer: table, sources }); const loading = c.appendLoadingMessage(); await c.streamAssistantResponse({}, loading); - assert.deepEqual(rows(bubble(app)), [['Alpha', '1.25', 'A - B', 'src'], ['Beta', '2-4', 'unchanged', 'src']]); + // The chip shows its source number now rather than the word "src", so a + // reader can match a marker to the numbered list under the answer. + assert.deepEqual(rows(bubble(app)), [['Alpha', '1.25', 'A - B', '2'], ['Beta', '2-4', 'unchanged', '1']]); assert.match(bubble(app).querySelector('.assistant-cite').title, /Source 2: Synthetic B, page 19/); assert.equal(bubble(app).querySelectorAll('td')[1].getAttribute('align'), 'right'); assert.equal(app.window.getComputedStyle(bubble(app).querySelectorAll('td')[1]).textAlign, 'right'); @@ -110,7 +112,7 @@ test('code, math, escaped pipes and URLs cannot become lists or steal source-col assert.equal(rows(bubble(app)).length, 2); assert.equal(rows(bubble(app))[0][1], 'A - B [1] and $x - y$ and \\(a - b\\)'); assert.equal(rows(bubble(app))[1][1], 'a|b and URL'); - assert.equal(rows(bubble(app))[1][2], 'src'); + assert.equal(rows(bubble(app))[1][2], '1'); assert.equal(bubble(app).querySelector('code .assistant-cite'), null); assert.equal(bubble(app).querySelector('a[href^="https:"]').getAttribute('href'), 'https://example.test/a-b?q=1%7C2'); for (const literal of ['`' + collapse(table) + '`', '~~~md\n' + collapse(table) + '\n~~~', ' ' + collapse(table), '$$' + collapse(table) + '$$', '\\[' + collapse(table) + '\\]', 'https://example.test/' + collapse(table).replace(/ /g, '%20')]) { @@ -177,7 +179,7 @@ test('legacy recovery refuses missing boundaries; supports independent lines and reopen(app, collapse(escaped), 1); assert.equal(rows(bubble(app)).length, 2); assert.equal(rows(bubble(app))[0][1], 'a|b and x|y and $x - y$'); - assert.equal(rows(bubble(app))[0][2], 'src'); + assert.equal(rows(bubble(app))[0][2], '2'); reopen(app, collapse(table) + '\n\nCaption two\n\n' + collapse(table), 1); assert.equal(bubble(app).querySelectorAll('table').length, 2); for (const raw of ['| A | B | --- | --- | x | y |', '| A | B | | --- | --- | | x | y', '| A | B | | --- | --- | | x | y | trailing prose']) { @@ -198,7 +200,7 @@ test('math/code literals and sentinel-shaped input survive postprocessing withou assert.equal(bubble(app).querySelector('code').textContent, 'a|b [2][1] $notmath$'); assert.equal(bubble(app).querySelector('code .katex, .katex .assistant-cite'), null); assert.deepEqual(expressions, ['x \\mid y [1]', 'a - b']); - assert.equal(rows(bubble(app))[1][2], 'src'); + assert.equal(rows(bubble(app))[1][2], '1'); for (const raw of ['$$\n' + table + '\n$$', '~~~md\n' + table + '\n[1][2] $code$\n~~~', '\uE000html:0\uE001 and `\uE000markdown:0\uE001 [2][1]`']) { reopen(app, raw); assert.equal(bubble(app).querySelectorAll('table, .assistant-cite').length, 0); @@ -408,7 +410,12 @@ test('durable jobs preserve legacy provenance, provisional/clicked turn sources assert.match(app.document.querySelector('#assistant-source-2').textContent, /Synthetic B.*19/is); await c.performAutosave(); const saved = app.saves.at(-1); assert.equal(saved.messages[0].content, raw); assert.equal(saved.messages[0].retainedAnswer, retained); assert.equal(saved.messages[0].legacyClipped, true); - assert.deepEqual(saved.messages[0].sources, sources); assert.deepEqual(saved.sources, secondSources); + assert.deepEqual(saved.messages[0].sources, sources); + // Sources are reordered into citation order on the final render, and anything + // retrieved but never cited is marked so the panel can say so. This answer + // cites only the first, so the second is carried along as "not cited". + assert.deepEqual(saved.sources, + [secondSources[0], Object.assign({}, secondSources[1], { uncited: true })]); assert.deepEqual(saved.messages[0].imageJobs, [{ jobId: id }]); assert.equal(saved.messages.at(-1).content, second); c.restoreSavedChat(saved); await tick(); const before = JSON.stringify(c.messages); diff --git a/test/citation-ordering.test.js b/test/citation-ordering.test.js new file mode 100644 index 00000000..b3a2ad1f --- /dev/null +++ b/test/citation-ordering.test.js @@ -0,0 +1,111 @@ +// Sources arrived in retrieval order — an order the reader never sees and +// cannot follow. An answer whose first citation was [7] opened a list that +// began at [1], so matching a marker to a source meant hunting for it. +// +// Borrowed from the quiz app, where validating citations and ordering them fall +// out of the same pass: it collects the sources the answer actually used into an +// insertion-ordered map, so the list comes back in first-citation order for +// free. Reference lists in published writing are numbered by first appearance +// for the same reason. +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'); + +function load() { + const src = fs.readFileSync(path.join(__dirname, '..', 'public/js/assistant/citations.js'), 'utf8') + .replace(/^import[\s\S]*?from ['"][^'"]+['"];\s*/gm, '') + .replace(/^export /gm, ''); + const ctx = { window: {}, document: undefined, console }; + vm.createContext(ctx); + vm.runInContext(src + '\nthis.orderSourcesByCitation = orderSourcesByCitation;', ctx); + return ctx; +} + +const four = [ + { number: 1, title: 'Alpha' }, { number: 2, title: 'Beta' }, + { number: 3, title: 'Gamma' }, { number: 4, title: 'Delta' } +]; + +test('sources come back in the order the answer cites them', () => { + const { orderSourcesByCitation } = load(); + const out = orderSourcesByCitation('Third first [3]. Then the first [1].', four); + assert.deepEqual(Array.from(out.sources.slice(0, 2), s => s.title), ['Gamma', 'Alpha']); + assert.equal(out.text, 'Third first [1]. Then the first [2].'); +}); + +test('a cluster is renumbered as a whole, in its own order', () => { + const { orderSourcesByCitation } = load(); + const out = orderSourcesByCitation('Both of these [4, 2] agree.', four); + assert.equal(out.text, 'Both of these [1, 2] agree.'); + assert.deepEqual(Array.from(out.sources.slice(0, 2), s => s.title), ['Delta', 'Beta']); +}); + +test('renumbering happens in one pass, so nothing is renumbered twice', () => { + // Rewriting number by number turns 2 into 1, then that 1 into whatever 1 + // maps to. The whole text is rewritten once instead. + const { orderSourcesByCitation } = load(); + const out = orderSourcesByCitation('[2] then [1] then [2] again.', four); + assert.equal(out.text, '[1] then [2] then [1] again.'); +}); + +test('a source cited twice keeps its first position', () => { + const { orderSourcesByCitation } = load(); + const out = orderSourcesByCitation('[3] ... [1] ... [3] again.', four); + assert.deepEqual(Array.from(out.sources.slice(0, 2), s => s.title), ['Gamma', 'Alpha']); +}); + +test('retrieved but uncited sources follow, marked and still numbered', () => { + // The panel is also a view of what the search returned, so they stay — just + // no longer mixed in among the numbers the answer used. + const { orderSourcesByCitation } = load(); + const out = orderSourcesByCitation('Only this one [2].', four); + assert.equal(out.sources[0].title, 'Beta'); + assert.equal(out.sources[0].uncited, undefined); + assert.equal(out.sources.length, 4, 'nothing is dropped'); + assert.deepEqual(Array.from(out.sources.slice(1), s => s.uncited), [true, true, true]); + assert.deepEqual(Array.from(out.sources, s => s.number), [1, 2, 3, 4], 'numbering stays contiguous'); +}); + +test('an invented citation reserves no place and is left alone', () => { + // It is not turned into a link either; the audit records it. What matters + // here is that it cannot push a real source down the list. + const { orderSourcesByCitation } = load(); + const out = orderSourcesByCitation('Invented [9]. Real [2].', four); + assert.equal(out.sources[0].title, 'Beta'); + assert.match(out.text, /Invented \[9\]/, 'the unresolved marker is untouched'); + assert.match(out.text, /Real \[1\]/); +}); + +test('a cluster containing an invented number is left whole', () => { + // Renumbering half of it would silently change which source the good half + // points at. + const { orderSourcesByCitation } = load(); + const out = orderSourcesByCitation('Mixed [2, 9].', four); + assert.equal(out.text, 'Mixed [2, 9].'); +}); + +test('an answer that cites nothing is returned untouched', () => { + const { orderSourcesByCitation } = load(); + const out = orderSourcesByCitation('No citations here.', four); + assert.equal(out.text, 'No citations here.'); + assert.deepEqual(Array.from(out.sources), four); +}); + +test('the originals are not mutated, so a stored answer stays readable', () => { + // Its text still holds the original markers; renumbering in place would make + // the two disagree. + const { orderSourcesByCitation } = load(); + const before = JSON.parse(JSON.stringify(four)); + orderSourcesByCitation('[3] [1]', four); + assert.deepEqual(four, before); +}); + +test('sources with no number field fall back to position', () => { + const { orderSourcesByCitation } = load(); + const bare = [{ title: 'One' }, { title: 'Two' }, { title: 'Three' }]; + const out = orderSourcesByCitation('Cite the third [3].', bare); + assert.equal(out.sources[0].title, 'Three'); + assert.equal(out.text, 'Cite the third [1].'); +}); diff --git a/test/clinical-conversation.test.js b/test/clinical-conversation.test.js index 36a84c7d..5914c458 100644 --- a/test/clinical-conversation.test.js +++ b/test/clinical-conversation.test.js @@ -175,6 +175,9 @@ function browserUI(options = {}) { window: dom.window, document: dom.window.document, navigator: dom.window.navigator, console: quiet, AbortController, TextDecoder, TextEncoder, URL, Blob, crypto: require("node:crypto").webcrypto, setTimeout() {}, showToast() {}, escapeHtml, escapeAttr: escapeHtml, + // Renumbering is exercised in test/citation-ordering.test.js against the + // real implementation; here it only has to exist and pass things through. + orderSourcesByCitation: (text, sources) => ({ text: text, sources: sources || [] }), renderAssistantMarkdown: text => escapeHtml(text), renderSourcesList: () => '', ...options.renderers, EMPTY_PROMPT_SETS: [[]], createAssistantExporter: () => ({ invalidate() {}, exportAnswerPdf() {} }), createAssistantImageStore: () => ({ renderGeneratedImage: src => '', clear() {} }),