diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 4a4ee74b9f..40a4b2b82c 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -1453,6 +1453,9 @@ async function maybePublishPrPublicSurface( const slop = buildSlopAssessment({ changedFiles: slopFiles.map((file) => ({ path: file.path, additions: file.additions, deletions: file.deletions })), description: pr.body, + // Reuse the collision report already built for this gate run so a duplicate-cluster PR is flagged (#563). + collisions, + pullNumber: pr.number, }); slopRisk = slop.slopRisk; advisory.findings.push(...slop.findings); diff --git a/src/signals/slop.ts b/src/signals/slop.ts index 1fbc260458..0beac80da3 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, type CollisionReport, type SignalFinding } from "./engine"; import { isCodeFile, isTestFile } from "./local-branch"; import { hasLocalTestEvidence, isTestPath } from "./test-evidence"; import { isFocusManifestPublicSafe } from "./focus-manifest"; @@ -20,6 +20,10 @@ export type SlopAssessmentInput = { 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; + /** The repo's collision report, paired with {@link pullNumber}, so an open PR sitting in a high-risk + * duplicate cluster (2+ open PRs) can be flagged. Both must be present for the signal to evaluate. */ + collisions?: CollisionReport | undefined; + pullNumber?: number | undefined; }; export type SlopAssessment = { @@ -38,6 +42,7 @@ export const SLOP_WEIGHTS = { nonSubstantivePadding: 30, emptyDescription: 15, lowQualityCommitMessage: 15, + duplicateClusterMembership: 15, } as const; export const SLOP_RUBRIC_MARKDOWN = [ @@ -54,6 +59,7 @@ export const SLOP_RUBRIC_MARKDOWN = [ "- non-substantive padding (generated / vendored / minified output as source)", "- empty pull request description on a code change", "- generic or empty commit message", + "- duplicate / overlapping pull request (high-risk collision cluster)", ].join("\n"); const MIN_CHURN_LINES = 40; @@ -69,18 +75,21 @@ export function buildSlopAssessment(input: SlopAssessmentInput): SlopAssessment const nonSubstantivePaddingFinding = buildNonSubstantivePaddingFinding(input); const emptyDescriptionFinding = buildEmptyDescriptionFinding(input); const lowQualityCommitMessageFinding = buildLowQualityCommitMessageFinding(input); + const duplicateClusterFinding = buildDuplicateClusterFinding(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); 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), + (lowQualityCommitMessageFinding ? SLOP_WEIGHTS.lowQualityCommitMessage : 0) + + (duplicateClusterFinding ? SLOP_WEIGHTS.duplicateClusterMembership : 0), 0, 100, ); @@ -183,6 +192,31 @@ export function buildLowQualityCommitMessageFinding(input: SlopAssessmentInput): }; } +// Fires when the PR sits in a HIGH-risk collision cluster that holds 2+ open pull requests — genuine +// overlapping/duplicate work — using the caller-supplied buildCollisionReport (#557). The 2+-PR bar is +// deliberate: buildCollisionReport also marks a healthy issue↔its-own-linking-PR pair as high-risk, so +// requiring two pull-request items keeps this blocking signal false-positive-averse. Static, public-safe text. +export function buildDuplicateClusterFinding(input: SlopAssessmentInput): SignalFinding | null { + const { collisions, pullNumber } = input; + if (collisions === undefined || pullNumber === undefined) return null; + const inHighRiskDuplicateCluster = collisions.clusters.some( + (cluster) => + cluster.risk === "high" && + cluster.items.filter((item) => item.type === "pull_request").length >= 2 && + cluster.items.some((item) => item.type === "pull_request" && item.number === pullNumber), + ); + if (!inHighRiskDuplicateCluster) return null; + const detail = "This pull request overlaps a high-risk cluster of other open pull requests doing similar work."; + return { + code: "duplicate_cluster_membership", + title: "Pull request duplicates other open work", + severity: "warning", + detail, + action: "Check for an existing pull request or issue covering this change and coordinate or consolidate before continuing.", + 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 363ebe6946..c3ea2739b7 100644 --- a/test/unit/slop.test.ts +++ b/test/unit/slop.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + buildDuplicateClusterFinding, buildEmptyIssueBodyFinding, buildIssueSlopAssessment, buildLowQualityCommitMessageFinding, @@ -16,6 +17,18 @@ import { const FORBIDDEN_PUBLIC_TERMS = /wallet|hotkey|coldkey|mnemonic|reward|payout|raw trust|trust score|scoreability|private reviewability|\/Users|\/home|\/tmp/i; +// Minimal CollisionReport fixtures for the duplicate-cluster signal (#563). +type ClusterRisk = "low" | "medium" | "high"; +const item = (type: "pull_request" | "issue", number: number) => ({ type, number, title: `item ${number}` }); +const pr = (number: number) => item("pull_request", number); +const issue = (number: number) => item("issue", number); +const collisionReport = (clusters: Array<{ id: string; risk: ClusterRisk; reason: string; items: ReturnType[] }>) => ({ + repoFullName: "owner/repo", + generatedAt: "2026-06-18T00:00:00.000Z", + summary: { clusterCount: clusters.length, highRiskCount: clusters.filter((cluster) => cluster.risk === "high").length, itemsReviewed: clusters.reduce((total, cluster) => total + cluster.items.length, 0) }, + clusters, +}); + describe("buildSlopAssessment", () => { it("exports rubric bands and a deterministic assessment shell", () => { expect(SLOP_RUBRIC_MARKDOWN).toContain("clean"); @@ -50,6 +63,44 @@ describe("buildSlopAssessment", () => { expect(buildLowQualityCommitMessageFinding({ commitMessages: ["", "update"] })?.detail).toMatch(/generic/i); }); + it("raises duplicate-cluster slop when the PR sits in a high-risk cluster with 2+ open PRs (#563)", () => { + const collisions = collisionReport([ + { id: "c1", risk: "high", reason: "overlap", items: [pr(7), pr(8), issue(3)] }, + ]); + const result = buildSlopAssessment({ collisions, pullNumber: 7 }); + expect(result.slopRisk).toBe(SLOP_WEIGHTS.duplicateClusterMembership); + expect(result.band).toBe("low"); + expect(result.findings).toEqual([expect.objectContaining({ code: "duplicate_cluster_membership", severity: "warning" })]); + expect(JSON.stringify(result)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); + + it("does not raise duplicate-cluster slop without a 2+-PR high-risk cluster containing this PR (#563)", () => { + // missing context → no signal + expect(buildDuplicateClusterFinding({})).toBeNull(); + expect(buildDuplicateClusterFinding({ collisions: collisionReport([]) })).toBeNull(); + // high-risk but only this PR + an issue (healthy linkage) → not a duplicate-PR cluster + expect(buildDuplicateClusterFinding({ collisions: collisionReport([{ id: "c", risk: "high", reason: "r", items: [pr(7), issue(3)] }]), pullNumber: 7 })).toBeNull(); + // two PRs but the cluster is not high-risk + expect(buildDuplicateClusterFinding({ collisions: collisionReport([{ id: "c", risk: "medium", reason: "r", items: [pr(7), pr(8)] }]), pullNumber: 7 })).toBeNull(); + // high-risk 2-PR cluster, but this PR is not a member + expect(buildDuplicateClusterFinding({ collisions: collisionReport([{ id: "c", risk: "high", reason: "r", items: [pr(8), pr(9)] }]), pullNumber: 7 })).toBeNull(); + }); + + it("stacks the duplicate-cluster weight with another signal into the expected band (#563)", () => { + const result = buildSlopAssessment({ + // code file with no test evidence → missing_test_evidence (30); non-empty description suppresses empty_description. + changedFiles: [{ path: "src/parser.ts", additions: 10, deletions: 1 }], + description: "Refactor the parser.", + // high-risk cluster of 2 open PRs including this one → duplicate_cluster_membership (15). + collisions: collisionReport([{ id: "c1", risk: "high", reason: "overlap", items: [pr(7), pr(8)] }]), + pullNumber: 7, + }); + expect(result.slopRisk).toBe(SLOP_WEIGHTS.missingTestEvidence + SLOP_WEIGHTS.duplicateClusterMembership); + expect(result.band).toBe("elevated"); + expect(result.findings.map((finding) => finding.code).sort()).toEqual(["duplicate_cluster_membership", "missing_test_evidence"]); + expect(JSON.stringify(result)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); + 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 }],