Repository navigation
perf(ui): render Markdown diagrams with Pele - #6267
Conversation
7480f54 to
28a20fb
Compare
jackwener
left a comment
There was a problem hiding this comment.
Review result: blocked
Reviewed exact head 28a20fbd673bb580ec30adb5382c13403c108d6f. The migration delivers a substantial first-render and bundle-size improvement, but I found one user-path performance blocker and one attribution blocker.
[P1] Coalesce width-driven Pele renders instead of rendering every ResizeObserver tick
packages/ui/src/mermaid-diagram.tsx:269-337 copies every observed pixel-width change into component state and then synchronously calls Pele's render(). Every mounted diagram therefore performs layout independently for every width event while the window or a split pane is being dragged. There is no debounce, width bucketing, resize-settle gate, or cross-instance coalescing.
I reproduced this through the built Maka theme, Markdown, and MermaidDiagram components in Chromium using a normal Markdown document with three automatically rendered 200-edge flowcharts. Each diagram was below the 4,000-character source limit, their combined source was below 8,000 characters, and three diagrams is the current automatic-render maximum. Ten viewport changes alternating between 900 and 500 pixels produced 66 SVG child mutations and ten renderer-thread long tasks of 241–446 ms; the resize loop took about 5.75 seconds. Current main produced no SVG mutations and no long tasks during the same resize sequence. Suppressing only this component's ResizeObserver callbacks on the PR head removed the repeated stalls, leaving a single 50 ms task, so the width-driven render path is causal.
Please debounce or bucket width changes and render once after resizing settles while retaining the previous SVG, then add a deterministic browser regression that bounds the render count during a resize burst. This needs to be fixed before approval because ordinary resizing with several valid diagrams repeatedly blocks the renderer thread and contradicts the PR's performance goal.
[P2] Record Devin in the generated-code trailer
The PR body says Devin materially implemented the per-source render cache, its regression test, and the output-safety tests. The sole commit currently carries only Generated-by: Codex. CONTRIBUTING.md requires every affected commit to carry a Generated-by: <tool> trailer for material generated content. Please amend the commit so it retains trailers for both applicable tools.
Verification
- The production-component comparison independently confirmed the intended benefit: initial rendering of the same three 200-edge diagrams improved from about 9.37 seconds on current
mainto about 1.57 seconds on this head. - The corresponding minimal component bundle fell from 3,761,952 bytes / 1,040,888 bytes gzip on
mainto 1,001,300 bytes / 346,175 bytes gzip on this head. @maka/coreand@maka/uibuilds passed, as did the 821-test UI suite, all three real-browser diagram tests, eight Desktop palette/Vite contract tests, Biome, andgit diff --check.- I also checked representative flowchart, sequence, state, pie, timeline, ER, git graph, mindmap, C4, packet, architecture, directive, frontmatter,
classDef, andlinkStylesyntax, plus malicious script/HTML/link/style inputs. - Exact-head hosted CI, dependency audit, packaging, and platform checks are green. The PR is currently open, non-draft, and mergeable against current
main.
I found no other P0–P2 issues in the reviewed head.
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
Replace Mermaid.js with Pele while preserving source view, fullscreen, clipboard export, render budgets and safe SVG output. Use native zoom and CSS themes, retain bounded caches with one template per source, and embed fonts in PNG exports. Add real browser coverage and regression tests for cache eviction and untrusted diagrams, simplify the integration through ablation, and refresh the bundled dependency notices. Refs apache#6257 Generated-by: Codex Generated-by: Devin
Pele layout is synchronous, so laying every diagram out again on each ResizeObserver tick blocked the renderer while a window or pane was dragged. Apply width changes only after resizing settles; the mounted SVG keeps scaling with CSS meanwhile. Also skip relayout when the container is at least as wide as the diagram's natural layout or when the diagram does not adapt to width, matching Pele's own mount(). Generated-by: Codex Generated-by: Devin Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
28a20fb to
7eb192f
Compare
|
Response to review 5646841614 (head is now Resize relayout — accepted, graded P2 rather than P1. The mechanism is real. At
Generated-by trailer — accepted. Verification:
|
jackwener
left a comment
There was a problem hiding this comment.
Re-review result: no findings
Reviewed exact head 7eb192f71bfd9d11c8bfe5c309bffd9b34b4dd56. Both findings from my review of 28a20fbd673bb580ec30adb5382c13403c108d6f are closed, and I found no new P0–P2 issues.
- The resize path now retains the mounted SVG, coalesces observer notifications behind a 150 ms settle interval, and only runs Pele again when the final container is narrower than the diagram's natural layout and the diagram actually adapts to width. In a built production-component Chromium probe with three automatically rendered 200-edge flowcharts, a 40-step width drag produced one three-diagram relayout batch instead of one batch per step. Removing only the settle gate from the generated component made the new regression fail and exposed 17 intermediate SVG replacements.
- The two commits now retain both applicable
Generated-by: CodexandGenerated-by: Devintrailers, matching the PR's updated AI-use declaration.
The head is directly based on current main (a4b5f86c5256044d063a62045754e4a3411a8426), so the reviewed tree is also the current merge result. @maka/core and @maka/ui builds passed, as did the complete 823-test UI suite, all three real-Chromium Mermaid tests, eight Desktop palette/Vite contract tests, changed-file Biome, and git diff --check. The production probe rendered all three large diagrams initially in about 1.34 seconds. Exact-head hosted checks are successful.
No native Windows clipboard export, packaged Electron restart, or exhaustive compatibility pass over every Mermaid extension was repeated in this incremental review.
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
jackwener
left a comment
There was a problem hiding this comment.
Approval
Approved exact head 7eb192f71bfd9d11c8bfe5c309bffd9b34b4dd56. I found no unresolved P0–P2 issues.
Both findings from the earlier review are closed. The resize path keeps the mounted SVG visible, coalesces a burst behind a 150 ms settle interval, and only lays out again when the final container is narrower than the diagram's natural layout and the diagram adapts to width. A built production-component Chromium probe with three automatically rendered 200-edge flowcharts reduced a 40-step drag to one three-diagram relayout batch; removing the settle gate made the regression fail with 17 intermediate SVG replacements. Both commits also retain the applicable Generated-by: Codex and Generated-by: Devin trailers.
The head is directly based on current main a4b5f86c5256044d063a62045754e4a3411a8426, so no separate merge delta exists. @maka/core and @maka/ui builds, the complete 823-test UI suite, all three real-Chromium Mermaid tests, eight Desktop palette/Vite contract tests, changed-file Biome, and git diff --check passed. The exact-head required test check and all applicable hosted checks are successful. The PR is open, non-draft, mergeable, and has no review threads.
No merge or contributor-source change was performed.
Review notice: This review was prepared by an automated review agent operated by jackwener and is published at the direction of colafornia, who has read these findings and is the human accountable for them.
me2seeks
left a comment
There was a problem hiding this comment.
Reviewed exact head 7eb192f. No blocking findings.
Checked the Pele migration, width-settle relayout and per-source cache, instance SVG references, untrusted-output limits, theme handling, PNG export, dependency changes, and packaged attribution. The resize path retains the mounted SVG during a burst and coalesces layout after 150 ms.
Independent local validation in an isolated checkout: core/UI builds; all 823 UI tests; all 3 real Chromium diagram tests; 2 Desktop Vite/KaTeX contract tests; 2 third-party notice tests; git diff --check. Exact-head hosted checks are successful (the Eval job is skipped), the PR is mergeable, and there are no review threads.
Native OS clipboard integration and exhaustive Mermaid extension compatibility were not independently tested.
Automated review prepared and submitted by Codex at me2seeks's explicit direction. This does not replace the independent human review required by CONTRIBUTING.md.
English
Summary
Markdown diagrams in conversations, Daily Review, and Markdown previews now render with Pele 1.9.0 instead of Mermaid.js. Diagrams render faster, the desktop app ships less JavaScript, and a theme change no longer renders diagrams again.
Visual comparison
Left: Mermaid.js; right: Pele. Both sides use the same source, width, and real Maka component and theme. The examples adapt Pele's flowchart and sequence examples to Maka scenarios.
The 1× recording renders a 128-node, 260-edge diagram of eight eval batches side by side. In this single run, component-to-next-paint time was 613.5 ms with Mermaid.js and 93.5 ms with Pele.
Diagram source · Capture method and example attribution
Verification
Tested head:
7eb192f71, rebased ontoa4b5f86c5.npm run build,npm run typecheck,npm run lint,npm run format:check,npx knip --workspace packages/ui, andnpx knip --workspace apps/desktoppassed.npm --workspace @maka/ui run test:dist: 823 tests passed. Each regression test below fails when its protection is removed:foreignObject, or images, and a 501-edge graph is rejected.node --test packages/ui/scripts/mermaid.browser.test.mjs: 3 real Chromium tests passed. They cover flowchart, sequence, state, pie, and timeline diagrams; Chinese, subgraph, and multiline labels; narrow layouts; zoom and pan; fullscreen and source; theme changes; syntax and resource failures; and PNG colors and fonts. CI does not run this script.28a20fbd6produced 225 SVG mutations, one relayout of every diagram per step. This head produced 12, one relayout per diagram after resizing settled. These local diagrams take about 15 ms each to lay out, so they did not reproduce the 241–446 ms renderer tasks that the review of28a20fbd6reported for heavier diagrams.Earlier validation on
28a20fbd6, which has the same tree as4ae8d4caa:npm ci --ignore-scripts, followed by the repository's dependency patches, andnpm --workspace @maka/ui run buildpassed in an isolated checkout.Performance
Baseline
a509747d6uses Mermaid.js 11.17.2;ae3240952uses Pele 1.9.0. Each renderer ran five alternating times in fresh Chromium pages with production esbuild bundles, rendering the same 101-node, 100-edge Chinese flowchart in a 640 px container. Both completed 5/5 runs. The later cache change does not alter the first-render path. The later narrow-container check adds Pele passes to a cold render in a narrow container; Verification reports its measured effect.The component path covers script execution, Maka Theme +
MermaidDiagrammounting, SVG completion, and the next animation frame. Neither path measures network transfer or Electron startup. These are local macOS arm64 results (12 logical CPUs, Node 24.22.1, no CPU throttling), not a speed guarantee.Raw samples · Vite output sizes · Benchmark script (not included in this PR)
Implementation details
Pele uses its own layout and styling. This change does not promise identical Mermaid output or support for every Mermaid extension.
Fixes #6257
Review focus
libsetting.tsconfig.base.jsonnow setslib: ["ES2024", "DOM", "DOM.Iterable"]. Before this change, ES2023+ library types reachedpackages/uiby accident: Mermaid's types importtype-festwithout declaring it, and that import resolved to a hoistedtype-fest@0.13.1whosesource/basic.d.tscontains/// <reference lib="esnext"/>. Without Mermaid,findLastIndexandPromise.withResolversfail type checking inpackages/uiand inapps/desktop/tsconfig.storybook.json. ES2024 is the lowest setting that passes.pele@1.9.0was published on 2026-10-22 with npm provenance from GitHub Actions. The project's first release was on 2026-10-06.@maka/ui, rather than relying on Mermaid's former root-level dependency.Remaining before ready for review
ResizeObserver loop completed with undelivered notificationswarning. The same Workhub conversation produced two warnings on upstreamac1d288c4and two on this branch, with no Mermaid diagrams present. The trace identified synchronous virtua list-resize updates; this reproduction predates the migration. Published builds were not tested.MaxListenersExceededWarningreproduction on7eb192f71. Restarting with the existing development profile and switching history sessions rapidly 20 times did not trigger it; the listener count peaked at 10. Two additional read-only subscriptions throughmaka.sessions.subscribeEventsraiseddestroyedlisteners from 9 to 11, triggering the warning during listener registration inRuntimeHostSessionObservationRegistry.observe(). Unsubscribing returned the count to 9. This registration and cleanup path is unchanged from pre-migration basea4b5f86c5. This identifies a listener-threshold trigger; it does not establish a persistent leak. The original UI sequence and an upstream runtime comparison remain unverified.AI use
Tool(s) and scope: OpenAI Codex implemented the migration, tests, benchmarks, visual captures, and the first description under user direction. Devin reviewed the change; implemented the per-source render cache, its regression test, and the output-safety tests; removed the benchmark script and an unused CSS rule; and re-captured the review screenshots and recording with English examples. After review, Codex implemented the narrow-container relayout check, and Devin made relayout wait until resizing settles and wrote its regression test. Affected commits carry
Generated-bytrailers; keep them when squashing.Checklist
Does this PR entail a change in behavior?
中文
概述
会话、每日回顾和 Markdown 预览中的图表改由 Pele 1.9.0 渲染,不再使用 Mermaid.js。图表渲染更快,桌面应用分发的 JavaScript 更少,切换主题时也不再重新渲染图表。
视觉对比
左侧为 Mermaid.js,右侧为 Pele。两侧使用相同的源码、宽度,以及真实的 Maka 组件与主题。示例由 Pele 官方的流程图和时序图示例改编为 Maka 场景。
1× 原速录屏并排渲染一张包含 8 个评测批次、128 个节点、260 条连线的图表。此次单次运行中,从组件调用到下一绘制帧的耗时为 Mermaid.js 613.5 ms、Pele 93.5 ms。
图表源码 · 采集方法与示例来源
验证
已验证提交:
7eb192f71,基于a4b5f86c5完成 rebase。npm run build、npm run typecheck、npm run lint、npm run format:check、npx knip --workspace packages/ui和npx knip --workspace apps/desktop均通过。npm --workspace @maka/ui run test:dist:823 项测试通过。下列每项回归测试在移除对应保护后都会失败:foreignObject和图片,并拒绝 501 条连线的图。node --test packages/ui/scripts/mermaid.browser.test.mjs:3 项真实 Chromium 测试通过,覆盖流程图、时序图、状态图、饼图和时间线图;中文、子图和多行标签;窄容器;缩放与平移;全屏与源码;主题切换;语法与资源限制失败;PNG 颜色与字体。CI 不运行此脚本。28a20fbd6产生 225 次 SVG 变更,即每一步都重新布局全部图表;本提交产生 12 次,即停止调整后每张图重新布局一次。本地这些图每张布局约 15 ms,因此没有复现审阅意见针对28a20fbd6报告的、更复杂图表下 241–446 ms 的渲染线程长任务。此前在
28a20fbd6上完成的验证(与4ae8d4caa的文件树相同):npm ci --ignore-scripts安装,随后应用仓库的依赖补丁,npm --workspace @maka/ui run build通过。性能
基线
a509747d6使用 Mermaid.js 11.17.2;ae3240952使用 Pele 1.9.0。每个引擎在新建的 Chromium 页面中交替运行五次,使用生产模式 esbuild 包,在 640 px 容器中渲染同一张 101 个节点、100 条连线的中文流程图。两侧均 5/5 成功。之后的缓存修改不影响首次渲染路径。之后加入的窄容器判断会在窄容器冷渲染时增加 Pele 渲染次数,实测影响见“验证”一节。组件路径包含脚本执行、Maka Theme 与
MermaidDiagram挂载、SVG 完成及下一动画帧。两条路径都不测量网络传输或 Electron 启动。以上是本机 macOS arm64 结果(12 个逻辑核心、Node 24.22.1、无 CPU 降速),不代表性能保证。原始数据 · Vite 产物大小 · 基准脚本(未包含在本 PR 中)
实现细节
Pele 使用自己的布局与样式。此次迁移不承诺与 Mermaid 输出完全一致,也不承诺兼容全部 Mermaid 扩展。
Fixes #6257
审阅重点
lib。tsconfig.base.json现在设置了lib: ["ES2024", "DOM", "DOM.Iterable"]。此前packages/ui获得 ES2023 及以上的类型库纯属偶然:Mermaid 的类型文件导入了未声明依赖的type-fest,该导入解析到提升到根目录的type-fest@0.13.1,其source/basic.d.ts含有/// <reference lib="esnext"/>。移除 Mermaid 后,findLastIndex和Promise.withResolvers会在packages/ui和apps/desktop/tsconfig.storybook.json中类型检查失败。ES2024 是能通过检查的最低设置。pele@1.9.0于 2026-10-22 发布,带有来自 GitHub Actions 的 npm 来源证明。该项目的首个版本发布于 2026-10-06。@maka/ui解析 CSS,不再依赖原先由 Mermaid 带入根目录的间接依赖。标记可审阅前的待办
ResizeObserver loop completed with undelivered notifications警告。同一个 Workhub 会话在上游ac1d288c4与本分支各出现两次警告,页面均没有 Mermaid 图表。调用追踪确认触发路径为 virtua 列表尺寸变化时的同步更新;此次复现的问题在迁移前已存在。尚未测试发布版。7eb192f71上定位 WebContentsMaxListenersExceededWarning的受控复现路径。使用原开发数据重启并快速切换历史会话 20 次,均未触发警告,监听器峰值为 10。通过maka.sessions.subscribeEvents增加两个只读订阅后,destroyed监听器从 9 增至 11,在RuntimeHostSessionObservationRegistry.observe()注册监听器时触发警告;取消订阅后回到 9。该注册与清理路径相较迁移前基线a4b5f86c5没有改动。此结果确认了监听器数量超过默认阈值的触发路径,尚不能据此认定存在持续泄漏。最初触发警告的 UI 操作序列及上游运行时对照仍未验证。AI 使用
工具与范围:OpenAI Codex 在用户指导下完成迁移、测试、性能基准、视觉采集及最初的说明。Devin 审阅了改动,实现了按源码缓存渲染结果、对应的回归测试和输出安全测试,删除了基准脚本和一条未使用的 CSS 规则,并用英文示例重新采集了审阅截图与录屏。审阅后,Codex 实现了窄容器下的重新布局判断,Devin 让重新布局等到停止调整大小后再执行,并编写了对应的回归测试。相关提交带有
Generated-by标记,压缩合并时请保留。检查清单
此 PR 是否改变行为?