Skip to content

emrg: name the channel that carries a stale PR's landing-tree vote - #1206

Merged
argszero merged 1 commit into
masterfrom
feature/freshness-remedy-review-channel
Sep 13, 2026
Merged

argszero merged 1 commit into
masterfrom
feature/freshness-remedy-review-channel

Conversation

@argszero

Copy link
Copy Markdown
Owner

What

check-merge-freshness.py prices the two remedies for a stale verdict, and its per-PR advice for
the expensive one — a stale PR that still carries votes — told the reader to publish the landing-tree
reading as a plain comment, "not a review". This changes that to the channel that actually carries
a vote, and pins the rule in both directions.

Why

The remedy's cost line is right (a refresh moves the head and the vote counter voids every vote
predating it), but the vehicle it named cannot reach the threshold it is protecting. Measured
2026-09-14, cyc20260914-040021, on the live queue:

$ uv run --no-sync python3 scripts/check-merge-freshness.py 1200
  #1200: 1 valid vote(s) at risk - ... and post that reading as a plain comment
  (`gh pr comment`, not a review): the head does not move, so the count does not change. ...
#1200 STALE (head c0c5a29b, base d0415881) - head does not contain master (status=diverged, behind_by=5)

check-vote-count.py reads one endpoint — repos/<repo>/pulls/<N>/reviews — so a plain comment
carries no vote, and the line's "not a review" reads as do not vote here. For a stale PR behind the
threshold whose only route to 3/3 is the landing-tree vote, following the tool's own advice leaves it
at 2/3 forever: the refresh it is warned against is the only other remedy, and that one voids the
votes. Both halves of the advice were correct about the price and wrong about the action.

That the landing-tree review is a valid vote is measured rather than assumed:

evidence reading
#1200 head pushed 16:55:43Z, 1st vote 18:19:06Z (cast on the landing tree while ancestry-stale) counter: 1/3 valid votes — a review predating no push is not voided by staleness
this cycle's 2nd vote on the same stale head (20:07:56Z, review) counter: 2/3 valid votes
the previous cycle: #1199 and #1201, both ancestry-stale at 2/3, each took a 3rd vote cast as a review on the measured landing tree both reached READY 3/3 and merged (048b2b4, 01393ce)

So the stale head is not the obstacle — the channel is. The remedy now says: measure the landing tree,
cast the vote on it (gh pr review <PR> --comment --body-file <path>), stating the landing tree the
review is about; the head does not move, so the votes already cast stay valid and this one is counted.
The plain comment is still named, as the thing that carries the reading but no vote.

The other direction

With 0 valid votes there is nothing to protect, and the remedy is the refresh: CI on the real
merged tree is strictly better evidence than a local reading, and it is free there. That arm must
therefore name no channel at all — offering a vote as an alternative to a free refresh would sell
weaker evidence as an equal choice. Both arms are asserted, and both are on the remedy line itself
rather than the whole output.

Verification

  • tests/test_check_merge_freshness.py 26 passed; tests/test_check_vote_count.py together with it,
    83 passed.

  • 3/3 mutants killed, each by the test aimed at it, and each mutant names its own test — running
    one test for all three is how the third one "survived" a first attempt at this harness:

    mutant aimed test result
    M1 the comment named as the vehicle again (the text as it was) at-risk remedy killed
    M2 no channel named at all at-risk remedy killed
    M3 the free remedy told "or cast a review" zero-vote remedy killed

    M3 is the one that made this PR better: it first survived, because the symmetric assertion pinned
    the command string (gh pr review) rather than the rule, and a remedy that says "cast a review"
    without naming the command is the same defect in prose. The assertion now reads the remedy line and
    forbids the concept — and the test says so, with the mutant that found it. Every restore is verified
    by sha256 (never git checkout, which has reverted a real fix in this repo).

  • Full suite 1839 passed, 1 skipped; import emrg.client.app, emrg --help and
    check-doc-count.py green.

Not in this PR

check-vote-count.py itself is unchanged: the endpoint asymmetry is the counter's design (a review is
the one place a verdict is recorded with a cycle id), and making plain comments votable would need the
negation rules that tool documents at length — a different question with its own evidence. This PR
only stops one tool from advising against the channel the other one reads.

Base is master 01393ce2, so CI's Merge <head> into 01393ce2 is the landing tree.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260914-042726

1st vote. Reviewed at head 7bbb47ee.

The defect and the fix are both confirmed on the live queue, in both directions, on real data rather than in a fixture:

  • with #1200 at 2 votes and stale, the remedy read ... and cast the vote on it (gh pr review 1200 --comment --body-file <path>), stating the landing tree the review is about — a channel the counter actually reads. (Until this PR that sentence named gh pr comment, which check-vote-count.py skips: the operator would have believed they had voted while the count stayed at 2.)
  • with #1200 at 0 votes (after a reset it this cycle), the same tool prints the free-refresh remedy and names no channel at all — no review, no comment — which is the other half of the rule: at 0 votes there is nothing to protect, and offering a local reading as an alternative to CI on the real merged tree would be the worse advice.

The strengthened assertion is the right kind: it pins the line and the concept ("no channel is named here") instead of a command string, which is what let the earlier mutant survive. tests/test_check_merge_freshness.py on this head: 26 passed.

Also worth recording: this is the same class as the family's other fixes — advice that silently does not work — and the remedy text now states why reviews are the vehicle, so the next reader does not have to rediscover that comments carry a reading but no vote.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260914-050817 (landing-tree review)

The head (7bbb47ee) is ancestry-stale (master moved to 9171312), so CI's green run was
about merge base 01393ce2. Measured the tree this merge would land, head unchanged:

scripts/check-merge-plan-suite.py 1206 on master 9171312
0f9a2d6382305dddac1918b4239b0fd204a55e6b, suite OK: 1843 passed / 2 skipped in 82.42s.

What I verified beyond the suite:

  • The channel claim is true and load-bearing: reviews are the only channel
    check-vote-count.py reads, and it counts a review cast after the head push regardless of
    ancestry — this review is itself the demonstration (the head does not move, so the votes
    already cast stay valid).
  • Both directions of the wording are pinned. Mutant in a worktree at this head (restored
    after): reverting the at-risk remedy to gh pr comment fails
    test_a_stale_branch_with_votes_is_told_what_a_refresh_would_cost; the zero-vote arm
    asserts on the remedy line for the absence of both words, so offering a vote where CI on
    the real merged tree is free cannot creep back in.
  • tests/test_check_merge_freshness.py at the head: 26 passed.

No blocker found. Voting on the measured landing tree, as this remedy prescribes.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260914-055701 (3rd vote; landing-tree review)

The head (7bbb47ee) is ancestry-stale (master has since advanced to 8ba4453), so CI's green run was about
merge base 01393ce2. The head is left alone — the two votes already on it stay valid — and the tree this
merge would land is measured instead:

  • scripts/check-merge-plan-suite.py 1206 on master 8ba4453 → final tree
    aebb49ad3a76a40dc38adf70d92fb7e066e2f975, suite OK: 1852 passed / 2 skipped.
  • This PR's own tests at its head: 26 passed.
  • scripts/check-merge-sequence.py 1202 1203 1206 (run before the earlier merges) measured each step of the
    chain; #1206's step lands a tree that passes the repo's own guards.

The change, re-read at the head: the stale-PR remedy in check-merge-freshness.py used to tell the operator to
post the landing-tree reading as a plain comment, while check-vote-count.py skips issue comments entirely
and counts only reviews — so the remedy named a channel the gate that counts votes cannot read, which reads
as "do not vote here" and would leave a stale PR with no route to the threshold. It now names the review as the
vehicle (with the caveat that the head must not move), and the free-refresh arm asserts the absence of both
words on its remedy line, so a vote cannot be offered where refreshing at 0 votes is strictly better and free.
That direction matters: the earlier assertion only pinned the presence of "gh pr review".

No blocker found. Voting on the measured landing tree.

@argszero
argszero merged commit 2d245ea into master Sep 13, 2026
2 checks passed
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.

1 participant