feat: optimize chat markdown rendering#1894
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughThis PR adds MarkStream streaming/final markdown controls, propagates markdown virtualization flags through chat rendering, and updates pending-assistant placeholder sizing and render-key reuse. It also adds related docs, tests, and dependency bumps. ChangesMarkStream Chat Rendering Optimization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/renderer/src/components/markdown/MarkdownRenderer.vue (1)
111-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider naming constants for
maxLiveNodes/liveNodeBuffertuning values.
220and60are magic numbers embedded directly in the computed logic with no explanation of the tuning rationale.♻️ Proposed refactor
+const COMPLETED_MAX_LIVE_NODES = 220 +const COMPLETED_LIVE_NODE_BUFFER = 60 + const shouldVirtualizeNodes = computed(() => props.virtualizeNodes && !isStreaming.value) const resolvedNodeVirtual = computed(() => shouldVirtualizeNodes.value ? ('auto' as const) : false ) -const maxLiveNodes = computed(() => (shouldVirtualizeNodes.value ? 220 : 0)) -const liveNodeBuffer = computed(() => (shouldVirtualizeNodes.value ? 60 : 0)) +const maxLiveNodes = computed(() => (shouldVirtualizeNodes.value ? COMPLETED_MAX_LIVE_NODES : 0)) +const liveNodeBuffer = computed(() => + shouldVirtualizeNodes.value ? COMPLETED_LIVE_NODE_BUFFER : 0 +)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/components/markdown/MarkdownRenderer.vue` around lines 111 - 127, The computed tuning values in MarkdownRenderer are using magic numbers, so extract the 220 and 60 thresholds into named constants near the existing computed props. Update maxLiveNodes and liveNodeBuffer to reference those constants, and use clear names that reflect the virtualization tuning intent so the values are easier to understand and adjust later.src/renderer/src/components/message/MessageItemAssistant.vue (1)
58-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a small pass-through test for
disableMarkdownVirtualization.The prop-forwarding chain to
MessageBlockContentisn't covered by a dedicated test in this cohort —ChatPage.test.ts/MessageList.test.tsstub out this component entirely, so a break in this specific hop (e.g. renamed prop) wouldn't be caught.Also applies to: 252-252
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/components/message/MessageItemAssistant.vue` at line 58, Add a focused pass-through test for MessageItemAssistant to verify it forwards disableMarkdownVirtualization to MessageBlockContent. The current ChatPage.test.ts and MessageList.test.ts stubs skip this component chain, so create a dedicated test around MessageItemAssistant that mounts it with disableMarkdownVirtualization set and asserts the prop reaches MessageBlockContent. Use the MessageItemAssistant and MessageBlockContent symbols to locate the forwarding path and protect against renamed or dropped props.src/renderer/src/pages/ChatPage.vue (1)
427-427: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFragile forward-reference for
clearMessageWindowMeasurements.
clearMessageWindowMeasurementsstarts as a no-op, is invoked from animmediate: truewatcher (line 948) beforemessageWindowexists, and is only rebound to the realclearMeasurementsat line 1276. This works today only because the watcher callback runs fully synchronously up to that call (noawaitbeforehand), so the real function is bound before any subsequent session change fires. A future refactor (e.g., adding an earlyawait, or reorderingmessageWindowcreation) could silently reintroduce a no-op call on real session switches.Consider adding a short comment documenting this ordering dependency to protect it from accidental breakage.
Also applies to: 948-948, 1276-1276
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/pages/ChatPage.vue` at line 427, Document the ordering dependency around clearMessageWindowMeasurements in ChatPage.vue so the forward-reference is intentional and protected from refactors. Add a brief comment near the initial no-op declaration and/or the immediate watcher usage explaining that the watcher runs before messageWindow exists and that clearMessageWindowMeasurements is rebound to clearMeasurements later in setup, so future changes must preserve this sequencing. Reference the clearMessageWindowMeasurements variable, the immediate watcher, and the clearMeasurements assignment to make the dependency easy to find.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/src/components/markdown/MarkdownRenderer.vue`:
- Line 9: MarkdownRenderer is hardcoding the MarkStream preset to chat, so all
preview surfaces inherit chat-specific behavior instead of using the appropriate
default/docs preset. Update the MarkdownRenderer component to accept a
configurable mode prop (or equivalent) and stop forcing mode="chat" in the
shared render path, then pass the desired mode from chat callers only while
leaving artifact, workspace, and skill-detail previews on the default behavior.
---
Nitpick comments:
In `@src/renderer/src/components/markdown/MarkdownRenderer.vue`:
- Around line 111-127: The computed tuning values in MarkdownRenderer are using
magic numbers, so extract the 220 and 60 thresholds into named constants near
the existing computed props. Update maxLiveNodes and liveNodeBuffer to reference
those constants, and use clear names that reflect the virtualization tuning
intent so the values are easier to understand and adjust later.
In `@src/renderer/src/components/message/MessageItemAssistant.vue`:
- Line 58: Add a focused pass-through test for MessageItemAssistant to verify it
forwards disableMarkdownVirtualization to MessageBlockContent. The current
ChatPage.test.ts and MessageList.test.ts stubs skip this component chain, so
create a dedicated test around MessageItemAssistant that mounts it with
disableMarkdownVirtualization set and asserts the prop reaches
MessageBlockContent. Use the MessageItemAssistant and MessageBlockContent
symbols to locate the forwarding path and protect against renamed or dropped
props.
In `@src/renderer/src/pages/ChatPage.vue`:
- Line 427: Document the ordering dependency around
clearMessageWindowMeasurements in ChatPage.vue so the forward-reference is
intentional and protected from refactors. Add a brief comment near the initial
no-op declaration and/or the immediate watcher usage explaining that the watcher
runs before messageWindow exists and that clearMessageWindowMeasurements is
rebound to clearMeasurements later in setup, so future changes must preserve
this sequencing. Reference the clearMessageWindowMeasurements variable, the
immediate watcher, and the clearMeasurements assignment to make the dependency
easy to find.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bfa4a344-9d73-487c-857c-4d7147319d3d
📒 Files selected for processing (13)
docs/features/markstream-chat-rendering-optimization/plan.mddocs/features/markstream-chat-rendering-optimization/spec.mddocs/features/markstream-chat-rendering-optimization/tasks.mdsrc/renderer/src/components/chat/MessageList.vuesrc/renderer/src/components/chat/MessageListRow.vuesrc/renderer/src/components/markdown/MarkdownRenderer.vuesrc/renderer/src/components/message/MessageBlockContent.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/pages/ChatPage.vuetest/renderer/components/ChatPage.test.tstest/renderer/components/MarkdownRenderer.test.tstest/renderer/components/MessageList.test.tstest/renderer/components/message/MessageBlockContent.test.ts
zhangmo8
force-pushed
the
feat/markstream-chat-rendering
branch
from
42370c2 to
39e6480
Compare
zerob13
reviewed
Jul 8, 2026
zerob13
left a comment
zerob13
left a comment
Collaborator
There was a problem hiding this comment.
整体评价
这个 PR 质量很高，SDD 文档完整，实现思路清晰。MarkStream 的 streaming/final 状态控制和虚拟化逻辑都设计得很合理，chat-send-jitter 的修复也很精准。
但有一个严重问题需要修复：
🔴 Critical Issue
MarkdownRenderer 硬编码了 mode="chat"
位置: src/renderer/src/components/markdown/MarkdownRenderer.vue:9
<NodeRenderer
mode="chat" // 👈 这会影响所有使用 MarkdownRenderer 的场景
...
/>问题:
- Artifact Markdown 预览和Workspace Markdown 预览也会使用 chat mode 的渲染行为
- 这两个场景应该使用 MarkStream 的默认/docs preset，而不是 chat preset
- 虽然你在
MarkdownArtifact.vue和WorkspacePreviewPane.vue中传了final=true、smooth-streaming=false、virtualize-nodes=false，但mode="chat"仍会影响 MarkStream 的其他行为（比如 batch rendering 策略、node mounting 优先级等）
修复方案:
// MarkdownRenderer.vue
const props = withDefaults(
defineProps<{
// ...existing props
+ mode?: 'chat' | 'default' // 或者用 'docs'
}>(),
{
+ mode: 'default',
// ...
}
)
<NodeRenderer
- mode="chat"
+ :mode="props.mode"
...
/>然后在 MessageBlockContent.vue 中明确传 mode="chat":
<MarkdownRenderer
:content="part.content"
+ mode="chat"
:streaming="isStreamingPart(part)"
...
/>这样 artifact/workspace 预览就会使用正确的默认行为。
🟡 Minor Issues
1. Magic numbers 应该提取为常量
位置: src/renderer/src/components/markdown/MarkdownRenderer.vue:141-142
const STATIC_MAX_LIVE_NODES = 260
const STATIC_LIVE_NODE_BUFFER = 80这两个值已经在代码里了，但在 computed 中又出现了 220 和 60（虽然我看代码里用的是常量，可能是 CodeRabbit 看错了？）。如果确实存在硬编码的地方，统一使用常量。
2. clearMessageWindowMeasurements 的 forward-reference 模式
位置: src/renderer/src/pages/ChatPage.vue:429, 951, 1309
这个模式是对的，但建议加个注释说明为什么要这样做：
// Initialized as no-op, rebound after messageWindow is created (line 1309).
// The immediate watcher at line 940 runs before messageWindow exists, so this
// forward-reference pattern ensures session-switch cleanup always works.
let clearMessageWindowMeasurements = () => {}✅ 做得好的地方
- SDD 流程完整: spec.md + plan.md + tasks.md 都很清晰
- 测试覆盖充分: 所有关键路径都有对应的测试
- 兼容性考虑周全:
- 搜索时禁用虚拟化
- session 切换清理 measurements
- streaming/completed 状态分离
- 性能优化合理: streaming 用小 batch，completed 用大 batch + 虚拟化
- chat-send-jitter 修复精准: 用真实的 spinner 高度估算，避免了 submit 时的跳动
Action Items
- 修复
mode="chat"硬编码问题（critical） - 添加
clearMessageWindowMeasurements注释（optional） - 确认是否有 magic number 未提取（如果 CodeRabbit 误报就忽略）
修复 critical issue 后这个 PR 就可以合并了。
zerob13
reviewed
Jul 8, 2026
zerob13
left a comment
zerob13
left a comment
Collaborator
There was a problem hiding this comment.
Critical: mode="chat" 被硬编码在 MarkdownRenderer.vue:9，这会让所有使用 MarkdownRenderer 的场景（包括 Artifact 预览、Workspace Markdown 预览）都使用 chat mode。
应该把 mode 改为可配置的 prop，默认值为 'default'，然后在 MessageBlockContent.vue 中明确传 mode="chat"。
:mode="props.mode"
同时在 props 定义中添加:
mode?: 'chat' | 'default'默认值设为 'default'。
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/src/components/chat/MessageListRow.vue`:
- Line 109: The delta lookup in MessageListRow is inconsistent with the height
cache key: `setMeasuredHeight()` stores entries by `renderKey ?? id`, but
`getEntry()` in the same component still resolves only by `id`, so placeholder
or streaming rows can miss their first delta and skip scroll correction. Update
the lookup logic to use the same identifier priority as the cached measurement
(`renderKey` first, then `id`) in `getEntry()` and any related matching code so
it stays aligned with `messageId` and `setMeasuredHeight()`.
In `@test/renderer/components/MessageList.test.ts`:
- Around line 71-77: The `useMessageCapture` mock in `MessageList.test.ts` uses
a plain object for `isCapturingRef`, so `unref()` in `MessageList.vue` never
unwraps it and `shouldDisableMarkdownVirtualization` stays truthy for the wrong
reason. Update the mock to use a real Vue `ref` for `isCapturing` (inside the
`vi.hoisted`/mock setup) and make the tests mutate that ref rather than
replacing the object. Also adjust the reset logic in the affected
`beforeEach`/test cases so they work with the shared ref returned by
`useMessageCapture`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f61b1182-bed2-4e89-89dd-c4c81133cc9b
📒 Files selected for processing (23)
docs/features/markstream-chat-rendering-optimization/plan.mddocs/features/markstream-chat-rendering-optimization/spec.mddocs/features/markstream-chat-rendering-optimization/tasks.mddocs/issues/chat-send-jitter/spec.mdsrc/renderer/src/components/artifacts/MarkdownArtifact.vuesrc/renderer/src/components/chat/MessageList.vuesrc/renderer/src/components/chat/MessageListRow.vuesrc/renderer/src/components/chat/messageListItems.tssrc/renderer/src/components/markdown/MarkdownRenderer.vuesrc/renderer/src/components/message/MessageBlockContent.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/components/sidepanel/viewer/WorkspacePreviewPane.vuesrc/renderer/src/composables/message/useMessageWindow.tssrc/renderer/src/pages/ChatPage.vuetest/renderer/components/ChatPage.test.tstest/renderer/components/MarkdownArtifact.test.tstest/renderer/components/MarkdownRenderer.test.tstest/renderer/components/MessageList.test.tstest/renderer/components/MessageListRow.test.tstest/renderer/components/WorkspacePreviewPane.test.tstest/renderer/components/message/MessageBlockContent.test.tstest/renderer/components/message/MessageItemAssistant.test.tstest/renderer/composables/useMessageWindow.test.ts
✅ Files skipped from review due to trivial changes (5)
- docs/issues/chat-send-jitter/spec.md
- src/renderer/src/components/chat/messageListItems.ts
- docs/features/markstream-chat-rendering-optimization/tasks.md
- docs/features/markstream-chat-rendering-optimization/spec.md
- docs/features/markstream-chat-rendering-optimization/plan.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/renderer/src/components/message/MessageBlockContent.vue
- src/renderer/src/components/markdown/MarkdownRenderer.vue
- test/renderer/components/MarkdownRenderer.test.ts
- test/renderer/components/message/MessageBlockContent.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1897
What this PR optimizes
This PR keeps DeepChat's current chat DOM/scroll model and uses
markstream-vuemore deliberately inside each Markdown message. It optimizes both states:It intentionally does not replace the outer
MessageListwith a virtual timeline in this slice, because DeepChat's current row-level behavior also owns search, capture, spotlight jump, artifacts, tool calls, and scroll anchoring.Concrete rendering behavior
Streaming assistant Markdown
MarkdownRenderernow passes explicit chat streaming settings tomarkstream-vue:mode="chat"final=falsesmooth-streaming="auto"typewriter=truecode-block-stream=truefade=falseThis favors smooth token flow and avoids large per-token DOM commits.
Completed/static Markdown
Completed chat Markdown now uses MarkStream's completed-content path:
final=truesmooth-streaming=falsetypewriter=falsecode-block-stream=falsenode-virtual="auto"max-live-nodes=260live-node-buffer=80This keeps long completed responses responsive without changing DeepChat's outer message windowing.
Compatibility guardrails
Key files
src/renderer/src/components/markdown/MarkdownRenderer.vuesrc/renderer/src/components/message/MessageBlockContent.vuesrc/renderer/src/components/chat/MessageList.vuesrc/renderer/src/pages/ChatPage.vuesrc/renderer/src/components/artifacts/MarkdownArtifact.vuesrc/renderer/src/components/sidepanel/viewer/WorkspacePreviewPane.vueTests
pnpm exec vitest --config vitest.config.renderer.ts test/renderer/components/MarkdownRenderer.test.ts test/renderer/components/message/MessageBlockContent.test.ts test/renderer/components/MessageList.test.ts test/renderer/components/ChatPage.test.ts test/renderer/components/WorkspacePreviewPane.test.ts test/renderer/components/MarkdownArtifact.test.tspnpm run formatpnpm run i18npnpm run lintpnpm run typecheck:webpnpm exec vitest --config vitest.config.renderer.ts test/renderer/performance/chatRendering.perf.test.tspnpm exec vitest --config vitest.config.renderer.ts test/renderer/composables/useMessageWindow.test.ts test/renderer/components/MessageListRow.test.ts test/renderer/components/MessageList.test.ts test/renderer/components/ChatPage.test.tsNotes
No benchmark numbers are claimed here. The PR is based on MarkStream's documented chat/performance presets plus focused tests for the integration and compatibility guardrails.
Summary by CodeRabbit