Skip to content

test(index): size the block-max-score fixture so its assertion can fail - #9249

Open
jackylee-ch wants to merge 1 commit into
lance-format:mainfrom
jackylee-ch:test/block-max-capacity-fixture
Open

jackylee-ch wants to merge 1 commit into
lance-format:mainfrom
jackylee-ch:test/block-max-capacity-fixture

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

test_block_max_scores_capacity_matches_block_count guards the Vec::with_capacity(num_blocks) in calculate_block_max_scores_with_block_size, but its fixture produced four blocks and Vec<f32> allocates a minimum of four elements. The capacity assertion therefore held for any hint of four or fewer, including none at all: dropping the hint entirely left the test green. It still caught the original over-allocation the assertion was added for, since that reserved one slot per document rather than per block.

Grow the fixture past that minimum and assert the block count exceeds it, so a later shrink cannot quietly restore the coincidence. Also parametrize over both entries of VALID_BLOCK_SIZES: the block count is derived from the block_size argument, and only the 128 path was reached through the wrapper.

Testing

cargo test --release -p lance-index --all-features for the module (5 passed), plus cargo fmt and clippy with -D warnings. With the capacity hint removed the fixture now allocates 8 against 6 expected blocks and both cases fail.

`test_block_max_scores_capacity_matches_block_count` guards the
`Vec::with_capacity(num_blocks)` in `calculate_block_max_scores_with_block_size`,
but its fixture produced four blocks and `Vec<f32>` allocates a minimum of four
elements. The capacity assertion therefore held for any hint of four or fewer,
including none at all: dropping the hint entirely left the test green. It still
caught the original over-allocation the assertion was added for, since that
reserved one slot per document rather than per block.

Grow the fixture past that minimum and assert the block count exceeds it, so a
later shrink cannot quietly restore the coincidence. Also parametrize over both
entries of `VALID_BLOCK_SIZES`: the block count is derived from the `block_size`
argument, and only the 128 path was reached through the wrapper.

## Testing

`cargo test --release -p lance-index --all-features` for the module (5 passed),
plus `cargo fmt` and clippy with `-D warnings`. With the capacity hint removed the
fixture now allocates 8 against 6 expected blocks and both cases fail.
@github-actions github-actions Bot added chore A-index Vector index, linalg, tokenizer labels Sep 15, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate recommendation: approve.

The larger fixture makes the capacity regression observable, and parametrizing the block-size-dependent path covers both supported sizes. This is a focused guard for the indexing-memory optimization established in #5718.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-index Vector index, linalg, tokenizer chore K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant