feat(diff): Consolidate B2 — merge reviewers' table-shell + section format changes + cross-kind note fix#261
Merged
Conversation
…trPr/tblPr/tblGrid/tblPrEx) + byte-level verifiers Phase 0: strengthen the format-blind composite verifiers first — new Docs.ShellSection byte-level shell/section projection, asserted alongside the text projections in CompositeFuzzTests (3/4/5-way regression guard) and the new ConsolidateBlockFormatB2Tests (per-family reject=base/accept=winner). Phase 1: carve TrackTableFormatChanges (+ TrackSectionFormatChanges) slices out of the umbrella TrackBlockFormatChanges (mirrors B1's paragraph slice; public opt-out cascades to all three); the composite turns the table slice ON. Single-reviewer table shells merge via the two-way single-source render. Multi-reviewer cells stamp w:tcPrChange at the composed-table render gap (was: shell swapped with no marker -> reject != base). Multi-reviewer trPr/tblPr/tblGrid/tblPrEx compose per-element via ComposeTableAndRowShells (mirrors ComposeCellShell: 0->base/agree->consensus/>=2->conflict/non-stackable), carried on IrAuthoredRowOp.TrPr/TblPrEx + IrCompositeOp.TableShell and stamped by ApplyComposedShell. Revision surface re-gated onto the table slice. reject == base / accept == policy-winner hold at the property-byte level. B1 ceiling pins re-baselined.
…nge, trailing + inline) Carve TrackSectionFormatChanges out of the umbrella (public opt-out cascades); the composite turns it ON. Trailing sectPr is not a body block op (Word compares it at the document level), so ComposeTrailingSection composes base vs each reviewer's trailing IrSectionBreak.FormatFingerprint (modeled + unmodeled digest), mirroring ComposeCellShell; the winner rides on IrCompositeScript.TrailingSectPr and the composite renderer stamps w:sectPrChange (inner = base, header/footer refs preserved) via ApplyComposedTrailingSectPr. Inline (in-pPr) sectPr merges by riding B1's paragraph FormatOnly path (SectionKey re-gated onto the section slice). Two-way section emission + revision surface re-gated onto the section slice (byte-identical when all slices on; 470 two-way + composite regression green). reject == base / accept == policy-winner at the property-byte level.
…ed (v1 decision, never silent-drop) Per the ratified v1 decision, a reviewer editing a paragraph's TEXT and pPr is conflict-routed, not inline-composed. The existing ParagraphPropsUnchanged guard already implements this: a cross-reviewer text+pPr collision falls out of token-composition into a RECORDED block conflict (never a silent format drop, because RenderComposedParagraph clones base pPr and is never reached with a pPr-changed reviewer); a SINGLE reviewer's text+pPr edit tracks both (w:ins/w:del + w:pPrChange). True inline text+format compose is deferred to B3. No production change — pins only.
…OG/CLAUDE/ir_diff_engine Consolidate now merges the paragraph (B1) + table-shell + section (B2) block-format families with per-element attribution + native markup; competing edits conflict per policy; reject == base / accept == policy-winner at the property-byte level. text+pPr stays conflict-routed (v1 decision). The only remaining block-format ceiling is split/merge-member pPrChange (a principled decline).
…y once, not per-op The composite revision renderer renders each op through a two-way one-op mini-script, and the two-way renderer appends the DOCUMENT-LEVEL trailing-sectPr FormatChanged revision (it compares the two docs' final section blocks). So a section-changing reviewer with N ops emitted the section revision N times (6x in a 2-paragraph+section edit). New internal IrDiffSettings.EmitTrailingSectionRevision (default true, two-way unaffected) is set false for the composite per-op mini-scripts; the composite renderer emits the section revision EXACTLY ONCE from script.TrailingSectPr, attributed to the section winner. Pinned.
…s-shell conflict + shell revisions (adversarial review) Adversarial silent-drop review found two correctness holes in ComposeTableAndRowShells's per-row shell composition, both when a reviewer restructures rows AND another touches the table: - Finding A (HIGH): row shells paired POSITIONALLY guarded only by row count. A co-toucher's row delete/insert either dropped a trPr/tblPrEx change on a kept row (count mismatch) or, worse, mis-paired it to the wrong row and stamped a PHANTOM w:trPrChange corrupting the accept output (net-zero restructure). Fixed with ResolveRightRowForBaseRow: anchor-aware right-row resolution (a content toucher maps via its TableDiff row op; a pure-shell FormatOnly toucher is content-equal so positional is sound) — a co-toucher's restructure can never shift the pairing. - Finding B (MEDIUM): a pure-shell reformat of a row another reviewer DELETES was attached to the DeleteRow op and silently dropped. Now a delete-vs-reformat records a conflict (never silent); the delete wins. - Finding C: multi-reviewer table-shell changes (AuthoredRows path) + the trailing section were absent from GetConsolidatedRevisions. The composite revision renderer now emits Table/TableRow/TableCell shell revisions (digest-guarded for cells) attributed to each winner, matching two-way GetRevisions. Pinned by 4 new tests (row-delete+shell, delete-vs-shell conflict, multi-reviewer shell revisions).
…ral column changes (round-trip review) Adversarial round-trip review found tblGrid was composed purely on TblGridDigest inequality with no column-count guard. A structural column ADD/REMOVE also changes TblGridDigest, so B2 misclassified it as a grid-PROPERTY change and composed it independently — leaving the accepted grid's gridCol count disagreeing with the tc count (a malformed table Word would auto-repair). reject == base always held; the break was accept-side. Now tblGrid composes as a property change only for a reviewer whose COLUMN COUNT matches base (a pure gridCol-width edit); a column add/remove is left to ComposeTableDiffs (native w:cellIns/w:cellDel), which owns the grid — no spurious w:tblGridChange. Pinned by two tests (column-add emits no tblGridChange; pure width change still composes). The other 6 round-trip hunts verified SAFE.
…n reviewer-inserted notes A reference nested inside a reviewer-INSERTED note's definition body was never rewritten from the reviewer's id space to the output id space. The body-reference rewrite (step 2) only visits body clones and the renumber sweep (step 4) keys on the output-old id, so a reviewer inserting a footnote whose text cites an endnote the same reviewer ALSO inserts — whose target becomes a fresh output id, not a base id — left the nested reference on the reviewer id and dangling on merge/accept/reject. IrCompositeMarkupRenderer.ApplyCompositeNoteDiffs now rewrites the nested references inside every reviewer-inserted note definition (all-ins, single-id-space content) through the same outputId map, before the body-order renumber carries them to their final ids. Base-matched notes already worked and are untouched. New IrCompositeCrossKindNoteTests: base cross-kind nesting survives the N-way renumber (x3 policies); inserted-fn-cites-existing-en and inserted-en-cites- existing-fn; and inserted-fn-cites-inserted-en (the previously-dangling case). Each asserts structural reference resolvability on merge/accept/reject; a schema check runs alongside (excluding Sem_MissingReferenceElement, which OpenXmlValidator false-positives on cross-part note-body refs). Non-vacuity confirmed by sabotaging the cross-kind remap. Full suite 2640/0/3. Closes the last untested corner of the note-scope Consolidate merge.
JSv4
force-pushed
the
feat/diff-block-format-consolidate-b2
branch
from
83bf62d to
077d11c
Compare
JSv4
changed the title
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
1 participant
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.
Summary
Builds on Consolidate B1 (paragraph
w:pPrmerge, already inmainvia #253) to close the last "Consolidate ignores block-format" ceiling, plus fixes a cross-kind note-nesting corruption in the N-way note merge.DocxDiff.Consolidatenow merges reviewers' table-shell and section format changes (B2), and every family holds the round-trip contract at the property-byte level:reject ≡ base,accept ≡ the policy-resolved composite.1. Consolidate B2 — reviewers' table-shell and section changes now MERGE
w:tcPrChange/w:trPrChange/w:tblPrChange/w:tblGridChange/w:tblPrExChange) composed per element (per-celltcPr, per-rowtrPr+tblPrEx, per-tabletblPr+tblGrid): 0 changers → base, all agree → consensus, ≥2 distinct → a recordedDocxDiffConflictresolved by policy (a shell cannot stack). Disjoint edits compose; only a genuinely-contested element conflicts. Composes cleanly with feat(diff): close the N-way Consolidate gap — note merge, column composition, native split/merge/row-moves, cell-shell visibility #250's column add/remove and row-move. Also fixes a feat(diff): close the N-way Consolidate gap — note merge, column composition, native split/merge/row-moves, cell-shell visibility #250-era gap where a composed cell's shell was swapped in with now:tcPrChangemarker (so reject kept the reviewer's shell) — the marker is now stamped with inner = base.w:sectPrChange, trailing + inline) viaComposeTrailingSection(document-finalw:sectPr) / B1's paragraph path (inline in-pPrsectPr).TrackBlockFormatChangesumbrella is sliced into paragraph/table/section flags; the composite turns all three ON while forcing the umbrella OFF, so two-wayCompareis byte-identical.Docs.ShellSectionbyte-level canonical projection (every bodyw:tcPr/trPr/tblPr/tblGrid/tblPrEx+ trailingw:sectPr) asserted alongside the text projections inCompositeFuzzTests+ConsolidateBlockFormatB2Tests— without it a lost shell passed silently.trPrchange → phantomw:trPrChangecorrupting accept), atblGridcolumn-count guard (a structural column-add was misclassified as a grid-property change → malformed accept), and single-emit of the trailing-section revision (was double-emitting once per op).pPris conflict-routed, not inline-composed (theParagraphPropsUnchangedguard keeps the collision out of token-composition — a recorded conflict, never a silent format drop). True inline compose deferred.2. Cross-kind note nesting fix (Consolidate M-A #4)
A reference nested inside a reviewer-inserted note's body was never rewritten from the reviewer's id space to the output id space, so a reviewer inserting a footnote citing an endnote they also insert (target → a fresh output id) left the nested reference dangling on merge/accept/reject.
ApplyCompositeNoteDiffsnow rewrites inserted-note nested refs through theoutputIdmap before the body-order renumber. NewIrCompositeCrossKindNoteTests(base cross-kind nesting survives the N-way renumber across all policies; inserted-cites-existing both directions; inserted-cites-inserted, the fixed dangler), non-vacuity confirmed by sabotaging the remap.Oracle note: note-in-note references are valid OOXML but LibreOffice Writer's DOCX import cannot load any document containing one (cross-kind or same-kind) — verified against the raw base fixtures. So the oracle there is structural reference-resolvability + Word, not LibreOffice; the fix's value is "no silent dangling reference on a valid input," consistent with Consolidate's never-a-silent-drop principle. The renderable note-merge path (footnotes + endnotes at body level) is LibreOffice-render-verified end-to-end.
Verification
main's B1 baseline + the 15 other commitsmaingained, incl. .NET 10).ConsolidateParityScoreboard+ 3/4/5-wayCompositeFuzz(byte-levelDocs.ShellSectionguard) +IrVsWmlComparerratchet green;OpenXmlValidatorclean; headless-LibreOffice render oracle on the renderable paths.main's B1 with zero conflicts (main's B1 ≡ the B1 this work was authored on); only doc[Unreleased]merges needed resolution.v1 decisions (pinned, never a silent drop)
Ripple
No new serialized public surface (
FormatChange.Scopeand theTrackBlockFormatChangesopt-out already landed in #252/#253; shell attributions and note-ref rewrites are render-only). CHANGELOG / CLAUDE.md /docs/architecture/ir_diff_engine.mdupdated.