mirror of
https://github.com/Skyvern-AI/skyvern.git
synced 2026-07-09 16:09:13 +00:00
SKY-11607: Make screenshots easier to find in run UI (#6985)
This commit is contained in:
parent
dd88233151
commit
f4efa23a5d
7 changed files with 360 additions and 26 deletions
|
|
@ -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,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<ArtifactApiResponse>
|
||||
>({
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
<RunHero
|
||||
{...propsWithScreenshots}
|
||||
recordingUrls={["https://example.com/rec.mp4"]}
|
||||
/>,
|
||||
);
|
||||
|
||||
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(<RunHero {...propsWithScreenshots} running />);
|
||||
|
||||
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(
|
||||
<RunHero
|
||||
{...baseProps}
|
||||
showDebugStream={false}
|
||||
running={false}
|
||||
heroSelection={{
|
||||
kind: "action",
|
||||
artifactId: null,
|
||||
stepId: null,
|
||||
actionOrder: 0,
|
||||
}}
|
||||
heroLabel="Clicked submit"
|
||||
/>,
|
||||
);
|
||||
|
||||
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(
|
||||
|
|
|
|||
|
|
@ -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={<PlayIcon className="h-3 w-3" />}
|
||||
/>
|
||||
) : null}
|
||||
{hasScreenshots ? (
|
||||
<ViewToggle
|
||||
active={center === "screenshot"}
|
||||
onClick={() => setCenterView("screenshots")}
|
||||
compact={compact}
|
||||
label="Screenshots"
|
||||
icon={<ImageIcon className="h-3 w-3" />}
|
||||
/>
|
||||
) : null}
|
||||
<ViewToggle
|
||||
active={center === "code"}
|
||||
onClick={() => toggleCenter("code")}
|
||||
|
|
|
|||
|
|
@ -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<Record<string, unknown>>,
|
||||
}));
|
||||
|
||||
vi.mock("../../hooks/useWorkflowRunWithWorkflowQuery", () => ({
|
||||
|
|
@ -36,7 +43,12 @@ vi.mock("../../hooks/useDebugSessionQuery", () => ({
|
|||
vi.mock("../../editor/hooks/useIsGeneratingCode", () => ({
|
||||
useIsGeneratingCode: () => false,
|
||||
}));
|
||||
vi.mock("./RunHero", () => ({ RunHero: () => <div data-testid="run-hero" /> }));
|
||||
vi.mock("./RunHero", () => ({
|
||||
RunHero: (props: Record<string, unknown>) => {
|
||||
mocks.runHeroProps.push(props);
|
||||
return <div data-testid="run-hero" />;
|
||||
},
|
||||
}));
|
||||
vi.mock("../../workflowRun/WorkflowRunVerificationCodeForm", () => ({
|
||||
WorkflowRunVerificationCodeForm: () => null,
|
||||
}));
|
||||
|
|
@ -102,6 +114,23 @@ function buildBlockItem(
|
|||
};
|
||||
}
|
||||
|
||||
function buildAction(
|
||||
overrides: Partial<ActionsApiResponse> = {},
|
||||
): 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(
|
||||
<MemoryRouter>
|
||||
<RunView workflowRunId="wr_1" />
|
||||
</MemoryRouter>,
|
||||
);
|
||||
|
||||
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(
|
||||
<MemoryRouter>
|
||||
<RunView workflowRunId="wr_1" />
|
||||
</MemoryRouter>,
|
||||
);
|
||||
|
||||
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(
|
||||
<MemoryRouter>
|
||||
<RunView workflowRunId="wr_1" />
|
||||
</MemoryRouter>,
|
||||
);
|
||||
|
||||
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,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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={
|
||||
<RunOverviewButton
|
||||
|
|
|
|||
|
|
@ -2,7 +2,12 @@ import { create } from "zustand";
|
|||
|
||||
// Which tab takes over the hero center. "default" leaves the live stream /
|
||||
// recording / screenshot logic in charge; the rest are explicit center tabs.
|
||||
export type RunCenterView = "default" | "code" | "inputs" | "outputs";
|
||||
export type RunCenterView =
|
||||
| "default"
|
||||
| "screenshots"
|
||||
| "code"
|
||||
| "inputs"
|
||||
| "outputs";
|
||||
|
||||
type RunViewState = {
|
||||
// The frame the user is inspecting. null means "follow the live edge" while
|
||||
|
|
@ -26,7 +31,14 @@ export const useRunViewStore = create<RunViewState>((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" }),
|
||||
}));
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue