Skip to content

Bound nearest ANN probe budget - #591

Open
mikemikimike wants to merge 8 commits into
ModernRelay:mainfrom
mikemikimike:fix/ann-probe-budget
Open

Bound nearest ANN probe budget#591
mikemikimike wants to merge 8 commits into
ModernRelay:mainfrom
mikemikimike:fix/ann-probe-budget

Conversation

@mikemikimike

@mikemikimike mikemikimike commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Bound Lance IVF nearest-neighbor scans to 20 probes by default.
  • Added OMNIGRAPH_ANN_NPROBES; invalid and non-positive values fall back to 20.
  • Documented the deployment tuning knob.

This prevents Lance 11's unset maximum probe budget from probing every IVF partition while preserving nearest filtering and ordering behavior.

Validation

  • cargo fmt --all -- --check
  • cargo test -p omnigraph-engine --lib ann_probe_budget_tests --locked (3 passed)
  • cargo test -p omnigraph-engine --test search --locked (32 passed)
  • OMNIGRAPH_ANN_NPROBES=1 cargo test -p omnigraph-engine --test search nearest_returns_k_closest --locked (1 passed)
  • cargo test -p omnigraph-engine --test traversal --locked (28 passed)
  • Lance guards: 31 passed, 2 pre-existing Windows/local-manifest failures
  • Engine lib tests: 371 passed, 2 pre-existing manifest/Windows failures, 1 ignored

The canonical workspace feature-superset test and Clippy remain blocked by pre-existing errors outside this change in omnigraph-bench, omnigraph-azure-admission, and omnigraph-engine/tests/end_to_end.rs.

Closes #567

Greptile Summary

The PR bounds Lance IVF nearest-neighbor scans to 20 probes by default while allowing deployment-level tuning and an uncapped completeness retry for under-filled filtered searches.

  • Adds bounded ANN probe configuration through OMNIGRAPH_ANN_NPROBES.
  • Retries filtered nearest queries when the bounded pass returns fewer rows than requested.
  • Adds instrumentation and regression coverage for bounded and uncapped scan plans.
  • Documents the behavior in the user guide and v0.10.0 release notes.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
crates/omnigraph/src/exec/query.rs Adds ANN probe budgeting, filtered-query under-fill detection, and an uncapped completeness retry.
crates/omnigraph/src/table_store.rs Exposes Lance scanner controls for minimum and maximum IVF probe counts.
crates/omnigraph/src/instrumentation.rs Adds test-scoped observability for ANN budgets and uncapped retries.
crates/omnigraph/tests/search.rs Exercises the optimized multi-partition IVF under-fill and retry path end to end.
crates/omnigraph/tests/lance_surface_guards.rs Guards the expected Lance plans for bounded scans and minimum-only retries.
docs/releases/v0.10.0.md Resolves the prior review finding by documenting the new default, tuning variable, and retry behavior.
docs/user/search/index.md Documents ANN probe tuning and filtered-search completeness behavior for users.

Reviews (7): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile

Comment thread docs/user/search/index.md Outdated
@ragnorc

ragnorc commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for jumping on #567. One correctness interaction needs addressing before merge

Scanner::nprobes() sets both minimum_nprobes and maximum_nprobes. Lance's probing is lazy - the expensive probe-every-partition behavior is the late search, which only runs when the first probes yield fewer than k results (typically because a prefilter starved them). So the capped max buys performance precisely in the cases where it also drops results: a filtered nearest on a multi-partition index can now silently return fewer rows than limit.

And multi-partition is reachable in stock OmniGraph: we build ivf_flat(1, …), but optimize splits that partition - our own guard (lance_surface_guards.rs:2244) asserts partition_count > 1 after delete+optimize at only 20K rows, and then deliberately sets nprobes(partition_count) to stay exact. At #567's 852k scale, >20 partitions is plausible, and any OMNIGRAPH_ANN_NPROBES below the partition count has the same effect. That would contradict the "limit 10 means the top ten matches that satisfy the filters" promise in the same doc page this PR edits.

Suggestions (any one of these makes it sound):

  1. Set only minimum_nprobes(20) and leave the max unset — recall parity for the common case, completeness preserved; or
  2. Keep the bounded budget and add an unfilled-limit retry, mirroring the bm25 pattern fix(query): push the limit clause down into bm25 full-text scans #574 just established; or
  3. Explicitly re-contract nearest as bounded-recall — but then the owning docs need to say so.

Also worth adding: a behavioral test on an optimize-split index (the guards file already builds one at :2194), a lance_surface_guards pin for the min/max/late-search semantics we now depend on, and — since this knob can affect results — preferring the config/cluster-level surface the issue reporter offered over a process-global env var, with the effective budget recorded in instrumentation.

@mikemikimike

Copy link
Copy Markdown
Contributor Author

Thanks for catching this. Fixed in e5e1a60 (mikemikimike@e5e1a60).

The ANN scanner now sets Lance minimum_nprobes and maximum_nprobes explicitly. A filtered nearest query keeps the bounded first pass, then retries once with the same minimum and maximum_nprobes unset when the result under-fills the requested limit. This preserves the existing latency/recall knob for normal queries while allowing late search to reach all partitions when prefiltering starves the capped pass. The effective min/max budget and retry are recorded by the query instrumentation seam.

Added the optimized multi-partition IVF behavioral regression, Lance plan guards for bounded versus minimum-only probing, and search documentation/release notes. Local validation after merging upstream/main: cargo +1.97.0 test -p omnigraph-engine --test search --locked (40 passed); cargo +1.97.0 check -p omnigraph-engine --locked (passed); cargo +1.97.0 clippy -p omnigraph-engine --lib --locked -- -D warnings (passed); cargo +1.97.0 fmt --all -- --check (passed). The full surface guard suite remains 27/33 because six pre-existing Windows/Lance local _refs/branches permission or e_tag guards fail in this environment.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

performance: nearest() sets no ANN probe budget — Lance's unbounded default probes every IVF partition (289s vs 7.5s at 852k × 3072-dim)

2 participants