[jayzed80]

Fixes #6593

Root cause

next_first_line (crates/layout/src/flow.rs, called from Flow::keep_together, flow.rs:348 on main) measures the room a keep-with-next paragraph needs: its own height plus the paragraphs it is kept with. It recursed only depth < 8 kept paragraphs deep and then counted the next one as an unkept paragraph. In a chain of 69 kept one-line paragraphs, every paragraph therefore "needed" about ten lines. Once those ten lines no longer fitted, the cut-short chain did fit on an empty page (need <= pb.next_room(..)), so the paragraph moved there. The page broke about ten lines short of the bottom, and the rest of the chain moved to the next page (case 1). It is the same in case 2: item 31 sees items 32 to 40 plus the 500 pt picture, which fits on a new page, so the items move with the picture.

The blank page (third case in the comments) comes from the same look-ahead. It counted the heading after the empty kept paragraph and the items after the heading. The heading has w:pageBreakBefore, so it starts a new page anyway. Moving the empty paragraph left it alone on a blank page.

Fix

In crates/layout/src/flow.rs:

• next_first_line is now a bounded loop (chain_room) instead of recursion. It keeps the same eight-paragraph look-ahead (MAX_KEPT_AHEAD), so chains that fit behave as before.
• A chain ends at a paragraph with a page break before it, because nothing after that paragraph needs room on this page. This fixes the blank page.
• keep_together: before moving a keep-with-next paragraph, chain_overflows walks back to the paragraph that starts the chain. The walk stops at a paragraph whose predecessor is not kept, or at a page break before. From there it measures the whole chain against the room on a fresh page. If the chain cannot fit on one page, keep with next gives way and the chain breaks at the page bottom. Only keep lines together can still move that paragraph.
• The measurement stops as soon as the chain is taller than the page. Every count and walk is capped at 512 paragraphs (MAX_CHAIN); a longer chain is treated as too long for one page. Paragraph layouts come from the existing look-ahead cache (Ctx::para_ahead).

docs/layout-parity.md: status line, the keep-with-next row and a revision-history row.

Tests

New tests in crates/layout/src/tests.rs:

• keep_with_next_chain_longer_than_the_page_breaks_at_the_bottom (case 1): the page of every paragraph matches the same document without keep with next.
• keep_with_next_chain_before_a_tall_picture_keeps_its_list_on_the_page (case 2 and the blank-page case): heading and items stay together, the 400 × 500 pt picture goes to the next page alone. With 36 body lines, the empty kept paragraph stays on page 1.

The test document's default font fits 40 lines on a page, so case 2 uses 30 items instead of 40 to keep heading plus items on one page.

Without the fix (flow.rs from upstream/main, tests in place):

test tests::keep_with_next_chain_before_a_tall_picture_keeps_its_list_on_the_page ... FAILED
test tests::keep_with_next_chain_longer_than_the_page_breaks_at_the_bottom ... FAILED
assertion `left == right` failed: a chain too long for a page breaks where unkept lines do
  left: [0, 0, 1 ×31, 2 ×40]   (page 2: heading + 30 lines)
 right: [0, 0, 1 ×40, 2 ×31]   (page 2: heading + 39 lines, the page full)
assertion `left == right` failed: the empty kept paragraph stays on the first page
  left: [0 ×36, 1, 2 ×22, 3 ×10]   (empty paragraph alone on page 2, list split over pages 3 and 4)
 right: [0 ×37, 1 ×31, 2]

With the fix: test result: ok. 3 passed (the keep_with_next_chain filter, including the existing #966 chain test).

Checks run

• cargo test -j 4 -p wordcraft-layout: 716 passed, 3 failed: auto_hyphenation_breaks_long_words, fragmented_hidden_text_lays_out_like_the_visible_text and hyphenation_and_proofing_follow_each_runs_language. All three need hyphenation patterns from CRAFT_I18N_DIR, which this machine does not have. They are hyphenation tests; this change does not touch hyphenation.
• cargo test -j 4 -p wordcraft-engine --no-fail-fast: everything passes except cmd::packs::tests::swiss_german_and_portugal_portuguese_keep_their_languages_rules, which also needs German hyphenation patterns.
• cargo clippy -j 4 -p wordcraft-layout --all-targets -- -D warnings: clean.
• cargo fmt -p wordcraft-layout --check: clean.

Not run

• Full cargo xtask ci (wasm, assets, layers) and the other crates' tests. CI covers them.
• The reporter's DOCX files were not converted end to end. The tests build the same structure in the document model.
• Word itself was not run. The expected results come from the reporter's Word 16.0.20326 measurements in the issue.
• This does not touch page anchors (Place body text frames at page anchors #6535) or kept text frames (DOCX: body paragraphs with page-anchored w:framePr are laid out in the text flow instead of at their frame position #6531).

Summary by CodeRabbit

• Bug Fixes
  • Improved pagination for paragraphs and lists marked to stay together. When a kept chain is longer than a page, it now breaks across pages rather than forcing the entire chain together.
  • Page-break headings and their kept content are handled more consistently, including when large images need to move to the next page.
• Documentation
  • Updated the layout guide with the keep-with-next pagination behavior and related examples.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The layout flow now measures keep-with-next chains against page space and handles chains that exceed it. New regression tests cover long chains and page-break-before behavior. The layout parity document records these cases.

Changes: Keep-with-next pagination

Layer / File(s)	Summary
Measure kept paragraph chains crates/layout/src/flow.rs	Chain measurement accounts for paragraph layout constraints, stops at page breaks, and limits look-ahead and full-chain checks.
Place overlong chains and verify pagination crates/layout/src/flow.rs, crates/layout/src/tests.rs, docs/layout-parity.md	Pagination handles chains that exceed available page space. Tests cover long chains and page-break-before behavior. The parity document records these cases.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: aiongg, silvertree2010

Merge Risk: Low · up to f19be

This change makes an oversized keep-with-next chain break at the page bottom and stops chains at paragraphs with a page break before them. In documents with paragraph spacing or numbered lists, a chain that only just fits on a page may still be split instead of moved to the next page. The effect is limited to near-boundary cases and affects layout fidelity, not data. It can be followed up after merge.

Pre-merge checks: 3 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Out of Scope Changes check		The flow.rs diff (merge base to head) changes only the keep-with-next logic: next_first_line, the new chain_room, chain_overflows, para_ahead_or_lay, and the keep_together guard. All of these match th…	Confirm the docs/layout-parity.md diff. If the layout.fitOnePage history entry was removed, restore it or move that removal to its own change. Keep only the status line, keep-with-next row and the new revision-history row for #6593.

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main behavior change: a keep-with-next chain that is too long for one page now breaks at the page bottom. The raw summary and PR objectives both describe this change in flow.rs, wi…
Linked Issues check		Scope: directly linked issue #6593 only. Its two reduced repros are the coding requirements. The #3850, #933 and #6531 references are context, not objectives. [#6593] Oversized keep-with-next chain b…

Full details: Title check

Explanation

The title names the main behavior change: a keep-with-next chain that is too long for one page now breaks at the page bottom. The raw summary and PR objectives both describe this change in flow.rs, with added tests and a docs update. The title is specific and matches the linked issue #6593. The issue number in parentheses is a minor extra, not a defect.

Full details: Linked Issues check

Explanation

Scope: directly linked issue #6593 only. Its two reduced repros are the coding requirements. The #3850, #933 and #6531 references are context, not objectives.

[#6593] Oversized keep-with-next chain breaks at the page bottom (case 1). In crates/layout/src/flow.rs, keep_together now calls chain_overflows before moving a keep-with-next paragraph. chain_overflows walks back to the chain start (stopping at a page-break-before paragraph or a non-kept predecessor) and measures the whole chain with chain_room, capped at MAX_CHAIN (512). If the chain exceeds the room on a fresh page, a paragraph that does not keep lines stays in place (return before). Only keep lines together can still move it. This removes the premature move that the eight-paragraph look-ahead caused.

[#6593] Tall picture after a kept list (case 2). The look-ahead no longer hands the chain from item 31 onward to a fresh page. The chain from the heading is measured as overflowing, so keep-lines items use only their own height. The list stays on the page. The picture, which keeps lines, moves alone when its own height does not fit. The test keep_with_next_chain_before_a_tall_picture_keeps_its_list_on_the_page covers this with 30 items, a deviation from the 40-item repro that the PR discloses.

[#6593] Blank page. next_first_line returns 0 when the following paragraph has page_break_before, and chain_room stops at such a paragraph. The empty kept paragraph before the page-break-before heading therefore needs only its own height and stays on page 1.

Unverified points: the PR says the reduced-case expectations come from Word measurements, but the issue says these files were not run in Word. The tests were not run by this review. For a chain that starts mid-page and overflows, the code keeps the start in place and breaks at the bottom. The issue text says 'laid out from the top of the page'. The reduced repros start at a page-break-before heading, so they do not test this mid-page case. The full document (#6531 context) was not reproduced.

Full details: Out of Scope Changes check

Explanation

The flow.rs diff (merge base to head) changes only the keep-with-next logic: next_first_line, the new chain_room, chain_overflows, para_ahead_or_lay, and the keep_together guard. All of these match the #6593 objectives. The new tests in crates/layout/src/tests.rs cover those objectives. Both support the change.

The docs/layout-parity.md diff could not be read in full. Its output was truncated at 16000 bytes. The PR summary says the existing 'November 12 layout.fitOnePage history entry is removed'. That removal is unrelated to keep-with-next and would be out of scope. The visible part of the diff shows only the status line. Whether the layout.fitOnePage text was removed or only the date and change text changed is not established.

Resolution

Confirm the docs/layout-parity.md diff. If the layout.fitOnePage history entry was removed, restore it or move that removal to its own change. Keep only the status line, keep-with-next row and the new revision-history row for #6593.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: storytold/wordcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: b02baeb
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3f46b16f-c95e-41cb-a78e-7b51c30a121d

Commits

Reviewing files that changed from the base of the PR and between b02baeb and f19bed0.

Files selected for processing: 3

• crates/layout/src/flow.rs
• crates/layout/src/tests.rs
• docs/layout-parity.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.