feat(vamana): implement khive-vamana ANN crate (ADR-048)#504
Conversation
…or pre-normalized f32 vectors Standalone leaf crate (no khive deps) implementing DiskANN-faithful Vamana: - Two-pass alpha rayon batch build (pass 1: alpha=1.0, pass 2: alpha=config.alpha) - Alpha-squared pruning per DiskANN paper (not prompt shorthand) - Generation-counter VisitedSet for O(1) clear between queries - 8-lane unrolled l2_squared hot path (SIMD-friendly, no sqrt) - Mmap-backed load with bytemuck zero-copy f32 cast - Binary persistence: metadata.bin + graph.bin (magic-prefixed) + vectors.bin - 46 unit+integration tests; recall@10 = 0.977 for both 1000×384 and 5000×384 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… improvements F1 [HIGH]: use checked_mul for num_vectors*dimensions in load() to prevent overflow on crafted metadata files causing downstream OOB access. F2 [HIGH]: change assert_eq! to debug_assert_eq! in l2_squared so dimension mismatches cannot panic-crash the MCP process in release builds; outer validation already gates normal paths. Test gated to cfg(debug_assertions). F3 [MEDIUM]: replace rejection-sampling in initial_random_adjacency with a partial Fisher-Yates shuffle over a candidate pool, eliminating O(n²) worst case when num_vectors is close to max_degree. F5 [MEDIUM]: add assert in with_dimensions(0) to fail eagerly at config construction time; add corresponding should_panic test. All 45 unit tests and 2 integration benchmarks pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Merge sequence
Merge #499 first — PRs #500, #502, #503 depend on it (GitHub auto-retargets on merge). |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RuntimeConfig::default() now includes additional_embedding_models with ParaphraseMultilingualMiniLmL12V2, which gets registered in the DB and causes note-creation tests to attempt model loading. In CI, the ONNX model files don't exist, causing 5 integration tests to fail with "model initialization failed: IO error: No such file or directory". Fix: explicitly set additional_embedding_models: vec![] in all test RuntimeConfig blocks that set embedding_model: None. Also includes cargo fmt and deno fmt fixes for runtime.rs and ADR docs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 649ccbce31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Ok(robust_prune_inner( | ||
| vectors, dimensions, node, all, alpha, max_degree, |
There was a problem hiding this comment.
When this public robust_prune API is called with a graph whose node/candidate IDs are not backed by the supplied vectors slice (for example VamanaGraph::new(5, 0) with fewer than 5 vector rows, or a candidate ID >= vectors.len()/dimensions), these unchecked IDs are passed into robust_prune_inner, which indexes row(...) and panics instead of returning VamanaError. Since the method already returns Result, callers can crash the process with invalid input rather than handling an error.
Useful? React with 👍 / 👎.
|
|
||
| let vecs = self.vectors()?; | ||
| let num_queries = queries.len() / self.dimensions; | ||
| let denom = k.min(self.num_vectors) as f64; |
There was a problem hiding this comment.
When callers ask recall_at_k with k == 0 and any non-empty query set, denom becomes 0.0; the exact and ANN result sets are empty, so the method computes 0.0 / 0.0 and returns Ok(NaN). That silently poisons benchmark/experiment metrics instead of reporting invalid input, so this should return an error (or define a non-NaN value) before the division.
Useful? React with 👍 / 👎.
1 participant
Summary
VamanaConfigTest plan
cargo clippy -p khive-vamana --all-targets -- -D warningscleancargo check --workspaceclean🤖 Generated with Claude Code