From 22e53e19417d343906ef6fcd5101ee0c014c0df1 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Fri, 14 Aug 2026 04:20:12 -0500 Subject: [PATCH 1/3] Implement structural revision registry --- Docxodus.Tests/DocxSessionRevisionTests.cs | 56 +- .../DocxSessionStructuralRevisionTests.cs | 561 +++++++++++ .../DocxSessionTableAddressingTests.cs | 13 +- ...DocxSessionTrackedStructuredDeleteTests.cs | 46 + Docxodus.Tests/McpServerDispatcherTests.cs | 8 +- Docxodus/DocxSession.cs | 479 ++++++++-- Docxodus/Internal/DocxSessionJson.cs | 55 +- Docxodus/Internal/DocxSessionOps.cs | 6 + Docxodus/Internal/RevisionOps.cs | 898 +++++++++++++++++- Docxodus/Internal/RevisionRegistry.cs | 102 ++ Docxodus/RevisionProcessor.cs | 15 +- docs/architecture/docx_agent_server.md | 47 +- docs/architecture/docx_mutation_api.md | 31 +- npm/src/index.ts | 3 + npm/src/session.ts | 10 + npm/src/types.ts | 38 +- npm/tests/docx-session-revisions.spec.ts | 10 +- python/src/docx_scalpel/__init__.py | 2 + python/src/docx_scalpel/enums.py | 4 + python/src/docx_scalpel/session.py | 8 + python/src/docx_scalpel/types.py | 38 +- python/tests/test_revisions.py | 27 +- tools/mcp-server/Dispatcher.cs | 48 +- tools/mcp-server/ToolCatalog.cs | 11 +- tools/python-host/Dispatcher.cs | 2 + wasm/DocxodusWasm/DocxSessionBridge.cs | 8 + 26 files changed, 2281 insertions(+), 245 deletions(-) create mode 100644 Docxodus.Tests/DocxSessionStructuralRevisionTests.cs create mode 100644 Docxodus/Internal/RevisionRegistry.cs diff --git a/Docxodus.Tests/DocxSessionRevisionTests.cs b/Docxodus.Tests/DocxSessionRevisionTests.cs index 41833f61..cc59f1ba 100644 --- a/Docxodus.Tests/DocxSessionRevisionTests.cs +++ b/Docxodus.Tests/DocxSessionRevisionTests.cs @@ -245,19 +245,22 @@ public void DS370_ListRevisions_ReadsMarkupIdentityAuthorsAndText() Assert.Equal(3, revs.Count); - Assert.Equal("rev101", revs[0].Id); + Assert.StartsWith("rev2-", revs[0].Id); + Assert.Equal(new[] { "101" }, revs[0].ConstituentIds); Assert.Equal("insert", revs[0].Type); Assert.Equal("Alice", revs[0].Author); Assert.Equal("2026-01-02T03:04:05Z", revs[0].Date); Assert.Equal("New York", revs[0].Text); Assert.NotNull(revs[0].AnchorId); - Assert.Equal("rev102", revs[1].Id); + Assert.StartsWith("rev2-", revs[1].Id); + Assert.Equal(new[] { "102" }, revs[1].ConstituentIds); Assert.Equal("delete", revs[1].Type); Assert.Equal("Bob", revs[1].Author); Assert.Equal("Boston", revs[1].Text); - Assert.Equal("rev103", revs[2].Id); + Assert.StartsWith("rev2-", revs[2].Id); + Assert.Equal(new[] { "103" }, revs[2].ConstituentIds); Assert.Equal("delete", revs[2].Type); Assert.Equal("This sentence is gone.", revs[2].Text); } @@ -269,7 +272,8 @@ public void DS371_ListRevisions_GroupsWhollyInsertedParagraphAsOneRevision() var revs = s.ListRevisions(); var rev = Assert.Single(revs); - Assert.Equal("rev201", rev.Id); // min w:id over runs + mark + Assert.StartsWith("rev2-", rev.Id); + Assert.Contains("201", rev.ConstituentIds); Assert.Equal("insert", rev.Type); Assert.Equal("Alice", rev.Author); Assert.Equal("Whole new paragraph.¶", rev.Text); @@ -282,7 +286,8 @@ public void DS372_ListRevisions_MovePairIsOneRevision() var revs = s.ListRevisions(); var rev = Assert.Single(revs); - Assert.Equal("rev500", rev.Id); + Assert.StartsWith("rev2-", rev.Id); + Assert.Contains("500", rev.ConstituentIds); Assert.Equal("move", rev.Type); Assert.Equal("moved bit", rev.Text); } @@ -294,7 +299,8 @@ public void DS373_ListRevisions_DeletedRowIsOneRevision() var revs = s.ListRevisions(); var rev = Assert.Single(revs); - Assert.Equal("rev601", rev.Id); + Assert.StartsWith("rev2-", rev.Id); + Assert.Contains("601", rev.ConstituentIds); Assert.Equal("delete", rev.Type); Assert.Equal("Bob", rev.Author); Assert.Contains("A2", rev.Text); @@ -324,13 +330,14 @@ public void DS374_SelectiveAcceptAndReject_MixedResolutionInOneSession() public void DS375_ResolvingOneRevision_LeavesOtherIdsStable() { using var s = new DocxSession(BuildMixedRevisionsDoc()); - var before = s.ListRevisions().Select(r => r.Id).ToArray(); - Assert.Equal(new[] { "rev101", "rev102", "rev103" }, before); + var before = s.ListRevisions().ToArray(); + Assert.Equal(new[] { "101", "102", "103" }, + before.Select(r => Assert.Single(r.ConstituentIds)).ToArray()); Assert.True(s.AcceptRevision("rev102").Success); var after = s.ListRevisions(); - Assert.Equal(new[] { "rev101", "rev103" }, after.Select(r => r.Id).ToArray()); + Assert.Equal(new[] { before[0].Id, before[2].Id }, after.Select(r => r.Id).ToArray()); Assert.Equal("New York", after[0].Text); } @@ -462,7 +469,8 @@ public void DS384_FormatChange_ListedAndResolvedBothWays() using (var s = new DocxSession(BuildFormatChangeDoc())) { var rev = Assert.Single(s.ListRevisions()); - Assert.Equal("rev401", rev.Id); + Assert.StartsWith("rev2-", rev.Id); + Assert.Equal(new[] { "401" }, rev.ConstituentIds); Assert.Equal("format", rev.Type); Assert.Equal("Carol", rev.Author); Assert.Equal("Bold now.", rev.Text); @@ -499,8 +507,8 @@ public void DS410_AddCommentByRevisionId_BracketsExactContentExtent( var result = s.AddCommentToRevision(revisionId, "Reviewer", "Discuss this revision."); Assert.True(result.Success, result.Error?.Message); Assert.Single(s.ListComments()); - Assert.Equal(new[] { "rev101", "rev102", "rev103" }, - s.ListRevisions().Select(r => r.Id).ToArray()); + Assert.Equal(new[] { "101", "102", "103" }, + s.ListRevisions().Select(r => Assert.Single(r.ConstituentIds)).ToArray()); var bytes = s.Save(); using var ms = new MemoryStream(bytes); @@ -581,7 +589,7 @@ public void DS413_RejectCommentedInsertedParagraph_MovesCollapsedAnchorToSurvivo { using var s = new DocxSession(BuildInsertedParagraphDoc()); Assert.True(s.AddCommentToRevision("rev201", "Reviewer", "Do we need this paragraph?").Success); - Assert.Equal("rev201", Assert.Single(s.ListRevisions()).Id); + Assert.Contains("201", Assert.Single(s.ListRevisions()).ConstituentIds); Assert.True(s.RejectRevision("rev201").Success); Assert.Single(s.ListComments()); @@ -759,7 +767,8 @@ public void DS386_AcceptRevision_IsUndoable() Assert.True(s.Undo()); var restored = s.ListRevisions(); - Assert.Equal(new[] { "rev101", "rev102", "rev103" }, restored.Select(r => r.Id).ToArray()); + Assert.Equal(new[] { "101", "102", "103" }, + restored.Select(r => Assert.Single(r.ConstituentIds)).ToArray()); Assert.True(s.Redo()); Assert.Equal(2, s.ListRevisions().Count); @@ -830,7 +839,8 @@ public void DS390_ApplyFormat_Tracked_EmitsPerRunSnapshotsAndSessionStamp() // Adjacent per-run markers surface as one user-visible format revision. var listed = Assert.Single(s.ListRevisions()); - Assert.Equal("rev1001", listed.Id); + Assert.StartsWith("rev2-", listed.Id); + Assert.Equal(new[] { "1001", "1002" }, listed.ConstituentIds); Assert.Equal("format", listed.Type); Assert.Equal("Format Reviewer", listed.Author); Assert.Equal("Alpha Beta", listed.Text); @@ -969,7 +979,8 @@ public void DS393_ApplyFormat_Tracked_PreservesExistingRevisionBaselineAndMetada // OOXML allows one rPrChange only. The later edit folds into Carol's pending // change so reject still reaches the original empty formatting baseline. var listed = Assert.Single(s.ListRevisions()); - Assert.Equal("rev401", listed.Id); + Assert.StartsWith("rev2-", listed.Id); + Assert.Equal(new[] { "401" }, listed.ConstituentIds); Assert.Equal("Carol", listed.Author); using (var trackedMs = new MemoryStream(s.Save())) @@ -1035,7 +1046,8 @@ public void DS395_ApplyFormat_Tracked_FailureRestoresWholeOperationSnapshot() Assert.Equal(EditErrorCode.InternalError, result.Error!.Code); Assert.True(XNode.DeepEquals(before, MainDocumentRoot(s.Save()))); var revision = Assert.Single(s.ListRevisions()); - Assert.Equal("rev401", revision.Id); + Assert.StartsWith("rev2-", revision.Id); + Assert.Equal(new[] { "401" }, revision.ConstituentIds); Assert.Equal("Carol", revision.Author); Assert.False(s.Undo()); // failed operations do not remain on the history stack } @@ -1058,14 +1070,16 @@ public void DS396_ApplyFormat_Tracked_SeparateAdjacentCallsResolveIndependently( var revisions = s.ListRevisions(); Assert.Equal(2, revisions.Count); - Assert.Equal(new[] { "rev1001", "rev1002" }, revisions.Select(r => r.Id).ToArray()); + Assert.All(revisions, r => Assert.StartsWith("rev2-", r.Id)); + Assert.Equal(new[] { "1001", "1002" }, + revisions.Select(r => Assert.Single(r.ConstituentIds)).ToArray()); Assert.Equal(new[] { "Alpha ", "Beta" }, revisions.Select(r => r.Text).ToArray()); Assert.All(revisions, r => Assert.Equal("One Reviewer", r.Author)); Assert.NotEqual(revisions[0].Date, revisions[1].Date); Assert.True(s.RejectRevision(revisions[0].Id).Success); var remaining = Assert.Single(s.ListRevisions()); - Assert.Equal("rev1002", remaining.Id); + Assert.Equal(new[] { "1002" }, remaining.ConstituentIds); Assert.Equal("Beta", remaining.Text); using (var rejectedFirstMs = new MemoryStream(s.Save())) @@ -1106,7 +1120,7 @@ public void DS397_ApplyFormat_Tracked_RevertingWholePendingChangeDropsMarker() } Assert.True(s.Undo()); - Assert.Equal("rev401", Assert.Single(s.ListRevisions()).Id); + Assert.Equal(new[] { "401" }, Assert.Single(s.ListRevisions()).ConstituentIds); Assert.True(s.Redo()); Assert.Empty(s.ListRevisions()); } @@ -1122,7 +1136,7 @@ public void DS398_ApplyFormat_Tracked_RevertingPartialPendingChangeKeepsOnlyRema new FormatOp { Bold = false }); Assert.True(result.Success, result.Error?.Message); var remaining = Assert.Single(s.ListRevisions()); - Assert.Equal("rev401", remaining.Id); + Assert.Equal(new[] { "401" }, remaining.ConstituentIds); Assert.Equal(" now.", remaining.Text); using (var ms = new MemoryStream(s.Save())) diff --git a/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs b/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs new file mode 100644 index 00000000..4133af3b --- /dev/null +++ b/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs @@ -0,0 +1,561 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#nullable enable + +using System; +using System.IO; +using System.Linq; +using System.Xml.Linq; +using DocumentFormat.OpenXml.Packaging; +using DocumentFormat.OpenXml.Validation; +using Docxodus; +using Xunit; + +namespace Docxodus.Tests; + +public class DocxSessionStructuralRevisionTests +{ + private static string Fixture(string relative) => + Path.GetFullPath(Path.Combine(AppContext.BaseDirectory, "../../../../TestFiles", relative)); + + [Theory] + [InlineData("RP/RP034-Deleted-Cells.docx", RevisionFamily.CellDelete)] + [InlineData("RP/RP035-Inserted-Cells.docx", RevisionFamily.CellInsert)] + [InlineData("RP/RP036-Vert-Merged-Cells.docx", RevisionFamily.CellMerge)] + [InlineData("RP/RP016-Deleted-CC.docx", RevisionFamily.ContentControlDelete)] + [InlineData("RP/RP017-Inserted-CC.docx", RevisionFamily.ContentControlInsert)] + [InlineData("RP/RP021-Inserted-Numbering-Properties.docx", RevisionFamily.NumberingPropertiesInsert)] + [InlineData("RP/RP026-NumberingChange.docx", RevisionFamily.NumberingChange)] + public void DS45501_RealFixture_ListsNewFamilyAsSupported(string relative, RevisionFamily family) + { + using var session = new DocxSession(File.ReadAllBytes(Fixture(relative))); + var matching = session.ListRevisions().Where(r => r.Family == family).ToList(); + + Assert.NotEmpty(matching); + Assert.All(matching, revision => + { + Assert.StartsWith("rev2-", revision.Id); + Assert.Equal("/word/document.xml", revision.PartUri); + Assert.Equal("body", revision.Scope); + Assert.NotEmpty(revision.ConstituentIds); + Assert.NotEmpty(revision.AffectedAnchors); + Assert.Equal(RevisionResolutionStatus.Supported, revision.ResolutionStatus); + Assert.Null(revision.Diagnostic); + }); + } + + [Theory] + [InlineData("RP/RP034-Deleted-Cells.docx", true)] + [InlineData("RP/RP034-Deleted-Cells.docx", false)] + [InlineData("RP/RP035-Inserted-Cells.docx", true)] + [InlineData("RP/RP035-Inserted-Cells.docx", false)] + [InlineData("RP/RP036-Vert-Merged-Cells.docx", true)] + [InlineData("RP/RP036-Vert-Merged-Cells.docx", false)] + [InlineData("RP/RP016-Deleted-CC.docx", true)] + [InlineData("RP/RP016-Deleted-CC.docx", false)] + [InlineData("RP/RP017-Inserted-CC.docx", true)] + [InlineData("RP/RP017-Inserted-CC.docx", false)] + [InlineData("RP/RP021-Inserted-Numbering-Properties.docx", true)] + [InlineData("RP/RP021-Inserted-Numbering-Properties.docx", false)] + [InlineData("RP/RP026-NumberingChange.docx", true)] + [InlineData("RP/RP026-NumberingChange.docx", false)] + public void DS45502_IndividualAndBulkResolution_MatchProcessorOracle(string relative, bool accept) + { + var input = File.ReadAllBytes(Fixture(relative)); + var oracle = accept + ? RevisionProcessor.AcceptRevisions(new WmlDocument("oracle.docx", input)).DocumentByteArray + : RevisionProcessor.RejectRevisions(new WmlDocument("oracle.docx", input)).DocumentByteArray; + + byte[] individual; + using (var session = new DocxSession(input)) + { + for (int guard = 0; guard < 1000 && session.ListRevisions().Count > 0; guard++) + { + var revision = session.ListRevisions()[0]; + var result = accept + ? session.AcceptRevision(revision.Id) + : session.RejectRevision(revision.Id); + Assert.True(result.Success, result.Error?.Message); + } + Assert.Empty(session.ListRevisions()); + individual = session.Save(); + } + + byte[] bulk; + using (var session = new DocxSession(input)) + { + var result = accept ? session.AcceptAllRevisions() : session.RejectAllRevisions(); + Assert.True(result.Success, result.Error?.Message); + Assert.Empty(session.ListRevisions()); + bulk = session.Save(); + + Assert.True(session.Undo()); + Assert.NotEmpty(session.ListRevisions()); + Assert.True(session.Redo()); + Assert.Empty(session.ListRevisions()); + } + + var oracleRoot = MainRoot(oracle); + var individualRoot = MainRoot(individual); + var bulkRoot = MainRoot(bulk); + Assert.True(XNode.DeepEquals(oracleRoot, individualRoot), + FirstDifference(oracleRoot, individualRoot)); + Assert.True(XNode.DeepEquals(oracleRoot, bulkRoot), + FirstDifference(oracleRoot, bulkRoot)); + + // A few of the Office-produced fixtures contain extension attributes that the + // SDK validator does not recognize. Resolution must not introduce any new + // validation failures beyond those already present in the processor oracle. + Assert.Equal(ValidationErrors(oracle), ValidationErrors(bulk)); + } + + [Theory] + [InlineData("insert_row", true, RevisionFamily.RowInsert)] + [InlineData("insert_row", false, RevisionFamily.RowInsert)] + [InlineData("delete_row", true, RevisionFamily.RowDelete)] + [InlineData("delete_row", false, RevisionFamily.RowDelete)] + [InlineData("insert_column", true, RevisionFamily.CellInsert)] + [InlineData("insert_column", false, RevisionFamily.CellInsert)] + public void DS45503_TrackedTableMutation_ResolvesToDirectOrOriginal( + string operation, bool accept, RevisionFamily family) + { + var baseline = BuildTableDocument(); + byte[] expected; + if (accept) + { + using var direct = new DocxSession(baseline); + var directResult = ApplyTableMutation(direct, operation); + Assert.True(directResult.Success, directResult.Error?.Message); + expected = direct.Save(); + } + else + { + expected = baseline; + } + + byte[] actual; + using (var tracked = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + RevisionAuthor = "Structural Reviewer", + })) + { + var edit = ApplyTableMutation(tracked, operation); + Assert.True(edit.Success, edit.Error?.Message); + + var revision = Assert.Single(tracked.ListRevisions()); + Assert.Equal(family, revision.Family); + Assert.Equal("Structural Reviewer", revision.Author); + Assert.Equal(RevisionResolutionStatus.Supported, revision.ResolutionStatus); + Assert.True(revision.AffectedAnchors.Count >= 2); + + var resolution = accept + ? tracked.AcceptRevision(revision.Id) + : tracked.RejectRevision(revision.Id); + Assert.True(resolution.Success, resolution.Error?.Message); + Assert.Empty(tracked.ListRevisions()); + actual = tracked.Save(); + } + + Assert.True(XNode.DeepEquals(MainRoot(expected), MainRoot(actual)), + FirstDifference(MainRoot(expected), MainRoot(actual))); + Assert.Equal(ValidationErrors(expected), ValidationErrors(actual)); + } + + [Fact] + public void DS45504_StableId_SurvivesRelistingSaveAndUnrelatedResolution() + { + var input = File.ReadAllBytes(Fixture("RP/RP034-Deleted-Cells.docx")); + using var session = new DocxSession(input); + var before = session.ListRevisions(); + var cell = Assert.Single(before, r => r.Family == RevisionFamily.CellDelete); + Assert.Equal(cell.Id, Assert.Single(session.ListRevisions(), r => + r.Family == RevisionFamily.CellDelete).Id); + + var unrelated = before.FirstOrDefault(r => r.Id != cell.Id); + if (unrelated is not null) + Assert.True(session.AcceptRevision(unrelated.Id).Success); + Assert.Equal(cell.Id, Assert.Single(session.ListRevisions(), r => + r.Family == RevisionFamily.CellDelete).Id); + + using var reopened = new DocxSession(session.Save()); + Assert.Equal(cell.Id, Assert.Single(reopened.ListRevisions(), r => + r.Family == RevisionFamily.CellDelete).Id); + } + + [Theory] + [InlineData("missing_id", RevisionResolutionStatus.Malformed, EditErrorCode.RevisionMalformed)] + [InlineData("duplicate_id", RevisionResolutionStatus.Ambiguous, EditErrorCode.RevisionAmbiguous)] + [InlineData("unsupported_move", RevisionResolutionStatus.Unsupported, EditErrorCode.RevisionUnsupported)] + public void DS45505_InvalidTopology_IsListedAndFailsClosed( + string shape, RevisionResolutionStatus status, EditErrorCode errorCode) + { + var input = BuildInvalidRevisionDocument(shape); + using var session = new DocxSession(input); + var before = MainRoot(session.Save()); + var invalid = session.ListRevisions().Where(r => r.ResolutionStatus == status).ToList(); + Assert.NotEmpty(invalid); + if (shape == "duplicate_id") + Assert.Equal(invalid.Count, invalid.Select(r => r.Id).Distinct().Count()); + var revision = invalid[0]; + Assert.NotNull(revision.Diagnostic); + + var result = session.AcceptRevision(revision.Id); + Assert.False(result.Success); + Assert.Equal(errorCode, result.Error!.Code); + Assert.True(XNode.DeepEquals(before, MainRoot(session.Save()))); + Assert.False(session.Undo()); + } + + [Fact] + public void DS45506_BulkResolution_BlockedEntryIsAtomic() + { + var input = BuildInvalidRevisionDocument("unsupported_move"); + using var session = new DocxSession(input); + var before = MainRoot(session.Save()); + + var result = session.AcceptAllRevisions(); + + Assert.False(result.Success); + Assert.Equal(EditErrorCode.RevisionUnsupported, result.Error!.Code); + Assert.True(XNode.DeepEquals(before, MainRoot(session.Save()))); + Assert.False(session.Undo()); + } + + [Theory] + [InlineData("apply", true, RevisionFamily.NumberingPropertiesInsert)] + [InlineData("apply", false, RevisionFamily.NumberingPropertiesInsert)] + [InlineData("remove", true, RevisionFamily.PropertiesChange)] + [InlineData("remove", false, RevisionFamily.PropertiesChange)] + [InlineData("level", true, RevisionFamily.PropertiesChange)] + [InlineData("level", false, RevisionFamily.PropertiesChange)] + public void DS45507_TrackedListMutation_UsesNativeRevisionAndRoundTrips( + string operation, bool accept, RevisionFamily family) + { + var baseline = BuildListBaseline(operation); + byte[] expected; + if (accept) + { + using var direct = new DocxSession(baseline); + Assert.True(ApplyListMutation(direct, operation).Success); + expected = direct.Save(); + } + else + { + expected = baseline; + } + + byte[] actual; + using (var tracked = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + RevisionAuthor = "List Reviewer", + })) + { + var edit = ApplyListMutation(tracked, operation); + Assert.True(edit.Success, edit.Error?.Message); + var revision = Assert.Single(tracked.ListRevisions()); + Assert.Equal(family, revision.Family); + Assert.Equal("List Reviewer", revision.Author); + var resolution = accept + ? tracked.AcceptRevision(revision.Id) + : tracked.RejectRevision(revision.Id); + Assert.True(resolution.Success, resolution.Error?.Message); + Assert.Empty(tracked.ListRevisions()); + actual = tracked.Save(); + } + + Assert.True(XNode.DeepEquals(MainRoot(expected), MainRoot(actual)), + FirstDifference(MainRoot(expected), MainRoot(actual))); + } + + [Fact] + public void DS45508_UnsupportedTrackedStructuralOperations_DoNotMutateOrCreateHistory() + { + var baseline = BuildTableDocument(); + using var session = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + }); + var before = MainRoot(session.Save()); + var cell = FirstCellAnchor(session); + + var deleteColumn = session.DeleteTableColumn(cell); + Assert.False(deleteColumn.Success); + Assert.Equal(EditErrorCode.TrackedOperationUnsupported, deleteColumn.Error!.Code); + + var merge = session.MergeCells(cell, 1, 2); + Assert.False(merge.Success); + Assert.Equal(EditErrorCode.TrackedOperationUnsupported, merge.Error!.Code); + + var paragraphs = session.Project().AnchorIndex.Values + .Where(a => a.Anchor.Scope == "body" && a.Anchor.Kind == "p" + && (a.TextPreview.StartsWith("First", StringComparison.Ordinal) + || a.TextPreview.StartsWith("Second", StringComparison.Ordinal))) + .Select(a => a.Anchor.Id).Take(2).ToArray(); + var listRange = session.ApplyListFormatRange( + paragraphs[0], paragraphs[1], ListFormat.Decimal); + Assert.False(listRange.Success); + Assert.Equal(EditErrorCode.TrackedOperationUnsupported, listRange.Error!.Code); + + Assert.True(XNode.DeepEquals(before, MainRoot(session.Save()))); + Assert.False(session.Undo()); + } + + [Fact] + public void DS45509_UnresolvedCellStructure_BlocksFurtherTableMutationWithoutHistory() + { + using var session = new DocxSession( + File.ReadAllBytes(Fixture("RP/RP035-Inserted-Cells.docx"))); + var before = MainRoot(session.Save()); + + var result = session.InsertTableRow(FirstCellAnchor(session), Position.After); + + Assert.False(result.Success); + Assert.Equal(EditErrorCode.UnresolvedStructuralRevision, result.Error!.Code); + Assert.True(XNode.DeepEquals(before, MainRoot(session.Save()))); + Assert.False(session.Undo()); + } + + [Fact] + public void DS45510_IdenticalNativeIdsInDifferentParts_AreIndependent() + { + using var session = new DocxSession(BuildCrossPartDuplicateIdDocument()); + var revisions = session.ListRevisions(); + Assert.Equal(2, revisions.Count); + Assert.All(revisions, revision => + { + Assert.Equal(new[] { "777" }, revision.ConstituentIds); + Assert.Equal(RevisionResolutionStatus.Supported, revision.ResolutionStatus); + }); + Assert.Equal(2, revisions.Select(revision => revision.Id).Distinct().Count()); + Assert.Contains(revisions, revision => revision.PartUri == "/word/document.xml" + && revision.Scope == "body"); + var header = Assert.Single(revisions, revision => revision.PartUri.StartsWith( + "/word/header", StringComparison.Ordinal)); + Assert.Equal("hdr1", header.Scope); + + Assert.True(session.AcceptRevision(header.Id).Success); + + var remaining = Assert.Single(session.ListRevisions()); + Assert.Equal("/word/document.xml", remaining.PartUri); + Assert.Equal("777", Assert.Single(remaining.ConstituentIds)); + } + + private static byte[] BuildTableDocument() + { + using var session = new DocxSession(DocxSessionTests.BuildDS001_SimpleTwoParagraphs()); + var paragraph = session.Project().AnchorIndex.Values + .First(a => a.Anchor.Scope == "body" && a.Anchor.Kind == "p").Anchor.Id; + var result = session.InsertTable(paragraph, Position.After, 2, 2, + new TableInsertOptions + { + CellContents = new[] { "A1", "B1", "A2", "B2" }, + ColumnWidths = new[] { 2400, 3200 }, + }); + Assert.True(result.Success, result.Error?.Message); + return session.Save(); + } + + private static string FirstCellAnchor(DocxSession session) => + session.Project().AnchorIndex.Values + .First(a => a.Anchor.Scope == "body" && a.Anchor.Kind == "tc").Anchor.Id; + + private static EditResult ApplyTableMutation(DocxSession session, string operation) + { + var cell = FirstCellAnchor(session); + return operation switch + { + "insert_row" => session.InsertTableRow(cell, Position.After), + "delete_row" => session.DeleteTableRow(cell), + "insert_column" => session.InsertTableColumn(cell, Position.After), + _ => throw new ArgumentOutOfRangeException(nameof(operation)), + }; + } + + private static byte[] BuildListBaseline(string operation) + { + var input = DocxSessionTests.BuildDS001_SimpleTwoParagraphs(); + if (operation == "apply") return input; + using var session = new DocxSession(input); + var paragraph = FirstBodyTextAnchor(session); + var result = session.ApplyListFormat(paragraph, ListFormat.Decimal); + Assert.True(result.Success, result.Error?.Message); + return session.Save(); + } + + private static EditResult ApplyListMutation(DocxSession session, string operation) + { + var anchor = FirstBodyTextAnchor(session); + return operation switch + { + "apply" => session.ApplyListFormat(anchor, ListFormat.Decimal), + "remove" => session.RemoveListMembership(anchor), + "level" => session.SetListLevel(anchor, 1), + _ => throw new ArgumentOutOfRangeException(nameof(operation)), + }; + } + + private static string FirstBodyTextAnchor(DocxSession session) => + session.Project().AnchorIndex.Values + .First(a => a.Anchor.Scope == "body" && a.Anchor.Kind is "p" or "li" + && a.TextPreview.StartsWith("First", StringComparison.Ordinal)).Anchor.Id; + + private static byte[] BuildInvalidRevisionDocument(string shape) + { + if (shape == "missing_id") + return MutateMain(File.ReadAllBytes(Fixture("RP/RP035-Inserted-Cells.docx")), root => + root.Descendants(W.cellIns).First().Attribute(W.id)?.Remove()); + + return MutateMain(DocxSessionTests.BuildDS001_SimpleTwoParagraphs(), root => + { + var paragraphs = root.Descendants(W.p).Take(2).ToArray(); + if (shape == "duplicate_id") + { + foreach (var paragraph in paragraphs) + { + var run = paragraph.Elements(W.r).First(); + run.ReplaceWith(new XElement(W.ins, + new XAttribute(W.id, "777"), + new XAttribute(W.author, "Duplicate"), + new XAttribute(W.date, "2026-01-01T00:00:00Z"), + new XElement(run))); + } + return; + } + + if (shape == "unsupported_move") + { + paragraphs[0].AddFirst(new XElement(W.customXmlMoveFromRangeStart, + new XAttribute(W.id, "888"), + new XAttribute(W.author, "Mover"), + new XAttribute(W.date, "2026-01-01T00:00:00Z"))); + paragraphs[0].Add(new XElement(W.customXmlMoveFromRangeEnd, + new XAttribute(W.id, "888"))); + return; + } + + throw new ArgumentOutOfRangeException(nameof(shape)); + }); + } + + private static byte[] BuildCrossPartDuplicateIdDocument() + { + var input = DocxSessionTests.BuildDS001_SimpleTwoParagraphs(); + using var stream = new MemoryStream(); + stream.Write(input); + stream.Position = 0; + using (var document = WordprocessingDocument.Open(stream, true)) + { + var main = document.MainDocumentPart!; + var header = main.AddNewPart(); + header.PutXDocument(new XDocument(new XElement(W.hdr, + new XAttribute(XNamespace.Xmlns + "w", W.w), + new XElement(W.p, + new XElement(W.ins, + new XAttribute(W.id, "777"), + new XAttribute(W.author, "Header Reviewer"), + new XAttribute(W.date, "2026-01-01T00:00:00Z"), + new XElement(W.r, new XElement(W.t, "Header revision"))))))); + + var mainDocument = main.GetXDocument(); + var firstRun = mainDocument.Root!.Descendants(W.p).First().Elements(W.r).First(); + firstRun.ReplaceWith(new XElement(W.ins, + new XAttribute(W.id, "777"), + new XAttribute(W.author, "Body Reviewer"), + new XAttribute(W.date, "2026-01-01T00:00:00Z"), + new XElement(firstRun))); + var body = mainDocument.Root.Element(W.body)!; + var sectPr = body.Elements(W.sectPr).LastOrDefault(); + if (sectPr is null) + { + sectPr = new XElement(W.sectPr); + body.Add(sectPr); + } + XNamespace relationships = + "http://schemas.openxmlformats.org/officeDocument/2006/relationships"; + sectPr.AddFirst(new XElement(W.headerReference, + new XAttribute(relationships + "id", main.GetIdOfPart(header)), + new XAttribute(W.type, "default"))); + main.PutXDocument(); + } + return stream.ToArray(); + } + + private static byte[] MutateMain(byte[] input, Action mutate) + { + using var stream = new MemoryStream(); + stream.Write(input); + stream.Position = 0; + using (var document = WordprocessingDocument.Open(stream, true)) + { + var xDocument = document.MainDocumentPart!.GetXDocument(); + mutate(xDocument.Root!); + document.MainDocumentPart.PutXDocument(); + } + return stream.ToArray(); + } + + private static XElement MainRoot(byte[] bytes) + { + using var stream = new MemoryStream(bytes); + using var document = WordprocessingDocument.Open(stream, false); + var root = new XElement(document.MainDocumentPart!.GetXDocument().Root!); + + // The legacy all-revisions processor removes Word's transient GoBack bookmark + // while selective resolution intentionally preserves unrelated bookmarks. + // Exclude that application affordance from semantic parity comparisons. + var goBackIds = root.Descendants(W.bookmarkStart) + .Where(e => (string?)e.Attribute(W.name) == "_GoBack") + .Select(e => (string?)e.Attribute(W.id)) + .Where(id => id != null) + .ToHashSet(StringComparer.Ordinal); + foreach (var bookmark in root.Descendants(W.bookmarkStart) + .Where(e => (string?)e.Attribute(W.name) == "_GoBack") + .Concat(root.Descendants(W.bookmarkEnd) + .Where(e => goBackIds.Contains((string?)e.Attribute(W.id)))).ToList()) + bookmark.Remove(); + foreach (var attribute in root.DescendantsAndSelf().Attributes() + .Where(a => a.Name.Namespace == W.w + && a.Name.LocalName.StartsWith("rsid", StringComparison.Ordinal)).ToList()) + attribute.Remove(); + foreach (var whitespace in root.DescendantNodes().OfType() + .Where(t => string.IsNullOrWhiteSpace(t.Value)).ToList()) + whitespace.Remove(); + foreach (var husk in root.Descendants() + .Where(e => (e.Name == W.pPr || e.Name == W.rPr || e.Name == W.trPr) + && !e.HasElements && !e.HasAttributes).ToList()) + husk.Remove(); + return root; + } + + private static string[] ValidationErrors(byte[] bytes) + { + using var stream = new MemoryStream(bytes); + using var document = WordprocessingDocument.Open(stream, false); + return new OpenXmlValidator().Validate(document) + .Select(e => $"{e.Id}|{e.Description}|{e.Path?.XPath}") + .OrderBy(e => e, StringComparer.Ordinal) + .ToArray(); + } + + private static string FirstDifference(XElement expected, XElement actual) + { + var expectedNodes = expected.DescendantsAndSelf().ToList(); + var actualNodes = actual.DescendantsAndSelf().ToList(); + int count = Math.Min(expectedNodes.Count, actualNodes.Count); + for (int i = 0; i < count; i++) + { + var left = expectedNodes[i]; + var right = actualNodes[i]; + if (left.Name != right.Name || left.Value != right.Value + || !left.Attributes().OrderBy(a => a.Name.ToString()) + .Select(a => (a.Name, a.Value)) + .SequenceEqual(right.Attributes().OrderBy(a => a.Name.ToString()) + .Select(a => (a.Name, a.Value)))) + return $"first difference at element {i}: expected {left}, actual {right}"; + } + return $"element counts differ: expected {expectedNodes.Count}, actual {actualNodes.Count}"; + } +} diff --git a/Docxodus.Tests/DocxSessionTableAddressingTests.cs b/Docxodus.Tests/DocxSessionTableAddressingTests.cs index d6510dd6..04d56475 100644 --- a/Docxodus.Tests/DocxSessionTableAddressingTests.cs +++ b/Docxodus.Tests/DocxSessionTableAddressingTests.cs @@ -356,7 +356,7 @@ public void DT256_HeaderTable_UsesScopedCanonicalAnchors() } [Fact] - public void DT257_TableInsideRevisionWrapper_HasCanonicalAnchorsWithoutRevisionEmission() + public void DT257_TableInsideRevisionWrapper_HasCanonicalAnchorsAndNativeRowRevision() { var revisedRow = " element.Name == W + "ins"); + var rows = tableElement.Elements(W + "tr").ToList(); + Assert.Equal(2, rows.Count); + Assert.Equal("7", (string?)Assert.Single(rows[0].Element(W + "trPr")!.Elements(W + "ins")) + .Attribute(W + "id")); + var inserted = Assert.Single(rows[1].Element(W + "trPr")!.Elements(W + "ins")); + Assert.NotEqual("7", (string?)inserted.Attribute(W + "id")); + Assert.Single(rows[1].Descendants(W + "rPr").Elements(W + "ins")); } } diff --git a/Docxodus.Tests/DocxSessionTrackedStructuredDeleteTests.cs b/Docxodus.Tests/DocxSessionTrackedStructuredDeleteTests.cs index 2a3496ec..995a5549 100644 --- a/Docxodus.Tests/DocxSessionTrackedStructuredDeleteTests.cs +++ b/Docxodus.Tests/DocxSessionTrackedStructuredDeleteTests.cs @@ -209,6 +209,52 @@ public void DS477_DeleteSection_TracksControlAndReportsSectionPropertyFallThroug AssertSchemaValid(tracked); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void DS478_SessionRegistry_ResolvesAuthoredControlDeletionAtomically(bool accept) + { + byte[] tracked; + using (var authoring = OpenTrackedSession(BuildDocument( + ParagraphWithText("before"), + ParagraphWithText("delete start"), + BlockControl("controlled", ParagraphWithText("controlled paragraph")), + ParagraphWithText("after")))) + { + var projection = authoring.Project(); + var from = FindByText(authoring, projection, "delete start"); + var to = FindByText(authoring, projection, "after"); + Assert.True(authoring.DeleteRange(from, to).Success); + tracked = authoring.Save(); + } + + using var review = new DocxSession(tracked); + var structured = Assert.Single(review.ListRevisions(), revision => + revision.Family == RevisionFamily.ContentControlDelete); + Assert.Equal(RevisionResolutionStatus.Supported, structured.ResolutionStatus); + Assert.Contains(structured.AffectedAnchors, anchor => anchor.Kind == "p"); + + var result = accept + ? review.AcceptRevision(structured.Id) + : review.RejectRevision(structured.Id); + + Assert.True(result.Success, result.Error?.Message); + var body = Body(review.Save()); + if (accept) + { + Assert.Empty(body.Elements(W.sdt)); + Assert.DoesNotContain("controlled paragraph", body.Value); + } + else + { + Assert.Single(body.Elements(W.sdt)); + Assert.Contains("controlled paragraph", body.Value); + } + Assert.DoesNotContain(body.Descendants(), element => + element.Name == W.customXmlDelRangeStart || element.Name == W.customXmlDelRangeEnd); + AssertSchemaValid(review.Save()); + } + private static DocxSession OpenTrackedSession(byte[] bytes) => new(bytes, new DocxSessionSettings { diff --git a/Docxodus.Tests/McpServerDispatcherTests.cs b/Docxodus.Tests/McpServerDispatcherTests.cs index 699c7671..9b360dfb 100644 --- a/Docxodus.Tests/McpServerDispatcherTests.cs +++ b/Docxodus.Tests/McpServerDispatcherTests.cs @@ -1786,7 +1786,13 @@ public void MCP136_TrackChanges_SelectiveAcceptAndRejectByRevisionId() var insertRev = Assert.Single(revisions, r => r.GetProperty("type").GetString() == "insert"); Assert.Equal("selective edit", insertRev.GetProperty("text").GetString()); Assert.Equal("Reviewer A", insertRev.GetProperty("author").GetString()); - Assert.StartsWith("rev", insertRev.GetProperty("id").GetString()); + Assert.StartsWith("rev2-", insertRev.GetProperty("id").GetString()); + Assert.Equal("content_insert", insertRev.GetProperty("family").GetString()); + Assert.Equal("supported", insertRev.GetProperty("resolutionStatus").GetString()); + Assert.Equal("/word/document.xml", insertRev.GetProperty("partUri").GetString()); + Assert.Equal("body", insertRev.GetProperty("scope").GetString()); + Assert.NotEmpty(insertRev.GetProperty("constituentIds").EnumerateArray()); + Assert.NotEmpty(insertRev.GetProperty("affectedAnchors").EnumerateArray()); // Accept the insertion; the deletion keeps its id and resolves independently. var accepted = Parse(Dispatcher.Call(_store, "docxodus_track_changes", J( diff --git a/Docxodus/DocxSession.cs b/Docxodus/DocxSession.cs index a068b9fa..8ddc2ee6 100644 --- a/Docxodus/DocxSession.cs +++ b/Docxodus/DocxSession.cs @@ -1291,22 +1291,58 @@ public sealed record CommentListEntry( public bool? Resolved { get; init; } } +public enum RevisionFamily +{ + ContentInsert, + ContentDelete, + Move, + ParagraphMark, + RowInsert, + RowDelete, + CellInsert, + CellDelete, + CellMerge, + ContentControlInsert, + ContentControlDelete, + NumberingPropertiesInsert, + NumberingChange, + PropertiesChange, + Unsupported, +} + +public enum RevisionResolutionStatus +{ + Supported, + Unsupported, + Malformed, + Ambiguous, +} + +public sealed record RevisionDiagnostic(string Code, string Message); + /// -/// One tracked revision, read directly off the live document's markup in document -/// order — see . is stable -/// while the underlying markup exists (derived from the markup's own w:id -/// attributes, so resolving OTHER revisions never renames it) and is what -/// / -/// address. is "insert", "delete", "move" -/// (a linked move pair — both sides resolve together), or "format". -/// / are the true w:author/w:date -/// from the markup (date null when absent). is the revision's -/// visible text (the deleted text for deletions, for a revised paragraph -/// mark, the affected text for format changes). is the -/// containing block's anchor (null when the block isn't projection-addressable). +/// One part-qualified, markup-native tracked revision. +/// contains every currently addressable structure the atomic revision can change; +/// remains the convenient first/primary anchor. +/// Unsupported, malformed, and ambiguous markup is deliberately listed and fails +/// closed when resolution is requested. /// -public sealed record RevisionListEntry( - string Id, string Type, string Author, string? Date, string Text, string? AnchorId); +public sealed record RevisionListEntry +{ + required public string Id { get; init; } + required public string Type { get; init; } + required public RevisionFamily Family { get; init; } + required public IReadOnlyList ConstituentIds { get; init; } + required public string Author { get; init; } + public string? Date { get; init; } + required public string Text { get; init; } + required public string PartUri { get; init; } + required public string Scope { get; init; } + public string? AnchorId { get; init; } + required public IReadOnlyList AffectedAnchors { get; init; } + required public RevisionResolutionStatus ResolutionStatus { get; init; } + public RevisionDiagnostic? Diagnostic { get; init; } +} /// Summary returned by . public sealed record CompactResult @@ -1619,7 +1655,6 @@ public enum EditErrorCode ManagedBookmark, EmptyHyperlinkSpan, UnsupportedInlineBoundary, - TrackedOperationUnsupported, ImageNotFound, InvalidImageData, @@ -1647,6 +1682,21 @@ public enum EditErrorCode /// A mutation batch step names an unsupported operation or a read-only action. InvalidBatchStep, + /// The revision family is visible but has no safe selective resolver. + RevisionUnsupported, + + /// The native marker topology is incomplete or internally inconsistent. + RevisionMalformed, + + /// The native identity/topology maps to more than one possible operation. + RevisionAmbiguous, + + /// The requested mutation has no reversible native tracked-change encoding. + TrackedOperationUnsupported, + + /// A structural edit was refused because its table still has unresolved structure revisions. + UnresolvedStructuralRevision, + InternalError, } @@ -2606,43 +2656,37 @@ public IReadOnlyList ListComments() // ─── Tracked revisions: markup-native listing + selective resolution (issue #318) ─── /// - /// Enumerate the document's tracked revisions directly off the live markup, in - /// document order across every story RevisionProcessor walks (body, headers, - /// footers, footnotes, endnotes). Contiguous markup of the same kind and author - /// groups into one entry per user-visible change (an inserted paragraph is ONE - /// revision: its runs plus its mark); a named move pair is one "move" entry - /// covering both sides. Ids derive from the markup's w:id attributes, so - /// they are stable across calls and across resolution of other revisions — - /// unlike the re-diff listing, authors/dates are the markup's own. Not - /// enumerated in v1 (still resolved by whole-document accept/reject): - /// cellIns/cellDel/cellMerge, content-control ins/del - /// ranges, and numPr numbering-ins markers. + /// Enumerate the document's tracked revisions from a live, part-aware registry. + /// Cell structure, content-control envelopes, and numbering families are atomic + /// entries alongside the existing content/move/property families. Bad native + /// markup remains visible with a diagnostic and cannot be resolved accidentally. /// public IReadOnlyList ListRevisions() { ThrowIfDisposed(); _ = AnchorIndex(); // guarantees Unids so entries can carry block anchors - var parts = RevisionStoryParts(); - var groups = Internal.RevisionOps.Enumerate(parts.Select(p => p.Root).ToList()); - var result = new List(groups.Count); - foreach (var g in groups) - { - var partUri = parts[g.PartIndex].Part.Uri.ToString(); - string? anchorId = null; - if (g.Units.Count > 0) - { - var first = g.Units[0]; - for (var a = first.Paragraph ?? first.MarkedRow ?? first.Element; a is not null; a = a.Parent) - { - var unid = (string?)a.Attribute(PtOpenXml.Unid); - if (unid is null) continue; - if (AnchorForUnid(unid, partUri) is { } anch) anchorId = anch.Id; - break; - } - } - result.Add(new RevisionListEntry( - g.Id, g.Type, g.Author, g.Date, Internal.RevisionOps.GroupText(g), anchorId)); + var registry = BuildRevisionRegistry(); + var result = new List(registry.Entries.Count); + foreach (var g in registry.Entries) + { + var affected = RevisionGroupAnchors(g, g.PartUri); + result.Add(new RevisionListEntry + { + Id = g.Id, + Type = g.Type, + Family = g.Family, + ConstituentIds = Internal.RevisionOps.ConstituentIds(g), + Author = g.Author, + Date = g.Date, + Text = Internal.RevisionOps.GroupText(g), + PartUri = g.PartUri, + Scope = g.Scope, + AnchorId = affected.Count == 0 ? null : affected[0].Id, + AffectedAnchors = affected, + ResolutionStatus = g.ResolutionStatus, + Diagnostic = g.Diagnostic, + }); } return result; } @@ -2660,6 +2704,14 @@ public IReadOnlyList ListRevisions() /// old properties. public EditResult RejectRevision(string revisionId) => ResolveRevision(revisionId, accept: false); + /// Accept every live revision through the same fail-closed resolver used by + /// . The complete operation is one undo step. + public EditResult AcceptAllRevisions() => ResolveAllRevisions(accept: true); + + /// Reject every live revision through the same fail-closed resolver used by + /// . The complete operation is one undo step. + public EditResult RejectAllRevisions() => ResolveAllRevisions(accept: false); + private EditResult ResolveRevision(string revisionId, bool accept) { if (_disposed) return EditResult.Fail(EditErrorCode.SessionDisposed, "session disposed"); @@ -2667,14 +2719,16 @@ private EditResult ResolveRevision(string revisionId, bool accept) return EditResult.Fail(EditErrorCode.RevisionNotFound, "revision id is empty"); _ = AnchorIndex(); - var parts = RevisionStoryParts(); - var groups = Internal.RevisionOps.Enumerate(parts.Select(p => p.Root).ToList()); - var group = groups.FirstOrDefault(x => x.Id == revisionId); + var registry = BuildRevisionRegistry(); + var group = registry.Find(revisionId); if (group is null) return EditResult.Fail(EditErrorCode.RevisionNotFound, $"revision not found: {revisionId}"); - var owningPart = parts[group.PartIndex].Part; - var partUri = owningPart.Uri.ToString(); + if (RevisionResolutionError(group) is { } resolutionError) + return resolutionError; + + var partUri = group.PartUri; + var owningPart = ResolvePart(partUri); // Capture the block anchors the resolution touches BEFORE applying — elements // detach during Apply and can no longer be resolved to a part afterwards. @@ -2683,8 +2737,9 @@ private EditResult ResolveRevision(string revisionId, bool accept) _history.RecordPreOp(TakeSnapshot()); try { - var removedElements = Internal.RevisionOps.Apply(group, accept); - SweepOrphanedStoryRelationships(owningPart); + var removedElements = registry.Resolve(group, accept); + if (owningPart is not null) + SweepOrphanedStoryRelationships(owningPart); var removed = new List(); var seenRemoved = new HashSet(StringComparer.Ordinal); @@ -2715,14 +2770,115 @@ private EditResult ResolveRevision(string revisionId, bool accept) } } + private EditResult ResolveAllRevisions(bool accept) + { + if (_disposed) return EditResult.Fail(EditErrorCode.SessionDisposed, "session disposed"); + + _ = AnchorIndex(); + var registry = BuildRevisionRegistry(); + if (registry.Entries.Count == 0) + return new EditResult { Success = true }; + + var blocked = registry.Entries.FirstOrDefault(g => + g.ResolutionStatus != RevisionResolutionStatus.Supported); + if (blocked is not null) + return RevisionResolutionError(blocked)!; + + var modified = registry.Entries.SelectMany(g => RevisionGroupAnchors(g, g.PartUri)) + .GroupBy(a => a.Id, StringComparer.Ordinal).Select(g => g.First()).ToList(); + + _history.RecordPreOp(TakeSnapshot()); + try + { + var removedElements = registry.ResolveAll(accept); + foreach (var story in RevisionStoryParts()) + SweepOrphanedStoryRelationships(story.Part); + var removed = new List(); + var seenRemoved = new HashSet(StringComparer.Ordinal); + foreach (var element in removedElements) + { + var partUri = PartUriOf(element) ?? registry.Entries + .FirstOrDefault(g => g.Units.Any(u => ReferenceEquals(u.Element, element) + || ReferenceEquals(u.MarkedCell, element) + || ReferenceEquals(u.MarkedRow, element) + || ReferenceEquals(u.StructuredWrapper, element)))?.PartUri; + foreach (var descendant in element.DescendantsAndSelf()) + { + var unid = (string?)descendant.Attribute(PtOpenXml.Unid); + if (unid is null) continue; + if (AnchorForUnid(unid, partUri) is { } anchor && seenRemoved.Add(anchor.Id)) + removed.Add(anchor); + } + } + + InvalidateProjectionCache(); + return new EditResult + { + Success = true, + Modified = modified.Where(a => !seenRemoved.Contains(a.Id)).ToList(), + Removed = removed, + }; + } + catch (Internal.RevisionResolutionException ex) + { + RollbackFailedOp(); + return RevisionResolutionError(ex.Group)!; + } + catch (Exception ex) + { + LastInternalError = ex; + RollbackFailedOp(); + return EditResult.Fail(EditErrorCode.InternalError, ex.Message); + } + } + + private static EditResult? RevisionResolutionError(Internal.RevisionOps.RevisionGroup group) + { + var code = group.ResolutionStatus switch + { + RevisionResolutionStatus.Unsupported => EditErrorCode.RevisionUnsupported, + RevisionResolutionStatus.Malformed => EditErrorCode.RevisionMalformed, + RevisionResolutionStatus.Ambiguous => EditErrorCode.RevisionAmbiguous, + _ => (EditErrorCode?)null, + }; + return code is null ? null : EditResult.Fail(code.Value, + group.Diagnostic?.Message ?? "revision cannot be resolved safely"); + } + private List RevisionGroupAnchors( Internal.RevisionOps.RevisionGroup group, string partUri) { var anchors = new List(); var seen = new HashSet(StringComparer.Ordinal); + var structuralTables = group.Units.Where(u => u.Kind == Internal.RevisionOps.UnitKind.CellMark) + .Select(u => u.Table).Where(t => t is not null).Select(t => t!).Distinct().ToList(); + foreach (var table in structuralTables) + { + foreach (var element in table.DescendantsAndSelf()) + { + var unid = (string?)element.Attribute(PtOpenXml.Unid); + if (unid is not null && AnchorForUnid(unid, partUri) is { } anchor + && seen.Add(anchor.Id)) + anchors.Add(anchor); + } + } + foreach (var unit in group.Units) { - for (var element = unit.Paragraph ?? unit.MarkedRow ?? unit.Element; + var start = unit.MarkedCell ?? unit.Paragraph ?? unit.MarkedRow + ?? unit.StructuredWrapper ?? unit.Element; + if (unit.StructuredWrapper is { } wrapper) + { + foreach (var element in wrapper.DescendantsAndSelf()) + { + var descendantUnid = (string?)element.Attribute(PtOpenXml.Unid); + if (descendantUnid is not null + && AnchorForUnid(descendantUnid, partUri) is { } descendantAnchor + && seen.Add(descendantAnchor.Id)) + anchors.Add(descendantAnchor); + } + } + for (var element = start; element is not null; element = element.Parent) { var unid = (string?)element.Attribute(PtOpenXml.Unid); @@ -2735,19 +2891,35 @@ private List RevisionGroupAnchors( return anchors; } + private Internal.RevisionRegistry BuildRevisionRegistry() + { + var parts = RevisionStoryParts(); + return Internal.RevisionRegistry.Build(parts.Select(p => + new Internal.RevisionRegistry.Part( + p.Part.Uri.ToString(), p.Scope, p.Root)).ToList()); + } + /// The story parts revision markup lives in, in the fixed order the /// revision enumeration indexes them (main, headers, footers, footnotes, endnotes /// — the same set RevisionProcessor's whole-document accept/reject walks). - private List<(OpenXmlPart Part, XElement Root)> RevisionStoryParts() + private List<(OpenXmlPart Part, XElement Root, string Scope)> RevisionStoryParts() { - var list = new List<(OpenXmlPart, XElement)>(); - foreach (var part in EnumerateProjectedPartsForScopes( - ProjectionScopes.Body | ProjectionScopes.Headers | ProjectionScopes.Footers - | ProjectionScopes.Footnotes | ProjectionScopes.Endnotes)) + var list = new List<(OpenXmlPart, XElement, string)>(); + void Add(OpenXmlPart part, string scope) { var root = part.GetXDocument().Root; - if (root is not null) list.Add((part, root)); + if (root is not null) list.Add((part, root, scope)); } + + var main = _doc!.MainDocumentPart; + if (main is null) return list; + Add(main, "body"); + int index = 1; + foreach (var header in main.HeaderParts) Add(header, $"hdr{index++}"); + index = 1; + foreach (var footer in main.FooterParts) Add(footer, $"ftr{index++}"); + if (main.FootnotesPart is not null) Add(main.FootnotesPart, "fn"); + if (main.EndnotesPart is not null) Add(main.EndnotesPart, "en"); return list; } @@ -6842,19 +7014,8 @@ private void MarkTableRowsAsTrackedRevision( XElement table, bool inserted, string author, string date) { EnsureTrackRevisionsEnabled(); - var wrapperName = inserted ? W.ins : W.del; foreach (var row in table.Descendants(W.tr).ToList()) - { - var trPr = row.Element(W.trPr); - if (trPr is null) - { - trPr = new XElement(W.trPr); - row.AddFirst(trPr); - } - trPr.Add(CreateRevisionEnvelope(wrapperName, author, date)); - foreach (var paragraph in row.Descendants(W.p).ToList()) - MarkParagraphContentAndMark(paragraph, wrapperName, author, date); - } + MarkRowAsTrackedRevision(row, inserted, author, date); } public EditResult InsertParagraph(string anchorId, Position pos, string markdownPayload) @@ -8829,18 +8990,20 @@ public EditResult AddCommentToRevision( return EditResult.Fail(EditErrorCode.RevisionNotFound, "revision id is empty"); _ = AnchorIndex(); - var parts = RevisionStoryParts(); - var groups = Internal.RevisionOps.Enumerate(parts.Select(p => p.Root).ToList()); - var group = groups.FirstOrDefault(x => x.Id == revisionId); + var registry = BuildRevisionRegistry(); + var group = registry.Find(revisionId); if (group is null) return EditResult.Fail(EditErrorCode.RevisionNotFound, $"revision not found: {revisionId}"); + if (RevisionResolutionError(group) is { } resolutionError) + return resolutionError; + var commentTarget = Internal.RevisionOps.CommentTarget(group); if (commentTarget is null) return EditResult.Fail(EditErrorCode.RevisionNotFound, $"revision has no commentable extent: {revisionId}"); - var partUri = parts[group.PartIndex].Part.Uri.ToString(); + var partUri = group.PartUri; var modified = RevisionGroupAnchors(group, partUri); return AddCommentCore(author, markdownPayload, initials, date, placeMarkers: id => @@ -9614,6 +9777,51 @@ private static IReadOnlyList InvalidatedCellAnchors(TableAnchorMapping m .Select(location => location.Anchor) .ToList(); + private EditResult? RefuseUnresolvedTableStructure(XElement table, string anchorId) + { + var pending = BuildRevisionRegistry().Entries.FirstOrDefault(group => + group.Units.Any(unit => ReferenceEquals(unit.Table, table)) + && (group.Family == RevisionFamily.CellInsert + || group.Family == RevisionFamily.CellDelete + || group.Family == RevisionFamily.CellMerge)); + return pending is null ? null : EditResult.Fail( + EditErrorCode.UnresolvedStructuralRevision, + $"table has unresolved {pending.Family} revision {pending.Id}; resolve it before another structural mutation", + anchorId); + } + + private static EditResult TrackedStructureUnsupported(string operation, string anchorId) => + EditResult.Fail(EditErrorCode.TrackedOperationUnsupported, + $"{operation} has no reversible native tracked-change encoding on this document shape; no changes were made", + anchorId); + + private static XElement PropertySnapshot(XElement? properties, XName propertyName, XName changeName, + params XName[] excluded) + { + var exclude = excluded.Append(changeName).ToHashSet(); + return new XElement(propertyName, + properties?.Attributes().Where(a => !a.IsNamespaceDeclaration && a.Name != PtOpenXml.Unid), + properties?.Elements().Where(e => !exclude.Contains(e.Name)).Select(e => new XElement(e))); + } + + private static bool PropertySnapshotEquals(XElement snapshot, XElement? current, XName changeName, + params XName[] excluded) => + XNode.DeepEquals(snapshot, PropertySnapshot(current, snapshot.Name, changeName, excluded)); + + private void MarkRowAsTrackedRevision(XElement row, bool inserted, string author, string date) + { + var wrapperName = inserted ? W.ins : W.del; + var trPr = row.Element(W.trPr); + if (trPr is null) + { + trPr = new XElement(W.trPr); + row.AddFirst(trPr); + } + trPr.Add(CreateRevisionEnvelope(wrapperName, author, date)); + foreach (var paragraph in row.Descendants(W.p).ToList()) + MarkParagraphContentAndMark(paragraph, wrapperName, author, date); + } + /// An empty clone of 's shell (width, borders, shading, /// valign). Merge markup is always dropped — a clone is a fresh cell, never half of somebody /// else's merge — except w:gridSpan when is set, which a new @@ -9642,6 +9850,7 @@ public EditResult InsertTableRow(string cellAnchorId, Position pos) { if (ResolveCell(cellAnchorId, out _, out _, out var tr, out var tbl, out var target) is { } err) return err; + if (RefuseUnresolvedTableStructure(tbl!, cellAnchorId) is { } pending) return pending; var before = CaptureTableMetadata(tbl!); _history.RecordPreOp(TakeSnapshot()); @@ -9675,6 +9884,12 @@ public EditResult InsertTableRow(string cellAnchorId, Position pos) if (pos == Position.Before) tr.AddBeforeSelf(newTr); else tr.AddAfterSelf(newTr); + if (_trackedChanges == TrackedChangeMode.RenderInline) + { + MarkRowAsTrackedRevision(newTr, inserted: true, + _revisionAuthor ?? "docxodus", NextTrackedFormatRevisionDate()); + } + InvalidateProjectionCache(); return new EditResult { @@ -9701,11 +9916,19 @@ public EditResult InsertTableColumn(string cellAnchorId, Position pos) { if (ResolveCell(cellAnchorId, out _, out var tc, out var tr, out var tbl, out var target) is { } err) return err; + if (RefuseUnresolvedTableStructure(tbl!, cellAnchorId) is { } pending) return pending; var anchorCell = RowGrid(tr!).First(g => g.Tc == tc); int boundary = pos == Position.Before ? anchorCell.Start : anchorCell.End; var before = CaptureTableMetadata(tbl!); + var tracked = _trackedChanges == TrackedChangeMode.RenderInline; + var oldGrid = tracked ? new XElement(tbl!.Element(W.tblGrid) ?? new XElement(W.tblGrid)) : null; + var oldRows = tracked ? tbl!.Elements(W.tr).ToDictionary(row => row, + row => PropertySnapshot(row.Element(W.trPr), W.trPr, W.trPrChange, W.ins, W.del)) : null; + var oldCells = tracked ? tbl!.Descendants(W.tc).ToDictionary(cell => cell, + cell => PropertySnapshot(cell.Element(W.tcPr), W.tcPr, W.tcPrChange, + W.cellIns, W.cellDel, W.cellMerge)) : null; _history.RecordPreOp(TakeSnapshot()); try { @@ -9762,6 +9985,37 @@ public EditResult InsertTableColumn(string cellAnchorId, Position pos) else right.Tc!.AddBeforeSelf(newTc); } + if (tracked) + { + var author = _revisionAuthor ?? "docxodus"; + var date = NextTrackedFormatRevisionDate(); + var trackedGrid = tbl.Element(W.tblGrid) + ?? throw new InvalidOperationException("tracked column insertion has no table grid"); + trackedGrid.Add(CreateRevisionEnvelope(W.tblGridChange, author, date, oldGrid!)); + + foreach (var pair in oldRows!) + { + if (PropertySnapshotEquals(pair.Value, pair.Key.Element(W.trPr), W.trPrChange, + W.ins, W.del)) continue; + var trPr = pair.Key.Element(W.trPr) ?? new XElement(W.trPr); + if (trPr.Parent is null) pair.Key.AddFirst(trPr); + trPr.Add(CreateRevisionEnvelope(W.trPrChange, author, date, pair.Value)); + } + foreach (var pair in oldCells!) + { + if (PropertySnapshotEquals(pair.Value, pair.Key.Element(W.tcPr), W.tcPrChange, + W.cellIns, W.cellDel, W.cellMerge)) continue; + var tcPr = GetOrCreateTcPr(pair.Key); + tcPr.Add(CreateRevisionEnvelope(W.tcPrChange, author, date, pair.Value)); + } + foreach (var cell in newCells) + { + GetOrCreateTcPr(cell).Add(CreateRevisionEnvelope(W.cellIns, author, date)); + foreach (var paragraph in cell.Elements(W.p)) + MarkParagraphContentAndMark(paragraph, W.ins, author, date); + } + } + InvalidateProjectionCache(); return new EditResult { @@ -9790,12 +10044,24 @@ public EditResult DeleteTableRow(string cellAnchorId) var removalRoot = tbl!.Elements(W.tr).Count() <= 1 ? tbl : tr!; if (ValidateBookmarkRemoval(new[] { removalRoot }, cellAnchorId) is { } bookmarkError) return bookmarkError; + if (RefuseUnresolvedTableStructure(tbl!, cellAnchorId) is { } pending) return pending; + if (_trackedChanges == TrackedChangeMode.RenderInline + && tr!.ElementsAfterSelf(W.tr).FirstOrDefault() is { } trackedNext + && RowGrid(tr).Any(g => VMergeRestart(g.Tc) == true + && AlignedCell(trackedNext, g) is { } heir && VMergeRestart(heir) == false)) + return TrackedStructureUnsupported( + "DeleteTableRow across a vertical-merge restart", cellAnchorId); var before = CaptureTableMetadata(tbl!); _history.RecordPreOp(TakeSnapshot()); try { - if (tbl!.Elements(W.tr).Count() <= 1) tbl.Remove(); + if (_trackedChanges == TrackedChangeMode.RenderInline) + { + MarkRowAsTrackedRevision(tr!, inserted: false, + _revisionAuthor ?? "docxodus", NextTrackedFormatRevisionDate()); + } + else if (tbl!.Elements(W.tr).Count() <= 1) tbl.Remove(); else { if (tr!.ElementsAfterSelf(W.tr).FirstOrDefault() is { } next) @@ -9835,6 +10101,9 @@ public EditResult DeleteTableColumn(string cellAnchorId) if (ResolveCell(cellAnchorId, out _, out var tc, out var tr, out var tbl, out var target) is { } err) return err; var hyperlinkOwner = Internal.OwnedPartRelationships.FindOwner(_doc!, tbl!); + if (RefuseUnresolvedTableStructure(tbl!, cellAnchorId) is { } pending) return pending; + if (_trackedChanges == TrackedChangeMode.RenderInline) + return TrackedStructureUnsupported("DeleteTableColumn", cellAnchorId); int doomed = RowGrid(tr!).First(g => g.Tc == tc).Start; int existingColumns = GridColumnCount(tbl!); @@ -9963,6 +10232,9 @@ public EditResult MergeCells(string cellAnchorId, int rowSpan, int colSpan, { if (ResolveCell(cellAnchorId, out _, out var tc, out var tr, out var tbl, out var target) is { } err) return err; + if (RefuseUnresolvedTableStructure(tbl!, cellAnchorId) is { } pending) return pending; + if (_trackedChanges == TrackedChangeMode.RenderInline) + return TrackedStructureUnsupported("MergeCells", cellAnchorId); var opts = options ?? new TableMergeOptions(); if (rowSpan < 1 || colSpan < 1 || (long)rowSpan * colSpan < 2) @@ -10077,6 +10349,9 @@ public EditResult UnmergeCells(string cellAnchorId) { if (ResolveCell(cellAnchorId, out _, out var tc, out var tr, out var tbl, out var target) is { } err) return err; + if (RefuseUnresolvedTableStructure(tbl!, cellAnchorId) is { } pending) return pending; + if (_trackedChanges == TrackedChangeMode.RenderInline) + return TrackedStructureUnsupported("UnmergeCells", cellAnchorId); var shape = RowGrid(tr!).First(g => g.Tc == tc); bool? vMerge = VMergeRestart(tc!); @@ -10492,6 +10767,38 @@ public EditResult SetTableRowOptions(string cellAnchorId, TableRowOptions? optio } } + private EditResult? RefuseNestedTrackedListChange(XElement paragraph, string anchorId) + { + if (_trackedChanges != TrackedChangeMode.RenderInline) return null; + return paragraph.Element(W.pPr)?.Element(W.pPrChange) is null + ? null + : EditResult.Fail(EditErrorCode.UnresolvedStructuralRevision, + "paragraph has an unresolved property revision; resolve it before another tracked list mutation", + anchorId); + } + + private void TrackListPropertyMutation(XElement paragraph, XElement oldPPr, bool insertedNumPr) + { + if (_trackedChanges != TrackedChangeMode.RenderInline) return; + var author = _revisionAuthor ?? "docxodus"; + var date = NextTrackedFormatRevisionDate(); + var pPr = paragraph.Element(W.pPr); + var oldBase = PropertySnapshot(oldPPr, W.pPr, W.pPrChange, W.rPr, W.sectPr); + var newBase = PropertySnapshot(pPr, W.pPr, W.pPrChange, W.rPr, W.sectPr); + if (XNode.DeepEquals(oldBase, newBase)) return; + if (pPr is null) + { + pPr = new XElement(W.pPr); + paragraph.AddFirst(pPr); + } + if (insertedNumPr && pPr.Element(W.numPr) is { } numPr) + { + numPr.Add(CreateRevisionEnvelope(W.ins, author, date)); + return; + } + pPr.Add(CreateRevisionEnvelope(W.pPrChange, author, date, oldBase)); + } + public EditResult SetListLevel(string anchorId, int levelDelta) { if (_disposed) return EditResult.Fail(EditErrorCode.SessionDisposed, "session disposed"); @@ -10503,9 +10810,12 @@ public EditResult SetListLevel(string anchorId, int levelDelta) var element = target.Resolve(_doc!); if (element is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "element null", anchorId); + if (RefuseNestedTrackedListChange(element, anchorId) is { } pending) return pending; var pPr = element.Element(W.pPr); var numPr = pPr?.Element(W.numPr); + var oldPPr = new XElement(pPr ?? new XElement(W.pPr)); + bool insertedNumPr = numPr is null; // Resolve the effective (numId, current ilvl). A direct w:numPr wins; otherwise the // paragraph is a list item only via its pStyle chain (e.g. python-docx "List Bullet", @@ -10550,6 +10860,7 @@ public EditResult SetListLevel(string anchorId, int levelDelta) new XElement(W.ilvl, new XAttribute(W.val, next)), new XElement(W.numId, new XAttribute(W.val, effectiveNumId!.Value)))); } + TrackListPropertyMutation(element, oldPPr, insertedNumPr); // Flush the body mutation to the part stream immediately — same as NumberingFactory does for // the numbering part. Without this the materialized w:numPr lives only in the in-memory // XDocument; under WASM the typed-DOM/XDocument divergence means a later Save() serializes @@ -10605,8 +10916,10 @@ public EditResult RemoveListMembership(string anchorId) "RemoveListMembership requires a paragraph, heading, or list-item anchor", anchorId); var element = target.Resolve(_doc!); if (element is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "element null", anchorId); + if (RefuseNestedTrackedListChange(element, anchorId) is { } pending) return pending; var pPr = element.Element(W.pPr); + var oldPPr = new XElement(pPr ?? new XElement(W.pPr)); var directNumPr = pPr?.Element(W.numPr); // Removing a direct numPr can expose numbering inherited from the paragraph style. // Materialize Word's numId=0 sentinel in that case so the explicit removal wins over @@ -10623,6 +10936,7 @@ public EditResult RemoveListMembership(string anchorId) new XElement(W.ilvl, new XAttribute(W.val, 0)), new XElement(W.numId, new XAttribute(W.val, 0)))); } + TrackListPropertyMutation(element, oldPPr, insertedNumPr: false); InvalidateProjectionCache(); var updated = AnchorForUnid(target.Unid, target.PartUri) ?? target.Anchor; return new EditResult @@ -10650,6 +10964,10 @@ public EditResult ApplyListFormat(string anchorId, ListFormat kind) return EditResult.Fail(EditErrorCode.AnchorWrongKind, "ApplyListFormat requires a paragraph anchor", anchorId); var element = target.Resolve(_doc!); if (element is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "element null", anchorId); + if (RefuseNestedTrackedListChange(element, anchorId) is { } pending) return pending; + + var oldPPr = new XElement(element.Element(W.pPr) ?? new XElement(W.pPr)); + bool insertedNumPr = oldPPr.Element(W.numPr) is null && kind != ListFormat.None; _history.RecordPreOp(TakeSnapshot()); try @@ -10670,6 +10988,8 @@ public EditResult ApplyListFormat(string anchorId, ListFormat kind) new XElement(W.numId, new XAttribute(W.val, numId)))); } + TrackListPropertyMutation(element, oldPPr, insertedNumPr); + InvalidateProjectionCache(); var freshIndex = AnchorIndex(); var updated = AnchorForUnid(target.Unid, target.PartUri) ?? target.Anchor; @@ -10702,6 +11022,8 @@ public EditResult ApplyListFormat(string anchorId, ListFormat kind) public EditResult ApplyListFormatRange(string firstAnchorId, string lastAnchorId, ListFormat kind) { if (_disposed) return EditResult.Fail(EditErrorCode.SessionDisposed, "session disposed"); + if (_trackedChanges == TrackedChangeMode.RenderInline) + return TrackedStructureUnsupported("ApplyListFormatRange", firstAnchorId); var firstTarget = FindAnchor(firstAnchorId); if (firstTarget is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, $"first anchor not found: {firstAnchorId}", firstAnchorId); @@ -10829,6 +11151,9 @@ public EditResult ClearListStartOverride(string anchorId) => private EditResult ApplyListStartOverride(string anchorId, int? value) { if (_disposed) return EditResult.Fail(EditErrorCode.SessionDisposed, "session disposed"); + if (_trackedChanges == TrackedChangeMode.RenderInline) + return TrackedStructureUnsupported( + value is null ? "ClearListStartOverride" : "SetListStartOverride", anchorId); var target = FindAnchor(anchorId); if (target is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "anchor not found", anchorId); diff --git a/Docxodus/Internal/DocxSessionJson.cs b/Docxodus/Internal/DocxSessionJson.cs index 813392ed..4e1cb525 100644 --- a/Docxodus/Internal/DocxSessionJson.cs +++ b/Docxodus/Internal/DocxSessionJson.cs @@ -1866,9 +1866,7 @@ public static string SerializeCommentList(IReadOnlyList commen return sb.ToString(); } - /// Serialize output: - /// [{"id","type","author","date"?,"text","anchorId"?}] (date/anchorId - /// omitted when null). + /// Serialize the strict, part-aware revision registry wire shape. public static string SerializeRevisionList(IReadOnlyList revisions) { var sb = new StringBuilder(64 + revisions.Count * 128); @@ -1879,16 +1877,65 @@ public static string SerializeRevisionList(IReadOnlyList revi var r = revisions[i]; sb.Append("{\"id\":").Append(JsonString(r.Id)) .Append(",\"type\":").Append(JsonString(r.Type)) + .Append(",\"family\":").Append(JsonString(RevisionFamilyWire(r.Family))) + .Append(",\"constituentIds\":["); + for (int c = 0; c < r.ConstituentIds.Count; c++) + { + if (c > 0) sb.Append(','); + sb.Append(JsonString(r.ConstituentIds[c])); + } + sb.Append(']') .Append(",\"author\":").Append(JsonString(r.Author)); if (r.Date is not null) sb.Append(",\"date\":").Append(JsonString(r.Date)); - sb.Append(",\"text\":").Append(JsonString(r.Text)); + sb.Append(",\"text\":").Append(JsonString(r.Text)) + .Append(",\"partUri\":").Append(JsonString(r.PartUri)) + .Append(",\"scope\":").Append(JsonString(r.Scope)); if (r.AnchorId is not null) sb.Append(",\"anchorId\":").Append(JsonString(r.AnchorId)); + sb.Append(",\"affectedAnchors\":"); + AppendAnchorArray(sb, r.AffectedAnchors); + sb.Append(",\"resolutionStatus\":") + .Append(JsonString(RevisionStatusWire(r.ResolutionStatus))); + if (r.Diagnostic is not null) + { + sb.Append(",\"diagnostic\":{\"code\":") + .Append(JsonString(r.Diagnostic.Code)) + .Append(",\"message\":").Append(JsonString(r.Diagnostic.Message)) + .Append('}'); + } sb.Append('}'); } sb.Append(']'); return sb.ToString(); } + private static string RevisionFamilyWire(RevisionFamily family) => family switch + { + RevisionFamily.ContentInsert => "content_insert", + RevisionFamily.ContentDelete => "content_delete", + RevisionFamily.Move => "move", + RevisionFamily.ParagraphMark => "paragraph_mark", + RevisionFamily.RowInsert => "row_insert", + RevisionFamily.RowDelete => "row_delete", + RevisionFamily.CellInsert => "cell_insert", + RevisionFamily.CellDelete => "cell_delete", + RevisionFamily.CellMerge => "cell_merge", + RevisionFamily.ContentControlInsert => "content_control_insert", + RevisionFamily.ContentControlDelete => "content_control_delete", + RevisionFamily.NumberingPropertiesInsert => "numbering_properties_insert", + RevisionFamily.NumberingChange => "numbering_change", + RevisionFamily.PropertiesChange => "properties_change", + _ => "unsupported", + }; + + private static string RevisionStatusWire(RevisionResolutionStatus status) => status switch + { + RevisionResolutionStatus.Supported => "supported", + RevisionResolutionStatus.Unsupported => "unsupported", + RevisionResolutionStatus.Malformed => "malformed", + RevisionResolutionStatus.Ambiguous => "ambiguous", + _ => "unsupported", + }; + /// /// Parse an ISO-8601 comment date from the wire; null/empty → null (the deterministic /// no-date default). An unparseable string throws at diff --git a/Docxodus/Internal/DocxSessionOps.cs b/Docxodus/Internal/DocxSessionOps.cs index 34b6bdb3..d31163c5 100644 --- a/Docxodus/Internal/DocxSessionOps.cs +++ b/Docxodus/Internal/DocxSessionOps.cs @@ -861,6 +861,12 @@ public static string RejectRevision(int handle, string revisionId, MutationPreconditions? preconditions = null) => Mutate(handle, preconditions, null, s => s.RejectRevision(revisionId)); + public static string AcceptAllRevisions(int handle) => + DocxSessionJson.Serialize(SessionRegistry.Get(handle).AcceptAllRevisions()); + + public static string RejectAllRevisions(int handle) => + DocxSessionJson.Serialize(SessionRegistry.Get(handle).RejectAllRevisions()); + // ─── Undo / Redo ──────────────────────────────────────────────────── public static bool Undo(int handle) => SessionRegistry.Get(handle).Undo(); diff --git a/Docxodus/Internal/RevisionOps.cs b/Docxodus/Internal/RevisionOps.cs index 2c226d4a..73b1f463 100644 --- a/Docxodus/Internal/RevisionOps.cs +++ b/Docxodus/Internal/RevisionOps.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Security.Cryptography; using System.Text; using System.Xml.Linq; @@ -19,14 +20,14 @@ namespace Docxodus.Internal; /// semantics (unwrap vs. remove, w:delText restore, paragraph-mark coalescing /// into the following paragraph, row removal, stored-property restore). /// -/// v1 scope: run-content ins/del (any story), paragraph-mark ins/del, table-row -/// ins/del (w:trPr markers absorb their row's content markup), named move -/// pairs (both sides resolve together), and the format-change family +/// Scope: run-content ins/del (any story), paragraph-mark ins/del, table-row +/// ins/del (w:trPr markers absorb their row's content markup), table-cell +/// insertion/deletion/vertical-merge operations, content-control envelope ranges, +/// numbering-property insertion/numbering cache changes, named move pairs (both +/// sides resolve together), and the format-change family /// (rPrChange/pPrChange/sectPrChange/tblPrChange/ /// trPrChange/tcPrChange/tblGridChange/tblPrExChange). -/// Exotic families without per-revision semantics here (cellIns/cellDel/ -/// cellMerge, content-control ins/del ranges, numPr/ins) are not -/// enumerated; whole-document accept-all/reject-all still handles them. +/// Unsupported or malformed native markup is enumerated explicitly and fails closed. /// internal static class RevisionOps { @@ -34,18 +35,31 @@ internal static class RevisionOps internal const string TypeDelete = "delete"; internal const string TypeMove = "move"; internal const string TypeFormat = "format"; + internal const string TypeStructure = "structure"; - internal enum UnitKind { Content, ParaMark, RowMark, PropsChange } + internal enum UnitKind + { + Content, + ParaMark, + RowMark, + PropsChange, + CellMark, + NumberingPropertiesInsert, + NumberingChange, + StructuredRange, + Unsupported, + } /// One revision markup element, positioned in document order (a paragraph's /// mark unit is repositioned to the END of its paragraph — that is where the pilcrow /// lives semantically, and what makes multi-paragraph runs of markup group). internal sealed class RevisionUnit { - public required XElement Element { get; init; } - public required UnitKind Kind { get; init; } - public required string Type { get; init; } - public required string Author { get; init; } + required public XElement Element { get; init; } + required public UnitKind Kind { get; init; } + required public string Type { get; init; } + required public RevisionFamily Family { get; init; } + required public string Author { get; init; } public string? Date { get; init; } /// Move-range name when the unit sits inside a named move range — such /// units group per name (both sides of the pair) rather than by adjacency. @@ -54,16 +68,25 @@ internal sealed class RevisionUnit /// For RowMark: the w:tr itself. For Content: the marked row the /// unit sits inside (so the row group absorbs it), else null. public XElement? MarkedRow { get; init; } + public XElement? MarkedCell { get; init; } + public XElement? Table { get; init; } + public XElement? StructuredWrapper { get; init; } public long? Wid { get; init; } + public string? NativeId { get; init; } } internal sealed class RevisionGroup { public string Id { get; set; } = ""; - public required string Type { get; init; } - public required string Author { get; init; } + required public string Type { get; init; } + required public RevisionFamily Family { get; init; } + required public string Author { get; init; } public string? Date { get; set; } - public required int PartIndex { get; init; } + required public int PartIndex { get; init; } + public string PartUri { get; set; } = ""; + public string Scope { get; set; } = "body"; + public RevisionResolutionStatus ResolutionStatus { get; set; } = RevisionResolutionStatus.Supported; + public RevisionDiagnostic? Diagnostic { get; set; } public List Units { get; } = new(); /// Move-range marker elements (start/end, both sides) removed when the /// group resolves. @@ -85,6 +108,18 @@ internal sealed class RevisionCommentTarget private static readonly XName[] RevWrapperNames = { W.ins, W.del, W.moveFrom, W.moveTo }; + private static readonly HashSet StructuredRangeNames = new() + { + W.customXmlDelRangeStart, W.customXmlDelRangeEnd, + W.customXmlInsRangeStart, W.customXmlInsRangeEnd, + }; + + private static readonly HashSet UnsupportedRangeNames = new() + { + W.customXmlMoveFromRangeStart, W.customXmlMoveFromRangeEnd, + W.customXmlMoveToRangeStart, W.customXmlMoveToRangeEnd, + }; + private static readonly HashSet PropsChangeNames = new() { W.rPrChange, W.pPrChange, W.sectPrChange, W.tblPrChange, @@ -101,20 +136,132 @@ internal sealed class RevisionCommentTarget // ─── Enumeration ──────────────────────────────────────────────────── - internal static List Enumerate(IReadOnlyList partRoots) + internal static List Enumerate( + IReadOnlyList<(string PartUri, string Scope, XElement Root)> parts) { var groups = new List(); - for (int pi = 0; pi < partRoots.Count; pi++) + for (int pi = 0; pi < parts.Count; pi++) { var ctx = new WalkCtx(); var units = new List(); - WalkChildren(partRoots[pi].Elements(), ctx, null, null, null, units); + WalkChildren(parts[pi].Root.Elements(), ctx, null, null, null, units); BuildGroups(units, ctx, pi, groups); + AddStructuredRangeGroups(parts[pi].Root, pi, groups); + AddUnsupportedGroups(parts[pi].Root, pi, groups); + + foreach (var group in groups.Where(g => g.PartIndex == pi)) + { + group.PartUri = parts[pi].PartUri; + group.Scope = parts[pi].Scope; + } + + CoalesceTableStructureGroups(groups, pi); + AbsorbTrackedStructuredPayload(groups, pi); } + ValidateGroups(groups); AssignIds(groups); return groups; } + private static void ValidateGroups(List groups) + { + foreach (var group in groups.Where(g => g.ResolutionStatus == RevisionResolutionStatus.Supported)) + { + if (group.Units.Any(u => string.IsNullOrEmpty(u.NativeId)) + && group.Units.Any(u => u.Kind != UnitKind.PropsChange + || u.Element.Name != W.tblGridChange)) + { + group.ResolutionStatus = RevisionResolutionStatus.Malformed; + group.Diagnostic = new RevisionDiagnostic( + "missing_revision_id", + "A live revision marker has no w:id and cannot be addressed stably."); + continue; + } + + if (group.Family == RevisionFamily.CellInsert + || group.Family == RevisionFamily.CellDelete + || group.Family == RevisionFamily.CellMerge) + { + var cells = group.Units.Where(u => u.Kind == UnitKind.CellMark).ToList(); + if (cells.Count == 0 || cells.Any(u => u.MarkedCell is null || u.Table is null)) + { + group.ResolutionStatus = RevisionResolutionStatus.Malformed; + group.Diagnostic = new RevisionDiagnostic( + "orphan_cell_revision", + "A cell structural marker is not a direct property of a table cell."); + continue; + } + + if (group.Family == RevisionFamily.CellDelete) + { + var deleted = cells.Select(u => u.MarkedCell!).ToHashSet(); + bool invalidRow = deleted.GroupBy(c => c.Parent).Any(byRow => + { + var rowCells = byRow.Key?.Elements(W.tc).ToList() ?? new List(); + int firstDeleted = rowCells.FindIndex(deleted.Contains); + return firstDeleted == 0 || rowCells.All(deleted.Contains); + }); + if (invalidRow) + { + group.ResolutionStatus = RevisionResolutionStatus.Malformed; + group.Diagnostic = new RevisionDiagnostic( + "unabsorbable_cell_deletion", + "A deleted-cell run has no surviving predecessor that can absorb its grid columns."); + continue; + } + } + + if (group.Family == RevisionFamily.CellMerge && cells.Any(u => + (string?)u.Element.Attribute(W.vMerge) is not ("rest" or "cont"))) + { + group.ResolutionStatus = RevisionResolutionStatus.Malformed; + group.Diagnostic = new RevisionDiagnostic( + "invalid_cell_merge_state", + "w:cellMerge must carry w:vMerge='rest' or 'cont'."); + } + } + + if (group.Family == RevisionFamily.NumberingPropertiesInsert + && group.Units.Any(u => u.Element.Parent?.Name != W.numPr + || u.Paragraph is null)) + { + group.ResolutionStatus = RevisionResolutionStatus.Malformed; + group.Diagnostic = new RevisionDiagnostic( + "orphan_numbering_revision", + "A numbering revision marker is not a direct child of paragraph w:numPr."); + } + + if (group.Family == RevisionFamily.NumberingChange + && group.Units.Any(u => (u.Element.Parent?.Name != W.numPr + && u.Element.Parent?.Name != W.fldChar) + || u.Paragraph is null)) + { + group.ResolutionStatus = RevisionResolutionStatus.Malformed; + group.Diagnostic = new RevisionDiagnostic( + "orphan_numbering_revision", + "w:numberingChange is not attached to paragraph numbering properties or a LISTNUM field."); + } + } + + // Reusing one live revision id for multiple independent groups in one part + // makes an id-based operation inherently ambiguous. Range-pair duplication was + // diagnosed earlier and groups already coalesced into one operation are fine. + foreach (var collision in groups.SelectMany(g => ConstituentIds(g) + .Select(id => (Group: g, Id: id))) + .GroupBy(x => (x.Group.PartUri, x.Id)) + .Where(g => g.Select(x => x.Group).Distinct().Count() > 1)) + { + foreach (var group in collision.Select(x => x.Group).Distinct() + .Where(g => g.ResolutionStatus == RevisionResolutionStatus.Supported)) + { + group.ResolutionStatus = RevisionResolutionStatus.Ambiguous; + group.Diagnostic = new RevisionDiagnostic( + "duplicate_revision_id", + $"w:id '{collision.Key.Id}' identifies multiple live revisions in {collision.Key.PartUri}."); + } + } + } + /// /// Resolve the exact live extent a Word comment should bracket for a revision. Content /// revisions bracket their outer revision wrappers, which keeps the comment markers outside @@ -214,6 +361,11 @@ private static void WalkChildren(IEnumerable children, WalkCtx ctx, foreach (var child in children) { var n = child.Name; + // Content-control envelope ranges are paired and validated in a dedicated + // pass. Treating their starts as ordinary adjacent units loses the two-pair + // topology that identifies the wrapper whose existence is revised. + if (StructuredRangeNames.Contains(n) || UnsupportedRangeNames.Contains(n)) + continue; if (n == W.moveFromRangeStart || n == W.moveToRangeStart) { var name = (string?)child.Attribute(W.name); @@ -240,6 +392,62 @@ private static void WalkChildren(IEnumerable children, WalkCtx ctx, } if (n == W.p) { WalkParagraph(child, ctx, markedRow, markedRowType, sink); continue; } if (n == W.tr) { WalkRow(child, ctx, sink); continue; } + if ((n == W.cellIns || n == W.cellDel || n == W.cellMerge) + && child.Parent?.Name == W.tcPr) + { + var cell = child.Ancestors(W.tc).FirstOrDefault(); + var table = child.Ancestors(W.tbl).FirstOrDefault(); + var family = n == W.cellIns ? RevisionFamily.CellInsert + : n == W.cellDel ? RevisionFamily.CellDelete + : RevisionFamily.CellMerge; + sink.Add(new RevisionUnit + { + Element = child, + Kind = UnitKind.CellMark, + Type = n == W.cellIns ? TypeInsert : n == W.cellDel ? TypeDelete : TypeStructure, + Family = family, + Author = AuthorOf(child), + Date = (string?)child.Attribute(W.date), + MarkedCell = cell, + MarkedRow = cell?.Parent, + Table = table, + Wid = WidOf(child), + NativeId = (string?)child.Attribute(W.id), + }); + continue; + } + if (n == W.ins && child.Parent?.Name == W.numPr) + { + sink.Add(new RevisionUnit + { + Element = child, + Kind = UnitKind.NumberingPropertiesInsert, + Type = TypeInsert, + Family = RevisionFamily.NumberingPropertiesInsert, + Author = AuthorOf(child), + Date = (string?)child.Attribute(W.date), + Paragraph = child.Ancestors(W.p).FirstOrDefault(), + Wid = WidOf(child), + NativeId = (string?)child.Attribute(W.id), + }); + continue; + } + if (n == W.numberingChange) + { + sink.Add(new RevisionUnit + { + Element = child, + Kind = UnitKind.NumberingChange, + Type = TypeFormat, + Family = RevisionFamily.NumberingChange, + Author = AuthorOf(child), + Date = (string?)child.Attribute(W.date), + Paragraph = child.Ancestors(W.p).FirstOrDefault(), + Wid = WidOf(child), + NativeId = (string?)child.Attribute(W.id), + }); + continue; + } if ((n == W.ins || n == W.del || n == W.moveFrom || n == W.moveTo) && IsContentWrapper(child)) { sink.Add(MakeUnit(child, UnitKind.Content, paragraph, markedRow, markedRowType, ctx)); @@ -268,6 +476,46 @@ private static void WalkParagraph(XElement p, WalkCtx ctx, { foreach (var pc in pPr.Descendants().Where(d => PropsChangeNames.Contains(d.Name))) sink.Add(MakePropsUnit(pc, p)); + + // pPr is otherwise handled specially so paragraph-mark revisions can be + // emitted at the semantic pilcrow position. Inventory the numbering-only + // families explicitly, while excluding archived *PrChange payloads. + foreach (var numPr in pPr.DescendantsAndSelf(W.numPr) + .Where(np => !np.Ancestors().Any(a => PropsChangeNames.Contains(a.Name)))) + { + foreach (var marker in numPr.Elements(W.ins)) + { + sink.Add(new RevisionUnit + { + Element = marker, + Kind = UnitKind.NumberingPropertiesInsert, + Type = TypeInsert, + Family = RevisionFamily.NumberingPropertiesInsert, + Author = AuthorOf(marker), + Date = (string?)marker.Attribute(W.date), + Paragraph = p, + Table = p.Ancestors(W.tbl).FirstOrDefault(), + Wid = WidOf(marker), + NativeId = (string?)marker.Attribute(W.id), + }); + } + foreach (var marker in numPr.Elements(W.numberingChange)) + { + sink.Add(new RevisionUnit + { + Element = marker, + Kind = UnitKind.NumberingChange, + Type = TypeFormat, + Family = RevisionFamily.NumberingChange, + Author = AuthorOf(marker), + Date = (string?)marker.Attribute(W.date), + Paragraph = p, + Table = p.Ancestors(W.tbl).FirstOrDefault(), + Wid = WidOf(marker), + NativeId = (string?)marker.Attribute(W.id), + }); + } + } } WalkChildren(p.Elements().Where(e => e.Name != W.pPr), ctx, p, markedRow, markedRowType, sink); @@ -298,10 +546,13 @@ private static void WalkRow(XElement tr, WalkCtx ctx, List sink) Element = mark, Kind = UnitKind.RowMark, Type = rowType, + Family = rowType == TypeInsert ? RevisionFamily.RowInsert : RevisionFamily.RowDelete, Author = AuthorOf(mark), Date = (string?)mark.Attribute(W.date), MarkedRow = tr, + Table = tr.Parent, Wid = WidOf(mark), + NativeId = (string?)mark.Attribute(W.id), }); } } @@ -341,12 +592,19 @@ private static RevisionUnit MakeUnit(XElement el, UnitKind kind, XElement? parag Element = el, Kind = kind, Type = type, + Family = kind == UnitKind.ParaMark + ? RevisionFamily.ParagraphMark + : type == TypeInsert ? RevisionFamily.ContentInsert + : type == TypeDelete ? RevisionFamily.ContentDelete + : RevisionFamily.Move, Author = AuthorOf(el), Date = (string?)el.Attribute(W.date), MoveName = moveName, Paragraph = paragraph, MarkedRow = markedRowType == type ? markedRow : null, + Table = el.Ancestors(W.tbl).FirstOrDefault(), Wid = WidOf(el), + NativeId = (string?)el.Attribute(W.id), }; } @@ -356,10 +614,13 @@ private static RevisionUnit MakePropsUnit(XElement el, XElement? paragraph) => Element = el, Kind = UnitKind.PropsChange, Type = TypeFormat, + Family = RevisionFamily.PropertiesChange, Author = AuthorOf(el), Date = (string?)el.Attribute(W.date), Paragraph = paragraph, + Table = el.Ancestors(W.tbl).FirstOrDefault(), Wid = WidOf(el), + NativeId = (string?)el.Attribute(W.id), }; private static string AuthorOf(XElement el) => (string?)el.Attribute(W.author) ?? "unknown"; @@ -374,6 +635,7 @@ private static void BuildGroups(List units, WalkCtx ctx, int partI { var moveGroups = new Dictionary(StringComparer.Ordinal); var rowGroupByTr = new Dictionary(); + var cellGroups = new List(); RevisionGroup? cur = null; RevisionGroup? lastRowGroup = null; @@ -383,7 +645,14 @@ private static void BuildGroups(List units, WalkCtx ctx, int partI { if (!moveGroups.TryGetValue(u.MoveName, out var mg)) { - mg = new RevisionGroup { Type = TypeMove, Author = u.Author, Date = u.Date, PartIndex = partIndex }; + mg = new RevisionGroup + { + Type = TypeMove, + Family = RevisionFamily.Move, + Author = u.Author, + Date = u.Date, + PartIndex = partIndex, + }; if (ctx.RangeMarkers.TryGetValue(u.MoveName, out var markers)) mg.RangeMarkers.AddRange(markers); moveGroups[u.MoveName] = mg; @@ -393,6 +662,32 @@ private static void BuildGroups(List units, WalkCtx ctx, int partI continue; } + if (u.Kind == UnitKind.CellMark) + { + var cellGroup = cellGroups.FirstOrDefault(g => + g.Family == u.Family && g.Author == u.Author && g.Date == u.Date + && ReferenceEquals(g.Units[0].Table, u.Table)); + if (cellGroup is null) + { + cellGroup = NewGroup(u, partIndex); + cellGroups.Add(cellGroup); + groups.Add(cellGroup); + } + else + { + cellGroup.Units.Add(u); + } + cur = null; + continue; + } + + if (u.Kind == UnitKind.NumberingPropertiesInsert || u.Kind == UnitKind.NumberingChange) + { + groups.Add(NewGroup(u, partIndex)); + cur = null; + continue; + } + if (u.Kind == UnitKind.RowMark) { if (lastRowGroup is not null && lastRowGroup.Type == u.Type && lastRowGroup.Author == u.Author @@ -452,11 +747,244 @@ private static void BuildGroups(List units, WalkCtx ctx, int partI private static RevisionGroup NewGroup(RevisionUnit u, int partIndex) { - var g = new RevisionGroup { Type = u.Type, Author = u.Author, Date = u.Date, PartIndex = partIndex }; + var g = new RevisionGroup + { + Type = u.Type, + Family = u.Family, + Author = u.Author, + Date = u.Date, + PartIndex = partIndex, + }; g.Units.Add(u); return g; } + /// + /// Recognize the exact two-range topology Word uses to revise an SDT envelope. + /// Any unpaired, duplicated, or topologically misplaced marker remains visible as + /// a malformed/ambiguous registry entry instead of being silently ignored. + /// + private static void AddStructuredRangeGroups( + XElement root, int partIndex, List groups) + { + var allMarkers = root.Descendants() + .Where(e => StructuredRangeNames.Contains(e.Name)) + .ToList(); + var used = new HashSet(); + + foreach (var sdt in root.Descendants(W.sdt)) + { + var content = sdt.Element(W.sdtContent); + if (content is null) continue; + + var before = sdt.ElementsBeforeSelf().LastOrDefault(); + var after = sdt.ElementsAfterSelf().FirstOrDefault(); + var firstInside = content.Elements().FirstOrDefault(); + var lastInside = content.Elements().LastOrDefault(); + if (before is null || after is null || firstInside is null || lastInside is null) + continue; + + bool isInsert = before.Name == W.customXmlInsRangeStart; + bool isDelete = before.Name == W.customXmlDelRangeStart; + if (!isInsert && !isDelete) continue; + + var startName = isInsert ? W.customXmlInsRangeStart : W.customXmlDelRangeStart; + var endName = isInsert ? W.customXmlInsRangeEnd : W.customXmlDelRangeEnd; + var firstId = (string?)before.Attribute(W.id); + var secondId = (string?)lastInside.Attribute(W.id); + if (firstInside.Name != endName || lastInside.Name != startName || after.Name != endName + || string.IsNullOrEmpty(firstId) || string.IsNullOrEmpty(secondId) + || (string?)firstInside.Attribute(W.id) != firstId + || (string?)after.Attribute(W.id) != secondId) + { + continue; + } + + // Both starts describe one wrapper revision and must carry a coherent stamp. + var author = AuthorOf(before); + var date = (string?)before.Attribute(W.date); + if (AuthorOf(lastInside) != author || (string?)lastInside.Attribute(W.date) != date) + continue; + + var family = isInsert + ? RevisionFamily.ContentControlInsert + : RevisionFamily.ContentControlDelete; + var unit = new RevisionUnit + { + Element = before, + Kind = UnitKind.StructuredRange, + Type = isInsert ? TypeInsert : TypeDelete, + Family = family, + Author = author, + Date = date, + Paragraph = sdt.AncestorsAndSelf(W.p).FirstOrDefault(), + MarkedCell = sdt.Ancestors(W.tc).FirstOrDefault(), + MarkedRow = sdt.Ancestors(W.tr).FirstOrDefault(), + Table = sdt.Ancestors(W.tbl).FirstOrDefault(), + StructuredWrapper = sdt, + Wid = WidOf(before), + NativeId = firstId, + }; + var group = NewGroup(unit, partIndex); + group.RangeMarkers.AddRange(new[] { before, firstInside, lastInside, after }); + groups.Add(group); + used.UnionWith(group.RangeMarkers); + } + + foreach (var markerGroup in allMarkers.Where(m => !used.Contains(m)) + .GroupBy(m => (Family: RangeFamily(m.Name), Id: (string?)m.Attribute(W.id)))) + { + var markers = markerGroup.ToList(); + var starts = markers.Where(m => m.Name.LocalName.EndsWith("RangeStart", StringComparison.Ordinal)).ToList(); + var family = markerGroup.Key.Family; + var exemplar = starts.FirstOrDefault() ?? markers[0]; + var unit = new RevisionUnit + { + Element = exemplar, + Kind = UnitKind.StructuredRange, + Type = family == RevisionFamily.ContentControlInsert ? TypeInsert : TypeDelete, + Family = family, + Author = AuthorOf(exemplar), + Date = (string?)exemplar.Attribute(W.date), + Paragraph = exemplar.Ancestors(W.p).FirstOrDefault(), + MarkedCell = exemplar.Ancestors(W.tc).FirstOrDefault(), + MarkedRow = exemplar.Ancestors(W.tr).FirstOrDefault(), + Table = exemplar.Ancestors(W.tbl).FirstOrDefault(), + Wid = WidOf(exemplar), + NativeId = (string?)exemplar.Attribute(W.id), + }; + var group = NewGroup(unit, partIndex); + group.RangeMarkers.AddRange(markers); + bool duplicate = markers.Count(m => m.Name.LocalName.EndsWith("RangeStart", StringComparison.Ordinal)) > 1 + || markers.Count(m => m.Name.LocalName.EndsWith("RangeEnd", StringComparison.Ordinal)) > 1; + group.ResolutionStatus = duplicate + ? RevisionResolutionStatus.Ambiguous + : RevisionResolutionStatus.Malformed; + group.Diagnostic = new RevisionDiagnostic( + duplicate ? "duplicate_range_id" : "malformed_range_topology", + duplicate + ? "Content-control revision range id is duplicated in its owning part." + : "Content-control revision ranges do not form Word's exact two-pair SDT envelope topology."); + groups.Add(group); + } + } + + private static RevisionFamily RangeFamily(XName name) => + name == W.customXmlInsRangeStart || name == W.customXmlInsRangeEnd + ? RevisionFamily.ContentControlInsert + : RevisionFamily.ContentControlDelete; + + private static void AddUnsupportedGroups(XElement root, int partIndex, List groups) + { + foreach (var markerGroup in root.Descendants() + .Where(e => UnsupportedRangeNames.Contains(e.Name)) + .GroupBy(e => ((string?)e.Attribute(W.id), e.Name.LocalName.Contains("MoveFrom", StringComparison.Ordinal)))) + { + var markers = markerGroup.ToList(); + var exemplar = markers.FirstOrDefault(m => m.Name.LocalName.EndsWith("RangeStart", StringComparison.Ordinal)) + ?? markers[0]; + var unit = new RevisionUnit + { + Element = exemplar, + Kind = UnitKind.Unsupported, + Type = TypeMove, + Family = RevisionFamily.Unsupported, + Author = AuthorOf(exemplar), + Date = (string?)exemplar.Attribute(W.date), + Paragraph = exemplar.Ancestors(W.p).FirstOrDefault(), + Table = exemplar.Ancestors(W.tbl).FirstOrDefault(), + Wid = WidOf(exemplar), + NativeId = (string?)exemplar.Attribute(W.id), + }; + var group = NewGroup(unit, partIndex); + group.RangeMarkers.AddRange(markers); + group.ResolutionStatus = RevisionResolutionStatus.Unsupported; + group.Diagnostic = new RevisionDiagnostic( + "unsupported_custom_xml_move_range", + "customXml move-range revisions are listed but cannot be selectively resolved."); + groups.Add(group); + } + } + + /// + /// Word records one cell-structure action as live cell marks plus associated table, + /// cell, paragraph-property, and content revisions. Fold that coherent stamp into + /// one atomic registry entry. Archived markers inside *PrChange payloads were never + /// walked, so they cannot be mistaken for live operations. + /// + private static void CoalesceTableStructureGroups(List groups, int partIndex) + { + var cellGroups = groups.Where(g => g.PartIndex == partIndex + && (g.Family == RevisionFamily.CellInsert + || g.Family == RevisionFamily.CellDelete + || g.Family == RevisionFamily.CellMerge)) + .ToList(); + + foreach (var byTable in cellGroups.GroupBy(g => g.Units[0].Table)) + { + if (byTable.Key is null) continue; + var tableCellGroups = byTable.ToList(); + var candidates = groups.Where(g => g.PartIndex == partIndex + && !tableCellGroups.Contains(g) + && g.Units.Count > 0 + && g.Units.All(u => ReferenceEquals(u.Table, byTable.Key))) + .ToList(); + + foreach (var candidate in candidates) + { + var matches = tableCellGroups.Where(c => + c.Author == candidate.Author && c.Date == candidate.Date).ToList(); + if (matches.Count == 0 && candidate.Units.All(u => u.Element.Name == W.tblGridChange) + && candidate.Author == "unknown" && candidate.Date is null) + { + matches = tableCellGroups; + } + + if (matches.Count == 1) + { + matches[0].Units.AddRange(candidate.Units); + matches[0].RangeMarkers.AddRange(candidate.RangeMarkers); + groups.Remove(candidate); + } + else if (matches.Count > 1) + { + candidate.ResolutionStatus = RevisionResolutionStatus.Ambiguous; + candidate.Diagnostic = new RevisionDiagnostic( + "ambiguous_table_structure_cluster", + "An unattributed table property revision matches multiple live cell operations."); + foreach (var match in matches) + { + match.ResolutionStatus = RevisionResolutionStatus.Ambiguous; + match.Diagnostic = candidate.Diagnostic; + } + } + } + } + } + + private static void AbsorbTrackedStructuredPayload(List groups, int partIndex) + { + foreach (var structured in groups.Where(g => g.PartIndex == partIndex + && (g.Family == RevisionFamily.ContentControlInsert + || g.Family == RevisionFamily.ContentControlDelete) + && g.ResolutionStatus == RevisionResolutionStatus.Supported).ToList()) + { + var wrapper = structured.Units[0].StructuredWrapper; + if (wrapper is null) continue; + var candidates = groups.Where(g => g != structured && g.PartIndex == partIndex + && g.Type == structured.Type && g.Author == structured.Author && g.Date == structured.Date + && g.Units.Count > 0 + && g.Units.All(u => u.Element.AncestorsAndSelf().Contains(wrapper))) + .ToList(); + foreach (var candidate in candidates) + { + structured.Units.AddRange(candidate.Units); + structured.RangeMarkers.AddRange(candidate.RangeMarkers); + groups.Remove(candidate); + } + } + } + private static bool AdjacentFormatRuns(XElement prevChange, XElement curChange) { var runA = prevChange.Parent?.Parent; @@ -533,25 +1061,96 @@ private static bool IsIgnorableBetween(XElement element) => private static void AssignIds(List groups) { - var seen = new HashSet(StringComparer.Ordinal); - int fallback = 0; - foreach (var g in groups) + groups.Sort((a, b) => + { + int part = a.PartIndex.CompareTo(b.PartIndex); + if (part != 0) return part; + var ae = a.Units.FirstOrDefault()?.Element ?? a.RangeMarkers.FirstOrDefault(); + var be = b.Units.FirstOrDefault()?.Element ?? b.RangeMarkers.FirstOrDefault(); + if (ae is null || be is null) return ae is null ? (be is null ? 0 : 1) : -1; + return XNode.DocumentOrderComparer.Compare(ae, be); + }); + + var identityMaterial = groups.ToDictionary(g => g, g => + { + var constituents = ConstituentKeys(g); + return g.PartUri + "\n" + g.Family + "\n" + string.Join("\n", constituents); + }); + + foreach (var candidate in groups.GroupBy(g => StableId(identityMaterial[g]))) { - long? min = null; - foreach (var u in g.Units) - if (u.Wid is { } w && (min is null || w < min)) min = w; - foreach (var m in g.RangeMarkers) - if (WidOf(m) is { } w && (min is null || w < min)) min = w; - var baseId = min is { } mv - ? "rev" + mv.ToString(System.Globalization.CultureInfo.InvariantCulture) - : "revu" + fallback++; - var id = baseId; - int suffix = 2; - while (!seen.Add(id)) id = baseId + "-" + suffix++; - g.Id = id; + if (candidate.Count() == 1) + { + candidate.First().Id = candidate.Key; + continue; + } + + // Invalid documents can reuse one native id for independent live operations. + // They remain fail-closed/ambiguous, but list ids must still be unique so a + // transport cannot silently overwrite one entry in an id-keyed map. The + // collision ordinal is stable under resolution of unrelated revisions. + int ordinal = 0; + foreach (var group in candidate) + group.Id = StableId(identityMaterial[group] + "\ncollision:" + ordinal++); } } + private static string StableId(string material) + { + var digest = SHA256.HashData(Encoding.UTF8.GetBytes(material)); + // Opaque, part-qualified, deterministic identity. Twenty hex characters + // provide 80 bits while keeping transport payloads compact. + return "rev2-" + Convert.ToHexStringLower(digest.AsSpan(0, 10)); + } + + internal static IReadOnlyList ConstituentIds(RevisionGroup group) => + group.Units.Select(u => u.NativeId) + .Concat(group.RangeMarkers.Select(m => (string?)m.Attribute(W.id))) + .Where(id => !string.IsNullOrEmpty(id)) + .Select(id => id!) + .Distinct(StringComparer.Ordinal) + .OrderBy(id => long.TryParse(id, out var n) ? n : long.MaxValue) + .ThenBy(id => id, StringComparer.Ordinal) + .ToList(); + + internal static string? LegacyId(RevisionGroup group) + { + var ids = ConstituentIds(group); + var numeric = ids.Select(id => long.TryParse(id, out var value) ? value : (long?)null) + .Where(value => value.HasValue) + .Select(value => value!.Value) + .DefaultIfEmpty() + .Min(); + return ids.Any(id => long.TryParse(id, out _)) + ? "rev" + numeric.ToString(System.Globalization.CultureInfo.InvariantCulture) + : null; + } + + private static IReadOnlyList ConstituentKeys(RevisionGroup group) + { + var keys = group.Units.Select(u => + u.Element.Name.NamespaceName + ":" + u.Element.Name.LocalName + ":" + + (u.NativeId ?? ElementPath(u.Element))) + .Concat(group.RangeMarkers.Select(m => + m.Name.NamespaceName + ":" + m.Name.LocalName + ":" + + ((string?)m.Attribute(W.id) ?? ElementPath(m)))) + .Distinct(StringComparer.Ordinal) + .OrderBy(k => k, StringComparer.Ordinal) + .ToList(); + return keys.Count > 0 ? keys : new[] { "empty" }; + } + + private static string ElementPath(XElement element) + { + var segments = new Stack(); + for (var current = element; current is not null; current = current.Parent) + { + int index = current.ElementsBeforeSelf(current.Name).Count(); + segments.Push(current.Name.LocalName + "[" + index + "]"); + } + return string.Join("/", segments); + } + // ─── Listing text ─────────────────────────────────────────────────── internal static string GroupText(RevisionGroup g) @@ -579,6 +1178,19 @@ internal static string GroupText(RevisionGroup g) && u.Element.Parent?.Parent is { } para && para.Name == W.p) AppendVisibleText(para, W.t, sb); break; + case UnitKind.CellMark: + if (u.MarkedCell is { } cell) + AppendVisibleText(cell, u.Family == RevisionFamily.CellDelete ? W.delText : W.t, sb); + break; + case UnitKind.NumberingPropertiesInsert: + case UnitKind.NumberingChange: + if (u.Paragraph is { } numberedParagraph) + AppendVisibleText(numberedParagraph, W.t, sb); + break; + case UnitKind.StructuredRange: + if (u.StructuredWrapper is { } wrapper) + AppendVisibleText(wrapper, W.t, sb); + break; } } return sb.ToString(); @@ -608,6 +1220,10 @@ private static void AppendVisibleText(XElement el, XName textName, StringBuilder /// internal static List Apply(RevisionGroup g, bool accept) { + if (g.ResolutionStatus != RevisionResolutionStatus.Supported) + throw new InvalidOperationException(g.Diagnostic?.Message + ?? "revision cannot be resolved safely"); + var removedBlocks = new List(); var touchedParagraphs = new HashSet(); @@ -644,6 +1260,21 @@ internal static List Apply(RevisionGroup g, bool accept) else RemoveRow(u.MarkedRow!, removedBlocks); } + foreach (var u in g.Units.Where(u => u.Kind == UnitKind.NumberingPropertiesInsert)) + { + if (Detached(u.Element)) continue; + var numPr = u.Element.Parent; + if (accept) + u.Element.Remove(); + else if (numPr?.Name == W.numPr) + numPr.Remove(); + } + + foreach (var u in g.Units.Where(u => u.Kind == UnitKind.NumberingChange)) + if (!Detached(u.Element)) u.Element.Remove(); + + ResolveCellStructure(g, accept, removedBlocks); + // Paragraph marks last, in reverse document order, so multi-paragraph coalescing // cascades into the single surviving paragraph exactly as RevisionProcessor's // grouped transform does. @@ -672,6 +1303,8 @@ internal static List Apply(RevisionGroup g, bool accept) } } + ResolveStructuredWrapper(g, accept, removedBlocks); + foreach (var m in g.RangeMarkers) if (!Detached(m)) m.Remove(); @@ -684,6 +1317,196 @@ internal static List Apply(RevisionGroup g, bool accept) return removedBlocks; } + private static void ResolveCellStructure( + RevisionGroup group, bool accept, List removedElements) + { + var cellUnits = group.Units.Where(u => u.Kind == UnitKind.CellMark).ToList(); + if (cellUnits.Count == 0) return; + + if (group.Family == RevisionFamily.CellInsert) + { + if (accept) + { + foreach (var unit in cellUnits) + if (!Detached(unit.Element)) unit.Element.Remove(); + } + else + { + foreach (var cell in cellUnits.Select(u => u.MarkedCell) + .Where(c => c is not null).Select(c => c!).Distinct().ToList()) + { + if (Detached(cell)) continue; + cell.Remove(); + removedElements.Add(cell); + } + } + } + else if (group.Family == RevisionFamily.CellDelete) + { + if (accept) + AcceptDeletedCells(cellUnits, removedElements); + else + foreach (var unit in cellUnits) + if (!Detached(unit.Element)) unit.Element.Remove(); + } + else if (group.Family == RevisionFamily.CellMerge) + { + foreach (var unit in cellUnits) + { + if (Detached(unit.Element)) continue; + if (accept) + { + var revised = (string?)unit.Element.Attribute(W.vMerge); + if (revised == "rest") + unit.Element.ReplaceWith(new XElement(W.vMerge, + new XAttribute(W.val, "restart"))); + else if (revised == "cont") + unit.Element.ReplaceWith(new XElement(W.vMerge, + new XAttribute(W.val, "continue"))); + else + unit.Element.Remove(); + } + else + { + var original = (string?)unit.Element.Attribute(W.vMergeOrig); + if (original == "rest") + unit.Element.ReplaceWith(new XElement(W.vMerge, + new XAttribute(W.val, "restart"))); + else if (original == "cont") + unit.Element.ReplaceWith(new XElement(W.vMerge, + new XAttribute(W.val, "continue"))); + else + unit.Element.Remove(); + } + } + } + + // Rejecting associated tcPrChange revisions can expose archived structural + // marks from the old property shell. They belong to the same operation but are + // not live revisions; remove only marks carrying this exact operation stamp. + if (!accept) + { + var structuralName = group.Family == RevisionFamily.CellInsert ? W.cellIns + : group.Family == RevisionFamily.CellDelete ? W.cellDel + : W.cellMerge; + foreach (var table in cellUnits.Select(u => u.Table).Where(t => t is not null) + .Select(t => t!).Distinct()) + { + foreach (var marker in table.Descendants() + .Where(e => e.Name == structuralName) + .Where(e => AuthorOf(e) == group.Author + && (string?)e.Attribute(W.date) == group.Date).ToList()) + marker.Remove(); + } + } + } + + /// + /// Accept cell deletion using grid units, not physical-cell count. Consecutive + /// deleted cells contribute the sum of their pre-existing gridSpan values to the + /// nearest surviving predecessor. + /// + private static void AcceptDeletedCells( + IReadOnlyList units, List removedElements) + { + var deleted = units.Select(u => u.MarkedCell).Where(c => c is not null) + .Select(c => c!).ToHashSet(); + foreach (var row in deleted.Select(c => c.Parent).Where(r => r is not null) + .Select(r => r!).Distinct().ToList()) + { + var cells = row.Elements(W.tc).ToList(); + XElement? predecessor = null; + int pendingSpan = 0; + foreach (var cell in cells) + { + if (deleted.Contains(cell)) + { + pendingSpan += CellGridSpan(cell); + cell.Remove(); + removedElements.Add(cell); + continue; + } + + if (pendingSpan > 0) + { + if (predecessor is null) + throw new InvalidOperationException( + "A deleted-cell run has no surviving predecessor to absorb its grid columns."); + SetCellGridSpan(predecessor, CellGridSpan(predecessor) + pendingSpan); + pendingSpan = 0; + } + predecessor = cell; + } + if (pendingSpan > 0) + { + if (predecessor is null) + throw new InvalidOperationException( + "Resolving the cell deletion would remove every cell in a row."); + SetCellGridSpan(predecessor, CellGridSpan(predecessor) + pendingSpan); + } + } + } + + private static int CellGridSpan(XElement cell) => + Math.Max(1, (int?)cell.Element(W.tcPr)?.Element(W.gridSpan)?.Attribute(W.val) ?? 1); + + private static void SetCellGridSpan(XElement cell, int span) + { + var tcPr = cell.Element(W.tcPr); + if (tcPr is null) + { + tcPr = new XElement(W.tcPr); + cell.AddFirst(tcPr); + } + var gridSpan = tcPr.Element(W.gridSpan); + if (span <= 1) + { + gridSpan?.Remove(); + return; + } + if (gridSpan is null) + { + gridSpan = new XElement(W.gridSpan, new XAttribute(W.val, span)); + var before = tcPr.Elements().FirstOrDefault(e => + e.Name == W.hMerge || e.Name == W.vMerge || e.Name == W.tcBorders + || e.Name == W.shd || e.Name == W.noWrap || e.Name == W.tcMar + || e.Name == W.textDirection || e.Name == W.tcFitText + || e.Name == W.vAlign || e.Name == W.hideMark + || e.Name == W.cellIns || e.Name == W.cellDel || e.Name == W.cellMerge + || e.Name == W.tcPrChange); + if (before is null) tcPr.Add(gridSpan); + else before.AddBeforeSelf(gridSpan); + } + else + { + gridSpan.SetAttributeValue(W.val, span); + } + } + + private static void ResolveStructuredWrapper( + RevisionGroup group, bool accept, List removedElements) + { + if (group.Family != RevisionFamily.ContentControlInsert + && group.Family != RevisionFamily.ContentControlDelete) + return; + + var wrapper = group.Units.FirstOrDefault(u => u.Kind == UnitKind.StructuredRange) + ?.StructuredWrapper; + if (wrapper is null || Detached(wrapper)) return; + + bool wrapperSurvives = group.Family == RevisionFamily.ContentControlInsert + ? accept + : !accept; + if (wrapperSurvives) return; + + var content = wrapper.Element(W.sdtContent); + var nodes = content?.Nodes().Where(n => n is not XElement e + || !StructuredRangeNames.Contains(e.Name)).ToList() ?? new List(); + foreach (var node in nodes) node.Remove(); + wrapper.ReplaceWith(nodes); + removedElements.Add(wrapper); + } + /// /// A comment wholly contained by a row would otherwise lose its range/reference when /// selective resolution removes that row. Move complete marker triples to the nearest @@ -826,10 +1649,13 @@ private static void CleanParagraphHusks(XElement p) var pPr = p.Element(W.pPr); if (pPr is null) return; var rPr = pPr.Element(W.rPr); - if (rPr is not null && !rPr.HasElements && !rPr.HasAttributes) rPr.Remove(); - if (!pPr.HasElements && !pPr.HasAttributes) pPr.Remove(); + if (rPr is not null && !rPr.HasElements && HasNoSemanticAttributes(rPr)) rPr.Remove(); + if (!pPr.HasElements && HasNoSemanticAttributes(pPr)) pPr.Remove(); } + private static bool HasNoSemanticAttributes(XElement element) => + element.Attributes().All(a => a.IsNamespaceDeclaration || a.Name == PtOpenXml.Unid); + // ─── Format-change resolution ─────────────────────────────────────── private static void AcceptProps(XElement change) diff --git a/Docxodus/Internal/RevisionRegistry.cs b/Docxodus/Internal/RevisionRegistry.cs new file mode 100644 index 00000000..1453ecd0 --- /dev/null +++ b/Docxodus/Internal/RevisionRegistry.cs @@ -0,0 +1,102 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#nullable enable + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Xml.Linq; + +namespace Docxodus.Internal; + +/// +/// A live, part-aware registry of every tracked revision the session can see. The +/// registry is intentionally rebuilt after each resolution: rejecting a property +/// change can expose older revision markup from its archived property shell, while +/// resolving a structural parent can detach nested revisions. +/// +internal sealed class RevisionRegistry +{ + internal sealed record Part(string PartUri, string Scope, XElement Root); + + private readonly IReadOnlyList _parts; + + private RevisionRegistry(IReadOnlyList parts, List entries) + { + _parts = parts; + Entries = entries; + } + + internal IReadOnlyList Entries { get; } + + internal static RevisionRegistry Build(IReadOnlyList parts) => + new(parts, RevisionOps.Enumerate(parts + .Select(p => (p.PartUri, p.Scope, p.Root)).ToList())); + + internal RevisionOps.RevisionGroup? Find(string id) + { + var exact = Entries.FirstOrDefault(entry => entry.Id == id); + if (exact is not null) return exact; + + // Backward-compatible input only: legacy revNNN ids are accepted when they + // identify exactly one current group. Listings always return the stable rev2 id. + var legacy = Entries.Where(entry => RevisionOps.LegacyId(entry) == id).ToList(); + return legacy.Count == 1 ? legacy[0] : null; + } + + internal static RevisionDiagnostic? ResolutionDiagnostic(RevisionOps.RevisionGroup group) => + group.ResolutionStatus == RevisionResolutionStatus.Supported + ? null + : group.Diagnostic ?? new RevisionDiagnostic( + "unresolved_revision", + "The revision cannot be resolved safely."); + + internal List Resolve(RevisionOps.RevisionGroup group, bool accept) => + RevisionOps.Apply(group, accept); + + /// + /// Resolve every currently live revision through the same selective resolver used + /// by individual operations. Rebuild after every group so newly exposed archived + /// revisions are handled and detached nested groups disappear naturally. + /// + internal List ResolveAll(bool accept) + { + var removed = new List(); + var registry = this; + var attemptedElements = new HashSet(); + for (int guard = 0; guard < 100_000; guard++) + { + if (registry.Entries.Count == 0) return removed; + + var blocked = registry.Entries.FirstOrDefault(entry => + entry.ResolutionStatus != RevisionResolutionStatus.Supported); + if (blocked is not null) + throw new RevisionResolutionException(blocked); + + var next = registry.Entries[0]; + var progressElement = next.Units.FirstOrDefault()?.Element + ?? next.RangeMarkers.FirstOrDefault(); + if (progressElement is null || !attemptedElements.Add(progressElement)) + throw new InvalidOperationException( + "Bulk revision resolution made no progress; the document was left unchanged."); + + removed.AddRange(registry.Resolve(next, accept)); + registry = Build(_parts); + } + + throw new InvalidOperationException( + "Bulk revision resolution exceeded its safety limit; the document was left unchanged."); + } +} + +internal sealed class RevisionResolutionException : InvalidOperationException +{ + internal RevisionResolutionException(RevisionOps.RevisionGroup group) + : base(group.Diagnostic?.Message ?? "revision cannot be resolved safely") + { + Group = group; + } + + internal RevisionOps.RevisionGroup Group { get; } +} diff --git a/Docxodus/RevisionProcessor.cs b/Docxodus/RevisionProcessor.cs index 23398f79..687eefb2 100644 --- a/Docxodus/RevisionProcessor.cs +++ b/Docxodus/RevisionProcessor.cs @@ -3090,15 +3090,12 @@ private static object AcceptDeletedCellsTransform(XNode node) return null; if (g.Key.CollectionType == DeletedCellCollectionType.Other) return (object)g; - XElement gridSpanElement = g - .First() - .Elements(W.tcPr) - .Elements(W.gridSpan) - .FirstOrDefault(); - int gridSpan = gridSpanElement != null ? - (int)gridSpanElement.Attribute(W.val) : - 1; - int newGridSpan = gridSpan + g.Count() - 1; + // GridSpan counts logical grid columns, not physical cells. + // A deleted cell can itself span several columns, so widening + // by g.Count()-1 corrupts tables with pre-existing spans. + int newGridSpan = g.Where(e => e.Name == W.tc).Sum(tc => + (int?)tc.Elements(W.tcPr).Elements(W.gridSpan) + .Attributes(W.val).FirstOrDefault() ?? 1); XElement currentTcPr = g.First().Elements(W.tcPr).FirstOrDefault(); // The absorbing cell may have NO tcPr at all (minimal cells) — synthesize one // carrying just the widened gridSpan instead of dereferencing null. diff --git a/docs/architecture/docx_agent_server.md b/docs/architecture/docx_agent_server.md index 914efd01..17158d86 100644 --- a/docs/architecture/docx_agent_server.md +++ b/docs/architecture/docx_agent_server.md @@ -59,14 +59,10 @@ Docxodus.DocxSession (the real work — see docs/architecture/docx_mutation_ap ``` `SessionStore` is the one piece of state this server owns that `DocxSessionOps` doesn't: a -string `session_id` → `{ handle, location, settings }` map. It exists for two reasons: - -1. `docxodus_save` needs to remember the location a session was opened from so "save" can mean - "write back to the same document" without the caller repeating it. -2. `docxodus_track_changes`'s `accept_all`/`reject_all` need to swap a session's entire - underlying document for a whole-document byte transform (`RevisionProcessor.AcceptRevisions`/ - `RejectRevisions`, which operate on bytes, not a live session) while the caller keeps - addressing the same `session_id` — see `SessionStore.Rebind`. +string `session_id` → `{ handle, location, settings }` map. The external protocol uses that +unguessable id as its document capability, while `docxodus_save` uses the remembered location +to write back without requiring the caller to repeat the path. Tracked-revision resolution now +mutates the live session through `DocxSessionOps`; it does not rebind a whole-document transform. Session ids are 16 random bytes, not a counter. The id **is** the capability — holding one is what lets a caller act on that document — so a guessable id would let anything able to make tool @@ -434,28 +430,25 @@ interactive edits. This setting is distinct from display: `docxodus_get_content( always renders pending markup as ``/``; accepting or rejecting it requires an explicit track-changes action. -`list` (issue #318) reads the revision set directly off the live session's markup — -`DocxSession.ListRevisions` enumerates `w:ins`/`w:del`/`w:moveFrom`/`w:moveTo`, paragraph-mark -and table-row markers, and the `*PrChange` format-change family across body, headers, footers, -footnotes, and endnotes, grouping physically contiguous same-kind/same-author markup into one -entry per user-visible change. Each entry carries a stable `id` (derived from the markup's own -`w:id` attributes, so resolving other revisions never renames it), `type` -(`insert`/`delete`/`move`/`format` — a `move` is a linked pair covering both sides), the -markup's true `author`/`date`, its visible `text`, and the containing block's `anchorId`. +`list` (issues #318 and #455) reads the live, part-aware revision registry. It includes +content, paragraph/row/property changes, named moves, cell insert/delete/merge operations, +content-control envelopes, and numbering-property revisions across body, headers, footers, +footnotes, and endnotes. Each entry carries an opaque stable `rev2-…` id, coarse `type`, exact +`family`, native `constituentIds`, author/date/text, owning `partUri` and canonical `scope`, all +affected anchors, and a fail-closed `resolutionStatus` plus diagnostic when needed. This replaced the original listing, which re-diffed `RevisionProcessor.RejectRevisions` vs `.AcceptRevisions` output through `DocxDiffOps.GetRevisionsJson` — that shape had no stable identity to address, substituted engine-default authors/dates for the markup's real ones, and -cost ~3s on a 49-page document. `author`/`changeType` are display-only filters applied after -the fact. +cost ~3s on a 49-page document. `author`, `changeType`, `family`, `resolutionStatus`, and +`partUri` are display-only filters applied after the fact. `accept`/`reject` resolve ONE revision by `revisionId` as an ordinary undoable session mutation (`DocxSession.AcceptRevision`/`RejectRevision` — no whole-document `RevisionProcessor` round-trip, no session rebind, anchors stay live), returning the standard EditResult envelope with the affected blocks in `modified`/`removed`. An unknown or -already-resolved id fails with `revision_not_found`. `accept_all`/`reject_all` remain for -whole-document resolution: they transform via `RevisionProcessor` and swap the session's -underlying handle in place (`SessionStore.Rebind`), which also covers the exotic families the -per-revision listing does not enumerate (see Known gaps). +already-resolved id fails with `revision_not_found`. Unsafe registry entries fail with a typed +unsupported/malformed/ambiguous error. `accept_all`/`reject_all` use that same resolver, +rebuilding the live registry after each entry; the complete operation is atomic and undoable. ### `docxodus_mutations` — atomic batches, explicit partial apply, or isolated preview @@ -594,12 +587,10 @@ Capabilities a full-featured document-editing agent surface might have, that Doc doesn't yet support — called out explicitly rather than faked, per this server's design goal of never claiming a capability it doesn't have: -- **Exotic revision families aren't individually resolvable.** Issue #318 closed the - selective-resolution gap for the common families — `docxodus_track_changes` `accept`/`reject` - resolve one insert/delete/move/format revision by `revisionId` — but - `w:cellIns`/`w:cellDel`/`w:cellMerge`, content-control ins/del ranges, and `w:numPr` - numbering-ins markers are not enumerated by `list` and have no per-revision resolution; - `accept_all`/`reject_all` (whole-document `RevisionProcessor`) still handle them. +- **Unsafe revision topology fails closed.** The live registry resolves the supported content, + property, table-cell, content-control, and numbering families. A recognized family with no + safe resolver, or malformed/ambiguous native topology, remains visible with a diagnostic and + blocks both selective and bulk resolution until the source document is repaired. - **New lists inserted via a bare markdown payload don't get real Word numbering.** A `"- item"` block parses to a `kind: "li"` anchor with no `w:numPr` (documented in `docx_mutation_api.md`). This server's `docxodus_list`/`docxodus_create` route around it by diff --git a/docs/architecture/docx_mutation_api.md b/docs/architecture/docx_mutation_api.md index 60b64395..24023008 100644 --- a/docs/architecture/docx_mutation_api.md +++ b/docs/architecture/docx_mutation_api.md @@ -227,6 +227,13 @@ Two conventions worth pinning down because they affect agent reasoning: **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. +Structural tracking is deliberately capability-gated. Row insertion/deletion and column +insertion emit native Word row/cell/property revisions; single-paragraph list application, +removal, and level changes emit `numPr/w:ins` or `pPrChange`. Shapes without a safely reversible +encoding—tracked column deletion, merge/unmerge, range list formatting, and list-start +overrides—return `TrackedOperationUnsupported` without mutation or history. A table with a live +cell structural revision returns `UnresolvedStructuralRevision` before another structural edit. + **`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`. ## When to use what @@ -1143,26 +1150,25 @@ Semantics: stdio host + docx-scalpel (`set_tracked_changes`/`set_revision_author`), MCP (`docxodus_track_changes` action `set_mode`). -## Per-revision accept/reject (issue #318) +## Tracked-revision registry and resolution (issues #318, #455) Tracked-change resolution used to be all-or-nothing (`RevisionProcessor` over the whole document); the single most common review action — accept one revision, reject another — -required emulation. Three session ops close that gap: +required emulation. The session now exposes one live, part-aware registry and uses it for +individual and bulk resolution: | Method | Description | |--------|-------------| -| `ListRevisions()` | Read-only: `RevisionListEntry(Id, Type, Author, Date?, Text, AnchorId?)` in document order across body, headers, footers, footnotes, endnotes. Read directly off the live markup — no accept-all/reject-all re-diff — so `Author`/`Date` are the markup's true `w:author`/`w:date` and the call is cheap on large documents. Physically contiguous markup of the same kind+author groups into ONE entry per user-visible change: an inserted paragraph is one revision (runs + mark, `Text` ends in `¶`), a row-deleted table row absorbs its cell markup, a named move pair is one `"move"` entry covering both sides. `Id` (`"rev"` + the group's smallest `w:id`) is stable across calls and across resolution of *other* revisions. `Type` is `"insert"`/`"delete"`/`"move"`/`"format"`. | +| `ListRevisions()` | Read-only entries in document order across body, headers, footers, footnotes, and endnotes. Each carries an opaque stable `Id` (`rev2-…`), coarse `Type`, exact `Family`, native `ConstituentIds`, owning `PartUri`/canonical `Scope`, primary `AnchorId`, every `AffectedAnchor`, and a `ResolutionStatus` plus optional diagnostic. Authors/dates come from the live markup. Atomic entries include content and paragraph/row/property changes, named moves, cell insert/delete/merge operations, content-control envelopes, and numbering-property revisions. Unsupported, malformed, and ambiguous markup stays visible and fails closed. Legacy `revNNN` ids are accepted only as unambiguous inputs and are never emitted. | | `AcceptRevision(id)` | Resolve ONE revision, keeping the change: unwrap `w:ins`/`w:moveTo`, carry out `w:del`/`w:moveFrom` (paragraph-mark deletions coalesce into the following paragraph, row deletions drop the row — the last row drops the table), drop the `*PrChange` element keeping current properties. An ordinary undoable mutation returning the `EditResult` envelope (`Modified` = touched blocks, `Removed` = blocks the resolution deleted). | | `RejectRevision(id)` | The inverse: remove insertions, restore deletions (`w:delText` → `w:t`, marks stripped), keep a move at its source, restore a format change's stored old properties (preserving the children the `CT_*Base` inner schema excludes — mark revisions on a paragraph-mark `rPr`, header/footer references on `sectPr`, `rPr`/`sectPr` on `pPr`). | +| `AcceptAllRevisions()` / `RejectAllRevisions()` | Resolve the complete live registry through the same selective resolver as one atomic undo step. The registry is rebuilt after every entry so resolving a property shell can expose older archived revisions safely. Any unsupported, malformed, or ambiguous entry rolls back the whole operation. | -Mechanics live in `Docxodus/Internal/RevisionOps.cs`; the per-element semantics mirror -`RevisionProcessor`'s transforms, applied to one group in place. An unknown, already-resolved, -or since-removed id fails with `RevisionNotFound` — re-list for the current set. v1 does not -enumerate `w:cellIns`/`w:cellDel`/`w:cellMerge`, content-control ins/del ranges, or -`w:numPr` numbering-ins markers; whole-document accept/reject still handles those. Wired -through every surface: WASM/npm (`listRevisions`/`acceptRevision`/`rejectRevision`), stdio -host + docx-scalpel (`list_revisions`/`accept_revision`/`reject_revision`), MCP -(`docxodus_track_changes` actions `list`/`accept`/`reject` with `revisionId`). +Mechanics live in `Docxodus/Internal/RevisionRegistry.cs` and `RevisionOps.cs`; the +per-element semantics mirror `RevisionProcessor`'s transforms, applied to one atomic group in +place. An unknown, already-resolved, or since-removed id fails with `RevisionNotFound`; unsafe +entries use `RevisionUnsupported`, `RevisionMalformed`, or `RevisionAmbiguous`. Wired through +every surface: WASM/npm, the stdio host + docx-scalpel, and MCP `docxodus_track_changes`. The same ids can be passed to `AddCommentToRevision` (or `docxodus_comment add` with `revisionId`) to anchor review discussion to the exact live change before it is resolved. @@ -1695,9 +1701,10 @@ Errors are grouped by what the agent should do in response, not by where in the | Re-project and re-derive the anchor from current text | `AnchorNotFound` | | Re-list revisions (`ListRevisions`) and reissue with a current id | `RevisionNotFound` | | Re-list native objects and reissue with a current id/name | `HyperlinkNotFound`, `BookmarkNotFound` | +| Inspect the revision diagnostic and repair/reopen the source document | `RevisionUnsupported`, `RevisionMalformed`, `RevisionAmbiguous` | | Re-read the anchor's kind via `GetAnchorInfo`, reissue with the right op or coordinates | `AnchorWrongKind`, `TableAnchorMigrationRequired`, `AnchorsNotAdjacent`, `InvalidPosition`, `OffsetOutOfRange`, `EmptyCommentSpan`, `EmptyHyperlinkSpan` | | Fix the target/name or resolve the existing reference first | `DuplicateBookmarkName`, `InvalidBookmarkName`, `InvalidHyperlinkTarget`, `MissingBookmarkTarget`, `BookmarkInUse`, `ManagedBookmark` | -| Choose a safe run/range boundary or switch subsequent edits out of tracked mode | `UnsupportedInlineBoundary`, `TrackedOperationUnsupported` | +| Choose a safe run/range boundary, resolve the pending structural revision, or switch subsequent edits out of tracked mode | `UnsupportedInlineBoundary`, `UnresolvedStructuralRevision`, `TrackedOperationUnsupported` | | Fix the markdown payload (the message names what's wrong) | `MalformedMarkdown`, `UnsupportedMarkdownSyntax`, `AnchorTokenInPayload` | | Call the v1 op the message names, or fall back to `Raw.InsertXml` | `TableInsertNotSupported`, `FootnoteRefNotSupported`, `CommentMarkerNotSupported`, `ImageInsertNotSupported` | | Re-query `ListStyles()` for a current style id, or `GetListMembership()` for the valid numbering level | `UnknownStyle`, `InvalidListLevel` | diff --git a/npm/src/index.ts b/npm/src/index.ts index 491b872d..cff151a2 100644 --- a/npm/src/index.ts +++ b/npm/src/index.ts @@ -118,6 +118,9 @@ export type { ParagraphBorderEdge, ParagraphFormatOp, RevisionListEntry, + RevisionDiagnostic, + RevisionFamily, + RevisionResolutionStatus, SectionInfo, SessionRevisionType, TableBorderScope, diff --git a/npm/src/session.ts b/npm/src/session.ts index addd6e60..a08a353f 100644 --- a/npm/src/session.ts +++ b/npm/src/session.ts @@ -1081,6 +1081,16 @@ export class DocxSession { return JSON.parse(this.wasm.RejectRevision(this.handle, revisionId)) as EditResult; } + /** Accept every supported live revision as one undoable session mutation. */ + acceptAllRevisions(): EditResult { + return JSON.parse(this.wasm.AcceptAllRevisions(this.handle)) as EditResult; + } + + /** Reject every supported live revision as one undoable session mutation. */ + rejectAllRevisions(): EditResult { + return JSON.parse(this.wasm.RejectAllRevisions(this.handle)) as EditResult; + } + // ─── Tier C: formatting ────────────────────────────────────────────── applyFormat(anchorId: string, span: CharSpan | null, op: FormatOp): EditResult { diff --git a/npm/src/types.ts b/npm/src/types.ts index bf046727..19016124 100644 --- a/npm/src/types.ts +++ b/npm/src/types.ts @@ -1214,6 +1214,8 @@ export interface DocxodusWasmExports { ListRevisions: (handle: number) => string; AcceptRevision: (handle: number, revisionId: string) => string; RejectRevision: (handle: number, revisionId: string) => string; + AcceptAllRevisions: (handle: number) => string; + RejectAllRevisions: (handle: number) => string; ApplyFormat: (handle: number, anchor: string, spanJson: string, opJson: string) => string; ApplyFormatBySubstring: (handle: number, anchor: string, substring: string, opJson: string) => string; SetParagraphStyle: (handle: number, anchor: string, styleId: string) => string; @@ -1353,7 +1355,11 @@ export type EditErrorCode = | "managed_bookmark" | "empty_hyperlink_span" | "unsupported_inline_boundary" + | "revision_unsupported" + | "revision_malformed" + | "revision_ambiguous" | "tracked_operation_unsupported" + | "unresolved_structural_revision" | "internal_error"; export interface AnchorRef { @@ -1642,24 +1648,40 @@ export interface CommentListEntry { /** Revision kind in a markup-native revision listing. A `move` entry is a linked * move pair — both sides resolve together. */ -export type SessionRevisionType = "insert" | "delete" | "move" | "format"; +export type SessionRevisionType = "insert" | "delete" | "move" | "format" | "structure"; +export type RevisionFamily = + | "content_insert" | "content_delete" | "move" | "paragraph_mark" + | "row_insert" | "row_delete" | "cell_insert" | "cell_delete" | "cell_merge" + | "content_control_insert" | "content_control_delete" + | "numbering_properties_insert" | "numbering_change" | "properties_change" + | "unsupported"; +export type RevisionResolutionStatus = "supported" | "unsupported" | "malformed" | "ambiguous"; + +export interface RevisionDiagnostic { + code: string; + message: string; +} /** - * One tracked revision read directly off the live markup, in document order — see - * {@link DocxSession.listRevisions}. `id` is stable while the underlying markup exists - * (derived from the markup's own `w:id` attributes — resolving OTHER revisions never - * renames it) and is what acceptRevision/rejectRevision address. `author`/`date` are - * the markup's true `w:author`/`w:date`. `text` is the revision's visible text (deleted - * text for deletions, `¶` for a revised paragraph mark, the affected text for format - * changes). `anchorId` is the containing block's anchor when addressable. + * One part-qualified atomic revision from the live registry. `id` is an opaque, + * deterministic `rev2-…` identity; `constituentIds` exposes the native Word ids. + * `family` identifies the exact operation, while `type` is its coarse display class. + * Unsafe native topology remains listed through `resolutionStatus` and `diagnostic`. */ export interface RevisionListEntry { id: string; type: SessionRevisionType; + family: RevisionFamily; + constituentIds: string[]; author: string; date?: string; text: string; + partUri: string; + scope: string; anchorId?: string; + affectedAnchors: AnchorRef[]; + resolutionStatus: RevisionResolutionStatus; + diagnostic?: RevisionDiagnostic; } export interface CharSpan { diff --git a/npm/tests/docx-session-revisions.spec.ts b/npm/tests/docx-session-revisions.spec.ts index 4f64ecf5..101b04a1 100644 --- a/npm/tests/docx-session-revisions.spec.ts +++ b/npm/tests/docx-session-revisions.spec.ts @@ -15,7 +15,7 @@ async function waitForDocxodus(page: Page) { await page.waitForFunction(() => (window as any).DocxodusReady === true, { timeout: 30000 }); } -// Issue #318 — markup-native revision listing + selective per-revision accept/reject. +// Issues #318/#455 — live revision registry + selective and bulk resolution. test.describe('DocxSession revision review (WASM bridge)', () => { test.beforeEach(async ({ page }) => { await page.goto('/test-harness.html'); @@ -57,7 +57,11 @@ test.describe('DocxSession revision review (WASM bridge)', () => { insertAuthor: insertRev?.author, insertText: insertRev?.text, insertHasAnchor: typeof insertRev?.anchorId === 'string', - idsStartWithRev: listed.every((r) => typeof r.id === 'string' && r.id.startsWith('rev')), + richRegistry: listed.every((r) => + typeof r.id === 'string' && r.id.startsWith('rev2-') && + typeof r.family === 'string' && Array.isArray(r.constituentIds) && + r.partUri === '/word/document.xml' && r.scope === 'body' && + Array.isArray(r.affectedAnchors) && r.resolutionStatus === 'supported'), acceptOk: accepted.success, remainingIds: remaining.map((r: any) => r.id), deleteId: deleteRev?.id, @@ -76,7 +80,7 @@ test.describe('DocxSession revision review (WASM bridge)', () => { expect(result.insertAuthor).toBe('Spec Reviewer'); expect(result.insertText).toBe('Tracked rewrite.'); expect(result.insertHasAnchor).toBe(true); - expect(result.idsStartWithRev).toBe(true); + expect(result.richRegistry).toBe(true); expect(result.acceptOk).toBe(true); // Resolving one revision leaves the other's id untouched. expect(result.remainingIds).toEqual([result.deleteId]); diff --git a/python/src/docx_scalpel/__init__.py b/python/src/docx_scalpel/__init__.py index 0dfbbf70..6dc88274 100644 --- a/python/src/docx_scalpel/__init__.py +++ b/python/src/docx_scalpel/__init__.py @@ -153,6 +153,7 @@ RetainedTableAnchor, PreconditionFailure, PreconditionTarget, + RevisionDiagnostic, RevisionListEntry, RunFormatting, RunFormattingInfo, @@ -270,6 +271,7 @@ "RetainedTableAnchor", "PreconditionFailure", "PreconditionTarget", + "RevisionDiagnostic", "RevisionListEntry", "RunFormatting", "RunFormattingInfo", diff --git a/python/src/docx_scalpel/enums.py b/python/src/docx_scalpel/enums.py index 73431eef..334e9d29 100644 --- a/python/src/docx_scalpel/enums.py +++ b/python/src/docx_scalpel/enums.py @@ -189,7 +189,11 @@ class EditErrorCode(str, Enum): MANAGED_BOOKMARK = "managed_bookmark" EMPTY_HYPERLINK_SPAN = "empty_hyperlink_span" UNSUPPORTED_INLINE_BOUNDARY = "unsupported_inline_boundary" + REVISION_UNSUPPORTED = "revision_unsupported" + REVISION_MALFORMED = "revision_malformed" + REVISION_AMBIGUOUS = "revision_ambiguous" TRACKED_OPERATION_UNSUPPORTED = "tracked_operation_unsupported" + UNRESOLVED_STRUCTURAL_REVISION = "unresolved_structural_revision" INTERNAL_ERROR = "internal_error" @classmethod diff --git a/python/src/docx_scalpel/session.py b/python/src/docx_scalpel/session.py index 31421ab1..1ace9d2a 100644 --- a/python/src/docx_scalpel/session.py +++ b/python/src/docx_scalpel/session.py @@ -1515,6 +1515,14 @@ def reject_revision(self, revision_id: str) -> EditResult: self._call("reject_revision", {"revisionId": revision_id}) ) + def accept_all_revisions(self) -> EditResult: + """Accept every supported live revision as one undoable mutation.""" + return EditResult._from_wire(self._call("accept_all_revisions", {})) + + def reject_all_revisions(self) -> EditResult: + """Reject every supported live revision as one undoable mutation.""" + return EditResult._from_wire(self._call("reject_all_revisions", {})) + # -- Tier C: formatting ----------------------------------------------- def apply_format( diff --git a/python/src/docx_scalpel/types.py b/python/src/docx_scalpel/types.py index 34a886a3..e2dde5db 100644 --- a/python/src/docx_scalpel/types.py +++ b/python/src/docx_scalpel/types.py @@ -1919,37 +1919,59 @@ def _from_wire(cls, d: Mapping[str, Any]) -> "CommentListEntry": ) +@dataclass(frozen=True, slots=True) +class RevisionDiagnostic: + code: str + message: str + + @classmethod + def _from_wire(cls, d: Mapping[str, Any]) -> "RevisionDiagnostic": + return cls(code=d.get("code", ""), message=d.get("message", "")) + + @dataclass(frozen=True, slots=True) class RevisionListEntry: """One tracked revision read directly off the live markup — see ``Session.list_revisions``. - ``id`` is stable while the underlying markup exists (derived from the markup's - own ``w:id`` attributes — resolving OTHER revisions never renames it) and is what - ``accept_revision``/``reject_revision`` address. ``type`` is ``"insert"``, - ``"delete"``, ``"move"`` (a linked move pair — both sides resolve together), or - ``"format"``. ``author``/``date`` are the markup's true ``w:author``/``w:date`` - (``date`` ``None`` when absent). ``text`` is the revision's visible text (deleted - text for deletions, ``¶`` for a revised paragraph mark, the affected text for - format changes). ``anchor_id`` is the containing block's anchor when addressable. + ``id`` is an opaque, deterministic ``rev2-…`` identity; ``constituent_ids`` exposes + the native Word ids. ``family`` identifies the exact atomic operation while ``type`` + is its coarse display class. Part/scope and every affected anchor are included. + Unsafe native topology remains listed through ``resolution_status`` and + ``diagnostic`` and fails closed when resolution is requested. """ id: str type: str author: str + family: str = "unsupported" + constituent_ids: tuple[str, ...] = () date: str | None = None text: str = "" + part_uri: str = "" + scope: str = "" anchor_id: str | None = None + affected_anchors: tuple[Anchor, ...] = () + resolution_status: str = "unsupported" + diagnostic: RevisionDiagnostic | None = None @classmethod def _from_wire(cls, d: Mapping[str, Any]) -> "RevisionListEntry": return cls( id=d["id"], type=d.get("type", ""), + family=d.get("family", "unsupported"), + constituent_ids=tuple(d.get("constituentIds", ())), author=d.get("author", "unknown"), date=d.get("date"), text=d.get("text", ""), + part_uri=d.get("partUri", ""), + scope=d.get("scope", ""), anchor_id=d.get("anchorId"), + affected_anchors=tuple(Anchor._from_wire(a) for a in d.get("affectedAnchors", ())), + resolution_status=d.get("resolutionStatus", "unsupported"), + diagnostic=(RevisionDiagnostic._from_wire(d["diagnostic"]) + if d.get("diagnostic") is not None else None), ) diff --git a/python/tests/test_revisions.py b/python/tests/test_revisions.py index f9294f41..9ffc46f1 100644 --- a/python/tests/test_revisions.py +++ b/python/tests/test_revisions.py @@ -1,4 +1,4 @@ -"""Selective per-revision accept/reject (issue #318). +"""Live revision registry and selective/bulk resolution (issues #318 and #455). ``list_revisions`` reads tracked revisions directly off the live markup with stable ids and the markup's true authors; ``accept_revision``/``reject_revision`` @@ -36,7 +36,14 @@ def test_list_revisions_reads_markup_identity(tour_plan_bytes: bytes) -> None: assert len(revisions) == 2 # one delete (old text) + one insert (new text) assert {r.type for r in revisions} == {"delete", "insert"} assert all(r.author == "py-reviewer" for r in revisions) - assert all(r.id.startswith("rev") for r in revisions) + assert all(r.id.startswith("rev2-") for r in revisions) + assert {r.family for r in revisions} == {"content_delete", "content_insert"} + assert all(r.constituent_ids for r in revisions) + assert all(r.part_uri == "/word/document.xml" for r in revisions) + assert all(r.scope == "body" for r in revisions) + assert all(r.affected_anchors for r in revisions) + assert all(r.resolution_status == "supported" for r in revisions) + assert all(r.diagnostic is None for r in revisions) insert = next(r for r in revisions if r.type == "insert") assert insert.text == "Tracked rewrite." assert insert.anchor_id is not None @@ -91,3 +98,19 @@ def test_reject_revision_is_undoable(tour_plan_bytes: bytes) -> None: assert session.undo() assert insert.id in [r.id for r in session.list_revisions()] + + +def test_bulk_resolution_is_undoable(tour_plan_bytes: bytes) -> None: + for accept in (True, False): + with open_session(tour_plan_bytes) as session: + session.set_tracked_changes(TrackedChangeMode.RENDER_INLINE) + anchor = _first_body_paragraph(session) + assert session.replace_text(anchor, "Bulk resolution.").success + before = [r.id for r in session.list_revisions()] + + result = (session.accept_all_revisions() if accept + else session.reject_all_revisions()) + assert result.success, result.error + assert session.list_revisions() == () + assert session.undo() + assert [r.id for r in session.list_revisions()] == before diff --git a/tools/mcp-server/Dispatcher.cs b/tools/mcp-server/Dispatcher.cs index 396e19d8..1e3bd860 100644 --- a/tools/mcp-server/Dispatcher.cs +++ b/tools/mcp-server/Dispatcher.cs @@ -696,7 +696,8 @@ private static string TrackChanges(SessionStore store, JsonElement args) // true authors/dates, and none of the ~seconds-long accept-all/reject-all // re-diff the old listing paid on large documents. var revisionsJson = "{\"revisions\":" + DocxSessionOps.ListRevisions(session.Handle) + "}"; - return FilterRevisions(revisionsJson, OptStr(args, "author"), OptStr(args, "changeType")); + return FilterRevisions(revisionsJson, OptStr(args, "author"), OptStr(args, "changeType"), + OptStr(args, "family"), OptStr(args, "resolutionStatus"), OptStr(args, "partUri")); } case "accept": return Guarded(session, ParsePreconditions(args, MutationTarget(args)), () => @@ -705,31 +706,11 @@ private static string TrackChanges(SessionStore store, JsonElement args) return Guarded(session, ParsePreconditions(args, MutationTarget(args)), () => DocxSessionOps.RejectRevision(session.Handle, Str(args, "revisionId"))); case "accept_all": - { - var preconditions = ParsePreconditions(args, MutationTarget(args)); - var check = Check(session, preconditions); - if (check is not null) return check; - var nextVersion = checked(DocxSessionOps.GetVersion(session.Handle) + 1); - // SaveWithAnchorIds (not Save) so the transformed bytes still carry the - // PtOpenXml:Unid attributes Rebind's reopen needs to keep anchor ids stable. - var bytes = DocxSessionOps.SaveWithAnchorIds(session.Handle); - var accepted = RevisionProcessor.AcceptRevisions(new WmlDocument("session.docx", bytes)); - store.Rebind(session, accepted.DocumentByteArray); - DocxSessionOps.RestoreVersionAfterRebind(session.Handle, nextVersion); - return "{\"success\":true}"; - } + return Guarded(session, ParsePreconditions(args, MutationTarget(args)), () => + DocxSessionOps.AcceptAllRevisions(session.Handle)); case "reject_all": - { - var preconditions = ParsePreconditions(args, MutationTarget(args)); - var check = Check(session, preconditions); - if (check is not null) return check; - var nextVersion = checked(DocxSessionOps.GetVersion(session.Handle) + 1); - var bytes = DocxSessionOps.SaveWithAnchorIds(session.Handle); - var rejected = RevisionProcessor.RejectRevisions(new WmlDocument("session.docx", bytes)); - store.Rebind(session, rejected.DocumentByteArray); - DocxSessionOps.RestoreVersionAfterRebind(session.Handle, nextVersion); - return "{\"success\":true}"; - } + return Guarded(session, ParsePreconditions(args, MutationTarget(args)), () => + DocxSessionOps.RejectAllRevisions(session.Handle)); case "set_mode": { var modeStr = Str(args, "mode"); @@ -751,9 +732,11 @@ private static string TrackChanges(SessionStore store, JsonElement args) } } - private static string FilterRevisions(string revisionsJson, string? author, string? changeType) + private static string FilterRevisions(string revisionsJson, string? author, string? changeType, + string? family, string? resolutionStatus, string? partUri) { - if (author is null && changeType is null) return revisionsJson; + if (author is null && changeType is null && family is null + && resolutionStatus is null && partUri is null) return revisionsJson; using var doc = JsonDocument.Parse(revisionsJson); if (!doc.RootElement.TryGetProperty("revisions", out var revisions) || revisions.ValueKind != JsonValueKind.Array) return revisionsJson; @@ -767,6 +750,17 @@ private static string FilterRevisions(string revisionsJson, string? author, stri if (changeType is not null && (!r.TryGetProperty("type", out var t) || !string.Equals(t.GetString(), changeType, StringComparison.OrdinalIgnoreCase))) continue; + if (family is not null + && (!r.TryGetProperty("family", out var f) || !string.Equals(f.GetString(), family, StringComparison.OrdinalIgnoreCase))) + continue; + if (resolutionStatus is not null + && (!r.TryGetProperty("resolutionStatus", out var status) + || !string.Equals(status.GetString(), resolutionStatus, StringComparison.OrdinalIgnoreCase))) + continue; + if (partUri is not null + && (!r.TryGetProperty("partUri", out var part) + || !string.Equals(part.GetString(), partUri, StringComparison.Ordinal))) + continue; kept.Add(r.GetRawText()); } return "{\"revisions\":[" + string.Join(",", kept) + "]}"; diff --git a/tools/mcp-server/ToolCatalog.cs b/tools/mcp-server/ToolCatalog.cs index 3b5c992b..556d9857 100644 --- a/tools/mcp-server/ToolCatalog.cs +++ b/tools/mcp-server/ToolCatalog.cs @@ -454,17 +454,20 @@ internal static class ToolCatalog """), new ToolDefinition( "docxodus_track_changes", - "List, selectively accept/reject (by revisionId), or bulk-resolve tracked changes (w:ins/w:del/w:moveFrom/w:moveTo/w:*PrChange) already present in the document — or switch how the session records its OWN subsequent edits (set_mode).", + "List, selectively accept/reject (by revisionId), or atomically bulk-resolve live tracked changes including structural cell, content-control, and numbering families — or switch how the session records its OWN subsequent edits (set_mode).", """ { "type": "object", "properties": { "sessionId": { "type": "string" }, "preconditions": { "type": "object", "description": "Optional optimistic guards for accept/reject/accept_all/reject_all." }, - "action": { "type": "string", "enum": ["list", "accept", "reject", "accept_all", "reject_all", "set_mode"], "description": "'list' reads revisions directly off the live markup — each entry carries a stable id, the markup's true author/date, its text, and the containing block's anchorId. 'accept'/'reject' resolve ONE revision by revisionId (undoable via docxodus_undo; other revisions keep their ids). 'accept_all'/'reject_all' resolve the whole document (not undoable — the session is rebound to the transformed bytes). 'set_mode' switches the session's own recording mode mid-workflow (issue #304)." }, - "revisionId": { "type": "string", "description": "accept/reject: the id from 'list' (e.g. 'rev101'). Unknown or already-resolved ids fail with revision_not_found — re-list for the current set." }, + "action": { "type": "string", "enum": ["list", "accept", "reject", "accept_all", "reject_all", "set_mode"], "description": "'list' returns the live part-aware registry, including all affected anchors and fail-closed diagnostics. Individual and bulk resolution use the same resolver and are undoable. Bulk resolution is atomic and refuses unsupported/malformed/ambiguous entries." }, + "revisionId": { "type": "string", "description": "accept/reject: the opaque stable id from 'list' (e.g. 'rev2-a1b2c3d4e5f60718293a'). Legacy revNNN ids remain accepted only when uniquely resolvable. Unknown or already-resolved ids fail with revision_not_found — re-list for the current set." }, "author": { "type": "string", "description": "list: only return revisions by this author." }, - "changeType": { "type": "string", "enum": ["insert", "delete", "move", "format"], "description": "list: only return revisions of this type. A 'move' entry is a linked pair — accepting/rejecting it resolves both sides." }, + "changeType": { "type": "string", "enum": ["insert", "delete", "move", "format", "structure"], "description": "list: only return revisions of this coarse type." }, + "family": { "type": "string", "description": "list: exact family filter, such as cell_delete, content_control_insert, or numbering_change." }, + "resolutionStatus": { "type": "string", "enum": ["supported", "unsupported", "malformed", "ambiguous"], "description": "list: fail-closed resolution status filter." }, + "partUri": { "type": "string", "description": "list: exact owning package-part URI." }, "mode": { "type": "string", "enum": ["accept", "render_inline", "strip_deletions"], "description": "set_mode: how SUBSEQUENT mutations are recorded (same values as docxodus_open's trackedChanges). Never touches already-applied edits — accept does not resolve existing revisions (use accept/accept_all), render_inline does not retroactively track prior direct edits. Not undoable." }, "revisionAuthor": { "type": "string", "description": "set_mode: author stamped on subsequent tracked-change markup. Absent = leave the current author unchanged; empty string = reset to the 'docxodus' default." } }, diff --git a/tools/python-host/Dispatcher.cs b/tools/python-host/Dispatcher.cs index 3bb8cae4..d4fb55e4 100644 --- a/tools/python-host/Dispatcher.cs +++ b/tools/python-host/Dispatcher.cs @@ -176,6 +176,8 @@ public static string Dispatch(string op, JsonElement args) "list_revisions" => DocxSessionOps.ListRevisions(Handle(args)), "accept_revision" => DocxSessionOps.AcceptRevision(Handle(args), Str(args, "revisionId")), "reject_revision" => DocxSessionOps.RejectRevision(Handle(args), Str(args, "revisionId")), + "accept_all_revisions" => DocxSessionOps.AcceptAllRevisions(Handle(args)), + "reject_all_revisions" => DocxSessionOps.RejectAllRevisions(Handle(args)), "apply_format" => DocxSessionOps.ApplyFormat( Handle(args), Str(args, "anchorId"), ParseOptionalSpan(args, "span"), ParseFormatOp(args, "op")), diff --git a/wasm/DocxodusWasm/DocxSessionBridge.cs b/wasm/DocxodusWasm/DocxSessionBridge.cs index f2094e40..044b2988 100644 --- a/wasm/DocxodusWasm/DocxSessionBridge.cs +++ b/wasm/DocxodusWasm/DocxSessionBridge.cs @@ -635,6 +635,14 @@ public static string AcceptRevision(int h, string revisionId) => public static string RejectRevision(int h, string revisionId) => DocxSessionOps.RejectRevision(h, revisionId); + /// Accept all supported live revisions as one undoable session mutation. + [JSExport] + public static string AcceptAllRevisions(int h) => DocxSessionOps.AcceptAllRevisions(h); + + /// Reject all supported live revisions as one undoable session mutation. + [JSExport] + public static string RejectAllRevisions(int h) => DocxSessionOps.RejectAllRevisions(h); + [JSExport] public static string ApplyFormat(int h, string anchor, string spanJson, string opJson) => DocxSessionOps.ApplyFormat(h, anchor, ParseSpan(spanJson), DocxSessionJson.ParseFormatOp(opJson)); From 551e450aea014c4a681bac191ae932d827698a69 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Fri, 14 Aug 2026 04:57:04 -0500 Subject: [PATCH 2/3] Harden tracked structural revisions --- .../DocxSessionStructuralRevisionTests.cs | 386 ++++++++++++++++++ Docxodus.Tests/McpServerDispatcherTests.cs | 149 +++++++ Docxodus/DocxSession.cs | 266 +++++++++++- Docxodus/Internal/NumberingFactory.cs | 19 +- Docxodus/Internal/RevisionOps.cs | 129 +++++- tools/mcp-server/Dispatcher.cs | 16 + tools/mcp-server/ToolCatalog.cs | 4 +- 7 files changed, 930 insertions(+), 39 deletions(-) diff --git a/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs b/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs index 4133af3b..a0e19fa6 100644 --- a/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs +++ b/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs @@ -343,6 +343,392 @@ public void DS45510_IdenticalNativeIdsInDifferentParts_AreIndependent() Assert.Equal("777", Assert.Single(remaining.ConstituentIds)); } + [Fact] + public void DS45511_TrackedSdtDelete_AcceptMatchesDirectWithoutParagraphHusks() + { + var baseline = BuildSdtDeleteBaseline(); + byte[] expected; + using (var direct = new DocxSession(baseline)) + { + Assert.True(direct.DeleteRange(AnchorByText(direct, "delete start"), + AnchorByText(direct, "after")).Success); + expected = direct.Save(); + } + + byte[] actual; + using (var tracked = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + RevisionAuthor = "SDT Reviewer", + })) + { + Assert.True(tracked.DeleteRange(AnchorByText(tracked, "delete start"), + AnchorByText(tracked, "after")).Success); + var resolved = tracked.AcceptAllRevisions(); + Assert.True(resolved.Success, resolved.Error?.Message); + Assert.Empty(tracked.ListRevisions()); + actual = tracked.Save(); + } + + var expectedRoot = MainRoot(expected); + var actualRoot = MainRoot(actual); + Assert.True(XNode.DeepEquals(expectedRoot, actualRoot), + FirstDifference(expectedRoot, actualRoot)); + Assert.Equal(new[] { "before", "after" }, + actualRoot.Descendants(W.p).Select(paragraph => paragraph.Value).ToArray()); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void DS45512_TrackedInsertParagraph_RoundTrips(bool accept) + { + var baseline = DocxSessionTests.BuildDS001_SimpleTwoParagraphs(); + byte[] expected; + if (accept) + { + using var direct = new DocxSession(baseline); + Assert.True(direct.InsertParagraph(FirstBodyTextAnchor(direct), Position.After, + "inserted paragraph").Success); + expected = direct.Save(); + } + else + { + expected = baseline; + } + + byte[] actual; + using (var tracked = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + RevisionAuthor = "Paragraph Reviewer", + })) + { + Assert.True(tracked.InsertParagraph(FirstBodyTextAnchor(tracked), Position.After, + "inserted paragraph").Success); + var revision = Assert.Single(tracked.ListRevisions()); + Assert.Equal(RevisionFamily.ContentInsert, revision.Family); + var resolved = accept + ? tracked.AcceptRevision(revision.Id) + : tracked.RejectRevision(revision.Id); + Assert.True(resolved.Success, resolved.Error?.Message); + Assert.Empty(tracked.ListRevisions()); + actual = tracked.Save(); + } + + Assert.True(XNode.DeepEquals(MainRoot(expected), MainRoot(actual)), + FirstDifference(MainRoot(expected), MainRoot(actual))); + } + + [Theory] + [InlineData("split")] + [InlineData("merge")] + [InlineData("insert_table")] + public void DS45513_UnsafeTrackedStructuralOperations_FailTypedAndUndoFree(string operation) + { + var baseline = DocxSessionTests.BuildDS001_SimpleTwoParagraphs(); + using var session = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + }); + var paragraphs = session.Project().AnchorIndex.Values + .Where(anchor => anchor.Anchor.Scope == "body" && anchor.Anchor.Kind == "p") + .Select(anchor => anchor.Anchor.Id).Take(2).ToArray(); + + var result = operation switch + { + "split" => session.SplitParagraph(paragraphs[0], 1), + "merge" => session.MergeParagraphs(paragraphs[0], paragraphs[1]), + "insert_table" => session.InsertTable(paragraphs[0], Position.After, 1, 1), + _ => throw new ArgumentOutOfRangeException(nameof(operation)), + }; + + Assert.False(result.Success); + Assert.Equal(EditErrorCode.TrackedOperationUnsupported, result.Error!.Code); + Assert.True(XNode.DeepEquals(MainRoot(baseline), MainRoot(session.Save()))); + Assert.False(session.Undo()); + } + + [Theory] + [InlineData("paragraph_style", true)] + [InlineData("paragraph_style", false)] + [InlineData("paragraph_format", true)] + [InlineData("paragraph_format", false)] + [InlineData("column_widths", true)] + [InlineData("column_widths", false)] + [InlineData("table_borders", true)] + [InlineData("table_borders", false)] + [InlineData("cell_shading", true)] + [InlineData("cell_shading", false)] + [InlineData("row_options", true)] + [InlineData("row_options", false)] + public void DS45514_TrackedPropertyMutation_RoundTrips(string operation, bool accept) + { + var baseline = operation.StartsWith("paragraph", StringComparison.Ordinal) + ? DocxSessionTests.BuildDS001_SimpleTwoParagraphs() + : BuildTableDocument(); + byte[] expected; + if (accept) + { + using var direct = new DocxSession(baseline); + Assert.True(ApplyPropertyMutation(direct, operation).Success); + expected = direct.Save(); + } + else + { + expected = baseline; + } + + byte[] actual; + using (var tracked = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + RevisionAuthor = "Format Reviewer", + })) + { + var edit = ApplyPropertyMutation(tracked, operation); + Assert.True(edit.Success, edit.Error?.Message); + var revision = Assert.Single(tracked.ListRevisions()); + Assert.Equal(RevisionFamily.PropertiesChange, revision.Family); + var resolved = accept + ? tracked.AcceptRevision(revision.Id) + : tracked.RejectRevision(revision.Id); + Assert.True(resolved.Success, resolved.Error?.Message); + Assert.Empty(tracked.ListRevisions()); + actual = tracked.Save(); + } + + var expectedRoot = MainRoot(expected); + var actualRoot = MainRoot(actual); + Assert.True(XNode.DeepEquals(expectedRoot, actualRoot), + FirstDifference(expectedRoot, actualRoot)); + Assert.Equal(ValidationErrors(expected), ValidationErrors(actual)); + } + + [Fact] + public void DS45515_StableIdsAndSequentialRows_RemainIndependent() + { + using (var content = new DocxSession(BuildSeparatedInsertions())) + { + var before = content.ListRevisions(); + var first = Assert.Single(before, revision => revision.ConstituentIds.SequenceEqual(new[] { "1" })); + var separator = Assert.Single(before, revision => revision.ConstituentIds.SequenceEqual(new[] { "2" })); + var third = Assert.Single(before, revision => revision.ConstituentIds.SequenceEqual(new[] { "3" })); + + Assert.True(content.AcceptRevision(separator.Id).Success); + + var after = content.ListRevisions(); + Assert.Equal(first.Id, Assert.Single(after, + revision => revision.ConstituentIds.SequenceEqual(new[] { "1" })).Id); + Assert.Equal(third.Id, Assert.Single(after, + revision => revision.ConstituentIds.SequenceEqual(new[] { "3" })).Id); + } + + using var rows = new DocxSession(BuildTableDocument(), new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + RevisionAuthor = "Row Reviewer", + }); + var cell = FirstCellAnchor(rows); + Assert.True(rows.InsertTableRow(cell, Position.After).Success); + Assert.True(rows.InsertTableRow(cell, Position.After).Success); + var rowRevisions = rows.ListRevisions() + .Where(revision => revision.Family == RevisionFamily.RowInsert).ToList(); + Assert.Equal(2, rowRevisions.Count); + var survivingId = rowRevisions[1].Id; + Assert.True(rows.AcceptRevision(rowRevisions[0].Id).Success); + Assert.Equal(survivingId, Assert.Single(rows.ListRevisions(), + revision => revision.Family == RevisionFamily.RowInsert).Id); + } + + [Theory] + [InlineData("math_ctrlpr")] + [InlineData("numbering_delete")] + [InlineData("run_properties_delete")] + public void DS45516_UnhandledRecognizedFamilies_AreListedAndBlockBulk(string shape) + { + var input = BuildUnsupportedRevisionDocument(shape); + using var session = new DocxSession(input); + var before = MainRoot(session.Save()); + var revision = Assert.Single(session.ListRevisions()); + Assert.Equal(RevisionFamily.Unsupported, revision.Family); + Assert.Equal(RevisionResolutionStatus.Unsupported, revision.ResolutionStatus); + Assert.Equal("unsupported_revision_family", revision.Diagnostic!.Code); + + var result = session.AcceptAllRevisions(); + + Assert.False(result.Success); + Assert.Equal(EditErrorCode.RevisionUnsupported, result.Error!.Code); + Assert.True(XNode.DeepEquals(before, MainRoot(session.Save()))); + Assert.False(session.Undo()); + } + + [Fact] + public void DS45517_NonnumericNativeId_IsMalformedAndFailsClosed() + { + var input = MutateMain(DocxSessionTests.BuildDS001_SimpleTwoParagraphs(), root => + { + var run = root.Descendants(W.p).First().Elements(W.r).First(); + run.ReplaceWith(new XElement(W.ins, + new XAttribute(W.id, "not-an-integer"), + new XAttribute(W.author, "Bad Producer"), + new XAttribute(W.date, "2026-01-01T00:00:00Z"), + new XElement(run))); + }); + using var session = new DocxSession(input); + var revision = Assert.Single(session.ListRevisions()); + Assert.Equal(RevisionResolutionStatus.Malformed, revision.ResolutionStatus); + Assert.Equal("invalid_revision_id", revision.Diagnostic!.Code); + + var result = session.AcceptRevision(revision.Id); + + Assert.False(result.Success); + Assert.Equal(EditErrorCode.RevisionMalformed, result.Error!.Code); + Assert.False(session.Undo()); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void DS45518_RejectTrackedListCreation_RestoresPackageAndUndoRedo(bool bulk) + { + var baseline = DocxSessionTests.BuildDS001_SimpleTwoParagraphs(); + Assert.False(HasNumberingPart(baseline)); + using var session = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + RevisionAuthor = "List Reviewer", + }); + Assert.True(session.ApplyListFormat(FirstBodyTextAnchor(session), ListFormat.Decimal).Success); + Assert.True(HasNumberingPart(session.Save())); + var revision = Assert.Single(session.ListRevisions()); + + var rejected = bulk + ? session.RejectAllRevisions() + : session.RejectRevision(revision.Id); + + Assert.True(rejected.Success, rejected.Error?.Message); + Assert.False(HasNumberingPart(session.Save())); + Assert.True(XNode.DeepEquals(MainRoot(baseline), MainRoot(session.Save()))); + Assert.True(session.Undo()); + Assert.True(HasNumberingPart(session.Save())); + Assert.Single(session.ListRevisions()); + Assert.True(session.Redo()); + Assert.False(HasNumberingPart(session.Save())); + Assert.Empty(session.ListRevisions()); + } + + private static EditResult ApplyPropertyMutation(DocxSession session, string operation) + { + if (operation.StartsWith("paragraph", StringComparison.Ordinal)) + { + var paragraph = FirstBodyTextAnchor(session); + return operation switch + { + "paragraph_style" => session.SetParagraphStyle(paragraph, "Heading2"), + "paragraph_format" => session.SetParagraphFormat(paragraph, + new ParagraphFormatOp { Alignment = ParagraphAlignment.Center, SpacingAfter = 240 }), + _ => throw new ArgumentOutOfRangeException(nameof(operation)), + }; + } + + var cell = FirstCellAnchor(session); + return operation switch + { + "column_widths" => session.SetColumnWidths(cell, new[] { 2600, 3000 }), + "table_borders" => session.SetTableBorders(cell, + new TableBorderSpec { Style = "double", Size = 8, Color = "CC0000" }), + "cell_shading" => session.SetCellShading(cell, "D9EAF7", TableShadingScope.Row), + "row_options" => session.SetTableRowOptions(cell, + new TableRowOptions { RepeatHeader = true, AllowBreakAcrossPages = false, HeightTwips = 480 }), + _ => throw new ArgumentOutOfRangeException(nameof(operation)), + }; + } + + private static byte[] BuildSdtDeleteBaseline() => + MutateMain(DocxSessionTests.BuildDS001_SimpleTwoParagraphs(), root => + { + var body = root.Element(W.body)!; + var sectPr = body.Element(W.sectPr) is { } section + ? new XElement(section) + : null; + body.ReplaceNodes( + Paragraph("before"), + Paragraph("delete start"), + new XElement(W.sdt, + new XElement(W.sdtPr, + new XElement(W.tag, new XAttribute(W.val, "controlled"))), + new XElement(W.sdtContent, Paragraph("controlled paragraph"))), + Paragraph("after"), + sectPr); + }); + + private static byte[] BuildSeparatedInsertions() => + MutateMain(DocxSessionTests.BuildDS001_SimpleTwoParagraphs(), root => + { + var paragraph = root.Descendants(W.p).First(); + paragraph.ReplaceNodes( + RevisionWrapper(W.ins, "1", "A", "2026-01-01T00:00:00Z", "one"), + RevisionWrapper(W.del, "2", "B", "2026-01-01T00:00:00Z", "separator"), + RevisionWrapper(W.ins, "3", "A", "2026-01-02T00:00:00Z", "three")); + }); + + private static byte[] BuildUnsupportedRevisionDocument(string shape) => + MutateMain(DocxSessionTests.BuildDS001_SimpleTwoParagraphs(), root => + { + var paragraph = root.Descendants(W.p).First(); + var marker = new XElement(W.del, + new XAttribute(W.id, "44"), + new XAttribute(W.author, "Unsupported Reviewer"), + new XAttribute(W.date, "2026-01-01T00:00:00Z")); + switch (shape) + { + case "math_ctrlpr": + paragraph.ReplaceNodes(new XElement(M.oMath, + new XElement(M.f, + new XElement(M.fPr, new XElement(M.ctrlPr, marker)), + new XElement(M.num, new XElement(W.r, new XElement(W.t, "1"))), + new XElement(M.den, new XElement(W.r, new XElement(W.t, "2")))))); + break; + case "numbering_delete": + paragraph.AddFirst(new XElement(W.pPr, + new XElement(W.numPr, + new XElement(W.ilvl, new XAttribute(W.val, 0)), + new XElement(W.numId, new XAttribute(W.val, 1)), + marker))); + break; + case "run_properties_delete": + paragraph.Elements(W.r).First().AddFirst(new XElement(W.rPr, marker)); + break; + default: + throw new ArgumentOutOfRangeException(nameof(shape)); + } + }); + + private static XElement Paragraph(string text) => + new(W.p, new XElement(W.r, new XElement(W.t, text))); + + private static XElement RevisionWrapper( + XName name, string id, string author, string date, string text) => + new(name, + new XAttribute(W.id, id), + new XAttribute(W.author, author), + new XAttribute(W.date, date), + new XElement(W.r, + new XElement(name == W.del ? W.delText : W.t, text))); + + private static string AnchorByText(DocxSession session, string text) => + session.Project().AnchorIndex.Values.Single(anchor => + string.Equals(session.GetAnchorInfo(anchor.Anchor.Id)?.TextPreview, + text, StringComparison.Ordinal)).Anchor.Id; + + private static bool HasNumberingPart(byte[] bytes) + { + using var stream = new MemoryStream(bytes); + using var document = WordprocessingDocument.Open(stream, false); + return document.MainDocumentPart!.NumberingDefinitionsPart is not null; + } + private static byte[] BuildTableDocument() { using var session = new DocxSession(DocxSessionTests.BuildDS001_SimpleTwoParagraphs()); diff --git a/Docxodus.Tests/McpServerDispatcherTests.cs b/Docxodus.Tests/McpServerDispatcherTests.cs index 9b360dfb..bc38535d 100644 --- a/Docxodus.Tests/McpServerDispatcherTests.cs +++ b/Docxodus.Tests/McpServerDispatcherTests.cs @@ -1308,6 +1308,155 @@ public void MCP097_AtomicStepPreconditionsUseBatchStartState() Assert.DoesNotContain("first atomic state", markdown); } + [Fact] + public void MCP099_TrackChangesBatchPreviewIsIsolatedAndAtomicApplyResolvesRevision() + { + var sessionId = OpenSession(trackedChanges: "render_inline"); + var sessionArg = JsonSerializer.Serialize(sessionId); + InsertParagraph(sessionId, "tracked paragraph"); + + var listed = Parse(Dispatcher.Call(_store, "docxodus_track_changes", J( + $$"""{"sessionId":{{sessionArg}},"action":"list"}"""))); + var revision = Assert.Single(listed.GetProperty("revisions").EnumerateArray()); + var revisionId = revision.GetProperty("id").GetString()!; + Assert.Equal("content_insert", revision.GetProperty("family").GetString()); + + var preview = Parse(Dispatcher.Call(_store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "mode": "preview", + "steps": [ + { "tool": "docxodus_track_changes", "args": { "action": "reject", "revisionId": {{JsonSerializer.Serialize(revisionId)}} } } + ] + } + """))); + Assert.Equal("ok", preview.GetProperty("status").GetString()); + Assert.True(preview.GetProperty("success").GetBoolean()); + Assert.True(preview.GetProperty("preview").GetBoolean()); + Assert.True(Assert.Single(preview.GetProperty("steps").EnumerateArray()) + .GetProperty("success").GetBoolean()); + + var afterPreview = Parse(Dispatcher.Call(_store, "docxodus_track_changes", J( + $$"""{"sessionId":{{sessionArg}},"action":"list"}"""))); + Assert.Equal(revisionId, + Assert.Single(afterPreview.GetProperty("revisions").EnumerateArray()).GetProperty("id").GetString()); + Assert.Contains("tracked paragraph", Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"markdown"}"""))).GetProperty("markdown").GetString()!); + + var applied = Parse(Dispatcher.Call(_store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "mode": "atomic", + "steps": [ + { "tool": "docxodus_track_changes", "args": { "action": "reject", "revisionId": {{JsonSerializer.Serialize(revisionId)}} } } + ] + } + """))); + Assert.Equal("ok", applied.GetProperty("status").GetString()); + Assert.True(applied.GetProperty("success").GetBoolean()); + Assert.Equal(0, Parse(Dispatcher.Call(_store, "docxodus_track_changes", J( + $$"""{"sessionId":{{sessionArg}},"action":"list"}"""))).GetProperty("revisions").GetArrayLength()); + Assert.DoesNotContain("tracked paragraph", Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"markdown"}"""))).GetProperty("markdown").GetString()!); + } + + [Fact] + public void MCP102_TrackChangesBulkAcceptAndRejectAllAreAtomicBatchSteps() + { + var sessionId = OpenSession(); + var sessionArg = JsonSerializer.Serialize(sessionId); + var anchor = FirstBodyAnchorId(sessionId, _store); + Assert.True(ReplaceText(_store, sessionId, anchor, "baseline") + .GetProperty("success").GetBoolean()); + SetMode(sessionId, "render_inline"); + + Assert.True(ReplaceText(_store, sessionId, anchor, "accepted replacement") + .GetProperty("success").GetBoolean()); + var accepted = Parse(Dispatcher.Call(_store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "mode": "atomic", + "steps": [ + { "tool": "docxodus_track_changes", "args": { "action": "accept_all" } } + ] + } + """))); + Assert.Equal("ok", accepted.GetProperty("status").GetString()); + Assert.True(accepted.GetProperty("success").GetBoolean()); + Assert.Contains("accepted replacement", Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"markdown"}"""))).GetProperty("markdown").GetString()!); + + Assert.True(ReplaceText(_store, sessionId, anchor, "rejected replacement") + .GetProperty("success").GetBoolean()); + var rejected = Parse(Dispatcher.Call(_store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "mode": "atomic", + "steps": [ + { "tool": "docxodus_track_changes", "args": { "action": "reject_all" } } + ] + } + """))); + Assert.Equal("ok", rejected.GetProperty("status").GetString()); + Assert.True(rejected.GetProperty("success").GetBoolean()); + var markdown = Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"markdown"}"""))).GetProperty("markdown").GetString()!; + Assert.Contains("accepted replacement", markdown); + Assert.DoesNotContain("rejected replacement", markdown); + } + + [Fact] + public void MCP103_TrackChangesReadOnlyBatchStepsFailStructuredAndSchemaAdvertisesMutations() + { + var sessionId = OpenSession(); + var sessionArg = JsonSerializer.Serialize(sessionId); + foreach (var action in new[] { "list", "set_mode" }) + { + var actionArg = JsonSerializer.Serialize(action); + var receipt = Parse(Dispatcher.Call(_store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "mode": "atomic", + "steps": [ + { "tool": "docxodus_track_changes", "args": { "action": {{actionArg}} } } + ] + } + """))); + Assert.Equal("failed", receipt.GetProperty("status").GetString()); + Assert.False(receipt.GetProperty("success").GetBoolean()); + var failure = receipt.GetProperty("failure"); + Assert.Equal("docxodus_track_changes", failure.GetProperty("tool").GetString()); + Assert.Equal(action, failure.GetProperty("action").GetString()); + Assert.Equal("invalid_batch_step", + failure.GetProperty("error").GetProperty("code").GetString()); + + var legacy = Assert.Throws(() => Dispatcher.Call( + _store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "mode": "apply", + "steps": [ + { "tool": "docxodus_track_changes", "args": { "action": {{actionArg}} } } + ] + } + """))); + Assert.Contains(action, legacy.Message, StringComparison.Ordinal); + } + + var mutations = Assert.Single(ToolCatalog.Tools, tool => tool.Name == "docxodus_mutations"); + using var schema = JsonDocument.Parse(mutations.InputSchemaJson); + var tools = schema.RootElement.GetProperty("properties").GetProperty("steps") + .GetProperty("items").GetProperty("properties").GetProperty("tool") + .GetProperty("enum").EnumerateArray().Select(value => value.GetString()).ToList(); + Assert.Contains("docxodus_track_changes", tools); + } + // ─── Tool catalog ─────────────────────────────────────────────────── [Fact] diff --git a/Docxodus/DocxSession.cs b/Docxodus/DocxSession.cs index 8ddc2ee6..b6190922 100644 --- a/Docxodus/DocxSession.cs +++ b/Docxodus/DocxSession.cs @@ -2729,6 +2729,9 @@ private EditResult ResolveRevision(string revisionId, bool accept) var partUri = group.PartUri; var owningPart = ResolvePart(partUri); + var rejectedNumberingIds = accept + ? Array.Empty() + : NumberingIdsIntroducedBy(group); // Capture the block anchors the resolution touches BEFORE applying — elements // detach during Apply and can no longer be resolved to a part afterwards. @@ -2740,6 +2743,8 @@ private EditResult ResolveRevision(string revisionId, bool accept) var removedElements = registry.Resolve(group, accept); if (owningPart is not null) SweepOrphanedStoryRelationships(owningPart); + if (!accept && rejectedNumberingIds.Count > 0) + PruneUnreferencedDocxodusNumbering(rejectedNumberingIds); var removed = new List(); var seenRemoved = new HashSet(StringComparer.Ordinal); @@ -2786,6 +2791,9 @@ private EditResult ResolveAllRevisions(bool accept) var modified = registry.Entries.SelectMany(g => RevisionGroupAnchors(g, g.PartUri)) .GroupBy(a => a.Id, StringComparer.Ordinal).Select(g => g.First()).ToList(); + IReadOnlyList rejectedNumberingIds = accept + ? Array.Empty() + : registry.Entries.SelectMany(NumberingIdsIntroducedBy).Distinct().ToList(); _history.RecordPreOp(TakeSnapshot()); try @@ -2793,6 +2801,8 @@ private EditResult ResolveAllRevisions(bool accept) var removedElements = registry.ResolveAll(accept); foreach (var story in RevisionStoryParts()) SweepOrphanedStoryRelationships(story.Part); + if (!accept && rejectedNumberingIds.Count > 0) + PruneUnreferencedDocxodusNumbering(rejectedNumberingIds); var removed = new List(); var seenRemoved = new HashSet(StringComparer.Ordinal); foreach (var element in removedElements) @@ -2845,6 +2855,68 @@ private EditResult ResolveAllRevisions(bool accept) group.Diagnostic?.Message ?? "revision cannot be resolved safely"); } + private static IReadOnlyList NumberingIdsIntroducedBy( + Internal.RevisionOps.RevisionGroup group) => + group.Units.Where(unit => unit.Kind == Internal.RevisionOps.UnitKind.NumberingPropertiesInsert) + .Select(unit => (string?)unit.Element.Parent?.Element(W.numId)?.Attribute(W.val)) + .Where(value => int.TryParse(value, out _)) + .Select(value => int.Parse(value!, System.Globalization.CultureInfo.InvariantCulture)) + .Distinct() + .ToList(); + + /// Remove Docxodus-authored numbering instances made unreachable by rejecting their + /// native numPr insertion. Native revision markup cannot carry a package-part before-image; + /// pruning only our fixed-nsid definitions recovers exact package parity without touching + /// unrelated producer numbering. + private void PruneUnreferencedDocxodusNumbering(IReadOnlyCollection? candidateIds) + { + var main = _doc!.MainDocumentPart; + var part = main?.NumberingDefinitionsPart; + var root = part?.GetXDocument().Root; + if (main is null || part is null || root is null) return; + + var referenced = new HashSet(); + IEnumerable referenceParts = EnumerateProjectedParts() + .Where(projected => !ReferenceEquals(projected, part)); + if (main.StyleDefinitionsPart is { } styles + && !referenceParts.Any(projected => ReferenceEquals(projected, styles))) + referenceParts = referenceParts.Append(styles); + foreach (var referencePart in referenceParts) + { + foreach (var numId in referencePart.GetXDocument().Descendants(W.numId)) + if (int.TryParse((string?)numId.Attribute(W.val), out var value) && value > 0) + referenced.Add(value); + } + + var candidates = candidateIds?.ToHashSet() ?? root.Elements(W.num) + .Select(num => (string?)num.Attribute(W.numId)) + .Where(value => int.TryParse(value, out _)) + .Select(value => int.Parse(value!, System.Globalization.CultureInfo.InvariantCulture)) + .ToHashSet(); + bool changed = false; + foreach (var num in root.Elements(W.num).ToList()) + { + if (!int.TryParse((string?)num.Attribute(W.numId), out var numId) + || !candidates.Contains(numId) || referenced.Contains(numId)) + continue; + var abstractId = (string?)num.Element(W.abstractNumId)?.Attribute(W.val); + var abstractNum = root.Elements(W.abstractNum) + .FirstOrDefault(candidate => (string?)candidate.Attribute(W.abstractNumId) == abstractId); + if (abstractNum is null || !Internal.NumberingFactory.IsDocxodusDefinition(abstractNum)) + continue; + + num.Remove(); + changed = true; + if (!root.Elements(W.num).Any(other => + (string?)other.Element(W.abstractNumId)?.Attribute(W.val) == abstractId)) + abstractNum.Remove(); + } + + if (!changed) return; + if (!root.Elements().Any()) main.DeletePart(part); + else part.PutXDocument(); + } + private List RevisionGroupAnchors( Internal.RevisionOps.RevisionGroup group, string partUri) { @@ -6026,10 +6098,10 @@ private IEnumerable EnumerateProjectedPartsForSnapshot() // document left w:footnotePr and two orphan styles behind permanently. if (main.DocumentSettingsPart is not null) yield return main.DocumentSettingsPart; if (main.StyleDefinitionsPart is not null) yield return main.StyleDefinitionsPart; - - // The NUMBERING part is deliberately NOT here: list ops are additive-only by design - // (ApplyListStartOverride clones a fresh w:num rather than mutating a possibly shared one), - // which is what keeps undo correct without snapshotting it. See ApplyListStartOverride. + // List mutations can create the numbering part and tracked rejection must restore the + // exact pre-edit package, not merely remove the paragraph's numPr. Snapshot both its XML + // and topology; RestoreSnapshot reconciles create/delete below. + if (main.NumberingDefinitionsPart is not null) yield return main.NumberingDefinitionsPart; var annotationsPart = Internal.AnnotationsCustomXml.Find(_doc); if (annotationsPart is not null) yield return annotationsPart; } @@ -7069,6 +7141,14 @@ public EditResult InsertParagraph(string anchorId, Position pos, string markdown foreach (var n in newElements) { after.AddAfterSelf(n); after = n; } } + if (_trackedChanges == TrackedChangeMode.RenderInline) + { + var author = _revisionAuthor ?? "docxodus"; + var date = NextTrackedFormatRevisionDate(); + foreach (var paragraph in newElements) + MarkParagraphContentAndMark(paragraph, W.ins, author, date); + } + foreach (var n in newElements) PromoteHyperlinkRelationships(n); InvalidateProjectionCache(); @@ -7103,6 +7183,8 @@ public EditResult SplitParagraph(string anchorId, int characterOffset) if (characterOffset < 0 || characterOffset > totalText.Length) return EditResult.Fail(EditErrorCode.OffsetOutOfRange, $"offset {characterOffset} out of [0, {totalText.Length}]", anchorId); + if (_trackedChanges == TrackedChangeMode.RenderInline) + return TrackedStructureUnsupported("SplitParagraph", anchorId); _history.RecordPreOp(TakeSnapshot()); try @@ -7230,6 +7312,8 @@ public EditResult MergeParagraphs(string firstAnchorId, string secondAnchorId) if (!ReferenceEquals(firstEl.NextNode, secondEl)) return EditResult.Fail(EditErrorCode.AnchorsNotAdjacent, "MergeParagraphs requires second anchor to be the immediate next sibling of first"); + if (_trackedChanges == TrackedChangeMode.RenderInline) + return TrackedStructureUnsupported("MergeParagraphs", firstAnchorId); _history.RecordPreOp(TakeSnapshot()); try @@ -7722,22 +7806,31 @@ public EditResult SetParagraphStyle(string anchorId, string styleId) if (target.Anchor.Kind is not ("p" or "h" or "li")) return EditResult.Fail(EditErrorCode.AnchorWrongKind, "SetParagraphStyle requires a paragraph anchor", anchorId); + var element = target.Resolve(_doc); + if (element is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "element null", anchorId); + if (RefuseNestedTrackedParagraphPropertyChange(element, anchorId) is { } pending) return pending; + + // Style synthesis mutates the styles part. Capture before it so rejecting the generated + // pPrChange (or undoing this op) restores package state as well as the paragraph XML. + var preOp = TakeSnapshot(); + // Find-or-create well-known built-in styles (Title, Subtitle, Heading1-9) the document // hasn't defined yet, so applying one works instead of silently failing. Mirrors the inline // "Code" character style. A truly unknown custom id is left untouched and still rejected. if (!Internal.StyleFactory.EnsureParagraphStyle(_doc!, styleId)) return EditResult.Fail(EditErrorCode.UnknownStyle, $"style id not found: {styleId}", anchorId); - var element = target.Resolve(_doc); - if (element is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "element null", anchorId); - - _history.RecordPreOp(TakeSnapshot()); + var oldPPr = new XElement(element.Element(W.pPr) ?? new XElement(W.pPr)); + _history.RecordPreOp(preOp); try { var pPr = element.Element(W.pPr); if (pPr is null) { pPr = new XElement(W.pPr); element.AddFirst(pPr); } pPr.Element(W.pStyle)?.Remove(); pPr.AddFirst(new XElement(W.pStyle, new XAttribute(W.val, styleId))); + if (_trackedChanges == TrackedChangeMode.RenderInline) + TrackPropertyMutation(pPr, oldPPr, W.pPrChange, + _revisionAuthor ?? "docxodus", NextTrackedFormatRevisionDate(), W.rPr, W.sectPr); InvalidateProjectionCache(); // Anchor kind may have flipped (e.g., p → h); look it up in the fresh index. @@ -7855,6 +7948,7 @@ public EditResult SetParagraphFormat(string anchorId, ParagraphFormatOp op) var element = target.Resolve(_doc!); if (element is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "element null", anchorId); + if (RefuseNestedTrackedParagraphPropertyChange(element, anchorId) is { } pending) return pending; if (op.FirstLineIndent is not null && op.HangingIndent is not null) return EditResult.Fail(EditErrorCode.InvalidParagraphFormat, @@ -7867,6 +7961,7 @@ public EditResult SetParagraphFormat(string anchorId, ParagraphFormatOp op) return EditResult.Fail(EditErrorCode.InvalidParagraphFormat, "lineSpacingRule requires lineSpacing (w:lineRule qualifies w:line)", anchorId); + var oldPPr = new XElement(element.Element(W.pPr) ?? new XElement(W.pPr)); _history.RecordPreOp(TakeSnapshot()); try { @@ -7956,6 +8051,10 @@ public EditResult SetParagraphFormat(string anchorId, ParagraphFormatOp op) if (op.ClearBorders is true || op.TopBorder is not null || op.BottomBorder is not null) ApplyParagraphBorders(pPr, op.TopBorder, op.BottomBorder, op.ClearBorders is true); + if (_trackedChanges == TrackedChangeMode.RenderInline) + TrackPropertyMutation(pPr, oldPPr, W.pPrChange, + _revisionAuthor ?? "docxodus", NextTrackedFormatRevisionDate(), W.rPr, W.sectPr); + InvalidateProjectionCache(); // pPr-only writes (jc/ind/spacing/pBdr/pageBreakBefore) can't change an anchor's // kind — KindFor derives it from pStyle/numPr, which this op never touches — nor @@ -9474,6 +9573,8 @@ public EditResult InsertTable(string anchorId, Position pos, int rows, int cols, if (colWidths is not null && (colWidths.Count != cols || colWidths.Any(w => w <= 0))) return EditResult.Fail(EditErrorCode.MalformedMarkdown, $"ColumnWidths must have one positive width per column ({cols}); got {colWidths.Count}", anchorId); + if (_trackedChanges == TrackedChangeMode.RenderInline) + return TrackedStructureUnsupported("InsertTable", anchorId); _history.RecordPreOp(TakeSnapshot()); try @@ -9795,6 +9896,17 @@ private static EditResult TrackedStructureUnsupported(string operation, string a $"{operation} has no reversible native tracked-change encoding on this document shape; no changes were made", anchorId); + private EditResult? RefuseNestedTrackedPropertyChange( + IEnumerable<(XElement? Properties, XName ChangeName)> properties, string anchorId) + { + if (_trackedChanges != TrackedChangeMode.RenderInline) return null; + var pending = properties.FirstOrDefault(pair => pair.Properties?.Element(pair.ChangeName) is not null); + return pending.Properties is null ? null : EditResult.Fail( + EditErrorCode.UnresolvedStructuralRevision, + $"the target already has an unresolved {pending.ChangeName.LocalName}; resolve it before another tracked property mutation", + anchorId); + } + private static XElement PropertySnapshot(XElement? properties, XName propertyName, XName changeName, params XName[] excluded) { @@ -9808,6 +9920,17 @@ private static bool PropertySnapshotEquals(XElement snapshot, XElement? current, params XName[] excluded) => XNode.DeepEquals(snapshot, PropertySnapshot(current, snapshot.Name, changeName, excluded)); + /// Append a native *PrChange only when the live base properties differ from the + /// captured old value. Callers that change several properties as one operation pass the same + /// author/date stamp so the registry exposes one atomic table-format revision. + private void TrackPropertyMutation(XElement current, XElement oldProperties, XName changeName, + string author, string date, params XName[] excluded) + { + var oldBase = PropertySnapshot(oldProperties, current.Name, changeName, excluded); + if (PropertySnapshotEquals(oldBase, current, changeName, excluded)) return; + current.Add(CreateRevisionEnvelope(changeName, author, date, oldBase)); + } + private void MarkRowAsTrackedRevision(XElement row, bool inserted, string author, string date) { var wrapperName = inserted ? W.ins : W.del; @@ -10514,6 +10637,19 @@ public EditResult SetColumnWidths(string cellAnchorId, IReadOnlyList widths $"widths must list one positive twip value per column ({colCount}); got {widthsTwips?.Count ?? 0}", cellAnchorId); + var tracked = _trackedChanges == TrackedChangeMode.RenderInline; + var tableProperties = tbl.Element(W.tblPr); + var cellProperties = tbl.Descendants(W.tc).Select(cell => cell.Element(W.tcPr)).ToList(); + if (RefuseNestedTrackedPropertyChange( + new[] { (grid, W.tblGridChange), (tableProperties, W.tblPrChange) } + .Concat(cellProperties.Select(properties => (properties, W.tcPrChange))), + cellAnchorId) is { } pending) + return pending; + var oldGrid = tracked ? new XElement(grid ?? new XElement(W.tblGrid)) : null; + var oldTableProperties = tracked ? new XElement(tableProperties ?? new XElement(W.tblPr)) : null; + var oldCellProperties = tracked ? tbl.Descendants(W.tc).ToDictionary(cell => cell, + cell => new XElement(cell.Element(W.tcPr) ?? new XElement(W.tcPr))) : null; + _history.RecordPreOp(TakeSnapshot()); try { @@ -10553,6 +10689,17 @@ public EditResult SetColumnWidths(string cellAnchorId, IReadOnlyList widths new XElement(W.tblLayout, new XAttribute(W.type, "fixed")), TblPrChildOrder); + if (tracked) + { + var author = _revisionAuthor ?? "docxodus"; + var date = NextTrackedFormatRevisionDate(); + TrackPropertyMutation(grid, oldGrid!, W.tblGridChange, author, date); + TrackPropertyMutation(tblPr, oldTableProperties!, W.tblPrChange, author, date); + foreach (var pair in oldCellProperties!) + TrackPropertyMutation(GetOrCreateTcPr(pair.Key), pair.Value, + W.tcPrChange, author, date, W.cellIns, W.cellDel, W.cellMerge); + } + return TableStyleResult(target!, CompleteTableMapping(before, tbl)); } catch (Exception ex) @@ -10581,6 +10728,13 @@ public EditResult SetTableBorders(string cellAnchorId, TableBorderSpec? spec = n return EditResult.Fail(EditErrorCode.InvalidTableStyling, "border size (eighths of a point) must be >= 0", cellAnchorId); + var tracked = _trackedChanges == TrackedChangeMode.RenderInline; + var existingTblPr = tbl!.Element(W.tblPr); + if (RefuseNestedTrackedPropertyChange( + new[] { (existingTblPr, W.tblPrChange) }, cellAnchorId) is { } pending) + return pending; + var oldTblPr = tracked ? new XElement(existingTblPr ?? new XElement(W.tblPr)) : null; + _history.RecordPreOp(TakeSnapshot()); try { @@ -10613,6 +10767,10 @@ public EditResult SetTableBorders(string cellAnchorId, TableBorderSpec? spec = n SetChildInOrder(borders, edge, TblBordersEdgeOrder); } + if (tracked) + TrackPropertyMutation(tblPr, oldTblPr!, W.tblPrChange, + _revisionAuthor ?? "docxodus", NextTrackedFormatRevisionDate()); + return TableStyleResult(target!); } catch (Exception ex) @@ -10651,10 +10809,18 @@ public EditResult SetCellShading(string cellAnchorId, string? fillColor, else fill = "auto"; } + var cells = scope == TableShadingScope.Row ? tr!.Elements(W.tc).ToList() : new List { tc! }; + if (RefuseNestedTrackedPropertyChange( + cells.Select(cell => (cell.Element(W.tcPr), W.tcPrChange)), + cellAnchorId) is { } pending) + return pending; + var tracked = _trackedChanges == TrackedChangeMode.RenderInline; + var oldCellProperties = tracked ? cells.ToDictionary(cell => cell, + cell => new XElement(cell.Element(W.tcPr) ?? new XElement(W.tcPr))) : null; + _history.RecordPreOp(TakeSnapshot()); try { - var cells = scope == TableShadingScope.Row ? tr!.Elements(W.tc).ToList() : new List { tc! }; foreach (var cell in cells) { if (clear) @@ -10668,6 +10834,17 @@ public EditResult SetCellShading(string cellAnchorId, string? fillColor, TcPrChildOrder); } + if (tracked) + { + var author = _revisionAuthor ?? "docxodus"; + var date = NextTrackedFormatRevisionDate(); + foreach (var pair in oldCellProperties!) + if (!PropertySnapshotEquals(pair.Value, pair.Key.Element(W.tcPr), + W.tcPrChange, W.cellIns, W.cellDel, W.cellMerge)) + TrackPropertyMutation(GetOrCreateTcPr(pair.Key), pair.Value, + W.tcPrChange, author, date, W.cellIns, W.cellDel, W.cellMerge); + } + return TableStyleResult(target!); } catch (Exception ex) @@ -10702,6 +10879,13 @@ public EditResult SetTableRowOptions(string cellAnchorId, TableRowOptions? optio return EditResult.Fail(EditErrorCode.InvalidTableStyling, "row height in twips must be >= 0", cellAnchorId); + var tracked = _trackedChanges == TrackedChangeMode.RenderInline; + var existingTrPr = tr!.Element(W.trPr); + if (RefuseNestedTrackedPropertyChange( + new[] { (existingTrPr, W.trPrChange) }, cellAnchorId) is { } pending) + return pending; + var oldTrPr = tracked ? new XElement(existingTrPr ?? new XElement(W.trPr)) : null; + _history.RecordPreOp(TakeSnapshot()); try { @@ -10757,6 +10941,17 @@ public EditResult SetTableRowOptions(string cellAnchorId, TableRowOptions? optio // (Unid) that Save() strips anyway. if (trPr is not null && !trPr.HasElements) trPr.Remove(); + if (tracked) + { + trPr = tr.Element(W.trPr); + if (!PropertySnapshotEquals(oldTrPr!, trPr, W.trPrChange, W.ins, W.del)) + { + if (trPr is null) { trPr = new XElement(W.trPr); tr.AddFirst(trPr); } + TrackPropertyMutation(trPr, oldTrPr!, W.trPrChange, + _revisionAuthor ?? "docxodus", NextTrackedFormatRevisionDate(), W.ins, W.del); + } + } + return TableStyleResult(target!); } catch (Exception ex) @@ -10767,13 +10962,13 @@ public EditResult SetTableRowOptions(string cellAnchorId, TableRowOptions? optio } } - private EditResult? RefuseNestedTrackedListChange(XElement paragraph, string anchorId) + private EditResult? RefuseNestedTrackedParagraphPropertyChange(XElement paragraph, string anchorId) { if (_trackedChanges != TrackedChangeMode.RenderInline) return null; return paragraph.Element(W.pPr)?.Element(W.pPrChange) is null ? null : EditResult.Fail(EditErrorCode.UnresolvedStructuralRevision, - "paragraph has an unresolved property revision; resolve it before another tracked list mutation", + "paragraph has an unresolved property revision; resolve it before another tracked property mutation", anchorId); } @@ -10810,7 +11005,7 @@ public EditResult SetListLevel(string anchorId, int levelDelta) var element = target.Resolve(_doc!); if (element is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "element null", anchorId); - if (RefuseNestedTrackedListChange(element, anchorId) is { } pending) return pending; + if (RefuseNestedTrackedParagraphPropertyChange(element, anchorId) is { } pending) return pending; var pPr = element.Element(W.pPr); var numPr = pPr?.Element(W.numPr); @@ -10916,7 +11111,7 @@ public EditResult RemoveListMembership(string anchorId) "RemoveListMembership requires a paragraph, heading, or list-item anchor", anchorId); var element = target.Resolve(_doc!); if (element is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "element null", anchorId); - if (RefuseNestedTrackedListChange(element, anchorId) is { } pending) return pending; + if (RefuseNestedTrackedParagraphPropertyChange(element, anchorId) is { } pending) return pending; var pPr = element.Element(W.pPr); var oldPPr = new XElement(pPr ?? new XElement(W.pPr)); @@ -10964,7 +11159,7 @@ public EditResult ApplyListFormat(string anchorId, ListFormat kind) return EditResult.Fail(EditErrorCode.AnchorWrongKind, "ApplyListFormat requires a paragraph anchor", anchorId); var element = target.Resolve(_doc!); if (element is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "element null", anchorId); - if (RefuseNestedTrackedListChange(element, anchorId) is { } pending) return pending; + if (RefuseNestedTrackedParagraphPropertyChange(element, anchorId) is { } pending) return pending; var oldPPr = new XElement(element.Element(W.pPr) ?? new XElement(W.pPr)); bool insertedNumPr = oldPPr.Element(W.numPr) is null && kind != ListFormat.None; @@ -11117,8 +11312,8 @@ public EditResult ApplyListFormatRange(string firstAnchorId, string lastAnchorId /// Restart (or seed) the anchored list item's numbering at — Word's /// Set Numbering Value… → Set value to (issue #314). Writes a /// w:lvlOverride[@w:ilvl]/w:startOverride[@w:val] on a DEDICATED w:num instance: - /// the item's current num is cloned (never mutated — it may be shared, and the numbering part - /// is not snapshotted for undo), and the anchored paragraph plus every FOLLOWING paragraph of + /// the item's current num is cloned (never mutated because it may be shared), and the anchored + /// paragraph plus every FOLLOWING paragraph of /// the same numbering instance in the part is repointed at the clone. An anchored item /// mid-sequence therefore splits the sequence exactly like Word: earlier items keep their /// numbers, the anchored item shows , and the tail continues from it. @@ -11624,6 +11819,8 @@ internal void InvalidateProjectionCache() /// AddComment creates on a document that had no comments. /// The same, for commentsExtended/commentsIds, which /// AddCommentReply/SetCommentResolved create when upgrading a flat comment. + /// The numbering relationship/URI when present, so undo and + /// tracked rejection can remove a part created by a list mutation or recreate one on redo. internal sealed record DocumentSnapshot( long Version, System.Collections.Generic.IReadOnlyList<(string PartUri, XDocument Xml)> Parts, @@ -11631,6 +11828,7 @@ internal sealed record DocumentSnapshot( System.Collections.Generic.IReadOnlyList<(string RelId, bool IsFootnote, string PartUri)> NoteParts, System.Collections.Generic.IReadOnlyList<(string RelId, string PartUri)> CommentParts, System.Collections.Generic.IReadOnlyList<(string RelId, bool IsCommentsEx, string PartUri)> CommentThreadingParts, + System.Collections.Generic.IReadOnlyList<(string RelId, string PartUri)> NumberingParts, System.Collections.Generic.IReadOnlyList<(string PartUri, string RelId, string Uri, bool IsExternal)> HyperlinkRelationships, System.Collections.Generic.IReadOnlyList<(string PartUri, string ContentType, byte[] Bytes)> ImageParts, System.Collections.Generic.IReadOnlyList<(string OwnerPartUri, string RelId, string TargetPartUri)> ImageRelationships, @@ -11671,6 +11869,7 @@ internal DocumentSnapshot TakeSnapshot() var noteParts = new System.Collections.Generic.List<(string, bool, string)>(); var commentParts = new System.Collections.Generic.List<(string, string)>(); var commentThreadingParts = new System.Collections.Generic.List<(string, bool, string)>(); + var numberingParts = new System.Collections.Generic.List<(string, string)>(); var hyperlinkRelationships = new System.Collections.Generic.List<(string, string, string, bool)>(); var imageParts = new System.Collections.Generic.List<(string, string, byte[])>(); var imageRelationships = new System.Collections.Generic.List<(string, string, string)>(); @@ -11692,6 +11891,9 @@ internal DocumentSnapshot TakeSnapshot() if (main.WordprocessingCommentsIdsPart is not null) commentThreadingParts.Add((main.GetIdOfPart(main.WordprocessingCommentsIdsPart), false, main.WordprocessingCommentsIdsPart.Uri.ToString())); + if (main.NumberingDefinitionsPart is not null) + numberingParts.Add((main.GetIdOfPart(main.NumberingDefinitionsPart), + main.NumberingDefinitionsPart.Uri.ToString())); } foreach (var owner in Internal.OwnedPartRelationships.StoryParts(_doc!)) { @@ -11710,8 +11912,8 @@ internal DocumentSnapshot TakeSnapshot() linkedImageRelationships.Add((owner.PartUri, relationship.Id, relationship.Uri.ToString())); } return new DocumentSnapshot(_version, parts, hfParts, noteParts, commentParts, - commentThreadingParts, hyperlinkRelationships, imageParts, imageRelationships, - linkedImageRelationships); + commentThreadingParts, numberingParts, hyperlinkRelationships, imageParts, + imageRelationships, linkedImageRelationships); } /// @@ -11729,6 +11931,7 @@ internal DocumentSnapshot TakePackageSnapshot() Array.Empty<(string RelId, bool IsFootnote, string PartUri)>(), Array.Empty<(string RelId, string PartUri)>(), Array.Empty<(string RelId, bool IsCommentsEx, string PartUri)>(), + Array.Empty<(string RelId, string PartUri)>(), Array.Empty<(string PartUri, string RelId, string Uri, bool IsExternal)>(), Array.Empty<(string PartUri, string ContentType, byte[] Bytes)>(), Array.Empty<(string OwnerPartUri, string RelId, string TargetPartUri)>(), @@ -11885,6 +12088,7 @@ internal void RestoreSnapshot(DocumentSnapshot snapshot) // Reply/resolve can introduce commentsExtended/commentsIds; reconcile their topology // after restoring the base comments part. ReconcileCommentThreadingParts(main, snapshot, byUri); + ReconcileNumberingPart(main, snapshot, byUri); } RestoreHyperlinkRelationships(snapshot); @@ -12108,6 +12312,32 @@ private static void ReconcileCommentThreadingParts( } } + /// Restore numbering-part topology as well as content. In particular, rejecting or + /// undoing the first tracked list mutation in a document must remove the newly-created part; + /// redo recreates it with its original relationship id. + private static void ReconcileNumberingPart( + MainDocumentPart main, DocumentSnapshot snapshot, + System.Collections.Generic.Dictionary byUri) + { + var snapshotPart = snapshot.NumberingParts.FirstOrDefault(); + var live = main.NumberingDefinitionsPart; + + if (live is not null + && (snapshotPart.RelId is null + || !string.Equals(main.GetIdOfPart(live), snapshotPart.RelId, StringComparison.Ordinal))) + { + main.DeletePart(live); + live = null; + } + + if (live is null && snapshotPart.RelId is not null + && byUri.TryGetValue(snapshotPart.PartUri, out var xml)) + { + var restored = main.AddNewPart(snapshotPart.RelId); + restored.PutXDocument(new XDocument(xml)); + } + } + internal int NextRevisionId() => System.Threading.Interlocked.Increment(ref _revisionCounter); private void ThrowIfDisposed() diff --git a/Docxodus/Internal/NumberingFactory.cs b/Docxodus/Internal/NumberingFactory.cs index 41e949ff..7ad60349 100644 --- a/Docxodus/Internal/NumberingFactory.cs +++ b/Docxodus/Internal/NumberingFactory.cs @@ -13,8 +13,8 @@ namespace Docxodus.Internal; /// / /// use this when no suitable numbering exists. Definitions are tagged with a fixed marker /// w:nsid per format and resolved find-or-create, so the op is idempotent across calls, -/// save/reopen, and undo (the numbering part is not snapshotted; the paragraph's w:numPr -/// is). +/// save/reopen, and undo/redo. Session snapshots cover both the numbering part and paragraph +/// w:numPr references. /// internal static class NumberingFactory { @@ -40,6 +40,14 @@ internal static class NumberingFactory _ => throw new ArgumentOutOfRangeException(nameof(fmt), fmt, "no numbering definition for this format"), }; + internal static bool IsDocxodusDefinition(XElement abstractNum) + { + var nsid = (string?)abstractNum.Element(W + "nsid")?.Attribute(W + "val"); + return Enum.GetValues() + .Where(format => format != ListFormat.None) + .Any(format => string.Equals(nsid, NsidFor(format), StringComparison.OrdinalIgnoreCase)); + } + // Standard Word bullet cycle (•, o, ▪) for synthesized nested levels — same glyph/font set // BuildAbstractNum emits for our own multi-level lists, so source and synthesized lists nest // identically. @@ -222,10 +230,9 @@ private static XElement BuildLevel(string numFmtToken, bool paren, int lvl, stri /// new numId, or null when resolves to no w:num. /// /// - /// Additive-only ON PURPOSE: the source num is never mutated, because the numbering part is - /// not snapshotted (see the class remarks) — the caller repoints the affected paragraphs' - /// w:numPr at the clone, which IS snapshotted, so undo restores the paragraphs and - /// merely strands the clone unreferenced (harmless, like an undone ). + /// Additive-only ON PURPOSE: the source num may be shared by paragraphs outside the requested + /// sequence, so the caller repoints only the affected paragraphs' w:numPr at a clone. + /// Session snapshots restore both the paragraph references and the numbering definition. /// public static int? CloneNumWithStartOverride(WordprocessingDocument doc, int numId, int ilvl, int? value) { diff --git a/Docxodus/Internal/RevisionOps.cs b/Docxodus/Internal/RevisionOps.cs index 73b1f463..753061aa 100644 --- a/Docxodus/Internal/RevisionOps.cs +++ b/Docxodus/Internal/RevisionOps.cs @@ -120,6 +120,12 @@ internal sealed class RevisionCommentTarget W.customXmlMoveToRangeStart, W.customXmlMoveToRangeEnd, }; + private static readonly HashSet MoveRangeNames = new() + { + W.moveFromRangeStart, W.moveFromRangeEnd, + W.moveToRangeStart, W.moveToRangeEnd, + }; + private static readonly HashSet PropsChangeNames = new() { W.rPrChange, W.pPrChange, W.sectPrChange, W.tblPrChange, @@ -178,6 +184,17 @@ private static void ValidateGroups(List groups) continue; } + if (group.Units.Any(u => !string.IsNullOrEmpty(u.NativeId) && u.Wid is null) + || group.RangeMarkers.Any(marker => + !long.TryParse((string?)marker.Attribute(W.id), out _))) + { + group.ResolutionStatus = RevisionResolutionStatus.Malformed; + group.Diagnostic = new RevisionDiagnostic( + "invalid_revision_id", + "A live revision marker has a nonnumeric w:id and cannot be addressed safely."); + continue; + } + if (group.Family == RevisionFamily.CellInsert || group.Family == RevisionFamily.CellDelete || group.Family == RevisionFamily.CellMerge) @@ -636,6 +653,7 @@ private static void BuildGroups(List units, WalkCtx ctx, int partI var moveGroups = new Dictionary(StringComparer.Ordinal); var rowGroupByTr = new Dictionary(); var cellGroups = new List(); + var tablePropertyGroups = new List(); RevisionGroup? cur = null; RevisionGroup? lastRowGroup = null; @@ -691,6 +709,7 @@ private static void BuildGroups(List units, WalkCtx ctx, int partI if (u.Kind == UnitKind.RowMark) { if (lastRowGroup is not null && lastRowGroup.Type == u.Type && lastRowGroup.Author == u.Author + && lastRowGroup.Date == u.Date && lastRowGroup.Units[^1].MarkedRow is { } prevTr && u.MarkedRow is { } tr2 && prevTr.Parent == tr2.Parent && OnlyIgnorableBetween(prevTr, tr2)) { @@ -715,7 +734,15 @@ private static void BuildGroups(List units, WalkCtx ctx, int partI if (u.Kind == UnitKind.PropsChange) { - if (cur is not null && cur.Type == TypeFormat && cur.Author == u.Author + var tablePropertyGroup = u.Table is null ? null : tablePropertyGroups.FirstOrDefault(group => + group.Author == u.Author && group.Date == u.Date + && ReferenceEquals(group.Units[0].Table, u.Table)); + if (tablePropertyGroup is not null) + { + tablePropertyGroup.Units.Add(u); + cur = null; + } + else if (cur is not null && cur.Type == TypeFormat && cur.Author == u.Author && cur.Date == u.Date && cur.Units[^1].Element.Name == W.rPrChange && u.Element.Name == W.rPrChange && AdjacentFormatRuns(cur.Units[^1].Element, u.Element)) @@ -726,12 +753,14 @@ private static void BuildGroups(List units, WalkCtx ctx, int partI { cur = NewGroup(u, partIndex); groups.Add(cur); + if (u.Table is not null) tablePropertyGroups.Add(cur); } continue; } // Content / paragraph-mark insert, delete, or unranged move — adjacency grouping. if (cur is not null && cur.Type == u.Type && cur.Author == u.Author + && cur.Date == u.Date && cur.Units[^1].Element.Name == u.Element.Name && Contiguous(cur.Units[^1], u)) { @@ -904,6 +933,62 @@ private static void AddUnsupportedGroups(XElement root, int partIndex, List group.PartIndex == partIndex) + .SelectMany(group => group.Units.Select(unit => unit.Element) + .Concat(group.RangeMarkers)) + .ToHashSet(); + foreach (var marker in root.Descendants() + .Where(IsRecognizedRevisionMarker) + .Where(marker => !represented.Contains(marker)) + .Where(marker => !marker.Ancestors().Any(ancestor => PropsChangeNames.Contains(ancestor.Name)))) + { + var type = marker.Name == W.ins ? TypeInsert + : marker.Name == W.del ? TypeDelete + : marker.Name == W.moveFrom || marker.Name == W.moveTo + || MoveRangeNames.Contains(marker.Name) ? TypeMove + : PropsChangeNames.Contains(marker.Name) || marker.Name == W.numberingChange + ? TypeFormat + : TypeStructure; + var unit = new RevisionUnit + { + Element = marker, + Kind = UnitKind.Unsupported, + Type = type, + Family = RevisionFamily.Unsupported, + Author = AuthorOf(marker), + Date = (string?)marker.Attribute(W.date), + Paragraph = marker.Ancestors(W.p).FirstOrDefault(), + MarkedCell = marker.Ancestors(W.tc).FirstOrDefault(), + MarkedRow = marker.Ancestors(W.tr).FirstOrDefault(), + Table = marker.Ancestors(W.tbl).FirstOrDefault(), + Wid = WidOf(marker), + NativeId = (string?)marker.Attribute(W.id), + }; + var group = NewGroup(unit, partIndex); + group.ResolutionStatus = RevisionResolutionStatus.Unsupported; + group.Diagnostic = new RevisionDiagnostic( + "unsupported_revision_family", + $"{marker.Name} is recognized tracked-change markup but cannot be selectively resolved."); + groups.Add(group); + represented.Add(marker); + } + } + + private static bool IsRecognizedRevisionMarker(XElement element) + { + var name = element.Name; + return RevWrapperNames.Contains(name) + || MoveRangeNames.Contains(name) + || StructuredRangeNames.Contains(name) + || UnsupportedRangeNames.Contains(name) + || PropsChangeNames.Contains(name) + || name == W.cellIns || name == W.cellDel || name == W.cellMerge + || name == W.numberingChange; } /// @@ -1275,6 +1360,15 @@ internal static List Apply(RevisionGroup g, bool accept) ResolveCellStructure(g, accept, removedBlocks); + // If resolving this revision removes an SDT envelope, expose its paragraphs before + // resolving their pilcrows. A last paragraph inside w:sdtContent can then coalesce with + // the following body paragraph instead of being mistaken for the end of its container + // and surviving as an empty husk. Range markers are transparent revision scaffolding and + // must likewise be gone before paragraph adjacency is evaluated. + ResolveStructuredWrapper(g, accept, removedBlocks); + foreach (var marker in g.RangeMarkers) + if (!Detached(marker)) marker.Remove(); + // Paragraph marks last, in reverse document order, so multi-paragraph coalescing // cascades into the single surviving paragraph exactly as RevisionProcessor's // grouped transform does. @@ -1296,18 +1390,25 @@ internal static List Apply(RevisionGroup g, bool accept) } else { - // No following paragraph to coalesce into (last block of its - // container) — the mark cannot go away; strip the revision. - u.Element.Remove(); + // A wholly inserted/deleted final paragraph has no successor whose mark can + // survive. Once its revised content is gone, remove the empty block itself; + // a paragraph with unrelated surviving content keeps its unavoidable mark. + if (!u.Paragraph.Elements().Any(element => element.Name != W.pPr + && !IsIgnorableBetween(element))) + { + var paragraph = u.Paragraph; + paragraph.Remove(); + removedBlocks.Add(paragraph); + touchedParagraphs.Remove(paragraph); + } + else + { + u.Element.Remove(); + } } } } - ResolveStructuredWrapper(g, accept, removedBlocks); - - foreach (var m in g.RangeMarkers) - if (!Detached(m)) m.Remove(); - foreach (var p in touchedParagraphs) { if (Detached(p)) continue; @@ -1662,9 +1763,10 @@ private static void AcceptProps(XElement change) { var parent = change.Parent; change.Remove(); - if (parent is not null && !parent.HasElements && !parent.HasAttributes + if (parent is not null && !parent.HasElements && HasNoSemanticAttributes(parent) && (parent.Name == W.rPr || parent.Name == W.pPr || parent.Name == W.trPr - || parent.Name == W.tblPrEx)) + || parent.Name == W.tblPr || parent.Name == W.tcPr + || parent.Name == W.tblGrid || parent.Name == W.tblPrEx)) { parent.Remove(); } @@ -1725,8 +1827,9 @@ private static void RejectProps(XElement change) // A change whose stored old property set was empty leaves an empty husk — // remove it (Word writes no empty rPr/pPr), mirroring AcceptProps. - if (!parent.HasElements && !parent.HasAttributes - && (pn == W.rPr || pn == W.pPr || pn == W.trPr || pn == W.tblPrEx)) + if (!parent.HasElements && HasNoSemanticAttributes(parent) + && (pn == W.rPr || pn == W.pPr || pn == W.trPr + || pn == W.tblPr || pn == W.tcPr || pn == W.tblGrid || pn == W.tblPrEx)) { parent.Remove(); } diff --git a/tools/mcp-server/Dispatcher.cs b/tools/mcp-server/Dispatcher.cs index 1e3bd860..fdd7c56a 100644 --- a/tools/mcp-server/Dispatcher.cs +++ b/tools/mcp-server/Dispatcher.cs @@ -687,6 +687,11 @@ private static string TrackChanges(SessionStore store, JsonElement args) { var session = Session(store, args); var action = Str(args, "action"); + return RunTrackChangesAction(session, action, args); + } + + private static string RunTrackChangesAction(DocSession session, string action, JsonElement args) + { switch (action) { case "list": @@ -874,6 +879,7 @@ private static IReadOnlyList BuildMutationBatchSteps( "docxodus_comment" => RunCommentAction(session, action, mutationArgs), "docxodus_links" => RunLinksAction(session, action, mutationArgs), "docxodus_images" => RunImagesAction(session, action, mutationArgs), + "docxodus_track_changes" => RunTrackChangesAction(session, action, mutationArgs), _ => throw new McpToolException($"docxodus_mutations does not accept \"{stepTool}\" as a step"), }, () => ValidateMutationBatchStep(session, stepTool, action, stepArgs))); @@ -906,6 +912,8 @@ private static IReadOnlyList BuildMutationBatchSteps( or "add_bookmark" or "move_bookmark" or "rename_bookmark" or "remove_bookmark", "docxodus_images" => action is "insert" or "replace" or "set_dimensions" or "set_metadata" or "set_floating_layout" or "remove", + "docxodus_track_changes" => action is "accept" or "reject" + or "accept_all" or "reject_all", _ => false, }; return known ? null : new EditError( @@ -1199,6 +1207,14 @@ private static void ValidateMutationBatchArguments(string tool, string action, J case ("docxodus_images", "remove"): RequireStrings(args, "imageId"); break; + + case ("docxodus_track_changes", "accept"): + case ("docxodus_track_changes", "reject"): + RequireStrings(args, "revisionId"); + break; + case ("docxodus_track_changes", "accept_all"): + case ("docxodus_track_changes", "reject_all"): + break; } } diff --git a/tools/mcp-server/ToolCatalog.cs b/tools/mcp-server/ToolCatalog.cs index 556d9857..8da54f24 100644 --- a/tools/mcp-server/ToolCatalog.cs +++ b/tools/mcp-server/ToolCatalog.cs @@ -476,7 +476,7 @@ internal static class ToolCatalog """), new ToolDefinition( "docxodus_mutations", - "Apply or safely preview a batch of mutating edit/format/create/table/list/comment/link/image actions. Atomic mode commits as one unit; preview executes the identical batch path against an isolated complete package clone and never mutates the live session or its undo/redo history.", + "Apply or safely preview a batch of mutating edit/format/create/table/list/comment/link/image/track-changes actions. Atomic mode commits as one unit; preview executes the identical batch path against an isolated complete package clone and never mutates the live session or its undo/redo history.", """ { "type": "object", @@ -493,7 +493,7 @@ internal static class ToolCatalog "items": { "type": "object", "properties": { - "tool": { "type": "string", "enum": ["docxodus_edit", "docxodus_format", "docxodus_create", "docxodus_table", "docxodus_list", "docxodus_comment", "docxodus_links", "docxodus_images"] }, + "tool": { "type": "string", "enum": ["docxodus_edit", "docxodus_format", "docxodus_create", "docxodus_table", "docxodus_list", "docxodus_comment", "docxodus_links", "docxodus_images", "docxodus_track_changes"] }, "args": { "type": "object", "description": "The same arguments that tool's action takes, minus sessionId (inherited from the batch)." } }, "required": ["tool", "args"] From 4934188e6664044cddb1b80dd290fbd44eb8b281 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Fri, 14 Aug 2026 05:42:06 -0500 Subject: [PATCH 3/3] Harden structural revision audit edges --- .../DocxSessionStructuralRevisionTests.cs | 352 ++++++++++++++++++ Docxodus/DocxSession.cs | 58 ++- Docxodus/Internal/DocxSessionOps.cs | 3 - Docxodus/Internal/NumberingFactory.cs | 41 ++ Docxodus/Internal/RevisionOps.cs | 14 +- Docxodus/Internal/StyleFactory.cs | 19 +- docs/architecture/docx_agent_server.md | 4 +- tools/mcp-server/README.md | 9 +- tools/mcp-server/SessionStore.cs | 36 +- 9 files changed, 480 insertions(+), 56 deletions(-) diff --git a/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs b/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs index a0e19fa6..df59e059 100644 --- a/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs +++ b/Docxodus.Tests/DocxSessionStructuralRevisionTests.cs @@ -618,6 +618,358 @@ public void DS45518_RejectTrackedListCreation_RestoresPackageAndUndoRedo(bool bu Assert.Empty(session.ListRevisions()); } + [Theory] + [InlineData("level")] + [InlineData("switch_format")] + public void DS45519_TrackedListMutation_RequiringNumberingSidePartChangeFailsClosed( + string operation) + { + var baseline = BuildSingleLevelBulletDocument(); + using var session = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + }); + var anchor = FirstBodyTextAnchor(session); + var operationBaseline = session.Save(); + var beforeNumbering = NumberingPartBytes(operationBaseline); + + var result = operation == "level" + ? session.SetListLevel(anchor, 1) + : session.ApplyListFormat(anchor, ListFormat.Decimal); + + Assert.False(result.Success); + Assert.Equal(EditErrorCode.TrackedOperationUnsupported, result.Error!.Code); + Assert.Empty(session.ListRevisions()); + var after = session.Save(); + Assert.True(beforeNumbering.SequenceEqual(NumberingPartBytes(after))); + Assert.True(XNode.DeepEquals(MainRoot(operationBaseline), MainRoot(after))); + PackageEquivalence.AssertSamePackage( + new WmlDocument("baseline.docx", operationBaseline), + new WmlDocument("after.docx", after)); + Assert.False(session.Undo()); + } + + [Fact] + public void DS45520_TrackedListFormat_UsingExistingDefinitionRejectsWithPackageParity() + { + var baseline = BuildBulletAndDecimalDocument(); + using var session = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + RevisionAuthor = "List Reviewer", + }); + var anchor = FirstBodyTextAnchor(session); + var operationBaseline = session.Save(); + var beforeNumbering = NumberingPartBytes(operationBaseline); + + var edit = session.ApplyListFormat(anchor, ListFormat.Decimal); + + Assert.True(edit.Success, edit.Error?.Message); + var revision = Assert.Single(session.ListRevisions()); + Assert.Equal(RevisionFamily.PropertiesChange, revision.Family); + Assert.True(session.RejectRevision(revision.Id).Success); + var after = session.Save(); + Assert.True(beforeNumbering.SequenceEqual(NumberingPartBytes(after))); + Assert.True(XNode.DeepEquals(MainRoot(operationBaseline), MainRoot(after))); + Assert.Equal(PackagePartUris(operationBaseline), PackagePartUris(after)); + } + + [Fact] + public void DS45521_TrackedParagraphStyleRequiresExistingDefinitionAndStyleTopologyUndoes() + { + var baseline = BuildDocumentWithoutStylesPart(); + Assert.False(HasStylesPart(baseline)); + using (var tracked = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + })) + { + var anchor = FirstBodyTextAnchor(tracked); + var operationBaseline = tracked.Save(); + var result = tracked.SetParagraphStyle(anchor, "Heading2"); + + Assert.False(result.Success); + Assert.Equal(EditErrorCode.TrackedOperationUnsupported, result.Error!.Code); + Assert.Empty(tracked.ListRevisions()); + var after = tracked.Save(); + Assert.False(HasStylesPart(after)); + Assert.True(XNode.DeepEquals(MainRoot(operationBaseline), MainRoot(after))); + PackageEquivalence.AssertSamePackage( + new WmlDocument("baseline.docx", operationBaseline), + new WmlDocument("after.docx", after)); + Assert.False(tracked.Undo()); + } + + using var direct = new DocxSession(baseline); + Assert.True(direct.SetParagraphStyle( + FirstBodyTextAnchor(direct), "Heading2").Success); + Assert.True(HasStylesPart(direct.Save())); + Assert.True(direct.Undo()); + Assert.False(HasStylesPart(direct.Save())); + Assert.True(direct.Redo()); + Assert.True(HasStylesPart(direct.Save())); + } + + [Fact] + public void DS45522_TrackedParagraphStyleRejectDoesNotChangeStylesPart() + { + var baseline = DocxSessionTests.BuildDS001_SimpleTwoParagraphs(); + using var session = new DocxSession(baseline, new DocxSessionSettings + { + TrackedChanges = TrackedChangeMode.RenderInline, + }); + var anchor = FirstBodyTextAnchor(session); + var operationBaseline = session.Save(); + var beforeStyles = StylesPartBytes(operationBaseline); + + Assert.True(session.SetParagraphStyle(anchor, "Heading2").Success); + var revision = Assert.Single(session.ListRevisions()); + Assert.True(session.RejectRevision(revision.Id).Success); + + var after = session.Save(); + Assert.True(beforeStyles.SequenceEqual(StylesPartBytes(after))); + Assert.True(XNode.DeepEquals(MainRoot(operationBaseline), MainRoot(after))); + Assert.Equal(PackagePartUris(operationBaseline), PackagePartUris(after)); + } + + [Theory] + [InlineData("del_text", true)] + [InlineData("del_text", false)] + [InlineData("del_instr_text", true)] + [InlineData("del_instr_text", false)] + public void DS45523_OrphanDeletedPayload_IsListedAndBlocksBulk( + string shape, bool accept) + { + var input = BuildOrphanDeletedPayloadDocument(shape); + using var session = new DocxSession(input); + var before = MainRoot(session.Save()); + var revision = Assert.Single(session.ListRevisions()); + Assert.Equal(RevisionFamily.Unsupported, revision.Family); + Assert.Equal(RevisionResolutionStatus.Unsupported, revision.ResolutionStatus); + Assert.Equal("unsupported_revision_family", revision.Diagnostic!.Code); + + var result = accept + ? session.AcceptAllRevisions() + : session.RejectAllRevisions(); + + Assert.False(result.Success); + Assert.Equal(EditErrorCode.RevisionUnsupported, result.Error!.Code); + Assert.True(XNode.DeepEquals(before, MainRoot(session.Save()))); + Assert.False(session.Undo()); + } + + [Theory] + [InlineData("del_text")] + [InlineData("del_instr_text")] + public void DS45524_DeletedPayloadInsideClaimedWrapper_IsNotDoubleCounted(string shape) + { + using var session = new DocxSession(BuildOrdinaryDeletionDocument(shape)); + + var revision = Assert.Single(session.ListRevisions()); + + Assert.Equal(RevisionFamily.ContentDelete, revision.Family); + Assert.Equal(RevisionResolutionStatus.Supported, revision.ResolutionStatus); + } + + [Theory] + [InlineData("outer_accept_inner_accept")] + [InlineData("inner_accept_outer_accept")] + [InlineData("outer_accept_inner_reject")] + [InlineData("inner_reject_outer_accept")] + public void DS45525_NestedIndependentRevisions_ResolveInEitherOrder(string order) + { + using var session = new DocxSession(BuildNestedRevisionDocument()); + var initial = session.ListRevisions(); + Assert.Equal(2, initial.Count); + var outer = Assert.Single(initial, revision => + revision.ConstituentIds.SequenceEqual(new[] { "901" })); + var inner = Assert.Single(initial, revision => + revision.ConstituentIds.SequenceEqual(new[] { "902" })); + bool innerAccepted = order.Contains("inner_accept", StringComparison.Ordinal); + + if (order.StartsWith("outer", StringComparison.Ordinal)) + { + Assert.True(session.AcceptRevision(outer.Id).Success); + var remaining = Assert.Single(session.ListRevisions()); + Assert.Equal(inner.Id, remaining.Id); + Assert.True((innerAccepted + ? session.AcceptRevision(remaining.Id) + : session.RejectRevision(remaining.Id)).Success); + } + else + { + Assert.True((innerAccepted + ? session.AcceptRevision(inner.Id) + : session.RejectRevision(inner.Id)).Success); + var remaining = Assert.Single(session.ListRevisions()); + Assert.Equal(outer.Id, remaining.Id); + Assert.True(session.AcceptRevision(remaining.Id).Success); + } + + Assert.Empty(session.ListRevisions()); + Assert.Equal(innerAccepted ? "outer-before outer-after" : "outer-before inner outer-after", + MainRoot(session.Save()).Descendants(W.p).First().Value); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void DS45526_NestedIndependentRevisions_BulkMatchesProcessor(bool accept) + { + var input = BuildNestedRevisionDocument(); + var expected = accept + ? RevisionProcessor.AcceptRevisions(new WmlDocument("accept.docx", input)).DocumentByteArray + : RevisionProcessor.RejectRevisions(new WmlDocument("reject.docx", input)).DocumentByteArray; + using var session = new DocxSession(input); + + var result = accept + ? session.AcceptAllRevisions() + : session.RejectAllRevisions(); + + Assert.True(result.Success, result.Error?.Message); + Assert.Empty(session.ListRevisions()); + var actual = session.Save(); + Assert.True(XNode.DeepEquals(MainRoot(expected), MainRoot(actual)), + FirstDifference(MainRoot(expected), MainRoot(actual))); + } + + private static byte[] BuildSingleLevelBulletDocument() + { + byte[] listed; + using (var session = new DocxSession(DocxSessionTests.BuildDS001_SimpleTwoParagraphs())) + { + Assert.True(session.ApplyListFormat( + FirstBodyTextAnchor(session), ListFormat.Bullet).Success); + listed = session.Save(); + } + + using var stream = new MemoryStream(); + stream.Write(listed); + stream.Position = 0; + using (var document = WordprocessingDocument.Open(stream, true)) + { + var numbering = document.MainDocumentPart!.NumberingDefinitionsPart!; + var root = numbering.GetXDocument().Root!; + var abstractNum = Assert.Single(root.Elements(W.abstractNum)); + foreach (var extra in abstractNum.Elements(W.lvl).Skip(1).ToList()) extra.Remove(); + var multiLevelType = abstractNum.Element(W.multiLevelType); + if (multiLevelType is null) + abstractNum.AddFirst(new XElement(W.multiLevelType, new XAttribute(W.val, "singleLevel"))); + else + multiLevelType.SetAttributeValue(W.val, "singleLevel"); + numbering.PutXDocument(); + } + return stream.ToArray(); + } + + private static byte[] BuildBulletAndDecimalDocument() + { + using var session = new DocxSession(DocxSessionTests.BuildDS001_SimpleTwoParagraphs()); + var anchors = session.Project().AnchorIndex.Values + .Where(anchor => anchor.Anchor.Scope == "body" && anchor.Anchor.Kind == "p") + .Select(anchor => anchor.Anchor.Id).Take(2).ToArray(); + Assert.True(session.ApplyListFormat(anchors[0], ListFormat.Bullet).Success); + Assert.True(session.ApplyListFormat(anchors[1], ListFormat.Decimal).Success); + return session.Save(); + } + + private static byte[] BuildDocumentWithoutStylesPart() + { + using var stream = new MemoryStream(); + stream.Write(DocxSessionTests.BuildDS001_SimpleTwoParagraphs()); + stream.Position = 0; + using (var document = WordprocessingDocument.Open(stream, true)) + { + var main = document.MainDocumentPart!; + if (main.StyleDefinitionsPart is { } styles) main.DeletePart(styles); + } + return stream.ToArray(); + } + + private static byte[] BuildOrphanDeletedPayloadDocument(string shape) => + MutateMain(DocxSessionTests.BuildDS001_SimpleTwoParagraphs(), root => + { + var name = shape switch + { + "del_text" => W.delText, + "del_instr_text" => W.delInstrText, + _ => throw new ArgumentOutOfRangeException(nameof(shape)), + }; + root.Descendants(W.p).First().ReplaceNodes( + new XElement(W.r, new XElement(name, "orphan deleted payload"))); + }); + + private static byte[] BuildOrdinaryDeletionDocument(string shape) => + MutateMain(DocxSessionTests.BuildDS001_SimpleTwoParagraphs(), root => + { + var name = shape switch + { + "del_text" => W.delText, + "del_instr_text" => W.delInstrText, + _ => throw new ArgumentOutOfRangeException(nameof(shape)), + }; + root.Descendants(W.p).First().ReplaceNodes( + new XElement(W.del, + new XAttribute(W.id, "900"), + new XAttribute(W.author, "Reviewer"), + new XAttribute(W.date, "2026-01-01T00:00:00Z"), + new XElement(W.r, new XElement(name, "ordinary deletion")))); + }); + + private static byte[] BuildNestedRevisionDocument() => + MutateMain(DocxSessionTests.BuildDS001_SimpleTwoParagraphs(), root => + { + root.Descendants(W.p).First().ReplaceNodes( + new XElement(W.ins, + new XAttribute(W.id, "901"), + new XAttribute(W.author, "Outer Reviewer"), + new XAttribute(W.date, "2026-01-01T00:00:00Z"), + new XElement(W.r, new XElement(W.t, "outer-before ")), + new XElement(W.del, + new XAttribute(W.id, "902"), + new XAttribute(W.author, "Inner Reviewer"), + new XAttribute(W.date, "2026-01-02T00:00:00Z"), + new XElement(W.r, new XElement(W.delText, "inner "))), + new XElement(W.r, new XElement(W.t, "outer-after")))); + }); + + private static byte[] NumberingPartBytes(byte[] bytes) + { + using var stream = new MemoryStream(bytes); + using var document = WordprocessingDocument.Open(stream, false); + using var partStream = document.MainDocumentPart!.NumberingDefinitionsPart!.GetStream(); + using var copy = new MemoryStream(); + partStream.CopyTo(copy); + return copy.ToArray(); + } + + private static bool HasStylesPart(byte[] bytes) + { + using var stream = new MemoryStream(bytes); + using var document = WordprocessingDocument.Open(stream, false); + return document.MainDocumentPart!.StyleDefinitionsPart is not null; + } + + private static byte[] StylesPartBytes(byte[] bytes) + { + using var stream = new MemoryStream(bytes); + using var document = WordprocessingDocument.Open(stream, false); + using var partStream = document.MainDocumentPart!.StyleDefinitionsPart!.GetStream(); + using var copy = new MemoryStream(); + partStream.CopyTo(copy); + return copy.ToArray(); + } + + private static string[] PackagePartUris(byte[] bytes) + { + using var stream = new MemoryStream(bytes); + using var document = WordprocessingDocument.Open(stream, false); + return document.GetPackage().GetParts() + .Select(part => part.Uri.ToString()) + .OrderBy(uri => uri, StringComparer.Ordinal) + .ToArray(); + } + private static EditResult ApplyPropertyMutation(DocxSession session, string operation) { if (operation.StartsWith("paragraph", StringComparison.Ordinal)) diff --git a/Docxodus/DocxSession.cs b/Docxodus/DocxSession.cs index b6190922..2f407a5a 100644 --- a/Docxodus/DocxSession.cs +++ b/Docxodus/DocxSession.cs @@ -7810,8 +7810,14 @@ public EditResult SetParagraphStyle(string anchorId, string styleId) if (element is null) return EditResult.Fail(EditErrorCode.AnchorNotFound, "element null", anchorId); if (RefuseNestedTrackedParagraphPropertyChange(element, anchorId) is { } pending) return pending; + if (_trackedChanges == TrackedChangeMode.RenderInline + && !Internal.StyleFactory.HasParagraphStyle(_doc!, styleId)) + return TrackedStructureUnsupported( + $"SetParagraphStyle requiring synthesis of style '{styleId}'", anchorId); + // Style synthesis mutates the styles part. Capture before it so rejecting the generated - // pPrChange (or undoing this op) restores package state as well as the paragraph XML. + // pPrChange (or undoing this direct-mode op) restores package state as well as paragraph XML. + // Tracked mode preflights above because native pPrChange has no styles-part before-image. var preOp = TakeSnapshot(); // Find-or-create well-known built-in styles (Title, Subtitle, Heading1-9) the document @@ -11036,6 +11042,11 @@ public EditResult SetListLevel(string anchorId, int levelDelta) if (next < 0 || next > 8) return EditResult.Fail(EditErrorCode.InvalidListLevel, $"resulting list level {next} out of [0,8]", anchorId); + if (_trackedChanges == TrackedChangeMode.RenderInline && effectiveNumId.HasValue + && Internal.NumberingFactory.WouldEnsureLevelDefinedMutate( + _doc!, effectiveNumId.Value, next)) + return TrackedStructureUnsupported( + $"SetListLevel requiring numbering level {next} synthesis", anchorId); _history.RecordPreOp(TakeSnapshot()); // Nesting only renders if the abstractNum actually DEFINES the target level — many docs @@ -11163,6 +11174,11 @@ public EditResult ApplyListFormat(string anchorId, ListFormat kind) var oldPPr = new XElement(element.Element(W.pPr) ?? new XElement(W.pPr)); bool insertedNumPr = oldPPr.Element(W.numPr) is null && kind != ListFormat.None; + if (_trackedChanges == TrackedChangeMode.RenderInline + && kind != ListFormat.None && !insertedNumPr + && Internal.NumberingFactory.WouldEnsureNumberingMutate(_doc!, kind)) + return TrackedStructureUnsupported( + $"ApplyListFormat requiring synthesis of {kind} numbering", anchorId); _history.RecordPreOp(TakeSnapshot()); try @@ -11753,10 +11769,6 @@ private void OnHistoryPopUndo(DocumentSnapshot snapshot) _version = snapshot.Version; } - /// Restore the caller-visible version after rolling back speculative preview work. - /// Internal by design: committed undo/redo must remain monotonic. - internal void RestoreVersionAfterRebind(long version) => _version = version; - /// /// Dispose the session and abandon any active transactions. Active scopes may only be /// abandoned by their owner thread; otherwise this throws and leaves the session usable so @@ -11819,6 +11831,8 @@ internal void InvalidateProjectionCache() /// AddComment creates on a document that had no comments. /// The same, for commentsExtended/commentsIds, which /// AddCommentReply/SetCommentResolved create when upgrading a flat comment. + /// The styles relationship/URI when present, so undo can remove a + /// styles part synthesized by a direct-mode style mutation or recreate it on redo. /// The numbering relationship/URI when present, so undo and /// tracked rejection can remove a part created by a list mutation or recreate one on redo. internal sealed record DocumentSnapshot( @@ -11828,6 +11842,7 @@ internal sealed record DocumentSnapshot( System.Collections.Generic.IReadOnlyList<(string RelId, bool IsFootnote, string PartUri)> NoteParts, System.Collections.Generic.IReadOnlyList<(string RelId, string PartUri)> CommentParts, System.Collections.Generic.IReadOnlyList<(string RelId, bool IsCommentsEx, string PartUri)> CommentThreadingParts, + System.Collections.Generic.IReadOnlyList<(string RelId, string PartUri)> StyleParts, System.Collections.Generic.IReadOnlyList<(string RelId, string PartUri)> NumberingParts, System.Collections.Generic.IReadOnlyList<(string PartUri, string RelId, string Uri, bool IsExternal)> HyperlinkRelationships, System.Collections.Generic.IReadOnlyList<(string PartUri, string ContentType, byte[] Bytes)> ImageParts, @@ -11869,6 +11884,7 @@ internal DocumentSnapshot TakeSnapshot() var noteParts = new System.Collections.Generic.List<(string, bool, string)>(); var commentParts = new System.Collections.Generic.List<(string, string)>(); var commentThreadingParts = new System.Collections.Generic.List<(string, bool, string)>(); + var styleParts = new System.Collections.Generic.List<(string, string)>(); var numberingParts = new System.Collections.Generic.List<(string, string)>(); var hyperlinkRelationships = new System.Collections.Generic.List<(string, string, string, bool)>(); var imageParts = new System.Collections.Generic.List<(string, string, byte[])>(); @@ -11891,6 +11907,9 @@ internal DocumentSnapshot TakeSnapshot() if (main.WordprocessingCommentsIdsPart is not null) commentThreadingParts.Add((main.GetIdOfPart(main.WordprocessingCommentsIdsPart), false, main.WordprocessingCommentsIdsPart.Uri.ToString())); + if (main.StyleDefinitionsPart is not null) + styleParts.Add((main.GetIdOfPart(main.StyleDefinitionsPart), + main.StyleDefinitionsPart.Uri.ToString())); if (main.NumberingDefinitionsPart is not null) numberingParts.Add((main.GetIdOfPart(main.NumberingDefinitionsPart), main.NumberingDefinitionsPart.Uri.ToString())); @@ -11912,7 +11931,7 @@ internal DocumentSnapshot TakeSnapshot() linkedImageRelationships.Add((owner.PartUri, relationship.Id, relationship.Uri.ToString())); } return new DocumentSnapshot(_version, parts, hfParts, noteParts, commentParts, - commentThreadingParts, numberingParts, hyperlinkRelationships, imageParts, + commentThreadingParts, styleParts, numberingParts, hyperlinkRelationships, imageParts, imageRelationships, linkedImageRelationships); } @@ -11932,6 +11951,7 @@ internal DocumentSnapshot TakePackageSnapshot() Array.Empty<(string RelId, string PartUri)>(), Array.Empty<(string RelId, bool IsCommentsEx, string PartUri)>(), Array.Empty<(string RelId, string PartUri)>(), + Array.Empty<(string RelId, string PartUri)>(), Array.Empty<(string PartUri, string RelId, string Uri, bool IsExternal)>(), Array.Empty<(string PartUri, string ContentType, byte[] Bytes)>(), Array.Empty<(string OwnerPartUri, string RelId, string TargetPartUri)>(), @@ -12088,6 +12108,7 @@ internal void RestoreSnapshot(DocumentSnapshot snapshot) // Reply/resolve can introduce commentsExtended/commentsIds; reconcile their topology // after restoring the base comments part. ReconcileCommentThreadingParts(main, snapshot, byUri); + ReconcileStylePart(main, snapshot, byUri); ReconcileNumberingPart(main, snapshot, byUri); } @@ -12338,6 +12359,31 @@ private static void ReconcileNumberingPart( } } + /// Restore styles-part topology as well as content. Style synthesis can create the + /// optional part, so an ordinary undo must remove it and redo must recreate it. + private static void ReconcileStylePart( + MainDocumentPart main, DocumentSnapshot snapshot, + System.Collections.Generic.Dictionary byUri) + { + var snapshotPart = snapshot.StyleParts.FirstOrDefault(); + var live = main.StyleDefinitionsPart; + + if (live is not null + && (snapshotPart.RelId is null + || !string.Equals(main.GetIdOfPart(live), snapshotPart.RelId, StringComparison.Ordinal))) + { + main.DeletePart(live); + live = null; + } + + if (live is null && snapshotPart.RelId is not null + && byUri.TryGetValue(snapshotPart.PartUri, out var xml)) + { + var restored = main.AddNewPart(snapshotPart.RelId); + restored.PutXDocument(new XDocument(xml)); + } + } + internal int NextRevisionId() => System.Threading.Interlocked.Increment(ref _revisionCounter); private void ThrowIfDisposed() diff --git a/Docxodus/Internal/DocxSessionOps.cs b/Docxodus/Internal/DocxSessionOps.cs index d31163c5..55bd9721 100644 --- a/Docxodus/Internal/DocxSessionOps.cs +++ b/Docxodus/Internal/DocxSessionOps.cs @@ -84,9 +84,6 @@ public static string CheckPreconditions(int handle, MutationPreconditions? preco : new EditResult { Success = false, Error = error }); } - internal static void RestoreVersionAfterRebind(int handle, long version) => - SessionRegistry.Get(handle).RestoreVersionAfterRebind(version); - public static string ExecuteBatch( int handle, MutationBatchMode mode, diff --git a/Docxodus/Internal/NumberingFactory.cs b/Docxodus/Internal/NumberingFactory.cs index 7ad60349..58888d57 100644 --- a/Docxodus/Internal/NumberingFactory.cs +++ b/Docxodus/Internal/NumberingFactory.cs @@ -102,6 +102,28 @@ public static int EnsureNumbering(WordprocessingDocument doc, ListFormat fmt) return (int)num.Attribute(W + "numId")!; } + /// + /// Return whether would have to add either the + /// Docxodus-owned abstract definition or its concrete w:num instance. + /// This is a read-only preflight for tracked operations: a paragraph revision can + /// restore w:numPr, but cannot carry a before-image for numbering.xml. + /// + internal static bool WouldEnsureNumberingMutate(WordprocessingDocument doc, ListFormat fmt) + { + if (fmt == ListFormat.None) return false; + var root = doc.MainDocumentPart?.NumberingDefinitionsPart?.GetXDocument().Root; + if (root is null) return true; + + string nsid = NsidFor(fmt); + var abstractNum = root.Elements(W + "abstractNum") + .FirstOrDefault(a => (string?)a.Element(W + "nsid")?.Attribute(W + "val") == nsid); + if (abstractNum is null) return true; + + var abstractId = (string?)abstractNum.Attribute(W + "abstractNumId"); + return abstractId is null || !root.Elements(W + "num").Any(n => + (string?)n.Element(W + "abstractNumId")?.Attribute(W + "val") == abstractId); + } + private static int NextId(XElement root, string elemLocalName, string idAttrLocalName) { int max = 0; @@ -360,4 +382,23 @@ static int LvlOf(XElement e) => } return mutated; } + + /// + /// Return whether would mutate the live abstract + /// numbering definition. Missing/invalid numbering references return false because + /// would likewise make no package change. + /// + internal static bool WouldEnsureLevelDefinedMutate( + WordprocessingDocument doc, int numId, int targetIlvl) + { + if (targetIlvl < 0 || targetIlvl > 8) return false; + var root = doc.MainDocumentPart?.NumberingDefinitionsPart?.GetXDocument().Root; + var num = root?.Elements(W + "num") + .FirstOrDefault(n => (string?)n.Attribute(W + "numId") == numId.ToString()); + var abstractId = (string?)num?.Element(W + "abstractNumId")?.Attribute(W + "val"); + var abstractNum = abstractId is null ? null : root?.Elements(W + "abstractNum") + .FirstOrDefault(a => (string?)a.Attribute(W + "abstractNumId") == abstractId); + return abstractNum is not null && !abstractNum.Elements(W + "lvl").Any(level => + (string?)level.Attribute(W + "ilvl") == targetIlvl.ToString()); + } } diff --git a/Docxodus/Internal/RevisionOps.cs b/Docxodus/Internal/RevisionOps.cs index 753061aa..b6ef212d 100644 --- a/Docxodus/Internal/RevisionOps.cs +++ b/Docxodus/Internal/RevisionOps.cs @@ -945,10 +945,15 @@ private static void AddUnsupportedGroups(XElement root, int partIndex, List !represented.Contains(marker)) + // w:delText/w:delInstrText are payload, not independent revisions, when + // they sit beneath a deletion wrapper already claimed by the registry. + // Orphan instances still need an explicit fail-closed entry. + .Where(marker => !IsClaimedDeletionPayload(marker, represented)) .Where(marker => !marker.Ancestors().Any(ancestor => PropsChangeNames.Contains(ancestor.Name)))) { var type = marker.Name == W.ins ? TypeInsert - : marker.Name == W.del ? TypeDelete + : marker.Name == W.del || marker.Name == W.delText + || marker.Name == W.delInstrText ? TypeDelete : marker.Name == W.moveFrom || marker.Name == W.moveTo || MoveRangeNames.Contains(marker.Name) ? TypeMove : PropsChangeNames.Contains(marker.Name) || marker.Name == W.numberingChange @@ -988,9 +993,14 @@ private static bool IsRecognizedRevisionMarker(XElement element) || UnsupportedRangeNames.Contains(name) || PropsChangeNames.Contains(name) || name == W.cellIns || name == W.cellDel || name == W.cellMerge - || name == W.numberingChange; + || name == W.numberingChange || name == W.delText || name == W.delInstrText; } + private static bool IsClaimedDeletionPayload( + XElement marker, IReadOnlySet represented) => + (marker.Name == W.delText || marker.Name == W.delInstrText) + && marker.Ancestors(W.del).Any(represented.Contains); + /// /// Word records one cell-structure action as live cell marks plus associated table, /// cell, paragraph-property, and content revisions. Fold that coherent stamp into diff --git a/Docxodus/Internal/StyleFactory.cs b/Docxodus/Internal/StyleFactory.cs index bade9c87..269a4ef2 100644 --- a/Docxodus/Internal/StyleFactory.cs +++ b/Docxodus/Internal/StyleFactory.cs @@ -204,21 +204,28 @@ public static bool EnsureParagraphStyle(WordprocessingDocument doc, string style var main = doc.MainDocumentPart; if (main is null) return false; - var part = EnsureStylesPart(main); - - var root = part.GetXDocument().Root!; - bool exists = root.Elements(W + "style") - .Any(st => (string?)st.Attribute(W + "styleId") == styleId); - if (exists) return true; + if (HasParagraphStyle(doc, styleId)) return true; var def = BuiltInParagraphStyle(styleId); if (def is null) return false; // unknown custom id — leave it; caller reports UnknownStyle + var part = EnsureStylesPart(main); + var root = part.GetXDocument().Root!; root.Add(def); part.PutXDocument(); return true; } + /// Read-only existence check used before a tracked paragraph-style edit. + /// A w:pPrChange can restore the paragraph's style reference, but it cannot + /// remove a style definition or styles part synthesized by the edit. + internal static bool HasParagraphStyle(WordprocessingDocument doc, string styleId) + { + var root = doc.MainDocumentPart?.StyleDefinitionsPart?.GetXDocument().Root; + return root is not null && root.Elements(W + "style") + .Any(style => (string?)style.Attribute(W + "styleId") == styleId); + } + /// Canonical definition for a well-known built-in paragraph style, or null if unknown. private static XElement? BuiltInParagraphStyle(string styleId) { diff --git a/docs/architecture/docx_agent_server.md b/docs/architecture/docx_agent_server.md index 17158d86..f11f8de0 100644 --- a/docs/architecture/docx_agent_server.md +++ b/docs/architecture/docx_agent_server.md @@ -47,7 +47,7 @@ tools/mcp-server/Program.cs — JSON-RPC transport: initialize, tools/lis ▼ tools/mcp-server/Dispatcher.cs — (tool, action) → Docxodus API call; arg parsing only │ also: tools/mcp-server/SessionStore.cs (external session_id - │ → DocxSessionOps handle + opened-from location + settings) + │ → DocxSessionOps handle + opened-from location) │ ├──▶ IDocumentStore ───────── where bytes come from and go to (see Document storage) │ LocalFileDocumentStore scope-rooted local filesystem — the only backend today @@ -59,7 +59,7 @@ Docxodus.DocxSession (the real work — see docs/architecture/docx_mutation_ap ``` `SessionStore` is the one piece of state this server owns that `DocxSessionOps` doesn't: a -string `session_id` → `{ handle, location, settings }` map. The external protocol uses that +string `session_id` → `{ handle, location }` map. The external protocol uses that unguessable id as its document capability, while `docxodus_save` uses the remembered location to write back without requiring the caller to repeat the path. Tracked-revision resolution now mutates the live session through `DocxSessionOps`; it does not rebind a whole-document transform. diff --git a/tools/mcp-server/README.md b/tools/mcp-server/README.md index 8fa0f536..e40edbc2 100644 --- a/tools/mcp-server/README.md +++ b/tools/mcp-server/README.md @@ -104,11 +104,10 @@ exposed, because the underlying Docxodus engine doesn't have them (rather than f these are called out so agents/tooling built against this server know to route around them): -- **Exotic revision families aren't individually resolvable.** `docxodus_track_changes` - lists and selectively resolves inserts/deletes/moves/format changes by `revisionId` - (issue #318), but `w:cellIns`/`w:cellDel`/`w:cellMerge`, content-control ins/del - ranges, and `w:numPr` numbering-ins markers are not enumerated — `accept_all`/ - `reject_all` still resolve those. +- **Unsafe revision topology fails closed.** `docxodus_track_changes` lists cell, + content-control, numbering, text, move, row, and property revisions with stable ids. + Unsupported, malformed, or ambiguous native markup remains visible with a diagnostic; + individual and bulk resolution return a typed error without changing the session. - **New lists inserted via markdown don't get real numbering** unless promoted afterward with `docxodus_list`'s `apply_format` action (which does write real `w:numPr`). - **Generated-id previews are semantic rather than necessarily byte-identical.** Preview runs diff --git a/tools/mcp-server/SessionStore.cs b/tools/mcp-server/SessionStore.cs index a27e2b57..a0405c59 100644 --- a/tools/mcp-server/SessionStore.cs +++ b/tools/mcp-server/SessionStore.cs @@ -9,30 +9,24 @@ namespace Docxodus.McpServer; /// /// One open document. Wraps the raw integer handle with the -/// bookkeeping a stdio tool server needs that the handle alone doesn't carry: the store location -/// it was opened from (so docxodus_save can default to "write back"), and the settings -/// it was opened with (so a whole-document transform — see — can reopen -/// an equivalent session instead of losing tracked-change/undo configuration). +/// store location the stdio server needs so docxodus_save can default to "write back". /// internal sealed class DocSession { required public string Id { get; init; } - public int Handle { get; set; } + public int Handle { get; init; } /// Store-resolved location this session was opened from — already checked to be in /// scope, so a save back to it needs no re-validation. Null only if a session was opened from /// bytes with no origin. public string? Location { get; set; } - - public DocxSessionSettings Settings { get; set; } = new(); } /// /// External session-id → registry for the MCP tool surface. /// Deliberately separate from (which only -/// knows integer handles): tools like docxodus_track_changes's accept-all/reject-all -/// need to swap the underlying handle for a fresh one built from transformed bytes while the -/// caller keeps addressing the same session id — see . +/// knows integer handles) so callers use unguessable capability ids and retain document-store +/// location metadata without exposing process-local handles. /// internal sealed class SessionStore { @@ -56,7 +50,6 @@ public DocSession Open(byte[] bytes, string? location, DocxSessionSettings setti Id = NewSessionId(), Handle = handle, Location = location, - Settings = settings, }; _sessions[session.Id] = session; return session; @@ -84,27 +77,6 @@ public void Close(string sessionId) DocxSessionOps.CloseSession(session.Handle); } - /// - /// Replace a session's underlying document with in place: opens - /// a fresh handle (with the session's original settings, unchanged) and closes the old one, - /// without changing the external session id the caller addresses it by. Used by - /// whole-document byte transforms — currently only "accept every tracked change" / "reject - /// every tracked change" — that have no in-place session mutation of their own. - /// must have been produced from a Unid-preserving save (see - /// ) so the reopened session resolves the same - /// anchor ids the caller already has cached; the session's OWN - /// is passed through unchanged (not - /// forced true) so a later docxodus_save to disk still strips that bookkeeping the - /// way a save-to-disk should. - /// - public void Rebind(DocSession session, byte[] newBytes) - { - var newHandle = DocxSessionOps.OpenSession(newBytes, session.Settings); - var oldHandle = session.Handle; - session.Handle = newHandle; - DocxSessionOps.CloseSession(oldHandle); - } - public void CloseAll() { foreach (var kv in _sessions)