Skip to content

Commit dff3d45

Browse files
committed
perf(desktop): key sidebar git-watch off agent activity, not every row
DashboardSidebarWorkspaceStatusProvider called watchGit for every sidebar row to keep diff-stat caches warm. Verified live against real data in #6848: opening the dashboard alone (no workspace opened) caused 79 of 90 non-archived workspaces to become watched, since the sidebar is normally mounted for the whole session — the load and terminal-lag scenario #6729 set out to fix, reintroduced through a different subscriber. An idle workspace's git state can't change without user interaction, so its diff count doesn't need to stay live — it only needs to be correct the next time it's opened, which useDiffStats' normal query fetch already handles. Non-active rows don't render a diff count today anyway (only the active row does, per the existing `entries` memo), so losing liveness for an idle row costs a first-open loading flash, not a wrong number on screen. Now only workspaces with a currently running/blocked/attention-needing agent (working/permission/review/failed — any non-idle PaneStatus) or the active workspace hold live git-watch interest. The signal is already computed here (bindingRowsByIndex + deriveTerminalAgentStatus, the same data driving the status dot) — no new plumbing. The full lifecycle-listener pass (agent:lifecycle/terminal:lifecycle/git:changed) still covers every row regardless, since that's what detects a row transitioning into activity in the first place; only the watchGit/unwatchGit calls — the ones with real host-side cost — are now gated. Split into two effects so the (still full-coverage, cheap) listener registration doesn't churn every time an unrelated workspace's terminal status flips: one for listeners (unchanged, keyed on the full target list), one for git-watch diffing (new, keyed on a fingerprint-stabilized "currently worth watching" set), plus the existing final-unmount cleanup. Verified: typecheck and biome clean; no automated test exists for this component (none did before this change either). Logic re-reviewed by hand against the exact structure verified live via CDP against real production data in #6848, but not independently re-verified live in this follow-up — happy to spin up a fresh dev session and confirm the watched-workspace count actually drops if useful before merge. Follow-up to #6848 / #6729. Claude-Session: https://claude.ai/code/session_01NmLFihnhebmL9bbbojGYCR
1 parent 1ff427b commit dff3d45

1 file changed

Lines changed: 107 additions & 53 deletions

File tree

apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/providers/DashboardSidebarWorkspaceStatusProvider/DashboardSidebarWorkspaceStatusProvider.tsx

Lines changed: 107 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -225,26 +225,94 @@ export function DashboardSidebarWorkspaceStatusProvider({
225225
combine: (results) => results.map((result) => result.data),
226226
});
227227

228-
// One lifecycle/git subscription pass for the whole sidebar (the per-row
229-
// hooks used to register these per workspace, several times over). The
230-
// git:changed invalidation keeps diff-stats caches fresh for rows that
231-
// aren't currently displaying them (staleTime is Infinity there, so a
232-
// missed invalidation would freeze counts on the next activation).
233-
//
234-
// GitWatcher only watches a workspace while someone holds interest
235-
// (#6729) — this call site talks to the bus directly rather than through
236-
// useWorkspaceEvent, so it must drive watchGit/unwatchGit itself. This
237-
// registers every workspace row shown in the sidebar, which for a heavy
238-
// user is most/all of their non-archived workspaces — preserves today's
239-
// "every row's diff count stays live" behavior exactly, but means this
240-
// provider alone can keep a large fraction of the workspace population
241-
// watched whenever the dashboard is open. Worth revisiting (e.g.
242-
// staleTime-based refetch for non-active rows instead of a live watch)
243-
// as a follow-up.
228+
// Only the active row renders diff stats, so one query serves the sidebar.
229+
const activeDiffStats = useDiffStats(activeWorkspaceId ?? "", {
230+
enabled: activeWorkspaceId !== null,
231+
});
232+
233+
const manualUnread = useV2NotificationStore((state) => state.manualUnread);
234+
const terminalSeenAt = useV2NotificationStore(
235+
(state) => state.terminalSeenAt,
236+
);
237+
238+
// Per-target terminal statuses — shared by the git-watch gating below and
239+
// the entries memo further down, so the two never drift.
240+
const statusesByIndex = useMemo(
241+
() =>
242+
targets.map((_target, index) => {
243+
const bindingRows = bindingRowsByIndex[index] ?? [];
244+
const statuses = new Map<string, PaneStatus>();
245+
for (const binding of bindingRows) {
246+
statuses.set(
247+
binding.terminalId,
248+
deriveTerminalAgentStatus({
249+
lastEventType: binding.lastEventType,
250+
lastEventAt: binding.lastEventAt,
251+
lastSeenAt: terminalSeenAt[binding.terminalId],
252+
}),
253+
);
254+
}
255+
return statuses;
256+
}),
257+
[targets, bindingRowsByIndex, terminalSeenAt],
258+
);
259+
260+
// Only workspaces with a currently running/blocked/attention-needing
261+
// agent (or the active workspace) are worth a live git:changed watch —
262+
// see #6729/#6848. An idle workspace's diff count can't change without
263+
// user interaction, so it's fine for it to refresh only when actually
264+
// opened (useDiffStats' normal query fetch) instead of paying for a live
265+
// host-side watcher (DB lookup + git subprocess + fs.watch attach) on
266+
// the chance the user clicks into it next. Non-active rows never render
267+
// a diff count today anyway (see `entries` below) — losing liveness here
268+
// only means a first-open loading flash instead of an instant number.
269+
const computedGitWatchTargets = useMemo(() => {
270+
const result: Array<{ workspaceId: string; hostUrl: string }> = [];
271+
targets.forEach((target, index) => {
272+
if (!target.hostUrl) return;
273+
const isActive = target.workspaceId === activeWorkspaceId;
274+
const statuses = statusesByIndex[index];
275+
const hasActiveTerminal = statuses
276+
? [...statuses.values()].some((status) => status !== "idle")
277+
: false;
278+
if (isActive || hasActiveTerminal) {
279+
result.push({
280+
workspaceId: target.workspaceId,
281+
hostUrl: target.hostUrl,
282+
});
283+
}
284+
});
285+
return result;
286+
}, [targets, statusesByIndex, activeWorkspaceId]);
287+
// Fingerprint-stabilized for the same reason `targets` is above: this
288+
// must only change identity when the watch set actually changes, not on
289+
// every unrelated bindingRows refetch.
290+
const previousGitWatchTargetsRef = useRef<{
291+
fingerprint: string;
292+
targets: Array<{ workspaceId: string; hostUrl: string }>;
293+
} | null>(null);
294+
const gitWatchTargets = useMemo(() => {
295+
const fingerprint = JSON.stringify(computedGitWatchTargets);
296+
const previous = previousGitWatchTargetsRef.current;
297+
if (previous?.fingerprint === fingerprint) return previous.targets;
298+
previousGitWatchTargetsRef.current = {
299+
fingerprint,
300+
targets: computedGitWatchTargets,
301+
};
302+
return computedGitWatchTargets;
303+
}, [computedGitWatchTargets]);
304+
305+
// One lifecycle/git-listener subscription pass for the whole sidebar
306+
// (the per-row hooks used to register these per workspace, several
307+
// times over). Covers every row, not just `gitWatchTargets` — this is
308+
// what detects a row transitioning into activity in the first place
309+
// (agent:lifecycle/terminal:lifecycle), and a git:changed listener for a
310+
// workspace nobody's watching simply never fires. Cheap to tear down
311+
// and redo every render, unlike the watchGit/unwatchGit calls in the
312+
// effect below.
244313
useEffect(() => {
245314
const cleanups: Array<() => void> = [];
246315
const retainedHostUrls = new Set<string>();
247-
const nextGitWatched = new Map<string, string>();
248316
for (const { workspaceId, hostUrl } of targets) {
249317
if (!hostUrl) continue;
250318
const bus = getHostEventBus(hostUrl);
@@ -268,17 +336,25 @@ export function DashboardSidebarWorkspaceStatusProvider({
268336
});
269337
}),
270338
);
271-
nextGitWatched.set(workspaceId, hostUrl);
272339
}
340+
return () => {
341+
for (const cleanup of cleanups) cleanup();
342+
};
343+
}, [targets, queryClient]);
273344

274-
// Diff against the previous run's watched set: only call
275-
// watchGit/unwatchGit for rows that actually changed. Unlike the
276-
// on() listener registrations above (cheap to tear down and redo
277-
// every render), watchGit/unwatchGit carry real host-side cost — a
278-
// DB lookup, a git subprocess, and a live fs.watch attach/teardown.
279-
// Blindly unwatching then rewatching every row whenever a single
280-
// unrelated workspace is created, renamed, or reassigned a host
281-
// would reintroduce exactly the fan-out storm #6729 set out to fix.
345+
// GitWatcher only watches a workspace while someone holds interest
346+
// (#6729) — this call site talks to the bus directly rather than
347+
// through useWorkspaceEvent, so it must drive watchGit/unwatchGit
348+
// itself. Diffs against the previous run's watched set: only calls
349+
// watchGit/unwatchGit for rows that actually entered or left
350+
// `gitWatchTargets`, since — unlike the listener registrations above —
351+
// these carry real host-side cost (a DB lookup, a git subprocess, and a
352+
// live fs.watch attach/teardown). Blindly unwatching then rewatching
353+
// everything on every change would reintroduce a fan-out storm.
354+
useEffect(() => {
355+
const nextGitWatched = new Map(
356+
gitWatchTargets.map(({ workspaceId, hostUrl }) => [workspaceId, hostUrl]),
357+
);
282358
const prevGitWatched = gitWatchedRef.current;
283359
for (const [workspaceId, hostUrl] of prevGitWatched) {
284360
if (nextGitWatched.get(workspaceId) !== hostUrl) {
@@ -291,16 +367,12 @@ export function DashboardSidebarWorkspaceStatusProvider({
291367
}
292368
}
293369
gitWatchedRef.current = nextGitWatched;
294-
295-
return () => {
296-
for (const cleanup of cleanups) cleanup();
297-
};
298-
}, [targets, queryClient]);
370+
}, [gitWatchTargets]);
299371

300372
// Release git-watch interest for every currently-watched row on final
301373
// unmount. Deliberately a separate, dep-less effect: its cleanup only
302-
// runs once, at unmount, so an intermediate targets change (handled by
303-
// the diffing above) can't trip this and undo it.
374+
// runs once, at unmount, so an intermediate gitWatchTargets change
375+
// (handled by the diffing above) can't trip this and undo it.
304376
useEffect(() => {
305377
return () => {
306378
for (const [workspaceId, hostUrl] of gitWatchedRef.current) {
@@ -310,16 +382,6 @@ export function DashboardSidebarWorkspaceStatusProvider({
310382
};
311383
}, []);
312384

313-
// Only the active row renders diff stats, so one query serves the sidebar.
314-
const activeDiffStats = useDiffStats(activeWorkspaceId ?? "", {
315-
enabled: activeWorkspaceId !== null,
316-
});
317-
318-
const manualUnread = useV2NotificationStore((state) => state.manualUnread);
319-
const terminalSeenAt = useV2NotificationStore(
320-
(state) => state.terminalSeenAt,
321-
);
322-
323385
const previousEntriesRef = useRef(
324386
new Map<string, SidebarWorkspaceStatusEntry>(),
325387
);
@@ -329,18 +391,10 @@ export function DashboardSidebarWorkspaceStatusProvider({
329391
targets.forEach((target, index) => {
330392
const bindingRows = bindingRowsByIndex[index] ?? [];
331393
const bindings = new Map<string, TerminalAgentBinding>();
332-
const statuses = new Map<string, PaneStatus>();
333394
for (const binding of bindingRows) {
334395
bindings.set(binding.terminalId, binding);
335-
statuses.set(
336-
binding.terminalId,
337-
deriveTerminalAgentStatus({
338-
lastEventType: binding.lastEventType,
339-
lastEventAt: binding.lastEventAt,
340-
lastSeenAt: terminalSeenAt[binding.terminalId],
341-
}),
342-
);
343396
}
397+
const statuses = statusesByIndex[index] ?? new Map<string, PaneStatus>();
344398
const hasManualUnread = Boolean(manualUnread[target.workspaceId]);
345399
let hasAttentionTerminal = false;
346400
for (const status of statuses.values()) {
@@ -373,8 +427,8 @@ export function DashboardSidebarWorkspaceStatusProvider({
373427
}, [
374428
targets,
375429
bindingRowsByIndex,
430+
statusesByIndex,
376431
manualUnread,
377-
terminalSeenAt,
378432
activeWorkspaceId,
379433
activeDiffStats,
380434
]);

0 commit comments

Comments
 (0)