Context
reconcileLiveDuplicateSiblings's doc comment (src/queue/duplicate-detection.ts:61-72) claims: "Only a LOWER-numbered overlapping sibling can demote this PR from winner, so re-fetch the LIVE state of just those siblings." But the code (lines 85-89) fetches live state for all overlapping open siblings regardless of number, and the real winner election (packages/loopover-engine/src/duplicate-winner.ts) is based on linkedIssueClaimedAt (claim time), not PR number — a higher-numbered PR that claimed the linked issue earlier can and does demote a lower-numbered one.
Confirmed by the function's own test file (test/unit/reconcile-live-duplicate-siblings.test.ts), whose test titles explicitly state a higher overlapping sibling being LIVE-closed is dropped "because claim-time election lets higher numbers demote" — directly contradicting the doc comment. Git history (closed issue #3816, "elect duplicate-cluster winner by GitHub's true PR creation time... not sync-observation time") shows the election semantics were reworked after this comment was written, and the comment was never updated.
This is not a live functional bug today — the code already does the safe, correct thing (fetches all overlapping siblings, not just lower-numbered ones). But it's a landmine: a future contributor "optimizing" per the comment (skip fetching higher-numbered siblings) would reintroduce a real correctness bug, since claim-time can make a higher-numbered PR the true winner.
Requirements
Correct the doc comment on reconcileLiveDuplicateSiblings (src/queue/duplicate-detection.ts:61-72) to describe the actual claim-time-based election semantics instead of the stale PR-number-based description, so a future reader isn't misled into "fixing" the function into a real bug. No code/behavior change — this is a comment-only correction.
Deliverables
Test Coverage Requirements
None — comment-only change, no behavior change, no new coverage needed. If practical, verify no existing test asserts anything about the comment's stale claim (unlikely, since comments aren't executable).
Expected Outcome
reconcileLiveDuplicateSiblings's doc comment matches its own test suite's documented behavior and the real post-#3816 election semantics, removing a landmine that could mislead a future contributor into reintroducing a correctness bug.
Links & Resources
Context
reconcileLiveDuplicateSiblings's doc comment (src/queue/duplicate-detection.ts:61-72) claims: "Only a LOWER-numbered overlapping sibling can demote this PR from winner, so re-fetch the LIVE state of just those siblings." But the code (lines 85-89) fetches live state for all overlapping open siblings regardless of number, and the real winner election (packages/loopover-engine/src/duplicate-winner.ts) is based onlinkedIssueClaimedAt(claim time), not PR number — a higher-numbered PR that claimed the linked issue earlier can and does demote a lower-numbered one.Confirmed by the function's own test file (
test/unit/reconcile-live-duplicate-siblings.test.ts), whose test titles explicitly state a higher overlapping sibling being LIVE-closed is dropped "because claim-time election lets higher numbers demote" — directly contradicting the doc comment. Git history (closed issue #3816, "elect duplicate-cluster winner by GitHub's true PR creation time... not sync-observation time") shows the election semantics were reworked after this comment was written, and the comment was never updated.This is not a live functional bug today — the code already does the safe, correct thing (fetches all overlapping siblings, not just lower-numbered ones). But it's a landmine: a future contributor "optimizing" per the comment (skip fetching higher-numbered siblings) would reintroduce a real correctness bug, since claim-time can make a higher-numbered PR the true winner.
Requirements
Correct the doc comment on
reconcileLiveDuplicateSiblings(src/queue/duplicate-detection.ts:61-72) to describe the actual claim-time-based election semantics instead of the stale PR-number-based description, so a future reader isn't misled into "fixing" the function into a real bug. No code/behavior change — this is a comment-only correction.Deliverables
reconcileLiveDuplicateSiblingsaccurately describes that any overlapping sibling (regardless of PR number, lower or higher) can demote the current PR, because election is based onlinkedIssueClaimedAt, not PR number — citing fix(dup-winner): elect duplicate-cluster winner by GitHub's true PR creation time, not gittensory's own sync-observation time #3816 for context.Test Coverage Requirements
None — comment-only change, no behavior change, no new coverage needed. If practical, verify no existing test asserts anything about the comment's stale claim (unlikely, since comments aren't executable).
Expected Outcome
reconcileLiveDuplicateSiblings's doc comment matches its own test suite's documented behavior and the real post-#3816 election semantics, removing a landmine that could mislead a future contributor into reintroducing a correctness bug.Links & Resources
src/queue/duplicate-detection.ts:61-89(the stale comment and the actually-correct code)packages/loopover-engine/src/duplicate-winner.ts(the real, claim-time-based election logic)test/unit/reconcile-live-duplicate-siblings.test.ts(the test titles that already document the correct semantics)