diff --git a/src/openhuman/memory_store/vectors/store.rs b/src/openhuman/memory_store/vectors/store.rs index 90fd8594d..5dae581a7 100644 --- a/src/openhuman/memory_store/vectors/store.rs +++ b/src/openhuman/memory_store/vectors/store.rs @@ -326,18 +326,32 @@ impl VectorStore { })? .collect::>>()?; - let mut scored: Vec = rows + let scanned = rows.len(); + + // Score-only intermediate: keep metadata as the raw JSON string instead + // of parsing it here. We scan every vector in the namespace but return + // only `limit` rows, so parsing the metadata of all N candidates would + // throw away all but `limit` parses. Defer the parse until after the + // truncation below, where it runs `limit` times instead of N. + struct ScoredRow { + score: f64, + id: String, + namespace: String, + text: String, + meta_str: String, + } + + let mut scored: Vec = rows .into_iter() - .map(|(id, ns, text, blob, meta_str)| { + .map(|(id, namespace, text, blob, meta_str)| { let stored_vec = bytes_to_vec(&blob); let score = cosine_similarity(query_vec, &stored_vec); - let metadata = serde_json::from_str(&meta_str).unwrap_or(serde_json::Value::Null); - SearchResult { - id, - namespace: ns, - text, + ScoredRow { score, - metadata, + id, + namespace, + text, + meta_str, } }) .collect(); @@ -350,14 +364,37 @@ impl VectorStore { }); scored.truncate(limit); + // Parse metadata only for the rows that survived truncation. Invalid + // JSON falls back to Null, but log it so data issues stay diagnosable. + let results: Vec = scored + .into_iter() + .map(|row| { + let metadata = serde_json::from_str(&row.meta_str).unwrap_or_else(|err| { + tracing::debug!( + target: "embeddings.store", + "[vector-store] invalid metadata json: id={}, ns={}, err={err}", + row.id, + row.namespace, + ); + serde_json::Value::Null + }); + SearchResult { + id: row.id, + namespace: row.namespace, + text: row.text, + score: row.score, + metadata, + } + }) + .collect(); + tracing::trace!( target: "embeddings.store", - "[vector-store] search_by_vector: ns={namespace}, scanned={}, returned={}", - scored.len() + scored.capacity() - scored.len(), // approximate total before truncate - scored.len() + "[vector-store] search_by_vector: ns={namespace}, scanned={scanned}, returned={}", + results.len() ); - Ok(scored) + Ok(results) } // ── Delete / management ────────────────────────────────── diff --git a/src/openhuman/memory_store/vectors/store_tests.rs b/src/openhuman/memory_store/vectors/store_tests.rs index c5b9e9af6..2d3c656d5 100644 --- a/src/openhuman/memory_store/vectors/store_tests.rs +++ b/src/openhuman/memory_store/vectors/store_tests.rs @@ -295,6 +295,78 @@ fn search_by_vector_limit_zero() { .is_empty()); } +/// Metadata is parsed only for rows that survive truncation, so the parse +/// must align with the post-sort order — each returned row must carry its +/// own metadata, and dropped rows must not appear. This pins the deferred +/// parse against an off-by-one or mis-zipped mapping after sort/truncate. +#[test] +fn search_by_vector_returns_metadata_of_surviving_rows() { + let store = fake_store(3); + store + .insert_with_vector("near", "ns", "t", &[1.0, 0.0, 0.0], json!({"tag": "near"})) + .unwrap(); + store + .insert_with_vector("mid", "ns", "t", &[0.7, 0.7, 0.0], json!({"tag": "mid"})) + .unwrap(); + store + .insert_with_vector("far", "ns", "t", &[0.0, 0.0, 1.0], json!({"tag": "far"})) + .unwrap(); + + let results = store.search_by_vector("ns", &[1.0, 0.0, 0.0], 2).unwrap(); + + assert_eq!(results.len(), 2, "limit should drop the least similar row"); + assert_eq!(results[0].id, "near"); + assert_eq!(results[1].id, "mid"); + assert!( + results.iter().all(|hit| hit.id != "far"), + "the truncated row must not leak into the results" + ); + // The deferred parse must attach each row's own metadata, not a neighbour's. + for hit in &results { + assert_eq!( + hit.metadata.get("tag").and_then(|v| v.as_str()), + Some(hit.id.as_str()), + "metadata tag should match the row id for {}", + hit.id + ); + } +} + +/// A row with corrupt metadata JSON (e.g. a hand-edited or partially written +/// DB) must not break the search — the deferred parse falls back to `Null` +/// rather than dropping the row or erroring. Inserted raw because +/// `insert_with_vector` always serializes valid JSON. +#[test] +fn search_by_vector_falls_back_to_null_on_invalid_metadata() { + let store = fake_store(3); + { + let conn = store.conn.lock(); + conn.execute( + "INSERT INTO vectors (id, namespace, text, embedding, metadata, created_at, updated_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", + rusqlite::params![ + "bad", + "ns", + "t", + vec_to_bytes(&[1.0, 0.0, 0.0]), + "{not valid json", + 0.0_f64, + 0.0_f64 + ], + ) + .unwrap(); + } + + let results = store.search_by_vector("ns", &[1.0, 0.0, 0.0], 5).unwrap(); + + assert_eq!(results.len(), 1, "the row must still be returned"); + assert_eq!(results[0].id, "bad"); + assert!( + results[0].metadata.is_null(), + "invalid metadata json must fall back to Null" + ); +} + #[test] fn search_by_vector_scores_correct() { let store = fake_store(3);