Preserve large-integer precision when decoding tool arguments - #528
Preserve large-integer precision when decoding tool arguments#528PratikDhanave wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes loss of integer precision (> 2^53) when decoding tool-call JSON arguments by switching the schema-application step to decode numbers as json.Number instead of float64, ensuring large int64/uint64 arguments round-trip correctly into typed handlers.
Changes:
- Update
applySchemato decode JSON withjson.Decoder+UseNumber()to preserve large integer precision. - Add a black-box regression test proving
Format.Unmarshalpreserves a2^53+1integer.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| agent/format/jsonformat/encoding.go | Switch argument decoding to UseNumber() in the schema/defaults path to avoid float64 truncation. |
| agent/format/jsonformat/encoding_test.go | Add regression test ensuring Unmarshal preserves large int64 values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Decode with UseNumber so integers beyond 2^53 are not silently | ||
| // truncated by being decoded into float64 and re-marshalled. | ||
| dec := json.NewDecoder(bytes.NewReader(data)) | ||
| dec.UseNumber() | ||
| if err := dec.Decode(&v); err != nil { | ||
| return nil, fmt.Errorf("unmarshaling arguments: %w", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Addressed — after Decode the code checks for io.EOF and rejects trailing data, restoring json.Unmarshal's strictness.
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "fmt" | ||
|
|
There was a problem hiding this comment.
Done — added the io import and a strict trailing-data check (dec.Token() must return io.EOF). Note: validate() can't also use UseNumber — json.Number is a string type the jsonschema validator rejects as a non-integer, which broke every test; left as a comment in the PR.
| // Decode with UseNumber so integers beyond 2^53 are not silently | ||
| // truncated by being decoded into float64 and re-marshalled. | ||
| dec := json.NewDecoder(bytes.NewReader(data)) | ||
| dec.UseNumber() | ||
| if err := dec.Decode(&v); err != nil { |
There was a problem hiding this comment.
The UseNumber decode preserves large-integer precision in the result value the tool receives. validate() re-decodes separately only for schema checking (pre-existing behavior) — json.Number is a string kind the jsonschema validator rejects as non-integer, so it can't be used there. The precision fix applies where it matters (the decoded result) without altering validation.
22af497 to
f7805b6
Compare
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
parity-approved
f7805b6 to
0496f60
Compare
This comment has been minimized.
This comment has been minimized.
applySchema decoded incoming JSON arguments into an interface{} via
json.Unmarshal, which turns every JSON number into a float64. Integer
arguments beyond 2^53 (int64/uint64) were therefore silently truncated
before being re-marshalled and handed to the typed handler — e.g. a tool
called with {"n":9007199254740993} received 9007199254740992.
Decode with json.Decoder.UseNumber() so numbers are preserved as json.Number
and round-trip exactly. Only the input path (Format.Unmarshal) was affected;
the output/Normalize path already operates on the typed value.
Adds a black-box test asserting a 2^53+1 argument round-trips exactly.
Address review feedback: switching to json.Decoder for UseNumber made applySchema tolerate trailing data after the first JSON value, unlike the json.Unmarshal it replaced. Restore strict single-value parsing by requiring io.EOF after the decoded value, and add a regression test. (The related suggestion to also decode with UseNumber inside validate() is not applied: json.Number is a string type that the jsonschema validator rejects as a non-integer, so it cannot be used there.)
0496f60 to
081b78e
Compare
Parity Review — No Issues FoundThis PR fixes a silent precision bug in the unexported Scope: Both changed files ( Cross-repo parity: This is a correctness fix for Go-specific number handling ( Labels: Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Summary
applySchema(agent/format/jsonformat/encoding.go), reached viaFormat.Unmarshal, decodes incoming JSON arguments into aninterface{}withjson.Unmarshal, which turns every JSON number into afloat64. Integer arguments beyond 2^53 are therefore silently truncated before being re-marshalled and handed to the typed handler:Every
functoolcall with anint64/uint64argument is affected. (The output /Normalizepath is not — it operates on the already-typed value and doesn't round-trip through this number-losing step.)Fix
Decode with
json.Decoder+UseNumber(), so numbers are preserved asjson.Numberand round-trip exactly through the re-marshal.Public API
No exported symbols change.
Tests
Adds
TestFormat_Unmarshal_PreservesLargeIntegerPrecision(black-box): a2^53+1argument. It comes back as9007199254740992before this change and exactly9007199254740993after. Full./agent/format/jsonformatand./tool/functoolsuites pass (no regression).