Repository navigation
Conversation
f3a02d6 to
c708f8f
Compare
|
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:
📝 WalkthroughWalkthroughFolder workspaces are integrated into sidebar visibility filtering, host routing, grouped and pinned row construction, status-based sorting, and reveal-target calculation. Rendered row keys now identify folder workspace copies across virtual rows, DOM options, activation, rename, and active-surface selection. Duplicate folder rows are deduplicated in sidebar ordering. Folder pin metadata updates are awaited before revealing the corresponding sidebar row. Tests cover visibility, host routing, grouping, identity, reveal keys, and deferred pinning. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/src/components/sidebar/WorktreeList.tsx (1)
5661-5691: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winStabilize
visibleFolderWorkspacesForRowsidentity acrossagentStatusEpochbumps.This memo now depends on
agentStatusEpoch, so when sleeping workspaces are hidden it recomputes to a brand-new array on every epoch bump even when the visible set is unchanged. It feeds bothfolderWorkspaceSortComparatorand therowsuseMemodependency list, so a fresh identity churns the entirerows/sectionRows/renderedWorktreeschain per epoch — the exact churn the siblingvisibleWorktreesavoids viauseReusedArrayIdentity(see the comment at Line 5557).Consider wrapping the result the same way for consistency.
♻️ Proposed change
- const visibleFolderWorkspacesForRows = useMemo(() => { + const recomputedVisibleFolderWorkspacesForRows = useMemo(() => { // Why agentStatusEpoch: sleeping-state membership must update immediately // without subscribing to the full agent-status map (same as worktrees). void agentStatusEpoch return computeVisibleFolderWorkspaces(folderWorkspaces, { @@ }, [ agentStatusEpoch, browserTabsByWorktree, defaultHostId, folderWorkspaces, projectGroups, ptyIdsByTabId, showSleepingWorkspaces, tabsByWorktree, visibleHostIdSet ]) + const visibleFolderWorkspacesForRows = useReusedArrayIdentity( + recomputedVisibleFolderWorkspacesForRows + )
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fab14ea5-5a65-4fb0-bff9-4d1908e30110
📥 Commits
Reviewing files that changed from the base of the PR and between 42bff5e and f3a02d6f0153867917a1fa34164aa282bc3d8963.
📒 Files selected for processing (15)
src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.tssrc/renderer/src/components/sidebar/WorktreeList.tsxsrc/renderer/src/components/sidebar/host-section-folder-workspace-rows.test.tssrc/renderer/src/components/sidebar/host-section-rows.test.tssrc/renderer/src/components/sidebar/host-section-rows.tssrc/renderer/src/components/sidebar/visible-worktrees.test.tssrc/renderer/src/components/sidebar/visible-worktrees.tssrc/renderer/src/components/sidebar/worktree-list-folder-reveal.test.tssrc/renderer/src/components/sidebar/worktree-list-folder-reveal.tssrc/renderer/src/components/sidebar/worktree-list-groups.test.tssrc/renderer/src/components/sidebar/worktree-list-groups.tssrc/renderer/src/components/sidebar/worktree-sidebar-row-preference.test.tssrc/renderer/src/components/sidebar/worktree-sidebar-row-preference.tssrc/renderer/src/store/slices/worktrees.test.tssrc/renderer/src/store/slices/worktrees.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/src/components/sidebar/WorktreeList.tsx (1)
5706-5708: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant array allocation.
recomputedVisibleFolderWorkspacesForRowsis already a newly evaluated array. Spreading it again creates an unnecessary array allocation before identity reuse.♻️ Proposed refactor
- const visibleFolderWorkspacesForRows = useReusedArrayIdentity([ - ...recomputedVisibleFolderWorkspacesForRows - ]) + const visibleFolderWorkspacesForRows = useReusedArrayIdentity( + recomputedVisibleFolderWorkspacesForRows + )
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 36ef0e3c-4385-4ac0-b09c-4c4abd096c3b
📒 Files selected for processing (4)
src/renderer/src/components/sidebar/WorktreeList.card-memo-stability.test.tsxsrc/renderer/src/components/sidebar/WorktreeList.tsxsrc/renderer/src/store/slices/worktrees.test.tssrc/renderer/src/store/slices/worktrees.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/src/store/slices/worktrees.test.ts
- src/renderer/src/store/slices/worktrees.ts
Conflict resolution notes: - visible-worktrees.test.ts: main moved the sidebar-filter tests to sidebar-filter-state.test.ts; keep that split and retain only this branch's folder-workspace visibility tests. - WorktreeList: use main's extracted filterProjectGroupsForVisibleHosts, and keep computeVisibleFolderWorkspaces (a superset of main's filterFolderWorkspacesForVisibleHosts: archived + sleeping filters). - navigateWorktree: adopt main's rendered-rows cycling (getCyclableWorktreeIds / resolveCycledWorktreeId) instead of the rebuilt all-expanded layout, and extend it to folder-workspace rows so arrow keys and Cmd+1-9 stay on the same order. Folder keys dispatch through activateAndRevealWorkspace for the path-status gate. - visible-worktrees.ts: split computeVisibleFolderWorkspaces into visible-folder-workspaces.ts; both sides grew the file past max-lines.
|
Thanks for this, @atiakr, and for the before/after screenshots! main already fixed it in #15404 (1272406), which closed #15362: folder-workspace membership is now decided above the Group by switch, so folder workspaces render under Status, PR status and None too (including folder-only flat lists and collapsed lane headers). It shipped in v1.4.187. Closing as superseded — please reopen if you see a grouping mode where they still go missing. |
Summary
When the left sidebar is grouped by Status or PR, folder workspaces disappear from the list. They only showed up when grouping by repo.
Folder workspaces now render in the right group under every grouping mode.
Header counts, sort order, host/sleeping filters, and Pinned all behave the same as they do for git worktrees.
Screenshots
before
after
Testing
pnpm lint— oxlint clean for the changed dirs (sidebar,store/slices/worktrees.ts). Fullpnpm lintfails only on pre-existing switch-exhaustiveness errors in unrelated files (skill-freshness-group.tsx:101,daemon-server.ts:672) that this PR does not touch.pnpm typecheckpnpm test— the 6 changed sidebar test files pass (221 tests). Full suite: 33,206 passed; the 50 failures are all pre-existing and unrelated to this change (terminal IME composition tests, likely Node-24-vs-26 environment), in subsystems this PR does not touch.pnpm build—build:desktop(typecheck + electron-vite + web) succeeds. Fullpnpm buildfails only at the native macOSbuild:computer-use(Swiftlipo) step, an unrelated native toolchain issue outside this renderer change.Additional validation (all passing):
worktree-list-groups.test.ts— folder rows in None/Status/PR lanes, Pinned emission + counts, duplicate-in-groups, mixed-host counts, sort interleavingvisible-worktrees.test.ts—computeVisibleFolderWorkspaces: archived always dropped, sleeping filter, live-agent keep, host-scope filterworktree-list-folder-reveal.test.ts— reveal keys for Pinned/single-location, natural-lane under duplicate-in-groups, flat-lane in non-repo groupings, project headers in repo groupingworktree-sidebar-row-preference.test.ts—isWorkspaceRowIdentityRendered, natural-copy dedupe/orderhost-section-rows.test.ts— folder rows counted in host sections without double-counting pinned duplicatesAI Review Report
The AI code review checked the following risks:
Cross-platform: This change is pure renderer (React) sidebar logic. It does not touch file paths, keyboard shortcuts, shell execution, or Electron platform branches, so behavior is identical on macOS, Linux, and Windows.
Security Audit
No changes to input handling, command execution, path handling, auth, secrets, dependencies, or IPC. This only changes renderer-side list composition logic and adds no new dependencies.
Notes
duplicate-in-groupsa pinned folder renders in both Pinned and its natural lane; undersingle-locationit renders once.pnpm lint/pnpm test/pnpm buildstill to be confirmed in CI.ELI5
Folder workspaces vanished from the sidebar when grouped by Status or PR; they only showed under repo grouping. They now appear in the right group for every grouping mode, with the same filters and counts as git worktrees.