Repository navigation
fix(cli): print analyze results on stdout, keep prompts on stderr - #1725
Conversation
`main.py` has one module-level `Console(stderr=True)`, and every `analyze`
command renders through it. So the primary product went to stderr while
`cli_helpers.console` (stdout) carried the init chatter — the split was
inverted for exactly the commands whose whole job is emitting data, and
`cgc analyze callers f > out.txt` captured nothing but the banner.
Add `analyze_result_console` (stdout) and route the 63 result prints in the
10 `analyze_*` commands through it. Deliberately unchanged:
- `cgc stats` / `cgc list` — already stdout via `cli_helpers`, untouched.
- argument validation errors — stay on the stderr console; they are not a
product and must not pollute a redirected document.
- empty-result notices ("No callers found") — move with the tables, since a
script reading stdout still needs to see that the query ran.
CodeGraphContext#1650/CodeGraphContext#1655 establish the opposite-facing contract (machine formats must keep
stdout pure), so this is checked against it rather than assumed compatible:
`_cli_helpers.console` and its `_file` save/restore are not touched here, and
the CliRunner suite plus `test_analyze_complexity_ci.py` pass unmodified
(21 passed). A new integration test locks all four behaviours per stream.
|
@MorningStar0709 is attempting to deploy a commit to the shashankss1205's projects Team on Vercel. A member of the Team first needs to authorize it. |
Shashankss1205
left a comment
There was a problem hiding this comment.
This is the right fix and the reasoning about what doesn't move is what sells it.
The bug is real: main.py holds a single module-level Console(stderr=True), so cgc analyze callers X > out.txt captures the init banner and drops the actual result table — the one command family whose entire job is emitting data had its streams inverted. Adding a second stdout console rather than flipping the existing one keeps the blast radius to the ten analyze_* commands.
The three-way split you drew is correct:
- Result tables → stdout. The product goes on the product stream.
- Empty-result notices → stdout. Right call, and the non-obvious one. A script reading stdout needs to distinguish "query ran, zero rows" from "nothing was written because something broke", and that signal is worthless on a stream the caller redirected away.
- Argument-validation errors → stderr. Also right.
Unknown --formatis not a result and must not land in a redirected document.
I specifically wanted the #1650/#1655 interaction checked rather than assumed, and you checked it: those PRs scope a _cli_helpers.console._file save/restore to keep stdout pure for machine formats, and this change adds a separate console object instead of touching that mechanism. Running test_analyze_complexity_ci.py and the CliRunner suite unmodified is the correct way to prove no .file materialization leak was reintroduced — that regression bit us once already in #1655.
Yes to the follow-up on the five find commands, in the same shape. Keeping it out of this PR was the right instinct.
Verified locally against the full unit + integration suite: 1596 passed, 8 skipped.
…1734) * test(cli): lock find result tables to the stdout stream Mirrors tests/integration/test_analyze_text_output_stream.py for the seven `find` subcommands. Fails today: the result headers and empty-result notices are still on the stderr console. Refs #1725 * fix(cli): print find results on stdout, keep diagnostics on stderr `analyze` got a stdout console in #1725; the seven `find` subcommands still carried their tables there. A separate console per command family keeps the blast radius inside `find`: result tables, empty-result notices and the truncation/wildcard footers move to stdout, while the `Invalid --type` argument error and the Neo4j full-text-index diagnostics stay on stderr.
Problem
src/codegraphcontext/cli/main.pyhas a single module-levelConsole(stderr=True)(line 90), and everyanalyzecommand renders its result through it. So the primary product goes to stderr whilecli_helpers.console(plainConsole(), stdout) carries the init chatter — the split is inverted for exactly the commands whose whole job is emitting data.cgc analyze callers f > out.txttherefore captures nothing useful.Change
Adds
analyze_result_console = Console()and routes the 63 result prints across the 10analyze_*commands through it.Deliberately unchanged:
cgc stats/cgc list— already stdout viacli_helpers, untouched.Unknown --format ...) — stay on the stderr console; they are not a product and must not pollute a redirected document.No callers found for 'x') — move with the tables, since a script reading stdout still needs to see that the query ran and returned nothing.Checked against #1650 / #1655 rather than assumed compatible
Those PRs establish the opposite-facing contract (stdout must stay pure for machine formats) via a scoped
_cli_helpers.console._filesave/restore. Neither is touched here — this change only adds a second console insidemain.py. Verified by running, unmodified:tests/integration/test_analyze_complexity_ci.py— 4 passed (--format jsonstdout still parses,--fail-on-violationsexit codes unchanged)tests/integration/cli/test_cli_commands.py— CliRunner suite green, so no.filevs._filematerialization leakTests
New
tests/integration/test_analyze_text_output_stream.py(4 tests) locks all four per-stream behaviours: table on stdout, empty notice on stdout, machine-format stdout purity, and validation error absent from stdout. Hermetic (per-test HOME, kuzudb, subprocess with separated streams), following the fixture shape of the #1333 test.Locally: new tests 4/4; combined regression run 21 passed;
ruff checkclean on the new file, andmain.pygoes 19 -> 19 findings (no new ones).Scope note
The 5
findcommands have the same inverted split. I kept this PR toanalyzeon purpose to keep the review surface small — happy to follow up withfindif you want it in the same shape.