refactor(reads): remove leader consistency selector (EN-1946)

[gfyrag]

Stack 5/7 for EN-1946. Removes the unreleased leader consistency selector while retaining stale. Unknown leader metadata now follows the existing default linearizable route.

[NumaryBot]

✅ Approve — automated review

The leader selector is removed consistently while stale and default linearizable behavior remain intact. Prior documentation and forwarding-test concerns are resolved at HEAD.

No findings.

[codecov]

Codecov Report

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

Additional details and impacted files

@@                          Coverage Diff                          @@
##           feat/en-1946-inspect-index-alignment    #1893   +/-   ##
=====================================================================
  Coverage                                 77.40%   77.41%           
=====================================================================
  Files                                       458      458           
  Lines                                     48643    48632   -11     
=====================================================================
- Hits                                      37654    37647    -7     
- Misses                                     7825     7829    +4     
+ Partials                                   3164     3156    -8     

Flag	Coverage Δ
e2e	77.41% <100.00%> (+<0.01%)	⬆️
scenario	77.41% <100.00%> (+<0.01%)	⬆️
unit	77.41% <100.00%> (+<0.01%)	⬆️

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.

[shipfox-ai]

Both axis findings above were independently confirmed against the code; rejected candidates included style-only test renames and baseline smells suppressed by repo conventions.

Standards

1. [P2] Stale "leader-local" exception in the read-path subsystem index — docs/technical/architecture/subsystems/read-path/README.md:8-10 still states that "leader-local, audit-index, and usagestore exceptions are documented" in the consensus matrix. This diff removes the leader-local exception from raft-consensus.md, so the authoritative index now points readers to an exception that no longer exists, contradicting the read-path contract it summarizes. This violates AGENTS.md's Documentation-maintenance rule ("Documentation is part of the change when behavior... change"). Fix: drop "leader-local" from the list (or, if the syncing-follower fallback is meant, name that explicitly).
2. [P3] Missing blank line before the audit caveat in CLI docs — docs/ops/cli.md:96-98: deleting the leader bullet and its trailing separator leaves Filtered \audit list` has an endpoint-specific asynchronous-index caveat...immediately after thestalebullet with no blank line. Markdown renders this as a lazy continuation of thestale` list item, implying the caveat is specific to stale consistency when it applies to filtered audit reads generally. Fix: add a blank line before that paragraph.

Spec

1. [P3] Dangling comment referencing the deleted leader branch — internal/bootstrap/controller_routed.go:94-102 (the leader_fallback branch) still opens with "Same as the explicit-leader branch: the read leaves this node, so... Unlike that branch, the barrier already recorded above is KEPT". The explicit-leader branch this compares against was deleted by this PR, so the contrast is unresolvable for a reader. The spec called for removing the leader selector; this residual comment describing the removed code path is incomplete cleanup. Fix: rewrite the comment to stand alone about the fallback path.
2. [P3] Deleted e2e test leaves the surviving forwarded-read receipt relay uncovered — tests/e2e/cluster/get_transaction_receipt_forwarded_test.go was deleted, which is justified since its explicit Consistency: leader trigger no longer exists. However, the behavior it guarded survives on the remaining forwarding path: a no-signer follower taking the leader_fallback route in RoutedController.readCtrl (internal/bootstrap/controller_routed.go:87-110) still relays the leader-signed receipt via internal/adapter/grpc/server_bucket.go:318-322. No other cluster e2e test covers receipts (verified by search), so the heterogeneous-signer relay on forwarded reads now has zero regression coverage. Non-blocking, but consider re-aiming the deleted test at the fallback trigger instead of dropping it outright.

The core change is correct: ConsistencyLeader and its routing branch, CLI flag/completion, leader e2e test, and leader docs are fully removed; extractConsistency now falls unknown x-consistency: leader metadata through to the default linearizable route (covered by TestExtractConsistency_LeaderIsNotSupported); stale is retained end-to-end; no residual ConsistencyLeader/leader-selector references remain in code or authoritative docs, and openapi.yml never exposed the header. What remains are documentation-hygiene leftovers from the removal and one coverage gap. Recommendation: approve with comments — none of the findings above is a correctness bug, but all are cheap to fix and should land before merge.

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 unreleased x-consistency: leader read-consistency selector while retaining stale, and routes unknown consistency metadata to the existing default linearizable path. The runtime change is correct and well covered: extractConsistency now accepts only ConsistencyStale (internal/adapter/grpc/consistency.go:81-89, covered by the new TestExtractConsistency_LeaderIsNotSupported), stale is preserved end-to-end (server route, CLI help, enum completion, docs/ops/cli.md), and the syncing-follower fallback still marks the profile as forwarded (internal/bootstrap/controller_routed.go:130). However, the removal was not carried through the authoritative documentation: two documents still tell readers the deleted mode exists and has weaker partition semantics, which directly contradicts the new behavior, and the docs/ops/cli.md edit left malformed markdown behind. Recommendation: request changes — the code is sound, but the contradictory documentation must be fixed before merge; all fixes are documentation-only.

Standards

1. [Blocking] Authoritative documentation still describes the removed leader mode.

   • docs/technical/architecture/subsystems/consensus/raft-consensus.md:302-306 still reads: "An explicit leader read can do the same when it reaches a node that still considers itself leader: the local-leader routing shortcut serves local state without ReadIndex. Neither mode should be used when quorum-confirmed freshness is required during a partition."
   • docs/technical/architecture/subsystems/read-path/README.md:8-11 still lists "leader-local" among the consistency exceptions "documented in the consensus matrix".

   This diff deleted ConsistencyLeader, so x-consistency: leader now silently resolves to the default linearizable path. A caller or operator following either document will send leader expecting a routed, barrier-free read (and stale-on-isolated-leader semantics) that no longer exists. This breaches AGENTS.md "Documentation maintenance" ("Documentation is part of the change when behavior, architecture, interfaces, CLI, or APIs change") — the update was made, but these leader references were left intact. Fix: remove/rewrite both passages so only stale, checkpoint, audit-index/usagestore, and the syncing-follower fallback remain.

2. Broken markdown introduced in docs/ops/cli.md:97-99. The hunk that deleted the leader bullet also consumed the blank line separating the bullet list from the following paragraph, so "Filtered audit list has an endpoint-specific asynchronous-index caveat…" now sits directly under the stale bullet and renders inside that list item as a lazy continuation. This breaches the same AGENTS.md documentation-maintenance rule (the CLI doc was updated, but malformed) and the definition-of-done step "inspect the final diff". Fix: restore the blank line and rewrap the orphaned sentence.

3. [Non-blocking] Durable traceability chain is incomplete for a consistency/API-semantic change. The only committed rationale added is at docs/technical/architecture/data-flows.md:555-558: "The unreleased v3 x-consistency: leader selector was removed by EN-1946." docs/technical/contributing/product-technical-traceability.md explicitly classifies API-semantic and consistency changes as significant and states that external issue identifiers may establish the need, but the committed evidence "must retain enough requirement and decision context to remain understandable without those private or mutable sources." Nothing in-repo records why the existing behavior was insufficient, the observable requirement, alternatives considered, or how the outcome is validated — future reviewers cannot distinguish a deliberate product/operational constraint from cleanup preference. Add the missing chain to the owning committed document (or a relevant code comment) per the traceability contract.

Spec

Spec of record (PR body): "Removes the unreleased leader consistency selector while retaining stale. Unknown leader metadata now follows the existing default linearizable route."

1. Partial removal: leftover references to the deleted selector in docs and code comments. Beyond the two documentation locations listed under Standards, the diff leaves comments describing the now-deleted mode:

   • internal/bootstrap/controller_routed.go:95-98 — the kept syncing-follower fallback comment still says "Same as the explicit-leader branch: the read leaves this node… Unlike that branch, the barrier already recorded above is KEPT…", but the explicit-leader branch it compares against was deleted by this very diff.
   • internal/query/profile.go:106-107 — the Forwarded field comment says "(an explicit leader read, or the syncing-follower fallback)"; profile.go:114-115 says "so an explicit leader read with min_log_sequence set reports a wait"; profile.go:492-495 says "0 for an explicit leader read, non-zero when a local barrier failed before the fallback".

   Otherwise the removal is complete and correct: no ConsistencyLeader constant, x-consistency: leader routing, markForwardedIfRemote, or leader-mode CLI text remains anywhere (verified by repo-wide search; the surviving getLeaderCtrl calls serve pre-existing write forwarding and the syncing-follower fallback, not the removed mode). Requirement 2 of the spec holds: stale is retained end-to-end, and unknown metadata degrades to linearizable exactly as specified. There is no scope creep — the deleted tests/e2e/cluster/get_transaction_receipt_forwarded_test.go exclusively exercised leader-forwarded receipt relaying, whose production trigger no longer exists, and the reworded QueryProfile.Forwarded proto/docs comments are direct consequences of the removal.

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

[NumaryBot]

NumaryBot posted 2 new inline findings.

Summary: #1893 (comment)

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1893 (comment)

[shipfox-ai]

Review of PR #1893 — refactor(reads): remove leader consistency selector (EN-1946)

The functional removal is correct and complete at the code level: ConsistencyLeader is gone from internal/adapter/grpc/consistency.go, readCtrl's leader branch and markForwardedIfRemote are deleted, the CLI flag/help/completions and generated proto comments are updated, x-consistency: leader now falls through to the default linearizable route with a dedicated test (TestExtractConsistency_LeaderIsNotSupported), stale is retained end-to-end, and openapi.yml never exposed the header, so there is no HTTP-surface drift. However, the removal was not carried through the documentation layer: two authoritative architecture docs still describe the deleted mode and now contradict the code, several code comments still reference the removed branch, one doc edit broke Markdown structure, and the repo's own traceability contract is not satisfied for a consistency-strategy change. All findings below are documentation/comment-level; no correctness, security, or compatibility defect was found in the code.

Recommendation: request changes — the code is mergeable, but the repo's definition of done treats documentation as part of the change, and the fixes are small.

Standards

1. [Hard] Authoritative docs still describe the removed leader mode and now contradict the code.

   • docs/technical/architecture/subsystems/consensus/raft-consensus.md:302-306 still reads: "An explicit leader read can do the same when it reaches a node that still considers itself leader: the local-leader routing shortcut serves local state without ReadIndex. Neither mode should be used when quorum-confirmed freshness is required during a partition." This diff removed the leader bullet from the consensus matrix in this same file (~line 574) but left this partition-section paragraph, so the document now contradicts both itself and internal/adapter/grpc/consistency.go. This is safety-relevant prose about stale reads during partitions.
   • docs/technical/architecture/subsystems/read-path/README.md:8-10 still lists "leader-local, audit-index, and usagestore exceptions are documented in the consensus matrix" — the leader-local exception no longer exists.
   • Breach of AGENTS.md "Documentation maintenance" ("Documentation is part of the change when behavior, architecture, interfaces, CLI, or APIs change"; subsystem READMEs are authoritative per docs/technical/agent-context.md). Remove or rewrite both passages.

2. [Hard] Required product-to-technical traceability is missing.

   • docs/technical/architecture/data-flows.md:556-558 records only: "The unreleased v3 x-consistency: leader selector was removed by EN-1946; callers choose between the default linearizable route and an explicit stale local read."
   • docs/technical/contributing/product-technical-traceability.md requires the full chain (need → current limitation → requirement → decision/alternatives → validation) for "API or semantic changes" and "consistency strategies", and explicitly states that external issue identifiers alone are insufficient: the committed evidence "must retain enough requirement and decision context to remain understandable without those private or mutable sources". The one-line note and the PR body do not state why the mode is being removed (e.g., the stale-serving hazard of an isolated former leader with CheckQuorum disabled), what alternatives were considered, or how the removal is validated. Add a concise motivation/decision note to the owning subsystem documentation and reference it from the PR.

3. Stale code comments referencing the deleted branch. AGENTS.md: "Interface/behavior change: update relevant code comments."

   • internal/query/profile.go:106-107 — Forwarded "is true when the read was routed to another node (an explicit leader read, or the syncing-follower fallback)".
   • internal/query/profile.go:113-115 — "so an explicit leader read with min_log_sequence set reports a wait on a healthy cluster".
   • internal/query/profile.go:491-494 — "barrierDurationUs then covers the local attempt only: 0 for an explicit leader read, non-zero when a local barrier failed".
   • internal/bootstrap/controller_routed.go:95-102 — the leader_fallback comment opens "Same as the explicit-leader branch: … Unlike that branch, the barrier already recorded above is KEPT…" — the branch it contrasts against was deleted by this diff.
     Rewrite these to describe only the syncing-follower fallback.

4. Malformed Markdown introduced in docs/ops/cli.md:96-98. The hunk deleting the leader bullet also consumed the blank line, so "Filtered audit list has an endpoint-specific asynchronous-index caveat…" now sits directly under the stale bullet and renders as a lazy continuation of that list item — implying the caveat is stale-specific. Restore the blank line. (Also a breach of the definition-of-done step "inspect the final diff for unrelated changes".)

No baseline code smells beyond the above: the 14-file fan-out is the inherent cost of removing a cross-cutting selector, and the string-typed consistency levels are a pre-existing pattern this diff merely narrows. No misc/proto/bucket.proto ↔ bucket.pb.go drift.

Spec

Spec of record: "Removes the unreleased leader consistency selector while retaining stale. Unknown leader metadata now follows the existing default linearizable route."

1. [Partial] "Removes the unreleased leader consistency selector" is delivered in code but not in authoritative documentation and comments. The runtime selector is fully gone (verified: no ConsistencyLeader remains anywhere in target/; CLI help/completion, e2e leader test, and proto comments all updated; the deleted get_transaction_receipt_forwarded_test.go exercised only the removed trigger, so its deletion is a required consequence). But the following still describe the removed mode as active at HEAD: docs/technical/architecture/subsystems/consensus/raft-consensus.md:302-306, docs/technical/architecture/subsystems/read-path/README.md:8-10, internal/query/profile.go:106-107, 113-115, 491-494, and internal/bootstrap/controller_routed.go:95-97. Future maintainers reading these authoritative sources will conclude the routing/profile behavior still exists.

2. [Minor] The docs/ops/cli.md:96-98 edit is partially incorrect. Deleting the leader bullet consumed the blank line, so the general filtered-audit list caveat now renders as a stale-specific continuation. This misdocuments the retained stale mode that the spec explicitly keeps.

3. [Non-blocking observation] Coverage gap for the surviving forwarded-read receipt relay. The deleted tests/e2e/cluster/get_transaction_receipt_forwarded_test.go was the only cluster-level e2e test of the heterogeneous-signer receipt relay (a no-signer contacted node relaying a leader-signed receipt verbatim). That behavior survives on the leader_fallback path in RoutedController.readCtrl but now has no cluster e2e coverage. Deletion was required by the spec; consider follow-up coverage of the relay on the fallback path.

No scope creep and no behaviour-looking-wrong findings: "while retaining stale" holds end-to-end (extractConsistency accepts ConsistencyStale, readCtrl serves it locally, --min-log-sequence still applies regardless of level), and "unknown leader metadata follows the existing default linearizable route" is implemented and tested.

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 unreleased x-consistency: leader consistency selector while retaining stale, per EN-1946. The runtime change is correct and well-tested: extractConsistency now accepts only stale (internal/adapter/grpc/consistency.go:85-89), unknown leader metadata falls through to the default linearizable route and is covered by TestExtractConsistency_LeaderIsNotSupported, the stale route and syncing-follower fallback are preserved, the CLI flag/help/completions and docs/ops/cli.md were updated, and no code, workload, or openapi.yml path still offers the selector. However, the cleanup is incomplete: authoritative subsystem documentation and several code comments still describe the deleted explicit-leader branch, and the diff introduces a small Markdown regression in docs/ops/cli.md. Recommendation: request changes — the required follow-up is documentation/comment-only; once those stale references are removed, this is ready to merge.

Standards

1. [P2][blocking] Complete the leader-mode documentation and code-comment cleanup.

   • docs/technical/architecture/subsystems/consensus/raft-consensus.md:303-306 — still states: "An explicit leader read can do the same when it reaches a node that still considers itself leader: the local-leader routing shortcut serves local state without ReadIndex." That path no longer exists; x-consistency: leader now follows the default linearizable route. The authoritative partition-semantics text tells operators a weaker mode remains available.
   • docs/technical/architecture/subsystems/read-path/README.md:9 — still lists "leader-local" among the documented read-consistency exceptions.
   • internal/bootstrap/controller_routed.go:95 — comment "Same as the explicit-leader branch: the read leaves this node, so…" references a branch deleted by this diff; only the syncing-follower fallback remains.
   • internal/query/profile.go:106-107, 115, 492-495 — the Forwarded field doc and the profile log-fields comment still describe "an explicit leader read" (e.g. "0 for an explicit leader read" at line 494), which can no longer occur.
     This violates AGENTS.md Documentation Maintenance: "update the matching docs/technical/architecture/ subsystem documentation and its README" and "Interface/behavior change: update relevant code comments." Resolution: remove or rewrite every selector-specific reference, keeping only descriptions of stale and the distinct syncing-follower fallback.

2. [P3][non-blocking] Restore the paragraph boundary in docs/ops/cli.md.
   docs/ops/cli.md:97-99: deleting the leader bullet also removed the blank line, so "Filtered audit list has an endpoint-specific asynchronous-index caveat…" now renders as a lazy continuation of the stale bullet, wrongly associating a general endpoint caveat with stale reads. Insert a blank line before the audit paragraph.

3. [P3][non-blocking] The forwarded-read receipt-relay path loses its only end-to-end coverage.
   The deleted tests/e2e/cluster/get_transaction_receipt_forwarded_test.go was the only cluster-level test that a no-signer node relays a remotely signed receipt. That production behavior survives via the syncing-follower fallback: RoutedController.readCtrl still forwards (controller_routed.go:95-110), and internal/adapter/grpc/server_bucket.go:318-322 still relays whatever receipt the routed controller returns without re-signing. docs/technical/contributing/testing.md (Regression preservation) requires a guard for each surviving production path of a shared mechanism. The deleted spec's own trigger (the selector) is gone, so deleting it is within scope — but consider retargeting the heterogeneous-signer scenario to the surviving fallback so relay behavior is not left e2e-uncovered.

4. [P3][non-blocking] Record the decision motivation in-repo.
   docs/technical/architecture/data-flows.md:557-559 records only that the selector "was removed by EN-1946" — an external, private Jira reference. docs/technical/contributing/product-technical-traceability.md requires the need → limitation → decision chain in-repo for API-semantic and consistency changes, and states that when classification is ambiguous the motivation must be documented rather than silently assumed exempt. Removal of an (unreleased) API surface is arguably semantic; commit the motivation chain in the owning documentation or obtain an explicit human classification that this is mechanical.

Spec

The PR body's three requirements are otherwise fully implemented: (1) the selector is removed across code, CLI, tests, and documentation, with ConsistencyLinearizable/ConsistencyStale the only remaining levels; (2) stale is retained with its no-barrier local route (controller_routed.go:66-71) and its CLI documentation and examples; (3) unknown leader metadata follows the default linearizable ReadIndex route (consistency.go:85-89), asserted by TestExtractConsistency_LeaderIsNotSupported. No scope creep was found — the deleted e2e spec, markForwardedIfRemote, and their unit tests existed solely to serve the removed trigger, and the proto/comment edits describe the removal.

1. [P2] "Removes the unreleased leader consistency selector" is only partially fulfilled in documentation and source comments.
   The same locations listed under Standards finding 1 (raft-consensus.md:303-306, read-path/README.md:9, controller_routed.go:95, profile.go:106-117, 492-495) still present the selector as an available consistency mode with specific partition semantics, or describe its diagnostic behavior. These claims are now impossible: extractConsistency accepts only stale. Operators are told a removed weaker mode still exists, and maintainers read comments about a nonexistent branch. Resolution: finish the doc/comment rewrite described in Standards finding 1.

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

[NumaryBot]

🛑 Changes requested — automated review

The runtime removal is correct and prior documentation findings are fixed. Only stale explanatory comments remain; they do not affect behavior.

Findings outside the diff

🟠 [major] Remove remaining explicit-leader profile references — internal/bootstrap/controller_routed.go:95

After deleting the explicit-leader branch, this comment still compares the fallback against that nonexistent path, while internal/query/profile.go:106-116,494-495 likewise documents explicit leader reads as producers of Forwarded. This now misstates the profiling contract and should be updated as part of the required code-comment synchronization (AGENTS.md:99).

Summary: #1893 (comment)

[NumaryBot]

💬 Comments — automated review

The runtime and public documentation changes correctly remove the leader selector. Only stale internal comments remain; they do not affect behavior or tests.

Findings outside the diff

🟡 [minor] Remove stale explicit-leader references — internal/bootstrap/controller_routed.go:95

After removing the explicit leader route, this comment still compares fallback behavior with that deleted branch. The same stale behavior description remains in internal/query/profile.go:106-115 and :492-495, incorrectly documenting Forwarded and zero barrier duration as outcomes of an explicit leader read. Update these comments per AGENTS.md:89.

Summary: #1893 (comment)

[shipfox-ai]

Both prior reviews agree with the code, and so does my independent verification: the runtime removal is complete and correct. ConsistencyLeader is gone (no constant, no leader routing branch, no markForwardedIfRemote), stale is retained end-to-end (internal/adapter/grpc/consistency.go:85-87 → local serve in internal/bootstrap/controller_routed.go:66-70), and unknown x-consistency metadata falls through extractConsistency's default to ConsistencyFromContext's linearizable default, covered by TestExtractConsistency_LeaderIsNotSupported. CLI help/completions, proto comments, and the consistency matrix in raft-consensus.md were all updated, and deleting the leader-only e2e test is in scope since its trigger no longer exists. No scope creep and no wrong implementation found. However, the removal stops at the runtime boundary: the authoritative consensus doc and several live source comments still present the deleted mode as an available path, which violates the repository's mandatory documentation-maintenance rules and leaves the Forwarded/barrier profile contract describing a branch that no longer exists. These are doc/comment-only fixes, but they are required by AGENTS.md, so I recommend request changes (small scope: reword stale passages; no code changes needed).

Standards

1. [High] Stale authoritative architecture doc still documents the removed leader mode. docs/technical/architecture/subsystems/consensus/raft-consensus.md:303-306 still reads: "An explicit leader read can do the same when it reaches a node that still considers itself leader: the local-leader routing shortcut serves local state without ReadIndex. Neither mode should be used when quorum-confirmed freshness is required during a partition." This diff deletes the mode but edits this same file, and the file's own consistency matrix (~line 574) was correctly updated — the document now contradicts the code and itself. Violates AGENTS.md "Documentation maintenance" ("Documentation is part of the change when behavior, architecture, interfaces, CLI, or APIs change"). Rewrite the paragraph to cover only stale vs. linearizable.

2. [High] Stale code comments reference the deleted explicit-leader branch. Violates AGENTS.md: "Interface/behavior change: update relevant code comments."

   • internal/bootstrap/controller_routed.go:95-104 — the syncing-follower fallback comment opens "Same as the explicit-leader branch: … Unlike that branch, the barrier already recorded above is KEPT…", but the branch it contrasts was deleted by this diff. Rewrite to describe the fallback directly (the barrier-retention rationale itself is still valid and worth keeping).
   • internal/query/profile.go:106-107 — Forwarded "is true when the read was routed to another node (an explicit leader read, or the syncing-follower fallback)".
   • internal/query/profile.go:114-115 — "so an explicit leader read with min_log_sequence set reports a wait".
   • internal/query/profile.go:493-495 — "barrierDurationUs then covers the local attempt only: 0 for an explicit leader read, non-zero when a local barrier failed before the fallback" — the "0 for an explicit leader read" case can no longer occur.

3. [Medium] Product-to-technical traceability chain is incomplete. Removing a consistency selector falls squarely under docs/technical/contributing/product-technical-traceability.md ("API or semantic changes"; "consistency … strategies"). The only in-repo rationale is docs/technical/architecture/data-flows.md:557-559's "The unreleased v3 x-consistency: leader selector was removed by EN-1946" — an external, private ticket ID, which the traceability doc explicitly says is insufficient ("the committed repository evidence must retain enough requirement and decision context to remain understandable without those private or mutable sources"). The safety rationale that justified the mode's removal (isolated former leader can serve stale local state because CheckQuorum is disabled) is currently recoverable only from the same stale paragraph in finding 1 that must be rewritten. Mitigating context: v3 is unreleased, so the decision is cheap to justify — but the doc requires the motivation be recorded "rather than silently assuming it is exempt". Add a concise durable rationale (why the selector was unsafe/unnecessary, and that the syncing-follower fallback covers the useful forwarding case) in the owning read-path/API documentation.

4. [Low, advisory] Coverage regression on the forwarded-receipt relay path. The deleted tests/e2e/cluster/get_transaction_receipt_forwarded_test.go was the only cluster-level e2e proving that a no-signer follower relays a leader-signed receipt verbatim on a forwarded read. That production behavior survives via the syncing-follower fallback (controller_routed.go → finishLeaderFallback → MarkForwarded; the adapter relays the remote receipt as-is per internal/adapter/grpc/server_bucket.go:316-322), and the test's deletion was required since its x-consistency: leader trigger is gone — but the remaining receipt e2e (tests/e2e/business/get_transaction_receipt_http_test.go) covers only the local path. Consider retargeting the heterogeneous-signer scenario at the fallback (or explicitly recording that the relay contract is now covered only by unit-level evidence), per the regression-preservation guidance in docs/technical/contributing/testing.md.

Spec

Spec of record: "Removes the unreleased leader consistency selector while retaining stale. Unknown leader metadata now follows the existing default linearizable route."

1. [Major] Requirement "removes the leader consistency selector" is partial in documentation and comments. The runtime selector is fully removed and every functional surface (CLI flag help/completions, proto comments in misc/proto/bucket.proto / internal/proto/servicepb/bucket.pb.go, the consistency matrix, query-profile.md) was updated, but the following live text still presents the removed mode as available, contradicting the updated code and the reworded proto/query-profile.md text:

   • docs/technical/architecture/subsystems/consensus/raft-consensus.md:303-306 (see Standards Fix typo #1);
   • internal/bootstrap/controller_routed.go:95-104 (see Standards update spec #2);
   • internal/query/profile.go:106-107, :114-115, :493-495 (see Standards update spec #2).
     These are present-tense descriptions of the source-level Forwarded/barrier contract, not historical notes, and can mislead future maintenance of the profile accounting they describe.

2. No other spec findings. "Retaining stale" holds end-to-end (extractConsistency accepts ConsistencyStale; readCtrl serves it locally; CLI and docs updated; stale e2e and antithesis coverage remain). "Unknown leader metadata follows the default linearizable route" is implemented (default: return ctx → linearizable default) and directly tested by TestExtractConsistency_LeaderIsNotSupported. The reworded QueryProfile.Forwarded comments and the deletion of the leader-only e2e test are direct consequences of the removal, not scope creep.

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

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1893 (comment)

[NumaryBot]

🛑 Changes requested — automated review

The runtime refactor appears sound, but it removes the sole end-to-end regression test for receipt relay through a forwarding path that still exists.

Findings outside the diff

🟠 [major] Preserve forwarded-receipt coverage through the fallback — tests/e2e/cluster/get_transaction_receipt_forwarded_test.go:83

Deleting this test removes the only end-to-end guard that a no-signer follower relays a receipt produced by a remote leader. readCtrl still forwards GetTransaction when a follower is syncing or loses leadership, so retain this scenario using that fallback; regression coverage must remain additive across production triggers per AGENTS.md:78.

Summary: #1893 (comment)

[shipfox-ai]

Final review — PR #1893 (refactor(reads): remove leader consistency selector, EN-1946)

The diff cleanly removes the unreleased x-consistency: leader selector end-to-end: ConsistencyLeader is gone from internal/adapter/grpc/consistency.go, the explicit leader branch and markForwardedIfRemote are deleted from internal/bootstrap/controller_routed.go, the CLI help/completion and all subsystem docs no longer describe the mode, and the generated bucket.pb.go comments match misc/proto/bucket.proto. Unknown metadata now falls through to the default linearizable route, asserted by the renamed TestExtractConsistency_LeaderIsNotSupported. A repo-wide search confirms no residual reference to the selector in code, clients, or documentation (remaining "leader" mentions describe Raft leadership or the syncing-follower fallback, accurately), and openapi.yml never exposed x-consistency, so the HTTP surface is untouched. The spec is faithfully implemented. The one material problem is on the standards axis: the only committed rationale for the removal is a private Jira key, while this very diff deletes the prose that carried the motivating limitation, leaving the repository without the traceability chain its own contributing standard requires. Recommendation: request changes — the required fix is a small documentation addition; no code change is needed.

Standards

[P1] Add the missing in-repo traceability chain for the consistency-API decision — docs/technical/architecture/data-flows.md:557

The tombstone "The unreleased v3 x-consistency: leader selector was removed by EN-1946" is the only committed evidence of why the selector was removed. docs/technical/contributing/product-technical-traceability.md explicitly lists "API or semantic changes" and "consistency … strategies" among decisions requiring the in-repo chain (need → limitation → requirement → decision → validation), and states that external IDs alone are insufficient: "the committed repository evidence must retain enough requirement and decision context to remain understandable without those private or mutable sources." This diff deletes the very prose that carried the motivating limitation (an isolated former leader could serve stale state with CheckQuorum disabled), so after merge a future reviewer cannot reconstruct the requirement, the decision rationale, or the validation from the repository. The standard also requires that ambiguous cases "document the motivation rather than silently assuming it is exempt." Fix: add a short motivation/decision note (2–3 lines stating the staleness hazard of the local-leader shortcut and the decision to keep only linearizable + stale) to the owning subsystem documentation (data-flows.md or the consensus/read-path pages) in this PR, and summarize the chain in the PR description. If a sibling PR in the EN-1946 stack carries it, link that committed path.

[P3] Forwarded-read receipt relay lost its only cluster-level guard — deleted tests/e2e/cluster/get_transaction_receipt_forwarded_test.go

Non-blocking. The deleted spec was the only automated test that a no-signer follower relays a leader-signed receipt on a forwarded read. That behavior survives on the syncing-follower fallback (internal/bootstrap/controller_routed.go:97-118, finishLeaderFallback → remote GetTransaction), but a repo-wide search of tests/ finds no remaining coverage of receipt relaying across a forwarded hop (only the local-path receipt test at tests/e2e/business/get_transaction_receipt_http_test.go:70). The deletion itself complies with the regression-test rule — the test's only production trigger (Consistency: leader) is removed with it — but per testing.md's "preserve a focused regression test … when appropriate" guidance, consider a follow-up that retargets the heterogeneous-signer scenario at the leader_fallback trigger.

No other standards findings: documentation maintenance is complete across docs/ops/cli.md, the consensus/read-path docs, and code comments; the .proto/.pb.go comment refresh is tooling-managed; the fan-out across 16 files is mandated by the documentation-maintenance rule and is not a shotgun-surgery smell.

Spec

The Spec axis has no confirmed material finding.

All three requirements from the PR body are implemented and verified at HEAD:

1. "Removes the unreleased leader consistency selector" — ConsistencyLeader no longer exists; extractConsistency accepts only stale; the CLI flag help and enum completion list only stale/linearizable; the leader e2e spec, the markForwardedIfRemote helper and its unit tests are deleted; all documentation describing the mode is updated.
2. "while retaining stale" — retained end-to-end: local stale route with no barrier (controller_routed.go:70-75), CLI/docs coverage, and existing e2e (tests/e2e/cluster/raft_test.go:279, restore_test.go:922) and antithesis workloads still exercise it.
3. "Unknown leader metadata now follows the existing default linearizable route" — the default branch of extractConsistency leaves the context untouched, so ConsistencyFromContext returns linearizable; asserted by TestExtractConsistency_LeaderIsNotSupported (internal/adapter/grpc/consistency_test.go:77-84), and a manually supplied old CLI value degrades safely the same way.

No missing/partial requirements, no scope creep (every non-obvious edit — profile comments, CLI docs, deleted leader-fallback tests — is a direct consequence of the removal), and no implemented-but-wrong behavior: the stale local route and the syncing-follower fallback (which keeps its recorded barrier and marks the profile forwarded) behave exactly as the updated query-profile.md documents.

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

[NumaryBot]

🛑 Changes requested — automated review

The runtime refactor appears correct, but it removes unique regression coverage for receipt relay through a forwarding path that still exists.

Findings outside the diff

🟠 [major] Preserve forwarded-receipt coverage through the fallback — tests/e2e/cluster/get_transaction_receipt_forwarded_test.go:83

Deleting this test removes the only end-to-end guard that a no-signer follower relays a receipt produced by a remote leader. readCtrl still forwards GetTransaction when a follower is syncing or loses leadership, so retain this scenario using that fallback; regression coverage must remain additive across production triggers per AGENTS.md:78.

Summary: #1893 (comment)

[shipfox-ai]

This PR removes the unreleased leader read-consistency selector end to end — gRPC metadata extraction, the explicit-leader routing branch in RoutedController.readCtrl, the CLI flag help/completion, deleted leader-specific unit and e2e tests, and the matching documentation/proto comments — while retaining stale and degrading unknown x-consistency metadata (including leader) to the default linearizable route. The spec claims are fully delivered and verified in code, with no scope creep. However, the repository's own traceability standard requires a durable, committed decision record for consistency-strategy/API changes, and the only in-repo evidence here is an ambiguous external ticket reference, so this is request changes on that single docs item.

Standards

[P2][blocking] Missing durable intent chain for a significant consistency/API decision — docs/technical/architecture/data-flows.md:557 is the only committed evidence for the removal: "The unreleased v3 x-consistency: leader selector was removed by EN-1946". docs/technical/contributing/product-technical-traceability.md explicitly covers consistency-strategy and API-semantic changes and states that external identifiers are not sufficient: "the committed repository evidence must retain enough requirement and decision context to remain understandable without those private or mutable sources", and "For a significant change with no existing committed evidence, add or update durable evidence in the same PR." No committed decision record for this removal exists anywhere in the repository. Worse, the two other in-repo EN-1946 references (tests/e2e/business/cross_store_value_skew_test.go:136, tests/e2e/business/cross_store_snapshot_alignment_test.go:143) attribute that ticket to a different behavior entirely (fixed main-snapshot horizon alignment waits), so the sole citation is ambiguous even to a reader with repo access. The rationale (e.g. that an isolated former leader can serve stale state in this mode, and that v3 is unreleased so no compatibility burden exists) is currently only reconstructible by a reviewer, which the standard forbids. Resolution: add the need → limitation → requirement → decision → alternatives → validation chain to the owning read-path/consensus documentation (or a focused decision note), and point the PR summary to it. This is a docs-only change; nothing in the implementation needs to move.

[P3][non-blocking] Forwarded-read receipt relay lost its only e2e regression guard — tests/e2e/cluster/get_transaction_receipt_forwarded_test.go was deleted. Its specific trigger (explicit Consistency: leader) was legitimately removed, so the deletion is in scope — but the behavior it guarded survives: on a forwarded read, the contacted node relays the signing node's authoritative receipt verbatim (internal/adapter/grpc/server_bucket.go:316-322, internal/adapter/grpc/client_bucket.go:69-74), now reachable only via the syncing-follower fallback in RoutedController.readCtrl (internal/bootstrap/controller_routed.go:104-131). No remaining spec in tests/e2e/cluster exercises a receipt on a forwarded read (response_signing_test.go is single-node and local-path only), so a regression where a no-signer contacted node drops or re-derives the receipt would no longer be caught end to end. Judgement call: re-aim the heterogeneous-signer spec at the surviving fallback trigger (e.g. a follower still syncing) rather than dropping the coverage outright.

No other confirmed material standards findings: the documentation fan-out is required by the AGENTS.md documentation-maintenance rule and is fully carried through at HEAD (consensus matrix, read-path README, query pipeline/profile, prepared queries, CLI); the protobuf edits are comment-only with no field/renumbering concerns; new/renamed unit tests follow testing.md conventions (t.Parallel(), no sleeps, and TestExtractConsistency_LeaderIsNotSupported uniquely asserts the new fall-through branch).

Spec

No confirmed material findings. All three PR-body claims are delivered and independently verified at HEAD:

1. "Removes the unreleased leader consistency selector" — ConsistencyLeader no longer exists in internal/adapter/grpc/consistency.go; extractConsistency (line ~85) accepts only ConsistencyStale; the explicit-leader branch and markForwardedIfRemote are deleted from RoutedController.readCtrl; the CLI help and enum completion list only stale/linearizable (cmd/ledgerctl/main.go:121-122); a repo-wide search finds no residual leader-selector references in code or authoritative docs (the only mention documents the removal itself, subject to the Standards finding above).
2. "while retaining stale" — stale is still extracted and served locally without a barrier (route local_stale), covered by TestExtractConsistency_Stale and the rewritten whitespace test, and documented in docs/ops/cli.md.
3. "Unknown leader metadata now follows the existing default linearizable route" — x-consistency: leader hits the default branch, leaves the context unchanged, and ConsistencyFromContext returns the ConsistencyLinearizable default, so readCtrl executes ReadIndexAndWait; asserted by TestExtractConsistency_LeaderIsNotSupported (internal/adapter/grpc/consistency_test.go:77-86).

No scope creep: every hunk serves the removal. The e2e test deletion and the comment-only proto regeneration (misc/proto/bucket.proto / internal/proto/servicepb/bucket.pb.go) are consequences of the removed behavior; openapi.yml never exposed x-consistency, so the HTTP surface is unaffected.

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

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1893 (comment)

[shipfox-ai]

Final review — PR #1893 (removal of the unreleased v3 x-consistency: leader read-consistency selector)

This PR cleanly removes the leader read-consistency mode across the gRPC interceptor, routed controller, CLI flag/completions, tests, and eight documentation files, leaving stale and the default linearizable (ReadIndex) paths. I verified the removal is complete (no ConsistencyLeader or leader-selector references remain in production code; the only remaining mention of leader in consistency_test.go:80 is the intentional negative test), that MarkForwarded() now has exactly one production call site so the rewritten query-profile semantics are accurate, and that the rewritten e2e fallback test is technically sound (helper APIs exist, TransferLeadership blocks and reports the new leader, ReadIndex is dispatched per read and failed on leadership loss). Sending x-consistency: leader now silently defaults to the strictly safer linearizable path, which is the intended and tested behavior for this unreleased v3 surface. The code itself is correct and well-tested; the remaining confirmed findings are process/documentation items only.

Recommendation: approve with comments.

Standards

P2 — Durable intent chain for an API/semantic change is not committed

• Location: docs/technical/architecture/data-flows.md:556-558 (the only durable rationale added), with the safety rationale paragraphs deleted from docs/technical/architecture/subsystems/consensus/raft-consensus.md and docs/ops/cli.md.
• Documented standard: docs/technical/contributing/product-technical-traceability.md explicitly lists "API or semantic changes" and "consistency … strategies" as decisions requiring the chain need → limitation → requirement → decision → implementation → validation, and states that external issues may establish the need but "the committed repository evidence must retain enough requirement and decision context to remain understandable without those private or mutable sources."
• Impact: The only committed rationale is that the selector "was removed by EN-1946" — a pointer to an external ticket whose contents are not in the repository. The removed paragraphs contained the actual motivation (an isolated former leader could serve stale local state without a ReadIndex barrier because CheckQuorum is disabled); that reasoning now survives nowhere. A future reviewer or auditor cannot reconstruct from committed evidence why this client-visible mode was removed or what requirement the rewritten regression test (tests/e2e/cluster/get_transaction_receipt_forwarded_test.go) validates.
• Resolution: Add a short durable rationale (need, limitation, requirement, validation) to the owning read-path/API documentation — e.g. in data-flows.md or raft-consensus.md — summarizing why the mode was unsafe and how the fallback test proves the replacement behavior, and reference it from the PR description.

P3 — Hand-edited generated file; confirm just generate-proto is a no-op

• Location: internal/proto/servicepb/bucket.pb.go:8033-8035.
• Documented standard: AGENTS.md:128 ("After modifying .proto files, run just generate-proto immediately") and docs/technical/contributing/protobuf.md require generated .pb.go output to come from the generator, not hand edits.
• Mitigation verified during review: the hand-edited comment is textually identical to the updated source comment at misc/proto/bucket.proto:1352-1354, and protoc copies .proto comments verbatim into the generated Go file, so regeneration should reproduce these bytes exactly. This is why this is P3 rather than blocking.
• Resolution: Run just generate-proto once and confirm zero diff before merge, so the PR checklist item "Protobufs regenerated if modified" (docs/technical/contributing/conventions.md:131) is demonstrably satisfied.

Spec

No confirmed material finding — no spec is available for this change (EN-1946 is a Linear ticket not reachable through the GitHub-only integration; there is no linked GitHub issue and no spec file in the repository). The change's stated scope is nonetheless consistently implemented and validated: the leader constant, routing branch, CLI completion/help entry, and all leader-read documentation are removed while ConsistencyStale and its local-read branch remain; the removed selector's regression coverage is replaced by a test of the remaining automatic leader-fallback path; and internal/adapter/grpc/consistency_test.go:77-86 verifies x-consistency: leader now falls back to the default linearizable behavior.

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

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1893 (comment)

[NumaryBot]

NumaryBot review complete: no remaining inline findings.

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

Summary: #1893 (comment)

[shipfox-ai]

This PR removes the unreleased v3 x-consistency: leader selector (EN-1946) while retaining stale, with unknown leader metadata falling through to the default linearizable route. I verified both candidate reports against the diff and the checkout at HEAD: the removal is complete and consistent (ConsistencyLeader, the readCtrl leader-routing branch, and markForwardedIfRemote are gone with no residual references; extractConsistency accepts only stale; TestExtractConsistency_LeaderIsNotSupported covers the leader→linearizable fallthrough; CLI help/completion and openapi.yml show no surface drift). I also independently inspected the riskiest change — the rewritten e2e receipt test — and found it sound: the gateway interceptor's false-drops-MsgReadIndex semantics, the synchronous TransferLeadership confirmation, the failAllPendingReads(ErrNotLeader) path on lead change, and the signer topology (nodes 1 and 3 sign, node 2 does not) all line up with readCtrl's fallback. Two minor, non-blocking documentation findings remain on the Standards axis; the Spec axis is clean. Recommendation: approve with comments.

Standards

1. [Minor, non-blocking] The consistency-strategy removal lacks the in-repo traceability chain the repo's own contract requires. docs/technical/architecture/data-flows.md:556-559 records only that "the unreleased v3 x-consistency: leader selector was removed by EN-1946" plus the resulting modes. docs/technical/contributing/product-technical-traceability.md lists "API or semantic changes" and "consistency … strategies" as requiring the need → limitation → requirement → decision → validation chain, states an external identifier alone is insufficient, and says that "when classification is ambiguous, document the motivation rather than silently assuming it is exempt." Nothing at HEAD states why the mode was removed — and the hazard rationale the old docs/ops/cli.md / raft-consensus.md text carried (an isolated former leader with CheckQuorum disabled can serve stale state in leader-local mode) was deleted rather than relocated. Mitigating: v3 is unreleased, so there is no compatibility burden and the change is arguably near-mechanical — but then that classification should be recorded explicitly. Resolution: add a concise motivation/validation note to the owning read-path or consensus documentation (or record an explicit, supportable mechanical-maintenance classification) and link it from data-flows.md.

2. [Minor, non-blocking] Forwarded-profile descriptions mention only the syncing-follower trigger, but the fallback also fires on leadership invalidation. RoutedController.readCtrl forwards on both ErrNodeSyncing and ErrNotLeader (internal/bootstrap/controller_routed.go:90), yet the changed text describes forwarding as a "syncing-follower" path only: the Forwarded field comment in internal/query/profile.go:105-116, the readCtrl doc comment at internal/bootstrap/controller_routed.go:56-58, and docs/technical/architecture/subsystems/read-path/query-profile.md:105-108,251-253. The diff's own proto comments (misc/proto/bucket.proto:1352-1356 / internal/proto/servicepb/bucket.pb.go) now correctly describe both causes ("still catching up or its in-flight ReadIndex is invalidated by a leadership change") — and the rewritten e2e test exercises precisely the leadership-invalidation case producing a forwarded read — so the diff is internally inconsistent. Impact: maintainers and query-profile consumers can misread forwarded=true as evidence the replica was syncing. Resolution: align profile.go, the readCtrl comment, and query-profile.md with the proto wording.

Spec

No confirmed material finding. Both stated requirements are implemented and verified at HEAD: the leader selector is fully removed (with stale retained and CLI/HTTP surfaces consistent — openapi.yml never exposed the header), and unknown leader metadata follows the existing default linearizable route via extractConsistency's default arm plus ConsistencyFromContext's linearizable default, covered by TestExtractConsistency_LeaderIsNotSupported. The one candidate spec observation — that the e2e rewrite of tests/e2e/cluster/get_transaction_receipt_forwarded_test.go goes beyond adjusting to the removal — was rejected after verification: the old spec exercised the now-removed Consistency: leader trigger and would be dead coverage; the rewrite preserves the surviving forwarded-receipt relay behavior via the pre-existing leadership-change fallback and adds no unrequested production behavior.

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 #1893 "refactor(reads): remove leader consistency selector (EN-1946)"

Fixed point 6cef75d → HEAD f7154c7 (1 commit, 16 files, +158/−202). Both candidate reports were re-verified line-by-line against diff.patch and the HEAD checkout. The spec of record is fully delivered: ConsistencyLeader and the explicit leader routing branch are gone with zero residual references repo-wide (only the historical note at docs/technical/architecture/data-flows.md:557 remains, intentionally); stale is retained end-to-end (interceptor, readCtrl local route, CLI flag/help/completion, docs, antithesis and e2e workloads); and x-consistency: leader now falls through extractConsistency (internal/adapter/grpc/consistency.go:81-86) to the default ReadIndex-based linearizable route, asserted by TestExtractConsistency_LeaderIsNotSupported. The rewritten e2e receipt test was independently checked against the harness (WithNodeInstruments is 0-based while node IDs are 1-based; the gateway interceptor passes/blocks as the test assumes; internal/infra/node/node.go:1089-1093 fails pending ReadIndex requests with ErrNotLeader on any observed leader change; TransferLeadership waits server-side and echoes the transferee) — its synchronization is deterministic and consistent with the repo's no-time.Sleep rule. The deleted unit/e2e tests only covered production triggers this same diff removed, so the "regression coverage must not replace an existing trigger" rule is not violated. No compatibility burden applies (v3 unreleased, and openapi.yml never exposed the header).

Recommendation: approve with comments. The code, behavior, and tests are correct; the two findings below are documentation/traceability gaps that should be addressed in or immediately after this PR. No code changes are required.

Standards

[P2] Durable traceability for the consistency semantic change is incomplete

docs/technical/architecture/data-flows.md:557, PR-wide — standard: docs/technical/contributing/product-technical-traceability.md.

Removing a consistency selector is explicitly in scope for that standard ("API or semantic changes", "consistency … strategies"), which requires committed evidence for the full chain (need → limitation → requirement → decision/alternatives → validation) that "must retain enough requirement and decision context to remain understandable without those private or mutable sources". The only new durable rationale is the single sentence at data-flows.md:557, which points at external ticket EN-1946 for the why. Worse, this diff deletes the previous in-repo statement of the limitation — the isolated-former-leader staleness caveat in docs/ops/cli.md and the corresponding raft-consensus.md paragraphs — without retaining it anywhere as the decision's justification. A future reviewer cannot reconstruct from the repository why the selector was removed (risk? unused? redundancy with linearizable?), what alternatives were considered, or how the removal was validated. Add a short decision paragraph (e.g. to raft-consensus.md or the read-path README) recording the need, the limitation, and the alternatives, and reference it from the PR description per the standard's summary template.

[P3] Forwarding-cause comments name only one of the two triggers

internal/query/profile.go:106-116, internal/bootstrap/controller_routed.go:57-58, docs/technical/architecture/subsystems/read-path/query-profile.md:123.

The fallback in readCtrl fires on either ErrNodeSyncing or ErrNotLeader (internal/bootstrap/controller_routed.go:91), and node.go:1089-1093 fails pending ReadIndex requests with ErrNotLeader whenever the observed leader changes — exactly the path this PR's own e2e test exercises. Yet profile.go:106 says "Forwarded is true when a syncing follower routed the read to the leader" and controller_routed.go:57 says forwarding happens "if the local node is still syncing", both omitting leadership invalidation — while the same PR's bucket.proto/bucket.pb.go comment correctly names both causes, making the godoc internally inconsistent with the wire contract. Additionally, query-profile.md:123 retains "the common cause is a caller asking for read-your-writes", which is vestigial after this change: per the new paragraph at query-profile.md:105, a forwarded non-zero barrier now always means a failed local ReadIndex attempt (syncing or leadership churn), and "read-your-writes" is merely the default level, not a distinguishing cause. Impact: readers of the godoc will conclude forwarding implies replica lag and may dismiss leadership-churn causes when triaging profiles. Update both comments to name both triggers and drop or rewrite the stale sentence. (The subsystem docs query-profile.md:105-113 and the proto comment already state both causes correctly, which is why this is P3 rather than P2.)

Rejected from the candidate reports, after verification: GLM's two minor smell observations (duplicated TransferLeadershipRequest construction, forwarded-receipt-* file naming) are style nitpicks with no correctness, security, compatibility, or test-risk impact; GLM's claim that the forwarding comments are fully accurate is contradicted by controller_routed.go:91 and is superseded by the P3 above; Codex's P2-blocking framing of that finding is downgraded because the authoritative operator-facing docs already name both causes.

Spec

No confirmed material finding. All three requirements of the PR body were verified directly against the checkout:

• "Removes the unreleased leader consistency selector" — ConsistencyLeader has zero matches repo-wide; internal/adapter/grpc/consistency.go defines only stale and linearizable; the leader branch, markForwardedIfRemote, and their tests are deleted with no dangling references; CLI help and RegisterEnumCompletion list only stale, linearizable (cmd/ledgerctl/main.go:121-122); all leader read-path docs (CLI, raft-consensus, read-path README/prepared-queries/query-pipeline/query-profile, data-flows) and the proto + generated comments were updated in sync; remaining "leader" mentions are unrelated domains (cluster admin, conf-change, metrics, backup) or the deliberate historical note.
• "while retaining stale" — extractConsistency still accepts ConsistencyStale; readCtrl serves it locally without a barrier (route: local_stale); docs and workloads preserved.
• "Unknown leader metadata now follows the existing default linearizable route" — the switch at internal/adapter/grpc/consistency.go:81-86 leaves the context untouched for leader, so ConsistencyFromContext returns ConsistencyLinearizable, covered by TestExtractConsistency_LeaderIsNotSupported (consistency_test.go:77-86). The silent fall-through (rather than an error) matches the spec exactly, and v3's unreleased status removes any compatibility obligation.

Independent inspection of the riskiest areas (e2e harness semantics, node ReadIndex failure paths, gateway interception, CLI/validation surface, openapi.yml, residual references) surfaced no additional material issue on either axis.

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

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #1893 (comment)

[NumaryBot]

NumaryBot review complete: no remaining inline findings.

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

Summary: #1893 (comment)

[shipfox-ai]

This PR removes the unreleased x-consistency: leader read-consistency selector while retaining stale, and routes previously leader-selected reads through the default linearizable path. I verified the implementation end to end: ConsistencyLeader is fully removed with no dangling references, extractConsistency now ignores leader metadata so ConsistencyFromContext falls back to ConsistencyLinearizable, and RoutedController.readCtrl runs every non-stale read through the ReadIndexAndWait barrier with the existing syncing/leadership-change follower fallback intact. The CLI flag, completions, architecture docs, proto/generated comments, and profile comments are all updated in step, and the rewritten e2e regression test deterministically covers the only remaining forwarded-read path. Recommendation: approve with comments — one documentation-completeness item under the repository's traceability standard should be addressed before or shortly after merge.

Standards

1. Missing durable product-to-technical traceability for an API/consistency semantic decision (moderate)

Location: docs/technical/architecture/data-flows.md:557, with the corresponding behavior change in internal/adapter/grpc/consistency.go (extractConsistency) and internal/bootstrap/controller_routed.go (readCtrl).

docs/technical/contributing/product-technical-traceability.md requires the full need → limitation → requirement → decision → alternatives → validation chain for API or semantic changes and consistency-strategy changes, and docs/technical/agent-context.md (line 145) makes that document the required source of review intent for significant decisions. The only durable in-repo evidence added is a single sentence: "The unreleased v3 x-consistency: leader selector was removed by EN-1946; callers choose between the default linearizable route and an explicit stale local read." That records the decision and its replacement, but not why the old behavior was insufficient, the observable requirement, why removal is proportionate, the alternatives considered (including doing nothing), or how validation proves the outcome. Per the standard, a Jira reference alone is not sufficient: "the committed repository evidence must retain enough requirement and decision context to remain understandable without those private or mutable sources." Impact: a future reviewer or auditor cannot reconstruct the motivation for removing an externally selectable consistency mode from the repository alone.

Resolution: extend the note in docs/technical/architecture/data-flows.md (or the owning read-path/consensus documentation) with the concise motivation and validation chain, and point the PR summary at it.

No other confirmed standards findings. The broad edits across CLI, architecture docs, tests, proto, and generated code are the documentation/protobuf synchronization the repository's own standards mandate, not shotgun surgery; the regenerated bucket.pb.go comment matches misc/proto/bucket.proto verbatim; and the rewritten tests keep unique, additive branch coverage (the whitespace test still uniquely exercises TrimSpace via " stale ").

Spec

No confirmed material findings. Verified against the PR intent ("Removes the unreleased leader consistency selector while retaining stale. Unknown leader metadata now follows the existing default linearizable route"):

• Selector removed completely. ConsistencyLeader is deleted (internal/adapter/grpc/consistency.go), the leader routing branch and markForwardedIfRemote are removed from readCtrl (internal/bootstrap/controller_routed.go), CLI help/completions drop leader (cmd/ledgerctl/main.go:121-122), and a repo-wide search finds no remaining ConsistencyLeader/x-consistency: leader usage outside the intentional removal note.
• Stale retained. The ConsistencyStale constant, the case ConsistencyStale branch, the local_stale route, CLI flag text, and e2e stale usages are behaviorally untouched.
• Leader metadata → linearizable. extractConsistency's default branch returns the context unchanged and ConsistencyFromContext defaults to ConsistencyLinearizable; TestExtractConsistency_LeaderIsNotSupported asserts exactly this, and the updated raft-consensus.md claim ("All non-stale reads require a successful ReadIndex, including reads served on the node that currently reports itself as leader") matches the code.
• The rewritten e2e test is necessary, not scope creep. The old test exercised the now-removed Consistency: leader path, so a rewrite was forced. I independently verified the replacement's choreography is sound and deterministic: internal/infra/node/node.go:1092-1094 fails pending ReadIndex futures with ErrNotLeader on any observed leader change, which triggers the leader_fallback branch under test; TransferLeader blocks until the transfer is confirmed (node.go:1873-1903); the gateway interceptor (pkg/testserver/gateway.go) returns false = drop, and the test synchronizes via channels and Eventually with no time.Sleep; and the signer topology (nodes 1 and 3 sign, node 2 never) guarantees the fallback target is always a signer. No flakiness or risky-test defect found.

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