Skip to content

Commit 5b8d150

Browse files
committed
feat(visual): automatically detect hover interactions from CSS diffs
review.visual.interactions required a maintainer to hand-author CSS selectors ahead of time -- the actual goal is for relevant screenshots/GIFs to get captured automatically for any frontend/visual change, with no pre-selection step at all. Adds review.visual.auto_detect_interactions: scans a PR's own diff for a newly-added `:hover`/`:focus-visible` CSS rule (plain .css/.scss/.sass/.less only -- a Tailwind utility class or CSS-in-JS hover state has no selector to extract this way) and captures a hover-interaction GIF for it automatically, targeting the PR's own first captured route. Composes with the existing manual `interactions` list (a hand-authored selector always wins over the same one auto-detected); both draw from the same 3-per-PR cap. Default false, byte-identical to today.
1 parent 6c834ab commit 5b8d150

10 files changed

Lines changed: 525 additions & 10 deletions

File tree

.loopover.yml.example

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,14 @@ review:
705705
# drag_to: ".done-column" # Drag DESTINATION selector. Required when action is "drag"; an entry
706706
# # missing it is dropped at parse time. Ignored for hover/click.
707707
# label: "Reorder card into Done"
708+
# # Zero-configuration alternative to interactions above: capture a hover-interaction GIF for any CSS
709+
# # selector THIS PR'S OWN DIFF newly adds a `:hover`/`:focus-visible` rule for -- no maintainer selector-
710+
# # authoring needed at all. Scoped to plain .css/.scss/.sass/.less files (a Tailwind utility class or
711+
# # CSS-in-JS `:hover` state has no selector to extract this way). Composes with interactions above (a
712+
# # manually-configured selector always wins over the same one auto-detected); both draw from the SAME
713+
# # 3-per-PR cap. Bool. Default: false (byte-identical to today, no auto-detection). SELF-HOST ONLY (same
714+
# # gate as interactions/gif above).
715+
# auto_detect_interactions: true
708716
# # Config-as-code enable/disable for this repo, layered ON TOP OF (never a replacement for) the
709717
# # LOOPOVER_REVIEW_SCREENSHOTS + per-repo cutover-allowlist env-var gate above (#4083). Bool or null.
710718
# # Default: null (unset) ⇒ defers entirely to that env-var gate's own decision -- byte-identical to today.

config/examples/loopover.full.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,14 @@ review:
719719
# drag_to: ".done-column" # Drag DESTINATION selector. Required when action is "drag"; an entry
720720
# # missing it is dropped at parse time. Ignored for hover/click.
721721
# label: "Reorder card into Done"
722+
# # Zero-configuration alternative to interactions above: capture a hover-interaction GIF for any CSS
723+
# # selector THIS PR'S OWN DIFF newly adds a `:hover`/`:focus-visible` rule for -- no maintainer selector-
724+
# # authoring needed at all. Scoped to plain .css/.scss/.sass/.less files (a Tailwind utility class or
725+
# # CSS-in-JS `:hover` state has no selector to extract this way). Composes with interactions above (a
726+
# # manually-configured selector always wins over the same one auto-detected); both draw from the SAME
727+
# # 3-per-PR cap. Bool. Default: false (byte-identical to today, no auto-detection). SELF-HOST ONLY (same
728+
# # gate as interactions/gif above).
729+
# auto_detect_interactions: true
722730
# # Config-as-code enable/disable for this repo, layered ON TOP OF (never a replacement for) the
723731
# # LOOPOVER_REVIEW_SCREENSHOTS + per-repo cutover-allowlist env-var gate above (#4083). Bool or null.
724732
# # Default: null (unset) ⇒ defers entirely to that env-var gate's own decision -- byte-identical to today.

packages/loopover-engine/src/focus-manifest.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,6 +1016,12 @@ export type VisualConfig = {
10161016
* (hover/click) — for behavior a static screenshot can't show that isn't scroll-linked (see `gif` above
10171017
* for scroll-linked evidence). Empty (default) ⇒ byte-identical to today, no interaction capture. */
10181018
interactions: VisualInteraction[];
1019+
/** `review.visual.autoDetectInteractions` (#auto-interaction-detection): capture a hover-interaction GIF
1020+
* for any CSS selector this PR's OWN diff newly adds a `:hover`/`:focus-visible` rule for — zero
1021+
* maintainer selector-authoring required, unlike `interactions` above (still available for a hand-
1022+
* curated demonstration; the two compose, deduped against each other). false (default) ⇒ byte-identical
1023+
* to today. Self-host only, same gate as `interactions`/`gif` (isScrollGifAvailable). */
1024+
autoDetectInteractions: boolean;
10191025
};
10201026

10211027
/** A `prefers-color-scheme` value the capture pipeline can emulate before rendering (#3678). */
@@ -1088,6 +1094,7 @@ export const EMPTY_VISUAL_CONFIG: VisualConfig = {
10881094
bugAnalysis: false,
10891095
bugAnalysisNotify: [],
10901096
interactions: [],
1097+
autoDetectInteractions: false,
10911098
};
10921099

10931100
/** 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
29963003
bugAnalysis: override.bugAnalysis ? override.bugAnalysis : base.bugAnalysis,
29973004
bugAnalysisNotify: pickOverlayStringList(override.bugAnalysisNotify, base.bugAnalysisNotify),
29983005
interactions: override.interactions.length > 0 ? [...override.interactions] : [...base.interactions],
3006+
autoDetectInteractions: override.autoDetectInteractions ? override.autoDetectInteractions : base.autoDetectInteractions,
29993007
};
30003008
}
30013009

@@ -3203,7 +3211,8 @@ function visualConfigPresent(config: VisualConfig): boolean {
32033211
config.actionsFallback ||
32043212
config.bugAnalysis ||
32053213
config.bugAnalysisNotify.length > 0 ||
3206-
config.interactions.length > 0
3214+
config.interactions.length > 0 ||
3215+
config.autoDetectInteractions
32073216
);
32083217
}
32093218

@@ -3307,8 +3316,9 @@ function parseVisualConfig(value: JsonValue | undefined, warnings: string[]): Vi
33073316
const bugAnalysis = normalizeOptionalBoolean(record.bug_analysis, "review.visual.bug_analysis", warnings) === true;
33083317
const bugAnalysisNotify = parseVisualBugAnalysisNotify(record.bug_analysis_notify, warnings);
33093318
const interactions = parseVisualInteractions(record.interactions, warnings);
3319+
const autoDetectInteractions = normalizeOptionalBoolean(record.auto_detect_interactions, "review.visual.auto_detect_interactions", warnings) === true;
33103320

3311-
return { productionUrl, preview: { urlTemplate }, routes: { paths, maxRoutes }, themes, gif, enabled, themeStorageKey, actionsFallback, bugAnalysis, bugAnalysisNotify, interactions };
3321+
return { productionUrl, preview: { urlTemplate }, routes: { paths, maxRoutes }, themes, gif, enabled, themeStorageKey, actionsFallback, bugAnalysis, bugAnalysisNotify, interactions, autoDetectInteractions };
33123322
}
33133323

33143324
// 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
37213731
return entry;
37223732
});
37233733
}
3734+
if (review.visual.autoDetectInteractions) visual.auto_detect_interactions = true;
37243735
out.visual = visual;
37253736
}
37263737
if (review.linkedIssueSatisfaction !== null) out.linkedIssueSatisfaction = review.linkedIssueSatisfaction;

src/queue/processors.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10384,6 +10384,13 @@ async function maybePublishPrPublicSurface(
1038410384
const visualFiles = unifiedFiles
1038510385
.map((file) => file.path)
1038610386
.filter(isVisualPath);
10387+
// #auto-interaction-detection: only ever read by buildCapture when review.visual.autoDetectInteractions
10388+
// is on for this repo -- carries each changed file's own diff patch text (visualFiles above is bare
10389+
// paths), the same file.payload?.patch shape review-diff.ts/grounding-wire.ts already read elsewhere.
10390+
const changedCssFiles = unifiedFiles.map((file) => ({
10391+
path: file.path,
10392+
patch: typeof file.payload?.patch === "string" ? file.payload.patch : undefined,
10393+
}));
1038710394
if (resolveConvergedFeature(env, repoFocusManifestForComment, "screenshots", repoFullName) && visualFiles.length > 0) {
1038810395
try {
1038910396
const token = await createInstallationToken(env, installationId);
@@ -10410,7 +10417,7 @@ async function maybePublishPrPublicSurface(
1041010417
const capture =
1041110418
reviewVisualConfig.enabled === false
1041210419
? { routes: [], interactions: [], previewPending: false }
10413-
: await buildCapture(env, token, captureTarget, visualFiles, githubRateLimitAdmissionKeyForInstallation(installationId), reviewVisualConfig);
10420+
: await buildCapture(env, token, captureTarget, visualFiles, githubRateLimitAdmissionKeyForInstallation(installationId), reviewVisualConfig, changedCssFiles);
1041410421
beforeAfter = capture.routes;
1041510422
interactionPreviews = capture.interactions;
1041610423
// Screenshot-table gate satisfaction (#4110): a successful capture (a real before+after render pair

src/review/visual/capture.ts

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
import { captureInteractionFrames, captureScrollFrames, captureShot, DESKTOP_VIEWPORT, MOBILE_VIEWPORT, type InteractionAction, type ShotTheme, type Viewport } from "./shot";
3232
import { compareCapturedScreenshots, isVisualDiffAvailable, type VisualDiffOutcome } from "./pixel-diff";
3333
import { encodeScrollGif, isScrollGifAvailable } from "./scroll-gif";
34+
import { detectAutoHoverInteractions, type ChangedCssFile } from "./interaction-detection";
3435

3536
const NAMESPACE = "loopover";
3637
const DEFAULT_ROUTES = ["/"];
@@ -608,6 +609,12 @@ export type VisualCaptureConfig = {
608609
* ⇒ byte-identical to today, no interaction capture. Capped at MAX_INTERACTIONS regardless of how many
609610
* are configured. */
610611
interactions?: readonly VisualInteractionInput[] | null | undefined;
612+
/** `review.visual.autoDetectInteractions` (#auto-interaction-detection): capture a hover-interaction GIF
613+
* for any CSS selector this PR's OWN diff newly adds a `:hover`/`:focus-visible` rule for — no maintainer
614+
* selector-authoring needed, unlike `interactions` above (the two compose, deduped against each other).
615+
* false/absent (default) ⇒ byte-identical to today. Requires `changedCssFiles` (below) to be passed too;
616+
* without it there is nothing to detect against regardless of this flag. */
617+
autoDetectInteractions?: boolean | null | undefined;
611618
};
612619

613620
/**
@@ -616,7 +623,19 @@ export type VisualCaptureConfig = {
616623
* collapsible). Fully fail-safe — a missing preview / failed render degrades to placeholders or dashes; this
617624
* NEVER throws (the caller also wraps it in try/catch so a capture failure can't sink a review).
618625
*/
619-
export async function buildCapture(env: Env, token: string, target: CaptureTarget, visualFiles: string[], rateLimitAdmissionKey?: GitHubRateLimitAdmissionKey | undefined, visualConfig?: VisualCaptureConfig | null | undefined): Promise<CaptureResult> {
626+
export async function buildCapture(
627+
env: Env,
628+
token: string,
629+
target: CaptureTarget,
630+
visualFiles: string[],
631+
rateLimitAdmissionKey?: GitHubRateLimitAdmissionKey | undefined,
632+
visualConfig?: VisualCaptureConfig | null | undefined,
633+
// #auto-interaction-detection: the SAME changed-file set visualFiles is derived from, but carrying each
634+
// file's diff patch text too (visualFiles alone is bare paths) -- only ever read when
635+
// visualConfig.autoDetectInteractions is true. Absent/undefined (every pre-existing call site) ⇒
636+
// byte-identical to today, no auto-detection attempted regardless of the config flag.
637+
changedCssFiles?: readonly ChangedCssFile[] | undefined,
638+
): Promise<CaptureResult> {
620639
const repo = parseRepo(target.repoFullName);
621640
const apiVersion = "2022-11-28";
622641
// 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
791810
// interaction target" shape. Gated on isScrollGifAvailable() (reused: the encode step is frame-source-
792811
// agnostic, see scroll-gif.ts) since there is no point capturing frames this build can never assemble into
793812
// a GIF -- self-host only, same as the scroll-GIF path above.
794-
const interactionsConfigured = (visualConfig?.interactions ?? []).slice(0, MAX_INTERACTIONS);
813+
const manualInteractions = visualConfig?.interactions ?? [];
814+
// #auto-interaction-detection: a maintainer-configured selector always wins on overlap -- an explicit
815+
// entry may carry a label/path/action the detector could never infer, so a hand-authored duplicate is
816+
// dropped from the auto-detected set rather than the other way around. Both selector sets are compared
817+
// case-insensitively, matching detectAutoHoverInteractions' own dedup.
818+
const manualSelectors = new Set(manualInteractions.map((interaction) => interaction.selector.toLowerCase()));
819+
const autoDetectedInteractions: VisualInteractionInput[] =
820+
visualConfig?.autoDetectInteractions && changedCssFiles
821+
? detectAutoHoverInteractions(changedCssFiles)
822+
.filter((selector) => !manualSelectors.has(selector.toLowerCase()))
823+
.map((selector) => ({
824+
selector,
825+
action: "hover" as const,
826+
// captureRoutes[0] is unreachable-undefined by construction here, not a reachable false case:
827+
// `themes` above is always at least `[undefined]` and `routes` (resolveVisualRoutes ->
828+
// mapFilesToRoutes) always falls back to DEFAULT_ROUTES when nothing else resolves, so the
829+
// routes x themes double loop above always pushes at least one entry -- noUncheckedIndexedAccess
830+
// still requires the optional chaining at the type level.
831+
/* v8 ignore next */
832+
path: captureRoutes[0]?.path ?? null,
833+
}))
834+
: [];
835+
const interactionsConfigured = [...manualInteractions, ...autoDetectedInteractions].slice(0, MAX_INTERACTIONS);
795836
const interactionRoutes: CaptureInteractionRoute[] = [];
796837
// Interactions aren't multiplied per-theme (see comment above) -- when review.visual.themes configures more
797838
// than one, the first configured theme is what interaction GIFs render in; themes[0] is `undefined` by
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// Automatic hover-interaction detection from CSS diffs (#auto-interaction-detection). PURE, no DB/network —
2+
// mirrors visual-findings.ts's own "pure decision logic only" convention. The whole point of
3+
// review.visual.interactions (capture.ts / shot.ts) was originally a maintainer hand-authoring CSS selectors
4+
// ahead of time; that still exists for a maintainer-curated demonstration, but requires foreknowledge of
5+
// what's interactive and worth showing. This module is the zero-configuration alternative: read the PR's own
6+
// diff for a newly ADDED `:hover`/`:focus-visible` CSS rule and capture ITS selector automatically — no
7+
// maintainer selector-authoring step at all. Scoped to plain CSS/SCSS/SASS/LESS stylesheets (the only case a
8+
// selector is syntactically explicit in the diff text); a Tailwind utility class or CSS-in-JS `:hover` state
9+
// has no equivalent selector to extract this way and is out of scope here.
10+
11+
/** One changed file's path + unified-diff patch text — the same `file.payload?.patch` shape every other
12+
* diff-reading module in this codebase already uses (review-diff.ts, grounding-wire.ts, ...). `patch`
13+
* absent (a binary file, or a diff GitHub didn't include) ⇒ that file contributes no selectors. */
14+
export type ChangedCssFile = { path: string; patch?: string | undefined };
15+
16+
const CSS_FILE_EXTENSIONS = [".css", ".scss", ".sass", ".less"];
17+
18+
// Mirrors capture.ts's MAX_INTERACTIONS reasoning: bounds how many auto-detected selectors this module ever
19+
// returns, independent of how many `:hover`/`:focus-visible` rules a large stylesheet diff actually touches.
20+
const MAX_AUTO_DETECTED_INTERACTIONS = 3;
21+
// A selector this long is either a hostile/malformed diff line or a compound rule not worth interacting with
22+
// (e.g. an entire multi-selector block) — mirrors focus-manifest.ts's MAX_ITEM_LENGTH-style bound.
23+
const MAX_SELECTOR_LENGTH = 300;
24+
25+
// Matches a unified-diff ADDED line (`+`-prefixed, not the `+++` file-header line) whose CSS rule selector
26+
// ends in `:hover` or `:focus-visible`, immediately followed by optional whitespace and the rule's opening
27+
// `{`. Capturing only ADDED lines is deliberate: an EXISTING :hover rule this PR never touched says nothing
28+
// about what changed, and would fire this feature on every single PR that merely touches a stylesheet.
29+
const HOVER_SELECTOR_LINE_PATTERN = /^\+(?!\+\+)\s*([^{}\n]+?):(?:hover|focus-visible)\s*\{/;
30+
31+
function isCssFile(path: string): boolean {
32+
const lower = path.toLowerCase();
33+
return CSS_FILE_EXTENSIONS.some((ext) => lower.endsWith(ext));
34+
}
35+
36+
/** The regex's own capture group spans from the line start to the LAST `:hover`/`:focus-visible` it found
37+
* (non-greedy backtracking) — for a comma-separated selector LIST (`.a:hover, .b:hover { ... }`), that
38+
* swallows every earlier selector's OWN `:hover` mid-string too (`.a:hover, .b`), not just `.b`. Since the
39+
* match only anchors on the FINAL `:hover`/`:focus-visible` in the list, the text after the last comma is
40+
* always the one real selector that rule actually matched against — take that, discarding the earlier
41+
* list entries this capture can't cleanly separate rather than returning a mangled, unusable string. */
42+
function lastSelectorInList(capturedGroup: string): string {
43+
const lastCommaIndex = capturedGroup.lastIndexOf(",");
44+
return (lastCommaIndex === -1 ? capturedGroup : capturedGroup.slice(lastCommaIndex + 1)).trim();
45+
}
46+
47+
/**
48+
* Detect newly-added `:hover`/`:focus-visible` CSS selectors across `files`' diff patches, capped at
49+
* {@link MAX_AUTO_DETECTED_INTERACTIONS} and deduped case-insensitively. Selectors are returned in
50+
* first-seen order (the order their files appear in `files`, then line order within each patch) — the
51+
* caller decides what page/theme to capture them against. An unparseable/absent patch, a non-CSS file, or a
52+
* selector exceeding {@link MAX_SELECTOR_LENGTH} contributes nothing; this NEVER throws.
53+
*/
54+
export function detectAutoHoverInteractions(files: readonly ChangedCssFile[]): string[] {
55+
const selectors: string[] = [];
56+
const seen = new Set<string>();
57+
for (const file of files) {
58+
if (selectors.length >= MAX_AUTO_DETECTED_INTERACTIONS) break;
59+
if (!isCssFile(file.path) || !file.patch) continue;
60+
for (const line of file.patch.split("\n")) {
61+
if (selectors.length >= MAX_AUTO_DETECTED_INTERACTIONS) break;
62+
const match = HOVER_SELECTOR_LINE_PATTERN.exec(line);
63+
if (!match) continue;
64+
const selector = lastSelectorInList(match[1]!);
65+
if (!selector || selector.length > MAX_SELECTOR_LENGTH) continue;
66+
const key = selector.toLowerCase();
67+
if (seen.has(key)) continue;
68+
seen.add(key);
69+
selectors.push(selector);
70+
}
71+
}
72+
return selectors;
73+
}

0 commit comments

Comments
 (0)