mirror of
https://github.com/runbear-io/beardrive.git
synced 2026-08-25 08:08:08 +02:00
fix(webapp): show changes works in every history feed, not just per-file (BEA-58) (#116)
The unified diff already shipped, but HistoryView passed the `diff` prop only when the route targeted a single file. A reviewer who opened the project-wide feed first — the natural entry point for "what did this agent run change?" — found only Open this version / Download and concluded the product had no diff at all. Pass it unconditionally. `prevBlob` was already a per-path lookup over the whole loaded window, so a mixed-path feed diffs each row against its own predecessor with no new lookup; `run.idx[k]` does the same for rows inside a run card. The existing gates are untouched: deletes never diff, a row with no earlier version in the window says so, binary/too-large keep their download-both message, and nothing fetches until a row is expanded. The folder listing's "Recent changes" teaser stays diff-free on purpose: it fetches n=20 with no Load more, so nearly every row would read "First version". Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5330532f7f
commit
71e52d5704
@@ -217,12 +217,68 @@ test("per-file history: a binary version says so instead of diffing", async ({ p
|
||||
await expect(dv.locator("a", { hasText: "download this version" })).toBeVisible();
|
||||
});
|
||||
|
||||
test("the whole-project feed carries no diff controls", async ({ page }) => {
|
||||
// BEA-58: the diff shipped, but only the per-file page passed the prop, so a
|
||||
// reviewer who opened the whole-project feed first concluded the product had
|
||||
// no diff at all. Every feed offers it now — and the assertion that matters is
|
||||
// that a row in a MIXED-path feed diffs against its own path's predecessor.
|
||||
|
||||
test("the whole-project feed diffs a row against its own predecessor", async ({ page }) => {
|
||||
await login(page);
|
||||
const pid = await wikiId(page);
|
||||
await page.goto(`/${pid}/history`);
|
||||
await expect(page.locator(".history .hentry").first()).toBeVisible();
|
||||
await expect(page.locator(".history .hdiff-btn")).toHaveCount(0);
|
||||
// Never nth(0): the newest row is the run's, not guide.md's.
|
||||
const row = page.locator(".history > .hentry", { hasText: "guide.md" }).first();
|
||||
await expect(row).toContainText("edited");
|
||||
await row.locator(".hdiff-btn").click();
|
||||
const dv = row.locator(".dv");
|
||||
await expect(dv).toBeVisible();
|
||||
// guide.md's own first version — not whichever row sits below it in the feed.
|
||||
await expect(dv.locator(".dv-rm")).toContainText("First version of the guide.");
|
||||
await expect(dv.locator(".dv-ins")).toContainText("Second version of the guide, with more detail.");
|
||||
await expect(dv.locator(".dv-add")).toHaveText("+1");
|
||||
await expect(dv.locator(".dv-del")).toHaveText("−1");
|
||||
// Expanding is not navigating.
|
||||
await expect(page).toHaveURL(`/${pid}/history`);
|
||||
|
||||
// A delete has no content to diff, and says nothing at all — not "first
|
||||
// version".
|
||||
const del = page.locator(".hentry.delete", { hasText: "scratch.md" }).first();
|
||||
await expect(del.locator(".hdiff-btn")).toHaveCount(0);
|
||||
await expect(del.locator(".hdiff-none")).toHaveCount(0);
|
||||
// index.md has one version only, so it says so rather than rendering an
|
||||
// empty diff.
|
||||
const only = page.locator(".history > .hentry", { hasText: "index.md" }).first();
|
||||
await expect(only.locator(".hdiff-btn")).toHaveCount(0);
|
||||
await expect(only.locator(".hdiff-none")).toContainText("nothing to compare against");
|
||||
await expect(only.locator(".dv")).toHaveCount(0);
|
||||
|
||||
// Rows inside a run card get it too: run.idx[k] indexes back into the flat
|
||||
// list, so the card's rows compare against their own paths — the rewritten
|
||||
// file against its 24h-old version, and the file the run CREATED against
|
||||
// nothing.
|
||||
const inCard = page.locator(".hrun-body .hentry", { hasText: "notes/readme.md" }).first();
|
||||
await inCard.locator(".hdiff-btn").click();
|
||||
await expect(inCard.locator(".dv-rm")).toContainText("Nested folder content.");
|
||||
await expect(inCard.locator(".dv-ins")).toContainText("Rewritten during the agent run.");
|
||||
const created = page.locator(".hrun-body .hentry", { hasText: "runbook.md" }).first();
|
||||
await expect(created.locator(".hdiff-btn")).toHaveCount(0);
|
||||
await expect(created.locator(".hdiff-none")).toContainText("nothing to compare against");
|
||||
});
|
||||
|
||||
test("the folder feed diffs a row against its own predecessor", async ({ page }) => {
|
||||
await login(page);
|
||||
const pid = await wikiId(page);
|
||||
await page.goto(`/${pid}/history/notes`);
|
||||
// The run touched one file inside notes/, so it is a bare row here, not a
|
||||
// card. Its predecessor is the 24h-old version, further down this feed.
|
||||
const row = page.locator(".history > .hentry", { hasText: "notes/readme.md" }).first();
|
||||
await row.locator(".hdiff-btn").click();
|
||||
const dv = row.locator(".dv");
|
||||
await expect(dv).toBeVisible();
|
||||
await expect(dv.locator(".dv-rm")).toContainText("Nested folder content.");
|
||||
await expect(dv.locator(".dv-ins")).toContainText("Rewritten during the agent run.");
|
||||
// Expanding is not navigating.
|
||||
await expect(page).toHaveURL(`/${pid}/history/notes`);
|
||||
});
|
||||
|
||||
test("folder listing's Full history goes to the subtree feed", async ({ page }) => {
|
||||
|
||||
@@ -61,14 +61,14 @@ export function HistoryRow({
|
||||
}: {
|
||||
entry: HistoryEntry;
|
||||
// Its own prop, not something nested in `diff`: the version controls below
|
||||
// belong on every feed, and `diff` is per-file-only by design.
|
||||
// belong on every row that has content, diff or no diff.
|
||||
apiBase: string;
|
||||
// The row's own version (e.blob) rides along: a row is an address for the
|
||||
// bytes it describes, not a shortcut to whatever the file says now.
|
||||
onOpen: (path: string, version?: string) => void;
|
||||
// Present only in the per-file history view, where "the previous version"
|
||||
// is unambiguous. `prev` is the sha of the entry before this one on the
|
||||
// same path; absent means this is the first version.
|
||||
// Present in every paged history feed. `prev` is the sha of the entry
|
||||
// before this one ON THE SAME PATH; absent means there is no earlier
|
||||
// version in the loaded window.
|
||||
diff?: { apiBase: string; prev?: string };
|
||||
restore?: RestoreAction;
|
||||
// Only ever offered on an add inside a run card, where "this run created
|
||||
|
||||
@@ -89,9 +89,6 @@ export function HistoryView(props: {
|
||||
/>
|
||||
);
|
||||
if (!data) return bar ? <div className="history">{bar}</div> : null;
|
||||
// Diffs are a per-file affair: the subtree feed mixes paths, and each row
|
||||
// there would need its own predecessor lookup for no review benefit.
|
||||
const perFile = !!target && !isFolder(target);
|
||||
// Entries arrive newest-first, so a row's predecessor is the next entry
|
||||
// below it on the same path that still has content. This keeps scanning the
|
||||
// flat list, never a group: it is a per-path lookup, and grouping must not
|
||||
@@ -144,7 +141,6 @@ export function HistoryView(props: {
|
||||
run={item.run}
|
||||
onOpen={props.onOpen}
|
||||
apiBase={apiBase}
|
||||
perFile={perFile}
|
||||
prevBlob={prevBlob}
|
||||
restoreSha={restoreSha}
|
||||
restore={restore}
|
||||
@@ -156,7 +152,12 @@ export function HistoryView(props: {
|
||||
entry={entries[item.i]}
|
||||
apiBase={apiBase}
|
||||
onOpen={props.onOpen}
|
||||
diff={perFile ? { apiBase, prev: prevBlob(item.i) } : undefined}
|
||||
/* Every feed, not just the per-file one (BEA-58): prevBlob is a
|
||||
per-path lookup, so a mixed-path feed diffs each row against its
|
||||
own predecessor. A review-focused reader opens the whole-project
|
||||
feed first, and finding no diff there read as "this product has
|
||||
none". */
|
||||
diff={{ apiBase, prev: prevBlob(item.i) }}
|
||||
restore={restore}
|
||||
restoreSha={restoreSha(item.i)}
|
||||
/* no `remove`: an add outside a run card isn't attributable to a
|
||||
@@ -185,7 +186,6 @@ function RunGroup({
|
||||
run,
|
||||
onOpen,
|
||||
apiBase,
|
||||
perFile,
|
||||
prevBlob,
|
||||
restoreSha,
|
||||
restore,
|
||||
@@ -194,7 +194,6 @@ function RunGroup({
|
||||
run: Run;
|
||||
onOpen: (path: string, version?: string) => void;
|
||||
apiBase: string;
|
||||
perFile: boolean;
|
||||
prevBlob: (i: number) => string | undefined;
|
||||
restoreSha: (i: number) => string | undefined;
|
||||
restore?: RestoreAction;
|
||||
@@ -240,7 +239,10 @@ function RunGroup({
|
||||
entry={e}
|
||||
apiBase={apiBase}
|
||||
onOpen={onOpen}
|
||||
diff={perFile ? { apiBase, prev: prevBlob(run.idx[k]) } : undefined}
|
||||
/* run.idx[k] indexes back into the flat list, so a row inside a
|
||||
card compares against its own predecessor, not its neighbour
|
||||
in the card. */
|
||||
diff={{ apiBase, prev: prevBlob(run.idx[k]) }}
|
||||
restore={restore}
|
||||
remove={remove}
|
||||
restoreSha={restoreSha(run.idx[k])}
|
||||
|
||||
+23
-23
File diff suppressed because one or more lines are too long
@@ -5,7 +5,7 @@
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||||
<title>BearDrive</title>
|
||||
<link rel="icon" href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 32 32' fill='%23f5a623'><rect x='4' y='4' width='5.6' height='24'/><rect x='11.2' y='4' width='14.4' height='11.2'/><rect x='11.2' y='16.8' width='16.8' height='11.2'/></svg>">
|
||||
<script type="module" crossorigin src="/assets/index-w2PDXp5a.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-BMdHuKLj.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-DKdbhP6i.css">
|
||||
</head>
|
||||
<body>
|
||||
|
||||
Reference in New Issue
Block a user