From 3b626d7ab452f3b52468993de50754df76d5b5db Mon Sep 17 00:00:00 2001 From: luciferlive112116 <291889058+luciferlive112116@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:41:01 +0800 Subject: [PATCH] feat(signals): wire test coverage classification across contributor surfaces Extend test-evidence with fixture-path detection and a shared coverage summary, then surface weak/adequate/strong classification in local workspace intelligence, slop assessments, the contributor open-PR monitor, and the OpenAPI contract. Co-authored-by: Cursor --- apps/gittensory-ui/public/openapi.json | 43 ++++++++ src/openapi/schemas.ts | 10 ++ src/signals/contributor-open-pr-monitor.ts | 50 ++++++++-- src/signals/local-workspace-intelligence.ts | 8 +- src/signals/slop.ts | 41 +++++++- src/signals/test-evidence.ts | 97 +++++++++++++++++-- test/unit/contributor-open-pr-monitor.test.ts | 82 +++++++++++++--- .../unit/local-workspace-intelligence.test.ts | 34 +++++++ test/unit/slop.test.ts | 52 ++++++++++ test/unit/test-evidence.test.ts | 77 ++++++++++++++- 10 files changed, 456 insertions(+), 38 deletions(-) diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index 81b276a664..29ed98e7f6 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -2714,6 +2714,7 @@ "needs_author", "failing_checks", "missing_tests", + "weak_test_coverage", "duplicate_prone", "reviewable", "should_close_or_withdraw", @@ -7620,6 +7621,47 @@ }, "rerunWhen": { "type": "string" + }, + "testCoverage": { + "type": "object", + "properties": { + "classification": { + "type": "string", + "enum": [ + "strong", + "adequate", + "weak", + "absent" + ] + }, + "changedPathCount": { + "type": "number" + }, + "sourcePathCount": { + "type": "number" + }, + "testPathCount": { + "type": "number" + }, + "fixturePathCount": { + "type": "number" + }, + "testToChangedRatio": { + "type": "number" + }, + "guidance": { + "type": "string" + } + }, + "required": [ + "classification", + "changedPathCount", + "sourcePathCount", + "testPathCount", + "fixturePathCount", + "testToChangedRatio", + "guidance" + ] } }, "required": [ @@ -7628,6 +7670,7 @@ "branch", "changedFiles", "testEvidence", + "testCoverage", "linkedIssues", "baseFreshness", "ciStatusHints", diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index 5c8d068ea8..a45f06e7b1 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -335,6 +335,7 @@ export const ContributorOpenPrNextStepPacketSchema = z "needs_author", "failing_checks", "missing_tests", + "weak_test_coverage", "duplicate_prone", "reviewable", "should_close_or_withdraw", @@ -2158,6 +2159,15 @@ export const LocalWorkspaceIntelligenceSchema = z }), ), }), + testCoverage: z.object({ + classification: z.enum(["strong", "adequate", "weak", "absent"]), + changedPathCount: z.number(), + sourcePathCount: z.number(), + testPathCount: z.number(), + fixturePathCount: z.number(), + testToChangedRatio: z.number(), + guidance: z.string(), + }), linkedIssues: z.array(z.number()), baseFreshness: z.object({ status: z.enum(["fresh", "stale", "possibly_stale", "unknown"]), diff --git a/src/signals/contributor-open-pr-monitor.ts b/src/signals/contributor-open-pr-monitor.ts index ffa786cccf..bc99295e9a 100644 --- a/src/signals/contributor-open-pr-monitor.ts +++ b/src/signals/contributor-open-pr-monitor.ts @@ -12,7 +12,7 @@ import type { CheckSummaryRecord, PullRequestFileRecord, PullRequestRecord, Pull import { nowIso } from "../utils/json"; import { buildRoleContext } from "./engine"; import { isFailingCheckSummary } from "./local-branch"; -import { isTestPath } from "./test-evidence"; +import { buildTestCoverageSummary, type TestCoverageClassification } from "./test-evidence"; export type OpenPrWorkClassification = | "approved" @@ -21,6 +21,7 @@ export type OpenPrWorkClassification = | "needs_author" | "failing_checks" | "missing_tests" + | "weak_test_coverage" | "duplicate_prone" | "reviewable" | "should_close_or_withdraw" @@ -89,7 +90,16 @@ export async function buildContributorOpenPrMonitor(env: Env, login: string): Pr duplicateProne: duplicateNumbers.has(pr.number), missingTests: missingTestsFromFiles(files), }); - packets.push(buildNextStepPacket(classified, reviews, checks, duplicateNumbers.has(pr.number), missingTestsFromFiles(files))); + packets.push( + buildNextStepPacket( + classified, + reviews, + checks, + duplicateNumbers.has(pr.number), + missingTestsFromFiles(files), + weakTestCoverageFromFiles(files), + ), + ); } const detection = detectPendingPrScenario({ @@ -128,7 +138,7 @@ export async function buildContributorOpenPrMonitor(env: Env, login: string): Pr export function mapPendingClassToWorkClassification( classified: ClassifiedOpenPullRequest, - args: { changeRequestCount: number; checkFailureCount: number; duplicateProne: boolean; missingTests: boolean }, + args: { changeRequestCount: number; checkFailureCount: number; duplicateProne: boolean; missingTests: boolean; weakTestCoverage: boolean }, ): OpenPrWorkClassification { if (classified.classification === "maintainer_lane") return "maintainer_lane"; if (classified.classification === "draft") return "draft"; @@ -137,6 +147,7 @@ export function mapPendingClassToWorkClassification( if (args.checkFailureCount > 0) return "failing_checks"; if (args.changeRequestCount > 0) return "needs_author"; if (args.missingTests) return "missing_tests"; + if (args.weakTestCoverage) return "weak_test_coverage"; if (classified.classification === "merge_ready") return "approved"; if (classified.classification === "blocked") return "blocked"; return "reviewable"; @@ -148,10 +159,17 @@ function buildNextStepPacket( checks: CheckSummaryRecord[], duplicateProne: boolean, missingTests: boolean, + weakTestCoverage: boolean, ): ContributorOpenPrNextStepPacket { const changeRequestCount = reviews.filter((review) => review.state.toUpperCase() === "CHANGES_REQUESTED").length; const checkFailureCount = checks.filter(isFailingCheckSummary).length; - const classification = mapPendingClassToWorkClassification(classified, { changeRequestCount, checkFailureCount, duplicateProne, missingTests }); + const classification = mapPendingClassToWorkClassification(classified, { + changeRequestCount, + checkFailureCount, + duplicateProne, + missingTests, + weakTestCoverage, + }); const nextSteps = nextStepsForClassification(classification, classified.repoFullName, classified.number); const summary = `${classified.repoFullName}#${classified.number}: ${classification.replace(/_/g, " ")} — ${classified.title}`; return { @@ -176,6 +194,11 @@ function nextStepsForClassification(classification: OpenPrWorkClassification, re return [`Address review comments on ${ref} and push updates.`, `Reply on the PR thread summarizing what changed.`]; case "missing_tests": return [`Add or update tests on ${ref} if the repo expects test coverage.`, `Note test commands run in the PR description.`]; + case "weak_test_coverage": + return [ + `Broaden test or fixture coverage on ${ref} so it matches the source files touched.`, + `Add focused regression tests for the modules changed and note the commands run in the PR description.`, + ]; case "duplicate_prone": return [`Check overlap with other open PRs in ${repoFullName}; close or consolidate duplicates.`, `Comment on ${ref} linking the canonical PR if one exists.`]; case "stale": @@ -259,10 +282,18 @@ function normalizeTitle(title: string): string { } function missingTestsFromFiles(files: PullRequestFileRecord[]): boolean { - if (files.length === 0) return false; - const codeFiles = files.filter((file) => file.path && !isTestPath(file.path)); - const testFiles = files.filter((file) => file.path && isTestPath(file.path)); - return codeFiles.length > 0 && testFiles.length === 0; + const classification = testCoverageClassificationFromFiles(files); + return classification === "absent"; +} + +function weakTestCoverageFromFiles(files: PullRequestFileRecord[]): boolean { + return testCoverageClassificationFromFiles(files) === "weak"; +} + +function testCoverageClassificationFromFiles(files: PullRequestFileRecord[]): TestCoverageClassification | null { + const paths = files.map((file) => file.path).filter(Boolean); + if (paths.length === 0) return null; + return buildTestCoverageSummary(paths).classification; } function priorityRank(classification: OpenPrWorkClassification): number { @@ -271,6 +302,7 @@ function priorityRank(classification: OpenPrWorkClassification): number { "needs_author", "duplicate_prone", "missing_tests", + "weak_test_coverage", "blocked", "should_close_or_withdraw", "stale", @@ -294,6 +326,8 @@ export const __contributorOpenPrMonitorInternals = { buildMonitorGuidance, duplicatePronePullNumbers, missingTestsFromFiles, + weakTestCoverageFromFiles, + testCoverageClassificationFromFiles, priorityRank, buildNextStepPacket, }; diff --git a/src/signals/local-workspace-intelligence.ts b/src/signals/local-workspace-intelligence.ts index 07f326a48d..be0751d165 100644 --- a/src/signals/local-workspace-intelligence.ts +++ b/src/signals/local-workspace-intelligence.ts @@ -1,6 +1,6 @@ import { isPassingValidation } from "./local-branch"; import type { LocalBranchAnalysis, LocalBranchAnalysisInput, LocalBranchChangedFile, LocalBranchValidation } from "./local-branch"; -import { isTestPath } from "./test-evidence"; +import { buildTestCoverageSummary, isTestEvidencePath, type TestCoverageSummary } from "./test-evidence"; import { sanitizeLocalScorerWarnings } from "./local-scorer-diagnostics"; export type LocalWorkspaceIntelligence = { @@ -30,6 +30,7 @@ export type LocalWorkspaceIntelligence = { passedValidationCount: number; commands: LocalBranchValidation[]; }; + testCoverage: TestCoverageSummary; linkedIssues: number[]; baseFreshness: LocalBranchAnalysis["baseFreshness"]; ciStatusHints: string[]; @@ -55,11 +56,13 @@ export function buildLocalWorkspaceIntelligence(args: { changedFiles: LocalBranchChangedFile[]; }): LocalWorkspaceIntelligence { const validation = args.input.validation ?? []; - const testFileCount = args.changedFiles.filter((file) => isTestPath(file.path)).length; + const changedPaths = args.changedFiles.map((file) => file.path).filter(Boolean); + const testFileCount = changedPaths.filter((path) => isTestEvidencePath(path)).length; const passedValidationCount = validation.filter(isPassingValidation).length; const hasTestFiles = testFileCount > 0; const hasValidation = passedValidationCount > 0; const testEvidenceLevel = hasTestFiles && hasValidation ? "both" : hasTestFiles ? "test_files" : hasValidation ? "validation_commands" : "none"; + const testCoverage = buildTestCoverageSummary(changedPaths); const scorer = args.input.localScorer; return { @@ -81,6 +84,7 @@ export function buildLocalWorkspaceIntelligence(args: { passedValidationCount, commands: validation, }, + testCoverage, linkedIssues: [...(args.input.linkedIssues ?? [])].sort((left, right) => left - right), baseFreshness: args.analysis.baseFreshness, ciStatusHints: [...(args.input.ciStatusHints ?? [])], diff --git a/src/signals/slop.ts b/src/signals/slop.ts index a22916cc38..fd7699b123 100644 --- a/src/signals/slop.ts +++ b/src/signals/slop.ts @@ -1,6 +1,6 @@ import { GENERIC_COMMIT_PATTERN, hasClearNoIssueRationale, type SignalFinding } from "./engine"; import { isCodeFile, isTestFile } from "./local-branch"; -import { hasLocalTestEvidence, isTestPath } from "./test-evidence"; +import { buildTestCoverageSummary, hasLocalTestEvidence, isTestEvidencePath, isTestPath } from "./test-evidence"; import { isFocusManifestPublicSafe } from "./focus-manifest"; import { classifyChangedFile } from "./path-matchers"; @@ -47,6 +47,7 @@ export type SlopAssessment = { export const SLOP_WEIGHTS = { trivialWhitespaceChurn: 30, missingTestEvidence: 15, + weakTestCoverage: 10, nonSubstantivePadding: 30, emptyDescription: 15, lowQualityCommitMessage: 15, @@ -65,6 +66,7 @@ export const SLOP_RUBRIC_MARKDOWN = [ "Current deterministic signals:", "- trivial / whitespace-only churn", "- missing test evidence", + "- weak test coverage (some tests, but disproportionately low for the source diff)", "- non-substantive padding (generated / vendored / minified output as source)", "- empty pull request description on a code change", "- generic or empty commit message", @@ -86,6 +88,7 @@ export function buildSlopAssessment(input: SlopAssessmentInput): SlopAssessment const findings: SignalFinding[] = []; const trivialChurnFinding = buildTrivialWhitespaceChurnFinding(input); const missingTestEvidenceFinding = buildMissingTestEvidenceFinding(input); + const weakTestCoverageFinding = buildWeakTestCoverageFinding(input); const nonSubstantivePaddingFinding = buildNonSubstantivePaddingFinding(input); const emptyDescriptionFinding = buildEmptyDescriptionFinding(input); const lowQualityCommitMessageFinding = buildLowQualityCommitMessageFinding(input); @@ -93,6 +96,7 @@ export function buildSlopAssessment(input: SlopAssessmentInput): SlopAssessment const noLinkedIssueRationaleFinding = buildNoLinkedIssueRationaleFinding(input); if (trivialChurnFinding) findings.push(trivialChurnFinding); if (missingTestEvidenceFinding) findings.push(missingTestEvidenceFinding); + if (weakTestCoverageFinding) findings.push(weakTestCoverageFinding); if (nonSubstantivePaddingFinding) findings.push(nonSubstantivePaddingFinding); if (emptyDescriptionFinding) findings.push(emptyDescriptionFinding); if (lowQualityCommitMessageFinding) findings.push(lowQualityCommitMessageFinding); @@ -102,6 +106,7 @@ export function buildSlopAssessment(input: SlopAssessmentInput): SlopAssessment const slopRisk = clamp( (trivialChurnFinding ? SLOP_WEIGHTS.trivialWhitespaceChurn : 0) + (missingTestEvidenceFinding ? SLOP_WEIGHTS.missingTestEvidence : 0) + + (weakTestCoverageFinding ? SLOP_WEIGHTS.weakTestCoverage : 0) + (nonSubstantivePaddingFinding ? SLOP_WEIGHTS.nonSubstantivePadding : 0) + (emptyDescriptionFinding ? SLOP_WEIGHTS.emptyDescription : 0) + (lowQualityCommitMessageFinding ? SLOP_WEIGHTS.lowQualityCommitMessage : 0) + @@ -262,7 +267,7 @@ export function buildMissingTestEvidenceFinding(input: SlopAssessmentInput): Sig // per-file line counts are unavailable we trust the path (can't prove emptiness); when known, require a few // added lines so a stub can't fake coverage. (#audit-3.1) const hasSubstantiveTestFile = changedFiles.some((file) => { - if (!(isTestFile(file.path) || isTestPath(file.path))) return false; + if (!(isTestFile(file.path) || isTestEvidencePath(file.path))) return false; return file.additions === undefined || nonNegative(file.additions) >= MIN_SUBSTANTIVE_TEST_ADDITIONS; }); const hasChangedTestPaths = hasSubstantiveTestFile || hasLocalTestEvidence({ tests: input.tests, testFiles: input.testFiles }); @@ -287,6 +292,38 @@ export function buildMissingTestEvidenceFinding(input: SlopAssessmentInput): Sig }; } +export function buildWeakTestCoverageFinding(input: SlopAssessmentInput): SignalFinding | null { + const changedFiles = input.changedFiles ?? []; + const changedPaths = changedFiles.map((file) => file.path).filter(Boolean); + const coverage = buildTestCoverageSummary(changedPaths); + if (coverage.classification !== "weak") return null; + if (coverage.sourcePathCount === 0) return null; + + const hasSubstantiveTestFile = changedFiles.some((file) => { + if (!isTestEvidencePath(file.path)) return false; + return file.additions === undefined || nonNegative(file.additions) >= MIN_SUBSTANTIVE_TEST_ADDITIONS; + }); + if (!hasSubstantiveTestFile && !hasLocalTestEvidence({ tests: input.tests, testFiles: input.testFiles })) return null; + + const detail = ensurePublicSafeText( + `Changed paths include ${coverage.sourcePathCount} source file(s) but only ${coverage.testPathCount + coverage.fixturePathCount} test/fixture path(s) (${Math.round(coverage.testToChangedRatio * 100)}% of the diff).`, + "Source changes outnumber accompanying test or fixture evidence.", + ); + const action = ensurePublicSafeText( + coverage.guidance, + "Add focused regression tests or fixtures for the touched modules.", + ); + + return { + code: "weak_test_coverage", + title: "Test coverage is disproportionately weak", + severity: "info", + detail, + action, + publicText: detail, + }; +} + export function buildTrivialWhitespaceChurnFinding(input: SlopAssessmentInput): SignalFinding | null { const changedFiles = input.changedFiles ?? []; const lineTotals = summarizeChangedLines(changedFiles); diff --git a/src/signals/test-evidence.ts b/src/signals/test-evidence.ts index 928dff87a9..54b0cce059 100644 --- a/src/signals/test-evidence.ts +++ b/src/signals/test-evidence.ts @@ -1,3 +1,8 @@ +import { isCodeFile } from "./local-branch"; + +export const TEST_COVERAGE_STRONG_RATIO = 0.4; +export const TEST_COVERAGE_ADEQUATE_RATIO = 0.2; + export function isTestPath(file: string): boolean { return ( /(^|\/)(test|tests|spec|__tests__)\//i.test(file) || @@ -10,23 +15,97 @@ export function isTestPath(file: string): boolean { ); } +/** Fixture, mock, and test-data directories carry regression evidence even when the filename is not a test suffix. */ +export function isFixturePath(file: string): boolean { + return /(^|\/)(fixtures?|testdata|test-data|__fixtures__|mocks?|__mocks__)\//i.test(file); +} + +export function isTestEvidencePath(file: string): boolean { + return isTestPath(file) || isFixturePath(file); +} + export function hasLocalTestEvidence(input: { tests?: string[] | undefined; testFiles?: string[] | undefined }): boolean { - return (input.tests ?? []).length > 0 || (input.testFiles ?? []).some((file) => isTestPath(file)); + return (input.tests ?? []).length > 0 || (input.testFiles ?? []).some((file) => isTestEvidencePath(file)); } /** * Coarse classification of how much test coverage accompanies a set of changed paths. - * Used by slop signals to weight diffs that touch source but include no tests differently - * from those with proportionally strong test changes. + * Used by slop signals, workspace intelligence, and the contributor open-PR monitor to + * distinguish absent, weak, adequate, and strong test accompaniment on code changes. */ export type TestCoverageClassification = "strong" | "adequate" | "weak" | "absent"; +export type TestCoverageSummary = { + classification: TestCoverageClassification; + changedPathCount: number; + sourcePathCount: number; + testPathCount: number; + fixturePathCount: number; + /** Share of changed paths that are test or fixture evidence (0 when no paths). */ + testToChangedRatio: number; + guidance: string; +}; + export function classifyTestCoverage(changedPaths: string[]): TestCoverageClassification { - if (changedPaths.length === 0) return "absent"; - const testCount = changedPaths.filter(isTestPath).length; - if (testCount === 0) return "absent"; - const ratio = testCount / changedPaths.length; - if (ratio >= 0.4) return "strong"; - if (ratio >= 0.2) return "adequate"; + return buildTestCoverageSummary(changedPaths).classification; +} + +export function buildTestCoverageSummary(changedPaths: string[]): TestCoverageSummary { + const uniquePaths = [...new Set(changedPaths.filter(Boolean))]; + const testPathCount = uniquePaths.filter(isTestPath).length; + const fixturePathCount = uniquePaths.filter((path) => isFixturePath(path) && !isTestPath(path)).length; + const evidencePathCount = testPathCount + fixturePathCount; + const sourcePathCount = uniquePaths.filter((path) => isCodeFile(path) && !isTestEvidencePath(path)).length; + const changedPathCount = uniquePaths.length; + const testToChangedRatio = changedPathCount === 0 ? 0 : roundRatio(evidencePathCount / changedPathCount); + const classification = classifyCoverageRatio(evidencePathCount, testToChangedRatio, sourcePathCount); + return { + classification, + changedPathCount, + sourcePathCount, + testPathCount, + fixturePathCount, + testToChangedRatio, + guidance: coverageGuidanceFor(classification, sourcePathCount, evidencePathCount), + }; +} + +export function coverageGuidanceFor( + classification: TestCoverageClassification, + sourcePathCount: number, + evidencePathCount = 0, +): string { + if (sourcePathCount === 0) { + return evidencePathCount > 0 + ? "Only test or fixture paths changed; no source-code accompaniment is expected." + : "No source-code paths changed; test coverage guidance does not apply."; + } + switch (classification) { + case "strong": + return "Test or fixture changes are proportionally strong for the source files touched."; + case "adequate": + return "Some focused tests or fixtures accompany the source changes; consider adding edge-case coverage if the diff is broad."; + case "weak": + return "Source changes outnumber test evidence; add focused regression tests or fixtures for the touched modules."; + case "absent": + default: + return "Code changes lack accompanying test files or fixtures; add focused regression coverage or explain why existing tests suffice."; + } +} + +function classifyCoverageRatio( + evidencePathCount: number, + testToChangedRatio: number, + sourcePathCount: number, +): TestCoverageClassification { + if (sourcePathCount === 0) return evidencePathCount > 0 ? "strong" : "absent"; + if (evidencePathCount === 0) return "absent"; + if (testToChangedRatio >= TEST_COVERAGE_STRONG_RATIO) return "strong"; + if (testToChangedRatio >= TEST_COVERAGE_ADEQUATE_RATIO) return "adequate"; return "weak"; } + +function roundRatio(value: number): number { + if (!Number.isFinite(value)) return 0; + return Math.round(value * 1000) / 1000; +} diff --git a/test/unit/contributor-open-pr-monitor.test.ts b/test/unit/contributor-open-pr-monitor.test.ts index cc1e105cc3..84d4f148de 100644 --- a/test/unit/contributor-open-pr-monitor.test.ts +++ b/test/unit/contributor-open-pr-monitor.test.ts @@ -70,7 +70,7 @@ describe("contributor open PR monitor", () => { reviews: [approvedReview(1)], checks: [], }); - expect(mapPendingClassToWorkClassification(approved, { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false })).toBe("approved"); + expect(mapPendingClassToWorkClassification(approved, { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false, weakTestCoverage: false })).toBe("approved"); const failing = classifyOpenPullRequest({ pr: pr({ number: 2 }), @@ -78,7 +78,7 @@ describe("contributor open PR monitor", () => { reviews: [approvedReview(2)], checks: [{ id: "c1", repoFullName: "entrius/allways-ui", pullNumber: 2, name: "ci", status: "completed", conclusion: "failure", payload: {} }], }); - expect(mapPendingClassToWorkClassification(failing, { changeRequestCount: 0, checkFailureCount: 1, duplicateProne: false, missingTests: false })).toBe("failing_checks"); + expect(mapPendingClassToWorkClassification(failing, { changeRequestCount: 0, checkFailureCount: 1, duplicateProne: false, missingTests: false, weakTestCoverage: false })).toBe("failing_checks"); const needsAuthor = classifyOpenPullRequest({ pr: pr({ number: 3 }), @@ -86,7 +86,7 @@ describe("contributor open PR monitor", () => { reviews: [{ ...approvedReview(3), state: "CHANGES_REQUESTED" }], checks: [], }); - expect(mapPendingClassToWorkClassification(needsAuthor, { changeRequestCount: 1, checkFailureCount: 0, duplicateProne: false, missingTests: false })).toBe("needs_author"); + expect(mapPendingClassToWorkClassification(needsAuthor, { changeRequestCount: 1, checkFailureCount: 0, duplicateProne: false, missingTests: false, weakTestCoverage: false })).toBe("needs_author"); const staleDate = new Date(Date.now() - 20 * 86_400_000).toISOString(); const stale = classifyOpenPullRequest({ @@ -95,33 +95,40 @@ describe("contributor open PR monitor", () => { reviews: [approvedReview(4)], checks: [], }); - expect(mapPendingClassToWorkClassification(stale, { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false })).toBe("should_close_or_withdraw"); + expect(mapPendingClassToWorkClassification(stale, { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false, weakTestCoverage: false })).toBe("should_close_or_withdraw"); expect( mapPendingClassToWorkClassification( classifyOpenPullRequest({ pr: pr({ number: 5, title: "fix overlap" }), roleContext: outsideContributorRole, reviews: [approvedReview(5)], checks: [] }), - { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: true, missingTests: false }, + { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: true, missingTests: false, weakTestCoverage: false }, ), ).toBe("duplicate_prone"); expect( mapPendingClassToWorkClassification( classifyOpenPullRequest({ pr: pr({ number: 6 }), roleContext: outsideContributorRole, reviews: [approvedReview(6)], checks: [] }), - { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: true }, + { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: true, weakTestCoverage: false }, ), ).toBe("missing_tests"); + expect( + mapPendingClassToWorkClassification( + classifyOpenPullRequest({ pr: pr({ number: 14 }), roleContext: outsideContributorRole, reviews: [approvedReview(14)], checks: [] }), + { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false, weakTestCoverage: true }, + ), + ).toBe("weak_test_coverage"); + expect( mapPendingClassToWorkClassification( classifyOpenPullRequest({ pr: pr({ number: 7, authorAssociation: "OWNER" }), roleContext: outsideContributorRole, reviews: [approvedReview(7)], checks: [] }), - { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false }, + { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false, weakTestCoverage: false }, ), ).toBe("maintainer_lane"); expect( mapPendingClassToWorkClassification( classifyOpenPullRequest({ pr: pr({ number: 8 }), roleContext: maintainerRole, reviews: [approvedReview(8)], checks: [] }), - { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false }, + { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false, weakTestCoverage: false }, ), ).toBe("maintainer_lane"); @@ -131,15 +138,15 @@ describe("contributor open PR monitor", () => { reviews: [], checks: [], }); - expect(mapPendingClassToWorkClassification(draft, { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false })).toBe("draft"); + expect(mapPendingClassToWorkClassification(draft, { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false, weakTestCoverage: false })).toBe("draft"); const blocked = classifyOpenPullRequest({ pr: pr({ number: 12 }), roleContext: outsideContributorRole, reviews: [], checks: [] }); - expect(mapPendingClassToWorkClassification(blocked, { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false })).toBe("blocked"); + expect(mapPendingClassToWorkClassification(blocked, { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false, weakTestCoverage: false })).toBe("blocked"); expect( mapPendingClassToWorkClassification( { repoFullName: "entrius/allways-ui", number: 13, title: "mystery", classification: "unknown" as never, reasons: [] }, - { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false }, + { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false, weakTestCoverage: false }, ), ).toBe("reviewable"); }); @@ -151,7 +158,7 @@ describe("contributor open PR monitor", () => { reviews: [], checks: [], }); - expect(mapPendingClassToWorkClassification(nativeDraft, { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false })).toBe("draft"); + expect(mapPendingClassToWorkClassification(nativeDraft, { changeRequestCount: 0, checkFailureCount: 0, duplicateProne: false, missingTests: false, weakTestCoverage: false })).toBe("draft"); }); it("builds contributor-wide monitor answer from registered repos only", async () => { @@ -242,6 +249,7 @@ describe("contributor open PR monitor", () => { [], false, false, + false, ); expect(packet.nextSteps.join(" ")).not.toMatch(/\/Users\/|\/home\/|upload source/i); }); @@ -254,6 +262,7 @@ describe("contributor open PR monitor", () => { [{ id: "1", repoFullName: "entrius/allways-ui", pullNumber: 55, name: "ci", status: "failure", conclusion: null, payload: {} }], false, false, + false, ); expect(statusCarried.classification).toBe("failing_checks"); expect(statusCarried.nextSteps.join(" ")).toContain("Fix failing checks"); @@ -264,6 +273,7 @@ describe("contributor open PR monitor", () => { [{ id: "2", repoFullName: "entrius/allways-ui", pullNumber: 55, name: "ci", status: "completed", conclusion: "startup_failure", payload: {} }], false, false, + false, ); expect(startupFailure.classification).toBe("failing_checks"); }); @@ -449,4 +459,52 @@ describe("contributor open PR monitor", () => { expect(monitor.pullRequests[0]?.classification).toBe("needs_author"); expect(monitor.pullRequests[1]?.classification).toBe("approved"); }); + + it("classifies disproportionately weak test coverage separately from fully missing tests", async () => { + const env = createTestEnv(); + vi.spyOn(repositories, "listRepositories").mockResolvedValue([ + { fullName: "entrius/allways-ui", owner: "entrius", name: "allways-ui", isInstalled: true, isRegistered: true, isPrivate: false }, + ] as Awaited>); + vi.spyOn(repositories, "listContributorPullRequests").mockResolvedValue([pr({ number: 60 })]); + vi.spyOn(repositories, "listPullRequests").mockResolvedValue([pr({ number: 60 })]); + vi.spyOn(repositories, "listPullRequestReviews").mockResolvedValue([approvedReview(60)]); + vi.spyOn(repositories, "listCheckSummaries").mockResolvedValue([]); + vi.spyOn(repositories, "listPullRequestFiles").mockResolvedValue([ + ...Array.from({ length: 9 }, (_, index) => ({ + repoFullName: "entrius/allways-ui", + pullNumber: 60, + path: `src/file${index}.ts`, + additions: 12, + deletions: 0, + changes: 12, + payload: {}, + })), + { + repoFullName: "entrius/allways-ui", + pullNumber: 60, + path: "test/single.test.ts", + additions: 8, + deletions: 0, + changes: 8, + payload: {}, + }, + ]); + + const monitor = await buildContributorOpenPrMonitor(env, "miner-a"); + expect(monitor.pullRequests[0]?.classification).toBe("weak_test_coverage"); + expect(monitor.pullRequests[0]?.nextSteps.join(" ")).toMatch(/Broaden test or fixture coverage/i); + }); + + it("derives coverage classification from cached PR file paths", () => { + const { testCoverageClassificationFromFiles, weakTestCoverageFromFiles, missingTestsFromFiles } = __contributorOpenPrMonitorInternals; + const weakFiles = [ + ...Array.from({ length: 9 }, (_, index) => ({ repoFullName: "entrius/allways-ui", pullNumber: 1, path: `src/file${index}.ts`, additions: 1, deletions: 0, changes: 1, payload: {} })), + { repoFullName: "entrius/allways-ui", pullNumber: 1, path: "test/single.test.ts", additions: 1, deletions: 0, changes: 1, payload: {} }, + ]; + expect(testCoverageClassificationFromFiles(weakFiles)).toBe("weak"); + expect(weakTestCoverageFromFiles(weakFiles)).toBe(true); + expect(missingTestsFromFiles(weakFiles)).toBe(false); + expect(testCoverageClassificationFromFiles([])).toBeNull(); + expect(missingTestsFromFiles([])).toBe(false); + }); }); diff --git a/test/unit/local-workspace-intelligence.test.ts b/test/unit/local-workspace-intelligence.test.ts index 5b2511d3a9..58030929c7 100644 --- a/test/unit/local-workspace-intelligence.test.ts +++ b/test/unit/local-workspace-intelligence.test.ts @@ -313,4 +313,38 @@ describe("local workspace intelligence v2", () => { expect(intelligence.linkedIssues).toEqual([7, 19, 42]); }); + + it("reports test coverage classification alongside test evidence level", () => { + const sources = Array.from({ length: 9 }, (_, index) => ({ path: `src/file${index}.ts`, status: "modified" as const })); + const intelligence = buildLocalWorkspaceIntelligence({ + input: { + login: "oktofeesh1", + repoFullName: repo.fullName, + changedFiles: [...sources, { path: "test/single.test.ts", status: "added" }], + }, + analysis: { + baseFreshness: { status: "fresh", changedFileCount: 10, testFileCount: 1, passedValidationCount: 0, warnings: [] }, + branchQualityBlockers: [], + accountStateBlockers: [], + recommendedRerunCondition: "Rerun after any branch, base, or PR state changes before opening/submitting.", + prPacket: { + titleSuggestion: "Broad service refactor", + markdown: "# Broad service refactor\n", + bodySections: [], + reviewerNotes: [], + validationSummary: { passed: 0, failed: 0, notRun: 0, commands: [] }, + publicSafeWarnings: [], + }, + }, + changedFiles: [...sources, { path: "test/single.test.ts", status: "added" }], + }); + + expect(intelligence.testEvidence.level).toBe("test_files"); + expect(intelligence.testCoverage).toMatchObject({ + classification: "weak", + sourcePathCount: 9, + testPathCount: 1, + guidance: expect.stringMatching(/outnumber test evidence/i), + }); + }); }); diff --git a/test/unit/slop.test.ts b/test/unit/slop.test.ts index d43dc895d6..029e0243d8 100644 --- a/test/unit/slop.test.ts +++ b/test/unit/slop.test.ts @@ -7,6 +7,7 @@ import { buildIssueSlopAssessment, buildLowQualityCommitMessageFinding, buildMissingTestEvidenceFinding, + buildWeakTestCoverageFinding, buildNoLinkedIssueRationaleFinding, buildNonSubstantivePaddingFinding, buildSlopAssessment, @@ -391,6 +392,57 @@ describe("buildMissingTestEvidenceFinding", () => { }); }); +describe("buildWeakTestCoverageFinding", () => { + it("fires when source changes are accompanied by disproportionately weak test evidence", () => { + const sources = Array.from({ length: 9 }, (_, index) => ({ path: `src/file${index}.ts`, additions: 12, deletions: 0 })); + const finding = buildWeakTestCoverageFinding({ + changedFiles: [...sources, { path: "test/single.test.ts", additions: 8, deletions: 0 }], + description: "Broad refactor across service modules.", + }); + + expect(finding).toMatchObject({ + code: "weak_test_coverage", + severity: "info", + title: "Test coverage is disproportionately weak", + }); + expect(JSON.stringify(finding)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); + + it("does not fire when coverage is absent (missing_test_evidence owns that case)", () => { + expect( + buildWeakTestCoverageFinding({ + changedFiles: [{ path: "src/api/routes.ts", additions: 20, deletions: 0 }], + }), + ).toBeNull(); + }); + + it("does not fire when coverage is adequate or strong", () => { + expect( + buildWeakTestCoverageFinding({ + changedFiles: [ + { path: "src/a.ts", additions: 10, deletions: 0 }, + { path: "src/b.ts", additions: 10, deletions: 0 }, + { path: "src/c.ts", additions: 10, deletions: 0 }, + { path: "test/a.test.ts", additions: 12, deletions: 0 }, + ], + }), + ).toBeNull(); + }); + + it("adds weak coverage weight without making missing-test-only diffs blockable alone", () => { + const result = buildSlopAssessment({ + changedFiles: [ + ...Array.from({ length: 9 }, (_, index) => ({ path: `src/file${index}.ts`, additions: 12, deletions: 0 })), + { path: "test/single.test.ts", additions: 8, deletions: 0 }, + ], + description: "Broad refactor across service modules.", + }); + expect(result.findings.map((finding) => finding.code)).toContain("weak_test_coverage"); + expect(result.slopRisk).toBe(SLOP_WEIGHTS.weakTestCoverage); + expect(result.band).toBe("low"); + }); +}); + describe("buildTrivialWhitespaceChurnFinding", () => { it("keeps public reason strings sanitized", () => { const finding = buildTrivialWhitespaceChurnFinding({ diff --git a/test/unit/test-evidence.test.ts b/test/unit/test-evidence.test.ts index f0997c4b5b..8ca9dd64c9 100644 --- a/test/unit/test-evidence.test.ts +++ b/test/unit/test-evidence.test.ts @@ -1,5 +1,15 @@ import { describe, expect, it } from "vitest"; -import { classifyTestCoverage, hasLocalTestEvidence, isTestPath } from "../../src/signals/test-evidence"; +import { + buildTestCoverageSummary, + classifyTestCoverage, + coverageGuidanceFor, + hasLocalTestEvidence, + isFixturePath, + isTestEvidencePath, + isTestPath, + TEST_COVERAGE_ADEQUATE_RATIO, + TEST_COVERAGE_STRONG_RATIO, +} from "../../src/signals/test-evidence"; describe("test evidence helpers", () => { it("detects common test path conventions", () => { @@ -17,6 +27,18 @@ describe("test evidence helpers", () => { expect(isTestPath("src/widget.rs")).toBe(false); }); + it("detects fixture, mock, and test-data directories as test evidence", () => { + expect(isFixturePath("test/fixtures/pr.json")).toBe(true); + expect(isFixturePath("src/__fixtures__/payload.ts")).toBe(true); + expect(isFixturePath("testdata/input.yaml")).toBe(true); + expect(isFixturePath("test-data/sample.json")).toBe(true); + expect(isFixturePath("mocks/github.ts")).toBe(true); + expect(isFixturePath("__mocks__/client.ts")).toBe(true); + expect(isFixturePath("src/widget.ts")).toBe(false); + expect(isTestEvidencePath("test/fixtures/pr.json")).toBe(true); + expect(isTestEvidencePath("src/widget.ts")).toBe(false); + }); + it("does not treat framework or integration directory names alone as test evidence", () => { expect(isTestPath("src/integration/auth.ts")).toBe(false); expect(isTestPath("src/playwright/client.ts")).toBe(false); @@ -27,11 +49,17 @@ describe("test evidence helpers", () => { expect(isTestPath("src/cypress/client.cy.ts")).toBe(true); }); - it("treats explicit test file lists as evidence", () => { + it("treats explicit test file lists and fixture paths as evidence", () => { expect(hasLocalTestEvidence({ testFiles: ["internal/cache_test.go"] })).toBe(true); + expect(hasLocalTestEvidence({ testFiles: ["test/fixtures/payload.json"] })).toBe(true); expect(hasLocalTestEvidence({ tests: [] })).toBe(false); expect(hasLocalTestEvidence({})).toBe(false); }); + + it("documents stable ratio thresholds for coverage classification", () => { + expect(TEST_COVERAGE_STRONG_RATIO).toBe(0.4); + expect(TEST_COVERAGE_ADEQUATE_RATIO).toBe(0.2); + }); }); describe("classifyTestCoverage", () => { @@ -44,19 +72,58 @@ describe("classifyTestCoverage", () => { }); it("classifies >= 40% test ratio as strong", () => { - // 2 source + 2 test = 50% expect(classifyTestCoverage(["src/a.ts", "src/b.ts", "test/a.test.ts", "test/b.test.ts"])).toBe("strong"); expect(classifyTestCoverage(["src/a.ts", "src/b.ts", "e2e/a.spec.ts", "e2e/b.spec.ts"])).toBe("strong"); }); it("classifies 20%–39% test ratio as adequate", () => { - // 3 source + 1 test = 25% expect(classifyTestCoverage(["src/a.ts", "src/b.ts", "src/c.ts", "test/a.test.ts"])).toBe("adequate"); }); it("classifies > 0% but < 20% test ratio as weak", () => { - // 9 source + 1 test ≈ 10% const sources = Array.from({ length: 9 }, (_, i) => `src/file${i}.ts`); expect(classifyTestCoverage([...sources, "test/single.test.ts"])).toBe("weak"); }); + + it("counts fixture directories toward coverage without requiring a test suffix", () => { + const sources = Array.from({ length: 5 }, (_, i) => `src/file${i}.ts`); + expect(classifyTestCoverage([...sources, "test/fixtures/payload.json"])).toBe("weak"); + expect(classifyTestCoverage([...sources, "test/a.test.ts", "test/fixtures/payload.json"])).toBe("adequate"); + }); +}); + +describe("buildTestCoverageSummary", () => { + it("returns guidance that stays public-safe and actionable", () => { + const summary = buildTestCoverageSummary(["src/a.ts", "src/b.ts", "src/c.ts", "test/a.test.ts"]); + expect(summary).toMatchObject({ + classification: "adequate", + changedPathCount: 4, + sourcePathCount: 3, + testPathCount: 1, + fixturePathCount: 0, + testToChangedRatio: 0.25, + }); + expect(summary.guidance).toMatch(/Some focused tests/i); + expect(JSON.stringify(summary)).not.toMatch(/wallet|hotkey|payout|trust score/i); + }); + + it("treats test-only diffs as strong without source accompaniment", () => { + const summary = buildTestCoverageSummary(["test/unit/cache.test.ts", "test/fixtures/cache.json"]); + expect(summary.classification).toBe("strong"); + expect(summary.sourcePathCount).toBe(0); + expect(coverageGuidanceFor("strong", 0, 2)).toMatch(/Only test or fixture paths changed/i); + }); + + it("returns absent guidance when code changes have no test evidence", () => { + const summary = buildTestCoverageSummary(["src/auth.ts", "src/utils.ts", "README.md"]); + expect(summary.classification).toBe("absent"); + expect(summary.guidance).toMatch(/lack accompanying test files/i); + expect(coverageGuidanceFor("absent", 2)).toMatch(/lack accompanying test files/i); + }); + + it("deduplicates repeated paths before counting", () => { + const summary = buildTestCoverageSummary(["src/a.ts", "src/a.ts", "src/b.ts", "src/c.ts", "test/a.test.ts"]); + expect(summary.changedPathCount).toBe(4); + expect(summary.classification).toBe("adequate"); + }); });