diff --git a/src/api/routes.ts b/src/api/routes.ts index 3ca414ce50..2e4c545787 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -414,6 +414,8 @@ const slopRiskSchema = z.object({ tests: z.array(z.string().max(400)).max(2000).optional(), testFiles: z.array(z.string().max(400)).max(2000).optional(), commitMessages: z.array(z.string().max(2000)).max(200).optional(), + hasLinkedIssue: z.boolean().optional(), + issueDiscoveryLane: z.boolean().optional(), }); const issueSlopSchema = z.object({ title: z.string().max(500).optional(), diff --git a/src/mcp/server.ts b/src/mcp/server.ts index eee8d411d0..fd872a8e67 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -651,6 +651,8 @@ const checkSlopRiskShape = { tests: z.array(z.string().max(400)).max(2000).optional(), testFiles: z.array(z.string().max(400)).max(2000).optional(), commitMessages: z.array(z.string().max(2000)).max(200).optional(), + hasLinkedIssue: z.boolean().optional(), + issueDiscoveryLane: z.boolean().optional(), }; const checkSlopRiskOutputSchema = { diff --git a/src/signals/engine.ts b/src/signals/engine.ts index c541ebcc29..2fe73b76e8 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -4470,7 +4470,9 @@ export function buildPrTextLint(input: PrTextLintInput): PrTextLintReport { }; } -function hasClearNoIssueRationale(pr: Pick): boolean { +// Exported so the deterministic no-linked-issue slop signal (#562) and the public PR-panel traceability check +// share ONE definition of a "clear no-issue rationale" (maintenance / docs-only / "no issue: …" in the PR text). +export function hasClearNoIssueRationale(pr: Pick): boolean { return /\b(no issue\s*(?:because|:)|no linked issue\s*(?:because|:)|no ticket\s*(?:because|:)|maintenance|docs? only|typo|chore|cleanup)\b/i.test([pr.title, pr.body ?? ""].join(" ")); } diff --git a/src/signals/slop.ts b/src/signals/slop.ts index c72812235e..404f090327 100644 --- a/src/signals/slop.ts +++ b/src/signals/slop.ts @@ -1,4 +1,4 @@ -import { GENERIC_COMMIT_PATTERN, type SignalFinding } from "./engine"; +import { GENERIC_COMMIT_PATTERN, hasClearNoIssueRationale, type SignalFinding } from "./engine"; import { isCodeFile, isTestFile } from "./local-branch"; import { hasLocalTestEvidence, isTestPath } from "./test-evidence"; import { isFocusManifestPublicSafe } from "./focus-manifest"; @@ -23,6 +23,12 @@ export type SlopAssessmentInput = { /** True when this PR sits in a high-risk duplicate cluster (2+ open PRs) — the caller computes it from the * collision report via {@link isPullRequestInDuplicateCluster}. Undefined on surfaces without repo context. */ inDuplicateCluster?: boolean | undefined; + /** Whether this PR links at least one issue (caller computes from `linkedIssues.length > 0`). Only an explicit + * `false` can trip the no-linked-issue-without-rationale signal; undefined means the surface has no issue data. */ + hasLinkedIssue?: boolean | undefined; + /** True when the contributor/repo is in the issue-discovery lane, where PRs without a linked issue are expected + * and so the no-linked-issue-without-rationale signal does not apply. */ + issueDiscoveryLane?: boolean | undefined; }; export type SlopAssessment = { @@ -42,6 +48,7 @@ export const SLOP_WEIGHTS = { emptyDescription: 15, lowQualityCommitMessage: 15, duplicateClusterMembership: 15, + noLinkedIssueWithoutRationale: 15, } as const; export const SLOP_RUBRIC_MARKDOWN = [ @@ -59,6 +66,7 @@ export const SLOP_RUBRIC_MARKDOWN = [ "- empty pull request description on a code change", "- generic or empty commit message", "- duplicate / overlapping pull request (high-risk collision cluster)", + "- no linked issue and no rationale (outside the issue-discovery lane)", ].join("\n"); const MIN_CHURN_LINES = 40; @@ -75,12 +83,14 @@ export function buildSlopAssessment(input: SlopAssessmentInput): SlopAssessment const emptyDescriptionFinding = buildEmptyDescriptionFinding(input); const lowQualityCommitMessageFinding = buildLowQualityCommitMessageFinding(input); const duplicateClusterFinding = buildDuplicateClusterFinding(input); + const noLinkedIssueRationaleFinding = buildNoLinkedIssueRationaleFinding(input); if (trivialChurnFinding) findings.push(trivialChurnFinding); if (missingTestEvidenceFinding) findings.push(missingTestEvidenceFinding); if (nonSubstantivePaddingFinding) findings.push(nonSubstantivePaddingFinding); if (emptyDescriptionFinding) findings.push(emptyDescriptionFinding); if (lowQualityCommitMessageFinding) findings.push(lowQualityCommitMessageFinding); if (duplicateClusterFinding) findings.push(duplicateClusterFinding); + if (noLinkedIssueRationaleFinding) findings.push(noLinkedIssueRationaleFinding); const slopRisk = clamp( (trivialChurnFinding ? SLOP_WEIGHTS.trivialWhitespaceChurn : 0) + @@ -88,7 +98,8 @@ export function buildSlopAssessment(input: SlopAssessmentInput): SlopAssessment (nonSubstantivePaddingFinding ? SLOP_WEIGHTS.nonSubstantivePadding : 0) + (emptyDescriptionFinding ? SLOP_WEIGHTS.emptyDescription : 0) + (lowQualityCommitMessageFinding ? SLOP_WEIGHTS.lowQualityCommitMessage : 0) + - (duplicateClusterFinding ? SLOP_WEIGHTS.duplicateClusterMembership : 0), + (duplicateClusterFinding ? SLOP_WEIGHTS.duplicateClusterMembership : 0) + + (noLinkedIssueRationaleFinding ? SLOP_WEIGHTS.noLinkedIssueWithoutRationale : 0), 0, 100, ); @@ -208,6 +219,26 @@ export function buildDuplicateClusterFinding(input: SlopAssessmentInput): Signal }; } +// Fires when the caller reports NO linked issue (#562), the PR body carries no clear no-issue rationale, and the +// repo is not in the issue-discovery lane (where unlinked PRs are expected). High-precision: only an explicit +// `hasLinkedIssue: false` trips it — absent data (undefined) is not a signal — and any clear rationale +// (maintenance / docs-only / "no issue: …") clears it. Reuses engine.ts `hasClearNoIssueRationale` so this signal +// and the public PR-panel traceability check agree on what counts as a rationale. Static, public-safe text. +export function buildNoLinkedIssueRationaleFinding(input: SlopAssessmentInput): SignalFinding | null { + if (input.hasLinkedIssue !== false) return null; + if (input.issueDiscoveryLane === true) return null; + if (hasClearNoIssueRationale({ title: "", body: input.description ?? "" })) return null; + const detail = "This pull request links no issue and gives no rationale for working without one."; + return { + code: "no_linked_issue_without_rationale", + title: "No linked issue and no rationale", + severity: "warning", + detail, + action: "Link the issue this addresses, or explain in the description why no issue applies (e.g. a typo, docs-only, or maintenance change).", + publicText: detail, + }; +} + export function buildMissingTestEvidenceFinding(input: SlopAssessmentInput): SignalFinding | null { const changedFiles = input.changedFiles ?? []; const changedPaths = changedFiles.map((file) => file.path).filter(Boolean); diff --git a/test/unit/slop.test.ts b/test/unit/slop.test.ts index 1b64d47c17..600dacbebe 100644 --- a/test/unit/slop.test.ts +++ b/test/unit/slop.test.ts @@ -5,6 +5,7 @@ import { buildIssueSlopAssessment, buildLowQualityCommitMessageFinding, buildMissingTestEvidenceFinding, + buildNoLinkedIssueRationaleFinding, buildNonSubstantivePaddingFinding, buildSlopAssessment, buildTrivialWhitespaceChurnFinding, @@ -23,6 +24,7 @@ describe("buildSlopAssessment", () => { expect(SLOP_RUBRIC_MARKDOWN).toContain("missing test evidence"); expect(SLOP_RUBRIC_MARKDOWN).toContain("trivial / whitespace-only churn"); expect(SLOP_RUBRIC_MARKDOWN).toContain("generic or empty commit message"); + expect(SLOP_RUBRIC_MARKDOWN).toContain("no linked issue and no rationale"); const clean = buildSlopAssessment({}); expect(clean).toEqual({ slopRisk: 0, band: "clean", findings: [] }); @@ -60,6 +62,28 @@ describe("buildSlopAssessment", () => { expect(buildLowQualityCommitMessageFinding({ commitMessages: ["", "update"] })?.detail).toMatch(/generic/i); }); + it("raises no-linked-issue-without-rationale slop when there is no issue and no rationale (#562)", () => { + const result = buildSlopAssessment({ hasLinkedIssue: false }); + expect(result.slopRisk).toBe(SLOP_WEIGHTS.noLinkedIssueWithoutRationale); + expect(result.band).toBe("low"); + expect(result.findings).toEqual([expect.objectContaining({ code: "no_linked_issue_without_rationale", severity: "warning" })]); + expect(JSON.stringify(result)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); + + it("does not raise no-linked-issue slop when an issue is linked, a rationale is present, the lane is issue-discovery, or no data is supplied (#562)", () => { + expect(buildSlopAssessment({ hasLinkedIssue: true }).findings).toEqual([]); + expect(buildSlopAssessment({ hasLinkedIssue: false, description: "Docs only: fix a typo in the README." }).findings).toEqual([]); + expect(buildSlopAssessment({ hasLinkedIssue: false, issueDiscoveryLane: true }).findings).toEqual([]); + expect(buildSlopAssessment({}).findings).toEqual([]); + }); + + it("reuses the shared no-issue rationale helper and treats absent linked-issue data as no signal (#562)", () => { + expect(buildNoLinkedIssueRationaleFinding({ hasLinkedIssue: undefined })).toBeNull(); + // a maintenance/cleanup rationale in the body clears the signal even with no linked issue + expect(buildNoLinkedIssueRationaleFinding({ hasLinkedIssue: false, description: "Routine maintenance; no issue needed." })).toBeNull(); + expect(buildNoLinkedIssueRationaleFinding({ hasLinkedIssue: false, description: "" })).toMatchObject({ code: "no_linked_issue_without_rationale" }); + }); + it("raises duplicate-cluster slop when the PR is flagged as in a duplicate cluster (#563)", () => { const result = buildSlopAssessment({ inDuplicateCluster: true }); expect(result.slopRisk).toBe(SLOP_WEIGHTS.duplicateClusterMembership);