refactor(api): remove public minimum log sequence (EN-1946)

[gfyrag]

Summary

Stack 7/7 for EN-1946. This final PR removes the unreleased client-facing min_log_sequence surface after the preceding stack has made Raft applied index the common projection horizon.

• removes min_log_sequence from protobuf, HTTP/OpenAPI, CLI, routing, errors, tests, and authoritative docs
• sequentially realigns unreleased protobuf field and enum numbers without reserved declarations
• retains AuditSuccess.min_log_sequence / max_log_sequence and all internal native log/audit cursors
• retains stale; removing stale is explicitly deferred to a second decision/ticket
• removes the superseded audit min-log workaround while preserving certified audit alignment

Stack

1. feat(reads): certify projection Raft progress (EN-1946) #1889 — projection Raft certificates and checkpoint readiness
2. feat(reads): align live queries to Raft horizons (EN-1946) #1890 — live query alignment
3. fix(reads): pin prepared query definition to snapshot (EN-1946) #1894 — prepared-query snapshot pinning
4. fix(reads): align index inspection to Raft horizon (EN-1946) #1891 — InspectIndex alignment
5. refactor(reads): remove leader consistency selector (EN-1946) #1893 — remove leader consistency
6. test(antithesis): use certified projection horizons (EN-1946) #1892 — migrate Antithesis projection assertions
7. refactor(api): remove public minimum log sequence (EN-1946) #1881 — remove public min_log_sequence (this PR)

Jira: https://formance-team.atlassian.net/browse/EN-1946

Validation

Targeted Go 1.26 package tests, the complete Antithesis workload module, and scripts/agent-check pass on the reconstructed final tree. Fresh per-PR validation and review are being run after the split.

Scope notes

Usage projections remain deliberately eventual and outside the common horizon. The independent routing/receipt fixes from #1876 remain in the target branch.

[NumaryBot]

✅ Approve — automated review

The public minimum-log-sequence surface is removed consistently, including generated protobuf code and affected callers. No actionable correctness defects were found.

No findings.

[codecov]

Codecov Report

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

Additional details and impacted files

@@                               Coverage Diff                               @@
##           test/en-1946-antithesis-projection-horizons    #1881      +/-   ##
===============================================================================
+ Coverage                                        77.37%   77.49%   +0.12%     
===============================================================================
  Files                                              458      458              
  Lines                                            48632    48550      -82     
===============================================================================
- Hits                                             37629    37626       -3     
+ Misses                                            7839     7782      -57     
+ Partials                                          3164     3142      -22     

Flag	Coverage Δ
e2e	77.49% <100.00%> (+0.12%)	⬆️
scenario	77.49% <100.00%> (+0.12%)	⬆️
unit	77.49% <100.00%> (+0.12%)	⬆️

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

[NumaryBot]

🛑 Changes requested — automated review

No runtime correctness defect was identified. The remaining issue is authoritative gRPC documentation that still describes the removed API.

Findings outside the diff

🟠 [major] Update the gRPC API documentation — misc/proto/common.proto:1846

Removing this field leaves docs/technical/architecture/subsystems/api/grpc-api.md:654-686 with an uncompilable example using ReadOptions.MinLogSequence and a false description of its consistency behavior. Update that authoritative API documentation as required by AGENTS.md:82-89.

Summary: #1881 (comment)

[NumaryBot]

NumaryBot review complete: no remaining inline findings.

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

Summary: #1881 (comment)

[NumaryBot]

🛑 Changes requested — automated review

The implementation appears correct, but the authoritative gRPC documentation still describes and demonstrates the removed API field. This is a non-blocking documentation defect.

Findings outside the diff

🔴 [blocker] Update the gRPC guide for the removed field — misc/proto/common.proto:1763

Removing this field leaves docs/technical/architecture/subsystems/api/grpc-api.md:654-686 with an uncompilable MinLogSequence example and obsolete consistency semantics. Update that authoritative guide as required by AGENTS.md:82-89.

Summary: #1881 (comment)

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1881 (comment)

[shipfox-ai]

This PR removes the public min_log_sequence surface (protobuf, HTTP/OpenAPI, CLI, routing, errors, tests) in favor of automatic per-projection certification against a fixed Raft horizon. The code removal itself is thorough and correct: I verified zero client-facing remnants of MinLogSequence in the gRPC/HTTP adapters, CLI, and error taxonomy; the ErrorReason renumbering (63–67) is gap-free and consistent across common.proto and all four generated flavors; AuditSuccess.minLogSequence and the internal native cursors are retained as specified; stale is retained; and alignment coverage survives the deleted workaround tests at the controller layer. However, the PR left two authoritative documentation surfaces contradicting its own new contract — one of them the gRPC API guide, which still documents and demonstrates the removed field.

Recommendation: request changes (documentation-only fixes; no code changes required).

Standards

1. [Hard] internal/adapter/http/handlers_list_audit_entries.go:35-45 — doc comment edited by this PR still asserts the removed best-effort contract. The updated comment claims HTTP filtered audit reads "always perform a live, best-effort read", "may transiently omit very recent entries that have not yet been indexed", and that "a client needing a pinned or consistency-bounded audit read must use the gRPC surface". This is now factually wrong: both surfaces share the same controller path (RoutedController.ListAuditEntries → ListAuditEntriesFrom, which verifies the audit projection's certificate against the fixed main-store horizon — controller_default.go:1802), and this PR's own openapi.yml (listAuditEntries description) and http-api.md:401-414 now document certified alignment that "fails explicitly rather than returning a partial page" with a guarantee "matching the gRPC surface". The sentence "This HTTP endpoint intentionally does not expose either" is also broken — only one gRPC-only option (checkpointId) remains. Violates AGENTS.md documentation maintenance for behavior changes.

2. [Minor] misc/proto/bucket.proto:1315-1316 (mirrored at internal/proto/servicepb/bucket.pb.go:7985-7986) — garbled comment in a block this PR rewrote. "The value does NOT identify which wait: a / a failed ReadIndex attempt…" — the leftover article leaves a broken sentence in the QueryProfile.barrier_duration_us doc.

3. [Minor] cmd/ledgerctl/cmdutil/flags.go:115-116 — stale comment adjacent to this PR's edit. BuildReadOptions still says "Returns nil when both fields are zero", but ConsistencyFlags now carries only CheckpointID.

Judgement-call candidates (duplicated barrier rationale across four files, the 60-file blast radius, the deleted server_bucket_audit_consistency_test.go) were reviewed and rejected: they are mandated by the repo's own synchronization standards or are legitimate dead-test removal with coverage preserved (controller_default_audit_alignment_test.go covers cancellation, disabled/rebuilding projections, and horizon trimming), with no correctness, security, compatibility, or test-risk impact.

Spec

1. [Missing requirement] docs/technical/architecture/subsystems/api/grpc-api.md:650-686 — the authoritative gRPC API guide still documents the removed MinLogSequence contract. The "Audit Entries" section's example still constructs commonpb.ReadOptions{MinLogSequence: lastWrittenSequence} (line 660) — a field that no longer exists on the generated ReadOptions, so the example does not compile — and the prose (lines 680-686) still describes the non-zero/zero minimum-sequence wait semantics and "Checkpoint reads ignore the bound". The spec bullet "removes min_log_sequence from protobuf, HTTP/OpenAPI, CLI, routing, errors, tests, and authoritative docs" is therefore only partially fulfilled, and this guide now contradicts the rewritten http-api.md, which states the HTTP guarantee "matches the gRPC surface". Resolution: drop the field from the example and describe the automatic fixed-horizon audit projection alignment, including checkpoint behavior.

Otherwise the spec is fully implemented: all five PR-body bullets were verified against the diff and checkout (field and error removal, gap-free renumbering without reserved, retention of AuditSuccess.min/max_log_sequence and internal cursors, stale retention, and removal of the superseded audit workaround with certified alignment preserved). The stale HTTP audit-handler comment overlapping finding 1 above is reported under Standards, where it belongs.

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

[shipfox-ai]

This PR removes the public min_log_sequence consistency control (EN-1946) across protobuf, gRPC/HTTP/CLI surfaces, error codes, tests, and docs, replacing it with automatic fixed-Raft-horizon projection certification. I verified the code changes against the diff and the tree: the field removal and sequential renumbering (ExecutePreparedQueryRequest.mode = 6, AggregateVolumesRequest 3–6, ErrorReason 63–67) comply with AGENTS.md's unreleased-v3 rules (no reserved, no hand-rolled wire sites affected), generated code and mocks are consistently regenerated, the audit-native AuditSuccess/AppliedProposal fields and ConsistencyStale are correctly retained, the certified alignment path is implemented (DefaultController.ListAuditEntriesFrom waits on the audit projection's Raft certificate covering the fixed main-store horizon), and the deleted handler-layer workaround tests are covered by the replacement controller-level alignment tests. All five spec bullets are implemented correctly in code. However, the PR is not complete on the documentation axis: one authoritative architecture guide still documents the deleted field with example code that no longer compiles, and several HTTP consistency contracts still describe the removed best-effort semantics. Recommendation: request changes — the code itself is sound and approved; merge after the documentation fixes below.

Standards

1. Stale authoritative gRPC guide documents the removed ReadOptions.MinLogSequence API. docs/technical/architecture/subsystems/api/grpc-api.md:655-664 still shows a live example constructing Read: &commonpb.ReadOptions{MinLogSequence: lastWrittenSequence}, and the paragraph at grpc-api.md:681-686 explains how "a non-zero MinLogSequence makes every gRPC node … wait for its log index to reach the bound and for its local audit index to reach the live audit head". commonpb.ReadOptions no longer has this field (the only remaining field is checkpoint_id), so the example no longer compiles against the current proto and the prose describes wait semantics this PR deletes — while the sibling http-api.md was updated. This violates AGENTS.md "Documentation maintenance" ("Documentation is part of the change when behavior, architecture, interfaces, CLI, or APIs change"). Fix: rewrite the example without Read options and replace the bound-wait paragraph with the fixed-horizon certification wording already used in query-pipeline.md and indexer.md.

2. Stale HTTP consistency contracts understate the new guarantees and point clients at a removed gRPC capability. The diff edits these comments/descriptions but leaves them describing the pre-PR behavior: internal/adapter/http/handlers_list_audit_entries.go:34-45 still says the endpoint "always performs a live, best-effort read" whose indexed filters "may transiently omit very recent entries" and that clients needing a consistency-bounded audit read "must use the gRPC surface"; internal/adapter/http/handlers_list_transactions.go:30-38 and internal/adapter/http/handlers_list_signing_keys.go:9-13, plus openapi.yml:678-681 and openapi.yml:2346-2349, likewise retain "live, best-effort read" and direct callers to gRPC for "consistency-bounded" reads. This contradicts actual behavior: the routes run through RoutedController.readCtrl (internal/bootstrap/controller_routed.go:59-86), which performs the default linearizable ReadIndex barrier, and the audit route additionally waits for the audit projection's certificate covering the fixed main-store horizon — the updated openapi.yml audit section itself now states "The consistency guarantee matches the gRPC surface". They also contradict the removal of every gRPC consistency bound except checkpoint pinning, so "use gRPC for consistency-bounded reads" now sends clients after a capability that no longer exists. This violates "Interface/behavior change: update relevant code comments" and the openapi.yml synchronization rule. Fix: rewrite the affected comments and OpenAPI descriptions to state that live reads are linearizable with automatic projection alignment, and that the only remaining explicit option is checkpoint selection.

No other confirmed material standards finding. The protobuf renumbering, mock regeneration, error-code cleanup, and remaining docs/drafts/** mentions of min_log_sequence (non-authoritative per AGENTS.md) were all verified clean. GLM's judgement-call smells (single-field ConsistencyFlags, broad cross-file edit) are style observations with no correctness or compatibility impact and are not retained.

Spec

1. Partial: "removes min_log_sequence from … authoritative docs" — one authoritative doc was missed. docs/technical/architecture/subsystems/api/grpc-api.md:655-686 (not touched by the diff) still exposes the deleted request field in a runnable-looking example and documents the removed log/audit bound-wait behavior. Against HEAD the sample is a build error in example code, not just stale prose, and clients are told to rely on a consistency control this PR deletes. Update this section to describe automatic fixed-horizon audit alignment and checkpoint selection only. (Same root cause as Standards Fix typo #1.)

2. Partial: the changed HTTP audit-handler comment contradicts the preserved certified alignment. internal/adapter/http/handlers_list_audit_entries.go:34-45 still describes filtered audit reads as best-effort reads that "may transiently omit" entries and implies projection catch-up is a gRPC-only option. That contradicts this PR's own updated openapi.yml audit contract and the implemented behavior (DefaultController.ListAuditEntriesFrom, internal/application/ctrl/controller_default.go:1785-1840, waits for a projection certificate covering the fixed main-store horizon and fails explicitly on disabled/rebuilding/cancelled/deadline). Leaving the obsolete contract in place defeats the spec's intent to remove the superseded audit min-log workaround while preserving certified alignment, and leaves maintainers with the wrong behavior model. (Same root cause as Standards update spec #2, spec-shaped.)

All other spec bullets verified passing: full code removal of min_log_sequence (zero remaining references to the CLI flag, HTTP body field, ReadOptions/request fields, ErrReadIndexNotCaughtUp, or READ_INDEX_NOT_CAUGHT_UP); compliant sequential renumbering with no reserved reintroduction; correct retention of AuditSuccess.min_log_sequence/max_log_sequence and internal cursors; ConsistencyStale retained with routing support; and the superseded workaround removed while certified alignment and its regression coverage (controller_default_audit_alignment_test.go) remain. No scope creep found.

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

[NumaryBot]

NumaryBot review complete: no remaining inline findings.

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

Summary: #1881 (comment)

[NumaryBot]

NumaryBot review complete: no remaining inline findings.

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

Summary: #1881 (comment)

[shipfox-ai]

This PR completes EN-1946 by removing the client-facing min_log_sequence request surface from the protobufs, gRPC/HTTP adapters, CLI, error taxonomy, tests, and authoritative docs; sequentially realigning the affected protobuf fields/enums (63–67, mode = 6, AggregateVolumesRequest 3–6) without reserved; and replacing the superseded handler-level audit wait workaround with a controller-level fixed-Raft-horizon alignment (audit projection certification, disabled/rebuilding failure, horizon trimming, checkpoint-creation gating in internal/application/indexbuilder/process_logs.go:320). I independently verified the renumbering is gap-free and consistently regenerated across all pb/vtproto/reader/dethash flavors, that AuditSuccess.min_log_sequence and the internal proposal cursors are retained, that stale remains wired (internal/adapter/grpc/consistency.go:17), and that the live read path still guarantees the main store has caught up to the ReadIndex horizon (ReadIndexAndWait includes WaitForApplied; ListAuditEntriesFrom defensively re-checks at internal/application/ctrl/controller_default.go:1808). Recommendation: approve with comments — one minor stale comment below.

Standards

[Minor] Stale "which wait" phrasing left in a comment this PR touched — misc/proto/bucket.proto:1316, mirrored at internal/proto/servicepb/bucket.pb.go:7985, and retained at internal/query/profile.go:112. The rewritten QueryProfile.barrier_duration_us comment still reads "The value does NOT identify which wait: a failed ReadIndex attempt … is what triggers the fallback." This PR removed the only other wait (min_log_sequence catch-up — AddBarrierWait now has exactly one production caller, the ReadIndex barrier in internal/bootstrap/controller_routed.go:80), so there is no plurality left to disambiguate and the sentence contradicts its own follow-up clause. Every sibling public surface was fully rewritten (docs/technical/architecture/subsystems/read-path/query-profile.md, openapi.yml barrierDurationUs), making the authoritative wire comment inconsistent with them. This violates AGENTS.md line 89 ("Interface/behavior change: update relevant code comments") — the comment was edited but not fully updated. Impact: an operator consulting the wire docs to interpret a non-zero barrier_duration_us on a forwarded read is pointed at a disambiguation that no longer exists. Fix is a one-line edit dropping "which wait" at all three locations.

No other standards finding: the 61-file blast radius is mandated by AGENTS.md's synchronized protobuf/generated-code/mocks/docs requirements, the renumbering follows the repo's no-reserved policy, mocks are mockgen-generated, edited tests use t.Parallel() with no sleeps, and docs maintenance (openapi.yml, api-comparison.md, cli.md, grpc-api.md, http-api.md, plus the traceability ADR whose read-snapshot-consistency.md link resolves) is satisfied.

Spec

No confirmed material finding. All five spec bullets were verified against the diff and the checkout: (1) min_log_sequence removal is complete across protobuf, HTTP/OpenAPI, CLI, routing, errors, and authoritative docs, with the only remaining minLogSequence hit in openapi.yml being the spec-mandated AuditSuccess response field; (2) field/enum realignment is sequential and gap-free with no reserved; (3) AuditSuccess.min_log_sequence/max_log_sequence and internal audit/proposal cursors are untouched; (4) stale is retained and its deferral is recorded in the EN-1946 decision section of product-technical-traceability.md; (5) waitMinLogSequence/waitFilteredAuditConsistency and the handler-level test file are deleted, replaced by DefaultController.ListAuditEntriesFrom alignment covered by controller_default_audit_alignment_test.go, with checkpoint creation gated on the audit projection and usage deliberately left eventual per the scope note. No missing/partial requirement, no scope creep, and no incorrectly implemented behavior. Two non-blocking cleanups: readstore.WaitForSequence/WaitForAuditSequence are now production-unused (test-only), and internal/query/profile.go:112 carries the same vestigial "which wait" wording noted above.

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

[shipfox-ai]

Review summary — PR #1881 (remove public minimum log sequence, 81556e9 → 495e20f)

This PR removes the client-facing min_log_sequence surface (protobuf ReadOptions/ExecutePreparedQueryRequest/AggregateVolumesRequest, gRPC/HTTP handlers, CLI flags, the READ_INDEX_NOT_CAUGHT_UP error reason, and the corresponding handler-level waits and tests) in favor of the controller-level fixed Raft projection-horizon alignment (EN-1946). I verified every candidate finding from both independent reviews against the diff and the code, and independently inspected the riskiest areas: field renumbering in the generated vtproto/dethash codecs, the DefaultController.ListAuditEntriesFrom alignment/verification path, RoutedController horizon propagation, the deleted Antithesis driver's registration and ledger prefix, and all residual min_log_sequence/MinLogSequence/READ_INDEX_NOT_CAUGHT_UP references. The change is faithful to its scope: renumbering is sequential with no reserved reintroduced (AGENTS.md rule), retained surfaces (AuditSuccess.min/max_log_sequence, AppliedProposal.MinLogSequence, readstore native cursors, stale consistency, usage projection carve-out) are untouched, the controller now verifies audit-projection Raft progress against the main snapshot's applied index with its own tests, and no orphaned references or registration entries remain. One minor documented-standards finding was confirmed; nothing material on the spec axis.

Recommendation: approve with comments (single minor, non-blocking comment cleanup).

Standards

Minor — stale code comment left behind by the interface change

• internal/query/audit_filter.go:68-69 — the doc comment on AuditFilterNeedsIndex still reads "The gRPC handler uses it before the consistency wait, so compiling first would capture stale index candidates before the barrier intended to make them fresh." This PR deleted both the gRPC handler call site and the pre-routing consistency wait (waitMinLogSequence/waitFilteredAuditConsistency in internal/adapter/grpc/server_bucket.go); the sole production caller is now DefaultController.ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1821), which applies the predicate after opening the main snapshot and waiting for the Raft-horizon-aligned audit snapshot. This violates the documented rule "Interface/behavior change: update relevant code comments" (AGENTS.md §Documentation maintenance). Impact: the comment directs maintainers to a call site that no longer exists and describes a wait mechanism that was removed, which can mislead future work on the audit read path.

All other documented standards check out: sequential proto realignment without reserved (AGENTS.md:10), regenerated mocks for the changed Controller/Backend interfaces, and authoritative doc maintenance (docs/ops/cli.md, openapi.yml, api-comparison.md, subsystem docs). The judgement-call "Speculative Generality" candidate on cmd/ledgerctl/cmdutil/flags.go (ConsistencyFlags reduced to one field, signing/list.go passing an empty struct) was rejected: it is a style preference with no correctness, security, compatibility, or test-risk impact, and the type still mirrors the documented shared-flag contract.

Spec

No confirmed material finding. All requirements verified against the checkout:

• R1 (removal everywhere): no min_log_sequence/MinLogSequence remains in commonpb/servicepb generated code, gRPC/HTTP adapters, the CLI, or the executor; the READ_INDEX_NOT_CAUGHT_UP error reason, its conversion branch, and the handler audit-consistency tests are fully removed. Remaining mentions are limited to the retained AuditSuccess range, internal applied-proposal/native-cursor concepts, historical "removed by EN-1946" notes in authoritative docs, and docs/drafts/prepared-queries.md (non-authoritative per AGENTS.md).
• R2 (sequential realignment, no reserved): ExecutePreparedQueryRequest.mode 7→6, AggregateVolumesRequest fields 4–7→3–6, ErrorReason 63 deleted with 64–68 re-packed to 63–67 — consistent across .proto, .pb.go, vtproto wire tags, and dethash; no reserved reintroduced in misc/proto/.
• R3 (retentions): AuditSuccess.min_log_sequence = 1 intact (misc/proto/audit.proto:97, openapi.yml AuditSuccess schema); AppliedProposal.MinLogSequence and readstore WaitForSequence/WaitForAuditSequence/NotifyProgress retained and still used by the controller alignment path.
• R4 (stale retained): stale handling and documentation untouched; removal explicitly deferred.
• R5 (workaround removed, alignment preserved): waitFilteredAuditConsistency/waitMinLogSequence deleted; RoutedController.ListAuditEntries still propagates the ReadIndex horizon (withLocalBarrierHorizon), and ListAuditEntriesFrom verifies the audit projection's Raft progress against the main snapshot's durable applied index, failing explicitly on disabled/rebuilding projections — with controller-level tests retained. Deleting the handler-level regression tests is acceptable under the additive-regression rule since their production trigger was removed, and the deleted Antithesis driver left no orphaned registration or ledger prefix.

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

[shipfox-ai]

Both independent reviews of PR #1881 ("refactor(api): remove public minimum log sequence", EN-1946) were verified against diff.patch (d0271c3...1f6effe1) and the head checkout. The removal itself is faithful and complete across protobuf, generated code, HTTP/OpenAPI, CLI, gRPC routing, error types, and authoritative docs: ReadOptions.min_log_sequence and the corresponding fields on AggregateVolumesRequest/ExecutePreparedQueryRequest are deleted and realigned sequentially with no reserved declarations (exactly as AGENTS.md mandates for unreleased v3), ErrReadIndexNotCaughtUp / ERROR_REASON_READ_INDEX_NOT_CAUGHT_UP are gone repo-wide, the retained AuditSuccess.min_log_sequence / AppliedProposal.MinLogSequence internal audit cursors are untouched with live callers, no dangling references to the deleted waitMinLogSequence/waitFilteredAuditConsistency helpers or the removed Antithesis parallel_driver_minlogseq driver remain, and the deleted error path is soundly replaced by context-bound alignment waits (WaitForRaftProgress). However, one call site was missed when the Controller.ListAuditEntries signature dropped its minLogSequence parameter, which breaks compilation of the internal/application/ctrl test package. Recommendation: request changes — a one-line fix, but required validation cannot pass as-is.

Standards

1. [Blocking] internal/application/ctrl test package no longer compiles — stale sixth argument in TestListAuditEntriesRejectsMainSnapshotBehindReadBarrier.
   internal/application/ctrl/controller_default_audit_alignment_test.go:58 still calls ctrl.ListAuditEntries(ctx, 10, 0, nil, false, 0) — six arguments — while the PR changed the signature to five (internal/application/ctrl/controller.go:55, internal/application/ctrl/controller_default.go:1772). Every other 6-arg call in this file was updated by the diff (lines 147, 156, 167, 172, 184, 203); this one was missed. Go has no default arguments, so go test ./internal/application/ctrl fails with "too many arguments in call to ctrl.ListAuditEntries", violating the PR checklist's "code compiles / tests pass" requirement and blocking the retained audit-alignment regression tests from running at all.
   Fix: remove the trailing 0 at line 58 (the test's "behind ReadIndex horizon" assertion is unaffected) and rerun the package tests plus the canonical PR validation.

No other Standards findings: the protobuf realignment, documentation updates, retained internal cursors, and deleted-test/driver cleanup were all verified compliant with the repo's documented standards.

Spec

1. [Blocking] The minLogSequence removal from tests is incomplete, breaking required validation.
   Same root cause as the Standards finding above: the spec bullet "removes min_log_sequence from protobuf, HTTP/OpenAPI, CLI, routing, errors, tests, and authoritative docs" claims targeted package tests pass, but internal/application/ctrl/controller_default_audit_alignment_test.go:58 still passes the removed minLogSequence argument, so the affected package cannot build and the PR's stated validation cannot have run against this tree.
   Fix: drop the sixth argument at line 58 and rerun go test ./internal/application/ctrl plus scripts/agent-check.

No other Spec findings: all five PR-body bullets were verified against the diff — the retained stale consistency surface, the retained AuditSuccess/AppliedProposal cursors (including openapi.yml's AuditSuccess.minLogSequence at line 5522 and the CLI/ledgerctl audit display), and the enum/field renumbering all match the stated scope. The deletion of server_bucket_audit_consistency_test.go and the parallel_driver_minlogseq driver falls under "removes min_log_sequence from … tests"; no registration or config referencing them lingers, and the surviving behavioural properties (disabled/rebuilding projection failure, cancellation, aligned snapshot, horizon trimming) remain covered in controller_default_audit_alignment_test.go and controller_default_audit_horizon_test.go.

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 — formancehq/ledger PR #1881 (refactor(api): remove public minimum log sequence, EN-1946)

This PR removes the public min_log_sequence read option across protobuf, gRPC/HTTP/CLI surfaces, routing, errors, tests, and docs, with sequential field/enum realignment per the repo's unreleased-v3 policy. The removal itself is faithful and consistent: no client-facing min_log_sequence survives (the remaining AuditSuccess/AppliedProposal fields are intentionally retained internals), the deleted ErrReadIndexNotCaughtUp error path has no leftover references, the renumbered messages (ExecutePreparedQueryRequest, AggregateVolumesRequest, ReadOptions) are not among the hand-rolled wire-decode sites listed in docs/technical/contributing/protobuf.md, mocks were regenerated, and the superseded audit min-log workaround and its 420-line test file were removed together. However, one stale test call site was missed, leaving the controller test package uncompilable, and the ErrorReason renumbering contradicts an explicit in-file contract. Recommendation: request changes (single-line fix required before merge).

Standards

1. ErrorReason renumbering violates the enum's own documented contract — misc/proto/common.proto:1136 (and the mirrored enum in misc/proto/bucket.proto's include of common.proto) states: "Do NOT renumber or remove values; append new ones." That rule exists because ErrorReason is wire-bound and persisted inside AuditFailure/IdempotencyFailure (authoritative audit-chain data), with the error kind re-derived at replay and check time via domain.KindForReason. This PR removes ERROR_REASON_READ_INDEX_NOT_CAUGHT_UP (formerly 63) and shifts ERROR_REASON_STALE_CLUSTER_POLICY…ERROR_REASON_CHECKPOINT_NOT_FOUND from 64–68 down to 63–67 across common.proto, commonpb/*.pb.go, and the raw descriptor. Under AGENTS.md's v3-unreleased/state-wipe policy there is no cross-build compatibility burden, and the name↔domain-string mapping remains mechanical, so this is not a correctness bug within a single build — but the in-file contract was neither honored nor updated, and it now documents the opposite of what the code does. Either keep the enum append-only (gap instead of renumber, accepting the tension with AGENTS.md's "no gaps" field rule, which speaks to fields, not enum values) or update the comment in the same commit to record the override and confirm no persisted audit data with the old numbering can be replayed. Impact: a stale, now-contradicted stability contract on a persisted enum — the next contributor renumbering by example will silently remap stored audit failure reasons.

Spec

1. Missed test migration leaves internal/application/ctrl tests uncompilable (blocking) — internal/application/ctrl/controller_default_audit_alignment_test.go:58 still calls ctrl.ListAuditEntries(ctx, 10, 0, nil, false, 0) with six arguments, but the production signature was reduced to five parameters (internal/application/ctrl/controller_default.go:1796, mirroring the Controller interface at internal/application/ctrl/controller.go:55). The diff updated every other ListAuditEntries call in this same file (hunks covering lines ~147, 156, 167, 172, 184, 203) plus all mocks and handlers, but skipped this one, which predates the change with the now-removed minLogSequence argument. go build ./... still passes, but go test ./internal/application/ctrl/ fails to compile ("too many arguments in call to ctrl.ListAuditEntries"), directly contradicting the spec requirement that min_log_sequence be removed "from … tests" and making the package's test suite unrunnable as submitted. Resolution: drop the trailing 0 — the test's intent survives, because the ReadIndex-horizon rejection in ListAuditEntriesFrom (controller_default.go:1844-1847) is checked unconditionally before filter validation, so the unfiltered call with horizon 13 > applied index 12 still exercises the "behind ReadIndex horizon" error.

The non-authoritative docs/drafts/prepared-queries.md still mentions min_log_sequence, but AGENTS.md designates docs/drafts/** non-authoritative, so this is not a spec breach.

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

[shipfox-ai]

This PR removes the client-facing min_log_sequence surface across protobuf (ReadOptions, ExecutePreparedQueryRequest, AggregateVolumesRequest), the CLI (--min-log-sequence flags and cmdutil helpers), the HTTP prepared-query body, controller/routed/backend signatures, the ErrReadIndexNotCaughtUp error reason and its gRPC error-detail conversion, the Antithesis min-log driver, and the associated authoritative docs — while sequentially renumbering the affected unreleased protobuf field and enum numbers without reserved declarations. I verified the diff against the tree: the deleted server-side waits (waitMinLogSequence, waitFilteredAuditConsistency) are genuinely superseded by the certificate-based alignment in DefaultController.ListAuditEntriesFrom (explicit failure on disabled/rebuilding audit projections, cancellation preserved, horizon set by RoutedController.withLocalBarrierHorizon; stale retains fixed local-horizon alignment), the vtproto/dethash wire tags match the renumbered .proto files, the remaining min_log_sequence references are only the retained AuditSuccess/AppliedProposal fields (including openapi.yml:5522, which is the AuditSuccess schema, not a leftover request field), non-authoritative drafts, and removal notes, and no dangling callers of the removed symbols remain. The deleted audit-consistency and Antithesis coverage is replaced by the controller-level alignment tests. Recommendation: approve.

Standards

No confirmed material finding. The cross-file spread is the coordinated removal of one public API surface as prescribed by the repo's protobuf and docs-maintenance standards; protobuf renumbering is sequential with no reserved reintroduced; mocks are regenerated rather than hand-edited; CLI/OpenAPI/authoritative docs are updated. Candidates inspected and not retained: the now-unreferenced readstore.Store.WaitForSequence/WaitForAuditSequence primitives (internal, test-exercised, no correctness or compatibility impact) and cross-document duplication of the new consistency narrative (each affected file is updated per the repo's own documented requirement).

Spec

No confirmed material finding. Every PR-body bullet is implemented: the field is gone from protobuf, HTTP/OpenAPI, CLI, routing, errors, tests, and authoritative docs; AuditSuccess.min_log_sequence/max_log_sequence, internal native log/audit cursors, and the stale consistency level are retained; the superseded audit min-log workaround is replaced by the certified audit-alignment controller path; usage projections remain deliberately eventual and are documented as such. The new "Recorded decision: projection read horizon (EN-1946)" section in product-technical-traceability.md is a self-describing decision record added by the change it describes — noted for awareness, but it is defensible under "authoritative docs" and carries no correctness risk.

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

[shipfox-ai]

This PR removes the client-facing min_log_sequence surface (protobuf, HTTP/OpenAPI, CLI, routing, error reason, tests, authoritative docs), sequentially renumbers the unreleased wire fields/enums without reserved, retains AuditSuccess.min_log_sequence/max_log_sequence, the stale consistency level, and all internal native cursors, and replaces the deleted handler-side audit min-log workaround with the certified Raft-horizon alignment in ListAuditEntriesFrom (with successor coverage in controller_default_audit_alignment_test.go). I verified both candidate reports against the diff and the tree: all five spec bullets are implemented; the protobuf renumbering is gap-free; mocks are regenerated; and there are zero dangling READ_INDEX_NOT_CAUGHT_UP/ErrReadIndexNotCaughtUp references. Every confirmed finding below is a minor documentation-consistency residue; no behavioral, correctness, or compatibility defect was found. Recommendation: approve with comments.

Standards

1. [Minor] Vestigial "which wait" wording left in the authoritative barrier comments, contradicting the rewritten docs. With waitMinLogSequence deleted, AddBarrierWait has exactly one production caller (internal/bootstrap/controller_routed.go:80), so a non-zero forwarded=true barrier now unambiguously means a failed ReadIndex attempt — which is exactly how the rewritten docs/technical/architecture/subsystems/read-path/query-profile.md ("A non-zero value in the last row comes from a failed ReadIndex attempt") and openapi.yml (barrierDurationUs: "non-zero means a ReadIndex attempt failed") now describe it. Yet the authoritative wire comment still reads "The value does NOT identify which wait: a failed ReadIndex attempt…" at misc/proto/bucket.proto:1316 (mirrored at internal/proto/servicepb/bucket.pb.go:7985), and internal/query/profile.go:111 still says "a non-zero value does not identify which wait occurred". With only one wait in existence, the disambiguation is meaningless and the proto comment now contradicts its rewritten public siblings. This violates AGENTS.md:89 ("Interface/behavior change: update relevant code comments"). One-line fix per site: drop the "which wait" clause and the dangling article.

2. [Minor] Production-dead wait machinery retained with comments still describing live production users. readstore.Store.WaitForSequence (internal/storage/readstore/store.go:981) and readstore.Store.WaitForAuditSequence (internal/storage/readstore/audit_index.go:43) lost their last production callers with the deletion of waitMinLogSequence/waitFilteredAuditConsistency; only tests call them now. Stale references remain: internal/application/auditindexer/indexer.go:285-289 justifies its (still necessary — it wakes WaitForAuditRaftProgress) NotifyProgress call with "Wake any live filtered-audit reader blocked in WaitForAuditSequence", a reader class that no longer exists in production, and internal/application/indexbuilder/builder.go:769 says "Always wake WaitForSequence waiters". Delete the two now test-only exported methods (or annotate them as test-only) and repoint the comments at the actual production waiters, so the removed min-log-sequence gate does not appear to still exist.

Rejected from the candidate reports as lacking material impact: cmdutil.ConsistencyFlags collapsing to a single field and the empty cmdutil.ConsistencyFlags{} literal at cmd/ledgerctl/signing/list.go (style-only no-op).

Spec

1. [Minor] docs/ops/cli.md:83-95 overpromises projection coverage that the spec explicitly carves out. The rewritten guidance says a read through RoutedController.readCtrl establishes a horizon "for FSM-backed data and every secondary projection used by the request", and that stale "aligns every projection it uses to the fixed horizon". However, ledgerctl ledgers stats routes through readCtrl (internal/bootstrap/controller_routed.go:363-369) and reads its usage counters from an intentionally unaligned usagestore snapshot (internal/application/ctrl/controller_default.go:681-712), while this PR's own additions to usagebuilder.md and the EN-1946 decision record state the usage projection "remains eventual consistency and does not participate in this common horizon". The authoritative CLI doc thus promises a consistency property the spec excludes. Qualify both statements to the certified read/audit projections and call out the stats/usage exception.

2. [Minor] internal/query/audit_filter.go:68-70 still documents the deleted handler-side alignment mechanism. The comment says "The gRPC handler uses it before the consistency wait, so compiling first would capture stale index candidates before the barrier intended to make them fresh." After this PR the gRPC handler no longer calls AuditFilterNeedsIndex at all and waitFilteredAuditConsistency is deleted; the sole production caller is DefaultController.ListAuditEntriesFrom (internal/application/ctrl/controller_default.go:1845), which uses the predicate after opening the fixed main snapshot to gate compilation on the audit projection's Raft certificate (WaitForAuditRaftProgress). Certified alignment itself is preserved and correctly covered by the successor tests; only this implementation comment is stale and points maintainers at a removed mechanism. Update it to describe the controller's certificate gate.

No missing or partial spec requirements and no scope creep were found. The renumbered wire tags, retained stale path, checkpoint certificate verification, and doc rewrites are all consistent with the PR body's five bullets; leftover min_log_sequence in docs/drafts/prepared-queries.md is non-authoritative per AGENTS.md and out of scope.

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

[gfyrag]

Follow-up on f3983bf142 after the latest review:

• corrected the QueryProfile barrier wording against the actual code: a forwarded read can record either the syncing precheck or a pending ReadIndex invalidated by a leadership change;
• updated native/certified projection waiter comments without removing the internal log/audit cursors;
• documented the eventual usage-counter exception in the CLI consistency contract;
• updated the audit-filter comment to describe the controller certificate gate;
• reconciled the persisted ErrorReason comment with the unreleased-v3 sequential-renumbering policy.

Validation on the final tree:

• targeted domain/query/readstore/indexer tests: PASS
• bash scripts/agent-check: PASS
• AI_REVIEW_BASE_SHA=4710460dab375e1d57cb0b8032e8e7bd0edf66b3 bash scripts/agent-check-pr: PASS, including full race tests, business E2E, cluster E2E, and 62/62 Schemathesis endpoints.

The previous unit failure was a runtime pprof heap-profile read racing with Sonic JIT module loading; the same parent package was green and the final HTTP package passed 5 targeted repetitions, 3 full-package repetitions, and the full race gate. The previous cluster timeout was an old follower-rejoin ListLedgers callback blocked on a stale gRPC call; the parent run and the final local cluster suite are green. A fresh CI run is now evaluating this head.

[shipfox-ai]

Final review of PR #1881 (formancehq/ledger) — "refactor(api): remove public minimum log sequence (EN-1946)". I verified both prior reports against diff.patch (4710460 → f3983bf, 1 commit, 69 files) and the checkout. The removal is complete and consistent on every surface: ReadOptions.min_log_sequence, ExecutePreparedQueryRequest.min_log_sequence, and AggregateVolumesRequest.min_log_sequence are deleted from misc/proto/*.proto with sequentially realigned successors and regenerated pb/vtproto/dethash/reader code; the CLI flags, HTTP body field, OpenAPI schema, routing parameter (Controller.ListAuditEntries now 5-arg across controller, RoutedController, gRPC/HTTP clients, and all four mock sets), ErrReadIndexNotCaughtUp with its error reason, KindForReason arm, and gRPC detail branch, the Antithesis parallel_driver_minlogseq driver, and the superseded waitMinLogSequence/waitFilteredAuditConsistency workaround are all gone. Retained surfaces are intact: AuditSuccess.min/max_log_sequence (misc/proto/audit.proto:97, openapi.yml:5522), AppliedProposal.min_log_sequence, readstore.WaitForSequence/WaitForAuditSequence, ConsistencyStale, and the parallel_driver_stale_reads/parallel_driver_projection_alignment drivers. The renumbering (mode 7→6, AggregateVolumesRequest 4–7→3–6, ErrorReason 63–68→63–67) is sanctioned by AGENTS.md:10 and docs/technical/contributing/protobuf.md:63; I additionally audited the hand-rolled wire sites (protowire_postings.go decodes only the unrenumbered Log/Posting family, predicted_index.go its own message) and checked the regenerated vt/dethash tag bytes (mode 0x38→0x30, checkpoint_id 0x31→0x29, group_by_prefixes 0x2A→0x22, use_max_precision 0x20→0x18) — all correct. No dangling references to the deleted driver, no unused imports left in server.go/server_bucket.go, and the deleted scenario test file is covered additively by controller_default_audit_alignment_test.go and readstore/wait_audit_test.go. Recommendation: approve with comments — no blocking findings on either axis.

Standards

No confirmed hard violation of the documented standards (protobuf renumbering policy, mock regeneration, documentation maintenance, and engineering conventions all verified against AGENTS.md / docs/technical/contributing/).

Non-blocking observations (verified, no correctness/security/compatibility impact):

1. Data clump / residual plumbing — cmd/ledgerctl/cmdutil/flags.go:105-116, cmd/ledgerctl/signing/list.go:55. ConsistencyFlags now carries only CheckpointID, yet BuildListOptions still threads the one-field struct through every list caller, and signing/list.go (an endpoint with no consistency options at all) passes a literal cmdutil.ConsistencyFlags{}. Dropping the parameter from consistency-less endpoints would remove dead plumbing. Cleanup only.

2. Test-only primitives retained — internal/storage/readstore/store.go:983, internal/storage/readstore/audit_index.go:44. WaitForSequence and WaitForAuditSequence no longer have production callers (tests only; comments say the certified paths use WaitForRaftProgress/WaitForAuditRaftProgress). The spec explicitly retains internal native cursors, so this is intentional retention, not speculative generality to remove now — recorded so a future cleanup pass can revisit if the native-cursor lifecycle checks never materialize.

Spec

No confirmed material finding. All five PR-body commitments verified independently against the diff and checkout:

• Removal on every surface — protobuf, HTTP/OpenAPI, CLI (all --min-log-sequence registrations and readers gone; cmd/ledgerctl/cmdutil/flags_test.go:143 asserts the flag is nil; cmd/ledgerctl/audit/list.go uses AddConsistencyFlags, which now registers only --checkpoint-id), routing, errors, tests, and authoritative docs (cli.md, grpc-api.md, http-api.md, raft-consensus.md, indexer.md, prepared-queries.md, query-checkpoints.md, query-filter.md, query-pipeline.md, query-profile.md, usagebuilder.md, api-comparison.md, read-consistency-projections.json). Whole-tree grep shows only the retained audit/proposal fields, a test asserting the flag's absence, historical "removed by EN-1946" mentions, and non-authoritative docs/drafts/prepared-queries.md (drafts are explicitly non-authoritative per AGENTS.md, and the spec scoped removal to authoritative docs).
• Sequential realignment without reserved — matches AGENTS.md policy; no reserved reintroduced in misc/proto/; value maps and raw descriptors consistent.
• Retained AuditSuccess.min/max_log_sequence and internal native cursors — confirmed in proto, checker/indexbuilder/state consumers, and openapi (AuditSuccess at openapi.yml:5522 is the retained audit surface, not the removed option).
• stale retained — ConsistencyStale in internal/bootstrap/controller_routed.go, e2e coverage, and parallel_driver_stale_reads driver all present; deferral recorded in product-technical-traceability.md.
• Workaround removal preserves certified alignment — the controller-level ListAuditEntries path aligns the audit projection to the fixed ReadBarrierHorizon, with replacement coverage in controller_default_audit_alignment_test.go.

No missing/partial requirements, no scope creep, and no implemented-but-wrong behavior found.

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