Skip to content

perf(ui): scope prompt rail observation to mounted turns - #4203

Open
Astro-Han wants to merge 1 commit into
mainfrom
perf/prompt-rail-turn-membership-observer
Open

perf(ui): scope prompt rail observation to mounted turns#4203
Astro-Han wants to merge 1 commit into
mainfrom
perf/prompt-rail-turn-membership-observer

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Prompt Anchor Rail used to observe the entire chat scroll subtree for childList changes. Streaming Markdown, math, and Mermaid work therefore woke the rail even when the mounted Turn membership had not changed.

This change keeps the existing observer and incremental membership logic, but attaches it to the direct parent of the existing keyed [data-virtual-turn-id] nodes and scans only that mounted Turn list. It does not change the transcript range/virtualizer, streaming preprocessing, Selection Quote, Mermaid, or scroll ownership.

The performance study used the shared experimental baseline 2fa1f78ec5746ed4c2db5c49d38987a34ae9122f. This branch is now rebased onto main at 593efb9df79d66ad0dad8033884042dbffd754fe, which includes the independent WorkHub CI stabilization in #4210. No intervening main commit changed packages/ui/src/prompt-anchor-rail.tsx, so the measured candidate diff and its natural owner are unchanged.

Performance evidence

Method and declared gate

  • Frozen production renderer A: baseline SHA above, without this diff.
  • Frozen production renderer B: the same SHA, with only this diff.
  • Electron 43.4.1 / Chromium 150.0.7871.224, 1000 x 760 viewport, same machine, fixture, dependencies, and production build.
  • Seven interleaved pairs: A-B, B-A, A-B, B-A, A-B, B-A, A-B.
  • Read-only CDP harness wrapped MutationObserver, IntersectionObserver, ResizeObserver, and scroll callbacks, observed long tasks/LoAF, collected CDP CPU profiles and performance metrics, and forced GC only before the memory sample. It did not write product state and is not included in this PR.
  • Predeclared primary metric: Prompt Rail MutationObserver callback CPU during a real Runtime Host fake-backend streaming update.
  • Acceptance gate: median improvement >=20%, at least 6/7 paired wins, improvement beyond noise, no key correctness/performance/memory regression >10%, and no increase in architectural authority/state/representation/schedulers.

Primary raw pairs

Pair Order A callback CPU B callback CPU A -> B callbacks A -> B mutation records Winner
1 A-B 453.7 ms 0.0 ms 153 -> 0 306 -> 0 B
2 B-A 423.3 ms 0.0 ms 153 -> 0 302 -> 0 B
3 A-B 408.8 ms 0.0 ms 153 -> 0 306 -> 0 B
4 B-A 358.0 ms 0.0 ms 152 -> 0 300 -> 0 B
5 A-B 338.8 ms 0.0 ms 153 -> 0 306 -> 0 B
6 B-A 401.4 ms 0.0 ms 153 -> 0 302 -> 0 B
7 A-B 304.0 ms 0.0 ms 152 -> 0 300 -> 0 B

Median / MAD: 401.4 / 43.4 ms -> 0.0 / 0.0 ms, a 100% reduction with 7/7 paired wins. The lower edge of baseline median minus MAD was still 358.0 ms, while every candidate run had zero Prompt Rail wakeups, so the result is outside measured noise.

Matrix attribution and guardrails

Exploratory attribution selected Prompt Rail as the single coherent owner: its broad subtree MutationObserver dominated real streaming and rich-content mutation CPU across the matrix. Selection Quote's document scroll handler dominated only the active-selection scenario, while Mermaid theme/resize callbacks were small. No Selection Quote or Mermaid change is bundled here.

Metric A median / MAD B median / MAD Change
Prompt Rail MO CPU, real stream (primary) 401.4 / 43.4 ms 0.0 / 0.0 ms 100% better
Prompt Rail MO CPU, long-scroll Turn membership 796.5 / 16.6 ms 781.6 / 14.7 ms 1.87% better
Long-scroll frame interval P95 50.0 ms 50.0 ms unchanged
Long-scroll frame interval P99 66.7 ms 59.3 ms 11.09% better
Long-scroll wall time 3,895 ms 3,878 ms 0.44% better
Active-quote frame interval P95 249.7 ms 250.3 ms 0.24% worse
Active-quote frame interval P99 392.6 ms 383.4 ms 2.34% better
Active-quote wall time 10,473 ms 10,369 ms 0.99% better
Selection Quote document-scroll callback CPU 1,186.9 / 148.8 ms 1,204.6 / 555.0 ms 1.49% worse
Real-stream wall time 7,044 ms 7,001 ms 0.61% better
Four-Mermaid render wall time 14,936 ms 14,933 ms 0.02% better
Theme-toggle wall time 2,090 ms 2,080 ms 0.48% better
Resize wall time 573 ms 570 ms 0.52% better
Heap after GC 73,041,280 / 103,004 B 73,036,392 / 50,272 B 0.01% better
DOM nodes 16,251 / 0 16,251 / 0 unchanged

The long-scroll membership path remains active (40 -> 39 median callbacks) because real mounted Turn additions/removals still need rail bookkeeping; the removed work is specifically unrelated descendant churn. The candidate did not shift that cost into Selection Quote or the virtualizer.

For Mermaid attribution, an inspected candidate run sampled 12.42 ms of Mermaid/Markdown/Dagre self CPU during four-diagram rendering, 3.86 ms during theme toggle, and 2.52 ms during resize. Each diagram's viewport ResizeObserver fired six times during resize with effectively 0.00 ms instrumented callback CPU; paired wall medians were unchanged. Mermaid was therefore not the dominant owner.

All 14 formal runs preserved the product contracts: quote visible before and hidden after scroll, streaming tail distance 0, one mounted streamed Turn, four Mermaid diagrams producing 32 nested SVG nodes, theme rerender observed, and four layouts ready after resize. No key regression exceeded 10%; the largest measured regression was 1.49% in Selection Quote callback CPU.

Entropy ledger

Dimension Before After Net
Session/Turn/Transcript/Lifecycle authorities 1: Runtime Host 1: Runtime Host no change
Persistent product state added by this slice 0 0 no change
Prompt Rail ephemeral derived collections 2: idByElement, visible 2: same collections no change
Message/data representations added by this slice 0 0 no change
Prompt Rail observers/schedulers 1 IO + 1 broad subtree MO + existing scroll rAF 1 IO + 1 direct Turn-membership MO + same scroll rAF same count, narrower owner
Code root-wide scan and arbitrary descendant subscription keyed mounted-Turn-list scan and direct membership subscription 4 added, 3 removed

Added concepts: none. mountedTurnList is a local reference to the existing React/virtualizer keyed Turn membership seam, not state, a cache, a protocol, or a scheduler. It is necessary to name the natural subscription owner.

Deleted concepts/paths: arbitrary Markdown/math/Mermaid/other descendant mutation wakeups and root-wide initial Turn scans. The replaced subtree: true path is deleted in this same diff; there is no compatibility path or follow-up cleanup. Authority, state, representation, and scheduler counts do not increase, while the observer's semantic reach decreases, so the slice is net entropy-reducing despite the one-line LOC increase.

The required one-way architecture remains intact: Runtime Host owns transcript facts; Desktop range remains an immutable bounded projection; React supplies stable Turn identity; useChatScroll remains the only scroll authority; Astryx remains the streaming scheduler; Chromium retains offscreen layout/paint ownership; the CDP harness remains measurement-only.

Verification

  • npm run build — production build passed.
  • npm --workspace @maka/ui run test — 269 passed.
  • cd apps/desktop && npx playwright test --config e2e/playwright.config.ts e2e/prompt-rail.spec.ts — 7 passed on the final candidate.
  • Quote selection + transcript scroll + accessibility targeted E2E — 13 passed, 1 existing test.fixme skip (a drag begun while the answer streams still offers a quote, documented upstream Astryx node-rebuild limitation).
  • npx @biomejs/biome check packages/ui/src/prompt-anchor-rail.tsx — passed.
  • git diff --check — passed.
  • Fourteen frozen production A/B runs — completed with the raw primary values above.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex performed the CDP attribution study, implemented the scoped observer change, ran the paired A/B analysis, and prepared this evidence. The commit includes a Generated-by: Codex trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

The read-only performance acceptance harness fails the declared primary gate on the frozen baseline and passes it on the candidate; existing product E2E covers Prompt Rail behavior, scroll anchoring, quote selection, theme/resize, and accessibility.

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/XS Under 10 readable lines label Aug 29, 2026
@Astro-Han
Astro-Han marked this pull request as ready for review August 29, 2026 16:55

@ARE404 ARE404 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.

Approving perf(ui): scope prompt rail observation to mounted turns at head 6476be8e.

I reviewed the single-file change and verified the scoped observation is coverage-preserving:

  • In the virtualized DOM, [data-virtual-turn-id] is set on .maka-turn-virtual-item (chat-view.tsx:734), so mountedTurnList resolves to the .maka-chat-message-list that hosts all mounted turns; every .maka-turn-virtual-item and its [data-turn-id] descendants remain inside it. Scoping the IntersectionObserver iteration and the MutationObserver to this container therefore observes the same turn set as before.
  • Turn mount/unmount is a direct childList change on the message list under virtualization, so childList without subtree still catches it. One [P3] note on this is inline (prompt-anchor-rail.tsx:394).
  • No P0–P2; 0 unresolved review threads.

Note on CI: the test check was still settling on this exact head at review time (run 33264030950, in progress); I'm approving on the code/findings as requested, and the prompt-rail e2e covers the mounted/virtualized path.

简体中文

批准 perf(ui): scope prompt rail observation to mounted turns,head 6476be8e
审查单文件改动,确认收缩观察范围不丢覆盖:虚拟化 DOM 里 data-virtual-turn-id 设在 .maka-turn-virtual-item(chat-view.tsx:734),所以 mountedTurnList 解析到承载所有挂载 turn 的 .maka-chat-message-list,每个 .maka-turn-virtual-item 及其 [data-turn-id] 后代仍在其内——IntersectionObserver 迭代和 MutationObserver 都收缩到这个容器,观察的 turn 集合与改前一致。虚拟化下 turn 的挂载/卸载是对消息列表的直接 childList 变化,所以 childList(无 subtree)仍能捕获。行内一条 [P3](prompt-anchor-rail.tsx:394)。无 P0–P2;0 未解决线程。CI 备注:approve 时 test 仍在 exact head 上跑(run 33264030950 进行中),我基于代码/结论批准,prompt-rail e2e 覆盖了挂载/虚拟化路径。

Comment thread packages/ui/src/prompt-anchor-rail.tsx

@zhiiw zhiiw 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.

Reviewed at exact head 6476be8e (verified unchanged at review time).

Four-line observer scoping, verified rather than assumed:

  • The structural assumption holds in the real DOM: chat-view.tsx renders every [data-virtual-turn-id] wrapper as a flat sibling (alongside the virtual spacers) under one scroll-content parent, so querySelector('[data-virtual-turn-id]').parentElement is the single element whose direct childList changes exactly when mounted-Turn membership changes. The membership bookkeeping (visitTurnElements over added/removed nodes) still finds the nested [data-turn-id] elements inside each added wrapper, so observe/unobserve coverage is unchanged.
  • The eliminated wakeups were pure waste: with subtree: true on the scroll root, every streaming Markdown/math/Mermaid mutation anywhere in the transcript fired the callback, which then re-scanned and re-resolved for no membership change — matching the measured 152–153 callbacks and ~300–450 ms of CPU per stream, now 0.
  • Mount ordering is safe: the effect re-runs when turns/railTurns/turnIndexById change, and mountedTurns are mapped in the same React commit, so the wrapper nodes exist when the effect queries for them; the empty→non-empty transition re-runs the effect the same way. Spacer mounts/unmounts still fire the observer, preserving the old settle behavior at a tiny fraction of the old rate.
  • Scope discipline: the change touches only the observation target — transcript range/virtualizer, streaming preprocessing, Selection Quote, Mermaid, and scroll ownership are all untouched, and the rail's own arithmetic (railTurns mapping, jump-hold, safe-area) is unchanged.

The performance evidence is methodologically sound: frozen A/B production renderers at the same SHA, seven interleaved pairs, a predeclared primary metric and acceptance gate, and guardrail metrics reported alongside (long-scroll membership CPU within 2%, frame intervals unchanged). The result (7/7 wins, 100% reduction of the primary metric) is consistent with what the diff predicts.

Executed on a real Windows machine at this head: clean forced rebuild, prompt-anchor-rail unit suite 9/9. The hosted test job (which includes the prompt-rail Playwright spec) was still running at approval time.


Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

简体中文

四行观察范围收窄,全部核实而非假设:chat-view 里所有 [data-virtual-turn-id] 包装节点确实是同一滚动内容父元素下的平铺兄弟节点,所以「第一个 turn 节点的 parent」就是挂载 Turn 成员变化的精确观察点;visitTurnElements 仍能在新增包装节点里找到嵌套的 [data-turn-id],观察覆盖不变。被消灭的唤醒全是浪费:旧 subtree:true 下流式 Markdown/math/Mermaid 的每次变更都触发回调白扫一遍(与实测 152-153 次回调、300-450ms CPU 吻合,现为 0)。挂载时序安全:effect 依赖 turns 变化重跑时节点已提交。范围纪律:虚拟化/流式预处理/Selection Quote/Mermaid/滚动所有权全未动。性能证据方法学扎实(同 SHA 冻结 A/B、七次交错、预先声明的指标与门槛)。本机真 Windows 干净重建 + prompt-anchor-rail 单测 9/9;CI test(含 prompt-rail e2e)批时仍在跑。

@hqhq1025 hqhq1025 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.

Reviewed exact head 6476be8e6d388613225b892902c314b6cec90b0e.

No P0-P3 findings. The change narrows the initial turn scan and membership observer from the full scroll subtree to the Astryx message-list inner container. I verified that chat-view.tsx renders each keyed [data-virtual-turn-id] wrapper as a direct child of that container and that each wrapper contains its [data-turn-id] section, so direct childList additions/removals still drive the existing observe/unobserve bookkeeping while streaming Markdown, math, and Mermaid descendant churn no longer wakes it. The current main merge tree is conflict-free and preserves this DOM contract.

Validation on this head: @maka/ui build and test suite passed (269 tests), desktop production build passed, Prompt Rail Playwright E2E passed (7/7 under Xvfb), Biome passed for the changed file, and git diff --check passed. No production code or tests should be deleted, and no deeper refactor is required. No protected-area effect was identified in this UI-only diff.

Residual gap: the hosted test check is still running, and I did not independently reproduce the PR's seven-pair CPU benchmark; the reviewed code path and local behavioral coverage are consistent with the claimed optimization. This revision is ready from code review, subject to the repository's required checks and independent human review.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I reviewed this at 6476be8e6d388613225b892902c314b6cec90b0e. No P0, P1, P2, or P3.

The Prompt Rail observer used to watch the whole chat scroll subtree, so streaming Markdown, math, and Mermaid mutations woke it even when mounted Turn membership had not changed. This keeps the same IntersectionObserver and add/remove bookkeeping, and attaches the MutationObserver to the parent of the keyed [data-virtual-turn-id] nodes with childList only. Added wrappers still carry their nested [data-turn-id] section, so visitTurnElements still finds them. I found no leftover subtree: true path on this observer, and the change does not touch scroll ownership, Selection Quote, or the virtualizer.

The rail effect still depends on railTurns, which is already identity-stable during streaming, so this does not rebuild the observer on token updates. ChatView mounts those keyed Turn wrappers as siblings of the virtual spacers; the first [data-virtual-turn-id] parent is that list, not a per-Turn shell.

Hosted test is red on this head because three WorkHub e2e cases failed (workhub-layout, two workhub-reconstruction). Those suites do not exercise this file. The rest of Desktop e2e passed, including the Prompt Rail cases. I am not treating that red as a finding on this change. The merge tree against current main is clean. This is a performance change; I am not merging it.

简体中文

我审的是 6476be8e6d388613225b892902c314b6cec90b0e。没有 P0/P1/P2/P3。

Prompt Rail 的 MutationObserver 以前盯整棵滚动子树,流式 Markdown / 公式 / Mermaid 的内部变动也会叫醒它。现在只盯 keyed [data-virtual-turn-id] 节点的父节点,而且只看 childList。新挂上的 Turn 包装节点里已经带着 [data-turn-id],原有的 add/remove 扫描仍然找得到。滚动所有权、划词引用、virtualizer 都没动。

hosted test 红在三条 WorkHub e2e,不经过这个文件。Prompt Rail 相关 e2e 过了。这是性能改动,我不合入。


Automated review notice: This comment was posted by an automated review agent operated by WAWQAQ. It is not an independent human review and does not replace one.

Keep Prompt Rail membership observation on the existing keyed Turn list instead of waking for arbitrary Markdown subtree mutations.

Generated-by: Codex
@Astro-Han
Astro-Han force-pushed the perf/prompt-rail-turn-membership-observer branch from 6476be8 to 18798c6 Compare August 29, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XS Under 10 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants