Repository navigation
fix(wework): hide completed thinking placeholders - #2387
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughChangesReasoning display lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ReasoningState
participant MessageList
participant ThinkingBlockItem
ReasoningState->>MessageList: stream reasoning progress
MessageList->>MessageList: normalize and filter processing blocks
MessageList->>ThinkingBlockItem: render unfinished thinking content
ThinkingBlockItem-->>MessageList: show live preview
ReasoningState->>MessageList: complete or cancel turn
MessageList->>ThinkingBlockItem: exclude completed thinking block
ThinkingBlockItem-->>MessageList: remove reasoning preview and placeholder
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
qdaxb
marked this pull request as ready for review
August 3, 2026 10:50
qdaxb
enabled auto-merge
August 3, 2026 10:50
![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: 5
🤖 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 `@docs/en/wework/developer-guide/wework-e2e-automation.md`:
- Line 110: Update the documentation sentence describing active streaming
reasoning to match the implemented coverage: either add an assertion in
streaming-text.scenario.mjs that verifies the “Thinking · summary” row while
streaming, or revise the sentence to mention only the existing post-completion
reasoning-removal check.
In `@docs/zh/wework/developer-guide/wework-e2e-automation.md`:
- Line 110: Update the documentation sentence describing the streaming reasoning
check to match the executable coverage: either add an assertion in
streaming-text.scenario.mjs for the active “正在思考 · 摘要” row during streaming, or
revise the sentence to mention only the existing post-completion reasoning
removal check.
In `@wework/e2e/desktop/scenarios/streaming-text.scenario.mjs`:
- Around line 711-719: Update
wework/e2e/desktop/scenarios/streaming-text.scenario.mjs:711-719 to pause the
reasoning mock before terminal events and assert the active thinking row and
current reasoning summary, then retain completion-cleanup assertions after
releasing the mock. Update
docs/en/wework/developer-guide/wework-e2e-automation.md:110 and
docs/zh/wework/developer-guide/wework-e2e-automation.md:110 to accurately
describe the live reasoning assertion, or document only completion cleanup if
that is the intended coverage.
- Around line 711-721: Synchronize the reasoning-removal assertions in the
streaming scenario with the terminal response state before reading
expandedProcessingSnapshot. Reuse the scenario’s existing completion-wait
pattern, or poll until both thinking-toggle-button and REASONING_SUMMARY are
absent, then perform the assertions and capture the screenshot.
In `@wework/src/components/chat/blocks/ToolBlockItem.tsx`:
- Line 864: Update the guard in the reasoning preview rendering path around
buildBlockPreview so block.content is rejected when it is empty or contains only
whitespace, while preserving the existing isRunning check and rendering behavior
for substantive content.
🪄 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 Plus
Run ID: 95368ff4-3256-4cb8-a1a3-f113f6825370
📒 Files selected for processing (9)
docs/en/wegent/developer-guide/local-device-architecture.mddocs/en/wework/developer-guide/wework-e2e-automation.mddocs/zh/wegent/developer-guide/local-device-architecture.mddocs/zh/wework/developer-guide/wework-e2e-automation.mdwework/e2e/desktop/scenarios/streaming-text.scenario.mjswework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/blocks/ToolBlockItem.test.tsxwework/src/components/chat/blocks/ToolBlockItem.tsx
| Matrix submissions use a 10-second timeout. If the composer already displays a submission error, the runner throws that exact error immediately. Otherwise, a stalled protocol stage reports its current stage and captured requests instead of waiting through the general UI timeout. | ||
|
|
||
| `e2e:desktop:streaming-text` runs an isolated streaming-message state regression through a scenario module. It uses the real Tauri WebView, Executor, and Codex app-server while a loopback Responses SSE keeps a partial reply active. The scenario first verifies that a completed reasoning summary can be collapsed and expanded. It then starts a long-running command and confirms that the tool-row duration continues increasing after switching tasks while the tool-group header does not show a turn-wide aggregate duration. The scenario next builds a long multi-turn conversation beyond the virtualization threshold and verifies the “Thinking” position, user scroll anchor, streaming growth, and viewport stability after reopening the task. It also verifies that the waiting state disappears after completion. The scenario retains screenshots for reasoning, tool timing, ready, streaming, and completed stages; its scenario-specific Codex configuration disables plugin extensions to isolate direct message streaming. | ||
| `e2e:desktop:streaming-text` runs an isolated streaming-message state regression through a scenario module. It uses the real Tauri WebView, Executor, and Codex app-server while a loopback Responses SSE keeps a partial reply active. The scenario first verifies that streaming reasoning appears as “Thinking · summary” and that the reasoning summary and its placeholder are removed after completion. It then starts a long-running command and confirms that the tool-row duration continues increasing after switching tasks while the tool-group header does not show a turn-wide aggregate duration. The scenario next builds a long multi-turn conversation beyond the virtualization threshold and verifies the “Thinking” position, user scroll anchor, streaming growth, and viewport stability after reopening the task. It also verifies that the waiting state disappears after completion. The scenario retains screenshots for reasoning, tool timing, ready, streaming, and completed stages; its scenario-specific Codex configuration disables plugin extensions to isolate direct message streaming. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not claim coverage that the scenario does not assert.
The scenario currently checks reasoning removal after completion. It does not assert that the active “Thinking · summary” row appears during streaming. Add that assertion to streaming-text.scenario.mjs, or change this sentence to describe only the existing completion check.
As per coding guidelines, changed core user flows must have corresponding automated E2E regression coverage.
Based on the supplied scenario, active reasoning is not currently asserted.
🧰 Tools
🪛 LanguageTool
[style] ~110-~110: Consider placing the discourse marker ‘first’ at the beginning of the sentence for more clarity.
Context: ...onses SSE keeps a partial reply active. The scenario first verifies that streaming reasoning appea...
(SENT_START_FIRST_PREMIUM)
🤖 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 `@docs/en/wework/developer-guide/wework-e2e-automation.md` at line 110, Update
the documentation sentence describing active streaming reasoning to match the
implemented coverage: either add an assertion in streaming-text.scenario.mjs
that verifies the “Thinking · summary” row while streaming, or revise the
sentence to mention only the existing post-completion reasoning-removal check.
Source: Coding guidelines
| 矩阵提交等待上限为 10 秒。如果编辑器已经显示提交错误，runner 会立即抛出该错误；否则在协议阶段未按时推进时输出当前阶段和已捕获请求，避免失败后长时间无反馈。 | ||
|
|
||
| `e2e:desktop:streaming-text` 通过场景模块运行独立的流式消息状态回归。它使用真实 Tauri WebView、Executor 和 Codex app-server，通过 loopback Responses SSE 保持部分回复处于运行状态。场景先验证完成的推理摘要可以收起和展开，再启动长命令，确认工具行耗时在切换任务后连续递增，同时工具分组标题不显示整轮累计耗时。随后场景构造超过虚拟化阈值的多轮长对话，验证“正在思考”位置、用户滚动锚点、流式增长和任务重开后的视口稳定性，并在响应完成后确认等待状态消失。该场景会保存推理、工具计时、就绪、流式和完成阶段的截图；场景专用 Codex 配置会关闭插件扩展，以隔离验证消息直出链路。 | ||
| `e2e:desktop:streaming-text` 通过场景模块运行独立的流式消息状态回归。它使用真实 Tauri WebView、Executor 和 Codex app-server，通过 loopback Responses SSE 保持部分回复处于运行状态。场景先验证流式推理显示“正在思考 · 摘要”，并在响应完成后移除推理摘要及其占位，再启动长命令，确认工具行耗时在切换任务后连续递增，同时工具分组标题不显示整轮累计耗时。随后场景构造超过虚拟化阈值的多轮长对话，验证“正在思考”位置、用户滚动锚点、流式增长和任务重开后的视口稳定性，并在响应完成后确认等待状态消失。该场景会保存推理、工具计时、就绪、流式和完成阶段的截图；场景专用 Codex 配置会关闭插件扩展，以隔离验证消息直出链路。 |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Keep the Chinese documentation aligned with the executable coverage.
The scenario checks reasoning removal after completion, but it does not assert that the active “正在思考 · 摘要” row appears during streaming. Add that assertion to streaming-text.scenario.mjs, or change this sentence to describe only the existing completion check.
As per coding guidelines, changed core user flows must have corresponding automated E2E regression coverage.
Based on the supplied scenario, active reasoning is not currently asserted.
🤖 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 `@docs/zh/wework/developer-guide/wework-e2e-automation.md` at line 110, Update
the documentation sentence describing the streaming reasoning check to match the
executable coverage: either add an assertion in streaming-text.scenario.mjs for
the active “正在思考 · 摘要” row during streaming, or revise the sentence to mention
only the existing post-completion reasoning removal check.
Source: Coding guidelines
Comment on lines
+711
to
+719
| assert.equal( | ||
| expandedProcessingSnapshot.testIds.includes('thinking-toggle-button'), | ||
| false, | ||
| 'The completed response retained a reasoning placeholder' | ||
| ) | ||
| assert.equal( | ||
| expandedProcessingSnapshot.text.includes(REASONING_SUMMARY), | ||
| false, | ||
| 'The collapsed reasoning disclosure exposed its full summary' | ||
| 'The completed response retained its reasoning summary' |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Align the regression assertions with the documented reasoning lifecycle.
The scenario currently checks only post-completion removal. The two guides state that it verifies the live reasoning row, but the mock completes the reasoning stream before the snapshot.
wework/e2e/desktop/scenarios/streaming-text.scenario.mjs#L711-L719: hold the mock before terminal events and assert the activethinkingrow and current summary.docs/en/wework/developer-guide/wework-e2e-automation.md#L110-L110: update the sentence after the active assertion exists, or document only completion cleanup.docs/zh/wework/developer-guide/wework-e2e-automation.md#L110-L110: apply the same correction in Chinese.
As per coding guidelines, changed core user flows must have corresponding automated E2E regression coverage.
Based on the PR objective, the live reasoning preview is required behavior.
📍 Affects 3 files
wework/e2e/desktop/scenarios/streaming-text.scenario.mjs#L711-L719(this comment)docs/en/wework/developer-guide/wework-e2e-automation.md#L110-L110docs/zh/wework/developer-guide/wework-e2e-automation.md#L110-L110
🤖 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/e2e/desktop/scenarios/streaming-text.scenario.mjs` around lines 711 -
719, Update wework/e2e/desktop/scenarios/streaming-text.scenario.mjs:711-719 to
pause the reasoning mock before terminal events and assert the active thinking
row and current reasoning summary, then retain completion-cleanup assertions
after releasing the mock. Update
docs/en/wework/developer-guide/wework-e2e-automation.md:110 and
docs/zh/wework/developer-guide/wework-e2e-automation.md:110 to accurately
describe the live reasoning assertion, or document only completion cleanup if
that is the intended coverage.
Source: Coding guidelines
Comment on lines
+711
to
+721
| assert.equal( | ||
| expandedProcessingSnapshot.testIds.includes('thinking-toggle-button'), | ||
| false, | ||
| 'The completed response retained a reasoning placeholder' | ||
| ) | ||
| assert.equal( | ||
| expandedProcessingSnapshot.text.includes(REASONING_SUMMARY), | ||
| false, | ||
| 'The collapsed reasoning disclosure exposed its full summary' | ||
| 'The completed response retained its reasoning summary' | ||
| ) | ||
| await capture(control, 'streaming-text-01-reasoning-collapsed.png') | ||
| await control.command('click', '[data-testid="thinking-toggle-button"]') | ||
| await control.command('waitFor', '[data-testid="thinking-detail"]', { | ||
| text: REASONING_SUMMARY, | ||
| timeoutMs: uiTimeoutMs, | ||
| }) | ||
| await capture(control, 'streaming-text-02-reasoning-expanded.png') | ||
| await capture(control, 'streaming-text-01-reasoning-removed.png') |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wait for the terminal state before reading the snapshot.
The assistant text can render before the following response.completed event finishes the lifecycle update. This immediate snapshot can still contain the reasoning block and make the assertion or screenshot flaky. Wait for the stable send-message state before taking expandedProcessingSnapshot, or poll until both absence conditions are true.
Proposed synchronization
+ await control.command(
+ 'waitFor',
+ `${ACTIVE_WORKBENCH_SELECTOR} [data-testid="send-message-button"]`,
+ { stableMs: 750, timeoutMs: uiTimeoutMs }
+ )
const expandedProcessingSnapshot = JSON.parse(Based on the existing completion wait pattern in this scenario.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| assert.equal( | |
| expandedProcessingSnapshot.testIds.includes('thinking-toggle-button'), | |
| false, | |
| 'The completed response retained a reasoning placeholder' | |
| ) | |
| assert.equal( | |
| expandedProcessingSnapshot.text.includes(REASONING_SUMMARY), | |
| false, | |
| 'The collapsed reasoning disclosure exposed its full summary' | |
| 'The completed response retained its reasoning summary' | |
| ) | |
| await capture(control, 'streaming-text-01-reasoning-collapsed.png') | |
| await control.command('click', '[data-testid="thinking-toggle-button"]') | |
| await control.command('waitFor', '[data-testid="thinking-detail"]', { | |
| text: REASONING_SUMMARY, | |
| timeoutMs: uiTimeoutMs, | |
| }) | |
| await capture(control, 'streaming-text-02-reasoning-expanded.png') | |
| await capture(control, 'streaming-text-01-reasoning-removed.png') | |
| await control.command( | |
| 'waitFor', | |
| `${ACTIVE_WORKBENCH_SELECTOR} [data-testid="send-message-button"]`, | |
| { stableMs: 750, timeoutMs: uiTimeoutMs } | |
| ) | |
| const expandedProcessingSnapshot = JSON.parse( | |
| ... | |
| ) | |
| assert.equal( | |
| expandedProcessingSnapshot.testIds.includes('thinking-toggle-button'), | |
| false, | |
| 'The completed response retained a reasoning placeholder' | |
| ) | |
| assert.equal( | |
| expandedProcessingSnapshot.text.includes(REASONING_SUMMARY), | |
| false, | |
| 'The completed response retained its reasoning summary' | |
| ) | |
| await capture(control, 'streaming-text-01-reasoning-removed.png') |
🤖 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/e2e/desktop/scenarios/streaming-text.scenario.mjs` around lines 711 -
721, Synchronize the reasoning-removal assertions in the streaming scenario with
the terminal response state before reading expandedProcessingSnapshot. Reuse the
scenario’s existing completion-wait pattern, or poll until both
thinking-toggle-button and REASONING_SUMMARY are absent, then perform the
assertions and capture the screenshot.
| const [expanded, setExpanded] = useState(false) | ||
|
|
||
| if (!block.content) return null | ||
| if (!block.content || !isRunning) return null |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Suppress whitespace-only reasoning previews.
Line 864 treats whitespace-only block.content as active reasoning. buildBlockPreview() then returns an empty string and renders an empty live preview with the updating fallback. Require non-whitespace content before rendering.
Proposed fix
- if (!block.content || !isRunning) return null
+ if (!block.content.trim() || !isRunning) return null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!block.content || !isRunning) return null | |
| if (!block.content.trim() || !isRunning) return null |
🤖 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/blocks/ToolBlockItem.tsx` at line 864, Update the
guard in the reasoning preview rendering path around buildBlockPreview so
block.content is rejected when it is empty or contains only whitespace, while
preserving the existing isRunning check and rendering behavior for substantive
content.
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
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.
What changed
Why
Completed reasoning blocks were retained as repeated “思考过程 · N 字符” rows, which made the conversation timeline noisy even though reasoning is only useful as live progress feedback.
Validation
ai:verifyrun confirmed live “正在思考” feedback and no completed reasoning placeholderSummary by CodeRabbit
Enhancements
Documentation
Tests