Skip to content
Closed
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
3 changes: 3 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
38 changes: 36 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, type CollisionReport, type SignalFinding } from "./engine";
import { isCodeFile, isTestFile } from "./local-branch";
import { hasLocalTestEvidence, isTestPath } from "./test-evidence";
import { isFocusManifestPublicSafe } from "./focus-manifest";
Expand All @@ -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 = {
Expand All @@ -38,6 +42,7 @@ export const SLOP_WEIGHTS = {
nonSubstantivePadding: 30,
emptyDescription: 15,
lowQualityCommitMessage: 15,
duplicateClusterMembership: 15,
} as const;

export const SLOP_RUBRIC_MARKDOWN = [
Expand All @@ -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;
Expand All @@ -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,
);
Expand Down Expand Up @@ -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);
Expand Down
51 changes: 51 additions & 0 deletions test/unit/slop.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, it } from "vitest";
import {
buildDuplicateClusterFinding,
buildEmptyIssueBodyFinding,
buildIssueSlopAssessment,
buildLowQualityCommitMessageFinding,
Expand All @@ -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<typeof item>[] }>) => ({
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");
Expand Down Expand Up @@ -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 }],
Expand Down
Loading