From 9ec3dd8be106070e581ab64a2a967fd88c701264 Mon Sep 17 00:00:00 2001 From: karlo Date: Tue, 16 Jun 2026 13:16:31 -0700 Subject: [PATCH] perf(sidebar): stop re-rendering on every streamed token The sidebar consumed the whole `sessions` record via `useSessions()`, which immer replaces on every appended event (one per streamed token). Since the sidebar is mounted at the root, that re-rendered the whole tree on every token. `deriveTaskData` only reads four session fields (isPromptPending, pendingPermissions size, cloudStatus, cloudOutput.pr_url) -- never `events`: - Add `computeSidebarSessionSignature` (core, pure): a primitive signature of just those fields. - Add `useSidebarSessionMap` (ui): subscribes to that signature and rebuilds the taskId -> session map only when a sidebar-relevant field changes. - `useSidebarData` uses it instead of `useSessions()`. Render-count test: 20 streamed tokens caused 20 sidebar re-renders before, 0 after (and 1 when a relevant field actually changes). Part of #2162 Generated-By: PostHog Code Task-Id: 8e9f327d-84b1-4608-9f48-c3038dbf87ca --- packages/core/src/sidebar/buildSidebarData.ts | 29 +++++++++ .../computeSidebarSessionSignature.test.ts | 52 ++++++++++++++++ .../ui/src/features/sessions/sessionStore.ts | 1 + .../ui/src/features/sessions/useSession.ts | 25 ++++++++ .../sessions/useSidebarSessionMap.test.tsx | 62 +++++++++++++++++++ .../ui/src/features/sidebar/useSidebarData.ts | 13 +--- 6 files changed, 171 insertions(+), 11 deletions(-) create mode 100644 packages/core/src/sidebar/computeSidebarSessionSignature.test.ts create mode 100644 packages/ui/src/features/sessions/useSidebarSessionMap.test.tsx diff --git a/packages/core/src/sidebar/buildSidebarData.ts b/packages/core/src/sidebar/buildSidebarData.ts index 7c61c08a1e..ad5aff7dc5 100644 --- a/packages/core/src/sidebar/buildSidebarData.ts +++ b/packages/core/src/sidebar/buildSidebarData.ts @@ -1,3 +1,4 @@ +import type { AgentSession } from "@posthog/shared"; import type { TaskRunStatus } from "@posthog/shared/domain-types"; import { getRepositoryInfo } from "./groupTasks"; import type { TaskData } from "./sidebarData.types"; @@ -211,3 +212,31 @@ export function sliceChronological( hasMore: sortedUnpinnedTasks.length > historyVisibleCount, }; } + +/** Only the session fields the sidebar actually reads (see deriveTaskData). */ +type SidebarSessionFields = Pick< + AgentSession, + "taskId" | "isPromptPending" | "pendingPermissions" | "cloudStatus" +> & { cloudOutput?: { pr_url?: unknown } | null }; + +/** + * A compact, primitive signature of just the session fields the sidebar reads. + * The sidebar subscribes to this string instead of the whole sessions record, + * so streaming token appends — which only mutate `session.events` — don't + * re-render the sidebar (and, since it's mounted at the root, the whole tree). + */ +export function computeSidebarSessionSignature( + sessions: Record, +): string { + const parts: string[] = []; + for (const s of Object.values(sessions)) { + if (!s.taskId) continue; + const prUrl = + typeof s.cloudOutput?.pr_url === "string" ? s.cloudOutput.pr_url : ""; + parts.push( + `${s.taskId}\t${s.isPromptPending ? 1 : 0}\t${s.pendingPermissions.size}\t${s.cloudStatus ?? ""}\t${prUrl}`, + ); + } + parts.sort(); + return parts.join("\n"); +} diff --git a/packages/core/src/sidebar/computeSidebarSessionSignature.test.ts b/packages/core/src/sidebar/computeSidebarSessionSignature.test.ts new file mode 100644 index 0000000000..09ab89feed --- /dev/null +++ b/packages/core/src/sidebar/computeSidebarSessionSignature.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "vitest"; +import { computeSidebarSessionSignature } from "./buildSidebarData"; + +// Minimal session shape the signature reads. `events` is included to prove it +// is ignored (the streaming hot path only mutates `events`). +function session(over: Record = {}) { + return { + taskId: "t1", + isPromptPending: false, + pendingPermissions: new Map(), + cloudStatus: undefined, + cloudOutput: null, + events: [], + ...over, + } as never; +} + +describe("computeSidebarSessionSignature", () => { + it("ignores events, so streaming tokens don't change it", () => { + const few = computeSidebarSessionSignature({ + r1: session({ events: [1, 2] }), + }); + const many = computeSidebarSessionSignature({ + r1: session({ events: [1, 2, 3, 4, 5, 6, 7] }), + }); + expect(many).toBe(few); + }); + + it.each([ + { label: "isPromptPending", over: { isPromptPending: true } }, + { label: "cloudStatus", over: { cloudStatus: "in_progress" } }, + { + label: "pendingPermissions size", + over: { pendingPermissions: new Map([["p", {}]]) }, + }, + { + label: "cloudOutput.pr_url", + over: { cloudOutput: { pr_url: "https://x/pr/1" } }, + }, + ])("changes when $label changes", ({ over }) => { + const before = computeSidebarSessionSignature({ r1: session() }); + expect(computeSidebarSessionSignature({ r1: session(over) })).not.toBe( + before, + ); + }); + + it("skips sessions without a taskId", () => { + expect( + computeSidebarSessionSignature({ r1: session({ taskId: "" }) }), + ).toBe(""); + }); +}); diff --git a/packages/ui/src/features/sessions/sessionStore.ts b/packages/ui/src/features/sessions/sessionStore.ts index 7d91221d6c..20ae6f98b0 100644 --- a/packages/ui/src/features/sessions/sessionStore.ts +++ b/packages/ui/src/features/sessions/sessionStore.ts @@ -75,6 +75,7 @@ export { useQueuedMessagesForTask, useSessionForTask, useSessions, + useSidebarSessionMap, useThoughtLevelConfigOptionForTask, } from "./useSession"; diff --git a/packages/ui/src/features/sessions/useSession.ts b/packages/ui/src/features/sessions/useSession.ts index 0352ee1095..960bf3ff21 100644 --- a/packages/ui/src/features/sessions/useSession.ts +++ b/packages/ui/src/features/sessions/useSession.ts @@ -6,7 +6,9 @@ import { extractAvailableCommandsFromEvents, extractUserPromptsFromEvents, } from "@posthog/core/sessions/sessionEvents"; +import { computeSidebarSessionSignature } from "@posthog/core/sidebar/buildSidebarData"; import type { PermissionRequest } from "@posthog/ui/features/sessions/sessionLogTypes"; +import { useMemo } from "react"; import { shallow } from "zustand/shallow"; import { type Adapter, @@ -19,6 +21,29 @@ import { export const useSessions = () => useSessionStore((s) => s.sessions); +/** + * The sidebar's view of sessions, keyed by taskId. Subscribes only to a + * signature of the fields the sidebar reads (see computeSidebarSessionSignature), + * so streaming token appends — which only mutate `events` — don't re-render the + * sidebar (which is mounted at the root). The map is rebuilt from the live + * snapshot only when a sidebar-relevant field actually changes. + */ +export const useSidebarSessionMap = (): Map => { + const signature = useSessionStore((s) => + computeSidebarSessionSignature(s.sessions), + ); + // `signature` is the trigger, not read inside: rebuild the map from the live + // snapshot only when a sidebar-relevant field changes (not on every token). + // biome-ignore lint/correctness/useExhaustiveDependencies: keyed by signature on purpose + return useMemo(() => { + const map = new Map(); + for (const session of Object.values(useSessionStore.getState().sessions)) { + if (session.taskId) map.set(session.taskId, session); + } + return map; + }, [signature]); +}; + /** O(1) lookup using taskIdIndex */ export const useSessionForTask = ( taskId: string | undefined, diff --git a/packages/ui/src/features/sessions/useSidebarSessionMap.test.tsx b/packages/ui/src/features/sessions/useSidebarSessionMap.test.tsx new file mode 100644 index 0000000000..e2bd300b82 --- /dev/null +++ b/packages/ui/src/features/sessions/useSidebarSessionMap.test.tsx @@ -0,0 +1,62 @@ +import type { AcpMessage, AgentSession } from "@posthog/shared"; +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it } from "vitest"; +import { sessionStoreSetters, useSessionStore } from "./sessionStore"; +import { useSessions, useSidebarSessionMap } from "./useSession"; + +function makeSession(taskId: string, taskRunId: string): AgentSession { + return { + taskId, + taskRunId, + events: [], + isPromptPending: false, + pendingPermissions: new Map(), + } as unknown as AgentSession; +} + +const TOKEN = {} as AcpMessage; + +/** Mount a hook and count how many times it (re)renders. */ +function countRenders(hook: () => T): () => number { + let n = 0; + renderHook(() => { + n++; + return hook(); + }); + return () => n; +} + +describe("sidebar session subscription — re-render cost during streaming", () => { + beforeEach(() => { + useSessionStore.setState({ sessions: {}, taskIdIndex: {} }); + sessionStoreSetters.setSession(makeSession("t1", "r1")); + }); + + it("baseline: useSessions() re-renders on every streamed token", () => { + const renders = countRenders(() => useSessions()); + const before = renders(); + for (let i = 0; i < 20; i++) { + act(() => sessionStoreSetters.appendEvents("r1", [TOKEN])); + } + // Old behaviour: one re-render per token (and the sidebar is at the root). + expect(renders() - before).toBe(20); + }); + + it("fixed: useSidebarSessionMap() ignores streamed tokens (0 re-renders)", () => { + const renders = countRenders(() => useSidebarSessionMap()); + const before = renders(); + for (let i = 0; i < 20; i++) { + act(() => sessionStoreSetters.appendEvents("r1", [TOKEN])); + } + expect(renders() - before).toBe(0); + }); + + it("fixed: still re-renders when a sidebar-relevant field changes", () => { + const renders = countRenders(() => useSidebarSessionMap()); + const before = renders(); + act(() => + sessionStoreSetters.updateSession("r1", { isPromptPending: true }), + ); + expect(renders() - before).toBe(1); + }); +}); diff --git a/packages/ui/src/features/sidebar/useSidebarData.ts b/packages/ui/src/features/sidebar/useSidebarData.ts index a040204883..f6c01e0784 100644 --- a/packages/ui/src/features/sidebar/useSidebarData.ts +++ b/packages/ui/src/features/sidebar/useSidebarData.ts @@ -18,7 +18,7 @@ import type { AppView } from "@posthog/ui/router/useAppView"; import { useEffect, useMemo, useRef } from "react"; import { useArchivedTaskIds } from "../archive/useArchivedTaskIds"; import { useProvisioningStore } from "../provisioning/store"; -import { useSessions } from "../sessions/sessionStore"; +import { useSidebarSessionMap } from "../sessions/sessionStore"; import { useSuspendedTaskIds } from "../suspension/useSuspendedTaskIds"; import { useSlackTasks, useTaskSummaries, useTasks } from "../tasks/useTasks"; import { useWorkspaces } from "../workspace/useWorkspace"; @@ -41,7 +41,6 @@ export function useSidebarData({ const archivedTaskIds = useArchivedTaskIds(); const suspendedTaskIds = useSuspendedTaskIds(); const provisioningTaskIds = useProvisioningStore((s) => s.activeTasks); - const sessions = useSessions(); const { timestamps } = useTaskViewed(); const historyVisibleCount = useSidebarStore( (state) => state.historyVisibleCount, @@ -140,15 +139,7 @@ export function useSidebarData({ const activeTaskId = activeView.type === "task-detail" ? (activeView.taskId ?? null) : null; - const sessionByTaskId = useMemo(() => { - const map = new Map(); - for (const session of Object.values(sessions)) { - if (session.taskId) { - map.set(session.taskId, session); - } - } - return map; - }, [sessions]); + const sessionByTaskId = useSidebarSessionMap(); const taskData = useMemo( () =>