Third review round, cut short by a transport hang after ~11h, but it had already found
the thing that mattered: the guard added last round could never fire.
`read_existing_verdict` asks for `limit=100` and refused when the page came back with
100 rows. This instance caps `limit` at the server-wide `MAX_RESPONSE_ITEMS`, MEASURED
AT 50 — `/issues?limit=100` returns 50 items. A response can therefore never carry 100
rows, so the comparison was unreachable and the hole it was written for was still open.
The sting is that the repo already knew. `scripts/pr-changed-files.sh`, two test files
and `ci.script-tests-job` all document that Gitea caps `limit` at `MAX_RESPONSE_ITEMS`
(50 in the PR #619 measurement). The review found it by grepping this codebase, not
upstream. Writing a guard against a constant the repo had already measured as wrong is
the same failure as the unfaithful test double two rounds ago: a number believed rather
than checked.
So this is now the THIRD guard for one hole, and the first two were both no-ops:
1. `.statuses | length` vs `.total_count` — `total_count` is the count for the PAGE
RETURNED, not the commit (`?limit=1` on a 6-context head gives
`len=1, total_count=1`). Equal by construction.
2. "refuse when the page is full at 100" — dead code, as above.
3. Ask the server. Completeness is needed ONLY to justify "no verdict exists on this
head", so when the row is absent from page 1 the job reads PAGE 2, and refuses if
it carries anything. Cap-independent: no reconfiguration re-breaks it, and nothing
is hardcoded that a measurement could contradict.
Measured to make sure page 2 is real rather than assumed: `?limit=3&page=2` on
3aed43c6 returns three further rows, and `page=9` returns the same `statuses: null`
terminator the timeline uses.
The probe is skipped when the row IS on page 1, because there is nothing to learn — the
combined endpoint returns the latest status per CONTEXT, so a context cannot recur on a
later page.
TEST-DOUBLE FIDELITY, again the fiddly part. The stub now honours `page`, and that guard
had to go BEFORE the read-counting modes: `appears-on-read:N` counts how many times the
job has LOOKED at the status, and the completeness probe is part of the same look, not a
further one. Letting it increment those counters shifted "the verdict appears on read N"
by one and broke three mid-run-race tests — a false red that would have been easy to
"fix" by adjusting the expected counts, which would have quietly destroyed what those
three tests measure.
Verification: 455 green. Four more mutations, each as intended — deleting the probe
(red), accepting a non-empty page 2 (red), refusing even on an EMPTY page 2 (red, the
deadlock control), and reverting the twin `statuses: null` gate (48 red). Twenty-one
mutations across four rounds.
Refs: #751
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
timothyandClaude Opus 5
951dae26a9