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
33 changes: 33 additions & 0 deletions scripts/review-queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,36 @@ def window_note(window: Window | None) -> str:
return ""


def pair_note(count: int) -> str:
"""The reading that answers a question no row asks, when there is more than one PR.

Every row above answers "what is the next action for **this** PR". Whether two open PRs
can land **together** is a different question, with its own tool, and that tool is named
only in the merge-gate list a cycle reads when it is already merging — so a queue of
`vote` rows reads as "two things are ready" when the truth can be "these two cannot both
land as they stand".

Measured before this note existed: two PRs that each added tests at the same anchor in one
file both read `vote`, and `check-merge-pairs.py` answered **blocked in both directions**.
The cost is the discovery time rather than the conflict: the first merged, the second had
to merge master in and resolve that file, which moves its head — and a head push voids
every vote standing on it. Measured after that resolution: two approvals turned `VOID` and
the PR fell from 2/3 to 1/3, so two cycles of review were spent again on content that had
not changed.

The note does not claim that any pair conflicts, and does not fold the pairs itself:
building those trees is the pair tool's cost, and paying it on every queue run would make
every cycle pay for a question most of them do not have.
"""
if count < 2:
return ""
return (
f"note: {count} open PR(s) - whether they can land together is a SEPARATE reading, "
f"not a merge order; the rows above answer only 'what next for this PR': "
f"$ {RUNNER} scripts/check-merge-pairs.py"
)


def render(reading: Reading, action: Action) -> str:
"""One PR's block: what is true, then what to do, then the exact command."""
head = reading.head[:8] if reading.head else "????????"
Expand Down Expand Up @@ -1782,6 +1812,9 @@ def main(argv: list[str] | None = None) -> int:
f"{kind} {count}" for kind, count in sorted(tally.items())
)
print(f"{len(readings)} PR(s): {summary}")
note = pair_note(len(readings))
if note:
print(note)
if unread:
print(
f"unmeasurable: {', '.join(f'#{pr}' for pr in unread)} - "
Expand Down
45 changes: 45 additions & 0 deletions tests/test_review_queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -2942,3 +2942,48 @@ def test_an_entry_with_no_progress_still_reads_the_issue_side(monkeypatch):
assert links.states_asked == [], (
"the entry names no number, so there is nothing for the one-number reader to ask"
)


def test_two_open_prs_are_told_the_pair_question_is_a_separate_reading(mod, monkeypatch, capsys):
"""Every row answers "what next for this PR"; landing two together is another question.

A queue of `vote` rows reads as "two things are ready", when the truth can be "these two
cannot both land as they stand". Measured before this note existed: two PRs each adding
tests at the same anchor in one file both read `vote`, and `check-merge-pairs.py` answered
**blocked in both directions**. The cost is the discovery time, not the conflict — the
second branch then had to merge master in and resolve that file, which moves its head, and
a head push voids every vote on it: measured after that resolution, two approvals turned
VOID and the PR fell from 2/3 to 1/3.

The note must name the reading and print a command that runs on this host, and it must not
claim that any pair conflicts — it points at the question, it does not answer it.
"""
votes = FakeVotes(reviews=[], push=_push(2026, 9, 17, 10, 0))
fresh = FakeFresh()
rc = _run(mod, monkeypatch, votes, fresh,
["1", "2", "--cycle", CYCLE, "--cycles-log", str(NO_RECORDS)])
out = capsys.readouterr().out
assert rc == 0
assert "2 PR(s)" in out
notes = [line for line in out.splitlines() if "check-merge-pairs" in line]
assert len(notes) == 1, out
assert notes[0].startswith("note:"), notes[0]
assert "SEPARATE" in notes[0], notes[0]
assert f"{mod.RUNNER} scripts/check-merge-pairs.py" in notes[0], (
"the note must carry the runner the rest of the file prints - a bare "
"`scripts/check-merge-pairs.py` is not executable on this host"
)
assert "blocked" not in notes[0] and "conflict" not in notes[0], (
"the note points at the question; it does not claim a pair conflicts"
)


def test_one_open_pr_is_not_told_about_a_pair_that_does_not_exist(mod, monkeypatch, capsys):
"""The control: with a single PR there is no pair, so the note has no subject."""
votes = FakeVotes(reviews=[], push=_push(2026, 9, 17, 10, 0))
fresh = FakeFresh()
rc = _run(mod, monkeypatch, votes, fresh, ["1", "--cycle", CYCLE, "--cycles-log", str(NO_RECORDS)])
out = capsys.readouterr().out
assert rc == 0
assert "1 PR(s)" in out
assert "check-merge-pairs" not in out, out
Loading