Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 164 additions & 0 deletions apps/desktop/src/app/chat/index.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
import { useState } from 'react'
import { MemoryRouter } from 'react-router'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

import { assistantTextPart, type ChatMessage } from '@/lib/chat-messages'
import {
$activeSessionId,
$awaitingResponse,
$busy,
$contextSuggestions,
$currentCwd,
$currentModel,
$currentProvider,
$freshDraftReady,
$gatewayState,
$messages,
$selectedStoredSessionId,
$sessions
} from '@/store/session'

const threadRenderCount = vi.hoisted(() => ({ current: 0 }))

vi.mock('@/components/assistant-ui/thread', async () => {
const React = await import('react')

return {
Thread: () => {
threadRenderCount.current += 1

return React.createElement('div', { 'data-testid': 'thread' })
}
}
})

vi.mock('@/components/Backdrop', async () => {
const React = await import('react')

return { Backdrop: () => React.createElement('div', { 'data-testid': 'backdrop' }) }
})

vi.mock('@/components/prompt-overlays', () => ({ PromptOverlays: () => null }))
vi.mock('@/components/chat/vibe-hearts', () => ({ COMPOSER_HEART_CONFIG: {}, HeartField: () => null }))
vi.mock('@/lib/model-options', () => ({
modelOptionsQueryKey: (...parts: unknown[]) => ['model-options', ...parts],
requestModelOptions: vi.fn(async () => ({ models: [] }))
}))
vi.mock('./chat-drop-overlay', () => ({ ChatDropOverlay: () => null }))
vi.mock('./chat-swap-overlay', () => ({ ChatSwapOverlay: () => null }))
vi.mock('./composer', () => ({ ChatBar: () => null, ChatBarFallback: () => null }))
vi.mock('./hooks/use-file-drop-zone', () => ({
useFileDropZone: () => ({ dragKind: null, dropHandlers: {} })
}))
vi.mock('./sidebar/session-actions-menu', async () => {
const React = await import('react')

return {
SessionActionsMenu: ({ children }: { children: React.ReactNode }) =>
React.createElement('div', { 'data-testid': 'session-actions-menu' }, children)
}
})

const { ChatView } = await import('./index')

function assistantMessage(id: string, text: string): ChatMessage {
return {
id,
parts: [assistantTextPart(text)],
role: 'assistant'
}
}

describe('ChatView render isolation', () => {
beforeEach(() => {
threadRenderCount.current = 0
$activeSessionId.set('runtime-1')
$awaitingResponse.set(false)
$busy.set(false)
$contextSuggestions.set([])
$currentCwd.set('/work')
$currentModel.set('test-model')
$currentProvider.set('test-provider')
$freshDraftReady.set(false)
$gatewayState.set('closed')
$messages.set([assistantMessage('assistant-1', 'Stable historical answer')])
$selectedStoredSessionId.set('stored-1')
$sessions.set([{ id: 'stored-1', message_count: 1, title: 'Stable chat' } as never])
})

afterEach(() => {
cleanup()
vi.restoreAllMocks()
$activeSessionId.set(null)
$awaitingResponse.set(false)
$busy.set(false)
$contextSuggestions.set([])
$currentCwd.set('')
$currentModel.set('')
$currentProvider.set('')
$freshDraftReady.set(false)
$gatewayState.set('idle')
$messages.set([])
$selectedStoredSessionId.set(null)
$sessions.set([])
})

it('does not re-render chat history when an unrelated parent idle tick updates', () => {
const props = {
gateway: null,
maxVoiceRecordingSeconds: 120,
onAddContextRef: vi.fn(),
onAddUrl: vi.fn(),
onAttachDroppedItems: vi.fn(),
onAttachImageBlob: vi.fn(),
onBranchInNewChat: vi.fn(),
onCancel: vi.fn(),
onDeleteSelectedSession: vi.fn(),
onEdit: vi.fn(),
onPasteClipboardImage: vi.fn(),
onPickFiles: vi.fn(),
onPickFolders: vi.fn(),
onPickImages: vi.fn(),
onReload: vi.fn(),
onRemoveAttachment: vi.fn(),
onRetryResume: vi.fn(),
onSteer: vi.fn(),
onSubmit: vi.fn(),
onThreadMessagesChange: vi.fn(),
onToggleSelectedPin: vi.fn(),
onTranscribeAudio: vi.fn()
}

const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false } }
})

function ParentTickHarness() {
const [tick, setTick] = useState(0)

return (
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={['/stored-1']}>
<button onClick={() => setTick(value => value + 1)} type="button">
parent tick {tick}
</button>
<ChatView {...props} />
</MemoryRouter>
</QueryClientProvider>
)
}

render(<ParentTickHarness />)

expect(screen.getByTestId('thread')).toBeTruthy()
expect(threadRenderCount.current).toBe(1)

fireEvent.click(screen.getByRole('button', { name: /parent tick/i }))

// memo(ChatView) with stable props must absorb the parent's idle tick —
// the transcript (Thread) must not re-render. This is PR #38470's contract.
expect(threadRenderCount.current).toBe(1)
})
})
9 changes: 6 additions & 3 deletions apps/desktop/src/app/chat/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useStore } from '@nanostores/react'
import { useQuery } from '@tanstack/react-query'
import type { ReadableAtom } from 'nanostores'
import type * as React from 'react'
import { Suspense, useCallback, useEffect, useMemo, useState } from 'react'
import { memo, Suspense, useCallback, useEffect, useMemo, useState } from 'react'
import { useLocation } from 'react-router'

import type { SubmitTextOptions } from '@/app/session/hooks/use-prompt-actions/utils'
Expand Down Expand Up @@ -240,7 +240,10 @@ function ChatRuntimeBoundary({
return <AssistantRuntimeProvider runtime={runtime}>{children}</AssistantRuntimeProvider>
}

export function ChatView({
// Memoized: the tile caller (session-tile.tsx) and the contrib surface re-render
// on idle ticks unrelated to the chat; with stable callback props (hoisted to
// useCallback at the call sites) memo() lets the whole chat shell skip those.
export const ChatView = memo(function ChatView({
className,
gateway,
modelMenuContent,
Expand Down Expand Up @@ -596,4 +599,4 @@ export function ChatView({
</ChatRuntimeBoundary>
</div>
)
}
})
50 changes: 39 additions & 11 deletions apps/desktop/src/app/chat/session-tile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,13 @@ function buildTileView(storedSessionId: string): SessionView {
}
}

// Module-level constants so these ChatView props are referentially stable —
// tiles have no pin/delete affordance, and transcription needs no per-tile state.
const noop = () => undefined

const tileTranscribeAudio = async (audio: Blob) =>
(await transcribeAudio(await blobToDataUrl(audio), audio.type)).transcript

function TileChat({
runtimeId,
storedSessionId,
Expand Down Expand Up @@ -144,6 +151,27 @@ function TileChat({
scope: { add: attachments.add, remove: attachments.remove, target: scope.target }
})

// ChatView is memo()d — every callback prop must be referentially stable or
// the memo never holds and each tile-level render (idle ticks, unrelated
// store updates) re-renders the whole chat shell. The individual composer
// functions are useCallback'd inside useComposerActions, so hoisting these
// wrappers onto them keeps identity stable across renders.
const { addContextRefAttachment, pasteClipboardImage, pickContextPaths, pickImages, removeAttachment } = composer

const onAddUrl = useCallback(
(url: string) => addContextRefAttachment(`@url:${formatRefValue(url)}`, url),
[addContextRefAttachment]
)
const onPasteClipboardImage = useCallback(
(opts?: { silent?: boolean }) => pasteClipboardImage(opts),
[pasteClipboardImage]
)
const onPickFiles = useCallback(() => void pickContextPaths('file'), [pickContextPaths])
const onPickFolders = useCallback(() => void pickContextPaths('folder'), [pickContextPaths])
const onPickImages = useCallback(() => void pickImages(), [pickImages])
const onRemoveAttachment = useCallback((id: string) => void removeAttachment(id), [removeAttachment])
const onRetryResume = useCallback(() => patchSessionTile(storedSessionId, { error: undefined }), [storedSessionId])

// Per-tile model menu — rendered under this tile's SessionView so the pill
// + switch target THIS runtime, not the primary (which may be mid-turn).
const modelMenuContent = useMemo(
Expand All @@ -165,27 +193,27 @@ function TileChat({
<ChatView
gateway={gateway}
modelMenuContent={modelMenuContent}
onAddContextRef={composer.addContextRefAttachment}
onAddUrl={url => composer.addContextRefAttachment(`@url:${formatRefValue(url)}`, url)}
onAddContextRef={addContextRefAttachment}
onAddUrl={onAddUrl}
onAttachDroppedItems={composer.attachDroppedItems}
onAttachImageBlob={composer.attachImageBlob}
onCancel={actions.cancelRun}
onDeleteSelectedSession={() => undefined}
onDeleteSelectedSession={noop}
onDismissError={actions.dismissError}
onEdit={actions.editMessage}
onPasteClipboardImage={opts => composer.pasteClipboardImage(opts)}
onPickFiles={() => void composer.pickContextPaths('file')}
onPickFolders={() => void composer.pickContextPaths('folder')}
onPickImages={() => void composer.pickImages()}
onPasteClipboardImage={onPasteClipboardImage}
onPickFiles={onPickFiles}
onPickFolders={onPickFolders}
onPickImages={onPickImages}
onReload={actions.reloadFromMessage}
onRemoveAttachment={id => void composer.removeAttachment(id)}
onRemoveAttachment={onRemoveAttachment}
onRestoreToMessage={actions.restoreToMessage}
onRetryResume={() => patchSessionTile(storedSessionId, { error: undefined })}
onRetryResume={onRetryResume}
onSteer={actions.steerPrompt}
onSubmit={actions.submitText}
onThreadMessagesChange={actions.handleThreadMessagesChange}
onToggleSelectedPin={() => undefined}
onTranscribeAudio={async audio => (await transcribeAudio(await blobToDataUrl(audio), audio.type)).transcript}
onToggleSelectedPin={noop}
onTranscribeAudio={tileTranscribeAudio}
/>
</ComposerScopeProvider>
</SessionViewProvider>
Expand Down
6 changes: 5 additions & 1 deletion apps/desktop/src/lib/incremental-external-store-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,13 @@ export function useIncrementalExternalStoreRuntime<T extends ThreadMessage>(
): AssistantRuntime {
const [runtime] = useState(() => new IncrementalExternalStoreRuntimeCore(store as ExternalStoreAdapter))

// Re-sync the adapter only when it actually changes — a dep-less effect ran
// on EVERY render of the chat surface. `__internal_setAdapter` early-exits
// when the store is unchanged, so gating on [runtime, store] is behavior-
// preserving while skipping the per-render call entirely.
useEffect(() => {
runtime.setAdapter(store as ExternalStoreAdapter)
})
}, [runtime, store])

const { modelContext } = useRuntimeAdapters() ?? {}

Expand Down
Loading