fix: parse SSE retry metadata as decimal - #624
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 33927791253 --repo openai/openai-ruby \
--name castiron-custom-code-33927791253-1 --dir /tmp/castiron-custom-code-33927791253-1
git apply --stat /tmp/castiron-custom-code-33927791253-1/custom-code.patch
cat /tmp/castiron-custom-code-33927791253-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 6b9ee8729c3ac429cc93716e8b51bce603de3405 08fcfd1a7f6cdfd4e5af16161c7ba9c0e3122a57
python3 scripts/castiron/custom_code_report.py report \
--base 6b9ee8729c3ac429cc93716e8b51bce603de3405 \
--head 08fcfd1a7f6cdfd4e5af16161c7ba9c0e3122a57 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-08fcfd1a7f6c
cat /tmp/castiron-custom-code-08fcfd1a7f6c/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 the surrounding SSE decoding, raw Chat stream, and cleanup paths. Explicit base 10 fixes leading-zero retry values while preserving the existing digit-only guard. The regression tests cover decimal metadata and continued delivery through stream_raw for 0, 8, 08, 09, and 010. No substantive findings.
Static review only: Ruby is unavailable in this environment, so I did not run tests or independently reproduce the reported suite results.
2 participants
Problem
WHATWG SSE retry fields containing ASCII digits are decimal integers, but Ruby Integer(value) infers prefixes. On a valid Chat event stream, retry: 08 and retry: 09 raised ArgumentError before the following data event, while retry: 010 was interpreted as 8 instead of 10.
User-visible behavior
A public Chat stream_raw call now accepts legal leading-zero retry metadata and still yields the following chunk:
With the same valid synthetic data event after it, retry: 08 previously raised ArgumentError; now the chunk is delivered and the retry metadata is 8. Likewise, retry: 010 is stored as 10, not 8. Required digit-only values 0, 8, 08, 09, and 010 map to 0, 8, 8, 9, and 10.
Fix
At the existing digit-only retry conversion in decode_sse, pass explicit base 10 to Integer. The existing ASCII-digit guard is unchanged, so malformed, signed, fractional, and non-ASCII retry values remain ignored.
Scope and compatibility
The diff is limited to that one numeric conversion and a focused regression test. It does not add reconnection, retry policy, parser state, payload limits, API changes, dependencies, generated changes, or alter event data, event IDs, UTF-8/BOM handling, fragments, line endings, EOF dispatch, or stream closing behavior.
Verification
Limitations
The full suite retains one existing skipped test and emitted existing dependency/runtime warnings. No live API examples were run.