fix: preserve UTF-8 SSE stream boundaries - #623
Conversation
jbeckwith-oai
added
codex-maintenance
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. |
Castiron custom code✅ No new custom-code files detected. 47 mixed files remain; 0 existing customizations changed. Compared 47 existing customizations unchanged
7 more in the full report. A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 33924590211 --repo openai/openai-ruby \
--name castiron-custom-code-33924590211-1 --dir /tmp/castiron-custom-code-33924590211-1
git apply --stat /tmp/castiron-custom-code-33924590211-1/custom-code.patch
cat /tmp/castiron-custom-code-33924590211-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 06e2c91dc349ba36d12ece05bf0b0cbf1ba0e220 5c2e65785775c15352e79f6ec2539f62c44ef547
python3 scripts/castiron/custom_code_report.py report \
--base 06e2c91dc349ba36d12ece05bf0b0cbf1ba0e220 \
--head 5c2e65785775c15352e79f6ec2539f62c44ef547 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-5c2e65785775
cat /tmp/castiron-custom-code-5c2e65785775/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Reviewed both changed files and traced the SSE/JSONL decoders through the raw and helper streaming paths. The binary buffer keeps delimiter offsets consistent across UTF-8 fragments, and stripping one BOM after line reassembly preserves embedded and later BOMs. I found no substantive issues in the encoding-label handling, source-string ownership, CR/LF handling, cleanup, or existing binary large-payload path. Validation was static: Ruby is unavailable in this environment, so I could not independently run the regression suite.
2 participants
Problem
Valid UTF-8 SSE streams could lose data at the decoder boundary when a supported custom
OpenAI::HTTPClientreturned ordinary RubyStringchunks instead of binary strings. A single leading UTF-8 BOM caused the first event to be ignored, and UTF-8-labeled fragmentation could drop later non-ASCII events and the terminal finish reason.User impact
Applications using custom HTTP transports could receive incomplete raw or helper streams even when the wire bytes were valid SSE. For example, a stream containing
你好世界 endcould return only你好世界with nostopfinish reason, and a leading BOM could removestartfromstart end.Fix
US-ASCIIprefix label to yield to later UTF-8 content.Verification
start; UTF-8 fragments lostendandstop.你好世界 endandstop.mise exec ruby@4.0.6 -- bundle exec rake test TEST=test/openai/sse_utf8_boundary_test.rb— 5 runs, 8 assertions, 0 failures, 0 errors.mise exec ruby@4.0.6 -- bundle exec rake lint— passed.mise exec ruby@4.0.6 -- bundle exec rake typecheck— passed.Compatibility and scope
The change is limited to
decode_lines/decode_sseand one focused regression test. It preserves LF/CRLF handling, ASCII/no-BOM streams, embedded BOM content, caller-owned fragment bytes and encodings, JSONL behavior, one-shot closing, and large-payload semantics. It does not change public APIs, charset policy, transport behavior, dependencies, generated files, SSE retry/event-ID semantics, EOF dispatch,[DONE], or reconnection behavior.Limitations
Verification uses deterministic offline synthetic responses through the public custom transport path; no live API examples were run.