Skip to content

Commit 269f9b8

Browse files
authored
fix(mothership): roll back a failed effort save by pick, not by value (#8655)
Picks queued behind an in-flight save run onMutate at once, so after low, high, low a failed first save dropped the newest low and the picker fell back to the stale server effort. Each pick now carries a token and a failed save drops only its own pick.
1 parent 46cd710 commit 269f9b8

6 files changed

Lines changed: 101 additions & 21 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/model-selector.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export function ModelSelector() {
3535
const { chatId } = useChatSurface()
3636
const { data: chatHistory } = useMothershipChatHistory(chatId)
3737
const chatPick = useMothershipEffortStore((state) =>
38-
chatId ? state.chatEfforts[chatId] : undefined
38+
chatId ? state.chatEfforts[chatId]?.effort : undefined
3939
)
4040
const newChatEffort = useMothershipEffortStore((state) => state.newChatEffort)
4141
const setNewChatEffort = useMothershipEffortStore((state) => state.setNewChatEffort)

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3411,7 +3411,7 @@ export function useChat(
34113411
options?.requestMode === 'assistant'
34123412
? undefined
34133413
: requestChatId
3414-
? (effortStore.chatEfforts[requestChatId] ??
3414+
? (effortStore.chatEfforts[requestChatId]?.effort ??
34153415
queryClient.getQueryData<MothershipChatHistory>(
34163416
mothershipChatKeys.detail(requestChatId)
34173417
)?.effort)

‎apps/sim/hooks/queries/mothership-chats.test.ts‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { jsonResponse } from '@sim/testing/helpers/http'
22
import { reactQueryMock, reactQueryMockFns } from '@sim/testing/mocks/react-query.mock'
33
import { sleep } from '@sim/utils/helpers'
4+
import type { MutationObserverOptions } from '@tanstack/react-query'
45
import { beforeEach, describe, expect, it, vi } from 'vitest'
56

67
const { suspendBrowserScope, suspendTerminalScope, clearChat } = vi.hoisted(() => ({
@@ -23,7 +24,12 @@ vi.mock('@/lib/terminal/transport', () => ({
2324
suspendTerminalScope,
2425
}))
2526

26-
import { useDeleteMothershipChats } from '@/hooks/queries/mothership-chats'
27+
import type { MothershipEffort } from '@/lib/mothership/model-options'
28+
import {
29+
useDeleteMothershipChats,
30+
useSetMothershipChatEffort,
31+
} from '@/hooks/queries/mothership-chats'
32+
import { useMothershipEffortStore } from '@/stores/mothership-effort/store'
2733

2834
const queryClient = reactQueryMockFns.mockQueryClient
2935

@@ -93,4 +99,40 @@ describe('tasks query boundary parsing', () => {
9399
queryKey: ['mothership-chats', 'detail', 'chat-b'],
94100
})
95101
})
102+
103+
it('keeps the latest effort pick when an earlier queued save of the same value fails', async () => {
104+
const tanstack =
105+
await vi.importActual<typeof import('@tanstack/react-query')>('@tanstack/react-query')
106+
const client = new tanstack.QueryClient()
107+
const observer = new tanstack.MutationObserver(
108+
client,
109+
useSetMothershipChatEffort('chat-1') as unknown as MutationObserverOptions<
110+
void,
111+
Error,
112+
MothershipEffort,
113+
{ pick: number }
114+
>
115+
)
116+
const saves = [
117+
Promise.withResolvers<Response>(),
118+
Promise.withResolvers<Response>(),
119+
Promise.withResolvers<Response>(),
120+
]
121+
for (const save of saves) vi.mocked(fetch).mockReturnValueOnce(save.promise)
122+
useMothershipEffortStore.getState().reset()
123+
const outcomes = (['low', 'high', 'low'] as const).map((effort) =>
124+
observer.mutate(effort).catch(() => undefined)
125+
)
126+
await sleep(1)
127+
expect(fetch).toHaveBeenCalledTimes(1)
128+
129+
saves[0].resolve(new Response('save failed', { status: 500 }))
130+
await outcomes[0]
131+
expect(useMothershipEffortStore.getState().chatEfforts['chat-1']?.effort).toBe('low')
132+
133+
saves[1].resolve(jsonResponse({ success: true }))
134+
saves[2].resolve(jsonResponse({ success: true }))
135+
await Promise.all(outcomes)
136+
expect(useMothershipEffortStore.getState().chatEfforts['chat-1']?.effort).toBe('low')
137+
})
96138
})

‎apps/sim/hooks/queries/mothership-chats.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -610,11 +610,15 @@ export function useSetMothershipChatEffort(chatId: string | undefined) {
610610
return setChatEffort({ chatId, effort })
611611
},
612612
scope: { id: `mothership-chat-effort:${chatId ?? ''}` },
613+
// Runs at once even while an earlier save for this chat holds the scope, so a failed
614+
// save rolls back its own pick by token, never a later pick of the same value.
613615
onMutate: (effort) => {
614-
if (chatId) useMothershipEffortStore.getState().setChatEffort(chatId, effort)
616+
if (!chatId) return undefined
617+
return { pick: useMothershipEffortStore.getState().setChatEffort(chatId, effort) }
615618
},
616-
onError: (_error, effort) => {
617-
if (chatId) useMothershipEffortStore.getState().dropChatEffort(chatId, effort)
619+
onError: (_error, _effort, context) => {
620+
if (chatId && context)
621+
useMothershipEffortStore.getState().dropChatEffort(chatId, context.pick)
618622
},
619623
onSuccess: (_data, effort) => {
620624
queryClient.setQueryData<MothershipChatHistory>(

‎apps/sim/stores/mothership-effort/store.test.ts‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,30 @@ describe('Build reasoning preferences', () => {
4242

4343
it('keeps a newer chat pick when an older pick fails to save', () => {
4444
const store = useMothershipEffortStore.getState()
45-
store.setChatEffort('chat-1', 'low')
45+
const low = store.setChatEffort('chat-1', 'low')
46+
const high = store.setChatEffort('chat-1', 'high')
47+
store.dropChatEffort('chat-1', low)
48+
expect(useMothershipEffortStore.getState().chatEfforts['chat-1']?.effort).toBe('high')
49+
store.dropChatEffort('chat-1', high)
50+
expect(useMothershipEffortStore.getState().chatEfforts).toEqual({})
51+
})
52+
53+
it('keeps a newer pick of the same value when the first pick fails to save', () => {
54+
const store = useMothershipEffortStore.getState()
55+
const first = store.setChatEffort('chat-1', 'low')
4656
store.setChatEffort('chat-1', 'high')
47-
store.dropChatEffort('chat-1', 'low')
48-
expect(useMothershipEffortStore.getState().chatEfforts).toEqual({ 'chat-1': 'high' })
49-
store.dropChatEffort('chat-1', 'high')
57+
const latest = store.setChatEffort('chat-1', 'low')
58+
store.dropChatEffort('chat-1', first)
59+
expect(useMothershipEffortStore.getState().chatEfforts['chat-1']?.effort).toBe('low')
60+
store.dropChatEffort('chat-1', latest)
5061
expect(useMothershipEffortStore.getState().chatEfforts).toEqual({})
5162
})
63+
64+
it('keeps an adopted new-chat pick when a stale save token fails', () => {
65+
const store = useMothershipEffortStore.getState()
66+
const stale = store.setChatEffort('chat-1', 'low')
67+
store.adoptNewChatEffort('chat-1', 'low')
68+
store.dropChatEffort('chat-1', stale)
69+
expect(useMothershipEffortStore.getState().chatEfforts['chat-1']?.effort).toBe('low')
70+
})
5271
})

‎apps/sim/stores/mothership-effort/store.ts‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ import {
77
resolveMothershipModelSettings,
88
} from '@/lib/mothership/model-options'
99

10+
/** A chat's effort pick and the token that tells it apart from other picks of the same value. */
11+
interface ChatEffortPick {
12+
effort: MothershipEffort
13+
pick: number
14+
}
15+
1016
interface MothershipEffortState {
1117
modelSelection: ModelSelection
1218
setModel: (model: ModelSelection['model']) => void
@@ -21,10 +27,11 @@ interface MothershipEffortState {
2127
* Picks made in existing chats this session, by chat id. They win over the chat's loaded
2228
* value, so a detail refetch or a save still in flight never shows or sends an older one.
2329
*/
24-
chatEfforts: Record<string, MothershipEffort>
25-
setChatEffort: (chatId: string, effort: MothershipEffort) => void
26-
/** Drops a pick whose save failed, unless a newer pick replaced it. */
27-
dropChatEffort: (chatId: string, effort: MothershipEffort) => void
30+
chatEfforts: Record<string, ChatEffortPick>
31+
/** Records a pick and returns its token for {@link MothershipEffortState.dropChatEffort}. */
32+
setChatEffort: (chatId: string, effort: MothershipEffort) => number
33+
/** Drops a pick whose save failed, unless a newer pick replaced it, even one of the same value. */
34+
dropChatEffort: (chatId: string, pick: number) => void
2835
/** Moves the new-chat pick onto the chat its first send created. */
2936
adoptNewChatEffort: (chatId: string, effort: MothershipEffort) => void
3037
reset: () => void
@@ -39,6 +46,9 @@ const initialState: Pick<
3946
chatEfforts: {},
4047
}
4148

49+
/** Counts picks across chats so a token never repeats within a session. */
50+
let lastChatEffortPick = 0
51+
4252
function withModelSelection(
4353
modelSelection: ModelSelection
4454
): Pick<MothershipEffortState, 'modelSelection'> {
@@ -55,18 +65,23 @@ export const useMothershipEffortStore = create<MothershipEffortState>()(
5565
setModel: (model) =>
5666
set((state) => withModelSelection({ model, fastMode: state.modelSelection.fastMode })),
5767
setNewChatEffort: (newChatEffort) => set({ newChatEffort }),
58-
setChatEffort: (chatId, effort) =>
59-
set((state) => ({ chatEfforts: { ...state.chatEfforts, [chatId]: effort } })),
60-
dropChatEffort: (chatId, effort) =>
68+
setChatEffort: (chatId, effort) => {
69+
const pick = ++lastChatEffortPick
70+
set((state) => ({ chatEfforts: { ...state.chatEfforts, [chatId]: { effort, pick } } }))
71+
return pick
72+
},
73+
dropChatEffort: (chatId, pick) =>
6174
set((state) => {
62-
if (state.chatEfforts[chatId] !== effort) return state
75+
if (state.chatEfforts[chatId]?.pick !== pick) return state
6376
return { chatEfforts: omit(state.chatEfforts, [chatId]) }
6477
}),
65-
adoptNewChatEffort: (chatId, effort) =>
78+
adoptNewChatEffort: (chatId, effort) => {
79+
const pick = ++lastChatEffortPick
6680
set((state) => ({
6781
newChatEffort: null,
68-
chatEfforts: { ...state.chatEfforts, [chatId]: effort },
69-
})),
82+
chatEfforts: { ...state.chatEfforts, [chatId]: { effort, pick } },
83+
}))
84+
},
7085
reset: () => set(initialState),
7186
}),
7287
{

0 commit comments

Comments
 (0)