Context
src/review/review-diff.ts, diffFilePriority (lines 23-29), line 24 hand-rolls a large regex literal listing every recognized lockfile filename (package-lock\.json|npm-shrinkwrap\.json|pnpm-lock\.yaml|yarn\.lock|... — 30 alternatives) to rank lockfiles at the lowest review priority (tier 4, dropped first when a PR's diff exceeds DEFAULT_DIFF_BUDGET).
This file's own header comment (lines 17-22) explicitly documents that its test-file detection was previously fixed to delegate to the canonical isTestPath matcher specifically "so this matcher can't drift" from the shared source of truth — listing several real conventions (pytest test_*.py, Go *_test.go, Ruby *_spec.rb, Cypress/Playwright) the old inline regex used to miss, misclassifying those tests as tier-0 "source" and letting them displace real source under budget pressure ("the exact opposite of this function's job").
The lockfile detection got no equivalent treatment: packages/loopover-engine/src/signals/path-matchers.ts already exports a canonical isLockfile(path) function backed by an exported LOCKFILE_NAMES Set — currently byte-for-byte identical in content to review-diff.ts's inline regex (confirmed by direct comparison of both lists). diffFilePriority doesn't use it; it maintains its own independent, hand-duplicated copy. Any future lockfile-ecosystem addition to the canonical LOCKFILE_NAMES set (this has happened before — e.g. cartfile.resolved/gopkg.lock/shard.lock/rebar.lock/renv.lock/chart.lock were added together as a batch, per that file's own comment) will silently NOT be reflected in diffFilePriority's ranking until someone remembers to hand-update this second copy — the new format would be misclassified as tier-0 "source code" (highest priority, survives budget trimming first) instead of tier-4 "generated" (dropped first first), which is exactly the drift class this file's own header already fixed once for test-file detection. Issue #7526 (closed) is a prior real instance of exactly this class of gap in this same function (a vendored-directory pattern miss).
Requirements
- Replace the inline lockfile-name regex alternation on
review-diff.ts line 24 with a call to the canonical isLockfile from packages/loopover-engine/src/signals/path-matchers.ts (imported the same way this file already imports isTestPath from ../signals/test-evidence at line 9 — via the appropriate src/signals/* re-export shim if one exists for isLockfile, or directly from the engine package path, matching this repo's existing engine-consumption convention documented in test-evidence.ts's header).
- The
.min.(js|css)|.map|.snap suffix-based generated-file matching currently combined into the same regex alternation must be preserved — only the lockfile-name portion is being delegated to the canonical matcher, not the whole line's behavior.
- Verify (and note in the PR description) that no existing test in
test/unit/review-diff.test.ts relies on a lockfile filename that isLockfile/LOCKFILE_NAMES does NOT already recognize — if one is found, flag it explicitly as a pre-existing behavior change rather than silently absorbing it.
Deliverables
Test Coverage Requirements
99%+ Codecov patch coverage, branch-counted, on every changed line/branch in review-diff.ts, per codecov.yml.
Expected Outcome
diffFilePriority's lockfile-tier classification can never drift from the canonical LOCKFILE_NAMES set again — a future ecosystem addition to the canonical set is automatically reflected here with no second hand-edit required, closing the same class of drift risk this file already fixed for test-file detection.
Links & Resources
Context
src/review/review-diff.ts,diffFilePriority(lines 23-29), line 24 hand-rolls a large regex literal listing every recognized lockfile filename (package-lock\.json|npm-shrinkwrap\.json|pnpm-lock\.yaml|yarn\.lock|...— 30 alternatives) to rank lockfiles at the lowest review priority (tier 4, dropped first when a PR's diff exceedsDEFAULT_DIFF_BUDGET).This file's own header comment (lines 17-22) explicitly documents that its test-file detection was previously fixed to delegate to the canonical
isTestPathmatcher specifically "so this matcher can't drift" from the shared source of truth — listing several real conventions (pytesttest_*.py, Go*_test.go, Ruby*_spec.rb, Cypress/Playwright) the old inline regex used to miss, misclassifying those tests as tier-0 "source" and letting them displace real source under budget pressure ("the exact opposite of this function's job").The lockfile detection got no equivalent treatment:
packages/loopover-engine/src/signals/path-matchers.tsalready exports a canonicalisLockfile(path)function backed by an exportedLOCKFILE_NAMESSet— currently byte-for-byte identical in content toreview-diff.ts's inline regex (confirmed by direct comparison of both lists).diffFilePrioritydoesn't use it; it maintains its own independent, hand-duplicated copy. Any future lockfile-ecosystem addition to the canonicalLOCKFILE_NAMESset (this has happened before — e.g.cartfile.resolved/gopkg.lock/shard.lock/rebar.lock/renv.lock/chart.lockwere added together as a batch, per that file's own comment) will silently NOT be reflected indiffFilePriority's ranking until someone remembers to hand-update this second copy — the new format would be misclassified as tier-0 "source code" (highest priority, survives budget trimming first) instead of tier-4 "generated" (dropped first first), which is exactly the drift class this file's own header already fixed once for test-file detection. Issue #7526 (closed) is a prior real instance of exactly this class of gap in this same function (a vendored-directory pattern miss).Requirements
review-diff.tsline 24 with a call to the canonicalisLockfilefrompackages/loopover-engine/src/signals/path-matchers.ts(imported the same way this file already importsisTestPathfrom../signals/test-evidenceat line 9 — via the appropriatesrc/signals/*re-export shim if one exists forisLockfile, or directly from the engine package path, matching this repo's existing engine-consumption convention documented intest-evidence.ts's header)..min.(js|css)|.map|.snapsuffix-based generated-file matching currently combined into the same regex alternation must be preserved — only the lockfile-name portion is being delegated to the canonical matcher, not the whole line's behavior.test/unit/review-diff.test.tsrelies on a lockfile filename thatisLockfile/LOCKFILE_NAMESdoes NOT already recognize — if one is found, flag it explicitly as a pre-existing behavior change rather than silently absorbing it.Deliverables
review-diff.ts'sdiffFilePrioritydelegates lockfile-name matching to the canonicalisLockfile(orLOCKFILE_NAMES) instead of a hand-duplicated regex list.test/unit/review-diff.test.ts's existing lockfile-priority test cases still pass unchanged.Test Coverage Requirements
99%+ Codecov patch coverage, branch-counted, on every changed line/branch in
review-diff.ts, percodecov.yml.Expected Outcome
diffFilePriority's lockfile-tier classification can never drift from the canonicalLOCKFILE_NAMESset again — a future ecosystem addition to the canonical set is automatically reflected here with no second hand-edit required, closing the same class of drift risk this file already fixed for test-file detection.Links & Resources
src/review/review-diff.ts(diffFilePriority)packages/loopover-engine/src/signals/path-matchers.ts(isLockfile,LOCKFILE_NAMES)src/signals/path-matchers.ts(the repo-side re-export shim)diffFilePrioritydrift/gap fix for vendored-directory patterns.