You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Part of #4325. Follow-up to #4369 (num_ctx capping) — same underlying concern, deliberately split out since this needs a real dependency/build change and #4369 didn't.
Context
Visual-vision screenshots are captured with fullPage: true (src/review/visual/shot.ts:225), so even though the desktop viewport is only 1440px wide (DESKTOP_VIEWPORT, shot.ts:67), the actual image HEIGHT scales with the page's full scrollable content — a long page can produce a very tall image. Qwen's dynamic-resolution vision encoder tokenizes proportional to pixel count, so an oversized image directly inflates prefill cost/latency per vision call, on top of the KV-cache concern #4369 already addressed via num_ctx.
Decision (made explicitly, not by default): resize ONLY the bytes fed to the vision model, not the stored/displayed screenshots. The same before/after URLs are also embedded in the PR comment for human reviewers (fetchShotContentBlock fetches the identical URL used for display) — those must stay full quality. A capture-layer fix (bounding the Puppeteer capture itself) was considered and rejected: it would touch the shared capture pipeline used by every consumer, not just vision, and would mean vision only ever sees a cropped view rather than the full page (risking missed below-the-fold regressions).
Requirements
Add image resizing (downscale to ~1280px longest edge, preserving aspect ratio) specifically in the vision call path (fetchShotContentBlock in src/review/visual/capture.ts, or a vision-specific wrapper around it) — do not touch the stored/displayed image bytes used elsewhere.
Requires a real image-processing library (sharp is the standard choice: fast, well-maintained, but ships native per-platform binaries).
This is a real build-pipeline change, not just an app-code edit: scripts/build-selfhost.mjs --all bundles literally everything into a single dist/server.mjs with zero node_modules at runtime (see its own top-of-file comment). A native-binding dependency like sharp cannot be bundled by esbuild — it needs to be marked external in the bundle config AND the Dockerfile's runtime stage (currently only copies dist/server.mjs + migrations + config/examples, no node_modules at all) needs to actually install/copy sharp + its correct platform binary for the container's architecture.
Verify the Docker image still builds and boots after this change (the "build + boot smoke test" CI workflow — fix(selfhost): stop .dockerignore from breaking the engine's own build #4354 was a recent real breakage in this exact area, so treat this build path with real care) before considering this done.
Deliverables
sharp added as a dependency, correctly excluded from the esbuild bundle and installed into the runtime Docker image
Downscale step added to the vision-only image path, aspect ratio preserved, ~1280px longest edge
Before/after latency numbers for a real tall-page screenshot pair, GPU vision call
Unit tests for the resize step (aspect ratio preserved, already-small images left alone or handled gracefully, resize failure degrades to sending the original image rather than dropping it)
Expected outcome
Vision calls on long pages no longer pay prefill cost proportional to full-page height, while PR-comment-displayed screenshots are completely unaffected.
Part of #4325. Follow-up to #4369 (num_ctx capping) — same underlying concern, deliberately split out since this needs a real dependency/build change and #4369 didn't.
Context
Visual-vision screenshots are captured with
fullPage: true(src/review/visual/shot.ts:225), so even though the desktop viewport is only 1440px wide (DESKTOP_VIEWPORT,shot.ts:67), the actual image HEIGHT scales with the page's full scrollable content — a long page can produce a very tall image. Qwen's dynamic-resolution vision encoder tokenizes proportional to pixel count, so an oversized image directly inflates prefill cost/latency per vision call, on top of the KV-cache concern #4369 already addressed vianum_ctx.Decision (made explicitly, not by default): resize ONLY the bytes fed to the vision model, not the stored/displayed screenshots. The same before/after URLs are also embedded in the PR comment for human reviewers (
fetchShotContentBlockfetches the identical URL used for display) — those must stay full quality. A capture-layer fix (bounding the Puppeteer capture itself) was considered and rejected: it would touch the shared capture pipeline used by every consumer, not just vision, and would mean vision only ever sees a cropped view rather than the full page (risking missed below-the-fold regressions).Requirements
fetchShotContentBlockinsrc/review/visual/capture.ts, or a vision-specific wrapper around it) — do not touch the stored/displayed image bytes used elsewhere.sharpis the standard choice: fast, well-maintained, but ships native per-platform binaries).scripts/build-selfhost.mjs --allbundles literally everything into a singledist/server.mjswith zeronode_modulesat runtime (see its own top-of-file comment). A native-binding dependency likesharpcannot be bundled by esbuild — it needs to be markedexternalin the bundle config AND the Dockerfile's runtime stage (currently only copiesdist/server.mjs+migrations+config/examples, nonode_modulesat all) needs to actually install/copysharp+ its correct platform binary for the container's architecture.Deliverables
sharpadded as a dependency, correctly excluded from the esbuild bundle and installed into the runtime Docker imagedocker buildverification (mirroring how fix(selfhost): stop .dockerignore from breaking the engine's own build #4354 was diagnosed/verified) that the runtime image still builds and boots correctlyExpected outcome
Vision calls on long pages no longer pay prefill cost proportional to full-page height, while PR-comment-displayed screenshots are completely unaffected.
Effort
M