From 79a8cff3d6d9bf711eda25cbfe68cd0b37dc68ba Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 8 Oct 2026 10:17:26 -0700 Subject: [PATCH] Fix Mermaid diagrams that stay on error after replies finish (#26480) * Fix Mermaid diagram recovery after streaming replies * Clarify when the Mermaid render queue releases tasks --- .../components/editor/MermaidBlock.test.tsx | 108 ++++++++++++++++++ .../src/components/editor/MermaidBlock.tsx | 37 +++--- .../NativeChatMarkdown.mermaid.test.tsx | 70 ++++++++++++ .../native-chat/NativeChatMarkdown.tsx | 1 + .../components/sidebar/CommentMarkdown.tsx | 10 +- .../comment-markdown-element-renderers.tsx | 7 +- 6 files changed, 211 insertions(+), 22 deletions(-) create mode 100644 src/renderer/src/components/editor/MermaidBlock.test.tsx create mode 100644 src/renderer/src/components/native-chat/NativeChatMarkdown.mermaid.test.tsx diff --git a/src/renderer/src/components/editor/MermaidBlock.test.tsx b/src/renderer/src/components/editor/MermaidBlock.test.tsx new file mode 100644 index 00000000000..af0415eb07e --- /dev/null +++ b/src/renderer/src/components/editor/MermaidBlock.test.tsx @@ -0,0 +1,108 @@ +// @vitest-environment happy-dom +import '@testing-library/jest-dom/vitest' +import { act, cleanup, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mermaid = vi.hoisted(() => ({ + initialize: vi.fn(), + render: vi.fn<(id: string, content: string) => Promise<{ svg: string }>>(), + sanitize: vi.fn<(svg: string) => string>() +})) +vi.mock('mermaid', () => ({ default: mermaid })) +vi.mock('dompurify', () => ({ default: { sanitize: mermaid.sanitize } })) + +import MermaidBlock from './MermaidBlock' + +const partial = 'flowchart TD\n A["Sync with main ' +const complete = `${partial}once"]` +const diagram = 'Sync with main once' + +beforeEach(() => { + mermaid.render.mockReset() + mermaid.render.mockResolvedValue({ svg: diagram }) + mermaid.sanitize.mockReset().mockImplementation((svg) => svg) +}) +afterEach(cleanup) + +describe('MermaidBlock rendering', () => { + it('replaces an invalid partial source error with the completed diagram', async () => { + mermaid.render.mockRejectedValueOnce(new Error('Parse error on line 2')) + const { container, rerender } = render() + await screen.findByText(/Diagram error: Parse error on line 2/) + expect(container.querySelector('pre')?.textContent).toBe(partial) + + rerender() + await waitFor(() => + expect(container.querySelector('svg')).toHaveTextContent('Sync with main once') + ) + expect(screen.queryByText(/Diagram error:/)).not.toBeInTheDocument() + expect(container.querySelector('pre')).toBeNull() + }) + + it.each(['success', 'failure'])( + 'ignores an obsolete render %s after content changes', + async (outcome) => { + const obsolete = Promise.withResolvers<{ svg: string }>() + const current = Promise.withResolvers<{ svg: string }>() + mermaid.render.mockReturnValueOnce(obsolete.promise).mockReturnValueOnce(current.promise) + const { container, rerender } = render() + await waitFor(() => expect(mermaid.render).toHaveBeenCalledTimes(1)) + rerender() + + await act(async () => { + if (outcome === 'success') { + obsolete.resolve({ svg: 'Obsolete' }) + } else { + obsolete.reject(new Error('Obsolete parse error')) + } + await obsolete.promise.catch(() => {}) + }) + await waitFor(() => expect(mermaid.render).toHaveBeenCalledTimes(2)) + expect(container).not.toHaveTextContent('Obsolete') + await act(async () => current.resolve({ svg: diagram })) + await waitFor(() => + expect(container.querySelector('svg')).toHaveTextContent('Sync with main once') + ) + expect(container.querySelector('.mermaid-error')).toBeNull() + } + ) + + it('sanitizes the successful SVG before displaying it', async () => { + const unsafeSvg = + 'Safe diagram' + mermaid.render.mockResolvedValue({ svg: unsafeSvg }) + mermaid.sanitize.mockReturnValueOnce('Safe diagram') + const { container } = render() + await screen.findByText('Safe diagram') + expect(mermaid.sanitize).toHaveBeenCalledWith(unsafeSvg, { USE_PROFILES: { svg: true } }) + expect(container.querySelector('script')).toBeNull() + expect(container.querySelector('svg')).not.toHaveAttribute('onload') + }) + + it('keeps new diagrams queued behind a render already in progress', async () => { + const first = Promise.withResolvers<{ svg: string }>() + const second = Promise.withResolvers<{ svg: string }>() + mermaid.render.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise) + const { rerender } = render( + <> + + + + ) + await waitFor(() => expect(mermaid.render).toHaveBeenCalledTimes(1)) + await act(async () => first.resolve({ svg: diagram })) + await waitFor(() => expect(mermaid.render).toHaveBeenCalledTimes(2)) + + rerender( + <> + + + + + ) + await act(async () => {}) + expect(mermaid.render).toHaveBeenCalledTimes(2) + await act(async () => second.resolve({ svg: diagram })) + await waitFor(() => expect(mermaid.render).toHaveBeenCalledTimes(3)) + }) +}) diff --git a/src/renderer/src/components/editor/MermaidBlock.tsx b/src/renderer/src/components/editor/MermaidBlock.tsx index f25de3983ff..a29b64d8464 100644 --- a/src/renderer/src/components/editor/MermaidBlock.tsx +++ b/src/renderer/src/components/editor/MermaidBlock.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useId, useRef, useState } from 'react' +import React, { useEffect, useId, useState } from 'react' import type mermaidNamespace from 'mermaid' import DOMPurify from 'dompurify' import { getMermaidConfig } from './mermaid-config' @@ -29,18 +29,21 @@ type MermaidBlockProps = { // where one render can clobber another's temporary DOM node. Serializing all // render calls through a single promise chain avoids this. // -// The queue is replaced with a fresh promise after each render completes so -// that old .then() closures (which capture containerRef, content, and id) +// The queue is replaced with a fresh promise once all waiting renders complete so +// that old .then() closures (which capture content and id) // become unreachable and can be GC'd. Without this, the chain grows with // every MermaidBlock mount/unmount cycle for the lifetime of the renderer. let renderQueue: Promise = Promise.resolve() function enqueueRender(fn: () => Promise): void { - renderQueue = renderQueue.then(fn, fn).then(() => { + const tail = renderQueue.then(fn, fn).then(() => { // Why: collapse the chain back to a single resolved promise so previous // closures do not remain reachable through a growing .then() chain. - renderQueue = Promise.resolve() + if (renderQueue === tail) { + renderQueue = Promise.resolve() + } }) + renderQueue = tail } /** @@ -53,8 +56,7 @@ export default function MermaidBlock({ htmlLabels = false }: MermaidBlockProps): React.JSX.Element { const id = useId().replace(/:/g, '_') - const containerRef = useRef(null) - const [error, setError] = useState(null) + const [result, setResult] = useState<{ svg: string } | { error: string } | null>(null) useEffect(() => { let cancelled = false @@ -72,18 +74,17 @@ export default function MermaidBlock({ // broken foreignObject label path again. mermaid.initialize(getMermaidConfig(isDark, htmlLabels)) const { svg } = await mermaid.render(`mermaid-${id}`, content) - if (!cancelled && containerRef.current) { + if (!cancelled) { // Why: although mermaid uses DOMPurify internally, we add an explicit // sanitization pass as defense-in-depth against XSS in case upstream // behaviour changes or a mermaid version ships without sanitization. - containerRef.current.innerHTML = DOMPurify.sanitize(svg, { - USE_PROFILES: { svg: true } + setResult({ + svg: DOMPurify.sanitize(svg, { USE_PROFILES: { svg: true } }) }) - setError(null) } } catch (err) { if (!cancelled) { - setError(err instanceof Error ? err.message : 'Invalid mermaid syntax') + setResult({ error: err instanceof Error ? err.message : 'Invalid mermaid syntax' }) // Mermaid leaves an error element in the DOM on failure — clean it up. const errorEl = document.getElementById(`d${`mermaid-${id}`}`) errorEl?.remove() @@ -99,11 +100,12 @@ export default function MermaidBlock({ } }, [content, htmlLabels, isDark, id]) - if (error) { + if (result && 'error' in result) { return (
- {translate('auto.components.editor.MermaidBlock.dcc132e691', 'Diagram error:')} {error} + {translate('auto.components.editor.MermaidBlock.dcc132e691', 'Diagram error:')}{' '} + {result.error}
           {content}
@@ -112,5 +114,10 @@ export default function MermaidBlock({
     )
   }
 
-  return 
+ return ( +
+ ) } diff --git a/src/renderer/src/components/native-chat/NativeChatMarkdown.mermaid.test.tsx b/src/renderer/src/components/native-chat/NativeChatMarkdown.mermaid.test.tsx new file mode 100644 index 00000000000..8a9eda3b245 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatMarkdown.mermaid.test.tsx @@ -0,0 +1,70 @@ +// @vitest-environment happy-dom +import '@testing-library/jest-dom/vitest' +import { cleanup, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mermaid = vi.hoisted(() => ({ initialize: vi.fn(), render: vi.fn() })) +vi.mock('mermaid', () => ({ default: mermaid })) +vi.mock('dompurify', () => ({ default: { sanitize: (svg: string) => svg } })) +vi.mock('@/store', () => ({ + useAppStore: (selector: (state: { settings: { theme: string } }) => unknown) => + selector({ settings: { theme: 'light' } }) +})) + +import { NativeChatMarkdown } from './NativeChatMarkdown' +import { NativeChatCodeBlock } from './NativeChatCodeBlock' +import { TooltipProvider } from '@/components/ui/tooltip' + +const partial = 'flowchart TD\n A["Sync with main ' +const complete = `${partial}once"]` + +function reply(source: string, streaming: boolean): React.JSX.Element { + return ( + + + + ) +} + +beforeEach(() => { + mermaid.render.mockReset() + mermaid.render.mockResolvedValue({ svg: 'Sync with main once' }) +}) +afterEach(cleanup) + +describe('native chat Mermaid fences', () => { + it('shows partial source while streaming and renders the diagram when the reply finishes', async () => { + const { container, rerender } = render(reply(partial, true)) + expect(container.querySelector('[data-native-chat-code-content]')?.textContent).toContain( + partial + ) + expect(container.querySelector('[data-code-language="mermaid"]')).toBeInTheDocument() + expect(container.querySelector('.mermaid-block')).toBeNull() + expect(mermaid.render).not.toHaveBeenCalled() + + rerender(reply(complete, true)) + expect(container.querySelector('pre')?.textContent).toContain(complete) + expect(mermaid.render).not.toHaveBeenCalled() + + rerender(reply(complete, false)) + await waitFor(() => + expect(container.querySelector('svg')).toHaveTextContent('Sync with main once') + ) + expect(mermaid.render).toHaveBeenCalledWith(expect.any(String), complete) + expect(container.querySelector('pre')).toBeNull() + expect(screen.queryByText(/Diagram error:/)).not.toBeInTheDocument() + }) + + it('still shows the existing error and source for an invalid finished reply', async () => { + mermaid.render.mockRejectedValue(new Error('Invalid mermaid syntax')) + const { container } = render(reply(partial, false)) + await screen.findByText(/Diagram error: Invalid mermaid syntax/) + expect(container.querySelector('pre')?.textContent).toBe(partial.trimEnd()) + expect(container.querySelector('svg')).toBeNull() + }) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatMarkdown.tsx b/src/renderer/src/components/native-chat/NativeChatMarkdown.tsx index 4033b51fc62..d93b2110787 100644 --- a/src/renderer/src/components/native-chat/NativeChatMarkdown.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMarkdown.tsx @@ -27,6 +27,7 @@ export function NativeChatMarkdown({ extension && streaming ? withoutPendingNativeChatVisualDirectiveTail(content) : content } extension={extension} + renderMermaid={!streaming} className={cn('native-chat-markdown', className)} /> ) diff --git a/src/renderer/src/components/sidebar/CommentMarkdown.tsx b/src/renderer/src/components/sidebar/CommentMarkdown.tsx index 190d93a768d..8ea88eb578b 100644 --- a/src/renderer/src/components/sidebar/CommentMarkdown.tsx +++ b/src/renderer/src/components/sidebar/CommentMarkdown.tsx @@ -220,6 +220,7 @@ type CommentMarkdownProps = React.ComponentPropsWithoutRef<'div'> & { linkifyFilePaths?: boolean expandImages?: boolean renderCodeBlock?: DocumentCodeBlockRenderer + renderMermaid?: boolean extension?: CommentMarkdownExtension } @@ -238,6 +239,7 @@ const CommentMarkdown = React.memo( linkifyFilePaths = false, expandImages = false, renderCodeBlock, + renderMermaid = true, extension, ...rest }, @@ -246,17 +248,17 @@ const CommentMarkdown = React.memo( const baseComponents = React.useMemo(() => { if (!onLinkClick) { return variant === 'document' - ? renderCodeBlock - ? createDocumentCommentMarkdownComponents(undefined, renderCodeBlock) + ? renderCodeBlock || !renderMermaid + ? createDocumentCommentMarkdownComponents(undefined, renderCodeBlock, renderMermaid) : documentCommentMarkdownComponents : expandImages ? createCompactCommentMarkdownComponents(undefined, true) : compactCommentMarkdownComponents } return variant === 'document' - ? createDocumentCommentMarkdownComponents(onLinkClick, renderCodeBlock) + ? createDocumentCommentMarkdownComponents(onLinkClick, renderCodeBlock, renderMermaid) : createCompactCommentMarkdownComponents(onLinkClick, expandImages) - }, [expandImages, renderCodeBlock, variant, onLinkClick]) + }, [expandImages, renderCodeBlock, renderMermaid, variant, onLinkClick]) const components = React.useMemo( () => (extension ? { ...baseComponents, ...extension.components } : baseComponents), [baseComponents, extension] diff --git a/src/renderer/src/components/sidebar/comment-markdown-element-renderers.tsx b/src/renderer/src/components/sidebar/comment-markdown-element-renderers.tsx index bfcfedfa92a..9ca2a24271b 100644 --- a/src/renderer/src/components/sidebar/comment-markdown-element-renderers.tsx +++ b/src/renderer/src/components/sidebar/comment-markdown-element-renderers.tsx @@ -250,7 +250,8 @@ export function createCompactCommentMarkdownComponents( export function createDocumentCommentMarkdownComponents( onLinkClick?: CommentMarkdownLinkClickHandler, - renderCodeBlock?: DocumentCodeBlockRenderer + renderCodeBlock?: DocumentCodeBlockRenderer, + renderMermaid = true ): Components { return { p: ({ children }) =>

{children}

, @@ -272,7 +273,7 @@ export function createDocumentCommentMarkdownComponents( ), code: ({ className, children }) => - isMermaidFence(className) ? ( + renderMermaid && isMermaidFence(className) ? ( renderMermaidFence( children, 'my-3 min-w-0 max-w-full overflow-x-auto rounded-md border border-border/60 p-3 [&_.mermaid-block]:min-w-0 [&_.mermaid-block_pre]:my-0 [&_.mermaid-block_pre]:max-h-80 [&_.mermaid-block_pre]:max-w-full [&_.mermaid-block_pre]:overflow-x-auto [&_.mermaid-block_pre]:rounded-md [&_.mermaid-block_pre]:bg-accent [&_.mermaid-block_pre]:p-3 [&_.mermaid-block_pre]:font-mono [&_.mermaid-block_pre]:text-[12px]' @@ -284,7 +285,7 @@ export function createDocumentCommentMarkdownComponents( ), // Mermaid fences render a
, which is invalid inside
, so unwrap them.
     pre: ({ children }) =>
-      isMermaidPre(children) ? (
+      renderMermaid && isMermaidPre(children) ? (
         <>{children}
       ) : renderCodeBlock ? (
         renderCodeBlock({ children, language: extractCodeFenceLanguage(children) })