Repository navigation
perf(desktop): only query diff stats for the active sidebar workspace item - #6098
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesDiff statistics query flow
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant DashboardSidebarWorkspaceItem
participant useDiffStats
participant WorkspaceEvents
participant DiffStatsQuery
DashboardSidebarWorkspaceItem->>useDiffStats: Pass active-row enable state
useDiffStats->>DiffStatsQuery: Run query when enabled
WorkspaceEvents->>useDiffStats: Notify workspace changes
useDiffStats->>DiffStatsQuery: Invalidate cached statistics
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx:82">
P3: The optimization skips the git status query for inactive rows, but the active row still fires the `git.getStatus` query even when the sidebar is collapsed, where the row only renders the collapsed button and never shows the +/− counts. So there's still one wasted background read in a collapsed sidebar. Consider gating on the collapsed state as well so the fetch only happens when the expanded active row can actually render the badge.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Item as "DashboardSidebarWorkspaceItem"
participant Hook as "useDiffStats"
participant Query as "QueryClient (React Query)"
participant Git as "Host Service (git.getStatus)"
participant Bus as "Shared git:changed Event Bus"
Note over Item,Bus: Default enabled: true (hover-card and other callers unaffected)
Note over Item,Bus: sidebar item passes enabled: isActive
Item->>Hook: useDiffStats(id, { enabled: isActive })
Hook->>Bus: subscribe("git:changed", invalidate)
Hook->>Query: useQuery({ queryKey, enabled })
alt isActive = true
Query->>Git: git.getStatus.query({ workspaceId })
Git-->>Query: DiffStats { additions, deletions }
Query-->>Hook: diff stats (cached)
Hook-->>Item: render +N/-M badge
else isActive = false
Note over Query,Git: query disabled - no git.getStatus call fired
end
opt git:changed event arrives for this workspace
Bus->>Hook: invalidation callback (subscription kept while disabled)
Hook->>Query: invalidateQueries({ queryKey })
alt item is currently active
Query->>Git: refetch and update stats
else item is inactive
Note over Query: stats marked stale (staleTime: Infinity)
Note over Query: refetched when item becomes active
end
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| // Only the active workspace row shows line counts, so skip the per-item | ||
| // git status query everywhere else. | ||
| const diffStats = useDiffStats(id, { enabled: isActive }); |
Contributor
There was a problem hiding this comment.
P3: The optimization skips the git status query for inactive rows, but the active row still fires the git.getStatus query even when the sidebar is collapsed, where the row only renders the collapsed button and never shows the +/− counts. So there's still one wasted background read in a collapsed sidebar. Consider gating on the collapsed state as well so the fetch only happens when the expanded active row can actually render the badge.
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/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx, line 82:
<comment>The optimization skips the git status query for inactive rows, but the active row still fires the `git.getStatus` query even when the sidebar is collapsed, where the row only renders the collapsed button and never shows the +/− counts. So there's still one wasted background read in a collapsed sidebar. Consider gating on the collapsed state as well so the fetch only happens when the expanded active row can actually render the badge.</comment>
<file context>
@@ -78,6 +77,10 @@ export function DashboardSidebarWorkspaceItem({
+ // Only the active workspace row shows line counts, so skip the per-item
+ // git status query everywhere else.
+ const diffStats = useDiffStats(id, { enabled: isActive });
+
const { v2Workspaces: v2WorkspaceActions } = useOptimisticCollectionActions();
</file context>
Member
Author
There was a problem hiding this comment.
Intentional: the collapsed active item's query shares its cache key with the hover-card overlay and keeps the badge instant when the sidebar expands, so it's one warm read rather than waste. The N-per-sidebar reads were the problem this PR targets.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
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
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
Every
DashboardSidebarWorkspaceItemcalleduseDiffStats(id), firing a backgroundgit.getStatusrequest per sidebar workspace — even though the expanded row only renders the +/− line-count badge for the active item. On a sidebar with N workspaces, N−1 of those git status reads were pure waste.useDiffStatsnow takes an optional{ enabled }option (defaults totrue; hover-card overlay and other callers unaffected) gating thegit.getStatusquery.git:changedsubscription intentionally stays on while disabled: it's a cheap listener on the shared per-host event bus, and its invalidation marks the cached stats stale so counts refetch when the item becomes active again (withstaleTime: Infinity, gating it would freeze counts at their last-active values).DashboardSidebarWorkspaceItempasses{ enabled: isActive }.The v1
WorkspaceSidebaralready gates its fetch on hover, so no changes there.Test plan
bun run lintclean,tsc --noEmitpasses inapps/desktopSummary by cubic
Fetch diff stats only for the active sidebar workspace to eliminate redundant
git.getStatuscalls and reduce background work.useDiffStatsnow accepts{ enabled }, used byDashboardSidebarWorkspaceItem, while thegit:changedsubscription remains to invalidate stale data so counts refresh when the item becomes active again.Written for commit aefca5f. Summary will update on new commits.