[codex] Fix git status refresh storm#4731
Conversation
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (12)
📝 WalkthroughWalkthroughFrontend: adds WorkspaceGitStatusProvider and migrates sidebar/changes hooks to consume provider-backed git status. Backend: introduces getGitStatusSnapshot, a global GitStatusRefreshLimiter with priority handling, TRPC integration to use the limiter/snapshot, and a profiling script for large-repo scenarios. ChangesBackend Git Status Refactoring
Frontend Provider-Based Architecture
Sequence Diagram (high-level request flow)sequenceDiagram
participant Renderer
participant TRPC as TRPC:getStatus
participant Limiter as GitStatusRefreshLimiter
participant Snapshot as getGitStatusSnapshot
participant Git as GitCLI
Renderer->>TRPC: git.getStatus(workspaceId, baseBranch?, priority)
TRPC->>Limiter: run({workspaceId, requestKey, priority, run: ...})
Limiter->>Snapshot: invoke run() when scheduled
Snapshot->>Git: execute git commands (diff/numstat/ls-files)
Git-->>Snapshot: staged/unstaged/ignored info
Snapshot-->>Limiter: GitStatusSnapshot
Limiter-->>TRPC: resolved snapshot
TRPC-->>Renderer: return status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
|
Ready to review this PR? Stage has broken it down into 7 individual chapters for you: Chapters generated by Stage for commit 4ec4054 on May 19, 2026 8:55pm UTC. |
85db2b3 to
554dc8f
Compare
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
There was a problem hiding this comment.
2 issues found across 15 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.ts (1)
18-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore
git:changedevent invalidation to prevent permanently stale diff stats.The query has infinite
staleTimeand disabled window-focus refetch. Without theuseWorkspaceEventinvalidation listener that was removed, git status changes won't trigger UI updates, leaving the workspace diff stats frozen after the initial fetch. Both sidebar and hover card displays depend on live data.Restore the
useWorkspaceEvent("git:changed", ...)invalidation as shown in the proposed diff. This pattern is already used inuseGitStatus.tsand aligns with the event system design.Proposed fix
-import { useQuery } from "`@tanstack/react-query`"; -import { useMemo } from "react"; +import { useQuery, useQueryClient } from "`@tanstack/react-query`"; +import { useCallback, useMemo } from "react"; import { getHostServiceClientByUrl } from "renderer/lib/host-service-client"; +import { useWorkspaceEvent } from "../useWorkspaceEvent"; import { useWorkspaceHostUrl } from "../useWorkspaceHostUrl"; @@ export function useDiffStats(workspaceId: string): DiffStats | null { const hostUrl = useWorkspaceHostUrl(workspaceId); + const queryClient = useQueryClient(); const queryKey = useMemo( () => ["diff-stats", hostUrl, workspaceId] as const, [hostUrl, workspaceId], ); + + const invalidate = useCallback(() => { + void queryClient.invalidateQueries({ queryKey }); + }, [queryClient, queryKey]); + + useWorkspaceEvent("git:changed", workspaceId, invalidate);🤖 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/hooks/host-service/useDiffStats/useDiffStats.ts` around lines 18 - 29, The diff stats query uses useQuery with staleTime = Infinity and no window refetch, so reintroduce the workspace event invalidation for "git:changed" to avoid permanently stale data: add a useWorkspaceEvent("git:changed", ...) hook in useDiffStats.ts that calls the react-query invalidation for the same queryKey used by the useQuery (the queryKey variable) so that when getHostServiceClientByUrl(...).git.getStatus.query data changes the queryClient.invalidateQueries(queryKey) is triggered; this mirrors the pattern in useGitStatus.ts and ensures sidebar/hover card UIs update on git 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.
Outside diff comments:
In `@apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.ts`:
- Around line 18-29: The diff stats query uses useQuery with staleTime =
Infinity and no window refetch, so reintroduce the workspace event invalidation
for "git:changed" to avoid permanently stale data: add a
useWorkspaceEvent("git:changed", ...) hook in useDiffStats.ts that calls the
react-query invalidation for the same queryKey used by the useQuery (the
queryKey variable) so that when
getHostServiceClientByUrl(...).git.getStatus.query data changes the
queryClient.invalidateQueries(queryKey) is triggered; this mirrors the pattern
in useGitStatus.ts and ensures sidebar/hover card UIs update on git changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29f62f39-14fe-4592-92ed-9290a9ff7e6a
📒 Files selected for processing (15)
apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.tsapps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useChangeset/useChangeset.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/providers/WorkspaceGitStatusProvider/WorkspaceGitStatusProvider.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/providers/WorkspaceGitStatusProvider/index.tspackages/host-service/package.jsonpackages/host-service/scripts/git-status-large-repo-profile.tspackages/host-service/src/trpc/router/git/git.tspackages/host-service/src/trpc/router/git/utils/git-helpers.tspackages/host-service/src/trpc/router/git/utils/git-status-refresh-limiter.test.tspackages/host-service/src/trpc/router/git/utils/git-status-refresh-limiter.tspackages/host-service/src/trpc/router/git/utils/git-status.ts
Greptile SummaryThis PR fixes a git-status refresh storm caused by every
Confidence Score: 3/5The limiter and provider consolidation are solid, but useDiffStats was left disconnected from any refresh mechanism after its event subscription was removed. The core limiter logic and WorkspaceGitStatusProvider refactor are well-structured and tested. The defect in useDiffStats is real: it still holds its own separate React Query cache with staleTime set to Infinity, and the git:changed listener that was its only refresh trigger was removed without wiring it to the new provider. Any component showing diff additions/deletions will display frozen values for the lifetime of the component until the user blurs and refocuses the window. useDiffStats.ts needs either a connection to useWorkspaceGitStatus() or a restored invalidation path for its own query cache.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/trpc/router/git/utils/git-status-refresh-limiter.ts | New class that serializes git status refreshes per workspace, collapses duplicate request keys into one trailing task, and caps global concurrency. Logic is correct and well-tested. |
| packages/host-service/src/trpc/router/git/git.ts | Wires the getStatus procedure through gitStatusRefreshLimiter; the requestKey correctly serializes baseBranch so different base branches queue independently. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/providers/WorkspaceGitStatusProvider/WorkspaceGitStatusProvider.tsx | New context provider that owns one useGitStatus instance per workspace; gates fetching on sidebarOpen |
| apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.ts | Removes the git:changed event subscription, but the hook still uses its own separate React Query cache with staleTime: Infinity — no replacement refresh trigger is added, so diff stats will be stale after git changes while the window is focused. |
| apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts | Adds enabled parameter, consolidates getDiff invalidation here (moved from useChangeset), correctly passes payload to differentiate path-specific vs full invalidations. |
| packages/host-service/scripts/git-status-large-repo-profile.ts | Large profiler script; maxActiveRefreshes for limited/event-bus mode is hacked to Math.max(1, ...) because activeRefreshes is never incremented in that code path, making the metric misleading. |
Sequence Diagram
sequenceDiagram
participant FS as Filesystem
participant GW as GitWatcher
participant EB as EventBus
participant R as Renderer (WorkspaceGitStatusProvider)
participant L as GitStatusRefreshLimiter
participant G as git subprocess(es)
FS->>GW: file change
GW->>EB: git:changed (debounced)
EB->>R: git:changed event
R->>R: invalidate workspaceTrpc.git.getStatus
R->>L: "limiter.run({workspaceId, requestKey})"
note over L: collapse duplicate requestKey or queue trailing task
note over L: cap activeCount < concurrency
L->>G: getGitStatusSnapshot()
G-->>L: GitStatusSnapshot
L-->>R: resolved promise
R->>R: update shared context value
R-->>R: useChangeset, useChangesTab, WorkspaceSidebar re-render
Comments Outside Diff (1)
-
packages/host-service/scripts/git-status-large-repo-profile.ts, line 1138-1141 (link)maxActiveRefreshesis always 0 for limited/event-bus modeIn
runEventBusScenariowithmode === "limited",activeRefreshesandmaxActiveRefreshesare only updated insiderunRefresh(), which is the unbounded path. In the limited path,actualRefreshesis incremented inside thegitfactory butactiveRefreshesis never touched. TheMath.max(1, maxActiveRefreshes)fallback masks this by forcing the result to 1, making the metric misleading — the real peak concurrency (which the limiter controls) is not captured at all. To get an accurate reading, hook theactiveRefreshescounter into thegitfactory's enter/exit points (similar to howrunRefreshtracks it in the compute flow).Prompt To Fix With AI
This is a comment left during a code review. Path: packages/host-service/scripts/git-status-large-repo-profile.ts Line: 1138-1141 Comment: **`maxActiveRefreshes` is always 0 for limited/event-bus mode** In `runEventBusScenario` with `mode === "limited"`, `activeRefreshes` and `maxActiveRefreshes` are only updated inside `runRefresh()`, which is the unbounded path. In the limited path, `actualRefreshes` is incremented inside the `git` factory but `activeRefreshes` is never touched. The `Math.max(1, maxActiveRefreshes)` fallback masks this by forcing the result to 1, making the metric misleading — the real peak concurrency (which the limiter controls) is not captured at all. To get an accurate reading, hook the `activeRefreshes` counter into the `git` factory's enter/exit points (similar to how `runRefresh` tracks it in the compute flow). How can I resolve this? If you propose a fix, please make it concise.
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/hooks/host-service/useDiffStats/useDiffStats.ts:28-34
**Stale diff-stats after git changes**
`useDiffStats` uses its own React Query cache (`["diff-stats", hostUrl, workspaceId]`) that is completely separate from the `workspaceTrpc.git.getStatus` cache that `useGitStatus` and `WorkspaceGitStatusProvider` invalidate. Removing the `useWorkspaceEvent("git:changed", …)` call leaves `staleTime: Infinity` with no replacement refresh trigger. While the window is focused, any git change (stage, unstage, commit) will no longer cause `useDiffStats` to return fresh additions/deletions — the values freeze at their initial fetch until the user blurs and refocuses the window. Either connect this hook to `useWorkspaceGitStatus()` (computing stats from the shared context data instead of a separate query), or re-add the event-driven invalidation targeting the `["diff-stats", …]` query key.
### Issue 2 of 2
packages/host-service/scripts/git-status-large-repo-profile.ts:1138-1141
**`maxActiveRefreshes` is always 0 for limited/event-bus mode**
In `runEventBusScenario` with `mode === "limited"`, `activeRefreshes` and `maxActiveRefreshes` are only updated inside `runRefresh()`, which is the unbounded path. In the limited path, `actualRefreshes` is incremented inside the `git` factory but `activeRefreshes` is never touched. The `Math.max(1, maxActiveRefreshes)` fallback masks this by forcing the result to 1, making the metric misleading — the real peak concurrency (which the limiter controls) is not captured at all. To get an accurate reading, hook the `activeRefreshes` counter into the `git` factory's enter/exit points (similar to how `runRefresh` tracks it in the compute flow).
Reviews (1): Last reviewed commit: "fix git status refresh storm" | Re-trigger Greptile
| refetchOnWindowFocus: false, | ||
| staleTime: Number.POSITIVE_INFINITY, | ||
| }); | ||
|
|
||
| const invalidate = useCallback(() => { | ||
| void queryClient.invalidateQueries({ queryKey }); | ||
| }, [queryClient, queryKey]); | ||
|
|
||
| useWorkspaceEvent("git:changed", workspaceId, invalidate); | ||
|
|
||
| return useMemo<DiffStats | null>(() => { | ||
| if (!status) return null; | ||
|
|
There was a problem hiding this comment.
Stale diff-stats after git changes
useDiffStats uses its own React Query cache (["diff-stats", hostUrl, workspaceId]) that is completely separate from the workspaceTrpc.git.getStatus cache that useGitStatus and WorkspaceGitStatusProvider invalidate. Removing the useWorkspaceEvent("git:changed", …) call leaves staleTime: Infinity with no replacement refresh trigger. While the window is focused, any git change (stage, unstage, commit) will no longer cause useDiffStats to return fresh additions/deletions — the values freeze at their initial fetch until the user blurs and refocuses the window. Either connect this hook to useWorkspaceGitStatus() (computing stats from the shared context data instead of a separate query), or re-add the event-driven invalidation targeting the ["diff-stats", …] query key.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.ts
Line: 28-34
Comment:
**Stale diff-stats after git changes**
`useDiffStats` uses its own React Query cache (`["diff-stats", hostUrl, workspaceId]`) that is completely separate from the `workspaceTrpc.git.getStatus` cache that `useGitStatus` and `WorkspaceGitStatusProvider` invalidate. Removing the `useWorkspaceEvent("git:changed", …)` call leaves `staleTime: Infinity` with no replacement refresh trigger. While the window is focused, any git change (stage, unstage, commit) will no longer cause `useDiffStats` to return fresh additions/deletions — the values freeze at their initial fetch until the user blurs and refocuses the window. Either connect this hook to `useWorkspaceGitStatus()` (computing stats from the shared context data instead of a separate query), or re-add the event-driven invalidation targeting the `["diff-stats", …]` query key.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/host-service/src/trpc/router/git/utils/git-status-refresh-limiter.test.ts (1)
14-285: ⚡ Quick winAdd a regression test for
clear()during active work.Given the limiter now has non-trivial lifecycle state, a test for “clear while one task is running and one is queued” would lock intended semantics (no negative active count, no post-clear requeue from stale workspace state).
🤖 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 `@packages/host-service/src/trpc/router/git/utils/git-status-refresh-limiter.test.ts` around lines 14 - 285, Add a regression test to GitStatusRefreshLimiter that starts one long-running run (use deferred), queues a second run for the same or different workspace, calls limiter.clear() while the first is still running, then resolve the first and assert: (1) the first run completed normally, (2) the queued second never started (track via an events array or start markers), and (3) the limiter accepts new runs afterwards (i.e., a subsequent run starts normally). Use the existing test style and functions (GitStatusRefreshLimiter, run, deferred) to reproduce “clear during active work” and ensure no negative active count or post-clear requeue occurs.
🤖 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
`@packages/host-service/src/trpc/router/git/utils/git-status-refresh-limiter.ts`:
- Around line 72-76: The clear() method is unsafe because it resets
this.activeCount and this.readyQueue while in-flight tasks and stale closures
can still decrement activeCount or requeue work; add a generation guard:
introduce a numeric this.generation incremented inside clear(), have
run/settlement closures capture the generation at start and on settle only
proceed (decrement activeCount, push to readyQueue, call scheduleNext, etc.) if
the captured generation === this.generation, and ensure clear() increments
generation before clearing this.workspaces and this.readyQueue so any later
callbacks are ignored and cannot underflow activeCount or revive cleared work.
---
Nitpick comments:
In
`@packages/host-service/src/trpc/router/git/utils/git-status-refresh-limiter.test.ts`:
- Around line 14-285: Add a regression test to GitStatusRefreshLimiter that
starts one long-running run (use deferred), queues a second run for the same or
different workspace, calls limiter.clear() while the first is still running,
then resolve the first and assert: (1) the first run completed normally, (2) the
queued second never started (track via an events array or start markers), and
(3) the limiter accepts new runs afterwards (i.e., a subsequent run starts
normally). Use the existing test style and functions (GitStatusRefreshLimiter,
run, deferred) to reproduce “clear during active work” and ensure no negative
active count or post-clear requeue occurs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3409a86f-d3a8-49ac-a760-06420edeb1d9
📒 Files selected for processing (15)
apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.tsapps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useChangeset/useChangeset.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/providers/WorkspaceGitStatusProvider/WorkspaceGitStatusProvider.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/providers/WorkspaceGitStatusProvider/index.tspackages/host-service/package.jsonpackages/host-service/scripts/git-status-large-repo-profile.tspackages/host-service/src/trpc/router/git/git.tspackages/host-service/src/trpc/router/git/utils/git-helpers.tspackages/host-service/src/trpc/router/git/utils/git-status-refresh-limiter.test.tspackages/host-service/src/trpc/router/git/utils/git-status-refresh-limiter.tspackages/host-service/src/trpc/router/git/utils/git-status.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/providers/WorkspaceGitStatusProvider/index.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/providers/WorkspaceGitStatusProvider/WorkspaceGitStatusProvider.tsx
- apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts
- packages/host-service/src/trpc/router/git/git.ts
- packages/host-service/src/trpc/router/git/utils/git-helpers.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx
- apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.ts
- packages/host-service/scripts/git-status-large-repo-profile.ts
- packages/host-service/src/trpc/router/git/utils/git-status.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useChangeset/useChangeset.ts
554dc8f to
4ec4054
Compare
What changed
git.getStatusrefresh limiter that serializes refreshes per workspace, keeps one trailing refresh per request key, and caps global concurrent status refreshes.useGitStatusowner/provider per workspace instead of multiple consumers independently subscribing togit:changedand refetching status.-C) from status diff calls while keeping rename detection (-M).Why
Under worktree churn,
GitWatchercorrectly debounces filesystem events, but every emittedgit:changedcould still trigger overlapping full git status refreshes. Each refresh fans out to many git subprocesses, so sustained churn could produce a process swarm and pin CPU. This bounds the expensive work in the host service and removes duplicate renderer ownership of the live status query.The limiter bounds event storms per workspace, but rapid switching can still enqueue work across many distinct workspaces. To avoid stale background work delaying the workspace the user is actually looking at, foreground requests are scheduled ahead of background sidebar stats without cancelling visible requests.
Validation
bun test packages/host-service/src/trpc/router/git/utils/git-status-refresh-limiter.test.ts packages/host-service/test/integration/git.integration.test.tsbun run --cwd packages/host-service typecheckbun run --cwd apps/desktop typecheckbun run lintgit diff --checkLarge event-flow profiler, 30k tracked files, 1k dirty files, 60 worktree mutations, 100ms artificial git delay:
I also tried a more aggressive server-side trailing debounce, but rejected it because queued tRPC callers can hit the existing 15s
getStatustimeout under heavy churn. This PR keeps the safer fix that bounds process fanout without delaying the normal first refresh.Summary by CodeRabbit
Improvements
New Features