fix(merge): safe cross-Qube dimension merging with translation map#105
Draft
HCookie wants to merge 1 commit into
Draft
fix(merge): safe cross-Qube dimension merging with translation map#105HCookie wants to merge 1 commit into
HCookie wants to merge 1 commit into
Conversation
- Build a dimension translation map before merging, so two Qubes with dimensions interned in different orders merge correctly (previously could cross-intersect unrelated dimensions sharing the same interner ID) - Handle the case where a dimension exists only in `other` during node_merge (copy subtree instead of silently dropping) - Fix drop_recurse to iteratively splice out consecutive matching dimensions (previously could skip intermediate levels) - Add Clone for Node and Qube - Add axes(), dimensions(), all_dim_ids(), get_or_intern_dim() methods - Add prepend() to wrap tree under a new parent node Also includes coordinate correctness fixes required for safe merging: - Fix intersection to return Empty instead of panicking on cross-type/empty - Implement Mixed coordinates to_string() - Fix leading-zero consistency across multi-value coordinate strings - Add type coercion between Strings and Integers when extending
This was referenced Jun 23, 2026
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
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.
Summary
Fixes a critical bug where merging two Qubes built from different sources (with dimensions interned in different orders) would cross-intersect unrelated dimensions, causing panics or silent data corruption.
Problem
When two Qubes intern their dimensions in different orders, the same integer ID can refer to different dimension names. The merge logic previously compared dimensions by ID directly, meaning e.g.
numbercoords could be intersected againsttimecoords.Solution
Dimension translation map
other's dimension IDs toself's by interning all ofother's dimension names intoself's key_store.Missing dimension handling
other, copy its entire subtree intoself(previously silently dropped).drop_recurse fix
Supporting changes
CloneforNodeandQubeaxes(),dimensions(),all_dim_ids(),get_or_intern_dim(),prepend()methodsTests
Part of the split from #102. Independently mergeable.