fix(desktop): v2 sidebar section count matches visual grouping#3544
fix(desktop): v2 sidebar section count matches visual grouping#3544saddlepaddle merged 1 commit intomainfrom
Conversation
…spaces Ungrouped workspaces that render after a section header are already visually grouped with that section (shared accent color, collapse together) and get committed into it on next drag. The count, however, was reading section.workspaces which only included workspaces whose DB sectionId already matched — so a freshly created section showed "(0)" while the user sees N workspaces beneath it. Reparent those workspaces into section.workspaces in the data layer so the count matches the visual grouping and DnD commit behavior.
📝 WalkthroughWalkthroughModified the dashboard sidebar project child assembly logic to reparent "ungrouped" workspace children that appear after section headers into the most recent section, updating their accentColor and maintaining section workspace counts in the output structure. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 docstrings
🧪 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 |
Greptile SummaryThis PR fixes a bug where v2 sidebar section workspace counts showed The fix adds a post-sort reparenting pass in Key points:
Confidence Score: 4/5Safe to merge; fix is logically correct for the primary bug with one edge case to verify regarding pending workspaces. The reparenting logic is sound, consistent with how useSidebarDnd.ts already models grouping, and carries no double-counting risk for DB-matched section members. The only open question is whether pending workspaces (tabOrder = MAX_SAFE_INTEGER) being absorbed into the last section is intentional — if not, a one-line guard resolves it. No data-loss or security concerns. useDashboardSidebarData.ts — verify pending workspace reparenting behavior at lines 338–349.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts | Adds a post-sort reparenting pass that moves ungrouped workspace children following a section into that section's workspaces array; clean fix with one pending-workspace edge case to verify. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[childEntries sorted by tabOrder] --> B{Iterate sortedChildren}
B --> C{child.type?}
C -- section --> D[currentSection = child.section\npush to children]
D --> B
C -- workspace\nAND currentSection != null --> E[currentSection.workspaces.push\nworkspace + accentColor]
E --> F{workspace.creationStatus set?\ni.e. pending workspace?}
F -- No, regular workspace --> B
F -- Yes, pending workspace\ntabOrder = MAX_SAFE_INTEGER --> G[⚠️ Also reparented into\nlast section if one exists]
G --> B
C -- workspace\nAND currentSection == null --> H[push to children as\ntop-level entry]
H --> B
B -- done --> I[sidebarProject.children = children\nsection.workspaces includes\nboth DB-matched + reparented members]
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts
Line: 338-349
Comment:
**Pending workspaces will be reparented into the last section**
`pendingWorkspaces` are injected into `childEntries` with `tabOrder: PENDING_WORKSPACE_TAB_ORDER` (`Number.MAX_SAFE_INTEGER`), so they always sort to the very end. If any section exists in the project, the reparenting loop will inevitably assign them to `currentSection` — meaning a "Creating…" or "Failed" workspace row will:
1. Receive the last section's accent color instead of `null`.
2. Count toward that section's workspace total.
3. Disappear when the user collapses that section.
If the visual render intentionally places pending workspaces under the last section header (which would make this data change correct and desirable), this is fine. But if pending workspaces are meant to always float at the top level regardless of sections, a guard is needed:
```typescript
} else if (currentSection && child.type === "workspace" && !child.workspace.creationStatus) {
currentSection.workspaces.push({
...child.workspace,
accentColor: currentSection.color,
});
} else {
children.push(child);
}
```
Worth double-checking whether the sidebar renderer visually groups pending workspaces with the last section before merging.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(desktop): v2 sidebar section count r..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (const child of sortedChildren) { | ||
| if (child.type === "section") { | ||
| currentSection = child.section; | ||
| children.push(child); | ||
| } else if (currentSection) { | ||
| currentSection.workspaces.push({ | ||
| ...child.workspace, | ||
| accentColor: currentSection.color, | ||
| }); | ||
| } else { | ||
| children.push(child); | ||
| } |
There was a problem hiding this comment.
pendingWorkspaces are injected into childEntries with tabOrder: PENDING_WORKSPACE_TAB_ORDER (Number.MAX_SAFE_INTEGER), so they always sort to the very end. If any section exists in the project, the reparenting loop will inevitably assign them to currentSection — meaning a "Creating…" or "Failed" workspace row will:
- Receive the last section's accent color instead of
null. - Count toward that section's workspace total.
- Disappear when the user collapses that section.
If the visual render intentionally places pending workspaces under the last section header (which would make this data change correct and desirable), this is fine. But if pending workspaces are meant to always float at the top level regardless of sections, a guard is needed:
} else if (currentSection && child.type === "workspace" && !child.workspace.creationStatus) {
currentSection.workspaces.push({
...child.workspace,
accentColor: currentSection.color,
});
} else {
children.push(child);
}Worth double-checking whether the sidebar renderer visually groups pending workspaces with the last section before merging.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts
Line: 338-349
Comment:
**Pending workspaces will be reparented into the last section**
`pendingWorkspaces` are injected into `childEntries` with `tabOrder: PENDING_WORKSPACE_TAB_ORDER` (`Number.MAX_SAFE_INTEGER`), so they always sort to the very end. If any section exists in the project, the reparenting loop will inevitably assign them to `currentSection` — meaning a "Creating…" or "Failed" workspace row will:
1. Receive the last section's accent color instead of `null`.
2. Count toward that section's workspace total.
3. Disappear when the user collapses that section.
If the visual render intentionally places pending workspaces under the last section header (which would make this data change correct and desirable), this is fine. But if pending workspaces are meant to always float at the top level regardless of sections, a guard is needed:
```typescript
} else if (currentSection && child.type === "workspace" && !child.workspace.creationStatus) {
currentSection.workspaces.push({
...child.workspace,
accentColor: currentSection.color,
});
} else {
children.push(child);
}
```
Worth double-checking whether the sidebar renderer visually groups pending workspaces with the last section before merging.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts`:
- Around line 328-351: The loop that reparents ungrouped children into the last
seen section (`sortedChildren` → `children`, `currentSection`) must skip
transient pending workspaces so they aren't silently moved into a section;
change the reparenting branch to detect workspace creation status (check
`child.workspace.creationStatus` or the `creationStatus` discriminator on
`DashboardSidebarProjectChild`) and treat those items like ungrouped entries
(push them into `children` instead of into `currentSection.workspaces`), and
when you do reparent non-pending workspaces append them into
`currentSection.workspaces` and then sort/merge that array by `tabOrder` so
in-section ordering matches `tabOrder`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b381f83-2a88-4d7c-bd1e-d291ff86454c
📒 Files selected for processing (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts
| const sortedChildren = childEntries | ||
| .sort((left, right) => left.tabOrder - right.tabOrder) | ||
| .map(({ child }) => child); | ||
|
|
||
| // Ungrouped workspaces rendered after a section header are visually | ||
| // grouped with that section (shared accent, collapse-together) and will | ||
| // be committed into it on next DnD. Reparent them here so section counts | ||
| // match what the user sees. | ||
| const children: DashboardSidebarProjectChild[] = []; | ||
| let currentSection: DashboardSidebarSection | null = null; | ||
| for (const child of sortedChildren) { | ||
| if (child.type === "section") { | ||
| currentSection = child.section; | ||
| children.push(child); | ||
| } else if (currentSection) { | ||
| currentSection.workspaces.push({ | ||
| ...child.workspace, | ||
| accentColor: currentSection.color, | ||
| }); | ||
| } else { | ||
| children.push(child); | ||
| } | ||
| } | ||
| sidebarProject.children = children; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the useDashboardSidebarData.ts file to understand the pending workspace handling
fd -t f "useDashboardSidebarData.ts" apps/desktopRepository: superset-sh/superset
Length of output: 207
🏁 Script executed:
# Read the file to see the full context around pending workspaces and the reparenting logic
cat -n "apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts" | head -360 | tail -100Repository: superset-sh/superset
Length of output: 3448
🏁 Script executed:
# Find where PENDING_WORKSPACE_TAB_ORDER is defined
rg "PENDING_WORKSPACE_TAB_ORDER" apps/desktop/srcRepository: superset-sh/superset
Length of output: 455
🏁 Script executed:
# Search for creationStatus usage to understand the discriminator
rg "creationStatus" apps/desktop/src -A 2 -B 2Repository: superset-sh/superset
Length of output: 24687
🏁 Script executed:
# Check for git history or comments around the reparenting logic
git log --oneline -20 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts 2>/dev/null || echo "Git history not available"Repository: superset-sh/superset
Length of output: 148
🏁 Script executed:
# Look for additional context around the reparenting section—check broader context
sed -n '315,355p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.tsRepository: superset-sh/superset
Length of output: 1265
🏁 Script executed:
# Search for any TODOs or design notes related to pending workspaces and sections
rg "pending.*section|section.*pending" apps/desktop/src -i -A 2 -B 2Repository: superset-sh/superset
Length of output: 709
Pending workspaces are silently reparented into the last section, inflating its count and visually grouping transient items incorrectly.
Pending workspaces are added to childEntries with tabOrder = PENDING_WORKSPACE_TAB_ORDER (Number.MAX_SAFE_INTEGER), causing them to sort at the tail of sortedChildren. When the project contains at least one section, the reparenting loop then moves every pending (creating/failed/preparing) item into the last section's workspaces array, applying its color and visually attaching unrelated in-flight items to that section.
The reparenting logic was designed to sync section counts with visual grouping for ungrouped user-placed workspaces awaiting DnD commit. Pending workspaces are intentionally transient (marked with creationStatus, filtered from shortcuts, rendered as disabled) and should remain separate from sections to preserve distinct visual and logical hierarchy. This unintended reparenting violates that separation.
Separately, when reparenting does occur, items are appended at the end of section.workspaces rather than merged in tabOrder order, so within-section ordering may briefly misalign with tabOrder until the next DnD commit.
Exclude pending items from reparenting using the creationStatus field discriminator, and (optionally) merge reparented ungrouped workspaces into section.workspaces in tabOrder order:
🛠 Guard to exclude pending workspaces
for (const child of sortedChildren) {
if (child.type === "section") {
currentSection = child.section;
children.push(child);
- } else if (currentSection) {
+ } else if (currentSection && !child.workspace.creationStatus) {
currentSection.workspaces.push({
...child.workspace,
accentColor: currentSection.color,
});
} else {
children.push(child);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts`
around lines 328 - 351, The loop that reparents ungrouped children into the last
seen section (`sortedChildren` → `children`, `currentSection`) must skip
transient pending workspaces so they aren't silently moved into a section;
change the reparenting branch to detect workspace creation status (check
`child.workspace.creationStatus` or the `creationStatus` discriminator on
`DashboardSidebarProjectChild`) and treat those items like ungrouped entries
(push them into `children` instead of into `currentSection.workspaces`), and
when you do reparent non-pending workspaces append them into
`currentSection.workspaces` and then sort/merge that array by `tabOrder` so
in-section ordering matches `tabOrder`.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 1 file
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/hooks/useDashboardSidebarData/useDashboardSidebarData.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts:342">
P2: Pending workspaces (`creationStatus` set) have `tabOrder = Number.MAX_SAFE_INTEGER`, so they always sort to the end and will be reparented into the last section by this branch. This causes them to inherit the section's accent color, inflate its count, and hide when the section is collapsed. Add a guard to exclude pending workspaces from reparenting.</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts:343">
P2: Reparented workspaces are always appended to `section.workspaces`, which can break tab-order ordering within a section when DB-owned section members already exist.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (child.type === "section") { | ||
| currentSection = child.section; | ||
| children.push(child); | ||
| } else if (currentSection) { |
There was a problem hiding this comment.
P2: Pending workspaces (creationStatus set) have tabOrder = Number.MAX_SAFE_INTEGER, so they always sort to the end and will be reparented into the last section by this branch. This causes them to inherit the section's accent color, inflate its count, and hide when the section is collapsed. Add a guard to exclude pending workspaces from reparenting.
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/hooks/useDashboardSidebarData/useDashboardSidebarData.ts, line 342:
<comment>Pending workspaces (`creationStatus` set) have `tabOrder = Number.MAX_SAFE_INTEGER`, so they always sort to the end and will be reparented into the last section by this branch. This causes them to inherit the section's accent color, inflate its count, and hide when the section is collapsed. Add a guard to exclude pending workspaces from reparenting.</comment>
<file context>
@@ -324,9 +324,31 @@ export function useDashboardSidebarData() {
+ if (child.type === "section") {
+ currentSection = child.section;
+ children.push(child);
+ } else if (currentSection) {
+ currentSection.workspaces.push({
+ ...child.workspace,
</file context>
| } else if (currentSection) { | |
| } else if (currentSection && !child.workspace.creationStatus) { |
| currentSection = child.section; | ||
| children.push(child); | ||
| } else if (currentSection) { | ||
| currentSection.workspaces.push({ |
There was a problem hiding this comment.
P2: Reparented workspaces are always appended to section.workspaces, which can break tab-order ordering within a section when DB-owned section members already exist.
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/hooks/useDashboardSidebarData/useDashboardSidebarData.ts, line 343:
<comment>Reparented workspaces are always appended to `section.workspaces`, which can break tab-order ordering within a section when DB-owned section members already exist.</comment>
<file context>
@@ -324,9 +324,31 @@ export function useDashboardSidebarData() {
+ currentSection = child.section;
+ children.push(child);
+ } else if (currentSection) {
+ currentSection.workspaces.push({
+ ...child.workspace,
+ accentColor: currentSection.color,
</file context>
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…spaces (superset-sh#3544) Ungrouped workspaces that render after a section header are already visually grouped with that section (shared accent color, collapse together) and get committed into it on next drag. The count, however, was reading section.workspaces which only included workspaces whose DB sectionId already matched — so a freshly created section showed "(0)" while the user sees N workspaces beneath it. Reparent those workspaces into section.workspaces in the data layer so the count matches the visual grouping and DnD commit behavior.
…tion-count upstream取り込み: v2 sidebar section count fix (superset-sh#3544)
…al dim sync) - 1979f4c fix(desktop): v2 sidebar section count reflects visually grouped workspaces (superset-sh#3544) → PR #315 (clean cherry-pick) - 867ef87 fix(desktop): sync v1 terminal dimensions to backend on connect (superset-sh#3545) → PR #316 (manual port for fork cold-restore divergence)
1 participant
Summary
(0)while N workspaces visually rendered under them.tabOrdercan land before ungrouped workspaces'tabOrder, so those workspaces render after the section header.groupInfoinuseSidebarDnd.tsalready treats them as in-section (for accent color + collapse), andcommitToDbreassigns theirsectionIdon next drag — butsection.workspacesonly included DB-matched members, so the count stayed at 0.useDashboardSidebarData.ts, after sorting children bytabOrder, reparent ungrouped workspaces following a section into that section'sworkspacesarray. Count, visual grouping, and DnD commit now agree.Test plan
commitToDbstill assignssectionIdcorrectly (unchanged)Summary by cubic
Fixes the v2 sidebar so section counts include workspaces that appear under a section. Counts now match the visual grouping and DnD behavior.
useDashboardSidebarData, after sorting bytabOrder, reparent ungrouped workspaces that follow a section into that section’sworkspaces.sectionIdas before.Written for commit 549f6c3. Summary will update on new commits.
Summary by CodeRabbit