fix(chunker): platform-independent hash_item + collapse single-child root - #244
Merged
Conversation
The tree is content-addressed, so equal get_hash() => byte-identical subtree. The old diff descended incorrectly and could silently drop changes (root-only style data loss). Rewrite diff_nodes_recursive to: short-circuit equal-hash subtrees, merge-join leaf pairs, and for internal nodes skip children whose stored child-hash matches (O(diff), no load) — flattening only the divergent region. Kept honest by a #[cfg(test)] full-leaf flatten oracle (diff_nodes_flatten) that every new differential test asserts byte-identical against, plus ground-truth add/remove/modify assertions. Old diff_recursive retained (dead_code). Uses only existing APIs; no format/hash change. Tests: mod odiff_differential.
…single-child root Two content-defined-chunker canonicality bugs: 1. hash_item fed the item through the slice Hash impl, which prefixes a platform-width usize length (8 bytes native, 4 on wasm32) before the bytes — so chunk boundaries, and thus every tree's shape and root hash, DIFFER between 32- and 64-bit targets. Use hasher.write(item) to hash the raw content bytes only (in streaming_chunker and in ProllyNode's NodeChunk). Deterministic across targets. 2. A content-defined boundary on the final item could leave the root a single-child internal node — a non-minimal spine diverging from a fresh batch build. Collapse single-child internal roots in done(). Because merge() diffs internally, the new boundaries surface the root-only diff data-loss bug during merge, so this fix REQUIRES the canonical-structural-diff fix (this branch is stacked on it). Golden value in test_delete updated for the corrected boundary. Tests: mod chunker_invariants.
# Conflicts: # src/tree.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolved version of #193 against current main. Preserves prefix-free hashing and structural-diff coverage already merged, plus the chunker canonicality changes and invariant tests from #193. Supersedes #193 after CI passes.