clawhdf5/research/VERIFICATION_BRIEF.md

Verification Brief — branch verify/v3-plus-v6

Independent audit of three already-implemented fixes:

• P1 — clawhdf5-ann::hnsw::compute_distance now delegates to clawhdf5-accel's runtime-dispatched SIMD kernels (l2_distance, cosine_similarity) instead of scalar loops.
• P2 — clawhdf5-io::async_read::AsyncFileReader now opens the file handle once and caches it + its length behind a tokio::sync::Mutex.
• PR1 — clawhdf5-migrate writes SHINES provenance (hdf5_writer.rs) and verifies it on read-back (validate.rs).

Branch state audited: verify/v3-plus-v6 @ 07b7301 (merge of the v3 ann/io/migrate work and v6 agent/format work). All three areas' existing test suites (cargo test -p clawhdf5-accel -p clawhdf5-ann -p clawhdf5-io --features async -p clawhdf5-migrate --release) pass — 41 + 23 + 89 + 26 tests green. That is expected: the defect below is a numerical edge case none of the existing tests exercise.

---

P1 — SIMD distance in clawhdf5-ann — DEFECT FOUND

File: crates/clawhdf5-accel/src/scalar.rs, avx2.rs, avx512.rs, neon.rs (all four backends share the bug identically; it surfaces in callers through crates/clawhdf5-ann/src/hnsw.rs:54, compute_distance's 1.0 - clawhdf5_accel::cosine_similarity(a, b)).

Problem: The near-zero-norm guard in cosine_similarity changed threshold during the SIMD migration, and the new threshold is wrong.

Old scalar loop (pre-SIMD, hnsw.rs @ 55959b4):

let denom = norm_a.sqrt() * norm_b.sqrt();
if denom < f32::EPSILON {
    1.0
} else {
    1.0 - (dot / denom)
}

New code, identical in all four clawhdf5-accel backends (e.g. scalar.rs:23-24):

let denom = (norm_a * norm_b).sqrt();
if denom == 0.0 { 0.0 } else { dot / denom }

The old code clamped any near-zero denominator (anything under f32::EPSILON ≈ 1.19e-7, not just exact zero) to a safe "maximally dissimilar" result. The new code only special-cases an exact 0.0 denominator; anything smaller but nonzero falls through to dot / denom.

For genuinely-zero vectors the two are equivalent (denom == 0.0 in both, and 1.0 - 0.0 == 1.0 matches the old 1.0), and the existing test (hnsw.rs::cosine_zero_vector, clawhdf5-accel::test_cosine_zero_vector) only covers that case — which is why it didn't catch this.

But for vectors with a small (not exactly zero) norm, the two diverge sharply. Concrete repro (values confirmed via a standalone build of both functions):

a = b = [1e-4]        // tiny but nonzero, identical vectors
old cosine distance = 1.0   // "unreliable direction" fallback, correctly
                             // caps degenerate near-zero vectors at max distance
new cosine distance = 0.0   // computed as fully identical

denom here is 1e-8, comfortably below f32::EPSILON (1.19e-7) but not == 0.0, so the old guard fired and the new one doesn't. This is not a narrow floating-point-rounding footgun — the divergence spans roughly three orders of magnitude of vector norm (anything with denom in (0, 1.19e-7)), and it flips the result from "maximally dissimilar" to "identical," the two opposite ends of the distance range. Any HNSW cosine index that indexes or queries a near-zero-magnitude embedding (e.g. an embedder's output for empty/masked/degenerate input, or a soft-deleted/ zeroed-out placeholder vector) will silently rank it as a near-duplicate of other near-zero vectors instead of correctly pushing it to the bottom of results.

Mismatched-length and truly-empty inputs were also checked: empty vectors (a.len() == b.len() == 0) behave identically old vs. new (both hit the zero-denominator path → distance 1.0). Mismatched lengths now panic via assert_eq! in every backend, versus the old code's for i in 0..a.len() (which panicked on OOB if b was shorter, or silently truncated to a's length if b was longer). No caller reaches this: HnswIndex::build_with_metric and insert both assert equal dimensions before any compute_distance call, so mismatched lengths are unreachable in practice — not flagging as a separate defect.

Proposed fix: Restore the epsilon-threshold guard in all four clawhdf5-accel cosine_similarity backends (scalar.rs, avx2.rs, avx512.rs, neon.rs), replacing if denom == 0.0 { 0.0 } with if denom < f32::EPSILON { 0.0 }, so 1.0 - cosine_similarity(...) in hnsw.rs reproduces the old denom < f32::EPSILON → 1.0 fallback exactly. Add a regression test in clawhdf5-accel (e.g. test_cosine_near_zero_norm_clamped) asserting cosine_similarity(&[1e-4], &[1e-4]) returns 0.0 (so 1.0 - sim == 1.0, matching the old HNSW fallback) rather than 1.0, and a matching test in hnsw.rs (cosine_near_zero_vector, alongside the existing cosine_zero_vector) using a tiny-but-nonzero vector pair to lock in compute_distance == 1.0.

TASK: INT-01 — Restore f32::EPSILON near-zero-denom guard in clawhdf5-accel cosine_similarity (all 4 backends) + regression tests

---

P2 — Cached async file handle in clawhdf5-io — SOUND, no defect

File: crates/clawhdf5-io/src/async_read.rs, AsyncFileReader::read_at / ::len (lines 96-126).

Checked against the pre-fix version (diff in b08df7b, which per-call opened a fresh tokio::fs::File and re-stat'd the length):

• No seek/read interleaving across tasks. read_at takes let mut guard = self.handle.lock().await once at the top and then borrows file from that guard (guard.as_mut()) for the rest of the function, including both the seek(...).await and read_exact(...).await calls. Because file is a live borrow of guard, the Rust borrow checker forces guard (and therefore the lock) to stay held across both await points — it cannot be dropped until the whole function returns. tokio::sync::Mutex is specifically designed to be held across .await (unlike std::sync::Mutex), so a second task's read_at call blocks at .lock().await until the first task's seek+read pair has fully completed. A seek from one task can never be followed by a read from another task on the same descriptor.
• Lazy-init race is also covered by the same lock. The if guard.is_none() open-and-populate branch runs under the same guard acquired at the top, so two concurrent first-callers can't both open+overwrite the cached handle; the second one to acquire the lock sees guard.is_some() and reuses it.
• Cached length staleness. The length is cached forever once populated — intentional and documented in the struct's doc comment ("cached for the lifetime of this reader"). Grepped the whole workspace (AsyncFileReader outside async_read.rs itself): zero other callers exist yet, so there's no current code path where a caller observes a stale length against a file that changed size mid-lifetime. If the backing file were truncated externally during the reader's life, the stale (larger) cached length would make read_at attempt to read more than remains on disk — but that fails loudly via read_exact's UnexpectedEof rather than silently returning corrupted/truncated data, which is a safe failure mode, not a correctness bug.
• Short-read/truncation semantics. The offset >= file_len → empty, to_read = len.min(available) logic is byte-for-byte unchanged from the pre-fix version; only the source of file_len changed (cached vs. freshly stat'd). For the current, only-consumer-is-itself usage pattern (open once, read many times, file not mutated externsally during the reader's life) the observable behavior is identical to before.

No item raised for P2.

---

PR1 — SHINES provenance in clawhdf5-migrate — SOUND, no defect

Files: crates/clawhdf5-migrate/src/hdf5_writer.rs, crates/clawhdf5-migrate/src/main.rs, crates/clawhdf5-migrate/src/validate.rs, crates/clawhdf5-migrate/src/hdf5_reader.rs.

• Current-run source path / timestamp on --incremental merges. write_hdf5 (hdf5_writer.rs:23) computes timestamp = iso8601_now() fresh on every call — it is never read from the merged data struct, so the top-level migrated_at attribute and the per-dataset .with_provenance("clawhdf5-migrate", timestamp, source_opt) calls (hdf5_writer.rs:147,177,189) always carry the current run's wall-clock time, incremental or not. For source_path: hdf5_reader::read_hdf5 (used to load the incremental base) explicitly returns source_path: String::new() with a comment noting the caller must carry the real path forward (hdf5_reader.rs:52-56); main.rs:160 (base.source_path = source.source_path) does exactly that — it overwrites the re-read base's placeholder with the freshly re-read SQLite source's path before calling write_hdf5, not a previous run's path. Traced through: on an --incremental run, both the top-level attributes and every per-dataset provenance attribute reflect the current run, not a stale one. test_incremental_migration (main.rs) exercises the merge path and passes, though it doesn't assert on source_path/migrated_at specifically — the coding phase could add that assertion as cheap extra insurance, but it's not fixing a defect, just tightening coverage.
• Hash-mismatch vs. absent-attribute handling. verify_chunk_provenance (validate.rs:161-184) returns Err(...) (fails loudly, wired through validate_hdf5's ?) only on VerifyResult::Mismatch, i.e. an actual recomputed-vs-stored SHA-256 disagreement. VerifyResult::NoHash (attribute absent, e.g. an older output file) is handled separately — it sets all_present = false and continues, returning Ok(false) from verify_chunk_provenance (surfaced as ValidationSummary::provenance_verified == false, not an error). This is correctly asymmetric: real corruption is a hard error, merely-missing provenance metadata is a soft "unverified" signal, matching the documented contract in the function's doc comment.

No item raised for PR1.

---

Summary

Item	Verdict	Follow-up
P1 SIMD distance	Defect — cosine near-zero-norm guard weakened from < f32::EPSILON to == 0.0 across all 4 backends	INT-01
P2 async file handle	Sound	none
PR1 migrate provenance	Sound	none