Skip to content

Commit e407ad3

Browse files
committed
docs(eager-closure): state deletedSourcesSince's real rename semantics
Approval-round note on PR #3298: plain `git diff` pairs renames by DEFAULT, and --diff-filter=D drops the paired ones, so the docstring's "WITHOUT -M, so a rename the similarity detector would pair stays visible" had it backwards -- the list covers only deletions pairing did NOT absorb, and paired renames are answered by renamedSince. Captured the maintainer's companion constraint too: dropping the default pairing (--no-renames) would put a re-homed hub in the deleted set and refuse the very splits the tolerance exists for. Fixed at the three siblings carrying the same claim: the function docstring, the rule's deletedSourceTexts field doc, and the gate's cost comment.
1 parent 0749922 commit e407ad3

4 files changed

Lines changed: 17 additions & 10 deletions

File tree

‎docs/adr/0027-descriptor-root-vs-eager-closure-budget.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ shippable, so an oversized hub is addressable debt rather than accepted state.
155155
statement texts close it down to a stated threshold: two identical normalized statements from
156156
one deleted source is the move signal, one is tolerated as coincidence (short constants repeat
157157
across the repo), and a transplant rewritten statement-by-statement stays outside a textual
158-
probe — what remains heuristic-free is the LIST of deleted sources (an `-M`-less diff), not the
158+
probe — what remains heuristic-free is the LIST of deleted sources (git's own delete listing,
159+
needing no similarity pairing to have been attempted), not the
159160
content match.
160161
- **Weight alone (#2469 candidate 1):** refuted by the planted drop test — a swap can keep the
161162
statement total flat while quietly making the closure evaluate different modules.

‎scripts/__tests__/closure-growth-rule.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,11 @@ export function closureGrowthEvidence(params: {
8080
renamedBaseToHead?: ReadonlyMap<string, string>;
8181
/**
8282
* Weight-bearing statement texts of each merge-base production source DELETED since the
83-
* merge-base (`deletedSourcesSince`, which deliberately skips `-M`, mapped through
84-
* `statementTextsOf`). Git's rename detection is a similarity heuristic and a rewritten move
85-
* evades it, showing up as delete + brand-new path; content is not a heuristic, so an added
86-
* module carrying a deleted source's code is a move of pre-existing code, not a new file.
83+
* merge-base (`deletedSourcesSince`, which lists only the deletions git's DEFAULT rename
84+
* pairing did not absorb, mapped through `statementTextsOf`). Git's rename detection is a
85+
* similarity heuristic and a rewritten move evades it, showing up as delete + brand-new path;
86+
* content is not a heuristic, so an added module carrying a deleted source's code is a move of
87+
* pre-existing code, not a new file.
8788
*/
8889
deletedSourceTexts?: readonly (readonly string[])[];
8990
}): ClosureGrowthEvidence {

‎scripts/__tests__/committed-source-tree.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,14 @@ export function renamedSince(repoRoot: string, base: string): ReadonlyMap<string
5555
}
5656

5757
/**
58-
* Production source paths tracked at `base` but gone from the working tree, WITHOUT `-M`, so a
59-
* rename the similarity detector would pair stays visible here too. `renamedSince` answers by
60-
* detection and misses a moved-and-rewritten module; the split tolerance reads these paths'
61-
* content directly so such a move stays non-novel (#3298 review).
58+
* Production source paths tracked at `base` but gone from the working tree. Plain `git diff`
59+
* already pairs renames by default, so this lists only the deletions that pairing did NOT
60+
* absorb -- a moved-and-rewritten module the detector could not pair, which `renamedSince`
61+
* answers by detection and therefore misses; the split tolerance reads these paths' content
62+
* directly so such a move stays non-novel (#3298 review). The default pairing is load-bearing:
63+
* it keeps a split's re-homed hub OUT of this list (paired to its closest part), and passing
64+
* `--no-renames` would put it here, making the split's own parts share two statements with
65+
* "deleted source" and refusing the very extraction the tolerance exists for.
6266
*/
6367
export function deletedSourcesSince(repoRoot: string, base: string): string[] {
6468
const status = git(repoRoot, ['diff', '--name-only', '--diff-filter=D', '-z', base]);

‎scripts/__tests__/eager-closure-budgets.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ import {
6363
* - Cost: one unit-lane test file plus two data/reader modules; five git processes for the
6464
* merge-base side (merge-base, ls-tree, one cat-file batch for the whole base tree — which is
6565
* also where the novelty check reads a deleted source's statements — one rename diff, and one
66-
* `-M`-less delete diff listing the sources the rename diff cannot pair), no device. The
66+
* default-pairing delete diff listing only the deletions rename detection did not absorb),
67+
* no device. The
6768
* walker memoizes per-file edges per tree and parses each file once per distinct content, so
6869
* the base tree pays only for the files the branch changed.
6970
* - Kill criterion: if two consecutive quarters show no rule ever firing, or

0 commit comments

Comments
 (0)