feat(diff): close the N-way Consolidate gap — note merge, column composition, native split/merge/row-moves, cell-shell visibility#250
Merged
Conversation
A note reference nested in the OPPOSITE note kind's body (an endnote reference inside a footnote definition, or vice versa) kept its stale w:id after the body-order renumber pass: each kind's RenumberNoteIds call swept only its OWN part for nested references, so the other kind's remap never reached it and the reference dangled on compare/accept/reject. RenumberNoteIds now returns its old->new id map and a single RemapNestedNoteReferences sweep runs after BOTH kinds' passes, covering both note parts for both reference kinds (same-kind nesting behaves exactly as before). Benefits every Compare() call, and is the renumbering groundwork for N-way note merging in Consolidate.
… more silent drop A width/gridSpan/vMerge/shading-only cell edit left every cell/row/ table hash identical: the table pair classified EqualBlock and the edit silently vanished from Compare AND Consolidate with zero conflict recorded — a soundness bug violating the never-silently- drop invariant. - IrReader.BuildCell now folds a canonical digest of the whole w:tcPr into the cell's ContentHash (rows/tables inherit it), and stores it on IrCell.ShellDigest so the N-way merger can compare shells without source-element access. - Pairwise Compare: a shell-only edit now surfaces as a count-stable ModifyRow and lands via the right-shelled fine table render. - Consolidate: IrAuthoredCellOp carries ShellSourceReviewer/ ShellRightCellAnchor; a changed cell's shell is sourced from the editing reviewer (single editor), the agreeing reviewers (consensus), or policy-resolved with a recorded IrConflict when two reviewers set different shells on the same cell (IrCompositeMerger.ComposeCellShell). The composed-table renderer clones the attributed reviewer's tcPr instead of always the base's. - Composite JSON: additive shellSourceReviewer/shellRightCellAnchor fields on composed cells. - IR golden snapshots regenerated (cell/row/table contentHash only). - New fixtures: pairwise width-only visibility; width edit composing with another reviewer's text edit; agreeing-width consensus; competing-width conflict per policy; gridSpan merge STOP fallback.
…solidate SplitBlock/MergeBlock were unconditionally lowered to del/ins in the N-way merge, and ANY reviewer MovedRow forced the whole-table block- conflict fallback. Now the same sole-toucher eligibility that drives native move composition applies to all three: - BuildTouchers extracted from PlanMoves as the single eligibility source (base anchor -> touching reviewers; a MergeBlock registers its consumed anchors). - ApplySplitMergePlan keeps a reviewer's Split/Merge NATIVE when the reviewer is the sole toucher of every consumed base paragraph, and lowers colliding ones to del/ins (existing conflict machinery). - MergeBlockStream dispatches native MergeBlocks via a first-consumed- anchor index (a MergeBlock has no LeftAnchor, so base-anchor grouping could never see it); the remaining consumed anchors emit no block op but keep insert slotting. GroupInsertsByPrecedingAnchor advances past a merge's consumed region so a following insert lands after the whole merge markup, never between its members. - Cell mini-bodies run the same plan, fixing a silent drop: a MergeBlock inside a multi-editor cell reached neither grouping map and vanished. - Composite tables: MovedRowsComposable replaces the NoReviewerHasMovedRow STOP gate — an uncontested row move lowers to the del+ins row shape the two-way renderer itself uses and composes; a contested one (same base row also edited/deleted/moved by another reviewer) still falls back to the recorded whole-table conflict. Native split/merge render through the existing two-way markup path (inserted/deleted pilcrows + per-segment spans), so a single reviewer's split/merge in a consolidate now matches Compare's markup shape. New fixtures: uncontested split/merge compose (+ round-trip, all policies), contested split/merge lower with recorded conflicts, insert-after-merge ordering, single-reviewer native shape, uncontested row move composes / contested row move falls back.
…idate structurally complete Closes the last two structural gaps in DocxDiff.Consolidate; a reviewer's edit now always composes or records an IrConflict, never silently drops, and never hard-fails. N-way note-scope merge (was: NotSupportedException for any N>=2 note edit; single-reviewer consolidates silently omitted note edits): - IrCompositeMerger.MergeNoteScopes groups reviewers' note diffs by base note id and runs the SAME MergeBlockStream dispatch the body uses over each note's base blocks — disjoint note edits compose, identical ones reach consensus, contested ones (incl. whole-note delete vs edit) are recorded conflicts resolved by policy. Reviewer-inserted notes pass through authored. In-note structural ops are conservatively lowered (content-preserving). - IrCompositeScript gains typed NoteOps (IrCompositeNoteDiff) and NoteIdMaps (per-reviewer note-id correspondence). - IrCompositeMarkupRenderer renders composed ops inside the footnotes/endnotes parts, creates reviewer-inserted definitions under fresh output ids (part created from the inserting reviewer's boilerplate when the base lacks it), rewrites reviewer-sourced body references from each reviewer's id space to the base-anchored output space (per-reviewer ref clones tracked via the existing RegisterMediaReferences choke point; del/moveFrom content is base-sourced and skipped), then runs the same body-order renumber + cross-kind nested-ref sweep as two-way Compare. - Consolidated revisions cover note edits (body, then footnotes, then endnotes); composite JSON serializes noteOps with per-op attribution. Column add/remove composition (was: any cell-count change forced the whole-table conflict fallback): - ComposeRowCells pairs each reviewer's cell ops by BASE cell anchor (not position); right-only ops become authored InsertCells (native w:tcPr/w:cellIns — kept on accept, removed on reject), left-only ops authored DeleteCells (w:cellDel — removed on accept via Word's gridSpan-absorb semantics, restored on reject); cell delete-vs-edit is a recorded conflict per policy. Single-modifier rows flow the same shapes through AuthoredCellsForSingleReviewer. - The AllColumnStructureStable STOP gate is removed; the only remaining table fallback is a CONTESTED row move. - Fixes a pre-existing RevisionProcessor NRE: accepting a cellDel'd cell whose absorbing neighbor had no w:tcPr. The ConsolidateParityScoreboard metric is STRENGTHENED to include referenced footnote/endnote texts in the accept ≡ right ratchet (all 84 corpus cases hold at the floor). New fixtures: disjoint/conflicting /inserted/deleted notes (all policies, round-trip), id-shifted inserted notes, two-reviewer note inserts, attributed note revisions, column add/remove/delete-vs-edit/gridSpan-merge composition. Verified: full .NET suite green (2522), Release + WASM-mode builds clean, LibreOffice load backstop clean over 99 consolidated outputs (9 structural scenarios x 3 policies x merged/accepted/rejected).
JSv4
added a commit
that referenced
this pull request
Jul 4, 2026
…nge/w:trPrChange/w:tblPrChange/w:tblGridChange/w:sectPrChange (#252) * test(diff): Phase 0 characterization pins for block-format changes (pPr/trPr/tblPr/tblGrid/tcPr/sectPr untracked today) * feat(diff): Phase 1 — native w:pPrChange for paragraph-property changes Detect paragraph-and-above property deltas at block pairing (ParaKey folded into IrModeledFormat.BlockSignature under a new internal TrackBlockFormatChanges flag; direct numId/ilvl now modeled in IrParaFormat). Render native w:pPrChange (and w:pPr/w:rPr/w:rPrChange for a changed paragraph mark) in the FormatOnly, Modify, and move-destination paths, carrying the OLD (left) pPr so accept ≡ right and reject ≡ left hold at the pPr level. Surface a Paragraph-scoped DocxDiffFormatChange (new DocxDiffFormatChangeScope; WmlComparerCompatible excludes non-Run scopes by construction). Consolidate forced off (v1 ceiling, pinned). Also fixes a latent consume-side bug: RejectRevisions dropped an inline w:sectPr when rebuilding pPr from a w:pPrChange (section break vanished on reject). * feat(diff): Phase 2+3 — native table-shell + section property change markup Phase 2 (table family): split the lumped table-shell digest into per-element TblPrDigest/TblGridDigest (IrTable) + TrPrDigest (IrRow), flattened so empty≡absent. Render native w:tblPrChange/w:tblGridChange/w:trPrChange (FormatOnly + Modified table paths) and w:tcPrChange (Modified cells) carrying the OLD shell; reject restores shell bytes. Report Table/TableRow/TableCell-scope FormatChanged revisions (digest-grade). This makes the #250-noted cell-shell edits tracked, not just visible. Phase 3 (section): stamp native w:sectPrChange on the trailing body sectPr when its PROPERTIES differ (page size/margins/orientation/type/columns), applying the right properties and preserving the left in the marker; references (header/footer machinery) untouched. Section-scope FormatChanged revision (modeled fields). Fixes a latent RevisionProcessor bug: rejecting a sectPrChange dropped header/footer references (CT_SectPrBase has none) — now the references are preserved across reject. AssertRoundTrip strengthened with per-table shell digests + a reference-normalized trailing-sectPr property digest, so the whole renderer battery verifies shell/section round-trip. Consolidate stays forced-off (v1 ceiling). Golden IR snapshots regenerated (digest layout only; ContentHash unchanged). * test(diff): real-doc block-format battery (HC029) — all five markers, round-trip, determinism, LibreOffice load backstop * docs+ripple(diff): block-format-change family — additive scope wire (Ops/npm/python), ir_diff_engine + format_change_detection + ooxml_corner_cases + CLAUDE + CHANGELOG * fix(diff): code-review findings — Consolidate revision leak, trPr/tblPrEx schema order, revision/markup shell-digest consistency - Gate the table-shell REVISION emitters on TrackBlockFormatChanges so the Consolidate v1 ceiling holds on GetConsolidatedRevisions too (a table-shell-only reviewer edit was reporting a Table/TableRow/TableCell FormatChanged the composite markup never backed). - InsertShellInSchemaOrder: a freshly-created w:trPr now lands AFTER an existing w:tblPrEx (CT_Row orders tblPrEx before trPr) — the AddFirst placement produced schema-invalid rows. - Add flattened per-element digests IrRow.TrPrShellDigest (w:trPr children only, no tblPrEx) and IrCell.TcPrShellDigest (empty≡absent), and drive the revision surface + round-trip harness from them, so GetRevisions and Compare's markup agree: a w:tblPrEx-only or empty-vs-absent-shell change is untracked in BOTH (was over-reported by the revision only). TrPrDigest (full row shell) still drives the fingerprint, so tblPrEx changes stay visible. Reviewer: pr-review-toolkit:code-reviewer (findings 1/2/3).
This was referenced Jul 4, 2026
JSv4
added a commit
that referenced
this pull request
Jul 12, 2026
… comment (#270) Issue #231 (compose footnote/endnote content edits across reviewers in the DocxDiff composite merger) was already implemented by #250/#261: MergeNoteScopes iterates { Footnote, Endnote } and ApplyCompositeNoteDiffs renders both scopes; the NoteOps-null tripwire is gone. This closes the two residual gaps that evaluation surfaced, both squarely in the IR-based DiffDocx engine: - Endnote regression coverage was missing. The dedicated IrCompositeNoteTests fixture is footnote-only, yet the acceptance criterion names "footnote/endnote text". IrCompositeEndnoteTests mirrors it for the endnote branch: disjoint endnote edits compose, same-endnote edits conflict per policy, consolidated revisions attribute endnote edits, and reject == base under all policies. - A stale comment in LowerStructuralOps still claimed "the composite path does not yet compose them across reviewers" (a pre-#250 leftover). Corrected: note scopes pass through body-op lowering unchanged and are composed later by MergeNoteScopes. No engine behavior change. Base 2-way parity stays 179/179; note suites green. Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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.
Makes
DocxDiff.Consolidatestructurally complete — the last engineering gate blocking the default-engine flip (M-A). The governing invariant throughout: a reviewer's edit is never silently dropped — it composes, or it is recorded as anIrConflict.What changed (leverage order)
1. Cross-kind nested note-ref renumbering (pairwise bug, benefits every
Compare)A note reference nested in the opposite note kind's body (an endnote ref inside a footnote definition, or vice versa) kept a stale
w:idafter the body-order renumber pass — each kind's pass swept only its own part.RenumberNoteIdsnow returns its old→new map and a singleRemapNestedNoteReferencessweep runs after both kinds' passes, covering both parts for both reference kinds.2. Cell-shell (
w:tcPr) edits visible and composableA width/gridSpan/vMerge/shading-only cell edit left every cell/row/table hash identical → classified
EqualBlock→ silently vanished fromCompareandConsolidatewith zero conflict recorded (a soundness bug). The wholew:tcPrnow participates in the cellContentHash(IrCell.ShellDigest); pairwiseComparesurfaces such edits, and the composed table sources a changed cell's shell from its editing reviewer, reaches consensus on agreeing shells, and records a conflict for competing ones (ComposeCellShell). IR golden snapshots regenerated (hash values only).3. Native split/merge + row-move composition
SplitBlock/MergeBlockwere unconditionally lowered to del/ins; any reviewerMovedRowforced the whole-table conflict fallback. The same sole-toucher eligibility that drives native move composition (via the extractedBuildTouchersmap) now covers all three:ApplySplitMergePlan) and render with the two-way inserted/deleted-pilcrow markup;MergeBlockStreamdispatches a nativeMergeBlock(null left anchor) via a first-consumed-anchor index, and insert routing advances past the merged region.MergeBlockinside a multi-editor table cell reached neither grouping map and vanished.4. Column add/remove composition + N-way note merge (new subsystems)
ComposeRowCellspairs each reviewer's cell ops by base cell anchor (not position). A right-only op becomes an authoredInsertCell(nativew:tcPr/w:cellIns— kept on accept, removed on reject); a left-only op an authoredDeleteCell(w:cellDel— removed on accept via Word's gridSpan-absorb semantics, restored on reject); cell delete-vs-edit is a recorded conflict per policy. TheAllColumnStructureStableSTOP gate is gone. (Also fixes a pre-existingRevisionProcessorNRE accepting acellDelcell whose absorbing neighbor had notcPr.)IrCompositeMerger.MergeNoteScopesruns the same per-block dispatch the body uses over each base-matched note (disjoint edits compose, consensus dedupes, contested edits — incl. whole-note delete vs edit — conflict per policy); reviewer-inserted notes land under fresh output ids. The composite renderer applies composed ops inside the note parts, rewrites reviewer-sourced body references from each reviewer's id space into the base-anchored output space (NoteIdMaps+ per-reviewer ref-clone tracking), and runs the same renumber + cross-kind sweep asCompare. TheNotSupportedExceptionguard (IrCompositeMerger.cs) and the renderer tripwire are removed — replaced by the real merge. Consolidated revisions and the composite JSON cover note ops.Validation
ConsolidateParityScoreboardTestsstrengthened: the accept ≡ right ratchet metric now includes referenced footnote/endnote texts (was: notes excluded as a v1 limitation); all 84 corpus cases hold at the floor.WASM_BUILD=true) builds clean.Docs updated:
docs/architecture/ir_diff_engine.md(dispatch table + limitations rewritten to the new engine state),CLAUDE.md,CHANGELOG.md.Out of scope (per the goal): fuzzy row-move detection, N:M split/merge, and the pairwise/WmlComparer-parity backlog (pPrChange, complex fields, RTL). The two open PR #245 decisions (FormatComparison default, semver) apply to the flip (M-E), not this work.
🤖 Generated with Claude Code
https://claude.ai/code/session_012GswvVQ2XdcE6kcW8skAs8
Generated by Claude Code