From f3c3f47d995fd45aab4ffbc017616d52a967b333 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 11 Sep 2026 21:10:50 +0200 Subject: [PATCH] fix: voice mode reads the answer that just arrived, and reads what the page shows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs, one cause each. It read the previous answer. setBusy(false) is what announces assistant-answer-done, and it ran before lastAnswer was assigned — so every listener was handed the answer before last. It now fires after the answer exists both in that variable and on the page. A test asserts the order, because the order is the whole bug. And it read the markdown. The better answer than unpicking the markup is not to have any: the rendered bubble is already the answer with its headings, emphasis and tables resolved, so voice mode reads that. It cannot drift from what the reader is looking at, and it needs no rules about what "##" sounds like. Read from a clone, with the parts that are not the answer removed — the action buttons, the sources list, the follow-up suggestions, code blocks and tables — so the page itself is untouched. A bubble still thinking is never read. speakableText() stays as the fallback for when the bubble cannot be found, since raw markdown read aloud is worse than silence. Separately: e2e/seed.js hashed with bcrypt directly, so seeded accounts did not exercise the argon2id path production writes. It uses the app's own hasher now. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- e2e/seed.js | 6 ++++-- public/js/clinicalAssistant.js | 33 +++++++++++++++++++++++++++++-- test/assistant-voice-mode.test.js | 28 +++++++++++++++++++++++++- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/e2e/seed.js b/e2e/seed.js index a3f5a980..4a6752b9 100644 --- a/e2e/seed.js +++ b/e2e/seed.js @@ -34,7 +34,9 @@ require('fs').readFileSync('/proc/1/environ', 'utf8').split('\0').forEach(functi }); var db = require('../src/db/database'); -var bcrypt = require('bcryptjs'); +// The app's own hasher, not bcrypt directly: production writes argon2id, and a +// seeded account hashed any other way exercises a path real users do not take. +var passwords = require('../src/utils/passwords'); var TEST_DOMAIN = '@ped-ai.test'; var PASSWORD = process.env.E2E_TEST_PASSWORD || 'E2E-testPassword123!'; @@ -49,7 +51,7 @@ async function seed(account) { if (email.slice(-TEST_DOMAIN.length) !== TEST_DOMAIN) { throw new Error('refusing to seed ' + email + ': only ' + TEST_DOMAIN + ' addresses may be seeded'); } - var hash = await bcrypt.hash(PASSWORD, 12); + var hash = await passwords.hash(PASSWORD); var existing = await db.get('SELECT id, role, email_verified, disabled FROM users WHERE email = ?', [email]); if (!existing) { await db.run( diff --git a/public/js/clinicalAssistant.js b/public/js/clinicalAssistant.js index 3c3721d6..1db5b848 100644 --- a/public/js/clinicalAssistant.js +++ b/public/js/clinicalAssistant.js @@ -534,12 +534,16 @@ import { if (request && request.cancelled) return; - setBusy(false, 'Ready'); lastAnswer = finalData.answer || finalData.markdown || ''; lastSources = finalData.sources || finalData.citations || streamSources; replaceLoadingMessage(loading, lastAnswer, lastSources, finalData.suggestions || []); attachImageJobs(loading, messages[messages.length - 1], finalData.imageJobs || []); renderSources(lastSources); + // Last, not first. setBusy(false) announces assistant-answer-done, and + // firing it before lastAnswer was assigned meant every listener — voice + // mode among them — was handed the *previous* answer. It now fires once the + // answer exists both in that variable and on the page. + setBusy(false, 'Ready'); } function renderStreamingAnswerHtml(text, sources) { @@ -1260,7 +1264,32 @@ import { var detail = (e && e.detail) || {}; if (detail.isError) { if (status) status.textContent = 'Answer failed — tap to speak'; return; } if (status) status.textContent = 'Speaking…'; - speakAnswerVoice(String(detail.answer || '')); + speakAnswerVoice(renderedAnswerText() || String(detail.answer || '')); + } + + // What the page shows, which is already the answer with its markup rendered + // away — headings are headings, bold is bold, a table is a table. Reading + // that is both simpler and more faithful than unpicking the markdown, and it + // cannot drift from what the reader is looking at. + // + // The markdown remains the fallback for the case where the bubble is not + // found, and is reduced first, because raw markdown read aloud is worse than + // silence. + function renderedAnswerText() { + var wrap = document.getElementById('assistant-messages'); + if (!wrap) return ''; + var rows = wrap.querySelectorAll('.assistant-msg.assistant'); + var last = rows[rows.length - 1]; + if (!last || last.classList.contains('assistant-loading-msg')) return ''; + var bubble = last.querySelector('.assistant-bubble'); + if (!bubble) return ''; + // A clone, so removing the parts that are not the answer does not touch the + // page: the action buttons, the sources list and the follow-up suggestions + // are all inside the bubble and none of them is worth reading out. + var copy = bubble.cloneNode(true); + copy.querySelectorAll('.assistant-msg-actions, .assistant-sources, .assistant-suggestions, button, pre, table') + .forEach(function (node) { node.remove(); }); + return String(copy.innerText || copy.textContent || '').replace(/\n{2,}/g, '\n').trim(); } // What a person would read out, not what the model wrote. Passed raw, a diff --git a/test/assistant-voice-mode.test.js b/test/assistant-voice-mode.test.js index 9ec62fd2..bcfd48e6 100644 --- a/test/assistant-voice-mode.test.js +++ b/test/assistant-voice-mode.test.js @@ -40,7 +40,33 @@ test('ending the call stops the voice', () => { assert.match(src, /conversationMode\.speaking = audio;/); }); -test('the answer is read, not its markup', () => { +test('the answer is read only once it exists', () => { + // setBusy(false) announces assistant-answer-done, and it used to run before + // lastAnswer was assigned — so every listener, voice mode included, was handed + // the previous answer. Tested by position, because that is the whole bug. + const body = src.slice(src.indexOf('if (request && request.cancelled) return;')); + const assigned = body.indexOf("lastAnswer = finalData.answer"); + const rendered = body.indexOf('replaceLoadingMessage(loading, lastAnswer'); + const announced = body.indexOf("setBusy(false, 'Ready')"); + assert.ok(assigned < announced, 'the answer is assigned before it is announced'); + assert.ok(rendered < announced, 'and rendered before it is announced'); +}); + +test('voice mode reads what the page shows', () => { + // The rendered bubble is the answer with its markup already resolved, so + // reading it is both simpler than unpicking markdown and impossible to drift + // from what the reader is looking at. + assert.match(src, /function renderedAnswerText\(\)/); + assert.match(src, /speakAnswerVoice\(renderedAnswerText\(\) \|\| String\(detail\.answer \|\| ''\)\)/); + // Never the bubble that is still thinking. + assert.match(src, /last\.classList\.contains\('assistant-loading-msg'\)\) return '';/); + // A clone: the things that are not the answer are removed from a copy, not + // from the page. + assert.match(src, /var copy = bubble\.cloneNode\(true\);/); + assert.match(src, /assistant-msg-actions, \.assistant-sources, \.assistant-suggestions, button, pre, table/); +}); + +test('the markdown fallback is still reduced before it is spoken', () => { // The raw answer went straight to the speaker. It is markdown, and a browser // voice reads "#" and "**" aloud or stumbles over them. assert.match(src, /var text = speakableText\(markdown\);/);