diff --git a/src/api/routes.ts b/src/api/routes.ts index 651dcbda64..3ca414ce50 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -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(), diff --git a/src/mcp/server.ts b/src/mcp/server.ts index cf77d60f82..eee8d411d0 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -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 = { diff --git a/src/signals/engine.ts b/src/signals/engine.ts index c2849785c0..80aed7f29a 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -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". diff --git a/src/signals/slop.ts b/src/signals/slop.ts index d2ea5b9dd0..1fbc260458 100644 --- a/src/signals/slop.ts +++ b/src/signals/slop.ts @@ -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"; @@ -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 = { @@ -35,6 +37,7 @@ export const SLOP_WEIGHTS = { missingTestEvidence: 30, nonSubstantivePadding: 30, emptyDescription: 15, + lowQualityCommitMessage: 15, } as const; export const SLOP_RUBRIC_MARKDOWN = [ @@ -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; @@ -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, ); @@ -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); diff --git a/test/unit/slop.test.ts b/test/unit/slop.test.ts index f595542c20..363ebe6946 100644 --- a/test/unit/slop.test.ts +++ b/test/unit/slop.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { buildEmptyIssueBodyFinding, buildIssueSlopAssessment, + buildLowQualityCommitMessageFinding, buildMissingTestEvidenceFinding, buildNonSubstantivePaddingFinding, buildSlopAssessment, @@ -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 }],