diff --git a/packages/loopover-engine/src/duplicate-winner.ts b/packages/loopover-engine/src/duplicate-winner.ts index e135e36a73..471faddb18 100644 --- a/packages/loopover-engine/src/duplicate-winner.ts +++ b/packages/loopover-engine/src/duplicate-winner.ts @@ -35,21 +35,6 @@ export type DuplicateClaimMember = { createdAt?: string | null | undefined; }; -/** - * True iff `prNumber` is the cluster winner: the minimum of `{prNumber} ∪ openSiblingNumbers`. An empty - * sibling list ⇒ the PR is alone in (or out of) the cluster ⇒ winner. A sibling list that happens to contain - * `prNumber` itself is harmless — the comparison is still min-based. - * - * @deprecated Use {@link isDuplicateClusterWinnerByClaim}. PR-number election is retained only for legacy - * compatibility callers that do not have claim timestamps. - */ -export function isDuplicateClusterWinner(prNumber: number, openSiblingNumbers: number[]): boolean { - for (const sibling of openSiblingNumbers) { - if (sibling < prNumber) return false; - } - return true; -} - /** * True iff `pr` is the earliest-elected claimant in the open duplicate cluster (see the module doc's * "ELECTION ORDER" note). Sparse legacy rows fail closed; ties between equally-ordered members use PR number. diff --git a/packages/loopover-engine/src/index.ts b/packages/loopover-engine/src/index.ts index fa2d760187..673372d445 100644 --- a/packages/loopover-engine/src/index.ts +++ b/packages/loopover-engine/src/index.ts @@ -703,7 +703,6 @@ export * as scoringModel from "./scoring/model.js"; export * as scoringPreview from "./scoring/preview.js"; export * as scoringPendingPrScenarios from "./scoring/pending-pr-scenarios.js"; export { - isDuplicateClusterWinner, isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber, type DuplicateClaimMember, diff --git a/packages/loopover-engine/src/predicted-gate.ts b/packages/loopover-engine/src/predicted-gate.ts index 921924d9db..a42e584f1a 100644 --- a/packages/loopover-engine/src/predicted-gate.ts +++ b/packages/loopover-engine/src/predicted-gate.ts @@ -205,7 +205,7 @@ export function buildPredictedGateVerdict(args: { // `duplicateWinnerEnabled` is INTENTIONALLY omitted (#dup-winner): the prospective PR is synthetic #0, but a // real new PR opened into an existing duplicate cluster gets the HIGHEST number ⇒ it is always a duplicate // LOSER, never the winner. So the predictor must keep showing the duplicate finding (the honest pre-submit - // answer). Threading the flag here would let isDuplicateClusterWinner(0, …) treat #0 as the winner and + // answer). Threading the flag here would let the winner election treat a placeholder #0 as the winner and // falsely suppress the block — a false-optimism regression. Do NOT add it without modeling #0 as the loser. // Thread linked-issue authors from the issues snapshot so the predictor surfaces the self-authored-linked-issue // finding too — evaluateGateCheck below already receives gate.selfAuthoredLinkedIssue, but without this finding it diff --git a/packages/loopover-engine/src/signals/duplicate-winner.ts b/packages/loopover-engine/src/signals/duplicate-winner.ts index 4da2bd3d1e..444dfd780b 100644 --- a/packages/loopover-engine/src/signals/duplicate-winner.ts +++ b/packages/loopover-engine/src/signals/duplicate-winner.ts @@ -7,7 +7,6 @@ * `../signals/duplicate-winner.js` import keeps resolving without a call-site change. */ export { - isDuplicateClusterWinner, isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber, type DuplicateClaimMember, diff --git a/packages/loopover-engine/test/duplicate-winner-signals-shim.test.ts b/packages/loopover-engine/test/duplicate-winner-signals-shim.test.ts index f8a18c28fe..38ae95be8f 100644 --- a/packages/loopover-engine/test/duplicate-winner-signals-shim.test.ts +++ b/packages/loopover-engine/test/duplicate-winner-signals-shim.test.ts @@ -5,7 +5,6 @@ import * as topLevel from "../dist/duplicate-winner.js"; import * as signalsShim from "../dist/signals/duplicate-winner.js"; test("signals/duplicate-winner is a thin re-export of the top-level module (#4251)", () => { - assert.equal(signalsShim.isDuplicateClusterWinner, topLevel.isDuplicateClusterWinner); assert.equal(signalsShim.isDuplicateClusterWinnerByClaim, topLevel.isDuplicateClusterWinnerByClaim); assert.equal( signalsShim.resolveDuplicateClusterWinnerNumber, diff --git a/packages/loopover-engine/test/duplicate-winner.test.ts b/packages/loopover-engine/test/duplicate-winner.test.ts index ac4c9d6145..5a33eab034 100644 --- a/packages/loopover-engine/test/duplicate-winner.test.ts +++ b/packages/loopover-engine/test/duplicate-winner.test.ts @@ -2,29 +2,15 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { - isDuplicateClusterWinner, isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber, } from "../dist/index.js"; test("barrel: the public entrypoint re-exports the duplicate-winner adjudication API", () => { - assert.equal(typeof isDuplicateClusterWinner, "function"); assert.equal(typeof isDuplicateClusterWinnerByClaim, "function"); assert.equal(typeof resolveDuplicateClusterWinnerNumber, "function"); }); -test("isDuplicateClusterWinner: the lowest open sibling number wins", () => { - assert.equal(isDuplicateClusterWinner(5, [7, 9]), true); -}); - -test("isDuplicateClusterWinner: a lower open sibling beats this PR (loser)", () => { - assert.equal(isDuplicateClusterWinner(5, [3, 9]), false); -}); - -test("isDuplicateClusterWinner: an empty sibling list is always a winner", () => { - assert.equal(isDuplicateClusterWinner(5, []), true); -}); - test("isDuplicateClusterWinnerByClaim: an empty sibling list is always a winner", () => { assert.equal(isDuplicateClusterWinnerByClaim({ number: 5 }, []), true); }); diff --git a/src/queue/duplicate-detection.ts b/src/queue/duplicate-detection.ts index 4f0663adc6..1bca24a61c 100644 --- a/src/queue/duplicate-detection.ts +++ b/src/queue/duplicate-detection.ts @@ -65,7 +65,7 @@ export function dupWinnerLinkedDuplicateWinnerNumber( * GitHub but is still cached `open` would keep "winning" the duplicate cluster, demoting the real lowest-OPEN PR * to a loser and auto-closing it via the `duplicate_pr_risk` blocker. Only a LOWER-numbered overlapping sibling * can demote this PR from winner, so re-fetch the LIVE state of just those siblings and drop any that are no - * longer open. Then the downstream election ({@link isDuplicateClusterWinner}) reflects ground truth. + * longer open. Then the downstream election ({@link isDuplicateClusterWinnerByClaim}) reflects ground truth. * * FAIL-OPEN to the stored state: a sibling is dropped ONLY on a positive "not open" confirmation — an unreadable * live fetch keeps it, so a transient GitHub hiccup never newly spares a real loser. Flag-OFF (default), no diff --git a/src/services/maintainer-activation.ts b/src/services/maintainer-activation.ts index fd7b021090..bfc064bfca 100644 --- a/src/services/maintainer-activation.ts +++ b/src/services/maintainer-activation.ts @@ -62,7 +62,7 @@ export function buildMaintainerActivationPreview(args: { const codeCounts = new Map(); const samples: MaintainerActivationSample[] = recent.map((pr) => { const advisory = buildPullRequestAdvisory(args.repo, pr, { - // Open-only siblings: a closed/merged PR isn't competing, and isDuplicateClusterWinner's invariant + // Open-only siblings: a closed/merged PR isn't competing, and isDuplicateClusterWinnerByClaim's invariant // requires open-only numbers — match the live pipeline (processors.ts:559/800), which feeds the // winner adjudication the same open-filtered set. otherOpenPullRequests: args.pullRequests.filter((other) => other.number !== pr.number && other.state === "open"), diff --git a/src/signals/duplicate-winner.ts b/src/signals/duplicate-winner.ts index 4c2e2baf46..63b1ce2366 100644 --- a/src/signals/duplicate-winner.ts +++ b/src/signals/duplicate-winner.ts @@ -8,7 +8,6 @@ * module, matching the #2282 scoring-preview extraction) is the source of truth. */ export { - isDuplicateClusterWinner, isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber, type DuplicateClaimMember, diff --git a/test/unit/duplicate-winner.test.ts b/test/unit/duplicate-winner.test.ts index a4f0babb2e..b26ddaf339 100644 --- a/test/unit/duplicate-winner.test.ts +++ b/test/unit/duplicate-winner.test.ts @@ -1,40 +1,10 @@ import { describe, expect, it } from "vitest"; -import { isDuplicateClusterWinner, isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "../../src/signals/duplicate-winner"; +import { isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "../../src/signals/duplicate-winner"; import { dupWinnerLinkedDuplicateCount, dupWinnerLinkedDuplicateWinnerNumber, linkedIssueDuplicatePullRequestsForGate } from "../../src/queue/processors"; import type { PullRequestRecord } from "../../src/types"; import { listOtherOpenPullRequests, listOtherOpenPullRequestsForAuthor, upsertPullRequestFromGitHub } from "../../src/db/repositories"; import { createTestEnv } from "../helpers/d1"; -describe("isDuplicateClusterWinner (#dup-winner)", () => { - it("the lowest open sibling number wins", () => { - expect(isDuplicateClusterWinner(12, [13, 14])).toBe(true); - }); - - it("a lower open sibling beats this PR (loser)", () => { - expect(isDuplicateClusterWinner(14, [12, 13])).toBe(false); - }); - - it("an empty sibling list ⇒ winner (alone in/out of the cluster)", () => { - expect(isDuplicateClusterWinner(7, [])).toBe(true); - }); - - it("a sibling list that contains self is still min-based (winner when self is lowest)", () => { - expect(isDuplicateClusterWinner(12, [12, 13])).toBe(true); - }); - - it("a sibling list that contains self plus a lower sibling ⇒ loser", () => { - expect(isDuplicateClusterWinner(13, [12, 13])).toBe(false); - }); - - it("cascade: once the lowest sibling closes (drops out of the open set), the next-lowest becomes the winner", () => { - // Cluster {12, 13, 14}. PR 13 is a loser while 12 is still open. - expect(isDuplicateClusterWinner(13, [12, 14])).toBe(false); - // PR 12 closes (red CI) → it leaves the OPEN sibling set the caller passes. Re-eval of PR 13 now sees only - // {14} as the open sibling → 13 is the new winner. No permanently-orphaned cluster. - expect(isDuplicateClusterWinner(13, [14])).toBe(true); - }); -}); - describe("isDuplicateClusterWinnerByClaim (#dup-winner claim election)", () => { const claim = (number: number, linkedIssueClaimedAt: string | null) => ({ number, linkedIssueClaimedAt }); diff --git a/test/unit/predicted-gate-engine-coverage.test.ts b/test/unit/predicted-gate-engine-coverage.test.ts index e70d81b38c..1e1488eccc 100644 --- a/test/unit/predicted-gate-engine-coverage.test.ts +++ b/test/unit/predicted-gate-engine-coverage.test.ts @@ -25,7 +25,7 @@ import { isGuardrailHit, matchesAny, } from "../../packages/loopover-engine/src/signals/change-guardrail"; -import { isDuplicateClusterWinner, isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "../../packages/loopover-engine/src/signals/duplicate-winner"; +import { isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "../../packages/loopover-engine/src/signals/duplicate-winner"; import { buildCollisionReport, buildPreflightResult, buildPublicReadinessScore, buildQueueHealth, classifyBountyLifecycle, itemSharesPlannedLinkedIssue, predictedGateEngineInternals, termOverlap, unionScopedOverlapClusters } from "../../packages/loopover-engine/src/signals/predicted-gate-engine"; import type { CollisionItem, FocusManifest, IssueQualityReport, PreMergeCheck, PullRequestRecord, RepositoryRecord } from "../../packages/loopover-engine/src/types/predicted-gate-types"; @@ -249,9 +249,7 @@ describe("predicted-gate engine module coverage (#2283)", () => { expect(blocked.conclusion).toBe("failure"); }); - it("exercises deprecated duplicate winner helper and lane advice branches", () => { - expect(isDuplicateClusterWinner(1, [2, 3])).toBe(true); - expect(isDuplicateClusterWinner(3, [1, 2])).toBe(false); + it("exercises the inactive lane advice branch", () => { const inactive = buildPreflightResult( { repoFullName: "acme/widgets", title: "Fix", body: "Closes #7", linkedIssues: [7] }, { ...REPO, registryConfig: { ...REPO.registryConfig!, emissionShare: 0 } },