mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-08 21:03:51 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The artifacts surface is where board users inspect files, media, and documents produced by agents. > - Grouped artifact stacks make that surface easier to scan by task, but the first pass still made grouping feel secondary to media filters. > - The follow-up request was to make grouping the default and give the grouping control the same icon-only outline treatment used on the issues page. > - This pull request keeps the existing artifact grouping API/UI, then polishes the artifacts toolbar state and Storybook review coverage. > - The benefit is that `/artifacts` now opens in the task-stack view by default while preserving explicit flat-mode filtering via `groupBy=none`. ## Linked Issues or Issue Description No public GitHub issue exists for this internal Paperclip task. ### Subsystem affected ui/ — React + Vite board UI. ### Problem or motivation The `/artifacts` grouping affordance was visually placed after the media filters, rendered as a text button, and defaulted to a flat artifact list. Internal follow-up `PAP-10465` requested the grouping icon move left of the filters, become an icon-only outlined button like `/issues`, and make Task grouping the default. ### Proposed solution Default `/artifacts` to grouped Task stacks, keep explicit flat mode available as `groupBy=none`, move the grouping control before the media chips, and restyle it as the shared icon-only outline button pattern. ### Alternatives considered Leaving flat mode as the implicit default was rejected because it does not satisfy the follow-up. Keeping a text label on the grouping trigger was rejected because `/issues` already established the icon-only outline pattern for this class of toolbar control. ### Roadmap alignment This aligns with the `Artifacts & Work Products` roadmap item by making generated outputs easier to inspect and operate from the board UI. ## What Changed - Defaulted the `/artifacts` page to `groupBy=task` when no grouping URL param is present, while keeping explicit flat mode available with `groupBy=none`. - Moved the group control before the media filter chips and changed it to an icon-only outlined button using the shared `Button` pattern. - Updated artifact page tests to cover default Task grouping, explicit flat mode, trigger ordering, and icon-only outline metadata. - Updated the artifact Storybook story so its toolbar mock matches the production ordering and grouped Task is documented as the default mode. ## Verification - `pnpm exec vitest run ui/src/pages/Artifacts.test.tsx ui/src/components/artifacts/ArtifactGroupCard.test.tsx` — passed. - `pnpm --filter @paperclipai/ui typecheck` — passed. - `pnpm --filter @paperclipai/server typecheck` — passed. - `git diff --check` — passed. - QA visual validation from internal follow-up PAP-10466 passed desktop/mobile scenarios. Screenshot evidence attached there: - Desktop default: http://paperclip-dev:3100/api/attachments/bc81305d-f5de-485c-abeb-9e7c3d9d8539/content - Desktop toolbar close-up: http://paperclip-dev:3100/api/attachments/3375a62b-2110-48f3-bafa-ea98c00f99f7/content - Mobile default: http://paperclip-dev:3100/api/attachments/bfc5642e-9248-431e-9bac-36284dec1c89/content - Mobile toolbar close-up: http://paperclip-dev:3100/api/attachments/ca79401a-5ba8-464d-bc6e-aeffd47fe695/content - GitHub PR checks on head `431964c8b` — passed, including Greptile 5/5. ## Risks Low to medium risk. The main behavior shift is intentional: `/artifacts` now queries grouped Task stacks by default. Existing flat mode remains available through the grouping menu and explicit `groupBy=none` URLs. ## Model Used OpenAI Codex, GPT-5.4 class coding model in this Paperclip heartbeat environment, with shell, git, test, and GitHub CLI tool use. Context window managed by the Codex runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
430 lines
13 KiB
TypeScript
430 lines
13 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { flushSync } from "react-dom";
|
|
import { createRoot } from "react-dom/client";
|
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
|
import { MemoryRouter } from "react-router-dom";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { Artifacts } from "./Artifacts";
|
|
import type { CompanyArtifact, CompanyArtifactGroup } from "../api/artifacts";
|
|
|
|
const companyState = vi.hoisted(() => ({
|
|
selectedCompanyId: "company-1",
|
|
}));
|
|
|
|
const breadcrumbState = vi.hoisted(() => ({
|
|
setBreadcrumbs: vi.fn(),
|
|
}));
|
|
|
|
const artifactsApiMock = vi.hoisted(() => ({
|
|
list: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("../context/CompanyContext", () => ({
|
|
useCompany: () => companyState,
|
|
}));
|
|
|
|
vi.mock("../context/BreadcrumbContext", () => ({
|
|
useBreadcrumbs: () => breadcrumbState,
|
|
}));
|
|
|
|
vi.mock("../api/artifacts", () => ({
|
|
artifactsApi: artifactsApiMock,
|
|
}));
|
|
|
|
// Render the menu inline (no radix portal / pointer-capture) so option clicks
|
|
// are deterministic in jsdom.
|
|
vi.mock("@/components/ui/dropdown-menu", () => ({
|
|
DropdownMenu: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
|
DropdownMenuTrigger: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
|
DropdownMenuContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
|
DropdownMenuLabel: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
|
DropdownMenuItem: ({
|
|
children,
|
|
onSelect,
|
|
...rest
|
|
}: {
|
|
children: React.ReactNode;
|
|
onSelect?: () => void;
|
|
}) => (
|
|
<button type="button" onClick={onSelect} {...rest}>
|
|
{children}
|
|
</button>
|
|
),
|
|
}));
|
|
|
|
vi.mock("../components/artifacts/ArtifactCard", () => ({
|
|
ArtifactCard: ({ artifact }: { artifact: CompanyArtifact }) => (
|
|
<article data-testid="artifact-card">{artifact.title}</article>
|
|
),
|
|
ArtifactPreview: ({ artifact }: { artifact: CompanyArtifact }) => (
|
|
<div data-testid="artifact-preview">{artifact.title}</div>
|
|
),
|
|
}));
|
|
|
|
type ObserverCallback = IntersectionObserverCallback;
|
|
|
|
let latestObserverCallback: ObserverCallback | null = null;
|
|
|
|
class MockIntersectionObserver {
|
|
readonly root = null;
|
|
readonly rootMargin = "";
|
|
readonly thresholds = [];
|
|
|
|
constructor(callback: ObserverCallback) {
|
|
latestObserverCallback = callback;
|
|
}
|
|
|
|
observe = vi.fn();
|
|
unobserve = vi.fn();
|
|
disconnect = vi.fn();
|
|
takeRecords = vi.fn(() => []);
|
|
}
|
|
|
|
function sampleArtifact(overrides: Partial<CompanyArtifact> = {}): CompanyArtifact {
|
|
return {
|
|
id: "artifact-1",
|
|
source: "document",
|
|
mediaKind: "document",
|
|
title: "Launch Brief",
|
|
previewText: "launch brief preview",
|
|
contentType: "text/markdown",
|
|
contentPath: null,
|
|
openPath: null,
|
|
downloadPath: null,
|
|
issue: { id: "issue-1", identifier: "PAP-42", title: "Ship launch" },
|
|
project: null,
|
|
createdByAgent: null,
|
|
updatedAt: "2026-06-01T00:00:00.000Z",
|
|
href: "/PAP/issues/PAP-42#document-brief",
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
function sampleGroup(overrides: Partial<CompanyArtifactGroup> = {}): CompanyArtifactGroup {
|
|
return {
|
|
id: "task:issue-1",
|
|
groupBy: "task",
|
|
issue: { id: "issue-1", identifier: "PAP-42", title: "Ship launch" },
|
|
title: "Ship launch",
|
|
count: 3,
|
|
mediaKinds: ["document"],
|
|
previewArtifacts: [sampleArtifact()],
|
|
updatedAt: "2026-06-01T00:00:00.000Z",
|
|
href: "/PAP/artifacts?groupBy=task&groupIssueId=issue-1",
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
async function flush() {
|
|
await Promise.resolve();
|
|
await new Promise((resolve) => setTimeout(resolve, 0));
|
|
}
|
|
|
|
async function waitForAssertion(assertion: () => void, attempts = 50) {
|
|
let lastError: unknown;
|
|
for (let attempt = 0; attempt < attempts; attempt += 1) {
|
|
try {
|
|
assertion();
|
|
return;
|
|
} catch (error) {
|
|
lastError = error;
|
|
await flush();
|
|
}
|
|
}
|
|
throw lastError;
|
|
}
|
|
|
|
function renderArtifacts(container: HTMLDivElement, initialEntries: string[] = ["/artifacts"]) {
|
|
const root = createRoot(container);
|
|
const queryClient = new QueryClient({
|
|
defaultOptions: { queries: { retry: false } },
|
|
});
|
|
flushSync(() => {
|
|
root.render(
|
|
<QueryClientProvider client={queryClient}>
|
|
<MemoryRouter initialEntries={initialEntries}>
|
|
<Artifacts />
|
|
</MemoryRouter>
|
|
</QueryClientProvider>,
|
|
);
|
|
});
|
|
return { root, queryClient };
|
|
}
|
|
|
|
describe("Artifacts page", () => {
|
|
let container: HTMLDivElement;
|
|
let originalIntersectionObserver: typeof IntersectionObserver | undefined;
|
|
|
|
beforeEach(() => {
|
|
container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
breadcrumbState.setBreadcrumbs.mockReset();
|
|
artifactsApiMock.list.mockReset();
|
|
latestObserverCallback = null;
|
|
originalIntersectionObserver = window.IntersectionObserver;
|
|
window.IntersectionObserver = MockIntersectionObserver as unknown as typeof IntersectionObserver;
|
|
});
|
|
|
|
afterEach(() => {
|
|
window.IntersectionObserver = originalIntersectionObserver as typeof IntersectionObserver;
|
|
container.remove();
|
|
});
|
|
|
|
it("requests task-grouped artifact stacks by default", async () => {
|
|
artifactsApiMock.list.mockResolvedValue({ artifacts: [], groups: [sampleGroup()], nextCursor: null });
|
|
|
|
const { root } = renderArtifacts(container);
|
|
|
|
await waitForAssertion(() => {
|
|
expect(artifactsApiMock.list).toHaveBeenCalledWith("company-1", {
|
|
kind: "all",
|
|
q: undefined,
|
|
groupBy: "task",
|
|
groupIssueId: undefined,
|
|
limit: 30,
|
|
cursor: undefined,
|
|
});
|
|
const groupControl = container.querySelector('[data-testid="artifact-group-control"]') as HTMLButtonElement;
|
|
const allFilter = [...container.querySelectorAll('[role="tab"]')]
|
|
.find((element) => element.textContent === "All") as HTMLButtonElement;
|
|
expect(groupControl).not.toBeNull();
|
|
expect(groupControl.textContent).toBe("");
|
|
expect(groupControl.getAttribute("data-variant")).toBe("outline");
|
|
expect(groupControl.getAttribute("data-size")).toBe("icon");
|
|
expect(groupControl.getAttribute("data-group-by")).toBe("task");
|
|
expect(Boolean(groupControl.compareDocumentPosition(allFilter) & Node.DOCUMENT_POSITION_FOLLOWING)).toBe(true);
|
|
});
|
|
|
|
flushSync(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("debounces artifact search into the artifacts API", async () => {
|
|
artifactsApiMock.list
|
|
.mockResolvedValueOnce({ artifacts: [], groups: [sampleGroup()], nextCursor: null })
|
|
.mockResolvedValueOnce({ artifacts: [], groups: [], nextCursor: null });
|
|
|
|
const { root } = renderArtifacts(container);
|
|
|
|
const input = container.querySelector('input[aria-label="Search artifacts"]') as HTMLInputElement;
|
|
expect(input).not.toBeNull();
|
|
|
|
flushSync(() => {
|
|
const nativeSetter = Object.getOwnPropertyDescriptor(
|
|
HTMLInputElement.prototype,
|
|
"value",
|
|
)!.set!;
|
|
nativeSetter.call(input, "launch");
|
|
input.dispatchEvent(new Event("input", { bubbles: true }));
|
|
});
|
|
|
|
await new Promise((resolve) => setTimeout(resolve, 300));
|
|
|
|
await waitForAssertion(() => {
|
|
expect(artifactsApiMock.list).toHaveBeenLastCalledWith("company-1", {
|
|
kind: "all",
|
|
q: "launch",
|
|
groupBy: "task",
|
|
groupIssueId: undefined,
|
|
limit: 30,
|
|
cursor: undefined,
|
|
});
|
|
});
|
|
|
|
flushSync(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("keeps the artifacts grid max-width constrained and left aligned", async () => {
|
|
artifactsApiMock.list.mockResolvedValue({ artifacts: [sampleArtifact()], nextCursor: null });
|
|
|
|
const { root } = renderArtifacts(container, ["/artifacts?groupBy=none"]);
|
|
|
|
await waitForAssertion(() => {
|
|
expect(container.querySelector('[data-testid="artifact-card"]')).not.toBeNull();
|
|
});
|
|
|
|
const pageShell = container.firstElementChild as HTMLElement | null;
|
|
expect(pageShell?.className).toContain("max-w-6xl");
|
|
expect(pageShell?.className).not.toContain("mx-auto");
|
|
|
|
flushSync(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("fetches the next artifact page when the sentinel intersects", async () => {
|
|
artifactsApiMock.list
|
|
.mockResolvedValueOnce({
|
|
artifacts: [sampleArtifact({ id: "artifact-1", title: "First Artifact" })],
|
|
nextCursor: "cursor-2",
|
|
})
|
|
.mockResolvedValueOnce({
|
|
artifacts: [sampleArtifact({ id: "artifact-2", title: "Second Artifact" })],
|
|
nextCursor: null,
|
|
});
|
|
|
|
const { root } = renderArtifacts(container, ["/artifacts?groupBy=none"]);
|
|
|
|
await waitForAssertion(() => {
|
|
expect(container.textContent).toContain("First Artifact");
|
|
expect(latestObserverCallback).not.toBeNull();
|
|
});
|
|
|
|
latestObserverCallback?.(
|
|
[{ isIntersecting: true } as IntersectionObserverEntry],
|
|
{} as IntersectionObserver,
|
|
);
|
|
|
|
await waitForAssertion(() => {
|
|
expect(artifactsApiMock.list).toHaveBeenLastCalledWith("company-1", {
|
|
kind: "all",
|
|
q: undefined,
|
|
groupBy: "none",
|
|
groupIssueId: undefined,
|
|
limit: 30,
|
|
cursor: "cursor-2",
|
|
});
|
|
expect(container.textContent).toContain("Second Artifact");
|
|
});
|
|
|
|
flushSync(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("switches grouping via the group control and refetches stacks", async () => {
|
|
artifactsApiMock.list.mockImplementation((_companyId: string, params?: { groupBy?: string }) => {
|
|
if (params?.groupBy === "none") {
|
|
return Promise.resolve({ artifacts: [sampleArtifact()], nextCursor: null });
|
|
}
|
|
return Promise.resolve({ artifacts: [], groups: [sampleGroup()], nextCursor: null });
|
|
});
|
|
|
|
const { root } = renderArtifacts(container);
|
|
|
|
await waitForAssertion(() => {
|
|
expect(container.querySelector('[data-testid="artifact-group-card"]')).not.toBeNull();
|
|
});
|
|
|
|
const noneOption = container.querySelector(
|
|
'[data-testid="artifact-group-option-none"]',
|
|
) as HTMLButtonElement;
|
|
expect(noneOption).not.toBeNull();
|
|
|
|
flushSync(() => {
|
|
noneOption.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true }));
|
|
});
|
|
|
|
await waitForAssertion(() => {
|
|
expect(artifactsApiMock.list).toHaveBeenLastCalledWith("company-1", {
|
|
kind: "all",
|
|
q: undefined,
|
|
groupBy: "none",
|
|
groupIssueId: undefined,
|
|
limit: 30,
|
|
cursor: undefined,
|
|
});
|
|
expect(container.querySelector('[data-testid="artifact-card"]')).not.toBeNull();
|
|
const groupControl = container.querySelector('[data-testid="artifact-group-control"]') as HTMLElement;
|
|
expect(groupControl.getAttribute("data-group-by")).toBe("none");
|
|
});
|
|
|
|
flushSync(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("renders stack cards from a grouped URL with count metadata", async () => {
|
|
artifactsApiMock.list.mockResolvedValue({
|
|
artifacts: [],
|
|
groups: [sampleGroup({ count: 4 })],
|
|
nextCursor: null,
|
|
});
|
|
|
|
const { root } = renderArtifacts(container, ["/artifacts?groupBy=task"]);
|
|
|
|
await waitForAssertion(() => {
|
|
expect(artifactsApiMock.list).toHaveBeenCalledWith("company-1", {
|
|
kind: "all",
|
|
q: undefined,
|
|
groupBy: "task",
|
|
groupIssueId: undefined,
|
|
limit: 30,
|
|
cursor: undefined,
|
|
});
|
|
const card = container.querySelector('[data-testid="artifact-group-card"]') as HTMLElement;
|
|
expect(card).not.toBeNull();
|
|
expect(card.getAttribute("data-count")).toBe("4");
|
|
expect(card.getAttribute("data-stacked")).toBe("true");
|
|
expect(card.getAttribute("href")).toBe("/artifacts?groupIssueId=issue-1");
|
|
expect(card.textContent).toContain("4 artifacts");
|
|
});
|
|
|
|
flushSync(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("opens a stack from the URL and shows the back affordance and artifacts", async () => {
|
|
artifactsApiMock.list.mockResolvedValue({
|
|
artifacts: [sampleArtifact({ title: "Stacked Artifact" })],
|
|
selectedGroup: sampleGroup(),
|
|
nextCursor: null,
|
|
});
|
|
|
|
const { root } = renderArtifacts(container, [
|
|
"/artifacts?groupBy=task&groupIssueId=issue-1",
|
|
]);
|
|
|
|
await waitForAssertion(() => {
|
|
expect(artifactsApiMock.list).toHaveBeenCalledWith("company-1", {
|
|
kind: "all",
|
|
q: undefined,
|
|
groupBy: "task",
|
|
groupIssueId: "issue-1",
|
|
limit: 30,
|
|
cursor: undefined,
|
|
});
|
|
expect(container.querySelector('[data-testid="artifact-stack-back"]')).not.toBeNull();
|
|
expect(
|
|
(container.querySelector('[data-testid="artifact-stack-back"]') as HTMLAnchorElement).getAttribute("href"),
|
|
).toBe("/artifacts");
|
|
expect(container.textContent).toContain("Stacked Artifact");
|
|
expect(container.querySelector('[data-testid="artifact-card"]')).not.toBeNull();
|
|
});
|
|
|
|
flushSync(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("preserves the media filter when grouping", async () => {
|
|
artifactsApiMock.list.mockResolvedValue({
|
|
artifacts: [],
|
|
groups: [sampleGroup()],
|
|
nextCursor: null,
|
|
});
|
|
|
|
const { root } = renderArtifacts(container, ["/artifacts?kind=image&groupBy=task"]);
|
|
|
|
await waitForAssertion(() => {
|
|
expect(artifactsApiMock.list).toHaveBeenCalledWith("company-1", {
|
|
kind: "image",
|
|
q: undefined,
|
|
groupBy: "task",
|
|
groupIssueId: undefined,
|
|
limit: 30,
|
|
cursor: undefined,
|
|
});
|
|
});
|
|
|
|
flushSync(() => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
});
|