feat(cli): migrate inspect and cluster; pin renderer claims from both sides - #84
Conversation
… sides Migrates two more commands and closes the hole that let the claim lists be wrong twice. `inspect` becomes four sections — metadata, a wildcard-path table, fingerprints, and the domain/structural findings from #61 — with the "not a verdict" caveat preserved as a note. `cluster` becomes a summary plus a nested cluster listing, and gains a note pointing at --similarity-threshold when everything lands in one cluster, which is the common first experience on source input. Then the part that matters. #83 added a test asserting every *claimed* command/format pair really renders. That caught over-claiming but not under-claiming, and the lists were under-claiming: measuring every command rather than the claimed ones shows `governance` has a real Markdown renderer (render_governance_markdown, from vajra-stats) and `compare` has both. Like `cascade` and `score` before them, they were being told they had no renderer while emitting one. So the guard is now two-sided: claimed pair => output must differ from that command's text output unclaimed pair => output must be identical, and the notice must fire Under-claiming is now as detectable as over-claiming, which is what should have been true from the start. Three commands needed bespoke fixtures to get there: `core-team` wants author_name/author_email/date rather than the subject-based shape, `compare` takes two inputs so it cannot use the single-input helper, and `batch` wants a directory. Measured coverage: markdown 11 commands, compact-ai 4. Four tripwire tests had to be repointed because the commands they watched got migrated. That is the mechanism working, not friction — each names its successor in a comment. Refs #75. Genuinely unrendered for markdown now: batch, core-team, drift, score.
|
Warning Review limit reached
Next review available in: 47 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 (2)
📝 WalkthroughWalkthroughCLI format declarations and documentation now match additional Markdown and compact-AI renderers. ChangesCLI renderer honesty
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
🧹 Nitpick comments (2)
vajra-cli/src/main.rs (1)
1394-1405: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDomain-hint heuristics lack a "guess, not fact" disclaimer.
The Structural Findings section explicitly preserves a note that its severities are "not a verdict" (lines 1414-1417), but "Domain Type Recognition" — which is inherently a pattern-match guess (is this string an SSN? an email?) — has no equivalent disclaimer. As per coding guidelines, "Label every heuristic, make every score decomposable, and never silently present an inference or guess as truth."
If the previous hand-written output had such a note, it appears to have been dropped in this migration; if it never had one, this is still worth adding for consistency with the structural-findings treatment.
🤖 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 `@vajra-cli/src/main.rs` around lines 1394 - 1405, Add a clear “guess, not fact” disclaimer to the Domain Type Recognition report generated in the domain_hints block, explicitly indicating that recognized types are heuristic pattern matches rather than verified facts. Keep the existing table and Structural Findings behavior unchanged.Source: Coding guidelines
vajra-cli/tests/format_honesty.rs (1)
186-227: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
compare_renders_both_claimed_formatsdoesn't assert the claimed formats stay silent.Every other claimed-renderer case in this file asserts
err.is_empty()(seeclaimed_renderers_produce_distinct_output), confirming a claimed format doesn't also emit the "no renderer" warning. This test discards stderr (let (rendered, _) = invoke(format)?;) for bothmarkdownandcompact-ai, so it wouldn't catchcomparesilently warning despite being claimed.✅ Proposed fix
let (text, _) = invoke("text")?; for format in ["markdown", "compact-ai"] { - let (rendered, _) = invoke(format)?; + let (rendered, err) = invoke(format)?; + assert!( + err.is_empty(), + "compare --format {format} is claimed, so must not warn: {err:?}" + ); assert_ne!( rendered, text, "compare is claimed for {format} but matches its text output" ); }🤖 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 `@vajra-cli/tests/format_honesty.rs` around lines 186 - 227, Update compare_renders_both_claimed_formats to retain stderr from each invoke call and assert it is empty for both markdown and compact-ai, matching the validation in claimed_renderers_produce_distinct_output while preserving the existing distinct-output assertions.
🤖 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 `@vajra-cli/src/main.rs`:
- Around line 1345-1426: Update the Format::Text | Format::Markdown |
Format::CompactAi branch in cmd_inspect so every rendered report variant is
passed through maybe_redact(..., cli) before print! outputs it. Apply redaction
to both report.to_markdown() and report.to_text(), preserving the existing
format selection and ensuring Domain Type Recognition values are redacted when
--redact is enabled.
---
Nitpick comments:
In `@vajra-cli/src/main.rs`:
- Around line 1394-1405: Add a clear “guess, not fact” disclaimer to the Domain
Type Recognition report generated in the domain_hints block, explicitly
indicating that recognized types are heuristic pattern matches rather than
verified facts. Keep the existing table and Structural Findings behavior
unchanged.
In `@vajra-cli/tests/format_honesty.rs`:
- Around line 186-227: Update compare_renders_both_claimed_formats to retain
stderr from each invoke call and assert it is empty for both markdown and
compact-ai, matching the validation in claimed_renderers_produce_distinct_output
while preserving the existing distinct-output assertions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 92105687-d775-42df-9a5b-48a1c1fecd52
📒 Files selected for processing (3)
docs/src/commands.mdvajra-cli/src/main.rsvajra-cli/tests/format_honesty.rs
CodeRabbit flagged inspect's new branch as omitting maybe_redact. Checking, the original omitted it too — inspect has never redacted its text output, only its JSON. So not a regression, but a real bug, and auditing every command showed it is systematic: inspect json only fingerprint neither invariants neither separation neither cluster neither batch neither --redact is documented as applying redaction patterns before output. For six commands it silently did nothing. separation is the clearest case: its operating-point table prints field *values* in the rule column, so reached stdout with --redact set. Fixed for the five migrated commands, which is a one-line change each because the renderer funnels every one through a single output site — that consolidation is the concrete payoff of the migration beyond formats. batch is not yet migrated and still does not redact; it is on the #75 list. Test pins the behaviour both ways: the fixture must actually leak without the flag, or the test would pass vacuously against output that never contained the value.
|
Good catch, and it was worse than one branch.
$ vajra separation pii.json --label-field label --base-rate 0.1 --quiet | grep -c example.com
1
$ vajra separation pii.json --label-field label --base-rate 0.1 --redact --quiet | grep -c example.com
0Fixed for all five migrated commands. It's a one-line change each because the renderer funnels every command through a single output site — that consolidation turns out to be the concrete payoff of the migration beyond formats, which I hadn't anticipated.
The test pins it both ways: it asserts the fixture actually leaks without the flag first, otherwise it would pass vacuously against output that never contained the value. |
1 participant
Refs #75. Two more migrations, plus closing the hole that let the claim lists be wrong twice.
Migrations
inspect→ four sections (metadata, wildcard-path table, fingerprints, domain/structural findings from #61), with the "not a verdict" caveat preserved as a note.cluster→ summary plus nested listing, and gains a note pointing at--similarity-thresholdwhen everything lands in one cluster — which is the common first experience on source input, so worth saying inline.The part that matters: the guard was one-sided
#83 added a test asserting every claimed pair really renders. That caught over-claiming. It did not catch under-claiming — and the lists were under-claiming.
Measuring every command rather than only the claimed ones:
governancehasrender_governance_markdown(fromvajra-stats) andcomparehas both. Likecascadeandscorebefore them, they were being told they had no renderer while emitting one. That's the third time this list has been wrong, and every time from reasoning about code instead of measuring it.Now pinned from both sides
Under-claiming is now as detectable as over-claiming. Three commands needed bespoke fixtures to get there:
core-teamwantsauthor_name/author_email/daterather than the subject-based shape,comparetakes two inputs so it can't use the single-input helper,batchwants a directory.Measured coverage
markdown: 11 commands. compact-ai: 4.
Both higher than I've been reporting, and now trustworthy because they're measured rather than asserted.
A note on the tripwires
Four tests had to be repointed because the commands they watched got migrated. That's the mechanism working rather than friction — each now names its successor in a comment, so the next migration knows where to look.
Full workspace suite: 79 test binaries, 0 failures.
cargo fmt --checkclean. 0 clippy diagnostics.Remaining
Genuinely unrendered for markdown:
batch,core-team,drift,score. Four commands, each mechanical.Summary by CodeRabbit
New Features
inspect,cluster,compare, andgovernance.compare.Bug Fixes