Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,6 @@ function makeReviewPanelCacheKey(params: {

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

/** 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;

Expand Down Expand Up @@ -204,6 +199,12 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
const lastDiffRefreshTriggerRef = useRef<number | null>(null);
const lastFileTreeRefreshTriggerRef = useRef<number | null>(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<ReviewSearchState>(
getReviewSearchStateKey(workspaceId),
Expand Down Expand Up @@ -309,11 +310,8 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
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();
Expand Down Expand Up @@ -422,8 +420,9 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
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);
Expand Down Expand Up @@ -503,8 +502,9 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
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);
Expand All @@ -524,6 +524,13 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
}
}

// 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
Expand Down
56 changes: 33 additions & 23 deletions src/browser/stores/WorkspaceStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, number>();
private fileModifyingToolSubs = new MapStore<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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
};

/**
Expand Down