fix(policies): keep PDF headings with their section (CS-704)#3358
Conversation
Policy PDF exports orphaned section headings at the bottom of a page while the body flowed to the next page. Both shared generators only checked that the heading line itself fit before rendering it — there was no keep-together logic, so a heading could land on the last line of a page. Before rendering a heading, reserve room for the heading PLUS the first few lines of the following section (HEADING_KEEP_WITH_LINES); if that doesn't fit, push the whole heading to the next page. The reserve accounts for each generator's actual cursor advancement (the app's per-line lineHeight checks vs. the API renderer's DEFAULT_BREAK_SPACE look-ahead) and only applies when content actually follows the heading, so a trailing heading isn't bumped to a page of its own. Heading lines now also render without per-line page-break checks, so a multi-line heading can no longer be split across pages. Applied to both: - apps/app/src/lib/pdf-generator.ts (single-policy in-app export) - apps/api/src/trust-portal/policy-pdf-renderer.service.ts (full policy-pack export in-app and via the Trust Portal) Adds a regression test that sweeps headings across page boundaries and asserts none are separated from the first line of their section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013q6YNeuzKbUdX4yZ3yTnZk
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
cubic analysis
2 issues found across 3 files
Confidence score: 3/5
- In
apps/api/src/trust-portal/policy-pdf-renderer.service.ts, thehasFollowingContentcheck appears to treat trailing empty paragraphs/hard breaks as real content, so headings may reserve extra keep-together space unnecessarily and force avoidable page breaks or layout gaps in generated PDFs — tighten the guard to ignore empty trailing nodes before merging. - In
apps/api/src/trust-portal/policy-pdf-renderer.service.ts, the heading rendering loop no longer does per-line page-break checks, so very long wrapped headings can run past the bottom margin and produce clipped/overflowed output — restore per-line pagination checks (or equivalent overflow handling) before merging.
Linked issue analysis
Linked issue: CS-704: create a ticket in team CS "[Bug] - Policy PDF export splits sections across multiple pages
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Headings are not orphaned at page bottoms — a heading stays on the same page as the first lines of its section | The PR adds explicit keep‑together checks before rendering headings so the heading plus following lines fit or the heading is pushed to the next page. The API tests include a sweep that fails before the fix and passes after it. |
| ✅ | Fix applied to both single-policy (app) and full-pack (API) exporters | Both the app generator and the API renderer were updated so the same keep‑together behavior applies across exports. |
| ✅ | Reserve logic implemented: require heading + first N lines (HEADING_KEEP_WITH_LINES = 3) with generator-specific measurement | PR introduces HEADING_KEEP_WITH_LINES and uses generator-specific calculations (lineHeight for app; DEFAULT_BREAK_SPACE look‑ahead for API) when computing required space before rendering a heading. |
| ✅ | Trailing headings are not pushed to their own page when no content follows | The implementation only reserves following-section space when content actually follows a heading, and there is a regression test to ensure a trailing heading stays on the current page. |
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Two issues raised by cubic on the CS-704 keep-together change: 1. hasFollowingContent only checked array position, so a heading followed only by empty paragraphs or hard breaks (common trailing nodes in TipTap docs) still reserved keep-together space and could be bumped to a lonely page — defeating the trailing-heading guard. Now checks whether any following sibling renders visible text (hasRenderableTextAfter). Applied to both the app and API generators. 2. The API heading render loop dropped per-line page-break checks, so a pathological heading wrapping past a full page would overflow the bottom margin instead of paginating. Restored a per-line check as a safety net; the up-front keep-together reserve means normal headings never trip it, so they still can't be split across pages. Adds regression tests for both (verified failing without the fixes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013q6YNeuzKbUdX4yZ3yTnZk
|
@cubic-dev-ai review it |
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
cubic analysis
2 issues found across 3 files
Confidence score: 3/5
- In
apps/api/src/trust-portal/policy-pdf-renderer.service.ts, heading pagination can fail whenrequiredHeightexceeds usable page height, sincecheckPageBreakonly advances one page and never clamps; very tall wrapped headings can produce blank/invalid layout pages for users — clamp/split oversized heading blocks (or loop page-break checks) before merging. - In
apps/api/src/trust-portal/policy-pdf-renderer.service.ts, the fixed keep-together reserve only protects headings followed by plain paragraphs, so headings before other block types can still orphan, which conflicts with CS-704 and can degrade document readability — apply keep-with-next logic consistently across block types and add a regression test for mixed content.
Linked issue analysis
Linked issue: CS-704: create a ticket in team CS "[Bug] - Policy PDF export splits sections across multiple pages
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Keep headings with the first few lines of their section (add keep‑together logic so headings are not orphaned at page bottoms). | PR adds an explicit keep‑together reserve (HEADING_KEEP_WITH_LINES = 3) and uses it when deciding page breaks, ensuring a heading is pushed to the next page if its following lines don't fit. |
| ✅ | Apply the fix to both single‑policy (in‑app) and full‑pack (API/Trust Portal) PDF generators. | Both the app generator and the API renderer were updated with matching keep‑together logic and in‑code sync comments. |
| ✅ | Do not push a trailing heading (a heading with no following visible content) onto its own page. | The implementation checks whether visible content actually follows a heading before reserving keep‑together space; tests assert trailing headings stay on the same page. |
| ✅ | Handle multi‑line or pathological headings safely (don't silently overflow; paginate long headings rather than let them overflow or split normal headings across pages). | Headings are measured up‑front so normal multi‑line headings are kept intact; per‑line checks act as a safety net so extremely long headings paginate correctly. A test verifies a very long heading spans multiple pages rather than overflowing. |
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…bic) Second cubic pass on the CS-704 change raised two more edge cases: 1. The keep-together reserve was a fixed text-only slab, so a heading immediately followed by a table with a tall first row (or by leading hard breaks) could still be orphaned — the table's own per-row page-break fires after the heading already committed. Replaced the position/text check with sectionLeadHeight(), which measures what the following section actually needs: a few text lines, a table's first-row height, or 0 for a trailing heading (skipping empty paragraphs and hard breaks). 2. An oversized heading (wrapping past a full page) could emit a blank leading page: requiredHeight exceeded the usable page height, so the up-front checkPageBreak added a page while the current one was still empty. Capped requiredHeight to the usable page height; the per-line loop paginates the heading across pages instead. Applied to both generators (app + API). Adds regression tests for a heading-before-tall-table and the blank-page guard (both verified failing without the fixes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013q6YNeuzKbUdX4yZ3yTnZk
|
@cubic-dev-ai review it |
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
cubic analysis
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Linked issue analysis
Linked issue: CS-704: create a ticket in team CS "[Bug] - Policy PDF export splits sections across multiple pages
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Add keep‑together logic so a heading is only rendered if it plus the first few lines of its section fit on the page (HEADING_KEEP_WITH_LINES = 3). | Both generators introduce HEADING_KEEP_WITH_LINES and compute a heading reserve before rendering. |
| ✅ | Prevent orphaned headings — headings must share a page with the first line(s) of their section. | Heading rendering now reserves space for the heading plus section lead and the API tests assert no heading is orphaned across many offsets. |
| ✅ | Apply the fix to both single‑policy (app) and full‑pack / API generators. | Both the app module and the API renderer were updated with matching keep‑together logic and notes to keep them in sync. |
| ✅ | Do not push a trailing heading (no visible following content) onto a fresh page. | The code only reserves lead space when visible content follows; there is a unit test covering trailing headings. |
| ✅ | Handle tables: reserve the table first‑row height so a heading followed by a tall table row is not orphaned. | The renderer measures a table's first row and includes it in the reserve; test ensures headings stay with table first rows. |
| ✅ | Avoid inserting a blank leading page for an oversized heading (cap the reserve to usable page height). | Required reserve is capped to usable page height; test asserts the first page carries the oversized heading rather than a blank page before it. |
| ✅ | Safety net: very long headings paginate rather than overflow the page. | Per‑line checks remain for pathological multi‑page headings; test verifies a very long heading spans pages. |
|
🎉 This PR is included in version 3.98.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What & why
Fixes CS-704 — policy PDF exports split sections across pages: a section heading landed at the bottom of a page while its body flowed to the next page (orphaned heading). Reported for both single-policy and full-pack exports.
Root cause: both PDF generators computed the heading page-break wrong. They only guaranteed the heading (or a fixed slab) fit before rendering it — never the heading plus the first lines of its section — so a heading could render on the last usable line of a page and its body would break to the next page.
Fix
Before rendering a heading, reserve room for the heading plus the first few lines of the following section (
HEADING_KEEP_WITH_LINES = 3). If it doesn't fit, the whole heading is pushed to the next page. The reserve is tuned to each generator's real cursor advancement:apps/app/src/lib/pdf-generator.ts) — powers the single-policy in-app export (the fallback used whenever a policy has no manually-uploaded PDF). Per-line body checks uselineHeight, so the reserve isheadingHeight + spacingAfter + keepWithLines·lineHeight.apps/api/src/trust-portal/policy-pdf-renderer.service.ts) — powers the full policy-pack export (in-app and the Trust Portal) viarenderPoliciesPdfBuffer. ItscheckPageBreakuses a 20mm (DEFAULT_BREAK_SPACE) look-ahead per body line, so the reserve adds one such look-ahead.These two files carry explicit "keep in sync" markers, so both were updated together.
Additional correctness improvements:
Out of scope: the ticket's soft "Consider basic orphan/widow control" bullet. The reported, reproducible defect is stranded headings; aggressive block-level widow control would repaginate every existing policy and risks new layout regressions, so it's intentionally deferred.
Tests
apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts:cd apps/api && npx jest src/trust-portal/policy-pdf-renderer.service.spec.ts→ 22 passed. Typecheck of both touched files is clean (repo has an unrelated pre-existing RED baseline).The app generator applies the identical algorithm (proven by the API spec); it isn't separately unit-tested because it's a browser module that triggers a
doc.save()download and doesn't expose the buffer — testing it would need a refactor out of scope for this fix.🤖 Generated with Claude Code
Summary by cubic
Keeps section headings with the start of their section in policy PDF exports (CS-704), including cases with tables and oversized headings. Works for both single‑policy and full‑pack PDFs.
HEADING_KEEP_WITH_LINES = 3): only render a heading if it and the next 3 body lines fit; otherwise move it to the next page (matches the Linear ask).sectionLeadHeight: skips empty paragraphs/hard breaks, handles tables by using the first row’s height, and only reserves space when visible content follows (trailing headings stay on the current page).headingHeight + spacingAfter + keepWithLines·lineHeight; API adds aDEFAULT_BREAK_SPACElook‑ahead. The reserve is capped to the usable page height to avoid blank leading pages for oversized headings. Applied to both generators.Written for commit 439a453. Summary will update on new commits.