refactor(code-review): share tree traversal order, drop re-export indirection

Cleanup pass on the diff-review-matches-file-tree change:

- Extract orderedTreeDirs/orderedTreeFiles into the shared changeTree
  helper and have both flattenNode (review ordering) and ChangesTreeView
  (the rendered tree) consume them. The two surfaces now share one
  traversal, so they can't drift on sort order again — the original bug.
- Drop the orderPathsLikeTree/sortReviewItemsByTreeOrder re-export
  aliases from reviewItemBuilders; ReviewPage imports the core helpers
  directly. Removes a layer of indirection with no consumer relying on
  the renamed names.
- Drop the unused TreeNode/buildChangesTree/compactTree back-compat
  aliases from ChangesTreeView; it uses the core names directly.

Net -73 lines; behavior unchanged.

Generated-By: PostHog Code
Task-Id: 4eb23888-6afa-4fb9-9b7a-22d6a48e1721

Author: adboio

packages/core/src/git-interaction/changeTree.ts

@@ -55,20 +55,25 @@ export function compactChangeTree(node: ChangeTreeNode): ChangeTreeNode {
 
 const compareLocale = (a: string, b: string) => a.localeCompare(b);
 
-function flattenNode(node: ChangeTreeNode, out: ChangedFile[]) {
-  const sortedDirs = [...node.children.values()].sort((a, b) =>
+export function orderedTreeDirs(node: ChangeTreeNode): ChangeTreeNode[] {
+  return [...node.children.values()].sort((a, b) =>
     compareLocale(a.name, b.name),
   );
-  const sortedFiles = [...node.files].sort((a, b) => {
+}
+
+export function orderedTreeFiles(node: ChangeTreeNode): ChangedFile[] {
+  return [...node.files].sort((a, b) => {
     const aName = a.path.split("/").pop() ?? "";
     const bName = b.path.split("/").pop() ?? "";
     return compareLocale(aName, bName);
   });
+}
 
-  for (const child of sortedDirs) {
+function flattenNode(node: ChangeTreeNode, out: ChangedFile[]) {
+  for (const child of orderedTreeDirs(node)) {
     flattenNode(child, out);
   }
-  for (const file of sortedFiles) {
+  for (const file of orderedTreeFiles(node)) {
     out.push(file);
   }
 }

packages/ui/src/features/code-review/components/ReviewPage.tsx

@@ -1,6 +1,10 @@
 import type { parsePatchFiles } from "@pierre/diffs";
 import type { ResolvedDiffSource } from "@posthog/core/code-review/resolveDiffSource";
 import type { PrCommentThread } from "@posthog/core/code-review/types";
+import {
+  orderPathsLikeChangeTree,
+  sortByChangeTreeOrder,
+} from "@posthog/core/git-interaction/changeTree";
 import { useHostTRPC } from "@posthog/host-router/react";
 import type { ChangedFile, Task } from "@posthog/shared/domain-types";
 import { Flex, Text } from "@radix-ui/themes";
@@ -33,9 +37,7 @@ import {
   buildRemoteReviewItems,
   buildUntrackedReviewItems,
   changedFileSignature,
-  orderPathsLikeTree,
   patchFileSignature,
-  sortReviewItemsByTreeOrder,
 } from "./reviewItemBuilders";
 
 const EMPTY_CHANGED_FILES: ChangedFile[] = [];
@@ -348,7 +350,7 @@ function LocalReviewContent({
 
   const stagedItems = useMemo(
     () =>
-      sortReviewItemsByTreeOrder(
+      sortByChangeTreeOrder(
         buildPatchReviewItems({
           files: stagedParsedFiles,
           staged: true,
@@ -363,7 +365,7 @@ function LocalReviewContent({
           prUrl,
           commentThreads,
         }),
-        orderPathsLikeTree(
+        orderPathsLikeChangeTree(
           stagedParsedFiles.map((f) => f.name ?? f.prevName ?? ""),
         ),
       ),
@@ -384,7 +386,7 @@ function LocalReviewContent({
 
   const changesItems = useMemo(
     () =>
-      sortReviewItemsByTreeOrder(
+      sortByChangeTreeOrder(
         [
           ...buildPatchReviewItems({
             files: unstagedParsedFiles,
@@ -411,7 +413,7 @@ function LocalReviewContent({
             onStageFile,
           }),
         ],
-        orderPathsLikeTree([
+        orderPathsLikeChangeTree([
           ...unstagedParsedFiles.map((f) => f.name ?? f.prevName ?? ""),
           ...untrackedFiles.map((f) => f.path),
         ]),
@@ -552,7 +554,7 @@ function RemoteReviewPage({
 
   const items = useMemo(
     () =>
-      sortReviewItemsByTreeOrder(
+      sortByChangeTreeOrder(
         buildRemoteReviewItems({
           files,
           taskId,
@@ -562,7 +564,7 @@ function RemoteReviewPage({
           toggleFile: reviewState.toggleFile,
           commentThreads,
         }),
-        orderPathsLikeTree(files.map((f) => f.path)),
+        orderPathsLikeChangeTree(files.map((f) => f.path)),
       ),
     [
       commentThreads,

packages/ui/src/features/code-review/components/reviewItemBuilders.test.ts

@@ -1,12 +1,6 @@
 import type { ChangedFile } from "@posthog/shared/domain-types";
 import { describe, expect, it } from "vitest";
-import type { ReviewListItem } from "../commentFileFilter";
-import {
-  changedFileSignature,
-  orderPathsLikeTree,
-  patchFileSignature,
-  sortReviewItemsByTreeOrder,
-} from "./reviewItemBuilders";
+import { changedFileSignature, patchFileSignature } from "./reviewItemBuilders";
 
 const changedFile = (over: Partial<ChangedFile>): ChangedFile => ({
   path: "a.ts",
@@ -81,58 +75,3 @@ describe("patchFileSignature", () => {
     expect(a).not.toBe(b);
   });
 });
-
-describe("orderPathsLikeTree", () => {
-  it("orders paths like the file tree, not git byte order", () => {
-    expect(
-      orderPathsLikeTree([
-        "Beta.txt",
-        "ZETA_CAPS.txt",
-        "alpha.txt",
-        "src/Apple.ts",
-        "src/beta.ts",
-        "zeta.txt",
-      ]),
-    ).toEqual([
-      "src/Apple.ts",
-      "src/beta.ts",
-      "alpha.txt",
-      "Beta.txt",
-      "ZETA_CAPS.txt",
-      "zeta.txt",
-    ]);
-  });
-});
-
-describe("sortReviewItemsByTreeOrder", () => {
-  const item = (key: string, filePaths: string[]): ReviewListItem => ({
-    key,
-    filePaths,
-    node: null,
-  });
-
-  it("reorders file items to match the given tree order", () => {
-    const items = [
-      item("a", ["zeta.txt"]),
-      item("b", ["alpha.txt"]),
-      item("c", ["src/x.ts"]),
-    ];
-    const ordered = sortReviewItemsByTreeOrder(items, [
-      "src/x.ts",
-      "alpha.txt",
-      "zeta.txt",
-    ]);
-    expect(ordered.map((i) => i.key)).toEqual(["c", "b", "a"]);
-  });
-
-  it("keeps items whose path is not in the order last", () => {
-    const items = [item("a", ["unknown.ts"]), item("b", ["alpha.txt"])];
-    const ordered = sortReviewItemsByTreeOrder(items, ["alpha.txt"]);
-    expect(ordered.map((i) => i.key)).toEqual(["b", "a"]);
-  });
-
-  it("returns the same array when there is no tree order", () => {
-    const items = [item("a", ["zeta.txt"]), item("b", ["alpha.txt"])];
-    expect(sortReviewItemsByTreeOrder(items, [])).toBe(items);
-  });
-});

packages/ui/src/features/code-review/components/reviewItemBuilders.tsx

@@ -5,19 +5,12 @@ import {
   computeSkipExpansion,
 } from "@posthog/core/code-review/reviewItemKeys";
 import type { PrCommentThread } from "@posthog/core/code-review/types";
-import {
-  orderPathsLikeChangeTree,
-  sortByChangeTreeOrder,
-} from "@posthog/core/git-interaction/changeTree";
 import type { ChangedFile } from "@posthog/shared/domain-types";
 import { makeFileKey } from "../../git-interaction/utils/fileKey";
 import type { ReviewListItem } from "../commentFileFilter";
 import type { DiffOptions } from "../types";
 import { PatchRow, RemoteRow, UntrackedRow } from "./ReviewRows";
 
-export const orderPathsLikeTree = orderPathsLikeChangeTree;
-export const sortReviewItemsByTreeOrder = sortByChangeTreeOrder<ReviewListItem>;
-
 export function changedFileSignature(file: ChangedFile): string | null {
   if (file.patch) return contentHash(file.patch);
   if (file.sha) return `${file.status}:${file.sha}`;

packages/ui/src/features/task-detail/components/ChangesTreeView.tsx

@@ -2,18 +2,15 @@ import {
   buildChangeTree,
   type ChangeTreeNode,
   compactChangeTree,
+  orderedTreeDirs,
+  orderedTreeFiles,
 } from "@posthog/core/git-interaction/changeTree";
 import type { ChangedFile } from "@posthog/shared/domain-types";
 import { TreeDirectoryRow } from "@posthog/ui/primitives/TreeDirectoryRow";
 import { useCallback, useMemo, useState } from "react";
 
-export type TreeNode = ChangeTreeNode;
-
-export const buildChangesTree = buildChangeTree;
-export const compactTree = compactChangeTree;
-
 interface ChangesTreeNodeProps {
-  node: TreeNode;
+  node: ChangeTreeNode;
   depth: number;
   collapsedDirs: Set<string>;
   onToggleDir: (path: string) => void;
@@ -28,20 +25,8 @@ function ChangesTreeNode({
   renderFile,
 }: ChangesTreeNodeProps) {
   const isCollapsed = collapsedDirs.has(node.path);
-  const sortedDirs = useMemo(
-    () =>
-      [...node.children.values()].sort((a, b) => a.name.localeCompare(b.name)),
-    [node.children],
-  );
-  const sortedFiles = useMemo(
-    () =>
-      [...node.files].sort((a, b) => {
-        const aName = a.path.split("/").pop() || "";
-        const bName = b.path.split("/").pop() || "";
-        return aName.localeCompare(bName);
-      }),
-    [node.files],
-  );
+  const sortedDirs = useMemo(() => orderedTreeDirs(node), [node]);
+  const sortedFiles = useMemo(() => orderedTreeFiles(node), [node]);
 
   return (
     <>
@@ -80,7 +65,10 @@ interface ChangesTreeViewProps {
 }
 
 export function ChangesTreeView({ files, renderFile }: ChangesTreeViewProps) {
-  const tree = useMemo(() => compactTree(buildChangesTree(files)), [files]);
+  const tree = useMemo(
+    () => compactChangeTree(buildChangeTree(files)),
+    [files],
+  );
   const [collapsedDirs, setCollapsedDirs] = useState<Set<string>>(new Set());
 
   const handleToggleDir = useCallback((path: string) => {