Repository navigation
feat(search): fold shared path prefix in file-list passthrough - #4170
Conversation
`rtk grep -l`, `grep -L`, `rg -l`, and `rg --files` print every path in full and record 0/0 savings. Agents search from absolute roots, so each line repeats the same long directory prefix. Fold that prefix into a `<prefix> (N files)` header and print each path's tail. The transform is lossless, keeps engine order, cuts only on whole components, and runs under `never_worse`. It applies only to a bare file list: any other shape flag (`-c`, `-Z`, `--null`, `--json`, `-q`) changes the line and leaves the list verbatim. Streaming stdin passthrough is untouched. Track the fold with real sizes so it shows in `rtk gain`; unchanged passthrough keeps recording 0/0. Follow-up to rtk-ai#2630, which targeted the pre-rtk-ai#2641 grep_cmd.rs.
📊 Automated PR Analysis
SummaryAdds prefix-folding for bare file-list output from Review Checklist
Analyzed automatically by wshm · This is an automated analysis, not a human review. |
A folded file list carries its shared prefix in the header line, so `grep -rl foo src | tail -3` rewritten to `rtk grep` returned tails with no prefix. List forms (-l/-L/--files, per `is_bare_file_list`) are now left raw in producer position; other grep/rg forms rewrite as before. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`lines()` drops a `\r` before `\n`, so a name ending in `\r` folded into a path that does not exist, and an rg `--hyperlink-format` OSC 8 link, which `strip_ansi` keeps, split the header inside the escape. The fold now refuses any output containing `\r` or ESC. The fold integration tests are gated to unix like the other search tests and use `--null`, the spelling rtk itself passes to grep. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…dows `-Z` is `--decompress` on BSD grep, so use `--null`, which both greps and rtk itself use. BSD grep prints both counts and names under `-lc` where GNU grep prints names only, so compare that case against raw grep instead of a GNU count. The test drives POSIX paths and system grep, as every other search integration test does, so gate it on unix. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Prefix folding can corrupt valid paths containing embedded newlines; this losslessness issue must be addressed.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Adds shared-directory-prefix folding for bare grep/rg file-list output while preserving order and tracking savings.
Changes:
- Folds common path prefixes into compact headers.
- Adds detection, pipeline safeguards, tests, and documentation.
- Preserves verbatim output for unsupported formats and inputs.
| File | Summary |
|---|---|
tests/search_file_list_fold_test.rs |
Adds folding and passthrough integration tests. |
tests/grep_faithful_format_test.rs |
Updates file-list fidelity validation. |
src/discover/registry.rs |
Prevents unsafe pipeline rewrites. |
src/cmds/system/search.rs |
Implements list detection and prefix folding. |
src/cmds/system/README.md |
Documents folding behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
KuSh
left a comment
There was a problem hiding this comment.
Round 1 of 3 — APPROVED (three blocking items, all resolved by the fixup below; CI green on the fixup head)
Claim: rtk grep -l/-L and rtk rg -l/--files fold the directory prefix shared by every path into a <prefix> (N files) header, losslessly, and record the saving.
Scope: accept (round 1, frozen)
Ran: develop merge-base 0c279f5 vs head 2b2a662 vs head merged on develop 005eb9c (three target dirs, md5 distinct) × real grep/rg on this repo's tree: -rl, -rL, rg -l, rg --files, absolute and relative roots, multiple roots, -e×2, -i, single-file hit, no match (exit 1), missing path (exit 2 + stderr), -lc, -l0, -lz, --color=always, filenames with CR / ESC / space / leading dash, rg --hyperlink-format; stdin as /dev/null and as FIFO; tracking rows read back from the DB. Invariants: under the hook (every row through rtk rewrite first, pipe consumers included) | every decision (fold / no fold per shape flag) | exact conditions (stdin closed vs FIFO). Savings on absolute roots: 61% (grep -rl 2102→818 B), 70% (rg --files 5394→1593 B) vs the 20% floor (CONTRIBUTING.md); relative roots fold less (grep -rl … src 14%, . 6%), guarded by never_worse, not a finding.
Matrix: never — ran now
CI @2b2a662c (run 35593905660): fmt, clippy, security scans, benchmark and test (ubuntu-latest) green; test (macos-latest) and test (windows-latest) red, all in tests/search_file_list_fold_test.rs (see the CI item below, fixed in the fixup). CI @762a8e62 (fixup head, run 36120223567): 11/11 green, including test (macos-latest) and test (windows-latest)
Blocks merge (3, frozen at round 1)
-
src/discover/registry.rs(producer rewrite, not in this diff) — a folded list is no longer line-faithful, and the hook still rewrites it in front ofhead/tail/cat. Since #3343grep/rgare producer-safe, sogrep -rl foo /abs | tail -3becomesrtk grep -rl foo /abs | tail -3. On develop the-loutput was verbatim andtailreturned the last three paths. With this PRtaildrops the header and returns tails with no prefix:$ rtk grep -rl LazyLock $R/src | tail -3 # develop $R/src/cmds/rust/cargo_cmd.rs ... $ rtk grep -rl LazyLock $R/src | tail -3 # this PR cmds/rust/cargo_cmd.rs ...rg -l … | tail -n +2drops the header instead of the first file (28 lines against raw's 27), andrg --files … | catgets one line more than raw. Five of five list-into-consumer forms tried were wrong;| xargs,$( ),> fileand| while readare not rewritten, so they are not reached. This is #4207's class (positional consumers after the #3343 rewrite), which is pre-existing for groupedgrep -noutput, but for-l/-L/--filesit is new: develop's output there is byte-identical to raw.
Fixup dadcfde:producer_output_is_line_faithfulkeeps a grep/rg producer raw whenis_bare_file_listsays it is a list (is_bare_file_listbecomespub(crate)). Other grep/rg forms,-lc, and-e -lstill rewrite. Testtest_rewrite_pipe_producer_file_list_stays_raw, which fails when the veto is mutated totrue. -
src/cmds/system/search.rs:631— the fold accepts lines that are not plain paths.str::lines()drops a\rbefore\n, so a file namedx\rcomes out asx, a path that does not exist (raw and develop keep it). rg's--hyperlink-formatOSC 8 links are not removed bystrip_ansi, so the shared prefix ends insideESC]8;;file://…/and every tail starts mid-escape. Fixup 3223a80:fold_path_prefixreturnsNonewhen the raw text holds\0,\ror ESC. The check is on the raw text becauselines()has already eaten the\r. Unit assertsCR in a nameandOSC 8 hyperlink; theCR in a nameassert fails when the guard is removed. -
tests/search_file_list_fold_test.rs— red on macOS and Windows CI. Windows: 4 of 8 fail because the test builds expected paths with/while grep there prints mixed separators (…\src/b/none.rsvs expected…\src\b/none.rs); the fold itself works (…\src/ (3 files)). The file has no#![cfg(unix)], which every other search integration test has. macOS:null_joined_list_is_verbatimfails because BSD grep reads-Zas--decompress, so the output is newline-joined;count_with_list_is_verbatimfails because BSD grep prints bothpath:countandpathunder-lc(7 prefixed lines, the test expects GNU's 3). rtk's own behaviour is right in both: no header, output forwarded as is. Fixup 762a8e6:#![cfg(unix)],--null(what rtk itself passes to grep), and the-lccase compared against rawgrep -rlcoutput instead of a GNU count.
Optional — will not hold merge; threads resolved by me
- Copilot's thread at
search.rs:637(newline inside a filename): a newline-named file makes the header count wrong, but nothing is lost relative to raw. Withcommon/common\ncommon/fileandcommon/other\ncommon/file,rtk grep -rl needle commonprintscommon/ (4 files)for 2 files, and prefix + tails rebuilds rawgrep -rlbyte for byte. Raw newline-separated output is already ambiguous for such names;--nullis the unambiguous form, and it is never folded. With absolute roots the fragments do not share the prefix, so nothing folds at all. - A tie in
never_worsekeeps the fold, so a fold that saves 0 estimated tokens is reshaped and tracked as a 0% row.estimate_tokens(f) < estimate_tokens(&cleaned)would avoid it. is_bare_file_listtokenizes a third time and repeats the list-flag names fromis_format_flag_token; a shared "shape kind" would keep the two tables from drifting.
Filed as follow-ups
- #4238
rtk rg --files <path>treats the path as a pattern when stdin is a pipe: pre-existing routing (stdin_is_readable()), outside this PR's changes. With a pipe on stdin the list streams verbatim and is tracked 0/0; with</dev/nullit folds. The output is correct either way.
Checked and correct — no need to re-verify
- The fold is lossless on real trees: prefix + tail rebuilds
grep -rlbyte for byte andrg -las a set (rg's own order varies run to run). - Component cut, relative roots, multiple roots,
-e×2,-i, leading-dash and space filenames all fold correctly. -lc,-l0,-lz,--json,-q, rg-L(follow): no fold.- No match gives exit 1 with empty stdout; a missing path gives exit 2 and the stderr is forwarded unchanged.
- Tracking: a fold records real sizes as
rtk grep/rtk rg; no fold stays at 0/0 passthrough. - A single hit and piped
-linput are left verbatim. grep -rl -- -cfolds (the tokenizer already treats post---tokens as positional).- Gate on head and on head merged with develop 005eb9c: fmt, clippy (0 warnings),
cargo test --allall clean. Fixup head 762a8e6: same, 3937 passed; merged with develop 2bf3eb7: full gate clean (3952 passed) and the fold test 8/8.
Hypotheses built and dropped
- "
--followed by a flag-shaped pattern disables the fold": dropped byrtk grep -rl -- -cover two matching files (stdin/dev/null), which folds. The first repro matched a single file. - "An ESC byte in a filename splits the header": the fold is already refused on develop and on the head, and the fixup refuses it explicitly.
Your questions
- none asked.
Next
Fixup dadcfde + 3223a80 + 762a8e6 pushed:
- keep a grep/rg file-list producer raw before
head/tail/cat - refuse to fold output containing CR or ESC
- make the fold test pass on BSD grep and skip it on Windows
CI is green on 762a8e6, so this is approved.
Rounds: 1/3. Threads: 0 open, 1 resolved this round (Copilot, answered above).

Follow-up to #2630, re-targeted at
search.rsafter #2641 removedgrep_cmd.rs(thanks @KuSh for the pointer).Problem
rtk grep -l,grep -L,rg -l, andrg --filespass through verbatim. Each line prints the full path and the run records 0/0 savings. Agents search from absolute roots, so every line repeats the same long directory prefix.Change
A bare file list has its shared directory prefix folded into a header:
prefix + tail, engine order is kept, no cap or tee./a/foobar/x+/a/foobaz/yshare/a/, not/a/fooba).never_worse.-c,-Z/--null,--json,-qchange or remove the line, so those stay verbatim.rg -L(--follow) is not treated as a list.-lon piped input) is untouched.rtk gain; unchanged passthrough still records 0/0.Nonewhen there is nothing to fold: fewer than two lines, a blank or NUL-joined line, no shared directory, or only/shared.Known limitation: only
/separates components, so Windows-style\paths never fold.Tests
fold_path_prefix(fold, order, component cut, lossless round-trip, allNonecases) andis_bare_file_list(per-engine list flags, rejection of other shape flags,-e -las a pattern).tests/search_file_list_fold_test.rs):grep -rl,grep -rL,rg -l,rg --filesfold; single line,-Z,-lc, and piped stdin stay verbatim; no-match exits 1 with empty stdout.grep_faithful_format_test::dash_l_and_dash_cap_l_match_grepnow unfolds the header and compares path-for-path against realgrep, since byte-identity is no longer the contract for-l/-L.cargo fmt,cargo clippy --all-targets, andcargo testare clean apart fromgit_log_malformed_digit_run_propagates_real_git_error, which fails identically ondevelophere (local git version wording).