Skip to content

fix: remove duplicate accumulate_delta in assistants streaming - #3578

Open
bunlongheng wants to merge 1 commit into
openai:mainfrom
bunlongheng:fix/deduplicate-accumulate-delta-in-assistants-streaming
Open

fix: remove duplicate accumulate_delta in assistants streaming#3578
bunlongheng wants to merge 1 commit into
openai:mainfrom
bunlongheng:fix/deduplicate-accumulate-delta-in-assistants-streaming

Conversation

@bunlongheng

Copy link
Copy Markdown

Summary

lib/streaming/_assistants.py contained a private copy of accumulate_delta (lines 983-1041) that was byte-for-byte identical to the canonical version in lib/streaming/_deltas.py:6-64.

The chat streaming path already imports from _deltas (see lib/streaming/chat/_completions.py:25), but assistants streaming maintained its own duplicate - meaning any bugfix to one copy would silently miss the other.

Change

  • Remove the 59-line duplicate from _assistants.py
  • Import accumulate_delta from ._deltas (same as chat streaming already does)
  • Remove the now-unused is_dict/is_list imports from _utils

Verification

ruff check src/openai/lib/streaming/_assistants.py  # all checks passed
pytest tests/lib/test_assistants.py tests/test_streaming.py  # 28 passed

No behavior change - both call sites (lines 910, 961) continue calling the same function, now through the shared import.

@bunlongheng
bunlongheng requested a review from a team as a code owner August 5, 2026 15:35

@ting-hong-shieh ting-hong-shieh 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.

Reviewed head 73d248d63125558e1a670f19af955c9186839edf against current main at d9029e3ada3c008b4631d78a425743445730892a.

I independently compared the two implementations at the PR's base: their accumulate_delta function ASTs are identical, covering lines 6–64 in _deltas.py and 983–1041 in _assistants.py. The head applies cleanly to current main, and the shared import preserves the private _assistants.accumulate_delta module attribute.

Validation on the current-main merge:

  • pytest -q tests/lib/test_assistants.py tests/test_streaming.py -n 0 — 28 passed
  • Ruff lint and format checks on _assistants.py — passed
  • Pyright on _assistants.py — 0 errors
  • git diff --check — passed

The scoped Mypy command reports nine errors in the unrelated _files.py; the same command produces the same nine errors on unmodified d9029e3a, so this PR introduces no Mypy delta.

I found no correctness or compatibility regression in this deduplication. All checks were local; no API request, credentials, network service, or audio hardware was used.

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