fix(visual): stop silently dropping mobile screenshots of long pages (#9790)

MAX_SCREENSHOT_HEIGHT was 10000, derived for the 1440-wide DESKTOP viewport --
MAX_SCREENSHOT_PIXELS is literally 1440 x 10000. Reusing that height bound for the
390-wide MOBILE viewport rejected captures costing a third as much: an ordinary
long docs page renders ~10850px tall at 390 wide, which is 4.2M pixels against a
14.4M budget. The shot was discarded and the function returned null.

On the ORB that was 64 mobile screenshots dropped in a single hour, weakening the
visual gate on exactly the viewport most likely to show a responsive regression.

The renderer was never the constraint. Verified against this deployment's own
browserless (v2 / Chrome 149): a 390 x 20000 full-page capture returns 200 in
~157KB. Raise the height bound to 20000 and let MAX_SCREENSHOT_PIXELS remain the
real cost ceiling, which judges a narrow-tall page by what it actually costs.

Desktop is unchanged: 1440 x 10847 is 15.6M pixels and still fails the pixel cap.

Author: JSONbored

src/review/visual/shot.ts

@@ -68,8 +68,23 @@ type ScreenshotPage = {
 export const DESKTOP_VIEWPORT: Viewport = { width: 1440, height: 900 };
 export const MOBILE_VIEWPORT: Viewport = { width: 390, height: 844 }; // iPhone-class portrait
 const VIEWPORT = DESKTOP_VIEWPORT;
-export const MAX_SCREENSHOT_HEIGHT = 10000;
-export const MAX_SCREENSHOT_PIXELS = 14_400_000; // 1440 × 10000, matching the full-page cap.
+// A sanity bound against a pathological infinite-scroll page, NOT a cost proxy -- cost is bounded by
+// MAX_SCREENSHOT_PIXELS below, and separately by MAX_SCREENSHOT_BYTES.
+//
+// It used to be 10000, which was derived for the 1440-wide DESKTOP viewport (see the pixel cap's own
+// comment: 1440 × 10000). Applied unchanged to the 390-wide MOBILE viewport it rejected captures costing a
+// third as much: an ordinary long docs page renders ~10850px tall at 390 wide, which is 4.2M pixels against
+// a 14.4M budget -- comfortably affordable, silently dropped. On the ORB that was 64 mobile screenshots
+// discarded in a single hour, weakening the visual gate precisely on the viewport most likely to reveal a
+// responsive regression.
+//
+// 20000 is empirically verified against this deployment's own renderer (browserless v2 / Chrome 149):
+// a 390 × 20000 full-page capture returns 200 in ~157KB. The renderer was never the binding constraint.
+// Desktop is unaffected -- 1440 × 20000 is 28.8M pixels and still fails the pixel cap.
+export const MAX_SCREENSHOT_HEIGHT = 20000;
+// The real cost ceiling: width × height, so a narrow-tall page is judged by what it actually costs to
+// render rather than by height alone.
+export const MAX_SCREENSHOT_PIXELS = 14_400_000; // 1440 × 10000 — one full desktop-width page.
 export const MAX_SCREENSHOT_BYTES = 5 * 1024 * 1024;
 const SCREENSHOT_TIMEOUT_MS = 10000;
 const SCREENSHOT_HEIGHT_PROBE_TIMEOUT_MS = 2_000;

test/unit/visual-shot.test.ts

@@ -1,5 +1,5 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
-import { captureInteractionFrames, captureScrollFrames, captureShot, handleShot } from "../../src/review/visual/shot";
+import { captureInteractionFrames, captureScrollFrames, captureShot, handleShot, MAX_SCREENSHOT_HEIGHT, MAX_SCREENSHOT_PIXELS, MOBILE_VIEWPORT, DESKTOP_VIEWPORT } from "../../src/review/visual/shot";
 import { counterValue, resetMetrics } from "../../src/selfhost/metrics";
 
 const mocks = vi.hoisted(() => ({
@@ -1113,3 +1113,32 @@ describe("visual capture result metric (#9487)", () => {
   });
 
 });
+
+describe("a narrow-tall mobile page is judged by cost, not height alone", () => {
+  // Observed on the ORB: 64 mobile captures discarded in one hour, every one at width 390. An ordinary long
+  // docs page is ~10850px tall at that width -- 4.2M pixels against a 14.4M budget. The old 10000 height cap
+  // was derived for the 1440-wide DESKTOP viewport (MAX_SCREENSHOT_PIXELS is literally 1440 × 10000), so
+  // reusing it for mobile rejected captures costing a third as much, weakening the visual gate on exactly
+  // the viewport most likely to reveal a responsive regression.
+  it("REGRESSION: the real observed mobile page (390 × 10847) is now within both caps", () => {
+    const width = MOBILE_VIEWPORT.width;
+    const height = 10847;
+    expect(width).toBe(390);
+    expect(height).toBeLessThanOrEqual(MAX_SCREENSHOT_HEIGHT);
+    expect(width * height).toBeLessThanOrEqual(MAX_SCREENSHOT_PIXELS);
+  });
+
+  it("INVARIANT: desktop is unaffected — the pixel cap still rejects a wide page of the same height", () => {
+    // The cost ceiling must not have been widened by raising the height bound.
+    const area = DESKTOP_VIEWPORT.width * 10847;
+    expect(area).toBeGreaterThan(MAX_SCREENSHOT_PIXELS);
+  });
+
+  it("INVARIANT: the height cap is a sanity bound the renderer can actually satisfy", () => {
+    // 20000 is verified against this deployment's renderer (browserless v2 / Chrome 149): a 390 × 20000
+    // full-page capture returns 200 in ~157KB. If this is ever raised past what the renderer can produce,
+    // captures fail at request time instead of being cheaply rejected here.
+    expect(MAX_SCREENSHOT_HEIGHT).toBe(20000);
+    expect(MOBILE_VIEWPORT.width * MAX_SCREENSHOT_HEIGHT).toBeLessThanOrEqual(MAX_SCREENSHOT_PIXELS);
+  });
+});