mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-10-29 18:52:45 +08:00
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.
This commit is contained in:
@@ -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"])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<DiffPanelProps> = (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<DiffPanelProps> = (props) => {
|
||||
<Show when={props.diffs.length > 0}>
|
||||
<div class="am-diff-content" data-component="session-review" ref={scroller}>
|
||||
<Accordion multiple value={open()} onChange={setOpen}>
|
||||
<For each={props.diffs}>
|
||||
<For each={sorted()}>
|
||||
{(diff) => {
|
||||
const isAdded = () => diff.status === "added"
|
||||
const isDeleted = () => diff.status === "deleted"
|
||||
|
||||
@@ -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<FullScreenDiffViewProps> = (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<FullScreenDiffViewProps> = (props) =>
|
||||
<Show when={props.diffs.length > 0}>
|
||||
<div class="am-review-diff-content" data-component="session-review">
|
||||
<Accordion multiple value={open()} onChange={setOpen}>
|
||||
<For each={props.diffs}>
|
||||
<For each={sorted()}>
|
||||
{(diff) => {
|
||||
const isAdded = () => diff.status === "added"
|
||||
const isDeleted = () => diff.status === "deleted"
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user