Static review of all 19 changed files at e3948fd. One P2 finding: the custom Hudi and Iceberg delete-file Parquet factories omit the cache, retaining per-file counter registration on those paths. No additional verified defect was found. There were no existing inline threads to carry forward and no additional user-provided focus.

Required checkpoint conclusions:

• Goal and proof: cached generic readers avoid repeated registration and reduce Variant ownership copies; the inline finding identifies uncovered native paths. Added unit tests inspect identity, concurrency, lock avoidance, aggregation, isolation, disabled profiles and retained lifetime.
• Scope and clarity: changes are focused on profile handles, ownership and propagation across the scan stack; decoding and table semantics are unchanged.
• Concurrency: scanner workers share one cache per local scan state. call_once publishes its immutable handles; existing atomic counter updates remain additive. No new reverse lock order was found; registration is confined to initialization.
• Lifecycle and static initialization: worker context ownership retains RuntimeState/LocalState during cache use. Close does not dereference the cache. Deferred Variant state copies retain the shared counter bundle, including selected/subtree states. Disabled defaults are function-local statics with no cross-translation-unit dependency.
• Configuration: no new configuration or dynamic-update behavior.
• Compatibility: constructor changes are internal and default the cache for standalone readers; no wire or stored-format change.
• Thrift/protobuf: no changed definitions or field IDs.
• Parallel paths: Hive, native Iceberg data, Paimon and Fluss forwarding inspected; Hudi base files and Iceberg delete files are the omissions in the inline finding. Other concrete FileReader initialization paths retain their explicit profile setup.
• Conditional checks: cached/standalone and enabled/disabled profile branches have concrete ownership purposes and explanatory comments; no silent error continuation introduced.
• Test coverage: new tests exercise the cache directly and direct ParquetReader use, but do not exercise cache consumption through the custom table factories. Actual wrapper-path coverage should accompany the fix.
• Test-result changes: no expected-result artifacts were changed.
• Observability: counter names, units, hierarchy and per-reader delta snapshots are preserved; cached and fallback readers safely reuse counter identities.
• Transactions and persistence: no transaction, EditLog, failover or persistence behavior changed.
• Data writes and atomicity: no write or commit path changed.
• FE/BE variables: no new transmitted variable or sending path.
• Performance: lazy registration and one retained Variant bundle remove repeated work where the cache is consumed; the finding covers missed factories. Benchmark claims were not independently reproduced.
• Other invariants: no additional data-correctness, version/delete visibility, memory ownership or error-propagation regression was established in the changed paths.

Validation limits: static review only, as required; no builds, tests, sanitizer runs or performance measurements were executed. Added tests and the author's reported results were inspected, not reproduced. No unresolved code-path candidate remains beyond the verified finding.

Review converged after 2 round(s).

Reviewed PR #69300 at 83312d80085fa2aff2bfd494dd06f6357b3ee626 using the supplied aggregate diff and changed-file list. One new P2 finding: Iceberg $position_deletes scans still register Parquet counters for each split because their nested reader omits the new cache. No new P0/P1 or correctness defect was substantiated.

Existing review context: thread 4317030285 is fixed in this head. Both the Hudi custom factory and the ordinary Iceberg delete-file factory now pass the scan cache; the new system-table finding follows a different class and child profile. No existing P0/P1 remains to carry forward. There were no additional user-provided focus points.

Critical checkpoint conclusions:

Static review covered all 22 changed paths and related control flow. No builds, tests, source changes or runtime validation were performed, as required by the review contract. No unresolved candidate or static coverage gap remains. Review convergence is reported separately by the delivery metadata.

Review converged after 2 round(s).

No new actionable findings in the 25 changed paths at eb06287. The two existing P2 concerns are fixed: the Hudi and ordinary Iceberg delete-file factories forward the cache, and the position-delete system table resolves a distinct child-profile cache across splits and scanners. No existing P0/P1 finding remains to carry forward. No additional user focus was supplied.

The code-review skill checkpoints have these conclusions:

• Goal and proof: the code removes repeated Parquet counter registration and the duplicate common init hook. Added tests check actual reader initialization, shared counter identity and accumulation, independent scans, nested factories, concurrent publication and retained Variant ownership.
• Scope and abstraction: changes stay focused on profiling ownership, initialization and forwarding. Headers use forward declarations where ownership permits; physical decoding and table-schema mapping remain in their existing layers.
• Concurrency and critical state: scan workers share immutable counter handles published by std::call_once. The child-cache map has its own mutex; RuntimeProfile serializes registration and child creation. Counter values retain their atomic update behavior.
• Lock cost and order: the child-cache lock covers lookup/allocation only, and counter registration occurs after it is released. Profile child creation also releases its lock before cache lookup. No new reverse lock order was found, and warm cache reads avoid the counter-registration lock.
• Lifecycle and static initialization: workers retain the PipelineFragmentContext, which owns RuntimeStates and their local states/cache, even during asynchronous scanner stop. Queued tasks with expired execution context return before scanner access. Deferred Variant states retain the shared counter bundle without a profile/cache ownership cycle. No cross-translation-unit initializer dependency was introduced.
• Configuration: no configuration item or dynamic-update behavior changes.
• Compatibility: constructor additions preserve existing callers through a default null cache. No stored-file, on-disk metadata or wire-format change requires rolling-upgrade adaptation.
• Thrift/Protobuf IDs: no gensrc definitions changed; branch field-ID comparison is not applicable.
• Parallel paths: generic, Hudi, ordinary Iceberg delete, nested position-delete, Paimon and Fluss routes were traced. Standalone schema/row-id readers keep the uncached fallback. ORC and other concrete readers retain their own initialization; removing the base hook does not omit a concrete profile setup.
• Conditional checks and failures: null-cache fallback and child-cache creation have reachable standalone/nested uses and explanatory comments. Status/error propagation and null-profile behavior remain intact; no silent-success failure path was found.
• Test coverage: focused unit tests exercise the modified boundaries, including negative null-profile behavior and contention/lifetime cases. Hudi and nested tests inspect real factories without reading data; Parquet and Iceberg delete-file tests open actual local files. Full runtime, sanitizer, cancellation and performance validation was not reproduced in this static-only review.
• Test-result changes: no regression .out files changed; the new unit-test assertions match the counter identity/aggregation contracts. Their execution results remain unverified here.
• Observability: existing names, hierarchy and accumulation remain; nested position-delete counters stay on the child profile, and non-Parquet scans do not eagerly acquire Parquet counters. No new per-row logging or timers were added.
• Transactions and persistence: no transaction, EditLog, recovery or persistent state changes; visibility/version and MoW delete-bitmap invariants are unaffected by the reviewed changes.
• Data writes and crash atomicity: no data-write or commit path changes; resource lifetime changes were reviewed through scanner shutdown and deferred materialization.
• FE/BE propagation: no new transmitted variable or sending path; the cache is an in-process observer propagated through the relevant reader factories.
• Performance: initialization work moves from each file to one counter set per scan profile, with one retained Variant bundle reference per copy. Child caches are retained rather than recreated per split. The author's benchmark gains were not independently verified.
• Other risks and user concerns: cache identity, teardown, missing parallel initialization and deferred ownership risks were dismissed with caller evidence. Both existing review threads were revalidated without duplicate comments; no unresolved code candidate remains.

Coverage is complete for the supplied aggregate diff and affected call chains. Validation was static only: no builds, tests, source modifications or benchmark runs were performed under this review contract.

All three round-1 reviewers returned NO_NEW_VALUABLE_FINDINGS on the unchanged empty finding set; main verification dismissed every initial risk and recorded no unresolved candidate. Review converged in one round.

Review converged after 1 round(s).

Reviewed all 25 changed files (19 production files and six test files) at 024ea81a94780949e4a26d6ebe4f78f0a1495964. No new actionable findings. The review converged in round 1: both complete-scope reviewers and the dedicated risk reviewer returned NO_NEW_VALUABLE_FINDINGS, and all risk questions and candidate dispositions were independently checked.

Both existing P2 issues are fixed at this head: custom Hudi/Iceberg factories now forward the cache, and nested position-delete system-table readers retain a separate cache for their child RuntimeProfile across splits/scanners. Neither is reposted. No existing P0/P1 blocker remains in the reviewed thread context. No additional user-provided review focus was supplied.

1. Goal and evidence: the cache registers Parquet handles once per scan/profile; the Variant bundle reduces ownership copies to one shared reference. Added tests inspect identity, aggregation, registration-lock avoidance, repeated files, custom factories and retained counter lifetime.
2. Scope and design: the changes are focused on counter ownership, forwarding and removal of duplicate initialization. Schema mapping, decoding, predicate/delete semantics and physical file selection are unaffected by the changed logic.
3. Concurrency and locking: scanner workers share immutable handles published by call_once; child-cache insertion uses its own mutex and counter values remain atomic. Child-map locks are released before profile initialization. No new lock inversion or production thread entry was found.
4. Lifecycle and static initialization: worker execution pins the fragment context and its RuntimeState/local cache through scanner use and cleanup. Close preserves cache/profile ownership; failed opens finalize, and recursive rebuild creates a new fragment context. Deferred Variant states retain their shared counters after scanner/profile destruction without a cycle. Disabled bundles use function-local initialization; no cross-TU static dependency is introduced.
5. Configuration: no configuration items are added; dynamic updates are not applicable.
6. Compatibility: changes are internal source interfaces with standalone defaults. No storage/wire format or dependency/toolchain change requires rolling-upgrade compatibility handling; binary artifacts were not built in this review.
7. Thrift/protobuf: no definitions are changed, so branch field-ID comparison is not applicable.
8. Parallel paths: generic, Hudi and Iceberg factories plus Hudi/Paimon/Fluss hybrid, union-lake and tail-reader paths forward the cache with the matching profile. Nested position deletes use child counters; standalone/no-profile paths retain their defaults. Every current concrete reader that needs profile initialization still calls it explicitly.
9. Conditional checks: the cache-present, standalone and child-ownership branches correspond to concrete lifecycle cases and have explanatory comments; no changed decision suppresses a data error.
10. Test coverage: six changed BE test files cover the central cache, ownership and factory contracts. Fluss/Paimon forwarding, cancellation and recursive teardown were traced statically. The patch adds no SQL integration tests; no missing coverage was tied to a verified defect.
11. Test results: no expected-result artifacts change. The new assertions match the inspected identity, accumulation and ownership contracts. Author-reported test and benchmark results were not independently executed.
12. Observability: counter names, types and hierarchy remain intact. Per-file statistics and published-delta snapshots remain reader-local, while nested counters stay on the child profile; no additional metric family is needed for handle reuse.
13. Transactions and persistence: no visibility/version, delete-bitmap, EditLog, replay or failover logic changes.
14. Data writes and crash behavior: no data-writing path changes. New query-scoped owners use RAII and introduce no persistent recovery state.
15. FE/BE variables: no transmitted field is added; the new option is internal BE plumbing.
16. Performance: repeated Parquet counter registration and per-counter shared ownership copies are removed. Cache creation is bounded by scan/profile count, with no added per-row work or changed data I/O. Speedup magnitude was not independently measured.
17. Other invariants: no substantiated change to row results, nullability, error propagation or resource ownership was found. The incomplete-type constructor concern was dismissed by the actual header include chain.

Validation was static only, as required: no builds, tests, sanitizer runs or performance measurements were performed. No changed paths remain uncovered and no candidate or risk question remains unresolved within that scope.

Review converged after 1 round(s).