refactor(ai): drop reasoning items when they finish - #47100
Open
rekram1-node wants to merge 1 commit into
Open
Conversation
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.
Why
ParserState.reasoningItemsis the parser's working set for in-flight reasoning items: per-summary_indexfragment status, which indexes streamed deltas, and the encrypted content carried onto later summary parts. Since #45789 an entry was never removed when its item finished;output_item.doneflippedopen: falseand left the dead entry in the map for the rest of the stream. Every later handler then checked!item?.opento ignore a reappearance of that id, andonOutputItemDoneshort-circuited onopen === false.That is the same replay tombstone pattern removed for messages and function calls in #46965. A finished reasoning item can only reappear on a resumed stream (
background: true+starting_after), which our client never requests, so theopenflag guarded inputs the parser cannot receive. The real fix that shipped alongside it in #45789 is the overlap check instepfor a different reasoning item starting while one is still open; that check readslifecycle.reasoning, not the dead entry, and is unchanged here.What changes
ReasoningStreamItemlosesopen. An entry exists while the item is in flight and is deleted onoutput_item.done, the same shape asstate.tools.!item?.openguards become!item. Deltas, summary-part events, and done finals for an unknown or finished item are still no-ops.onOutputItemDoneno longer early-returns foropen === false; the done-only path emitsreasoning-start/reasoning-endfrom the item and records nothing.lifecycle.reasoning.has(item.id)with a bare item id was always false because that set only holds${id}:${index}keys.done, re-added, and post-done delta events from two lifecycle fixtures and the openai-responses dedup fixture; remaining assertions are unchanged. Duplicateaddedfor a still-pending reasoning item is still covered and still a no-op.Verification