fix(MermaidViewer): clean up orphan error SVG left in document.body

[GoldenXPig]

When mermaid 11.13.0 fails to parse a diagram, errorRenderer.draw() paints a large error icon SVG into the temporary d{id} wrapper that mermaid appends to document.body, then immediately re-throws — skipping the trailing removeTempElements() call (mermaid.core.mjs:1078-1110). The orphan SVG (viewBox 0 0 2412 512) accumulates at the bottom of the page on every failed render.

Add a cleanupOrphans() helper in the render effect that removes the d{id} wrapper div and the {id} SVG node, invoked from a try/finally around mermaid.render() and from the effect cleanup. The component's own error UI is unchanged — failures still surface as a red banner with the original code inside the card.

Summary by CodeRabbit

• Bug Fixes
  • Fixed an issue where Mermaid diagram rendering failures could leave orphaned SVG elements on the page. Improved cleanup mechanisms now ensure these orphaned elements are properly removed, preventing performance degradation and maintaining a cleaner visual experience.

[gemini-code-assist]

Code Review

This pull request introduces a cleanup mechanism to remove orphaned DOM elements left by Mermaid after failed rendering attempts. The feedback highlights a potential race condition when using a stable ID across render attempts and suggests adding a unique suffix for isolation. Additionally, there is a concern that the cleanup logic might inadvertently remove the active diagram from the UI, so it is recommended to restrict element removal to those directly attached to the document body.

[gemini-code-assist]

Using a stable id across multiple render attempts can lead to race conditions. If a new render starts before a previous one finishes, they will both attempt to use the same DOM ID, which can cause Mermaid to fail or behave unexpectedly.

Consider appending a unique suffix to the id for each render attempt to ensure isolation.

Suggested change

	const id = `mermaid-${uniqueId}`;
	const id = `mermaid-${uniqueId}-${Math.random().toString(36).slice(2, 9)}`;

[gemini-code-assist]

The current cleanupOrphans implementation might inadvertently remove the active diagram from the UI. When React renders the SVG into the component's tree, it will have the same id. If cleanupOrphans is called (e.g., during effect cleanup when the code changes), it will find the SVG in the UI and remove it from the DOM, causing the diagram to disappear prematurely.

To prevent this, ensure that only elements directly attached to document.body (Mermaid's temporary orphans) are removed.

    const cleanupOrphans = (): void => {
      const wrapper = document.getElementById(`d${id}`);
      if (wrapper?.parentElement === document.body) wrapper.remove();
      const svg = document.getElementById(id);
      if (svg?.parentElement === document.body) svg.remove();
    };

[coderabbitai]

📝 Walkthrough

Walkthrough

The MermaidViewer component's render effect now defines a stable diagram id outside the try/catch block and introduces a cleanupOrphans() function to remove orphaned DOM nodes created during render failures. Cleanup is executed in both a finally block and the effect's cleanup function.

Changes

Cohort / File(s)	Summary
Mermaid rendering robustness src/renderer/components/chat/viewers/MermaidViewer.tsx	Added stable diagram ID definition outside render try/catch and implemented cleanupOrphans() routine to remove orphaned DOM nodes (d{id} and id}) from failed renders; cleanup executes in finally block and effect cleanup function.

Possibly related PRs

• feat: render mermaid code blocks as interactive diagrams (#124) #128: Adds the MermaidViewer component and its initial rendering logic.
• perf: lazy-load mermaid to reduce main bundle size #164: Refactors mermaid loading and rendering to use async dynamic imports in the render effect.

Suggested labels

bug

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/renderer/components/chat/viewers/MermaidViewer.tsx (1)

75-97: Possible race between overlapping renders that share the same id.

uniqueId is stable for the component instance, so when code or isDark changes while a previous render() is still in flight, both invocations call mermaid.render(id, …) with the same id. Sequence to consider:

1. Render A is awaiting mermaid.render.
2. Effect re-runs (deps change) → teardown sets cancelled = true and calls cleanupOrphans().
3. Render B starts and mermaid.render appends a fresh d${id} wrapper to document.body.
4. Render A finally settles → its finally runs cleanupOrphans() and yanks Render B's live wrapper out from under it.

In practice this window is small and only matters under rapid prop changes, but it's the kind of thing that can produce a flaky empty/error state. Two cheap mitigations:

♻️ Option 1 — guard the finally with the cancelled flag

       } finally {
-        cleanupOrphans();
+        // Only the active render owns these orphans; a stale (cancelled) render
+        // could otherwise remove the next attempt's in-flight temp nodes.
+        if (!cancelled) cleanupOrphans();
       }

Note: the effect-teardown call at line 96 still ensures cleanup on unmount / dep change.

♻️ Option 2 — use a per-attempt id

Generate a fresh suffix per render() invocation so concurrent attempts don't collide, and have each invocation clean up only its own id.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/renderer/components/chat/viewers/MermaidViewer.tsx` around lines 75 - 97,
The race is caused by concurrent render() calls sharing the stable
component-wide id; change render() to use a fresh per-attempt id/suffix (e.g.
attemptId or renderSuffix) created inside the render function and pass that
composed id to m.render(...) so each invocation renders into a unique wrapper,
and adjust cleanupOrphans()/cleanup call inside render() to only remove the
wrapper for that attemptId (leave the effect teardown to still set cancelled =
true and perform global cleanup). Update references: render (the async
function), id (component id), ensureMermaidInit, cleanupOrphans, setSvg and
setError so each attempt only cleans up its own DOM node and does not remove
other in-flight renderers' wrappers.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/renderer/components/chat/viewers/MermaidViewer.tsx`:
- Around line 75-97: The race is caused by concurrent render() calls sharing the
stable component-wide id; change render() to use a fresh per-attempt id/suffix
(e.g. attemptId or renderSuffix) created inside the render function and pass
that composed id to m.render(...) so each invocation renders into a unique
wrapper, and adjust cleanupOrphans()/cleanup call inside render() to only remove
the wrapper for that attemptId (leave the effect teardown to still set cancelled
= true and perform global cleanup). Update references: render (the async
function), id (component id), ensureMermaidInit, cleanupOrphans, setSvg and
setError so each attempt only cleans up its own DOM node and does not remove
other in-flight renderers' wrappers.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1fc1be47-7a3e-496a-9262-26c236a012a1

📥 Commits

Reviewing files that changed from the base of the PR and between a4e3b34 and 33cf92c.

📒 Files selected for processing (1)

• src/renderer/components/chat/viewers/MermaidViewer.tsx