Skip to content

fix(engine): diffFilePriority's vendored-directory pattern misses vendored/third_party/third-party/bower_components/jspm_packages #7526

Description

@JSONbored

Context

if (/(^|\/)(dist|build|out|coverage|vendor|node_modules)\//i.test(path)) return 4;

This is diffFilePriority's priority-4 ("least-useful-to-review, never real collision evidence") bucket.
Both of its call sites document that bucket identically:

// packages/loopover-engine/src/signals/engine.ts:5238-5239
// packages/loopover-engine/src/signals/predicted-gate-engine.ts:976-977
/** True when two changed-file lists share at least one path that isn't a lockfile/generated/vendor artifact
 *  (diffFilePriority's least-useful-to-review bucket) — a shared package-lock.json or dist/ output is touched
 *  incidentally by unrelated PRs and is not evidence of a real collision. */
function sharesMeaningfulFile(left: string[] | undefined, right: string[] | undefined): boolean {
  ...
  return left.some((path) => rightSet.has(path) && diffFilePriority(path) < 4);
}

But the regex only matches vendor (singular) as a directory name — it misses vendored, third_party,
third-party, bower_components, and jspm_packages, which is exactly the vendored-directory set the
sibling matcher in this same codebase already recognizes:

// packages/loopover-engine/src/signals/path-matchers.ts:111-116
function isVendoredFileFrom(parts: NormalizedPath): boolean {
  return /(^|\/)(vendor|vendored|third_party|third-party|node_modules|bower_components|jspm_packages)\//.test(
    parts.norm,
  );
}

This isn't a case of the two drifting apart over time — path-matchers.ts already had
vendor|vendored|third_party|third-party|node_modules at its original creation (#752/#1045), before
diff-file-priority.ts was added at all (#3882), and bower_components/jspm_packages were added to
path-matchers.ts later (#2777, "classify bower_components and jspm_packages as vendored") with no
corresponding update to diff-file-priority.ts. Practical effect: a file shared between two unrelated
PRs under e.g. third_party/foo.js, vendored/bar.js, or bower_components/baz.js gets priority 0
("ordinary source") instead of 4, so sharesMeaningfulFile returns true for it — inflating the
collision/duplicate-cluster signal on exactly the kind of vendored-artifact path both call sites'
own doc comments say should be excluded, the same way a shared vendor/-directory file already is.

Requirements

  • Extend diffFilePriority's directory-name alternation to match path-matchers.ts's isVendoredFileFrom
    set exactly: vendor, vendored, third_party, third-party, bower_components, jspm_packages
    in addition to the existing dist, build, out, coverage, node_modules.
  • Do not otherwise change diffFilePriority's lockfile/doc/test branches — this issue is scoped to the
    directory-name alternation only.
  • Add a code comment on the merged pattern noting it must stay in sync with path-matchers.ts's
    isVendoredFileFrom (the two already had this obligation implicitly; make it explicit so a future
    addition to one doesn't silently skip the other again).

Deliverables

  • diffFilePriority's directory regex includes all of vendor|vendored|third_party|third-party|dist|build|out|coverage|node_modules|bower_components|jspm_packages.
  • A code comment cross-referencing path-matchers.ts's isVendoredFileFrom as the source of truth to keep in sync.

Test Coverage Requirements

99%+ Codecov patch gate on src/**. Add test cases in diffFilePriority's own test file asserting
priority 4 for a path under each newly-added directory name (third_party/x.js, third-party/x.js,
vendored/x.js, bower_components/x.js, jspm_packages/x.js), plus a regression test on
sharesMeaningfulFile (in whichever of signals/engine.ts/signals/predicted-gate-engine.ts's test
suites already covers it) confirming a file shared only under one of these paths no longer counts as
meaningful evidence of a collision.

Expected Outcome

A file shared between two PRs under any of the six vendored-directory names now falls into the same
priority-4 bucket a vendor/-directory file already does, so it can never inflate
collision/duplicate-cluster evidence in either signals/engine.ts or signals/predicted-gate-engine.ts.

Links & Resources

  • packages/loopover-engine/src/signals/path-matchers.tsisVendoredFileFrom, the sibling to converge onto.
  • packages/loopover-engine/src/signals/engine.ts and packages/loopover-engine/src/signals/predicted-gate-engine.ts — both call sites' sharesMeaningfulFile.

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions