From 055e1f65d6101fc14823e39398e37c83b0e76322 Mon Sep 17 00:00:00 2001 From: oktofeesh1 <287075021+oktofeesh1@users.noreply.github.com> Date: Sat, 20 Jun 2026 09:20:09 -0700 Subject: [PATCH] =?UTF-8?q?feat(signals):=20slop=20signal=20=E2=80=94=20ge?= =?UTF-8?q?neric/empty=20commit=20message=20(#564)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the low-quality-commit-message deterministic slop signal (epic #530). 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 / "." ...). - Reuses the #549 lint tool's GENERIC_COMMIT_PATTERN (now exported from engine) so the gate and the lint advisory agree on what counts as "generic" — one source of truth, no drift. - High-precision / false-positive-averse (this score can gate): a specific subject never trips it, even one that isn't a Conventional Commit; only a bare generic word that IS the whole subject does. No commit data → no signal. - Weighted 15 (a secondary weak-effort signal, like empty-description); static, public-safe detail text. - Exposed on the two lint self-check surfaces that already mirror each other: POST /v1/lint/slop-risk (slopRiskSchema) and the gittensory_check_slop_risk MCP tool (checkSlopRiskShape), via an optional `commitMessages` field. Tests cover generic → fires, specific/none → clean, all-blank → empty, and first-non-blank primary selection. Full test:coverage green (branches 97.03%). Closes #564 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/api/routes.ts | 1 + src/mcp/server.ts | 1 + src/signals/engine.ts | 4 +++- src/signals/slop.ts | 32 ++++++++++++++++++++++++++++++-- test/unit/slop.test.ts | 24 ++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 3 deletions(-) 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 }],