Repository navigation
Conversation
7d5d3a2 to
b959132
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:
📝 WalkthroughWalkthroughAdds project-group depth and cycle validation, persistence clamping, and atomic reparent updates across shared, main-process, RPC, preload, and repository state layers. Extends the sidebar with subgroup creation and host-compatibility handling. Reworks project-group header dragging to support nesting, cross-bucket reordering, un-nesting, subtree protection, and reparent commits, with expanded tests and localized UI strings. 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6ffe422e-e76d-496a-880f-d1ff6b30f3f6
📥 Commits
Reviewing files that changed from the base of the PR and between 1ba71d2 and 7d5d3a2014fa36f2956e0d103686ca5f04a7a475.
📒 Files selected for processing (27)
src/main/ipc/repos.tssrc/main/persistence.test.tssrc/main/persistence.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/methods/repo.test.tssrc/main/runtime/rpc/methods/repo.tssrc/preload/api-types.tssrc/renderer/src/components/sidebar/WorktreeList.tsxsrc/renderer/src/components/sidebar/project-group-header-drag-commit.test.tssrc/renderer/src/components/sidebar/project-group-header-drag-commit.tssrc/renderer/src/components/sidebar/project-group-header-drag-contract.tssrc/renderer/src/components/sidebar/project-group-header-drag-start.test.tssrc/renderer/src/components/sidebar/project-group-header-drag-start.tssrc/renderer/src/components/sidebar/project-group-header-drag.tssrc/renderer/src/components/sidebar/project-group-header-drop-target.test.tssrc/renderer/src/components/sidebar/project-group-header-drop-target.tssrc/renderer/src/components/sidebar/project-group-header-drop.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/store/slices/repos-project-groups-nesting.test.tssrc/renderer/src/store/slices/repos-project-groups.test.tssrc/renderer/src/store/slices/repos.tssrc/shared/project-groups.test.tssrc/shared/project-groups.ts
👮 Files not reviewed due to content moderation or server errors (7)
- src/renderer/src/components/sidebar/project-group-header-drag-start.test.ts
- src/renderer/src/components/sidebar/project-group-header-drag-start.ts
- src/renderer/src/i18n/locales/en.json
- src/renderer/src/i18n/locales/es.json
- src/renderer/src/i18n/locales/ja.json
- src/renderer/src/i18n/locales/ko.json
- src/renderer/src/i18n/locales/zh.json
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
…ring render Mutating the ref in the render body can leak an abandoned concurrent render's collapse state into the async reparent callback. Sync it in useLayoutEffect so only committed state is observed.
…parent update Hosts that predate nested groups strip parentGroupId but may still persist the rest of a compound update (e.g. tabOrder), leaving local order drifted from the host. Refetch groups on the mismatch before reporting failure.
17c5e3e to
445ae16
Compare
…drags Drop targeting ran every frame during a drag and rebuilt the full group-id Set, child map, subtree, and depth walk from a fresh copy of the whole catalog on each call — O(catalog) work several times per frame, which janks at the 130k-group scale the repo tests. Snapshot a reparent validator on the drag session instead: it indexes parents once at drag start and memoizes ancestor depths, so per-frame checks walk a single parent chain. The one-shot authoritative check delegates to the same validator (moved to project-group-reparent.ts to respect the max-lines budget), and the commit path still revalidates against live state.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f825b704-2379-46de-afbf-0bad6fab45b2
📒 Files selected for processing (12)
src/main/persistence.tssrc/renderer/src/components/sidebar/project-group-header-drag-commit.test.tssrc/renderer/src/components/sidebar/project-group-header-drag-contract.tssrc/renderer/src/components/sidebar/project-group-header-drag-start.tssrc/renderer/src/components/sidebar/project-group-header-drag.tssrc/renderer/src/components/sidebar/project-group-header-drop-target.test.tssrc/renderer/src/components/sidebar/project-group-header-drop-target.tssrc/renderer/src/store/slices/repos.tssrc/shared/project-group-reparent.test.tssrc/shared/project-group-reparent.tssrc/shared/project-groups.test.tssrc/shared/project-groups.ts
💤 Files with no reviewable changes (2)
- src/shared/project-groups.test.ts
- src/shared/project-groups.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/renderer/src/components/sidebar/project-group-header-drop-target.test.ts
- src/renderer/src/components/sidebar/project-group-header-drag-contract.ts
- src/renderer/src/store/slices/repos.ts
- src/main/persistence.ts
- src/renderer/src/components/sidebar/project-group-header-drop-target.ts
- src/renderer/src/components/sidebar/project-group-header-drag-commit.test.ts
- src/renderer/src/components/sidebar/project-group-header-drag-start.ts
- src/renderer/src/components/sidebar/project-group-header-drag.ts
…romotion Session creation on pointer-down scanned the catalog several times and measured header rects even for plain header clicks. Build the reparent index (parent map, subtree, height in one iterable pass — no array copy) and measure rects only once the gesture crosses the drag threshold, so clicks pay nothing. Also stop memoizing ancestor depths for cyclic chains: cached cycle depths depend on which target was queried first, so cyclic input now returns the chain length uncached, matching getProjectGroupDepth.
…r-down Session creation summed every bucket on each pointer-down to decide if a drag is meaningful, leaving an O(buckets) scan on the plain-click path. WorktreeList already memoizes the total header count, so thread it through the hook and drop the per-click scan.
… boundary Locks in the promotion contract: below the threshold no catalog index or rect measurement happens, crossing it builds the index exactly once, and further pointer moves reuse it.
Summary
Fixes #8785
Project groups in the sidebar can now be nested up to 3 levels, by drag or from the context menu:
The data model already supported nesting (
parentGroupId— folder-scan import produces nested groups today), so this is mostly unlocking it:updateProjectGroupacceptsparentGroupIdacross store/IPC/RPC/persistence, with self/cycle/missing-parent/depth validation shared between the renderer preflight and an atomic main-process check. Groups stay sidebar-only organization; execution semantics are untouched.Screenshots
nested-groups-demo-final.mov
Testing
pnpm lintpnpm typecheckpnpm testpnpm buildpnpm run test:e2e -- tests/e2e/project-group-manual-sort.spec.ts— 3/3 passed on macOS (group-header ordering, the area this PR touches)src/shared/project-groups.test.ts), drop-zone geometry incl. descendant/depth/current-parent suppression (project-group-header-drop-target.test.ts), nest/cross-bucket/un-nest commits (project-group-header-drag-commit.test.ts), atomic rejection in persistence (persistence.test.ts), and parentGroupId threading + legacy-host handling in the store (repos-project-groups-nesting.test.ts).Also verified by hand in the dev app: nest, un-nest with subtree, cycle drop is a dead zone, "New subgroup" hidden at the cap, structure survives restart.
AI Review Report
Ran a full review pass over the diff plus a separate convention audit. It checked drag session staleness (groups deleted mid-drag, virtualized rect measurement), the cross-bucket commit math, atomicity of the persistence update, and macOS/Linux/Windows compatibility — the drag is pointer-event based with no
metaKey, path, or shell assumptions, and no Electron platform-specific surface is touched. It flagged two things I fixed: a stale collapse-state closure in the reparent callback (now read through a ref at promise resolution) and az.string()that should have been the RPC schema'sOptionalStringidiom. Remaining notes were eventual-consistency observations that reconcile on the next fetch.Security Audit
No new IPC channels or commands. The change adds one optional nullable string field (
parentGroupId) to the existing validatedprojectGroups:updateschema (zod on both IPC and RPC), and the main process re-validates it (self/cycle/depth/missing parent) before applying, rejecting the whole update on failure. No path handling, no shell execution, no auth/secrets involvement.Notes
parentGroupIdkey from the update schema; the renderer detects that from the returned group and shows a "Could not move group" toast instead of desyncing the sidebar. Same handling when "New subgroup" runs against an old host (group is created at root, with a toast).normalizeProjectGroupsnow also breaks persisted parent cycles and clamps chains past 10 levels as a corruption guard (memoized, O(n) — the loader handles six-figure group catalogs in tests).ELI5
Project groups in the sidebar can nest up to three levels via drag-and-drop or the menu. Drop on a group’s center to nest, edges to reorder, or drag out to un-nest.