feat(reads): align live queries to Raft horizons (EN-1946)

[gfyrag]

Summary

• consume the preceding projection certificates at a fixed main-store Raft horizon;
• align only the read/audit projections actually used by each query;
• propagate the local ReadIndex horizon into controller execution;
• distinguish disabled audit projection (permanent FailedPrecondition) from rebuilding (retryable Unavailable);
• preserve main-horizon trimming and snapshot lifecycle guarantees.

Stack

EN-1946 stack 3/8. Depends on #1889 (feat/en-1946-projection-certificates).

Merge/review order: #1897 → #1889 → #1890 → #1894 → #1891 → #1893 → #1892 → #1881.

Validation

Final base: 01f5df3.
Final head: de64081.
Canonical PR validation: PASS, including full race validation, business/cluster E2E, and Schemathesis 62/62.
Independent exact diff review: APPROVE (residual risk MEDIUM).

Jira: EN-1946. Do not merge automatically.

[NumaryBot]

✅ Approve — automated review

No actionable correctness defects remain in the current diff. Previously raised documentation and projection-selectivity concerns have been addressed at HEAD.

No findings.

[codecov]

Codecov Report

❌ Patch coverage is 91.08280% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.20%. Comparing base (01f5df3) to head (de64081).

Files with missing lines	Patch %	Lines
internal/application/ctrl/controller_default.go	84.84%	10 Missing ⚠️
internal/query/aligned_snapshot.go	94.64%	3 Missing ⚠️
internal/bootstrap/controller_routed.go	95.65%	1 Missing ⚠️

Additional details and impacted files

@@                           Coverage Diff                            @@
##           feat/en-1946-projection-certificates    #1890      +/-   ##
========================================================================
- Coverage                                 83.43%   83.20%   -0.24%     
========================================================================
  Files                                       459      459              
  Lines                                     42645    42757     +112     
========================================================================
- Hits                                      35581    35574       -7     
- Misses                                     7059     7178     +119     
  Partials                                      5        5              

Flag	Coverage Δ
e2e	83.20% <91.08%> (-0.24%)	⬇️
scenario	83.20% <91.08%> (-0.24%)	⬇️
unit	83.20% <91.08%> (-0.24%)	⬇️

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: #1890 (comment)

[NumaryBot]

NumaryBot review complete: no remaining inline findings.

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

Summary: #1890 (comment)

[shipfox-ai]

This PR (EN-1946) reworks cross-projection read alignment so that filtered reads wait for each projection's Raft certificate at a fixed main-store applied-index horizon H instead of the moving fold cursor, wires the ReadIndex horizon R through the routed controller, extends the same treatment to the audit projection in ListAuditEntriesFrom, and rewrites the affected architecture docs and tests. The core mechanism is sound — the certification/trim logic is coherent, lifecycle handling in ListAuditEntriesFrom (the closeHandle flag and joinedCloser) is correct, and the new tests pin the important invariants. However, the PR's own documentation is materially false in two places (it documents minLogSequence away from the prepared-query surface while the field remains live and enforced, and it claims reads "wait only for projections the query actually uses" while main-store-only transaction filters still wait), and a stale comment now contradicts the implemented checkpoint behavior. Recommendation: request changes — the blocking items are documentation/comment corrections, not mechanism rework.

Standards

[P2][blocking] Canonical docs remove a still-supported prepared-query field

docs/technical/architecture/subsystems/read-path/query-filter.md:52 and :67-71 drop minLogSequence from the prepared-query execute row and state the execute body carries "only parameters, pagination and mode". This contradicts the code in the same tree: misc/proto/bucket.proto:1117 still defines min_log_sequence on ExecutePreparedQueryRequest, internal/adapter/http/handlers_execute_prepared_query.go:37-42 still decodes it and :87-94 still forwards it, and internal/query/executor.go:154 still enforces it. The same PR's query-pipeline.md explicitly says "The legacy min_log_sequence request field is still honored as an additional client-selected native-sequence floor", so the repository's two authoritative read-path documents now disagree, and the canonical API-surface table is false. This breaches AGENTS.md's documentation-maintenance rule. Restore minLogSequence to the execute row and note, or actually remove the field in a separately scoped API change.

[P2][blocking] Stale checkpoint comment contradicts the implemented behavior

internal/adapter/grpc/server_bucket.go:982-991 still claims CreateQueryCheckpoint waits for the log-index builder "but NOT for the separate async audit indexer", that "a filtered checkpoint read can omit audit entries that do exist", and that the fix "is out of scope here". All three claims are now false: checkpoint creation waits for the audit projection's certificate before exposing the checkpoint (internal/application/indexbuilder/process_logs.go:320-324), and a filtered checkpoint read whose audit snapshot is behind the main horizon fails explicitly with ErrIndexBuilding("audit checkpoint") instead of omitting entries (internal/application/ctrl/controller_default.go:1826-1828). The comment describes a failure mode this PR eliminated and points maintainers at a "follow-up" that no longer exists. Update it to describe the current wait-and-fail contract. (Note: the companion comment at controller_default.go:1759-1763 about the audit query surface was checked and remains accurate — indexed filters still resolve through the audit index, seq-only/nil filters through the sequence bound — so no change is required there.)

[P3][non-blocking] Intentional error discards lack the required justification comments

New bare _ = discards at internal/application/ctrl/controller_default.go:1783, 1816, 1825, 1838, 1852 carry no justification comment. AGENTS.md ("use _ = ... with a justification comment when intentional") and docs/technical/contributing/conventions.md:62 (the documented "Good" pattern is _ = file.Close() // Best effort cleanup) both require one. These are cleanup paths, and the file's pre-existing style is similar, but the documented rule's letter asks for a comment — add "best-effort cleanup" markers or propagate the error where meaningful.

[P3][non-blocking] Duplicated "read H, assert ≥ R" horizon logic

internal/application/ctrl/controller_default.go:1786-1792 re-implements mainAppliedHorizon from internal/query/aligned_snapshot.go:32-48 verbatim — ReadLastAppliedIndex → compare to ReadBarrierHorizon → identical error string "main-store snapshot applied index %d is behind ReadIndex horizon %d". Both copies were added in this PR and must stay in sync for linearizability; drift would silently weaken the barrier check on one path. Extract the shared helper (the audit path can call mainAppliedHorizon(ctx, handle) directly). This violates AGENTS.md's "Prefer existing repository patterns and DRY solutions" rule.

[P3][non-blocking] OpenReservedQueryHandle is dead abstraction as committed

internal/query/aligned_snapshot.go:266-273 (plus the reserve parameter threaded through openQueryHandle at :275-277) has no caller and no test anywhere in the tree; only the definition exists. Its comment defers to a future consumer "whose need ... cannot be described by an API query filter". As committed this is speculative generality with subtle, untested lease-reservation semantics on an exported API. Either land it with its consumer in this stack, or defer it to that PR.

Rejected during verification: the e2e rewrite in tests/e2e/business/cross_store_snapshot_alignment_test.go is not a regression-trigger replacement — the production trigger itself changed (rejection-then-aligned became bounded waiting), so testing the old FailedPrecondition path would assert removed behavior, and the new form still pins "a successful response must already be aligned"; the per-method delegation repetition in controller_routed.go matches that file's established style; the state := "disabled"/"rebuilding" string construction is a nitpick.

Spec

[P2] "Waits only for projections the query actually uses" does not hold for main-store-only transaction filters

The PR's documentation (docs/technical/architecture/subsystems/read-path/query-pipeline.md, "It waits only for projections the query actually uses"; same promise in read-snapshot-consistency.md and the EN-1946 alignment contract) is only true for the audit path. AlignmentOwed (internal/query/aligned_snapshot.go:51-53) returns true for every non-nil filter, yet several transaction filter shapes read exclusively from the main store: the reverted condition uses the FSM-maintained reversion bitset (internal/query/compile.go:417-421 — "No index is required — the bitset is always maintained by the FSM") and the tx-id condition uses the Pebble history zone (compile.go:1254-1255 — "No index is required"). A query filtered only on reverted or tx-id therefore enters AlignedIndexSnapshot and blocks on WaitForRaftProgress for a read projection it never consults; under write pressure or with a stalled/broken projection, such a read is delayed or times out even though its data is fully present in the main snapshot. The same PR introduced exactly the correct, precise gate for audit (AuditFilterNeedsIndex in controller_default.go:1804 and server_bucket.go:997-1003), so the gap is demonstrably fixable with the pattern this PR itself established. Resolution: derive projection dependence from the filter tree (including boolean combinations, as auditFilterNeedsIndex already does) and skip projection alignment for main-store-only shapes — or correct the documentation to state that filtered account/transaction reads unconditionally align to the read projection.

No other Spec findings: no external spec was reachable (EN-1946 lives in an external tracker), so the Spec axis was evaluated against the behavior contract the PR itself documents.

---

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 #1890 ("feat(reads): align live queries to raft horizons")

This PR replaces sequence-cursor alignment of asynchronous projections with fixed Raft-horizon certificates (ReadIndex result R, main-snapshot applied index H, per-projection certificates ≥ H), adds explicit failure for disabled/rebuilding/lagging audit projections (including frozen checkpoint reads), threads the barrier horizon through the routed controller, and rewrites the read-path architecture docs plus tests. The core mechanism is coherent, well-commented, and covered by new unit and e2e tests. However, the PR's own documentation makes two claims the code contradicts: the canonical API-surface table drops the still-honored minLogSequence field, and the "waits only for projections the query actually uses" contract is not implemented for main-store-only transaction filters. A stale checkpoint-limitation comment also describes a failure mode this PR eliminates. Recommendation: request changes — the blocking items are documentation/contract corrections that should land before merge; no correctness defect was found in the new alignment logic itself.

Standards

[P2] Canonical API docs drop a field the live API still honors. docs/technical/architecture/subsystems/read-path/query-filter.md:52 removes minLogSequence from the prepared-query execute row and the note at lines 67–71 now says the execute body carries "only parameters, pagination (cursor, pageSize) and mode". The code disagrees: misc/proto/bucket.proto still defines min_log_sequence on ExecutePreparedQueryRequest, internal/adapter/http/handlers_execute_prepared_query.go:41 still decodes it, and internal/query/executor.go:151-160 still enforces it as a freshness floor. Worse, this same PR's query-pipeline.md:115-118 states the legacy field "is still honored as an additional client-selected native-sequence floor" — the two authoritative documents in one PR now contradict each other, and the API-surface table is false. This breaches AGENTS.md's documentation-maintenance rule (docs are part of the change when behavior/interfaces change). Restore the row/note, or remove the field in a separately scoped API change.

[P2] "It waits only for projections the query actually uses" is not what the code does. docs/technical/architecture/subsystems/read-path/query-pipeline.md:98 promises projection waits are gated by actual use, but AlignmentOwed (internal/query/aligned_snapshot.go:62-66) returns true for any non-nil filter. Valid transaction filters can be entirely main-store-only: compileRevertedCondition reads the FSM-maintained reversion bitset (internal/query/compile.go:417-427) and compileTxIDCondition reads the main Pebble history zone (internal/query/compile.go:1254-1268); neither consults the read index. ListTransactions (and prepared-query execution) with a txID-only or reverted-only filter therefore waits for the read-index projection's Raft certificate at H — a stalled or rebuilding projection delays or times out a read that never touches it, and a disabled read projection has no explicit failure path on this route at all. Derive projection dependence from the compiled filter tree (as the audit path already does with AuditFilterNeedsIndex), or amend the docs to state unconditional alignment for all filtered reads.

[P2] Stale checkpoint-limitation comment contradicts the new contract. internal/adapter/grpc/server_bucket.go:982-991 still says CreateQueryCheckpoint does NOT wait for the async audit indexer, that a filtered checkpoint read "can omit audit entries that do exist in the checkpoint's audit zone" and "never catches up", and that the fix "is out of scope here". All three are now false: the index builder waits for the audit Raft certificate before materializing a checkpoint (internal/application/indexbuilder/process_logs.go:308-324), and a filtered audit read against a frozen store whose audit projection lags fails explicitly with ErrIndexBuilding{"audit checkpoint"} (internal/application/ctrl/controller_default.go:1817-1827) instead of silently omitting entries. Rewrite the comment to describe the current wait-and-fail contract; as written it directs maintainers to "fix" a solved problem and distrust checkpoint reads.

[P3] New _ = error discards lack justification comments. internal/application/ctrl/controller_default.go:1783,1816,1825,1838,1852 introduce _ = handle.Close() / _ = auditSnap.Close() discards without comments. AGENTS.md:68 permits discards only "with a justification comment when intentional". Pre-existing style in this file is similar, so severity is softened — add best-effort-cleanup markers or propagate the errors.

[P3] Duplicated main-horizon/ReadIndex check. internal/application/ctrl/controller_default.go:1787-1792 re-implements mainAppliedHorizon (internal/query/aligned_snapshot.go:32-44) verbatim, down to the error string "main-store snapshot applied index %d is behind ReadIndex horizon %d". Both copies were added in this PR; future drift in one copy would silently weaken the linearizability check. AGENTS.md:70 prefers existing patterns and DRY solutions — the read handle satisfies dal.PebbleGetter, so the shared helper can be used directly.

[P3] Unused exported reservation API. OpenReservedQueryHandle (internal/query/aligned_snapshot.go:267-273, plus the reserve parameter on openQueryHandle) has no caller or test anywhere in the tree; its comment defers to a future consumer whose need "cannot be described by an API query filter". Land it together with that consumer, or defer it to that stack.

Spec

No spec available — no confirmed material finding. There is no issue reference in the commit message, no reachable linked tracker entry (EN-1946 lives in an external Linear instance), and the PR body is a one-line stack note, so no requirement-level specification could be verified against the diff. Candidate spec findings that referenced unverifiable external requirements were not retained; the verifiable code/doc contradictions they surfaced are reported under Standards above.

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 replaces fold-cursor-based cross-store alignment with a fixed-Raft-horizon scheme: a main-store snapshot's durable applied index H (verified against the ReadIndex result R) becomes the horizon each used projection must certify, the audit list path gains its own certified-snapshot alignment with explicit disabled/rebuilding failures, and the routed controller now propagates the linearizable horizon to local execution. The design is coherent and well tested at the unit level, but it lands with one materially false authoritative documentation change that must be fixed before merge, plus a handful of non-blocking polish items.

Recommendation: request changes (single small documentation fix; everything else is non-blocking).

Standards

[P2][blocking] query-filter.md no longer documents the still-supported minLogSequence field on prepared-query execute
Location: docs/technical/architecture/subsystems/read-path/query-filter.md:52 and :69-71
Evidence: the diff rewrites the execute row to parameters, mode and the note to "carries no filter — only parameters ... and mode", but the code still fully supports the field: internal/adapter/http/handlers_execute_prepared_query.go:41 decodes minLogSequence and :93 forwards it, internal/query/executor.go:154-156 enforces it, and the query-pipeline.md rewrite in this same PR states "The legacy min_log_sequence request field is still honored as an additional client-selected native-sequence floor".
Impact: authoritative API documentation tells clients a supported freshness control does not exist, and the two rewritten read-path docs contradict each other. Per docs/technical/contributing/ai-review.md (P2: "misleading authoritative docs"; blocking when authoritative guidance is made materially false), this must be fixed — either restore minLogSequence in the table and note, or remove the field from the API consistently.

[P3][non-blocking] New bare _ = discards lack the documented justification comment
Location: internal/application/ctrl/controller_default.go:1783,1816,1825,1838,1852 (_ = handle.Close() / _ = auditSnap.Close() in the new ListAuditEntriesFrom cleanup paths) and internal/query/aligned_snapshot_test.go:140
Evidence: AGENTS.md ("use _ = ... with a justification comment when intentional") and docs/technical/contributing/conventions.md:61-62 (_ = file.Close() // Best effort cleanup).
Impact: cleanup intent is not self-documenting. Low priority: the surrounding file already uses this bare pattern pervasively, so this is a polish item, not a regression.

[P3][non-blocking] Duplicated wrapper/alignment shapes invite drift
Locations: internal/bootstrap/controller_routed.go:252,261,279,329,338,392 and internal/application/ctrl/controller_default.go:1806-1830
Evidence: six routed methods repeat the identical c, barrier, err := b.readCtrl(ctx); ... c.X(b.withLocalBarrierHorizon(ctx, c, barrier), ...) shape; a future call site that forgets withLocalBarrierHorizon silently loses the linearizable horizon with no compile-time or test signal. Separately, ListAuditEntriesFrom's retry loop re-implements AlignedIndexSnapshot's open→read-certificate→close→wait→retry shape (internal/query/aligned_snapshot.go:108-165) with its own frozen-store and error semantics, so audit and projection alignment can drift apart.
Impact: maintenance/consistency risk only. A shared helper for each shape would make omission impossible.

[P3][non-blocking] OpenReservedQueryHandle is exported with zero callers
Location: internal/query/aligned_snapshot.go:267-273
Evidence: repository-wide search finds no call site beyond the declaration; it is speculative generality expanding the internal query API. Inline it back until a consumer (e.g. a later stack PR) actually needs it.

[P3][non-blocking] New "waits only for projections the query actually uses" claim is broader than the code
Location: docs/technical/architecture/subsystems/read-path/query-pipeline.md (Projection alignment section, step 3) vs internal/query/aligned_snapshot.go:61-63
Evidence: AlignmentOwed returns filter != nil for ACCOUNTS/TRANSACTIONS, so e.g. a reverted-only transaction filter — compiled purely from the FSM bitset and main-store universe (internal/query/compile.go:417-449, no read-index access) — still waits for the read projection's Raft certificate. A stalled read index can therefore block a main-store-only read (availability only; results stay correct). The gating predates this PR, but the PR introduces the absolute doc claim (the new audit path's AuditFilterNeedsIndex is the precise counterpart). Soften the claim or derive projection need from filter leaves.

Spec

No spec is available for this PR: the commit message (feat(reads): align live queries to raft horizons, 7fadd98...c6abd2ec) contains no issue reference and no spec file exists in the repository. There is consequently no confirmed material spec finding. For the record, the candidate spec claims raised during review were checked against the code and documentation and do not hold as spec violations: GetLedgerStats/GetTemplateUsage usage-store reads were not changed and the PR's own consensus documentation explicitly retains the mixed-provenance caveat; the minLogSequence wait on unfiltered audit reads is unchanged, intentional, documented legacy behavior; and the over-broad AlignmentOwed gating is pre-existing and is retained above as a documentation-consistency item.

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 rewires cross-store alignment from a native-sequence fold wait to fixed Raft-horizon certificates, and updates the read-path documentation accordingly. The core mechanism (fixed main-store horizon H, per-projection certificates re-read through the snapshot they vouch for, explicit failure instead of silent lag) is sound and well-tested for the audit path. However, the diff's own documentation contradicts both the live API surface and the actual gate logic in two places, it changes frozen-checkpoint reads from serve-as-is to hard-fail without a migration path for checkpoints created before the certificate keys existed, and it leaves a now-false safety comment on the checkpoint path. Recommendation: request changes — fix the documentation contradictions and either migrate or scope-guard the legacy-checkpoint regression before merge.

Standards

[P2] query-filter.md documents a prepared-query surface that no longer matches the API. The diff removes minLogSequence from the execute row (docs/technical/architecture/subsystems/read-path/query-filter.md:52) and from the body-content note (query-filter.md:67-71, now "only parameters ... pagination ... and mode"), but the field is still fully live: misc/proto/bucket.proto:1117 defines it, internal/adapter/http/handlers_execute_prepared_query.go still decodes and forwards it, internal/query/executor.go:151-158 still enforces it, and openapi.yml:5052 still documents it on the request schema. Worse, query-pipeline.md:115 — changed by this same diff — states "The legacy min_log_sequence request field is still honored", so two authoritative read-path documents now contradict each other and the API-surface table is materially false. This breaches AGENTS.md's documentation-maintenance rules ("API endpoint change: update ... openapi.yml"). Restore the row/note, or remove the field consistently in a separately scoped change.

[P2] query-pipeline.md claims selective projection waits the code does not implement. query-pipeline.md:98 states "It waits only for projections the query actually uses", but the gate is AlignmentOwed (internal/query/aligned_snapshot.go:69-70: filter != nil || target == LOGS) — every filtered account/transaction query enters the alignment wait regardless of which projections its filter consults (see the Spec section for the concrete main-store-only counterexamples). The added documentation is false as written. Either derive projection dependence from the filter tree or narrow the claim.

[P2] Filtered reads against pre-certificate query checkpoints now hard-fail, with no migration. Before this diff, AlignedIndexSnapshot served frozen stores as-is; the new frozen arm (internal/query/aligned_snapshot.go:165-169) errors when the frozen projection's Raft certificate is behind the checkpoint's applied index, and the audit path returns ErrIndexBuilding{"audit checkpoint"} (internal/application/ctrl/controller_default.go:1826-1828). The certificate keys (ReadRaftProgressKey/AuditRaftProgressKey) are new in this series; a missing key reads as 0 (internal/storage/readstore/cursor.go:26-33), and checkpoints are Pebble CreateCheckpoint copies of the live readstore. Query checkpoints created by pre-series builds therefore carry certificate 0, so after upgrading, every filtered account/transaction read and every index-filtered audit read against them fails deterministically until the client deletes and recreates each checkpoint. There is no startup backfill of the cursor on a legacy readstore (WriteRaftProgress is only written by the builder while processing new logs) and no rebuild trigger for an existing marker-bearing checkpoint directory. Either backfill the certificate (or treat a missing key on a frozen store as "legacy checkpoint, serve as-is"), or document the delete-and-recreate requirement in this PR.

[P2] Obsolete checkpoint-limitation comment contradicts the changed behavior. internal/adapter/grpc/server_bucket.go:982-991 still claims checkpoint creation waits for the log-index builder but "NOT" for the audit indexer, that a filtered checkpoint read "can omit audit entries ... never catches up", and that the fix "is out of scope here". All three are now false: checkpoint creation waits for the audit certificate (internal/application/indexbuilder/process_logs.go:308-324), a lagging frozen audit projection fails explicitly (controller_default.go:1826-1828), and this diff's raft-consensus.md states "Checkpoint creation waits for both read and audit certificates". AGENTS.md requires "Interface/behavior change: update relevant code comments"; a stale comment on a data-completeness path is actively misleading. Rewrite it to state the wait/fail contract.

[P3] Intentional error discards lack the required justification comment. The new code in internal/application/ctrl/controller_default.go:1783,1816,1825,1838,1852 uses bare _ = handle.Close() / _ = auditSnap.Close(). AGENTS.md ("use _ = ... with a justification comment when intentional") and docs/technical/contributing/conventions.md:62 (Good pattern: _ = file.Close() // Best effort cleanup) require a comment. Add one, or propagate the error.

Judgement calls (not blocking):

• Possible Duplicated Code — ListAuditEntriesFrom re-implements mainAppliedHorizon inline (internal/application/ctrl/controller_default.go:1788-1793 vs internal/query/aligned_snapshot.go:39-52), down to the identical "main-store snapshot applied index %d is behind ReadIndex horizon %d" error string. Both copies were added by this diff and must stay in sync; consider one exported helper. (The audit open→read-certificate→close→wait→retry loop also mirrors AlignedIndexSnapshot's shape.)
• Possible Speculative Generality — OpenReservedQueryHandle (internal/query/aligned_snapshot.go:267-274) has no caller or test anywhere in the tree; inline it until its consumer lands.

Spec

Reviewing against the PR-body specification ("align each live read only with the projections it uses, at the fixed main-store snapshot horizon"), one confirmed failure:

[P2] Main-store-only transaction filters still wait for the read projection. compileRevertedCondition reads only the FSM-maintained reversion bitset and the main-store universe ("No index is required", internal/query/compile.go:415-462), and compileTxIDCondition reads only the Pebble transaction history ("No index is required", internal/query/compile.go:1253-1308). Yet the unchanged coarse gate AlignmentOwed routes every filtered transaction list (internal/application/ctrl/list_entities.go:84-93) and prepared query (internal/query/executor.go:126-129) through AlignedIndexSnapshot, which blocks on WaitForRaftProgress until the read projection certifies H — a projection these filters never consult. Impact: if the read projection is stalled or rebuilding, tx-ID-only or reverted-only live reads block until the caller's deadline (or fail) despite holding all required data in the fixed main snapshot, unnecessarily coupling main-store read availability to indexer liveness. The new tests precisely cover audit sequence-only filters (internal/application/ctrl/controller_default_audit_alignment_test.go) but prove nothing equivalent for ordinary transaction filters. Resolution: derive read-projection dependence from the filter tree (including boolean combinations), and add tests showing tx-ID/reverted-only list and prepared-query reads succeed while the read certificate is unavailable, while genuinely indexed filters still wait. (This is the behavioral half of the query-pipeline.md:98 documentation finding above; the remediations overlap but both artifacts need fixing.)

No other missing/incorrect requirement or material scope creep was confirmed.

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

This PR (EN-1946, base 9a055a0 → head c5e68ed5) replaces the sequence-based read-index alignment with a fixed main-store Raft horizon H (verified against the ReadIndex result R) plus per-projection Raft certificates, extends the same scheme to audit reads, and propagates the local barrier horizon through the routed controller. The core mechanics are sound: the certificate is re-read through the snapshot it vouches for, waits target fixed H rather than the moving head, the audit wait loop is context-bounded and fails explicitly on a frozen store, and the new ListAuditEntriesFrom resource lifecycle (handle/audit-snapshot closers, joinedCloser) closes correctly on every path I traced. However, the change alters several observable behaviors — filtered audit reads now wait or fail instead of silently omitting, and legacy min_log_sequence remains honored — while authoritative docs and code comments still describe the old contracts. Three such contradictions are confirmed and blocking.

Recommendation: request changes — the code needs no structural rework, but the blocking documentation contradictions below must be resolved (and the spec deviation either fixed or explicitly acknowledged) before merge.

Standards

[P2][blocking] query-filter.md drops minLogSequence from the prepared-query execute surface, which the code still honors

• Location: docs/technical/architecture/subsystems/read-path/query-filter.md:52 (table row) and :67-71 (note), vs internal/adapter/http/handlers_execute_prepared_query.go:41,93, internal/query/executor.go:151-158, internal/proto/servicepb/bucket.pb.go (min_log_sequence field).
• Evidence: The doc now says the execute body carries "only parameters, pagination and mode", but the handler still decodes and forwards MinLogSequence and the executor still rejects execution when mainSeq < req.GetMinLogSequence(). This PR's own query-pipeline.md:115-118 states the opposite: "The legacy min_log_sequence request field is still honored" — the two authoritative read-path docs now contradict each other.
• Impact: Misleading authoritative API documentation; violates AGENTS.md (Documentation maintenance: docs are part of the change when APIs change).
• Resolution: Restore minLogSequence in the table row and note (as "legacy, still honored, not the causal alignment proof"), or remove the field end-to-end consistently.

[P2][blocking] "Waits only for projections the query actually uses" is false for reverted-only and tx-id-only filters

• Location: docs/technical/architecture/subsystems/read-path/query-pipeline.md:98, vs internal/query/aligned_snapshot.go:62-63 (AlignmentOwed), internal/query/compile.go:417-457 (compileRevertedCondition), internal/query/compile.go:1256-1305 (compileTxIDCondition).
• Evidence: AlignmentOwed returns true for any non-nil filter. But compileRevertedCondition reads the FSM-maintained reversion bitset through the main reader and compileTxIDCondition uses pebbleTxExists/NewPebbleTxRangeIterator on the main reader — neither consults the read index — yet both still block on the read projection's Raft certificate at H (aligned_snapshot.go:160-173). The PR itself demonstrates the precise gating pattern for audit (AuditFilterNeedsIndex), so the account/transaction path is inconsistent with both the doc and its own audit design.
• Impact: Misleading authoritative documentation; the availability consequence is captured in the Spec section below.
• Resolution: Derive projection use from the filter leaves (as done for audit), or narrow the doc claim to match actual behavior.

[P2][blocking] Audit alignment behavior changed, but the HTTP and checkpoint audit contracts still describe the old best-effort semantics

• Location: internal/adapter/http/handlers_list_audit_entries.go:39-45; docs/technical/architecture/subsystems/api/http-api.md:404-409; internal/adapter/grpc/server_bucket.go:982-991.
• Evidence: The new ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1804-1832) waits for ReadAuditRaftProgress >= H for every index-needed filter (including from HTTP, which passes a zero legacy floor), and fails explicitly with ErrIndexBuilding when the audit projection is disabled/rebuilding or when a checkpoint's frozen audit snapshot lags. Yet the HTTP handler comment and http-api.md still claim index-backed HTTP reads are "best-effort and may omit entries not yet indexed", and server_bucket.go:982-991 still claims checkpoint creation does "NOT [wait] for the separate async audit indexer", that filtered checkpoint reads "can omit audit entries ... it never catches up", and that the fix "is out of scope here" — all three statements are false in this tree (creation waits via WaitForAuditRaftProgress, internal/application/indexbuilder/process_logs.go:313-321, and a lagging frozen audit snapshot now returns ErrIndexBuilding{Index: "audit checkpoint"} instead of omitting).
• Impact: Clients relying on "may omit, never fails" will not expect the new blocking wait, ErrIndexBuilding errors, or deadline failures on filtered audit reads; misleading authoritative guidance. Violates AGENTS.md (Documentation maintenance and "Interface/behavior change: update relevant code comments").
• Resolution: Rewrite the HTTP comment, http-api.md, and the checkpoint comment to state that filtered audit reads wait for the audit Raft certificate at the fixed main-store horizon and fail explicitly when the projection is unavailable or lagging (frozen/checkpoint case included).

[P3] New intentional error discards lack the required justification comments

• Location: internal/application/ctrl/controller_default.go:1783,1816,1825,1838,1852.
• Evidence: New _ = handle.Close() and _ = auditSnap.Close() discards carry no comment. AGENTS.md ("use _ = ... with a justification comment when intentional") and docs/technical/contributing/conventions.md:61-62 (// Best effort cleanup) both require one. The file's pre-existing style is similar, so this is non-blocking.
• Resolution: Add short reason comments (e.g. // Best effort cleanup).

[P3] Horizon verification and its error text are duplicated in two files added by this PR

• Location: internal/application/ctrl/controller_default.go:1786-1790 vs internal/query/aligned_snapshot.go:32-44 (mainAppliedHorizon).
• Evidence: The inline ReadLastAppliedIndex + H >= R check in ListAuditEntriesFrom reimplements mainAppliedHorizon verbatim, including the identical error string "main-store snapshot applied index %d is behind ReadIndex horizon %d". Both copies must stay in sync for linearizability. Violates AGENTS.md ("Prefer existing repository patterns and DRY solutions"). Non-blocking.
• Resolution: Export and reuse mainAppliedHorizon (or an equivalent shared verifier) from the query package.

[P3] OpenReservedQueryHandle is exported with no caller or test

• Location: internal/query/aligned_snapshot.go:266-277 (and the reserve parameter of openQueryHandle).
• Evidence: No call sites anywhere in the tree; the comment defers to a future consumer. Speculative generality — land it with the consumer or inline it back. Non-blocking.
• Resolution: Remove until a consumer exists, or add the consumer and a test in this stack.

Checked and clean: the ListAuditEntriesFrom snapshot/closer lifecycle has no leaks (the closeHandle flag plus joinedCloser cover success and all error paths); the audit wait loop is bounded by the caller's context and fails loudly on a frozen store; barrier-horizon propagation correctly attaches only to local reads (remote hops establish their own horizon, covered by controller_routed_test.go); the H ≥ R check being skipped on the unaligned path is safe because WaitForApplied precedes handle creation; e2e changes preserve the original alignment regression trigger and add the new bounded-wait path; tests use t.Parallel() and no time.Sleep.

Spec

Spec source: the PR description — "Consumes the projection certificates from the preceding PR to align each live read only with the projections it uses, at the fixed main-store snapshot horizon."

[P2] Live reads wait on projections they do not use, violating the selective-alignment requirement

• Location: internal/query/aligned_snapshot.go:62-63 (AlignmentOwed); callers internal/application/ctrl/list_entities.go, internal/query/executor.go.
• Evidence: AlignmentOwed returns true for every non-nil account/transaction filter, so OpenQueryHandle reserves history and AlignedIndexSnapshot waits for the read projection's Raft certificate at H. But valid transaction filters can be entirely main-store-backed: compileRevertedCondition (compile.go:417-457) reads the FSM-maintained reversion bitset and compileTxIDCondition (compile.go:1256-1305) reads the main Pebble transaction history — neither consults the read projection. The audit path added by this PR gates precisely on filter shape (AuditFilterNeedsIndex), proving the selective pattern exists and is not applied here.
• Impact: If the read projection is lagging or stopped, a reverted-only or tx-id-only live query (including prepared queries) blocks until the caller's context expires — or fails — even though its complete answer is already available from the fixed main-store snapshot. This contradicts the stated requirement "only with the projections it uses" and is the behavioral counterpart of the query-pipeline.md:98 Standards finding.
• Resolution: Derive projection dependency from the filter tree (including boolean combinations), use that predicate consistently in OpenQueryHandle and prepared-query execution, and add tests proving main-store-only filtered queries do not wait while genuinely indexed filters do.

No other confirmed missing requirements, scope creep, or incorrect implementations were established from the available spec.

---

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 reworks live-read alignment onto fixed Raft horizons: each read pins the main store's durable applied index H (verified against the ReadIndex horizon R), waits for each projection's independent Raft certificate to reach H, and re-reads that certificate from the projection snapshot it will iterate. The audit read path gains the same treatment plus an explicit disabled/rebuilding/rebuilding-checkpoint failure, and routed reads now propagate the local barrier horizon. The core design is sound and well-tested, but the PR leaves authoritative documentation materially false in two places, contradicts its own stated alignment requirement for main-store-only filters, leaves a stale code comment about checkpoint audit behavior, and introduces a compatibility regression for checkpoints created before the certificate keys existed. Request changes.

Standards

[P2][blocking] The canonical API-surface table now misstates the still-honored minLogSequence execute field
docs/technical/architecture/subsystems/read-path/query-filter.md:52 and :67-71 remove minLogSequence from the prepared-query execute row and state the body carries "only parameters … and mode". But the field is still fully supported: internal/adapter/http/handlers_execute_prepared_query.go:32-42 decodes it, :86-94 forwards it, internal/query/executor.go:154-158 still enforces it (ErrReadIndexNotCaughtUp), openapi.yml:5052 still documents it, misc/proto/bucket.proto:1221 still defines it — and this same PR's query-pipeline.md says the legacy field "is still honored as an additional client-selected native-sequence floor during this staged rollout". This breaches AGENTS.md's documentation-maintenance rule ("API endpoint change: update … openapi.yml") and makes authoritative guidance self-contradictory across the same PR.
Resolution: restore the row/note in query-filter.md, or remove the field consistently across proto/handler/executor/openapi in a separately scoped API change.

[P2][blocking] "Waits only for projections the query actually uses" is not what the code does
docs/technical/architecture/subsystems/read-path/query-pipeline.md:98 claims each read "waits only for projections the query actually uses", but AlignmentOwed (internal/query/aligned_snapshot.go:68-70) is filter != nil || target == …LOGS — it never inspects which leaves the filter uses. Filters that read only main-store data still block on the read projection's certificate through list_entities.go:101 → AlignedIndexSnapshot: compileRevertedCondition (internal/query/compile.go:415-419, "No index is required — the bitset is always maintained by the FSM") and compileTxIDCondition (compile.go:1252-1255, "No index is required"). See the Spec section for the corresponding requirement violation.
Resolution: derive projection dependence from the filter tree (including boolean combinations), or narrow the documented claim.

[P2][blocking] Stale checkpoint-limitation comment contradicts the implemented wait-and-fail contract
internal/adapter/grpc/server_bucket.go:982-991 still claims checkpoint creation "waits for the log-index builder … but NOT for the separate async audit indexer" and that "a filtered checkpoint read can omit audit entries". The tree no longer behaves that way: checkpoint creation now waits for the audit certificate (internal/application/indexbuilder/process_logs.go:314-328, WaitForAuditRaftProgress before createReadIndexCheckpoint), and a lagging frozen audit projection fails explicitly with ErrIndexBuilding{"audit checkpoint"} (internal/application/ctrl/controller_default.go:1826-1828), while AlignedIndexSnapshot fails a frozen read projection behind the checkpoint horizon (internal/query/aligned_snapshot.go:165-169). This PR even updates raft-consensus.md to say "Checkpoint creation waits for both read and audit certificates before publishing readiness", leaving the code comment the only stale statement. Breaches AGENTS.md "Interface/behavior change: update relevant code comments".
Resolution: rewrite the comment to the current wait-and-fail contract and drop the "tracked as a follow-up" framing.

[P2][blocking] Checkpoints created before the certificate keys existed now hard-fail their reads (upgrade compatibility)
AlignedIndexSnapshot previously served frozen stores as-is; this PR replaces that fast path with a certificate check that errors when ReadRaftProgressFrom(snap) < H (internal/query/aligned_snapshot.go:165-169). A missing progress key reads as 0 (internal/storage/readstore/cursor.go:22-27, documented ErrNotFound-as-0 mapping), while the checkpoint's main store has carried a durable LastAppliedIndex for a long time (state.SetAppliedIndex is written by the FSM applier and read by restore/backup/WAL paths). Any checkpoint read index materialized by a build that predates WriteRaftProgress therefore certifies 0 and fails permanently: filtered account/transaction lists and every LOGS list on such a checkpoint error with "frozen read projection at Raft index 0 is behind main checkpoint horizon N", and filtered audit reads fail with ErrIndexBuilding{"audit checkpoint"} (controller_default.go:1826-1828). There is no migration or version gate (openCheckpointStores only gates on the .ready marker, which old checkpoints have), and the checkpoint cannot be recreated — creation happens inline when the builder crosses the historical CreatedQueryCheckpoint log, so the deleted snapshot's point-in-time is unrecoverable. The code comment justifies the new failure as protection against "a malformed or incomplete checkpoint", but a valid pre-certificate checkpoint is indistinguishable from a malformed one and is failed too. Both candidate reviews missed this.
Resolution: tolerate a missing (as opposed to merely low) certificate in frozen stores — e.g., fall back to the native fold cursor the old path used, gate on a stored format version, or refuse to open pre-certificate checkpoints with an explicit actionable error — and cover it with a test.

[P3][non-blocking] Intentional error discards lack justification comments
New _ = …Close() discards at internal/application/ctrl/controller_default.go:1783, 1816, 1825, 1838, 1852 (and in the new tests, e.g. controller_default_audit_alignment_test.go:29,43 and aligned_snapshot_test.go:140) carry no reason. AGENTS.md requires "_ = ... with a justification comment when intentional" and conventions.md gives the "best effort cleanup" pattern. The pre-existing file style happens to omit them too, so this is polish, not a blocker.

[P3][non-blocking][judgement call] Duplicated main-horizon check
internal/application/ctrl/controller_default.go:1786-1792 re-implements mainAppliedHorizon (internal/query/aligned_snapshot.go:36-48) verbatim, down to the identical error strings ("reading main-store applied index: …" / "main-store snapshot applied index %d is behind ReadIndex horizon %d"); both copies are added by this diff. AGENTS.md: "Prefer existing repository patterns and DRY solutions" — export the helper and reuse it.

[P3][non-blocking][judgement call] Speculative generality: OpenReservedQueryHandle
internal/query/aligned_snapshot.go:267-277 (and its reserve plumbing) has no caller and no test anywhere in the tree; the only reachable entry point is OpenQueryHandle. Inline it until its consumer lands.

Considered and rejected: the e2e rewrites in tests/e2e/business/ are justified — the production trigger genuinely changed (wait bounded by caller context against a fixed horizon), so cross_store_value_skew_test.go correctly probes with a 250 ms bound and accepts only DeadlineExceeded, and the long fixed-target budget in cross_store_snapshot_alignment_test.go matches the new contract; the repeated routed-controller delegation in controller_routed.go matches that file's established style.

Spec

Spec source: the PR's own stated requirement — "Consumes the projection certificates from the preceding PR to align each live read only with the projections it uses, at the fixed main-store snapshot horizon."

[P2] Main-store-only filters still wait for a projection they do not use
AlignmentOwed (internal/query/aligned_snapshot.go:62-70) treats every non-nil filter — and every LOGS target — as depending on the read projection, so such reads block in AlignedIndexSnapshot until the projection's Raft certificate reaches the fixed H (aligned_snapshot.go:108-177), reachable from ordinary lists (internal/application/ctrl/list_entities.go:101), filtered aggregation, and prepared queries (internal/query/executor.go:126-130). But several filter shapes read only main-store data: transaction-ID filters scan the Pebble history zone (internal/query/compile.go:1254-1309, "No index is required"), and reverted filters use the FSM-maintained bitset (compile.go:417-462, "No index is required"). A reverted-only or tx-id-only live read therefore blocks on the asynchronous read projection even though it never consults it; if that projection is stalled or disabled, an otherwise answerable read hangs until the caller's deadline. This is the behavioral counterpart of the query-pipeline.md:98 drift flagged under Standards, and it contradicts the "only with the projections it uses" requirement.
Resolution: derive projection dependence from the filter tree (including boolean combinations, mirroring AuditFilterNeedsIndex's shape analysis), skipping certificate alignment for main-store-only shapes, and add a test proving such reads succeed while read-projection progress is unavailable. If the broader gate is deliberate for this stack stage, narrow the PR's stated requirement instead and document the residual coupling.

---

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 reworks live-read alignment: each read now pins a fixed main-store applied index H, verifies H >= R for linearizable reads, waits for per-projection Raft certificates at that fixed horizon (including a new audit-projection alignment in ListAuditEntriesFrom), and rewrites the read-path documentation accordingly. The core code changes are sound — resource lifecycles in the rewritten audit path are correct, checkpoint reads correctly bypass the new routed-horizon check, and the behavior changes are well covered by new tests. However, the PR's own documentation fails its own bar: the authoritative query-filter page documents away an API field the API still honors (contradicting both openapi.yml and this same PR's query-pipeline page), and a now-false safety-critical comment about checkpoint audit completeness was left in place on a function this PR rewrote. Recommendation: request changes — both blocking items are documentation-only and quick to fix.

Standards

[blocking] Authoritative API doc drops a field the API still honors

docs/technical/architecture/subsystems/read-path/query-filter.md:52 removes minLogSequence from the prepared-query execute row, and lines 68–74 now state the execute body carries "only parameters … pagination (cursor, pageSize) and mode". The field is still live and enforced: misc/proto/bucket.proto:1117 (min_log_sequence on the execute request), internal/adapter/http/handlers_execute_prepared_query.go:41,93 (decoded and forwarded), internal/query/executor.go:154 (enforced as a freshness floor), and openapi.yml:5052 still documents it. Worse, this same PR's query-pipeline.md:115-118 states the field "is still honored as an additional client-selected native-sequence floor" — the two rewritten authoritative docs contradict each other. Per AGENTS.md:87 ("API endpoint change: … update openapi.yml"; docs are part of the change), restore the row/note, or remove the field in a separately scoped API change.

[blocking] Stale checkpoint-limitation comment contradicts the changed behavior

internal/adapter/grpc/server_bucket.go:982-992 still claims checkpoint creation does NOT wait for the async audit indexer, that a filtered checkpoint read "can omit audit entries" and "never catches up", and that the proper fix "is out of scope here". All three are now false: checkpoint creation waits for the audit Raft certificate (internal/application/indexbuilder/process_logs.go:320), and a lagging frozen audit projection now fails explicitly with ErrIndexBuilding{"audit checkpoint"} (internal/application/ctrl/controller_default.go:1826-1828, added in this PR). This comment sits directly above the call to the function this PR rewrote; per AGENTS.md:89 ("Interface/behavior change: update relevant code comments"), rewrite it to describe the current readiness/failure contract.

[non-blocking] "Waits only for projections the query actually uses" is false as written

query-pipeline.md:98 claims the controller "waits only for projections the query actually uses", but AlignmentOwed (internal/query/aligned_snapshot.go:61-63) returns true for every non-nil filter. reverted-only and tx-ID-only transaction filters never consult the read index (internal/query/compile.go:417-419 and 1254-1255 — "No index is required") yet still block on the read projection's certificate via AlignedIndexSnapshot. Impact is availability-only (a stalled read projection can time out reads that need no projection). Soften the claim, or gate on the filter's actual leaves — AuditFilterNeedsIndex shows the required shape.

[non-blocking] Bare _ = discards lack justification comments

New code at internal/application/ctrl/controller_default.go:1783,1816,1825,1838,1852 discards Close() errors without a reason. AGENTS.md:68 ("use _ = ... with a justification comment when intentional") and docs/technical/contributing/conventions.md:61-62 require one. Pre-existing style in the file is similar, so this is polish, not a regression — but new lines should follow the rule.

[non-blocking] Judgement call — duplicated main-horizon check

controller_default.go:1785-1788 re-implements query.mainAppliedHorizon (internal/query/aligned_snapshot.go:36-51) verbatim, including the identical error string "main-store snapshot applied index %d is behind ReadIndex horizon %d". Both copies are added in this PR; drift would silently weaken the linearizability check. The handle satisfies dal.PebbleGetter, so call the shared helper (per AGENTS.md DRY guidance).

[non-blocking] Judgement call — Speculative Generality

OpenReservedQueryHandle (internal/query/aligned_snapshot.go:267-274, plus the reserve parameter on openQueryHandle) has no caller or test anywhere in the tree. Defer it until its consumer lands.

Spec

No confirmed material finding. No spec source is available for EN-1946 (none in the workspace, none referenced in the repository), so spec conformance could not be independently verified; the candidate spec finding derived from an unverifiable quoted requirement was not retained. The underlying behavior concern (main-store-only filtered reads waiting on the read projection) is captured above as the documentation-accuracy Standards finding.

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 rewires live reads so each query aligns only with the projections it actually uses, at a fixed main-store Raft horizon (H >= R for linearizable reads), and routes filtered audit reads through the same certificate-gated snapshot machinery. I verified both prior reports against diff.patch and the code. The core alignment logic is sound: the H >= R gate, the per-projection certificate loop, the frozen-checkpoint fail-loud behavior, the audit readiness mapping, and the filterUsesReadIndex parity with compile's dispatch all check out (I confirmed each parity arm — address, builtin-uint, reverted, ledger — against compile.go directly). What survives review is one real P2: the behavior change left four authoritative HTTP/gRPC/CLI documentation surfaces describing the old best-effort audit semantics, which AGENTS.md explicitly forbids. No correctness, security, or compatibility defect was found in the changed code paths.

Recommendation: approve with comments.

Standards

1. [P2] Behavior change left authoritative docs/comments describing the old best-effort audit semantics. AGENTS.md: "Documentation is part of the change when behavior ... changes"; "Interface/behavior change: update relevant code comments." This PR routes HTTP filtered audit reads through ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1713), which now waits for the audit Raft certificate at the fixed main horizon and fails explicitly (ErrAuditDisabled / ErrIndexBuilding) instead of omitting entries — yet four surfaces still document the old contract:

   • internal/adapter/http/handlers_list_audit_entries.go:35-45 still says the endpoint "always performs a live, best-effort read" and that a non-seq filter "may transiently omit very recent entries that have not yet been indexed". False: the handler now blocks on the certificate wait and can return AUDIT_DISABLED / index-building errors where it previously returned (possibly incomplete) results.
   • docs/technical/architecture/subsystems/api/http-api.md:404-409 still says "index-backed reads are best-effort and may omit entries not yet indexed".
   • docs/technical/architecture/subsystems/api/grpc-api.md:678-686 still describes the minLogSequence-driven audit wait ("every gRPC node ... waits for its local audit index to reach the live audit head sampled afterward. With a zero bound, those index-backed results are best-effort") — superseded by the fixed-horizon certificate wait, which applies regardless of the bound (the bound is now only an additional native-sequence floor, per the updated raft-consensus.md and query-pipeline.md).
   • docs/ops/cli.md:2360-2372 asserts "checkpoint creation waits for the log index but not yet for the audit indexer ... may omit entries", directly contradicting both the code and this diff's own addition to raft-consensus.md ("Checkpoint creation waits for both read and audit certificates before publishing readiness").
     Impact: clients and operators relying on these surfaces will expect silent omission instead of blocking waits and explicit errors — the exact contract this PR exists to change.

2. [P3] New intentional _ = error discards lack the required justification comment. AGENTS.md:68: "use _ = ... with a justification comment when intentional"; docs/technical/contributing/conventions.md:62 shows the required form (_ = file.Close() // Best effort cleanup). New sites in this diff without justification: internal/application/ctrl/controller_default.go:929, 941 (AggregateVolumes snapshot defers), controller_default.go:1766, 1775, 1790, 1804 (audit snapshot cleanup in ListAuditEntriesFrom), and internal/query/aligned_snapshot.go:204, 216. Mitigating: the surrounding pre-existing code uses the same bare style, and these are best-effort closes, so this is a documentation-rule gap, not a behavioral risk.

3. [Judgement] Duplicated main-horizon gate. ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1733-1738) re-implements the check that query.mainAppliedHorizon (internal/query/aligned_snapshot.go:36-48) already provides, down to the byte-identical error "main-store snapshot applied index %d is behind ReadIndex horizon %d". These are the two H >= R linearizability gates; a future edit to one (e.g. relaxing the comparison or changing the message) would silently weaken the other path. Extract and share the helper.

4. [Judgement] Duplicated audit readiness mapping and certify-wait loop. The disabled → ErrAuditDisabled / rebuilding → ErrIndexBuilding{"audit (rebuilding)"} mapping now exists in both internal/application/admission/admission.go:873-886 and controller_default.go:1753-1759; the open-snapshot → read-certificate → close → wait → retry loop shape exists in both internal/query/aligned_snapshot.go:176-232 and controller_default.go:1761-1784 (with subtly different frozen-store outcomes: ErrIndexBuilding{"audit checkpoint"} vs. a plain "frozen read projection ... behind main checkpoint horizon" error). Drift between the two would yield inconsistent error kinds for the same condition or a weaker alignment guarantee on one read path. Keep one source of truth.

5. [Judgement] Dispatch-mirroring drift risk. filterUsesReadIndex (internal/query/aligned_snapshot.go) hand-mirrors compile's per-leaf storage choice (internal/query/compile.go:174-201), and AuditFilterNeedsIndex (internal/query/audit_filter.go) mirrors CompileAuditFilter the same way. A future filter arm added to the compiler but not the predicates would silently skip a required certificate wait (torn reads) or add a needless wait. The comprehensive table tests (TestAlignmentOwedWaitsOnlyForUsedReadProjection, TestAuditFilterNeedsIndex) mitigate but must be updated in lockstep; a shared leaf-classification source would remove the risk.

6. [Low, judgement] Reduced mutation sensitivity in an e2e regression guard. tests/e2e/business/cross_store_value_skew_test.go:136-152 previously failed on any probe error ("an error here is a real failure"); the new probe tolerates DeadlineExceeded and narrowly-scoped Unavailable. The spec therefore no longer proves that a filtered read waits out a lagging fold rather than rejecting — a timeout is now an accepted outcome. The torn-row invariant is still enforced on every successful probe and the Unavailable branch still requires the alignment-wait message, so the guard is narrowed, not removed; the change is deliberate and well-commented, but worth noting for anyone mutating the alignment code against this spec.

Spec

No confirmed material finding. No usable spec exists for this change: the PR body is a single line ("Stack 2/7 for EN-1946") referencing an internal tracker not available for review, so the Spec axis is empty by construction.

Reviewed independently by GLM (glm-5.3-flash) and DeepSeek (deepseek-v4-pro-0813) via Shipfox; verified and synthesized by GLM.

[shipfox-ai]

This PR replaces the best-effort audit/read-index contract with a fixed-horizon, certificate-gated alignment: filtered audit reads wait for the audit projection's Raft certificate at the main snapshot's durable applied index H (verified against the routed ReadIndex horizon R) and fail explicitly with ErrAuditDisabled/ErrIndexBuilding; main-store-only filter shapes no longer acquire projection waits; and query-checkpoint creation now waits for both read and audit certificates. The implementation is sound: the snapshot lifecycle closes on every error path, candidates ahead of the main audit horizon are trimmed, malformed filters keep their InvalidArgument contract before the readiness gate, and the new tests are parallel, deterministic, and pin the changed behavior (including the e2e probe relaxation, which keeps the torn-row assertion). No blocking correctness or security defect was found. However, four authoritative documentation surfaces still describe the superseded best-effort audit contract that this diff deliberately retires, and one linearizability guard was copy-pasted rather than shared. Recommendation: approve with comments — the documentation rewrites and the two small DRY extractions should land before or immediately with merge.

Standards

1. [Hard — documentation maintenance] Four authoritative surfaces still describe the superseded best-effort audit contract. AGENTS.md makes docs part of the change when behavior changes. The diff changes filtered audit reads (on every transport, since ListAuditEntries now discards minLogSequence and the controller gate applies unconditionally) to wait for the audit Raft certificate at fixed H and fail explicitly, yet:

   • internal/adapter/http/handlers_list_audit_entries.go:38-42 still claims a "live, best-effort read" that "may transiently omit very recent entries" — the opposite of the new wait-and-fail behavior this handler now gets via ListAuditEntries(..., 0).
   • docs/technical/architecture/subsystems/api/http-api.md:404-409 still says "index-backed reads are best-effort and may omit entries not yet indexed".
   • docs/technical/architecture/subsystems/api/grpc-api.md:684 still says "With a zero bound, those index-backed results are best-effort" (the non-zero-bound description on lines 680-683 remains accurate).
   • docs/ops/cli.md:2355-2374 still claims filtered audit reads are "eventually consistent when --min-log-sequence is zero", that nodes "do not wait for audit-index progress", and that checkpoint creation "waits for the log index but not yet for the audit indexer" — contradicted by this diff's own raft-consensus.md ("Checkpoint creation waits for both read and audit certificates") and by internal/application/indexbuilder/process_logs.go:317 (WaitForAuditRaftProgress before createReadIndexCheckpoint).
     Rewrite all four to the new wait-and-fail contract.

2. [Hard — DRY] The H >= R linearizability gate is duplicated verbatim. internal/application/ctrl/controller_default.go:1736-1742 re-implements query.mainAppliedHorizon (internal/query/aligned_snapshot.go:33-47) line-for-line, including the identical error "main-store snapshot applied index %d is behind ReadIndex horizon %d". Both copies are new in this diff; drift would silently weaken one linearizability gate. The *dal.ReadHandle satisfies dal.PebbleGetter, so export and reuse the helper.

3. [Judgement call — DRY] The disabled/rebuilding → error mapping now exists twice. internal/application/admission/admission.go:875-881 and controller_default.go:1753-1760 both map disabled → domain.ErrAuditDisabled and rebuilding → &domain.ErrIndexBuilding{Index: "audit (rebuilding)"}. Share one classifier so the two gates cannot diverge in error kind or reason.

4. [Judgement call — drift risk] filterUsesReadIndex/auditFilterNeedsIndex hand-mirror compile's storage choices with no shared guard. Verified consistent today (address on ACCOUNTS uses the always-on main-store existence index, compile.go:1044-1054; reverted uses the FSM bitset, compile.go:417-421; txID uses Pebble history, compile.go:1254-1256; TestAlignmentOwedWaitsOnlyForUsedReadProjection pins it), but a compiler-only change to a leaf's storage path would silently skip or add a projection wait, changing the consistency contract without a failing test. Add a cross-reference comment or a parity test keyed off the compiler itself.

5. [Low — documented-standard breach, non-blocking] Intentional error discards lack the required justification comment. AGENTS.md:68 and conventions.md:62 require _ = … // <reason> for intentional discards. New sites in this diff: controller_default.go:929, 941 (_ = snap.Close()), :1732 (_ = handle.Close()), and :1766, 1775, 1790, 1804 (_ = auditSnap.Close()). These are best-effort cleanup and match the file's pervasive pre-existing style, hence non-blocking — add the marker.

6. [Judgement call — Speculative Generality] OpenReservedQueryHandle has no production caller. internal/query/aligned_snapshot.go:321-328 (plus the reserve parameter threaded through openQueryHandle) is exercised only by its own test (aligned_snapshot_test.go:377). Plausible groundwork, but inline it until its consumer lands so the reclamation-reservation contract has exactly one live user.

Spec

No spec is available for this change, so the Spec axis has no confirmed material finding.

Reviewed independently by GLM (glm-5.3-flash) and DeepSeek (deepseek-v4-pro-0813) via Shipfox; verified and synthesized by GLM.

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1890 (comment)

[shipfox-ai]

Final review of formancehq/ledger PR #1890 — "feat(reads): align live queries to Raft horizons (EN-1946)" (fixed point 0004a07 → 4553f98).

This PR replaces the fold-sequence alignment model with fixed-Raft-horizon alignment: routed linearizable reads now carry their ReadIndex horizon R in the context, every aligned read pins the main snapshot's durable applied index H and asserts H >= R, and each projection actually consulted by the query must publish a Raft certificate >= H read back through the snapshot it vouches for. The audit path gains the same treatment (certificate-gated snapshot, explicit ErrAuditDisabled/ErrIndexBuilding failures, main-horizon trimming), and query-checkpoint creation now waits for the audit certificate. The implementation is sound: the routing/barrier-horizon plumbing, the wait loops, snapshot lifecycles and the new error taxonomy are correct and well tested, and I independently verified the checkpoint-read path bypasses the horizon correctly and that the filterUsesReadIndex/compile storage parity holds leaf-by-leaf. One real documentation-maintenance breach stands out: four authoritative surfaces still describe the removed "best-effort may omit" audit contract. Recommendation: approve with comments — no runtime correctness defect was found; the stale documentation should be corrected in this change or immediately before merge.

Standards

1. [Major] Stale authoritative docs/comments still describe the removed "best-effort may omit" audit contract (AGENTS.md "Documentation maintenance"; conventions.md PR checklist "Documentation updated if necessary"). The diff makes filtered audit reads wait for the audit certificate at the fixed main horizon, fail explicitly (ErrAuditDisabled/ErrIndexBuilding), and makes checkpoint creation wait for the audit certificate (internal/application/indexbuilder/process_logs.go:317). Four surfaces still state the old contract, contradicting the PR's own updates to raft-consensus.md, query-pipeline.md and read-snapshot-consistency.md:

   • docs/ops/cli.md:2354-2373 — "eventually consistent when --min-log-sequence is zero", nodes "do not wait for audit-index progress", and the checkpoint caveat "may omit entries whose audit-zone rows exist in the checkpoint but were not indexed … a frozen checkpoint never catches up … a tracked follow-up" (now false; checkpoint creation does wait).
   • docs/technical/architecture/subsystems/api/http-api.md:407 — "index-backed reads are best-effort and may omit entries not yet indexed".
   • docs/technical/architecture/subsystems/api/grpc-api.md:680-684 — "With a zero bound, those index-backed results are best-effort".
   • internal/adapter/http/handlers_list_audit_entries.go:38-42 — "performs a live, best-effort read … may transiently omit very recent entries".
     Impact: operator/client-facing documentation under-promises and mis-describes implemented behavior; the CLI checkpoint note explicitly claims a tracked follow-up that this PR closes. Update all four to the fixed-horizon contract.

2. [Minor] Duplicated projection-alignment logic creates drift risk on linearizability gates — all copies added by this diff:

   • query.mainAppliedHorizon (internal/query/aligned_snapshot.go:36-48) is re-implemented inline in internal/application/ctrl/controller_default.go:1736-1742, including the byte-identical error "main-store snapshot applied index %d is behind ReadIndex horizon %d". Export and reuse the helper.
   • The audit readiness mapping (disabled → ErrAuditDisabled, rebuilding → &domain.ErrIndexBuilding{Index: "audit (rebuilding)"}) now exists in both internal/application/admission/admission.go:875-881 and controller_default.go:1754-1760. Extract a shared classifier.
   • The audit wait loop (controller_default.go:1762-1784) is a second hand-rolled alignment loop mirroring AlignedIndexSnapshot's shape. Consolidating reduces the chance one copy silently weakens its freshness proof.

3. [Minor] filterUsesReadIndex hand-mirrors compile's per-leaf storage choice (internal/query/aligned_snapshot.go:77-117). Parity is verified correct today (address → index only on TRANSACTIONS; TX_ID and reverted → main store; metadata/reference/log leaves → index; combinators recursive), but a compiler-only change could silently add or skip a projection wait — a skipped wait would serve stale index-backed results. Derive the dependence from compilation or add an exhaustive parity test over every condition kind × target.

4. [Minor] Speculative generality: OpenReservedQueryHandle (internal/query/aligned_snapshot.go:326) has no production caller — only TestOpenReservedQueryHandleHoldsReclaimFloor exercises it, and the doc comment defers to a not-yet-landed consumer. Prefer inlining until that consumer ships.

Rejected candidate findings: the "intentional error discards lack justification comments" item (40+ identical pre-existing bare _ = ... discards exist in controller_default.go; AGENTS.md also mandates preferring existing repository patterns, and cleanup-only Close errors carry no correctness impact); and the controller_routed.go:326 logger-guard syntax inconsistency (style only).

Spec

No spec is available for this PR, so the Spec axis has no confirmed material finding.

Reviewed independently by GLM (glm-5.3-flash) and DeepSeek (deepseek-v4-pro-0813) 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: #1890 (comment)

[shipfox-ai]

This PR reworks cross-store read alignment: linearizable reads now fix a main-store Raft horizon H (verified against the ReadIndex horizon R), wait only for projections the query actually uses, and re-read each projection's certificate through the very snapshot it vouches for; audit reads get the same treatment with a certificate-gated audit snapshot and main-horizon trimming. The core design is sound and well-tested — the certificate is read through the snapshot itself, main-store-only filters no longer acquire projection waits (verified against compile.go: ACCOUNTS address/reverted/tx-ID leaves all read the main reader), malformed audit filters keep their InvalidArgument contract before the readiness gate, and the gRPC handler still honors minLogSequence, so no compatibility break was found. Remaining issues are documentation-standard violations and duplicated consistency-critical logic; none blocks merge. Recommendation: approve with comments.

Standards

1. [Hard — documented standard] New _ = error discards without the justification comment required by AGENTS.md:68 ("use _ = ... with a justification comment when intentional"; docs/technical/contributing/conventions.md:62 shows the Good form). New sites, all comment-free: internal/application/ctrl/controller_default.go:929 and :941 (the two new defer func() { _ = snap.Close() }() branches in AggregateVolumes), :1766, :1775, :1790, :1804 (the four _ = auditSnap.Close() sites in ListAuditEntriesFrom), and internal/application/ctrl/controller_default_audit_alignment_test.go:76 (defer func() { _ = frozen.Close() }()). Mechanical fix: add // Best effort cleanup or propagate the error.

2. [Judgement — Duplicated Code on a linearizability guard] The "read LastAppliedIndex; if a barrier is present and H < R, error" check is written twice with a byte-identical error string: unexported query.mainAppliedHorizon (internal/query/aligned_snapshot.go:36-50) and inline in ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1736-1741). Because the helper is unexported, ctrl could not reuse it; a future fix to one copy (e.g. relaxing or reordering the guard) would silently diverge from the other on a consistency invariant. Export and share the helper.

3. [Judgement — Duplicated Code on audit readiness] The disabled/rebuilding → error mapping now exists twice: internal/application/admission/admission.go:874-881 and internal/application/ctrl/controller_default.go:1747-1752, including the repeated literal "audit (rebuilding)". The checkpoint-admission gate and the live-read gate must agree on error kind and reason (the new tests pin Precondition vs Unavailable); extract one classifier so they cannot drift.

4. [Judgement — drift risk] filterUsesReadIndex (internal/query/aligned_snapshot.go:77) hand-mirrors compile's per-leaf storage choice. Verified today against compile.go (ACCOUNTS address → pebbleReader at :1081/:1114, TRANSACTIONS address → indexReader at :1101, reverted → pebbleReader at :424, tx ID → Pebble history at :1256), and pinned by table tests — but the parity is by-convention, not derived. A compile-only change mirrored on one side would silently add or skip a projection wait and change the consistency contract. Consider deriving dependence from compilation itself or an exhaustive parity test.

5. [Judgement — Speculative Generality] OpenReservedQueryHandle (internal/query/aligned_snapshot.go:326) has no production caller — only TestOpenReservedQueryHandleHoldsReclaimFloor. If a later PR in this stack consumes it, fine; otherwise inline it and the reserve parameter until then.

6. [Judgement — relaxed regression trigger] tests/e2e/business/cross_store_value_skew_test.go:136-152 now accepts DeadlineExceeded and narrowly-matched Unavailable probes as success (continue) instead of failing, because EN-1946 waits out a lagging projection under sustained pressure. The skew assertion itself still runs on every served probe and the served > 0 floor remains, and the relaxation is commented and narrowed — but it does weaken the previous "every probe must succeed" trigger, so please confirm this is the intended test contract going forward.

Spec

No spec was available for this change: the commit message (feat(reads): align live queries to raft horizons) references no issue, no linked GitHub issue was reachable, and no spec file matches this feature (EN-1946 is an external Jira key not accessible via the read-only integrations). The Spec axis therefore has no confirmed material finding — the change was not assessed against a specification. The in-repo documentation updates (raft-consensus, query-pipeline, read-snapshot-consistency, query-checkpoints, prepared-queries) were reviewed and are consistent with the implementation.

Reviewed independently by GLM (glm-5.3-flash) and DeepSeek (deepseek-v4-pro-0813) via Shipfox; verified and synthesized by GLM.

[shipfox-ai]

This PR moves projection-backed reads from fold-sequence alignment to Raft-certificate alignment at a fixed main-store applied index H, threads the local ReadIndex horizon R through the routed controller, gates audit reads on the audit projection's certificate, and updates six architecture documents plus tests to match. I verified every retained candidate finding directly against diff.patch and the code, including a leaf-by-leaf parity check of filterUsesReadIndex against compile's storage dispatch, the snapshot ownership paths in ListAuditEntriesFrom, and checkpoint compatibility (the base branch already publishes Raft certificates, so pre-existing checkpoints remain readable). No correctness, security, or compatibility defect was confirmed. The findings below are maintainability and documented-standard drift; recommendation: approve with comments.

Standards

1. Duplicated linearizability guard (medium). The H >= R barrier check exists twice, both copies added by this diff: mainAppliedHorizon (internal/query/aligned_snapshot.go:36-49) and a verbatim inline copy in ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1736-1742), including the identical "main-store snapshot applied index %d is behind ReadIndex horizon %d" error. If one copy is later relaxed (e.g. dropping the ReadBarrierHorizon comparison), audit reads would silently lose the linearizability guarantee that entity lists keep. Have ListAuditEntriesFrom call query.MainAppliedHorizon instead of inlining it.

2. Unjustified _ = error discards (low, documented standard). AGENTS.md ("Handle them explicitly, or use _ = ... with a justification comment when intentional") and docs/technical/contributing/conventions.md (Error Handling: _ = file.Close() // Best effort cleanup) require a justification comment on intentional discards. New code omits it at internal/application/ctrl/controller_default.go:1766, 1775, 1790, 1804 (_ = auditSnap.Close()) and at the two new defer func() { _ = snap.Close() }() in AggregateVolumes (controller_default.go:929, 941). The file already uses bare discards pervasively, so this is cosmetic, but the new lines breach the documented rule's letter — add // Best effort cleanup.

3. Duplicated audit readiness mapping (low). checkQueryCheckpointProjectionReady (internal/application/admission/admission.go:876-880) and ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1754-1760) both map AuditProjectionState() to domain.ErrAuditDisabled / &domain.ErrIndexBuilding{Index: "audit (rebuilding)"} with the same literal. Drift would give checkpoint admission and live audit reads different error kinds for the same projection state. Extract one shared readiness helper. (The snapshot→read-certificate→wait retry loop in ListAuditEntriesFrom likewise repeats the shape of AlignedIndexSnapshot; the same helper could own it.)

4. Speculative generality — OpenReservedQueryHandle (low). internal/query/aligned_snapshot.go:326 (plus the reserve parameter on openQueryHandle) has no production caller; only its own test (internal/query/aligned_snapshot_test.go:377) exercises it. If it exists for a later PR in the stack, say so in the PR description; otherwise inline it back until a real consumer lands.

5. Repeated switches — projection-dependence predicates mirror the compiler (low). filterUsesReadIndex (internal/query/aligned_snapshot.go:96-127) re-encodes, leaf by leaf, the storage choices that compile dispatches on (internal/query/compile.go:177-207); auditFilterNeedsIndex does the same for CompileAuditFilter. I verified the parity holds for every arm today, and malformed/unknown filters still fail loudly via Compile on both the aligned and non-aligned paths. But the coupling is real: a compiler-only change (a new arm, or a leaf promoted to/from the main store) would silently skip or add a projection wait and change the consistency contract; the table tests pin today's parity but must be updated in lockstep. Deriving both classifications from a single shared source would remove this hazard.

Spec

No spec is available for this PR: EN-1946 lives in an external tracker not reachable through the read-only GitHub integrations, the PR body is a one-line stack note, and no spec file exists in the repository or checkout. The Spec axis therefore has no confirmed material finding.

Reviewed independently by GLM (glm-5.3-flash) and DeepSeek (deepseek-v4-pro-0813) via Shipfox; verified and synthesized by GLM.

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1890 (comment)

[shipfox-ai]

Final review — PR #1890 (feat/read-alignment)

Fixed point 493035a → HEAD 81726d5. This PR replaces native-sequence fold alignment with fixed Raft-horizon alignment: routed reads now carry the local ReadIndex horizon R in the context, every aligned read verifies the main snapshot's durable applied index H >= R, and projection-backed queries wait for an independent Raft certificate >= H re-read from the exact snapshot the query uses. Audit reads get the same treatment independently, checkpoint readiness now certifies both projections, and main-store-only filter shapes (transaction ID, reverted, account-address, empty OR) no longer acquire a projection wait or lease. Docs were updated consistently, and the new behavior is well pinned by deterministic tests (barrier rejection, frozen-checkpoint certification, cancellation, selectivity, and horizon trimming).

I verified both candidate reports against diff.patch and the code. Two GLM candidates were rejected on inspection: the claimed auditFilterNeedsIndex nil-Or comment/behavior mismatch is factually wrong (this diff rewrote the comment; f.Or == nil returns true/conservative and an empty OrFilter{} returns false, exactly as the new comment and TestListAuditEntriesEmptyOrDoesNotDependOnAuditProjection state), and the e2e error-substring assertions in cross_store_value_skew_test.go are a deliberate, commented narrowing of Unavailable, not a defect. Independent inspection of the riskiest areas — the audit wait loop's snapshot lifecycle, the AggregateVolumes hold/snapshot split, the pin=0 main-store-only Compile path, the leaf classification against compile.go's actual storage dispatch (Reverted reads the FSM bitset, tx-ID and ACCOUNTS-address read the main Pebble reader), routed horizon propagation, and the existence of production WriteRaftProgress/WriteAuditRaftProgress writers in indexbuilder/auditindexer — found no additional correctness issue. No blocking finding remains.

Recommendation: approve with comments. All confirmed findings are maintainability/drift risks, none affects current behavior.

Standards

1. [P2] The H >= R linearizability guard is duplicated, byte-identical, in two places added by this diff. mainAppliedHorizon (internal/query/aligned_snapshot.go:36-48) and the inline copy in ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1735-1739) implement the same read-then-check, down to the error text "main-store snapshot applied index %d is behind ReadIndex horizon %d". The query helper is unexported, so the controller reimplemented it. This check is the core of the PR's linearizability proof; drift (e.g., one site dropping the < vs <= comparison or the error) would silently weaken one read path while tests pin only the other. AGENTS.md's DRY rule applies directly. Export query.MainAppliedHorizon (or equivalent) and call it from the controller.

2. [P3] The audit projection-state → domain-error mapping now exists twice. internal/application/admission/admission.go:877-884 (checkQueryCheckpointProjectionReady) and internal/application/ctrl/controller_default.go:1757-1763 (ListAuditEntriesFrom) both map disabled → domain.ErrAuditDisabled and rebuilding → &domain.ErrIndexBuilding{Index: "audit (rebuilding)"}. The frozen and ErrAuditProjectionUnavailable variants inside the controller's wait loop legitimately differ (checkpoint vs rebuild semantics) and are out of scope, but the shared (disabled, rebuilding) pair should come from one helper so the two callers cannot drift into different error contracts for the same projection state.

3. [P3] OpenReservedQueryHandle has no production caller. internal/query/aligned_snapshot.go:326 (plus the reserve parameter threaded through openQueryHandle at line 330) is exercised only by TestOpenReservedQueryHandleHoldsReclaimFloor in this tree; the doc comment defers to a future consumer. Speculative generality: inline it (or land the consumer in the same stack) until a real caller exists, so the reclamation-reservation contract isn't maintained against no production use.

4. [P3] Parallel switch trees hand-mirror compile's storage dispatch. filterUsesReadIndex (internal/query/aligned_snapshot.go:77) mirrors compile's per-leaf index-vs-main-store choice (the comment admits it "mirrors the storage choice made by compile"), and auditFilterNeedsIndex (internal/query/audit_filter.go:86) mirrors CompileAuditFilter. Parity is correct today — I verified the main-store-only classifications against the actual compilers (Reverted reads the FSM bitset, tx-ID reads the Pebble history zone, ACCOUNTS address matching reads the main reader) — and the table tests pin current leaves. The residual risk: a new filter leaf added to compile.go but not to both predicates would misclassify which reads wait for projection alignment, silently skipping (stale results) or adding (unavailable reads) the wait. Consider deriving the classification from the compiler itself or an exhaustive shared leaf table.

5. [P3] New bare _ = error discards lack the required justification comment. The new _ = auditSnap.Close() sites in ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1766, 1775, 1790, 1804) and the defer func() { _ = snap.Close() }() sites in AggregateVolumes (lines ~929, ~941) use intentional discards without the reason AGENTS.md ("use _ = ... with a justification comment when intentional") and docs/technical/contributing/conventions.md (// Best effort cleanup) ask for. Mitigated by the file's pervasive pre-existing bare-discard style — a one-comment fix, not a rework.

Spec

No spec is available for this PR; the spec axis was not reviewed.

---

Reviewed independently by GLM (glm-5.3-flash) and DeepSeek (deepseek-v4-pro-0813) via Shipfox; verified and synthesized by GLM.

[gfyrag]

All inline findings on the final head are now answered and resolved. Please refresh the automated review status against 81726d5.

[gfyrag]

Final ownership pass on 81726d5: fixed authoritative consistency docs/comments and projection selectivity; all inline threads are answered and resolved. The latest reclaim-hold claim was rejected with code evidence: callers retain ownership and release on every error or defer path. Canonical full+E2E+Schemathesis validation PASS; exact final review APPROVE (MEDIUM).

[shipfox-ai]

This PR (EN-1946) replaces the fold-sequence alignment of read/audit projections with a fixed main-store Raft horizon: reads now capture a durable LastAppliedIndex H, verify H >= R against the local ReadIndex result, and wait for each used projection's Raft certificate to reach H before opening the snapshot the query actually reads. I verified both candidate reports against the diff and the code, and independently re-checked the riskiest areas (checkpoint reads vs. the new H >= R check, snapshot lifecycle on every error path in ListAuditEntriesFrom, filterUsesReadIndex/auditFilterNeedsIndex parity with compile, and the e2e probe relaxation). The spec requirements are implemented faithfully; no blocking issue was found. Recommendation: approve with comments — the findings below are minor or judgement-call items worth addressing in this or a fast-follow PR.

Standards

1. Duplicated H >= R linearizability guard (judgement call — highest impact). mainAppliedHorizon (internal/query/aligned_snapshot.go:36) is unexported, so ListAuditEntriesFrom reimplements the check inline at internal/application/ctrl/controller_default.go:1742-1745, including the byte-identical error "main-store snapshot applied index %d is behind ReadIndex horizon %d". Both copies were added by this diff. Drift between them would silently weaken the barrier on one read path (the audit path cannot fall back to a weaker check — it is the alignment proof). Export and reuse the helper.

2. Minor documented-standard breach: unjustified _ = error discards. AGENTS.md ("Do not ignore errors. Handle them explicitly, or use _ = ... with a justification comment when intentional") and docs/technical/contributing/conventions.md (// Good - explicit discard with reason) require a reason on intentional discards. New sites added here carry none: controller_default.go:929,941 (defer func() { _ = snap.Close() }() in the two new AggregateVolumes branches) and controller_default.go:1766,1775,1790,1804 (_ = auditSnap.Close() in ListAuditEntriesFrom). Mitigating: the file already uses identical bare discards (e.g. lines 1051, 1652) and AGENTS.md also says to prefer existing patterns, so impact is low — add // Best effort cleanup.

3. Duplicated audit projection-state → error mapping (judgement call). internal/application/admission/admission.go:876-881 and controller_default.go:1754-1759 both map disabled → domain.ErrAuditDisabled / rebuilding → &domain.ErrIndexBuilding{Index: "audit (rebuilding)"}. This mapping is load-bearing for the FailedPrecondition-vs-Unavailable contract and is table-tested on both sides, but a shared classifier would keep the kinds/reasons from drifting.

4. Hand-mirrored compile dispatch (judgement call, low). filterUsesReadIndex (aligned_snapshot.go:69-113) and auditFilterNeedsIndex (audit_filter.go) re-encode the compiler's per-leaf storage decisions. I verified parity arm-by-arm against compile.go today (Address→main store on ACCOUNTS vs. AddressTxIterator on TRANSACTIONS, TX_BUILTIN_INDEX_ID→main history, Reverted→FSM bitset, all other leaves→index, unknown/malformed leaves fail loud in Compile), and the table tests pin it — but a compiler-only change to one leaf's storage path would silently skip or add a projection wait. Derive the classification from compilation or add an exhaustive parity test.

5. Speculative generality (low). OpenReservedQueryHandle (aligned_snapshot.go:326, plus the reserve parameter on openQueryHandle and the queryHandleStore widening of OpenQueryHandle) has no production call site in this diff — only its own test. Fine if a consumer lands in a stacked PR; otherwise inline it until needed.

Checked and clean: new tests use t.Parallel() and no time.Sleep; the e2e value-skew probe relaxation keeps the served > 0 floor and narrows accepted errors to the genuinely changed production trigger (unbounded alignment waits now need a per-probe deadline); docs were updated in the same change as required.

Spec

No confirmed material finding. All five PR-body requirements are implemented faithfully and were verified in code:

• Fixed main-store horizon with certificate consumption, never chasing the moving head (AlignedIndexSnapshot in internal/query/aligned_snapshot.go; mirrored for audit in ListAuditEntriesFrom), re-read through the very snapshot the query uses.
• Alignment is owed only for projections the query actually consults (AlignmentOwed/filterUsesReadIndex, AuditFilterNeedsIndex/auditFilterNeedsIndex), pinned by TestAlignmentOwedWaitsOnlyForUsedReadProjection, TestAggregateVolumesMainStoreOnlyFilterDoesNotWaitForReadProjection, and TestAuditFilterNeedsIndex.
• The local ReadIndex horizon is propagated by RoutedController.withLocalBarrierHorizon (controller_routed.go) only for locally-served reads and consumed by the H >= R check; checkpoint reads bypass the routed barrier entirely (readController returns a checkpoint-scoped local controller), so the check cannot spuriously fail a frozen read.
• Disabled audit projection maps to FailedPrecondition (KindPrecondition) and rebuilding to retryable Unavailable (KindUnavailable), per domain/errors.go and tests on both the admission and controller paths.
• Main-horizon trimming (MainHorizonKeep) and audit hiSeq clamping are preserved; every error path in ListAuditEntriesFrom closes auditSnap, and success wraps both in joinedCloser{auditSnap, handle} (TestJoinedCloserClosesEverySnapshotAndJoinsErrors, TestListAuditEntriesUsesAlignedAuditSnapshotAndMainHorizon).

One behavior tightening was examined and judged in-scope rather than scope creep: a frozen checkpoint whose projection certificate lags its applied index now fails explicitly ("frozen read projection … behind main checkpoint horizon" / ErrIndexBuilding{"audit checkpoint"}) instead of being served with a possibly incomplete frozen projection. This is a direct, documented consequence of the new checkpoint-readiness contract (the builder publishes .ready only after certifying), so it is defensive, not a deviation.

Reviewed independently by GLM (glm-5.3-flash) and DeepSeek (deepseek-v4-pro-0813) via Shipfox; verified and synthesized by GLM.

[shipfox-ai]

This PR fixes the main read/audit horizon at the already-open main snapshot (H), verifies H >= R against the ReadIndex result, gates each projection on its own Raft certificate, and propagates the routed horizon through RoutedController.withLocalBarrierHorizon — with selectivity (AlignmentOwed/filterUsesReadIndex, AuditFilterNeedsIndex) ensuring queries never wait on projections they don't read. I verified all five PR-body bullets are implemented and that the risky areas hold up: the audit alignment loop re-reads the certificate through the snapshot it vouches for; the audit snapshot is transferred to the closing cursor (joinedCloser); checkpoint reads bypass the routed controller so the H >= R check cannot false-fail on stale checkpoints; live index candidates are trimmed to the main audit horizon; and lease releases are idempotent. The candidate claims that unknown filter shapes could silently serve stale results (they cannot today — Compile rejects every unmapped kind loudly via the exhaustive generated validity table) and that the audit snapshot lacks GC protection (the event GC sweeps only the metadata/exists zones, never the audit index) were both checked against the code and rejected. Recommendation: approve with comments — no blocking issues; the findings below are judgement calls.

Standards

1. Duplicated Code / drift risk (medium, most actionable) — filterUsesReadIndex (internal/query/aligned_snapshot.go:77-116) re-encodes, per its own comment, the storage choice Compile makes, and its default: return false (line 115) is non-conservative — the audit counterpart auditFilterNeedsIndex defaults to true. I verified the mirror is currently exact (every oneof arm is covered; unknown kinds fail loudly at rejectInvalidCondition against the exhaustive generated table, backed by TestConditionKindsCoverEveryOneofArm), so no stale reads are possible today. But a future indexed leaf registered in the validity table and Compile's switch without updating this predicate would silently skip the alignment wait and compile the leaf against a plain main-store snapshot (list_entities.go:147-156, controller_default.go AggregateVolumes else-branch) — silent stale results rather than a wait or an error. Combined with the two other cascades over the same oneof (auditFilterNeedsIndex's new Or case, Compile's dispatch), consider one shared "which leaves use the index" predicate both sites consult.
2. Duplicated Code (low) — ListAuditEntriesFrom re-implements query.mainAppliedHorizon inline (internal/application/ctrl/controller_default.go:1735-1742), including the identical error string "main-store snapshot applied index %d is behind ReadIndex horizon %d" (internal/query/aligned_snapshot.go:36-50). mainAppliedHorizon is unexported, so ctrl cannot reuse it; export it (or move the check) so the horizon proof stays single-sourced.
3. Duplicated Code (low) — the disabled/rebuilding audit gate is verbatim in two places: internal/application/admission/admission.go:874-882 and internal/application/ctrl/controller_default.go:1754-1760, both constructing &domain.BusinessError{Err: domain.ErrAuditDisabled} / &domain.ErrIndexBuilding{Index: "audit (rebuilding)"}. Extract one shared readiness helper.
4. Test-induced guard inconsistency (low) — internal/bootstrap/controller_routed.go:326 adds && b.Node != nil && b.Logger() != nil to ListAccounts so the new test can inject a zero-value &node.Node{}, while the sibling paths (GetTransaction line 248, GetAccount lines 305-306) still call b.Node.Logger() unguarded. Production never has a nil Node; the defensive check is applied to one path only to serve a test.
5. Weakened failure-path discrimination in an e2e probe (low) — tests/e2e/business/cross_store_value_skew_test.go:148 accepts any codes.DeadlineExceeded with no message assertion, while the Unavailable arm (lines 149-153) is properly narrowed by substrings. Per testing.md's "failure-path assertions must distinguish the intended branch", a retry/timeout bug elsewhere surfacing as DeadlineExceeded would pass the probe. The intent is documented inline, so this is a judgement call, not a hard breach.

Spec

All five PR-body bullets are implemented as specified: fixed horizon H with the H >= R check in AlignedIndexSnapshot and ListAuditEntriesFrom; selectivity via AlignmentOwed/filterUsesReadIndex and AuditFilterNeedsIndex/empty-OR; horizon propagation through RoutedController.withLocalBarrierHorizon (local reads only; remote hops re-barrier); disabled = permanent FailedPrecondition vs rebuilding = retryable Unavailable, pinned by tests asserting KindPrecondition/KindUnavailable; MainHorizonKeep trimming and snapshot/lease lifecycles preserved (audit snapshot handed to the closing cursor). No missing requirements. Two confirmed notes:

1. Frozen audit checkpoint reports a retryable error for an unfixable condition (low) — internal/application/ctrl/controller_default.go:1777 returns &domain.ErrIndexBuilding{Index: "audit checkpoint"} (KindUnavailable) when a frozen store's audit certificate lags H. A frozen snapshot never advances, and the read-projection counterpart returns a plain non-retryable error instead (internal/query/aligned_snapshot.go:219-223). The retryable class mislabels a permanent condition and invites pointless retries; the spec's disabled-vs-rebuilding split does not cover this third case.
2. Speculative surface (low) — query.OpenReservedQueryHandle (internal/query/aligned_snapshot.go:321-327) is new exported API with no caller in this diff; only its own test exercises it (aligned_snapshot_test.go:367). Presumably consumed later in the EN-1946 stack (3/8), but on this branch it is unused surface.

Reviewed independently by GLM (glm-5.3-flash) and DeepSeek (deepseek-v4-pro-0813) via Shipfox; verified and synthesized by GLM.