Skip to content

fix(wework): preserve viewport during virtualized streaming - #2298

Merged
qdaxb merged 3 commits into
mainfrom
fix/wework-streaming-scroll-measurement
Jul 29, 2026
Merged

qdaxb merged 3 commits into
mainfrom
fix/wework-streaming-scroll-measurement

Conversation

@qdaxb

@qdaxb qdaxb commented Jul 29, 2026 •

Copy link
Copy Markdown
Contributor

问题

长对话启用消息虚拟化后,用户在 AI 流式输出期间向上阅读历史内容,活动中的助手消息会离开虚拟列表渲染范围并被卸载。它在屏幕外继续增长时无法被测量,最终重新挂载时会从估算高度跳到真实高度,导致当前阅读位置和最终 scrollTop 漂移。

修改

  • 将当前活动的流式消息始终加入 TanStack Virtual 的渲染 range,即使它位于可视范围和 overscan 之外,也持续测量真实高度。
  • 增加虚拟列表单元测试,验证屏幕外的活动流式消息仍保持挂载。
  • 将桌面 E2E 扩展为超过虚拟化阈值的多轮长对话:滚动到已完成的历史内容后,让屏幕外响应继续增长,并验证列表总高度增长、文本锚点和 scrollTop 在流式完成及重新打开任务后保持稳定。
  • 补充中英文开发文档,记录屏幕外流式消息的测量约束和 E2E 覆盖范围。

验证

  • pnpm --filter wework test(233 个测试文件、2319 个测试通过)
  • 变更文件 Prettier 与 ESLint 检查
  • pnpm --filter wework e2e:desktop:streaming-text
  • 独立真实 Tauri 会话 WEWORK_PORT=1421 pnpm --filter wework ai:verify start
  • pre-push:ESLint、TypeScript、完整 Wework 单测全部通过

Summary by CodeRabbit

  • Bug Fixes

    • Improved long conversation streaming so active responses continue updating reliably, even when scrolled offscreen.
    • Ensured streaming messages remain rendered within virtualization, including correct handling when also targeted for forced navigation (no duplicates).
    • Preserved scroll position and visible text anchors during streaming, completion, and task reopening.
    • Maintained accurate message sizing and history navigation for virtualized conversations, with correct streaming indicator behavior.
  • Documentation

    • Updated English and Chinese guides with the revised streaming/scrolling validation flow and virtualization/performance diagnostic constraints.
  • Tests

    • Added coverage for streaming-message virtualization and deduplication scenarios.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc0814da-cfe4-4d4e-8b67-0d98f7713577

📥 Commits

Reviewing files that changed from the base of the PR and between f14f943 and b6dcee3.

📒 Files selected for processing (7)
  • docs/en/wework/developer-guide/wework-e2e-automation.md
  • docs/en/wework/developer-guide/wework-performance-diagnostics.md
  • docs/zh/wework/developer-guide/wework-e2e-automation.md
  • docs/zh/wework/developer-guide/wework-performance-diagnostics.md
  • wework/e2e/desktop/scenarios/streaming-text.scenario.mjs
  • wework/src/components/chat/MessageList.tsx
  • wework/src/components/chat/MessageList.virtualization.test.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
  • wework/src/components/chat/MessageList.tsx
  • docs/en/wework/developer-guide/wework-e2e-automation.md
  • docs/zh/wework/developer-guide/wework-e2e-automation.md
  • docs/en/wework/developer-guide/wework-performance-diagnostics.md
  • docs/zh/wework/developer-guide/wework-performance-diagnostics.md
  • wework/e2e/desktop/scenarios/streaming-text.scenario.mjs

📝 Walkthrough

Walkthrough

The PR keeps active streaming messages mounted outside the visible virtual range, expands the desktop streaming scenario with scripted history turns, and updates scroll, anchoring, task-switching, virtualization, and diagnostic documentation in English and Chinese.

Changes

Streaming virtualization and E2E validation

Layer / File(s) Summary
Keep active streaming messages mounted
wework/src/components/chat/MessageList.tsx, wework/src/components/chat/MessageList.virtualization.test.tsx, docs/.../wework-performance-diagnostics.md
The virtualizer now retains the active streaming row, with tests and diagnostics describing offscreen measurement and anchor restoration behavior.
Generate history and streaming responses
wework/e2e/desktop/scenarios/streaming-text.scenario.mjs
The scenario adds scripted history turns, matching mock SSE responses, and updated completion generation.
Validate virtualized scroll stability
wework/e2e/desktop/scenarios/streaming-text.scenario.mjs, docs/en/.../wework-e2e-automation.md, docs/zh/.../wework-e2e-automation.md
E2E checks now cover scroll-height growth, visible-anchor stability, task reopening, turn navigation, and completion-state indicators.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Scenario
  participant TauriApp
  participant MessageList
  participant MockSSE
  Scenario->>TauriApp: submit history and streaming prompts
  TauriApp->>MockSSE: request scripted response
  MockSSE-->>TauriApp: emit assistant completion or append content
  TauriApp->>MessageList: update message status and content
  MessageList-->>Scenario: preserve virtualized row and scroll state
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: preserving viewport stability during virtualized streaming.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/wework-streaming-scroll-measurement

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qdaxb
qdaxb marked this pull request as ready for review July 29, 2026 06:36

@coderabbitai coderabbitai Bot 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
wework/src/components/chat/MessageList.tsx (1)

309-318: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Dedupe forcedIndexes before merging — forceVirtualMessageId can collide with the streaming index.

[forcedVirtualMessageIndex, streamingVirtualMessageIndex].filter(index => index >= 0 && !indexes.includes(index)) only excludes indexes already present in the base indexes array; it does not dedupe the two source values against each other. If a caller sets forceVirtualMessageId to the id of the currently streaming message (e.g. turn navigation jumping to the actively-streaming turn — exactly the scenario exercised by the new STREAMING_TURN_INDEX E2E check), forcedVirtualMessageIndex === streamingVirtualMessageIndex, and the same index is pushed twice into forcedIndexes. The final range then contains that index twice, so messageVirtualizer.getVirtualItems() yields two rows with the identical key (from getItemKey), producing a React duplicate-key warning and unpredictable duplicate/overlapping DOM for that message.

🐛 Proposed fix
-      const forcedIndexes = [forcedVirtualMessageIndex, streamingVirtualMessageIndex].filter(
-        index => index >= 0 && !indexes.includes(index)
-      )
+      const forcedIndexes = Array.from(
+        new Set([forcedVirtualMessageIndex, streamingVirtualMessageIndex])
+      ).filter(index => index >= 0 && !indexes.includes(index))
🤖 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 `@wework/src/components/chat/MessageList.tsx` around lines 309 - 318, Update
the rangeExtractor callback to deduplicate forcedVirtualMessageIndex and
streamingVirtualMessageIndex against both the base indexes and each other before
merging. Ensure the returned sorted range contains each virtual message index at
most once, including when both forced values are identical.
🤖 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.

Outside diff comments:
In `@wework/src/components/chat/MessageList.tsx`:
- Around line 309-318: Update the rangeExtractor callback to deduplicate
forcedVirtualMessageIndex and streamingVirtualMessageIndex against both the base
indexes and each other before merging. Ensure the returned sorted range contains
each virtual message index at most once, including when both forced values are
identical.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 727462c1-e778-4a2f-930a-da8eba769c3c

📥 Commits

Reviewing files that changed from the base of the PR and between 4129e87 and f14f943.

📒 Files selected for processing (7)
  • docs/en/wework/developer-guide/wework-e2e-automation.md
  • docs/en/wework/developer-guide/wework-performance-diagnostics.md
  • docs/zh/wework/developer-guide/wework-e2e-automation.md
  • docs/zh/wework/developer-guide/wework-performance-diagnostics.md
  • wework/e2e/desktop/scenarios/streaming-text.scenario.mjs
  • wework/src/components/chat/MessageList.tsx
  • wework/src/components/chat/MessageList.virtualization.test.tsx

@qdaxb
qdaxb force-pushed the fix/wework-streaming-scroll-measurement branch from f14f943 to b6dcee3 Compare July 29, 2026 06:56
@qdaxb
qdaxb added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit 4c5a744 Jul 29, 2026
35 checks passed
@qdaxb
qdaxb deleted the fix/wework-streaming-scroll-measurement branch July 29, 2026 07:54
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.

1 participant