From 6f50867aa1ee94fc7582340ce728655e173db144 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 2 Mar 2026 12:04:37 -0800 Subject: [PATCH] Improve store referential stability and fix reactive terminal state - Deep clone DEFAULT_THREAD_TERMINAL_STATE in createDefaultThreadTerminalState - Remove imperative getTerminalState from terminal store; export selectThreadTerminalState for reactive selectors instead - Fix Sidebar terminal activity indicator not updating (was using non-reactive getter) - Add no-op bailouts to app store transitions (updateThread, markThreadVisited, markThreadUnread, setError, setThreadBranch, setProjectExpanded, setRuntimeMode) to prevent unnecessary re-renders - Use createDefaultThreadTerminalState in clearTerminalState to avoid storing frozen reference Co-Authored-By: Claude Opus 4.6 --- apps/web/src/components/ChatView.tsx | 6 +- apps/web/src/components/Sidebar.tsx | 7 +- apps/web/src/store.ts | 103 +++++++++++++----------- apps/web/src/terminalStateStore.test.ts | 16 ++-- apps/web/src/terminalStateStore.ts | 15 ++-- 5 files changed, 79 insertions(+), 68 deletions(-) diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 6e146b17968a37cc6cadfaad44fd50c201cf13c5..8d87198559bb19e3637df7013ba2f6dfbb31fd34 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -124,7 +124,7 @@ import { useComposerDraftStore, useComposerThreadDraft, } from "../composerDraftStore"; -import { useTerminalStateStore } from "../terminalStateStore"; +import { selectThreadTerminalState, useTerminalStateStore } from "../terminalStateStore"; import { clamp } from "effect/Number"; function formatMessageMeta(createdAt: string, duration: string | null): string { @@ -515,7 +515,9 @@ export default function ChatView({ threadId }: ChatViewProps) { setMessagesScrollElement(element); }, []); - const terminalState = useTerminalStateStore((state) => state.getTerminalState(threadId)); + const terminalState = useTerminalStateStore((state) => + selectThreadTerminalState(state.terminalStateByThreadId, threadId), + ); const storeSetTerminalOpen = useTerminalStateStore((s) => s.setTerminalOpen); const storeSetTerminalHeight = useTerminalStateStore((s) => s.setTerminalHeight); const storeSplitTerminal = useTerminalStateStore((s) => s.splitTerminal); diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 9ec8b3261d9c4ca505120aaee79c6df4d5dcc002..d5921da8a9b8145272f1db209aadadc913445625 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -21,7 +21,7 @@ import { gitRemoveWorktreeMutationOptions, gitStatusQueryOptions } from "../lib/ import { serverConfigQueryOptions } from "../lib/serverReactQuery"; import { readNativeApi } from "../nativeApi"; import { type DraftThreadEnvMode, useComposerDraftStore } from "../composerDraftStore"; -import { useTerminalStateStore } from "../terminalStateStore"; +import { selectThreadTerminalState, useTerminalStateStore } from "../terminalStateStore"; import { toastManager } from "./ui/toast"; import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "./ui/collapsible"; import { Tooltip, TooltipPopup, TooltipTrigger } from "./ui/tooltip"; @@ -240,7 +240,7 @@ export default function Sidebar() { (store) => store.getDraftThreadByProjectId, ); const getDraftThread = useComposerDraftStore((store) => store.getDraftThread); - const getTerminalState = useTerminalStateStore((state) => state.getTerminalState); + const terminalStateByThreadId = useTerminalStateStore((state) => state.terminalStateByThreadId); const setProjectDraftThreadId = useComposerDraftStore((store) => store.setProjectDraftThreadId); const setDraftThreadContext = useComposerDraftStore((store) => store.setDraftThreadContext); const clearProjectDraftThreadId = useComposerDraftStore( @@ -829,7 +829,8 @@ export default function Sidebar() { ); const prStatus = prStatusIndicator(prByThreadId.get(thread.id) ?? null); const terminalStatus = terminalStatusFromRunningIds( - getTerminalState(thread.id).runningTerminalIds, + selectThreadTerminalState(terminalStateByThreadId, thread.id) + .runningTerminalIds, ); return ( diff --git a/apps/web/src/store.ts b/apps/web/src/store.ts index 3152db2b88c90f473da89229f8bc78b40fba2a78..e761bc30d472a89e01962874bc21b49c4cce95f6 100644 --- a/apps/web/src/store.ts +++ b/apps/web/src/store.ts @@ -101,7 +101,14 @@ function updateThread( threadId: ThreadId, updater: (t: Thread) => Thread, ): Thread[] { - return threads.map((t) => (t.id === threadId ? updater(t) : t)); + let changed = false; + const next = threads.map((t) => { + if (t.id !== threadId) return t; + const updated = updater(t); + if (updated !== t) changed = true; + return updated; + }); + return changed ? next : threads; } function mapProjectsFromReadModel( @@ -277,34 +284,30 @@ export function markThreadVisited( ): AppState { const at = visitedAt ?? new Date().toISOString(); const visitedAtMs = Date.parse(at); - return { - ...state, - threads: updateThread(state.threads, threadId, (thread) => { - const previousVisitedAtMs = thread.lastVisitedAt ? Date.parse(thread.lastVisitedAt) : NaN; - if ( - Number.isFinite(previousVisitedAtMs) && - Number.isFinite(visitedAtMs) && - previousVisitedAtMs >= visitedAtMs - ) { - return thread; - } - return { ...thread, lastVisitedAt: at }; - }), - }; + const threads = updateThread(state.threads, threadId, (thread) => { + const previousVisitedAtMs = thread.lastVisitedAt ? Date.parse(thread.lastVisitedAt) : NaN; + if ( + Number.isFinite(previousVisitedAtMs) && + Number.isFinite(visitedAtMs) && + previousVisitedAtMs >= visitedAtMs + ) { + return thread; + } + return { ...thread, lastVisitedAt: at }; + }); + return threads === state.threads ? state : { ...state, threads }; } export function markThreadUnread(state: AppState, threadId: ThreadId): AppState { - return { - ...state, - threads: updateThread(state.threads, threadId, (thread) => { - if (!thread.latestTurn?.completedAt) return thread; - const latestTurnCompletedAtMs = Date.parse(thread.latestTurn.completedAt); - if (Number.isNaN(latestTurnCompletedAtMs)) return thread; - const unreadVisitedAt = new Date(latestTurnCompletedAtMs - 1).toISOString(); - if (thread.lastVisitedAt === unreadVisitedAt) return thread; - return { ...thread, lastVisitedAt: unreadVisitedAt }; - }), - }; + const threads = updateThread(state.threads, threadId, (thread) => { + if (!thread.latestTurn?.completedAt) return thread; + const latestTurnCompletedAtMs = Date.parse(thread.latestTurn.completedAt); + if (Number.isNaN(latestTurnCompletedAtMs)) return thread; + const unreadVisitedAt = new Date(latestTurnCompletedAtMs - 1).toISOString(); + if (thread.lastVisitedAt === unreadVisitedAt) return thread; + return { ...thread, lastVisitedAt: unreadVisitedAt }; + }); + return threads === state.threads ? state : { ...state, threads }; } export function toggleProject(state: AppState, projectId: Project["id"]): AppState { @@ -321,19 +324,21 @@ export function setProjectExpanded( projectId: Project["id"], expanded: boolean, ): AppState { - return { - ...state, - projects: state.projects.map((p) => - p.id === projectId ? { ...p, expanded } : p, - ), - }; + let changed = false; + const projects = state.projects.map((p) => { + if (p.id !== projectId || p.expanded === expanded) return p; + changed = true; + return { ...p, expanded }; + }); + return changed ? { ...state, projects } : state; } export function setError(state: AppState, threadId: ThreadId, error: string | null): AppState { - return { - ...state, - threads: updateThread(state.threads, threadId, (t) => ({ ...t, error })), - }; + const threads = updateThread(state.threads, threadId, (t) => { + if (t.error === error) return t; + return { ...t, error }; + }); + return threads === state.threads ? state : { ...state, threads }; } export function setThreadBranch( @@ -342,21 +347,21 @@ export function setThreadBranch( branch: string | null, worktreePath: string | null, ): AppState { - return { - ...state, - threads: updateThread(state.threads, threadId, (t) => { - const cwdChanged = t.worktreePath !== worktreePath; - return { - ...t, - branch, - worktreePath, - ...(cwdChanged ? { session: null } : {}), - }; - }), - }; + const threads = updateThread(state.threads, threadId, (t) => { + if (t.branch === branch && t.worktreePath === worktreePath) return t; + const cwdChanged = t.worktreePath !== worktreePath; + return { + ...t, + branch, + worktreePath, + ...(cwdChanged ? { session: null } : {}), + }; + }); + return threads === state.threads ? state : { ...state, threads }; } export function setRuntimeMode(state: AppState, mode: RuntimeMode): AppState { + if (state.runtimeMode === mode) return state; return { ...state, runtimeMode: mode }; } @@ -377,7 +382,7 @@ interface AppStore extends AppState { setRuntimeMode: (mode: RuntimeMode) => void; } -export const useStore = create((set, _get) => ({ +export const useStore = create((set) => ({ ...readPersistedState(), syncServerReadModel: (readModel) => set((state) => syncServerReadModel(state, readModel)), diff --git a/apps/web/src/terminalStateStore.test.ts b/apps/web/src/terminalStateStore.test.ts index 314b0e23c3e03ea12089067d9e92bc33bb5b6889..f91a40139f3c10996e96b828196598d8c34495e6 100644 --- a/apps/web/src/terminalStateStore.test.ts +++ b/apps/web/src/terminalStateStore.test.ts @@ -1,7 +1,7 @@ import { ThreadId } from "@t3tools/contracts"; import { beforeEach, describe, expect, it } from "vitest"; -import { useTerminalStateStore } from "./terminalStateStore"; +import { selectThreadTerminalState, useTerminalStateStore } from "./terminalStateStore"; const THREAD_ID = ThreadId.makeUnsafe("thread-1"); @@ -14,7 +14,7 @@ describe("terminalStateStore actions", () => { }); it("returns a closed default terminal state for unknown threads", () => { - const terminalState = useTerminalStateStore.getState().getTerminalState(THREAD_ID); + const terminalState = selectThreadTerminalState(useTerminalStateStore.getState().terminalStateByThreadId, THREAD_ID); expect(terminalState).toEqual({ terminalOpen: false, terminalHeight: 280, @@ -31,7 +31,7 @@ describe("terminalStateStore actions", () => { store.setTerminalOpen(THREAD_ID, true); store.splitTerminal(THREAD_ID, "terminal-2"); - const terminalState = useTerminalStateStore.getState().getTerminalState(THREAD_ID); + const terminalState = selectThreadTerminalState(useTerminalStateStore.getState().terminalStateByThreadId, THREAD_ID); expect(terminalState.terminalOpen).toBe(true); expect(terminalState.terminalIds).toEqual(["default", "terminal-2"]); expect(terminalState.activeTerminalId).toBe("terminal-2"); @@ -43,7 +43,7 @@ describe("terminalStateStore actions", () => { it("creates new terminals in a separate group", () => { useTerminalStateStore.getState().newTerminal(THREAD_ID, "terminal-2"); - const terminalState = useTerminalStateStore.getState().getTerminalState(THREAD_ID); + const terminalState = selectThreadTerminalState(useTerminalStateStore.getState().terminalStateByThreadId, THREAD_ID); expect(terminalState.terminalIds).toEqual(["default", "terminal-2"]); expect(terminalState.activeTerminalId).toBe("terminal-2"); expect(terminalState.activeTerminalGroupId).toBe("group-terminal-2"); @@ -57,12 +57,12 @@ describe("terminalStateStore actions", () => { const store = useTerminalStateStore.getState(); store.splitTerminal(THREAD_ID, "terminal-2"); store.setTerminalActivity(THREAD_ID, "terminal-2", true); - expect(useTerminalStateStore.getState().getTerminalState(THREAD_ID).runningTerminalIds).toEqual([ + expect(selectThreadTerminalState(useTerminalStateStore.getState().terminalStateByThreadId, THREAD_ID).runningTerminalIds).toEqual([ "terminal-2", ]); store.setTerminalActivity(THREAD_ID, "terminal-2", false); - expect(useTerminalStateStore.getState().getTerminalState(THREAD_ID).runningTerminalIds).toEqual([]); + expect(selectThreadTerminalState(useTerminalStateStore.getState().terminalStateByThreadId, THREAD_ID).runningTerminalIds).toEqual([]); }); it("resets to default and clears persisted entry when closing the last terminal", () => { @@ -70,7 +70,7 @@ describe("terminalStateStore actions", () => { store.closeTerminal(THREAD_ID, "default"); expect(useTerminalStateStore.getState().terminalStateByThreadId[THREAD_ID]).toBeUndefined(); - expect(useTerminalStateStore.getState().getTerminalState(THREAD_ID).terminalIds).toEqual(["default"]); + expect(selectThreadTerminalState(useTerminalStateStore.getState().terminalStateByThreadId, THREAD_ID).terminalIds).toEqual(["default"]); }); it("keeps a valid active terminal after closing an active split terminal", () => { @@ -79,7 +79,7 @@ describe("terminalStateStore actions", () => { store.splitTerminal(THREAD_ID, "terminal-3"); store.closeTerminal(THREAD_ID, "terminal-3"); - const terminalState = useTerminalStateStore.getState().getTerminalState(THREAD_ID); + const terminalState = selectThreadTerminalState(useTerminalStateStore.getState().terminalStateByThreadId, THREAD_ID); expect(terminalState.activeTerminalId).toBe("terminal-2"); expect(terminalState.terminalIds).toEqual(["default", "terminal-2"]); expect(terminalState.terminalGroups).toEqual([ diff --git a/apps/web/src/terminalStateStore.ts b/apps/web/src/terminalStateStore.ts index 3f3621803bb201ce71d556725c8c0abfbb48f2ed..20f66910a5bd8f66f176a1880e1df4ee64f837fd 100644 --- a/apps/web/src/terminalStateStore.ts +++ b/apps/web/src/terminalStateStore.ts @@ -163,7 +163,12 @@ const DEFAULT_THREAD_TERMINAL_STATE: ThreadTerminalState = Object.freeze({ }); function createDefaultThreadTerminalState(): ThreadTerminalState { - return { ...DEFAULT_THREAD_TERMINAL_STATE }; + return { + ...DEFAULT_THREAD_TERMINAL_STATE, + terminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.terminalIds], + runningTerminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.runningTerminalIds], + terminalGroups: copyTerminalGroups(DEFAULT_THREAD_TERMINAL_STATE.terminalGroups), + }; } function getDefaultThreadTerminalState(): ThreadTerminalState { @@ -403,7 +408,7 @@ function setThreadTerminalActivity( return { ...normalized, runningTerminalIds: [...runningTerminalIds] }; } -function selectThreadTerminalState( +export function selectThreadTerminalState( terminalStateByThreadId: Record, threadId: ThreadId, ): ThreadTerminalState { @@ -444,7 +449,6 @@ function updateTerminalStateByThreadId( interface TerminalStateStoreState { terminalStateByThreadId: Record; - getTerminalState: (threadId: ThreadId) => ThreadTerminalState; setTerminalOpen: (threadId: ThreadId, open: boolean) => void; setTerminalHeight: (threadId: ThreadId, height: number) => void; splitTerminal: (threadId: ThreadId, terminalId: string) => void; @@ -461,7 +465,7 @@ interface TerminalStateStoreState { export const useTerminalStateStore = create()( persist( - (set, get) => { + (set) => { const updateTerminal = ( threadId: ThreadId, updater: (state: ThreadTerminalState) => ThreadTerminalState, @@ -483,7 +487,6 @@ export const useTerminalStateStore = create()( return { terminalStateByThreadId: {}, - getTerminalState: (threadId) => selectThreadTerminalState(get().terminalStateByThreadId, threadId), setTerminalOpen: (threadId, open) => updateTerminal(threadId, (state) => setThreadTerminalOpen(state, open)), setTerminalHeight: (threadId, height) => @@ -501,7 +504,7 @@ export const useTerminalStateStore = create()( setThreadTerminalActivity(state, terminalId, hasRunningSubprocess), ), clearTerminalState: (threadId) => - updateTerminal(threadId, () => getDefaultThreadTerminalState()), + updateTerminal(threadId, () => createDefaultThreadTerminalState()), }; }, { -- 2.54.0