diff --git a/.changeset/nk0rpv31.md b/.changeset/nk0rpv31.md new file mode 100644 index 0000000..60029a7 --- /dev/null +++ b/.changeset/nk0rpv31.md @@ -0,0 +1,5 @@ +--- +"diffhub": patch +--- + +Stop the diff viewer from auto-refreshing. A background detector now surfaces an "Updates available" indicator on the refresh button instead of reloading the diff on its own, so the view only changes when you refresh (button or `R`). Also fixed the sidebar file tree flashing on diff updates by rebuilding it only when the set of files changes. diff --git a/apps/cli/components/DiffApp.test.tsx b/apps/cli/components/DiffApp.test.tsx index 551cc3f..6e5d7f9 100644 --- a/apps/cli/components/DiffApp.test.tsx +++ b/apps/cli/components/DiffApp.test.tsx @@ -541,7 +541,8 @@ describe("DiffApp review flow", () => { }); }); - it("refetches files and diff when the watch stream reports a change", async () => { + it("flags updates available on a watch-stream change and refreshes on demand", async () => { + const user = userEvent.setup(); let version = 1; const fetchMock = vi.fn((input, init) => { const url = typeof input === "string" ? input : input.toString(); @@ -585,23 +586,31 @@ describe("DiffApp review flow", () => { expect(countFetchCalls(fetchMock, "/api/diff")).toBe(1); }); FakeEventSource.instances[0]?.emitReady(); - await screen.findByRole("button", { name: /Live.*force refresh diff/i }); + await screen.findByRole("button", { name: /Up to date.*force refresh diff/i }); + // A change on disk raises the indicator but must NOT refresh the view. version = 2; + const diffCallsBeforeChange = countFetchCalls(fetchMock, "/api/diff"); FakeEventSource.instances[0]?.emitChange(); + await screen.findByRole("button", { name: /Updates available.*force refresh diff/i }); + expect(screen.queryByText(/watched/)).toBeNull(); + expect(countFetchCalls(fetchMock, "/api/diff")).toBe(diffCallsBeforeChange); + + // Manual refresh pulls in the new diff and clears the indicator. + await user.click(screen.getByRole("button", { name: /force refresh diff/i })); + await screen.findByText(/watched/); - await screen.findByRole("button", { name: /Updated just now.*force refresh diff/i }); - expect(countFetchCalls(fetchMock, "/api/files")).toBeGreaterThanOrEqual(2); - expect(countFetchCalls(fetchMock, "/api/comments")).toBe(1); - expect(countFetchCalls(fetchMock, "/api/diff")).toBeGreaterThanOrEqual(2); + await screen.findByRole("button", { name: /Up to date.*force refresh diff/i }); + expect(countFetchCalls(fetchMock, "/api/diff")).toBeGreaterThan(diffCallsBeforeChange); expect(FakeEventSource.instances[0]?.url).toBe("/api/watch"); unmount(); expect(FakeEventSource.instances[0]?.close).toHaveBeenCalledOnce(); }); - it("supports a polling fallback without opening EventSource", async () => { + it("flags updates available via polling without opening EventSource", async () => { + const user = userEvent.setup(); let version = 1; const fetchMock = vi.fn((input, init) => { const url = typeof input === "string" ? input : input.toString(); @@ -642,19 +651,26 @@ describe("DiffApp review flow", () => { await screen.findByText("feature/diff-review"); await waitFor(() => { - expect(countFetchCalls(fetchMock, "/api/diff")).toBeGreaterThanOrEqual(1); + expect(countFetchCalls(fetchMock, "/api/diff")).toBe(1); }); - await screen.findByRole("button", { name: /Live.*force refresh diff/i }); + await screen.findByRole("button", { name: /Up to date.*force refresh diff/i }); + // Polling notices the change and raises the indicator without re-streaming. version = 2; - - await screen.findByText(/polled/); - await screen.findByRole("button", { name: /Updated just now.*force refresh diff/i }); + await screen.findByRole("button", { name: /Updates available.*force refresh diff/i }); expect(FakeEventSource.instances).toHaveLength(0); + expect(screen.queryByText(/polled/)).toBeNull(); + expect(countFetchCalls(fetchMock, "/api/diff")).toBe(1); expect( fetchMock.mock.calls.some(([input]) => String(input).startsWith("/api/files?refresh=1")), ).toBeTruthy(); - expect(countFetchCalls(fetchMock, "/api/comments")).toBe(1); + + // Manual refresh applies the pending change. + await user.click(screen.getByRole("button", { name: /force refresh diff/i })); + + await screen.findByText(/polled/); + await screen.findByRole("button", { name: /Up to date.*force refresh diff/i }); + expect(countFetchCalls(fetchMock, "/api/comments")).toBe(2); unmount(); }); diff --git a/apps/cli/components/DiffApp.tsx b/apps/cli/components/DiffApp.tsx index c78e01b..eab200a 100644 --- a/apps/cli/components/DiffApp.tsx +++ b/apps/cli/components/DiffApp.tsx @@ -12,8 +12,8 @@ import { toCommentSide } from "@/lib/comment-sides"; import type { Comment, CommentTag } from "@/lib/comment-types"; import { exportCommentsAsPrompt } from "@/lib/export-comments"; import { useLocalStorage } from "@/lib/use-local-storage"; -import { getWatchStatusMeta } from "@/lib/watch-status"; -import type { WatchStatus } from "@/lib/watch-status"; +import { getRefreshStatusMeta } from "@/lib/watch-status"; +import type { WatchHealth } from "@/lib/watch-status"; import { WATCH_STREAM_EVENTS } from "@/lib/watch-stream"; import type { DisplaySettings, DiffThemeSelection } from "@diffhub/diff-core"; import { @@ -284,8 +284,14 @@ export const DiffApp = ({ [setStatsPanel], ); const [refreshing, setRefreshing] = useState(false); - const [watchStatus, setWatchStatus] = useState("connecting"); - const watchStatusTimerRef = useRef | null>(null); + // Connection health of the background change-detector (it never refreshes the + // diff itself — see `checkForUpdates`). + const [watchStatus, setWatchStatus] = useState("connecting"); + // Set when the background detector notices the diff changed on disk; cleared + // by a manual refresh. Drives the status-bar "updates available" indicator. + const [updatesAvailable, setUpdatesAvailable] = useState(false); + const updatesAvailableRef = useRef(false); + const checkingRef = useRef(false); const [loadError, setLoadError] = useState(null); const [diffMode, setDiffMode] = useLocalStorage( "diffhub-diffMode", @@ -469,6 +475,10 @@ export const DiffApp = ({ } const nextFilesData = (await filesResponse.json()) as FilesData; + // The view is about to reflect the latest diff, so any pending + // "updates available" indicator is now resolved. + updatesAvailableRef.current = false; + setUpdatesAvailable(false); const shouldUpdateFiles = !areFilesDataEqual(filesDataRef.current, nextFilesData); const shouldUpdateComments = nextComments !== null && !areCommentsEqual(commentsRef.current, nextComments); @@ -500,6 +510,45 @@ export const DiffApp = ({ pollFilesRef.current = pollFiles; }, [pollFiles]); + // Background change-detector. Fetches the lightweight file-stats payload and, + // if the diff differs from what's on screen, raises the "updates available" + // flag WITHOUT touching the rendered diff — so the sidebar never flashes and + // the view only changes when the user manually refreshes. + const checkForUpdates = useCallback(async () => { + // No baseline yet (initial load in flight), a manual refresh is running, a + // check is already in flight, or the indicator is already raised. + if ( + filesDataRef.current === null || + fetchingRef.current || + checkingRef.current || + updatesAvailableRef.current + ) { + return; + } + + checkingRef.current = true; + try { + const response = await fetch(`/api/files${buildFilesQuery({ forceRefresh: true })}`); + if (!response.ok) { + return; + } + const nextFilesData = (await response.json()) as FilesData; + if (!areFilesDataEqual(filesDataRef.current, nextFilesData)) { + updatesAvailableRef.current = true; + setUpdatesAvailable(true); + } + } catch (error) { + console.error("[diffhub] background update check failed", { error }); + } finally { + checkingRef.current = false; + } + }, [buildFilesQuery]); + + const checkForUpdatesRef = useRef(checkForUpdates); + useEffect(() => { + checkForUpdatesRef.current = checkForUpdates; + }, [checkForUpdates]); + useEffect(() => { if (watchMode === "poll") { let active = true; @@ -508,37 +557,12 @@ export const DiffApp = ({ if (!active) { return; } - - void (async () => { - const didUpdate = await pollFilesRef.current({ - forceRefresh: true, - includeComments: false, - }); - if (!active) { - return; - } - if (!didUpdate) { - return; - } - setWatchStatus("updated"); - if (watchStatusTimerRef.current) { - clearTimeout(watchStatusTimerRef.current); - } - watchStatusTimerRef.current = setTimeout(() => { - if (active) { - setWatchStatus("live"); - } - }, 2500); - })(); + void checkForUpdatesRef.current(); }, watchPollMs); return () => { active = false; clearInterval(interval); - if (watchStatusTimerRef.current) { - clearTimeout(watchStatusTimerRef.current); - watchStatusTimerRef.current = null; - } }; } @@ -550,38 +574,20 @@ export const DiffApp = ({ let active = true; setWatchStatus("connecting"); const source = new EventSource("/api/watch"); - const clearWatchStatusTimer = () => { - if (watchStatusTimerRef.current) { - clearTimeout(watchStatusTimerRef.current); - watchStatusTimerRef.current = null; - } - }; const handleReady = () => { - clearWatchStatusTimer(); setWatchStatus("live"); }; const handleChange = () => { - clearWatchStatusTimer(); - void (async () => { - await pollFilesRef.current({ forceRefresh: true, includeComments: false }); - if (!active) { - return; - } - setWatchStatus("updated"); - watchStatusTimerRef.current = setTimeout(() => { - if (active) { - setWatchStatus("live"); - } - }, 2500); - })(); + if (!active) { + return; + } + void checkForUpdatesRef.current(); }; const handleWatchError = (event: Event) => { console.error("[diffhub] file watch stream reported an error", { event }); - clearWatchStatusTimer(); setWatchStatus("offline"); }; const handleStreamError = () => { - clearWatchStatusTimer(); setWatchStatus("offline"); }; @@ -592,7 +598,6 @@ export const DiffApp = ({ return () => { active = false; - clearWatchStatusTimer(); source.removeEventListener(WATCH_STREAM_EVENTS.READY, handleReady); source.removeEventListener(WATCH_STREAM_EVENTS.CHANGE, handleChange); source.removeEventListener(WATCH_STREAM_EVENTS.ERROR, handleWatchError); @@ -892,7 +897,7 @@ export const DiffApp = ({ baseBranch={filesData?.baseBranch ?? "main"} refreshing={refreshing} onRefresh={handleManualRefresh} - watch={getWatchStatusMeta(watchStatus, refreshing)} + watch={getRefreshStatusMeta(updatesAvailable, watchStatus)} commentCount={comments.length} onCopyComments={handleCopyComments} diffMode={diffMode} diff --git a/apps/cli/lib/watch-status.ts b/apps/cli/lib/watch-status.ts index 293fcc6..15a2ca6 100644 --- a/apps/cli/lib/watch-status.ts +++ b/apps/cli/lib/watch-status.ts @@ -1,55 +1,45 @@ -export type WatchStatus = "connecting" | "live" | "offline" | "updated"; +export type WatchHealth = "connecting" | "live" | "offline"; -export interface WatchStatusMeta { - /** Chip styling (border + bg + text) used by the StatusBar pill. */ - className: string; - /** Solid background colour for the System Monitor status dot. */ +export interface RefreshStatusMeta { + /** Solid background colour for the status-bar refresh dot. */ dotClassName: string; label: string; } /** - * Shared mapping from watch state → display metadata. The StatusBar renders - * `dotClassName` as a status dot and surfaces `label` via the refresh button's - * tooltip/aria-label. `updating` (a manual or change-triggered refresh in - * flight) takes precedence over the base state. + * Display metadata for the status-bar refresh control. The diff never refreshes + * on its own — a background detector only watches for changes and raises + * `updatesAvailable` so the user knows a manual refresh would pull in new + * changes. The StatusBar renders `dotClassName` as a status dot and surfaces + * `label` via the refresh button's tooltip/aria-label. */ -export const getWatchStatusMeta = (status: WatchStatus, updating: boolean): WatchStatusMeta => { - if (updating) { +export const getRefreshStatusMeta = ( + updatesAvailable: boolean, + watchHealth: WatchHealth, +): RefreshStatusMeta => { + if (updatesAvailable) { return { - className: "border-blue-500/30 bg-blue-500/10 text-blue-600 dark:text-blue-400", - dotClassName: "bg-blue-500", - label: "Updating…", + dotClassName: "bg-blue-500 animate-pulse", + label: "Updates available", }; } - if (status === "updated") { + if (watchHealth === "offline") { return { - className: "border-diff-green/30 bg-diff-green/10 text-diff-green", - dotClassName: "bg-diff-green", - label: "Updated just now", - }; - } - - if (status === "offline") { - return { - className: "border-amber-500/30 bg-amber-500/10 text-amber-600 dark:text-amber-400", dotClassName: "bg-amber-500", - label: "Watch offline", + label: "Change detection offline", }; } - if (status === "connecting") { + if (watchHealth === "connecting") { return { - className: "border-border bg-muted/40 text-muted-foreground", dotClassName: "bg-muted-foreground", label: "Connecting…", }; } return { - className: "border-border bg-muted/40 text-muted-foreground", dotClassName: "bg-diff-green", - label: "Live", + label: "Up to date", }; }; diff --git a/packages/diff-core/src/chrome/file-tree-body.tsx b/packages/diff-core/src/chrome/file-tree-body.tsx index 7381735..8d84c54 100644 --- a/packages/diff-core/src/chrome/file-tree-body.tsx +++ b/packages/diff-core/src/chrome/file-tree-body.tsx @@ -104,16 +104,24 @@ const FileTreeBody = ({ const search = useFileTreeSearch(model); - // Push a fresh path set + git status whenever the diff's files change. - const isFirstSync = useRef(true); + // Rebuilding the tree (`resetPaths`) clears its DOM and visibly flashes, so do + // it only when the *set* of files actually changes — not on every re-fetch + // that merely changed line counts. Status/decoration changes are applied + // incrementally via `setGitStatus` below, which doesn't flash. + const pathsKey = useMemo(() => paths.join("\n"), [paths]); + const prevPathsKeyRef = useRef(null); useEffect(() => { - if (isFirstSync.current) { - isFirstSync.current = false; + if (prevPathsKeyRef.current === null) { + prevPathsKeyRef.current = pathsKey; return; } + if (prevPathsKeyRef.current === pathsKey) { + return; + } + prevPathsKeyRef.current = pathsKey; model.resetPaths(paths); model.setGitStatus(gitStatus); - }, [model, paths, gitStatus]); + }, [model, paths, pathsKey, gitStatus]); // Comments aren't part of the tree input; re-applying git status nudges the // virtualizer to re-render visible rows so comment decorations update live.