chore(review): delegate diffFilePriority's lockfile match to the canonical isLockfile

packages/loopover-engine/src/review/diff-file-priority.ts

@@ -1,7 +1,10 @@
+import { isLockfile } from "../signals/path-matchers.js";
 import { isTestPath } from "../signals/test-evidence.js";
 
 export function diffFilePriority(path: string): number {
-  if (/(^|\/)(package-lock\.json|npm-shrinkwrap\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lock|bun\.lockb|cargo\.lock|poetry\.lock|pipfile\.lock|composer\.lock|gemfile\.lock|go\.sum|go\.work\.sum|uv\.lock|packages\.lock\.json|flake\.lock|deno\.lock|pubspec\.lock|podfile\.lock|mix\.lock|package\.resolved|gradle\.lockfile|pdm\.lock|conan\.lock|pixi\.lock|cartfile\.resolved|gopkg\.lock|shard\.lock|rebar\.lock|renv\.lock|chart\.lock)$|\.(min\.(js|css)|map|snap)$/i.test(path)) return 4;
+  // Lockfile-NAME matching delegates to the canonical isLockfile/LOCKFILE_NAMES so no copy of this
+  // function can drift from the shared set (the #4605 Finding 1 class); suffix patterns stay inline.
+  if (isLockfile(path) || /\.(min\.(js|css)|map|snap)$/i.test(path)) return 4;
   // Must stay in sync with signals/path-matchers.ts's isVendoredFileFrom -- the two already had this
   // obligation implicitly (bower_components/jspm_packages were added there in #2777 with no corresponding
   // update here, #7526) and now match the same directory-name set exactly.

scripts/check-engine-parity.ts

@@ -57,9 +57,12 @@ export const SAFE_URL_MARKERS = Object.freeze([
 
 /** `diffFilePriority` is duplicated by FUNCTION, not by file: two byte-identical host copies
  *  (review-diff.ts, review-grounding.ts) and a differently-named engine copy (diff-file-priority.ts) —
- *  none share a filename, so the directory scan never pairs them. The `cartfile\.resolved` marker directly
- *  regression-guards #4605 Finding 1: the engine copy's Carthage-lockfile regex had silently drifted to
- *  `cartfile\.lock`, which is not a real filename (Carthage's lockfile is `Cartfile.resolved`). */
+ *  none share a filename, so the directory scan never pairs them. The `isLockfile(path)` marker
+ *  regression-guards #4605 Finding 1 at its root: that bug was the engine copy's hand-rolled
+ *  Carthage-lockfile regex silently drifting to `cartfile\.lock` (not a real filename — Carthage's is
+ *  `Cartfile.resolved`). Since #8357 every copy delegates lockfile-NAME matching to the canonical
+ *  `isLockfile`/`LOCKFILE_NAMES`, so no copy owns a name list that CAN drift; asserting the delegation is
+ *  present is therefore a strictly stronger guard than asserting one literal name inside a private regex. */
 export const DIFF_FILE_PRIORITY_TWIN_PAIR: NamedTwinPair = Object.freeze({
   area: "diff-file-priority",
   hostRelative: "src/review/review-diff.ts",
@@ -70,7 +73,7 @@ export const DIFF_FILE_PRIORITY_TWIN_PAIR: NamedTwinPair = Object.freeze({
 
 export const DIFF_FILE_PRIORITY_MARKERS = Object.freeze([
   "export function diffFilePriority(path: string): number {",
-  "cartfile\\.resolved",
+  "isLockfile(path)",
 ] as const);
 
 /** `sharesMeaningfulFile` is a near-duplicate helper (the host folds its guard clause into one `if`; the

src/review/review-diff.ts

@@ -6,6 +6,7 @@
 // which survived even with full-file grounding on). This builder orders source-first, reduces oversized
 // patches hunk-aware instead of dropping them, and always lists patch-less/over-budget files. (#accuracy-gap-1)
 
+import { isLockfile } from "../signals/path-matchers";
 import { isTestPath } from "../signals/test-evidence";
 import type { listPullRequestFiles } from "../db/repositories";
 
@@ -19,9 +20,13 @@ export const DEFAULT_DIFF_BUDGET = 80_000;
  *  Test detection delegates to the canonical `isTestPath` so this matcher can't drift from it — the
  *  previous inline regex missed real conventions (pytest `test_*.py`, Go `*_test.go`, Ruby `*_spec.rb`,
  *  Cypress/Playwright `.cy`/`.e2e`, a bare `spec/` dir), so those tests were ranked as SOURCE(0) and
- *  could displace real source under a tight budget — the exact opposite of this function's job. */
+ *  could displace real source under a tight budget — the exact opposite of this function's job.
+ *  Lockfile detection delegates to the canonical `isLockfile`/`LOCKFILE_NAMES` for the same reason: a
+ *  hand-duplicated name list silently misses every future ecosystem addition to the shared set (which has
+ *  been extended in batches before), ranking a new lockfile format as SOURCE(0) instead of GENERATED(4). */
 export function diffFilePriority(path: string): number {
-  if (/(^|\/)(package-lock\.json|npm-shrinkwrap\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lock|bun\.lockb|cargo\.lock|poetry\.lock|pipfile\.lock|composer\.lock|gemfile\.lock|go\.sum|go\.work\.sum|uv\.lock|packages\.lock\.json|flake\.lock|deno\.lock|pubspec\.lock|podfile\.lock|mix\.lock|package\.resolved|gradle\.lockfile|pdm\.lock|conan\.lock|pixi\.lock|cartfile\.resolved|gopkg\.lock|shard\.lock|rebar\.lock|renv\.lock|chart\.lock)$|\.(min\.(js|css)|map|snap)$/i.test(path)) return 4;
+  // Only the lockfile-NAME portion is delegated; the suffix-based generated-file patterns stay inline.
+  if (isLockfile(path) || /\.(min\.(js|css)|map|snap)$/i.test(path)) return 4;
   if (/(^|\/)(dist|build|out|coverage|vendor|node_modules)\//i.test(path)) return 4;
   if (/\.(md|mdx|markdown|rst|adoc|asciidoc|txt)$/i.test(path)) return 2;
   if (isTestPath(path)) return 1;

src/review/review-grounding.ts

@@ -22,6 +22,7 @@
 // review-grounding.ts -- if reviewbot's copy is ever resynced from this file, carry this piece forward too,
 // or reviewbot will regress to the old all-or-nothing truncation this was written to eliminate.
 
+import { isLockfile } from "../signals/path-matchers";
 import { isTestPath } from "../signals/test-evidence";
 import { neutralizePromptInjection } from "./prompt-injection";
 
@@ -189,7 +190,9 @@ export function buildGrounding(f: GroundingFlags, checks?: CheckAggregate, fileC
  *  copy missed pytest `test_*.py`, Go `*_test.go`, Ruby `*_spec.rb`, Cypress/Playwright `.cy`/`.e2e`, and a
  *  bare `spec/` dir — so those tests ranked as SOURCE(0) and were inlined ahead of real source). */
 export function diffFilePriority(path: string): number {
-  if (/(^|\/)(package-lock\.json|npm-shrinkwrap\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lock|bun\.lockb|cargo\.lock|poetry\.lock|pipfile\.lock|composer\.lock|gemfile\.lock|go\.sum|go\.work\.sum|uv\.lock|packages\.lock\.json|flake\.lock|deno\.lock|pubspec\.lock|podfile\.lock|mix\.lock|package\.resolved|gradle\.lockfile|pdm\.lock|conan\.lock|pixi\.lock|cartfile\.resolved|gopkg\.lock|shard\.lock|rebar\.lock|renv\.lock|chart\.lock)$|\.(min\.(js|css)|map|snap)$/i.test(path)) return 4;
+  // Lockfile-NAME matching delegates to the canonical isLockfile/LOCKFILE_NAMES so no copy of this
+  // function can drift from the shared set (the #4605 Finding 1 class); suffix patterns stay inline.
+  if (isLockfile(path) || /\.(min\.(js|css)|map|snap)$/i.test(path)) return 4;
   if (/(^|\/)(dist|build|out|coverage|vendor|node_modules)\//i.test(path)) return 4;
   if (/\.(md|mdx|markdown|rst|adoc|asciidoc|txt)$/i.test(path)) return 2;
   if (isTestPath(path)) return 1;

test/unit/check-engine-parity-script.test.ts

@@ -376,18 +376,19 @@ describe("check-engine-parity script", () => {
       }
     });
 
-    it("diffFilePriority markers cover the exact Cartfile.resolved regression (#4605 Finding 1)", () => {
-      // Reproduces the actual bug: the engine copy's regex previously matched `cartfile\.lock` (not a
-      // real Carthage filename) instead of `cartfile\.resolved`. A body missing the resolved-lockfile
-      // marker fails presence — exactly what the old, un-checked engine copy would have done.
+    it("diffFilePriority markers cover the Cartfile.resolved regression class at its root (#4605 Finding 1, #8357)", () => {
+      // Reproduces the actual bug's ROOT CAUSE: a copy that hand-rolls its own lockfile-name regex can
+      // silently drift (the engine copy once matched `cartfile\.lock`, not a real Carthage filename).
+      // Since #8357 every copy delegates to the canonical isLockfile, so a body that still owns a private
+      // name list fails presence — exactly what the old, un-checked engine copy would have done.
       const buggyEngineBody =
         "export function diffFilePriority(path: string): number {\n" +
         "  if (/cartfile\\.lock$/i.test(path)) return 4;\n" +
         "  return 0;\n" +
         "}\n";
       const fixedHostBody =
         "export function diffFilePriority(path: string): number {\n" +
-        "  if (/cartfile\\.resolved$/i.test(path)) return 4;\n" +
+        "  if (isLockfile(path)) return 4;\n" +
         "  return 0;\n" +
         "}\n";
       const result = checkGateDecisionTwinPresence({