fix: a recorded note's definition carries insertion markup, and the stateless reject reads it - #638
Merged
Merged
Conversation
…tateless 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.
…ntry 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.
…cument 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.
… 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
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 #636
The problem
#634's adversarial review empirically confirmed the mirror of the bug it was fixing. The
stateless reject pruned every note definition its resolution orphaned, unconditionally. That
unconditional prune existed as cover for #614's deliberate choice to leave a session-recorded
note's definition unmarked — and the omission made the redline ambiguous to a stateless consumer:
w:inscitation next to an unmarked definition is what a session-authored note insertionproduced (insertFootnote/insertEndnote under render_inline recording write an irreversible mark instead of refusing #614), where reject must remove the definition;
the baseline already owned — where reject must keep it, because rejecting reproduces the
baseline.
The unguarded prune picked the first case and ate the second: rejecting that redline deleted a
content-bearing definition the baseline was entitled to keep,
Reject(Compare(l, r)) ≠ l—verified by two independent review agents before this fix (baseline 1 note, rejected 0).
The fix
The same move #631 made on the accept side: put the missing fact into the redline, then read it.
The definition's content now records.
InsertFootnote/InsertEndnoteundertracked-change recording mark every definition run
w:insand insertion-mark the paragraphmarks, through
MarkParagraphContentAndMark— the session's existing single owner of "this wholeparagraph is one revision" (the
MoveBlockmachinery), so no new mechanism. This matches whatWord writes for an inserted footnote (its text renders as an insertion in the note pane) and what
the diff engine's own redline has always carried for a wholly-inserted note — verified against
the WC035 insert-direction redline, whose definition arrives fully
w:ins-marked. #614'sunmarked-definition rationale ("no independently meaningful resolution") described a granularity
Word itself permits; the real cost of the omission was the producer ambiguity above.
The reject prune takes the same blockless guard as accept.
PruneNotesEmptiedByAcceptbecomes
PruneNotesEmptiedByResolution: a definition the resolution both orphaned and emptiedgoes (a wholly-inserted note on reject, a wholly-deleted note on accept); an unmarked husk keeps
its content and stays.
Reject(Compare(l, r)) ≡ landAccept(Compare(l, r)) ≡ rnow both holdfor the note stores, with one shared, direction-neutral rule. The session's own editorial resolve
paths (DS418) deliberately keep the unguarded rule, as before.
Compatibility
A redline saved from a #614-era session (its definition unmarked — a shape shippable for four
days) now rejects to an orphaned husk through the stateless path instead of losing the note;
re-authoring the redline with the current version restores full reversibility. Recorded in the
CHANGELOG entry.
Tests
DS434_StatelessReject_PreservesABaselinesOwnHuskThatTheCounterpartCites(new) — thereview's reproduced mirror case, red before this fix (rejected count 0 instead of 1).
DS369_RecordedNoteDefinition_IsInsertionMarked_AndStatelessResolutionsReadIt(new, bothnote kinds) — pins the marking shape (every run inside
w:ins, paragraph marksinsertion-marked), that the stateless reject takes the note with zero diff against the baseline
(the insertFootnote/insertEndnote under render_inline recording write an irreversible mark instead of refusing #614 contract, now achieved by reading the markup rather than by the unguarded prune), and
that the stateless accept keeps the note with the markup resolved away.
family (DS429–433) pass unchanged.
attributable to this change (no corpus fixture carries a live
w:insnote citation, and thereject path is not corpus-exercised); the only content mismatch is Accepting a redline does not reproduce the counterpart's note store, and neither choice is right for every document #631's already-merged RP050
row.
Part 2 of #636: recorded, deliberately unhandled
The "cited childless shell" robustness nit (a definition emptied while its citation survives)
requires synthetic input Word's UI cannot author — deleting all of a note's text tracked leaves
the final paragraph mark unmarked, so the real Word flow accepts to an empty-paragraph note,
which we handle correctly. The degenerate shape and the deferred fix (leave a minimal empty
paragraph, pending Word-side evidence) are now recorded in the corner-cases doc's RP050 entry,
which is the right institutional memory for it — so this PR closes the issue rather than leaving
a tracker item whose next step cannot be taken in this environment.
After the second adversarial round (third and fourth commits)
The review's verification pass empirically confirmed three behavioral findings, all addressed:
DS435, red before the fix):WmlComparerre-synthesizes an unmarked
w:footnoteRefmarker run into every inserted definition itrenders, so a purely-blockless guard shipped an orphaned marker-husk on reject where the
pre-Reject has the mirror of #631's husk bug (blocked by #614's unmarked definitions), and accept can ship a cited childless note #636 unguarded prune removed the note. The predicate is now scaffolding-aware
(
IsEmptiedToScaffolding): note-reference-mark runs count as emptiness, real content neverdoes.
paragraph share a single author/date pair; previously each paragraph took its own monotonic
date, splintering one insert into separately-dated revision groups.
DS369asserts onedistinct
w:dateacross body and note.DS436). The reviewer's suggestedrepair — restore a minimal empty paragraph — was implemented and then reverted by evidence:
the corpus genuinely contains cited childless notes (
WC064-Footnoteships one;WC063-Footnote-Mod's note is childless), and the repair broke both WC round-trip suites,because
Accept(Compare(l, r)) ≡ rmeans reproducing exactly that shape. The engine will notmanufacture content the counterpart does not have; the decision and its evidence are recorded
in the corner-cases entry.
DS434pins the session-leg distinctness (the editorial reject still strips the husk —deliberately, now said on purpose) and shares one husk fixture builder with
DS430; thesame-release CHANGELOG's insertFootnote/insertEndnote under render_inline recording write an irreversible mark instead of refusing #614 entry and both architecture docs are reconciled with the new
contract; and the rule's owner now documents the session interlock (the session resolver keeps
a note's last block alive, so naively unifying it onto the guarded prune would silently stop
pruning session-rejected recorded notes).
Final state: full suite 4,228 passed / 0 failed; corpus differential vs main shows zero
library-attributable movement (the only differing rows are the #633 harness's new observation
channels).