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
15 changes: 0 additions & 15 deletions packages/loopover-engine/src/duplicate-winner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion packages/loopover-engine/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/loopover-engine/src/predicted-gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion packages/loopover-engine/src/signals/duplicate-winner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
* `../signals/duplicate-winner.js` import keeps resolving without a call-site change.
*/
export {
isDuplicateClusterWinner,
isDuplicateClusterWinnerByClaim,
resolveDuplicateClusterWinnerNumber,
type DuplicateClaimMember,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 0 additions & 14 deletions packages/loopover-engine/test/duplicate-winner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion src/queue/duplicate-detection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/services/maintainer-activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function buildMaintainerActivationPreview(args: {
const codeCounts = new Map<string, number>();
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"),
Expand Down
1 change: 0 additions & 1 deletion src/signals/duplicate-winner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
* module, matching the #2282 scoring-preview extraction) is the source of truth.
*/
export {
isDuplicateClusterWinner,
isDuplicateClusterWinnerByClaim,
resolveDuplicateClusterWinnerNumber,
type DuplicateClaimMember,
Expand Down
32 changes: 1 addition & 31 deletions test/unit/duplicate-winner.test.ts
Original file line number Diff line number Diff line change
@@ -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 });

Expand Down
6 changes: 2 additions & 4 deletions test/unit/predicted-gate-engine-coverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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 } },
Expand Down