fix: accepting a redline takes a wholly-deleted note's definition with it - #634
Merged
Conversation
…h it The redline itself distinguishes the two reference-less-husk cases (issue #631). A note definition the counterpart KEPT is reconciled by the note differ's reference-correspondence pass into a matched equal pair and passes through the redline unmarked; one the counterpart DELETED pairs with nothing and lands with every block deletion-marked -- runs in w:del, paragraph mark deleted -- exactly the shape Word itself authors for a tracked note deletion (RP050-Deleted-Footnote). Accepting strips the second shape bare, and nothing removed the stripped shell: the stateless accept shipped a childless <w:footnote/> the counterpart did not have, and disagreed with DocxSession.AcceptAllRevisions about the same redline. The note-lifecycle rule (single owner: Internal/NoteReferenceOps.cs) gains PruneNotesEmptiedByAccept: remove a definition whose last citation this accept removed AND which the accept left without block content. The guard is exact -- an unmarked paragraph always survives an accept, so a definition only comes out blockless when the redline deleted the whole note -- and it is what keeps Accept(Compare(l, r)) == r true for a counterpart's own husk (DS430) while making it true for a deleted note (DS432, new; red before this fix). Applied in RevisionProcessor's shared accept core, so every accepted view -- the stateless accept, the PreAcceptInputRevisions flatten, IrReader's revision view, WmlComparer's input flatten -- agrees on what accepting a wholly-deleted note means. No policy setting: with the representation read, there is no decision left to push onto callers. Full suite 4,222 passed / 0 failed. Corpus differential vs a main baseline: exactly 1 mismatch of 12,882 -- RP050-Deleted-Footnote#preaccept/redline, the one fixture whose defining feature is a tracked note deletion, whose flattened view now drops the emptied husk. Word's own accepted oracle for RP050 keeps a childless shell where we remove the element; the difference is recorded in docs/ooxml_corner_cases.md, the reversibility sweep compares semantically and stays green, and removal is the schema-valid form CT_FtnEdn requires.
…ighten the rule's prose to one owner Post-review round on #631. DS433 pins the shared accept core on the Word-authored RP050 fixture -- the single private funnel behind the stateless facade, the PreAcceptInputRevisions flatten, IrReader's revision view, WmlComparer's input flatten and WmlToHtmlConverter's accept -- which DS432 alone did not cover (red on main: the childless shell counts as a note there). The transport-agreement claim is scoped to the shape it is true for: session and stateless accept agree about a wholly-deleted note, and stay deliberately distinct on the counterpart-kept husk (DS430 vs DS418) -- the test comment, docx_mutation_api.md and the corner-case entry now say so, and the corner-case entry gains the minimal XML reproducer and renderer comparison table the house rules require, plus the accurate description of WHY RRS001 stays green (RP050 is pinned acceptEquivalent:false; recovery is asserted through story text, not modeled-semantic equivalence). The rule's rationale now lives once, on NoteReferenceOps.PruneNotesEmptiedBy- Accept; the two RevisionProcessor comments shrink to pointers. IsBlockless documents why a direct-children check is sufficient after accept's coalescing pass and how it relates to the wider pre-accept block classifiers. And ReferencedNoteIds early-outs when the document has no notes parts at all, instead of walking the body and every header/footer twice per resolution on note-free documents -- the common case on the comparison flatten paths.
JSv4
added a commit
that referenced
this pull request
Aug 31, 2026
…tateless reject reads it (#638) * fix: a recorded note's definition carries insertion markup, and the stateless reject reads it The stateless reject pruned every definition its resolution orphaned, unconditionally. That was cover for #614's deliberately unmarked definitions -- and it made the redline ambiguous to a stateless consumer: a w:ins citation next to an unmarked definition is ALSO what a comparison produces when the counterpart merely cites a husk the baseline already owned, and rejecting that redline deleted the content-bearing definition the baseline was entitled to keep. Reject(Compare(l, r)) != l -- the exact mirror of the #631 accept bug, found by #634's adversarial review and filed as #636. InsertFootnote/InsertEndnote under RenderInline now mark the definition's content inserted, through MarkParagraphContentAndMark -- the existing single owner of "this whole paragraph is one revision" -- matching what Word writes and what the diff engine's own redline already carried for a wholly-inserted note. With every producer marking, the stateless reject takes the same blockless guard as the accept side: PruneNotesEmptiedByAccept becomes PruneNotesEmptiedByResolution, and a definition the resolution both orphaned and emptied goes while an unmarked husk stays. DS369 pins the marking and both stateless resolutions on session-authored redlines (both note kinds); DS434 pins the mirror case the review reproduced (baseline-owned husk, counterpart citation: reject now preserves it). The #614 family (DS366-368) and the #631 family (DS429-433) stay green unchanged. Full suite 4,226 passed / 0 failed; corpus differential adds zero movement beyond #631's known RP050 row (no fixture carries a live w:ins note citation, and the reject path is not corpus-exercised). Compatibility: a redline saved from a #614-era session (definition unmarked) now rejects to an orphaned husk through the stateless path instead of losing the note; re-authoring with the current version restores full reversibility. Noted in the CHANGELOG. * docs: record the cited-childless residual shape in the corner-cases entry The second defect #636's review confirmed -- a definition emptied while its citation survives ships as a cited childless shell -- is a degenerate shape Word's UI cannot author, so it lives in the corner-cases doc as known, deliberately-unhandled behavior with the deferred fix sketched, rather than as an open tracker item. * review: make DS434/DS369 observe the shapes they exist to pin, and document partial resolution The gap sweep on PR #638 found both new tests asserting counts where the claims are about shape. DS434 now asserts the redline's load-bearing premise directly -- the citation arrives inside w:ins while the reconciled definition passes through unmarked -- and checks each resolution's substance: reject leaves the husk's content with no surviving body reference (content-level, because the redline renumbers note ids in body-reference order and an unreferenced husk can only pair by id, so GetRevisions reads the same husk at a shifted id as a delete+insert of identical text), and accept reproduces the counterpart to zero revisions. DS369's accept leg now asserts the body citation survived -- without it, an accept that shipped an uncited invisible note passed every count-based assert in the suite. Also documents the one granularity consequence of marking the definition, shared with the diff engine's own redlines which carry the identical shape: the definition's insertions are individually resolvable, so rejecting only them while keeping the citation yields an empty but still-cited note -- the ordinary meaning of partially resolving a compound change; resolve-all in either direction never produces it. * review: read WmlComparer's marker scaffolding as emptiness, share one revision date, and preserve cited bare notes Second adversarial round on #638, three behavioral findings, each verified: The guard learns the LEGACY engine's dialect. WmlComparer re-synthesizes an UNMARKED w:footnoteRef marker run into every inserted definition it renders, so a purely-blockless guard shipped an orphaned marker-husk on reject where the pre-#636 unguarded prune removed the note. IsEmptiedToScaffolding treats note-reference-mark scaffolding as emptiness; DS435 pins both resolutions of a WmlComparer redline over the WC035 pair. One user action, one revision identity: the citation envelope and every definition paragraph now share a single author/date pair (previously each MarkParagraphContentAndMark call took its own monotonic date, splintering one insert into per-paragraph revision groups under exact-date grouping). DS369 asserts a single distinct w:date across body and note. A still-cited note a resolution strips bare stays bare. The reviewer's suggested repair -- restore a minimal empty paragraph -- was implemented and then REVERTED by evidence: the corpus genuinely contains cited childless notes (WC064-Footnote ships one; WC063-Footnote-Mod's note is childless), and the repair broke both WC round-trip suites because Accept(Compare(l,r)) == r means reproducing exactly that shape. DS436 pins the contract: the partial-resolution sequence keeps the note cited and bare, and the orphan-scoped prune never deletes a note out from under its citation. Also: DS434 gains the session-leg distinctness pin (the editorial reject still strips the husk -- deliberately distinct, now said on purpose) and shares one husk fixture builder with DS430; stale/contradictory prose in docx_mutation_api.md, ooxml_corner_cases.md and the CHANGELOG (including the same release notes' #614 entry) is reconciled; the reject-side comment shrinks to a pointer at the rule's owner, which now documents the scaffolding predicate, the preserved-shape decision, and the session interlock (the session resolver keeps a note's last block alive, so unifying it onto the guarded prune would silently stop pruning session-rejected recorded notes). Full suite 4,228 passed / 0 failed. Corpus differential: zero library-attributable movement.
JSv4
pushed a commit
that referenced
this pull request
Aug 31, 2026
#638 completes what #625 started on the path this demo exercises. #625 made the citation the reversible unit but left the definition unmarked, compensating with an unconditional prune on the stateless reject. That made the redline ambiguous to a stateless consumer: a w:ins citation beside an unmarked definition is also exactly what a comparison emits when the counterpart merely cites a husk the baseline already owned, and the unguarded prune ate that husk. Now both halves record and the prune is guarded on the definition being emptied as well as orphaned. The demo's comments and the tracked_changes.md pointer both described the #625 mechanism, so both were describing an encoding the engine no longer writes. Corrected. The browser assertion is extended rather than left alone, because the old one would still pass against the ambiguous shape: it checked the citation run sits inside a w:ins and stopped there. It now also pulls the note definitions out of the redline and requires the one Act II authored to carry insertion markup. Identifying it by its text matters — the part also holds Word's two reserved separator notes, which carry no markup and must not. No re-measurement. #634 and #638 are on the note-lifecycle and reject paths; the stress loop times docxDiffGetRevisions and docxDiffCompareProducts, which neither touches. Published figures stand as measured at #627. Verified on the rebuilt engine: 14/14 browser assertions, 61/61 node checks. Also merges #633 and #637, both test-only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #631
The problem
Accepting a redline through the stateless path (
DocxDiffOps.AcceptRevisions, which is what npm,python, MCP and WASM all reach) did not reproduce the counterpart's note store. On the
WC035-Footnotepair — a real Word-authored document pair where the change deletes the onlyfootnote — accepting the comparison's redline left 1 footnote definition where the counterpart
has 0, and
DocxSession.AcceptAllRevisionson the very same redline produced 0. Twotransports, same redline, different deliverables.
#625 had deliberately shipped the note-lifecycle rule on the reject side only, because a symmetric
implementation broke the other husk case: a counterpart that keeps a reference-less definition
(the shape a naive SDK edit leaves behind) is entitled to keep it through
Accept(Compare(l, r)) ≡ r. The issue's open question was whether the redline can even tell thetwo cases apart.
The finding: the redline already tells them apart
It can, and it already does — which makes the issue's option 1 ("represent the note-store
difference in the redline") mostly a matter of reading a representation the engine already
writes:
reconciles the baseline's cited definition with the counterpart's uncited-but-identical one into
a matched, equal pair (
AppendUnreferencedNotes), so the definition passes through the redlinewith its content unmarked.
all-
DeleteBlockops, and lands in the redline with every block deletion-marked: runs insidew:del, paragraph mark carryingw:rPr/w:del.Accepting therefore treats them differently already: Case A's definition survives with its
content; Case B's is stripped to a childless
<w:footnote w:id=".."/>. What was missing is thelast step: nothing removed that stripped-bare shell.
The fix
The note-lifecycle rule (single owner:
Internal/NoteReferenceOps.cs) gains an accept-sidevariant,
PruneNotesEmptiedByAccept: remove a definition whose last citation this accept removedand which the accept left without block content. The blockless guard is exact — an unmarked
paragraph always survives an accept even when its runs are deleted, so a definition can only come
out bare when every one of its blocks was deletion-marked, i.e. when the redline said "this whole
note is deleted". Word's own tracked deletions carry exactly this shape:
RP050-Deleted-Footnote(Word-authored) marks every run of the definition
w:deland deletes the paragraph mark. Onehonest nuance, now recorded in
docs/ooxml_corner_cases.md: Word's accept of that fixture lefta childless
<w:footnote w:id="1"/>shell in the oracle rather than removing the element. Weremove it — the schema-valid form (
CT_FtnEdnrequires a block child), what the session has donesince #516, and materially safer in LibreOffice, which re-associates references to definitions
positionally and is thrown off by leftover definitions. The reversibility sweep compares against
Word's oracle semantically, where the two forms are the same note store, and stays green.
RevisionProcessor's accept core captures the cited-note ids before the walk and prunes after it,mirroring what the reject side has done since #614 — same scoping (only citations this operation
removed; a pre-existing dangling note is left alone), same whole-package citation scan (a note
still cited from a header survives). Because the rule sits in the shared accept core, every place
an accepted view of a document is computed — the stateless accept,
DocxDiff'sPreAcceptInputRevisionsflatten,IrReader's revision view, andWmlComparer's input flatten —now agrees about what accepting a wholly-deleted note means, instead of leaving a bare husk to
ride into downstream redlines as debris.
No policy setting was added: with the representation read correctly, accept reproduces the
counterpart in both cases and there is no decision left to push onto callers.
Tests
DS432_StatelessAccept_TakesAWhollyDeletedNoteWithItsCitation(new, red before the fix:1 note instead of 0): on the WC035 pair compared in the deleting direction, the stateless accept
now matches the counterpart's note count, agrees with
DocxSession.AcceptAllRevisionson thesame redline, and the stateless reject still reproduces the baseline.
DS430_StatelessAccept_PreservesACounterpartsOwnOrphanedNote— the Case A pin the issueflagged for revisiting — passes unchanged; only its rationale comment was updated. The guard is
what keeps it true.
header-citation scan) all pass unchanged.
Corpus differential
Baseline on a main worktree,
--checkon this branch, 678 documents: exactly 1 mismatch of12,882 observations —
RP/RP050-Deleted-Footnote.docx#preaccept/redline, on the result channelonly. That is the one corpus fixture whose defining feature is a Word-authored tracked note
deletion: under
PreAcceptInputRevisionsthe input flatten's accepted view now drops the emptiedhusk instead of letting a childless
<w:footnote/>ride into the rendered redline as debris. Noother row moved, and every other channel stayed
clean/stable/unchanged.Docs
docs/architecture/docx_mutation_api.md's note-lifecycle section now describes the accept-sideguard instead of the old "accept deliberately does not apply it" contract;
CHANGELOG.mdrecordsthe behaviour change under Fixed;
docs/ooxml_corner_cases.mdgains the RP050 finding with itsminimal XML reproducer and a Word / LibreOffice / Docxodus comparison table.
After adversarial review (second commit)
A multi-verifier review of this diff confirmed one coverage gap and two prose overclaims, all
fixed here, and two pre-existing neighbouring defects, filed as #636 rather than bolted on:
DS433_AcceptFunnel_TakesAWordAuthoredNoteDeletionsDefinition(new, red on main): everyaccept in the library funnels through one private core — the stateless facade, the
PreAcceptInputRevisionsflatten,IrReader's revision view,WmlComparer's input flatten,WmlToHtmlConverter's accept — and DS432 pinned only the facade. DS433 pins the funnel itselfon the Word-authored RP050 fixture (an assertion the existing RP001/RRS001 sweeps were verified
to be structurally blind to).
accept agree about a wholly-deleted note; on the counterpart-kept husk they remain deliberately
distinct contracts (DS430 vs DS418), and the docs now say so instead of implying general parity.
acceptEquivalent: falsethere; recovery is asserted through story text, not modeled-semanticequivalence) — corrected, and the entry gains the reproducer/table the house rules require.
NoteReferenceOps.PruneNotesEmptiedByAccept(
RevisionProcessor's two comments shrink to pointers),IsBlocklessdocuments why adirect-children check is sufficient after accept's coalescing pass, and
ReferencedNoteIdsearly-outs on documents with no notes parts instead of walking every story twice per resolution.
of this bug but its guard collides with insertFootnote/insertEndnote under render_inline recording write an irreversible mark instead of refusing #614's deliberate unmarked-definition choice (needs its
own design round), and an accept can ship a schema-degenerate cited childless note when the
definition is emptied while its citation survives.