fix: preserve binary multipart chunks - #324
Conversation
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed the adapter and all production call sites. Converting enumerated chunks to ASCII-8BIT before buffering preserves the exact bytes, eliminates UTF-8/BINARY compatibility failures, and leaves IO/StringIO behavior and transport framing unchanged. Bounded reads remain streaming with memory limited to the current source chunk/buffer; the unbounded-read path retains its existing whole-stream semantics. Regression coverage exercises both bounded and whole-stream mixed-encoding reads. Local validation: util suite 38/38 (160 assertions), focused mixed-encoding tests, an end-to-end multipart text+binary byte-integrity probe, RuboCop on both changed files, and git diff --check all pass.
|
Thank you very much for tracking this down and putting together the focused mixed-encoding fix, @Guflly. Normalizing every multipart Enumerator chunk to binary was the essential missing piece for #317, and your reproduction made the UTF-8-followed-by-binary failure concrete and straightforward to verify. We carried that exact insight into #354 and combined it with the complete I’m closing this PR because #354 supersedes it. Your contribution was crucial to the final fix, and we genuinely appreciate the clear report, targeted implementation, and regression coverage. Thank you! 🙏 |
## Summary - preserve arbitrary binary bytes when multipart request bodies yield a mix of UTF-8 and binary chunks - honor `IO#read(max_len, out_string)` byte limits, buffering, EOF, negative-length, and output-buffer semantics for Enumerator-backed bodies - retain native `IO` and `StringIO` delegation, including implicit `to_int` length conversion - update RBI/RBS contracts and add focused regression coverage This fixes openai#317 and consolidates the complete behavior needed from openai#275, openai#294, and openai#324 on the current SDK-owned `ReadIOAdapter` implementation. ## Ownership This is handwritten SDK runtime behavior introduced as an SDK-owned component in openai#345. It does not change OpenAPI inputs, Castiron facts, renderer rules, templates, or generator configuration. ## Validation - `mise exec ruby@4.0.6 -- ./scripts/test` — 531 runs, 1,736 assertions, 0 failures - `mise exec ruby@4.0.6 -- bundle exec rake lint` — RuboCop, Sorbet, and 1,211 RBS files clean - `mise exec ruby@4.0.6 -- bundle exec rake build:gem` - installed-gem adapter probe - Ruby 3.3.12 mixed-encoding compatibility probe - 1,000 randomized read sequences compared against `StringIO` - real local HTTP multipart upload with a 1 MiB binary file and UTF-8 prompt, verified byte-for-byte by SHA-256
2 participants
Summary
Fixes #317.
Validation
main