From afe86a84b3c7981fdc359cf7d83f7e56776b967e Mon Sep 17 00:00:00 2001 From: Isaac Sanchez-Hawkins <266845420+isanchez404@users.noreply.github.com> Date: Fri, 8 May 2026 16:11:37 -0400 Subject: [PATCH] fix(git): dedupe status fetches by mode (#1166) * fix(git): dedupe status fetches by mode * test(git): assert reused status result --------- Co-authored-by: Isaac Sanchez --- packages/ui/src/stores/useGitStore.test.ts | 94 ++++++++++++++++++++++ packages/ui/src/stores/useGitStore.ts | 16 ++-- 2 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 packages/ui/src/stores/useGitStore.test.ts diff --git a/packages/ui/src/stores/useGitStore.test.ts b/packages/ui/src/stores/useGitStore.test.ts new file mode 100644 index 00000000..3ec3f971 --- /dev/null +++ b/packages/ui/src/stores/useGitStore.test.ts @@ -0,0 +1,94 @@ +import { beforeEach, describe, expect, test } from 'bun:test'; +import type { GitStatus } from '@/lib/api/types'; +import { useGitStore } from './useGitStore'; + +type Deferred = { + promise: Promise; + resolve: (value: T) => void; + reject: (error: unknown) => void; +}; + +type GitAPI = Parameters['fetchStatus']>[1]; + +const createDeferred = (): Deferred => { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +}; + +const createStatus = (diffStats?: GitStatus['diffStats']): GitStatus => ({ + current: 'main', + tracking: null, + ahead: 0, + behind: 0, + files: [], + isClean: true, + diffStats, +}); + +const createGitApi = (getGitStatus: GitAPI['getGitStatus']): GitAPI => ({ + checkIsGitRepository: async () => true, + getGitStatus, + getGitBranches: async () => ({ all: [], current: 'main', branches: {} }), + getGitLog: async () => ({ all: [], latest: null, total: 0 }), + getCurrentGitIdentity: async () => null, + getGitFileDiff: async (_directory, options) => ({ original: '', modified: '', path: options.path }), +}); + +describe('useGitStore', () => { + beforeEach(() => { + useGitStore.setState({ + directories: new Map(), + activeDirectory: null, + }); + }); + + test('does not reuse an in-flight light status request for full status', async () => { + const requests: Deferred[] = []; + const statusCalls: Array<{ directory: string; options?: { mode?: 'light' } }> = []; + const git = createGitApi((directory, options) => { + statusCalls.push({ directory, options }); + const request = createDeferred(); + requests.push(request); + return request.promise; + }); + + const lightPromise = useGitStore.getState().fetchStatus('/repo', git, { mode: 'light', silent: true }); + const fullPromise = useGitStore.getState().fetchStatus('/repo', git, { silent: true }); + await Promise.resolve(); + + expect(statusCalls).toEqual([ + { directory: '/repo', options: { mode: 'light' } }, + { directory: '/repo', options: undefined }, + ]); + + requests[0].resolve(createStatus()); + requests[1].resolve(createStatus({ 'src/index.ts': { insertions: 1, deletions: 0 } })); + await Promise.all([lightPromise, fullPromise]); + }); + + test('reuses an in-flight full status request for light status', async () => { + const requests: Deferred[] = []; + const statusCalls: Array<{ directory: string; options?: { mode?: 'light' } }> = []; + const git = createGitApi((directory, options) => { + statusCalls.push({ directory, options }); + const request = createDeferred(); + requests.push(request); + return request.promise; + }); + + const fullPromise = useGitStore.getState().fetchStatus('/repo', git, { silent: true }); + const lightPromise = useGitStore.getState().fetchStatus('/repo', git, { mode: 'light', silent: true }); + await Promise.resolve(); + + expect(statusCalls).toEqual([{ directory: '/repo', options: undefined }]); + + requests[0].resolve(createStatus({ 'src/index.ts': { insertions: 1, deletions: 0 } })); + const [fullResult, lightResult] = await Promise.all([fullPromise, lightPromise]); + expect(lightResult).toBe(fullResult); + }); +}); diff --git a/packages/ui/src/stores/useGitStore.ts b/packages/ui/src/stores/useGitStore.ts index 8de19192..bac0ff12 100644 --- a/packages/ui/src/stores/useGitStore.ts +++ b/packages/ui/src/stores/useGitStore.ts @@ -22,6 +22,7 @@ const DIFF_PREFETCH_LARGE_FILE_THRESHOLD = 500; // skip prefetch for files with // Diff cache limits to prevent memory bloat with many modified files const DIFF_CACHE_MAX_ENTRIES = 30; const DIFF_CACHE_MAX_TOTAL_SIZE_BYTES = 20 * 1024 * 1024; // 20MB +type GitStatusFetchMode = 'full' | 'light'; interface DirectoryGitState { isGitRepo: boolean | null; @@ -90,9 +91,11 @@ interface GitAPI { const inFlightDiffFetchesByDirectory = new Map>(); const diffFetchGenerationByDirectory = new Map(); -const inFlightStatusFetchesByDirectory = new Map>(); +const inFlightStatusFetches = new Map>(); const inFlightEnsureAllByDirectory = new Map>(); +const getStatusFetchKey = (directory: string, mode: GitStatusFetchMode): string => `${mode}:${directory}`; + const getDiffFetchGeneration = (directory: string): number => diffFetchGenerationByDirectory.get(directory) ?? 0; @@ -306,7 +309,10 @@ export const useGitStore = create()( }, fetchStatus: async (directory, git, options = {}) => { - const existing = inFlightStatusFetchesByDirectory.get(directory); + const statusFetchMode: GitStatusFetchMode = options.mode ?? 'full'; + const statusFetchKey = getStatusFetchKey(directory, statusFetchMode); + const existing = inFlightStatusFetches.get(statusFetchKey) + ?? (statusFetchMode === 'light' ? inFlightStatusFetches.get(getStatusFetchKey(directory, 'full')) : undefined); if (existing) { return existing; } @@ -428,13 +434,13 @@ export const useGitStore = create()( return statusChanged; })(); - inFlightStatusFetchesByDirectory.set(directory, fetchPromise); + inFlightStatusFetches.set(statusFetchKey, fetchPromise); try { return await fetchPromise; } finally { - if (inFlightStatusFetchesByDirectory.get(directory) === fetchPromise) { - inFlightStatusFetchesByDirectory.delete(directory); + if (inFlightStatusFetches.get(statusFetchKey) === fetchPromise) { + inFlightStatusFetches.delete(statusFetchKey); } } },