From a04b0939596d69cf65365d4b902b072b8f53ad05 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 21 Aug 2026 13:31:46 +0200 Subject: [PATCH] fix(agent-manager): address edit preview review findings --- .../kilo-ui/src/components/message-part.tsx | 18 ++++-- .../kilo-vscode/src/DiffVirtualProvider.ts | 1 + .../tests/unit/edit-preview.test.ts | 31 ++++++++++ .../tests/unit/kilo-ui-contract.test.ts | 7 ++- .../agent-manager/AgentManagerApp.tsx | 18 +++--- .../webview-ui/agent-manager/edit-preview.ts | 59 ++++++++++++++++++- .../diff-viewer/VirtualDiffView.tsx | 11 ++-- .../src/types/messages/permissions.ts | 1 + packages/ui/src/context/data.tsx | 2 + 9 files changed, 124 insertions(+), 24 deletions(-) diff --git a/packages/kilo-ui/src/components/message-part.tsx b/packages/kilo-ui/src/components/message-part.tsx index 9ad72a708b..201aa55b69 100644 --- a/packages/kilo-ui/src/components/message-part.tsx +++ b/packages/kilo-ui/src/components/message-part.tsx @@ -2804,10 +2804,20 @@ ToolRegistry.register({ ? [{ file: file.relativePath, patch: diff.patch, - // Patch metadata reports 0 for some added files; fall back to the - // parsed hunks so the header matches the rendered diff. - additions: diff.additions || diff.fileDiff.additionLines.length, - deletions: diff.deletions || diff.fileDiff.deletionLines.length, + status: + file.type === "add" + ? ("added" as const) + : file.type === "delete" + ? ("deleted" as const) + : ("modified" as const), + additions: + file.type === "add" && diff.additions === 0 + ? diff.fileDiff.hunks.reduce((sum, hunk) => sum + hunk.additionLines, 0) + : diff.additions, + deletions: + file.type === "delete" && diff.deletions === 0 + ? diff.fileDiff.hunks.reduce((sum, hunk) => sum + hunk.deletionLines, 0) + : diff.deletions, }] : [] }) diff --git a/packages/kilo-vscode/src/DiffVirtualProvider.ts b/packages/kilo-vscode/src/DiffVirtualProvider.ts index 6f0661771f..3bc5eeee4c 100644 --- a/packages/kilo-vscode/src/DiffVirtualProvider.ts +++ b/packages/kilo-vscode/src/DiffVirtualProvider.ts @@ -9,6 +9,7 @@ export interface DiffVirtualFile { patch?: string additions: number deletions: number + status?: "added" | "deleted" | "modified" files?: Omit[] initialDiffStyle: "unified" | "split" } diff --git a/packages/kilo-vscode/tests/unit/edit-preview.test.ts b/packages/kilo-vscode/tests/unit/edit-preview.test.ts index 631b9569bd..7d8d07233b 100644 --- a/packages/kilo-vscode/tests/unit/edit-preview.test.ts +++ b/packages/kilo-vscode/tests/unit/edit-preview.test.ts @@ -2,8 +2,11 @@ import { describe, expect, it } from "bun:test" import { createRoot } from "solid-js" import { createEditPreview, + diffCounts, isEditPreviewDiff, previewMatchesContext, + sessionTreeContains, + sessionWorktree, } from "../../webview-ui/agent-manager/edit-preview" const diff = { @@ -90,4 +93,32 @@ describe("Agent Manager edit preview", () => { expect(previewMatchesContext("session-1", "session-1", null, undefined)).toBe(true) expect(previewMatchesContext("session-1", "session-1", "wt-1", undefined)).toBe(false) }) + + it("keeps nested subagent previews in the parent worktree", () => { + const sessions = [ + { id: "parent", parentID: null }, + { id: "child", parentID: "parent" }, + { id: "grandchild", parentID: "child" }, + ] + const managed = [{ id: "parent", worktreeId: "wt-1" }] + + expect(sessionTreeContains("grandchild", "parent", sessions)).toBe(true) + expect(sessionTreeContains("parent", "grandchild", sessions)).toBe(false) + expect(sessionWorktree("grandchild", sessions, managed)).toBe("wt-1") + expect( + previewMatchesContext("grandchild", "parent", "wt-1", "wt-1", (child, root) => + sessionTreeContains(child, root, sessions), + ), + ).toBe(true) + }) + + it("preserves explicit zero counts and excludes hunk context from fallbacks", () => { + const hunks = [{ additionLines: 1, deletionLines: 0 }] + expect(diffCounts({ additions: 0, deletions: 0 }, hunks, "added")).toEqual({ additions: 1, deletions: 0 }) + expect(diffCounts({ additions: 0, deletions: 0 }, hunks, "modified")).toEqual({ additions: 0, deletions: 0 }) + expect(diffCounts({ additions: 4, deletions: 0 }, [{ additionLines: 2, deletionLines: 1 }], "deleted")).toEqual({ + additions: 4, + deletions: 1, + }) + }) }) diff --git a/packages/kilo-vscode/tests/unit/kilo-ui-contract.test.ts b/packages/kilo-vscode/tests/unit/kilo-ui-contract.test.ts index 05e8f0c849..be90e579eb 100644 --- a/packages/kilo-vscode/tests/unit/kilo-ui-contract.test.ts +++ b/packages/kilo-vscode/tests/unit/kilo-ui-contract.test.ts @@ -223,8 +223,11 @@ describe("Write and apply_patch patch rendering contracts (source)", () => { it("apply_patch skips files whose patch has no parsable hunks", () => { expect(patchBlock).toContain("value.fileDiff.hunks.length") - expect(patchBlock).toContain("diff.additions || diff.fileDiff.additionLines.length") - expect(patchBlock).toContain("diff.deletions || diff.fileDiff.deletionLines.length") + expect(patchBlock).toContain('file.type === "add"') + expect(patchBlock).toContain("diff.additions === 0") + expect(patchBlock).toContain("diff.deletions === 0") + expect(patchBlock).toContain("hunk.additionLines") + expect(patchBlock).toContain("hunk.deletionLines") }) it("apply_patch open action preserves every file in a multi-file payload", () => { diff --git a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx index b93433e360..9c961a822f 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx @@ -184,7 +184,12 @@ import { DocumentPanelHost } from "./documents/DocumentPanelHost" import { createDocumentInspector } from "../documents/state" import { attachSubagentEvent, createSubagentController } from "./subagent-tabs" import { EditPreviewPanel } from "./EditPreviewPanel" -import { createAgentManagerEditPreview, createEditPreviewContextGuard } from "./edit-preview" +import { + createAgentManagerEditPreview, + createEditPreviewContextGuard, + sessionTreeContains, + sessionWorktree, +} from "./edit-preview" import { buildShortcutCategories } from "./shortcuts" import { tracker } from "./telemetry" import { createChatFocus, createFocusBridge, createPromptFocus, forgetTerminalFocus, hasQuestionOption } from "./focus" @@ -206,7 +211,6 @@ interface SetupState { type SidebarSelection = typeof LOCAL | string | null export type SidePanelState = SidePanel | null const isMac = typeof navigator !== "undefined" && /Mac|iPhone|iPad/.test(navigator.userAgent) - import { parseBindingTokens } from "./keybind-tokens" import { defaultBindings } from "./keybind-defaults" const AgentManagerContent: Component = () => { @@ -351,7 +355,7 @@ const AgentManagerContent: Component = () => { setHistory, setReviewActive, () => setSidePanel(SidePanel.EditPreview), - () => setSidePanel(null), + () => setSidePanel((prev) => (prev === SidePanel.EditPreview ? null : prev)), diffStyle.style, setSharedDiffStyle, ) @@ -359,8 +363,9 @@ const AgentManagerContent: Component = () => { editPreview.preview, () => session.currentSessionID() ?? undefined, () => selection() ?? null, - (id: string) => managedSessions().find((item) => item.id === id)?.worktreeId ?? undefined, + (id: string) => sessionWorktree(id, session.sessions(), managedSessions()), editPreview.close, + (child, parent) => sessionTreeContains(child, parent, session.sessions()), ) const markdown = createMarkdownRender(vscode) const worktreeStats = () => registry.active().worktreeStats() @@ -383,7 +388,6 @@ const AgentManagerContent: Component = () => { fontFamily: getComputedStyle(document.documentElement).getPropertyValue("--vscode-editor-font-family").trim(), fontSize: readFontSize(), }) - const nsKey = (sel: string) => `${currentProjectId() ?? "single"}:${sel}` const terms = createTerminalState(() => { const sel = selection() @@ -493,7 +497,6 @@ const AgentManagerContent: Component = () => { setSidePanel, }) const cancelAmbientSetup = ambientSetup.cancel - const [pendingDelete, setPendingDelete] = createSignal(null) let pendingDeleteTimer: ReturnType | undefined const cancelPendingDelete = () => { @@ -512,11 +515,8 @@ const AgentManagerContent: Component = () => { ), ) onCleanup(() => clearTimeout(pendingDeleteTimer)) - const tabMemory = () => registry.active().tabMemory.all() - const reviewOpen = createMemo(() => selection() !== null && reviewOpenByContext()[selection()!] === true) - const setReviewOpenForContext = (context: string, open: boolean) => { setReviewOpenByContext((prev) => { if (prev[context] === open) return prev diff --git a/packages/kilo-vscode/webview-ui/agent-manager/edit-preview.ts b/packages/kilo-vscode/webview-ui/agent-manager/edit-preview.ts index 88eca77dd6..1fe7bb796f 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/edit-preview.ts +++ b/packages/kilo-vscode/webview-ui/agent-manager/edit-preview.ts @@ -10,13 +10,61 @@ export interface EditPreview { markdown: boolean } +interface SessionLike { + id: string + parentID?: string | null +} + +interface ManagedLike { + id: string + worktreeId?: string | null +} + +export function sessionTreeContains(id: string, root: string, sessions: SessionLike[]): boolean { + const seen = new Set() + let current: string | undefined = id + while (current && !seen.has(current)) { + if (current === root) return true + seen.add(current) + current = sessions.find((item) => item.id === current)?.parentID ?? undefined + } + return false +} + +export function sessionWorktree(id: string, sessions: SessionLike[], managed: ManagedLike[]): string | undefined { + const seen = new Set() + let current: string | undefined = id + while (current && !seen.has(current)) { + const worktree = managed.find((item) => item.id === current)?.worktreeId + if (worktree) return worktree + seen.add(current) + current = sessions.find((item) => item.id === current)?.parentID ?? undefined + } + return undefined +} + +export function diffCounts( + diff: Pick, + hunks: Array<{ additionLines: number; deletionLines: number }>, + status?: PermissionFileDiff["status"], +) { + const additions = hunks.reduce((sum, hunk) => sum + hunk.additionLines, 0) + const deletions = hunks.reduce((sum, hunk) => sum + hunk.deletionLines, 0) + return { + additions: status === "added" && diff.additions === 0 ? additions : diff.additions, + deletions: status === "deleted" && diff.deletions === 0 ? deletions : diff.deletions, + } +} + export function previewMatchesContext( previewSessionID: string | undefined, currentSessionID: string | null | undefined, selection: string | null | undefined, worktreeID: string | undefined, + related?: (previewSessionID: string, currentSessionID: string) => boolean, ): boolean { - if (!previewSessionID || previewSessionID !== currentSessionID) return false + if (!previewSessionID || !currentSessionID) return false + if (previewSessionID !== currentSessionID && !related?.(previewSessionID, currentSessionID)) return false if (worktreeID) return worktreeID === selection return selection === LOCAL || selection === null } @@ -27,17 +75,22 @@ export function createEditPreviewContextGuard( selection: Accessor, owner: (sessionID: string) => string | undefined, close: () => void, + related?: (previewSessionID: string, currentSessionID: string) => boolean, ) { createEffect( on( () => { const item = preview() const worktree = item?.sessionID ? owner(item.sessionID) : undefined - return `${item?.sessionID ?? ""}:${current() ?? ""}:${selection() ?? "unassigned"}:${worktree ?? "local"}` + const currentID = current() + const nested = + item?.sessionID && currentID ? (related?.(item.sessionID, currentID) === true ? "nested" : "same") : "none" + return `${item?.sessionID ?? ""}:${current() ?? ""}:${selection() ?? "unassigned"}:${worktree ?? "local"}:${nested}` }, () => { const item = preview() - if (item && !previewMatchesContext(item.sessionID, current(), selection(), owner(item.sessionID!))) close() + if (item && !previewMatchesContext(item.sessionID, current(), selection(), owner(item.sessionID!), related)) + close() }, { defer: true }, ), diff --git a/packages/kilo-vscode/webview-ui/diff-viewer/VirtualDiffView.tsx b/packages/kilo-vscode/webview-ui/diff-viewer/VirtualDiffView.tsx index e5722e9e52..b6c399364c 100644 --- a/packages/kilo-vscode/webview-ui/diff-viewer/VirtualDiffView.tsx +++ b/packages/kilo-vscode/webview-ui/diff-viewer/VirtualDiffView.tsx @@ -8,12 +8,14 @@ import { normalize } from "@kilocode/kilo-ui/session-diff" import { useLanguage } from "../src/context/language" import { EXTREME_DIFF_CHANGED_LINES } from "./diff-open-policy" import { isMarkdownFile, MarkdownDiffView } from "./MarkdownDiffView" +import { diffCounts } from "../agent-manager/edit-preview" export interface VirtualDiffFile { file: string patch?: string additions: number deletions: number + status?: "added" | "deleted" | "modified" files?: VirtualDiffFile[] } @@ -65,15 +67,12 @@ export const VirtualDiffView: Component = (props) => { return value }) - // Provided counts win, but added files often report 0 while the patch has - // real lines, so the parsed hunks are the fallback. + // Provided counts win, but some added files report 0 while the patch has + // real changed lines. Hunk counts exclude context lines. const counts = createMemo(() => { const value = view() if (!value) return { additions: props.diff.additions, deletions: props.diff.deletions } - return { - additions: props.diff.additions || value.fileDiff.additionLines.length, - deletions: props.diff.deletions || value.fileDiff.deletionLines.length, - } + return diffCounts(props.diff, value.fileDiff.hunks, props.diff.status) }) // Hunk-bounded patches render fully and let the surrounding list scroll, so a diff --git a/packages/kilo-vscode/webview-ui/src/types/messages/permissions.ts b/packages/kilo-vscode/webview-ui/src/types/messages/permissions.ts index d00118e1f6..ae4c4bb5f1 100644 --- a/packages/kilo-vscode/webview-ui/src/types/messages/permissions.ts +++ b/packages/kilo-vscode/webview-ui/src/types/messages/permissions.ts @@ -18,6 +18,7 @@ export interface PermissionFileDiff { patch?: string additions: number deletions: number + status?: "added" | "deleted" | "modified" files?: PermissionFileDiff[] } diff --git a/packages/ui/src/context/data.tsx b/packages/ui/src/context/data.tsx index 1013034556..2d4921a6a6 100644 --- a/packages/ui/src/context/data.tsx +++ b/packages/ui/src/context/data.tsx @@ -55,6 +55,7 @@ export type OpenDiffFn = (diff: { patch?: string // kilocode_change additions: number deletions: number + status?: "added" | "deleted" | "modified" // kilocode_change // kilocode_change start - multi-file patch preview payload files?: Array<{ file: string @@ -63,6 +64,7 @@ export type OpenDiffFn = (diff: { patch?: string additions: number deletions: number + status?: "added" | "deleted" | "modified" // kilocode_change }> // kilocode_change end }) => void