fix: preserve chat stream moderation results - #616
Conversation
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 33896536957 --repo openai/openai-ruby \
--name castiron-custom-code-33896536957-1 --dir /tmp/castiron-custom-code-33896536957-1
git apply --stat /tmp/castiron-custom-code-33896536957-1/custom-code.patch
cat /tmp/castiron-custom-code-33896536957-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 91d0e86ed995f54361b329d7641af9f20163f27d ba771cec1145abc878a730b91f701e529e15d063
python3 scripts/castiron/custom_code_report.py report \
--base 91d0e86ed995f54361b329d7641af9f20163f27d \
--head ba771cec1145abc878a730b91f701e529e15d063 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-ba771cec1145
cat /tmp/castiron-custom-code-ba771cec1145/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
Treating moderation as sticky stream state is the correct accumulation rule: a moderation-only terminal chunk must update the snapshot, while later omitted or nil values must not erase an earlier result. The raw-stream parity assertions cover that boundary well.
markstuart-oai
left a comment
There was a problem hiding this comment.
The stream accumulator now carries moderation into the initial snapshot and updates it only from later non-nil chunks, preserving the existing sticky stream-state semantics. The focused public-API coverage exercises final moderation-only chunks, initial values, nil/omitted updates, and raw-stream isolation. Exact-head CI is green across the Ruby test matrix, lint, types, packaging, CodeQL, and Castiron. I could not rerun the focused test locally because the required Ruby 4.0.6 toolchain is not installed on this review box.
Problem
Moderated Chat Completions can deliver the existing moderation field on streamed raw chunks, including a final moderation-only chunk with empty choices. The chat streaming helper accumulated content and usage but dropped moderation, so current snapshots and get_final_completion returned nil even though stream_raw exposed the result.
User impact
Applications using client.chat.completions.stream for moderated completions now receive the same moderation result already available through client.chat.completions.stream_raw. Unmoderated streams remain nil.
Fix
The helper now carries moderation into the initial parsed snapshot and, for later non-nil chunks, deep-dumps and coerces the field into the existing final ChatCompletion moderation type. Omitted or explicit nil later values do not erase a previously received moderation result. Raw chunks are not mutated.
Synthetic regression proof
The focused WebMock SSE test uses only fake local data through the public stream_raw and stream APIs. Before this fix, a final empty-choices moderation chunk retained nested moderation data in stream_raw while helper snapshots and final completion returned nil. After this fix, snapshots and final completion retain the same nested input/output success and error variants, boolean flags, and numeric scores. The test also covers initial moderation surviving later omitted and nil fields, and the unmoderated nil control.
Compatibility and scope
No public signatures, generated models, dependencies, transport behavior, parsing timing, event types, safety policy, tool-call accumulation, logprob accumulation, or payload limits change. Content, choices, usage, event ordering, and raw chunk values remain covered. The diff is limited to the handwritten stream helper and one focused regression test.
Verification
Limitations
This is an offline synthetic regression proof. It does not claim live server observation and does not introduce moderation enforcement.