From 4d4eb2c4e68d0768a087857120bb16927f0a8fc5 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 27 Jul 2026 16:45:47 -0400 Subject: [PATCH] Show run artifacts by file with version history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The artifacts page grouped captures by stage, which is the storage key `(stage, retry, path)` rather than anything a reader thinks in. A file rewritten by four stages appeared as four separate rows under four headings, with no indication they were the same file. Group by path instead. Each file is one row showing its latest capture; earlier captures disclose inline behind a chevron with the producing stage, size, and the byte change that capture introduced. Three fixes fall out of the regrouping: - Order versions by the producing stage's `startedAt`. The previous sort was alphabetical by stage label, which scrambled history — a report that grew 8.42 KB -> 13.16 -> 14.32 -> 17.48 rendered newest-first under a heading implying it was the earliest. - Drop captures from graph control nodes (`start`, `exit`) via the existing `isVisibleStage` helper. Those nodes run no work, so the files they match are pre-existing workspace files swept up by the capture globs, not run output. This is display-side only; the capture path still stores them. - Show the retry badge at `retry > 1` rather than `retry > 0`. Attempts are 1-based, so the old condition matched every capture and rendered a "retry 1" badge on every group. Grouping lives in a separate module so it is testable without React. Co-Authored-By: Claude Opus 5 (1M context) --- apps/fabro-web/app/routes/run-artifacts.tsx | 257 +++++++++--------- .../app/routes/run-artifacts/group.test.ts | 185 +++++++++++++ .../app/routes/run-artifacts/group.ts | 124 +++++++++ 3 files changed, 440 insertions(+), 126 deletions(-) create mode 100644 apps/fabro-web/app/routes/run-artifacts/group.test.ts create mode 100644 apps/fabro-web/app/routes/run-artifacts/group.ts diff --git a/apps/fabro-web/app/routes/run-artifacts.tsx b/apps/fabro-web/app/routes/run-artifacts.tsx index c44b6c633..df719716a 100644 --- a/apps/fabro-web/app/routes/run-artifacts.tsx +++ b/apps/fabro-web/app/routes/run-artifacts.tsx @@ -1,14 +1,15 @@ -import { useMemo } from "react"; +import { useMemo, useState } from "react"; import { useParams } from "react-router"; -import { ArrowDownTrayIcon, PaperClipIcon } from "@heroicons/react/24/outline"; -import type { RunArtifactEntry } from "@qltysh/fabro-api-client"; +import { ArrowDownTrayIcon, ChevronRightIcon, PaperClipIcon } from "@heroicons/react/24/outline"; import { EmptyState, ErrorState, LoadingState } from "../components/state"; import { StageSidebar } from "../components/stage-sidebar"; import { stageArtifactDownloadUrl } from "../lib/api-client"; import { formatBytes } from "../lib/format"; import { useRunArtifacts, useRunStages } from "../lib/queries"; -import { formatStageLabel, mapRunStagesToSidebarStages } from "../lib/stage-sidebar"; +import { mapRunStagesToSidebarStages } from "../lib/stage-sidebar"; +import type { ArtifactFile, ArtifactVersion } from "./run-artifacts/group"; +import { groupArtifactsByFile } from "./run-artifacts/group"; export const handle = { wide: true }; @@ -40,6 +41,9 @@ function RunArtifactsBody({ artifactsQuery: ReturnType; stages: ReturnType; }) { + const entries = artifactsQuery.data?.data ?? []; + const files = useMemo(() => groupArtifactsByFile(entries, stages), [entries, stages]); + if (artifactsQuery.error) { return ( ; } - const entries = artifactsQuery.data?.data ?? []; - if (entries.length === 0) { + if (files.length === 0) { return ( ); } - return ; + return ; } -interface StageGroup { - key: string; - stageId: string; - retry: number; - label: string; - entries: RunArtifactEntry[]; - totalBytes: number; -} - -function groupArtifacts( - entries: readonly RunArtifactEntry[], - stages: ReturnType, -): StageGroup[] { - const stageLabels = new Map(); - for (const stage of stages) { - stageLabels.set(stage.id, formatStageLabel(stage)); - } - - const groups = new Map(); - for (const entry of entries) { - const key = `${entry.stage_id}#${entry.retry}`; - const existing = groups.get(key); - if (existing) { - existing.entries.push(entry); - existing.totalBytes += entry.size; - } else { - groups.set(key, { - key, - stageId: entry.stage_id, - retry: entry.retry, - label: stageLabels.get(entry.stage_id) ?? entry.node_slug, - entries: [entry], - totalBytes: entry.size, - }); +function ArtifactList({ runId, files }: { runId: string; files: readonly ArtifactFile[] }) { + const { captures, latestBytes, storedBytes } = useMemo(() => { + let captures = 0; + let latestBytes = 0; + let storedBytes = 0; + for (const file of files) { + captures += file.versions.length; + latestBytes += file.latest.size; + for (const version of file.versions) storedBytes += version.size; } - } + return { captures, latestBytes, storedBytes }; + }, [files]); - for (const group of groups.values()) { - group.entries.sort((a, b) => a.relative_path.localeCompare(b.relative_path)); - } - const sortedGroups = Array.from(groups.values()); - sortedGroups.sort((a, b) => { - const labelCmp = a.label.localeCompare(b.label); - return labelCmp !== 0 ? labelCmp : a.retry - b.retry; - }); - return sortedGroups; -} - -function ArtifactList({ - runId, - entries, - stages, -}: { - runId: string; - entries: readonly RunArtifactEntry[]; - stages: ReturnType; -}) { - const groups = useMemo(() => groupArtifacts(entries, stages), [entries, stages]); - const totalBytes = useMemo( - () => entries.reduce((sum, entry) => sum + entry.size, 0), - [entries], - ); + // Only mention versions once some file actually has more than one. + const versioned = captures > files.length; return (
-
+

- {entries.length} {entries.length === 1 ? "artifact" : "artifacts"} + {files.length} {files.length === 1 ? "file" : "files"} + {versioned && ( + · {captures} versions + )}

- {formatBytes(totalBytes)} total + {versioned + ? `${formatBytes(latestBytes)} latest · ${formatBytes(storedBytes)} stored` + : `${formatBytes(latestBytes)} total`}
- {groups.map((group) => ( - - ))} +
+ {files.map((file) => ( + + ))} +
); } -function StageGroupCard({ runId, group }: { runId: string; group: StageGroup }) { +function ArtifactFileRow({ runId, file }: { runId: string; file: ArtifactFile }) { + const [expanded, setExpanded] = useState(false); + const earlier = file.versions.slice(1); + return ( -
-
-
-

{group.label}

- {group.retry > 0 && ( - - retry {group.retry} - - )} -
- - {group.entries.length} {group.entries.length === 1 ? "file" : "files"} - {" · "} - {formatBytes(group.totalBytes)} +
+
+ {earlier.length > 0 ? ( + + ) : ( +
-
    - {group.entries.map((entry) => ( - - ))} -
-
+ + {earlier.length > 0 && ( + + {file.versions.length} versions + + )} + + {file.latest.stageLabel} + + {formatBytes(file.latest.size)} + + +
+ + {expanded && earlier.length > 0 && ( +
    + {earlier.map((version) => ( +
  • + + {version.stageLabel} + {version.retry > 1 && ( + attempt {version.retry} + )} + + + {formatBytes(version.size)} + + + +
    • + ))} +
+ )} + ); } -function ArtifactRow({ runId, entry }: { runId: string; entry: RunArtifactEntry }) { - const href = stageArtifactDownloadUrl( - runId, - entry.stage_id, - entry.relative_path, - entry.retry, - ); - +function SizeDelta({ delta }: { delta: number | null }) { + if (delta === null) { + return first; + } + const tone = delta < 0 ? "text-amber" : "text-mint"; + const sign = delta < 0 ? "−" : "+"; return ( -
  • - - {entry.relative_path} - - - {formatBytes(entry.size)} - - - -
    • + + {sign} + {formatBytes(Math.abs(delta))} + + ); +} + +function DownloadLink({ + runId, + path, + version, +}: { + runId: string; + path: string; + version: ArtifactVersion; +}) { + const href = stageArtifactDownloadUrl(runId, version.stageId, path, version.retry); + return ( + + ); } diff --git a/apps/fabro-web/app/routes/run-artifacts/group.test.ts b/apps/fabro-web/app/routes/run-artifacts/group.test.ts new file mode 100644 index 000000000..a4cfa2ff9 --- /dev/null +++ b/apps/fabro-web/app/routes/run-artifacts/group.test.ts @@ -0,0 +1,185 @@ +import { describe, expect, test } from "bun:test"; +import { StageHandler, StageState } from "@qltysh/fabro-api-client"; +import type { RunArtifactEntry } from "@qltysh/fabro-api-client"; + +import type { Stage } from "../../lib/stage-sidebar"; +import { groupArtifactsByFile, splitArtifactPath } from "./group"; + +function stage(nodeId: string, startedAt: string | null, visit = 1): Stage { + return { + id: `${nodeId}@${visit}`, + name: nodeId, + handler: StageHandler.AGENT, + nodeId, + visit, + graphVisit: null, + resumedFromStageId: null, + status: StageState.SUCCEEDED, + duration: "1s", + startedAt, + providerUsed: null, + }; +} + +function artifact( + nodeSlug: string, + path: string, + size: number, + retry = 1, + visit = 1, +): RunArtifactEntry { + return { + stage_id: `${nodeSlug}@${visit}`, + node_slug: nodeSlug, + retry, + relative_path: path, + size, + }; +} + +/** Mirrors run 01KYJ8ZR0N: one report rewritten by four stages. */ +const REPORT = ".ai/reports/2026-07-27-wrk-002-instance-lifecycle.md"; + +const STAGES: Stage[] = [ + stage("start", "2026-07-27T17:12:08Z"), + stage("plan", "2026-07-27T17:21:44Z"), + stage("implement_plan", "2026-07-27T17:44:18Z"), + stage("simplify", "2026-07-27T18:43:11Z"), + stage("consolidate_reviews", "2026-07-27T19:44:26Z"), + stage("fix_review_findings", "2026-07-27T19:49:22Z"), +]; + +describe("splitArtifactPath", () => { + test("splits a nested path into directory prefix and filename", () => { + expect(splitArtifactPath(".ai/reports/run.md")).toEqual({ + dir: ".ai/reports/", + name: "run.md", + }); + }); + + test("leaves a root-level path without a directory", () => { + expect(splitArtifactPath("README.md")).toEqual({ dir: "", name: "README.md" }); + }); +}); + +describe("groupArtifactsByFile", () => { + test("collapses repeated captures of one path into a single file", () => { + const files = groupArtifactsByFile( + [ + artifact("consolidate_reviews", REPORT, 14323), + artifact("fix_review_findings", REPORT, 17483), + artifact("implement_plan", REPORT, 8422), + artifact("simplify", REPORT, 13162), + ], + STAGES, + ); + + expect(files).toHaveLength(1); + expect(files[0].path).toBe(REPORT); + expect(files[0].dir).toBe(".ai/reports/"); + expect(files[0].name).toBe("2026-07-27-wrk-002-instance-lifecycle.md"); + expect(files[0].versions).toHaveLength(4); + }); + + test("orders versions newest first using stage start time, not stage name", () => { + const files = groupArtifactsByFile( + [ + artifact("consolidate_reviews", REPORT, 14323), + artifact("fix_review_findings", REPORT, 17483), + artifact("implement_plan", REPORT, 8422), + artifact("simplify", REPORT, 13162), + ], + STAGES, + ); + + expect(files[0].versions.map((v) => v.stageLabel)).toEqual([ + "fix_review_findings", + "consolidate_reviews", + "simplify", + "implement_plan", + ]); + expect(files[0].latest.size).toBe(17483); + }); + + test("reports the byte change each capture introduced, oldest capture first", () => { + const files = groupArtifactsByFile( + [ + artifact("implement_plan", REPORT, 8422), + artifact("simplify", REPORT, 13162), + artifact("consolidate_reviews", REPORT, 14323), + artifact("fix_review_findings", REPORT, 17483), + ], + STAGES, + ); + + // versions are newest-first, so deltas read 17483-14323, 14323-13162, ... + expect(files[0].versions.map((v) => v.delta)).toEqual([3160, 1161, 4740, null]); + }); + + test("drops captures from graph control nodes", () => { + const files = groupArtifactsByFile( + [ + artifact("start", ".ai/reports/pre-existing.md", 12402), + artifact("plan", ".ai/plans/plan.md", 21749), + ], + STAGES, + ); + + expect(files.map((file) => file.path)).toEqual([".ai/plans/plan.md"]); + }); + + test("sorts files by their most recent capture", () => { + const files = groupArtifactsByFile( + [ + artifact("plan", ".ai/plans/plan.md", 21749), + artifact("fix_review_findings", REPORT, 17483), + artifact("simplify", ".ai/reviews/bugs.xml", 5231), + ], + STAGES, + ); + + expect(files.map((file) => file.path)).toEqual([ + REPORT, + ".ai/reviews/bugs.xml", + ".ai/plans/plan.md", + ]); + }); + + test("keeps retries of one stage as separate ordered versions", () => { + const files = groupArtifactsByFile( + [ + artifact("simplify", REPORT, 13162, 2), + artifact("simplify", REPORT, 9000, 1), + ], + STAGES, + ); + + expect(files[0].versions.map((v) => v.retry)).toEqual([2, 1]); + expect(files[0].latest.size).toBe(13162); + expect(files[0].versions.map((v) => v.delta)).toEqual([4162, null]); + }); + + test("falls back to a stable order when stages have not loaded yet", () => { + const files = groupArtifactsByFile( + [ + artifact("simplify", REPORT, 13162), + artifact("implement_plan", REPORT, 8422), + ], + [], + ); + + expect(files[0].versions.map((v) => v.stageLabel)).toEqual([ + "simplify", + "implement_plan", + ]); + }); + + test("returns no files when every capture came from a control node", () => { + const files = groupArtifactsByFile( + [artifact("start", ".ai/reports/pre-existing.md", 12402)], + STAGES, + ); + + expect(files).toEqual([]); + }); +}); diff --git a/apps/fabro-web/app/routes/run-artifacts/group.ts b/apps/fabro-web/app/routes/run-artifacts/group.ts new file mode 100644 index 000000000..99ee84f83 --- /dev/null +++ b/apps/fabro-web/app/routes/run-artifacts/group.ts @@ -0,0 +1,124 @@ +import type { RunArtifactEntry } from "@qltysh/fabro-api-client"; + +import { isVisibleStage } from "../../data/runs"; +import type { Stage } from "../../lib/stage-sidebar"; +import { formatStageLabel } from "../../lib/stage-sidebar"; + +/** One capture of a file, written by a single stage attempt. */ +export interface ArtifactVersion { + stageId: string; + stageLabel: string; + retry: number; + size: number; + /** Byte change this capture introduced; null for the first capture. */ + delta: number | null; +} + +/** One artifact path together with its capture history, newest first. */ +export interface ArtifactFile { + path: string; + /** Directory prefix including the trailing slash, or "" at the root. */ + dir: string; + name: string; + versions: ArtifactVersion[]; + latest: ArtifactVersion; +} + +export function splitArtifactPath(path: string): { dir: string; name: string } { + const idx = path.lastIndexOf("/"); + return idx >= 0 + ? { dir: path.slice(0, idx + 1), name: path.slice(idx + 1) } + : { dir: "", name: path }; +} + +interface StageInfo { + label: string; + order: number; +} + +/** + * Chronological position of each stage, keyed by stage ID. + * + * Stage IDs are `node@visit`, where `visit` counts visits to that one node — it + * is not a run-wide ordinal, so it cannot order stages against each other. + * `startedAt` is the only run-wide clock available here. + */ +function stageInfoById(stages: readonly Stage[]): Map { + const chronological = stages + .map((stage, index) => ({ stage, index })) + .sort((a, b) => { + const at = a.stage.startedAt; + const bt = b.stage.startedAt; + // Stages that have not started yet sort last but keep a stable order. + if (at === null && bt === null) return a.index - b.index; + if (at === null) return 1; + if (bt === null) return -1; + const cmp = at.localeCompare(bt); + return cmp !== 0 ? cmp : a.index - b.index; + }); + + const info = new Map(); + chronological.forEach(({ stage }, order) => { + info.set(stage.id, { label: formatStageLabel(stage), order }); + }); + return info; +} + +/** + * Collapse raw `(stage, retry, path)` capture keys into one entry per file, + * carrying the ordered history of every capture of that path. + * + * Captures from graph control nodes (`start`, `exit`) are dropped: those nodes + * run no work, so anything they match is a pre-existing workspace file rather + * than something the run produced. + */ +export function groupArtifactsByFile( + entries: readonly RunArtifactEntry[], + stages: readonly Stage[], +): ArtifactFile[] { + const stageInfo = stageInfoById(stages); + const byPath = new Map>(); + + for (const entry of entries) { + if (!isVisibleStage(entry.node_slug)) continue; + + // Until the stages request resolves there is no clock to order by, so + // unresolved stages fall back to a stable sort on stage ID. + const info = stageInfo.get(entry.stage_id); + const version: ArtifactVersion & { order: number } = { + stageId: entry.stage_id, + stageLabel: info?.label ?? entry.node_slug, + retry: entry.retry, + size: entry.size, + delta: null, + order: info?.order ?? Number.MAX_SAFE_INTEGER, + }; + const bucket = byPath.get(entry.relative_path); + if (bucket) bucket.push(version); + else byPath.set(entry.relative_path, [version]); + } + + const files: Array<{ file: ArtifactFile; order: number }> = []; + for (const [path, versions] of byPath) { + // Oldest first, so each version's delta is the change that capture introduced. + versions.sort( + (a, b) => + a.order - b.order || a.retry - b.retry || a.stageId.localeCompare(b.stageId), + ); + versions.forEach((version, index) => { + version.delta = index === 0 ? null : version.size - versions[index - 1].size; + }); + + const newestFirst = versions.slice().reverse(); + const latest = newestFirst[0]; + const { dir, name } = splitArtifactPath(path); + files.push({ + file: { path, dir, name, versions: newestFirst, latest }, + order: latest.order, + }); + } + + // Most recently written file first — the page answers "what just happened?". + files.sort((a, b) => b.order - a.order || a.file.path.localeCompare(b.file.path)); + return files.map((entry) => entry.file); +}