fix: encode TOON from the JSON form, so agents see what the API returned (COR-14205) - #34
Open
Bradenream wants to merge 2 commits into
Open
Bradenream wants to merge 2 commits into
Bradenream wants to merge 2 commits into
Conversation
TOON is the default output in agent mode, so it is what every coding agent
reads. It was encoded from the SDK's Go values by reflection, and gotoon gets
those wrong in four ways:
- It uses a json tag verbatim as the key: "instructions,omitzero". On
`agent get`, 42 distinct keys looked like this.
- It turns an optional-nullable field (a map[bool]*T) into null even when
the field is set. `agent get --include-instructions` returned 11,934
characters of instructions as JSON and `null` as TOON, so agents never saw
the instructions.
- It prints a union as its Go wrapper (StableToolV2API: null,
StableToolV2Function: {...}) instead of the member the API returned.
- It ignores omitempty and omitzero, so every unset member of every union
was printed. `document list` came to 4.9 MB of TOON against 1.27 MB of
JSON; it is now 1.4 MB.
The --include-headers path already avoided all of this by encoding the JSON
form. jsonValue now does that for both paths: marshal with the SDK's own JSON
rules, decode into plain maps and slices, then encode. TOON carries exactly
what the json format does, and a test holds the two together. On the 4.9 MB
response, run time is unchanged (about 1.1 s, dominated by the network).
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
JSON normalization currently rounds integer values above the float64 safe range.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Updates TOON output to encode SDK responses using their JSON representation.
Changes:
- Adds shared JSON normalization before TOON encoding.
- Adds unit and CLI regression coverage for SDK tags, nullable fields, and unions.
| File | Description |
|---|---|
internal/output/output.go |
Normalizes responses through JSON before TOON encoding. |
internal/output/toon_test.go |
Adds Go regression tests for TOON rendering. |
test/toon-output.test.ts |
Adds end-to-end agent-mode TOON tests. |
Files not reviewed (1)
- internal/output/output.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…(COR-14205) Review found that jsonValue decodes every number to float64, so an integer past ±(2^53−1) reached the TOON encoder already rounded: 9007199254740993 became 9007199254740992. That predates this PR, since gotoon itself holds every number as a float64, but the helper's doc claimed an exact rendering it did not deliver. TOON now decodes numbers exactly and follows the TOON spec's rule for integers outside the encoder's numeric domain (Appendix E): a safe integer stays a number, and one beyond the range is written as a quoted decimal string, with every digit kept. Other numbers come back as the float64 they were marshaled from. JSON, YAML and jq output are unchanged; jsonValue's doc now says numbers decode to float64. Tests: 2^53+1 and 2^53−1 on both TOON paths (plain and --include-headers), plus a table test of the number conversion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TOON is the default output in agent mode (
CLAUDECODE,CURSOR_AGENT, …), so it is what every coding agent reads fromvf.output.Resulthanded the SDK's Go values straight togotoon.Encode, which walks them by reflection and gets the SDK's types wrong in four ways:agent getshowed 42 distinct keys like"instructions,omitzero".nulleven when set. They are amap[bool]*Tunderneath, and gotoon turns a map with non-string keys into null.agent get --include-instructionsreturned 11,934 characters of instructions as JSON andnullas TOON, so agents never saw the instructions.StableToolV2API: null,StableToolV2Function: {…}) instead of the member the API returned.omitemptyandomitzeroare ignored. Every unset union member is printed, sodocument listcame to 4.9 MB of TOON against 1.27 MB of JSON.The
--include-headerspath already avoided all of this by encoding the JSON form. A sharedjsonValuehelper now does the same for plain TOON: marshal with the SDK's JSON rules, decode into plain maps and slices, then encode. TOON carries exactly what--output-format jsondoes. JSON, YAML, table and pretty output are unchanged.Before and after
Measured live and read-only on a real project.
agent get: keys with tag optionsagent get --include-instructions: instructionsnulltool list --global: shapedocument list: TOON size (JSON is 1.27 MB)document list: run timeTest plan
gofmt,go vet ./...andgo test ./...pass;go.modis unchanged.internal/output/toon_test.go: 4 Go tests on real SDK types. They cover plain keys, a set optional-nullable field, a flat union, and TOON equal to the JSON output re-encoded. All 4 fail on master and pass here.test/toon-output.test.ts: 3 hermetic cases on the built binary. They run with a mock server, isolated HOME and agent mode. All 3 fail with master's binary and pass here.The 4 vitest cases that already fail on master (
docs-command×2,flag-errors,flag-raw-text) still fail here and are unrelated.Fixes COR-14205