From ee93725e83fc631d76fbe52e6f1a12780f2858dc Mon Sep 17 00:00:00 2001 From: chengyongru <61816729+chengyongru@users.noreply.github.com> Date: Sun, 26 Jul 2026 19:05:53 +0800 Subject: [PATCH] fix(webui): restore file edit diff display (#5096) --- .../thread/AgentActivityCluster.tsx | 5 + .../thread/activity/FileEditRow.tsx | 354 ++++++++++++++++-- .../src/tests/agent-activity-cluster.test.tsx | 141 ++++++- 3 files changed, 445 insertions(+), 55 deletions(-) diff --git a/webui/src/components/thread/AgentActivityCluster.tsx b/webui/src/components/thread/AgentActivityCluster.tsx index acde2f6a..fddd92e0 100644 --- a/webui/src/components/thread/AgentActivityCluster.tsx +++ b/webui/src/components/thread/AgentActivityCluster.tsx @@ -40,6 +40,7 @@ import { isAgentActivityMember, isReasoningOnlyAssistant, } from "@/lib/activity-timeline"; +import { useFileEditDisplayMode } from "@/hooks/useFileEditDisplayMode"; import { useLogoFallback } from "@/hooks/useLogoFallback"; import { logoFallbackUrls } from "@/lib/provider-brand"; import { canonicalToolTrace, formatToolCallTrace } from "@/lib/tool-traces"; @@ -144,6 +145,7 @@ export function AgentActivityCluster({ onOpenFilePreview, }: AgentActivityClusterProps) { const { t } = useTranslation(); + const fileEditDisplayMode = useFileEditDisplayMode(); const pageVisible = usePageVisibility(); const activityMessages = useMemo(() => coalesceActivityMessages(messages), [messages]); const fileEdits = useMemo( @@ -305,6 +307,7 @@ export function AgentActivityCluster({
@@ -331,6 +334,7 @@ export function AgentActivityCluster({ {fileEdits.length ? ( ) : null} @@ -1031,6 +1035,7 @@ function summarizeFileEdits(edits: UIFileEdit[], active: boolean): FileEditSumma operation: edit.operation, pending: !!edit.pending && !edit.path, error: edit.error, + diff: edit.diff, }]; }); } diff --git a/webui/src/components/thread/activity/FileEditRow.tsx b/webui/src/components/thread/activity/FileEditRow.tsx index 3d7ce287..f87e36b1 100644 --- a/webui/src/components/thread/activity/FileEditRow.tsx +++ b/webui/src/components/thread/activity/FileEditRow.tsx @@ -1,16 +1,45 @@ +import { useEffect, useMemo, useState } from "react"; import { AlertCircle, CheckCircle2, + ChevronDown, + ChevronRight, + ChevronUp, CircleDashed, + ExternalLink, } from "lucide-react"; import { useTranslation } from "react-i18next"; import { FileReferenceChip } from "@/components/FileReferenceChip"; -import type { UIFileEdit } from "@/lib/types"; +import { codeLanguageFromPath } from "@/lib/code-language"; +import { + hasRenderableFileDiff, + parseRenderableFileDiff, + type RenderableFileDiff, + type RenderableFileDiffHunk, +} from "@/lib/file-diff"; +import type { FileEditDisplayMode } from "@/lib/local-preferences"; +import type { UIFileDiff, UIFileEdit } from "@/lib/types"; import { cn } from "@/lib/utils"; import { ActivityStep } from "./ActivityStep"; import { DiffPair } from "./DiffPair"; +import { DiffSyntaxHighlight } from "./DiffSyntaxHighlight"; + +const INITIAL_VISIBLE_DIFF_LINES = 160; +const AUTO_COLLAPSE_DIFF_LINES = INITIAL_VISIBLE_DIFF_LINES; + +interface VisibleDiffHunk { + hunk: RenderableFileDiffHunk; + skippedBefore: number; +} + +interface VisibleDiff { + hunks: VisibleDiffHunk[]; + hiddenLineCount: number; +} + +const EMPTY_VISIBLE_DIFF: VisibleDiff = { hunks: [], hiddenLineCount: 0 }; export interface FileEditSummary { key: string; @@ -24,13 +53,16 @@ export interface FileEditSummary { operation?: UIFileEdit["operation"]; pending: boolean; error?: string; + diff?: UIFileDiff; } export function FileEditGroup({ edits, + displayMode, onOpenFilePreview, }: { edits: FileEditSummary[]; + displayMode: FileEditDisplayMode; onOpenFilePreview?: (path: string) => void; }) { if (edits.length === 0) return null; @@ -40,6 +72,7 @@ export function FileEditGroup({ ))} @@ -49,9 +82,11 @@ export function FileEditGroup({ function FileEditRow({ edit, + displayMode, onOpenFilePreview, }: { edit: FileEditSummary; + displayMode: FileEditDisplayMode; onOpenFilePreview?: (path: string) => void; }) { const { t } = useTranslation(); @@ -59,6 +94,7 @@ function FileEditRow({ const failed = edit.status === "error"; const action = fileEditAction(edit, editing, failed); const hasCountedDiff = !failed && !edit.binary && hasVisibleDiffStats(edit); + const showDiff = canRenderDiff(edit, displayMode); const statusIcon = failed ? ( ) : editing ? ( @@ -68,42 +104,54 @@ function FileEditRow({ ); return ( - - {statusIcon} - - )} - active={editing} - tone={failed ? "error" : editing ? "active" : "success"} - className="text-xs" - ariaLabel={edit.path ? `${action} ${edit.path}` : action} - label={edit.pending && !edit.path - ? t("message.fileEditPreparing", { defaultValue: "Preparing file edit…" }) - : ( - - {action} - - {hasCountedDiff ? : null} +
+ + {statusIcon} )} - /> + active={editing} + tone={failed ? "error" : editing ? "active" : "success"} + className="text-xs" + ariaLabel={edit.path ? `${action} ${edit.path}` : action} + label={edit.pending && !edit.path + ? t("message.fileEditPreparing", { defaultValue: "Preparing file edit…" }) + : ( + + {action} + + {hasCountedDiff ? : null} + + )} + /> + {showDiff ? ( +
+ +
+ ) : null} +
); } @@ -117,3 +165,239 @@ function fileEditAction(edit: FileEditSummary, editing: boolean, failed: boolean if (editing) return deleting ? "Deleting" : "Editing"; return deleting ? "Deleted" : "Edited"; } + +function canRenderDiff(edit: FileEditSummary, displayMode: FileEditDisplayMode): boolean { + return ( + displayMode !== "summary" + && edit.status !== "editing" + && edit.status !== "error" + && hasRenderableFileDiff(edit.diff) + ); +} + +function FileUnifiedDiff({ + diff, + collapsed, + previewPath, + onOpenFilePreview, +}: { + diff: UIFileDiff; + collapsed: boolean; + previewPath?: string; + onOpenFilePreview?: (path: string) => void; +}) { + const { t } = useTranslation(); + const tx = (key: string, fallback: string) => t(key, { defaultValue: fallback }); + const [open, setOpen] = useState(false); + const [expandedLines, setExpandedLines] = useState(false); + const renderableDiff = useMemo(() => parseRenderableFileDiff(diff), [diff]); + const language = useMemo(() => codeLanguageFromPath(previewPath), [previewPath]); + const totalLineCount = useMemo(() => countDiffLines(renderableDiff), [renderableDiff]); + const shouldAutoCollapse = totalLineCount > AUTO_COLLAPSE_DIFF_LINES || !!diff.truncated; + const startsCollapsed = collapsed || shouldAutoCollapse; + const shouldRenderBody = !startsCollapsed || open; + const shouldLimitLines = totalLineCount > INITIAL_VISIBLE_DIFF_LINES; + const lineLimit = expandedLines || !shouldLimitLines + ? totalLineCount + : INITIAL_VISIBLE_DIFF_LINES; + const visibleDiff = useMemo( + () => shouldRenderBody + ? selectVisibleDiffLines(renderableDiff, lineLimit, totalLineCount) + : EMPTY_VISIBLE_DIFF, + [lineLimit, renderableDiff, shouldRenderBody, totalLineCount], + ); + const lineCountLabel = t("message.fileEditDiffLineCount", { + count: diff.truncated ? `${totalLineCount}+` : totalLineCount, + defaultValue: "{{count}} lines", + }); + const viewDiffLabel = shouldAutoCollapse + ? tx("message.fileEditViewLargeDiff", "View large diff") + : tx("message.fileEditViewDiff", "View diff"); + + useEffect(() => { + setOpen(false); + setExpandedLines(false); + }, [diff]); + + const handleToggleOpen = () => { + if (open) setExpandedLines(false); + setOpen(!open); + }; + + if (totalLineCount === 0) return null; + + const renderBody = () => ( +
+ {visibleDiff.hunks.map(({ hunk, skippedBefore }, index) => ( +
0 && "border-t border-border/45")} + > + {skippedBefore > 0 ? : null} +
+ +
+
+ ))} + {visibleDiff.hiddenLineCount > 0 ? ( +
+ +
+ ) : expandedLines && shouldLimitLines ? ( +
+ +
+ ) : null} + {diff.truncated ? ( +
+ + {tx("message.fileEditDiffTruncated", "Diff truncated. Open the file for the full change.")} + + {previewPath && onOpenFilePreview ? ( + + ) : null} +
+ ) : null} +
+ ); + + if (!startsCollapsed) return renderBody(); + + return ( +
+ + {open ? renderBody() : null} +
+ ); +} + +function countDiffLines(diff: RenderableFileDiff): number { + return diff.hunks.reduce((total, hunk) => total + hunk.lines.length, 0); +} + +function selectVisibleDiffLines( + diff: RenderableFileDiff, + lineLimit: number, + totalLineCount: number, +): VisibleDiff { + if (lineLimit >= totalLineCount) { + return { + hunks: diff.hunks.map((hunk, index) => ({ + hunk, + skippedBefore: index > 0 ? countSkippedUnchangedLines(diff.hunks[index - 1], hunk) : 0, + })), + hiddenLineCount: 0, + }; + } + + let remaining = Math.max(0, lineLimit); + const hunks: VisibleDiffHunk[] = []; + let previousHunk: RenderableFileDiffHunk | null = null; + for (const hunk of diff.hunks) { + if (remaining <= 0) break; + const skippedBefore = previousHunk ? countSkippedUnchangedLines(previousHunk, hunk) : 0; + if (hunk.lines.length <= remaining) { + hunks.push({ hunk, skippedBefore }); + remaining -= hunk.lines.length; + previousHunk = hunk; + continue; + } + hunks.push({ hunk: { ...hunk, lines: hunk.lines.slice(0, remaining) }, skippedBefore }); + remaining = 0; + previousHunk = hunk; + } + return { + hunks, + hiddenLineCount: Math.max(0, totalLineCount - lineLimit), + }; +} + +function countSkippedUnchangedLines( + previous: RenderableFileDiffHunk, + current: RenderableFileDiffHunk, +): number { + const oldGap = current.old_start - (previous.old_start + previous.old_lines); + const newGap = current.new_start - (previous.new_start + previous.new_lines); + return Math.max(0, oldGap, newGap); +} + +function DiffHunkGap({ lineCount }: { lineCount: number }) { + const { t } = useTranslation(); + return ( +
+ + ... + + + {t("message.fileEditUnchangedLinesHidden", { + count: lineCount, + defaultValue: "{{count}} unchanged lines hidden", + })} + +
+ ); +} diff --git a/webui/src/tests/agent-activity-cluster.test.tsx b/webui/src/tests/agent-activity-cluster.test.tsx index 1b0fee11..2ad1117d 100644 --- a/webui/src/tests/agent-activity-cluster.test.tsx +++ b/webui/src/tests/agent-activity-cluster.test.tsx @@ -2,6 +2,7 @@ import { act, fireEvent, render, screen, waitFor } from "@testing-library/react" import { describe, expect, it, vi } from "vitest"; import { AgentActivityCluster } from "@/components/thread/AgentActivityCluster"; +import { DEFAULT_LOCAL_PREFS, writeLocalPreferences } from "@/lib/local-preferences"; import type { CliAppInfo, McpPresetInfo, UIMessage } from "@/lib/types"; const BLENDER_CLI_APP: CliAppInfo = { @@ -448,7 +449,7 @@ describe("AgentActivityCluster", () => { } }); - it("keeps file edits flat even when the legacy diff preference is enabled", () => { + it("renders file edit diffs and responds to preference changes", () => { localStorage.setItem( "nanobot-webui.settings-preferences", JSON.stringify({ fileEditDisplayMode: "diff" }), @@ -488,17 +489,27 @@ describe("AgentActivityCluster", () => { />, ); - expect(screen.queryByTestId("file-edit-diff")).not.toBeInTheDocument(); - expect(screen.queryByText("return ;")).not.toBeInTheDocument(); - expect(screen.queryByText("return ;")).not.toBeInTheDocument(); + expect(screen.getByTestId("file-edit-diff")).toBeInTheDocument(); + expect(screen.getByText("return ;")).toBeInTheDocument(); + expect(screen.getByText("return ;")).toBeInTheDocument(); expect(screen.getByTestId("activity-file-reference")).toHaveTextContent("src/app.tsx"); expect(screen.getAllByTestId("activity-diff-pair")).toHaveLength(1); + + act(() => { + writeLocalPreferences({ ...DEFAULT_LOCAL_PREFS, fileEditDisplayMode: "summary" }); + }); + expect(screen.queryByTestId("file-edit-diff")).not.toBeInTheDocument(); + + act(() => { + writeLocalPreferences({ ...DEFAULT_LOCAL_PREFS, fileEditDisplayMode: "diff" }); + }); + expect(screen.getByTestId("file-edit-diff")).toBeInTheDocument(); } finally { localStorage.removeItem("nanobot-webui.settings-preferences"); } }); - it("does not render diff hunks inside the activity list", () => { + it("renders folded separators between separated file edit hunks", () => { localStorage.setItem( "nanobot-webui.settings-preferences", JSON.stringify({ fileEditDisplayMode: "diff" }), @@ -544,16 +555,18 @@ describe("AgentActivityCluster", () => { />, ); - expect(screen.queryByTestId("file-edit-diff-hunk-gap")).not.toBeInTheDocument(); + expect(screen.getByTestId("file-edit-diff-hunk-gap")).toHaveTextContent( + "21 unchanged lines hidden", + ); expect(screen.queryByText("@@ -25,3 +25,3 @@")).not.toBeInTheDocument(); - expect(screen.queryByText("return newSecond;")).not.toBeInTheDocument(); + expect(screen.getByText("return newSecond;")).toBeInTheDocument(); expect(screen.getByTestId("activity-file-reference")).toHaveTextContent("src/app.tsx"); } finally { localStorage.removeItem("nanobot-webui.settings-preferences"); } }); - it("summarizes long file edit diffs without an expansion control", () => { + it("keeps long file edit diffs collapsed until opened", () => { localStorage.setItem( "nanobot-webui.settings-preferences", JSON.stringify({ fileEditDisplayMode: "diff" }), @@ -592,16 +605,46 @@ describe("AgentActivityCluster", () => { />, ); - expect(screen.queryByTestId("file-edit-diff-toggle")).not.toBeInTheDocument(); + const toggle = screen.getByTestId("file-edit-diff-toggle"); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect(toggle).toHaveTextContent("View large diff"); + expect(toggle).toHaveTextContent("165 lines"); expect(screen.queryByTestId("file-edit-diff")).not.toBeInTheDocument(); expect(screen.queryByText("line-1")).not.toBeInTheDocument(); - expect(screen.getByText("+165")).toBeInTheDocument(); + + fireEvent.click(toggle); + + expect(toggle).toHaveAttribute("aria-expanded", "true"); + expect(screen.getByText("line-160")).toBeInTheDocument(); + expect(screen.queryByText("line-161")).not.toBeInTheDocument(); + expect(screen.getByTestId("file-edit-diff-expand-lines")).toHaveTextContent( + "Show 5 more lines", + ); + + fireEvent.click(screen.getByTestId("file-edit-diff-expand-lines")); + + expect(screen.getByText("line-165")).toBeInTheDocument(); + expect(screen.getByTestId("file-edit-diff-collapse-lines")).toHaveTextContent( + "Show fewer lines", + ); + + fireEvent.click(screen.getByTestId("file-edit-diff-collapse-lines")); + + expect(screen.queryByText("line-165")).not.toBeInTheDocument(); + expect(screen.getByTestId("file-edit-diff-expand-lines")).toHaveTextContent( + "Show 5 more lines", + ); + + fireEvent.click(toggle); + + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect(screen.queryByTestId("file-edit-diff")).not.toBeInTheDocument(); } finally { localStorage.removeItem("nanobot-webui.settings-preferences"); } }); - it("ignores the legacy collapsed diff mode in the activity list", () => { + it("does not mount collapsed file edit diffs until opened", () => { localStorage.setItem( "nanobot-webui.settings-preferences", JSON.stringify({ fileEditDisplayMode: "collapsed_diff" }), @@ -641,16 +684,24 @@ describe("AgentActivityCluster", () => { />, ); - expect(screen.queryByTestId("file-edit-diff-toggle")).not.toBeInTheDocument(); + const toggle = screen.getByTestId("file-edit-diff-toggle"); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect(toggle).toHaveTextContent("View diff"); + expect(toggle).toHaveTextContent("3 lines"); expect(screen.queryByTestId("file-edit-diff")).not.toBeInTheDocument(); expect(screen.queryByText("return ;")).not.toBeInTheDocument(); - expect(screen.getByTestId("activity-file-reference")).toHaveTextContent("src/app.tsx"); + + fireEvent.click(toggle); + + expect(toggle).toHaveAttribute("aria-expanded", "true"); + expect(screen.getByTestId("file-edit-diff")).toBeInTheDocument(); + expect(screen.getByText("return ;")).toBeInTheDocument(); } finally { localStorage.removeItem("nanobot-webui.settings-preferences"); } }); - it("opens the edited file directly instead of expanding a truncated diff", () => { + it("offers the file preview entry point when a diff payload is truncated", () => { localStorage.setItem( "nanobot-webui.settings-preferences", JSON.stringify({ fileEditDisplayMode: "diff" }), @@ -691,9 +742,15 @@ describe("AgentActivityCluster", () => { />, ); - expect(screen.queryByTestId("file-edit-diff-toggle")).not.toBeInTheDocument(); + const toggle = screen.getByTestId("file-edit-diff-toggle"); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect(toggle).toHaveTextContent("View large diff"); expect(screen.queryByTestId("file-edit-diff-truncated")).not.toBeInTheDocument(); - fireEvent.click(screen.getByTestId("activity-file-reference")); + + fireEvent.click(toggle); + + expect(screen.getByTestId("file-edit-diff-truncated")).toHaveTextContent("Diff truncated"); + fireEvent.click(screen.getByTestId("file-edit-diff-open-file")); expect(onOpenFilePreview).toHaveBeenCalledWith("/repo/src/app.tsx"); } finally { @@ -1601,7 +1658,7 @@ describe("AgentActivityCluster", () => { expect(screen.queryByText(/\[Errno 13\]/)).not.toBeInTheDocument(); }); - it("renders repeated edits for the same path as separate actions", () => { + it("keeps repeated edits for the same path as separate actions", () => { localStorage.setItem( "nanobot-webui.settings-preferences", JSON.stringify({ fileEditDisplayMode: "diff" }), @@ -1679,9 +1736,9 @@ describe("AgentActivityCluster", () => { expect(failedRow).toBeInTheDocument(); expect(failedRow).not.toHaveAttribute("title"); expect(screen.queryByText("patch failed")).not.toBeInTheDocument(); - expect(screen.queryByTestId("file-edit-diff")).not.toBeInTheDocument(); - expect(screen.queryByText("")).not.toBeInTheDocument(); - expect(screen.queryByText("const fps = 60;")).not.toBeInTheDocument(); + expect(screen.getAllByTestId("file-edit-diff")).toHaveLength(2); + expect(screen.getByText("")).toBeInTheDocument(); + expect(screen.getByText("const fps = 60;")).toBeInTheDocument(); expect(screen.getAllByText("+2").length).toBeGreaterThan(0); expect(screen.getAllByText("-1").length).toBeGreaterThan(0); expect(screen.getAllByText("+6").length).toBeGreaterThan(0); @@ -1691,6 +1748,50 @@ describe("AgentActivityCluster", () => { } }); + it("keeps the latest failed attempt visible after an earlier edit succeeded", () => { + render( + , + ); + + expect(screen.getByText("Edited")).toBeInTheDocument(); + expect(screen.getByText("Could not edit")).toBeInTheDocument(); + expect(screen.getAllByTestId("activity-file-reference")).toHaveLength(2); + }); + it("keeps tool event embeds out of the flat activity list", () => { render(