From f4efa23a5d90e26abebcc797c8cf18bee16b99af Mon Sep 17 00:00:00 2001 From: Aaron Perez Date: Wed, 1 Jul 2026 19:42:43 -0400 Subject: [PATCH] SKY-11607: Make screenshots easier to find in run UI (#6985) --- .../routes/workflows/studio/runProjections.ts | 2 + .../studio/runview/HeroScreenshot.tsx | 5 +- .../workflows/studio/runview/RunHero.test.tsx | 67 ++++++++ .../workflows/studio/runview/RunHero.tsx | 95 ++++++++-- .../workflows/studio/runview/RunView.test.tsx | 162 +++++++++++++++++- .../workflows/studio/runview/RunView.tsx | 39 ++++- skyvern-frontend/src/store/RunViewStore.ts | 16 +- 7 files changed, 360 insertions(+), 26 deletions(-) diff --git a/skyvern-frontend/src/routes/workflows/studio/runProjections.ts b/skyvern-frontend/src/routes/workflows/studio/runProjections.ts index db31cf984..2d684e485 100644 --- a/skyvern-frontend/src/routes/workflows/studio/runProjections.ts +++ b/skyvern-frontend/src/routes/workflows/studio/runProjections.ts @@ -73,6 +73,7 @@ export type FilmstripFrame = { label: string; status: Status; blockType: string | null; + screenshotArtifactId: string | null; stepId: string | null; actionOrder: number | null; }; @@ -119,6 +120,7 @@ export function buildFilmstrip( label: actionLabel(action), status: action.status, blockType: block.block_type ?? null, + screenshotArtifactId: action.screenshot_artifact_id ?? null, stepId: action.step_id, actionOrder: action.action_order, }); diff --git a/skyvern-frontend/src/routes/workflows/studio/runview/HeroScreenshot.tsx b/skyvern-frontend/src/routes/workflows/studio/runview/HeroScreenshot.tsx index 311dfe241..3c069027c 100644 --- a/skyvern-frontend/src/routes/workflows/studio/runview/HeroScreenshot.tsx +++ b/skyvern-frontend/src/routes/workflows/studio/runview/HeroScreenshot.tsx @@ -66,7 +66,10 @@ export function HeroScreenshot({ // Fallback path only when the action carries no explicit screenshot id: the // step's action screenshots, indexed by action order (newest-first), like legacy. - const useStepFallback = Boolean(action?.stepId) && !action?.artifactId; + const useStepFallback = + Boolean(action?.stepId) && + action?.actionOrder != null && + !action?.artifactId; const { data: stepArtifacts, isLoading: loadingStep } = useQuery< Array >({ diff --git a/skyvern-frontend/src/routes/workflows/studio/runview/RunHero.test.tsx b/skyvern-frontend/src/routes/workflows/studio/runview/RunHero.test.tsx index 67e6cba19..988c02366 100644 --- a/skyvern-frontend/src/routes/workflows/studio/runview/RunHero.test.tsx +++ b/skyvern-frontend/src/routes/workflows/studio/runview/RunHero.test.tsx @@ -77,6 +77,73 @@ describe("RunHero Code surface (single source of truth)", () => { }); }); +describe("RunHero screenshot discoverability", () => { + const propsWithScreenshots = { + ...baseProps, + showDebugStream: false, + running: false, + heroSelection: { + kind: "action" as const, + artifactId: "art_1", + stepId: "step_1", + actionOrder: 0, + }, + heroLabel: "Clicked submit", + hasScreenshots: true, + }; + + test("completed runs with recordings expose a Screenshots return path", () => { + render( + , + ); + + expect(screen.queryByTestId("hero-recording")).not.toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: "Screenshots" })); + expect(screen.queryByTestId("hero-screenshot")).not.toBeNull(); + expect(screen.queryByTestId("hero-recording")).toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: "Code" })); + expect(screen.queryByTestId("workflow-run-code")).not.toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: "Screenshots" })); + expect(screen.queryByTestId("hero-screenshot")).not.toBeNull(); + expect(screen.queryByTestId("workflow-run-code")).toBeNull(); + }); + + test("running runs can return from live view to available screenshots", () => { + render(); + + expect(screen.queryByTestId("run-live-stream")).not.toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: "Screenshots" })); + expect(screen.queryByTestId("hero-screenshot")).not.toBeNull(); + expect(screen.queryByTestId("run-live-stream")).toBeNull(); + }); + + test("does not infer the Screenshots toggle from a screenshot-less selection", () => { + render( + , + ); + + expect(screen.queryByRole("button", { name: "Screenshots" })).toBeNull(); + }); +}); + describe("RunHero header dedupe", () => { test("recording view does not echo the active toggle label in the header", () => { render( diff --git a/skyvern-frontend/src/routes/workflows/studio/runview/RunHero.tsx b/skyvern-frontend/src/routes/workflows/studio/runview/RunHero.tsx index 95b27553f..b2aa2c9e7 100644 --- a/skyvern-frontend/src/routes/workflows/studio/runview/RunHero.tsx +++ b/skyvern-frontend/src/routes/workflows/studio/runview/RunHero.tsx @@ -6,6 +6,7 @@ import { ExclamationTriangleIcon, FileTextIcon, GlobeIcon, + ImageIcon, ListBulletIcon, PlayIcon, ReloadIcon, @@ -13,7 +14,7 @@ import { import { Button } from "@/components/ui/button"; import { StreamStatusPanel } from "@/routes/streaming/StreamDiagnostics"; -import { useRunViewStore } from "@/store/RunViewStore"; +import { type RunCenterView, useRunViewStore } from "@/store/RunViewStore"; import { cn } from "@/util/utils"; import { WorkflowRunCode } from "../../workflowRun/WorkflowRunCode"; @@ -37,6 +38,7 @@ type RunHeroProps = { codeGenerating?: boolean; browserSessionId: string | null; recordingUrls: string[]; + hasScreenshots?: boolean; elapsed: string; inputs?: ReactNode; outputs?: ReactNode; @@ -58,6 +60,57 @@ type CenterView = // Below this header width the toggle/dropdown labels collapse to icons. const HEADER_COMPACT_BELOW_PX = 640; +// RunCenterView stores user intent; the hero renders screenshots in one surface. +function resolveCenterView({ + centerView, + hasScreenshots, + hasInputs, + hasOutputs, + scrubbing, + showDebugStream, + recordingOpen, + hasRecording, + running, + failed, +}: { + centerView: RunCenterView; + hasScreenshots: boolean; + hasInputs: boolean; + hasOutputs: boolean; + scrubbing: boolean; + showDebugStream: boolean; + recordingOpen: boolean; + hasRecording: boolean; + running: boolean; + failed: boolean; +}): CenterView { + if (centerView === "screenshots" && hasScreenshots) { + return "screenshot"; + } + if (centerView === "code") { + return "code"; + } + if (centerView === "inputs" && hasInputs) { + return "inputs"; + } + if (centerView === "outputs" && hasOutputs) { + return "outputs"; + } + if (scrubbing) { + return "screenshot"; + } + if (showDebugStream) { + return recordingOpen && hasRecording ? "recording" : "stream"; + } + if (running) { + return "stream"; + } + if (hasRecording && !failed) { + return "recording"; + } + return "screenshot"; +} + function ViewToggle({ active, onClick, @@ -107,6 +160,7 @@ export function RunHero({ codeGenerating = false, browserSessionId, recordingUrls, + hasScreenshots = false, elapsed, inputs, outputs, @@ -159,24 +213,18 @@ export function RunHero({ // An explicit tab wins; otherwise a block run defaults to the live debug // stream and a full run to its stream (running) / recording / screenshot. - const center: CenterView = - centerView === "code" - ? "code" - : centerView === "inputs" && inputs - ? "inputs" - : centerView === "outputs" && outputs - ? "outputs" - : scrubbing - ? "screenshot" - : showDebugStream - ? recordingOpen && hasRecording - ? "recording" - : "stream" - : running - ? "stream" - : hasRecording && !failed - ? "recording" - : "screenshot"; + const center = resolveCenterView({ + centerView, + hasScreenshots, + hasInputs: Boolean(inputs), + hasOutputs: Boolean(outputs), + scrubbing, + showDebugStream, + recordingOpen, + hasRecording, + running, + failed, + }); // The right-side toggles own the view identity (Live/Recording/Code) and the // in-card "Inspecting step" bar owns a scrubbed action's description; the @@ -249,6 +297,15 @@ export function RunHero({ icon={} /> ) : null} + {hasScreenshots ? ( + setCenterView("screenshots")} + compact={compact} + label="Screenshots" + icon={} + /> + ) : null} toggleCenter("code")} diff --git a/skyvern-frontend/src/routes/workflows/studio/runview/RunView.test.tsx b/skyvern-frontend/src/routes/workflows/studio/runview/RunView.test.tsx index e3930d664..736332bd5 100644 --- a/skyvern-frontend/src/routes/workflows/studio/runview/RunView.test.tsx +++ b/skyvern-frontend/src/routes/workflows/studio/runview/RunView.test.tsx @@ -1,11 +1,17 @@ // @vitest-environment jsdom -import { cleanup, fireEvent, render, within } from "@testing-library/react"; +import { + act, + cleanup, + fireEvent, + render, + within, +} from "@testing-library/react"; import { MemoryRouter } from "react-router-dom"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { type ReactNode } from "react"; -import { Status } from "@/api/types"; +import { ActionTypes, Status, type ActionsApiResponse } from "@/api/types"; import { useRunViewStore } from "@/store/RunViewStore"; import type { WorkflowRunBlock, @@ -16,6 +22,7 @@ import { RunView } from "./RunView"; const mocks = vi.hoisted(() => ({ workflowRun: undefined as unknown, timeline: undefined as unknown, + runHeroProps: [] as Array>, })); vi.mock("../../hooks/useWorkflowRunWithWorkflowQuery", () => ({ @@ -36,7 +43,12 @@ vi.mock("../../hooks/useDebugSessionQuery", () => ({ vi.mock("../../editor/hooks/useIsGeneratingCode", () => ({ useIsGeneratingCode: () => false, })); -vi.mock("./RunHero", () => ({ RunHero: () =>
})); +vi.mock("./RunHero", () => ({ + RunHero: (props: Record) => { + mocks.runHeroProps.push(props); + return
; + }, +})); vi.mock("../../workflowRun/WorkflowRunVerificationCodeForm", () => ({ WorkflowRunVerificationCodeForm: () => null, })); @@ -102,6 +114,23 @@ function buildBlockItem( }; } +function buildAction( + overrides: Partial = {}, +): ActionsApiResponse { + return { + action_id: "act_default", + action_type: ActionTypes.Click, + status: Status.Completed, + intention: null, + description: null, + reasoning: null, + step_id: "step_default", + action_order: 0, + screenshot_artifact_id: null, + ...overrides, + } as ActionsApiResponse; +} + function seedForLoopRun() { // current_index 0 keeps the header's fallback chip on "Iteration 1" so the // timeline's "Iteration 2" row is the only "Iteration 2" before selection. @@ -147,6 +176,7 @@ afterEach(() => { cleanup(); mocks.workflowRun = undefined; mocks.timeline = undefined; + mocks.runHeroProps = []; }); beforeEach(() => useRunViewStore.getState().reset()); @@ -177,3 +207,129 @@ describe("RunView iteration selection", () => { expect(scope.queryByText("Iteration 2 value")).toBeNull(); }); }); + +describe("RunView screenshot center view", () => { + test("selects the latest action frame when a running run opens Screenshots", () => { + mocks.timeline = [ + buildBlockItem( + buildBlock({ + workflow_run_block_id: "wrb_1", + actions: [ + buildAction({ + action_id: "act_1", + action_order: 0, + screenshot_artifact_id: "art_1", + }), + ], + }), + ), + ]; + mocks.workflowRun = { + workflow_run_id: "wr_1", + status: Status.Running, + workflow: { + workflow_definition: { blocks: [], finally_block_label: null }, + }, + }; + + render( + + + , + ); + + const props = mocks.runHeroProps[mocks.runHeroProps.length - 1]; + expect(props?.heroSelection).toBeNull(); + + act(() => { + useRunViewStore.getState().pinFrame("stream"); + useRunViewStore.getState().setCenterView("screenshots"); + }); + + const screenshotProps = mocks.runHeroProps[mocks.runHeroProps.length - 1]; + expect(screenshotProps?.hasScreenshots).toBe(true); + expect(screenshotProps?.heroSelection).toMatchObject({ + kind: "action", + artifactId: "art_1", + stepId: "step_default", + actionOrder: 0, + }); + }); + + test("does not advertise screenshots for actions without screenshot candidates", () => { + mocks.timeline = [ + buildBlockItem( + buildBlock({ + workflow_run_block_id: "wrb_1", + actions: [ + buildAction({ + action_id: "act_1", + step_id: null, + screenshot_artifact_id: null, + }), + ], + }), + ), + ]; + mocks.workflowRun = { + workflow_run_id: "wr_1", + status: Status.Completed, + workflow: { + workflow_definition: { blocks: [], finally_block_label: null }, + }, + }; + + render( + + + , + ); + + const props = mocks.runHeroProps[mocks.runHeroProps.length - 1]; + expect(props?.hasScreenshots).toBe(false); + expect(props?.heroSelection).toMatchObject({ + kind: "action", + artifactId: null, + stepId: null, + }); + }); + + test("does not advertise step screenshots without an action order", () => { + mocks.timeline = [ + buildBlockItem( + buildBlock({ + workflow_run_block_id: "wrb_1", + actions: [ + buildAction({ + action_id: "act_1", + action_order: null, + screenshot_artifact_id: null, + }), + ], + }), + ), + ]; + mocks.workflowRun = { + workflow_run_id: "wr_1", + status: Status.Completed, + workflow: { + workflow_definition: { blocks: [], finally_block_label: null }, + }, + }; + + render( + + + , + ); + + const props = mocks.runHeroProps[mocks.runHeroProps.length - 1]; + expect(props?.hasScreenshots).toBe(false); + expect(props?.heroSelection).toMatchObject({ + kind: "action", + artifactId: null, + stepId: "step_default", + actionOrder: null, + }); + }); +}); diff --git a/skyvern-frontend/src/routes/workflows/studio/runview/RunView.tsx b/skyvern-frontend/src/routes/workflows/studio/runview/RunView.tsx index 666992027..896cdb951 100644 --- a/skyvern-frontend/src/routes/workflows/studio/runview/RunView.tsx +++ b/skyvern-frontend/src/routes/workflows/studio/runview/RunView.tsx @@ -58,6 +58,19 @@ type RunViewProps = { onRetry?: () => void; }; +function hasScreenshotCandidate(selection: HeroSelection | null): boolean { + if (!selection) { + return false; + } + if (selection.kind === "action") { + return Boolean( + selection.artifactId || + (selection.stepId && selection.actionOrder != null), + ); + } + return true; +} + /** * Fused run view: browser hero on the left, run timeline tree + block detail on * the right, sharing one pinned item via RunViewStore. @@ -91,6 +104,7 @@ export function RunView({ enabled: false, }); const pinnedFrameId = useRunViewStore((s) => s.pinnedFrameId); + const centerView = useRunViewStore((s) => s.centerView); const pinFrame = useRunViewStore((s) => s.pinFrame); const resetRunView = useRunViewStore((s) => s.reset); const headerCompact = useRunViewStore((s) => s.headerCompact); @@ -214,12 +228,15 @@ export function RunView({ ); const lastFrame = frames.length > 0 ? frames[frames.length - 1] : null; + const showingScreenshots = centerView === "screenshots"; const finalized = workflowRun ? statusIsFinalized(workflowRun) : false; const finallyBlockLabel = workflowRun?.workflow?.workflow_definition?.finally_block_label ?? null; const selectedId = - pinnedFrameId ?? (running ? "stream" : lastFrame?.id) ?? null; + pinnedFrameId ?? + (running && !showingScreenshots ? "stream" : lastFrame?.id) ?? + null; const activeItem = useMemo( () => findActiveItem(timeline ?? [], selectedId, finalized, finallyBlockLabel), @@ -258,6 +275,25 @@ export function RunView({ } return null; }, [activeItem, timeline, activeIteration]); + const hasScreenshotFrame = useMemo( + () => + frames.some( + (frame) => + frame.screenshotArtifactId != null || + (frame.stepId != null && frame.actionOrder != null), + ), + [frames], + ); + const activeSelectionHasScreenshot = useMemo( + () => hasScreenshotCandidate(heroSelection), + [heroSelection], + ); + // Frames are the usual source; the selection keeps the toggle visible while + // filmstrip data is still catching up. + const hasScreenshots = useMemo( + () => hasScreenshotFrame || activeSelectionHasScreenshot, + [hasScreenshotFrame, activeSelectionHasScreenshot], + ); const heroLabel = isAction(activeItem) ? actionLabel(activeItem) @@ -357,6 +393,7 @@ export function RunView({ codeGenerating={codeGenerating} browserSessionId={workflowRun.browser_session_id ?? null} recordingUrls={recordingUrls} + hasScreenshots={hasScreenshots} elapsed={elapsed} overview={ ((set) => ({ // Inspecting a frame or jumping to live/recording always drops any override. pinFrame: (id) => set({ pinnedFrameId: id, centerView: "default" }), jumpToLive: () => set({ pinnedFrameId: null, centerView: "default" }), - setCenterView: (view) => set({ centerView: view }), + setCenterView: (view) => + set((state) => ({ + centerView: view, + pinnedFrameId: + view === "screenshots" && state.pinnedFrameId === "stream" + ? null + : state.pinnedFrameId, + })), setHeaderCompact: (compact) => set({ headerCompact: compact }), reset: () => set({ pinnedFrameId: null, centerView: "default" }), }));