fix(apiform): support primitive pointers in comma arrays - #119
Conversation
|
@codex review |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfc696f92e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| item = reflect.Value{} | ||
| break | ||
| } | ||
| item = item.Elem() |
There was a problem hiding this comment.
Preserve float32 precision after pointer unwrapping
When a comma-form array contains a non-nil *float32, this new unwrapping path reaches the existing combined float case, which calls strconv.FormatFloat with a 64-bit size. Consequently, a value such as float32(1.2) is sent as 1.2000000476837158 rather than 1.2, unlike the scalar encoder's dedicated Float32 handling. Distinguish reflect.Float32 from reflect.Float64 when serializing these newly supported pointer elements.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 877e5ff. Pointer-unwrapped float32 values now use 32-bit formatting, with a regression for float32(1.2) producing 1.2. Focused apiform tests pass.
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Castiron custom code✅ No new custom-code files detected. 5 mixed files remain; 0 existing customizations changed. Compared 5 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 34938109832 --repo openai/openai-cli \
--name castiron-custom-code-34938109832-1 --dir /tmp/castiron-custom-code-34938109832-1
git apply --stat /tmp/castiron-custom-code-34938109832-1/custom-code.patch
cat /tmp/castiron-custom-code-34938109832-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin ac4e7cb16c9271b712c2cde5f64a0904c8815c4e 4ab802fcc66e36ebea577c0567896031e524568a
python3 scripts/castiron/custom_code_report.py report \
--base ac4e7cb16c9271b712c2cde5f64a0904c8815c4e \
--head 4ab802fcc66e36ebea577c0567896031e524568a --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-4ab802fcc66e
cat /tmp/castiron-custom-code-4ab802fcc66e/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
saioai
left a comment
There was a problem hiding this comment.
i checked the updated branch at 4ab802f. the pointer cases fail on main and pass here, and #78’s direct float32 case passes against this code too. i ran the apiform race tests 10 times, plus the internal tests, compilation, vet, and module checks. those passed locally, and github’s checks are green too. looks good to me.
Automated Release PR --- ## [1.16.0](openai/openai-cli@v1.15.0...v1.16.0) (2026-09-22) ### Features * **api:** add external storage and safety case commands ([openai#229](openai#229)) ([c30c961](openai@c30c961)) * **api:** add prompt-cache prewarming ([openai#196](openai#196)) ([9032f58](openai@9032f58)) * **api:** add response transformation hooks ([openai#223](openai#223)) ([8de34a7](openai@8de34a7)) * **api:** add webhook endpoint management ([openai#201](openai#201)) ([dea465a](openai@dea465a)) * **cli:** support custom request headers ([openai#190](openai#190)) ([07e3ed9](openai@07e3ed9)) ### Bug Fixes * **apiform:** preserve float32 precision in comma arrays ([openai#78](openai#78)) ([774612f](openai@774612f)) * **apiform:** strip path components from upload filenames ([openai#120](openai#120)) ([a92e143](openai@a92e143)) * **apiform:** support primitive pointers in comma arrays ([openai#119](openai#119)) ([9d146cd](openai@9d146cd)) * **apiquery:** preserve narrow numeric parameters ([openai#76](openai#76)) ([b152780](openai@b152780)) * **apiquery:** reject complex elements in comma arrays ([openai#118](openai#118)) ([46faf2c](openai@46faf2c)) * **apiquery:** reject non-string map keys ([openai#88](openai#88)) ([687a097](openai@687a097)) * **autocomplete:** omit hidden flags from suggestions ([openai#122](openai#122)) ([137d59b](openai@137d59b)) * **cmd:** include backslash in path detection for @ file references ([openai#38](openai#38)) ([f984eb1](openai@f984eb1)) * **debug:** redact sensitive response headers ([openai#32](openai#32)) ([ee62f85](openai@ee62f85)) * **explore:** avoid panic when printing an empty result set ([openai#68](openai#68)) ([f74838c](openai@f74838c)) * install Linux package binaries under /usr/bin ([openai#34](openai#34)) ([ac4e7cb](openai@ac4e7cb)) * **jsonview:** avoid width underflow in static string rendering ([openai#117](openai#117)) ([cbdf265](openai@cbdf265)) * **jsonview:** preserve literal object keys in pretty output and explorer ([openai#143](openai#143)) ([25664bf](openai@25664bf)) * omit redirect destinations from multipart upload errors ([openai#189](openai#189)) ([642d511](openai@642d511)) * **output:** stop pagination at max items ([openai#43](openai#43)) ([552840e](openai@552840e)) * **requestflag:** preserve JSON numbers written in exponent form ([openai#195](openai#195)) ([20db45e](openai@20db45e)) ### Chores * **api:** clarify Live SIP call help ([openai#193](openai#193)) ([79435c4](openai@79435c4)) * **api:** document MCP connector deprecation ([openai#202](openai#202)) ([1d4e76c](openai@1d4e76c)) * **api:** update image request examples ([openai#197](openai#197)) ([4b75e9b](openai@4b75e9b)) * **deps:** bump the codeql group across 1 directory with 2 updates ([openai#161](openai#161)) ([97734f3](openai@97734f3)) * **deps:** bump the codeql group across 1 directory with 2 updates ([openai#224](openai#224)) ([0713595](openai@0713595)) * **deps:** bump the go-minor-and-patch group across 1 directory with 2 updates ([openai#144](openai#144)) ([aa1158d](openai@aa1158d)) * **deps:** update openai-go to v3.61.0 ([openai#184](openai#184)) ([de52b2e](openai@de52b2e)) * **deps:** update openai-go to v3.63.0 ([openai#198](openai#198)) ([3f2c883](openai@3f2c883)) * **deps:** update openai-go to v3.63.1 ([openai#204](openai#204)) ([eb415e5](openai@eb415e5)) * **deps:** update openai-go to v3.64.0 ([openai#206](openai#206)) ([0169bff](openai@0169bff)) * **deps:** update openai-go to v3.64.2 ([openai#225](openai#225)) ([a7859e5](openai@a7859e5)) ### Documentation * correct bootstrap dependency check description ([openai#181](openai#181)) ([4aa657c](openai@4aa657c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: openai-sdks[bot] <284451331+openai-sdks[bot]@users.noreply.github.com> Co-authored-by: saioai <vguvvala@openai.com>
Summary
Dereference non-nil pointer/interface elements when encoding multipart arrays with
FormatComma, while preserving the existing empty-string representation for nil elements.Problem
internal/apiform.encodeArrayalready special-cases nil pointer/interface elements in comma-form arrays, but non-nil pointers are never dereferenced before the primitive-type switch.As a result, a valid primitive array such as:
fails with:
even though the pointed-to values are ordinary strings and the same encoder supports primitive values directly.
Fix
Unwrap pointer and interface layers before primitive comma serialization. If a nil value is encountered at any layer, keep the existing behavior of adding an empty comma element.
Complex non-primitive elements continue to return the same error as before.
Regression coverage
Added a focused multipart regression for
[]*string{&first, nil, &second}and verify it serializes as:Validation
The branch is based directly on current upstream
main(ee92673a416c0851a5fe8907a2453db7bd450633) and contains one commit touching onlyinternal/apiform/encoder.goplus focused regression coverage. Full Go test execution is left to repository CI.Risk
Low. The change only affects pointer/interface elements inside
FormatCommaarrays. Direct primitives, nil elements, complex-element rejection, and all other multipart array formats retain their existing behavior.