Skip to content

feat(sidebar): add manual project-group nesting controls (#8785) - #8945

Closed
rodboev wants to merge 3 commits into
stablyai:mainfrom
rodboev:pr/manual-project-group-nesting
Closed

rodboev wants to merge 3 commits into
stablyai:mainfrom
rodboev:pr/manual-project-group-nesting

Conversation

@rodboev

@rodboev rodboev commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add New subgroup, Move to group, and Move to root actions to project-group header menus so manual group organization can match Orca's existing nested renderer.
  • Carry parentGroupId through the existing create and update transport, with persistence rejecting self, descendant, and missing-parent assignments.
  • Keep project-group drag and drop behavior unchanged so this slice stays disjoint from PR #6911.

Refs #8785.

Screenshots

No screenshot. Local headed Electron proof stayed deferred in this session after visible Orca windows and firewall prompts disrupted the desktop.

Testing

  • pnpm lint
  • pnpm typecheck
  • pnpm test
  • pnpm build
  • Added or updated high-quality tests that would catch regressions

Focused validation to run:

  • pnpm exec vitest run --config config/vitest.config.ts src/main/persistence.test.ts src/main/ipc/repos-remote.test.ts src/main/runtime/rpc/methods/repo.test.ts src/renderer/src/store/slices/repos-project-groups.test.ts src/renderer/src/components/sidebar/worktree-list-groups.test.ts
  • pnpm run typecheck:node
  • pnpm run typecheck:web
  • pnpm run verify:localization-catalog && pnpm run verify:localization-coverage
  • pnpm exec oxlint --config config/oxlint-react-doctor.json src/main/ipc/repos.ts src/main/persistence.ts src/main/runtime/orca-runtime.ts src/main/runtime/rpc/methods/repo.ts src/preload/api-types.ts src/renderer/src/components/sidebar/WorktreeList.tsx src/renderer/src/store/slices/repos.ts tests/e2e/project-group-nesting.spec.ts
  • pnpm run test:e2e:headful -- tests/e2e/project-group-nesting.spec.ts --workers=1, intentionally not rerun because this session is avoiding visible Electron work

AI Review Report

  • Cycle rejection, atomic invalid updates, local/runtime payload parity, candidate filtering, nested rendering preservation, localization, and drag-surface containment all received final-diff review coverage.
  • The create path now preserves invalid empty-string parents through runtime transport, so persistence rejects them instead of silently coercing them into root creation.
  • Menu-driven subgroup and reparent actions stay disjoint from the competing drag surface in PR #6911.
  • Headed Electron menu visibility and screenshot proof remain CI-owned until tests/e2e/project-group-nesting.spec.ts is rerun in a non-disruptive environment.

Security Audit

IPC and runtime callers now preserve the full three-state parentGroupId contract into persistence, and persistence remains the final validation owner for self, descendant, missing-parent, and invalid-create rejection. Failed create and update attempts return null without partial mutation or notification side effects.

Notes

This slice intentionally references the broader issue instead of closing it. Drag-to-nest and drag-to-root behavior remain outside scope. Hierarchy depth remains arbitrary and follows Orca's existing recursive renderer.

Local headed Electron proof was deferred in this session after disruptive Orca windows and firewall prompts during UI-owner verification. The branch carries the narrow Playwright coverage, but screenshot-backed rendered-layout proof remains pending until it can be rerun in a non-disruptive environment.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Project groups now support nested parent relationships across persistence, IPC, RPC, preload, runtime, and renderer state APIs. Creation handles invalid parents with nullable results, while updates validate parent targets and prevent cycles. The sidebar adds subgroup creation, movement between valid groups, and movement to root, with localized labels. Persistence, renderer, RPC, and end-to-end tests cover these flows and their visual nesting behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The template sections are present, but the AI Review Report does not explicitly confirm cross-platform review for macOS, Linux, and Windows. Add an explicit cross-platform review note covering macOS, Linux, and Windows, including shortcuts, labels, paths, shell behavior, and any Electron-specific differences.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: manual project-group nesting controls.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/renderer/src/components/sidebar/WorktreeList.tsx (1)

4397-4420: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Extract getProjectGroupSubtreeIds out of the .filter callback.

getProjectGroupSubtreeIds iterates over all project groups to build a subtree map. Calling it inside .filter recomputes this map for every group during every render of the row, resulting in O(V * N^2) allocations per frame (where V is the number of visible headers and N is the total number of groups).

Wrap the logic in an IIFE to evaluate the subtree IDs once per row render, reducing the complexity to O(V * N).

⚡ Proposed performance refactor
-                              <DropdownMenuSubContent>
-                                {projectGroups
-                                  .filter(
-                                    (group) =>
-                                      !getProjectGroupSubtreeIds(
-                                        projectGroups,
-                                        row.projectGroup!.id as string
-                                      ).has(group.id)
-                                  )
-                                  .map((group) => (
-                                    <DropdownMenuItem
-                                      key={group.id}
-                                      onSelect={() =>
-                                        handleMoveProjectGroup(
-                                          row.projectGroup!.id as string,
-                                          group.id
-                                        )
-                                      }
-                                    >
-                                      <span className="max-w-48 truncate">{group.name}</span>
-                                    </DropdownMenuItem>
-                                  ))}
-                              </DropdownMenuSubContent>
+                              <DropdownMenuSubContent>
+                                {(() => {
+                                  const subtreeIds = getProjectGroupSubtreeIds(
+                                    projectGroups,
+                                    row.projectGroup!.id as string
+                                  )
+                                  return projectGroups
+                                    .filter((group) => !subtreeIds.has(group.id))
+                                    .map((group) => (
+                                      <DropdownMenuItem
+                                        key={group.id}
+                                        onSelect={() =>
+                                          handleMoveProjectGroup(
+                                            row.projectGroup!.id as string,
+                                            group.id
+                                          )
+                                        }
+                                      >
+                                        <span className="max-w-48 truncate">{group.name}</span>
+                                      </DropdownMenuItem>
+                                    ))
+                                })()}
+                              </DropdownMenuSubContent>

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 53c95ecf-64ff-4a2b-a3b9-ded2846c88c7

📥 Commits

Reviewing files that changed from the base of the PR and between 69c14fa and c6ca03c.

📒 Files selected for processing (16)
  • src/main/ipc/repos.ts
  • src/main/persistence.test.ts
  • src/main/persistence.ts
  • src/main/runtime/orca-runtime.ts
  • src/main/runtime/rpc/methods/repo.test.ts
  • src/main/runtime/rpc/methods/repo.ts
  • src/preload/api-types.ts
  • src/renderer/src/components/sidebar/WorktreeList.tsx
  • 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
  • src/renderer/src/store/slices/repos-project-groups.test.ts
  • src/renderer/src/store/slices/repos.ts
  • tests/e2e/project-group-nesting.spec.ts
👮 Files not reviewed due to content moderation or server errors (8)
  • src/main/runtime/rpc/methods/repo.ts
  • src/preload/api-types.ts
  • src/main/runtime/orca-runtime.ts
  • src/renderer/src/store/slices/repos.ts
  • src/main/runtime/rpc/methods/repo.test.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/ja.json

@rodboev

rodboev commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of #8939 — I missed that you'd already called this on #8785 and had a PR up. Yours covers the drag half too, which mine deliberately skipped.

One thing that might be useful: I'd written an e2e spec at tests/e2e/project-group-nesting.spec.ts covering the menu-driven nesting flow. Happy to open it as a follow-up once yours lands, or you're welcome to lift it from this branch.

@rodboev rodboev closed this Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant