From 59aaf306bd7ab7b8b51bbbaf390974b7a6e0129e Mon Sep 17 00:00:00 2001 From: "(CJ) Chukwudi Nwobodo" <142016413+chuks-qua@users.noreply.github.com> Date: Thu, 24 Sep 2026 15:43:31 +0100 Subject: [PATCH 1/3] fix(web): commit cold thread activations with narrative and pin the reading row Cold activation committed a narrative-free tail page before the richer conversation page resolved, so the response painted first and reasoning popped in above it. Fetch the page concurrently and commit once; the tail remains a failure fallback. Keep loaded narrative resident instead of evicting on row unmount, and warm the tail's narrative on activation. Also pin the reading row's screen position captured at user-scroll time. Resolving the anchor through provisional heights drifted on every measurement pass after a prepend, sliding the viewport upward. --- .../threadStore-message-cache.test.ts | 32 +++--- .../web/src/components/ui/virtual-viewport.ts | 48 +++++++- .../__tests__/thread-hydrator.test.ts | 106 ++++++++++++++---- .../conversation/hydration/thread-hydrator.ts | 62 ++++++++-- .../features/conversation/hydration/types.ts | 2 +- .../messages/TranscriptNarrativeRow.tsx | 6 - .../__tests__/transcript-viewport.test.ts | 22 ++++ apps/web/src/stores/threadStore.ts | 69 +++--------- 8 files changed, 238 insertions(+), 109 deletions(-) diff --git a/apps/web/src/__tests__/threadStore-message-cache.test.ts b/apps/web/src/__tests__/threadStore-message-cache.test.ts index b35447d91..5a188556d 100644 --- a/apps/web/src/__tests__/threadStore-message-cache.test.ts +++ b/apps/web/src/__tests__/threadStore-message-cache.test.ts @@ -110,7 +110,8 @@ await activateTestConversation("t1"); ] satisfies NarrativeEntry[]); await activateTestConversation("t1"); - expect(useThreadStore.getState().isNarrativeLoaded("t1", assistant.id)).toBe(false); + // Activation prefetches tail detail, so the window is already resident. + expect(useThreadStore.getState().isNarrativeLoaded("t1", assistant.id)).toBe(true); await useThreadStore.getState().loadNarrativeForMessage(assistant.id, "t1"); @@ -166,8 +167,8 @@ await activateTestConversation("t1"); ] satisfies NarrativeEntry[]); await activateTestConversation("t1"); - await useThreadStore.getState().loadNarrativeForMessage(assistant.id, "t1"); + // The activation prefetch consumes the first window and leaves the cursor. expect(mockTransport.loadTurn).toHaveBeenCalledTimes(1); expect(useThreadStore.getState().isNarrativeLoaded("t1", assistant.id)).toBe(false); @@ -300,7 +301,7 @@ await activateTestConversation("t1"); .mockResolvedValueOnce(tooLargeWindow); await activateTestConversation("t1"); - await useThreadStore.getState().loadNarrativeForMessage(assistant.id, "t1"); + // The activation prefetch consumed the first window; this requests the next. await useThreadStore.getState().loadNarrativeForMessage(assistant.id, "t1"); const record = getThreadRecord(useThreadStore.getState().records, "t1"); @@ -368,8 +369,8 @@ await activateTestConversation("t1"); expect(useThreadStore.getState().isNarrativeLoaded("t1", assistant.id)).toBe(false); }); - it("keeps detail through a virtual-row handoff and evicts after the final row releases", async () => { - const assistant = createMockMessage({ id: "assistant-lease", thread_id: "t1", role: "assistant" }); + it("retains loaded detail across a thread switch round trip", async () => { + const assistant = createMockMessage({ id: "assistant-retain", thread_id: "t1", role: "assistant" }); (mockTransport.loadConversationPage as ReturnType).mockResolvedValue({ messages: [assistant], hasMore: false, narrativeByMessage: {}, }); @@ -379,18 +380,19 @@ await activateTestConversation("t1"); await activateTestConversation("t1"); await useThreadStore.getState().loadNarrativeForMessage(assistant.id, "t1"); - useThreadStore.getState().retainNarrativeForMessage(assistant.id, "t1"); - useThreadStore.getState().retainNarrativeForMessage(assistant.id, "t1"); - useThreadStore.getState().releaseNarrativeForMessage(assistant.id, "t1"); - await Promise.resolve(); - - expect(getThreadRecord(useThreadStore.getState().records, "t1").narrativeByMessage[assistant.id]).toBeDefined(); + expect(useThreadStore.getState().isNarrativeLoaded("t1", assistant.id)).toBe(true); + const detailCalls = vi.mocked(mockTransport.loadTurn).mock.calls.length; - useThreadStore.getState().releaseNarrativeForMessage(assistant.id, "t1"); - await Promise.resolve(); + useThreadStore.setState((s) => ({ + currentThreadId: "t2", + records: patchThreadRecord(s.records, "t2", { messages: [] }), + })); + await activateTestConversation("t1"); - expect(getThreadRecord(useThreadStore.getState().records, "t1").narrativeByMessage[assistant.id]).toBeUndefined(); - expect(useThreadStore.getState().isNarrativeLoaded("t1", assistant.id)).toBe(false); + const record = getThreadRecord(useThreadStore.getState().records, "t1"); + expect(record.narrativeByMessage[assistant.id]).toBeDefined(); + expect(useThreadStore.getState().isNarrativeLoaded("t1", assistant.id)).toBe(true); + expect(vi.mocked(mockTransport.loadTurn).mock.calls.length).toBe(detailCalls); }); it("on cache hit, does not call conversation.page and renders from cache", async () => { diff --git a/apps/web/src/components/ui/virtual-viewport.ts b/apps/web/src/components/ui/virtual-viewport.ts index fbb59f8f9..9294d6fb6 100644 --- a/apps/web/src/components/ui/virtual-viewport.ts +++ b/apps/web/src/components/ui/virtual-viewport.ts @@ -43,6 +43,7 @@ export class VirtualViewport { private expectedScrollTop = 0; private disposed = false; private animating = false; + private pinnedAnchor: { key: string; top: number } | undefined; constructor( container: HTMLElement, @@ -84,7 +85,7 @@ export class VirtualViewport { }, hooks: { onCommit: () => this.publishVisible(), - onResize: () => this.applyPosition(), + onResize: () => this.applyPosition(true), }, }]); this.viewport = this.context.dom.viewport; @@ -146,7 +147,7 @@ export class VirtualViewport { if (!this.rowIndexes.has(key)) this.heights.delete(key); } this.list.setItems([...this.rows]); - this.applyPosition(); + this.applyPosition(true); if (restoreFocus && focused.isConnected && document.activeElement !== focused) { focused.focus({ preventScroll: true }); } @@ -179,7 +180,7 @@ export class VirtualViewport { if (this.disposed) return; this.context.rebuildSizeCache(); this.context.updateContentSize(this.context.sizeCache.getTotalSize()); - this.applyPosition(); + this.applyPosition(true); }); } @@ -192,20 +193,44 @@ export class VirtualViewport { return true; } - private applyPosition(): void { + private applyPosition(correctAnchor = false): void { if (this.disposed || !this.viewport) return; + const anchor = correctAnchor ? this.readingAnchorHost() : undefined; const target = Math.max(0, this.targetScrollTop()); // A short initial page needs its trailing space retained when history is prepended. this.context.updateContentSize(Math.max( this.context.sizeCache.getTotalSize(), this.position.kind === "reading" ? target + this.viewport.clientHeight : 0, )); - if (!this.animating) this.context.scrollTo(target); + if (!this.animating) { + this.context.scrollTo(target); + this.correctAnchorDrift(anchor); + } this.expectedScrollTop = this.viewport.scrollTop; this.context.forceRender(); this.onPosition(this.position); } + /** The reading row's live host plus the screen position it must keep. */ + private readingAnchorHost(): { host: HTMLDivElement; top: number } | undefined { + if (this.position.kind !== "reading") return undefined; + const host = this.hosts.get(this.position.key); + // Content changes push the anchored row between measurement passes, so the + // pin must come from the user's last scroll, not the current drifted rect. + const pinned = this.pinnedAnchor; + const top = pinned?.key === this.position.key + ? pinned.top + : host?.isConnected ? host.getBoundingClientRect().top : undefined; + return host && top !== undefined ? { host, top } : undefined; + } + + /** Sizes resolve through provisional heights; the host's real rect is truth. */ + private correctAnchorDrift(anchor: { host: HTMLDivElement; top: number } | undefined): void { + if (!anchor?.host.isConnected) return; + const drift = anchor.host.getBoundingClientRect().top - anchor.top; + if (drift !== 0) this.viewport.scrollTop += drift; + } + private targetScrollTop(): number { const cache = this.context.sizeCache; let top = this.viewport.scrollTop; @@ -240,6 +265,7 @@ export class VirtualViewport { this.context.cancelScroll(); this.animating = false; this.position = this.readingPosition(); + this.pinReadingAnchor(); this.onPosition(this.position); }; @@ -264,10 +290,18 @@ export class VirtualViewport { const atEnd = this.viewport.scrollHeight - this.viewport.clientHeight - top <= 2; const anchor = this.readingPosition(); this.position = this.options.positionOnScroll?.(anchor, atEnd) ?? anchor; + this.pinReadingAnchor(); this.expectedScrollTop = top; this.onPosition(this.position); }; + private pinReadingAnchor(): void { + const host = this.position.kind === "reading" ? this.hosts.get(this.position.key) : undefined; + this.pinnedAnchor = this.position.kind === "reading" && host?.isConnected + ? { key: this.position.key, top: host.getBoundingClientRect().top } + : undefined; + } + /** Captures the first visible row independently of React's commit timing. */ getReadingAnchor(): { key: string; offset: number } | undefined { const position = this.readingPosition(); @@ -283,9 +317,11 @@ export class VirtualViewport { this.context.smoothScrollTo(() => Math.max(0, this.targetScrollTop()), 250, undefined, () => { this.animating = false; this.applyPosition(); + this.pinReadingAnchor(); }); } this.applyPosition(); + this.pinReadingAnchor(); } /** Restores an absolute offset and captures its row anchor. */ @@ -293,12 +329,14 @@ export class VirtualViewport { this.context.scrollTo(top); this.position = this.readingPosition(); this.applyPosition(); + this.pinReadingAnchor(); } /** Compensates for a layout inset changing the viewport's screen position. */ shiftReadingPosition(delta: number): void { if (this.position.kind !== "reading" || delta === 0) return; this.position = { ...this.position, offset: this.position.offset + delta }; + if (this.pinnedAnchor) this.pinnedAnchor = { ...this.pinnedAnchor, top: this.pinnedAnchor.top + delta }; this.applyPosition(); } diff --git a/apps/web/src/features/conversation/hydration/__tests__/thread-hydrator.test.ts b/apps/web/src/features/conversation/hydration/__tests__/thread-hydrator.test.ts index 04b19be06..ad9a79e77 100644 --- a/apps/web/src/features/conversation/hydration/__tests__/thread-hydrator.test.ts +++ b/apps/web/src/features/conversation/hydration/__tests__/thread-hydrator.test.ts @@ -14,7 +14,7 @@ import { * Asserts on store state after hydrate(), not internal sub-module calls. */ import { describe, it, expect, beforeEach, vi } from "vitest"; -import { useThreadStore, extractPendingPlanQuestions } from "@/stores/threadStore"; +import { useThreadStore, extractPendingPlanQuestions, HISTORY_PAGE_SIZE } from "@/stores/threadStore"; import { useWorkspaceStore } from "@/features/projects/state/workspaceStore"; import { useTaskStore } from "@/stores/taskStore"; import { usePlanStore } from "@/stores/planStore"; @@ -41,7 +41,7 @@ import { coerceTaskStatus } from "@/stores/taskStore"; import { getTransport } from "@/transport"; import type { Message } from "@/transport"; import { PERMISSION_MODES, INTERACTION_MODES } from "@mcode/contracts"; -import type { ConversationPage, ConversationTail, GoalLookupResult, GoalState, TurnSnapshot } from "@mcode/contracts"; +import type { ConversationPage, GoalLookupResult, GoalState, TurnSnapshot } from "@mcode/contracts"; import { CONVERSATION_OLDER_PAGE_MAX_BYTES } from "@mcode/contracts"; import { clearScrollMemory, rememberScrollTop } from "@/components/chat/scrollPositionMemory"; @@ -93,8 +93,8 @@ function createStoreHydrator(): ThreadHydrator { getWorkspaceThread: (threadId) => useWorkspaceStore.getState().threads.find((t) => t.id === threadId), flushPendingTextDeltas: () => {}, - loadNarrativeForMessage: (messageId) => - useThreadStore.getState().loadNarrativeForMessage(messageId), + loadNarrativeForMessage: (messageId, threadId) => + useThreadStore.getState().loadNarrativeForMessage(messageId, threadId), setPlanQuestions: (threadId, questions) => useThreadStore.getState().setPlanQuestions(threadId, questions), extractPendingPlanQuestions, @@ -485,6 +485,47 @@ describe("ThreadHydrator", () => { expect(getCachedRecord(THREAD_A)?.messages).toEqual([msgA]); }); + it("warms tail narrative detail for assistant messages during activation", async () => { + const assistant = createMockMessage({ id: "a-assistant", thread_id: THREAD_A, role: "assistant", sequence: 2 }); + (mockTransport.loadConversationPage as ReturnType).mockResolvedValue({ + messages: [msgA, assistant], + hasMore: false, + narrativeByMessage: {}, + }); + (mockTransport.loadTurn as ReturnType).mockResolvedValue([ + { kind: "assistantMessage", messageId: assistant.id, sequence: 2, body: "done", sortOrder: 0 }, + ]); + + await hydrator.hydrate(THREAD_A, "active"); + + await vi.waitFor(() => { + expect(mockTransport.loadTurn).toHaveBeenCalledWith(THREAD_A, { + limit: 1, + before: assistant.sequence + 1, + detail: { limit: 100 }, + }); + }); + expect(useThreadStore.getState().isNarrativeLoaded(THREAD_A, assistant.id)).toBe(true); + }); + + it("skips tail narrative prefetch while the thread is running", async () => { + const assistant = createMockMessage({ id: "a-running-assistant", thread_id: THREAD_A, role: "assistant", sequence: 2 }); + (mockTransport.loadConversationPage as ReturnType).mockResolvedValue({ + messages: [msgA, assistant], + hasMore: false, + narrativeByMessage: {}, + }); + useThreadStore.setState({ runningThreadIds: new Set([THREAD_A]) }); + + await hydrator.hydrate(THREAD_A, "active"); + await vi.waitFor(() => { + expect(readActiveThreadField((record) => record.loading)).toBe(false); + }); + + expect(mockTransport.loadTurn).not.toHaveBeenCalled(); + expect(useThreadStore.getState().isNarrativeLoaded(THREAD_A, assistant.id)).toBe(false); + }); + it("does not let delayed snapshot hydration replace a running turn file summary", async () => { const liveSummary = { revision: 2, @@ -672,7 +713,7 @@ describe("ThreadHydrator", () => { direction: "older", generation: 4, conversationRevision: expect.any(Number), - limit: 50, + limit: 25, maxBytes: CONVERSATION_OLDER_PAGE_MAX_BYTES, }); @@ -762,7 +803,8 @@ describe("ThreadHydrator", () => { }, ); - for (let index = 0; index < 4; index++) { + const pageCount = 200 / HISTORY_PAGE_SIZE; + for (let index = 0; index < pageCount; index++) { await useThreadStore.getState().loadOlderMessages(THREAD_A); } @@ -771,7 +813,7 @@ describe("ThreadHydrator", () => { ); expect(readActiveThreadField((record) => record.hasNewerMessages)).toBe(true); - for (let index = 0; index < 4; index++) { + for (let index = 0; index < pageCount; index++) { await useThreadStore.getState().loadNewerMessages(THREAD_A); } @@ -1836,6 +1878,12 @@ describe("ThreadHydrator", () => { })); const tailLoader = vi.fn().mockResolvedValue({ messages: [msgA], sessionNotices, hasMore: true }); mockTransport.loadConversationTail = tailLoader; + vi.mocked(mockTransport.loadConversationPage).mockResolvedValue({ + messages: [msgA], + sessionNotices, + hasMore: true, + narrativeByMessage: {}, + }); try { await hydrator.hydrate(THREAD_A, "active"); @@ -1865,18 +1913,19 @@ describe("ThreadHydrator", () => { noticeKey: "fresh-warning", }, }); - let resolveTail!: (tail: ConversationTail) => void; - mockTransport.loadConversationTail = vi.fn().mockImplementation( - () => new Promise((resolve) => { resolveTail = resolve; }), + let resolvePage!: (page: ConversationPage) => void; + mockTransport.loadConversationTail = vi.fn().mockResolvedValue({ messages: [msgA], sessionNotices: [], hasMore: false }); + vi.mocked(mockTransport.loadConversationPage).mockImplementation( + () => new Promise((resolve) => { resolvePage = resolve; }), ); try { const hydration = hydrator.hydrate(THREAD_A, "active"); - await vi.waitFor(() => expect(mockTransport.loadConversationTail).toHaveBeenCalledTimes(1)); + await vi.waitFor(() => expect(mockTransport.loadConversationPage).toHaveBeenCalled()); useThreadStore.setState((state) => ({ records: patchThreadRecord(state.records, THREAD_A, { sessionNotices: [] }), })); - resolveTail({ messages: [msgA], sessionNotices: [notice], hasMore: false }); + resolvePage({ messages: [msgA], sessionNotices: [notice], hasMore: false, narrativeByMessage: {} }); await hydration; expect(useThreadStore.getState().records.get(THREAD_A)?.sessionNotices).toEqual([notice]); @@ -1901,14 +1950,15 @@ describe("ThreadHydrator", () => { noticeKey: "security-a", }, }); - let resolveTail!: (tail: ConversationTail) => void; - mockTransport.loadConversationTail = vi.fn().mockImplementation( - () => new Promise((resolve) => { resolveTail = resolve; }), + let resolvePage!: (page: ConversationPage) => void; + mockTransport.loadConversationTail = vi.fn().mockResolvedValue({ messages: [msgA], sessionNotices: [], hasMore: false }); + vi.mocked(mockTransport.loadConversationPage).mockImplementation( + () => new Promise((resolve) => { resolvePage = resolve; }), ); try { const hydration = hydrator.hydrate(THREAD_A, "active"); - await vi.waitFor(() => expect(mockTransport.loadConversationTail).toHaveBeenCalledTimes(1)); + await vi.waitFor(() => expect(mockTransport.loadConversationPage).toHaveBeenCalled()); useThreadStore.getState().handleAgentEvent({ type: "system", threadId: THREAD_A, @@ -1923,7 +1973,7 @@ describe("ThreadHydrator", () => { noticeKey: "warning-b", }, }); - resolveTail({ messages: [msgA], sessionNotices: [securityNotice], hasMore: false }); + resolvePage({ messages: [msgA], sessionNotices: [securityNotice], hasMore: false, narrativeByMessage: {} }); await hydration; expect(useThreadStore.getState().records.get(THREAD_A)?.sessionNotices.map((notice) => notice.id)) @@ -1961,18 +2011,19 @@ describe("ThreadHydrator", () => { sessionNotices: [fetchedNotice], }), })); - let resolveTail!: (tail: ConversationTail) => void; - mockTransport.loadConversationTail = vi.fn().mockImplementation( - () => new Promise((resolve) => { resolveTail = resolve; }), + let resolvePage!: (page: ConversationPage) => void; + mockTransport.loadConversationTail = vi.fn().mockResolvedValue({ messages: [msgA], sessionNotices: [], hasMore: false }); + vi.mocked(mockTransport.loadConversationPage).mockImplementation( + () => new Promise((resolve) => { resolvePage = resolve; }), ); try { const hydration = hydrator.hydrate(THREAD_A, "active"); - await vi.waitFor(() => expect(mockTransport.loadConversationTail).toHaveBeenCalledTimes(1)); + await vi.waitFor(() => expect(mockTransport.loadConversationPage).toHaveBeenCalled()); useThreadStore.setState((state) => ({ records: patchThreadRecord(state.records, THREAD_A, { sessionNotices: [liveNotice] }), })); - resolveTail({ messages: [msgA], sessionNotices: [fetchedNotice], hasMore: false }); + resolvePage({ messages: [msgA], sessionNotices: [fetchedNotice], hasMore: false, narrativeByMessage: {} }); await hydration; expect(useThreadStore.getState().records.get(THREAD_A)?.sessionNotices).toEqual([liveNotice]); @@ -2012,6 +2063,12 @@ describe("ThreadHydrator", () => { sessionNotices: [notice], hasMore: false, }); + vi.mocked(mockTransport.loadConversationPage).mockResolvedValue({ + messages: [msgA], + sessionNotices: [notice], + hasMore: false, + narrativeByMessage: {}, + }); try { await hydrator.hydrate(THREAD_A, "active"); @@ -2078,6 +2135,11 @@ describe("ThreadHydrator", () => { delete tailMessage.files_changed; const tailLoader = vi.fn().mockResolvedValue({ messages: [tailMessage], hasMore: false }); mockTransport.loadConversationTail = tailLoader; + vi.mocked(mockTransport.loadConversationPage).mockResolvedValue({ + messages: [tailMessage], + hasMore: false, + narrativeByMessage: {}, + }); try { await hydrator.hydrate(THREAD_A, "active"); diff --git a/apps/web/src/features/conversation/hydration/thread-hydrator.ts b/apps/web/src/features/conversation/hydration/thread-hydrator.ts index 4f8440cf4..c37b651a9 100644 --- a/apps/web/src/features/conversation/hydration/thread-hydrator.ts +++ b/apps/web/src/features/conversation/hydration/thread-hydrator.ts @@ -304,6 +304,9 @@ export const MESSAGE_FETCH_SIZE = 2; /** Maximum messages retained across the tail and warmed older-history page. */ export const BACKGROUND_PREFETCH_LIMIT = 100; +/** Tail messages whose persisted narrative detail warms before their rows mount. */ +const NARRATIVE_PREFETCH_TAIL_MESSAGES = 10; + /** Older messages warmed after first paint and held outside live React state. */ export const HISTORY_PREFETCH_SIZE = BACKGROUND_PREFETCH_LIMIT - MESSAGE_FETCH_SIZE; @@ -384,6 +387,7 @@ export class ThreadHydrator { const resident = this.deps.getState().records.get(threadId); if (resident && hasResidentContent(resident) && hasCommittedWindow(resident) && !opts.force) { this.synchronizeConversation(threadId); + this.prefetchVisibleNarrative(threadId); return true; } const cached = getCachedRecord(threadId); @@ -394,6 +398,7 @@ export class ThreadHydrator { select: false, }); this.synchronizeConversation(threadId); + this.prefetchVisibleNarrative(threadId); return true; } @@ -502,7 +507,10 @@ export class ThreadHydrator { }; }); const committed = this.deps.getState().records.get(threadId); - if (committed && opts.isCurrent()) this.synchronizeConversation(threadId); + if (committed && opts.isCurrent()) { + this.synchronizeConversation(threadId); + this.prefetchVisibleNarrative(threadId); + } } catch (error) { if (!opts.isCurrent()) return; this.deps.setState((state: ThreadHydratorWriteState) => { @@ -629,6 +637,28 @@ export class ThreadHydrator { if (record) cacheRecord(threadId, projectConversationCacheState(record)); } + /** + * Warm persisted narrative detail for the tail a display is about to paint. + * Without this, a thread switch renders each assistant response first and + * pops its reasoning/tool rows in above it once the visible-row fetch lands. + * Running threads are skipped: their newest turn's detail is still being + * persisted and the volatile timeline already covers it. + */ + private prefetchVisibleNarrative(threadId: string): void { + const state = this.deps.getState(); + const isDisplayed = + state.currentThreadId === threadId + || this.deps.isDisplayConversationVisible?.(threadId) === true; + if (!isDisplayed || state.runningThreadIds.has(threadId)) return; + const record = state.records.get(threadId); + if (!record) return; + for (const message of record.messages.slice(-NARRATIVE_PREFETCH_TAIL_MESSAGES)) { + if (message.role !== "assistant" || message.is_internal) continue; + if (record.narrativeByMessage[message.id]) continue; + void this.deps.loadNarrativeForMessage(message.id, threadId); + } + } + /** Merge delayed file metadata only when the current cache still retains those messages. */ mergeCachedFileChanges(threadId: string, filesChanged: Record): void { const cached = getCachedRecord(threadId); @@ -985,6 +1015,7 @@ export class ThreadHydrator { ) { this.scheduleEarlierHistoryPrefetch(threadId, renderableCachedRecord); } + this.prefetchVisibleNarrative(threadId); } /** Run auxiliary fanout after the first paint, unless this selection is superseded. */ @@ -1256,6 +1287,9 @@ export class ThreadHydrator { if (!this.fetchCommitContextMatches(threadId, context)) return this.settleDiscardedFetch(threadId, context); this.commitFetchedConversation(threadId, loaded.page, context); recordThreadCommit(threadId, "network-fetch"); + // Fires concurrently with the tail followup so detail lands as close to + // first paint as the transport allows. + this.prefetchVisibleNarrative(threadId); this.scheduleFetchedConversationAuxiliaries(threadId, opts, context.epoch); const tailFollowupInvalidated = loaded.usedTail && await this.hydrateTailNarrative(threadId, context); this.publishPendingPlanQuestions(threadId); @@ -1285,11 +1319,20 @@ export class ThreadHydrator { const goalLookup = this.transport().getThreadGoal(threadId).catch(() => null); const requestedLimit = options?.fetchLimit ?? MESSAGE_FETCH_SIZE; const tailLoader = this.transport().loadConversationTail; - const usedTail = tailLoader != null && requestedLimit <= MESSAGE_FETCH_SIZE; - const page = usedTail && tailLoader - ? await this.loadConversationTail(threadId, requestedLimit, tailLoader) - : await this.transport().loadConversationPage(threadId, requestedLimit); - return { page, usedTail, snapshots, goalLookup }; + if (tailLoader != null && requestedLimit <= MESSAGE_FETCH_SIZE) { + // The narrative page must land in the first commit: committing the bare + // tail paints each response ahead of its reasoning and visibly reorders + // the transcript. The tail call stays only as a fallback payload. + const [tailResult, pageResult] = await Promise.allSettled([ + this.loadConversationTail(threadId, requestedLimit, tailLoader), + this.transport().loadConversationPage(threadId, requestedLimit), + ]); + if (pageResult.status === "fulfilled") return { page: pageResult.value, usedTail: false, snapshots, goalLookup }; + if (tailResult.status === "fulfilled") return { page: tailResult.value, usedTail: true, snapshots, goalLookup }; + throw pageResult.reason; + } + const page = await this.transport().loadConversationPage(threadId, requestedLimit); + return { page, usedTail: false, snapshots, goalLookup }; } private async loadConversationTail( @@ -1388,7 +1431,12 @@ export class ThreadHydrator { const retained = new Set(current.messages.map((message) => message.id)); return { records: patchThreadRecord(state.records, threadId, { - narrativeByMessage: Object.fromEntries(Object.entries(page.narrativeByMessage).filter(([id]) => retained.has(id))), + // Merge over resident detail: the followup page may omit narrative for + // messages the visible-tail prefetch just warmed. + narrativeByMessage: { + ...current.narrativeByMessage, + ...Object.fromEntries(Object.entries(page.narrativeByMessage).filter(([id]) => retained.has(id))), + }, answeredPlanMessageIds: new Set((page.answeredPlanMessageIds ?? []).filter((id) => retained.has(id))), }), }; diff --git a/apps/web/src/features/conversation/hydration/types.ts b/apps/web/src/features/conversation/hydration/types.ts index 14fa742ca..de651237f 100644 --- a/apps/web/src/features/conversation/hydration/types.ts +++ b/apps/web/src/features/conversation/hydration/types.ts @@ -88,7 +88,7 @@ export interface ThreadHydratorDeps { ) => void; getWorkspaceThread: (threadId: string) => HydratorWorkspaceThread | undefined; flushPendingTextDeltas: () => void; - loadNarrativeForMessage: (messageId: string) => Promise; + loadNarrativeForMessage: (messageId: string, threadId: string) => Promise; setPlanQuestions: (threadId: string, questions: PlanQuestion[]) => void; extractPendingPlanQuestions: ( messages: Message[], diff --git a/apps/web/src/features/conversation/messages/TranscriptNarrativeRow.tsx b/apps/web/src/features/conversation/messages/TranscriptNarrativeRow.tsx index ed58b2c06..2d756dc35 100644 --- a/apps/web/src/features/conversation/messages/TranscriptNarrativeRow.tsx +++ b/apps/web/src/features/conversation/messages/TranscriptNarrativeRow.tsx @@ -22,13 +22,7 @@ interface TranscriptNarrativeRowProps { export function VisibleNarrativeLoader({ threadId, messageId }: { threadId: string; messageId: string }) { const records = useThreadRecord(threadId, (record) => record.narrativeByMessage[messageId]); const load = useThreadStore((state) => state.loadNarrativeForMessage); - const retain = useThreadStore((state) => state.retainNarrativeForMessage); - const release = useThreadStore((state) => state.releaseNarrativeForMessage); - useEffect(() => { - retain(messageId, threadId); - return () => release(messageId, threadId); - }, [messageId, release, retain, threadId]); useEffect(() => { void load(messageId, threadId); }, [load, messageId, records, threadId]); diff --git a/apps/web/src/features/conversation/messages/__tests__/transcript-viewport.test.ts b/apps/web/src/features/conversation/messages/__tests__/transcript-viewport.test.ts index e36bac60d..dd82999ce 100644 --- a/apps/web/src/features/conversation/messages/__tests__/transcript-viewport.test.ts +++ b/apps/web/src/features/conversation/messages/__tests__/transcript-viewport.test.ts @@ -130,6 +130,28 @@ describe("transcript viewport", () => { expect(position).toEqual({ kind: "reading", key: "9", offset: 25 }); }); + it("pins the reading row to its scrolled screen position while prepended rows settle", () => { + const tops = new Map(rows.map((row, index) => [row.id, index * 100])); + vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockImplementation(function (this: HTMLElement) { + const key = this.getAttribute("data-transcript-key"); + const contentTop = key === null ? 0 : tops.get(key) ?? 0; + return new DOMRect(0, contentTop - view.viewport.scrollTop, 600, 100); + }); + view.moveTo({ kind: "reading", key: "9", offset: 25 }); + expect(view.viewport.scrollTop).toBe(925); + const anchor = hosts.find((host) => host.id === "9")!; + // Prepended rows mount at provisional heights but really measure 300 each. + for (const [id, top] of tops) tops.set(id, top + 400); + view.setRows([{ id: "older-1", height: 100 }, { id: "older-2", height: 100 }, ...rows]); + expect(anchor.element.getBoundingClientRect().top).toBe(-25); + expect(view.viewport.scrollTop).toBe(1325); + // A later measurement pass pushes the row again; the pin still wins. + for (const [id, top] of tops) tops.set(id, top + 100); + view.setRows([{ id: "older-1", height: 100 }, { id: "older-2", height: 100 }, ...rows]); + expect(anchor.element.getBoundingClientRect().top).toBe(-25); + expect(view.viewport.scrollTop).toBe(1425); + }); + it("preserves the reading row when history fills a previously short viewport", () => { const recent = [{ id: "recent", height: 120 }]; view.setRows(recent); diff --git a/apps/web/src/stores/threadStore.ts b/apps/web/src/stores/threadStore.ts index 52d64b666..f8aed5a8a 100644 --- a/apps/web/src/stores/threadStore.ts +++ b/apps/web/src/stores/threadStore.ts @@ -234,10 +234,6 @@ interface ThreadState { /** Fetch one bounded persisted-detail window for a rendered assistant message. */ loadNarrativeForMessage: (messageId: string, threadId?: string) => Promise; - /** Keep a persisted turn available while one of its virtual rows is mounted. */ - retainNarrativeForMessage: (messageId: string, threadId?: string) => void; - /** Release a virtual-row lease and evict after all rows for the turn unmount. */ - releaseNarrativeForMessage: (messageId: string, threadId?: string) => void; /** Return whether a complete narrative payload has been loaded for a message. */ isNarrativeLoaded: (threadId: string, messageId: string) => boolean; /** Drop the cached narrative for a message and revoke any pending detail window. */ @@ -300,48 +296,29 @@ interface ThreadState { const dequeueTimers = new Map>(); /** - * Module-level detail-window leases. They stay outside Zustand because they - * coordinate requests without changing transcript render state. + * Module-level detail-window bookkeeping. It stays outside Zustand because it + * coordinates requests without changing transcript render state. Loaded detail + * stays on the record until the narrative byte budget trims it or the thread is + * cleared, so revisiting a turn or switching threads does not refetch it. */ const narrativeInflight = new Map>(); const narrativeGeneration = new Map(); const narrativeCursor = new Map(); /** Detail windows that ended before their effective limit. */ const narrativeLoaded = new Set(); -/** Mounted virtual rows that keep one persisted turn available while it is read. */ -const narrativeLeaseCounts = new Map(); -/** Invalidates a queued release when another row for the same turn mounts. */ -const narrativeReleaseGeneration = new Map(); const NARRATIVE_DETAIL_LIMIT = 100; function narrativeKey(threadId: string, messageId: string): string { return `${threadId}\u0000${messageId}`; } -function revokeNarrativeLease(key: string): void { +function revokeNarrativeLoadState(key: string): void { narrativeGeneration.set(key, (narrativeGeneration.get(key) ?? 0) + 1); narrativeInflight.delete(key); narrativeCursor.delete(key); narrativeLoaded.delete(key); } -function retainNarrativeLease(key: string): void { - narrativeLeaseCounts.set(key, (narrativeLeaseCounts.get(key) ?? 0) + 1); - narrativeReleaseGeneration.set(key, (narrativeReleaseGeneration.get(key) ?? 0) + 1); -} - -function releaseNarrativeLease(key: string, release: () => void): void { - const count = narrativeLeaseCounts.get(key) ?? 0; - if (count <= 1) narrativeLeaseCounts.delete(key); - else narrativeLeaseCounts.set(key, count - 1); - const generation = (narrativeReleaseGeneration.get(key) ?? 0) + 1; - narrativeReleaseGeneration.set(key, generation); - queueMicrotask(() => { - if (narrativeLeaseCounts.has(key) || narrativeReleaseGeneration.get(key) !== generation) return; - release(); - }); -} - function clearNarrativeLoadState(threadId: string): void { const prefix = `${threadId}\u0000`; const keys = new Set([ @@ -349,14 +326,10 @@ function clearNarrativeLoadState(threadId: string): void { ...narrativeInflight.keys(), ...narrativeGeneration.keys(), ...narrativeCursor.keys(), - ...narrativeLeaseCounts.keys(), - ...narrativeReleaseGeneration.keys(), ]); for (const key of keys) { if (!key.startsWith(prefix)) continue; - revokeNarrativeLease(key); - narrativeLeaseCounts.delete(key); - narrativeReleaseGeneration.delete(key); + revokeNarrativeLoadState(key); } } @@ -367,6 +340,8 @@ interface NarrativeLoadContext { message: Message; generation: number; detailAfter: NarrativeDetailCursor | undefined; + /** True while the record still holds merged detail for this message. */ + hasDetail: boolean; } function resolveNarrativeLoadContext( @@ -384,10 +359,11 @@ function resolveNarrativeLoadContext( message, generation: narrativeGeneration.get(cacheKey) ?? 0, detailAfter: narrativeCursor.get(cacheKey), + hasDetail: current.narrativeByMessage[messageId] != null, }; } -function hasCurrentNarrativeLease(cacheKey: string, generation: number): boolean { +function isCurrentNarrativeGeneration(cacheKey: string, generation: number): boolean { return (narrativeGeneration.get(cacheKey) ?? 0) === generation; } @@ -1073,7 +1049,7 @@ export const useThreadStore = create((zustandSet, get) => { getWorkspaceThread: (threadId) => useWorkspaceStore.getState().threads.find((t) => t.id === threadId), flushPendingTextDeltas, - loadNarrativeForMessage: (messageId) => get().loadNarrativeForMessage(messageId), + loadNarrativeForMessage: (messageId, threadId) => get().loadNarrativeForMessage(messageId, threadId), setPlanQuestions: (threadId, questions) => get().setPlanQuestions(threadId, questions), extractPendingPlanQuestions, getTasksForThread: (threadId) => useTaskStore.getState().tasksByThread[threadId] ?? [], @@ -3688,8 +3664,8 @@ export const useThreadStore = create((zustandSet, get) => { if (!currentId) return; const context = resolveNarrativeLoadContext(get().records, currentId, messageId); if (!context) return; - const { cacheKey, message, generation, detailAfter } = context; - if (narrativeLoaded.has(cacheKey)) return; + const { cacheKey, message, generation, detailAfter, hasDetail } = context; + if (narrativeLoaded.has(cacheKey) && hasDetail) return; const existing = narrativeInflight.get(cacheKey); if (existing) return existing; const p = getTransport() @@ -3702,7 +3678,7 @@ export const useThreadStore = create((zustandSet, get) => { }, }) .then((entries) => { - if (!hasCurrentNarrativeLease(cacheKey, generation)) return; + if (!isCurrentNarrativeGeneration(cacheKey, generation)) return; const current = narrativeRecordWithMessage(get().records, currentId, messageId); if (!current) return; const merged = mergeNarrativeEntries(current.narrativeByMessage[messageId], entries); @@ -3711,7 +3687,7 @@ export const useThreadStore = create((zustandSet, get) => { // later visible lease cannot silently skip detail that did not fit. return; } - if (!hasCurrentNarrativeLease(cacheKey, generation)) return; + if (!isCurrentNarrativeGeneration(cacheKey, generation)) return; recordNarrativeWindowCursor(cacheKey, entries); // The detail rows cause the loader effect to run immediately. Clear // this completed request first so that effect can request the next @@ -3738,25 +3714,12 @@ export const useThreadStore = create((zustandSet, get) => { return p; }, - retainNarrativeForMessage: (messageId, explicitThreadId) => { - const threadId = explicitThreadId ?? get().currentThreadId; - if (!threadId) return; - retainNarrativeLease(narrativeKey(threadId, messageId)); - }, - - releaseNarrativeForMessage: (messageId, explicitThreadId) => { - const threadId = explicitThreadId ?? get().currentThreadId; - if (!threadId) return; - const key = narrativeKey(threadId, messageId); - releaseNarrativeLease(key, () => get().evictNarrativeForMessage(messageId, threadId)); - }, - isNarrativeLoaded: (threadId, messageId) => narrativeLoaded.has(narrativeKey(threadId, messageId)), evictNarrativeForMessage: (messageId, explicitThreadId) => { const currentId = explicitThreadId ?? get().currentThreadId; if (!currentId) return; - revokeNarrativeLease(narrativeKey(currentId, messageId)); + revokeNarrativeLoadState(narrativeKey(currentId, messageId)); patchRec(currentId, (rec) => { if (!(messageId in rec.narrativeByMessage)) return {}; const next = { ...rec.narrativeByMessage }; From f189ac629a61469bdb90b0020f772b62ac1823dc Mon Sep 17 00:00:00 2001 From: "(CJ) Chukwudi Nwobodo" <142016413+chuks-qua@users.noreply.github.com> Date: Thu, 24 Sep 2026 15:43:40 +0100 Subject: [PATCH 2/3] feat(web): fill the transcript viewport on open and fetch history earlier The two-message tail paints fast but leaves most of the screen empty. Top up with older pages after each commit until the transcript covers 1.5x the viewport (bounded at three pages). Page size drops to 25 and the boundary trigger moves out to one viewport height so the next page is resident before the edge. The pagination spinner now only appears when a fetch exceeds 300ms. --- apps/web/src/__tests__/pagination.test.ts | 4 +- .../conversation/messages/MessageList.tsx | 21 +++++++- .../messages/MessageListOverlays.tsx | 17 +++++-- .../MessageList.thread-switch.test.tsx | 50 +++++++++++++------ apps/web/src/stores/threadStore.ts | 2 +- 5 files changed, 70 insertions(+), 24 deletions(-) diff --git a/apps/web/src/__tests__/pagination.test.ts b/apps/web/src/__tests__/pagination.test.ts index 4e6223076..c0caf56d2 100644 --- a/apps/web/src/__tests__/pagination.test.ts +++ b/apps/web/src/__tests__/pagination.test.ts @@ -110,7 +110,7 @@ await activateTestConversation(threadId); expect(getTestThreadOldestLoadedSequence(threadId)).toBe(1); expect(getTestThreadHasMoreMessages(threadId)).toBe(false); expect(getTestThreadIsLoadingMore(threadId)).toBe(false); - expect(mockTransport.getMessages).toHaveBeenCalledWith(threadId, 50, 51); + expect(mockTransport.getMessages).toHaveBeenCalledWith(threadId, 25, 51); }); it("loadOlderMessages is a no-op when hasMore is false", async () => { @@ -541,7 +541,7 @@ await activateTestConversation(threadId); await useThreadStore.getState().loadOlderMessages(threadId); - expect(mockTransport.loadConversationPage).toHaveBeenCalledWith(threadId, 50, 5); + expect(mockTransport.loadConversationPage).toHaveBeenCalledWith(threadId, 25, 5); expect(getTestActiveMessages()).toEqual([ older, residentAtSharedSequence, diff --git a/apps/web/src/features/conversation/messages/MessageList.tsx b/apps/web/src/features/conversation/messages/MessageList.tsx index bb7c8ba6f..c4ee3b9ea 100644 --- a/apps/web/src/features/conversation/messages/MessageList.tsx +++ b/apps/web/src/features/conversation/messages/MessageList.tsx @@ -26,6 +26,8 @@ import { estimateMessageListItemHeight, type MessageListItem } from "./message-l import { TranscriptViewport, type TranscriptHost, type TranscriptPosition } from "./transcript-viewport"; const SOURCE_NAVIGATION_MAX_FRAMES = 20; +const FILL_VIEWPORT_RATIO = 1.5; +const MAX_FILL_PAGES = 3; /** A card source request that MessageList resolves through its resident transcript. */ export interface SelectedTextCommentSourceNavigationRequest { @@ -274,7 +276,8 @@ function loadHistoryAtBoundary(viewport: HTMLElement, data: MessageListData, dir const isOlder = direction === "older"; const remaining = isOlder ? viewport.scrollTop : viewport.scrollHeight - viewport.scrollTop - viewport.clientHeight; const available = isOlder ? data.hasMore && !data.isLoadingMore : data.hasNewer && !data.isLoadingNewer; - if (remaining > 200 || !available) return false; + // Fetch a page before the edge so the next page is resident on arrival. + if (remaining > viewport.clientHeight || !available) return false; const load = isOlder ? data.loadOlderMessages : data.loadNewerMessages; void load(data.renderedThreadId); return true; @@ -384,6 +387,22 @@ function ThreadTranscript({ data, ...props }: MessageListProps & { readonly data if (positionRef.current.kind !== "end") setHasNewContent(true); }, [data.streamingText, lastItemKey]); + const fillPages = useRef(0); + useEffect(() => { + fillPages.current = 0; + }, [data.renderedThreadId]); + // The tail paints fast but rarely covers the viewport; top up until it does. + useEffect(() => { + const view = controllerRef.current; + const current = latest.current.data; + if (!view || !restored.current || !current.isRenderedVisible || !current.renderedThreadId) return; + if (current.loading || current.isLoadingMore || !current.hasMore || fillPages.current >= MAX_FILL_PAGES) return; + const viewport = view.viewport; + if (viewport.scrollHeight >= viewport.clientHeight * FILL_VIEWPORT_RATIO) return; + fillPages.current += 1; + void current.loadOlderMessages(current.renderedThreadId); + }, [items, data.isLoadingMore, data.hasMore, data.renderedThreadId]); + const loadRequestedHistory = useCallback(() => { const view = controllerRef.current; const current = latest.current.data; diff --git a/apps/web/src/features/conversation/messages/MessageListOverlays.tsx b/apps/web/src/features/conversation/messages/MessageListOverlays.tsx index e38a16769..d006ef10c 100644 --- a/apps/web/src/features/conversation/messages/MessageListOverlays.tsx +++ b/apps/web/src/features/conversation/messages/MessageListOverlays.tsx @@ -3,7 +3,7 @@ import { Skeleton } from "@/components/ui/skeleton"; import { StickyUserMessage } from "@/components/chat/StickyUserMessage"; import { PRIMARY_CONTENT_RAIL_CLASS } from "@/lib/layout-rails"; import type { SelectedTextComment } from "@mcode/contracts"; -import { useMemo, type RefObject } from "react"; +import { useEffect, useMemo, useState, type RefObject } from "react"; import type { SelectedTextCommentEditorDraft } from "@/stores/composerDraftStore"; import type { Message } from "@/transport/types"; import { ScrollToBottomButton } from "./ScrollToBottomButton"; @@ -79,8 +79,8 @@ export function MessageListOverlays({ )} - {isLoadingMore && } - {isLoadingNewer && } + {isLoadingMore && } + {isLoadingNewer && } {onSelectedTextComment && ( { + if (delayMs === 0) return; + const timer = setTimeout(() => setVisible(true), delayMs); + return () => clearTimeout(timer); + }, [delayMs]); const positionClass = placement === "top" ? "top-2" : "bottom-2"; + if (!visible) return null; return (
diff --git a/apps/web/src/features/conversation/messages/__tests__/MessageList.thread-switch.test.tsx b/apps/web/src/features/conversation/messages/__tests__/MessageList.thread-switch.test.tsx index b06fcab95..ff9a0a7c9 100644 --- a/apps/web/src/features/conversation/messages/__tests__/MessageList.thread-switch.test.tsx +++ b/apps/web/src/features/conversation/messages/__tests__/MessageList.thread-switch.test.tsx @@ -6,9 +6,6 @@ import { createAgentModelState, type AgentItem, type AgentTurn, type Message, ty const loadOlderMessagesSpy = vi.fn(); const loadNewerMessagesSpy = vi.fn(); const loadNarrativeForMessageSpy = vi.fn(); -const evictNarrativeForMessageSpy = vi.fn(); -const retainNarrativeForMessageSpy = vi.fn(); -const releaseNarrativeForMessageSpy = vi.fn(); class LayoutObserver implements ResizeObserver { static instances: LayoutObserver[] = []; @@ -146,9 +143,6 @@ vi.mock("@/stores/threadStore", () => ({ loadOlderMessages: loadOlderMessagesSpy, loadNewerMessages: loadNewerMessagesSpy, loadNarrativeForMessage: loadNarrativeForMessageSpy, - retainNarrativeForMessage: retainNarrativeForMessageSpy, - releaseNarrativeForMessage: releaseNarrativeForMessageSpy, - evictNarrativeForMessage: evictNarrativeForMessageSpy, isNarrativeLoaded: () => false, }); }), @@ -225,9 +219,6 @@ beforeEach(() => { loadOlderMessagesSpy.mockClear(); loadNewerMessagesSpy.mockClear(); loadNarrativeForMessageSpy.mockClear(); - retainNarrativeForMessageSpy.mockClear(); - releaseNarrativeForMessageSpy.mockClear(); - evictNarrativeForMessageSpy.mockClear(); loadingValue = false; activeThreadIdValue = "thread-A"; messagesValue = [{ id: "m1", sequence: 1 }]; @@ -273,7 +264,7 @@ describe("MessageList thread switch", () => { expect(rail).not.toHaveClass("overflow-x-hidden"); }); - it("hydrates a mounted assistant from its owning thread and releases its virtual lease after unmount", async () => { + it("hydrates a mounted assistant from its owning thread", async () => { activeThreadIdValue = "thread-A"; currentThreadIdValue = "thread-A"; messagesValue = [{ @@ -284,14 +275,11 @@ describe("MessageList thread switch", () => { content: "Child result", }]; - const view = render(); + render(); await waitFor(() => { expect(loadNarrativeForMessageSpy).toHaveBeenCalledWith("child-answer", "thread-B"); - expect(retainNarrativeForMessageSpy).toHaveBeenCalledWith("child-answer", "thread-B"); }); - view.unmount(); - expect(releaseNarrativeForMessageSpy).toHaveBeenCalledWith("child-answer", "thread-B"); }); it("renders growing canonical child text before completion without duplicating its bubble", () => { @@ -350,6 +338,8 @@ describe("MessageList thread switch", () => { it("loads and scrolls a virtualized source before it reconstructs the saved range", async () => { messagesValue = [{ id: "m1", sequence: 2, thread_id: "thread-A", role: "assistant", content: "Current message" }]; hasMoreMessagesValue = true; + // A tall viewport keeps the viewport-fill effect out of this scenario. + vi.spyOn(HTMLElement.prototype, "scrollHeight", "get").mockReturnValue(5000); const comment: SelectedTextComment = { id: "11111111-1111-4111-8111-111111111111", displayNumber: 1, @@ -421,6 +411,7 @@ describe("MessageList thread switch", () => { it("marks a source unavailable after its required history page fails to load", async () => { messagesValue = [{ id: "m1", sequence: 2, thread_id: "thread-A", role: "assistant", content: "Current message" }]; hasMoreMessagesValue = true; + vi.spyOn(HTMLElement.prototype, "scrollHeight", "get").mockReturnValue(5000); loadOlderMessagesSpy.mockResolvedValueOnce("failed"); const comment: SelectedTextComment = { id: "11111111-1111-4111-8111-111111111111", @@ -460,6 +451,7 @@ describe("MessageList thread switch", () => { it("keeps the latest source request active when an earlier history request fails", async () => { messagesValue = [{ id: "m1", sequence: 3, thread_id: "thread-A", role: "assistant", content: "Current message" }]; hasMoreMessagesValue = true; + vi.spyOn(HTMLElement.prototype, "scrollHeight", "get").mockReturnValue(5000); const commentA: SelectedTextComment = { id: "11111111-1111-4111-8111-111111111111", displayNumber: 1, @@ -1326,8 +1318,34 @@ describe("MessageList thread switch", () => { expect(container.querySelector('[data-message-id="thread-A-11"]')).not.toBeNull(); }); + it("tops up an underfilled transcript with older history until it covers the viewport", async () => { + messagesValue = transcriptRows().slice(0, 8); + hasMoreMessagesValue = true; + loadOlderMessagesSpy.mockImplementation(async () => { + messagesValue = [ + ...Array.from({ length: 10 }, (_, index) => ({ + id: `older-${index}`, thread_id: "thread-A", sequence: -10 + index, + role: "assistant" as const, content: `Older ${index}`, + })), + ...messagesValue, + ]; + hasMoreMessagesValue = false; + return "loaded"; + }); + const { container, rerender } = render(); + await waitFor(() => expect(loadOlderMessagesSpy).toHaveBeenCalledWith("thread-A")); + act(() => rerender()); + await measureRows(container); + expect(loadOlderMessagesSpy).toHaveBeenCalledTimes(1); + }); + it("loads older and newer history only after a gesture reaches its boundary", async () => { - messagesValue = transcriptRows(); + // Tall content keeps the viewport-fill effect out and leaves room to sit + // more than one viewport-height away from the bottom boundary. + messagesValue = Array.from({ length: 30 }, (_, sequence) => ({ + id: `thread-A-${sequence}`, thread_id: "thread-A", sequence, + role: "assistant" as const, content: `Message ${sequence}`, + })); hasMoreMessagesValue = hasNewerMessagesValue = true; const { container } = render(); await measureRows(container); @@ -1338,7 +1356,7 @@ describe("MessageList thread switch", () => { expect(loadOlderMessagesSpy).toHaveBeenCalledExactlyOnceWith("thread-A"); fireEvent.wheel(viewport, { deltaY: 100 }); expect(loadNewerMessagesSpy).not.toHaveBeenCalled(); - viewport.scrollTop = 350; + viewport.scrollTop = 2100; fireEvent.scroll(viewport); expect(loadNewerMessagesSpy).toHaveBeenCalledExactlyOnceWith("thread-A"); }); diff --git a/apps/web/src/stores/threadStore.ts b/apps/web/src/stores/threadStore.ts index f8aed5a8a..f7ada856d 100644 --- a/apps/web/src/stores/threadStore.ts +++ b/apps/web/src/stores/threadStore.ts @@ -620,7 +620,7 @@ export function countActiveSubagentCalls(calls: ToolCall[] | undefined): number } /** Number of messages to fetch per directional pagination request. */ -export const HISTORY_PAGE_SIZE = 50; +export const HISTORY_PAGE_SIZE = 25; /** Maximum messages kept in the in-memory sliding window. */ export const MESSAGE_WINDOW_SIZE = 200; From 521628ced3fe3faad7c14fddb18889899a7cb5ce Mon Sep 17 00:00:00 2001 From: "(CJ) Chukwudi Nwobodo" <142016413+chuks-qua@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:20:25 +0100 Subject: [PATCH 3/3] perf(web): keep recent transcripts mounted and swap visibility on thread switches A warm thread switch remounted the whole virtualized transcript: the projection caches died with the component and vlist rebuilt ~15 row components before the transcript could paint, so revisiting a thread cost 173-630ms even though the record commit was ~3ms. Keep the five most recently selected transcripts mounted, hidden with visibility:hidden and inert instead of display:none (which would collapse the virtualizer to zero height and destroy every row). Hidden transcripts hold a residency display lease so their records stay resident and self-heal after cache eviction; eviction past the LRU cap releases the lease and falls back to the existing cache path. A resident target record is now paintable immediately instead of waiting for the hydration commit. Measured on a production build with 150-message threads: warm switches drop to ~70-135ms; memory stays bounded at ~2MB per retained transcript and flat across churn. --- .../messages/__tests__/ChatView.test.tsx | 75 ++++++++++------- .../messages/chat-view/ChatViewSurface.tsx | 83 +++++++++++++++++-- .../messages/chat-view/useChatViewState.ts | 33 +++++++- .../residency/conversation-residency.ts | 5 ++ 4 files changed, 156 insertions(+), 40 deletions(-) diff --git a/apps/web/src/features/conversation/messages/__tests__/ChatView.test.tsx b/apps/web/src/features/conversation/messages/__tests__/ChatView.test.tsx index d8904c583..4c9ef57a1 100644 --- a/apps/web/src/features/conversation/messages/__tests__/ChatView.test.tsx +++ b/apps/web/src/features/conversation/messages/__tests__/ChatView.test.tsx @@ -57,6 +57,17 @@ const { return () => chatViewDisplayLeaseListeners.delete(listener); }), getDisplayConversationSnapshot: vi.fn(() => chatViewDisplayLeaseIdsRef.current), + mountDisplayConversation: vi.fn((threadId: string) => { + if (!chatViewDisplayLeaseIdsRef.current.includes(threadId)) { + chatViewDisplayLeaseIdsRef.current = [...chatViewDisplayLeaseIdsRef.current, threadId]; + for (const listener of chatViewDisplayLeaseListeners) listener(); + } + return Promise.resolve(); + }), + unmountDisplayConversation: vi.fn((threadId: string) => { + chatViewDisplayLeaseIdsRef.current = chatViewDisplayLeaseIdsRef.current.filter((id) => id !== threadId); + for (const listener of chatViewDisplayLeaseListeners) listener(); + }), }, }; }); @@ -133,6 +144,7 @@ vi.mock("@/transport", () => ({ vi.mock("@/features/conversation/residency/conversation-residency", () => ({ getConversationResidency: () => chatViewResidencyMock, + tryGetConversationResidency: () => chatViewResidencyMock, })); // Composer and MessageList have deep dependencies; stub them out. @@ -183,6 +195,7 @@ import { getThreadSwitchTelemetryCounters, } from "@/lib/thread-switch-telemetry"; import { ChatView } from "../ChatView"; +import { KEPT_ALIVE_THREAD_COUNT } from "../chat-view/useChatViewState"; /** Build a minimal Thread fixture. */ function makeThread(overrides: Partial = {}): Thread { @@ -758,7 +771,7 @@ describe("ChatView - Thread Title Double-Click Rename", () => { expect(screen.getByTestId("conversation-transition-shell")).toHaveTextContent("Thread 2"); expect(screen.getByTestId("conversation-transition-shell")).toHaveAttribute("data-thread-id", "thread-2"); expect(screen.queryByTestId("conversation-loading")).not.toBeInTheDocument(); - expect(screen.queryByTestId("message-list")).not.toBeInTheDocument(); + for (const el of screen.queryAllByTestId("message-list")) expect(el).not.toBeVisible(); }); it("keeps startup progress visible while the durable thread hydrates", () => { @@ -873,7 +886,9 @@ describe("ChatView - Thread Title Double-Click Rename", () => { view.rerender(); expect(screen.getByTestId("chat-message-stage")).toBeInTheDocument(); - expect(screen.getByTestId("message-list")).toBeInTheDocument(); + const visible = screen.getAllByTestId("message-list").find((el) => el.style.display !== "none"); + expect(visible).toBeDefined(); + expect(visible).toHaveAttribute("data-display-thread-id", persisted.id); expect(screen.queryByTestId("thread-preparing-shell")).not.toBeInTheDocument(); expect(screen.queryByTestId("conversation-transition-shell")).not.toBeInTheDocument(); expect(chatViewTransportMock.getThreadStartup.mock.calls).toHaveLength(recoveryCallsBeforeAgentAdmission.get + 1); @@ -1129,8 +1144,11 @@ describe("ChatView - Thread Title Double-Click Rename", () => { act(() => rerender()); expect(screen.getByTestId("chat-header-title")).toHaveTextContent("Thread 2"); - expect(screen.getByTestId("message-list")).toHaveAttribute("data-display-thread-id", thread1.id); - expect(screen.getByTestId("message-list").parentElement).toHaveAttribute("inert"); + const held = screen.getAllByTestId("message-list") + .find((el) => el.getAttribute("data-display-thread-id") === thread1.id); + expect(held).toBeDefined(); + expect(held).toBeVisible(); + expect(held!.closest("[inert]")).not.toBeNull(); expect(screen.getByTestId("conversation-hold-overlay")).toHaveTextContent("Thread 2"); const targetRecord = { @@ -1145,7 +1163,9 @@ describe("ChatView - Thread Title Double-Click Rename", () => { act(() => rerender()); expect(screen.queryByTestId("conversation-hold-overlay")).not.toBeInTheDocument(); - expect(screen.getByTestId("message-list")).not.toHaveAttribute("data-display-thread-id"); + const visible = screen.getAllByTestId("message-list").find((el) => el.style.display !== "none"); + expect(visible).toBeDefined(); + expect(visible).toHaveAttribute("data-display-thread-id", thread2.id); }); it("holds the outgoing transcript for an empty running target", () => { @@ -1180,7 +1200,10 @@ describe("ChatView - Thread Title Double-Click Rename", () => { }); act(() => rerender()); - expect(screen.getByTestId("message-list")).toHaveAttribute("data-display-thread-id", thread1.id); + const held = screen.getAllByTestId("message-list") + .find((el) => el.getAttribute("data-display-thread-id") === thread1.id); + expect(held).toBeDefined(); + expect(held).toBeVisible(); expect(screen.getByTestId("conversation-hold-overlay")).toBeInTheDocument(); expect(screen.queryByTestId("thread-preparing-shell")).not.toBeInTheDocument(); expect(screen.queryByTestId("startup-progress")).not.toBeInTheDocument(); @@ -1228,7 +1251,7 @@ describe("ChatView - Thread Title Double-Click Rename", () => { act(() => rerender()); expect(screen.queryByTestId("conversation-hold-overlay")).not.toBeInTheDocument(); - expect(screen.queryByTestId("message-list")).not.toBeInTheDocument(); + for (const el of screen.queryAllByTestId("message-list")) expect(el).not.toBeVisible(); expect(screen.getByTestId("conversation-transition-shell")).toHaveAttribute("data-thread-id", thread3.id); }); @@ -1244,7 +1267,7 @@ describe("ChatView - Thread Title Double-Click Rename", () => { expect(screen.getByTestId("conversation-error")).toHaveTextContent("Conversation request failed"); expect(screen.queryByTestId("conversation-loading")).not.toBeInTheDocument(); - expect(screen.queryByTestId("message-list")).not.toBeInTheDocument(); + for (const el of screen.queryAllByTestId("message-list")) expect(el).not.toBeVisible(); }); it("keeps a live turn visible when hydration fails before any messages are resident", () => { @@ -1336,31 +1359,32 @@ describe("ChatView - Thread Title Double-Click Rename", () => { }); it("retries a rejected thread unsubscription", async () => { - const thread1 = makeThread({ id: "thread-1", title: "Thread 1" }); - const thread2 = makeThread({ id: "thread-2", title: "Thread 2" }); + // Kept-alive transcripts stay subscribed through their display lease, so + // thread-1 must age out of the retained set before it unsubscribes. + const threads = Array.from({ length: KEPT_ALIVE_THREAD_COUNT + 1 }, (_, index) => + makeThread({ id: `thread-${index + 1}`, title: `Thread ${index + 1}` })); setupWorkspaceMock(defaultWorkspaceState({ - activeThreadId: thread1.id, - threads: [thread1, thread2], + activeThreadId: threads[0]!.id, + threads, })); const { rerender } = render(); await waitFor(() => { - expect(chatViewTransportMock.subscribeThread).toHaveBeenCalledWith(thread1.id); + expect(chatViewTransportMock.subscribeThread).toHaveBeenCalledWith(threads[0]!.id); }); chatViewTransportMock.unsubscribeThread .mockRejectedValueOnce(new Error("temporary unsubscribe failure")) .mockResolvedValue(undefined); - setupWorkspaceMock(defaultWorkspaceState({ - activeThreadId: thread2.id, - threads: [thread1, thread2], - })); - chatViewThreadMockRef.current = defaultThreadState({ currentThreadId: thread2.id }); - rerender(); + for (const thread of threads.slice(1)) { + setupWorkspaceMock(defaultWorkspaceState({ activeThreadId: thread.id, threads })); + chatViewThreadMockRef.current = defaultThreadState({ currentThreadId: thread.id }); + act(() => rerender()); + } await waitFor(() => { expect(chatViewTransportMock.unsubscribeThread).toHaveBeenCalledTimes(2); - expect(chatViewTransportMock.unsubscribeThread).toHaveBeenLastCalledWith(thread1.id); + expect(chatViewTransportMock.unsubscribeThread).toHaveBeenLastCalledWith(threads[0]!.id); }, { timeout: 3000 }); }); @@ -1629,17 +1653,12 @@ describe("ChatView - Thread Title Double-Click Rename", () => { await new Promise((resolve) => setTimeout(resolve, 0)); expect(setThreadSubscriptions).toHaveBeenCalledTimes(1); + // The outgoing transcript stays subscribed through its kept-alive lease. requests[0]?.resolve(); await waitFor(() => { - expect(setThreadSubscriptions).toHaveBeenCalledTimes(2); - expect(requests[1]?.input).toEqual({ - threadIds: ["thread-2"], - revisions: { "thread-2": { conversationRevision: 0, rosterRevision: 0 } }, - }); + for (const request of requests) request.resolve(); + expect(serverThreadIds).toEqual(["thread-2", "thread-1"]); }); - - requests[1]?.resolve(); - await waitFor(() => expect(serverThreadIds).toEqual(["thread-2"])); }); it("clears a pending atomic set on unmount without retrying after it settles", async () => { diff --git a/apps/web/src/features/conversation/messages/chat-view/ChatViewSurface.tsx b/apps/web/src/features/conversation/messages/chat-view/ChatViewSurface.tsx index 2aa48a4ea..19c101b40 100644 --- a/apps/web/src/features/conversation/messages/chat-view/ChatViewSurface.tsx +++ b/apps/web/src/features/conversation/messages/chat-view/ChatViewSurface.tsx @@ -1,4 +1,4 @@ -import { type ComponentProps, type ReactNode } from "react"; +import { useEffect, type ComponentProps, type ReactNode } from "react"; import { Bug, GitFork, Hammer, SearchCode, ScanSearch } from "lucide-react"; import type { RecoveryIncident, SelectedTextComment } from "@mcode/contracts"; import { Badge } from "@/components/ui/badge"; @@ -30,6 +30,7 @@ import type { SubagentRosterTarget } from "../../narrative"; import { Composer } from "../../composer/Composer"; import { SavingDelayedDialog } from "../../saving/SavingDelayedDialog"; import { MessageList, type SelectedTextCommentSourceNavigationRequest } from "../MessageList"; +import { tryGetConversationResidency } from "../../residency/conversation-residency"; import type { ChatViewState } from "./useChatViewState"; const NEW_THREAD_STARTERS = [ @@ -386,7 +387,59 @@ function getConversationStage(state: ChatViewState): ConversationStage { return "messages"; } -/** Renders one selected conversation stage without taking over MessageList scrolling. */ +/** Renders one retained transcript and holds a display lease while it is hidden. */ +function KeptAliveTranscript({ + threadId, + selected, + visible, + leadingContent, + messageListProps, +}: { + threadId: string; + selected: boolean; + visible: boolean; + leadingContent: ReactNode; + messageListProps: Omit, "leadingContent" | "displayThreadId">; +}) { + // The lease keeps a hidden transcript's record resident and self-heals it after + // cache eviction; releasing on selection is a no-op while the thread is current. + useEffect(() => { + if (selected) return; + const residency = tryGetConversationResidency(); + if (!residency) return; + void residency.mountDisplayConversation(threadId); + return () => residency.unmountDisplayConversation(threadId); + }, [selected, threadId]); + // display:none would collapse the virtualized viewport to zero height and make + // the virtualizer destroy every row, so hidden transcripts stay laid out but + // unpainted; revealing one is a style flip instead of a DOM rebuild. + return ( +
+ +
+ ); +} + +/** Renders the hold, transition, or error overlay above the retained transcripts. */ +function ConversationStageOverlay({ stage, state, thread }: { stage: ConversationStage; state: ChatViewState; thread: WorkspaceThread }) { + switch (stage) { + case "hold": + return ; + case "transition": + return ; + case "error": + return ; + default: + return null; + } +} + +/** Renders retained transcripts and swaps visibility instead of remounting on switches. */ function ConversationStageContent({ stage, state, @@ -400,12 +453,26 @@ function ConversationStageContent({ leadingContent: ReactNode; messageListProps: Omit, "leadingContent" | "displayThreadId">; }) { - if (stage === "hold") { - return
; - } - if (stage === "transition") return ; - if (stage === "error") return ; - return ; + const visibleThreadId = + stage === "hold" ? state.displayHoldThreadId + : stage === "messages" ? state.activeThreadId + : null; + const transcripts = state.recentThreadIds.map((id) => ( + + )); + return ( +
+
{transcripts}
+ +
+ ); } /** Renders the conversation stage without taking over MessageList scrolling. */ diff --git a/apps/web/src/features/conversation/messages/chat-view/useChatViewState.ts b/apps/web/src/features/conversation/messages/chat-view/useChatViewState.ts index e9ba1fc83..1097846ab 100644 --- a/apps/web/src/features/conversation/messages/chat-view/useChatViewState.ts +++ b/apps/web/src/features/conversation/messages/chat-view/useChatViewState.ts @@ -1,4 +1,4 @@ -import { useMemo, useRef, useSyncExternalStore } from "react"; +import { useMemo, useRef, useState, useSyncExternalStore } from "react"; import { MAX_THREAD_SUBSCRIPTIONS } from "@mcode/contracts"; import { useElementWidth } from "@/hooks/useElementWidth"; import { overviewResponsivePaddingRight } from "@/lib/composer-layout"; @@ -11,11 +11,14 @@ import { useWorkspaceStore } from "@/features/projects/state/workspaceStore"; import { useActiveWorkspaceThread, useParentThreadExists } from "@/features/projects/state/workspace-selectors"; import { hasResidentContent } from "../../hydration/resident-content"; import { getConversationResidency } from "../../residency/conversation-residency"; -import { useActiveThreadRecord } from "../../state"; +import { useActiveThreadRecord, useThreadRecord } from "../../state"; import { useOutgoingTranscriptHold } from "./useOutgoingTranscriptHold"; const EMPTY_DISPLAYED_THREAD_IDS: readonly string[] = []; +/** Recently selected transcripts kept mounted so switching back skips a full rebuild. */ +export const KEPT_ALIVE_THREAD_COUNT = 5; + /** Subscribes to retained conversations that remain visible after selection changes. */ function subscribeDisplayedConversations(listener: () => void): () => void { return getConversationResidency().subscribeDisplayConversations(listener); @@ -26,6 +29,21 @@ function getDisplayedConversationSnapshot(): readonly string[] { return getConversationResidency().getDisplayConversationSnapshot(); } +/** Tracks the most recently selected thread IDs, capped for bounded transcript retention. */ +function useRecentThreadIds(activeThreadId: string | null | undefined): string[] { + const [recentThreadIds, setRecentThreadIds] = useState([]); + const previousActiveThreadId = useRef(null); + if (activeThreadId !== previousActiveThreadId.current) { + previousActiveThreadId.current = activeThreadId ?? null; + if (activeThreadId) { + setRecentThreadIds((ids) => + [activeThreadId, ...ids.filter((id) => id !== activeThreadId)] + .slice(0, KEPT_ALIVE_THREAD_COUNT)); + } + } + return recentThreadIds; +} + /** Selects bounded subscriptions for the active, visible, and running conversations. */ function getDesiredThreadIds( activeThreadId: string | null, @@ -91,6 +109,10 @@ export function useChatViewState() { const threadPaneWidth = useElementWidth(chatPaneRef, activeThreadId); const reserveOverviewSpace = useOverviewStore((state) => state.reserveThreadId === activeThreadId); const isAgentRunning = activeThreadId ? runningThreadIds.has(activeThreadId) : false; + // A resident target record (kept-alive or previously hydrated) can paint + // immediately; gating on the hydration commit would hide already-rendered + // content behind the transition shell on every warm switch. + const targetResidentContent = useThreadRecord(activeThreadId, hasResidentContent); const { targetPaintable } = getConversationPaintState( activeThreadId, hydratedThreadId, @@ -98,7 +120,9 @@ export function useChatViewState() { isAgentRunning, residentContent, ); - const displayHoldThreadId = useOutgoingTranscriptHold(activeThreadId, targetPaintable); + const effectiveTargetPaintable = targetPaintable || targetResidentContent; + const displayHoldThreadId = useOutgoingTranscriptHold(activeThreadId, effectiveTargetPaintable); + const recentThreadIds = useRecentThreadIds(activeThreadId); const activeWorkspaceName = useMemo( () => workspaces.find((workspace) => workspace.id === (activeThread?.workspace_id ?? activeWorkspaceId))?.name ?? "", [workspaces, activeThread?.workspace_id, activeWorkspaceId], @@ -126,6 +150,7 @@ export function useChatViewState() { isAgentRunning, messageCount, parentThreadExists, + recentThreadIds, reserveOverviewSpace, residentContent, runningThreadIds, @@ -135,7 +160,7 @@ export function useChatViewState() { setActiveThread, setForkMode, sidebarCollapsed, - targetPaintable, + targetPaintable: effectiveTargetPaintable, threadPaneWidth, updateThreadTitle, overviewPaddingRight: reserveOverviewSpace ? overviewResponsivePaddingRight() : undefined, diff --git a/apps/web/src/features/conversation/residency/conversation-residency.ts b/apps/web/src/features/conversation/residency/conversation-residency.ts index ee220fa50..10e3f7c48 100644 --- a/apps/web/src/features/conversation/residency/conversation-residency.ts +++ b/apps/web/src/features/conversation/residency/conversation-residency.ts @@ -278,6 +278,11 @@ export function registerConversationResidency(residency: ConversationResidency): registeredConversationResidency = residency; } +/** Return the registered residency authority, or null before the thread store initializes it. */ +export function tryGetConversationResidency(): ConversationResidency | null { + return registeredConversationResidency; +} + /** Return the internal residency authority after the thread store initializes it. */ export function getConversationResidency(): ConversationResidency { if (!registeredConversationResidency) {