Preserve untouched OPC part payloads across DocxSession saves - #684
Merged
Conversation
A single tracked text replacement through DocxSession rewrote the payload of 23 of TestFiles/NVCA-Model-COI.docx's 44 parts: every header and footer, both note parts, styles and settings. Only word/document.xml and word/settings.xml had any semantic change — the edit itself, and the <w:trackRevisions/> the tracked-change mode requires. The other 21 differed by exactly three bytes. Decoded, before and after were character-for-character identical; the saved parts simply carried a UTF-8 byte-order mark their inputs did not. XmlWriter's default UTF-8 encoding emits one, Word's parts generally carry none, and PtOpenXmlUtil's part writers took the default. Save offers every projected part for serialization — deliberately, since a part can hold a cached story edit without ever having been given an anchor — so every part it touched picked up the mark. Part writes now keep whatever convention the stored payload already had. A BOM is not part of the XML infoset and OPC readers accept either form, so the honest rule is to leave it as found rather than impose one: a document that has them keeps them, one that does not stays clean, and neither acquires a payload change from a save that changed nothing. Fixing it in the writer rather than in Save is what makes the invariant hold everywhere. The same three bytes were also being added by the anchor-index Unid flush and by the transaction checkpoint's cached-tree overlay, and .NET, WASM/npm, the Python host and MCP all save through this one serializer. On the reported case this takes the changed-part count from 23 to 2 — exactly the two the issue says must change. Full suite green at 3940. Closes #668
An untouched part is preserved because the round-trip through LINQ-to-XML reproduces it, not because the write is skipped, so it is worth stating in a test what that round-trip does and does not carry. Empty-element spelling IS carried: a header doctored to use <w:x></w:x> long form comes back long form. That is the case a naive reading of 'every part is reserialized' would expect to break, and it covers what OOXML producers actually emit. Whitespace inside a tag is not part of the XML infoset and does not survive. No producer emits it; the test records the limit rather than leaving a reader to discover it.
JSv4
added a commit
that referenced
this pull request
Sep 2, 2026
Keeps both [Unreleased] entries. The test-project warning baseline goes to 694, not the 693 git would have merged in silently: this branch and #684 each added one test file and each wrote 693 from a base of 692, which is an identical change git does not treat as a conflict.
JSv4
pushed a commit
that referenced
this pull request
Sep 3, 2026
The README's diagnostic says a moving ratio means the two paths changed by different amounts and something real happened. True, but it never said how much movement counts, which makes it unfalsifiable in the direction that matters: any wobble can be read as a finding. Two consecutive full + HTML runs on one build within one hour read 77x and 84x. That is the noise floor measured rather than guessed, and #653 -- which halved the ratio outright -- is the other end of the scale. Stating both puts a number on the rule: under about 10% is noise, approaching a halving or doubling is the engine, and in between you measure again. Found while checking #684, which changed the OPC part serializer so a save keeps whatever byte-order-mark convention a part already had. That is the save path, so it is the denominator of every ratio in the table, and the extra three-byte read per part write predicted a slower recording path. It did not happen: avgMutateMs came in at 1.39-1.65 ms against the ~2 ms the table publishes, the ratios held at 33x / 47x / 77-84x against a published 33 / 45 / 78, and the absolutes moved in both directions at once. Nothing to republish. 14 browser assertions and 61 node checks pass on the merged build.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The measurement
One tracked text replacement on
TestFiles/NVCA-Model-COI.docxrewrote the decompressed payload of 23 of the package's 44 parts: every header and footer, both note parts, styles and settings. Only two of those had any semantic reason to change —word/document.xml(the edit) andword/settings.xml(the<w:trackRevisions/>the tracked-change mode requires).What the other 21 changes actually were
Three bytes. Decoded as text, before and after were character-for-character identical:
XmlWriter's default UTF-8 encoding emits a byte-order mark. Word's parts generally carry none.PtOpenXmlUtil's part writers took the default, andSaveoffers every projected part for serialization — deliberately, because a part can hold a cached story edit without ever having been given an anchor — so every part it touched picked up the mark.The fix
A part write keeps whatever byte-order-mark convention the stored payload already had.
A BOM is not part of the XML infoset and OPC readers accept either form, so the honest rule is to leave it as found rather than to impose one. A document that has them keeps them; one that does not stays clean; and neither acquires a payload change from a save that changed nothing.
Fixing it in the writer rather than in
Saveis what makes the invariant hold everywhere. The same three bytes were also being added by the anchor-index Unid flush and by the transaction checkpoint's cached-tree overlay — and, per the issue's ripple requirement, .NET, WASM/npm, the Python host and MCP all save through this one serializer, so none of them needs its own change.Result
Exactly the set the issue names as legitimate.
What guarantees the invariant, and where it stops
Worth being precise, because the mechanism is not what it might look like. An untouched part ends up byte-identical because the round-trip through LINQ-to-XML reproduces it, not because the write is skipped. So the fair question is what that round-trip carries.
MarkupDetailOutsideTheXmlInfosetIsNormalizedRatherThanPreservedanswers it with a doctored fixture rather than an assurance:<w:x></w:x>long form comes back long form — LINQ-to-XML tracks the distinction. This is the case a naive reading of "every part is reserialized" would expect to break, and it is the one OOXML producers actually exercise.The invariant itself is pinned by test on a real 44-part document, so it is enforced as an outcome regardless of which mechanism delivers it.
What I removed on the way
My first attempt also added a conditional write to
Save— serialize into a buffer, compare against the stored payload, write only on a difference. Once the encoding rule was in place I checked whether it was doing anything, and it was not: the suite passed identically with it reverted. It could only ever skip a write whose bytes would have been identical anyway, which is observationally equivalent to just writing. It went in the bin rather than into the PR.Tests
DocxSessionPartPayloadIdentityTests, six tests over decompressed part payloads (the ZIP container stays implementation-defined — compression and entry order are explicitly not part of the invariant):word/document.xmlandword/settings.xml;word/document.xml— otherwise "nothing changed" could be satisfied by losing the edit;Non-vacuous: reverting only the writer change fails four of them.
Full .NET suite: 3940 passed, 0 failed, 3 skipped. Library also compiles under
WASM_BUILD.Closes #668
🤖 Generated with Claude Code
https://claude.ai/code/session_01VSnwkK1Nx6zZb2RnnoxKdx