💭 I'm checking the diff for these preview browser files against the UI consistency criteria—shared primitives, Tailwind usage, styling patterns—but the changes seem focused on behavior like CDP session handling rather than styling, so I want to look at the actual head code to confirm.

1. browse_code — apps/web/src/browser/HostedBrowserWebview.tsx

2. browse_code — apps/web/src/components/preview/previewGuestViewport.ts

💭 Checking whether the previously flagged DevTools detach issue was resolved, I see the diff now restores viewportOverride in restoreControlSession, so that concern is addressed. Otherwise this PR is mostly desktop/IPC plumbing with minimal UI styling changes, the key user-facing behavior being that the guest viewport override applies during live dragging via the effective viewport. I also notice the code assigns the current effective viewport into a ref directly during render, which is a React anti-pattern that could produce stale writes under concurrent rendering or StrictMode, though it's borderline whether this counts as a UI-consistency finding versus a purely behavioral React concern. Checking other angles: whether the override applies correctly when the tab is hidden or inactive given this PR's focus on hidden viewport fixes, whether stale overrides linger past tab close (they seem removed properly), and whether duplicate registration paths cause harmless redundancy. Mobile-specific metrics override logic is out of scope for this desktop-focused change, so I want to shift attention to the readiness gating in the PreviewAutomationHosts file.

3. browse_code — apps/web/src/components/preview/PreviewAutomationHosts.tsx

💭 I'm tracing how automation-triggered viewport resizing flows through the store into HostedBrowserWebview's effect, which re-applies the viewport non-automation style—this seems idempotent and not a real consistency conflict, just something worth double-checking. Since this is a UI Consistency check focused on primitives/CSS, and the changed files don't introduce new markup or classes, this could pass cleanly. But given the check tends to default toward finding issues, I want to verify there's no real behavioral regression—specifically checking whether desktopByTabId is keyed by server tabId or runtime tab id, since that mismatch could cause a genuine bug in the preview state store.

4. git_tools — desktopByTabId

💭 I'm confirming that desktopByTabId is keyed correctly by server tabId. I also notice that useThreadPreviewState(threadRef) subscribes to the entire thread preview state, which triggers re-renders on any preview change—a potential performance concern, especially since other components use useShallow to avoid this issue while HostedBrowserWebview doesn't. That said, frequent re-renders aren't really in scope for this UI consistency check, so I want to look at the useThreadPreviewState signature to see what selector pattern it typically uses.

5. git_tools — useThreadPreviewState

💭 That seems consistent with other usage. Now I'm considering actual UI regressions this PR could introduce during resize drags — the effect fires on each viewport key change, sending device metrics overrides via IPC per frame to keep the guest in sync. I'm noting that for fill mode, since the viewport key stays stable, the effect only fires once on mount, sending a clear signal. There's also the question of multiple browser view instances, each with its own runtime tab ID, and whether the host webview element resizes in step with the guest during drag — they should track the same effective viewport, so that's consistent. I'm also examining the register path and whether the effect dependencies properly handle the guest viewport ref at mount time, and whether the swap during crash recovery could leave hasWebContents stale so the effect fails to re-fire on a new webContents instance. Now I'm thinking about zoom factor handling — the CDP override sets a fixed device scale factor while the host applies its own zoom, and I want to trace whether the element's CSS pixel width matches what the toolbar displays given the emulation override, and whether the mobile emulation heuristic on the desktop side interacts correctly with this. That mobile-emulation flag lives in Manager.ts on the desktop side, but my comments need to target in-scope web files, so I can only really attach observations to previewGuestViewport.ts where the viewport mapping actually lives, even though the mobile decision itself is out of scope. Let me instead trace the automation resize path through HostedBrowserWebview's applier — how the persist-to-server call interacts with the store update and the rAF-driven re-render, checking whether the host reapplying its override after a rollback could race with automation's own override application.

6. git_tools — runBrowserViewportMutation|browserViewportSettingKey|function useBrowserViewportResize

7. browse_code — apps/web/src/browser/browserViewportLayout.ts

8. browse_code — apps/web/src/browser/useBrowserViewportResize.ts

💭 I'm noticing that during a drag, the effective viewport is always freeform, even if the committed viewport was a preset, and that resolves fine once commitDrag finalizes it. But I'm spotting a subtle timing gap: after commitDrag updates the store with the freeform width/height but before the drag state itself is cleared, there could be a brief window where the state is inconsistent. Checking fitSourceContent behavior, I confirm that fixed viewports get their own fitted size while fill viewports get their fitted freeform size applied to the element, with the guest override cleared so the guest measures the same size — this stays consistent. The real issue I'm circling toward is what happens with hidden or inactive tabs, since the PR title points specifically at "fix/preview-hidden-viewport," where the wrapper gets positioned offscreen with a hidden size while the webview itself may not update correctly. For a fixed viewport this hidden size roughly matches the intended dimensions, but for a fill viewport it falls back to a default or content size, meaning the guest reports that fallback size while hidden — this looks like pre-existing behavior rather than a new bug. I also look at how guestViewportRef.current gets mutated on every render and read inside a rAF callback in the effect, which seems intentional for coalescing updates rather than a race condition, and I confirm the effect properly reruns on the relevant dependency changes. The more promising lead is whether the webview element gets recreated when runtimeTabId stays the same but something else changes. I check whether hasWebContents might toggle false then true again after a crash and recreation, since the manager clears webContents on destroy — this seems handled correctly since it's a dependency of the effect. Then I think about a real potential regression: applying the device metrics override on every pointermove during a resize drag could cause heavy re-layout and visible flicker, and more importantly the mobile/desktop breakpoint at 768px could flip repeatedly mid-drag, causing the guest to toggle rendering modes while dragging — a real UI issue, though the actual fix would live in different code than what I'm reviewing. I'm reconsidering whether this is actually a bug or an intentional tradeoff, since drag-live application was explicitly requested in an earlier review comment. I want to verify there's no staleness issue with the viewport ref used in the registration effect, since it's set during render but the effect could read it before it updates. I'm also noticing a couple minor code smells — a redundant catch on an already-wrapped promise, and a preview bridge reference captured slightly differently than expected — but nothing critical. Now I'm tracing whether using the unselective preview state hook causes unnecessary re-renders on every navigation event, and whether that cascades into layout recomputation since the layout object gets recreated each render. I'm worried this could create an infinite render loop: presentContent updates the store, which HostedBrowserWebview subscribes to via useShallow, and since content is a new object reference each time, that could trigger another re-render and another presentContent call. But this seems like pre-existing behavior, so I want to check whether presentContent already dedupes identical content.

9. browse_code — apps/web/src/browser/browserSurfaceStore.ts

💭 No clear issue turns up from deduping that path, so I'm reconsidering scope—there's no markup or class changes in the PR, and I'm wondering whether the UI consistency check applies more broadly given prior behavioral comments. I want to check if the latest commit actually addressed the previous review comment. Yes—restoreControlSession now re-applies the viewport override per the diff. I'm tracing whether it's also triggered on devtools-closed and webview swap events, and I notice applyViewportOverride is referenced before its const declaration within the same function body—but since the whole generator function completes before any invocation happens, this isn't actually a TDZ problem, matching the same pattern likely used for applyColorScheme elsewhere (though that's outside this file's scope anyway). Looking at hasWebContents gating, the effect correctly skips applying when there's no web content yet, and both the registration path and the effect path handle this consistently. I'm considering the unmount case—if HostedBrowserWebview unmounts while the session persists, the override stays in the manager and gets reapplied on re-attach, which is actually the desired behavior since overrides are keyed by tabId and only cleared on tab close. Now I'm checking how the automation host wires up the viewport setter through the bridge to see if there's a similar consistency there. I'm now working through whether automation resizes and human resizes could double-apply the same override redundantly, and whether that redundant IPC call through the human path's control session setup could cause any unintended side effects like attaching a debugger session without properly marking it as agent-controlled. There's also a subtler race to consider: if automation triggers a resize that fails its viewport check and rolls back, but the host's rAF-scheduled apply from the earlier store update lands after that rollback completes, the guest could end up pinned at a stale size while the UI shows the rolled-back value. Given the rollback happens on a multi-second timeout while the host's rAF fires within milliseconds, this ordering issue is unlikely in practice but still worth flagging. I also want to check whether a human drag interleaving with an in-flight automation resize could cause similar drift, since only automation mutations are serialized. And I should verify that writing the effective viewport into a ref during render (before the effect scheduling) doesn't cause staleness under React 18's double-invoke behavior, since the effect's rAF callback reads from that ref later. Actually, I've found something concrete: when fitSourceContent is enabled and viewport mode is fill, the webview element gets sized to the fitted viewport rather than the effective one, but the override applier is still pushing the effective (fill) viewport instead of the fitted size — that's a real mismatch worth digging into further. Now I'm reasoning through zoomFactor: the element's CSS width derives from viewport width times scale, and with zoom applied to the guest's webContents, the guest's effective CSS viewport should equal the raw viewport width when scale is 1 — but I need to check whether the CDP device metrics override expects device-independent pixels here, since that could throw off the calculation. This is too speculative to confirm without evidence, so I shouldn't flag the zoom interaction as a definite issue — I need proof of ownership or semantic equivalence before raising it as a real cleanup concern. Actually, I realize zoom could be legitimate: the preview supports a zoomFactor prop, and if users zoom in, the guest layout might not match the webview element size, causing clipping. I should check how previewViewportReadiness compares declaredViewport against the measured window.innerWidth to see if zoom is already accounted for.

10. browse_code — apps/web/src/components/preview/previewViewportReadiness.ts

11. git_tools — innerWidth|devicePixelRatio

💭 I'm considering that the guest zoom comment might explain a real regression: without the override, the element scales so window.innerWidth matches the requested width, but with Chrome dividing by zoom under the override, the measured viewport could end up as width/zoom, causing the resize check to time out and roll back at non-1 zoom levels. Still, I can't be fully confident about Chrome's exact zoom semantics here.

12. git_tools — zoomFactor

💭 Zoom is a legitimate user-facing feature, so combining preview zoom with a fixed viewport is expected, and the layout code multiplies element sizes by the zoom factor to keep the guest's CSS viewport matching the preset size. I'm now considering how setting device metrics override with a scale factor of 1 interacts with page zoom, since the emulated viewport is defined in DIP while zoom applies separately. Both mechanisms scale CSS pixels but through different layers: DevTools' device mode uses a visual "fit to window" scale that's distinct from actual page zoom, whereas Electron's setZoomFactor directly changes the CSS-to-device-pixel mapping, so window.innerWidth ends up being the overridden width divided by the zoom factor rather than staying fixed. So with a 1.5x zoom and a 390px preset, innerWidth would drop to 260, breaking the readiness check expecting ~390, causing timeouts and rollback in automated resizing, and causing a mismatch between guest layout size and host element size in manual resizing. Before this change the widget size equaled the element size and zoom correctly divided out to give the right innerWidth — this PR's override appears to break that relationship for any non-1x zoom factor. The fix likely needs to scale the emulated width/height by the zoom factor before passing it to the override, so the layout still comes out correct after Chromium divides by zoom again. But before committing to that, I want to check the repo for any existing comments or tests explaining how zoom and emulation interact. I recall a note in the viewport readiness code about Electron rounding CSS pixels through the guest's fractional zoom/device scale, which supports the idea that the measured width is derived by dividing the element's device pixels by zoom — but I'm not certain how that interacts with the override once emulation is active. I'm now digging into Chromium's DevTools protocol behavior around Emulation.setDeviceMetricsOverride, where width and deviceScaleFactor are specified in device-independent pixels, and recalling that DevToolsEmulator::EnableDeviceEmulation may actually reset the page zoom level to 1 while device emulation is active, storing the prior embedder zoom level for later restoration. Enabling device emulation likely resets the guest's zoom to 100%, meaning the preview's zoom setting could silently stop applying whenever a fixed viewport is set, and restoring afterward creates another inconsistency. Either way, there's a mismatch between the new override and the existing per-tab zoomFactor that the host layout math depends on — if zoom resets to 100%, guest content would render at roughly 2/3 scale and appear letterboxed, a visible regression whenever zoomFactor isn't 1, especially since desktop mode explicitly sets deviceScaleFactor to 1. That deviceScaleFactor override also forces devicePixelRatio to 1, which on a HiDPI display would previously have been 2. This changes how images and canvas render, potentially disabling @2x asset loading and making the preview appear blurrier or lower fidelity on Retina screens. I'm checking the file structure though — the actual override logic lives in Manager.ts, which is out of scope for my include patterns, so I need to find where I can comment. I'm looking at previewGuestViewport.ts instead, since that mapper decides what viewport settings to send to the guest, and it may not be accounting for the tab's zoom factor the way other consumers do. But I'm second-guessing whether multiplying width by zoomFactor is even the right fix, since emulation resetting zoom could make that wrong too. Given the uncertainty, I'm leaning toward posting a single comment that asks for verification of the zoom interaction rather than asserting a fix. Let me check if the codebase already used device metrics override elsewhere before this PR, particularly for screenshots, to see how zoom was handled there.

13. git_tools — setDeviceMetricsOverride|captureBeyondViewport|setZoomFactor|zoomFactor",|Emulation.

💭 I'm recalling that page zoom is applied separately from device emulation, and the guest's CSS viewport gets scaled by that zoom factor on top of the preset device size. This means the override needs to account for how Chromium's DevToolsEmulator handles device emulation alongside zoom, since the two interact when both are active. For non-mobile emulation, the emulated size sets the widget dimensions in DIP, and zoom then applies normally to convert to CSS pixels — so with zoom 1.25 and a width override of 1024, innerWidth ends up around 819, creating a visible mismatch between the toolbar setting and actual layout that would trip the automation readiness check and cause resize rollback at non-default zoom. For mobile sizes, the zoom override of 1 makes innerWidth match exactly, but at the cost of ignoring the user's actual zoom preference. Since the default zoom factor is user-configurable, anyone with a non-100% default would hit this issue on every preset — I'm fairly confident that omitting zoomFactor from the override calculation is a genuine defect, since every other part of the layout pipeline converts between CSS px and host px using that factor, while this override sends raw CSS px with a fixed deviceScaleFactor of 1. I'm now identifying where in the codebase to leave this comment, starting with the preview guest viewport component. Also worth noting: setting deviceScaleFactor to 1 forces DPR 1 in the guest even though the display previously gave DPR 2, which is a visible behavior change for responsive images and canvas rendering. CDP treats deviceScaleFactor 0 as "use system default," so passing 0 would preserve prior behavior — but that's owned by the desktop-side file, not the mapper, so I'd rather keep my comment focused on what the web-side mapper omits regarding zoom, and reconsider whether it belongs on HostedBrowserWebview instead where zoomFactor is actually known. Since the mapper can't access zoom, the real fix means threading zoomFactor into the override function or having desktop apply it — the minimal change being to pass zoomFactor alongside the setting. I'll leave a comment on the relevant lines and note the call sites, while also checking whether the automation host's setting is similarly affected by zoom (it is), and reconsidering whether multiplying by zoom is even correct given that Chromium's emulated view size is in device-independent pixels rather than raw CSS pixels. Working through the actual math: page zoom of 1.25 means one CSS pixel equals 1.25 DIP, so a widget sized at 1024 DIP with that zoom factor renders as 819 CSS pixels of layout width — confirming that emulation setting the widget size to view_size in DIP does produce the expected layout width. But then the mobile emulation flag complicates the fix, since it forces zoom factor to exactly 1, making layout width equal the requested width directly — so any correction depends on whether mobile mode is active. I think the right way to frame this is that the override skips accounting for the tab's actual zoom factor, unlike every other consumer, so at non-default zoom the guest's layout viewport diverges from the requested size and the readiness check comparing within a pixel would fail — this seems like a solid, concrete finding even without a clean one-line fix. With a 1024-wide freeform frame at zoom 1.25, the element is 1280 host px but the guest only renders 819 CSS px scaled to 1024 device px, leaving a visible blank strip on the right and bottom—a clear visual bug tied to a supported feature. I'll check whether zoomFactor defaults to 1 for most users, but confirm the setting still applies and the issue is valid, so I'll post this as one comment. Now I'm considering whether navigation within the guest could reset device metrics overrides—it shouldn't, since they persist on the session, so that path is fine. For anchoring the comment, previewGuestViewport.ts is the cleaner spot since it covers both call sites, so I'll place it there around lines 13-19 rather than the HostedBrowserWebview call site. The core issue: previewGuestViewportOverride sends the raw CSS-pixel size directly to the device metrics override API, but the guest actually renders at the tab's zoom factor, and every other consumer of this setting scales by that factor. Since the override width is a DIP size rather than zoom-adjusted, a 1024×768 preset at 1.25 zoom would lay out at roughly 819 CSS px, leaving a blank strip where the guest doesn't fill the element. I want to note that Chromium also overrides page zoom to 1 under mobile emulation, so mobile-flagged sizes will behave inconsistently compared to desktop sizes using the same control -- this needs the tab's zoom factor threaded into the mapper, since both call sites already have access to it, and should be verified with a non-100% preview zoom before merging. I'm confirming this should fail the check since it defaults to failure when there's a finding, and checking that no prior comment already flagged the zoom issue before posting the review.

14. modify_pr — One finding on the new guest viewport override: the CDP size is sent without accounting for the tab's page zoom, which the rest of the preview layout consistently scales by.