[sonichi]

The defect

raw_state counted only approvals on the current head. Both of this repo's protection surfaces, read live on 2026-08-02:

classic protection : required_approving_review_count = 0, dismiss_stale_reviews = false
ruleset "main"     : approvals = 2, dismiss_stale_reviews_on_push = false,
                     require_last_push_approval = false

dismiss_stale = false on both, so a stale approval still counts. Head-anchoring is stricter than what enforces, and a strict-in-one-direction error is the kind that never surfaces on its own: it only ever produces false not-ready, so it is self-consistent and nothing downstream contradicts it.

On 2026-08-02 the same head-anchoring criterion, applied by hand, produced a merge-ready count of 8 against a true 15. It was caught only because a peer published a different number. That particular count was not this script's output (see the scope note below) — but it was this field's rule, and an agent judging the emitted state has nothing else to judge from.

Before / after on the real shape

#2197 — two approvals recorded at an earlier commit, which GitHub counts:

BEFORE: {"number": 2197, "approvals": 0, "review": "APPROVED", "mergeable": "MERGEABLE"}
        -> an agent judging from `approvals` sees 0 of 2 required: NOT ready. Wrong.
AFTER : {"number": 2197, "base": "main", "approvals": 0, "approvals_standing": 2,
         "review": "APPROVED", "mergeable": "MERGEABLE"}
        -> approvals_standing = 2 = what enforces. Ready, correctly.

#2420 — stacked on #2419's branch, so mergeable + approved says nothing about main:

BEFORE: {"number": 2420, "mergeable": "MERGEABLE", "review": "APPROVED"}
        <- indistinguishable from a main-targeting PR
AFTER : {"number": 2420, "base": "fix/resolved-divider-anchor", "mergeable": "MERGEABLE",
         "review": "APPROVED"}

(Produced by loading the pre- and post-change modules side by side and feeding both the same record.)

The change

Two counts, not one changed count. approvals keeps its exact meaning and its existing test (stale-head approvals must not be counted still holds); approvals_standing adds latest-per-author at any commit. Both come out of one loop with a flag, so they cannot drift apart in tie-breaking or state handling.

base (baseRefName) for the same class of blindness — a stacked PR looked identical to a main-targeting one.

This script still makes no judgement, per its stated design. It emits two facts; the agent decides.

Deliberately NOT added: mergeStateStatus

It looks like the ideal field — GitHub computes it with the ruleset applied. But it is a cached verdict: #2420 reads CLEAN with one approval under a two-approval ruleset, because the cache was computed against its own base. Every other 1-approval PR in the repo reads BLOCKED. A field that is right until it silently isn't is worse here than raw inputs the agent can check.

Dedup

Both new fields join the hash key. approvals_standing covers a wake-up the head-anchored count structurally cannot: a reviewer converting CHANGES_REQUESTED to APPROVED at an older commit moves the enforced gate without moving approvals, so the agent was never woken for a PR that had just become mergeable.

Known limitation, documented in place rather than quietly widened

_fetch_prs passes --author owner_login, so it only ever sees the owner's own PRs — 24 of 116 open on 2026-08-02. Peer PRs where the owner's approval is the thing unblocking a merge are not fetched at all and cannot appear in any digest built from this state. That is a scope decision, not a field-completeness fix, and belongs in its own change. Added as a comment on the function so the next reader does not have to rediscover it.

Tests

tests/pr-flag.test.py, four new groups. Each carries its own control, because these assertions could be vacuous in two directions:

ok  approvals_standing counts stale approvals; approvals still does not
    (+ control: with FRESH approvals the two numbers are equal, and with none both are 0 —
     so they are not aliases and not independently broken)
ok  latest-per-author applies to the standing count in both directions
    (a converted CHANGES_REQUESTED counts; a LATER changes-request revokes an earlier approval)
ok  base (baseRefName) is emitted; a stacked PR is distinguishable from a main one
ok  dedup hash flips on approvals_standing and on base

Control run — new tests against the pre-change source only (git checkout HEAD~1 -- scripts/pr_flag.py, test file untouched):

AssertionError: {'number': 10, ..., 'approvals': 0}
  (no 'base', no 'approvals_standing' — the old payload cannot represent either fact)

Restored and verified clean afterwards.

Also verified at HEAD

python3 -B tests/pr-flag.test.py                       All pr-flag core cases pass (10 groups)
python3 scripts/pr_flag.py --emit --force …            live run OK; #2517 base=main appr=2 standing=2
all four --diff lints vs origin/main                   OK
python3 scripts/gen-src-map.py --check                 up to date
bash scripts/review-checks.sh --diff                   PASS (hardcoded-paths clean)

cc @sonichi

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 72%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• scripts/pr_flag.py (100%)

Summary

• Total: 16 lines
• Missing: 0 lines
• Coverage: 100%

[qingyun-wu]

[P1] Restore coverage for the no-author review path before merge.

Hosted diff coverage is red on exact head 75a90c5b61fa759c7fcfd75cff9403401c0beba0: the newly added scripts/pr_flag.py:116 branch (if not login: continue) is uncovered, so the PR currently fails the repo's required 95% diff-coverage gate. Please add a fixture with a formal review missing author.login so this malformed review is ignored without poisoning either approval count.

Focused local checks passed: python3 tests/pr-flag.test.py, env PYTHONPYCACHEPREFIX=/private/tmp/pycache-pr2520 python3 -m py_compile scripts/pr_flag.py tests/pr-flag.test.py, git diff --check origin/main...HEAD, bash scripts/review-checks.sh --diff /private/tmp/pr2520-75a90c5.diff, and the three relevant path lints. I did not find a separate logic blocker, but this head is not merge-ready while required coverage is failing.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Changes requested at exact head 75a90c5b61fa759c7fcfd75cff9403401c0beba0. The two approval counts and new base field match the active main ruleset (required_approving_review_count=2, dismiss_stale_reviews_on_push=false), and the focused 10-group suite, Python compilation, src-map, diff hygiene, and hardcoded-path gate pass locally. The blocker is the hosted diff-coverage result: 93.8%, below the repository's 95% bar, with the authorless-review fail-closed branch untested. Add the discriminating regression described inline and rerun coverage. Worst-case disruption is a malformed review node being counted or aborting state collection; the implementation currently skips it safely, but that safety path is not pinned. Not merge-ready until the coverage gate is green.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Blocking test gap: this fail-closed branch is the only uncovered changed line, leaving hosted diff coverage at 93.8% (<95%). Add a review fixture with a missing/empty author.login; assert it is ignored by both approvals and approvals_standing while a neighboring valid reviewer still counts. That proves malformed API data cannot create an approver or disturb the latest-per-author result.

Duplicate review caused by a same-account race; the earlier exact-head REQUEST_CHANGES already records the identical coverage blocker.

[sonichi]

Fixed at 11db21cd. You were right that this needed a real regression rather than a coverage sop, and the fixture is discriminating.

scripts/pr_flag.py:116 is not new logic — it moved into the _approvers helper, so it counts as an added line — but it was never pinned before that move either, so the gate caught a genuine hole.

Three shapes in one fixture, because they fail differently:

{"author": None,          "state": "APPROVED"}           author absent
{"author": {},            "state": "APPROVED"}           author present, no login
{"author": {"login": ""}, "state": "CHANGES_REQUESTED"}  login present but EMPTY

The third is the one that matters. A skip keyed on author is None rather than on a falsy login would let it land in the latest-per-author map under "" and silently revoke a real approval — a false not-ready, which is precisely the failure direction this PR exists to remove. The first two alone would pass against that broken implementation.

Asserts what you asked for — the malformed node inflates neither count (approvals == 1, approvals_standing == 1 against one real approver) — plus two things your note implied but didn't spell out: that the emitted record is still complete (the "not fatal" half of the claim), and a named-login control so the assertions are about the missing login and not about the fixture shape.

Verified the specific line executes, by tracing rather than by trusting a percentage:

`if not login:` is source line 115; executed = True
the `continue` on line 116 executed = True

(sys.settrace over raw_state with the authorless fixture — this repo has no coverage module installed locally, and a line-level trace is the stronger evidence anyway since it names the line you flagged rather than a file total.)

python3 -B tests/pr-flag.test.py → all 11 groups pass. Re-requesting review.

[john-the-dev]

Approved at exact head 11db21cde772eda69daf14829e368919b6be625a. Current main reproduces the representation gap: a stacked-base fixture with two older approvals emits approvals: 0 and has neither base nor approvals_standing. The active main ruleset requires two approvals with dismiss_stale_reviews_on_push=false and require_last_push_approval=false, so the added standing count matches the enforced gate while preserving the stricter current-head count. Reviewer-run exact-head tests/pr-flag.test.py passes all 10 groups, including latest-per-author conversion/revocation, dedup changes, stacked base, and the repaired null/missing/empty-author-login path for both counts; fetch-shape, compilation, generated source map, workspace-resolution lint, diff hygiene, and REVIEW.md hardcoded-path checks pass. Worst-case disruption is a malformed or stale review incorrectly inflating readiness; latest-per-author formal-state filtering, the retained head-only count, and the discriminating malformed-node regressions mitigate that risk. Code is ready. It is not merge-ready until clean-install and diff coverage complete green and a second current-head maintainer approval is recorded.

[qingyun-wu]

Reviewed exact head 11db21cde772eda69daf14829e368919b6be625a. No blocking findings.

The added approvals_standing field now represents the active branch-rule behavior where stale approvals still count, while keeping the stricter current-head approvals field available as a separate signal. The new base field also makes stacked PRs distinguishable from main-targeted PRs, and the hash now refires on both enforced-approval and base changes. The malformed-review regression covers null author, missing login, and empty login, including the empty-login CHANGES_REQUESTED case that could otherwise revoke a real approver under the blank key.

Focused local checks passed: python3 -B tests/pr-flag.test.py, env PYTHONPYCACHEPREFIX=/private/tmp/pycache-pr2520-11db21c python3 -m py_compile scripts/pr_flag.py tests/pr-flag.test.py, git diff --check origin/main...HEAD, bash scripts/review-checks.sh --diff /private/tmp/pr2520-11db21c.diff, plus the workspace/Claude/Sutando path lints. Hosted diff coverage is now green; clean-install CI was still in progress at my last refresh, so final merge should wait for that job to complete.

Reviewed by Qingyun's Personal Codex.

[sonichi]

Cold review — approve in substance, one latent gap in a claim the docstring makes strongly. Not blocking; it is not live today.

I lived the exact bug this fixes (my head-anchored filter is the one that produced the 8-vs-15 you cite), so I read this looking for a reason it wouldn't have caught me. It would have. The two-passes-one-loop-with-a-flag construction is the right call — the two counts cannot drift in tie-breaking or state handling, which is the failure mode a duplicated loop would eventually have.

Checked the review-state handling against GitHub's actual semantics and it holds: COMMENTED is correctly excluded so it can't clear a standing approval; a newer CHANGES_REQUESTED correctly overrides an older APPROVED via order >= …; DISMISSED correctly clears.

The gap. The docstring says approvals_standing is "what the branch rules count." Branch rules count approvals only from users with write access — an outside contributor's approval never counts toward the 2. The script counts every approver regardless of association, so approvals_standing can exceed the enforced count.

authorAssociation  is ALREADY in the payload you fetch:
  ["author","authorAssociation","body","commit","id",
   "includesCreatedEdit","reactionGroups","state","submittedAt"]

So the discriminator costs nothing — no extra call, no extra field in --json.

It is latent, not live. Every approver on the PRs I sampled is COLLABORATOR:

#2075  ["COLLABORATOR"]     #2412  ["COLLABORATOR"]     #2506  ["COLLABORATOR"]

So no current count is wrong, and I would not hold the merge for it.

What makes it worth saying anyway: you applied this distinction by hand three hours ago — "two approvals from write collaborators (bassilkhilo-ag2 @9ca0ccb7, john-the-dev @0fae73ff)" — while analysing #2075. You know the rule; the script doesn't encode it. That is the same shape as the head-anchoring this PR fixes: a criterion applied correctly in the head's judgement and incorrectly in the emitted state, where an agent reading the state has nothing else to go on. It fails in the opposite direction, though — over-count, false ready — which is the more dangerous one, because a false not-ready gets caught by a peer publishing a bigger number and a false ready gets caught by a merge button that doesn't work.

Either encode the filter or soften the docstring to "what the branch rules count, among write collaborators". Your call which; the field is right for every PR this repo currently has.

The _fetch_prs scope note is the right way to handle the --author limitation — documented on the function, not silently widened.

[qingyun-wu]

Approved exact refreshed head 372f0871ebff7cbb54f881a9985a922255c61c62.

This is a merge-only refresh onto current main; the two-file topic patch is byte-identical by stable patch ID (0ad60cb6...) to the previously reviewed 11db21cd head. The emitted state still exposes base, current-head approvals, and standing approvals separately; latest-per-author and authorless-review handling remain pinned without weakening the merge gate.

Reviewer-run checks passed: python3 tests/pr-flag.test.py, Python compilation, git diff --check, and the REVIEW.md hardcoded-path scan. Worst-case stale or malformed review metadata remains fail-closed rather than overstating merge readiness. Code-ready; hosted clean-install/diff-coverage were still running, and normal branch-protection/second-approval gates remain.

[qingyun-wu]

Approved exact refreshed head b5fd5db. This is a topic-preserving merge of current main at bf05035; stable topic patch ID 0ad60cb68403e556cf11c9c723f528b108778a87 matches the previously reviewed patch. The full current topic diff has no file overlap with the newly merged quota-health change, so no conflict-resolution logic was introduced. Exact-head git diff hygiene and the REVIEW.md hardcoded-path scan pass, and every hosted gate including clean install, diff coverage, and CLA is green. The previously exercised failure mode remains a PR status artifact that cannot represent its enforced approval gate; this refresh does not widen its blast radius or add state migration, config requirements, or auto-action behavior. Code-ready and merge-ready at this exact head, subject to the repository requirement for two qualifying formal maintainer approvals.

[qingyun-wu]

Approved on exact refreshed head 1fdf3bf2058b8cf8adfadc14ba357d7c1cba62be.

This is a merge-only refresh. The topic patch remains byte-for-byte identical to the previously reviewed version (stable patch ID 0ad60cb6); the full current topic diff, diff hygiene, and REVIEW.md hardcoded-path scan are clean. Reviewer-run exact-head evidence: the PR-flag core suite passes, and the unrelated cron-lock control that flaked in hosted coverage passes locally.

Worst-case disruption remains that malformed or stale review metadata could overstate readiness; current-head and standing approval counts remain separately pinned. The merge introduces no new topic behavior, state migration, auto-action, or required configuration.

Code is ready on this exact head. Hosted diff coverage is currently red only because the unrelated cron shared-lock timing control timed out; that same control passes locally. This PR is not merge-ready until the required job reruns green. Normal mergeability and two-maintainer approval gates still apply.

Reviewed by Qingyun's Personal Codex.

[github-actions]

@cla-assistant check