diff --git a/src/browser/components/RightSidebar/CodeReview/ReviewControls.tsx b/src/browser/components/RightSidebar/CodeReview/ReviewControls.tsx index 38ccde421e..6946a1332e 100644 --- a/src/browser/components/RightSidebar/CodeReview/ReviewControls.tsx +++ b/src/browser/components/RightSidebar/CodeReview/ReviewControls.tsx @@ -16,8 +16,6 @@ interface ReviewControlsProps { isLoading?: boolean; workspaceId: string; workspacePath: string; - /** If set, show an auto-refresh countdown (e.g., for origin/* bases). */ - autoRefreshSecondsRemaining?: number | null; refreshTrigger?: number; } @@ -29,7 +27,6 @@ export const ReviewControls: React.FC = ({ isLoading = false, workspaceId, workspacePath, - autoRefreshSecondsRemaining, refreshTrigger, }) => { // Local state for input value - only commit on blur/Enter @@ -86,11 +83,6 @@ export const ReviewControls: React.FC = ({ return (
- {autoRefreshSecondsRemaining != null && ( - - Refresh in: {String(autoRefreshSecondsRemaining).padStart(2, "\u2007")}s - - )} {onRefresh && } >; -const REVIEW_AUTO_REFRESH_INTERVAL_MS = 30_000; +/** Check if a tool may modify files and should trigger diff refresh */ +function isFileModifyingTool(toolName: string): boolean { + return toolName.startsWith("file_edit_") || toolName === "bash"; +} + +/** Debounce delay for auto-refresh after tool completion */ +const TOOL_REFRESH_DEBOUNCE_MS = 3000; function getOriginBranchForFetch(diffBase: string): string | null { const trimmed = diffBase.trim(); @@ -175,6 +182,7 @@ export const ReviewPanel: React.FC = ({ }) => { const { api } = useAPI(); const panelRef = useRef(null); + const scrollContainerRef = useRef(null); const searchInputRef = useRef(null); // Unified diff state - discriminated union makes invalid states unrepresentable @@ -234,60 +242,49 @@ export const ReviewPanel: React.FC = ({ [diffState] ); - const [autoRefreshSecondsRemaining, setAutoRefreshSecondsRemaining] = useState( - null - ); - const [isAutoRefreshing, setIsAutoRefreshing] = useState(false); - const autoRefreshDeadlineRef = useRef(null); + // Track whether refresh is in-flight (for origin/* fetch) + const isRefreshingRef = useRef(false); + + // Track user interaction with review notes (pause auto-refresh while focused) + const isUserInteractingRef = useRef(false); + const pendingRefreshRef = useRef(false); + + // Save scroll position before refresh to restore after + const savedScrollTopRef = useRef(null); + const [filters, setFilters] = useState({ showReadHunks: showReadHunks, diffBase: diffBase, includeUncommitted: includeUncommitted, }); - // Auto-refresh diffs every 30s (with a user-visible countdown). - // For origin/* bases, fetches from remote first to pick up upstream changes. + // Auto-refresh on file-modifying tool completions (debounced 3s). + // Respects user interaction - if user is focused on review input, queues refresh for after blur. useEffect(() => { - if (!api || isCreating) { - autoRefreshDeadlineRef.current = null; - setAutoRefreshSecondsRemaining(null); - return; - } - - autoRefreshDeadlineRef.current = Date.now() + REVIEW_AUTO_REFRESH_INTERVAL_MS; - - const resetCountdown = () => { - autoRefreshDeadlineRef.current = Date.now() + REVIEW_AUTO_REFRESH_INTERVAL_MS; - setAutoRefreshSecondsRemaining(Math.ceil(REVIEW_AUTO_REFRESH_INTERVAL_MS / 1000)); - }; - - resetCountdown(); + if (!api || isCreating) return; - let lastRenderedSeconds: number | null = null; + let debounceTimer: ReturnType | null = null; - const interval = setInterval(() => { - const deadline = autoRefreshDeadlineRef.current; - if (!deadline) return; + const performRefresh = () => { + // Skip if document not visible (user switched tabs/windows) + if (document.hidden) return; - const msRemaining = deadline - Date.now(); - const secondsRemaining = Math.max(0, Math.ceil(msRemaining / 1000)); - if (secondsRemaining !== lastRenderedSeconds) { - lastRenderedSeconds = secondsRemaining; - setAutoRefreshSecondsRemaining(secondsRemaining); + // Skip if user is actively entering a review note + if (isUserInteractingRef.current) { + pendingRefreshRef.current = true; + return; } - // Fire when deadline passed (not when display shows 0) - if (msRemaining > 0) return; - if (isAutoRefreshing) return; + // Skip if already refreshing (for origin/* bases with fetch) + if (isRefreshingRef.current) return; - setIsAutoRefreshing(true); - - // Reset early so we don't immediately re-fire if fetch takes time. - resetCountdown(); + // Save scroll position before refresh + savedScrollTopRef.current = scrollContainerRef.current?.scrollTop ?? null; const originBranch = getOriginBranchForFetch(filters.diffBase); if (originBranch) { // Remote base: fetch before refreshing diff + isRefreshingRef.current = true; api.workspace .executeBash({ workspaceId, @@ -298,22 +295,54 @@ export const ReviewPanel: React.FC = ({ console.debug("ReviewPanel origin fetch failed", err); }) .finally(() => { - setIsAutoRefreshing(false); + isRefreshingRef.current = false; setRefreshTrigger((prev) => prev + 1); }); } else { // Local base: just refresh diff - setIsAutoRefreshing(false); setRefreshTrigger((prev) => prev + 1); } - }, 250); + }; + + const scheduleRefresh = () => { + if (debounceTimer) clearTimeout(debounceTimer); + debounceTimer = setTimeout(performRefresh, TOOL_REFRESH_DEBOUNCE_MS); + }; + + const unsubscribe = workspaceStore.onToolCallEnd((wsId, toolName) => { + if (wsId !== workspaceId) return; + if (!isFileModifyingTool(toolName)) return; + scheduleRefresh(); + }); return () => { - clearInterval(interval); - autoRefreshDeadlineRef.current = null; - setAutoRefreshSecondsRemaining(null); + unsubscribe(); + if (debounceTimer) clearTimeout(debounceTimer); }; - }, [api, workspaceId, filters.diffBase, isCreating, isAutoRefreshing]); + }, [api, workspaceId, filters.diffBase, isCreating]); + + // Sync panel focus with interaction tracking; fire pending refresh on blur + useEffect(() => { + isUserInteractingRef.current = isPanelFocused; + + // When user stops interacting, fire any pending refresh + if (!isPanelFocused && pendingRefreshRef.current) { + pendingRefreshRef.current = false; + handleRefreshRef.current(); + } + }, [isPanelFocused]); + + // Restore scroll position after auto-refresh completes + useEffect(() => { + if ( + diffState.status === "loaded" && + savedScrollTopRef.current !== null && + scrollContainerRef.current + ) { + scrollContainerRef.current.scrollTop = savedScrollTopRef.current; + savedScrollTopRef.current = null; + } + }, [diffState.status]); // Focus panel when focusTrigger changes (preserves current hunk selection) @@ -323,18 +352,15 @@ export const ReviewPanel: React.FC = ({ handleRefreshRef.current = () => { if (!api || isCreating) return; - // Reset countdown on manual refresh so the user doesn't see an immediate auto-refresh. - autoRefreshDeadlineRef.current = Date.now() + REVIEW_AUTO_REFRESH_INTERVAL_MS; - setAutoRefreshSecondsRemaining(Math.ceil(REVIEW_AUTO_REFRESH_INTERVAL_MS / 1000)); + // Skip if already refreshing (for origin/* bases with fetch) + if (isRefreshingRef.current) { + setRefreshTrigger((prev) => prev + 1); + return; + } const originBranch = getOriginBranchForFetch(filters.diffBase); if (originBranch) { - if (isAutoRefreshing) { - setRefreshTrigger((prev) => prev + 1); - return; - } - - setIsAutoRefreshing(true); + isRefreshingRef.current = true; api.workspace .executeBash({ @@ -346,7 +372,7 @@ export const ReviewPanel: React.FC = ({ console.debug("ReviewPanel origin fetch failed", err); }) .finally(() => { - setIsAutoRefreshing(false); + isRefreshingRef.current = false; setRefreshTrigger((prev) => prev + 1); }); @@ -899,12 +925,8 @@ export const ReviewPanel: React.FC = ({ stats={stats} onFiltersChange={setFilters} onRefresh={handleRefresh} - autoRefreshSecondsRemaining={autoRefreshSecondsRemaining} isLoading={ - diffState.status === "loading" || - diffState.status === "refreshing" || - isLoadingTree || - isAutoRefreshing + diffState.status === "loading" || diffState.status === "refreshing" || isLoadingTree } workspaceId={workspaceId} workspacePath={workspacePath} @@ -986,7 +1008,7 @@ export const ReviewPanel: React.FC = ({
{/* Single scrollable area containing both file tree and hunks */} -
+
{/* FileTree at the top */} {(fileTree ?? isLoadingTree) && (
diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index 3afff97b9c..3bec1766a1 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -268,6 +268,11 @@ export class WorkspaceStore { // Idle compaction notification callbacks (called when backend signals idle compaction needed) private idleCompactionCallbacks = new Set<(workspaceId: string) => void>(); + // Tool-call-end callbacks (for file-modifying tool completions that trigger diff refresh) + private toolCallEndCallbacks = new Set< + (workspaceId: string, toolName: string, toolCallId: string) => void + >(); + // Idle callback handles for high-frequency delta events to reduce re-renders during streaming. // Data is always updated immediately in the aggregator; only UI notification is scheduled. // Using requestIdleCallback adapts to actual CPU availability rather than a fixed timer. @@ -394,6 +399,7 @@ export class WorkspaceStore { aggregator.handleToolCallEnd(data as never); this.states.bump(workspaceId); this.consumerManager.scheduleCalculation(workspaceId, aggregator); + this.notifyToolCallEnd(workspaceId, toolCallEnd.toolName, toolCallEnd.toolCallId); }, "reasoning-delta": (workspaceId, aggregator, data) => { aggregator.handleReasoningDelta(data as never); @@ -1314,6 +1320,30 @@ export class WorkspaceStore { } } + /** + * Subscribe to tool-call-end events (for diff refresh on file modifications). + * Returns unsubscribe function. + */ + onToolCallEnd( + callback: (workspaceId: string, toolName: string, toolCallId: string) => void + ): () => void { + this.toolCallEndCallbacks.add(callback); + return () => this.toolCallEndCallbacks.delete(callback); + } + + /** + * Notify all listeners that a tool call completed. + */ + private notifyToolCallEnd(workspaceId: string, toolName: string, toolCallId: string): void { + for (const callback of this.toolCallEndCallbacks) { + try { + callback(workspaceId, toolName, toolCallId); + } catch (error) { + console.error("Error in tool-call-end callback:", error); + } + } + } + // Private methods /** @@ -1548,6 +1578,8 @@ function getStoreInstance(): WorkspaceStore { export const workspaceStore = { onIdleCompactionNeeded: (callback: (workspaceId: string) => void) => getStoreInstance().onIdleCompactionNeeded(callback), + onToolCallEnd: (callback: (workspaceId: string, toolName: string, toolCallId: string) => void) => + getStoreInstance().onToolCallEnd(callback), }; /**