Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,12 @@ settings:
# themes:
# - light
# - dark
# # Also capture a short scroll-through GIF per route (desktop only) — evidence for scroll-linked behavior
# # (parallax, reveal-on-scroll, a sticky header) that a static screenshot can't show (#3612). Rendered as
# # a separate "Scroll preview" section alongside the static before/after table, never replacing it.
# # SELF-HOST ONLY (no effect on the hosted service) and the heaviest capture mode here — up to 6 extra
# # renders per side, ~4s wall-clock per side measured in practice. Bool. Default: false (no scroll capture).
# gif: false
# # Maintainer overrides for the public review-panel CONTENT (not what gittensory measures). The
# # Gittensor attribution + register link is always appended to the footer regardless; maintainer text
# # failing the public-safe filter is dropped, never published.
Expand Down
6 changes: 6 additions & 0 deletions config/examples/gittensory.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,12 @@ settings:
# themes:
# - light
# - dark
# # Also capture a short scroll-through GIF per route (desktop only) — evidence for scroll-linked behavior
# # (parallax, reveal-on-scroll, a sticky header) that a static screenshot can't show (#3612). Rendered as
# # a separate "Scroll preview" section alongside the static before/after table, never replacing it.
# # SELF-HOST ONLY (no effect on the hosted service) and the heaviest capture mode here — up to 6 extra
# # renders per side, ~4s wall-clock per side measured in practice. Bool. Default: false (no scroll capture).
# gif: false
# # Maintainer overrides for the public review-panel CONTENT (not what gittensory measures). The
# # Gittensor attribution + register link is always appended to the footer regardless; maintainer text
# # failing the public-safe filter is dropped, never published.
Expand Down
9 changes: 9 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
"@types/pngjs": "^6.0.5",
"@vitest/coverage-v8": "^4.1.9",
"drizzle-kit": "^0.31.10",
"gifenc": "^1.0.3",
"git-cliff": "^2.13.1",
"github-actionlint": "^1.7.12",
"node-addon-api": "^8.8.0",
Expand Down
2 changes: 2 additions & 0 deletions scripts/build-selfhost.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ await esbuild.build({
// importer, always as "./pixel-diff" (same-directory sibling) — the stub's own `import type` back to
// the original is erased before bundling and never reaches this resolver.
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") }));
},
},
],
Expand Down
46 changes: 44 additions & 2 deletions src/review/unified-comment-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,45 @@ export function buildBeforeAfterCollapsible(routes: CaptureRoute[]): UnifiedColl
return { title: "Visual preview", body, rawHtml: true };
}

/**
* Build the "Scroll preview" collapsible from the same before/after capture routes (#3612) — rendered
* ALONGSIDE "Visual preview", never replacing it, since a scroll-through GIF is evidence for scroll-linked
* behavior (parallax, reveal-on-scroll, a sticky header) that a single static screenshot can't show, not a
* substitute for the static before/after comparison. Self-host only (`review.visual.gif`, off by default —
* see capture.ts's `gifWanted`) and desktop-viewport only in this first cut, so there is no Viewport column
* here (unlike "Visual preview"'s desktop/mobile rows). Same clickable-thumbnail markup and public-safety
* argument as `buildBeforeAfterCollapsible`. Returns null when no route has a GIF, so the section is omitted
* entirely for every repo that hasn't opted in — byte-identical to pre-#3612 for everyone else.
*/
export function buildScrollPreviewCollapsible(routes: CaptureRoute[]): UnifiedCollapsible | null {
const attr = (value: string): string =>
value.replace(/[&"<>]/g, (char) => ({ "&": "&amp;", '"': "&quot;", "<": "&lt;", ">": "&gt;" })[char] as string);
const markdownCode = (value: string): string =>
`\`${value
.replace(/\\/g, "\\\\")
.replace(/`/g, "\\`")
.replace(/\|/g, "\\|")
.replace(/[<>]/g, (char) => (char === "<" ? "&lt;" : "&gt;"))}\``;
const cell = (url: string | undefined, label: string): string =>
url ? `<a href="${attr(url)}" target="_blank" rel="noopener"><img width="360" alt="${attr(label)}" src="${attr(url)}"></a>` : "—";
const rows: string[] = [];
for (const route of routes) {
if (!route.beforeGifUrl && !route.afterGifUrl) continue;
const path = markdownCode(route.path);
const themeSuffix = route.theme ? ` (${route.theme})` : "";
rows.push(`| ${path}${themeSuffix} | ${cell(route.beforeGifUrl, `before ${route.path}${themeSuffix} (scroll)`)} | ${cell(route.afterGifUrl, `after ${route.path}${themeSuffix} (scroll)`)} |`);
}
if (rows.length === 0) return null;
const body = [
"| Route | Before (production) | After (this PR's preview) |",
"| --- | --- | --- |",
...rows,
"",
"_A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show._",
].join("\n");
return { title: "Scroll preview", body, rawHtml: true };
}

/** A changed file's path + line deltas — everything `buildChangedFilesSummaryCollapsible` needs to group and
* total. Deliberately narrower than `PullRequestFileRecord` (path/additions/deletions only) so the bridge
* doesn't drag GitHub's full file-record shape into its pure-rendering surface. */
Expand Down Expand Up @@ -552,8 +591,11 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string
// Visual-capture port: when before/after routes are present, append a "Visual preview" collapsible to the
// extra sections. Flag-OFF (the processor passes no beforeAfter) ⇒ extraCollapsibles is unchanged.
const visualCollapsible = args.beforeAfter && args.beforeAfter.length > 0 ? buildBeforeAfterCollapsible(args.beforeAfter) : null;
const extraCollapsibles =
visualCollapsible !== null ? [...(withFindingCategories ?? []), visualCollapsible] : withFindingCategories;
const withVisual = visualCollapsible !== null ? [...(withFindingCategories ?? []), visualCollapsible] : withFindingCategories;
// #3612: "Scroll preview" renders ALONGSIDE "Visual preview" (never replacing it) — self-host + gif:true
// only, so this is null (no section, no behavior change) for every repo that hasn't opted in.
const scrollCollapsible = args.beforeAfter && args.beforeAfter.length > 0 ? buildScrollPreviewCollapsible(args.beforeAfter) : null;
const extraCollapsibles = scrollCollapsible !== null ? [...(withVisual ?? []), scrollCollapsible] : withVisual;

const body = renderUnifiedReviewComment(input, {
brand: args.brand ?? "Gittensory review",
Expand Down
75 changes: 69 additions & 6 deletions src/review/visual/capture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ import {
getPreviewBuildState,
parseRepo,
} from "./preview-url";
import { captureShot, DESKTOP_VIEWPORT, MOBILE_VIEWPORT, type ShotTheme, type Viewport } from "./shot";
import { captureScrollFrames, captureShot, DESKTOP_VIEWPORT, MOBILE_VIEWPORT, type ShotTheme, type Viewport } from "./shot";
import { compareCapturedScreenshots, isVisualDiffAvailable, type VisualDiffOutcome } from "./pixel-diff";
import { encodeScrollGif, isScrollGifAvailable } from "./scroll-gif";

const NAMESPACE = "gittensory";
const DEFAULT_ROUTES = ["/"];
Expand All @@ -34,7 +35,9 @@ const MAX_CONFIGURED_ROUTES = 5;
* per viewport (#3674) — self-host only (isVisualDiffAvailable), and only when the diff clears the visual-
* diff module's own noise threshold; undefined slot ⇒ a dash cell either way. `theme` is set only when
* `review.visual.themes` (#3678) configured more than the implicit single default capture — undefined means
* "the one, un-emulated default render", exactly like today. */
* "the one, un-emulated default render", exactly like today. `beforeGifUrl`/`afterGifUrl` (#3612) are a
* short scroll-through animation — self-host only (isScrollGifAvailable) and only when `review.visual.gif`
* opts in; desktop viewport only in this first cut (see buildCapture's scoping note). */
export interface CaptureRoute {
path: string;
theme?: ShotTheme | undefined;
Expand All @@ -44,6 +47,8 @@ export interface CaptureRoute {
afterUrlMobile?: string | undefined;
diffUrl?: string | undefined;
diffUrlMobile?: string | undefined;
beforeGifUrl?: string | undefined;
afterGifUrl?: string | undefined;
}

/** The capture pipeline's result: the rendered routes, plus whether a preview build is still pending. */
Expand Down Expand Up @@ -214,10 +219,54 @@ async function uploadDiffImage(
return `${shotBase}/${NAMESPACE}/shot?key=${encodeURIComponent(key)}`;
}

/** Per-repo `review.visual` config, as resolved by the caller from the manifest (#3609 / #3610 / #3678).
* Absent ⇒ byte-identical to today (GitHub-native discovery, automatic route inference, single default-
* theme capture, built-in route cap). */
export type VisualCaptureConfig = { preview?: VisualPreviewInput | null | undefined; routes?: VisualRoutesInput | null | undefined; themes?: readonly ShotTheme[] | null | undefined };
// How long each frame shows when the assembled GIF plays back (#3612) — a quick "evidence clip" pace: the
// full MAX_SCROLL_STEPS (6, see shot.ts) loop takes ~3s, long enough to read, short enough to stay a glance.
const GIF_FRAME_DELAY_MS = 500;

/**
* Capture a scroll-through sequence for `page` and assemble it into a GIF (#3612), or undefined when there's
* no page, the render fails/auth-walls, storage is unavailable, or this build can't assemble GIFs at all
* (isScrollGifAvailable — hosted mode; see scroll-gif.ts). Caches on the same fingerprint scheme as
* `capturePage`/`uploadDiffImage` — a scroll capture is the most expensive thing this pipeline does (up to 6
* extra renders plus a full encode), so a re-review of the same head must never redo it.
*/
async function captureScrollGif(
env: Env,
target: CaptureTarget,
page: string,
slot: "before" | "after",
viewportName: "desktop" | "mobile",
viewport: Viewport,
theme?: ShotTheme | undefined,
): Promise<string | undefined> {
if (!page) return undefined;
const shotBase = env.PUBLIC_API_ORIGIN;
if (!env.REVIEW_AUDIT || !shotBase) return undefined;
const fingerprint = await sha256Hex(`${target.headSha ?? target.prNumber}:scrollgif:${slot}:${viewportName}:${page}${theme ? `:${theme}` : ""}`);
const key = `${NAMESPACE}/shots/${fingerprint.slice(0, 40)}.gif`;
const url = `${shotBase}/${NAMESPACE}/shot?key=${encodeURIComponent(key)}`;
const cached = await env.REVIEW_AUDIT.get(key).catch(() => null);
if (cached) return url;
const { frames, authWalled } = await captureScrollFrames(env, page, viewport, theme ? { theme } : {}).catch(() => ({ frames: [] as Uint8Array[], authWalled: false }));
if (authWalled || frames.length === 0) return undefined;
const gifBytes = await encodeScrollGif(
frames.map((png) => ({ png })),
GIF_FRAME_DELAY_MS,
);
if (!gifBytes) return undefined;
await env.REVIEW_AUDIT.put(key, gifBytes, { httpMetadata: { contentType: "image/gif" } }).catch(() => undefined);
return url;
}

/** Per-repo `review.visual` config, as resolved by the caller from the manifest (#3609 / #3610 / #3678 /
* #3612). Absent ⇒ byte-identical to today (GitHub-native discovery, automatic route inference, single
* default-theme capture, built-in route cap, no scroll-GIF). */
export type VisualCaptureConfig = {
preview?: VisualPreviewInput | null | undefined;
routes?: VisualRoutesInput | null | undefined;
themes?: readonly ShotTheme[] | null | undefined;
gif?: boolean | null | undefined;
};

/**
* Build the before/after capture for a PR: resolve the preview URL, derive routes from the changed UI files,
Expand Down Expand Up @@ -278,6 +327,12 @@ export async function buildCapture(env: Env, token: string, target: CaptureTarge
// #3674: resolved ONCE per call, not per route/viewport — false in every hosted build (see pixel-diff.ts),
// so capturePage never pays the extra cached-bytes-read cost unless self-host's real diff module is active.
const diffAvailable = isVisualDiffAvailable();
// #3612: gated on BOTH the opt-in config AND isScrollGifAvailable — hosted mode can never assemble a GIF
// (see scroll-gif.ts), so this must short-circuit before capturing a single scroll frame there, not just
// before encoding one. Desktop-viewport only in this first cut: a scroll-through GIF is already the
// heaviest capture mode (up to 6 extra renders per side), and doubling it for mobile is a narrower-scope
// call deferred to a follow-up rather than shipped speculatively (matches #3674's hosted-diff deferral).
const gifWanted = visualConfig?.gif === true && isScrollGifAvailable();
const routes = resolveVisualRoutes(visualFiles, visualConfig?.routes);
// #3678: an explicit, non-empty theme list captures the SAME routes once per theme, each tagged on its
// CaptureRoute entry. [undefined] (the default, absent config) renders the single un-emulated default —
Expand Down Expand Up @@ -308,6 +363,12 @@ export async function buildCapture(env: Env, token: string, target: CaptureTarge
uploadDiffImage(env, target, path, "desktop", desktopDiff, theme),
uploadDiffImage(env, target, path, "mobile", mobileDiff, theme),
]);
const [beforeGifUrl, afterGifUrl] = gifWanted
? await Promise.all([
captureScrollGif(env, target, beforePage, "before", "desktop", DESKTOP_VIEWPORT, theme),
afterPage ? captureScrollGif(env, target, afterPage, "after", "desktop", DESKTOP_VIEWPORT, theme) : Promise.resolve<string | undefined>(undefined),
])
: [undefined, undefined];
captureRoutes.push({
path,
...(theme ? { theme } : {}),
Expand All @@ -317,6 +378,8 @@ export async function buildCapture(env: Env, token: string, target: CaptureTarge
afterUrlMobile: afterMobileShot.url,
...(diffUrl ? { diffUrl } : {}),
...(diffUrlMobile ? { diffUrlMobile } : {}),
...(beforeGifUrl ? { beforeGifUrl } : {}),
...(afterGifUrl ? { afterGifUrl } : {}),
});
}
}
Expand Down
29 changes: 29 additions & 0 deletions src/review/visual/scroll-gif.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Scroll-through GIF assembly seam for the before/after capture pipeline (#3612). WORKER-SAFE DEFAULT: a no-op.
//
// A scroll-linked interaction (parallax, reveal-on-scroll, a sticky header) isn't visible in a single static
// screenshot — this assembles a short sequence of viewport-cropped frames (captured while scrolling down the
// page, see `captureScrollFrames` in ./shot) into one animated image. Turning those frames into a real
// animated image needs decoding each captured frame back to raw pixels, which — like the pixel-diff provider
// in ./pixel-diff — depends on Node's `Buffer` and a native-leaning image-decode step the Cloudflare Workers
// runtime doesn't guarantee. `test/unit/worker-entry-boundary.test.ts` enforces the same boundary here as it
// does for the pixel-diff module. `capture.ts` (Worker-reachable) imports ONLY this file; never the self-host
// module directly. `scripts/build-selfhost.mjs`'s esbuild plugin swaps this exact specifier for a real
// implementation when bundling the self-host entry (`src/server.ts`) — the same module-substitution pattern
// already used for pixel-diff and `@cloudflare/puppeteer` in that same build. The Worker's own (wrangler)
// bundle never applies that swap, so hosted mode always uses this no-op — zero behavior change, zero added
// capture cost, until a Workers-compatible image-decode path exists.
export type ScrollGifFrame = { png: Uint8Array };

/** True when this build can actually assemble a scroll-through GIF (self-host only, see module header).
* Callers use this to decide whether it's worth paying the extra cost of capturing stepped scroll frames at
* all — always false here, so nothing about the existing capture path changes in hosted mode. */
export function isScrollGifAvailable(): boolean {
return false;
}

/** Assemble captured frames into an animated image. Always null in the Worker-safe default — self-host's
* swapped-in implementation does the real encode. Callers must treat null as "no GIF available", never a
* failure — a missing GIF degrades the collapsible section to omitting that route, not an error. */
export async function encodeScrollGif(_frames: readonly ScrollGifFrame[], _frameDelayMs: number): Promise<Uint8Array | null> {
return null;
}
Loading
Loading