Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 2 additions & 0 deletions src/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
4 changes: 3 additions & 1 deletion src/signals/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4470,7 +4470,9 @@ export function buildPrTextLint(input: PrTextLintInput): PrTextLintReport {
};
}

function hasClearNoIssueRationale(pr: Pick<PullRequestRecord, "title" | "body">): 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<PullRequestRecord, "title" | "body">): 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(" "));
}

Expand Down
35 changes: 33 additions & 2 deletions src/signals/slop.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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 = {
Expand All @@ -42,6 +48,7 @@ export const SLOP_WEIGHTS = {
emptyDescription: 15,
lowQualityCommitMessage: 15,
duplicateClusterMembership: 15,
noLinkedIssueWithoutRationale: 15,
} as const;

export const SLOP_RUBRIC_MARKDOWN = [
Expand All @@ -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;
Expand All @@ -75,20 +83,23 @@ 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) +
(missingTestEvidenceFinding ? SLOP_WEIGHTS.missingTestEvidence : 0) +
(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,
);
Expand Down Expand Up @@ -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);
Expand Down
24 changes: 24 additions & 0 deletions test/unit/slop.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
buildIssueSlopAssessment,
buildLowQualityCommitMessageFinding,
buildMissingTestEvidenceFinding,
buildNoLinkedIssueRationaleFinding,
buildNonSubstantivePaddingFinding,
buildSlopAssessment,
buildTrivialWhitespaceChurnFinding,
Expand All @@ -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: [] });
Expand Down Expand Up @@ -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);
Expand Down
Loading