fix(git): stop silently truncating large diffs#3022
Open
georgebashi wants to merge 1 commit into
Open
Conversation
`compact_diff` (used by `git diff`, `git show`, `git stash show`, `gh pr diff`, `glab mr diff`, and the `git-diff` pipe filter) silently dropped diff content at two levels: a 100-line-per-hunk cap and a 500-line global cap. Content past the cap was replaced with a "... (N lines truncated)" / "... (more changes truncated)" marker. This caused the exact data-loss class rtk-ai#827 was meant to fix. rtk-ai#827 ("never truncate diff content", closes rtk-ai#827) only patched the sibling `condense_unified_diff` in diff_cmd.rs (the `rtk diff` stdin path) — its message claimed it also covered the "git diff path", but that path uses `compact_diff` in git.rs, which was never touched. The caps were also effectively unconfigurable: every `git::run` caller passes `max_lines: None`, so `unwrap_or(500)` always applied. The failure is worst under a pipe: the PreToolUse hook rewrites `gh pr diff URL | grep X` to `rtk gh pr diff URL | grep X` (rtk-ai#1560), so grep scans the truncated output and silently finds nothing — an agent concludes the content isn't there. Reported for `gh pr diff | grep` returning empty while `rtk proxy gh pr diff | grep` found the matches. Make `compact_diff` never drop diff content, matching the decision already made for `condense_unified_diff`. Only lossless compression remains: strip diff metadata (index/mode/`---`/`+++`) and append a per-file `+N -M` summary. As a side effect this also fixes rtk-ai#1852 (leading context before the first change in a hunk was dropped) and removes the misleading "[full diff: rtk git diff --no-compact]" hint that named the wrong command on the `gh pr diff` path. The now-unused `max_lines` parameter is retained (prefixed `_`) on `run_diff`/`run_show` to avoid churning the `git::run` signature and its callers; the `--no-compact` / `--stat` passthrough escape hatches are unchanged. - Rewrite `compact_diff(diff)` to drop no `+`/`-`/context lines - Update all 6 call sites (git.rs x3, gh_cmd, glab_cmd, pipe_cmd) - Replace the two truncation-asserting unit tests with content-preservation tests; add a leading-context test (rtk-ai#1852) - Add an end-to-end integration test: a 1200-line `rtk git diff` preserves a marker near the end and stays greppable Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: George Bashi <georgeb@yelp.com>
|
|
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
2 participants
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.
Summary
compact_diffcapped output at 100 lines per hunk and 500 lines total, silently replacing later content with truncation markers. This affectedgit diff/show/stash show,gh pr diff,glab mr diff, and thegit-diffpipe filter.+N -Msummaries. All six call sites were updated; existing passthrough options are unchanged.compact_diffside of the never-truncate fix, which patched the siblingcondense_unified_diffbut missed these paths. It also restores failed downstream searches such asgh pr diff | grep, and fixes a leading-context bug.git/gh/glab): fix(hooks): don't rewrite pipe-feeding segments whose filter changes output shape #2965, fix: add is_unsupported_shape() guard before rewrite_compound() #2903guard::never_worsestill prevents output from exceeding raw input.git log -ppatch bodies dropped: Hook-rewrittengit log -p | rg …silently drops patch bodies → false-clean secret scans #2944, fix(git): preserve patch output from log commands #2951--porcelain/--name-onlycompaction: rtk compacts machine-bound git output (--porcelain / --name-only / piped), silently corrupting consumers #2487rtk diffbreaksgit apply/ exit codes: rtk diff and rtk git diff produce output that breaks programmatic consumers (patch, git apply, --name-only, exit codes) #1918Test plan
cargo fmt --all --check && cargo clippy --all-targets && cargo test— 2,495 passed, 8 ignored, no warningsgit_diff_large_change_preserves_all_content: a 1,200-line diff retains and can grep its final-line markerCloses #1852.
AI-assisted contribution.