fix(pdf): budget readiness from the load allowance, return it to the caller#841
Merged
Conversation
📝 WalkthroughWalkthroughThe PDF preparation flow reduces the ChangesPDF readiness budgeting
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/pdf/src/index.js`:
- Around line 122-124: Update the timeout calculation in the waitForReady call
to use nullish coalescing instead of logical OR, so an autoTimeout value of 0 is
preserved and only undefined falls back to timeout.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b9a66df7-7156-48a9-a34a-faca89f36522
📒 Files selected for processing (2)
packages/function/README.mdpackages/pdf/src/index.js
…caller The readiness gate waits for a page to settle — page-load work — but was budgeted from `timeouts.action` (timeout/11), halved by READY_BUDGET_RATIO: ~1.2s. A hydrating document needs 2-3s, so the gate timed out on every tall page and the zero-capture fast path never fired in production. Production, chapter-804 (46 pages), every split trace: ready height=76210 images=29 painted=2 resets=0 timedOut=true duration=1207 <- probe ready height=71325 images=4 painted=2 resets=0 timedOut=false duration=844 <- sibling `duration=1207` is exactly the cap. The starved gate also under-measured the document (36149 vs 38034 from a settled context), which feeds the page count and mis-sizes the split. Budget it from the load allowance goto assigns to this phase instead, at a quarter share (~3.9s at the default request budget). Pages settle in 0.6-3.3s, and the gate returns as soon as the page is quiet, so this caps only a page that never settles. The blank-page poll keeps its own action budget, measured from after the gate so a slow gate cannot starve it. Also return the readiness result from `prepare`. A caller reusing one load across page-ranges (microlink-api's parallel renderer) can then skip that reuse when readiness could not be confirmed, rather than printing a page it never verified had settled. Verified against the local checkout: ch-806 prepare=2595ms timedOut=false height=38034 | render OK ch-804 prepare=1400ms timedOut=false height=71325 | render OK Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`||` treats an explicit `autoTimeout` of 0 as absent and grants a fresh action budget instead. `??` falls back only when the caller passed no budget at all. Not reachable today — goto computes `gotoTimeout` once from the request timeout and never decrements it, so it cannot pass 0 — but `??` is the operator the intent calls for, and correct if goto ever passes a remaining budget.
Kikobeats
force-pushed
the
fix/readiness-budget
branch
from
July 18, 2026 19:04
9847c07 to
f0db59d
Compare
This was referenced Jul 23, 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
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.
Problem
The readiness gate waits for a page to settle — page-load work — but was budgeted from
timeouts.action(timeout/11), then halved byREADY_BUDGET_RATIO: ~1.2s. A hydrating document needs 2-3s, so the gate timed out on every tall page and the zero-capture fast path added in #837/#839 has never fired in production.Every production split trace for a 46-page document shows it:
duration=1207is exactly the cap, every time.Two consequences:
timedOut=truefails its guard, so every render falls through to the screenshot poll — the cost fix(pdf): navigation-tolerant readiness gate; skip screenshot poll for painted pages #837 existed to remove.36149where a settled context reports38034. Callers that derive a page count from that (microlink-api's parallel renderer) mis-size their split.Change
Budget the gate from the load allowance
gotoassigns to this phase, at a quarter share (~3.9s at the default request budget). Observed settle times are 0.6-3.3s, and the gate returns as soon as the page is quiet — so this caps only a page that never settles, it is not a cost added to healthy pages.The blank-page screenshot poll keeps its own action budget, now measured from after the gate, so a slow gate cannot starve it.
Also return the readiness result from
prepare. A caller reusing one load across page-ranges can then skip that reuse when readiness could not be confirmed, instead of printing a page it never verified had settled.preparepreviously returnedundefined; the non-autobranch still does.Verification
Against the local checkout, both previously
timedOut=truein production:chapter-806's height now matches the settled-context value (38034, not 36149).
Caveat: on native GL the old budget may not have timed out either, so this run is consistent with the fix rather than isolating it. The
timedOut=trueevidence is from the GPU-less fleet, and that is where it needs confirming after release.waitForReadyitself is unchanged; its 8 unit tests pass.🤖 Generated with Claude Code
Note
Medium Risk
Changes core PDF auto-wait timing and prepare’s return value for
waitUntil: 'auto', which can affect production render speed, fast-path behavior, and downstream split sizing for callers that consume readiness.Overview
Fixes
waitUntil: 'auto'PDF prep so the readiness gate gets enough time to finish on slow/hydrating pages instead of almost always hittingtimedOut=true.The gate’s cap is now 25% of the phase load timeout
gotopasses intowaitUntilAuto(not the tinytimeouts.actionslice that capped it around ~1.2s). That should let the zero-screenshot fast path run when the page actually settles, and yield more accurate height for split/page-count logic. The blank-page screenshot poll still uses the action budget, but its clock starts after the gate so a long settle doesn’t eat that budget.preparein auto mode now returns the readiness result (includingtimedOut) so callers reusing one load across page ranges can bail out and reload when readiness wasn’t confirmed.Reviewed by Cursor Bugbot for commit f0db59d. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit