From 24c58e479aeb8e2d6506eeb48d9c8bc012c9863d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:38:06 -0500 Subject: [PATCH 01/13] Improve task artifacts with rich cards and editable stories (#14469) Render eight artifact card types from real task records and share them with editable Storybook stories. Preserve document review and media/file actions, and load bounded CSV previews on request. Co-Authored-By: Paperclip --- tests/e2e/artifact-tab-arrival.spec.ts | 2 +- .../artifacts/IssueArtifactCard.test.tsx | 275 +++++++ .../artifacts/IssueArtifactCard.tsx | 277 +++++++ .../artifacts/IssueArtifactFile.test.tsx | 73 ++ .../artifacts/RichArtifactCards.tsx | 733 ++++++++++++++++++ .../IssuePropertiesArtifactsTab.tsx | 530 +++++++------ ...ssuePropertiesMarkdownWorkProduct.test.tsx | 29 +- ui/src/lib/artifact-card-data.test.ts | 127 +++ ui/src/lib/artifact-card-data.ts | 164 ++++ ui/storybook/stories/artifact-cards/README.md | 70 ++ .../stories/artifact-cards/commit.stories.tsx | 45 ++ .../stories/artifact-cards/data.stories.tsx | 47 ++ .../artifact-cards/document.stories.tsx | 43 + .../stories/artifact-cards/file.stories.tsx | 51 ++ .../stories/artifact-cards/image.stories.tsx | 45 ++ .../artifact-cards/link-preview.stories.tsx | 41 + .../artifact-cards/pull-request.stories.tsx | 68 ++ .../stories/artifact-cards/storyConfig.tsx | 51 ++ .../stories/artifact-cards/video.stories.tsx | 41 + 19 files changed, 2479 insertions(+), 233 deletions(-) create mode 100644 ui/src/components/artifacts/IssueArtifactCard.test.tsx create mode 100644 ui/src/components/artifacts/IssueArtifactCard.tsx create mode 100644 ui/src/components/artifacts/IssueArtifactFile.test.tsx create mode 100644 ui/src/components/artifacts/RichArtifactCards.tsx create mode 100644 ui/src/lib/artifact-card-data.test.ts create mode 100644 ui/src/lib/artifact-card-data.ts create mode 100644 ui/storybook/stories/artifact-cards/README.md create mode 100644 ui/storybook/stories/artifact-cards/commit.stories.tsx create mode 100644 ui/storybook/stories/artifact-cards/data.stories.tsx create mode 100644 ui/storybook/stories/artifact-cards/document.stories.tsx create mode 100644 ui/storybook/stories/artifact-cards/file.stories.tsx create mode 100644 ui/storybook/stories/artifact-cards/image.stories.tsx create mode 100644 ui/storybook/stories/artifact-cards/link-preview.stories.tsx create mode 100644 ui/storybook/stories/artifact-cards/pull-request.stories.tsx create mode 100644 ui/storybook/stories/artifact-cards/storyConfig.tsx create mode 100644 ui/storybook/stories/artifact-cards/video.stories.tsx diff --git a/tests/e2e/artifact-tab-arrival.spec.ts b/tests/e2e/artifact-tab-arrival.spec.ts index a04a54f9b3f..e4f29c6adbd 100644 --- a/tests/e2e/artifact-tab-arrival.spec.ts +++ b/tests/e2e/artifact-tab-arrival.spec.ts @@ -50,7 +50,7 @@ for (const mobile of [false, true]) { await expect(artifacts).toBeVisible(); await expect(artifacts).toHaveAttribute("aria-selected", "false"); await artifacts.click(); - await expect(panel.getByText("Arriving report", { exact: true })).toBeVisible(); + await expect(panel.getByRole("heading", { name: "Arriving report", level: 2, exact: true })).toBeVisible(); await page.screenshot({ path: testInfo.outputPath("artifact-opened-by-user.png"), fullPage: true }); }); } diff --git a/ui/src/components/artifacts/IssueArtifactCard.test.tsx b/ui/src/components/artifacts/IssueArtifactCard.test.tsx new file mode 100644 index 00000000000..75402929e38 --- /dev/null +++ b/ui/src/components/artifacts/IssueArtifactCard.test.tsx @@ -0,0 +1,275 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vitest"; +import type { IssueWorkProduct } from "@paperclipai/shared"; +import { + IssueArtifactFile, + IssueWorkProductArtifactCard, +} from "./IssueArtifactCard"; + +import { DocumentCard } from "./RichArtifactCards"; + +function product(overrides: Partial = {}): IssueWorkProduct { + return { + id: "wp-1", + companyId: "company-1", + projectId: null, + issueId: "issue-1", + executionWorkspaceId: null, + runtimeServiceId: null, + type: "pull_request", + provider: "github", + externalId: null, + title: "Actual artifact title", + url: "https://github.com/org/repo/pull/42", + status: "active", + reviewState: "none", + isPrimary: false, + healthStatus: "unknown", + summary: "Actual artifact summary", + metadata: null, + createdByRunId: null, + createdAt: new Date("2026-09-28"), + updatedAt: new Date("2026-09-28"), + ...overrides, + }; +} +function render(wp: IssueWorkProduct) { + return renderToStaticMarkup( + + + , + ); +} + +describe("production artifact cards", () => { + it("renders supplied PR metadata without synthesizing missing counts, checks or branch names", () => { + const sparse = render(product()); + expect(sparse).toContain("Actual artifact title"); + expect(sparse).toContain("Actual artifact summary"); + expect(sparse).toContain("Actual agent"); + expect(sparse).toContain("Checks not available"); + expect(sparse).not.toContain("Checks passed"); + expect(sparse).not.toContain("+0"); + expect(sparse).not.toContain("#0"); + expect(sparse).not.toContain("master"); + const full = render( + product({ + metadata: { + number: 42, + repo: "org/repo", + state: "draft", + additions: 0, + deletions: 8, + changedFiles: 2, + headRef: "feature", + baseRef: "main", + }, + }), + ); + for (const value of [ + "#42", + "org/repo", + "Draft", + "+0", + "−8", + "feature", + "main", + ]) + expect(full).toContain(value); + }); + it("preserves review and unhealthy states", () => { + expect( + render(product({ reviewState: "changes_requested", status: "merged" })), + ).toContain("Changes requested"); + expect( + render(product({ type: "preview_url", healthStatus: "unhealthy" })), + ).toContain("Unhealthy"); + expect( + render(product({ type: "preview_url", healthStatus: "unknown" })), + ).not.toContain("Healthy"); + }); + it.each([ + ["ready_for_review", "Review"], + ["changes_requested", "Changes requested"], + ])( + "preserves PR status %s without a separate reviewState", + (status, label) => { + expect(render(product({ status, reviewState: "none" }))).toContain(label); + }, + ); + + it("preserves browser-open actions for attachments and signed external file URLs", () => { + const signedUrl = + "https://files.example/report.pdf?signature=abc&expires=123"; + const external = render( + product({ + type: "artifact", + url: signedUrl, + metadata: { contentType: "application/pdf" }, + }), + ); + expect(external).toContain( + 'href="https://files.example/report.pdf?signature=abc&expires=123"', + ); + expect(external).not.toContain("?download=1"); + expect(external).toContain("Open file"); + expect(external).toContain("Download file"); + + const local = renderToStaticMarkup( + + + , + ); + expect(local).toContain( + 'href="/api/attachments/pdf-1/content" target="_blank"', + ); + expect(local).toContain( + 'href="/api/attachments/pdf-1/content?download=1" download="report.pdf"', + ); + }); + + it("selects commit, link, image, video and file renderers from real records", () => { + expect( + render( + product({ + type: "commit", + metadata: { sha: "123456789abcdef", repo: "org/repo" }, + }), + ), + ).toContain("12345678"); + expect(render(product({ type: "preview_url" }))).toContain("Open link"); + const attachment = { + contentPath: "/api/attachments/file-1/content", + byteSize: 25, + }; + expect( + render( + product({ + type: "artifact", + metadata: { + ...attachment, + contentType: "image/png", + originalFilename: "actual.png", + }, + }), + ), + ).toContain("View image"); + const video = render( + product({ + type: "artifact", + metadata: { + ...attachment, + contentType: "video/mp4", + originalFilename: "actual.mp4", + }, + }), + ); + expect(video).toContain(" { + const client = new QueryClient(); + client.setQueryData( + ["artifact-csv", "csv-1", "/api/attachments/csv-1/content", "Today"], + { columns: ["Region"], rows: [["Actual region"]], truncated: false }, + ); + const html = renderToStaticMarkup( + + + , + ); + expect(html).toContain("Actual region"); + expect(html).toContain("View data"); + }); + it("does not fetch remote Markdown images just to render a document preview", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).not.toContain('src="https://tracker.example'); + expect(html).toContain('href="https://tracker.example/image.png"'); + expect(html).toContain('src="/api/attachments/image-1/content"'); + }); + it("does not automatically load remote thumbnail or poster metadata", () => { + const link = render( + product({ + type: "preview_url", + metadata: { imageUrl: "https://tracker.example/pixel.png" }, + }), + ); + expect(link).not.toContain(" { + expect(render(product({ type: "branch" }))).toContain("Open on GitHub"); + expect( + render(product({ type: "runtime_service", healthStatus: "unhealthy" })), + ).toContain("Unhealthy"); + expect( + render(product({ type: "preview_url", url: "javascript:alert(1)" })), + ).not.toContain("javascript:"); + }); +}); diff --git a/ui/src/components/artifacts/IssueArtifactCard.tsx b/ui/src/components/artifacts/IssueArtifactCard.tsx new file mode 100644 index 00000000000..90118a87d70 --- /dev/null +++ b/ui/src/components/artifacts/IssueArtifactCard.tsx @@ -0,0 +1,277 @@ +import { useContext, useState } from "react"; +import { useQuery } from "@tanstack/react-query"; +import { + getAttachmentArtifactWorkProductMetadata, + type IssueWorkProduct, +} from "@paperclipai/shared"; +import { IssueGalleryContext } from "@/context/IssueGalleryContext"; +import { ImageGalleryModal } from "@/components/ImageGalleryModal"; +import { + RichWorkProductCard, + stateChipFor, +} from "@/components/task-chat/RichWorkProductCard"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { isImageLikeOutput, isVideoLikeOutput } from "@/lib/issue-output"; +import { attachmentDownloadPath } from "@/lib/issue-attachments"; +import { workProductHref } from "@/lib/issue-artifacts"; +import { formatDateTime } from "@/lib/utils"; +import { + artifactText as text, + artifactNumber as number, + artifactUrl, + artifactPreviewUrl, + artifactFileSize, + CSV_PREVIEW_MAX_BYTES, + loadArtifactCsv, +} from "@/lib/artifact-card-data"; +import { + CommitCard, + DataCard, + FileCard, + ImageCard, + LinkPreviewCard, + PullRequestCard, + VideoCard, + type ArtifactIdentity, +} from "./RichArtifactCards"; + +export interface IssueArtifactFileProps extends ArtifactIdentity { + id: string; + filename: string; + contentType: string; + contentPath: string; + downloadPath: string; + openPath?: string; + byteSize: number | null; + metadata?: Record | null; +} + +/** Shared by uploads and promoted uploads; both use the issue's existing gallery. */ +export function IssueArtifactFile(props: IssueArtifactFileProps) { + const { metadata = null } = props; + const openGallery = useContext(IssueGalleryContext); + const [galleryOpen, setGalleryOpen] = useState(false); + const [csvRequested, setCsvRequested] = useState(false); + const image = isImageLikeOutput(props.contentType, props.filename); + const video = isVideoLikeOutput(props.contentType, props.filename); + const csv = + props.contentType.split(";")[0] === "text/csv" || + /\.csv$/i.test(props.filename); + const tooLarge = + props.byteSize !== null && props.byteSize > CSV_PREVIEW_MAX_BYTES; + const localCsv = + csv && + /^\/api\/attachments\/[a-zA-Z0-9-]+\/content$/.test(props.contentPath); + const data = useQuery({ + queryKey: ["artifact-csv", props.id, props.contentPath, props.updatedAt], + queryFn: ({ signal }) => loadArtifactCsv(props.contentPath, signal), + enabled: localCsv && !tooLarge && csvRequested, + retry: false, + staleTime: Infinity, + }); + const contentPath = artifactUrl(props.contentPath); + const downloadPath = artifactUrl(props.downloadPath); + const onOpen = () => { + if (!openGallery?.(contentPath)) setGalleryOpen(true); + }; + if ((image || video) && contentPath) { + return ( + <> + {video ? ( + + ) : ( + + )} + {galleryOpen && ( + + )} + + ); + } + if (localCsv && !tooLarge && data.data) + return ; + return ( +
+ setCsvRequested(true)} + > + {data.isFetching ? "Loading preview…" : "Preview data"} + + ) : undefined + } + /> + {csv && (tooLarge || data.isError || !localCsv) && ( +
+ {tooLarge + ? "CSV is too large to preview. Download the file to view it." + : data.isError + ? data.error.message + : "CSV preview is unavailable. Download the file to view it."} + {data.isError && ( + + )} +
+ )} +
+ ); +} + +export function IssueWorkProductArtifactCard({ + workProduct: wp, + author, +}: { + workProduct: IssueWorkProduct; + author: string; +}) { + const m = wp.metadata; + const href = artifactUrl(workProductHref(wp)); + const chip = stateChipFor(wp.type, wp.status, wp.reviewState); + const identity: ArtifactIdentity = { + title: wp.title, + summary: wp.summary ?? "", + author, + updatedAt: formatDateTime(wp.updatedAt), + statusBadge: + wp.healthStatus === "unhealthy" ? ( + Unhealthy + ) : chip ? ( + {chip.label} + ) : undefined, + }; + const diff = { + additions: number(m, "additions"), + deletions: number(m, "deletions"), + filesChanged: number(m, "changedFiles", "files"), + }; + if (wp.type === "pull_request") { + const state = + text(m, "state") || (wp.status === "active" ? "open" : wp.status); + const checks = text(m, "checks"); + return ( + + ); + } + if (wp.type === "commit") + return ( + + ); + if (wp.type === "preview_url") + return ( + + ); + if (wp.type === "artifact") { + const attachment = getAttachmentArtifactWorkProductMetadata(wp); + const contentPath = artifactUrl( + attachment?.contentPath || text(m, "contentPath") || href, + ); + return ( + + ); + } + // Branches, external documents, and runtime services keep their existing actions and health semantics. + return ; +} diff --git a/ui/src/components/artifacts/IssueArtifactFile.test.tsx b/ui/src/components/artifacts/IssueArtifactFile.test.tsx new file mode 100644 index 00000000000..a4f7b845f8b --- /dev/null +++ b/ui/src/components/artifacts/IssueArtifactFile.test.tsx @@ -0,0 +1,73 @@ +// @vitest-environment jsdom +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { describe, expect, it, vi } from "vitest"; +import { IssueArtifactFile } from "./IssueArtifactCard"; +import { loadArtifactCsv } from "@/lib/artifact-card-data"; + +vi.mock("@/lib/artifact-card-data", async (importOriginal) => ({ + ...(await importOriginal()), + loadArtifactCsv: vi.fn(), +})); + +describe("CSV preview consent", () => { + it("does not fetch any CSV until its preview is requested", async () => { + const load = vi + .mocked(loadArtifactCsv) + .mockResolvedValue({ + columns: ["Name"], + rows: [["Actual row"]], + truncated: false, + }); + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + try { + await act(async () => + root.render( + + {Array.from({ length: 20 }, (_, i) => ( + + ))} + , + ), + ); + expect(load).not.toHaveBeenCalled(); + const preview = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Preview data", + ); + expect(preview).toBeDefined(); + await act(async () => preview!.click()); + await vi.waitFor(() => + expect(container.textContent).toContain("Actual row"), + ); + expect(load).toHaveBeenCalledTimes(1); + expect(load).toHaveBeenCalledWith( + "/api/attachments/csv-0/content", + expect.any(AbortSignal), + ); + expect(container.textContent).toContain("View data"); + } finally { + await act(async () => root.unmount()); + client.clear(); + container.remove(); + vi.clearAllMocks(); + } + }); +}); diff --git a/ui/src/components/artifacts/RichArtifactCards.tsx b/ui/src/components/artifacts/RichArtifactCards.tsx new file mode 100644 index 00000000000..2c09febcb56 --- /dev/null +++ b/ui/src/components/artifacts/RichArtifactCards.tsx @@ -0,0 +1,733 @@ +import { useState, type ReactNode } from "react"; +import ReactMarkdown from "react-markdown"; +import { + ArrowRight, + CircleCheck, + CircleHelp, + Clock, + ExternalLink, + File, + FileText, + Film, + GitBranch, + GitCommitHorizontal, + GitMerge, + GitPullRequest, + Globe, + Image as ImageIcon, + Table2, + TriangleAlert, +} from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { Badge } from "@/components/ui/badge"; +import { artifactPreviewUrl, artifactUrl } from "@/lib/artifact-card-data"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogHeader, + DialogTitle, + DialogTrigger, +} from "@/components/ui/dialog"; + +// Renderers contain UI labels only. All artifact-specific content is passed in. +// Example values live exclusively in the individual stories' args. +export interface ArtifactIdentity { + title: string; + summary: string; + author: string; + updatedAt: string; + statusBadge?: ReactNode; +} + +function Identity({ + title, + summary, +}: Pick) { + return ( +
+

+ {title} +

+ {summary && ( +

+ {summary} +

+ )} +
+ ); +} + +function Footer({ + author, + updatedAt, + action, + statusBadge, +}: Pick & { + action: ReactNode; +}) { + return ( +
+ + {[author, updatedAt].filter(Boolean).join(" · ")} + + {statusBadge} + {action} +
+ ); +} + +function Card({ children }: { children: ReactNode }) { + return ( +
+ {children} +
+ ); +} + +function SourceLink({ url, children }: { url: string; children: ReactNode }) { + return url ? ( + + ) : ( + + ); +} + +function Viewer({ + title, + description, + action, + children, +}: { + title: string; + description: string; + action: string; + children: ReactNode; +}) { + return ( + + + + + + + {title} + {description} + + {children} + + + ); +} + +interface DiffProps { + additions?: number | null; + deletions?: number | null; + filesChanged?: number | null; +} +function Diff({ additions, deletions, filesChanged }: DiffProps) { + return ( +
+ {additions != null && ( + + +{additions.toLocaleString("en-US")} + + )} + {deletions != null && ( + + −{deletions.toLocaleString("en-US")} + + )} + {filesChanged != null && ( + + {filesChanged} {filesChanged === 1 ? "file" : "files"} + + )} +
+ ); +} + +export interface PullRequestCardProps extends ArtifactIdentity, DiffProps { + number?: number | null; + repository: string; + sourceBranch: string; + targetBranch: string; + state: "open" | "draft" | "merged" | "closed" | "unknown"; + checks: "passed" | "pending" | "failed" | "unknown"; + evidenceSource: string; + reviewSummary: string; + url: string; +} + +const checksLabel = { + passed: "Checks passed", + pending: "Checks pending", + failed: "Checks failed", + unknown: "Checks not available", +}; +const stateLabel = { + open: "Open", + draft: "Draft", + merged: "Merged", + closed: "Closed", + unknown: "State unknown", +}; + +export function PullRequestCard(props: PullRequestCardProps) { + const { + number, + repository, + sourceBranch, + targetBranch, + state, + checks, + evidenceSource, + reviewSummary, + url, + } = props; + const CheckIcon = { + passed: CircleCheck, + pending: Clock, + failed: TriangleAlert, + unknown: CircleHelp, + }[checks]; + const checkTone = { + passed: "text-(--status-task-icon-done)", + pending: "text-muted-foreground", + failed: "text-(--status-task-icon-blocked)", + unknown: "text-muted-foreground", + }[checks]; + return ( + +
+
+ + Pull request{" "} + {number != null && #{number}} + + + {state === "merged" && } + {stateLabel[state]} + +
+ +
+ {repository} + {(sourceBranch || targetBranch) && ( +
+ + + {sourceBranch} + + {sourceBranch && targetBranch && ( + + )} + + {targetBranch} + +
+ )} +
+ +
+
+ + + {checksLabel[checks]} + + {reviewSummary && ( +

{reviewSummary}

+ )} + {evidenceSource && ( +

+ Source: {evidenceSource} +

+ )} +
+