diff --git a/.loopover.yml.example b/.loopover.yml.example index 0cb6f8d433..277d60434b 100644 --- a/.loopover.yml.example +++ b/.loopover.yml.example @@ -705,6 +705,14 @@ review: # drag_to: ".done-column" # Drag DESTINATION selector. Required when action is "drag"; an entry # # missing it is dropped at parse time. Ignored for hover/click. # label: "Reorder card into Done" +# # Zero-configuration alternative to interactions above: capture a hover-interaction GIF for any CSS +# # selector THIS PR'S OWN DIFF newly adds a `:hover`/`:focus-visible` rule for -- no maintainer selector- +# # authoring needed at all. Scoped to plain .css/.scss/.sass/.less files (a Tailwind utility class or +# # CSS-in-JS `:hover` state has no selector to extract this way). Composes with interactions above (a +# # manually-configured selector always wins over the same one auto-detected); both draw from the SAME +# # 3-per-PR cap. Bool. Default: false (byte-identical to today, no auto-detection). SELF-HOST ONLY (same +# # gate as interactions/gif above). +# auto_detect_interactions: true # # Config-as-code enable/disable for this repo, layered ON TOP OF (never a replacement for) the # # LOOPOVER_REVIEW_SCREENSHOTS + per-repo cutover-allowlist env-var gate above (#4083). Bool or null. # # Default: null (unset) ⇒ defers entirely to that env-var gate's own decision -- byte-identical to today. diff --git a/config/examples/loopover.full.yml b/config/examples/loopover.full.yml index d3456c5872..7060c96bdd 100644 --- a/config/examples/loopover.full.yml +++ b/config/examples/loopover.full.yml @@ -719,6 +719,14 @@ review: # drag_to: ".done-column" # Drag DESTINATION selector. Required when action is "drag"; an entry # # missing it is dropped at parse time. Ignored for hover/click. # label: "Reorder card into Done" +# # Zero-configuration alternative to interactions above: capture a hover-interaction GIF for any CSS +# # selector THIS PR'S OWN DIFF newly adds a `:hover`/`:focus-visible` rule for -- no maintainer selector- +# # authoring needed at all. Scoped to plain .css/.scss/.sass/.less files (a Tailwind utility class or +# # CSS-in-JS `:hover` state has no selector to extract this way). Composes with interactions above (a +# # manually-configured selector always wins over the same one auto-detected); both draw from the SAME +# # 3-per-PR cap. Bool. Default: false (byte-identical to today, no auto-detection). SELF-HOST ONLY (same +# # gate as interactions/gif above). +# auto_detect_interactions: true # # Config-as-code enable/disable for this repo, layered ON TOP OF (never a replacement for) the # # LOOPOVER_REVIEW_SCREENSHOTS + per-repo cutover-allowlist env-var gate above (#4083). Bool or null. # # Default: null (unset) ⇒ defers entirely to that env-var gate's own decision -- byte-identical to today. diff --git a/packages/loopover-engine/src/focus-manifest.ts b/packages/loopover-engine/src/focus-manifest.ts index cc1bbddd93..02aebe9447 100644 --- a/packages/loopover-engine/src/focus-manifest.ts +++ b/packages/loopover-engine/src/focus-manifest.ts @@ -1016,6 +1016,12 @@ export type VisualConfig = { * (hover/click) — for behavior a static screenshot can't show that isn't scroll-linked (see `gif` above * for scroll-linked evidence). Empty (default) ⇒ byte-identical to today, no interaction capture. */ interactions: VisualInteraction[]; + /** `review.visual.autoDetectInteractions` (#auto-interaction-detection): capture a hover-interaction GIF + * for any CSS selector this PR's OWN diff newly adds a `:hover`/`:focus-visible` rule for — zero + * maintainer selector-authoring required, unlike `interactions` above (still available for a hand- + * curated demonstration; the two compose, deduped against each other). false (default) ⇒ byte-identical + * to today. Self-host only, same gate as `interactions`/`gif` (isScrollGifAvailable). */ + autoDetectInteractions: boolean; }; /** A `prefers-color-scheme` value the capture pipeline can emulate before rendering (#3678). */ @@ -1088,6 +1094,7 @@ export const EMPTY_VISUAL_CONFIG: VisualConfig = { bugAnalysis: false, bugAnalysisNotify: [], interactions: [], + autoDetectInteractions: false, }; /** One `review.path_instructions[]` entry: a manifest path glob + the public-safe instructions to apply when a @@ -2996,6 +3003,7 @@ function overlayVisualConfig(base: VisualConfig, override: VisualConfig): Visual bugAnalysis: override.bugAnalysis ? override.bugAnalysis : base.bugAnalysis, bugAnalysisNotify: pickOverlayStringList(override.bugAnalysisNotify, base.bugAnalysisNotify), interactions: override.interactions.length > 0 ? [...override.interactions] : [...base.interactions], + autoDetectInteractions: override.autoDetectInteractions ? override.autoDetectInteractions : base.autoDetectInteractions, }; } @@ -3203,7 +3211,8 @@ function visualConfigPresent(config: VisualConfig): boolean { config.actionsFallback || config.bugAnalysis || config.bugAnalysisNotify.length > 0 || - config.interactions.length > 0 + config.interactions.length > 0 || + config.autoDetectInteractions ); } @@ -3307,8 +3316,9 @@ function parseVisualConfig(value: JsonValue | undefined, warnings: string[]): Vi const bugAnalysis = normalizeOptionalBoolean(record.bug_analysis, "review.visual.bug_analysis", warnings) === true; const bugAnalysisNotify = parseVisualBugAnalysisNotify(record.bug_analysis_notify, warnings); const interactions = parseVisualInteractions(record.interactions, warnings); + const autoDetectInteractions = normalizeOptionalBoolean(record.auto_detect_interactions, "review.visual.auto_detect_interactions", warnings) === true; - return { productionUrl, preview: { urlTemplate }, routes: { paths, maxRoutes }, themes, gif, enabled, themeStorageKey, actionsFallback, bugAnalysis, bugAnalysisNotify, interactions }; + return { productionUrl, preview: { urlTemplate }, routes: { paths, maxRoutes }, themes, gif, enabled, themeStorageKey, actionsFallback, bugAnalysis, bugAnalysisNotify, interactions, autoDetectInteractions }; } // A hard cap so a hostile/huge manifest can't turn every PR close into a giant @-mention blast — mirrors @@ -3721,6 +3731,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue return entry; }); } + if (review.visual.autoDetectInteractions) visual.auto_detect_interactions = true; out.visual = visual; } if (review.linkedIssueSatisfaction !== null) out.linkedIssueSatisfaction = review.linkedIssueSatisfaction; diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 78b9a0c12b..3a5d5ac985 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -10384,6 +10384,13 @@ async function maybePublishPrPublicSurface( const visualFiles = unifiedFiles .map((file) => file.path) .filter(isVisualPath); + // #auto-interaction-detection: only ever read by buildCapture when review.visual.autoDetectInteractions + // is on for this repo -- carries each changed file's own diff patch text (visualFiles above is bare + // paths), the same file.payload?.patch shape review-diff.ts/grounding-wire.ts already read elsewhere. + const changedCssFiles = unifiedFiles.map((file) => ({ + path: file.path, + patch: typeof file.payload?.patch === "string" ? file.payload.patch : undefined, + })); if (resolveConvergedFeature(env, repoFocusManifestForComment, "screenshots", repoFullName) && visualFiles.length > 0) { try { const token = await createInstallationToken(env, installationId); @@ -10410,7 +10417,7 @@ async function maybePublishPrPublicSurface( const capture = reviewVisualConfig.enabled === false ? { routes: [], interactions: [], previewPending: false } - : await buildCapture(env, token, captureTarget, visualFiles, githubRateLimitAdmissionKeyForInstallation(installationId), reviewVisualConfig); + : await buildCapture(env, token, captureTarget, visualFiles, githubRateLimitAdmissionKeyForInstallation(installationId), reviewVisualConfig, changedCssFiles); beforeAfter = capture.routes; interactionPreviews = capture.interactions; // Screenshot-table gate satisfaction (#4110): a successful capture (a real before+after render pair diff --git a/src/review/visual/capture.ts b/src/review/visual/capture.ts index a13743ac3e..44cc2f0007 100644 --- a/src/review/visual/capture.ts +++ b/src/review/visual/capture.ts @@ -31,6 +31,7 @@ import { import { captureInteractionFrames, captureScrollFrames, captureShot, DESKTOP_VIEWPORT, MOBILE_VIEWPORT, type InteractionAction, type ShotTheme, type Viewport } from "./shot"; import { compareCapturedScreenshots, isVisualDiffAvailable, type VisualDiffOutcome } from "./pixel-diff"; import { encodeScrollGif, isScrollGifAvailable } from "./scroll-gif"; +import { detectAutoHoverInteractions, type ChangedCssFile } from "./interaction-detection"; const NAMESPACE = "loopover"; const DEFAULT_ROUTES = ["/"]; @@ -608,6 +609,12 @@ export type VisualCaptureConfig = { * ⇒ byte-identical to today, no interaction capture. Capped at MAX_INTERACTIONS regardless of how many * are configured. */ interactions?: readonly VisualInteractionInput[] | null | undefined; + /** `review.visual.autoDetectInteractions` (#auto-interaction-detection): capture a hover-interaction GIF + * for any CSS selector this PR's OWN diff newly adds a `:hover`/`:focus-visible` rule for — no maintainer + * selector-authoring needed, unlike `interactions` above (the two compose, deduped against each other). + * false/absent (default) ⇒ byte-identical to today. Requires `changedCssFiles` (below) to be passed too; + * without it there is nothing to detect against regardless of this flag. */ + autoDetectInteractions?: boolean | null | undefined; }; /** @@ -616,7 +623,19 @@ export type VisualCaptureConfig = { * collapsible). Fully fail-safe — a missing preview / failed render degrades to placeholders or dashes; this * NEVER throws (the caller also wraps it in try/catch so a capture failure can't sink a review). */ -export async function buildCapture(env: Env, token: string, target: CaptureTarget, visualFiles: string[], rateLimitAdmissionKey?: GitHubRateLimitAdmissionKey | undefined, visualConfig?: VisualCaptureConfig | null | undefined): Promise { +export async function buildCapture( + env: Env, + token: string, + target: CaptureTarget, + visualFiles: string[], + rateLimitAdmissionKey?: GitHubRateLimitAdmissionKey | undefined, + visualConfig?: VisualCaptureConfig | null | undefined, + // #auto-interaction-detection: the SAME changed-file set visualFiles is derived from, but carrying each + // file's diff patch text too (visualFiles alone is bare paths) -- only ever read when + // visualConfig.autoDetectInteractions is true. Absent/undefined (every pre-existing call site) ⇒ + // byte-identical to today, no auto-detection attempted regardless of the config flag. + changedCssFiles?: readonly ChangedCssFile[] | undefined, +): Promise { const repo = parseRepo(target.repoFullName); const apiVersion = "2022-11-28"; // before = production. review.visual.production_url (#3611 follow-up) ALWAYS wins when set -- PUBLIC_SITE_ORIGIN @@ -791,7 +810,29 @@ export async function buildCapture(env: Env, token: string, target: CaptureTarge // interaction target" shape. Gated on isScrollGifAvailable() (reused: the encode step is frame-source- // agnostic, see scroll-gif.ts) since there is no point capturing frames this build can never assemble into // a GIF -- self-host only, same as the scroll-GIF path above. - const interactionsConfigured = (visualConfig?.interactions ?? []).slice(0, MAX_INTERACTIONS); + const manualInteractions = visualConfig?.interactions ?? []; + // #auto-interaction-detection: a maintainer-configured selector always wins on overlap -- an explicit + // entry may carry a label/path/action the detector could never infer, so a hand-authored duplicate is + // dropped from the auto-detected set rather than the other way around. Both selector sets are compared + // case-insensitively, matching detectAutoHoverInteractions' own dedup. + const manualSelectors = new Set(manualInteractions.map((interaction) => interaction.selector.toLowerCase())); + const autoDetectedInteractions: VisualInteractionInput[] = + visualConfig?.autoDetectInteractions && changedCssFiles + ? detectAutoHoverInteractions(changedCssFiles) + .filter((selector) => !manualSelectors.has(selector.toLowerCase())) + .map((selector) => ({ + selector, + action: "hover" as const, + // captureRoutes[0] is unreachable-undefined by construction here, not a reachable false case: + // `themes` above is always at least `[undefined]` and `routes` (resolveVisualRoutes -> + // mapFilesToRoutes) always falls back to DEFAULT_ROUTES when nothing else resolves, so the + // routes x themes double loop above always pushes at least one entry -- noUncheckedIndexedAccess + // still requires the optional chaining at the type level. + /* v8 ignore next */ + path: captureRoutes[0]?.path ?? null, + })) + : []; + const interactionsConfigured = [...manualInteractions, ...autoDetectedInteractions].slice(0, MAX_INTERACTIONS); const interactionRoutes: CaptureInteractionRoute[] = []; // Interactions aren't multiplied per-theme (see comment above) -- when review.visual.themes configures more // than one, the first configured theme is what interaction GIFs render in; themes[0] is `undefined` by diff --git a/src/review/visual/interaction-detection.ts b/src/review/visual/interaction-detection.ts new file mode 100644 index 0000000000..d8bef1bc05 --- /dev/null +++ b/src/review/visual/interaction-detection.ts @@ -0,0 +1,73 @@ +// Automatic hover-interaction detection from CSS diffs (#auto-interaction-detection). PURE, no DB/network — +// mirrors visual-findings.ts's own "pure decision logic only" convention. The whole point of +// review.visual.interactions (capture.ts / shot.ts) was originally a maintainer hand-authoring CSS selectors +// ahead of time; that still exists for a maintainer-curated demonstration, but requires foreknowledge of +// what's interactive and worth showing. This module is the zero-configuration alternative: read the PR's own +// diff for a newly ADDED `:hover`/`:focus-visible` CSS rule and capture ITS selector automatically — no +// maintainer selector-authoring step at all. Scoped to plain CSS/SCSS/SASS/LESS stylesheets (the only case a +// selector is syntactically explicit in the diff text); a Tailwind utility class or CSS-in-JS `:hover` state +// has no equivalent selector to extract this way and is out of scope here. + +/** One changed file's path + unified-diff patch text — the same `file.payload?.patch` shape every other + * diff-reading module in this codebase already uses (review-diff.ts, grounding-wire.ts, ...). `patch` + * absent (a binary file, or a diff GitHub didn't include) ⇒ that file contributes no selectors. */ +export type ChangedCssFile = { path: string; patch?: string | undefined }; + +const CSS_FILE_EXTENSIONS = [".css", ".scss", ".sass", ".less"]; + +// Mirrors capture.ts's MAX_INTERACTIONS reasoning: bounds how many auto-detected selectors this module ever +// returns, independent of how many `:hover`/`:focus-visible` rules a large stylesheet diff actually touches. +const MAX_AUTO_DETECTED_INTERACTIONS = 3; +// A selector this long is either a hostile/malformed diff line or a compound rule not worth interacting with +// (e.g. an entire multi-selector block) — mirrors focus-manifest.ts's MAX_ITEM_LENGTH-style bound. +const MAX_SELECTOR_LENGTH = 300; + +// Matches a unified-diff ADDED line (`+`-prefixed, not the `+++` file-header line) whose CSS rule selector +// ends in `:hover` or `:focus-visible`, immediately followed by optional whitespace and the rule's opening +// `{`. Capturing only ADDED lines is deliberate: an EXISTING :hover rule this PR never touched says nothing +// about what changed, and would fire this feature on every single PR that merely touches a stylesheet. +const HOVER_SELECTOR_LINE_PATTERN = /^\+(?!\+\+)\s*([^{}\n]+?):(?:hover|focus-visible)\s*\{/; + +function isCssFile(path: string): boolean { + const lower = path.toLowerCase(); + return CSS_FILE_EXTENSIONS.some((ext) => lower.endsWith(ext)); +} + +/** The regex's own capture group spans from the line start to the LAST `:hover`/`:focus-visible` it found + * (non-greedy backtracking) — for a comma-separated selector LIST (`.a:hover, .b:hover { ... }`), that + * swallows every earlier selector's OWN `:hover` mid-string too (`.a:hover, .b`), not just `.b`. Since the + * match only anchors on the FINAL `:hover`/`:focus-visible` in the list, the text after the last comma is + * always the one real selector that rule actually matched against — take that, discarding the earlier + * list entries this capture can't cleanly separate rather than returning a mangled, unusable string. */ +function lastSelectorInList(capturedGroup: string): string { + const lastCommaIndex = capturedGroup.lastIndexOf(","); + return (lastCommaIndex === -1 ? capturedGroup : capturedGroup.slice(lastCommaIndex + 1)).trim(); +} + +/** + * Detect newly-added `:hover`/`:focus-visible` CSS selectors across `files`' diff patches, capped at + * {@link MAX_AUTO_DETECTED_INTERACTIONS} and deduped case-insensitively. Selectors are returned in + * first-seen order (the order their files appear in `files`, then line order within each patch) — the + * caller decides what page/theme to capture them against. An unparseable/absent patch, a non-CSS file, or a + * selector exceeding {@link MAX_SELECTOR_LENGTH} contributes nothing; this NEVER throws. + */ +export function detectAutoHoverInteractions(files: readonly ChangedCssFile[]): string[] { + const selectors: string[] = []; + const seen = new Set(); + for (const file of files) { + if (selectors.length >= MAX_AUTO_DETECTED_INTERACTIONS) break; + if (!isCssFile(file.path) || !file.patch) continue; + for (const line of file.patch.split("\n")) { + if (selectors.length >= MAX_AUTO_DETECTED_INTERACTIONS) break; + const match = HOVER_SELECTOR_LINE_PATTERN.exec(line); + if (!match) continue; + const selector = lastSelectorInList(match[1]!); + if (!selector || selector.length > MAX_SELECTOR_LENGTH) continue; + const key = selector.toLowerCase(); + if (seen.has(key)) continue; + seen.add(key); + selectors.push(selector); + } + } + return selectors; +} diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 76bd61d070..8f895817b6 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -4865,6 +4865,7 @@ describe("review.visual (#3609 preview.url_template / #3610 routes)", () => { bugAnalysis: false, bugAnalysisNotify: [], interactions: [], + autoDetectInteractions: false, }); expect(m.review.present).toBe(true); expect(parseFocusManifest({ review: reviewConfigToJson(m.review) }).review.visual).toEqual(m.review.visual); @@ -4960,7 +4961,7 @@ describe("review.visual (#3609 preview.url_template / #3610 routes)", () => { it("resolveReviewVisualConfig: null manifest yields empty defaults; a set manifest passes through", () => { expect(resolveReviewVisualConfig(null)).toEqual({ ...EMPTY_VISUAL_CONFIG }); const manifest = parseFocusManifest({ review: { visual: { routes: { paths: ["/app"] } } } }); - expect(resolveReviewVisualConfig(manifest)).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: ["/app"], maxRoutes: null }, themes: [], gif: false, enabled: null, themeStorageKey: null, actionsFallback: false, bugAnalysis: false, bugAnalysisNotify: [], interactions: [] }); + expect(resolveReviewVisualConfig(manifest)).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: ["/app"], maxRoutes: null }, themes: [], gif: false, enabled: null, themeStorageKey: null, actionsFallback: false, bugAnalysis: false, bugAnalysisNotify: [], interactions: [], autoDetectInteractions: false }); }); }); @@ -5106,7 +5107,7 @@ describe("review.visual.gif (#3612 scroll-through GIF capture)", () => { it("composes with themes — both configured independently and both round-trip", () => { const m = parseFocusManifest({ review: { visual: { gif: true, themes: ["dark"] } } }); - expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: ["dark"], gif: true, enabled: null, themeStorageKey: null, actionsFallback: false, bugAnalysis: false, bugAnalysisNotify: [], interactions: [] }); + expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: ["dark"], gif: true, enabled: null, themeStorageKey: null, actionsFallback: false, bugAnalysis: false, bugAnalysisNotify: [], interactions: [], autoDetectInteractions: false }); expect(reviewConfigToJson(m.review)).toEqual({ visual: { themes: ["dark"], gif: true } }); }); @@ -5209,7 +5210,7 @@ describe("review.visual.theme_storage_key (#4109 localStorage theme-forcing fall it("composes with themes — both configured independently and both round-trip", () => { const m = parseFocusManifest({ review: { visual: { themes: ["dark"], theme_storage_key: "theme" } } }); - expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: ["dark"], gif: false, enabled: null, themeStorageKey: "theme", actionsFallback: false, bugAnalysis: false, bugAnalysisNotify: [], interactions: [] }); + expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: ["dark"], gif: false, enabled: null, themeStorageKey: "theme", actionsFallback: false, bugAnalysis: false, bugAnalysisNotify: [], interactions: [], autoDetectInteractions: false }); expect(reviewConfigToJson(m.review)).toEqual({ visual: { themes: ["dark"], theme_storage_key: "theme" } }); }); @@ -5264,7 +5265,7 @@ describe("review.visual.actions_fallback (#4112 GitHub-Actions build-and-serve f it("composes with gif — both configured independently and both round-trip", () => { const m = parseFocusManifest({ review: { visual: { actions_fallback: true, gif: true } } }); - expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: true, enabled: null, themeStorageKey: null, actionsFallback: true, bugAnalysis: false, bugAnalysisNotify: [], interactions: [] }); + expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: true, enabled: null, themeStorageKey: null, actionsFallback: true, bugAnalysis: false, bugAnalysisNotify: [], interactions: [], autoDetectInteractions: false }); expect(reviewConfigToJson(m.review)).toEqual({ visual: { gif: true, actions_fallback: true } }); }); @@ -5319,7 +5320,7 @@ describe("review.visual.bugAnalysis (PR-intent-aware vision + out-of-scope issue it("composes with gif — both configured independently and both round-trip", () => { const m = parseFocusManifest({ review: { visual: { bug_analysis: true, gif: true } } }); - expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: true, enabled: null, themeStorageKey: null, actionsFallback: false, bugAnalysis: true, bugAnalysisNotify: [], interactions: [] }); + expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: true, enabled: null, themeStorageKey: null, actionsFallback: false, bugAnalysis: true, bugAnalysisNotify: [], interactions: [], autoDetectInteractions: false }); expect(reviewConfigToJson(m.review)).toEqual({ visual: { gif: true, bug_analysis: true } }); }); @@ -5501,6 +5502,59 @@ describe("review.visual.interactions (#interaction-gif-capture)", () => { }); }); +describe("review.visual.autoDetectInteractions (#auto-interaction-detection)", () => { + it("parses auto_detect_interactions: true, marks present, and round-trips", () => { + const m = parseFocusManifest({ review: { visual: { auto_detect_interactions: true } } }); + expect(m.review.visual.autoDetectInteractions).toBe(true); + expect(m.review.present).toBe(true); + expect(reviewConfigToJson(m.review)).toEqual({ visual: { auto_detect_interactions: true } }); + }); + + it("absent auto_detect_interactions defaults to false and does not mark review present on its own", () => { + expect(parseFocusManifest({}).review.visual.autoDetectInteractions).toBe(false); + expect(parseFocusManifest({ review: { visual: {} } }).review.present).toBe(false); + }); + + it("auto_detect_interactions: false does not mark review present, so the whole review block round-trips to null", () => { + const m = parseFocusManifest({ review: { visual: { auto_detect_interactions: false } } }); + expect(m.review.visual.autoDetectInteractions).toBe(false); + expect(reviewConfigToJson(m.review)).toBeNull(); + }); + + it("warns and defaults to false when auto_detect_interactions is not a boolean", () => { + const bad = parseFocusManifest({ review: { visual: { auto_detect_interactions: "yes" } } }); + expect(bad.review.visual.autoDetectInteractions).toBe(false); + expect(bad.warnings.some((w) => /review\.visual\.auto_detect_interactions.*must be a boolean/.test(w))).toBe(true); + }); + + it("composes with a manually-configured interactions list — both round-trip independently", () => { + const m = parseFocusManifest({ review: { visual: { auto_detect_interactions: true, interactions: [{ selector: ".x", action: "hover" }] } } }); + expect(m.review.visual.autoDetectInteractions).toBe(true); + expect(m.review.visual.interactions).toEqual([{ selector: ".x", action: "hover", dragTo: null, path: null, label: null }]); + expect(reviewConfigToJson(m.review)).toEqual({ + visual: { interactions: [{ selector: ".x", action: "hover" }], auto_detect_interactions: true }, + }); + }); + + it("resolveReviewVisualConfig passes a configured auto_detect_interactions: true through", () => { + const manifest = parseFocusManifest({ review: { visual: { auto_detect_interactions: true } } }); + expect(resolveReviewVisualConfig(manifest).autoDetectInteractions).toBe(true); + }); + + it("overlay: a per-repo auto_detect_interactions: true wins over a global-default false", () => { + const globalDefault = parseReviewConfigMapping({ visual: { auto_detect_interactions: false } }, []); + const perRepo = parseReviewConfigMapping({ visual: { auto_detect_interactions: true } }, []); + expect(overlayReviewConfig(globalDefault, perRepo).visual.autoDetectInteractions).toBe(true); + }); + + it("overlay: an unset per-repo auto_detect_interactions falls back to the global-default true — this is how the operator turns it on fleet-wide from the global-default .loopover.yml", () => { + const globalDefault = parseReviewConfigMapping({ visual: { auto_detect_interactions: true } }, []); + const perRepo = parseReviewConfigMapping({ visual: { routes: { paths: ["/app"] } } }, []); + expect(overlayReviewConfig(globalDefault, perRepo).visual.autoDetectInteractions).toBe(true); + expect(overlayReviewConfig(globalDefault, perRepo).visual.routes.paths).toEqual(["/app"]); + }); +}); + describe("review.pre_merge_checks (#review-pre-merge-checks)", () => { it("parses checks (name + assertions + when_paths + enforce), marks present, and round-trips", () => { const m = parseFocusManifest({ diff --git a/test/unit/interaction-detection.test.ts b/test/unit/interaction-detection.test.ts new file mode 100644 index 0000000000..b3613555f6 --- /dev/null +++ b/test/unit/interaction-detection.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from "vitest"; +import { detectAutoHoverInteractions } from "../../src/review/visual/interaction-detection"; + +describe("detectAutoHoverInteractions (#auto-interaction-detection)", () => { + it("detects a newly-added :hover rule's selector from a .css file", () => { + const patch = "@@ -1,3 +1,4 @@\n .foo {\n color: red;\n }\n+.blocks-row:hover {\n+ background: blue;\n+}\n"; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([".blocks-row"]); + }); + + it("detects :focus-visible the same way as :hover", () => { + const patch = "+button:focus-visible {\n+ outline: 2px solid blue;\n+}\n"; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual(["button"]); + }); + + it("returns [] for a non-CSS file, even with an identical-looking patch", () => { + const patch = "+.blocks-row:hover {\n+ background: blue;\n+}\n"; + expect(detectAutoHoverInteractions([{ path: "src/component.tsx", patch }])).toEqual([]); + }); + + it("returns [] when the file has no patch at all (binary file / GitHub omitted it)", () => { + expect(detectAutoHoverInteractions([{ path: "src/styles.css" }])).toEqual([]); + }); + + it("ignores a REMOVED :hover rule (a '-'-prefixed line) — this PR didn't add it", () => { + const patch = "-.old-hover:hover {\n- color: red;\n-}\n"; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([]); + }); + + it("ignores an unchanged context line (no +/- prefix)", () => { + const patch = " .unchanged-hover:hover {\n color: red;\n }\n"; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([]); + }); + + it("does not mistake the '+++' unified-diff file-header line for an added line", () => { + const patch = "+++ b/src/styles.css\n+.real-addition:hover {\n+ color: red;\n+}\n"; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([".real-addition"]); + }); + + it("dedupes the same selector seen twice, case-insensitively, keeping the first casing", () => { + const patch = "+.Card:hover {\n+ color: red;\n+}\n+.card:hover {\n+ color: blue;\n+}\n"; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([".Card"]); + }); + + it("caps at 3 selectors even when a stylesheet diff adds more", () => { + const patch = Array.from({ length: 5 }, (_, i) => `+.item-${i}:hover {\n+ color: red;\n+}`).join("\n"); + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([".item-0", ".item-1", ".item-2"]); + }); + + it("preserves file order, then line order within each file, up to the cap", () => { + const files = [ + { path: "a.css", patch: "+.a:hover {\n+ color: red;\n+}" }, + { path: "b.css", patch: "+.b:hover {\n+ color: red;\n+}" }, + ]; + expect(detectAutoHoverInteractions(files)).toEqual([".a", ".b"]); + }); + + it("stops scanning FILES entirely (not just lines) once the cap is already reached by an earlier file", () => { + const files = [ + { path: "a.css", patch: "+.a:hover {\n+ color: red;\n+}\n+.b:hover {\n+ color: red;\n+}\n+.c:hover {\n+ color: red;\n+}" }, + { path: "b.css", patch: "+.d:hover {\n+ color: red;\n+}" }, + ]; + expect(detectAutoHoverInteractions(files)).toEqual([".a", ".b", ".c"]); + }); + + it("drops a selector exceeding the length bound rather than including it", () => { + const longSelector = `.${"x".repeat(400)}`; + const patch = `+${longSelector}:hover {\n+ color: red;\n+}`; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([]); + }); + + it("captures a compound selector chain (multiple classes/combinators) intact", () => { + const patch = "+.nav-item > a.link:hover {\n+ color: red;\n+}"; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([".nav-item > a.link"]); + }); + + it("extracts only the LAST selector from a comma-separated hover rule, discarding the earlier ones cleanly", () => { + const patch = "+.foo:hover, .bar:hover {\n+ color: red;\n+}"; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([".bar"]); + }); + + it("extracts the last selector from a THREE-entry comma-separated list", () => { + const patch = "+.a:hover, .b:hover, .c:hover {\n+ color: red;\n+}"; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([".c"]); + }); + + it("recognizes .scss, .sass, and .less files the same way as .css", () => { + for (const ext of [".scss", ".sass", ".less"]) { + const patch = "+.hover-target:hover {\n+ color: red;\n+}"; + expect(detectAutoHoverInteractions([{ path: `src/styles${ext}`, patch }])).toEqual([".hover-target"]); + } + }); + + it("is case-insensitive on the file extension itself", () => { + const patch = "+.hover-target:hover {\n+ color: red;\n+}"; + expect(detectAutoHoverInteractions([{ path: "src/Styles.CSS", patch }])).toEqual([".hover-target"]); + }); + + it("matches an indented rule inside a nested block (e.g. a media query)", () => { + const patch = "+@media (min-width: 768px) {\n+ .responsive-hover:hover {\n+ color: red;\n+ }\n+}"; + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([".responsive-hover"]); + }); + + it("returns [] for an empty files list", () => { + expect(detectAutoHoverInteractions([])).toEqual([]); + }); + + it("never throws on a patch with no matching rules at all", () => { + const patch = "+.foo {\n+ color: red;\n+}\n-.bar {\n- color: blue;\n-}"; + expect(() => detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).not.toThrow(); + expect(detectAutoHoverInteractions([{ path: "src/styles.css", patch }])).toEqual([]); + }); +}); diff --git a/test/unit/visual-capture.test.ts b/test/unit/visual-capture.test.ts index 23b34622d1..ae2ef81e79 100644 --- a/test/unit/visual-capture.test.ts +++ b/test/unit/visual-capture.test.ts @@ -2272,6 +2272,206 @@ describe("buildCapture interaction-GIF wiring (#interaction-gif-capture)", () => }); }); +describe("buildCapture auto-detected interaction wiring (#auto-interaction-detection)", () => { + const hoverPatch = "+.blocks-row:hover {\n+ background: blue;\n+}"; + + it("captures an auto-detected hover selector when auto_detect_interactions is on and changedCssFiles carries a new :hover rule", async () => { + const gifAvailableSpy = vi.spyOn(scrollGifModule, "isScrollGifAvailable").mockReturnValue(true); + const captureInteractionSpy = vi.spyOn(shotModule, "captureInteractionFrames").mockResolvedValue({ + frames: [new Uint8Array([1, 2, 3])], + authWalled: false, + }); + const encodeSpy = vi.spyOn(scrollGifModule, "encodeScrollGif").mockResolvedValue(new Uint8Array([7, 8, 9])); + try { + const env = createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "https://prod.example.com", REVIEW_AUDIT: memoryReviewAudit() }); + const result = await buildCapture( + env, + "installation-token", + { repoFullName: "owner/repo", prNumber: 71, previewUrl: "https://preview.example.com" }, + ["apps/loopover-ui/src/styles.css"], + undefined, + { autoDetectInteractions: true }, + [{ path: "apps/loopover-ui/src/styles.css", patch: hoverPatch }], + ); + expect(result.interactions).toHaveLength(1); + expect(result.interactions[0]?.selector).toBe(".blocks-row"); + } finally { + gifAvailableSpy.mockRestore(); + captureInteractionSpy.mockRestore(); + encodeSpy.mockRestore(); + } + }); + + it("never auto-detects when auto_detect_interactions is unset, even with a qualifying CSS diff", async () => { + const gifAvailableSpy = vi.spyOn(scrollGifModule, "isScrollGifAvailable").mockReturnValue(true); + const captureInteractionSpy = vi.spyOn(shotModule, "captureInteractionFrames"); + try { + const result = await buildCapture( + createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "https://prod.example.com" }), + "installation-token", + { repoFullName: "owner/repo", prNumber: 72, previewUrl: "https://preview.example.com" }, + ["apps/loopover-ui/src/styles.css"], + undefined, + undefined, + [{ path: "apps/loopover-ui/src/styles.css", patch: hoverPatch }], + ); + expect(captureInteractionSpy).not.toHaveBeenCalled(); + expect(result.interactions).toEqual([]); + } finally { + gifAvailableSpy.mockRestore(); + captureInteractionSpy.mockRestore(); + } + }); + + it("never auto-detects when auto_detect_interactions is on but changedCssFiles is omitted entirely", async () => { + const gifAvailableSpy = vi.spyOn(scrollGifModule, "isScrollGifAvailable").mockReturnValue(true); + const captureInteractionSpy = vi.spyOn(shotModule, "captureInteractionFrames"); + try { + const result = await buildCapture( + createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "https://prod.example.com" }), + "installation-token", + { repoFullName: "owner/repo", prNumber: 73, previewUrl: "https://preview.example.com" }, + ["apps/loopover-ui/src/styles.css"], + undefined, + { autoDetectInteractions: true }, + ); + expect(captureInteractionSpy).not.toHaveBeenCalled(); + expect(result.interactions).toEqual([]); + } finally { + gifAvailableSpy.mockRestore(); + captureInteractionSpy.mockRestore(); + } + }); + + it("targets the PR's own first captured route for an auto-detected selector, not '/'", async () => { + const gifAvailableSpy = vi.spyOn(scrollGifModule, "isScrollGifAvailable").mockReturnValue(true); + const captureInteractionSpy = vi.spyOn(shotModule, "captureInteractionFrames").mockResolvedValue({ + frames: [new Uint8Array([1, 2, 3])], + authWalled: false, + }); + const encodeSpy = vi.spyOn(scrollGifModule, "encodeScrollGif").mockResolvedValue(new Uint8Array([7, 8, 9])); + try { + const env = createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "https://prod.example.com", REVIEW_AUDIT: memoryReviewAudit() }); + await buildCapture( + env, + "installation-token", + { repoFullName: "owner/repo", prNumber: 74, previewUrl: "https://preview.example.com" }, + ["apps/loopover-ui/src/routes/pricing.tsx"], + undefined, + { autoDetectInteractions: true, routes: { paths: ["/pricing"] } }, + [{ path: "apps/loopover-ui/src/styles.css", patch: hoverPatch }], + ); + expect(captureInteractionSpy).toHaveBeenCalledWith(env, "https://prod.example.com/pricing", ".blocks-row", "hover", expect.anything(), {}, undefined); + } finally { + gifAvailableSpy.mockRestore(); + captureInteractionSpy.mockRestore(); + encodeSpy.mockRestore(); + } + }); + + it("a manually-configured selector wins over the SAME selector auto-detected, without duplicating it", async () => { + const gifAvailableSpy = vi.spyOn(scrollGifModule, "isScrollGifAvailable").mockReturnValue(true); + const captureInteractionSpy = vi.spyOn(shotModule, "captureInteractionFrames").mockResolvedValue({ + frames: [new Uint8Array([1, 2, 3])], + authWalled: false, + }); + const encodeSpy = vi.spyOn(scrollGifModule, "encodeScrollGif").mockResolvedValue(new Uint8Array([7, 8, 9])); + try { + const env = createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "https://prod.example.com", REVIEW_AUDIT: memoryReviewAudit() }); + const result = await buildCapture( + env, + "installation-token", + { repoFullName: "owner/repo", prNumber: 75, previewUrl: "https://preview.example.com" }, + ["apps/loopover-ui/src/styles.css"], + undefined, + { autoDetectInteractions: true, interactions: [{ selector: ".blocks-row", action: "hover", label: "Blocks row hover" }] }, + [{ path: "apps/loopover-ui/src/styles.css", patch: hoverPatch }], + ); + expect(result.interactions).toHaveLength(1); + expect(result.interactions[0]?.label).toBe("Blocks row hover"); + } finally { + gifAvailableSpy.mockRestore(); + captureInteractionSpy.mockRestore(); + encodeSpy.mockRestore(); + } + }); + + it("a manually-configured selector list still leaves room for a DIFFERENT auto-detected selector, up to the cap", async () => { + const gifAvailableSpy = vi.spyOn(scrollGifModule, "isScrollGifAvailable").mockReturnValue(true); + const captureInteractionSpy = vi.spyOn(shotModule, "captureInteractionFrames").mockResolvedValue({ + frames: [new Uint8Array([1, 2, 3])], + authWalled: false, + }); + const encodeSpy = vi.spyOn(scrollGifModule, "encodeScrollGif").mockResolvedValue(new Uint8Array([7, 8, 9])); + try { + const env = createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "https://prod.example.com", REVIEW_AUDIT: memoryReviewAudit() }); + const result = await buildCapture( + env, + "installation-token", + { repoFullName: "owner/repo", prNumber: 76, previewUrl: "https://preview.example.com" }, + ["apps/loopover-ui/src/styles.css"], + undefined, + { autoDetectInteractions: true, interactions: [{ selector: ".manual-target", action: "hover" }] }, + [{ path: "apps/loopover-ui/src/styles.css", patch: hoverPatch }], + ); + expect(result.interactions.map((interaction) => interaction.selector).sort()).toEqual([".blocks-row", ".manual-target"]); + } finally { + gifAvailableSpy.mockRestore(); + captureInteractionSpy.mockRestore(); + encodeSpy.mockRestore(); + } + }); + + it("caps the TOTAL (manual + auto-detected) at MAX_INTERACTIONS, not each independently", async () => { + const gifAvailableSpy = vi.spyOn(scrollGifModule, "isScrollGifAvailable").mockReturnValue(true); + const captureInteractionSpy = vi.spyOn(shotModule, "captureInteractionFrames").mockResolvedValue({ + frames: [new Uint8Array([1, 2, 3])], + authWalled: false, + }); + const encodeSpy = vi.spyOn(scrollGifModule, "encodeScrollGif").mockResolvedValue(new Uint8Array([7, 8, 9])); + try { + const env = createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "https://prod.example.com", REVIEW_AUDIT: memoryReviewAudit() }); + const manyHoverPatch = "+.a:hover {\n+ color: red;\n+}\n+.b:hover {\n+ color: red;\n+}\n+.c:hover {\n+ color: red;\n+}"; + const result = await buildCapture( + env, + "installation-token", + { repoFullName: "owner/repo", prNumber: 77, previewUrl: "https://preview.example.com" }, + ["apps/loopover-ui/src/styles.css"], + undefined, + { autoDetectInteractions: true, interactions: [{ selector: ".manual-1", action: "hover" }, { selector: ".manual-2", action: "hover" }] }, + [{ path: "apps/loopover-ui/src/styles.css", patch: manyHoverPatch }], + ); + expect(result.interactions).toHaveLength(3); + expect(result.interactions.map((interaction) => interaction.selector)).toEqual([".manual-1", ".manual-2", ".a"]); + } finally { + gifAvailableSpy.mockRestore(); + captureInteractionSpy.mockRestore(); + encodeSpy.mockRestore(); + } + }); + + it("never calls captureInteractionFrames when the diff has no qualifying :hover rule at all", async () => { + const gifAvailableSpy = vi.spyOn(scrollGifModule, "isScrollGifAvailable").mockReturnValue(true); + const captureInteractionSpy = vi.spyOn(shotModule, "captureInteractionFrames"); + try { + const result = await buildCapture( + createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "https://prod.example.com" }), + "installation-token", + { repoFullName: "owner/repo", prNumber: 78, previewUrl: "https://preview.example.com" }, + ["apps/loopover-ui/src/app.tsx"], + undefined, + { autoDetectInteractions: true }, + [{ path: "apps/loopover-ui/src/app.tsx", patch: "+const x = 1;" }], + ); + expect(captureInteractionSpy).not.toHaveBeenCalled(); + expect(result.interactions).toEqual([]); + } finally { + gifAvailableSpy.mockRestore(); + captureInteractionSpy.mockRestore(); + } + }); +}); + describe("hasSuccessfulBotCapture (#4110)", () => { const REAL_BEFORE = "https://api.example/loopover/shot?key=loopover%2Fshots%2Fbefore.png"; const REAL_AFTER = "https://api.example/loopover/shot?key=loopover%2Fshots%2Fafter.png"; diff --git a/test/unit/visual-config-wiring.test.ts b/test/unit/visual-config-wiring.test.ts index 219e0e89ed..2e75b6ff60 100644 --- a/test/unit/visual-config-wiring.test.ts +++ b/test/unit/visual-config-wiring.test.ts @@ -27,6 +27,7 @@ describe("review.visual wiring (#3609 / #3610)", () => { bugAnalysis: false, bugAnalysisNotify: [], interactions: [], + autoDetectInteractions: false, }); expect(loadSpy).toHaveBeenCalledWith(expect.anything(), "acme/widgets"); loadSpy.mockRestore();