diff --git a/apps/web/components/task/file-browser-parts.tsx b/apps/web/components/task/file-browser-parts.tsx index 41b13fde77..389815a62d 100644 --- a/apps/web/components/task/file-browser-parts.tsx +++ b/apps/web/components/task/file-browser-parts.tsx @@ -28,6 +28,7 @@ import type { FileInfo } from "@/lib/state/store"; import type { WorkspaceRestorationAttempt } from "@/lib/state/slices/session-runtime/workspace-restoration"; import type { FileBrowserRow } from "./file-browser-hooks"; import { areTreeNodeRowPropsEqual, type TreeNodeRowProps } from "./file-tree-row-props"; +import { measureFileTreeElement } from "./file-tree-measurement"; import { InlineFileInput } from "./inline-file-input"; import { FileContextMenu, @@ -545,6 +546,7 @@ function VirtualizedFileTreeView(props: FileBrowserContentAreaProps) { const row = virtualRows[index]; return row?.type === "create" ? `create:${row.parentPath}` : (row?.row.path ?? index); }, + measureElement: measureFileTreeElement, overscan: 5, }); diff --git a/apps/web/components/task/file-tree-geometry.test.ts b/apps/web/components/task/file-tree-geometry.test.ts new file mode 100644 index 0000000000..5302c71f5b --- /dev/null +++ b/apps/web/components/task/file-tree-geometry.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from "vitest"; +import { + fileTreeGeometryIssues, + type FileTreeViewportGeometry, +} from "../../e2e/tests/task/file-tree-geometry"; + +function geometry(overrides: Partial): FileTreeViewportGeometry { + return { + bottom: 100, + clientHeight: 100, + rows: [{ path: "row-0", top: 0, bottom: 28 }], + scrollTop: 0, + top: 0, + treeHeight: 28, + ...overrides, + }; +} + +describe("file-tree viewport geometry assertions", () => { + it("rejects a blank top edge", () => { + expect( + fileTreeGeometryIssues( + geometry({ + rows: [{ path: "row-4", top: 30, bottom: 58 }], + treeHeight: 300, + }), + ), + ).toContainEqual(expect.stringContaining("blank gap at the viewport top")); + }); + + it("rejects a blank bottom edge when overflowing content continues below", () => { + expect( + fileTreeGeometryIssues( + geometry({ + rows: [ + { path: "row-0", top: 0, bottom: 28 }, + { path: "row-1", top: 28, bottom: 80 }, + ], + treeHeight: 300, + }), + ), + ).toContainEqual(expect.stringContaining("blank gap at the viewport bottom")); + }); + + it("allows trailing space for a short tree", () => { + expect( + fileTreeGeometryIssues( + geometry({ + rows: [{ path: "folder", top: 0, bottom: 28 }], + treeHeight: 99, + }), + ), + ).toEqual([]); + }); + + it("allows the configured end padding at the end of overflowing content", () => { + expect( + fileTreeGeometryIssues( + geometry({ + rows: [ + { path: "row-4", top: -20, bottom: 8 }, + { path: "row-5", top: 8, bottom: 92 }, + ], + scrollTop: 200, + treeHeight: 300, + }), + ), + ).toEqual([]); + }); +}); diff --git a/apps/web/components/task/file-tree-measurement.test.ts b/apps/web/components/task/file-tree-measurement.test.ts new file mode 100644 index 0000000000..8458282d27 --- /dev/null +++ b/apps/web/components/task/file-tree-measurement.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "vitest"; +import { Virtualizer } from "@tanstack/react-virtual"; +import { measureFileTreeElement } from "./file-tree-measurement"; + +function createVirtualizer(estimateSize: number): Virtualizer { + return new Virtualizer({ + count: 1, + getScrollElement: () => null, + estimateSize: () => estimateSize, + scrollToFn: () => {}, + observeElementRect: () => {}, + observeElementOffset: () => {}, + getItemKey: () => "row-a", + }); +} + +function resizeEntry(element: HTMLDivElement, blockSize: number): ResizeObserverEntry { + return { + target: element, + borderBoxSize: [{ blockSize, inlineSize: 320 }], + } as unknown as ResizeObserverEntry; +} + +describe("file-tree row measurements", () => { + it("retains a positive cached row size when a hidden row reports zero height", () => { + const element = document.createElement("div"); + element.dataset.index = "0"; + const virtualizer = createVirtualizer(28); + virtualizer.itemSizeCache.set("row-a", 32); + + expect(measureFileTreeElement(element, resizeEntry(element, 0), virtualizer)).toBe(32); + }); + + it.each([28, 44])( + "uses the current estimate when no positive size exists (%dpx)", + (estimateSize) => { + const element = document.createElement("div"); + element.dataset.index = "0"; + const virtualizer = createVirtualizer(estimateSize); + + expect(measureFileTreeElement(element, resizeEntry(element, 0), virtualizer)).toBe( + estimateSize, + ); + }, + ); + + it("accepts a later positive measurement after a cached fallback", () => { + const element = document.createElement("div"); + element.dataset.index = "0"; + const virtualizer = createVirtualizer(28); + virtualizer.itemSizeCache.set("row-a", 28); + + expect(measureFileTreeElement(element, resizeEntry(element, 48), virtualizer)).toBe(48); + }); +}); diff --git a/apps/web/components/task/file-tree-measurement.ts b/apps/web/components/task/file-tree-measurement.ts new file mode 100644 index 0000000000..61c15d0468 --- /dev/null +++ b/apps/web/components/task/file-tree-measurement.ts @@ -0,0 +1,19 @@ +import { measureElement as tanstackMeasureElement } from "@tanstack/react-virtual"; +import type { Virtualizer } from "@tanstack/react-virtual"; + +export function measureFileTreeElement( + element: HTMLDivElement, + entry: ResizeObserverEntry | undefined, + instance: Virtualizer, +): number { + const measuredSize = tanstackMeasureElement(element, entry, instance); + if (measuredSize > 0) return measuredSize; + + const index = instance.indexFromElement(element); + const key = instance.options.getItemKey(index); + // Hidden rows must retain positive cached geometry until the browser can measure them again. + const cachedSize = instance.itemSizeCache.get(key); + return cachedSize !== undefined && cachedSize > 0 + ? cachedSize + : instance.options.estimateSize(index); +} diff --git a/apps/web/e2e/tests/task/file-tree-geometry.ts b/apps/web/e2e/tests/task/file-tree-geometry.ts new file mode 100644 index 0000000000..acbfeccd36 --- /dev/null +++ b/apps/web/e2e/tests/task/file-tree-geometry.ts @@ -0,0 +1,112 @@ +export type FileTreeRowBounds = Readonly<{ + bottom: number; + path: string | null; + top: number; +}>; + +export type FileTreeViewportGeometry = Readonly<{ + bottom: number; + clientHeight: number; + rows: readonly FileTreeRowBounds[]; + scrollTop: number; + top: number; + treeHeight: number; +}>; + +export type FileTreeGeometryOptions = Readonly<{ + contentTopInset?: number; + endPadding?: number; + expectedPaths?: readonly string[]; + tolerance?: number; +}>; + +export const FILE_TREE_ROW_EDGE_TOLERANCE_PX = 2; +export const FILE_TREE_END_PADDING_PX = 8; + +function expectedPathIssues( + rows: readonly FileTreeRowBounds[], + expectedPaths: readonly string[] | undefined, +): string[] { + if (!expectedPaths) return []; + const actualPaths = rows.flatMap((row) => (row.path === null ? [] : [row.path])); + if ( + actualPaths.length === expectedPaths.length && + actualPaths.every((path, index) => path === expectedPaths[index]) + ) { + return []; + } + return [ + `file tree paths differ: expected ${JSON.stringify(expectedPaths)}, got ${JSON.stringify(actualPaths)}`, + ]; +} + +function viewportEdgeIssues( + geometry: FileTreeViewportGeometry, + options: FileTreeGeometryOptions = {}, +): string[] { + const tolerance = options.tolerance ?? FILE_TREE_ROW_EDGE_TOLERANCE_PX; + const contentTopInset = options.contentTopInset ?? 0; + const endPadding = options.endPadding ?? FILE_TREE_END_PADDING_PX; + const issues: string[] = []; + const [firstRow] = geometry.rows; + const lastRow = geometry.rows.at(-1); + + if (!firstRow || !lastRow) return issues; + + const contentTop = geometry.top + contentTopInset; + if (firstRow.top > contentTop + tolerance) { + issues.push( + `file tree has a blank gap at the viewport top: first row starts ${firstRow.top - contentTop}px below content top`, + ); + } + + const treeBottom = geometry.top - geometry.scrollTop + geometry.treeHeight; + const treeIsShort = geometry.treeHeight < geometry.clientHeight; + if (!treeIsShort) { + const treeEndsAtViewport = treeBottom <= geometry.bottom + endPadding + tolerance; + const contentBottom = treeEndsAtViewport + ? Math.min(geometry.bottom, treeBottom) - endPadding + : geometry.bottom; + if (lastRow.bottom < contentBottom - tolerance) { + issues.push( + `file tree has a blank gap at the viewport bottom: last row ends ${contentBottom - lastRow.bottom}px before content bottom`, + ); + } + } + + return issues; +} + +function adjacentRowIssues(rows: readonly FileTreeRowBounds[], tolerance: number): string[] { + const issues: string[] = []; + for (let index = 1; index < rows.length; index += 1) { + const previous = rows[index - 1]; + const current = rows[index]; + if (!previous || !current) continue; + const gap = current.top - previous.bottom; + if (gap < -tolerance) { + issues.push( + `file tree rows ${previous.path ?? "unknown"} and ${current.path ?? "unknown"} overlap by ${-gap}px`, + ); + } else if (gap > tolerance) { + issues.push( + `file tree rows ${previous.path ?? "unknown"} and ${current.path ?? "unknown"} have a blank gap of ${gap}px`, + ); + } + } + + return issues; +} + +export function fileTreeGeometryIssues( + geometry: FileTreeViewportGeometry, + options: FileTreeGeometryOptions = {}, +): string[] { + if (geometry.rows.length === 0) return ["file tree has no mounted rows in its viewport"]; + const tolerance = options.tolerance ?? FILE_TREE_ROW_EDGE_TOLERANCE_PX; + return [ + ...expectedPathIssues(geometry.rows, options.expectedPaths), + ...viewportEdgeIssues(geometry, options), + ...adjacentRowIssues(geometry.rows, tolerance), + ]; +} diff --git a/apps/web/e2e/tests/task/large-file-tree-virtualization-helpers.ts b/apps/web/e2e/tests/task/large-file-tree-virtualization-helpers.ts index 74b639fa57..b308b90279 100644 --- a/apps/web/e2e/tests/task/large-file-tree-virtualization-helpers.ts +++ b/apps/web/e2e/tests/task/large-file-tree-virtualization-helpers.ts @@ -1,11 +1,16 @@ import path from "node:path"; -import type { Locator } from "@playwright/test"; +import { expect, type Locator } from "@playwright/test"; import type { Page } from "@playwright/test"; import type { SeedData } from "../../fixtures/test-base"; import type { ApiClient } from "../../helpers/api-client"; import type { BackendContext } from "../../fixtures/backend"; import { GitHelper, makeGitEnv } from "../../helpers/git-helper"; import { SessionPage } from "../../pages/session-page"; +import { + fileTreeGeometryIssues, + type FileTreeGeometryOptions, + type FileTreeViewportGeometry, +} from "./file-tree-geometry"; export const LARGE_FILE_TREE_FOLDER = "large-file-tree"; export const LARGE_FILE_TREE_COUNT = 600; @@ -14,6 +19,66 @@ export function largeFileTreePath(index: number): string { return `${LARGE_FILE_TREE_FOLDER}/entry-${index.toString().padStart(4, "0")}.txt`; } +/** Wait for a hidden-panel transition to pass through the browser's layout observers. */ +export async function waitForFileTreeLayoutSettle(page: Page): Promise { + await page.evaluate( + () => + new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))), + ); +} + +async function readFileTreeViewportGeometry(viewport: Locator): Promise { + return viewport.evaluate((element) => { + const viewportRect = element.getBoundingClientRect(); + const firstVirtualRow = element.querySelector("[data-index]"); + const treeContainer = firstVirtualRow?.parentElement; + const rows = Array.from(element.querySelectorAll("[data-index]")) + .map((row) => { + const rect = row.getBoundingClientRect(); + return { + bottom: rect.bottom, + path: + row.querySelector('[data-testid="file-tree-node"]')?.dataset.path ?? null, + top: rect.top, + }; + }) + .filter((row) => row.bottom > viewportRect.top && row.top < viewportRect.bottom) + .sort((left, right) => left.top - right.top); + return { + bottom: viewportRect.bottom, + clientHeight: element.clientHeight, + rows, + scrollTop: element.scrollTop, + top: viewportRect.top, + treeHeight: treeContainer?.getBoundingClientRect().height ?? 0, + }; + }); +} + +/** Return the ordered rows currently intersecting the file-tree viewport. */ +export async function visibleFileTreePaths(viewport: Locator): Promise { + const geometry = await readFileTreeViewportGeometry(viewport); + return geometry.rows.flatMap((row) => (row.path === null ? [] : [row.path])); +} + +/** Assert that mounted rows cover the visible viewport without blank intervals. */ +export async function expectContiguousVisibleFileTreeRows( + viewport: Locator, + options: FileTreeGeometryOptions = {}, +): Promise { + const geometry = await readFileTreeViewportGeometry(viewport); + const issues = fileTreeGeometryIssues(geometry, options); + expect(issues, issues.join("; ")).toEqual([]); +} + +/** Compare an expected ordered visible window after a panel transition. */ +export async function expectVisibleFileTreePaths( + viewport: Locator, + expectedPaths: readonly string[], +): Promise { + expect(await visibleFileTreePaths(viewport)).toEqual(expectedPaths); +} + /** Reveal the final seeded row even when later tests have added rows after the fixture folder. */ export async function scrollToLastLargeFile(lastFile: Locator, viewport: Locator): Promise { await viewport.evaluate((element) => { diff --git a/apps/web/e2e/tests/task/large-file-tree-virtualization.spec.ts b/apps/web/e2e/tests/task/large-file-tree-virtualization.spec.ts index 86549c4aaf..b59055d729 100644 --- a/apps/web/e2e/tests/task/large-file-tree-virtualization.spec.ts +++ b/apps/web/e2e/tests/task/large-file-tree-virtualization.spec.ts @@ -2,9 +2,13 @@ import { test, expect } from "../../fixtures/test-base"; import { LARGE_FILE_TREE_FOLDER, LARGE_FILE_TREE_COUNT, + expectContiguousVisibleFileTreeRows, + expectVisibleFileTreePaths, largeFileTreePath, scrollToLastLargeFile, setupLargeFileTreeTask, + visibleFileTreePaths, + waitForFileTreeLayoutSettle, } from "./large-file-tree-virtualization-helpers"; test.describe("Large file tree virtualization", () => { @@ -82,4 +86,66 @@ test.describe("Large file tree virtualization", () => { await expect(input).toBeVisible({ timeout: 15_000 }); await expect(input).toBeFocused({ timeout: 5_000 }); }); + + test("restores contiguous rows after Files is hidden and reopened", async ({ + apiClient, + seedData, + backend, + testPage, + }) => { + test.setTimeout(120_000); + const session = await setupLargeFileTreeTask({ + testPage, + apiClient, + seedData, + backend, + title: "Large file tree restoration", + }); + + await session.clickTab("Files"); + const folder = session.fileTreeNode(LARGE_FILE_TREE_FOLDER); + const viewport = session.fileTreeScrollViewport(); + await expect(folder).toBeVisible({ timeout: 15_000 }); + await expect(viewport).toBeVisible({ timeout: 15_000 }); + await waitForFileTreeLayoutSettle(testPage); + await expectContiguousVisibleFileTreeRows(viewport); + const collapsedPaths = await visibleFileTreePaths(viewport); + + await session.clickTab("Changes"); + await expect(session.files).toBeHidden(); + await waitForFileTreeLayoutSettle(testPage); + await session.clickTab("Files"); + await expect(viewport).toBeVisible({ timeout: 15_000 }); + await expect(folder).toBeVisible({ timeout: 15_000 }); + await expectContiguousVisibleFileTreeRows(viewport); + await expectVisibleFileTreePaths(viewport, collapsedPaths); + + await folder.click(); + const firstFile = largeFileTreePath(0); + await expect(session.fileTreeNode(firstFile)).toBeVisible({ timeout: 15_000 }); + await expectContiguousVisibleFileTreeRows(viewport); + const expandedTopPaths = await visibleFileTreePaths(viewport); + + await session.clickTab("Changes"); + await expect(session.files).toBeHidden(); + await waitForFileTreeLayoutSettle(testPage); + await session.clickTab("Files"); + await expect(session.fileTreeNode(firstFile)).toBeVisible({ timeout: 15_000 }); + await expectContiguousVisibleFileTreeRows(viewport); + await expectVisibleFileTreePaths(viewport, expandedTopPaths); + + const lastFile = largeFileTreePath(LARGE_FILE_TREE_COUNT - 1); + await scrollToLastLargeFile(session.fileTreeNode(lastFile), viewport); + await expect(session.fileTreeNode(lastFile)).toBeVisible({ timeout: 15_000 }); + await expectContiguousVisibleFileTreeRows(viewport); + const scrolledPaths = await visibleFileTreePaths(viewport); + + await session.clickTab("Changes"); + await expect(session.files).toBeHidden(); + await waitForFileTreeLayoutSettle(testPage); + await session.clickTab("Files"); + await expect(session.fileTreeNode(lastFile)).toBeVisible({ timeout: 15_000 }); + await expectContiguousVisibleFileTreeRows(viewport); + await expectVisibleFileTreePaths(viewport, scrolledPaths); + }); }); diff --git a/apps/web/e2e/tests/task/mobile-large-file-tree-virtualization.spec.ts b/apps/web/e2e/tests/task/mobile-large-file-tree-virtualization.spec.ts index 353b74429a..4150421efe 100644 --- a/apps/web/e2e/tests/task/mobile-large-file-tree-virtualization.spec.ts +++ b/apps/web/e2e/tests/task/mobile-large-file-tree-virtualization.spec.ts @@ -1,10 +1,15 @@ import { test, expect } from "../../fixtures/test-base"; +import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions"; import { LARGE_FILE_TREE_COUNT, LARGE_FILE_TREE_FOLDER, + expectContiguousVisibleFileTreeRows, + expectVisibleFileTreePaths, largeFileTreePath, scrollToLastLargeFile, setupLargeFileTreeTask, + visibleFileTreePaths, + waitForFileTreeLayoutSettle, } from "./large-file-tree-virtualization-helpers"; test.describe("Mobile large file tree virtualization", () => { @@ -25,9 +30,23 @@ test.describe("Mobile large file tree virtualization", () => { await testPage.getByRole("button", { name: "Files" }).tap(); const folder = session.fileTreeNode(LARGE_FILE_TREE_FOLDER); + const viewport = session.fileTreeScrollViewport(); await expect(folder).toBeVisible({ timeout: 15_000 }); + await expect(viewport).toBeVisible({ timeout: 15_000 }); + await waitForFileTreeLayoutSettle(testPage); + await expectContiguousVisibleFileTreeRows(viewport); + const collapsedPaths = await visibleFileTreePaths(viewport); + + await testPage.getByRole("button", { name: "Chat" }).tap(); + await expect(viewport).toBeHidden(); + await testPage.getByRole("button", { name: "Files" }).tap(); + await expect(folder).toBeVisible({ timeout: 15_000 }); + await expectContiguousVisibleFileTreeRows(viewport); + await expectVisibleFileTreePaths(viewport, collapsedPaths); + await folder.tap(); await expect(session.fileTreeNode(largeFileTreePath(0))).toBeVisible({ timeout: 15_000 }); + await expectContiguousVisibleFileTreeRows(viewport); await expect .poll(() => session.visibleFileTreeNodes().count(), { timeout: 5_000 }) .toBeLessThan(80); @@ -39,8 +58,6 @@ test.describe("Mobile large file tree virtualization", () => { expect(firstActionsBox!.width).toBeGreaterThanOrEqual(44); expect(firstActionsBox!.height).toBeGreaterThanOrEqual(44); - const viewport = session.fileTreeScrollViewport(); - await expect(viewport).toBeVisible(); await scrollToLastLargeFile( session.fileTreeNode(largeFileTreePath(LARGE_FILE_TREE_COUNT - 1)), viewport, @@ -54,6 +71,8 @@ test.describe("Mobile large file tree virtualization", () => { expect(lastActionsBox).not.toBeNull(); expect(lastActionsBox!.width).toBeGreaterThanOrEqual(44); expect(lastActionsBox!.height).toBeGreaterThanOrEqual(44); + await expectContiguousVisibleFileTreeRows(viewport); + await assertNoDocumentHorizontalOverflow(testPage, "mobile large file tree"); await session.fileTreeNode(lastFile).tap(); const viewer = testPage.getByTestId("mobile-file-viewer-panel"); diff --git a/docs/plans/file-tree-hidden-measurements/plan.md b/docs/plans/file-tree-hidden-measurements/plan.md new file mode 100644 index 0000000000..e71dff4162 --- /dev/null +++ b/docs/plans/file-tree-hidden-measurements/plan.md @@ -0,0 +1,198 @@ +--- +created: 2026-09-16 +status: implemented +requirements: + - REQ-UI-FILE-TREE-CHAT-CONTEXT-001 +system_design: + - ../../specs/ui/system-design/task-surface-render-isolation.md +legacy_specs: [] +--- + +# Implementation Plan: File Tree Hidden Measurements + +## Overview + +Keep visible file-tree rows contiguous after the Files panel hides and reopens. +One work order adds a measurement guard and regression coverage. +The implementation is complete. + +## Ownership and assumptions + +UI owns row geometry and virtualization. Filesystem data and task lifecycle retain their existing owners. +The user supplied the intended complete tree and requested reproduction and repair. +The repair preserves file operations, row order, scroll ownership, and mobile navigation. +No material product choice remains unresolved. + +## Confirmed cause and reproduction + +`VirtualizedFileTreeView` in `apps/web/components/task/file-browser-parts.tsx` +uses the default TanStack row measurement through `ref={virtualizer.measureElement}`. +That measurement accepts zero-height `ResizeObserver` entries from hidden rows. +The entries replace valid cached heights and disrupt the virtual range on restoration. +Desktop Dockview uses `defaultRenderer="always"`, so inactive panels can retain mounted content. + +A temporary browser fixture used the actual `FileTreeView`, shared `ScrollArea`, +row components, providers, and application CSS. Its viewport height was 560px. +The fixture supplied 18 directory rows and a button that toggled the ancestor's +`display` between `flex` and `none`. Two animation frames let each visibility +transition reach the browser observers. An initial owner update connected the +fixture's viewport ref, equivalent to the real tree's asynchronous data arrival. + +| Scenario | First mounted index after restoration | Blank space at viewport top | Mounted rows | +| --- | --- | --- | --- | +| Original, compact rows | 12 | 234px | 6 | +| Temporary guard, compact rows, three cycles | 0 | 0px | 18 | +| Original, 44px touch rows at 393px browser width | 12 | 296px | 6 | +| Temporary guard, 44px touch rows at 393px browser width | 0 | 0px | 18 | + +Desktop `scrollTop` remained zero. The broken state persisted after layout settled. +The touch reproduction changed `scrollTop` to 100px. The temporary guard retained zero. +The touch experiment exercised row sizing in the same fixture, not the complete mobile task layout. + +The experiment changed only a temporary copy of the renderer. It kept a cached +row height, or its estimate, when default measurement returned zero. +This proves a matching failure path, while the permanent implementation and +integration coverage below verify the complete interaction. + +## Scope + +### In scope + +- Guard unavailable row measurements in the file-tree virtualizer. +- Preserve positive dynamic measurements for normal rows and creation controls. +- Cover hidden-panel restoration and existing large-tree navigation on desktop and phone. + +### Out of scope + +- Dependency upgrades, changes to shared ScrollArea, or other virtualized lists. +- File loading, backend APIs, persisted layout, selection, and file operations. +- A new mobile composition or public documentation. + +## Technical approach + +Add `apps/web/components/task/file-tree-measurement.ts` for the typed measurement +callback. Delegate actual measurement to the installed TanStack function. +Accept positive measured heights. Otherwise retain the positive cached value +for `instance.options.getItemKey(index)`, or use `estimateSize(index)`. +Wire this callback into `VirtualizedFileTreeView` without changing its overscan, +row keys, viewport, reveal effects, or mounted-row bound. + +Keep dynamic measurement. Fixed heights cannot cover every inline control or +responsive transition. Increasing overscan can conceal the defect without +protecting the measurement cache. + +## ASCII UI preview + +UI-01: Files panel after restoration, with the scroll position at the top. + +```text +Observed desktop Corrected desktop Corrected phone Files ++-------------------+ +-------------------+ +----------------------+ +| Files toolbar | | Files toolbar | | Files toolbar | +| | | > folder-00 | | > folder-00 [...]| +| blank gap | | > folder-01 | | > folder-01 [...]| +| | | > folder-02 | | > folder-02 [...]| +| > folder-12 | | ... | | ... | +| > folder-13 | | > folder-17 | +----------------------+ ++-------------------+ +-------------------+ | Existing bottom nav | + +----------------------+ +``` + +The toolbar and phone navigation retain their current positions. +The existing Files viewport is the only vertical scroll owner. +All rows intersecting that viewport must appear without gaps or overlaps +(`AC-UI-FILE-TREE-CHAT-CONTEXT-001.10`). Names and spacing in this sketch are illustrative. + +The mobile exemplar is `components/task/mobile/session-mobile-layout.tsx`. +Users enter through the Files bottom-navigation action and tap a row to open it. +The existing full-height surface supports repeated file exploration. +It retains touch actions, safe-area clearance, and the current shared tree model. +Mobile panel switching currently unmounts Files, so tests cover that real path +in addition to hidden-measurement component coverage with touch-sized rows. + +## Tests + +`components/task/file-tree-measurement.test.ts` covers the measurement contract: + +- A zero observer entry retains the previous positive height. +- A zero first measurement uses the node or creation-row estimate. +- A later positive measurement replaces the fallback after restoration or a touch-size change. + +These cases support `.10` while preserving compact spacing `.9` and touch actions `.7`. +Use the real TanStack measurement function with controlled element geometry. +The zero-height regression must fail before the guard is implemented. + +`components/task/file-tree-geometry.test.ts` covers the shared +viewport validator. It rejects a blank top edge, rejects a blank bottom edge when +the tree overflows the viewport, and permits legitimate trailing space for a short +tree or the file-tree container's end padding. + +## E2E tests + +Extend the existing desktop and mobile virtualization specs, preserving their current scenarios. + +- Desktop: hide Files through an actual Dockview tab transition, then restore it. + Cover a short tree at the top and a large tree after scrolling. + Assert the first visible row reaches the viewport content top, the last visible + row reaches the bottom when overflowing content continues below, and adjacent + rows have no gaps or overlaps. The short-tree transition compares its ordered + visible path window before and after restoration. +- Phone: enter Files, navigate away and return, then scroll and open a file. + Reuse the same top, bottom, and adjacent-row validator, compare the ordered + short-tree path window after Files to Chat to Files navigation, and assert + reachable touch actions and no horizontal document overflow. +- Preserve the fewer-than-80 mounted-row bound for the 600-entry fixture, + last-file access, and create-input reveal. + +The implementation work order contains the exact build and test commands. + +## Work orders + +- [x] [Task 01: Preserve valid file-tree measurements](task-01-preserve-measurements.md) + +This work order is sequential and has no dependencies. + +## Verification results + +Temporary browser reproduction and guard comparison passed as recorded above. +The production measurement callback now retains a positive row-keyed cache entry, +or the current row estimate, when TanStack receives a zero-height observer entry. +Positive measurements remain authoritative for compact rows, touch rows, and +inline creation controls. + +- Focused Vitest: 4 files and 14 tests passed, including four shared viewport + geometry cases for blank top, overflowing blank bottom, short-tree trailing + space, and end padding. +- Frontend typecheck passed. +- Targeted ESLint passed for the measurement implementation, renderer, and E2E helpers/specs. +- Production-served desktop E2E: 3 tests passed, including collapsed, expanded-top, + and scrolled Files restoration through Dockview. The restoration assertions + compare ordered visible path windows at the same scroll offsets. +- Production-served mobile E2E: 3 tests passed, including the large-tree touch flow + and the two existing file-tree chat-context scenarios. The large-tree flow uses + the shared viewport validator and compares the collapsed path window after + Files navigation. +- E2E sleep lint passed. +- `python3 scripts/list-docs.py validate`: passed (281 decisions, 973 specifications). +- `python3 scripts/lint-spec-files.py --all`: passed. +- `git diff --check`: passed. + +The temporary browser fixture, copied renderer, and browser configuration were +removed after the experiment. The named browser and isolated app were stopped. +Public documentation does not change. + +## Risks + +- Disabling measurement entirely would break variable-height controls. +- A fallback keyed by index can reuse another row's size after a tree change. +- Shared viewport checks must track intentional content insets and end padding if + the Files layout changes. +- The isolated fixture remains diagnostic evidence; the production E2E scenarios + cover the Dockview and mobile navigation lifecycles. + +## Related implementation records + +- [Original virtualization work order](../task-surface-render-isolation/task-02-virtualize-file-tree.md). +- [Responsive row spacing](../file-tree-responsive-spacing/plan.md). + +Their historical results remain unchanged. This package owns the new visibility regression. diff --git a/docs/plans/file-tree-hidden-measurements/task-01-preserve-measurements.md b/docs/plans/file-tree-hidden-measurements/task-01-preserve-measurements.md new file mode 100644 index 0000000000..9f08c27314 --- /dev/null +++ b/docs/plans/file-tree-hidden-measurements/task-01-preserve-measurements.md @@ -0,0 +1,161 @@ +--- +id: "01-preserve-measurements" +title: "Preserve valid file-tree measurements" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-UI-FILE-TREE-CHAT-CONTEXT-001 +acceptance_criteria: + - AC-UI-FILE-TREE-CHAT-CONTEXT-001.7 + - AC-UI-FILE-TREE-CHAT-CONTEXT-001.9 + - AC-UI-FILE-TREE-CHAT-CONTEXT-001.10 +system_design: + - ../../specs/ui/system-design/task-surface-render-isolation.md +--- + +# Task 01: Preserve Valid File Tree Measurements + +## Summary + +Reject zero-height measurements from hidden file-tree rows. +Retain valid cached geometry until the browser can measure each row again. +The [plan](plan.md#confirmed-cause-and-reproduction) records the matching browser reproduction. + +## In scope + +- A typed file-tree measurement callback and focused regression tests. +- Wiring the callback into `VirtualizedFileTreeView`. +- Desktop panel-restoration and phone Files-navigation E2E coverage. + +## Out of scope + +Dependency upgrades, global observer changes, panel remount policies, backend +changes, public copy, and persisted state. + +## Acceptance + +1. Unavailable measurements use a positive cache entry for the current row key, + or its current estimate. Positive measurements remain authoritative. +2. Files restoration shows every expected viewport row without gaps or overlaps, + including compact and touch-sized rows (`.9`, `.10`). +3. Large trees retain bounded mounting, last-file access, create reveal, and + reachable 44px touch actions (`.7`). + +## ASCII UI preview + +UI-01: Files restored at the top. Full comparison: [plan preview](plan.md#ascii-ui-preview). + +```text +Desktop Files Phone Files ++-------------------+ +----------------------+ +| Files toolbar | | Files toolbar | +| > folder-00 | | > folder-00 [...]| +| > folder-01 | | > folder-01 [...]| +| > folder-02 | | > folder-02 [...]| +| ... | | ... | ++-------------------+ +----------------------+ + | Existing bottom nav | + +----------------------+ +``` + +The existing viewport owns scrolling. Rows remain contiguous after restoration. +Phone touch actions retain their current geometry. This implements `.7`, `.9`, and `.10`. + +## Implementation sequence + +1. Mark this work order `in_progress`. +2. Add `file-tree-measurement.test.ts` with the zero-height regression. +3. Run it against a callback that delegates to the current default measurement. +4. Record the behavioral failure: expected cached height or estimate, received zero. +5. Implement the fallback and wire the callback into the real virtualizer. +6. Extend both existing virtualization specs with the plan's transition scenarios. +7. Add shared viewport geometry assertions and focused cases for blank top, overflowing + blank bottom, legitimate short-tree trailing space, and end padding. +8. Run every verification command and record the results. +9. Compare the rendered desktop and phone geometry with UI-01. +10. Mark this work order `done` and synchronize the plan. + +## Verification + +Run from the repository root. If dependencies are absent, first run +`(cd apps && pnpm install --frozen-lockfile)`. + +```bash +(cd apps/web && pnpm exec vitest run components/task/file-tree-measurement.test.ts components/task/file-browser-render-identity.test.tsx components/task/file-browser-responsive.test.tsx components/task/file-tree-geometry.test.ts) +(cd apps/web && pnpm run typecheck) +(cd apps/web && pnpm exec eslint components/task/file-tree-measurement.ts components/task/file-tree-measurement.test.ts components/task/file-tree-geometry.test.ts components/task/file-browser-parts.tsx e2e/tests/task/file-tree-geometry.ts e2e/tests/task/large-file-tree-virtualization-helpers.ts e2e/tests/task/large-file-tree-virtualization.spec.ts e2e/tests/task/mobile-large-file-tree-virtualization.spec.ts) +(cd apps/web && pnpm e2e:run --project chromium tests/task/large-file-tree-virtualization.spec.ts) +(cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-large-file-tree-virtualization.spec.ts tests/task/mobile-file-tree-chat-context.spec.ts) +(cd apps/web && pnpm run lint:e2e-sleeps -- e2e/tests/task/large-file-tree-virtualization.spec.ts e2e/tests/task/mobile-large-file-tree-virtualization.spec.ts) +python3 scripts/list-docs.py validate +python3 scripts/lint-spec-files.py --all +git diff --check +``` + +The managed E2E commands build the current production sources. +Run desktop and mobile commands sequentially. + +## Files likely touched + +- `apps/web/components/task/file-tree-measurement.ts` (new). +- `apps/web/components/task/file-tree-measurement.test.ts` (new). +- `apps/web/components/task/file-browser-parts.tsx`. +- `apps/web/e2e/tests/task/file-tree-geometry.ts` (new). +- `apps/web/components/task/file-tree-geometry.test.ts` (new). +- `apps/web/e2e/tests/task/large-file-tree-virtualization.spec.ts`. +- `apps/web/e2e/tests/task/mobile-large-file-tree-virtualization.spec.ts`. +- `apps/web/e2e/tests/task/large-file-tree-virtualization-helpers.ts`. + +## Dependencies + +None. + +## Risks + +Cache identity must follow the row key. Accept later positive measurements so +responsive sizes and inline controls can change height. +Actual mobile Files navigation unmounts the tree. It does not reproduce desktop +Dockview lifetime, so retain the controlled hidden-measurement regression too. + +## Parallelism + +`sequential` + +## Inputs + +- [Requirements](../../specs/ui/requirements/file-tree-chat-context.md), criteria `.7`, `.9`, and `.10`. +- [System design](../../specs/ui/system-design/task-surface-render-isolation.md#file-tree-virtualization). +- Existing desktop and mobile virtualization specs and their shared fixture. +- `file-browser-render-identity.test.tsx` for nearby component-test conventions. + +## Results + +- RED: the focused measurement test against TanStack's default callback returned + `0` for a hidden row where the positive cached height was `32`. +- GREEN: `file-tree-measurement.ts` delegates positive measurements to TanStack + and otherwise retains the current row-keyed positive cache entry or estimate. + `VirtualizedFileTreeView` uses the callback without changing its scroll owner, + row keys, estimates, overscan, or reveal effects. +- Review RED: the original intersection-filtered viewport-edge checks accepted + synthetic blank top and bottom gaps because every retained row satisfied the + later edge comparisons. +- Review GREEN: the shared pure viewport validator checks the first row's top, + the last row's bottom for overflowing trees, and adjacent row bounds. It skips + the bottom fill requirement for short trees and allows the container's end + padding. Four focused geometry cases passed. +- Focused Vitest passed 4 files and 14 tests. It covers cached zero measurements, + first-measurement estimates, later positive measurements, and the shared + blank-edge/short-tree geometry contract. +- Frontend typecheck and targeted ESLint passed. +- Desktop production-served E2E passed 3 tests. The new scenario restores the + collapsed tree, expanded tree at the top, and expanded tree after scrolling + through actual Dockview tab transitions. It compares ordered visible path + windows before and after restoration and checks visible row bounds. +- Mobile production-served E2E passed 3 tests. The large-tree scenario checks + Files to Chat to Files navigation, the shared contiguous-row validator, the + ordered collapsed path window, 44px actions, last-file access, and document + overflow, alongside the existing chat-context tests. +- E2E sleep lint, specification validation, specification lint, and `git diff --check` + all passed. diff --git a/docs/specs/ui/requirements/file-tree-chat-context.md b/docs/specs/ui/requirements/file-tree-chat-context.md index e4977b3965..7df6055966 100644 --- a/docs/specs/ui/requirements/file-tree-chat-context.md +++ b/docs/specs/ui/requirements/file-tree-chat-context.md @@ -28,6 +28,7 @@ Users inspecting a task's Files tab must currently return to the chat composer a - **AC-UI-FILE-TREE-CHAT-CONTEXT-001.7:** On phone, tablet, and coarse-pointer desktop-workbench compositions, each eligible file-tree row exposes a visible, accessible action trigger. The trigger opens the existing responsive menu treatment with a touch target at least 44px high; long press or right-click is not required. - **AC-UI-FILE-TREE-CHAT-CONTEXT-001.8:** All new visible labels and feedback use the existing localization catalogs. - **AC-UI-FILE-TREE-CHAT-CONTEXT-001.9:** File-tree rows shall retain compact single-line spacing in every responsive composition. A visible coarse-pointer action, including in the desktop workbench, shall not cause wrapping; long names shall truncate instead. +- **AC-UI-FILE-TREE-CHAT-CONTEXT-001.10:** After the Files panel reopens or changes size, every row within its scroll viewport shall remain visible and ordered without blank gaps. Recovery shall not require a manual scroll, file operation, or page reload. This applies to desktop and touch layouts. ## Migrated source detail diff --git a/docs/specs/ui/system-design/task-surface-render-isolation.md b/docs/specs/ui/system-design/task-surface-render-isolation.md index 978dbad0ff..825956403c 100644 --- a/docs/specs/ui/system-design/task-surface-render-isolation.md +++ b/docs/specs/ui/system-design/task-surface-render-isolation.md @@ -76,6 +76,16 @@ Rows use measured sizes because editing controls and touch layouts can change ro virtualizer uses an estimated size before measurement. It updates positions after a measured size changes. +Only positive row measurements can replace cached sizes. A hidden panel can +produce zero-height observer entries even though its rows remain connected. +Those entries represent unavailable geometry, not a row with no content. +The measurement callback retains the last positive size for the row key, or +uses the current estimate when no positive measurement exists. +When the panel becomes visible, normal measurement accepts its actual row sizes. +This applies to node rows and inline creation controls in both pointer modes. +It implements `AC-UI-FILE-TREE-CHAT-CONTEXT-001.10` without another scroll owner +or changes to Dockview panel lifetime. + Active-file reveal and restored scroll operations resolve a row index from `visibleRows`. They use the virtualizer scroll API before they query a row element. This sequence makes an offscreen row available before focus or selection work uses its element. @@ -148,6 +158,11 @@ task data as the current complete list. Unit tests record render counts for controlled updates. Browser E2E tests record the number of mounted rows for a large generated file tree. +Measurement tests cover positive sizes, unavailable sizes with a cached value, +and unavailable sizes before the first measurement. Browser regressions hide +and restore the Files panel, then compare visible row bounds with the viewport. +Coverage includes compact desktop rows, touch rows, and scrolled large trees. + Final performance evaluation uses a warmed production build without React DevTools or CPU profiler startup in the measured window. The report includes main-thread work, long tasks, and mounted-row counts. Development traces remain diagnostic evidence only.