Skip to content

Commit 8ebd58f

Browse files
committed
fix(source-chat): adopt session transcript
before claiming the shared list and restore console spy safely
1 parent 2ba4c5c commit 8ebd58f

2 files changed

Lines changed: 109 additions & 17 deletions

File tree

frontend/src/lib/hooks/use-source-chat.test.tsx

Lines changed: 78 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
55
import { describe, it, expect, vi, beforeEach } from 'vitest'
66
import { useSourceChat } from './use-source-chat'
77
import { sourceChatApi } from '@/lib/api/source-chat'
8-
import { SourceChatSession, SourceChatSessionWithMessages } from '@/lib/types/api'
8+
import { SourceChatSession, SourceChatSessionWithMessages, SourceChatMessage } from '@/lib/types/api'
99

1010
// useTranslation is mocked globally in setup.ts (t returns the key string).
1111

@@ -451,19 +451,24 @@ describe('useSourceChat sendMessage streaming', () => {
451451
vi.mocked(sourceChatApi.sendMessage).mockRejectedValue(axiosError)
452452

453453
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
454-
const { result } = renderHook(() => useSourceChat('source:1'), { wrapper: makeWrapper() })
454+
// A failing assertion must not leave the spy installed — it would silence
455+
// console.error for every test that runs after this one.
456+
try {
457+
const { result } = renderHook(() => useSourceChat('source:1'), { wrapper: makeWrapper() })
455458

456-
await waitFor(() => expect(result.current.currentSessionId).toBe('session:1'))
459+
await waitFor(() => expect(result.current.currentSessionId).toBe('session:1'))
457460

458-
await act(async () => {
459-
await result.current.sendMessage('hello')
460-
})
461+
await act(async () => {
462+
await result.current.sendMessage('hello')
463+
})
461464

462-
expect(consoleError).toHaveBeenCalled()
463-
const logged = JSON.stringify(consoleError.mock.calls)
464-
expect(logged).not.toContain('secret-token')
465-
expect(logged).not.toContain('Authorization')
466-
consoleError.mockRestore()
465+
expect(consoleError).toHaveBeenCalled()
466+
const logged = JSON.stringify(consoleError.mock.calls)
467+
expect(logged).not.toContain('secret-token')
468+
expect(logged).not.toContain('Authorization')
469+
} finally {
470+
consoleError.mockRestore()
471+
}
467472
})
468473

469474
it('keeps a send on its own session when the user switches during pre-send hydration', async () => {
@@ -528,6 +533,68 @@ describe('useSourceChat sendMessage streaming', () => {
528533
expect(result.current.messages).toEqual(otherMessages)
529534
})
530535

536+
it("adopts the switched-to session's cached messages before a submit claims the shared list", async () => {
537+
const otherSession: SourceChatSession = { ...session, id: 'session:2', title: 'Other' }
538+
const aMessages = [
539+
{ id: 'msg-a', type: 'ai' as const, content: 'answered in A', timestamp: '2026-01-01T00:00:00Z' },
540+
]
541+
const pendingInB = [
542+
{ id: 'msg-b-pending', type: 'human' as const, content: 'hello', timestamp: '2026-01-01T00:00:00Z' },
543+
]
544+
vi.mocked(sourceChatApi.listSessions).mockResolvedValue([session, otherSession])
545+
// The backend persists the exchange, so the send's final refetch sees it.
546+
let bMessages: SourceChatMessage[] = pendingInB
547+
vi.mocked(sourceChatApi.getSession).mockImplementation(
548+
(_sourceId, sessionId) =>
549+
Promise.resolve(
550+
sessionId === 'session:2'
551+
? { ...otherSession, messages: bMessages }
552+
: { ...session, messages: aMessages },
553+
) as any
554+
)
555+
vi.mocked(sourceChatApi.sendMessage).mockImplementation(() => {
556+
bMessages = [
557+
...pendingInB,
558+
{ id: 'ai-b', type: 'ai' as const, content: 'answer', timestamp: '2026-01-01T00:00:01Z' },
559+
]
560+
return Promise.resolve(
561+
sseStream([{ type: 'ai_message', content: 'answer' }, { type: 'complete' }]) as any
562+
)
563+
})
564+
565+
const { result } = renderHook(() => useSourceChat('source:1'), { wrapper: makeWrapper() })
566+
567+
// Visit session:2 so its query is cached, then return to session:1 — the
568+
// shared list belongs to session:1 again.
569+
act(() => {
570+
result.current.switchSession('session:2')
571+
})
572+
await waitFor(() => expect(result.current.messages).toEqual(pendingInB))
573+
act(() => {
574+
result.current.switchSession('session:1')
575+
})
576+
await waitFor(() => expect(result.current.messages).toEqual(aMessages))
577+
578+
// Submit in the same tick as the switch, before the session-query effect
579+
// can swap the list over — the send must bring session:2's transcript with
580+
// it instead of writing into session:1's.
581+
let sendPromise!: Promise<void>
582+
act(() => {
583+
result.current.switchSession('session:2')
584+
sendPromise = result.current.sendMessage('hello')
585+
})
586+
587+
await act(async () => {
588+
await sendPromise
589+
})
590+
591+
// The trailing turn is one session:2 already holds server-side — reusing
592+
// its id lets the backend dedup instead of appending a duplicate human turn.
593+
const [, , payload] = vi.mocked(sourceChatApi.sendMessage).mock.calls[0]
594+
expect(payload.message_id).toBe('msg-b-pending')
595+
expect(result.current.messages.map((m) => m.content)).toEqual(['hello', 'answer'])
596+
})
597+
531598
it('rehydrates the streaming session when the user switches away and back', async () => {
532599
const otherSession: SourceChatSession = { ...session, id: 'session:2', title: 'Other' }
533600
const persisted = [

frontend/src/lib/hooks/use-source-chat.ts

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { sourceChatApi } from '@/lib/api/source-chat'
99
import { selectMessageId } from '@/lib/utils/source-chat-message'
1010
import {
1111
SourceChatSession,
12+
SourceChatSessionWithMessages,
1213
SourceChatMessage,
1314
SourceChatContextIndicator,
1415
CreateSourceChatSessionRequest,
@@ -249,7 +250,6 @@ export function useSourceChat(sourceId: string) {
249250

250251
const streamSessionId = sessionId
251252
streamingSessionRef.current = streamSessionId
252-
messagesSessionRef.current = streamSessionId
253253

254254
// The turn belongs to the session it was composed in, so it is still sent
255255
// after the user navigates away — but the shared list then shows another
@@ -259,20 +259,45 @@ export function useSourceChat(sourceId: string) {
259259
currentSessionIdRef.current === streamSessionId &&
260260
messagesSessionRef.current === streamSessionId
261261

262+
// The shared list must actually represent this session before the send
263+
// writes into it. A submit can land between `switchSession` (which changes
264+
// the selected session) and the session-query effect (which swaps the list
265+
// to that session's messages), so the list may still show the previous
266+
// session's transcript here — claiming it without first applying this
267+
// session's messages would append the turn and its streamed answer to the
268+
// wrong transcript, and the cached-snapshot path below would never replace
269+
// the list. Adopt this session's own state instead: a list that already
270+
// represents the session keeps its content (it can be fresher than the
271+
// cache — it carries optimistic turns), and a user who navigated away
272+
// during hydration gets neither the claim nor the overwrite.
273+
const adoptSessionList = (authoritative: SourceChatMessage[]): SourceChatMessage[] => {
274+
if (currentSessionIdRef.current !== streamSessionId) return authoritative
275+
if (messagesSessionRef.current === streamSessionId) return messagesRef.current
276+
messagesSessionRef.current = streamSessionId
277+
applyMessages(authoritative)
278+
return messagesRef.current
279+
}
280+
262281
// `messages` only holds authoritative state once the session query has
263282
// resolved, and the composer is gated on `isStreaming` alone — a send can
264283
// land while that query is still in flight. Deriving the id from the empty
265284
// list would mint a fresh one for a turn the server still holds as pending,
266285
// and the backend would append a duplicate human turn.
267286
let knownMessages = messagesRef.current
268-
if (!sessionJustCreated && !queryClient.getQueryData(['sourceChatSession', sourceId, streamSessionId])) {
287+
const cachedSnapshot = sessionJustCreated
288+
? undefined
289+
: queryClient.getQueryData<SourceChatSessionWithMessages>([
290+
'sourceChatSession',
291+
sourceId,
292+
streamSessionId
293+
])
294+
if (cachedSnapshot?.messages) {
295+
knownMessages = adoptSessionList(cachedSnapshot.messages)
296+
} else if (!sessionJustCreated) {
269297
try {
270298
const hydrated = await fetchSession(streamSessionId)
271299
if (hydrated?.messages) {
272-
knownMessages = hydrated.messages
273-
if (ownsMessages()) {
274-
applyMessages(knownMessages)
275-
}
300+
knownMessages = adoptSessionList(hydrated.messages)
276301
}
277302
} catch (err) {
278303
console.error('Error loading chat session before send:', getLogSafeErrorMessage(err))

0 commit comments

Comments
 (0)