feat(review): add scroll-through GIF evidence capture#3688
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-06 05:50:31 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 7 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3688 +/- ##
==========================================
+ Coverage 93.36% 93.37% +0.01%
==========================================
Files 315 316 +1
Lines 32204 32296 +92
Branches 11812 11841 +29
==========================================
+ Hits 30066 30158 +92
Misses 1507 1507
Partials 631 631
🚀 New features to boost your workflow:
|
0bc75eb to
284f83b
Compare
c573e98 to
ab36e2c
Compare
ab36e2c to
2a71b44
Compare
Summary
src/review/visual/shot.ts— newcaptureScrollFrames, mirroringcaptureShot's SSRF guard / sub-request interception / auth-wall detection exactly (duplicated rather than shared, since this is security-sensitive code). Also fixeshandleShot's?key=serve path, which previously hardcodedcontent-type: image/pngregardless of what was actually stored — now derived from the key's extension so a.gifobject animates correctly.src/review/visual/scroll-gif.tsis the Worker-safe no-op default;src/selfhost/stubs/scroll-gif.tsis the real self-host implementation (pngjsdecode →gifencquantize/encode, both zero-Node-dependency-but-swapped-for-consistency — see module header).scripts/build-selfhost.mjsswaps the specifier only when bundlingsrc/server.ts;test/unit/worker-entry-boundary.test.ts's forbidden-identifier list now also coversgifenc.src/review/visual/capture.ts— newcaptureScrollGif, cached on the same fingerprint scheme as the existing screenshot/diff uploads (a scroll capture is the most expensive thing this pipeline does — a re-review of the same head must never redo it). Gated on bothreview.visual.gifandisScrollGifAvailable()— hosted mode never even attempts a scroll frame, so there's zero added cost there regardless of a repo's config.src/signals/focus-manifest.ts—review.visual.gif: boolean(defaultfalse), composes independently withreview.visual.themes(feat(review): add review.visual.themes config for dark-mode capture #3678).src/review/unified-comment-bridge.ts— new "Scroll preview" collapsible, rendered alongside "Visual preview" (never replacing it), same clickable-thumbnail markup/escaping..gittensory.yml.example+config/examples/gittensory.full.ymlboth document the newgif:key (kept byte-identical perconfig-templates.test.ts).Part of #3607. Closes #3612.
Scope
src/review/visual/shot.ts,src/review/visual/capture.ts,src/review/visual/scroll-gif.ts(new),src/selfhost/stubs/scroll-gif.ts(new),src/selfhost/stubs/gifenc.d.ts(new —gifencships no types).src/signals/focus-manifest.ts,src/review/unified-comment-bridge.ts.scripts/build-selfhost.mjs,test/unit/worker-entry-boundary.test.ts..gittensory.yml.example,config/examples/gittensory.full.yml.gifenc(devDependency, matchingpngjs/pixelmatch's existing classification — bundled by esbuild regardless of dep/devDep).scroll-gif.test.ts(new),selfhost-scroll-gif-stub.test.ts(new),visual-shot.test.ts,visual-capture.test.ts,visual-collapsible.test.ts,focus-manifest.test.ts,signals-coverage.test.ts,visual-config-wiring.test.ts.Validation
npx tsc --noEmitclean.npx vitest run).git diff --checkclean.npm audit --audit-level=moderate— 0 vulnerabilities.browserless/chromium+puppeteer-core, a real 6-band 5400px page): 6 distinct frames captured in 2.45s, decode+quantize+encode in 173ms — ~4.1s total per side. Independently re-verified the produced GIF with Pillow (not my own encoder): 6 real frames, 500ms each, infinite loop, 1440×900, ~40KB. A single-viewport-height page correctly yields exactly 1 frame (no wasted animation of a static page).pngjs, NodeBuffer), so — like feat(review): wire up existing pixel-diff into a 'Diff' column (self-host) #3674's pixel-diff — it's self-host-only by construction. The Cloudflare Worker (hosted) path never runs this code at all (isScrollGifAvailable()gates it before a single scroll frame is captured), so there's no Workers isolate CPU-ms exposure to measure. The self-host process is a normal long-running Node server, not subject to that limit; the real, relevant number is the ~4s-per-side wall-clock measured above.capturePage's older code — not part of this patch).packages: external) and--all(bundle-everything, the Docker-image mode) both build cleanly withgifencfully inlined, no leftover unresolved import.ui:openapi/cf-typegen/migrations not applicable.Safety
review.visual.gifconfigured) is byte-identical to pre-(stretch) Short-clip/GIF evidence capture for non-static-visible changes #3612 behavior: no scroll frames captured, no "Scroll preview" section, no new R2/blob-store keys. Verified by dedicated tests in every touched file..gittensory.yml— enforced byisScrollGifAvailable()short-circuiting inbuildCapture, and separately enforced structurally by the Worker-entry-boundary test.wantedPaths; does not touchsite/,CNAME, or**/lovable/**.