13 KiB
13 KiB
Goose2 Zustand Refactor Progress
Current Status
- Phase 1 implementation complete.
- Phase 2 implementation complete.
- Phase 3 implementation complete.
Documents
- Added master review:
goose2-zustand-state-management-review.md - Added master plan:
goose2-zustand-state-management-improvement-plan.md - Added per-phase execution plans:
phase-1-selector-cleanup.mdphase-2-selector-read-layer.mdphase-3-session-side-effects.mdphase-4-store-boundaries.mdphase-5-session-workflow-failure-policy.mdphase-6-project-store.mdphase-7-persistence.mdphase-8-test-reset-coverage.mdphase-9-optional-immer.md
Decisions
- Keep simple numbered phases without
A/Bsub-phases. - Do not make test reset cleanup Phase 1.
- Clean up test reset patterns opportunistically when a refactor PR already touches those tests.
- Keep the dedicated test reset and coverage pass as Phase 8.
- Create per-phase execution files so each phase has scope, guardrails, validation, and success criteria.
- In Phase 1, keep existing hook/component public contracts unchanged. For example,
usePersonas()should keep returningpersonasandisLoadingeven though its current production consumer only uses command functions. - For Phase 3, use explicit
chatSessionOperations.tsfunctions for backend-aware chat-session workflows. UsepatchSessionfor local-only store updates. For title and project updates, call backend first and patch local state only after success.
Phase Status
| Phase | Name | Status | Notes |
|---|---|---|---|
| 1 | Remove whole-store subscriptions | Complete | Replaced broad subscriptions in usePersonas.ts, useChat.ts, Sidebar.tsx, and AppShell.tsx. |
| 2 | Introduce selector-first read layer | Complete | Added initial chatSessionStore, chatStore, agentStore, and projectStore selector helpers. Deferred derived values, store-boundary issues, and component-boundary issues to later phases. |
| 3 | Separate session side effects | Complete | Removed generic updateSession; local patches use patchSession, and title/project persistence uses explicit operations. |
| 4 | Split clearest store boundaries | Pending | Split agentStore UI state, then chatSessionStore UI state. |
| 5 | Decide session workflow failure policy | Pending | Behavior-policy follow-up after Phase 4 clarifies session-store boundaries. |
| 6 | Refactor project store | Pending | Make mutation policy explicit. |
| 7 | Standardize persistence | Pending | Do after boundaries are clearer. |
| 8 | Test reset and coverage | Pending | Dedicated cleanup after boundary changes stabilize. |
| 9 | Optional Immer | Pending | Skip unless update readability still justifies it. |
Known Current-Code Findings To Recheck Before Phase 1
- Broad subscription scan is clean after Phase 1:
rg "use[A-Za-z0-9]+Store\\(\\)" ui/goose2/src
useShallowis currently unused inui/goose2/src.- Phase 3 removed
chatSessionStore.updateSession; local-only session changes now usepatchSession. projectStorecurrently owns local cache, API orchestration, state, active selection, and reorder behavior.agentStorecurrently mixes catalog/provider state with persona editor UI state.
Next Step
- Start Phase 4 using
phase-4-store-boundaries.md.
Validation Log
- Phase 1 broad subscription scan: clean.
- Phase 1 TypeScript check:
cd ui/goose2 && pnpm exec tsc --noEmitpassed. - Phase 1 focused tests:
cd ui/goose2 && pnpm test -- useChat usePersonas Sidebarpassed. - Phase 2 Step 1 TypeScript check:
cd ui/goose2 && pnpm exec tsc --noEmitpassed. - Phase 2 Step 2 TypeScript check:
cd ui/goose2 && pnpm exec tsc --noEmitpassed. - Phase 2 Step 3A TypeScript check:
cd ui/goose2 && pnpm exec tsc --noEmitpassed. - Phase 2 Step 3B TypeScript check:
cd ui/goose2 && pnpm exec tsc --noEmitpassed. - Phase 2 Step 4A TypeScript check:
cd ui/goose2 && pnpm exec tsc --noEmitpassed. - Phase 2 Step 4B TypeScript check:
cd ui/goose2 && pnpm exec tsc --noEmitpassed. - Phase 2 focused tests:
cd ui/goose2 && pnpm test -- useChat usePersonas Sidebar useProviderInventorypassed. - Phase 2
useProviderInventory/useShallowassessment complete: no code change recommended. - Phase 2 final broad subscription scan:
rg "use[A-Za-z0-9]+Store\\(\\)" ui/goose2/srcfound no matches. - Phase 2 final
useShallowscan:rg "useShallow|zustand/shallow" ui/goose2/srcfound no matches, as expected. - Phase 3 TypeScript check:
cd ui/goose2 && pnpm exec tsc --noEmitpassed. - Phase 3 focused tests:
cd ui/goose2 && pnpm test -- chatSessionOperations chatSessionStorepassed; Vitest ran 109 files / 632 tests. - Phase 3 final generic
updateSessionscan: nochatSessionStore.updateSessionaction or call sites remain; only explicitupdateSessionTitle/updateSessionProjectoperation/API names remain.
Follow-Ups To Revisit Later
- Revisit whether
usePersonas()should remain both a read hook and command hook. TodayAgentsViewalready readspersonasandpersonasLoadingthrough direct store selectors and only usesusePersonas()forcreatePersona,updatePersona,deletePersona, andrefreshFromDisk. Do not change that contract during Phase 1. - Revisit the sidebar runtime selector in Phase 2. Phase 1 selects the whole
sessionStateByIdrecord to avoid a complex derived selector, which is still narrower than the wholechatStore; Phase 2 should evaluate whether visible sidebar session items need a dedicated selector/helper. - Keep selector fallbacks stable. Phase 1 found that returning a fresh
[]from a selector fallback inuseChatcan trigger React snapshot churn; use module-level constants or derive fallback values outside the selector. - In Phase 8, keep component-test store mocks selector-aware. Phase 1 production code now calls bound hooks with selectors, so mocks need to support
useStore((state) => ...)as well as any remaining direct mock patterns. - Phase 1 removed dead code exposed by the selector cleanup:
retryLastMessage,buildSkillRetryOptions, and the unusedfindLastIndexhelper. - After Phase 3 creates explicit title/project workflow actions, revisit backend failure policy for rename and project assignment. Options include backend-first, optimistic with rollback, refresh-on-failure, or optimistic with user-visible error.
- After Phase 4 clarifies
chatSessionStoreboundaries, revisit archive/unarchive. They currently remain store actions with API calls and optimistic-without-rollback behavior: local visibility changes immediately, backend failures are logged, and local state is not restored.
Phase 2 Audit Notes
- Completed Step 1:
- Added
chatSessionSelectors.tswithselectSessions,selectActiveSessionId,selectHasHydratedSessions, andselectSessionsLoading. - Updated
AppShell.tsxandSidebar.tsxto use those simple field selectors.
- Added
- Completed Step 2:
- Added
chatSelectors.tswithselectMessagesBySessionandselectSessionStateById. - Updated
AppShell.tsxandSidebar.tsxto use those simple field selectors.
- Added
- Completed Step 3A:
- Added
agentSelectors.tswithselectPersonasandselectPersonasLoading. - Updated
usePersonas.ts,AgentsView.tsx, anduseChatSessionController.tsto use persona domain selectors. - Left persona editor UI state selectors inline so Phase 4 extraction remains visible.
- Added
- Completed Step 3B:
- Added
selectSelectedProvidertoagentSelectors.ts. - Updated
AppShell.tsxanduseProviderSelection.tsto use the selected-provider selector. - Left provider lists/loading/actions inline pending later
agentStoreboundary work.
- Added
- Completed Step 4A:
- Renamed the internal
ProjectStatetype to exportedProjectStorebecause the type includes state and actions. - Added
projectSelectors.tswithselectProjects. - Updated direct
projectsfield reads inAppShell.tsx,Sidebar.tsx,SkillsView.tsx,SessionHistoryView.tsx, anduseChatSessionController.ts. - Left project actions, loading, and derived project lookup reads inline pending Phase 6 project-store ownership work.
- Renamed the internal
- Completed Step 4B:
- Updated
SessionHistoryView.tsxto use existingselectSessionsandselectMessagesBySession. - A scan found no remaining exact inline reads for selector helpers already created.
- Left derived/id-dependent reads, UI-state reads, one-off loading reads, and action selectors inline.
- Updated
- Completed
useProviderInventory/useShallowassessment:useProviderInventoryreadsentriesandloadingwith separate narrow selectors.- Grouping these into an object selector with
useShallowwould not meaningfully reduce rerenders because consumers should still update when either value changes. - Kept the hook unchanged.
- Simple selector candidates:
- chat:
selectMessagesBySession,selectSessionStateById - chat sessions:
selectSessions,selectActiveSessionId,selectHasHydratedSessions,selectSessionsLoading - agents:
selectPersonas,selectPersonasLoading,selectSelectedProvider - projects:
selectProjects,selectFetchProjects,selectReorderProjects
- chat:
- Derived values should stay derived, not become stored attributes:
- active session from
sessions + activeSessionId - Home session from
sessions + homeSessionId - project lookup by id from
projects + projectId - sidebar grouped session rows from
sessions + messagesBySession + sessionStateById + project ids - Phase 4 should recheck active/Home session derivation and sidebar grouping after chat/session UI state moves.
- Phase 6 should recheck project lookup helpers after project-store ownership is clearer.
- active session from
- Duplicated or legacy state to revisit later:
chatStore.activeSessionIdandchatSessionStore.activeSessionIdduplicate active-session selection. Track for Phase 4 boundary review.agentStore.isLoadingmay be a legacy/general loading flag now that personas, agents, and providers have specific loading fields. Track for Phase 4 and test-reset cleanup in Phase 8.
- Imperative derived helper methods to revisit later:
chatStore.getActiveMessageschatSessionStore.getActiveSessionchatSessionStore.getArchivedSessionsprojectStore.getActiveProjectagentStore.getActiveAgentagentStore.getAgentsByPersonaagentStore.getBuiltinPersonasagentStore.getCustomPersonas- Keep these only where useful for
getState()/callback code; prefer selectors or pure helpers for React reads.
- Store-boundary signals to carry into later phases:
agentStoremixes personas, providers, active agent, selected provider, and persona editor UI. Track for Phase 4.chatStoremixes messages, runtime, drafts, queue, loading, scroll targets, and cleanup. Track for Phase 4 / Phase 9 reassessment.chatSessionStoremixes session records, hydration/loading, active selection, creation, archive, mutation, context panel state, and workspace UI. Track for Phase 3 and Phase 4.
- Phase 2 guardrail:
- Selectors should not hide real feature complexity. For
agentStore, use selectors only for stable repeated domain reads such as personas and selected provider. - Do not add selectors for persona editor UI state in Phase 2; keep that visible for Phase 4 UI-store extraction.
- If a feature still needs many store dependencies after selector cleanup, track it as a component/store boundary issue instead of adding more selectors.
- Selector files created in Phase 2 are part of the read layer, not proof that the current store boundaries are final.
- When Phase 4 or Phase 6 moves state across stores, update, move, or delete the corresponding Phase 2 selectors in the same change.
- Selectors should not hide real feature complexity. For
- Component-boundary signals to carry into later phases:
AppShellis orchestration-heavy: projects, sessions, Home setup, provider/model choice, keyboard shortcuts, dialogs, sidebar wiring, and chat activation. Track as a later component decomposition concern.Sidebarperforms visible-session filtering, project grouping, runtime badge derivation, search resolver creation, and expanded-project persistence. Track derived helpers during Phase 2 and possible component decomposition later.- Rename behavior is owned high in
AppShelland prop-drilled throughSidebar/sidebar sections toSidebarChatRow, and throughSessionHistoryViewtoSessionCard. Track for later component/action-controller cleanup. - Rename row components close edit mode immediately and expose void callbacks, so backend-first rename failures cannot currently show inline pending/error state. Track UX policy in Phase 5.
- Phase 2 implementation plan:
- Start with small chat-session selector helpers because they are simple field selectors.
- Refactor only
AppShell.tsxandSidebar.tsxfirst. - Do not use selectors to hide the larger Phase 3-5 architecture problems.