refactor(web): convert subagent-store to Zustand with direct named actions#31201
Conversation
…tions Closes LUM-1640 - Convert subagent-store.ts from useReducer/dispatch pattern to Zustand store with create() + createSelectors() and direct named actions: spawnSubagent, changeStatus, receiveEvent, loadDetail, setConversationId, reset - Remove dispatchSubagent from StreamHandlerContext — the last remaining Dispatch<Action> threading in the stream handler pipeline - Update subagent-handlers.ts to call useSubagentStore.getState() directly, matching the pattern from interaction-handlers.ts - Remove dispatchSubagent from use-stream-event-handler, use-send-message, use-conversation-history, use-conversation-loader, and test-helpers - Move subagent-status-helpers.ts from chat/lib/ to domains/subagents/status-helpers.ts - Rename SubagentMapState to SubagentState - Remove stale SubagentStatus re-export from store - Rewrite all tests to use Zustand store API (31 tests, all passing) Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| // Subagent | ||
| subagentEntries: SubagentEntry[]; | ||
| subagentState: SubagentMapState; | ||
| subagentState: SubagentState; |
There was a problem hiding this comment.
🚩 chat-page.tsx mock still uses Map() for byId but is pre-existing and outside the diff
At apps/web/src/domains/chat/chat-page.tsx:221, the skeleton/loading state for subagentState uses { byId: new Map(), orderedIds: [], entries: [] } cast through as unknown as. The SubagentState.byId type is Record<string, SubagentEntry> (a plain object), not a Map. Property access like state.byId[id] would return undefined on a Map (you'd need .get(id)). This works accidentally for an empty collection but is semantically wrong. This is pre-existing (predates this PR) and hidden behind the as unknown as cast, but the type rename from SubagentMapState to SubagentState makes this a good moment to clean it up.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 0b6261d. byId: new Map() with phantom entries property (hidden behind as unknown as cast) replaced with correct { byId: {}, orderedIds: [] } matching SubagentState.
| loadDetail: (params) => { | ||
| const { byId } = get(); | ||
| const existing = byId[params.subagentId]; | ||
| if (!existing) return; | ||
|
|
||
| set({ | ||
| byId: { | ||
| ...byId, | ||
| [params.subagentId]: { | ||
| ...existing, | ||
| status: params.status ?? existing.status, | ||
| objective: params.objective ?? existing.objective, | ||
| inputTokens: params.inputTokens ?? existing.inputTokens, | ||
| outputTokens: params.outputTokens ?? existing.outputTokens, | ||
| totalCost: params.totalCost ?? existing.totalCost, | ||
| events: params.events.length > 0 ? params.events : existing.events, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🚩 loadDetail action is defined but has no callers in the codebase
The loadDetail action (lines 285–303) was faithfully ported from the old SUBAGENT_DETAIL_LOADED reducer case, but grep finds zero call sites for either loadDetail or SUBAGENT_DETAIL_LOADED anywhere in the codebase. Per the AGENTS.md dead code removal rule: 'Proactively remove unused code during every change.' This appears to be dead code that could be removed. That said, it may be called from code outside this repo (e.g., a companion app) or be intended for imminent use, so flagging for investigation rather than as a rule violation.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed — zero callers in the codebase for either loadDetail or the old SUBAGENT_DETAIL_LOADED. Removed in 0b6261d (both the interface declaration and the implementation).
- Remove loadDetail action (zero callers in codebase) per dead code
removal rule
- Fix chat-page.tsx skeleton: byId was new Map() (wrong type) with a
phantom 'entries' property, hidden behind 'as unknown as' cast.
Now uses correct { byId: {}, orderedIds: [] } matching SubagentState
Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
There was a problem hiding this comment.
✦ APPROVE
Value: Completes the state management migration for subagent streaming — removes dispatchSubagent prop-drilling from 4 hooks and StreamHandlerContext, replacing it with direct Zustand store access. Developers working in the stream handler pipeline no longer need to thread dispatch callbacks through component boundaries.
What this does: Converts subagent-store.ts from a useReducer + Dispatch<SubagentAction> pattern to a Zustand store with direct named actions, wrapped with createSelectors. Removes dispatchSubagent: Dispatch<SubagentAction> from use-stream-event-handler, use-send-message, use-conversation-history, use-conversation-loader, and StreamHandlerContext. Bonus: moves subagent-status-helpers from domains/chat/lib/ → domains/subagents/ for better co-location.
Selector pattern — correct. createSelectors applied at the module level, wrapping useSubagentStoreBase. All downstream atomic .use.field() subscriptions are available. Follows auth-store.ts and organization-store.ts as canonical examples per CONVENTIONS.md.
Non-React access pattern — correct. Stream handlers (subagent-handlers.ts) and hook callbacks (use-conversation-history, use-send-message) call useSubagentStore.getState() directly. This is exactly right — getState() for non-React code, no subscription overhead, no hook rule violations.
Logic fidelity — clean. All 5 reducer cases (SUBAGENT_SPAWNED, SUBAGENT_STATUS_CHANGED, SUBAGENT_EVENT_RECEIVED, SUBAGENT_DETAIL_LOADED, SUBAGENT_CONVERSATION_ID_SET) are faithfully preserved as named actions. The text-delta coalescing logic and message_complete skip are intact.
get() usage inside actions — correct for reading current state before computing the next. Zustand's set() is synchronous and immediately commits, so subsequent get() calls in the same event loop tick always see the latest state. No stale-closure risk here.
Test migration — clean. beforeEach(() => getState().reset()) properly isolates module-level store state between test cases. Direct getState() calls eliminate the need for React rendering to test store logic — exactly the pattern the KB recommends.
_ctx in two handlers — handleSubagentStatusChanged and handleSubagentEvent now take _ctx: StreamHandlerContext but don't use it. Non-blocking: if the handler registration interface doesn't require a uniform signature, these could be simplified to (). Not worth holding the PR for.
loadDetail — no callers found. Devin flagged this correctly. SUBAGENT_DETAIL_LOADED was already caller-less in the pre-existing reducer (the action existed but nothing dispatched it in this PR's scope). Non-blocking — likely reserved for the detail fetch path that loads subagent timeline events from the API after the panel opens. Worth a follow-up pass to either wire it up or remove it.
Vellum Constitution — Distinct: the assistant's subagent pipeline now owns its state cleanly — no plumbing through component boundaries, no reducer dispatch threading. That's a more coherent abstraction.
Prompt / plan
Closes LUM-1640
Converts the last remaining reducer-style dispatch threading in the stream handler pipeline to a Zustand store with direct named actions, completing the state management migration started by the interaction store (#31142) and turn store (#31144) conversions.
Why needed
dispatchSubagent: Dispatch<SubagentAction>was threaded through 6 layers of hooks and theStreamHandlerContext— the exact prop-drilling anti-pattern Zustand eliminates. With the interaction and turn stores already converted,dispatchSubagentwas the sole remainingDispatch<Action>inStreamHandlerContext.What changed
Store conversion (
subagent-store.ts):subagentReducer+ action union types with Zustandcreate()+createSelectors()storespawnSubagent(),changeStatus(),receiveEvent(),loadDetail(),setConversationId(),reset()SubagentMapState→SubagentState(the "Map" was an implementation detail)SubagentStatusre-export — consumers import fromevent-types.tsdirectlyset()calls instead of reducer casesDispatch threading removal:
dispatchSubagentfromStreamHandlerContext(types.ts)UseStreamEventHandlerParamsand the hook's destructuring/depsUseSendMessageParams— replaced withuseSubagentStore.getState().reset()UseConversationHistoryParams— replaced with direct store callsUseConversationLoaderParams(pass-through to history)test-helpers.tsmockHandler refactor (
subagent-handlers.ts):useSubagentStore.getState().actionName()directlyinteraction-handlers.tsFile move (
subagent-status-helpers.ts→domains/subagents/status-helpers.ts):chat/lib/Tests (
subagent-store.test.ts):beforeEachresetBenefits
createSelectors+ direct named actions patternspawnSubagent()) are more discoverable than discriminated union dispatch ({ type: "SUBAGENT_SPAWNED", ... })Safety
References
set()APIAlternatives not taken
dispatch()wrapper: This was the pattern in the closed PR feat(chat): Convert conversation-list useReducer to Zustand store #31140 for conversation-list-store. Rejected because Zustand explicitly recommends direct actions over dispatch/reducer — the reducer pattern adds ceremony (action types, switch statements, discriminated unions) with no benefit when using Zustand.subagent-handlers.tstodomains/subagents/: Deferred to a separate PR — it's a larger move touching the stream-handlers barrel export and would bloat this diff.Test plan
bun test src/domains/subagents/subagent-store.test.ts— 31 tests passbunx tsc --noEmit— zero new errors (all errors are pre-existing@/generated/*module resolution)bun run lint— cleanLink to Devin session: https://app.devin.ai/sessions/565d827296144ac9bf12bd108169e5ef
Requested by: @ashleeradka