diff --git a/src/components/ConversationCard/answer-buffer.mjs b/src/components/ConversationCard/answer-buffer.mjs new file mode 100644 index 000000000..13f8341f8 --- /dev/null +++ b/src/components/ConversationCard/answer-buffer.mjs @@ -0,0 +1,71 @@ +/** + * Animation frames are the fastest way to coalesce a burst of updates, but not every host + * has them (jsdom and other headless renderers do not). Fall back to a timer there so the + * buffer can always be created. + * @param {typeof globalThis} [host] + * @returns {{requestFrame: (callback: () => void) => unknown, cancelFrame: (handle: unknown) => void}} + */ +export function createFrameScheduler(host = globalThis) { + if ( + typeof host.requestAnimationFrame === 'function' && + typeof host.cancelAnimationFrame === 'function' + ) { + return { + requestFrame: (callback) => host.requestAnimationFrame(callback), + cancelFrame: (handle) => host.cancelAnimationFrame(handle), + } + } + return { + requestFrame: (callback) => setTimeout(callback, 16), + cancelFrame: (handle) => clearTimeout(handle), + } +} + +/** + * Coalesces streamed answer text so a burst of chunks renders once per frame instead of + * once per chunk, while never losing the newest text. + * @param {object} params + * @param {(callback: () => void) => unknown} params.requestFrame + * @param {(handle: unknown) => void} params.cancelFrame + * @param {(answer: string) => void} params.render + */ +export function createAnswerBuffer({ requestFrame, cancelFrame, render }) { + let pending = null + let frame = null + + const cancelPendingFrame = () => { + if (frame === null) return + cancelFrame(frame) + frame = null + } + + const takePending = () => { + const answer = pending + pending = null + return answer + } + + return { + /** Queue the newest answer, scheduling a render only when none is already scheduled. */ + push(answer) { + pending = answer + if (frame !== null) return + frame = requestFrame(() => { + frame = null + const latest = takePending() + if (latest !== null) render(latest) + }) + }, + /** Render the newest answer before the conversation is finalized. */ + flush() { + cancelPendingFrame() + const latest = takePending() + if (latest !== null) render(latest) + }, + /** Drop the newest answer without rendering it, e.g. when a retry starts. */ + discard() { + cancelPendingFrame() + pending = null + }, + } +} diff --git a/src/components/ConversationCard/index.jsx b/src/components/ConversationCard/index.jsx index dbba60187..f3476d756 100644 --- a/src/components/ConversationCard/index.jsx +++ b/src/components/ConversationCard/index.jsx @@ -50,6 +50,7 @@ import { isSupersededGenerationMessage, isSupersededRequestMessage, } from './session.mjs' +import { createAnswerBuffer, createFrameScheduler } from './answer-buffer.mjs' const logo = Browser.runtime.getURL('logo.png') const UNMATCHED_API_MODE_VALUE = '__current-session-api-mode__' @@ -187,6 +188,7 @@ function ConversationCard(props) { const newSession = initSession({ ...session, question: props.question }) partialAnswerRef.current = '' retryRecordRef.current = null + answerBufferRef.current.discard() setSession(newSession) await postMessage({ session: newSession }) } @@ -221,6 +223,19 @@ function ConversationCard(props) { }) } + const answerBufferRef = useRef(null) + if (answerBufferRef.current === null) { + answerBufferRef.current = createAnswerBuffer({ + ...createFrameScheduler(), + render: (answer) => updateAnswer(answer, false, 'answer'), + }) + } + + // A buffered frame can outlive a hidden page, so drop it when the card goes away. + useEffect(() => { + return () => answerBufferRef.current?.discard() + }, []) + const portMessageListener = (msg) => { if (disposedRef.current) return if (isSupersededRequestMessage(msg, requestGenerationIdRef.current)) return @@ -228,12 +243,13 @@ function ConversationCard(props) { if (msg.answer) { partialAnswerRef.current = msg.answer - updateAnswer(msg.answer, false, 'answer') + answerBufferRef.current.push(msg.answer) } if (msg.session) { setSession(msg.done ? { ...msg.session, isRetry: false } : msg.session) } if (msg.done) { + answerBufferRef.current.flush() const partialAnswer = partialAnswerRef.current const retryRecord = retryRecordRef.current const completionState = getInterruptedCompletionState(msg, partialAnswer, retryRecord) @@ -249,6 +265,7 @@ function ConversationCard(props) { setIsReady(true) } if (msg.error) { + answerBufferRef.current.flush() const retryRecord = retryRecordRef.current setSession((currentSession) => finalizeInterruptedSession(currentSession, '', retryRecord)) switch (msg.error) { @@ -431,10 +448,17 @@ function ConversationCard(props) { return } if (disposedRef.current) return + // A dropped transport ends the stream without a final message, so flush here: the newest + // chunk still renders on a hidden page, where animation frames are paused. A foreground + // generation (Bing web) streams through its own transport, though, so this keepalive + // port dropping must not unlock sending. + if (foregroundPortsRef.current.size === 0) { + answerBufferRef.current.flush() + setIsReady(true) + } const nextPort = Browser.runtime.connect() portRef.current = nextPort setPort(nextPort) - setIsReady(true) } const closeChatsMessageListener = (message) => { @@ -479,6 +503,7 @@ function ConversationCard(props) { }, [port, conversationItemData]) const getRetryFn = (session) => async () => { + answerBufferRef.current.discard() updateAnswer(`

${t('Waiting for response...')}

`, false, 'answer') setIsReady(false) @@ -671,6 +696,7 @@ function ConversationCard(props) { } partialAnswerRef.current = '' retryRecordRef.current = null + answerBufferRef.current.discard() Browser.runtime.sendMessage({ type: 'DELETE_CONVERSATION', data: { @@ -797,6 +823,7 @@ function ConversationCard(props) { ) partialAnswerRef.current = '' retryRecordRef.current = null + answerBufferRef.current.discard() setConversationItemData([...conversationItemData, newQuestion, newAnswer]) setIsReady(false) diff --git a/src/components/MarkdownRender/highlight-options.mjs b/src/components/MarkdownRender/highlight-options.mjs new file mode 100644 index 000000000..bedb598fb --- /dev/null +++ b/src/components/MarkdownRender/highlight-options.mjs @@ -0,0 +1,52 @@ +/** + * Shared rehype-highlight options. + * + * Auto-detection compiles every registered grammar the first time it runs, which costs + * >100ms on a cold page. Scanning a curated subset keeps that near a few milliseconds. + * Explicitly labelled code blocks are unaffected: the subset only narrows automatic + * detection, they still use every grammar lowlight has registered. + * + * The subset is lowlight's registered set minus entries that only mislead detection: + * `shell` (the Shell Session console-prompt grammar, not a `bash` alias; `bash` aliases + * only `sh`/`zsh`), `plaintext`/`python-repl`/`php-template` (not useful for detection), + * and `arduino`/`objectivec`/`vbnet`/`wasm` (rare in answers, and they win ambiguous `c`, + * `cpp`, `ini` and `sql` matches away from the right language). Keep the + * order as lowlight registers them: `highlightAuto` settles equal-relevance candidates by + * subset order, so reordering this list changes which language an ambiguous block detects as. + */ +export const highlightOptions = { + detect: true, + subset: [ + 'bash', + 'c', + 'cpp', + 'csharp', + 'css', + 'diff', + 'go', + 'graphql', + 'ini', + 'java', + 'javascript', + 'json', + 'kotlin', + 'less', + 'lua', + 'makefile', + 'markdown', + 'perl', + 'php', + 'python', + 'r', + 'ruby', + 'rust', + 'scss', + 'sql', + 'swift', + 'typescript', + 'xml', + 'yaml', + ], + ignoreMissing: true, + plainText: ['diagnostic'], +} diff --git a/src/components/MarkdownRender/markdown-without-katex.jsx b/src/components/MarkdownRender/markdown-without-katex.jsx index 0733b7129..498bcc673 100644 --- a/src/components/MarkdownRender/markdown-without-katex.jsx +++ b/src/components/MarkdownRender/markdown-without-katex.jsx @@ -5,6 +5,7 @@ import remarkGfm from 'remark-gfm' import remarkBreaks from 'remark-breaks' import { Pre } from './Pre' import { Hyperlink } from './Hyperlink' +import { highlightOptions } from './highlight-options.mjs' import { memo, useState } from 'react' import { useTranslation } from 'react-i18next' @@ -176,17 +177,7 @@ export function MarkdownRender(props) { ]} unwrapDisallowed={true} remarkPlugins={[remarkGfm, remarkBreaks]} - rehypePlugins={[ - rehypeRaw, - [ - rehypeHighlight, - { - detect: true, - ignoreMissing: true, - plainText: ['diagnostic'], - }, - ], - ]} + rehypePlugins={[rehypeRaw, [rehypeHighlight, highlightOptions]]} components={{ a: Hyperlink, pre: Pre, diff --git a/src/components/MarkdownRender/markdown.jsx b/src/components/MarkdownRender/markdown.jsx index f9cb8eb3a..8f92ac47b 100644 --- a/src/components/MarkdownRender/markdown.jsx +++ b/src/components/MarkdownRender/markdown.jsx @@ -8,6 +8,7 @@ import remarkGfm from 'remark-gfm' import remarkBreaks from 'remark-breaks' import { Pre } from './Pre' import { Hyperlink } from './Hyperlink' +import { highlightOptions } from './highlight-options.mjs' import { memo, useState } from 'react' import { useTranslation } from 'react-i18next' @@ -179,18 +180,7 @@ export function MarkdownRender(props) { ]} unwrapDisallowed={true} remarkPlugins={[remarkMath, remarkGfm, remarkBreaks]} - rehypePlugins={[ - rehypeKatex, - rehypeRaw, - [ - rehypeHighlight, - { - detect: true, - ignoreMissing: true, - plainText: ['diagnostic'], - }, - ], - ]} + rehypePlugins={[rehypeKatex, rehypeRaw, [rehypeHighlight, highlightOptions]]} components={{ a: Hyperlink, pre: Pre, diff --git a/tests/setup/conversation-card-lifecycle-loader-hooks.mjs b/tests/setup/conversation-card-lifecycle-loader-hooks.mjs index 2cd6889c0..f6c36fa7e 100644 --- a/tests/setup/conversation-card-lifecycle-loader-hooks.mjs +++ b/tests/setup/conversation-card-lifecycle-loader-hooks.mjs @@ -40,7 +40,14 @@ const sources = { return null } `, - 'test:conversation-item': 'export default function ConversationItem() { return null }', + 'test:conversation-item': ` + export default function ConversationItem(props) { + if (props.type === 'answer') { + globalThis.__CONVERSATION_LIFECYCLE_TEST__.answerContents.push(props.content) + } + return null + } + `, 'test:conversation-utils': ` export const apiModeToModelName = () => 'test-model' export const createElementAtPosition = () => document.createElement('div') diff --git a/tests/unit/components/answer-buffer.test.mjs b/tests/unit/components/answer-buffer.test.mjs new file mode 100644 index 000000000..2cad9e5f8 --- /dev/null +++ b/tests/unit/components/answer-buffer.test.mjs @@ -0,0 +1,150 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { + createAnswerBuffer, + createFrameScheduler, +} from '../../../src/components/ConversationCard/answer-buffer.mjs' + +function createFakeFrames() { + let nextHandle = 1 + const callbacks = new Map() + const cancelled = [] + + return { + cancelled, + requestFrame(callback) { + const handle = nextHandle++ + callbacks.set(handle, callback) + return handle + }, + cancelFrame(handle) { + cancelled.push(handle) + callbacks.delete(handle) + }, + runFrames() { + const pending = [...callbacks.values()] + callbacks.clear() + for (const callback of pending) callback() + }, + } +} + +function setup() { + const frames = createFakeFrames() + const renders = [] + const buffer = createAnswerBuffer({ + requestFrame: frames.requestFrame, + cancelFrame: frames.cancelFrame, + render: (answer) => renders.push(answer), + }) + return { frames, renders, buffer } +} + +test('a burst of chunks renders once per frame with only the newest text', () => { + const { frames, renders, buffer } = setup() + + buffer.push('a') + buffer.push('ab') + buffer.push('abc') + assert.deepEqual(renders, []) + + frames.runFrames() + + assert.deepEqual(renders, ['abc']) +}) + +test('a chunk arriving after a frame schedules the next render', () => { + const { frames, renders, buffer } = setup() + + buffer.push('a') + frames.runFrames() + buffer.push('ab') + frames.runFrames() + + assert.deepEqual(renders, ['a', 'ab']) +}) + +test('flush renders the newest chunk so completion cannot drop the last one', () => { + const { frames, renders, buffer } = setup() + + buffer.push('a') + buffer.push('ab') + buffer.flush() + assert.deepEqual(renders, ['ab']) + assert.equal(frames.cancelled.length, 1) + + frames.runFrames() + assert.deepEqual(renders, ['ab'], 'the cancelled frame must not render again') +}) + +test('flush without pending text does not render', () => { + const { renders, buffer } = setup() + + buffer.flush() + + assert.deepEqual(renders, []) +}) + +test('discard drops the pending text without rendering it', () => { + const { frames, renders, buffer } = setup() + + buffer.push('a') + buffer.discard() + frames.runFrames() + + assert.deepEqual(renders, []) +}) + +test('pushing after a discard schedules a fresh frame', () => { + const { frames, renders, buffer } = setup() + + buffer.push('a') + buffer.discard() + buffer.push('b') + frames.runFrames() + + assert.deepEqual(renders, ['b']) +}) + +test('createFrameScheduler uses the host animation frames when available', () => { + const requested = [] + const cancelled = [] + const scheduler = createFrameScheduler({ + requestAnimationFrame: (callback) => { + requested.push(callback) + return 7 + }, + cancelAnimationFrame: (handle) => cancelled.push(handle), + }) + + const handle = scheduler.requestFrame(() => {}) + scheduler.cancelFrame(handle) + + assert.equal(requested.length, 1) + assert.deepEqual(cancelled, [7]) +}) + +test('createFrameScheduler falls back to a timer without animation frames', async () => { + const scheduler = createFrameScheduler({}) + const renders = [] + + await new Promise((resolve) => { + scheduler.requestFrame(() => { + renders.push('tick') + resolve() + }) + }) + + assert.deepEqual(renders, ['tick']) +}) + +test('the timer fallback cancels a pending frame', async () => { + const scheduler = createFrameScheduler({}) + const renders = [] + const handle = scheduler.requestFrame(() => renders.push('tick')) + + scheduler.cancelFrame(handle) + await new Promise((resolve) => setTimeout(resolve, 32)) + + assert.deepEqual(renders, []) +}) diff --git a/tests/unit/components/conversation-card-lifecycle.test.mjs b/tests/unit/components/conversation-card-lifecycle.test.mjs index fb5d37e39..e91dd8322 100644 --- a/tests/unit/components/conversation-card-lifecycle.test.mjs +++ b/tests/unit/components/conversation-card-lifecycle.test.mjs @@ -126,6 +126,7 @@ const resetState = () => { state.generateAnswersCount += 1 } state.runtimeOnMessage.clear() + state.answerContents = [] } const mountCard = (container, props = {}) => { @@ -237,6 +238,34 @@ test('remote runtime Port disconnect reconnects and unmount cleans the replaceme assert.equal(state.ports.length, 2) }) +test('a remote Port disconnect during a foreground generation does not unlock sending', async () => { + const state = globalThis.__CONVERSATION_LIFECYCLE_TEST__ + const container = document.createElement('div') + document.body.append(container) + state.foreground = true + state.generateAnswers = () => { + state.generateAnswersCount += 1 + return new Promise(() => {}) + } + + mountCard(container) + await waitFor( + () => typeof state.inputBoxProps?.onSubmit === 'function', + 'InputBox did not render', + ) + + state.inputBoxProps.onSubmit('question') + await waitFor(() => state.generateAnswersCount === 1, 'foreground provider did not start') + await waitFor(() => state.inputBoxProps.enabled === false, 'sending was not locked') + + act(() => state.ports[0].emitRemoteDisconnect()) + + // The keepalive Port is replaced, but the foreground stream still owns the answer, so + // sending stays locked until that stream ends on its own. + assert.equal(state.ports.length, 2) + assert.equal(state.inputBoxProps.enabled, false) +}) + test('close button disposes foreground transport before onClose', async () => { const state = globalThis.__CONVERSATION_LIFECYCLE_TEST__ const container = document.createElement('div') @@ -730,3 +759,80 @@ test('provider failure disconnects fake Port and removes stale listeners', async act(() => render(null, container)) }) + +test('a burst of streamed chunks stays buffered until completion flushes it', () => { + const state = globalThis.__CONVERSATION_LIFECYCLE_TEST__ + const container = document.createElement('div') + document.body.append(container) + const session = { + ...baseSession(), + question: 'why?', + conversationRecords: [{ question: 'why?', answer: 'partial' }], + } + + mountCard(container, { question: 'why?', session }) + const port = state.ports[0] + + act(() => port.onMessage.trigger({ answer: 'a' })) + act(() => port.onMessage.trigger({ answer: 'ab' })) + assert.equal(state.answerContents.includes('ab'), false, 'the burst must not render per chunk') + + act(() => port.onMessage.trigger({ answer: 'abc', done: true, session })) + + assert.equal(state.answerContents.includes('abc'), true, 'completion must flush the newest chunk') +}) + +test('switching the question drops a buffered answer from the previous one', async () => { + const state = globalThis.__CONVERSATION_LIFECYCLE_TEST__ + const container = document.createElement('div') + document.body.append(container) + + mountCard(container, { question: 'first', session: { ...baseSession(), question: 'first' } }) + const port = state.ports[0] + + act(() => port.onMessage.trigger({ answer: 'stale answer' })) + + act(() => { + render( + h(ConversationCard, { + session: { ...baseSession(), question: 'second' }, + question: 'second', + }), + container, + ) + }) + + await new Promise((resolve) => setTimeout(resolve, 32)) + + assert.equal(state.answerContents.includes('stale answer'), false) +}) + +test('a dropped transport flushes the buffered answer before reconnecting', () => { + const state = globalThis.__CONVERSATION_LIFECYCLE_TEST__ + const container = document.createElement('div') + document.body.append(container) + const session = { + ...baseSession(), + question: 'why?', + conversationRecords: [{ question: 'why?', answer: 'partial' }], + } + + mountCard(container, { question: 'why?', session }) + const port = state.ports[0] + + act(() => port.onMessage.trigger({ answer: 'newest chunk' })) + assert.equal( + state.answerContents.includes('newest chunk'), + false, + 'the chunk must be buffered first', + ) + + act(() => port.emitRemoteDisconnect()) + + assert.equal(state.ports.length, 2) + assert.equal( + state.answerContents.includes('newest chunk'), + true, + 'a dropped transport must flush the newest chunk synchronously', + ) +}) diff --git a/tests/unit/components/highlight-options.test.mjs b/tests/unit/components/highlight-options.test.mjs new file mode 100644 index 000000000..fca17cec9 --- /dev/null +++ b/tests/unit/components/highlight-options.test.mjs @@ -0,0 +1,80 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { unified } from 'unified' +import remarkParse from 'remark-parse' +import remarkRehype from 'remark-rehype' +import rehypeHighlight from 'rehype-highlight' +import { highlightOptions } from '../../../src/components/MarkdownRender/highlight-options.mjs' + +async function renderCodeNodes(markdown) { + const processor = unified() + .use(remarkParse) + .use(remarkRehype, { allowDangerousHtml: true }) + .use(rehypeHighlight, highlightOptions) + + const tree = await processor.run(processor.parse(markdown)) + const nodes = [] + const walk = (node) => { + if (node.type === 'element' && node.tagName === 'code') nodes.push(node) + for (const child of node.children ?? []) walk(child) + } + walk(tree) + return nodes +} + +function classNames(node) { + return node.properties?.className ?? [] +} + +test('auto-detection still labels blocks from the configured subset', async () => { + const [code] = await renderCodeNodes('```\nconst value = computeSomething(alpha, beta)\n```\n') + + const detected = classNames(code).find((name) => name.startsWith('language-')) + assert.ok( + detected && highlightOptions.subset.includes(detected.replace('language-', '')), + `expected a detection within the subset, got ${classNames(code).join(', ')}`, + ) + // A failure inside highlightAuto is swallowed by ignoreMissing, so token spans are the + // evidence that the whole subset resolved. + assert.ok( + code.children.some((child) => classNames(child).some((name) => name.startsWith('hljs-'))), + 'expected token spans from auto-detection', + ) +}) + +test('the subset only narrows detection, not labelled blocks', async () => { + // `objectivec` is registered by lowlight but deliberately left out of the detection subset. + assert.equal(highlightOptions.subset.includes('objectivec'), false) + + const [code] = await renderCodeNodes('```objectivec\nNSString *value = @"hi";\n```\n') + + assert.ok( + code.children.some((child) => classNames(child).some((name) => name.startsWith('hljs-'))), + 'expected token spans for a labelled language outside the detection subset', + ) +}) + +test('auto-detection keeps the common unlabelled languages', async () => { + const cases = [ + ['diff', 'diff --git a/x b/x\n@@ -1,3 +1,3 @@\n-old line\n+new line'], + ['markdown', '# Title\n\nSome *emphasis* and a [link](http://example.com).\n\n- item'], + ['lua', 'local function add(a, b)\n return a + b\nend\nprint(add(1, 2))'], + ] + + for (const [language, snippet] of cases) { + const [code] = await renderCodeNodes(`\`\`\`\n${snippet}\n\`\`\`\n`) + const detected = classNames(code).find((name) => name.startsWith('language-')) + assert.equal( + detected, + `language-${language}`, + `expected ${language} to stay auto-detectable, got ${classNames(code).join(', ')}`, + ) + } +}) + +test('an unknown language label is ignored instead of failing the render', async () => { + const [code] = await renderCodeNodes('```not-a-language\nconst value = 1\n```\n') + + assert.equal(code.children.length, 1) + assert.equal(code.children[0].type, 'text') +})