Skip to content

Add Mermaid rendering to Markdown - #1929

Merged
Astro-Han merged 12 commits into
apache:mainfrom
ARE404:agent/mermaid-rendering
Aug 3, 2026
Merged

Add Mermaid rendering to Markdown#1929
Astro-Han merged 12 commits into
apache:mainfrom
ARE404:agent/mermaid-rendering

Conversation

@ARE404

@ARE404 ARE404 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What changed

  • render settled mermaid fenced code blocks as diagrams in the shared Markdown surface
  • lazy-load Mermaid so ordinary Markdown and application startup do not pay the runtime cost
  • follow the resolved light/dark theme and re-render diagrams when it changes
  • keep source visible while streaming and when parsing fails or exceeds limits
  • add localized fallback/status copy and responsive diagram styling
  • add unit, fake-backend fixture, and Electron end-to-end coverage
  • refresh the production third-party notice inventory

Why

The Astryx Markdown pipeline previously treated Mermaid fences as ordinary code blocks. This adds the missing product integration while keeping assistant-authored diagram input behind explicit rendering and security boundaries.

Safety and performance

  • Mermaid uses securityLevel: strict, HTML labels are disabled, and bind functions are never attached
  • source is capped at 20,000 characters and graph edges at 500
  • renders are serialized to bound concurrent layout work
  • Mermaid is dynamically imported only after a completed fence is present

Validation

  • npm --workspace @maka/ui run typecheck
  • npm --workspace @maka/ui test (238 tests)
  • targeted FakeBackend suite (7 tests)
  • npm --workspace @maka/desktop run build:with-deps
  • targeted Electron Playwright Mermaid journey
  • Biome lint/format, third-party notice check, dead CSS check, and git diff --check
  • production dependency audit found no Mermaid-related advisories (the existing MCP/Hono moderate advisory remains unchanged)

@ARE404
ARE404 marked this pull request as ready for review August 3, 2026 08:36
@Astro-Han

Copy link
Copy Markdown
Contributor

The Mermaid integration point is sound. Astryx 0.2.0 does not ship a Mermaid or generic diagram renderer, and Markdown.components.code is the intended seam for custom fenced-code rendering. Keeping the full Mermaid package behind a dynamic import also makes sense here. I would not replace it with Mermaid Tiny, internal diagram imports, a Web Worker, or server-side rendering.

The security path looks reasonable too: securityLevel: 'strict', HTML labels disabled, no bindFunctions, and Mermaid's own DOMPurify pass before the SVG reaches React. Serializing initialize + render is necessary because Mermaid configuration is module-global.

I think the viewer should still be revised before merge.

  1. P2: fullscreen is visually fullscreen, but it is not modal.

    The expanded branch uses a fixed portal and manually handles body overflow, Escape, and trigger focus restoration. It does not use the native dialog top layer, make the background inert, or contain Tab focus. A keyboard user can tab from the diagram into controls hidden behind the overlay, and a screen reader can continue browsing the AppShell.

    Astryx already has Dialog variant="fullscreen" with native showModal(), scroll locking, Escape handling, and focus restoration. This should use that component instead of maintaining a parallel fullscreen implementation. Astryx documents <dialog> as supported without special handling: browser support.

  2. P2: the current limits are not a real renderer resource boundary.

    The 20,000-character limit is useful, but maxEdges: 500 is Mermaid's default and only protects specific flowchart link creation. It does not limit independent nodes, sequence participants, class-diagram elements, SVG output size, or the number of Mermaid fences in one message.

    A sub-20k flowchart can still contain thousands of unconnected nodes. The Promise queue prevents concurrent renders, but parsing and layout still run on the Electron renderer thread. Unmounted or superseded diagrams also remain in the queue because cleanup only prevents the final setState.

    I would add:

    • a per-Markdown automatic diagram count;
    • a total Mermaid source budget per Markdown;
    • a lower automatic-render threshold;
    • a source fallback with an explicit Render action when a diagram exceeds that threshold.

    A Promise timeout would not fix this because same-thread layout cannot be interrupted.

  3. P2: the viewport creates a touch scrolling dead zone.

    .maka-mermaid-viewport always sets touch-action: none, while the pointer handler intentionally does nothing when the fitted diagram has no overflow. In that common initial state, dragging over the diagram neither pans the diagram nor scrolls the conversation.

    Keep normal vertical scrolling in the fitted state. Disable browser gestures only while the diagram is actually pannable, or use a narrower value such as pan-y where appropriate.

  4. P2: the toolbar has no narrow-container fallback.

    The toolbar is a single row inside an overflow: hidden container. It always contains four buttons and a fixed-width zoom output. Maka does not enforce a large minimum window width, and the conversation can be narrowed further by adjacent panels, so the right-side actions can be clipped.

    This needs a container-level fallback. Hiding the title first is probably enough for the first breakpoint; at smaller widths, keep Expand visible and move secondary zoom/reset actions into the fullscreen view or an overflow menu.

  5. P2: successful rendering removes the accessible source alternative.

    Loading and error states retain the Mermaid source, but the successful state exposes only generic figure and viewport labels. Unless generated Mermaid includes accTitle and accDescr, a screen reader gets node text without a reliable description of edges and groups.

    Keep a View source disclosure in the successful state. That is useful beyond accessibility too: agent-generated diagrams should remain inspectable.

  6. P3: add one hostile-input regression fixture.

    The current E2E covers a normal diagram. A small security fixture containing a click ... javascript: directive, HTML event attributes, and an init/frontmatter override would lock the trust boundary down. The assertion should inspect the rendered DOM for executable or navigable nodes. I did not find evidence of a current XSS, so this is test hardening rather than a security finding.

A simpler component boundary would look like this:

flowchart TD
    A["Astryx Markdown<br/>components.code"] --> B{"Settled Mermaid fence?"}
    B -- No --> C["Astryx CodeBlock"]
    B -- Yes --> D{"Within automatic render budget?"}
    D -- No --> E["Source + explicit Render action"]
    D -- Yes --> F["Inline Mermaid<br/>fit-to-width"]
    F --> G["View source disclosure"]
    F --> H["Expand"]
    H --> I["Astryx Dialog<br/>variant=fullscreen"]
    I --> J["Astryx Toolbar + IconButton"]
    I --> K["Zoom, reset, and pan"]
Loading

This keeps Mermaid-specific rendering local while letting Astryx own code presentation, toolbar behavior, modal semantics, focus, Escape, and scroll locking. Inline diagrams stay quiet: fit-to-width, View source, and Expand. The complete zoom and pan controls only appear where they have enough room to be useful.

CodeBlock and IconButton are already the right choices. I would also replace the hand-written role="toolbar" wrapper with Astryx Toolbar, which owns the toolbar label, sizing context, keyboard hints, and edge behavior. Astryx Lightbox is not a substitute here because its public media contract is limited to images and video.

中文建议

底层接法可以保留:继续用 Markdown.components.code、动态加载完整 Mermaid,并维持严格安全配置和串行渲染。建议收缩自建 viewer:行内只保留适宽、查看源码和展开;全屏改用 Astryx Dialog,控制栏改用 Toolbar + IconButton。同时补上渲染总预算、触屏滚动、窄宽布局和源码替代。

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the latest head. The previous viewer feedback is addressed: fullscreen now uses the Astryx fullscreen Dialog and Toolbar, fitted diagrams preserve vertical touch scrolling, narrow containers retain the Expand action, successful renders keep a source disclosure, automatic rendering has count/source budgets with an explicit Render fallback, and the hostile-input regression fixture locks down the SVG boundary. CI is fully green. Approving for merge.

One non-blocking P2 follow-up: applyMermaidRenderBudget only recognizes fences that begin after up to three spaces. Astryx also parses Mermaid fences inside Markdown containers such as blockquotes and lists, but those retain prefixes like > during the raw-source scan. As a result, four blockquoted Mermaid fences all remain automatic instead of the fourth becoming deferred, bypassing the per-Markdown three-diagram and total-source budgets.

Please consider enforcing the budget against the parsed code blocks, or extending the scanner to handle Markdown container prefixes, with blockquote/list regression cases. The existing per-diagram hard limit and serialized render queue keep this from blocking the otherwise complete integration.

@Astro-Han
Astro-Han merged commit d9775ff into apache:main Aug 3, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants