Skip to content

fix(desktop): restore v2 sidebar LOC indicator #4222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 1 commit into from
May 8, 2026
+23 −21
Merged

fix(desktop): restore v2 sidebar LOC indicator #4222

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 23 additions & 21 deletions apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,44 @@
import { workspaceTrpc } from "@superset/workspace-client";
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 interface DiffStats {
additions: number;
deletions: number;
}

/**
* Diff stats for a single workspace, derived from the shared `git.getStatus`
* query cache. Subscribes to `git:changed` and invalidates the query — React
* Query collapses concurrent invalidations from sibling consumers (e.g.
* `useGitStatus`, multiple sidebar tiles) into a single refetch.
*/
export function useDiffStats(workspaceId: string): DiffStats | null {
const utils = workspaceTrpc.useUtils();
const { data: status } = workspaceTrpc.git.getStatus.useQuery(
{ workspaceId },
{
enabled: Boolean(workspaceId),
// Match the pre-RQ behavior: only update on `git:changed`, never
// on focus. Multiple sidebar tiles each have their own query key,
// so focus refetch would re-fan out the very work this hook is
// supposed to consolidate.
refetchOnWindowFocus: false,
},
const hostUrl = useWorkspaceHostUrl(workspaceId);
const queryClient = useQueryClient();
const queryKey = useMemo(
() => ["diff-stats", hostUrl, workspaceId] as const,
[hostUrl, workspaceId],
);

const { data: status } = useQuery({
queryKey,
enabled: Boolean(workspaceId) && Boolean(hostUrl),
queryFn: () => {
if (!hostUrl) return null;
return getHostServiceClientByUrl(hostUrl).git.getStatus.query({
workspaceId,
});
},
refetchOnWindowFocus: false,
staleTime: Number.POSITIVE_INFINITY,
});
Comment on lines +29 to +31
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

staleTime: POSITIVE_INFINITY creates a silent stale-data risk on first mount.

With git:changed as the sole refresh trigger, any change that occurs between component mount and listener registration (useWorkspaceEvent subscribing) will leave the UI permanently stale for that session — no background refetch or window-focus refetch will rescue it. The PR notes this is intentional ("push-authoritative"), but it's worth confirming the event subscription is guaranteed to be in place before any relevant git:changed events can fire.

If there's a realistic race on initial mount (e.g., workspace git state changes during the first render cycle), consider a short staleTime (e.g., 30 s) or a conservative gcTime paired with an initial refetchOnMount: true to ensure the first load is always fresh, while still avoiding redundant window-focus refetches.

🤖 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 29 - 31, The query config in useDiffStats() sets staleTime to
Number.POSITIVE_INFINITY which risks permanent stale data before
useWorkspaceEvent() subscribes; update the query options in the useDiffStats
hook to ensure an initial fresh load by either setting a short staleTime (e.g.,
30_000 ms) or adding refetchOnMount: true (and optionally a conservative
cacheTime/gcTime) so the first mount always refetches while keeping
refetchOnWindowFocus: false; locate the query options object in useDiffStats and
adjust the staleTime/refetchOnMount (and optionally cacheTime) accordingly.

Comment on lines +29 to +31
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stale indicator if git:changed event is missed on reconnect

staleTime: Number.POSITIVE_INFINITY + refetchOnWindowFocus: false means the LOC chip updates only when a git:changed WebSocket event is received. If the event bus connection drops and reconnects, any file edits made during the gap are silently invisible — the chip keeps showing the pre-disconnect value until the user makes another change that triggers a fresh push event. In the previous staleTime: 0 regime a component remount (e.g., after a tab switch) would naturally re-prime the cache; that safety net is now gone.

Consider adding refetchOnReconnect: true (verify it fires for the non-window WebSocket path) or having the event bus emit a synthetic git:changed after reconnect so the existing invalidation path kicks in.

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: 29-31

Comment:
**Stale indicator if `git:changed` event is missed on reconnect**

`staleTime: Number.POSITIVE_INFINITY` + `refetchOnWindowFocus: false` means the LOC chip updates *only* when a `git:changed` WebSocket event is received. If the event bus connection drops and reconnects, any file edits made during the gap are silently invisible — the chip keeps showing the pre-disconnect value until the user makes another change that triggers a fresh push event. In the previous `staleTime: 0` regime a component remount (e.g., after a tab switch) would naturally re-prime the cache; that safety net is now gone.

Consider adding `refetchOnReconnect: true` (verify it fires for the non-window WebSocket path) or having the event bus emit a synthetic `git:changed` after reconnect so the existing invalidation path kicks in.

How can I resolve this? If you propose a fix, please make it concise.


const invalidate = useCallback(() => {
void utils.git.getStatus.invalidate({ workspaceId });
}, [utils, workspaceId]);
void queryClient.invalidateQueries({ queryKey });
}, [queryClient, queryKey]);
Comment on lines 33 to +35
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The invalidate callback captures queryKey at the time it was last memoized. When hostUrl is still null (the workspace host is loading), queryKey is ["diff-stats", null, workspaceId] and any git:changed event fired in that window calls invalidateQueries with that null key — targeting a query that was never enabled and has no cached data, so the invalidation is silently dropped. Adding an early-return guard makes the intent explicit and avoids the vacuous call.

Suggested change
const invalidate = useCallback(() => {
void utils.git.getStatus.invalidate({ workspaceId });
}, [utils, workspaceId]);
void queryClient.invalidateQueries({ queryKey });
}, [queryClient, queryKey]);
const invalidate = useCallback(() => {
if (!hostUrl) return;
void queryClient.invalidateQueries({ queryKey });
}, [queryClient, queryKey, hostUrl]);
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: 33-35

Comment:
The `invalidate` callback captures `queryKey` at the time it was last memoized. When `hostUrl` is still `null` (the workspace host is loading), `queryKey` is `["diff-stats", null, workspaceId]` and any `git:changed` event fired in that window calls `invalidateQueries` with that null key — targeting a query that was never enabled and has no cached data, so the invalidation is silently dropped. Adding an early-return guard makes the intent explicit and avoids the vacuous call.

```suggestion
	const invalidate = useCallback(() => {
		if (!hostUrl) return;
		void queryClient.invalidateQueries({ queryKey });
	}, [queryClient, queryKey, hostUrl]);
```

How can I resolve this? If you propose a fix, please make it concise.


useWorkspaceEvent("git:changed", workspaceId, invalidate);

return useMemo<DiffStats | null>(() => {
if (!status) return null;

// Deduplicate by path — a file can appear in multiple categories.
const byPath = new Map<string, { additions: number; deletions: number }>();
for (const file of status.againstBase) byPath.set(file.path, file);
for (const file of status.staged) byPath.set(file.path, file);
Expand Down
Loading