From 61a1852dddb55d4e11264c0ddc1a4a4d2a422126 Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Tue, 27 Oct 2026 16:24:50 +0000 Subject: [PATCH 1/3] Fix a join reporting an empty result for a non-empty byte node DenseByteNode::join_child_into passed on the None that two empty nodes join to. merge_from_list_node merges the per-slot statuses, so a list node holding only dangling paths the byte node also has made the whole join None, and the byte node's other contents were dropped. Two dangling paths joined stay one dangling path, which is no change: Identity. join_child_into_left already did this. This is the debug assertion the algebraic fuzzer reported as finding 6, and one way a join lost values. --- src/dense_byte_node.rs | 7 +++++-- src/trie_map.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/dense_byte_node.rs b/src/dense_byte_node.rs index 0b7602c5..465409a6 100644 --- a/src/dense_byte_node.rs +++ b/src/dense_byte_node.rs @@ -147,8 +147,11 @@ impl> ByteNode if self.mask.test_bit(k) { let cf = unsafe { self.values.get_unchecked_mut(ix) }; match cf.rec_mut() { - Some(existing_node) => { - existing_node.join_into(node) + Some(existing_node) => match existing_node.join_into(node) { + //Only two empty nodes join to nothing: the path stays dangling and nothing changes. + // `None` would tell the caller this whole node is empty + AlgebraicStatus::None => AlgebraicStatus::Identity, + status => status, }, None => { cf.set_rec(node); diff --git a/src/trie_map.rs b/src/trie_map.rs index 5c0b1dfd..27b348fc 100644 --- a/src/trie_map.rs +++ b/src/trie_map.rs @@ -1592,6 +1592,33 @@ mod tests { assert_eq!(map.val_at(b"foo/bar"), Some(&300)); assert_eq!(map.val_at(b"missing"), None); } + + /// Joining a byte node with a list node whose only contents are dangling paths the byte node + /// also has. Each list slot joins to nothing, which must not make the whole join empty + #[test] + fn map_join_with_dangling_only_list_node() { + let mut a = PathMap::::new(); + a.set_val_at([0u8, 1, 2, 2, 2], 1); + a.set_val_at([1u8, 0], 1); + a.create_path([2u8, 0]); + a.create_path([2u8, 1]); + let mut b = a.clone(); + a.set_val_at([2u8, 2], 2); + //Rebuilt rather than cloned, so the two share no nodes + let mut b_fresh = PathMap::::new(); + b_fresh.set_val_at([0u8, 1, 2, 2, 2], 1); + b_fresh.set_val_at([1u8, 0], 1); + b_fresh.create_path([2u8, 0]); + b_fresh.create_path([2u8, 1]); + b.remove_val_at([2u8, 2], false); + + for b in [&b, &b_fresh] { + for joined in [a.join(b), b.join(&a)] { + assert_eq!(joined.val_at([2u8, 2]), Some(&2)); + assert_eq!(joined.val_count(), 3); + } + } + } } //GOAT, Consider refactor of zipper traits. `WriteZipper` -> `PathWriter`. Zipper is split into the zipper From ce47f12d34d80a4d04f2f2303c6eee3a23fb0a16 Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Wed, 4 Nov 2026 17:51:41 +0000 Subject: [PATCH 2/3] Reduce the join empty-status test to a minimal repro Two dangling paths shared by both sides and a value only in the left one: the join must keep the value. --- src/trie_map.rs | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/src/trie_map.rs b/src/trie_map.rs index 27b348fc..2d574b30 100644 --- a/src/trie_map.rs +++ b/src/trie_map.rs @@ -1597,27 +1597,14 @@ mod tests { /// also has. Each list slot joins to nothing, which must not make the whole join empty #[test] fn map_join_with_dangling_only_list_node() { - let mut a = PathMap::::new(); - a.set_val_at([0u8, 1, 2, 2, 2], 1); - a.set_val_at([1u8, 0], 1); - a.create_path([2u8, 0]); - a.create_path([2u8, 1]); - let mut b = a.clone(); - a.set_val_at([2u8, 2], 2); - //Rebuilt rather than cloned, so the two share no nodes - let mut b_fresh = PathMap::::new(); - b_fresh.set_val_at([0u8, 1, 2, 2, 2], 1); - b_fresh.set_val_at([1u8, 0], 1); - b_fresh.create_path([2u8, 0]); - b_fresh.create_path([2u8, 1]); - b.remove_val_at([2u8, 2], false); - - for b in [&b, &b_fresh] { - for joined in [a.join(b), b.join(&a)] { - assert_eq!(joined.val_at([2u8, 2]), Some(&2)); - assert_eq!(joined.val_count(), 3); - } - } + let mut a = PathMap::<()>::new(); + a.create_path([0u8]); + a.create_path([1u8]); + a.set_val_at([2u8], ()); + let mut b = PathMap::<()>::new(); + b.create_path([0u8]); + b.create_path([1u8]); + assert!(a.join(&b).contains([2u8])); } } From 23b1f58e674a20e0a0819c954b02cf69e49b4b95 Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Wed, 4 Nov 2026 18:03:31 +0000 Subject: [PATCH 3/3] Test that a join keeps a value across clone-shared structure Finding 4 of the algebraic fuzzer: with b a clone of a plus one value, a.join(&b) dropped that value while b.join(&a) kept it. Same cause as the empty-status fix: the byte node reported the list node's join as empty. --- src/trie_map.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/trie_map.rs b/src/trie_map.rs index 2d574b30..49c72936 100644 --- a/src/trie_map.rs +++ b/src/trie_map.rs @@ -1606,6 +1606,21 @@ mod tests { b.create_path([1u8]); assert!(a.join(&b).contains([2u8])); } + + /// `b` is a clone of `a` plus one value, so the two share nodes. A join may not lose that + /// value in either operand order. Found by the algebraic fuzzer's `join-commutative` law. + #[test] + fn map_join_keeps_value_across_shared_structure() { + let mut a = PathMap::::new(); + for p in [&[0u8, 0, 0, 0, 0][..], &[1, 0, 1], &[1, 0, 3]] { + a.create_path(p); + } + let mut b = a.clone(); + b.set_val_at([1u8, 0, 0, 0], 1); + + assert_eq!(a.join(&b).val_at([1u8, 0, 0, 0]), Some(&1)); + assert_eq!(b.join(&a).val_at([1u8, 0, 0, 0]), Some(&1)); + } } //GOAT, Consider refactor of zipper traits. `WriteZipper` -> `PathWriter`. Zipper is split into the zipper