From ecefcd174b1343db4ba63297da2dd1aa23a92e54 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:19:42 -0700 Subject: [PATCH] Recover branch compare on visibility change Add recovery mode that reuses cached branch comparison data when the window regains focus instead of clearing results and forcing a refresh. This preserves the diff display during operations like rebasing that may cause the window to go to the background. --- .../source-control/sync/use-branch-compare.ts | 67 +++++---- ...use-source-control-branch-compare.test.tsx | 140 +++++++++++++++++- 2 files changed, 174 insertions(+), 33 deletions(-) diff --git a/src/renderer/src/components/right-sidebar/source-control/sync/use-branch-compare.ts b/src/renderer/src/components/right-sidebar/source-control/sync/use-branch-compare.ts index 3979ddf8e35..5699d826e64 100644 --- a/src/renderer/src/components/right-sidebar/source-control/sync/use-branch-compare.ts +++ b/src/renderer/src/components/right-sidebar/source-control/sync/use-branch-compare.ts @@ -3,6 +3,7 @@ import { installWindowVisibilityInterval } from '@/lib/window-visibility-interva import { getConnectionId } from '@/lib/connection-context' import { getRuntimeGitBranchCompare, type RuntimeGitContext } from '@/runtime/runtime-git-client' import { useAppStore } from '@/store' +import { createLoadingBranchCompareSummary } from '@/store/slices/editor/git/branch-compare-state' import type { GitUpstreamStatus } from '../../../../../../shared/git-status-types' import { shouldClearBranchCompareForMissingBase } from './base-ref-resolution' import { @@ -18,7 +19,7 @@ export const BRANCH_REFRESH_INTERVAL_MS = 30_000 const BRANCH_REFRESH_IDLE_MULTIPLIER = 1 const BRANCH_REFRESH_MAX_INTERVAL_MS = 5 * 60_000 -type BranchCompareRefreshKind = 'immediate' | 'interval' +type BranchCompareRefreshKind = 'immediate' | 'interval' | 'recovery' export function useSourceControlBranchCompare({ activeRepoSettings, @@ -40,10 +41,7 @@ export function useSourceControlBranchCompare({ isBranchVisible: boolean activeGitStatusHead: string | null remoteStatus: GitUpstreamStatus | undefined -}): { - refreshBranchCompare: () => Promise - refreshBranchCompareRef: React.RefObject<() => Promise> -} { +}) { const beginGitBranchCompareRequest = useAppStore((s) => s.beginGitBranchCompareRequest) const setGitBranchCompareResult = useAppStore((s) => s.setGitBranchCompareResult) const clearGitBranchCompare = useAppStore((s) => s.clearGitBranchCompare) @@ -67,27 +65,19 @@ export function useSourceControlBranchCompare({ return } const requestKey = `${activeWorktreeId}:${compareBaseRef}:${Date.now()}` - const existingSummary = - useAppStore.getState().gitBranchCompareSummaryByWorktree[activeWorktreeId] - // Why: only reset to 'loading' on the first request or a base-ref change; resetting on every poll caused a visible loading→error→loading flicker. - const baseRefChanged = existingSummary && existingSummary.baseRef !== compareBaseRef - const shouldResetToLoading = !existingSummary || baseRefChanged - if (shouldResetToLoading) { - beginGitBranchCompareRequest(activeWorktreeId, requestKey, compareBaseRef) - } else { - beginGitBranchCompareRequest(activeWorktreeId, requestKey, compareBaseRef, { - preserveExistingSummary: true - }) - } + const summary = useAppStore.getState().gitBranchCompareSummaryByWorktree[activeWorktreeId] + // Why: polling should preserve results unless the comparison base changed. + beginGitBranchCompareRequest(activeWorktreeId, requestKey, compareBaseRef, { + preserveExistingSummary: !!summary && summary.baseRef === compareBaseRef + }) try { - const connectionId = getConnectionId(activeWorktreeId) ?? undefined const result = await getRuntimeGitBranchCompare( { // Why: route the branch compare by the repo OWNER host, not the focused runtime. settings: activeRepoSettings, worktreeId: activeWorktreeId, worktreePath, - connectionId + connectionId: getConnectionId(activeWorktreeId) ?? undefined }, compareBaseRef, kind === 'interval' ? 'background' : 'interactive' @@ -96,12 +86,8 @@ export function useSourceControlBranchCompare({ } catch (error) { setGitBranchCompareResult(activeWorktreeId, requestKey, { summary: { - baseRef: compareBaseRef, - baseOid: null, + ...createLoadingBranchCompareSummary(compareBaseRef), compareRef: branchName, - headOid: null, - mergeBase: null, - changedFiles: 0, status: 'error', errorMessage: error instanceof Error ? error.message : 'Branch compare failed' }, @@ -154,11 +140,23 @@ export function useSourceControlBranchCompare({ const startBranchCompare = useCallback( async (kind: BranchCompareRefreshKind) => { - if (kind === 'immediate') { + if (kind === 'recovery') { + const summary = + useAppStore.getState().gitBranchCompareSummaryByWorktree[activeWorktreeId ?? ''] + // Why: reuse an in-flight result if it recovered the visible data before this rerun. + if (summary && summary.status !== 'loading' && summary.baseRef === compareBaseRef) { + scheduleBranchComparePoll() + return + } + } + if (kind !== 'interval') { clearBranchComparePollTimer() } if (branchCompareInFlightRef.current) { - if (kind === 'immediate' || branchCompareRerunRef.current === null) { + if ( + branchCompareRerunRef.current !== 'immediate' && + (kind !== 'interval' || branchCompareRerunRef.current === null) + ) { branchCompareRerunRef.current = kind } return branchCompareRunPromiseRef.current ?? undefined @@ -191,21 +189,27 @@ export function useSourceControlBranchCompare({ branchCompareRerunRef.current = null if (rerunKind === 'immediate') { await refreshBranchCompareRef.current() + } else if (rerunKind === 'recovery') { + await startBranchCompareRef.current('recovery') } else if (rerunKind === 'interval') { scheduleBranchComparePoll() } } })() branchCompareRunPromiseRef.current = runPromise - try { - await runPromise - } finally { + await runPromise.finally(() => { if (branchCompareRunPromiseRef.current === runPromise) { branchCompareRunPromiseRef.current = null } - } + }) }, - [clearBranchComparePollTimer, runBranchCompare, scheduleBranchComparePoll] + [ + activeWorktreeId, + compareBaseRef, + clearBranchComparePollTimer, + runBranchCompare, + scheduleBranchComparePoll + ] ) const refreshBranchCompare = useCallback( () => startBranchCompare('immediate'), @@ -280,6 +284,7 @@ export function useSourceControlBranchCompare({ branchComparePollEnabledRef.current = true const stopInterval = installWindowVisibilityInterval({ run: () => void startBranchCompareRef.current('interval'), + runOnVisible: () => void startBranchCompareRef.current('recovery'), jitterOnVisible: true, intervalMs: BRANCH_REFRESH_INTERVAL_MS }) diff --git a/src/renderer/src/components/right-sidebar/use-source-control-branch-compare.test.tsx b/src/renderer/src/components/right-sidebar/use-source-control-branch-compare.test.tsx index ea32c80039b..6d6cd377c63 100644 --- a/src/renderer/src/components/right-sidebar/use-source-control-branch-compare.test.tsx +++ b/src/renderer/src/components/right-sidebar/use-source-control-branch-compare.test.tsx @@ -9,7 +9,10 @@ const mocks = vi.hoisted(() => ({ beginGitBranchCompareRequest: vi.fn(), setGitBranchCompareResult: vi.fn(), clearGitBranchCompare: vi.fn(), - gitBranchCompareSummaryByWorktree: {} as Record + gitBranchCompareSummaryByWorktree: {} as Record< + string, + { baseRef: string; status?: string } | undefined + > })) vi.mock('@/runtime/runtime-git-client', () => ({ @@ -235,6 +238,9 @@ describe('useSourceControlBranchCompare scheduler', () => { vi.useFakeTimers() const first = deferred() mocks.getRuntimeGitBranchCompare.mockReturnValueOnce(first.promise) + mocks.gitBranchCompareSummaryByWorktree = { + A: { baseRef: 'origin/main', status: 'ready' } + } // Visible mounts run once immediately through the visibility interval. await mount({ isBranchVisible: true }) expect(mocks.getRuntimeGitBranchCompare).toHaveBeenCalledTimes(1) @@ -296,7 +302,8 @@ describe('useSourceControlBranchCompare scheduler', () => { expect(mocks.beginGitBranchCompareRequest).toHaveBeenLastCalledWith( 'A', expect.any(String), - 'origin/dev' + 'origin/dev', + { preserveExistingSummary: false } ) }) @@ -363,3 +370,132 @@ describe('useSourceControlBranchCompare scheduler', () => { expect(mocks.getRuntimeGitBranchCompare).toHaveBeenCalledTimes(2) }) }) + +describe('branch comparison visibility recovery', () => { + it.each(['loading', 'missing', 'base-change'])( + 'bypasses slow polling backoff for %s data', + async (reason) => { + vi.useFakeTimers() + const first = deferred() + mocks.getRuntimeGitBranchCompare.mockReturnValueOnce(first.promise) + const root = await mount({ isBranchVisible: true, statusHead: 'head-1' }) + await act(async () => { + root.render() + await vi.advanceTimersByTimeAsync(90_000) + first.resolve(OK) + }) + await flush() + mocks.gitBranchCompareSummaryByWorktree = + reason === 'missing' + ? {} + : { + A: { baseRef: 'origin/main', status: reason === 'loading' ? 'loading' : 'ready' } + } + await act(async () => { + root.render( + + ) + }) + await flush() + expect(mocks.getRuntimeGitBranchCompare).toHaveBeenCalledTimes(2) + expect(mocks.getRuntimeGitBranchCompare).toHaveBeenLastCalledWith( + expect.objectContaining({ worktreeId: 'A' }), + reason === 'base-change' ? 'origin/dev' : 'origin/main', + 'interactive' + ) + } + ) + + it('preserves slow polling backoff when reopening valid data', async () => { + vi.useFakeTimers() + const first = deferred() + mocks.getRuntimeGitBranchCompare.mockReturnValueOnce(first.promise) + const root = await mount({ isBranchVisible: true, statusHead: 'head-1' }) + await act(async () => { + root.render() + await vi.advanceTimersByTimeAsync(90_000) + first.resolve(OK) + }) + await flush() + mocks.gitBranchCompareSummaryByWorktree = { + A: { baseRef: 'origin/main', status: 'ready' } + } + await act(async () => { + root.render() + }) + await act(async () => { + await vi.advanceTimersByTimeAsync(89_999) + }) + expect(mocks.getRuntimeGitBranchCompare).toHaveBeenCalledTimes(1) + await act(async () => { + await vi.advanceTimersByTimeAsync(1) + }) + expect(mocks.getRuntimeGitBranchCompare).toHaveBeenCalledTimes(2) + expect(mocks.getRuntimeGitBranchCompare).toHaveBeenLastCalledWith( + expect.objectContaining({ worktreeId: 'A' }), + 'origin/main', + 'background' + ) + }) + + it('coalesces rapid stale reopenings behind a slow request', async () => { + const first = deferred() + mocks.getRuntimeGitBranchCompare.mockReturnValueOnce(first.promise) + const root = await mount({ isBranchVisible: true, statusHead: 'head-1' }) + mocks.gitBranchCompareSummaryByWorktree = { + A: { baseRef: 'origin/main', status: 'loading' } + } + for (let i = 0; i < 5; i++) { + await act(async () => { + root.render() + }) + await act(async () => { + root.render() + }) + } + expect(mocks.getRuntimeGitBranchCompare).toHaveBeenCalledTimes(1) + await act(async () => { + first.resolve(OK) + }) + await flush() + expect(mocks.getRuntimeGitBranchCompare).toHaveBeenCalledTimes(2) + }) +}) + +it('reuses a recovered in-flight result after reopening instead of immediately comparing twice', async () => { + vi.useFakeTimers() + const first = deferred() + mocks.getRuntimeGitBranchCompare.mockReturnValueOnce(first.promise) + const root = await mount({ isBranchVisible: true, statusHead: 'head-1' }) + mocks.gitBranchCompareSummaryByWorktree = { + A: { baseRef: 'origin/main', status: 'loading' } + } + await act(async () => { + root.render() + }) + await act(async () => { + root.render() + }) + mocks.setGitBranchCompareResult.mockImplementation(() => { + mocks.gitBranchCompareSummaryByWorktree = { + A: { baseRef: 'origin/main', status: 'ready' } + } + }) + await act(async () => { + first.resolve(OK) + }) + await flush() + expect(mocks.getRuntimeGitBranchCompare).toHaveBeenCalledTimes(1) + await act(async () => { + await vi.advanceTimersByTimeAsync(BRANCH_REFRESH_INTERVAL_MS - 1) + }) + expect(mocks.getRuntimeGitBranchCompare).toHaveBeenCalledTimes(1) + await act(async () => { + await vi.advanceTimersByTimeAsync(1) + }) + expect(mocks.getRuntimeGitBranchCompare).toHaveBeenCalledTimes(2) +})