Skip to content

fix: merge streaming tool deltas by logical index - #3329

Closed
FU-max-boop wants to merge 3 commits into
openai:mainfrom
FU-max-boop:fix/merge-duplicate-tool-call-deltas
Closed

FU-max-boop wants to merge 3 commits into
openai:mainfrom
FU-max-boop:fix/merge-duplicate-tool-call-deltas

Conversation

@FU-max-boop

@FU-max-boop FU-max-boop commented May 28, 2026

Copy link
Copy Markdown

Summary

Fixes #3201.

  • Collapse duplicate indexed list deltas in the initial streaming chunk before storing them in the accumulator.
  • Merge later indexed entries by their logical index field, with positional fallback for full Assistants snapshots that omit the delta-only index.
  • Reuse the chat streaming delta accumulator from Assistants streaming so both paths share the fix.

The branch is rebased onto the current main; the Assistants import conflict was resolved while preserving the current timeout handling.

Validation

  • pytest tests/lib/test_streaming_deltas.py tests/lib/chat/test_completions_streaming.py -q -o addopts="" — 17 passed
  • ruff check src/openai/lib/streaming/_deltas.py src/openai/lib/streaming/_assistants.py tests/lib/test_streaming_deltas.py
  • ruff format --check src/openai/lib/streaming/_deltas.py src/openai/lib/streaming/_assistants.py tests/lib/test_streaming_deltas.py
  • scripts/run-pyright src/openai/lib/streaming/_deltas.py src/openai/lib/streaming/_assistants.py tests/lib/test_streaming_deltas.py
  • git diff --check

@FU-max-boop
FU-max-boop requested a review from a team as a code owner May 28, 2026 18:48
@FU-max-boop
FU-max-boop force-pushed the fix/merge-duplicate-tool-call-deltas branch from 442dbb7 to 4f72662 Compare August 12, 2026 07:45

Copy link
Copy Markdown
Author

Rebased this fix onto the current main and resolved the Assistants import conflict while preserving the newer timeout handling. The focused streaming suite is green (17 tests), and Ruff, Pyright, formatting, and diff checks all pass.

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

Copy link
Copy Markdown

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: 4f7266274a

ℹ️ 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 thread src/openai/lib/streaming/_deltas.py

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

Copy link
Copy Markdown

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: b5258e82e2

ℹ️ 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 thread src/openai/lib/streaming/_deltas.py
@marcuswood-oai

Copy link
Copy Markdown
Contributor

Thanks for taking the time to work on this! We’ve merged the fix for duplicate-index tool-call deltas in #3425, so I’m closing this as a duplicate. We appreciate the contribution.

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.

Streaming tool_call deltas with duplicate indexes in first chunk are accumulated incorrectly

2 participants