diff --git a/src/signals/path-matchers.ts b/src/signals/path-matchers.ts new file mode 100644 index 0000000000..15a384806a --- /dev/null +++ b/src/signals/path-matchers.ts @@ -0,0 +1,135 @@ +import { isCodeFile, isTestFile } from "./local-branch"; +import { isTestPath } from "./test-evidence"; + +// Pure, deterministic path matchers for slop classification (#561). Siblings to `isTestFile` / +// `isTestPath`: they identify changed files that are NOT genuine hand-authored effort — machine- +// generated output, vendored/imported third-party code, minified bundles, dependency lockfiles, and +// docs — so slop signals can tell a padded diff from real work. Path-only and side-effect-free. + +function normalize(path: string): string { + return String(path ?? "") + .replace(/\\/g, "/") + .toLowerCase(); +} + +function basename(path: string): string { + const norm = normalize(path); + const slash = norm.lastIndexOf("/"); + return slash >= 0 ? norm.slice(slash + 1) : norm; +} + +function extension(path: string): string { + const base = basename(path); + const dot = base.lastIndexOf("."); + return dot > 0 ? base.slice(dot + 1) : ""; +} + +const LOCKFILE_NAMES: ReadonlySet = new Set([ + "package-lock.json", + "npm-shrinkwrap.json", + "yarn.lock", + "pnpm-lock.yaml", + "bun.lockb", + "cargo.lock", + "poetry.lock", + "pipfile.lock", + "composer.lock", + "gemfile.lock", + "go.sum", + "uv.lock", + "packages.lock.json", + "flake.lock", +]); + +const DEPENDENCY_MANIFEST_NAMES: ReadonlySet = new Set([ + "package.json", + "cargo.toml", + "go.mod", + "requirements.txt", + "pyproject.toml", + "pipfile", + "gemfile", + "composer.json", + "build.gradle", + "build.gradle.kts", + "pom.xml", +]); + +const DOCS_EXTENSIONS: ReadonlySet = new Set(["md", "mdx", "markdown", "rst", "adoc", "asciidoc"]); + +/** Machine-generated output (codegen, protobuf, source maps, typegen). */ +export function isGeneratedFile(path: string): boolean { + const norm = normalize(path); + return ( + /(^|\/)(__generated__|generated)\//.test(norm) || + /\.(generated|gen)\.[^/]+$/.test(norm) || + /\.pb\.(go|ts|js)$/.test(norm) || + /_pb2\.pyi?$/.test(norm) || + /\.g\.dart$/.test(norm) || + /\.(js|jsx|ts|tsx|css)\.map$/.test(norm) || + basename(norm) === "worker-configuration.d.ts" + ); +} + +/** Third-party / imported code that lives in the repo but is not the contributor's work. */ +export function isVendoredFile(path: string): boolean { + return /(^|\/)(vendor|vendored|third_party|third-party|node_modules)\//.test(normalize(path)); +} + +/** Dependency lockfiles (resolved trees), e.g. `package-lock.json`, `go.sum`, `Cargo.lock`. */ +export function isLockfile(path: string): boolean { + return LOCKFILE_NAMES.has(basename(path)); +} + +/** Minified bundles, e.g. `app.min.js`, `styles.min.css`. */ +export function isMinifiedFile(path: string): boolean { + return /\.min\.[a-z0-9]+$/.test(normalize(path)); +} + +/** Documentation files (by extension or a top-level `docs/` directory). */ +export function isDocsFile(path: string): boolean { + const norm = normalize(path); + return /(^|\/)docs?\//.test(norm) || DOCS_EXTENSIONS.has(extension(norm)); +} + +/** Dependency manifests (declare dependencies), e.g. `package.json`, `go.mod`, `pyproject.toml`. */ +export function isDependencyManifestFile(path: string): boolean { + return DEPENDENCY_MANIFEST_NAMES.has(basename(path)); +} + +/** + * Files that masquerade as substantive source/work but are machine-produced or imported — the set a + * padded diff inflates its size with. Lockfiles, dependency manifests, and docs are legitimate change + * categories and are deliberately excluded here (they have their own matchers for reuse). + */ +export function isNonSubstantivePaddingFile(path: string): boolean { + return isMinifiedFile(path) || isGeneratedFile(path) || isVendoredFile(path); +} + +export type ChangedFileCategory = + | "minified" + | "generated" + | "vendored" + | "lockfile" + | "dependency_manifest" + | "test" + | "docs" + | "source" + | "other"; + +/** + * Classify a changed file into a single category. Non-substantive padding categories + * (minified/generated/vendored) take precedence so they are never miscounted as substantive source + * or test effort; lockfiles and dependency manifests are recognized before generic docs/source. + */ +export function classifyChangedFile(path: string): ChangedFileCategory { + if (isMinifiedFile(path)) return "minified"; + if (isGeneratedFile(path)) return "generated"; + if (isVendoredFile(path)) return "vendored"; + if (isLockfile(path)) return "lockfile"; + if (isDependencyManifestFile(path)) return "dependency_manifest"; + if (isTestFile(path) || isTestPath(path)) return "test"; + if (isDocsFile(path)) return "docs"; + if (isCodeFile(path)) return "source"; + return "other"; +} diff --git a/src/signals/slop.ts b/src/signals/slop.ts index 803ed34750..387d05e2c1 100644 --- a/src/signals/slop.ts +++ b/src/signals/slop.ts @@ -2,6 +2,7 @@ import type { SignalFinding } from "./engine"; import { isCodeFile, isTestFile } from "./local-branch"; import { hasLocalTestEvidence, isTestPath } from "./test-evidence"; import { isFocusManifestPublicSafe } from "./focus-manifest"; +import { classifyChangedFile } from "./path-matchers"; export type SlopBand = "clean" | "low" | "elevated" | "high"; @@ -27,10 +28,12 @@ export type SlopAssessment = { // Deterministic, high-precision signals only — this score is the ONLY thing allowed to gate (block), so it // must be false-positive-averse. Heuristic/AI "this reads low-effort" judgments stay ADVISORY elsewhere and -// never feed this score. Weights sum to 75 so the `high` band (>=60) is reachable from two strong signals. +// never feed this score. Each "strong" signal is weighted 30 so the `high` band (>=60) is reachable from any +// two of them; `clamp(.,0,100)` keeps the stacked score bounded. export const SLOP_WEIGHTS = { trivialWhitespaceChurn: 30, missingTestEvidence: 30, + nonSubstantivePadding: 30, emptyDescription: 15, } as const; @@ -45,24 +48,31 @@ export const SLOP_RUBRIC_MARKDOWN = [ "Current deterministic signals:", "- trivial / whitespace-only churn", "- missing test evidence", + "- non-substantive padding (generated / vendored / minified output as source)", "- empty pull request description on a code change", ].join("\n"); const MIN_CHURN_LINES = 40; const MAX_SOURCE_LINE_SHARE = 0.15; +// A padded diff is one whose churn is dominated by non-substantive output. Set at half the diff so a PR +// with any meaningful share of real, hand-authored files cannot trip it. +const PADDING_DOMINANCE_SHARE = 0.5; export function buildSlopAssessment(input: SlopAssessmentInput): SlopAssessment { const findings: SignalFinding[] = []; const trivialChurnFinding = buildTrivialWhitespaceChurnFinding(input); const missingTestEvidenceFinding = buildMissingTestEvidenceFinding(input); + const nonSubstantivePaddingFinding = buildNonSubstantivePaddingFinding(input); const emptyDescriptionFinding = buildEmptyDescriptionFinding(input); if (trivialChurnFinding) findings.push(trivialChurnFinding); if (missingTestEvidenceFinding) findings.push(missingTestEvidenceFinding); + if (nonSubstantivePaddingFinding) findings.push(nonSubstantivePaddingFinding); if (emptyDescriptionFinding) findings.push(emptyDescriptionFinding); const slopRisk = clamp( (trivialChurnFinding ? SLOP_WEIGHTS.trivialWhitespaceChurn : 0) + (missingTestEvidenceFinding ? SLOP_WEIGHTS.missingTestEvidence : 0) + + (nonSubstantivePaddingFinding ? SLOP_WEIGHTS.nonSubstantivePadding : 0) + (emptyDescriptionFinding ? SLOP_WEIGHTS.emptyDescription : 0), 0, 100, @@ -75,6 +85,55 @@ export function buildSlopAssessment(input: SlopAssessmentInput): SlopAssessment }; } +// Fires when a high-churn diff is dominated by generated/vendored/minified output (files that carry code +// extensions and so slip past the source-share check in `trivialWhitespaceChurn`) while genuine source and +// test effort is negligible — i.e. the diff is padded to look substantive. Lockfiles, dependency manifests, +// and docs are legitimate change categories and never count toward the padding share, so dependency bumps +// and docs PRs cannot trip this. +export function buildNonSubstantivePaddingFinding(input: SlopAssessmentInput): SignalFinding | null { + const totals = summarizePaddingLines(input.changedFiles ?? []); + if (totals.changedLineCount < MIN_CHURN_LINES) return null; + if (totals.paddingLineCount === 0) return null; + if (totals.paddingLineCount / totals.changedLineCount < PADDING_DOMINANCE_SHARE) return null; + if (totals.substantiveLineCount / totals.changedLineCount > MAX_SOURCE_LINE_SHARE) return null; + return buildPaddingFinding(totals.changedLineCount, totals.paddingLineCount); +} + +function summarizePaddingLines(changedFiles: SlopChangedFile[]): { + changedLineCount: number; + paddingLineCount: number; + substantiveLineCount: number; +} { + let changedLineCount = 0; + let paddingLineCount = 0; + let substantiveLineCount = 0; + for (const file of changedFiles) { + const lines = nonNegative(file.additions) + nonNegative(file.deletions); + if (lines === 0) continue; + changedLineCount += lines; + const category = classifyChangedFile(file.path); + if (category === "minified" || category === "generated" || category === "vendored") { + paddingLineCount += lines; + } else if (category === "source" || category === "test") { + substantiveLineCount += lines; + } + } + return { changedLineCount, paddingLineCount, substantiveLineCount }; +} + +function buildPaddingFinding(changedLineCount: number, paddingLineCount: number): SignalFinding { + // Only integer counts are interpolated, so the text is public-safe by construction. + const detail = `${paddingLineCount} of ${changedLineCount} changed line(s) are in generated, vendored, or minified files with little substantive source.`; + return { + code: "non_substantive_padding", + title: "Diff is mostly generated, vendored, or minified output", + severity: "warning", + detail, + action: "Exclude generated, vendored, and minified output and keep the diff focused on substantive changes.", + publicText: detail, + }; +} + // Fires only when a real code change ships with an empty / whitespace-only description — a high-precision // weak-effort signal. A non-empty description (even a terse one) never trips it, to avoid false positives. export function buildEmptyDescriptionFinding(input: SlopAssessmentInput): SignalFinding | null { diff --git a/test/unit/path-matchers.test.ts b/test/unit/path-matchers.test.ts new file mode 100644 index 0000000000..0550f59b65 --- /dev/null +++ b/test/unit/path-matchers.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, it } from "vitest"; +import { + classifyChangedFile, + isDependencyManifestFile, + isDocsFile, + isGeneratedFile, + isLockfile, + isMinifiedFile, + isNonSubstantivePaddingFile, + isVendoredFile, +} from "../../src/signals/path-matchers"; + +describe("isGeneratedFile", () => { + it("matches generated output by directory, suffix, codegen, and source maps", () => { + for (const path of [ + "src/__generated__/schema.ts", + "app/generated/client.ts", + "src/api.generated.ts", + "src/types.gen.ts", + "proto/service.pb.go", + "proto/service.pb.ts", + "gen/service_pb2.py", + "gen/service_pb2.pyi", + "lib/models.g.dart", + "dist/app.js.map", + "styles/site.css.map", + "worker-configuration.d.ts", + "C:\\repo\\src\\api.generated.ts", + ]) { + expect(isGeneratedFile(path)).toBe(true); + } + }); + + it("does not match hand-authored files that merely contain the word", () => { + for (const path of ["src/generated-helpers.ts", "src/regenerated.ts", "src/codegen.ts", "src/app.ts"]) { + expect(isGeneratedFile(path)).toBe(false); + } + }); +}); + +describe("isVendoredFile", () => { + it("matches vendored / third-party directories", () => { + for (const path of ["vendor/lib.go", "vendored/x.js", "third_party/y.py", "third-party/z.ts", "node_modules/pkg/index.js"]) { + expect(isVendoredFile(path)).toBe(true); + } + }); + + it("does not match files that only resemble vendor names", () => { + for (const path of ["src/vendor.ts", "src/vendoring.ts"]) { + expect(isVendoredFile(path)).toBe(false); + } + }); +}); + +describe("isLockfile", () => { + it("matches known lockfiles regardless of directory or case", () => { + for (const path of [ + "package-lock.json", + "frontend/yarn.lock", + "pnpm-lock.yaml", + "Cargo.lock", + "go.sum", + "uv.lock", + "poetry.lock", + ]) { + expect(isLockfile(path)).toBe(true); + } + }); + + it("does not match dependency manifests or other json", () => { + for (const path of ["package.json", "tsconfig.json", "data/values.json"]) { + expect(isLockfile(path)).toBe(false); + } + }); +}); + +describe("isMinifiedFile", () => { + it("matches minified bundles", () => { + for (const path of ["dist/app.min.js", "public/styles.min.css", "vendor/lib.min.mjs"]) { + expect(isMinifiedFile(path)).toBe(true); + } + }); + + it("does not match unminified files", () => { + for (const path of ["src/app.js", "src/minify.ts", "src/app.minify.js"]) { + expect(isMinifiedFile(path)).toBe(false); + } + }); +}); + +describe("isDocsFile", () => { + it("matches docs by extension or a docs directory", () => { + for (const path of ["README.md", "guide.mdx", "notes.rst", "manual.adoc", "docs/architecture.ts", "doc/legacy.md"]) { + expect(isDocsFile(path)).toBe(true); + } + }); + + it("does not match source, config, or extensionless files outside docs", () => { + for (const path of ["src/app.ts", "config.json", "notes.txt", "LICENSE", ".gitignore"]) { + expect(isDocsFile(path)).toBe(false); + } + }); +}); + +describe("defensive input handling", () => { + it("treats null/undefined paths as non-matching, uncategorized input", () => { + for (const path of [null, undefined] as unknown as string[]) { + expect(isLockfile(path)).toBe(false); + expect(isGeneratedFile(path)).toBe(false); + expect(classifyChangedFile(path)).toBe("other"); + } + }); +}); + +describe("isDependencyManifestFile", () => { + it("matches dependency manifests", () => { + for (const path of ["package.json", "Cargo.toml", "go.mod", "requirements.txt", "pyproject.toml", "build.gradle.kts"]) { + expect(isDependencyManifestFile(path)).toBe(true); + } + }); + + it("does not match lockfiles or arbitrary config", () => { + for (const path of ["package-lock.json", "tsconfig.json"]) { + expect(isDependencyManifestFile(path)).toBe(false); + } + }); +}); + +describe("isNonSubstantivePaddingFile", () => { + it("flags generated / vendored / minified output as padding", () => { + for (const path of ["src/api.generated.ts", "vendor/lib.go", "dist/app.min.js"]) { + expect(isNonSubstantivePaddingFile(path)).toBe(true); + } + }); + + it("does not flag lockfiles, manifests, docs, tests, or real source as padding", () => { + for (const path of ["package-lock.json", "package.json", "README.md", "test/unit/app.test.ts", "src/app.ts"]) { + expect(isNonSubstantivePaddingFile(path)).toBe(false); + } + }); +}); + +describe("classifyChangedFile", () => { + it("classifies each representative path into its category", () => { + const cases: Array<[string, ReturnType]> = [ + ["dist/app.min.js", "minified"], + ["src/api.generated.ts", "generated"], + ["vendor/lib.go", "vendored"], + ["package-lock.json", "lockfile"], + ["package.json", "dependency_manifest"], + ["test/unit/app.test.ts", "test"], + ["README.md", "docs"], + ["src/app.ts", "source"], + ["data/values.json", "other"], + ]; + for (const [path, expected] of cases) { + expect(classifyChangedFile(path)).toBe(expected); + } + }); + + it("prioritizes padding categories over test/source so they are never counted as effort", () => { + expect(classifyChangedFile("__generated__/schema.test.ts")).toBe("generated"); + expect(classifyChangedFile("vendor/pkg/index.test.js")).toBe("vendored"); + expect(classifyChangedFile("dist/bundle.min.js")).toBe("minified"); + }); +}); diff --git a/test/unit/slop.test.ts b/test/unit/slop.test.ts index af04807430..3c435f4dce 100644 --- a/test/unit/slop.test.ts +++ b/test/unit/slop.test.ts @@ -3,6 +3,7 @@ import { buildEmptyIssueBodyFinding, buildIssueSlopAssessment, buildMissingTestEvidenceFinding, + buildNonSubstantivePaddingFinding, buildSlopAssessment, buildTrivialWhitespaceChurnFinding, buildUnfilledIssueTemplateFinding, @@ -232,3 +233,85 @@ describe("buildIssueSlopAssessment (#533 issue-side triage)", () => { expect(buildEmptyIssueBodyFinding({ body: "has content" })).toBeNull(); }); }); + +describe("buildNonSubstantivePaddingFinding (#561 path-matcher signal)", () => { + const FORBIDDEN = + /wallet|hotkey|coldkey|mnemonic|reward|payout|raw trust|trust score|scoreability|private reviewability|\/Users|\/home|\/tmp/i; + + it("fires when generated/vendored/minified output dominates a high-churn diff with negligible source", () => { + const finding = buildNonSubstantivePaddingFinding({ + changedFiles: [ + { path: "dist/bundle.min.js", additions: 300, deletions: 100 }, + { path: "vendor/lib.go", additions: 50, deletions: 0 }, + { path: "src/app.ts", additions: 4, deletions: 2 }, + { path: "test/unit/app.test.ts", additions: 6, deletions: 0 }, + { path: "untouched.ts", additions: 0, deletions: 0 }, // zero-line entry is skipped + ], + }); + expect(finding).toMatchObject({ code: "non_substantive_padding", severity: "warning" }); + expect(JSON.stringify(finding)).not.toMatch(FORBIDDEN); + }); + + it("does not fire when substantive source/test work is present", () => { + // Padding is the minority of the churn. + expect( + buildNonSubstantivePaddingFinding({ + changedFiles: [ + { path: "dist/bundle.min.js", additions: 20, deletions: 0 }, + { path: "src/app.ts", additions: 100, deletions: 30 }, + { path: "test/unit/app.test.ts", additions: 40, deletions: 0 }, + ], + }), + ).toBeNull(); + // Padding dominates by count, but real source is still a meaningful share of the diff. + expect( + buildNonSubstantivePaddingFinding({ + changedFiles: [ + { path: "dist/bundle.min.js", additions: 60, deletions: 0 }, + { path: "src/app.ts", additions: 30, deletions: 10 }, + ], + }), + ).toBeNull(); + }); + + it("does not fire for dependency bumps or docs-only diffs", () => { + expect( + buildNonSubstantivePaddingFinding({ + changedFiles: [ + { path: "package-lock.json", additions: 400, deletions: 200 }, + { path: "package.json", additions: 2, deletions: 2 }, + ], + }), + ).toBeNull(); + expect( + buildNonSubstantivePaddingFinding({ + changedFiles: [{ path: "docs/guide.md", additions: 300, deletions: 100 }], + }), + ).toBeNull(); + }); + + it("does not fire below the churn threshold or with no padding files", () => { + expect( + buildNonSubstantivePaddingFinding({ changedFiles: [{ path: "dist/app.min.js", additions: 20, deletions: 0 }] }), + ).toBeNull(); + expect( + buildNonSubstantivePaddingFinding({ changedFiles: [{ path: "src/app.ts", additions: 200, deletions: 50 }] }), + ).toBeNull(); + expect(buildNonSubstantivePaddingFinding({})).toBeNull(); + }); + + it("contributes to the aggregate slop assessment without colliding with trivial-churn", () => { + const result = buildSlopAssessment({ + changedFiles: [ + { path: "dist/bundle.min.js", additions: 300, deletions: 100 }, + { path: "src/app.ts", additions: 5, deletions: 2 }, + { path: "test/unit/app.test.ts", additions: 8, deletions: 0 }, + ], + description: "Rebuild the minified bundle.", + }); + expect(result.findings.map((finding) => finding.code)).toEqual(["non_substantive_padding"]); + expect(result.slopRisk).toBe(SLOP_WEIGHTS.nonSubstantivePadding); + expect(result.band).toBe("elevated"); + expect(JSON.stringify(result)).not.toMatch(FORBIDDEN); + }); +});