revert: "fix(desktop): show local diff stats in v1 workspace hover card (#3881)"#3887
revert: "fix(desktop): show local diff stats in v1 workspace hover card (#3881)"#3887saddlepaddle merged 1 commit intomainfrom
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe PR removes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Greptile SummaryThis PR reverts #3881 (which added local diff stats to the v1 sidebar hover card) because the underlying flicker issue is in the v2 dashboard sidebar, not v1. On top of the revert it adds net-new functionality to Confidence Score: 5/5Safe to merge; the only finding is dead All substantive changes are a clean revert with a small additive feature in WorkspaceRow. The single remaining finding is P2 style/cleanup (unused apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx — the leftover
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx | The localDiffStats useMemo and diffStats derivation remain after the prop removal, leaving dead computation; everything else is a clean revert. |
| apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/WorkspaceHoverCard/WorkspaceHoverCard.tsx | Reverts to rendering pr.additions/pr.deletions directly; removes the diffStats prop and associated badge; the "No PR for this branch" section simplification looks correct. |
| apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/CollapsedWorkspaceItem.tsx | Cleanly removes diffStats prop and its forwarding to WorkspaceHoverCardContent. |
| apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceContextMenu.tsx | Cleanly removes diffStats prop and its forwarding to WorkspaceHoverCardContent. |
| apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx | Adds useHoverGitHubStatus and renders PR-based diff stats in the list view row; zero-guard (`pr.additions > 0 |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[WorkspaceListItem] -->|hover| B[CollapsedWorkspaceItem]
A -->|right-click| C[WorkspaceContextMenu]
B -->|hover card| D["WorkspaceHoverCardContent\nreads pr.additions/pr.deletions\nvia usePRStatus internally"]
C -->|hover card| D
A -.->|diffStats computed\nbut never passed| X["❌ dead code\n(localDiffStats / diffStats)"]
E["WorkspaceRow (new)"] -->|onMouseEnter| F[useHoverGitHubStatus]
F -->|githubStatus.pr| G["+{pr.additions} -{pr.deletions}\nguarded: > 0 check"]
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx:176-193
**Dead code: `localDiffStats` and `diffStats` are computed but never consumed**
The `localDiffStats` memo and the `diffStats` derivation are both still present after the revert, but the `diffStats` prop was removed from both `CollapsedWorkspaceItem` and `WorkspaceContextMenu`, so nothing actually reads these values. The `useMemo` will still run on every `localChanges` change, wasting a small amount of compute on a hot render path.
Reviews (1): Last reviewed commit: "Revert "fix(desktop): show local diff st..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| workspaceId: id, | ||
| }); | ||
|
|
||
| const localDiffStats = useMemo(() => { | ||
| if (!localChanges) return null; | ||
| const allFiles = | ||
| localChanges.againstBase.length > 0 | ||
| ? localChanges.againstBase | ||
| : [ | ||
| ...localChanges.staged, | ||
| ...localChanges.unstaged, | ||
| ...localChanges.untracked, | ||
| ]; | ||
| const additions = allFiles.reduce((sum, f) => sum + (f.additions || 0), 0); | ||
| const deletions = allFiles.reduce((sum, f) => sum + (f.deletions || 0), 0); | ||
| if (additions === 0 && deletions === 0) return null; | ||
| return { additions, deletions }; | ||
| }, [localChanges]); |
There was a problem hiding this comment.
localDiffStats and diffStats are computed but never consumed
The localDiffStats memo and the diffStats derivation are both still present after the revert, but the diffStats prop was removed from both CollapsedWorkspaceItem and WorkspaceContextMenu, so nothing actually reads these values. The useMemo will still run on every localChanges change, wasting a small amount of compute on a hot render path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx
Line: 176-193
Comment:
**Dead code: `localDiffStats` and `diffStats` are computed but never consumed**
The `localDiffStats` memo and the `diffStats` derivation are both still present after the revert, but the `diffStats` prop was removed from both `CollapsedWorkspaceItem` and `WorkspaceContextMenu`, so nothing actually reads these values. The `useMemo` will still run on every `localChanges` change, wasting a small amount of compute on a hot render path.
How can I resolve this? If you propose a fix, please make it concise.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Reverts #3881 — that PR was applied to the v1 sidebar/hover card, but the actual flicker we were chasing is in the v2 dashboard sidebar. Backing it out so v2 can be addressed cleanly on its own.
Test plan
pr.additions/pr.deletions).Summary by cubic
Reverts the v1 hover card/local diff stats change and restores PR-based additions/deletions. This keeps v1 stable while we handle the v2 sidebar flicker separately.
additions/deletions; local diff stats removed.diffStatsprop from v1 sidebar components.useHoverGitHubStatus.Written for commit 5ed236f. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Bug Fixes
Refactor