Refactor workspace sidebar project child ordering#2161
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR implements mixed top-level ordering of project children (ungrouped workspaces and sections interleaved by tabOrder) across backend, utilities, queries, React hooks, cache mutation logic, and sidebar UI/DnD, and adds planning docs and tests to support the migration. Changes
Sequence DiagramsequenceDiagram
participant UI as UI Component<br/>(WorkspaceSection / Sidebar)
participant Hook as useReorderProjectChildren<br/>Hook
participant TRPC as TRPC Server<br/>(reorderProjectChildren)
participant DB as Database
participant Cache as React Query<br/>Cache
UI->>Hook: mutate({ projectId, fromIndex, toIndex })
Hook->>TRPC: call reorderProjectChildren mutation
TRPC->>DB: fetch project workspaces & sections
TRPC->>TRPC: getProjectChildItems(projectId, workspaces, sections)
TRPC->>TRPC: reorderProjectChildItems(...) -> reordered list
TRPC->>DB: update tabOrder on affected workspaces & sections
TRPC->>Hook: return success
Hook->>Cache: invalidateWorkspaceQueries()
Cache->>UI: refetch grouped data (includes topLevelItems)
UI->>UI: re-render with updated mixed top-level order
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
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/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx:315">
P1: This condition hides the ungrouped drop zone whenever sections exist, which prevents dragging workspaces out of a section in projects that currently have no top-level workspaces.</violation>
</file>
<file name="apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts">
<violation number="1" location="apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts:50">
P3: This optimistic grouped-cache transform duplicates `useDeleteWorkspace` logic. Extract it into a shared helper so close/delete stay behaviorally consistent as grouped sidebar data evolves.
(Based on your team's feedback about avoiding duplicated business logic in cache workflows.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/useWorkspaceDnD.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/useWorkspaceDnD.ts:59">
P2: Skip `reorderProjectChildren` when the section index did not change; the current code performs a no-op reorder mutation and unnecessary writes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| sectionId={null} | ||
| sections={sections} | ||
| orderedWorkspaceIds={orderedWorkspaceIds} | ||
| {topLevelChildren.length === 0 && ( |
Contributor
There was a problem hiding this comment.
P1: This condition hides the ungrouped drop zone whenever sections exist, which prevents dragging workspaces out of a section in projects that currently have no top-level workspaces.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx, line 315:
<comment>This condition hides the ungrouped drop zone whenever sections exist, which prevents dragging workspaces out of a section in projects that currently have no top-level workspaces.</comment>
<file context>
@@ -253,44 +312,52 @@ export function ProjectSection({
- sectionId={null}
- sections={sections}
- orderedWorkspaceIds={orderedWorkspaceIds}
+ {topLevelChildren.length === 0 && (
+ <div
+ {...ungroupedDropZone.handlers}
</file context>
Suggested change
| {topLevelChildren.length === 0 && ( | |
| {workspaces.length === 0 && ( |
| ); | ||
| } else { | ||
| reorderWorkspaces.mutate( | ||
| reorderProjectChildren.mutate( |
Contributor
There was a problem hiding this comment.
P2: Skip reorderProjectChildren when the section index did not change; the current code performs a no-op reorder mutation and unnecessary writes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/useWorkspaceDnD.ts, line 59:
<comment>Skip `reorderProjectChildren` when the section index did not change; the current code performs a no-op reorder mutation and unnecessary writes.</comment>
<file context>
@@ -54,7 +56,7 @@ export function useWorkspaceDnD({
);
} else {
- reorderWorkspaces.mutate(
+ reorderProjectChildren.mutate(
{
projectId: item.projectId,
</file context>
| })) | ||
| .filter((group) => group.workspaces.length > 0), | ||
| .map((group) => { | ||
| const isTopLevelWorkspace = group.workspaces.some( |
Contributor
There was a problem hiding this comment.
P3: This optimistic grouped-cache transform duplicates useDeleteWorkspace logic. Extract it into a shared helper so close/delete stay behaviorally consistent as grouped sidebar data evolves.
(Based on your team's feedback about avoiding duplicated business logic in cache workflows.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts, line 50:
<comment>This optimistic grouped-cache transform duplicates `useDeleteWorkspace` logic. Extract it into a shared helper so close/delete stay behaviorally consistent as grouped sidebar data evolves.
(Based on your team's feedback about avoiding duplicated business logic in cache workflows.) </comment>
<file context>
@@ -46,11 +46,34 @@ export function useCloseWorkspace(
- }))
- .filter((group) => group.workspaces.length > 0),
+ .map((group) => {
+ const isTopLevelWorkspace = group.workspaces.some(
+ (w) => w.id === id,
+ );
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts (1)
472-483:⚠️ Potential issue | 🟠 Major
createBranchWorkspacestill bypasses the mixed top-level ordering.The new top-level workspace creation paths append via shared project-child order, but the branch-workspace path at Line 591 still hardcodes
tabOrder: 0and only shiftsworkspaceson Lines 601-619. If a section is already first, both children end up at0, and the nextgetProjectChildItemspass will move the workspace ahead of the section via tie ordering rather than user intent. Please routecreateBranchWorkspacethrough the same helper, or renormalize sections there too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts` around lines 472 - 483, createBranchWorkspace currently hardcodes tabOrder and only shifts workspaces, causing conflicts with the mixed top-level ordering; update createBranchWorkspace to use the same ordering helper as other creation paths by calling getMaxProjectChildTabOrder(input.projectId) and set tabOrder to max+1 when inserting into workspaces (or alternatively invoke the shared normalization routine used by the other paths). Also ensure you adjust/renormalize section items consistently by calling getProjectChildItems and applying the same shift/normalize logic (the insertion in createBranchWorkspace and the shifting block that currently operates on workspaces must mirror the shared behavior used elsewhere) so no two items end up with the same tabOrder.
🧹 Nitpick comments (4)
apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts (1)
49-76: Consider extracting this grouped-cache mutation into a shared helper.This block is effectively duplicated in
apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts:58-90. Pulling it into one helper would reduce drift the next time the sidebar model changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts` around lines 49 - 76, The grouped-cache mutation that filters out a workspace (the map/filter block in useCloseWorkspace) is duplicated in useDeleteWorkspace; extract it into a shared helper (e.g., updateGroupsRemovingWorkspace(groups, id) or normalizeGroupsAfterWorkspaceRemoval) that accepts the groups array and the workspace id and returns the new groups array using the same map/filter logic (preserve topLevelItems removal when isTopLevelWorkspace, remove workspace from group.workspaces and section.workspaces, then filter out empty groups). Replace the inlined code in useCloseWorkspace and useDeleteWorkspace with a call to this helper, export/import it from a common module, and ensure TypeScript types for Group/Section/Workspace and any tests or usage of mutate callbacks are updated accordingly.apps/desktop/src/lib/trpc/routers/workspaces/procedures/sections.ts (1)
98-127: Consider deprecatingreorderSectionsin favor ofreorderProjectChildren.With the new mixed top-level ordering model,
reorderSectionsoperates only on sections and doesn't account for interleaved ungrouped workspaces. This could lead to inconsistent tabOrder values if used alongside the newreorderProjectChildrenmutation.If
reorderSectionsis still needed for section-only reordering within the UI, consider adding a comment clarifying its intended use case versus the newerreorderProjectChildren.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/sections.ts` around lines 98 - 127, The reorderSections mutation (reorderSections) only reorders workspaceSections and can conflict with the newer reorderProjectChildren when project-level mixed ordering exists; mark reorderSections as deprecated and add a clear comment above the procedure explaining it only reorders sections within a project (does not handle interleaved ungrouped workspaces), advise using reorderProjectChildren for mixed top-level ordering, and keep the existing logic (uses localDb, workspaceSections, reorderItems) for any UI flows that specifically require section-only reordering.apps/desktop/src/lib/trpc/routers/workspaces/procedures/status.ts (1)
64-97: Indices alone make this reorder mutation brittle.The server rewrites whichever child currently sits at
fromIndex. If a top-level child is inserted or removed while a drag is in flight, this can persist the wrong reorder. The drag payload already has the child'skind/id, so passing that through and verifyingitems[fromIndex]still matches would make the mutation deterministic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/status.ts` around lines 64 - 97, The reorderProjectChildren mutation is brittle because it blindly reorders whatever is at items[fromIndex]; change the mutation to accept the dragged child's identifier (e.g., kind and id) in the input, then verify that items[fromIndex] matches that identifier before calling reorderProjectChildItems in order to make the operation deterministic; if items[fromIndex] does not match, locate the correct current index by matching kind/id in the items array (or return an error) and use that resolved index as the source to call reorderProjectChildItems with fromIndex and toIndex. Ensure updates touch the input z.object schema, the mutation handler (reorderProjectChildren), and the logic that computes and passes indices to reorderProjectChildItems.apps/desktop/src/lib/trpc/routers/workspaces/utils/project-children-order.test.ts (1)
60-78: Please lock down no-op and bounds handling here too.
reorderProjectChildrennow depends onreorderProjectChildItemsfor all index semantics, but this suite only exercises the happy path. AddingfromIndex === toIndexand out-of-range cases would make regressions much easier to catch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/project-children-order.test.ts` around lines 60 - 78, Add tests to cover no-op and bounds behavior for reorderProjectChildItems (and mirror semantics expected from reorderProjectChildren): add a test where fromIndex === toIndex to assert the output is normalized (tabOrder reset to 0..n-1) but item order unchanged, and add tests for out-of-range indices (negative and indices >= items.length) to assert they are handled deterministically (e.g., treated as clamped to 0 or items.length-1 or throw—match reorderProjectChildren behavior) so regressions are caught; reference the functions reorderProjectChildItems and reorderProjectChildren when adding these cases to project-children-order.test.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/plans/20260306-2140-mixed-project-children-ordering.md`:
- Around line 41-54: The ExecPlan is stale: update the checklist to reflect
completed work (check the remaining box for running typecheck/QA or mark
appropriately) and fill in the decision logs (previously TBD) to reflect that
`topLevelItems` was added to `getAllGrouped` and `reorderProjectChildren` (and
that `computeVisualOrder` was moved onto the shared mixed top-level ordering
helper), and confirm how new items should receive `tabOrder`; ensure lines
referencing `topLevelItems`, `reorderProjectChildren`, `getAllGrouped`, and
`computeVisualOrder` are updated to the current state and remove or adjust any
outstanding TODOs before merging.
In `@apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts`:
- Around line 68-76: The empty-group filter currently sums group.workspaces and
section.workspaces to decide whether to keep a group; change it to check
group.topLevelItems (e.g., replace the whole predicate in the .filter callback)
so the filter keeps groups when group.topLevelItems.length > 0 — update the
.filter to use group.topLevelItems.length instead of the manual sum over
workspaces/sections to prevent removing groups that still have visible top-level
children.
---
Outside diff comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts`:
- Around line 472-483: createBranchWorkspace currently hardcodes tabOrder and
only shifts workspaces, causing conflicts with the mixed top-level ordering;
update createBranchWorkspace to use the same ordering helper as other creation
paths by calling getMaxProjectChildTabOrder(input.projectId) and set tabOrder to
max+1 when inserting into workspaces (or alternatively invoke the shared
normalization routine used by the other paths). Also ensure you
adjust/renormalize section items consistently by calling getProjectChildItems
and applying the same shift/normalize logic (the insertion in
createBranchWorkspace and the shifting block that currently operates on
workspaces must mirror the shared behavior used elsewhere) so no two items end
up with the same tabOrder.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/sections.ts`:
- Around line 98-127: The reorderSections mutation (reorderSections) only
reorders workspaceSections and can conflict with the newer
reorderProjectChildren when project-level mixed ordering exists; mark
reorderSections as deprecated and add a clear comment above the procedure
explaining it only reorders sections within a project (does not handle
interleaved ungrouped workspaces), advise using reorderProjectChildren for mixed
top-level ordering, and keep the existing logic (uses localDb,
workspaceSections, reorderItems) for any UI flows that specifically require
section-only reordering.
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/status.ts`:
- Around line 64-97: The reorderProjectChildren mutation is brittle because it
blindly reorders whatever is at items[fromIndex]; change the mutation to accept
the dragged child's identifier (e.g., kind and id) in the input, then verify
that items[fromIndex] matches that identifier before calling
reorderProjectChildItems in order to make the operation deterministic; if
items[fromIndex] does not match, locate the correct current index by matching
kind/id in the items array (or return an error) and use that resolved index as
the source to call reorderProjectChildItems with fromIndex and toIndex. Ensure
updates touch the input z.object schema, the mutation handler
(reorderProjectChildren), and the logic that computes and passes indices to
reorderProjectChildItems.
In
`@apps/desktop/src/lib/trpc/routers/workspaces/utils/project-children-order.test.ts`:
- Around line 60-78: Add tests to cover no-op and bounds behavior for
reorderProjectChildItems (and mirror semantics expected from
reorderProjectChildren): add a test where fromIndex === toIndex to assert the
output is normalized (tabOrder reset to 0..n-1) but item order unchanged, and
add tests for out-of-range indices (negative and indices >= items.length) to
assert they are handled deterministically (e.g., treated as clamped to 0 or
items.length-1 or throw—match reorderProjectChildren behavior) so regressions
are caught; reference the functions reorderProjectChildItems and
reorderProjectChildren when adding these cases to
project-children-order.test.ts.
In `@apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts`:
- Around line 49-76: The grouped-cache mutation that filters out a workspace
(the map/filter block in useCloseWorkspace) is duplicated in useDeleteWorkspace;
extract it into a shared helper (e.g., updateGroupsRemovingWorkspace(groups, id)
or normalizeGroupsAfterWorkspaceRemoval) that accepts the groups array and the
workspace id and returns the new groups array using the same map/filter logic
(preserve topLevelItems removal when isTopLevelWorkspace, remove workspace from
group.workspaces and section.workspaces, then filter out empty groups). Replace
the inlined code in useCloseWorkspace and useDeleteWorkspace with a call to this
helper, export/import it from a common module, and ensure TypeScript types for
Group/Section/Workspace and any tests or usage of mutate callbacks are updated
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: de92f204-80d9-428a-b1d7-3f50d6f51395
📒 Files selected for processing (24)
apps/desktop/plans/20260306-2140-mixed-project-children-ordering.mdapps/desktop/src/lib/trpc/routers/workspaces/procedures/create.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/sections.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/status.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/project-children-order.test.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/project-children-order.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/visual-order.test.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/visual-order.tsapps/desktop/src/renderer/hooks/useWorkspaceShortcuts.tsapps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.tsapps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.tsapps/desktop/src/renderer/react-query/workspaces/useReorderProjectChildren.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/useWorkspaceDnD.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSection/WorkspaceSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/constants.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/utils/reorderProjectChildrenInCache.tsapps/desktop/src/renderer/screens/main/hooks/useBranchSyncInvalidation/useBranchSyncInvalidation.tsapps/desktop/src/renderer/stores/active-drag-item.test.ts
Comment on lines
+41
to
+54
| ## Progress | ||
|
|
||
| - [x] (2026-03-07 05:40Z) Draft ExecPlan | ||
| - [x] (2026-03-07 05:43Z) Inventory ordering assumptions across `getAllGrouped`, `computeVisualOrder`, sidebar DnD, and workspace shortcuts | ||
| - [x] (2026-03-07 05:45Z) Add shared server helper for mixed top-level project child ordering | ||
| - [x] (2026-03-07 05:45Z) Add backend tests for mixed workspace/section ordering | ||
| - [x] (2026-03-07 05:45Z) Route new top-level workspace/section creation through shared top-level child ordering | ||
| - [x] (2026-03-07 05:47Z) Move `computeVisualOrder` onto the shared mixed top-level ordering helper | ||
| - [x] (2026-03-07 06:19Z) Add `topLevelItems` to `getAllGrouped` and update keyboard shortcuts to use mixed top-level ordering | ||
| - [x] (2026-03-07 06:19Z) Replace sidebar rendering with one ordered top-level list per project | ||
| - [x] (2026-03-07 06:19Z) Replace section-only reorder mutation/UI flow with mixed top-level reorder flow | ||
| - [x] (2026-03-07 06:19Z) Update keyboard shortcut and next/previous workspace ordering | ||
| - [ ] Audit create/move paths so new items get correct top-level `tabOrder` | ||
| - [ ] Run `bun run typecheck`, targeted desktop tests, and manual sidebar DnD QA |
Contributor
There was a problem hiding this comment.
This ExecPlan is already stale.
Line 54 is still unchecked, and Lines 68-75 leave both decision logs as TBD even though this PR already adds topLevelItems and reorderProjectChildren. Since Line 3 says these sections must stay current, please update the plan before merge.
Also applies to: 66-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/plans/20260306-2140-mixed-project-children-ordering.md` around
lines 41 - 54, The ExecPlan is stale: update the checklist to reflect completed
work (check the remaining box for running typecheck/QA or mark appropriately)
and fill in the decision logs (previously TBD) to reflect that `topLevelItems`
was added to `getAllGrouped` and `reorderProjectChildren` (and that
`computeVisualOrder` was moved onto the shared mixed top-level ordering helper),
and confirm how new items should receive `tabOrder`; ensure lines referencing
`topLevelItems`, `reorderProjectChildren`, `getAllGrouped`, and
`computeVisualOrder` are updated to the current state and remove or adjust any
outstanding TODOs before merging.
Comment on lines
+68
to
+76
| .filter( | ||
| (group) => | ||
| group.workspaces.length + | ||
| group.sections.reduce( | ||
| (sum, section) => sum + section.workspaces.length, | ||
| 0, | ||
| ) > | ||
| 0, | ||
| ), |
Contributor
There was a problem hiding this comment.
Use topLevelItems for the empty-group check.
This filter only counts remaining workspaces. If the last workspace in a project is closed but the project still has section items, the whole group disappears from the optimistic cache until refetch even though topLevelItems still has visible children.
💡 Proposed fix
- .filter(
- (group) =>
- group.workspaces.length +
- group.sections.reduce(
- (sum, section) => sum + section.workspaces.length,
- 0,
- ) >
- 0,
- ),
+ .filter((group) => group.topLevelItems.length > 0),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .filter( | |
| (group) => | |
| group.workspaces.length + | |
| group.sections.reduce( | |
| (sum, section) => sum + section.workspaces.length, | |
| 0, | |
| ) > | |
| 0, | |
| ), | |
| .filter((group) => group.topLevelItems.length > 0), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts` around
lines 68 - 76, The empty-group filter currently sums group.workspaces and
section.workspaces to decide whether to keep a group; change it to check
group.topLevelItems (e.g., replace the whole predicate in the .filter callback)
so the filter keeps groups when group.topLevelItems.length > 0 — update the
.filter to use group.topLevelItems.length instead of the manual sum over
workspaces/sections to prevent removing groups that still have visible top-level
children.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/utils/reorderProjectChildrenInCache.test.ts (1)
56-73: Consider adding a test forundefinedinput.The isolation test is valuable. However, looking at the real call site in
WorkspaceSection.tsx, the function receivesoldDatafromsetData(undefined, (oldData) => ...)which could beundefinedif the cache is empty.Consider adding a test to verify the function handles
undefinedgracefully:test("returns undefined when data is undefined", () => { expect(reorderProjectChildrenInCache(undefined, "p1", 0, 1)).toBeUndefined(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/utils/reorderProjectChildrenInCache.test.ts` around lines 56 - 73, Add a unit test to ensure reorderProjectChildrenInCache handles undefined input: add a test in reorderProjectChildrenInCache.test.ts that calls reorderProjectChildrenInCache(undefined, "p1", 0, 1) and asserts the result is undefined (e.g., expect(...).toBeUndefined()), since the real caller in WorkspaceSection.tsx may pass oldData as undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/utils/reorderProjectChildrenInCache.test.ts`:
- Around line 56-73: Add a unit test to ensure reorderProjectChildrenInCache
handles undefined input: add a test in reorderProjectChildrenInCache.test.ts
that calls reorderProjectChildrenInCache(undefined, "p1", 0, 1) and asserts the
result is undefined (e.g., expect(...).toBeUndefined()), since the real caller
in WorkspaceSection.tsx may pass oldData as undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 87d904e7-4b65-4c0f-855d-3aea1de4757d
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/utils/reorderProjectChildrenInCache.test.ts
This was referenced Mar 16, 2026
9 tasks
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
1 participant
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.
Summary
topLevelItemsfrom grouped workspaces and keep optimistic sidebar caches in syncTesting
Summary by cubic
Refactored the desktop workspace sidebar to treat a project’s top‑level children (ungrouped workspaces + sections) as one ordered list. You can now drag sections and ungrouped workspaces together, and keyboard navigation follows the same order.
New Features
Refactors
Written for commit 54026e2. Summary will update on new commits.
Summary by CodeRabbit