fix(cli): keep report --json stdout valid when --html/--csv are also set - #33
Merged
Merged
Conversation
…o set
`--json` is documented as the machine-readable output "for pipelines" and
prints the report to stdout. But when --html and/or --csv were passed too, the
"✓ dashboard: <path>" / "✓ finance statement: <path>" confirmations were
printed to the SAME stdout, appended after the JSON document — so the natural
combination `flightdeck report --json --html dash.html > data.json` (get the
machine JSON AND persist the dashboard) wrote a file that no longer parses as
JSON ("Extra data").
Route the two write confirmations to stderr when --json is set (the module
already has `err = Console(stderr=True)`), so stdout carries only the JSON
document. Without --json they stay on stdout next to the human-readable report,
so the non-pipeline output is unchanged.
The regression test runs the real CLI in a subprocess: the in-process
CliRunner does not separate rich's stderr console, so only genuine OS streams
demonstrate the fix (stdout parses as JSON; the confirmation lands on stderr).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Adversarial gate: PASS. An independent opus reviewer attacked from five angles (non-json regression, incomplete fix, subprocess-test soundness, the rich-stderr fd assumption, and exit-code/encoding) and could not refute it (high confidence).
Full local gate green: 204 passed, coverage 93.17%, ruff clean, demo + |
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.
Bug
--jsonis documented as machine output "for pipelines," but combining it with--html/--csvcorrupts the JSON. The✓ dashboard: <path>/✓ finance statement: <path>confirmations were printed to the same stdout, after the JSON document, so:wrote a file that fails to parse (
json.JSONDecodeError: Extra data).Root cause
In the
reportcommand the html/csv success messages went unconditionally to the stdoutconsole, the same consoleconsole.print_jsonwrites to — with no guard foras_json.Fix
Route the two confirmations to
err(stderr, already defined in the module) when--jsonis set, so stdout carries only the JSON. Without--jsonthey stay on stdout next to the human-readable report — non-pipeline output is unchanged. The files are still written either way.Test
Adds
test_report_json_with_html_keeps_stdout_valid_json. It runs the real CLI in a subprocess because the in-processCliRunnerdoes not separate rich's stderr console — only genuine OS streams show the fix. Asserts stdout parses as JSON, the confirmation lands on stderr, and both files were written. Verified it fails onmain(Extra data) and passes on the branch.Full local gate green: 204 passed, coverage 93.17% (≥85%),
ruffclean, offline demo +audit verifyintact.🤖 Found and fixed by an autonomous
bughuntiteration.