Skip to content

Commit ad4525e

Browse files
authored
🤖 refactor: replace countdown auto-refresh with tool-completion-driven refresh (#1229)
Replace the arbitrary 30-second countdown timer with event-driven refresh triggered by file-modifying tool completions. ## Changes - Add `onToolCallEnd` subscription to WorkspaceStore - Subscribe to tool-call-end events in ReviewPanel - Trigger refresh 3s after `file_edit_*` or `bash` tool completes (debounced) - Pause auto-refresh while user is focused on panel - Fire pending refresh when user blurs panel - Remove countdown UI from ReviewControls ## Benefits - **No arbitrary timing** - refresh only when files actually change - **Better SSH workspace UX** - no countdown freezing during slow fetches - **Respects user interaction** - won't disrupt active review work - **Simpler code** - removes polling interval and countdown state --- _Generated with `mux` • Model: `anthropic:claude-opus-4-5` • Thinking: `high`_
1 parent 96703dc commit ad4525e

File tree

3 files changed

+115
-69
lines changed

3 files changed

+115
-69
lines changed

‎src/browser/components/RightSidebar/CodeReview/ReviewControls.tsx‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ interface ReviewControlsProps {
1616
isLoading?: boolean;
1717
workspaceId: string;
1818
workspacePath: string;
19-
/** If set, show an auto-refresh countdown (e.g., for origin/* bases). */
20-
autoRefreshSecondsRemaining?: number | null;
2119
refreshTrigger?: number;
2220
}
2321

@@ -29,7 +27,6 @@ export const ReviewControls: React.FC<ReviewControlsProps> = ({
2927
isLoading = false,
3028
workspaceId,
3129
workspacePath,
32-
autoRefreshSecondsRemaining,
3330
refreshTrigger,
3431
}) => {
3532
// Local state for input value - only commit on blur/Enter
@@ -86,11 +83,6 @@ export const ReviewControls: React.FC<ReviewControlsProps> = ({
8683

8784
return (
8885
<div className="bg-separator border-border-light flex flex-wrap items-center gap-3 border-b px-3 py-2 text-[11px]">
89-
{autoRefreshSecondsRemaining != null && (
90-
<span className="text-muted whitespace-nowrap tabular-nums">
91-
Refresh in: {String(autoRefreshSecondsRemaining).padStart(2, "\u2007")}s
92-
</span>
93-
)}
9486
{onRefresh && <RefreshButton onClick={onRefresh} isLoading={isLoading} />}
9587
<label className="text-muted font-medium whitespace-nowrap">Base:</label>
9688
<input

‎src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx‎

Lines changed: 83 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import { matchesKeybind, KEYBINDS, formatKeybind } from "@/browser/utils/ui/keyb
4343
import { applyFrontendFilters } from "@/browser/utils/review/filterHunks";
4444
import { cn } from "@/common/lib/utils";
4545
import { useAPI, type APIClient } from "@/browser/contexts/API";
46+
import { workspaceStore } from "@/browser/stores/WorkspaceStore";
4647

4748
/** Stats reported to parent for tab display */
4849
interface ReviewPanelStats {
@@ -126,7 +127,13 @@ function makeReviewPanelCacheKey(params: {
126127

127128
type ExecuteBashResult = Awaited<ReturnType<APIClient["workspace"]["executeBash"]>>;
128129

129-
const REVIEW_AUTO_REFRESH_INTERVAL_MS = 30_000;
130+
/** Check if a tool may modify files and should trigger diff refresh */
131+
function isFileModifyingTool(toolName: string): boolean {
132+
return toolName.startsWith("file_edit_") || toolName === "bash";
133+
}
134+
135+
/** Debounce delay for auto-refresh after tool completion */
136+
const TOOL_REFRESH_DEBOUNCE_MS = 3000;
130137

131138
function getOriginBranchForFetch(diffBase: string): string | null {
132139
const trimmed = diffBase.trim();
@@ -175,6 +182,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
175182
}) => {
176183
const { api } = useAPI();
177184
const panelRef = useRef<HTMLDivElement>(null);
185+
const scrollContainerRef = useRef<HTMLDivElement>(null);
178186
const searchInputRef = useRef<HTMLInputElement>(null);
179187

180188
// Unified diff state - discriminated union makes invalid states unrepresentable
@@ -234,60 +242,49 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
234242
[diffState]
235243
);
236244

237-
const [autoRefreshSecondsRemaining, setAutoRefreshSecondsRemaining] = useState<number | null>(
238-
null
239-
);
240-
const [isAutoRefreshing, setIsAutoRefreshing] = useState(false);
241-
const autoRefreshDeadlineRef = useRef<number | null>(null);
245+
// Track whether refresh is in-flight (for origin/* fetch)
246+
const isRefreshingRef = useRef(false);
247+
248+
// Track user interaction with review notes (pause auto-refresh while focused)
249+
const isUserInteractingRef = useRef(false);
250+
const pendingRefreshRef = useRef(false);
251+
252+
// Save scroll position before refresh to restore after
253+
const savedScrollTopRef = useRef<number | null>(null);
254+
242255
const [filters, setFilters] = useState<ReviewFiltersType>({
243256
showReadHunks: showReadHunks,
244257
diffBase: diffBase,
245258
includeUncommitted: includeUncommitted,
246259
});
247260

248-
// Auto-refresh diffs every 30s (with a user-visible countdown).
249-
// For origin/* bases, fetches from remote first to pick up upstream changes.
261+
// Auto-refresh on file-modifying tool completions (debounced 3s).
262+
// Respects user interaction - if user is focused on review input, queues refresh for after blur.
250263
useEffect(() => {
251-
if (!api || isCreating) {
252-
autoRefreshDeadlineRef.current = null;
253-
setAutoRefreshSecondsRemaining(null);
254-
return;
255-
}
256-
257-
autoRefreshDeadlineRef.current = Date.now() + REVIEW_AUTO_REFRESH_INTERVAL_MS;
258-
259-
const resetCountdown = () => {
260-
autoRefreshDeadlineRef.current = Date.now() + REVIEW_AUTO_REFRESH_INTERVAL_MS;
261-
setAutoRefreshSecondsRemaining(Math.ceil(REVIEW_AUTO_REFRESH_INTERVAL_MS / 1000));
262-
};
263-
264-
resetCountdown();
264+
if (!api || isCreating) return;
265265

266-
let lastRenderedSeconds: number | null = null;
266+
let debounceTimer: ReturnType<typeof setTimeout> | null = null;
267267

268-
const interval = setInterval(() => {
269-
const deadline = autoRefreshDeadlineRef.current;
270-
if (!deadline) return;
268+
const performRefresh = () => {
269+
// Skip if document not visible (user switched tabs/windows)
270+
if (document.hidden) return;
271271

272-
const msRemaining = deadline - Date.now();
273-
const secondsRemaining = Math.max(0, Math.ceil(msRemaining / 1000));
274-
if (secondsRemaining !== lastRenderedSeconds) {
275-
lastRenderedSeconds = secondsRemaining;
276-
setAutoRefreshSecondsRemaining(secondsRemaining);
272+
// Skip if user is actively entering a review note
273+
if (isUserInteractingRef.current) {
274+
pendingRefreshRef.current = true;
275+
return;
277276
}
278277

279-
// Fire when deadline passed (not when display shows 0)
280-
if (msRemaining > 0) return;
281-
if (isAutoRefreshing) return;
278+
// Skip if already refreshing (for origin/* bases with fetch)
279+
if (isRefreshingRef.current) return;
282280

283-
setIsAutoRefreshing(true);
284-
285-
// Reset early so we don't immediately re-fire if fetch takes time.
286-
resetCountdown();
281+
// Save scroll position before refresh
282+
savedScrollTopRef.current = scrollContainerRef.current?.scrollTop ?? null;
287283

288284
const originBranch = getOriginBranchForFetch(filters.diffBase);
289285
if (originBranch) {
290286
// Remote base: fetch before refreshing diff
287+
isRefreshingRef.current = true;
291288
api.workspace
292289
.executeBash({
293290
workspaceId,
@@ -298,22 +295,54 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
298295
console.debug("ReviewPanel origin fetch failed", err);
299296
})
300297
.finally(() => {
301-
setIsAutoRefreshing(false);
298+
isRefreshingRef.current = false;
302299
setRefreshTrigger((prev) => prev + 1);
303300
});
304301
} else {
305302
// Local base: just refresh diff
306-
setIsAutoRefreshing(false);
307303
setRefreshTrigger((prev) => prev + 1);
308304
}
309-
}, 250);
305+
};
306+
307+
const scheduleRefresh = () => {
308+
if (debounceTimer) clearTimeout(debounceTimer);
309+
debounceTimer = setTimeout(performRefresh, TOOL_REFRESH_DEBOUNCE_MS);
310+
};
311+
312+
const unsubscribe = workspaceStore.onToolCallEnd((wsId, toolName) => {
313+
if (wsId !== workspaceId) return;
314+
if (!isFileModifyingTool(toolName)) return;
315+
scheduleRefresh();
316+
});
310317

311318
return () => {
312-
clearInterval(interval);
313-
autoRefreshDeadlineRef.current = null;
314-
setAutoRefreshSecondsRemaining(null);
319+
unsubscribe();
320+
if (debounceTimer) clearTimeout(debounceTimer);
315321
};
316-
}, [api, workspaceId, filters.diffBase, isCreating, isAutoRefreshing]);
322+
}, [api, workspaceId, filters.diffBase, isCreating]);
323+
324+
// Sync panel focus with interaction tracking; fire pending refresh on blur
325+
useEffect(() => {
326+
isUserInteractingRef.current = isPanelFocused;
327+
328+
// When user stops interacting, fire any pending refresh
329+
if (!isPanelFocused && pendingRefreshRef.current) {
330+
pendingRefreshRef.current = false;
331+
handleRefreshRef.current();
332+
}
333+
}, [isPanelFocused]);
334+
335+
// Restore scroll position after auto-refresh completes
336+
useEffect(() => {
337+
if (
338+
diffState.status === "loaded" &&
339+
savedScrollTopRef.current !== null &&
340+
scrollContainerRef.current
341+
) {
342+
scrollContainerRef.current.scrollTop = savedScrollTopRef.current;
343+
savedScrollTopRef.current = null;
344+
}
345+
}, [diffState.status]);
317346

318347
// Focus panel when focusTrigger changes (preserves current hunk selection)
319348

@@ -323,18 +352,15 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
323352
handleRefreshRef.current = () => {
324353
if (!api || isCreating) return;
325354

326-
// Reset countdown on manual refresh so the user doesn't see an immediate auto-refresh.
327-
autoRefreshDeadlineRef.current = Date.now() + REVIEW_AUTO_REFRESH_INTERVAL_MS;
328-
setAutoRefreshSecondsRemaining(Math.ceil(REVIEW_AUTO_REFRESH_INTERVAL_MS / 1000));
355+
// Skip if already refreshing (for origin/* bases with fetch)
356+
if (isRefreshingRef.current) {
357+
setRefreshTrigger((prev) => prev + 1);
358+
return;
359+
}
329360

330361
const originBranch = getOriginBranchForFetch(filters.diffBase);
331362
if (originBranch) {
332-
if (isAutoRefreshing) {
333-
setRefreshTrigger((prev) => prev + 1);
334-
return;
335-
}
336-
337-
setIsAutoRefreshing(true);
363+
isRefreshingRef.current = true;
338364

339365
api.workspace
340366
.executeBash({
@@ -346,7 +372,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
346372
console.debug("ReviewPanel origin fetch failed", err);
347373
})
348374
.finally(() => {
349-
setIsAutoRefreshing(false);
375+
isRefreshingRef.current = false;
350376
setRefreshTrigger((prev) => prev + 1);
351377
});
352378

@@ -899,12 +925,8 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
899925
stats={stats}
900926
onFiltersChange={setFilters}
901927
onRefresh={handleRefresh}
902-
autoRefreshSecondsRemaining={autoRefreshSecondsRemaining}
903928
isLoading={
904-
diffState.status === "loading" ||
905-
diffState.status === "refreshing" ||
906-
isLoadingTree ||
907-
isAutoRefreshing
929+
diffState.status === "loading" || diffState.status === "refreshing" || isLoadingTree
908930
}
909931
workspaceId={workspaceId}
910932
workspacePath={workspacePath}
@@ -986,7 +1008,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
9861008
</div>
9871009

9881010
{/* Single scrollable area containing both file tree and hunks */}
989-
<div className="flex min-h-0 flex-1 flex-col overflow-y-auto">
1011+
<div ref={scrollContainerRef} className="flex min-h-0 flex-1 flex-col overflow-y-auto">
9901012
{/* FileTree at the top */}
9911013
{(fileTree ?? isLoadingTree) && (
9921014
<div className="border-border-light flex w-full flex-[0_0_auto] flex-col overflow-hidden border-b">

‎src/browser/stores/WorkspaceStore.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,11 @@ export class WorkspaceStore {
268268
// Idle compaction notification callbacks (called when backend signals idle compaction needed)
269269
private idleCompactionCallbacks = new Set<(workspaceId: string) => void>();
270270

271+
// Tool-call-end callbacks (for file-modifying tool completions that trigger diff refresh)
272+
private toolCallEndCallbacks = new Set<
273+
(workspaceId: string, toolName: string, toolCallId: string) => void
274+
>();
275+
271276
// Idle callback handles for high-frequency delta events to reduce re-renders during streaming.
272277
// Data is always updated immediately in the aggregator; only UI notification is scheduled.
273278
// Using requestIdleCallback adapts to actual CPU availability rather than a fixed timer.
@@ -394,6 +399,7 @@ export class WorkspaceStore {
394399
aggregator.handleToolCallEnd(data as never);
395400
this.states.bump(workspaceId);
396401
this.consumerManager.scheduleCalculation(workspaceId, aggregator);
402+
this.notifyToolCallEnd(workspaceId, toolCallEnd.toolName, toolCallEnd.toolCallId);
397403
},
398404
"reasoning-delta": (workspaceId, aggregator, data) => {
399405
aggregator.handleReasoningDelta(data as never);
@@ -1314,6 +1320,30 @@ export class WorkspaceStore {
13141320
}
13151321
}
13161322

1323+
/**
1324+
* Subscribe to tool-call-end events (for diff refresh on file modifications).
1325+
* Returns unsubscribe function.
1326+
*/
1327+
onToolCallEnd(
1328+
callback: (workspaceId: string, toolName: string, toolCallId: string) => void
1329+
): () => void {
1330+
this.toolCallEndCallbacks.add(callback);
1331+
return () => this.toolCallEndCallbacks.delete(callback);
1332+
}
1333+
1334+
/**
1335+
* Notify all listeners that a tool call completed.
1336+
*/
1337+
private notifyToolCallEnd(workspaceId: string, toolName: string, toolCallId: string): void {
1338+
for (const callback of this.toolCallEndCallbacks) {
1339+
try {
1340+
callback(workspaceId, toolName, toolCallId);
1341+
} catch (error) {
1342+
console.error("Error in tool-call-end callback:", error);
1343+
}
1344+
}
1345+
}
1346+
13171347
// Private methods
13181348

13191349
/**
@@ -1548,6 +1578,8 @@ function getStoreInstance(): WorkspaceStore {
15481578
export const workspaceStore = {
15491579
onIdleCompactionNeeded: (callback: (workspaceId: string) => void) =>
15501580
getStoreInstance().onIdleCompactionNeeded(callback),
1581+
onToolCallEnd: (callback: (workspaceId: string, toolName: string, toolCallId: string) => void) =>
1582+
getStoreInstance().onToolCallEnd(callback),
15511583
};
15521584

15531585
/**

0 commit comments

Comments
 (0)