feat: prioritize TOON format for agent parsing with JSON fallback - #1497
feat: prioritize TOON format for agent parsing with JSON fallback#1497DyanGalih wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe CLI now supports JSON, pretty JSON, and TOON output. Commands propagate the selected format through success and failure responses. Initialization stores the selected agent output format in project configuration. ChangesOutput format support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Command
participant printJson
participant formatAgentOutput
CLI->>Command: invoke with selected format
Command->>printJson: pass response payload and format
printJson->>formatAgentOutput: serialize payload
formatAgentOutput-->>printJson: return formatted text
printJson-->>CLI: write formatted response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/index.ts (1)
305-328: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMap the new flags to
jsonandformatbefore dispatch.
--json-prettysetsoptions.jsonPretty, and--toonsetsoptions.toon. This handler passes onlyoptions.json, so both flags leave the command in text mode. The same pattern appears in the other changed command registrations and failure handlers.Add one output-format resolver. It should map the flags to
'json','json-pretty', or'toon'. Passjson: trueand the resolvedformatto command handlers. Pass the same format tofailWithError. Reject conflicting output flags or define a documented precedence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/index.ts` around lines 305 - 328, Introduce a shared output-format resolver for the changed command registrations, mapping jsonPretty, toon, and json to 'json-pretty', 'toon', or 'json' and handling conflicting flags with a clear rejection or documented precedence. Update each affected action handler to pass json: true and the resolved format to its command execution and to pass that same format to failWithError, including the handler around resolveRootForCommand and the other changed registrations.
🧹 Nitpick comments (1)
src/commands/change.ts (1)
131-131: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a typed
formatoption before callingprintJson.The CLI accepts
--json,--json-pretty, and--toon, but the focused handlers do not pass that choice into their typed option contracts, so the selected format is read only throughany. Addformat?: OutputFormatto the relevant option types and callprintJson(..., options.format ?? 'json-pretty')instead of relying on eachjson?: booleanbranch to default to'json'.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/change.ts` at line 131, Add format?: OutputFormat to the typed option contracts used by the focused handlers, then update every printJson call at src/commands/change.ts:131-131, src/commands/change.ts:189-189, src/commands/change.ts:267-267, src/commands/config.ts:241-241, src/commands/spec.ts:120-120, src/commands/spec.ts:189-189, src/commands/spec.ts:241-241, src/commands/workflow/schemas.ts:28-28, and src/commands/workflow/templates.ts:86-86 to pass options.format ?? 'json-pretty' instead of reading format through any or deriving it from json flags. Ensure the existing CLI choices --json, --json-pretty, and --toon flow through the typed format option.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/index.ts`:
- Around line 184-195: Update the output-format prompt guard around
agentOutputFormat to match InitCommand.canPromptInteractively(): only prompt
when options?.tools is undefined and isInteractive receives { interactive:
options?.interactive }, preventing prompts for --no-interactive and --tools
invocations.
In `@src/commands/validate.ts`:
- Line 219: Preserve the resolved OutputFormat by passing it to every affected
printJson call instead of boolean literals or relying on the default:
src/commands/validate.ts lines 219, 311, and 367; src/core/list.ts lines 113,
151, 174, 184, and 208; src/core/archive.ts line 271; src/commands/schema.ts
lines 372-375, 420-424, 467-470, 499-503, 519-524, 538-541, 565-568, 582-586,
605-609, 638-645, 655-658, 694-697, 713-717, 790-794, 904-910, and 925-928;
src/commands/workflow/instructions.ts lines 172, 515, and 621; and
src/commands/workflow/status.ts line 114.
In `@src/core/project-config.ts`:
- Around line 46-50: Update readProjectConfig to validate raw.agentOutputFormat
with the same z.enum(['json', 'toon']) schema and assign the parsed result to
config.agentOutputFormat, preserving optional behavior when the field is absent.
---
Outside diff comments:
In `@src/cli/index.ts`:
- Around line 305-328: Introduce a shared output-format resolver for the changed
command registrations, mapping jsonPretty, toon, and json to 'json-pretty',
'toon', or 'json' and handling conflicting flags with a clear rejection or
documented precedence. Update each affected action handler to pass json: true
and the resolved format to its command execution and to pass that same format to
failWithError, including the handler around resolveRootForCommand and the other
changed registrations.
---
Nitpick comments:
In `@src/commands/change.ts`:
- Line 131: Add format?: OutputFormat to the typed option contracts used by the
focused handlers, then update every printJson call at
src/commands/change.ts:131-131, src/commands/change.ts:189-189,
src/commands/change.ts:267-267, src/commands/config.ts:241-241,
src/commands/spec.ts:120-120, src/commands/spec.ts:189-189,
src/commands/spec.ts:241-241, src/commands/workflow/schemas.ts:28-28, and
src/commands/workflow/templates.ts:86-86 to pass options.format ?? 'json-pretty'
instead of reading format through any or deriving it from json flags. Ensure the
existing CLI choices --json, --json-pretty, and --toon flow through the typed
format option.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ce707705-a1b9-41f8-9aee-780769c0bd42
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
package.jsonsrc/cli/index.tssrc/commands/change.tssrc/commands/config.tssrc/commands/schema.tssrc/commands/shared-output.tssrc/commands/spec.tssrc/commands/validate.tssrc/commands/workflow/instructions.tssrc/commands/workflow/schemas.tssrc/commands/workflow/shared.tssrc/commands/workflow/status.tssrc/commands/workflow/templates.tssrc/core/archive.tssrc/core/config-prompts.tssrc/core/format-output.tssrc/core/init.tssrc/core/list.tssrc/core/project-config.ts
- Updated package version in package.json from 1.7.0 to 1.8.0. - Added @toon-format/toon as a dependency. - Enhanced CLI commands to support TOON format output. - Introduced printJson function to handle different output formats (JSON, pretty JSON, TOON). - Updated various command files to utilize the new printJson function for consistent output formatting. - Modified InitCommand to accept agentOutputFormat option for specifying output format. - Updated project configuration schema to include agentOutputFormat.
81e7795 to
d28f701
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/index.ts`:
- Around line 632-640: Update the action handler’s reserved `apply` and
`archive` branches to pass options augmented with `format:
resolveOutputFormat(options)` into `applyInstructionsCommand` and
`archiveInstructionsCommand`, matching the existing `instructionsCommand` branch
while preserving all other options.
- Around line 69-73: Normalize CLI options before command dispatch by using
resolveOutputFormat to detect any selected structured format and setting json to
true when a format is returned. Pass these normalized options to every command
so --json-pretty and --toon consistently enable machine-readable behavior while
preserving the selected output format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 622871f5-ff70-423b-8e61-200370b7e69f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
package.jsonsrc/cli/index.tssrc/commands/change.tssrc/commands/config.tssrc/commands/schema.tssrc/commands/shared-output.tssrc/commands/spec.tssrc/commands/validate.tssrc/commands/workflow/instructions.tssrc/commands/workflow/new-change.tssrc/commands/workflow/schemas.tssrc/commands/workflow/shared.tssrc/commands/workflow/status.tssrc/commands/workflow/templates.tssrc/core/archive.tssrc/core/config-prompts.tssrc/core/format-output.tssrc/core/init.tssrc/core/list.tssrc/core/project-config.tstest-spec-command-tmp/openspec/specs/auth/spec.mdtest-spec-command-tmp/openspec/specs/payment/spec.md
🚧 Files skipped from review as they are similar to previous changes (14)
- src/core/format-output.ts
- src/core/project-config.ts
- src/commands/workflow/shared.ts
- src/core/config-prompts.ts
- src/core/init.ts
- src/commands/shared-output.ts
- src/commands/workflow/status.ts
- src/commands/spec.ts
- src/core/archive.ts
- src/core/list.ts
- src/commands/config.ts
- package.json
- src/commands/schema.ts
- src/commands/change.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/core/completions/command-registry.test.ts (1)
211-212: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd runtime output tests.
These assertions validate only
COMMAND_REGISTRYmetadata. They can pass while commands accept the flags but emit human output. Add focused success and failure tests for the new output modes in each command family.As per coding guidelines, run the focused file with
pnpm exec vitest run test/core/completions/command-registry.test.ts.Also applies to: 253-262
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/core/completions/command-registry.test.ts` around lines 211 - 212, Add focused runtime tests in test/core/completions/command-registry.test.ts for the json-pretty and toon output modes across each affected command family, covering both successful output and failure output rather than only COMMAND_REGISTRY metadata. Use the existing command execution and assertion helpers, then run the focused test file with pnpm exec vitest run test/core/completions/command-registry.test.ts.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/context.ts`:
- Around line 173-174: The output format is not propagated beyond flag
registration, so JSON-pretty and TOON selections are ignored. In
src/commands/context.ts:173-174 and src/commands/doctor.ts:198-199, add a shared
format option type to the action signatures and pass it through success and
failure output calls; in src/commands/store.ts:711-712 and
src/commands/workset.ts:619-620, extend command option types and both output
paths to use the selected format, printing TOON when the format is toon while
preserving existing JSON behavior.
---
Nitpick comments:
In `@test/core/completions/command-registry.test.ts`:
- Around line 211-212: Add focused runtime tests in
test/core/completions/command-registry.test.ts for the json-pretty and toon
output modes across each affected command family, covering both successful
output and failure output rather than only COMMAND_REGISTRY metadata. Use the
existing command execution and assertion helpers, then run the focused test file
with pnpm exec vitest run test/core/completions/command-registry.test.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 15c68df3-271b-4a84-bc14-d9a3f2279387
📒 Files selected for processing (10)
src/commands/config.tssrc/commands/context.tssrc/commands/doctor.tssrc/commands/schema.tssrc/commands/spec.tssrc/commands/store.tssrc/commands/workset.tssrc/core/completions/command-registry.tssrc/core/completions/shared-flags.tstest/core/completions/command-registry.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/commands/config.ts
- src/commands/schema.ts
- src/commands/spec.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test-hook.mjs (1)
4-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd executable assertions to the Commander option probes.
Both scripts only log the action options. A regression can still exit successfully.
test-hook.mjs#L4-L17: Assert that the action receivesoptions.json === true.test-options.js#L3-L18: Assert that--tooncauses the action to receiveoptions.json === true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test-hook.mjs` around lines 4 - 17, The Commander option probes currently only log action options, so regressions can exit successfully; replace those observational logs with executable assertions. In test-hook.mjs lines 4-17, assert that the action callback receives options.json === true, and in test-options.js lines 3-18, assert that invoking the command with --toon causes the action to receive options.json === true.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/index.ts`:
- Line 625: Update the empty-status output path in statusCommand to serialize
its payload through printJson(payload, options.format) instead of
JSON.stringify. Preserve the existing behavior for non-empty status results
while ensuring --toon and compact JSON formatting are honored.
- Around line 76-85: Update the return type of normalizeOptions to replace the
empty-object type {} with object, preserving the existing conditional type
behavior and returned format property.
---
Nitpick comments:
In `@test-hook.mjs`:
- Around line 4-17: The Commander option probes currently only log action
options, so regressions can exit successfully; replace those observational logs
with executable assertions. In test-hook.mjs lines 4-17, assert that the action
callback receives options.json === true, and in test-options.js lines 3-18,
assert that invoking the command with --toon causes the action to receive
options.json === true.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 08fba0b3-4d26-41cc-8b10-81ac855b9f46
📒 Files selected for processing (3)
src/cli/index.tstest-hook.mjstest-options.js
|
Thanks for putting this together and for focusing on token efficiency. After reviewing the implementation, we are going to keep this PR closed because it breaks OpenSpec’s single canonical machine-readable contract and introduces unsafe retry semantics for mutating commands. Current OpenSpec deliberately has one agent protocol: This branch creates three formats across the CLI (
The PR’s advertised fallback is to retry with The second invocation now targets a change that has already moved and can report a misleading failure; the same pattern is worse for any future non-idempotent command. A serialization optimization must never require re-executing an operation to understand whether it succeeded. It also adds a new runtime dependency and format branches across most commands for speculative token savings without providing a stable, independently versioned schema or parser capability negotiation. Compact consumers can already select only the fields they need from canonical JSON, while OpenSpec retains one testable protocol. The architectural cost and ambiguity outweigh the claimed token benefit, so JSON should remain the sole machine-readable contract and this PR should not be merged. We appreciate the substantial work that went into this. If you would like to explore a narrower proposal—such as opt-in TOON output for read-only commands, with explicit format signaling and no retry of mutations—we would be happy to discuss that design in an issue before another implementation. |
2 participants
What
This PR updates the agent skill instructions (specifically
openspec-bulk-archive-change) to explicitly prioritize parsing OpenSpec's structured--toonserialized output over standard JSON, while retaining a robust--jsonfallback mechanism.Why
The primary motivation for this change is token efficiency for advanced LLM integrations.
--toonflag, we optimize token usage without sacrificing the structured data needed by the agents.--jsonparsing. If the agent encounters a parsing failure with the compact TOON format, it will automatically retry the command with--jsonand parse the standard format to guarantee workflow completion.Verification
Summary by CodeRabbit
--json-prettyand--toonoptions across supported commands.