fix(rvf): native COW dual-graph query correct for cosine metric (recall 0.10→~1.0)

Root cause
----------
The manifest format stored `profile_id` at byte [18] of the header but left
byte [19] as a reserved zero — it did NOT persist the `DistanceMetric`.  When
`boot()` deserialized a manifest it only restored `epoch`, `dimension`, and
`profile_id`; the metric always stayed at `DistanceMetric::L2` (the
`RvfOptions::default()`).

In the COW dual-graph ANN path (`query_via_index_cow`) the parent store is
lazily opened via `open_readonly()` → `boot()`.  Because `boot()` never
restored the metric, every COW child opened its parent with `metric = L2`,
even when the store family was Cosine.  The parent HNSW was then built with
the L2 distance function, and parent query results were L2-ordered distances.
Merging those with the child's cosine distances broke the result ordering:
cosine recall@10 measured at ≈ 0.10 for 32-dim random vectors.

Fix
---
* `DistanceMetric` gets two new `pub(crate)` helpers:
  - `to_id() -> u8`: L2=0, InnerProduct=1, Cosine=2
  - `from_id(u8) -> Self`: reverse mapping (unknown → L2, backward-compatible)

* The manifest write path (`write_manifest_seg_with_identity`) now encodes
  the metric into byte [19] of the header (previously a reserved zero).
  Old stores have 0x00 there → `from_id(0)` == L2 — correct default.

* `ParsedManifest` gains a `metric: DistanceMetric` field parsed from byte [19].

* `boot()` restores `self.options.metric = manifest.metric` so every
  `open()` / `open_readonly()` correctly reflects the stored metric.

Before/after recall
-------------------
| Path                | Before fix | After fix |
|---------------------|-----------|-----------|
| COW cosine recall@10 | ≈ 0.10    | 1.0000    |
| COW L2 recall@10     | 1.0000    | 1.0000    |

Regression test
---------------
New test `cow_ann_recall_vs_exact_cosine` in `cow_ann_recall.rs` mirrors the
existing L2 test with `metric = DistanceMetric::Cosine` and cosine ground
truth; asserts recall@10 ≥ 0.95.  The L2 test (`cow_ann_recall_vs_exact`)
is unchanged and still passes.

Follow-on
---------
A new `@ruvector/rvf-node` native binding build is needed to ship this fix
to the Node.js surface used by agenticow.  Until then, agenticow's existing
L2-normalize workaround (driving the engine with pre-normalized vectors so
L2 and cosine rankings agree) remains correct and safe to keep.

Co-Authored-By: claude-flow <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_019rVRYrRDKyxYK18kuVrDSf
This commit is contained in:
ruvnet 2026-06-28 20:31:30 -04:00
parent 39193f565b
commit fa418a0d1c
5 changed files with 237 additions and 2 deletions

View file

@ -19,6 +19,35 @@ pub enum DistanceMetric {
Cosine,
}
impl DistanceMetric {
/// Encode this metric as a single byte for manifest persistence.
///
/// Encoding: 0 = L2 (default / backward-compatible), 1 = InnerProduct, 2 = Cosine.
/// Old manifests written before this field existed have 0x00 at that byte
/// (it was a reserved zero), so they boot correctly as L2.
pub(crate) fn to_id(self) -> u8 {
match self {
DistanceMetric::L2 => 0,
DistanceMetric::InnerProduct => 1,
DistanceMetric::Cosine => 2,
}
}
/// Decode a metric from a manifest byte.
///
/// Unknown values fall back to L2 for forward-compatibility: a store
/// written by a newer version with an unknown metric ID is treated as
/// L2-distance, which is at least type-safe even if not semantically
/// correct.
pub(crate) fn from_id(id: u8) -> Self {
match id {
1 => DistanceMetric::InnerProduct,
2 => DistanceMetric::Cosine,
_ => DistanceMetric::L2,
}
}
}
/// Compression profile for stored vectors.
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
pub enum CompressionProfile {

View file

@ -6,6 +6,7 @@
//! 3. Background: parse Level 1 -> full segment directory
//! 4. On-demand: load cold segments as queries need them
use crate::options::DistanceMetric;
use rvf_types::{FileIdentity, SegmentHeader, SegmentType, SEGMENT_HEADER_SIZE, SEGMENT_MAGIC};
use std::io::{self, Read, Seek, SeekFrom};
@ -31,6 +32,11 @@ pub(crate) struct ParsedManifest {
pub dimension: u16,
pub total_vectors: u64,
pub profile_id: u8,
/// Distance metric decoded from byte [19] of the manifest header.
///
/// Stores written before this field existed have 0x00 there (reserved),
/// which decodes as `DistanceMetric::L2` — the backward-compatible default.
pub metric: DistanceMetric,
pub segment_dir: Vec<SegDirEntry>,
pub deleted_ids: Vec<u64>,
pub file_identity: Option<FileIdentity>,
@ -159,6 +165,9 @@ fn parse_manifest_payload(payload: &[u8]) -> Option<ParsedManifest> {
]);
let seg_count = u32::from_le_bytes([payload[14], payload[15], payload[16], payload[17]]);
let profile_id = payload[18];
// Byte [19] encodes the distance metric (was reserved zero in older stores).
// DistanceMetric::from_id(0) == L2, so old stores boot correctly.
let metric = DistanceMetric::from_id(payload[19]);
let mut offset = 22; // past header (4+2+8+4+1+3)
@ -269,6 +278,7 @@ fn parse_manifest_payload(payload: &[u8]) -> Option<ParsedManifest> {
dimension,
total_vectors,
profile_id,
metric,
segment_dir,
deleted_ids,
file_identity,

View file

@ -1387,6 +1387,7 @@ impl RvfStore {
self.options.dimension,
total_vectors,
self.options.profile,
self.options.metric.to_id(),
&new_segment_dir,
&empty_dels,
fi,
@ -2467,6 +2468,13 @@ impl RvfStore {
self.epoch = manifest.epoch;
self.options.dimension = manifest.dimension;
self.options.profile = manifest.profile_id;
// Restore the distance metric persisted in the manifest header (byte
// [19], previously a reserved zero). Old stores read 0x00 there and
// boot as L2 — the correct backward-compatible default. Without this
// restore, COW dual-graph queries open the parent via open_readonly()
// which goes through boot() and was silently resetting the metric to
// L2, breaking cosine queries (recall@10 ≈ 0.10 → ≈ 1.0 after fix).
self.options.metric = manifest.metric;
// Pre-size the slab from the manifest so the cold-open load does a
// single allocation instead of growing through repeated doublings.
self.vectors = VectorData::with_capacity(
@ -2577,6 +2585,7 @@ impl RvfStore {
self.options.dimension,
total_vectors,
self.options.profile,
self.options.metric.to_id(),
&self.segment_dir,
&deleted_ids,
fi,

View file

@ -137,7 +137,7 @@ impl SegmentWriter {
/// Write a minimal MANIFEST_SEG recording current state.
///
/// This is a simplified manifest that stores:
/// - epoch, dimension, total_vectors, total_segments, profile_id
/// - epoch, dimension, total_vectors, total_segments, profile_id, metric_id
/// - segment directory entries (seg_id, offset, length, type)
/// - deletion bitmap (vector IDs as simple packed u64 array)
/// - file identity (68 bytes, appended for lineage provenance)
@ -149,6 +149,7 @@ impl SegmentWriter {
dimension: u16,
total_vectors: u64,
profile_id: u8,
metric_id: u8,
segment_dir: &[(u64, u64, u64, u8)], // (seg_id, offset, payload_len, seg_type)
deleted_ids: &[u64],
) -> io::Result<(u64, u64)> {
@ -158,6 +159,7 @@ impl SegmentWriter {
dimension,
total_vectors,
profile_id,
metric_id,
segment_dir,
deleted_ids,
None,
@ -165,6 +167,10 @@ impl SegmentWriter {
}
/// Write a MANIFEST_SEG with optional FileIdentity appended.
///
/// The `metric_id` is encoded into header byte [19] (previously a reserved
/// zero byte): 0 = L2, 1 = InnerProduct, 2 = Cosine. Old stores that
/// wrote 0x00 at that position boot correctly as L2 (backward-compatible).
#[allow(clippy::too_many_arguments)]
pub(crate) fn write_manifest_seg_with_identity<W: Write + Seek>(
&mut self,
@ -173,6 +179,7 @@ impl SegmentWriter {
dimension: u16,
total_vectors: u64,
profile_id: u8,
metric_id: u8,
segment_dir: &[(u64, u64, u64, u8)],
deleted_ids: &[u64],
file_identity: Option<&rvf_types::FileIdentity>,
@ -190,12 +197,15 @@ impl SegmentWriter {
let mut payload = Vec::with_capacity(payload_size);
// Manifest header.
// Layout: epoch[0..4] | dim[4..6] | total_vecs[6..14] | seg_count[14..18]
// | profile_id[18] | metric_id[19] | reserved[20..22]
payload.extend_from_slice(&epoch.to_le_bytes());
payload.extend_from_slice(&dimension.to_le_bytes());
payload.extend_from_slice(&total_vectors.to_le_bytes());
payload.extend_from_slice(&seg_count.to_le_bytes());
payload.push(profile_id);
payload.extend_from_slice(&[0u8; 3]); // reserved
payload.push(metric_id); // byte [19]: distance metric identifier
payload.extend_from_slice(&[0u8; 2]); // bytes [20..22]: reserved
// Segment directory.
for &(sid, off, plen, stype) in segment_dir {

View file

@ -205,6 +205,183 @@ fn cow_ann_recall_vs_exact() {
println!("PASS: cow_ann_recall_vs_exact (recall@{K} = {recall:.4})");
}
// ===========================================================================
// TEST 5: cow_ann_recall_vs_exact_cosine
// ===========================================================================
/// Cosine-metric COW recall regression test.
///
/// This is the primary regression test for the native COW dual-graph cosine
/// bug (fixed in this PR): before the fix the parent store was re-opened via
/// `open_readonly()` which went through `boot()` without restoring the metric,
/// so the parent defaulted to L2. The parent HNSW was built with L2 distance
/// and returned L2-ordered candidates that were then merged with the child's
/// cosine distances — completely breaking the ordering.
///
/// Before fix: cosine recall@10 ≈ 0.10 (bug reproducible here).
/// After fix : cosine recall@10 ≥ 0.95 (metric persisted in manifest).
///
/// Design mirrors `cow_ann_recall_vs_exact` (L2) with:
/// - `metric: DistanceMetric::Cosine`
/// - ground-truth computed via cosine distance (1 cos_sim)
/// - same child-edit mix (60 new, 20 override, 10 tombstone)
fn make_cosine_opts(dim: u16) -> RvfOptions {
RvfOptions {
dimension: dim,
metric: DistanceMetric::Cosine,
..Default::default()
}
}
/// Cosine distance: 1 dot(a,b)/(‖a‖·‖b‖).
fn cosine_dist(a: &[f32], b: &[f32]) -> f32 {
let mut dot = 0.0f32;
let mut na = 0.0f32;
let mut nb = 0.0f32;
for (x, y) in a.iter().zip(b.iter()) {
dot += x * y;
na += x * x;
nb += y * y;
}
let denom = (na * nb).sqrt();
if denom < f32::EPSILON {
1.0
} else {
1.0 - dot / denom
}
}
/// Exact brute-force k-NN over a slice of (id, vector) pairs using cosine
/// distance.
fn exact_knn_cosine(query: &[f32], corpus: &[(u64, Vec<f32>)], k: usize) -> Vec<u64> {
let mut dists: Vec<(u64, f32)> = corpus
.iter()
.map(|(id, v)| (*id, cosine_dist(query, v)))
.collect();
dists.sort_by(|a, b| a.1.total_cmp(&b.1).then_with(|| a.0.cmp(&b.0)));
dists.iter().take(k).map(|(id, _)| *id).collect()
}
#[test]
fn cow_ann_recall_vs_exact_cosine() {
let dir = TempDir::new().unwrap();
let base_path = dir.path().join("base_cos.rvf");
let child_path = dir.path().join("child_cos.rvf");
// Use the same dim/count as the L2 test so the parent slab is large enough
// for HNSW to kick in on both arms.
const DIM: u16 = 32;
const BASE_N: usize = 1_200;
const K: usize = 10;
// ── Build base store (cosine metric) ─────────────────────────────────
let mut base = RvfStore::create(&base_path, make_cosine_opts(DIM)).unwrap();
let base_vecs: Vec<Vec<f32>> = (0..BASE_N)
.map(|i| lcg_vector(DIM as usize, i as u64 + 20_000))
.collect();
let base_refs: Vec<&[f32]> = base_vecs.iter().map(|v| v.as_slice()).collect();
let base_ids: Vec<u64> = (0..BASE_N as u64).collect();
base.ingest_batch(&base_refs, &base_ids, None).unwrap();
base.close().unwrap();
// ── Branch ───────────────────────────────────────────────────────────
let mut base = RvfStore::open(&base_path).unwrap();
// Verify the metric was persisted correctly (sanity check).
assert_eq!(
base.metric(),
DistanceMetric::Cosine,
"base store metric must survive close()+open() round-trip"
);
let mut child = base.branch(&child_path).unwrap();
base.close().unwrap();
// ── Child edits ───────────────────────────────────────────────────────
// (a) 60 new vectors (IDs 5000..5059)
const NEW_START: u64 = 5_000;
const NEW_COUNT: usize = 60;
let new_vecs: Vec<Vec<f32>> = (0..NEW_COUNT)
.map(|i| lcg_vector(DIM as usize, 29_000 + i as u64))
.collect();
let new_refs: Vec<&[f32]> = new_vecs.iter().map(|v| v.as_slice()).collect();
let new_ids: Vec<u64> = (NEW_START..NEW_START + NEW_COUNT as u64).collect();
child.ingest_batch(&new_refs, &new_ids, None).unwrap();
// (b) Override 20 parent vectors (IDs 0..19).
const OVERRIDE_COUNT: usize = 20;
let override_vecs: Vec<Vec<f32>> = (0..OVERRIDE_COUNT)
.map(|i| lcg_vector(DIM as usize, 99_000 + i as u64))
.collect();
let override_refs: Vec<&[f32]> = override_vecs.iter().map(|v| v.as_slice()).collect();
let override_ids: Vec<u64> = (0..OVERRIDE_COUNT as u64).collect();
child
.ingest_batch(&override_refs, &override_ids, None)
.unwrap();
// (c) Tombstone 10 parent vectors (IDs 100..109).
const TOMBSTONE_START: u64 = 100;
const TOMBSTONE_COUNT: usize = 10;
let tombstone_ids: Vec<u64> =
(TOMBSTONE_START..TOMBSTONE_START + TOMBSTONE_COUNT as u64).collect();
child.delete(&tombstone_ids).unwrap();
// ── Build cosine ground-truth corpus visible from child ───────────────
let mut ground_truth_corpus: Vec<(u64, Vec<f32>)> = Vec::new();
let override_set: std::collections::HashSet<u64> = override_ids.iter().copied().collect();
let tombstone_set: std::collections::HashSet<u64> = tombstone_ids.iter().copied().collect();
for (i, v) in base_vecs.iter().enumerate() {
let id = i as u64;
if override_set.contains(&id) || tombstone_set.contains(&id) {
continue;
}
ground_truth_corpus.push((id, v.clone()));
}
for (i, v) in override_vecs.iter().enumerate() {
ground_truth_corpus.push((override_ids[i], v.clone()));
}
for (i, v) in new_vecs.iter().enumerate() {
ground_truth_corpus.push((new_ids[i], v.clone()));
}
// ── Query ─────────────────────────────────────────────────────────────
// Use a query vector near parent vector 500 (not overridden, not tombstoned).
let query = lcg_vector(DIM as usize, 500 + 20_000);
// Exact cosine ground truth.
let exact_top_k = exact_knn_cosine(&query, &ground_truth_corpus, K);
assert_eq!(exact_top_k.len(), K, "ground truth must return K={K} results");
// COW ANN via dual-graph merge (the path that was broken before the fix).
let ann_opts = QueryOptions {
ef_search: 300,
..Default::default()
};
let ann_results = child.query(&query, K, &ann_opts).unwrap();
assert_eq!(ann_results.len(), K, "ANN query must return K={K} results");
let ann_ids: Vec<u64> = ann_results.iter().map(|r| r.id).collect();
let recall = recall_at_k(&ann_ids, &exact_top_k);
println!(
"cow_ann_recall_vs_exact_cosine: recall@{K} = {:.4} (ANN top-{K}: {:?})",
recall, ann_ids
);
// Before the fix this assertion fired with recall ≈ 0.10.
// After the fix (metric persisted in manifest → parent re-opened with
// the correct Cosine metric) recall@10 must be ≥ 0.95.
assert!(
recall >= 0.95,
"recall@{K} {:.4} is below the 0.95 contract — \
possible metric-persistence regression (ANN={:?}, exact={:?})",
recall,
ann_ids,
exact_top_k
);
child.close().unwrap();
println!("PASS: cow_ann_recall_vs_exact_cosine (recall@{K} = {recall:.4})");
}
// ===========================================================================
// TEST 2: cow_ann_override_correctness
// ===========================================================================