Conversation
…s (SD-2557)
When a Word document stores a `w:sectPr` on a paragraph immediately before a
TOC (or other `docPartObj`) SDT, the section break was dropped. The TOC
ended up rendering on the same page as the prior section's content instead
of starting a new page as Word does.
Root cause in pm-adapter:
- `findParagraphsWithSectPr` (sections/analysis.ts) recurses into `index`,
`bibliography`, and `tableOfAuthorities` to count their children as
paragraphs, but NOT into `documentPartObject` / `tableOfContents`.
- As a result, section ranges treat a TOC SDT as a single opaque unit —
its children don't occupy paragraph indices.
- When processing flow, `handleParagraphNode` / `handleIndexNode` /
`handleBibliographyNode` / `handleTableOfAuthoritiesNode` each emit a
pending section break before the paragraph whose index matches
`nextSection.startParagraphIndex`.
- `handleDocumentPartObjectNode` and `handleTableOfContentsNode` did NOT
run this check, so the deferred break only fired on the next body
paragraph AFTER the SDT. The SDT's content rendered in the PREVIOUS
section, with no page break before it.
Fix:
- Add `emitPendingSectionBreakForParagraph(sectionState, nextBlockId,
blocks, recordBlockKind)` helper in sections/breaks.ts that centralizes
the "check, emit, advance" pattern.
- Call the helper at the top of `handleDocumentPartObjectNode` and
`handleTableOfContentsNode` — once per SDT. Since the SDT's children
don't affect `currentParagraphIndex` (`findParagraphsWithSectPr` skips
them), the check fires correctly at the SDT boundary: if the SDT sits
at a section boundary, the nextPage break is emitted so the SDT renders
on a new page.
- No changes to section-range computation — counting stays consistent.
Verified against both fixtures from the issue (Highstreet Proposal Sample,
Heffernan Proposal Sample): cover stays on its own page, TOC starts on a
new page, matching Word.
Tests:
- 3 new unit tests in document-part-object.test.ts covering:
- Section break emitted at SDT boundary
- No emission when SDT is not at a section boundary
- No-op when sectionState is undefined
- 1740 pm-adapter tests pass (up from 1737), 604 layout-engine tests pass.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ld emission (SD-2557)
The initial fix only handled the sectPr BEFORE a TOC docPartObj (on the
empty paragraph that precedes the SDT). It missed the SECOND sectPr that
Word commonly stores on the trailing empty paragraph INSIDE the TOC SDT —
which signals "TOC section ends here, next section starts on new page".
Because `findParagraphsWithSectPr` did not recurse into `documentPartObject`
or `tableOfContents`, that inner sectPr was never discovered, so no
section-range boundary was built between the TOC and the following body
content. SuperDoc rendered the TOC and the first body section stacked on
the same page (just one page later than before the first fix).
Complete fix:
1. `findParagraphsWithSectPr` (sections/analysis.ts) now recurses into
`documentPartObject` and `tableOfContents` in addition to `index` /
`bibliography` / `tableOfAuthorities`. This lets section-range analysis
see sectPrs stored anywhere inside a TOC SDT.
2. `handleDocumentPartObjectNode` (non-TOC branch) emits the pending
section break before each child paragraph and advances
`currentParagraphIndex` — matching the pattern in `handleIndexNode`,
`handleBibliographyNode`, and `handleTableOfAuthoritiesNode`.
3. `processTocChildren` (toc.ts) accepts `sectionState` via its context
arg and performs the same per-child emit + increment dance as the
paragraph handlers. This handles the TOC branch of
`handleDocumentPartObjectNode` and the nested `tableOfContents`
recursion path.
With all three changes, the Highstreet fixture now renders exactly like
Word:
- Page 1: cover
- Page 2: TOC alone
- Page 3: ABOUT US body
- Page 4: ON BEHALF OF HIGHSTREET
- Page 5: WORKERS COMPENSATION
Tests:
- 4 new tests in document-part-object.test.ts (non-TOC emission,
non-boundary no-op, undefined state, sectionState passthrough to
processTocChildren)
- 1741/1741 pm-adapter, 604/604 layout-engine, 11377/11377 super-editor
![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: 158dd29d64
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } = context; | ||
|
|
||
| // See handleDocumentPartObjectNode for rationale (SD-2557). | ||
| emitPendingSectionBreakForParagraph({ sectionState, nextBlockId, blocks, recordBlockKind }); |
There was a problem hiding this comment.
findParagraphsWithSectPr now counts tableOfContents child paragraphs (sections/analysis.ts), but this handler only emits a pending break once at node entry and never advances sectionState.currentParagraphIndex per TOC paragraph. When a document has a tableOfContents node before later section boundaries, the section cursor falls behind and later sectionBreak blocks are emitted at the wrong point (often only by the end-of-document fallback), which changes pagination compared with Word.
Useful? React with 👍 / 👎.
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
2 participants
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.
Summary
When a Word document stores a
w:sectPron a paragraph immediately before a TOC (or otherdocPartObj) SDT, the nextPage section break was dropped during pm-adapter conversion. The TOC rendered on the same page as the prior section's content instead of starting a new page as Word does.Linear: SD-2557
Root cause
In
packages/layout-engine/pm-adapter/:findParagraphsWithSectPr(sections/analysis.ts) recurses intoindex,bibliography, andtableOfAuthoritiesto count their children as paragraphs, but NOT intodocumentPartObject/tableOfContents.handleParagraphNode,handleIndexNode,handleBibliographyNode, andhandleTableOfAuthoritiesNodeeach emit a pending section break before the paragraph whose index matchesnextSection.startParagraphIndex.handleDocumentPartObjectNodeandhandleTableOfContentsNodedid NOT run this check — so the deferred break only fired on the next body paragraph AFTER the SDT, leaving the SDT's content in the previous section with no page break before it.Fix
Add an
emitPendingSectionBreakForParagraph(sectionState, nextBlockId, blocks, recordBlockKind)helper insections/breaks.tsthat centralizes the "check, emit, advance" pattern, then call it at the top ofhandleDocumentPartObjectNodeandhandleTableOfContentsNode— once per SDT. Since the SDT's children don't affectcurrentParagraphIndex(findParagraphsWithSectPrskips them), the check fires correctly at the SDT boundary: if the SDT sits at a section boundary, the nextPage break is emitted so the SDT renders on a new page.No changes to section-range computation — counting stays consistent.
Verification
Both fixtures from the Linear issue:
Side-by-side page-by-page PDF available at
/tmp/sd-2557-fixtures/SD-2557-comparison.pdf.Test plan
document-part-object.test.ts:@superdoc/pm-adaptertests pass (up from 1737)@superdoc/layout-enginetests passDemo
Related