test(diff): pin gridSpan/vMerge property-only table change scope (#230)#257
Merged
Conversation
Issue #230 flagged that the IR did not model property-only table/cell changes (w:tcPr / w:tblPr / gridSpan / vMerge), so a reviewer's formatting-only table edit read as unchanged. The cell-shell case was resolved by the block-format-change family (native w:tcPrChange; IrCell.ShellDigest folded into the cell ContentHash) and by consolidate cell-shell composition. This closes the issue's remaining acceptance criterion -- documenting the chosen gridSpan/vMerge scope -- and adds the direct regression proof that was missing: - BlockFormatChangeTests.{GridSpanOnly,VMergeOnly}_cell_change_is_tracked_with_native_tcPrChange: a gridSpan- or vMerge-only change (cell count stable, text unchanged) is detected in 2-way Compare, renders native w:tcPrChange with a TableCell FormatChanged revision, and round-trips (accept == right, reject == left) at the tcPr-byte level; schema-valid. - IrCompositeTableTests.VMerge_only_cell_edit_composes: the same edit composes in Consolidate like a width/shading edit (lands on accept, reject == base, no conflict) under all three conflict policies. - docs/architecture/ir_diff_engine.md: the gridSpan/vMerge scope decision (detect / compose / column-structure fallback / changed-name granularity). No engine change -- the behavior already held; this makes it auditable and regression-proof. Full suite green (2607 passed, 3 skipped).
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 #230.
What #230 asked
The IR did not model property-only table/cell changes (
w:tcPr/w:tblPr/gridSpan/vMerge): a reviewer who changed only cell shading/borders/width, table properties, or cell spanning produced an IR whose content/format fingerprints were identical to the base, so both 2-wayDocxDiff.Compareand consolidate treated the table asEqualBlock— the change was silently invisible (no revision, no markup, no conflict).Acceptance criteria: (1) a
tcPr-only cell change is detected as a format change in 2-wayCompareand composes/conflicts in consolidate; (2) document the chosen scope forgridSpan/vMerge(detect vs. compose); (3) base 2-way parity floor holds, full suite green.What I found (proved with a fixture)
I first wrote a throwaway probe that ran
DocxDiff.Compare/Consolidateover programmatic tables and inspected the IR hashes + emitted markup. Ground truth:IrCell.ShellDigest(the wholew:tcPr) is folded into the cellContentHash, so a shell edit makes the tableModified.Comparenow tracks every property-only case — shading, width,vMerge-only, andgridSpan-only — as nativew:tcPrChangewith aTableCellFormatChangedrevision, round-trippingaccept ≡ right/reject ≡ leftat the tcPr-byte level.vMerge/gridSpancell-shell edit exactly like a width/shading edit (lands on accept,reject ≡ base, conflict on competing shells).The genuinely-unmet parts were criterion (2) — the
gridSpan/vMergescope was never explicitly documented — and the absence of direct regression fixtures for that corner. So this PR closes the proof + documentation gap rather than touching the (already-correct) engine.Changes
BlockFormatChangeTests.{GridSpanOnly,VMergeOnly}_cell_change_is_tracked_with_native_tcPrChange— 2-way pins: agridSpan- /vMerge-only change (cell count stable, text unchanged) → nativew:tcPrChange,TableCellFormatChangedrevision, clean round-trip, schema-valid, no spuriousins/del.IrCompositeTableTests.VMerge_only_cell_edit_composes(3 conflict policies) — consolidate pin: the reviewer's vMerge lands on accept,reject ≡ base, no conflict.docs/architecture/ir_diff_engine.md— the explicitgridSpan/vMergescope decision: detect (2-way) / compose (consolidate) / column-structure fallback /["shell"]changed-name granularity rationale.CHANGELOG.md—[Unreleased]entry.No production code changed — the behavior already held; this makes it auditable and regression-proof.
Verification
IrAlignerAdversarialTests.Scale_guard_..., a wall-clock micro-benchmark that failed only under full-suite CPU contention and passes 3/3 in isolation — unrelated to these test/doc-only changes).IrParityScoreboardTests,ConsolidateParityScoreboardTests) green — the 2-way parity floor holds.Generated by Claude Code