From 2f15ab84f613bf647e3177835184fa35bc1b8734 Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Tue, 25 Aug 2026 13:17:41 -0700 Subject: [PATCH 1/6] feat: declarative edge uuid_fields with infores-derived namespace Edge ids were UUIDv3 hashes of the entire emitted record, so every field was an identity field: a corrected p_value, a bumped nlp level inside supporting_text, or a reordered source row each minted a brand-new id, and downstream consumers saw a new edge rather than the same edge updated. Graph gains an optional `uuid_fields` list naming the fields that constitute edge identity; only those feed the hash. Unset preserves the whole-record derivation, so the feature is strictly opt-in. Narrowing the inputs removes the accidental cross-graph uniqueness full-record hashing provided, so the UUID domain moves onto the graph's own infores when `uuid_fields` is set, with `uuid_domain` as the escape hatch for graphs that must share an id space. Three correctness fixes make this sound: - Nested values leaked key order into the hash. Only top-level keys were sorted; nested values reached the digest via `Value::to_string`, which under serde_json's preserve_order emits insertion order. Canonicalization now recurses; array order is preserved because it is semantic. - `false` was dropped along with its key while strip_nulls deliberately keeps it, so `{subject, negated: false}` and `{subject}` derived the same id. - Dedup keyed on full record bytes, which was only safe while the id was a pure function of those bytes. Edges now dedup on the derived id itself, so the output can never carry one id twice: an exact repeat collapses, and two genuinely different edges claiming one id raise `uuid-fields-not-a-key`. Keying on the id also drops the dedup set from a full copy of every record (~800 bytes each) to 24 bytes per edge, and the content hash is computed before the id is inserted so no record is cloned to strip it back out. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NuGRZ2fn3gE5zkUrv56Jqh --- rust/src/json.rs | 32 ++- rust/src/ndjson.rs | 413 +++++++++++++++++++++++++++++++++++---- rust/src/uuid.rs | 275 ++++++++++++++++++++++---- src/tablassert/cli.py | 12 +- src/tablassert/errors.py | 2 + src/tablassert/lib.py | 34 +++- src/tablassert/models.py | 62 ++++++ src/tablassert/rs.pyi | 2 +- src/tablassert/study.py | 21 +- 9 files changed, 771 insertions(+), 82 deletions(-) diff --git a/rust/src/json.rs b/rust/src/json.rs index cecbdff..d933223 100644 --- a/rust/src/json.rs +++ b/rust/src/json.rs @@ -65,10 +65,40 @@ pub fn strip_nulls(value: &Value) -> Value { } } -pub fn stable_json_bytes(value: &Value) -> serde_json::Result> { +/// Serialize in the record's own key order. This is the EMITTED form -- what actually +/// gets written to the NDJSON -- so it must not reorder anything. It is deliberately not +/// a canonical form: use `canonical_json_bytes` when comparing two records for equality. +pub fn emitted_json_bytes(value: &Value) -> serde_json::Result> { serde_json::to_vec(value) } +/// Serialize with every object's keys sorted, recursively. +/// +/// `serde_json` is built with `preserve_order`, so plain `to_vec` leaks insertion order: +/// two logically identical records that arrived with different key order produce +/// different bytes. The edge deduper compares records for equality, so it needs a form +/// where "same content" means "same bytes"; array order is preserved because it is +/// semantic. +pub fn canonical_json_bytes(value: &Value) -> serde_json::Result> { + serde_json::to_vec(&canonical_value(value)) +} + +fn canonical_value(value: &Value) -> Value { + match value { + Value::Object(entries) => { + let mut keys: Vec<&String> = entries.keys().collect(); + keys.sort_unstable(); + let mut sorted: Map = Map::with_capacity(entries.len()); + for key in keys { + sorted.insert(key.clone(), canonical_value(&entries[key])); + } + Value::Object(sorted) + } + Value::Array(items) => Value::Array(items.iter().map(canonical_value).collect()), + _ => value.clone(), + } +} + #[cfg(test)] mod tests { use super::strip_nulls; diff --git a/rust/src/ndjson.rs b/rust/src/ndjson.rs index 667109c..39b27bc 100644 --- a/rust/src/ndjson.rs +++ b/rust/src/ndjson.rs @@ -1,4 +1,4 @@ -use crate::json::{stable_json_bytes, strip_nulls}; +use crate::json::{canonical_json_bytes, emitted_json_bytes, strip_nulls}; use crate::uuid::uuid_for_json_object; use pyo3::exceptions::PyRuntimeError; use pyo3::prelude::*; @@ -6,19 +6,23 @@ use rustc_hash::FxHashMap; use serde_json::Value; use std::fs::File; use std::io::{BufRead, BufReader, BufWriter, Write}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; +use uuid::Uuid; use xxhash_rust::xxh64::xxh64; fn runtime_error(error: impl ToString) -> PyErr { PyRuntimeError::new_err(error.to_string()) } -/// Collision-safe dedup decision. Records are bucketed by xxh64 but a record -/// is suppressed ONLY when its canonical bytes exactly match an existing entry -/// in the bucket, so two DISTINCT records whose xxh64 collides both survive. -/// (The former `HashSet` keyed on the hash alone silently dropped the -/// second record on a collision — data loss at scale.) Returns true when -/// `bytes` is new and was recorded. +/// Collision-safe dedup decision for the NODES stream. Records are bucketed by xxh64 +/// but a record is suppressed ONLY when its canonical bytes exactly match an existing +/// entry in the bucket, so two DISTINCT records whose xxh64 collides both survive. +/// (The former `HashSet` keyed on the hash alone silently dropped the second record +/// on a collision — data loss at scale.) Returns true when `bytes` is new and recorded. +/// +/// Edges do NOT use this path: their `id` is a derived hash, so they dedup on the id +/// itself (see `EdgeIndex`), which both guarantees id uniqueness and keeps 16 bytes per +/// edge instead of the whole record. fn record_if_new(seen: &mut FxHashMap>>, bytes: &[u8]) -> bool { let bucket = seen.entry(xxh64(bytes, 0)).or_default(); if bucket.iter().any(|existing| existing.as_slice() == bytes) { @@ -29,8 +33,8 @@ fn record_if_new(seen: &mut FxHashMap>>, bytes: &[u8]) -> bool } } -fn label_edge(mut value: Value, domain: &str) -> PyResult { - let id: String = uuid_for_json_object(domain, &value) +fn label_edge(mut value: Value, domain: &str, fields: Option<&[String]>) -> PyResult { + let id: String = uuid_for_json_object(domain, &value, fields) .ok_or_else(|| runtime_error("expected JSON object"))?; value .as_object_mut() @@ -39,31 +43,169 @@ fn label_edge(mut value: Value, domain: &str) -> PyResult { Ok(value) } -fn finalize_record(value: Value, is_edges: bool, domain: &str) -> PyResult> { +/// A record ready to write, plus (for edges) the content hash of the record as it stood +/// BEFORE its `id` was inserted. +/// +/// Hashing pre-insertion matters twice over: the id is a pure function of the other +/// fields, so a post-insertion hash would agree whenever the ids agree -- exactly the +/// divergence this is meant to detect -- and it avoids cloning every record just to strip +/// one key back out. +struct Finalized { + value: Value, + content: u64, +} + +fn finalize_record( + value: Value, + is_edges: bool, + domain: &str, + fields: Option<&[String]>, +) -> PyResult> { let cleaned: Value = strip_nulls(&value); let is_empty_object: bool = matches!(&cleaned, Value::Object(map) if map.is_empty()); if is_empty_object { return Ok(None); } - if is_edges { - label_edge(cleaned, domain).map(Some) - } else { - Ok(Some(cleaned)) + if !is_edges { + return Ok(Some(Finalized { + value: cleaned, + content: 0, + })); } + let content: u64 = xxh64(&canonical_json_bytes(&cleaned).map_err(runtime_error)?, 0); + label_edge(cleaned, domain, fields).map(|value| Some(Finalized { value, content })) +} + +/// Edge dedup state: derived id -> hash of that edge's canonical, id-free content. +/// +/// Keying on the id (not on the whole record, as the nodes path does) is what makes +/// `uuid_fields` safe: once the hash covers only a subset of the record, two records can +/// share an id while differing in bytes, and byte-keyed dedup would happily emit both -- +/// duplicate edge ids in the output. It is also ~20-25x cheaper: 24 bytes per edge +/// instead of a full copy of every record (~800 bytes each at the 10M-edge scale). +#[derive(Default)] +struct EdgeIndex { + seen: FxHashMap<[u8; 16], u64>, +} + +/// What to do with an edge whose id has been seen before. +enum EdgeVerdict { + /// First sighting of this id -- write it. + Fresh, + /// Byte-identical to the record that already claimed this id -- suppress it. + Duplicate, + /// A DIFFERENT record claims this id: the declared `uuid_fields` are not a key. + Divergent, +} + +impl EdgeIndex { + fn classify(&mut self, id: [u8; 16], content: u64) -> EdgeVerdict { + match self.seen.get(&id) { + None => { + self.seen.insert(id, content); + EdgeVerdict::Fresh + } + Some(existing) if *existing == content => EdgeVerdict::Duplicate, + Some(_) => EdgeVerdict::Divergent, + } + } +} + +fn edge_id_bytes(value: &Value) -> PyResult<[u8; 16]> { + let id: &str = value + .get("id") + .and_then(Value::as_str) + .ok_or_else(|| runtime_error("labeled edge is missing its id"))?; + Uuid::parse_str(id) + .map(Uuid::into_bytes) + .map_err(|error| runtime_error(format!("edge id {id} is not a UUID: {error}"))) +} + +/// Recover the record that first claimed `id` so the error can name what actually +/// differs. Runs ONLY on the failure path -- the build is about to abort, so re-reading +/// the partial output costs nothing in the happy case. +fn find_written_edge(output: &Path, id: &str) -> Option { + let reader = BufReader::new(File::open(output).ok()?); + reader + .lines() + .map_while(Result::ok) + .filter(|line| line.contains(id)) + .find_map(|line| { + let value: Value = serde_json::from_str(&line).ok()?; + (value.get("id").and_then(Value::as_str) == Some(id)).then_some(value) + }) +} + +/// Top-level keys whose values differ between two records. +fn differing_keys(left: &Value, right: &Value) -> Vec { + let (Some(left), Some(right)) = (left.as_object(), right.as_object()) else { + return Vec::new(); + }; + let mut keys: Vec = left + .keys() + .chain(right.keys()) + .filter(|key| key.as_str() != "id") + .filter(|key| left.get(*key) != right.get(*key)) + .cloned() + .collect(); + keys.sort_unstable(); + keys.dedup(); + keys +} + +/// Build the `uuid-fields-not-a-key` diagnostic. +/// +/// The author's declared `uuid_fields` do not uniquely identify an edge in this graph, so +/// two genuinely different records derived the same id. Emitting both would ship duplicate +/// ids; silently dropping one would lose data. Fail, and name the fields that would fix it. +fn not_a_key_error(output: &Path, id: &str, incoming: &Value, fields: Option<&[String]>) -> PyErr { + let declared: String = match fields { + Some(fields) => fields.join(", "), + None => "".to_string(), + }; + let differing: String = find_written_edge(output, id) + .map(|existing| differing_keys(&existing, incoming)) + .filter(|keys| !keys.is_empty()) + .map_or_else( + || " (could not recover the first record to diff)".to_string(), + |keys| format!(" they differ in: {}", keys.join(", ")), + ); + let context: String = ["subject", "predicate", "object"] + .iter() + .filter_map(|key| { + incoming + .get(*key) + .and_then(Value::as_str) + .map(|value| format!("{key}={value}")) + }) + .collect::>() + .join(" "); + runtime_error(format!( + "uuid-fields-not-a-key: declared uuid_fields are not a key for this graph.\n\ + \x20 id {id} is claimed by 2 different edges.\n\ + {differing}\n\ + \x20 declared uuid_fields: {declared}\n\ + \x20 offending edge: {context}\n\ + Add a discriminating field to `uuid_fields` (a qualifier, `has_supporting_studies` \ + for the source row, or the statistic that actually differs)." + )) } #[pyfunction] -#[pyo3(signature = (input, output, is_edges, domain=None))] +#[pyo3(signature = (input, output, is_edges, domain=None, uuid_fields=None))] pub fn dedup_ndjson( input: PathBuf, output: PathBuf, is_edges: bool, domain: Option, + uuid_fields: Option>, ) -> PyResult<()> { let domain: String = domain.unwrap_or_else(|| "TABLASSERT".to_string()); + let fields: Option<&[String]> = uuid_fields.as_deref(); let reader: BufReader = BufReader::new(File::open(input).map_err(runtime_error)?); - let mut writer: BufWriter = BufWriter::new(File::create(output).map_err(runtime_error)?); - let mut seen: FxHashMap>> = FxHashMap::default(); + let mut writer: BufWriter = BufWriter::new(File::create(&output).map_err(runtime_error)?); + let mut nodes: FxHashMap>> = FxHashMap::default(); + let mut edges: EdgeIndex = EdgeIndex::default(); reader .lines() @@ -72,14 +214,35 @@ pub fn dedup_ndjson( .map(|line| { line.and_then(|line| serde_json::from_str::(&line).map_err(runtime_error)) }) - .map(|value| value.and_then(|value| finalize_record(value, is_edges, &domain))) + .map(|value| value.and_then(|value| finalize_record(value, is_edges, &domain, fields))) .try_for_each(|record| -> PyResult<()> { - if let Some(value) = record? { - let bytes: Vec = stable_json_bytes(&value).map_err(runtime_error)?; - if record_if_new(&mut seen, &bytes) { - writer.write_all(&bytes).map_err(runtime_error)?; - writer.write_all(b"\n").map_err(runtime_error)?; + let Some(Finalized { value, content }) = record? else { + return Ok(()); + }; + let write: bool = if is_edges { + // Edges dedup on their derived id, so the output can never carry the same + // id twice: an exact repeat is suppressed and a genuine divergence aborts + // the build rather than shipping a duplicate. + match edges.classify(edge_id_bytes(&value)?, content) { + EdgeVerdict::Fresh => true, + EdgeVerdict::Duplicate => false, + EdgeVerdict::Divergent => { + writer.flush().map_err(runtime_error)?; + let id: &str = value["id"].as_str().unwrap_or_default(); + return Err(not_a_key_error(&output, id, &value, fields)); + } } + } else { + record_if_new( + &mut nodes, + &emitted_json_bytes(&value).map_err(runtime_error)?, + ) + }; + if write { + writer + .write_all(&emitted_json_bytes(&value).map_err(runtime_error)?) + .map_err(runtime_error)?; + writer.write_all(b"\n").map_err(runtime_error)?; } Ok(()) })?; @@ -91,7 +254,7 @@ pub fn dedup_ndjson( #[cfg(test)] mod tests { - use super::{dedup_ndjson, record_if_new}; + use super::{dedup_ndjson, differing_keys, record_if_new}; use rustc_hash::FxHashMap; use serde_json::Value; use std::fs; @@ -124,7 +287,7 @@ mod tests { let output = dir.path().join("nodes.ndjson"); fs::write(&input, "{\"id\":\"A\"}\n{\"id\":\"B\"}\n{\"id\":\"A\"}\n").expect("write input"); - dedup_ndjson(input, output.clone(), false, None).expect("dedup nodes"); + dedup_ndjson(input, output.clone(), false, None, None).expect("dedup nodes"); let lines: Vec = fs::read_to_string(output) .expect("read output") @@ -148,7 +311,7 @@ mod tests { ) .expect("write input"); - dedup_ndjson(input, output.clone(), false, None).expect("dedup nodes"); + dedup_ndjson(input, output.clone(), false, None, None).expect("dedup nodes"); let lines: Vec = fs::read_to_string(output) .expect("read output") @@ -169,8 +332,14 @@ mod tests { ) .expect("write input"); - dedup_ndjson(input, output.clone(), true, Some("TABLASSERT".to_string())) - .expect("dedup edges"); + dedup_ndjson( + input, + output.clone(), + true, + Some("TABLASSERT".to_string()), + None, + ) + .expect("dedup edges"); let line = fs::read_to_string(output).expect("read output"); let value: Value = serde_json::from_str(line.trim()).expect("json"); @@ -192,8 +361,14 @@ mod tests { ) .expect("write input"); - dedup_ndjson(input, output.clone(), true, Some("TABLASSERT".to_string())) - .expect("dedup edges"); + dedup_ndjson( + input, + output.clone(), + true, + Some("TABLASSERT".to_string()), + None, + ) + .expect("dedup edges"); let lines: Vec = fs::read_to_string(output) .expect("read output") @@ -217,7 +392,7 @@ mod tests { ) .expect("write input"); - dedup_ndjson(input, output.clone(), false, None).expect("dedup nodes"); + dedup_ndjson(input, output.clone(), false, None, None).expect("dedup nodes"); let line = fs::read_to_string(output).expect("read output"); let value: Value = serde_json::from_str(line.trim()).expect("json"); @@ -232,7 +407,7 @@ mod tests { let output = dir.path().join("nodes.ndjson"); fs::write(&input, "{}\n{\"drop\":\"NA\"}\n").expect("write input"); - dedup_ndjson(input, output.clone(), false, None).expect("dedup nodes"); + dedup_ndjson(input, output.clone(), false, None, None).expect("dedup nodes"); assert_eq!(fs::read_to_string(output).expect("read output"), ""); } @@ -244,11 +419,181 @@ mod tests { let output = dir.path().join("nodes.ndjson"); fs::write(&input, "\n \n{\"id\":\"A\"}\n\n{\"id\":\"A\"}\n \n").expect("write input"); - dedup_ndjson(input, output.clone(), false, None).expect("dedup nodes"); + dedup_ndjson(input, output.clone(), false, None, None).expect("dedup nodes"); assert_eq!( fs::read_to_string(output).expect("read output"), "{\"id\":\"A\"}\n" ); } + + fn write_edges(dir: &std::path::Path, body: &str) -> (std::path::PathBuf, std::path::PathBuf) { + let input = dir.join("edges.ndjson.tmp"); + let output = dir.join("edges.ndjson"); + fs::write(&input, body).expect("write input"); + (input, output) + } + + fn edge_ids(output: &std::path::Path) -> Vec { + fs::read_to_string(output) + .expect("read output") + .lines() + .map(|line| { + serde_json::from_str::(line).expect("json")["id"] + .as_str() + .expect("edge id") + .to_string() + }) + .collect() + } + + #[test] + fn declared_uuid_fields_hold_the_id_still_across_attribute_edits() { + // WHY: the whole point of `uuid_fields`. Two builds whose only difference is a + // p_value must produce the SAME edge id, so downstream sees one edge updated + // rather than one retired and one created. + let dir = tempdir().expect("tempdir"); + let fields = Some(vec![ + "subject".to_string(), + "predicate".to_string(), + "object".to_string(), + ]); + + let (before_in, before_out) = write_edges( + dir.path(), + "{\"subject\":\"A\",\"predicate\":\"r\",\"object\":\"B\",\"p_value\":\"0.01\"}\n", + ); + dedup_ndjson(before_in, before_out.clone(), true, None, fields.clone()).expect("dedup"); + + let after_dir = tempdir().expect("tempdir"); + let (after_in, after_out) = write_edges( + after_dir.path(), + "{\"subject\":\"A\",\"predicate\":\"r\",\"object\":\"B\",\"p_value\":\"0.99\",\"effect_size\":1.5}\n", + ); + dedup_ndjson(after_in, after_out.clone(), true, None, fields).expect("dedup"); + + assert_eq!(edge_ids(&before_out), edge_ids(&after_out)); + } + + #[test] + fn undeclared_uuid_fields_let_the_id_drift() { + // WHY: the contrast case -- with no `uuid_fields`, the same attribute edit moves + // the id. This is the pre-16.0.0 behavior the default still preserves. + let dir = tempdir().expect("tempdir"); + let (before_in, before_out) = write_edges( + dir.path(), + "{\"subject\":\"A\",\"predicate\":\"r\",\"object\":\"B\",\"p_value\":\"0.01\"}\n", + ); + dedup_ndjson(before_in, before_out.clone(), true, None, None).expect("dedup"); + + let after_dir = tempdir().expect("tempdir"); + let (after_in, after_out) = write_edges( + after_dir.path(), + "{\"subject\":\"A\",\"predicate\":\"r\",\"object\":\"B\",\"p_value\":\"0.99\"}\n", + ); + dedup_ndjson(after_in, after_out.clone(), true, None, None).expect("dedup"); + + assert_ne!(edge_ids(&before_out), edge_ids(&after_out)); + } + + #[test] + fn uuid_fields_that_are_not_a_key_abort_the_build() { + // WHY: narrowing the hash inputs can make two DIFFERENT edges share an id. + // Shipping both would emit duplicate ids; dropping one would lose data. Fail, + // and name the fields that would disambiguate. + let dir = tempdir().expect("tempdir"); + let (input, output) = write_edges( + dir.path(), + concat!( + "{\"subject\":\"A\",\"predicate\":\"r\",\"object\":\"B\",\"p_value\":\"0.01\"}\n", + "{\"subject\":\"A\",\"predicate\":\"r\",\"object\":\"B\",\"p_value\":\"0.99\"}\n" + ), + ); + let fields = Some(vec![ + "subject".to_string(), + "predicate".to_string(), + "object".to_string(), + ]); + let error = dedup_ndjson(input, output, true, None, fields).expect_err("not a key"); + let message = error.to_string(); + assert!(message.contains("uuid-fields-not-a-key"), "{message}"); + assert!(message.contains("p_value"), "{message}"); + assert!(message.contains("subject=A"), "{message}"); + } + + #[test] + fn identical_edges_still_collapse_under_declared_uuid_fields() { + // WHY: a true repeat is not a key violation -- it is the duplicate the deduper + // exists to collapse. + let dir = tempdir().expect("tempdir"); + let (input, output) = write_edges( + dir.path(), + concat!( + "{\"subject\":\"A\",\"predicate\":\"r\",\"object\":\"B\",\"p_value\":\"0.01\"}\n", + "{\"subject\":\"A\",\"predicate\":\"r\",\"object\":\"B\",\"p_value\":\"0.01\"}\n" + ), + ); + let fields = Some(vec!["subject".to_string(), "object".to_string()]); + dedup_ndjson(input, output.clone(), true, None, fields).expect("dedup edges"); + assert_eq!(edge_ids(&output).len(), 1); + } + + #[test] + fn key_order_alone_no_longer_ships_a_duplicate_id() { + // WHY: `uuid_for_json_object` sorts keys but the emitted bytes preserve insertion + // order, so byte-keyed dedup used to keep BOTH of these -- same id, two lines. + // Keying on the id collapses them. + let dir = tempdir().expect("tempdir"); + let (input, output) = write_edges( + dir.path(), + concat!( + "{\"subject\":\"A\",\"object\":\"B\"}\n", + "{\"object\":\"B\",\"subject\":\"A\"}\n" + ), + ); + dedup_ndjson(input, output.clone(), true, None, None).expect("dedup edges"); + assert_eq!(edge_ids(&output).len(), 1); + } + + #[test] + fn the_domain_separates_identical_edges_across_graphs() { + // WHY: two graphs asserting the same triple must not mint the same id, which is + // what makes a narrow `uuid_fields` safe across KGs. + let left_dir = tempdir().expect("tempdir"); + let (left_in, left_out) = + write_edges(left_dir.path(), "{\"subject\":\"A\",\"object\":\"B\"}\n"); + dedup_ndjson( + left_in, + left_out.clone(), + true, + Some("infores:left".to_string()), + None, + ) + .expect("dedup"); + + let right_dir = tempdir().expect("tempdir"); + let (right_in, right_out) = + write_edges(right_dir.path(), "{\"subject\":\"A\",\"object\":\"B\"}\n"); + dedup_ndjson( + right_in, + right_out.clone(), + true, + Some("infores:right".to_string()), + None, + ) + .expect("dedup"); + + assert_ne!(edge_ids(&left_out), edge_ids(&right_out)); + } + + #[test] + fn differing_keys_reports_only_real_differences_and_never_the_id() { + let left = serde_json::json!({"id": "x", "subject": "A", "p_value": "0.01"}); + let right = + serde_json::json!({"id": "y", "subject": "A", "p_value": "0.99", "effect_size": 1.0}); + assert_eq!( + differing_keys(&left, &right), + vec!["effect_size", "p_value"] + ); + } } diff --git a/rust/src/uuid.rs b/rust/src/uuid.rs index bffd092..1893083 100644 --- a/rust/src/uuid.rs +++ b/rust/src/uuid.rs @@ -1,5 +1,5 @@ use pyo3::prelude::*; -use serde_json::Value; +use serde_json::{Map, Value}; use uuid::Uuid; const NIL_NAMESPACE: Uuid = Uuid::from_bytes([0; 16]); @@ -8,17 +8,46 @@ fn uuid3(namespace: Uuid, name: &str) -> Uuid { Uuid::new_v3(&namespace, name.as_bytes()) } +/// Recursively sort object keys so a nested value's serialization is independent of +/// insertion order. `serde_json` is built with `preserve_order`, so `Value::to_string` +/// on a nested object emits INSERTION order -- the top-level sort in +/// `uuid_for_json_object` never reached inside `sources` or `has_supporting_studies`, +/// and a polars struct-field reordering silently re-minted every edge id. Array order +/// is preserved: it is semantic in JSON, and reordering it would conflate genuinely +/// different sequences. +fn canonicalize(value: &Value) -> Value { + match value { + Value::Object(entries) => { + let mut keys: Vec<&String> = entries.keys().collect(); + keys.sort_unstable(); + let mut sorted: Map = Map::with_capacity(entries.len()); + for key in keys { + sorted.insert(key.clone(), canonicalize(&entries[key])); + } + Value::Object(sorted) + } + Value::Array(items) => Value::Array(items.iter().map(canonicalize).collect()), + _ => value.clone(), + } +} + fn uuid_part(value: &Value) -> Option { match value { - Value::Null | Value::Bool(false) => None, - Value::Bool(true) => Some("true".to_string()), + // `null` never reaches the hash: `strip_nulls` removes those keys before + // labeling. Dropping it here too keeps the two passes in agreement. + Value::Null => None, + // `false` MUST hash, even though it is falsy. `strip_nulls` deliberately keeps + // it (`negated: false` is a meaningful Biolink value), so dropping the key here + // made `{subject: A, negated: false}` and `{subject: A}` derive the SAME id + // while remaining distinct records -- a duplicate-id path. + Value::Bool(flag) => Some(if *flag { "true" } else { "false" }.to_string()), Value::Number(number) => Some(number.to_string()), - Value::String(text) => (!text.is_empty()).then_some(text.clone()), - Value::Array(_) | Value::Object(_) => Some(value.to_string()), + Value::String(text) => (!text.is_empty()).then(|| text.clone()), + Value::Array(_) | Value::Object(_) => Some(canonicalize(value).to_string()), } } -pub fn uuid_from_parts(domain: &str, values: impl IntoIterator) -> String { +pub fn uuid_from_parts(domain: &str, values: impl IntoIterator>) -> String { let domainspace: Uuid = uuid3(NIL_NAMESPACE, domain); // Injective encoding: length-prefix each part as `:` so no two // distinct part lists serialize to the same string. A plain separator join is @@ -28,37 +57,52 @@ pub fn uuid_from_parts(domain: &str, values: impl IntoIterator) - // with respect to its inputs. let mut joined: String = String::new(); for part in values { + let part: &str = part.as_ref(); joined.push_str(&part.len().to_string()); joined.push(':'); - joined.push_str(&part); + joined.push_str(part); } uuid3(domainspace, &joined).to_string() } -pub fn uuid_for_json_object(domain: &str, value: &Value) -> Option { - value - .as_object() - .map(|object| { - // Canonicalize: sort entries by key so the same logical object - // hashes identically regardless of insertion order (serde_json's - // preserve_order otherwise leaks key order into the UUID). Feed each - // key and its normalized value as SEPARATE parts — never a combined - // "key=value" string — so the encoding stays injective: a combined form - // would let {"a":"b=c"} and {"a=b":"c"} collide on the part "a=b=c". - // Entries whose value normalizes to nothing (null, false, empty — see - // `uuid_part`) are dropped, key included. - let mut entries: Vec<(&String, &Value)> = object.iter().collect(); - entries.sort_by_key(|(left, _)| *left); - let mut parts: Vec = Vec::with_capacity(entries.len() * 2); - for (key, value) in entries { - if let Some(part) = uuid_part(value) { - parts.push(key.clone()); - parts.push(part); - } - } - parts - }) - .map(|values| uuid_from_parts(domain, values)) +/// Derive an object's UUID, optionally over a declared subset of its keys. +/// +/// `fields` is the graph config's `uuid_fields`. `None` hashes every key (the default, +/// byte-compatible with pre-16.0.0 output); `Some` hashes only the named top-level keys, +/// so an edge's id stops moving when a non-identity attribute (`p_value`, `effect_size`, +/// `supporting_text`) changes. A declared field absent from the record contributes +/// nothing at all -- neither key nor value -- so `{subject: A}` and +/// `{subject: A, negated: true}` stay distinct. +pub fn uuid_for_json_object( + domain: &str, + value: &Value, + fields: Option<&[String]>, +) -> Option { + let object: &Map = value.as_object()?; + // Canonicalize: visit entries in sorted key order so the same logical object + // hashes identically regardless of insertion order (serde_json's preserve_order + // otherwise leaks key order into the UUID). Feed each key and its normalized + // value as SEPARATE parts -- never a combined "key=value" string -- so the + // encoding stays injective: a combined form would let {"a":"b=c"} and + // {"a=b":"c"} collide on the part "a=b=c". Entries whose value normalizes to + // nothing (null, empty -- see `uuid_part`) are dropped, key included. + let mut keys: Vec<&String> = match fields { + Some(declared) => declared + .iter() + .filter(|key| object.contains_key(*key)) + .collect(), + None => object.keys().collect(), + }; + keys.sort_unstable(); + keys.dedup(); + let mut parts: Vec = Vec::with_capacity(keys.len() * 2); + for key in keys { + if let Some(part) = uuid_part(&object[key]) { + parts.push(key.clone()); + parts.push(part); + } + } + Some(uuid_from_parts(domain, parts)) } #[pyfunction] @@ -83,7 +127,7 @@ mod tests { #[test] fn uuid_for_json_object_returns_uuid_shape() { let value = json!({"subject": "A", "object": "B", "predicate": "biolink:related_to"}); - let id = uuid_for_json_object("TABLASSERT", &value).expect("object UUID"); + let id = uuid_for_json_object("TABLASSERT", &value, None).expect("object UUID"); Uuid::parse_str(&id).expect("valid UUID"); } @@ -94,8 +138,8 @@ mod tests { // in preserve_order insertion order, so a reordered object changed ID. let forward = json!({"subject": "A", "object": "B", "predicate": "r"}); let backward = json!({"predicate": "r", "object": "B", "subject": "A"}); - let forward_id = uuid_for_json_object("TABLASSERT", &forward).expect("object UUID"); - let backward_id = uuid_for_json_object("TABLASSERT", &backward).expect("object UUID"); + let forward_id = uuid_for_json_object("TABLASSERT", &forward, None).expect("object UUID"); + let backward_id = uuid_for_json_object("TABLASSERT", &backward, None).expect("object UUID"); assert_eq!(forward_id, backward_id); } @@ -105,8 +149,8 @@ mod tests { // share values under different keys get distinct UUIDs. let ab = json!({"a": "x", "b": "y"}); let cd = json!({"c": "x", "d": "y"}); - let ab_id = uuid_for_json_object("TABLASSERT", &ab).expect("object UUID"); - let cd_id = uuid_for_json_object("TABLASSERT", &cd).expect("object UUID"); + let ab_id = uuid_for_json_object("TABLASSERT", &ab, None).expect("object UUID"); + let cd_id = uuid_for_json_object("TABLASSERT", &cd, None).expect("object UUID"); assert_ne!(ab_id, cd_id); } @@ -118,8 +162,8 @@ mod tests { // (["a","b=c"] vs ["a=b","c"]) keep them distinct. let split_value = json!({"a": "b=c"}); let split_key = json!({"a=b": "c"}); - let value_id = uuid_for_json_object("TABLASSERT", &split_value).expect("object UUID"); - let key_id = uuid_for_json_object("TABLASSERT", &split_key).expect("object UUID"); + let value_id = uuid_for_json_object("TABLASSERT", &split_value, None).expect("object UUID"); + let key_id = uuid_for_json_object("TABLASSERT", &split_key, None).expect("object UUID"); assert_ne!(value_id, key_id); } @@ -143,4 +187,159 @@ mod tests { ); assert_ne!(nested, flat); } + + #[test] + fn nested_object_key_order_does_not_change_the_uuid() { + // WHY: the top-level sort never reached inside nested values, which were hashed + // via `Value::to_string` under serde_json's `preserve_order`. A polars struct + // field reordering inside `sources` therefore re-minted every edge id. + let forward = json!({"subject": "A", "sources": [{"resource_id": "infores:x", "resource_role": "primary_knowledge_source"}]}); + let backward = json!({"subject": "A", "sources": [{"resource_role": "primary_knowledge_source", "resource_id": "infores:x"}]}); + let forward_id = uuid_for_json_object("TABLASSERT", &forward, None).expect("object UUID"); + let backward_id = uuid_for_json_object("TABLASSERT", &backward, None).expect("object UUID"); + assert_eq!(forward_id, backward_id); + } + + #[test] + fn nested_array_order_still_changes_the_uuid() { + // WHY: canonicalization sorts object KEYS only. Array order is semantic, so two + // different sequences must stay distinguishable. + let forward = json!({"publications": ["PMID:1", "PMID:2"]}); + let backward = json!({"publications": ["PMID:2", "PMID:1"]}); + let forward_id = uuid_for_json_object("TABLASSERT", &forward, None).expect("object UUID"); + let backward_id = uuid_for_json_object("TABLASSERT", &backward, None).expect("object UUID"); + assert_ne!(forward_id, backward_id); + } + + #[test] + fn false_is_hashed_rather_than_dropped() { + // WHY: `strip_nulls` keeps `false` (`negated: false` is meaningful), so dropping + // the key here made these two DISTINCT records derive the SAME id. + let negated = json!({"subject": "A", "negated": false}); + let bare = json!({"subject": "A"}); + let negated_id = uuid_for_json_object("TABLASSERT", &negated, None).expect("object UUID"); + let bare_id = uuid_for_json_object("TABLASSERT", &bare, None).expect("object UUID"); + assert_ne!(negated_id, bare_id); + // ...and `false` stays distinguishable from `true`. + let affirmed = json!({"subject": "A", "negated": true}); + let affirmed_id = uuid_for_json_object("TABLASSERT", &affirmed, None).expect("object UUID"); + assert_ne!(negated_id, affirmed_id); + } + + #[test] + fn declared_fields_ignore_undeclared_changes() { + // WHY: the whole point of `uuid_fields` -- an attribute-only edit must not move + // the id. + let fields = vec![ + "subject".to_string(), + "predicate".to_string(), + "object".to_string(), + ]; + let before = json!({"subject": "A", "predicate": "r", "object": "B", "p_value": "0.01"}); + let after = json!({"subject": "A", "predicate": "r", "object": "B", "p_value": "0.99", "effect_size": 1.5}); + let before_id = + uuid_for_json_object("TABLASSERT", &before, Some(&fields)).expect("object UUID"); + let after_id = + uuid_for_json_object("TABLASSERT", &after, Some(&fields)).expect("object UUID"); + assert_eq!(before_id, after_id); + } + + #[test] + fn declared_fields_still_track_declared_changes() { + let fields = vec![ + "subject".to_string(), + "predicate".to_string(), + "object".to_string(), + ]; + let before = json!({"subject": "A", "predicate": "r", "object": "B"}); + let after = json!({"subject": "A", "predicate": "r", "object": "C"}); + let before_id = + uuid_for_json_object("TABLASSERT", &before, Some(&fields)).expect("object UUID"); + let after_id = + uuid_for_json_object("TABLASSERT", &after, Some(&fields)).expect("object UUID"); + assert_ne!(before_id, after_id); + } + + #[test] + fn declared_field_order_does_not_matter() { + // WHY: `uuid_fields` is a set, not a sequence -- reordering the config list must + // not re-mint every id. + let forward = vec!["subject".to_string(), "object".to_string()]; + let backward = vec!["object".to_string(), "subject".to_string()]; + let value = json!({"subject": "A", "object": "B", "p_value": "0.01"}); + let forward_id = + uuid_for_json_object("TABLASSERT", &value, Some(&forward)).expect("object UUID"); + let backward_id = + uuid_for_json_object("TABLASSERT", &value, Some(&backward)).expect("object UUID"); + assert_eq!(forward_id, backward_id); + } + + #[test] + fn a_missing_declared_field_contributes_nothing() { + // WHY: an absent declared field must not silently alias onto a present one. + let fields = vec!["subject".to_string(), "negated".to_string()]; + let bare = json!({"subject": "A"}); + let present = json!({"subject": "A", "negated": true}); + let bare_id = + uuid_for_json_object("TABLASSERT", &bare, Some(&fields)).expect("object UUID"); + let present_id = + uuid_for_json_object("TABLASSERT", &present, Some(&fields)).expect("object UUID"); + assert_ne!(bare_id, present_id); + } + + #[test] + fn distinct_domains_separate_identical_records() { + // WHY: two graphs may legitimately assert the same triple. Namespacing by the + // graph's infores keeps their ids apart even under a narrow `uuid_fields`. + let fields = vec![ + "subject".to_string(), + "predicate".to_string(), + "object".to_string(), + ]; + let value = json!({"subject": "A", "predicate": "r", "object": "B"}); + let left = + uuid_for_json_object("infores:left-kg", &value, Some(&fields)).expect("object UUID"); + let right = + uuid_for_json_object("infores:right-kg", &value, Some(&fields)).expect("object UUID"); + assert_ne!(left, right); + } + + #[test] + fn uuid_for_json_object_rejects_non_objects() { + assert!(uuid_for_json_object("TABLASSERT", &json!(["a"]), None).is_none()); + assert!(uuid_for_json_object("TABLASSERT", &json!("a"), None).is_none()); + } + + #[test] + fn golden_vectors_pin_the_encoding() { + // WHY: nothing pinned an actual UUID value before, so the derivation could change + // silently and no assertion would fail. These vectors are the regression tripwire: + // if one moves, edge ids in every published graph moved with it, and that needs a + // MAJOR bump plus a CHANGELOG migration note. + let record = json!({ + "subject": "NCBITaxon:846", + "predicate": "biolink:affects", + "object": "FB:FBgn0002557", + "p_value": "9.6407e-03", + "sources": [{"resource_id": "infores:multiomicskg", "resource_role": "primary_knowledge_source"}], + }); + assert_eq!( + uuid_for_json_object("TABLASSERT", &record, None).expect("object UUID"), + "1fd95137-b2de-3963-9552-3c8b35d1f758" + ); + let fields = vec![ + "subject".to_string(), + "predicate".to_string(), + "object".to_string(), + ]; + assert_eq!( + uuid_for_json_object("infores:multiomicskg", &record, Some(&fields)) + .expect("object UUID"), + "7cf7352c-114f-3e7a-9e38-70ba678c958f" + ); + assert_eq!( + uuid_from_parts("domain", ["a", "b"]), + "1a8199fb-c8eb-381a-85a2-33ce009c506e" + ); + } } diff --git a/src/tablassert/cli.py b/src/tablassert/cli.py index 7d617c9..48797c4 100644 --- a/src/tablassert/cli.py +++ b/src/tablassert/cli.py @@ -250,7 +250,17 @@ def build_graph_pipeline( start(f"{g.name} · v{g.version}") # on_phase drives the phase tag (scan → normalize → write-nodes → write-edges → dedup → rig); # on_subgraph ticks the bar once per subgraph, so the total is len(subgraphs). - compile_graph(subgraphs, g.name, g.version, g.rig, section_sources if audit_sources else None, on_phase=sub_step, on_subgraph=advance) + compile_graph( + subgraphs, + g.name, + g.version, + g.rig, + section_sources if audit_sources else None, + on_phase=sub_step, + on_subgraph=advance, + uuid_fields=g.uuid_fields, + uuid_domain=g.uuid_namespace, + ) # Stage 7/7 (only with --qc): assert over the final NDJSON files. if qc: diff --git a/src/tablassert/errors.py b/src/tablassert/errors.py index af843dc..6436cac 100644 --- a/src/tablassert/errors.py +++ b/src/tablassert/errors.py @@ -42,6 +42,8 @@ "rig-terms-empty", "rig-legacy-keys", "rig-validation-failed", + "uuid-bad-fields", + "uuid-fields-not-a-key", ] diff --git a/src/tablassert/lib.py b/src/tablassert/lib.py index 0cf827b..53c717a 100644 --- a/src/tablassert/lib.py +++ b/src/tablassert/lib.py @@ -1553,15 +1553,22 @@ def upstream_resource_ids(repo: Repositories) -> list[str]: return [InformationResources.PUBMED.value] -def dedup_stream(p_in: Path, is_edges: bool) -> None: +def dedup_stream(p_in: Path, is_edges: bool, domain: str = "TABLASSERT", uuid_fields: list[str] | None = None) -> None: """Remove null values from and deduplicate an NDJSON stream. Args: p_in: Path to the input ``.ndjson.tmp`` file. is_edges: When True, also add UUIDs to edges via the Rust deduper. + domain: UUID namespace for derived edge ids (``Graph.uuid_namespace``). + Ignored for nodes, whose ids are CURIEs rather than derived hashes. + uuid_fields: Optional edge fields that constitute edge identity + (``Graph.uuid_fields``). ``None`` hashes the whole record. Notes: - Also adds UUIDs to edges. + Also adds UUIDs to edges. Edges deduplicate on their derived id, so the + output can never carry the same id twice: an exact repeat is collapsed, + while two genuinely different edges deriving one id abort the build with + ``uuid-fields-not-a-key`` rather than shipping a duplicate. Returns: ``None``; writes the deduplicated stream alongside ``p_in`` with no @@ -1572,7 +1579,7 @@ def dedup_stream(p_in: Path, is_edges: bool) -> None: if p_out.is_file(): p_out.unlink() - rs.dedup_ndjson(p_in, p_out, is_edges, "TABLASSERT") + rs.dedup_ndjson(p_in, p_out, is_edges, domain, uuid_fields) p_in.unlink() @@ -1719,6 +1726,8 @@ def _write_ndjson( rig: RIGConfig, section_sources: list[dict[str, object]] | None, on_phase: Callable[[str], None] | None = None, + domain: str = "TABLASSERT", + uuid_fields: list[str] | None = None, ) -> None: """Write, dedup, and RIG the KGX NDJSON outputs. @@ -1745,6 +1754,8 @@ def _write_ndjson( on_phase: Optional callback fired with ``"write-nodes"``, ``"write-edges"``, ``"dedup"`` and ``"rig"`` at each phase boundary, used to drive progress UX. + domain: UUID namespace for derived edge ids (``Graph.uuid_namespace``). + uuid_fields: Optional edge identity fields (``Graph.uuid_fields``). """ # Phase: write-nodes. Collection point: appending to output files. if on_phase is not None: @@ -1765,7 +1776,7 @@ def _write_ndjson( # Phase: dedup. if on_phase is not None: on_phase("dedup") - dedup_stream(edges_tmp, is_edges=True) + dedup_stream(edges_tmp, is_edges=True, domain=domain, uuid_fields=uuid_fields) # The deduper hashes the record WITH the literal `{edge_id}` placeholder still # in place, so edge ids stay deterministic regardless of this resolution pass. _resolve_edge_id_placeholders(edges_tmp.with_suffix("")) @@ -1786,6 +1797,8 @@ def compile_graph( section_sources: list[dict[str, object]] | None = None, on_phase: Callable[[str], None] | None = None, on_subgraph: Callable[[], None] | None = None, + uuid_fields: list[str] | None = None, + uuid_domain: str | None = None, ) -> None: """Aggregate subgraph parquets for NDJSON KGX export using a lazy scan. @@ -1804,6 +1817,13 @@ def compile_graph( ``write-edges`` / ``dedup`` / ``rig``), used to drive progress UX. on_subgraph: Optional callback fired once per processed subgraph, used to tick the progress bar. + uuid_fields: Optional edge fields that constitute edge identity + (``Graph.uuid_fields``). ``None`` hashes the whole edge record, so any + change to any field re-mints the id. + uuid_domain: Optional explicit UUID namespace. Defaults to the graph's + infores when ``uuid_fields`` is set -- narrowing the hash inputs + removes the accidental cross-graph uniqueness that full-record hashing + provided -- and to ``TABLASSERT`` otherwise. Returns: ``None``; writes ``_.nodes.ndjson``, @@ -1813,6 +1833,8 @@ def compile_graph( Raises: TablassertError: With code ``rig-validation-failed`` when the generated RIG fails its built-in audit (nothing is then written). + RuntimeError: Tagged ``uuid-fields-not-a-key`` when two genuinely + different edges derive one id under the declared ``uuid_fields``. """ rig_cfg: RIGConfig = rig if isinstance(rig, RIGConfig) else RIGConfig.model_validate(rig) out_dir: Path = Path(rig_cfg.artifact_base_path) @@ -1827,10 +1849,12 @@ def compile_graph( if n.exists(): n.unlink() + domain: str = uuid_domain or (rig_cfg.source_info.infores_id if uuid_fields else "TABLASSERT") + subnodes: list[pl.LazyFrame] subedges: list[pl.LazyFrame] subnodes, subedges = _collect_subframes(subgraphs, on_phase, on_subgraph, rig_cfg.source_info.infores_id) - _write_ndjson(subnodes, subedges, n, e, name, version, rig_cfg, section_sources, on_phase) + _write_ndjson(subnodes, subedges, n, e, name, version, rig_cfg, section_sources, on_phase, domain, uuid_fields) def resolve_many( diff --git a/src/tablassert/models.py b/src/tablassert/models.py index 0ed82cd..ca96b2b 100644 --- a/src/tablassert/models.py +++ b/src/tablassert/models.py @@ -1122,6 +1122,54 @@ class Graph(TablaBase): tables: list[Path] = Field(..., description="Paths to table YAML files included in this graph.", examples=[["tables/tutorial-table.yaml"]]) fullmap: Path = Field(..., description="Base fullmap directory or fullmap redb file for entity resolution.", examples=[".fullmap"]) rig: RIGConfig = Field(..., description="Resource Ingest Guide metadata emitted as _.RIG.yaml.") + uuid_fields: list[str] | None = Field( + default=None, + description="Edge fields that constitute edge identity; only these feed the derived edge `id`, so an attribute-only change leaves it alone. Unset hashes the whole record.", + examples=[["subject", "predicate", "object", "publications", "has_supporting_studies"]], + ) + uuid_domain: str | None = Field( + default=None, + description="Explicit UUID namespace. Defaults to `rig.source_info.infores_id` when `uuid_fields` is set, `TABLASSERT` otherwise. Set it only when graphs must deliberately share an id space.", + examples=["infores:multiomicskg"], + ) + + @model_validator(mode="after") + def validate_uuid_fields(self: Self) -> Self: + """Reject a `uuid_fields` list that cannot identify an edge. + + Every entry must be a real emittable edge field, or the id would silently derive + from nothing and every edge in the graph would collide. `id` itself is rejected + because it is the value being derived. Casing follows `Annotation.clean_annotation` + so `Subject` and `subject` both work and mixed-case Biolink slots survive. + """ + if self.uuid_fields is None: + return self + if not self.uuid_fields: + raise TablassertValidationError( + "`uuid_fields` was given as an empty list. Omit the key entirely to hash the whole record, or name the fields that identify an edge.", + code="uuid-bad-fields", + ) + canonical: list[str] = [] + for field in self.uuid_fields: + lowered: str = field.strip().lower() + canonical.append(next((allowed for allowed in ALLOWED_EDGE_FIELDS if allowed.lower() == lowered), lowered)) + if len(set(canonical)) != len(canonical): + duplicated: str = ", ".join(sorted({f for f in canonical if canonical.count(f) > 1})) + raise TablassertValidationError(f"`uuid_fields` repeats: {duplicated}. Each field may appear once.", code="uuid-bad-fields") + if "id" in canonical: + raise TablassertValidationError("`uuid_fields` may not contain `id`: the edge id is what these fields derive.", code="uuid-bad-fields") + unknown: list[str] = sorted(f for f in canonical if f not in ALLOWED_EDGE_FIELDS) + if unknown: + raise TablassertValidationError( + f"`uuid_fields` names fields that are never emitted on an edge: {', '.join(unknown)}. " + "An edge id derived from an absent field would be identical for every edge sharing the " + "remaining fields. Unknown columns fold into `supporting_text`; name that instead if you " + "meant to include them.", + code="uuid-bad-fields", + ) + # Persist the canonicalized spellings so the Rust deduper matches record keys exactly. + object.__setattr__(self, "uuid_fields", canonical) + return self @model_validator(mode="before") @classmethod @@ -1148,3 +1196,17 @@ def reject_legacy_rig_keys(cls, data: Any) -> Any: def infores_id(self) -> str: """Graph-level primary knowledge source infores (from ``rig.source_info.infores_id``).""" return self.rig.source_info.infores_id + + @property + def uuid_namespace(self) -> str: + """UUID domain for this graph's edge ids. + + Hashing only ``uuid_fields`` removes the accidental cross-graph uniqueness that + full-record hashing provided: two graphs asserting the same triple from the same + publication would derive the same id. Namespacing on the graph's own infores makes + that structurally impossible. With no ``uuid_fields`` the domain stays the historic + ``TABLASSERT`` constant, so default-configured graphs keep deriving as before. + """ + if self.uuid_domain is not None: + return self.uuid_domain + return self.infores_id if self.uuid_fields else "TABLASSERT" diff --git a/src/tablassert/rs.pyi b/src/tablassert/rs.pyi index 863abf6..c6e58c6 100644 --- a/src/tablassert/rs.pyi +++ b/src/tablassert/rs.pyi @@ -7,7 +7,7 @@ from typing import Any def build_fullmap_db( output: Path, classes: list[Path], synonyms: list[Path], threads: int | None = None, progress: Callable[[int, int, int, str], None] | None = None ) -> None: ... -def dedup_ndjson(input: Path, output: Path, is_edges: bool, domain: str | None = None) -> None: ... +def dedup_ndjson(input: Path, output: Path, is_edges: bool, domain: str | None = None, uuid_fields: list[str] | None = None) -> None: ... def extract_prebuilt_fullmap(archive: Path, output: Path, progress: Callable[[str], None] | None = None) -> None: ... def fullmap_source_version() -> str: ... def hydrate_categories(db: Path) -> list[str]: ... diff --git a/src/tablassert/study.py b/src/tablassert/study.py index 6ad1a5d..4a40c28 100644 --- a/src/tablassert/study.py +++ b/src/tablassert/study.py @@ -16,6 +16,7 @@ "whitespace-values": "values with leading/trailing whitespace", "empty-or-null-values": "null or empty values", "duplicate-node-ids": "duplicate node ids", + "duplicate-edge-ids": "duplicate edge ids", "unnamed-nodes": "nodes with no name or an empty name", "unidentified-nodes": "nodes with no id or an empty id", "incomplete-edges": "edges missing subject, predicate, or object", @@ -46,6 +47,9 @@ class _FileScan: """Accumulated facts from one streamed pass over an NDJSON file.""" ids: set[str] + #: Edge ``id`` values already seen. Kept apart from ``ids``, which on an edge + #: scan holds the subject/object CURIEs the declared/isolated cross-check needs. + edge_ids: set[str] duplicate_ids: Counter[str] whitespace: Counter[str] empty_null: Counter[str] @@ -94,7 +98,7 @@ def _scan_ndjson(path: Path, *, edge: bool) -> _FileScan: A :class:`_FileScan`; ``missing`` is set (and nothing else) when the file does not exist, so a typo'd path can never read as a clean pass. """ - scan: _FileScan = _FileScan(set(), Counter(), Counter(), Counter(), Counter(), Counter(), Counter(), 0, not path.is_file(), path) + scan: _FileScan = _FileScan(set(), set(), Counter(), Counter(), Counter(), Counter(), Counter(), Counter(), 0, not path.is_file(), path) if scan.missing: return scan with path.open(encoding="utf-8") as handle: @@ -127,6 +131,15 @@ def _scan_ndjson(path: Path, *, edge: bool) -> _FileScan: if _is_empty_or_null(value): scan.empty_null[key] += 1 if edge: + # The deduper keys edges on their derived id, so a duplicate here means + # something bypassed it (a hand-built file, or two files concatenated). + # KGX requires edge ids to be unique, so assert it independently rather + # than trusting the writer -- the symmetric check to duplicate-node-ids. + edge_id: object = record.get("id") + if isinstance(edge_id, str): + if edge_id in scan.edge_ids: + scan.duplicate_ids[edge_id] += 1 + scan.edge_ids.add(edge_id) for role in ("subject", "object"): ident: object = record.get(role) if isinstance(ident, str): @@ -176,7 +189,8 @@ def _scan_ndjson(path: Path, *, edge: bool) -> _FileScan: def study_kgx(nodes_path: Path, edges_path: Path, *, example_limit: int = 10) -> list[StudyViolation]: """Assert over the final KGX NDJSON files, in the spirit of studyKGtsvs.pl. - Streams both files once each and checks: duplicate node ids, nodes with no + Streams both files once each and checks: duplicate node ids, duplicate edge + ids, nodes with no name or an empty name, nodes with no id or an empty id, edges missing any of ``subject``/``predicate``/``object``, nodes referenced by edges but never declared (``undeclared``), declared nodes participating in no edge @@ -212,6 +226,9 @@ def study_kgx(nodes_path: Path, edges_path: Path, *, example_limit: int = 10) -> if nodes.duplicate_ids: examples = [ident for ident, _ in nodes.duplicate_ids.most_common(example_limit)] violations.append(StudyViolation("duplicate-node-ids", "nodes", len(nodes.duplicate_ids), examples)) + if edges.duplicate_ids: + examples = [ident for ident, _ in edges.duplicate_ids.most_common(example_limit)] + violations.append(StudyViolation("duplicate-edge-ids", "edges", len(edges.duplicate_ids), examples)) if nodes.unnamed: examples = [ident for ident, _ in nodes.unnamed.most_common(example_limit)] violations.append(StudyViolation("unnamed-nodes", "nodes", sum(nodes.unnamed.values()), examples)) From 3af323c6b4303e7c5f5fc00320a794e78e1f5689 Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Tue, 25 Aug 2026 13:17:48 -0700 Subject: [PATCH 2/6] test: golden UUID vectors and uuid_fields coverage Nothing pinned an actual UUID value, so the derivation could change silently and no assertion would fail. Three golden vectors now fix the full-record, declared-field, and raw-parts derivations; if one moves, edge ids in every published graph moved with it. Also covers: nested key-order stability, array order still mattering, `false` staying distinguishable, declared fields ignoring undeclared edits while tracking declared ones, declared-field order not mattering, cross-domain separation, the not-a-key abort and its diagnostic, exact repeats still collapsing, key-order-only duplicates no longer shipping two lines, Graph validation of uuid_fields, and the duplicate-edge-ids study assertion. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NuGRZ2fn3gE5zkUrv56Jqh --- tests/test_lib.py | 56 ++++++++++++++++++++++++++++++ tests/test_models.py | 81 ++++++++++++++++++++++++++++++++++++++++++++ tests/test_study.py | 33 ++++++++++++++++++ 3 files changed, 170 insertions(+) diff --git a/tests/test_lib.py b/tests/test_lib.py index fa0ef0c..66bd573 100644 --- a/tests/test_lib.py +++ b/tests/test_lib.py @@ -3520,3 +3520,59 @@ def test_inline_supporting_study_metadata_without_row_has_no_result_fallback() - out: pl.DataFrame = inline_supporting_study(lf, "table", "data.tsv", False).collect() study: dict[str, Any] = out["has_supporting_studies"].to_list()[0]["table"] assert study == {"id": "table", "name": "data.tsv", "study_size": 9} + + +def test_dedup_stream_edges_declared_uuid_fields_survive_attribute_edits(tmp_path: Path) -> None: + """declared `uuid_fields` hold the edge id still when only an attribute changes. + + This is the point of the feature: a corrected p_value must leave the id alone so + downstream sees one edge updated, not one retired and one created. + """ + import json + + fields: list[str] = ["subject", "object", "predicate"] + + def build(name: str, body: str) -> str: + p_in: Path = tmp_path / f"{name}.ndjson.tmp" + p_in.write_text(body) + lib.dedup_stream(p_in, is_edges=True, domain="infores:test-kg", uuid_fields=fields) + line: str = (tmp_path / f"{name}.ndjson").read_text().strip() + return json.loads(line)["id"] + + before: str = build("before", '{"subject":"A","object":"B","predicate":"r","p_value":"0.01"}\n') + after: str = build("after", '{"subject":"A","object":"B","predicate":"r","p_value":"0.99","effect_size":1.5}\n') + assert before == after + # ...and the whole-record default still drifts, so the opt-in is what changed things. + drifted: str = build("drifted", '{"subject":"A","object":"B","predicate":"r","p_value":"0.99","effect_size":1.5}\n') + p_in: Path = tmp_path / "plain.ndjson.tmp" + p_in.write_text('{"subject":"A","object":"B","predicate":"r","p_value":"0.01"}\n') + lib.dedup_stream(p_in, is_edges=True) + plain: str = json.loads((tmp_path / "plain.ndjson").read_text().strip())["id"] + assert plain != drifted + + +def test_dedup_stream_edges_reject_uuid_fields_that_are_not_a_key(tmp_path: Path) -> None: + """two different edges deriving one id abort the build instead of shipping a duplicate.""" + p_in: Path = tmp_path / "edges.ndjson.tmp" + p_in.write_text('{"subject":"A","object":"B","predicate":"r","p_value":"0.01"}\n{"subject":"A","object":"B","predicate":"r","p_value":"0.99"}\n') + + with pytest.raises(RuntimeError) as exc_info: + lib.dedup_stream(p_in, is_edges=True, domain="infores:test-kg", uuid_fields=["subject", "object", "predicate"]) + + message: str = str(exc_info.value) + assert "uuid-fields-not-a-key" in message + # The diagnostic must name the field that would disambiguate, or it is unactionable. + assert "p_value" in message + + +def test_dedup_stream_edges_domain_separates_graphs(tmp_path: Path) -> None: + """the same triple in two graphs derives two ids, so a narrow key set stays safe.""" + import json + + def build(name: str, domain: str) -> str: + p_in: Path = tmp_path / f"{name}.ndjson.tmp" + p_in.write_text('{"subject":"A","object":"B","predicate":"r"}\n') + lib.dedup_stream(p_in, is_edges=True, domain=domain, uuid_fields=["subject", "object", "predicate"]) + return json.loads((tmp_path / f"{name}.ndjson").read_text().strip())["id"] + + assert build("left", "infores:left-kg") != build("right", "infores:right-kg") diff --git a/tests/test_models.py b/tests/test_models.py index 8bae1d5..bd4f7c8 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1137,3 +1137,84 @@ def test_deprecated_hook_is_noop_for_non_dict_input(recwarn: pytest.WarningsReco with pytest.raises(ValidationError): Section.model_validate(["not", "a", "mapping"]) assert [w for w in recwarn if issubclass(w.category, UserWarning)] == [] + + +def test_graph_uuid_fields_default_to_the_historic_namespace(rig_factory: Any) -> None: + """an unset `uuid_fields` keeps the whole-record hash under the TABLASSERT domain.""" + graph: Graph = Graph(name="TEST", version="1.0.0", tables=[Path("./table.yaml")], fullmap=Path("./fullmap"), rig=rig_factory()) + assert graph.uuid_fields is None + assert graph.uuid_domain is None + # Byte-compatible with pre-16.0.0 output: default-configured graphs derive as before. + assert graph.uuid_namespace == "TABLASSERT" + + +def test_graph_uuid_fields_namespace_on_the_graph_infores(rig_factory: Any) -> None: + """declaring `uuid_fields` moves the domain onto the graph's own infores. + + Hashing a subset removes the accidental cross-graph uniqueness full-record hashing + provided -- two graphs asserting one triple would otherwise mint one id. + """ + graph: Graph = Graph( + name="TEST", + version="1.0.0", + tables=[Path("./table.yaml")], + fullmap=Path("./fullmap"), + rig=rig_factory(), + uuid_fields=["subject", "predicate", "object"], + ) + assert graph.uuid_namespace == "infores:test-kg" + + +def test_graph_uuid_domain_overrides_the_derived_namespace(rig_factory: Any) -> None: + """an explicit `uuid_domain` wins, so sharded graphs can share one id space.""" + base: dict[str, Any] = {"name": "TEST", "version": "1.0.0", "tables": [Path("./table.yaml")], "fullmap": Path("./fullmap"), "rig": rig_factory()} + with_fields: Graph = Graph.model_validate({**base, "uuid_fields": ["subject"], "uuid_domain": "infores:shared"}) + assert with_fields.uuid_namespace == "infores:shared" + # Meaningful on its own: renamespace a full-record hash without narrowing it. + without_fields: Graph = Graph.model_validate({**base, "uuid_domain": "infores:shared"}) + assert without_fields.uuid_namespace == "infores:shared" + + +def test_graph_uuid_fields_canonicalize_casing(rig_factory: Any) -> None: + """any casing canonicalizes onto the allow-listed spelling, as annotations do.""" + graph: Graph = Graph.model_validate( + { + "name": "TEST", + "version": "1.0.0", + "tables": [Path("./table.yaml")], + "fullmap": Path("./fullmap"), + "rig": rig_factory(), + "uuid_fields": ["Subject", "PREDICATE", " object "], + } + ) + # The Rust deduper matches record keys exactly, so the stored spellings must be canonical. + assert graph.uuid_fields == ["subject", "predicate", "object"] + + +@pytest.mark.parametrize( + ("uuid_fields", "reason"), + [ + ([], "empty"), + (["subject", "subject"], "repeats"), + (["subject", "id"], "may not contain `id`"), + (["subject", "not_a_real_field"], "never emitted"), + ], +) +def test_graph_rejects_uuid_fields_that_cannot_identify_an_edge(rig_factory: Any, uuid_fields: list[str], reason: str) -> None: + """a `uuid_fields` list that cannot be a key is rejected at config time. + + Each of these would derive an id from nothing, from `id` itself, or from a field no + edge carries -- collapsing every edge in the graph onto one identifier. + """ + data: dict[str, Any] = { + "name": "TEST", + "version": "1.0.0", + "tables": [Path("./table.yaml")], + "fullmap": Path("./fullmap"), + "rig": rig_factory(), + "uuid_fields": uuid_fields, + } + with pytest.raises(ValidationError) as exc_info: + Graph.model_validate(data) + assert "uuid-bad-fields" in str(exc_info.value) + assert reason in str(exc_info.value) diff --git a/tests/test_study.py b/tests/test_study.py index 40ae30a..9e19b70 100644 --- a/tests/test_study.py +++ b/tests/test_study.py @@ -397,3 +397,36 @@ def test_study_final_ndjson_passes_clean(monkeypatch: Any, tmp_path: Path) -> No monkeypatch.chdir(tmp_path) monkeypatch.setattr(study, "study_kgx", lambda *args: []) cli.study_final_ndjson("g", "1", tmp_path) + + +def test_duplicate_edge_ids(tmp_path: Path) -> None: + """an edge id appearing on more than one line fails the duplicate assertion. + + The deduper keys edges on their derived id, so this can only fire when something + bypassed it -- a hand-built file, or two builds concatenated. KGX requires edge ids + to be unique, so the study asserts it independently of the writer. + """ + nodes, _ = _clean(tmp_path) + edges: Path = _write_ndjson( + tmp_path / "e.ndjson", + _records( + {"id": "dup-id", "subject": "HGNC:5", "object": "HGNC:6", "predicate": "biolink:related_to"}, + {"id": "dup-id", "subject": "HGNC:5", "object": "HGNC:6", "predicate": "biolink:affects"}, + ), + ) + checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges)) + assert checks["duplicate-edge-ids"].count == 1 + assert checks["duplicate-edge-ids"].examples == ["dup-id"] + + +def test_distinct_edge_ids_pass(tmp_path: Path) -> None: + """distinct edge ids raise no duplicate violation.""" + nodes, _ = _clean(tmp_path) + edges: Path = _write_ndjson( + tmp_path / "e.ndjson", + _records( + {"id": "a", "subject": "HGNC:5", "object": "HGNC:6", "predicate": "biolink:related_to"}, + {"id": "b", "subject": "HGNC:6", "object": "HGNC:5", "predicate": "biolink:related_to"}, + ), + ) + assert "duplicate-edge-ids" not in _checks(study.study_kgx(nodes, edges)) From 57802ee885b75dc2b404b296b81270a4a029c646 Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Tue, 25 Aug 2026 13:28:05 -0700 Subject: [PATCH 3/6] docs: document uuid_fields and correct the stale namespace_uuid page docs/api/utils.md documented a "\t".join(values) encoding removed some releases ago (the join has been length-prefixed for collision-safety since) and claimed edge ids covered only "subject, predicate, object, qualifiers, and publication" -- which was never true, and is precisely what uuid_fields now makes achievable. Both corrected, with the canonicalization, namespacing and uniqueness rules documented alongside. The graph config reference gains a Stable edge ids section: how to pick a field set, why the set must be a key, how to read the uuid-fields-not-a-key failure, when to reach for uuid_domain, and the one-time id churn that adopting uuid_fields implies. The guidance is empirical, not aspirational: the field set this page recommends as a starting point is NOT a key for MultiomicsKG 3.0.0 (2,476 collisions in 1.27M edges, all pairs that differed only in the NLP level recorded in supporting_text after two raw strings resolved onto one CURIE), so the page says to build and let the failure name what is missing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NuGRZ2fn3gE5zkUrv56Jqh --- CHANGELOG.md | 25 +++++++++ docs/api/utils.md | 66 +++++++++++++++++------ docs/configuration/graph.md | 101 ++++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b82354..a485d9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,31 @@ All notable changes to this project are documented in this file. +## 16.0.0 - 2026-08-25 + +### Breaking Changes +- **Edge ids are canonicalized before hashing, so every existing edge id changes once.** The derivation sorted only *top-level* keys: nested values reached the digest through `Value::to_string`, which under serde_json's `preserve_order` emits insertion order. Every edge carries a `sources` array, so a polars struct-field reordering — or a Biolink release that shifts a nested slot — silently re-minted ids across a whole graph while nothing about the assertion had changed. Canonicalization now recurses (object keys sorted at every depth; array order preserved, because it is semantic), which makes ids robust to that churn at the cost of one migration. Relatedly, `false` is now hashed as `"false"` instead of being dropped along with its key: `strip_nulls` deliberately keeps `false` (`negated: false` is a meaningful Biolink value), so dropping it made `{subject, negated: false}` and `{subject}` — two distinct records — derive the *same* id. No shipped graph is affected by that second fix (the 1.27M-edge MultiomicsKG 3.0.0 build contains zero `false` values); the canonicalization fix moves every id. + + **Migration:** readers pinning Tablassert edge ids must re-key against the rebuilt graph. There is no mapping from old id to new — the old value was a hash of a serialization detail. This is a one-time event: adopt `uuid_fields` (below) in the same rebuild and ids stop moving for attribute changes thereafter. + +### Added +- **`uuid_fields`: a graph config declares which edge fields constitute edge identity.** Edge ids were UUIDv3 hashes of the *entire* emitted record, so every field was an identity field — a corrected `p_value`, a bumped `subject_nlp_level` inside `supporting_text`, a changed `sources[].source_record_urls`, or a reordered source row (the `row:` inside `has_supporting_studies`) each minted a brand-new id, and downstream Translator consumers saw a new edge rather than the same edge with updated attributes. `Graph` gained an optional `uuid_fields` list; when set, only those top-level edge keys feed the hash, and everything else is free to change. Entries canonicalize onto their allow-listed spelling exactly as annotations do, and a list that cannot be a key — empty, repeating, containing `id`, or naming a field no edge emits — is rejected at config time as `uuid-bad-fields`. Leaving `uuid_fields` unset preserves the whole-record hash, so the feature is strictly opt-in. Measured on MultiomicsKG 3.0.0 (1,265,355 edges): re-analysing `p_value` and `effect_size` on every row moved **930,081 ids (73%)** under the whole-record hash and **none at all** under a declared `uuid_fields`. + +- **The UUID namespace derives from the graph's infores when `uuid_fields` is declared.** Full-record hashing gave cross-graph uniqueness by accident: two graphs asserting the same triple from the same publication were separated by their differing `sources` and `supporting_text`. A narrow field set removes that accident, so the domain moves from the `TABLASSERT` constant onto `rig.source_info.infores_id`, making the separation structural instead. A new optional `uuid_domain` overrides it for the opposite case — graphs that must deliberately *share* an id space, such as a KG compiled in shards or renamed across versions while keeping its published ids. Both default to the historic constant when `uuid_fields` is unset. + +- **A `duplicate-edge-ids` assertion in `--qc` study output.** `study_kgx` checked `duplicate-node-ids` but had no edge equivalent, even though KGX requires edge ids to be unique. The symmetric check now streams the final edges file and reports duplicates independently of the writer, catching an id collision from any source. + +### Fixed +- **Two edges deriving one id abort the build instead of shipping a duplicate.** Dedup keyed on the full canonical record bytes, which was safe only because the id was a pure function of those bytes. That equivalence breaks the moment the hash covers a subset, and it was already imperfect: `stable_json_bytes` never sorted despite its name, so two logically identical records arriving with different key order derived one id, produced different bytes, and *both* shipped. Edges now dedup on the derived id itself — an exact repeat collapses as before, while two genuinely different edges claiming one id raise `uuid-fields-not-a-key` with the id, the fields that differ (recovered by re-reading the partial output, on the failure path only), and the declared field list. The nodes path is unchanged: node ids are CURIEs, not derived hashes. + +### Performance +- **Edge dedup holds 24 bytes per edge instead of a full copy of every record: 2469 MB -> 100 MB peak on a 1.27M-edge graph.** `record_if_new` retained the complete bytes of every unique record to make suppression byte-exact — roughly 800 bytes per edge, and ~2.4 GB resident on MultiomicsKG 3.0.0 purely for the dedup set. The edge path now keys a `FxHashMap<[u8; 16], u64>` on the raw UUID bytes with an xxh64 of the id-free record as the value: a measured **25x** reduction that also removes the per-record heap allocation, and on the same graph took the dedup pass from 477s to 37s (the old path's allocation churn dominated; the margin will be smaller on machines under less memory pressure). The content hash is computed *before* the id is inserted, so no record is cloned to strip it back out, and a declared `uuid_fields` list additionally cuts the number of keys the hasher visits per edge. + +### Changed +- **`docs/api/utils.md` now describes the derivation that actually runs.** The page documented a `"\t".join(values)` encoding removed some releases ago (the join has been length-prefixed for collision-safety) and claimed edge ids covered only "subject, predicate, object, qualifiers, and publication" — which was never true, and is precisely what `uuid_fields` now makes achievable. Both are corrected, and the canonicalization, namespacing, and uniqueness rules are documented alongside. + +- **Golden UUID vectors pin the derivation.** No test asserted a concrete UUID value, so any change to the encoding could land silently. Three vectors now fix the full-record, declared-field, and raw-parts derivations; if one moves, edge ids in every published graph moved with it. + ## 15.1.0 - 2026-08-25 ### Added diff --git a/docs/api/utils.md b/docs/api/utils.md index 1eb2918..c6ff0d5 100644 --- a/docs/api/utils.md +++ b/docs/api/utils.md @@ -54,7 +54,7 @@ Domain string used to create the namespace UUID. The default domain used interna **`values: list[str]`** -The values to incorporate into the UUID. Empty/None entries are filtered out, the rest are joined with tabs (`"\t"`) and hashed within the domain namespace. +The values to incorporate into the UUID. Empty entries are dropped; each surviving value is length-prefixed as `:` and concatenated, then hashed within the domain namespace. ### Return Value @@ -68,38 +68,70 @@ Returns a string representation of a UUID v3: `"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxx domain_uuid = uuid3(UUID("00000000-0000-0000-0000-000000000000"), domain) ``` -**Step 2:** Join the (filtered) values with tabs and hash within the namespace: +**Step 2:** Length-prefix each value and hash the concatenation within that namespace: ```python -return str(uuid3(domain_uuid, "\t".join(values))) +joined = "".join(f"{len(v.encode())}:{v}" for v in values) +return str(uuid3(domain_uuid, joined)) ``` +The length prefix is what makes the encoding **injective**. A plain separator join is ambiguous +whenever a value contains the separator: `["a", "x\tb", "y"]` and `["a", "x", "b", "y"]` both join +to `"a\tx\tb\ty"`, so two different inputs would derive the same UUID. + ### Deterministic Behavior Same inputs always produce the same UUID; different inputs (or different domains) produce different UUIDs. -### Use Case: KGX Edge IDs +## Edge IDs -Tablassert uses this function to generate reproducible edge identifiers from the edge's subject, predicate, object, qualifiers, and publication: +Edge ids are **not** built by calling `namespace_uuid()` from Python. They are assigned inside the +Rust deduper (`dedup_ndjson`) as each edge is written, from the record itself. -```python -from tablassert.rs import namespace_uuid +### Which fields feed the id + +By default, **every field** of the emitted edge. That makes the id maximally sensitive: a corrected +`p_value`, a new `supporting_text` entry, a reordered source row, or a Biolink release that renames +a slot all mint a brand-new id, and downstream consumers see a new edge rather than an updated one. -edge_id = namespace_uuid( - "TABLASSERT", - ["HGNC:11998", "biolink:associated_with", "MONDO:0005148", "PMC11708054"], -) -# e.g. "2cfea591-0f8f-33af-a7df-03da531d3359" +A graph config can instead declare which fields constitute edge *identity*: + +```yaml +# graph.yaml +uuid_fields: [subject, predicate, object, publications, has_supporting_studies] ``` -**Benefits:** -- **Reproducible:** the same edge always gets the same ID across runs -- **Collision-resistant:** MD5 hashing makes collisions extremely unlikely -- **Traceable:** the ID incorporates the edge components (subject, predicate, object, provenance) +Only those fields then feed the hash, so attribute-only changes leave the id alone. See +[Graph Configuration](../configuration/graph.md#stable-edge-ids) for how to choose a field set. + +### Canonicalization + +Before hashing, the record is normalized so that only *meaning* reaches the digest: + +- object keys are sorted, recursively, so insertion order never changes the id; +- array order is preserved, because it is semantic; +- each key and its value are fed as **separate** parts, so the key/value boundary cannot shift to + create a collision (`{"a": "b=c"}` and `{"a=b": "c"}` stay distinct); +- `null` and empty values drop out, key included; `false` is hashed as `"false"`, because + `negated: false` is a meaningful Biolink value. + +### Namespace + +The domain defaults to `"TABLASSERT"`. When `uuid_fields` is declared it becomes the graph's +`rig.source_info.infores_id`, so two graphs asserting the same triple can never mint the same id — +the uniqueness that full-record hashing provided by accident becomes structural. `uuid_domain` +overrides it for graphs that must deliberately share an id space. + +### Uniqueness + +Edges deduplicate on their derived id, so an output file can never carry the same id twice. An +exact repeat collapses; two genuinely different edges deriving one id abort the build with +`uuid-fields-not-a-key`, naming the fields that would disambiguate them. ### KGX Compliance -NCATS Translator KGX requires edge IDs to be globally unique and, where possible, deterministic. `namespace_uuid()` satisfies both: UUID v3 with domain namespacing yields unique, reproducible identifiers. +NCATS Translator KGX requires edge IDs to be globally unique and, where possible, deterministic. +UUID v3 with domain namespacing satisfies both. ## Next Steps diff --git a/docs/configuration/graph.md b/docs/configuration/graph.md index 8f8e89d..e3fabcf 100644 --- a/docs/configuration/graph.md +++ b/docs/configuration/graph.md @@ -27,6 +27,107 @@ QC auditing and verbose logging are controlled at build time via the `build-kg - The legacy top-level RIG fields (`description`, `contributions`, `ui_explanation`, `infores`) are **rejected** with a migration pointer; they now live under `rig:`. +### Optional Fields + +| Field | Type | Description | +|-------|------|-------------| +| `uuid_fields` | List[str] | Edge fields that constitute edge identity. Only these feed the derived edge `id` (see [Stable edge ids](#stable-edge-ids)) | +| `uuid_domain` | String | Explicit UUID namespace. Defaults to `rig.source_info.infores_id` when `uuid_fields` is set, `TABLASSERT` otherwise | + +## Stable edge ids + +Every edge gets a deterministic `id`: a UUID v3 derived from the edge itself. By default it is +derived from the **whole record**, which makes it maximally brittle — a corrected `p_value`, a new +`supporting_text` entry, a reordered source row, or a Biolink release that renames a slot all mint a +brand-new id. Downstream Translator consumers then see a new edge where they should see the same +edge with updated attributes. + +`uuid_fields` fixes that by naming the fields that actually identify an edge: + +```yaml +uuid_fields: [subject, predicate, object, publications, has_supporting_studies] +``` + +Everything else is then free to change without moving the id. + +### Choosing a field set + +Start from what identifies an assertion, and what each entry buys you: + +- **`subject` / `predicate` / `object`** — the assertion itself. +- **`publications`** — the evidence it rests on. +- **`has_supporting_studies`** — carries `has_study_results[].id` (`row:`), the **row + discriminator**. Include it whenever one table contributes several rows that share a subject, + predicate, and object. It also carries the `study_*` metadata, but those come from per-section + config and are far more stable than `p_value` or `effect_size`. + +Add qualifiers (`object_direction_qualifier`, `anatomical_context_qualifier`, …) when they +*distinguish* assertions rather than merely describe them. + +Leave out anything that is an observation *about* the edge rather than the edge's identity: +`p_value`, `effect_size`, `effect_type`, `original_subject` / `original_object`, `supporting_text`, +`sources`, `category`, `knowledge_level`, `agent_type`. + +**Then build, and let the failure tell you what is missing.** That list is a starting point, not an +answer — whether it is a key depends on your data, and the only way to find out is to run it. On a +real 1.27M-edge graph the set above left 2,476 collisions (0.2% of edges): pairs whose subject, +predicate, object, publication and row were identical, differing only in the NLP level recorded in +`supporting_text` because two raw strings had resolved onto the same CURIE. Adding +`supporting_text` made it a key. + +Expect to iterate once or twice. Each failure names the id, the fields that differ, and one +offending edge, so each round is mechanical. + +### What it buys + +On that same 1.27M-edge graph, re-analysing the statistics (new `p_value` and `effect_size` on +every row) and rebuilding: + +| | edge ids that changed | +|---|---| +| no `uuid_fields` (whole-record hash) | 930,081 of 1,265,355 — **73%** | +| `uuid_fields` declared | 0 of 1,265,355 — **none** | + +### The field set must be a key + +Narrowing what feeds the hash means two different edges can derive the same id. Tablassert refuses +to ship duplicate edge ids, so that is a build failure, not a silent collapse: + +``` +uuid-fields-not-a-key: declared uuid_fields are not a key for this graph. + id 83ade536-9b07-34ec-a2f9-abf0fb5b6a2f is claimed by 2 different edges. + they differ in: effect_size, p_value + declared uuid_fields: subject, predicate, object + offending edge: subject=A predicate=r object=B +Add a discriminating field to `uuid_fields` (...). +``` + +The fix is whatever the message names: add the qualifier that separates them, +`has_supporting_studies` for the source row, or the statistic that genuinely differs. Two rows with +an identical subject, predicate, and object that differ only in `p_value` are exactly this case — +and were previously producing two ids for what config claimed was one assertion. + +An exact duplicate is *not* a violation: identical edges collapse, as they always have. + +### Namespacing + +Because a narrow field set no longer distinguishes graphs by accident, declaring `uuid_fields` +moves the UUID namespace onto the graph's own `rig.source_info.infores_id`. Two graphs asserting +the same triple from the same publication then still derive different ids, structurally. + +Set `uuid_domain` only when graphs must deliberately **share** an id space — a KG compiled in +shards, or one renamed across versions that has to keep its published ids: + +```yaml +uuid_domain: infores:multiomicskg +``` + +### Migration + +Adding `uuid_fields` to an existing graph **changes every edge id in it, once**. That is the cost of +switching identity models; ids are stable from then on. Plan it as a deliberate version bump and +tell your consumers. + ### The `rig:` section The `rig:` section carries every human-authored RIG fact. Its shape mirrors the released [RIG schema](https://github.com/biolink/resource-ingest-guide-schema), so the generated `.RIG.yaml` is always schema-shaped. The generator derives only mechanical facts from the build (generated artifact file entries, observed edge/node type summaries) and **validates the complete document before writing anything**, so a build never leaves behind an invalid or incomplete RIG. From 4a8d482c6bc35c76b73ad1f8bf3be25d62507438 Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Tue, 25 Aug 2026 13:28:05 -0700 Subject: [PATCH 4/6] chore(release): 16.0.0 Major: recursive canonicalization of nested values changes every existing edge id once. Every edge carries a `sources` array whose sorted key order differs from its insertion order, so the derivation moves for all of them. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NuGRZ2fn3gE5zkUrv56Jqh --- CITATION.cff | 2 +- pyproject.toml | 2 +- uv.lock | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CITATION.cff b/CITATION.cff index 06e6200..8e5807f 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -2,7 +2,7 @@ cff-version: 1.2.0 message: "If you use Tablassert, please cite it as below." type: software title: Tablassert -version: 15.1.0 +version: 16.0.0 license: Apache-2.0 repository-code: https://github.com/SkyeAv/Tablassert abstract: Tablassert is a highly performant declarative knowledge graph backend for bioinformatics that extracts knowledge assertions from tabular data, performs entity resolution and data quality control, and exports NCATS Translator-compliant Knowledge Graph Exchange (KGX) NDJSON. diff --git a/pyproject.toml b/pyproject.toml index 95870bf..ca86680 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "tablassert" -version = "15.1.0" +version = "16.0.0" description = "Extract knowledge assertions from tabular data into NCATS Translator-compliant KGX NDJSON — declaratively, with entity resolution and quality control built in." authors = [ { name = "Skye Lane Goetz", email = "sgoetz@isbscience.org" } diff --git a/uv.lock b/uv.lock index 26a9c5d..51776ba 100644 --- a/uv.lock +++ b/uv.lock @@ -3471,7 +3471,7 @@ wheels = [ [[package]] name = "tablassert" -version = "15.1.0" +version = "16.0.0" source = { editable = "." } dependencies = [ { name = "biolink-model" }, From b37dc118039e7f55a479d1c1cfb219758d9efbf6 Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Tue, 25 Aug 2026 13:28:57 -0700 Subject: [PATCH 5/6] docs: link the 16.0.0 changelog entry to its PR Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NuGRZ2fn3gE5zkUrv56Jqh --- CHANGELOG.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a485d9a..f479c89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,27 +5,27 @@ All notable changes to this project are documented in this file. ## 16.0.0 - 2026-08-25 ### Breaking Changes -- **Edge ids are canonicalized before hashing, so every existing edge id changes once.** The derivation sorted only *top-level* keys: nested values reached the digest through `Value::to_string`, which under serde_json's `preserve_order` emits insertion order. Every edge carries a `sources` array, so a polars struct-field reordering — or a Biolink release that shifts a nested slot — silently re-minted ids across a whole graph while nothing about the assertion had changed. Canonicalization now recurses (object keys sorted at every depth; array order preserved, because it is semantic), which makes ids robust to that churn at the cost of one migration. Relatedly, `false` is now hashed as `"false"` instead of being dropped along with its key: `strip_nulls` deliberately keeps `false` (`negated: false` is a meaningful Biolink value), so dropping it made `{subject, negated: false}` and `{subject}` — two distinct records — derive the *same* id. No shipped graph is affected by that second fix (the 1.27M-edge MultiomicsKG 3.0.0 build contains zero `false` values); the canonicalization fix moves every id. +- **Edge ids are canonicalized before hashing, so every existing edge id changes once.** The derivation sorted only *top-level* keys: nested values reached the digest through `Value::to_string`, which under serde_json's `preserve_order` emits insertion order. Every edge carries a `sources` array, so a polars struct-field reordering — or a Biolink release that shifts a nested slot — silently re-minted ids across a whole graph while nothing about the assertion had changed. Canonicalization now recurses (object keys sorted at every depth; array order preserved, because it is semantic), which makes ids robust to that churn at the cost of one migration. Relatedly, `false` is now hashed as `"false"` instead of being dropped along with its key: `strip_nulls` deliberately keeps `false` (`negated: false` is a meaningful Biolink value), so dropping it made `{subject, negated: false}` and `{subject}` — two distinct records — derive the *same* id. No shipped graph is affected by that second fix (the 1.27M-edge MultiomicsKG 3.0.0 build contains zero `false` values); the canonicalization fix moves every id. ([#122](https://github.com/SkyeAv/Tablassert/pull/122)) **Migration:** readers pinning Tablassert edge ids must re-key against the rebuilt graph. There is no mapping from old id to new — the old value was a hash of a serialization detail. This is a one-time event: adopt `uuid_fields` (below) in the same rebuild and ids stop moving for attribute changes thereafter. ### Added -- **`uuid_fields`: a graph config declares which edge fields constitute edge identity.** Edge ids were UUIDv3 hashes of the *entire* emitted record, so every field was an identity field — a corrected `p_value`, a bumped `subject_nlp_level` inside `supporting_text`, a changed `sources[].source_record_urls`, or a reordered source row (the `row:` inside `has_supporting_studies`) each minted a brand-new id, and downstream Translator consumers saw a new edge rather than the same edge with updated attributes. `Graph` gained an optional `uuid_fields` list; when set, only those top-level edge keys feed the hash, and everything else is free to change. Entries canonicalize onto their allow-listed spelling exactly as annotations do, and a list that cannot be a key — empty, repeating, containing `id`, or naming a field no edge emits — is rejected at config time as `uuid-bad-fields`. Leaving `uuid_fields` unset preserves the whole-record hash, so the feature is strictly opt-in. Measured on MultiomicsKG 3.0.0 (1,265,355 edges): re-analysing `p_value` and `effect_size` on every row moved **930,081 ids (73%)** under the whole-record hash and **none at all** under a declared `uuid_fields`. +- **`uuid_fields`: a graph config declares which edge fields constitute edge identity.** Edge ids were UUIDv3 hashes of the *entire* emitted record, so every field was an identity field — a corrected `p_value`, a bumped `subject_nlp_level` inside `supporting_text`, a changed `sources[].source_record_urls`, or a reordered source row (the `row:` inside `has_supporting_studies`) each minted a brand-new id, and downstream Translator consumers saw a new edge rather than the same edge with updated attributes. `Graph` gained an optional `uuid_fields` list; when set, only those top-level edge keys feed the hash, and everything else is free to change. Entries canonicalize onto their allow-listed spelling exactly as annotations do, and a list that cannot be a key — empty, repeating, containing `id`, or naming a field no edge emits — is rejected at config time as `uuid-bad-fields`. Leaving `uuid_fields` unset preserves the whole-record hash, so the feature is strictly opt-in. Measured on MultiomicsKG 3.0.0 (1,265,355 edges): re-analysing `p_value` and `effect_size` on every row moved **930,081 ids (73%)** under the whole-record hash and **none at all** under a declared `uuid_fields`. ([#122](https://github.com/SkyeAv/Tablassert/pull/122)) -- **The UUID namespace derives from the graph's infores when `uuid_fields` is declared.** Full-record hashing gave cross-graph uniqueness by accident: two graphs asserting the same triple from the same publication were separated by their differing `sources` and `supporting_text`. A narrow field set removes that accident, so the domain moves from the `TABLASSERT` constant onto `rig.source_info.infores_id`, making the separation structural instead. A new optional `uuid_domain` overrides it for the opposite case — graphs that must deliberately *share* an id space, such as a KG compiled in shards or renamed across versions while keeping its published ids. Both default to the historic constant when `uuid_fields` is unset. +- **The UUID namespace derives from the graph's infores when `uuid_fields` is declared.** Full-record hashing gave cross-graph uniqueness by accident: two graphs asserting the same triple from the same publication were separated by their differing `sources` and `supporting_text`. A narrow field set removes that accident, so the domain moves from the `TABLASSERT` constant onto `rig.source_info.infores_id`, making the separation structural instead. A new optional `uuid_domain` overrides it for the opposite case — graphs that must deliberately *share* an id space, such as a KG compiled in shards or renamed across versions while keeping its published ids. Both default to the historic constant when `uuid_fields` is unset. ([#122](https://github.com/SkyeAv/Tablassert/pull/122)) -- **A `duplicate-edge-ids` assertion in `--qc` study output.** `study_kgx` checked `duplicate-node-ids` but had no edge equivalent, even though KGX requires edge ids to be unique. The symmetric check now streams the final edges file and reports duplicates independently of the writer, catching an id collision from any source. +- **A `duplicate-edge-ids` assertion in `--qc` study output.** `study_kgx` checked `duplicate-node-ids` but had no edge equivalent, even though KGX requires edge ids to be unique. The symmetric check now streams the final edges file and reports duplicates independently of the writer, catching an id collision from any source. ([#122](https://github.com/SkyeAv/Tablassert/pull/122)) ### Fixed -- **Two edges deriving one id abort the build instead of shipping a duplicate.** Dedup keyed on the full canonical record bytes, which was safe only because the id was a pure function of those bytes. That equivalence breaks the moment the hash covers a subset, and it was already imperfect: `stable_json_bytes` never sorted despite its name, so two logically identical records arriving with different key order derived one id, produced different bytes, and *both* shipped. Edges now dedup on the derived id itself — an exact repeat collapses as before, while two genuinely different edges claiming one id raise `uuid-fields-not-a-key` with the id, the fields that differ (recovered by re-reading the partial output, on the failure path only), and the declared field list. The nodes path is unchanged: node ids are CURIEs, not derived hashes. +- **Two edges deriving one id abort the build instead of shipping a duplicate.** Dedup keyed on the full canonical record bytes, which was safe only because the id was a pure function of those bytes. That equivalence breaks the moment the hash covers a subset, and it was already imperfect: `stable_json_bytes` never sorted despite its name, so two logically identical records arriving with different key order derived one id, produced different bytes, and *both* shipped. Edges now dedup on the derived id itself — an exact repeat collapses as before, while two genuinely different edges claiming one id raise `uuid-fields-not-a-key` with the id, the fields that differ (recovered by re-reading the partial output, on the failure path only), and the declared field list. The nodes path is unchanged: node ids are CURIEs, not derived hashes. ([#122](https://github.com/SkyeAv/Tablassert/pull/122)) ### Performance -- **Edge dedup holds 24 bytes per edge instead of a full copy of every record: 2469 MB -> 100 MB peak on a 1.27M-edge graph.** `record_if_new` retained the complete bytes of every unique record to make suppression byte-exact — roughly 800 bytes per edge, and ~2.4 GB resident on MultiomicsKG 3.0.0 purely for the dedup set. The edge path now keys a `FxHashMap<[u8; 16], u64>` on the raw UUID bytes with an xxh64 of the id-free record as the value: a measured **25x** reduction that also removes the per-record heap allocation, and on the same graph took the dedup pass from 477s to 37s (the old path's allocation churn dominated; the margin will be smaller on machines under less memory pressure). The content hash is computed *before* the id is inserted, so no record is cloned to strip it back out, and a declared `uuid_fields` list additionally cuts the number of keys the hasher visits per edge. +- **Edge dedup holds 24 bytes per edge instead of a full copy of every record: 2469 MB -> 100 MB peak on a 1.27M-edge graph.** `record_if_new` retained the complete bytes of every unique record to make suppression byte-exact — roughly 800 bytes per edge, and ~2.4 GB resident on MultiomicsKG 3.0.0 purely for the dedup set. The edge path now keys a `FxHashMap<[u8; 16], u64>` on the raw UUID bytes with an xxh64 of the id-free record as the value: a measured **25x** reduction that also removes the per-record heap allocation, and on the same graph took the dedup pass from 477s to 37s (the old path's allocation churn dominated; the margin will be smaller on machines under less memory pressure). The content hash is computed *before* the id is inserted, so no record is cloned to strip it back out, and a declared `uuid_fields` list additionally cuts the number of keys the hasher visits per edge. ([#122](https://github.com/SkyeAv/Tablassert/pull/122)) ### Changed -- **`docs/api/utils.md` now describes the derivation that actually runs.** The page documented a `"\t".join(values)` encoding removed some releases ago (the join has been length-prefixed for collision-safety) and claimed edge ids covered only "subject, predicate, object, qualifiers, and publication" — which was never true, and is precisely what `uuid_fields` now makes achievable. Both are corrected, and the canonicalization, namespacing, and uniqueness rules are documented alongside. +- **`docs/api/utils.md` now describes the derivation that actually runs.** The page documented a `"\t".join(values)` encoding removed some releases ago (the join has been length-prefixed for collision-safety) and claimed edge ids covered only "subject, predicate, object, qualifiers, and publication" — which was never true, and is precisely what `uuid_fields` now makes achievable. Both are corrected, and the canonicalization, namespacing, and uniqueness rules are documented alongside. ([#122](https://github.com/SkyeAv/Tablassert/pull/122)) -- **Golden UUID vectors pin the derivation.** No test asserted a concrete UUID value, so any change to the encoding could land silently. Three vectors now fix the full-record, declared-field, and raw-parts derivations; if one moves, edge ids in every published graph moved with it. +- **Golden UUID vectors pin the derivation.** No test asserted a concrete UUID value, so any change to the encoding could land silently. Three vectors now fix the full-record, declared-field, and raw-parts derivations; if one moves, edge ids in every published graph moved with it. ([#122](https://github.com/SkyeAv/Tablassert/pull/122)) ## 15.1.0 - 2026-08-25 From e936985c14739ff698532c7290dda6c292a4112f Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Tue, 25 Aug 2026 13:30:00 -0700 Subject: [PATCH 6/6] Revert "chore(release): 16.0.0" This reverts commit 4a8d482. Version stays at 15.1.0 so merging this does not trip tag-version.yml and pipy.yml, which fire on a pyproject.toml version change landing on main. The 16.0.0 CHANGELOG entry stays as written and is ready for whenever the release commit is made deliberately. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NuGRZ2fn3gE5zkUrv56Jqh --- CITATION.cff | 2 +- pyproject.toml | 2 +- uv.lock | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CITATION.cff b/CITATION.cff index 8e5807f..06e6200 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -2,7 +2,7 @@ cff-version: 1.2.0 message: "If you use Tablassert, please cite it as below." type: software title: Tablassert -version: 16.0.0 +version: 15.1.0 license: Apache-2.0 repository-code: https://github.com/SkyeAv/Tablassert abstract: Tablassert is a highly performant declarative knowledge graph backend for bioinformatics that extracts knowledge assertions from tabular data, performs entity resolution and data quality control, and exports NCATS Translator-compliant Knowledge Graph Exchange (KGX) NDJSON. diff --git a/pyproject.toml b/pyproject.toml index ca86680..95870bf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "tablassert" -version = "16.0.0" +version = "15.1.0" description = "Extract knowledge assertions from tabular data into NCATS Translator-compliant KGX NDJSON — declaratively, with entity resolution and quality control built in." authors = [ { name = "Skye Lane Goetz", email = "sgoetz@isbscience.org" } diff --git a/uv.lock b/uv.lock index 51776ba..26a9c5d 100644 --- a/uv.lock +++ b/uv.lock @@ -3471,7 +3471,7 @@ wheels = [ [[package]] name = "tablassert" -version = "16.0.0" +version = "15.1.0" source = { editable = "." } dependencies = [ { name = "biolink-model" },