fix(rewind): false "compressed turn" error when mid-turn messages exist - #4580
Merged
Conversation
… mismatch Mid-turn user messages (typed during tool execution) were added to UI history as type 'user', causing isRealUserTurn to count them. But in the API history, they are merged into the preceding tool_result Content (alongside functionResponse), making them invisible to isUserTextContent. This UI/API count mismatch caused computeApiTruncationIndex to return -1, producing a false "Cannot rewind to a compressed turn" error. Fix: change mid-turn messages from type 'user' to type 'notification' in both live and resume paths so isRealUserTurn no longer counts them. Closes #4579
Contributor
📋 Review SummaryThis PR fixes a false "Cannot rewind to a compressed turn" error that occurs when users type messages during tool execution. The root cause is a mismatch between UI history counting and API history counting for mid-turn user messages. The fix changes mid-turn messages from 🔍 General Feedback
🎯 Specific Feedback🔵 Low
✅ Highlights
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a rewind failure caused by UI/API “user turn” counting diverging when a user submits messages mid-tool-execution. The change reclassifies those mid-turn UI entries as notification so they no longer count as real user turns, aligning rewind truncation with the API history shape.
Changes:
- Reclassify mid-turn user messages from
user→notificationin both live streaming and resume reconstruction paths. - Add
MessageType.NOTIFICATIONand update affected tests. - Add a regression test ensuring
computeApiTruncationIndexstays consistent when mid-turn text is merged into afunctionResponseAPI content.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/ui/utils/resumeHistoryUtils.ts | Restores mid_turn_user_message records as notification history items to avoid counting them as real turns. |
| packages/cli/src/ui/utils/resumeHistoryUtils.test.ts | Updates assertion to expect restored mid-turn entries as notification. |
| packages/cli/src/ui/utils/historyMapping.test.ts | Adds regression test covering mid-turn messages + hybrid tool_result content and correct truncation index. |
| packages/cli/src/ui/types.ts | Adds MessageType.NOTIFICATION = 'notification' for consistent typed usage. |
| packages/cli/src/ui/hooks/useGeminiStream.ts | Emits mid-turn drained messages into UI history as notification instead of user. |
| packages/cli/src/ui/hooks/useGeminiStream.test.tsx | Updates expectations for mid-turn drain UI items to be NOTIFICATION. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
approved these changes
May 28, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review
wenshao
approved these changes
May 28, 2026
Collaborator
PR #4580 Verification ReportBranch: Build & Type Check
Unit Tests
Code ReviewProblem solved: Mid-turn user messages (BTW messages sent during tool execution) were stored in UI history as Fix: Change mid-turn messages from
Why this is correct: VerdictReady to merge. Clean, minimal fix that correctly separates mid-turn display items from real user turns. The new — wenshao |
yiliang114
approved these changes
May 29, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Clean fix — root cause is well understood and the change is minimal. The only minor concern is that mid-turn messages now render as InfoMessage instead of UserMessage (visual identity change), but that's a reasonable trade-off for correctness.
| @@ -2347,8 +2347,7 @@ export const useGeminiStream = ( | |||
| config | |||
| .getChatRecordingService() | |||
Collaborator
There was a problem hiding this comment.
[Suggestion] Minor UX concern: mid-turn messages now render as <InfoMessage> (dimmed/grey) instead of <UserMessage> (prominent > prefix). Users might not recognize their own mid-turn text in the transcript. Consider whether a distinguishing visual treatment is warranted (e.g., a dimmed variant of UserMessage).
Not blocking — the correctness fix is more important than the visual regression.
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.
Summary
type: 'user'totype: 'notification'in UI history soisRealUserTurnno longer counts them, eliminating the UI/API turn count mismatch that causedcomputeApiTruncationIndexto return -1Closes #4579
Root Cause
When a user types during tool execution, the mid-turn drain (
useGeminiStream.ts:2346) merges the text into the same API Content asfunctionResponseparts. But it also adds a separatetype: 'user'item to UI history (useGeminiStream.ts:2351).isUserTextContentskips the hybrid API entry (hasfunctionResponse), whileisRealUserTurncounts the UI item → mismatch → rewind fails.Changes
types.tsNOTIFICATION = 'notification'toMessageTypeenumuseGeminiStream.ts:2351MessageType.USER→MessageType.NOTIFICATION(live path)resumeHistoryUtils.ts:290type: 'user'→type: 'notification'(resume path)useGeminiStream.test.tsxresumeHistoryUtils.test.tshistoryMapping.test.tsTest plan
npx vitest run packages/cli/src/ui/utils/historyMapping.test.ts— 17 tests passnpx vitest run packages/cli/src/ui/utils/resumeHistoryUtils.test.ts— 5 tests passnpx vitest run packages/cli/src/ui/hooks/useGeminiStream.test.tsx— 101 tests passqwen→ ask model to call 3 tools → type "插入" during execution →/rewindlast turn → should succeedqwen --continue→/rewind→ should succeed🤖 Generated with Qwen Code