Conversation
`openai audio:transcriptions create --temperature inf` (also -inf, nan, infinity, in any case) exited 0 and put the literal bytes "+Inf"/"NaN" in the multipart temperature part, sending a value no API accepts and reporting success. Piped YAML bodies (`temperature: .inf` on stdin) reached the same multipart encoder without going through typed flags, so they sent "+Inf" too. JSON-body endpoints only failed late during marshaling with "json: unsupported value: +Inf" after flag parsing had already accepted the value.
b7bd869 to
7d6be10
Compare
|
Rebased onto current
One claim in the description could have been invalidated by the new base, so I re-checked it: Checks re-run on
Still not run: Routing note: AGENTS.md lists request parsing among the areas that need SDK CODEOWNER review, and this touches flag parsing plus multipart encoding — worth tagging whoever owns that. The three workflow runs on this head are still queued for a maintainer to approve. |
What happens
--temperature inf(also-inf,nan,infinity, in any case) is accepted by flag parsing, and the multipart encoder then writes the literal bytes+InforNaNinto the part body. The request is sent, the API rejects it, and the CLI's own failure is only the server's confusing message. Piped YAML reaches the same encoder without ever going through a typed flag, and JSON-body endpoints fail late during marshaling withjson: unsupported value: +Infafter flag parsing had already accepted the value.Measured on
4b75e9bagainst a local capture listener, both with exit code 0:What changes
Both boundaries now reject non-finite floats:
parseCLIArgroutesfloat64and*float64through aparseFiniteFloathelper that returns the same*strconv.NumErrorit returns for malformed numeric input, so the existing flag-error wrapper reportsinvalid value "inf" for flag -temperature: strconv.ParseFloat: parsing "inf": invalid syntax.apiformroutes every float it formats through aformatFloathelper that returnsapiform: unsupported value: +Inf, which also covers the piped-YAML route and the comma-array, repeat and indexed array encoders.No CLI surface is added or changed: same flags, same accepted values for every finite number, same defaults. Only values that previously produced an unwireable request now fail during parsing.
Encoding is unchanged for finite values, including the cases covered by #195 — exponent form still writes
1000for1e3, and negative zero still writes-0. Both are now pinned by the existingTestEncodetable so the rejection cannot be widened into them by accident.Scope and limits
internal/apiquery/encoder.go:164-168formatsfloat32/float64query parameters the same way, andstrconv.FormatFloatwould renderNaN/+Infthere too. I left it alone rather than guarding it: there is currently nofloat32/float64value anywhere underpkg/cmd/, so no query parameter can reach that arm. If a future generated command adds one, it will need the same check. Say so if you would rather have it handled now for symmetry.Testing
go1.25.0 darwin/arm64. The new tests were also run against unmodified sources in a separate worktree at
4b75e9bwith only the two test files copied in, so the failure below is the defect and not the environment:--- FAILlines (49 ininternal/requestflag, 16 ininternal/apiform), with the wire bytes in the message, e.g.expected an error encoding map[temperature:+Inf], got body "...name=\"foo\"\r\n\r\n+Inf".go test -count=1 ./internal/requestflag/ ./internal/apiform/→ bothok, rc=0.go test -count=1 ./internal/...→ rc=0, 9 packages ok, 0 failures.go test ./... -run '^$'→ rc=0 (compiles every package, includingpkg/cmd)../scripts/lint→ rc=0.go vet ./internal/apiform/ ./internal/requestflag/→ rc=0.gofmt -lon the four changed files → empty.go mod verify→ all modules verified.requestflag.goreverted,internal/apiformstaysokwhileinternal/requestflagfails; with onlyencoder.goreverted,internal/requestflagstaysokwhileinternal/apiformfails, and the piped-YAML command sends+Infagain with exit 0.Not run:
./scripts/test, which starts the source-based local mock server and installs the pinned Deno runtime. Thepkg/cmdmock-server integration tests are therefore compile-checked here, not executed.Both files are handwritten and outside
pkg/cmd/, so they are not Castiron-generated and.castiron-ratchet.jsonis untouched.