Skip to content

fix(git): keep every commit in git log --stat output - #3028

Closed
georgyia wants to merge 1 commit into
rtk-ai:developfrom
georgyia:fix/git-log-stat-drops-commits
Closed

georgyia wants to merge 1 commit into
rtk-ai:developfrom
georgyia:fix/git-log-stat-drops-commits

Conversation

@georgyia

Copy link
Copy Markdown

Fixes #2882

Problem

rtk git log --stat drops most commits, with no notice and no escape hatch. Measured on this repo:

$ git log --stat -30 --format=%h | grep -cE '^[0-9a-f]+$'
30
$ rtk git log --stat -30 | grep -cE '^[0-9a-f]{7,40} '
9                       # 21 commits gone, silently

The reporter saw 18 of 50 missing. An agent reading this gets a confidently incomplete history.

Root cause

Not merge-commit handling, which is where the issue reasonably pointed — merge commits are a symptom.

With --stat, git appends the diffstat after the pretty-format output, i.e. after RTK's trailing ---END--- marker:

<hash> <subject> (date) <author>     <- commit N, from --pretty=format
<body>
---END---                            <- marker terminates the format
 CONTRIBUTING.md | 8 ++++----        <- ...but the diffstat lands *here*
 1 file changed, 4 insertions(+), 4 deletions(-)

So output.split("---END---") yields blocks shaped [diffstat of commit N-1] + [header of commit N] + [body of commit N]. filter_log_output takes the block's first line as the header — which is now a diffstat line — and the real commit header becomes a body line, capped at 3 and collapsed into [+N lines omitted]. Each commit that has a diffstat consumes the next commit's slot.

Merge commits produce no diffstat (git log --stat omits it for merges by default), so the corruption is irregular rather than uniform. That is why it presented as "the parser loses its place around merge commits": they are the blocks where the pattern breaks step, not the cause.

Fix

Lead each commit with the marker instead of terminating it:

-"--pretty=format:%h %s (%ar) <%an>%n%b%n---END---"
+"--pretty=format:---RTK-COMMIT---%n%h %s (%ar) <%an>%n%b"

Every block now begins at a real commit header, and anything git appends after the format — the diffstat — stays inside the block of the commit it describes. No commit can be knocked out of step, and merge commits need no special-casing since the anchor no longer depends on a diffstat being present.

Blank blocks are filtered before take(limit), because the empty region ahead of the first marker would otherwise consume one commit's slot.

Diffstat lines share the existing body budget and overflow through the existing [+N lines omitted] notice, so the issue's "Expected" is met on both counts: every commit is preserved, and anything dropped is disclosed.

Scope note: --stat stays filtered rather than switching to passthrough. #2951 proposes passthrough for -p/--patch and its tests assert --stat keeps being filtered; fixing the parser satisfies both, so the two PRs are complementary rather than contradictory. They touch the same function and whichever lands second will need a trivial rebase.

Verification

native rtk before rtk after
--stat -10 10 commits — 10
--stat -30 30 commits 9 30
--stat -50 50 commits — 50

Hash-level set comparison at -30: commits present in native but absent from rtk went from 21 to 0.

  • New regression tests fail on the pre-fix parser: test_filter_log_output_stat_keeps_every_commit (a merge commit + two diffstat-bearing commits, asserting the middle one survives, mirroring the a0530e8 / 463e523 / deaa799 case in the issue) and test_filter_log_output_stat_overflow_is_disclosed.
  • No regression: old vs new binary output is byte-identical for git log, git log -5, git log --oneline -20, git log -3 --format=%H.
  • cargo fmt --all --check clean, cargo clippy --all-targets zero warnings, cargo test --all 2495 passed / 0 failed.

Token savings, stated plainly: git log --stat -30 goes from 59.3% to 53.1% — the extra words are the 21 recovered commits. Worth being explicit that this filter never met the 60% target on --stat; the old number was closer to it only because it was discarding 70% of the data. I'd rather surface that than quietly claim a win. If you want --stat back above 60%, the natural lever is compacting each diffstat to its N files changed summary line, which I'm happy to add here or in a follow-up.

Test plan

  • cargo fmt --all && cargo clippy --all-targets && cargo test --all
  • Manual testing: rtk git log --stat -10/-30/-50 compared against native git log --stat by hash set
  • Regression tests added, confirmed failing before the fix
  • Token savings measured and reported above

`rtk git log --stat` dropped most commits with no notice: 30 commits in,
9 out, and 18 of 50 absent in the original report.

With --stat (or --numstat/--shortstat) git appends the diffstat *after* the
pretty-format output — that is, after RTK's trailing ---END--- marker. Splitting
on that marker therefore left each diffstat heading the *next* block, where the
parser read its first line as the commit header and demoted the real header to a
body line, capped at 3. Merge commits carry no diffstat at all, so the damage
was irregular rather than uniform — which is why it read as a merge-commit bug.

Lead each commit with the marker instead of terminating it. Every block then
starts at a real header and each diffstat stays with the commit it describes,
so no commit can be knocked out of step. Diffstat lines share the existing body
budget and overflow through the existing "[+N lines omitted]" notice, so what is
dropped is disclosed.

--stat stays filtered rather than passed through, keeping it compatible with
the -p/--patch passthrough proposed in rtk-ai#2951.

Fixes rtk-ai#2882
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@KuSh KuSh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @georgyia, LGTM! Thanks for your work.
Just one thing: you'll have to sign the CLA before we can merge this PR.

Comment thread src/cmds/git/git.rs
None => continue,
};
// Remaining lines are the body — keep up to 3 non-empty, non-trailer lines
let all_body_lines: Vec<&str> = lines

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: diffstat lines from git log --stat get mixed into the same untagged all_body_lines list as real commit-message body text, and they’re truncated with the default width of 80 chars.

For a large commit and/or a deeply nested path, for example src/cmds/git/git.rs | 123 ++++++++++++++++++++++++++++++++++++++++++++--------, which is 83 chars, the line gets cut with a trailing .... The insertion/deletion counts are then silently lost, and there’s no indication that the truncated line was diffstat data rather than commit-message prose.

This seems like a simple fix that could fit in that PR

@KuSh KuSh self-assigned this Aug 14, 2026
@pszymkowiak

Copy link
Copy Markdown
Collaborator

Closing: already fixed on develop. git log --stat, --numstat and the other diff-shape forms now request raw passthrough since ca89767 (#3575, released in v0.46.0), so nothing is dropped. Verified: rtk git log -50 --stat on this repo yields 50 of 50 commits, byte-identical to native git, and a --no-ff merge between diffstat commits is kept. The --no-merges injection on the compact forms is a separate issue, tracked in #1853. Thanks for the PR.

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.

rtk git log --stat silently drops ~36% of commits (no truncation notice, unlike git diff)

4 participants