feat: add TOON usage output - #3021
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6df165e4ed
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Codex review: found issues before merge. Reviewed August 17, 2026, 7:54 PM ET / 23:54 UTC. ClawSweeper reviewWhat this changesThis PR adds Merge readinessThe implementation is a coherent, well-proven usage-only TOON formatter, but it adds a maintained public CLI contract and needs a maintainer product decision before merge. Priority: P3 Review scores
Verification
How this fits togetherCodexBar’s CLI collects provider usage payloads and renders them for terminal and agent consumers. The PR adds TOON only at the final usage-output rendering step, leaving provider fetching and other commands unchanged. flowchart LR
A[CLI arguments] --> B[Usage command]
B --> C[Provider usage payloads]
C --> D[Output preference]
D --> E[JSON renderer]
D --> F[TOON renderer]
E --> G[Terminal and agents]
F --> G
Decision needed
Why: The patch is bounded and proof-backed, but accepting a new documented machine-readable format commits the project to compatibility expectations beyond an internal display change. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: If the public contract is approved, land the narrowly scoped usage-only formatter and clarify that only commands which already expose Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds a new optional output mode rather than fixing a reported failing behavior; its body supplies after-fix CLI transcripts and TOON-decoder results. Is this the best way to solve the issue? Unclear pending product approval: the usage-only implementation is the narrowest technical path, but maintainers must choose whether to support this additional public output contract. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f7723d33a169. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (11 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1393312e7b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
022b116 to
a7a0df1
Compare
a7a0df1 to
c20c3a4
Compare
Resolves #2996
Summary
Adds
codexbar usage --format toon, which emits the same data as--format json, encoded as TOON v4.1—a denser, human-readable format intended for agents that would otherwise parse JSON.Adds
ToonFormatter, a genericEncodable→ TOON encoder rather than a hand-mapped schema. It round-trips values through a customEncoderconformance into an order-preserving node tree instead of usingJSONSerialization, which does not preserve key order, then serializes that tree according to the TOON spec.rows[N]{label,value}:.Reuses the existing
--format jsonfetch/render pipeline for--format toon, including the same credits, color, and account behavior. Only the final output step changes.Does not add
.toonto the sharedOutputFormatenum. That enum is used by several other commands with exhaustive format switches (hooks,config,cost,cache, andcookie), and adding full TOON support across those commands is outside the scope of this change.usage --format toonis the only command wired up, and TOON recognition is scoped to that command in the resolver as well, so no other command's--formathandling changes. This intentionally keeps the implementation to the MVP described in Feature request: --format toon for agent-compatible structured output #2996: a presentation-only formatter over the existing JSON schema, with no new data or providers.Addressed review feedback
TOON recognition is restricted to the
usagecommand.tooninside the sharedCLIOutputPreferences.from(values:), which is also called bycost,cache,config,hooks,diagnose,guard,plugins, andcookie. Those commands advertise onlytext | json, so they silently accepted--format toonand emitted JSON where they previously emitted text.from(values:)and bothresolveOutputFormatoverloads now take anallowsToonparameter that defaults tofalse.resolveUsageOutputPreferencesis the only site that passestrue.commandSupportsToon(argv:), which mirrorseffectiveArgv: a leadingusage, or a barecodexbar --format toonthat resolves to the implicitusagecommand.toonis an unrecognized--formatvalue that falls through to text, or to JSON only when--json/--json-onlyis also passed—identical to how--format xmlhas always behaved.docs/cli.mdnow states this explicitly:usage --format toonis the only command that supports it, and every other command accepts only--format text|json.Early failures now render as TOON, both before and after argument parsing.
CLIOutputPreferencescarries atoonRequestedflag.runUsage—includingSelf.exit,printError, andloadConfig—respect it.CLIOutputPreferences.from(argv:)provides an argv-level bootstrap scanner for failures that happen beforeParsedValuesexists, such asProgram.resolverejecting an unknown option.--format toonand--format=toonare recognized, so early exits remain consistent with the requested output format rather than falling back to JSON or plain stderr text.--helpadvertises TOON.CLIHelp.usageHelpandCLIHelp.rootHelp—the hand-written help strings thatCLIEntryactually prints ahead of Commander's descriptor—now advertisetext|json|toonand include a--format toonexample.codexbar costand the other commands keep advertising--format text|json, which now matches their behavior exactly.Tabular collapse no longer fabricates fields.
encodeIfPresentfor optional fields, so an absent value means the key is omitted rather than encoded asnull.nullvalues are introduced.Non-finite doubles are rejected.
Commander does not reject
--format toonduring parsing.CommandParser.parsedoes not validate an option value against its declared type. It only rejects unknown option names and missing values.@Optionproperty'sExpressibleFromArgumentconstraint is compile-time-only, andCommanderError.invalidValuehas no call sites in the package.usage --format toonparses and renders TOON successfully.--format toonat parse time does not reproduce, and no parser change is required.Tests
ToonFormatterTestsnullis fabricated and no{...}table header is emittedProviderPayloadencodingCLIOutputTestsrenderProviderPayloadsselects TOON vs. JSON based ontoonRequested--format toonand--format=toon--jsonoverrides an earlier TOON requestParsedValuesresolves to TOON throughresolveUsageOutputPreferencesand to plain text through the sharedfrom(values:)decodeFormatreturns.textfor--format toon, and.jsononly when--jsonis present—identical to its handling of an unrelated unsupported value such asxmlcost,cache,config,hooks,diagnose,guard, andserveasserts no TOON request and no format change, while the implicit-usage form (--format toonwith no command) still resolves to TOONCLIEntryTestsProgram.resolveparse failure--format toonand--format=toonare verified to emit the TOON error payload on stdoutcost,diagnose, andcacheinvoked with--format toonand an unrecognized option must keep stdout empty and report the parse failure as text on stderrEach of the new command-boundary tests was verified to be discriminating: with the scoping fix temporarily reverted, all three fail; with it restored, all three pass.
CLIProviderSelectionTestsusageHelpandrootHelpadvertise--format text|json|toonVerification
swift build— cleanmake check— clean, 0 lint violationsFocused CLI suites (
CLIOutputTests,CLIEntryTests) — passing, including all new command-boundary regressionsFull
make test— every suite passes except two pre-existingAdaptiveRefreshTimerTeststiming failures (AdaptiveRefreshTimerTests.swift:39and:78, both 30sCancellationErrorwhile waiting onstore.adaptiveRefreshScheduledAt). Those reproduce identically with this PR's changes stashed out; they live in the app'sUsageStorerefresh-timer path and come from unrelated commits on this branch.Validated real CLI behavior with:
codexbar usage --bogus-flag-xyz --format tooncodexbar usage --bogus-flag-xyz --format=toonDecoded real output with the official reference decoder,
@toon-format/toonv4.1.1, confirming interoperability with the published TOON spec.Real CLI behavior
Interoperability with a real TOON consumer
Decoded real output using the official reference decoder,
@toon-format/toonv4.1.1, matching the spec version targeted by this encoder and independent of CodexBar's own tests:--format jsonoutput for the same invocation, byte-for-byte after accounting for key order.rows[2]{id,label}:decodes to{"rows":[{"id":1,"label":"Ada"},{"id":2,"label":"Bob"}]}.rows[2]:list form decodes to{"rows":[{"id":1,"note":"hi"},{"id":2}]}. Row 2 has nonotekey, exactly matching the JSON produced byencodeIfPresent.Example