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/__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/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/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__/ChatView.test.tsx b/apps/web/src/features/conversation/messages/__tests__/ChatView.test.tsx index c145f40f5..7e71b28b2 100644 --- a/apps/web/src/features/conversation/messages/__tests__/ChatView.test.tsx +++ b/apps/web/src/features/conversation/messages/__tests__/ChatView.test.tsx @@ -65,6 +65,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(); + }), }, }; }); @@ -152,6 +163,7 @@ vi.mock("@/transport", () => ({ vi.mock("@/features/conversation/residency/conversation-residency", () => ({ getConversationResidency: () => chatViewResidencyMock, + tryGetConversationResidency: () => chatViewResidencyMock, })); // Composer and MessageList have deep dependencies; stub them out. @@ -204,6 +216,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 { @@ -792,7 +805,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", () => { @@ -907,7 +920,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); @@ -1326,8 +1341,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 = { @@ -1342,7 +1360,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", () => { @@ -1377,7 +1397,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(); @@ -1425,7 +1448,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); }); @@ -1441,7 +1464,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", () => { @@ -1533,31 +1556,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 }); }); @@ -1826,17 +1850,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/__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/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/features/conversation/messages/chat-view/ChatViewSurface.tsx b/apps/web/src/features/conversation/messages/chat-view/ChatViewSurface.tsx index 65792b7ff..9f6c1d5a2 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, useState } from "react"; +import { useEffect, useState, 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"; @@ -31,6 +31,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 = [ @@ -387,7 +388,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, @@ -401,12 +454,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) { diff --git a/apps/web/src/stores/threadStore.ts b/apps/web/src/stores/threadStore.ts index 52d64b666..f7ada856d 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; } @@ -644,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; @@ -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 };