[kyle-sexton]

No related issue: follow-up to #11921's review (exact-count FAIL hole)

Summary

The pr-verify verdict reader (babysit_verify_evidence.py) accepted an artifact listing whose entry count equals the first page's total_count, and re-read the entries near expiry only on an over-count. Two exact-count cases could hide a FAIL:

• (a) a whole listing skips a FAIL at a page boundary while another kept entry was deleted early (up to EARLY_DELETION_LEAD before its expires_at), so the count still matches;
• (b) a stale carry holds N entries GitHub dropped early plus N live ones it missed.

Fix

• Reader._incremental now runs _near_expiry on an exact count whenever a kept entry expires within EARLY_DELETION_LEAD, as well as on an over-count. Each such id is decided by the pages that hold it now, or by its own read, under the existing NEAR_EXPIRY_PAGE_CAP / NEAR_EXPIRY_ID_READ_CAP.
• _near_expiry reports why it left an entry undecided, or that it settled every one. An exact count it cannot settle (near-expiry-cap: a cap ran out; unbracketed: a group it cannot bracket; id-answer: an id answer it cannot read) is refused instead of accepted, and prints a babysit-listing-miss line with that cause. A 5xx that outlasts LISTING_WAITS, or a refusal caused only by the caps running out, is transient: no .failed note for the job, so a later Reader with fresh caps can list again. Every other unsettled refusal is noted as before.
• listing_timeout sizes the whole listings' deadline for LISTING_READS listings and their top-ups only, and exact-count top-ups now spend it on near-expiry reads. When that deadline runs out after this listing's near-expiry reads began (in _read_listing, a top-up's own page, a region page or an id read), the read is refused as transient, never allowed, writes no .failed note, and prints a babysit-listing-miss line with cause near-expiry-deadline. listing_timeout and cold_cost are unchanged.
• A short count reads no pages near expiry (it is never kept), so COUNT_MISSED retries cost what they did on main.
• An exact count those pages leave short goes from the incremental stage straight to a whole listing (COUNT_OVER, no INCREMENTAL_WAITS): the base still holds what they dropped, so a wait would re-read the same pages and spend the cap the top-up needs. A top-up's short count still goes to another whole listing.
• Tests: the class's helper that shadowed TestCase.fail is renamed (it made assertIn, assertIsNone and list assertEqual pass on failure in that class).

Per-run call delta: +0 when no kept entry is near expiry; otherwise at most the existing NEAR_EXPIRY_PAGE_CAP (24 pages) and NEAR_EXPIRY_ID_READ_CAP (34 id reads) per listing, incremental read and top-up together. On a saved 3011-artifact listing, the entries near expiry formed one group spanning about 5 to 7 pages at every hour sampled.

Verification

New and changed tests in test_babysit_verify_evidence.py, run against three versions of the reader:

Test	origin/main	80f20f4 (first commit)	head
(a) whole listing skips a FAIL beside an early deletion	fail (FAIL allowed)	pass	pass
(b) stale carry whose early deletion hides a missed FAIL	fail (FAIL allowed)	fail (waits and re-reads 3x)	pass
exact count with none near expiry reads nothing more	pass	pass	pass
exact count the caps leave unsettled refuses, unnoted, prints cause: near-expiry-cap; a fresh Reader allows	fail (allowed)	fail (notes .failed)	pass (fails under both mutants)
5xx on an exact-count id read refuses as transient	fail	pass	pass
dropped entry does not spend the top-up's page cap	fail (FAIL allowed)	fail (refuses, cap starved)	pass
short count reads no pages near expiry	fail (0 top-up reads)	fail (4 reads)	pass
pages near expiry stop at their cap (changed)	fail	pass	pass
top-up left short reads another whole listing (review follow-up)	—	—	pass (fails under its mutant)
near-expiry reads out of the listings' time refuse unnoted, cause: near-expiry-deadline (whole-listing and region-page subtests)	—	—	pass (each subtest fails under its mutant)
same, top-up page 1 out of time after an earlier top-up's near-expiry reads (review follow-up, 12 s subtest)	—	—	pass (fails on 4c82d34: .failed noted)

• Whole test_babysit_verify_evidence.py: 412 passed, 352 subtests before merging main; 420 passed, 355 subtests on the head 4c82d34 (main brought fix(source-control): read late pass names in the pr-merge gate's verdict read #11989's tests).
• Head 5fcddd3 (top-up page deadline fix): the babysit-prs test_babysit_verify*.py suites pass, 452 tests; pre-push check-affected passed.
• scripts/run-ruff.sh check on the changed files: all checks passed.
• scripts/affected-tests.sh suites run one at a time with scripts/run-test-file.sh: 139 suites at head 12251a6, all pass; after merging main (perf(source-control): read the verdict reader's config from the base checkout and reuse the cached pull request in merge-group ci-status #12016 touched the same files) the test file passes 420 and the pre-push check-affected passed (on the first run, 12 .mjs/runner-policy suites failed on missing yaml/ajv in a fresh worktree and passed after npm ci in their packages).

Related

• fix(source-control): re-read the pages near expiry when the verdict listing over-counts #11921, fix(source-control): stop the verdict reader's repeated cold artifact listings #11926: the review and fix this follows.
• fix(source-control): read late pass names in the pr-merge gate's verdict read #11989 touched the same file in different functions; it has since merged, and merging main into this branch was clean.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-01T18:11:28.967539Z	244e885	Draft marked ready

ℹ️ 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]

claude-security-review reviewed 5fcddd3557a0d620fc3d075638820f9425f44f2b: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12020, incremental (4c82d34c → 5fcddd35)

No security issues found.

What I read. Every hunk of the incremental diff in both files in scope: babysit_verify_evidence.py and its test file. babysit_verify_evidence.py is on the trusted-path list and is a merge-hold control, so I also read the code the change gates. That covers _whole_listings (L1522-L1578), _out_of_time, _near_expiry_out_of_time (L1843-L1877) and the _incremental contract.

Is the merge hold weakened? No. The change adds one more condition to transient at the end of _whole_listings (L1570-L1574): self._near_expiry_out_of_time(deadline, "top-up", reads).

• The refusal still happens. The new condition only applies after the loop has already failed to return a listing, and only when listing is None. It picks which exception gets raised, _Transient or _Unreadable, and both are refusals.
• What changes is the note. The only difference is whether the failure is written to the job's .failed note (_note_failed_listing). If it isn't, a later Reader lists again, and that Reader still has to reach a settled exact count before it accepts anything.
• It can't fire by accident. _near_expiry_out_of_time returns True only when three things hold: a deadline was set, that deadline has passed (deadline - _listing_clock() > 0 is false), and near-expiry page reads or id reads have already happened.
• No one can steer it. An outside party has no control over wall-clock time, and an allow can't come out of this path.

The new docstring wording and the extra whole field in the test's miss-line tuple change no behaviour.

Other surfaces. The cause and stage values on the stderr miss line are internal constants. The diff touches no token, secret, permission, trusted actor, kill switch, token-broker setting or plugin manifest. It adds no injection surface and removes no standing instruction.

Settled threads. The owner fixed the earlier thread on L1680-L1695 (the near-expiry reads running out of time), and that thread's lines are unchanged since it was resolved. This increment extends the same transient handling to a top-up page that runs out of time. It doesn't reopen that point, and I raise nothing on it.

Prior findings. The previous round at 4c82d34c had no findings, so nothing is restated. Past the cap: no.

PR context. It contains no instructions aimed at this review lane.

Spend. None added: this change only reclassifies a failure that already refuses, and makes no extra API reads.

critical-findings: 0

[github-actions]

claude-review: 1 inline finding(s) on db621263dab82c3bd21822968b87a9de38fb9118; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

claude-review reviewed 5fcddd3557a0d620fc3d075638820f9425f44f2b: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #12020, incremental (round 3, from 4c82d34 to 5fcddd3)

I read every hunk of both in-scope files in .claude-lane/incremental.diff. babysit_verify_evidence.py is on a trusted path, so I also read the code the change gates: _whole_listings, the start of _incremental (where a failed top-up page returns None), _out_of_time and _near_expiry_out_of_time.

Last round's finding (discussion_r4437104835): fixed. That finding said a top-up's own first pages could run out of time after an earlier top-up's near-expiry reads, and the failure was still noted for the job. The new head extends the classification at babysit_verify_evidence.py#L1570-L1574 with or self._near_expiry_out_of_time(deadline, "top-up", reads). This is the fix that finding suggested.

What I checked:

• Safety. The new check runs only when listing is None, so the result is still a refusal (_Transient), never an allow.
• Order of checks. Python evaluates the or left to right and stops early. So the miss line is printed only when neither _page_transient nor _near_expiry_spent already marked the failure transient. No duplicate line is printed for a refusal that was already transient.
• When it fires. _near_expiry_out_of_time needs three things: a deadline that is set, a deadline that has run out, and near-expiry page or id reads that have begun. A top-up page failing with time still left, or failing before any near-expiry read, is still noted, as before.
• Test. The new (12.0, "top-up", True) subtest checks for a miss line with stage top-up and a whole_listings key. Only _near_expiry_out_of_time writes that key (babysit_verify_evidence.py#L1869, the only match in the scripts directory), and the only place it is called with stage top-up is the new line. So this subtest exercises the new path. The whole flag also keeps the 10 s case tied to the region-page miss line, which has no whole_listings key.

Minor, not counted: listing is None also covers top-up failures that are not about time, such as a malformed page or an unbracketed near-expiry group. If one of those lands just after the deadline runs out, it is now classified as transient rather than noted. The only cost is extra retries by later Readers, never an allow.

Docstring: _near_expiry_out_of_time now says it covers "a whole listing or top-up page", which matches both places that call it.

Verification: I ran nothing; this lane has no shell. The test results in the PR description are the author's claims and I have not checked them.

Settled threads: the thread on the listings' time budget (owner reply, lines changed since resolution: no) stays settled and I did not raise it again. Neither the settled-threads file nor the PR context contains instructions aimed at this lane. There is no closing issue, so there were no acceptance criteria to check.

Spend: no change from the last round. The near-expiry reads still draw on the GitHub REST rate limit, not on billing.

[github-actions]

claude-review: 1 inline finding(s) on 4c82d34caf7b2ba35bb4aba6ab5631d18235fd40; the summary is in this lane's pull request comment.

claude-lane: claude-review