diff --git a/crates/clawhdf5-format/src/datatype.rs b/crates/clawhdf5-format/src/datatype.rs index 2a83e0b..b59652f 100644 --- a/crates/clawhdf5-format/src/datatype.rs +++ b/crates/clawhdf5-format/src/datatype.rs @@ -640,7 +640,8 @@ impl Datatype { mantissa_size, exponent_bias, } => { - let mut bf0 = 0x20u8; // bit 5: sign location bit (standard IEEE 754) + // Bits 4-5: mantissa normalization = 2 (implied leading 1, IEEE 754). + let mut bf0 = 0x20u8; match byte_order { DatatypeByteOrder::BigEndian => { bf0 |= 0x01; @@ -650,9 +651,14 @@ impl Datatype { } _ => {} } - // bf[1] bits 0-1: mantissa normalization = 2 (MSB not stored, IEEE 754) - let bf1 = 0x3fu8; // matching what h5py generates - let mut buf = Self::build_header(1, 1, [bf0, bf1, 0], *size); + // Bits 8-15: the sign bit's position, the top bit of the value. + // This was hard-coded to 63, which is right only for f64: the + // HDF5 library rejects any other float with "sign bit position + // out of bounds", so every f32 dataset and attribute we wrote + // was unreadable by h5py and libhdf5. + let sign_location = + (u32::from(*bit_offset) + u32::from(*bit_precision)).saturating_sub(1) as u8; + let mut buf = Self::build_header(1, 1, [bf0, sign_location, 0], *size); buf.extend_from_slice(&bit_offset.to_le_bytes()); buf.extend_from_slice(&bit_precision.to_le_bytes()); buf.push(*exponent_location); @@ -818,6 +824,20 @@ fn build_dt_header(class: u8, version: u8, bf: [u8; 3], size: u32) -> Vec { mod tests { use super::*; + #[test] + fn float_sign_location_is_the_top_bit_of_the_value() { + // The HDF5 library rejects a float whose sign position is not inside + // its precision; this was hard-coded to 63, so every f32 we wrote was + // unreadable by h5py. Byte 2 of the message is the sign position. + use crate::type_builders::{make_f32_type, make_f64_type}; + for (dt, sign) in [(make_f32_type(), 31), (make_f64_type(), 63)] { + let bytes = dt.serialize(); + assert_eq!(bytes[2], sign, "{dt:?}"); + let (parsed, _) = Datatype::parse(&bytes).unwrap(); + assert_eq!(parsed, dt); + } + } + // Helper to build a fixed-point datatype message fn build_fixed_point( size: u32, diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index c57691b..c262cd4 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -86,6 +86,12 @@ pub(crate) fn build_dataset_oh( let mut dl = Vec::new(); dl.push(4); // version dl.push(1); // class = contiguous + // An empty dataset has no storage: its address must be the undefined + // address, as libhdf5 writes it. A real address with size 0 trips + // libhdf5's `addr + size <= addr` overflow check, and it refuses the + // dataset as "invalid dataset size, likely file corruption" — which made + // every store with no sessions or knowledge graph unreadable by h5py. + let data_addr = if data_size == 0 { u64::MAX } else { data_addr }; dl.extend_from_slice(&data_addr.to_le_bytes()); dl.extend_from_slice(&data_size.to_le_bytes()); w.add_message(MessageType::DataLayout, dl); diff --git a/crates/clawhdf5/tests/h5py_interop_tests.rs b/crates/clawhdf5/tests/h5py_interop_tests.rs index 370b64e..acac2dc 100644 --- a/crates/clawhdf5/tests/h5py_interop_tests.rs +++ b/crates/clawhdf5/tests/h5py_interop_tests.rs @@ -1387,3 +1387,42 @@ with h5py.File("{path_str}", "w", libver="latest") as f: ); } } + +#[test] +fn clawhdf5_writes_f32_h5py_reads() { + // Every f32 dataset used to be unreadable by h5py ("sign bit position out + // of bounds"): the float datatype's sign position was hard-coded for f64. + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("ours_f32.h5"); + let path_str = path.display().to_string(); + let values: Vec = vec![1.5, -2.25, 3.0e-7, 65536.5, f32::MAX, -0.0]; + + let mut fb = FileBuilder::new(); + fb.create_dataset("plain").with_f32_data(&values); + fb.create_dataset("chunked") + .with_f32_data(&values) + .with_shape(&[values.len() as u64]) + .with_chunks(&[4]) + .with_deflate(6); + fb.write(&path).unwrap(); + + let bits = values + .iter() + .map(|v| v.to_bits().to_string()) + .collect::>() + .join(","); + let script = format!( + r#" +import h5py, numpy as np +expected = np.array([{bits}], dtype=np.uint32) +with h5py.File("{path_str}", "r") as f: + for name in ("plain", "chunked"): + d = f[name] + assert d.dtype == np.float32, (name, d.dtype) + assert (d[:].view(np.uint32) == expected).all(), (name, d[:]) +print("ok") +"# + ); + assert_eq!(run_python_output(&script), "ok"); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index 57aa18e..7d41173 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -227,3 +227,55 @@ wrong structure. All four are fixed and covered by interop tests against HDF5 2.0 at sizes that cross each boundary, including paged data blocks. Files written by this crate are unaffected — this was purely a read-path bug. + +## Every `f32` dataset we wrote was unreadable by h5py / libhdf5 + +**Status:** fixed 2026-09-23, after v2.7.0. **Every +release up to and including v2.7.0 is affected** — the encoder was already +wrong in v2.1.0. + +The floating-point datatype message carries the position of the sign bit +(bits 8–15 of its class bit field). `clawhdf5-format` wrote 63 for every +float, which is correct only for `f64`. libhdf5 validates the field, so opening +any `f32` dataset written by this crate failed: + +``` +KeyError: 'Unable to synchronously open object (sign bit position out of bounds)' +``` + +That covers every agent store (`/memory/embeddings`, `norms` and +`activation_weights` are `f32`). `clawhdf5` itself ignores the field on read, +and the interop suites only ever wrote `f64` from our side, so nothing here +noticed. + +**Fix:** the sign position is computed from the type (`bit_offset + +bit_precision - 1`: 15, 31, 63 for half, single, double). Regression tests: +`float_sign_location_is_the_top_bit_of_the_value` (byte level), +`clawhdf5_writes_f32_h5py_reads` and the agent's +`h5py_reads_every_dataset_of_an_agent_store`. + +**Existing files:** an agent store is rewritten in full at every checkpoint, so +it becomes readable by h5py at its next checkpoint with a fixed build. Other +files with `f32` datasets need to be rewritten. + +## Empty datasets we wrote were unreadable by h5py / libhdf5 + +**Status:** fixed 2026-09-23, after v2.7.0. Every +release up to and including v2.7.0 is affected. + +A dataset with no elements was written with a real file address and a storage +size of 0. libhdf5 guards contiguous storage with an overflow check +(`addr + size <= addr`) that is always true when the size is 0, so it rejected +the dataset: + +``` +KeyError: 'Unable to synchronously open object (invalid dataset size, likely file corruption)' +``` + +In practice: every agent store without sessions or a knowledge graph — the +`/sessions` and `/knowledge_graph` datasets are empty until something is added +— could not be read by h5py even once the `f32` bug above was fixed. Found by +the same agent-store interop test. + +**Fix:** an empty contiguous dataset gets the undefined address (all `0xff`), +which is what libhdf5 itself writes.