diff --git a/public/css/assistant.css b/public/css/assistant.css index 43294b6a..429cefd2 100644 --- a/public/css/assistant.css +++ b/public/css/assistant.css @@ -177,6 +177,12 @@ .assistant-source-excerpt ul, .assistant-source-excerpt ol { padding-left:16px; margin:4px 0; } .assistant-source-excerpt strong { color:var(--g700); } .assistant-muted { color:var(--g500); font-size:12px; line-height:1.6; } +/* The block still arriving. It keeps the bubble's own type — it was a bare +
, which fell back to the browser's black monospace default and was the
+   dark flash people saw while a table streamed. pre-wrap so the line breaks of
+   a half-finished table are visible rather than collapsed into one run. */
+.assistant-streaming-text { margin:0; white-space:pre-wrap; overflow-wrap:anywhere; color:var(--g700); }
+.assistant-stream-settled:not(:empty) + .assistant-streaming-text:not([hidden]) { margin-top:10px; }
 .assistant-mermaid { background:white; border:1px solid var(--g200); border-radius:10px; padding:10px; margin:10px 0; overflow:auto; }
 .assistant-source-modal { position:fixed; inset:0; z-index:10000; background:rgba(15,23,42,.72); display:flex; align-items:center; justify-content:center; padding:24px; }
 .assistant-source-modal .modal-content { width:100%; max-width:640px; max-height:88vh; overflow:auto; }
diff --git a/public/js/clinicalAssistant.js b/public/js/clinicalAssistant.js
index fbf454af..39184705 100644
--- a/public/js/clinicalAssistant.js
+++ b/public/js/clinicalAssistant.js
@@ -40,7 +40,6 @@ import {
   var statusChoices = { allowedChatModels: [], allowedImageModels: [], chatModel: '', imageModel: '' };
   var activeAssistantRequest = null;
   var conversationChars = null;
-  var STREAM_MARKDOWN_LIMIT = 3500;
   var AUTOSAVE_DELAY_MS = 800;
   var currentChatId = null;
   var autosaveTimer = null;
@@ -535,8 +534,7 @@ import {
       loading.classList.remove('assistant-loading-msg');
       bubble.classList.remove('assistant-thinking');
       bubble.assistantSources = streamSources;
-      bubble.innerHTML = partial ? renderStreamingAnswerHtml(partial, streamSources) : '

Generating answer...

'; - renderEmbeddedBlocks(bubble); + renderStreamingInto(bubble, partial, streamSources); var wrap = document.getElementById('assistant-messages'); if (wrap) wrap.scrollTop = wrap.scrollHeight; } @@ -600,18 +598,113 @@ import { setBusy(false, 'Ready'); } - function renderStreamingAnswerHtml(text, sources) { - if (shouldUseLightweightStreamingRender(text)) { - return '
' + escapeHtml(text) + '
'; - } - return renderAssistantBubbleHtml(text, sources, false); + // ---- streaming render ----------------------------------------------------- + // + // Markdown only makes sense once a block is finished. Half a table is a row of + // pipes; half a fence is a stray ```. Re-parsing the whole partial answer every + // frame therefore flashed between a broken parse and the real thing, and the + // old answer to that was to give up on markdown entirely past a size or a + // pipe-row count and show raw text in a
 — the black monospace block.
+  //
+  // Instead: split the text at the last finished block, render everything before
+  // it as markdown once and *append* it, and keep only the unfinished tail as
+  // plain text. Settled content is never re-parsed and never re-rendered, so a
+  // diagram or chart that has already drawn is not thrown away on the next
+  // token, and the tail is the only thing that changes each frame.
+  //
+  // Streaming is a preview. The 'done' handler still renders the whole answer
+  // from scratch through fillMessageBubble, so anything this shows mid-stream —
+  // a loose list briefly split in two, a reference link not yet defined — is
+  // settled correctly by the final pass. That is what makes appending safe.
+
+  function beginStreamingRender(bubble) {
+    bubble.innerHTML = '';
+    var state = {
+      settled: document.createElement('div'),
+      tail: document.createElement('p'),
+      consumed: 0
+    };
+    state.settled.className = 'assistant-stream-settled';
+    state.tail.className = 'assistant-streaming-text';
+    bubble.appendChild(state.settled);
+    bubble.appendChild(state.tail);
+    bubble.assistantStreamState = state;
+    return state;
   }
 
-  function shouldUseLightweightStreamingRender(text) {
-    text = String(text || '');
-    if (text.length > STREAM_MARKDOWN_LIMIT) return true;
-    var pipeRows = text.split('\n').filter(function (line) { return /^\s*\|.*\|\s*$/.test(line); }).length;
-    return pipeRows >= 8;
+  function renderStreamingInto(bubble, text, sources) {
+    if (!text) {
+      bubble.assistantStreamState = null;
+      bubble.innerHTML = '

Generating answer...

'; + return; + } + var state = bubble.assistantStreamState || beginStreamingRender(bubble); + var cut = settledMarkdownLength(text, state.consumed); + if (cut > state.consumed) { + var holder = document.createElement('div'); + holder.innerHTML = renderAssistantBubbleHtml(text.slice(state.consumed, cut), sources, false); + state.consumed = cut; + while (holder.firstChild) state.settled.appendChild(holder.firstChild); + // Over the whole settled subtree, not just the new nodes: a newly appended + // node may itself be the diagram, which querySelectorAll would skip. Blocks + // already drawn are marked and skipped inside renderEmbeddedBlocks. + renderEmbeddedBlocks(state.settled); + } + var tail = text.slice(state.consumed); + state.tail.textContent = tail; + state.tail.hidden = !tail; + } + + /** + * How much of `text` is finished markdown, as a length. + * + * A blank line outside a code fence ends a block. Two exceptions, both of + * them blank lines a block owns rather than is separated by: the gap between + * the items of a loose list, where cutting renders one list as two each + * restarting at 1, and a blank line within an indented code block. When + * nothing qualifies — a single long paragraph, a fence opened on the first + * line — this returns `from` and everything stays in the tail, which is both + * the old behaviour and correct. + */ + function settledMarkdownLength(text, from) { + var fence = null; + var offset = 0; + var best = from; + var lines = String(text).split('\n'); + for (var i = 0; i < lines.length; i++) { + var line = lines[i]; + var opener = line.match(/^\s{0,3}(```+|~~~+)/); + if (fence) { + if (opener && opener[1][0] === fence[0] && opener[1].length >= fence.length) fence = null; + } else if (opener) { + fence = opener[1]; + } else if (!line.trim()) { + // Past the blank line, so the settled chunk ends with the break and the + // tail starts on real content. + var cut = offset + line.length + 1; + if (cut > from && !continuesBlock(lines, i)) best = cut; + } + offset += line.length + 1; // the \n that split() removed + } + return best; + } + + /** + * Whether the blank line at `i` sits inside one block rather than between two. + * + * It takes both sides to tell: a list item *after* the blank only means a list + * is being split if there was already one before it. Looking forward alone + * rejects the perfectly good boundary between a sentence and the list it + * introduces, which is most of what this assistant writes. + */ + function continuesBlock(lines, i) { + var ITEM = /^(\s{4,}|\s*([-*+]|\d+[.)])\s)/; + var next = ''; + for (var f = i + 1; f < lines.length; f++) { if (lines[f].trim()) { next = lines[f]; break; } } + if (!next || !ITEM.test(next)) return false; + var prev = ''; + for (var b = i - 1; b >= 0; b--) { if (lines[b].trim()) { prev = lines[b]; break; } } + return ITEM.test(prev); } function parseSseEvent(block) { @@ -698,6 +791,9 @@ import { } function fillMessageBubble(bubble, role, content, sources, suggestions, rawHtml, options) { + // The final render replaces everything, so the streaming state's nodes are + // now detached; leaving it set would have a later frame append into nothing. + bubble.assistantStreamState = null; bubble.assistantSources = role === 'assistant' && Array.isArray(sources) ? sources : []; bubble.assistantRawContent = String(content || ''); var renderOptions = Object.assign({}, options || {}, { notice: '' }); @@ -919,13 +1015,24 @@ import { }); } + // Mermaid diagrams and Chart.js canvases, drawn once each. + // + // Both are expensive and both are claimed: mermaid.render is async and + // replaces the element's contents, and a second Chart on the same canvas + // throws "Canvas is already in use". Streaming calls this again every time a + // block settles, so each element is marked when it is taken and skipped + // afterwards — otherwise a diagram near the top of a long answer would be torn + // down and redrawn on every block that followed it. function renderEmbeddedBlocks(root) { wireCodeBlocks(root); function mermaidSource(el) { var raw = el.getAttribute('data-mermaid') || ''; try { return decodeURIComponent(raw); } catch (e) { return raw; } } - root.querySelectorAll('[data-mermaid]').forEach(function (el) { + root.querySelectorAll('[data-mermaid]:not([data-drawn])').forEach(function (el) { + // Marked before the await, not after: two calls a frame apart would both + // get past an await-side check and race to render the same element. + el.setAttribute('data-drawn', '1'); ensureMermaid().then(function () { if (!window.mermaid) { el.textContent = mermaidSource(el); return; } var id = 'assistant-mermaid-' + Math.random().toString(16).slice(2); @@ -934,8 +1041,9 @@ import { .catch(function () { el.textContent = mermaidSource(el); }); }); }); - root.querySelectorAll('canvas[data-chart]').forEach(function (canvas) { - if (!window.Chart) return; + root.querySelectorAll('canvas[data-chart]:not([data-drawn])').forEach(function (canvas) { + if (!window.Chart) return; // unmarked, so it draws if Chart.js arrives later + canvas.setAttribute('data-drawn', '1'); try { var cfg = JSON.parse(canvas.getAttribute('data-chart') || '{}'); new window.Chart(canvas.getContext('2d'), cfg); diff --git a/test/assistant-citations.test.js b/test/assistant-citations.test.js index 25837b77..8ac98c9f 100644 --- a/test/assistant-citations.test.js +++ b/test/assistant-citations.test.js @@ -273,12 +273,17 @@ test('does not repair malformed tab-separated clinical summary rows into a table assert.match(html, /If you need details/); }); -test('clinical assistant streams long table answers as lightweight text before final render', async () => { +test('a streaming table renders block by block, not as a raw text dump', async () => { + // Previously a long or pipe-heavy answer gave up on markdown entirely and + // went into an unstyled
 — the dark flash while a table streamed. Now
+  // the finished blocks are markdown and only the unfinished tail is text.
+  // Behaviour lives in test/assistant-streaming-blocks.test.js; this guards
+  // against the old bail-out returning.
   const source = await fs.readFile(path.join(__dirname, '..', 'public', 'js', 'clinicalAssistant.js'), 'utf8');
-  assert.match(source, /STREAM_MARKDOWN_LIMIT/);
-  assert.match(source, /renderStreamingAnswerHtml\(partial, streamSources\)/);
-  assert.match(source, /assistant-streaming-text/);
-  assert.match(source, /pipeRows >= 8/);
+  assert.doesNotMatch(source, /STREAM_MARKDOWN_LIMIT/);
+  assert.doesNotMatch(source, /pipeRows >= 8/);
+  assert.match(source, /function settledMarkdownLength\(text, from\)/);
+  assert.match(source, /renderStreamingInto\(bubble, partial, streamSources\)/);
 });
 
 test('mermaid source survives the sanitiser and round-trips', async () => {
diff --git a/test/assistant-streaming-blocks.test.js b/test/assistant-streaming-blocks.test.js
new file mode 100644
index 00000000..819a94e5
--- /dev/null
+++ b/test/assistant-streaming-blocks.test.js
@@ -0,0 +1,207 @@
+// A markdown table only parses once its last row has arrived. Re-parsing the
+// whole partial answer every 180ms therefore flashed between a broken parse and
+// the real thing, and the old answer was to abandon markdown past a size or a
+// pipe-row count and dump raw text into an unstyled 
 — the black monospace
+// block people saw while a table streamed.
+//
+// Now the text is split at the last finished block: everything before it is
+// rendered once and appended, and only the unfinished tail is plain text.
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const vm = require('node:vm');
+const { JSDOM } = require('jsdom');
+const { marked } = require('marked');
+
+const read = file => fs.readFileSync(path.join(__dirname, '..', file), 'utf8');
+
+function ui(t) {
+  const dom = new JSDOM('
' + read('public/components/assistant.html') + '
', + { url: 'https://example.test', runScripts: 'outside-only' }); + const window = dom.window; + window.eval(read('public/js/accountBoundary.js')); + window.AccountBoundary.enter({ id: 'synthetic-streaming-owner' }, true); + window.marked = marked; + window.DOMPurify = require('dompurify')(window); + window.matchMedia = () => ({ matches: true }); + const context = { + window, document: window.document, console, URL, Blob, TextDecoder, AbortController, + setTimeout() {}, showToast() {}, EMPTY_PROMPT_SETS: [[]], + createAssistantImageStore: () => ({ clear() {}, renderGeneratedImage: () => '' }), + fetchSavedAssistantChats: async () => ({ success: true, chats: [] }), + saveAssistantChat: async () => ({ success: true }) + }; + vm.createContext(context); + for (const file of ['assistant/citations.js', 'assistant/sources.js', 'assistant/sharing.js', + 'generatedImages.js', 'assistant/export.js', 'clinicalAssistant.js']) { + vm.runInContext(read('public/js/' + file) + .replace(/^import[\s\S]*?from ['"][^'"]+['"];\s*/gm, '').replace(/^export /gm, ''), context); + } + t.after(() => window.close()); + return context; +} + +const bubbleFor = ctx => ctx.document.createElement('div'); +const TABLE = '| Drug | Dose |\n| --- | --- |\n| Adrenaline | 10 mcg/kg |\n| Amiodarone | 5 mg/kg |'; + +// ---- where the cut falls --------------------------------------------------- + +test('a finished paragraph settles, the one still arriving does not', (t) => { + const { settledMarkdownLength } = ui(t); + const text = 'First paragraph, complete.\n\nSecond one, still typ'; + const cut = settledMarkdownLength(text, 0); + assert.equal(text.slice(0, cut).trim(), 'First paragraph, complete.'); + assert.equal(text.slice(cut).trim(), 'Second one, still typ'); +}); + +test('a half-written table stays in the tail rather than parsing as broken', (t) => { + const { settledMarkdownLength } = ui(t); + const text = 'Resuscitation doses:\n\n| Drug | Dose |\n| --- | --- |\n| Adrenaline | 10 mc'; + const cut = settledMarkdownLength(text, 0); + assert.equal(text.slice(0, cut).trim(), 'Resuscitation doses:'); + assert.ok(text.slice(cut).includes('| Adrenaline')); +}); + +test('a blank line inside an open code fence is not a boundary', (t) => { + const { settledMarkdownLength } = ui(t); + // Cutting here would render a stray ``` and leave the rest unfenced. + const text = 'Example:\n\n```js\nconst a = 1;\n\nconst b = 2;'; + const cut = settledMarkdownLength(text, 0); + assert.equal(text.slice(0, cut).trim(), 'Example:'); +}); + +test('a closed fence settles, and the fence state resets after it', (t) => { + const { settledMarkdownLength } = ui(t); + const text = '```js\nconst a = 1;\n```\n\nAfter the block.\n\nStill writ'; + const cut = settledMarkdownLength(text, 0); + assert.ok(text.slice(0, cut).includes('After the block.')); + assert.equal(text.slice(cut).trim(), 'Still writ'); +}); + +test('the blank line inside a loose list is not a boundary', (t) => { + const { settledMarkdownLength } = ui(t); + // Cutting there renders one list as two, each restarting at 1. + const text = 'Steps:\n\n1. Airway\n\n2. Breathing\n\n3. Circu'; + const cut = settledMarkdownLength(text, 0); + assert.equal(text.slice(0, cut).trim(), 'Steps:'); + assert.ok(text.slice(cut).startsWith('1. Airway')); +}); + +test('but the blank line before a list starts is a boundary', (t) => { + const { settledMarkdownLength } = ui(t); + // Rejecting on the following line alone would refuse this too, and a sentence + // introducing a list is most of what this assistant writes. + const text = 'Assess in this order:\n\n- Airway\n- Breath'; + const cut = settledMarkdownLength(text, 0); + assert.equal(text.slice(0, cut).trim(), 'Assess in this order:'); +}); + +test('an unbroken paragraph settles nothing, which is the old behaviour', (t) => { + const { settledMarkdownLength } = ui(t); + const text = 'One very long sentence that has not finished yet and contains no blank line'; + assert.equal(settledMarkdownLength(text, 0), 0); +}); + +test('the search resumes from what was already consumed', (t) => { + const { settledMarkdownLength } = ui(t); + // A boundary already passed must never be returned again, or the same text + // would be appended twice. + const text = 'A.\n\nB.\n\nC still'; + const first = settledMarkdownLength(text, 0); + assert.ok(settledMarkdownLength(text, first) >= first); + assert.equal(settledMarkdownLength('A.\n\nB still', 4), 4); +}); + +// ---- what lands in the bubble ---------------------------------------------- + +test('the settled part is real markdown while the tail is still plain text', (t) => { + const ctx = ui(t); + const bubble = bubbleFor(ctx); + ctx.renderStreamingInto(bubble, 'Doses:\n\n' + TABLE + '\n\n| Half | tab', []); + const settled = bubble.querySelector('.assistant-stream-settled'); + assert.ok(settled.querySelector('table'), 'the finished table should be a table'); + assert.equal(settled.querySelectorAll('table tbody tr').length, 2); + const tail = bubble.querySelector('.assistant-streaming-text'); + assert.equal(tail.textContent.trim(), '| Half | tab'); + assert.equal(tail.querySelector('table'), null, 'the unfinished row must not be parsed'); +}); + +test('no raw-text
 dump, however long or table-heavy the answer', (t) => {
+  const ctx = ui(t);
+  const bubble = bubbleFor(ctx);
+  // Both of the old bail-out conditions at once: over 3500 characters, and far
+  // more than eight pipe rows. This used to render as one black 
.
+  const long = 'Intro.\n\n' + (TABLE + '\n\n').repeat(60) + 'Trailing sen';
+  assert.ok(long.length > 3500);
+  ctx.renderStreamingInto(bubble, long, []);
+  assert.ok(bubble.querySelectorAll('table').length >= 20);
+  assert.equal(bubble.querySelector('pre'), null, 'nothing should fall back to a 
 dump');
+  assert.equal(bubble.querySelector('.assistant-streaming-text').textContent, 'Trailing sen');
+});
+
+test('settled content is appended, never re-rendered', (t) => {
+  const ctx = ui(t);
+  const bubble = bubbleFor(ctx);
+  ctx.renderStreamingInto(bubble, 'First.\n\nSecond stil', []);
+  const settled = bubble.querySelector('.assistant-stream-settled');
+  const firstNode = settled.firstChild;
+  ctx.renderStreamingInto(bubble, 'First.\n\nSecond.\n\nThird stil', []);
+  assert.equal(settled.firstChild, firstNode, 'the first block should be the same node, untouched');
+  assert.equal(settled.children.length, 2);
+  assert.equal(bubble.querySelector('.assistant-streaming-text').textContent, 'Third stil');
+});
+
+test('a diagram that has drawn is not torn down by the next block', async (t) => {
+  const ctx = ui(t);
+  const bubble = bubbleFor(ctx);
+  let renders = 0;
+  ctx.window.mermaid = {
+    initialize() {},
+    render(id, src) { renders++; return Promise.resolve({ svg: '' }); }
+  };
+  ctx.renderStreamingInto(bubble, '```mermaid\ngraph TD; A-->B;\n```\n\nNext par', []);
+  const diagram = bubble.querySelector('[data-mermaid]');
+  assert.ok(diagram, 'the diagram placeholder should be in the settled part');
+  // Several more blocks arrive, each re-running renderEmbeddedBlocks.
+  ctx.renderStreamingInto(bubble, '```mermaid\ngraph TD; A-->B;\n```\n\nNext paragraph.\n\nAnd another.\n\nStill go', []);
+  await new Promise(resolve => setImmediate(resolve)); // ensureMermaid resolves on a microtask
+  assert.equal(bubble.querySelector('[data-mermaid]'), diagram, 'same element');
+  assert.equal(diagram.getAttribute('data-drawn'), '1');
+  assert.equal(renders, 1, 'mermaid.render should be called once per diagram, not once per block');
+});
+
+test('a chart canvas is constructed once — a second Chart on it throws', (t) => {
+  const ctx = ui(t);
+  const bubble = bubbleFor(ctx);
+  const built = [];
+  ctx.window.Chart = function (c, cfg) { built.push(cfg); };
+  const chart = '```chart\n{"type":"bar","data":{"labels":["a"],"datasets":[{"data":[1]}]}}\n```';
+  ctx.renderStreamingInto(bubble, chart + '\n\nA para', []);
+  ctx.renderStreamingInto(bubble, chart + '\n\nA paragraph.\n\nAnother.\n\nStill go', []);
+  if (bubble.querySelector('canvas[data-chart]')) assert.equal(built.length, 1);
+});
+
+test('an empty stream shows the placeholder and holds no stale state', (t) => {
+  const ctx = ui(t);
+  const bubble = bubbleFor(ctx);
+  ctx.renderStreamingInto(bubble, '', []);
+  assert.match(bubble.innerHTML, /Generating answer/);
+  assert.equal(bubble.assistantStreamState, null);
+});
+
+test('the final render drops the streaming state, so no frame appends into a detached node', (t) => {
+  const ctx = ui(t);
+  const bubble = bubbleFor(ctx);
+  ctx.renderStreamingInto(bubble, 'First.\n\nSecond stil', []);
+  assert.ok(bubble.assistantStreamState);
+  ctx.fillMessageBubble(bubble, 'assistant', 'First.\n\nSecond, finished.', [], null, false);
+  assert.equal(bubble.assistantStreamState, null);
+  assert.equal(bubble.querySelector('.assistant-stream-settled'), null);
+});
+
+test('the tail is styled as prose, not left to the browser monospace default', (t) => {
+  const css = read('public/css/assistant.css');
+  assert.match(css, /\.assistant-streaming-text \{[^}]*white-space:pre-wrap/);
+  assert.match(css, /\.assistant-streaming-text \{[^}]*color:var\(--g700\)/);
+});