fix: sha256:06c5b2f1c52ed3b85132b911fd77132149aee69e17cff5736923f2c59a0dce1f · gabriel/muse

fix: imports/variables dimension honors its own unordered-set merge semantics (musehub#219)

Audit: reproduced live -- a pure reorder of existing imports on one branch, combined with a pure addition on another, produced a false file-level conflict identical to what a line-based tool would produce. The dimension schema explicitly declares imports/variables as SetSchema(identity="by_content"), "order is semantically irrelevant" -- the actual diff/merge behavior directly contradicted this.

Root cause, two stacked bugs:

1. _canonical_key (the symbol tree's dict key) bakes in node.lineno for every symbol kind, including imports/variables. A pure reorder changes every import's key, so diff_symbol_trees's content-match rename- detection pass treated each one as "moved" -- a spurious ReplaceOp for a change that should be invisible per the dimension's own declared identity="by_content" contract.

2. Even after suppressing that spurious op, blob reconstruction still fell back to three_way_merge_lines (line-based 3-way diff), which cannot reconcile a pure reorder on one side with an insertion on the other -- reordering moves every line, so the edit regions always overlap regardless of what the structured diff reports. Verified this computationally before writing any fix.

Fix: - diff_symbol_trees: suppress the "moved" ReplaceOp for import/variable-kind symbols when content_id matches at a different address (pure reorder) -- now correctly produces zero per-symbol ops (the file-level "reformatted -- no semantic change" label already existed for this case and is the accurate signal here). - New _merge_unordered_set_blob: reconstructs the merged file via a real three-way SET merge (survivors = base entries neither side removed, plus either side's new entries) whenever every symbol in the file is an unordered-set kind. Two safety gates, both with dedicated regression tests: (a) requires the file's content *outside* those symbols' line ranges to be byte-identical across base/ours/theirs -- comment-blind content_id comparison alone would have broken the existing comment-only independence merge (muse#92); (b) calls parse_symbols directly (not the exception-swallowing _parse_with_fallback) so a genuine SyntaxError is never silently treated as "zero imports" -- caught two real regressions in test_phase7_merge_correctness.py (IC_01, IC_04) before landing this.

Design decision (confirmed via AskUserQuestion): implement the full set- based reconstruction now rather than defer, despite the ticket's own "not urgent" framing -- the root cause was fully diagnosable and the fix cleanly scoped once traced.

Found and filed separately (not fixed here, confirmed NOT caused by this change via muse-dev shelf isolation): muse#112 -- the pre-existing independence-merge path duplicates import lines when a reorder+addition is combined with a disjoint function change in the same file. Reproduces identically with this fix shelved aside and restored.

TDD: wrote tests/test_imports_unordered_set_merge.py first (8 tests) -- the ticket's exact reorder+addition case (both directions), reorder+removal, pure-reorder-produces-no-moved-op, the same fix extended to the variables dimension (per the ticket's own suggested follow-up), and two fall-through guards (genuine function conflict still surfaces; divergent unparseable content still surfaces, not silently merged). Confirmed genuine RED via muse-dev shelf save/pop isolation -- 5 of 8 tests failed pre-fix (the other 3 pass either way, as expected for guard/regression tests).

Verified live via muse-dev against a real sandbox repo, reproducing the ticket's exact repro steps end-to-end: muse merge succeeds cleanly with the correct deduplicated, properly-ordered import list -- no --force needed.

Regression: 822 passed across every test file touching diff_symbol_trees, build_diff_ops, or merge_ops (code-plugin, independence-merge, phase2/3/5/7 merge-correctness, stress-merge, commit/diff/rename/semver-classification suites) -- no regressions.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

+28

symbols added

symbols modified

files changed

1260

files in snapshot

dead code introduced

Semantic Changes 35 symbols

~ tests/test_imports_unordered_set_merge.py .py 24 symbols added

+ TestGenuineChangesStillFallThrough class class TestGenuineChangesStillFallThrough L213–261

+ test_function_body_change_is_not_affected_by_import_reorder method method test_function_body_change_is_not_affected_by_import_reorder L218–239

+ test_unparseable_divergent_file_still_surfaces_conflict method method test_unparseable_divergent_file_still_surfaces_conflict L241–261

+ TestPureReorderIsANoOp class class TestPureReorderIsANoOp L156–184

+ test_pure_reorder_alone_produces_no_moved_op method method test_pure_reorder_alone_produces_no_moved_op L157–184

+ TestReorderPlusAddition class class TestReorderPlusAddition L74–133

+ test_addition_plus_reorder_is_symmetric method method test_addition_plus_reorder_is_symmetric L114–133

+ test_reorder_plus_addition_does_not_conflict method method test_reorder_plus_addition_does_not_conflict L78–93

+ test_reorder_plus_addition_merged_content_is_correct method method test_reorder_plus_addition_merged_content_is_correct L95–112

+ TestReorderPlusRemoval class class TestReorderPlusRemoval L136–153

+ test_reorder_plus_removal_does_not_conflict method method test_reorder_plus_removal_does_not_conflict L137–153

+ TestVariablesDimensionToo class class TestVariablesDimensionToo L187–210

+ test_variable_reorder_plus_addition_does_not_conflict method method test_variable_reorder_plus_addition_does_not_conflict L192–210

+ _oid function function _oid L47–48

+ _read_merged_blob function function _read_merged_blob L66–71

+ _snap function function _snap L58–63

+ _write_blob function function _write_blob L51–55

+ CodePlugin import import CodePlugin L41–41

+ Mapping import import Mapping L35–35

+ TYPE_CHECKING import import TYPE_CHECKING L36–36

+ annotations import import annotations L32–32

+ blob_id import import blob_id L40–40

+ pathlib import import pathlib L34–34

+ pytest import import pytest L38–38

~ muse/plugins/code/plugin.py .py 3 symbols added, 6 symbols modified

+ _merge_unordered_set_blob function function _merge_unordered_set_blob L1598–1707

+ SymbolRecord import import SymbolRecord L136–136

+ UNORDERED_SET_KINDS import import UNORDERED_SET_KINDS L141–141

~ CodePlugin

~ merge_ops

~ SymbolTree

~ build_diff_ops

~ delta_summary

~ parse_symbols

~ muse/plugins/code/symbol_diff.py .py 1 symbol added, 1 symbol modified

+ UNORDERED_SET_KINDS variable variable UNORDERED_SET_KINDS L75–75

~ diff_symbol_trees

Files Changed

+1 ~2

1260 in snapshot

+ tests/test_imports_unordered_set_merge.py .py

~ muse/plugins/code/plugin.py .py

~ muse/plugins/code/symbol_diff.py .py

← Older Oldest on fix/219-imports-unordered-set-merge

All commits

Newer → Latest on fix/219-imports-unordered-set-merge

0 comments

No comments yet. Be the first to start the discussion.

To add a comment, use the Muse CLI: muse hub commit comment sha256:06c5b2f1c52ed3b85132b911fd77132149aee69e17cff5736923f2c59a0dce1f --body "your comment"