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
31 changes: 10 additions & 21 deletions packages/gittensory-engine/src/duplicate-winner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,11 @@
* caller can compute the winner ONCE per review run and thread the result boolean consistently into every
* surface (advisory finding, close reason, slop, panels), so they agree by construction.
*
* ELECTION ORDER (#dup-winner true-creation-time): prefer each PR's true GitHub `pull_request.created_at` —
* the real order contributors opened their PRs in — over `linkedIssueClaimedAt` (gittensory's own sync-time,
* i.e. whenever a webhook/sweep/backfill pass happened to OBSERVE the linked issue). Sync order and creation
* order diverge whenever processing isn't strictly FIFO (a stalled sweep catching up on a backlog, backfill
* reordering, webhook delivery delay), under the old claim-time-only rule, that divergence could crown a
* LATER contributor the winner and close the PR of whoever actually opened first. `createdAt` is compared
* only when BOTH sides of a given comparison have a valid one; otherwise this falls back to the legacy
* claim-time comparison unchanged, so sparse/legacy rows keep their existing fail-closed behavior exactly.
* ELECTION ORDER: compare `linkedIssueClaimedAt`, the time gittensory first observed the PR claiming
* the issue. GitHub `pull_request.created_at` is intentionally not an ordering signal here: contributors can
* edit an old placeholder PR to add a linked issue later, so creation time would let backdated claims steal
* duplicate-winner credit from the PR that actually claimed the issue first. Sparse legacy rows that lack
* claim timing keep failing closed so unknown ordering cannot suppress duplicate evidence.
*
* INVARIANT (the caller MUST honor it): {@link openSiblingNumbers} carries OPEN-only sibling PR numbers. The
* existing sources already exclude closed/merged PRs. Once the winner closes (e.g. red CI), it leaves the open
Expand All @@ -34,7 +31,7 @@
export type DuplicateClaimMember = {
number: number;
linkedIssueClaimedAt?: string | null | undefined;
/** GitHub's true PR creation time. See the module doc's "ELECTION ORDER" note. */
/** GitHub's true PR creation time. Retained for caller compatibility; not used for winner ordering. */
createdAt?: string | null | undefined;
};

Expand All @@ -55,8 +52,7 @@ export function isDuplicateClusterWinner(prNumber: number, openSiblingNumbers: n

/**
* True iff `pr` is the earliest-elected claimant in the open duplicate cluster (see the module doc's
* "ELECTION ORDER" note for the createdAt-vs-claim-time precedence). Sparse legacy rows fail closed; ties
* between equally-ordered members use PR number.
* "ELECTION ORDER" note). Sparse legacy rows fail closed; ties between equally-ordered members use PR number.
*/
export function isDuplicateClusterWinnerByClaim(pr: DuplicateClaimMember, openSiblings: DuplicateClaimMember[]): boolean {
if (openSiblings.length === 0) return true;
Expand All @@ -67,18 +63,11 @@ export function isDuplicateClusterWinnerByClaim(pr: DuplicateClaimMember, openSi
}

/**
* True iff `pr` is ordered at or ahead of `sibling` for cluster-winner purposes. Prefers `createdAt` when BOTH
* sides have a valid one (the true creation-time order); otherwise falls back to the legacy `linkedIssueClaimedAt`
* comparison unchanged (including its fail-closed-on-missing/invalid-timestamp behavior), so a mixed
* legacy/modern cluster never silently guesses using two different clocks for the two sides of one comparison.
* True iff `pr` is ordered at or ahead of `sibling` for cluster-winner purposes. Only the observed linked-issue
* claim time participates in the election; `createdAt` is deliberately ignored because an older PR can claim a
* linked issue later by editing its body.
*/
function prPrecedesSibling(pr: DuplicateClaimMember, sibling: DuplicateClaimMember): boolean {
const prCreated = claimTimeMs(pr.createdAt);
const siblingCreated = claimTimeMs(sibling.createdAt);
if (prCreated !== null && siblingCreated !== null) {
if (prCreated !== siblingCreated) return prCreated < siblingCreated;
return pr.number <= sibling.number;
}
const prClaim = claimTimeMs(pr.linkedIssueClaimedAt);
if (prClaim === null) return false;
const siblingClaim = claimTimeMs(sibling.linkedIssueClaimedAt);
Expand Down
37 changes: 16 additions & 21 deletions packages/gittensory-engine/src/signals/duplicate-winner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,27 @@
* caller can compute the winner ONCE per review run and thread the result boolean consistently into every
* surface (advisory finding, close reason, slop, panels), so they agree by construction.
*
* ELECTION ORDER (#dup-winner true-creation-time): prefer each PR's true GitHub `pull_request.created_at` —
* the real order contributors opened their PRs in — over `linkedIssueClaimedAt` (gittensory's own sync-time,
* i.e. whenever a webhook/sweep/backfill pass happened to OBSERVE the linked issue). Sync order and creation
* order diverge whenever processing isn't strictly FIFO (a stalled sweep catching up on a backlog, backfill
* reordering, webhook delivery delay), under the old claim-time-only rule, that divergence could crown a
* LATER contributor the winner and close the PR of whoever actually opened first. `createdAt` is compared
* only when BOTH sides of a given comparison have a valid one; otherwise this falls back to the legacy
* claim-time comparison unchanged, so sparse/legacy rows keep their existing fail-closed behavior exactly.
* ELECTION ORDER: compare `linkedIssueClaimedAt`, the time gittensory first observed the PR claiming
* the issue. GitHub `pull_request.created_at` is intentionally not an ordering signal here: contributors can
* edit an old placeholder PR to add a linked issue later, so creation time would let backdated claims steal
* duplicate-winner credit from the PR that actually claimed the issue first. Sparse legacy rows that lack
* claim timing keep failing closed so unknown ordering cannot suppress duplicate evidence.
*
* INVARIANT (the caller MUST honor it): {@link openSiblingNumbers} carries OPEN-only sibling PR numbers. The
* existing sources already exclude closed/merged PRs. Once the winner closes (e.g. red CI), it leaves the open
* set and the next-earliest OPEN claimant becomes the winner on re-eval — no permanently-orphaned cluster.
*
* SECOND CONSUMER (#2278): this module is intentionally engine-hosted (not `src/`-only) because its election
* logic is reusable for the miner's own soft-claim adjudication — deciding which of several miners claiming
* the same issue proceeds. A future contributor wiring the miner's local claim ledger should import this
* module rather than reimplementing the election rule, so both the maintainer gate and the miner agree on
* exactly one winner by construction.
*/

export type DuplicateClaimMember = {
number: number;
linkedIssueClaimedAt?: string | null | undefined;
/** GitHub's true PR creation time. See the module doc's "ELECTION ORDER" note. */
/** GitHub's true PR creation time. Retained for caller compatibility; not used for winner ordering. */
createdAt?: string | null | undefined;
};

Expand All @@ -49,8 +52,7 @@ export function isDuplicateClusterWinner(prNumber: number, openSiblingNumbers: n

/**
* True iff `pr` is the earliest-elected claimant in the open duplicate cluster (see the module doc's
* "ELECTION ORDER" note for the createdAt-vs-claim-time precedence). Sparse legacy rows fail closed; ties
* between equally-ordered members use PR number.
* "ELECTION ORDER" note). Sparse legacy rows fail closed; ties between equally-ordered members use PR number.
*/
export function isDuplicateClusterWinnerByClaim(pr: DuplicateClaimMember, openSiblings: DuplicateClaimMember[]): boolean {
if (openSiblings.length === 0) return true;
Expand All @@ -61,18 +63,11 @@ export function isDuplicateClusterWinnerByClaim(pr: DuplicateClaimMember, openSi
}

/**
* True iff `pr` is ordered at or ahead of `sibling` for cluster-winner purposes. Prefers `createdAt` when BOTH
* sides have a valid one (the true creation-time order); otherwise falls back to the legacy `linkedIssueClaimedAt`
* comparison unchanged (including its fail-closed-on-missing/invalid-timestamp behavior), so a mixed
* legacy/modern cluster never silently guesses using two different clocks for the two sides of one comparison.
* True iff `pr` is ordered at or ahead of `sibling` for cluster-winner purposes. Only the observed linked-issue
* claim time participates in the election; `createdAt` is deliberately ignored because an older PR can claim a
* linked issue later by editing its body.
*/
function prPrecedesSibling(pr: DuplicateClaimMember, sibling: DuplicateClaimMember): boolean {
const prCreated = claimTimeMs(pr.createdAt);
const siblingCreated = claimTimeMs(sibling.createdAt);
if (prCreated !== null && siblingCreated !== null) {
if (prCreated !== siblingCreated) return prCreated < siblingCreated;
return pr.number <= sibling.number;
}
const prClaim = claimTimeMs(pr.linkedIssueClaimedAt);
if (prClaim === null) return false;
const siblingClaim = claimTimeMs(sibling.linkedIssueClaimedAt);
Expand Down
2 changes: 1 addition & 1 deletion src/signals/duplicate-winner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Duplicate-winner adjudication (#dup-winner), extracted to `@jsonbored/gittensory-engine` (#2278) so the
* maintainer gate and the miner's own soft-claim adjudication (a later Phase-0 issue) import the identical,
* versioned election logic instead of drifting apart. See the engine module's doc comment for the full
* election-order rationale (createdAt-vs-claim-time precedence, fail-closed semantics).
* election-order rationale (claim-time election, anti-backdating semantics).
*
* packages/gittensory-engine/src/duplicate-winner.ts (imported via relative source path, not the published
* module, matching the #2282 scoring-preview extraction) is the source of truth.
Expand Down
76 changes: 25 additions & 51 deletions test/unit/duplicate-winner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,76 +92,50 @@ describe("isDuplicateClusterWinnerByClaim (#dup-winner claim election)", () => {
});
});

describe("isDuplicateClusterWinnerByClaim createdAt precedence (#dup-winner true-creation-time)", () => {
describe("isDuplicateClusterWinnerByClaim claim-time election with createdAt present (#dup-winner anti-backdating)", () => {
const member = (number: number, createdAt: string | null, linkedIssueClaimedAt: string | null) => ({ number, createdAt, linkedIssueClaimedAt });

it("REGRESSION: elects the PR that GitHub says opened first, even when gittensory OBSERVED (claimed) the later-opened sibling first", () => {
// PR 13 truly opened first (10:00) but gittensory's stalled sweep only got around to syncing/claiming it at
// 11:00. PR 14 opened later (10:05) but was claimed immediately (10:06) because the sweep happened to reach
// it first. Under the old claim-time-only rule, 14 would wrongly win and 13 (the real first mover) would be
// closed as the "duplicate." createdAt must override that.
it("REGRESSION: does not let an older placeholder PR steal winner credit by adding the issue later", () => {
// PR 12 was opened first but only edited in the linked issue after PR 13 had already claimed it. The
// anti-backdating signal is linkedIssueClaimedAt, so createdAt must not override the actual claim order.
expect(
isDuplicateClusterWinnerByClaim(
member(13, "2026-06-29T10:00:00.000Z", "2026-06-29T11:00:00.000Z"),
[member(14, "2026-06-29T10:05:00.000Z", "2026-06-29T10:06:00.000Z")],
),
).toBe(true);
// And symmetrically, the later-created PR no longer wins just because it was claimed first.
expect(
isDuplicateClusterWinnerByClaim(
member(14, "2026-06-29T10:05:00.000Z", "2026-06-29T10:06:00.000Z"),
[member(13, "2026-06-29T10:00:00.000Z", "2026-06-29T11:00:00.000Z")],
member(12, "2026-06-29T09:00:00.000Z", "2026-06-29T10:05:00.000Z"),
[member(13, "2026-06-29T09:30:00.000Z", "2026-06-29T10:00:00.000Z")],
),
).toBe(false);
});

it("falls back to claim-time comparison when only ONE side has a valid createdAt (mixed legacy/modern cluster)", () => {
// pr has createdAt; sibling (a legacy row) does not — never mix clocks across the two sides of one
// comparison. pr's claim (10:00) is earlier than sibling's claim (10:05) ⇒ pr still wins via the fallback.
expect(
isDuplicateClusterWinnerByClaim(
{ number: 12, createdAt: "2026-06-29T09:00:00.000Z", linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z" },
[{ number: 13, createdAt: null, linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" }],
member(13, "2026-06-29T09:30:00.000Z", "2026-06-29T10:00:00.000Z"),
[member(12, "2026-06-29T09:00:00.000Z", "2026-06-29T10:05:00.000Z")],
),
).toBe(true);
// Same mixed case, but pr's own claim is later than the sibling's ⇒ pr loses via the fallback.
expect(
isDuplicateClusterWinnerByClaim(
{ number: 12, createdAt: "2026-06-29T09:00:00.000Z", linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" },
[{ number: 13, createdAt: null, linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z" }],
),
).toBe(false);
});

it("falls back to claim-time comparison when a createdAt value is present but unparseable", () => {
it("ignores createdAt even when both sides have valid values", () => {
expect(
isDuplicateClusterWinnerByClaim(
member(12, "not-a-date", "2026-06-29T10:00:00.000Z"),
[member(13, "2026-06-29T09:00:00.000Z", "2026-06-29T10:05:00.000Z")],
member(14, "2026-06-29T10:05:00.000Z", "2026-06-29T10:00:00.000Z"),
[member(13, "2026-06-29T10:00:00.000Z", "2026-06-29T11:00:00.000Z")],
),
).toBe(true);
});

it("tie-breaks equal createdAt values by PR number, mirroring the claim-time tie-break", () => {
expect(isDuplicateClusterWinnerByClaim(member(12, "2026-06-29T10:00:00.000Z", null), [member(13, "2026-06-29T10:00:00.000Z", null)])).toBe(true);
expect(isDuplicateClusterWinnerByClaim(member(13, "2026-06-29T10:00:00.000Z", null), [member(12, "2026-06-29T10:00:00.000Z", null)])).toBe(false);
});

it("createdAt-based cases are unaffected by (and do not require) a claim timestamp at all", () => {
expect(isDuplicateClusterWinnerByClaim(member(12, "2026-06-29T10:00:00.000Z", null), [member(13, "2026-06-29T10:05:00.000Z", null)])).toBe(true);
it("still fails closed when createdAt is present but claim timing is missing", () => {
expect(isDuplicateClusterWinnerByClaim(member(12, "2026-06-29T10:00:00.000Z", null), [member(13, "2026-06-29T10:05:00.000Z", null)])).toBe(false);
});
});

describe("resolveDuplicateClusterWinnerNumber (#dup-winner-credit)", () => {
it("returns this PR's own number when it is the winner", () => {
expect(resolveDuplicateClusterWinnerNumber({ number: 12, createdAt: "2026-06-29T10:00:00.000Z" }, [{ number: 13, createdAt: "2026-06-29T10:05:00.000Z" }])).toBe(12);
expect(resolveDuplicateClusterWinnerNumber({ number: 12, linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z" }, [{ number: 13, linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" }])).toBe(12);
});

it("returns the actual winning sibling's number when this PR is a loser, even with multiple siblings", () => {
expect(
resolveDuplicateClusterWinnerNumber({ number: 14, createdAt: "2026-06-29T10:10:00.000Z" }, [
{ number: 13, createdAt: "2026-06-29T10:00:00.000Z" },
{ number: 15, createdAt: "2026-06-29T10:05:00.000Z" },
resolveDuplicateClusterWinnerNumber({ number: 14, linkedIssueClaimedAt: "2026-06-29T10:10:00.000Z" }, [
{ number: 13, linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z" },
{ number: 15, linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" },
]),
).toBe(13);
});
Expand Down Expand Up @@ -223,9 +197,9 @@ describe("dupWinnerLinkedDuplicateCount (#dup-winner close-reason seam)", () =>
expect(dupWinnerLinkedDuplicateCount([], 12, "2026-06-29T10:00:00.000Z", false)).toBe(0);
});

it("REGRESSION (#dup-winner true-creation-time): createdAt overrides a claim-time-only verdict when passed through", () => {
// By claim time alone this PR (12) would lose to sibling 13 (claimed earlier, 10:00 vs 10:05). But 12's true
// createdAt (09:00) precedes 13's (09:30), so passing createdAt flips the verdict to a win (count 0).
it("REGRESSION (#dup-winner anti-backdating): createdAt does not override claim-time ordering when passed through", () => {
// PR 12 is older, but sibling 13 claimed the linked issue first; passing createdAt must not suppress the
// duplicate count for the later claimant.
expect(
dupWinnerLinkedDuplicateCount(
[{ number: 13, linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z", createdAt: "2026-06-29T09:30:00.000Z" }],
Expand All @@ -234,21 +208,21 @@ describe("dupWinnerLinkedDuplicateCount (#dup-winner close-reason seam)", () =>
true,
"2026-06-29T09:00:00.000Z",
),
).toBe(0);
).toBe(1);
});
});

describe("dupWinnerLinkedDuplicateWinnerNumber (#dup-winner-credit close-reason naming seam)", () => {
it("flag OFF ⇒ null regardless of who would win (generic wording, byte-identical to before this existed)", () => {
expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 13, createdAt: "2026-06-29T10:05:00.000Z" }], 12, undefined, false, "2026-06-29T10:00:00.000Z")).toBeNull();
expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 13, linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" }], 12, "2026-06-29T10:00:00.000Z", false, "2026-06-29T10:00:00.000Z")).toBeNull();
});

it("winner + flag ON ⇒ null (nothing to name — its own close reason omits the duplicate cause entirely)", () => {
expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 13, createdAt: "2026-06-29T10:05:00.000Z" }], 12, undefined, true, "2026-06-29T10:00:00.000Z")).toBeNull();
expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 13, linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" }], 12, "2026-06-29T10:00:00.000Z", true, "2026-06-29T10:00:00.000Z")).toBeNull();
});

it("loser + flag ON ⇒ the actual winning sibling's number", () => {
expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 12, createdAt: "2026-06-29T10:00:00.000Z" }], 14, undefined, true, "2026-06-29T10:10:00.000Z")).toBe(12);
expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 12, linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z" }], 14, "2026-06-29T10:10:00.000Z", true, "2026-06-29T10:10:00.000Z")).toBe(12);
});

it("loser + flag ON, but the election is too ambiguous ⇒ null (falls back to generic wording)", () => {
Expand Down Expand Up @@ -305,7 +279,7 @@ describe("listOtherOpenPullRequests ordering (#audit-3.9)", () => {
});
});

describe("upsertPullRequestFromGitHub createdAt threading (#dup-winner true-creation-time)", () => {
describe("upsertPullRequestFromGitHub createdAt threading", () => {
it("populates createdAt from GitHub's true pull_request.created_at on the IMMEDIATE upsert return, not just on a later DB round-trip", async () => {
const env = createTestEnv();
const record = await upsertPullRequestFromGitHub(env, "owner/repo", {
Expand Down
Loading