Skip to content

fix(merge): safe cross-Qube dimension merging with translation map#105

Draft
HCookie wants to merge 1 commit into
mainfrom
fix/merge-dimension-safety
Draft

fix(merge): safe cross-Qube dimension merging with translation map#105
HCookie wants to merge 1 commit into
mainfrom
fix/merge-dimension-safety

Conversation

@HCookie

@HCookie HCookie commented Jun 23, 2026

Copy link
Copy Markdown
Member

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. number coords could be intersected against time coords.

Solution

Dimension translation map

  • Before merging, build a mapping from other's dimension IDs to self's by interning all of other's dimension names into self's key_store.
  • All merge comparisons now go through this map.

Missing dimension handling

  • When a dimension exists only in other, copy its entire subtree into self (previously silently dropped).

drop_recurse fix

  • Iteratively splice out consecutive matching dimensions instead of only one level (previously could skip intermediate levels).

Supporting changes

  • Add Clone for Node and Qube
  • Add axes(), dimensions(), all_dim_ids(), get_or_intern_dim(), prepend() methods
  • Include coordinate intersection safety fixes (return Empty instead of panicking on cross-type/empty operands)

Tests

  • Test merging Qubes with different dimension interning order
  • Test that shared dimensions merge correctly

Part of the split from #102. Independently mergeable.

- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant