Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
UI consistency review of the mermaid diagram/overlay work. Two findings, both about ownership of the new overlay styling rather than the feature itself.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — ... You can add or adjust custom eligibility rules. Learn more. |
There was a problem hiding this comment.
One finding: the overlay scene's surface loses the custom-theme bridge that the inline mermaid code block has, so the two diverge under html[data-theme-id] palettes.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding: the streaming/idle mermaid source block does not carry the code font-size preference that every other chat code surface uses.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding: the mermaid error fallback in ChatMarkdown.tsx reuses the chat-markdown-mermaid-error surface without the data-markdown-copy contract the other three mermaid states carry, which changes copy behavior for that surface. Earlier findings (raw .dark selector, portaled overlay chrome/theming, pending-source font size) look resolved on this head.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding on the new mermaid CSS: the pending-source rule's color loses to the runtime theme bridge, so the dimmed treatment only applies on the default palette. Everything else (chrome variants, @variant dark usage, copy fences, overlay theme override) looks consistent with the surrounding chat-markdown surfaces.
Posted via Macroscope — UI Consistency
Agent mermaid fences showed as highlighted source only. Chat markdown now lazy-loads mermaid, draws the diagram by default, and lets the user toggle back to the code. Copy still yields the original fence. Co-authored-by: paul <paul@recker.io>
Wide diagrams still shrink to the chat column inline. An expand control opens a dialog where wheel zooms and drag pans, so chat scroll no longer fights the diagram. Co-authored-by: paul <paul@recker.io>
The first layout pass ran before Base UI put the overlay scene in the DOM, so wide diagrams opened at 100% instead of shrinking to the viewport. Fit and wheel listeners now bind when the scene element actually exists. Co-authored-by: paul <paul@recker.io>
The detail view cloned mermaid SVG without rewriting its stylesheet selectors, so nodes fell back to black fills and inherited black label text. Zoom also scaled a shrink-to-fit SVG around the wrong origin and could throw the diagram off-screen. The overlay now remaps theme CSS, pins the SVG to its viewBox size, treats fit as 100%, zooms around the diagram center, and keeps a sliver of the diagram in view. Co-authored-by: paul <paul@recker.io>
…orrect Review on pingdotgg#7497 found cache-key collisions, overlay CSS rewriting hex colors, portal-dead chrome classes, and mermaid remounting on every streamed token. Cache by full source, remap stylesheet selectors only, use ghost-muted overlay actions, and keep markdown renderers stable via refs. Cursor Grok 4.6 Co-authored-by: paul <paul@recker.io>
The expand overlay is portaled out of .chat-markdown, so it missed the custom-theme bridge that switches inline mermaid/code blocks to --code-background. Use a portal-safe html[data-theme-id] selector so the expanded diagram sits on the same surface as the preview. Cursor Grok 4.6 Co-authored-by: paul <paul@recker.io>
The streaming/idle mermaid fence is a bare pre, so it missed the --font-size-code token that other chat code surfaces use. Route it through the same preference so the source does not jump size when the diagram settles. Cursor Grok 4.6 Co-authored-by: paul <paul@recker.io>
The RenderErrorBoundary fallback used the mermaid error surface without data-markdown-copy, so selecting it dropped the block instead of copying the fence. Pass the same mermaid source payload the other mermaid states already use. Cursor Grok 4.6 Co-authored-by: paul <paul@recker.io>
Mermaid defaults htmlLabels to true, so diagrams could emit foreignObject HTML even with securityLevel strict. Turn htmlLabels off and lock it, strip foreignObject/object and data:text/html URLs, remap quoted url(#id) and single-quoted ids for the overlay, keep a zoomed diagram on-canvas across resizes, and seed the inline preview from the SVG cache so a remount does not flash source. Cursor Grok 4.6 Co-authored-by: paul <paul@recker.io>
Strip unclosed dangerous SVG tags, keep pending source dim under custom themes, and refit the overlay when a streaming diagram's canvas changes after pan/zoom. Co-authored-by: paul <paul@recker.io>
83de423 to
7a8deff
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7a8deff. Configure here.
| } | ||
| }); | ||
|
|
||
| return svg; |
There was a problem hiding this comment.
Cached SVG ID collisions
Medium Severity
renderMermaidSvg caches the sanitized SVG with bake-in DOM ids from nextMermaidDomId, and inline mounts inject that string unchanged. A second fence with the same source+theme reuses those ids, so markers, gradients, and #t3-mermaid-… theme selectors collide in the document. The overlay already remaps ids via prepareMermaidOverlaySvg; inline previews do not.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7a8deff. Configure here.
| event.preventDefault(); | ||
| if (sceneRef.current) fitToScene(sceneRef.current); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Overlay zoom ignores IME
Low Severity
The new overlay onKeyDown handles printable keys (+, =, -, _, 0) and always preventDefaults without bailing on event.isComposing. That violates the project rule for new keyboard handlers that intercept printable keys: they need an isComposing guard so IME composition is not disrupted.
Triggered by learned rule: Keyboard shortcut handlers: check isComposing and use correct -popup data-slot selectors
Reviewed by Cursor Bugbot for commit 7a8deff. Configure here.
There was a problem hiding this comment.
One ownership finding on the new overlay CSS. Everything else (shared Button/Dialog/Tooltip composition, @variant dark usage, the themed --code-background bridge, and the chat-chrome toolbar treatment on the inline code block) looks consistent with the existing chat markdown surfaces.
Posted via Macroscope — UI Consistency
| .chat-markdown-mermaid-overlay-scene { | ||
| position: relative; | ||
| flex: 1; | ||
| min-width: 0; | ||
| min-height: 0; | ||
| overflow: hidden; | ||
| overscroll-behavior: none; | ||
| touch-action: none; | ||
| cursor: grab; | ||
| user-select: none; | ||
| /* Default palette matches the inline mermaid code block. Custom themes tint | ||
| --secondary/--input with the accent, so the overlay uses --code-background | ||
| via html[data-theme-id] .chat-markdown-mermaid-overlay-scene. */ | ||
| background: var(--secondary); | ||
|
|
||
| @variant dark { | ||
| background: color-mix(in srgb, var(--input) 32%, transparent); | ||
| } | ||
| } |
There was a problem hiding this comment.
These declarations are single-owner presentation for MermaidOverlay's scene, and they restate in global CSS what the equivalent inline surface already expresses at its owner: MarkdownCodeBlock's root carries bg-secondary ... dark:bg-input/32 in ChatMarkdown.tsx, with only the themed override living here. Splitting the same palette/geometry across two owners is what lets the inline block and the overlay drift apart on the next tweak.
Suggest keeping only the html[data-theme-id] .chat-markdown-mermaid-overlay-scene bridge (line 1788) in index.css and moving the rest onto the elements in MermaidOverlay.tsx: scene → relative flex-1 min-w-0 min-h-0 overflow-hidden overscroll-none touch-none cursor-grab select-none bg-secondary dark:bg-input/32 data-[panning=true]:cursor-grabbing, canvas → absolute top-0 left-0 origin-top-left. The unlayered themed rule still outranks those utilities, exactly as it does for .chat-markdown-codeblock today. .chat-markdown-mermaid-overlay-canvas svg should stay global since it styles the injected SVG.
Posted via Macroscope — UI Consistency
2 participants
What Changed
Adding Mermaid chart rendering in the chat UI with direct preview and a larger view for zoom and pan.
Why
Claude's output became unreadable, thus asking for visualizations help. Always copying the mermaid code between T3Code and mermaid.live is not ideal
UI Changes
Screen.Recording.2026-08-19.at.09.38.24.mov
Checklist
Note
Medium Risk
Injects mermaid-generated SVG via innerHTML; sanitization and strict mode reduce XSS risk but this is still untrusted model output in the DOM. New mermaid dependency and streaming remount work also affect chat rendering.
Overview
Chat markdown now renders
mermaid/mmdfences as diagrams on web and desktop (mobile still shows source). Users can toggle source vs diagram, copy the fence, and expand into a pan/zoom overlay.Rendering is lazy, LRU-cached, and initialized in mermaid strict mode with extra SVG sanitization. Streaming parse failures keep the last good diagram.
ChatMarkdownreads streaming text from refs so diagram state is not remounted every token.Copying a rendered (or failed) diagram pastes the original mermaid fence, not SVG.
Reviewed by Cursor Bugbot for commit 7a8deff. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add mermaid diagram rendering, zoom/pan overlay, and source copy to chat markdown
MarkdownCodeBlockgains a toolbar toggle between diagram and source code, plus an expand button; wrap-lines control is hidden in diagram view.ChatMarkdownnow uses refs (textRef,isStreamingRef, link-metadata maps) to prevent ReactMarkdown remounts during streaming, preserving diagram state across token updates.data-markdown-copyfrom mermaid surfaces so copying a rendered diagram yields the original fence source, not raw SVG.MarkdownCodeBlockadds three new optional props (diagram,canExpandDiagram,onExpandDiagram); existing callers that render code blocks without these props are unaffected, but any code relying on the absence of adata-mermaid-viewattribute or the wrap-lines control always being present in theprerenderer path should be checked.Macroscope summarized 7a8deff.