Inline response recording and remove process_items indirection #7310

aibrahim-oai · 2025-11-26T03:17:00Z

Inline response recording during streaming: run_turn now records items as they arrive instead of building a ProcessedResponseItem list and post‑processing via process_items.
Simplify turn handling: handle_output_item_done returns the follow‑up signal + optional tool future; needs_follow_up is set only there, and in‑flight tool futures are drained once at the end (errors logged, no extra state writes).
Flattened stream loop: removed process_items indirection and the extra output queue
- Tests: relaxed tool_parallelism::tool_results_grouped to allow any completion order while still requiring matching call/output IDs.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

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

chatgpt-codex-connector · 2025-11-26T03:20:47Z

codex-rs/core/src/codex.rs

+                                sess.record_conversation_items(
+                                    &turn_context,
+                                    std::slice::from_ref(&item),
+                                )
+                                .await;


Avoid persisting partial items before stream retries

Stream items are written to conversation history immediately in the OutputItemDone handler (record_conversation_items at lines 2225-2229), but run_turn will retry the same turn on transient stream errors (see the retry loop at lines 2113-2137). If the SSE disconnects before ResponseEvent::Completed, the partial tool call or message is already persisted and will be included in the next prompt when the turn retries, producing duplicated/partial conversation history and misleading turn diffs. The retry should happen before committing history or roll back the writes when the turn is retried.

Useful? React with 👍 / 👎.

aibrahim-oai · 2025-12-01T22:44:31Z

@codex review this

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

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

codex-rs/core/src/codex.rs

aibrahim-oai · 2025-12-01T23:08:47Z

codex-rs/core/src/codex.rs

-
-                        output.push_back(
-                            async move {
-                                Ok(ProcessedResponseItem {


That's what I mean. we were pushing to output directly

pakrym-oai · 2025-12-02T01:28:23Z

codex-rs/core/src/codex.rs

            Ok(turn_output) => {
-                let processed_items = turn_output;
+                let TurnRunResult {
+                    needs_follow_up,


do we need both of these? Can there be end of turn without assistant message?

end_of_turn_message: Option<..> ?

For OpenAI provider no. I am not sure about azure or why it was implemented like that originally

From the API point of view, yes you can. But from a client point of view, a turn is stopped when we decide so it should be agnostic from the provider

codex-rs/core/src/codex.rs

codex-rs/core/src/stream_events_utils.rs

pakrym-oai · 2025-12-02T01:35:18Z

codex-rs/core/src/stream_events_utils.rs

+/// history and rollout stay in sync even if the turn is later cancelled.
+pub(crate) type InFlightFuture<'f> = Pin<Box<dyn Future<Output = Result<()>> + Send + 'f>>;
+
+pub(crate) struct OutputItemResult {


does it feel like we unnessesarily mixing tool (long running) and non-tool items?

This type is almost like a union, either we return a future or a possible end of turn.

or reasoning

I can have the match at an earlier layer. just wanted to decrease the code in codex.rs

pakrym-oai · 2025-12-02T01:38:08Z

codex-rs/core/src/codex.rs

+                if let Some(agent_message) = output_result.last_agent_message {
+                    last_agent_message = Some(agent_message);
+                }
+                needs_follow_up |= output_result.needs_follow_up;


this value always comes from the last item, isn't it?

yes, ideally a tool call will be at the end if we need a follow up. Not the most correct if ResponsesAPI changed under us though

pakrym-oai · 2025-12-02T01:43:10Z

codex-rs/core/src/stream_events_utils.rs

+            }
+        }
+        Ok(None) => {
+            if let Some(turn_item) = handle_non_tool_response_item(&item).await {


We handle handle_non_tool_response_item for item_added in the main loop. Should we do the same for done?

jif-oai · 2025-12-02T09:35:37Z

Can you make sure this fixes the issue described here?
#7038

codex-rs/core/src/codex.rs

codex-rs/core/src/stream_events_utils.rs

codex-rs/core/src/turn_event.rs

codex-rs/core/src/stream_events_utils.rs

jif-oai · 2025-12-02T09:45:49Z

codex-rs/core/src/error.rs

 }

 impl From<CancelErr> for CodexErr {
    fn from(_: CancelErr) -> Self {


Should we at least log the error?

we log errors. which part you mean?

codex/codex-rs/core/src/codex.rs

Lines 2163 to 2165 in 5e5fc7b

Err(err) => {

tracing::error!(error = ?err, "in-flight tool future failed during drain");

}

jif-oai · 2025-12-02T09:47:46Z

codex-rs/core/src/codex.rs

            Ok(turn_output) => {
-                let processed_items = turn_output;
+                let TurnRunResult {
+                    needs_follow_up,


From the API point of view, yes you can. But from a client point of view, a turn is stopped when we decide so it should be agnostic from the provider

codex-rs/core/src/codex.rs

…t's cheap to clone

Inline response recording

387700a

chatgpt-codex-connector bot reviewed Nov 26, 2025

View reviewed changes

aibrahim-oai added 8 commits November 25, 2025 21:19

Extract turn event helpers

f2b9e7a

Silence clippy arg warning

dd29745

crate

aeff47f

crate

9cccdea

decrease mut

1ed5eda

decrease mut

3d0b8ab

linear

ebff3c7

linear

0b3876f

chatgpt-codex-connector bot reviewed Dec 1, 2025

View reviewed changes

codex-rs/core/src/codex.rs Outdated Show resolved Hide resolved

aibrahim-oai commented Dec 1, 2025

View reviewed changes

pakrym-oai reviewed Dec 2, 2025

View reviewed changes

codex-rs/core/src/codex.rs Outdated Show resolved Hide resolved

pakrym-oai reviewed Dec 2, 2025

View reviewed changes

codex-rs/core/src/codex.rs Show resolved Hide resolved

pakrym-oai reviewed Dec 2, 2025

View reviewed changes

codex-rs/core/src/codex.rs Outdated Show resolved Hide resolved

pakrym-oai reviewed Dec 2, 2025

View reviewed changes

codex-rs/core/src/stream_events_utils.rs Show resolved Hide resolved

pakrym-oai reviewed Dec 2, 2025

View reviewed changes

jif-oai reviewed Dec 2, 2025

View reviewed changes

aibrahim-oai added 7 commits December 2, 2025 11:45

You don't need to play with lifetime specifiers if you have an Arc. I…

9aa245d

…t's cheap to clone

Impl default on it. It will simplify the code below

371ab42

An Arc of Arc like this seems a bit odd

e7334e0

name

edca8ad

todo

5e5fc7b

lint

68b8a5c

parallel tests

1e93b30

parallel tests

5e53dfb

jif-oai approved these changes Dec 4, 2025

View reviewed changes

aibrahim-oai added 3 commits December 4, 2025 08:26

Merge branch 'main' into linear-processing

021d0f5

error or panic

820be70

wire

716972a

aibrahim-oai merged commit 6e6338a into main Dec 4, 2025
26 checks passed

aibrahim-oai deleted the linear-processing branch December 4, 2025 20:17

github-actions bot locked and limited conversation to collaborators Dec 4, 2025

	Err(err) => {
	tracing::error!(error = ?err, "in-flight tool future failed during drain");
	}

Inline response recording and remove process_items indirection #7310

Inline response recording and remove process_items indirection #7310

Conversation

aibrahim-oai commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

aibrahim-oai commented Dec 1, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jif-oai commented Dec 2, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aibrahim-oai commented Nov 26, 2025 •

edited

Loading