[Ali2Arslan]

Summary

MiniBlockChunkIndex is cached per page and column. Its RowMapping kept the
nested-only prefix sums, trailer bitset, and item counts inline, so the largest
variant increased the resident footprint of every flat mapping.

This moves nested-only metadata behind a Box and stores trailer bits as a
bit-packed Box<[u8]> without an Arrow buffer header. It also derives the chunk
count from byte_starts instead of duplicating it in UniformFlat. The current
Planar sparse path and shared MiniBlockScheduler are unchanged, and there is no
file format change.

Evidence

On the same three-chunk fixtures used by the chunk-index tests, the inline
MiniBlockChunkIndex representation drops from 144 B to 72 B on the 64-bit test
build. The cache's modeled DeepSizeOf also drops by 72 B for the uniform and
flat fixtures. Nested pages pay one additional Box allocation and indirection,
and their modeled weight rises by 8 B. This is representation evidence, not an
RSS or latency benchmark.

[coderabbitai]

📝 Walkthrough

Walkthrough

Sparse primitive decoding replaces cached chunk metadata and cumulative offsets with MiniBlockChunkIndex. Nested row mappings now use boxed trailer bitsets, while scheduling, value slicing, and compactness tests use the updated index representations.

Changes

Compact chunk index migration

Layer / File(s)	Summary
Refactor nested row mappings rust/lance-encoding/src/encodings/logical/primitive/chunk_index.rs, rust/lance-encoding/src/encodings/logical/primitive.rs	Nested mappings now wrap prefix sums, item counts, and boxed trailer-bit storage; parsing and index operations use the new representation.
Build and cache the sparse value index rust/lance-encoding/src/encodings/logical/primitive/sparse.rs, rust/lance-encoding/src/encodings/logical/primitive.rs	Removes legacy chunk metadata and offsets, validates metadata-derived ranges, constructs MiniBlockChunkIndex, and caches it in sparse structural state.
Use and validate compact indexes rust/lance-encoding/src/encodings/logical/primitive/sparse.rs, rust/lance-encoding/src/encodings/logical/primitive.rs	Updates chunk scheduling and value slicing to use index lookups, and tests row-mapping shapes, boundary cases, and deep-size accounting.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• lance-format/lance#7651: Continues the mini-block compact chunk-index work in nested mappings and sparse decoding.
• lance-format/lance#7756: Touches sparse value-path integration and per-chunk value metadata.
• lance-format/lance#7889: Modifies the sparse structural page read and value-chunk scheduling pipeline.

Suggested labels: enhancement

Suggested reviewers: xuanwo

Sequence Diagram(s)

sequenceDiagram
  participant PageInitialization
  participant build_value_chunk_index
  participant SparseStructuralCacheableState
  participant schedule_ranges
  participant MiniBlockChunkIndex
  participant append_value_range
  PageInitialization->>build_value_chunk_index: metadata bytes
  build_value_chunk_index->>SparseStructuralCacheableState: constructed value_index
  PageInitialization->>schedule_ranges: cached value_index
  schedule_ranges->>MiniBlockChunkIndex: find_chunk for visible range
  schedule_ranges->>append_value_range: selected chunk range
  append_value_range->>MiniBlockChunkIndex: first_row and items_in_chunk

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately summarizes the sparse encoding cache compaction change.
Description check	✅ Passed	The description is clearly related to the chunk-index refactor and memory reduction changes in the PR.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

rust/lance-encoding/src/encodings/logical/primitive/sparse.rs (1)

5654-5670: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider covering the enlarged-last-chunk case. The test exercises uniform_flat and flat, but not a UniformFlat page whose last chunk holds more values than the non-last chunks (e.g. [(_, 2), (_, 4)]), which is where the arithmetic row→chunk mapping is most fragile. Adding a case that both initializes and resolves a value inside that last chunk would guard the concern raised on the build_value_chunk_index uniform selection.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/lance-encoding/src/encodings/logical/primitive/sparse.rs` around lines
5654 - 5670, Extend value_chunk_index_stays_compact_across_row_mappings to cover
a UniformFlat layout with a larger final chunk, such as value counts 2 then 4.
Initialize this case through init_value_index and resolve an index targeting a
value in the final chunk, verifying the row-to-chunk mapping succeeds and
remains correct.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@rust/lance-encoding/src/encodings/logical/primitive/sparse.rs`:
- Around line 5654-5670: Extend
value_chunk_index_stays_compact_across_row_mappings to cover a UniformFlat
layout with a larger final chunk, such as value counts 2 then 4. Initialize this
case through init_value_index and resolve an index targeting a value in the
final chunk, verifying the row-to-chunk mapping succeeds and remains correct.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 12680847-b85e-4db3-b13c-561a3cd669cd

📥 Commits

Reviewing files that changed from the base of the PR and between 3a72f8a and 1ca3eb8.

📒 Files selected for processing (2)

• rust/lance-encoding/src/encodings/logical/primitive.rs
• rust/lance-encoding/src/encodings/logical/primitive/sparse.rs

💤 Files with no reviewable changes (1)

• rust/lance-encoding/src/encodings/logical/primitive.rs

[codecov]

Codecov Report

❌ Patch coverage is 95.25692% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...encoding/src/encodings/logical/primitive/sparse.rs	94.23%	8 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

[Ali2Arslan]

Ran an evaluation on this. Summary: the memory win is large where the chunk index dominates, timings show four significant improvements and no significant regression, and there is one caveat worth knowing.

I've pushed the benchmarks I used as test(encoding): add sparse structural read-path benchmarks so the numbers below are reproducible:

CPUS=<physical core ids> rust/lance-encoding/benches/sparse/run_ab.sh d8030a364 539a59dac 8

d8030a364 is this branch's merge-base, so the comparison isolates the two commits here. Identical bench code runs on both arms.

Memory: resident scheduler state after DecodeBatchScheduler::try_new

This is LanceCache::size_bytes(), i.e. state retained once per page per column for as long as the dataset is open.

case	before	after	delta
uniform/many_chunks (3.2M values)	97.91 KiB	12.92 KiB	−86.8%
nested/list_of_list	49.09 KiB	6.82 KiB	−86.1%
wide/32_columns (32 pages)	398.99 KiB	71.74 KiB	−82.0%
sweep/256k_values	8.25 KiB	1.71 KiB	−79.2%
sweep/16k_values	767 B	795 B	+3.7%
degenerate/single_value	287 B	735 B	+156.1%
non_uniform/many_chunks	6.21 MiB	6.12 MiB	−1.5%
automatic/skewed_lists	63.23 KiB	62.00 KiB	−1.9%

Allocation count during initialize drops from 93 to 91 per page, which confirms the two-fewer-temporary-vectors claim exactly rather than by inference.

Timing: 8 interleaved rounds, arms rotated, pinned to physical cores

Paired per-round deltas with an exact two-sided sign-flip permutation test. Only significant results shown; everything else came back as noise or not significant, and nothing regressed significantly.

benchmark	delta	p
sparse_init/uniform_many_chunks	−20.1%	0.016
sparse_init/nested_list_of_list	−18.3%	0.016
sparse_init/wide_32_columns	−14.1%	0.031
sparse_take/wide_32_columns	−11.1%	0.016

Full scans are flat, as expected since the chunk index affects scheduling rather than the value-decode inner loop.

Caveat: a constant +448 bytes per sparse page

A single-chunk page goes from 287 B to 735 B. I checked whether that was fixed or scaling by measuring 64 single-value pages: the cost was exactly 64 × 448 B, so it is a flat per-page addition. The sweep puts break-even at roughly 16k leaf values per page, above which the saving climbs to ~87%. Good trade for normal pages; mildly negative only for tables of very many tiny pages.

[lance-gatekeeper]

⚠️ Gate recommendation: approve with a non-blocking risk.

The narrowed change reduces flat chunk-index metadata without changing the durable format or row/chunk semantics. The earlier sparse-path concerns no longer apply.

The remaining non-blocking risk is nested-page performance: nested metadata gains one allocation and indirection, with a small increase in modeled cache weight. Nested-heavy initialization and scan latency have not been measured; a targeted comparison would quantify that cost. No correctness change is requested.

[Xuanwo]

Current head keeps the existing Planar sparse path and limits the change to private cached index metadata. Applicable CI is green, and the remaining nested-page latency uncertainty is bounded and acceptable for this scope.