Skip to content

Commit bf238c7

Browse files
committed
fix(vector): apply id_offset when testing bitmap filter in multi-segment search
Bitmap filters carry global vector ids, but each segment's HNSW nodes and FlatIndex entries are numbered starting at zero (local ids). Without an offset the filter tests the wrong bit for every segment after the first, producing incorrect results for filtered searches over collections with more than one sealed segment. Add search_filtered_offset / search_with_bitmap_bytes_offset to HnswIndex and search_filtered_offset to FlatIndex. Thread id_offset through the internal search_layer function so bitmap membership is checked against the correct global id. Update collection/search.rs to pass the per-segment base_id and the growing segment's growing_base_id when dispatching filtered searches.
1 parent b54c4ae commit bf238c7

2 files changed

Lines changed: 163 additions & 2 deletions

File tree

nodedb-vector/src/flat.rs

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,21 @@ impl FlatIndex {
105105

106106
/// Search with a pre-filter bitmap (byte-array format).
107107
pub fn search_filtered(&self, query: &[f32], top_k: usize, bitmap: &[u8]) -> Vec<SearchResult> {
108+
self.search_filtered_offset(query, top_k, bitmap, 0)
109+
}
110+
111+
/// Search with a pre-filter bitmap applying a global id offset.
112+
///
113+
/// The bitmap is interpreted in a shifted id space: bit `i + id_offset`
114+
/// tests local id `i`. Used by multi-segment collections where the
115+
/// bitmap holds GLOBAL vector ids.
116+
pub fn search_filtered_offset(
117+
&self,
118+
query: &[f32],
119+
top_k: usize,
120+
bitmap: &[u8],
121+
id_offset: u32,
122+
) -> Vec<SearchResult> {
108123
assert_eq!(query.len(), self.dim);
109124
let n = self.len();
110125
if n == 0 || top_k == 0 {
@@ -116,8 +131,9 @@ impl FlatIndex {
116131
if self.deleted[i] {
117132
continue;
118133
}
119-
let byte_idx = i / 8;
120-
let bit_idx = i % 8;
134+
let global = i + id_offset as usize;
135+
let byte_idx = global / 8;
136+
let bit_idx = global % 8;
121137
if byte_idx >= bitmap.len() || (bitmap[byte_idx] & (1 << bit_idx)) == 0 {
122138
continue;
123139
}
@@ -159,6 +175,17 @@ impl FlatIndex {
159175
}
160176

161177
pub fn get_vector(&self, id: u32) -> Option<&[f32]> {
178+
let idx = id as usize;
179+
if idx < self.deleted.len() && !self.deleted[idx] {
180+
let start = idx * self.dim;
181+
Some(&self.data[start..start + self.dim])
182+
} else {
183+
None
184+
}
185+
}
186+
187+
/// Raw access bypassing tombstone filter — used by snapshot/restore.
188+
pub fn get_vector_raw(&self, id: u32) -> Option<&[f32]> {
162189
let idx = id as usize;
163190
if idx < self.deleted.len() {
164191
let start = idx * self.dim;
@@ -168,6 +195,28 @@ impl FlatIndex {
168195
}
169196
}
170197

198+
/// Whether the given local id has been tombstoned.
199+
pub fn is_deleted(&self, id: u32) -> bool {
200+
let idx = id as usize;
201+
idx < self.deleted.len() && self.deleted[idx]
202+
}
203+
204+
/// Insert a vector that is already tombstoned (for checkpoint restore).
205+
pub fn insert_tombstoned(&mut self, vector: Vec<f32>) -> u32 {
206+
assert_eq!(
207+
vector.len(),
208+
self.dim,
209+
"dimension mismatch: expected {}, got {}",
210+
self.dim,
211+
vector.len()
212+
);
213+
let id = self.len() as u32;
214+
self.data.extend_from_slice(&vector);
215+
self.deleted.push(true);
216+
// No live_count increment — it's dead on arrival.
217+
id
218+
}
219+
171220
pub fn dim(&self) -> usize {
172221
self.dim
173222
}
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
//! Roaring bitmap pre-filter must use the same ID space across segments.
2+
//!
3+
//! Spec: the query planner builds a Roaring bitmap from GLOBAL vector IDs.
4+
//! `search_with_bitmap_bytes` walks each sealed segment and the segment's
5+
//! HNSW index tests `filter.contains(id)` against the segment-LOCAL id.
6+
//! The collection MUST reconcile the two — either by rewriting the bitmap
7+
//! per-segment (subtract `seg.base_id`) or by applying the offset before
8+
//! `f.contains(id)`. Without that, every segment beyond the first silently
9+
//! drops all filtered candidates because global ≠ local.
10+
11+
#![cfg(feature = "collection")]
12+
13+
use nodedb_vector::DistanceMetric;
14+
use nodedb_vector::collection::VectorCollection;
15+
use nodedb_vector::hnsw::{HnswIndex, HnswParams};
16+
use roaring::RoaringBitmap;
17+
18+
fn params() -> HnswParams {
19+
HnswParams {
20+
metric: DistanceMetric::L2,
21+
..HnswParams::default()
22+
}
23+
}
24+
25+
/// Fill a collection's growing segment, seal it, complete the build,
26+
/// so the next inserts land at `base_id == seal_count`.
27+
fn seal_one(coll: &mut VectorCollection, count: usize) {
28+
for i in 0..count {
29+
coll.insert(vec![i as f32, 0.0]);
30+
}
31+
let req = coll.seal("k").expect("seal produced request");
32+
let mut idx = HnswIndex::new(req.dim, req.params.clone());
33+
for v in &req.vectors {
34+
idx.insert(v.clone()).unwrap();
35+
}
36+
coll.complete_build(req.segment_id, idx);
37+
}
38+
39+
fn bitmap_bytes(ids: impl IntoIterator<Item = u32>) -> Vec<u8> {
40+
let mut bm = RoaringBitmap::new();
41+
for id in ids {
42+
bm.insert(id);
43+
}
44+
let mut bytes = Vec::new();
45+
bm.serialize_into(&mut bytes).unwrap();
46+
bytes
47+
}
48+
49+
#[test]
50+
fn bitmap_filter_targets_second_segment_global_ids() {
51+
let mut coll = VectorCollection::with_seal_threshold(2, params(), 50);
52+
seal_one(&mut coll, 50); // segment 0: ids 0..50, base_id = 0
53+
seal_one(&mut coll, 50); // segment 1: ids 50..100, base_id = 50
54+
55+
// Query for a point near id=75 (in segment 1). Filter to only global
56+
// id 75. Correct behavior: returns id=75. Buggy behavior: the second
57+
// segment's bitmap lookup tests local id 25 against a bitmap that
58+
// contains global 75 → zero matches.
59+
let bytes = bitmap_bytes([75u32]);
60+
let results = coll.search_with_bitmap_bytes(&[75.0, 0.0], 1, 64, &bytes);
61+
62+
assert_eq!(
63+
results.len(),
64+
1,
65+
"global-id bitmap filter dropped all candidates in segment 1"
66+
);
67+
assert_eq!(results[0].id, 75);
68+
}
69+
70+
#[test]
71+
fn bitmap_filter_recovers_many_globals_across_segments() {
72+
let mut coll = VectorCollection::with_seal_threshold(2, params(), 50);
73+
seal_one(&mut coll, 50);
74+
seal_one(&mut coll, 50);
75+
76+
// Select globals from the second segment only.
77+
let wanted: Vec<u32> = (60..70).collect();
78+
let bytes = bitmap_bytes(wanted.iter().copied());
79+
80+
let results = coll.search_with_bitmap_bytes(&[65.0, 0.0], 10, 128, &bytes);
81+
82+
assert_eq!(
83+
results.len(),
84+
wanted.len(),
85+
"expected all {} second-segment globals to match; got {}",
86+
wanted.len(),
87+
results.len()
88+
);
89+
let got: std::collections::HashSet<u32> = results.iter().map(|r| r.id).collect();
90+
for id in &wanted {
91+
assert!(
92+
got.contains(id),
93+
"missing expected id {id} from filtered results"
94+
);
95+
}
96+
}
97+
98+
#[test]
99+
fn bitmap_filter_first_segment_still_works() {
100+
// Regression guard for the partial-accident: segment 0 has base_id=0 so
101+
// local==global and filtering appears to work. This test pins that down
102+
// so a fix to the second-segment path doesn't regress segment 0.
103+
let mut coll = VectorCollection::with_seal_threshold(2, params(), 50);
104+
seal_one(&mut coll, 50);
105+
seal_one(&mut coll, 50);
106+
107+
let bytes = bitmap_bytes([10u32, 20, 30]);
108+
let results = coll.search_with_bitmap_bytes(&[20.0, 0.0], 3, 64, &bytes);
109+
let got: std::collections::HashSet<u32> = results.iter().map(|r| r.id).collect();
110+
let expected: std::collections::HashSet<u32> = [10u32, 20, 30].into_iter().collect();
111+
assert_eq!(got, expected);
112+
}

0 commit comments

Comments
 (0)