Skip to content

Commit f464017

Browse files
committed
feat: show last refresh time and trigger in tooltip
Adds debugging visibility to ReviewPanel's refresh button tooltip: - Shows how long ago the last refresh occurred (e.g., '5s ago', '2m ago') - Shows what triggered the refresh (manual click, tool completion, focus, etc.) - Persisted per-workspace so it survives workspace switches RefreshController now uses rate-limiting instead of debouncing: - First event starts timer; subsequent events don't reset it - Guarantees refresh fires within N seconds of first trigger - Events during in-flight refresh queue a follow-up after completion - Better UX during rapid tool calls (refreshes periodically vs never) Trigger types tracked: - manual: user clicked refresh button - scheduled: rate-limited tool completion - priority: rate-limited tool completion (active workspace) - focus: window regained focus - visibility: tab became visible - unpaused: interaction ended, flushing pending - in-flight-followup: queued while previous refresh was running Also adds formatRelativeTimeCompact() to dateTime utils.
1 parent f415518 commit f464017

File tree

9 files changed

+432
-41
lines changed

9 files changed

+432
-41
lines changed

src/browser/components/RightSidebar/CodeReview/RefreshButton.tsx

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,30 @@
55
import React, { useState, useRef, useEffect } from "react";
66
import { Tooltip, TooltipTrigger, TooltipContent } from "@/browser/components/ui/tooltip";
77
import { formatKeybind, KEYBINDS } from "@/browser/utils/ui/keybinds";
8+
import { formatRelativeTimeCompact } from "@/browser/utils/ui/dateTime";
89
import { cn } from "@/common/lib/utils";
10+
import type { LastRefreshInfo, RefreshTrigger } from "@/browser/utils/RefreshController";
911

1012
interface RefreshButtonProps {
1113
onClick: () => void;
1214
isLoading?: boolean;
15+
/** Debug info about last refresh (timestamp and trigger) */
16+
lastRefreshInfo?: LastRefreshInfo | null;
1317
}
1418

15-
export const RefreshButton: React.FC<RefreshButtonProps> = ({ onClick, isLoading = false }) => {
19+
/** Human-readable trigger labels */
20+
const TRIGGER_LABELS: Record<RefreshTrigger, string> = {
21+
manual: "manual click",
22+
scheduled: "tool completion",
23+
priority: "tool completion (priority)",
24+
focus: "window focus",
25+
visibility: "tab visible",
26+
unpaused: "interaction ended",
27+
"in-flight-followup": "queued followup",
28+
};
29+
30+
export const RefreshButton: React.FC<RefreshButtonProps> = (props) => {
31+
const { onClick, isLoading = false, lastRefreshInfo } = props;
1632
// Track animation state for graceful stopping
1733
const [animationState, setAnimationState] = useState<"idle" | "spinning" | "stopping">("idle");
1834
const spinOnceTimeoutRef = useRef<number | null>(null);
@@ -76,9 +92,19 @@ export const RefreshButton: React.FC<RefreshButtonProps> = ({ onClick, isLoading
7692
</button>
7793
</TooltipTrigger>
7894
<TooltipContent side="bottom" align="start">
79-
{animationState !== "idle"
80-
? "Refreshing..."
81-
: `Refresh diff (${formatKeybind(KEYBINDS.REFRESH_REVIEW)})`}
95+
{animationState !== "idle" ? (
96+
"Refreshing..."
97+
) : (
98+
<span>
99+
Refresh diff ({formatKeybind(KEYBINDS.REFRESH_REVIEW)})
100+
{lastRefreshInfo && (
101+
<span className="text-muted block text-[10px]">
102+
Last: {formatRelativeTimeCompact(lastRefreshInfo.timestamp)} via{" "}
103+
{TRIGGER_LABELS[lastRefreshInfo.trigger]}
104+
</span>
105+
)}
106+
</span>
107+
)}
82108
</TooltipContent>
83109
</Tooltip>
84110
);

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import React, { useState } from "react";
66
import { usePersistedState } from "@/browser/hooks/usePersistedState";
77
import type { ReviewFilters, ReviewStats } from "@/common/types/review";
8+
import type { LastRefreshInfo } from "@/browser/utils/RefreshController";
89
import { RefreshButton } from "./RefreshButton";
910
import { UntrackedStatus } from "./UntrackedStatus";
1011

@@ -17,6 +18,8 @@ interface ReviewControlsProps {
1718
workspaceId: string;
1819
workspacePath: string;
1920
refreshTrigger?: number;
21+
/** Debug info about last refresh */
22+
lastRefreshInfo?: LastRefreshInfo | null;
2023
}
2124

2225
export const ReviewControls: React.FC<ReviewControlsProps> = ({
@@ -28,6 +31,7 @@ export const ReviewControls: React.FC<ReviewControlsProps> = ({
2831
workspaceId,
2932
workspacePath,
3033
refreshTrigger,
34+
lastRefreshInfo,
3135
}) => {
3236
// Local state for input value - only commit on blur/Enter
3337
const [inputValue, setInputValue] = useState(filters.diffBase);
@@ -83,7 +87,13 @@ export const ReviewControls: React.FC<ReviewControlsProps> = ({
8387

8488
return (
8589
<div className="bg-separator border-border-light flex flex-wrap items-center gap-3 border-b px-3 py-2 text-[11px]">
86-
{onRefresh && <RefreshButton onClick={onRefresh} isLoading={isLoading} />}
90+
{onRefresh && (
91+
<RefreshButton
92+
onClick={onRefresh}
93+
isLoading={isLoading}
94+
lastRefreshInfo={lastRefreshInfo}
95+
/>
96+
)}
8797
<label className="text-muted font-medium whitespace-nowrap">Base:</label>
8898
<input
8999
type="text"

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
845845
workspaceId={workspaceId}
846846
workspacePath={workspacePath}
847847
refreshTrigger={refreshTrigger}
848+
lastRefreshInfo={refreshController.lastRefreshInfo}
848849
/>
849850

850851
{diffState.status === "error" ? (

src/browser/hooks/useReviewRefreshController.ts

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { useEffect, useRef, useMemo } from "react";
22
import { workspaceStore } from "@/browser/stores/WorkspaceStore";
33
import type { APIClient } from "@/browser/contexts/API";
4-
import { RefreshController } from "@/browser/utils/RefreshController";
4+
import { RefreshController, type LastRefreshInfo } from "@/browser/utils/RefreshController";
5+
import { usePersistedState } from "@/browser/hooks/usePersistedState";
56

67
/** Debounce delay for auto-refresh after tool completion */
78
const TOOL_REFRESH_DEBOUNCE_MS = 3000;
@@ -43,6 +44,8 @@ export interface ReviewRefreshController {
4344
setInteracting: (interacting: boolean) => void;
4445
/** Whether a git fetch is currently in-flight */
4546
isRefreshing: boolean;
47+
/** Info about the last completed refresh (for debugging) */
48+
lastRefreshInfo: LastRefreshInfo | null;
4649
}
4750

4851
/**
@@ -57,22 +60,30 @@ export interface ReviewRefreshController {
5760
export function useReviewRefreshController(
5861
options: UseReviewRefreshControllerOptions
5962
): ReviewRefreshController {
60-
const { workspaceId, api, isCreating, onRefresh, scrollContainerRef, onGitStatusRefresh } =
61-
options;
63+
const { workspaceId, api, isCreating, scrollContainerRef } = options;
6264

6365
// Refs for values that executeRefresh needs at call time (avoid stale closures)
6466
const diffBaseRef = useRef(options.diffBase);
6567
diffBaseRef.current = options.diffBase;
6668

67-
const onGitStatusRefreshRef = useRef(onGitStatusRefresh);
68-
onGitStatusRefreshRef.current = onGitStatusRefresh;
69+
const onRefreshRef = useRef(options.onRefresh);
70+
onRefreshRef.current = options.onRefresh;
71+
72+
const onGitStatusRefreshRef = useRef(options.onGitStatusRefresh);
73+
onGitStatusRefreshRef.current = options.onGitStatusRefresh;
6974

7075
// Scroll position to restore after refresh
7176
const savedScrollTopRef = useRef<number | null>(null);
7277

7378
// User interaction state (pauses auto-refresh)
7479
const isInteractingRef = useRef(false);
7580

81+
// Track last refresh info - persisted per workspace so it survives workspace switches
82+
const [lastRefreshInfo, setLastRefreshInfo] = usePersistedState<LastRefreshInfo | null>(
83+
`review-last-refresh:${workspaceId}`,
84+
null
85+
);
86+
7687
// Create RefreshController once, with stable callbacks via refs
7788
const controller = useMemo(() => {
7889
const ctrl = new RefreshController({
@@ -97,14 +108,16 @@ export function useReviewRefreshController(
97108
}
98109
}
99110

100-
onRefresh();
111+
onRefreshRef.current();
101112
onGitStatusRefreshRef.current?.();
102113
},
114+
onRefreshComplete: setLastRefreshInfo,
103115
});
104116
ctrl.bindListeners();
105117
return ctrl;
106118
// workspaceId/api/isCreating changes require new controller with updated closure
107-
}, [workspaceId, api, isCreating, onRefresh, scrollContainerRef]);
119+
// Note: options.onRefresh is accessed via ref to avoid recreating controller on every render
120+
}, [workspaceId, api, isCreating, scrollContainerRef, setLastRefreshInfo]);
108121

109122
// Cleanup on unmount or when controller changes
110123
useEffect(() => {
@@ -115,12 +128,18 @@ export function useReviewRefreshController(
115128
useEffect(() => {
116129
if (!api || isCreating) return;
117130

118-
const unsubscribe = workspaceStore.subscribeFileModifyingTool(
119-
() => controller.schedule(),
120-
workspaceId
121-
);
122-
123-
return unsubscribe;
131+
console.debug(`[ReviewRefresh] subscribing to tool completions for ${workspaceId}`);
132+
const unsubscribe = workspaceStore.subscribeFileModifyingTool(() => {
133+
console.debug(
134+
`[ReviewRefresh] tool completion received for ${workspaceId}, calling schedule()`
135+
);
136+
controller.schedule();
137+
}, workspaceId);
138+
139+
return () => {
140+
console.debug(`[ReviewRefresh] unsubscribing from tool completions for ${workspaceId}`);
141+
unsubscribe();
142+
};
124143
}, [api, workspaceId, isCreating, controller]);
125144

126145
// Public API
@@ -144,6 +163,7 @@ export function useReviewRefreshController(
144163
get isRefreshing() {
145164
return controller.isRefreshing;
146165
},
166+
lastRefreshInfo,
147167
};
148168
}
149169

src/browser/stores/WorkspaceStore.test.ts

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,4 +773,151 @@ describe("WorkspaceStore", () => {
773773
expect(live).toBeNull();
774774
});
775775
});
776+
777+
describe("file-modifying tool subscription", () => {
778+
it("notifies subscribers when bash tool completes", async () => {
779+
const workspaceId = "file-modify-test-1";
780+
const notifications: string[] = [];
781+
782+
// Subscribe BEFORE creating workspace
783+
const unsubscribe = store.subscribeFileModifyingTool((wsId) => {
784+
notifications.push(wsId);
785+
}, workspaceId);
786+
787+
mockOnChat.mockImplementation(async function* (): AsyncGenerator<
788+
WorkspaceChatMessage,
789+
void,
790+
unknown
791+
> {
792+
yield { type: "caught-up" };
793+
await Promise.resolve();
794+
yield {
795+
type: "tool-call-end",
796+
workspaceId,
797+
messageId: "m1",
798+
toolCallId: "call-1",
799+
toolName: "bash",
800+
result: { success: true, output: "done", exitCode: 0, wall_duration_ms: 1 },
801+
timestamp: 1,
802+
};
803+
});
804+
805+
createAndAddWorkspace(store, workspaceId);
806+
await new Promise((resolve) => setTimeout(resolve, 10));
807+
808+
expect(notifications).toContain(workspaceId);
809+
expect(notifications.length).toBe(1);
810+
811+
unsubscribe();
812+
});
813+
814+
it("notifies subscribers when file_edit tool completes", async () => {
815+
const workspaceId = "file-modify-test-2";
816+
const notifications: string[] = [];
817+
818+
const unsubscribe = store.subscribeFileModifyingTool((wsId) => {
819+
notifications.push(wsId);
820+
}, workspaceId);
821+
822+
mockOnChat.mockImplementation(async function* (): AsyncGenerator<
823+
WorkspaceChatMessage,
824+
void,
825+
unknown
826+
> {
827+
yield { type: "caught-up" };
828+
await Promise.resolve();
829+
yield {
830+
type: "tool-call-end",
831+
workspaceId,
832+
messageId: "m1",
833+
toolCallId: "call-1",
834+
toolName: "file_edit_replace_string",
835+
result: { success: true },
836+
timestamp: 1,
837+
};
838+
});
839+
840+
createAndAddWorkspace(store, workspaceId);
841+
await new Promise((resolve) => setTimeout(resolve, 10));
842+
843+
expect(notifications).toContain(workspaceId);
844+
unsubscribe();
845+
});
846+
847+
it("does NOT notify for non-file-modifying tools", async () => {
848+
const workspaceId = "file-modify-test-3";
849+
const notifications: string[] = [];
850+
851+
const unsubscribe = store.subscribeFileModifyingTool((wsId) => {
852+
notifications.push(wsId);
853+
}, workspaceId);
854+
855+
mockOnChat.mockImplementation(async function* (): AsyncGenerator<
856+
WorkspaceChatMessage,
857+
void,
858+
unknown
859+
> {
860+
yield { type: "caught-up" };
861+
await Promise.resolve();
862+
yield {
863+
type: "tool-call-end",
864+
workspaceId,
865+
messageId: "m1",
866+
toolCallId: "call-1",
867+
toolName: "web_search", // Not file-modifying
868+
result: { success: true },
869+
timestamp: 1,
870+
};
871+
});
872+
873+
createAndAddWorkspace(store, workspaceId);
874+
await new Promise((resolve) => setTimeout(resolve, 10));
875+
876+
expect(notifications.length).toBe(0);
877+
unsubscribe();
878+
});
879+
880+
it("only notifies subscribers for the specific workspace", async () => {
881+
const workspaceId1 = "file-modify-test-4a";
882+
const workspaceId2 = "file-modify-test-4b";
883+
const notifications1: string[] = [];
884+
const notifications2: string[] = [];
885+
886+
const unsubscribe1 = store.subscribeFileModifyingTool((wsId) => {
887+
notifications1.push(wsId);
888+
}, workspaceId1);
889+
890+
const unsubscribe2 = store.subscribeFileModifyingTool((wsId) => {
891+
notifications2.push(wsId);
892+
}, workspaceId2);
893+
894+
// Only emit tool completion for workspace1
895+
mockOnChat.mockImplementation(async function* (): AsyncGenerator<
896+
WorkspaceChatMessage,
897+
void,
898+
unknown
899+
> {
900+
yield { type: "caught-up" };
901+
await Promise.resolve();
902+
yield {
903+
type: "tool-call-end",
904+
workspaceId: workspaceId1,
905+
messageId: "m1",
906+
toolCallId: "call-1",
907+
toolName: "bash",
908+
result: { success: true, output: "done", exitCode: 0, wall_duration_ms: 1 },
909+
timestamp: 1,
910+
};
911+
});
912+
913+
createAndAddWorkspace(store, workspaceId1);
914+
await new Promise((resolve) => setTimeout(resolve, 10));
915+
916+
expect(notifications1.length).toBe(1);
917+
expect(notifications2.length).toBe(0); // Not notified - different workspace
918+
919+
unsubscribe1();
920+
unsubscribe2();
921+
});
922+
});
776923
});

src/browser/stores/WorkspaceStore.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,9 @@ export class WorkspaceStore {
482482

483483
// Track file-modifying tools for ReviewPanel diff refresh
484484
if (toolCallEnd.toolName.startsWith("file_edit_") || toolCallEnd.toolName === "bash") {
485+
console.debug(
486+
`[WorkspaceStore] file-modifying tool completed: ${toolCallEnd.toolName} for ${workspaceId}`
487+
);
485488
this.fileModifyingToolMs.set(workspaceId, Date.now());
486489
this.fileModifyingToolSubs.bump(workspaceId);
487490
}

0 commit comments

Comments
 (0)