fix(migrate): write a real clawhdf5-agent store
clawhdf5-migrate wrote a layout of its own (/chunks, /sessions, /entities, /relations, root attributes, no /meta or schema_version) that HDF5Memory::open rejects, so a "migrated" SQLite database could not be used as agent memory — contrary to the README. It now writes through the agent's own API (HDF5Memory::create/open, save_batch, the session cache and the knowledge graph), so there is no second copy of the schema: - sessions and entities/relations carry over; deleted rows become deleted records (or are left out with --skip-deleted); - embeddings follow the library default (float16), --f32 opts out and --float16 is a hidden no-op, as in clawhdf5-cli; the `half`-based conversion is gone; - every source row is checked before the output is created: a wrong embedding length, an empty embedding, a dimension that differs from an existing store's, or a float16 value beyond +-65504 is an error naming the chunk id, and an existing store is left untouched; - --incremental opens the existing store, adds only rows it does not hold (matched by content) and follows the source's deleted flags; - a source with no memory rows needs --embedding-dim; - validation reads the result back with HDF5Memory::open_read_only, compares every field (embeddings bit for bit, round_to_f16 of the source for float16) and checks a migrated record is found by search. clawhdf5-agent gains HDF5Memory::sessions()/sessions_mut(), HDF5Memory::delete_batch (one save, all-or-nothing, no auto-compact), SessionCache::add_at, and re-exports SessionCache/SessionEntry. The old layout's per-dataset SHA-256 provenance attributes have no place in the agent schema and are gone. An adversarial review found two blockers (silent truncation of long embeddings; an --incremental dimension check that could never fire) and four majors (a failed run wiping the existing store, dim-0 stores, deleted-flag drift); all are fixed with regression tests. 42 migrate tests, incl. h5py opening a migrated store. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -1,192 +1,266 @@
|
||||
use clawhdf5::reader::File as Hdf5File;
|
||||
use clawhdf5_format::provenance::VerifyResult;
|
||||
//! Validate a migration by reading the store back the way an agent would:
|
||||
//! through `HDF5Memory::open_read_only`, comparing what it loads with the
|
||||
//! SQLite source, and running a search for a migrated record.
|
||||
|
||||
use std::path::Path;
|
||||
|
||||
use clawhdf5_agent::{AgentMemory, HDF5Memory, SearchOptions};
|
||||
use clawhdf5_format::float16::round_to_f16;
|
||||
|
||||
use crate::hdf5_reader::read_hdf5;
|
||||
use crate::sqlite_reader::SqliteData;
|
||||
use crate::store_writer::{Migration, US_PER_SEC};
|
||||
|
||||
type BoxErr = Box<dyn std::error::Error>;
|
||||
|
||||
/// Summary of a migration validation.
|
||||
#[derive(Debug)]
|
||||
pub struct ValidationSummary {
|
||||
pub chunks: u64,
|
||||
pub sessions: u64,
|
||||
pub entities: u64,
|
||||
pub relations: u64,
|
||||
pub embedding_dim: u64,
|
||||
/// Number of rows whose full content was compared against the source.
|
||||
/// Records in the store (including tombstones).
|
||||
pub count: usize,
|
||||
/// Records in the store that are not deleted.
|
||||
pub active: usize,
|
||||
pub sessions: usize,
|
||||
pub entities: usize,
|
||||
pub relations: usize,
|
||||
pub embedding_dim: usize,
|
||||
pub float16: bool,
|
||||
/// Rows whose full content was compared against the source.
|
||||
pub rows_checked: u64,
|
||||
/// Whether the `chunks/text` and `chunks/embeddings` SHINES provenance
|
||||
/// hashes (written via [`crate::hdf5_writer`]) were both present and
|
||||
/// matched their recomputed SHA-256 on read-back. `false` when either
|
||||
/// dataset has no provenance metadata (e.g. an older output file) or
|
||||
/// there are zero chunks to check.
|
||||
pub provenance_verified: bool,
|
||||
/// Whether a search for a migrated record found it (`false` when there
|
||||
/// was no active migrated record with an embedding to search for).
|
||||
pub search_checked: bool,
|
||||
}
|
||||
|
||||
/// Validate a migrated HDF5 file against the source data.
|
||||
/// Validate the store at `path` against the source rows `migration` wrote.
|
||||
///
|
||||
/// Reads the written file back and compares actual content — chunk text,
|
||||
/// embeddings, and every session/entity/relation field — to the source, not
|
||||
/// just the row counts. When `full` is false a representative sample of chunk
|
||||
/// rows is content-checked (counts and all other groups are always checked in
|
||||
/// full); when `full` is true every chunk row is compared too. `float16` widens
|
||||
/// the embedding tolerance to allow for half-precision quantization.
|
||||
pub fn validate_hdf5(
|
||||
path: &str,
|
||||
/// Counts and the session / entity / relation rows are always checked in
|
||||
/// full. Memory records are content-checked on a representative sample, or
|
||||
/// all of them with `full`. Embeddings must match exactly: the source values
|
||||
/// themselves in an `f32` store, their [`round_to_f16`] in a `float16` one.
|
||||
pub fn validate_store(
|
||||
path: &Path,
|
||||
source: &SqliteData,
|
||||
migration: &Migration,
|
||||
full: bool,
|
||||
float16: bool,
|
||||
) -> Result<ValidationSummary, BoxErr> {
|
||||
let got = read_hdf5(path)?;
|
||||
let provenance_verified = verify_chunk_provenance(path)?;
|
||||
let mut mem = HDF5Memory::open_read_only(path)?;
|
||||
let float16 = mem.config().float16;
|
||||
let dim = mem.config().embedding_dim;
|
||||
|
||||
// ---- Counts ----
|
||||
check_count("chunk", got.chunks.len(), source.chunks.len())?;
|
||||
check_count("session", got.sessions.len(), source.sessions.len())?;
|
||||
check_count("entity", got.entities.len(), source.entities.len())?;
|
||||
check_count("relation", got.relations.len(), source.relations.len())?;
|
||||
if got.embedding_dim != source.embedding_dim {
|
||||
check_count("record", mem.count(), migration.store_count)?;
|
||||
if float16 != migration.float16 {
|
||||
return Err(format!(
|
||||
"embedding_dim mismatch: HDF5 has {}, source has {}",
|
||||
got.embedding_dim, source.embedding_dim
|
||||
"float16 mismatch: store {float16}, expected {}",
|
||||
migration.float16
|
||||
)
|
||||
.into());
|
||||
}
|
||||
if dim != migration.embedding_dim {
|
||||
return Err(format!(
|
||||
"embedding_dim mismatch: store has {dim}, expected {}",
|
||||
migration.embedding_dim
|
||||
)
|
||||
.into());
|
||||
}
|
||||
if !migration.appended_to_existing {
|
||||
check_count("record", mem.count(), migration.records.len())?;
|
||||
check_count("session", mem.sessions().len(), migration.sessions.len())?;
|
||||
check_count(
|
||||
"entity",
|
||||
mem.knowledge().entities.len(),
|
||||
migration.entities.len(),
|
||||
)?;
|
||||
check_count(
|
||||
"relation",
|
||||
mem.knowledge().relations.len(),
|
||||
migration.relations.len(),
|
||||
)?;
|
||||
}
|
||||
|
||||
// ---- Chunk content (sampled or full) ----
|
||||
let (emb_abs, emb_rel) = if float16 { (1e-2, 1e-2) } else { (1e-4, 0.0) };
|
||||
// ---- Memory records (sampled or full) ----
|
||||
let mut rows_checked = 0u64;
|
||||
for i in sample_indices(source.chunks.len(), full) {
|
||||
let (s, g) = (&source.chunks[i], &got.chunks[i]);
|
||||
if s.id != g.id {
|
||||
return Err(field_err("chunk", i, "id", s.id, g.id));
|
||||
let expected_value = |v: f32| if float16 { round_to_f16(v) } else { v };
|
||||
for k in sample_indices(migration.records.len(), full) {
|
||||
let (idx, src) = migration.records[k];
|
||||
let s = &source.chunks[src];
|
||||
let c = &mem.cache;
|
||||
if idx >= c.len() {
|
||||
return Err(
|
||||
format!("record {idx} (chunk id {}) is missing from the store", s.id).into(),
|
||||
);
|
||||
}
|
||||
if s.chunk != g.chunk {
|
||||
let id = s.id;
|
||||
if c.chunks[idx] != s.chunk {
|
||||
return Err(format!(
|
||||
"chunk[{i}].text mismatch: source {:?}, HDF5 {:?}",
|
||||
"record {idx} (chunk id {id}) text mismatch: source {:?}, store {:?}",
|
||||
truncate(&s.chunk),
|
||||
truncate(&g.chunk)
|
||||
truncate(&c.chunks[idx])
|
||||
)
|
||||
.into());
|
||||
}
|
||||
if s.session_id != g.session_id || s.source_channel != g.source_channel || s.tags != g.tags
|
||||
if c.source_channels[idx] != s.source_channel
|
||||
|| c.session_ids[idx] != s.session_id
|
||||
|| c.tags[idx] != s.tags
|
||||
{
|
||||
return Err(format!("chunk[{i}] string field mismatch").into());
|
||||
return Err(format!("record {idx} (chunk id {id}) string field mismatch").into());
|
||||
}
|
||||
if s.deleted != g.deleted {
|
||||
return Err(field_err("chunk", i, "deleted", s.deleted, g.deleted));
|
||||
}
|
||||
if s.embedding.len() != g.embedding.len() {
|
||||
if c.timestamps[idx].to_bits() != s.timestamp.to_bits() {
|
||||
return Err(format!(
|
||||
"chunk[{i}] embedding length mismatch: {} vs {}",
|
||||
s.embedding.len(),
|
||||
g.embedding.len()
|
||||
"record {idx} (chunk id {id}) timestamp mismatch: source {}, store {}",
|
||||
s.timestamp, c.timestamps[idx]
|
||||
)
|
||||
.into());
|
||||
}
|
||||
for (k, (&a, &b)) in s.embedding.iter().zip(g.embedding.iter()).enumerate() {
|
||||
if (a - b).abs() > emb_abs + emb_rel * a.abs() {
|
||||
return Err(
|
||||
format!("chunk[{i}].embedding[{k}] mismatch: source {a}, HDF5 {b}").into(),
|
||||
);
|
||||
let deleted = c.tombstones[idx] != 0;
|
||||
if deleted != (s.deleted != 0) {
|
||||
return Err(format!(
|
||||
"record {idx} (chunk id {id}) deleted mismatch: source {}, store {deleted}",
|
||||
s.deleted != 0
|
||||
)
|
||||
.into());
|
||||
}
|
||||
let got = c.embeddings.get(idx).unwrap_or(&[]);
|
||||
if got.len() != s.embedding.len() {
|
||||
return Err(format!(
|
||||
"record {idx} (chunk id {id}) embedding length mismatch: source {}, store {}",
|
||||
s.embedding.len(),
|
||||
got.len()
|
||||
)
|
||||
.into());
|
||||
}
|
||||
for (j, (&a, &b)) in s.embedding.iter().zip(got).enumerate() {
|
||||
let want = expected_value(a);
|
||||
if want.to_bits() != b.to_bits() && !(want.is_nan() && b.is_nan()) {
|
||||
return Err(format!(
|
||||
"record {idx} (chunk id {id}) embedding[{j}] mismatch: source {a}, \
|
||||
expected {want}, store {b}"
|
||||
)
|
||||
.into());
|
||||
}
|
||||
}
|
||||
rows_checked += 1;
|
||||
}
|
||||
|
||||
// ---- Other groups (always full — they are small) ----
|
||||
for (i, (s, g)) in source.sessions.iter().zip(got.sessions.iter()).enumerate() {
|
||||
if s.id != g.id
|
||||
|| s.start_idx != g.start_idx
|
||||
|| s.end_idx != g.end_idx
|
||||
|| s.channel != g.channel
|
||||
|| s.summary != g.summary
|
||||
{
|
||||
return Err(format!("session[{i}] mismatch").into());
|
||||
// ---- Records tombstoned because their source row was deleted ----
|
||||
for &(idx, src) in &migration.deleted_in_store {
|
||||
let s = &source.chunks[src];
|
||||
let c = &mem.cache;
|
||||
if idx >= c.len() || c.chunks[idx] != s.chunk || c.timestamps[idx] != s.timestamp {
|
||||
return Err(format!("record {idx} (chunk id {}) mismatch or missing", s.id).into());
|
||||
}
|
||||
rows_checked += 1;
|
||||
}
|
||||
for (i, (s, g)) in source.entities.iter().zip(got.entities.iter()).enumerate() {
|
||||
if s.id != g.id
|
||||
|| s.name != g.name
|
||||
|| s.entity_type != g.entity_type
|
||||
|| s.embedding_idx != g.embedding_idx
|
||||
{
|
||||
return Err(format!("entity[{i}] mismatch").into());
|
||||
}
|
||||
rows_checked += 1;
|
||||
}
|
||||
for (i, (s, g)) in source
|
||||
.relations
|
||||
.iter()
|
||||
.zip(got.relations.iter())
|
||||
.enumerate()
|
||||
{
|
||||
if s.src != g.src || s.tgt != g.tgt || s.relation != g.relation {
|
||||
return Err(format!("relation[{i}] mismatch").into());
|
||||
if c.tombstones[idx] == 0 {
|
||||
return Err(format!(
|
||||
"record {idx} (chunk id {}) is deleted in the source but active in the store",
|
||||
s.id
|
||||
)
|
||||
.into());
|
||||
}
|
||||
rows_checked += 1;
|
||||
}
|
||||
|
||||
// ---- Sessions ----
|
||||
let sessions = mem.sessions();
|
||||
for &(at, src) in &migration.sessions {
|
||||
let s = &source.sessions[src];
|
||||
let (Some(e), Some(summary)) = (sessions.entries.get(at), sessions.summaries.get(at))
|
||||
else {
|
||||
return Err(format!("session {:?} is missing from the store", s.id).into());
|
||||
};
|
||||
if e.id != s.id
|
||||
|| e.start_idx != s.start_idx.max(0) as u64
|
||||
|| e.end_idx != s.end_idx.max(0) as u64
|
||||
|| e.channel != s.channel
|
||||
|| *summary != s.summary
|
||||
|| e.ts != s.timestamp * US_PER_SEC
|
||||
{
|
||||
return Err(format!("session {:?} mismatch", s.id).into());
|
||||
}
|
||||
rows_checked += 1;
|
||||
}
|
||||
|
||||
// ---- Knowledge graph ----
|
||||
let kg = mem.knowledge();
|
||||
for &(id, src) in &migration.entities {
|
||||
let s = &source.entities[src];
|
||||
let Some(e) = kg.get_entity(id) else {
|
||||
return Err(format!(
|
||||
"entity {:?} (id {}) is missing from the store",
|
||||
s.name, s.id
|
||||
)
|
||||
.into());
|
||||
};
|
||||
if e.name != s.name || e.entity_type != s.entity_type || e.embedding_idx != s.embedding_idx
|
||||
{
|
||||
return Err(format!("entity {:?} (id {}) mismatch", s.name, s.id).into());
|
||||
}
|
||||
rows_checked += 1;
|
||||
}
|
||||
for &(at, src) in &migration.relations {
|
||||
let s = &source.relations[src];
|
||||
let r = kg.relations.get(at);
|
||||
let ok = r.is_some_and(|r| {
|
||||
Some(&r.src) == migration.entity_ids.get(&s.src)
|
||||
&& Some(&r.tgt) == migration.entity_ids.get(&s.tgt)
|
||||
&& r.relation == s.relation
|
||||
&& r.weight == s.weight as f32
|
||||
&& r.ts == s.timestamp * US_PER_SEC
|
||||
});
|
||||
if !ok {
|
||||
return Err(format!(
|
||||
"relation {} -[{}]-> {} mismatch or missing",
|
||||
s.src, s.relation, s.tgt
|
||||
)
|
||||
.into());
|
||||
}
|
||||
rows_checked += 1;
|
||||
}
|
||||
|
||||
// ---- A migrated record must be findable by search ----
|
||||
let probe = migration
|
||||
.records
|
||||
.iter()
|
||||
.copied()
|
||||
.find(|&(idx, _)| dim > 0 && mem.cache.tombstones[idx] == 0);
|
||||
let search_checked = match probe {
|
||||
None => false,
|
||||
Some((idx, _)) => {
|
||||
let query = mem.cache.embeddings[idx].to_vec();
|
||||
let text = mem.cache.chunks[idx].clone();
|
||||
let hits = mem.search(&query, &text, &SearchOptions::new(10));
|
||||
// A record with the same text is as good a hit: the source may
|
||||
// hold duplicates, and they tie.
|
||||
if !hits.iter().any(|h| h.index == idx || h.chunk == text) {
|
||||
return Err(format!(
|
||||
"search for migrated record {idx} ({:?}) did not return it",
|
||||
truncate(&text)
|
||||
)
|
||||
.into());
|
||||
}
|
||||
true
|
||||
}
|
||||
};
|
||||
|
||||
Ok(ValidationSummary {
|
||||
chunks: got.chunks.len() as u64,
|
||||
sessions: got.sessions.len() as u64,
|
||||
entities: got.entities.len() as u64,
|
||||
relations: got.relations.len() as u64,
|
||||
embedding_dim: got.embedding_dim as u64,
|
||||
count: mem.count(),
|
||||
active: mem.count_active(),
|
||||
sessions: mem.sessions().len(),
|
||||
entities: mem.knowledge().entities.len(),
|
||||
relations: mem.knowledge().relations.len(),
|
||||
embedding_dim: dim,
|
||||
float16,
|
||||
rows_checked,
|
||||
provenance_verified,
|
||||
search_checked,
|
||||
})
|
||||
}
|
||||
|
||||
fn check_count(kind: &str, got: usize, expected: usize) -> Result<(), BoxErr> {
|
||||
if got != expected {
|
||||
return Err(format!("{kind} count mismatch: HDF5 has {got}, source has {expected}").into());
|
||||
return Err(format!("{kind} count mismatch: store has {got}, expected {expected}").into());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Re-verify the SHA-256 provenance hash of `chunks/text` and
|
||||
/// `chunks/embeddings` against their actual stored bytes, catching
|
||||
/// post-write corruption that a plain content comparison against the
|
||||
/// in-memory source wouldn't (the source is compared against what
|
||||
/// `read_hdf5` decoded, not against the raw bytes on disk).
|
||||
///
|
||||
/// Returns `Ok(true)` only if both datasets exist and both hashes match.
|
||||
/// Returns `Ok(false)` (not an error) if a dataset has no provenance
|
||||
/// attributes at all (e.g. a file written before this check existed) or
|
||||
/// there are zero chunks. Returns an error only on an actual hash mismatch —
|
||||
/// that indicates real corruption.
|
||||
fn verify_chunk_provenance(path: &str) -> Result<bool, BoxErr> {
|
||||
let file = Hdf5File::open(path)?;
|
||||
let Ok(chunks) = file.group("chunks") else {
|
||||
return Ok(false);
|
||||
};
|
||||
let mut all_present = true;
|
||||
for name in ["text", "embeddings"] {
|
||||
let Ok(ds) = chunks.dataset(name) else {
|
||||
all_present = false;
|
||||
continue;
|
||||
};
|
||||
match ds.verify_provenance()? {
|
||||
VerifyResult::Ok => {}
|
||||
VerifyResult::NoHash => all_present = false,
|
||||
VerifyResult::Mismatch { stored, computed } => {
|
||||
return Err(format!(
|
||||
"provenance hash mismatch on chunks/{name}: stored {stored}, recomputed {computed} — data may be corrupted"
|
||||
)
|
||||
.into());
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(all_present)
|
||||
}
|
||||
|
||||
fn field_err<T: std::fmt::Display>(kind: &str, i: usize, field: &str, s: T, g: T) -> BoxErr {
|
||||
format!("{kind}[{i}].{field} mismatch: source {s}, HDF5 {g}").into()
|
||||
}
|
||||
|
||||
fn truncate(s: &str) -> String {
|
||||
if s.len() <= 40 {
|
||||
s.to_string()
|
||||
@@ -196,7 +270,7 @@ fn truncate(s: &str) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
/// Indices of chunk rows to content-check. Full = all; otherwise a spread of
|
||||
/// Indices of records to content-check. Full = all; otherwise a spread of
|
||||
/// representative rows (first/last and evenly-spaced interior samples).
|
||||
fn sample_indices(n: usize, full: bool) -> Vec<usize> {
|
||||
if n == 0 {
|
||||
|
||||
Reference in New Issue
Block a user