From 901dd11a137bf83c1b730094aa466e78e71a30e2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 12 Aug 2026 10:37:42 -0700 Subject: [PATCH] improve(ui): header scope dropdown and unchanged-line expansion in session diff (#122711) * feat(ui): expand session diff navigation Release note: Make the session diff scope selectable from its header and let working-tree diffs reveal unchanged file context in bounded chunks. * fix(ui): keep unchanged-line expander labels inside their row Chromium's flex layout on button elements pins the label line box to the button's vertical center, painting the gap count into the diff row above. Grid centering avoids the quirk; skip rows also center-align their controls. * fix(ui): revalidate diff snapshot before expanding unchanged lines Addresses the ClawSweeper P2 stale collapsed-gap content finding by revalidating the target file patch before reading current file text. --- ui/src/e2e/chat-session-diff.e2e.test.ts | 64 +++++- ui/src/i18n/locales/en.ts | 3 + ui/src/lib/chat/session-diff-gaps.test.ts | 77 +++++++ ui/src/lib/chat/session-diff-gaps.ts | 85 ++++++++ ui/src/lib/chat/session-diff.test.ts | 12 +- ui/src/lib/chat/session-diff.ts | 14 +- ui/src/lib/chat/tool-call-diff.ts | 8 + .../pages/chat/components/chat-diff-render.ts | 3 +- .../chat/components/chat-session-workspace.ts | 23 +++ ui/src/pages/chat/components/chat-sidebar.ts | 4 +- .../chat/components/session-diff-menus.ts | 3 +- .../components/session-diff-panel.test.ts | 64 +++++- .../chat/components/session-diff-panel.ts | 192 ++++++++++++++++-- .../chat/components/session-diff-render.ts | 7 +- ui/src/styles/chat/sidebar.css | 69 +++++++ 15 files changed, 590 insertions(+), 38 deletions(-) create mode 100644 ui/src/lib/chat/session-diff-gaps.test.ts create mode 100644 ui/src/lib/chat/session-diff-gaps.ts diff --git a/ui/src/e2e/chat-session-diff.e2e.test.ts b/ui/src/e2e/chat-session-diff.e2e.test.ts index 76f9e4ce9521..281f03a5cdab 100644 --- a/ui/src/e2e/chat-session-diff.e2e.test.ts +++ b/ui/src/e2e/chat-session-diff.e2e.test.ts @@ -40,11 +40,21 @@ const APP_PATCH = [ "index 1111111..2222222 100644", "--- a/src/app.ts", "+++ b/src/app.ts", - "@@ -10,3 +10,4 @@", + "@@ -30,3 +30,4 @@", " context line", "-removed line", "+replacement line", "+extra line", + " trailing context", + "", +].join("\n"); + +const APP_FILE_TEXT = [ + ...Array.from({ length: 29 }, (_, index) => `unchanged line ${index + 1}`), + "context line", + "replacement line", + "extra line", + "trailing context", "", ].join("\n"); @@ -93,8 +103,22 @@ describeControlUiE2e("session diff panel", () => { mergeBase: { sha: "0011223", subject: "Initial commit" }, }; const gateway = await installMockGateway(page, { - featureMethods: ["chat.metadata", "chat.startup", "sessions.diff"], + featureMethods: ["chat.metadata", "chat.startup", "sessions.diff", "sessions.files.get"], methodResponses: { + "sessions.files.get": { + sessionKey: "main", + root: "/tmp/checkout", + file: { + path: "src/app.ts", + workspacePath: "src/app.ts", + name: "app.ts", + kind: "modified", + missing: false, + previewKind: "text", + contentEncoding: "utf8", + content: APP_FILE_TEXT, + }, + }, "sessions.diff": { cases: [ { @@ -206,14 +230,30 @@ describeControlUiE2e("session diff panel", () => { .poll(() => modified.locator(".session-diff__filename").textContent()) .toBe("app.ts"); await expect.poll(() => modified.locator(".session-diff__directory").textContent()).toBe("src"); - // Hunk starting at old line 10 renders a leading gap marker. + // Hunk starting at old line 30 renders a leading expandable gap marker. await expect .poll(() => modified.locator(".chat-diff__row--skip").first().textContent()) - .toContain("9 unmodified lines"); + .toContain("29 unmodified lines"); await expect .poll(() => modified.locator(".chat-diff__row--add").first().textContent()) .toContain("replacement line"); + await modified.getByRole("button", { name: "Show next 20 unmodified lines" }).click(); + await expect.poll(async () => (await gateway.getRequests("sessions.diff")).length).toBe(2); + await expect + .poll(async () => (await gateway.getRequests("sessions.files.get"))[0]?.params) + .toMatchObject({ path: "src/app.ts" }); + await expect + .poll(() => modified.locator(".chat-diff__row").first().textContent()) + .toContain("unchanged line 1"); + await expect + .poll(() => modified.locator(".chat-diff__row--skip").first().textContent()) + .toContain("9 unmodified lines"); + await modified.getByRole("button", { name: "Show previous 9 unmodified lines" }).click(); + await expect.poll(() => modified.locator(".chat-diff__row--skip").count()).toBe(0); + await expect.poll(async () => (await gateway.getRequests("sessions.files.get")).length).toBe(1); + await expect.poll(async () => (await gateway.getRequests("sessions.diff")).length).toBe(3); + const untracked = files.nth(1); await expect .poll(() => untracked.locator(".session-diff__badge").textContent()) @@ -230,41 +270,43 @@ describeControlUiE2e("session diff panel", () => { await panel.getByRole("button", { name: "Change view options" }).click(); await page.getByRole("menuitem", { name: "Switch to Unified Diff" }).click(); await expect.poll(() => modified.locator(".chat-diff").count()).toBe(1); - // View-only toggles reuse parsed patches and do not refetch the RPC. - await expect.poll(async () => (await gateway.getRequests("sessions.diff")).length).toBe(1); + // View-only toggles reuse parsed patches after the expansion revalidations. + await expect.poll(async () => (await gateway.getRequests("sessions.diff")).length).toBe(3); // Collapsing a file hides its diff body. await modified.locator(".session-diff__file-toggle").click(); await expect.poll(() => modified.locator(".chat-diff").count()).toBe(0); await panel.getByRole("button", { name: "Refresh changes" }).click(); - await expect.poll(async () => (await gateway.getRequests("sessions.diff")).length).toBe(2); + await expect.poll(async () => (await gateway.getRequests("sessions.diff")).length).toBe(4); // Refresh keeps the current collapse state instead of expanding every file. await expect.poll(() => modified.locator(".chat-diff").count()).toBe(0); await modified.locator(".session-diff__file-toggle").click(); await expect.poll(() => modified.locator(".chat-diff").count()).toBe(1); - await panel.getByRole("button", { name: "Choose change scope" }).click(); + // The section-title button opens the same scope menu as the footer. + await panel.locator(".session-diff__section-title").click(); await page .locator('openclaw-session-diff-menu wa-dropdown-item[value="scope:uncommitted"]') .click(); await expect - .poll(() => panel.locator(".session-diff__section-title").textContent()) + .poll(() => panel.locator(".session-diff__section-title span").textContent()) .toBe("Uncommitted"); await expect.poll(() => panel.locator(".session-diff__file").count()).toBe(1); await expect .poll(async () => (await gateway.getRequests("sessions.diff")).at(-1)?.params) .toMatchObject({ scope: "uncommitted" }); - await panel.getByRole("button", { name: "Choose change scope" }).click(); + await panel.locator(".session-diff__footer").click(); await page .locator('openclaw-session-diff-menu wa-dropdown-item[value="scope:commit:abc1234"]') .click(); await expect - .poll(() => panel.locator(".session-diff__section-title").textContent()) + .poll(() => panel.locator(".session-diff__section-title span").textContent()) .toBe("abc1234 First feature change"); await expect .poll(async () => (await gateway.getRequests("sessions.diff")).at(-1)?.params) .toMatchObject({ scope: "commit", commit: "abc1234" }); + await expect.poll(() => panel.locator(".session-diff__gap-controls").count()).toBe(0); }); it("hides the diff toggle until the workspace becomes a git checkout", async () => { diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index d9c987c29673..a903008bed62 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -5462,6 +5462,9 @@ export const en: TranslationMap = { openInEditor: "Open in Editor", revealInFileTree: "Reveal in File Tree", unmodifiedLines: "{count} unmodified lines", + expandPreviousLines: "Show previous {count} unmodified lines", + expandNextLines: "Show next {count} unmodified lines", + expandAllLines: "Show all {count} unmodified lines", binaryFile: "Binary file", untracked: "untracked", tooLarge: "Diff too large to display.", diff --git a/ui/src/lib/chat/session-diff-gaps.test.ts b/ui/src/lib/chat/session-diff-gaps.test.ts new file mode 100644 index 000000000000..67dcc23e4433 --- /dev/null +++ b/ui/src/lib/chat/session-diff-gaps.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "vitest"; +import { expandSessionDiffGap } from "./session-diff-gaps.ts"; +import type { DiffLine, DiffLineGap } from "./tool-call-diff.ts"; + +const formatGap = (count: number) => `${count} unmodified lines`; +const gap: DiffLineGap = { oldStart: 1, newStart: 1, count: 50 }; +const fileLines = Array.from({ length: 52 }, (_, index) => `line ${index + 1}`); +const lines: DiffLine[] = [ + { kind: "skip", text: formatGap(gap.count), gap }, + { kind: "ctx", lineNo: 51, text: "line 51" }, + { kind: "add", lineNo: 52, text: "line 52" }, +]; + +describe("expandSessionDiffGap", () => { + it.each([ + { + direction: "down" as const, + contextStart: 1, + contextEnd: 20, + marker: { oldStart: 21, newStart: 21, count: 30 }, + markerIndex: 20, + }, + { + direction: "up" as const, + contextStart: 31, + contextEnd: 50, + marker: { oldStart: 1, newStart: 1, count: 30 }, + markerIndex: 0, + }, + ])( + "reveals a continuous $direction chunk and shrinks the marker", + ({ direction, contextStart, contextEnd, marker, markerIndex }) => { + const expanded = expandSessionDiffGap(lines, gap, fileLines, direction, formatGap); + + expect(expanded?.[markerIndex]).toMatchObject({ kind: "skip", gap: marker }); + const context = expanded?.filter((line) => line.kind === "ctx" && (line.lineNo ?? 0) <= 50); + expect(context?.at(0)).toEqual({ + kind: "ctx", + lineNo: contextStart, + text: `line ${contextStart}`, + }); + expect(context?.at(-1)).toEqual({ + kind: "ctx", + lineNo: contextEnd, + text: `line ${contextEnd}`, + }); + }, + ); + + it("reveals the full gap with continuous new-side line numbers", () => { + const expanded = expandSessionDiffGap(lines, gap, fileLines, "all", formatGap); + + expect(expanded?.some((line) => line.kind === "skip")).toBe(false); + expect(expanded?.map((line) => line.lineNo)).toEqual( + Array.from({ length: 52 }, (_, index) => index + 1), + ); + }); + + it.each(["down", "up", "all"] as const)( + "reveals a gap of 25 lines or fewer in one %s click", + (direction) => { + const smallGap: DiffLineGap = { oldStart: 1, newStart: 1, count: 25 }; + const smallLines: DiffLine[] = [ + { kind: "skip", text: formatGap(smallGap.count), gap: smallGap }, + ]; + const expanded = expandSessionDiffGap(smallLines, smallGap, fileLines, direction, formatGap); + + expect(expanded).toHaveLength(25); + expect(expanded?.some((line) => line.kind === "skip")).toBe(false); + }, + ); + + it("leaves the marker unchanged when working-tree content no longer matches the patch", () => { + const staleLines = fileLines.with(50, "changed since diff"); + expect(expandSessionDiffGap(lines, gap, staleLines, "down", formatGap)).toBeNull(); + }); +}); diff --git a/ui/src/lib/chat/session-diff-gaps.ts b/ui/src/lib/chat/session-diff-gaps.ts new file mode 100644 index 000000000000..8e79b95e6e8e --- /dev/null +++ b/ui/src/lib/chat/session-diff-gaps.ts @@ -0,0 +1,85 @@ +import type { DiffLine, DiffLineGap } from "./tool-call-diff.ts"; + +export type SessionDiffGapDirection = "down" | "up" | "all"; + +const GAP_CHUNK_SIZE = 20; +const EXPAND_WHOLE_GAP_MAX = 25; + +export function splitSessionDiffFileText(text: string): string[] { + const lines = text.replace(/\r\n/g, "\n").replace(/\r/g, "\n").split("\n"); + if (lines.at(-1) === "") { + lines.pop(); + } + return lines; +} + +function fileMatchesPatch(lines: readonly DiffLine[], fileLines: readonly string[]): boolean { + for (const line of lines) { + if ( + (line.kind === "add" || line.kind === "ctx") && + line.lineNo !== undefined && + fileLines[line.lineNo - 1] !== line.text + ) { + return false; + } + } + return true; +} + +function contextRows(fileLines: readonly string[], start: number, count: number): DiffLine[] { + return Array.from({ length: count }, (_, index) => ({ + kind: "ctx" as const, + lineNo: start + index, + text: fileLines[start + index - 1]!, + })); +} + +/** Replaces part or all of one unchanged-lines marker with working-tree context. */ +export function expandSessionDiffGap( + lines: readonly DiffLine[], + target: DiffLineGap, + fileLines: readonly string[], + direction: SessionDiffGapDirection, + formatGap: (count: number) => string, +): DiffLine[] | null { + const index = lines.findIndex((line) => line.kind === "skip" && line.gap === target); + const gapEnd = target.newStart + target.count - 1; + if ( + index < 0 || + target.newStart < 1 || + target.count < 1 || + gapEnd > fileLines.length || + !fileMatchesPatch(lines, fileLines) + ) { + return null; + } + + const revealCount = + direction === "all" || target.count <= EXPAND_WHOLE_GAP_MAX + ? target.count + : Math.min(GAP_CHUNK_SIZE, target.count); + const remainingCount = target.count - revealCount; + const revealStart = direction === "up" ? target.newStart + remainingCount : target.newStart; + const revealed = contextRows(fileLines, revealStart, revealCount); + const replacement: DiffLine[] = []; + if (direction === "up" && remainingCount > 0) { + replacement.push({ + kind: "skip", + text: formatGap(remainingCount), + gap: { ...target, count: remainingCount }, + }); + } + replacement.push(...revealed); + if (direction !== "up" && remainingCount > 0) { + replacement.push({ + kind: "skip", + text: formatGap(remainingCount), + gap: { + oldStart: target.oldStart + revealCount, + newStart: target.newStart + revealCount, + count: remainingCount, + }, + }); + } + return [...lines.slice(0, index), ...replacement, ...lines.slice(index + 1)]; +} diff --git a/ui/src/lib/chat/session-diff.test.ts b/ui/src/lib/chat/session-diff.test.ts index bdc21dcac5e1..4c4e3c99e88e 100644 --- a/ui/src/lib/chat/session-diff.test.ts +++ b/ui/src/lib/chat/session-diff.test.ts @@ -26,13 +26,21 @@ describe("parseSessionDiffPatch", () => { const { lines, truncated } = parseSessionDiffPatch(PATCH, gap); expect(truncated).toBe(false); // Leading gap: first hunk starts at old line 3. - expect(lines[0]).toEqual({ kind: "skip", text: "2 unmodified lines" }); + expect(lines[0]).toEqual({ + kind: "skip", + text: "2 unmodified lines", + gap: { oldStart: 1, newStart: 1, count: 2 }, + }); expect(lines[1]).toEqual({ kind: "ctx", lineNo: 3, text: "context" }); expect(lines[2]).toEqual({ kind: "del", lineNo: 4, text: "old line" }); expect(lines[3]).toEqual({ kind: "add", lineNo: 4, text: "new line" }); expect(lines[4]).toEqual({ kind: "add", lineNo: 5, text: "added line" }); // Gap between hunk 1 (old lines 3-4 consumed) and hunk 2 (old line 160). - expect(lines[5]).toEqual({ kind: "skip", text: "155 unmodified lines" }); + expect(lines[5]).toEqual({ + kind: "skip", + text: "155 unmodified lines", + gap: { oldStart: 5, newStart: 6, count: 155 }, + }); expect(lines[6]).toEqual({ kind: "ctx", lineNo: 161, text: "more context" }); expect(lines[7]).toEqual({ kind: "del", lineNo: 161, text: "tail old" }); expect(lines).toHaveLength(8); diff --git a/ui/src/lib/chat/session-diff.ts b/ui/src/lib/chat/session-diff.ts index 6c5d510f2aeb..a5caa3c9ae4f 100644 --- a/ui/src/lib/chat/session-diff.ts +++ b/ui/src/lib/chat/session-diff.ts @@ -29,8 +29,9 @@ export function parseSessionDiffPatch( let inHunk = false; let oldNo = 0; let newNo = 0; - // Next expected old-file line after the previous hunk; drives gap counts. + // Next expected lines after the previous hunk; drive inter-hunk gap coordinates. let oldNext: number | undefined; + let newNext: number | undefined; const rawLines = patch.replace(/\r\n/g, "\n").split("\n"); if (rawLines.at(-1) === "") { rawLines.pop(); @@ -42,7 +43,15 @@ export function parseSessionDiffPatch( const newStart = Number.parseInt(hunk[2] ?? "", 10); const gap = oldNext === undefined ? oldStart - 1 : oldStart - oldNext; if (gap > 0) { - lines.push({ kind: "skip", text: formatGap(gap) }); + lines.push({ + kind: "skip", + text: formatGap(gap), + gap: { + oldStart: oldNext ?? oldStart - gap, + newStart: newNext ?? newStart - gap, + count: gap, + }, + }); } oldNo = oldStart; newNo = newStart; @@ -69,6 +78,7 @@ export function parseSessionDiffPatch( newNo += 1; } oldNext = oldNo; + newNext = newNo; } return { lines, truncated }; } diff --git a/ui/src/lib/chat/tool-call-diff.ts b/ui/src/lib/chat/tool-call-diff.ts index 1e1810bda113..2339c6bb327d 100644 --- a/ui/src/lib/chat/tool-call-diff.ts +++ b/ui/src/lib/chat/tool-call-diff.ts @@ -9,10 +9,18 @@ export type DiffLineKind = "add" | "del" | "ctx" | "file" | "skip"; +export type DiffLineGap = { + oldStart: number; + newStart: number; + count: number; +}; + export type DiffLine = { kind: DiffLineKind; /** 1-based line number in the file (new file for adds/ctx, old file for dels). */ lineNo?: number; + /** Session-diff coordinates for an expandable unchanged-lines marker. */ + gap?: DiffLineGap; text: string; }; diff --git a/ui/src/pages/chat/components/chat-diff-render.ts b/ui/src/pages/chat/components/chat-diff-render.ts index 5a4ee6c19e25..f52c430c3356 100644 --- a/ui/src/pages/chat/components/chat-diff-render.ts +++ b/ui/src/pages/chat/components/chat-diff-render.ts @@ -30,6 +30,7 @@ export function renderDiffStatChips(stat: DiffStat & { modified?: number }) { export function renderDiffBlock( lines: readonly DiffLine[], outcome: ToolCardOutcome = "succeeded", + renderSkip?: (line: DiffLine) => unknown, ) { const hasLineNumbers = lines.some((line) => line.lineNo !== undefined); return html` @@ -47,7 +48,7 @@ export function renderDiffBlock( return html`
${hasLineNumbers ? html`` : nothing} - ${line.text || "⋯"} + ${(renderSkip?.(line) ?? line.text) || "⋯"}
`; } const kindClass = diff --git a/ui/src/pages/chat/components/chat-session-workspace.ts b/ui/src/pages/chat/components/chat-session-workspace.ts index 911f242fa066..7d895d338f42 100644 --- a/ui/src/pages/chat/components/chat-session-workspace.ts +++ b/ui/src/pages/chat/components/chat-session-workspace.ts @@ -835,6 +835,8 @@ export function createSessionWorkspaceProps( /** Sidebar payload whose loader refetches sessions.diff for the pane's session. */ function buildSessionDiffSidebarContent(state: SessionWorkspaceHost): SidebarContent { const sessionKey = state.sessionKey; + const canLoadFileText = + isGatewayMethodAdvertised(state, "sessions.files.get") === true && Boolean(state.client); return { kind: "session-diff", load: async (scope) => { @@ -847,6 +849,27 @@ function buildSessionDiffSidebarContent(state: SessionWorkspaceHost): SidebarCon ...scope, }); }, + loadFileText: canLoadFileText + ? async (path) => { + try { + const result = await state.sessions.getFile(sessionKey, path, { + agentId: scopedAgentParamsForSession(state, sessionKey).agentId, + }); + const file = result?.file; + if ( + !file || + (file.previewKind !== undefined && file.previewKind !== "text") || + (file.contentEncoding !== undefined && file.contentEncoding !== "utf8") || + typeof file.content !== "string" + ) { + return null; + } + return file.content; + } catch { + return null; + } + } + : undefined, openFile: (path) => openFile(state, getWorkspaceState(state), path), revealFile: (path) => revealSessionWorkspaceFile(state, path), }; diff --git a/ui/src/pages/chat/components/chat-sidebar.ts b/ui/src/pages/chat/components/chat-sidebar.ts index 52006938c0f3..75a6eec7b06f 100644 --- a/ui/src/pages/chat/components/chat-sidebar.ts +++ b/ui/src/pages/chat/components/chat-sidebar.ts @@ -26,7 +26,7 @@ import { OpenClawLightDomElement } from "../../../lit/openclaw-element.ts"; import "./session-diff-panel.ts"; import { renderChatSidebarEditorMenu } from "./chat-sidebar-editor-menu.ts"; import type { FileEditorViewHandle } from "./file-editor-view.ts"; -import type { SessionDiffLoader } from "./session-diff-panel.ts"; +import type { SessionDiffFileTextLoader, SessionDiffLoader } from "./session-diff-panel.ts"; type DetailUnavailableReason = "not_found" | "oversized" | "not_visible"; type DetailFullMessageResult = { @@ -81,6 +81,7 @@ type SessionDiffSidebarContent = { kind: "session-diff"; /** Fetches a fresh sessions.diff snapshot; the panel refetches on refresh. */ load: SessionDiffLoader; + loadFileText?: SessionDiffFileTextLoader; openFile?: (path: string) => void; revealFile?: (path: string) => void; rawText?: string | null; @@ -594,6 +595,7 @@ function renderMarkdownSidebar(props: MarkdownSidebarProps) { : content.kind === "session-diff" ? html`` diff --git a/ui/src/pages/chat/components/session-diff-menus.ts b/ui/src/pages/chat/components/session-diff-menus.ts index 55a35bd1021c..f75963c39250 100644 --- a/ui/src/pages/chat/components/session-diff-menus.ts +++ b/ui/src/pages/chat/components/session-diff-menus.ts @@ -32,6 +32,7 @@ export type SessionDiffMenuData = trigger: HTMLElement; active: SessionDiffScope; result: SessionsDiffResult; + placement?: "top-start" | "bottom-start"; } | { kind: "sync"; @@ -281,7 +282,7 @@ class SessionDiffMenu extends OpenClawLightDomElement { if (!menu) { return nothing; } - const placement = menu.kind === "scope" ? "top-start" : "bottom-end"; + const placement = menu.kind === "scope" ? (menu.placement ?? "top-start") : "bottom-end"; const width = menu.kind === "sync" ? 360 : menu.kind === "scope" ? 340 : 240; const menuLabel = menu.kind === "file" diff --git a/ui/src/pages/chat/components/session-diff-panel.test.ts b/ui/src/pages/chat/components/session-diff-panel.test.ts index 08fd4a6e31be..058848b14f83 100644 --- a/ui/src/pages/chat/components/session-diff-panel.test.ts +++ b/ui/src/pages/chat/components/session-diff-panel.test.ts @@ -2,10 +2,11 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { SessionsDiffResult } from "../../../../../packages/gateway-protocol/src/index.js"; -import type { SessionDiffLoader } from "./session-diff-panel.ts"; +import type { SessionDiffFileTextLoader, SessionDiffLoader } from "./session-diff-panel.ts"; import "./session-diff-panel.ts"; type SessionDiffElement = HTMLElement & { + loadFileText: SessionDiffFileTextLoader | null; loader: SessionDiffLoader | null; readonly updateComplete: Promise; }; @@ -29,6 +30,43 @@ function result(branch: string): SessionsDiffResult { }; } +const SNAPSHOT_PATCH = [ + "--- a/example.txt", + "+++ b/example.txt", + "@@ -3 +3 @@", + "-before", + "+snapshot line", +].join("\n"); + +const FRESH_PATCH = [ + "--- a/example.txt", + "+++ b/example.txt", + "@@ -1,3 +1,3 @@", + " fresh gap edit", + " context", + "-before", + "+fresh snapshot line", +].join("\n"); + +function fileResult(patch: string): SessionsDiffResult { + return { + sessionKey: "agent:main:test", + branch: "feature/test", + baseRef: "main", + files: [ + { + path: "example.txt", + status: "modified", + additions: 1, + deletions: 1, + patch, + }, + ], + additions: 1, + deletions: 1, + }; +} + afterEach(() => { document.body.replaceChildren(); }); @@ -56,4 +94,28 @@ describe("SessionDiffPanel", () => { expect(panel.textContent).toContain("feature/latest"); expect(panel.textContent).not.toContain("feature/stale"); }); + + it("refreshes the diff instead of expanding file text from a stale gap snapshot", async () => { + const loader = vi + .fn() + .mockResolvedValueOnce(fileResult(SNAPSHOT_PATCH)) + .mockResolvedValueOnce(fileResult(FRESH_PATCH)); + const loadFileText = vi + .fn() + .mockResolvedValue(["expanded current file line", "context", "snapshot line"].join("\n")); + const panel = document.createElement("openclaw-session-diff") as SessionDiffElement; + panel.loader = loader; + panel.loadFileText = loadFileText; + document.body.append(panel); + + await vi.waitFor(() => expect(panel.querySelector(".session-diff__gap-count")).not.toBeNull()); + (panel.querySelector(".session-diff__gap-count") as HTMLButtonElement).click(); + + await vi.waitFor(() => expect(panel.textContent).toContain("fresh snapshot line")); + expect(loader).toHaveBeenCalledTimes(2); + expect(loader).toHaveBeenNthCalledWith(2, { scope: "all" }); + expect(loadFileText).not.toHaveBeenCalled(); + expect(panel.textContent).not.toContain("expanded current file line"); + expect(panel.querySelector(".session-diff__gap-controls")).toBeNull(); + }); }); diff --git a/ui/src/pages/chat/components/session-diff-panel.ts b/ui/src/pages/chat/components/session-diff-panel.ts index 7db54f9d1ffd..57fbfde6043a 100644 --- a/ui/src/pages/chat/components/session-diff-panel.ts +++ b/ui/src/pages/chat/components/session-diff-panel.ts @@ -10,11 +10,17 @@ import type { import { icons } from "../../../components/icons.ts"; import "../../../components/tooltip.ts"; import { t } from "../../../i18n/index.ts"; +import { + expandSessionDiffGap, + splitSessionDiffFileText, + type SessionDiffGapDirection, +} from "../../../lib/chat/session-diff-gaps.ts"; import { pairSessionDiffLines, type SessionSplitDiffRow, } from "../../../lib/chat/session-diff-split.ts"; import { parseSessionDiffPatch, type ParsedFilePatch } from "../../../lib/chat/session-diff.ts"; +import type { DiffLine } from "../../../lib/chat/tool-call-diff.ts"; import { copyToClipboard } from "../../../lib/clipboard.ts"; import { openEditor } from "../../../lib/editor-links.ts"; import { OpenClawLightDomElement } from "../../../lit/openclaw-element.ts"; @@ -30,6 +36,7 @@ import "./session-diff-menus.ts"; import { renderSessionSplitDiff } from "./session-diff-render.ts"; export type SessionDiffLoader = (params: SessionDiffScope) => Promise; +export type SessionDiffFileTextLoader = (path: string) => Promise; type FileView = { file: SessionDiffFile; @@ -125,8 +132,29 @@ function shellArgument(value: string): string { return /^[A-Za-z0-9_./:@+-]+$/.test(value) ? value : `'${value.replaceAll("'", `'\\''`)}'`; } +function scopeParams(scope: SessionDiffScope): SessionDiffScope { + return scope.scope === "commit" + ? { scope: "commit", commit: scope.commit } + : { scope: scope.scope }; +} + +function taskResult(result: SessionsDiffResult): SessionDiffTaskResult { + return { + result, + views: result.files.map((file) => ({ + file, + parsed: file.patch + ? parseSessionDiffPatch(file.patch, (count) => + t("chat.sessionDiff.unmodifiedLines", { count: String(count) }), + ) + : null, + })), + }; +} + class SessionDiffPanel extends OpenClawLightDomElement { @property({ attribute: false }) loader: SessionDiffLoader | null = null; + @property({ attribute: false }) loadFileText: SessionDiffFileTextLoader | null = null; @property({ attribute: false }) openFile: ((path: string) => void) | null = null; @property({ attribute: false }) revealFile: ((path: string) => void) | null = null; @@ -137,6 +165,9 @@ class SessionDiffPanel extends OpenClawLightDomElement { @state() private wrap = loadPreferences().wrap; private readonly splitCache = new WeakMap(); + private readonly fileTextCache = new WeakMap>(); + private readonly unavailableFileText = new WeakSet(); + private prefetchedDiffResult: SessionsDiffResult | null = null; private readonly diffTask = new Task(this, { args: () => @@ -150,18 +181,9 @@ class SessionDiffPanel extends OpenClawLightDomElement { return null; } const params: SessionDiffScope = scope === "commit" ? { scope, commit: commit! } : { scope }; - const result = await loader(params); - return { - result, - views: result.files.map((file) => ({ - file, - parsed: file.patch - ? parseSessionDiffPatch(file.patch, (count) => - t("chat.sessionDiff.unmodifiedLines", { count: String(count) }), - ) - : null, - })), - }; + const result = this.prefetchedDiffResult ?? (await loader(params)); + this.prefetchedDiffResult = null; + return taskResult(result); }, onComplete: (value) => { const currentPaths = new Set(value?.views.map((view) => view.file.path) ?? []); @@ -189,7 +211,11 @@ class SessionDiffPanel extends OpenClawLightDomElement { this.collapsedPaths = next; } - private openAnchoredMenu(event: Event, menu: SessionDiffMenuDraft, upward = false): void { + private openAnchoredMenu( + event: Event, + menu: SessionDiffMenuDraft, + placement: "bottom-end" | "bottom-start" | "top-start" = "bottom-end", + ): void { event.stopPropagation(); const trigger = event.currentTarget; if (!(trigger instanceof HTMLElement)) { @@ -198,7 +224,11 @@ class SessionDiffPanel extends OpenClawLightDomElement { const bounds = trigger.getBoundingClientRect(); this.menu = { ...menu, - anchor: { x: upward ? bounds.left : bounds.right, y: upward ? bounds.top : bounds.bottom }, + ...(menu.kind === "scope" && placement !== "bottom-end" ? { placement } : {}), + anchor: { + x: placement.endsWith("start") ? bounds.left : bounds.right, + y: placement.startsWith("top") ? bounds.top : bounds.bottom, + }, trigger, } as SessionDiffMenuData; } @@ -310,6 +340,119 @@ class SessionDiffPanel extends OpenClawLightDomElement { return rows; } + private canExpandGaps(view: FileView): boolean { + return ( + this.scope.scope !== "commit" && + Boolean(this.loadFileText) && + view.file.binary !== true && + view.parsed !== null && + !view.parsed.truncated && + !this.unavailableFileText.has(view) + ); + } + + private loadFileLines(view: FileView): Promise { + const cached = this.fileTextCache.get(view); + if (cached) { + return cached; + } + const load = this.loadFileText; + const pending = load + ? load(view.file.path) + .then((text) => (text === null ? null : splitSessionDiffFileText(text))) + .catch(() => null) + : Promise.resolve(null); + this.fileTextCache.set(view, pending); + return pending; + } + + private async expandGap( + view: FileView, + line: DiffLine, + direction: SessionDiffGapDirection, + ): Promise { + const parsed = view.parsed; + const loader = this.loader; + if (!parsed || !line.gap || !loader || !this.canExpandGaps(view)) { + return; + } + const scope = this.scope; + let freshResult: SessionsDiffResult; + try { + freshResult = await loader(scopeParams(scope)); + } catch { + return; + } + if ( + this.loader !== loader || + this.scope !== scope || + !this.diffTask.value?.views.includes(view) + ) { + return; + } + const freshFile = freshResult.files.find((file) => file.path === view.file.path); + // The panel renders a snapshot; revalidate its patch server-side because gap-interior + // edits are invisible to row validation. The remaining diff-to-file fetch race is a few + // milliseconds and is an accepted tradeoff without shared snapshot identity. + if (!freshFile || freshFile.patch !== view.file.patch) { + this.fileTextCache.delete(view); + this.prefetchedDiffResult = freshResult; + await this.diffTask.run(); + return; + } + const fileLines = await this.loadFileLines(view); + if (!fileLines || !this.diffTask.value?.views.includes(view)) { + this.unavailableFileText.add(view); + this.requestUpdate(); + return; + } + const expanded = expandSessionDiffGap(parsed.lines, line.gap, fileLines, direction, (count) => + t("chat.sessionDiff.unmodifiedLines", { count: String(count) }), + ); + if (!expanded) { + this.unavailableFileText.add(view); + this.requestUpdate(); + return; + } + parsed.lines = expanded; + this.splitCache.delete(parsed); + this.requestUpdate(); + } + + private renderGap(view: FileView, line: DiffLine): unknown { + const gap = line.gap; + if (!gap || !this.canExpandGaps(view)) { + return line.text; + } + const chunkCount = gap.count <= 25 ? gap.count : Math.min(20, gap.count); + return html` + + + + `; + } + private renderFileBody(view: FileView): TemplateResult { const { file, parsed } = view; if (file.binary === true) { @@ -318,8 +461,11 @@ class SessionDiffPanel extends OpenClawLightDomElement { if (!parsed) { return html`
${t("chat.sessionDiff.tooLarge")}
`; } + const renderGap = (line: DiffLine) => this.renderGap(view, line); return html` - ${this.split ? renderSessionSplitDiff(this.splitRows(parsed)) : renderDiffBlock(parsed.lines)} + ${this.split + ? renderSessionSplitDiff(this.splitRows(parsed), renderGap) + : renderDiffBlock(parsed.lines, "succeeded", renderGap)} ${parsed.truncated ? html`
${t("chat.sessionDiff.truncatedFile")}
` : nothing} @@ -421,7 +567,7 @@ class SessionDiffPanel extends OpenClawLightDomElement { type="button" aria-label=${t("chat.sessionDiff.scopeMenu")} @click=${(event: Event) => - this.openAnchoredMenu(event, { kind: "scope", active: this.scope, result }, true)} + this.openAnchoredMenu(event, { kind: "scope", active: this.scope, result }, "top-start")} > ${label}${icons.chevronUp} `; @@ -447,7 +593,19 @@ class SessionDiffPanel extends OpenClawLightDomElement { } return html` ${this.renderSummary(result)} -
${this.scopeTitle(result)}
+
${result.unavailableReason === "unknown_commit" ? html`
${t("chat.sessionDiff.unknownCommit")}
` diff --git a/ui/src/pages/chat/components/session-diff-render.ts b/ui/src/pages/chat/components/session-diff-render.ts index bbcc8580dbfa..391828d97310 100644 --- a/ui/src/pages/chat/components/session-diff-render.ts +++ b/ui/src/pages/chat/components/session-diff-render.ts @@ -17,7 +17,10 @@ function renderSplitSide(line: DiffLine | undefined, side: "left" | "right") {
`; } -export function renderSessionSplitDiff(rows: readonly SessionSplitDiffRow[]) { +export function renderSessionSplitDiff( + rows: readonly SessionSplitDiffRow[], + renderSkip?: (line: DiffLine) => unknown, +) { return html`
- ${row.line.text || "⋯"} + ${(renderSkip?.(row.line) ?? row.line.text) || "⋯"}
`; } return html`
diff --git a/ui/src/styles/chat/sidebar.css b/ui/src/styles/chat/sidebar.css index 77d1b203d301..9770ce81b577 100644 --- a/ui/src/styles/chat/sidebar.css +++ b/ui/src/styles/chat/sidebar.css @@ -1995,12 +1995,30 @@ openclaw-session-discussion { } .session-diff__section-title { + display: flex; + align-items: center; + gap: 4px; + width: 100%; padding: 11px 8px 6px; + border: 0; + background: transparent; color: var(--muted); font-size: var(--control-ui-text-xs); font-weight: 650; letter-spacing: 0.08em; + text-align: left; text-transform: uppercase; + cursor: var(--cursor-action); +} + +.session-diff__section-title:hover, +.session-diff__section-title:focus-visible { + color: var(--text); +} + +.session-diff__section-title svg { + width: 12px; + height: 12px; } .session-diff__files { @@ -2188,6 +2206,55 @@ openclaw-session-discussion { padding-right: 8px; } +/* Skip rows carry 18px expander buttons; the shared baseline row alignment + derives the strip's baseline from an icon-only button and shears the text + upward, so center these rows instead (tool cards keep baseline rows). */ +.session-diff__file .chat-diff__row--skip { + align-items: center; +} + +.session-diff__file .chat-diff__row--skip .chat-diff__text { + display: inline-flex; + align-items: center; +} + +.session-diff__gap-controls { + display: inline-flex; + align-items: center; + gap: 3px; +} + +/* Grid, not flex: Chromium's flex layout on