feat: implement boundary-aware PDF slicing and tighten report template prose#678
Conversation
…e prose - Add data-pdf-nosplit markers to key images in Section 01 and 02 of the report template. - Implement a boundary-aware slicing loop in generatePDFReport that prevents splitting protected elements across pages. - Normalize Markdown content by collapsing redundant newlines and trimming whitespace. - Implement a custom p renderer for ReactMarkdown to skip empty paragraphs. - Tighten paragraph spacing with prose-p:my-1 Tailwind utilities. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 27 minutes and 29 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe report template now normalizes markdown, suppresses empty paragraphs, and marks selected images as non-splittable. PDF generation now measures those regions, scales the template, and paginates the rendered output around them. ChangesReport rendering and PDF layout
Sequence Diagram(s)sequenceDiagram
participant generatePDFReport
participant html2canvas
participant jsPDF
generatePDFReport->>generatePDFReport: measure templateRect and protected ranges
generatePDFReport->>html2canvas: clone the template at templateWidth
html2canvas->>generatePDFReport: return canvas content
generatePDFReport->>jsPDF: addImage slices with protected cuts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/report-template.tsx`:
- Around line 26-30: The normalization in normalizeMarkdown is collapsing all
repeated newlines before parsing, which also alters fenced/preformatted markdown
content and can corrupt code, logs, or intentionally spaced blocks. Update
normalizeMarkdown in report-template.tsx to track whether the parser is inside a
fenced code block and only collapse extra blank lines when outside fences,
keeping the existing downstream ReactMarkdown rendering intact.
In `@lib/utils/report-generator.ts`:
- Around line 35-61: The DOM style mutation in report generation is not
guaranteed to be restored if measurement or protected range collection fails. In
report-generator.ts, wrap the `element.style.width` and `element.style.position`
mutation around the `getBoundingClientRect`/`querySelectorAll` logic in a
`try/finally`, and restore the original inline style in the `finally` block; if
`originalStyle` was empty, remove the `style` attribute instead of setting it
back.
- Around line 100-105: The no-split handling in report generation is still using
the 10px layout buffer, which can split protected elements that start close to
the current position. Update the cut logic in report-generator’s straddle
handling to rely on the loop’s 0.5 progress threshold instead of
`currentPosition + 10`, while keeping the existing `elementHeight <= pdfHeight`
guard, so elements near the next cut are only moved when they actually belong to
the next page.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4fa0b616-bf12-4bb5-8d2b-cb3d1c2a9d93
📒 Files selected for processing (2)
components/report-template.tsxlib/utils/report-generator.ts
📜 Review details
🧰 Additional context used
🪛 React Doctor (0.5.8)
components/report-template.tsx
[warning] 188-188: Your users can see & submit the wrong data when this list reorders or filters, so use a stable id like key={item.id}, not the array index "index".
Use a stable id from the item, like key={item.id} or key={item.slug}. Index keys break when the list reorders or filters.
(no-array-index-as-key)
[warning] 188-188: Your users can see & submit the wrong data when this list reorders.
Use a stable key from your data so reordered items keep the right state and DOM.
(no-array-index-key)
🔇 Additional comments (2)
components/report-template.tsx (1)
32-42: LGTM!Also applies to: 149-149, 216-218, 227-227
lib/utils/report-generator.ts (1)
4-29: LGTM!Also applies to: 63-79, 81-99, 108-113
| const normalizeMarkdown = (content: string): string => { | ||
| return content | ||
| .replace(/\n{3,}/g, '\n\n') // Collapse 3+ newlines to 2 | ||
| .trim() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid normalizing inside fenced markdown blocks.
Line 28 collapses all 3+ newlines before markdown parsing, including inside fenced/preformatted blocks. That can corrupt report content such as code, logs, or intentionally spaced examples rendered through the downstream ReactMarkdown calls.
Proposed fix
const normalizeMarkdown = (content: string): string => {
- return content
- .replace(/\n{3,}/g, '\n\n') // Collapse 3+ newlines to 2
- .trim()
+ let inFence = false
+ let blankCount = 0
+
+ return content
+ .trim()
+ .split('\n')
+ .filter(line => {
+ if (/^\s*(```|~~~)/.test(line)) {
+ inFence = !inFence
+ }
+
+ if (!inFence && !line.trim()) {
+ blankCount += 1
+ return blankCount <= 1
+ }
+
+ blankCount = 0
+ return true
+ })
+ .join('\n')
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const normalizeMarkdown = (content: string): string => { | |
| return content | |
| .replace(/\n{3,}/g, '\n\n') // Collapse 3+ newlines to 2 | |
| .trim() | |
| } | |
| const normalizeMarkdown = (content: string): string => { | |
| let inFence = false | |
| let blankCount = 0 | |
| return content | |
| .trim() | |
| .split('\n') | |
| .filter(line => { | |
| if (/^\s*( |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/report-template.tsx` around lines 26 - 30, The normalization in
normalizeMarkdown is collapsing all repeated newlines before parsing, which also
alters fenced/preformatted markdown content and can corrupt code, logs, or
intentionally spaced blocks. Update normalizeMarkdown in report-template.tsx to
track whether the parser is inside a fenced code block and only collapse extra
blank lines when outside fences, keeping the existing downstream ReactMarkdown
rendering intact.
| const originalStyle = element.getAttribute('style') || '' | ||
| element.style.width = `${templateWidth}px` | ||
| element.style.position = 'relative' | ||
|
|
||
| const templateRect = element.getBoundingClientRect() | ||
| const pdf = new jsPDF({ | ||
| orientation: 'portrait', | ||
| unit: 'px', | ||
| format: 'a4' | ||
| }) | ||
|
|
||
| const pdfWidth = pdf.internal.pageSize.getWidth() | ||
| const pdfHeight = pdf.internal.pageSize.getHeight() | ||
| const pdfScale = pdfWidth / templateWidth | ||
|
|
||
| const protectedRanges = Array.from(element.querySelectorAll('[data-pdf-nosplit]')) | ||
| .map(el => { | ||
| const rect = el.getBoundingClientRect() | ||
| return { | ||
| top: (rect.top - templateRect.top) * pdfScale, | ||
| bottom: (rect.bottom - templateRect.top) * pdfScale | ||
| } | ||
| }) | ||
| .sort((a, b) => a.top - b.top) | ||
|
|
||
| // Restore original style | ||
| element.setAttribute('style', originalStyle) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Restore the mutated DOM style in a finally block.
If measurement or range derivation throws after Lines 36-37, the report DOM keeps the temporary inline width/position. Wrap the mutation in try/finally and remove the style attribute when it was originally absent.
Proposed fix
-const originalStyle = element.getAttribute('style') || ''
-element.style.width = `${templateWidth}px`
-element.style.position = 'relative'
+const originalStyle = element.getAttribute('style')
+let protectedRanges: Array<{ top: number; bottom: number }> = []
-const templateRect = element.getBoundingClientRect()
const pdf = new jsPDF({
orientation: 'portrait',
unit: 'px',
format: 'a4'
})
const pdfWidth = pdf.internal.pageSize.getWidth()
const pdfHeight = pdf.internal.pageSize.getHeight()
const pdfScale = pdfWidth / templateWidth
-const protectedRanges = Array.from(element.querySelectorAll('[data-pdf-nosplit]'))
- .map(el => {
- const rect = el.getBoundingClientRect()
- return {
- top: (rect.top - templateRect.top) * pdfScale,
- bottom: (rect.bottom - templateRect.top) * pdfScale
- }
- })
- .sort((a, b) => a.top - b.top)
-
-// Restore original style
-element.setAttribute('style', originalStyle)
+try {
+ element.style.width = `${templateWidth}px`
+ element.style.position = 'relative'
+
+ const templateRect = element.getBoundingClientRect()
+ protectedRanges = Array.from(element.querySelectorAll('[data-pdf-nosplit]'))
+ .map(el => {
+ const rect = el.getBoundingClientRect()
+ return {
+ top: (rect.top - templateRect.top) * pdfScale,
+ bottom: (rect.bottom - templateRect.top) * pdfScale
+ }
+ })
+ .sort((a, b) => a.top - b.top)
+} finally {
+ if (originalStyle === null) {
+ element.removeAttribute('style')
+ } else {
+ element.setAttribute('style', originalStyle)
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const originalStyle = element.getAttribute('style') || '' | |
| element.style.width = `${templateWidth}px` | |
| element.style.position = 'relative' | |
| const templateRect = element.getBoundingClientRect() | |
| const pdf = new jsPDF({ | |
| orientation: 'portrait', | |
| unit: 'px', | |
| format: 'a4' | |
| }) | |
| const pdfWidth = pdf.internal.pageSize.getWidth() | |
| const pdfHeight = pdf.internal.pageSize.getHeight() | |
| const pdfScale = pdfWidth / templateWidth | |
| const protectedRanges = Array.from(element.querySelectorAll('[data-pdf-nosplit]')) | |
| .map(el => { | |
| const rect = el.getBoundingClientRect() | |
| return { | |
| top: (rect.top - templateRect.top) * pdfScale, | |
| bottom: (rect.bottom - templateRect.top) * pdfScale | |
| } | |
| }) | |
| .sort((a, b) => a.top - b.top) | |
| // Restore original style | |
| element.setAttribute('style', originalStyle) | |
| const originalStyle = element.getAttribute('style') | |
| let protectedRanges: Array<{ top: number; bottom: number }> = [] | |
| const pdf = new jsPDF({ | |
| orientation: 'portrait', | |
| unit: 'px', | |
| format: 'a4' | |
| }) | |
| const pdfWidth = pdf.internal.pageSize.getWidth() | |
| const pdfHeight = pdf.internal.pageSize.getHeight() | |
| const pdfScale = pdfWidth / templateWidth | |
| try { | |
| element.style.width = `${templateWidth}px` | |
| element.style.position = 'relative' | |
| const templateRect = element.getBoundingClientRect() | |
| protectedRanges = Array.from(element.querySelectorAll('[data-pdf-nosplit]')) | |
| .map(el => { | |
| const rect = el.getBoundingClientRect() | |
| return { | |
| top: (rect.top - templateRect.top) * pdfScale, | |
| bottom: (rect.bottom - templateRect.top) * pdfScale | |
| } | |
| }) | |
| .sort((a, b) => a.top - b.top) | |
| } finally { | |
| if (originalStyle === null) { | |
| element.removeAttribute('style') | |
| } else { | |
| element.setAttribute('style', originalStyle) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/utils/report-generator.ts` around lines 35 - 61, The DOM style mutation
in report generation is not guaranteed to be restored if measurement or
protected range collection fails. In report-generator.ts, wrap the
`element.style.width` and `element.style.position` mutation around the
`getBoundingClientRect`/`querySelectorAll` logic in a `try/finally`, and restore
the original inline style in the `finally` block; if `originalStyle` was empty,
remove the `style` attribute instead of setting it back.
| // Only move the cut if the element starts after our current position (with small buffer) | ||
| // and fits on a single page. | ||
| if (straddle && straddle.top > currentPosition + 10) { | ||
| const elementHeight = straddle.bottom - straddle.top | ||
| if (elementHeight <= pdfHeight) { | ||
| effectiveCut = straddle.top |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don’t let the 10px buffer split protected elements.
A no-split element that starts within 10px of currentPosition and extends past nextPossibleCut is still split, even when elementHeight <= pdfHeight. Use the loop’s 0.5 progress threshold instead of a 10px layout buffer.
Proposed fix
-// Only move the cut if the element starts after our current position (with small buffer)
+// Only move the cut if it advances the cursor and the element fits on a single page.
// and fits on a single page.
-if (straddle && straddle.top > currentPosition + 10) {
+if (straddle && straddle.top > currentPosition + 0.5) {
const elementHeight = straddle.bottom - straddle.top
if (elementHeight <= pdfHeight) {
effectiveCut = straddle.top
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Only move the cut if the element starts after our current position (with small buffer) | |
| // and fits on a single page. | |
| if (straddle && straddle.top > currentPosition + 10) { | |
| const elementHeight = straddle.bottom - straddle.top | |
| if (elementHeight <= pdfHeight) { | |
| effectiveCut = straddle.top | |
| // Only move the cut if it advances the cursor and the element fits on a single page. | |
| // and fits on a single page. | |
| if (straddle && straddle.top > currentPosition + 0.5) { | |
| const elementHeight = straddle.bottom - straddle.top | |
| if (elementHeight <= pdfHeight) { | |
| effectiveCut = straddle.top |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/utils/report-generator.ts` around lines 100 - 105, The no-split handling
in report generation is still using the 10px layout buffer, which can split
protected elements that start close to the current position. Update the cut
logic in report-generator’s straddle handling to rely on the loop’s 0.5 progress
threshold instead of `currentPosition + 10`, while keeping the existing
`elementHeight <= pdfHeight` guard, so elements near the next cut are only moved
when they actually belong to the next page.
|
10:31:15.734 Running build in Washington, D.C., USA (East) – iad1 10:32:34.127 134 | 10:32:34.128 > 135 | 10:32:34.128 | ^ 10:32:34.128 136 | {normalizeMarkdown(aiSummary)} 10:32:34.128 137 | 10:32:34.128 138 | 10:32:34.172 Next.js build worker exited with code: 1 and signal: null 10:32:34.179 error: script "build" exited with code 1 10:32:34.195 Error: Command "bun run build" exited with 1 |
- Update `MarkdownComponents` in `components/report-template.tsx` to make `children` optional. - This ensures compatibility with `react-markdown`'s `Components` type and resolves the build failure. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
PR created automatically by Jules for task 17795820845734913689 started by @ngoiyaeric
Summary by CodeRabbit