fix(app): align the scrolling diff order with the Changes tree (#3913)

The Changes view listed the same files in two different orders. The
tree rail put directories before files at each level of the path.
The scrolling diff compared the full path byte by byte. A root file
such as .gitlab-ci.yml therefore opened the diff, but sat below every
directory in the tree. Scrolling the diff did not walk the tree from
top to bottom.

Two sorts caused this. sortTree in git/diff-tree.ts sorted the tree.
compareCheckoutDiffPaths in git/diff-order.ts sorted the flat list at
ingest.

The tree is now the only ordering authority.
orderCheckoutDiffFiles builds the tree and reads the file sequence
back out of it. compareCheckoutDiffPaths is deleted, so the two
surfaces can no longer disagree.

The daemon keeps its own whole-path sort, so daemons that are not
updated also get this fix. Commit diffs keep the order that git
gives them, because that panel shows no tree rail.
This commit is contained in:
Christoph Leiter
2026-08-27 08:00:25 +00:00
committed by GitHub
parent 657152c47a
commit 2755596b9f
4 changed files with 96 additions and 30 deletions
@@ -19,6 +19,10 @@ interface WorkspaceFixtureOptions {
includeDeletedFile?: boolean;
includeNestedFolders?: boolean;
includeRenamedFile?: boolean;
// A root-level file whose name sorts BEFORE the root-level "src" directory.
// zz-untracked.txt cannot stand in for it: "src" < "zz", so that file lands
// last whether paths are compared whole or segment by segment.
includeRootFileSortingFirst?: boolean;
includeUntrackedFile?: boolean;
}
@@ -461,6 +465,31 @@ test("changes tree aligns every file status after its diff stat", async ({ page
expect(statusBounds.x - (statBounds.x + statBounds.width)).toBeGreaterThanOrEqual(8);
});
test("the scrolling diff lists files in changes tree order", async ({ page }) => {
const workspace = await createWorkspaceWithMountedTabDiff({
includeNestedFolders: true,
includeRootFileSortingFirst: true,
});
await useUnwrappedDiffLines(page);
await openWorkspaceChanges(page, workspace);
const tree = changesTree(page);
const treeNames = tree.locator('[data-testid^="diff-tree-file-"][data-testid$="-name"]');
const diffNames = page.locator('[data-testid^="diff-file-"][data-testid$="-name"]');
// Directories sort before files at every level, so the root note lands last
// even though "a-root-note.txt" compares below "src/..." as a whole string.
const expected = ["changed.ts", "root.ts", "use-mounted-tab-set.ts", "a-root-note.txt"];
// Equal counts also prove no folder is collapsed: flattenDiffTree drops the
// descendants of a collapsed folder, which would make the tree a subsequence
// of the diff rather than a match, and mask a real ordering difference.
await expect(treeNames).toHaveCount(expected.length);
await expect(diffNames).toHaveCount(expected.length);
await expect(treeNames).toHaveText(expected);
await expect(diffNames).toHaveText(expected);
});
test("changes context menu recursively collapses descendant folders", async ({ page }) => {
const workspace = await createWorkspaceWithMountedTabDiff({ includeNestedFolders: true });
await useUnwrappedDiffLines(page);
@@ -1285,6 +1314,9 @@ async function createWorkspaceWithMountedTabDiff(
if (options.includeUntrackedFile) {
await writeFile(path.join(repo.path, "zz-untracked.txt"), "remove me\n");
}
if (options.includeRootFileSortingFirst) {
await writeFile(path.join(repo.path, "a-root-note.txt"), "root note\n");
}
if (options.includeDeletedFile) {
await unlink(path.join(repo.path, "src/zz-deleted.ts"));
}
+42 -12
View File
@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { compareCheckoutDiffPaths, orderCheckoutDiffFiles } from "./diff-order";
import { orderCheckoutDiffFiles } from "./diff-order";
function createFile(path: string, additions = 0) {
return {
@@ -12,21 +12,51 @@ function createFile(path: string, additions = 0) {
};
}
function order(paths: string[]): string[] {
return orderCheckoutDiffFiles(paths.map((path) => createFile(path))).map((file) => file.path);
}
describe("checkout diff ordering", () => {
it("compares paths deterministically", () => {
expect(compareCheckoutDiffPaths("a.ts", "b.ts")).toBeLessThan(0);
expect(compareCheckoutDiffPaths("b.ts", "a.ts")).toBeGreaterThan(0);
expect(compareCheckoutDiffPaths("same.ts", "same.ts")).toBe(0);
it("sorts sibling files by name", () => {
expect(order(["zeta.ts", "alpha.ts", "beta.ts"])).toEqual(["alpha.ts", "beta.ts", "zeta.ts"]);
});
it("sorts files by path", () => {
const ordered = orderCheckoutDiffFiles([
createFile("zeta.ts"),
createFile("alpha.ts"),
createFile("beta.ts"),
]);
it("puts a directory before a file that sits beside it", () => {
expect(order(["z.ts", "a/b.ts"])).toEqual(["a/b.ts", "z.ts"]);
});
expect(ordered.map((file) => file.path)).toEqual(["alpha.ts", "beta.ts", "zeta.ts"]);
it("keeps loose root files below root directories", () => {
// The reported bug: comparing whole paths byte-for-byte put ".gitlab-ci.yml"
// first (because "." < "b"), while the tree rail listed it last.
expect(
order([
".gitlab-ci.yml",
"process-compose.yaml",
"bin/lib/node-stamp.sh",
"samera-one/package.json",
]),
).toEqual([
"bin/lib/node-stamp.sh",
"samera-one/package.json",
".gitlab-ci.yml",
"process-compose.yaml",
]);
});
it("ranks a nested directory above a shallower file with the same prefix", () => {
// "src/a.ts" < "src/a/z.ts" as whole strings, but "a" is a directory here.
expect(order(["src/a.ts", "src/a/z.ts"])).toEqual(["src/a/z.ts", "src/a.ts"]);
});
it("orders a directory ahead of a file sharing its name", () => {
// Converting an extensionless file into a directory puts both in one diff.
expect(order(["bin/stack", "bin/stack/run"])).toEqual(["bin/stack/run", "bin/stack"]);
});
it("returns lists too short to reorder untouched", () => {
const single = [createFile("only.ts")];
expect(orderCheckoutDiffFiles([])).toEqual([]);
expect(orderCheckoutDiffFiles(single)).toBe(single);
});
it("preserves relative order for equal paths", () => {
+12 -13
View File
@@ -1,19 +1,18 @@
import type { SubscribeCheckoutDiffResponse } from "@getpaseo/protocol/messages";
type ParsedDiffFile = SubscribeCheckoutDiffResponse["payload"]["files"][number];
export function compareCheckoutDiffPaths(left: string, right: string): number {
if (left === right) {
return 0;
}
return left < right ? -1 : 1;
}
import type { ParsedDiffFile } from "@getpaseo/protocol/messages";
import { buildDiffTree, flattenDiffTree } from "@/git/diff-tree";
// The Changes tree is the single ordering authority: `sortTree` (diff-tree.ts)
// ranks directories before files within a level, then compares names. Surfaces
// that render the flat file list — the scrolling diff — derive their order from
// the tree here instead of reimplementing the rule, so the two cannot disagree.
export function orderCheckoutDiffFiles(files: ParsedDiffFile[]): ParsedDiffFile[] {
if (files.length < 2) {
return files;
}
const ordered = [...files];
ordered.sort((a, b) => compareCheckoutDiffPaths(a.path, b.path));
return ordered;
// Path compression only merges display rows for single-child directory
// chains, so it is skipped; the collapsed set is empty because the flat list
// always carries every file, whatever the rail has collapsed.
return flattenDiffTree(buildDiffTree(files), new Set()).flatMap((row) =>
row.kind === "file" ? [row.file] : [],
);
}
+10 -5
View File
@@ -1,8 +1,13 @@
import type { ParsedDiffFile } from "@/git/use-diff-query";
// Builds a directory hierarchy from the flat, path-sorted `ParsedDiffFile[]` the
// Changes view renders. The tree renders on every form factor, consistent with
// the Files explorer.
// Builds a directory hierarchy from the `ParsedDiffFile[]` the Changes view
// renders. The tree renders on every form factor, consistent with the Files
// explorer.
//
// `sortTree` below is the single ordering authority for the Changes view. The
// flat scrolling diff does not sort for itself — `orderCheckoutDiffFiles`
// (diff-order.ts) builds this tree and reads its file sequence back out — so
// changing `sortTree` reorders both surfaces together, by construction.
//
// Directory nodes are keyed by their FULL uncompressed path (e.g. "packages/app/src").
// That path is the stable identity used to persist folder-collapse state, so the
@@ -55,8 +60,8 @@ function sortTree(node: DiffTreeDirNode): void {
// directories before files within a level
return a.kind === "dir" ? -1 : 1;
}
// Plain ASCII comparison, matching compareCheckoutDiffPaths (diff-order.ts)
// so the tree order is consistent with the rest of the Changes view.
// Plain ASCII, not localeCompare: the order must not shift with the
// device locale, and every surface derives from this comparison.
if (a.name === b.name) return 0;
return a.name < b.name ? -1 : 1;
});