mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
fix(diagnostics): drop the open-file cap, keep the heap census
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.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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<PersistedOpenFile> = {}) {
|
||||
return {
|
||||
filePath: `/repo/src/file-${index}.ts`,
|
||||
relativePath: `src/file-${index}.ts`,
|
||||
worktreeId: 'wt-1',
|
||||
language: 'typescript',
|
||||
...overrides
|
||||
} satisfies PersistedOpenFile
|
||||
}
|
||||
|
||||
function sessionWith(
|
||||
openFilesByWorktree: Record<string, PersistedOpenFile[]>,
|
||||
activeFileIdByWorktree?: Record<string, string>
|
||||
): 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)
|
||||
})
|
||||
})
|
||||
@@ -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<string, PersistedOpenFile[]> | 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<number>()
|
||||
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)}`)
|
||||
}
|
||||
Reference in New Issue
Block a user