From ec487c8320a5f4706ef7c7bef3089c88cc493a3d Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Wed, 16 Sep 2026 22:17:49 +0000 Subject: [PATCH 1/2] Keep a value in subtract when the other side only passes through ByteNode::psubtract_abstract dropped a value at a byte where the other node has no value, only a longer path. The value now survives. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019R2H8fnco29asY2v3TPbtF --- src/dense_byte_node.rs | 10 ++++++---- src/trie_map.rs | 29 +++++++++++++++++++++++++++++ src/write_zipper.rs | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/dense_byte_node.rs b/src/dense_byte_node.rs index 65568d9c..42c77321 100644 --- a/src/dense_byte_node.rs +++ b/src/dense_byte_node.rs @@ -418,10 +418,11 @@ impl> ByteNode let cf = unsafe{ self_node.values.get_unchecked(cf_idx) }; let mut new_cf = Cf::new(None, None); - //If there is a value at this key_byte, and the other node contains a value, subtract them + //If there is a value at this key_byte, and the other node contains a value, subtract them; + // otherwise keep it if let Some(self_val) = cf.val() { - if let Some(other_val) = other.node_get_val(&[key_byte]) { - match self_val.psubtract(other_val) { + match other.node_get_val(&[key_byte]) { + Some(other_val) => match self_val.psubtract(other_val) { AlgebraicResult::None => { is_identity = false; }, AlgebraicResult::Identity(mask) => { debug_assert_eq!(mask, SELF_IDENT); //subtract is not commutative @@ -431,7 +432,8 @@ impl> ByteNode is_identity = false; new_cf.set_val(e); }, - } + }, + None => new_cf.set_val(self_val.clone()), } } diff --git a/src/trie_map.rs b/src/trie_map.rs index efe75ed6..a9cc1da0 100644 --- a/src/trie_map.rs +++ b/src/trie_map.rs @@ -876,6 +876,35 @@ mod tests { // c-> List("aaa") } + /// `subtract` keeps a value when the other map only has a longer path through it + #[test] + fn map_subtract_keeps_value_under_other_path() { + let mut a: PathMap = PathMap::new(); + a.insert([0], 0); + a.insert([0, 0], 0); + a.insert([0, 0, 0], 0); + a.insert([1, 0, 0], 0); + let mut b: PathMap = PathMap::new(); + b.insert([0, 0], 0); + b.insert([1, 0, 0], 0); + + let diff = a.subtract(&b); + let paths: Vec<(Vec, u64)> = diff.iter().map(|(k, v)| (k.to_vec(), *v)).collect(); + assert_eq!(paths, vec![(vec![0], 0), (vec![0, 0, 0], 0)]); + + // distinct values + let mut a: PathMap = PathMap::new(); + a.insert([0], 5); + a.insert([0, 0], 6); + a.insert([0, 0, 0], 7); + a.insert([1, 0, 0], 8); + let diff = a.subtract(&b); + assert_eq!(diff.val_at([0]), Some(&5)); + assert_eq!(diff.val_at([0, 0]), Some(&6)); // 6 - 0 is Element(6) for u64 + assert_eq!(diff.val_at([0, 0, 0]), Some(&7)); + assert_eq!(diff.val_at([1, 0, 0]), Some(&8)); + } + #[test] fn map_insert_test() { let keys = [ diff --git a/src/write_zipper.rs b/src/write_zipper.rs index 0892e475..1cc65e7c 100644 --- a/src/write_zipper.rs +++ b/src/write_zipper.rs @@ -3754,6 +3754,39 @@ mod tests { assert_eq!(map.iter().count(), 1); } + /// `subtract_into` keeps a value when the source only has a longer path through it + #[test] + fn write_zipper_subtract_into_value_under_source_path() { + let mut map: PathMap = PathMap::new(); + map.insert([0], 0); + map.insert([0, 0], 0); + map.insert([0, 0, 0], 0); + map.insert([1, 0, 0], 0); + let mut src: PathMap = PathMap::new(); + src.insert([0, 0], 0); + src.insert([1, 0, 0], 0); + + assert_eq!(map.write_zipper().subtract_into(&src.read_zipper(), false), AlgebraicStatus::Element); + let remaining: Vec<(Vec, u64)> = map.iter().map(|(k, v)| (k.to_vec(), *v)).collect(); + assert_eq!(remaining, vec![(vec![0], 0), (vec![0, 0, 0], 0)]); + + // Same shape, reached through a join first + let mut map: PathMap = PathMap::new(); + map.insert([], 0); + map.insert([0], 0); + map.insert([0, 0, 0], 0); + let mut src: PathMap = PathMap::new(); + src.insert([], 0); + src.insert([0, 0], 0); + src.insert([1, 0, 0], 0); + let mut wz = map.write_zipper(); + wz.join_into(&src.read_zipper()); + wz.subtract_into(&src.read_zipper(), false); + drop(wz); + let remaining: Vec<(Vec, u64)> = map.iter().map(|(k, v)| (k.to_vec(), *v)).collect(); + assert_eq!(remaining, vec![(vec![0], 0), (vec![0, 0, 0], 0)]); + } + /// Tests how `subtract_into` handles dangling paths, including situations with extraneous empty nodes hanging around #[test] fn write_zipper_subtract_into_test2() { From c14900049ae92200c3f74afad01953a41ae99047 Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Wed, 16 Sep 2026 23:29:09 +0000 Subject: [PATCH 2/2] Fix dense subtract keeping a dangling path the source reaches Against a list or tiny node, psubtract_abstract dropped a dangling slot from the result but left the identity flag set, so the caller kept the destination and its dangling path. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019R2H8fnco29asY2v3TPbtF --- src/dense_byte_node.rs | 4 +++- src/write_zipper.rs | 47 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/dense_byte_node.rs b/src/dense_byte_node.rs index 42c77321..ba66a7cb 100644 --- a/src/dense_byte_node.rs +++ b/src/dense_byte_node.rs @@ -463,10 +463,12 @@ impl> ByteNode } } - //If we ended up with a value or a link in the CF, insert it into a new node + //Keep the CF if it still has a value or link; otherwise the location, dangling or not, is gone if new_cf.has_rec() || new_cf.has_val() { new_node.mask.set_bit(key_byte); new_node.values.push(new_cf); + } else { + is_identity = false; } } else { new_node.mask.set_bit(key_byte); diff --git a/src/write_zipper.rs b/src/write_zipper.rs index 1cc65e7c..e149a2dd 100644 --- a/src/write_zipper.rs +++ b/src/write_zipper.rs @@ -6705,4 +6705,51 @@ mod tests { } assert_eq!(keys(&m), ["cx", "cy", "d"]); } + + /// Every location in `map`, dangling ones included, with its value + fn all_locations(map: &PathMap) -> Vec<(Vec, Option)> { + let mut rz = map.read_zipper(); + let mut locations = vec![]; + loop { + locations.push((rz.path().to_vec(), rz.val().cloned())); + if !rz.to_next_step() { break } + } + locations + } + + /// Dense `subtract_into` against a list or tiny node drops a dangling path the source reaches + #[test] + fn write_zipper_subtract_into_dense_drops_reached_dangling_path() { + // Empty onward link at [2]; the source (a LineListNode) reaches [2] + let mut dst = PathMap::::new(); + for b in [1u8, 3, 4] { dst.set_val_at(&[b], 1); } + dst.create_path(&[2u8]); + let mut src = PathMap::::new(); + src.set_val_at(&[2u8, 0, 1], 246); + let mut wz = dst.write_zipper(); + assert_eq!(wz.subtract_into(&src.read_zipper(), false), AlgebraicStatus::Element); + drop(wz); + assert_eq!(all_locations(&dst), vec![(vec![], None), (vec![1], Some(1)), (vec![3], Some(1)), (vec![4], Some(1))]); + + // Dangling [2] from an empty meet; the source is a TinyRefNode + let mut dst = PathMap::::new(); + dst.set_val_at(&[2u8, 0, 0, 0, 0], 0); + dst.set_val_at(&[0u8, 0, 0, 0, 0], 0); + dst.set_val_at(&[0u8], 0); + let mut srcs = PathMap::::new(); + srcs.set_val_at(&[1u8, 2, 0], 0); + let mut wz = dst.write_zipper(); + wz.descend_to_byte(2); + let ra = srcs.read_zipper_at_path(&[1u8]); + let rb = srcs.read_zipper_at_path(&[1u8, 0]); + assert_eq!(wz.meet_2(&ra, &rb), AlgebraicStatus::None); + wz.reset(); + assert_eq!({ let mut probe = wz.fork_read_zipper(); probe.descend_to(&[2u8]); probe.path_exists() }, true, "the meet should leave [2] dangling"); + assert_eq!(wz.subtract_into(&ra, false), AlgebraicStatus::Element); + drop(wz); + assert_eq!(all_locations(&dst), vec![ + (vec![], None), (vec![0], Some(0)), (vec![0, 0], None), (vec![0, 0, 0], None), + (vec![0, 0, 0, 0], None), (vec![0, 0, 0, 0, 0], Some(0)), + ]); + } }