fix(web): stabilize nested Review tree expansion (#2702)

apps/web/components/review/review-file-tree.test.tsx

@@ -7,6 +7,10 @@ import { ReviewFileTree } from "./review-file-tree";
 afterEach(cleanup);
 
 const APP_PATH = "src/app.tsx";
+const README_PATH = "README.md";
+const OUTER_SCOPE = "vendor/outer";
+const INNER_SCOPE = "vendor/outer/vendor/inner";
+const SUBMODULE_NODE_TEST_ID = "submodule-node";
 
 function file(overrides: Partial<ReviewFile>): ReviewFile {
   return {
@@ -75,7 +79,7 @@ describe("ReviewFileTree ordering", () => {
   it("preserves review-list order when a directory is interleaved with a root file", () => {
     const files = [
       file({ path: "src/a.ts" }),
-      file({ path: "README.md" }),
+      file({ path: README_PATH }),
       file({ path: "src/b.ts" }),
     ];
 
@@ -201,11 +205,11 @@ describe("ReviewFileTree", () => {
 
   it("marks a nested submodule boundary with an accessible scope label", () => {
     renderTree([
-      file({ path: "README.md" }),
+      file({ path: README_PATH }),
       file({ path: "src/a.ts", repository_name: "vendor/lib" }),
     ]);
 
-    const node = screen.getByTestId("submodule-node");
+    const node = screen.getByTestId(SUBMODULE_NODE_TEST_ID);
     expect(node.textContent).toContain("lib");
     expect(within(node).getByRole("button").getAttribute("aria-label")).toBe(
       "Submodule: vendor/lib",
@@ -213,14 +217,103 @@ describe("ReviewFileTree", () => {
   });
 });
 
+describe("ReviewFileTree late updates", () => {
+  it("expands directories introduced by a later review-source update", () => {
+    const initialFiles = [
+      file({ path: README_PATH }),
+      file({ path: README_PATH, repository_name: OUTER_SCOPE }),
+    ];
+    const props: Parameters<typeof ReviewFileTree>[0] = {
+      files: initialFiles,
+      reviewedFiles: new Set<string>(),
+      staleFiles: new Set<string>(),
+      commentCountByFile: {},
+      selectedFile: null,
+      filter: "",
+      onFilterChange: vi.fn(),
+      onSelectFile: vi.fn(),
+      onToggleReviewed: vi.fn(),
+    };
+    const { rerender } = render(<ReviewFileTree {...props} />);
+    const findScope = (repositoryName: string) =>
+      screen
+        .queryAllByTestId(SUBMODULE_NODE_TEST_ID)
+        .find((node) => node.getAttribute("data-repository-name") === repositoryName);
+
+    expect(findScope(OUTER_SCOPE)).toBeTruthy();
+    expect(findScope(INNER_SCOPE)).toBeUndefined();
+
+    rerender(
+      <ReviewFileTree
+        {...props}
+        files={[
+          ...initialFiles,
+          file({
+            path: README_PATH,
+            repository_name: INNER_SCOPE,
+          }),
+        ]}
+      />,
+    );
+
+    expect(findScope(INNER_SCOPE)).toBeTruthy();
+  });
+});
+
+describe("ReviewFileTree late-update collapse state", () => {
+  it("preserves a manually collapsed directory when later files arrive", () => {
+    const initialFiles = [
+      file({ path: README_PATH }),
+      file({ path: "src/a.ts", repository_name: OUTER_SCOPE }),
+    ];
+    const props: Parameters<typeof ReviewFileTree>[0] = {
+      files: initialFiles,
+      reviewedFiles: new Set<string>(),
+      staleFiles: new Set<string>(),
+      commentCountByFile: {},
+      selectedFile: null,
+      filter: "",
+      onFilterChange: vi.fn(),
+      onSelectFile: vi.fn(),
+      onToggleReviewed: vi.fn(),
+    };
+    const { rerender } = render(<ReviewFileTree {...props} />);
+    const outer = screen
+      .getAllByTestId(SUBMODULE_NODE_TEST_ID)
+      .find((node) => node.getAttribute("data-repository-name") === OUTER_SCOPE);
+    fireEvent.click(within(outer!).getByRole("button"));
+    expect(screen.queryByText("a.ts")).toBeNull();
+
+    rerender(
+      <ReviewFileTree
+        {...props}
+        files={[
+          ...initialFiles,
+          file({
+            path: README_PATH,
+            repository_name: INNER_SCOPE,
+          }),
+        ]}
+      />,
+    );
+
+    expect(screen.queryByText("a.ts")).toBeNull();
+    expect(
+      screen
+        .queryAllByTestId(SUBMODULE_NODE_TEST_ID)
+        .find((node) => node.getAttribute("data-repository-name") === INNER_SCOPE),
+    ).toBeUndefined();
+  });
+});
+
 describe("ReviewFileTree multi-repo identity", () => {
   it("disambiguates same-named files in different repos via composite key", () => {
-    const fA = file({ path: "README.md", repository_name: "frontend", repository_id: "f" });
-    const fB = file({ path: "README.md", repository_name: "backend", repository_id: "b" });
+    const fA = file({ path: README_PATH, repository_name: "frontend", repository_id: "f" });
+    const fB = file({ path: README_PATH, repository_name: "backend", repository_id: "b" });
     const { onSelectFile } = renderTree([fA, fB]);
     // There are two rows with the same display name; clicking each must
     // dispatch a distinct key.
-    const rows = screen.getAllByText("README.md");
+    const rows = screen.getAllByText(README_PATH);
     expect(rows).toHaveLength(2);
     fireEvent.click(rows[0]);
     fireEvent.click(rows[1]);
@@ -232,14 +325,14 @@ describe("ReviewFileTree multi-repo identity", () => {
 
   it("exposes a stable repository discriminator for same-path rows", () => {
     renderTree([
-      file({ path: "README.md", repository_name: "frontend", repository_id: "f" }),
-      file({ path: "README.md", repository_name: "backend", repository_id: "b" }),
+      file({ path: README_PATH, repository_name: "frontend", repository_id: "f" }),
+      file({ path: README_PATH, repository_name: "backend", repository_id: "b" }),
     ]);
 
     const identities = screen
       .getAllByTestId("review-file-row")
       .map((row) => `${row.dataset.repositoryName}:${row.dataset.filePath}`)
       .sort();
-    expect(identities).toEqual(["backend:README.md", "frontend:README.md"]);
+    expect(identities).toEqual([`backend:${README_PATH}`, `frontend:${README_PATH}`]);
   });
 });

apps/web/components/review/review-file-tree.tsx

@@ -1,6 +1,6 @@
 "use client";
 
-import { memo, useMemo, useRef } from "react";
+import { memo, useEffect, useMemo, useRef } from "react";
 import {
   IconChevronDown,
   IconChevronRight,
@@ -37,6 +37,19 @@ const REVIEW_GET_PATH = (n: FileTreeNode) => n.path;
 const REVIEW_GET_CHILDREN = (n: FileTreeNode) => n.children;
 const REVIEW_IS_DIR = (n: FileTreeNode) => Boolean(n.isDir);
 
+function collectDirPaths(nodes: FileTreeNode[]): string[] {
+  const paths: string[] = [];
+  const walk = (list: FileTreeNode[]) => {
+    for (const node of list) {
+      if (!node.isDir) continue;
+      paths.push(node.path);
+      if (node.children) walk(node.children);
+    }
+  };
+  walk(nodes);
+  return paths;
+}
+
 export const ReviewFileTree = memo(function ReviewFileTree({
   files,
   reviewedFiles,
@@ -51,7 +64,7 @@ export const ReviewFileTree = memo(function ReviewFileTree({
   const inputRef = useRef<HTMLInputElement>(null);
   const tree = useMemo(() => buildFileTree(files), [files]);
 
-  const { visibleRows, toggle } = useTree<FileTreeNode>({
+  const { visibleRows, toggle, setExpanded } = useTree<FileTreeNode>({
     nodes: tree,
     getPath: REVIEW_GET_PATH,
     getChildren: REVIEW_GET_CHILDREN,
@@ -59,6 +72,21 @@ export const ReviewFileTree = memo(function ReviewFileTree({
     defaultExpanded: "all",
   });
 
+  // Auto-expand directories that arrive with a later review-source update.
+  // Existing directories keep the reviewer's explicit collapsed state.
+  const seenDirPathsRef = useRef<Set<string>>(new Set(collectDirPaths(tree)));
+  useEffect(() => {
+    const current = collectDirPaths(tree);
+    const newDirs = current.filter((path) => !seenDirPathsRef.current.has(path));
+    if (newDirs.length === 0) return;
+    setExpanded((prev) => {
+      const next = new Set(prev);
+      for (const path of newDirs) next.add(path);
+      return next;
+    });
+    for (const path of newDirs) seenDirPathsRef.current.add(path);
+  }, [tree, setExpanded]);
+
   return (
     <div className="flex flex-col h-full text-sm">
       <ReviewFilterInput ref={inputRef} value={filter} onChange={onFilterChange} />

docs/plans/review-tree-late-expansion/plan.md

@@ -0,0 +1,54 @@
+---
+spec: docs/specs/ui/submodule-review.md
+created: 2026-08-15
+status: implemented
+---
+
+# Implementation Plan: Review tree late expansion
+
+## Overview
+
+Harden the desktop Review file tree against incremental review-source updates. The existing `useTree` state expands directories present on mount, but the Review sidebar can receive parent and nested-submodule files in separate updates. The plan adds Review-specific first-seen directory reconciliation, preserves manual collapse state, and proves the behavior with a focused component regression plus the existing nested-submodule E2E.
+
+The fix stays within the accepted nested-submodule repository-scope decision. It does not change repository discovery, source precedence, API payloads, mobile Review composition, or locator timeouts.
+
+## Frontend
+
+### Review file tree
+
+- `apps/web/components/review/review-file-tree.tsx`: track directory paths already observed by the tree. When `files` introduces new directory paths, add only those paths to the expanded set. Keep existing paths unchanged so an explicit user collapse remains authoritative.
+- Reuse the existing `useTree` `setExpanded` API and the same first-seen-directory behavior already used by `apps/web/components/task/changes-panel-tree.tsx`; do not broaden the shared hook contract for this narrow Review behavior.
+
+## Tests
+
+- **Late nested scope:** extend `apps/web/components/review/review-file-tree.test.tsx` with a rerender that adds a nested submodule file after the parent scope is present. Verify the intermediate directory and nested `submodule-node` become visible.
+- **Manual collapse preservation:** retain the existing Review tree collapse coverage and ensure the new reconciliation only expands paths not observed before, not an already-seen directory.
+
+## E2E Tests
+
+- **Scenario:** GIVEN Review receives parent and nested submodule diffs through separate source updates, WHEN the existing desktop nested-submodule Review flow opens and expands the parent boundary, THEN the nested boundary is visible and all three `README.md` diffs remain reviewable.
+- **File:** `apps/web/e2e/tests/review/submodule-review.spec.ts` (existing regression; no selector or timeout change planned).
+- **Mobile note:** the desktop `ReviewFileTree` is hidden below the existing `md` breakpoint. Phone Review uses the existing sticky diff header, so no mobile interaction changes and no new mobile test are required; the existing mobile submodule Review test remains a non-targeted smoke check.
+
+## Verification Results
+
+Completed 2026-08-15.
+
+- RED unit test: `cd apps && pnpm --filter @kandev/web test -- components/review/review-file-tree.test.tsx` failed at the new late-directory assertion, with 17 passing and 1 failing test before the source change.
+- GREEN unit test: the same command passed with 18 tests.
+- Desktop E2E: `cd apps/web && pnpm e2e:run tests/review/submodule-review.spec.ts -- --grep "shows nested scopes and commits child gitlinks through the UI" --workers=1 --retries=0` passed 1 test.
+- Desktop pressure E2E: the same scenario with `--no-build --repeat-each=4` passed 4/4 runs.
+- Mobile smoke E2E: `cd apps/web && pnpm e2e:run --project mobile-chrome tests/review/mobile-submodule-review.spec.ts` passed 1 test.
+- TypeScript: `cd apps && pnpm --filter @kandev/web typecheck` reported no errors.
+- Web lint: `cd apps && pnpm --filter @kandev/web lint` passed with zero warnings.
+- PR screenshot capture used a disposable spec and synthetic submodule fixture data; the temporary spec was removed, the manifest-to-file mapping was validated, and the PNG was compressed.
+
+## Implementation Waves And Parallel Candidates
+
+Sequential single-task repair:
+
+- [x] [Task 01: Expand late Review directories](task-01-expand-late-review-directories.md)
+
+## Open Questions
+
+None.

docs/plans/review-tree-late-expansion/task-01-expand-late-review-directories.md

@@ -0,0 +1,67 @@
+---
+id: "01-expand-late-review-directories"
+title: "Expand late Review directories"
+status: done
+wave: 1
+depends_on: []
+plan: "plan.md"
+spec: "../../specs/ui/submodule-review.md"
+---
+
+# Task 01: Expand late Review directories
+
+## Acceptance
+
+- A Review tree that receives a nested submodule file after its parent scope automatically exposes the newly introduced intermediate directory and nested boundary.
+- A directory already observed and manually collapsed remains collapsed when later files arrive; the fix does not reset the reviewer's expansion choices.
+- The existing desktop nested-submodule E2E passes without relying on a retry-only success.
+
+## Verification
+
+```bash
+cd apps && pnpm --filter @kandev/web test -- components/review/review-file-tree.test.tsx
+cd apps/web && pnpm e2e:run tests/review/submodule-review.spec.ts -- --grep "shows nested scopes and commits child gitlinks through the UI" --workers=1 --retries=0
+cd apps/web && pnpm e2e:run --project mobile-chrome tests/review/mobile-submodule-review.spec.ts
+```
+
+The managed E2E runner rebuilds the production backend and Vite assets before the desktop check. The focused unit test is the RED test and must fail before the source change, then pass after it.
+
+## Files likely touched
+
+- `apps/web/components/review/review-file-tree.tsx`
+- `apps/web/components/review/review-file-tree.test.tsx`
+
+## Dependencies
+
+None.
+
+## Parallelism
+
+`sequential` — the regression test and the component change share the same files and must follow Red-Green-Refactor.
+
+## Inputs
+
+- `docs/specs/ui/submodule-review.md`, especially the nested-boundary and late-source scenarios.
+- `docs/plans/review-tree-late-expansion/plan.md`.
+- Existing first-seen directory reconciliation in `apps/web/components/task/changes-panel-tree.tsx`.
+- `/tdd`, `/e2e`, and `/mobile-parity` guidance.
+
+## Output contract
+
+Report the RED and GREEN unit-test results, exact E2E commands and counts, files changed, cleanup evidence, residual risks, and synchronized task/plan status.
+
+## Results
+
+Completed 2026-08-15.
+
+- RED: the new rerender regression failed before the implementation (`17 passed, 1 failed`).
+- GREEN: the focused ReviewFileTree suite passed (`18 passed`).
+- `cd apps/web && pnpm e2e:run tests/review/submodule-review.spec.ts -- --grep "shows nested scopes and commits child gitlinks through the UI" --workers=1 --retries=0` passed (`1 passed`).
+- `cd apps/web && pnpm e2e:run --no-build tests/review/submodule-review.spec.ts -- --grep "shows nested scopes and commits child gitlinks through the UI" --workers=1 --retries=0 --repeat-each=4` passed (`4 passed`).
+- `cd apps/web && pnpm e2e:run --project mobile-chrome tests/review/mobile-submodule-review.spec.ts` passed (`1 passed`).
+- `cd apps && pnpm --filter @kandev/web typecheck` reported no errors.
+- `cd apps && pnpm --filter @kandev/web lint` passed with zero warnings.
+- Changed files: `apps/web/components/review/review-file-tree.tsx`, `apps/web/components/review/review-file-tree.test.tsx`, `docs/specs/ui/submodule-review.md`, `docs/plans/review-tree-late-expansion/plan.md`, and this task file.
+- Residual risk: the Review tree still rebuilds and walks its small directory model when `files` changes; no shared `useTree` behavior or mobile tree surface changed.
+- The disposable screenshot capture spec was removed after capture. The synthetic desktop screenshot manifest was non-empty, every manifest entry mapped to an existing file, and the PNG was compressed for PR publication.
+- Synchronization: this task is `done`, the parent plan is `implemented` with Task 01 checked, and the linked submodule Review spec records the late-source expansion contract.

docs/specs/ui/submodule-review.md

@@ -17,6 +17,7 @@ Repositories that use Git submodules currently show only a changed gitlink commi
 - Changes and Review include changed files from every initialized submodule present when the task repository graph is discovered, including recursively nested submodules.
 - Parent-repository files and submodule files can appear in the same Review session. Each file keeps a repository-relative path and an unambiguous repository scope.
 - Review renders submodule scopes in their task-workspace directory hierarchy and gives each submodule boundary a small visible and accessible indication. A nested submodule appears beneath its parent submodule rather than as an unrelated top-level repository.
+- Review keeps newly discovered directory and submodule scope nodes reachable when review sources arrive in separate updates. It expands only directories introduced by the update and preserves any collapsed state the reviewer chose for existing directories.
 - When Kandev can show the child files for a changed submodule, Review omits the parent's raw gitlink-only row. If the child is unavailable, Review retains the parent gitlink change instead of hiding the only evidence of the change.
 - Uncommitted and committed submodule changes use the same source precedence, diff limits, reviewed hashes, stale detection, anchored comments, findings, filtering, status markers, and file-opening behavior as other Review files.
 - Each submodule compares against the gitlink commit recorded by its parent at the parent's comparison anchor. The comparison does not expand to unrelated history from the submodule's default branch.
@@ -50,6 +51,7 @@ For a multi-repository task, a top-level attached repository keeps its existing
 
 - **GIVEN** a task repository with an initialized submodule containing an uncommitted file edit and a changed parent file, **WHEN** Review opens, **THEN** both textual diffs appear and the submodule file sits beneath a marked submodule boundary.
 - **GIVEN** a submodule containing another initialized submodule, **WHEN** files change in both children, **THEN** Review shows both nested directory boundaries and each file's repository-relative diff.
+- **GIVEN** Review first receives a parent submodule file and later receives a file from an initialized nested submodule, **WHEN** the later review-source update is rendered, **THEN** the intermediate directory and nested boundary are expanded automatically so the nested scope is reachable without an extra expansion step, while previously collapsed existing directories remain collapsed.
 - **GIVEN** a submodule commit made after the task comparison point, **WHEN** the session reloads or agentctl restarts and Review opens, **THEN** the committed file diff is computed from the parent-recorded gitlink commit and remains visible.
 - **GIVEN** a parent gitlink change whose child file diffs are available, **WHEN** Review builds its file list, **THEN** it shows the child files without a duplicate raw gitlink-only row.
 - **GIVEN** an unavailable submodule whose gitlink changed, **WHEN** Review opens, **THEN** the parent gitlink row remains visible and other repositories continue to render.
@@ -68,3 +70,5 @@ For a multi-repository task, a top-level attached repository keeps its existing
 ## Implementation plan
 
 See [Nested Submodule Review plan](../../plans/submodule-review/plan.md).
+
+The late-source tree expansion repair is tracked in the [Review tree late expansion plan](../../plans/review-tree-late-expansion/plan.md).