No media is available for this review.
';
- return;
- }
+ elements["screenshots-tab"].classList.toggle("hidden", mediaCount === 0);
const visibleIds = new Set([...screenshots.map((shot) => shot.id), ...changedImages.map((image) => image.id)]);
for (const [id, url] of state.screenshotUrls) {
if (!visibleIds.has(id)) {
@@ -4920,39 +4925,64 @@ async function renderScreenshots() {
state.screenshotUrls.delete(id);
}
}
+ if (mediaCount === 0) {
+ elements.screenshots.innerHTML = 'No media is available for this review.
';
+ return;
+ }
+ async function mediaUrl(id, path) {
+ if (state.screenshotUrls.has(id)) return state.screenshotUrls.get(id);
+ let pending = state.screenshotLoads.get(id);
+ if (!pending) {
+ pending = apiBlob(path).finally(() => state.screenshotLoads.delete(id));
+ state.screenshotLoads.set(id, pending);
+ }
+ const blob = await pending;
+ if (requestId !== state.screenshotRenderId) return;
+ if (!state.screenshotUrls.has(id)) state.screenshotUrls.set(id, URL.createObjectURL(blob));
+ return state.screenshotUrls.get(id);
+ }
elements.screenshots.innerHTML = [
...screenshots.map((shot) => `
diff --git a/scripts/check-monaco.ts b/scripts/check-monaco.ts
index 24a5a90..2d17b26 100644
--- a/scripts/check-monaco.ts
+++ b/scripts/check-monaco.ts
@@ -23,7 +23,7 @@ try {
}
const groups = {
briefing: { samplePr: false, scripts: ["briefing-entry", "briefing-source", "briefing", "briefing-code", "briefing-diagrams"] },
- local: { samplePr: false, scripts: ["monaco-foundation", "monaco-scroll", "monaco-click-focus", "file-header", "monaco-edits", "monaco-edits-syntax", "monaco-themes", "renderer-preference", "monaco-gutter", "monaco-wrapping", "markdown-preview", "monaco-semantic", "monaco-mount", "monaco-paged-draft"] },
+ local: { samplePr: false, scripts: ["monaco-foundation", "monaco-scroll", "monaco-click-focus", "media", "file-header", "monaco-edits", "monaco-edits-syntax", "monaco-themes", "renderer-preference", "monaco-gutter", "monaco-wrapping", "markdown-preview", "monaco-semantic", "monaco-mount", "monaco-paged-draft"] },
navigation: { samplePr: true, scripts: ["monaco-phase", "monaco-phase-scroll", "monaco-upward-tour", "monaco-reviewed-scroll", "monaco-sticky", "monaco-file-explorer"] },
discussions: { samplePr: true, scripts: ["review-pane", "renderer-comments", "monaco-context", "monaco-edits", "monaco-edits-syntax", "monaco-discussions"] },
before: { samplePr: true, beforeTour: true, scripts: ["monaco-before-tour"] },
diff --git a/scripts/media-browser.mjs b/scripts/media-browser.mjs
new file mode 100644
index 0000000..e025dd4
--- /dev/null
+++ b/scripts/media-browser.mjs
@@ -0,0 +1,80 @@
+import assert from "node:assert/strict";
+import { pathToFileURL } from "node:url";
+const [playwrightPath, url] = process.argv.slice(2);
+const { chromium } = await import(pathToFileURL(playwrightPath).href);
+const browser = await chromium.launch({ channel: process.env.CI ? undefined : "chrome", headless: true });
+const png = Buffer.from("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=", "base64");
+const changedCount = 60;
+try {
+ for (const [curatedCount, includeChanged] of [[2, false], [0, false], [2, true], [0, true]]) {
+ const page = await browser.newPage({ viewport: { width: 1400, height: 850 }, reducedMotion: "reduce" });
+ const requested = new Set(), errors = [];
+ page.on("pageerror", error => errors.push(error.message));
+ await page.route("**/api/update", route => route.fulfill({ json: { updateAvailable: false } }));
+ await page.route("**/api/session", async route => {
+ const response = await route.fetch();
+ const session = await response.json();
+ const image = session.comparison.files.find(file => file.path === "screens/receipt.png");
+ assert.ok(image, "Fixture retains a real before/after PNG");
+ session.comparison.files.push(...Array.from({ length: changedCount - 1 }, (_, index) => {
+ const path = `screens/reference-${index}.png`;
+ return { ...image, path, oldPath: path, newPath: path };
+ }));
+ session.screenshots = Array.from({ length: curatedCount }, (_, index) => ({ id: `qa-${index}`, name: `QA ${index}.png`, mimeType: "image/png", size: png.length, fileName: `qa-${index}.png` }));
+ session.mediaIncludeChangedImages = includeChanged;
+ await route.fulfill({ json: session });
+ });
+ await page.route("**/api/screenshots/*", route => route.fulfill({ body: png, contentType: "image/png" }));
+ await page.route("**/api/diff/image?*", async route => {
+ const request = new URL(route.request().url());
+ if (!request.searchParams.get("path").startsWith("screens/reference-")) return route.continue();
+ requested.add(`${request.searchParams.get("path")}:${request.searchParams.get("side")}`);
+ await route.fulfill({ body: png, contentType: "image/png" });
+ });
+ try {
+ await page.goto(url);
+ await page.waitForFunction(() => document.getElementById("diff-renderer")?.disabled === false);
+ const expected = curatedCount + (includeChanged ? 2 * changedCount : 0);
+ await page.waitForFunction(expected => document.getElementById("screenshots-tab-count").textContent === String(expected), expected);
+ await page.waitForTimeout(150);
+ assert.equal(requested.size, 0, "A hidden Media tab must not fetch changed images");
+ assert.equal(await page.locator("#screenshots-tab").isVisible(), expected > 0);
+ if (!expected) {
+ const tabs = await page.locator('.tab:not(.hidden)').evaluateAll(nodes => nodes.map(node => node.id));
+ assert.ok(!tabs.includes("screenshots-tab"), "Changed PNGs alone must not expose Media by default");
+ } else {
+ await page.locator("#screenshots-tab").click();
+ assert.equal(await page.locator("#screenshots .screenshot-card").count(), expected);
+ await page.waitForFunction(count => document.querySelectorAll("#screenshots [data-screenshot-id] img").length === count, curatedCount);
+ assert.equal(await page.locator("#screenshots [data-screenshot-id] img").count(), curatedCount);
+ if (!includeChanged) {
+ assert.equal(requested.size, 0, "Default Media must not fetch screenshot-test references");
+ assert.equal(await page.locator("#screenshots .changed-image-media").count(), 0);
+ } else {
+ const last = page.locator("#screenshots .changed-image-media").last();
+ assert.equal(await last.locator("img").count(), 0, "Offscreen cards remain unloaded");
+ // Explicit activation also works without observer visibility (e.g. assistive UI).
+ await last.locator("button").evaluate(button => button.click());
+ await last.locator("img").waitFor({ state: "attached" });
+ assert.ok(requested.has("screens/reference-58.png:new"));
+ const middle = page.locator("#screenshots .changed-image-media").nth(60);
+ assert.equal(await middle.locator("img").count(), 0);
+ await middle.scrollIntoViewIfNeeded();
+ await middle.locator("img").waitFor({ state: "visible" });
+ assert.ok(requested.size < changedCount, `Scrolling must not fetch the entire gallery (${requested.size} requests)`);
+ }
+ }
+ // The original PNG still opens in Changes with both captured versions.
+ await page.locator("#diff-tab").click();
+ await page.evaluate(() => diffCompassProductionHost.reveal("screens/receipt.png", "new", 1));
+ const comparison = page.locator('.diff-file[data-path="screens/receipt.png"] .image-diff');
+ await comparison.locator(".old img").waitFor({ state: "visible" });
+ await comparison.locator(".new img").waitFor({ state: "visible" });
+ assert.deepEqual(errors, []);
+ console.log(`PASS Media ${expected}: ${curatedCount} curated files, ${changedCount} changed PNGs, opt-in ${includeChanged}; lazy requests and Changes comparison`);
+ } finally {
+ await page.unrouteAll({ behavior: "wait" });
+ await page.close();
+ }
+ }
+} finally { await browser.close(); }
diff --git a/src/cli.ts b/src/cli.ts
index e514f21..1d80ccb 100755
--- a/src/cli.ts
+++ b/src/cli.ts
@@ -32,6 +32,7 @@ interface Options {
disableFindings: boolean;
previewRedirect: boolean;
media: string[];
+ mediaIncludeChangedImages?: boolean;
intentFile?: string;
resume?: string;
pullRequest?: string;
@@ -67,6 +68,8 @@ Options:
--port PORT Loopback port (default: random)
--preview-redirect Redirect / to the tokenized URL so remote previews work
--media PATH Add an image or MP4 to the Media tab (repeatable)
+ --media-include-changed-images
+ Also show changed PNGs in Media (loaded on demand)
--intent-file PATH Give the Briefing author-stated context from a text file
--resume FILE Resume a persisted review session
--version Print the installed version
@@ -100,6 +103,8 @@ function parseOptions(args: string[]): Options {
options.open = true;
} else if (arg === "--preview-redirect") {
options.previewRedirect = true;
+ } else if (arg === "--media-include-changed-images") {
+ options.mediaIncludeChangedImages = true;
} else if (arg === "--media" && value) {
options.media.push(resolve(value));
index += 1;
@@ -264,6 +269,7 @@ try {
includeFindings: options.disableFindings ? false : undefined,
pullRequest,
screenshots: options.media,
+ mediaIncludeChangedImages: options.mediaIncludeChangedImages ?? resumedState?.mediaIncludeChangedImages ?? repositoryConfig.mediaIncludeChangedImages,
authorIntent,
});
if (session.pullRequest?.workspaceRoot) {
diff --git a/src/config.ts b/src/config.ts
index 2cc39ae..8c1f617 100644
--- a/src/config.ts
+++ b/src/config.ts
@@ -16,6 +16,7 @@ export interface RepositoryConfig {
preferences: ReviewPreferences;
retention: RetentionConfig;
reviewPolicy: ReviewPolicy;
+ mediaIncludeChangedImages: boolean;
}
export const DEFAULT_REVIEW_POLICY: ReviewPolicy = {
@@ -85,7 +86,7 @@ export async function loadRepositoryConfig(repositoryRoot: string): Promise
24000)) ||
(session.screenshots !== undefined && (!Array.isArray(session.screenshots) || session.screenshots.length > 12 || !session.screenshots.every(validScreenshot) || new Set(session.screenshots.map((shot) => shot.id)).size !== session.screenshots.length)) ||
+ (session.mediaIncludeChangedImages !== undefined && typeof session.mediaIncludeChangedImages !== "boolean") ||
(session.outline !== undefined && !validOutline(session.outline)) ||
(session.selection !== undefined && !validSelection(session.selection)) ||
(session.agentContinuation !== undefined && !validContinuation(session.agentContinuation)) ||
diff --git a/src/types.ts b/src/types.ts
index a8ebce0..2096565 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -941,6 +941,8 @@ export interface PersistedReviewSession {
dryRuns?: DryRunTrace[];
pullRequest?: PullRequestContext;
screenshots?: ReviewScreenshot[];
+ /** Optional for older sessions; changed PNGs are excluded from Media by default. */
+ mediaIncludeChangedImages?: boolean;
authorIntent?: string;
revision: number;
status: ReviewStatus;
diff --git a/tests/config.test.ts b/tests/config.test.ts
index 6e8723f..89330c3 100644
--- a/tests/config.test.ts
+++ b/tests/config.test.ts
@@ -22,6 +22,7 @@ test("missing repository configuration uses conservative defaults", async () =>
preferences: DEFAULT_REVIEW_PREFERENCES,
retention: { includeArchived: false },
reviewPolicy: DEFAULT_REVIEW_POLICY,
+ mediaIncludeChangedImages: false,
});
expect(DEFAULT_REVIEW_PREFERENCES.diffLayout).toBe("split");
});
@@ -35,6 +36,7 @@ test("loads every supported repository default", async () => {
reasoning: "high",
diff: { layout: "split", changedFilesOnly: true, contextLines: 25 },
review: { requireAllFilesReviewed: true, closeOutcome: "cancelled" },
+ media: { includeChangedImages: true },
retention: { maxAgeDays: 30, maxSessions: 50, includeArchived: true },
analysis: {
instructions: ["Treat authentication changes as high risk."],
@@ -43,6 +45,7 @@ test("loads every supported repository default", async () => {
}));
expect(await loadRepositoryConfig(directory)).toEqual({
+ mediaIncludeChangedImages: true,
base: "parent-feature",
agent: "claude",
model: "opus",
@@ -83,6 +86,9 @@ test("fails closed for unknown, mistyped, oversized, and linked configuration",
const path = join(directory, ".diffcompass.json");
for (const value of [
{ unexpected: true },
+ { media: true },
+ { media: { includeChangedImages: "yes" } },
+ { media: { includeImages: true } },
{ diff: { contextLines: 2 } },
{ diff: { contextLines: 201 } },
{ diff: { changedFilesOnly: "yes" } },
diff --git a/tests/lifecycle.test.ts b/tests/lifecycle.test.ts
index 112e518..26774fd 100644
--- a/tests/lifecycle.test.ts
+++ b/tests/lifecycle.test.ts
@@ -298,6 +298,25 @@ test("startup screenshots are snapshotted, authenticated, and persisted across r
const resumed = await ReviewSession.create(repository, undefined, { ...noAdapters, stateFile, resume: true });
expect(resumed.snapshot().screenshots).toEqual(session.snapshot().screenshots);
+ expect(resumed.snapshot().mediaIncludeChangedImages).toBe(false);
+});
+
+test("changed-image Media opt-in persists, while legacy sessions default to curated media", async () => {
+ const { repository, stateFile } = await repositoryFixture();
+ await savePersistedSession(stateFile, persistedAttachedSession(repository));
+ const legacy = await ReviewSession.create(repository, undefined, { ...noAdapters, stateFile, resume: true });
+ expect(legacy.snapshot().mediaIncludeChangedImages).toBe(false);
+ await legacy.dispose();
+ const enabled = await ReviewSession.create(repository, undefined, { ...noAdapters, stateFile, resume: true, mediaIncludeChangedImages: true });
+ expect(enabled.snapshot().mediaIncludeChangedImages).toBe(true);
+ await enabled.dispose();
+ expect((await loadPersistedSession(stateFile)).mediaIncludeChangedImages).toBe(true);
+ const resumed = await ReviewSession.create(repository, undefined, { ...noAdapters, stateFile, resume: true });
+ expect(resumed.snapshot().mediaIncludeChangedImages).toBe(true);
+ await resumed.dispose();
+ const saved = JSON.parse(await readFile(stateFile, "utf8"));
+ await writeFile(stateFile, JSON.stringify({ ...saved, mediaIncludeChangedImages: "yes" }));
+ await expect(loadPersistedSession(stateFile)).rejects.toThrow();
});
test("selected diff preferences survive an existing session resume", async () => {
diff --git a/tests/media-cli.test.ts b/tests/media-cli.test.ts
new file mode 100644
index 0000000..8b5dde9
--- /dev/null
+++ b/tests/media-cli.test.ts
@@ -0,0 +1,74 @@
+import { expect, test } from "bun:test";
+import { mkdir, mkdtemp, readFile, readdir, realpath, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { runProcess } from "../src/process.ts";
+
+test("CLI media defaults, flag/config precedence and resume preserve imported evidence", async () => {
+ const root = await realpath(await mkdtemp(join(tmpdir(), "diffcompass-media-cli-")));
+ const repository = join(root, "repo"), stateRoot = join(root, "state");
+ const sessions = join(stateRoot, "diffcompass", "sessions");
+ const children: ReturnType[] = [];
+ async function start(args: string[]) {
+ const child = Bun.spawn([process.execPath, join(import.meta.dir, "../src/cli.ts"), "--repo", repository, "--base", "HEAD", "--attached", "--no-findings", ...args], {
+ cwd: repository, env: { ...process.env, XDG_STATE_HOME: stateRoot }, stdout: "ignore", stderr: "ignore",
+ });
+ children.push(child);
+ const deadline = Date.now() + 10_000;
+ while (child.exitCode === null && Date.now() < deadline) {
+ for (const name of (await readdir(sessions).catch(() => [])).filter(name => name.endsWith(".json"))) {
+ const path = join(sessions, name);
+ const lock = await readFile(`${path}.lock`, "utf8").then(JSON.parse).catch(() => null);
+ if (lock?.pid !== child.pid || !lock.reviewUrl) continue;
+ const state = JSON.parse(await readFile(path, "utf8"));
+ const response = await fetch(`${new URL(lock.reviewUrl).origin}/api/session`, { headers: { "x-diffcompass-token": state.browserToken } });
+ expect(response.status).toBe(200);
+ return { path, snapshot: await response.json(), stop: async () => {
+ child.kill();
+ const timer = setTimeout(() => child.kill("SIGKILL"), 2_000);
+ try { await child.exited; } finally { clearTimeout(timer); }
+ } };
+ }
+ await Bun.sleep(20);
+ }
+ throw Error("Media CLI fixture did not start");
+ }
+ const config = (enabled: boolean) => writeFile(join(repository, ".diffcompass.json"), JSON.stringify({ media: { includeChangedImages: enabled } }));
+ try {
+ await mkdir(repository);
+ for (const args of [["init", "-b", "main"], ["config", "user.name", "Media test"], ["config", "user.email", "media@example.invalid"]]) await runProcess(["git", ...args], repository);
+ await writeFile(join(repository, "app.ts"), "const version = 1;\n");
+ await runProcess(["git", "add", "."], repository);
+ await runProcess(["git", "commit", "-m", "Initial"], repository);
+ await writeFile(join(repository, "app.ts"), "const version = 2;\n");
+ const media = join(root, "qa.png");
+ await writeFile(media, Buffer.from("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=", "base64"));
+ const original = await start(["--media", media]);
+ expect(original.snapshot.mediaIncludeChangedImages).toBe(false);
+ expect(original.snapshot.screenshots).toHaveLength(1);
+ await original.stop();
+ await config(true);
+ const resumed = await start(["--resume", original.path]);
+ expect(resumed.snapshot.mediaIncludeChangedImages).toBe(false); // Saved choice wins over changed config.
+ expect(resumed.snapshot.screenshots).toEqual(original.snapshot.screenshots);
+ await resumed.stop();
+ await config(false);
+ const enabled = await start(["--resume", original.path, "--media-include-changed-images"]);
+ expect(enabled.snapshot.mediaIncludeChangedImages).toBe(true); // Explicit flag wins.
+ expect(enabled.snapshot.screenshots).toEqual(original.snapshot.screenshots);
+ await enabled.stop();
+ const enabledResume = await start(["--resume", original.path]);
+ expect(enabledResume.snapshot.mediaIncludeChangedImages).toBe(true);
+ await enabledResume.stop();
+ await config(true);
+ const configured = await start([]);
+ expect(configured.snapshot.mediaIncludeChangedImages).toBe(true);
+ expect(configured.snapshot.screenshots).toEqual([]);
+ await configured.stop();
+ const help = await runProcess([process.execPath, join(import.meta.dir, "../src/cli.ts"), "pr", "123", "--media-include-changed-images", "--help"], repository);
+ expect(help.stdout).toContain("--media-include-changed-images");
+ } finally {
+ for (const child of children) if (child.exitCode === null) { child.kill("SIGKILL"); await child.exited; }
+ await rm(root, { recursive: true, force: true });
+ }
+}, 30_000);
diff --git a/tests/ui.test.ts b/tests/ui.test.ts
index 14328c5..a86c7f4 100644
--- a/tests/ui.test.ts
+++ b/tests/ui.test.ts
@@ -1743,17 +1743,17 @@ test("images and videos are exposed through an authenticated media tab with an e
readFile(join(root, "public/app.js"), "utf8"),
readFile(join(root, "public/styles.css"), "utf8"),
]);
- expect(html).toContain('id="screenshots-tab" class="tab"');
+ expect(html).toContain('id="screenshots-tab" class="tab hidden"');
expect(html).toContain('id="screenshots-view" class="review-view hidden"');
expect(html).toContain('>Media 0');
expect(app).toContain("function changedImageMedia()");
expect(app).toContain("const mediaCount = screenshots.length + changedImages.length");
- expect(app).toContain('"conversations", "screenshots", ...(state.session?.pullRequest');
+ expect(app).toContain('state.session?.mediaIncludeChangedImages !== true');
expect(app).toContain('elements["screenshots-tab-count"].classList.toggle("hidden", mediaCount === 0)');
expect(app).toContain('No media is available for this review.');
expect(app).toContain('"X-DiffCompass-Token": state.apiToken');
- expect(app).toContain('apiBlob(`/api/screenshots/${encodeURIComponent(shot.id)}`)');
- expect(app).toContain('apiBlob(`/api/diff/image?${new URLSearchParams({ path: image.path, side: image.side })}`)');
+ expect(app).toContain('mediaUrl(shot.id, `/api/screenshots/${encodeURIComponent(shot.id)}`)');
+ expect(app).toContain('mediaUrl(image.id, `/api/diff/image?${new URLSearchParams({ path: image.path, side: image.side })}`)');
expect(app).toContain('shot.mimeType === "video/mp4"');
expect(styles).toContain(".screenshot-gallery { display: grid;");
expect(styles).toContain(".media-empty { grid-column: 1 / -1; }");