diff --git a/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.ts b/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.ts index 17792574193..b957f715eaa 100644 --- a/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.ts +++ b/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.ts @@ -1,4 +1,4 @@ -import { useMemo, useRef } from 'react' +import { useMemo } from 'react' import { useShallow } from 'zustand/react/shallow' import { useAppStore } from '@/store' import { getLiveAgentStatusByWorktreeId } from '@/lib/worktree-activity-state' @@ -33,8 +33,8 @@ import { import { computeWorkspaceCleanupFacetList, computeWorkspaceCleanupReviewInfoIndex, - type WorkspaceCleanupFacetListCache, - type WorkspaceCleanupReviewInfoCache + createWorkspaceCleanupFacetListCache, + createWorkspaceCleanupReviewInfoCache } from './workspace-cleanup-facet-row-caches' export type WorkspaceCleanupFacetRows = { @@ -117,13 +117,25 @@ export function useWorkspaceCleanupFacetRows({ () => countWorkspaceCleanupCandidateIds(candidates), [candidates] ) - const reviewLookup = useMemo( - () => buildWorkspaceCleanupReviewLookup({ repos, worktreesByRepo }), - [repos, worktreesByRepo] + // Why: each per-candidate cache lives in one memo together with the derived + // context it is keyed on — the memo deps ARE the cache invalidation, no refs + // (ref writes during render are unsafe under concurrent rendering), and + // interior fills are content-addressed and idempotent. + const reviewContext = useMemo( + () => ({ + sources: { hostedReviewCache, repos, settings, worktreesByRepo }, + lookup: buildWorkspaceCleanupReviewLookup({ repos, worktreesByRepo }), + cache: createWorkspaceCleanupReviewInfoCache() + }), + [hostedReviewCache, repos, settings, worktreesByRepo] ) - const worktreeById = useMemo( - () => buildWorkspaceCleanupWorktreeIndex(worktreesByRepo, repos), - [repos, worktreesByRepo] + const facetContext = useMemo( + () => ({ + worktreeById: buildWorkspaceCleanupWorktreeIndex(worktreesByRepo, repos), + workspaceStatuses: sources.workspaceStatuses, + cache: createWorkspaceCleanupFacetListCache() + }), + [repos, sources.workspaceStatuses, worktreesByRepo] ) const liveAgentStatusByWorktreeId = useMemo( () => getLiveAgentStatusByWorktreeId(sources.agentStatusByPaneKey, sources.tabsByWorktree, now), @@ -134,26 +146,17 @@ export function useWorkspaceCleanupFacetRows({ [sources.dismissals] ) - const reviewCacheRef = useRef(null) - const reviewInfoByWorktreeId = useMemo(() => { - const { cache, infos } = computeWorkspaceCleanupReviewInfoIndex({ - candidates, - candidateIdCounts, - reviewSources: { hostedReviewCache, repos, settings, worktreesByRepo }, - reviewLookup, - cache: reviewCacheRef.current - }) - reviewCacheRef.current = cache - return infos - }, [ - candidateIdCounts, - candidates, - hostedReviewCache, - repos, - reviewLookup, - settings, - worktreesByRepo - ]) + const reviewInfoByWorktreeId = useMemo( + () => + computeWorkspaceCleanupReviewInfoIndex({ + candidates, + candidateIdCounts, + reviewSources: reviewContext.sources, + reviewLookup: reviewContext.lookup, + cache: reviewContext.cache + }), + [candidateIdCounts, candidates, reviewContext] + ) const completedSizeByWorktreeId = useMemo( () => buildWorkspaceCleanupSizeIndex(sources.spaceWorktrees, candidates), @@ -169,33 +172,31 @@ export function useWorkspaceCleanupFacetRows({ ]) }, [candidates, completedSizeByWorktreeId, sources.spaceMeasurements]) - const facetCacheRef = useRef(null) - const facets = useMemo(() => { - const { cache, list } = computeWorkspaceCleanupFacetList({ + const facets = useMemo( + () => + computeWorkspaceCleanupFacetList({ + candidates, + sources: { + worktreeById: facetContext.worktreeById, + workspaceStatuses: facetContext.workspaceStatuses, + sizeBytesByWorktreeId: sizeByWorktreeId, + lastVisitedAtByWorktreeId: sources.lastVisitedAtByWorktreeId, + liveAgentStatusByWorktreeId, + reviewInfoByWorktreeId, + dismissedWorktreeIds + }, + cache: facetContext.cache + }), + [ candidates, - sources: { - worktreeById, - workspaceStatuses: sources.workspaceStatuses, - sizeBytesByWorktreeId: sizeByWorktreeId, - lastVisitedAtByWorktreeId: sources.lastVisitedAtByWorktreeId, - liveAgentStatusByWorktreeId, - reviewInfoByWorktreeId, - dismissedWorktreeIds - }, - cache: facetCacheRef.current - }) - facetCacheRef.current = cache - return list - }, [ - candidates, - dismissedWorktreeIds, - liveAgentStatusByWorktreeId, - reviewInfoByWorktreeId, - sizeByWorktreeId, - sources.lastVisitedAtByWorktreeId, - sources.workspaceStatuses, - worktreeById - ]) + dismissedWorktreeIds, + facetContext, + liveAgentStatusByWorktreeId, + reviewInfoByWorktreeId, + sizeByWorktreeId, + sources.lastVisitedAtByWorktreeId + ] + ) const result = useMemo( () => runWorkspaceCleanupQuery(facets, { filters, sort }, now), @@ -235,20 +236,13 @@ export function useWorkspaceCleanupFacetRows({ : EMPTY_FACET_COUNTS, [facetFilters, facetPanelOpen, facets, now] ) - const matchedIdsRef = useRef>(new Set()) - const facetMatchedWorktreeIds = useMemo(() => { - const next = new Set( - filterWorkspaceCleanupFacets(facets, facetFilters, now).map((row) => row.worktreeId) - ) - const previous = matchedIdsRef.current - // Why: destructive selection pruning keys off this set; identity must only - // change when membership does. - if (previous.size === next.size && [...next].every((id) => previous.has(id))) { - return previous - } - matchedIdsRef.current = next - return next - }, [facetFilters, facets, now]) + // Identity churn here is harmless: the only consumer reads the latest set + // inside a useEffectEvent body and never keys an effect on it. + const facetMatchedWorktreeIds = useMemo>( + () => + new Set(filterWorkspaceCleanupFacets(facets, facetFilters, now).map((row) => row.worktreeId)), + [facetFilters, facets, now] + ) const measuredSizeCount = useMemo(() => countWorkspaceCleanupMeasuredRows(facets), [facets]) const unmeasuredSizeCount = facets.length - measuredSizeCount diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-row-caches.ts b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-row-caches.ts index 40cf8fbb7fe..8cf146fbc33 100644 --- a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-row-caches.ts +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-row-caches.ts @@ -23,16 +23,20 @@ import { * rows reuse their previous facet/review objects and the row array itself * keeps identity when nothing changed, which is what lets React.memo and * downstream memos hold. + * + * The caches are owned by useMemo keyed on their invalidation inputs (never + * refs — ref writes during render are unsafe under concurrent rendering). + * Filling them during render is safe because they are content-addressed and + * idempotent: a replayed or discarded render stores the same value for the + * same key. */ -type SourceToken = readonly unknown[] +export type WorkspaceCleanupReviewInfoCache = WeakMap< + WorkspaceCleanupCandidate, + WorkspaceCleanupReviewInfo +> -function tokensEqual(left: SourceToken, right: SourceToken): boolean { - return left.length === right.length && left.every((value, index) => value === right[index]) -} - -export type WorkspaceCleanupReviewInfoCache = { - token: SourceToken - byCandidate: WeakMap +export function createWorkspaceCleanupReviewInfoCache(): WorkspaceCleanupReviewInfoCache { + return new WeakMap() } export function computeWorkspaceCleanupReviewInfoIndex(args: { @@ -40,25 +44,15 @@ export function computeWorkspaceCleanupReviewInfoIndex(args: { candidateIdCounts: ReadonlyMap reviewSources: WorkspaceCleanupRendererStateInputs reviewLookup: WorkspaceCleanupReviewLookup - cache: WorkspaceCleanupReviewInfoCache | null -}): { cache: WorkspaceCleanupReviewInfoCache; infos: Map } { - const { candidates, candidateIdCounts, reviewSources, reviewLookup } = args - const token: SourceToken = [ - reviewSources.hostedReviewCache, - reviewSources.repos, - reviewSources.settings, - reviewSources.worktreesByRepo - ] - let cache = args.cache - if (!cache || !tokensEqual(cache.token, token)) { - cache = { token, byCandidate: new WeakMap() } - } + cache: WorkspaceCleanupReviewInfoCache +}): Map { + const { candidates, candidateIdCounts, reviewSources, reviewLookup, cache } = args const infos = new Map() for (const candidate of candidates) { - let info = cache.byCandidate.get(candidate) + let info = cache.get(candidate) if (info === undefined) { info = getWorkspaceCleanupReviewInfo(candidate, reviewSources, reviewLookup) - cache.byCandidate.set(candidate, info) + cache.set(candidate, info) } infos.set( getWorkspaceCleanupHostIdentity( @@ -71,7 +65,7 @@ export function computeWorkspaceCleanupReviewInfoIndex(args: { infos.set(candidate.worktreeId, info) } } - return { cache, infos } + return infos } type FacetCacheEntry = { @@ -84,24 +78,22 @@ type FacetCacheEntry = { } export type WorkspaceCleanupFacetListCache = { - token: SourceToken byCandidate: WeakMap lastList: WorkspaceCleanupFacets[] } +export function createWorkspaceCleanupFacetListCache(): WorkspaceCleanupFacetListCache { + return { byCandidate: new WeakMap(), lastList: [] } +} + export function computeWorkspaceCleanupFacetList(args: { candidates: readonly WorkspaceCleanupCandidate[] sources: Required> & { workspaceStatuses: readonly WorkspaceStatusDefinition[] } - cache: WorkspaceCleanupFacetListCache | null -}): { cache: WorkspaceCleanupFacetListCache; list: WorkspaceCleanupFacets[] } { - const { candidates, sources } = args - const token: SourceToken = [sources.worktreeById, sources.workspaceStatuses] - let cache = args.cache - if (!cache || !tokensEqual(cache.token, token)) { - cache = { token, byCandidate: new WeakMap(), lastList: [] } - } + cache: WorkspaceCleanupFacetListCache +}): WorkspaceCleanupFacets[] { + const { candidates, sources, cache } = args const list = candidates.map((candidate) => { const hostIdentity = getWorkspaceCleanupHostIdentity( getWorkspaceCleanupCandidateHostId(candidate), @@ -144,8 +136,8 @@ export function computeWorkspaceCleanupFacetList(args: { // downstream memo skip its O(N) pass on no-op streaming ticks. const previous = cache.lastList if (previous.length === list.length && list.every((facet, index) => facet === previous[index])) { - return { cache, list: previous } + return previous } cache.lastList = list - return { cache, list } + return list }