From 85f6c7f2e5ff549aadb07434aff0e7dcadfdaf55 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 1 Aug 2026 05:18:48 -0700 Subject: [PATCH] fix(diagnostics): drop the open-file cap, keep the heap census MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review refuted the cap on its own evidence, so it comes out and the instrumentation ships alone. - Not restore-only: patchWorkspaceSession top-level-spreads the patch (persistence.ts:6226) and buildPersistedOpenFiles rebuilds openFilesByWorktree wholesale from the in-memory list (workspace-session.ts:114), so the first save after a capped restore deletes the dropped entries from disk permanently, with no undo. - Its consistency claim was false: hydration only backing-file-checks transient content types (tabs-hydration.ts:95), so capping the files leaves the unified editor tabs behind — tab chrome pointing at a document that no longer exists, re-orphaned on every boot. - The bundles do not support it: where the counter is recorded, the <=200-open-file cohort's mean heap at the 80% highwater (3372MB) is higher than the >500 cohort's (3260MB). Open-file count does not track this OOM. The census stays: median 835MB of growth between the 60% and 80% thresholds while store counters barely move says the problem is per-object size, not collection count, and editorContent.chars is aimed exactly there. --- src/renderer/src/App.tsx | 5 +- .../workspace-session-open-file-cap.test.ts | 115 ------------------ .../lib/workspace-session-open-file-cap.ts | 70 ----------- 3 files changed, 1 insertion(+), 189 deletions(-) delete mode 100644 src/renderer/src/lib/workspace-session-open-file-cap.test.ts delete mode 100644 src/renderer/src/lib/workspace-session-open-file-cap.ts diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 9c7361fe7fa..9b34a530029 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -137,7 +137,6 @@ import { collectFolderWorkspaceKeysFromSession, collectWorktreeHydrationRepoIdsFromSession } from './lib/workspace-session-hydration-keys' -import { capRestoredOpenFilesByWorktree } from './lib/workspace-session-open-file-cap' import { getStartupErrorFallbackUI, hydratePersistedUIAfterStartupRead @@ -964,9 +963,7 @@ function App(): React.JSX.Element { actions.awaitLocalRepoCatalogSettlement() ) if (!cancelled) { - // Why: cap before any slice reads it — editor, tab and tab-group hydration - // all derive from openFilesByWorktree and must agree on the same list. - const restoredSession = capRestoredOpenFilesByWorktree(sessionRead.session) + const restoredSession = sessionRead.session const sessionHydrationOptions = { additionalValidWorkspaceKeys: collectFolderWorkspaceKeysFromSession(restoredSession) } diff --git a/src/renderer/src/lib/workspace-session-open-file-cap.test.ts b/src/renderer/src/lib/workspace-session-open-file-cap.test.ts deleted file mode 100644 index f880c43e74f..00000000000 --- a/src/renderer/src/lib/workspace-session-open-file-cap.test.ts +++ /dev/null @@ -1,115 +0,0 @@ -import { describe, expect, it } from 'vitest' -import type { PersistedOpenFile, WorkspaceSessionState } from '../../../shared/types' -import { - capRestoredOpenFilesByWorktree, - MAX_RESTORED_OPEN_FILES_PER_WORKSPACE -} from './workspace-session-open-file-cap' - -function persistedFile(index: number, overrides: Partial = {}) { - return { - filePath: `/repo/src/file-${index}.ts`, - relativePath: `src/file-${index}.ts`, - worktreeId: 'wt-1', - language: 'typescript', - ...overrides - } satisfies PersistedOpenFile -} - -function sessionWith( - openFilesByWorktree: Record, - activeFileIdByWorktree?: Record -): WorkspaceSessionState { - return { openFilesByWorktree, activeFileIdByWorktree } as unknown as WorkspaceSessionState -} - -describe('capRestoredOpenFilesByWorktree', () => { - it('returns the same session object when every workspace is under the cap', () => { - const session = sessionWith({ - 'wt-1': Array.from({ length: MAX_RESTORED_OPEN_FILES_PER_WORKSPACE }, (_, i) => - persistedFile(i) - ) - }) - - expect(capRestoredOpenFilesByWorktree(session)).toBe(session) - }) - - it('keeps the newest entries when a workspace exceeds the cap', () => { - const total = MAX_RESTORED_OPEN_FILES_PER_WORKSPACE + 50 - const session = sessionWith({ - 'wt-1': Array.from({ length: total }, (_, i) => persistedFile(i)) - }) - - const kept = capRestoredOpenFilesByWorktree(session).openFilesByWorktree?.['wt-1'] ?? [] - - expect(kept).toHaveLength(MAX_RESTORED_OPEN_FILES_PER_WORKSPACE) - expect(kept[0]?.filePath).toBe(persistedFile(50).filePath) - expect(kept.at(-1)?.filePath).toBe(persistedFile(total - 1).filePath) - }) - - it('never drops a file with an unsaved draft, even from the oldest entries', () => { - const total = MAX_RESTORED_OPEN_FILES_PER_WORKSPACE + 50 - const files = Array.from({ length: total }, (_, i) => - persistedFile(i, i === 0 ? { dirtyDraftContent: 'unsaved work' } : {}) - ) - - const kept = capRestoredOpenFilesByWorktree(sessionWith({ 'wt-1': files })) - .openFilesByWorktree?.['wt-1'] - - expect(kept?.[0]?.dirtyDraftContent).toBe('unsaved work') - expect(kept).toHaveLength(MAX_RESTORED_OPEN_FILES_PER_WORKSPACE) - }) - - it('keeps every dirty file even when they alone exceed the cap', () => { - const total = MAX_RESTORED_OPEN_FILES_PER_WORKSPACE + 20 - const files = Array.from({ length: total }, (_, i) => - persistedFile(i, { dirtyDraftContent: `draft ${i}` }) - ) - - const kept = capRestoredOpenFilesByWorktree(sessionWith({ 'wt-1': files })) - .openFilesByWorktree?.['wt-1'] - - expect(kept).toHaveLength(total) - }) - - it('keeps the focused tab addressed by its raw path id', () => { - const total = MAX_RESTORED_OPEN_FILES_PER_WORKSPACE + 50 - const files = Array.from({ length: total }, (_, i) => persistedFile(i)) - const session = sessionWith({ 'wt-1': files }, { 'wt-1': files[0].filePath }) - - const kept = capRestoredOpenFilesByWorktree(session).openFilesByWorktree?.['wt-1'] ?? [] - - expect(kept[0]?.filePath).toBe(files[0].filePath) - }) - - it('keeps the focused tab addressed by its owner-qualified id', () => { - const total = MAX_RESTORED_OPEN_FILES_PER_WORKSPACE + 50 - const files = Array.from({ length: total }, (_, i) => persistedFile(i)) - const ownedId = `editor:wt-1:local:${encodeURIComponent(files[0].filePath)}` - const session = sessionWith({ 'wt-1': files }, { 'wt-1': ownedId }) - - const kept = capRestoredOpenFilesByWorktree(session).openFilesByWorktree?.['wt-1'] ?? [] - - expect(kept[0]?.filePath).toBe(files[0].filePath) - }) - - it('caps each workspace independently and leaves untouched ones referentially stable', () => { - const small = [persistedFile(1), persistedFile(2)] - const large = Array.from({ length: MAX_RESTORED_OPEN_FILES_PER_WORKSPACE + 5 }, (_, i) => - persistedFile(i) - ) - const session = sessionWith({ 'wt-small': small, 'wt-large': large }) - - const capped = capRestoredOpenFilesByWorktree(session) - - expect(capped.openFilesByWorktree?.['wt-small']).toBe(small) - expect(capped.openFilesByWorktree?.['wt-large']).toHaveLength( - MAX_RESTORED_OPEN_FILES_PER_WORKSPACE - ) - }) - - it('tolerates a session with no persisted editor files', () => { - const session = { activeFileIdByWorktree: {} } as unknown as WorkspaceSessionState - - expect(capRestoredOpenFilesByWorktree(session)).toBe(session) - }) -}) diff --git a/src/renderer/src/lib/workspace-session-open-file-cap.ts b/src/renderer/src/lib/workspace-session-open-file-cap.ts deleted file mode 100644 index 26721829046..00000000000 --- a/src/renderer/src/lib/workspace-session-open-file-cap.ts +++ /dev/null @@ -1,70 +0,0 @@ -import type { PersistedOpenFile, WorkspaceSessionState } from '../../../shared/types' - -/** - * Restore-time bound on persisted editor tabs, per workspace. - * - * Why: `openFilesByWorktree` is append-only and never pruned, so it accumulates - * across every restart. Crash bundles show sessions restoring 3,711 editor tabs - * into one renderer. 100 is far above any real working set. - */ -export const MAX_RESTORED_OPEN_FILES_PER_WORKSPACE = 100 - -/** - * Applied to the session once, before any slice hydrates it: the editor, tab and - * tab-group slices all derive their state from `openFilesByWorktree`, so capping - * it here keeps them consistent instead of leaving orphaned tabs behind. - */ -export function capRestoredOpenFilesByWorktree( - session: WorkspaceSessionState -): WorkspaceSessionState { - const openFilesByWorktree = session.openFilesByWorktree - if (!openFilesByWorktree) { - return session - } - - let capped: Record | null = null - for (const [workspaceKey, files] of Object.entries(openFilesByWorktree)) { - if (files.length <= MAX_RESTORED_OPEN_FILES_PER_WORKSPACE) { - continue - } - capped ??= { ...openFilesByWorktree } - capped[workspaceKey] = selectRestorableOpenFiles( - files, - session.activeFileIdByWorktree?.[workspaceKey] - ) - } - - return capped ? { ...session, openFilesByWorktree: capped } : session -} - -function selectRestorableOpenFiles( - files: readonly PersistedOpenFile[], - activeFileId: string | null | undefined -): PersistedOpenFile[] { - const keep = new Set() - files.forEach((file, index) => { - // Why: an unsaved draft is unrecoverable once dropped, and losing the tab the - // user left focused is the one omission they would notice immediately. - if (file.dirtyDraftContent !== undefined || addressesFile(activeFileId, file.filePath)) { - keep.add(index) - } - }) - // Why: newly opened files are appended, so the tail is the most recent working set. - for ( - let index = files.length - 1; - index >= 0 && keep.size < MAX_RESTORED_OPEN_FILES_PER_WORKSPACE; - index -= 1 - ) { - keep.add(index) - } - return files.filter((_, index) => keep.has(index)) -} - -// Why: hydration keys a file either by its raw path or by an owner-qualified id -// whose final segment is the encoded path; both forms reach activeFileIdByWorktree. -function addressesFile(activeFileId: string | null | undefined, filePath: string): boolean { - if (!activeFileId) { - return false - } - return activeFileId === filePath || activeFileId.endsWith(`:${encodeURIComponent(filePath)}`) -}