A non-numeric pattern stops at clap's parse error and falls back to raw
grep, so rtk and grep agree byte-for-byte whether or not `-l` is bound to
--max-len: the three existing cases still pass with the bug reintroduced.
Only a numeric pattern reaches the wrong answer -- rtk printed nothing and
exited 1 where grep listed the file.
Drop the clap-level test and the comment sentence with it. Develop's
test_grep_parse_files_with_matches_l already fails under the same mutation,
and -L never had a short binding on --max-len, so src/main.rs is untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Address review:
- benchmark.sh: with trailing_var_arg on extra_args, --max-len after the
positionals was swallowed and forwarded raw to grep (unknown option ->
bench FAIL). Move it before -rn/pattern so rtk parses it; verified 320KB
-> 14.8KB (~95%) and that --max-len 40 vs 200 changes output size.
- grep_faithful_format_test: the inline cmp closure duplicated the existing
assert_eq_grep helper (rtk_grep + grep_plain, same stdout/exit asserts);
drop it in.
`-l` was bound to RTK's `--max-len`, colliding with GNU grep where `-l`
means `--files-with-matches`. Running `grep -l <pattern>` made clap read
the pattern as a usize and error (`invalid value '<pattern>' for
'--max-len'`), dropping the command to a raw-grep fallback at 0% savings.
Drop the short binding. `-l`/`-L` now flow to `extra_args`, where the
existing `has_format_flag` already routes them to the grep passthrough
(GNU semantics, minimal output). `--max-len` keeps its long form and its
default of 80; nothing internal used the short.
- src/main.rs: remove `short = 'l'` from `Grep::max_len` + clap-parse
regression test.
- tests/grep_faithful_format_test.rs: end-to-end test asserting
`rtk grep -l/-L` is byte-identical to `grep -l/-L`, covering `-l`
leading and trailing.
- scripts/benchmark.sh: the max-len bench used the old `-l 40` alias ->
`--max-len 40`.
- docs/usage/FEATURES.md: drop the `-l` short from the grep options table.
The unit tests exercise `is_native_test_expression` directly, so inverting or
dropping the branch that calls it left the suite green while `rtk test -d dir`
went back to `sh: 0: Illegal option -d`.
Run the binary instead: native expressions carry the system `test` exit code,
`!` is neither answered backwards nor allowed to swallow a command, argument
boundaries survive, and a command still reaches the test runner.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#3788 rewrote the piped-stream half of diff_cmd.rs (condense_stdin,
condense_unified_diff_strict) while this branch rewrote the file-versus-file
half (Hunk, compute_diff, the renderers). The two conflicts are positional:
both sides add code at the same offset. Resolved as a union -- develop's
imports plus this branch's, and both blocks kept whole.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sqlfluff exits 1 whenever it finds violations, which is the normal case this
filter exists for. Two changes in the last push read that exit code as failure
and so disabled the filter exactly when it had something to compress:
- `sqlfluff_cmd` used `early_exit_on_failure()`, which dumps the injected JSON
raw. On a 51-file corpus that is 9.8 KB where plain `sqlfluff lint` is 1.1 KB.
- `lint_cmd` substituted `result.stderr`, which is always empty: sqlfluff writes
even fatal errors to stdout (verified against 2.3.5 and 4.3.0 - `Error:
Unknown dialect 'NOPE'` on stdout, stderr empty, exit 2). Every violation was
discarded in favour of a 25-byte `SQLFluff: failed (exit 1)`.
Both are one bug reached two ways, because both entry points restated the same
routing and format-flag logic and were then edited differently. Extract it into
`sqlfluff_cmd::plan`, which states once how rtk invokes sqlfluff and how it
reads the result back; `lint_cmd` now calls it instead of keeping a copy. The
exit code only disambiguates output the filter could not parse, so a fatal error
reaches the user verbatim while violations are still summarized.
Also in the report:
- Rank the `Violations:` section worst-file-first like every section above it.
Iterating sqlfluff's emission order and taking the first 50 dropped exactly
the file the summary ranked first.
- Build only the lines that get printed, instead of every line to keep fifty.
- Omit positions sqlfluff did not report rather than fabricating `:0:0`.
- Keep two path segments when no dbt root matches, so `migrations/orders.sql`
and `reports/orders.sql` stay distinct in the section meant to be opened.
- Treat a missing `fixes` key as unknown rather than zero. sqlfluff 2.x omits it
entirely, which silently suppressed the `sqlfluff fix` hint.
- Rename the savings test to the bound it actually asserts.
Tests cover both regressions against real captured output: 4.3.0 for the current
field names and fix data, 2.3.5 for the legacy names and the absent `fixes` key.
`\ No newline at end of file` describes the line directly above it. The
parser never renders context lines, so a marker git attached to an
unchanged last line landed under the last `-`/`+` line rendered and
claimed THAT line lacked a newline -- the one place the output stated
something untrue about the diff. The marker is now kept exactly when the
line above it was emitted (in-hunk after a `-`/`+`, or on the line right
after the budget closed on one), which keeps the witness of a
newline-only change and drops a marker that would describe the wrong
line. `emitted_at` replaces `closed_at`: the position test is the same
for both rules. Real fixture `git_diff_no_newline_raw.txt` carries both
shapes.
`# HG changeset patch` opens a message region like an mbox `From`, so
`hg export`'s column-0 headers and message no longer latch
`dropped_prologue` and send the whole changeset raw through its own
`diff -r <a> -r <b> <file>` echo. Real fixture `hg_export_raw.txt`
(Mercurial 7.0.1) joins the corpus.
`rename to`/`copy to` (and their `from`) go through `dequote`, so a
`core.quotepath` name has one spelling across a stream. The submodule
note keeps the range's direction qualifier:
`[file] sub (submodule e139196..b0ac9b1 rewind)`.
The body-line replay in `corpus_every_marked_body_line_survives` shares
the parser's per-parent presence rule (there is no second formula for
combined-diff columns); its doc now says so and names the oracle -- git's
header counts -- and it asserts a budget is never still owed at a
non-body line or at EOF, so under-consumption fails there too.
develop now carries the byte-equality verdict (0cb34ac, 428af61), so this
branch no longer argues for it. The alignment fix and the reporting that
depends on it remain.
compute_diff compared line N to line N, so a single inserted or deleted
line reported every line after it as modified. This aligns by Myers' greedy
edit script, so one insertion reads as one insertion.
The script folds into hunks, one per run of differing lines with no
matched line between. Both renderers group by hunk: the classic fallback
prints one NcM per region, as diff does, because the classic format cannot
express which old line became which new line, and rendering the pairing
split a region into an NcM plus a trailing NaM that diff never prints.
Inside a hunk the condensed render pairs old and new lines by best
similarity rather than by position, so an insertion at the head of a run
no longer shifts every pairing after it into a rewrite that did not happen.
Op::Keep carries a run length. One entry per matched line made the script
scale with the file rather than with the change, 45MB of separators for a
million-line pair with 700 rewrites. The aligner's trace is one flat vector
with each round's bookkeeping stored inline, so MAX_TRACE_CELLS charges
what is allocated: a per-round Vec cost 5.8x the cells being counted on a
lopsided pair.
The bool develop passes into render_diff cannot express the third case the
aligner produces, a comparison that ran and then refused to build a listing
because it would cost more than the raw text. FileComparison replaces the
bool with Identical, InvisibleDifference and Lines, and DiffResult::unaligned
carries the refusal reason. An empty change list is not a synonym for
identical, and routing a refusal through the identical branch exits 0 on
files that differ.
The invisible-difference message keeps develop's wording, so the phrase
diff_byte_accuracy_test matches on survives, and then names the measured
cause instead of stopping at "no line-content change". It is shown whenever
the case arises. A fixed token allowance above raw could not be met: the
message's shortest form is ~20 tokens and a one-line pair ~2, so the ceiling
sat under the message's floor and dropped it on 90% of one-line pairs,
printing the two indistinguishable blobs it exists to replace.
The render drops the file-pair header and the blank line, names the files
in the frame legend by argument position instead of by path, and prints
the counts line only once the listing runs to a screenful. On 120
agent-sized diffs the framing cost more than the change list saved: the
body alone is 7% smaller than diff, and each line of chrome erased that.
GNU diff translates `Only in` and `Binary files … differ` (git translates
none of its own lines). Rule 6 read the English spelling only, so on a
French machine `diff -ru g1 g2 | rtk diff -` dropped the three translated
fact lines as prose and, because a text section still parsed, returned
`Some` -- four files became one, silently.
Rule 3b now reads GNU diff's per-file `diff <opts> X Y` echo (printed
whenever it compares directories) as the mark of a `diff -r` stream. Such
a stream carries no prose, so a column-0 line no arm read there is a fact
in a language the parser does not speak, and rule 9 returns `None`
instead of dropping it. A line dropped before the echo settled the
question counts the same way once it does -- `diff -r` may list an
`Only in` first. Real fixture `diff_ru_fr_raw.txt` (`LC_ALL=fr_FR.UTF-8`,
diffutils 3.10) pins the raw fallback; the English `diff_ru_raw.txt`
still parses in full, as do `git log -p` streams whose prologue is prose.
The `Submodule <path> <a>..<b>` note now carries the range --
`[file] sub (submodule e139196..b0ac9b1)` -- for parity with the dirty
arm's `(submodule, modified content)`.
Failure detection, so a green run is never reported red:
- a failure marker vouches for the block it closes, so a frameless failure
such as a timeout keeps the line that says why it failed
- bun's marker and run footer are matched with the duration and test count
they carry, since a test is free to log a line that starts the same way
- deno's FAILURES entries are matched by shape, because a bare " => " also
occurs in arrow functions and assertion messages
- deno fences output it did not write, and names the fence for when it was
printed, so the rule is matched by shape and gates both the failures list
and the block engine
Diagnostics that were reaching nobody:
- deno's type-check errors are kept with the frame under them: they open no
block and carry no section, so a type error left the caller with a bare
"Type checking failed."
- bun's frames are kept, since two failing tests that share a name, and a
module error that has no marker at all, cannot otherwise be located
Runs whose output rtk must not touch:
- watch mode on every filtered entry point goes through unfiltered, since the
runners buffer the whole stream until a child that never exits
- a deno reporter the user named is left alone, because --reporter=junit and
--junit-path write the report to stdout
- bun build and deno compile run unfiltered: without an output flag the bundle
is stdout, with one the emitted-file summary is the point of the run, and on
failure the diagnostics are, none of which an errors-only filter keeps
Tool resolution:
- a package runner the user named wins over lockfile detection, and over a
tool that merely happens to be on PATH
- lockfile detection never resolves through bunx, which always fetches a
missing tool and cannot be told not to
- a tool rtk may fetch resolves through npx, the only runner that can
Input and output:
- bunx forwards the caller's stdin when it is a pipe, since the filtered path
buffers output and an inherited terminal would let a prompt wait invisibly
- strip the resolution lines bun actually prints when piped, count the tree
levels drawn with a leading pipe, and stop injecting --json into a command
the user typed
- the lint label records the arguments once, and the shell runners' placeholder
tool name stays out of their verbose line
Fixtures are real bun 1.2.20 and deno 2.9.6 captures for each case.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Treat pre-existing extensions without ownership state as uncertain, cover project-scope aliases, clean canonical sidecars after symlink removal, and classify protected extension overwrites as breaking.
BREAKING CHANGE: non-interactive installs of modified or unrelated Pi/OMP extensions now require --auto-patch to approve overwrites.
- core/runner.rs: bun and deno share a buffered engine and differ only in an
open, prove, close policy table, so a guard cannot land on one runtime alone
- core/runner.rs: deno gains the proof rule it never had, so a passing test
that logs its own error line no longer reports as a failed suite
- core/runner.rs: a block closes when the next one opens, so consecutive
stack-free failures stay separate
- tests: every scenario is asserted on both runtimes from real fixtures
- core/runner.rs: bun error blocks are kept only with proof bun wrote them
- core/runner.rs: emit diagnostics when no failure marker was printed
- core/runner.rs: deno section boundaries close an open failure block
- tests: real bun 1.3.6 and deno 2.9.6 fixtures for all three cases
- deno 2.x prints FAILURES section and 'FAILED | N passed' footer,
old anchors (test result:, failures:) never fired
- capture ERRORS diagnostics, strip ANSI before anchoring
- TestEcosystem enum picked once at boundary, match replaces
co-firable substring flags (cargo test contains go test)
- real deno 2.9.1 fixtures with golden and savings tests
- bun 1.x ignores --json for pm ls, JSON path never fired
- bun_cmd.rs: tree parser for glyph rows, sorted and deduped
- parser choice sniffs printed content, not the flag passed
- real bun 1.3.6 fixture replaces synthetic-JSON-only coverage
- runner.rs: anchor bun count lines, stop echoed source leak
- capture error and Expected/Received detail per failure
- add real bun 1.3.6 fixtures with golden and savings tests
Review round 3 on the file-level-fact arms, plus a cold pass on top:
- `Only in <dir>: <file>` splits on the FIRST `: ` (GNU diff's separator
follows the directory; a filename may carry its own).
- Standalone `Binary files X and Y differ` splits at the ` and ` where
both sides agree past their first component, so a filename containing
` and ` is not cut, and is named through `header_name` so the `a/`/`b/`
strip follows the same decision as a header pair.
- `* Unmerged path X` folds into the section git emits for X right after
it: one `[file] X (unmerged) (+A -B)` entry instead of two.
- `Submodule <path> <a>..<b>` keeps only the path in the name slot (the
tokens before the LAST `..` token, so a path containing `..` survives),
and the sha-less `Submodule <path> contains ... content` dirty line is a
fact too -- it used to be dropped silently beside a condensed sibling.
Real fixture `git_diff_submodule_dirty_raw.txt` joins the corpus.
- `rename to` / `copy to` is git's exact path; the `+++` pair no longer
re-derives it, which stripped a real `b/` directory under `--no-prefix`.
- `core.quotepath` wraps non-ASCII paths in quotes on every header line;
names are dequoted before the prefix strip.
- The header-pair hunk gate applies to open hunkless sections as well:
git only emits `---`/`+++` before the first `@@`, so a stray pair there
is lost content and falls back raw instead of renaming the section.
- `diff --cc X` carries one path and leaves `prefixed` to its header pair.
- `past_diffstat` comment states the first-section-mangled shape it alone
catches; the ` name | 3` bound says it fires always (the `---` separator
is the marked line that follows); the stdin doc says the never-worse
check is inlined and why.
A/B against 9106d48: 400 `git show` streams byte-identical; 290
format-patch series identical except 12 that fell back raw through a
prose pipe and now condense, zero new fallbacks.
Structure is read through a per-line ANSI/CR-stripped view and content is
pushed raw, so escapes that are part of the user's content survive (the
file path was already byte-faithful on the same input). Rule 8's mbox
tolerance now ends at the first diffstat line: an orphaned hunk body in a
hand-edited series falls back raw instead of being dropped as prose, while
version notes between `---` and the diffstat stay prose. `diff --git X Y`
with X == Y marks a --no-prefix stream whose header names keep their
leading a/ b/ (creations and deletions included); the check is confined
to the two-path form. The new-side `\ No newline` marker is kept only on
the line right after its hunk closes. The dead `!in_prologue` conjunct
and the variable behind it are gone.
Real `git diff --cached --no-prefix` fixture added to the corpus.
rtk diff compared files via str::lines(), which strips a trailing \r and
drops a trailing newline. CRLF-vs-LF and trailing-newline-only differences
therefore produced an empty line-diff and were reported as
"[ok] Files are identical" with exit 0 — a false-equal result that defeats
rtk diff used to verify two files are equal.
Guard the identical branch with a byte comparison: only claim identical when
the bytes truly match; otherwise surface the whitespace/line-ending
difference and exit 1 per the diff convention.
Fixes#3469
Co-authored-by: Luca "Hydr0gen" Vitale <78725363+Hydr0gen19@users.noreply.github.com>
Condense the Grep variant and its tests to the comment rules in
CONTRIBUTING.md (explain why, not what), and carry the -l/-t change
through the files that still assumed the old behavior.
- scripts/test-aristote.sh: add -r to the two grep assertions, since
`grep PATTERN <dir>` exits 2, and move the type filter to `rtk rg`,
the engine where -t is valid.
- docs/usage/FEATURES.md: extra args go to the engine actually invoked,
not always to rg; drop the per-row shortcut justifications.
- tests/search_compress_test.rs: guard grep-engine tests on
grep_available() instead of rg_available(), which skipped them into a
false green on a box without ripgrep.
- .claude/hooks/rtk-suggest.sh: reverted; the rg -> rtk rg fix is
correct but belongs in its own PR.
BREAKING CHANGE: `rtk grep --file-type` and its `-t` short are removed.
The option never reached the engine, so it was a silent no-op; `-t` now
flows through, which means `rtk rg -t rust` filters by type while
`rtk grep -t rust` returns grep's own `invalid option -- 't'`. `-l` and
`-m` under `rtk grep` are likewise the native grep flags now rather than
rtk's --max-len and --max.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- '\ No newline at end of file' survives to the output (in-hunk and the
common new-side position just after the budget closes): it is the only
witness of a trailing-newline-only change, which otherwise renders as
two byte-identical -/+ lines. Real fixture; proven load-bearing by
revert.
- Non-UTF-8 stdin now always takes the raw-bytes branch, even when the
stream parses: condensing would rewrite content bytes to U+FFFD, and
byte fidelity outranks savings. (Base hard-errored here; raw
passthrough is strictly better on both counts.)
- Doc-comment bounds recorded for the mbox header-pair spoof and the
raw-fallback newline append.
- File-level facts outside hunks become note-only entries instead of
vanishing while the stream condenses: 'Only in <dir>: <file>' and
standalone 'Binary files ... differ' (GNU diff -r), '* Unmerged path'
(git diff --ours during a merge), and 'Submodule <a>..<b>' headers.
Suppressed inside mbox message regions, where column-0 prose is
indistinguishable by value. Real fixtures captured for all four shapes;
fix proven load-bearing by revert.
- is_mbox_from accepts 64-hex separators: SHA-256 repos' format-patch
streams no longer fall back raw wholesale.
- .context() on the stdin read and raw-fallback write paths.
- Savings comment corrected to the measured corpus numbers (52-87%
metadata-heavy, single digits content-heavy); exemption escalation
unchanged.
- new file mode / deleted file mode sections (an empty file added or
removed) get their own notes; previously they vanished silently whenever
another file in the stream parsed cleanly. Real fixture captured, fix
proven load-bearing by revert.
- Signature tolerance (rule 6) now requires the stream to have carried an
mbox From separator; a bare '--' in a plain diff falls back raw.
- never_worse discrimination by token estimate instead of pointer
identity; stdin decoded once (Cow) instead of twice; hunk-body prefix
checked on the byte view without a per-line allocation.
- Structural fallback now emits the caller's exact bytes (a lossy UTF-8
decode or ANSI-stripped 'raw' is not raw); the decode-strip-parse-guard
pipeline moved into condense_stdin so it is unit-testable.
- Dropped the mailer-padding tolerance: a line shorter than the hunk's
prefix width now falls back instead of being guessed into context.
- Plain-diff fixtures recaptured with relative paths (no local usernames
or session ids); copy-only and mode-only sections get real fixtures and
a vanish-class test.
Parse the stream into (prologue)(file-header)(hunk)* regions before
classifying: hunk extent comes from the @@ line budget (one old-budget per
parent, so @@@ combined headers parse too), content is classified only
inside hunks, and any structural disagreement (invalid body prefix, budget
over/under-run, budget owed at EOF, malformed @@) falls back to raw
passthrough. Detector precedence is a documented total order on
condense_unified_diff_strict.
Fixes, each with its reproducer from
claudedocs/diff-classifier-review-2026-08-29.md as a test:
- SQL/Lua '-- comment' removals (--- on the wire) no longer dropped
- added '++ text' lines no longer parsed as file headers
- format-patch mbox prose and the '-- ' signature no longer counted
- +++ /dev/null deletions named by their --- side
- binary / rename-only / mode-only sections reported instead of vanishing
- --color input stripped (was: silently empty output), non-UTF-8 stdin
read lossily (was: hard error, no fallback)
- CRLF content bytes survive verbatim (split('\n'), not lines())
- b/ prefix stripped exactly once; timestamps tab-split from header names
- the '... +N more' trailer removed: it claimed truncation that never
happened (all lines were printed above it)
Fixture corpus: 13 real-producer captures (git 2.54, GNU diff) in
tests/fixtures/diff -- multi-file, -U0, -W, rename/delete/binary,
log -p, show --cc, format-patch single/series/cover, diff -u/-ru/-rN,
CRLF. Property tests: every marked body line survives to the output,
counters equal rendered lines, the fallback fires on zero corpus fixtures.
I deleted this test earlier on the reasoning that `-l` printed the same
thing with and without the fix. That reasoning was wrong, and so was the
mutant I used to "prove" it -- a word pattern.
The behavior splits on whether the PATTERN parses as a usize:
- Non-numeric ("TODO"): clap rejects, run_fallback re-runs raw grep, output
and exit code are identical either way. This is the case I tested, which
is why the mutant survived.
- Numeric: clap ACCEPTS. `-l 8080 hit.txt miss.txt` sets max_len=8080 and
leaves hit.txt as the pattern, miss.txt as the only path. No error, no
fallback -- just an empty result and exit 1 on a file that does match.
numeric_pattern_with_files_with_matches_flag pins it. Mutation-proved by
restoring `short = 'l'`:
`grep -l <number>` must find the match (#2628); status=Some(1) stderr=
test result: FAILED. 0 passed; 1 failed
Single-file form is worse still -- the filename becomes the pattern, no
path is left, and rtk falls back to stdin:
# mutant, one file
$ rtk grep -l 8080 one.txt < /dev/null -> exit=1, no output
$ rtk grep -l 8080 one.txt -> hangs on a terminal
# fixed
$ rtk grep -l 8080 one.txt -> one.txt, exit=0
Corrects the field comment, which repeated the wrong story ("could not
parse at all", "losing all compaction"). Nothing is lost to compaction:
`-l` is a has_format_flag passthrough on the fixed path too, so there are
no savings for this flag either way. The cost was a wrong answer.
Also repoints the last stale module reference. test_grep_version_routes_to
_extra_args said `grep_cmd::run forwards it to rg --version`; grep_cmd.rs
does not exist and the Grep arm routes to search::run, whose
--version/--help check execs against the engine unfiltered. `grep -rn
grep_cmd src/ tests/ docs/` is now empty.
1. The --file-type removal had ZERO coverage. Restoring the option long-only
(no `-t`) left the entire suite green, so nothing stopped it creeping
back. Added test_grep_file_type_option_is_gone, which pins that the flag
routes to extra_args. Note it is NOT a clap error -- unknown long flags
fall through to the trailing slot -- so an is_err() assertion would be
wrong; grep is what rejects it. Mutation-proved:
# --file-type restored long-only (+ file_type: _ so it compiles)
assertion `left == right` failed
left: ["FOO", "src/"]
right: ["--file-type", "rust", "FOO", "src/"]
test result: FAILED. 0 passed; 1 failed
2. max_len_long_option_still_binds_to_rtk guarded on rg_available(), but
`rtk grep` execs grep, never rg (search.rs: `Engine::Grep => "grep"`).
On a box without ripgrep the test skipped and reported success, hiding
the mutant it exists to catch. Added grep_available() and switched to it.
Proved with the call site discarding max_len and rg off PATH:
$ PATH=/usr/bin:/bin search_compress_test max_len_long_option
--max-len must still bind after the short form was removed
(narrow=83, default=83)
test result: FAILED. 0 passed; 1 failed
Before this change that invocation printed `ok`.
3. test_grep_options_after_pattern_stay_in_extra_args claimed to
demonstrate a `trailing_var_arg` gotcha. Removing trailing_var_arg leaves
it green -- `allow_hyphen_values` is what produces the behavior. Comment
corrected so it is not read as a guard on trailing_var_arg.
Also drops test_grep_parse_max_count_m_forwarded: it duplicates the
pre-existing test_try_parse_grep_dash_m_is_max_count (src/main.rs:3012),
which dies to the same mutant and asserts strictly more (it also pins
`max == 200`).
Walks back the Grep NOTE block, which overclaimed. It said this "completes"
the shadowed-short cleanup. It does not:
- clap's auto `-h` still beats grep/rg's --no-filename before the first
positional, and after it reaches search.rs's help check and execs
unfiltered. That is #2532's scope, so the comment now says so rather
than inviting the next reader to add disable_help_flag here.
- removing a clap short does not make the letter safe end-to-end.
search.rs shares one VALUE_FLAGS_SHORT table across both engines, so a
letter taking a value in rg but not grep still eats the next token in a
cluster: `rtk grep -rt FOO .` consumes FOO as -t's value no matter what
clap does. Noted as a search.rs defect, not fixed here.
Separately, .claude/hooks/rtk-suggest.sh mapped both `rg` and `grep` to
`rtk grep`, swapping engines. With --file-type gone, its suggestion for
`rg -t rust FOO` became `rtk grep -t rust FOO`, which grep rejects. Now
maps rg -> `rtk rg` and grep -> `rtk grep`.
Adds max_len_long_option_still_binds_to_rtk: the surviving long form must
still reach rtk's truncation now that `-l` is gone.
Mutation-proved. Discarding max_len at the call site (`max_len: _`, so the
mutant compiles) turns it red:
--max-len must still bind after the short form was removed
(narrow=83, default=83)
Restored, it passes (narrow=43, default=83).
Two things the test has to get right, both found by watching it fail:
- It needs a bulky match set. A small result passes through uncompressed
under the never-worse-than-plain guard, so a one-line file exercises the
passthrough and proves nothing.
- It measures matched lines only. rtk's "+N more [see remaining: ...]"
trailer is fixed-width chrome that --max-len does not govern, and it is
the longest line in either run.
No companion `-l` integration test: `-l` mis-binding never changed program
output. A clap parse error routes through run_fallback, which re-executes
the raw command, and the fixed path sends `-l` to the has_format_flag
passthrough -- so both sides print the same file list and exit 0. A test
asserting that would pass with or without the fix. The clap-layer test
test_grep_parse_files_with_matches_l is the real gate there; it fails when
the `-l` short is restored.
golangci_v2_json.txt is pretty-printed across 144 lines; golangci-lint emits its
JSON on a single line, so the fixture was a shape real output never takes. The
savings assertion counted whitespace-separated words, so the indentation was
doing the work -- against real single-line output that same helper reads 40
"words" for 4 KB and reports 30% for a filter that is doing its job. Measured
with estimate_tokens, the estimator RTK bills with, the real figure is 95.3%.
Replaced with output captured from golangci-lint in golang:1.27: a clean v2.13.2
report, a v2.13.2 report carrying 3 errcheck and 3 ineffassign findings, and the
628 bytes v1.64.8 writes to stderr when its vendored go/types cannot read Go
1.27 export data.
That last one backs a test the suite had no equivalent of: whatever the filter
is handed -- empty, whitespace, error text, truncated JSON, v1 or v2 -- it must
say something. Returning an empty string is what let `rtk golangci-lint run`
print nothing at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven limits, each reached with real ctest 4.3.4 output.
Result lines are validated against the run total, but the summary was not, and
`has_failures` trusts a summary over the parsed records. A test that forwards a
nested ctest run echoes that run's summary too, so a run killed before printing
its own reported the nested one: a SIGKILLed suite of 6 whose first test had
already failed rendered "ctest: 3/3 passed" off the forwarded 3-test summary.
CTest prints its summary once every test has finished, so a validated Start
behind a summary places that summary inside a test's output -- that identifies a
forwarded suite whatever its size, where a size bound alone only catches a
nested suite larger than the run. Two bounds cover a kill landing between the
forwarded summary and the next Start: the run total caps a genuine total, and a
summary cannot report fewer failures than the result lines already validated.
split_status_reason undid the fold's newline before a closing bracket with a
global replace of " ]", which also rewrites brackets belonging to the reason.
A real PASS_REGULAR_EXPRESSION of "zzz[0-9 ]end" rendered as Regex=[zzz[0-9]end]:
a character class silently loses the space it matches on, and the reader debugs
a pattern ctest never ran. Only the fold's own trailing " ]" is undone now.
--no-tests=error exits 8 and says "Errors while running CTest", but the empty
run rendered as a bare "ctest: no tests found" -- the one line that reads as a
benign outcome for a run that failed. The lines behind the no-tests marker are
kept, as they already are behind a failure.
-T Test is the canonical CI invocation and prints ordinary test output, yet it
bypassed the filter along with every other dashboard mode, so the most common
form saved nothing. The action decides now: Test stays filtered, every other
action still bypasses (-T Coverage opens with "Performing coverage"), and a
test model with no action to pair it with is left alone rather than guessed at.
looks_like_ctest_output required "Test project " to be the first non-empty line,
tolerating one specific preamble. `ctest -T Test` emits a repeated "Cannot find
file: .../DartConfiguration.tcl" ahead of the banner, so `ctest -T Test 2>&1 |
rtk pipe` fell through to identity. The banner is looked up within a short
leading window; a result or no-tests line must still follow it, which is what
keeps unrelated output out.
The failed list capped entries at the flat CAP_LIST while each entry renders a
header plus up to MAX_FAILURE_LINES detail lines, which is the multi-line case
core/README.md ("Deviating from a cap") reserves reduced() for. A 25-failure run
with 30 diagnostic lines apiece rendered 247 lines, 67.9% off the raw output --
above the floor but not compact. At reduced(CAP_LIST, 5) it renders 187 lines,
75.8% off, and the section tee still recovers every entry. The skipped and
raw-trailer lists keep the full cap: their entries are one line each.
Tracking bucketed ctest under "system" beside ls and grep, while discover
classifies it as a test runner; every other runner is bucketed by ecosystem.
Review round 6 of #2250.
- The run total used to validate result lines is the counter of the
first result line that carries one. ctest prints its own result line
before any output a test could forward, so that total identifies the
run under `--stop-on-failure` (scheduled > ran), with disabled tests,
under `-j`, and in a killed run whose test forwards a larger nested
suite. The summary-based reconciliation and the majority vote are
gone; `--stop-on-failure` details are back.
Real capture: tests/fixtures/ctest_stop_on_failure_raw.txt.
- Wrapped result lines are folded until their terminator instead of
giving up after eight physical lines, so a long `PASS_REGULAR_EXPRESSION`
list no longer drops the test and swallows its neighbour's block.
Real capture: tests/fixtures/ctest_long_regex_list_raw.txt.
- When the summary reports more failures than were parsed, the raw
`The following tests FAILED:` entries are rendered under
`failed (unparsed, raw):` (capped at CAP_LIST with a tee tail hint),
so a parsing gap degrades the output instead of erasing the names.
- Dashboard long flags (`--dashboard`, `--test-model`, `--test-action`,
`--script`, `--script-new-process`) bypass the filter like the short
forms.
Review response for upstream #3199 (KuSh round 2), plus preclear findings.
- Root-lane claims disarm on any fall-through line again (pre-d602a3b
behavior restored; tagged lanes keep the keyed-only rule). Fixes the
plain-mvn frame leak on the repo's own fixtures -- filter_compile output
is byte-identical to base ba7a9ce on mvn_test_fail_slice_raw.txt and
mvn_test_multifail_slice_raw.txt, pinned by fixture-backed tests. Full
base A/B: 56/56 outputs identical across all four entry points.
- Failures-summary tails are attributed per lane: dropped moves onto
SurefireLane, each lane's '+N more failures' tail flushes at that lane's
own aggregate (or at end-of-stream for truncated input), while the
reactor-wide entry budget stays shared. A module's tail can no longer be
collected under another module's header.
- is_lane_opener's [ERROR] arm narrowed to genuine compiler diagnostics
(FILE_COORD), so [tag] [ERROR] app logs no longer escape a buffered
block or get re-ordered ahead of it; they route by ownership rules.
- The [ERROR] arm sites only rewrite keep_continuation on the lane's own
keyed lines, matching every sibling arm/disarm site; interloper sweeps
now cover both [WARNING] and [ERROR] spellings on all three filters.
- insta removed (first dev-dependency, and .claude/rules/cli-testing.md
prescribes plain assert_eq! with include_str! fixtures): the four
snapshots are now expected-output fixtures under tests/fixtures/,
Cargo.lock drops insta/console/encode_unicode/similar.
- split_lane's no-trailing-space tagged blank pinned by tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review round 5 of #2250.
- Result lines are trusted only when their `N/M` total matches the run.
The run total is the final summary total, reconciled for disabled
tests (ctest counts them in `N/M` but not in the summary), or the most
frequent counter when no summary exists. A test that forwards a nested
ctest run can no longer replace a real record, truncate its own block
or inflate the totals; dedup keys on number and name so `--repeat`
retries still collapse. Framing uses the retained result lines only.
Real capture: tests/fixtures/ctest_nested_result_raw.txt.
- Dashboard client modes (`-D`, `-D<x>`, `-M`, `-T`, `-S`, `-SP`,
`--build-and-test`) bypass the filter like `-V` does, so diagnostics
such as a missing DartConfiguration.tcl are shown verbatim.
- Folded result-shaped logical lines are split back into physical lines
inside failure blocks, so each line is indented and counted against
the block cap.
Review round 4 of #2250.
- Failure blocks are bounded by validated CTest framing instead of any
line shaped like one. `Start N: name` counts only when a parsed test
has that number and name; the summary, total-time and "The following
tests FAILED:" lines count only at their last occurrence. A test that
prints ` Start 3: inner phase` or a nested summary keeps its
assertion evidence, and the outer summary stays authoritative.
Real capture: tests/fixtures/ctest_spoofed_framing_raw.txt.
- Collection and rendering of failure blocks are split. Blocks are
collected untruncated for every failed test, the section is rendered
with the existing entry and head+tail caps, and one section-level
`ctest-failed` tee holds the complete untruncated section whenever a
block or the list was cut. The per-test `ctest-failure` writes are
gone, so the hint no longer points at a colliding file (#3431) and the
`[full output:]` file contains no `... +N more lines` markers.
- The `***Failed <reason>` line stays outside the per-block cap.
Review round 2 of #2232.
- Diagnostics without a file prefix (`error TS5023:`, `error TS5058:`,
`error TS2688:`) match a third pattern, so they are counted, emitted as
blocks on the streaming path and grouped under `global (N errors)` on
the `rtk pipe --filter tsc` path. Output made only of global
diagnostics no longer reports "TypeScript compilation completed"
(#3220); the summary reads `TypeScript: N errors` when no file-scoped
diagnostic exists.
- `parse_diagnostic` returns a borrowed `Diagnostic` instead of raw
captures indexed by position.
- The exit-non-zero/no-diagnostics branch keeps the first and the last
`reduced(CAP_WARNINGS, 5)` non-empty lines instead of a plain tail: tsc
and npx print the cause first and boilerplate after it. Only retained
lines are ANSI-stripped, and the pipe path borrows plain input instead
of copying it.
- `--pretty` tests use a real TypeScript 6.0.3 capture and state that the
code frame after the blank line is dropped on purpose. New fixtures
cover a global config error and bare `tsc` without a project.
- summary line is authoritative for pass/fail routing and counts; a parsed
failure no longer overrides "0 tests failed" (--repeat until-pass)
- recognise retry result lines without the N/M prefix; the last result per
test number replaces the earlier one in place
- pre-result diagnostics are only claimed when the nearest boundary is a
Start line, so -j runs no longer absorb other tests' result lines
- failed entries are labeled "#N name (status, duration)" with their details
indented beneath; empty blocks can no longer shift attribution
- failed: and skipped: lists are capped at CAP_LIST with tee recovery; the
duplicated CTest FAILED name list is dropped, its trailer lines are kept
- failure detail keeps 2 head lines + 8 tail lines so pre-result diagnostics
such as "Could not find executable" survive truncation
- fixtures re-captured as stdout followed by stderr, matching runner.rs; add
real -j4 and --repeat until-pass:2 captures
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- migrate regex statics to std::sync::LazyLock (lazy_static left develop in 5269df7)
- use ..RtkRule::DEFAULT for the ctest rule (new pipeline_final_safe field)
- join wrapped `***Failed <reason>` result lines before parsing and surface the reason
- give ***Skipped tests their own bucket instead of reporting them as failures
- report "no tests found" only for the literal `No tests were found!!!` line when nothing parsed
- cap per-failure output at CAP_WARNINGS (tail) and the FAILED list at CAP_LIST, with tee hints
- add real ctest 3.25.2 fixtures with exact-output and >=60% savings tests
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nicolas Le Cam