silent-revert canary: git's default diff.renameLimit can decompose a large basename-changing relocation into a false positive

[kyle-sexton]

Deliberately excluded from #2843, which was scoped to pins only. -l0 is not a pin (see "Why this was excluded" below), so it is filed separately rather than smuggled in.

All figures below measured on git 2.55.0.windows.3.

The claim

attribute_file's header calls rename detection load-bearing, and it is right:

With --no-renames a git mv decomposes into a delete plus an add, the delete side reaches the enumeration below as a whole-file removal, and every line in a moved file gets attributed to whoever last touched it -- so relocating a large file a recent commit had added would fire.

-M was pinned in #2843 to stop a developer's diff.renames = false from causing exactly that. But -M does not make rename detection unconditional. Git applies its own default diff.renameLimit of 1000, and above it git silently stops pairing and reports the relocation as adds plus deletes — the precise decomposition the header says must never happen. No hostile developer config is required. Git's own default is enough.

Measured: -l0 means UNBOUNDED, not "zero/never"

Fixture: 1100 files relocated src/oldN.txt -> dst/newN.txt, each also having one line changed.

flags	result	stderr
-M (as shipped)	1100 A + 1100 D	warning: exhaustive rename detection was skipped due to too many files.
-M -l0	1100 R	none
-M -l5000	1100 R	none
-M -l1	1100 A + 1100 D	same warning

-l0 matches -l5000, not -l1. Git's own warning names the fix: "you may want to set your diff.renameLimit variable to at least 1100 and retry the command."

Canary consequence: a false positive with no hostile config

Same fixture, shipped threshold 200, shipped flags, empty git config:

SILENT REVERT SUSPECTED

  removed by   baea247c5  chore: relocate the corpus
  content from d9d1dd777  feat: land the corpus
               (1 commit(s) earlier on main)
  lines lost   11000  (threshold 200, window 40 commits)

Exit 1. With diff.renameLimit=0 or =5000 the same commit reports ok and exits 0.

This is a false positive reachable in CI on a clean machine. The relocated content is all present in the tree; the detector simply lost the pairing and blamed the old-side paths.

IMPORTANT QUALIFIERS — what actually reaches the limit

The exposure is much narrower than "relocate more than 1000 files". Git has two cheap pre-passes that pair files before the O(N^2) inexact pass, and diff.renameLimit gates neither of them. Both were measured.

1. A pure git mv with no content change never consults the limit. Byte-identical blobs pair in git's exact-rename pre-pass. 1100-file pure-move control fixture, default limit: 1100 R, no warning, canary exit 0. Even -l1 reports 1100 R.

2. A relocation that PRESERVES basenames never consults the limit either. This one is not in the original write-up and it materially narrows the finding. Git's basename-match optimization pairs src/fN.txt with dst/fN.txt cheaply, and that pass is not limit-gated. Measured:

fixture	-M (default)	-M -l1
1100 files, src/fN.txt -> dst/fN.txt, content touched	1100 R	1100 R
1100 files, src/oldN.txt -> dst/newN.txt, content touched	1100 A + 1100 D	1100 A + 1100 D

Confirmed again at 10 files with -l1: basenames preserved -> 10 R; basenames changed -> 10 A + 10 D plus the warning.

So the exposure is: relocate more than ~1000 files AND change their content AND change their basenames. A plain directory move — the common shape in this repo's skill and doc restructures — keeps basenames and is immune. A rename-and-relocate sweep is not. That is the shape a fix has to justify itself against.

Trip point

Measured for the shape src == dst == N, basename-changing and content-touched, with nothing else in the commit:

• N = 1000 -> 1000 R, no warning
• N = 1001 -> warning + 1001 A + 1001 D

Git's gate is a product, roughly (dst <= L || src <= L) && dst*src <= L*L, so a commit that also adds or deletes unrelated files moves the trip point. Do not treat 1001 as a fixed number.

Only scan_commit is affected — -M and any -l are INERT in attribute_file

attribute_file's diff is pathspec-limited to a single old-side file, so there is no destination candidate to pair with and rename detection has nothing to do. Verified by hashing the diff output for every MD-filtered path of two calibration commits (cc58cbc, 6 paths; 91e77fc, 55 paths) under -M, -M -l1 and -M -l0: byte-identical in all 61 cases.

A fix should therefore touch only the scan_commit enumeration site. Adding -l0 to attribute_file would be pure noise.

Why this was excluded from #2843 — state this explicitly

-l0 is not a pin, and the distinction is the whole reason this is a separate issue.

• --no-ext-diff, --no-textconv, --no-show-signature, --diff-algorithm=myers, -M, --no-ignore-revs-file all neutralize caller configuration. Each restores a git default. They change nothing about what the detector computes on a clean machine; they make a local run match CI.
• -l0 overrides git's own default and changes what the detector computes on every machine, CI included. It is a behavior change, not a normalization.
• It also removes a bound on git diff's O(N^2) rename cost — the exact bound git's default exists to impose.

#2843 was scoped to pins only, so this was correctly left out rather than quietly bundled.

Measured no-op on the current corpus

Adopting -l0 moves no calibration figure today. Paths presented to the enumeration diff per calibration commit:

commit	MD paths
f603880	10
9239f15	10
cc58cbc	6
6f0a311	4
91e77fc	55
c8470ef	12 (14 changed paths total: 12 M + 2 A)

Max 55 against a limit of 1000. Diff output was verified byte-identical with and without -l0 on all six commits. No commit in the calibration set contains a rename at all (--name-status shows only M, plus 2 A on c8470ef), so none of the 853 / 451 / 346 / 298 / 390 / 340 attributions move.

REQUIRED before adopting -l0: a runtime measurement

This is an acceptance condition, not a suggestion.

-l0 removes a cost bound on an O(N^2) algorithm, and the canary's full scan already takes about 9m21s (as reported by the lane that measured it). Whoever fixes this must measure the full-scan runtime with -l0 applied and record the before/after in the PR. If the scan time moves materially, -l5000 — a raised bound rather than no bound — is the safer landing and produces the identical 1100 R result on every fixture above.

Do not adopt -l0 on the strength of "it is a no-op on the current corpus". It is a no-op on paths; the runtime question is about what happens on the commit that finally isn't.

Related

Siblings from the same review pass: #2874, #2875, #2837, #2833, #2846, #2855, #2865, #2843, #2847, #2691, #2656. Companion recall-gap finding filed as #2883.

[kyle-sexton]

One follow-up measurement, and a cross-reference to #2880, which was filed on the adjacent stderr surface while this was being written.

The false positive announces itself. scan_commit's enumeration is the one git call in this script with no 2>/dev/null:

530:  done < <(git diff --name-only -z -M --diff-filter=MD "$parent" "$sha")

attribute_file's diff and blame both redirect stderr away (the subject of #2880); the enumeration does not. So git's own warning: exhaustive rename detection was skipped due to too many files. reaches the CI log, immediately above the SILENT REVERT SUSPECTED block — exactly as it did in the fixture run quoted in the body. CI invokes the script with no redirection of its own (.github/workflows/silent-revert-canary.yml).

That is worth knowing for triage in both directions:

• It bounds the urgency here. A reader of a real fire gets git telling them the pairing was skipped and naming the limit to raise, rather than a silent, inexplicable 11,000-line finding.
• It is a reason not to widen silent-revert canary: attribute_file swallows git stderr, so corpus finding counts are lower bounds #2880's stderr fix into this call site. Whatever silent-revert canary: attribute_file swallows git stderr, so corpus finding counts are lower bounds #2880 does about the two swallowed streams, the enumeration's stderr should stay visible — it is currently the only signal that distinguishes this false-positive shape from a genuine one.

[kyle-sexton]

Claimed by lane 2883+2884 (same script: scripts/check-silent-revert.sh). Pairing with #2883.

[kyle-sexton]

Claimed by lane 2883+2884 (same script: scripts/check-silent-revert.sh).