fix: do no work when optimize_indices is called but no new data has arrived

[westonpace]

Summary

• Make optimize_indices() a no-op when no fragment is unindexed and the caller hasn't asked for retrain or num_indices_to_merge >= 1. Without this, a steady-state dataset (everything already indexed) had every call rewrite identical scalar-index files and trigger a vector single-segment auto-rebalance, producing a new manifest, transaction file, and per-index files on every call.
• Adds a Python regression test that builds one of every index type (BTREE, BITMAP, LABEL_LIST, INVERTED, NGRAM, ZONEMAP, BLOOMFILTER, IVF_PQ), appends a fragment, runs optimize_indices() twice, and asserts the second call writes zero new files under the dataset directory.

Why

Two independent paths were both writing files when there was nothing to do:

1. Scalar indices — merge_indices in rust/lance/src/index/append.rs always called index.update(...) even when unindexed.is_empty(), producing a new UUID directory with identical contents.
2. Vector indices — the single-segment auto-rebalance from feat: optimize one segmented vector segment per run #6402 fires when num_segments > 1 && num_indices_to_merge.is_none() && !retrain && unindexed.is_empty(). After a prior optimize_indices() had folded a delta into its own segment, every subsequent call re-rebalanced one segment.

The user-visible symptom was that calling optimize_indices() in a loop kept bumping the dataset version and creating index files indefinitely, even when the dataset hadn't changed.

Approach

The fix lives in Dataset::optimize_indices in rust/lance/src/index.rs. Before calling merge_indices for each grouped delta set, we now skip the group when:

• the caller did not pass retrain = true, and
• num_indices_to_merge is None or Some(0) (no explicit delta consolidation requested), and
• every fragment in the dataset is already covered by the union of the group's existing fragment_bitmaps.

The bitmap union is computed from the metadata we already loaded (no extra IO). Pre-0.8 indices without a fragment_bitmap are treated conservatively — they still go through the merge path so they pick up the new format on first optimize.

If any index group has work to do, the rest of the function is unchanged; the new check is purely a continue. When every group is skipped, the existing if new_indices.is_empty() { return Ok(()); } short-circuit prevents any commit, so no manifest or transaction file is written.

Test plan

• New Python test test_optimize_indices_second_call_is_noop passes
• python/tests/test_optimize.py, python/tests/test_scalar_index.py, python/tests/test_vector_index.py — 225 passed, 10 skipped (pre-existing), 1 deselected (unrelated pre-existing failure in test_create_index_progress_callback_error_before_completion_propagates)
• cargo test -p lance --lib index:: — 389 passed, 1 ignored (the auto-rebalance test noted above)
• cargo clippy --lib --tests -- -D warnings — clean
• CI green

Notes for reviewers

• The fragment-bitmap-only check intentionally skips unioning bitmaps from system indices (__lance_frag_reuse, __lance_mem_wal); those are filtered out before this loop by the existing is_system_index check.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[codecov]

Codecov Report

❌ Patch coverage is 95.65217% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
rust/lance/src/index.rs	85.00%	3 Missing ⚠️
rust/lance/src/index/append.rs	98.61%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[westonpace]

@claude review once

[claude]

🔴 The new vector no-op check at append.rs:197 uses options.num_indices_to_merge.is_none(), but the matching scalar no-op check in index.rs uses is_none_or(|n| n == 0). OptimizeOptions::append() is defined as num_indices_to_merge: Some(0) and is used by real callers (e.g. cleanup.rs and tests via optimize_indices(&OptimizeOptions::append())), so on a steady-state vector index a second append() call still falls through to optimize_vector_indices, writes a new UUID directory + manifest, and reproduces exactly the regression this PR is trying to fix. Suggest using is_none_or(|n| n == 0) on append.rs:197 (and the matching use_single_segment_rebalance gate two lines below) to align the two checks with the PR description ("None or Some(0)").

Extended reasoning...

What the bug is

The PR description states the no-op should fire when num_indices_to_merge is None or Some(0). The two checks the PR introduces don't agree on what "no merge requested" means:

• Scalar path (rust/lance/src/index.rs:1260): options.num_indices_to_merge.is_none_or(|n| n == 0) — correctly covers both None and Some(0).
• Vector path (rust/lance/src/index/append.rs:197): options.num_indices_to_merge.is_none() — only covers None, not Some(0).

The pre-existing use_single_segment_rebalance gate two lines below (append.rs:204) is also is_none(), so for Some(0) neither gate fires.

Why Some(0) is a real, reachable input

OptimizeOptions::append() in rust/lance-index/src/optimize.rs:78-84 is defined as:

pub fn append() -> Self {
    Self {
        num_indices_to_merge: Some(0),
        ...
    }
}

This is a public entry point used by real Rust callers — notably rust/lance/src/dataset/cleanup.rs (auto-cleanup) calls optimize_indices(&OptimizeOptions::append()), and the test suite (rust/lance/src/index/append.rs:569) does the same. The Python regression test added in this PR uses the default kwargs, which build OptimizeOptions::default() (None), so the test happens to take the path that does work and never exercises Some(0).

Step-by-step proof on a steady-state vector index

1. Create a dataset with an IVF_PQ index covering all fragments, no unindexed fragments.
2. Call dataset.optimize_indices(&OptimizeOptions::append()) — options.num_indices_to_merge == Some(0).
3. In Dataset::optimize_indices (index.rs), the new scalar short-circuit at line 1260 has index_group_is_scalar == false for the vector group, so the continue is not taken. Control reaches merge_indices(..).
4. merge_indices calls merge_indices_with_unindexed_frags with unindexed == [].
5. At append.rs:191-200, the new vector no-op check evaluates unindexed.is_empty() && !options.retrain && options.num_indices_to_merge.is_none() && select_segment_for_single_rebalance(&ivf_view)?.is_none(). The third conjunct (is_none()) is false for Some(0), so the no-op short-circuit does not return None.
6. At append.rs:202-206, use_single_segment_rebalance is gated on options.num_indices_to_merge.is_none() — also false for Some(0). So we fall into the else branch (line 271+).
7. new_data_stream is None (unindexed.is_empty()). optimize_vector_indices is called.
8. In rust/lance/src/index/vector/ivf.rs around line 415, let writer = object_store.create(&index_file).await? unconditionally creates a new UUID-named index file.
9. Back in Dataset::optimize_indices, this populates new_indices with a non-empty IndexMergeResults, so the if new_indices.is_empty() { return Ok(()) } short-circuit does not fire, and the function commits a new manifest + transaction.
10. Result: every call to optimize_indices(&OptimizeOptions::append()) on a steady-state vector index writes a new UUID directory and bumps the dataset version — the exact symptom the PR aims to fix.

Impact

OptimizeOptions::append() is documented as an entry point and is wired into auto-cleanup, so this isn't a hypothetical input. Any Rust caller (or anyone building OptimizeOptions { num_indices_to_merge: Some(0), .. } directly) on a vector index keeps re-emitting manifests and index files indefinitely. The PR's own description ("None or Some(0)") states the intended invariant — only half of the implementation enforces it.

Suggested fix

Change line 197 and line 204 of rust/lance/src/index/append.rs from:

&& options.num_indices_to_merge.is_none()

to:

&& options.num_indices_to_merge.is_none_or(|n| n == 0)

to match the scalar path and the documented intent. Strengthening the Python regression test to also call optimize_indices(num_indices_to_merge=0) would lock this in.

[wjones127]

I think the claude comment is valid, but not an important edge case.

Thanks for implementing this!

[andrea-reale]

Thanks for this fix!

One thing I noted: this "fast path" is gated on num_indices_to_merge being 0 (or None). This means that if as a caller passes Some(arbitarily_large_n) than this goes back to copying all indices even if the scalar index is already optimal with only 1 delta.

Am I reading this wrong? Is this intended behavior?

[westonpace]

Am I reading this wrong? Is this intended behavior?

I think you're right, we could do better here. I guess it shouldn't matter at all what the value of num_indices_to_merge is.