Repository navigation
perf(desktop): cut sidebar hover and polling render fan-out - #6377
Conversation
Three sidebar rendering hot paths, pre-measured at ~365ms of long tasks for a plain mouse sweep across rows: - Hover-card PR refresh now goes through a per-workspace 45s cool-down gate (module-level, survives sidebar remounts), so sweeping across rows with an open card no longer fires a refreshByWorkspaces mutation + invalidation per row crossed. The refresh still fires only on the committed hoveredId. - One DashboardSidebarWorkspaceStatusProvider now owns the terminal-agent bindings queries, lifecycle-event subscriptions, and git:changed diff-stats invalidation for every visible workspace, replacing ~4 query observers and 2 bus listeners per row. Rows read a per-workspace entry through useSyncExternalStore, so a cache update re-renders only the rows whose entry changed. Query keys/staleTime are unchanged (dock badge aggregation still works); this cuts subscription fan-out, not fetches. The hover context is likewise split into a stable actions context plus a per-row isHovered subscription, so a hover commit re-renders two rows, not ~80. - Per-row AnimatePresence/motion.div wrappers in expanded project content are replaced with a CSS grid-rows/opacity transition; hidden rows stay mounted but inert, with their sortable disabled so DnD collision behavior matches the old unmount semantics. Section collapse/expand still animates and rows still hide while their section header is dragged.
📝 WalkthroughWalkthroughThe dashboard sidebar now centralizes workspace status, terminal bindings, unread state, attention state, and diff statistics. Hover actions use stable contexts with row-level subscriptions. Pull-request refreshes use a workspace-scoped cooldown gate. ChangesDashboard sidebar consolidation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DashboardSidebar
participant DashboardSidebarWorkspaceStatusProvider
participant DashboardSidebarWorkspaceItem
participant HostService
DashboardSidebar->>DashboardSidebarWorkspaceStatusProvider: provide workspace references
DashboardSidebarWorkspaceStatusProvider->>HostService: query bindings and diff statistics
HostService-->>DashboardSidebarWorkspaceStatusProvider: return workspace data
DashboardSidebarWorkspaceStatusProvider-->>DashboardSidebarWorkspaceItem: publish workspace status entry
DashboardSidebarWorkspaceItem->>DashboardSidebarWorkspaceStatusProvider: mark bound terminals seen
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx (1)
184-197: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueThe
JSON.stringifyfingerprint runs over every sidebar workspace.
computedTargetschanges identity wheneverhostWorkspacesCachechurns, so the fingerprint is re-serialized for the full workspace list each time. This is on the render path the PR optimizes. A field-wise comparison against the previous array avoids the allocation and scales the same.♻️ Optional: compare fields instead of serializing
- const previousTargetsRef = useRef<{ - fingerprint: string; - targets: WorkspaceStatusTarget[]; - } | null>(null); + const previousTargetsRef = useRef<WorkspaceStatusTarget[] | null>(null); const targets = useMemo(() => { - const fingerprint = JSON.stringify(computedTargets); - const previous = previousTargetsRef.current; - if (previous?.fingerprint === fingerprint) return previous.targets; - previousTargetsRef.current = { fingerprint, targets: computedTargets }; - return computedTargets; + const previous = previousTargetsRef.current; + if ( + previous && + previous.length === computedTargets.length && + previous.every( + (target, index) => + target.workspaceId === computedTargets[index].workspaceId && + target.hostUrl === computedTargets[index].hostUrl, + ) + ) { + return previous; + } + previousTargetsRef.current = computedTargets; + return computedTargets; }, [computedTargets]);🤖 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/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx` around lines 184 - 197, Replace the JSON.stringify-based fingerprinting in the targets useMemo with a field-wise comparison against previousTargetsRef.current.targets, checking the workspace and host URL fields that determine subscription changes. Reuse the previous targets array when those fields are unchanged, otherwise store and return computedTargets, while preserving the existing dependency and subscription behavior.
🤖 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.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarExpandedProjectContent/DashboardSidebarExpandedProjectContent.tsx`:
- Around line 136-183: Collapsed-section workspaces should not remain mounted or
receive status subscriptions. In
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarExpandedProjectContent/DashboardSidebarExpandedProjectContent.tsx#L136-L183,
restore conditional mounting for rows hidden by isInCollapsedSection and apply
the CSS grid-row transition only to the activeType === "section" drag-hide case;
in
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsx#L175-L192,
exclude workspaces in collapsed sections from statusWorkspaces so they do not
receive provider queries or subscriptions.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx`:
- Around line 136-141: Update the useEffect invoking onHoverCardOpen so the
optional async callback’s promise has a rejection handler, preventing
refreshWorkspacePullRequest failures from becoming unhandled rejections.
Preserve the existing isHovered, hostType, and id conditions while handling
rejected promises without changing successful hover behavior.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx`:
- Around line 287-305: Update the hasAttentionTerminal check in
DashboardSidebarWorkspaceStatusProvider to treat the "permission" status as an
attention terminal alongside "review" and "failed". Preserve the existing
isUnread calculation and status priority behavior.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx`:
- Around line 184-197: Replace the JSON.stringify-based fingerprinting in the
targets useMemo with a field-wise comparison against
previousTargetsRef.current.targets, checking the workspace and host URL fields
that determine subscription changes. Reuse the previous targets array when those
fields are unchanged, otherwise store and return computedTargets, while
preserving the existing dependency and subscription behavior.
🪄 Autofix
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 Plus
Run ID: b0dd2435-f360-487c-8623-dd2f54814c3f
📒 Files selected for processing (26)
apps/desktop/src/renderer/hooks/host-service/useDiffStats/index.tsapps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.tsapps/desktop/src/renderer/hooks/host-service/useV2NotificationStatus/index.tsapps/desktop/src/renderer/hooks/host-service/useV2NotificationStatus/useV2NotificationStatus.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarPinnedSection/DashboardSidebarPinnedSection.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarCollapsedProjectContent/DashboardSidebarCollapsedProjectContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarCollapsedProjectContent/components/SortableCollapsedWorkspaceItem/SortableCollapsedWorkspaceItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarExpandedProjectContent/DashboardSidebarExpandedProjectContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSessionsSection/DashboardSidebarSessionsSection.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/components/DashboardSidebarWorkspaceChips/DashboardSidebarWorkspaceChips.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/components/DashboardSidebarWorkspaceChips/hooks/useDashboardSidebarChipHoverSuppression/useDashboardSidebarChipHoverSuppression.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/components/DashboardSidebarWorkspaceChips/hooks/useDashboardSidebarWorkspaceRunningAgents/useDashboardSidebarWorkspaceRunningAgents.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceBulkContextMenu/DashboardSidebarWorkspaceBulkContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceContextMenu/DashboardSidebarWorkspaceContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/SortableWorkspaceItem/SortableWorkspaceItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/pullRequestRefreshCooldown.test.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/pullRequestRefreshCooldown.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarDndProvider/DashboardSidebarDndProvider.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarHoverProvider/DashboardSidebarHoverProvider.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarHoverProvider/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarWorkspaceStatusProvider/index.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/hooks/host-service/useV2NotificationStatus/index.ts
| // Rows collapse via a CSS grid-row transition instead of a | ||
| // per-row AnimatePresence/motion.div (~80 motion components | ||
| // cost real render time). Hidden rows stay mounted: `inert` | ||
| // removes them from focus/hit-testing and the disabled | ||
| // sortable unregisters their droppable, matching the old | ||
| // unmount behavior for DnD. | ||
| return ( | ||
| <AnimatePresence key={String(id)} initial={false}> | ||
| {!hidden && ( | ||
| <motion.div | ||
| initial={{ height: 0, opacity: 0 }} | ||
| animate={{ height: "auto", opacity: 1 }} | ||
| exit={{ height: 0, opacity: 0 }} | ||
| transition={{ duration: 0.15, ease: "easeOut" }} | ||
| > | ||
| <SortableWorkspaceItem | ||
| sortableId={String(id)} | ||
| workspace={workspace} | ||
| accentColor={group?.color} | ||
| isInSection={groupInfo.has(parsed.realId)} | ||
| onHoverCardOpen={() => | ||
| onWorkspaceHover(parsed.realId) | ||
| } | ||
| shortcutLabel={workspaceShortcutLabels.get( | ||
| parsed.realId, | ||
| )} | ||
| isSelected={ | ||
| canBulkSelect && | ||
| isWorkspaceSelected(parsed.realId) | ||
| } | ||
| onSelectionClick={ | ||
| canBulkSelect | ||
| ? (event) => | ||
| selectWorkspaceFromEvent(event, { | ||
| workspaceId: parsed.realId, | ||
| projectId, | ||
| orderedWorkspaceIds: | ||
| selectableWorkspaceIds, | ||
| }) | ||
| : undefined | ||
| } | ||
| disabled={ | ||
| workspace.type === "main" && | ||
| workspace.hostType === "local-device" | ||
| } | ||
| /> | ||
| </motion.div> | ||
| <div | ||
| key={String(id)} | ||
| className={cn( | ||
| "grid transition-[grid-template-rows,opacity] duration-150 ease-out", | ||
| hidden | ||
| ? "grid-rows-[0fr] opacity-0" | ||
| : "grid-rows-[1fr] opacity-100", | ||
| )} | ||
| </AnimatePresence> | ||
| inert={hidden} | ||
| > | ||
| <div className="min-h-0 overflow-hidden"> | ||
| <SortableWorkspaceItem | ||
| sortableId={String(id)} | ||
| workspace={workspace} | ||
| accentColor={group?.color} | ||
| isInSection={isInSection} | ||
| onHoverCardOpen={onWorkspaceHover} | ||
| shortcutLabel={workspaceShortcutLabels.get( | ||
| parsed.realId, | ||
| )} | ||
| isSelected={ | ||
| canBulkSelect && isWorkspaceSelected(parsed.realId) | ||
| } | ||
| onSelectionClick={ | ||
| canBulkSelect | ||
| ? (event) => | ||
| selectWorkspaceFromEvent(event, { | ||
| workspaceId: parsed.realId, | ||
| projectId, | ||
| orderedWorkspaceIds: selectableWorkspaceIds, | ||
| }) | ||
| : undefined | ||
| } | ||
| disabled={ | ||
| hidden || | ||
| (workspace.type === "main" && | ||
| workspace.hostType === "local-device") | ||
| } | ||
| /> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Collapsed-section workspaces now cost a mounted row plus a status query each. The shared root cause is that hidden rows no longer unmount while the status provider enumerates every project child, so workspaces inside collapsed sections gain work they previously avoided entirely.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarExpandedProjectContent/DashboardSidebarExpandedProjectContent.tsx#L136-L183: keep conditional mounting for rows hidden byisInCollapsedSection, and apply the CSS grid-row transition only to theactiveType === "section"drag-hide case.apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsx#L175-L192: exclude workspaces in collapsed sections fromstatusWorkspacesso the provider does not open a bindings query and three bus subscriptions for rows the user cannot see.
📍 Affects 2 files
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarExpandedProjectContent/DashboardSidebarExpandedProjectContent.tsx#L136-L183(this comment)apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsx#L175-L192
🤖 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/components/DashboardSidebarProjectSection/components/DashboardSidebarExpandedProjectContent/DashboardSidebarExpandedProjectContent.tsx`
around lines 136 - 183, Collapsed-section workspaces should not remain mounted
or receive status subscriptions. In
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarExpandedProjectContent/DashboardSidebarExpandedProjectContent.tsx#L136-L183,
restore conditional mounting for rows hidden by isInCollapsedSection and apply
the CSS grid-row transition only to the activeType === "section" drag-hide case;
in
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsx#L175-L192,
exclude workspaces in collapsed sections from statusWorkspaces so they do not
receive provider queries or subscriptions.
There was a problem hiding this comment.
Real trade-off, acknowledged: collapsed-section rows now stay mounted (clipped) instead of unmounting, and the provider computes entries for them. Each mounted row is far cheaper than before (memoized body + per-row selector), and measured sweeps improved net, but large collapsed sections do pay a new baseline. Follow-up candidate: skip status-entry computation for rows inside collapsed sections, or render clipped rows as placeholders. Deferring rather than blocking — the current numbers are strictly better than main for typical sidebars.
There was a problem hiding this comment.
@Kitenite, acknowledged. The measured improvement supports deferring this optimization from the current PR.
The collapsed-section baseline cost remains a valid follow-up item. A future change can avoid status-entry work for collapsed rows or replace mounted rows with placeholders.
🐇
✏️ Learnings added
Learnt from: Kitenite
Repo: superset-sh/superset PR: 6377
File: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarExpandedProjectContent/DashboardSidebarExpandedProjectContent.tsx:136-183
Timestamp: 2026-08-12T06:58:12.303Z
Learning: In `apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar`, collapsed-section workspace rows remain mounted and clipped after the sidebar rendering optimization. The workspace-status provider also computes entries for these rows. This creates a baseline cost for large collapsed sections, but current measured hover-sweep performance is better than `main` for typical sidebars. Mitigation is intentionally deferred.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
| const isHovered = useDashboardSidebarIsHovered(id); | ||
| useEffect(() => { | ||
| if (isHovered && hostType === "local-device") onHoverCardOpen?.(); | ||
| }, [isHovered, hostType, onHoverCardOpen]); | ||
| // Fires on the committed hover only (hoveredId set after OPEN_DELAY or an | ||
| // open-card switch), never on transient row mouseenter. | ||
| if (isHovered && hostType === "local-device") void onHoverCardOpen?.(id); | ||
| }, [isHovered, hostType, onHoverCardOpen, id]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle rejection from the now-async onHoverCardOpen.
The prop type changed to void | Promise<void> and the caller passes refreshWorkspacePullRequest, which performs a network refresh. void discards the promise without a rejection handler. If the refresh rejects — offline host, GitHub API error — the renderer raises an unhandled promise rejection on every hover.
🐛 Proposed fix: attach a rejection handler
const isHovered = useDashboardSidebarIsHovered(id);
useEffect(() => {
// Fires on the committed hover only (hoveredId set after OPEN_DELAY or an
// open-card switch), never on transient row mouseenter.
- if (isHovered && hostType === "local-device") void onHoverCardOpen?.(id);
+ if (!isHovered || hostType !== "local-device") return;
+ Promise.resolve(onHoverCardOpen?.(id)).catch(() => {
+ // Hover-triggered refresh is best-effort; failures are non-fatal.
+ });
}, [isHovered, hostType, onHoverCardOpen, id]);📝 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.
| const isHovered = useDashboardSidebarIsHovered(id); | |
| useEffect(() => { | |
| if (isHovered && hostType === "local-device") onHoverCardOpen?.(); | |
| }, [isHovered, hostType, onHoverCardOpen]); | |
| // Fires on the committed hover only (hoveredId set after OPEN_DELAY or an | |
| // open-card switch), never on transient row mouseenter. | |
| if (isHovered && hostType === "local-device") void onHoverCardOpen?.(id); | |
| }, [isHovered, hostType, onHoverCardOpen, id]); | |
| const isHovered = useDashboardSidebarIsHovered(id); | |
| useEffect(() => { | |
| // Fires on the committed hover only (hoveredId set after OPEN_DELAY or an | |
| // open-card switch), never on transient row mouseenter. | |
| if (!isHovered || hostType !== "local-device") return; | |
| Promise.resolve(onHoverCardOpen?.(id)).catch(() => { | |
| // Hover-triggered refresh is best-effort; failures are non-fatal. | |
| }); | |
| }, [isHovered, hostType, onHoverCardOpen, id]); |
🤖 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/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx`
around lines 136 - 141, Update the useEffect invoking onHoverCardOpen so the
optional async callback’s promise has a rejection handler, preventing
refreshWorkspacePullRequest failures from becoming unhandled rejections.
Preserve the existing isHovered, hostType, and id conditions while handling
rejected promises without changing successful hover behavior.
There was a problem hiding this comment.
Valid — the hover-open handler is now async and the call site discards the promise; a host-offline rejection would surface as an unhandled rejection. Queued as a follow-up quick-win batch together with the cooldown-on-failure and map-prune notes below.
There was a problem hiding this comment.
@Kitenite, agreed. The rejection handler is required to prevent unhandled renderer promise rejections during hover-triggered refreshes.
The thread remains open until the follow-up batch includes the fix.
🐇
You are interacting with an AI system.
| const hasManualUnread = Boolean(manualUnread[target.workspaceId]); | ||
| let hasAttentionTerminal = false; | ||
| for (const status of statuses.values()) { | ||
| if (status === "review" || status === "failed") { | ||
| hasAttentionTerminal = true; | ||
| break; | ||
| } | ||
| } | ||
| const candidate: SidebarWorkspaceStatusEntry = { | ||
| status: getHighestPriorityStatus([ | ||
| hasManualUnread ? "review" : undefined, | ||
| ...statuses.values(), | ||
| ]), | ||
| isUnread: hasManualUnread || hasAttentionTerminal, | ||
| bindings, | ||
| statuses, | ||
| diffStats: | ||
| target.workspaceId === activeWorkspaceId ? activeDiffStats : null, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Compare unread derivation between the new provider and prior/other consumers.
set -euo pipefail
echo "== consumers of SidebarWorkspaceStatusEntry.isUnread =="
rg -n -C5 --type=tsx --type=ts 'useSidebarWorkspaceStatus' apps/desktop/src
echo "== all sites deriving unread from pane statuses =="
rg -n -C4 --type=ts --type=tsx '"permission"' apps/desktop/src/renderer | rg -n -C4 'review|failed|unread' || true
echo "== prior unread logic in the notification module =="
git log --oneline -5 -- apps/desktop/src/renderer/hooks/host-service/useV2NotificationStatus/useV2NotificationStatus.ts
git show HEAD~1:apps/desktop/src/renderer/hooks/host-service/useV2NotificationStatus/useV2NotificationStatus.ts 2>/dev/null | rg -n -C6 'isUnread|permission|review|failed' || trueRepository: superset-sh/superset
Length of output: 2058
🏁 Script executed:
#!/bin/bash
set -euo pipefail
base="https://api.github.com/repos/superset-sh/superset"
path="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx"
echo "== provider file =="
curl -fsSL "$base/contents/$path" |
python3 -c 'import sys,json,base64; print(base64.b64decode(json.load(sys.stdin)["content"]).decode())' |
nl -ba | sed -n '240,325p'
echo "== references to provider and isUnread =="
for q in \
"useSidebarWorkspaceStatus" \
"SidebarWorkspaceStatusEntry" \
"useV2AttentionWorkspaceCount" \
"isUnread"
do
echo "-- $q --"
curl -fsSLG "$base/search/code" --data-urlencode "q=$q repo:superset-sh/superset" |
python3 -c 'import sys,json; d=json.load(sys.stdin); [print(x["path"]) for x in d.get("items",[])]' || true
done
echo "== notification-status history =="
curl -fsSL "$base/commits?path=apps/desktop/src/renderer/hooks/host-service/useV2NotificationStatus/useV2NotificationStatus.ts&per_page=5" |
python3 -c 'import sys,json; [print(x["sha"], x["commit"]["message"].splitlines()[0]) for x in json.load(sys.stdin)]'
echo "== current attention-count references from repository tree search API =="
curl -fsSLG "$base/search/code" \
--data-urlencode "q=useV2AttentionWorkspaceCount repo:superset-sh/superset" |
python3 -c 'import sys,json; d=json.load(sys.stdin); [print(x["path"]) for x in d.get("items",[])]' || trueRepository: superset-sh/superset
Length of output: 2029
🌐 Web query:
site:github.com/superset-sh/superset "hasAttentionTerminal" "useV2AttentionWorkspaceCount"
💡 Result:
In the Superset desktop codebase, hasAttentionTerminal and useV2AttentionWorkspaceCount are related components used for managing workspace attention states and notifications [1]. useV2AttentionWorkspaceCount is a React hook that provides a count of distinct workspaces currently requiring user attention (v2 notifications) [1]. It is derived from the selector selectV2AttentionWorkspaceCount, which iterates over notification store sources to count workspace IDs with active notification states [1]. This hook is typically used in the authenticated layout to manage UI elements such as dock badges [1]. The term hasAttentionTerminal does not appear as a standard, standalone exported function or hook in the codebase. However, logic related to terminal attention is handled through notification store states and specific hooks like useClearActivePaneAttention, which clears attention markers when a terminal pane enters a review state [2]. Terminal pane status and ownership are managed via systems that link workspace IDs with notification store entries, ensuring that attention is cleared appropriately when a user interacts with a workspace or terminal pane [3][4].
Citations:
- 1: feat(desktop): OS dock badge for unread + attention-needed workspaces #5351
- 2: [codex] Refactor v2 workspace page #3747
- 3: [codex] Harden v2 workspace terminal pane ownership #4878
- 4: [codex] clear v2 ready notification on workspace click #3765
🌐 Web query:
site:github.com/superset-sh/superset "DashboardSidebarWorkspaceStatusProvider" "permission"
💡 Result:
The term DashboardSidebarWorkspaceStatusProvider is not a standard permission or formal API component identifier in the documented architecture of the referenced system (Superset/Desktop). Rather, the "permission" referenced in this context relates to a specific agent status level [1][2]. In the system's status architecture, workspace and pane status indicators are used to reflect agent lifecycle states. The "permission" state is one of four defined statuses for an agent: 1. permission: The agent is blocked and requires user intervention (displayed with a red indicator) [1][2]. 2. working: The agent is actively processing (amber) [1][2]. 3. review: The agent has completed a task and is ready for review (green) [1][2]. 4. idle: No active process or required action (default) [1][2]. These statuses are aggregated and displayed in the Dashboard Sidebar to provide users with visibility into workspace and agent activity [1][3]. The UI components, such as DashboardSidebarWorkspaceStatusBadge (or its successor patterns) and various StatusIndicator components, consume these statuses to render the correct UI state [1][3][4]. The system manages these statuses through a store (often Zustand) that tracks agent lifecycle events, which are then exposed via tRPC subscriptions or mutations (such as clearWorkspaceStatuses) to the renderer [1][5]. If you are encountering a "permission" status, it indicates that an automated agent has reached a point where it is waiting for your authorization to continue [1][2].
Citations:
- 1: feat(desktop): add 3-color workspace status indicators #588
- 2: https://github.com/superset-sh/superset/blob/2f2c2059/apps/desktop/src/shared/tabs-types.ts
- 3: feat(desktop): running agents section in workspace sidebar #5414
- 4: feat(desktop): show PR state as sidebar workspace icon #3694
- 5: fix(desktop,host-service): make Clear Status clear wedged agent working state #5744
🌐 Web query:
site:github.com/superset-sh/superset "useV2NotificationStatus" "isUnread"
💡 Result:
In the context of the superset-sh/superset desktop codebase, useV2NotificationStatus is a React hook used to manage and derive the notification status for a specific workspace [1]. It is primarily located within the host-service renderer hooks (typically at apps/desktop/src/renderer/hooks/host-service/useV2NotificationStatus/useV2NotificationStatus.ts) [1][2]. The isUnread property (often accessed via related hooks or selectors like useV2WorkspaceIsUnread or selectV2WorkspaceIsUnread) represents whether a workspace has unread notifications or requires user attention [3][4]. This state is derived by combining data from terminal agent statuses (via useTerminalAgentStatuses) and manual unread flags (manualUnread) [1]. Key aspects of this system include: - Derived Status: The hook aggregates various agent lifecycle states—such as working, permission-needed, review-needed, and failed states—to determine if a workspace is in an active or "unread" state [1][5][4]. - Integration: It acts as a single source of truth for workspace notification status, replacing older store-selector functions to provide a more consistent UI experience across sidebar items, dock badges, and status indicators [1][4]. - Manual State: Users can manually toggle the "unread" status for a workspace, which is persisted in the v2-notifications Zustand store [3]. - Functionality: The hook is often used in components like DashboardSidebarWorkspaceItem to handle visual indicators (e.g., badges) and user interactions (e.g., "Mark as Read"/"Mark as Unread" functionality) [3].
Citations:
- 1: fix(desktop): derive terminal agent status from host bindings (single source of truth) #5449
- 2: desktop-v1.13.0...desktop-v1.13.1
- 3: Add v2 mark workspace as unread #3773
- 4: feat(desktop): OS dock badge for unread + attention-needed workspaces #5351
- 5: feat(desktop,host-service): surface agent failures as a "failed" status #5697
🌐 Web query:
site:github.com/superset-sh/superset/pull/5351 "permission" "isUnread"
💡 Result:
There is no standard or widely recognized permission named isUnread. In technical contexts, isUnread is typically used as a boolean attribute or state flag in notification systems, email clients, or messaging applications to indicate whether a message or workspace item has been viewed by the user [1]. In software development, such as in the implementation of desktop notification stores, isUnread is often grouped with other status indicators—such as permission-related requirements or review needs—to manage user attention and UI elements like dock badges [1]. It is not a permission itself, but rather a status flag used to track user interaction state [1].
Citations:
🌐 Web query:
site:github.com/superset-sh/superset/pull/5449 "permission" "isUnread"
💡 Result:
There is no standard software permission known as isUnread. The term isUnread typically refers to a boolean property or state attribute used in software development to track whether a user or system has viewed a specific item, such as a message, notification, or workspace [1]. In technical implementations, isUnread is commonly used to manage UI states, such as bolding unread items or displaying notification badges [1]. It is distinct from a permission, which defines whether a user has the authorization to perform an action [1]. In some development contexts, developers may create custom hooks or state management functions—such as useV2WorkspaceIsUnread—to derive this status from underlying data bindings [1].
Citations:
🌐 Web query:
site:github.com/superset-sh/superset/pull/5697 "permission" "unread"
💡 Result:
The term permission unread does not refer to a standard technical protocol or universal computer system state, but rather appears as a specific UI/UX status implementation within certain software environments, such as Claude Code [1]. In this context, permission is a status indicator—often represented as a red pulsing dot—that denotes a state where an agent or process requires user attention or authorization [1]. This status is integrated into the application's attention management system, meaning that it contributes to the UI's unread or dock-badge counts, signaling to the user that there is an actionable event or a pending permission request that needs to be addressed [1].
Citations:
🌐 Web query:
site:github.com/superset-sh/superset/pull/3773 "permission" "unread"
💡 Result:
There is no standard technical definition for a permission labeled as unread in computer systems or cybersecurity. In software development, the term unread typically refers to a status flag or UI state for messages, notifications, or workspaces rather than an access control permission [1]. For example, in application development, mark as unread features allow users to manually toggle the read or unread status of an item [1]. This functionality is managed through application logic—such as notification stores or database flags—to trigger UI elements like indicators or notification dots, and is not a security permission that governs access to data [1]. If you encounter the term in a specific software context, it likely refers to a user interface feature for tracking interaction status rather than a system-level permission [1].
Citations:
🌐 Web query:
"hasAttentionTerminal" superset-sh/superset
💡 Result:
The term hasAttentionTerminal does not appear to be a standard or public identifier associated with the Apache Superset project (apache/superset) [1][2]. The query appears to refer to the "superset-sh/superset" project, which is a distinct, separate tool [3]. Superset.sh describes itself as a platform for AI-powered coding and agent-based workflows, featuring built-in terminal capabilities and IDE-like functionality [3][4]. The repository for this project contains internal code related to terminal management (e.g., terminal.ts) and agent bindings [5][6], but hasAttentionTerminal is not a recognized public API or documented feature in that repository. It is likely an internal variable, function name, or transient reference within the source code of that specific project.
Citations:
- 1: https://github.com/apache/superset/
- 2: https://apache.googlesource.com/superset/+/refs/heads/master/AGENTS.md
- 3: https://github.com/superset-sh/superset?aid=rectENAk8IKmIXwJe
- 4: https://superset-sh-superset.mintlify.app/guides/running-agents
- 5: https://github.com/superset-sh/superset/blob/2f2c2059/apps/desktop/src/lib/trpc/routers/terminal/terminal.ts
- 6: feat(host-service): terminalAgents tracker + pane wiring #4901
🌐 Web query:
"selectV2AttentionWorkspaceCount" superset-sh/superset
💡 Result:
The selectV2AttentionWorkspaceCount selector is a function defined within the v2-notifications store of the superset-sh/superset desktop application [1]. Its purpose is to calculate the number of distinct workspaces that currently have active notifications requiring user attention [1]. Specifically, the selector iterates through the state of notification sources and counts all unique workspaceId values present in the store [1]. Because every entry in this specific notification store is tracked as an active status, this count effectively represents the total number of workspaces that have any live notifications (such as those requiring review) [1]. This selector is primarily used in the desktop application's authenticated layout to manage UI elements like dock badges, which provide users with a quick visual indicator of how many workspaces require their attention [1].
Citations:
🌐 Web query:
"hasAttentionTerminal" "permission"
💡 Result:
There is no standard or widely recognized permission named hasAttentionTerminal. Research into technical contexts where this term might appear indicates it is likely a misremembered or conflated reference to concepts found in specific agentic AI frameworks, terminal UI management, or Android service development: 1. Agentic AI & Terminal UI Systems: Various terminal-based AI projects (e.g., agent-terminal, termic) use terms like "attention" to manage states where an agent is blocked and waiting for user input [1][2][3]. These systems often implement signals (like "attention" or "PermissionRequest") to notify the user when the terminal requires action, but these are application-level state management patterns rather than standard OS or API permissions [1][2][4][5]. 2. Android Attention Service: Android includes an Attention Service API (e.g., BIND_ATTENTION_SERVICE, ACCESS_ATTENTION_LISTENER) used for managing device-level attention sensing (like gaze tracking or detecting user presence) [6][7][8]. This is a specialized system-level permission, not a terminal-specific one [6][8]. 3. Node.js Permissions: Node.js has a built-in Permission Model with a process.permission.has method, but it is used to check access to system resources like the file system, network, or child processes, not for "attention terminals" [9]. If you encountered this term in a specific codebase, it is likely a custom-defined property or function name within that project's internal API [2][10][4].
Citations:
- 1: DaniAkash/agent-terminal@cabdaaa
- 2: simion/termic@9c22b73
- 3: https://github.com/getpaseo/paseo/blob/860fcb2e/packages/server/src/server/agent-attention-policy.ts
- 4: https://github.com/crynta/terax-ai/blob/460657aa/src/modules/agents/components/AgentNotificationsBridge.tsx
- 5: https://github.com/ZipLyne-Agency/CodeGrid/blob/main/src/hooks/useAttention.ts
- 6: https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/service/attention/AttentionService.java
- 7: https://android.googlesource.com/platform/frameworks/base/+/3e3ce44105fb21bc1b993961996acd8a689b0435/services/core/java/com/android/server/attention/AttentionManagerService.java
- 8: https://gitlab.e.foundation/e/os/android_frameworks_base/-/commit/3dfbb80d4353715d4b43401ce518459d2340f141
- 9: http://nodejs.org/dist/latest/docs/api/permissions.html
- 10: https://github.com/getpaseo/paseo/blob/860fcb2e/packages/app/e2e/helpers/terminal-dsl.ts
🌐 Web query:
"statuses.values()" "isUnread" superset
💡 Result:
The query "statuses.values isUnread superset" appears to conflate distinct technical concepts from different software contexts rather than referring to a singular API or function. 1. Superset (Superset.sh): In this platform, workspaces have an isUnread property (a boolean) stored in a SQLite database to track whether a workspace contains notifications requiring attention [1][2][3]. While the SDK has a method tasks.statuses.list [4], there is no standard API command statuses.values that interacts with isUnread in this context [4][1]. 2. Programming/Data Modeling: There is no standard computer science or language-specific function called statuses.values.isUnread. The term "superset" often refers to a set that contains all elements of another set, but it does not describe a common relationship between a values function and an isUnread property [5][6][7]. 3. Other Contexts: Systems like WPPConnect (WhatsApp JS) use status models (StatusV3Model) [6][8] and Gmail Apps Script uses an isUnread method [9], but these are distinct from the structures mentioned in your query. If you are encountering an error or trying to achieve a specific result (such as filtering a list of statuses), it is likely that statuses.values refers to an iterable collection of objects where you need to check if any item possesses an isUnread flag, or you are working with a data structure where one category of status is a superset of "unread" items. Please provide more context or the specific programming language/library if you need a code-based solution.
Citations:
- 1: https://mintlify.wiki/superset-sh/superset/concepts/workspaces
- 2: https://superset-sh-superset.mintlify.app/concepts/workspaces
- 3: feat(desktop): add Workbench/Review mode + group tabs + workspace navigation sidebar #586
- 4: https://docs.superset.sh/sdk/reference
- 5: Add v2 mark workspace as unread #3773
- 6: https://wppconnect.io/wa-js/classes/whatsapp.StatusV3Model.html
- 7: https://tigerconnect.github.io/js-sdk/quickstart/js/messages/unread-counts/
- 8: https://github.com/wppconnect-team/wa-js/blob/main/src/whatsapp/models/StatusV3Model.ts
- 9: https://developers.google.com/apps-script/reference/gmail/gmail-message
🌐 Web query:
site:github.com/superset-sh/superset/pull/5351 "Dashboard Sidebar" notifications
💡 Result:
Dashboard sidebar notifications are a UI design pattern used to alert users to new, unread, or attention-required items within specific sections of a web application. By surfacing notification counts directly within the navigation sidebar, developers can provide immediate visibility into system states without requiring the user to navigate away from their current view [1]. Common implementation patterns for sidebar notifications include: Notification Badges: These are small, visual indicators (often numeric counters or dots) placed on sidebar menu items to represent the number of pending actions or unread messages within that specific workspace or category [1]. State Management: Systems often utilize a centralized notification store to track statuses across different application areas. For example, a dashboard might track distinct workspaces that require attention, such as those needing user review or containing unread content, while filtering out items where background processes are still in a working state [1]. Cross-Platform Integration: In desktop applications, sidebar notification data can be mirrored to the operating system level, such as displaying a badge on the taskbar or dock icon. This is typically achieved by mounting a controller in the application layout that observes the state store and communicates with the underlying OS APIs via inter-process communication (IPC) [1]. To manage these notifications effectively, applications often employ controllers mounted at the authenticated layout level to ensure global state synchronization and proper cleanup of notification indicators upon user sign-out [1].
Citations:
🌐 Web query:
site:github.com/superset-sh/superset/pull/5351 "review" "failed" "permission"
💡 Result:
The phrase review failed permission does not refer to a standard, universally defined error message in computing or software development. Based on the available documentation, the terms review and permission appear as distinct status categories within specific application workflows, such as agent-based workspace management [1]. In these contexts, they often signify different states requiring human attention: review typically indicates a task or output is ready for verification, while permission refers to a state where an agent or process is blocked and requires user authorization to proceed [1]. If you are encountering this sequence as an error, it likely represents a compound status indicating that a process failed because it reached a review stage but could not proceed due to a lack of necessary permissions.
Citations:
🌐 Web query:
site:github.com/superset-sh/superset/pull/5697 "selectV2AttentionWorkspaceCount"
💡 Result:
No indexed results were found for:
site:github.com/superset-sh/superset/pull/5697 "selectV2AttentionWorkspaceCount"
The pull request or identifier may be private, removed, renamed, or not indexed by search.
🌐 Web query:
site:github.com/superset-sh/superset/pull/3747 "permission" "review" "failed"
💡 Result:
PR #3747 is “[codex] Refactor v2 workspace page”, merged on April 26, 2026. (github.com)
- Review: CodeRabbit identified 2 actionable comments and 1 nitpick.
- Failed check: Docstring Coverage — 0%, below the required 80% threshold.
- Critical issue: A dirty-tab close
Promisemay never resolve if the alert is dismissed without selecting an action. - Other issue:
openFilePanemay store relative paths ifworktreePathhas not loaded yet. - No apparent permission failure was shown on the PR page.
Citations:
Include permission in hasAttentionTerminal. A workspace with only a permission terminal contributes to useV2AttentionWorkspaceCount but currently reports isUnread: false, which makes the row state inconsistent with the attention badge.
🤖 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/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx`
around lines 287 - 305, Update the hasAttentionTerminal check in
DashboardSidebarWorkspaceStatusProvider to treat the "permission" status as an
attention terminal alongside "review" and "failed". Preserve the existing
isUnread calculation and status priority behavior.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 26 files
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx:244">
P2: Git changes for the active workspace now invalidate the same diff-stats query twice: this sidebar-wide listener runs alongside the `useDiffStats(activeWorkspaceId)` listener. Skipping the active workspace here while retaining listeners for inactive targets would avoid redundant refetch work and possible cancellation/restart of the active request.</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx:288">
P2: `hasAttentionTerminal` only checks for `"review"` or `"failed"` statuses, but `useV2AttentionWorkspaceCount` (via `getHighestPriorityStatus`) treats a `permission` status as needing attention too. A workspace with only a `permission` terminal will increment the OS dock badge count while its sidebar row reports `isUnread: false`, leaving the row's visual state inconsistent with the attention indicator.</violation>
</file>
<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:451">
P2: The gate records the cooldown timestamp on shouldRefresh() before refreshByWorkspaces.mutate runs, so a failed or rejected refresh still suppresses further hover-triggered refreshes for the full 45s window — the workspace's stale PR chip can't recover via hover and only polling (10s) updates it. Move the timestamp recording to after a successful mutate, or clear it on failure so a transient host-service error doesn't block retries.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/SortableWorkspaceItem/SortableWorkspaceItem.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/SortableWorkspaceItem/SortableWorkspaceItem.tsx:13">
P2: Type now permits async handlers (Promise<void>) but the consuming call site discards the promise without a catch (`void onHoverCardOpen?.(id)` in DashboardSidebarWorkspaceItem.tsx:140). The actual handler `refreshWorkspacePullRequest` awaits `pullRequests.refreshByWorkspaces.mutate` and `queryClient.invalidateQueries`, both of which can reject on network/API failure, so a rejected refresh becomes an unhandled promise rejection (crash risk in Electron). Either keep the callback synchronous or await/catch at the call site.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/pullRequestRefreshCooldown.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/pullRequestRefreshCooldown.ts:20">
P3: The module-level cooldown map grows permanently with every distinct workspace hovered, even after that workspace is removed. Pruning entries once their cooldown expires (or using a bounded cache) would avoid an unbounded renderer-lifetime memory leak.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Sidebar as DashboardSidebar
participant StatusProv as SidebarWorkspaceStatusProvider
participant StatusStore as StatusStore (per-workspace)
participant Rows as SidebarWorkspaceItem (rows)
participant HoverProv as HoverProvider
participant DataHook as useDashboardSidebarData
participant Gate as PullRequestRefreshGate
participant HostClient as Host Service Client
participant QueryCache as React Query Cache
participant EventBus as Workspace EventBus
Note over Sidebar,Rows: NEW: Sidebar-level status subscription replaces ~80 per-row query subscriptions
Sidebar->>StatusProv: Provide workspaces (pinned + sessions + projects) + activeWorkspaceId
StatusProv->>QueryCache: useQueries - terminalAgentBindings per workspace (once)
StatusProv->>EventBus: Subscribe agent:lifecycle, terminal:lifecycle, git:changed (per workspace)
StatusProv->>QueryCache: useDiffStats (only for active workspace)
loop Every render/poll tick
QueryCache-->>StatusProv: Bindings + diff stats data
StatusProv->>StatusProv: Compute status entries (status, isUnread, bindings, statuses, diffStats)
StatusProv-->>StatusStore: replaceEntries(next) - only changed entries queued
end
StatusStore->>StatusStore: flushNotifications() (post-commit)
StatusStore-->>Rows: NEW: Notify only rows whose entry changed
Rows->>StatusStore: useSidebarWorkspaceStatus(id) - subscribe to own entry
Note over Rows,HoverProv: CHANGED: Hover state split - actions context stable, hovered-id via subscription
Rows->>HoverProv: useDashboardSidebarHoverActions() (stable - no re-renders)
Rows->>HoverProv: useDashboardSidebarIsHovered(id) (per-row subscription)
HoverProv-->>Rows: Notify only 2 affected rows on hover commit
Note over Rows,DataHook: CHANGED: Hover-open PR refresh gets 45s cooldown
Rows->>DataHook: onHoverCardOpen(workspaceId)
DataHook->>Gate: shouldRefresh(workspaceId, now)
alt Within cooldown (< 45s since last refresh)
Gate-->>DataHook: false (skip refresh)
DataHook-->>Rows: No host call
else Cooldown expired
Gate-->>DataHook: true (record timestamp)
DataHook->>HostClient: pullRequests.refreshByWorkspaces.mutate()
HostClient-->>DataHook: Mutation result
DataHook->>QueryCache: Invalidate PR queries for workspace
QueryCache-->>Rows: Updated PR data
end
Note over Sidebar,QueryCache: CHANGED: Per-row framer-motion replaced with CSS grid-row collapse
Sidebar->>Sidebar: CSS transition (grid-rows-[0fr/1fr], opacity) for collapsed rows
Sidebar->>Sidebar: Rows stay mounted but inert when hidden
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ); | ||
| } | ||
| const hasManualUnread = Boolean(manualUnread[target.workspaceId]); | ||
| let hasAttentionTerminal = false; |
There was a problem hiding this comment.
P2: hasAttentionTerminal only checks for "review" or "failed" statuses, but useV2AttentionWorkspaceCount (via getHighestPriorityStatus) treats a permission status as needing attention too. A workspace with only a permission terminal will increment the OS dock badge count while its sidebar row reports isUnread: false, leaving the row's visual state inconsistent with the attention indicator.
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/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx, line 288:
<comment>`hasAttentionTerminal` only checks for `"review"` or `"failed"` statuses, but `useV2AttentionWorkspaceCount` (via `getHighestPriorityStatus`) treats a `permission` status as needing attention too. A workspace with only a `permission` terminal will increment the OS dock badge count while its sidebar row reports `isUnread: false`, leaving the row's visual state inconsistent with the attention indicator.</comment>
<file context>
@@ -0,0 +1,382 @@
+ );
+ }
+ const hasManualUnread = Boolean(manualUnread[target.workspaceId]);
+ let hasAttentionTerminal = false;
+ for (const status of statuses.values()) {
+ if (status === "review" || status === "failed") {
</file context>
There was a problem hiding this comment.
Not a regression — the pre-PR row-level useV2WorkspaceIsUnread used exactly review || failed (see bb3daa4, useV2NotificationStatus.ts:84); the provider preserves it. permission was only ever counted by useV2AttentionWorkspaceCount (dock badge), which is intentionally broader — permission shows as the red status dot, not the unread mark. Pre-existing split between two metrics; happy to align them in a follow-up if we want them unified.
| (candidate) => candidate.machineId === workspace.hostId, | ||
| ); | ||
| if (!target?.hostUrl) return; | ||
| if (!pullRequestRefreshGate.shouldRefresh(workspaceId, Date.now())) { |
There was a problem hiding this comment.
P2: The gate records the cooldown timestamp on shouldRefresh() before refreshByWorkspaces.mutate runs, so a failed or rejected refresh still suppresses further hover-triggered refreshes for the full 45s window — the workspace's stale PR chip can't recover via hover and only polling (10s) updates it. Move the timestamp recording to after a successful mutate, or clear it on failure so a transient host-service error doesn't block retries.
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 451:
<comment>The gate records the cooldown timestamp on shouldRefresh() before refreshByWorkspaces.mutate runs, so a failed or rejected refresh still suppresses further hover-triggered refreshes for the full 45s window — the workspace's stale PR chip can't recover via hover and only polling (10s) updates it. Move the timestamp recording to after a successful mutate, or clear it on failure so a transient host-service error doesn't block retries.</comment>
<file context>
@@ -444,6 +448,9 @@ export function useDashboardSidebarData() {
(candidate) => candidate.machineId === workspace.hostId,
);
if (!target?.hostUrl) return;
+ if (!pullRequestRefreshGate.shouldRefresh(workspaceId, Date.now())) {
+ return;
+ }
</file context>
There was a problem hiding this comment.
Valid but low-impact by design: the 10s PR poll is the freshness backstop, so a failed hover-refresh being suppressed for one 45s window only delays the hover-triggered fast path, not correctness. Will move the timestamp write to post-success in the follow-up batch.
| accentColor?: string | null; | ||
| isInSection?: boolean; | ||
| onHoverCardOpen?: () => void; | ||
| onHoverCardOpen?: (workspaceId: string) => void | Promise<void>; |
There was a problem hiding this comment.
P2: Type now permits async handlers (Promise) but the consuming call site discards the promise without a catch (void onHoverCardOpen?.(id) in DashboardSidebarWorkspaceItem.tsx:140). The actual handler refreshWorkspacePullRequest awaits pullRequests.refreshByWorkspaces.mutate and queryClient.invalidateQueries, both of which can reject on network/API failure, so a rejected refresh becomes an unhandled promise rejection (crash risk in Electron). Either keep the callback synchronous or await/catch at the call site.
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/SortableWorkspaceItem/SortableWorkspaceItem.tsx, line 13:
<comment>Type now permits async handlers (Promise<void>) but the consuming call site discards the promise without a catch (`void onHoverCardOpen?.(id)` in DashboardSidebarWorkspaceItem.tsx:140). The actual handler `refreshWorkspacePullRequest` awaits `pullRequests.refreshByWorkspaces.mutate` and `queryClient.invalidateQueries`, both of which can reject on network/API failure, so a rejected refresh becomes an unhandled promise rejection (crash risk in Electron). Either keep the callback synchronous or await/catch at the call site.</comment>
<file context>
@@ -10,7 +10,7 @@ interface SortableWorkspaceItemProps {
accentColor?: string | null;
isInSection?: boolean;
- onHoverCardOpen?: () => void;
+ onHoverCardOpen?: (workspaceId: string) => void | Promise<void>;
shortcutLabel?: string;
disabled?: boolean;
</file context>
There was a problem hiding this comment.
Same as CodeRabbit's note on the item — folding into the follow-up quick-win batch (catch on the discarded promise).
| bus.on("terminal:lifecycle", workspaceId, invalidateBindings), | ||
| ); | ||
| cleanups.push( | ||
| bus.on("git:changed", workspaceId, () => { |
There was a problem hiding this comment.
P2: Git changes for the active workspace now invalidate the same diff-stats query twice: this sidebar-wide listener runs alongside the useDiffStats(activeWorkspaceId) listener. Skipping the active workspace here while retaining listeners for inactive targets would avoid redundant refetch work and possible cancellation/restart of the active request.
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/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx, line 244:
<comment>Git changes for the active workspace now invalidate the same diff-stats query twice: this sidebar-wide listener runs alongside the `useDiffStats(activeWorkspaceId)` listener. Skipping the active workspace here while retaining listeners for inactive targets would avoid redundant refetch work and possible cancellation/restart of the active request.</comment>
<file context>
@@ -0,0 +1,382 @@
+ bus.on("terminal:lifecycle", workspaceId, invalidateBindings),
+ );
+ cleanups.push(
+ bus.on("git:changed", workspaceId, () => {
+ void queryClient.invalidateQueries({
+ queryKey: getDiffStatsQueryKey(hostUrl, workspaceId),
</file context>
There was a problem hiding this comment.
Valid micro-dedup — will skip the active workspace in the sidebar-wide invalidation listener in the follow-up batch.
| export function createPullRequestRefreshGate( | ||
| cooldownMs: number = PULL_REQUEST_REFRESH_COOLDOWN_MS, | ||
| ): PullRequestRefreshGate { | ||
| const lastRefreshAtByWorkspaceId = new Map<string, number>(); |
There was a problem hiding this comment.
P3: The module-level cooldown map grows permanently with every distinct workspace hovered, even after that workspace is removed. Pruning entries once their cooldown expires (or using a bounded cache) would avoid an unbounded renderer-lifetime memory leak.
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/pullRequestRefreshCooldown.ts, line 20:
<comment>The module-level cooldown map grows permanently with every distinct workspace hovered, even after that workspace is removed. Pruning entries once their cooldown expires (or using a bounded cache) would avoid an unbounded renderer-lifetime memory leak.</comment>
<file context>
@@ -0,0 +1,31 @@
+export function createPullRequestRefreshGate(
+ cooldownMs: number = PULL_REQUEST_REFRESH_COOLDOWN_MS,
+): PullRequestRefreshGate {
+ const lastRefreshAtByWorkspaceId = new Map<string, number>();
+ return {
+ shouldRefresh(workspaceId, now) {
</file context>
There was a problem hiding this comment.
Valid — the map is tiny (8-byte timestamp per distinct hovered workspace per session) but pruning expired entries is a 3-liner; follow-up batch.
CDP stress battery on this branch (same methodology as #6364's)All phases driven with trusted Verified:
|
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…t list (#6514) * feat(desktop): make sidebar groups reorderable anywhere in the project list Groups were stuck at the bottom: section drags could only sort against other section headers, the drop handler rebuilt the list as "ungrouped first, then sections", and membership was positional — any ungrouped row rendered below a header was absorbed into that group, so a group above an ungrouped row was unrepresentable. Membership is now explicit in the DnD state (flat ws id -> flat section id). Section drags reorder top-level units (header + members move as one) against ungrouped rows; a dropped workspace joins the group of the row that ends up directly above it; in-place drops restore the pre-drag membership. The builder no longer absorbs trailing ungrouped rows, so interleaved orders round-trip through tabOrder persistence. Drag-feel fixes this surfaced: - The dnd translate lived inside each row's overflow-hidden collapse wrapper (from #6377), so displaced rows translated out of their clip box and vanished mid-drag — including during plain workspace drags. The wrapper now lives inside SortableWorkspaceItem with the transform on the outermost element. - A section pickup collapses the member rows; the shrink clamps scrollTop when scrolled near the bottom and desyncs dnd-kit's cached rects and overlay transform (drops landed rows away from the pointer, ghost drifted off the cursor). Fixed with an instant drag-collapse, a live-DOM pointer collision for section drags, a hidden in-list header during the drag (the overlay ghost is the representation), and a SectionDragSpacer that keeps scrollHeight constant while members are hidden. * fix(desktop): keep rows in place when ungrouping or deleting a group deleteSection and moveWorkspaceToSection(null) inserted rows "before the first section", which was only correct while groups clustered at the bottom. With groups placeable anywhere, both would teleport rows above the first group in the project. Deleting a group now replaces its own top-level slot with its members, and Ungroup lands the row directly below its former group (no-op when already ungrouped in the same project; cross-project moves keep the old fallback). * fix(desktop): multiselect UX — sticky bulk toolbar, ordered ungroup, prune hidden selections Three multiselect issues surfaced by end-to-end testing: - The bulk-actions toolbar replaces the Workspaces header inside the sidebar scroller, so selecting rows at the bottom of a long list left it scrolled ~1400px out of view — selections appeared to have no actions at all. It is now sticky at the scroller top with a blurred backdrop. - Bulk Ungroup processed rows in visual order while each row inserts at the same slot below its former group, stacking the results reversed. Iterate back-to-front so the selection keeps its order. - Members of a collapsed group stayed selected while hidden, leaving invisible rows armed for bulk actions (including Delete). Collapsing a group now prunes its rows from the selection via availableWorkspaceIds. * refactor(desktop): drop the dead hidden-selection preservation path Since collapsing a group prunes its rows from the selection (via availableWorkspaceIds), the selection is always a subset of the visible ordered list — the preserveIds merge in selectedIdsInVisualOrder could no longer be reached outside a single-commit race. Reduce it to an ordered filter and replace the three preserve-hidden tests with the new invariant: stale ids are dropped, and an unlisted anchor re-anchors the next additive click. * fix(desktop): snappier sidebar drag pickup, no navigation on cancelled drags Lower the MouseSensor activation distance from 8px to 5px — enough to absorb the 1-3px of jitter a real click carries, less dead travel before the drag engages. The trailing click after an activated drag is already swallowed by dnd-kit itself (capture-phase document click listener armed at activation, detached one event loop after the drag ends), verified end-to-end: in-row drags don't navigate and header drags don't toggle collapse. The one case dnd-kit misses is an Escape-cancel with the button still held: its listener detaches ~50ms after the cancel, and the click fires on the later release — navigating the row under the cursor. Mirror dnd-kit's own technique for that case in onDragCancel: swallow the next click, disarming right after the release so subsequent clicks work. * refactor(desktop): stock closestCenter for section drags The custom live-DOM pointer collision existed because a section pickup used to clamp the scroller's scrollTop, desyncing dnd-kit's rect store and scroll compensation. SectionDragSpacer has since removed the clamp at the source and the drag-collapse is instant, so the rect store stays accurate — dnd-kit's recommended-for-sortables closestCenter (filtered to the active container) now tracks 1:1, verified at parked positions (exactly N rows displaced at N slots, ghost pixel-exact on the pointer) and on exact drops in both directions.
Stacked follow-up to #6364 (sidebar pin drag-and-drop). Addresses the three pre-existing sidebar rendering costs identified while profiling that work.
Summary
pullRequestRefreshCooldown, unit-tested): opening a row's hover card no longer fires an unthrottled hostpullRequests.refreshByWorkspacesmutation + query invalidation per hover.DashboardSidebarWorkspaceStatusProviderholds a single sidebar-level subscription with per-row selectors (a row re-renders only when its entry changes); the hover provider is split so rows consume a stable actions context instead of churning state.Why / Context
CPU profiles during the #6364 work showed a plain mouse sweep across sidebar rows costing ~358ms of long tasks (worst 129ms) with
shallowEqualObjects/useBaseQueryhot — hover-open side effects, per-row react-query subscriptions notified on every 10s PR poll, and ~80 motion components.How It Works
DashboardSidebarWorkspaceStatusProvidersubscribes once and exposes selector hooks; poll ticks no longer run ~80 per-row subscription compare passes.Verification
Authored by an orchestrated worker agent; independently re-verified by the coordinator, then rebased onto post-#6364 main with one conflict reconciled (
DashboardSidebarDndProvider: kept the actions-context split and the try/finally suppression release + fire-time open-timer re-check from 1aab78f — both suppression guards confirmed present after the merge).bun run typecheck,bun run lint(exit 0, no post-lint diff), 92/92 tests across the sidebar + state + host-service-hook suites — run on the rebased branch.Manual QA for reviewer (dev dataset couldn't exercise these)
Risks / Rollback
Summary by cubic
Reduce sidebar render fan-out on hover and polling to cut long-task time during a hover sweep by ~50% and limit re-renders to only affected rows.
pullRequests.refreshByWorkspacescalls per hover (unit-tested).DashboardSidebarWorkspaceStatusProviderto replace ~80 per-row query subscriptions with one sidebar-level subscription + per-row selectors; also centralizes lifecycle event listeners and diff-stats invalidation.isHoveredsubscription so a hover commit re-renders only the two involved rows.Written for commit 99e71a6. Summary will update on new commits.
Summary by CodeRabbit