[cgwalters-bot]

Stage 2 of the pass took about 200 requests after two repositories were followed, so the pass hit the start reserve with candidates unconsidered. A recorded pass (--record, once) shows 62 pull requests at two requests each, of which 29 only come to skip: labelled needs-human.

Changed: a labelled pull request no stale stop could act on (no conflict, no change request) takes no read of its own, from one query per repository with such a pull request; and a pull request's comments come with its query, the REST list only past the last 100. The label is now read with the stop rather than after it, so a stop under a minute old with no label reads the label again.

Replay of that recording, its reads rewritten offline for the new queries from its own answers:

	before	after
pull requests stage	173	98
pass, all stages	447	382

Every pull request's decision line is the same. The one other difference in the log is the weekly sweep's sweep: budget: … line going away, as it now has room (its own reads take the schedule stage from 168 to 178). Triage pool choices are unstable between replays of either build, so they were not compared.

Verified: just check, exit 0 (cargo test -p board-forge: 620 passed, 0 failed).

Checked live by hand: LABELLED_QUERY as written, on one repository; comments(last: …) is oldest first and its fullDatabaseId matches the REST ids. Not verified: a live pass with the new binary. An applying pass's stage figure is higher than a preview's by its own writes (206 against 173 before this). operatorAnswers, which the pass config does not accept, keeps every labelled pull request read whole; only poll policy can set it, and no test exercises that branch.

Generated-by: https://github.com/cgwalters/#llms

[github-actions]

VERDICT: APPROVE: no blockers; a labelled pull request still reaches every write path it could reach before

Reviewed head: b39d72f9c791a1875194a429142eb9e59a02b75d (merge base f1dd2c7).

What I checked and why I am not blocking:

• Skipping labelled pull requests (left_labelled / labelled_skip). Before this change, pull() could do only two things with a needs-human pull request other than skip it: stale and the operatorAnswers answer path. stacked, removals, fix/review runs and check retries all bail on labelled(&pull), and decide returns skip: labelled needs-human. With operatorAnswers set, the shortcut is turned off. may_be_stale covers more than stale can act on: it reads anything not MERGEABLE, any CHANGES_REQUESTED review (the job's verdict is a review state as well, see newest_verdict), and reviews past the page. A failed query or budget spend returns an empty map, so every pull request is read whole as before. The line, the Wait (operator_wait gives needs-human), Stepped (only where !merge_only, which is the same as author == bot from the list filter), ran (take returns None for skip) and flying (in_flight is false when labelled) all match the old path. The shortcut makes no writes, so the worst it can do is miss a stale action, and the superset check prevents that.
• Comments from PULL_QUERY. rest_comment names authors with author_login, so a Bot called after the bot gets [bot] and cannot forge a marker (the test covers this). Ids come from fullDatabaseId, ordering is ascending as in REST, and the REST list is used when hasPreviousPage is not false, including when comments is missing. GraphQL is uncached, so the rule that an applying pass reads markers uncached still holds.
• Stop/label ordering. One query now returns both the label and the stop. A stop with no label is read again if it is under 60 s old or its time is unknown. self.now is never later than the real time, so the check errs towards reading again. The comments listed past 100 are read after the label, and the same re-read covers them.
• No manifests, lockfiles, workflows, permissions, CODEOWNERS or merge policy are changed. Stage 2 is not built on Forgejo, so the new query is not reachable there.

Follow-ups

1. important: test coverage. The recorded PULL_QUERY answers in the existing pass tests have no comments field, so almost every existing test still takes the REST fallback in pull_comments (crates/board-forge/src/poll.rs:5676). The GraphQL-shaped comments reach only pull_request_comments_come_with_its_query. Stale close, claim fencing and the check-rerun receipt scan read comments by id/user.login/created_at, and none of them are run against comments built by rest_comment. Add at least one stale-close or claim case whose comments come from the query.
2. nit: the label is now decided at the start of the walk (crates/board-forge/src/poll.rs:6319), not when each pull request's turn comes. A person who takes needs-human off while the walk is running gets their reset one pass later. A labelled pull request closed between the list query and its turn now produces a needs-human wait instead of skip: closed since the list was read. Both are bounded to one pass, but neither is documented or tested.
3. nit: when LABELLED_QUERY fails, it falls back without saying so (crates/board-forge/src/poll.rs:5712): no line, no error. That is safe, but a replay or log cannot show that the saving was lost. A diagnostic line would help.
4. nit: rest_comment fails the whole pull request (an error line on every pass) when a node's fullDatabaseId is missing or does not parse (crates/board-forge/src/poll.rs:3471). Falling back to the REST list would be more robust.
5. nit: merge_only (mergeAuthors) labelled pull requests also go through may_be_stale (crates/board-forge/src/poll.rs:5718), although stale needs is_harness. A conflicting dependabot pull request is read whole with nothing to act on, which costs requests but is not a correctness problem.
6. nit: the doc line /// One pull request's decision line, and whether it is in flight. was left above the new read_pull (crates/board-forge/src/poll.rs:5649). read_pull now has two summaries and pull has none.

Generated by review for #552 at b39d72f

[github-actions]

VERDICT: APPROVE: no blocker found; labelled pull requests skipped without a read come to the same decision, Wait and Stepped as before

Reviewed head: 4515da68e676822e976d10f069b0ebfdff48647a (merge-base 2ccc28d).

What I checked against the base:

• labelled_skip (crates/board-forge/src/poll.rs ~5720) against the full pull path for a labelled pull request. decide returns skip: labelled needs-human first. stacked, the merge-queue removal, the fix run and the review run all exclude labelled pull requests. in_flight is false. step_kind("skip") is None, so caps are unaffected. take does nothing for a skip with no queue. operator_wait gives needs-human whatever the author. Stepped keyed on !merge_only matches the old author == bot, because numbers already filters followed authors from the REST list. moved_at reads the same paths LABELLED_QUERY returns. The line text is the same.
• may_be_stale is a superset of what stale acts on. head_review yields ChangesRequested only from a review with state CHANGES_REQUESTED (newest_verdict), and conflicts are covered by mergeable != MERGEABLE. Reviews past the page, a failed query and pull requests past 100 are all read whole. operatorAnswers disables the shortcut, which keeps recover_answer_stop/answer working. staleDays: 0 skips every labelled pull request, which is right because stale returns early there.
• A pull request closed after the open list was read drops out of states: OPEN and is read whole. One labelled after the list was read is skipped, which is correct.
• rest_comment produces every field the comment consumers read (id, user.login, body, created_at, updated_at). The Bot [bot] suffix keeps an App named like the bot from writing a marker, and a test covers that. find_marker works by position, and comments(last: 100) is oldest first, as in REST. The stop is always a new POST (needs_human → post_marker), so stopped_at's created_at is the time of the stop.

This touches no workflow, permission, dependency or lockfile. The worst merged outcome I can see is a false "label removed" reset (the next finding), which starts fresh fix rounds on a pull request another pass just stopped. That is still bounded by the claim protocol and the fix bounds.

Follow-ups

1. important: crates/board-forge/src/poll.rs ~5763-5782 (pull). The old invariant was "stop read strictly before label". It is now a timing heuristic: labels and comments come from the same GraphQL query, or the labels come before the REST comments once there are more than 100 comments. A stop with no label is trusted as a reset once self.now - created_at >= STOP_WRITES_SECONDS (60 s). Correctness now depends on the runner's clock being no more than about 60 s ahead of GitHub's, and on GraphQL never showing the comment without the label for longer than that. If either fails, this pass reads another pass's fresh stop as a person's reset and dispatches a round. Neither is likely, but the doc comment should state both assumptions. A cheaper, exact fix for the REST fallback is to keep the old order there: read the REST comments first, then read_pull.
2. important: crates/board-forge/src/pass/tests.rs pull_request_comments_come_with_its_query. No case has a stop that is new and comes from the REST fallback (more = true), which is the path where the labels are read before the comments. There is also no case for fullDatabaseId missing or non-numeric, which now fails the whole pull request with a pull request comment has no id.
3. important: crates/board-forge/src/pass/tests.rs labelled_pull_requests_are_read_only_to_act_on. No case covers operatorAnswers: true, where the shortcut must be off; the PR body admits this branch is untested. There is also no merge_only (mergeAuthors) labelled pull request, which must yield a Wait and no Stepped. Both are one row each in the table.
4. nit: crates/board-forge/src/poll.rs ~6319 (follow). left_labelled is computed once before the repository's loop. A label a person removes while the walk is in progress is still skipped until the next pass, where the old per-pull read would have caught it. This only adds latency, but the doc comment could say so.
5. nit: crates/board-forge/src/poll.rs ~5700 (left_labelled). The LABELLED_QUERY request is spent before the per-pull loop's start_kind, so per-kind request tallies charge it to whatever kind the previous repository ended on.

Generated by review for #552 at 4515da6