feat: add P1 conditional visual review evidence

references/visual-evidence.md

@@ -0,0 +1,48 @@
+# Visual evidence method
+
+Use this method as a conditional review axis when `scripts/lib/visual-evidence.mjs` reports `required: true` for the current diff.
+
+It is not a standalone mutation workflow. It supplies evidence to review/status/merge gates for UI-affecting changes.
+
+## Triggering
+
+Visual evidence is required for changes with concrete visual-surface signals, including:
+
+- stylesheets;
+- product visual assets;
+- UI component/page/screen markup with visual or accessibility changes;
+- equivalent rendered-surface changes detected by the planner.
+
+Do not require product screenshots merely because arbitrary JavaScript/TypeScript changed or because a documentation image changed.
+
+## Evidence contract
+
+Evidence must be bound to the current PR head SHA. Accepted evidence kinds are:
+
+- screenshot;
+- video/recording;
+- deterministic render artifact.
+
+A text statement such as “looks good”, an old screenshot, or an artifact from another head is not visual evidence.
+
+Capture the smallest useful set that proves the changed states. Include relevant responsive, error, empty, loading, focus, or interaction states when the diff materially affects them. Do not manufacture huge screenshot matrices for unrelated surfaces.
+
+## Execution
+
+1. Run the normal review-scope planner first.
+2. If `visualEvidence.required` is false, do not load this method further.
+3. Start the application or supported preview path using the repository's documented setup. Do not weaken authentication, security, or production configuration merely to obtain a screenshot.
+4. Exercise the changed visual surface.
+5. Record artifacts with the exact current `headRefOid`.
+6. Validate them with `validateVisualEvidence(...)` before treating the axis as satisfied.
+7. If the preview/render cannot be executed because a real dependency, credential, environment, or tool is unavailable, return a specific `blocked` reason. Do not convert the blocker into a clean verdict.
+
+## Review use
+
+Visual evidence answers only rendered-behaviour questions. It does not replace code review, accessibility reasoning, tests, security review, or current-head ship gates.
+
+A visually correct screenshot cannot prove hidden state transitions, race safety, authorization, data integrity, or compatibility.
+
+## Provenance
+
+The conditional rendered-evidence idea was informed by `OutThisLife/brooklyn-skills` `visual-verify` (MIT, copyright Brooklyn Nicholson). GitHub Delivery implements it as a head-bound evidence axis inside its existing review architecture rather than as an independent approval system.

scripts/lib/review-scope.mjs

@@ -1,6 +1,7 @@
 import { spawnSync } from "node:child_process";
 
 import { PROBE_REGISTRY, validateProbeRegistry } from "./probe-registry.mjs";
+import { planVisualEvidence } from "./visual-evidence.mjs";
 
 const CODE_RE = /\.(?:[cm]?[jt]sx?|mjs|cjs|py|go|rs|java|kt|rb|php|cs|swift|c|cc|cpp|h|hpp|vue|svelte)$/i;
 const DOC_RE = /\.(?:md|txt|rst|adoc)$/i;
@@ -185,6 +186,7 @@ export function assertCompletePrFileEnumeration(expectedCount, observedCount) {
 
 export function planReviewScope(input = {}) {
   const files = (input.files || []).map(normalizeFile).filter((file) => file.path);
+  const visualEvidence = planVisualEvidence(files);
   const evidence = new Map();
   const lensEvidence = new Map();
   const removedControlLeads = [];
@@ -303,6 +305,7 @@ export function planReviewScope(input = {}) {
     bugLenses,
     securityReview: { depth: securityDepth, requiredDomains: requiredSecurity.map((item) => item.id) },
     bugReview: { depth: bugDepth, requiredLenses: requiredBug.map((item) => item.id) },
+    visualEvidence,
     requiredProbes,
     probeEvidence,
     baselineScreens: logicFiles.length ? ["authn", "authz", "secrets_config", "injection", "error_propagation", "boundary_conditions"] : [],
@@ -314,6 +317,7 @@ export function planReviewScope(input = {}) {
       "Use renamed source and destination paths when interpreting ownership and security boundaries.",
       "Do not skip a domain solely because another review tool reported clean results.",
       "Every probe in `requiredProbes` names a Must-probe block in bug-review.md / security-review.md; walk each one against the diff.",
+      ...(visualEvidence.required ? ["Rendered visual evidence is required for this diff; load references/visual-evidence.md and bind artifacts to the current head SHA."] : []),
     ],
   };
 }

scripts/lib/visual-evidence.mjs

@@ -0,0 +1,91 @@
+const STYLE_PATH_RE = /\.(?:css|scss|sass|less|styl)$/i;
+const VISUAL_ASSET_RE = /\.(?:avif|gif|jpe?g|png|svg|webp)$/i;
+const DOC_PATH_RE = /(^|\/)docs?\//i;
+const UI_PATH_RE = /(^|\/)(?:app|pages?|views?|screens?|components?|ui|frontend|web|client)(\/|$)/i;
+const UI_CODE_RE = /\.(?:html?|jsx?|tsx?|vue|svelte)$/i;
+const VISUAL_LINE_RE = /(?:className=|class=|style=|<img\b|<svg\b|<video\b|<canvas\b|<button\b|<dialog\b|<input\b|<select\b|<textarea\b|display\s*:|grid|flex|padding|margin|font|color|background|border|width|height|position\s*:|aria-|role=)/i;
+
+function patchChangedLines(patch = "") {
+  return String(patch).split(/\r?\n/).filter((line) =>
+    (line.startsWith("+") && !line.startsWith("+++")) ||
+    (line.startsWith("-") && !line.startsWith("---")),
+  ).map((line) => line.slice(1));
+}
+
+function normalizeFile(raw = {}) {
+  const path = String(raw.path ?? raw.filename ?? "").trim();
+  if (!path) throw new Error("visual_evidence_file_path_required");
+  return {
+    path,
+    previousPath: raw.previousPath ?? raw.previous_filename ?? null,
+    status: String(raw.status ?? "modified"),
+    patch: String(raw.patch ?? ""),
+  };
+}
+
+function reasonFor(file) {
+  const paths = [file.path, file.previousPath].filter(Boolean);
+  if (paths.some((path) => STYLE_PATH_RE.test(path))) return { score: 6, reason: "stylesheet_changed" };
+  if (paths.some((path) => VISUAL_ASSET_RE.test(path) && !DOC_PATH_RE.test(path))) {
+    return { score: 5, reason: "visual_asset_changed" };
+  }
+  if (paths.some((path) => UI_CODE_RE.test(path) && UI_PATH_RE.test(path))) {
+    const lines = patchChangedLines(file.patch);
+    if (!file.patch || lines.some((line) => VISUAL_LINE_RE.test(line))) return { score: 5, reason: "ui_surface_changed" };
+  }
+  if (UI_CODE_RE.test(file.path) && patchChangedLines(file.patch).some((line) => VISUAL_LINE_RE.test(line))) {
+    return { score: 4, reason: "visual_markup_or_style_changed" };
+  }
+  return null;
+}
+
+export function planVisualEvidence(rawFiles = []) {
+  const hits = [];
+  let score = 0;
+  for (const raw of rawFiles || []) {
+    const file = normalizeFile(raw);
+    const reason = reasonFor(file);
+    if (!reason) continue;
+    score = Math.max(score, reason.score);
+    hits.push({ file: file.path, reason: reason.reason, score: reason.score });
+  }
+  return {
+    required: hits.length > 0,
+    confidence: score >= 6 ? "high" : score >= 4 ? "medium" : "none",
+    files: [...new Set(hits.map((hit) => hit.file))].sort(),
+    reasons: hits,
+    acceptedEvidenceKinds: hits.length ? ["screenshot", "video", "render"] : [],
+  };
+}
+
+function exactSha(value) {
+  const text = String(value ?? "").trim().toLowerCase();
+  return /^[0-9a-f]{40,64}$/.test(text) ? text : null;
+}
+
+function usableArtifact(artifact, expectedHead) {
+  const kind = String(artifact?.kind ?? "").toLowerCase();
+  if (!["screenshot", "video", "render"].includes(kind)) return false;
+  const head = exactSha(artifact?.headRefOid);
+  if (!head || head !== expectedHead) return false;
+  const locator = String(artifact?.url ?? artifact?.path ?? artifact?.artifactId ?? "").trim();
+  return Boolean(locator);
+}
+
+export function validateVisualEvidence({ plan, headRefOid, artifacts = [], blocker = null } = {}) {
+  if (!plan?.required) return { state: "not_required", complete: true, artifacts: [] };
+  const expectedHead = exactSha(headRefOid);
+  if (!expectedHead) return { state: "unknown", complete: false, reason: "visual_evidence_head_missing", artifacts: [] };
+
+  const validArtifacts = (artifacts || []).filter((artifact) => usableArtifact(artifact, expectedHead));
+  if (validArtifacts.length > 0) {
+    return { state: "satisfied", complete: true, artifacts: validArtifacts };
+  }
+
+  const blockerReason = String(blocker?.reason ?? "").trim();
+  if (blocker?.state === "blocked" && blockerReason) {
+    return { state: "blocked", complete: false, reason: blockerReason, artifacts: [] };
+  }
+
+  return { state: "missing", complete: false, reason: "visual_evidence_required", artifacts: [] };
+}

tests/unit/review-scope-visual-evidence.test.mjs

@@ -0,0 +1,41 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { planReviewScope } from "../../scripts/lib/review-scope.mjs";
+
+test("review scope surfaces conditional visual evidence for UI changes", () => {
+  const plan = planReviewScope({
+    repo: "acme/widgets",
+    pr: 42,
+    headRefOid: "a".repeat(40),
+    files: [{
+      filename: "src/components/Card.tsx",
+      status: "modified",
+      patch: "@@ -1 +1 @@\n-return <div />;\n+return <button className=\"primary\">Go</button>;",
+      additions: 1,
+      deletions: 1,
+    }],
+  });
+
+  assert.equal(plan.visualEvidence.required, true);
+  assert.deepEqual(plan.visualEvidence.files, ["src/components/Card.tsx"]);
+  assert.ok(plan.instructions.some((line) => line.includes("references/visual-evidence.md")));
+});
+
+test("review scope does not demand visual evidence for backend-only logic", () => {
+  const plan = planReviewScope({
+    repo: "acme/widgets",
+    pr: 43,
+    headRefOid: "b".repeat(40),
+    files: [{
+      filename: "src/server/token.ts",
+      status: "modified",
+      patch: "@@ -1 +1 @@\n-return oldToken;\n+return nextToken;",
+      additions: 1,
+      deletions: 1,
+    }],
+  });
+
+  assert.equal(plan.visualEvidence.required, false);
+  assert.equal(plan.instructions.some((line) => line.includes("references/visual-evidence.md")), false);
+});

tests/unit/visual-evidence.test.mjs

@@ -0,0 +1,78 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { planVisualEvidence, validateVisualEvidence } from "../../scripts/lib/visual-evidence.mjs";
+
+const HEAD = "a".repeat(40);
+
+test("requires visual evidence for stylesheet changes", () => {
+  const plan = planVisualEvidence([{ path: "src/styles/app.css", patch: "+.card { display: grid; }" }]);
+  assert.equal(plan.required, true);
+  assert.equal(plan.confidence, "high");
+  assert.deepEqual(plan.files, ["src/styles/app.css"]);
+});
+
+test("requires visual evidence for UI markup changes but not arbitrary TypeScript", () => {
+  const ui = planVisualEvidence([{ path: "src/components/Card.tsx", patch: "+return <button className=\"primary\">Go</button>;" }]);
+  const backend = planVisualEvidence([{ path: "src/server/token.ts", patch: "+return token.length;" }]);
+  assert.equal(ui.required, true);
+  assert.equal(backend.required, false);
+});
+
+test("documentation images do not force product visual evidence", () => {
+  const plan = planVisualEvidence([{ path: "docs/example.png", status: "modified" }]);
+  assert.equal(plan.required, false);
+});
+
+test("visual assets renamed between docs and product paths still protect the product-side change", () => {
+  const intoProduct = planVisualEvidence([{
+    path: "src/assets/example.png",
+    previousPath: "docs/example.png",
+    status: "renamed",
+  }]);
+  assert.equal(intoProduct.required, true);
+  assert.deepEqual(intoProduct.files, ["src/assets/example.png"]);
+
+  const outOfProduct = planVisualEvidence([{
+    path: "docs/example.png",
+    previousPath: "src/assets/example.png",
+    status: "renamed",
+  }]);
+  assert.equal(outOfProduct.required, true);
+  assert.deepEqual(outOfProduct.files, ["docs/example.png"]);
+});
+
+test("visual evidence must be bound to the current head", () => {
+  const plan = planVisualEvidence([{ path: "src/styles/app.css", patch: "+color: red;" }]);
+  const stale = validateVisualEvidence({
+    plan,
+    headRefOid: HEAD,
+    artifacts: [{ kind: "screenshot", headRefOid: "b".repeat(40), path: "shot.png" }],
+  });
+  assert.equal(stale.state, "missing");
+  assert.equal(stale.complete, false);
+
+  const fresh = validateVisualEvidence({
+    plan,
+    headRefOid: HEAD,
+    artifacts: [{ kind: "screenshot", headRefOid: HEAD, path: "shot.png" }],
+  });
+  assert.equal(fresh.state, "satisfied");
+  assert.equal(fresh.complete, true);
+});
+
+test("an honest runtime blocker remains blocked, never satisfied", () => {
+  const plan = planVisualEvidence([{ path: "src/styles/app.css", patch: "+color: red;" }]);
+  const result = validateVisualEvidence({
+    plan,
+    headRefOid: HEAD,
+    blocker: { state: "blocked", reason: "preview cannot start without required service" },
+  });
+  assert.equal(result.state, "blocked");
+  assert.equal(result.complete, false);
+});
+
+test("non-visual changes need no artifact", () => {
+  const plan = planVisualEvidence([{ path: "src/server/token.ts", patch: "+return token.length;" }]);
+  assert.deepEqual(validateVisualEvidence({ plan }), { state: "not_required", complete: true, artifacts: [] });
+});