perf(review): downscale before/after screenshots before the local VLM

Dockerfile

@@ -68,6 +68,14 @@ USER root
 ARG INSTALL_VISUAL_REVIEW=false
 COPY package*.json ./
 RUN if [ "$INSTALL_VISUAL_REVIEW" = "true" ]; then npm install puppeteer-core@22.13.1 --ignore-scripts; fi
+# sharp (#4370): esbuild marks it `external` in the --all bundle (a native per-platform binary can't be
+# bundled into dist/server.mjs, see scripts/build-selfhost.mjs), so it must be installed separately here,
+# same reason as puppeteer-core above -- but unconditional (not behind an opt-in build-arg): it's a core
+# dependency of the vision-image-downscale path, not an optional external-sidecar feature. --ignore-scripts
+# is safe here: sharp's own platform binary ships as an npm `optionalDependencies` entry
+# (@img/sharp-<platform>-<arch>) that npm's normal os/cpu-matched install resolves on its own, not via
+# sharp's postinstall script.
+RUN npm install sharp@0.34.5 --ignore-scripts
 # Data dir (the SQLite file) — owned by the unprivileged node user; mount a volume here to persist.
 RUN mkdir -p /data && chown -R node:node /data /app
 # Expose the optional user-installed CLIs only after all root build steps have completed, so a

package.json

@@ -101,6 +101,7 @@
     "hono": "^4.12.27",
     "ioredis": "^5.11.1",
     "pg": "^8.22.0",
+    "sharp": "^0.34.5",
     "yaml": "^2.9.0",
     "zod": "^4.4.3"
   },

scripts/build-selfhost.mjs

@@ -23,7 +23,10 @@ await esbuild.build({
   sourcesContent: true,
   sourceRoot: "/app/dist",
   // External: nothing (bundle all) vs every package (external). node: builtins are always external on node.
-  ...(bundleAll ? {} : { packages: "external" }),
+  // sharp (#4370) is ALWAYS external even in --all mode: it ships a native per-platform binary esbuild
+  // cannot bundle, unlike sharp's own JS glue code -- the Dockerfile installs it separately into the
+  // runtime image (see the runtime-base stage).
+  ...(bundleAll ? { external: ["sharp"] } : { packages: "external" }),
   // Bundling CJS deps into an ESM output needs require/__dirname/__filename shimmed (some deps call them).
   ...(bundleAll
     ? {
@@ -54,6 +57,8 @@ await esbuild.build({
         build.onResolve({ filter: /^\.\/pixel-diff$/ }, () => ({ path: resolve(root, "src/selfhost/stubs/pixel-diff.ts") }));
         // Same pattern for scroll-through GIF assembly (#3612) — pngjs decode + gifenc encode need Node Buffer.
         build.onResolve({ filter: /^\.\/scroll-gif$/ }, () => ({ path: resolve(root, "src/selfhost/stubs/scroll-gif.ts") }));
+        // Same pattern for vision-image downscaling (#4370) — sharp is a native binding, Worker-unsafe.
+        build.onResolve({ filter: /^\.\/image-downscale$/ }, () => ({ path: resolve(root, "src/selfhost/stubs/image-downscale.ts") }));
       },
     },
   ],

src/review/visual/capture.ts

@@ -16,6 +16,7 @@
 // default TanStack route convention; those hooks can return if a per-repo visual config is added.
 import { base64Encode, sha256Hex } from "../../utils/crypto";
 import type { AiContentBlock } from "../../types";
+import { downscaleForVision } from "./image-downscale";
 import type { GitHubRateLimitAdmissionKey } from "../../github/client";
 import { dispatchVisualCaptureFallback, fallbackShotR2Key, isFallbackDispatchInFlight, markFallbackDispatched } from "./actions-fallback";
 import {
@@ -116,7 +117,7 @@ export async function fetchShotContentBlock(url: string): Promise<AiContentBlock
     const response = await fetch(url);
     if (!response.ok) return undefined;
     const bytes = new Uint8Array(await response.arrayBuffer());
-    return { type: "image", data: base64Encode(bytes), mimeType: "image/png" };
+    return { type: "image", data: base64Encode(await downscaleForVision(bytes)), mimeType: "image/png" };
   } catch {
     return undefined;
   }

src/review/visual/image-downscale.ts

@@ -0,0 +1,11 @@
+// Vision-image downscale provider seam (#4370). WORKER-SAFE DEFAULT: a no-op.
+//
+// The real downscale uses a native image-resizing binding that can't run on the Cloudflare Workers runtime
+// — that's why `capture.ts` (which IS Worker-reachable) imports ONLY this file, never the real dependency
+// directly. Mirrors the pixel-diff.ts seam exactly: `scripts/build-selfhost.mjs`'s esbuild plugin swaps
+// this specifier for a real implementation (`src/selfhost/stubs/image-downscale.ts`) when bundling the
+// self-host entry (`src/server.ts`). The Worker's own (wrangler) bundle never applies that swap, so hosted
+// mode always returns the input unchanged — zero behavior change, zero added cost.
+export async function downscaleForVision(png: Uint8Array): Promise<Uint8Array> {
+  return png;
+}

src/selfhost/stubs/image-downscale.ts

@@ -0,0 +1,32 @@
+// Self-host replacement for src/review/visual/image-downscale.ts (#4370). Swapped in by
+// scripts/build-selfhost.mjs's esbuild plugin, the same mechanism used for @cloudflare/puppeteer and
+// ./pixel-diff — this file is only ever bundled into dist/server.mjs, never the Worker entry, so it's safe
+// to depend on sharp (a native binding) here. Unlike puppeteer-core, sharp is marked `external` in the
+// --all esbuild bundle (a native binding can't be bundled) and installed separately into the runtime Docker
+// image (see the Dockerfile) rather than lazily imported at call time — sharp has no browser-sidecar-style
+// opt-in the way puppeteer-core does, so a plain static import is fine.
+import sharp from "sharp";
+
+/** Longest-edge cap for the bytes sent to the local VLM — NOT the stored/displayed screenshot (the same URL
+ *  is embedded verbatim in the PR comment; see review/visual/capture.ts's unified-comment-bridge caller).
+ *  shot.ts captures `fullPage: true`, so image HEIGHT scales with the page's full scrollable content even
+ *  though the viewport is a fixed 1440px wide — Qwen's dynamic-resolution vision encoder tokenizes
+ *  proportional to pixel count, so an oversized tall page inflates vision prefill cost/latency for no
+ *  quality gain a human reviewer would notice at chat-image resolution. */
+const VISION_MAX_DIMENSION_PX = 1280;
+
+/** Downscale `png` so its longest edge is at most {@link VISION_MAX_DIMENSION_PX}, preserving aspect ratio
+ *  and never enlarging an already-small image. Any decode/resize failure (a corrupt/unexpected payload)
+ *  degrades to the ORIGINAL bytes rather than dropping the image — a vision call on a full-size image is
+ *  strictly better than no image at all. */
+export async function downscaleForVision(png: Uint8Array): Promise<Uint8Array> {
+  try {
+    const resized = await sharp(png)
+      .resize({ width: VISION_MAX_DIMENSION_PX, height: VISION_MAX_DIMENSION_PX, fit: "inside", withoutEnlargement: true })
+      .png()
+      .toBuffer();
+    return new Uint8Array(resized);
+  } catch {
+    return png;
+  }
+}

test/unit/image-downscale.test.ts

@@ -0,0 +1,14 @@
+import { describe, expect, it } from "vitest";
+import { downscaleForVision } from "../../src/review/visual/image-downscale";
+
+describe("image-downscale Worker-safe default (#4370)", () => {
+  it("returns the input bytes unchanged, since the real implementation is self-host only", async () => {
+    const png = new Uint8Array([137, 80, 78, 71, 1, 2, 3]);
+    await expect(downscaleForVision(png)).resolves.toBe(png);
+  });
+
+  it("returns an empty input unchanged", async () => {
+    const empty = new Uint8Array([]);
+    await expect(downscaleForVision(empty)).resolves.toBe(empty);
+  });
+});

test/unit/selfhost-image-downscale-stub.test.ts

@@ -0,0 +1,51 @@
+// Tests for the self-host vision-image-downscale stub (#4370). This module is never bundled into the
+// Worker entry (scripts/build-selfhost.mjs swaps it in only when building src/server.ts — see
+// test/unit/worker-entry-boundary.test.ts for the enforced side of that), so it's safe to depend on sharp
+// / real PNG fixtures here, mirroring test/unit/selfhost-pixel-diff-stub.test.ts's own fixture style.
+import sharp from "sharp";
+import { describe, expect, it } from "vitest";
+import { downscaleForVision } from "../../src/selfhost/stubs/image-downscale";
+
+async function solidPng(width: number, height: number): Promise<Uint8Array> {
+  const buf = await sharp({ create: { width, height, channels: 3, background: { r: 10, g: 20, b: 30 } } })
+    .png()
+    .toBuffer();
+  return new Uint8Array(buf);
+}
+
+async function dimensionsOf(png: Uint8Array): Promise<{ width: number; height: number }> {
+  const meta = await sharp(Buffer.from(png)).metadata();
+  return { width: meta.width ?? 0, height: meta.height ?? 0 };
+}
+
+describe("selfhost image-downscale stub (#4370)", () => {
+  it("downscales a wide image so the longest edge (width) is capped, preserving aspect ratio", async () => {
+    const wide = await solidPng(2000, 500);
+    const result = await downscaleForVision(wide);
+    const { width, height } = await dimensionsOf(result);
+    expect(width).toBe(1280);
+    expect(height).toBe(320); // 500/2000 * 1280
+  });
+
+  it("downscales a tall image (the real fullPage-capture shape) so the longest edge (height) is capped", async () => {
+    const tall = await solidPng(500, 2000);
+    const result = await downscaleForVision(tall);
+    const { width, height } = await dimensionsOf(result);
+    expect(height).toBe(1280);
+    expect(width).toBe(320); // 500/2000 * 1280
+  });
+
+  it("leaves an already-small image's dimensions unchanged (withoutEnlargement)", async () => {
+    const small = await solidPng(800, 600);
+    const result = await downscaleForVision(small);
+    const { width, height } = await dimensionsOf(result);
+    expect(width).toBe(800);
+    expect(height).toBe(600);
+  });
+
+  it("degrades to the ORIGINAL bytes (never drops the image) when the input isn't a valid image", async () => {
+    const garbage = new Uint8Array([1, 2, 3, 4, 5]);
+    const result = await downscaleForVision(garbage);
+    expect(result).toBe(garbage);
+  });
+});