A third cold review (Opus, isolated worktree) returned NOT MERGEABLE with one
High and three Medium. All are addressed.
HIGH — the stated motivation was wrong, and self-contradictory once round 4
landed. Under the server default (`created_unix DESC`) page 1 holds the NEWEST
rows and ids are monotonic with `created_at`, so page 1 already carried the
true maximum id AND every row newer than the mark — the only rows the
post-write check selects on. A single-page read therefore missed a raced
verdict only if more than 50 rows were created INSIDE the write window, not
merely on "a head with more than 50 rows", which the issue, the comments and
the docs all asserted. Reviewer executed an order-faithful DESC stub: a
page-1-only reader repairs identically to the full walk.
What actually removed #761's stall is retiring #751's page-2 probe, not the
paging. The walk still earns its place, for a reason now stated instead of the
false one: it stops the gate's one fail-toward-SUCCESS path depending on an
undocumented ordering the server honours only coarsely (page 1 came back
`114,112,113,111,110`). That measurement was deleted in commit 1 and is
restored, since round 4's safety argument rests on exactly it.
MEDIUM/real defect — the string-id TWIN, live on `main` and one expression
away from the fix already made: `select((.id? // 0) > $since)`. jq orders
strings above every number, so a PRE-EXISTING row with `"id": "3"` reads as
newer than any mark, is counted as having raced the write, and gets the sticky
sentinel plus a false "was overwritten" on EVERY later run — a permanent
per-sha stall no re-trigger clears. Now numeric-only, with a test.
Also fixed: a non-empty history carrying no numeric id was collapsed to a mark
of 0 (making every pre-existing row look newer); it is now reported unusable
and the check is skipped. `sleep` no longer fires after the final attempt.
Three unpinned clauses now have tests, each proved by an executed mutation:
- the page cap is a refusal, not a terminator (1050-row fixture)
- the `::error::` found-vs-unverifiable distinction (forcing `raced_why=human`
reddened nothing before)
- the walk requests no sort order — a structural guard on round 4's
withdrawal, which nothing mechanical protected. It reads request LINES, not
comments, since the withdrawal note names the parameter to explain it.
Honest scoping, not new code: the test double is ordering-blind, so the paging
tests prove WALK COMPLETENESS, not that a real raced verdict would otherwise be
missed — under DESC it would not be. The stub comment and the docstrings now
say so rather than implying the stronger claim.
Docs: `ci-cd.md` and the record's `rule:` carry the corrected reachability, the
DESC dependency of the partial-mark fallback, and both rejected alternatives
stated as rejected alternatives rather than as draft chronology
(`docs.no-session-narrative`).
Verification: `scripts/tests` 1094 passed, 2 skipped; eleven executed
mutations across rounds 2-5, each reddening exactly its named test;
decisions_validate and build_decisions_catalog --check exit 0.
refs #763
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
timothyandClaude Opus 5
53ee44cbc1