fix(agent-manager): address edit preview review findings

This commit is contained in:
marius-kilocode
2026-08-21 13:31:46 +02:00
parent 35d9933eb6
commit a04b093959
9 changed files with 124 additions and 24 deletions
@@ -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,
}]
: []
})
@@ -9,6 +9,7 @@ export interface DiffVirtualFile {
patch?: string
additions: number
deletions: number
status?: "added" | "deleted" | "modified"
files?: Omit<DiffVirtualFile, "files" | "initialDiffStyle">[]
initialDiffStyle: "unified" | "split"
}
@@ -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,
})
})
})
@@ -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", () => {
@@ -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<string | null>(null)
let pendingDeleteTimer: ReturnType<typeof setTimeout> | 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
@@ -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<string>()
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<string>()
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<PermissionFileDiff, "additions" | "deletions">,
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<string | null | undefined>,
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 },
),
@@ -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<VirtualDiffViewProps> = (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
@@ -18,6 +18,7 @@ export interface PermissionFileDiff {
patch?: string
additions: number
deletions: number
status?: "added" | "deleted" | "modified"
files?: PermissionFileDiff[]
}
+2
View File
@@ -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