Skip to content

signals: exempt generated/vendored/minified files from both test-evidence code-side checks #9696

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

The canonical "does this diff have code that needs tests" predicate lives in buildMissingTestEvidenceFindingpackages/loopover-engine/src/signals/slop.ts:285:

const codePaths = changedPaths.filter((path) => isCodeFile(path) && !PADDING_CATEGORIES.has(classifyChangedFile(path)));
if (codePaths.length === 0) return null;

with PADDING_CATEGORIES = new Set(["minified", "generated", "vendored"]) (slop.ts:111), because "generated/vendored/minified output carries source-file extensions (e.g. protoc's .pb.go stubs) and so passes the plain isCodeFile check, but nobody hand-writes tests for mechanically regenerated code."

Two consumers claim to mirror it and do not.

(a) False-positive positive signal. src/signals/improvement.ts:163-165 defines hasCodeFileToEvaluate as (changedFiles ?? []).some((file) => Boolean(file.path) && isCodeFile(file.path)), used at :226 to disambiguate the null from buildMissingTestEvidenceFinding per the doc at :221-224. With changedFiles = [{ path: "api/service.pb.go", additions: 500, deletions: 0 }] and no tests: isCodeFile is true (.go is a source extension), so the guard passes; slop's codePaths is empty (classifyChangedFile("api/service.pb.go") === "generated"), so buildMissingTestEvidenceFinding returns null; buildAddedTestEvidenceFinding therefore emits code: "added_test_evidence", detail: "Code changes are accompanied by test evidence." for a PR containing zero tests. improvementScore becomes 10 and hasApplicableSignal true, so the band is "minor" instead of "insufficient-signal". Same for vendor/**/*.go and dist/app.min.js.

(b) False-positive negative signal, with the parity claim written in the comment. src/signals/contributor-open-pr-monitor.ts:266-271 filters codeFiles with bare isCodeFile under a comment claiming it "Mirrors the isCodeFile code-side used by slop.ts's buildMissingTestEvidenceFinding". A PR touching only vendor/x/y.go with no test files yields codeFiles.length > 0 && testFiles.length === 0 -> missingTests: true -> classification "missing_tests", and the contributor is told to add tests for vendored code, while the gate-side signal deliberately exempts exactly that case.

Requirements

  • Export the padding-category exemption from @loopover/engine as a single reusable predicate — export function isTestableCodePath(path: string): boolean returning isCodeFile(path) && !PADDING_CATEGORIES.has(classifyChangedFile(path)) — in packages/loopover-engine/src/signals/slop.ts, and make buildMissingTestEvidenceFinding's filter at slop.ts:285 call it so there is exactly one implementation.
  • Re-export it from the existing shim src/signals/slop.ts alongside the symbols already re-exported there.
  • hasCodeFileToEvaluate (src/signals/improvement.ts:163-165) uses isTestableCodePath instead of bare isCodeFile.
  • missingTestsFromFiles (src/signals/contributor-open-pr-monitor.ts:266-271) uses isTestableCodePath for its codeFiles filter, and its comment is corrected to describe the real predicate.
  • isTestPath / isTestFile usage on the test side of both functions is unchanged.
  • scripts/check-engine-parity.ts must continue to recognise src/signals/slop.ts as a pure shim — add only a re-export line, no logic.

⚠️ Required pattern: buildMissingTestEvidenceFinding at packages/loopover-engine/src/signals/slop.ts:278-286 is the definition of correctness here, and src/signals/slop.ts / src/signals/path-matchers.ts are the established re-export-shim pattern for reaching engine code from src/. It does NOT satisfy this issue to hand-copy the !PADDING_CATEGORIES.has(classifyChangedFile(path)) expression into each of the two consumers (that is a third and fourth hand-written copy of the rule this issue exists to unify); to fix only improvement.ts and leave contributor-open-pr-monitor.ts's parity comment false; or to change PADDING_CATEGORIES membership.

Deliverables

  • isTestableCodePath is exported from packages/loopover-engine/src/signals/slop.ts, re-exported from src/signals/slop.ts, and is the only place the isCodeFile && !PADDING_CATEGORIES expression appears (grep-verifiable).
  • Named regression test: buildAddedTestEvidenceFinding({ changedFiles: [{ path: "api/service.pb.go", additions: 500, deletions: 0 }] }) with no test evidence returns null (previously it returned an added_test_evidence finding).
  • The same input plus a hand-written src/service.go still returns the positive finding when test evidence is present, and null when it is absent — both arms asserted.
  • Named regression test: missingTestsFromFiles([{ path: "vendor/x/y.go" }]) returns false, while missingTestsFromFiles([{ path: "src/x/y.go" }]) still returns true.
  • The parity comment at src/signals/contributor-open-pr-monitor.ts:266-268 names the real predicate.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example fixing improvement.ts and its tests while leaving contributor-open-pr-monitor.ts on bare isCodeFile — does not resolve this issue.

Test Coverage Requirements

99%+ Codecov patch coverage, branch-counted, on src/**. src/signals/improvement.ts and src/signals/contributor-open-pr-monitor.ts are inside coverage.include. packages/loopover-engine/src/** is ALSO inside coverage.include (it carries its own engine Codecov flag), so the new engine predicate is gated too and requires its own unit test covering a generated path, a vendored path, a minified path, and an ordinary source path. Both arms of both changed consumer conditionals need a test.

Expected Outcome

A protoc-, vendor-, or bundler-only diff stops being credited with "added test evidence" it does not have, and stops being told to add tests for machine-generated files. The one definition of "code that needs tests" lives in the engine and all three consumers read it.

Links & Resources

packages/loopover-engine/src/signals/slop.ts:105-115, :278-295; src/signals/improvement.ts:163-165, :219-243; src/signals/contributor-open-pr-monitor.ts:265-272; src/signals/path-matchers.ts, src/signals/test-evidence.ts.

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