From 0e48a724d0b32b0bcd1187b6b8a75404a1509a7f Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:04:04 +0200 Subject: [PATCH] chore: restack visual evidence on compact consolidation parent --- references/visual-evidence.md | 48 ++++++++++ scripts/lib/review-scope.mjs | 4 + scripts/lib/visual-evidence.mjs | 91 +++++++++++++++++++ .../review-scope-visual-evidence.test.mjs | 41 +++++++++ tests/unit/visual-evidence.test.mjs | 78 ++++++++++++++++ 5 files changed, 262 insertions(+) create mode 100644 references/visual-evidence.md create mode 100644 scripts/lib/visual-evidence.mjs create mode 100644 tests/unit/review-scope-visual-evidence.test.mjs create mode 100644 tests/unit/visual-evidence.test.mjs diff --git a/references/visual-evidence.md b/references/visual-evidence.md new file mode 100644 index 00000000..058d176f --- /dev/null +++ b/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. diff --git a/scripts/lib/review-scope.mjs b/scripts/lib/review-scope.mjs index bb90f0e4..82eb5e0d 100644 --- a/scripts/lib/review-scope.mjs +++ b/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."] : []), ], }; } diff --git a/scripts/lib/visual-evidence.mjs b/scripts/lib/visual-evidence.mjs new file mode 100644 index 00000000..342a8add --- /dev/null +++ b/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=| + (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: [] }; +} diff --git a/tests/unit/review-scope-visual-evidence.test.mjs b/tests/unit/review-scope-visual-evidence.test.mjs new file mode 100644 index 00000000..2085722d --- /dev/null +++ b/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
;\n+return ;", + 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); +}); diff --git a/tests/unit/visual-evidence.test.mjs b/tests/unit/visual-evidence.test.mjs new file mode 100644 index 00000000..3694c4079 --- /dev/null +++ b/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 ;" }]); + 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: [] }); +});