Repository navigation
fix(docx-compare): pair header/footer text-box stories across an added or removed section break - #1492
Conversation
…d or removed section break A section break added or removed beside a header or footer that holds a text box aborted the comparison whenever the part's ordinary content changed (text beside the box, a table row, a box resize), with "unpaired ancillary text-box story is not owned exclusively by a removed section slot". Text-box stories paired by their binding closure (section ordinal x kind x role), which only works while the section topology matches; once a section is added or removed the ordinals stop naming the same pages, and the content and scaffold fallbacks cannot pair a story whose ordinary content changed. Pair such stories through the section alignment Reject All and Accept All produce instead. The body-level sectPr is the last section on both sides (the body comparison records a paragraph-level break on paragraph marks, #1144), the other sections align by what they show, and a part is paired with the original story every aligned page showing it showed, in both directions. A shared part is one story however many sections show it, and a story shown only by a section with no counterpart stays with the inserted/deleted lifecycle checks. An aligned pair gets the same ordinary and lifecycle story comparisons as a topology-matched one; the assembled package is still held page by page to both inputs. A resized box is listed with the drawing scope, as before. The positional header/footer entry a changed section count leaves at another index is now judged against the original with the listed drawings aligned, as the projection check already judges it, so it is dropped when the story is carried; a drawing that draws more than its box keeps the part's entry. Fixes: #1487
… own listed drawings aligned Positional header/footer entries left by a changed section count are judged against the original with the drawings of parts whose listed drawings describe only their boxes aligned, whether or not the part's ordinary content also changed. Adds opt-in LibreOffice evidence for a footer text-box redline across an added and a removed section break. Refs: #1487
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review Summary: ✅ CompletedThis 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)
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
… section Review of #1492 found two pairing defects in the predicted section alignment for header/footer text-box stories: - A retained leading section whose footer changed beside its box, next to an added or removed inheriting section, left the story unpaired: the leading runs shared no section signature and a gap of unequal length was discarded, so the comparison aborted. Unequal runs between two aligned sections now align as a whole; the two-way uniqueness check still pairs a story only when every section of both runs shows it. - Pinning last section to last section paired a removed final footer with the unchanged first one before the exact pair could form, turning a listed removal into a projection-guard abort. The pin now yields when the sections' content places one side's final section elsewhere. Refs: #1487
Codex review round 1 (bd40c32):
|
Codex confirmation round 2 (6cdc896):
|
Post-merge smoke at 2672d14 (squash on origin/main): REGRESSION FOUND on one real documentThe build is a detached worktree at 2672d14 after (a) Issue repro variants: all PASS
(b) Full dogfood recipe (
|
| criterion | nvca-spa | nvca-va | common-paper-csa |
|---|---|---|---|
| C1 compare succeeds | PASS | PASS | FAIL |
| C2 unrepresented empty/expected | FAIL (known, separate) | FAIL (known, separate) | - |
| C3 schema gate | PASS | PASS | - |
| C4 accept/reject faithful (all parts, both appliers) | PASS | PASS | - |
| C4b comparison-layer-only projections keep pending records | PASS | PASS | - |
| C5 revision ids unique | PASS | PASS | - |
| C6 each change redlined in place | PASS | PASS | - |
| C8 Word | DEFERRED | DEFERRED | - |
Pre-merge at bd40c32, common-paper-csa passed C1 and C3–C6.
- The C1 failure:
word/header5.xml#w:txbxContent[0]: unpaired ancillary text-box story is not owned exclusively by a removed section slot. - The C2 failures on the two NVCA documents are the separate over-listing of positional header/footer entries (docx-compare: unrepresentedChanges lists section and header/footer changes the redline already represents #1427 subtraction with NUMPAGES fields). They are unchanged from before the merge.
Cause, bisected by patching the built JS in the PR worktree: the round-1 fix 6cdc896, specifically the alignment of unequal-length section runs as a whole.
- With that branch disabled, common-paper-csa passes C1 and C3–C6 again.
- With only the final-section un-pinning disabled, it still fails.
The document's sections:
- Original:
[h1, h3, h5]. Revised:[h1, h3, h5, h5], where the added section inherits h5. footer1.xmlis shared by every section, holds a text box, and changes.- Because footer1 changes, every section signature differs between the sides, so the leading runs
[s0, s1]and[s0, s1, s2]share no match. - The gap of unequal length is now aligned as a whole. Revised h5 (shown by revised section 2) then picks up original h1 and h3 as backward sources, so the two-way check refuses h5↔h5, which bd40c32 paired through the pinned final sections.
C7 LibreOffice: not run. LibreOffice does not launch on this host. Even docx-core's generic "headless PDF conversion succeeds" scenario times out after 120 s in the pre-submit reruns.
Per the run rules I am not reverting. A narrower fix is proposed in the next comment.
Proposed follow-up fix for the post-merge regression (branch pushed, no PR opened)Branch Change: unequal section runs are no longer added to the aligned sections' two-way map.
Evidence:
Not yet done: the full pre-submit chain, a Codex review and a PR. These are waiting on the supervisor's decision under the "smoke failed, stop" rule. |
Problem
compareDocumentsaborted withunpaired ancillary text-box story is not owned exclusively by a removed section slotwhen the revised copy added (or removed) a section break and a header or footer holding a text box changed its ordinary content: text beside the box, a table row, or a box resize. It aborted whether the break restated the header/footer references or not. The cumulative dogfood smoke hit this on all three documents and stopped thefullrecipe at F2.Cause: header/footer text-box stories paired by binding closure (section ordinal × kind × role), and only while the section topology matched. Once the section count changes, ordinals stop naming the same pages, and the content/scaffold fallbacks cannot pair a story whose ordinary content changed. A box-text-only change already passed, because the scaffold fallback blanks box content.
Approach
textBoxRevisionSafety.ts,sectionAlignedTextBoxStoryPairs: when the topology changed, header/footer stories (at least one side holding a text box) are paired through the section alignment that Reject All and Accept All produce.w:sectPris the last section on both sides. The body comparison records a paragraph-level break on paragraph marks (docx-compare: reject-all cannot restore a tracked section-break change (header/footer bindings lost; added empty break survives) #1144), so the last sections correspond. The exception is when what the sections show places one side's last section elsewhere (a final section added or removed outright); then the content alignment is used. The other sections align by what they show. Between two aligned sections, a run of unmatched sections of equal length aligns in order, and runs of unequal length align as a whole, so a story pairs only when every section of both runs shows it (review round 1).w:sectPrChangesnapshot cannot carry them.storiesShareSlots). The assembled package is still held page by page to both inputs byassertAncillaryTextBoxStoryProjection, so a wrong pairing fails closed instead of publishing.A resized box is listed with the
drawingscope, as before (#1382). After a section count change, the outer comparison leaves positional header/footer entries at the other section index. These are now judged against the original with only the drawing-accounted parts' drawings aligned (drawingAccountedOriginal), which is how the projection check already judges them. So they are dropped when the story is carried. A drawing that draws more than its box (a group, a second shape, a shared template) keeps the part's own entry, and the existing #1382 tests pin that.Representation
w:sectPr(§17.13.5.20), and a deleted mark on the empty boundary paragraph that keeps a removed one (§17.13.5.15). The issue text mentionsw:pPrChange(§17.13.5.29), but that is not the shape the engine emits or that docx-compare: reject-all cannot restore a tracked section-break change (header/footer bindings lost; added empty break survives) #1144 chose, so the tests assert the paragraph-mark shape.w:ins/w:del, whole-roww:trPr/w:ins, and box text insidew:txbxContent.@conformance ECMA-376 edition 5, Part 1 § 17.10.5,§ 17.10.2(header/footer references and inheritance),§ 17.6.18(paragraphw:sectPr),§ 17.13.5.20/§ 17.13.5.15(inserted/deleted paragraph mark). All were checked againstspec-compliance/registry/ecma-376.md.Evidence
pipeline-section-break-header-footer-text-box.test.ts, 105 cases:w:insand onew:deloutside the box);acceptChanges/rejectChanges;drawingentries.iso-resize-syntheticvariantsnone footer|header beside|resize|text|none 0|1|2): every variant now passes. The resize cases list only the drawing.SAFE_DOCX_SECTION_BREAK_TEXT_BOX_READER_REQUIRED=1, run through the heavy-run gate). Shape: a footer with text beside a DrawingML box, plus a box-text edit.test:run, spec-coverage, conformance citations and doc) passed, andcheck:real-corpuspassed, after merging origin/main @ 1b1149e.~/.cache/safe-docx-compare-fanout/word-staged/1487/. The staged compared files pass the schema gate.Left out and why
w:footerReferencer:idliterally (rIdStoryvsrIdCopy) and fails. This happens for plain footers with no text box too, before any story pairing, so it is outside this lane. When the break is removed and a box is present, the removed section's restored reference resolves to the part that now carries the aligned redline, and the projection check refuses it (fail closed, no corruption). Both are follow-ups.fullrecipe: C1 (compare succeeds) now passes on all three documents, and so do C3–C6 and C4b. C2 still lists positional header/footer entries at the shifted section indexes. This is not specific to text boxes: it reproduces with every text box switched off.subtractRepresentedPackageChanges(docx-compare: unrepresentedChanges lists section and header/footer changes the redline already represents #1427) never drops a header/footer entry in a package whose header/footer fields read other stories, such as NUMPAGES in the recipe's "Page X of Y" row. That is a listing-precision follow-up; C4 confirms the redline itself carries those stories.[A, B]→[A]): the removed footer stays listed (scope: footer, kind: removed), as on main. After Reject All the final section shows the revised body-level references (aw:sectPrChangesnapshot is CT_SectPrBase, §17.13.5.32), so B cannot be shown again. When A is edited as well, the comparison aborts on main and here alike (the projection guard). Follow-up.drawingnote (allowed by the full-redline policy for shape changes). The box text and the part's ordinary content are tracked.Fixes #1487