fix: prevent stale todo list from overwriting current state#301623
Closed
pdrgds wants to merge 3 commits intomicrosoft:mainfrom
Closed
fix: prevent stale todo list from overwriting current state#301623pdrgds wants to merge 3 commits intomicrosoft:mainfrom
pdrgds wants to merge 3 commits intomicrosoft:mainfrom
Conversation
…crosoft#299234) ChatToolInvocationPart's constructor called setTodos() from toolSpecificData on every render. When parts were constructed out of chronological order (lazy rendering, collapsed sections, scroll virtualization), an older invocation's data overwrote the current state. Move todo list hydration from the UI part constructor to the two proper data entry points in ChatModel: _deserializeRequest() for session restore and acceptResponseProgress() for live background agent data.
…ale-hydration-299234
…crosoft#299234) - Extract shared `deserializeTodoList()` helper to eliminate duplicated mapping logic between deserialization and acceptResponseProgress paths - Tighten multi-request deserialization test to assert exact call count and verify both intermediate and final hydration states - Add test for acceptResponseProgress runtime hydration path
Author
|
Closing this PR while seeking guidance on the preferred approach. See my comment on #299234 for two alternative fix strategies. |
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.
Summary
Fixes #299234
ChatToolInvocationPart's constructor calledsetTodos()fromtoolSpecificDataon every render. When parts were constructed out of chronological order (lazy rendering, collapsed sections, scroll virtualization), an older invocation's data overwrote the current state.setTodos()side-effect from the UI constructor. Move todo list hydration to the two proper data entry points inChatModel:_deserializeRequest()— for session restore from storage (scans all parts, hydrates only the latest)acceptResponseProgress()— for live background agent data arriving viatoolInvocationSerializedTest plan
chatToolInvocationPart.test.ts— instantiates realChatToolInvocationPartwith stale todoList data, assertssetTodosis not called. Fails on main, passes on this branch.chatModel.test.ts— verifies deserialization hydrates todo list correctly (single and multiple invocations, latest state wins)manageTodoListTool.test.ts— write/read operations,pastTenseMessagegeneration, steering mid-task consistency