Repository navigation
feat: clarify output formats for non-interactive mode - #1579
Conversation
📋 Review SummaryThis PR clarifies the three output formats in non-interactive mode (json, stream-json, text) by unifying JSON and TEXT modes to use the same JsonOutputAdapter internally. The changes simplify tool output handling and remove redundant streaming text output handlers for TEXT mode. Overall, the changes improve the clarity and consistency of output format behavior. 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
| usage, | ||
| stats, | ||
| summary: message, | ||
| showResult: config.getOutputFormat() === OutputFormat.TEXT, |
There was a problem hiding this comment.
If showResult depends only on --output-format it can be derived by adapter.config.getOutputFormat() so no extra option properties is needed here.
| // Emit the entire messages array as JSON (includes all main agent + subagent messages) | ||
| const json = JSON.stringify(this.messages); | ||
| process.stdout.write(`${json}\n`); | ||
| } |
There was a problem hiding this comment.
We can route JSON and Text both to JSON output (which you have already done) and skip all non-result messages depending on this.config.getOutputFormat(), so that the implementation gap between JSON and Text lays here and here only.
| outputFormat === OutputFormat.JSON || | ||
| outputFormat === OutputFormat.TEXT | ||
| ) { | ||
| adapter = new JsonOutputAdapter(config); |
There was a problem hiding this comment.
Note that there are serveral cases where behaviours depends on the existence of adapter.
Please check and ensure the existence of adapter when output format is text makes no funtional regression.
There was a problem hiding this comment.
Okay, since the change eliminates the case where the adapter doesn't exist, I've removed some redundant checks and handling of the adapter's non-existence (the original text mode).
feat: clarify output formats for non-interactive mode
TLDR
Clarify the three output formats in non-interactive mode: json (non-streaming structured output), stream-json (streaming structured output), and text (non-streaming final result only, similar to Claude Code).
Dive Deeper
Output Format Behavior
Implementation Details
showResultflagReviewer Test Plan
Run non-interactive mode with different output formats:
Verify:
Testing Matrix
Linked issues / bugs
N/A