Skip to content

fix(git): keep +/-/@@ diff markers at column 0 in compact_diff (adopt upstream #3646) - #118

Merged
kylehgc merged 2 commits into
developfrom
fix/compact-diff-column0
Aug 24, 2026
Merged

kylehgc merged 2 commits into
developfrom
fix/compact-diff-column0

Conversation

@kylehgc

@kylehgc kylehgc commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Closes #102.

What this adopts

Adoption of upstream rtk-ai/rtk#3646 by @ninearif (Arif Waram): keep +/-/@@ diff markers at column 0 in compact_diff.

The ticket named upstream #3269 (@iliaal). During Phase 2 a cluster check found rtk-ai#3646, opened 2026-08-23, which is a strict superset of rtk-ai#3269:

  • Same core fix — all four format!(" {}", line) sites emit at column 0 (hunk header, added, removed, context).
  • Additionally re-spells the per-file tally +N -M → (+N -M), matching the notation diff_cmd.rs already uses, so the tally can never be read as an added line even if a consumer strips indentation.
  • Stronger regression test (test_compact_diff_keeps_diff_markers_at_column_zero): asserts @@ at column 0, context-line leading space, an exhaustive filter that only real hunk lines start with +/-, and the tally re-spell.

Adopting rtk-ai#3646 leaves no residual gap; rtk-ai#3269 is referenced as the passed-over alternative. Both fix the same defect; they conflict textually and cannot both be cherry-picked.

Commits

Commit Author Kind
fix(git): keep +/-/@@ diff markers at column 0 in compact_diff Arif Waram (@ninearif) Cherry-pick of upstream rtk-ai#3646, byte-identical, clean apply on post-sync develop

No fork-authored amendments were needed: all 129 existing git-module tests and the full suite pass unchanged — no fork test had encoded the old indent.

Bug

compact_diff indented every hunk header and hunk body line by two spaces, so any anchored consumer of rtk-filtered diff output (grep -E '^[+-]', grep '^-') silently matched nothing. Because the hook rewrites git → rtk git transparently, the empty result is indistinguishable from "no matching changes" — a convincing false negative, not an error. Affects all compact_diff callers: git diff, git show, git stash show, gh/glab diff views, rtk pipe.

Repro (real binaries, scratch repo with 3 changed lines)

Raw git:

$ git diff | grep -cE '^[+-]'
5        # 2 file headers + 3 real changed lines

Before (installed rtk.exe, pre-fix):

$ rtk git diff | grep -cE '^[+-]'
0
$ echo $?
1        # exit 1, empty stdout — looks exactly like "no changes"

Filtered output had every marker at column 2:

lorem.md
  @@ -1,6 +1,7 @@
  -Sed do eiusmod tempor.
  +Sed do eiusmod TEMPOR.
   Ut enim ad minim.
  +- new bullet
   Duis aute irure.
  +2 -1

After (this branch's build):

$ rtk git diff | grep -cE '^[+-]'
3        # exactly the real changed lines
$ rtk git diff | grep -E '^\+.*bullet'
+- new bullet
lorem.md
@@ -1,6 +1,7 @@
-Sed do eiusmod tempor.
+Sed do eiusmod TEMPOR.
 Ut enim ad minim.
+- new bullet
 Duis aute irure.
  (+2 -1)

(3 vs raw 5: compact mode drops the --- a/ / +++ b/ file headers by design; all real hunk lines are found and the tally no longer masquerades.)

Quality gate

cargo fmt --all && cargo clippy --all-targets && cargo test --all — clean on the branch (2717 unit tests + all integration suites, 0 failures, clippy zero warnings). The same gate also ran green on the merged develop right after today's upstream sync (13 commits), before branching.

Deliberately not fixed

  • Compact output is still not an applyable patch (diff --git/---/+++ headers are dropped by design) — use rtk proxy git diff for a faithful patch. The indent was never what made that case fail.
  • Upstream fix(git-diff): emit hunk lines at column 0 so ^- anchors again rtk-ai/rtk#3269 stays open upstream; no comment was posted there (declining/steering upstream PRs is contributor outreach, a separate decision).

ninearif and others added 2 commits August 23, 2026 21:12
compact_diff indented every hunk header and hunk line by two spaces.
That moved the unified-diff markers off the line start, so anything
reading `rtk git diff` through a pipe as a diff — `grep -E '^[+-]'`,
patch, diff-aware tooling — matched nothing and exited 1. Because the
hook rewrites `git` to `rtk git` transparently, that reads as a clean
"no matching changes" result: a silent false negative.

Emit hunk headers and hunk lines exactly as git prints them, and spell
the per-file summary as `(+N -M)` so it cannot be mistaken for an added
line (the notation diff_cmd.rs already uses). Output shrinks by two
bytes per hunk line. Adds a regression test.
The Avant/Apres example still showed the pre-fix indented format (and was
already stale in other ways: a '~' modified marker and '(+5/-2)' tally that
compact_diff never emitted). Redraw it from real output of the new format:
markers at column 0, per-file tally indented as '  (+N -M)'.

Fork-authored amendment to the rtk-ai#3646 adoption; CONTRIBUTING requires doc
updates for changes to documented behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kylehgc

kylehgc commented Aug 24, 2026

Copy link
Copy Markdown
Owner Author

Review round (pre-merge)

Three passes ran on the branch diff: upstream's code-reviewer.md prompt (uncontaminated, general-purpose agent), and /code-review's Standards + Spec axes.

Verdicts: upstream reviewer APPROVE, 0 critical / 2 important; Spec axis: no missing requirements, no scope creep; Standards axis: 2 hard findings, 2 judgement calls.

Acted on

  • Stale doc example (Standards, hard — CONTRIBUTING §Documentation): docs/usage/FEATURES.md still showed the pre-fix indented rtk git diff format (and was already stale beyond that: a ~ marker and (+5/-2) tally the filter never emitted). Redrawn from real output of the new format in fork-authored amendment 22ee0d4. Docs-only change; the green gate on 75ec765 stands (no Rust code references the file).

Deliberately left byte-identical (cherry-pick integrity)

Follow-ups (out of scope for this adoption)

  • condense_unified_diff in src/cmds/git/diff_cmd.rs:373,401 (the … | rtk diff stdin path) still indents changed lines two spaces — same silent-false-negative class, but a separate command with a deliberately different ultra-condensed format ([file] headers, no @@/context), outside compact_diff's call graph. Needs its own ticket and design decision (its output does not claim to look like git diff).
  • No ≥60% count_tokens savings test exists for compact_diff — pre-existing gap; this change only shrinks output (−2 bytes per hunk line, +2 per file tally).

Cross-checked reviewer claims

  • All 6 compact_diff call sites traced (git diff/show/stash-show, gh, glab, pipe); none re-parse or re-indent the output.
  • CRLF: str::lines() keeps a trailing \r, which does not disturb the column-0 anchor — holds on Windows git output.

@kylehgc
kylehgc merged commit 68d0521 into develop Aug 24, 2026
3 checks passed
@kylehgc
kylehgc deleted the fix/compact-diff-column0 branch August 24, 2026 01:21
KuSh pushed a commit that referenced this pull request Aug 30, 2026
The stdin diff path (git diff | rtk diff -) emitted changed lines
two-space indented, pushing their +/- markers to column 2. Anchored
greps over the output (grep -E '^[+-]') matched nothing — a silent
false negative for any script or CI gate built on diff conventions.

Same class as the compact_diff fix in #118 (upstream rtk-ai#3646), same
design rule applied: change lines at column 0; the [file] header and
overflow line are unchanged and never start with +/-.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

adopt git diff: emit hunk lines at column 0 so ^- anchors again (upstream #3269)

2 participants