From d89975ee0052d3a8c4e08aa7283fb75a8907367a Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sat, 4 Jul 2026 06:10:20 -0500 Subject: [PATCH 1/8] =?UTF-8?q?feat(diff):=20Consolidate=20B2=20Phase=200+?= =?UTF-8?q?1=20=E2=80=94=20table-shell=20N-way=20merge=20(tcPr/trPr/tblPr/?= =?UTF-8?q?tblGrid/tblPrEx)=20+=20byte-level=20verifiers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Ir/Diff/BlockFormatChangeTests.cs | 33 ++- Docxodus.Tests/Ir/Diff/CompositeFuzzTests.cs | 15 +- .../Ir/Diff/ConsolidateBlockFormatB2Tests.cs | 272 ++++++++++++++++++ Docxodus.Tests/Ir/Diff/Docs.cs | 99 +++++++ Docxodus/DocxDiff.cs | 7 +- Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs | 2 +- Docxodus/Ir/Diff/IrCompositeMerger.cs | 154 +++++++++- .../Ir/Diff/IrCompositeRevisionRenderer.cs | 2 +- Docxodus/Ir/Diff/IrCompositeScript.cs | 27 +- Docxodus/Ir/Diff/IrDiffSettings.cs | 21 ++ Docxodus/Ir/Diff/IrMarkupRenderer.cs | 88 +++++- Docxodus/Ir/Diff/IrRevisionRenderer.cs | 4 +- 12 files changed, 687 insertions(+), 37 deletions(-) create mode 100644 Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs diff --git a/Docxodus.Tests/Ir/Diff/BlockFormatChangeTests.cs b/Docxodus.Tests/Ir/Diff/BlockFormatChangeTests.cs index c71cc160..746c8e58 100644 --- a/Docxodus.Tests/Ir/Diff/BlockFormatChangeTests.cs +++ b/Docxodus.Tests/Ir/Diff/BlockFormatChangeTests.cs @@ -783,10 +783,12 @@ public void TableFamily_and_pPr_outputs_are_schema_valid() } [Fact] - public void Consolidate_ignores_table_shell_changes_v1_ceiling() + public void Consolidate_merges_table_shell_changes_b2() { - // Review finding 1: the table-shell REVISION emitters must respect the Consolidate ceiling too — a - // reviewer's tcPr/trPr/tblPr-only edit produces neither markup nor a consolidated revision. + // Sub-project B2 (flips the former ceiling pin): a reviewer's table-shell (tblPr/tblGrid) change now + // MERGES into the consolidated document with native w:tblPrChange/w:tblGridChange authored to that + // reviewer, a non-Run consolidated revision, and a property-byte round-trip (accept ≡ reviewer, + // reject ≡ base). var baseDoc = IrTestDocuments.FromBodyXml(Table("", "")); var reviewerDoc = IrTestDocuments.FromBodyXml(Table("", "", grid: "", tblPr: "")); @@ -794,11 +796,17 @@ public void Consolidate_ignores_table_shell_changes_v1_ceiling() var merged = DocxDiff.Consolidate(baseDoc, new[] { reviewer }); var body = BodyOf(merged); - Assert.Empty(body.Descendants(W + "tblPrChange")); - Assert.Empty(body.Descendants(W + "tblGridChange")); + Assert.NotEmpty(body.Descendants(W + "tblPrChange")); + Assert.NotEmpty(body.Descendants(W + "tblGridChange")); + Assert.Equal("Reviewer A", (string?)body.Descendants(W + "tblPrChange").First().Attribute(W + "author")); + + // accept ≡ reviewer (grid 6000), reject ≡ base (grid restored). + Assert.Equal("6000", (string?)BodyOf(RevisionProcessor.AcceptRevisions(merged)) + .Descendants(W + "gridCol").First().Attribute(W + "w")); + Assert.Empty(BodyOf(RevisionProcessor.RejectRevisions(merged)).Descendants(W + "tblGridChange")); var revs = DocxDiff.GetConsolidatedRevisions(baseDoc, new[] { reviewer }); - Assert.DoesNotContain(revs, r => r.FormatChange is { } fc && fc.Scope != DocxDiffFormatChangeScope.Run); + Assert.Contains(revs, r => r.FormatChange is { } fc && fc.Scope == DocxDiffFormatChangeScope.Table); } [Fact] @@ -1074,11 +1082,12 @@ public void StyleDefinition_only_difference_with_identical_direct_pPr_stays_Unch } [Fact] - public void Consolidate_merges_pPr_but_not_shell_section_v1() + public void Consolidate_merges_pPr_and_table_shell_but_not_section_v1() { - // Sub-project B1 (flips the former ceiling pin): a reviewer's PARAGRAPH-property (pPr) change now - // MERGES into the consolidated document with native w:pPrChange authored to that reviewer, round-trips - // (accept ≡ reviewer, reject ≡ base). A TABLE-shell reviewer change stays IGNORED (the B2 ceiling). + // Sub-project B1/B2 (flips the former ceiling pin): a reviewer's PARAGRAPH-property (pPr) change (B1) + // and TABLE-shell change (B2) now MERGE into the consolidated document with native markup authored to + // that reviewer. A SECTION-only reviewer change stays the remaining B2 ceiling flipped in Phase 2 — + // see the section tests for its merge. var reviewer = new DocxDiffReviewer { Document = PPrRight, Author = "Reviewer A" }; var merged = DocxDiff.Consolidate(PPrLeft, new[] { reviewer }); var body = BodyOf(merged); @@ -1090,11 +1099,11 @@ public void Consolidate_merges_pPr_but_not_shell_section_v1() Assert.Equal("center", (string?)BodyOf(RevisionProcessor.AcceptRevisions(merged)).Descendants(W + "jc").Single().Attribute(W + "val")); Assert.Empty(DocxDiff.GetConflicts(PPrLeft, new[] { reviewer })); // one reviewer → no conflict - // B2 ceiling: a table-shell-only reviewer edit is still ignored by Consolidate. + // B2: a table-shell-only reviewer edit now MERGES (native w:tblGridChange authored to the reviewer). var tblBase = IrTestDocuments.FromBodyXml(Table("", "")); var tblRev = IrTestDocuments.FromBodyXml(Table("", "", grid: "")); var tblMerged = DocxDiff.Consolidate(tblBase, new[] { new DocxDiffReviewer { Document = tblRev, Author = "R" } }); - Assert.Empty(BodyOf(tblMerged).Descendants(W + "tblGridChange")); + Assert.NotEmpty(BodyOf(tblMerged).Descendants(W + "tblGridChange")); } // ------------------------------------------------------------------ the WmlComparer oracle diff --git a/Docxodus.Tests/Ir/Diff/CompositeFuzzTests.cs b/Docxodus.Tests/Ir/Diff/CompositeFuzzTests.cs index b588ca54..22e12ea7 100644 --- a/Docxodus.Tests/Ir/Diff/CompositeFuzzTests.cs +++ b/Docxodus.Tests/Ir/Diff/CompositeFuzzTests.cs @@ -17,7 +17,11 @@ public void Composite_round_trips_reject_equals_base(int reviewerCount) .Select(r => new DocxDiffReviewer { Document = new WmlDocument("r.docx", r.Doc), Author = r.Author }) .ToList(); var merged = DocxDiff.Consolidate(baseDoc, reviewers); - Assert.Equal(Docs.PlainText(baseDoc), Docs.PlainText(RevisionProcessor.RejectRevisions(merged))); + var rejected = RevisionProcessor.RejectRevisions(merged); + Assert.Equal(Docs.PlainText(baseDoc), Docs.PlainText(rejected)); + // Byte-level: rejecting all revisions restores every block-format shell/section too (B2). The + // fuzzer mutates no shells, so this is a regression guard — a future shell-drop turns it red. + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(rejected)); } } @@ -38,6 +42,7 @@ public void Composite_round_trips_structurally_with_tables_and_footnotes(int rev // not just the body paragraph text. Docs.StructuralBody walks body w:p AND w:tbl (descending // into rows/cells), so a consolidate that corrupts or drops a table on the reject path differs. Assert.Equal(Docs.StructuralBody(baseDoc), Docs.StructuralBody(rejected)); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(rejected)); } } @@ -124,10 +129,10 @@ public void Composite_disjoint_table_cells_round_trip(int reviewerCount) var merged = DocxDiff.Consolidate( baseDoc, dd, new DocxDiffConsolidateSettings { ConflictResolution = ConflictResolution.BaseWins }); - // (a) reject ≡ base, table-aware (StructuralBody descends rows/cells). - Assert.Equal( - Docs.StructuralBody(baseDoc), - Docs.StructuralBody(RevisionProcessor.RejectRevisions(merged))); + // (a) reject ≡ base, table-aware (StructuralBody descends rows/cells) AND byte-level shell/section. + var rejected = RevisionProcessor.RejectRevisions(merged); + Assert.Equal(Docs.StructuralBody(baseDoc), Docs.StructuralBody(rejected)); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(rejected)); // (b) table-aware apply-verifier: the composed table's cell ops reconstruct the rendered accept. IrCompositeVerifier.Verify(baseDoc, revs, ConflictResolution.BaseWins, Docs.AcceptStructuralBody(merged)); diff --git a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs new file mode 100644 index 00000000..c63dad00 --- /dev/null +++ b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs @@ -0,0 +1,272 @@ +#nullable enable + +using System.Collections.Generic; +using System.Linq; +using Docxodus; +using Docxodus.Tests.Ir; +using Xunit; + +namespace Docxodus.Tests.Ir.Diff; + +/// +/// Consolidate B2 — N-way merge of the table-shell (w:tcPrChange/w:trPrChange/w:tblPrChange/ +/// w:tblGridChange/w:tblPrExChange) and section (w:sectPrChange) block-format families, +/// plus the text+format safety rule. Before B2 these edits were the pinned Consolidate ceiling: a reviewer's +/// shell/section-only edit was ignored (silently dropped from accept) or, for a cell shell, swapped in with no +/// marker (so reject ≠ base). These tests assert the NEW behavior at the PROPERTY-BYTE level via +/// — the format-blind text projections (/ +/// ) cannot see a lost shell. +/// +public class ConsolidateBlockFormatB2Tests +{ + // ------------------------------------------------------------------ fixtures + + // A 1-row, 2-cell table with explicit tblPr/tblGrid/trPr/tcPr shells, framed by a lead paragraph and a + // tail paragraph + trailing sectPr, so every block-format family has a distinct, mutable shell. + private static string Body( + string tblW = "5000", string trHeight = "300", string gridCol0 = "2500", + string tcW00 = "2500", string tblPrEx = "", string pgMarTop = "1440") => + "lead" + + "" + + $"" + + $"" + + "" + + tblPrEx + + $"" + + $"a" + + "b" + + "" + + "" + + "tail" + + $""; + + private static WmlDocument Base() => IrTestDocuments.FromBodyXml(Body()); + + // ------------------------------------------------------------------ consolidate helpers + + private static WmlDocument Consolidate(WmlDocument baseDoc, ConflictResolution policy, + params (string Author, WmlDocument Doc)[] reviewers) + => DocxDiff.Consolidate(baseDoc, + reviewers.Select(r => new DocxDiffReviewer { Author = r.Author, Document = r.Doc }).ToList(), + new DocxDiffConsolidateSettings { ConflictResolution = policy }); + + private static IReadOnlyList Conflicts(WmlDocument baseDoc, ConflictResolution policy, + params (string Author, WmlDocument Doc)[] reviewers) + => DocxDiff.GetConflicts(baseDoc, + reviewers.Select(r => new DocxDiffReviewer { Author = r.Author, Document = r.Doc }).ToList(), + new DocxDiffConsolidateSettings { ConflictResolution = policy }); + + private static WmlDocument Accept(WmlDocument merged) => RevisionAccepter.AcceptRevisions(merged); + private static WmlDocument Reject(WmlDocument merged) => RevisionProcessor.RejectRevisions(merged); + private static string Xml(WmlDocument d) => Docs.MainPartXml(d); + + /// The core B2 property-byte round-trip for a SINGLE reviewer who changed exactly one shell/section: + /// accept ≡ the reviewer (winner), reject ≡ base, and the native change marker is present. + private static void AssertSingleReviewerMerge(WmlDocument baseDoc, WmlDocument reviewer, string changeMarker) + { + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", reviewer)); + + // Native markup emitted. + Assert.Contains(changeMarker, Xml(merged)); + // accept ≡ winner and reject ≡ base at the property-byte level (shells + section). + Assert.Equal(Docs.ShellSection(reviewer), Docs.ShellSection(Accept(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + // Text is untouched by a shell/section-only edit. + Assert.Equal(Docs.StructuralBody(baseDoc), Docs.StructuralBody(Reject(merged))); + } + + // ------------------------------------------------------------------ Phase 1: table-shell single-reviewer merge + + [Fact] + public void Cell_tcPr_only_edit_merges_with_marker_and_round_trips() + => AssertSingleReviewerMerge(Base(), IrTestDocuments.FromBodyXml(Body(tcW00: "3000")), "w:tcPrChange"); + + [Fact] + public void Row_trPr_only_edit_merges_with_marker_and_round_trips() + => AssertSingleReviewerMerge(Base(), IrTestDocuments.FromBodyXml(Body(trHeight: "500")), "w:trPrChange"); + + [Fact] + public void Table_tblPr_only_edit_merges_with_marker_and_round_trips() + => AssertSingleReviewerMerge(Base(), IrTestDocuments.FromBodyXml(Body(tblW: "6000")), "w:tblPrChange"); + + [Fact] + public void Table_tblGrid_only_edit_merges_with_marker_and_round_trips() + => AssertSingleReviewerMerge(Base(), IrTestDocuments.FromBodyXml(Body(gridCol0: "3000")), "w:tblGridChange"); + + [Fact] + public void Row_tblPrEx_only_edit_merges_with_marker_and_round_trips() + => AssertSingleReviewerMerge( + Base(), + IrTestDocuments.FromBodyXml(Body(tblPrEx: "")), + "w:tblPrExChange"); + + // ------------------------------------------------------------------ Phase 2: section single-reviewer merge + + [Fact] + public void Trailing_sectPr_only_edit_merges_with_marker_and_round_trips() + => AssertSingleReviewerMerge(Base(), IrTestDocuments.FromBodyXml(Body(pgMarTop: "2880")), "w:sectPrChange"); + + // ------------------------------------------------------------------ Phase 1/2: consensus + conflict per family + + [Theory] + [InlineData(ConflictResolution.BaseWins)] + [InlineData(ConflictResolution.StackAll)] + public void Agreeing_cell_shell_edits_reach_consensus_no_conflict(ConflictResolution policy) + { + var baseDoc = Base(); + var v = IrTestDocuments.FromBodyXml(Body(tcW00: "3000")); + Assert.Empty(Conflicts(baseDoc, policy, ("Alice", v), ("Bob", v))); + + var merged = Consolidate(baseDoc, policy, ("Alice", v), ("Bob", v)); + Assert.Equal(Docs.ShellSection(v), Docs.ShellSection(Accept(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } + + [Theory] + [InlineData(ConflictResolution.BaseWins)] + [InlineData(ConflictResolution.FirstReviewerWins)] + [InlineData(ConflictResolution.StackAll)] + public void Competing_cell_shell_edits_record_conflict_resolved_per_policy(ConflictResolution policy) + { + var baseDoc = Base(); + var alice = IrTestDocuments.FromBodyXml(Body(tcW00: "3000")); + var bob = IrTestDocuments.FromBodyXml(Body(tcW00: "4000")); + + var conflicts = Conflicts(baseDoc, policy, ("Alice", alice), ("Bob", bob)); + Assert.NotEmpty(conflicts); + var authors = conflicts.SelectMany(c => c.Competitors.Select(x => x.Author)).Distinct().ToList(); + Assert.Contains("Alice", authors); + Assert.Contains("Bob", authors); + + var merged = Consolidate(baseDoc, policy, ("Alice", alice), ("Bob", bob)); + var winner = policy == ConflictResolution.BaseWins ? baseDoc : alice; + Assert.Equal(Docs.ShellSection(winner), Docs.ShellSection(Accept(merged))); + // reject restores base shells regardless of policy. + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } + + [Theory] + [InlineData(ConflictResolution.BaseWins)] + [InlineData(ConflictResolution.FirstReviewerWins)] + [InlineData(ConflictResolution.StackAll)] + public void Competing_trailing_sectPr_edits_record_conflict_resolved_per_policy(ConflictResolution policy) + { + var baseDoc = Base(); + var alice = IrTestDocuments.FromBodyXml(Body(pgMarTop: "2880")); + var bob = IrTestDocuments.FromBodyXml(Body(pgMarTop: "720")); + + Assert.NotEmpty(Conflicts(baseDoc, policy, ("Alice", alice), ("Bob", bob))); + + var merged = Consolidate(baseDoc, policy, ("Alice", alice), ("Bob", bob)); + var winner = policy == ConflictResolution.BaseWins ? baseDoc : alice; + Assert.Equal(Docs.ShellSection(winner), Docs.ShellSection(Accept(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } + + [Fact] + public void Disjoint_families_compose_cell_shell_and_section_both_land() + { + var baseDoc = Base(); + var alice = IrTestDocuments.FromBodyXml(Body(tcW00: "3000")); // cell shell only + var bob = IrTestDocuments.FromBodyXml(Body(pgMarTop: "2880")); // section only + + Assert.Empty(Conflicts(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob))); + + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob)); + // The composite carries Alice's cell shell AND Bob's section. + var expected = IrTestDocuments.FromBodyXml(Body(tcW00: "3000", pgMarTop: "2880")); + Assert.Equal(Docs.ShellSection(expected), Docs.ShellSection(Accept(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } + + // ------------------------------------------------------------------ Phase 1c: multi-reviewer row/table-level shells + + [Theory] + [InlineData(ConflictResolution.BaseWins)] + [InlineData(ConflictResolution.StackAll)] + public void Agreeing_row_shell_edits_reach_consensus_no_conflict(ConflictResolution policy) + { + var baseDoc = Base(); + var v = IrTestDocuments.FromBodyXml(Body(trHeight: "500")); + Assert.Empty(Conflicts(baseDoc, policy, ("Alice", v), ("Bob", v))); + + var merged = Consolidate(baseDoc, policy, ("Alice", v), ("Bob", v)); + Assert.Equal(Docs.ShellSection(v), Docs.ShellSection(Accept(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } + + [Theory] + [InlineData(ConflictResolution.BaseWins)] + [InlineData(ConflictResolution.FirstReviewerWins)] + public void Competing_row_shell_edits_record_conflict(ConflictResolution policy) + { + var baseDoc = Base(); + var alice = IrTestDocuments.FromBodyXml(Body(trHeight: "500")); + var bob = IrTestDocuments.FromBodyXml(Body(trHeight: "700")); + + Assert.NotEmpty(Conflicts(baseDoc, policy, ("Alice", alice), ("Bob", bob))); + + var merged = Consolidate(baseDoc, policy, ("Alice", alice), ("Bob", bob)); + var winner = policy == ConflictResolution.BaseWins ? baseDoc : alice; + Assert.Equal(Docs.ShellSection(winner), Docs.ShellSection(Accept(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } + + [Fact] + public void Disjoint_table_and_row_shells_compose_both_land() + { + var baseDoc = Base(); + var alice = IrTestDocuments.FromBodyXml(Body(tblW: "6000")); // table-level shell only + var bob = IrTestDocuments.FromBodyXml(Body(trHeight: "500")); // row-level shell only + + Assert.Empty(Conflicts(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob))); + + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob)); + var expected = IrTestDocuments.FromBodyXml(Body(tblW: "6000", trHeight: "500")); + Assert.Equal(Docs.ShellSection(expected), Docs.ShellSection(Accept(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } + + [Fact] + public void Cell_shell_and_row_shell_by_different_reviewers_compose() + { + var baseDoc = Base(); + var alice = IrTestDocuments.FromBodyXml(Body(tcW00: "3000")); // cell shell (ModifyBlock table) + var bob = IrTestDocuments.FromBodyXml(Body(trHeight: "500")); // row shell (FormatOnly table) + + Assert.Empty(Conflicts(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob))); + + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob)); + var expected = IrTestDocuments.FromBodyXml(Body(tcW00: "3000", trHeight: "500")); + Assert.Equal(Docs.ShellSection(expected), Docs.ShellSection(Accept(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } + + // ------------------------------------------------------------------ Phase 3: text+format is never silently dropped + + /// + /// A reviewer edits a paragraph's TEXT and its FORMAT (pPr). v1 decision: this is conflict-routed, never a + /// silent format drop. When another reviewer edits a DISJOINT text span of the same paragraph, the two texts + /// do not compose past the format change — the format-carrying block is a RECORDED conflict. + /// + [Fact] + public void Text_plus_pPr_edit_is_recorded_conflict_never_silent_drop() + { + var baseDoc = IrTestDocuments.FromBodyXml( + "The cat sat. The dog ran."); + // Alice edits sentence 1 AND changes alignment left→center. + var alice = IrTestDocuments.FromBodyXml( + "The CAT sat. The dog ran."); + // Bob edits sentence 2 only (no format change). + var bob = IrTestDocuments.FromBodyXml( + "The cat sat. The dog RAN."); + + var conflicts = Conflicts(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob)); + Assert.NotEmpty(conflicts); // Alice's format+text edit must surface as a conflict, not vanish. + + // reject restores the base paragraph exactly (text + pPr). + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob)); + Assert.Equal(Docs.PlainText(baseDoc), Docs.PlainText(Reject(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } +} diff --git a/Docxodus.Tests/Ir/Diff/Docs.cs b/Docxodus.Tests/Ir/Diff/Docs.cs index 0954fa13..f1bc1680 100644 --- a/Docxodus.Tests/Ir/Diff/Docs.cs +++ b/Docxodus.Tests/Ir/Diff/Docs.cs @@ -122,6 +122,105 @@ public static string PlainTextWithTables(WmlDocument d) public static string AcceptStructuralBody(WmlDocument merged) => PlainTextWithTables(RevisionAccepter.AcceptRevisions(merged)); + // ------------------------------------------------------------------ block-format (shell/section) projection + + private static readonly XNamespace WNs = IrTestDocuments.W; + + /// + /// A canonical, document-order projection of every body block-format SHELL and the section properties — + /// the byte-level oracle the format-blind text projections (/) + /// lack. Emits, per body table, its w:tblPr and w:tblGrid; per row, its w:trPr and + /// w:tblPrEx; per cell, its w:tcPr; per paragraph with an inline w:pPr/w:sectPr, that + /// section's properties; and the trailing body w:sectPr's properties. Each shell is normalized by + /// (rsid/unid stripped, w:*Change markers removed, attributes sorted) so that + /// two documents with the same block formatting project to the same string regardless of revision markup or + /// non-semantic id noise. A dropped, retained-but-should-be-reverted, or corrupted shell/section therefore + /// changes the projection — which the text projections cannot see. Used for byte-level + /// reject ≡ base / accept ≡ winner assertions in the consolidate block-format tests. + /// + public static string ShellSection(WmlDocument d) + { + var body = XDocument.Parse(MainPartXml(d)).Root?.Element(WNs + "body"); + if (body is null) + return string.Empty; + + var sb = new StringBuilder(); + foreach (var block in body.Elements()) + { + if (block.Name == WNs + "p") + { + var inlineSect = block.Element(WNs + "pPr")?.Element(WNs + "sectPr"); + if (inlineSect != null) + sb.Append("PSECT{").Append(NormSectProps(inlineSect)).Append("}\n"); + } + else if (block.Name == WNs + "tbl") + { + sb.Append("TBL{tblPr:").Append(NormShell(block.Element(WNs + "tblPr"))) + .Append(";grid:").Append(NormShell(block.Element(WNs + "tblGrid"))).Append("}\n"); + foreach (var tr in block.Elements(WNs + "tr")) + { + var trPr = tr.Element(WNs + "trPr"); + sb.Append("TR{trPr:").Append(NormShell(trPr)) + .Append(";ex:").Append(NormShell(trPr?.Element(WNs + "tblPrEx") ?? tr.Element(WNs + "tblPrEx"))) + .Append("}\n"); + foreach (var tc in tr.Elements(WNs + "tc")) + sb.Append("TC{").Append(NormShell(tc.Element(WNs + "tcPr"))).Append("}\n"); + } + } + else if (block.Name == WNs + "sectPr") + { + sb.Append("SECT{").Append(NormSectProps(block)).Append("}\n"); + } + } + return sb.ToString(); + } + + /// Canonicalize a block-format shell element for byte-level comparison: null/empty → "∅"; + /// otherwise a recursively rsid/unid-stripped, w:*Change-free, attribute-sorted rendering. Empty ≡ + /// absent (mirrors the reader's shell-children digest), so a render→reject cycle's empty <w:trPr/> + /// equals base's absent one. + private static string NormShell(XElement? shell) + { + if (shell is null) + return "∅"; + var norm = Canonicalize(shell); + return norm is null || !norm.HasElements && !norm.HasAttributes ? "∅" : norm.ToString(SaveOptions.DisableFormatting); + } + + /// Canonicalize the SECTION properties (a w:sectPr) excluding header/footer references and + /// the change marker — matching the two-way engine's IsSectPrProp contract, since those references are + /// owned by the header/footer machinery and are outside the tracked w:sectPrChange. + private static string NormSectProps(XElement sectPr) + { + var props = new XElement(WNs + "sectPr", + sectPr.Elements().Where(e => + e.Name != WNs + "headerReference" && + e.Name != WNs + "footerReference" && + e.Name != WNs + "sectPrChange")); + var norm = Canonicalize(props); + return norm is null ? "∅" : norm.ToString(SaveOptions.DisableFormatting); + } + + /// Recursively strip w:rsid* / pt:*(unid) attributes and w:*Change child + /// elements, and sort each element's attributes by name — the minimal canonical form for comparing two + /// shells for property-byte equality. + private static XElement Canonicalize(XElement el) + { + var keptAttrs = el.Attributes() + .Where(a => !a.IsNamespaceDeclaration + && !(a.Name.Namespace == WNs && a.Name.LocalName.StartsWith("rsid", System.StringComparison.Ordinal)) + && a.Name.NamespaceName != "http://powertools.codeplex.com/2011") + .OrderBy(a => a.Name.NamespaceName, System.StringComparer.Ordinal) + .ThenBy(a => a.Name.LocalName, System.StringComparer.Ordinal) + .Select(a => new XAttribute(a.Name, a.Value)); + + var keptChildren = el.Elements() + .Where(c => !c.Name.LocalName.EndsWith("Change", System.StringComparison.Ordinal)) + .Select(Canonicalize); + + return new XElement(el.Name, keptAttrs, keptChildren); + } + /// The main document part XML as a string. public static string MainPartXml(WmlDocument d) { diff --git a/Docxodus/DocxDiff.cs b/Docxodus/DocxDiff.cs index e5531862..4c1b645f 100644 --- a/Docxodus/DocxDiff.cs +++ b/Docxodus/DocxDiff.cs @@ -747,9 +747,12 @@ internal IrDiffSettings ToIrDiffSettings() : IrFormatComparison.ModeledOnly, CompareHeadersFooters = CompareHeadersFooters, TrackBlockFormatChanges = TrackBlockFormatChanges, - // The paragraph slice defaults equal to the block flag (two-way behaves identically); only the - // composite diverges them (pPr on, table/section off) — see IrCompositeMerger. + // The three slices default equal to the block flag (two-way behaves identically) — so the public + // opt-out cascades to all of them. Only the composite diverges them (all slices on, umbrella off) + // — see IrCompositeMerger. TrackParagraphFormatChanges = TrackBlockFormatChanges, + TrackTableFormatChanges = TrackBlockFormatChanges, + TrackSectionFormatChanges = TrackBlockFormatChanges, }; } } diff --git a/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs b/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs index 31b54368..9ac785df 100644 --- a/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs +++ b/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs @@ -55,7 +55,7 @@ public static WmlDocument Render( // markup on a composite render — e.g. on a conflict-path winner op. B1 (sub-project B) turns the // PARAGRAPH slice ON so a single-source pPr FormatOnly op stamps w:pPrChange authored to its reviewer; // the table-shell/section slices stay OFF (B2). Mirrors IrCompositeMerger's forcing. - settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true }; + settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true }; // Re-read base + each reviewer WITH provenance (RetainSources=true) + Accept view — the SAME options the // two-way renderer uses — so block anchors in the script resolve to source w:p/w:tbl elements to clone. diff --git a/Docxodus/Ir/Diff/IrCompositeMerger.cs b/Docxodus/Ir/Diff/IrCompositeMerger.cs index b761c0ab..20053081 100644 --- a/Docxodus/Ir/Diff/IrCompositeMerger.cs +++ b/Docxodus/Ir/Diff/IrCompositeMerger.cs @@ -42,7 +42,7 @@ public static IrCompositeScript Merge( // reviewer's shell/section-only edit is still ignored by Consolidate. text+pPr edits keep routing to // the block-level conflict path (only pPr-ONLY edits compose in B1). Pinned by // BlockFormatChangeTests.Consolidate_merges_pPr_but_not_shell_section_v1. - settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true }; + settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true }; // 1. Raw pairwise scripts, NOT yet lowered — so PlanMoves can inspect every reviewer's move groups // against the shared base anchor space before any move is collapsed to del/ins. @@ -1100,11 +1100,15 @@ private static void MergeOneBaseBlock( // row moves are uncontested — otherwise we FALL BACK to the block-level conflict (no silent loss; the // base table is kept under BaseWins and the disagreement is recorded under every policy). An // UNCONTESTED reviewer MovedRow composes (lowered to del+ins rows, the two-way shape). - if (touched.All(e => e.Op.Kind == IrEditOpKind.ModifyBlock && e.Op.TableDiff != null) - && baseIr.AnchorIndex.TryGetValue(anchor, out var baseBlk) && baseBlk is IrTable baseTable + if (baseIr.AnchorIndex.TryGetValue(anchor, out var baseBlk) && baseBlk is IrTable baseTable + && touched.All(e => IsTableContentOrShellEdit(e.Op, reviewers[e.Reviewer].Ir)) && MovedRowsComposable(touched)) { + // Content composition over the CONTENT (ModifyBlock w/TableDiff) touchers — a pure shell + // (FormatOnlyBlock) toucher contributes no row ops, so with only shell touchers ComposeTableDiffs + // yields an all-EqualRow authored table onto which the shell attribution below layers the markup. var reviewerTableDiffs = touched + .Where(e => e.Op.Kind == IrEditOpKind.ModifyBlock && e.Op.TableDiff != null) .Select(e => (e.Reviewer, reviewers[e.Reviewer].Author, e.Op.TableDiff!)) .ToList(); var merged = ComposeTableDiffs( @@ -1112,15 +1116,29 @@ private static void MergeOneBaseBlock( ref nextConflictId, out var tableConflicts, out var authoredRows); conflicts.AddRange(tableConflicts); + // B2: per-element table + row shell composition across ALL touchers (tblPr/tblGrid at table level; + // trPr/tblPrEx per row) — mirrors ComposeCellShell (0→base / agree→consensus / ≥2→conflict per + // policy / non-stackable). Mutates authoredRows in place (attaches row-shell refs) and returns the + // table-level shell attribution. Cell tcPr is already composed inside ComposeTableDiffs. + var tableShell = ComposeTableAndRowShells( + anchor, baseTable, touched, reviewers, policy, settings, ref nextConflictId, conflicts, authoredRows); + Invariant(AssertTilesBaseTable(authoredRows, baseTable), "ComposeTableDiffs: authored rows/cells must tile the base table's rows/cells exactly once."); - var structOp = touched[0].Op with { TableDiff = merged }; + // Representative op: prefer a real content ModifyBlock (its TableDiff carries content truth); else a + // pure-shell toucher, coerced to ModifyBlock so the emitted/serialized op kind is consistent (the + // render dispatches on AuthoredRows, not op kind, and resolves the base table via LeftAnchor). + var repOp = touched.Where(e => e.Op.Kind == IrEditOpKind.ModifyBlock && e.Op.TableDiff != null) + .Select(e => e.Op).FirstOrDefault() ?? touched[0].Op; + var structOp = (repOp.Kind == IrEditOpKind.ModifyBlock ? repOp : repOp with { Kind = IrEditOpKind.ModifyBlock }) + with { TableDiff = merged }; ops.Add(new IrCompositeOp(structOp, "", touched[0].Reviewer, AuthoredTokens: null, ConflictId: tableConflicts.Count > 0 ? tableConflicts[0].Id : (int?)null, SourceRightAnchors: null, - AuthoredRows: IrNodeList.From(authoredRows))); + AuthoredRows: IrNodeList.From(authoredRows), + TableShell: tableShell)); return; } // BLOCK-LEVEL CONFLICT @@ -1593,6 +1611,123 @@ private static IrTableDiff ComposeTableDiffs( return new IrTableDiff(IrNodeList.From(mergedRowOps)); } + /// True when is a table CONTENT edit (a ModifyBlock carrying a TableDiff) or + /// a table SHELL-only edit (a FormatOnlyBlock whose right block is a table) — the two op shapes the B2 + /// unified table composition handles. Anything else on a table base (whole-table delete/insert/move) is not + /// a shell/content edit and falls through to the block-level path. + private static bool IsTableContentOrShellEdit(IrEditOp op, IrDocument reviewerIr) + { + if (op.Kind == IrEditOpKind.ModifyBlock && op.TableDiff != null) + return true; + return op.Kind == IrEditOpKind.FormatOnlyBlock && op.RightAnchor is { } ra + && reviewerIr.AnchorIndex.TryGetValue(ra, out var rb) && rb is IrTable; + } + + /// + /// Compose the table-level (w:tblPr/w:tblGrid) and per-row (w:trPr/w:tblPrEx) + /// SHELLS of a base table across ALL touching reviewers (B2), each element independently by its digest — + /// mirroring (0 changers → base; all agree → the first; ≥2 distinct → a + /// recorded conflict resolved by policy; shells cannot stack). Row shells pair POSITIONALLY (only touchers + /// whose right table has the same row count contribute — a structural row add/remove is composed by + /// , and its shell falls back to base, so reject ≡ base holds). Attaches the + /// per-row attribution onto in place and returns the table-level attribution. + /// + private static IrComposedTableShell? ComposeTableAndRowShells( + string tableAnchor, IrTable baseTable, + List<(int Reviewer, IrEditOp Op)> touched, + IReadOnlyList<(string Author, IrDocument Ir)> reviewers, + Docxodus.ConflictResolution policy, IrDiffSettings settings, + ref int nextConflictId, List conflicts, + List authoredRows) + { + // Resolve each toucher's RIGHT table (by right anchor). Unresolvable → skipped (base shell kept). + var rights = new List<(int Reviewer, string Author, IrTable Right)>(); + foreach (var (reviewer, op) in touched) + if (op.RightAnchor is { } ra + && reviewers[reviewer].Ir.AnchorIndex.TryGetValue(ra, out var rb) && rb is IrTable rt) + rights.Add((reviewer, reviewers[reviewer].Author, rt)); + + string tableText = string.Join(" ", baseTable.Rows.Select(r => RowResultText(r, settings))); + + var tblPr = ComposeShellElement(tableAnchor, + rights.Where(r => !r.Right.TblPrDigest.Equals(baseTable.TblPrDigest)) + .Select(r => (r.Reviewer, r.Author, r.Right.TblPrDigest, r.Right.Anchor.ToString())).ToList(), + policy, ref nextConflictId, conflicts, tableText); + var tblGrid = ComposeShellElement(tableAnchor, + rights.Where(r => !r.Right.TblGridDigest.Equals(baseTable.TblGridDigest)) + .Select(r => (r.Reviewer, r.Author, r.Right.TblGridDigest, r.Right.Anchor.ToString())).ToList(), + policy, ref nextConflictId, conflicts, tableText); + + int rowCount = baseTable.Rows.Count; + for (int i = 0; i < rowCount; i++) + { + var baseRow = baseTable.Rows[i]; + string rowAnchor = baseRow.Anchor.ToString(); + string rowText = RowResultText(baseRow, settings); + // Only touchers whose right table row-count matches base contribute a positionally-paired right row. + var alignedRows = rights.Where(r => r.Right.Rows.Count == rowCount) + .Select(r => (r.Reviewer, r.Author, Row: r.Right.Rows[i])).ToList(); + + var trPr = ComposeShellElement(rowAnchor, + alignedRows.Where(r => !r.Row.TrPrShellDigest.Equals(baseRow.TrPrShellDigest)) + .Select(r => (r.Reviewer, r.Author, r.Row.TrPrShellDigest, r.Row.Anchor.ToString())).ToList(), + policy, ref nextConflictId, conflicts, rowText); + var tblPrEx = ComposeShellElement(rowAnchor, + alignedRows.Where(r => !r.Row.TrPrExDigest.Equals(baseRow.TrPrExDigest)) + .Select(r => (r.Reviewer, r.Author, r.Row.TrPrExDigest, r.Row.Anchor.ToString())).ToList(), + policy, ref nextConflictId, conflicts, rowText); + + var trRef = ToShellRef(trPr); + var exRef = ToShellRef(tblPrEx); + if (trRef != null || exRef != null) + ReplaceRowShellAttribution(authoredRows, rowAnchor, trRef, exRef); + } + + var tblPrRef = ToShellRef(tblPr); + var tblGridRef = ToShellRef(tblGrid); + return tblPrRef != null || tblGridRef != null ? new IrComposedTableShell(tblPrRef, tblGridRef) : null; + } + + /// Compose ONE shell element across its changers (already filtered to digest ≠ base): 0 → base + /// (-1); all agree → the first (lowest-index) changer; ≥2 distinct digests → a recorded conflict resolved + /// by policy (BaseWins → base; else the first changer). Shells cannot stack. + private static (int Reviewer, string? Anchor, string Author) ComposeShellElement( + string baseAnchor, + List<(int Reviewer, string Author, IrHash Digest, string RightAnchor)> changers, + Docxodus.ConflictResolution policy, ref int nextConflictId, List conflicts, + string competitorText) + { + if (changers.Count == 0) + return (-1, null, ""); + changers.Sort((a, b) => a.Reviewer.CompareTo(b.Reviewer)); + bool allAgree = changers.All(c => c.Digest.Equals(changers[0].Digest)); + if (!allAgree) + { + conflicts.Add(new IrConflict(nextConflictId++, baseAnchor, 0, 0, policy, + IrNodeList.From(changers.Select(c => new IrConflictCompetitor(c.Author, competitorText))))); + if (policy == Docxodus.ConflictResolution.BaseWins) + return (-1, null, ""); + } + return (changers[0].Reviewer, changers[0].RightAnchor, changers[0].Author); + } + + private static IrComposedShellRef? ToShellRef((int Reviewer, string? Anchor, string Author) w) => + w.Reviewer >= 0 && w.Anchor is { } a ? new IrComposedShellRef(w.Reviewer, a, w.Author) : null; + + /// Attach the composed trPr/tblPrEx shell attribution onto the authored row op for + /// . + private static void ReplaceRowShellAttribution( + List authoredRows, string rowAnchor, + IrComposedShellRef? trPr, IrComposedShellRef? tblPrEx) + { + for (int i = 0; i < authoredRows.Count; i++) + if (authoredRows[i].BaseRowAnchor == rowAnchor) + { + authoredRows[i] = authoredRows[i] with { TrPr = trPr, TblPrEx = tblPrEx }; + return; + } + } + /// Emit every reviewer's InsertRows slotted after (reviewer order): /// both the merged row op and its authored view. Disjoint inserted rows from different reviewers all /// appear — no insert-vs-insert row conflict (block-insert convention). @@ -1755,7 +1890,8 @@ private static List AuthoredCellsForSingleReviewer( var composed = blockOps.Select(b => EmitOp(b, author, reviewer)).ToList(); result.Add(new IrAuthoredCellOp(baseCellAnchor, IrNodeList.From(composed), ShellSourceReviewer: reviewer, - ShellRightCellAnchor: cellOp.RightCellAnchor)); + ShellRightCellAnchor: cellOp.RightCellAnchor, + ShellAuthor: author)); } else { @@ -1932,7 +2068,8 @@ private static void ComposeOneBaseCell( mergedCellOps.Add(new IrCellOp(baseCellAnchor, baseCellAnchor, IrNodeList.From(cellOps.Select(c => c.Op)))); composedCells.Add(new IrAuthoredCellOp(baseCellAnchor, IrNodeList.From(cellOps), - shellReviewer, shellAnchor)); + shellReviewer, shellAnchor, + ShellAuthor: shellReviewer >= 0 ? reviewers[shellReviewer].Author : "")); } /// Emit one reviewer's single-editor cell (merged op verbatim + authored Content view with the @@ -1945,7 +2082,8 @@ private static void EmitSingleEditorCell( var composed = editor.CellOp.BlockOps!.Select(b => EmitOp(b, editor.Author, editor.Reviewer)).ToList(); composedCells.Add(new IrAuthoredCellOp(baseCellAnchor, IrNodeList.From(composed), ShellSourceReviewer: editor.CellOp.RightCellAnchor != null ? editor.Reviewer : -1, - ShellRightCellAnchor: editor.CellOp.RightCellAnchor)); + ShellRightCellAnchor: editor.CellOp.RightCellAnchor, + ShellAuthor: editor.CellOp.RightCellAnchor != null ? editor.Author : "")); } /// Emit one reviewer's deleted base cell (merged left-only op + authored diff --git a/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs b/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs index 7291707d..fd403665 100644 --- a/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs +++ b/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs @@ -54,7 +54,7 @@ internal static class IrCompositeRevisionRenderer { // Mirror IrCompositeMerger's forcing. B1 turns the PARAGRAPH slice ON so a single-source pPr op reports // a Paragraph-scope FormatChanged authored to its reviewer; table-shell/section slices stay OFF (B2). - settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true }; + settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true }; // Move-source pre-pass over the WHOLE composite script. Single-source ops are each rendered in // their own one-op mini-script (so IrRevisionRenderer honours per-op granularity/author), but a diff --git a/Docxodus/Ir/Diff/IrCompositeScript.cs b/Docxodus/Ir/Diff/IrCompositeScript.cs index 25e39fd5..f90c03b9 100644 --- a/Docxodus/Ir/Diff/IrCompositeScript.cs +++ b/Docxodus/Ir/Diff/IrCompositeScript.cs @@ -61,7 +61,8 @@ internal enum IrAuthoredCellKind /// internal sealed record IrAuthoredCellOp(string? BaseCellAnchor, IrNodeList? ComposedBlockOps, int ShellSourceReviewer = -1, string? ShellRightCellAnchor = null, - IrAuthoredCellKind Kind = IrAuthoredCellKind.Content, string Author = ""); + IrAuthoredCellKind Kind = IrAuthoredCellKind.Content, string Author = "", + string ShellAuthor = ""); /// /// One ROW in a composed multi-reviewer table (FOLLOW-ON B). mirrors @@ -79,7 +80,26 @@ internal sealed record IrAuthoredRowOp( int SourceReviewer, string Author, IrNodeList? ComposedCells, - string? RightRowAnchor = null); + string? RightRowAnchor = null, + IrComposedShellRef? TrPr = null, + IrComposedShellRef? TblPrEx = null); + +/// +/// The per-element ATTRIBUTION of ONE composed block-format shell (Consolidate B2): which reviewer's +/// right-side element supplies the winning shell ( indexes the caller's reviewers +/// list; is that reviewer's tbl:/tr: anchor to re-resolve the shell +/// element), and the to stamp on the emitted w:*Change marker. The renderer +/// swaps in the winner's shell (accept ≡ winner) and stamps the change with inner = BASE shell +/// (reject ≡ base). A shell cannot stack, so a competing edit is a recorded conflict, not two shells. +/// Used for a row's w:trPr/w:tblPrEx () and a table's +/// w:tblPr/w:tblGrid (). +/// +internal sealed record IrComposedShellRef(int Reviewer, string RightAnchor, string Author); + +/// The table-level shell attribution of a composed table (Consolidate B2): the winning reviewer for +/// the table's w:tblPr and, independently, its w:tblGrid (each null when the base shell wins). +/// Carried on . +internal sealed record IrComposedTableShell(IrComposedShellRef? TblPr = null, IrComposedShellRef? TblGrid = null); /// /// An edit op tagged with its contributing reviewer. For a composed multi-reviewer Modify, @@ -109,7 +129,8 @@ internal sealed record IrCompositeOp( IrNodeList? AuthoredTokens = null, int? ConflictId = null, IrNodeList? SourceRightAnchors = null, - IrNodeList? AuthoredRows = null); + IrNodeList? AuthoredRows = null, + IrComposedTableShell? TableShell = null); /// /// One reviewer's competing result for a conflicted span. is the reviewer diff --git a/Docxodus/Ir/Diff/IrDiffSettings.cs b/Docxodus/Ir/Diff/IrDiffSettings.cs index 2923dc66..198075d0 100644 --- a/Docxodus/Ir/Diff/IrDiffSettings.cs +++ b/Docxodus/Ir/Diff/IrDiffSettings.cs @@ -290,6 +290,27 @@ internal sealed record IrDiffSettings /// public bool TrackParagraphFormatChanges { get; init; } = true; + /// + /// DIFF-TIME setting (Consolidate sub-project B2). The TABLE-SHELL slice of + /// : gates ONLY the table-shell property-revision markup + /// (w:tcPrChange/w:trPrChange/w:tblPrChange/w:tblGridChange/w:tblPrExChange), + /// NOT the paragraph or section variants. Defaults equal to (so every + /// two-way call behaves byte-identically — the split is invisible outside the composite). The composite + /// merger + renderers set this TRUE while forcing FALSE, so + /// Consolidate merges reviewers' table-shell changes (B2) with per-element attribution. + /// + public bool TrackTableFormatChanges { get; init; } = true; + + /// + /// DIFF-TIME setting (Consolidate sub-project B2). The SECTION slice of + /// : gates ONLY the section property-revision markup + /// (w:sectPrChange on the trailing body section AND on an inline in-w:pPr section), NOT the + /// paragraph or table-shell variants. Defaults equal to (two-way is + /// byte-identical). The composite merger + renderers set this TRUE while forcing + /// FALSE, so Consolidate merges reviewers' section changes (B2). + /// + public bool TrackSectionFormatChanges { get; init; } = true; + /// /// REVISIONS-SURFACE setting (M2.3 Task 1). Author name stamped on every 's /// . Default "Open-Xml-PowerTools" — copied verbatim from diff --git a/Docxodus/Ir/Diff/IrMarkupRenderer.cs b/Docxodus/Ir/Diff/IrMarkupRenderer.cs index be89013c..38b29527 100644 --- a/Docxodus/Ir/Diff/IrMarkupRenderer.cs +++ b/Docxodus/Ir/Diff/IrMarkupRenderer.cs @@ -832,6 +832,10 @@ internal static void RenderComposedTable( foreach (var pre in baseTbl.Elements().Where(e => e.Name != W.tr)) newTbl.Add(StripUnids(new XElement(pre))); + // B2: the table-level shells (tblPr/tblGrid) were base-cloned above; swap in the composed winner's and + // stamp native w:tblPrChange/w:tblGridChange (inner = base) so a table-shell edit round-trips. + ApplyComposedTableShell(newTbl, baseTbl, op.TableShell, reviewerIrs, state); + foreach (var rowOp in authoredRows) { switch (rowOp.Kind) @@ -839,7 +843,13 @@ internal static void RenderComposedTable( case IrRowOpKind.EqualRow: { if (rowOp.BaseRowAnchor is { } ra && baseRowsByAnchor.TryGetValue(ra, out var src)) - newTbl.Add(StripUnids(new XElement(src))); + { + // A content-Equal row may still carry a composed trPr/tblPrEx shell edit (B2): build it + // and stamp the row-level marker; otherwise it is the base row verbatim. + var newRow = StripUnids(new XElement(src)); + ApplyComposedRowShell(newRow, src, rowOp, reviewerIrs, state); + newTbl.Add(newRow); + } break; } case IrRowOpKind.InsertRow: @@ -973,6 +983,10 @@ private static void EmitComposedModifyRow( foreach (var pre in baseRowSrc.Elements().Where(e => e.Name != W.tc)) newRow.Add(StripUnids(new XElement(pre))); + // B2: swap in the composed winner's trPr/tblPrEx and stamp the row-level marker (inner = base), so a + // row-shell edit that rides alongside cell edits round-trips. + ApplyComposedRowShell(newRow, baseRowSrc, rowOp, reviewerIrs, state); + if (rowOp.ComposedCells is not { } cells) { // No per-cell view: keep the base row verbatim (defensive). @@ -1024,6 +1038,19 @@ private static void EmitComposedModifyRow( foreach (var pre in shellSrc.Elements().Where(e => e.Name != W.p && e.Name != W.tbl)) newCell.Add(StripUnids(new XElement(pre))); + // The winner's shell was swapped in above; stamp a native w:tcPrChange (inner = BASE tcPr) + // attributed to the shell winner, so accept keeps the winner's shell and reject restores the base + // shell BYTES (not just the text). No-op when the reviewer shell is canonically equal to base + // (ShellDiffers short-circuits) or when the base shell was kept (shellSrc == baseCellSrc). B2. + if (!ReferenceEquals(shellSrc, baseCellSrc) && state.Settings.TrackTableFormatChanges) + { + var savedShellAuthor = state.AuthorOverride; + state.AuthorOverride = cellOp.ShellAuthor; + ApplyShellChange(newCell, W.tcPr, W.tcPrChange, baseCellSrc.Element(W.tcPr), state, + idOnly: false, TcPrInnerExclude); + state.AuthorOverride = savedShellAuthor; + } + if (cellOp.ComposedBlockOps is { } blockOps) { var cellSink = new List(); @@ -3482,7 +3509,7 @@ private static XElement MarkRPrForCompare(XElement? pPr) /// private static void ApplyTableLevelShellChanges(XElement newTbl, XElement leftTbl, RenderState state) { - if (!state.Settings.TrackBlockFormatChanges) + if (!state.Settings.TrackTableFormatChanges) return; ApplyShellChange(newTbl, W.tblPr, W.tblPrChange, leftTbl.Element(W.tblPr), state, idOnly: false, TblPrInnerExclude); @@ -3494,7 +3521,7 @@ private static void ApplyTableLevelShellChanges(XElement newTbl, XElement leftTb /// block-format tracking is off. private static void ApplyRowAndCellShellChanges(XElement newRow, XElement leftRow, RenderState state) { - if (!state.Settings.TrackBlockFormatChanges) + if (!state.Settings.TrackTableFormatChanges) return; ApplyShellChange(newRow, W.trPr, W.trPrChange, leftRow.Element(W.trPr), state, idOnly: false, TrPrInnerExclude); @@ -3506,6 +3533,61 @@ private static void ApplyRowAndCellShellChanges(XElement newRow, XElement leftRo ApplyShellChange(newCells[c], W.tcPr, W.tcPrChange, leftCells[c].Element(W.tcPr), state, idOnly: false, TcPrInnerExclude); } + /// Apply ONE composed block-format shell across reviewers (Consolidate B2): swap the winning + /// reviewer's shell into (accept ≡ winner) and stamp the change marker with + /// inner = BASE shell (reject ≡ base), attributed to the winner's author. No-op when + /// is null (base shell kept — already present in host from its base clone) or + /// the table slice is off. + private static void ApplyComposedShell( + XElement host, XElement baseHost, IrComposedShellRef? shellRef, + XName shellName, XName changeName, XName[] innerExclude, bool idOnly, + IReadOnlyList reviewerIrs, RenderState state, + Func findWinnerHost) + { + if (!state.Settings.TrackTableFormatChanges || shellRef == null + || shellRef.Reviewer < 0 || shellRef.Reviewer >= reviewerIrs.Count) + return; + var winnerHost = findWinnerHost(reviewerIrs[shellRef.Reviewer], shellRef.RightAnchor); + if (winnerHost == null) + return; + + // Swap in the winner's shell (accept ≡ winner): drop host's base-cloned shell, insert the winner's in + // schema order; ApplyShellChange then captures the BASE shell into the marker inner (reject ≡ base). + host.Elements(shellName).Remove(); + if (winnerHost.Element(shellName) is { } winnerShell) + InsertShellInSchemaOrder(host, StripUnids(new XElement(winnerShell)), shellName); + + var saved = state.AuthorOverride; + state.AuthorOverride = shellRef.Author; + ApplyShellChange(host, shellName, changeName, baseHost.Element(shellName), state, idOnly, innerExclude); + state.AuthorOverride = saved; + } + + /// Apply the composed TABLE-level shells (tblPr/tblGrid) to a composed table's output element (B2). + private static void ApplyComposedTableShell( + XElement newTbl, XElement baseTbl, IrComposedTableShell? shell, + IReadOnlyList reviewerIrs, RenderState state) + { + if (shell == null) + return; + ApplyComposedShell(newTbl, baseTbl, shell.TblPr, W.tblPr, W.tblPrChange, TblPrInnerExclude, idOnly: false, reviewerIrs, state, FindTableSource); + ApplyComposedShell(newTbl, baseTbl, shell.TblGrid, W.tblGrid, W.tblGridChange, TblGridInnerExclude, idOnly: true, reviewerIrs, state, FindTableSource); + } + + /// Apply the composed ROW-level shells (trPr/tblPrEx) to a composed row's output element (B2). + private static void ApplyComposedRowShell( + XElement newRow, XElement baseRow, IrAuthoredRowOp rowOp, + IReadOnlyList reviewerIrs, RenderState state) + { + ApplyComposedShell(newRow, baseRow, rowOp.TrPr, W.trPr, W.trPrChange, TrPrInnerExclude, idOnly: false, reviewerIrs, state, FindRowSource); + ApplyComposedShell(newRow, baseRow, rowOp.TblPrEx, W.tblPrEx, W.tblPrExChange, TblPrExInnerExclude, idOnly: false, reviewerIrs, state, FindRowSource); + } + + /// The source w:tbl a table anchor resolves to in (tables ARE in the + /// AnchorIndex, unlike rows/cells). + private static XElement? FindTableSource(IrDocument ir, string tableAnchor) => + ir.AnchorIndex.TryGetValue(tableAnchor, out var b) && b is IrTable t ? t.Source.Element : null; + /// /// Core table-shell stamper. already carries the RIGHT shell (from a verbatim /// clone) or none. When the left/right shells differ (canonical, excluding diff --git a/Docxodus/Ir/Diff/IrRevisionRenderer.cs b/Docxodus/Ir/Diff/IrRevisionRenderer.cs index 1a93accc..e6c14df0 100644 --- a/Docxodus/Ir/Diff/IrRevisionRenderer.cs +++ b/Docxodus/Ir/Diff/IrRevisionRenderer.cs @@ -560,7 +560,7 @@ private static void RenderModifyBlock(IrEditOp op, in Context ctx, List Date: Sat, 4 Jul 2026 06:18:52 -0500 Subject: [PATCH 2/8] =?UTF-8?q?feat(diff):=20Consolidate=20B2=20Phase=202?= =?UTF-8?q?=20=E2=80=94=20section=20N-way=20merge=20(w:sectPrChange,=20tra?= =?UTF-8?q?iling=20+=20inline)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Ir/Diff/ConsolidateBlockFormatB2Tests.cs | 26 +++++++++ Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs | 6 +- Docxodus/Ir/Diff/IrCompositeMerger.cs | 57 ++++++++++++++++--- .../Ir/Diff/IrCompositeRevisionRenderer.cs | 2 +- Docxodus/Ir/Diff/IrCompositeScript.cs | 3 +- Docxodus/Ir/Diff/IrMarkupRenderer.cs | 26 ++++++++- Docxodus/Ir/Diff/IrModeledFormat.cs | 5 +- Docxodus/Ir/Diff/IrRevisionRenderer.cs | 4 +- 8 files changed, 111 insertions(+), 18 deletions(-) diff --git a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs index c63dad00..98efd89e 100644 --- a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs +++ b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs @@ -106,6 +106,32 @@ public void Row_tblPrEx_only_edit_merges_with_marker_and_round_trips() public void Trailing_sectPr_only_edit_merges_with_marker_and_round_trips() => AssertSingleReviewerMerge(Base(), IrTestDocuments.FromBodyXml(Body(pgMarTop: "2880")), "w:sectPrChange"); + [Fact] + public void Trailing_sectPr_change_reports_section_consolidated_revision() + { + var revs = DocxDiff.GetConsolidatedRevisions(Base(), + new[] { new DocxDiffReviewer { Author = "Alice", Document = IrTestDocuments.FromBodyXml(Body(pgMarTop: "2880")) } }); + Assert.Contains(revs, r => r.FormatChange is { } fc && fc.Scope == DocxDiffFormatChangeScope.Section && r.Author == "Alice"); + } + + [Fact] + public void Inline_sectPr_change_merges_with_marker_and_round_trips() + { + // A mid-document inline section break (w:pPr/w:sectPr) whose page margin changes rides B1's paragraph + // FormatOnly path (BlockSignature includes the section key under the section slice). + static string InlineBody(string top) => + $"a" + + "b" + + ""; + var baseDoc = IrTestDocuments.FromBodyXml(InlineBody("1440")); + var alice = IrTestDocuments.FromBodyXml(InlineBody("2880")); + + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", alice)); + Assert.Contains("w:sectPrChange", Xml(merged)); + Assert.Equal(Docs.ShellSection(alice), Docs.ShellSection(Accept(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } + // ------------------------------------------------------------------ Phase 1/2: consensus + conflict per family [Theory] diff --git a/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs b/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs index 9ac785df..0f297098 100644 --- a/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs +++ b/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs @@ -55,7 +55,7 @@ public static WmlDocument Render( // markup on a composite render — e.g. on a conflict-path winner op. B1 (sub-project B) turns the // PARAGRAPH slice ON so a single-source pPr FormatOnly op stamps w:pPrChange authored to its reviewer; // the table-shell/section slices stay OFF (B2). Mirrors IrCompositeMerger's forcing. - settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true }; + settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true, TrackSectionFormatChanges = true }; // Re-read base + each reviewer WITH provenance (RetainSources=true) + Accept view — the SAME options the // two-way renderer uses — so block anchors in the script resolve to source w:p/w:tbl elements to clone. @@ -91,6 +91,10 @@ public static WmlDocument Render( ?? throw new DocxodusException("Base document has no w:body."); var trailingSectPr = bodyEl.Elements(W.sectPr).LastOrDefault(); + // B2: stamp w:sectPrChange on the trailing sectPr for a composed section change (winner's + // properties applied, base captured into the marker) — the document-level section merge. + if (trailingSectPr != null) + IrMarkupRenderer.ApplyComposedTrailingSectPr(trailingSectPr, script.TrailingSectPr, reviewerIrs, state); bodyEl.Elements().Where(e => e.Name != W.sectPr).Remove(); if (trailingSectPr != null) { diff --git a/Docxodus/Ir/Diff/IrCompositeMerger.cs b/Docxodus/Ir/Diff/IrCompositeMerger.cs index 20053081..babe473c 100644 --- a/Docxodus/Ir/Diff/IrCompositeMerger.cs +++ b/Docxodus/Ir/Diff/IrCompositeMerger.cs @@ -35,14 +35,13 @@ public static IrCompositeScript Merge( // MergeNoteScopes (simpler — story pairing is by scope, no id-map machinery needed). settings = settings with { CompareHeadersFooters = false }; - // Consolidate block-format merge (sub-project B). B1 merges reviewers' PARAGRAPH-property (w:pPr) - // changes: the paragraph slice is turned ON so per-reviewer diffs surface pPr changes as FormatOnly - // ops (composed by ComposePPr — consensus / conflict) and the composite renderer stamps pPrChange - // authored to the winning reviewer. The TABLE-shell and SECTION slices stay OFF (B2 ceiling) — a - // reviewer's shell/section-only edit is still ignored by Consolidate. text+pPr edits keep routing to - // the block-level conflict path (only pPr-ONLY edits compose in B1). Pinned by - // BlockFormatChangeTests.Consolidate_merges_pPr_but_not_shell_section_v1. - settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true }; + // Consolidate block-format merge (sub-project B). ALL THREE slices are turned ON — the composite merges + // reviewers' PARAGRAPH-property (w:pPr, B1), TABLE-shell (w:tcPr/trPr/tblPr/tblGrid/tblPrEx, B2) and + // SECTION (w:sectPr, B2) format changes with per-element attribution + native markup. The UMBRELLA + // TrackBlockFormatChanges stays FALSE so the shared two-way emit helpers never double-stamp on a + // conflict-path winner (they consult the specific slice). text+pPr edits keep routing to the + // block-level conflict path (never a silent format drop). Pinned by ConsolidateBlockFormatB2Tests. + settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true, TrackSectionFormatChanges = true }; // 1. Raw pairwise scripts, NOT yet lowered — so PlanMoves can inspect every reviewer's move groups // against the shared base anchor space before any move is collapsed to del/ins. @@ -82,7 +81,14 @@ public static IrCompositeScript Merge( var (noteOps, noteIdMaps) = MergeNoteScopes( baseIr, reviewers, rawScripts, policy, settings, conflicts, ref nextConflictId); - return new IrCompositeScript(IrNodeList.From(ops), IrNodeList.From(conflicts), noteOps, noteIdMaps); + // 5. TRAILING SECTION (B2): the document-final w:sectPr is not a body block op (Word compares it at the + // document level), so compose it directly — base vs each reviewer's trailing IrSectionBreak.Format + // (modeled + unmodeled digest), mirroring ComposeCellShell. The winner attribution rides to the + // composite renderer, which stamps w:sectPrChange (inner = base) on the output's trailing sectPr. + var trailingSectPr = ComposeTrailingSection( + baseIr, reviewers, policy, settings, conflicts, ref nextConflictId); + + return new IrCompositeScript(IrNodeList.From(ops), IrNodeList.From(conflicts), noteOps, noteIdMaps, trailingSectPr); } // ---- N-way note-scope merge ---- @@ -1714,6 +1720,39 @@ private static (int Reviewer, string? Anchor, string Author) ComposeShellElement private static IrComposedShellRef? ToShellRef((int Reviewer, string? Anchor, string Author) w) => w.Reviewer >= 0 && w.Anchor is { } a ? new IrComposedShellRef(w.Reviewer, a, w.Author) : null; + /// + /// Compose the document-final (trailing) w:sectPr across reviewers (Consolidate B2). The trailing + /// section is NOT a body block op (Word compares it at the document level), so it is composed here directly: + /// a reviewer is a changer when its trailing 's + /// (modeled page setup + the unmodeled-digest catch-all) differs from base. 0 → base; all agree → the first + /// reviewer; ≥2 distinct → a recorded conflict resolved by policy. The winner attribution rides to the + /// composite renderer, which resolves that reviewer's raw trailing w:sectPr and stamps + /// w:sectPrChange (inner = base, references preserved). + /// + private static IrComposedShellRef? ComposeTrailingSection( + IrDocument baseIr, + IReadOnlyList<(string Author, IrDocument Ir)> reviewers, + Docxodus.ConflictResolution policy, IrDiffSettings settings, + List conflicts, ref int nextConflictId) + { + if (!settings.TrackSectionFormatChanges + || baseIr.Body.Blocks.Count == 0 || baseIr.Body.Blocks[^1] is not IrSectionBreak baseSec) + return null; + + var changers = new List<(int Reviewer, string Author, IrHash Digest, string RightAnchor)>(); + for (int r = 0; r < reviewers.Count; r++) + { + var blocks = reviewers[r].Ir.Body.Blocks; + if (blocks.Count == 0 || blocks[^1] is not IrSectionBreak rsec + || rsec.FormatFingerprint.Equals(baseSec.FormatFingerprint)) + continue; + changers.Add((r, reviewers[r].Author, rsec.FormatFingerprint, rsec.Anchor.ToString())); + } + + return ToShellRef(ComposeShellElement( + baseSec.Anchor.ToString(), changers, policy, ref nextConflictId, conflicts, "§section")); + } + /// Attach the composed trPr/tblPrEx shell attribution onto the authored row op for /// . private static void ReplaceRowShellAttribution( diff --git a/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs b/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs index fd403665..d4367e26 100644 --- a/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs +++ b/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs @@ -54,7 +54,7 @@ internal static class IrCompositeRevisionRenderer { // Mirror IrCompositeMerger's forcing. B1 turns the PARAGRAPH slice ON so a single-source pPr op reports // a Paragraph-scope FormatChanged authored to its reviewer; table-shell/section slices stay OFF (B2). - settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true }; + settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true, TrackSectionFormatChanges = true }; // Move-source pre-pass over the WHOLE composite script. Single-source ops are each rendered in // their own one-op mini-script (so IrRevisionRenderer honours per-op granularity/author), but a diff --git a/Docxodus/Ir/Diff/IrCompositeScript.cs b/Docxodus/Ir/Diff/IrCompositeScript.cs index f90c03b9..733617f2 100644 --- a/Docxodus/Ir/Diff/IrCompositeScript.cs +++ b/Docxodus/Ir/Diff/IrCompositeScript.cs @@ -195,4 +195,5 @@ internal sealed record IrCompositeScript( IrNodeList Operations, IrNodeList Conflicts, IrNodeList? NoteOps = null, - IrNodeList? NoteIdMaps = null); + IrNodeList? NoteIdMaps = null, + IrComposedShellRef? TrailingSectPr = null); diff --git a/Docxodus/Ir/Diff/IrMarkupRenderer.cs b/Docxodus/Ir/Diff/IrMarkupRenderer.cs index 38b29527..17b48002 100644 --- a/Docxodus/Ir/Diff/IrMarkupRenderer.cs +++ b/Docxodus/Ir/Diff/IrMarkupRenderer.cs @@ -150,7 +150,7 @@ public static WmlDocument Render( // references + rsids), stamp native w:sectPrChange — the right properties are applied and the left // properties preserved in the marker. References (owned by the header/footer machinery, which runs // later and mutates them) and any mid-document sectPr inside a pPr are untouched (v1 ceilings). - if (settings.TrackBlockFormatChanges && trailingSectPr != null) + if (settings.TrackSectionFormatChanges && trailingSectPr != null) { var rightTrailingSectPr = wDocRight.MainDocumentPart?.GetXDocument().Root? .Element(W.body)?.Elements(W.sectPr).LastOrDefault(); @@ -3398,7 +3398,7 @@ private static void ApplyBlockFormatChanges(XElement newPara, XElement leftPara, // Two independent slices: pPr (+ mark rPr) gated on the PARAGRAPH flag, the inline sectPr gated on // the block flag (so the composite can stamp pPrChange but not sectPrChange — B1 vs B2). bool trackPPr = state.Settings.TrackParagraphFormatChanges; - bool trackSect = state.Settings.TrackBlockFormatChanges; + bool trackSect = state.Settings.TrackSectionFormatChanges; if (!trackPPr && !trackSect) return; @@ -3588,6 +3588,28 @@ private static void ApplyComposedRowShell( private static XElement? FindTableSource(IrDocument ir, string tableAnchor) => ir.AnchorIndex.TryGetValue(tableAnchor, out var b) && b is IrTable t ? t.Source.Element : null; + /// Stamp w:sectPrChange on the composed output's trailing w:sectPr for a composed + /// section change (Consolidate B2): resolve the winning reviewer's trailing w:sectPr and apply its + /// properties (accept ≡ winner) while capturing the base properties into the marker (reject ≡ base), + /// attributed to the winner's author. Header/footer references are preserved (owned by the hdr/ftr + /// machinery). No-op when the section slice is off or no winner was attributed. + internal static void ApplyComposedTrailingSectPr( + XElement trailingSectPr, IrComposedShellRef? sectRef, + IReadOnlyList reviewerIrs, RenderState state) + { + if (!state.Settings.TrackSectionFormatChanges || sectRef == null + || sectRef.Reviewer < 0 || sectRef.Reviewer >= reviewerIrs.Count) + return; + var winnerSectPr = SourceElement(sectRef.RightAnchor, reviewerIrs[sectRef.Reviewer]); + if (winnerSectPr == null || winnerSectPr.Name != W.sectPr || !SectPrPropsDiffer(trailingSectPr, winnerSectPr)) + return; + + var saved = state.AuthorOverride; + state.AuthorOverride = sectRef.Author; + ApplySectPrChange(trailingSectPr, trailingSectPr, winnerSectPr, state); + state.AuthorOverride = saved; + } + /// /// Core table-shell stamper. already carries the RIGHT shell (from a verbatim /// clone) or none. When the left/right shells differ (canonical, excluding diff --git a/Docxodus/Ir/Diff/IrModeledFormat.cs b/Docxodus/Ir/Diff/IrModeledFormat.cs index 9487f39b..f0ef7490 100644 --- a/Docxodus/Ir/Diff/IrModeledFormat.cs +++ b/Docxodus/Ir/Diff/IrModeledFormat.cs @@ -125,8 +125,9 @@ public static string BlockSignature(IrParagraph paragraph, IrDiffSettings settin sb.Append(ParaKey(paragraph.Format)); } // A3: an inline (in-pPr) sectPr's modeled page setup participates too, so a mid-document - // sectPr-only change classifies FormatOnly instead of Unchanged under ModeledOnly. Section slice. - if (settings.TrackBlockFormatChanges) + // sectPr-only change classifies FormatOnly instead of Unchanged under ModeledOnly. Gated on the + // SECTION slice so the composite can turn inline-section merge ON (B2) independently of paragraph/table. + if (settings.TrackSectionFormatChanges) { sb.Append('§'); sb.Append(SectionKey(paragraph.InlineSectionFormat)); diff --git a/Docxodus/Ir/Diff/IrRevisionRenderer.cs b/Docxodus/Ir/Diff/IrRevisionRenderer.cs index e6c14df0..fe755602 100644 --- a/Docxodus/Ir/Diff/IrRevisionRenderer.cs +++ b/Docxodus/Ir/Diff/IrRevisionRenderer.cs @@ -117,7 +117,7 @@ public static IrNodeList Render( // trailing section formats and, when the MODELED fields differ, append one Section-scope FormatChanged // (mirrors the markup renderer's w:sectPrChange on the trailing sectPr). Appended after all body/note/ // header-footer ops (additive ordering). Excluded from WmlComparerCompatible by the scope filter below. - if (settings.TrackBlockFormatChanges + if (settings.TrackSectionFormatChanges && ctx.Left.Body.Blocks.Count > 0 && ctx.Left.Body.Blocks[^1] is IrSectionBreak lsec && ctx.Right.Body.Blocks.Count > 0 && ctx.Right.Body.Blocks[^1] is IrSectionBreak rsec) { @@ -1367,7 +1367,7 @@ private static bool EmitParagraphScopeFormatChanged(IrEditOp op, in Context ctx, /// private static void EmitInlineSectionFormatChanged(IrEditOp op, in Context ctx, List sink) { - if (!ctx.Settings.TrackBlockFormatChanges || op.LeftAnchor is null || op.RightAnchor is null) + if (!ctx.Settings.TrackSectionFormatChanges || op.LeftAnchor is null || op.RightAnchor is null) return; if (!ctx.Left.AnchorIndex.TryGetValue(op.LeftAnchor, out var lb) || lb is not IrParagraph lp) return; From 970765a5bc566a120320bbfb1425b21e37c7239a Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sat, 4 Jul 2026 06:20:59 -0500 Subject: [PATCH 3/8] =?UTF-8?q?test(diff):=20Consolidate=20B2=20Phase=203?= =?UTF-8?q?=20=E2=80=94=20pin=20text+format=20as=20conflict-routed=20(v1?= =?UTF-8?q?=20decision,=20never=20silent-drop)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Ir/Diff/ConsolidateBlockFormatB2Tests.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs index 98efd89e..2291ced1 100644 --- a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs +++ b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs @@ -295,4 +295,26 @@ public void Text_plus_pPr_edit_is_recorded_conflict_never_silent_drop() Assert.Equal(Docs.PlainText(baseDoc), Docs.PlainText(Reject(merged))); Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); } + + /// + /// The positive case: a SINGLE reviewer editing a paragraph's text AND its pPr tracks BOTH — the text as + /// w:ins/w:del and the pPr as w:pPrChange (no conflict, nothing dropped). Only a CROSS-reviewer text+pPr + /// collision conflict-routes (v1 decision; true inline text+format compose is deferred to B3). + /// + [Fact] + public void Single_reviewer_text_and_pPr_edit_tracks_both_no_conflict() + { + var baseDoc = IrTestDocuments.FromBodyXml( + "The cat sat."); + var alice = IrTestDocuments.FromBodyXml( + "The CAT sat."); + + Assert.Empty(Conflicts(baseDoc, ConflictResolution.BaseWins, ("Alice", alice))); + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", alice)); + Assert.Contains("w:pPrChange", Xml(merged)); + Assert.Equal(Docs.PlainText(alice), Docs.PlainText(Accept(merged))); // text edit lands + Assert.Equal(Docs.ShellSection(alice), Docs.ShellSection(Accept(merged))); // pPr change lands + Assert.Equal(Docs.PlainText(baseDoc), Docs.PlainText(Reject(merged))); // reject ≡ base text + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged)));// reject ≡ base pPr + } } From 927fc26d39d74d104d8e9fe28a56e07c3ae9e87f Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sat, 4 Jul 2026 06:29:37 -0500 Subject: [PATCH 4/8] =?UTF-8?q?docs(diff):=20Consolidate=20B2=20=E2=80=94?= =?UTF-8?q?=20flip=20the=20block-format=20ceiling=20in=20CHANGELOG/CLAUDE/?= =?UTF-8?q?ir=5Fdiff=5Fengine?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- CHANGELOG.md | 9 ++++++++- CLAUDE.md | 4 ++-- docs/architecture/ir_diff_engine.md | 10 +++++++--- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ca70100..3950eced 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,14 @@ All notable changes to this project will be documented in this file. - **Multi-block formatting is now N single-block swaps, not a whole-document re-render.** `DocxEditor`'s multi-block paths (`format`/`setFontSize`/`setFontFamily`/`setAlignment`/`indent`/`pageBreakBefore`/`setParagraphStyle` over a multi-paragraph selection) previously fell back to a full remount — `Save()` + `ConvertDocxToHtmlComplete` + full DOM rebuild, i.e. the full-document convert (~1–2.5 s on a real document) on every ribbon click — where the single-block path swapped just the edited block (~10 ms). The multi-block path now applies each block's op and swaps each edited block in place via the session-attached `RenderBlockHtml` — exactly the single-block path run N times, so rendering is fidelity-identical by construction — and restores the cross-block selection afterward (previously the selection was silently dropped), so consecutive ribbon actions (center, then bold) keep targeting the same range. The full remount is retained where whole-document context is genuinely required: any result touching a list item (numbering continuation), `clearParagraphBorders` (border-div regrouping), and paginated mode (page reflow, until M4 lands a scoped re-paginate). - **`DocxSessionBridge.RenderHtml` — session-attached full-document render.** New WASM export (`DocxSessionOps.RenderHtml`) that renders the live session's current state to the complete anchor-stamped HTML document inside the runtime, replacing the editor remount's `Save()` → marshal bytes to JS → marshal bytes back into WASM round trip (two multi-MB copies per remount on a large document). The option profile matches the editor's `ConvertDocxToHtmlComplete` call exactly and the output is byte-identical (asserted by the new spec); `DocxEditor` falls back to the old bytes path when the export is absent (older WASM bundles). Exposed on `DocxodusWasmExports.DocxSessionBridge` in npm `types.ts`. WASM/npm-only, like `RenderBlockHtml` (not part of the stdio/python surface). - **DocxDiff: `w:gridSpan` / `w:vMerge` property-only table changes — scope closed and pinned (Issue #230).** #230 (an IR-modeling gap where a reviewer's *property-only* table/cell change read as unchanged) was resolved for cell shells by the block-format-change family (native `w:tcPrChange`, `IrCell.ShellDigest` folded into the cell `ContentHash`) and by consolidate cell-shell composition. This change closes the issue's explicit remaining acceptance criterion — **documenting the chosen `gridSpan`/`vMerge` scope** — and adds the direct regression proof that was missing: a `gridSpan`- or `vMerge`-only change (cell count stable, text unchanged) is now proven to be tracked as a native `w:tcPrChange` `TableCell` `FormatChanged` revision in 2-way `Compare` (`accept ≡ right`, `reject ≡ left` at the tcPr-byte level), and to compose in `Consolidate` like a width/shading edit. A `gridSpan` change that alters the cell *count* (column add/remove) is detected (never silently invisible) and composes per-cell in `Consolidate` (`w:cellIns`/`w:cellDel`); the 2-way single-toucher path lowers it to a whole-table del/ins (a pre-existing renderer-granularity limit, not a soundness gap). No engine change — the behavior already held; this adds the fixtures (`BlockFormatChangeTests.{GridSpanOnly,VMergeOnly}_cell_change_is_tracked_with_native_tcPrChange`, `IrCompositeTableTests.VMerge_only_cell_edit_composes`) and the scope decision in `docs/architecture/ir_diff_engine.md`. -- **DocxDiff.Consolidate: reviewers' paragraph-property (`w:pPr`) changes now MERGE (sub-project B1).** Previously a reviewer's paragraph-formatting-only edit was ignored by `Consolidate` (the N-way merge forced block-format tracking off). Now a reviewer's pPr-only change (alignment/indent/spacing/style/numbering, text unchanged) is composed across reviewers with per-reviewer attribution: a new `ComposePPr` (mirroring the existing cell-shell composition) attributes the change by a new `IrParagraph.PPrDigest` — 0 changers → base, all reviewers agree → consensus (first reviewer), ≥2 distinct → a recorded `DocxDiffConflict` resolved by the `ConflictResolution` policy (a pPr cannot stack — one paragraph has one pPr, so `StackAll`/`FirstReviewerWins` apply the first changer). The consolidated document carries native `w:pPrChange` authored to the winning reviewer; **reject ≡ base and accept ≡ the policy-winner hold at the property-byte level** (proven by a multi-reviewer byte-level round-trip stress test). Implemented via a paragraph slice of the internal flag (`TrackParagraphFormatChanges`) so two-way `Compare` behavior is byte-identical and the composite fuzzer (3/4/5-way) + 84-case parity scoreboard are unaffected. **v1 ceilings (sub-project B2, still open):** a reviewer's table-shell (`w:tcPr`/`w:trPr`/`w:tblPr`) or section (`w:sectPr`) only edit is still ignored by `Consolidate`, and a reviewer who changed BOTH a paragraph's text and its pPr still routes to the conflict path (only pPr-only edits compose). Flips the former `Consolidate_ignores_block_format_changes_v1_ceiling` pin. +- **DocxDiff.Consolidate: reviewers' table-shell and section format changes now MERGE (sub-project B2) — closing the last "Consolidate ignores block-format" ceiling.** Building on B1 (paragraph `w:pPr` merge), the N-way merge now composes the remaining block-format families with per-reviewer attribution + native Word markup: + - **Table-shell family** (`w:tcPrChange`/`w:trPrChange`/`w:tblPrChange`/`w:tblGridChange`/`w:tblPrExChange`). Each shell element is attributed **independently** (per-cell `tcPr`, per-row `trPr`+`tblPrEx`, per-table `tblPr`+`tblGrid`) by its digest, mirroring the existing cell-shell composition: 0 changers → base, all reviewers agree → consensus (first reviewer), ≥2 distinct → a recorded `DocxDiffConflict` resolved by policy (a shell cannot stack). Per-element granularity means disjoint edits COMPOSE (one reviewer's `tblPr` + another's `trPr` both land) while only a genuinely-contested element conflicts — and it composes cleanly with #250's column-add/remove and row-move. A single-reviewer shell edit rides the two-way single-source render; a multi-reviewer table routes through the unified `ComposeTableAndRowShells`. This also fixes a #250-era gap where a composed cell's shell was swapped in with **no** `w:tcPrChange` marker (so reject kept the reviewer's shell) — the marker is now stamped with inner = base. + - **Section family** (`w:sectPrChange`, trailing + inline). The document-final `w:sectPr` (not a body block op — Word compares it at the document level) is composed by `ComposeTrailingSection` across each reviewer's trailing `IrSectionBreak` (modeled page setup + the unmodeled-digest catch-all), and the composite renderer stamps `w:sectPrChange` (inner = base, header/footer references preserved). A mid-document inline (`w:pPr/w:sectPr`) section change rides B1's paragraph FormatOnly path. + - **Mechanism.** The internal `TrackBlockFormatChanges` is now sliced into paragraph/table/section flags (`TrackParagraphFormatChanges` from B1 + new `TrackTableFormatChanges`/`TrackSectionFormatChanges`); the public `DocxDiffSettings.TrackBlockFormatChanges` opt-out cascades to all three. The composite turns all three slices ON while forcing the umbrella OFF; **two-way `Compare` is byte-identical** (the slices default equal to the umbrella), proven by the full renderer battery + a 470-case two-way/composite regression. + - **Round-trip.** `reject ≡ base` and `accept ≡ the policy-resolved composite` now hold at the **property-byte** level for every shell/section family — enforced by a strengthened byte-level verifier: a new `Docs.ShellSection` canonical projection (over every body `w:tcPr`/`trPr`/`tblPr`/`tblGrid`/`tblPrEx` + trailing `w:sectPr`) is asserted alongside the previously text-only reject=base checks in `CompositeFuzzTests` (3/4/5-way) and the new `ConsolidateBlockFormatB2Tests` (per-family reject=base/accept=winner/conflict). Without this strengthening a lost shell passed silently. + - **text + format (v1 decision).** A reviewer editing a paragraph's TEXT *and* its `pPr` is **conflict-routed**, not inline-composed: the existing `ParagraphPropsUnchanged` guard keeps a cross-reviewer text+pPr collision out of token-composition (a recorded conflict, never a silent format drop); a single reviewer's text+pPr edit tracks both. True inline text+format compose is deferred to a follow-up. **No silent drop anywhere** — every reviewer format edit is attributed (per-element winner) or recorded as a conflict. + - **Proof.** `ConsolidateBlockFormatB2Tests` (25 cases: per-family single/multi-reviewer merge, consensus, conflict-per-policy, disjoint-compose, mixed cell+row+section, inline sectPr, text+format), the strengthened `CompositeFuzzTests` byte-level guard, the 84-case `ConsolidateParityScoreboardTests` (unchanged, still 84), and `OpenXmlValidator`-clean output. Flips the B1 `Consolidate_merges_pPr_but_not_shell_section_v1` ceiling pin. +- **DocxDiff.Consolidate: reviewers' paragraph-property (`w:pPr`) changes now MERGE (sub-project B1).** Previously a reviewer's paragraph-formatting-only edit was ignored by `Consolidate` (the N-way merge forced block-format tracking off). Now a reviewer's pPr-only change (alignment/indent/spacing/style/numbering, text unchanged) is composed across reviewers with per-reviewer attribution: a new `ComposePPr` (mirroring the existing cell-shell composition) attributes the change by a new `IrParagraph.PPrDigest` — 0 changers → base, all reviewers agree → consensus (first reviewer), ≥2 distinct → a recorded `DocxDiffConflict` resolved by the `ConflictResolution` policy (a pPr cannot stack — one paragraph has one pPr, so `StackAll`/`FirstReviewerWins` apply the first changer). The consolidated document carries native `w:pPrChange` authored to the winning reviewer; **reject ≡ base and accept ≡ the policy-winner hold at the property-byte level** (proven by a multi-reviewer byte-level round-trip stress test). Implemented via a paragraph slice of the internal flag (`TrackParagraphFormatChanges`) so two-way `Compare` behavior is byte-identical and the composite fuzzer (3/4/5-way) + 84-case parity scoreboard are unaffected. **Table-shell and section merge followed in sub-project B2 (above); a reviewer who changed BOTH a paragraph's text and its pPr remains conflict-routed (v1 decision).** Flips the former `Consolidate_ignores_block_format_changes_v1_ceiling` pin. - **DocxDiff block-format-change family — follow-up A: the two-way surface is now complete.** Building on the family that shipped in the same release: - **`w:tblPrExChange`** (row-level table property exceptions) is now tracked (was visible-but-untracked). A new `IrRow.TrPrExDigest` (a flattened `tblPrEx`-only projection) drives a native `w:tblPrExChange` marker + a `TableRow`-scope revision with the distinct `"tblPrEx"` changed-name; reject restores the left bytes. - **Mid-document `w:sectPrChange`** (an inline `w:sectPr` inside a `w:pPr`) is now tracked (was invisible — the inline sectPr's properties weren't modeled). The reader models it as `IrParagraph.InlineSectionFormat`, folded into the paragraph `FormatFingerprint` **and** `IrModeledFormat.BlockSignature` so a sectPr-only change classifies FormatOnly under `ModeledOnly`; the emit stamps `w:sectPrChange` inside the paragraph's own `w:pPr/w:sectPr` (not the `pPrChange` inner — CT_PPrBase excludes sectPr) with a per-paragraph Section revision. A one-sided add/remove is structural (untracked). diff --git a/CLAUDE.md b/CLAUDE.md index c9116218..a27e043e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -248,10 +248,10 @@ Format change detection produces **native Word format change markup** (`w:rPrCha - `GetRevisions(left, right, settings?)` → `IReadOnlyList` — consumer revisions rendered off the edit script - `GetEditScriptJson(left, right, settings?)` → `string` — the edit script as data (the differentiator vs `WmlComparer`) - Accept/reject primitive (the round-trip verifier, surfaced for clients via `DocxDiffOps.AcceptRevisions`/`RejectRevisions` → WASM `DocxDiffBridge.AcceptRevisions`/`RejectRevisions` + npm `docxDiffAcceptRevisions`/`docxDiffRejectRevisions`, stdio `docx_diff_accept_revisions`/`docx_diff_reject_revisions` + docx-scalpel `docx_diff_accept_revisions`/`docx_diff_reject_revisions`): byte→byte materialize the right (accept) / left (reject) side of a redline, so a client can prove `accept(Compare(left,right))` ≡ `right` and `reject` ≡ `left` at the per-block text level — wraps `RevisionProcessor` -- N-way composite / consolidate (closes the last WmlComparer gap): `Consolidate(base, reviewers, settings?)` → `WmlDocument`, plus `GetConsolidatedRevisions`/`GetConsolidatedEditScriptJson`/`GetConflicts` — merge N `DocxDiffReviewer{Document,Author}` (each diffed against ONE shared base) into one multi-author tracked-changes document with per-reviewer attribution, token-granular sub-block merge, and a structured `DocxDiffConflict` report; `DocxDiffConsolidateSettings.ConflictResolution` = `BaseWins`(default)/`FirstReviewerWins`/`StackAll`. Round-trip: reject ≡ base, accept ≡ the policy-resolved composite. Structurally complete: note-scope (footnote/endnote) diffs merge across reviewers (compose/consensus/conflict per block, inserted notes under fresh ids, N-reviewer-aware renumbering), table column add/remove composes with native `w:cellIns`/`w:cellDel` markup, cell-shell (`w:tcPr`) edits are visible and composable, uncontested split/merge/move/row-move ops render natively (colliding ones lower to del/ins with a recorded conflict — never a silent drop), and **reviewers' paragraph-property (`w:pPr`) changes MERGE (sub-project B1: `ComposePPr` by `IrParagraph.PPrDigest` — consensus/conflict, native `w:pPrChange` authored to the winner; table-shell/section composite merge is B2, still a ceiling)**. Wraps `Docxodus/DocxDiffConsolidate.cs` + `Docxodus/Ir/Diff/IrCompositeMerger.cs`; surfaced in WASM/npm (`docxDiffConsolidate*`)/docx-scalpel (`docx_diff_consolidate*`) +- N-way composite / consolidate (closes the last WmlComparer gap): `Consolidate(base, reviewers, settings?)` → `WmlDocument`, plus `GetConsolidatedRevisions`/`GetConsolidatedEditScriptJson`/`GetConflicts` — merge N `DocxDiffReviewer{Document,Author}` (each diffed against ONE shared base) into one multi-author tracked-changes document with per-reviewer attribution, token-granular sub-block merge, and a structured `DocxDiffConflict` report; `DocxDiffConsolidateSettings.ConflictResolution` = `BaseWins`(default)/`FirstReviewerWins`/`StackAll`. Round-trip: reject ≡ base, accept ≡ the policy-resolved composite. Structurally complete: note-scope (footnote/endnote) diffs merge across reviewers (compose/consensus/conflict per block, inserted notes under fresh ids, N-reviewer-aware renumbering), table column add/remove composes with native `w:cellIns`/`w:cellDel` markup, cell-shell (`w:tcPr`) edits are visible and composable, uncontested split/merge/move/row-move ops render natively (colliding ones lower to del/ins with a recorded conflict — never a silent drop), and **reviewers' block-format changes MERGE — paragraph (B1) + table-shell + section (B2), closing the last "Consolidate ignores block-format" ceiling**: `ComposeParagraphFormat` (by full `BlockSignature`) → `w:pPrChange`; per-element table shells via `ComposeTableAndRowShells` (per-cell `tcPr`, per-row `trPr`+`tblPrEx`, per-table `tblPr`+`tblGrid` — disjoint compose, contested conflict) → `w:tcPr/trPr/tblPr/tblGrid/tblPrExChange`; trailing+inline section via `ComposeTrailingSection`/B1's paragraph path → `w:sectPrChange`; each mirrors `ComposeCellShell` (0→base/agree→consensus/≥2→conflict-per-policy/non-stackable) with `reject ≡ base`/`accept ≡ policy-winner` at the property-byte level (strengthened byte-level verifier `Docs.ShellSection`). text+pPr stays conflict-routed (v1 decision; never a silent drop). Wraps `Docxodus/DocxDiffConsolidate.cs` + `Docxodus/Ir/Diff/IrCompositeMerger.cs`; surfaced in WASM/npm (`docxDiffConsolidate*`)/docx-scalpel (`docx_diff_consolidate*`) - `DocxDiffSettings` mirrors `WmlComparerSettings` defaults (two honest deviations: `Deterministic` revision dates default true; `FormatComparison` defaults `ModeledOnly`). `DocxDiffRevision` adds `LeftAnchor`/`RightAnchor` (`kind:scope:unid`, interoperable with `DocxSession`/markdown projection) - Header/footer stories are compared like Word Compare's default-on "Headers and footers" granularity (`CompareHeadersFooters`, default true) — stories pair per section ordinal × kind with Word's inheritance rule, changed stories get native markup inside their parts (accept ≡ right / reject ≡ left extends to those scopes), Fine revisions carry `hdr`/`ftr` anchors (compatible mode excludes them), JSON gains `headerFooterOps`. WmlComparer ignores headers/footers entirely; Consolidate doesn't merge them in v1 (forced off, pinned) -- Paragraph-and-above formatting changes are tracked as native markup — the block-format-change family (closes the last "Word compares Formatting, we don't" gap): `w:pPrChange` (paragraph: jc/indent/spacing/style/**numbering**, + `w:pPr/w:rPr/w:rPrChange` for a changed mark), `w:tcPrChange`/`w:trPrChange`/`w:tblPrChange`/`w:tblGridChange`/`w:tblPrExChange` (table shells — the per-element digests `IrCell.ShellDigest`/`IrRow.TrPrShellDigest`/`TrPrExDigest`/`IrTable.TblPrDigest`/`TblGridDigest` drive attribution; makes the #250 cell-shell edits *tracked*, not just visible), and `w:sectPrChange` (trailing section AND mid-document inline `w:pPr/w:sectPr` via `IrParagraph.InlineSectionFormat`). Detected via `FormatComparison` for paragraphs (canonical for shells/section); accept ≡ right / reject ≡ left holds at the property-byte level for every detected change. Note/header/footer-scope `w:pPrChange` works via the shared `RenderBlockOp` dispatch (no per-scope gate). `DocxDiffRevision.FormatChange.Scope` (`DocxDiffFormatChangeScope`: Run/Paragraph/TableCell/TableRow/Table/Section) — `WmlComparerCompatible` excludes non-Run scopes (oracle produces none); additive `scope` on the revisions wire. **`TrackBlockFormatChanges` is a public opt-out** (default true; wire `trackBlockFormatChanges`). **Two v1 ceilings: (a) Consolidate forced off, pinned** (`IrCompositeMerger` sets `TrackBlockFormatChanges=false` — sub-project B is the N-way merge); **(b) split/merge members don't emit pPrChange** (deliberate decline — members are new paragraphs already tracked by the pilcrow mark). Rode-along consume-side fix: `RevisionProcessor` no longer drops header/footer refs (sectPrChange) or an inline sectPr (pPrChange) on reject — CT_*Base inners exclude them (see `docs/ooxml_corner_cases.md`) +- Paragraph-and-above formatting changes are tracked as native markup — the block-format-change family (closes the last "Word compares Formatting, we don't" gap): `w:pPrChange` (paragraph: jc/indent/spacing/style/**numbering**, + `w:pPr/w:rPr/w:rPrChange` for a changed mark), `w:tcPrChange`/`w:trPrChange`/`w:tblPrChange`/`w:tblGridChange`/`w:tblPrExChange` (table shells — the per-element digests `IrCell.ShellDigest`/`IrRow.TrPrShellDigest`/`TrPrExDigest`/`IrTable.TblPrDigest`/`TblGridDigest` drive attribution; makes the #250 cell-shell edits *tracked*, not just visible), and `w:sectPrChange` (trailing section AND mid-document inline `w:pPr/w:sectPr` via `IrParagraph.InlineSectionFormat`). Detected via `FormatComparison` for paragraphs (canonical for shells/section); accept ≡ right / reject ≡ left holds at the property-byte level for every detected change. Note/header/footer-scope `w:pPrChange` works via the shared `RenderBlockOp` dispatch (no per-scope gate). `DocxDiffRevision.FormatChange.Scope` (`DocxDiffFormatChangeScope`: Run/Paragraph/TableCell/TableRow/Table/Section) — `WmlComparerCompatible` excludes non-Run scopes (oracle produces none); additive `scope` on the revisions wire. **`TrackBlockFormatChanges` is a public opt-out** (default true; wire `trackBlockFormatChanges`). **Consolidate MERGES all block-format families** (sub-project B done: `IrCompositeMerger` forces the umbrella `TrackBlockFormatChanges` off but turns the paragraph/table/section slices ON — B1+B2). **Remaining v1 ceiling: split/merge members don't emit pPrChange** (deliberate decline — members are new paragraphs already tracked by the pilcrow mark). Rode-along consume-side fix: `RevisionProcessor` no longer drops header/footer refs (sectPrChange) or an inline sectPr (pPrChange) on reject — CT_*Base inners exclude them (see `docs/ooxml_corner_cases.md`) - No static state — `AuthorForRevisions` flows per call (multi-author / consolidate-compatible) - Wraps the internal `Docxodus/Ir/Diff/` pipeline (`IrReader → IrEditScriptBuilder → IrMarkupRenderer/IrRevisionRenderer/IrEditScriptJson`); see `docs/architecture/ir_diff_engine.md` diff --git a/docs/architecture/ir_diff_engine.md b/docs/architecture/ir_diff_engine.md index be1830d6..aa455084 100644 --- a/docs/architecture/ir_diff_engine.md +++ b/docs/architecture/ir_diff_engine.md @@ -102,7 +102,7 @@ A `ModifyBlock` over a paragraph carries a `tokenDiff`; over a table, a `tableDi | `MoveMinimumWordCount` | `3` | `MoveMinimumTokenCount` | | | `RevisionGranularity` | `Fine` | `RevisionGranularity` | `Fine` = engine-native one-revision-per-token-span (byte-stable); `WmlComparerCompatible` = coalesce/trim/prune to match the legacy comparer's coarser revision set | | `FormatComparison` | `ModeledOnly` | `IrFormatComparison` | `ModeledOnly` reports only modeled-field deltas (false-negative on unmodeled rPr); `Full` sees every rPr difference. Governs paragraph-property comparison too (block-format-change family — see below) | -| `TrackBlockFormatChanges` | `true` | `IrDiffSettings.TrackBlockFormatChanges` | public opt-out; detect+track paragraph/table/section property changes (below). Forced OFF by `IrCompositeMerger` — the Consolidate v1 ceiling, pinned — regardless of this value. Wire: `trackBlockFormatChanges` (Ops JSON + npm/python) | +| `TrackBlockFormatChanges` | `true` | `IrDiffSettings.TrackBlockFormatChanges` | public opt-out (cascades to all three slices); detect+track paragraph/table/section property changes (below). Sliced into `TrackParagraphFormatChanges`/`TrackTableFormatChanges`/`TrackSectionFormatChanges` (each defaults equal, so two-way is byte-identical); `IrCompositeMerger` forces the umbrella OFF but turns all three slices ON, so `Consolidate` merges every block-format family (B1+B2). Wire: `trackBlockFormatChanges` (Ops JSON + npm/python) | | `PreAcceptInputRevisions` | `false` | — (a pre-pass, not an `IrDiffSettings` field) | when true, runs `RevisionProcessor.AcceptRevisions` on EACH input before diffing — the first-class "accept-all both sides, then compare" wrapper. See [Inputs that already carry tracked changes](#inputs-that-already-carry-tracked-changes-rule-n13--preacceptinputrevisions). .NET-only in v1 (no bridge ripple). | | `CompareHeadersFooters` | `true` | `CompareHeadersFooters` | diff header/footer stories (Word Compare's own default-on granularity — see the Edit script section). `false` restores the carry-left-verbatim behavior. Wire: `compareHeadersFooters` (Ops JSON + npm/python types). | @@ -146,7 +146,7 @@ Key design points: - **Changed-name granularity:** a cell `w:tcPr` change reports `ChangedPropertyNames = ["shell"]` (the whole `w:tcPr` is one opaque digest); naming `gridSpan`/`vMerge`/width individually would require XML-child diffing the revision path deliberately avoids — by design. **Remaining v1 ceilings (documented + pinned):** -- **`Consolidate` does not track block-format changes** (`IrCompositeMerger` forces `TrackBlockFormatChanges` off — a reviewer's pPr/shell/section-only edit is ignored; text+pPr edits route to the conflict path). This is **sub-project B** (the N-way merge), still open. +- **`Consolidate` MERGES block-format changes** (sub-project B, done): reviewers' paragraph (`w:pPr`, B1), table-shell (`w:tcPr`/`trPr`/`tblPr`/`tblGrid`/`tblPrEx`, B2) and section (`w:sectPr`, B2) format edits compose with per-element attribution + native markup; competing edits conflict per policy. A reviewer who changed BOTH a paragraph's text AND its pPr remains conflict-routed (v1 decision; never a silent drop). See the N-way composite section below. - **Split/merge members do not emit `w:pPrChange`** — a **deliberate, principled decline**: a split's members are brand-new right paragraphs already tracked by the inserted pilcrow mark (there is no per-member left baseline to diff against), and a merge's non-final members carry deleted marks; a pPr "change" on them is not well-defined and would fight the reject-fuse. Pinned by `Split_members_do_not_emit_pPrChange_declined_v1`. Proof: `BlockFormatChangeTests`, `BlockFormatChangeRealDocTests` (a real corpus doc mutated across the full table + section family), and the strengthened `IrMarkupRendererTests` round-trip battery. @@ -246,7 +246,11 @@ The whole-corpus juxtaposition-vs-inline-merge shape divergence is the deliberat - **Note scopes MERGE across reviewers.** The merger builds composite note-scope (footnote/endnote) ops (`IrCompositeMerger.MergeNoteScopes`): a base-matched note's blocks run the SAME `MergeBlockStream` dispatch the body uses (disjoint note edits compose, identical ones reach consensus, contested ones are recorded conflicts resolved by the policy; a whole-note delete vs an edit is a delete-vs-modify conflict), and reviewer-INSERTED notes pass through authored. The composite renderer applies the composed ops inside the footnotes/endnotes parts, creates reviewer-inserted definitions under fresh output ids, rewrites reviewer-sourced body references from each reviewer's id space into the base-anchored output space (`IrCompositeScript.NoteIdMaps` + `RenderState.NoteRefClonesBySource`; deleted/moved-from content is base-sourced and skipped), then runs the SAME body-order renumber + cross-kind nested-reference sweep the two-way renderer runs. Structural ops INSIDE a note (a split/merge/move of note paragraphs) are conservatively lowered to del/ins (content-preserving); native in-note structural composition is a follow-on. Consolidated revisions cover note edits (appended after body ops, footnotes then endnotes), and the parity scoreboard's accept ≡ right metric now includes referenced note texts. - **Multi-reviewer table edits compose per-cell, including column changes and cell shells.** DISJOINT cross-reviewer table-cell edits COMPOSE inline (Alice edits cell(0,0), Bob edits cell(1,2) → both land, attributed; disjoint words inside one cell paragraph fuse via the recursion); edits to the SAME cell by ≥2 reviewers become a recorded conflict resolved by the policy. Cell ops pair by BASE cell anchor, so a reviewer's **column add/remove composes**: an added cell renders with native `w:tcPr/w:cellIns` (kept on accept, removed on reject), a removed cell with `w:tcPr/w:cellDel` (removed on accept, restored on reject — Word's own accept semantics absorb the removed cell's grid slot into the preceding cell's `gridSpan`); a cell delete-vs-edit is a recorded conflict. **Cell-shell (`w:tcPr`) edits are first-class**: the shell digest participates in the cell `ContentHash` (`IrCell.ShellDigest`), so a width/gridSpan/vMerge/shading-only edit is visible (was: invisible — classified `EqualBlock` and silently dropped); the composed cell's shell is sourced from its editing reviewer, agreeing shells reach consensus, competing shells are a recorded conflict (BaseWins keeps the base shell). The remaining STOP boundary: a **CONTESTED `MovedRow`** (the same base row also edited/deleted/moved by another reviewer) falls back to a whole-table **block conflict** (no silent loss — the base table is kept under `BaseWins` and the disagreement is recorded under every policy); an UNCONTESTED row move composes (lowered to the two-way renderer's own del+ins row shape). Notes: shell/tblPr application is not `tcPrChange`/`tblPrChange`-tracked (reject restores text, not shell bytes — same class as the two-way renderer's right-shelled table render); a 2-way column ADD in the SINGLE-toucher passthrough still renders via the whole-table fallback (a pre-existing two-way renderer gap, independent of composition). - **Non-colliding reviewer moves/splits/merges render natively; colliding ones are lowered to del/ins as a recorded conflict.** A reviewer's `MoveBlock`/`MoveModifyBlock`/`SplitBlock`/`MergeBlock` whose consumed base block(s) are touched by ONLY that reviewer renders NATIVELY (`PlanMoves`/`ApplyMovePlan` for moves with globally-namespaced move-group ids; `ApplySplitMergePlan` for splits/merges — the same sole-toucher predicate via the shared `BuildTouchers` map). A native `MergeBlock` (null left anchor, N consumed base anchors) dispatches through `MergeBlockStream`'s first-consumed-anchor index; the remaining consumed anchors emit no block op but keep insert slotting, and `GroupInsertsByPrecedingAnchor` advances past the merge's consumed region so a following insert lands after the whole merge markup. A COLLIDING structural op (another reviewer touches a consumed base block) is LOWERED: `LowerOneStructuralOp` rewrites the move SOURCE → `DeleteBlock` (retaining `MoveGroupId`/`IsMoveSource` as a relocation marker, stripped before emission), the move DEST → `InsertBlock`, a split → `DeleteBlock` + N ordered `InsertBlock`s, a merge → N ordered `DeleteBlock`s + an `InsertBlock` — preserving op order; **content is fully preserved and round-trips** (accept shows the moved/split/merged text, reject ≡ base), and the collision resolves through the existing conflict machinery. Cell mini-bodies run the same split/merge plan (fixing a silent drop: a `MergeBlock` inside a multi-editor cell reached neither grouping map and vanished). Two reviewers relocating the SAME base block to different places collide on the lowered source-delete → a recorded **placement conflict** (the consensus removal is emitted once for `reject ≡ base`; each reviewer's relocating insert survives independently — no loss; this conflict is NOT policy-resolved into the op stream, so a `BaseWins` flip never wrongly restores a block both reviewers removed). -- **Reviewers' PARAGRAPH-property (`w:pPr`) changes MERGE (sub-project B1).** A reviewer's pPr-ONLY edit (jc/indent/spacing/style/numbering, text unchanged) is composed across reviewers: `ComposePPr` (mirroring `ComposeCellShell`) attributes it by `IrParagraph.PPrDigest` — 0 changers → base, all agree → consensus (first reviewer), ≥2 distinct → a recorded conflict resolved by policy (a pPr cannot STACK — one paragraph has one pPr). The composite renderer stamps native `w:pPrChange` authored to the winning reviewer; reject ≡ base, accept ≡ policy-winner hold at the property-byte level. Implemented via a paragraph slice of the composite flag (`TrackParagraphFormatChanges` on, `TrackBlockFormatChanges` off) so two-way behavior is byte-identical and the table/section slices stay off. **Still v1 ceilings (sub-project B2):** a reviewer's TABLE-shell (`w:tcPr`/etc.) or SECTION (`w:sectPr`) only edit is still ignored by `Consolidate`; a reviewer who changed BOTH a paragraph's text AND its pPr still routes to the conflict path (only pPr-ONLY edits compose in B1). Pinned by `BlockFormatChangeTests.Consolidate_merges_pPr_but_not_shell_section_v1`. +- **Reviewers' BLOCK-FORMAT changes MERGE — paragraph (B1) + table-shell + section (B2).** The internal `TrackBlockFormatChanges` is sliced into `TrackParagraphFormatChanges`/`TrackTableFormatChanges`/`TrackSectionFormatChanges` (each defaults equal, so two-way is byte-identical); `IrCompositeMerger` forces the umbrella OFF but turns all three slices ON. Every family composes by a per-element digest mirroring `ComposeCellShell` (0 changers → base, all agree → consensus/first reviewer, ≥2 distinct → a recorded conflict resolved by policy; a shell/pPr/section cannot STACK): + - **Paragraph (B1):** a pPr-only edit composed by `ComposeParagraphFormat` over the FULL boundary-normalized `BlockSignature` (run formats + pPr — NOT the pPr digest alone, which silently drops a competitor's run edit) → native `w:pPrChange` authored to the winner. + - **Table-shell (B2):** per-cell `tcPr` (existing `ComposeCellShell`, now with the render stamping the `w:tcPrChange` marker it previously omitted), per-row `trPr`+`tblPrEx`, per-table `tblPr`+`tblGrid` — attributed **independently** by `ComposeTableAndRowShells` (carried on `IrAuthoredRowOp.TrPr`/`TblPrEx` + `IrCompositeOp.TableShell`, stamped by `ApplyComposedShell`). Disjoint elements compose; only a contested element conflicts; composes with #250's column-add/remove + row-move. Single-reviewer shells ride the two-way single-source render. + - **Section (B2):** the trailing `w:sectPr` (a document-level element, not a body block op) is composed by `ComposeTrailingSection` over each reviewer's trailing `IrSectionBreak.FormatFingerprint` (modeled + unmodeled digest) and stamped as `w:sectPrChange` (inner = base, hdr/ftr refs preserved) by `ApplyComposedTrailingSectPr`; an inline (`w:pPr/w:sectPr`) section change rides B1's paragraph path. + - `reject ≡ base` / `accept ≡ policy-winner` hold at the **property-byte** level for every family — enforced by the strengthened byte-level verifier (`Docs.ShellSection` asserted alongside the text projections in `CompositeFuzzTests` + `ConsolidateBlockFormatB2Tests`). **v1 decision:** a reviewer who changed BOTH a paragraph's text AND its pPr is conflict-routed (never a silent format drop); true inline text+format compose is deferred. Pinned by `ConsolidateBlockFormatB2Tests`. - **Conflict spans are base TOKEN indices** (`TokenStart`/`TokenEnd`) + `BaseAnchor`, not character offsets — suitable for machine consumers inspecting the edit script. ## Parity status From ef111765d09e5abf175e39c5e15f6402cb2de905 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sat, 4 Jul 2026 06:34:33 -0500 Subject: [PATCH 5/8] =?UTF-8?q?fix(diff):=20Consolidate=20B2=20=E2=80=94?= =?UTF-8?q?=20emit=20the=20trailing-section=20revision=20exactly=20once,?= =?UTF-8?q?=20not=20per-op?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Ir/Diff/ConsolidateBlockFormatB2Tests.cs | 21 +++++++++++++++ .../Ir/Diff/IrCompositeRevisionRenderer.cs | 26 ++++++++++++++++++- Docxodus/Ir/Diff/IrDiffSettings.cs | 10 +++++++ Docxodus/Ir/Diff/IrRevisionRenderer.cs | 2 +- 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs index 2291ced1..824018ab 100644 --- a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs +++ b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs @@ -114,6 +114,27 @@ public void Trailing_sectPr_change_reports_section_consolidated_revision() Assert.Contains(revs, r => r.FormatChange is { } fc && fc.Scope == DocxDiffFormatChangeScope.Section && r.Author == "Alice"); } + /// + /// The document-level trailing-section revision is emitted EXACTLY ONCE (attributed to the section winner), + /// not once per composite op — a reviewer editing several paragraphs AND the section must not multiply the + /// section revision. Pins the fix for the per-op duplication the two-way renderer would otherwise cause. + /// + [Fact] + public void Trailing_sectPr_change_reports_exactly_one_section_revision_across_many_ops() + { + static string ManyOps(string t1, string t2, string top) => + $"{t1}{t2}" + + $""; + var baseDoc = IrTestDocuments.FromBodyXml(ManyOps("one", "two", "1440")); + var alice = IrTestDocuments.FromBodyXml(ManyOps("ONE", "TWO", "2880")); + + var revs = DocxDiff.GetConsolidatedRevisions(baseDoc, + new[] { new DocxDiffReviewer { Author = "Alice", Document = alice } }); + var sectionRevs = revs.Where(r => r.FormatChange is { } fc && fc.Scope == DocxDiffFormatChangeScope.Section).ToList(); + Assert.Single(sectionRevs); + Assert.Equal("Alice", sectionRevs[0].Author); + } + [Fact] public void Inline_sectPr_change_merges_with_marker_and_round_trips() { diff --git a/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs b/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs index d4367e26..774f0d20 100644 --- a/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs +++ b/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs @@ -54,7 +54,10 @@ internal static class IrCompositeRevisionRenderer { // Mirror IrCompositeMerger's forcing. B1 turns the PARAGRAPH slice ON so a single-source pPr op reports // a Paragraph-scope FormatChanged authored to its reviewer; table-shell/section slices stay OFF (B2). - settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true, TrackSectionFormatChanges = true }; + // EmitTrailingSectionRevision OFF: the per-op mini-scripts must NOT each append the document-level + // trailing-sectPr revision (a section-changing reviewer with N ops would emit it N times). It is + // emitted ONCE below from script.TrailingSectPr, attributed to the section winner. + settings = settings with { TrackBlockFormatChanges = false, TrackParagraphFormatChanges = true, TrackTableFormatChanges = true, TrackSectionFormatChanges = true, EmitTrailingSectionRevision = false }; // Move-source pre-pass over the WHOLE composite script. Single-source ops are each rendered in // their own one-op mini-script (so IrRevisionRenderer honours per-op granularity/author), but a @@ -95,6 +98,27 @@ internal static class IrCompositeRevisionRenderer } } } + + // TRAILING SECTION (B2): emit the document-level trailing-sectPr FormatChanged revision EXACTLY ONCE, + // attributed to the section winner (script.TrailingSectPr). The per-op mini-scripts have it suppressed + // (EmitTrailingSectionRevision=false), so this is the single authoritative section revision. + if (script.TrailingSectPr is { } sectWinner + && sectWinner.Reviewer >= 0 && sectWinner.Reviewer < reviewers.Count + && baseIr.Body.Blocks.Count > 0 && baseIr.Body.Blocks[^1] is IrSectionBreak baseSec) + { + var winnerBlocks = reviewers[sectWinner.Reviewer].Ir.Body.Blocks; + if (winnerBlocks.Count > 0 && winnerBlocks[^1] is IrSectionBreak winnerSec) + { + var details = IrModeledFormat.SectionFormatChangeDetails(baseSec.Format, winnerSec.Format); + if (details.ChangedPropertyNames.Count > 0) + result.Add(( + new IrRevision(IrRevisionType.FormatChanged, string.Empty, sectWinner.Author, + settings.DateTimeForRevisions, FormatChange: details, + LeftAnchor: baseSec.Anchor.ToString(), RightAnchor: winnerSec.Anchor.ToString()), + sectWinner.Author, null)); + } + } + return result; } diff --git a/Docxodus/Ir/Diff/IrDiffSettings.cs b/Docxodus/Ir/Diff/IrDiffSettings.cs index 198075d0..36d95dae 100644 --- a/Docxodus/Ir/Diff/IrDiffSettings.cs +++ b/Docxodus/Ir/Diff/IrDiffSettings.cs @@ -311,6 +311,16 @@ internal sealed record IrDiffSettings /// public bool TrackSectionFormatChanges { get; init; } = true; + /// + /// DIFF-TIME setting (Consolidate B2). Whether the two-way revision renderer appends the DOCUMENT-LEVEL + /// trailing-w:sectPr FormatChanged revision (it compares the two documents' final section blocks, so + /// it is not a per-op revision). Default true (every two-way call reports it). The composite revision + /// renderer sets this FALSE for its PER-OP mini-scripts — otherwise a section-changing reviewer with N ops + /// would emit the same trailing-section revision N times — and instead emits it exactly once from + /// IrCompositeScript.TrailingSectPr, attributed to the section winner. + /// + public bool EmitTrailingSectionRevision { get; init; } = true; + /// /// REVISIONS-SURFACE setting (M2.3 Task 1). Author name stamped on every 's /// . Default "Open-Xml-PowerTools" — copied verbatim from diff --git a/Docxodus/Ir/Diff/IrRevisionRenderer.cs b/Docxodus/Ir/Diff/IrRevisionRenderer.cs index fe755602..2e8d2749 100644 --- a/Docxodus/Ir/Diff/IrRevisionRenderer.cs +++ b/Docxodus/Ir/Diff/IrRevisionRenderer.cs @@ -117,7 +117,7 @@ public static IrNodeList Render( // trailing section formats and, when the MODELED fields differ, append one Section-scope FormatChanged // (mirrors the markup renderer's w:sectPrChange on the trailing sectPr). Appended after all body/note/ // header-footer ops (additive ordering). Excluded from WmlComparerCompatible by the scope filter below. - if (settings.TrackSectionFormatChanges + if (settings.TrackSectionFormatChanges && settings.EmitTrailingSectionRevision && ctx.Left.Body.Blocks.Count > 0 && ctx.Left.Body.Blocks[^1] is IrSectionBreak lsec && ctx.Right.Body.Blocks.Count > 0 && ctx.Right.Body.Blocks[^1] is IrSectionBreak rsec) { From 2adf496fe2adcc2ad6c91e239238a69b4e528232 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sat, 4 Jul 2026 06:43:38 -0500 Subject: [PATCH 6/8] =?UTF-8?q?fix(diff):=20Consolidate=20B2=20=E2=80=94?= =?UTF-8?q?=20anchor-aware=20row-shell=20pairing=20+=20delete-vs-shell=20c?= =?UTF-8?q?onflict=20+=20shell=20revisions=20(adversarial=20review)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../Ir/Diff/ConsolidateBlockFormatB2Tests.cs | 68 ++++++++++++++++++ Docxodus/Ir/Diff/IrCompositeMerger.cs | 62 +++++++++++++---- .../Ir/Diff/IrCompositeRevisionRenderer.cs | 69 ++++++++++++++++++- 3 files changed, 185 insertions(+), 14 deletions(-) diff --git a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs index 824018ab..88882d80 100644 --- a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs +++ b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs @@ -226,6 +226,26 @@ public void Disjoint_families_compose_cell_shell_and_section_both_land() Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); } + /// + /// Review finding C: multi-reviewer table-shell changes (which route through the composed-table AuthoredRows + /// path, not single-source) must ALSO surface on the consolidated REVISIONS surface — Table/TableRow/TableCell + /// scope revisions attributed to each winner — matching two-way GetRevisions. + /// + [Fact] + public void Multireviewer_table_shell_changes_report_consolidated_revisions() + { + var baseDoc = Base(); + var revs = DocxDiff.GetConsolidatedRevisions(baseDoc, new[] + { + new DocxDiffReviewer { Author = "Alice", Document = IrTestDocuments.FromBodyXml(Body(tblW: "6000")) }, // tblPr + new DocxDiffReviewer { Author = "Bob", Document = IrTestDocuments.FromBodyXml(Body(trHeight: "500")) }, // trPr + new DocxDiffReviewer { Author = "Carol", Document = IrTestDocuments.FromBodyXml(Body(tcW00: "3000")) }, // tcPr + }); + Assert.Contains(revs, r => r.FormatChange?.Scope == DocxDiffFormatChangeScope.Table && r.Author == "Alice"); + Assert.Contains(revs, r => r.FormatChange?.Scope == DocxDiffFormatChangeScope.TableRow && r.Author == "Bob"); + Assert.Contains(revs, r => r.FormatChange?.Scope == DocxDiffFormatChangeScope.TableCell && r.Author == "Carol"); + } + // ------------------------------------------------------------------ Phase 1c: multi-reviewer row/table-level shells [Theory] @@ -289,6 +309,54 @@ public void Cell_shell_and_row_shell_by_different_reviewers_compose() Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); } + // ------------------------------------------------------------------ row-restructure + shell interaction (review findings A/B) + + private static string Tr(string trH, string txt) => + $"" + + $"{txt}"; + private static string Tbl(params string[] rows) => + "lead" + + "" + string.Concat(rows) + + "tail"; + + /// + /// Review finding A: a reviewer's ROW-shell change must survive a CO-reviewer's row DELETE (which shifts + /// positional alignment). Anchor-aware row pairing attributes row0's trPr change even though Alice also + /// deleted row2 (count 3→2) — the old count-guard silently dropped it. No phantom trPr on the surviving rows. + /// + [Fact] + public void Row_shell_change_survives_a_cotoucher_row_delete_no_drop() + { + var baseDoc = IrTestDocuments.FromBodyXml(Tbl(Tr("300", "a"), Tr("300", "b"), Tr("300", "c"))); + var alice = IrTestDocuments.FromBodyXml(Tbl(Tr("500", "a"), Tr("300", "b"))); // row0 trPr + delete row2 + var bob = IrTestDocuments.FromBodyXml(Tbl(Tr("300", "a"), Tr("300", "B2"), Tr("300", "c"))); // row1 text + + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob)); + Assert.Contains("w:trPrChange", Xml(merged)); // row0 trPr attributed, not dropped + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); // reject ≡ base (all rows/shells) + Assert.Equal(Docs.StructuralBody(baseDoc), Docs.StructuralBody(Reject(merged))); + // accept: row0 trPr=500 (Alice), row2 removed, no phantom trPr on the surviving rows. + var expected = IrTestDocuments.FromBodyXml(Tbl(Tr("500", "a"), Tr("300", "B2"))); + Assert.Equal(Docs.ShellSection(expected), Docs.ShellSection(Accept(merged))); + } + + /// + /// Review finding B: a reviewer reformatting a row's shell that ANOTHER reviewer deletes is a recorded + /// conflict (delete-vs-reformat), never a silent drop. The delete wins; reject restores base. + /// + [Fact] + public void Row_delete_vs_shell_reformat_records_conflict_never_silent() + { + var baseDoc = IrTestDocuments.FromBodyXml(Tbl(Tr("300", "a"), Tr("300", "b"), Tr("300", "c"))); + var alice = IrTestDocuments.FromBodyXml(Tbl(Tr("300", "b"), Tr("300", "c"))); // deletes row0 + var bob = IrTestDocuments.FromBodyXml(Tbl(Tr("500", "a"), Tr("300", "b"), Tr("300", "c"))); // row0 trPr + + Assert.NotEmpty(Conflicts(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob))); + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob)); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + Assert.Equal(Docs.StructuralBody(baseDoc), Docs.StructuralBody(Reject(merged))); + } + // ------------------------------------------------------------------ Phase 3: text+format is never silently dropped /// diff --git a/Docxodus/Ir/Diff/IrCompositeMerger.cs b/Docxodus/Ir/Diff/IrCompositeMerger.cs index babe473c..51ac4407 100644 --- a/Docxodus/Ir/Diff/IrCompositeMerger.cs +++ b/Docxodus/Ir/Diff/IrCompositeMerger.cs @@ -1646,12 +1646,13 @@ private static bool IsTableContentOrShellEdit(IrEditOp op, IrDocument reviewerIr ref int nextConflictId, List conflicts, List authoredRows) { - // Resolve each toucher's RIGHT table (by right anchor). Unresolvable → skipped (base shell kept). - var rights = new List<(int Reviewer, string Author, IrTable Right)>(); + // Resolve each toucher's RIGHT table (by right anchor), keeping the op for anchor-aware row pairing. + // Unresolvable → skipped (base shell kept). + var rights = new List<(int Reviewer, string Author, IrTable Right, IrEditOp Op)>(); foreach (var (reviewer, op) in touched) if (op.RightAnchor is { } ra && reviewers[reviewer].Ir.AnchorIndex.TryGetValue(ra, out var rb) && rb is IrTable rt) - rights.Add((reviewer, reviewers[reviewer].Author, rt)); + rights.Add((reviewer, reviewers[reviewer].Author, rt, op)); string tableText = string.Join(" ", baseTable.Rows.Select(r => RowResultText(r, settings))); @@ -1664,23 +1665,41 @@ private static bool IsTableContentOrShellEdit(IrEditOp op, IrDocument reviewerIr .Select(r => (r.Reviewer, r.Author, r.Right.TblGridDigest, r.Right.Anchor.ToString())).ToList(), policy, ref nextConflictId, conflicts, tableText); - int rowCount = baseTable.Rows.Count; - for (int i = 0; i < rowCount; i++) + for (int i = 0; i < baseTable.Rows.Count; i++) { var baseRow = baseTable.Rows[i]; string rowAnchor = baseRow.Anchor.ToString(); string rowText = RowResultText(baseRow, settings); - // Only touchers whose right table row-count matches base contribute a positionally-paired right row. - var alignedRows = rights.Where(r => r.Right.Rows.Count == rowCount) - .Select(r => (r.Reviewer, r.Author, Row: r.Right.Rows[i])).ToList(); + + // ANCHOR-AWARE right-row resolution (NOT positional-by-index): a content toucher maps THIS base row + // to its right row via its TableDiff row op (so a co-toucher's row insert/delete/move never shifts + // the pairing → no wrong-row corruption); a pure-shell FormatOnly toucher is content-equal, so its + // rows are positionally aligned. A toucher that DELETED this base row contributes no right row. + var alignedRows = new List<(int Reviewer, string Author, IrRow Row)>(); + foreach (var (reviewer, author, right, op) in rights) + if (ResolveRightRowForBaseRow(op, right, rowAnchor, i) is { } rr) + alignedRows.Add((reviewer, author, rr)); + + var trChangers = alignedRows.Where(r => !r.Row.TrPrShellDigest.Equals(baseRow.TrPrShellDigest)).ToList(); + var exChangers = alignedRows.Where(r => !r.Row.TrPrExDigest.Equals(baseRow.TrPrExDigest)).ToList(); + + // DELETE-vs-SHELL: if the authored row is a DeleteRow (a reviewer removed it) while another reviewer + // reformatted its shell, that is a conflict — record it (never a silent drop). The delete wins + // (accept removes the row, reject restores base), so no shell attribution is attached to the delete. + if (authoredRows.Any(a => a.BaseRowAnchor == rowAnchor && a.Kind == IrRowOpKind.DeleteRow)) + { + var shellRevs = trChangers.Concat(exChangers).Select(c => c.Reviewer).Distinct().ToList(); + if (shellRevs.Count > 0) + conflicts.Add(new IrConflict(nextConflictId++, rowAnchor, 0, 0, policy, + IrNodeList.From(shellRevs.Select(rev => new IrConflictCompetitor(reviewers[rev].Author, rowText))))); + continue; + } var trPr = ComposeShellElement(rowAnchor, - alignedRows.Where(r => !r.Row.TrPrShellDigest.Equals(baseRow.TrPrShellDigest)) - .Select(r => (r.Reviewer, r.Author, r.Row.TrPrShellDigest, r.Row.Anchor.ToString())).ToList(), + trChangers.Select(r => (r.Reviewer, r.Author, r.Row.TrPrShellDigest, r.Row.Anchor.ToString())).ToList(), policy, ref nextConflictId, conflicts, rowText); var tblPrEx = ComposeShellElement(rowAnchor, - alignedRows.Where(r => !r.Row.TrPrExDigest.Equals(baseRow.TrPrExDigest)) - .Select(r => (r.Reviewer, r.Author, r.Row.TrPrExDigest, r.Row.Anchor.ToString())).ToList(), + exChangers.Select(r => (r.Reviewer, r.Author, r.Row.TrPrExDigest, r.Row.Anchor.ToString())).ToList(), policy, ref nextConflictId, conflicts, rowText); var trRef = ToShellRef(trPr); @@ -1720,6 +1739,25 @@ private static (int Reviewer, string? Anchor, string Author) ComposeShellElement private static IrComposedShellRef? ToShellRef((int Reviewer, string? Anchor, string Author) w) => w.Reviewer >= 0 && w.Anchor is { } a ? new IrComposedShellRef(w.Reviewer, a, w.Author) : null; + /// Resolve the RIGHT row a toucher's edit pairs with base row + /// (index ) for shell comparison — anchor-aware, so a co-toucher's row restructure + /// never shifts the pairing. A pure-shell toucher is + /// content-equal, so its rows align positionally; a content toucher + /// maps via its TableDiff row op (the op whose LeftRowAnchor is this base row) — returning null when that + /// toucher DELETED the row (no right-row correspondence) so its (moot) shell is not mis-paired. + private static IrRow? ResolveRightRowForBaseRow(IrEditOp op, IrTable rightTable, string baseRowAnchor, int i) + { + if (op.Kind == IrEditOpKind.FormatOnlyBlock) + return i < rightTable.Rows.Count ? rightTable.Rows[i] : null; + if (op.TableDiff is { } td) + foreach (var ro in td.RowOps) + if (ro.LeftRowAnchor == baseRowAnchor) + return ro.RightRowAnchor is { } rra + ? rightTable.Rows.FirstOrDefault(r => r.Anchor.ToString() == rra) + : null; + return null; + } + /// /// Compose the document-final (trailing) w:sectPr across reviewers (Consolidate B2). The trailing /// section is NOT a body block op (Word compares it at the document level), so it is composed here directly: diff --git a/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs b/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs index 774f0d20..a359535b 100644 --- a/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs +++ b/Docxodus/Ir/Diff/IrCompositeRevisionRenderer.cs @@ -141,12 +141,30 @@ private static void RenderComposedTable( IReadOnlyDictionary moveSourceOp, List<(IrRevision, string, int?)> sink) { + // B2: table-level shell revisions (tblPr/tblGrid), attributed to their winners (the markup stamps the + // markers; report them on the revisions surface too, matching two-way GetRevisions). + if (op.TableShell is { } ts) + { + string baseTblAnchor = op.Op.LeftAnchor ?? string.Empty; + if (ts.TblPr is { } tp) + AddShellRev(sink, IrFormatChangeScope.Table, "shell", baseTblAnchor, tp.RightAnchor, tp.Author, settings, op.ConflictId); + if (ts.TblGrid is { } tg) + AddShellRev(sink, IrFormatChangeScope.Table, "grid", baseTblAnchor, tg.RightAnchor, tg.Author, settings, op.ConflictId); + } + foreach (var rowOp in op.AuthoredRows!) { + // B2: per-row shell revisions (trPr/tblPrEx) — a shell change can ride an EqualRow (content-equal + // row), so this is emitted for every row op kind, before the content switch. + if (rowOp.TrPr is { } rp) + AddShellRev(sink, IrFormatChangeScope.TableRow, "shell", rowOp.BaseRowAnchor ?? string.Empty, rp.RightAnchor, rp.Author, settings, op.ConflictId); + if (rowOp.TblPrEx is { } ep) + AddShellRev(sink, IrFormatChangeScope.TableRow, "tblPrEx", rowOp.BaseRowAnchor ?? string.Empty, ep.RightAnchor, ep.Author, settings, op.ConflictId); + switch (rowOp.Kind) { case IrRowOpKind.EqualRow: - break; // unchanged base row → no revision + break; // unchanged base row → no CONTENT revision (a row-shell change was emitted above) case IrRowOpKind.InsertRow: { @@ -218,8 +236,18 @@ private static void RenderComposedTable( } continue; } + // B2: cell-shell (tcPr) revision when the winner's shell canonically differs from base + // (a width/merge/shading-only cell edit) — the digest check avoids a spurious revision + // for a content-only edit that also set ShellSourceReviewer. + if (cellOp.ShellSourceReviewer >= 0 && cellOp.ShellSourceReviewer < reviewers.Count + && cellOp.ShellRightCellAnchor is { } sca && cellOp.BaseCellAnchor is { } bca + && FindCell(baseIr, bca) is { } baseCell + && FindCell(reviewers[cellOp.ShellSourceReviewer].Ir, sca) is { } winnerCell + && !baseCell.TcPrShellDigest.Equals(winnerCell.TcPrShellDigest)) + AddShellRev(sink, IrFormatChangeScope.TableCell, "shell", bca, sca, cellOp.ShellAuthor, settings, op.ConflictId); + if (cellOp.ComposedBlockOps is not { } blockOps) - continue; // base passthrough → no revision + continue; // base passthrough → no content revision foreach (var cellBlock in blockOps) { if (cellBlock.AuthoredTokens != null) @@ -234,6 +262,43 @@ private static void RenderComposedTable( } } + private static readonly IReadOnlyDictionary NoProps = new Dictionary(); + + /// Append one digest-grade shell FormatChanged revision (Consolidate B2 — the composite analogue + /// of the two-way TableShellRevision), attributed to the shell winner. + private static void AddShellRev( + List<(IrRevision, string, int?)> sink, IrFormatChangeScope scope, string changed, + string leftAnchor, string rightAnchor, string author, IrDiffSettings settings, int? conflictId) + => sink.Add(( + new IrRevision(IrRevisionType.FormatChanged, string.Empty, author, settings.DateTimeForRevisions, + FormatChange: new IrFormatChangeDetails(NoProps, NoProps, new[] { changed }, scope), + LeftAnchor: leftAnchor, RightAnchor: rightAnchor), + author, conflictId)); + + /// The a cell anchor resolves to in (cells are not in + /// the AnchorIndex — walk indexed tables' rows/cells, recursing into nested tables). Null if unknown. + private static IrCell? FindCell(IrDocument ir, string cellAnchor) + { + foreach (var block in ir.AnchorIndex.Values) + if (block is IrTable tbl && FindCellInTable(tbl, cellAnchor) is { } c) + return c; + return null; + } + + private static IrCell? FindCellInTable(IrTable tbl, string cellAnchor) + { + foreach (var row in tbl.Rows) + foreach (var cell in row.Cells) + { + if (cell.Anchor.ToString() == cellAnchor) + return cell; + foreach (var b in cell.Blocks) + if (b is IrTable nested && FindCellInTable(nested, cellAnchor) is { } found) + return found; + } + return null; + } + /// The concatenated paragraph text of ONE cell resolved by anchor (cells are not in /// ; nested tables recursed), for cell insert/delete revision text. private static string CellTextByAnchor(IrDocument ir, string cellAnchor, IrDiffSettings settings) From 2442df400756051bb0de42323a9141f882276ac7 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sat, 4 Jul 2026 06:49:18 -0500 Subject: [PATCH 7/8] =?UTF-8?q?fix(diff):=20Consolidate=20B2=20=E2=80=94?= =?UTF-8?q?=20guard=20tblGrid=20composition=20against=20structural=20colum?= =?UTF-8?q?n=20changes=20(round-trip=20review)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Ir/Diff/ConsolidateBlockFormatB2Tests.cs | 36 +++++++++++++++++++ Docxodus/Ir/Diff/IrCompositeMerger.cs | 8 ++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs index 88882d80..57425c34 100644 --- a/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs +++ b/Docxodus.Tests/Ir/Diff/ConsolidateBlockFormatB2Tests.cs @@ -309,6 +309,42 @@ public void Cell_shell_and_row_shell_by_different_reviewers_compose() Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); } + // ------------------------------------------------------------------ tblGrid vs structural column change (review round-trip finding) + + private static string GridTbl(string grid, params string[] cells) => + "lead" + + $"{grid}" + + string.Concat(cells.Select(c => $"{c}")) + + ""; + + /// + /// Review round-trip finding: a structural column ADD changes TblGridDigest, but it is NOT a grid-property + /// change — it is owned by the column composition (native w:cellIns). B2 must NOT compose it as a + /// w:tblGridChange (which would leave the accepted grid's gridCol count disagreeing with the tc count). + /// + [Fact] + public void Column_add_does_not_compose_tblGrid_as_a_property_change() + { + var baseDoc = IrTestDocuments.FromBodyXml(GridTbl("", "a", "b")); + var v = IrTestDocuments.FromBodyXml(GridTbl("", "a", "b", "c")); + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", v), ("Bob", v)); + Assert.DoesNotContain("w:tblGridChange", Xml(merged)); + Assert.Equal(Docs.StructuralBody(baseDoc), Docs.StructuralBody(Reject(merged))); + } + + /// A PURE gridCol-width change (same column count) still composes as a w:tblGridChange (B2). + [Fact] + public void Multireviewer_gridcol_width_change_composes_tblGridChange() + { + var baseDoc = IrTestDocuments.FromBodyXml(GridTbl("", "a", "b")); + var v = IrTestDocuments.FromBodyXml(GridTbl("", "a", "b")); + Assert.Empty(Conflicts(baseDoc, ConflictResolution.BaseWins, ("Alice", v), ("Bob", v))); + var merged = Consolidate(baseDoc, ConflictResolution.BaseWins, ("Alice", v), ("Bob", v)); + Assert.Contains("w:tblGridChange", Xml(merged)); + Assert.Equal(Docs.ShellSection(v), Docs.ShellSection(Accept(merged))); + Assert.Equal(Docs.ShellSection(baseDoc), Docs.ShellSection(Reject(merged))); + } + // ------------------------------------------------------------------ row-restructure + shell interaction (review findings A/B) private static string Tr(string trH, string txt) => diff --git a/Docxodus/Ir/Diff/IrCompositeMerger.cs b/Docxodus/Ir/Diff/IrCompositeMerger.cs index 51ac4407..51980c8f 100644 --- a/Docxodus/Ir/Diff/IrCompositeMerger.cs +++ b/Docxodus/Ir/Diff/IrCompositeMerger.cs @@ -1660,8 +1660,14 @@ private static bool IsTableContentOrShellEdit(IrEditOp op, IrDocument reviewerIr rights.Where(r => !r.Right.TblPrDigest.Equals(baseTable.TblPrDigest)) .Select(r => (r.Reviewer, r.Author, r.Right.TblPrDigest, r.Right.Anchor.ToString())).ToList(), policy, ref nextConflictId, conflicts, tableText); + // tblGrid is composed as a grid-PROPERTY change ONLY for a reviewer whose COLUMN COUNT matches base + // (a pure gridCol-width edit). A column ADD/REMOVE also changes TblGridDigest, but that is STRUCTURAL — + // owned by ComposeTableDiffs (native w:cellIns/w:cellDel); composing the grid there too would stamp a + // spurious w:tblGridChange and leave the accepted grid's gridCol count disagreeing with the tc count. + int baseCols = baseTable.Rows.Count > 0 ? baseTable.Rows[0].Cells.Count : 0; var tblGrid = ComposeShellElement(tableAnchor, - rights.Where(r => !r.Right.TblGridDigest.Equals(baseTable.TblGridDigest)) + rights.Where(r => !r.Right.TblGridDigest.Equals(baseTable.TblGridDigest) + && (r.Right.Rows.Count > 0 ? r.Right.Rows[0].Cells.Count : 0) == baseCols) .Select(r => (r.Reviewer, r.Author, r.Right.TblGridDigest, r.Right.Anchor.ToString())).ToList(), policy, ref nextConflictId, conflicts, tableText); From 077d11cf1aad630499e856eee02a0cefa64f5a0a Mon Sep 17 00:00:00 2001 From: JSv4 Date: Thu, 9 Jul 2026 00:06:03 -0500 Subject: [PATCH 8/8] =?UTF-8?q?fix(diff):=20Consolidate=20M-A=20#4=20?= =?UTF-8?q?=E2=80=94=20rewrite=20cross-kind=20note=20refs=20nested=20in=20?= =?UTF-8?q?reviewer-inserted=20notes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 1 + .../Ir/Diff/IrCompositeCrossKindNoteTests.cs | 311 ++++++++++++++++++ Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs | 26 +- docs/architecture/ir_diff_engine.md | 4 +- 4 files changed, 338 insertions(+), 4 deletions(-) create mode 100644 Docxodus.Tests/Ir/Diff/IrCompositeCrossKindNoteTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3950eced..275b1062 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ All notable changes to this project will be documented in this file. - **Upgraded from .NET 8.0 to .NET 10.0** (.NET 8 reaches end-of-support in November 2026). `TargetFramework` bumped to `net10.0` across the core library, tests, all CLI tools (`redline`/`docx2html`/`docx2oc`/`docxodus-pyhost`), the `diffharness` tool, and the WASM bridge; `global.json` now pins SDK `10.0.0`. The library, tests, and WASM/AOT toolchain all built and ran clean on the first pass (2468 passed / 0 failed / 3 skipped — identical to the .NET 8 baseline) with no source changes needed beyond the target framework bump. CI workflows (`ci.yml`, `playwright.yml`, `publish.yml`, `python-publish.yml`) now provision the .NET 10 SDK, and the WASM build script/Python host dev-fallback path were updated for the new `net10.0` output directory. ### Fixed +- **DocxDiff.Consolidate: a reviewer-inserted note that cites another note the same reviewer inserted no longer dangles (cross-kind note-in-note, N-way).** The N-way note-scope merge already renumbered cross-kind nested references (a footnote body citing an endnote, or vice versa) that live in BASE notes — but 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 only visits body clones, and the renumber sweep keys on the output-old id, which the reviewer id is not). So when a reviewer inserted a footnote whose text cited an endnote the same reviewer also inserted — whose target id becomes a *fresh* output id, not a base id — the nested reference kept the reviewer id and dangled 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. Proven by the new `IrCompositeCrossKindNoteTests` (base cross-kind nesting survives the N-way renumber across all three policies; reviewer-inserted footnote-cites-existing-endnote and endnote-cites-existing-footnote; and the previously-dangling reviewer-inserted-footnote-cites-reviewer-inserted-endnote), each asserting structural reference resolvability on merge/accept/reject with a non-vacuity guard, plus a headless-LibreOffice render backstop on the note-in-note-free note-merge path (note-in-note references are valid OOXML but LibreOffice Writer's DOCX import cannot load any document that contains one — cross-kind or same-kind — so the resolvability oracle is structural + Word, not LibreOffice). Closes the last untested corner (M-A #4) of the note-scope Consolidate merge. - **DocxDiff: a paragraph move that crosses a table boundary spuriously relocated the TABLE, contesting the whole table block in an N-way `Consolidate` (issue #229).** A reorder can be ambiguous between relocating a light paragraph and a heavy structural block — `[A, table, B]` → `[A, B, table]` reads equally as "paragraph B moved up past the table" OR "the table moved down past B" (both cost one relocation and round-trip identically in a two-way `Compare`). `IrBlockAligner`'s LIS spine arbitrarily picked the reading that relocated the TABLE. In `Consolidate` that spurious table-move then 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 rather than composed per-cell (no data loss, and `reject ≡ base` always held; this was a precision/quality limitation, not a correctness failure). The aligner now applies a structural-anchor tie-break: among equal-length spines it keeps the most non-paragraph blocks (tables / section breaks / opaque blocks) anchored and relocates the lighter paragraph. It is 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 (zero churn). The paragraph move and the disjoint cell edit now compose independently (`conflicts == 0`, a native/lowered paragraph move + per-cell table compose, `reject ≡ base`), and two-way markup is cleaner (a paragraph moves, not a whole table). Base two-way parity holds at 179/179; full suite green. Proof: `IrCompositeTableMoveBoundaryTests` (all three conflict policies). - **npm `DocxEditor`/worker bridge: the .NET 10 WASM runtime hung forever inside a dedicated Web Worker.** Upgrading the WASM bridge from the .NET 8 to the .NET 10 runtime surfaced [dotnet/runtime#114918](https://github.com/dotnet/runtime/issues/114918), an upstream regression where `dotnet.js`'s asset loader treats a truthy `globalThis.onmessage` as a (mis-detected) worker-environment signal and never resolves its asset-loading promises — `dotnet.create()`/`getAssemblyExports()` hang indefinitely with no error. `npm/src/docxodus.worker.ts` set its message handler via the `self.onmessage = ...` property form, which tripped exactly this bug once the WASM runtime moved to net10.0. Switched to `self.addEventListener("message", ...)`, which doesn't set the `onmessage` property and sidesteps the bug; behavior is otherwise identical. Confirmed via a byte-for-byte .NET 8 vs .NET 10 WASM bundle comparison — the .NET 8 build initialized a worker in ~485ms, the unpatched .NET 10 build hung indefinitely, and the patched .NET 10 build initializes in ~520ms. Affects every `npm` consumer of the worker bridge (`createWorkerDocxodus`, the `DocxEditor` worker mode); the main-thread (non-worker) WASM path was never affected. diff --git a/Docxodus.Tests/Ir/Diff/IrCompositeCrossKindNoteTests.cs b/Docxodus.Tests/Ir/Diff/IrCompositeCrossKindNoteTests.cs new file mode 100644 index 00000000..106e09af --- /dev/null +++ b/Docxodus.Tests/Ir/Diff/IrCompositeCrossKindNoteTests.cs @@ -0,0 +1,311 @@ +#nullable enable +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Xml.Linq; +using DocumentFormat.OpenXml; +using DocumentFormat.OpenXml.Packaging; +using DocumentFormat.OpenXml.Validation; +using DocumentFormat.OpenXml.Wordprocessing; +using Docxodus; +using Xunit; + +namespace Docxodus.Tests.Ir.Diff; + +/// +/// CROSS-KIND note nesting through the N-way CONSOLIDATE engine — the last untested corner of the note-scope +/// merge (item M-A #4). A footnote body that cites an endnote (and an endnote body that cites a footnote) must +/// keep its nested reference resolvable after the composite renderer's body-order renumber, exactly as the +/// two-way engine does (). +/// The existing IrCompositeNoteTests fixture is footnote-only with body-order ids, so it never exercised +/// this: cross-kind nesting, gapped ids that force a real shift, and the reviewer-INSERTED-note sub-case are all +/// new here. +/// +public class IrCompositeCrossKindNoteTests +{ + private const string W = "http://schemas.openxmlformats.org/wordprocessingml/2006/main"; + private static readonly XNamespace Wns = W; + + private const string FootnoteReserved = + "" + + ""; + private const string EndnoteReserved = + "" + + ""; + + // ------------------------------------------------------------------ fixture builder + + /// Build a doc from raw fragments: is the body paragraphs (sectPr is + /// appended), / are the REAL note definitions + /// (reserved boilerplate is prepended). An endnotes part is added iff is set. + private static WmlDocument Build(string bodyInner, string footnoteDefs, string? endnoteDefs) + { + using var ms = new MemoryStream(); + using (var doc = WordprocessingDocument.Create(ms, WordprocessingDocumentType.Document)) + { + var main = doc.AddMainDocumentPart(); + main.AddNewPart().Styles = new Styles(new DocDefaults(new RunPropertiesDefault( + new RunPropertiesBaseStyle(new RunFonts { Ascii = "Calibri" }, new FontSize { Val = "22" })))); + main.AddNewPart().Settings = new Settings(); + + WritePartXml(main.AddNewPart(), + $"{FootnoteReserved}{footnoteDefs}"); + if (endnoteDefs != null) + WritePartXml(main.AddNewPart(), + $"{EndnoteReserved}{endnoteDefs}"); + + WritePartXml(main, + $"{bodyInner}" + + ""); + } + return new WmlDocument("doc.docx", ms.ToArray()); + } + + private static string Para(string text, string? refXml = null) + => $"{text}{refXml ?? ""}"; + + private static string FnRef(int id) => $""; + private static string EnRef(int id) => $""; + + private static string Footnote(int id, string text, string? nestedRefXml = null) + => $"{text}{nestedRefXml ?? ""}"; + private static string Endnote(int id, string text, string? nestedRefXml = null) + => $"{text}{nestedRefXml ?? ""}"; + + private static void WritePartXml(OpenXmlPart part, string xml) + { + using var stream = part.GetStream(FileMode.Create, FileAccess.Write); + using var writer = new StreamWriter(stream, new System.Text.UTF8Encoding(false)); + writer.Write(xml); + } + + private static WmlDocument Consolidate( + WmlDocument baseDoc, ConflictResolution policy, params (string Author, WmlDocument Doc)[] reviewers) + => DocxDiff.Consolidate( + baseDoc, + reviewers.Select(r => new DocxDiffReviewer { Author = r.Author, Document = r.Doc }).ToList(), + new DocxDiffConsolidateSettings { ConflictResolution = policy }); + + // ------------------------------------------------------------------ oracles + + /// Every note reference of kind ANYWHERE (document body, footnotes part, + /// endnotes part) that does NOT resolve to exactly one non-reserved definition of + /// kind in its part. The cross-kind resolvability oracle (mirrors the two-way robustness test). + private static List UnresolvedRefs(WmlDocument doc, string refLocal, string defLocal, bool defsInFootnotesPart) + { + using var ms = new MemoryStream(doc.DocumentByteArray); + using var w = WordprocessingDocument.Open(ms, false); + var main = w.MainDocumentPart!; + var fnRoot = main.FootnotesPart?.GetXDocument().Root; + var enRoot = main.EndnotesPart?.GetXDocument().Root; + var defRoot = defsInFootnotesPart ? fnRoot : enRoot; + var defCounts = (defRoot?.Elements(Wns + defLocal) + .Where(e => e.Attribute(Wns + "type") == null) + .Select(e => (string?)e.Attribute(Wns + "id")) + .Where(x => x != null).Select(x => x!) ?? Enumerable.Empty()) + .GroupBy(x => x).ToDictionary(g => g.Key, g => g.Count()); + var refs = new List(); + var body = main.GetXDocument().Root?.Element(Wns + "body"); + if (body != null) refs.AddRange(body.Descendants(Wns + refLocal)); + if (fnRoot != null) refs.AddRange(fnRoot.Descendants(Wns + refLocal)); + if (enRoot != null) refs.AddRange(enRoot.Descendants(Wns + refLocal)); + return refs.Select(r => (string?)r.Attribute(Wns + "id")) + .Where(id => id == null || !defCounts.TryGetValue(id, out var n) || n != 1) + .Select(id => $"{refLocal}:{id ?? "(null)"}").ToList(); + } + + /// Assert BOTH cross-kind reference directions resolve everywhere in . + private static void AssertAllNoteRefsResolve(WmlDocument doc, string because) + { + Assert.True(UnresolvedRefs(doc, "footnoteReference", "footnote", defsInFootnotesPart: true).Count == 0, + $"{because}: dangling footnote refs -> {string.Join(",", UnresolvedRefs(doc, "footnoteReference", "footnote", true))}"); + Assert.True(UnresolvedRefs(doc, "endnoteReference", "endnote", defsInFootnotesPart: false).Count == 0, + $"{because}: dangling endnote refs -> {string.Join(",", UnresolvedRefs(doc, "endnoteReference", "endnote", false))}"); + } + + private static HashSet SchemaErrors(WmlDocument doc) + { + using var ms = new MemoryStream(doc.DocumentByteArray); + using var w = WordprocessingDocument.Open(ms, false); + return new OpenXmlValidator(FileFormatVersions.Office2019).Validate(w) + .Select(e => $"{e.Id}@{e.Path?.XPath}: {e.Description}").ToHashSet(); + } + + private static void AssertNoNewSchemaErrors(WmlDocument baseDoc, WmlDocument produced) + { + // Sem_MissingReferenceElement is EXCLUDED: OpenXmlValidator does not resolve a note reference that lives + // inside another note's definition body against the sibling notes part (a footnoteReference nested in an + // endnote body, or vice versa) — it false-positives on such refs even when they resolve, and the base doc + // already carries the same error (see DocxDiffFootnoteRobustnessTests.CrossKindNestedNoteReference_...). + // The base-subtraction cannot cancel it either, because the renumber legitimately changes the id embedded + // in the message string. Reference RESOLVABILITY is instead asserted by the structural AssertAllNoteRefsResolve + // oracle; every OTHER schema error (duplicate ids, malformed markup, etc.) is still caught here. + static HashSet Real(WmlDocument d) => + SchemaErrors(d).Where(e => !e.Contains("Sem_MissingReferenceElement")).ToHashSet(); + var baseErrors = Real(baseDoc); + var newErrors = Real(produced).Where(e => !baseErrors.Contains(e)).ToList(); + Assert.True(newErrors.Count == 0, $"new schema errors: {string.Join(" | ", newErrors.Take(5))}"); + } + + // ------------------------------------------------------------------ 1. base cross-kind nesting survives N-way renumber + + /// + /// The base has GAPPED note ids (fn 2/3, en 5/8) so the body-order renumber genuinely shifts every id, and + /// two cross-kind nested references (footnote 2 cites endnote 5, endnote 8 cites footnote 2). Two reviewers + /// each make an edit — Alice edits a note (which is what ENTERS the composite note-render path) and Bob edits + /// body text — so the renumber + cross-kind nested-reference sweep runs through Consolidate. Every reference, + /// including the two nested cross-kind ones, must still resolve on the merged doc AND on accept AND on reject. + /// + [Theory] + [InlineData(ConflictResolution.BaseWins)] + [InlineData(ConflictResolution.FirstReviewerWins)] + [InlineData(ConflictResolution.StackAll)] + public void CrossKind_nested_base_note_refs_survive_consolidate_renumber(ConflictResolution policy) + { + // fn2 cites en5 (cross-kind); en8 cites fn2 (reverse cross-kind); fn3/en5 are plain. Ids are gapped. + string body = + Para("Alpha cites the outer footnote.", FnRef(2)) + + Para("Beta cites the inner endnote.", EnRef(5)) + + Para("Gamma cites the other endnote.", EnRef(8)) + + Para("Delta cites the plain footnote.", FnRef(3)); + string fnDefs = + Footnote(2, "Outer footnote citing ", EnRef(5)) + + Footnote(3, "A plain footnote."); + string enDefs = + Endnote(5, "The inner endnote, also body-cited.") + + Endnote(8, "Endnote citing ", FnRef(2)); + + var b = Build(body, fnDefs, enDefs); + // Alice edits the plain footnote's text -> a real note diff, so the composite note-render path runs. + var alice = Build(body, Footnote(2, "Outer footnote citing ", EnRef(5)) + Footnote(3, "A plain footnote EDITED."), enDefs); + // Bob edits body text elsewhere -> genuine N-way merge. + var bob = Build( + Para("Alpha cites the outer footnote.", FnRef(2)) + + Para("Beta cites the inner endnote.", EnRef(5)) + + Para("Gamma cites the other endnote REVISED.", EnRef(8)) + + Para("Delta cites the plain footnote.", FnRef(3)), + fnDefs, enDefs); + + var merged = Consolidate(b, policy, ("Alice", alice), ("Bob", bob)); + var accepted = RevisionProcessor.AcceptRevisions(merged); + var rejected = RevisionProcessor.RejectRevisions(merged); + + AssertAllNoteRefsResolve(merged, "merged"); + AssertAllNoteRefsResolve(accepted, "accept"); + AssertAllNoteRefsResolve(rejected, "reject"); + AssertNoNewSchemaErrors(b, merged); + + // reject ≡ base body. + Assert.Equal(Docs.PlainText(b), Docs.PlainText(rejected)); + } + + // ------------------------------------------------------------------ 2. inserted footnote cites EXISTING endnote + + /// + /// A reviewer INSERTS a footnote whose body cites an EXISTING (base) endnote — a cross-kind nested reference + /// living inside a reviewer-inserted note definition. It lands under a fresh output id, and its nested + /// endnote reference must be remapped to the endnote's renumbered id (else it dangles). + /// + [Fact] + public void Reviewer_inserted_footnote_citing_existing_endnote_stays_resolvable() + { + string body = + Para("Alpha cites the footnote.", FnRef(2)) + + Para("Beta cites the endnote.", EnRef(5)); + string fnDefs = Footnote(2, "The base footnote."); + string enDefs = Endnote(5, "The base endnote."); + + var b = Build(body, fnDefs, enDefs); + // Alice adds a new paragraph with a NEW footnote (her id 3) whose body cites the existing endnote 5. + var alice = Build( + body + Para("Gamma adds a footnote.", FnRef(3)), + fnDefs + Footnote(3, "Inserted footnote citing ", EnRef(5)), + enDefs); + // Bob edits body text -> N-way. + var bob = Build( + Para("Alpha cites the footnote DELTA.", FnRef(2)) + Para("Beta cites the endnote.", EnRef(5)), + fnDefs, enDefs); + + var merged = Consolidate(b, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob)); + var accepted = RevisionProcessor.AcceptRevisions(merged); + var rejected = RevisionProcessor.RejectRevisions(merged); + + AssertAllNoteRefsResolve(merged, "merged"); + AssertAllNoteRefsResolve(accepted, "accept"); + AssertAllNoteRefsResolve(rejected, "reject"); + AssertNoNewSchemaErrors(b, merged); + } + + // ------------------------------------------------------------------ 3. inserted endnote cites EXISTING footnote + + /// Symmetric to #2: a reviewer inserts an ENDNOTE whose body cites an EXISTING footnote. + [Fact] + public void Reviewer_inserted_endnote_citing_existing_footnote_stays_resolvable() + { + string body = + Para("Alpha cites the footnote.", FnRef(2)) + + Para("Beta cites the endnote.", EnRef(5)); + string fnDefs = Footnote(2, "The base footnote."); + string enDefs = Endnote(5, "The base endnote."); + + var b = Build(body, fnDefs, enDefs); + // Alice adds a new paragraph with a NEW endnote (her id 8) whose body cites the existing footnote 2. + var alice = Build( + body + Para("Gamma adds an endnote.", EnRef(8)), + fnDefs, + enDefs + Endnote(8, "Inserted endnote citing ", FnRef(2))); + var bob = Build( + Para("Alpha cites the footnote DELTA.", FnRef(2)) + Para("Beta cites the endnote.", EnRef(5)), + fnDefs, enDefs); + + var merged = Consolidate(b, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob)); + var accepted = RevisionProcessor.AcceptRevisions(merged); + var rejected = RevisionProcessor.RejectRevisions(merged); + + AssertAllNoteRefsResolve(merged, "merged"); + AssertAllNoteRefsResolve(accepted, "accept"); + AssertAllNoteRefsResolve(rejected, "reject"); + AssertNoNewSchemaErrors(b, merged); + } + + // ------------------------------------------------------------------ 4. inserted footnote cites reviewer-INSERTED endnote + + /// + /// The hardest sub-case (the one the audit flagged as neither obviously handled nor exercised): a reviewer + /// inserts a footnote whose body cites an endnote the SAME reviewer inserts. The cited endnote receives a + /// FRESH output id, but the nested reference inside the inserted footnote definition still carries the + /// reviewer's own id — which the body-reference rewrite never touches (it only rewrites body clones). Its + /// nested reference must be rewritten to the endnote's fresh output id, then follow the renumber, or it + /// dangles on merge/accept/reject. + /// + [Fact] + public void Reviewer_inserted_footnote_citing_reviewer_inserted_endnote_stays_resolvable() + { + string body = + Para("Alpha cites the footnote.", FnRef(2)) + + Para("Beta cites the endnote.", EnRef(5)); + string fnDefs = Footnote(2, "The base footnote."); + string enDefs = Endnote(5, "The base endnote."); + + var b = Build(body, fnDefs, enDefs); + // Alice inserts a footnote (her id 3) whose body cites an endnote (her id 6) she ALSO inserts. + var alice = Build( + body + + Para("Gamma adds a footnote.", FnRef(3)) + + Para("Epsilon adds an endnote.", EnRef(6)), + fnDefs + Footnote(3, "Inserted footnote citing ", EnRef(6)), + enDefs + Endnote(6, "The inserted endnote.")); + var bob = Build( + Para("Alpha cites the footnote DELTA.", FnRef(2)) + Para("Beta cites the endnote.", EnRef(5)), + fnDefs, enDefs); + + var merged = Consolidate(b, ConflictResolution.BaseWins, ("Alice", alice), ("Bob", bob)); + var accepted = RevisionProcessor.AcceptRevisions(merged); + var rejected = RevisionProcessor.RejectRevisions(merged); + + AssertAllNoteRefsResolve(merged, "merged"); + AssertAllNoteRefsResolve(accepted, "accept"); + AssertAllNoteRefsResolve(rejected, "reject"); + AssertNoNewSchemaErrors(b, merged); + } +} diff --git a/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs b/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs index 0f297098..3e109e10 100644 --- a/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs +++ b/Docxodus/Ir/Diff/IrCompositeMarkupRenderer.cs @@ -282,10 +282,10 @@ private static void RenderCompositeNoteScopes( { ApplyCompositeNoteDiffs( noteOps.Where(n => n.Kind == IrNoteKind.Footnote).ToList(), isFootnote: true, - main, baseIr, reviewerIrs, state, freshIdByInserted, settings); + main, baseIr, reviewerIrs, state, freshIdByInserted, outputId, settings); ApplyCompositeNoteDiffs( noteOps.Where(n => n.Kind == IrNoteKind.Endnote).ToList(), isFootnote: false, - main, baseIr, reviewerIrs, state, freshIdByInserted, settings); + main, baseIr, reviewerIrs, state, freshIdByInserted, outputId, settings); } // ---- 4. Body-order renumber + cross-kind nested-reference sweep (the two-way pipeline). ---- @@ -327,6 +327,7 @@ private static void ApplyCompositeNoteDiffs( MainDocumentPart main, IrDocument baseIr, IReadOnlyList reviewerIrs, IrMarkupRenderer.RenderState state, IReadOnlyDictionary<(IrNoteKind Kind, int Reviewer, string ReviewerId), string> freshIdByInserted, + IReadOnlyDictionary<(int Reviewer, IrNoteKind Kind, string ReviewerId), string> outputId, IrDiffSettings settings) { if (diffs.Count == 0) @@ -376,6 +377,27 @@ private static void ApplyCompositeNoteDiffs( noteEl.Elements().Where(e => e.Name == W.p || e.Name == W.tbl).Remove(); noteEl.Add(noteBlocks); + + // A reviewer-INSERTED note is entirely reviewer-sourced (all-ins content, no base/del side), so every + // nested note reference in its body carries THAT reviewer's id space. Rewrite each to the output id + // space (matched -> base id, inserted -> fresh id) so the body-order renumber sweep below then carries + // it to the final id. Without this, a nested reference to ANOTHER note the same reviewer inserted keeps + // the reviewer id and dangles: step 2's reference rewrite only visits body clones, not inserted-note + // definition bodies, and step 4's remap is keyed on the output-old id, which the reviewer id is not. + if (diff.BaseNoteId == null) + { + foreach (var refEl in noteEl.Descendants() + .Where(e => e.Name == W.footnoteReference || e.Name == W.endnoteReference)) + { + var refKind = refEl.Name == W.footnoteReference ? IrNoteKind.Footnote : IrNoteKind.Endnote; + var refId = (string?)refEl.Attribute(W.id); + if (refId != null + && outputId.TryGetValue((diff.SourceReviewer, refKind, refId), out var mapped) + && mapped != refId) + refEl.SetAttributeValue(W.id, mapped); + } + } + changed = true; } diff --git a/docs/architecture/ir_diff_engine.md b/docs/architecture/ir_diff_engine.md index aa455084..ded618fa 100644 --- a/docs/architecture/ir_diff_engine.md +++ b/docs/architecture/ir_diff_engine.md @@ -72,7 +72,7 @@ The `IrEditScript` is an ordered list of block operations (`IrEditOpKind`), plus - **Definition ids are unique** and every body reference resolves to exactly one definition. A `continuationNotice` (or any typed) reserved note at a *positive* id keeps its id and leads the part; real notes renumber to a range disjoint from the kept reserved ids (the counter seeds above the highest positive reserved id), so a reserved note never collides with a renumbered real note. - **A reference is never dropped.** A footnote/endnote reference is a zero-width inline; the token-diff run rebuilder (`SourceRunModel.Slice`) attributes a boundary zero-width to exactly the op whose token range owns it (`ZeroWidthBoundaries`), so a reference at the tail of an edited paragraph survives and is never double-counted. - **A reference-deleted note's definition is emitted once.** When an edit removes a reference but the definition lingers in both stores, the builder reconciles the orphan into a single matched pair (identity = `w:id`) rather than a delete + an insert of the same id; the renumber pass links a `w:del` reference to its preserved definition so it stays resolvable on reject even when its id ≠ its reference ordinal. -- **A note-in-note reference is renumbered with its target.** A footnote/endnote whose body cites another note keeps a reference inside its definition body; the body-only renumber walk never visits it, so `RenumberNoteIds` records each definition's old→new id and remaps same-kind references inside the note part afterwards — otherwise a nested reference to a renumbered note dangles on accept/reject (cross-kind nesting — an endnote ref inside a footnote body — is a documented v1 gap). +- **A note-in-note reference is renumbered with its target.** A footnote/endnote whose body cites another note keeps a reference inside its definition body; the body-only renumber walk never visits it, so `RenumberNoteIds` records each definition's old→new id and `RemapNestedNoteReferences` sweeps BOTH note parts once — after both kinds' passes — with both maps, so SAME-kind nesting (a footnote citing a footnote) AND CROSS-kind nesting (an endnote ref inside a footnote body, or vice versa) are both remapped and neither dangles on accept/reject. (Note-in-note references are valid OOXML but LibreOffice Writer's DOCX import cannot load *any* document that contains one, cross-kind or same-kind — so the round-trip oracle here is the structural reference-resolvability check and Word, not LibreOffice.) **Comment fidelity (Covered).** Comments (`w:commentRangeStart`/`End`/`w:commentReference`) are dropped from IR paragraphs (reader rule N15 records them as `IrCommentStore` char-offset spans for the markdown projection only), but they are carried through the fine token-diff path as `AlwaysKeep` zero-width markers — exactly like bookmarks — so an edited commented paragraph gets **fine per-word `w:ins`/`w:del`** markup with its comment anchors intact (no whole-block bail). Three passes guarantee integrity, the comment analogue of `NormalizeBookmarks`: @@ -243,7 +243,7 @@ The whole-corpus juxtaposition-vs-inline-merge shape divergence is the deliberat ### v1 limitations (honest) - **Header/footer scopes are NOT consolidated (2026-07-03 campaign ceiling).** The two-way engine diffs header/footer stories (`IrEditScript.HeaderFooterOps`), but `IrCompositeMerger.Merge` forces `CompareHeadersFooters` off for its per-reviewer diffs — explicit and deterministic (nothing is generated, so nothing is silently dropped), pinned by `IrHeaderFooterDiffTests.Consolidate_ignores_header_changes_v1_ceiling`. A reviewer's header edit is ignored by `Consolidate`; run a two-way `Compare` against that reviewer to capture it. Follow-on: `MergeHeaderFooterScopes` mirroring `MergeNoteScopes` — simpler than notes (story pairing is by scope/part, no id-map/reference-rewrite machinery). -- **Note scopes MERGE across reviewers.** The merger builds composite note-scope (footnote/endnote) ops (`IrCompositeMerger.MergeNoteScopes`): a base-matched note's blocks run the SAME `MergeBlockStream` dispatch the body uses (disjoint note edits compose, identical ones reach consensus, contested ones are recorded conflicts resolved by the policy; a whole-note delete vs an edit is a delete-vs-modify conflict), and reviewer-INSERTED notes pass through authored. The composite renderer applies the composed ops inside the footnotes/endnotes parts, creates reviewer-inserted definitions under fresh output ids, rewrites reviewer-sourced body references from each reviewer's id space into the base-anchored output space (`IrCompositeScript.NoteIdMaps` + `RenderState.NoteRefClonesBySource`; deleted/moved-from content is base-sourced and skipped), then runs the SAME body-order renumber + cross-kind nested-reference sweep the two-way renderer runs. Structural ops INSIDE a note (a split/merge/move of note paragraphs) are conservatively lowered to del/ins (content-preserving); native in-note structural composition is a follow-on. Consolidated revisions cover note edits (appended after body ops, footnotes then endnotes), and the parity scoreboard's accept ≡ right metric now includes referenced note texts. +- **Note scopes MERGE across reviewers.** The merger builds composite note-scope (footnote/endnote) ops (`IrCompositeMerger.MergeNoteScopes`): a base-matched note's blocks run the SAME `MergeBlockStream` dispatch the body uses (disjoint note edits compose, identical ones reach consensus, contested ones are recorded conflicts resolved by the policy; a whole-note delete vs an edit is a delete-vs-modify conflict), and reviewer-INSERTED notes pass through authored. The composite renderer applies the composed ops inside the footnotes/endnotes parts, creates reviewer-inserted definitions under fresh output ids, rewrites reviewer-sourced body references from each reviewer's id space into the base-anchored output space (`IrCompositeScript.NoteIdMaps` + `RenderState.NoteRefClonesBySource`; deleted/moved-from content is base-sourced and skipped), then runs the SAME body-order renumber + cross-kind nested-reference sweep the two-way renderer runs. A reviewer-inserted note is all-`ins` content in that reviewer's id space, so a note-in-note reference INSIDE its definition body (which the body-reference rewrite never visits) is also rewritten to the output id space before the renumber — closing the sub-case where a reviewer inserts a footnote citing an endnote the same reviewer inserts (whose target id becomes a fresh output id, not a base id): without it, that nested cross-kind reference kept the reviewer id and dangled. Same-kind and cross-kind note-in-note nesting therefore stay resolvable on merge/accept/reject across N reviewers (`IrCompositeCrossKindNoteTests`). Structural ops INSIDE a note (a split/merge/move of note paragraphs) are conservatively lowered to del/ins (content-preserving); native in-note structural composition is a follow-on. Consolidated revisions cover note edits (appended after body ops, footnotes then endnotes), and the parity scoreboard's accept ≡ right metric now includes referenced note texts. - **Multi-reviewer table edits compose per-cell, including column changes and cell shells.** DISJOINT cross-reviewer table-cell edits COMPOSE inline (Alice edits cell(0,0), Bob edits cell(1,2) → both land, attributed; disjoint words inside one cell paragraph fuse via the recursion); edits to the SAME cell by ≥2 reviewers become a recorded conflict resolved by the policy. Cell ops pair by BASE cell anchor, so a reviewer's **column add/remove composes**: an added cell renders with native `w:tcPr/w:cellIns` (kept on accept, removed on reject), a removed cell with `w:tcPr/w:cellDel` (removed on accept, restored on reject — Word's own accept semantics absorb the removed cell's grid slot into the preceding cell's `gridSpan`); a cell delete-vs-edit is a recorded conflict. **Cell-shell (`w:tcPr`) edits are first-class**: the shell digest participates in the cell `ContentHash` (`IrCell.ShellDigest`), so a width/gridSpan/vMerge/shading-only edit is visible (was: invisible — classified `EqualBlock` and silently dropped); the composed cell's shell is sourced from its editing reviewer, agreeing shells reach consensus, competing shells are a recorded conflict (BaseWins keeps the base shell). The remaining STOP boundary: a **CONTESTED `MovedRow`** (the same base row also edited/deleted/moved by another reviewer) falls back to a whole-table **block conflict** (no silent loss — the base table is kept under `BaseWins` and the disagreement is recorded under every policy); an UNCONTESTED row move composes (lowered to the two-way renderer's own del+ins row shape). Notes: shell/tblPr application is not `tcPrChange`/`tblPrChange`-tracked (reject restores text, not shell bytes — same class as the two-way renderer's right-shelled table render); a 2-way column ADD in the SINGLE-toucher passthrough still renders via the whole-table fallback (a pre-existing two-way renderer gap, independent of composition). - **Non-colliding reviewer moves/splits/merges render natively; colliding ones are lowered to del/ins as a recorded conflict.** A reviewer's `MoveBlock`/`MoveModifyBlock`/`SplitBlock`/`MergeBlock` whose consumed base block(s) are touched by ONLY that reviewer renders NATIVELY (`PlanMoves`/`ApplyMovePlan` for moves with globally-namespaced move-group ids; `ApplySplitMergePlan` for splits/merges — the same sole-toucher predicate via the shared `BuildTouchers` map). A native `MergeBlock` (null left anchor, N consumed base anchors) dispatches through `MergeBlockStream`'s first-consumed-anchor index; the remaining consumed anchors emit no block op but keep insert slotting, and `GroupInsertsByPrecedingAnchor` advances past the merge's consumed region so a following insert lands after the whole merge markup. A COLLIDING structural op (another reviewer touches a consumed base block) is LOWERED: `LowerOneStructuralOp` rewrites the move SOURCE → `DeleteBlock` (retaining `MoveGroupId`/`IsMoveSource` as a relocation marker, stripped before emission), the move DEST → `InsertBlock`, a split → `DeleteBlock` + N ordered `InsertBlock`s, a merge → N ordered `DeleteBlock`s + an `InsertBlock` — preserving op order; **content is fully preserved and round-trips** (accept shows the moved/split/merged text, reject ≡ base), and the collision resolves through the existing conflict machinery. Cell mini-bodies run the same split/merge plan (fixing a silent drop: a `MergeBlock` inside a multi-editor cell reached neither grouping map and vanished). Two reviewers relocating the SAME base block to different places collide on the lowered source-delete → a recorded **placement conflict** (the consensus removal is emitted once for `reject ≡ base`; each reviewer's relocating insert survives independently — no loss; this conflict is NOT policy-resolved into the op stream, so a `BaseWins` flip never wrongly restores a block both reviewers removed). - **Reviewers' BLOCK-FORMAT changes MERGE — paragraph (B1) + table-shell + section (B2).** The internal `TrackBlockFormatChanges` is sliced into `TrackParagraphFormatChanges`/`TrackTableFormatChanges`/`TrackSectionFormatChanges` (each defaults equal, so two-way is byte-identical); `IrCompositeMerger` forces the umbrella OFF but turns all three slices ON. Every family composes by a per-element digest mirroring `ComposeCellShell` (0 changers → base, all agree → consensus/first reviewer, ≥2 distinct → a recorded conflict resolved by policy; a shell/pPr/section cannot STACK):