diff --git a/apps/desktop/src/app/chat/index.test.tsx b/apps/desktop/src/app/chat/index.test.tsx new file mode 100644 index 000000000000..8caace4cc98b --- /dev/null +++ b/apps/desktop/src/app/chat/index.test.tsx @@ -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 ( + + + + + + + ) + } + + render() + + 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) + }) +}) diff --git a/apps/desktop/src/app/chat/index.tsx b/apps/desktop/src/app/chat/index.tsx index 3d72bf341088..7a0c0a2674c3 100644 --- a/apps/desktop/src/app/chat/index.tsx +++ b/apps/desktop/src/app/chat/index.tsx @@ -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' @@ -240,7 +240,10 @@ function ChatRuntimeBoundary({ return {children} } -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, @@ -596,4 +599,4 @@ export function ChatView({ ) -} +}) diff --git a/apps/desktop/src/app/chat/session-tile.tsx b/apps/desktop/src/app/chat/session-tile.tsx index 27954abc2a56..d279aea439e7 100644 --- a/apps/desktop/src/app/chat/session-tile.tsx +++ b/apps/desktop/src/app/chat/session-tile.tsx @@ -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, @@ -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( @@ -165,27 +193,27 @@ function TileChat({ 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} /> diff --git a/apps/desktop/src/lib/incremental-external-store-runtime.ts b/apps/desktop/src/lib/incremental-external-store-runtime.ts index 0df3ed9b2e09..1ccc5121d978 100644 --- a/apps/desktop/src/lib/incremental-external-store-runtime.ts +++ b/apps/desktop/src/lib/incremental-external-store-runtime.ts @@ -243,9 +243,13 @@ export function useIncrementalExternalStoreRuntime( ): 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() ?? {}