test(antithesis): use certified projection horizons (EN-1946)

[gfyrag]

Stack 6/7 for EN-1946. Migrates Antithesis assertions from the client-selected minimum log sequence to default linearizable reads backed by certified projection horizons. The stale fault scenario remains unchanged.

[NumaryBot]

✅ Approve — automated review

The migration preserves the dedicated MinLogSequence probe, uses default projection-aligned reads consistently, and fixes the previously discussed silent error paths in the new driver.

No findings.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.37%. Comparing base (615ac0f) to head (4710460).

Additional details and impacted files

@@                            Coverage Diff                             @@
##           feat/en-1946-remove-leader-consistency    #1892      +/-   ##
==========================================================================
- Coverage                                   77.41%   77.37%   -0.04%     
==========================================================================
  Files                                         458      458              
  Lines                                       48632    48632              
==========================================================================
- Hits                                        37647    37629      -18     
- Misses                                       7829     7839      +10     
- Partials                                     3156     3164       +8     

Flag	Coverage Δ
e2e	77.37% <ø> (-0.04%)	⬇️
scenario	77.37% <ø> (-0.04%)	⬇️
unit	77.37% <ø> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1892 (comment)

[shipfox-ai]

Review of PR #1892 (test(antithesis): use certified projection horizons)

The migration itself is correctly executed: no MinLogSequence / ReadOptions{...} usage remains anywhere under tests/antithesis/ (the minLogSequence occurrences left in singleton_driver_model are response-field extractions, not read options), the fault scenario is untouched, and I verified server-side that the replacement premise actually holds — the routed controller takes a ReadIndexAndWait horizon and DefaultController.ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1789-1832) explicitly waits for the async audit secondary index's Raft progress to reach the main applied index, so the migrated audit/log/list/query assertions are genuinely backed by a certified projection horizon, not just an assumed one. The IsTransient change (dropping READ_INDEX_NOT_CAUGHT_UP) is coherent cleanup, not scope creep: query.ErrReadIndexNotCaughtUp is only raised when req.GetMinLogSequence() > 0 (internal/storage/query/executor.go:156), so after this PR the workload can no longer receive it. The PR bot's "drivers no longer compile / unused resp, maxSeq" claim does not reproduce — those values are still used as diagnostics. However, the PR deletes the only chaos coverage of a still-supported production mechanism and silently swallows errors in its replacement driver, so I recommend request changes.

Standards

1. [P2][blocking][documented-standard violation] The still-supported client-selected MinLogSequence contract loses all regression coverage.
tests/antithesis/workload/bin/cmds/main/parallel_driver_minlogseq/main.go (whole file, per diff). AGENTS.md ("Regression tests are additive across production triggers: adding coverage for a new caller or failure path must not replace the existing regression trigger") and docs/technical/contributing/testing.md:21-29 require preserving an independent case for the original trigger. The legacy min_log_sequence field is still honored server-side as a client-selected floor (docs/technical/architecture/subsystems/read-path/query-pipeline.md:115-118, "during this staged rollout"; internal/adapter/grpc/server_bucket.go:1363 gates ExecutePreparedQuery on it) and is still exposed via the HTTP API (internal/adapter/http/handlers_execute_prepared_query.go:41,93). The PR deletes both probes that exercised it: the honored-floor probe (success with MinLogSequence = S reflecting the acked write) and the refusal-fidelity probes (readIndexRefusal/checkRefusal verifying READ_INDEX_NOT_CAUGHT_UP self-consistency, current < requested — a reason the server still emits, internal/adapter/grpc/server.go:649-666). A regression in either contract (refusal never raised, malformed or non-monotonic refusal metadata, floor silently ignored) would now ship with a fully green Antithesis run. Keep an independent explicit-floor case (success at MinLogSequence = S, refusal at S + Δ with metadata checks) alongside the new default-read probe.

2. [P2][blocking][documented-standard violation] The migrated driver discards every ExecutePreparedQuery error without any trace signal.
parallel_driver_minlogseq/main.go:108-110: the new hunk is if err != nil { return }. AGENTS.md ("Do not ignore errors") and tests/antithesis/README.md ("Swallow an error … at least log via LogCleanupError … or assert.Reachable(…) so the path stays visible"; the canonical skip pattern classifies unexpected errors with assert.Unreachable) forbid this. The driver's own header says "Transient errors → inconclusive, skip", but the code skips all errors, including definitive business answers (e.g. a persistent FailedPrecondition), which the classify interceptor deliberately does not flag. Consequence: any persistent query-path failure silently disables both remaining assertions of this driver while the run stays green (assert.Reachable at :111 only fires on success). Classify with IsTolerated/Unreachable or emit a skip Reachable.

3. [P3][non-blocking][documented-standard violation: stale comments + dead plumbing] Unused minLogSeq parameters and now-false causal-horizon comments across four drivers.
Per AGENTS.md ("Interface/behavior change: update relevant code comments"), the comment updates are only half-done and the sequence plumbing is dead:

• parallel_driver_reference_race/main.go:107-113 — countTransactionsWithReference still documents "a fixed causal horizon" and takes minLogSeq uint64 it never reads; the caller (:153, :158) still computes floor via writeMarker and threads it into the dead parameter; :73-74 leaves a broken sentence mid-wrap ("could correspond to. Returns / (0, false) …").
• parallel_driver_bulk_atomicity/main.go:76 and parallel_driver_ledger_recreate/main.go:81 — listIsEmpty/listMatches accept an unused minLogSeq; callers still compute it from the marker response (bulk_atomicity/main.go:220, ledger_recreate/main.go:178) solely to thread it into functions that discard it (it survives only as details metadata).
• parallel_driver_definitive_errors/main.go:62,220-223 — same shape; here the marker-sequence computation is fully dead (not even used in details).
• tests/antithesis/workload/internal/client.go:454 — the IsTransient doc still cites "lagging read → caught up" as a member of the retry set, which this PR removed.
  Impact: future readers believe the value still affects the RPC. Delete the parameters and dead computations, and reword the comments to the subsequent default linearizable read.

4. [P3][non-blocking][documented-standard violation: contradictory comment] eventually_cross_node_identity retry comment misstates the retried set.
main.go:153-157 (rewritten by this diff) says Raft transients surface as "DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable", but the retry at :165 uses IsTransient only, which deliberately excludes both Aborted and Canceled (internal/client.go IsAborted/IsCanceled docs; tests/antithesis/README.md:79-82). A future editor following this comment would broaden the retry set against the documented design. State the actual set (Unavailable | DeadlineExceeded | ExternalServiceError).

Spec

No confirmed material finding. The PR body's two claims verify: (1) the migration is complete — every ListTransactions/ListLogs/ListAuditEntries/ExecutePreparedQuery in the workload now uses the default read path, and the "certified projection horizons" premise is real, not just assumed (the audit secondary index, the one projection with independent async progress, is explicitly aligned to the ReadIndex horizon in DefaultController.ListAuditEntriesFrom, so the dropped MinLogSequence gate did not silently remove the audit freshness guarantee); (2) the fault scenario is unchanged — the diff touches only driver sources, internal/client.go, and README.md, with no scenario/fault configuration.

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.

[shipfox-ai]

This PR migrates the Antithesis workload from client-selected MinLogSequence read floors to default linearizable reads backed by certified projection horizons. I verified the migration is sound: no MinLogSequence or READ_INDEX_NOT_CAUGHT_UP usage remains under tests/antithesis/; the default read path routes through RoutedController.readCtrl → ReadIndexAndWait (internal/bootstrap/controller_routed.go:59-124, internal/infra/node/read_index.go:94-101), so the removed floors are compensated server-side; the stale-reads fault scenario is unchanged (parallel_driver_stale_reads still sends x-consistency: stale). The remaining findings are error-classification tightening and comment/dead-code cleanup; none undermines the migration's correctness. Recommendation: approve with comments.

Standards

1. Moderate — the rewritten prepared-query probe silently skips its core assertion on classified business errors.
   tests/antithesis/workload/bin/cmds/main/parallel_driver_minlogseq/main.go:103-106 adds a bare if err != nil { return } before the driver's only Reachable/Always assertions. The README's Error classification section prescribes the canonical "tolerate the error and skip" pattern — if !internal.IsTolerated(err) { assert.Unreachable(...); return } — precisely so that unexpected rejections are surfaced rather than swallowed. Because the classify interceptor (internal/client.go:222-241) treats business codes (NotFound, FailedPrecondition, …) as classified, a persistent business rejection of ExecutePreparedQuery makes this driver emit zero signal for the entire run — indistinguishable from never running (there is no upstream Sometimes sonde either). The codebase's own exemplar is parallel_driver_stale_reads/main.go:128-131. The file header's "transient errors → inconclusive, skip" convention only licenses skipping transient errors, not swallowing business ones on the driver's core property check. Classify the result per the canonical pattern and add the coverage sonde.

2. Minor — comment drift contradicting the documented classifier contract (two sites; AGENTS.md documentation-maintenance rule).

   • tests/antithesis/workload/bin/cmds/main/eventually_cross_node_identity/main.go:155-159: the diff-rewritten comment claims IsTransient retries "DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable." IsTransient (workload/internal/client.go:465-470) covers only Unavailable | DeadlineExceeded | ExternalServiceError; Aborted is deliberately surfaced loud, not retried (IsAborted, client.go:304-312), and Canceled is a local-lifecycle exit (IsCanceled, client.go:321+). In reality an Aborted from Barrier falls through to assert.Unreachable("cross-node oracle barrier unexpected error") at main.go:171-173 — so either the comment is wrong, or fault-window Raft aborts are being mis-triaged as workload findings. Align the comment (and, if Aborted genuinely surfaces under clog faults, that tension deserves an explicit decision, not a stale sentence).
   • workload/internal/client.go:454-455: the IsTransient doc still cites "lagging read → caught up" as a way the retry condition clears, but this very diff removed IsReadIndexNotCaughtUp from the set, so no remaining member matches that description. (The retryUnaryInterceptor comment was updated correctly; only this doc residue remains.)

3. Minor — dead sequence-floor plumbing and now-false freshness comments left in four drivers after the Read: option deletion.
   The list helpers still accept a minLogSeq uint64 parameter that is now ignored, and callers still extract and thread it, while comments still describe a client-side floor mechanism that no longer exists:

   • parallel_driver_bulk_atomicity/main.go:76 (ignored param), :188-189 — "the marker's sequence is a valid floor for reads on the primary ledger" is false post-change; :220 extraction now only feeds details["minLogSeq"].
   • parallel_driver_ledger_recreate/main.go:81 (ignored param), :171-172 — "its global log sequence is the freshness floor for every isolation read below" is false; :178 extraction.
   • parallel_driver_definitive_errors/main.go:62 (ignored param), :184 — "Consistency barrier with a usable read floor" is stale; :220 extraction.
   • parallel_driver_reference_race/main.go:113 (ignored param); writeMarker (:75) still returns a sequence whose only remaining use is threading the ignored parameter (:153, :158).
     Impact: misleading comments invite a future maintainer to reinstate client-side floors (or trust a freshness guarantee that no longer exists), and the inert plumbing obscures what the marker write is actually for now (a causal barrier / diagnostic). Drop the parameters and sequence extraction, or retitle the marker and retained details field to its actual role. Optionally, the marker-write shape repeated across these four drivers could be hoisted into workload/internal.

Spec

No confirmed material finding. No standalone spec was reachable (the PR's ticket reference is a Jira key not resolvable from this review environment), so the PR's stated intent was verified directly against the code: the client-selected-floor migration is complete and consistent (see summary above), and the "stale fault scenario remains unchanged" clause holds — parallel_driver_stale_reads/main.go is absent from the diff and still exercises x-consistency: stale prefix-consistency semantics.

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.

[NumaryBot]

NumaryBot review complete: no remaining inline findings.

Resolved 1 stale NumaryBot review thread (1 fixed, 0 outdated).

Summary: #1892 (comment)

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1892 (comment)

[shipfox-ai]

This PR migrates the Antithesis workload from client-selected MinLogSequence freshness floors to default linearizable reads backed by certified projection horizons, and removes IsReadIndexNotCaughtUp from the workload classifier. Both spec claims verify cleanly against the code: no MinLogSequence / READ_INDEX_NOT_CAUGHT_UP usage remains anywhere under tests/antithesis/, parallel_driver_stale_reads is untouched and still exercises x-consistency: stale, and the replacement premise is real (RoutedController.readCtrl takes a ReadIndexAndWait barrier at internal/bootstrap/controller_routed.go:79, with projection-alignment waiting on the read side). However, the rewritten parallel_driver_minlogseq driver swallows all errors before its only assertions, and the PR deletes the suite's only coverage of a server path that is still live. Recommendation: request changes.

Standards

[P2] The rewritten driver's core hunk swallows every error before its only assertions — tests/antithesis/workload/bin/cmds/main/parallel_driver_minlogseq/main.go:108-110.
The new code is execResp, err := client.ExecutePreparedQuery(...) followed by a bare if err != nil { return }. This violates the canonical "tolerate the error and skip" pattern in tests/antithesis/README.md (if !internal.IsTolerated(err) { assert.Unreachable(...); return }) and AGENTS.md ("Do not ignore errors"). The driver header comment licenses skipping only transients ("Unavailable, NotFound from lagging metadata"), but the code skips all errors — including permanent business answers such as a persistent FailedPrecondition. Since the classify interceptor treats business codes as classified, such a failure produces zero signal anywhere: neither assert.Reachable (:115) nor assert.Always (:135) fires and the chaos run stays green while the projection-alignment property is never exercised — exactly the "silently report all green on a real bug" failure mode the README's conventions exist to prevent. Classify the error (tolerate-and-skip with an Unreachable for anything unexpected, per the README pattern).

[P2] Regression coverage for a still-supported production trigger is deleted, not preserved — parallel_driver_minlogseq/main.go (both explicit-floor probes removed).
internal/query/executor.go:156-161 still returns ErrReadIndexNotCaughtUp when a request carries MinLogSequence > 0, and the field remains in the public API, so client-selected floors are still a live, honored-or-refused server behavior. The diff removes the only probes covering honored-floor success and self-consistent refusal (including the malformed-refusal-metadata check). docs/technical/contributing/testing.md ("Regression preservation and branch proof") requires preserving an independent case for the original trigger when a mechanism gains a new one, and AGENTS.md requires regression tests to be additive across production triggers. Keep an explicit-floor sibling probe alongside the new default-read probe.

[P3] Dead floor plumbing and now-false comments left behind in four drivers.
After the Read: &commonpb.ReadOptions{MinLogSequence: ...} lines were deleted, the helpers still accept unused minLogSeq parameters and callers still extract and thread sequences: parallel_driver_reference_race/main.go:113 (param unused; writeMarker's return value threaded in only for that), parallel_driver_bulk_atomicity/main.go:76,220-221, parallel_driver_ledger_recreate/main.go:81,178-179 (survives only as a details["minLogSeq"] diagnostic), parallel_driver_definitive_errors/main.go:62,220. The parallel_driver_reference_race/main.go:107 comment still claims reads happen "at a fixed causal horizon" — the RPC now carries no horizon; freshness comes from the default linearizable path. This breaches AGENTS.md's "Interface/behavior change: update relevant code comments" and obscures the actual freshness mechanism. Delete the inert parameters and extractions and correct the comments.

[P3] Classifier comments contradict the actual IsTransient contract.

• tests/antithesis/workload/internal/client.go:454-455: the IsTransient doc still says the retry condition clears "(no leader → elected, lagging read → caught up)", but this PR removed the "lagging read" member (IsReadIndexNotCaughtUp); no remaining member matches that clause.
• tests/antithesis/workload/bin/cmds/main/eventually_cross_node_identity/main.go:153-156: the rewritten comment claims Raft transients can surface as "DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable", but the retry at :165 uses IsTransient, which deliberately excludes both Aborted (surfaced loud via assert.Unreachable at :169 per the README) and Canceled. The comment invites maintainers to broaden error tolerance incorrectly. Align both comments with the actual retry set.

Spec

No confirmed material finding. Both claims verify: (1) the migration to default linearizable reads is complete across all list, audit, and prepared-query drivers, with the certified-horizon premise backed by real production code (ReadIndex barrier in internal/bootstrap/controller_routed.go:79, applied-index proof and projection-certificate waits in the aligned-snapshot path); (2) the stale fault scenario is unchanged — parallel_driver_stale_reads/main.go is absent from the diff and still sends x-consistency: stale, and the classifier cleanup cannot affect it because the removed error reason is only emitted for requests carrying a non-zero minimum sequence, which the workload no longer sends.

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.

[shipfox-ai]

Review summary: PR #1892 — test(antithesis): use certified projection horizons (EN-1946)

This PR migrates the Antithesis workload from client-selected ReadOptions.MinLogSequence / prepared-query MinLogSequence floors to default linearizable reads backed by the server's certified projection horizons, and removes the now-dead READ_INDEX_NOT_CAUGHT_UP transient classification. I verified the core premise against the code: the server's alignment machinery genuinely covers the new probe shapes (internal/query/aligned_snapshot.go:50-56 — AlignmentOwed requires alignment for filtered reads and all LOGS reads; AlignedIndexSnapshot certifies the projection against the ReadIndex horizon; internal/application/ctrl/controller_default.go:1808-1837 certifies audit projection progress for filtered audit reads, while unfiltered audit reads scan the main-store audit zone bounded by the main-store horizon), and no MinLogSequence input remains anywhere under tests/antithesis. The stale-reads driver (parallel_driver_stale_reads/main.go:73) is untouched and still exercises the x-consistency: stale path as before. However, two blocking Standards issues remain: the rewritten parallel_driver_minlogseq driver swallows query errors without classification, and the deleted floor probes were the only workload coverage of a server contract that is still live.

Recommendation: request changes (two blocking Standards findings; everything else is comment/cleanup-level).

Standards

[P1][blocking][documented-standard] Prepared-query driver swallows every error without classification

tests/antithesis/workload/bin/cmds/main/parallel_driver_minlogseq/main.go:108-110 replaces the old refusal-aware handling with a bare if err != nil { return } on ExecutePreparedQuery.

tests/antithesis/README.md:109-118 documents the canonical skip pattern (if !internal.IsTolerated(err) { assert.Unreachable(...); return }), and README.md:177-179 explicitly forbids swallowing an error without a trace signal (assert.Reachable("X skipped due to error", …) at minimum); AGENTS.md:68 says "Do not ignore errors." The classify interceptor (workload/internal/client.go:202-221) only fires for unclassified codes, so classified permanent answers — NotFound, InvalidArgument, generic FailedPrecondition — bypass it too. The new code therefore skips on all errors, including definitive business answers, while the file header (main.go:11-12) licenses skipping only transient errors. Since assert.Reachable("projection-aligned prepared query succeeded", …) at main.go:106 and the assert.Always at main.go:132 are this driver's only assertion sites, a persistent query-path failure silently reduces the driver to a no-op for entire runs with no triage signal beyond an absent Reachable count.

Fix: use the canonical pattern — if !internal.IsTolerated(err) { assert.Unreachable(...); return } — or at minimum emit a Reachable/Sometimes sonde on the skip path.

[P1][blocking][documented-standard] Explicit-floor regression coverage deleted while the server contract is still live

The diff deletes the only workload probes exercising two still-live server behaviors:

• Prepared-query floor honored-or-refused: internal/query/executor.go:156-161 still returns ErrReadIndexNotCaughtUp when MinLogSequence is requested, and internal/adapter/grpc/server.go:649-660 still converts it to a self-consistent FailedPrecondition with READ_INDEX_NOT_CAUGHT_UP metadata (requested/current).
• ListOptions.Read.MinLogSequence on the list RPCs: internal/adapter/grpc/server_bucket.go:597, 994-1006, 1135, 1363, 1387 still gate live reads on the client-supplied floor.

The deleted probes (the entire old parallel_driver_minlogseq body, plus the Read: fields across the list drivers) were the only Antithesis coverage asserting that these floors are honored on success and that refusals carry self-consistent metadata. AGENTS.md:78 ("Regression tests are additive across production triggers: adding coverage for a new caller or failure path must not replace the existing regression trigger") and docs/technical/contributing/testing.md:26-29 require preserving an independent case for the original trigger. As written, a future regression that ignores the supported floor or emits malformed refusal metadata can no longer be caught by this suite. If a later EN-1946 PR deletes these server inputs, the server fields and their remaining unit tests should be removed in the same change — until then, keep one independent explicit-floor probe alongside the new default-read probe.

[P3][non-blocking][documented-standard] Rewritten comments contradict the documented IsTransient contract

• tests/antithesis/workload/bin/cmds/main/eventually_cross_node_identity/main.go:151-156: the diff-rewritten comment claims Raft transients "can surface as DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable," but the retry at main.go:163 uses internal.IsTransient only, which deliberately excludes Aborted (surfaced loud via the classify interceptor — workload/internal/client.go:468-476, README.md:79-82) and Canceled (local lifecycle). The old text named READ_INDEX_NOT_CAUGHT_UP, which was in the set; the replacement names two members that are not. A maintainer reading this comment could broaden the retry set contrary to the documented design.
• tests/antithesis/workload/internal/client.go:454: the IsTransient doc still says the retry condition clears "(no leader → elected, lagging read → caught up)" — but this diff removed the only member matching "lagging read → caught up." The retryUnaryInterceptor comment (client.go:159-163) was updated; this residue was not.

AGENTS.md:84 requires comments to track behavior changes. Update both comments to describe the current retryable set.

[P3][non-blocking] Dead floor plumbing and stale mechanism comments left behind

The same floor-removal edit left dead code and now-false comments across four drivers:

• Unused minLogSeq uint64 parameters retained after the diff removed their only use: parallel_driver_reference_race/main.go:113 (the floor threaded from writeMarker at main.go:153-158 is fully ignored), parallel_driver_bulk_atomicity/main.go:76 (callers still compute and pass it at main.go:220-226), parallel_driver_ledger_recreate/main.go:81 (main.go:178-191), parallel_driver_definitive_errors/main.go:62 (main.go:220-223). Same for the now-vestigial maxSeq bookkeeping in parallel_driver_list_completeness/main.go:53,90-92 and parallel_driver_timestamp_order/main.go:68,108-110. Delete the parameters/extractions; keep the sequence in details metadata only where it remains useful diagnostics.
• Stale comments: parallel_driver_reference_race/main.go:106-107 still says the list runs "at a fixed causal horizon" though no horizon is passed anymore, and the writeMarker doc (main.go:70-74) has a broken mid-sentence wrap ("could correspond to. Returns / (0, false)") introduced by the rewrite.
• Naming: parallel_driver_minlogseq and internal.PrefixMinLogSeq now name a mechanism the driver no longer exercises; consider renaming when convenient (not worth churning the run harness alone).

Spec

No confirmed material Spec finding. The PR references only the external Jira key EN-1946, which is not resolvable as a spec in this repository, so no spec-conformance review was possible. For the record, the change is internally consistent with its stated intent: no client-selected MinLogSequence request input remains under tests/antithesis, the replacement default-read calls are sound given the server's alignment/certification machinery (verified as described in the summary), and the stale-read fault scenario is unchanged.

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.

[shipfox-ai]

Both reports were verified line-by-line against diff.patch and the code under ../../target; every finding below was independently reproduced in the code, and the unsupported or style-only candidates (naming of parallel_driver_minlogseq, maxSeq in list_completeness/timestamp_order, "duplicated marker shape") were dropped. The PR's two stated requirements are both genuinely implemented — all client-selected MinLogSequence floors are gone from the Antithesis workload and the default linearizable path really is backed by projection certification — but the rewritten prepared-query driver swallows all errors ahead of its only assertions, and the migration deleted the workload's only coverage of a still-live server contract. Recommendation: request changes (two P2 Standards findings).

Standards

[P2][blocking][documented-standard] Rewritten prepared-query driver swallows every error before its only assertions

Location: tests/antithesis/workload/bin/cmds/main/parallel_driver_minlogseq/main.go:107-110

The new hunk is a bare if err != nil { return } on ExecutePreparedQuery, directly ahead of the driver's only assert.Reachable (:110) and assert.Always (:133) sites. This violates AGENTS.md ("Do not ignore errors") and tests/antithesis/README.md ("Swallow an error … at least log via LogCleanupError … or assert.Reachable("X skipped due to error", …) so the path stays visible"; canonical pattern: if !internal.IsTolerated(err) { assert.Unreachable(...) }). The driver's own header comment claims only "Transient errors … → inconclusive, skip", but the code skips all errors — including definitive business answers (persistent NotFound/InvalidArgument/FailedPrecondition) that the classify interceptor deliberately does not flag, so a permanently broken query path produces zero assertion signal across entire green runs. Impact: the migrated oracle is weaker than the one it replaces and can silently never fire. Resolution: skip only tolerated transients/cancellation (internal.IsTolerated) and surface anything else with assert.Unreachable.

[P2][blocking][documented-standard] Regression coverage for the still-live explicit-floor trigger is deleted, not preserved

Location: tests/antithesis/workload/bin/cmds/main/parallel_driver_minlogseq/main.go (deleted honored-floor probe and readIndexRefusal/checkRefusal refusal-fidelity probes)

AGENTS.md: "Regression tests are additive across production triggers: adding coverage for a new caller or failure path must not replace the existing regression trigger", and docs/technical/contributing/testing.md: "When a shared mechanism gains a second production trigger, preserve an independent case for the original trigger." The server contract is fully live: internal/query/executor.go:156-161 still returns ErrReadIndexNotCaughtUp when MinLogSequence > 0, internal/adapter/grpc/server.go:649-659 still emits the READ_INDEX_NOT_CAUGHT_UP ErrorInfo with requested/current metadata, and internal/adapter/grpc/server_bucket.go gates reads on waitMinLogSequence at ~11 sites (597, 650, 699, 762, 1004, 1060, 1135, 1363, 1387, 1436, 1480). A repo-wide grep finds no *_test.go covering that refusal path — the deleted probes (honored-floor success, plus refusal self-consistency including malformed-metadata detection) were the only coverage of it anywhere. Impact: a regression that ignores an explicit floor or emits malformed refusal metadata can now land with no failing signal. Resolution: retain an independent explicit-floor/refusal probe alongside the new default-read probe (or add equivalent unit coverage).

[P3][non-blocking][documented-standard] Stale comments and dead floor plumbing left behind by the behavior change

AGENTS.md: "Interface/behavior change: update relevant code comments." Verified inaccurate or now-dead sites, all touched-adjacent:

• workload/internal/client.go:453-454 — IsTransient doc still cites "lagging read → caught up" as a clearing condition; that member was removed from the set.
• workload/internal/client.go:488-489 — isBusinessError doc still says "minus the two reasons that IsTransient already covers"; only ExternalServiceError remains.
• eventually_cross_node_identity/main.go:154-158 — comment (edited in this diff) claims Raft transients surface as "DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable", but the retry at :165 uses IsTransient, which deliberately excludes both Aborted (surfaced loud per README) and Canceled.
• parallel_driver_bulk_atomicity/main.go:188-190 — "the marker's sequence is a valid floor for reads on the primary ledger": no floor is passed anymore.
• parallel_driver_ledger_recreate/main.go:171-172 — "its global log sequence is the freshness floor for every isolation read": now false.
• parallel_driver_reference_race/main.go:70-74 — writeMarker doc still presents the returned sequence as a usable floor and the edit left a broken sentence ("could correspond to. Returns / (0, false)"); :106-107 still says "at a fixed causal horizon".
• Dead plumbing: the minLogSeq uint64 parameter survives in reference_race/main.go:113 (threaded from writeMarker at :166), bulk_atomicity/main.go:76 (:220-233), ledger_recreate/main.go:81 (:178-191), and definitive_errors/main.go:62 (:220-223, fully dead) although nothing reads it; in the first three the sequence is already kept as details diagnostics.

Resolution: fix the comments to describe the default linearizable barrier and drop the dead parameters, keeping sequences only as details diagnostics.

Spec

No confirmed material Spec finding. Both PR-body claims were verified:

1. "Migrates Antithesis assertions from the client-selected minimum log sequence to default linearizable reads backed by certified projection horizons" — implemented. Grep confirms zero remaining MinLogSequence/ReadOptions uses under tests/antithesis/. The replacement path is real: default reads route through RoutedController.readCtrl → ReadIndexAndWait (internal/bootstrap/controller_routed.go:59-141) with the horizon attached via withLocalBarrierHorizon; the query executor certifies the projection against that horizon via AlignmentOwed/AlignedIndexSnapshot (internal/query/aligned_snapshot.go:62-64, executor.go:128-161), and the rewritten prepared-query driver uses an address-prefix filter, so it exercises the certified alignment path rather than the unaligned fast path.
2. "The stale fault scenario remains unchanged" — holds. parallel_driver_stale_reads/main.go is absent from the diff and still sends x-consistency: stale (:73) with its prefix-consistency assertions and lag-observation sondes intact.

The classifier cleanup (removing IsReadIndexNotCaughtUp from IsTransient/isBusinessError in workload/internal/client.go) is justified consequence cleanup rather than scope creep: the server emits that condition only for a non-zero requested minimum sequence, which the migrated workload no longer sends.

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.

[shipfox-ai]

This PR migrates the Antithesis drivers from client-supplied MinLogSequence freshness floors to default linearizable reads backed by certified projection horizons. I verified the migration premise against the server: default reads go through ReadIndexAndWait with a fixed-horizon projection wait (docs/technical/architecture/subsystems/read-path/query-pipeline.md:76-101), so dropping the floors from the list reads is sound and the drivers' assertions remain valid. However, the change deletes the only regression coverage for the still-live MinLogSequence honored-or-refused behavior, leaves dead plumbing behind in four drivers, and rewrites one error-handling/comment site against the documented workload conventions. Recommendation: request changes — restore coverage for the MinLogSequence production trigger (or state its removal) before merge; the remaining items are small cleanups.

Standards

1. [High] The only regression coverage for the still-live MinLogSequence honored-or-refused behavior is deleted

tests/antithesis/workload/bin/cmds/main/parallel_driver_minlogseq/main.go.

The rewrite removes both probes: the honored-path probe (MinLogSequence: ackedSeq must include the acked write) and the fail-fast refusal probe (MinLogSequence = ackedSeq + 1e6 must be refused with self-consistent READ_INDEX_NOT_CAUGHT_UP metadata current < requested). The new driver only sends a default read. The behavior under test is still live in the server: internal/query/executor.go:155-160 returns ErrReadIndexNotCaughtUp whenever req.GetMinLogSequence() > 0 && mainSeq < req.GetMinLogSequence(), converted to FailedPrecondition + READ_INDEX_NOT_CAUGHT_UP with requested/current metadata in internal/adapter/grpc/server.go:649-655, and the public API docs still recommend the field for read-your-writes (docs/technical/architecture/subsystems/api/grpc-api.md:655-681). Nothing else covers it: a search of tests/e2e finds no MinLogSequence request usage. This violates AGENTS.md:78 ("Regression tests are additive across production triggers … must not replace the existing regression trigger"). Impact: a server regression in the freshness gate — honoring an unsatisfiable sequence, or emitting refusal metadata with current >= requested — would ship with no failing test anywhere. Fix: keep (or re-add as a separate driver) a probe pair covering honored-success and self-consistent refusal, alongside the new projection-alignment probe.

2. [Medium] Rewritten probe swallows all errors and pairs success with a passive Reachable

parallel_driver_minlogseq/main.go:108-111: the new hunk is if err != nil { return } followed by assert.Reachable("projection-aligned prepared query succeeded", …) with no upstream assert.Sometimes. This contradicts the documented conventions in tests/antithesis/README.md: the canonical tolerate-and-skip pattern requires if !internal.IsTolerated(err) { assert.Unreachable(…); return }; "What not to do" forbids silent swallows; and "A Reachable with no upstream Sometimes is passive." Impact: a definitive business error on the read (e.g. InvalidArgument, or a FailedPrecondition refusal) is indistinguishable from a partition, and Antithesis gets no coverage sonde biasing exploration toward the success path of the very property this driver exists to check. Fix: classify with IsTolerated/Unreachable and pair the success Reachable with a Sometimes.

3. [Medium] Dead minLogSeq plumbing and floor language left behind in four drivers

The diff removes every Read: &commonpb.ReadOptions{MinLogSequence: …} use but keeps the vestigial parameters, computed sequences, and floor wording:

• parallel_driver_bulk_atomicity/main.go:76 — listIsEmpty(..., minLogSeq uint64) param unused; call sites :226, :233 still pass it; :220-221 computes the marker sequence and stores it as details["minLogSeq"], a now-misleading key.
• parallel_driver_ledger_recreate/main.go:81 — listMatches(..., minLogSeq uint64) unused; call sites :184, :191; :178-179 same misleading detail key.
• parallel_driver_reference_race/main.go:113 — countTransactionsWithReference(..., minLogSeq uint64) unused; writeMarker (:76) still returns the marker sequence whose only consumer is this dead floor argument (:160-166); its comment also has a broken sentence ("…could correspond to. Returns / // (0, false)…").
• parallel_driver_definitive_errors/main.go:62 — referenceFilterCheck(..., minLogSeq uint64) unused; call site :223 passes a sequence computed at :220 that never reaches any request; the barrier comment at :186 still opens with "Consistency barrier with a usable read floor" although no read is floored anymore.

Go tolerates unused parameters, so this compiles, but it violates AGENTS.md's documentation-maintenance requirement and leaves names (minLogSeq, "floor") that falsely imply enforcement — a trap for the next editor. Fix: drop the four params, the floor argument, and writeMarker's sequence return (keep the marker write itself as the causal barrier), and reword the retained comments to describe default alignment.

4. [Low] Stale "two reasons" comment in the error-classification layer

tests/antithesis/workload/internal/client.go:483: the isBusinessError doc comment still says "FailedPrecondition minus the two reasons that IsTransient already covers", but this diff removes IsReadIndexNotCaughtUp, leaving only ExternalServiceError (see IsTransient, client.go:453-473). One-word fix.

Spec

No spec document was available to this verification pass; the only spec evidence is the PR-body line quoted by the Codex pass ("Migrates Antithesis assertions from the client-selected minimum log sequence to default linearizable reads backed by certified projection horizons"). Against that stated migration, one confirmed finding:

1. [Medium] Migrated retry comment still describes semantics the code does not implement

tests/antithesis/workload/bin/cmds/main/eventually_cross_node_identity/main.go:153-158 (hunk touched by this diff): waitForQuiescence's comment claims Raft transients "can surface as DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable." The code retries only internal.IsTransient(err), which deliberately excludes both Aborted and Canceled (tests/antithesis/workload/internal/client.go:453-473: "NOT in IsTransient: Aborted … Canceled"); either error reaches assert.Unreachable and aborts the quiescence window instead of retrying. The migration replaced the obsolete READ_INDEX_NOT_CAUGHT_UP description with another behaviorally false description, contradicting the PR's stated goal of moving the assertions onto accurate certified-horizon semantics. Fix the comment to name only Unavailable | DeadlineExceeded | ExternalServiceError; do not widen the retry policy, which would exceed the migration's scope.

---

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1892 (comment)

[shipfox-ai]

This PR migrates the Antithesis workload's list/audit/log assertions off the client-selected MinLogSequence floor onto default linearizable reads backed by certified projection horizons, and adds a new parallel_driver_projection_alignment driver for prepared queries. I verified the server-side premise independently: RoutedController.ExecutePreparedQuery performs ReadIndexAndWait and threads the barrier horizon (internal/bootstrap/controller_routed.go:354-361), filtered queries are owed alignment (internal/query/aligned_snapshot.go:62-64), and READ_INDEX_NOT_CAUGHT_UP is only emitted when an explicit MinLogSequence > 0 is not yet covered (internal/query/executor.go:151-160) — so removing it from IsTransient is safe for default reads, and the retained parallel_driver_minlogseq driver (which classifies the FailedPrecondition refusal explicitly) remains consistent. No production code is changed. The findings below are workload-hygiene issues; recommendation: approve with comments.

Standards

1. Hard — swallowed probe Apply error in the new driver. tests/antithesis/workload/bin/cmds/main/parallel_driver_projection_alignment/main.go:88-90 handles the probe-transaction Apply error with a bare return, with no classification and no assertion. This contradicts tests/antithesis/README.md ("What not to do": never swallow an error — classify it or leave a visible trace) and the driver's own package doc ("permanent prepared-query errors are reported as unreachable"): a permanent business error on this RPC silently ends the iteration, the assert.Always(found, ...) property is never evaluated, and the new driver reports "all green" on a real bug. Note the same driver already classifies the CreatePreparedQuery (:60-69) and ExecutePreparedQuery (:103-110) errors correctly — this one call site is the outlier. Emit Unreachable for non-tolerated errors before returning.

2. Hard — dead minLogSeq plumbing and now-false floor comments left in four drivers. The diff deletes every Read: &commonpb.ReadOptions{MinLogSequence: …} but retains the minLogSeq uint64 parameters, their threading, and stale floor language, breaching AGENTS.md's "Interface/behavior change: update relevant code comments":

   • parallel_driver_reference_race/main.go — countTransactionsWithReference takes an ignored minLogSeq (:113) while its doc still claims the read runs "at a fixed causal horizon" (:106-107); the caller computes floor via writeMarker only to feed the dead parameter. writeMarker's doc (:70-74) also carries a broken mid-sentence wrap ("…could correspond to. Returns / (0, false)…") left by the edit.
   • parallel_driver_bulk_atomicity/main.go — listIsEmpty ignores minLogSeq (:76); callers still extract and thread it (:220, :226, :233), and the comment at :189 still calls the marker sequence "a valid floor" for these reads.
   • parallel_driver_definitive_errors/main.go — referenceFilterCheck ignores minLogSeq (:62); :220/:223 compute and pass it with no remaining use; the comment at :184 still opens "Consistency barrier with a usable read floor".
   • parallel_driver_ledger_recreate/main.go — listMatches ignores minLogSeq (:81); :178-179/:184/:191 compute and pass it (details-only); the comment at :172 still calls the marker sequence "the freshness floor for every isolation read below" — false post-change.
     Remove the dead parameters/extractions or reword the comments to describe marker-only sequencing.

3. Moderate — retry comment contradicts the documented classification contract. eventually_cross_node_identity/main.go:153-157 was rewritten to say Raft transients surface as "DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable", but the retry at :165 uses IsTransient, which deliberately excludes both Aborted (surfaced loud per README) and Canceled (a local lifecycle event, per README not retry-safe). Following this comment would broaden the retry set against the documented design. Rewrite it to name only the classifications the code actually retries.

4. Minor — stale residue in the IsTransient doc. tests/antithesis/workload/internal/client.go:454-455 still says a retried code "clears (no leader → elected, lagging read → caught up)", but this diff removed the only member matching "lagging read → caught up" (READ_INDEX_NOT_CAUGHT_UP); no remaining member fits. The bullet list below it was updated correctly — this prose sentence was missed.

5. Judgement call — passive coverage sonde. parallel_driver_projection_alignment/main.go:111 emits assert.Reachable("projection-aligned prepared query succeeded", …) with no upstream Sometimes that fires on successful execution. Per README, such a Reachable is passive and Antithesis cannot bias toward the success path; consider a Sometimes sonde so the fuzzer steers toward the property's core path.

6. Judgement call — naming references the removed mechanism. parallel_driver_projection_alignment/main.go:27 and :52 use probeAccount = "minseq-probe:main" and the hardcoded prefix "minseq-probe:", referencing the min-seq mechanism this driver explicitly does not use; the names mislead future readers (they mirror the legacy parallel_driver_minlogseq driver). A rename to e.g. projection-probe: would be cheap now that the driver is new.

Spec

No confirmed findings. The diff matches the stated intent: all default-path list/audit/log assertions no longer populate ReadOptions.MinLogSequence; the new driver observes the prepared-query property without a client-selected floor; parallel_driver_stale_reads is untouched and still sends x-consistency: stale (unchanged behavior, as intended); the legacy parallel_driver_minlogseq driver is retained as coverage of the still-honored field, which does not contradict the migration since the server still honors explicit MinLogSequence (with the documented fail-fast refusal).

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1892 (comment)

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1892 (comment)

[NumaryBot]

NumaryBot review complete: no remaining inline findings.

Resolved 1 stale NumaryBot review thread (0 fixed, 1 outdated).

Summary: #1892 (comment)

[shipfox-ai]

This PR migrates the Antithesis drivers from client-selected ReadOptions.MinLogSequence floors to default linearizable reads backed by certified projection horizons, adds a dedicated parallel_driver_projection_alignment driver (with its projection owned-ledger prefix correctly registered), and removes the now-unreachable IsReadIndexNotCaughtUp member from the workload's transient-retry set. I verified the migration end to end: zero ReadOptions usage remains under tests/antithesis, the default-read path is real (RoutedController.readCtrl → ReadIndexAndWait, internal/bootstrap/controller_routed.go:79, applied on every list/read RPC), and the untouched parallel_driver_stale_reads scenario is unchanged. The one candidate P2 raised during review — loss of list-RPC floor regression coverage — was checked and cleared: internal/adapter/grpc/server_bucket_audit_consistency_test.go retains deterministic Read-floor cases (:178, :238, :270, :317, :354, :401; also client_bucket_test.go:390), and removing the floor from chaos drivers is the PR's stated purpose. What remains are comment-accuracy and dead-interface issues; nothing blocks merge. Recommendation: approve with comments.

Standards

P3 — Dead minLogSeq interface and stale floor vocabulary left behind by the migration

After deleting Read: &commonpb.ReadOptions{MinLogSequence: ...}, four list helpers still accept a minLogSeq uint64 parameter that is unused in their bodies, while callers still extract and thread the value:

• parallel_driver_bulk_atomicity/main.go:76 (listIsEmpty), callers :220→:226, :233
• parallel_driver_definitive_errors/main.go:62 (referenceFilterCheck), caller :220→:223
• parallel_driver_ledger_recreate/main.go:81 (listMatches), callers :178→:184, :191
• parallel_driver_reference_race/main.go:113 (countTransactionsWithReference), caller :153

Related stale vocabulary from the same migration:

• parallel_driver_reference_race/main.go:106-107 — countTransactionsWithReference doc still says matching happens "at a fixed causal horizon", though no horizon exists in the call anymore.
• parallel_driver_reference_race/main.go:70-74 — the rewritten writeMarker doc has a broken mid-sentence wrap ("could correspond to. Returns / (0, false)").
• parallel_driver_projection_alignment/main.go:27,54 — the new driver names its probe account minseq-probe:main and filters on the minseq-probe: prefix, reusing the vocabulary of the mechanism this PR removes; projection-probe would match the driver's actual property.

Impact: the dead parameters imply a freshness floor is still being supplied on reads that now rely on the default linearizable path, misleading future maintenance (AGENTS.md: "Interface/behavior change: update relevant code comments"). Resolution: drop the unused parameters (keep the details["minLogSeq"] diagnostics if wanted) and rewrite the residual floor/horizon wording.

P3 — Comments contradict the classifier after removing IsReadIndexNotCaughtUp

• eventually_cross_node_identity/main.go:153-157 — the rewritten comment claims Barrier retries handle "DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable", but the retry at :163 uses internal.IsTransient, which deliberately excludes Aborted (surfaced loud as a triage signal per tests/antithesis/README.md:81-84) and Canceled (local lifecycle). The actual set is Unavailable | DeadlineExceeded | ExternalServiceError. As written, the comment invites broadening the retry set with Aborted, which the README designates as a finding.
• internal/client.go:454-455 — the IsTransient doc still says the condition clears "(no leader → elected, lagging read → caught up)"; the "lagging read" clause refers to the removed READ_INDEX_NOT_CAUGHT_UP member.
• internal/client.go:485 — isBusinessError doc still says "minus the two reasons that IsTransient already covers"; only ExternalServiceError remains.
• parallel_driver_minlogseq/main.go:3-4 — the header still claims the list RPCs "are already covered by the chore: Update swagger with deprecated API response fields #398-era drivers"; this diff removed all list-RPC floor usage from those drivers, making the claim false (the driver's own prepared-query coverage is unaffected).

All four breach AGENTS.md's comment-accuracy rule and risk a future retry-policy regression. Resolution: describe only the actual IsTransient set, the single remaining business-error exception, and the new split of coverage (list RPCs on default reads, client floor covered by parallel_driver_minlogseq on the prepared-query path).

P3 — New driver's success Reachable has no upstream Sometimes sonde

parallel_driver_projection_alignment/main.go:118 emits assert.Reachable("projection-aligned prepared query succeeded", details) with no assert.Sometimes anywhere in the driver. tests/antithesis/README.md:156-158 documents that an unpaired Reachable is passive — Antithesis cannot bias toward making it fire. Since this driver exists specifically to drive the prepared-query alignment path, add a Sometimes (e.g. assert.Sometimes(err == nil || internal.IsTolerated(err), ...) around the ExecutePreparedQuery outcome) so the fuzzer gets a coverage signal for the new property.

Spec

No confirmed material finding.

Verification performed against HEAD: every migrated list/audit/log driver now sends no Read option and relies on the default linearizable path, which is real rather than deletion-only (readCtrl → ReadIndexAndWait gates each list/read RPC, with the barrier horizon bound to the request context). The still-explicit MinLogSequence usage is confined to the untouched parallel_driver_minlogseq contract test on the prepared-query path — a deliberate guard for the still-supported client-selected floor feature, not an unmigrated assertion; its only residue is the stale header comment flagged under Standards. parallel_driver_stale_reads/main.go:73 is absent from the diff and still issues x-consistency: stale reads, so "the stale fault scenario remains unchanged" holds, and no fault-scenario configuration is touched.

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.

[shipfox-ai]

This PR migrates the Antithesis drivers from client-supplied MinLogSequence floors to default linearizable reads and adds a new projection-alignment driver. The migration mechanics (driver registration, IsTolerated skip pattern, README taxonomy) are correct, but the new projection-alignment driver never actually exercises projection alignment — its query shape is served entirely by the main store — and the floor removal leaves stale comments, dead plumbing, and removes the only chaos-level coverage of the still-honored MinLogSequence list-RPC wait path. Recommendation: request changes.

Spec

[P2][blocking] The new projection-alignment driver never consults the projection

Location: tests/antithesis/workload/bin/cmds/main/parallel_driver_projection_alignment/main.go:43-55,105-138

The driver creates an ACCOUNTS prepared query whose only leaf is AddressMatch_HardcodedPrefix (:43-55), then asserts the acknowledged account is returned (:105-138). But internal/query/aligned_snapshot.go:101-102 classifies account-address predicates as main-store-only (case *commonpb.QueryFilter_Address: return target == commonpb.QueryTarget_QUERY_TARGET_TRANSACTIONS — false for ACCOUNTS), so AlignmentOwed (aligned_snapshot.go:64-69) is false and internal/query/executor.go:128-134 skips AlignedIndexSnapshot entirely. The driver's own header (:1-4) claims "the query waits only for the read projection to certify that same horizon" — that is not true for this query shape.

Consequence: the driver only proves the default main-store read-after-write path. A stalled, stale, or uncertified read projection — the exact behavior this driver was added for under the EN-1946 framing — cannot produce a finding here. Per docs/technical/contributing/testing.md ("Regression preservation and branch proof": a regression test is a guard for a specific production path, not an example that reaches nearby code), this is a blocking gap.

Fix: use a prepared-query leaf that actually requires the read index (e.g. AccountHasAsset, or a TRANSACTIONS-target address predicate) with the acknowledged write creating the indexed membership, so success necessarily traverses AlignedIndexSnapshot and its projection certificate.

No other spec deviations were found; the stale-read fault scenario is unchanged, and the server still emits READ_INDEX_NOT_CAUGHT_UP (internal/adapter/grpc/server.go:649-661), so the unchanged parallel_driver_minlogseq driver remains valid.

Standards

1. Regression-preservation gap: chaos-level coverage of the live MinLogSequence list-RPC wait path was removed

The diff strips Read: &commonpb.ReadOptions{MinLogSequence: …} from every list-RPC driver (parallel_driver_audit/main.go:46-51, parallel_driver_bulk_atomicity/main.go:83,248, parallel_driver_definitive_errors/main.go:69, parallel_driver_ledger_recreate/main.go:88, parallel_driver_list_completeness/main.go:129-131, parallel_driver_list_transactions/main.go:44-49,62, parallel_driver_logs/main.go:46-54, parallel_driver_reference_race/main.go:120, parallel_driver_timestamp_order/main.go:148). The server still honors that input: internal/adapter/grpc/server_bucket.go:482 (waitMinLogSequence) is called on the list paths at :597, :650, :699, :762, :1004, :1060, :1135, :1387, :1436 and :1480. parallel_driver_minlogseq covers only the prepared-query path — its own header (main.go:2-3) says the list RPCs "are already covered by the #398-era drivers", i.e. exactly the drivers this PR de-floored (that comment is now stale too). Unit tests (internal/adapter/grpc/client_bucket_test.go:390, server_bucket_audit_consistency_test.go) mitigate but do not restore end-to-end chaos coverage. This violates AGENTS.md:78 / testing.md:26-29 ("adding coverage … must not replace the existing regression trigger"). Keep at least one list-RPC driver exercising the floor.

2. Rewritten retry comment misstates the retry contract

tests/antithesis/workload/bin/cmds/main/eventually_cross_node_identity/main.go:153-159 — rewritten by this diff — claims Raft transients "can surface as DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable". The loop below retries only internal.IsTransient(err) (client.go:467-471), which deliberately excludes Aborted (client.go:310-313; README: "deliberately NOT in IsTransient … a finding worth triaging") and Canceled (client.go:323-326: "Not retry-safe"). An Aborted surfacing here would trip the Unreachable assertion, not retry. The comment was already partially wrong before this PR, but the rewrite added the false "deadline/cancellation" claim — correct it to describe the set IsTransient actually accepts.

3. Stale, now-false comments left by the incomplete migration (AGENTS.md:89)

• tests/antithesis/workload/internal/client.go:452-455 — the IsTransient doc still says retrying clears "(no leader → elected, lagging read → caught up)" although this diff removed IsReadIndexNotCaughtUp from the set (:467-471); sibling comments in the same file were updated, this one was not.
• parallel_driver_bulk_atomicity/main.go:188-192 — "the marker's sequence is a valid floor for reads on the primary ledger": reads no longer carry a floor; the sequence is now only diagnostic metadata (:220-221).
• parallel_driver_ledger_recreate/main.go:171-172 — "its global log sequence is the freshness floor for every isolation read below": no longer true; the reads below are default linearizable reads.

4. Dead floor plumbing obscures the actual freshness mechanism

The floor removal left unused minLogSeq uint64 parameters that callers still extract sequences solely to thread: parallel_driver_bulk_atomicity/main.go:76 (fed at :226), parallel_driver_ledger_recreate/main.go:81 (fed at :184), parallel_driver_definitive_errors/main.go:62 (fed at :223), parallel_driver_reference_race/main.go:113 (fed via writeMarker's return at :153). In a family of drivers whose entire soundness argument is read-after-write, dead "floor" parameters wrongly suggest a numeric freshness contract that no longer exists — the subsequent default linearizable read is the actual barrier. Remove the parameters and return plumbing; keep the sequence only where it is genuinely used as diagnostic data (e.g. bulk_atomicity's details["minLogSeq"]).

5. The new driver's success path has no coverage sonde

parallel_driver_projection_alignment/main.go:118 adds a success-only assert.Reachable("projection-aligned prepared query succeeded") with no upstream Sometimes that fires on query success — the driver contains no Sometimes at all. tests/antithesis/README.md ("Antithesis SDK usage") warns that such a Reachable is passive and recommends pairing on fragile paths; since the aligned read is the driver's whole purpose, Antithesis cannot bias executions toward it. (Fix together with the Spec finding.)

---

Both reports were verified line-by-line against diff.patch and the checkout: the projection-driver bypass, the still-honored server-side waitMinLogSequence, the retry-comment contradiction, and the stale comments are all confirmed in code. GLM's remaining baseline smells (duplicated marker-write shape, GetLogs()[len-1] message chains, the minseq-probe account name in the new driver) and its reference_race comment nitpicks were rejected as style with no material impact.

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.

[shipfox-ai]

This PR removes every client-side MinLogSequence freshness floor from the Antithesis drivers in favor of default linearizable reads, deletes the now-unneeded IsReadIndexNotCaughtUp classifier, and adds a new parallel_driver_projection_alignment driver. The core migration is sound: I verified that the dropped floors are genuinely compensated server-side (the routed-read ReadIndex barrier plus query.AlignmentOwed/AlignedIndexSnapshot in internal/query/aligned_snapshot.go and internal/query/executor.go:128-135), and that the classifier removal is safe because READ_INDEX_NOT_CAUGHT_UP now only surfaces on explicit MinLogSequence requests, which the untouched parallel_driver_minlogseq driver still validates directly. The stale-read fault scenario is unchanged. However, the new dedicated driver does not exercise the certified-projection mechanism it is named for — its query shape never acquires alignment — and the diff leaves dead minLogSeq plumbing and now-false comments behind. Recommendation: request changes (one blocking test-effectiveness issue on the PR's centerpiece driver; the rest is mechanical cleanup).

Standards

1. [Blocking] The new projection-alignment driver tests only the ReadIndex barrier, not projection alignment.
tests/antithesis/workload/bin/cmds/main/parallel_driver_projection_alignment/main.go:1-11 (doc comment) and :47-53 (filter). The driver's header claims "the query waits only for the read projection to certify that same horizon," but its prepared query targets ACCOUNTS with an AddressMatch_HardcodedPrefix condition. Account-address predicates are served entirely by the main store: filterUsesReadIndex returns false for QueryFilter_Address unless the target is TRANSACTIONS (internal/query/aligned_snapshot.go:97, see the AlignmentOwed doc at :51-63), so query.Execute skips AlignedIndexSnapshot entirely (internal/query/executor.go:128-135). The driver therefore exercises the linearizable ReadIndex barrier only; it cannot fail if prepared-query projection alignment is removed or broken, and its documentation asserts a property the query shape cannot reach. Resolution: use a filter leaf whose evaluation consumes the read projection (e.g. a Reference condition on TRANSACTIONS, or a metadata/has-asset condition on ACCOUNTS), arrange for the probe write to create the matching index row, and reword the header to match what is actually exercised.

2. [Non-blocking] Dead minLogSeq plumbing left behind in four drivers.
The diff removed the only reader of minLogSeq (the ReadOptions floor) but kept the parameter and the extraction/threading code, violating AGENTS.md's "Interface/behavior change: update relevant code comments" (AGENTS.md:89):

• parallel_driver_reference_race/main.go:113 — countTransactionsWithReference accepts minLogSeq uint64 and never reads it; caller :165-167 still obtains the marker floor solely to feed it.
• parallel_driver_bulk_atomicity/main.go:76 — listIsEmpty param unused; caller still extracts it at :220, stores it in details["minLogSeq"] at :221, and threads it at :226 and :233.
• parallel_driver_ledger_recreate/main.go:81 — listMatches param unused; extraction at :178, details["minLogSeq"] at :179, threaded at :184 and :191.
• parallel_driver_definitive_errors/main.go:62 — referenceFilterCheck param unused; the computation at :220 and threading at :223 are dead.
  Delete the params, extractions, and stale minLogSeq details keys, and reword the affected comments to describe the default linearizable read (the marker now serves only to establish write ordering; the existing "fixed causal horizon" wording in reference_race is accurate and can stay).

3. [Non-blocking] Rewritten comment in eventually_cross_node_identity contradicts the documented retry contract.
tests/antithesis/workload/bin/cmds/main/eventually_cross_node_identity/main.go:152-156: the sentence this diff rewrote claims Barrier transients "can surface as DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable." The retry at :166 uses IsTransient, which deliberately excludes Aborted (surfaced loud via assert.Unreachable at :169) and Canceled (local lifecycle; see the NOT in IsTransient list in workload/internal/client.go). The comment invites a future editor to broaden the retry set against the documented design. Reword to name only the codes IsTransient actually covers.

4. [Non-blocking] IsTransient doc residue contradicts the new error set.
tests/antithesis/workload/internal/client.go:454-455: "the condition clears (no leader → elected, lagging read → caught up)" — this diff removed the only member matching "lagging read" (IsReadIndexNotCaughtUp), and the sibling retryUnaryInterceptor comment was updated, but this clause was not. Drop the "lagging read → caught up" clause.

Spec

1. [Blocking] The dedicated replacement probe does not consume a certified projection.
Spec: "Migrates Antithesis assertions from the client-selected minimum log sequence to default linearizable reads backed by certified projection horizons." The new parallel_driver_projection_alignment driver is the PR's dedicated assertion for the replacement property, but as detailed in Standards #1, its ACCOUNTS + address-prefix query is answered entirely from the main store — AlignmentOwed is false and no projection wait or certificate ever occurs. It would remain green if the "certified projection horizon" half of the migration regressed for prepared queries. Scope note: the migration as a whole is not missing this coverage — the migrated drivers whose filters hit the read index (reference_race, bulk_atomicity, ledger_recreate, definitive_errors via Reference/Address filters on TRANSACTIONS; audit and logs via the LOGS target) do acquire alignment — but the one driver named after the property does not verify it. Fix by switching the probe to an indexed leaf as described in Standards #1. Everything else in the spec holds: no ReadOptions{MinLogSequence} usage remains in the migrated drivers, parallel_driver_minlogseq legitimately retains explicit-floor contract coverage, and the stale fault scenario is untouched.

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.

[shipfox-ai]

Final review — PR #1892 (formancehq/ledger)

This PR migrates the Antithesis assertion drivers from client-selected MinLogSequence read floors to default linearizable reads backed by certified projection horizons, adds a dedicated parallel_driver_projection_alignment driver for the new property, and removes READ_INDEX_NOT_CAUGHT_UP from the workload's transient-error retry set. I verified both candidate reports against the diff and the actual code. The behavioral core of the migration is correct: every driver that previously sent ReadOptions.MinLogSequence now sends none; the default read path really does wait for alignment (bootstrap/controller_routed.go:79 performs ReadIndexAndWait, and the default controller resolves through query.AlignedIndexSnapshot, which blocks until the projection certificate covers the fixed main-store horizon); the only remaining constructor of READ_INDEX_NOT_CAUGHT_UP is internal/query/executor.go:156-160, gated on req.GetMinLogSequence() > 0, which no migrated read sends; and parallel_driver_stale_reads is untouched, so the stale fault scenario is unchanged. What remains are stale/contradictory comments and dead parameter plumbing left behind by the migration — no behavioral defect was confirmed. Recommendation: approve with comments.

Standards

[P2] Rewritten retry-contract comments contradict the documented classification they describe

• tests/antithesis/workload/bin/cmds/main/eventually_cross_node_identity/main.go:151-158 — the comment (rewritten in this PR) claims Barrier retries cover "DeadlineExceeded, Aborted, or deadline/cancellation errors — all legitimately retryable". The loop retries only internal.IsTransient (Unavailable | DeadlineExceeded | ExternalServiceError). tests/antithesis/README.md states Aborted is deliberately NOT in IsTransient ("if it shows up under chaos, that is a finding worth triaging"), and Canceled is a local lifecycle event that ends the run, not a retried transient. The comment now instructs maintainers in exactly the direction the classification deliberately avoids.
• tests/antithesis/workload/internal/client.go:452-454 — the IsTransient doc still lists "lagging read → caught up" as a condition that clears on retry, but the READ_INDEX_NOT_CAUGHT_UP member was removed from the set in this same PR.
• tests/antithesis/workload/internal/client.go:488-489 — the isBusinessError doc still says "minus the two reasons that IsTransient already covers"; only ExternalServiceError remains.

Impact: a maintainer reading these comments could broaden retry tolerance (e.g. add Aborted) "as documented", masking chaos findings the classification intentionally surfaces. Update the comments to name exactly the three surviving members.

[P3] Dead minLogSeq parameters and residual floor language left behind by the migration

Four list helpers retained the minLogSeq uint64 parameter after its ReadOptions.MinLogSequence consumer was removed, and callers still extract and pass marker sequences into them:

• parallel_driver_bulk_atomicity/main.go:76 (listIsEmpty; callers at 220, 226, 233)
• parallel_driver_ledger_recreate/main.go:81 (listMatches; callers at 178, 184, 191)
• parallel_driver_definitive_errors/main.go:62 (referenceFilterCheck; callers at 220, 223)
• parallel_driver_reference_race/main.go:113 (countTransactionsWithReference; caller at 158)

Residual stale wording also survives: parallel_driver_reference_race/main.go:106-107 still describes the read as "at a fixed causal horizon", and parallel_driver_definitive_errors/main.go:184 still opens with "Consistency barrier with a usable read floor" — neither floor exists anymore; the reads are plain default linearizable reads. This is dead code plus comments that falsely imply the marker sequence still controls the RPC (contrary to the comment-maintenance rule). Remove the parameters, the now-unneeded sequence extraction (keeping it only where it feeds assertion details), and reword the comments.

[P3] New driver has no Sometimes sonde upstream of its Reachable

parallel_driver_projection_alignment/main.go:116-127 — the only Sometimes-free success path in the new driver: it returns early on tolerated errors and then calls assert.Reachable("projection-aligned prepared query succeeded", …). Per tests/antithesis/README.md ("A Reachable with no upstream Sometimes … is passive; prefer pairing them when the path is fragile"), Antithesis cannot bias exploration toward executing this driver's core property — the exact fragile path the driver exists to exercise. Every neighbouring driver pairs its Reachable with a Sometimes. Add a globally-unique Sometimes(internal.IsTolerated(err), …)-style sonde at the prepared-query execution step. Judgement call (the rule says "prefer"), hence P3.

Spec

No confirmed material finding. Both candidate Spec concerns were verified against the code and do not hold:

• READ_INDEX_NOT_CAUGHT_UP retry removal is not a latent false-finding risk. The error's only construction site is internal/query/executor.go:156-160, gated on req.GetMinLogSequence() > 0; no migrated read sends any floor (waitMinLogSequence is a no-op at 0, adapter/grpc/server_bucket.go:482-485), and the sole workload caller still sending an explicit floor is parallel_driver_minlogseq, which triggers the refusal deliberately and — per its own header (main.go:25-26, "FailedPrecondition is not retried by any client retry layer") — wants the refusal surfaced un-retried. Removing it from IsTransient actually aligns the client with that driver's documented assumption; the error remains classified (as a FailedPrecondition business code) so the classify interceptor stays complete.
• Preserving parallel_driver_minlogseq is intentional, not an incomplete migration. It probes the separately supported explicit min_log_sequence contract (honored / refused-with-READ_INDEX_NOT_CAUGHT_UP), which the read-path documentation keeps during the staged rollout.
• "The stale fault scenario remains unchanged" is satisfied. parallel_driver_stale_reads/main.go is absent from the diff and still pins x-consistency: stale; the removed retry classification cannot affect that path since it sends no minimum sequence.

The new PrefixProjectionAlign = "projection" prefix is properly registered in ownedLedgerPrefixes and does not hyphen-collide with any existing prefix.

Reviewed independently by GLM (glm-5.3-flash) and Codex (gpt-5.6-sol) via Shipfox; verified and synthesized by GLM.