Repository navigation
fix(realtime): honor native options for legacy beta sockets - #4120
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 codeEvaluated main: Trusted main policy: ✅ No new custom-code files detected. 39 mixed files remain; 0 existing customizations changed. Compared 39 existing customizations unchanged
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 39340595149 --repo openai/openai-python \
--name castiron-custom-code-39340595149-1 --dir /tmp/castiron-custom-code-39340595149-1
git apply --stat /tmp/castiron-custom-code-39340595149-1/custom-code.patch
cat /tmp/castiron-custom-code-39340595149-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 90c0bacc07ddd5662598db52b3c3011973011973 0a5cb26619c2cdaa605f10bba28cc4c0674cc46f
python3 scripts/castiron/custom_code_report.py report \
--base 90c0bacc07ddd5662598db52b3c3011973011973 \
--head 0a5cb26619c2cdaa605f10bba28cc4c0674cc46f --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-0a5cb26619c2
cat /tmp/castiron-custom-code-0a5cb26619c2/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at 0a5cb26. Two static reviews of the complete three-file diff and relevant connection managers, transport wrappers and option types found no actionable issues. Both Beta Realtime transports default max_size to None while preserving explicit caller values, including zero, without mutating the options dictionary. Sync signatures use the existing sync options type; async retains write_limit. Regressions cover oversized messages and explicit-limit rejection.
Hosted checks are successful/skipped; OkTest reports 236/236 passed. No candidate code or tests were executed.
3 participants
Legacy
client.beta.realtime.connect()inherited a 1 MiB native receive limit, so a larger event closed the connection with code 1009 unless callers already suppliedmax_size=None. Its synchronous option type also advertisedwrite_limit, which only native async connections support.Default to no receive size limit when the beta connection opens, while keeping every caller value, including finite limits and zero. Use the existing synchronous options type for beta sync connections; the legacy async type and its supported write limit stay available. This changes only the SDK-owned beta path.
Before the change, both default-large cases failed. Local native tests now pass on the minimum
websockets==13.0(15/15) andwebsockets==15.0.1(46/46 including Realtime recovery controls). Tests compare the whole received payload, check code 1009 for caller limits, and verify that the beta sync type rejects the async-only literal while beta async still accepts it.