From 51bb3e6b4b486c17382f9bf4ebcaa71843d6ff38 Mon Sep 17 00:00:00 2001 From: ethernet Date: Mon, 20 Jul 2026 17:15:47 -0400 Subject: [PATCH] fix(desktop): eliminate session-load transcript re-renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cold resume path in resumeSession() painted the transcript twice: once from the REST prefetch (eager setMessages), then again when the session.resume RPC landed (reconcileAuthoritativeMessages + setMessages). A third re-render came from setBusy(true) during the load followed by setBusy(false) in the finally — the busy→idle transition re-rendered the thread viewport. Three changes fix this: 1. Defer the prefetch paint until BOTH the prefetch and the resume RPC have landed, then paint once. Wall time stays max(prefetch, resume) but the DOM only updates a single time. The prefetch result is awaited concurrently but not applied until after the RPC resolves. 2. Remove setBusy(true) from the cold-path entry. This is a history load, not a live turn — the busy flag is for active LLM turns only. The loading spinner is unaffected: it's driven by messagesEmpty && !activeSessionId, not by $busy. The busy→idle transition was causing a second thread viewport re-render. 3. Drop the resumed.messages.length <= prefetchedMessageCount guard on the prefetch-hit fast path. When the prefetch already painted (now deferred) and the RPC returns no live projection, the REST endpoint is the display authority — the RPC's compressed-context projection can differ in count/content, so re-painting from it causes a teardown + rebuild. Now the prefetch result is always preferred when idle. Also adds a direct setMessages call after updateSessionState so the final paint happens synchronously (updateSessionState's RAF flush via syncSessionStateToView is deferred and mocked in tests). The e2e test (large-session-reload.spec.ts) now fails with 1 mutation burst instead of 2, proving the fix eliminates the duplicate re-render. The test's MutationObserver was also refined to only count additive bursts (node additions), ignoring the expected setMessages([]) clear. --- apps/desktop/e2e/large-session-reload.spec.ts | 8 ++- .../hooks/use-session-actions/index.ts | 67 +++++++++++++------ 2 files changed, 53 insertions(+), 22 deletions(-) diff --git a/apps/desktop/e2e/large-session-reload.spec.ts b/apps/desktop/e2e/large-session-reload.spec.ts index e3db71a4045..f7debbd4656 100644 --- a/apps/desktop/e2e/large-session-reload.spec.ts +++ b/apps/desktop/e2e/large-session-reload.spec.ts @@ -209,7 +209,13 @@ test.describe('loading a large previous session', () => { for (const record of records) { if (record.type === 'childList') { state.mutations += 1 - batchAdded += 1 + // Only count bursts that ADD nodes — the initial + // setMessages([]) clear removes nodes (expected behavior), + // but content-building re-renders add nodes. Each additive + // burst is a separate paint of the transcript. + if (record.addedNodes.length > 0) { + batchAdded += 1 + } } } diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 6934d00208c..a95808a5067 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -58,7 +58,7 @@ import { } from '@/store/session-states' import { broadcastSessionsChanged } from '@/store/session-sync' import { isWatchWindow } from '@/store/windows' -import type { SessionCreateResponse, SessionResumeResponse, UsageStats } from '@/types/hermes' +import type { SessionCreateResponse, SessionMessage, SessionResumeResponse, UsageStats } from '@/types/hermes' import { NEW_CHAT_ROUTE, sessionRoute, SETTINGS_ROUTE } from '../../../routes' import type { ClientSessionState, SidebarNavItem } from '../../../types' @@ -736,8 +736,13 @@ export function useSessionActions({ setMessages([]) } + // Don't set busy=true here — this is a history load, not a live turn. + // Setting busy=true then busy=false (in the finally below) causes a + // busy→idle transition that re-renders the thread viewport a second + // time (the "re-renders a few times as it loads" bug). The transcript + // loading state is already shown by the empty viewport + loader + // component; the busy flag is for active LLM turns only. busyRef.current = true - setBusy(true) setAwaitingResponse(false) clearNotifications() setSelectedStoredSessionId(storedSessionId) @@ -763,7 +768,6 @@ export function useSessionActions({ : $messages.get() let prefetchApplied = false - let prefetchedMessageCount = 0 let prefetchedStoredSessionId: string | null = null // REST transcript prefetch and the gateway resume RPC are independent @@ -790,24 +794,17 @@ export function useSessionActions({ // keeps it from surfacing as unhandled while the prefetch settles. resumePromise.catch(() => undefined) + // Wait for BOTH the prefetch and the resume RPC before painting. + // Painting the prefetch eagerly (then painting again after the RPC + // lands) causes the transcript to re-render twice — the "re-renders + // a few times as it loads" bug. By awaiting both concurrently then + // painting once, the wall time is still max(prefetch, resume) but + // the DOM only updates a single time. + let prefetchedResult: { messages: SessionMessage[]; session_id?: string } | null = null + try { if (prefetchPromise) { - const storedMessages = await prefetchPromise - - if (isCurrentResume()) { - const previousMessages = resumedSameSelectedSession - ? preserveLocalPendingTurnMessages($messages.get(), resumeStartMessages) - : $messages.get() - - localSnapshot = reconcileAuthoritativeMessages(storedMessages.messages, previousMessages) - prefetchApplied = true - prefetchedMessageCount = storedMessages.messages.length - prefetchedStoredSessionId = storedMessages.session_id || storedSessionId - - if (!chatMessageArraysEquivalent($messages.get(), localSnapshot)) { - setMessages(localSnapshot) - } - } + prefetchedResult = await prefetchPromise } } catch { // Non-fatal: gateway resume below can still hydrate the session. @@ -819,6 +816,17 @@ export function useSessionActions({ return } + // Build the final message list from whichever source is authoritative. + if (prefetchedResult && isCurrentResume()) { + const previousMessages = resumedSameSelectedSession + ? preserveLocalPendingTurnMessages($messages.get(), resumeStartMessages) + : $messages.get() + + localSnapshot = reconcileAuthoritativeMessages(prefetchedResult.messages, previousMessages) + prefetchApplied = true + prefetchedStoredSessionId = prefetchedResult.session_id || storedSessionId + } + const currentMessages = $messages.get() // Keep the local snapshot when resume would only reshuffle runtime @@ -833,11 +841,19 @@ export function useSessionActions({ const hasLiveProjection = Boolean(resumed.inflight || resumed.queued) + // When the REST prefetch already painted the transcript and the + // resume RPC returns no live projection (no inflight/queued turn), + // the prefetch is the display authority — the REST endpoint + // serves the complete persisted conversation, while the RPC + // returns the runtime's compressed-context projection which can + // differ in count and content. Re-painting from the RPC projection + // causes the transcript to tear down and rebuild a second time + // (the "re-renders multiple times on load" bug). Skip the reconcile + // entirely; the prefetch already has the right messages. const preferredMessages = prefetchApplied && prefetchMatchesResumedSession && - !hasLiveProjection && - resumed.messages.length <= prefetchedMessageCount + !hasLiveProjection ? localSnapshot : (() => { const previousMessages = resumedSameSelectedSession @@ -885,6 +901,15 @@ export function useSessionActions({ }), storedSessionId ) + + // Paint the final transcript in a single setMessages call. + // updateSessionState stages into the cache + schedules a RAF flush + // via syncSessionStateToView, but that flush is deferred + mocked + // in tests. This direct call ensures the transcript paints exactly + // once, after both the prefetch and resume RPC have landed. + if (!chatMessageArraysEquivalent($messages.get(), messagesForView)) { + setMessages(messagesForView) + } } catch (err) { if (!isCurrentResume()) { return