Scale file browser performance for large folders - #45
matchaboar wants to merge 3 commits into
Conversation
Move file-tree and batched Git work off the main actor, publish stable flat row models, and lazily realize sidebar rows with cancellation and signpost coverage. Add focused identity tests plus deterministic benchmark and issue nonatofabio#36 verification guidance.
|
Issue #36 automation is now pushed in cfdce0f. The test drives the production SwiftUI sidebar in-process and reads the underlying NSScrollView, so it needs no Accessibility grant. Controlled mutation: 23,295.150 pt failure; production: 198 transitions within 0.250 pt. The new read-only macOS workflow runs it in CI. |
|
The background scanning and stable rows look like a good direction, but there's a security issue we need to fix before merging. GitFileMetadata.swift runs Git with the repository's configuration still active. A local test with a harmless core.fsmonitor hook confirmed that both the diff and ls-files commands execute it. Since metadata collection starts when a document opens, this can run code just from reading a file, even with the sidebar hidden. This requires a repo directory with that local Git configuration; a tracked Markdown file alone isn't enough. Please disable fsmonitor and check the other Git config/attribute paths that can launch commands during these reads. Add a regression test using a real fixture repo with a marker hook, and verify it never runs. If we can't make automatic Git reads safe, let's keep the Git metadata part out of this PR for now. The history query also reads the full matching history into memory. Please put a bound on that work and test it with a repo that has a long history. Also clarify the benchmark wording: the per-file Git baseline is a comparison implementation, not something current main does. The 89 logic checks pass locally from the workspace, though the first run under /private/tmp hit path-normalization assertions. Please make those tests handle the path alias consistently too. Finally, rebase onto current main, coordinate the sidebar changes with #44, and get the build and scroll-stability CI passing on the result. |
Summary
Focused performance split from #43, rebased onto current
main.LazyVStackinstead of eagerly constructing the entire tree.Red/green evidence
The focused logic harness was first wired against the
mainshape and failed because the performance implementation files did not exist. With the implementation present,./run-tests.shpasses 56 focused FileTree, FileBrowserState, and Git metadata checks plus the existing SSH harness.Issue #36 now has an in-process AppKit/SwiftUI regression harness.
./run-ui-tests.shmounts the productionFileBrowserSidebarinNSHostingView, scrolls its actualNSScrollViewthrough 18 positions in a 1,200-row tree, and measures native clip-view offset across tab-count changes (including 2→1), selected-row changes, and collapse/re-expand cycles.A controlled unstable-identity mutation proved the test goes red:
The production implementation is green:
This avoids System Events and requires no Accessibility permission. TCC grants are user-controlled and cannot safely be self-issued; Linux containers cannot run AppKit. A dedicated
contents: readmacOS workflow runs both harnesses with persisted checkout credentials disabled.Performance runs
Environment: macOS 26.6.1, Apple M5 Max, optimized Swift; 1,000-document fixture, 1,231 visible rows, five runs via
scripts/profile-file-browser.sh 1000 5 5.The Git baseline uses 10 per-file processes versus three batched processes. Larger slow-baseline samples exceeded the five-minute bound and were deliberately not reported.
LazyVStack fallback
FileBrowserRowStackremains the adapter seam if a human acceptance pass finds an OS-specific issue not reproduced by CI. The narrow fallback is a.lazy/.eagerStablerealization policy that swaps onlyLazyVStackforVStack, retaining flat stable rows, background work, batching, and generation guards. If eager rendering is still too expensive, the longer-term adapter isNSCollectionViewwith explicit scroll-origin restoration.Validation
./run-tests.sh— passes./run-ui-tests.sh— 198 transitions, maximum 0.250 pt movement./build.sh— passes.github/workflows/performance-tests.yml— isolated read-only macOS CI job