From a4fb828b3c7c832aabe37b7302c1553c2cb901fd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:51:59 -0700 Subject: [PATCH] perf(renderer): scope folder review cache projections (#8136) * perf(renderer): scope folder review cache projections * fix(renderer): keep checks selector notifications constant-time --- .../FolderWorkspacePrChecksPanel.tsx | 20 +- ...rent-pr-checks-projection-selector.test.ts | 192 ++++++++++++++++++ .../parent-pr-checks-projection-selector.ts | 93 +++++++++ 3 files changed, 291 insertions(+), 14 deletions(-) create mode 100644 src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.test.ts create mode 100644 src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.ts diff --git a/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksPanel.tsx b/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksPanel.tsx index 67b1e1b06..0c64561ed 100644 --- a/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksPanel.tsx @@ -8,15 +8,12 @@ import { translate } from '@/i18n/i18n' import type { PRCheckDetail, PRCheckRunDetails } from '../../../../shared/types' import { getAttachedWorktreesForFolderWorkspace } from './folder-workspace-attached-worktrees' import { FolderWorkspacePrChecksRow } from './FolderWorkspacePrChecksRow' -import { - buildParentPrChecksProjection, - type ParentPrChecksRefreshOutcome, - type ParentPrChecksRow -} from './parent-pr-checks-rows' +import type { ParentPrChecksRefreshOutcome, ParentPrChecksRow } from './parent-pr-checks-rows' import { getParentPrChecksRefreshCandidates, runLimitedParentPrChecksRefreshes } from './parent-pr-checks-refresh' +import { createParentPrChecksProjectionSelector } from './parent-pr-checks-projection-selector' type FolderWorkspacePrChecksPanelProps = { isVisible?: boolean @@ -33,9 +30,6 @@ export default function FolderWorkspacePrChecksPanel({ const worktreesByRepo = useAppStore((s) => s.worktreesByRepo) const repos = useAppStore((s) => s.repos) const settings = useAppStore((s) => s.settings) - const hostedReviewCache = useAppStore((s) => s.hostedReviewCache) - const prCache = useAppStore((s) => s.prCache) - const checksCache = useAppStore((s) => s.checksCache) const fetchHostedReviewForBranch = useAppStore((s) => s.fetchHostedReviewForBranch) const fetchPRChecks = useAppStore((s) => s.fetchPRChecks) const fetchPRCheckDetails = useAppStore((s) => s.fetchPRCheckDetails) @@ -65,19 +59,17 @@ export default function FolderWorkspacePrChecksPanel({ worktreesByRepo ] ) - const projection = useMemo( + const projectionSelector = useMemo( () => - buildParentPrChecksProjection({ + createParentPrChecksProjectionSelector({ worktrees: childWorktrees, repos, settings, - hostedReviewCache, - prCache, - checksCache, refreshOutcomes }), - [childWorktrees, repos, settings, hostedReviewCache, prCache, checksCache, refreshOutcomes] + [childWorktrees, repos, settings, refreshOutcomes] ) + const projection = useAppStore(projectionSelector) const folderWorkspaceId = folderWorkspace?.id ?? null const headerSummary = useMemo( () => formatReviewChecksHeaderSummary(projection.summary), diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.test.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.test.ts new file mode 100644 index 000000000..75c288da7 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.test.ts @@ -0,0 +1,192 @@ +import { describe, expect, it, vi } from 'vitest' +import type { HostedReviewInfo } from '../../../../shared/hosted-review' +import type { Repo, Worktree } from '../../../../shared/types' +import { getHostedReviewCacheKey } from '@/store/slices/hosted-review' +import { buildParentPrChecksProjection } from './parent-pr-checks-rows' +import { createParentPrChecksProjectionSelector } from './parent-pr-checks-projection-selector' + +function repo(): Repo { + return { + id: 'repo-1', + path: '/repo', + displayName: 'Repo', + badgeColor: '#fff', + addedAt: 1, + kind: 'git' + } +} + +function worktree(index: number): Worktree { + return { + id: `worktree-${index}`, + path: `/worktrees/${index}`, + head: `head-${index}`, + branch: `refs/heads/feature-${index}`, + isBare: false, + isMainWorktree: false, + repoId: 'repo-1', + displayName: `Worktree ${index}`, + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + linkedGitLabMR: null, + linkedGitLabIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: index, + lastActivityAt: index + } +} + +function review(number: number): HostedReviewInfo { + return { + provider: 'github', + number, + title: `Review ${number}`, + state: 'open', + url: `https://example.test/review/${number}`, + status: 'success', + updatedAt: '2026-01-01T00:00:00.000Z', + mergeable: 'MERGEABLE', + headSha: 'head-0' + } +} + +describe('parent PR checks projection selector', () => { + it('does not inspect tracked keys when cache map references are unchanged', () => { + const cacheRead = vi.fn() + const observedCache = new Proxy( + {}, + { + get: (target, property, receiver) => { + cacheRead(property) + return Reflect.get(target, property, receiver) + } + } + ) + const buildProjection = vi.fn(buildParentPrChecksProjection) + const select = createParentPrChecksProjectionSelector( + { worktrees: [worktree(0)], repos: [repo()], settings: null, refreshOutcomes: new Map() }, + buildProjection + ) + const state = { + hostedReviewCache: observedCache, + prCache: observedCache, + checksCache: observedCache + } + const projection = select(state) + cacheRead.mockClear() + + for (let notification = 0; notification < 1_000; notification += 1) { + expect(select(state)).toBe(projection) + } + + expect(cacheRead).not.toHaveBeenCalled() + expect(buildProjection).toHaveBeenCalledTimes(1) + }) + + it('ignores unrelated global review-cache replacements at scale', () => { + const worktrees = Array.from({ length: 100 }, (_, index) => worktree(index)) + const buildProjection = vi.fn(buildParentPrChecksProjection) + const select = createParentPrChecksProjectionSelector( + { worktrees, repos: [repo()], settings: null, refreshOutcomes: new Map() }, + buildProjection + ) + let state = { hostedReviewCache: {}, prCache: {}, checksCache: {} } + let projection = select(state) + let wholeMapInvalidations = 0 + let scopedInvalidations = 0 + + for (let write = 0; write < 200; write += 1) { + const previous = state + state = { + ...state, + hostedReviewCache: { + ...state.hostedReviewCache, + [`unrelated-hosted-${write}`]: { data: null, fetchedAt: write } + } + } + wholeMapInvalidations += Number(previous.hostedReviewCache !== state.hostedReviewCache) + const next = select(state) + scopedInvalidations += Number(projection !== next) + projection = next + } + for (let write = 0; write < 200; write += 1) { + const previous = state + state = { + ...state, + prCache: { + ...state.prCache, + [`unrelated-pr-${write}`]: { data: null, fetchedAt: write } + } + } + wholeMapInvalidations += Number(previous.prCache !== state.prCache) + const next = select(state) + scopedInvalidations += Number(projection !== next) + projection = next + } + for (let write = 0; write < 200; write += 1) { + const previous = state + state = { + ...state, + checksCache: { + ...state.checksCache, + [`unrelated-checks-${write}`]: { data: [], fetchedAt: write } + } + } + wholeMapInvalidations += Number(previous.checksCache !== state.checksCache) + const next = select(state) + scopedInvalidations += Number(projection !== next) + projection = next + } + + expect(wholeMapInvalidations).toBe(600) + expect(scopedInvalidations).toBe(0) + expect(buildProjection).toHaveBeenCalledTimes(1) + }) + + it('rebuilds when a previously missing relevant entry appears', () => { + const activeRepo = repo() + const activeWorktree = worktree(0) + const buildProjection = vi.fn(buildParentPrChecksProjection) + const select = createParentPrChecksProjectionSelector( + { + worktrees: [activeWorktree], + repos: [activeRepo], + settings: null, + refreshOutcomes: new Map() + }, + buildProjection + ) + const initialState = { hostedReviewCache: {}, prCache: {}, checksCache: {} } + const initial = select(initialState) + const relevantKey = getHostedReviewCacheKey( + activeRepo.path, + 'feature-0', + null, + activeRepo.id, + activeRepo.connectionId, + activeRepo.executionHostId, + true + ) + const reviewEntry = { data: review(7), fetchedAt: 1, linkedReviewHintKey: '' } + const relevantState = { + ...initialState, + hostedReviewCache: { [relevantKey]: reviewEntry } + } + const updated = select(relevantState) + + expect(updated).not.toBe(initial) + expect(updated.rows[0]?.reviewLabel).toBe('#7') + expect(buildProjection).toHaveBeenCalledTimes(2) + + const unrelatedState = { + ...relevantState, + checksCache: { unrelated: { data: [], fetchedAt: 2 } } + } + expect(select(unrelatedState)).toBe(updated) + expect(buildProjection).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.ts new file mode 100644 index 000000000..02677575e --- /dev/null +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.ts @@ -0,0 +1,93 @@ +import type { AppState } from '@/store/types' +import { buildParentPrChecksProjection } from './parent-pr-checks-rows' +import type { + BuildParentPrChecksRowsArgs, + ParentPrChecksProjection +} from './parent-pr-checks-row-types' + +type ReviewCacheState = Pick +type ReviewCacheName = keyof ReviewCacheState +type ProjectionInputs = Omit< + BuildParentPrChecksRowsArgs, + 'checksCache' | 'hostedReviewCache' | 'prCache' +> +type ProjectionBuilder = (args: BuildParentPrChecksRowsArgs) => ParentPrChecksProjection +type CacheDependency = { + cacheName: ReviewCacheName + key: string + value: unknown +} + +function trackCacheReads( + state: ReviewCacheState, + cacheName: K, + dependencies: CacheDependency[] +): ReviewCacheState[K] { + return new Proxy(state[cacheName], { + get: (target, property, receiver) => { + const value = Reflect.get(target, property, receiver) + if (typeof property === 'string') { + dependencies.push({ cacheName, key: property, value }) + } + return value + } + }) +} + +function dependenciesAreCurrent( + state: ReviewCacheState, + previousState: ReviewCacheState, + dependencies: readonly CacheDependency[] +): boolean { + return dependencies.every( + ({ cacheName, key, value }) => + state[cacheName] === previousState[cacheName] || Reflect.get(state[cacheName], key) === value + ) +} + +function cacheReferencesAreCurrent( + state: ReviewCacheState, + previousState: ReviewCacheState +): boolean { + return ( + state.hostedReviewCache === previousState.hostedReviewCache && + state.prCache === previousState.prCache && + state.checksCache === previousState.checksCache + ) +} + +export function createParentPrChecksProjectionSelector( + inputs: ProjectionInputs, + buildProjection: ProjectionBuilder = buildParentPrChecksProjection +): (state: ReviewCacheState) => ParentPrChecksProjection { + let cached: { + cacheReferences: ReviewCacheState + dependencies: CacheDependency[] + projection: ParentPrChecksProjection + } | null = null + + return (state) => { + if (cached) { + if (cacheReferencesAreCurrent(state, cached.cacheReferences)) { + return cached.projection + } + if (dependenciesAreCurrent(state, cached.cacheReferences, cached.dependencies)) { + // Why: adopting unrelated replacement maps keeps later store notifications O(1). + cached.cacheReferences = state + return cached.projection + } + } + + const dependencies: CacheDependency[] = [] + // Why: global provider refreshes replace these maps frequently; track only + // cache keys the attached-worktree projection actually reads. + const projection = buildProjection({ + ...inputs, + hostedReviewCache: trackCacheReads(state, 'hostedReviewCache', dependencies), + prCache: trackCacheReads(state, 'prCache', dependencies), + checksCache: trackCacheReads(state, 'checksCache', dependencies) + }) + cached = { cacheReferences: state, dependencies, projection } + return projection + } +}