fix(desktop): keep sidebar from sticking on "creating" when Electric is slow#4390
Merged
Conversation
…is slow Mirrors the workspace-details fix from #4141 for the sidebar. After workspaces.create resolves end-to-end on the host service, the sidebar would keep rendering the in-flight "creating" row until Electric pushed the synced v2Workspaces row into collections — same root cause as the detail page, different consumer. If Electric was slow or disconnected, the row stuck on "creating" forever (until a refresh reset the in-memory in-flight store and Electric reconnected). useDashboardSidebarData now surfaces in-flight entries that carry a cloudRow as fully-synced sidebar rows, built from the cloud row plus the local state row useWorkspaceCreates.dispatch already inserts. The pending "creating" row is filtered out for those entries so we don't double-render. Manager.tsx still removes the in-flight entry once Electric catches up, at which point the live query takes over.
Contributor
📝 WalkthroughWalkthroughThe ChangesCloud Row Fallback Workspace Rendering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts (1)
300-331: ⚖️ Poor tradeoffFallback memo isn't reactive to
v2WorkspaceLocalStatemutations.
collections.v2WorkspaceLocalState.get(cloudRow.id)is a synchronous, non-subscribing read. The memo only recomputes whencollections,hosts,inFlightEntries, orlocalStateWorkspaceIdschange — andlocalStateWorkspaceIdsis derived from therawSidebarWorkspacesjoin, which stays empty for this workspace until Electric delivers thev2Workspacesrow.Net effect: while the fallback row is active (slow-Electric window), reordering, moving between sections, or hiding the new workspace updates
v2WorkspaceLocalStatebut the rendered fallback keeps the originalprojectId/tabOrder/sectionId/isHiddenuntil the live query takes over. It self-heals when Electric catches up, so it's a narrow UX glitch rather than a correctness bug — flagging as optional given the fallback is intentionally short-lived.If you want immediate reactivity, derive a
Map<workspaceId, sidebarState>from auseLiveQueryonv2WorkspaceLocalStateand read it in the memo instead of calling.get()directly:♻️ Sketch: make local-state lookups reactive
+ const { data: allLocalStates = [] } = useLiveQuery( + (q) => + q.from({ ls: collections.v2WorkspaceLocalState }).select(({ ls }) => ({ + workspaceId: ls.workspaceId, + sidebarState: ls.sidebarState, + })), + [collections], + ); + const localStateByWorkspaceId = useMemo( + () => new Map(allLocalStates.map((row) => [row.workspaceId, row.sidebarState])), + [allLocalStates], + ); + const cloudRowFallbackWorkspaces = useMemo(() => { if (inFlightEntries.length === 0) return []; const hostByMachineId = new Map( hosts.map((host) => [host.machineId, host]), ); const rows = inFlightEntries.flatMap((entry) => { const cloudRow = entry.cloudRow; if (!cloudRow) return []; if (localStateWorkspaceIds.has(cloudRow.id)) return []; - const localState = collections.v2WorkspaceLocalState.get(cloudRow.id); + const sidebarState = localStateByWorkspaceId.get(cloudRow.id); const host = hostByMachineId.get(cloudRow.hostId); return [ { id: cloudRow.id, - projectId: localState?.sidebarState.projectId ?? cloudRow.projectId, + projectId: sidebarState?.projectId ?? cloudRow.projectId, hostId: cloudRow.hostId, type: cloudRow.type, hostIsOnline: host?.isOnline ?? false, name: cloudRow.name, branch: cloudRow.branch, createdAt: cloudRow.createdAt, updatedAt: cloudRow.updatedAt, - tabOrder: - localState?.sidebarState.tabOrder ?? PENDING_WORKSPACE_TAB_ORDER, - sectionId: localState?.sidebarState.sectionId ?? null, - isHidden: localState?.sidebarState.isHidden ?? false, + tabOrder: sidebarState?.tabOrder ?? PENDING_WORKSPACE_TAB_ORDER, + sectionId: sidebarState?.sectionId ?? null, + isHidden: sidebarState?.isHidden ?? false, }, ]; }); return getVisibleSidebarWorkspaces(rows); - }, [collections, hosts, inFlightEntries, localStateWorkspaceIds]); + }, [hosts, inFlightEntries, localStateByWorkspaceId, localStateWorkspaceIds]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts` around lines 300 - 331, The fallback memo cloudRowFallbackWorkspaces currently reads collections.v2WorkspaceLocalState.get(cloudRow.id) synchronously and so doesn't react to later mutations; fix it by creating a reactive lookup (e.g. useLiveQuery that selects all v2WorkspaceLocalState entries and builds a Map<workspaceId, sidebarState>) and use that Map inside cloudRowFallbackWorkspaces instead of calling .get() directly; ensure the new reactiveMap (name it something like v2WorkspaceLocalStateMap) is added to the useMemo dependency array and keep other symbols unchanged (cloudRowFallbackWorkspaces, inFlightEntries, hosts, localStateWorkspaceIds, getVisibleSidebarWorkspaces) so fallback rows update immediately when sidebar state changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts`:
- Around line 300-331: The fallback memo cloudRowFallbackWorkspaces currently
reads collections.v2WorkspaceLocalState.get(cloudRow.id) synchronously and so
doesn't react to later mutations; fix it by creating a reactive lookup (e.g.
useLiveQuery that selects all v2WorkspaceLocalState entries and builds a
Map<workspaceId, sidebarState>) and use that Map inside
cloudRowFallbackWorkspaces instead of calling .get() directly; ensure the new
reactiveMap (name it something like v2WorkspaceLocalStateMap) is added to the
useMemo dependency array and keep other symbols unchanged
(cloudRowFallbackWorkspaces, inFlightEntries, hosts, localStateWorkspaceIds,
getVisibleSidebarWorkspaces) so fallback rows update immediately when sidebar
state changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8123e911-593f-4266-af9e-4b1f3e21c8b1
📒 Files selected for processing (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts
Contributor
Greptile SummaryThis PR mirrors the workspace-details fix from #4141 to the dashboard sidebar. When
Confidence Score: 4/5Safe to merge — the fix is additive and does not touch the live-query or in-flight-entry state machine. The core handoff logic is sound. The two flagged concerns are narrow edge cases: one requires a host row to be absent during workspace creation (unlikely in practice), and the other is a theoretical staleness window that the current write-once local-state usage pattern avoids. The single changed file is worth a focused read on the cloudRowFallbackWorkspaces memo and the localStateWorkspaceIds guard interaction.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts | Adds cloudRowFallbackWorkspaces memo and filters 'creating' entries with cloudRow from inFlightSidebarRows; logic is sound for the happy path but has two minor edge-case concerns around host-not-found behavior divergence and direct collection access. |
Sequence Diagram
sequenceDiagram
participant D as dispatch (useWorkspaceCreates)
participant HS as Host Service
participant E as Electric Sync
participant S as useDashboardSidebarData
D->>D: Insert v2WorkspaceLocalState row (locally)
D->>S: "inFlightEntries gains entry (state=creating, cloudRow=null)"
S->>S: inFlightSidebarRows shows creating badge
D->>HS: workspaces.create(...)
HS-->>D: Resolves entry.cloudRow set, state still creating
Note over S: Electric is slow, v2Workspaces not yet delivered
S->>S: inFlightSidebarRows filters out entry
S->>S: cloudRowFallbackWorkspaces emits synced row
S->>S: visibleSidebarWorkspaces shows workspace as fully synced
E-->>S: v2Workspaces row arrives, inner-join succeeds
S->>S: localStateWorkspaceIds gains cloudRow.id
S->>S: cloudRowFallbackWorkspaces evicts fallback row
S->>S: sidebarWorkspaces live query owns row
Note over D: Manager.tsx removes in-flight entry
D->>S: inFlightEntries loses entry entirely
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts:305-312
The fallback includes a workspace with `hostIsOnline: false` even when `cloudRow.hostId` doesn't match any entry in the `hosts` live-query result. The real `rawSidebarWorkspaces` live query uses an inner join with `v2Hosts`, so it would silently exclude the workspace in that same situation. In practice this window should be zero (the host row existed before the workspace was created), but adding an early-return guard makes the fallback's intent explicit and consistent with the live-query contract.
```suggestion
const rows = inFlightEntries.flatMap((entry) => {
const cloudRow = entry.cloudRow;
if (!cloudRow) return [];
// Electric already delivered; let the live query own this row.
if (localStateWorkspaceIds.has(cloudRow.id)) return [];
const localState = collections.v2WorkspaceLocalState.get(cloudRow.id);
const host = hostByMachineId.get(cloudRow.hostId);
// No host row yet — mirror the inner-join behaviour of rawSidebarWorkspaces.
if (!host) return [];
return [
```
### Issue 2 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts:310
**Direct collection read may not re-run on local-state updates**
`collections.v2WorkspaceLocalState.get(cloudRow.id)` reads the TanStack DB collection directly inside a `useMemo`. If `collections` is a stable context-object reference (i.e. its identity doesn't change when a row is upserted), this memo will not re-execute when `v2WorkspaceLocalState` is updated after `cloudRow` is set — so `projectId`, `tabOrder`, `sectionId`, and `isHidden` could be stale. In practice the local-state row is written by `useWorkspaceCreates.dispatch` before the API call and isn't mutated during the fallback window, so the impact is minimal today. But if that assumption changes (e.g. user reorders while Electric is slow), the fallback would silently render with outdated positioning.
Reviews (1): Last reviewed commit: "fix(desktop): keep sidebar from sticking..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+305
to
+312
| const rows = inFlightEntries.flatMap((entry) => { | ||
| const cloudRow = entry.cloudRow; | ||
| if (!cloudRow) return []; | ||
| // Electric already delivered; let the live query own this row. | ||
| if (localStateWorkspaceIds.has(cloudRow.id)) return []; | ||
| const localState = collections.v2WorkspaceLocalState.get(cloudRow.id); | ||
| const host = hostByMachineId.get(cloudRow.hostId); | ||
| return [ |
Contributor
There was a problem hiding this comment.
hostIsOnline: false even when cloudRow.hostId doesn't match any entry in the hosts live-query result. The real rawSidebarWorkspaces live query uses an inner join with v2Hosts, so it would silently exclude the workspace in that same situation. In practice this window should be zero (the host row existed before the workspace was created), but adding an early-return guard makes the fallback's intent explicit and consistent with the live-query contract.
Suggested change
| const rows = inFlightEntries.flatMap((entry) => { | |
| const cloudRow = entry.cloudRow; | |
| if (!cloudRow) return []; | |
| // Electric already delivered; let the live query own this row. | |
| if (localStateWorkspaceIds.has(cloudRow.id)) return []; | |
| const localState = collections.v2WorkspaceLocalState.get(cloudRow.id); | |
| const host = hostByMachineId.get(cloudRow.hostId); | |
| return [ | |
| const rows = inFlightEntries.flatMap((entry) => { | |
| const cloudRow = entry.cloudRow; | |
| if (!cloudRow) return []; | |
| // Electric already delivered; let the live query own this row. | |
| if (localStateWorkspaceIds.has(cloudRow.id)) return []; | |
| const localState = collections.v2WorkspaceLocalState.get(cloudRow.id); | |
| const host = hostByMachineId.get(cloudRow.hostId); | |
| // No host row yet — mirror the inner-join behaviour of rawSidebarWorkspaces. | |
| if (!host) return []; | |
| return [ |
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: 305-312
Comment:
The fallback includes a workspace with `hostIsOnline: false` even when `cloudRow.hostId` doesn't match any entry in the `hosts` live-query result. The real `rawSidebarWorkspaces` live query uses an inner join with `v2Hosts`, so it would silently exclude the workspace in that same situation. In practice this window should be zero (the host row existed before the workspace was created), but adding an early-return guard makes the fallback's intent explicit and consistent with the live-query contract.
```suggestion
const rows = inFlightEntries.flatMap((entry) => {
const cloudRow = entry.cloudRow;
if (!cloudRow) return [];
// Electric already delivered; let the live query own this row.
if (localStateWorkspaceIds.has(cloudRow.id)) return [];
const localState = collections.v2WorkspaceLocalState.get(cloudRow.id);
const host = hostByMachineId.get(cloudRow.hostId);
// No host row yet — mirror the inner-join behaviour of rawSidebarWorkspaces.
if (!host) return [];
return [
```
How can I resolve this? If you propose a fix, please make it concise.| if (!cloudRow) return []; | ||
| // Electric already delivered; let the live query own this row. | ||
| if (localStateWorkspaceIds.has(cloudRow.id)) return []; | ||
| const localState = collections.v2WorkspaceLocalState.get(cloudRow.id); |
Contributor
There was a problem hiding this comment.
collections.v2WorkspaceLocalState.get(cloudRow.id) reads the TanStack DB collection directly inside a useMemo. If collections is a stable context-object reference (i.e. its identity doesn't change when a row is upserted), this memo will not re-execute when v2WorkspaceLocalState is updated after cloudRow is set — so projectId, tabOrder, sectionId, and isHidden could be stale. In practice the local-state row is written by useWorkspaceCreates.dispatch before the API call and isn't mutated during the fallback window, so the impact is minimal today. But if that assumption changes (e.g. user reorders while Electric is slow), the fallback would silently render with outdated positioning.
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: 310
Comment:
**Direct collection read may not re-run on local-state updates**
`collections.v2WorkspaceLocalState.get(cloudRow.id)` reads the TanStack DB collection directly inside a `useMemo`. If `collections` is a stable context-object reference (i.e. its identity doesn't change when a row is upserted), this memo will not re-execute when `v2WorkspaceLocalState` is updated after `cloudRow` is set — so `projectId`, `tabOrder`, `sectionId`, and `isHidden` could be stale. In practice the local-state row is written by `useWorkspaceCreates.dispatch` before the API call and isn't mutated during the fallback window, so the impact is minimal today. But if that assumption changes (e.g. user reorders while Electric is slow), the fallback would silently render with outdated positioning.
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
May 25, 2026
…is slow (superset-sh#4390) Mirrors the workspace-details fix from superset-sh#4141 for the sidebar. After workspaces.create resolves end-to-end on the host service, the sidebar would keep rendering the in-flight "creating" row until Electric pushed the synced v2Workspaces row into collections — same root cause as the detail page, different consumer. If Electric was slow or disconnected, the row stuck on "creating" forever (until a refresh reset the in-memory in-flight store and Electric reconnected). useDashboardSidebarData now surfaces in-flight entries that carry a cloudRow as fully-synced sidebar rows, built from the cloud row plus the local state row useWorkspaceCreates.dispatch already inserts. The pending "creating" row is filtered out for those entries so we don't double-render. Manager.tsx still removes the in-flight entry once Electric catches up, at which point the live query takes over.
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
workspaces.createresolves on the host service, the sidebar's live query inner-joinsv2WorkspaceLocalState ∩ v2Workspaces ∩ v2Hosts— if Electric stalls onv2Workspaces, the inner join drops the row, the in-flight entry stays instate: "creating", and the pending row sticks forever (until refresh clears the in-memory store and Electric reconnects).useDashboardSidebarDatanow synthesizes a synced sidebar workspace from each in-flight entry'scloudRow(already cached by fix(desktop): unblock v2 workspace render when Electric is slow #4141) when the row hasn't been delivered by Electric yet, using the local state row thatuseWorkspaceCreates.dispatchinserts forprojectId/tabOrder/sectionId. The pending "creating" row is filtered out for those entries to avoid double-render.Manager.tsxstill removes the in-flight entry once Electric catches up — seamless handoff.Test plan
workspaces.createresolves, instead of sticking on "creating".workspaces.createto error — pending failed row still renders and can be dismissed.Summary by cubic
Prevents the dashboard sidebar from sticking on “creating” when Electric is slow by rendering a synced fallback from each in‑flight entry’s
cloudRowuntil the livev2Workspacesrow arrives. Seamless handoff with no duplicates or flicker; addresses SUPER-610.cloudRowdon’t render twice.cloudRowplus local sidebar state (projectId,tabOrder,sectionId) until sync delivers.Manager.tsxto remove the in‑flight entry once Electric catches up so the live query takes over.Written for commit f9ae618. Summary will update on new commits.
Summary by CodeRabbit