Skip to content

test(index): cover the IVF partition transformer and partition filter - #8765

Merged
Xuanwo merged 2 commits into
lance-format:mainfrom
jackylee-ch:test/ivf-partition-transform-coverage
Sep 2, 2026
Merged

test(index): cover the IVF partition transformer and partition filter#8765
Xuanwo merged 2 commits into
lance-format:mainfrom
jackylee-ch:test/ivf-partition-transform-coverage

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

ivf/transform.rs had no tests, unlike its siblings sq/transform.rs and
flat/transform.rs. Both types in it fail silently: a vector assigned to the
wrong partition is never searched in the right one, and a PartitionFilter that
keeps the wrong rows drops vectors from a sharded build without an error.

The subtle part is the guard at the top of PartitionTransformer::transform.
Partitions already present means skip — except when with_distance is set and
the distance column is missing, where it has to drop both and recompute, or it
returns a batch without the column it promised.

Adds 11 tests: nearest-centroid assignment, the loss metadata v3/shuffler.rs
reads back, opt-in distances, both skip and recompute branches, the two column
errors, and range filtering including the keep-nothing case.

Verified non-vacuous: zeroing the assignment, zeroing the loss, ignoring
with_distance, and making the filter keep everything fail 6 of the 11.

@github-actions github-actions Bot added A-index Vector index, linalg, tokenizer chore labels Aug 26, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 26, 2026
`ivf/transform.rs` had no tests, unlike its siblings `sq/transform.rs` and
`flat/transform.rs`. Both types in it fail silently: a vector assigned to the
wrong partition is never searched in the right one, and a `PartitionFilter` that
keeps the wrong rows drops vectors from a sharded build without an error.

The subtle part is the guard at the top of `PartitionTransformer::transform`.
Partitions already present means skip — except when `with_distance` is set and
the distance column is missing, where it has to drop both and recompute, or it
returns a batch without the column it promised.

Adds 11 tests: nearest-centroid assignment, the loss metadata `v3/shuffler.rs`
reads back, opt-in distances, both skip and recompute branches, the two column
errors, and range filtering including the keep-nothing case.

Verified non-vacuous: zeroing the assignment, zeroing the loss, ignoring
`with_distance`, and making the filter keep everything fail 6 of the 11.
@jackylee-ch
jackylee-ch force-pushed the test/ivf-partition-transform-coverage branch from 79d3c7f to d74bd76 Compare August 26, 2026 10:09
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 26, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 26, 2026
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 2, 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.

These focused tests exercise the IVF transform’s assignment, loss metadata, distance recomputation, error, and range-filtering contracts without changing production behavior.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 2, 2026
@Xuanwo
Xuanwo merged commit 4886d77 into lance-format:main Sep 2, 2026
38 checks passed
@jackylee-ch

Copy link
Copy Markdown
Contributor Author

Thanks @Xuanwo

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.

2 participants