Repository navigation
Conversation
Wraps `php run-tests.php` and collapses its per-test PASS chatter into one summary line plus a bounded list of failure diffs. On php-src's 5322-test suite with 467 failures, output drops from 1.3 MB to 6 KB (99.5% reduction). An all-pass run collapses to two lines. Handles PASS/FAIL/SKIP/BORK/WARN/LEAK/XFAIL/XLEAK, ANSI color codes, the ========DIFF======== / ========DONE======== blocks, and the trailing FAILED TEST SUMMARY block (which must not be re-counted). XFAIL and XLEAK are expected results (reported under "Expected fail"/"Expected leak" by run-tests.php) and do not count as failures. The hook rewrite maps `php run-tests.php …` to `rtk phpt …` via a rule in discover/rules.rs, with `phpt` added to the passthrough classifier. Diff buffering is capped at the display limit while a separate counter keeps the "+N more diff lines" note accurate. When the summary block reports failures but per-test lines were truncated out of the capture, the FAILURES header is still emitted with a note rather than dropped. Tests: inline units plus an insta snapshot and a count_tokens-based token-savings assertion (>=60%), per .claude/rules/cli-testing.md.
It was my fault I shouldn't have made that request
Running the filter against php-src instead of captured fixtures exposed
three defects.
run-tests.php redraws its progress line, so a serial run (no -j) delivers
the status after a carriage return: "TEST 1/2 [x.phpt]\rFAIL desc
[x.phpt]". The status anchor never matched, so every per-test status was
lost and failures printed as "details unavailable". Splitting on CR fixes
it. The original 5322-test validation ran with -j8, where workers suppress
the progress line, which is why this stayed hidden.
php writes its startup diagnostics to stderr ("Could not open input file:
run-tests.php", the illegal-switch help), and stdout_only dropped them.
The never-worse guard then had an empty base, so rtk printed nothing at
all. The filter now reads both streams and passes output through unchanged
when it isn't run-tests.php output.
run-tests.php emits DIFF blocks only with --show-diff or for a single
selected test, so multi-test runs listed failing test names without a
diff. rtk now asks for them unless the caller passed its own --show-* flag.
Verified on php-src: 733-test serial run, 12 failures with diffs, 97.1%
reduction (153884 to 4476 bytes).
--show-diff was injected unless any --show-* flag was present, but only --show-diff and --show-all produce the DIFF block. --show-slow, --show-mem, --show-out, etc. don't, so `rtk phpt --show-slow 1000 …` suppressed the injected --show-diff and stripped every failure of its actionable diff. Match only --show-diff/--show-all, print the truthful injected command in verbose mode, and add a regression test for the --show-slow case.
Prevents accidentally committing machine-local Claude Code settings, which can contain local tokens/paths (same risk class as CVE-2025-59536 / CVE-2026-21852 for committed local-settings files).
Closes #3184. mvnd is a separate binary, not a mvn wrapper, so it gets its own `rtk mvnd` entry point rather than folding into `rtk mvn`: a user who typed `mvnd` must get the daemon, never `mvn` or a `./mvnw` wrapper that happens to sit in the working directory. Everything downstream is shared — phase detection and all four filters are reused unchanged. mvnd's rolling/parallel console UI only engages on a TTY, so the output rtk captures is plain Maven output; daemon status chatter carries no [INFO]/[ERROR] prefix and is already dropped by the existing keep-lists. - src/main.rs: `Commands::Mvnd` -> `mvn_cmd::run_daemon` - src/cmds/jvm/mvn_cmd.rs: thread a `daemon` flag through binary selection; `run`/`run_daemon` share `run_tool` - src/discover/rules.rs: rewrite rule so the hook maps `mvnd <goal>` to `rtk mvnd <goal>` (the `mvn\b` pattern never matched `mvnd`) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code review found the README overclaimed. Two corrections: - `filter_quiet` has no keep-list, only a deliberate "keep anything else" safety net, so daemon chatter is NOT dropped there — only the three keep-list filters drop it. Documented as a known limit. - mvnd builds multi-module reactors in parallel regardless of TTY, so interleaved Surefire blocks degrade savings toward passthrough. Also documented rather than claimed away. Both need a real captured mvnd fixture to fix honestly, and both already apply to `mvn -T`, so neither is a regression. Also drops `mvn_binary_without_daemon_is_not_mvnd`: no non-daemon branch can return "mvnd", so it was asserting a literal against itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rtk init -g --auto-patch aborted with 'expected value at line 1 column 1'
on a settings.json that begins with a UTF-8 BOM — which Notepad and
PowerShell 5.1's Out-File -Encoding utf8 both write by default. The hook
was never installed and the error blamed the user's (valid) JSON. Worse,
the statusline patch path fell back to json!({}) on the parse failure
and would silently wipe existing settings on write-back.
strip_leading_bom already existed in hook_cmd.rs, guarding all four hook
stdin entry points; the file-read paths never got the same treatment.
Promote it to core::utils and apply it at every site that parses JSON a
human may have hand-edited: settings.json / hooks.json readers in init.rs
(12 sites), hook_check.rs, integrity.rs, and permissions.rs.
TOML sites (config.toml, filter DSL) are deliberately untouched: the toml
crate tolerates a leading BOM, pinned by a new regression test. The BOM
is dropped on write-back — RFC 8259 s8.1 forbids adding one to JSON.
Fork issue: #58 (stays open until the upstream PR is submitted)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ment Review round on PR #62 found two hand-edited JSON parse sites the sweep missed — read_composer_bin_dir (core/utils.rs) and parse_global_json_mtp_mode (dotnet_cmd.rs, where BOMs are most common) — plus a test comment claiming the toml crate rejects BOMs when it in fact tolerates them (which is the very reason TOML sites are untouched). Also adds the BOM+invalid-JSON edge test the spec asked for: stripping must not mask genuinely broken JSON. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`head -N <file>` and `tail -N <file>` rewrite to `rtk read`, but their branch in rewrite_segment_inner returned before the is_excluded() check further down, so listing "head" or "tail" in [hooks] exclude_commands had no effect. Users who want file contents read unfiltered had no way to opt out. Adds the exclusion check to that branch, mirroring how the Supported branch strips an env prefix before matching, plus regression tests covering both the excluded and non-excluded paths.
`-m` was derived as the short for RTK's `--max` (a display cap), colliding with GNU grep where `-m N` = `--max-count` (stop after N matches per file). `rtk grep -m N` set RTK's display cap and never forwarded --max-count, so grep didn't actually stop early — a silent semantic mismatch (no parse error). Drop the short. `-m` now flows to extra_args and is forwarded verbatim to grep/rg (it is not a has_format_flag letter, so it takes the normal engine path, not passthrough), applying genuine --max-count while RTK still compacts. This makes Grep consistent with Rg, which never had the short and already forwarded `-m` correctly. `--max` keeps its long form and default 200. - src/main.rs: drop `short` from Grep::max + clap-parse regression test. - tests/grep_faithful_format_test.rs: end-to-end `rtk grep -m N` == `grep -n -m N` (leading, trailing, N >= total) plus the per-file multi-file case. - docs/usage/FEATURES.md: drop the `-m` short; fix stale default 50 -> 200.
- Cap displayed entries at CAP_INVENTORY (50); overflow spills to a tee file holding ONLY the dropped entries, recoverable with one command: [see remaining: tail -n +1 ~/.local/share/rtk/tee/<ts>_ls-hidden.log] - Record RTK-filtered noise dirs (non-dot only) in the same tee file and surface counts as "... (N more, M filtered)" - Follow standard ls dotfile semantics: pass -a to the underlying ls only when the user asked for it (long format kept for parsing) - Drop the TTY-only Summary line and extension histogram: agents never saw it and it misled humans about agent-visible output Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZkPEr1jwjMViU4h4uPUpN
…blanks Review follow-up for #3199: real captured mvnd 1.0.6 fixtures plus regression tests, as requested. The fixtures surfaced a daemon-specific bug: mvnd routes all output through the daemon logger, which prefixes even blank lines with `[INFO] `. The Surefire failure-trail terminator keyed on a truly empty line, so under mvnd the trail never closed and everything after the last failing class passed through verbatim, help boilerplate included (35% savings instead of the >=60% floor). is_blank_separator now treats `[INFO]`-only lines as blank in the trail and re-arm states. - mvnd_reactor_pass_raw.txt: warmed `mvnd clean install` on the multi-module skeleton — SmartBuilder parallel reactor with `[module] [INFO]` prefixes and daemon chatter - mvnd_test_fail_raw.txt: warmed failing `mvnd test` (exit 1) on the multifail skeleton - mvnd_compile_error_raw.txt: `mvnd compile` syntax error - multifail-skeleton: six passing test classes so the failing-run capture is representative of real projects (an all-failures run is all signal and makes any savings floor meaningless) - six regression tests: reactor summary survives, failure names, messages and user stack frames survive, compile diagnostics survive, daemon chatter and module-prefixed noise dropped, >=60% token savings asserted on the success and failure runs - README: `rtk mvnd` added to the root command list Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three independent bugs bundled in issue #3401, each an over-loose match_command regex that activated a filter on commands it should never touch: - spring-boot: java -jar *.jar matched ANY jar invocation, not just Spring Boot apps, silently collapsing unrelated tool output to the filter's Spring-specific keep-list. Now requires "spring" in the jar filename (mvn/gradle alternatives were already Spring-specific). - liquibase: unanchored substring match fired on any command containing "/liquibase" anywhere, e.g. `rm -rf /opt/liquibase`. Anchored to the invoked program name instead. - ssh: `\b` can't match between "ssh" and a following "-", so ssh-keygen/ssh-add/ssh-copy-id incorrectly activated the plain-ssh connection filter. Replaced with an explicit (\s|$) boundary. TOML [[tests.*]] blocks only exercise each filter's output pipeline, never match_command routing, so added a Rust unit test per filter (colocated in toml_filter.rs) covering both the excluded false positive and the still-matching true positive. Closes #3401
Code review caught a case-sensitivity gap: the jar-filename check only matched a lowercase "spring" substring, so PascalCase/CamelCase artifact names (e.g. MySpringApp.jar) silently failed to activate the filter. Scope the case-insensitive flag to just that group so the mvn/gradle alternatives are unaffected.
Code review caught that \S* spans '/', so the "spring in jar name" check actually matched "spring" anywhere in the full path argument (e.g. a directory literally named spring-cache), not just the jar's own filename as intended. Split the path prefix from the filename match so "spring" must appear in the final path segment. Also closes a coverage gap: the ssh fix's end-of-string branch (bare `ssh` with no args) had no test.
…_diff All change lines are already printed in full (the comment says "Never truncate diff content"), but "... +N more" was appended when total changes exceeded 10. This confused LLM consumers into requesting "remaining" data that was already shown, wasting tokens.
`rtk find --max N` caps how many names are displayed but still scans and summarizes the whole tree, so its output must be compared against the full `find` a user would otherwise read. The old baseline piped through `head -N`, which truncates the raw scan to a different (early-terminated) operation. On small repos `head -10` produced fewer bytes than rtk's summary header, marking `find --max 10` as a spurious negative that failed the whole benchmark job. Dropping `head -N` makes both --max rows compare like-for-like and stable.
mvnd builds multi-module reactors in parallel and prefixes per-module log lines with `[module] `, interleaved line-by-line, while stack traces stay raw. The Surefire matchers keyed on `[INFO]`/`[ERROR]` at column 0, so in a parallel reactor a failing class lost its diagnostics: only the reactor summary and the generic goal error survived. - `split_lane` classifies on the prefix-stripped view and emits the original line, so module identity survives in the output. - One `SurefireLane` (block machine + keep-continuation + failures-summary cap) per module, so a passing close from one module can no longer be attributed to another module's open block. Unprefixed raw lines route to the lane that most recently opened a block or failure trail. - `filter_compile` classifies on the stripped view too. Adds a real captured fixture (`mvnd clean test` on the new multi-module-fail-skeleton, exit 1, interleaved reactor): the failing class, its assertion message, the user stack frame, and the summary entry all survive at 72.1% token savings. Adds insta snapshot regression tests locking the full filtered output of all four mvnd fixtures, per docs/contributing/CODING_PRACTICES.md. Updates the jvm README: the parallel-reactor limit it documented is fixed; only the `-q` daemon-chatter limit remains. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t-3184-upstream # Conflicts: # Cargo.lock
…tor-wide caps Three correctness fixes for mvnd parallel reactors, all driven through a single shared lane-routing path: - Raw-line ownership: an active failure trail now outranks an ordinary open block (Lanes::raw_owner), so a module opening a Surefire block between another module's failing close and its unprefixed exception trail can no longer swallow the diagnostics into a block that closes green. When ownership is genuinely ambiguous (several plain blocks open, no trail), the raw line is preserved verbatim instead of routed to a guess. - Compile continuations are lane-aware: filter_compile tracks keep_continuation per module via the same Lanes type, so an interleaved line from another module no longer disarms the flag between "[ERROR] cannot find symbol" and its raw symbol:/location: continuations. - The [ERROR] Failures: summary budget is reactor-wide: lanes keep only an in_summary flag while FailuresSummaryCap holds one shared count, so interleaved per-module summaries can no longer retain modules x cap entries. Same for the failing-class cap (FailingClassCap). filter_surefire_with_cap and filter_package_with_cap now share the routing/step/cap front half (drive_surefire_line) instead of carrying two copies, and every interleaving regression is asserted on both the surefire and package entry points. The mvnd rules.rs entry uses ..RtkRule::DEFAULT like its neighbours. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…vocation-wide Closes the three remaining multi-lane cases from review, by replacing the raw-line routing heuristics with one literal invariant instead of patching each case: - Lanes::raw_owner routes an unprefixed raw line only to a UNIQUE owner: exactly one lane in a failure trail (trails still outrank open blocks), else exactly one lane with an open block, else the unique lane with an armed compile continuation. Any tie — several trails, several blocks, or an armed lane alongside either — preserves the line verbatim rather than routing it into a lane that may drop it. The note_hot priority heuristic is gone; `hot` has a single writer (a failing close) and is only consulted when nothing is open and nothing is armed. - The armed-continuation claim lives in the per-lane flags and is scanned across all lanes, so a failing close stealing `hot` cannot orphan another module's pending symbol:/location: continuations; entering a Surefire block retires a lane's stale armed claim. - FailuresSummaryCap's budget spans the whole invocation (open_lanes and its reset are gone): sequential per-module summaries share one budget exactly like interleaved ones, so a run keeps at most cap entries, never modules x cap. Ownership state space is closed by construction — every claimant combination either routes uniquely, preserves verbatim, or is loss-free because a dropping trail implies the class cap is already exhausted (FailingClassCap::admit is monotonic; documented on raw_owner). Coverage moves from hand-picked orderings to exhaustive order-preserving merge sweeps, all asserted on both the surefire and package entry points: failing x passing module (210 merges), two failing modules at cap=1 (924), compile-error x passing (35), compile-error x failing (84) — mvnd's scheduler controls interleaving, so any merge is a run that can happen. Plus targeted regressions: sequential summary budget, interleaved compile continuations on the test/package paths, a cap=1 three-module case where continuations land during another lane's dropping trail, and block-entry arm retirement. The 14 targeted tests fail on the parent commit with only the code changes reverted; all four real-fixture insta snapshots are byte-identical, so the normal-path output is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The keep-list fall-through reset of keep_continuation exists to stop a stale flag after a lane's own dropped [ERROR] line — but a raw line only reaches a lane *because of* its armed claim, so letting it clear that claim was self-defeating: another module's unprefixed stray stdout landing between "[ERROR] cannot find symbol" and its raw symbol:/location: continuations disarmed the claim and dropped the diagnostics. drive_surefire_line now reports whether the line was keyed (module prefix, routed by key) or raw (routed by ownership claim), and all three filter paths only rewrite keep_continuation on a lane's own keyed lines. The sequential stale-flag protection is unchanged; kept raw lines could never arm anyway (an [ERROR]-initial line would have been keyed). Regression: the raw stray swept through every position of the compile-error sequence, on filter_surefire, filter_package, and filter_compile (mvnd_[package_|compile_]raw_stray_does_not_disarm_ continuation). All three fail on the parent commit with only the code changes reverted; the four real-fixture insta snapshots stay byte-identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same principle as the previous commit, applied to the other armed claim in the machine: trail_rearm carries a multi-failure class's keep/drop decision across its blank-separated per-test detail blocks, and step() disarmed it on any non-blank line — including a raw stray routed to the lane by ownership fallback. A stray landing between two detail blocks made the next per-test subline miss its trail re-entry, silently dropping its exception message (kept class) or leaking the block that should have been dropped (capped class). step() now takes the keyed flag and only disarms trail_rearm on the lane's own keyed lines. Per-test sublines are always keyed (they start with "[ERROR] "), so a raw line can neither be the re-entry line nor prove the trail is over. The sequential disarm paths are unchanged: "[INFO] Results:", RUNNING, and commit_failing/drop_failing all still clear it (surefire_rearm_disarms_at_results_boundary passes untouched). Regression: the multi-failure sequence x raw stray swept through all 10 positions on both the surefire and package entry points, plus the capped drop-side variant (the stray must not leak a capped class's second block through the [ERROR] catch-all). All four fail on the parent commit with only the code changes reverted; the four real-fixture insta snapshots stay byte-identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Two concurrently armed lanes are a tie on their own: raw continuation lines are preserved verbatim rather than routed to a guess. Pinned on all three filter paths (in filter_compile this is the route -> None arm, previously exercised only via armed+trail / armed+block ties). - filter_compile gains the same CRLF equivalence test the surefire and package paths already had, against the real mvnd compile fixture. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Also fixes two docs left stale by the barrel-import removal: hooks/pi/README.md and hooks/README.md still described bash-tool detection via the removed isToolCallEventType value import. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
perf(hook): avoid Pi package barrel import
- Drop the needless borrow at the format_diff_changes call site; with warnings = "deny" this was failing cargo clippy --all-targets and blocking the clippy job that build, test and release depend on. - Keep the original both-files dump as the timer.track baseline. The synthesized classic diff is empty for identical files, which recorded input_tokens = 0 and reported ~0% instead of ~46% in rtk gain --history. - Un-gate render_file_diff: fold it into render_diff rather than compiling a production helper only under cfg(test). - Update the scripts/test-all.sh assertion to match the real output.
…th files The guard picks between the classic fallback and the condensed render, so the fallback is what rtk diff has to beat -- but savings were still booked against a dump of both files, a string no command ever emits. The two had drifted apart, so rtk gain could record a loss: -5% on rtk diff Cargo.toml LICENSE, and -250% on two empty files. Measuring against the fallback makes a negative impossible rather than merely unobserved, since the guard already caps the shown output at it. Identical files, where diff prints nothing, keep the both-files baseline so their recorded ratio stays comparable; two near-empty files can make that dump cheaper than the verdict line, which would book a loss against the cheapest possible answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tput fix: render a real diff instead of dumping both files for modified lines
feat(phpt): add PHP .phpt filter for php-src run-tests.php (-99%)
Contributor
|
PR #1503 has been merged without rebase, causing ci to crash because we went for LazyLock instea of lazy_static meanwhile. |
fix(phpt): use LazyLock and RtkRule::DEFAULT so develop builds
Contributor
fix(hooks): match exclude_commands against the peeled command form
feat(ls): cap listing with tee tail hint and standard dotfile semantics
compact_diff indented every hunk-body line two spaces to nest it under the filename. The content was all there, but the indent moved it off column 0, so `git diff | grep '^-'` matched nothing and an audit asking "was anything removed?" got a confident, wrong "no" while --stat reported deletions. Worth being precise about the failure, because the earlier write-up was wrong on both counts: nothing was ever dropped, and added lines were broken identically (`^+` also matched nothing). It is not a deletions-only defect and not content loss. It is one indent breaking every anchored grep. Emit hunk bodies in git's own unified shape instead. Context lines already carry git's leading space, so they stay distinct from additions. rtk's own annotations (the `+N -M` tally, `... (N lines truncated)`) keep the indent precisely so these same greps never count them as diff lines. The output still cannot be mistaken for an applyable patch: rtk drops the `diff --git`/`---`/`+++` headers, so a patch parser rejects it either way. The indent was never what made that safe.
The `+++` and `---` guards ran inside the `in_hunk` branch, where they can only match content: git emits the file headers before the first `@@`, and `in_hunk` resets to false on every `diff --git`. So an added `++i;` or a removed `-- sql comment` was dropped from the body and left out of the `+N -M` tally. A diff doing both rendered as a bare hunk header with no body and no tally, and `grep -c '^-'` answered 0 for a file that really did lose a line. Leading context now has its own budget of three lines. The `hunk_shown > 0` guard silenced every context line before a hunk's first change, and giving it `max_hunk_lines` instead would let a long run of context crowd out the change it is there to frame. The truncation note splits by sign: `... (30 deletions, 30 additions truncated)`. It stays indented so the anchored greps this output is meant to support never count it, which means a reader who trusts `grep -c '^-'` past the 100-line hunk cap needs the note to say how many deletions it did not see. One merged line count could not. The comment on the column-0 change narrated the bug's history and sat on the `@@` branch while describing the branch below it. It now states the invariant, once, where the code it governs lives.
…text Dropping the `+++` / `---` guards assumed those headers only appear before the first `@@`. That holds for `diff --git` sections. It does not hold for the `diff --cc` sections git emits for unmerged paths: `diff --cc` never matched the `diff --git` branch, so `in_hunk` stayed set from the previous section and the `--- a/z.txt` / `+++ b/z.txt` headers of every later file were printed at column 0 and counted in the tally. On a two-file conflict with no deletions, `rtk git diff | grep -c "^-"` returned 1. The state reset now fires on any `diff --` prefix, and the path falls back to the trailing token for the `diff --cc <path>` shape, so the headers land before the first `@@` again. Same repo now returns 0. Leading context no longer counts against `max_lines`, so it cannot displace change lines. Its previous justification was wrong: before this branch, leading context was never emitted, so it could not have crowded anything out of `max_hunk_lines`. The cost it does carry is on the global budget, and on a five-commit diff of this repo it cost 57 change lines (144 shown, against 201 with the exemption). A diff-wide cap of `max_lines / 10` bounds what the exemption gives back: without it, a diff of many small hunks would spend three exempt lines on every one of them. The same diff comes out at 71.7% byte reduction against 71.8% on master, which showed zero anchorable change lines. `docs/usage/FEATURES.md` showed change lines indented under a `src/main.rs (+5/-2)` header, which no version of rtk emits. It now carries real captured output, plus the two limits of the anchored audit: the 100-line per-hunk cap, and that the output is not an applyable patch.
A hunk ran until the next header, so any trailing text was read as its body. `gh pr diff --patch` routes here, and GitHub's `.patch` is an mbox: each patch carries a bare `---` before the diffstat and a `-- ` signature after the diff, both at column 0. With hunk bodies now anchoring, a two-commit mbox with two real deletions reported `grep -c "^-"` = 5. `@@ -a,b +c,d @@` declares its own line counts, so the hunk now ends when they are consumed and anything after it is outside every hunk. That covers trailing prose and truncated input too, rather than the shapes we happen to know about, and a heuristic on `---` / `-- ` could not have worked: a deleted line whose content is `- ` renders as `-- `. `--patch` also joins `has_non_diff_format_flag`, so an mbox passes through whole. Someone asking for a patch wants one that applies. Leading context keeps the last lines before the change rather than the first. With `-U10` or `--function-context` the first three sit ten lines above it, and emitting those tells the reader that ctx3 precedes the deletion when ctx10 does. A three-slot ring buffer costs the same lines. The diff-wide budget is charged when the buffer drains, so a line the ring evicted costs nothing. Combined diffs carry one marker column per parent, so a line changed against only one of them holds its marker in column 2. Those were counted as context: neither in the `+N -M` tally nor reachable by an anchored grep. The tally reads both columns now. The grep gap is real and cannot be closed without rewriting the line, so FEATURES.md states it alongside the per-hunk cap. Against `git diff --numstat` on three complete commit ranges the tally is now exact, where master under-counted one of them by +2 -1. `diff_header_path` keeps paths containing spaces: git does not quote them, so `diff --cc my file.txt` needs the whole remainder, not the last token. FEATURES.md said 15 lines for a 24-line block and showed the stat's first line with a leading space it does not have. It now also carries a measured example with real compression, since the two-file one saves 11% in a document advertising 60-90%.
…very parent Four problems from the review, one of them a crash. `&line[..width]` slices by byte index. `--word-diff`, `--color-words` and `--word-diff-regex` emit body lines with no marker column, so their content lands where the markers are read, and a leading multi-byte character split mid-character: `rtk git diff --word-diff` aborted with exit 101 and empty stdout on a file holding `école`. `run_diff`, `run_show` and `git stash show -p` have no `catch_unwind`, so the panic was fatal there. The markers are ASCII by construction, so comparing bytes removes the boundary question. A combined diff carries one marker column per parent, and a space means opposite things depending on the line: on a change line the line is in that parent unchanged, on a removal line it is absent from it. Collapsing the columns into one add/delete pair lost that, so `old` never converged on real conflict output and the hunk never closed by count -- the declared-length termination this commit's predecessor is built around silently did not apply to `diff --cc`, which is what git emits for every unmerged path. The reverse case closed early and dropped the body: `@@@ -1,2 -1,4 +1,2 @@@` over two removals from the second parent alone rendered as a bare hunk header, with no tally and no truncation note. `HunkHeader` now tracks a budget per parent and ends the hunk when every one of them and the result are spent. The leading-context ring drained only on a hunk's first change line, so a hunk that ends without one discarded what it held and rendered as a bare header. Flushing wherever the hunk closes -- and at end of input -- keeps it consistent with the buffering, inside the same diff-wide budget. `diff_header_path` mishandled git's quoted paths. Under the default `core.quotepath`, git escapes a non-ASCII path and wraps it in `"`, which removes the ` b/` the plain form splits on, so the fallback returned the whole remainder -- both paths -- as the section header. The quoted form is handled explicitly now, and the doc comment names the distinction the code makes: git quotes non-ASCII bytes, not spaces.
Four more from an adversarial pass, two of them undermining the previous
commit's own rationale.
`--word-diff`, `--word-diff-regex` and `--color-words` drop the marker column
and put `[-removed-]` / `{+added+}` inline, so a word-diff body line is
arbitrary content in the position `compact_diff` reads as a marker. The byte
slicing stopped the panic, but not the misparse: a line starting `+` counts as
an addition and consumes the wrong budget, one starting `\` is dropped as a
no-newline annotation, and a real `git show --word-diff=plain` row reading
`diff --git [-a/src/main.rs...` opens a new file section. There is no faithful
compaction of a word diff, so `run_diff`, `run_show` and `git stash show -p`
pass these modes through, the way `gh pr diff --patch` already does for mbox.
`rtk pipe --filter git-diff` has no flags to read and still misparses word-diff
text piped into it; `compact_diff` reads a unified or combined diff, and that is
now what its callers give it.
A hunk header lists one range per marker column, and a mismatch was charged
against nothing. `@@@ -1 +1 @@@` left the second parent untracked, so ` -lost`
drove the tracked budgets to zero and the hunk closed with the removal
discarded. `@@@ -1 -1 -1 +0,0 @@@` declared a third parent with no column to
spend it, so that budget never reached zero, the hunk never closed by count, and
the mbox signature and version became its body. The ranges are reconciled with
the columns now: a missing one gets `usize::MAX`, which keeps the hunk open to
the next header rather than dropping content, and an extra one is dropped.
`diff_header_path` split on the first ` b/`, which is inside the path for a file
under a directory named `x b` -- `diff --git a/x b/y b/x b/y` reported `y`. And
`--no-prefix` or a custom `--src-prefix` leaves no ` b/` at all, so `diff --git
x x` reported both paths as the section header. Anything but a rename names the
same path twice, so the halves are the same length and the separating space sits
dead centre; splitting there handles all three and does not care what the
prefixes are. A rename's halves differ in length and still fall through to the
` b/` split.
Ordinary unified diffs are byte-identical to the previous head: verified on a
26,561-line real diff of this repo and on a 390-line one.
…plitting Four findings on the section header and the word-diff guard: - Under the default `core.quotepath` git escapes a non-ASCII path one octal byte at a time. Unwrapping the quotes without decoding them printed `\303\251t\303\251.txt`, so a grep for the file by name found nothing. - A single-path `diff --cc` / `diff --combined` header was split at its midpoint like a two-path one, reading a file called `dup dup` as `dup`. - Prefixes were matched by name, so `--dst-prefix` fell through and the header carried the whole pair. Matching the halves against each other reads any prefix, named or not. - `--word-diff=none` is the mode that turns a word diff back off, leaving an ordinary unified diff. Treating it as a word diff passed the raw diff through, so a defensive `--word-diff=none` lost every saving. Modes are last-one-wins, which is what that mode is for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A hunk truncated at exactly one line over the cap rendered `... (1 deletions truncated)`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eled `configuration.md` said an excluded tool "is covered however it is invoked". The stated limits then covered only tools without their own filter, and exactness — leaving three invocation forms of *filtered* tools promised but not delivered. Scope the sentence to the wrapper, interpreter and path spellings that are actually peeled, and table the three that are not, each with its cause: - `head -20 f` / `tail -n 5 f` — the line-range fast path returns before the exclusion is consulted (#2823); `head f` is excluded normally - `gradlew.bat build` / `mvnw.cmd test` — path stripping splits on `/`, so a `.bat`/`.cmd` spelling never reduces to the tool name (#3617) - `golangci run ./...` — `golangci run` is one of the rule's own aliases and is kept whole, so it misses a `golangci-lint` entry Each row verified against a build on `develop` at e533c40, including the documented workaround: `["golangci"]` excludes the alias form but not `golangci-lint run`, so both entries are needed. Docs only, no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(git-diff): emit hunk lines at column 0 so `^-` anchors again
ENV_PREFIX is anchored on sudo/env/VAR=val, and this branch is only reached once cmd_part starts with "head -" or "tail ", so the replace could never match. strip_disabled_prefix already peels any such prefix at the top of rewrite_segment_inner and recurses on the remainder. Match cmd_part directly and cover the prefixed forms with tests so the recursion carries the exclusion, not a strip that never fires. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`uv run` is a routable wrapper, so when the inner rewrite is dropped the loop falls through and re-tests `uv run <cmd>` as a `uv` invocation. With "head" excluded, `uv run head -20 f.rs` still became `rtk uv run head -20 f.rs` — the file contents went through an RTK filter anyway. Stop the fall-through when the wrapped command is excluded, so the exclusion holds whether or not a wrapper is in front of it. Same fix covers the identical hole for excluded `cat` under `uv run`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(hooks): honour exclude_commands for head and tail
docs(config): scope the exclude_commands coverage claim to what is peeled
pszymkowiak
approved these changes
Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feats
Fix
^-anchors again #3269Other
max_file_size#3125