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
1 change: 1 addition & 0 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ const slopRiskSchema = z.object({
description: z.string().max(20000).optional(),
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(),
});
const issueSlopSchema = z.object({
title: z.string().max(500).optional(),
Expand Down
1 change: 1 addition & 0 deletions src/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ const checkSlopRiskShape = {
description: z.string().max(20000).optional(),
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(),
};

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 @@ -4334,7 +4334,9 @@ export type PrTextLintReport = {
summary: string;
};

const GENERIC_COMMIT_PATTERN = /^(?:wip|fix(?:es|ed|ing)?|updat(?:e|es|ed|ing)|change[sd]?|edit[sd]?|patch|minor|tweak[sd]?|misc|cleanup|chore|stuff|temp|tmp|test|final|done|commit|asdf+|\.+)\b[\s.!]*$/i;
// Exported so the deterministic slop signal (#564) and the #549 lint tool share ONE definition of a
// "generic" commit subject — a single low-effort word (wip / fix / update / "." …) that is the whole subject.
export const GENERIC_COMMIT_PATTERN = /^(?:wip|fix(?:es|ed|ing)?|updat(?:e|es|ed|ing)|change[sd]?|edit[sd]?|patch|minor|tweak[sd]?|misc|cleanup|chore|stuff|temp|tmp|test|final|done|commit|asdf+|\.+)\b[\s.!]*$/i;
// Conventional Commit subject: one of CONTRIBUTING's allowed types, optional `(scope)`, optional `!`,
// then `: ` and a non-empty summary (e.g. `feat(api): add cursor pagination`). Single source of truth
// with CONTRIBUTING.md "Commit And PR Titles".
Expand Down
32 changes: 30 additions & 2 deletions src/signals/slop.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { SignalFinding } from "./engine";
import { GENERIC_COMMIT_PATTERN, type SignalFinding } from "./engine";
import { isCodeFile, isTestFile } from "./local-branch";
import { hasLocalTestEvidence, isTestPath } from "./test-evidence";
import { isFocusManifestPublicSafe } from "./focus-manifest";
Expand All @@ -18,6 +18,8 @@ export type SlopAssessmentInput = {
testFiles?: string[] | undefined;
/** PR/branch description. An empty/whitespace description on a code change is a weak-effort signal. */
description?: string | null | undefined;
/** The PR's commit subject line(s). A generic/empty primary subject (wip / fix / update / ".") is a weak-effort signal. */
commitMessages?: string[] | undefined;
};

export type SlopAssessment = {
Expand All @@ -35,6 +37,7 @@ export const SLOP_WEIGHTS = {
missingTestEvidence: 30,
nonSubstantivePadding: 30,
emptyDescription: 15,
lowQualityCommitMessage: 15,
} as const;

export const SLOP_RUBRIC_MARKDOWN = [
Expand All @@ -50,6 +53,7 @@ export const SLOP_RUBRIC_MARKDOWN = [
"- missing test evidence",
"- non-substantive padding (generated / vendored / minified output as source)",
"- empty pull request description on a code change",
"- generic or empty commit message",
].join("\n");

const MIN_CHURN_LINES = 40;
Expand All @@ -64,16 +68,19 @@ export function buildSlopAssessment(input: SlopAssessmentInput): SlopAssessment
const missingTestEvidenceFinding = buildMissingTestEvidenceFinding(input);
const nonSubstantivePaddingFinding = buildNonSubstantivePaddingFinding(input);
const emptyDescriptionFinding = buildEmptyDescriptionFinding(input);
const lowQualityCommitMessageFinding = buildLowQualityCommitMessageFinding(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);

const slopRisk = clamp(
(trivialChurnFinding ? SLOP_WEIGHTS.trivialWhitespaceChurn : 0) +
(missingTestEvidenceFinding ? SLOP_WEIGHTS.missingTestEvidence : 0) +
(nonSubstantivePaddingFinding ? SLOP_WEIGHTS.nonSubstantivePadding : 0) +
(emptyDescriptionFinding ? SLOP_WEIGHTS.emptyDescription : 0),
(emptyDescriptionFinding ? SLOP_WEIGHTS.emptyDescription : 0) +
(lowQualityCommitMessageFinding ? SLOP_WEIGHTS.lowQualityCommitMessage : 0),
0,
100,
);
Expand Down Expand Up @@ -155,6 +162,27 @@ export function buildEmptyDescriptionFinding(input: SlopAssessmentInput): Signal
};
}

// Fires when commit-message data is supplied and the primary subject is empty/whitespace, or is entirely a
// generic low-effort word (wip / fix / update / "." …) per the #549 lint tool's shared GENERIC_COMMIT_PATTERN.
// High-precision: a specific subject — even one that isn't a Conventional Commit — never trips this blocking
// signal; only a bare generic word that IS the whole subject does. Nothing to assess (undefined / no commit
// data) returns null. Static, public-safe detail text — no interpolation, like the issue-side findings.
export function buildLowQualityCommitMessageFinding(input: SlopAssessmentInput): SignalFinding | null {
if (input.commitMessages === undefined || input.commitMessages.length === 0) return null;
const messages = input.commitMessages.map((message) => message.trim()).filter((message) => message.length > 0);
const primary = messages[0];
if (primary !== undefined && !GENERIC_COMMIT_PATTERN.test(primary)) return null;
const detail = primary === undefined ? "The commit message is empty." : "The commit message is generic (e.g. wip / fix / update) with no specific detail.";
return {
code: "low_quality_commit_message",
title: "Commit message is generic or empty",
severity: "warning",
detail,
action: "Write a specific commit subject that names what changed and why (a Conventional Commit like 'feat(api): add cursor pagination' works well).",
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 @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
import {
buildEmptyIssueBodyFinding,
buildIssueSlopAssessment,
buildLowQualityCommitMessageFinding,
buildMissingTestEvidenceFinding,
buildNonSubstantivePaddingFinding,
buildSlopAssessment,
Expand All @@ -20,12 +21,35 @@ describe("buildSlopAssessment", () => {
expect(SLOP_RUBRIC_MARKDOWN).toContain("clean");
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");

const clean = buildSlopAssessment({});
expect(clean).toEqual({ slopRisk: 0, band: "clean", findings: [] });
expect(buildSlopAssessment({})).toEqual(clean);
});

it("raises low-quality-commit-message slop for a generic primary commit subject (#564)", () => {
const result = buildSlopAssessment({ commitMessages: ["wip"] });
expect(result.slopRisk).toBe(SLOP_WEIGHTS.lowQualityCommitMessage);
expect(result.band).toBe("low");
expect(result.findings).toEqual([expect.objectContaining({ code: "low_quality_commit_message", severity: "warning" })]);
expect(JSON.stringify(result)).not.toMatch(FORBIDDEN_PUBLIC_TERMS);
});

it("does not raise commit-message slop for a specific subject or when no commit data is supplied (#564)", () => {
expect(buildSlopAssessment({ commitMessages: ["feat(api): add cursor pagination to labels endpoint"] }).findings).toEqual([]);
expect(buildSlopAssessment({ commitMessages: [] }).findings).toEqual([]);
expect(buildSlopAssessment({}).findings).toEqual([]);
});

it("flags supplied-but-all-blank commit messages as empty, and uses the first non-blank as the primary subject (#564)", () => {
const empty = buildLowQualityCommitMessageFinding({ commitMessages: [" ", ""] });
expect(empty).toMatchObject({ code: "low_quality_commit_message" });
expect(empty?.detail).toMatch(/empty/i);
// leading blanks are skipped; the first real subject ("update") is what gets judged.
expect(buildLowQualityCommitMessageFinding({ commitMessages: ["", "update"] })?.detail).toMatch(/generic/i);
});

it("raises missing-test-evidence slop for code-only diffs without tests", () => {
const result = buildSlopAssessment({
changedFiles: [{ path: "src/registry/sync.ts", additions: 24, deletions: 2 }],
Expand Down
Loading