Skip to content

feat (web-shell): throttle Markdown AST parsing during streaming - #7904

Merged
wenshao merged 8 commits into
QwenLM:mainfrom
PratikWayase:perf/web-shell-streaming-markdown-ast
Aug 7, 2026
Merged

feat (web-shell): throttle Markdown AST parsing during streaming#7904
wenshao merged 8 commits into
QwenLM:mainfrom
PratikWayase:perf/web-shell-streaming-markdown-ast

Conversation

@PratikWayase

Copy link
Copy Markdown
Contributor

What this PR does

This PR introduces a throttling mechanism for Markdown parsing during assistant message streaming to prevent performance degradation. Instead of re-parsing the entire Markdown Abstract Syntax Tree (AST) on every single incoming text token, the parser now batches updates at ~80ms intervals. It also isolates the rendering logic into a strictly memoized component and memoizes the plugin arrays, ensuring that React skips unnecessary re-renders and heavy parsing work during the throttle window while maintaining an immediate, exact flush when streaming stops.

Why it's needed

During streaming, each incoming token previously triggered a full re-parse of the entire growing Markdown string through the react-markdown pipeline (including remark-gfm, remark-math, and rehype-katex). For long responses, this resulted in near-O(n²) cumulative CPU work, causing frame drops, high Interaction to Next Paint (INP), and visible UI stutter. The existing size guard only protected the Shiki syntax highlighting step, leaving the core AST parsing fully exposed to this performance bottleneck. This fix directly addresses the root cause by bounding the parse frequency regardless of token arrival rate.

Reviewer Test Plan

How to verify

  1. Trigger a long streaming response from the assistant (e.g., ask it to generate a 5,000+ character response containing code blocks, tables, or math formulas).
  2. Observe the UI responsiveness during the stream. The text should appear smoothly without noticeable frame drops or freezing.
  3. (Optional but recommended) Open the React DevTools Profiler while streaming. Verify that the ReactMarkdown component only commits renders at bounded intervals (~80ms) during streaming, rather than on every single token delta.
  4. Verify that the final settled content appears immediately and exactly when isStreaming becomes false, with no stale snapshots or "stall-then-dump" visual artifacts.

Evidence (Before & After)

Before: Noticeable UI lag, long tasks in the Performance tab, and stuttering during long streaming responses due to continuous full AST re-parsing.
After: Smooth, responsive streaming with bounded CPU usage per frame. The parser only runs at the throttled interval, and the final render flushes instantly upon completion.

Tested on

OS Status
🍏 macOS ⚠️
🪟 Windows
🐧 Linux ⚠️

Environment (optional)

Local development environment (npm run dev), verified via React DevTools Profiler and manual observation of long streaming responses.

Risk & Scope

  • Main risk or tradeoff: Throttling could theoretically cause a minor visual delay (up to 80ms) in text appearance. This is an acceptable tradeoff to prevent severe UI blocking, and the 80ms interval is below the threshold of human perception for "stall-then-dump" effects.
  • Not validated / out of scope: True incremental Markdown parsing. This PR implements a leading/trailing throttle, which is the recommended first-step mitigation per the issue discussion.
  • Breaking changes / migration notes: None. This is a pure internal performance optimization with no API or visual behavior changes.

Linked Issues

Closes #7273

中文说明

本 PR 的作用

本 PR 在助手消息流式传输期间引入了 Markdown 解析的节流(throttling)机制,以防止性能下降。解析器不再对每一个传入的文本 token 都重新解析整个 Markdown 抽象语法树(AST),而是以约 80 毫秒的间隔批量处理更新。此外,它将渲染逻辑隔离到一个严格记忆化(memoized)的组件中,并对插件数组进行了记忆化,确保 React 在节流窗口期内跳过不必要的重新渲染和繁重的解析工作,同时在流式传输停止时保持立即且精确的最终内容刷新。

为什么需要这样做

在流式传输期间,以前每一个传入的 token 都会触发通过 react-markdown 管道(包括 remark-gfmكمathrehype-katex)对整个不断增长的 Markdown 字符串进行完全重新解析。对于较长的回复,这会导致接近 O(n²) 的累积 CPU 工作量,从而导致掉帧、较高的 INP(下一次绘制的交互时间)以及明显的 UI 卡顿。现有的大小保护机制仅保护了 Shiki 语法高亮步骤,而使核心 AST 解析完全暴露于这一性能瓶颈之下。此修复通过限制解析频率(无论 token 到达速率如何)直接解决了根本原因。

审查者测试计划

如何验证

  1. 触发助手的长流式响应(例如,要求它生成一个包含代码块、表格或数学公式的 5000 字符以上的回复)。
  2. 观察流式传输期间的 UI 响应能力。文本应平滑出现,没有明显的掉帧或冻结。
  3. (可选但推荐)在流式传输期间打开 React DevTools Profiler。验证 ReactMarkdown 组件在流式传输期间仅以有界的间隔(约 80 毫秒)提交渲染,而不是在每个 token 增量时都渲染。
  4. 验证当 isStreaming 变为 false 时,最终稳定的内容能够立即且精确地显示,没有陈旧的快照或“停滞然后倾泻(stall-then-dump)”的视觉瑕疵。

证据(之前与之后)

之前:由于持续的全量 AST 重新解析,在长流式响应期间会出现明显的 UI 延迟、Performance 面板中的长任务以及卡顿。
之后:平滑、响应迅速的流式传输,每帧的 CPU 使用量受到限制。解析器仅在节流间隔运行,并且在完成时立即刷新最终渲染。

测试环境

操作系统 状态
🍏 macOS ⚠️
🪟 Windows
🐧 Linux ⚠️

环境(可选)

本地开发环境(npm run dev),通过 React DevTools Profiler 和对长流式响应的手动观察进行了验证。

风险与范围

  • 主要风险或权衡:节流理论上可能导致文本出现微小的视觉延迟(最多 80 毫秒)。为了防止严重的 UI 阻塞,这是一个可接受的权衡,并且 80 毫秒的间隔低于人类感知“停滞然后倾泻”效应的阈值。
  • 未验证 / 超出范围:真正的增量 Markdown 解析。本 PR 实现了首尾节流,这是根据 issue 讨论推荐的初步缓解措施。
  • 破坏性变更 / 迁移说明:无。这是一个纯粹的内部性能优化,没有 API 或视觉行为的改变。

关联的 Issue

Closes #7273

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on be1f3ef and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— be1f3ef 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed performance issue with a linked issue (#7273). The web-shell streaming path re-parses the full Markdown AST on every token, causing O(n²) cumulative CPU work and visible UI stutter on long responses. This is a real, reported problem — not theoretical hardening.

Direction: aligned. Streaming performance is a core UX concern for the web-shell, and throttling the parse frequency is the standard first-step mitigation. No auth/sandbox/public-contract concerns.

Size: not applicable — changes are in packages/web-shell/client/components/messages/, not core paths. 174 production lines + 2 test lines across 3 files.

Approach: the scope feels right. Moving the throttle from AssistantMessage into Markdown itself is the correct level — it protects the expensive ReactMarkdown parse directly and benefits any consumer of <Markdown>. Memoizing the plugin arrays and isolating the renderer in a memo'd component are natural complements. No unrelated changes or scope creep.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的性能问题,有关联 issue(#7273)。web-shell 流式传输路径在每个 token 上都重新解析完整的 Markdown AST,导致 O(n²) 累积 CPU 开销和长回复时明显的 UI 卡顿。这是真实的、已被报告的问题,不是理论性加固。

方向:对齐。流式传输性能是 web-shell 的核心 UX 关注点,限制解析频率是标准的第一步缓解措施。无 auth/sandbox/公共契约方面的顾虑。

规模:不适用——改动在 packages/web-shell/client/components/messages/,不涉及核心路径。3 个文件共 174 行生产代码 + 2 行测试代码。

方案:范围合理。将节流从 AssistantMessage 移入 Markdown 组件本身是正确的层级——直接保护昂贵的 ReactMarkdown 解析,并惠及所有 <Markdown> 的使用者。对插件数组进行 memoize 并将渲染器隔离到 memo 组件中是自然的补充。无无关改动或范围蔓延。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at be1f3eff2d32f72242ffaf69d7af7af22d675d85 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Code Review

The approach is sound and matches what I'd propose independently: throttle the content string at the component that owns the expensive parse, memoize the plugin arrays, and isolate the renderer behind memo(). The implementation is clean.

A few observations, none blocking:

  • useThrottledValue includes throttled in its effect dependency array, so the effect re-runs on every throttle flush. This is harmless (the body is a cheap time comparison) and needed for the streaming-stop flush check, but worth noting if anyone profiles effect churn later.
  • The non-monotonic bypass (!value.startsWith(throttled)) is a good safety valve — if content is replaced rather than appended (e.g. a retry or edit), it shows immediately instead of clinging to a stale prefix.
  • The components useMemo depends on renderedContent for the advanced-table reset key. Since renderedContent is now the throttled value, the components object recreates at the throttle interval rather than per-token — a minor secondary win.

No correctness bugs, security issues, or convention violations found. The test update (100ms wait for the 80ms throttle) is appropriate.

CI Test Evidence

CI is still running on the reviewed commit. Current status:

Final CI results for be1f3ef (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Capture web-shell visuals (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The ubuntu unit test suite and web-shell visual capture are still in flight. The finalize workflow will update this table once CI settles.

Not verified: real-scenario streaming behavior (requires the isolated @qwen-code /tmux job, which a maintainer can trigger). The PR's central claim is behavioral (smoother streaming), and static review + CI cannot fully substantiate it — but the mechanism is well-understood (bounding parse frequency) and the code is straightforward.

中文说明

代码审查

方案合理,与我独立提出的方案一致:在拥有昂贵解析操作的组件层面节流内容字符串,对插件数组进行 memoize,并用 memo() 隔离渲染器。实现干净。

几点观察,均非阻塞:

  • useThrottledValue 的 effect 依赖数组包含 throttled,因此每次节流刷新都会重新运行 effect。这无害(effect 体只是廉价的时间比较),且是流式停止时刷新检查所必需的,但如果后续有人分析 effect 频率,值得注意。
  • 非单调旁路(!value.startsWith(throttled))是一个好的安全阀——如果内容被替换而非追加(如重试或编辑),会立即显示而非 clinging 到过时的前缀。
  • components 的 useMemo 依赖 renderedContent(用于高级表格的 reset key)。由于 renderedContent 现在是节流后的值,components 对象以节流间隔重建而非每 token 重建——一个次要的额外收益。

未发现正确性 bug、安全问题或规范违反。测试更新(为 80ms 节流等待 100ms)是恰当的。

CI 测试证据

CI 仍在运行中。ubuntu 单元测试套件和 web-shell 视觉捕获仍在进行。finalize 工作流会在 CI 结束后更新此表。

未验证:真实场景流式传输行为(需要隔离的 @qwen-code /tmux 任务,维护者可触发)。PR 的核心声明是行为性的(更流畅的流式传输),静态审查 + CI 无法完全证实——但机制是well-understood(限制解析频率),代码也很直接。

Qwen Code · qwen3.8-max-preview

Reviewed at be1f3eff2d32f72242ffaf69d7af7af22d675d85 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean, focused fix for a real performance problem; only reservation is the behavioral claim hasn't been verified with real-scenario testing.

This is a well-scoped performance fix. The problem is real (linked issue, clear O(n²) mechanism), the solution is the standard first-step mitigation (bound the parse frequency), and the implementation is straightforward — a leading+trailing throttle hook, memoized plugin arrays, and an isolated memo'd renderer. No over-engineering, no scope creep, no drive-by changes.

The one thing I'd note: the PR's central claim is "smoother streaming", which is behavioral. Static review confirms the mechanism is correct, and CI will confirm nothing regresses, but the actual perceptual improvement hasn't been measured here. A maintainer can trigger @qwen-code /tmux for real-scenario verification if desired — though for a throttle at this level, the mechanism is well-understood enough that CI green is reasonable evidence.

Approval deferred until CI lands green on be1f3eff2d32f72242ffaf69d7af7af22d675d85.

中文说明

置信度:4/5 — 干净、聚焦的修复,解决真实的性能问题;唯一的保留是行为性声明未经真实场景测试验证。

这是一个范围良好的性能修复。问题是真实的(有关联 issue,清晰的 O(n²) 机制),方案是标准的第一步缓解(限制解析频率),实现也很直接——一个首尾节流 hook、memoized 插件数组、以及隔离的 memo 渲染器。无过度工程、无范围蔓延、无顺手改动。

唯一需要注意的是:PR 的核心声明是"更流畅的流式传输",这是行为性的。静态审查确认机制正确,CI 将确认无回归,但实际的感知改善尚未在此测量。维护者可触发 @qwen-code /tmux 进行真实场景验证——但对于这个层级的节流,机制足够 well-understood,CI 绿色是合理的证据。

审批延迟至 CI 在 be1f3eff2d32f72242ffaf69d7af7af22d675d85 上全部通过后执行。

Qwen Code · qwen3.8-max-preview

Reviewed at be1f3eff2d32f72242ffaf69d7af7af22d675d85 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head fd14b04. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

ℹ️ No screenshot changed against the PR base — but this PR edits 2 render-shaping files:

  • packages/web-shell/client/components/messages/AssistantMessage.tsx
  • packages/web-shell/client/components/messages/Markdown.tsx

Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to packages/web-shell/client/e2e/visuals/screenshots.spec.ts that seeds whatever state the UI is gated on; it then appears here as a head-only (NEW) capture.

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship — CI landed green after the review. ✅

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed. Suggestions are inline.

中文说明

已审查。 建议见行内评论。

— qwen3.8-max-preview via Qwen Code /review

Comment thread packages/web-shell/client/components/messages/Markdown.tsx
Comment thread packages/web-shell/client/components/messages/Markdown.tsx Outdated
Comment thread packages/web-shell/client/components/messages/Markdown.tsx Outdated

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues found. LGTM! ✅

中文说明

未发现问题。LGTM!✅

— qwen3.7-max via Qwen Code /review

@github-actions

Copy link
Copy Markdown
Contributor

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline.

中文说明

已审查——无阻断问题。 建议见行内评论。

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/components/messages/Markdown.tsx Outdated
Comment thread packages/web-shell/client/components/messages/Markdown.test.ts

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues found. LGTM! ✅

中文说明

未发现问题。LGTM!✅

— qwen3.8-max-preview via Qwen Code /review

@wenshao

wenshao commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

… throttle

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code resolved the merge conflicts and pushed the branch update.

Root cause

PR #7916 (feat(web-shell): render streaming charts with markdown-chart, commit 1d55d290a) landed on main after this PR branched, reworking the same Markdown component body in packages/web-shell/client/components/messages/Markdown.tsx that #7904 restructures to throttle AST parsing. Only Markdown.tsx conflicted (3 regions); the test file and AssistantMessage.tsx auto-merged.

Semantic, not textual

Both sides modified the same logic — plugin computation and return JSX of the Markdown component. main (#7916) rendered <ReactMarkdown> inline with chart-aware componentsWithCharts, wrapped in WebShellMarkdownChartProvider and a new MarkdownDocumentContext.Provider (consumed by isIncompleteTailFence to flag unterminated tail fences while streaming). HEAD (#7904) memoized renderedContent/plugins and rendered through a memo()-wrapped MemoizedMarkdownRenderer so react-markdown re-parses only on the throttled (80ms) content.

Resolution: main's chart JSX structure, rendering through the PR's memoized renderer (imports unioned: Options + isMarkdownFenceClosed):

const renderedMarkdown = (
  <MemoizedMarkdownRenderer
    content={renderedContent}
    components={componentsWithCharts} // main's chart components
    remarkPlugins={remarkPlugins}     // HEAD's memoized plugins
    rehypePlugins={rehypePlugins}
    urlTransform={markdownUrlTransform}
  />
);
// then wrapped: chart ? <WebShellMarkdownChartProvider source={renderedContent} ...>
//               : renderedMarkdown

What is load-bearing

  • Single-document invariant: the same throttled renderedContent feeds MemoizedMarkdownRenderer (what is parsed), WebShellMarkdownChartProvider source=, and MarkdownDocumentContext.Provider value=. isIncompleteTailFence slices the context document by AST node offsets — if any of the three got a different string (e.g. un-throttled content), offsets would be wrong.
  • Hook ordering: the plugin arrays are now useMemo hooks (HEAD), so if (!content) return null; must stay after the rehypePlugins memo — main's placement (before inline plugin consts) would break the Rules of Hooks.
  • Memo stability: componentsWithCharts (not renderedComponents) must be the components prop; it and the plugin arrays are memoized, so re-parsing happens only when renderedContent changes.

What I could not verify

No build/typecheck/tests run (not permitted here). The auto-merged Markdown.test.ts now holds both #7916's isIncomplete expectations and this PR's throttle suite; both stay satisfiable — the throttle's initial state passes content through unchanged, so the single-render fence test sees the full document. Chart rendering under throttling is a combination neither suite exercised alone; PR CI should cover it.

中文说明

根因:main 上的 PR #7916(streaming charts,提交 1d55d290a)改动了 Markdown.tsx 中同一个 Markdown 组件渲染主体,而本 PR(#7904)正重构同一区域以实现 AST 解析节流。仅 Markdown.tsx 有 3 处冲突。

语义冲突:main 内联渲染 <ReactMarkdown>(含图表 componentsWithCharts),外包 WebShellMarkdownChartProvider 与新增 MarkdownDocumentContext.Provider;HEAD 把插件与内容 useMemo 化,改用 memo 渲染器。解决方式:保留 main 图表 JSX 结构,经由 PR 的 memo 渲染器输出;导入取并集。

关键不变量:① 同一节流后的 renderedContent 必须同时传给 memo 渲染器、图表 Provider 的 sourceMarkdownDocumentContextisIncompleteTailFence 依赖节点偏移与所解析文档一致);② 插件改为 useMemo 后,if (!content) return null; 必须在所有 hook 之后;③ components prop 必须传 componentsWithCharts

未验证:本流程不运行构建/测试;"节流期间图表渲染"由 PR CI 验证。

Comment thread packages/web-shell/client/components/messages/Markdown.tsx
Comment on lines +808 to +814
if (
typeof value === 'string' &&
typeof throttled === 'string' &&
!value.startsWith(throttled)
) {
return value;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The non-monotonic bypass branch in useThrottledValue has no test coverage — no test renders a non-monotonic content change during streaming. — Concrete cost: if this condition were inverted (value.startsWith(throttled) instead of !value.startsWith(throttled)), a message regeneration (the assistant replacing its whole reply rather than appending) would show the stale throttled content for up to 80ms instead of the new content, and nothing in the suite would catch it. A mutation probe confirmed the branch is load-bearing and currently untested. Suggested test:

// render 'Hello world' (isStreaming), flush the throttle, then render
// 'Goodbye' (isStreaming) and assert 'Goodbye' appears immediately
// (without advancing timers)
中文说明

useThrottledValue 中的非单调旁路分支没有测试覆盖 —— 没有测试在流式传输期间渲染非单调的内容变化。具体代价:如果这个条件被反转(写成 value.startsWith(throttled) 而非 !value.startsWith(throttled)),消息重新生成(助手替换整条回复而非追加)时会显示陈旧的节流内容长达 80ms,而不是新内容,且测试套件中没有任何用例能捕获它。一个变异探针确认该分支是承重的,且目前未被测试。建议的测试见上方代码块。

— qwen3.8-max-preview via Qwen Code /review

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Critical] Moving the streaming throttle from AssistantMessage.tsx into the new useThrottledValue hook in Markdown.tsx (line 893, const throttledContent = useThrottledValue(content ?? '', isStreaming);) throttles every consumer, including the chart-rendering path. This breaks MarkdownChartRenderer.test.tsx > 'loads only after the active tail fence closes': the closed fence is a monotonic extension of the open fence, so the non-monotonic bypass does not fire, and the test's flushChart() drains only microtasks — not the 80ms setTimeout — so runtime.init is called 0 times (expected once). Probe-confirmed causal: the test fails with this PR applied (24 pass / 1 fail) and passes when the PR's three files are reverted to base (25 pass / 0 fail); the CI 'Test (ubuntu-latest, Node 22.x)' job fails on the same test. This is the same defect as the existing inline blocker (comment 3679125952), which still stands at the reviewed commit. Fix: in MarkdownChartRenderer.test.tsx, after the rerender to the closed fence, wait for the throttle to flush (await act(async () => { await new Promise((r) => setTimeout(r, 100)); })) before flushChart() — the same 100ms wait already added in Markdown.test.ts.

— qwen3.8-max-preview via Qwen Code /review

Comment thread packages/web-shell/client/components/messages/Markdown.tsx Outdated

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues found. LGTM! ✅

中文说明

未发现问题。LGTM!✅

— qwen3.8-max-preview via Qwen Code /review

@wenshao

wenshao commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Local verification — real-browser A/B, merge-base 2abfa3d vs head fd14b04

I built and ran this locally instead of eyeballing the Profiler, driving the real AssistantMessage / ThinkingMessage components in headless Chromium against a synthetic stream.

Verdict: correct and worth merging. It delivers a large, measurable win — but on a path the description doesn't mention, and it is roughly a wash on the path the description does name. Two notes below, neither blocking; the second comes with a measured one-line follow-up.

Method

A throwaway vite harness renders the real components (no daemon). react-markdown's import inside Markdown.tsx is swapped for a counting wrapper, so "AST parses" is a count of real full-document remark + rehype-katex parses, not a proxy. Render cost comes from React's <Profiler> (actualDuration), blocked time from PerformanceObserver('longtask'), frame rate from requestAnimationFrame ticks. Content advances on a wall-clock schedule, so both sides see the identical stream and the only variable is renderer cost. Test document is ~3.7 KB of headings, prose, a GFM table, KaTeX block + inline math, TS/bash fences and a task list, repeated ×1/×4/×8/×16. Mean of 2–3 runs per cell.

1. The premise in the description doesn't hold for main

each incoming token previously triggered a full re-parse of the entire growing Markdown string

That was true before #7408. AssistantMessage.tsx on main already carries useStreamingMarkdownContent — an 80 ms throttle applied to content before it reaches <Markdown>, and Markdown is memo()'d, so its whole subtree bails out between flushes. This PR deletes that hook and re-implements the same idea one level down. Measured on main, a 14.6 KB assistant answer streamed at 2 KB/s costs 64 parses, not one per token.

The path that genuinely is unthrottled on main is the one the description doesn't mention: ThinkingMessage renders <Markdown source="thinking" isStreaming={isStreaming}> with no upstream throttle, so an expanded reasoning panel re-parses the full document on every delta. Moving the throttle into Markdown fixes that for free — and that is where the numbers are.

metrics

Expanded reasoning panel, 14.6 KB @ 2 KB/s — the real win:

metric main this PR
AST parses 269 93 −65%
main-thread busy 80.1% 42.4% −47%
frame rate 36.9 fps 54.2 fps +47%
blocked (long-task) time 1516 ms 888 ms −41%
React render time 5872 ms 3116 ms −47%

At 269 parses over ~270 deltas that is exactly the 1-parse-per-token behaviour the PR set out to kill. This alone justifies the change.

Assistant answer @ 2 KB/s — already throttled on main, so the result is mixed:

final doc AST parses blocked ms fps React render ms
3.7 KB (@400 c/s) 99 → 115 0 → 0 60.1 → 60.1 1378 → 1183
14.6 KB 64 → 94 1196 → 69 54.0 → 58.4 2251 → 2269
29.3 KB 102 → 184 5300 → 3421 45.1 → 50.3 6280 → 6919
58.6 KB 174 → 335 13683 → 15310 39.3 → 36.6 14489 → 18715

Up to ~30 KB the PR is a clear improvement in distribution (blocked time −94% at 14.6 KB, frames +12% at 29.3 KB) even though it parses more often. At 58.6 KB the extra parses buy nothing and cost ~29% more CPU. See note 2.

First-paint latency also improves: main's throttle is trailing-only, so the first token of a response is invisible for a full interval (measured 90–98 ms). The PR's leading edge paints at 8 ms.

2. Correctness — clean

parity

  • Settled output is identical. After the stream ends, #stage innerHTML md5 is 20195b44… on both sides and the full-page screenshot md5 is 242c03f5… on both sides. Final rendered text length matched exactly in every run (3461 / 13847 / 13862 chars) — no stale snapshot, no truncated tail, no "stall-then-dump".
  • Non-monotonic content (rewind/edit shortens or replaces the string): the !value.startsWith(throttled) bypass fires and the new content paints on the next frame, same as main.
  • Stale-tail probe — last delta arrives and isStreaming never flips to false: both flush; the PR flushes in ≤5 ms (leading edge), main takes ~100 ms.
  • Tests. Full web-shell suite on the head: 151 files / 2450 tests pass. typecheck and eslint clean on all four touched files.
  • The new test is non-vacuous. Reverting only Markdown.tsx + AssistantMessage.tsx to the merge-base while keeping the PR's tests fails exactly one assertion:
    FAIL Markdown streaming throttle > flushes the latest content when multiple tokens arrive in one throttle window
    AssertionError: expected 'Token 1 Token 2 Token 3' not to contain 'Token 1 Token 2 Token 3'
    
    The two setTimeout(100) waits added to pre-existing tests still pass against base source, so they don't mask a regression.
  • memo() on MemoizedMarkdownRenderer really does bail: 24 parses for 152 deltas is only reachable if the memoised plugin/component references hold.

3. Note — the new throttle has no back-pressure

main and this PR arm the window differently, and it matters under load:

  • main re-arms its 80 ms timer inside the effect that runs after the previous flush has rendered, so its effective period is 80 ms + parse time. It backs off automatically as the document grows — measured median gap 98 → 168 ms from 3.7 KB → 58.6 KB.
  • This PR stamps lastRunRef.current = Date.now() immediately before setThrottled, i.e. before React renders and parses. Parse cost therefore lands inside the window, and the period stays pinned at exactly 80 ms at every document size (measured gap median 80 in all five configurations). Once a single parse approaches 80 ms the next effect already sees elapsed >= intervalMs and takes the leading-edge branch with zero idle in between.

That's why parses grow 17→93% and why the 58.6 KB case regresses against main.

Moving the stamp so it lands after the flush commits fixes it. I patched it locally and re-ran the whole matrix — it beats both main and this PR on every scenario measured, keeps the leading edge and the memoisation, and all 109 tests in the three touched suites still pass:

// stamp the window after the flush has actually rendered
useEffect(() => {
  lastRunRef.current = Date.now();
}, [throttled]);
// …and drop the two in-place `lastRunRef.current = Date.now()` assignments

follow-up

scenario metric main this PR PR + back-pressure
assistant 14.6 KB blocked ms 1196 69 0
assistant 29.3 KB fps 45.1 50.3 53.3
assistant 58.6 KB fps 39.3 36.6 45.1
assistant 58.6 KB render ms 14489 18715 12669
thinking 14.6 KB main-thread busy 80.1% 42.4% 25.6%
thinking 14.6 KB blocked ms 1516 888 17

Happy for this to land as-is and follow up separately — the PR is already a net win. But if @PratikWayase wants to fold it in, it's four lines.

Ask before squashing

Please update the description's "Why it's needed" section: as written it credits the PR with removing a per-token re-parse on the assistant path that #7408 had already removed, and omits the reasoning-panel win that is the actual payoff. The squash message becomes the changelog entry, so it's worth getting right.

中文版

本地验证 —— 真实浏览器 A/B,merge-base 2abfa3d vs head fd14b04

我在本地构建并跑了真实测试,而不是靠肉眼看 Profiler:在 headless Chromium 里驱动真实的 AssistantMessage / ThinkingMessage 组件,喂入合成的流式内容。

结论:实现正确,值得合入。 它带来了一个很大的、可测量的收益 —— 但收益出现在 PR 描述没有提到的那条路径上;而描述明确点名的那条路径,收益基本是打平。下面两点都不阻塞合并,其中第二点附带一个已实测的一行修复。

方法

用一个一次性的 vite harness 渲染真实组件(不需要 daemon)。把 Markdown.tsx 内部对 react-markdown 的 import 替换成一个计数包装器,所以**「AST 解析次数」是真实的全文档 remark + rehype-katex 解析计数**,不是间接指标。渲染耗时取自 React <Profiler>actualDuration,阻塞时长取自 PerformanceObserver('longtask'),帧率取自 requestAnimationFrame 回调计数。内容按墙钟时间推进,所以两边看到的是完全相同的流,唯一变量就是渲染器开销。测试文档约 3.7 KB,包含标题、正文、GFM 表格、KaTeX 块级+行内公式、TS/bash 代码块和任务列表,分别重复 ×1/×4/×8/×16。每个格子取 2–3 次运行的均值。

1. 描述里的前提对当前 main 不成立

each incoming token previously triggered a full re-parse of the entire growing Markdown string

这在 #7408 之前是对的。当前 mainAssistantMessage.tsx 里已经有 useStreamingMarkdownContent —— 一个 80 ms 节流,作用在 content 到达 <Markdown> 之前;而 Markdown 本身是 memo() 的,所以两次 flush 之间整棵子树会直接 bail out。本 PR 实际上是删掉那个 hook,然后在下一层重新实现同一个想法。实测:main 上一段 14.6 KB 的助手回复以 2 KB/s 流式输出,总共 64 次解析,而不是每 token 一次。

main 上真正没有节流的,恰恰是描述没提的那条:ThinkingMessage 渲染 <Markdown source="thinking" isStreaming={isStreaming}>,上游没有任何节流,所以展开的思考面板会在每一个 delta 上重新解析整个文档。把节流下沉到 Markdown 顺手修好了这条路径 —— 收益全在这里。

展开的思考面板,14.6 KB @ 2 KB/s —— 真正的收益:

指标 main 本 PR
AST 解析次数 269 93 −65%
主线程繁忙占比 80.1% 42.4% −47%
帧率 36.9 fps 54.2 fps +47%
阻塞(long-task)时长 1516 ms 888 ms −41%
React 渲染耗时 5872 ms 3116 ms −47%

269 次解析 / 约 270 个 delta,正好就是 PR 想干掉的「每 token 一次解析」。单凭这一条就足以合入。

助手回复 @ 2 KB/s —— main 上本来就有节流,所以结果是有得有失:

最终文档 AST 解析次数 阻塞 ms fps React 渲染 ms
3.7 KB(@400 c/s) 99 → 115 0 → 0 60.1 → 60.1 1378 → 1183
14.6 KB 64 → 94 1196 → 69 54.0 → 58.4 2251 → 2269
29.3 KB 102 → 184 5300 → 3421 45.1 → 50.3 6280 → 6919
58.6 KB 174 → 335 13683 → 15310 39.3 → 36.6 14489 → 18715

约 30 KB 以内,虽然解析次数更多,但分布明显更好(14.6 KB 时阻塞时长 −94%,29.3 KB 时帧率 +12%)。到 58.6 KB,多出来的解析没有换来任何收益,反而多烧约 29% CPU。见第 3 点。

首帧延迟也变好了: main 的节流是纯 trailing 的,所以回复的第一个 token 会有整整一个周期看不见(实测 90–98 ms)。本 PR 的 leading edge 在 8 ms 就画出来了。

2. 正确性 —— 干净

  • 稳定后的输出完全一致。 流结束后,两边 #stage innerHTML 的 md5 都是 20195b44…,整页截图的 md5 都是 242c03f5…。每一次运行最终渲染文本长度都精确相等(3461 / 13847 / 13862 字符)—— 没有陈旧快照,没有截断的尾巴,没有「停滞然后倾泻」。
  • 非单调内容(rewind/编辑导致字符串变短或被替换):!value.startsWith(throttled) 的旁路会触发,新内容在下一帧就画出来,与 main 一致。
  • 尾包滞留探测 —— 最后一个 delta 到达后 isStreaming 永远不翻成 false:两边都会 flush;本 PR 在 ≤5 ms 内完成(leading edge),main 需要约 100 ms。
  • 测试。 head 上 web-shell 全量套件:151 个文件 / 2450 个用例全绿。四个改动文件的 typecheckeslint 均干净。
  • 新增测试非空转。 只把 Markdown.tsx + AssistantMessage.tsx 回退到 merge-base、保留 PR 的测试,恰好挂掉一条断言:
    FAIL Markdown streaming throttle > flushes the latest content when multiple tokens arrive in one throttle window
    AssertionError: expected 'Token 1 Token 2 Token 3' not to contain 'Token 1 Token 2 Token 3'
    
    另外,加进两个既有测试里的 setTimeout(100) 等待在 base 源码上同样能过,说明它们没有掩盖回归。
  • MemoizedMarkdownRenderer 上的 memo() 确实生效了:152 个 delta 只有 24 次解析,只有在插件/组件引用真的稳定时才可能达到。

3. 提示 —— 新的节流没有背压

main 和本 PR 开启时间窗的方式不同,在高负载下差别很大:

  • main 是在「上一次 flush 已经渲染完成」之后运行的那个 effect 里重新 arm 80 ms 定时器,所以它的实际周期是 80 ms + 解析耗时。文档变大时它会自动退避 —— 实测中位间隔从 3.7 KB 的 98 ms 涨到 58.6 KB 的 168 ms。
  • 本 PRsetThrottled 之前就写下 lastRunRef.current = Date.now(),也就是在 React 渲染并解析之前。于是解析耗时落在了窗口内部,周期在任何文档大小下都被钉死在恰好 80 ms(五种配置实测中位间隔全是 80)。一旦单次解析逼近 80 ms,下一次 effect 就已经满足 elapsed >= intervalMs,直接走 leading-edge 分支,中间没有任何空闲。

这就是解析次数多出 17→93%、以及 58.6 KB 那一档相对 main 退化的原因。

把这个时间戳挪到 flush 提交之后再写就能修好。我在本地打了这个补丁并重跑了整个矩阵 —— 在测过的每一个场景上它都同时优于 main 和本 PR,leading edge 和 memo 化都保留,三个改动测试文件的 109 个用例依然全绿:

// 在 flush 真正渲染完成之后再标记时间窗
useEffect(() => {
  lastRunRef.current = Date.now();
}, [throttled]);
// …并删掉原来那两处就地的 `lastRunRef.current = Date.now()`
场景 指标 main 本 PR PR + 背压
assistant 14.6 KB 阻塞 ms 1196 69 0
assistant 29.3 KB fps 45.1 50.3 53.3
assistant 58.6 KB fps 39.3 36.6 45.1
assistant 58.6 KB 渲染 ms 14489 18715 12669
thinking 14.6 KB 主线程繁忙 80.1% 42.4% 25.6%
thinking 14.6 KB 阻塞 ms 1516 888 17

按现状合入、后续再单独跟进也完全可以 —— 本 PR 本身已经是净收益。但如果 @PratikWayase 想顺手带上,改动只有四行。

合并前的一个请求

麻烦更新一下描述里的 "Why it's needed":现在的写法把「移除助手路径上的每-token 重解析」算作本 PR 的功劳,而那件事 #7408 已经做过了;同时漏掉了思考面板这个真正的收益点。squash 后的提交信息会成为 changelog 条目,值得写准确。

@PratikWayase

Copy link
Copy Markdown
Contributor Author

@wenshao Is this PR ready to merge, or does it need any further modifications?

@wenshao
wenshao added this pull request to the merge queue Aug 7, 2026
Merged via the queue into QwenLM:main with commit 1fec1af Aug 7, 2026
48 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.

perf(web-shell): streaming re-parses the entire Markdown AST on every token

4 participants