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
123 changes: 120 additions & 3 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ import { randomUUID } from "node:crypto";
import { isRetryableJobError, RetryableJobError } from "./retryable";
import { screenshotsAllowed } from "../review/visual-wire";
import { isVisualPath } from "../review/visual/paths";
import { buildCapture, hasSuccessfulBotCapture, resolveVisualRoutes, type CaptureRoute } from "../review/visual/capture";
import { buildCapture, fetchShotContentBlock, hasSuccessfulBotCapture, resolveVisualRoutes, type CaptureRoute } from "../review/visual/capture";
import {
clearFallbackDispatchMarker,
fallbackShotFileName,
Expand All @@ -368,6 +368,13 @@ import {
FALLBACK_WORKFLOW_NAME,
parseFallbackRunCorrelation,
} from "../review/visual/actions-fallback";
import {
buildVisualRegressionFindings,
buildVisualVisionUserPrompt,
evaluateVisualVisionGate,
parseVisualVisionResponse,
VISUAL_VISION_SYSTEM_PROMPT,
} from "../review/visual/visual-findings";
import { incr } from "../selfhost/metrics";
import {
renderReviewingPlaceholder,
Expand Down Expand Up @@ -419,6 +426,7 @@ import { getLastRepoDocRefreshAttemptedAtBulk, performRepoDocRefresh } from "../
import { isRepoDocRefreshDue } from "../review/repo-doc-refresh-schedule";
import type { LocalBranchAnalysisInput } from "../signals/local-branch";
import {
callAiProvider,
hasPublicReviewAssessment,
isEnabled,
runGittensoryAiReview,
Expand Down Expand Up @@ -535,9 +543,10 @@ import {
runSelfTuneBreaker,
} from "../review/outcomes-wire";
import { neutralHoldReasonCode, recordNativeGateDecision } from "../review/parity-wire";
import type { SubmissionOutcome } from "../review/submitter-reputation";
import { getSubmitterReputation, type SubmissionOutcome } from "../review/submitter-reputation";
import type {
AdvisoryFinding,
AiContentBlock,
ContributorEvidenceRecord,
ContributorRepoStatRecord,
DetectedNotificationEvent,
Expand Down Expand Up @@ -8187,6 +8196,97 @@ class RetryablePublicSurfacePublishFailedError extends RetryableJobError {
}
}

/**
* AI-vision analysis of a confirmed visual regression (#4111 wiring): the existing pixel-diff threshold can
* tell "the pixels changed" but not "does it look broken" — a route the capture pipeline already flagged
* changed gets ONE more look from a real vision-capable model. Mirrors runAiReviewForAdvisory's own shape
* (resolve reputation + BYOK, gate, call, parse, mutate `args.advisory.findings`) so it can be exercised
* directly in tests without driving the full webhook pipeline. STRICTLY ADVISORY: `visual_regression_finding`
* can never become a gate blocker (see visual-findings.ts's header) — this only ever adds a "Visual findings"
* collapsible to the comment. Never throws: any failure (a broken image fetch, a provider error, an
* unparseable response) degrades to "no finding added", exactly like the capture block it runs after.
*/
export async function runVisualVisionForAdvisory(
env: Env,
args: {
repoFullName: string;
pr: { number: number };
author: string | null;
confirmedContributor: boolean;
settings: RepositorySettings;
advisory: { findings: AdvisoryFinding[] };
routes: readonly CaptureRoute[];
},
): Promise<void> {
if (args.routes.length === 0) return;
try {
const visionReputation = await getSubmitterReputation(env, args.repoFullName, args.author ?? undefined);
// BYOK resolution mirrors runAiReviewForAdvisory's own (re-resolved per-caller is this codebase's
// established convention for this exact 3-line block, not an anti-pattern — see e.g. runAiSlopForAdvisory).
const storedVisionKey =
args.confirmedContributor && args.settings.aiReviewByok
? await getDecryptedRepositoryAiKey(env, args.repoFullName)
: null;
const visionProviderKey =
storedVisionKey &&
(!args.settings.aiReviewProvider || args.settings.aiReviewProvider === storedVisionKey.provider)
? {
provider: storedVisionKey.provider,
key: storedVisionKey.key,
model: args.settings.aiReviewModel ?? storedVisionKey.model,
}
: null;
const visionGate = evaluateVisualVisionGate({
routes: args.routes,
reputationSignal: visionReputation.signal,
providerKey: visionProviderKey,
});
if (!visionGate.run) return;
// evaluateVisualVisionGate only ever returns run:true when its own providerKey input (the SAME
// visionProviderKey resolved above) was non-null -- this is a defensive type-narrowing guard for the
// callAiProvider call below, not a reachable false case.
/* v8 ignore next -- see comment above */
if (!visionProviderKey) return;
const images: AiContentBlock[] = [];
for (const route of visionGate.routes) {
// Show the model the viewport that actually crossed the pixel-diff threshold — a route can qualify via
// desktop, mobile, or both; preferring desktop only when BOTH changed keeps this a single before/after
// pair per route (the prompt's own "before, after order" contract), same as
// routeHasConfirmedVisualRegression's own desktop-first `||` check.
const useMobile = !route.diffUrl && Boolean(route.diffUrlMobile);
const beforeShotUrl = useMobile ? route.beforeUrlMobile : route.beforeUrl;
const afterShotUrl = useMobile ? route.afterUrlMobile : route.afterUrl;
if (!beforeShotUrl || !afterShotUrl) continue;
const [beforeBlock, afterBlock] = await Promise.all([
fetchShotContentBlock(beforeShotUrl),
fetchShotContentBlock(afterShotUrl),
]);
if (beforeBlock) images.push(beforeBlock);
if (afterBlock) images.push(afterBlock);
}
if (images.length === 0) return;
const visionResponse = await callAiProvider(
visionProviderKey,
VISUAL_VISION_SYSTEM_PROMPT,
buildVisualVisionUserPrompt(visionGate.routes),
600,
images,
);
if (!visionResponse.text) return;
const visionFindings = parseVisualVisionResponse(visionResponse.text);
args.advisory.findings.push(...buildVisualRegressionFindings(visionFindings));
} catch (error) {
console.log(
JSON.stringify({
event: "visual_vision_error",
repoFullName: args.repoFullName,
pull: args.pr.number,
message: errorMessage(error).slice(0, 200),
}),
);
}
}

async function maybePublishPrPublicSurface(
env: Env,
installationId: number,
Expand Down Expand Up @@ -8224,6 +8324,11 @@ async function maybePublishPrPublicSurface(
},
): Promise<ReturnType<typeof evaluateGateCheck> | undefined> {
const author = pr.authorLogin ?? null;
// Hoisted out of the try-block below (where it's actually resolved) so the AI-vision step further down --
// which needs the SAME already-resolved confirmed-Gittensor status for its own BYOK gate, mirroring
// runAiReviewForAdvisory's identical check -- can read it without a second, audit-event-duplicating
// getCachedOfficialMinerDetection lookup. Defaults false; only ever set true inside that try-block.
let confirmedContributor = false;
// Resolve the repo's action mode ONCE for the whole publish pass and thread it into every GitHub write below, so
// a dry-run / pause / global-freeze publishes NOTHING (check-run, comment, label) — the gate verdict is still
// computed + returned for the disposition logic, the writes are just suppressed + audited. (#dry-run-chokepoint)
Expand Down Expand Up @@ -8908,7 +9013,7 @@ async function maybePublishPrPublicSurface(
// Resolve the author's confirmed-Gittensor status. It feeds on-chain SCORING and the public surface, but
// it no longer gates the verdict — every author is hard-blocked the same way on a configured blocker, and
// a clean PR passes the same way. (#gate-nonconfirmed)
const confirmedContributor = official?.status === "confirmed";
confirmedContributor = official?.status === "confirmed";

// Anti-slop (#530/#532): only when opted in (slopGateMode !== "off"). Surface the deterministic slop
// findings as advisory context, and feed the score to the gate (it only blocks under slop: block + the
Expand Down Expand Up @@ -10431,6 +10536,18 @@ async function maybePublishPrPublicSurface(
);
}
}
// AI-vision analysis of a confirmed visual regression (#4111 wiring) — see runVisualVisionForAdvisory's
// own doc comment. Deliberately independent of the capture block above (its own try/catch there) so a
// vision failure can never affect the "Visual preview" section that block already rendered.
await runVisualVisionForAdvisory(env, {
repoFullName,
pr,
author,
confirmedContributor,
settings,
advisory,
routes: beforeAfter,
});
// review.memory (#2181, apply slice of #1964): before the unified comment renders, suppress/demote
// advisory (non-blocking) findings a maintainer already dismissed as false positives for this repo. ONLY
// ever applied to `commentGate.warnings` -- NEVER `commentGate.blockers` -- so this can never change the
Expand Down
22 changes: 21 additions & 1 deletion src/review/visual/capture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
// buildCapture), adapted to gittensory bindings + origins. The agent-config-driven route rules, authed-route
// preview session, and explicit-route override are intentionally dropped here — gittensory's UI uses the
// default TanStack route convention; those hooks can return if a per-repo visual config is added.
import { sha256Hex } from "../../utils/crypto";
import { base64Encode, sha256Hex } from "../../utils/crypto";
import type { AiContentBlock } from "../../types";
import type { GitHubRateLimitAdmissionKey } from "../../github/client";
import { dispatchVisualCaptureFallback, fallbackShotR2Key, isFallbackDispatchInFlight, markFallbackDispatched } from "./actions-fallback";
import {
Expand Down Expand Up @@ -88,6 +89,25 @@ export function hasSuccessfulBotCapture(routes: readonly CaptureRoute[]): boolea
return routes.some(routeHasRealBeforeAfterPair);
}

/**
* Fetch an already-captured shot (a `CaptureRoute.before*`/`after*` URL) and return it as an `AiContentBlock`
* for a vision-capable AI call (#4111 wiring) — every captured shot is a PNG (see `capturePage`'s
* `screenshot({type: "png", ...})` call in `./shot.ts`), so the MIME type is fixed rather than sniffed.
* Returns undefined on any fetch/read failure so one broken image degrades to "drop this image", never a
* thrown error — mirrors `capturePage`'s own "returns null on any failure so callers degrade gracefully"
* convention.
*/
export async function fetchShotContentBlock(url: string): Promise<AiContentBlock | undefined> {
try {
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" };
} catch {
return undefined;
}
}

/** Inputs the capture pipeline needs about the PR under review (resolved by the caller from gittensory data). */
export interface CaptureTarget {
repoFullName: string;
Expand Down
2 changes: 1 addition & 1 deletion src/utils/crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export async function decryptSecret(ciphertext: string, iv: string, keyMaterial:
return new TextDecoder().decode(decrypted);
}

function base64Encode(bytes: Uint8Array): string {
export function base64Encode(bytes: Uint8Array): string {
let binary = "";
for (const byte of bytes) binary += String.fromCharCode(byte);
return btoa(binary);
Expand Down
24 changes: 23 additions & 1 deletion test/unit/visual-capture.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
latestGitHubRestRateLimitObservation,
} from "../../src/github/client";
import { fallbackShotR2Key, markFallbackDispatched } from "../../src/review/visual/actions-fallback";
import { buildCapture, hasSuccessfulBotCapture, mapFilesToRoutes, resolvePreviewUrlTemplate, resolveVisualRoutes } from "../../src/review/visual/capture";
import { buildCapture, fetchShotContentBlock, hasSuccessfulBotCapture, mapFilesToRoutes, resolvePreviewUrlTemplate, resolveVisualRoutes } from "../../src/review/visual/capture";
import type { CaptureRoute } from "../../src/review/visual/capture";
import * as pixelDiffModule from "../../src/review/visual/pixel-diff";
import * as previewUrlModule from "../../src/review/visual/preview-url";
Expand Down Expand Up @@ -1555,3 +1555,25 @@ describe("review.visual.actions_fallback (#4112 GitHub-Actions build-and-serve f
expect(result.routes[0]?.afterUrl).toContain("placeholder=loading");
});
});

describe("fetchShotContentBlock (#4111)", () => {
afterEach(() => {
vi.unstubAllGlobals();
});

it("returns a base64-encoded image content block on a successful fetch", async () => {
vi.stubGlobal("fetch", vi.fn(async () => new Response(new Uint8Array([137, 80, 78, 71]), { status: 200 })));
const block = await fetchShotContentBlock("https://x/gittensory/shot?key=before");
expect(block).toEqual({ type: "image", data: Buffer.from([137, 80, 78, 71]).toString("base64"), mimeType: "image/png" });
});

it("returns undefined on a non-2xx response", async () => {
vi.stubGlobal("fetch", vi.fn(async () => new Response("not found", { status: 404 })));
await expect(fetchShotContentBlock("https://x/gittensory/shot?key=missing")).resolves.toBeUndefined();
});

it("returns undefined (never throws) when fetch itself rejects", async () => {
vi.stubGlobal("fetch", vi.fn(async () => { throw new Error("network down"); }));
await expect(fetchShotContentBlock("https://x/gittensory/shot?key=broken")).resolves.toBeUndefined();
});
});
Loading