Repository navigation
Conversation
…ycle-safe re-parenting Reintroduces the grouping tree extracted from #6274 (exact revert of the extraction commit, preserving all review-round fixes): sections gain parentSectionId + nullable projectId, depth-capped folders-first nesting in project and Sessions scopes, context-menu re-parenting, recursive hotkey reveal and bulk-select coverage.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 24 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/utils/getFlattenedV2WorkspaceIds/getFlattenedV2WorkspaceIds.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/utils/getFlattenedV2WorkspaceIds/getFlattenedV2WorkspaceIds.ts:41">
P1: Workspace removal navigation omits workspaces in dangling, cyclic, or over-depth groups even though sidebar renders those groups at root. Build this ordering from the same repaired tree (or apply its root-splicing rules) so navigation matches visible siblings.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx:66">
P2: “Move to group” includes the group’s current parent; selecting it silently reorders the group within that parent. Exclude `self.parentSectionId` from targets so this menu only offers actual re-parenting.</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx:76">
P2: Existing group menus retain stale targets after sidebar section updates because this memo never depends on reactive section data. Derive targets on each header render or subscribe this component to the sections query before memoizing.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const sessionRootSections = allSections | ||
| .filter( | ||
| (section) => | ||
| section.projectId === null && section.parentSectionId === null, |
There was a problem hiding this comment.
P1: Workspace removal navigation omits workspaces in dangling, cyclic, or over-depth groups even though sidebar renders those groups at root. Build this ordering from the same repaired tree (or apply its root-splicing rules) so navigation matches visible siblings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/utils/getFlattenedV2WorkspaceIds/getFlattenedV2WorkspaceIds.ts, line 41:
<comment>Workspace removal navigation omits workspaces in dangling, cyclic, or over-depth groups even though sidebar renders those groups at root. Build this ordering from the same repaired tree (or apply its root-splicing rules) so navigation matches visible siblings.</comment>
<file context>
@@ -23,22 +23,38 @@ export function getFlattenedV2WorkspaceIds(
+ const sessionRootSections = allSections
+ .filter(
+ (section) =>
+ section.projectId === null && section.parentSectionId === null,
+ )
+ .sort((left, right) => left.tabOrder - right.tabOrder);
</file context>
| moveTargets: sections | ||
| .filter( | ||
| (row) => | ||
| row.sectionId !== sectionId && |
There was a problem hiding this comment.
P2: “Move to group” includes the group’s current parent; selecting it silently reorders the group within that parent. Exclude self.parentSectionId from targets so this menu only offers actual re-parenting.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx, line 66:
<comment>“Move to group” includes the group’s current parent; selecting it silently reorders the group within that parent. Exclude `self.parentSectionId` from targets so this menu only offers actual re-parenting.</comment>
<file context>
@@ -29,12 +41,40 @@ interface SectionActionsMenuItemsProps
+ moveTargets: sections
+ .filter(
+ (row) =>
+ row.sectionId !== sectionId &&
+ canMoveSectionToParent(collections, sectionId, row.sectionId),
+ )
</file context>
| row.sectionId !== sectionId && | |
| row.sectionId !== sectionId && | |
| row.sectionId !== self.parentSectionId && |
| color: row.color, | ||
| })), | ||
| }; | ||
| }, [collections, sectionId]); |
There was a problem hiding this comment.
P2: Existing group menus retain stale targets after sidebar section updates because this memo never depends on reactive section data. Derive targets on each header render or subscribe this component to the sections query before memoizing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx, line 76:
<comment>Existing group menus retain stale targets after sidebar section updates because this memo never depends on reactive section data. Derive targets on each header render or subscribe this component to the sections query before memoizing.</comment>
<file context>
@@ -29,12 +41,40 @@ interface SectionActionsMenuItemsProps
+ color: row.color,
+ })),
+ };
+ }, [collections, sectionId]);
+
const selectedValue = color ?? PROJECT_COLOR_DEFAULT;
</file context>
|
This PR has had no activity for 53 days and will close in 7 days unless there is new activity. If this contribution is still important or relevant, please leave a comment to keep it open. If it closes, you are welcome to reopen it or ask a maintainer to reopen it. Thank you for contributing! |
|
This PR was automatically closed due to inactivity to help keep the review queue manageable. This does not mean the contribution was rejected. If this contribution is still important or relevant, please reopen it or leave a comment asking a maintainer to reopen it. Thank you for contributing! |
Summary
Stacked on #6274 (sessions) — extracted from it so the two features get independent review and revert levers under squash-merge. The single commit here is a byte-exact reconstruction of the grouping-tree code as it stood after all review rounds on #6274 (produced via
git revertof the extraction commit — no re-implementation).Generalizes flat sidebar sections into a nestable grouping tree:
v2SidebarSectionsschema extended in place (parentSectionId, nullableprojectIdfor the Sessions scope) — widening-with-default, zero migration; existing rows parse untouchedDashboardSidebarNestedSection, recursive); read-side cycle/dangling/depth violations splice to the scope root (never dropped); write-sidecanMoveSectionToParent(shared by the mutation and the menus) refuses self/descendant/cross-scope/over-depth targetsTesting
Carried over from the #6274 review cycle where this code was written and hardened: tree-safety unit tests (cycle/dangling/depth splices, session-scope nesting), plus CDP end-to-end verification of nested create/move/reveal/delete-reparent and reload persistence in both scopes. After #6274 merges this branch rebases onto main and CI re-runs the full suite.
Merge order
Merge #6274 first, then rebase this onto main (squash-merge invalidates the stacked base) and merge.
Summary by cubic
Adds nestable workspace groups to the sidebar for both Projects and Sessions. Improves organization and navigation with safe re-parenting, folders-first trees, and shortcuts that reveal nested items.
New Features
Migration
v2SidebarSectionswidened in place:parentSectionIdadded;projectIdis now nullable. No migration required; existing rows continue to parse.Written for commit 9b82e12. Summary will update on new commits.