From 30bb30e25c3d915451d6d7ba372a5544d3fbcf40 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 28 Oct 2026 12:56:11 -0700 Subject: [PATCH] Long chat replies stream with less stutter: parse each block once (#34821) * Reduce stuttering while long chat replies stream * Keep streaming markdown pipeline within lint limits * Type the streaming parser meter without a cast * Type the streamed markdown test reveal state * Preserve markdown render domains when configured syntax cannot reuse discovery --- .../native-chat/NativeChatMarkdown.tsx | 3 + .../NativeChatMarkdown.visual.test.tsx | 20 +- .../native-chat-visual-markdown-extension.tsx | 2 + .../CommentMarkdown.parse-reuse.test.tsx | 226 ++++++++ .../CommentMarkdown.parse-validity.test.tsx | 483 ++++++++++++++++++ .../components/sidebar/CommentMarkdown.tsx | 70 +-- .../comment-markdown-block-pipeline.tsx | 103 ++++ .../markdown-parse-meter.test-fixture.ts | 51 ++ ...wn-top-level-blocks-legacy.test-fixture.ts | 103 ++++ .../sidebar/markdown-top-level-blocks.ts | 111 +++- 10 files changed, 1098 insertions(+), 74 deletions(-) create mode 100644 src/renderer/src/components/sidebar/CommentMarkdown.parse-reuse.test.tsx create mode 100644 src/renderer/src/components/sidebar/CommentMarkdown.parse-validity.test.tsx create mode 100644 src/renderer/src/components/sidebar/comment-markdown-block-pipeline.tsx create mode 100644 src/renderer/src/components/sidebar/markdown-parse-meter.test-fixture.ts create mode 100644 src/renderer/src/components/sidebar/markdown-top-level-blocks-legacy.test-fixture.ts diff --git a/src/renderer/src/components/native-chat/NativeChatMarkdown.tsx b/src/renderer/src/components/native-chat/NativeChatMarkdown.tsx index 96682e6cd69..0596b2d7c44 100644 --- a/src/renderer/src/components/native-chat/NativeChatMarkdown.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMarkdown.tsx @@ -54,6 +54,9 @@ function withFileDestinations( ): CommentMarkdownExtension { return { remarkPlugins: [...(visualExtension?.remarkPlugins ?? []), remarkNativeChatFileDestinations], + extraFlowSyntaxMarkers: visualExtension?.remarkPlugins.length + ? visualExtension.extraFlowSyntaxMarkers + : [], sanitizeAttributes: visualExtension?.sanitizeAttributes ?? {}, components: visualExtension?.components ?? {} } diff --git a/src/renderer/src/components/native-chat/NativeChatMarkdown.visual.test.tsx b/src/renderer/src/components/native-chat/NativeChatMarkdown.visual.test.tsx index 1e71b1963c7..8743f4efab4 100644 --- a/src/renderer/src/components/native-chat/NativeChatMarkdown.visual.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMarkdown.visual.test.tsx @@ -104,7 +104,12 @@ describe('NativeChatMarkdown visuals', () => { it('keeps a mounted visual frame while the rest of the reply streams in', async () => { const { container, rerender } = render( withOwner( - + ) ) await waitFor(() => expect(container.querySelector('iframe')).not.toBeNull()) @@ -112,8 +117,19 @@ describe('NativeChatMarkdown visuals', () => { rerender( withOwner( + ) + ) + rerender( + withOwner( + ) diff --git a/src/renderer/src/components/native-chat/native-chat-visual-markdown-extension.tsx b/src/renderer/src/components/native-chat/native-chat-visual-markdown-extension.tsx index ec9d677e577..da3b54ae96f 100644 --- a/src/renderer/src/components/native-chat/native-chat-visual-markdown-extension.tsx +++ b/src/renderer/src/components/native-chat/native-chat-visual-markdown-extension.tsx @@ -3,6 +3,7 @@ import type { ExtraProps } from 'react-markdown' import type { CommentMarkdownExtension } from '@/components/sidebar/CommentMarkdown' import { NATIVE_CHAT_VISUAL_TITLE_MAX_LENGTH, + NATIVE_CHAT_VISUAL_DIRECTIVE_MARKER, isNativeChatVisualFileName } from '../../../../shared/native-chat/native-chat-visual-directive' import { NativeChatInlineVisual } from './NativeChatInlineVisual' @@ -55,6 +56,7 @@ function createVisualExtension( } return { remarkPlugins: [[remarkNativeChatVisuals, nonce]], + extraFlowSyntaxMarkers: [NATIVE_CHAT_VISUAL_DIRECTIVE_MARKER], sanitizeAttributes: { div: [...NATIVE_CHAT_VISUAL_PLACEHOLDER_PROPERTIES] }, components: { div: VisualPlaceholder } } diff --git a/src/renderer/src/components/sidebar/CommentMarkdown.parse-reuse.test.tsx b/src/renderer/src/components/sidebar/CommentMarkdown.parse-reuse.test.tsx new file mode 100644 index 00000000000..90007223007 --- /dev/null +++ b/src/renderer/src/components/sidebar/CommentMarkdown.parse-reuse.test.tsx @@ -0,0 +1,226 @@ +// @vitest-environment happy-dom +import React from 'react' +import { cleanup, render, waitFor } from '@testing-library/react' +import { afterEach, beforeAll, expect, it, vi } from 'vitest' +import type { Processor } from 'unified' +import type remarkParsePlugin from 'remark-parse' +import type * as ReactMarkdownModule from 'react-markdown' +import type * as MarkdownBlocksModule from './markdown-top-level-blocks' +import CommentMarkdown from './CommentMarkdown' +import { + advanceNativeChatText, + receiveNativeChatText, + type NativeChatTextReveal +} from '../../../../shared/native-chat/transcript/native-chat-text-reveal' + +const meter = vi.hoisted((): { legacy: boolean; sources: string[] } => ({ + legacy: false, + sources: [] +})) +vi.mock('remark-parse', async (original) => { + const { default: plugin } = await original<{ default: typeof remarkParsePlugin }>() + return { + default: function (this: Processor) { + plugin.call(this) + const parse = this.parser! + this.parser = (source, file) => { + meter.sources.push(source) + return parse(source, file) + } + } + } +}) +vi.mock('react-markdown', async (original) => { + const real = await original() + function measureRenderParser(this: Processor): void { + const parse = this.parser! + this.parser = (source, file) => { + meter.sources.push(source) + return parse(source, file) + } + } + return { + ...real, + default: (props: React.ComponentProps) => + real.default({ + ...props, + remarkPlugins: [measureRenderParser, ...(props.remarkPlugins ?? [])] + }) + } +}) +vi.mock('./markdown-top-level-blocks', async (original) => { + const current = await original() + const legacy = await import('./markdown-top-level-blocks-legacy.test-fixture') + return { + ...current, + splitMarkdownTopLevelBlocks: ( + ...args: Parameters + ) => { + const result = meter.legacy + ? legacy.splitMarkdownTopLevelBlocks(args[0], args[1], args[2]) + : current.splitMarkdownTopLevelBlocks(...args) + return result + } + } +}) +vi.mock('./CommentMermaidBlock', () => ({ default: () => null })) + +beforeAll(async () => { + const warm = render() + await waitFor(() => expect(warm.container.querySelector('.katex')).not.toBeNull()) + warm.unmount() +}) +afterEach(() => { + cleanup() + meter.legacy = false + meter.sources = [] +}) + +// The production reveal cadence supplies exactly the same prefixes to both renderers. +function sequence(full: string, start: number): string[] { + let state: NativeChatTextReveal = { source: full.slice(0, start), shown: start, arrivals: [] } + let nextArrival = 0 + const result = [state.source] + for (let frame = 0; frame <= 84; frame++) { + const now = (frame * 1000) / 60 + while (nextArrival <= Math.min(now, 1008)) { + const end = Math.min( + full.length, + start + Math.ceil(((full.length - start) * (nextArrival + 48)) / 1056) + ) + state = receiveNativeChatText(state, full.slice(0, end), true, false, nextArrival) + nextArrival += 48 + } + state = advanceNativeChatText(state, now) + const text = state.source.slice(0, state.shown) + if (text !== result.at(-1)) { + result.push(text) + } + } + if (result.at(-1) !== full) { + result.push(full) + } + return result +} + +const prose = + 'The implementation reads **stable values** and keeps the same behavior for every user. '.repeat( + 650 + ) +const fence = `\`\`\`text\n${'const result = readCurrentValue(input);\n'.repeat(1400)}\`\`\`` +const list = Array.from( + { length: 650 }, + (_, i) => `- Item ${i}: preserve **current behavior** and report the measured result.\n` +).join('') +const code = ({ children, sourceLine }: { children?: React.ReactNode; sourceLine?: number }) => ( + 
{children}
+) +const exists = () => true + +it.each([ + ['short', prose.slice(0, 800), 0], + ['paragraph', prose.slice(0, 50000), 40000], + ['fence', fence, 40000], + ['list', list, 35000], + ['settled fence and tail', `${fence}\n\n${prose.slice(0, 800)}`, fence.length + 2], + [ + 'backward boundaries', + 'Title\n===\n\n- first\n\n- second\n\n continued\n\n| a | b |\n| - | - |\n| 1 | 2 |\n', + 0 + ], + [ + 'global constructs', + 'See [docs][ref].\n\nMore.[^1]\n\n[ref]: https://example.com\n\n[^1]: note\n\n
\n\n**hidden**\n\n
\n\ntext', + 0 + ], + [ + 'math and repair', + 'Before **bold** and [link](https://example.com).\n\n$x^2$\n\n$$\nx+y\n$$\n\n\\[z\\]\n\n```math\nx+1\n```\n\n~~~math\ny\n~~~\n\n$unfinished **tail', + 0 + ], + [ + 'positions and links', + '\n\n ```ts\nline one\nline two\n ```\n\n ~~~text\nother\n ~~~\n\nEdited src/a.ts and [file](file:///tmp/a.ts).\n\n中文**标点。**后续\n', + 0 + ], + [ + 'indented blocks', + 'Intro\n\n ```text\n value\n ```\n\n - one\n\n nested code\n\n > quote\n > ```text\n > value\n > ```\n\nnext', + 0 + ], + ['internal BOM', 'one\n\n\uFEFFnext\n\n\uFEFF```text\ncode\n```\n\ntail', 0], + [ + 'line endings', + 'one\r\n\r\n > quote\r\n\r\n```text\r\ncode\r\n```\r\n\r\nlast\uFEFF word\r\n', + 0 + ] +])( + 'preserves exact DOM and copy text at every reveal update: %s', + (_name, full, start) => { + const old = render() + const next = render() + const props = { + math: true, + variant: 'document', + fadeWords: true, + unknownHtmlAsText: true, + allowFileUriLinks: true, + fileLinkExists: exists, + renderCodeBlock: code + } as const + const prefixes = sequence(full, start) + for (const [index, content] of [...prefixes, full].entries()) { + const growing = index < prefixes.length + meter.legacy = true + old.rerender() + meter.legacy = false + next.rerender() + expect(next.container.innerHTML, `at ${content.length} characters`).toBe( + old.container.innerHTML + ) + expect(next.container.textContent).toBe(old.container.textContent) + } + }, + 55000 +) + +it('uses one actual parse for discovery and rendering, then skips a completed fence', () => { + const source = `${fence}\n\nNext` + meter.sources = [] + const view = render() + expect(meter.sources).toEqual([source]) + meter.sources = [] + view.rerender() + expect(meter.sources).toEqual(['Next words']) + view.unmount() + + const staticView = render() + meter.sources = [] + staticView.rerender() + expect(meter.sources).toEqual([]) + staticView.unmount() + + meter.legacy = true + meter.sources = [] + render() + expect(meter.sources.reduce((count, text) => count + text.length, 0)).toBeGreaterThan( + source.length + ) +}) + +it('preserves the current cut when the link policy changes', () => { + const full = + 'Intro\n\n ```text\n value\n ```\n\n - one\n\n nested code\n\n > quote\n ' + const snapshots: string[] = [] + for (const legacy of [true, false]) { + meter.legacy = legacy + const view = render() + for (const content of sequence(full, 0)) { + view.rerender() + } + view.rerender( false} />) + snapshots.push(view.container.innerHTML) + view.unmount() + } + expect(snapshots[1]).toBe(snapshots[0]) +}) diff --git a/src/renderer/src/components/sidebar/CommentMarkdown.parse-validity.test.tsx b/src/renderer/src/components/sidebar/CommentMarkdown.parse-validity.test.tsx new file mode 100644 index 00000000000..355f3bef667 --- /dev/null +++ b/src/renderer/src/components/sidebar/CommentMarkdown.parse-validity.test.tsx @@ -0,0 +1,483 @@ +// @vitest-environment happy-dom +import React from 'react' +import { act, cleanup, render } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import type { Processor } from 'unified' +import type remarkParsePlugin from 'remark-parse' +import type * as ReactMarkdownModule from 'react-markdown' +import type * as BlocksModule from './markdown-top-level-blocks' +import type { Root } from 'mdast' +import CommentMarkdown, { type CommentMarkdownExtension } from './CommentMarkdown' +import { createParseWork, meterPositions, nodeCount } from './markdown-parse-meter.test-fixture' +import { NativeChatMarkdown } from '../native-chat/NativeChatMarkdown' +import { NativeChatVisualOwnerContext } from '../native-chat/native-chat-visual-owner' +import { NATIVE_CHAT_VISUAL_DIRECTIVE_MARKER } from '../../../../shared/native-chat/native-chat-visual-directive' +import { remarkNativeChatVisuals } from '../native-chat/native-chat-visual-markdown-syntax' + +const meter = vi.hoisted( + (): { legacy: boolean; work: ReturnType | null } => ({ + legacy: false, + work: null + }) +) +vi.mock('remark-parse', async (original) => { + const { default: plugin } = await original<{ default: typeof remarkParsePlugin }>() + return { + default: function (this: Processor) { + plugin.call(this) + const parse = this.parser! + this.parser = (source, file) => { + meter.work?.discovery.push(source) + const tree = parse(source, file) + if (isRoot(tree)) { + meterPositions(tree, () => { + if (meter.work) { + meter.work.shifts++ + } + }) + } + return tree + } + } + } +}) +vi.mock('react-markdown', async (original) => { + const real = await original() + function measureOriginal(this: Processor): void { + const parse = this.parser! + this.parser = (source, file) => { + meter.work?.render.push(source) + return parse(source, file) + } + } + return { + ...real, + default: (props: React.ComponentProps) => + real.default({ + ...props, + remarkPlugins: [measureOriginal, ...(props.remarkPlugins ?? [])] + }) + } +}) +vi.mock('./markdown-top-level-blocks', async (original) => { + const current = await original() + const legacy = await import('./markdown-top-level-blocks-legacy.test-fixture') + return { + ...current, + splitMarkdownTopLevelBlocks: ( + ...args: Parameters + ) => { + const result = meter.legacy + ? legacy.splitMarkdownTopLevelBlocks(args[0], args[1], args[2]) + : current.splitMarkdownTopLevelBlocks(...args) + if (meter.work) { + meter.work.blocks = Math.max(meter.work.blocks, result.blocks.length) + } + return result + } + } +}) +vi.mock('../native-chat/NativeChatInlineVisual', () => ({ + NativeChatInlineVisual: ({ file }: { file: string }) =>
+})) +vi.mock('../native-chat/use-native-chat-file-link-existence', () => ({ + useNativeChatFileLinkExists: () => undefined +})) +vi.mock('./CommentMermaidBlock', () => ({ default: () => null })) + +afterEach(() => { + cleanup() + meter.legacy = false + meter.work = null + vi.restoreAllMocks() + vi.useRealTimers() +}) +function isRoot(value: unknown): value is Root { + return ( + typeof value === 'object' && + value !== null && + 'type' in value && + value.type === 'root' && + 'children' in value && + Array.isArray(value.children) + ) +} +const owner = { target: { kind: 'local' }, sessionId: 's', worktreeId: 'w' } as const +const code = ({ children, sourceLine }: { children?: React.ReactNode; sourceLine?: number }) => ( + 
{children}
+) +function native( + content: string, + growing = true, + enabled = true, + variant: 'document' | undefined = undefined +) { + return ( + + + + ) +} +const directive = (i = 0) => `::orca-visual{file="v${i}.html"}` +const prefix = '```text\nsettled\n```\n\nNext' + +it.each([ + [ + 'adjacent', + `${prefix}\n\n${Array.from({ length: 10 }, (_, i) => directive(i)).join('\n')}\n\nend`, + 8 + ], + ['separated', `${Array.from({ length: 10 }, (_, i) => directive(i)).join('\n\n')}\n\nend`, 10], + ['lazy list', `- a\n ${directive()}\nfoo\n\nbar`, 0], + ['lazy quote', `> a\n${directive()}\nfoo\n\nbar`, 1], + ['nested', `- item\n\n ${directive()}\n\n> ${directive(1)}\n\nend`, 0], + ['fenced marker', `~~~text\n${directive()}\n~~~\n\nend`, 0], + ['title CRLF', '::orca-visual{file="v0.html" title="Title"}\r\n\r\nend', 1], + [ + 'invalid escaped overlength', + `\\${directive()}\n\n::orca-visual{file="../bad.html"}\n\n::orca-visual{file="v.html" title="${'x'.repeat(520)}"}\n\nend`, + 0 + ], + [ + 'chart prose math CJK', + `${directive()}\n\n中文**标点。** $x+1$ and **tail\n\n[ref]: https://example.com`, + 1 + ], + [ + 'BOM and sliced context', + `one\n\n\uFEFFnext\n\n ~~~text\n value\n ~~~\n\n${directive()}\n\nend`, + 1 + ] +])( + 'retains exact native DOM/text at every filtered prefix and completion: %s', + (_name, full, mounts) => { + const old = render(native('')) + const next = render(native('')) + for (let end = 0; end <= full.length + 1; end++) { + const content = full.slice(0, end) + meter.legacy = true + old.rerender(native(content, end <= full.length)) + meter.legacy = false + next.rerender(native(content, end <= full.length)) + expect(next.container.innerHTML, `prefix ${end}`).toBe(old.container.innerHTML) + expect(next.container.textContent).toBe(old.container.textContent) + } + expect(next.container.querySelectorAll('[data-visual-file]')).toHaveLength(mounts) + if (_name === 'adjacent') { + expect(next.container.textContent).toContain(directive(9)) + } + }, + 55000 +) + +it('preserves historical cuts across config, replacement, completion, re-entry and remount', () => { + const snapshots: string[][] = [] + for (const legacy of [true, false]) { + meter.legacy = legacy + let view = render(native(prefix, true, false)) + const results: string[] = [] + const steps: [string, boolean, boolean, 'document' | undefined][] = [ + [prefix, true, true, undefined], + [prefix, true, false, undefined], + [prefix, true, true, 'document'], + [`${prefix}\n\n${directive()}\n\nMore`, true, true, 'document'], + [`${prefix}\n\n${directive()}\n\nMore`, false, true, 'document'], + [`${prefix}\n\n${directive()}\n\nMore words`, true, true, 'document'], + ['replacement', true, true, undefined], + [directive(), true, true, undefined], + [directive(), false, true, undefined], + [`${prefix}\n\n::orca-visual{file="v`, true, true, undefined], + [`${prefix}\n\n::orca-visual{file="v`, false, true, undefined], + ['', true, true, undefined], + ['one\n\n\uFEFFtail', true, true, undefined], + [prefix, true, true, undefined] + ] + for (const step of steps) { + view.rerender(native(...step)) + results.push(view.container.innerHTML) + } + view.unmount() + view = render(native(`${directive()}\n\nend`)) + results.push(view.container.innerHTML) + view.unmount() + view = render(native(`${directive()}\n\nend`, false)) + results.push(view.container.innerHTML) + view.unmount() + snapshots.push(results) + } + expect(snapshots[1]).toEqual(snapshots[0]) +}) + +it('defaults unknown ordinary-node syntax to legacy cuts, but permits image-only plugins', () => { + function extraSyntax(this: Processor): void { + const parse = this.parser! + this.parser = (source, file) => parse(source.replaceAll(' / ', '\n\n\n'), file) + } + const unknown: CommentMarkdownExtension = { + remarkPlugins: [extraSyntax], + sanitizeAttributes: {}, + components: {} + } + const outputs: string[] = [] + for (const legacy of [true, false]) { + meter.legacy = legacy + const view = render() + outputs.push(view.container.innerHTML) + view.unmount() + } + expect(outputs[1]).toBe(outputs[0]) + meter.legacy = false + meter.work = createParseWork() + render( + + ) + expect(meter.work.discovery).toEqual([prefix]) + expect(meter.work.render).toEqual([]) +}) + +it('keeps selected fade nodes and their clocks through a declined tail and completion', () => { + vi.useFakeTimers() + const extension: CommentMarkdownExtension = { + remarkPlugins: [[remarkNativeChatVisuals, 'n']], + extraFlowSyntaxMarkers: [NATIVE_CHAT_VISUAL_DIRECTIVE_MARKER], + sanitizeAttributes: {}, + components: {} + } + const view = render( + + ) + const fence = view.container.querySelector('pre') + const word = [...view.container.querySelectorAll('[data-word]')].find( + (node) => node.textContent === 'selected' + )! + const range = document.createRange() + range.selectNodeContents(word) + document.getSelection()?.addRange(range) + document.dispatchEvent(new Event('selectionchange')) + act(() => vi.advanceTimersByTime(150)) + const content = `${prefix}\n\nselected words\n\n${directive()}\n\nnew words` + view.rerender( + + ) + expect(view.container.querySelector('pre')).toBe(fence) + expect(word.isConnected).toBe(true) + act(() => vi.advanceTimersByTime(150)) + expect(word.isConnected).toBe(true) + expect( + [...view.container.querySelectorAll('[data-word]')].some((node) => node.textContent === 'new') + ).toBe(true) + view.rerender( + + ) + expect(document.getSelection()?.toString()).toBe('selected') + document.getSelection()?.removeAllRanges() + document.dispatchEvent(new Event('selectionchange')) + act(() => vi.advanceTimersByTime(1)) + expect(word.isConnected).toBe(false) +}) + +function installWorkMeters(): void { + const clone = globalThis.structuredClone + vi.spyOn(globalThis, 'structuredClone').mockImplementation((value: T): T => { + if (meter.work && isRoot(value)) { + meter.work.cloneNodes += nodeCount(value) + meter.work.cloneBytes += JSON.stringify(value).length + } + return clone(value) + }) + const includes = String.prototype.includes + vi.spyOn(String.prototype, 'includes').mockImplementation(function ( + this: string, + marker, + position + ) { + if (meter.work && (marker === '\uFEFF' || marker === NATIVE_CHAT_VISUAL_DIRECTIVE_MARKER)) { + meter.work.scans.push({ source: String(this), marker }) + } + return includes.call(this, marker, position) + }) +} + +it.each([20, 200])( + 'bounds actual parsing, clone, position and scan work at production updates N=%i', + (n) => { + installWorkMeters() + const fence = `\`\`\`text\n${'value\n'.repeat(n)}\`\`\`\n\n` + const tails = [ + 'Next', + ...Array.from({ length: n }, (_, i) => `Next${'x'.repeat(i + 1)}`), + `Next${'x'.repeat(n)}\n\n${directive()}\n\nend` + ] + const runs: ReturnType[][] = [] + for (const legacy of [true, false]) { + meter.legacy = legacy + meter.work = null + const view = render(native('', true, true, 'document')) + const updates: ReturnType[] = [] + for (const tail of tails) { + meter.work = createParseWork() + view.rerender(native(fence + tail, true, true, 'document')) + updates.push(meter.work) + } + view.unmount() + runs.push(updates) + } + const optimized = runs[1] + expect(optimized[0].discovery).toEqual([fence + tails[0]]) + for (let i = 1; i <= n; i++) { + const work = optimized[i] + expect(work.discovery).toEqual([tails[i]]) + expect(work.render).toEqual([]) + expect(work.shifts).toBe(4) + expect(work.cloneNodes).toBe(3) + expect(work.cloneBytes).toBeLessThan(tails[i].length + 400) + expect(work.blocks).toBe(2) + expect(work.scans).toEqual([ + { source: fence + tails[i], marker: '\uFEFF' }, + { source: tails[i], marker: NATIVE_CHAT_VISUAL_DIRECTIVE_MARKER } + ]) + } + const rejected = optimized.at(-1)! + expect(rejected.discovery).toEqual([fence + tails.at(-1)]) + expect(rejected.render.some((source) => source.includes('value\n'))).toBe(false) + expect(rejected.render.reduce((n, s) => n + s.length, 0)).toBeLessThanOrEqual( + runs[0].at(-1)!.render.reduce((n, s) => n + s.length, 0) + ) + expect(rejected.shifts).toBe(0) + expect(rejected.cloneNodes).toBe(0) + expect(rejected.blocks).toBe(4) + } +) + +it.each([20, 200])('keeps visual-adjacent fallback parse work within main at N=%i', (n) => { + const runs: ReturnType[][] = [] + const outputs: string[][] = [] + for (const legacy of [true, false]) { + meter.legacy = legacy + meter.work = null + const view = render(native('')) + const updates: ReturnType[] = [] + const html: string[] = [] + for (let i = 0; i <= n; i++) { + meter.work = createParseWork() + view.rerender(native(`${directive()}\n\n${'word '.repeat(i)}end`)) + updates.push(meter.work) + html.push(view.container.innerHTML) + } + view.unmount() + runs.push(updates) + outputs.push(html) + } + expect(outputs[1]).toEqual(outputs[0]) + for (let i = 0; i <= n; i++) { + expect(runs[1][i].discovery).toEqual(runs[0][i].discovery) + expect(runs[1][i].render).toEqual(runs[0][i].render) + expect(runs[1][i].shifts).toBe(0) + } +}) + +it('leaves old visual/fence elements mounted when reuse resumes beyond an earlier marker', () => { + const full = `${directive()}\n\n${prefix}` + const view = render(native(full)) + const visual = view.container.querySelector('[data-visual-file]') + const fence = view.container.querySelector('pre') + view.rerender(native(`${full} words`)) + meter.work = createParseWork() + view.rerender(native(`${full} words again`)) + expect(meter.work.discovery).toEqual(['Next words again']) + view.rerender(native(`${full} words again\n\n${directive(1)}\n\nend`)) + expect(view.container.querySelector('[data-visual-file]')).toBe(visual) + expect(view.container.querySelector('pre')).toBe(fence) +}) + +it('invalidates raw grammar metadata when an extension nonce changes on the same source', () => { + const makeExtension = (nonce: string): CommentMarkdownExtension => ({ + remarkPlugins: [[remarkNativeChatVisuals, nonce]], + extraFlowSyntaxMarkers: [NATIVE_CHAT_VISUAL_DIRECTIVE_MARKER], + sanitizeAttributes: {}, + components: {} + }) + const snapshots: string[][] = [] + for (const legacy of [true, false]) { + meter.legacy = legacy + const first = makeExtension('first'), + second = makeExtension('second') + const view = render( + + ) + view.rerender( + + ) + const html = [view.container.innerHTML] + view.rerender( + + ) + html.push(view.container.innerHTML) + snapshots.push(html) + view.unmount() + } + expect(snapshots[1]).toEqual(snapshots[0]) +}) + +it.each([20, 200])( + 'excludes the settled prefix from syntax/clone work with a bounded live tail N=%i', + (n) => { + installWorkMeters() + const fence = `\`\`\`text\n${'value\n'.repeat(n)}\`\`\`\n\n` + let source = `${fence}word` + const view = render(native(source, true, true, 'document')) + for (let i = 1; i <= n; i++) { + source += '\n\nword' + meter.work = createParseWork() + view.rerender(native(source, true, true, 'document')) + expect(meter.work.discovery).toHaveLength(1) + expect(meter.work.discovery[0].length).toBeLessThanOrEqual(16) + expect(meter.work.render).toEqual([]) + expect(meter.work.shifts).toBeLessThanOrEqual(8) + expect(meter.work.cloneNodes).toBeLessThanOrEqual(6) + expect(meter.work.cloneBytes).toBeLessThanOrEqual(800) + expect(meter.work.scans).toEqual([ + { source, marker: '\uFEFF' }, + { source: meter.work.discovery[0], marker: NATIVE_CHAT_VISUAL_DIRECTIVE_MARKER } + ]) + expect(meter.work.blocks).toBe(i + 2) + } + } +) diff --git a/src/renderer/src/components/sidebar/CommentMarkdown.tsx b/src/renderer/src/components/sidebar/CommentMarkdown.tsx index 0fe2ec02785..27d00c86bf7 100644 --- a/src/renderer/src/components/sidebar/CommentMarkdown.tsx +++ b/src/renderer/src/components/sidebar/CommentMarkdown.tsx @@ -3,11 +3,14 @@ import { CommentMarkdownMermaidContext, CommentMarkdownMermaidSourceContext } from './comment-markdown-mermaid-policy' -import Markdown, { defaultUrlTransform, type Components } from 'react-markdown' +import { + defaultUrlTransform, + type Components, + type Options as MarkdownOptions +} from 'react-markdown' import remarkGfm from 'remark-gfm' import remarkBreaks from 'remark-breaks' import { remarkChatMath } from './chat-markdown-math' -import { useChatMathRendering } from './chat-markdown-math-rendering' import { copyChatMathSelection } from './chat-markdown-math-selection' import remarkCjkFriendly from 'remark-cjk-friendly/parseOnly' import rehypeRaw from 'rehype-raw' @@ -25,14 +28,13 @@ import { type FileLinkExists } from './comment-markdown-native-chat-file-links' import { CommentMarkdownBlockLineContext } from './comment-markdown-code-renderers' -import { rehypeWordFade } from './comment-markdown-word-fade' import { remarkUnknownHtmlAsText } from './comment-markdown-unknown-html-as-text' import { CommentMarkdownWords } from './CommentMarkdownWords' import { - splitMarkdownTopLevelBlocks, - type MarkdownBlock, - type MarkdownBlockSplit -} from './markdown-top-level-blocks' + MemoizedMarkdown, + useMarkdownBlocks, + markdownBlockContent +} from './comment-markdown-block-pipeline' import { GITHUB_CALLOUT_SANITIZE_ATTRIBUTE, remarkGitHubCallouts @@ -41,9 +43,9 @@ import { remarkAutolinkLiteralProseBoundary } from '@/lib/remark-autolink-litera export type { CommentMarkdownLinkClickHandler } from './comment-markdown-element-renderers' -type MarkdownPlugins = NonNullable['rehypePlugins']> -type RemarkPlugins = NonNullable['remarkPlugins']> -type UrlTransform = NonNullable['urlTransform']> +type MarkdownPlugins = NonNullable +type RemarkPlugins = NonNullable +type UrlTransform = NonNullable type GitHubRepoReference = { owner: string @@ -232,6 +234,8 @@ const rehypePlugins: MarkdownPlugins = [rehypeRaw, [rehypeSanitize, commentMarkd /** Keep extensions stable per surface; new objects rebuild components and remount their contents. */ export type CommentMarkdownExtension = { remarkPlugins: RemarkPlugins + /** Extra flow syntax requires one of these markers; omitted means unknown grammar. */ + extraFlowSyntaxMarkers?: readonly string[] sanitizeAttributes: Record components: Components } @@ -270,49 +274,6 @@ type CommentMarkdownProps = React.ComponentPropsWithoutRef<'div'> & { unknownHtmlAsText?: boolean } -/** One render of the pipeline; memoized so an unchanged block is not parsed again. */ -const MemoizedMarkdown = React.memo(function DeferredMarkdown({ - math, - fadeWords, - ...props -}: React.ComponentProps & { math: boolean; fadeWords: boolean }) { - const renderMath = useChatMathRendering(math) - const plugins = React.useMemo( - () => [ - ...(props.rehypePlugins ?? []), - ...(math ? [renderMath] : []), - ...(fadeWords ? [rehypeWordFade] : []) - ], - [props.rehypePlugins, math, renderMath, fadeWords] - ) - return -}) - -/** The content as the blocks to render. Cut into blocks from the first time it is growing, and - * from then on, so the stream ending does not redraw what is already there. */ -function useMarkdownBlocks( - content: string, - growing: boolean, - math: boolean -): readonly MarkdownBlock[] { - const [previous, setPrevious] = React.useState(null) - if (!growing && previous === null) { - return [{ start: 0, line: 1, text: content }] - } - if (previous?.source === content) { - return previous.blocks - } - const next = splitMarkdownTopLevelBlocks(content, previous, math) - setPrevious(next) - return next.blocks -} - -function markdownBlockContent(block: MarkdownBlock): string { - return block.literalMath && block.end !== undefined - ? block.text.slice(0, block.end - block.start) - : block.text -} - // Why forwardRef + rest props: Radix's HoverCardTrigger asChild merges a ref // and event handlers (onPointerEnter, onPointerLeave, data-state, etc.) onto // the child. Without forwarding both, the hover card cannot open or position. @@ -371,7 +332,7 @@ const CommentMarkdown = React.memo( const withExtension = extension ? [...withMath, ...extension.remarkPlugins] : withMath return githubRepo ? [...withExtension, remarkGitHubReferences(githubRepo)] : withExtension }, [extension, fileLinkExists, githubRepo, math, unknownHtmlAsText]) - const blocks = useMarkdownBlocks(content, growing, math) + const blocks = useMarkdownBlocks(content, growing, math, activeRemarkPlugins, extension) const [hadWordFade, setHadWordFade] = React.useState(fadeWords) if (fadeWords && !hadWordFade) { setHadWordFade(true) @@ -400,6 +361,7 @@ const CommentMarkdown = React.memo( {blocks.map((block, index) => ( ['remarkPlugins']> + +/** One render of the pipeline; memoized so an unchanged block is not parsed again. */ +export const MemoizedMarkdown = React.memo(function DeferredMarkdown({ + math, + fadeWords, + block, + ...props +}: React.ComponentProps & { + math: boolean + fadeWords: boolean + block?: MarkdownBlock +}) { + const renderMath = useChatMathRendering(math) + const plugins = React.useMemo( + () => [ + ...(props.rehypePlugins ?? []), + ...(math ? [renderMath] : []), + ...(fadeWords ? [rehypeWordFade] : []) + ], + [props.rehypePlugins, math, renderMath, fadeWords] + ) + const parsedPlugins = React.useMemo(() => { + if (!block?.tree) { + return props.remarkPlugins ?? [] + } + const tree = block.tree + const source = markdownBlockContent(block) + // Transforms mutate mdast; each render owns a fresh copy of the pristine parse. + function reuseParsedBlock(this: Processor): void { + const parse = this.parser + if (!parse) { + return + } + this.parser = (value, file) => (value === source ? structuredClone(tree) : parse(value, file)) + } + return [...(props.remarkPlugins ?? []), reuseParsedBlock] + }, [block, props.remarkPlugins]) + return +}) + +/** The content as the blocks to render. Cut into blocks from the first time it is growing, and + * from then on, so the stream ending does not redraw what is already there. */ +export function useMarkdownBlocks( + content: string, + growing: boolean, + math: boolean, + plugins: RemarkPlugins, + extension?: CommentMarkdownExtension +): readonly MarkdownBlock[] { + const parser = React.useMemo(() => unified().use(remarkParse).use(plugins), [plugins]) + const [state, setState] = React.useState<{ + parser: typeof parser + split: MarkdownBlockSplit + } | null>(null) + if (state && state.parser !== parser) { + const split = { + ...state.split, + blocks: state.split.blocks.map( + ({ tree: _tree, closedFence: _closedFence, ...block }) => block + ) + } + setState({ parser, split }) + return split.blocks + } + const previous = state?.split ?? null + if (!growing && state === null) { + return [{ start: 0, line: 1, text: content }] + } + if (previous?.source === content) { + return previous.blocks + } + const markers = extension?.extraFlowSyntaxMarkers + const knownGrammar = !extension?.remarkPlugins.length || markers !== undefined + const next = splitMarkdownTopLevelBlocks( + content, + previous, + math, + knownGrammar ? (source) => parser.parse(source) : undefined, + (tail) => !content.includes('\uFEFF') && !markers?.some((marker) => tail.includes(marker)) + ) + setState({ parser, split: next }) + return next.blocks +} + +export function markdownBlockContent(block: MarkdownBlock): string { + return block.literalMath && block.end !== undefined + ? block.text.slice(0, block.end - block.start) + : block.text +} diff --git a/src/renderer/src/components/sidebar/markdown-parse-meter.test-fixture.ts b/src/renderer/src/components/sidebar/markdown-parse-meter.test-fixture.ts new file mode 100644 index 00000000000..8c4d72be00e --- /dev/null +++ b/src/renderer/src/components/sidebar/markdown-parse-meter.test-fixture.ts @@ -0,0 +1,51 @@ +import type { Nodes, Root } from 'mdast' + +type ParseWork = { + discovery: string[] + render: string[] + shifts: number + cloneNodes: number + cloneBytes: number + scans: { source: string; marker: string }[] + blocks: number +} + +export function createParseWork(): ParseWork { + return { + discovery: [], + render: [], + shifts: 0, + cloneNodes: 0, + cloneBytes: 0, + scans: [], + blocks: 0 + } +} + +export function nodeCount(node: Nodes): number { + return 1 + ('children' in node ? node.children.reduce((n, child) => n + nodeCount(child), 0) : 0) +} + +// Meter actual coordinate writes, leaving the parser's values and clone semantics unchanged. +export function meterPositions(tree: Root, onWrite: () => void): void { + function visit(node: Nodes): void { + if (node.position) { + for (const point of [node.position.start, node.position.end]) { + let offset = point.offset + Object.defineProperty(point, 'offset', { + enumerable: true, + configurable: true, + get: () => offset, + set: (value: number) => { + onWrite() + offset = value + } + }) + } + } + if ('children' in node) { + node.children.forEach(visit) + } + } + visit(tree) +} diff --git a/src/renderer/src/components/sidebar/markdown-top-level-blocks-legacy.test-fixture.ts b/src/renderer/src/components/sidebar/markdown-top-level-blocks-legacy.test-fixture.ts new file mode 100644 index 00000000000..fb50574b17a --- /dev/null +++ b/src/renderer/src/components/sidebar/markdown-top-level-blocks-legacy.test-fixture.ts @@ -0,0 +1,103 @@ +// Frozen splitter: differential tests compare every streaming prefix with the previous renderer. +// Reparse the last two blocks on append; cross-block definitions or HTML fall back to the whole document. + +import type { Nodes } from 'mdast' +import remarkGfm from 'remark-gfm' +import remarkParse from 'remark-parse' +import { remarkChatMath } from './chat-markdown-math' +import { chatMathCodeFences } from './chat-markdown-math-nodes' +import { unified } from 'unified' + +export type MarkdownBlock = { + /** Source offset, retained for settled blocks on append. */ + start: number + /** One-based source line. */ + line: number + text: string + literalMath?: boolean + /** Parser-owned end, excluding separators held for the next block. */ + end?: number +} + +export type MarkdownBlockSplit = { + source: string + blocks: readonly MarkdownBlock[] + /** Cross-block content requires whole-document rendering. */ + whole: boolean +} + +/** The last block, which is still growing, and the one it may yet join. */ +const UNSETTLED_BLOCKS = 2 + +const blockParser = unified().use(remarkParse).use(remarkGfm) +const mathBlockParser = blockParser().use(remarkChatMath) + +/** Tags that cannot hold anything, so cannot hold a later block. */ +const VOID_TAG_PATTERN = /^<(?:br|hr|wbr|img)\b[^>]*>$/i + +/** References and non-void HTML can affect other blocks and require whole-document parsing. */ +function reachesAcrossBlocks(node: Nodes): boolean { + if (node.type === 'definition' || node.type === 'footnoteDefinition') { + return true + } + if (node.type === 'html') { + return !VOID_TAG_PATTERN.test(node.value.trim()) + } + return 'children' in node && node.children.some(reachesAcrossBlocks) +} + +function hasLiteralMath(node: Nodes | undefined): boolean { + if (!node) { + return false + } + return ( + node.data?.mathComplete === false || + node.data?.mathLiteral === true || + ('children' in node && node.children.some(hasLiteralMath)) + ) +} + +function wholeDocument(source: string, literalMath = false): MarkdownBlockSplit { + return { source, blocks: [{ start: 0, line: 1, text: source, literalMath }], whole: true } +} + +export function splitMarkdownTopLevelBlocks( + source: string, + previous: MarkdownBlockSplit | null, + math = false +): MarkdownBlockSplit { + const grewFromPrevious = previous !== null && source.startsWith(previous.source) + if (grewFromPrevious && previous.whole && !math) { + return wholeDocument(source) + } + const settled = grewFromPrevious ? previous.blocks.slice(0, -UNSETTLED_BLOCKS) : [] + const reparsed = grewFromPrevious ? previous.blocks.at(-UNSETTLED_BLOCKS) : undefined + const base = reparsed?.start ?? 0 + const baseLine = reparsed?.line ?? 1 + const tail = source.slice(base) + const tree = (math ? mathBlockParser : blockParser).parse(tail) + if (math) { + chatMathCodeFences(tree, tail) + } + if (reachesAcrossBlocks(tree)) { + return wholeDocument(source, math && hasLiteralMath(tree)) + } + // Each block runs to the next one's start, so the blank lines between stay in the source. + const points = tree.children.map((child) => + 'position' in child ? child.position?.start : undefined + ) + const starts = points.map((point) => point?.offset ?? 0) + starts[0] = 0 + const blocks = starts.map((start, index) => ({ + start: base + start, + line: baseLine + (index === 0 ? 0 : (points[index]?.line ?? 1) - 1), + text: tail.slice(start, starts[index + 1]), + ...(math + ? { + literalMath: hasLiteralMath(tree.children[index]), + end: base + (tree.children[index]?.position?.end.offset ?? tail.length) + } + : {}) + })) + return { source, blocks: [...settled, ...blocks], whole: false } +} diff --git a/src/renderer/src/components/sidebar/markdown-top-level-blocks.ts b/src/renderer/src/components/sidebar/markdown-top-level-blocks.ts index 8a925d78f30..b007fe1be52 100644 --- a/src/renderer/src/components/sidebar/markdown-top-level-blocks.ts +++ b/src/renderer/src/components/sidebar/markdown-top-level-blocks.ts @@ -1,6 +1,6 @@ -// Reparse the last two blocks on append; cross-block definitions or HTML fall back to the whole document. +// Reparse unsettled blocks; cross-block definitions or HTML require the whole document. -import type { Nodes } from 'mdast' +import type { Nodes, Root } from 'mdast' import remarkGfm from 'remark-gfm' import remarkParse from 'remark-parse' import { remarkChatMath } from './chat-markdown-math' @@ -16,6 +16,9 @@ export type MarkdownBlock = { literalMath?: boolean /** Parser-owned end, excluding separators held for the next block. */ end?: number + /** Pristine tree in block-local coordinates, before rendering transforms. */ + tree?: Root + closedFence?: boolean } export type MarkdownBlockSplit = { @@ -63,23 +66,43 @@ function wholeDocument(source: string, literalMath = false): MarkdownBlockSplit export function splitMarkdownTopLevelBlocks( source: string, previous: MarkdownBlockSplit | null, - math = false + math = false, + parseTree?: (source: string) => Root, + canReuse: (tail: string) => boolean = () => !source.includes('\uFEFF') ): MarkdownBlockSplit { + let parse = parseTree const grewFromPrevious = previous !== null && source.startsWith(previous.source) if (grewFromPrevious && previous.whole && !math) { return wholeDocument(source) } - const settled = grewFromPrevious ? previous.blocks.slice(0, -UNSETTLED_BLOCKS) : [] - const reparsed = grewFromPrevious ? previous.blocks.at(-UNSETTLED_BLOCKS) : undefined + // A sliced, indented opener must retain the original discovery context. + const last = grewFromPrevious ? previous.blocks.at(-1) : undefined + let unsettled = + parse && + grewFromPrevious && + previous.blocks.at(-UNSETTLED_BLOCKS)?.closedFence && + last && + source[last.start - 1] === '\n' + ? 1 + : UNSETTLED_BLOCKS + // Declining must restore the original discovery window before choosing any cuts. + const candidateBase = grewFromPrevious ? (previous.blocks.at(-unsettled)?.start ?? 0) : 0 + if (parse && !canReuse(source.slice(candidateBase))) { + parse = undefined + unsettled = UNSETTLED_BLOCKS + } + const settled = grewFromPrevious ? previous.blocks.slice(0, -unsettled) : [] + const reparsed = grewFromPrevious ? previous.blocks.at(-unsettled) : undefined const base = reparsed?.start ?? 0 const baseLine = reparsed?.line ?? 1 const tail = source.slice(base) - const tree = (math ? mathBlockParser : blockParser).parse(tail) + const tree = parse ? parse(tail) : (math ? mathBlockParser : blockParser).parse(tail) if (math) { chatMathCodeFences(tree, tail) } if (reachesAcrossBlocks(tree)) { - return wholeDocument(source, math && hasLiteralMath(tree)) + const whole = wholeDocument(source, math && hasLiteralMath(tree)) + return parse && base === 0 ? { ...whole, blocks: [{ ...whole.blocks[0], tree }] } : whole } // Each block runs to the next one's start, so the blank lines between stay in the source. const points = tree.children.map((child) => @@ -87,16 +110,68 @@ export function splitMarkdownTopLevelBlocks( ) const starts = points.map((point) => point?.offset ?? 0) starts[0] = 0 - const blocks = starts.map((start, index) => ({ - start: base + start, - line: baseLine + (index === 0 ? 0 : (points[index]?.line ?? 1) - 1), - text: tail.slice(start, starts[index + 1]), - ...(math - ? { - literalMath: hasLiteralMath(tree.children[index]), - end: base + (tree.children[index]?.position?.end.offset ?? tail.length) - } - : {}) - })) + const blocks = starts.map((start, index): MarkdownBlock => { + const child = tree.children[index] + const text = tail.slice(start, starts[index + 1]) + const line = index === 0 ? 1 : (points[index]?.line ?? 1) + const block: MarkdownBlock = { + start: base + start, + line: baseLine + line - 1, + text, + ...(math + ? { + literalMath: hasLiteralMath(child), + end: base + (child?.position?.end.offset ?? tail.length) + } + : {}) + } + const prior = grewFromPrevious ? previous.blocks[settled.length + index] : undefined + if ( + prior?.tree && + prior.start === block.start && + prior.text === text && + prior.literalMath === block.literalMath && + prior.end === block.end + ) { + return prior + } + if (parse && child) { + block.closedFence = isClosedFence(child, text, start) + // Slicing an indented opener changes its standalone parse, so preserve that parse. + if (index === 0 || points[index]?.column === 1) { + block.tree = { type: 'root', children: [child] } + shiftMarkdownPositions(block.tree, start, line - 1) + } + } + return block + }) return { source, blocks: [...settled, ...blocks], whole: false } } + +function shiftMarkdownPositions(node: Nodes, offset: number, lines: number): void { + if (node.position) { + for (const point of [node.position.start, node.position.end]) { + if (point.offset !== undefined) { + point.offset -= offset + } + point.line -= lines + } + } + if ('children' in node) { + node.children.forEach((child) => shiftMarkdownPositions(child, offset, lines)) + } +} + +function isClosedFence(node: Nodes, source: string, offset: number): boolean { + if (node.type !== 'code') { + return false + } + const opening = /^ {0,3}(`{3,}|~{3,})[^\r\n]*\r?\n/.exec(source)?.[1] + if (!opening || !node.position) { + return false + } + const end = (node.position.end.offset ?? offset + source.length) - offset + const authored = source.slice(0, end) + const lastLine = authored.slice(authored.lastIndexOf('\n') + 1).replace(/\r$/, '') + return new RegExp(`^ {0,3}${opening[0]}{${opening.length},}[ \t]*$`).test(lastLine) +}