Skip to content

perf(source-control): stop judging a pr-merge candidate once a lane merge is in flight - #8187

Merged
kyle-sexton merged 2 commits into
mainfrom
perf/pr-merge-inflight-early
Oct 18, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
perf/pr-merge-inflight-early

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: pr-merge throughput (2026-10-18), sweeps time out at 44 of ~106 candidates.

Summary

Run 39113451592 reached 44 of about 106 candidates before the 30-minute timeout, at about 41 s each, and 26 of its 43 refusals were in-flight:identity-entry-queued:#8036. pr-merge merges one lane pull request at a time, so while one is queued every other candidate is refused in flight, yet assess and score_unattended still read eligibility, promotion, risk evidence, changed files and the verifier verdict for a refusal that was already decided. This change returns that refusal at once.

Fix

  • in_flight also returns queued, every merge-queue pull request number (None when the queue was not read whole). The field is compare=False, so existing InFlight equality is unchanged.
  • In assess, once the rung is not off and in_flight refuses, the refusal returns at once when the candidate is not itself in the queue and is not a Dependabot pull request. The reason (in-flight:<reason>), enabled=True, and the rung, cap and in-flight details are the same as before. Eligibility, promotion, and through score_unattended (which returns early with no eligibility) the risk evidence, changed files and verifier verdict are no longer read.
  • These cases still get the full judgment, as before:
    • A queued candidate, because its own_refusal (ineligible, class above rung, verifier refusal, risk) is what removes its own entry, and the sweep skips the verifier hold for the candidate's own number.
    • A Dependabot candidate, because its eligibility decides the gate's dependency-manager hold (lane_dependabot in main), which changes the blocker list.
    • A candidate whose queue read was not whole.
  • What changes downstream: an off-queue in-flight candidate no longer gets its risk comment or verifier escalation in that run. Both depend only on the head and are written by the first run that judges it with nothing in flight. Before this change the candidates past about 44 were never judged at all. The merge-queue sweep in unattended_writes (identity queue, entry_hold, verifier hold for other entries, dequeue) still runs in every gate process.
  • Not moved earlier: the queue check stays after the post-merge and floor reads. A post-merge refusal (post-merge:..., including floor-unreadable) outranks the in-flight refusal. It is a stop refusal with enabled=False that dequeues every identity entry, so checking the queue first would change outcomes.

Verification

  • New tests:
    • Assess.test_an_in_flight_refusal_off_the_queue_reads_nothing_after_it patches eligibility, lane and Dependabot eligibility, and promotion to raise.
    • Assess.test_the_in_flight_refusal_matches_the_full_judgment checks that the reason and enabled match the full-read path, and that a queued candidate is still read in full with own_refusal set.
    • InFlight.test_the_queue_read_names_every_entry_or_none.
    • LaneMode.test_an_in_flight_refusal_skips_the_risk_and_verifier_reads runs end to end through main --merge. The risk evidence and changed files raise, the verifier is read only for the swept entry chore: adopt widened comment-hygiene policy and re-pin ci-workflows @9567b8b #9, no risk comment is written, and the sweep still judges chore: adopt widened comment-hygiene policy and re-pin ci-workflows @9567b8b #9.
  • With the early return disabled, the three behaviour tests fail on their assertions (risk evidence read, read after the refusal, and the list mismatch). On the unchanged source all four fail.
  • Babysit suite: python3 -m unittest discover -s tests -t . ran 1794 tests, OK.
  • scripts/run-ruff.sh check and format --check are clean. scripts/check-affected.sh: 7 passed, 1 skipped.

Related

🤖 Generated with Claude Code

…erge is in flight

pr-merge merges one lane pull request at a time, so while one sits in the
merge queue every other candidate is refused `in-flight:...`. `assess` still
read eligibility and promotion, and `score_unattended` still read the risk
evidence, the changed files and the verifier verdict, for a refusal already
decided.

`in_flight` now also returns every queued pull request number (None when the
queue was not read whole). Past an on rung, `assess` returns the in-flight
refusal at once when the candidate has no queue entry of its own and is not a
Dependabot pull request; the reason, `enabled` and the rung, cap and
in-flight details are unchanged. A queued candidate is still judged in full,
so its own refusal still dequeues it; a Dependabot one is too, because its
class decides the gate's dependency-manager hold. The risk comment and
verifier escalation of an off-queue in-flight candidate wait for a run with
nothing in flight. The merge-queue sweep in `unattended_writes` still runs.

The in-flight check stays after the post-merge and floor reads: a post-merge
refusal outranks it and is a stop refusal that dequeues every identity entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T10:11:43.772000Z 319016a PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 5bd968722f1d46bb31b912b2501f22c2f245c387: 0 finding(s), 0 as inline comments.

No findings in the incremental change since 319016a (the safety.md paragraph and two new tests).

I could not run the tests. The sandbox required approval for python3 -m unittest and I did not get it. My read of the tests and the source is by hand, and the author's verification claims in the PR body (1794 tests OK, ruff clean) are unverified by me.

  • safety.md: the new text matches the early return at plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py:2938-2952.
    • The return applies only when the rung is not off, the in-flight check refuses, and the candidate is not a Dependabot pull request.
    • It also requires a whole queue read in which the candidate has no entry of its own.
    • promoted is True in the pr-merge lane and None otherwise.
    • The reason is in-flight:<reason> with enabled=True.
    • The added sentence about the risk comment and verifier escalation is consistent with the author's description of unattended_writes.
  • test_a_rung_off_outranks_an_in_flight_refusal_off_the_queue: this pins the rung.rung != "off" guard.
    • With the rung off, the early return is skipped, so eligibility is read (read.called) and own_refusal is ineligible:fork.
    • The final reason falls to rung-off: ahead of in-flight:, matching the refusals tuple at lines 3016-3018.
    • Without the guard the reason would be in-flight:x, so the test can fail.
  • test_an_in_flight_refusal_still_reads_the_dependabot_class: this pins the not dependabot_pr guard.
    • The assertion out["unattended"]["dependabot"]["mode"] == "enforce" holds only if the Dependabot branch (lines 2953-2964) ran. The early return never sets details["dependabot"], so removing the guard would break it.
    • The candidate is a Dependabot pull request via the class setUp, and the in-flight entry is chore: adopt widened comment-hygiene policy and re-pin ci-workflows @9567b8b #9, not the candidate.

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 5bd968722f1d46bb31b912b2501f22c2f245c387: 0 finding(s), 0 as inline comments.

Security review: incremental, 319016a → 5bd9687

No security findings.

The change touches three files:

  • plugins/source-control/skills/babysit-prs/reference/safety.md is documentation. It states that a pull request refused in flight, with no merge-queue entry of its own and not from Dependabot, stops its reads at the refusal. The outcome is still a refusal (in-flight:<reason>). Skipping the promotion, eligibility and step 4 reads on that path can't produce a merge. It also leaves out the babysit-merge-risk comment and the verifier escalation for that run, but no queue entry waits on them, and the doc says the next full read writes both. This narrows no standing instruction, because the control still refuses.
  • test_babysit_merge_policy.py adds a test that a rung-off result outranks an off-queue in-flight refusal, which pins the safer ordering.
  • test_babysit_merge_unattended.py adds a test that the Dependabot class is still read on an in-flight refusal, which pins the dependency-manager guard.

The diff has no production code, workflow, token or secret handling, or injection surface. The two new tests only strengthen guards on the refuse path.

I didn't consult docs/conventions/instruction-exception-register/README.md. The doc change removes no rule and adds only a refusal-preserving short-circuit, so there was nothing to check against it.

…n-flight early return

A rung-off path with an off-queue in-flight refusal still refuses
`rung-off:` and reads eligibility, and a Dependabot pull request refused in
flight still reads its class, so the gate keeps no dependency-manager hold.
Each test fails with its guard removed from the early return.

safety.md now states where the reads stop on an in-flight refusal, that
`unattended.promoted` is then null (true in the pr-merge lane), and that the
risk comment and verifier escalation wait for the first run that reads the
pull request in full with nothing in flight.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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