Repository navigation
Conversation
…cale and viewport Snapshots assumed every tab renders at 2x in a 1280x800 viewport. A tab the desktop draws keeps the display's scale and its panel's viewport, so at 150% scaling the PNG came out 960x600 while the result reported 1280x800, and a viewport of another size was clipped wrongly. For those tabs the snapshot now reads devicePixelRatio and the viewport from the page and divides out the zoom the desktop applied. The reported size is read from the PNG header. Headless tabs capture as before. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| "({ ratio: devicePixelRatio, width: innerWidth, height: innerHeight })", | ||
| )) as { readonly ratio: number; readonly width: number; readonly height: number }; | ||
| return { | ||
| renderScale: page.ratio / tab.zoomFactor, |
There was a problem hiding this comment.
🟠 High preview/ServerBrowser.ts:1288
A snapshot requested immediately after a server-side zoom change can be cropped or incorrectly scaled: tab.zoomFactor is updated before the desktop applies setZoomFactor, so this calculation combines the old devicePixelRatio and viewport with the new zoom. Derive the zoom from acknowledged page state or wait for the desktop to apply it before calculating snapshot dimensions.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/preview/ServerBrowser.ts around line 1288:
A snapshot requested immediately after a server-side zoom change can be cropped or incorrectly scaled: `tab.zoomFactor` is updated before the desktop applies `setZoomFactor`, so this calculation combines the old `devicePixelRatio` and viewport with the new zoom. Derive the zoom from acknowledged page state or wait for the desktop to apply it before calculating snapshot dimensions.
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a focused server-side preview snapshot bug fix that derives desktop captures from the page’s actual scale and viewport while preserving headless behavior, with targeted tests covering the new cases. An unresolved High-severity finding identifies a possible zoom-update race that could still produce incorrectly scaled or cropped snapshots. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughDesktop-rendered snapshots now use page metrics and tab zoom to set capture scale and viewport dimensions. Snapshot metadata uses dimensions read from the PNG when available. Headless snapshots retain their fixed render scale. ChangesPreview snapshot sizing
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to No concrete issue remains that blocks merging. Real desktop-tab page zoom has not been validated, so its snapshot sizing remains worth checking. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change adds no new browser actions or access rights. Its exposure is limited to existing preview captures, but the effect of abnormal page-reported sizes has not been verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This branch has not been deployed
1 participant
Problem
preview_snapshotassumes a 2x render scale and falls back to a 1280×800 viewport when Playwright has no viewport size, as with desktop-drawn tabs. It asks Chromium for a clip at scale 0.5 over that viewport, and Chromium applies the scale to the display's real pixels. A headless tab runs at 2x in a viewport Playwright set, so its PNG is 1280×800. A tab the desktop app draws keeps the display's scale and the viewport its panel gives it:Change
For a tab the desktop draws, the snapshot reads
devicePixelRatio,innerWidth, andinnerHeightfrom the page. It divides out the zoom the desktop applied (the server publishes that zoom, and the desktop sets it on the webview), then uses the display scale and the viewport for the clip. The reported size is now read from the PNG itself. Headless tabs keep the fixed 2x and Playwright's viewport, so their captures are unchanged, including zoomed ones.Scope and approval
Closes #16690. The triage comment confirms the bug on
mainand names this approach as option (a): read the page's realdevicePixelRatiofor desktop-drawn tabs and use it in the scale and reported-size math. Option (b), forcing 2x emulation on the visible webview for each capture, would repaint the page the user is watching.Verification
An agent ran every check. No person tested or reviewed the change. The user resized and zoomed the dev app's window and sent the agent's prompt on request.
Automated (Linux,
apps/server):vp test run src/preview: 134 passed.ServerBrowser.test.ts, for a desktop-drawn tab: a page at ratio 1.5 and 1280×800 is clipped at scale 2/3 and reported as 1280×800; a page at 1067×667 is clipped to 1067×667; a page at 200% zoom reports ratio 3 and is still clipped at scale 2/3. All three fail againstmain'sServerBrowser.tsandServerBrowserPage.ts. A headless tab keeps clip scale 0.5 and never reads the page's ratio, on both.ServerBrowserPage.test.ts: real Chromium launched at 1x, 1.5x, and 2x with the matching render scale gives a 1280×800 PNG reported as 1280×800. Another passes a viewport one pixel off the page's real 539×939, the rounding a zoomed desktop page can produce, and checks the reported size is still the PNG's 539×939. It passes onmaintoo, which has no viewport input. Withmain's fixed 2x the same setups give 640×400, 960×600, and 1280×800, all reported as 1280×800.apps/servertypecheck passes. Lint and format pass on the changed files.Live, agent-operated: the desktop app from this branch (
vp run dev:desktop) on Windows 11 at 150% scaling,preview_snapshoton a local page with fine text and one-pixel lines.mainbbc11231b)main, 1280×800 viewport, 960×600:This branch at
bbc11231b, which handled this case the same way, 1280×800 viewport, 1280×800:This branch, 1067×667 viewport, 1280×800:
This branch, 539×939 viewport, 809×1409:
All but the narrow capture are soft because the webview was drawn smaller than the page (#9872). This PR changes size and coverage, not that.
Not checked: page zoom on a real desktop tab (no zoom control was found for server tabs; covered by the test above), 100% and 250% display scaling, macOS, and recordings, which capture at full scale. The clip origin still uses CSS page offsets, as on
main, so a zoomed desktop tab that is scrolled would capture from a point off by the zoom factor.Model: Claude Opus 5.5, Claude Fable 5.1 (review), GPT-6-Astra (review). Harness: Claude Code, Codex.