fix(diff): keep tables anchored when a paragraph move is ambiguous (#229)#256
Merged
Conversation
) A reorder can read equally as relocating a light paragraph or a heavy structural block: [A, table, B] -> [A, B, table] is both "paragraph B moved up past the table" and "the table moved down past B". IrBlockAligner's LIS spine arbitrarily picked the reading that relocated the TABLE. A two-way Compare round-trips either way, but in an N-way Consolidate the spurious table-move collided with a second reviewer's disjoint table-cell edit and collapsed the whole table to a block-level conflict — the cell edit was surfaced in the conflict instead of composed per-cell (no data loss; reject == base always held). Add a structural-anchor tie-break: among equal-length spines, keep the most non-paragraph blocks (tables / section breaks / opaque) anchored and relocate the lighter paragraph. Implemented as a maximum-weight longest-increasing- subsequence (cardinality stays the primary key, so the relocation count is unchanged) that fires ONLY when the plain patience-sort LIS relocated a structural block — the paragraph-only common path is byte-identical. Now the paragraph move and the disjoint cell edit compose independently (conflicts == 0, native/lowered paragraph move + per-cell table compose, reject == base). Base two-way parity holds at 179/179; full suite green. Adds IrCompositeTableMoveBoundaryTests (all three conflict policies).
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.
Closes #229.
Problem
When a reviewer moves a paragraph across a table boundary and a second reviewer edits a disjoint cell of that table,
DocxDiff.Consolidatecollapsed the whole table to a block-level conflict instead of composing the two edits independently.Root cause is in the pairwise aligner, not the composite merger. A reorder like
[A, table, B]→[A, B, table]is genuinely ambiguous:Both cost exactly one relocation and round-trip identically in a two-way
Compare.IrBlockAligner's LIS spine has two equal-length solutions here and the patience-sort tie-break arbitrarily picked the one that relocates the table. That spurious table-move then collides — in the N-way merge — with the second reviewer's disjoint cell edit, andIrCompositeMergerfalls back to a whole-table block conflict (BaseWinskeeps the base table; the cell edit is recorded in the conflict rather than composed per-cell). No data loss, andreject ≡ basealways held — it was a precision/quality limitation.Proof it existed
IrCompositeTableMoveBoundaryTestsreproduces the issue's exact sketch (baseparagraph A+ 2×2 table +paragraph B; Alice moves B before the table; Bob edits cell(1,1)). Before the fix the test failed on all three conflict policies with atbl:…-anchoredDocxDiffConflict(Alice's empty competitor + Bob's whole-tabled BOBcompetitor) — matching the issue verbatim.Fix
A structural-anchor tie-break in
IrBlockAligner: among equal-length spines, keep the most non-paragraph blocks (tables / section breaks / opaque blocks) anchored and relocate the lighter paragraph instead.BIG + (structural ? 1 : 0)withBIG = candidates.Count + 1. BecauseBIGdominates any achievable bonus total, cardinality stays the primary key — the number of relocated blocks is unchanged; the bonus is a pure tie-break.O(k log k)via a Fenwick prefix-max and is fully deterministic.This fixes the root cause at the aligner (also yielding cleaner two-way markup — a paragraph moves, not a whole table) rather than patching the composite merger.
Result
The paragraph move and the disjoint cell edit now compose independently:
conflicts == 0, a native/lowered paragraph move plus per-cell table compose,reject ≡ base, no content loss — the issue's acceptance criteria.Testing
IrCompositeTableMoveBoundaryTests(all threeConflictResolutionpolicies) — red before, green after.IrParityScoreboardTests).TreatWarningsAsErrors) andWASM_BUILDcompile clean; no warnings from the changed file.Files
Docxodus/Ir/Diff/IrBlockAligner.cs— the tie-break (guard + weighted LIS).Docxodus.Tests/Ir/Diff/IrCompositeTableMoveBoundaryTests.cs— regression test.CHANGELOG.md,docs/architecture/ir_diff_engine.md— documentation.No public API change, so no WASM/npm/python wrapper ripple was needed.
🤖 Generated with Claude Code
Generated by Claude Code