Repository navigation
fix(wework): reclassify final text as commentary - #2423
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 Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe change adds atomic migration of streamed assistant text into completed process blocks. Replacement identifiers flow from Codex events through the workbench. Unit, runtime, desktop E2E, and documentation coverage validate the phase change. ChangesStreamed text reclassification
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Codex
participant RuntimeEvents
participant ResponseStream
participant Workbench
participant DesktopE2E
Codex->>RuntimeEvents: complete streamed text as commentary
RuntimeEvents->>ResponseStream: emit block with replacesItemId
ResponseStream->>Workbench: dispatch block_created
Workbench->>Workbench: replace matching assistant text with process block
DesktopE2E->>Workbench: verify process-only text and no duplicate final content
Possibly related PRs
🚥 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 |
![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: 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 `@executor/src/agents/codex/run_state.rs`:
- Line 228: Update the final-message clearing flow around
clear_reclassified_final_message and final_message_ids_match so it returns
unless the current item_id is Some and equals self.final_message_id; do not
treat two missing IDs as a match. Add a regression case covering identifier-less
final and commentary items, while preserving migration only for same-item phase
reversals.
In `@wework/e2e/desktop/scenarios/streaming-text.scenario.mjs`:
- Around line 829-851: Wait for the streaming turn’s completed state, including
response.completed handling, after waiting for PHASE_FLIP_TEXT and before
asserting assistant content count or taking the snapshot. Keep the assertions
and subsequent new-chat transition unchanged, using the existing
completion-state selector or wait mechanism exposed by the scenario.
🪄 Autofix
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: c402ffa1-b2de-4b94-ba86-0d2d2e3da0b4
📒 Files selected for processing (14)
docs/en/wework/developer-guide/wework-e2e-automation.mddocs/zh/wework/developer-guide/wework-e2e-automation.mdexecutor/src/agents/codex/run_state.rsexecutor/src/agents/codex/tests.rsexecutor/src/runtime_work/events.rspackages/chat-core/src/workbench-message-reducer.tswework/e2e/desktop/scenarios/streaming-text.scenario.mjswework/src/features/workbench/runtimeConversationTurns.test.tswework/src/features/workbench/runtimeConversationTurns.tswework/src/features/workbench/runtimePaneMessages.test.tswework/src/features/workbench/runtimePaneMessages.tswework/src/stream/responseApiStream.test.tswework/src/stream/responseApiStream.tswework/src/types/api.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
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
final_answeritem ascommentaryWhy
Some DeepSeek responses provisionally start as
final_answerbut complete ascommentary. Wework previously appended the completed process block without removing the already streamed assistant text, so the same text appeared twice.Impact
Only same-item phase reversals are migrated. Normal final answers and ordinary process blocks keep their existing behavior.
Root cause
The executor forwarded the completed commentary as a new
response.block.createdevent, but the event contract did not identify the assistant text item that had to be replaced. The frontend therefore retained both representations.Checks
cargo fmt --manifest-path executor/Cargo.toml --checkcargo test --manifest-path executor/Cargo.toml reclassifpnpm --dir packages/chat-core buildpnpm --dir packages/chat-core test(128 tests)pnpm --dir wework typecheckpnpm --dir wework test(269 files, 2692 tests)pnpm --dir wework e2e:desktop:streaming-textpnpm --dir wework ai:verify startsessionSummary by CodeRabbit
Bug Fixes
Tests
Documentation