Automated review result: changes requested (static review of head 2e7d6a02e4c7b78403c1304856e7ab0aa4c0df71).

One blocking correctness issue is reported inline: the new physical-to-constant cache upgrade can feed ConstantColumnReader into eager physical inverted-index initialization and turn a previously successful hidden commit-TSO query into an initialization error.

Critical checkpoints:

• Goal, scope, and correctness: the focused cache suitability check fixes the reported physical-reader hit for ordinary row/segment-zone-map reads, and the same-key upsert fixes duplicate LRU nodes. It is not correct for the indexed continuation described inline.
• Concurrency and lifecycle: cache map/list mutation is mutex-protected; splice preserves the stored iterator; returned shared_ptrs remain valid across replacement/eviction. Last-writer-wins construction creates no second distinct defect.
• Configuration, compatibility, persistence, and writes: no new configuration, FE/BE field, RPC, serialized format, transaction/write/recovery behavior, or rolling-upgrade state is introduced.
• Parallel and special paths: constant data and segment-zone-map consumers are correct; page-zone-map, bloom, and later predicate-pruning paths are conservative. The eager inverted-index consumer is the unsafe exception. The separately tracked placeholder zone-map-builder gap in #67995 was not duplicated.
• Tests and results: the added unit tests distinguish the direct O-to-C cache fix and stale-LRU-node fix, but do not cover the physical-warm plus hidden-index continuation that exposes the inline issue. Per the review contract, no local build or tests were run. At this reviewed head, GitHub reports BE UT, COMPILE, P0, NonConcurrent, External Regression, Cloud regression, coverage, formatter, and style checks passed; performance remains pending. These are CI claims, not independent execution.
• Observability and performance: cache counters remain internally consistent with resident-key lookup semantics, and the upsert remains O(1); no additional substantive metric or hot-path regression was found.
• User focus: no additional review focus was supplied.

Review completion: complete after three bounded rounds. All candidates were independently verified and deduplicated; the accepted set is the single inline issue below, with no unresolved or capped finding.

Automated review result: changes requested (static review of exact head 3c7b4c426417d2f96beab15082a670fd7fc54d1b).

One blocking correctness issue is reported inline: the new same-key upsert allows an in-flight physical-reader miss to overwrite a completed constant-reader upgrade, so placeholder index or zone-map state can still drive filtering.

Critical checkpoints:

• Goal and proof: the sequential physical-to-constant cache upgrade and constant-reader index dispatch are fixed and unit-covered, but the stated authoritative-constant behavior is not preserved under concurrent misses.
• Scope: the four-file change is focused on reader polymorphism, cache replacement, and targeted BE unit tests.
• Concurrency: map/list operations are mutex-protected, but lookup, construction, and upsert form a semantic lost-update race; this is the blocking P1. No additional lock-order or deadlock issue was found.
• Lifecycle/static initialization: the non-null constant is the immutable commit TSO for one rowset/segment identity; reader/iterator ownership remains safe, with no cycle or cross-TU initialization issue.
• Configuration: no configuration item is added or changed.
• Compatibility/rolling upgrade: no wire, storage-format, RPC, or persisted-state change is introduced; the virtual dispatch change is internal to BE.
• Parallel paths: stable fresh-null inverted/ANN fallback retains row evaluation correctly. Bare index and expression-zone-map consumers must observe the authoritative constant, which the accepted race can violate. The existing sequential index-dispatch thread is not duplicated.
• Conditional checks: the new is_constant() condition explains and handles the sequential cases, but it does not define the concurrent winner at insertion.
• Test coverage: sequential replacement, one-node LRU accounting, constant zone-map/type behavior, and direct index no-op are covered. A barrier-controlled mixed physical/constant miss test and authoritative-winner return assertion are missing.
• Test results: no builds or tests were run locally because the review contract requires static-only review; no .out files apply.
• Observability: cache gauges remain balanced; no additional required log or metric was identified.
• Transaction/persistence: no EditLog, transaction, publish, or recovery data is changed.
• Data writes/crash safety: no write path or on-disk mutation is modified.
• Cross-layer variables: no new FE-BE variable or protocol field is introduced.
• Performance/memory: replacement remains O(1), avoids duplicate LRU nodes, and adds no row-loop work or ownership leak.
• Other issues: different-constant reuse and reused non-null index-output hypotheses were dismissed with call-path evidence; the separate bare placeholder zone-map issue is already tracked/fenced and was not duplicated.

User focus: no additional review focus was supplied.

Review completion: complete after two bounded rounds. The second normal and risk-focused reviews both returned NO_NEW_VALUABLE_FINDINGS; all candidates are accepted, dismissed with evidence, or duplicate-fenced. Accepted set: the single inline P1 below.

Automated review result: changes requested (static review of exact head 3c7b4c426417d2f96beab15082a670fd7fc54d1b).

One new blocking correctness issue is reported inline: ordinary LRU eviction can discard the only marker that commit TSO is logically constant between column-iterator and index-iterator setup, after which a bare lookup rebuilds and uses the placeholder's physical index.

The existing concurrent mixed-miss P1 remains valid in discussion_r4025665398 and is not duplicated. The earlier physical-index-dispatch P1 in discussion_r4017097815 is addressed by the new virtual constant no-op.

Critical checkpoints:

• Goal and proof: sequential physical-to-constant replacement, one-node LRU replacement, and resident-constant index fallback work, but constant authority is still lost after normal eviction and during the already-reported concurrent miss.
• Scope: the four-file change is focused on reader polymorphism, cache replacement, and targeted BE unit tests.
• Concurrency and lifecycle: map/list mutation is mutex-protected, splice preserves iterators, shared ownership is safe, and no deadlock, cycle, destructor, or static-initialization issue was found. The existing semantic lost-update race remains; the new inline issue is deterministic and single-threaded.
• Configuration: no setting is added, but the existing mutable cache capacity makes the eviction failure deterministic at one and still possible at the default capacity on a sufficiently wide indexed scan.
• Compatibility and rolling upgrade: no wire, RPC, storage-format, or persisted-state change is introduced; the virtual dispatch is BE-internal.
• Parallel paths: scalar/common-expression pruning, statistics, row/rowid reads, page zone maps, bloom/dictionary fallback, repeated inverted/ANN setup, variant readers, and cache destruction were traced. Constant methods are safe while the constant reader remains selected; a bare post-eviction miss loses that selection. The separate pre-constant builder issue tracked by #67995 was not duplicated.
• Conditional checks: is_constant() handles a resident sequential entry but cannot preserve semantics once that entry is absent. Preserving a non-null index output is intentional multi-index behavior, so an unconditional reset was rejected.
• Tests and results: the added tests cover sequential replacement, LRU uniqueness/eviction, constant zone-map/type behavior, and an initially-null index no-op. They do not compose eviction between row and index setup; a low-capacity end-to-end regression is required. The mixed-miss barrier test remains requested by the existing P1. No builds or tests were run locally because this review contract is static-only; no result files apply.
• Observability: cache accounting remains balanced; no separate logging or metrics defect was found.
• Transactions, persistence, writes, and crash safety: no applicable path is changed.
• Cross-layer variables: no FE-BE field or protocol variable is added.
• Performance and memory: lookup/replacement remain O(1), virtual dispatch is outside row-hot loops, and no leak or unbounded retention was found; a fix should preserve bounded reader caching while retaining a small authoritative semantic marker if needed.

User focus: no additional focus was supplied.

Review completion: complete after three bounded rounds. The final cache/header, end-to-end/test, and risk-focused passes all returned NO_NEW_VALUABLE_FINDINGS; every candidate is accepted, dismissed with evidence, or duplicate-fenced. Accepted new set: the single inline P1 below.

Automated review result: changes requested (static review of exact head 2e7d6a02e4c7b78403c1304856e7ab0aa4c0df71).

No new distinct inline comment is added in this pass: all substantiated failures are already covered by the existing review context. The current-head physical-to-constant/index failure remains in discussion_r4017097815, and the mixed-miss overwrite remains in discussion_r4025665398. The low-capacity eviction form is covered by discussion_r4032777651. The author's replies defer schema-derived protection to #67995, but that issue is still open and the protection is not present in this diff, so this head is not independently safe to merge.

Critical checkpoints:

• Goal and proof: the change correctly upgrades a sequential physical cache hit to a constant reader and repairs same-key LRU-node replacement. The two added unit tests directly prove those narrow cache contracts. They do not prove safe continuation into all physical-index consumers.
• Scope and clarity: the three-file change is focused, and the discriminator plus in-place splice are locally clear. The remaining correctness gap is a cross-consumer contract, not unrelated scope.
• Concurrency: map/list operations are protected by one mutex; splice preserves iterators and copied shared_ptrs remain alive. Construction outside the lock leaves the already-reported mixed physical/constant lost-update schedule; no fourth distinct race survived review.
• Lifecycle and static initialization: the non-null constant is the immutable commit TSO for one rowset/segment cache identity. Publish has only a pre-visible -1 to final transition, and compaction/schema-change outputs use separate rowset-scoped caches. No cycle, release, destructor, or cross-TU initialization problem was found.
• Configuration: no setting is added. The mutable capacity controls ordinary LRU eviction, which is why the existing eviction thread remains relevant; general runtime-shrink behavior predates this PR.
• Compatibility and rolling upgrade: no wire, RPC, storage-format, persisted-state, or FE/BE-variable change is introduced. The virtual discriminator is internal to an already-polymorphic BE hierarchy.
• Parallel and special paths: row production and segment-zone-map operations dispatch safely to constant implementations, while page-zone-map and Bloom paths decline pruning conservatively. The non-virtual physical new_index_iterator boundary remains unsafe when it receives the cached constant; eviction or the mixed-miss race can instead restore the placeholder's physical reader/index. These are the existing fenced P1s, not new comments.
• Conditional checks: the const_value/is_constant() condition is sufficient for a resident sequential entry, but cache residency and arrival order cannot encode the schema-level placeholder invariant required by later bare consumers.
• Test coverage and results: the new unit tests cover sequential replacement, constant zone-map/type behavior, one-node LRU accounting, and eviction ordering. They do not cover the already-requested indexed continuation, barrier-controlled mixed miss, or low-capacity end-to-end sequence. Per the review contract, no local build or test was run. At final refresh, GitHub reported compile, BE UT, P0/non-concurrent/external/cloud regression, coverage, formatter, and style checks passing; those are CI claims, not independent validation.
• Observability: entry-count and eviction accounting remain balanced. A proposed hit/miss telemetry concern was dismissed after repository-wide search found no consumer or documented contract stronger than key-residency lookup semantics. No new required log or metric was identified.
• Transactions, persistence, writes, and crash safety: the PR changes read-side reader selection only; no EditLog, publish metadata, transaction atomicity, delete bitmap, or data-write path is modified.
• Performance and memory: lookup/replacement remain O(1), the virtual check is outside row-hot loops, and ownership remains shared_ptr-safe. No material allocation, leak, or hot-path regression was found.
• User focus: no additional review focus was supplied.

Review completion: converged after two bounded rounds. All Round 2 normal and risk-focused reviews returned NO_NEW_VALUABLE_FINDINGS; every candidate is accepted, duplicate-fenced, or dismissed with concrete evidence. The new inline set is empty solely because the blocking correctness issues are already posted above.