Revert "[core] Support interleaved response items" - #31261
Conversation
This reverts commit 8917244.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c343e7a898
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let (Some(delta), Some(summary_index)) = (event.delta, event.summary_index) { | ||
| return Ok(Some(ResponseEvent::ReasoningSummaryDelta { | ||
| item_id, | ||
| delta, | ||
| summary_index, | ||
| })); |
There was a problem hiding this comment.
When the Responses stream interleaves a reasoning item with a later message/tool item and then sends another response.reasoning_summary_text.delta or response.reasoning_summary_part.added for the original reasoning item, this code drops the server-provided item_id; turn.rs then emits the app-server reasoning summary notification using whichever item is currently active, so clients can receive deltas/section breaks attached to the wrong item or lose them after the active item changes. Please keep the item_id on these events and route by that ID rather than active-item state.
AGENTS.md reference: AGENTS.md:L102-L107
Useful? React with 👍 / 👎.
| if let Some((_, mut consumer)) = active_tool_argument_diff_consumer.take() | ||
| && let Ok(Some(event)) = consumer.finish() | ||
| { | ||
| sess.send_event(&turn_context, event).await; | ||
| } |
There was a problem hiding this comment.
When an apply_patch custom tool call is streaming argument deltas and any unrelated output_item.done arrives before that custom tool call completes, this unconditional take() calls finish() on the patch diff consumer and drops it; later response.custom_tool_call_input.delta events for the same call are ignored, so clients stop receiving correct item/fileChange/patchUpdated progress even though the final tool call may still apply. Please only finish the consumer when the completed item is the matching custom tool call.
AGENTS.md reference: AGENTS.md:L104-L107
Useful? React with 👍 / 👎.
| let previously_active_item = active_item.take(); | ||
| let previously_streamed_item = if active_item_is_streaming_to_client { | ||
| previously_active_item | ||
| } else { | ||
| None | ||
| }; | ||
| let previously_streamed_item = completed_item_id | ||
| .as_deref() | ||
| .and_then(|item_id| streamed_items.remove(item_id)); | ||
| if completes_active_item { | ||
| active_item = None; | ||
| active_item_is_streaming_to_client = false; | ||
| } | ||
| active_item_is_streaming_to_client = false; |
There was a problem hiding this comment.
When the stream completes an earlier non-tool item while a later item is currently active, this unconditional active_item.take() treats the later item as the completed item's prior start and clears it. In an interleaved response such as reasoning started, message started, message done, reasoning done, the reasoning item can then be emitted as started twice (or have its buffered text lifecycle finalized against the wrong item), which corrupts the app-server/TUI item lifecycle for valid out-of-order output_item.done events. Please look up the completed item by its item ID and only clear the active slot when that ID matches.
AGENTS.md reference: AGENTS.md:L104-L107
Useful? React with 👍 / 👎.
Reverts #30876