From cac2043a4d9c1c161d0cbc61770478f1b99726ba Mon Sep 17 00:00:00 2001 From: Ammar Date: Sat, 20 Dec 2025 13:39:51 -0600 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20fix:=20auto-refresh=20ReviewPane?= =?UTF-8?q?l=20when=20workspace=20opened=20after=20tool=20completion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When file-modifying tools (file_edit_*, bash) complete while a workspace's ReviewPanel is not mounted, the panel now correctly fetches fresh data on mount instead of using stale cached data. ## Changes - Track file-modifying tool completion timestamps in WorkspaceStore - ReviewPanel checks timestamp on mount to skip cache if tools ran while closed - Simplified from dual mechanism (callbacks + timestamps) to single MapStore-based approach ## How it works 1. Tool completes → store timestamp + bump MapStore subscription 2. If panel open: subscription fires → debounced refresh 3. If panel was closed: on mount, detects timestamp → skips cache → fresh fetch --- _Generated with `mux` • Model: `anthropic:claude-opus-4-5` • Thinking: `high`_ --- .../RightSidebar/CodeReview/ReviewPanel.tsx | 35 +++++++----- src/browser/stores/WorkspaceStore.ts | 56 +++++++++++-------- 2 files changed, 54 insertions(+), 37 deletions(-) diff --git a/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx b/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx index a3574b9d4..a127b85b6 100644 --- a/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx +++ b/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx @@ -127,11 +127,6 @@ function makeReviewPanelCacheKey(params: { type ExecuteBashResult = Awaited>; -/** 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; @@ -204,6 +199,12 @@ export const ReviewPanel: React.FC = ({ const lastDiffRefreshTriggerRef = useRef(null); const lastFileTreeRefreshTriggerRef = useRef(null); + // Check if tools completed while we were unmounted - skip cache on initial mount if so. + // Computed once on mount, consumed after first load to avoid re-fetching on every mount. + const skipCacheOnMountRef = useRef( + workspaceStore.getFileModifyingToolMs(workspaceId) !== undefined + ); + // Unified search state (per-workspace persistence) const [searchState, setSearchState] = usePersistedState( getReviewSearchStateKey(workspaceId), @@ -309,11 +310,8 @@ export const ReviewPanel: React.FC = ({ debounceTimer = setTimeout(performRefresh, TOOL_REFRESH_DEBOUNCE_MS); }; - const unsubscribe = workspaceStore.onToolCallEnd((wsId, toolName) => { - if (wsId !== workspaceId) return; - if (!isFileModifyingTool(toolName)) return; - scheduleRefresh(); - }); + // Subscribe to file-modifying tool completions for this workspace + const unsubscribe = workspaceStore.subscribeFileModifyingTool(workspaceId, scheduleRefresh); return () => { unsubscribe(); @@ -422,8 +420,9 @@ export const ReviewPanel: React.FC = ({ gitCommand: numstatCommand, }); - // Fast path: use cached tree when switching workspaces (unless user explicitly refreshed). - if (!isManualRefresh) { + // Fast path: use cached tree when switching workspaces (unless user explicitly refreshed + // or tools completed while we were unmounted). + if (!isManualRefresh && !skipCacheOnMountRef.current) { const cachedTree = reviewPanelCache.get(cacheKey) as FileTreeNode | undefined; if (cachedTree) { setFileTree(cachedTree); @@ -503,8 +502,9 @@ export const ReviewPanel: React.FC = ({ gitCommand: diffCommand, }); - // Fast path: use cached diff when switching workspaces (unless user explicitly refreshed). - if (!isManualRefresh) { + // Fast path: use cached diff when switching workspaces (unless user explicitly refreshed + // or tools completed while we were unmounted). + if (!isManualRefresh && !skipCacheOnMountRef.current) { const cached = reviewPanelCache.get(cacheKey) as ReviewPanelDiffCacheValue | undefined; if (cached) { setDiagnosticInfo(cached.diagnosticInfo); @@ -524,6 +524,13 @@ export const ReviewPanel: React.FC = ({ } } + // Clear the skip-cache flag and store timestamp after first load. + // This prevents re-fetching on every filter change. + if (skipCacheOnMountRef.current) { + skipCacheOnMountRef.current = false; + workspaceStore.clearFileModifyingToolMs(workspaceId); + } + // Transition to appropriate loading state: // - "refreshing" if we have data (keeps UI stable during refresh) // - "loading" if no data yet diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index 3bec1766a..f472790e5 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -268,10 +268,12 @@ 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 - >(); + // Tracks when a file-modifying tool (file_edit_*, bash) last completed per workspace. + // ReviewPanel subscribes to trigger diff refresh. Two structures: + // - timestamps: actual Date.now() values for cache invalidation checks + // - subscriptions: MapStore for per-workspace subscription support + private fileModifyingToolMs = new Map(); + private fileModifyingToolSubs = new MapStore(); // 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. @@ -399,7 +401,12 @@ export class WorkspaceStore { aggregator.handleToolCallEnd(data as never); this.states.bump(workspaceId); this.consumerManager.scheduleCalculation(workspaceId, aggregator); - this.notifyToolCallEnd(workspaceId, toolCallEnd.toolName, toolCallEnd.toolCallId); + + // Track file-modifying tools for ReviewPanel diff refresh + if (toolCallEnd.toolName.startsWith("file_edit_") || toolCallEnd.toolName === "bash") { + this.fileModifyingToolMs.set(workspaceId, Date.now()); + this.fileModifyingToolSubs.bump(workspaceId); + } }, "reasoning-delta": (workspaceId, aggregator, data) => { aggregator.handleReasoningDelta(data as never); @@ -1321,27 +1328,26 @@ export class WorkspaceStore { } /** - * Subscribe to tool-call-end events (for diff refresh on file modifications). - * Returns unsubscribe function. + * Subscribe to file-modifying tool completions for a workspace. + * Used by ReviewPanel to trigger diff refresh. */ - onToolCallEnd( - callback: (workspaceId: string, toolName: string, toolCallId: string) => void - ): () => void { - this.toolCallEndCallbacks.add(callback); - return () => this.toolCallEndCallbacks.delete(callback); + subscribeFileModifyingTool(workspaceId: string, listener: () => void): () => void { + return this.fileModifyingToolSubs.subscribeKey(workspaceId, listener); } /** - * Notify all listeners that a tool call completed. + * Get when a file-modifying tool last completed for this workspace. + * Returns undefined if no tools have completed since last clear. */ - 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); - } - } + getFileModifyingToolMs(workspaceId: string): number | undefined { + return this.fileModifyingToolMs.get(workspaceId); + } + + /** + * Clear the file-modifying tool timestamp after ReviewPanel has consumed it. + */ + clearFileModifyingToolMs(workspaceId: string): void { + this.fileModifyingToolMs.delete(workspaceId); } // Private methods @@ -1578,8 +1584,12 @@ 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), + subscribeFileModifyingTool: (workspaceId: string, listener: () => void) => + getStoreInstance().subscribeFileModifyingTool(workspaceId, listener), + getFileModifyingToolMs: (workspaceId: string) => + getStoreInstance().getFileModifyingToolMs(workspaceId), + clearFileModifyingToolMs: (workspaceId: string) => + getStoreInstance().clearFileModifyingToolMs(workspaceId), }; /**