From 7b7dc8ece7a1561d06c2878a548ac1f69df00bb0 Mon Sep 17 00:00:00 2001 From: oktofeesh1 <287075021+oktofeesh1@users.noreply.github.com> Date: Sat, 20 Jun 2026 09:36:50 -0700 Subject: [PATCH] =?UTF-8?q?feat(signals):=20slop=20signal=20=E2=80=94=20du?= =?UTF-8?q?plicate-cluster=20membership=20(#563)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the duplicate-cluster-membership deterministic slop signal (epic #530). Fires when a PR sits in a HIGH-risk collision cluster that holds 2+ open pull requests — genuine overlapping/duplicate work. - The cluster determination lives in engine.ts as a pure, exported helper `isPullRequestInDuplicateCluster(collisions, pullNumber)` — next to the collision types it reads. slop.ts only takes a precomputed `inDuplicateCluster` boolean, so it needs no collision-type import. - The gate reuses the collision report it already builds — no extra DB load or compute on the hot gate path. - High-precision: the 2+-pull-request bar excludes a healthy issue↔its-own-PR pair (also high-risk), keeping this blocking signal false-positive-averse. - Weighted 15 (a secondary signal); static, public-safe text. Inert on the local lint surfaces (no repo collision context). Tests: the helper (signals.test.ts, typed CollisionReport fixtures — all branches) and the slop finding (slop.test.ts — flagged/not-flagged + the combined-weight band). Full test:coverage green (branches 97.03%). Closes #563 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/queue/processors.ts | 3 +++ src/signals/engine.ts | 15 +++++++++++++++ src/signals/slop.ts | 27 ++++++++++++++++++++++++++- test/unit/signals.test.ts | 26 ++++++++++++++++++++++++++ test/unit/slop.test.ts | 27 +++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 1 deletion(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 4a4ee74b9f..ba2b5adcca 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -122,6 +122,7 @@ import { buildBurdenForecast, buildCollisionEdges, buildCollisionReport, + isPullRequestInDuplicateCluster, buildConfigQuality, buildContributorFit, buildContributorOutcomeHistory, @@ -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); diff --git a/src/signals/engine.ts b/src/signals/engine.ts index 80aed7f29a..55cfe0682c 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -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[], diff --git a/src/signals/slop.ts b/src/signals/slop.ts index 1fbc260458..c72812235e 100644 --- a/src/signals/slop.ts +++ b/src/signals/slop.ts @@ -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 = { @@ -38,6 +41,7 @@ export const SLOP_WEIGHTS = { nonSubstantivePadding: 30, emptyDescription: 15, lowQualityCommitMessage: 15, + duplicateClusterMembership: 15, } as const; export const SLOP_RUBRIC_MARKDOWN = [ @@ -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; @@ -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, ); @@ -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); diff --git a/test/unit/signals.test.ts b/test/unit/signals.test.ts index 41fc4e325a..26f6ec6755 100644 --- a/test/unit/signals.test.ts +++ b/test/unit/signals.test.ts @@ -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 { @@ -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 + }); +}); diff --git a/test/unit/slop.test.ts b/test/unit/slop.test.ts index 363ebe6946..982abbd179 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, @@ -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 }],