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
3 changes: 3 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ import {
buildBurdenForecast,
buildCollisionEdges,
buildCollisionReport,
isPullRequestInDuplicateCluster,
buildConfigQuality,
buildContributorFit,
buildContributorOutcomeHistory,
Expand Down Expand Up @@ -1453,6 +1454,8 @@ 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).
inDuplicateCluster: isPullRequestInDuplicateCluster(collisions, pr.number),
});
slopRisk = slop.slopRisk;
advisory.findings.push(...slop.findings);
Expand Down
15 changes: 15 additions & 0 deletions src/signals/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,21 @@ export function buildCollisionReport(
return report;
}

/**
* True when an open PR sits in a HIGH-risk collision cluster that holds 2+ pull requests — i.e. genuine
* overlapping/duplicate work (#563). The 2+-pull-request bar is deliberate: buildCollisionReport also marks a
* healthy issue↔its-own-linking-PR pair high-risk, so requiring two pull-request items keeps callers (the
* deterministic slop gate) false-positive-averse. Pure.
*/
export function isPullRequestInDuplicateCluster(collisions: CollisionReport, pullNumber: number): boolean {
return 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),
);
}

export function buildQueueHealth(
repo: RepositoryRecord | null,
issues: IssueRecord[],
Expand Down
27 changes: 26 additions & 1 deletion src/signals/slop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ 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;
/** True when this PR sits in a high-risk duplicate cluster (2+ open PRs) — the caller computes it from the
* collision report via {@link isPullRequestInDuplicateCluster}. Undefined on surfaces without repo context. */
inDuplicateCluster?: boolean | undefined;
};

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

export const SLOP_RUBRIC_MARKDOWN = [
Expand All @@ -54,6 +58,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 +74,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 +191,23 @@ 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. The caller determines this via isPullRequestInDuplicateCluster (#563), whose
// 2+-pull-request bar keeps the blocking signal false-positive-averse (a healthy issue↔its-own-PR pair, also
// marked high-risk by buildCollisionReport, is excluded). Static, public-safe text.
export function buildDuplicateClusterFinding(input: SlopAssessmentInput): SignalFinding | null {
if (input.inDuplicateCluster !== true) 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
26 changes: 26 additions & 0 deletions test/unit/signals.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ import {
buildRegistryChangeReport,
buildRepoFitRecommendation,
detectGittensorContributor,
isPullRequestInDuplicateCluster,
shouldPublishPrIntelligenceComment,
type CollisionReport,
} from "../../src/signals/engine";
import { GITTENSOR_HOME_URL } from "../../src/github/footer";
import type {
Expand Down Expand Up @@ -1334,3 +1336,27 @@ function registrySnapshot(id: string, repositories: RegistrySnapshot["repositori
repositories,
};
}

describe("isPullRequestInDuplicateCluster (#563)", () => {
// Typed fixtures: the CollisionReport shape is compile-checked, so it cannot drift from the real type.
const report = (clusters: CollisionReport["clusters"]): CollisionReport => ({
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,
});
type Item = CollisionReport["clusters"][number]["items"][number];
const prItem = (number: number): Item => ({ type: "pull_request", number, title: `PR ${number}` });
const issueItem = (number: number): Item => ({ type: "issue", number, title: `issue ${number}` });

it("is true only for a high-risk cluster with 2+ pull requests that includes the PR", () => {
expect(isPullRequestInDuplicateCluster(report([{ id: "c", risk: "high", reason: "overlap", items: [prItem(7), prItem(8), issueItem(3)] }]), 7)).toBe(true);
});

it("is false for missing, insufficient, or non-matching clusters", () => {
expect(isPullRequestInDuplicateCluster(report([]), 7)).toBe(false); // no clusters
expect(isPullRequestInDuplicateCluster(report([{ id: "c", risk: "high", reason: "r", items: [prItem(7), issueItem(3)] }]), 7)).toBe(false); // only 1 PR (healthy issue↔PR pair)
expect(isPullRequestInDuplicateCluster(report([{ id: "c", risk: "medium", reason: "r", items: [prItem(7), prItem(8)] }]), 7)).toBe(false); // not high-risk
expect(isPullRequestInDuplicateCluster(report([{ id: "c", risk: "high", reason: "r", items: [prItem(8), prItem(9)] }]), 7)).toBe(false); // PR not a member
});
});
27 changes: 27 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 Down Expand Up @@ -50,6 +51,32 @@ describe("buildSlopAssessment", () => {
expect(buildLowQualityCommitMessageFinding({ commitMessages: ["", "update"] })?.detail).toMatch(/generic/i);
});

it("raises duplicate-cluster slop when the PR is flagged as in a duplicate cluster (#563)", () => {
const result = buildSlopAssessment({ inDuplicateCluster: true });
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 when not flagged (false or omitted) (#563)", () => {
expect(buildDuplicateClusterFinding({})).toBeNull();
expect(buildDuplicateClusterFinding({ inDuplicateCluster: false })).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.",
inDuplicateCluster: true, // → duplicate_cluster_membership (15)
});
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