perf: skip store notifications for unchanged project and folder catalogs (#23104)

* perf: skip store notifications for unchanged project and folder catalogs * perf(store): stop the all-host folder refresh from republishing equal restored owners The catalog gate landed in this branch suppressed the two catalog publications of an unchanged all-host folder refresh but left the trailing restored-session-owner cleanup, which rebuilt `restoredRuntimeHostIdByWorkspaceSessionKey` into a fresh object on every refresh and so always replaced the store root. Reference-equality readers (the live dashboard selector, the popout bridge, the tab-create entry gate) re-ran on that fresh-but-equal identity, so an unchanged all-host refresh still cost a full publication: 3 -> 1 rather than 3 -> 0. Reuse `reuseEqualRecordMap` to keep the previous record when the cleanup produces an equal one, and return the current state when it does. A cleanup that really retires an owner still publishes. Also seed the session writer from the current state at creation. `prev === null` is what bootstraps its first full write, so a writer created when the session gate was already open owed that write to whatever unrelated store tick arrived next. With equal catalogs no longer publishing, that incidental wake-up is no longer guaranteed; evaluating once at creation matches what editor-autosave-controller already does. * test(store): pin the session writer's creation-time seed The seed this branch added is the compensating fix for a real regression — it removed the equal-catalog tick that used to boot the writer's first full write — but nothing held it in place. Every existing subscriber case creates the writer with the gate closed and opens it afterwards, so the opening `setState` is itself the tick that produces that first write; deleting the seed left all five suites green. Cover the case the seed exists for: open `workspaceSessionReady` and `hydrationSucceeded` *before* creating the subscriber, then assert `persist` fires with a store-tick spy proving nothing woke it. Verified it fails (`persist` called 0 times) with the seed line removed and is the only failure. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
2026-09-27 08:02:35 +00:00 · 2026-09-27 00:28:39 -07:00
parent d301658208
commit 8e6c07178e
5 changed files with 290 additions and 18 deletions
@@ -102,6 +102,27 @@ describe('createSessionWriteSubscriber', () => {
    cleanup()
  })

+  it('writes when the gate is already open at creation, with no store tick to wake it', () => {
+    // Why the store-write spy: every other case here opens the gate *after* creating the
+    // subscriber, so the opening setState is itself the tick that produces the first write. With
+    // the gate already open only the creation-time seed can, and equal catalogs no longer publish
+    // a tick to stand in for it.
+    useAppStore.setState({ workspaceSessionReady: true, hydrationSucceeded: true })
+
+    const persist = vi.fn<(payload: WorkspaceSessionWrite) => void>()
+    const storeTicks = vi.fn()
+    const unsubStoreTicks = useAppStore.subscribe(storeTicks)
+    const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist })
+
+    vi.advanceTimersByTime(200)
+
+    expect(storeTicks).not.toHaveBeenCalled()
+    expect(persist).toHaveBeenCalledTimes(1)
+
+    unsubStoreTicks()
+    cleanup()
+  })
+
  it('re-checks the hydration gate when a pending debounce fires', () => {
    const persist = vi.fn<(payload: WorkspaceSessionWrite) => void>()
    const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist })
@@ -201,7 +201,7 @@ export function createSessionWriteSubscriber({
    return false
  }

-  const unsub = store.subscribe((state) => {
+  const evaluateSessionState = (state: AppState): void => {
    if (!shouldPersistWorkspaceSession(state)) {
      return
    }
@@ -255,7 +255,14 @@ export function createSessionWriteSubscriber({
      return
    }
    armFlushTimer()
-  })
+  }
+
+  // Why evaluate once here: `prev === null` is what bootstraps the first full write, so a writer
+  // created when the session gate is *already* open owed that write to whatever unrelated store
+  // tick happened to arrive next. Catalog refreshes no longer publish when nothing changed, so
+  // that incidental wake-up is not guaranteed; seed from the current state instead.
+  evaluateSessionState(store.getState())
+  const unsub = store.subscribe(evaluateSessionState)

  const unsubGateOpen = subscribeToPersistGateOpen?.(() => {
    if (pendingChangedFields.size === 0 || timer !== null) {
@@ -13,6 +13,7 @@ import {
  mergeFetchedFolderWorkspaceCatalog
 } from './folder-workspace-catalog'
 import { listRuntimeEnvironmentsForAllHostLoad } from '../runtime-catalog-hosts'
+import { reuseEqualRecordMap } from '../slices/repo-identity-reconcile'
 import { getFolderWorkspaceUpdateCoordinator } from './folder-workspace-mutations'

 export function createFolderWorkspaceCatalogActions(
@@ -47,11 +48,12 @@ export function createFolderWorkspaceCatalogActions(
            current.folderWorkspaces,
            current.projectGroups
          )
+          if (arrayElementsUnchanged(folderWorkspaces, current.folderWorkspaces)) {
+            return current
+          }
          return {
            folderWorkspaces,
-            ...(arrayElementsUnchanged(folderWorkspaces, current.folderWorkspaces)
-              ? {}
-              : { folderWorkspacePathStatuses: {} })
+            folderWorkspacePathStatuses: {}
          }
        })
      } catch (err) {
@@ -85,11 +87,12 @@ export function createFolderWorkspaceCatalogActions(
            current.folderWorkspaces,
            current.projectGroups
          )
+          if (arrayElementsUnchanged(folderWorkspaces, current.folderWorkspaces)) {
+            return current
+          }
          return {
            folderWorkspaces,
-            ...(arrayElementsUnchanged(folderWorkspaces, current.folderWorkspaces)
-              ? {}
-              : { folderWorkspacePathStatuses: {} })
+            folderWorkspacePathStatuses: {}
          }
        })
      }
@@ -130,12 +133,22 @@ export function createFolderWorkspaceCatalogActions(
        })
      )
      if (!failed) {
-        set((s) => ({
-          restoredRuntimeHostIdByWorkspaceSessionKey: clearRestoredFolderWorkspaceSessionOwners(
+        set((s) => {
+          // Why reuseEqualRecordMap: the cleanup rebuilds the record every refresh, and
+          // reference-equality readers (live dashboard selector, popout bridge) re-run on a
+          // fresh-but-equal identity, so the equal case must keep the previous one.
+          const restoredRuntimeHostIdByWorkspaceSessionKey = reuseEqualRecordMap(
            s.restoredRuntimeHostIdByWorkspaceSessionKey,
-            s
+            clearRestoredFolderWorkspaceSessionOwners(
+              s.restoredRuntimeHostIdByWorkspaceSessionKey,
+              s
+            )
          )
-        }))
+          return restoredRuntimeHostIdByWorkspaceSessionKey ===
+            s.restoredRuntimeHostIdByWorkspaceSessionKey
+            ? s
+            : { restoredRuntimeHostIdByWorkspaceSessionKey }
+        })
      }
    }
  }
@@ -32,11 +32,12 @@ export function createProjectGroupCatalogActions(
            return current
          }
          const { projectGroups } = mergeFetchedProjectGroupCatalog(catalog, current.projectGroups)
+          if (arrayElementsUnchanged(projectGroups, current.projectGroups)) {
+            return current
+          }
          return {
            projectGroups,
-            ...(arrayElementsUnchanged(projectGroups, current.projectGroups)
-              ? {}
-              : { folderWorkspacePathStatuses: {} })
+            folderWorkspacePathStatuses: {}
          }
        })
      } catch (err) {
@@ -55,11 +56,12 @@ export function createProjectGroupCatalogActions(
            return s
          }
          const { projectGroups } = mergeFetchedProjectGroupCatalog(catalog, s.projectGroups)
+          if (arrayElementsUnchanged(projectGroups, s.projectGroups)) {
+            return s
+          }
          return {
            projectGroups,
-            ...(arrayElementsUnchanged(projectGroups, s.projectGroups)
-              ? {}
-              : { folderWorkspacePathStatuses: {} })
+            folderWorkspacePathStatuses: {}
          }
        })
      }
@@ -0,0 +1,229 @@
+import { afterEach, beforeEach, expect, it, vi } from 'vitest'
+import type { FolderWorkspace } from '../../../../shared/folder-workspace-types'
+import type { ProjectGroup } from '../../../../shared/project-group-types'
+import { createTestStore } from './store-test-helpers'
+
+const group: ProjectGroup = {
+  id: 'group-1',
+  name: 'Group',
+  parentPath: '/parent',
+  parentGroupId: null,
+  createdFrom: 'manual',
+  tabOrder: 0,
+  isCollapsed: false,
+  color: null,
+  createdAt: 1,
+  updatedAt: 3
+}
+const folder: FolderWorkspace = {
+  id: 'folder-1',
+  projectGroupId: group.id,
+  name: 'Folder',
+  folderPath: '/parent/folder',
+  linkedTask: null,
+  comment: '',
+  isArchived: false,
+  isUnread: false,
+  isPinned: false,
+  sortOrder: 0,
+  lastActivityAt: 1,
+  createdAt: 1,
+  updatedAt: 3
+}
+const groupsList = vi.fn<() => Promise<ProjectGroup[]>>()
+const foldersList = vi.fn<() => Promise<FolderWorkspace[]>>()
+const folderUpdate = vi.fn<Window['api']['folderWorkspaces']['update']>()
+type TestStore = ReturnType<typeof createTestStore>
+type CatalogCase = {
+  label: string
+  catalog: 'projectGroups' | 'folderWorkspaces'
+  refresh: (store: TestStore) => Promise<void>
+}
+const cases: CatalogCase[] = [
+  {
+    label: 'selected groups',
+    catalog: 'projectGroups',
+    refresh: (s) => s.getState().fetchProjectGroups()
+  },
+  {
+    label: 'all-host local groups',
+    catalog: 'projectGroups',
+    refresh: (s) => s.getState().fetchProjectGroupsForAllHosts({ remoteHosts: 'skip' })
+  },
+  {
+    label: 'selected folders',
+    catalog: 'folderWorkspaces',
+    refresh: (s) => s.getState().fetchFolderWorkspaces()
+  },
+  {
+    label: 'all-host local folders',
+    catalog: 'folderWorkspaces',
+    refresh: (s) => s.getState().fetchFolderWorkspacesForAllHosts({ remoteHosts: 'skip' })
+  }
+]
+
+beforeEach(() => {
+  groupsList.mockReset().mockImplementation(async () => structuredClone([group]))
+  foldersList.mockReset().mockImplementation(async () => structuredClone([folder]))
+  folderUpdate.mockReset()
+  vi.stubGlobal('window', {
+    api: {
+      projectGroups: { list: groupsList },
+      folderWorkspaces: { list: foldersList, update: folderUpdate },
+      runtimeEnvironments: { list: async () => [] }
+    },
+    dispatchEvent: vi.fn()
+  })
+  vi.spyOn(console, 'error').mockImplementation(() => {})
+})
+afterEach(() => {
+  vi.restoreAllMocks()
+  vi.unstubAllGlobals()
+})
+
+function seed(): TestStore {
+  const store = createTestStore()
+  store.setState({
+    projectGroups: [{ ...group, executionHostId: 'local' }],
+    folderWorkspaces: [{ ...folder, executionHostId: 'local' }],
+    folderWorkspacePathStatuses: {
+      cached: {
+        status: { path: folder.folderPath, exists: true },
+        checkedAt: 1,
+        requestSnapshot: 'snapshot'
+      }
+    }
+  })
+  return store
+}
+
+it.each(cases)('does not notify for ten equal $label refreshes', async ({ refresh, catalog }) => {
+  const store = seed()
+  const initial = store.getState()
+  const changed = vi.fn()
+  const unsubscribe = store.subscribe(changed)
+  for (let index = 0; index < 10; index += 1) {
+    await refresh(store)
+  }
+  unsubscribe()
+  expect(changed).not.toHaveBeenCalled()
+  expect(store.getState()).toBe(initial)
+  expect(store.getState()[catalog]).toBe(initial[catalog])
+  expect(store.getState().folderWorkspacePathStatuses).toBe(initial.folderWorkspacePathStatuses)
+})
+
+it.each(cases)(
+  'publishes changed $label and invalidates path statuses',
+  async ({ refresh, catalog }) => {
+    const store = seed()
+    groupsList.mockResolvedValue([{ ...group, name: 'Changed group' }])
+    foldersList.mockResolvedValue([{ ...folder, name: 'Changed folder' }])
+    const changed = vi.fn()
+    const unsubscribe = store.subscribe(changed)
+    await refresh(store)
+    unsubscribe()
+    expect(changed).toHaveBeenCalledOnce()
+    expect(store.getState()[catalog][0]?.name).toMatch(/^Changed /)
+    expect(store.getState().folderWorkspacePathStatuses).toEqual({})
+  }
+)
+
+it.each(cases)('retains state after a failed $label read', async ({ refresh }) => {
+  const store = seed()
+  const initial = store.getState()
+  groupsList.mockRejectedValue(new Error('offline'))
+  foldersList.mockRejectedValue(new Error('offline'))
+  const changed = vi.fn()
+  const unsubscribe = store.subscribe(changed)
+  await refresh(store)
+  unsubscribe()
+  expect(store.getState()).toBe(initial)
+  expect(changed).not.toHaveBeenCalled()
+})
+
+it.each(cases)(
+  'fences an older $label response after a newer no-op response',
+  async ({ refresh, catalog }) => {
+    const store = seed()
+    const initial = store.getState()
+    const olderGroups = Promise.withResolvers<ProjectGroup[]>()
+    const olderFolders = Promise.withResolvers<FolderWorkspace[]>()
+    if (catalog === 'projectGroups') {
+      groupsList.mockReturnValueOnce(olderGroups.promise)
+    } else {
+      foldersList.mockReturnValueOnce(olderFolders.promise)
+    }
+    const older = refresh(store)
+    const changed = vi.fn()
+    const unsubscribe = store.subscribe(changed)
+    await refresh(store)
+    olderGroups.resolve([{ ...group, name: 'Obsolete group' }])
+    olderFolders.resolve([{ ...folder, name: 'Obsolete folder' }])
+    await older
+    unsubscribe()
+    expect(store.getState()).toBe(initial)
+    expect(changed).not.toHaveBeenCalled()
+  }
+)
+
+it.each(cases.filter((entry) => entry.catalog === 'folderWorkspaces'))(
+  'keeps update revision fencing after an equal $label response',
+  async ({ refresh }) => {
+    const store = seed()
+    const pending = Promise.withResolvers<FolderWorkspace>()
+    folderUpdate.mockReturnValueOnce(pending.promise)
+    const update = store.getState().updateFolderWorkspace(folder.id, { isUnread: true })
+    const initial = store.getState()
+    await refresh(store)
+    expect(store.getState()).toBe(initial)
+    pending.resolve({ ...folder, isUnread: true, updatedAt: 2 })
+    await update
+    expect(store.getState().folderWorkspaces[0]?.isUnread).toBe(false)
+    expect(store.getState().folderWorkspaces[0]?.updatedAt).toBe(3)
+  }
+)
+
+it('accepts a newer update after an equal catalog response', async () => {
+  const store = seed()
+  const pending = Promise.withResolvers<FolderWorkspace>()
+  folderUpdate.mockReturnValueOnce(pending.promise)
+  const update = store.getState().updateFolderWorkspace(folder.id, { isUnread: true })
+  await store.getState().fetchFolderWorkspaces()
+  pending.resolve({ ...folder, isUnread: true, updatedAt: 4 })
+  await update
+  expect(store.getState().folderWorkspaces[0]?.isUnread).toBe(true)
+  expect(store.getState().folderWorkspaces[0]?.updatedAt).toBe(4)
+})
+
+it('still clears restored folder owners after a successful all-host refresh', async () => {
+  const store = seed()
+  store.setState({
+    restoredRuntimeHostIdByWorkspaceSessionKey: {
+      'folder:folder-1': 'runtime:retired',
+      unrelated: 'runtime:kept'
+    }
+  })
+  const changed = vi.fn()
+  const unsubscribe = store.subscribe(changed)
+  await store.getState().fetchFolderWorkspacesForAllHosts()
+  unsubscribe()
+  expect(changed).toHaveBeenCalledOnce()
+  expect(store.getState().restoredRuntimeHostIdByWorkspaceSessionKey).toEqual({
+    unrelated: 'runtime:kept'
+  })
+})
+
+it('does not publish an all-host refresh whose restored owners need no cleanup', async () => {
+  const store = seed()
+  store.setState({ restoredRuntimeHostIdByWorkspaceSessionKey: { unrelated: 'runtime:kept' } })
+  const initial = store.getState()
+  const changed = vi.fn()
+  const unsubscribe = store.subscribe(changed)
+  await store.getState().fetchFolderWorkspacesForAllHosts()
+  unsubscribe()
+  expect(changed).not.toHaveBeenCalled()
+  expect(store.getState()).toBe(initial)
+  expect(store.getState().restoredRuntimeHostIdByWorkspaceSessionKey).toBe(
+    initial.restoredRuntimeHostIdByWorkspaceSessionKey
+  )
+})