Skip to content

feat(repo stats): default to walking all commits (with patch-id dedup)#41

Merged
kyle-rader merged 2 commits into
kyle-rader:mainfrom
kyle-rader-msft:user/kyrader/repo-stats-default-all
May 23, 2026
Merged

feat(repo stats): default to walking all commits (with patch-id dedup)#41
kyle-rader merged 2 commits into
kyle-rader:mainfrom
kyle-rader-msft:user/kyrader/repo-stats-default-all

Conversation

@kyle-rader-msft
Copy link
Copy Markdown
Contributor

@kyle-rader-msft kyle-rader-msft commented May 23, 2026

Problem

In v2.0.0 (Feb 2, 2026) we flipped the default commit walk in lk repo stats to --first-parent as a workaround for inflated contributor counts after merging a large external history into a downstream repo. That workaround over-corrected: it silently drops all authorship from history that was merged in via a side-parent — i.e. it hides every commit behind a graft merge from the contributor view.

Now that patch-id dedup (v2.5.0, PR #40) handles the duplicate-commit inflation properly, the --first-parent default is no longer needed and is actively wrong: it suppresses real contributions.

Case study (same repo as v2.5.0)

Mode Total commits Top entry for the migration driver
v2.0.x-v2.5.x default (--first-parent) 3,288 131 commits over 6.1 months ❌ drops 14+ months of work
v2.6.0 default (all + dedup) 4,476 (838 collapsed) 413 commits over 1.2 years ✅ matches reality
--no-dedup for reference 5,314 704 commits over 1.2 years ❌ inflated by ~290 dup patches

(The same person's oldest commit reachable from HEAD via --first-parent is 2025-11-17; via all refs it's 2025-03-10. The first-parent walk hides 8 months of legitimate authorship.)

Solution

  • Drop the old --all opt-in flag (it became meaningless once "all" is the default).
  • Add a new --first-parent opt-in flag for users who want the one-tally-per-merge release-history view.
  • Update the help text and README to reflect the new model: default = all commits with patch-id dedup; opt into --first-parent when you want PR-granularity.

Compat

Defaults change. Anyone scripted on the old default gets the (more accurate) all-commits view; they can restore previous output with --first-parent. The old default existed for ~3.5 months. Anyone scripted on lk repo stats --all should drop the flag — it now errors.

Bumps the crate version 2.5.0 → 2.6.0.

Flip the default commit walk from `--first-parent` back to all commits.
The `--first-parent` default was introduced as a workaround for inflated
contributor counts after the cross-repo-history merge in a downstream
Microsoft repo, but the workaround over-corrected — it silently dropped
all authorship from history that was merged in via a side-parent.

Now that patch-id dedup (v2.5.0) handles duplicate-commit inflation
properly, restoring the all-commits default gives a much more accurate
picture of who actually contributed to a repo.

Concrete impact in the case-study repo:

  Before this PR (default = --first-parent):
    Kyle Rader: 131 commits over 6.1 months  (drops 14+ months of work)

  After this PR (default = all + dedup):
    Kyle Rader: 413 commits over 1.2 years   (matches reality)

  --no-dedup (raw SHA counts, for reference):
    Kyle Rader: 704 commits over 1.2 years   (inflated by ~290 dup patches)

The `--first-parent` behavior remains available as an opt-in flag for
users who want one-tally-per-PR (e.g. mainline-merge release-history view).

The deprecated `--all` flag is kept as a hidden no-op so existing scripts
and muscle memory don't break — walking all commits is now the default.

Bumps the crate version 2.5.0 -> 2.6.0 (backward-compatible default flip).
Since we're introducing --first-parent as the opt-in toggle in the same
release, just drop --all entirely rather than carrying a no-op
deprecation shim.
@kyle-rader kyle-rader merged commit 732d9f2 into kyle-rader:main May 23, 2026
3 checks passed
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.

2 participants