fix(apiform): preserve float32 precision in comma arrays - #78
fix(apiform): preserve float32 precision in comma arrays#78sylvesterkaczmarek wants to merge 2 commits into
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. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
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. |
Summary
Keep multipart comma-array
float32serialization consistent with the existing scalarfloat32path.Fixes #77.
Problem
internal/apiformalready formats scalarfloat32values with 32-bit precision, butFormatCommaarrays combinefloat32andfloat64behind a 64-bit formatting call.That means the same source value can be represented differently depending only on whether it appears by itself or inside a comma-delimited form field.
For example, current
mainserializes:as:
but:
with
FormatCommaas:Root cause
The primitive form path correctly distinguishes widths:
The comma-array path instead uses one branch for both kinds and always passes
bitSize=64. Reflection returns floating-point values asfloat64, so the 64-bit formatting request preserves digits introduced by widening the originalfloat32.Fix
Split the comma-array branch by source kind:
reflect.Float32->FormatFloat(..., 32)reflect.Float64->FormatFloat(..., 64)No multipart boundaries, field names, array-format selection, integer formatting, boolean formatting, or
float64output changes.Regression coverage
The existing table-driven form encoder test now includes:
float32(0.1)control showing the existing expected representation;FormatComma[]float32{0.1, 1.5}regression requiring0.1,1.5.The comma-array case fails on current upstream
main, while the scalar control demonstrates the consistency requirement directly.Validation
The branch is based directly on upstream
mainatd082a010f7c6cacf407d8a1581446a7857f9f1bband is not behind it.Production diff: 3 additions and 1 deletion in
internal/apiform/encoder.go, plus focused table-driven coverage ininternal/apiform/form_test.go.Full repository validation is left to the repository's GitHub Actions checks.
Risk
Low. Only the textual formatting of
float32values insideFormatCommaarrays changes, and it becomes identical to the width-aware behavior already used for scalarfloat32fields.float64and all non-floating form values are unchanged.