Skip to content

Commit 6e7fef5

Browse files
committed
🤖 refactor: replace countdown auto-refresh with tool-completion-driven refresh
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
1 parent b98b323 commit 6e7fef5

File tree

3 files changed

+94
-70
lines changed

3 files changed

+94
-70
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: 62 additions & 62 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();
@@ -234,60 +241,40 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
234241
[diffState]
235242
);
236243

237-
const [autoRefreshSecondsRemaining, setAutoRefreshSecondsRemaining] = useState<number | null>(
238-
null
239-
);
240-
const [isAutoRefreshing, setIsAutoRefreshing] = useState(false);
241-
const autoRefreshDeadlineRef = useRef<number | null>(null);
244+
// Track whether refresh is in-flight (for origin/* fetch)
245+
const isRefreshingRef = useRef(false);
246+
247+
// Track user interaction with review notes (pause auto-refresh while focused)
248+
const isUserInteractingRef = useRef(false);
249+
const pendingRefreshRef = useRef(false);
250+
242251
const [filters, setFilters] = useState<ReviewFiltersType>({
243252
showReadHunks: showReadHunks,
244253
diffBase: diffBase,
245254
includeUncommitted: includeUncommitted,
246255
});
247256

248-
// Auto-refresh diffs every 30s (with a user-visible countdown).
249-
// For origin/* bases, fetches from remote first to pick up upstream changes.
257+
// Auto-refresh on file-modifying tool completions (debounced 3s).
258+
// Respects user interaction - if user is focused on review input, queues refresh for after blur.
250259
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();
265-
266-
let lastRenderedSeconds: number | null = null;
260+
if (!api || isCreating) return;
267261

268-
const interval = setInterval(() => {
269-
const deadline = autoRefreshDeadlineRef.current;
270-
if (!deadline) return;
262+
let debounceTimer: ReturnType<typeof setTimeout> | null = null;
271263

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);
264+
const performRefresh = () => {
265+
// Skip if user is actively entering a review note
266+
if (isUserInteractingRef.current) {
267+
pendingRefreshRef.current = true;
268+
return;
277269
}
278270

279-
// Fire when deadline passed (not when display shows 0)
280-
if (msRemaining > 0) return;
281-
if (isAutoRefreshing) return;
282-
283-
setIsAutoRefreshing(true);
284-
285-
// Reset early so we don't immediately re-fire if fetch takes time.
286-
resetCountdown();
271+
// Skip if already refreshing (for origin/* bases with fetch)
272+
if (isRefreshingRef.current) return;
287273

288274
const originBranch = getOriginBranchForFetch(filters.diffBase);
289275
if (originBranch) {
290276
// Remote base: fetch before refreshing diff
277+
isRefreshingRef.current = true;
291278
api.workspace
292279
.executeBash({
293280
workspaceId,
@@ -298,22 +285,42 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
298285
console.debug("ReviewPanel origin fetch failed", err);
299286
})
300287
.finally(() => {
301-
setIsAutoRefreshing(false);
288+
isRefreshingRef.current = false;
302289
setRefreshTrigger((prev) => prev + 1);
303290
});
304291
} else {
305292
// Local base: just refresh diff
306-
setIsAutoRefreshing(false);
307293
setRefreshTrigger((prev) => prev + 1);
308294
}
309-
}, 250);
295+
};
296+
297+
const scheduleRefresh = () => {
298+
if (debounceTimer) clearTimeout(debounceTimer);
299+
debounceTimer = setTimeout(performRefresh, TOOL_REFRESH_DEBOUNCE_MS);
300+
};
301+
302+
const unsubscribe = workspaceStore.onToolCallEnd((wsId, toolName) => {
303+
if (wsId !== workspaceId) return;
304+
if (!isFileModifyingTool(toolName)) return;
305+
scheduleRefresh();
306+
});
310307

311308
return () => {
312-
clearInterval(interval);
313-
autoRefreshDeadlineRef.current = null;
314-
setAutoRefreshSecondsRemaining(null);
309+
unsubscribe();
310+
if (debounceTimer) clearTimeout(debounceTimer);
315311
};
316-
}, [api, workspaceId, filters.diffBase, isCreating, isAutoRefreshing]);
312+
}, [api, workspaceId, filters.diffBase, isCreating]);
313+
314+
// Sync panel focus with interaction tracking; fire pending refresh on blur
315+
useEffect(() => {
316+
isUserInteractingRef.current = isPanelFocused;
317+
318+
// When user stops interacting, fire any pending refresh
319+
if (!isPanelFocused && pendingRefreshRef.current) {
320+
pendingRefreshRef.current = false;
321+
handleRefreshRef.current();
322+
}
323+
}, [isPanelFocused]);
317324

318325
// Focus panel when focusTrigger changes (preserves current hunk selection)
319326

@@ -323,18 +330,15 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
323330
handleRefreshRef.current = () => {
324331
if (!api || isCreating) return;
325332

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));
333+
// Skip if already refreshing (for origin/* bases with fetch)
334+
if (isRefreshingRef.current) {
335+
setRefreshTrigger((prev) => prev + 1);
336+
return;
337+
}
329338

330339
const originBranch = getOriginBranchForFetch(filters.diffBase);
331340
if (originBranch) {
332-
if (isAutoRefreshing) {
333-
setRefreshTrigger((prev) => prev + 1);
334-
return;
335-
}
336-
337-
setIsAutoRefreshing(true);
341+
isRefreshingRef.current = true;
338342

339343
api.workspace
340344
.executeBash({
@@ -346,7 +350,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
346350
console.debug("ReviewPanel origin fetch failed", err);
347351
})
348352
.finally(() => {
349-
setIsAutoRefreshing(false);
353+
isRefreshingRef.current = false;
350354
setRefreshTrigger((prev) => prev + 1);
351355
});
352356

@@ -899,12 +903,8 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
899903
stats={stats}
900904
onFiltersChange={setFilters}
901905
onRefresh={handleRefresh}
902-
autoRefreshSecondsRemaining={autoRefreshSecondsRemaining}
903906
isLoading={
904-
diffState.status === "loading" ||
905-
diffState.status === "refreshing" ||
906-
isLoadingTree ||
907-
isAutoRefreshing
907+
diffState.status === "loading" || diffState.status === "refreshing" || isLoadingTree
908908
}
909909
workspaceId={workspaceId}
910910
workspacePath={workspacePath}

‎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)