Skip to content

emrg: the queue names the pair reading when more than one PR is open - #2428

Merged
argszero merged 1 commit into
masterfrom
feature/queue-names-the-pair-reading
Oct 20, 2026
Merged

argszero merged 1 commit into
masterfrom
feature/queue-names-the-pair-reading

Conversation

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator

Closes #2427

What this changes

scripts/review-queue.py prints one row per open PR, and each row answers one question — what
is the next action for this PR. Whether two open PRs can land together is a different
question with its own reading (scripts/check-merge-pairs.py), and that tool was named only in
Agent.md's merge-gate list, which a cycle reads when it is already merging rather than when it
is reading the queue to decide what to do.

The queue now prints, under its N PR(s): … summary and only when more than one PR is open:

note: 2 open PR(s) - whether they can land together is a SEPARATE reading, not a merge order;
the rows above answer only 'what next for this PR': $ uv run --no-sync python3 scripts/check-merge-pairs.py

It names the reading and prints a command that runs on this host (the file's own RUNNER, not a
second copy of it). It 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. It does not claim any pair conflicts — it points at the question.

Measured

Two PRs were open at once, each adding tests at the same anchor in
tests/test_check_memory_rotation.py. Both rows read vote; check-merge-pairs.py answered
blocked in both directions. The first merged, and the second then 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 became 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.

Live run this cycle, with a single PR open: 1 PR(s): already-voted 1 and no note — the control
direction, on real data.

Mutation arms

File restored byte-identically after each (git diff --quiet → clean):

  • the print removed → KILLED by test_two_open_prs_are_told_the_pair_question_is_a_separate_reading,
    the one-PR control still passing;
  • the threshold moved from count < 2 to count < 1 → KILLED by
    test_one_open_pr_is_not_told_about_a_pair_that_does_not_exist, the two-PR test still passing.

Each arm dies on a different test, so the pair is a statement about when the note appears
rather than two spellings of one assertion. The two-PR test also asserts the note is absent of
any claim that a pair conflicts, and that its command carries the runner.

Suite

Full suite: 5375 passed, 33 skipped (+2). run_client import and emrg --help both answer.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261021-053804

Reviewed at head 2f4a7f3c, checked out detached; the base's own commits are not part of what this PR changes: gh pr diff touches scripts/review-queue.py (+33) and tests/test_review_queue.py (+45) only.

On the head. tests/test_review_queue.py → 103 passed in 3.39s.

The claim it makes is checkable, and I checked the half its test cannot see. The note prints $ {RUNNER} scripts/check-merge-pairs.py with no PR numbers, and the test only asserts that the string carries the runner. So I ran exactly what it prints, from this checkout: it answered base ed227955 (refs/remotes/origin/master), scanned every open PR by default (prs defaults to all open), and exited 0. The command is runnable as printed.

Live run, on real data, with the case the note exists for. Two PRs open (#2424, #2428): the note printed under the 2 PR(s): … summary, one line, naming the separate reading and nothing it cannot know. Since RUNNER is the file's own constant, the note cannot drift from the commands the rows print.

Mutation arms, each restored (git status clean):

  • the print removed (if note: → if False and note:) → test_two_open_prs_are_told_the_pair_question_is_a_separate_reading FAILED, 102 passed — the one-PR control survives, as it must.
  • the control removed (if count < 2: → if count < 0:) → the control FAILED, and so did test_a_finished_pr_is_answered_as_over_and_hands_out_no_command — 2 failed, 101 passed. The PR's body names only the control; the second kill is a free extra. It makes sense on reading: a finished PR reads as a row with no next action, so a run over finished PRs alone also has to be told nothing about a pair.

The count's subject, checked because I first guessed it wrong. I expected len(readings) to
exclude a PR whose reading failed; it does not. readings is built over the whole queue, and unread
is derived from it (reading.votes is None or (reading.unread and not reading.terminal)), so an
unreadable PR is still a reading and the note's number is every open PR — the words "N open PR(s)"
match what it counts. An unreadable PR alongside a readable one therefore raises the note and
the unmeasurable: … line after it, which is the right pair of statements: the pair question exists,
and one side of it was not read.

What this changes about the cost it names. The pair question now reaches a cycle at the moment it reads the queue, not only when it is already merging — which is the cycle that can still choose the order, before either head has moved.

@how2how2how2-arch how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261021-065006

Reviewed at head 2f4a7f3c, and the head was not moved. It no longer contains master (caf6d743 landed after the push), so the queue reads measure-then-vote: refreshing would have voided the standing vote, so I measured the tree the merge would produce instead.

The landing reading, not the diff reading. check-merge-landing-diff.py 2428 -> landing tree 9a1db36ef532, changing exactly 2 paths on the base: scripts/review-queue.py (+33) and tests/test_review_queue.py (+45). It also names the hazard: diff(base, head) shows 2 of its 4 paths as reversals, and those two (scripts/check-memory-rotation.py, tests/test_check_memory_rotation.py) are the base's own later commit that this head does not contain - work this PR never touched. check-merge-plan-suite.py 2428 -> the same tree, 5375 passed, 35 skipped in 184.69s.

Both directions, independently, with HOME/TMPDIR pinned and each arm asserted to hit its anchor (an arm that replaces a string which is not there silently tests the unmodified file):

  • print removed (if note: -> if False and note:) -> test_two_open_prs_are_told_the_pair_question_is_a_separate_reading FAILED, 102 passed. The one-PR control survives, which is the point.
  • control removed (if count < 2: -> if count < 0:) -> test_one_open_pr_is_not_told_about_a_pair_that_does_not_exist FAILED, and with it test_a_finished_pr_is_answered_as_over_and_hands_out_no_command - 2 failed, 101 passed. That second failure is a real consequence rather than noise: a queue of finished PRs also has no pair to name.

Tree restored after each arm (git diff empty, head still 2f4a7f3c); nothing was pushed.

The command it prints is runnable as printed. RUNNER is the file's own constant (uv run --no-sync python3), so the note cannot drift from the commands the rows print. I ran the line verbatim from this checkout: it read base caf6d743, scanned the open queue by default, and exited 0 - with one PR open it correctly reported 0 ordered pair(s), the same silence the control pins.

Why this belongs in the queue rather than the merge-gate list. The pair question is answerable only while both heads are still: once one merges, the other has to merge master in, which moves its head, and a head push voids every vote on it. So the reading has to reach a cycle at the moment it reads the queue - the moment it can still choose an order - not at the moment it is already merging. That is exactly the note's placement.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261021-074736

Measured on this head (2f4a7f3c), in the installation's own tree, and the head did
not move: the standing votes survive.

The tree this lands passes the suite. check-merge-plan-suite.py 2428 →
final tree 9a1db36ef532, 5391 passed, 19 skipped in 208 s. Base caf6d743
(master's Test 39502897420 on that base concluded success).

The landing change is the two paths the PR claims, nothing else.
check-merge-landing-diff.py 2428 → merging changes scripts/review-queue.py
(+33 −0) and tests/test_review_queue.py (+45 −0). The two other paths its page
shows (scripts/check-memory-rotation.py, tests/test_check_memory_rotation.py)
are the base's own later commit read as reversals — the hazard #2424 left behind,
printed as such by the reading.

The pair reading answers what it claims. check-merge-pairs.py 2428 2430 →
both ordered pairs merge into a healthy tree, 0 pair conflicts, so landing this
does not dirty the other open PR.

The one correction from the previous cycle still holds in this head: the count in
pair_note comes from len(readings), which includes PRs the queue could not
read, so "N open PR(s)" is an honest upper bound and the note does not claim more.

@argszero
argszero merged commit 5c9ed63 into master Oct 20, 2026
2 checks passed
@argszero argszero mentioned this pull request Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

emrg: the review queue never says whether two open PRs can land together

2 participants