perf(store): keep the churn probe off the shipped write path

Review hardening for the churn probe and the widened lint rules.

Probe: it no longer resolves a functional updater itself. Zustand keeps sole
ownership of when and with what argument an updater runs, so the middleware
cannot double-invoke it or hand it a stale state. Object partials still scope
the scan to the write's own keys; updater and replace writes fall back to the
full field list, which costs one Object.is per untouched field and nothing
more, since the deep compare only runs on replaced references.

store/index.ts installs the probe only when import.meta.env.DEV or
e2eConfig.exposeStore is set, the same gate as __store exposure. Nothing in the
app arms it, so a shipped build was paying a wrapper frame per write for a
diagnostic it could never read. The cascade probe stays unconditional because
crash telemetry arms it in the field.

Site capture now skips any *-probe.ts frame; under the real composition the
first non-node_modules frame was the cascade probe's wrapper, so every churn
was attributed to react-commit-cascade-write-probe.ts:32 instead of the caller.

Plugin: named-selector recording is restricted to module scope. A
component-local `const selectRows = ...` used to overwrite the entry for a
same-named imported selector and flag an unrelated useAppStore(selectRows).
The any-branch and every-branch allocation predicates are one function with a
flag, the Object.* static list is a Set, and import recording is a single pass.

Tests: updater called once with live state, identical-state writes ignored,
disarmed path forwards exact arguments without calling get(), full composition
with the cascade probe (no drop, no double, correct site), and the
module-scope shadowing case for the plugin.

config/oxlint-plugins/app-store-performance.mjs

@@ -8,6 +8,19 @@ const ALLOCATING_METHODS = new Set([
   'toSpliced',
   'with'
 ])
+const ALLOCATING_OBJECT_STATICS = new Set([
+  'assign',
+  'create',
+  'entries',
+  'fromEntries',
+  'keys',
+  'values'
+])
+const FUNCTION_NODES = new Set([
+  'ArrowFunctionExpression',
+  'FunctionDeclaration',
+  'FunctionExpression'
+])
 
 function identifierName(node) {
   return node?.type === 'Identifier' ? node.name : null
@@ -25,8 +38,12 @@ function propertyName(node) {
     : null
 }
 
+function functionNode(node) {
+  return FUNCTION_NODES.has(node?.type) ? node : null
+}
+
 function returnedExpressions(selector) {
-  if (selector?.type !== 'ArrowFunctionExpression' && selector?.type !== 'FunctionExpression') {
+  if (!functionNode(selector)) {
     return []
   }
   if (selector.body.type !== 'BlockStatement') {
@@ -37,10 +54,7 @@ function returnedExpressions(selector) {
     if (!node || typeof node !== 'object') {
       return
     }
-    if (
-      node !== selector.body &&
-      ['ArrowFunctionExpression', 'FunctionDeclaration', 'FunctionExpression'].includes(node.type)
-    ) {
+    if (node !== selector.body && FUNCTION_NODES.has(node.type)) {
       return
     }
     if (node.type === 'ReturnStatement') {
@@ -76,10 +90,7 @@ function unwrapShallowSelector(selector, shallowHooks) {
 }
 
 function isIdentitySelector(selector) {
-  if (selector?.type !== 'ArrowFunctionExpression' && selector?.type !== 'FunctionExpression') {
-    return false
-  }
-  const parameter = selector.params[0]
+  const parameter = functionNode(selector)?.params[0]
   if (parameter?.type !== 'Identifier') {
     return false
   }
@@ -88,14 +99,23 @@ function isIdentitySelector(selector) {
   )
 }
 
-function isAllocatingExpression(expression) {
-  if (expression?.type === 'ConditionalExpression') {
-    return (
-      isAllocatingExpression(expression.consequent) || isAllocatingExpression(expression.alternate)
-    )
-  }
-  if (expression?.type === 'LogicalExpression') {
-    return isAllocatingExpression(expression.left) || isAllocatingExpression(expression.right)
+/**
+ * `everyBranch` decides how a conditional counts. An inline selector is flagged
+ * when ANY branch allocates; a helper the selector delegates to must allocate on
+ * EVERY branch, so the `cache.get(k) ?? build(state)` identity-caching shape is
+ * not a false positive.
+ */
+function allocates(expression, everyBranch) {
+  const branches =
+    expression?.type === 'ConditionalExpression'
+      ? [expression.consequent, expression.alternate]
+      : expression?.type === 'LogicalExpression'
+        ? [expression.left, expression.right]
+        : null
+  if (branches) {
+    return everyBranch
+      ? branches.every((branch) => allocates(branch, true))
+      : branches.some((branch) => allocates(branch, false))
   }
   if (
     expression?.type === 'ArrayExpression' ||
@@ -107,23 +127,16 @@ function isAllocatingExpression(expression) {
   if (expression?.type !== 'CallExpression') {
     return false
   }
-  const method = propertyName(expression.callee)
-  if (method && ALLOCATING_METHODS.has(method)) {
-    return true
-  }
   const callee = expression.callee
+  const method = propertyName(callee)
   return (
-    callee.type === 'MemberExpression' &&
-    identifierName(callee.object) === 'Object' &&
-    ['assign', 'create', 'entries', 'fromEntries', 'keys', 'values'].includes(propertyName(callee))
+    ALLOCATING_METHODS.has(method) ||
+    (identifierName(callee.object) === 'Object' && ALLOCATING_OBJECT_STATICS.has(method))
   )
 }
 
-function importedLocalName(specifier, importedName) {
-  if (specifier.type !== 'ImportSpecifier' || identifierName(specifier.imported) !== importedName) {
-    return null
-  }
-  return identifierName(specifier.local)
+function isAllocatingExpression(expression) {
+  return allocates(expression, false)
 }
 
 // Project-local zustand hooks follow the use<Name>Store convention; React's
@@ -135,29 +148,27 @@ function isLocalModuleSource(source) {
   return typeof source === 'string' && (source.startsWith('.') || source.startsWith('@/'))
 }
 
-function isStoreHookName(name) {
-  return typeof name === 'string' && STORE_HOOK_NAME.test(name) && !NON_STORE_HOOKS.has(name)
-}
-
-function selectorFunction(node) {
-  return node?.type === 'ArrowFunctionExpression' || node?.type === 'FunctionExpression'
-    ? node
-    : null
+/** Module scope only: a component-local helper must not shadow a same-named import. */
+function isModuleScope(node) {
+  const parent = node.parent
+  return (
+    parent?.type === 'Program' ||
+    (parent?.type === 'ExportNamedDeclaration' && parent.parent?.type === 'Program')
+  )
 }
 
 /** Records module-scope `const selectX = (state) => ...` so identifier selectors resolve. */
 function recordNamedSelector(node, state) {
-  if (node.type === 'FunctionDeclaration') {
-    const name = identifierName(node.id)
-    if (name) {
-      state.namedSelectors.set(name, node)
-    }
+  if (!isModuleScope(node)) {
     return
   }
-  for (const declarator of node.declarations ?? []) {
-    const name = identifierName(declarator.id)
-    const initializer = selectorFunction(declarator.init)
-    if (name && initializer) {
+  const declared =
+    node.type === 'FunctionDeclaration'
+      ? [[node.id, node]]
+      : node.declarations.map((declarator) => [declarator.id, declarator.init])
+  for (const [id, initializer] of declared) {
+    const name = identifierName(id)
+    if (name && functionNode(initializer)) {
       state.namedSelectors.set(name, initializer)
     }
   }
@@ -165,51 +176,23 @@ function recordNamedSelector(node, state) {
 
 /** Inline function, or a module-scope selector referenced by name. */
 function resolveSelector(argument, state) {
-  const inline = selectorFunction(argument)
-  if (inline) {
-    return inline
-  }
-  const name = identifierName(argument)
-  return name ? (state.namedSelectors.get(name) ?? null) : null
-}
-
-/**
- * Allocation that happens on EVERY call, used when following a selector into a
- * helper. Deliberately stricter than isAllocatingExpression: a helper that returns
- * a cached reference on one branch and builds a fresh one on another is the normal
- * identity-caching shape, and flagging it would be a false positive.
- */
-function alwaysAllocates(expression) {
-  if (expression?.type === 'ConditionalExpression') {
-    return alwaysAllocates(expression.consequent) && alwaysAllocates(expression.alternate)
-  }
-  if (expression?.type === 'LogicalExpression') {
-    return alwaysAllocates(expression.left) && alwaysAllocates(expression.right)
-  }
-  return (
-    expression?.type === 'ArrayExpression' ||
-    expression?.type === 'ObjectExpression' ||
-    expression?.type === 'NewExpression' ||
-    (expression?.type === 'CallExpression' &&
-      ALLOCATING_METHODS.has(propertyName(expression.callee) ?? ''))
-  )
+  return functionNode(argument) ?? state.namedSelectors.get(identifierName(argument)) ?? null
 }
 
 /**
  * One hop: a selector that delegates to a module-scope helper is the idiomatic
  * shape here, and neither the inline-body check nor a reviewer reading the call
- * site can see what that helper returns.
+ * site can see what that helper returns. An unresolvable helper is left alone.
  */
 function expandThroughNamedHelper(expression, state) {
-  if (expression?.type !== 'CallExpression') {
-    return [expression]
-  }
-  const helper = state.namedSelectors.get(identifierName(expression.callee) ?? '')
-  if (!helper) {
-    return [expression]
-  }
-  const returned = returnedExpressions(helper)
-  return returned.length > 0 && returned.every(alwaysAllocates) ? returned : [expression]
+  const helper =
+    expression?.type === 'CallExpression'
+      ? state.namedSelectors.get(identifierName(expression.callee))
+      : undefined
+  const returned = helper ? returnedExpressions(helper) : []
+  return returned.length > 0 && returned.every((entry) => allocates(entry, true))
+    ? returned
+    : [expression]
 }
 
 function createRuleState() {
@@ -222,32 +205,27 @@ function createRuleState() {
 }
 
 function recordImports(node, state) {
-  if (node.source?.value === 'zustand/react/shallow') {
-    for (const specifier of node.specifiers) {
-      const localName = importedLocalName(specifier, 'useShallow')
-      if (localName) {
-        state.shallowHooks.add(localName)
-      }
-    }
-  }
-  for (const specifier of node.specifiers) {
-    const localName = importedLocalName(specifier, 'useAppStore')
-    if (localName) {
-      state.appStoreHooks.add(localName)
-    }
-  }
-  if (!isLocalModuleSource(node.source?.value)) {
-    return
-  }
+  const source = node.source?.value
   for (const specifier of node.specifiers) {
     if (specifier.type !== 'ImportSpecifier') {
       continue
     }
-    if (isStoreHookName(identifierName(specifier.imported))) {
-      const localName = identifierName(specifier.local)
-      if (localName) {
-        state.appStoreHooks.add(localName)
-      }
+    const imported = identifierName(specifier.imported)
+    const localName = identifierName(specifier.local)
+    if (!imported || !localName) {
+      continue
+    }
+    if (source === 'zustand/react/shallow' && imported === 'useShallow') {
+      state.shallowHooks.add(localName)
+    }
+    // useAppStore is the app store wherever it is re-exported from; sibling
+    // stores are trusted by naming convention only when they come from this codebase.
+    if (
+      STORE_HOOK_NAME.test(imported) &&
+      !NON_STORE_HOOKS.has(imported) &&
+      (imported === 'useAppStore' || isLocalModuleSource(source))
+    ) {
+      state.appStoreHooks.add(localName)
     }
   }
 }
@@ -307,9 +285,7 @@ function deferredSelectorRule(inspect) {
         )
         const report = inspect({
           selector: resolveSelector(argument, state),
-          argument,
           shallow,
-          node,
           state
         })
         if (report) {

config/scripts/app-store-performance-plugin.test.mjs

@@ -68,6 +68,20 @@ describe('app store performance Oxlint plugin', () => {
     ])
   })
 
+  it('does not let a component-local helper resolve a same-named imported selector', () => {
+    const diagnostics = lintSource(`
+      import { useAppStore } from '@/store'
+      import { selectRows } from './selectors'
+      const Other = () => {
+        const selectRows = (state) => state.rows.map((row) => row.id)
+        return selectRows
+      }
+      const Imported = () => useAppStore(selectRows)
+    `)
+
+    expect(diagnostics).toEqual([])
+  })
+
   it('covers sibling store hooks but not useSyncExternalStore', () => {
     const diagnostics = lintSource(`
       import { usePluginPanelsStore } from '@/store/plugin-panels'

src/renderer/src/store/index.ts

@@ -1,4 +1,4 @@
-import { create } from 'zustand'
+import { create, type StateCreator } from 'zustand'
 import type { AppState } from './types'
 import { createRepoSlice } from './slices/repos'
 import { createSparsePresetsSlice } from './slices/sparse-presets'
@@ -60,8 +60,16 @@ import {
 } from '@/lib/renderer-memory-profile'
 import { estimateStateCollectionKB } from '@/lib/state-collection-byte-estimate'
 
+// Why dev-only: nothing in the app arms the churn probe, so a shipped build would
+// pay its wrapper frame on every write for a diagnostic it can never read. The
+// cascade probe stays unconditional because crash telemetry arms it in the field.
+const withDevelopmentStoreProbes = (createState: StateCreator<AppState, [], []>) =>
+  import.meta.env.DEV || e2eConfig.exposeStore
+    ? withStoreIdentityChurnProbe(createState)
+    : createState
+
 export const useAppStore = create<AppState>()(
-  withStoreIdentityChurnProbe(
+  withDevelopmentStoreProbes(
     withReactCommitCascadeWriteProbe((...a) => {
       // Why: the inner api is only reachable here, before create() copies subscribe onto the hook.
       installStoreListenerCensus(a[2])

src/renderer/src/store/store-identity-churn-probe.test.ts

@@ -1,5 +1,6 @@
-import { beforeEach, describe, expect, it } from 'vitest'
-import { create } from 'zustand'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { create, type StoreApi } from 'zustand'
+import { withReactCommitCascadeWriteProbe } from './react-commit-cascade-write-probe'
 import {
   armStoreIdentityChurnProbe,
   disarmStoreIdentityChurnProbe,
@@ -97,20 +98,86 @@ describe('store identity churn probe', () => {
     expect(row.sites[0].site).toContain('store-identity-churn-probe.test')
   })
 
-  it('runs a functional update exactly once', () => {
-    // The probe resolves the updater itself before handing the object to zustand;
-    // calling it twice would double any work a slice does inside its updater.
+  it('leaves a functional updater to zustand: called once, with the live state', () => {
     const store = createProbeStore()
-    let calls = 0
+    const seen: unknown[] = []
     armStoreIdentityChurnProbe()
 
     store.setState((state) => {
-      calls += 1
+      seen.push(state)
       return { counter: state.counter + 1 }
     })
+    store.setState((state) => {
+      seen.push(state)
+      return { rows: [{ ...state.rows[0] }] }
+    })
 
-    expect(calls).toBe(1)
+    expect(seen).toHaveLength(2)
+    expect(seen[1]).toMatchObject({ counter: 1 })
     expect(store.getState().counter).toBe(1)
+    expect(churnFor('rows')).toBe(1)
+  })
+
+  it('ignores a write zustand itself drops as identical', () => {
+    const store = createProbeStore()
+    armStoreIdentityChurnProbe()
+
+    store.setState((state) => state)
+
+    expect(readStoreIdentityChurnReport()).toEqual([])
+  })
+
+  it('passes disarmed writes straight through without reading state', () => {
+    const innerSet = vi.fn()
+    const innerGet = vi.fn(() => ({ counter: 0 }))
+    const api = { setState: innerSet, getState: innerGet } as unknown as StoreApi<{
+      counter: number
+    }>
+    const creator = withStoreIdentityChurnProbe<{ counter: number }>(() => ({ counter: 0 }))
+    creator(innerSet, innerGet, api)
+    const updater = (state: { counter: number }) => ({ counter: state.counter + 1 })
+
+    api.setState(updater, true)
+
+    // The disarmed path forwards the exact arguments and never calls get().
+    expect(innerSet).toHaveBeenCalledTimes(1)
+    expect(innerSet.mock.calls[0]).toEqual([updater, true])
+    expect(innerGet).not.toHaveBeenCalled()
+  })
+
+  it('composes with the cascade probe without dropping or doubling a write', () => {
+    // Mirrors store/index.ts: churn probe outermost, cascade probe inside it.
+    const store = create<ProbeState>()(
+      withStoreIdentityChurnProbe(
+        withReactCommitCascadeWriteProbe((set) => ({
+          rows: [{ id: 'a', label: 'A' }],
+          entries: { a: { status: 'idle' } },
+          counter: 0,
+          refresh: (rows) => set({ rows }),
+          touch: (id, status) =>
+            set((state) => ({ entries: { ...state.entries, [id]: { status } } })),
+          bump: () => set((state) => ({ counter: state.counter + 1 }))
+        }))
+      )
+    )
+    let updaterCalls = 0
+    armStoreIdentityChurnProbe({ captureSites: true })
+
+    store.getState().bump()
+    store.setState((state) => {
+      updaterCalls += 1
+      return { counter: state.counter + 10 }
+    })
+    store.getState().refresh([{ id: 'a', label: 'A' }])
+    store.setState({ ...store.getState(), counter: 100 }, true)
+
+    expect(updaterCalls).toBe(1)
+    expect(store.getState().counter).toBe(100)
+    expect(store.getState().rows).toEqual([{ id: 'a', label: 'A' }])
+    // The named site is this test, not the sibling probe's wrapper frame.
+    const [row] = readStoreIdentityChurnReport()
+    expect(row).toMatchObject({ field: 'rows', churnedWrites: 1 })
+    expect(row.sites[0].site).toContain('store-identity-churn-probe.test')
   })
 
   it('still sees churn on a replace write', () => {

src/renderer/src/store/store-identity-churn-probe.ts

@@ -9,8 +9,13 @@
  * no data change to show for it.
  *
  * Cost when disarmed: one boolean field load per write, matching
- * react-commit-cascade-write-probe. Comparison work only happens while armed,
- * so this never sits on the keystroke path in a shipped build.
+ * react-commit-cascade-write-probe. Comparison work only happens while armed.
+ * Nothing in the app arms it, so store/index.ts installs it only in dev and
+ * store-exposing builds; a shipped build never runs the wrapper at all.
+ *
+ * The wrapper never resolves a functional updater itself: zustand keeps sole
+ * ownership of when and with what argument an updater runs, so the probe cannot
+ * double-invoke it or hand it a stale state.
  */
 import type { StateCreator } from 'zustand'
 
@@ -87,18 +92,16 @@ export function armStoreIdentityChurnProbe(options?: { captureSites?: boolean })
 }
 
 // Why the first non-probe, non-zustand frame: the caller that built the partial is
-// the code to fix; the frames above it are the shared write plumbing.
+// the code to fix; the frames above it are the shared write plumbing. Every store
+// write middleware is named *-probe.ts, so a sibling wrapper's frame is skipped too.
 const SOURCE_FRAME = /:\d+:\d+\)?$/
+const PROBE_FRAME = /-probe\.[cm]?[jt]s\b/
 
 function callingSite(): string {
   const stack = new Error('store identity churn site').stack?.split('\n') ?? []
   for (const line of stack.slice(2)) {
     const frame = line.trim()
-    if (
-      SOURCE_FRAME.test(frame) &&
-      !frame.includes('store-identity-churn-probe.ts') &&
-      !frame.includes('node_modules')
-    ) {
+    if (SOURCE_FRAME.test(frame) && !PROBE_FRAME.test(frame) && !frame.includes('node_modules')) {
       return frame
     }
   }
@@ -134,10 +137,10 @@ function recordSite(field: string): void {
 }
 
 /**
- * `fields` is the write's own keys, not the whole state: `set(partial)` merges, so
- * no field outside the partial can have changed. Scanning the full post-write state
- * would make the armed cost scale with the store's field count instead of the
- * write's size.
+ * `fields` is the write's own keys when they are knowable: `set(partial)` merges,
+ * so no field outside the partial can have changed. A functional updater or a
+ * replace write falls back to every field; the extra cost there is one Object.is
+ * per untouched field, since the deep compare only runs on replaced references.
  */
 function recordWrite(
   previous: Record<string, unknown>,
@@ -180,21 +183,18 @@ export function withStoreIdentityChurnProbe<TState>(
         return
       }
       const previous = get() as Record<string, unknown>
-      // Why resolve the updater here: zustand would otherwise compute the partial
-      // internally and the probe could only recover the changed fields by scanning
-      // the whole state. Same function, same argument, called once.
-      const resolved =
-        typeof partial === 'function'
-          ? (partial as (state: Record<string, unknown>) => unknown)(previous)
-          : partial
-      ;(set as (nextPartial: unknown, nextReplace?: unknown) => void)(resolved, replace)
+      // Why the write is passed through untouched: zustand owns when and how an
+      // updater runs. The probe only compares the states on either side of it.
+      ;(set as (nextPartial: unknown, nextReplace?: unknown) => void)(partial, replace)
       try {
         const next = get() as Record<string, unknown>
-        // A replace write drops absent fields, so every field is in play.
+        if (next === previous) {
+          return
+        }
         const fields =
-          replace === true || resolved === null || typeof resolved !== 'object'
-            ? Object.keys(next)
-            : Object.keys(resolved as Record<string, unknown>)
+          replace !== true && partial !== null && typeof partial === 'object'
+            ? Object.keys(partial)
+            : Object.keys(next)
         recordWrite(previous, next, fields)
       } catch {
         // A diagnostic on the app's universal write path must never break writes.