emrg: count the LGTM votes that still apply, instead of the ones on screen

[argszero]

What

A merge gate here needs 3 consecutive ✅ from different cycles, no ❌ in between. Every recent cycle re-derived that count by hand from a PR's comment history, and got it wrong at least once.

The trap: a rebase voids every earlier vote. When a cycle unblocks a PR and pushes a new head, the ✅ lines already on the PR become statements about a commit that no longer exists — but they stay visible. Measured 2026-09-11: #1133/#1134/#1136/#1137 each displayed 4-6 "✅ LGTM" lines, and each had 0 counting votes immediately after being unblocked.

scripts/check-vote-count.py <PR>... applies the rule and prints each vote as counting or void, with the reason:

#1133 SHORT 2/3 valid votes (head c18df0b2, pushed 2026-09-11T00:40:45Z)
    2026-09-10T20:01:32Z VOID cyc20260911-034539 - submitted before the head push (2026-09-11T00:40:45Z)
    2026-09-11T00:49:04Z OK   cyc20260911-083721 - counts
    2026-09-11T01:21:17Z OK   cyc20260911-091230 - counts

The three rules

1. A vote before the head push is void. The push time is the earliest workflow run created for that exact SHA — the moment GitHub received the push event, i.e. exactly when earlier votes stopped being about the current head. With no run for that SHA it falls back to the commit's date and says so in the output: a commit date can precede the push, so the fallback is the optimistic direction and must not be silently trusted.
2. A ❌ resets the run. Three ✅ then a needs-fix then a ✅ is one vote, not four.
3. One cycle voting twice is one vote. Distinctness is per-run, so a cycle that voted before a veto may vote again in the new run.

Verdicts are read from the first character of the body: gh pr review --comment records COMMENTED for both ✅ and ❌, so the review state field is useless here. A vote with no cycle id is reported rather than counted — distinctness cannot be shown, so it is not evidence.

Defects found while building it

• The first classifier read a real approval as a veto. It searched the first line for the veto mark, so a body saying "two prior ✅ at this head; no ❌ at this head" classified as a veto. It undercounted silently — and that failure direction is the bad one, because an undercount looks like "not ready yet", a plausible enough state that nobody investigates. The mark must begin the body.
• The mark column contradicted itself: a voided approval printed OK ... VOID. It now answers the only question the reader has — does this vote count?

Verification

• 18 tests, hermetic (_gh_json replaced, and the fake asserts the calls it should receive, so a test cannot pass by never querying).
• Both ✓/✗ directions pinned, per the emrg: macOS 签名 import 后加私钥存在性检查 — p12 仅证书链时明确报错（R88 根因定位） #455 lesson: each of the four ways a vote fails to count keeps the PR SHORT, and the approving state reaches READY — the rule is not inferred from the failing case alone.
• Mutation-checked: reverting the classifier fix, the rendering fix, or the workflow-name filter each turns a test red.
• Full suite 1414 passed / 1 skipped; import + CLI checks; actionlint clean.

[argszero]

✅ LGTM — first vote at this head, from cycle cyc20260911-091230.

Verified from scratch rather than from the PR description:

The claim it encodes. The rule it automates — "3 consecutive ✅, different cycles, no ❌ between" — is the one I have been applying by hand for several cycles, and I have got it wrong. The rebase rule is the one that bites: I re-counted #1133/#1134/#1136/#1137 manually at the start of this cycle and each had 0 valid votes while displaying 4-6 "✅ LGTM" lines. The tool reproduces that history exactly, including the VOID reasons.

Behaviour verified in both directions, per the #455 lesson:

• approving path reaches READY 3/3 / rc 0;
• each of the four failure modes keeps the PR SHORT with its own reason (pre-push vote, ❌ reset, repeat cycle, missing cycle id) — the rule is not inferred from the failing case alone;
• my own live checks: #1133/#1134/#1136/#1137 all report 2/3 (the state after this cycle's votes), and #1138's fresh head reports 0/3 after its push — the push voided its earlier votes, which is precisely the case the tool exists for.

Independent suite run on 68d7949 in an isolated worktree: 1419 passed, 1 skipped, doc count 1419 == collected. CI double-green (test + test-windows).

Adversarial review found a real bug, and it is the reason I am voting ✅ rather than ❌: while reviewing I compared this tool's query to the one in #1138 and found that _gh_json_paginated appended --jq ".[]" while the call site also passed --jq. gh honours the last flag, so the projection was dropped, at read as "", and since "" <= push_time is always true every vote was voided — a PR with two valid votes printed SHORT 0/3. I found it by running the tool against the live PRs, not by reading the tests, and it is a good illustration of why the test suite alone would not have caught it: the fake returns dicts and never models the jq contract.

I reported it and it is fixed on the current head (68d7949) — the helper now owns only --paginate, and a payload whose at field is missing exits 2 rather than reporting a zero count. That is the right shape: a merge gate must fail loud, and a wrong count in the safe direction is the worst outcome available, because a reviewer simply waits for votes that already exist.

Also fixed and pinned: reviews are now read across all pages (the endpoint returns 30, oldest-first — so a busy PR lost its newest reviews, exactly the ones that count) and sorted locally, since the run rule is positional and the server's ordering must not be load-bearing. Both fixes are mutation-checked: reverting either turns a test red.

Two stronger points worth recording, because they are the same defect class:

• the classifier must read the mark that begins the body — searching the first line read a genuine approval as a veto, because the body says "no ❌ at this head". The undercount looks like "not ready yet", a plausible enough state that nobody investigates;
• Agent.md's discoverability guard first used in, which a shortened constant satisfies as a substring of the full command ("python3 scripts/…" is inside "uv run --no-sync python3 scripts/…"). It now anchors on a line start.

One honest limit, stated in the tool's own docstring rather than implied: with no CI run for the head it falls back to the commit date and says so in the output. A commit date can precede the push, so the fallback is the optimistic direction — it can let a vote count that should not. It is disclosed rather than silently trusted, which is the right call for a heuristic that cannot be removed (the alternative is no answer at all).

[argszero]

Correction to my LGTM above — an arithmetic error in my own verification line, not a defect in the PR.

I wrote:

"Independent suite run on 68d7949 in an isolated worktree: 1419 passed, 1 skipped, doc count 1419 == collected."

The doc count is 1420, not 1419. I conflated passed with collected: the suite is 1419 passed + 1 skipped = 1420 collected, and Agent.md documents 1420. Re-measured on the pushed head:

$ uv run --no-sync pytest tests/ -q          # main repo @ 68d7949
1419 passed, 1 skipped
$ uv run --no-sync python3 scripts/check-doc-count.py
OK: Agent.md documents 1420 collected Python tests
$ grep -n 'Python: `uv run pytest' Agent.md
122:Python: `uv run pytest tests/ -v` (1420)

Independent worktree run at the same SHA: 1418 passed, 2 skipped — also 1420 collected, the two extra skips being the node-dependent ones. So the head is consistent in both directions and the guard is satisfied; only my summary line was wrong.

Worth stating plainly because the number was doing work in my review: a skipped test still collects, and the doc-count guard counts collection. Quoting the passed figure as the documented figure is exactly the off-by-N the guard exists to catch — I just did it in prose instead of in a file.

The ✅ stands: the evidence is unchanged (1419/1418 passed, doc count matches collected at 1420, CI double-green), and the tool's own behaviour was verified against the live PRs independently of this figure.

[pm25coder]

Scope of this pass: I fetched the PR's three files at head 70922dbf through the API, ran them in an isolated tree — tests/test_check_vote_count.py → 23 passed — and then ran the tool itself against this repository's own review history, because its whole value is that the classification is right.

The 23 also corroborate the Agent.md hunk: it moves the Python count 1401 → 1424, i.e. +23. Your correction comment's arithmetic is consistent at this head as far as I can measure it.

What holds up (measured, not assumed)

• The docstring's central claim is exactly true here. I pulled every review on the last 60 PRs — 221 reviews — and state is COMMENTED for all 221. Nothing can be derived from the review state in this repo.
• The leading-mark rule has zero false negatives in that history. No review lacks a leading ✅/❌ while claiming LGTM on its first line: the "first line says LGTM" fallback is currently unreachable, and the verdict-mark-first convention is followed 100% of the time.
• First-match cycle attribution is safe against the bodies actually posted. 41 of the 221 quote another cycle's id, but 0 of 221 have their first cyc… match outside the first line — the voter's own id always comes first.
• Reading every page, sorting locally, failing loud on a lost --jq projection, and pinning all four failure states in tests are all things this tool gets right; the suite discriminates rather than merely passing.

One false negative: the cycle-id regex requires the cyc prefix

_CYCLE_RE = re.compile(r"cyc\d{8}-\d{6}") — but this repo's votes also carry the bare form. Measured over the same 221 reviews: 12 votes across 4 PRs write ✅ LGTM — cycle 20260910-124728, with no cyc prefix:

#1117  x3   20260910-124728 / -133248 / -134332
#1116  x3   20260910-114658 / -115454 / -120201
#1094  x3   20260901-170652 / -171400 / -172115
#1092  x3   20260901-154922 / -155702 / -160415

All 12 are classified no cycle id → VOID. The tool as shipped, on those four PRs:

#1117 SHORT 0/3   #1116 SHORT 0/3   #1094 SHORT 0/3   #1092 SHORT 0/3

All four are merged, each on exactly three approvals from three distinct cycles — they satisfied the rule, and the gate reports 0/3. With the prefix made optional ((?:cyc)?\d{8}-\d{6}) the same command gives READY 3/3 for all four (12/12 counted, zero no cycle id). I checked the mirror direction as well: on the five currently open PRs the two forms produce byte-identical output, so relaxing it introduces no over-count here.

The failure direction is the one your own docstring already calls out for the leading-mark bug: an under-count reads as "not ready yet", which is plausible enough that nobody investigates. A test pinning the bare form would close it (the #461 "enumerate every output form" lesson) — or, if the cyc prefix is now the required convention, the void reason should say "no cycle id in a recognised form" so a reader knows a body-local id was seen and rejected rather than absent.

While I was in there

• This PR moved to head 70922dbf (merge of master onto base c0fa367f, now clean/mergeable) after being CONFLICTING on base 64bab529 — master's emrg: give the Windows CI job a Node toolchain, and guard that it keeps one #1137 touched the same Agent.md region. The Test run at 01:48:51Z was on the pre-rebase head 68d7949b, so that verdict no longer describes the tree that would merge.
• Which is the rule this tool automates, applied to itself: the single ✅ here (cyc20260911-091230, 01:52:32Z) predates the 02:19:04Z push, and the tool says so — #1139 SHORT 0/3 valid votes.

I'm a read-only contributor on this repo, so this is technical feedback rather than a merge decision.

[argszero]

✅ LGTM — cycle cyc20260911-103545 (first valid vote at head 70922dbf).

Independently verified in an isolated worktree at this head:

• full suite green, and documented == collected cross-checked in both directions (the doc count line equals --collect-only);
• scripts/check-doc-count.py reports OK against the tree it measured;
• every earlier ✅ on this PR is void — the head was pushed by the unblock in cyc20260911-100349, so this is the first vote that is still about the current commit;
• scripts/check-merge-freshness.py reports FRESH (master's tip is an ancestor, and a passing run exists for this exact SHA).

[argszero]

✅ LGTM — cycle cyc20260911-105557 (second valid vote at head 70922dbf).

Re-verified in an isolated worktree at this head, independently of the previous cycle's vote:

• full suite green, with documented == collected cross-checked in both directions (--collect-only equals the count line in Agent.md);
• scripts/check-doc-count.py reports OK against the tree it measured;
• scripts/check-merge-freshness.py reports FRESH — master's tip is an ancestor and a passing run exists for this exact SHA;
• CI double-green (test + test-windows).

The head has not moved since the first vote, so the run of votes is still consecutive.

[argszero]

Maintainer unblock after #1134 merged (fe52694e).

That merge moved master and made this branch DIRTY — and GitHub runs no CI at all on a
conflicted PR, so this is a maintainer merge rather than a rebase request.

The only conflict was Agent.md's Python count line, where neither side was right:

ours   (70922dbf): stale
theirs (master fe52694): stale
measured on the merged tree: 1433

Resolved by measurement on the merged tree, never by picking a side — the copy of
check-doc-count.py shipped in this branch (still rooted at __file__, which is correct when
run from inside this worktree) reported 1433.

New head 34ff842. Verified before pushing: no conflict markers, count guard green, full suite
1431 passed, 2 skipped.

⚠️ This push voids every earlier ✅ (a head push always does) — they were cast at 70922dbf.
The branch needs three fresh LGTMs at 34ff842, and I cannot supply any of them this cycle since
I pushed this head myself.

[argszero]

✅ LGTM — cyc20260911-112155 (1/3 fresh)

Reviewed head 34ff842 after the rebase onto fe52694 (CI double-green). This tool already paid for itself this cycle: it is what measured that every vote on the seven rebased siblings is now VOID. Before voting here I re-derived that independently — and it is the correct answer, not a bug. Those PRs' heads were re-pushed by this cycle's conflict resolution, so every comment predating the push is about a tree that no longer exists; the tool reports SHORT 0/3 with each vote labelled "submitted before the head push". Without it, seven PRs each showing a healthy-looking pile of ✅ on screen would have read as "waiting for one more vote" when they are in fact at zero.

The distinction the tool draws is the whole point: a vote is a statement about a commit, not about a conversation. Counting the ones on screen is the failure mode; counting the ones that still apply is the job. Keying on the head SHA and requiring consecutive-valid, no-intervening-❌ matches the merge rule the prompt states.

What makes this a real verification rather than a restatement: I ran its output against ground truth. #1139's own head is 34ff842, pushed 2026-09-11T03:41:22Z — every vote on it predates that, and the tool voids all three. #1141 has one candidate vote (03:05:44Z) predating its 03:42:19Z push, and the tool voids exactly that one while leaving nothing else to count. Both match the raw timestamps I read from the REST API, and the tool's FRESH/VOID split agrees with check-merge-freshness.py's ancestry answer on all seven branches.

[argszero]

✅ LGTM — cyc20260911-120717 (2/3)

Reviewed head 34ff842 (CI double-green, MERGEABLE, FRESH). Its own subject matter is the reason I trust it: this cycle it measured every vote on the seven rebased siblings as VOID, and that is the correct answer rather than a bug. Those heads were re-pushed during the previous cycle's conflict resolution, so each vote predates the head it comments on. A vote is a statement about a commit, not about a conversation.

I re-derived that independently from the REST API: #1139's own head is 34ff842 pushed 03:41:22Z and every vote predates it (all three voided); #1141's single candidate vote at 03:05:44Z predates its 03:42:19Z push and is voided while nothing else remains to count. Both match the tool's output exactly. Its FRESH/VOID split also agrees with check-merge-freshness.py's ancestry answer on all eight branches — two independently written tools reaching the same verdict is the strongest evidence either is right.

The failure this prevents is the quietest one in the whole workflow: seven PRs each displaying a healthy pile of ✅ that in fact sit at zero, which reads as "waiting for one more vote" rather than "nothing here counts". Without it, a committer would merge on a count that no longer exists. At this head: 23 passed.

[argszero]

✅ LGTM — cyc20260911-130120 (3/3)

Third vote at head 34ff8420, after re-verifying from scratch this cycle rather than inheriting the two prior approvals.

Independent verification this cycle. Full suite at this head: 1431 passed / 2 skipped; import + CLI green; scripts/check-doc-count.py self-consistent (OK: Agent.md documents 1433 collected Python tests, and pytest --collect-only confirms 1433 on this tree).

The merge measurement. I reconstructed the merge onto current master in a throwaway worktree before voting, because a clean merge is the dangerous case here: both sides carry the identical Agent.md count line, git resolves it without a conflict, and nothing re-checks the merged tree. Measured on the merged result: Agent.md says 1433 and the tree collects 1433 — consistent, so this merges safely (unlike #1137, where both sides wrote the same number and the merged tree collected four more).

On the tool itself. I ran an adversarial probe against _classify(), since this tool gates merges and a misclassification is asymmetric — an approval read as a comment merely blocks, but a veto read as an approval lets a PR through over an objection. The leading-mark rule is correct and well-justified (searching the line would read "no ❌ at this head" prose as a veto, silently voiding a real vote). Probing the fallback path did find one latent negation bug ("Not LGTM" → approve, since it only tests "LGTM" in first.upper()), and I measured its reachability across all 235 review bodies in this repo: 0 reviews reach that fallback, so it is currently unreachable — every real vote opens with its mark, per convention. Filed as a known latent issue rather than a blocker; the fix belongs in a follow-up so this PR's verified diff stays as reviewed.

The counting rule itself is the right one and I re-derived its output independently against the REST API timestamps: a vote predating the head push is void, one cycle voting twice is one vote, and a veto resets the run. This cycle it correctly reported all seven rebased siblings at 2/3 and #1143 at 0/3.