From 3574cca809e00e9b083d9aa694f683e89fd1c856 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:06:15 -0400 Subject: [PATCH 1/3] perf(pr-merge): judge the gate's candidate on the run's listing at any age The gate's own candidate is judged provisional on the run's saved artifact listing at any age: an allow is judged again on a listing of its own before the gate allows (refresh), and a FAIL, the FAIL cap or an old pull request stands, since no later upload clears them. Any other refusal is judged again on a listing under two minutes old, so none dequeues an entry on an older listing. Queued entries keep the two-minute bound. A candidate whose verdict allows and that the risk score then refuses lists nothing. A fixture of a pr-merge tick prints each job's babysit-gh-calls line and pins its listing count. Co-Authored-By: Claude Opus 5.5 --- .../perf-shared-artifact-listing-8d41c3e6.md | 7 + .../babysit-prs/scripts/babysit_merge.py | 4 +- .../scripts/babysit_verify_evidence.py | 73 +++- .../tests/test_babysit_verify_evidence.py | 323 +++++++++++++++++- 4 files changed, 387 insertions(+), 20 deletions(-) create mode 100644 .changes/source-control/perf-shared-artifact-listing-8d41c3e6.md diff --git a/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md b/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md new file mode 100644 index 0000000000..a95327a589 --- /dev/null +++ b/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Changed + +- The pr-merge gate judges its own candidate's verifier verdict `provisional` (`babysit_verify_evidence.py` `Reader.evidence(..., provisional=True)`) on the run's saved artifact listing at any age instead of listing again once that listing is two minutes old. An allow there still lists again before the gate allows the candidate, as before; a refusal no later upload clears (`STANDING`: a FAIL at the head, the FAIL cap, an old pull request) stands on it; any other refusal is judged again on a listing under two minutes old, so it never dequeues an entry on an older one. Queued entries are judged as before. A candidate whose verdict allows and that the gate then refuses for anything else, such as its risk score, no longer lists at all. In a fixture shaped like 2026-10-27's repository the merge job's listings fell from 4 to 3 (17 candidates in about 6 minutes) and from 4 to 2 (6 candidates two minutes apart) with an empty merge queue, and stay the same with a queued entry, whose keep needs a recent listing. diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index ae312d724d..278a0a5487 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -3309,7 +3309,9 @@ def score_unattended( max_fails=config.max_verify_fails, agent_identity=config.agent_identity, ) - verdict = reader.evidence(number, str(details["headSha"]), pr) + # Provisional: an allow is judged again on a listing of this gate + # run's own below, once nothing else refuses. + verdict = reader.evidence(number, str(details["headSha"]), pr, provisional=True) details["verify"] = verdict.report() inputs["verify_reader"] = reader if verdict.refusal: diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index 5bada5fe95..a7d8623c40 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py @@ -129,12 +129,18 @@ job holds the lanes App token. pr-merge.yml builds one `Reader` per candidate gate run and names a file in `LISTING_ENV`: a successful listing is saved there, and later candidates of -the run read it instead of paging again. No judgment uses a listing, saved -or in memory, begun more than `LISTING_MAX_AGE` before it: an older one is -read again and saved. A listing taken earlier in the run -can only lack later uploads, so a refusal on it stands; before the gate's own -candidate is allowed on it, babysit_merge.py calls `refresh()` and judges it -again on a listing of its own. A listing that got a 5xx or a fast network +the run read it instead of paging again. A listing taken earlier in the run +can only lack later uploads. No judgment uses a listing, saved or in +memory, begun more than `LISTING_MAX_AGE` before it (an older one is read +again and saved), except the gate's own candidate's, which babysit_merge.py +judges `provisional` on the listing at hand at any age: an allow stands on +it until the gate would allow the candidate, when babysit_merge.py calls +`refresh()` and judges it again on a listing of its own, and so does a +refusal no later upload clears (`STANDING`: a FAIL at the head, the FAIL +cap, an old pull request). Any other refusal is judged again on a recent +listing, so none dequeues the candidate's queued entry on an older one. A +candidate the gate refuses for anything else lists nothing more; queued +entries are judged on a recent listing. A listing that got a 5xx or a fast network error is read again after each of `LISTING_WAITS`, every read inside `LISTING_TIMEOUT_SECONDS` in all; any other failure, a timeout, a 429 or a rate-limit 403 included, is not. A failed listing is never saved, and is kept as well, so one @@ -263,8 +269,9 @@ def verdict_name(name: str) -> re.Match[str] | None: # reads about 300 pages at 1 s a page, inside every reader's step timeout # (judgment). LISTING_TIMEOUT_SECONDS = 300.0 -# The oldest listing a judgment uses, saved or in memory (see the bound), -# unless `evidence --shared-listing` lifts it (`Reader.listing_max_age`). +# The oldest listing a judgment uses, saved or in memory, but for a +# provisional one's allow or `STANDING` refusal (`Reader.evidence`), unless +# `evidence --shared-listing` lifts it (`Reader.listing_max_age`). LISTING_MAX_AGE = run_cache.MAX_AGE # The incremental listing (`Reader._incremental`). The listing comes newest id # first, but a lower id can be created after a higher one: across 6,835 @@ -321,6 +328,12 @@ def verdict_name(name: str) -> re.Match[str] | None: # gate and enable-auto-merge.mjs (`needsHuman`) read; clear-escalations.mjs # LANE_CAUSES.verdict shares nothing with it (lane-causes-person-codes.test.mjs). NEEDS_HUMAN = (UNTRUSTED_SPEC,) +# The refusals no upload after the listing clears (`Reader.evidence`): a +# counted FAIL at the head or the cap's FAILs stay counted whatever is +# uploaded later, as no pass clears them, and a pull request's age only +# grows. A pass uploaded later can clear any other refusal, and a read may +# answer otherwise. +STANDING = (FAIL, REPEATED_FAIL, TOO_OLD) ISSUE_FIELDS = ( "number lastEditedAt repository{nameWithOwner} " "labels(first:100){totalCount nodes{name}} " @@ -813,6 +826,8 @@ class Reader: # The oldest listing a judgment uses; None (`evidence --shared-listing`) # takes the run's saved listing at any age. listing_max_age: timedelta | None = LISTING_MAX_AGE + # `evidence` is judging on the listing at hand at any age. + _any_age: bool = False _artifacts: list[dict[str, Any]] | None = None _fetched_at: datetime | None = None # when `_artifacts` began to be read _saved: bool = False # `_artifacts` came from `listing_path` @@ -870,15 +885,10 @@ def _current_listing(self) -> list[dict[str, Any]]: without paging again.""" if self._listing_error is not None: raise _Unreadable(self._listing_error) - if self._artifacts is not None and _recent( - self._fetched_at, self.listing_max_age - ): + max_age = None if self._any_age else self.listing_max_age + if self._artifacts is not None and _recent(self._fetched_at, max_age): return self._artifacts - saved = ( - None - if self._own - else self._load_saved(self.listing_path, self.listing_max_age) - ) + saved = None if self._own else self._load_saved(self.listing_path, max_age) if saved is not None: self._fetched_at, self._artifacts = saved self._base = saved @@ -1679,12 +1689,41 @@ def _commits(self, number: int, head: str) -> set[str]: return {str(c.get("sha")) for c in commits} def evidence( - self, number: int, head: str, pr: dict[str, Any] | None = None + self, + number: int, + head: str, + pr: dict[str, Any] | None = None, + *, + provisional: bool = False, + ) -> Evidence: + """The judgment at `head`, on a listing begun at most + `listing_max_age` before it. A `provisional` one, whose allow the + caller judges again on a listing of its own before acting on it + (babysit_merge.py `refresh()`), takes the listing at hand at any age: + an allow or a refusal no later upload clears (`STANDING`) stands on + it, and any other refusal on an older listing is judged again on a + recent one.""" + judged = self._judge(number, head, pr, any_age=provisional) + if ( + not provisional + or judged.refusal is None + or judged.refusal in STANDING + or self._fetched_at is None + or _recent(self._fetched_at, self.listing_max_age) + ): + return judged + return self._judge(number, head, pr, any_age=False) + + def _judge( + self, number: int, head: str, pr: dict[str, Any] | None, *, any_age: bool ) -> Evidence: + self._any_age = any_age try: return self._evidence(number, head, pr) except (_Unreadable, postmerge.PostMergeReadError) as exc: return Evidence(UNREADABLE, head, (), 0, str(exc)) + finally: + self._any_age = False def _evidence(self, number: int, head: str, pr: dict[str, Any] | None) -> Evidence: if pr is None: diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index 3e0a9754d8..734bcbf08e 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -46,6 +46,7 @@ import babysit_merge as merge import babysit_merge_policy as policy import babysit_postmerge as postmerge +import babysit_run_cache as run_cache import babysit_verify_evidence as ve import test_babysit_merge_unattended as unattended @@ -2258,6 +2259,53 @@ def test_a_saved_listing_past_the_bound_is_not_used(self) -> None: self.assertEqual(evidence.refusal, "verify:fail") self.assertEqual(listings(fake), 1) + def provisional(self, fake: FakeGitHub) -> tuple[ve.Reader, ve.Evidence]: + """The gate's own candidate (babysit_merge.py): judged provisional.""" + with mock.patch.object(postmerge, "gh_capture", side_effect=fake): + reader = ve.Reader(REPO, "main", now=NOW) + return reader, reader.evidence(PR, HEAD, pull(), provisional=True) + + def saved_then( + self, before: FakeGitHub, later: FakeGitHub + ) -> tuple[ve.Reader, ve.Evidence]: + """`before` saves the run's listing; ten minutes on, past the bound, + a provisional judgment while GitHub holds `later`.""" + clock = [datetime(2026, 10, 16, 12, 0, tzinfo=UTC)] + with mock.patch.object(ve, "_wall_clock", side_effect=lambda: clock[0]): + self.gate(before) + clock[0] += timedelta(minutes=10) + return self.provisional(later) + + def test_a_fail_on_an_older_saved_listing_refuses_a_provisional_judgment( + self, + ) -> None: + # An upload since cannot clear a FAIL at the head: no listing. + failed = FakeGitHub([artifact(named(HEAD, "fail"), 1)], {1: run(1)}) + fake = FakeGitHub([artifact(named(HEAD, "fail"), 1)], {1: run(1)}) + fake.artifacts.append(artifact(named(HEAD, "pass"), 2)) + fake.runs[2] = run(2) + _, evidence = self.saved_then(failed, fake) + self.assertEqual(evidence.refusal, "verify:fail") + self.assertEqual(listings(fake), 0) + + def test_a_provisional_allow_stands_on_an_older_saved_listing(self) -> None: + # babysit_merge.py lists again before the gate allows (`saved`). + fake = passing() + reader, evidence = self.saved_then(passing(), fake) + self.assertIsNone(evidence.refusal) + self.assertTrue(reader.saved) + self.assertEqual(listings(fake), 0) + + def test_a_clearable_refusal_on_an_older_saved_listing_is_judged_again( + self, + ) -> None: + # The saved listing has no verdict; a pass uploaded since allows, so + # a missing verdict there would dequeue the candidate's entry wrongly. + reader, evidence = self.saved_then(FakeGitHub([], {}), fake := passing()) + self.assertIsNone(evidence.refusal) + self.assertFalse(reader.saved) + self.assertEqual(listings(fake), 1) + def test_a_listing_in_memory_past_the_bound_is_read_again(self) -> None: clock = [datetime(2026, 10, 16, 12, 0, tzinfo=UTC)] fake = passing() @@ -3340,6 +3388,8 @@ def setUp(self) -> None: self.promotion = policy.Promotion(None, "resolver-failed") self.kill = [] self.verdicts: dict[int, ve.Evidence] = {} + # Each judgment's pull request and whether it was provisional. + self.provisional: list[tuple[int, bool]] = [] self.read = mock.Mock( side_effect=lambda number, head: self.verdicts.get(number, evidence(None)) ) @@ -3355,7 +3405,10 @@ def setUp(self) -> None: # Writes whose argv holds one of these fail (HTTP 502). self.failing: set[str] = set() - def _evidence(self, number: int, head: str, pr: Any = None) -> ve.Evidence: + def _evidence( + self, number: int, head: str, pr: Any = None, *, provisional: bool = False + ) -> ve.Evidence: + self.provisional.append((number, provisional)) return self.read(number, head) def _capture(self, args: list[str]) -> Any: @@ -3572,6 +3625,14 @@ def test_a_candidate_refused_otherwise_does_not_list_again(self) -> None: refresh.assert_not_called() self.read.assert_called_once_with(unattended.PR, unattended.HEAD) + def test_only_the_candidate_is_judged_provisional(self) -> None: + # Its allow is judged again on its own listing before the gate allows + # it; a queued entry kept on the judgment has no such second read. + other = unattended.PR + 1 + self.queued = [other] + self.lane() + self.assertEqual(self.provisional, [(unattended.PR, True), (other, False)]) + def test_a_refusal_on_the_saved_listing_stands_without_a_listing(self) -> None: refresh = self.saved_listing() self.verdicts[unattended.PR] = evidence("verify:missing") @@ -3874,7 +3935,7 @@ def setUp(self) -> None: mock.patch.object( ve.Reader, "evidence", - side_effect=lambda n, head, pr=None: ( + side_effect=lambda n, head, pr=None, **_: ( self.reads.append(n) or ve.run_cache.note(listed) or evidence(None) ), ), @@ -4600,6 +4661,264 @@ def test_the_next_sweep_downloads_only_the_new_fails(self) -> None: self.assertEqual(after[ZIP_KIND], 6) +class APrMergeTickListsOnlyBeforeAnAllowOrAKeep(unittest.TestCase): + """Artifact listings and requests per pr-merge tick, each job's printed as + a `babysit-gh-calls` line, in a repository shaped like 2026-10-27's: 6,240 + artifacts uploaded four a minute, and the listing the last tick saved 15 + minutes before as the carried base. The merge job's skip filter lists + first; then 17 gate candidates 20 seconds apart while uploads go on: + eight refused by a FAIL at their head, seven whose verdict allows and + whose risk score then refuses, two allowed; with one queued entry that + each gate judges again once its keep is over `run_cache.MAX_AGE` old, or + none. enable-auto-merge judges five pull requests on its own saved + listing (`evidence --shared-listing`).""" + + START = LISTED_AT + UPLOAD_EVERY = timedelta(seconds=15) + FILLER = 6240 + # Each candidate in turn: refused by a FAIL ("fail"), its verdict allows + # and the risk score refuses ("risk"), or allowed ("allow"); the last two + # judge PR 41 at its head. + KINDS = "fail risk fail risk fail risk fail risk allow fail risk fail risk fail risk allow fail".split() + GATE_EVERY = timedelta(seconds=20) + + def setUp(self) -> None: + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.dir = pathlib.Path(tmp.name) + self.clock = [self.START] + self.next_id = 100_000 + fake = FakeGitHub([], {1: run(1)}, {named(HEAD, "pass"): record()}) + first = self.START - self.UPLOAD_EVERY * self.FILLER + for k in range(self.FILLER): + self.upload(fake, "pr-merge-run-cache", 3, first + self.UPLOAD_EVERY * k) + # The verdicts: a pass at PR 41's head, a FAIL at each other + # candidate's head, all uploaded within the last hour. + self.upload(fake, named(HEAD, "pass"), 1, self.START - timedelta(minutes=50)) + for n in self.refused(): + sha, run_id = self.head(n), 1000 + n + fail = f"{named(sha, 'fail', n)}-{ve.head_ref_digest(f'b{n}')}" + fake.runs[run_id] = run(run_id) + fake.records[fail] = record(n, sha, verdict="fail", head_ref=f"b{n}") + self.upload(fake, fail, run_id, self.START - timedelta(minutes=40)) + self.fake = fake + # The last tick's listing, read 15 minutes before this one. + carried_at = self.START - timedelta(minutes=15) + self.carry = self.dir / "carry.json" + self.carry.write_text( + json.dumps( + { + "fetchedAt": carried_at.isoformat(), + "artifacts": [ + a + for a in fake.artifacts + if ve._when(a["created_at"]) <= carried_at + ], + } + ) + ) + for patcher in ( + mock.patch.object(ve, "_wall_clock", side_effect=lambda: self.clock[0]), + mock.patch.object(run_cache, "now", side_effect=lambda: self.clock[0]), + mock.patch.object(policy, "pull_request", side_effect=self.pull), + ): + patcher.start() + self.addCleanup(patcher.stop) + + def refused(self) -> list[int]: + return [42 + i for i in range(self.KINDS.count("fail"))] + + @staticmethod + def head(number: int) -> str: + return HEAD if number == PR else format(number, "040x") + + def pull(self, repo: str, number: int) -> dict[str, Any]: + return pull( + number=number, + head={ + "ref": "feat/x" if number == PR else f"b{number}", + "sha": self.head(number), + }, + ) + + def upload(self, fake: FakeGitHub, name: str, run_id: int, at: datetime) -> None: + fake.artifacts.append( + dated(name, run_id, self.next_id, at, expires=at + timedelta(days=7)) + ) + self.next_id += 1 + + def tick(self, until: datetime) -> None: + """Move the clock, uploading four artifacts a minute on the way.""" + while self.clock[0] + self.UPLOAD_EVERY <= until: + self.clock[0] += self.UPLOAD_EVERY + self.upload(self.fake, "pr-review-cli-handoff", 3, self.clock[0]) + self.clock[0] = until + + def answer(self, cmd: list[str], **kwargs: Any) -> Any: + """gh as GitHub answers it: `-i` adds the status, `Date` and `Link`; + listing pages are 100 artifacts, newest id first.""" + headers = "-i" in cmd + args = [a for a in cmd[1:] if a != "-i"] + path = args[1] if args[:1] == ["api"] else "" + date = self.clock[0].strftime("%a, %d %b %Y %H:%M:%S GMT") + link = "" + listing = f"repos/{REPO}/actions/artifacts?per_page=100" + if path.startswith(listing): + page = int(path.partition("&page=")[2] or 1) + self.listings += page == 1 + ordered = sorted(self.fake.artifacts, key=lambda a: -a["id"]) + body = { + "total_count": len(ordered), + "artifacts": ordered[(page - 1) * 100 : page * 100], + } + if page * 100 < len(ordered): + link = f'Link: ; rel="next"\r\n' + proc = subprocess.CompletedProcess(cmd, 0, json.dumps(body), "") + elif re.fullmatch(rf"repos/{REPO}/pulls/[0-9]+/commits\?per_page=100", path): + number = int(path.split("/")[4]) + proc = subprocess.CompletedProcess( + cmd, 0, json.dumps([{"sha": self.head(number)}]), "" + ) + else: + proc = self.fake( + args, + env_overrides=kwargs.get("env_overrides"), + binary=kwargs.get("binary"), + ) + if not headers or proc.returncode != 0: + return proc + return subprocess.CompletedProcess( + cmd, 0, f"HTTP/2.0 200 OK\r\nDate: {date}\r\n{link}\r\n{proc.stdout}", "" + ) + + def environ(self, job: str) -> Any: + return mock.patch.dict( + "os.environ", + { + "BABYSIT_RUN_CACHE": str(self.dir / f"{job}-run.json"), + ve.LISTING_ENV: str(self.dir / f"{job}-listing.json"), + ve.LISTING_CARRY_ENV: str(self.carry), + ve.TOKEN_ENV: "job-token", + }, + ) + + def merge_job( + self, + queued: bool, + kinds: list[str] | None = None, + every: timedelta | None = None, + ) -> tuple[dict[str, int], list[Any]]: + kinds, every = kinds or self.KINDS, every or self.GATE_EVERY + refusals: list[Any] = [] + gh.reset_gh_call_counts() + self.listings = 0 + kept_at: datetime | None = None + with ( + self.environ("merge"), + mock.patch.object(gh, "run_command", side_effect=self.answer), + ): + # The skip filter (babysit_merge_skip.py filter) lists first, on + # the carried listing, and saves it for the gate. + skip = ve.Reader(REPO, "main") + skip.listing() + self.assertEqual(skip.listing_mode, "incremental") + self.skip_calls = gh.gh_call_counts() + refused = iter(self.refused()) + for i, kind in enumerate(kinds): + self.tick(self.START + every * (i + 1)) + number = next(refused) if kind == "fail" else PR + head, pr = self.head(number), self.pull(REPO, number) + # babysit_merge.py `_evaluate`: a fresh Reader per candidate, + # and its own listing before an allow. + reader = ve.Reader(REPO, "main", now=NOW) + evidence = reader.evidence(number, head, pr, provisional=True) + if evidence.refusal is None and kind == "allow" and reader.saved: + reader.refresh() + evidence = reader.evidence(number, head, pr) + refusals.append(evidence.refusal) + # `unattended_writes`: a queued entry judged again once its + # keep is past `run_cache.MAX_AGE` (`_kept_recently`). + if queued and ( + kept_at is None or self.clock[0] - kept_at > run_cache.MAX_AGE + ): + self.assertIsNone(reader.queued_hold(PR)) + kept_at = self.clock[0] + return gh.gh_call_counts(), refusals + + def arm_job(self) -> tuple[dict[str, int], list[Any]]: + refusals: list[Any] = [] + gh.reset_gh_call_counts() + self.listings = 0 + self.clock[0] = self.START + with ( + self.environ("arm"), + mock.patch.object(gh, "run_command", side_effect=self.answer), + ): + for number in [PR, *self.refused()[:4]]: + # enable-auto-merge.mjs: one `evidence --shared-listing` + # process per pull request. + reader = ve.Reader(REPO, "main", now=NOW, listing_max_age=None) + refusals.append( + reader.evidence( + number, self.head(number), self.pull(REPO, number) + ).refusal + ) + self.tick(self.clock[0] + timedelta(seconds=30)) + return gh.gh_call_counts(), refusals + + def report(self, label: str, counts: dict[str, int]) -> None: + payload = { + "fixture": label, + "skip-filter": getattr(self, "skip_calls", None), + "listings": self.listings, + "total": sum(counts.values()), + "calls": dict(sorted(counts.items())), + } + print( + f"babysit-gh-calls {json.dumps(payload, sort_keys=True)}", file=sys.stderr + ) + + def test_the_merge_job_with_an_empty_queue(self) -> None: + counts, refusals = self.merge_job(queued=False) + self.report("merge-job queue=0", counts) + self.assertEqual( + refusals.count(None), len(self.KINDS) - self.KINDS.count("fail") + ) + # The skip filter's listing and each allowed candidate's own: no + # refused candidate lists, whatever the saved listing's age. + self.assertEqual(self.listings, 1 + self.KINDS.count("allow")) + + def test_the_merge_job_with_a_queued_entry(self) -> None: + counts, refusals = self.merge_job(queued=True) + self.report("merge-job queue=1", counts) + self.assertEqual( + refusals.count(None), len(self.KINDS) - self.KINDS.count("fail") + ) + + def slow_merge_job(self, queued: bool) -> int: + # As run 40594967390's merge job: a candidate about every two minutes. + kinds = "risk fail risk fail risk allow".split() + counts, refusals = self.merge_job(queued, kinds, timedelta(minutes=2)) + self.report(f"merge-job slow queue={int(queued)}", counts) + self.assertEqual(refusals.count(None), 4) + return self.listings + + def test_a_slow_merge_job_with_an_empty_queue(self) -> None: + # The skip filter's listing, and the allowed candidate's own. + self.assertEqual(self.slow_merge_job(False), 2) + + def test_a_slow_merge_job_with_a_queued_entry(self) -> None: + # The queued keeps at 6 and 10 minutes find the listing past the + # bound and list again; the allow at 12 lists on its own. + self.assertEqual(self.slow_merge_job(True), 4) + + def test_enable_auto_merge(self) -> None: + counts, refusals = self.arm_job() + self.report("enable-auto-merge", counts) + self.assertEqual(refusals, [None, *["verify:fail"] * 4]) + self.assertEqual(self.listings, 1) + + class EvidenceCommandTest(unittest.TestCase): """`evidence ` prints the gate's judgment for enable-auto-merge.mjs, which arms only on a null refusal.""" From b18c26f58a970a320c546cb2c4d1453237765db9 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:30:47 -0400 Subject: [PATCH 2/3] perf(pr-merge): read the incremental listing again before paging it whole The first tick on a carried base read 4 incremental pages that missed the live count and then paged all 71 in the skip filter; the same base read again minutes later held the count. One more read on the same base costs about 4 pages against 71, and each read is still checked against the live total_count. Co-Authored-By: Claude Opus 5.5 --- .../perf-shared-artifact-listing-8d41c3e6.md | 4 +++ .../scripts/babysit_verify_evidence.py | 22 +++++++++++++--- .../tests/test_babysit_verify_evidence.py | 26 +++++++++++++++++-- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md b/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md index a95327a589..efccabf07b 100644 --- a/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md +++ b/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md @@ -5,3 +5,7 @@ bump: patch ### Changed - The pr-merge gate judges its own candidate's verifier verdict `provisional` (`babysit_verify_evidence.py` `Reader.evidence(..., provisional=True)`) on the run's saved artifact listing at any age instead of listing again once that listing is two minutes old. An allow there still lists again before the gate allows the candidate, as before; a refusal no later upload clears (`STANDING`: a FAIL at the head, the FAIL cap, an old pull request) stands on it; any other refusal is judged again on a listing under two minutes old, so it never dequeues an entry on an older one. Queued entries are judged as before. A candidate whose verdict allows and that the gate then refuses for anything else, such as its risk score, no longer lists at all. In a fixture shaped like 2026-10-27's repository the merge job's listings fell from 4 to 3 (17 candidates in about 6 minutes) and from 4 to 2 (6 candidates two minutes apart) with an empty merge queue, and stay the same with a queued entry, whose keep needs a recent listing. + +### Fixed + +- `babysit_verify_evidence.py` reads the incremental listing a second time on the same base before it pages the whole listing (`INCREMENTAL_READS`). The first pr-merge tick on a carried base (run 40600822658, 2026-10-27) read 4 pages that missed the live count by a moving page boundary or similar and then paged all 71 in the skip filter; the same base read again five minutes later held the live count in 4 pages. diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index a7d8623c40..d1b53aee84 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py @@ -121,9 +121,10 @@ incremental listings that day each held exactly that count and every artifact a full listing taken just after held, plus 0, 1 and 3 that the full listing skipped at its page boundaries though they still existed. An artifact that -expires during the read can only make the counts differ. Any other count, a -base that is missing, older or of another shape, or any failed read lists -every page, checked the same way. A carry forged to drop a FAIL must therefore +expires during the read can only make the counts differ. Any other count or +failed read is read again on the same base, up to `INCREMENTAL_READS` reads; +after those, or with a base that is missing, older or of another shape, it +lists every page, checked the same way. A carry forged to drop a FAIL must therefore also hold a fabricated entry in its place to match the live count, and forging it already needs code running in a scheduled or hand-started run of main's pr-merge.yml, whose merge job holds the lanes App token. @@ -289,6 +290,14 @@ def verdict_name(name: str) -> re.Match[str] | None: # for the whole listing that day (judgment on that data). LISTING_CARRY_MAX_AGE = timedelta(hours=3) LISTING_PAGE_SIZE = 100 +# The incremental reads on one base before a whole listing. The first +# pr-merge tick on a carried base (run 40600822658, 2026-10-27 23:19Z) read +# 4 pages that did not hold the live count and then paged all 71; the same +# base read again at 23:24Z held it in 4 pages. A page boundary that moves +# while the pages are read, as a whole listing's does (#10486), would do +# that. A second read costs about 4 pages against 71 (judgment on that +# data). +INCREMENTAL_READS = 2 # The whole listings `Reader._complete_listing` reads before it refuses. Of # three whole listings read on 2026-10-27, two skipped live artifacts at page # boundaries (1 and 3, a FAIL in each), and each skipped artifact was in every @@ -895,7 +904,12 @@ def _current_listing(self) -> list[dict[str, Any]]: self._saved = True return self._artifacts fetched_at = _wall_clock() - listing = self._incremental() + base = self._incremental_base() + listing = None + for _ in range(INCREMENTAL_READS if base is not None else 0): + listing = self._incremental(base) + if listing is not None: + break self.listing_mode = "incremental" if listing is None: self.listing_mode = "full" diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index 734bcbf08e..f147027684 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -3080,8 +3080,30 @@ def test_a_count_other_than_githubs_lists_every_page(self) -> None: reader, evidence = self.judge(fake) self.assertEqual(evidence.refusal, "verify:earlier-fail") self.assertEqual(reader.listing_mode, "full") - # The incremental first page, then the whole listing's own check. - self.assertEqual((fake.pages(), fake.full_listings()), (2, 1)) + # The incremental first page, read `INCREMENTAL_READS` times, then + # the whole listing's own check. + self.assertEqual( + (fake.pages(), fake.full_listings()), (ve.INCREMENTAL_READS + 1, 1) + ) + + def test_a_count_off_once_is_read_again_on_the_same_base(self) -> None: + # The first read's live count is one more than its pages hold, as + # when a page boundary moves during the read; the next read holds it + # and no page is listed whole. + self.write_carry(self.base) + fake = self.fake(dated(named(HEAD, "fail"), 2, 1100, LISTED_AT)) + rest, off = fake.rest, [len(self.base) + 2] + + def first_read_off(args: list[str], *rest_args: Any) -> Any: + first = any(a.endswith("&page=1") for a in args) + fake.live_total = off.pop() if first and off else None + return rest(args, *rest_args) + + fake.rest = first_read_off # type: ignore[method-assign] + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:fail") + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual((fake.pages(), fake.full_listings()), (4, 0)) def test_a_live_count_no_listing_can_hold_refuses(self) -> None: for delta in (-1, 1): From 534f8ab9b0e4ff84dae0cc3d4e0d63d30dde0bf2 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:26:56 -0400 Subject: [PATCH 3/3] fix(pr-merge): read the incremental listing again only on a count miss _incremental now answers COUNT_MISSED when its pages read whole but miss the live count, and None for a failed read. Only a count miss is read again on the same base; a timeout, a 429 or a rate-limit 403 pages the whole listing at once, as _read_listing's no-retry rule says. Co-Authored-By: Claude Opus 5.5 --- .../perf-shared-artifact-listing-8d41c3e6.md | 2 +- .../scripts/babysit_verify_evidence.py | 40 ++++++++++++------ .../tests/test_babysit_verify_evidence.py | 41 +++++++++++++++++++ 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md b/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md index efccabf07b..4c0fa3330e 100644 --- a/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md +++ b/.changes/source-control/perf-shared-artifact-listing-8d41c3e6.md @@ -8,4 +8,4 @@ bump: patch ### Fixed -- `babysit_verify_evidence.py` reads the incremental listing a second time on the same base before it pages the whole listing (`INCREMENTAL_READS`). The first pr-merge tick on a carried base (run 40600822658, 2026-10-27) read 4 pages that missed the live count by a moving page boundary or similar and then paged all 71 in the skip filter; the same base read again five minutes later held the live count in 4 pages. +- `babysit_verify_evidence.py` reads the incremental listing a second time on the same base before it pages the whole listing (`INCREMENTAL_READS`), only when its pages read whole but missed the live count; a failed read (a timeout, a 429 or a rate-limit 403 included) pages the whole listing at once, as before. The first pr-merge tick on a carried base (run 40600822658, 2026-10-27) read 4 pages that missed the live count (cause unverified) and then paged all 71 in the skip filter; the same base read again five minutes later held the live count in 4 pages. diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index d1b53aee84..f7cde25ee9 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py @@ -121,10 +121,12 @@ incremental listings that day each held exactly that count and every artifact a full listing taken just after held, plus 0, 1 and 3 that the full listing skipped at its page boundaries though they still existed. An artifact that -expires during the read can only make the counts differ. Any other count or -failed read is read again on the same base, up to `INCREMENTAL_READS` reads; -after those, or with a base that is missing, older or of another shape, it -lists every page, checked the same way. A carry forged to drop a FAIL must therefore +expires during the read can only make the counts differ. Pages that read +whole but hold any other count are read again on the same base, up to +`INCREMENTAL_READS` reads; after those, on any failed read (a timeout, a 429 +or a rate-limit 403 included, which are never read again), or with a base +that is missing, older or of another shape, it lists every page, checked the +same way. A carry forged to drop a FAIL must therefore also hold a fabricated entry in its place to match the live count, and forging it already needs code running in a scheduled or hand-started run of main's pr-merge.yml, whose merge job holds the lanes App token. @@ -298,6 +300,14 @@ def verdict_name(name: str) -> re.Match[str] | None: # that. A second read costs about 4 pages against 71 (judgment on that # data). INCREMENTAL_READS = 2 + + +class _CountMissed: + """`Reader._incremental`'s answer when its pages read whole but do not + hold the live count, the one case read again on the same base.""" + + +COUNT_MISSED = _CountMissed() # The whole listings `Reader._complete_listing` reads before it refuses. Of # three whole listings read on 2026-10-27, two skipped live artifacts at page # boundaries (1 and 3, a FAIL in each), and each skipped artifact was in every @@ -905,13 +915,15 @@ def _current_listing(self) -> list[dict[str, Any]]: return self._artifacts fetched_at = _wall_clock() base = self._incremental_base() - listing = None + listing: list[dict[str, Any]] | _CountMissed | None = COUNT_MISSED + # Only pages that read whole but missed the live count are read again; + # a failed read lists every page at once, as `_read_listing` decides. for _ in range(INCREMENTAL_READS if base is not None else 0): listing = self._incremental(base) - if listing is not None: + if listing is not COUNT_MISSED: break self.listing_mode = "incremental" - if listing is None: + if not isinstance(listing, list): self.listing_mode = "full" try: listing = self._complete_listing() @@ -1005,7 +1017,7 @@ def _complete_listing(self) -> list[dict[str, Any]]: else: raise _Unreadable("repository artifacts: an entry with no id") listing = self._incremental((read_at, list(union.values())), deadline) - if listing is not None: + if isinstance(listing, list): return listing raise _Unreadable( f"repository artifacts: {LISTING_READS} whole listings never held " @@ -1016,9 +1028,10 @@ def _incremental( self, base: tuple[datetime, list[dict[str, Any]]] | None = None, deadline: float | None = None, - ) -> list[dict[str, Any]] | None: + ) -> list[dict[str, Any]] | _CountMissed | None: """Every repository artifact, as the base (`_incremental_base`) and - the newest pages read now show them, or None for a full listing. + the newest pages read now show them; `COUNT_MISSED` when the pages + read but the result does not hold the live count, else None. The listing comes newest id first. Pages are read from the first until one holds an artifact at or below the base's highest id that @@ -1031,8 +1044,9 @@ def _incremental( expired or past its `expires_at` by the first page's `Date` (one with no `expires_at` stands unless expired). The result must hold exactly the first page's `total_count` artifacts, - GitHub's live count. Any other count, no base, an entry or page of - another shape, a missing `Date`, or any failed read gives None. + GitHub's live count: any other count gives `COUNT_MISSED`. No base, + an entry or page of another shape, a missing `Date`, or any failed + read gives None. `base` replaces `_incremental_base` and `deadline` its own `LISTING_TIMEOUT_SECONDS` (`_complete_listing`'s top-up).""" if base is not None and not all(_listed_entry(a) for a in base[1]): @@ -1105,7 +1119,7 @@ def _incremental( and (a["expires_at"] is None or _when(a["expires_at"]) > server_now) ] listing = sorted([*fresh.values(), *kept], key=lambda a: -a["id"]) - return listing if len(listing) == total else None + return listing if len(listing) == total else COUNT_MISSED def _save(self, listing: list[dict[str, Any]], fetched_at: datetime) -> None: """Best effort: a failed write leaves the next candidate to list.""" diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index f147027684..86dbaf0639 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -3086,6 +3086,47 @@ def test_a_count_other_than_githubs_lists_every_page(self) -> None: (fake.pages(), fake.full_listings()), (ve.INCREMENTAL_READS + 1, 1) ) + def test_a_failed_first_read_is_not_read_again_on_the_base(self) -> None: + # A 429, a rate-limit 403 or a timeout on the first incremental page + # is never read again (`_read_listing`'s rule): the whole listing + # follows at once, as before the count-miss retry. + failures: dict[str, Any] = { + "429": subprocess.CompletedProcess( + [], 1, "", "gh: API rate limit exceeded (HTTP 429)" + ), + "rate-limit 403": subprocess.CompletedProcess( + [], 1, "", "gh: API rate limit exceeded for installation (HTTP 403)" + ), + "timeout": timed_out(), + } + for label, failure in failures.items(): + with self.subTest(label): + self.saved.unlink(missing_ok=True) + self.write_carry(self.base) + fake = self.fake(dated(named(HEAD, "fail"), 2, 1100, LISTED_AT)) + first = [True] + + def page(args: list[str], **kwargs: Any) -> Any: + if first[0] and args[1].endswith("&page=1"): + first[0] = False + fake.calls.append(args) + if isinstance(failure, BaseException): + raise failure + return failure, {} + return fake(args, **kwargs), {"date": DATE} + + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve, "gh_api_with_headers", side_effect=page), + ): + reader = ve.Reader(REPO, "main", now=NOW) + evidence = reader.evidence(PR, HEAD, pull()) + self.assertEqual(evidence.refusal, "verify:fail") + self.assertEqual(reader.listing_mode, "full") + # The failed page, then the whole listing and its top-up, two + # pages down to 900; no second read on the carried base. + self.assertEqual((fake.pages(), fake.full_listings()), (3, 1)) + def test_a_count_off_once_is_read_again_on_the_same_base(self) -> None: # The first read's live count is one more than its pages hold, as # when a page boundary moves during the read; the next read holds it