fix: address review — plain fences while streaming
ds-review-bot finding: a growing fence retokenized on every chunk (quadratic main-thread work). MarkdownText gains a streaming flag — the streaming partial renders fences through the plain arm and the finalize swap highlights once; AssistantMarkdown threads its existing flag. (The zh Agent Note pair the review also flagged landed earlier on this branch.) New spec pins plain-while-streaming and highlighted-after-finalize.
This commit is contained in:
@@ -44,7 +44,7 @@ export const AssistantMarkdown = memo(function AssistantMarkdown({ blocks, strea
|
|||||||
<div className={css.root} data-streaming={streaming || undefined}>
|
<div className={css.root} data-streaming={streaming || undefined}>
|
||||||
{blocks.map((block, i) => {
|
{blocks.map((block, i) => {
|
||||||
switch (block.kind) {
|
switch (block.kind) {
|
||||||
case 'text': return <MarkdownText key={i} text={block.text} />
|
case 'text': return <MarkdownText key={i} text={block.text} streaming={streaming} />
|
||||||
case 'reasoning': return <ThinkRow key={i} text={block.text} running={streaming && i === last} />
|
case 'reasoning': return <ThinkRow key={i} text={block.text} running={streaming && i === last} />
|
||||||
// Tool-call heads render as tool rows in the chat view's grouping pass.
|
// Tool-call heads render as tool rows in the chat view's grouping pass.
|
||||||
case 'tool-call': return null
|
case 'tool-call': return null
|
||||||
|
|||||||
@@ -24,7 +24,9 @@ function sanitizeUrl(url: string): string {
|
|||||||
|
|
||||||
const safeUrl: UrlTransform = url => sanitizeUrl(url)
|
const safeUrl: UrlTransform = url => sanitizeUrl(url)
|
||||||
|
|
||||||
const components: Components = {
|
/** Build the component table; while `streaming`, fences render the plain arm (see CodeBlock). */
|
||||||
|
function buildComponents(streaming: boolean): Components {
|
||||||
|
return {
|
||||||
a: ({ href = '', children }) => {
|
a: ({ href = '', children }) => {
|
||||||
const safeHref = sanitizeUrl(href)
|
const safeHref = sanitizeUrl(href)
|
||||||
if (safeHref === '') return <>{children}</>
|
if (safeHref === '') return <>{children}</>
|
||||||
@@ -44,32 +46,40 @@ const components: Components = {
|
|||||||
<table>{children}</table>
|
<table>{children}</table>
|
||||||
</div>
|
</div>
|
||||||
),
|
),
|
||||||
// Fenced blocks route through the shared CodeBlock (shiki for registered
|
// Fenced blocks route through the shared CodeBlock (shiki for registered
|
||||||
// grammars, identical-geometry plain fallback for unknown/absent languages);
|
// grammars, identical-geometry plain fallback for unknown/absent
|
||||||
// inline code keeps the default <code> path (the :not(pre) rule styles it).
|
// languages); inline code keeps the default <code> path (the :not(pre)
|
||||||
pre: ({ children }) => {
|
// rule styles it). While the message streams, the fence renders the
|
||||||
const child = isValidElement<{ className?: string; children?: unknown }>(children) ? children : undefined
|
// plain arm — retokenizing a growing fence on every chunk is quadratic
|
||||||
const raw = child?.props.children
|
// main-thread work; the finalize swap highlights it once.
|
||||||
const text = typeof raw === 'string' ? raw : Array.isArray(raw) && typeof raw[0] === 'string' ? raw[0] : undefined
|
pre: ({ children }) => {
|
||||||
// A fence whose content isn't one plain string (never produced by the
|
const child = isValidElement<{ className?: string; children?: unknown }>(children) ? children : undefined
|
||||||
// markdown pipeline) keeps the stock <pre> rather than guessing.
|
const raw = child?.props.children
|
||||||
if (text === undefined) return <pre>{children}</pre>
|
const text = typeof raw === 'string' ? raw : Array.isArray(raw) && typeof raw[0] === 'string' ? raw[0] : undefined
|
||||||
const lang = /language-([\w-]+)/.exec(child?.props.className ?? '')?.[1]
|
// A fence whose content isn't one plain string (never produced by the
|
||||||
return <CodeBlock code={text} lang={lang} />
|
// markdown pipeline) keeps the stock <pre> rather than guessing.
|
||||||
},
|
if (text === undefined) return <pre>{children}</pre>
|
||||||
|
const lang = /language-([\w-]+)/.exec(child?.props.className ?? '')?.[1]
|
||||||
|
return <CodeBlock code={text} lang={streaming ? undefined : lang} />
|
||||||
|
},
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const staticComponents = buildComponents(false)
|
||||||
|
const streamingComponents = buildComponents(true)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Render untrusted assistant-authored Markdown as semantic React elements.
|
* Render untrusted assistant-authored Markdown as semantic React elements.
|
||||||
* @param props - Markdown source text preserved by the session projection.
|
* @param props - Markdown source text preserved by the session projection;
|
||||||
|
* `streaming` renders fences plain (highlighting lands on the finalize swap).
|
||||||
* @returns A GFM document with raw HTML, relative links, unsafe protocols, and remote images disabled.
|
* @returns A GFM document with raw HTML, relative links, unsafe protocols, and remote images disabled.
|
||||||
*/
|
*/
|
||||||
export function MarkdownText({ text }: { text: string }) {
|
export function MarkdownText({ text, streaming = false }: { text: string; streaming?: boolean }) {
|
||||||
return (
|
return (
|
||||||
<div className={css.markdown}>
|
<div className={css.markdown}>
|
||||||
<ReactMarkdown
|
<ReactMarkdown
|
||||||
remarkPlugins={remarkPlugins}
|
remarkPlugins={remarkPlugins}
|
||||||
components={components}
|
components={streaming ? streamingComponents : staticComponents}
|
||||||
urlTransform={safeUrl}
|
urlTransform={safeUrl}
|
||||||
>
|
>
|
||||||
{text}
|
{text}
|
||||||
|
|||||||
@@ -64,6 +64,16 @@ describe('MarkdownText', () => {
|
|||||||
expect(screen.getByRole('link', { name: 'https://deepseek.com' })).toBeTruthy()
|
expect(screen.getByRole('link', { name: 'https://deepseek.com' })).toBeTruthy()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('streaming renders fences plain; the finalize swap highlights them', () => {
|
||||||
|
const fence = '```ts\nconst answer = 42\n```'
|
||||||
|
const live = render(<MarkdownText text={fence} streaming />)
|
||||||
|
expect(live.container.querySelector('pre.shiki')).toBeNull()
|
||||||
|
expect(live.container.querySelector('pre code')?.textContent).toContain('const answer = 42')
|
||||||
|
live.unmount()
|
||||||
|
const done = render(<MarkdownText text={fence} />)
|
||||||
|
expect(done.container.querySelector('pre.shiki')).not.toBeNull()
|
||||||
|
})
|
||||||
|
|
||||||
it('neutralizes raw HTML, unsafe or relative links, and remote images', () => {
|
it('neutralizes raw HTML, unsafe or relative links, and remote images', () => {
|
||||||
const markdown = [
|
const markdown = [
|
||||||
'<script>globalThis.compromised = true</script>',
|
'<script>globalThis.compromised = true</script>',
|
||||||
|
|||||||
Reference in New Issue
Block a user