fix(home): surface when the workspace file scan is truncated - #10550
Open
NoiceHax wants to merge 2 commits into
Open
fix(home): surface when the workspace file scan is truncated#10550NoiceHax wants to merge 2 commits into
NoiceHax wants to merge 2 commits into
Conversation
DirectoryScanner drops a folder whenever its recursion comes back empty, which conflates "this folder holds no notebooks" with "we stopped looking". Notebooks nested deeper than MAX_DEPTH, past the file-count cap, or inside a directory that could not be read therefore disappeared from the home page workspace tree along with their parent folders, with nothing in the response or the UI to indicate the listing was incomplete. The scanner now records that a limit cut the walk short, workspaces expose it as is_truncated, and the workspace_files endpoint folds it into has_more, so the home and gallery pages warn that files and folders were skipped and point at starting marimo from a subfolder. Closes marimo-team#10064
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
Contributor
Contributor
|
@mscolnick I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Architecture diagram
sequenceDiagram
participant UI as Home/Gallery UI
participant API as workspace_files Endpoint
participant WS as Workspace (DirectoryWorkspace)
participant SC as DirectoryScanner
participant FS as Filesystem
Note over UI,FS: Workspace File Listing Flow (with truncation detection)
UI->>API: GET /api/workspace_files
API->>WS: Access workspace.files property
WS->>SC: scan() for lazy file tree
loop Directory traversal
SC->>FS: Recurse through directories
FS-->>SC: Directory entries
alt Depth limit exceeded
SC->>SC: Set truncated = true
SC-->>WS: Return None (skip folder)
else File count cap reached
SC->>SC: Set truncated = true
SC-->>WS: Stop traversal
else Time limit reached
SC->>SC: Set truncated = true
SC-->>WS: Return partial results
else OS error reading directory
SC->>SC: Set truncated = true
SC-->>WS: Skip unreadable folder
else Normal file found
SC-->>WS: Include file in results
end
end
SC-->>WS: Return file tree
WS-->>API: files + is_truncated flag
alt is_truncated = true OR max files reached
API->>API: Set has_more = true
else Full scan completed
API->>API: has_more = false
end
API-->>UI: WorkspaceFilesResponse (has_more, file_count, files)
alt has_more = true
UI->>UI: Show warning banner: "Some files and folders were skipped..."
else has_more = false
UI->>UI: Display complete workspace tree
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
An entry whose is_dir() or stat() raised OSError was skipped without setting the truncated flag, so a folder we could not read still looked like an empty folder and the pages showed no warning. That is the same silent drop the rest of this change is about, so set the flag there too. Also move the warning copy into a shared TruncatedWorkspaceBanner so the home and gallery pages cannot drift apart.
Author
|
Fixed both points from the cubic review. The per-entry OSError handler now sets truncated too, since an entry we cannot stat may have been a notebook or a folder of them, with a test for it. The banner copy moved into a shared TruncatedWorkspaceBanner used by both the home and gallery pages. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The home page workspace tree drops a folder whenever the scan of it comes back empty, so "this folder has no notebooks" and "we stopped looking" end up looking the same. Notebooks deeper than MAX_DEPTH, past the file cap, or inside a folder that could not be read vanish along with their parent folders, and nothing in the response says the list is short.
This does not change which files get listed. The scanner now sets a flag when a limit or an unreadable entry cut the walk short, DirectoryWorkspace reports it as is_truncated, and workspace_files folds it into has_more. Home and gallery share one banner saying files and folders were skipped, and suggesting you start marimo from a subfolder.
Tests are in tests/_server/test_directory_scanner.py and tests/_server/test_workspace.py. They cover the depth cap, the file cap, an entry that fails to stat, and the flag being reset between scans.
Closes #10064