From c350b176678c7aef0f0fd3f6227c4a610a15dbf8 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Fri, 14 Aug 2026 00:33:35 -0500 Subject: [PATCH] fix(session): track structured bulk deletes (#473) --- CHANGELOG.md | 9 + ...DocxSessionTrackedStructuredDeleteTests.cs | 370 ++++++++++++++++++ Docxodus/DocxSession.cs | 166 +++++--- Docxodus/Internal/StructuredRevisionOps.cs | 54 +++ Docxodus/Ir/Diff/IrMarkupRenderer.cs | 17 +- docs/architecture/docx_mutation_api.md | 39 +- 6 files changed, 587 insertions(+), 68 deletions(-) create mode 100644 Docxodus.Tests/DocxSessionTrackedStructuredDeleteTests.cs create mode 100644 Docxodus/Internal/StructuredRevisionOps.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 19a0fb42..ca402d7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,6 +89,15 @@ All notable changes to this project will be documented in this file. version bump at release time. ### Fixed +- **Tracked `DeleteRange` / `DeleteSection` no longer hard-remove block content + controls.** Block `w:sdt` envelopes now use Word-native paired custom-XML + deletion ranges while paragraphs, tables, and nested controls are marked + recursively, so accept removes the selection and reject restores locked or + data-bound controls intact. Anchors retained beneath a control are reported as + `Modified`, structural fall-through anchors as `Removed`, and ranges containing + unsupported `w:customXml` wrappers fail atomically with + `IncompatibleElementType` instead of silently deleting them. Paragraph-mark + revision properties are also inserted in schema order for styled headings. - One undo step is now always retained even when a single snapshot exceeds the whole budget, so undo cannot silently become unavailable on exactly the large documents where a mistaken edit is most expensive to lose. diff --git a/Docxodus.Tests/DocxSessionTrackedStructuredDeleteTests.cs b/Docxodus.Tests/DocxSessionTrackedStructuredDeleteTests.cs new file mode 100644 index 00000000..2a3496ec --- /dev/null +++ b/Docxodus.Tests/DocxSessionTrackedStructuredDeleteTests.cs @@ -0,0 +1,370 @@ +#nullable enable + +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Xml.Linq; +using Docxodus; +using DocumentFormat.OpenXml; +using DocumentFormat.OpenXml.Packaging; +using DocumentFormat.OpenXml.Validation; +using DocumentFormat.OpenXml.Wordprocessing; +using Xunit; +using WLock = DocumentFormat.OpenXml.Wordprocessing.Lock; +using WTable = DocumentFormat.OpenXml.Wordprocessing.Table; +using WTableCell = DocumentFormat.OpenXml.Wordprocessing.TableCell; +using WTableRow = DocumentFormat.OpenXml.Wordprocessing.TableRow; + +namespace Docxodus.Tests; + +public class DocxSessionTrackedStructuredDeleteTests +{ + [Fact] + public void DS473_DeleteRange_TracksBlockContentControlInsteadOfHardRemovingIt() + { + using var session = OpenTrackedSession(BuildDocument( + ParagraphWithText("before"), + ParagraphWithText("delete start"), + BlockControl("controlled", ParagraphWithText("controlled paragraph")), + ParagraphWithText("after"))); + var projection = session.Project(); + var from = FindByText(session, projection, "delete start"); + var controlled = FindByText(session, projection, "controlled paragraph"); + var to = FindByText(session, projection, "after"); + + var result = session.DeleteRange(from, to); + + Assert.True(result.Success, result.Error?.Message); + AssertAnchorAccounting(result, new[] { from, controlled }, Array.Empty()); + + var tracked = session.Save(); + var body = Body(tracked); + var control = Assert.Single(body.Elements(W.sdt)); + AssertEnvelopeRangeTopology(control, control.Element(W.sdtContent)!); + AssertSchemaValid(tracked); + } + + [Fact] + public void DS474_NestedLockedDataBoundControls_TrackAndRoundTrip() + { + var outer = LockedBoundControl( + "outer", + ParagraphWithText("outer paragraph"), + BlockControl("inner", ParagraphWithText("inner paragraph"))); + using var session = OpenTrackedSession(BuildDocument( + ParagraphWithText("before"), + ParagraphWithText("delete start"), + outer, + ParagraphWithText("after"))); + var projection = session.Project(); + var from = FindByText(session, projection, "delete start"); + var outerParagraph = FindByText(session, projection, "outer paragraph"); + var innerParagraph = FindByText(session, projection, "inner paragraph"); + var to = FindByText(session, projection, "after"); + + var result = session.DeleteRange(from, to); + + Assert.True(result.Success, result.Error?.Message); + AssertAnchorAccounting( + result, + new[] { from, outerParagraph, innerParagraph }, + Array.Empty()); + + var tracked = session.Save(); + var trackedBody = Body(tracked); + var trackedOuter = Assert.Single(trackedBody.Elements(W.sdt)); + var trackedInner = Assert.Single(trackedOuter.Descendants(W.sdt)); + AssertEnvelopeRangeTopology(trackedOuter, trackedOuter.Element(W.sdtContent)!); + AssertEnvelopeRangeTopology(trackedInner, trackedInner.Element(W.sdtContent)!); + Assert.Equal("sdtLocked", (string?)trackedOuter.Element(W.sdtPr)?.Element(W._lock)?.Attribute(W.val)); + Assert.Equal("/root/value", (string?)trackedOuter.Element(W.sdtPr)?.Element(W.dataBinding)?.Attribute(W.xpath)); + AssertSchemaValid(tracked); + + var accepted = Resolve(tracked, accept: true); + var acceptedBody = Body(accepted); + Assert.Empty(acceptedBody.Descendants(W.sdt)); + Assert.DoesNotContain("outer paragraph", acceptedBody.Value); + Assert.DoesNotContain("inner paragraph", acceptedBody.Value); + AssertSchemaValid(accepted); + + var rejected = Resolve(tracked, accept: false); + var rejectedBody = Body(rejected); + Assert.Equal(2, rejectedBody.Descendants(W.sdt).Count()); + var rejectedOuter = Assert.Single(rejectedBody.Elements(W.sdt)); + Assert.Equal("sdtLocked", (string?)rejectedOuter.Element(W.sdtPr)?.Element(W._lock)?.Attribute(W.val)); + Assert.Equal("/root/value", (string?)rejectedOuter.Element(W.sdtPr)?.Element(W.dataBinding)?.Attribute(W.xpath)); + Assert.Contains("outer paragraph", rejectedBody.Value); + Assert.Contains("inner paragraph", rejectedBody.Value); + AssertSchemaValid(rejected); + } + + [Fact] + public void DS475_ControlContainingTable_TracksEveryDescendantAnchorAndRoundTrips() + { + using var session = OpenTrackedSession(BuildDocument( + ParagraphWithText("before"), + ParagraphWithText("delete start"), + BlockControl("table-control", TwoCellTable()), + ParagraphWithText("after"))); + var projection = session.Project(); + var from = FindByText(session, projection, "delete start"); + var to = FindByText(session, projection, "after"); + var table = projection.AnchorIndex.Values.Single(target => target.Anchor.Kind == "tbl"); + var tableXml = XElement.Parse(session.Raw.GetXml(table.Anchor.Id)); + var tableUnids = tableXml.DescendantsAndSelf() + .Select(e => (string?)e.Attribute(PtOpenXml.Unid)) + .Where(id => id is not null) + .ToHashSet(StringComparer.Ordinal); + var expectedModified = projection.AnchorIndex.Values + .Where(target => tableUnids.Contains(target.Unid)) + .Select(target => target.Anchor.Id) + .Append(from) + .Distinct(StringComparer.Ordinal) + .ToList(); + + var result = session.DeleteRange(from, to); + + Assert.True(result.Success, result.Error?.Message); + AssertAnchorAccounting(result, expectedModified, Array.Empty()); + + var tracked = session.Save(); + var trackedBody = Body(tracked); + var control = Assert.Single(trackedBody.Elements(W.sdt)); + AssertEnvelopeRangeTopology(control, control.Element(W.sdtContent)!); + Assert.Single(control.Descendants(W.tr)); + Assert.Single(control.Descendants(W.trPr).Elements(W.del)); + Assert.Equal(2, control.Descendants(W.p) + .Count(p => p.Element(W.pPr)?.Element(W.rPr)?.Element(W.del) is not null)); + AssertSchemaValid(tracked); + + var accepted = Resolve(tracked, accept: true); + Assert.Empty(Body(accepted).Descendants(W.sdt)); + Assert.Empty(Body(accepted).Descendants(W.tbl)); + Assert.DoesNotContain("Cell A", Body(accepted).Value); + Assert.DoesNotContain("Cell B", Body(accepted).Value); + AssertSchemaValid(accepted); + + var rejected = Resolve(tracked, accept: false); + Assert.Single(Body(rejected).Descendants(W.sdt)); + Assert.Single(Body(rejected).Descendants(W.tbl)); + Assert.Contains("Cell A", Body(rejected).Value); + Assert.Contains("Cell B", Body(rejected).Value); + AssertSchemaValid(rejected); + } + + [Fact] + public void DS476_CustomXmlBlock_FailsBeforeMutationWithStructuredError() + { + using var session = OpenTrackedSession(BuildDocument( + ParagraphWithText("before"), + ParagraphWithText("delete start"), + CustomXmlBlock("clause", ParagraphWithText("custom payload")), + ParagraphWithText("after"))); + var projection = session.Project(); + var from = FindByText(session, projection, "delete start"); + var customParagraph = FindByText(session, projection, "custom payload"); + var to = FindByText(session, projection, "after"); + + var before = session.Save(); + var result = session.DeleteRange(from, to); + + Assert.False(result.Success); + Assert.Equal(EditErrorCode.IncompatibleElementType, result.Error?.Code); + Assert.Contains("w:customXml", result.Error?.Message); + AssertAnchorAccounting(result, Array.Empty(), Array.Empty()); + Assert.Equal(0, session.UndoCount); + + var after = session.Save(); + Assert.True(XNode.DeepEquals(Body(before), Body(after))); + var preserved = Assert.Single(Body(after).Elements(W.customXml)); + Assert.Equal("clause", (string?)preserved.Attribute(W.element)); + Assert.Contains("custom payload", preserved.Value); + Assert.Equal("custom payload", session.GetAnchorInfo(customParagraph)?.TextPreview); + AssertSchemaValid(after); + } + + [Fact] + public void DS477_DeleteSection_TracksControlAndReportsSectionPropertyFallThrough() + { + using var session = OpenTrackedSession(BuildDocument( + Heading("Delete section"), + BlockControl("section-control", ParagraphWithText("controlled section payload")), + new SectionProperties(new PageSize { Width = 12240, Height = 15840 }))); + var projection = session.Project(); + var heading = FindByText(session, projection, "Delete section"); + var controlled = FindByText(session, projection, "controlled section payload"); + var section = projection.AnchorIndex.Values.Single(target => target.Anchor.Kind == "sec").Anchor.Id; + + var result = session.DeleteSection(heading); + + Assert.True(result.Success, result.Error?.Message); + AssertAnchorAccounting(result, new[] { heading, controlled }, new[] { section }); + var tracked = session.Save(); + Assert.Single(Body(tracked).Elements(W.sdt)); + Assert.Empty(Body(tracked).Elements(W.sectPr)); + AssertSchemaValid(tracked); + } + + private static DocxSession OpenTrackedSession(byte[] bytes) => + new(bytes, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + RevisionAuthor = "issue-473", + }); + + private static string FindByText( + DocxSession session, + MarkdownProjection projection, + string text) => + projection.AnchorIndex.Values + .Single(target => session.GetAnchorInfo(target.Anchor.Id)?.TextPreview == text) + .Anchor.Id; + + private static Paragraph ParagraphWithText(string text) => + new(new Run(new Text(text))); + + private static SdtBlock BlockControl(string tag, params OpenXmlElement[] content) => + new( + new SdtProperties(new Tag { Val = tag }), + new SdtContentBlock(content)); + + private static SdtBlock LockedBoundControl(string tag, params OpenXmlElement[] content) => + new( + new SdtProperties( + new Tag { Val = tag }, + new WLock { Val = LockingValues.SdtLocked }, + new DataBinding + { + StoreItemId = "{11111111-1111-1111-1111-111111111111}", + XPath = "/root/value", + PrefixMappings = "xmlns:x='urn:docxodus:test'", + }), + new SdtContentBlock(content)); + + private static CustomXmlBlock CustomXmlBlock(string element, params OpenXmlElement[] content) + { + var customXml = new CustomXmlBlock(new CustomXmlProperties()) + { + Uri = "urn:docxodus:test", + Element = element, + }; + customXml.Append(content); + return customXml; + } + + private static Paragraph Heading(string text) => + new( + new ParagraphProperties(new ParagraphStyleId { Val = "Heading1" }), + new Run(new Text(text))); + + private static WTable TwoCellTable() => + new( + new TableProperties(new TableWidth { Width = "5000", Type = TableWidthUnitValues.Dxa }), + new TableGrid( + new GridColumn { Width = "2500" }, + new GridColumn { Width = "2500" }), + new WTableRow( + TableCell("Cell A"), + TableCell("Cell B"))); + + private static WTableCell TableCell(string text) => + new( + new TableCellProperties( + new TableCellWidth { Width = "2500", Type = TableWidthUnitValues.Dxa }), + ParagraphWithText(text)); + + private static byte[] BuildDocument(params OpenXmlElement[] blocks) + { + using var stream = new MemoryStream(); + using (var document = WordprocessingDocument.Create( + stream, + WordprocessingDocumentType.Document)) + { + var main = document.AddMainDocumentPart(); + main.Document = new Document(new Body(blocks)); + main.AddNewPart().Styles = new Styles( + new DocDefaults(), + new Style(new StyleName { Val = "heading 1" }) + { + Type = StyleValues.Paragraph, + StyleId = "Heading1", + }); + main.AddNewPart().Settings = new Settings(); + document.Save(); + } + + return stream.ToArray(); + } + + private static byte[] Resolve(byte[] tracked, bool accept) + { + var document = new WmlDocument("tracked.docx", tracked); + return (accept + ? RevisionProcessor.AcceptRevisions(document) + : RevisionProcessor.RejectRevisions(document)).DocumentByteArray; + } + + private static XElement Body(byte[] bytes) + { + using var stream = new MemoryStream(bytes); + using var document = WordprocessingDocument.Open(stream, false); + return new XElement(document.MainDocumentPart!.GetXDocument().Root!.Element(W.body)!); + } + + private static void AssertEnvelopeRangeTopology( + XElement wrapper, + XElement contentContainer) + { + var parent = Assert.IsType(wrapper.Parent); + var siblings = parent.Elements().ToList(); + var wrapperIndex = siblings.IndexOf(wrapper); + Assert.InRange(wrapperIndex, 1, siblings.Count - 2); + + var before = siblings[wrapperIndex - 1]; + var after = siblings[wrapperIndex + 1]; + Assert.Equal(W.customXmlDelRangeStart, before.Name); + Assert.Equal(W.customXmlDelRangeEnd, after.Name); + Assert.Equal("issue-473", (string?)before.Attribute(W.author)); + Assert.NotNull(before.Attribute(W.date)); + + var payload = contentContainer.Elements().ToList(); + var openingEnd = payload[0]; + var closingStart = payload[^1]; + Assert.Equal(W.customXmlDelRangeEnd, openingEnd.Name); + Assert.Equal(W.customXmlDelRangeStart, closingStart.Name); + Assert.Equal((string?)before.Attribute(W.id), (string?)openingEnd.Attribute(W.id)); + Assert.Equal((string?)closingStart.Attribute(W.id), (string?)after.Attribute(W.id)); + Assert.NotEqual((string?)before.Attribute(W.id), (string?)closingStart.Attribute(W.id)); + } + + private static void AssertAnchorAccounting( + EditResult result, + IEnumerable modified, + IEnumerable removed) + { + var expectedModified = modified.ToHashSet(StringComparer.Ordinal); + var expectedRemoved = removed.ToHashSet(StringComparer.Ordinal); + var actualModified = result.Modified.Select(anchor => anchor.Id).ToHashSet(StringComparer.Ordinal); + var actualRemoved = result.Removed.Select(anchor => anchor.Id).ToHashSet(StringComparer.Ordinal); + + Assert.Equal(expectedModified.OrderBy(id => id), actualModified.OrderBy(id => id)); + Assert.Equal(expectedRemoved.OrderBy(id => id), actualRemoved.OrderBy(id => id)); + Assert.Empty(actualModified.Intersect(actualRemoved)); + Assert.Equal(actualModified.Count, result.Modified.Count); + Assert.Equal(actualRemoved.Count, result.Removed.Count); + } + + private static void AssertSchemaValid(byte[] bytes) + { + using var stream = new MemoryStream(bytes); + using var document = WordprocessingDocument.Open(stream, false); + var errors = new OpenXmlValidator().Validate(document).ToList(); + Assert.True( + errors.Count == 0, + "Unexpected schema errors:\n" + string.Join("\n", errors.Select(error => error.Description))); + } +} diff --git a/Docxodus/DocxSession.cs b/Docxodus/DocxSession.cs index c38aba57..6d4a28e7 100644 --- a/Docxodus/DocxSession.cs +++ b/Docxodus/DocxSession.cs @@ -4274,9 +4274,15 @@ public EditResult DeleteBlock(string anchorId) /// w:pPr/w:rPr/w:del; each table row gets a w:trPr/w:del marker with /// its cell paragraphs wrapped recursively. Anchors stay live ( /// instead of ) so callers can re-address the same - /// blocks before changes are accepted. Block-level elements other than w:p - /// and w:tbl (e.g. w:sdt) are still structurally removed in this mode - /// — issue #177 follow-up if a consumer needs them tracked. + /// blocks before changes are accepted. Block-level w:sdt content controls use + /// paired w:customXmlDelRangeStart/End ranges for their envelopes plus + /// recursively tracked payload blocks (issue #473). Locked and data-bound controls use + /// the same shape: their metadata remains untouched until the revision is resolved. + /// Ranges containing w:customXml are rejected with + /// before mutation because this API + /// does not yet implement reversible deletion of that wrapper. Any other structural + /// fall-through is reported in rather than silently + /// disappearing. /// public EditResult DeleteRange(string fromAnchorId, string toAnchorIdExclusive) { @@ -4325,8 +4331,8 @@ public EditResult DeleteRange(string fromAnchorId, string toAnchorIdExclusive) /// "Level" is the same notion uses for the projection: /// Heading1 = 1, Heading2 = 2, etc.; Title = 1, Subtitle = 2. /// Tracked-change mode inherits 's behavior via the shared - /// DeleteSiblingRangeCore helper: paragraphs and tables are wrapped in - /// w:del markup rather than removed. + /// DeleteSiblingRangeCore helper, including native w:sdt envelope + /// deletion, anchor accounting, and the pre-mutation w:customXml refusal. /// public EditResult DeleteSection(string headingAnchorId) { @@ -4386,6 +4392,16 @@ private EditResult DeleteSiblingRangeCore( "'to' anchor does not follow 'from' in document order", anchorForPatchScope.Anchor.Id); + if (_trackedChanges == TrackedChangeMode.RenderInline && + toRemove.Any(element => + element.Name == W.customXml || element.Descendants(W.customXml).Any())) + { + return EditResult.Fail( + EditErrorCode.IncompatibleElementType, + "Tracked DeleteRange/DeleteSection does not support w:customXml wrappers; no changes were made.", + anchorForPatchScope.Anchor.Id); + } + _history.RecordPreOp(TakeSnapshot()); try { @@ -4396,43 +4412,59 @@ private EditResult DeleteSiblingRangeCore( { // Tracked-change path: mark each block with w:del markup rather than // removing it. Anchors stay live in the document tree so callers can - // re-address the same blocks before changes are accepted. Only the - // top-level block anchors are reported as Modified — descendants stay - // resolvable too, but enumerating them all would be noise (matches - // DeleteBlock's single-anchor contract in tracked mode). + // re-address the same blocks before changes are accepted. Ordinary + // paragraphs/tables retain DeleteBlock's single top-level-anchor + // contract. Structured wrappers have no anchor of their own, so every + // descendant anchor they keep live is reported as Modified. A remaining + // structural fall-through is a real removal and is reported as such. var modified = new List(); + var trackedRemoved = new List(); + var modifiedIds = new HashSet(StringComparer.Ordinal); + var trackedRemovedIds = new HashSet(StringComparer.Ordinal); foreach (var el in toRemove) { - var elUnid = (string?)el.Attribute(PtOpenXml.Unid); - if (elUnid is not null) - { - foreach (var kv in index) - if (kv.Value.Unid == elUnid) - modified.Add(kv.Value.Anchor); - } if (el.Name == W.p) + { + CollectAnchors(el, includeDescendants: false, index, modified, modifiedIds); MarkParagraphAsTrackedDeleted(el); + } else if (el.Name == W.tbl) + { + CollectAnchors(el, includeDescendants: false, index, modified, modifiedIds); MarkTableAsTrackedDeleted(el); + } + else if (el.Name == W.sdt) + { + CollectAnchors(el, includeDescendants: true, index, modified, modifiedIds); + MarkStructuredBlockAsTrackedDeleted(el); + } else - // Block kinds beyond w:p/w:tbl (e.g. w:sdt) — v1 falls back - // to structural removal for these, per the issue-#177 docstring. + { + CollectAnchors( + el, + includeDescendants: true, + index, + trackedRemoved, + trackedRemovedIds); el.Remove(); + } } InvalidateProjectionCache(); return new EditResult { Success = true, Modified = modified, + Removed = trackedRemoved, Patch = PatchFor(anchorForPatchScope), }; } var removed = new List(); + var removedIds = new HashSet(StringComparer.Ordinal); foreach (var el in toRemove) { // Collect this element's anchor plus every descendant anchor. - CollectAnchorsForRemoval(el, index, removed); + CollectAnchors(el, includeDescendants: true, index, removed, removedIds); el.Remove(); } InvalidateProjectionCache(); @@ -4451,25 +4483,21 @@ private EditResult DeleteSiblingRangeCore( } } - private static void CollectAnchorsForRemoval( + private static void CollectAnchors( XElement el, + bool includeDescendants, IReadOnlyDictionary index, - List removed) + List destination, + HashSet seenIds) { - var elUnid = (string?)el.Attribute(PtOpenXml.Unid); - if (elUnid is not null) - { - foreach (var kv in index) - if (kv.Value.Unid == elUnid) - removed.Add(kv.Value.Anchor); - } - foreach (var desc in el.Descendants()) + var candidates = includeDescendants ? el.DescendantsAndSelf() : new[] { el }; + foreach (var candidate in candidates) { - var dUnid = (string?)desc.Attribute(PtOpenXml.Unid); - if (dUnid is null) continue; - foreach (var kv in index) - if (kv.Value.Unid == dUnid) - removed.Add(kv.Value.Anchor); + var unid = (string?)candidate.Attribute(PtOpenXml.Unid); + if (unid is null) continue; + var anchor = index.Values.FirstOrDefault(target => target.Unid == unid)?.Anchor; + if (anchor is { } found && seenIds.Add(found.Id)) + destination.Add(found); } } @@ -9801,12 +9829,7 @@ private void MarkParagraphAsTrackedDeleted(XElement paragraph) pPr = new XElement(W.pPr); paragraph.AddFirst(pPr); } - var rPr = pPr.Element(W.rPr); - if (rPr is null) - { - rPr = new XElement(W.rPr); - pPr.AddFirst(rPr); - } + var rPr = GetOrCreatePPrChild(pPr, W.rPr); if (rPr.Element(W.del) is null) { var author = _revisionAuthor ?? "docxodus"; @@ -9843,16 +9866,67 @@ private void MarkTableAsTrackedDeleted(XElement table) foreach (var cell in row.Elements(W.tc)) { foreach (var child in cell.Elements().ToList()) - { - if (child.Name == W.p) - MarkParagraphAsTrackedDeleted(child); - else if (child.Name == W.tbl) - MarkTableAsTrackedDeleted(child); - } + MarkTrackedStructuredContentChild(child); } } } + /// + /// Tracks deletion of a block w:sdt wrapper without + /// discarding its ownership metadata. Two paired custom-XML deletion ranges cross + /// the opening and closing tags, while every payload block receives its ordinary + /// paragraph/table deletion markup. Accept therefore removes both wrapper and + /// payload; reject restores the original wrapper and content. + /// + private void MarkStructuredBlockAsTrackedDeleted(XElement wrapper) + { + if (wrapper.Name != W.sdt) + throw new InvalidOperationException($"unsupported structured wrapper: {wrapper.Name}"); + + var contentContainer = wrapper.Element(W.sdtContent) + ?? throw new InvalidOperationException("block w:sdt has no w:sdtContent"); + + foreach (var child in contentContainer.Elements().ToList()) + MarkTrackedStructuredContentChild(child); + + var author = _revisionAuthor ?? "docxodus"; + var date = DateTime.UtcNow.ToString("yyyy-MM-ddTHH:mm:ssZ"); + var boundaries = Internal.StructuredRevisionOps.AddCrossBoundaryMarkers( + contentContainer, + W.customXmlDelRangeStart, + W.customXmlDelRangeEnd, + name => CreateRevisionEnvelope(name, author, date)); + wrapper.AddBeforeSelf(boundaries.Before); + wrapper.AddAfterSelf(boundaries.After); + } + + /// + /// Recursively marks one block payload node. Nested SDT wrappers receive their own + /// reversible envelope; other transparent containers are preserved while their + /// block-bearing descendants are marked. + /// + private void MarkTrackedStructuredContentChild(XElement child) + { + if (child.Name == W.p) + { + MarkParagraphAsTrackedDeleted(child); + return; + } + if (child.Name == W.tbl) + { + MarkTableAsTrackedDeleted(child); + return; + } + if (child.Name == W.sdt) + { + MarkStructuredBlockAsTrackedDeleted(child); + return; + } + + foreach (var nested in child.Elements().ToList()) + MarkTrackedStructuredContentChild(nested); + } + private void PromoteHyperlinkRelationships(XElement paragraph) { var main = _doc!.MainDocumentPart!; diff --git a/Docxodus/Internal/StructuredRevisionOps.cs b/Docxodus/Internal/StructuredRevisionOps.cs new file mode 100644 index 00000000..937ebf01 --- /dev/null +++ b/Docxodus/Internal/StructuredRevisionOps.cs @@ -0,0 +1,54 @@ +#nullable enable + +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Xml.Linq; + +namespace Docxodus.Internal; + +/// +/// Builds the paired cross-boundary range topology Word uses to track the existence of +/// structured OOXML wrappers such as w:sdt. +/// +internal static class StructuredRevisionOps +{ + /// + /// Adds the two inner markers for a wrapper whose opening and closing tags are tracked, + /// and returns the two outer markers for the caller to place around the wrapper. + /// + /// + /// Range A starts before the wrapper and ends at the start of its content. Range B starts + /// at the end of the content and ends after the wrapper. The range ids must be distinct; + /// recognizes the wrapper by intersecting the elements + /// crossed by both ranges. + /// + internal static (XElement Before, XElement After) AddCrossBoundaryMarkers( + XElement contentContainer, + XName startName, + XName endName, + Func createRangeStart) + { + ArgumentNullException.ThrowIfNull(contentContainer); + ArgumentNullException.ThrowIfNull(startName); + ArgumentNullException.ThrowIfNull(endName); + ArgumentNullException.ThrowIfNull(createRangeStart); + + var before = createRangeStart(startName); + var beforeId = RequiredRangeId(before); + var openingEnd = new XElement(endName, new XAttribute(W.id, beforeId)); + + contentContainer.AddFirst(openingEnd); + + var closingStart = createRangeStart(startName); + var afterId = RequiredRangeId(closingStart); + contentContainer.Add(closingStart); + var after = new XElement(endName, new XAttribute(W.id, afterId)); + return (before, after); + } + + private static string RequiredRangeId(XElement start) => + (string?)start.Attribute(W.id) + ?? throw new InvalidOperationException("structured revision range start has no w:id"); +} diff --git a/Docxodus/Ir/Diff/IrMarkupRenderer.cs b/Docxodus/Ir/Diff/IrMarkupRenderer.cs index 9837f0cd..1456b52f 100644 --- a/Docxodus/Ir/Diff/IrMarkupRenderer.cs +++ b/Docxodus/Ir/Diff/IrMarkupRenderer.cs @@ -4141,18 +4141,11 @@ private static (XElement Before, XElement After) MarkWholeSdtEnvelope(XElement s var startName = IsDeleteGrade(kind) ? W.customXmlDelRangeStart : W.customXmlInsRangeStart; var endName = IsDeleteGrade(kind) ? W.customXmlDelRangeEnd : W.customXmlInsRangeEnd; - // Range A begins immediately before the control and ends as the first sdtContent child, so it contains - // the opening tag. Range B begins as the last sdtContent child and ends immediately after the control, - // so it contains the closing tag. AcceptDeletedAndMovedFromContentControls intersects those two sets. - var before = new XElement(startName, state.RevisionAttributes()); - var beforeId = (string?)before.Attribute(W.id) ?? ""; - content.AddFirst(new XElement(endName, new XAttribute(W.id, beforeId))); - - var afterStart = new XElement(startName, state.RevisionAttributes()); - var afterId = (string?)afterStart.Attribute(W.id) ?? ""; - content.Add(afterStart); - var after = new XElement(endName, new XAttribute(W.id, afterId)); - return (before, after); + return Internal.StructuredRevisionOps.AddCrossBoundaryMarkers( + content, + startName, + endName, + name => new XElement(name, state.RevisionAttributes())); } /// diff --git a/docs/architecture/docx_mutation_api.md b/docs/architecture/docx_mutation_api.md index 4855acdb..96aab0ed 100644 --- a/docs/architecture/docx_mutation_api.md +++ b/docs/architecture/docx_mutation_api.md @@ -123,7 +123,7 @@ Two conventions worth pinning down because they affect agent reasoning: - **`SplitParagraph` keeps the original Unid on the first half.** Reason: external systems (LLM context windows, search indices) bias toward the pre-split anchor position; keeping the prefix-half stable minimizes invalidation downstream. - **`MergeParagraphs` lets the first anchor absorb the second.** Symmetric reason: the first anchor is to the left in reading order and is more likely to be the one a caller has cached. -**Tracked-change mode shifts the semantics for `ReplaceText` and `DeleteBlock`.** When `Settings.TrackedChanges = RenderInline`, deletions don't remove elements — they wrap old runs in `w:del` and new content in `w:ins`. So the affected anchor stays live and appears in `Modified` instead of `Removed`. The agent's view of the world doesn't have to change; the `EditResult` shape is unchanged. The mode is switchable mid-session — see "Switching tracked-changes mode mid-session" below. +**Tracked-change mode shifts the semantics for `ReplaceText` and block deletion (`DeleteBlock`, `DeleteRange`, and `DeleteSection`).** When `Settings.TrackedChanges = RenderInline`, supported deletions don't remove elements — they wrap old runs in `w:del` and new content in `w:ins`. So the affected anchor stays live and appears in `Modified` instead of `Removed`. The agent's view of the world doesn't have to change; the `EditResult` shape is unchanged. The mode is switchable mid-session — see "Switching tracked-changes mode mid-session" below. **`ReplaceText` quietly strips a leading auto-number prefix from the payload.** When the target paragraph carries `w:numPr` (numbered heading or list item), the projector emits the resolved number inline (`## Fourth The total number…`) so a human can read what Word renders. An agent that echoes the visible heading back as its `ReplaceText` payload would otherwise see `Fourth Fourth: …` in the saved DOCX — the auto-number is still applied by Word, *and* the new run text now also starts with the prefix. The session resolves the number via the shared `Internal.ListNumberResolver` and strips a matching prefix (plus one optional separator: space, tab, or NBSP) from the payload before parsing. Idempotent — if the agent skipped the prefix, nothing is stripped. Documented in `DS091`/`DS091b`. @@ -322,11 +322,30 @@ wraps each removed paragraph's runs in `w:del` and marks the paragraph mark itself as deleted via `w:pPr/w:rPr/w:del`. Tables get `w:trPr/w:del` on every row (Word's row-deletion convention — there is no table-level "delete" markup), plus the same run/paragraph-mark wrapping inside every cell. Nested tables -recurse. Anchors stay live in the document tree, so the top-level block anchors -land in `EditResult.Modified` instead of `Removed` and callers can re-address -them before accepting the changes. Block kinds outside `w:p` / `w:tbl` (e.g. -`w:sdt` content controls in the middle of a range) still fall back to structural -removal in tracked mode — file a follow-up if a consumer needs them tracked. +recurse. + +Block-level `w:sdt` content controls are reversible too. Two paired +`w:customXmlDelRangeStart` / `w:customXmlDelRangeEnd` ranges cross the control's +opening and closing tags, matching Word's native content-control deletion shape. +Payload paragraphs and tables receive their normal deletion markup recursively; +nested block controls receive their own paired ranges. Accepting the revisions +therefore removes the control and its payload, while rejecting restores the +original wrapper, metadata, and content. Locked (`w:lock`) and data-bound +(`w:dataBinding`) controls use the same shape—the lock and binding metadata are +preserved until the revision is resolved. + +Anchor accounting describes what actually happened. Ordinary paragraph/table +top-level anchors remain the compact `Modified` contract. A structured wrapper +has no anchor of its own, so every anchored descendant retained under that +wrapper appears in `Modified`, without duplicates. A remaining structural +fall-through that must be hard-removed appears in `Removed`; it is never silently +omitted from both lists. + +`w:customXml` wrappers are deliberately unsupported in tracked bulk deletion. +If any selected block contains one, the operation fails before taking an undo +snapshot or changing the document with `IncompatibleElementType` and a message +identifying `w:customXml`. This is the explicit unsupported branch of the +custom-XML deletion contract; accepted-mode bulk deletion remains unchanged. ### `DeleteSection` — heading-bounded bulk removal @@ -339,9 +358,9 @@ If the target heading has no sibling-heading boundary after it, the section extends to the end of the parent. Built on `DeleteRange` semantics via the shared `DeleteSiblingRangeCore` helper: -same undo, same EditResult shape, same tracked-change behavior (paragraphs and -tables get `w:del` markup, anchors stay live, block kinds outside `w:p`/`w:tbl` -fall back to structural removal). +same undo, same `EditResult` accounting, the same native `w:sdt` envelope and +recursive payload markup, the same pre-mutation `w:customXml` refusal, and the +same reported structural fall-through. ## Finding anchors via tagged annotations @@ -794,7 +813,7 @@ rendering notes, appeared as a stray empty footnote with no citation. "reference note N again" op. - **Tracked-changes mode.** `Settings.TrackedChanges = RenderInline` does not wrap the citation in `w:ins` — consistent with every other insert op (`InsertParagraph`, `InsertTable`, - `InsertHorizontalRule`, `SetHeaderText`); only `ReplaceText`/`DeleteBlock`/`DeleteRange` track. + `InsertHorizontalRule`, `SetHeaderText`); only `ReplaceText`/`DeleteBlock`/`DeleteRange`/`DeleteSection` track. - **Narrowed projection scopes.** A session opened with `ProjectionSettings.Scopes` excluding `Footnotes`/`Endnotes` still writes the note correctly, but `Created` comes back without the note anchors — they resolve against a projection that omits the part. Family behavior, identical