Repository navigation
fix(logger): snapshot streams before deferred export - #919
Open
Gareth.Y (linjiaye929) wants to merge 1 commit into
Open
Gareth.Y (linjiaye929) wants to merge 1 commit into
Gareth.Y (linjiaye929) wants to merge 1 commit into
Conversation
Capture a stream branch when logging so caller consumption before export cannot remove its output. Keep per-attempt copies for yielded error chunks and final JSON-assembly failures. Cover real SSE parsing with the memory logger, consumption order, traced returns, and repeated resolution errors.
Gareth.Y (linjiaye929)
marked this pull request as ready for review
October 13, 2026 11:04
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This branch has not been deployed
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.
Summary
Logging a
BraintrustStreamcurrently postponescopy()until background export. If the application consumes its stream first, the logged output can becomeNone, lose a text prefix, or fail JSON assembly. This also affects a@tracedfunction that returns a stream its caller consumes once.Capture the logging branch at log time without reading chunks, then resolve it lazily. Keep a fresh per-attempt copy so yielded error chunks and final JSON-assembly errors remain errors on repeated resolution.
Validation
Python 3.12.14, actual
SSEClientover in-memory SSE bytes, and the SDK's native memory logger:test_logger.py,functions/test_stream.py, andtest_span_customizer.py: 248 passed, 4 skipped, 21 subtests passed. Skips require optional Pydantic or Python 3.14 t-strings.git diff --checkpassed.Tests run with cassette replay enabled, the Braintrust pytest plugin disabled, and a socket guard rejecting live connections. No live service or provider calls were used. Full nox/core/version-matrix, pylint/type-checker sessions, benchmarks, and
make fixupwere not run.Compatibility and limits
text_deltacan still terminate the parser and produceNoneon a later retry, as before.AI assistance
Prepared with AI assistance, including a separate local review of the reproduction, patch, tests, and compatibility boundaries.
CI status
As checked on 2026-10-13 at 11:03 UTC for head
9f6d44611c2ac07e127fda576f98f558b18c90d9, both upstream workflow runs reportcompleted / action_required:CI has not passed. The local validation above does not represent a successful upstream CI run.