Skip to content

Conversation

@BubbleCal
Copy link
Contributor

@BubbleCal BubbleCal commented Feb 2, 2026

this test searches for top1 which could lead to greedy search for HNSW, fix it by setting ef

Signed-off-by: BubbleCal <bubble-cal@outlook.com>
@github-actions github-actions bot added the bug Something isn't working label Feb 2, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2026

PR Review

Summary: This PR adds scan.ef(100) to fix a flaky test test_ann_prefilter for HNSW index.

Assessment

The fix is reasonable. The ef parameter controls the search exploration factor in HNSW - a higher value increases recall at the cost of search speed. For a test that's checking correctness of prefiltering behavior (not performance), setting ef(100) ensures the search is thorough enough to reliably find the correct result.

No Major Issues Found

This is a minimal, targeted fix that:

  • Only affects test code
  • Uses an established API (ef()) that's already defined in the scanner
  • Is consistent with the existing minimum_nprobes(100) already set for IVF indices

LGTM

Copy link
Collaborator

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thank you!

@codecov
Copy link

codecov bot commented Feb 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@BubbleCal BubbleCal merged commit da3e5dc into main Feb 2, 2026
32 checks passed
@BubbleCal BubbleCal deleted the yang/ann-ef100-test branch February 2, 2026 12:43
vivek-bharathan pushed a commit to vivek-bharathan/lance that referenced this pull request Feb 2, 2026
this test searches for top1 which could lead to greedy search for HNSW,
fix it by setting `ef`

Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants