From b6df712f444063c2759ef76aeeb0762da74a0b58 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 19 Jul 2026 19:46:09 -0500 Subject: [PATCH] refactor(desktop): DRY the computed-dedup into stableArray + freeze One shared `stableArray(prev, next)` helper replaces the duplicated element-equal/keep-prev logic in both stores, and freezes the shared ref so a future in-place mutation fails loud instead of silently corrupting the cache. Computed return type is now `readonly string[]` (it always was, immutably). --- apps/desktop/src/app/chat/sidebar/index.tsx | 2 +- apps/desktop/src/lib/stable-array.ts | 7 +++ apps/desktop/src/store/composer-status.ts | 33 ++++------- apps/desktop/src/store/session-states.ts | 62 ++++++++------------- 4 files changed, 41 insertions(+), 63 deletions(-) create mode 100644 apps/desktop/src/lib/stable-array.ts diff --git a/apps/desktop/src/app/chat/sidebar/index.tsx b/apps/desktop/src/app/chat/sidebar/index.tsx index 8f7e1420a57d..8add602e8c7f 100644 --- a/apps/desktop/src/app/chat/sidebar/index.tsx +++ b/apps/desktop/src/app/chat/sidebar/index.tsx @@ -716,7 +716,7 @@ export function ChatSidebar({ // session settles (its turn finished) or the window refocuses (an external // terminal may have changed things) — only while a project is entered, and // only the cheap per-repo `git worktree list`, never the heavy tree scan. - const prevWorkingIdsRef = useRef(workingSessionIds) + const prevWorkingIdsRef = useRef(workingSessionIds) useEffect(() => { const prev = prevWorkingIdsRef.current diff --git a/apps/desktop/src/lib/stable-array.ts b/apps/desktop/src/lib/stable-array.ts new file mode 100644 index 000000000000..b1e415ededfd --- /dev/null +++ b/apps/desktop/src/lib/stable-array.ts @@ -0,0 +1,7 @@ +/** Keep `prev`'s reference when it's element-equal to `next`, so a nanostores + * `computed` (notifies on `!==`) skips the emit when its projected list didn't + * actually change — e.g. status-id sets recomputed on every stream delta. + * `next` is frozen: the ref is shared across ticks, so an in-place mutation + * would corrupt the cache — fail loud instead. */ +export const stableArray = (prev: readonly T[], next: T[]): readonly T[] => + prev.length === next.length && prev.every((v, i) => v === next[i]) ? prev : Object.freeze(next) diff --git a/apps/desktop/src/store/composer-status.ts b/apps/desktop/src/store/composer-status.ts index dafc934d5fee..4ba910f47e34 100644 --- a/apps/desktop/src/store/composer-status.ts +++ b/apps/desktop/src/store/composer-status.ts @@ -1,6 +1,7 @@ import { atom, computed } from 'nanostores' import { translateNow } from '@/i18n' +import { stableArray } from '@/lib/stable-array' import type { TodoItem, TodoStatus } from '@/lib/todos' import { $gateway } from './gateway' @@ -44,36 +45,24 @@ export const $backgroundStatusBySession = atom { const ids = new Set() for (const [runtimeId, items] of Object.entries(bg)) { - if (!items.some(i => i.state === 'running')) { - continue - } + if (items.some(i => i.state === 'running')) { + const storedId = states[runtimeId]?.storedSessionId - const storedId = states[runtimeId]?.storedSessionId - - if (storedId) { - ids.add(storedId) + if (storedId) { + ids.add(storedId) + } } } - const next = [...ids] - - if (next.length === backgroundRunningCache.length && next.every((id, i) => id === backgroundRunningCache[i])) { - return backgroundRunningCache - } - - backgroundRunningCache = next - - return next + return (backgroundRunningIds = stableArray(backgroundRunningIds, [...ids])) }) // Rows the user X-ed away. The registry keeps finished processes around for a diff --git a/apps/desktop/src/store/session-states.ts b/apps/desktop/src/store/session-states.ts index e8bb58ff5ca7..b2e9da9c0ecc 100644 --- a/apps/desktop/src/store/session-states.ts +++ b/apps/desktop/src/store/session-states.ts @@ -27,6 +27,7 @@ import { noteActiveTreeGroup, revealTreePane } from '@/components/pane-shell/tree/store' +import { stableArray } from '@/lib/stable-array' import { readJson, writeJson } from '@/lib/storage' import { $activeGatewayProfile, normalizeProfileKey } from './profile' @@ -198,49 +199,30 @@ export function clearAllSessionStates() { $sessionStates.set({}) } -// Derived per-session status sets. `$sessionStates` already holds `busy` and -// `needsInput` for every runtime session (written by updateSessionState); these -// are pure projections of it, not independently maintained atoms. This keeps the -// data flow one-directional: gateway event → cache → $sessionStates → computed -// views, eliminating the "projection atom out of sync with cache" bug class. +// Derived per-session status sets — pure projections of `$sessionStates` (which +// holds `busy`/`needsInput` per runtime), keeping the data flow one-directional: +// gateway event → cache → $sessionStates → computed views. // -// CRITICAL for streaming perf: `$sessionStates` is republished on EVERY message -// delta (tens of times/sec during a turn), but the *membership* of these ID sets -// only changes on busy/needsInput edges. `computed` notifies on `!==`, so -// returning a fresh array each time would re-render the whole sidebar (and every -// row) per token. Return the PREVIOUS array reference when the contents match so -// nanostores skips the notify unless the set actually changed. -function stableIds(previous: string[], next: string[]): string[] { - if (previous.length === next.length && previous.every((id, i) => id === next[i])) { - return previous - } +// Perf: `$sessionStates` is republished on EVERY message delta (tens/sec during +// a turn), but these sets only change on busy/needsInput edges. `stableArray` +// keeps the prior reference when membership is unchanged so `computed` skips the +// emit — otherwise the whole sidebar + every row re-renders per token. +const storedIds = (states: Record, pred: (s: ClientSessionState) => boolean) => + Object.values(states) + .filter(s => pred(s) && s.storedSessionId) + .map(s => s.storedSessionId!) - return next -} +let workingIds: readonly string[] = [] +export const $workingSessionIds = computed( + $sessionStates, + states => (workingIds = stableArray(workingIds, storedIds(states, s => s.busy))) +) -let workingIdsCache: string[] = [] -export const $workingSessionIds = computed($sessionStates, states => { - workingIdsCache = stableIds( - workingIdsCache, - Object.values(states) - .filter(s => s.busy && s.storedSessionId) - .map(s => s.storedSessionId!) - ) - - return workingIdsCache -}) - -let attentionIdsCache: string[] = [] -export const $attentionSessionIds = computed($sessionStates, states => { - attentionIdsCache = stableIds( - attentionIdsCache, - Object.values(states) - .filter(s => s.needsInput && s.storedSessionId) - .map(s => s.storedSessionId!) - ) - - return attentionIdsCache -}) +let attentionIds: readonly string[] = [] +export const $attentionSessionIds = computed( + $sessionStates, + states => (attentionIds = stableArray(attentionIds, storedIds(states, s => s.needsInput))) +) // --------------------------------------------------------------------------- // Session tiles.