[AGE-3965] refactor(drive): virtualize surfaces + unify Files drawer - #5400
Conversation
Opening a session whose agent wrote a large file tree (11k+ files) into its cwd froze the main thread 2.5s+ and re-froze on every session update. Compute: replace O(n^2) folder inference in driveTree.ts with a precomputed ancestor-directory index (O(1) lookup); compute the file list and total size in one pass (driveFileStats). Split the useSessionDrive memo so structural work depends only on the listings, not the file-activity ticks, and index the recency stamping by path instead of rescanning every file per signal. Render: the listing is fetched whole and was rendered one DOM node per file. Virtualize the chat Files grid, the explorer folder view (VirtualTileGrid) and the file tree (flattened + windowed, index-based keyboard nav) with @tanstack/react-virtual; defer search with useDeferredValue. Make overscan the thumbnail prefetch window (FileThumb margin 600px, grid overscan 4 rows) so tiles load ahead of scroll.
…on scroll Follow-up to virtualizing the drive surfaces, from testing an 11k-file drive. Thumbnails: drop the IntersectionObserver gate (the grids are virtualized, so a mounted tile is already in view) and cache thumbnail bytes in a dedicated query with a short gcTime, so scrolling a tile out of the window and back no longer refetches and flashes. Memoize FileThumb so grid re-renders on scroll don't touch resolved tiles, and show a skeleton (not the type icon) while a thumbnail loads. Grid: the chat Files grid browsed nothing — it flat-dumped every file, unusable after an agent clones a repo. It now browses folders (breadcrumb + folder tiles), with search flattening to matches across the whole tree. Treat .mdx as markdown so docs repos show snippets.
…summary tier, client thumbnails, view modes Backend (mounts): get_mount_files/list_files gain order+limit+total for a bounded 'latest N' listing, and are git-aware in every mode — .git plumbing is never listed and .gitignore-matched paths are dropped (pathspec). Gitignore matching walks ancestor directories (git's model) so deeply-nested descendants like pnpm's node_modules/.pnpm/<pkg>/node_modules/... are ignored, and .gitignore files inside already-ignored dirs are never read (no read-storm/timeout after an install). Frontend: a lightweight summary tier (one merged latest-5 query per mount serving both the count via total and the recents) for the always-mounted chrome (rail, config Files section, runtime lens) so they never pull the whole tree; the browse surfaces keep the full drive, gated on open. Client-side downscaled thumbnails (cache the small data-URL, drop the original) keep memory bounded. FilesWindow gains Flat / Folders / List views with one persistent toolbar (search + origin + sort + show-hidden), responsive tile columns, and the shared DriveExplorer tree.
…ive fix + tests
- GET /mounts/{id}/files/page: path-sorted cursor pages (store list_objects_page via start_after),
git-aware pruning that jumps past ignored dirs so node_modules is never paged through
- fix stream_mounts_archive mtime ms->s (was crashing the zip generator mid-stream -> 0-byte files)
- unit tests for the paged listing
…veal transitions, grid/flat keyboard nav, download-all, context menus
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds git-aware mount listing with totals, shallow traversal, folder counts, streamed multi-mount ZIP archives, session drive summaries, lazy loading, virtualization, contextual file actions, thumbnails, repository metadata, and a unified session-scoped file drawer. ChangesMount browsing and archive backend
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SessionFilesDrawer
participant FilesDrawer
participant DriveExplorer
participant MountsRouter
participant MountsService
SessionFilesDrawer->>FilesDrawer: provide session-scoped drive state
FilesDrawer->>DriveExplorer: mount controlled explorer
DriveExplorer->>MountsRouter: request root or directory listing
MountsRouter->>MountsService: list_files with browsing options
MountsService-->>MountsRouter: return files and totals
MountsRouter-->>DriveExplorer: return listing response
DriveExplorer-->>FilesDrawer: render virtualized explorer
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Railway Preview Environment
Updated at 2026-07-20T19:49:32.334Z |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
web/oss/src/components/Drives/driveFlatFiles.ts (1)
60-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffRaw axios call deviates from the Fern-client guideline.
This endpoint is fetched via raw axios with
{params}, and the hook drives pagination throughuseState/useEffectrather thanatomWithQuery. Both deviate from the web guidelines. This appears intentional (the PR notes the Fern client wasn't regenerated for/mounts/{id}/files/page); flagging so the follow-up to regenerate the client and move this onto the Fern client is tracked. As per coding guidelines: "New frontend API calls must use the Fern client viagetAgentaSdkClient({host: getAgentaApiUrl()}), not raw axios" and "UseatomWithQuerywith TanStack Query for API data fetching … do not useuseEffectwith manual fetching for new code."Source: Coding guidelines
web/oss/src/components/Drives/chatFileRefs.tsx (1)
34-37: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winTighten the file-candidate heuristic to cut spurious existence checks.
/[./]/.test(t)matches almost any token with a dot or slash — abbreviations ("e.g."), version numbers ("v1.2.3"), decimals ("3.14"), and property-access chains ("user.name") all qualify as "file candidates." Since these aren't already known from records, each triggersOnDemandFileRef's on-demand content fetch once scrolled into view — a guaranteed-404 network request for ordinary prose/code that happens to contain a dot.♻️ Possible tightening
const fileCandidate = (text: string): string | null => { const t = text.trim().replace(/^\.?\/+/, "") - return t && /[./]/.test(t) ? t : null + // Require a path-ish shape: a slash, or a short trailing extension (avoids matching + // decimals, abbreviations, and dotted identifiers like `user.name`). + return t && /\/|\.[A-Za-z0-9]{1,8}$/.test(t) ? t : null }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a9075beb-2572-466d-9719-0ffa16499772
⛔ Files ignored due to path filters (4)
api/uv.lockis excluded by!**/*.lockweb/packages/agenta-api-client/src/generated/api/resources/mounts/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/mounts/client/requests/GetMountFilesRequest.tsis excluded by!**/generated/**web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (46)
api/oss/src/apis/fastapi/mounts/models.pyapi/oss/src/apis/fastapi/mounts/router.pyapi/oss/src/apis/fastapi/mounts/utils.pyapi/oss/src/core/mounts/dtos.pyapi/oss/src/core/mounts/service.pyapi/oss/src/core/store/storage.pyapi/oss/tests/pytest/unit/test_mounts_file_ops.pyapi/pyproject.tomlweb/oss/package.jsonweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/assets/markdown.tsxweb/oss/src/components/AgentChatSlice/components/Inspector/lenses/RuntimeLens.tsxweb/oss/src/components/AgentChatSlice/state/fileLinks.tsweb/oss/src/components/Drives/ContextRail.tsxweb/oss/src/components/Drives/DriveDrawer.tsxweb/oss/src/components/Drives/DriveExplorer.tsxweb/oss/src/components/Drives/DriveExplorerSkeleton.tsxweb/oss/src/components/Drives/DriveFileLinkProvider.tsxweb/oss/src/components/Drives/DriveFileRow.tsxweb/oss/src/components/Drives/DriveItemContextMenu.tsxweb/oss/src/components/Drives/FileThumb.tsxweb/oss/src/components/Drives/FilesDrawer.tsxweb/oss/src/components/Drives/FilesDrawerBody.tsxweb/oss/src/components/Drives/FilesWindow.tsxweb/oss/src/components/Drives/SessionFilesDrawer.tsxweb/oss/src/components/Drives/StorageFilesHeader.tsxweb/oss/src/components/Drives/StorageSection.tsxweb/oss/src/components/Drives/VirtualTileGrid.tsxweb/oss/src/components/Drives/agentDrive.tsweb/oss/src/components/Drives/chatFileRefs.tsxweb/oss/src/components/Drives/configDrive.tsweb/oss/src/components/Drives/driveFlatFiles.tsweb/oss/src/components/Drives/driveKinds.tsweb/oss/src/components/Drives/driveMedia.tsweb/oss/src/components/Drives/driveRepo.tsweb/oss/src/components/Drives/driveTree.tsweb/oss/src/components/Drives/fileMeta.tsxweb/oss/src/components/Drives/repoMeta.tsxweb/oss/src/components/Drives/useLazyDriveTree.tsxweb/oss/src/components/Drives/useSessionDrive.tsweb/oss/src/styles/globals.cssweb/packages/agenta-entities/src/session/api/api.tsweb/packages/agenta-entities/src/session/api/client.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/src/session/index.tsweb/packages/agenta-entities/src/session/state/mounts.ts
💤 Files with no reviewable changes (3)
- web/oss/src/components/Drives/FilesDrawerBody.tsx
- web/oss/src/components/Drives/FilesWindow.tsx
- web/oss/src/components/Drives/DriveDrawer.tsx
Dead exports left after the drawer unification / lazy-loading rewrite: - gridArrowKeyDown (VirtualTileGrid owns grid nav now) - useAgentDrive (eager agent-drive fetch, obsolete after lazy loading) - driveFiles / driveTotalSize wrappers (last caller removed) Also orders the RuntimeLens drive imports (import/order lint).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/oss/src/components/AgentChatSlice/components/Inspector/lenses/RuntimeLens.tsx (1)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove cross-module state to a shared store.
This component in
AgentChatSliceis importing thefilesDrawerOpenAtomFamilyatom directly from a component file (SessionFilesDrawer) in theDrivesmodule, which tightly couples the modules.As per coding guidelines: "Keep module-specific code within its module; move functionality shared across modules to the appropriate root components, hooks, assets, or types location" and "Prefer Jotai atoms for shared state, keep UI-only state local, place module state in module stores, and place cross-module state in the root store". Please extract
filesDrawerOpenAtomFamilyinto a shared root store (or shared state file) so it can be imported cleanly across modules.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f3a413e-6722-435a-a4e2-e37f5a620305
📒 Files selected for processing (4)
web/oss/src/components/AgentChatSlice/components/Inspector/lenses/RuntimeLens.tsxweb/oss/src/components/Drives/agentDrive.tsweb/oss/src/components/Drives/driveKeyboard.tsweb/oss/src/components/Drives/driveTree.ts
💤 Files with no reviewable changes (1)
- web/oss/src/components/Drives/driveKeyboard.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web/oss/src/components/Drives/agentDrive.ts
…ders, surface flat-page errors - ArchiveMount.mount_id typed as UUID so a malformed id is rejected at the boundary (422) instead of raising an uncaught ValueError (500) inside the archive handler. - Content-Disposition filenames (single-file download + archive) routed through one RFC 6266 helper that strips quotes/control chars and adds a percent-encoded filename*, closing a header-injection vector. - Flat-view page-load errors no longer force hasMore=false: the cursor is preserved and the footer becomes a retry affordance, so a mid-list failure is visible and recoverable. - Rename loop var shadowing re.sub; tighten chat file-link heuristic to a path-ish shape to cut guaranteed-404 probes.
…niteQuery
Addresses the review note that driveFlatFiles used raw axios + a hand-rolled useState/useEffect fetch loop, deviating from the web data-fetching guidelines.
- Regenerate the TypeScript Fern client from the local OpenAPI so the new mount routes are typed: adds mounts.getMountFilesPage + mounts.archiveMountFiles and their models (purely additive).
- Add queryMountFilePage to @agenta/entities/session (Fern client via getMountsClient + projectScopedRequest + zod validation), mirroring queryMountFiles/queryLatestMountFiles.
- Rewrite useFlatFilesInfinite on TanStack Query's useInfiniteQuery (shared QueryClient): the two folded mounts thread through one {srcIdx, cursor} page chain via getNextPageParam. Same public API and behavior; gains cache-backed instant reopen, native in-flight de-dup and retry, and removes the manual ref/effect state machine and raw axios.
…idth expand
UI simplification per design feedback — one view, less chrome:
- Remove the list/grid/flat view switcher and the flat view entirely; the drawer is now the tree navigator (tree pane + content pane) with a show/hide toggle for the tree pane (first toolbar slot, above the tree). The toggle animates: the Splitter stays mounted and the tree pane collapses to width 0 via a gated transition class (mirrors the playground splitter pattern; off during drag so resize stays 1:1).
- Fix QA's nested tree scrollbars: box-border on the tree container (preflight-off content-box was overflowing the overflow:auto Splitter panel).
- Add the app's full-width drawer-expand pattern (ArrowsOut/ArrowsIn in the header; 960px <-> clamp(960px,92vw,1800px), reset on close).
- Files-opener icon: ArrowSquareOut (read as 'open new tab') -> FolderOpen, in BOTH the config StorageFilesHeader and the chat ContextRail.
Dead code removed after dropping the flat view (FE + BE):
- FE: driveFlatFiles.ts, entity queryMountFilePage + schema + exports.
- BE: GET /mounts/{id}/files/page route + handler, list_files_page, list_objects_page, _first_ignored_ancestor_dir, MountFilePageResponse.
- Generated client: dropped getMountFilesPage + its request/response types (the /files/archive download-all route is untouched).
|
Hi @ardaerzin, following the backend review of this PR, I opened two small stacked PRs that implement the agreed pre-merge fixes so this can land on a sound base:
The intended order is #5411 into this branch, then #5412 into #5411, then this PR into main. Nothing here is merged; that stays your and Mahmoud's call. Happy to move any of it around or discuss the approach. Review and comments welcome on the two PRs. |
…of a broken 200 The download-all archive resolved each mount lazily inside the streaming generator, after the 200 headers were sent, so MountNotFound / MountStorageUnavailable never reached @handle_mount_exceptions and a bad mount id yielded a corrupt zip. Split iter_archive_members into build_archive_work_list (resolve + list, awaited before the StreamingResponse is built) and iter_archive_members (prefetch/yield only), so those errors surface as real 404/503 through the decorators. Claude-Session: https://claude.ai/code/session_01PTkjdoAPbhSEFR3ubpgzBU
The raw (non-git-aware) count-only path ran a full uncapped list_objects_v2 and hardcoded truncated=False, so a huge tree was enumerated whole. Page it bounded at _COUNT_CAP and report total_capped=True when more remain, matching the git-aware branch's bounded-count contract. Re-adds the generic ObjectStore.list_objects_page pagination primitive (removed alongside the flat Files view); this bounded count is now its sole consumer. Claude-Session: https://claude.ai/code/session_01PTkjdoAPbhSEFR3ubpgzBU
|
Confirmed out-of-band: the flat/paged view is gone for good. I've rebased #5411 and #5412 onto |
fix(api): relax drive path validation, harden archive, rename export route
fix(api): eager archive errors, capped count-only listing, rollup tests
Replace the antd Splitter tree/content split with a plain motion.div whose width animates 0<->width in one continuous pass, with the content pane flex-filling alongside. Fixes the double-resize: antd's Splitter re-derived panel flex-basis via its own ResizeObserver AFTER the CSS-transition window, snapping the width a second time (and re-flowing the grid twice). A motion tween has no such re-derivation. - Tree width is draggable via a lightweight pointer handle (thin 1px line + wide hit area, accent on hover/drag), persisted across a hide/show, clamped 180-480px; drag follows the pointer 1:1 (tween off), the toggle animates. - Grid keeps its robust CSS-only sizing (tiles resize via grid 1fr as the pane grows); no motion-layout on the tiles (it fought the CSS resize and janked). The column count snaps once when it crosses a threshold. - Remove the now-dead ag-drive-tree-splitter globals.css rules.
…me-perfect tree drag Give the tile grid's geometry a single owner — motion — so the column reflow finally animates without fighting CSS: - VirtualTileGrid drops the CSS grid entirely: each visible tile is an absolutely positioned motion.div whose x/y/width the component computes from the measured pane width. Same column count → targets apply with duration 0 (1:1 tracking, like 1fr); a column flip → a short spring window (detected during render, so the first frame animates). Virtualization is unchanged — rows window exactly as before, fed by uniform tile heights (width x 3/4 + measured text-block constant). - Anticipated reflow: on a tree-pane toggle, DriveExplorer announces the width shift, and the grid lays out ONCE for the final rest width and springs every tile straight to its final slot — one monotonic morph, no grow-then-shrink through the threshold. Chained announcements resolve rapid re-toggles; self-clears when the live width arrives (or 600ms safety). - Two overdamped springs by cause: soft pane-matched (400/46) for anticipated toggles, tight (700/60) for live mid-drag flips so tiles stay on the pointer's heels. 12px hysteresis stops threshold flip-flop when parking the drag handle on a boundary. - Tree drag rides a MotionValue pair (paneW/innerW): the pointer writes the DOM directly through motion's frame loop — zero React renders per move (state-per-move re-rendered the whole explorer per pointer event). Rest width commits once at drag end; the toggle animates the same value, so drag and toggle share one owner. Inner content reflows during a drag but clips during a collapse. - overflow-x-hidden on the grid scroll container: during an anticipated widen, tiles already sit at their final (wider) slots — the new column slides in from the clipped edge instead of flashing a horizontal scrollbar.
What was tackled (persistence layer, cold-load request reduction, boot render gates, session-UI render cost, request dedup, latest-revision persistence), what was deliberately left alone and why (Phase 2 list persistence, the sidebar Menu, deep config-panel work, org persistence, the billing 502, the serial boot gate chain), and the learnings worth not relearning. Explicitly scoped OUT: the drive/file/mount surfaces, which ship separately in PR Agenta-AI#5400 that this branch is stacked on.
Collects design and go-to-market material that was sitting uncommitted in the working tree so it survives a workspace cleanup. Design and research notes (docs/design/agent-workflows/): - selfhost-hardening plan and the runner engine-quality roadmap - research on client-tool delivery and concurrent human-in-the-loop approvals - the desloppify round-two assessment - root-cause writeups for warm-resume execution and frontend approval dispatch - the PR Agenta-AI#5400 mounts review handoff, review report and fix brief - the retheme hex audit and the stale positioning sweep Go-to-market material (docs/design/gtm/), moved out of the repo root: - the go-to-market wiki (was gtm.txt) - the README rewrite draft (was readme.new.md) - pricing page revision 3 (was website/new-pricing.json, unreferenced by the site) - the sign-in redesign handoff with its logos and design tokens
Context
The drive Files surfaces (the chat Files rail, the config Files section, and the Files drawer) rendered the whole file tree in the DOM. An agent that cloned an 11k-file repo into its cwd froze the main thread on open (#5367). The drawer also fetched the entire tree up front, so opening a multi-GB mount blocked for seconds. On top of that, two separate drawer implementations had drifted apart.
What this does
Rewrites the drive/files surfaces around lazy, virtualized loading, and unifies the two drawers into one.
@tanstack/react-virtual, so a 12k-entry folder never floods the DOM. The old O(n²) folder inference is now O(n).depth=1plus child counts) instead of the whole tree, so opening a huge mount is instant.GET /mounts/{id}/files/pagestreams path-sorted pages via the object store'sstart_after, pruning.gitand gitignored directories (it jumps pastnode_modulesinstead of paging through it). The frontend loads pages on scroll, so first paint is fast regardless of mount size.DriveDrawerand chatFilesWindow) are replaced by a single controlledDriveExplorerwith list / grid / flat view modes, shared by both hosts.agent-files), read with bounded concurrency. On Chromium it streams straight to disk via the File System Access API.The flat listing shape, before and after:
Before (whole tree, built client-side):
After (one cursor page):
Tests / notes
api/oss/tests/pytest/unit/test_mounts_file_ops.py. The streaming-archivemtimefix (milliseconds to seconds, which was producing 0-byte zips) is included here.tsc) and lint-clean across the Drives module./files/page. The drive module calls it via axios, the same way it reaches its other binary/download routes. Noted as a follow-up if we want it in the generated client.AgentChatSliceconversation, markdown, runtime inspector).What to QA