From 57aa3396f96df7bb0165c686e1734bfa36d4089a Mon Sep 17 00:00:00 2001 From: Marius Date: Fri, 27 Mar 2026 17:21:10 +0100 Subject: [PATCH] fix(agent-manager): align diff panel file order with file tree (#7779) buildFileTree now sorts children at every level (directories first, then files, both alphabetically). Both DiffPanel and FullScreenDiffView use a treeOrder() memo so the accordion order matches the tree sidebar. Previously the diff panel rendered files in git's flat path order with untracked files appended at the end, causing newly-added files to appear far from their directory siblings in the tree. --- .../kilo-vscode/tests/unit/file-tree.test.ts | 120 ++++++++++++++++-- .../webview-ui/agent-manager/DiffPanel.tsx | 7 +- .../agent-manager/FullScreenDiffView.tsx | 7 +- .../agent-manager/file-tree-utils.ts | 29 +++++ 4 files changed, 149 insertions(+), 14 deletions(-) diff --git a/packages/kilo-vscode/tests/unit/file-tree.test.ts b/packages/kilo-vscode/tests/unit/file-tree.test.ts index b997f86cfd4..a54908dab20 100644 --- a/packages/kilo-vscode/tests/unit/file-tree.test.ts +++ b/packages/kilo-vscode/tests/unit/file-tree.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect } from "bun:test" -import { buildFileTree, flatten, flattenChain, type FileTreeNode } from "../../webview-ui/agent-manager/file-tree-utils" +import { + buildFileTree, + flatten, + flattenChain, + treeOrder, + type FileTreeNode, +} from "../../webview-ui/agent-manager/file-tree-utils" import type { WorktreeFileDiff } from "../../webview-ui/src/types/messages" function diff(file: string, status?: "added" | "deleted" | "modified"): WorktreeFileDiff { @@ -13,13 +19,13 @@ describe("buildFileTree", () => { expect(buildFileTree([])).toEqual([]) }) - it("places root-level files at the top", () => { + it("sorts root-level files alphabetically", () => { const tree = buildFileTree([diff("README.md"), diff("package.json")]) expect(tree).toHaveLength(2) - expect(tree[0]!.name).toBe("README.md") - expect(tree[0]!.path).toBe("README.md") + expect(tree[0]!.name).toBe("package.json") + expect(tree[0]!.path).toBe("package.json") expect(tree[0]!.children).toBeUndefined() - expect(tree[1]!.name).toBe("package.json") + expect(tree[1]!.name).toBe("README.md") }) it("groups files under shared directories", () => { @@ -66,15 +72,15 @@ describe("buildFileTree", () => { expect(tree[0]!.children![0]!.children![0]!.children![0]!.children![0]!.name).toBe("e.ts") }) - it("mixes root-level files with directory files", () => { + it("sorts directories before files at each level", () => { const tree = buildFileTree([diff("README.md"), diff("src/index.ts"), diff("test/index.test.ts")]) - expect(tree).toHaveLength(3) // README.md, src/, test/ - expect(tree[0]!.name).toBe("README.md") - expect(tree[0]!.children).toBeUndefined() - expect(tree[1]!.name).toBe("src") + expect(tree).toHaveLength(3) // src/, test/, README.md + expect(tree[0]!.name).toBe("src") + expect(tree[0]!.children).toHaveLength(1) + expect(tree[1]!.name).toBe("test") expect(tree[1]!.children).toHaveLength(1) - expect(tree[2]!.name).toBe("test") - expect(tree[2]!.children).toHaveLength(1) + expect(tree[2]!.name).toBe("README.md") + expect(tree[2]!.children).toBeUndefined() }) it("attaches diff data to leaf nodes", () => { @@ -89,6 +95,17 @@ describe("buildFileTree", () => { expect(tree[0]!.diff).toBeUndefined() }) + it("sorts directories before files within each level", () => { + const tree = buildFileTree([diff("src/a.ts"), diff("src/b/c.ts")]) + const src = tree[0]! + expect(src.children).toHaveLength(2) + // Directory before file + expect(src.children![0]!.name).toBe("b") + expect(src.children![0]!.children).toBeDefined() + expect(src.children![1]!.name).toBe("a.ts") + expect(src.children![1]!.children).toBeUndefined() + }) + it("separates diverging paths with common prefix", () => { const tree = buildFileTree([diff("src/a.ts"), diff("src/b/c.ts")]) const src = tree[0]! @@ -310,3 +327,82 @@ describe("flatten", () => { expect(result[0]!.children![0]!.name).toBe("index.ts") }) }) + +// ── treeOrder ────────────────────────────────────────────────────────────── + +describe("treeOrder", () => { + it("returns empty array for empty input", () => { + expect(treeOrder([])).toEqual([]) + }) + + it("returns single-element array unchanged", () => { + const input = [diff("a.ts")] + expect(treeOrder(input)).toEqual(input) + }) + + it("groups files by directory", () => { + // Git lexicographic order interleaves directories: a/x, b/y, a/z + // but buildFileTree groups by directory: a/{x,z}, b/{y} + const input = [diff("a/x.ts"), diff("b/y.ts"), diff("a/z.ts")] + const result = treeOrder(input) + expect(result.map((d) => d.file)).toEqual(["a/x.ts", "a/z.ts", "b/y.ts"]) + }) + + it("groups untracked files with their directory siblings", () => { + // The actual bug: backend appends untracked/new files AFTER all tracked + // files. settings-io.ts (untracked, added) ends up after all the tracked + // i18n modifications, but should appear with its directory siblings. + const input = [ + diff("src/components/settings/about.ts"), // tracked modification + diff("src/i18n/ar.ts"), // tracked modification + diff("src/i18n/de.ts"), // tracked modification + diff("src/i18n/en.ts"), // tracked modification + diff("src/components/settings/settings-io.ts"), // untracked, appended last + diff("tests/unit/settings-io.test.ts"), // untracked, appended last + ] + const result = treeOrder(input) + // Tree groups by directory: components/settings/ together, i18n/ together + // Directories sorted alphabetically: components/ before i18n/ before tests/ + expect(result.map((d) => d.file)).toEqual([ + "src/components/settings/about.ts", + "src/components/settings/settings-io.ts", // now grouped with about.ts! + "src/i18n/ar.ts", + "src/i18n/de.ts", + "src/i18n/en.ts", + "tests/unit/settings-io.test.ts", + ]) + }) + + it("reorders when git path sort diverges from tree grouping", () => { + // Files from the same directory are scattered in the input (e.g. because + // untracked files are appended after tracked files) + const input = [ + diff("pkg/tests/a.test.ts"), + diff("pkg/webview/src/components/x.ts"), + diff("pkg/webview/src/i18n/ar.ts"), + diff("pkg/webview/src/i18n/de.ts"), + diff("pkg/tests/b.test.ts"), + ] + const result = treeOrder(input) + // Tree groups and sorts: tests/{a,b}, webview/src/{components/x, i18n/{ar,de}} + expect(result.map((d) => d.file)).toEqual([ + "pkg/tests/a.test.ts", + "pkg/tests/b.test.ts", + "pkg/webview/src/components/x.ts", + "pkg/webview/src/i18n/ar.ts", + "pkg/webview/src/i18n/de.ts", + ]) + }) + + it("sorts root-level files alphabetically", () => { + const input = [diff("README.md"), diff("package.json"), diff("tsconfig.json")] + const result = treeOrder(input) + expect(result.map((d) => d.file)).toEqual(["package.json", "README.md", "tsconfig.json"]) + }) + + it("puts directories before files at each level", () => { + const input = [diff("README.md"), diff("src/a.ts"), diff("src/b.ts"), diff("tsconfig.json")] + const result = treeOrder(input) + expect(result.map((d) => d.file)).toEqual(["src/a.ts", "src/b.ts", "README.md", "tsconfig.json"]) + }) +}) diff --git a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx index 96963fac288..3c00d20d5cd 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx @@ -22,6 +22,7 @@ import { } from "./review-annotations" import { LONG_DIFF_MARKER_FILE_COUNT, initialOpenFiles, isLargeDiffFile } from "./diff-open-policy" import { DiffEndMarker } from "./DiffEndMarker" +import { treeOrder } from "./file-tree-utils" // --- Data model --- @@ -67,6 +68,10 @@ export const DiffPanel: Component = (props) => { // only pruning happens so the user's manual collapse state is preserved. let initializedKey: string | undefined + // Reorder diffs to match the file-tree's depth-first visual order so + // scrolling through the accordion matches the tree grouping. + const sorted = createMemo(() => treeOrder(props.diffs)) + const comments = () => props.comments const setComments = (next: ReviewComment[]) => props.onCommentsChange(next) const updateComments = (updater: (prev: ReviewComment[]) => ReviewComment[]) => setComments(updater(comments())) @@ -380,7 +385,7 @@ export const DiffPanel: Component = (props) => { 0}>
- + {(diff) => { const isAdded = () => diff.status === "added" const isDeleted = () => diff.status === "deleted" diff --git a/packages/kilo-vscode/webview-ui/agent-manager/FullScreenDiffView.tsx b/packages/kilo-vscode/webview-ui/agent-manager/FullScreenDiffView.tsx index 03344ca8a11..041b1fbf142 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/FullScreenDiffView.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/FullScreenDiffView.tsx @@ -15,6 +15,7 @@ import type { DiffLineAnnotation, AnnotationSide, SelectedLineRange } from "@pie import type { WorktreeFileDiff } from "../src/types/messages" import { useLanguage } from "../src/context/language" import { FileTree } from "./FileTree" +import { treeOrder } from "./file-tree-utils" import { getDirectory, getFilename, lineCount, sanitizeReviewComments, type ReviewComment } from "./review-comments" import { buildFileAnnotations, @@ -74,6 +75,10 @@ export const FullScreenDiffView: Component = (props) => let scrollRef: HTMLDivElement | undefined let syncFrame: number | undefined + // Reorder diffs to match the file-tree's depth-first visual order so + // scrolling through the diff panel matches the tree on the left. + const sorted = createMemo(() => treeOrder(props.diffs)) + const comments = () => props.comments const setComments = (next: ReviewComment[]) => props.onCommentsChange(next) const updateComments = (updater: (prev: ReviewComment[]) => ReviewComment[]) => setComments(updater(comments())) @@ -481,7 +486,7 @@ export const FullScreenDiffView: Component = (props) => 0}>
- + {(diff) => { const isAdded = () => diff.status === "added" const isDeleted = () => diff.status === "deleted" diff --git a/packages/kilo-vscode/webview-ui/agent-manager/file-tree-utils.ts b/packages/kilo-vscode/webview-ui/agent-manager/file-tree-utils.ts index 3762d5993d4..a45c1ef9cc0 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/file-tree-utils.ts +++ b/packages/kilo-vscode/webview-ui/agent-manager/file-tree-utils.ts @@ -33,9 +33,23 @@ export function buildFileTree(diffs: WorktreeFileDiff[]): FileTreeNode[] { parent.push({ name: filename, path: diff.file, diff }) } + sortTree(root) return root } +/** Sort children at every level: directories first (alphabetically), then files (alphabetically). */ +function sortTree(nodes: FileTreeNode[]) { + nodes.sort((a, b) => { + const aDir = a.children ? 0 : 1 + const bDir = b.children ? 0 : 1 + if (aDir !== bDir) return aDir - bDir + return a.name.localeCompare(b.name) + }) + for (const node of nodes) { + if (node.children) sortTree(node.children) + } +} + // Flatten single-child directory chains: src/components/ instead of src > components export function flatten(nodes: FileTreeNode[]): FileTreeNode[] { return nodes.map((node) => { @@ -51,3 +65,18 @@ export function flattenChain(node: FileTreeNode): FileTreeNode { if (!child.children) return node return flattenChain({ name: `${node.name}/${child.name}`, path: child.path, children: child.children }) } + +/** Return diffs sorted to match the tree's depth-first visual order. */ +export function treeOrder(diffs: WorktreeFileDiff[]): WorktreeFileDiff[] { + if (diffs.length <= 1) return diffs + const tree = buildFileTree(diffs) + const result: WorktreeFileDiff[] = [] + function walk(nodes: FileTreeNode[]) { + for (const node of nodes) { + if (node.diff) result.push(node.diff) + if (node.children) walk(node.children) + } + } + walk(tree) + return result +}