From e5be88342d1e85dd625d60e82118f455c8eafbbd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:51:47 -0700 Subject: [PATCH] perf(renderer): reuse connection context indexes (#8085) * perf(renderer): reuse connection context indexes * perf(renderer): reuse connection indexes in tab bars * perf(renderer): reuse worktree index for diff comments * perf(renderer): index remaining diff comment selectors --- .../components/editor/CombinedDiffViewer.tsx | 5 +- .../src/components/editor/DiffSectionItem.tsx | 4 +- .../src/components/editor/DiffViewer.tsx | 4 +- .../components/editor/EditorPanelHeader.tsx | 5 +- .../src/components/editor/MonacoEditor.tsx | 4 +- .../components/editor/RichMarkdownEditor.tsx | 26 ++---- .../right-sidebar/SourceControl.tsx | 7 +- .../components/tab-bar/QuickLaunchButton.tsx | 11 +-- .../tab-bar/TabBar.context-menu.test.ts | 6 ++ .../src/components/tab-bar/TabBar.tsx | 25 +++--- .../src/lib/connection-context.test.ts | 52 ++++++++++- src/renderer/src/lib/connection-context.ts | 8 +- src/renderer/src/store/selectors.ts | 62 ++----------- .../worktree-diff-comments-selector.test.ts | 87 +++++++++++++++++++ .../store/worktree-diff-comments-selector.ts | 24 +++++ src/renderer/src/store/worktree-repo-index.ts | 61 +++++++++++++ 16 files changed, 279 insertions(+), 112 deletions(-) create mode 100644 src/renderer/src/store/worktree-diff-comments-selector.test.ts create mode 100644 src/renderer/src/store/worktree-diff-comments-selector.ts create mode 100644 src/renderer/src/store/worktree-repo-index.ts diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index 8f9eee91a..9ae116de4 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -21,6 +21,7 @@ import { setWithLRU } from '@/lib/scroll-cache' import { getConnectionIdForFile } from '@/lib/connection-context' import { getCombinedDiffSectionConnectionId } from './combined-diff-section-connection' import { findWorktreeById } from '@/store/slices/worktree-helpers' +import { selectWorktreeDiffCommentsOrEmpty } from '@/store/worktree-diff-comments-selector' import { writeRuntimeFile } from '@/runtime/runtime-file-client' import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' import { formatDiffComments } from '@/lib/diff-comments-format' @@ -234,7 +235,9 @@ export default function CombinedDiffViewer({ const openBranchAllDiffs = useAppStore((s) => s.openBranchAllDiffs) const updateSettings = useAppStore((s) => s.updateSettings) const clearDiffComments = useAppStore((s) => s.clearDiffComments) - const diffCommentsForWorktree = useAppStore((s) => s.getDiffComments(file.worktreeId)) + const diffCommentsForWorktree = useAppStore((s) => + selectWorktreeDiffCommentsOrEmpty(s, file.worktreeId) + ) const activeGroupId = useAppStore((s) => s.activeGroupIdByWorktree[file.worktreeId]) const isDark = settings?.theme === 'dark' || diff --git a/src/renderer/src/components/editor/DiffSectionItem.tsx b/src/renderer/src/components/editor/DiffSectionItem.tsx index ffb1aa36b..a070c38cb 100644 --- a/src/renderer/src/components/editor/DiffSectionItem.tsx +++ b/src/renderer/src/components/editor/DiffSectionItem.tsx @@ -13,7 +13,7 @@ import { monaco } from '@/lib/monaco-setup' import { detectLanguage } from '@/lib/language-detect' import { useAppStore } from '@/store' import { computeDiffEditorFontSize } from '@/lib/editor-font-zoom' -import { findWorktreeById } from '@/store/slices/worktree-helpers' +import { selectWorktreeDiffComments } from '@/store/worktree-diff-comments-selector' import { useDiffCommentDecorator, type DecoratedDiffComment @@ -106,7 +106,7 @@ export function DiffSectionItem({ // memo. Selecting a fresh `.filter(...)` result would invalidate on every // store change and cause needless re-renders of this section. const allDiffComments = useAppStore((s): DiffComment[] | undefined => - worktreeId ? findWorktreeById(s.worktreesByRepo, worktreeId)?.diffComments : undefined + selectWorktreeDiffComments(s, worktreeId) ) const diffComments = useMemo( () => (allDiffComments ?? []).filter((c) => c.filePath === section.path && isDiffComment(c)), diff --git a/src/renderer/src/components/editor/DiffViewer.tsx b/src/renderer/src/components/editor/DiffViewer.tsx index 9aae498c1..c119abee4 100644 --- a/src/renderer/src/components/editor/DiffViewer.tsx +++ b/src/renderer/src/components/editor/DiffViewer.tsx @@ -6,7 +6,7 @@ import { diffViewStateCache, setWithLRU } from '@/lib/scroll-cache' import { monaco } from '@/lib/monaco-setup' import { computeDiffEditorFontSize } from '@/lib/editor-font-zoom' import { useContextualCopySetup } from './useContextualCopySetup' -import { findWorktreeById } from '@/store/slices/worktree-helpers' +import { selectWorktreeDiffComments } from '@/store/worktree-diff-comments-selector' import { useDiffCommentDecorator } from '../diff-comments/useDiffCommentDecorator' import { DiffCommentPopover } from '../diff-comments/DiffCommentPopover' import { @@ -57,7 +57,7 @@ export default function DiffViewer({ // identity only changes when diffComments actually changes on this worktree. // Filtering by relativePath happens in a memo below. const allDiffComments = useAppStore((s): DiffComment[] | undefined => - worktreeId ? findWorktreeById(s.worktreesByRepo, worktreeId)?.diffComments : undefined + selectWorktreeDiffComments(s, worktreeId) ) const diffComments = useMemo( () => (allDiffComments ?? []).filter((c) => c.filePath === relativePath && isDiffComment(c)), diff --git a/src/renderer/src/components/editor/EditorPanelHeader.tsx b/src/renderer/src/components/editor/EditorPanelHeader.tsx index 23ce65305..6cbb2b6f6 100644 --- a/src/renderer/src/components/editor/EditorPanelHeader.tsx +++ b/src/renderer/src/components/editor/EditorPanelHeader.tsx @@ -1,6 +1,7 @@ import { useMemo } from 'react' import { Columns2, Eye, FileText, ListTree, Rows2 } from 'lucide-react' import { useAppStore } from '@/store' +import { selectWorktreeDiffCommentsOrEmpty } from '@/store/worktree-diff-comments-selector' import type { OpenFile } from '@/store/slices/editor' import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip' import EditorViewToggle, { @@ -81,7 +82,9 @@ export function EditorPanelHeader({ onToggleMarkdownFrontmatter, onExportMarkdownToPdf }: EditorPanelHeaderProps): React.JSX.Element { - const diffComments = useAppStore((s) => s.getDiffComments(activeFile.worktreeId)) + const diffComments = useAppStore((s) => + selectWorktreeDiffCommentsOrEmpty(s, activeFile.worktreeId) + ) const activeGroupId = useAppStore((s) => s.activeGroupIdByWorktree[activeFile.worktreeId]) const diffWordWrap = useAppStore((s) => s.settings?.diffWordWrap === true) const updateSettings = useAppStore((s) => s.updateSettings) diff --git a/src/renderer/src/components/editor/MonacoEditor.tsx b/src/renderer/src/components/editor/MonacoEditor.tsx index d99af690a..6c608ceef 100644 --- a/src/renderer/src/components/editor/MonacoEditor.tsx +++ b/src/renderer/src/components/editor/MonacoEditor.tsx @@ -33,7 +33,7 @@ import { type MarkdownDocLinkDecorationController } from './monaco-markdown-doc-link-decorations' import { buildGitConflictDecorations, hasGitConflictMarkers } from './monaco-conflict-decorations' -import { findWorktreeById } from '@/store/slices/worktree-helpers' +import { selectWorktreeDiffComments } from '@/store/worktree-diff-comments-selector' import type { DiffComment } from '../../../../shared/types' import { isMarkdownComment } from '@/lib/diff-comment-compat' import { formatMarkdownReviewNotes, type MarkdownReviewNote } from '@/lib/markdown-review-notes' @@ -140,7 +140,7 @@ export default function MonacoEditor({ const scrollToDiffCommentId = useAppStore((s) => s.scrollToDiffCommentId) const setScrollToDiffCommentId = useAppStore((s) => s.setScrollToDiffCommentId) const allDiffComments = useAppStore((s): DiffComment[] | undefined => - worktreeId ? findWorktreeById(s.worktreesByRepo, worktreeId)?.diffComments : undefined + selectWorktreeDiffComments(s, worktreeId) ) const editorFontSize = computeEditorFontSize( settings?.terminalFontSize ?? 13, diff --git a/src/renderer/src/components/editor/RichMarkdownEditor.tsx b/src/renderer/src/components/editor/RichMarkdownEditor.tsx index 94e0835ea..c59540529 100644 --- a/src/renderer/src/components/editor/RichMarkdownEditor.tsx +++ b/src/renderer/src/components/editor/RichMarkdownEditor.tsx @@ -2,6 +2,8 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import type { Editor } from '@tiptap/react' import type { DiffComment, MarkdownDocument } from '../../../../shared/types' import { useAppStore } from '@/store' +import { selectWorktreeDiffComments } from '@/store/worktree-diff-comments-selector' +import { getIndexedWorktreeById } from '@/store/worktree-repo-index' import { useLocalImagePick } from './useLocalImagePick' import { useRichMarkdownSearch } from './useRichMarkdownSearch' import type { LinkBubbleState } from './RichMarkdownLinkBubble' @@ -81,24 +83,12 @@ export default function RichMarkdownEditor({ const deleteDiffComment = useAppStore((s) => s.deleteDiffComment) const updateDiffComment = useAppStore((s) => s.updateDiffComment) const clearDeliveredDiffComments = useAppStore((s) => s.clearDeliveredDiffComments) - const allDiffComments = useAppStore((s): DiffComment[] | undefined => { - for (const list of Object.values(s.worktreesByRepo)) { - const worktree = list.find((candidate) => candidate.id === worktreeId) - if (worktree) { - return worktree.diffComments - } - } - return undefined - }) - const worktreeRoot = useAppStore((s) => { - for (const list of Object.values(s.worktreesByRepo)) { - const wt = list.find((w) => w.id === worktreeId) - if (wt) { - return wt.path - } - } - return null - }) + const allDiffComments = useAppStore((s): DiffComment[] | undefined => + selectWorktreeDiffComments(s, worktreeId) + ) + const worktreeRoot = useAppStore( + (s) => getIndexedWorktreeById(s.worktreesByRepo, worktreeId)?.path ?? null + ) const scrollContainerRef = useRef(null) const menu = useRichMarkdownMenuController({ markdownDocuments }) const isMac = navigator.userAgent.includes('Mac') diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 6a863e9a3..eee38d1eb 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -30,6 +30,7 @@ import { type LucideIcon } from 'lucide-react' import { useAppStore } from '@/store' +import { selectWorktreeDiffCommentsOrEmpty } from '@/store/worktree-diff-comments-selector' import { isSyncPushStageError, resolveRemoteOperationErrorMessage @@ -906,11 +907,13 @@ function SourceControlInner(): React.JSX.Element { const setRightSidebarOpen = useAppStore((s) => s.setRightSidebarOpen) const setRightSidebarTab = useAppStore((s) => s.setRightSidebarTab) // Why: pass activeWorktreeId directly (even when null/undefined) so the - // slice's getDiffComments returns its stable EMPTY_COMMENTS sentinel. An + // selector returns its stable empty sentinel. An // inline `[]` fallback would allocate a new array each store update, break // Zustand's Object.is equality, and cause this component plus the // diffCommentCountByPath memo to churn on every unrelated store change. - const diffCommentsForActive = useAppStore((s) => s.getDiffComments(activeWorktreeId)) + const diffCommentsForActive = useAppStore((s) => + selectWorktreeDiffCommentsOrEmpty(s, activeWorktreeId) + ) const diffCommentCount = diffCommentsForActive.length // Why: per-file counts are fed into each UncommittedEntryRow so a comment // badge can appear next to the status letter. Compute once per render so diff --git a/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx b/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx index 84681bbc8..a439a8b67 100644 --- a/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx +++ b/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx @@ -4,6 +4,7 @@ import { toast } from 'sonner' import { DropdownMenuItem, DropdownMenuShortcut } from '@/components/ui/dropdown-menu' import { getAgentCatalog, AgentIcon } from '@/lib/agent-catalog' import { useAppStore } from '@/store' +import { getConnectionIdFromState } from '@/lib/connection-context' import { useDetectedAgents } from '@/hooks/useDetectedAgents' import { useOptionalShortcutLabel } from '@/hooks/useShortcutLabel' import { launchAgentInNewTab } from '@/lib/launch-agent-in-new-tab' @@ -103,15 +104,7 @@ function QuickLaunchAgentMenuItemsInner({ // snapshot via getState()). This ensures the component re-renders when the // SSH connection state changes. Returns undefined when the worktree isn't // found (store not hydrated), null for local repos, string for remote. - const connectionId = useAppStore((s) => { - const allWorktrees = Object.values(s.worktreesByRepo ?? {}).flat() - const worktree = allWorktrees.find((w) => w.id === worktreeId) - if (!worktree) { - return undefined - } - const repo = s.repos?.find((r) => r.id === worktree.repoId) - return repo?.connectionId ?? null - }) + const connectionId = useAppStore((s) => getConnectionIdFromState(s, worktreeId)) const { detectedIds } = useDetectedAgents(connectionId) const defaultAgent = useAppStore((s) => s.settings?.defaultTuiAgent) const disabledAgents = useAppStore((s) => s.settings?.disabledTuiAgents ?? []) diff --git a/src/renderer/src/components/tab-bar/TabBar.context-menu.test.ts b/src/renderer/src/components/tab-bar/TabBar.context-menu.test.ts index d580d2c8c..026c5d25a 100644 --- a/src/renderer/src/components/tab-bar/TabBar.context-menu.test.ts +++ b/src/renderer/src/components/tab-bar/TabBar.context-menu.test.ts @@ -20,6 +20,8 @@ const useAppStoreMock = vi.fn( activeTabId: string | null activeTabType: 'terminal' | 'editor' | 'browser' | 'simulator' | null gitStatusByWorktree: Record + repos: never[] + worktreesByRepo: Record unifiedTabsByWorktree: Record activeGroupIdByWorktree: Record pinTab: typeof pinTabMock @@ -34,6 +36,8 @@ const useAppStoreMock = vi.fn( activeTabId: appStoreSnapshot.activeTabId, activeTabType: appStoreSnapshot.activeTabType, gitStatusByWorktree: {}, + repos: [], + worktreesByRepo: {}, unifiedTabsByWorktree: appStoreSnapshot.unifiedTabsByWorktree, activeGroupIdByWorktree: appStoreSnapshot.activeGroupIdByWorktree, pinTab: pinTabMock, @@ -107,6 +111,8 @@ useAppStoreExport.getState = vi.fn(() => ({ activeTabId: appStoreSnapshot.activeTabId, activeTabType: appStoreSnapshot.activeTabType, gitStatusByWorktree: {}, + repos: [], + worktreesByRepo: {}, unifiedTabsByWorktree: appStoreSnapshot.unifiedTabsByWorktree, activeGroupIdByWorktree: appStoreSnapshot.activeGroupIdByWorktree, pinTab: pinTabMock, diff --git a/src/renderer/src/components/tab-bar/TabBar.tsx b/src/renderer/src/components/tab-bar/TabBar.tsx index 2632fffe8..94c09c405 100644 --- a/src/renderer/src/components/tab-bar/TabBar.tsx +++ b/src/renderer/src/components/tab-bar/TabBar.tsx @@ -52,6 +52,7 @@ import { import { getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { getLocalProjectExecutionRuntimeContext } from '@/lib/local-preflight-context' +import { getConnectionIdFromState } from '@/lib/connection-context' import { useOptionalShortcutLabel, useShortcutLabel } from '@/hooks/useShortcutLabel' import { type BuiltInWindowsTerminalShell, @@ -313,13 +314,11 @@ function TabBarInner({ const activeRuntimeEnvironmentId = useAppStore( (s) => getRuntimeEnvironmentIdForWorktree(s, worktreeId)?.trim() || null ) - const worktreeConnectionId = useAppStore((s) => { - const worktree = Object.values(s.worktreesByRepo ?? {}) - .flat() - .find((entry) => entry.id === worktreeId) - const repo = worktree ? s.repos?.find((entry) => entry.id === worktree.repoId) : null - return repo?.connectionId?.trim() || null - }) + // Why: retained tab strips rerun selectors on every store write; reuse the + // canonical worktree/repo indexes instead of flattening both slices here. + const worktreeConnectionId = useAppStore( + (s) => getConnectionIdFromState(s, worktreeId)?.trim() || null + ) const worktreeRemotePlatform = useAppStore((s) => { if (!worktreeConnectionId) { return null @@ -331,15 +330,13 @@ function TabBarInner({ (s) => s.settings?.agentCmdOverrides ?? EMPTY_AGENT_CMD_OVERRIDES ) const agentDetectionTargetKey = useAppStore((s): string | undefined => { - const allWorktrees = Object.values(s.worktreesByRepo ?? {}).flat() - const worktree = allWorktrees.find((w) => w.id === worktreeId) - if (!worktree) { + const connectionId = getConnectionIdFromState(s, worktreeId) + if (connectionId === undefined) { return undefined } - const repo = s.repos?.find((r) => r.id === worktree.repoId) - const repoConnectionId = repo?.connectionId?.trim() - if (repoConnectionId) { - return `ssh:${repoConnectionId}` + const normalizedConnectionId = connectionId?.trim() + if (normalizedConnectionId) { + return `ssh:${normalizedConnectionId}` } const runtimeEnvironmentId = getRuntimeEnvironmentIdForWorktree(s, worktreeId)?.trim() if (runtimeEnvironmentId) { diff --git a/src/renderer/src/lib/connection-context.test.ts b/src/renderer/src/lib/connection-context.test.ts index 48d27ecf1..59e80c48a 100644 --- a/src/renderer/src/lib/connection-context.test.ts +++ b/src/renderer/src/lib/connection-context.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import type { FolderWorkspace, ProjectGroup, Repo } from '../../../shared/types' +import type { FolderWorkspace, ProjectGroup, Repo, Worktree } from '../../../shared/types' import { useAppStore } from '@/store' import type { AppState } from '@/store/types' import { @@ -521,6 +521,56 @@ describe('getConnectionIdFromState', () => { expect(getConnectionIdFromState(state, 'repo-ssh::/home/neil/repo-feature')).toBe('ssh-2') }) + it('indexes immutable worktree and repo snapshots once across repeated selector calls', () => { + let worktreeIdReads = 0 + let repoIdReads = 0 + const targetWorktreeId = 'worktree-99-99' + const targetRepoId = 'repo-99' + const worktreesByRepo: AppState['worktreesByRepo'] = {} + const repos: Repo[] = [] + + for (let repoIndex = 0; repoIndex < 100; repoIndex += 1) { + const repoId = `repo-${repoIndex}` + const repo = makeRepo({ + id: repoId, + ...(repoId === targetRepoId ? { connectionId: 'ssh-target' } : {}) + }) + Object.defineProperty(repo, 'id', { + enumerable: true, + get: () => { + repoIdReads += 1 + return repoId + } + }) + repos.push(repo) + worktreesByRepo[repoId] = Array.from({ length: 100 }, (_, worktreeIndex) => { + const worktreeId = `worktree-${repoIndex}-${worktreeIndex}` + const worktree = { repoId } as Worktree + Object.defineProperty(worktree, 'id', { + enumerable: true, + get: () => { + worktreeIdReads += 1 + return worktreeId + } + }) + return worktree + }) + } + const state: ConnectionContextState = { + folderWorkspaces: [], + projectGroups: [], + repos, + worktreesByRepo + } + + for (let lookup = 0; lookup < 200; lookup += 1) { + expect(getConnectionIdFromState(state, targetWorktreeId)).toBe('ssh-target') + } + + expect(worktreeIdReads).toBe(10_000) + expect(repoIdReads).toBe(100) + }) + it('returns null for a null worktreeId', () => { const state: ConnectionContextState = { folderWorkspaces: [], diff --git a/src/renderer/src/lib/connection-context.ts b/src/renderer/src/lib/connection-context.ts index 233ff5a2b..260b61499 100644 --- a/src/renderer/src/lib/connection-context.ts +++ b/src/renderer/src/lib/connection-context.ts @@ -1,4 +1,5 @@ import { useAppStore } from '@/store' +import { getIndexedRepoMap, getIndexedWorktreeMap } from '@/store/worktree-repo-index' import type { AppState } from '@/store/types' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import { getRepoIdFromWorktreeId } from '../../../shared/worktree-id' @@ -35,12 +36,13 @@ export function getConnectionIdFromState( if (parsedWorkspaceKey?.type === 'folder') { return getFolderWorkspaceConnectionId(state, parsedWorkspaceKey.folderWorkspaceId) } - const allWorktrees = Object.values(state.worktreesByRepo ?? {}).flat() - const worktree = allWorktrees.find((w) => w.id === worktreeId) + // Why: retained Zustand selectors call this on unrelated writes; reuse the + // immutable-slice indexes instead of flattening every worktree each time. + const worktree = getIndexedWorktreeMap(state.worktreesByRepo).get(worktreeId) // Why: SSH worktrees can be restored from session IDs before relay discovery // repopulates worktreesByRepo. The composite ID still carries the repo ID. const repoId = worktree?.repoId ?? getRepoIdFromWorktreeId(worktreeId) - const repo = state.repos?.find((r) => r.id === repoId) + const repo = getIndexedRepoMap(state.repos).get(repoId) if (!repo) { return undefined } diff --git a/src/renderer/src/store/selectors.ts b/src/renderer/src/store/selectors.ts index 1ffc7c40d..76dcb5336 100644 --- a/src/renderer/src/store/selectors.ts +++ b/src/renderer/src/store/selectors.ts @@ -4,6 +4,11 @@ import type { Repo, Worktree, TerminalTab } from '../../../shared/types' import type { AppState } from './types' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import { getProjectHostSetupProjectionFromState } from './project-host-setup-selector' +import { + getIndexedAllWorktrees as getCachedAllWorktrees, + getIndexedRepoMap as getCachedRepoMap, + getIndexedWorktreeMap as getCachedWorktreeMap +} from './worktree-repo-index' export { getProjectHostSetupProjectionFromState } from './project-host-setup-selector' @@ -12,10 +17,6 @@ const EMPTY_TABS: TerminalTab[] = [] const EMPTY_BROWSER_TABS: NonNullable = [] const EMPTY_UNIFIED_TABS: NonNullable = [] -type WorktreeSnapshot = { - allWorktrees: Worktree[] - worktreeMap: Map -} type FloatingVisibleTabCountState = Pick< AppState, 'browserTabsByWorktree' | 'openFiles' | 'tabsByWorktree' | 'unifiedTabsByWorktree' @@ -28,51 +29,9 @@ type FloatingVisibleTabCountCache = { count: number } -// Why: Zustand reruns selectors on every write, so hot-path flatten/map work -// needs cross-render caching. WeakMap ties each snapshot to the store slice ref -// without pinning old test/dev instances in memory once that slice is replaced. -const worktreeSnapshotCache = new WeakMap() const hasAnyWorktreesCache = new WeakMap() -const repoMapCache = new WeakMap>() let floatingVisibleTabCountCache: FloatingVisibleTabCountCache | null = null -function getWorktreeSnapshot(worktreesByRepo: AppState['worktreesByRepo']): WorktreeSnapshot { - const cachedSnapshot = worktreeSnapshotCache.get(worktreesByRepo) - if (cachedSnapshot) { - return cachedSnapshot - } - - // Why: a race between createWorktree (which appends) and fetchWorktrees - // (which replaces) can produce duplicate entries for the same worktree ID - // within a single repo's array. Deduplicating here prevents React from - // seeing duplicate keys, which can corrupt terminal DOM containers. - const worktreeMap = new Map() - // Why: this selector sits on hot Zustand subscription paths; avoid building - // a transient flattened array just to populate the snapshot cache. - for (const worktrees of Object.values(worktreesByRepo)) { - for (const worktree of worktrees) { - worktreeMap.set(worktree.id, worktree) - } - } - const allWorktrees = Array.from(worktreeMap.values()) - - const snapshot = { allWorktrees, worktreeMap } - worktreeSnapshotCache.set(worktreesByRepo, snapshot) - return snapshot -} - -function getCachedAllWorktrees(worktreesByRepo: AppState['worktreesByRepo']): Worktree[] { - return getWorktreeSnapshot(worktreesByRepo).allWorktrees -} - -function getCachedWorktreeMap(worktreesByRepo: AppState['worktreesByRepo']): Map { - const snapshot = worktreeSnapshotCache.get(worktreesByRepo) - if (snapshot) { - return snapshot.worktreeMap - } - return getWorktreeSnapshot(worktreesByRepo).worktreeMap -} - function getCachedHasAnyWorktrees(worktreesByRepo: AppState['worktreesByRepo']): boolean { const cached = hasAnyWorktreesCache.get(worktreesByRepo) if (cached !== undefined) { @@ -86,17 +45,6 @@ function getCachedHasAnyWorktrees(worktreesByRepo: AppState['worktreesByRepo']): return hasWorktrees } -function getCachedRepoMap(repos: AppState['repos']): Map { - const cachedMap = repoMapCache.get(repos) - if (cachedMap) { - return cachedMap - } - - const repoMap = new Map(repos.map((repo) => [repo.id, repo])) - repoMapCache.set(repos, repoMap) - return repoMap -} - export function selectFloatingVisibleTabCount(state: FloatingVisibleTabCountState): number { const terminalTabs = state.tabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_TABS const browserTabs = diff --git a/src/renderer/src/store/worktree-diff-comments-selector.test.ts b/src/renderer/src/store/worktree-diff-comments-selector.test.ts new file mode 100644 index 000000000..0f6ceb5d9 --- /dev/null +++ b/src/renderer/src/store/worktree-diff-comments-selector.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from 'vitest' +import type { DiffComment, Worktree } from '../../../shared/types' +import type { AppState } from './types' +import { + selectWorktreeDiffComments, + selectWorktreeDiffCommentsOrEmpty +} from './worktree-diff-comments-selector' +import { getIndexedWorktreeById } from './worktree-repo-index' + +function makeComment(id: string): DiffComment { + return { + id, + worktreeId: 'worktree-99-99', + filePath: 'src/index.ts', + lineNumber: 1, + body: 'Review note', + createdAt: 1, + updatedAt: 1, + source: 'diff', + side: 'modified' + } +} + +describe('selectWorktreeDiffComments', () => { + it('indexes one immutable worktree snapshot across retained editor selectors', () => { + let worktreeIdReads = 0 + const targetWorktreeId = 'worktree-99-99' + const comments = [makeComment('comment-1')] + const worktreesByRepo: AppState['worktreesByRepo'] = {} + + for (let repoIndex = 0; repoIndex < 100; repoIndex += 1) { + const repoId = `repo-${repoIndex}` + worktreesByRepo[repoId] = Array.from({ length: 100 }, (_, worktreeIndex) => { + const worktreeId = `worktree-${repoIndex}-${worktreeIndex}` + const worktree = { + repoId, + path: `/${repoId}/${worktreeId}`, + ...(worktreeId === targetWorktreeId ? { diffComments: comments } : {}) + } as Worktree + Object.defineProperty(worktree, 'id', { + enumerable: true, + get: () => { + worktreeIdReads += 1 + return worktreeId + } + }) + return worktree + }) + } + + for (let write = 0; write < 200; write += 1) { + // Model all seven comment selectors plus the rich-editor path selector. + expect(selectWorktreeDiffComments({ worktreesByRepo }, targetWorktreeId)).toBe(comments) + expect(selectWorktreeDiffComments({ worktreesByRepo }, targetWorktreeId)).toBe(comments) + expect(selectWorktreeDiffComments({ worktreesByRepo }, targetWorktreeId)).toBe(comments) + expect(selectWorktreeDiffComments({ worktreesByRepo }, targetWorktreeId)).toBe(comments) + expect(selectWorktreeDiffCommentsOrEmpty({ worktreesByRepo }, targetWorktreeId)).toBe( + comments + ) + expect(selectWorktreeDiffCommentsOrEmpty({ worktreesByRepo }, targetWorktreeId)).toBe( + comments + ) + expect(selectWorktreeDiffCommentsOrEmpty({ worktreesByRepo }, targetWorktreeId)).toBe( + comments + ) + expect(getIndexedWorktreeById(worktreesByRepo, targetWorktreeId)?.path).toBe( + '/repo-99/worktree-99-99' + ) + } + + expect(worktreeIdReads).toBe(10_000) + }) + + it('reads a replacement worktree snapshot and handles an absent id', () => { + const firstComments = [makeComment('comment-1')] + const nextComments = [makeComment('comment-2')] + const first = { repo: [{ id: 'worktree-1', diffComments: firstComments } as Worktree] } + const next = { repo: [{ id: 'worktree-1', diffComments: nextComments } as Worktree] } + + expect(selectWorktreeDiffComments({ worktreesByRepo: first }, 'worktree-1')).toBe(firstComments) + expect(selectWorktreeDiffComments({ worktreesByRepo: next }, 'worktree-1')).toBe(nextComments) + expect(selectWorktreeDiffComments({ worktreesByRepo: next }, null)).toBeUndefined() + expect(selectWorktreeDiffCommentsOrEmpty({ worktreesByRepo: next }, null)).toBe( + selectWorktreeDiffCommentsOrEmpty({ worktreesByRepo: next }, undefined) + ) + }) +}) diff --git a/src/renderer/src/store/worktree-diff-comments-selector.ts b/src/renderer/src/store/worktree-diff-comments-selector.ts new file mode 100644 index 000000000..c91ef64bc --- /dev/null +++ b/src/renderer/src/store/worktree-diff-comments-selector.ts @@ -0,0 +1,24 @@ +import type { DiffComment } from '../../../shared/types' +import type { AppState } from './types' +import { getIndexedWorktreeById } from './worktree-repo-index' + +const EMPTY_DIFF_COMMENTS = Object.freeze([]) as unknown as DiffComment[] + +export function selectWorktreeDiffComments( + state: Pick, + worktreeId: string | null | undefined +): DiffComment[] | undefined { + if (!worktreeId) { + return undefined + } + // Why: mounted Monaco and diff surfaces rerun this selector on every store + // write, so share the immutable-snapshot index instead of rescanning all worktrees. + return getIndexedWorktreeById(state.worktreesByRepo, worktreeId)?.diffComments +} + +export function selectWorktreeDiffCommentsOrEmpty( + state: Pick, + worktreeId: string | null | undefined +): DiffComment[] { + return selectWorktreeDiffComments(state, worktreeId) ?? EMPTY_DIFF_COMMENTS +} diff --git a/src/renderer/src/store/worktree-repo-index.ts b/src/renderer/src/store/worktree-repo-index.ts new file mode 100644 index 000000000..9c459f90f --- /dev/null +++ b/src/renderer/src/store/worktree-repo-index.ts @@ -0,0 +1,61 @@ +import type { Repo, Worktree } from '../../../shared/types' +import type { AppState } from './types' + +type WorktreeSnapshot = { + allWorktrees: Worktree[] + worktreeMap: Map +} + +// Why: Zustand reruns selectors on every write, so identity projections need +// cross-render caching without pinning replaced store snapshots in memory. +const worktreeSnapshotCache = new WeakMap() +const repoMapCache = new WeakMap>() + +function getWorktreeSnapshot(worktreesByRepo: AppState['worktreesByRepo']): WorktreeSnapshot { + const cachedSnapshot = worktreeSnapshotCache.get(worktreesByRepo) + if (cachedSnapshot) { + return cachedSnapshot + } + + // Why: a race between createWorktree (which appends) and fetchWorktrees + // (which replaces) can produce duplicate entries within one repo array. + const worktreeMap = new Map() + for (const worktrees of Object.values(worktreesByRepo)) { + for (const worktree of worktrees) { + worktreeMap.set(worktree.id, worktree) + } + } + const snapshot = { + allWorktrees: Array.from(worktreeMap.values()), + worktreeMap + } + worktreeSnapshotCache.set(worktreesByRepo, snapshot) + return snapshot +} + +export function getIndexedAllWorktrees(worktreesByRepo: AppState['worktreesByRepo']): Worktree[] { + return getWorktreeSnapshot(worktreesByRepo).allWorktrees +} + +export function getIndexedWorktreeMap( + worktreesByRepo: AppState['worktreesByRepo'] +): Map { + return getWorktreeSnapshot(worktreesByRepo).worktreeMap +} + +export function getIndexedWorktreeById( + worktreesByRepo: AppState['worktreesByRepo'], + worktreeId: string +): Worktree | undefined { + return getWorktreeSnapshot(worktreesByRepo).worktreeMap.get(worktreeId) +} + +export function getIndexedRepoMap(repos: AppState['repos']): Map { + const cachedMap = repoMapCache.get(repos) + if (cachedMap) { + return cachedMap + } + const repoMap = new Map(repos.map((repo) => [repo.id, repo])) + repoMapCache.set(repos, repoMap) + return repoMap +}