Skip to content

fix(orb): merge-train.ts's hand-rolled lockfile/vendor-directory lists have drifted from the canonical path-matchers set #8647

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

src/review/merge-train.ts:52-53 hand-rolls its own low-signal file/directory detection instead of
reusing the canonical, already-existing pattern set:

  • LOW_SIGNAL_FILENAME_RE matches only package-lock.json|yarn.lock|pnpm-lock.yaml|Cargo.lock — 4
    names, versus the canonical LOCKFILE_NAMES set
    (packages/loopover-engine/src/signals/path-matchers.ts:146-171, 24+ entries including
    npm-shrinkwrap.json, bun.lock/bun.lockb, poetry.lock, Pipfile.lock, composer.lock,
    Gemfile.lock, go.sum, uv.lock, deno.lock, pubspec.lock, mix.lock, pdm.lock, and more.
  • LOW_SIGNAL_DIR_RE matches only dist|build|coverage|node_modules, missing out and the entire
    vendored-code family (vendor|vendored|third_party|bower_components|jspm_packages).

Two sibling files in the same codebase, src/review/review-diff.ts and
src/review/review-grounding.ts, already delegate lockfile-name matching to the canonical
isLockfile() helper specifically to avoid this drift. merge-train.ts is the one holdout — its
own comment references "generated-file classification" but never imports the shared helper, out of
an apparent (mistaken) concern that doing so would pull in a whole review-pipeline module;
isLockfile is in fact a leaf utility with no such dependency.

Consequence: two PRs that both touch, for example, poetry.lock, go.sum, or a file under
third_party/ are treated by merge-train.ts as a real, meaningful overlap, and one PR is forced to
wait in the merge train behind the other — exactly the "routine noise" this module's own stated
purpose says lockfile/vendored touches should never cause. test/unit/merge-train.test.ts:130-132
only covers package-lock.json and dist/, confirming the gap is untested.

Requirements

  • Replace merge-train.ts's hand-rolled LOW_SIGNAL_FILENAME_RE with a check against the canonical
    isLockfile() helper (or the LOCKFILE_NAMES set it wraps) from
    packages/loopover-engine/src/signals/path-matchers.ts, matching the pattern already used by
    review-diff.ts / review-grounding.ts.
  • Extend LOW_SIGNAL_DIR_RE to include out and the vendored-code family (vendor, vendored,
    third_party, bower_components, jspm_packages), matching the canonical directory-classification
    pattern used elsewhere in the codebase for the same purpose.

Deliverables

  • merge-train.ts no longer hand-rolls its own lockfile-name regex; it uses the canonical
    isLockfile() helper.
  • merge-train.ts's low-signal directory detection includes out and the vendored-code family.
  • A new test in test/unit/merge-train.test.ts proving two PRs sharing only poetry.lock (or
    go.sum) decide {wait: false}, where today they decide {wait: true}.
  • A new test proving two PRs sharing only a file under vendor/ (or third_party/) decide
    {wait: false}.

All four Deliverables are required in the same PR.

Test Coverage Requirements

src/** is measured by codecov/patch (99%+ target, branch-counted). The two new tests must
exercise the previously-untested lockfile names and vendored-directory paths directly, not just
re-assert the already-covered package-lock.json/dist/ cases.

Expected Outcome

merge-train.ts's low-signal classification matches the canonical, already-correct pattern used by
its sibling files, so PRs touching any of the 24+ canonical lockfile formats or any vendored-code
directory are no longer forced into a spurious merge-train wait.

Links & Resources

  • src/review/merge-train.ts:52-53 (the file to fix)
  • packages/loopover-engine/src/signals/path-matchers.ts:146-171 (LOCKFILE_NAMES, isLockfile)
  • src/review/review-diff.ts, src/review/review-grounding.ts (the already-correct sibling pattern
    to mirror)
  • test/unit/merge-train.test.ts:130-132 (existing test file to extend)

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