Skip to content

[core] Support interleaved response items#30876

Open
alexi-openai wants to merge 3 commits into
mainfrom
codex/interleaved-reasoning-items
Open

[core] Support interleaved response items#30876
alexi-openai wants to merge 3 commits into
mainfrom
codex/interleaved-reasoning-items

Conversation

@alexi-openai

@alexi-openai alexi-openai commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • preserve reasoning item IDs on summary part and text delta events
  • track streamed response items by ID so reasoning summaries can continue after later items begin
  • keep TUI output complete and deduplicated when reasoning and final-answer events interleave

Stack

Validation

  • just test -p codex-api preserves_reasoning_summary_item_ids
  • just test -p codex-core interleaved_reasoning_summary_events_keep_reasoning_item_metadata
  • just test -p codex-tui live_reasoning_summary_is_not_rendered_twice_when_item_completes
  • just fix -p codex-api -p codex-core -p codex-tui
  • just fmt

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52c61a3738

ℹ️ 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".

Comment on lines +1995 to 2007
let completed_item_id = item.id().map(str::to_owned);
let previously_streamed_item = if let Some(item_id) = completed_item_id.as_deref() {
streamed_items.remove(item_id)
} else {
None
active_item
.as_ref()
.and_then(|active| streamed_items.remove(&active.id()))
};
let completes_active_item = match (&completed_item_id, active_item.as_ref()) {
(Some(completed_item_id), Some(active)) => completed_item_id == &active.id(),
(None, Some(_)) => true,
_ => false,
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve streaming tool diffs across unrelated item completions

When a custom tool call is streaming input deltas and another response item completes before that tool call's own output_item.done, this new ID-based path can process the unrelated completion while keeping the active item alive, but the handler has already take()n and finish()ed active_tool_argument_diff_consumer at the top of the arm. In interleaved streams such as custom_tool_call.added → patch deltas → reasoning.done → more patch deltas, the later deltas are ignored, so live apply-patch/tool-diff updates disappear even though the tool call is still in progress; the consumer should only be finished when the completed item is the active tool call/call_id.

Useful? React with 👍 / 👎.

Comment on lines +344 to +345
if let (Some(item_id), Some(delta), Some(summary_index)) =
(event.item_id, event.delta, event.summary_index)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep fallback for summary deltas without item IDs

For streams that emit response.reasoning_summary_text.delta (and the matching part-added event) while a reasoning item is active but do not include item_id, this new parser branch now returns Ok(None) and silently drops the live reasoning update. The previous code and existing test helper shape supported those deltas by associating them with the active reasoning item, so older/legacy response streams lose all streaming reasoning summaries even though the final reasoning item can still arrive; preserve a fallback path instead of requiring item_id unconditionally.

Useful? React with 👍 / 👎.

Comment thread codex-rs/core/src/session/turn.rs Outdated
Comment on lines +1999 to +2001
active_item
.as_ref()
.and_then(|active| streamed_items.remove(&active.id()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Don't let id-less tool completions close active items

When an interleaved output_item.done is a tool/function call without an id (the helpers and Responses payloads commonly carry only call_id), this fallback removes whichever message/reasoning item is currently active from streamed_items. A stream like reasoning.addedfunction_call.done → more reasoning_summary_text.delta will therefore report the reasoning item as unknown (or later emit a duplicate start on reasoning completion), even though the tool completion was unrelated to that active item. The fallback should not consume the active streamed item for id-less tool completions.

Useful? React with 👍 / 👎.

@dylan-hurd-oai dylan-hurd-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 final question here

Comment thread codex-rs/core/src/session/turn.rs Outdated
Comment thread codex-rs/core/src/session/turn.rs Outdated
Comment thread codex-rs/codex-api/src/sse/responses.rs
Comment thread codex-rs/core/src/session/turn.rs Outdated
let completed_item_id = item.id().map(str::to_owned);
let completes_active_item = match (&completed_item_id, active_item.as_ref()) {
(Some(completed_item_id), Some(active)) => completed_item_id == &active.id(),
(None, Some(active)) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

item_id is required for all non-tool call items coming from OpenAI, Azure, and Bedrock, so I think it's acceptable to drop items when item_id is None

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aligned - will update

@alexi-openai alexi-openai force-pushed the codex/interleaved-reasoning-items branch from d0da607 to 4d84452 Compare July 2, 2026 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants