A fourth cold review (Opus, isolated worktree, tests/double/docs focus)
reported no correctness bugs in shipped behaviour but two coverage defects on
exactly the two things this change advertises. Both are closed.
The partial-mark fallback's safety is a claim ABOUT THE ORDERING — page 1 holds
the newest rows, so a walk that fails later still saw the true maximum. The
fixture pinning it served ASCENDING ids, i.e. the arrangement the design calls
unsafe, and passed anyway because the raced row's id sat above even the partial
mark. It could not distinguish safe from unsafe.
The stub now HONOURS the sort parameter: order-faithful modes serve DESC by
default and ASC when the request asks. The new fixture holds a PRE-EXISTING
base-mismatched verdict at id 7055 among 60 rows. Under DESC the salvaged mark
is 7059 and that row is below it — the exemption correctly stands. Under ASC
the mark would be 7049 and that untouched row tests as NEWER, a sticky repair
on a head nothing raced. So re-adding `sort=highestindex` now reddens by
BEHAVIOUR, not only by the structural assertion added in round 5. Measured:
re-adding it reds both tests.
Most modes stay ordering-blind on purpose and now say so: they test walk
COMPLETENESS, which is order-independent, and insertion order is what lets a
fixture place a row beyond page 1.
Also fixed:
- `null` is accepted as an empty page. An array-only gate is the exact shape
of #751 — `count_retargets` had one, the timeline really did return `null`
past the end, and the fence withheld EVERY exemption from the day it
shipped. The same narrowing here is worse, because this walk's failure is
the STICKY sentinel: every exempt PR would need a hand-posted verdict, per
head. Tolerating `null` cannot misread `[]`. Proved by fixture.
- the fail-closed comment said "past the 1000-row page cap"; the bound is 950,
as the walk's own comment and both docs already said.
- the docs claimed "only a read returning no rows at all abandons the mark".
False: a VALIDATED empty history yields a mark of 0 and is not abandoned —
that is the normal first run. What abandons it is a read that both FAILED
and returned nothing. Corrected in ci-cd.md and the record `rule:`.
- a comment pointed at the page-2 probe "a few lines further down"; it was
deleted, so the deixis pointed at nothing.
- the stub claimed its logical-read counter "is only reached on a SUCCESSFUL
page-1 serve" — measured false; it counts page-1 requests, retries included.
- five `(round N)` markers removed. A round number is session chronology and
does not parse for a reader who never saw it (`docs.no-session-narrative`);
an issue number does. The four that remain predate this change.
Verification: `scripts/tests` 1096 passed, 2 skipped. Thirteen executed
mutations across rounds 2-6. The reviewer independently re-ran the earlier
matrix and confirmed it, with one correction carried here: two of those
mutations redden MORE than their named test, so "each reddening exactly its
named test" was wrong — they redden at least it.
refs #763
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
timothyandClaude Opus 5
89db6022d9