fix(cli): stop silently ignoring unimplemented output formats; docs follow-ups - #79
Conversation
…ollow-ups Two CodeRabbit findings on #77 that I merged before reading, plus the user-visible half of #75. The format gap, measured: `--format markdown` and `--format compact-ai` produce byte-identical output to `text` for eleven of the twelve commands. Only `essence` has a real renderer. The CLI advertises four formats and delivers two. Accepting a format flag and ignoring it is the same failure as reporting `errors: []` over a partial batch — the caller cannot distinguish "rendered as Markdown" from "fell back to text". Those commands now say so on stderr, name a format that does work, stay silent under --quiet so pipelines are unaffected, and leave stdout byte-identical. This does not close the renderer gap; it stops the interface lying about it. Docs follow-ups from the #77 review: - cmd-essence.md referenced `meta.truncated` next to the JSON example I had replaced. CodeRabbit read this as obsolete guidance for a removed key. Checking the actual output, `meta` is real — it belongs to the `--format compact-ai` shape (meta, doc, notable, motifs, drill, v), not to `--format json`. So the fix is to say which format carries it rather than delete accurate documentation. - cmd-query.md's shell example now passes --format json --quiet explicitly. The example was not broken — text mode already emits a bare number — but depending on that implicitly is the kind of assumption that stops being true without warning. Tests: 5 covering the format notice (announced for both unimplemented formats and points at json, silent for essence, silent for text/json, suppressed by --quiet, stdout unchanged). Refs #75.
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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 |
…rkdown Closes the design question blocking #75's first half, and starts the migration. Commands formatted their own output per format with println!, which is exactly why `--format markdown` and `--format compact-ai` fell through to text for eleven of twelve commands: each command had to implement every format, so most implemented one. A command now builds a `render::Report` — headings, key/value fields, tables, notes, nested lists — and the renderer emits it per format. Adding a format costs one implementation rather than twelve. The design question was whether one model covers outputs ranging from flat key/value to nested detail lists. It does, with five block types, validated against the two most awkward shapes rather than the easiest: `anomalies` needs a nested list (type instabilities with their distributions) and three tables with distinct columns. `anomalies` is migrated and now emits genuine Markdown — `##` headings, real `|---|` tables, `>` blockquoted notes — rather than the text output relabelled. It also gains a summary block it did not have before, since the model made one cheap. Format tracking is now per format rather than per command, because coverage is genuinely uneven: `anomalies` renders Markdown but has no compact-AI view. A command joins RENDERS_MARKDOWN only when it actually renders Markdown, so the notice added in #79 can never claim more than is true. That makes the migration incremental and self-documenting. Renderer details worth noting: pipes are escaped and newlines flattened inside Markdown cells, so a rule like `a | b` cannot break a table; ragged rows do not panic; empty tables show their empty-state rather than an empty table; and notes survive every format, since they carry the caveats that stop a number being over-read. Tests: 10 for the renderer (both formats, markdown differs from text, pipe escaping, newline flattening, empty state, notes survive, nested lists, ragged rows, determinism) and 3 more for format tracking (per-format exactness, migrated command emits real Markdown, unmigrated stdout unchanged). Refs #75. Remaining: migrate the other ten commands, and decide whether compact-ai gets a bespoke view per command or maps to json.
The lists added in #79 were wrong in both directions, because I built them by reading match arms instead of measuring output. `cascade` hand-rolls all three formats — cascade_text, cascade_md, cascade_json — and has done since before the shared renderer existed. It was therefore being told it has no markdown renderer while emitting genuine markdown. `score` likewise hand-rolls compact-ai. Both were warning about renderers they already had, which is the same category of wrongness the notice was introduced to remove. Corrected: markdown: anomalies cascade essence fingerprint invariants separation stats compact-ai: cascade essence score Reading the code was the wrong method, so the fix is a test that measures. `claimed_renderers_produce_distinct_output` runs every claimed command/format pair and asserts the output differs from that command's text output and that no warning is emitted. Nine pairs, including the two that needed non-trivial fixtures: cascade with its four field flags, score with commit-shaped records. That guard would have caught the original error, and it now makes the claim unfalsifiable by inspection: a command cannot be added to either list without demonstrably rendering that format. Refs #75. Markdown coverage is 7 of 12, compact-ai 3 of 12 — both higher than previously reported, because two commands were already there.
Two CodeRabbit findings from #77 that I merged before reading — my mistake — plus the user-visible half of #75.
The format gap, measured
--format markdownand--format compact-aiproduce byte-identical output totextfor eleven of the twelve commands. Onlyessencehas a real renderer:The CLI advertises four formats and delivers two. Accepting the flag and ignoring it is the same failure as reporting
errors: []over a partial batch — the caller can't distinguish "rendered as Markdown" from "fell back to text".Silent under
--quietso pipelines are unaffected; stdout is byte-identical either way, verified by test. This does not close the renderer gap — it stops the interface lying about it. #75 stays open for the renderer work, now with the gap measured rather than suspected.The two review findings
cmd-essence.mdmeta.truncated— CodeRabbit read this as obsolete guidance for a key I'd removed. Checking the actual output,metais real: it belongs to the--format compact-aishape (meta,doc,notable,motifs,drill,v), not the--format jsonexample I replaced. So the fix is to say which format carries it rather than delete accurate documentation. Confirmed:{"budget_limit":20,"budget_used":20,"profile":"ai","truncated":true}.cmd-query.mdbcexample — now passes--format json --quietexplicitly. The example wasn't actually broken (text mode already emits a bare1.584962500721156), but depending on that implicitly is the sort of assumption that stops being true without warning. Taking it as a robustness fix, not a bug fix.Process note
I merged #77 before its review landed. Both findings were mine and one was ironic — prose referencing a schema key in a PR whose entire purpose was removing invented schema keys. Worth stating plainly since the whole point of that PR was that fabricated docs are worse than none.
Tests
5 covering the notice: announced for both unimplemented formats and points at a format that works, silent for
essence, silent fortext/json, suppressed by--quiet, and stdout unchanged so this is purely diagnostic.Full workspace suite: 79 test binaries, 0 failures.
cargo fmt --checkclean. 0 clippy errors.Refs #75.