fix(pdf): detect covered content in readiness gate, keep zero-screenshot fast path#839
Merged
Conversation
The #837 painted/text fast path trusted in-page DOM signals alone. A page with 200+ chars of SSR text (or a decoded hero image) under a fixed white loading overlay passed waitForReady and skipped the white-screen poll, shipping a blank PDF. Take one cheap probe capture when the gate reports painted content; only skip the poll loop when that capture is non-white. Adds a regression test for the overlay case.
…screenshot fast path A fixed opaque overlay (or white-on-white text) passes every DOM paint signal while a capture stays white, so PR #837's fast path could ship a blank PDF. PR #838 fixed it with a probe screenshot on every painted page, which pays a capture on the fast path and makes the branch equivalent to the poll loop's first iteration. Instead, detect the cover in the in-page snapshot itself: - `covered`: hit-test the counted content samples for an unrelated opaque viewport-filling layer, plus a root-level scan for `pointer-events: none` overlays hit-testing can't see - skip text whose color matches its effective background (invisible on capture) when counting the `text` paint signal - pdf fast path requires `!ready.covered`; the probe screenshot is removed, so a clean painted page takes zero captures again Covered or suspicious pages fall through to the existing blank-SPA screenshot poll, where a capture verifies ground truth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe screenshot readiness snapshot now reports whether painted content is obscured by an opaque layer. PDF ChangesCovered readiness flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PageContent
participant WaitForReady
participant WaitUntilAuto
participant ScreenshotCapture
PageContent->>WaitForReady: report painted content and coverage
WaitForReady->>WaitUntilAuto: return covered readiness state
WaitUntilAuto->>ScreenshotCapture: retain blank/white polling when covered
ScreenshotCapture-->>WaitUntilAuto: return non-white capture
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
coverage: 80.796% (-1.5%) from 82.326% — fix/pdf-covered-fast-path into master |
This was referenced Jul 20, 2026
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
3 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.
Supersedes #838.
Bug
PR #837's fast path trusts DOM paint signals (visible images, 200+ chars of text) to skip the blank-SPA screenshot poll. A fixed opaque overlay (loading screen) or white-on-white text passes every DOM signal while a capture stays white: blank PDF.
Why not #838
#838 fixes it with a probe screenshot on every painted page. That makes the fast path behaviorally identical to the poll loop's first iteration: painted pages pay one capture, the whole point of #837 (zero screenshots for painted content) is gone, and the fall-through case captures twice back-to-back.
Fix
Detect the cover inside the in-page readiness snapshot instead, keeping the fast path screenshot-free:
coveredsignal inwaitForReady's snapshot: hit-test up to 4 counted content samples viaelementFromPoint, walking up from the hit for an unrelated opaque viewport-covering layer (the walk stops at the first common ancestor, whose background always paints below its own descendants). A root-level scan of<body>children catchespointer-events: noneoverlays hit-testing can't see (how fading loaders are styled).textpaint signal.!ready.covered; fix(pdf): verify probe screenshot before skipping blank-SPA poll #838's probe is removed and the poll loop restored. A clean painted page takes zero captures again.painted > 0 || text >= 200), so blank pages pay nothing extra per poll.Covered or suspicious pages fall through to the existing screenshot poll, where a capture verifies ground truth.
Cost
Known limits
Deep-nested
pointer-events: noneoverlays (not direct<body>children) and exotic paint (white canvas, blend modes) escape DOM detection; documented in code. Those pages keep #837's residual risk instead of paying a capture on every request.Validation
pointer-events: noneoverlay, transparent click-catcher, opaque app shell containing the content, white-on-white textstandardclean🤖 Generated with Claude Code
Note
Medium Risk
Changes when PDF auto-wait skips vs runs the screenshot poll, which directly affects blank vs correct output; scope is limited to readiness heuristics with broad new test coverage.
Overview
Fixes blank PDFs when DOM paint signals look ready but a full-viewport loading overlay (or similar) still blocks what a capture would show.
waitForReadysnapshot now exposescoveredand tightens thetextsignal: text that matches its effective background (e.g. white-on-white) no longer counts as painted. Coverage is detected in-page by hit-testing up to four content sample points (elementFromPoint+ walk for unrelated opaque layers) and by scanning directbodychildren forpointer-events: nonefixed/absolute overlays that hit-testing misses. The cover check runs only when the page could qualify for the painted fast path.PDF
waitUntil: autoonly skips the screenshot poll when!ready.coveredin addition to the existing painted / height / font checks, so overlay pages fall through to the existing white-screenshot poll without adding a probe capture on every painted page.New live-browser tests cover overlays, click-catchers, app shells, and white-on-white text; PDF unit tests assert covered fixtures still poll screenshots while clean painted pages stay at zero captures.
Reviewed by Cursor Bugbot for commit 4a68266. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit