fix(751): round-5 findings — a rationale that was itself vacuous, and a third regex round

Fifth cold review: MERGEABLE, no Blocker, no High. Four Low findings, none behavioural.
Fixing all four rather than accepting them, because two are the exact class this issue
exists to retire: text that reads as a checked reason and is not.

A VACUOUS RATIONALE, on the branch about vacuous rationales. The comment on the history
stub's `page` guard said it sits ahead of the read-counting modes "so the page-2 probe
cannot shift 'raced row appears on read N'", by analogy with the combined endpoint.
Measured: moving that guard AFTER the counter modes reddens NOTHING, because no history
mode that counts reads ever issues a page-2 request — `raced=1` on page 1 short-circuits
the probe. The real reason is the other half: page 2 must terminate for modes that
describe page 1 only, and dropping just that `print("[]")` reddens
`test_a_PRE_EXISTING_human_row_does_NOT_trigger_a_repair`. Comment now says which half is
load-bearing and which was wrong. (The COMBINED endpoint's guard genuinely is
counter-related — moving it reddens three mid-run-race tests.)

THE FALSE REPAIR IS STICKY, and the previous commit undersold it as "a stall a reviewer
can clear". It writes `$REPAIR_DESC`, which the classification refuses to grant an
exemption over and re-writes as a fixed point on every later run — so a spurious repair
removes that head's exemption PERMANENTLY, not for one run, and only a human verdict
clears it. Still the right direction against a forged green over a rejection, but it is a
per-sha loss of the exemption, and that is the argument for real paging (#763) rather
than living with this. Said in the comment now.

CORRECTING THE PREVIOUS COMMIT MESSAGE, which over-generalised: "uncertainty resolves to
a stall … never to leaving green" is true of the page-2 probe and NOT of the enclosing
path. An unreadable page 1, or a non-numeric high-water mark, still leaves the exemption
`success` standing unverified. The workflow's own comments state that correctly; the
message did not.

THIRD ROUND ON ONE REGEX, which is the documented budget for a string-matching predicate.
Assertion C started as `\w+\s*\(\)\s*\{`, gained `function\s+\w+` when review found
`function mk {` slipped it, and STILL missed the union form `function mk() {` — the
natural next spelling once the previous one is caught. Now
`^\s*(function\s+)?\w+\s*(\(\s*\))?\s*\{`, verified against all seven spellings.

THE COMPLETENESS COUNT, restored properly. Relaxing `len(bodies) >= 3` to `assert bodies`
fixed a false red but threw away the only check that the walk reached ALL run-bearing
steps: `max(len) > 5000` proves it reached the classifier and nothing about the short
ones, so a helper that silently stopped yielding them would pass an unscanned delimiter.
Now counted against the job's own step list, read directly rather than through the helper
under test — which catches a helper reading the wrong key or dropping steps, while still
tolerating a step being legitimately added or removed.

Verification: 460 green. Three mutations, each as intended — the union spelling `function
mk() {` (red, previously passed), a walk that drops the short steps (red, the property the
count guard restores), and a legitimate step deletion (PASSES, confirming the false red it
replaced stays fixed). Twenty-nine mutations across six rounds.

Refs: #751
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

.gitea/workflows/review-verdict.yml

@@ -1010,6 +1010,15 @@ jobs:
               # as absence. Only checked when page 1 looked clean — a race already found needs no
               # further evidence. Unreadable page 2 also counts as raced: this is the fail-toward-
               # SUCCESS path, so uncertainty must resolve to `pending`, never to leaving green.
+              #
+              # BE HONEST ABOUT THE COST: a repair here is STICKY, not a one-run stall. It writes
+              # `$REPAIR_DESC`, and the classification above refuses to grant an exemption over that
+              # sentinel and re-writes it as a fixed point on every later run — so a SPURIOUS repair
+              # (a head with more status rows than the page cap and no actual race) removes that head's
+              # exemption permanently, and only a human `post-review-verdict.sh` clears it. That is
+              # still the right direction, because the alternative is a forged green over a human
+              # rejection. But it is a per-sha loss of the exemption, not an inconvenience, which is
+              # the argument for replacing this with real paging (#763) rather than living with it.
               if [ "$raced" = "0" ]; then
                 more_hist=$(gh "$BASE_URL/repos/$REPO/statuses/$SHA?limit=100&page=2") || more_hist="UNREADABLE"
                 if [ "$more_hist" = "UNREADABLE" ]; then

scripts/tests/test_pr_changed_files.py

@@ -970,10 +970,19 @@ if "/timeline" in url:
 if "/statuses/" in url:
     mode = os.environ.get("STUB_HISTORY_MODE", "none")
 
-    # PAGE, honoured, and BEFORE the read counters — same reason as the combined endpoint (#751). The
-    # post-write check now reads page 2 to decide whether page 1 was the whole list, and the
-    # `*-after-post` modes key on how many times the job has LOOKED at the history; the page-2 probe is
-    # part of the same look. Letting it bump those counters would shift "raced row appears on read N".
+    # PAGE, honoured. THE REASON IS THE TERMINATOR, NOT THE COUNTERS — and that distinction was itself
+    # a finding (#751 round 5). An earlier version of this comment said the guard sits ahead of the
+    # read-counting modes "so the page-2 probe cannot shift 'raced row appears on read N'", by analogy
+    # with the combined endpoint. Measured: moving this guard AFTER the counter modes reddens NOTHING,
+    # because no history mode that counts reads ever issues a page-2 request — `raced=1` on page 1
+    # short-circuits the probe. So that half of the rationale read as a checked reason and was not,
+    # which is the precise class this branch exists to retire.
+    #
+    # What IS load-bearing: page 2 must terminate for every mode that describes page 1 only, or the job
+    # re-reads page 1's rows as a second page and concludes the list is longer than it is. Dropping
+    # just the `print("[]")` below reddens `test_a_PRE_EXISTING_human_row_does_NOT_trigger_a_repair`.
+    # (The ordering guard on the COMBINED endpoint is a different story and genuinely is counter-
+    # related — moving that one reddens three mid-run-race tests.)
     #
     # PAST THE END THIS ENDPOINT RETURNS `[]`, NOT `null` — measured at 1.27.1:
     # `/statuses/{sha}?limit=100&page=99` is the two bytes `[]`, while `/commits/{sha}/status` past the
@@ -2363,13 +2372,22 @@ def test_the_verdict_workflow_has_NO_expression_delimiter_in_any_run_body():
     # classifier is ~700 lines; a walk that returned nothing, or only the short steps, is the failure
     # to catch. Both are asserted on content, which no spelling change can spoof.
     bodies = [s["run"] for _, s in _iter_workflow_steps(WORKFLOW) if s.get("run")]
-    # NOT `>= 3`. The file has exactly three `run:` bodies today, so that bound had zero slack: cold
-    # review deleted the optional jq-preflight step — a legitimate simplification — and this redded
-    # with a message claiming the classifier had not been examined, which was untrue. The length
-    # assertion below already carries the whole property, so the count only needs to be non-empty.
-    assert bodies, (
-        "the YAML walk reached NO run: bodies in review-verdict.yml, so the assertion above examined "
-        "nothing and a green here proves nothing")
+    # NOT `>= 3`, and not merely non-empty either. `>= 3` had zero slack — cold review deleted the
+    # optional jq-preflight step, a legitimate simplification, and this redded claiming the classifier
+    # had not been examined, which was untrue. But relaxing it to `assert bodies` threw away the only
+    # check that the walk reached ALL run-bearing steps: `max(len) > 5000` proves it reached the
+    # classifier and nothing about the short ones, so a helper that silently stopped yielding them
+    # would let an unscanned delimiter through.
+    #
+    # So count against the job's own step list, read here rather than through the helper under test.
+    # That catches the failure the count existed for (a helper looking at the wrong key, or dropping
+    # steps) without breaking when a step is legitimately added or removed.
+    import yaml as _yaml
+    _steps = (_yaml.safe_load(WORKFLOW.read_text())["jobs"]["set-verdict-status"]["steps"] or [])
+    declared = sum(1 for s in _steps if isinstance(s, dict) and s.get("run"))
+    assert len(bodies) == declared, (
+        f"the YAML walk reached {len(bodies)} run: bodies but the job declares {declared} — the "
+        "assertion above did not examine every body, so a green here proves nothing")
     assert max(len(b) for b in bodies) > 5000, (
         "the YAML walk did not reach a substantial run: body — the ~700-line classifier is the one "
         "that must be scanned, so this test would be vacuous")
@@ -2623,7 +2641,11 @@ def test_the_dropped_step_guard_BEHAVIOURALLY_fails_without_the_marker_and_passe
     # assertion makes the intent explicit and fails loudly rather than subtly.
     body_before = "\n".join(ln for ln in prologue.splitlines()
                             if not ln.lstrip().startswith("#"))
-    assert not re.search(r"^\s*(\w+\s*\(\)|function\s+\w+)\s*\{", body_before, re.M), (
+    # Covers all four bash spellings: `mk() {`, `mk(){`, `function mk {`, `function mk() {`. The third
+    # was added when review found the first regex missed it, and the FOURTH still slipped that fix —
+    # the union form is the natural next spelling once `function mk {` is caught. Budget three rounds
+    # for any string-matching predicate.
+    assert not re.search(r"^\s*(function\s+)?\w+\s*(\(\s*\))?\s*\{", body_before, re.M), (
         "a function is defined before the marker write, so the write may be inside it and unreached "
         f"in production while this test still passes:\n{body_before}")