From bc1875bcfa3daa375eab318819ae2503a210666e Mon Sep 17 00:00:00 2001 From: rUv Date: Sun, 28 Jun 2026 14:23:44 -0400 Subject: [PATCH] =?UTF-8?q?feat(rvf):=20queryable=20COW=20branches=20?= =?UTF-8?q?=E2=80=94=20wire=20CowEngine=20read-through=20to=20query=20path?= =?UTF-8?q?=20(#617)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(rvf): queryable COW branches — wire CowEngine read-through to query path RVF's COW branch was created but not queryable: a derived/branched child returned only its own edits, never the inherited parent vectors. This wires parent read-through into the exact query path and exposes the real COW branch() through the node binding. Root cause (confirmed against source): - The node binding's derive() (rvf-node/src/lib.rs) called store::derive() which builds a child with cow_engine: None — a lineage/provenance delta, not a COW union. - The real COW path, branch() (store.rs), wires CowEngine::from_parent + a MembershipFilter, but was never exposed in the node binding. - query_exact()/read_path scanned only self.vectors (local edits). The cow_engine/membership_filter were consulted ONLY to *disable* the HNSW / RaBitQ fast paths (index_eligible/rabitq_eligible), never to merge parent data. CowEngine::read_vector/read_cluster were never called from the query path. - query_routed() additionally short-circuited to an empty result whenever self.vectors.len() == 0, so an unedited COW child returned nothing. - The existing cow_branching test asserted only MembershipFilter membership; it never ran a query, so the gap went unnoticed. Fix (exact-scan read-through slice): - store.rs: query_exact() now performs a COW read-through. For a COW child it lazily opens a read-only handle to the parent (cached in a new parent_store field), then merges every inherited parent vector that the child has not overridden (re-ingested locally) or deleted into the same bounded-heap scan — i.e. parent ∪ child-edits with the child winning on an id collision. Factored the heap admission into heap_consider(). - store.rs: query_routed() no longer short-circuits empty for COW children. - store.rs: branch() now sizes the MembershipFilter by max-id+1 (not the vector count) so sparse / non-contiguous ids are representable. - rvf-node/src/lib.rs: expose branch() (COW-enabled) alongside derive(). Scope: this is the EXACT (flat) read-through slice. The byte-level CowEngine::read_cluster path addresses raw cluster offsets, which do not correspond to RVF's segmented on-disk layout, so it cannot be wired to a real .rvf parent as-is. ANN-index (HNSW/RaBitQ) read-through across the COW boundary remains a follow-up; those paths already fall back to the exact scan for COW children, so correctness holds. Test: new branch_query_reads_through_to_parent — builds a 1k-vector base, branches a COW child, applies edits, and asserts (1) a query for a base vector returns it via read-through, (2) a query for an edited vector returns the child's override, (3) a newly added child vector is queryable, and (4) the branch file stays far smaller than the base (COW delta, not a full copy). All 9 cow_branching tests and the full rvf-runtime + rvf integration suites pass. Discovered via RVF-COW benchmarking in the agent-harness-generator (MetaHarness) project, which proved the branch was a lineage-only delta and pinpointed the unexposed branch() + unwired read path. Co-Authored-By: claude-flow Claude-Session: https://claude.ai/code/session_019rVRYrRDKyxYK18kuVrDSf * fix(ci): sync Cargo.lock to ruvector-sona 0.2.1 for lockfile integrity Regenerate Cargo.lock (offline, no external version bumps) so the local ruvector-sona workspace member resolves at 0.2.1 — fixes the `cargo metadata --locked` lockfile-integrity check. No source/dep changes in this PR; drift was inherited from the base branch. Co-Authored-By: claude-flow Claude-Session: https://claude.ai/code/session_019rVRYrRDKyxYK18kuVrDSf --------- Co-authored-by: ruvnet --- crates/rvf/rvf-node/src/lib.rs | 30 ++++ crates/rvf/rvf-runtime/src/store.rs | 156 ++++++++++++++++-- .../rvf-integration/tests/cow_branching.rs | 119 ++++++++++++- 3 files changed, 290 insertions(+), 15 deletions(-) diff --git a/crates/rvf/rvf-node/src/lib.rs b/crates/rvf/rvf-node/src/lib.rs index 9adf698b0..923bc91f4 100644 --- a/crates/rvf/rvf-node/src/lib.rs +++ b/crates/rvf/rvf-node/src/lib.rs @@ -764,6 +764,36 @@ impl RvfDatabase { }) } + /// Create a **queryable** copy-on-write branch of this store. + /// + /// Unlike [`derive`](Self::derive) — which records lineage only and + /// produces a child that can see just its own edits — `branch` wires a + /// real COW engine and membership filter so the child inherits the + /// parent's vectors. Querying the child returns `parent ∪ child-edits`: + /// the child's re-ingested vectors override the parent on an id + /// collision, and deletes in the child hide inherited vectors. The + /// branch stores only its own edits on disk, so it stays small + /// regardless of the parent's size. + /// + /// The parent should be frozen (see [`freeze`](Self::freeze)) before + /// branching to guarantee immutability of the inherited data. + #[napi] + pub fn branch(&self, child_path: String) -> Result { + let guard = self + .inner + .lock() + .map_err(|_| napi::Error::from_reason("Lock poisoned"))?; + let store = guard + .as_ref() + .ok_or_else(|| napi::Error::from_reason("Store is closed"))?; + + let child_store = store.branch(Path::new(&child_path)).map_err(map_rvf_err)?; + + Ok(RvfDatabase { + inner: Mutex::new(Some(child_store)), + }) + } + // ── Kernel / eBPF methods ──────────────────────────────────────── /// Embed a kernel image into this RVF file. diff --git a/crates/rvf/rvf-runtime/src/store.rs b/crates/rvf/rvf-runtime/src/store.rs index 41351a26a..6e0c240c4 100644 --- a/crates/rvf/rvf-runtime/src/store.rs +++ b/crates/rvf/rvf-runtime/src/store.rs @@ -89,6 +89,12 @@ pub struct RvfStore { /// Same single-builder gate as `index_building`, for the RaBitQ code /// book (its lazy build is an O(N) scan + encode). rabitq_building: AtomicBool, + /// Lazily-opened, read-only handle to the parent store, used for COW + /// read-through queries (parent ∪ child-edits). `None` for root stores + /// and until the first query on a COW child resolves it from + /// [`Self::parent_path`]. Boxed to break the recursive type and guarded + /// by a `Mutex` so `query(&self)` can populate it lazily. + parent_store: Mutex>>, } /// Clears an `AtomicBool` on drop, so a panicking index build can never @@ -153,6 +159,7 @@ impl RvfStore { index_building: AtomicBool::new(false), rabitq: Mutex::new(None), rabitq_building: AtomicBool::new(false), + parent_store: Mutex::new(None), }; store.write_manifest()?; @@ -207,6 +214,7 @@ impl RvfStore { index_building: AtomicBool::new(false), rabitq: Mutex::new(None), rabitq_building: AtomicBool::new(false), + parent_store: Mutex::new(None), }; store.boot()?; @@ -257,6 +265,7 @@ impl RvfStore { index_building: AtomicBool::new(false), rabitq: Mutex::new(None), rabitq_building: AtomicBool::new(false), + parent_store: Mutex::new(None), }; store.boot()?; @@ -434,7 +443,11 @@ impl RvfStore { return Err(err(ErrorCode::DimensionMismatch)); } - if self.vectors.len() == 0 { + // A COW child can have zero *local* vectors yet still resolve + // results from its parent via read-through, so only short-circuit + // when there is genuinely nothing to scan (no local data AND not a + // COW child). + if self.vectors.len() == 0 && self.cow_engine.is_none() { return Ok((Vec::new(), false)); } @@ -654,7 +667,10 @@ impl RvfStore { }; // Scan the contiguous slab in ordinal order (cache-friendly: rows - // are adjacent in memory, no per-vector pointer chase). + // are adjacent in memory, no per-vector pointer chase). For a COW + // child this slab holds only the child's own edits/overrides; the + // inherited parent vectors are merged in by the read-through scan + // below. for (vec_id, stored_vec) in self.vectors.iter() { if self.deletion_bitmap.is_deleted(vec_id) { continue; @@ -665,18 +681,16 @@ impl RvfStore { } } let dist = compute_distance(vector, stored_vec, &self.options.metric, query_norm_sq); - if heap.len() < k { - heap.push((OrderedFloat(dist), vec_id)); - } else if let Some(&(OrderedFloat(worst), worst_id)) = heap.peek() { - // Tie-break equal distances by smaller id so the selected - // k-set is independent of storage iteration order. - if dist < worst || (dist == worst && vec_id < worst_id) { - heap.pop(); - heap.push((OrderedFloat(dist), vec_id)); - } - } + heap_consider(&mut heap, k, dist, vec_id); } + // COW read-through: a branch created via [`Self::branch`] inherits + // its parent's vectors. Merge in every inherited parent vector that + // the child has neither overridden (re-ingested locally) nor + // deleted, so a query for a *base* vector returns it (parent ∪ + // child-edits, child wins on id collision). + self.cow_scan_parent(vector, query_norm_sq, k, options, &mut heap); + // Drain the max-heap into sorted results (closest first). let mut results: Vec = heap .into_iter() @@ -695,6 +709,94 @@ impl RvfStore { results } + /// COW read-through scan: merge the parent's inherited vectors into the + /// exact-scan result heap. + /// + /// A store created via [`Self::branch`] holds only its own edits in + /// `self.vectors` while the parent's vectors are inherited through the + /// [`MembershipFilter`]. Without this scan a query on the child would + /// only ever see the child's edits (the bug this fixes). Here we open + /// the parent read-only (lazily, cached for subsequent queries) and add + /// each inherited vector that the child has **not** overridden or + /// deleted, giving the git-like `parent ∪ child-edits` semantics where + /// the child wins on an id collision. + /// + /// This is the EXACT-scan (flat) read-through slice. Read-through across + /// the COW boundary for the HNSW / RaBitQ index paths is deliberately + /// out of scope (those paths are already disabled for COW children by + /// [`Self::index_eligible`] / [`Self::rabitq_eligible`], so queries fall + /// back here) and is tracked as a follow-up. + /// + /// No-op for root stores (no `cow_engine` / `membership_filter` / + /// `parent_path`). If the parent file cannot be opened the child simply + /// returns its own results rather than failing the query. + fn cow_scan_parent( + &self, + vector: &[f32], + query_norm_sq: f32, + k: usize, + options: &QueryOptions, + heap: &mut BinaryHeap<(OrderedFloat, u64)>, + ) { + // Only COW children (created by `branch`) read through to a parent. + if self.cow_engine.is_none() { + return; + } + let Some(filter) = self.membership_filter.as_ref() else { + return; + }; + let Some(parent_path) = self.parent_path.as_ref() else { + return; + }; + + // Lazily open (and cache) a read-only handle to the parent. A + // read-only open takes no writer lock, so it never contends with a + // parent still open for writing in the same process. + let mut guard = self.parent_store.lock().unwrap_or_else(|e| e.into_inner()); + if guard.is_none() { + match RvfStore::open_readonly(parent_path) { + Ok(parent) => *guard = Some(Box::new(parent)), + // Parent unavailable: fall back to child-only results. + Err(_) => return, + } + } + let parent = match guard.as_ref() { + Some(p) => p, + None => return, + }; + + // Dimension guard: a mismatched parent would corrupt distances. + if parent.options.dimension != self.options.dimension { + return; + } + + for (vec_id, stored_vec) in parent.vectors.iter() { + // Only vectors inherited at branch time are visible. + if !filter.contains(vec_id) { + continue; + } + // Child override wins: skip parent copy if re-ingested locally. + if self.vectors.get(vec_id).is_some() { + continue; + } + // Deleted in the child (tombstone) hides the inherited vector. + if self.deletion_bitmap.is_deleted(vec_id) { + continue; + } + // Defensive: also respect the parent's own tombstones. + if parent.deletion_bitmap.is_deleted(vec_id) { + continue; + } + if let Some(ref filter_expr) = options.filter { + if !filter::evaluate(filter_expr, vec_id, &parent.metadata) { + continue; + } + } + let dist = compute_distance(vector, stored_vec, &self.options.metric, query_norm_sq); + heap_consider(heap, k, dist, vec_id); + } + } + /// Query the store and return a full QualityEnvelope (ADR-033 §2.4). /// /// This is the preferred query API. The QualityEnvelope is the mandatory @@ -1945,8 +2047,14 @@ impl RvfStore { bytes_per_vec, )); - // Initialize membership filter with all parent vectors visible - let mut filter = MembershipFilter::new_include(total_vecs); + // Initialize membership filter with all parent vectors visible. + // Size the bitmap by max id + 1 (not the count) so sparse / non- + // contiguous ids are representable — `MembershipFilter::add` drops + // any id >= capacity, which would silently make those parent + // vectors invisible to read-through. + let max_id = self.vectors.ids().copied().max().unwrap_or(0); + let filter_capacity = max_id.saturating_add(1).max(total_vecs); + let mut filter = MembershipFilter::new_include(filter_capacity); for &vid in self.vectors.ids() { if !self.deletion_bitmap.is_deleted(vid) { filter.add(vid); @@ -2069,6 +2177,7 @@ impl RvfStore { index_building: AtomicBool::new(false), rabitq: Mutex::new(None), rabitq_building: AtomicBool::new(false), + parent_store: Mutex::new(None), }; store.write_manifest()?; @@ -2360,6 +2469,25 @@ fn compute_distance(a: &[f32], b: &[f32], metric: &DistanceMetric, a_norm_sq: f3 } } +/// Offer a `(distance, id)` candidate to a bounded max-heap of size `k`. +/// +/// Keeps the `k` closest candidates seen so far. On a full heap a candidate +/// is admitted when it is strictly closer than the current farthest, or ties +/// the farthest distance with a smaller id (deterministic tie-break, so the +/// selected k-set is independent of scan order across the local and +/// COW-parent passes). +#[inline] +fn heap_consider(heap: &mut BinaryHeap<(OrderedFloat, u64)>, k: usize, dist: f32, vec_id: u64) { + if heap.len() < k { + heap.push((OrderedFloat(dist), vec_id)); + } else if let Some(&(OrderedFloat(worst), worst_id)) = heap.peek() { + if dist < worst || (dist == worst && vec_id < worst_id) { + heap.pop(); + heap.push((OrderedFloat(dist), vec_id)); + } + } +} + #[derive(Clone, Copy, Debug, PartialEq)] struct OrderedFloat(f32); diff --git a/crates/rvf/tests/rvf-integration/tests/cow_branching.rs b/crates/rvf/tests/rvf-integration/tests/cow_branching.rs index 796b917d5..37d82abcb 100644 --- a/crates/rvf/tests/rvf-integration/tests/cow_branching.rs +++ b/crates/rvf/tests/rvf-integration/tests/cow_branching.rs @@ -3,7 +3,7 @@ //! Tests the core branching flow: creating a base store, deriving a child, //! verifying COW statistics, write coalescing, and parent immutability. -use rvf_runtime::options::{DistanceMetric, RvfOptions}; +use rvf_runtime::options::{DistanceMetric, QueryOptions, RvfOptions}; use rvf_runtime::RvfStore; use tempfile::TempDir; @@ -382,3 +382,120 @@ fn branch_membership_filter_excludes_deleted() { println!("PASS: branch_membership_filter_excludes_deleted"); } + +// =========================================================================== +// TEST 9: branch_query_reads_through_to_parent (the COW-queryability fix) +// =========================================================================== + +/// The core regression for the "COW branch is not queryable" bug. +/// +/// Builds a 1k-vector base, branches a COW child, applies a few edits, then +/// asserts: +/// 1. a query for a *base* vector returns it (parent read-through works, +/// even from a child with zero local edits); +/// 2. a query for an *edited* vector returns the child's value (override +/// wins over the inherited parent vector on an id collision); +/// 3. a brand-new vector added to the child is queryable; +/// 4. the branch file stays small — a COW delta, not a full copy. +#[test] +fn branch_query_reads_through_to_parent() { + let dir = TempDir::new().unwrap(); + let base_path = dir.path().join("base_rt.rvf"); + let child_path = dir.path().join("child_rt.rvf"); + let dim: u16 = 16; + let n: u64 = 1000; + + // Build a 1k-vector base with contiguous ids 0..n. + let mut base = RvfStore::create(&base_path, make_options(dim)).unwrap(); + let vectors: Vec> = (0..n).map(|i| random_vector(dim as usize, i)).collect(); + let refs: Vec<&[f32]> = vectors.iter().map(|v| v.as_slice()).collect(); + let ids: Vec = (0..n).collect(); + base.ingest_batch(&refs, &ids, None).unwrap(); + base.freeze().unwrap(); // immutable parent (correct COW usage) + + // Branch a COW child. + let mut child = base.branch(&child_path).unwrap(); + let opts = QueryOptions::default(); + + // (1) Empty-child read-through: query for a known BASE vector. Before + // the fix this returned nothing (only the child's own edits were + // searchable, and the child has none yet). + let base_id: u64 = 123; + let hits = child.query(&vectors[base_id as usize], 5, &opts).unwrap(); + assert!( + !hits.is_empty(), + "COW child must return parent results via read-through" + ); + assert_eq!( + hits[0].id, base_id, + "read-through must return the matching base vector as nearest" + ); + assert!( + hits[0].distance < 1e-3, + "exact base vector should match at ~0 distance, got {}", + hits[0].distance + ); + + // (2) Override: re-ingest an existing base id in the child with a new + // value. A query for the NEW value must return that id (child wins). + let override_id: u64 = 200; + let new_val = random_vector(dim as usize, 999_999); + child + .ingest_batch(&[new_val.as_slice()], &[override_id], None) + .unwrap(); + + let hits = child.query(&new_val, 5, &opts).unwrap(); + assert_eq!( + hits[0].id, override_id, + "child override must win over the inherited parent vector" + ); + assert!( + hits[0].distance < 1e-3, + "override exact match should be ~0, got {}", + hits[0].distance + ); + + // The OLD parent value of override_id is no longer present, so if that + // id appears at all it must not match its old value at ~0 distance. + let old_val = &vectors[override_id as usize]; + let hits = child.query(old_val, 5, &opts).unwrap(); + if let Some(h) = hits.iter().find(|h| h.id == override_id) { + assert!( + h.distance > 1e-3, + "overridden id must not still match its OLD value at ~0" + ); + } + + // (3) A brand-new vector added to the child is queryable. + let new_id: u64 = 5000; + let added = random_vector(dim as usize, 424_242); + child + .ingest_batch(&[added.as_slice()], &[new_id], None) + .unwrap(); + let hits = child.query(&added, 5, &opts).unwrap(); + assert_eq!( + hits[0].id, new_id, + "newly added child vector must be queryable" + ); + + // Base vector is still reachable after the child's edits. + let hits = child.query(&vectors[base_id as usize], 3, &opts).unwrap(); + assert_eq!( + hits[0].id, base_id, + "base vector must remain reachable after child edits" + ); + + // (4) Branch stays small: a COW delta, not a full copy. + child.close().unwrap(); + base.close().unwrap(); + let base_size = std::fs::metadata(&base_path).unwrap().len(); + let child_size = std::fs::metadata(&child_path).unwrap().len(); + assert!( + child_size.saturating_mul(10) < base_size, + "COW branch ({child_size} B) must be far smaller than base ({base_size} B)" + ); + + println!( + "PASS: branch_query_reads_through_to_parent -- base={base_size} B, child={child_size} B" + ); +}