diff --git a/src/signals/engine.ts b/src/signals/engine.ts index eba8212912..03f25522f5 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -28,6 +28,7 @@ import { extractLinkedIssueNumbers } from "../db/repositories"; import { sanitizePublicComment } from "../queue-intelligence"; import { labelMatchesPattern, projectLinkedIssueMultiplierForPlannedSolve, type LinkedIssueMultiplierStatus } from "../scoring/preview"; import { hasLocalTestEvidence, hasValidationNote, isTestPath } from "./test-evidence"; +import { isCodeFile, isTestFile } from "./path-matchers"; import { isFailingCheckSummary } from "./local-branch"; import { isDuplicateClusterWinnerByClaim } from "./duplicate-winner"; import { PREFLIGHT_LIMITS } from "./preflight-limits"; @@ -5528,25 +5529,6 @@ function sanitizeOutcomeDimensionKey(key: string): string { .trim(); } -function isCodeFile(file: string): boolean { - // Mirrors isCodeFile in local-branch.ts — kept in sync (cs/swift/groovy/php and C/C++/Objective-C added - // so native/C#/Swift/Groovy/PHP source counts as code, matching the test conventions - // isTestPath already recognizes; vue/svelte/astro match rag.ts, visual paths, and isCodePath; - // cc/hpp complete the C++ extension set alongside cpp/c/h; dart matches rag.ts and - // test-evidence's *_test.dart test convention). - return ( - /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs|py|rb|rs|kt|scala|java|go|sql|cs|swift|groovy|php|cpp|cc|c|h|hpp|m|vue|svelte|astro|dart)$/i.test( - file, - ) && !isTestFile(file) - ); -} - -function isTestFile(file: string): boolean { - // Single-sourced with the canonical matcher (test-evidence.ts isTestPath), mirroring local-branch.ts's - // isTestFile — so cy/e2e, __snapshots__, and module extensions stay in sync and can't drift. - return isTestPath(file); -} - function riskRank(risk: CollisionCluster["risk"]): number { if (risk === "high") return 3; /* v8 ignore next -- Low collision rank is the default branch; high/medium sorting behavior is covered by collision tests. */ diff --git a/src/signals/path-matchers.ts b/src/signals/path-matchers.ts index d61a3824db..e6ce3a4af2 100644 --- a/src/signals/path-matchers.ts +++ b/src/signals/path-matchers.ts @@ -1,4 +1,4 @@ -import { isTestPath } from "./test-evidence"; +import { isSourcePath, 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- @@ -15,22 +15,16 @@ export function isTestFile(file: string): boolean { return isTestPath(file); } -/** cs/swift/groovy/php plus C/C++/Objective-C round out the native/JVM/.NET/Swift/PHP set: isTestPath already - * recognizes their `SomethingTest(s)`/`Spec` test files, so their source must count as code too — - * otherwise a C#/Swift/Groovy/PHP/native source file is neither test nor code in the local scorer. - * vue/svelte/astro align with review/rag.ts CODE_EXT_RE, review/visual/paths.ts, and rules/advisory.ts - * isCodePath so every classifier agrees. cc/hpp round out the C++ set alongside cpp/c/h (rag.ts already - * indexes all four). dart aligns with rag.ts and test-evidence's *_test.dart convention (hand-authored - * .dart is source; generated .g.dart/.freezed.dart/.gr.dart part files stay non-code, mirroring the - * packages/gittensory-mcp and gittensor-score-preview classifiers, #3724). */ +// Extensions recognized as code outside test-evidence's isSourcePath core set (php, native, front-end +// frameworks, Dart). isSourcePath owns the JVM/.NET/Swift/Groovy/Kotlin-script set symmetric with isTestPath. +const EXTENDED_SOURCE_EXTENSION = /\.(php|cpp|cc|c|h|hpp|m|vue|svelte|astro|dart)$/i; + +/** cs/swift/groovy/kts plus php, C/C++/Objective-C, vue/svelte/astro, and dart — see isSourcePath for the + * canonical JVM/.NET/Swift/Groovy/Kotlin-script matcher kept symmetric with isTestPath. Generated Dart part + * files (.g.dart/.freezed.dart/.gr.dart) stay non-code (#3724). */ export function isCodeFile(file: string): boolean { - return ( - /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs|py|rb|rs|kt|scala|java|go|sql|cs|swift|groovy|php|cpp|cc|c|h|hpp|m|vue|svelte|astro|dart)$/i.test( - file, - ) && - !isTestFile(file) && - !/\.(g|freezed|gr)\.dart$/i.test(file) - ); + if (isSourcePath(file)) return true; + return EXTENDED_SOURCE_EXTENSION.test(file) && !isTestFile(file) && !/\.(g|freezed|gr)\.dart$/i.test(file); } function normalize(path: string): string { diff --git a/src/signals/test-evidence.ts b/src/signals/test-evidence.ts index a0381a20b6..ad4897c513 100644 --- a/src/signals/test-evidence.ts +++ b/src/signals/test-evidence.ts @@ -16,6 +16,19 @@ export function isTestPath(file: string): boolean { ); } +// Canonical hand-authored-source extensions — the SOURCE-side sibling of isTestPath's class-suffix rule. +// The two matchers MUST stay symmetric: isTestPath recognizes java/kt/kts/scala/cs/swift/groovy test files, +// so this set lists those same languages. Otherwise a C#/Swift/Groovy/Kotlin-script SOURCE change is classified +// as neither code nor test and silently escapes both the missing-tests gate signals and token scoring. +const SOURCE_FILE_EXTENSION = /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs|py|rb|rs|kt|kts|scala|java|cs|swift|groovy|go|sql)$/i; + +/** True iff `file` is a hand-authored program-source file: a recognized source extension that is not itself a + * test file. The single source of truth for every `isCodeFile` in the signals layer, so the source/test + * classifiers can never drift (the same way isCodeFile's `isTestFile` wrappers all delegate to isTestPath). */ +export function isSourcePath(file: string): boolean { + return SOURCE_FILE_EXTENSION.test(file) && !isTestPath(file); +} + export function hasLocalTestEvidence(input: { tests?: string[] | undefined; testFiles?: string[] | undefined }): boolean { return (input.tests ?? []).length > 0 || (input.testFiles ?? []).some((file) => isTestPath(file)); } diff --git a/test/unit/local-branch-file-classifiers.test.ts b/test/unit/local-branch-file-classifiers.test.ts index 963d9b3479..42b4ce6e8f 100644 --- a/test/unit/local-branch-file-classifiers.test.ts +++ b/test/unit/local-branch-file-classifiers.test.ts @@ -122,6 +122,7 @@ describe("isCodeFile", () => { "Api/Controllers/UserController.cs", "Sources/App/Router.swift", "src/main/groovy/Pipeline.groovy", + "app/Build.kts", // PHP source — isTestPath already recognizes PHPUnit/PHPSpec `SomethingTest`/`Spec` // files, so PHP source must count as code too (else it is neither test nor code). "app/Http/Controllers/UserController.php", @@ -165,6 +166,8 @@ describe("isCodeFile", () => { "app/Service/PaymentTest.php", // Dart co-located *_test.dart is test evidence, not source. "lib/models/user_test.dart", + "gradle/CartSpec.groovy", + "build/SettingsTests.kts", ]) { expect(isCodeFile(path)).toBe(false); } diff --git a/test/unit/path-matchers.test.ts b/test/unit/path-matchers.test.ts index de34039e65..45caf65722 100644 --- a/test/unit/path-matchers.test.ts +++ b/test/unit/path-matchers.test.ts @@ -615,6 +615,12 @@ describe("classifyChangedFile", () => { ["README.md", "docs"], ["src/app.ts", "source"], ["src/integration/auth.ts", "source"], + // C#/Swift/Groovy source classify as source (their manifests Package.swift/build.gradle.kts are matched + // as dependency_manifest above; the language sources themselves are real code). + ["Services/PaymentProcessor.cs", "source"], + ["Sources/App/Login.swift", "source"], + ["gradle/Cart.groovy", "source"], + ["app/Build.kts", "source"], ["data/values.json", "other"], ]; for (const [path, expected] of cases) { diff --git a/test/unit/signals.test.ts b/test/unit/signals.test.ts index df22f9aa5f..1fbe1af33d 100644 --- a/test/unit/signals.test.ts +++ b/test/unit/signals.test.ts @@ -456,6 +456,36 @@ describe("world-class backend signals", () => { expect(result.findings.map((finding) => finding.code)).not.toContain("missing_test_evidence"); }); + it("flags missing test evidence for Kotlin-script source (isTestPath already knows .kts tests)", () => { + const result = buildPreflightResult( + { + repoFullName: repo.fullName, + title: "Tune Gradle Kotlin build script", + body: "Fixes #7", + changedFiles: ["app/Build.kts"], + }, + repo, + issues, + pullRequests, + ); + expect(result.findings.map((finding) => finding.code)).toContain("missing_test_evidence"); + }); + + it("does not flag missing test evidence for generated Dart part files (engine delegates to path-matchers)", () => { + const result = buildPreflightResult( + { + repoFullName: repo.fullName, + title: "Regenerate freezed models", + body: "Fixes #7", + changedFiles: ["lib/models/user.g.dart", "lib/models/user.freezed.dart"], + }, + repo, + issues, + pullRequests, + ); + expect(result.findings.map((finding) => finding.code)).not.toContain("missing_test_evidence"); + }); + it("gates public comments to detected contributors and sanitizes comment text", () => { const currentPr = pullRequests[0]!; const priorPr: PullRequestRecord = { diff --git a/test/unit/test-evidence.test.ts b/test/unit/test-evidence.test.ts index 8f85eec4b6..83bf93ce89 100644 --- a/test/unit/test-evidence.test.ts +++ b/test/unit/test-evidence.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { classifyTestCoverage, detectTestConvention, hasLocalTestEvidence, hasValidationNote, isTestPath } from "../../src/signals/test-evidence"; +import { classifyTestCoverage, detectTestConvention, hasLocalTestEvidence, hasValidationNote, isSourcePath, isTestPath } from "../../src/signals/test-evidence"; describe("test evidence helpers", () => { it("detects common test path conventions", () => { @@ -165,6 +165,55 @@ describe("test evidence helpers", () => { }); }); +describe("isSourcePath", () => { + it("recognizes hand-authored source across every supported language", () => { + for (const path of [ + "src/index.ts", + "components/Button.tsx", + "src/loader.mjs", + "src/legacy.cjs", + "service/main.py", + "lib/parser.rb", + "engine/core.rs", + "android/App.kt", + "etl/Job.scala", + "server/Main.java", + "cmd/server/main.go", + "migrations/0001_init.sql", + ]) { + expect(isSourcePath(path)).toBe(true); + } + }); + + it("recognizes Kotlin-script source symmetrically with isTestPath's class-suffix rule", () => { + // Regression: isTestPath already recognizes `SomethingTests.kts`, but `.kts` source was missing from the + // code matcher, so an untested Gradle Kotlin-script change escaped the missing-tests signals. + expect(isSourcePath("app/Build.kts")).toBe(true); + expect(isSourcePath("gradle/Cart.groovy")).toBe(true); + expect(isSourcePath("Services/PaymentProcessor.cs")).toBe(true); + expect(isSourcePath("Sources/App/Login.swift")).toBe(true); + }); + + it("excludes test files even when they carry a source extension", () => { + for (const path of [ + "math.test.ts", + "Services/OrderTests.cs", + "Sources/App/LoginTests.swift", + "gradle/CartSpec.groovy", + "build/SettingsTests.kts", + "handler_test.go", + ]) { + expect(isSourcePath(path)).toBe(false); + } + }); + + it("excludes non-source assets and extensionless files", () => { + for (const path of ["README.md", "package.json", "assets/logo.png", "Dockerfile", "data/values.json"]) { + expect(isSourcePath(path)).toBe(false); + } + }); +}); + describe("classifyTestCoverage", () => { it("classifies an empty path list as absent", () => { expect(classifyTestCoverage([])).toBe("absent");