diff --git a/src/renderer/src/App.test.tsx b/src/renderer/src/App.test.tsx index 0f3dc5fd..b069be69 100644 --- a/src/renderer/src/App.test.tsx +++ b/src/renderer/src/App.test.tsx @@ -4842,6 +4842,41 @@ describe("App", () => { expect(sidebarRenderProbes.row.mock.calls.length).toBeLessThanOrEqual(4); }); + it("renders the App root once when switching between kept conversations (PERF-13)", async () => { + const conversations = Array.from({ length: 3 }, (_, c) => ({ + id: `00000000-0000-4000-9${String(c).padStart(3, "0")}-000000000100`, + projectId, title: `Warm switch ${c}`, updatedAt: 2_000 + c, + messages: [{ + id: `00000000-0000-4000-9${String(c).padStart(3, "0")}-000000000101`, + role: "assistant" as const, content: `Warm body ${c}`, createdAt: 2_000 + c, state: "complete" as const, + }], + })); + vi.mocked(api.conversations.list).mockResolvedValueOnce(conversations); + const { container } = render(); + const list = (): HTMLElement => container.querySelector(".conversation-list")!; + fireEvent.click((await within(list()).findByText("Warm switch 0")).closest("button")!); + await screen.findByText("Warm body 0"); + fireEvent.click(within(list()).getByText("Warm switch 1").closest("button")!); + await screen.findByText("Warm body 1"); + // Let deferred loads (attachment drafts, artifact hydration) settle. + await act(() => new Promise((resolve) => setTimeout(resolve, 50))); + // useUnviewedCompletions runs once per App render: it counts App renders. + assistantTasksProbe.mockClear(); + act(() => { + fireEvent.click(within(list()).getByText("Warm switch 0").closest("button")!); + }); + expect(document.querySelector(".conversation-item--active")).toHaveTextContent("Warm switch 0"); + // The pane order and composer popups no longer update state during render. + expect(assistantTasksProbe).toHaveBeenCalledTimes(1); + // A conversation not kept yet adds a pane; that is still one App render. + assistantTasksProbe.mockClear(); + act(() => { + fireEvent.click(within(list()).getByText("Warm switch 2").closest("button")!); + }); + expect(document.querySelector(".conversation-item--active")).toHaveTextContent("Warm switch 2"); + expect(assistantTasksProbe).toHaveBeenCalledTimes(1); + }); + it("copies and exports a streaming conversation with the text shown on screen", async () => { const { container } = render(); const composer = await screen.findByLabelText("向 GoodBuddy 提问"); diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 77fd3189..0f826588 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -139,7 +139,7 @@ import { messageRenderBatchSize, type ChatScrollSnapshot, } from "./ChatHistoryPane"; -import { reconcilePaneOrder } from "./pane-order"; +import { usePaneOrder } from "./pane-order"; import { getConversationDisplayTitle, isUnusedConversation, type Conversation } from "./chat-conversation"; import { ConversationListView, @@ -2377,8 +2377,8 @@ function App(): React.JSX.Element { ); // Kept panes must keep a stable DOM order. Reordering keyed siblings makes // React detach and reinsert scroll containers, which resets their scrollTop. - // The order lives in state and is reconciled during render; the helper - // returns the previous array when nothing changed, so this settles at once. + // The order is reconciled during render against the previous one (see + // usePaneOrder), without a state update, so a switch renders App once. const cachedConversationIds = useMemo( () => [activeId, ...cachedConversationViews.map((entry) => entry.key)].filter( @@ -2386,15 +2386,7 @@ function App(): React.JSX.Element { ), [activeId, cachedConversationViews], ); - const [conversationPaneOrderState, setConversationPaneOrderState] = - useState([]); - const conversationPaneOrder = reconcilePaneOrder( - conversationPaneOrderState, - cachedConversationIds, - ); - if (conversationPaneOrder !== conversationPaneOrderState) { - setConversationPaneOrderState(conversationPaneOrder); - } + const conversationPaneOrder = usePaneOrder(cachedConversationIds); const activeRuntimeResolution = useMemo( () => runtimeSettings diff --git a/src/renderer/src/conversation-selectors.test.tsx b/src/renderer/src/conversation-selectors.test.tsx new file mode 100644 index 00000000..9f969253 --- /dev/null +++ b/src/renderer/src/conversation-selectors.test.tsx @@ -0,0 +1,41 @@ +import { act, renderHook } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import type { Conversation } from "./chat-conversation"; +import { useConversationTitles } from "./conversation-selectors"; +import { createConversationStores } from "./conversation-store"; +import { usePaneOrder } from "./pane-order"; + +function conversation(id: string, title = id): Conversation { + return { id, title, updatedAt: 1, messages: [] } as unknown as Conversation; +} + +describe("useConversationTitles", () => { + it("keeps the same map while only non-title fields change", () => { + const { conversations: store } = createConversationStores( + [conversation("a"), conversation("b")], + { flushIntervalMs: 0 }, + ); + const { result } = renderHook(() => useConversationTitles(store, "New")); + const first = result.current; + act(() => store.set((current) => current.map((item) => ({ ...item, updatedAt: 2 })))); + expect(result.current).toBe(first); + act(() => store.set((current) => current.map((item) => + item.id === "b" ? { ...item, title: "Renamed" } : item))); + expect(result.current).not.toBe(first); + expect(result.current.get("b")).toBe("Renamed"); + }); +}); + +describe("usePaneOrder", () => { + it("keeps existing panes in place and returns the same array when unchanged", () => { + const { result, rerender } = renderHook(({ ids }) => usePaneOrder(ids), { + initialProps: { ids: ["a", "b"] }, + }); + const first = result.current; + expect(first).toEqual(["a", "b"]); + rerender({ ids: ["b", "a"] }); + expect(result.current).toBe(first); + rerender({ ids: ["c", "a", "b"] }); + expect(result.current).toEqual(["a", "b", "c"]); + }); +}); diff --git a/src/renderer/src/conversation-selectors.ts b/src/renderer/src/conversation-selectors.ts index b3d6b4fd..c44327f3 100644 --- a/src/renderer/src/conversation-selectors.ts +++ b/src/renderer/src/conversation-selectors.ts @@ -1,4 +1,4 @@ -import { useCallback } from "react"; +import { useCallback, useMemo } from "react"; import type { AssistantTask } from "../../shared/assistant-contracts"; import type { Message } from "./ChatTimeline"; import { getConversationDisplayTitle, isUnusedConversation, type Conversation } from "./chat-conversation"; @@ -21,19 +21,40 @@ export function useConversationCount(store: ConversationStore): number { return useConversationStoreSelector(store, selectCount); } -/** Display titles by conversation ID. */ +// Streaming replaces the list on every flush. The selector compares the +// titles in order and keeps the previous map while none changed, instead of +// building a new map each time only to compare it entry by entry. +function createTitlesSelector( + defaultTitle: string, +): (conversations: Conversation[]) => ReadonlyMap { + const previous: { ids: string[]; titles: string[]; map?: ReadonlyMap } = { + ids: [], + titles: [], + }; + return (conversations) => { + const titles = conversations.map((conversation) => + getConversationDisplayTitle(conversation, defaultTitle)); + if ( + previous.map && + previous.ids.length === conversations.length && + conversations.every((conversation, index) => + previous.ids[index] === conversation.id && previous.titles[index] === titles[index]) + ) { + return previous.map; + } + previous.ids = conversations.map((conversation) => conversation.id); + previous.titles = titles; + previous.map = new Map(previous.ids.map((id, index) => [id, titles[index]!])); + return previous.map; + }; +} + +/** Display titles by conversation ID; the same map while no title changes. */ export function useConversationTitles( store: ConversationStore, defaultTitle: string, ): ReadonlyMap { - const selector = useCallback( - (conversations: Conversation[]) => - new Map(conversations.map((conversation) => [ - conversation.id, - getConversationDisplayTitle(conversation, defaultTitle), - ])), - [defaultTitle], - ); + const selector = useMemo(() => createTitlesSelector(defaultTitle), [defaultTitle]); return useConversationStoreSelector(store, selector, sameMapEntries); } diff --git a/src/renderer/src/pane-order.ts b/src/renderer/src/pane-order.ts index 222a8551..745c917f 100644 --- a/src/renderer/src/pane-order.ts +++ b/src/renderer/src/pane-order.ts @@ -1,3 +1,5 @@ +import { useMemo, useState } from "react"; + /** * Reconciles a stable order for kept-alive panes. Ids already in `previous` * keep their relative position, ids missing from `ids` are dropped and new ids @@ -22,3 +24,19 @@ export function reconcilePaneOrder( } return next; } +const previousOrders = new WeakMap(); + +/** + * The reconciled pane order, derived during render. The previous order is + * kept per hook slot (like useStableDerivedValue), so a conversation switch + * renders App once instead of once more for a state update during render. + */ +export function usePaneOrder(ids: readonly string[]): readonly string[] { + const [slot] = useState(() => ({})); + const order = useMemo( + () => reconcilePaneOrder(previousOrders.get(slot) ?? [], ids), + [ids, slot], + ); + previousOrders.set(slot, order); + return order; +} \ No newline at end of file