Skip to content

Conversation

@aibrahim-oai
Copy link
Collaborator

  • Record stream items directly in run_turn, with tool outputs persisted on completion (unordered).
  • Drop ProcessedResponseItem/process_items; reuse shared helper for history+rollout writes.
  • Simplify TurnAborted error payload and clean up unused module.

Copy link
Contributor

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

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.

ℹ️ 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 2225 to 2229
sess.record_conversation_items(
&turn_context,
std::slice::from_ref(&item),
)
.await;
Copy link
Contributor

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

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