Files tab v2 and Cursor chat reliability#449
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
Warning Review limit reached
More reviews will be available in 24 minutes and 2 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (79)
✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review
Scope: 84 file(s), +6665 / −451
Verdict: Minor issues
This PR adds a v2 Files workbench (editor groups, specialized viewers, streaming reads, paginated tree loading) and hardens Cursor chat/CLI model handling (variant folding, deferred runtime reset on model switch, direct cursor-agent launch). The backend path-safety and range-read caps look sound; the main gaps are incomplete tree pagination UX and a few renderer memory/perf edges on very large assets.
🐛 Functionality
[Medium] Tree expansion stops at 10k entries with no way to load the rest
File: apps/desktop/src/renderer/components/files/v2/FilesWorkbench.tsx:L217-L223
Issue: When expanding a directory, the loader pages through listTreeChildren until it hits MAX_AUTO_LOADED_CHILDREN (10,000), then stores loadMoreOffset on the node and sets childrenTruncated. Nothing in the renderer reads those fields — FilesExplorer has no "Load more" row or handler — so entries beyond the cap are unreachable.
Repro: Open a workspace with a directory that has >10,000 visible children (e.g. a large node_modules with ignored files off), expand it in v2 (or v1 Files, which shares the same explorer).
Fix: In FilesExplorer, render a trailing row when node.childrenTruncated / node.loadMoreOffset != null, and on click call listTreeChildren with offset: loadMoreOffset, append results, and update loadMoreOffset until null.
⚡ Performance
[Medium] listTreeChildren re-reads the full directory on every page
File: apps/desktop/src/main/services/files/fileService.ts:L636-L652 and L790-L799
Issue: Each paginated request calls collectVisibleChildEntries, which readdirs the entire parent, filters/sorts all entries, then slices [offset, offset+limit). Expanding a 20k-entry folder issues ~5 full scans; a 100k-entry folder is worse.
Impact: Multi-second main-process stalls when expanding very large directories; scales with directory size × page count, not page size alone.
Fix: Cache the filtered/sorted visible entry list per (workspaceId, parentPath, includeIgnored) with invalidation on watcher events, or stream from a single readdir and yield pages without rescanning.
[Medium] PDF viewer loads the entire file into renderer memory
File: apps/desktop/src/renderer/components/files/v2/streamBytes.ts:L8-L37 and apps/desktop/src/renderer/components/files/v2/viewers/PdfViewer.tsx:L27-L32
Issue: PdfViewer calls streamFileBytes, which concatenates every readFileRange chunk into one Uint8Array with no byte cap. Large PDFs bypass the inline readFile binary limit because the viewer streams via range reads.
Impact: Opening a multi-hundred-MB PDF can allocate the full file in the renderer and freeze or crash the window.
Fix: Enforce a max PDF size (similar to LargeTextViewer's MAX_STREAM_BYTES), or use pdf.js range/stream transport without holding the whole file in memory.
🎨 UI/UX
[Low] Failed directory expansion shows an empty folder with no error
File: apps/desktop/src/renderer/components/files/v2/FilesWorkbench.tsx:L224-L225
Issue: loadDirectory catches all errors and ignores them (/* ignore — directory may have been removed */), so permission failures, missing workspace, or IPC timeouts look like an empty directory.
Fix: Surface the error via setError (or a per-node error flag) when the catch is not a benign race (e.g. workspace still matches and path still exists).
Notes
Positive patterns worth keeping: ensureSafePath on all new file APIs; UTF-8-safe readFileRange trimming; Monaco model registry reuse; Cursor model-switch deferral via pendingModelSwitchReset; sanitized markdown preview via existing PlanMarkdown.
Could not run the full desktop test suite in this environment; findings are from static review of the diff and head commit cf95162.
Sent by Cursor Automation: BUGBOT in Versic
|
@copilot review but do not make fixes |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
1 participant
Summary
Validation
Greptile Summary
This PR adds the Files v2 workbench (Monaco-backed editor groups, PDF/CSV/image/markdown viewers, paginated tree loading, git blame, range reads) and hardens Cursor chat/model runtime behaviour (deferred model-switch teardown during live turns, live SDK-policy sync, direct
cursor-agentlaunch replacing thecreate-chat/--resumeshell dance).CodeViewer, virtualizedLargeTextViewer/CsvViewer, aPdfViewerusingpdfjs-dist+streamFileBytes, and new back-end APIs (readFileRange,listTreeChildren,refreshGitDecorations,blame).pendingModelSwitchResetand processed on idle;syncLiveCursorSdkPolicypropagates mode/permission changes to a live SDK instance; CLI launches are simplified to directcursor-agentinvocation.Confidence Score: 4/5
Safe to merge with one fix recommended: the CsvViewer streaming loop should be capped before merging to avoid OOM crashes when opening large CSVs.
The Cursor runtime hardening and file-service API additions are solid. The only active defect introduced is in the new CsvViewer: it accumulates all streamed chunks via O(n²) string concatenation with no byte cap, while the analogous LargeTextViewer was deliberately given a 25 MB ceiling and an array-based accumulator. A large CSV will exhaust renderer memory and crash the tab before the fix is in place.
apps/desktop/src/renderer/components/files/v2/viewers/CsvViewer.tsx — the streaming accumulation loop needs the same MAX_STREAM_BYTES cap and parts-array pattern used by LargeTextViewer.
Important Files Changed
Sequence Diagram
sequenceDiagram participant R as Renderer participant P as Preload (IPC) participant FS as FileService (Main) participant Git as Git Process Note over R,FS: Large file / PDF open R->>P: "files.readFile({path})" P->>FS: readFile FS-->>P: "{content: firstChunk, isPartial:true, nextOffset:N}" P-->>R: FileContent (partial) loop Streaming chunks (readFileRange) R->>P: "files.readFileRange({offset:N, length:512KB})" P->>FS: readFileRange FS-->>P: "{encoding:base64|utf-8, content, nextOffset}" P-->>R: FilesReadFileRangeResult end Note over R,FS: Paginated directory load R->>P: "files.listTreeChildren({parentPath, offset})" P->>FS: listTreeChildren FS->>FS: collectPagedVisibleChildEntries (2s cache) FS-->>P: "{children[], nextOffset}" P-->>R: FilesListTreeChildrenResult Note over R,FS: Git decorations (separate from tree load) R->>P: "files.refreshGitDecorations({workspaceId})" P->>FS: refreshGitDecorations FS->>Git: git status Git-->>FS: changed files FS-->>P: "FilesGitStatusEvent {files[], directories[]}" P-->>R: GitStatusEventPrompt To Fix All With AI
Reviews (2): Last reviewed commit: "fix: address files streaming review" | Re-trigger Greptile