Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
07b7301ded | ||
|
|
d3c65ccb58 | ||
|
|
b08df7b628 |
@@ -12,6 +12,7 @@ categories = ["algorithms", "science"]
|
||||
[dependencies]
|
||||
clawhdf5-format = { path = "../clawhdf5-format", version = "2.1.0" }
|
||||
clawhdf5-io = { path = "../clawhdf5-io", version = "2.1.0" }
|
||||
clawhdf5-accel = { path = "../clawhdf5-accel", version = "2.1.0" }
|
||||
rayon = { version = "1", optional = true }
|
||||
|
||||
[features]
|
||||
|
||||
@@ -44,32 +44,14 @@ impl DistanceMetric {
|
||||
}
|
||||
|
||||
/// Compute distance between two vectors using the given metric.
|
||||
///
|
||||
/// Delegates to `clawhdf5-accel`'s runtime-dispatched SIMD kernels (AVX2 on
|
||||
/// x86_64, NEON on aarch64, portable scalar fallback elsewhere) — this is
|
||||
/// the hottest loop in both HNSW build and every `hybrid_search` query.
|
||||
fn compute_distance(a: &[f32], b: &[f32], metric: DistanceMetric) -> f32 {
|
||||
match metric {
|
||||
DistanceMetric::L2 => {
|
||||
let mut sum = 0.0f32;
|
||||
for i in 0..a.len() {
|
||||
let d = a[i] - b[i];
|
||||
sum += d * d;
|
||||
}
|
||||
sum.sqrt()
|
||||
}
|
||||
DistanceMetric::Cosine => {
|
||||
let mut dot = 0.0f32;
|
||||
let mut norm_a = 0.0f32;
|
||||
let mut norm_b = 0.0f32;
|
||||
for i in 0..a.len() {
|
||||
dot += a[i] * b[i];
|
||||
norm_a += a[i] * a[i];
|
||||
norm_b += b[i] * b[i];
|
||||
}
|
||||
let denom = norm_a.sqrt() * norm_b.sqrt();
|
||||
if denom < f32::EPSILON {
|
||||
1.0
|
||||
} else {
|
||||
1.0 - (dot / denom)
|
||||
}
|
||||
}
|
||||
DistanceMetric::L2 => clawhdf5_accel::l2_distance(a, b),
|
||||
DistanceMetric::Cosine => 1.0 - clawhdf5_accel::cosine_similarity(a, b),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -59,11 +59,16 @@ pub trait AsyncHDF5Read: Send + Sync {
|
||||
|
||||
/// Async file-backed reader using tokio for non-blocking I/O.
|
||||
///
|
||||
/// Opens a file and reads it asynchronously. The file is read into memory
|
||||
/// on first access, making subsequent operations fast.
|
||||
/// Opens a file and reads it asynchronously. The underlying file handle is
|
||||
/// opened once (lazily, on first access) and cached for the lifetime of this
|
||||
/// reader, so repeated granular `read_at` calls reuse the open descriptor
|
||||
/// and cached length instead of paying an open+stat syscall pair every time.
|
||||
/// The handle is guarded by a mutex, which also correctly serializes the
|
||||
/// seek-then-read pairs of concurrent callers sharing the one file position.
|
||||
#[derive(Debug)]
|
||||
pub struct AsyncFileReader {
|
||||
path: std::path::PathBuf,
|
||||
handle: tokio::sync::Mutex<Option<(tokio::fs::File, u64)>>,
|
||||
}
|
||||
|
||||
impl AsyncFileReader {
|
||||
@@ -73,6 +78,7 @@ impl AsyncFileReader {
|
||||
pub fn new<P: AsRef<Path>>(path: P) -> Self {
|
||||
Self {
|
||||
path: path.as_ref().to_path_buf(),
|
||||
handle: tokio::sync::Mutex::new(None),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -89,23 +95,33 @@ impl AsyncFileReader {
|
||||
|
||||
impl AsyncHDF5Read for AsyncFileReader {
|
||||
async fn read_at(&self, offset: u64, len: usize) -> io::Result<Vec<u8>> {
|
||||
let mut file = tokio::fs::File::open(&self.path).await?;
|
||||
let metadata = file.metadata().await?;
|
||||
let file_len = metadata.len();
|
||||
let mut guard = self.handle.lock().await;
|
||||
if guard.is_none() {
|
||||
let file = tokio::fs::File::open(&self.path).await?;
|
||||
let file_len = file.metadata().await?.len();
|
||||
*guard = Some((file, file_len));
|
||||
}
|
||||
let (file, file_len) = guard.as_mut().expect("just populated above");
|
||||
let file_len = *file_len;
|
||||
if offset >= file_len {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
let available = (file_len - offset) as usize;
|
||||
let to_read = len.min(available);
|
||||
tokio::io::AsyncSeekExt::seek(&mut file, io::SeekFrom::Start(offset)).await?;
|
||||
tokio::io::AsyncSeekExt::seek(file, io::SeekFrom::Start(offset)).await?;
|
||||
let mut buf = vec![0u8; to_read];
|
||||
file.read_exact(&mut buf).await?;
|
||||
Ok(buf)
|
||||
}
|
||||
|
||||
async fn len(&self) -> io::Result<u64> {
|
||||
let metadata = tokio::fs::metadata(&self.path).await?;
|
||||
Ok(metadata.len())
|
||||
let mut guard = self.handle.lock().await;
|
||||
if guard.is_none() {
|
||||
let file = tokio::fs::File::open(&self.path).await?;
|
||||
let file_len = file.metadata().await?.len();
|
||||
*guard = Some((file, file_len));
|
||||
}
|
||||
Ok(guard.as_ref().expect("just populated above").1)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -49,6 +49,10 @@ pub fn read_hdf5(path: &str) -> Result<SqliteData, BoxErr> {
|
||||
entities,
|
||||
relations,
|
||||
embedding_dim,
|
||||
// Not a SQLite read — the caller (incremental migration) carries
|
||||
// forward the current run's actual `source_path` from the fresh
|
||||
// SQLite read instead of using this placeholder.
|
||||
source_path: String::new(),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ pub fn write_hdf5(
|
||||
opts: &WriteOptions,
|
||||
) -> Result<(), Box<dyn std::error::Error>> {
|
||||
let mut builder = FileBuilder::new();
|
||||
let timestamp = iso8601_now();
|
||||
|
||||
// Root-level metadata attributes
|
||||
builder.set_attr("agent_id", AttrValue::String(opts.agent_id.clone()));
|
||||
@@ -27,8 +28,18 @@ pub fn write_hdf5(
|
||||
builder.set_attr("embedding_dim", AttrValue::I64(data.embedding_dim as i64));
|
||||
builder.set_attr("source", AttrValue::String("sqlite-migration".into()));
|
||||
builder.set_attr("version", AttrValue::I64(1));
|
||||
// Lineage: which SQLite database this output was migrated from and when,
|
||||
// plus the migrator tool version — so a chain of `--incremental` runs
|
||||
// still has an audit trail instead of every run overwriting the same
|
||||
// static attributes (see research/03_provenance.md, INT-03).
|
||||
builder.set_attr("source_path", AttrValue::String(data.source_path.clone()));
|
||||
builder.set_attr("migrated_at", AttrValue::String(timestamp.clone()));
|
||||
builder.set_attr(
|
||||
"migrator_version",
|
||||
AttrValue::String(env!("CARGO_PKG_VERSION").to_owned()),
|
||||
);
|
||||
|
||||
write_chunks_group(&mut builder, data, opts);
|
||||
write_chunks_group(&mut builder, data, opts, ×tamp);
|
||||
write_sessions_group(&mut builder, data);
|
||||
write_entities_group(&mut builder, data);
|
||||
write_relations_group(&mut builder, data);
|
||||
@@ -37,6 +48,36 @@ pub fn write_hdf5(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Current UTC time formatted as an ISO-8601 / RFC-3339 timestamp
|
||||
/// (`YYYY-MM-DDTHH:MM:SSZ`), with no external date/time dependency.
|
||||
fn iso8601_now() -> String {
|
||||
let secs = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.unwrap_or_default()
|
||||
.as_secs();
|
||||
let days = (secs / 86_400) as i64;
|
||||
let time_of_day = secs % 86_400;
|
||||
let (h, m, s) = (time_of_day / 3600, (time_of_day % 3600) / 60, time_of_day % 60);
|
||||
let (y, mo, d) = civil_from_days(days);
|
||||
format!("{y:04}-{mo:02}-{d:02}T{h:02}:{m:02}:{s:02}Z")
|
||||
}
|
||||
|
||||
/// Days-since-epoch to (year, month, day), Howard Hinnant's `civil_from_days`
|
||||
/// algorithm (proleptic Gregorian calendar, valid for the full `i64` range).
|
||||
fn civil_from_days(z: i64) -> (i64, u32, u32) {
|
||||
let z = z + 719_468;
|
||||
let era = if z >= 0 { z } else { z - 146_096 } / 146_097;
|
||||
let doe = (z - era * 146_097) as u64; // [0, 146096]
|
||||
let yoe = (doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365; // [0, 399]
|
||||
let y = yoe as i64 + era * 400;
|
||||
let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); // [0, 365]
|
||||
let mp = (5 * doy + 2) / 153; // [0, 11]
|
||||
let d = (doy - (153 * mp + 2) / 5 + 1) as u32; // [1, 31]
|
||||
let m = (if mp < 10 { mp + 3 } else { mp - 9 }) as u32; // [1, 12]
|
||||
let y = if m <= 2 { y + 1 } else { y };
|
||||
(y, m, d)
|
||||
}
|
||||
|
||||
/// Build a fixed-length string Datatype from the max byte length of the items.
|
||||
fn string_dtype(max_len: usize) -> Datatype {
|
||||
Datatype::String {
|
||||
@@ -66,7 +107,12 @@ fn apply_compression(ds: &mut clawhdf5_format::type_builders::DatasetBuilder, op
|
||||
}
|
||||
}
|
||||
|
||||
fn write_chunks_group(builder: &mut FileBuilder, data: &SqliteData, opts: &WriteOptions) {
|
||||
fn write_chunks_group(
|
||||
builder: &mut FileBuilder,
|
||||
data: &SqliteData,
|
||||
opts: &WriteOptions,
|
||||
timestamp: &str,
|
||||
) {
|
||||
let mut group = builder.create_group("chunks");
|
||||
let n = data.chunks.len() as u64;
|
||||
|
||||
@@ -78,6 +124,16 @@ fn write_chunks_group(builder: &mut FileBuilder, data: &SqliteData, opts: &Write
|
||||
|
||||
group.set_attr("count", AttrValue::I64(n as i64));
|
||||
|
||||
// Source attribution attached directly to the content-bearing datasets
|
||||
// (SHA-256 of the raw bytes + creator/timestamp/source), so the chunk
|
||||
// text and embeddings each carry their own verifiable provenance
|
||||
// (see clawhdf5_format::provenance / `Dataset::verify_provenance`).
|
||||
let source_opt = if data.source_path.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(data.source_path.as_str())
|
||||
};
|
||||
|
||||
// ids
|
||||
let ids: Vec<i64> = data.chunks.iter().map(|c| c.id).collect();
|
||||
group.create_dataset("id").with_i64_data(&ids);
|
||||
@@ -87,7 +143,8 @@ fn write_chunks_group(builder: &mut FileBuilder, data: &SqliteData, opts: &Write
|
||||
let (text_raw, text_len) = pack_strings(&texts);
|
||||
group
|
||||
.create_dataset("text")
|
||||
.with_compound_data(string_dtype(text_len), text_raw, n);
|
||||
.with_compound_data(string_dtype(text_len), text_raw, n)
|
||||
.with_provenance("clawhdf5-migrate", timestamp, source_opt);
|
||||
|
||||
// embeddings - flatten to [N, dim]
|
||||
let dim = data.embedding_dim;
|
||||
@@ -116,7 +173,8 @@ fn write_chunks_group(builder: &mut FileBuilder, data: &SqliteData, opts: &Write
|
||||
let ds = group
|
||||
.create_dataset("embeddings")
|
||||
.with_compound_data(f16_dtype, raw, n)
|
||||
.with_shape(&[n, dim as u64]);
|
||||
.with_shape(&[n, dim as u64])
|
||||
.with_provenance("clawhdf5-migrate", timestamp, source_opt);
|
||||
apply_compression(ds, opts);
|
||||
} else {
|
||||
let flat: Vec<f32> = data
|
||||
@@ -127,7 +185,8 @@ fn write_chunks_group(builder: &mut FileBuilder, data: &SqliteData, opts: &Write
|
||||
let ds = group
|
||||
.create_dataset("embeddings")
|
||||
.with_f32_data(&flat)
|
||||
.with_shape(&[n, dim as u64]);
|
||||
.with_shape(&[n, dim as u64])
|
||||
.with_provenance("clawhdf5-migrate", timestamp, source_opt);
|
||||
apply_compression(ds, opts);
|
||||
}
|
||||
|
||||
@@ -274,3 +333,30 @@ fn write_relations_group(builder: &mut FileBuilder, data: &SqliteData) {
|
||||
|
||||
builder.add_group(group.finish());
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod time_tests {
|
||||
use super::civil_from_days;
|
||||
|
||||
#[test]
|
||||
fn epoch_day_zero_is_1970_01_01() {
|
||||
assert_eq!(civil_from_days(0), (1970, 1, 1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn known_dates_roundtrip() {
|
||||
// 2026-08-16 is 20,681 days after 1970-01-01.
|
||||
assert_eq!(civil_from_days(20_681), (2026, 8, 16));
|
||||
// 2000-02-29 (leap day itself) and 2000-03-01 (the day after).
|
||||
assert_eq!(civil_from_days(11_016), (2000, 2, 29));
|
||||
assert_eq!(civil_from_days(11_017), (2000, 3, 1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn iso8601_now_has_expected_shape() {
|
||||
let ts = super::iso8601_now();
|
||||
assert_eq!(ts.len(), "2026-08-16T00:00:00Z".len());
|
||||
assert!(ts.starts_with("20")); // sanity: 21st-century year
|
||||
assert!(ts.ends_with('Z'));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -154,6 +154,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
base.entities = source.entities;
|
||||
base.relations = source.relations;
|
||||
base.embedding_dim = source.embedding_dim.max(base.embedding_dim);
|
||||
// Carry the current run's real SQLite source forward for
|
||||
// provenance — `base` (re-read from the prior HDF5 output) has
|
||||
// no meaningful source_path of its own.
|
||||
base.source_path = source.source_path;
|
||||
if cli.verbose {
|
||||
eprintln!("Incremental: appended {added} new chunks (id > {min_chunk_id})");
|
||||
}
|
||||
@@ -199,6 +203,11 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
summary.embedding_dim,
|
||||
summary.rows_checked,
|
||||
);
|
||||
if summary.provenance_verified {
|
||||
eprintln!("Provenance: chunks/text and chunks/embeddings SHA-256 hashes verified.");
|
||||
} else if cli.verbose {
|
||||
eprintln!("Provenance: no provenance hash found to verify (older output format?).");
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -51,6 +51,11 @@ pub struct SqliteData {
|
||||
pub entities: Vec<Entity>,
|
||||
pub relations: Vec<Relation>,
|
||||
pub embedding_dim: usize,
|
||||
/// Filesystem path of the SQLite database this data was read from, for
|
||||
/// provenance attribution on the HDF5 output. Empty when the data did
|
||||
/// not come directly from a SQLite read (e.g. re-read of a prior HDF5
|
||||
/// migration output for an incremental merge).
|
||||
pub source_path: String,
|
||||
}
|
||||
|
||||
/// A table name plus the ordered column names the reader maps by position.
|
||||
@@ -225,6 +230,7 @@ pub fn read_sqlite_filtered(
|
||||
entities,
|
||||
relations,
|
||||
embedding_dim: dim,
|
||||
source_path: path.to_owned(),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -1,3 +1,6 @@
|
||||
use clawhdf5::reader::File as Hdf5File;
|
||||
use clawhdf5_format::provenance::VerifyResult;
|
||||
|
||||
use crate::hdf5_reader::read_hdf5;
|
||||
use crate::sqlite_reader::SqliteData;
|
||||
|
||||
@@ -13,6 +16,12 @@ pub struct ValidationSummary {
|
||||
pub embedding_dim: u64,
|
||||
/// Number of 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,
|
||||
}
|
||||
|
||||
/// Validate a migrated HDF5 file against the source data.
|
||||
@@ -30,6 +39,7 @@ pub fn validate_hdf5(
|
||||
float16: bool,
|
||||
) -> Result<ValidationSummary, BoxErr> {
|
||||
let got = read_hdf5(path)?;
|
||||
let provenance_verified = verify_chunk_provenance(path)?;
|
||||
|
||||
// ---- Counts ----
|
||||
check_count("chunk", got.chunks.len(), source.chunks.len())?;
|
||||
@@ -126,6 +136,7 @@ pub fn validate_hdf5(
|
||||
relations: got.relations.len() as u64,
|
||||
embedding_dim: got.embedding_dim as u64,
|
||||
rows_checked,
|
||||
provenance_verified,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -136,6 +147,42 @@ fn check_count(kind: &str, got: usize, expected: usize) -> Result<(), BoxErr> {
|
||||
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()
|
||||
}
|
||||
|
||||
@@ -426,6 +426,7 @@ impl<'f> Dataset<'f> {
|
||||
Ok(data_read::read_as_strings(&raw, &dt)?)
|
||||
}
|
||||
|
||||
|
||||
// ----- Selection-based read methods -----
|
||||
|
||||
/// Read selected elements as raw bytes.
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
# Research: Performance — clawhdf5
|
||||
|
||||
Scope: opportunities not already covered by the Tier 1-4 hardening passes
|
||||
recorded in `ROADMAP.md`/`CHANGELOG.md`/`IMPROVEMENT_LOG.md` (O(1) chunk
|
||||
cache, rayon-parallel `prune_connections`, workspace-hoisted deps, etc).
|
||||
|
||||
## Finding P1 — HNSW's hot distance loop is scalar despite an existing SIMD crate
|
||||
|
||||
**Location:** `crates/clawhdf5-ann/src/hnsw.rs:47-74` (`compute_distance`), called
|
||||
from `greedy_closest` and `search_layer` — the innermost loop of both index
|
||||
build and every `hybrid_search` query.
|
||||
|
||||
**Problem:** `compute_distance` is a plain per-component `for i in 0..a.len()`
|
||||
scalar loop for both the `L2` and `Cosine` metrics. The workspace already ships
|
||||
`clawhdf5-accel` with runtime-dispatched AVX2/NEON/scalar-fallback
|
||||
`l2_distance`/`cosine_similarity` (`crates/clawhdf5-accel/src/lib.rs:125,148`),
|
||||
and `clawhdf5-agent` already depends on and uses it for its own linear cosine
|
||||
scan. `clawhdf5-ann/Cargo.toml` simply never lists `clawhdf5-accel` as a
|
||||
dependency, so the ANN crate — the one place with the tightest, most-called
|
||||
distance loop in the whole codebase — is the one place not using it.
|
||||
|
||||
**Fix implemented (INT-01):** Added `clawhdf5-accel` as a dependency of
|
||||
`clawhdf5-ann` and rewired `compute_distance` to call
|
||||
`clawhdf5_accel::l2_distance` / `clawhdf5_accel::cosine_similarity` (mapping
|
||||
`1.0 - similarity` for the cosine-distance semantics the rest of the file
|
||||
expects). The accel crate already carries its own scalar fallback for
|
||||
platforms without AVX2/NEON, so no separate fallback branch is needed here.
|
||||
Existing `hnsw.rs` unit tests (build/search/serialize round-trip) validate
|
||||
behavior is unchanged; no format or public-API change.
|
||||
|
||||
## Finding P2 — `AsyncFileReader::read_at` reopens and re-stats the file on every call
|
||||
|
||||
**Location:** `crates/clawhdf5-io/src/async_read.rs:90-104`.
|
||||
|
||||
**Problem:** Each `read_at` call does `tokio::fs::File::open` +
|
||||
`.metadata()` + `seek` + `read_exact` — two extra syscalls (open + stat) on
|
||||
every single granular read, with no persistent handle and no buffering. This
|
||||
directly defeats the purpose of the "chunked/granular async access" this type
|
||||
is documented for; callers doing many small reads (e.g. chunked dataset
|
||||
iteration) pay file-open overhead per chunk.
|
||||
|
||||
**Fix implemented (INT-02):** `AsyncFileReader` now lazily opens the file
|
||||
once and caches the open handle (plus its length) behind a `tokio::sync::Mutex`,
|
||||
so subsequent `read_at`/`len` calls reuse the already-open descriptor instead
|
||||
of reopening. First call pays one open+stat; every call after is just a
|
||||
seek+read (or a length lookup with no syscall at all, since length is cached
|
||||
at open time). Behavior (including short-read truncation semantics) is
|
||||
unchanged and covered by the existing `async_file_reader_*` tests.
|
||||
|
||||
## Not implemented — flagged for follow-up
|
||||
|
||||
- **HNSW build-loop parallelism** (`hnsw.rs` insert loop) — ROADMAP already
|
||||
notes this needs its own correctness-sensitive design pass (insert order
|
||||
affects the graph, unlike `prune_connections`'s embarrassingly-parallel
|
||||
per-node distance computation). Left as-is; out of scope for this pass.
|
||||
@@ -0,0 +1,42 @@
|
||||
# Research: Security — clawhdf5
|
||||
|
||||
Scope: opportunities not already covered by the shipped hardening (WAL CRC32
|
||||
trailer / `WAL_VERSION` 2, `MAX_WAL_FIELD_LEN` field caps, Android JNI length
|
||||
validation, `chunked_read.rs`/`data_read.rs` bounds-check + fuzz pass,
|
||||
decompression-bomb output bound, etc — see `ROADMAP.md`).
|
||||
|
||||
## Finding S1 — WAL v2 still allocates untrusted field buffers before the CRC32 check runs
|
||||
|
||||
**Location:** `crates/clawhdf5-agent/src/wal.rs`, entry read path
|
||||
(`read_len_prefixed_str`/`read_embedding` helpers feeding into the `Save`
|
||||
entry parser around lines 340-380; CRC verification happens afterward at
|
||||
~lines 246-255).
|
||||
|
||||
**Problem:** Each `Save` entry currently contains three independent
|
||||
length-prefixed strings plus one length-prefixed embedding buffer. Each field
|
||||
is capped individually at `MAX_WAL_FIELD_LEN` (64 MiB) — but that cap is
|
||||
checked and then the buffer is **allocated immediately** as each field's
|
||||
length prefix is read, before the entry's trailing CRC32 is ever checked. A
|
||||
single corrupted entry (bit-flipped length prefixes) can therefore force up
|
||||
to ~4 allocations near 64 MiB each (~256 MB) before the CRC finally rejects
|
||||
it. This is exactly what `ROADMAP.md`'s "What's Next" section already flags
|
||||
as open: *"a stronger per-entry format (explicit length prefix, avoiding the
|
||||
read-then-verify restructuring) could still be revisited."*
|
||||
|
||||
**Why not implemented in this pass:** Fixing this properly means a WAL format
|
||||
version bump (`WAL_VERSION` 3): frame each entry as one outer
|
||||
`[total_len: u32][entry_bytes][crc32: u32]`, read+CRC-check the whole raw
|
||||
entry buffer *first*, and only then parse the individual fields out of the
|
||||
already-verified buffer — mirroring the v1→v2 migration this file already
|
||||
does on open. That's a real, self-contained, well-testable change (the file
|
||||
already has a legacy-format migration test harness and corruption-detection
|
||||
tests to extend), but it touches the on-disk framing and the read/write pair
|
||||
needs to stay in lock-step, so it deserves its own dedicated
|
||||
implement-and-test pass rather than being bundled in alongside unrelated
|
||||
performance/provenance changes. Tracked as **INT-04** below for follow-up.
|
||||
|
||||
## Finding S2 — no dataset-level integrity check on the agent memory read path
|
||||
|
||||
See `research/03_provenance.md` finding PR2 (`INT-06`) — closely related to
|
||||
security (corruption detection on read), tracked there since the mechanism
|
||||
(`ProvenanceStore::verify_integrity`) is a provenance primitive.
|
||||
@@ -0,0 +1,97 @@
|
||||
# Research: Provenance — clawhdf5
|
||||
|
||||
Scope: data lineage, source attribution, and tamper-evidence for both the
|
||||
low-level HDF5 format layer and the higher-level agent-memory / migration
|
||||
tools built on top of it.
|
||||
|
||||
## Finding PR1 — SHINES provenance (SHA-256 + creator/timestamp/source) is fully built and tested, but zero production write paths use it
|
||||
|
||||
**Location:** `crates/clawhdf5-format/src/provenance.rs` (the whole module —
|
||||
`Provenance::build_attrs`, `sha256_hex`, `verify_dataset`) and
|
||||
`crates/clawhdf5-format/src/type_builders.rs:671-686`
|
||||
(`DatasetBuilder::with_provenance`, feature-gated on `provenance`, which is
|
||||
**on by default** in `clawhdf5-format`).
|
||||
|
||||
**Problem:** This is a complete, working, already-tested feature — it writes
|
||||
`_provenance_sha256` / `_provenance_creator` / `_provenance_timestamp` /
|
||||
`_provenance_source` attributes on a dataset and can re-verify the hash later
|
||||
via `verify_dataset`. `grep -rl with_provenance crates/` shows it is
|
||||
exercised only by `clawhdf5-format`'s own tests/benches
|
||||
(`tests/robustness_tests.rs`, `tests/writer_h5py_tests.rs`,
|
||||
`benches/bench.rs`). Neither `clawhdf5-agent` (the memory backend) nor
|
||||
`clawhdf5-migrate` (the SQLite→HDF5 migration tool — the one place data
|
||||
crosses a genuine trust/source boundary) calls it. Concretely,
|
||||
`crates/clawhdf5-migrate/src/hdf5_writer.rs:24-28` sets only a handful of
|
||||
static root attributes (`agent_id`, `embedder`, `embedding_dim`, a *constant*
|
||||
`source="sqlite-migration"`, a *constant* `version=1`) — there is no source
|
||||
file path, no content hash of the source database, no migration timestamp,
|
||||
and `--incremental` runs (`main.rs` ~122-133) overwrite these same static
|
||||
attributes on every append, so a chain of incremental merges leaves no audit
|
||||
trail: a corrupted incremental append is indistinguishable after the fact
|
||||
from a clean one.
|
||||
|
||||
**Fix implemented (INT-03):** Wired the *existing* SHINES provenance
|
||||
mechanism into the migration write path instead of inventing a new one:
|
||||
|
||||
- `clawhdf5-migrate/src/hdf5_writer.rs`: the `embeddings` and `text` chunk
|
||||
datasets are now built with `.with_provenance("clawhdf5-migrate", <RFC3339
|
||||
timestamp>, Some(<source sqlite path>))`, so each migrated dataset carries
|
||||
a verifiable SHA-256 of its own bytes plus who/when/where it came from.
|
||||
- `clawhdf5-migrate/src/sqlite_reader.rs`: `SqliteData` gained a
|
||||
`source_path: String` field (the SQLite path actually read), threaded
|
||||
through `read_sqlite_filtered`.
|
||||
- `clawhdf5-migrate/src/main.rs`: the incremental-merge arm now carries the
|
||||
*current* run's `source_path` forward instead of silently keeping
|
||||
whatever the previous run recorded.
|
||||
- `clawhdf5-migrate/src/validate.rs`: `validate_hdf5` now also calls
|
||||
`clawhdf5_format::provenance::verify_dataset` on the embeddings dataset and
|
||||
fails validation on a hash mismatch, so migration validation catches
|
||||
post-write corruption, not just source/dest content drift.
|
||||
|
||||
This directly closes the exact gap ROADMAP's "What's Next" implicitly left
|
||||
open (migration recorded no real lineage) using code that was already
|
||||
shipped, tested, and sitting unused one crate over — no new format version,
|
||||
no new dependency, minimal blast radius (2 struct-literal sites for the new
|
||||
`SqliteData` field, both updated).
|
||||
|
||||
## Finding PR2 — agent-level `MemoryProvenance`/`AnomalyDetector` are dead code on the real save path (ROADMAP claims Track 5 "complete")
|
||||
|
||||
**Location:** `crates/clawhdf5-agent/src/lib.rs` (`HDF5Memory::save` /
|
||||
`save_batch`, ~lines 538-572); `crates/clawhdf5-agent/src/provenance.rs`
|
||||
(`MemoryProvenance`, `ProvenanceStore::verify_integrity`/`mark_verified`);
|
||||
`crates/clawhdf5-agent/src/anomaly.rs` (`AnomalyDetector::check_rate_anomaly`
|
||||
/ `check_pattern_anomaly` / `check_source_anomaly`).
|
||||
|
||||
**Problem:** `ROADMAP.md` Track 5 ("Memory Security & Provenance") is marked
|
||||
🟢 Complete, but `save()`/`save_batch()` push straight into the in-memory
|
||||
cache + WAL without ever constructing a `MemoryProvenance` record, without
|
||||
ever calling any `AnomalyDetector` check, and without going through
|
||||
`SourceIsolation`. A `grep` for `provenance::`/`anomaly::` usage across the
|
||||
crate turns up only each module's own `#[cfg(test)]` block. So today a
|
||||
forged- or poisoned-source memory write is stored and later retrieved with
|
||||
zero attribution and zero anomaly screening, contradicting the shipped-status
|
||||
claim in the docs.
|
||||
|
||||
**Why not implemented in this pass:** This is a real fix, but it is
|
||||
core-save-path surgery — it has to interact correctly with the WAL replay
|
||||
path (a provenance record written to cache but not WAL, or vice versa, would
|
||||
silently desync memory from the durable log on crash-recovery) and with
|
||||
`save_batch`'s different code path from `save`. That needs its own focused
|
||||
implement-and-test pass with the existing `provenance.rs`/`anomaly.rs` unit
|
||||
tests as a base, rather than being bundled in under time pressure alongside
|
||||
unrelated changes. Tracked as **INT-05** below.
|
||||
|
||||
## Finding PR3 — nothing on the retrieval path ever calls `verify_integrity`
|
||||
|
||||
**Location:** `crates/clawhdf5-agent/src/provenance.rs:128`
|
||||
(`ProvenanceStore::verify_integrity`), vs. `search.rs`/`hybrid.rs` (no
|
||||
callers).
|
||||
|
||||
**Problem:** Even independent of PR2, nothing in the retrieval pipeline
|
||||
calls `verify_integrity` before returning a chunk to the caller, so
|
||||
corruption of stored chunk text is retrievable and usable without any check
|
||||
ever running.
|
||||
|
||||
**Why not implemented in this pass:** Blocked on PR2/INT-05 landing first —
|
||||
`verify_integrity` needs a `MemoryProvenance` record to check *against*, and
|
||||
none are currently produced. Tracked as **INT-06**, sequenced after INT-05.
|
||||
Reference in New Issue
Block a user