Repository navigation
fix(docx-compare): own terminal move breaks inside a story-closing content control - #1122
Conversation
…ntent control #1055 gave a whole-paragraph move with one body-story terminal endpoint Word-native break ownership (ordinary del/ins paragraph-mark revisions on the stable predecessor, moved content in paired move ranges) so LibreOffice Accept All and Reject All stop leaving an extra empty paragraph. The rule only recognised endpoints whose parent is w:body. The last paragraph of a block content control that closes the body story is the same terminal mark, so control([A,B,C]) -> control([C,A,B]) and -> control([B,C,A]) still left one empty paragraph after LibreOffice's projections. wholeParagraphMoveEndpoint now also recognises a w:sdtContent whose control (and any control it nests in) is followed by nothing but range boundaries and w:sectPr up to the body. A control that another block follows is left alone: its last paragraph has a real break after it and keeps the middle- move topology. The acceptor already resolves paragraph marks between direct siblings, so safe-docx's own projections stay exact. Tests: a LibreOffice oracle gate (issue-1101-libreoffice-sdt-terminal-move) covering the two reported cases plus middle-move, body-level and preceded- by-body-paragraph controls, skipping when soffice is unusable; and a structural matrix in paragraphMoveReview that pins the paragraph-mark revisions, range containment, the serializer verifier and the AST and native Accept/Reject projections without LibreOffice. Fixes: #1101
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
LLM gate (advisory)All evaluated rules passed - 6 pass, 0 warn, 0 error, 10 skipped, 16 total FindingsNone. All 16 rules (6 evaluated, 10 skipped)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7f1e1a25f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return index >= 0 && siblings.slice(index + 1).every((candidate) => | ||
| RANGE_BOUNDARY_LOCALS.has(candidate.localName) || | ||
| (candidate.namespaceURI === W_NS && candidate.localName === 'sectPr')); |
There was a problem hiding this comment.
When a terminal content control is followed by a valid body-level w:permEnd, w:proofErr, or custom-XML range marker before w:sectPr, this predicate returns false because RANGE_BOUNDARY_LOCALS contains only bookmark, comment, and move markers. These elements are explicitly included in the repository's full block-sibling range set (RANGE_MARKUP_BLOCK_SIBLING_LOCALS), and they do not add another paragraph break, so the control still closes the story. Such documents therefore retain the legacy move-mark topology and the LibreOffice trailing-empty-paragraph defect this change is intended to fix.
Useful? React with 👍 / 👎.
Codex review of #1122 asked for two coverage additions. The #1028 LibreOffice gate filtered empty paragraphs out of its terminal-control rows, so it could not see the extra empty paragraph #1101 is about; with a detected move those "enters" and "leaves" rows are now exact on text and paragraph shape, and the comment says why the "after it" row stays on the legacy topology. The structural matrix gains the containers closesBodyStory must accept (nested terminal controls) and reject (a customXml wrapper, a terminal control with no in-control predecessor), each with exact AST and native projections. Table cells are not represented because the comparator does not detect moves inside a cell. Ref: #1101
…ersions The comparator refuses to insert a block content control, so the scenario now keeps the control in both documents with a nested control in front of the moved paragraph. Range-marker parents are asserted per direction. Ref: #1101
Codex peer review (gpt-6-sol) — verdict: APPROVEPrompt: Context / Repository / Affected Files / Summary / Assumptions / Reviewer discipline / Areas of Uncertainty. Codex read all five affected files, ran
Noted while adding row 2, not changed here (pre-existing, legacy topology, outside #1101): when the moved paragraph is the last block of a story-closing control and follows a table inside it, the AST reject projection keeps one empty paragraph because Pre-submit was rerun after the fixes (see the checks on the final commit). |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Post-merge smokeSquash Real document. No public document in Command per variant (script
Both move shapes gave the same result in every row. Output excerpt (body level, styled, last→front): Reading. The content-control rows now behave exactly like the body-level rows, which is what #1101 asks for: the redlines carry identical The synthetic gate on the merged build: Files for a Word check (regenerate with the commands above; kept under Remote branch |
This branch was successfully deployed
1 participant
Why
#1055 gave a whole-paragraph move with one body-story terminal endpoint Word-native break ownership (ordinary
w:del/w:insparagraph-mark revisions on the stable predecessor; moved content stays in paired move ranges) so LibreOffice Accept All / Reject All stop leaving an extra empty paragraph. The rule only recognised endpoints whose parent isw:body. The last paragraph of a block content control that closes the body story is the same terminal mark, socontrol([A,B,C]) -> control([C,A,B])(Accept All) and-> control([B,C,A])(Reject All) still left one empty paragraph in LibreOffice, as #1101 reports.What
taggedTreeSerializer.ts: newclosesBodyStory(container);wholeParagraphMoveEndpointnow acceptsw:bodyor aw:sdtContentwhose control (and any control it nests in) is followed by nothing but range boundaries andw:sectPrup to the body. The predecessor and terminal checks run among that container's siblings, so the break revision moves to the preceding paragraph inside the same control. A control that another block follows is unchanged: its last paragraph has a real break after it and keeps the middle-move topology.trackChangesAcceptorAst.tsand docx-coreacceptChanges/rejectChangesresolve paragraph marks between direct siblings, which holds insidew:sdtContent, so safe-docx's own projections stay exact.refactor-tracked-paragraph-move-ownershipdelta; CHANGELOG entry.Tests
packages/docx-core/src/integration/issue-1101-libreoffice-sdt-terminal-move.test.ts(LibreOffice oracle; skips whensofficeis unusable): the two reported cases, a middle move inside the control, last-to-middle, a terminal control preceded by a body paragraph, and the two body-level cases. Asserts text andparagraphShapeequal identity on Accept and Reject. All 7 pass locally against/opt/homebrew/bin/soffice; on main the two reported cases produced["C","A","B",""]/["A","B","C",""].packages/docx-compare/src/tagged/paragraphMoveReview.test.ts: non-LO structural matrix (terminal destination, terminal source, terminal destination after a body paragraph, middle move inside the control, control followed by a body paragraph) pinning the paragraph-mark revisions, range-marker containment,verifySerializedMoveRanges, and exact AST and native Accept/Reject projections including control membership. Fails on main (expected [ Array(4) ] to deeply equal [ [ 'del' ], [], [ 'ins' ], [] ]).issue-941-libreoffice-terminal-move,issue-1028-libreoffice-sdt-boundary-move,nativeMoveParagraphParity,taggedTreeSerializer,terminalMarkCleanupParitypass. Full pre-submit (build, lint, test:run, spec-coverage, conformance-citations, conformance-doc, cycles) passed.Closes #1101