Conversation
…t omit them ChatCompletionStreamState._accumulate_chunk assigned both from every chunk unconditionally, so a trailing chunk that omits them wiped values the stream had already reported from the snapshot and from get_final_completion(). Guard both the way the adjacent moderation assignment already does.
cultosagent
added a commit
to cultosagent/dogma-registry
that referenced
this pull request
Sep 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes being requested
ChatCompletionStreamState._accumulate_chunkwrotesnapshot.usageandsnapshot.system_fingerprintfrom every chunk unconditionally:The two lines above the
moderationguard overwrite with whatever the current chunk carries, which for most chunks isNone. So a stream that reports usage on one chunk and then sends a trailing chunk that omits it loses both fields — fromcurrent_completion_snapshot, from every subsequentChunkEvent.snapshot, and fromget_final_completion().That trailing-chunk shape is not hypothetical: it is what
tests/lib/chat/test_stream_moderation.pymodels, where a metadata chunk arrives after the content chunks. #3864 established the rule formoderation— a later chunk that omits the field retains the last report — but the two siblings immediately above it were left unconditional. This applies the same guard to both.src/openai/lib/streaming/chat/_completions.py+4/-2, plus a new testtests/lib/chat/test_stream_metadata_retention.pythat mirrors the moderation test's structure with usage and the fingerprint reported on the first chunk and a trailing moderation chunk after them.Reproduction
Before the change, driving the real
.stream()path through a mocked SSE response (usageandsystem_fingerprinton chunk 0, a trailing moderation chunk at index 2), the per-chunk snapshots ofusage.total_tokensare:Driving
ChatCompletionStreamStatedirectly with the same chunk order — which is what.stream()accumulates through — shows the final object losing both fields while the sibling keeps its value:The same stream with
usageon the last chunk loses nothing, which is why the existing suite did not catch it — the loss is order-dependent. Reported values are never cleared by this change: a later chunk that does report usage still replaces the earlier one.Verification
main@bccad312, openai 3.16.1, Python 3.10.16, pydantic 2.12.5, macOS arm64,uv sync --frozen --all-extrasthenuv run --frozen --no-sync. The new test file is present in both runs, so the totals are directly comparable.pytest -o addopts= -q)main, fix reverted)tests/lib/chat/test_stream_metadata_retention.pyAt index 1 diff: None != 11tests/lib/chat/test_stream_moderation.pytests/lib/chat tests/lib/streaming tests/lib/responsestests/lib(full)I diffed the sorted
FAILEDlists from the two full runs: the only difference is the two new tests disappearing from it. The remaining 40 failures are pre-existing and identical on both trees — all intests/lib/test_fine_tuning_positional_arguments.py, which needs a local echo server.ruff format --checkandruff checkare clean on both changed files.Notes
_accumulate_chunkis already guarded:finish_reason(if choice.finish_reason:),logprobs(if choice.logprobs is not None:) andmoderation(if chunk.moderation is not None:, added by fix: preserve chat stream moderation results #3864).usageandsystem_fingerprintwere the only two unconditional ones left.finish_reason == "length"the code raisesLengthFinishReasonError(completion=completion_snapshot)with the comment "at the time of writing,.usagewill always beNonebut we include it here in case that is changed in the future". After this change the snapshot can carry a retainedusage, so that error now reports accurate usage — which is what the comment anticipates, not a behaviour it depends on being absent.stream_options.include_usagethe API sendsusageon the final chunk, so the guard is a no-op there.src/openai/lib/; the Castiron report should show no new custom-code files and no changed customizations.moderationguard that fix: preserve chat stream moderation results #3864 already merged, so it does not cover these two fields.Additional context & links
Follows the rule #3864 set for
moderationin the same function.