diff --git a/apps/desktop/src/stores/runtime/transcript-reading-runtime.ts b/apps/desktop/src/stores/runtime/transcript-reading-runtime.ts index cd4db240ba..da87c800f7 100644 --- a/apps/desktop/src/stores/runtime/transcript-reading-runtime.ts +++ b/apps/desktop/src/stores/runtime/transcript-reading-runtime.ts @@ -70,10 +70,21 @@ export function createTranscriptReadingRuntime({ get, set }: StoreAccess, read: if (signal.aborted || get().activeSessionId !== sessionId || !position.messageId) return false; const previous = get().transcriptViews[sessionId]; const range = currentView(sessionId); - if (get().selectingSessionId !== sessionId && - range.messages.some((message) => message.id === position.messageId)) return true; // A user navigation already in flight always wins over automatic restore. if (range.focus || range.loading) return false; + if (range.messages.some((message) => message.id === position.messageId)) { + // The page already holds the bookmark, so a bounded read around it would + // only narrow the transcript. Re-selecting a retained session commits + // that page while `selectingSessionId` is still set and then swaps + // `messages` for the revalidated window, so publish the page as the + // reading range: that keeps the bookmarked row, and the reader's + // offset, across the swap. Skipping the read also matters because a + // window around a bookmark in a tail page reports `hasMoreAfter`, which + // paints the load-later affordance on a conversation nobody left. + if (get().selectingSessionId === sessionId && !previous?.restored) + publish(sessionId, { ...range, restored: true }); + return true; + } const pending: TranscriptView = { ...range, restored: true, loading: "target" }; const rollback = () => { if (get().transcriptViews[sessionId] !== pending) return; diff --git a/apps/desktop/test/transcript-reading.test.mjs b/apps/desktop/test/transcript-reading.test.mjs index 575d353897..e9c1cfc24a 100644 --- a/apps/desktop/test/transcript-reading.test.mjs +++ b/apps/desktop/test/transcript-reading.test.mjs @@ -611,3 +611,36 @@ test("a restored range already at the tail uses canonical updates without adding assert.deepEqual(transcriptViewMessages(live, search).map((row) => [row.id, row.content]), [["tail", "persisted"]], "explicit search keeps its selected snapshot"); }); + +test("a bookmark inside the page being revalidated keeps that page instead of a narrower window", async () => { + // Returning to a retained session commits its page while `selectingSessionId` + // is still set and then swaps `messages` for the revalidated window. The + // bookmarked row is already in that page, so restoration must publish the page + // itself: reading a range around the anchor would both narrow the transcript + // and report `hasMoreAfter`, which paints the load-later affordance on a + // conversation nobody left. + const r = reader(async () => { throw new Error("unexpected read"); }, { + messages: [message("old"), message("tail")], selectingSessionId: "s", + }); + assert.equal(await r.restoreTranscriptReading("s", bookmark, new AbortController().signal), true); + assert.deepEqual(r.visible().map((row) => row.id), ["old", "tail"]); + assert.equal(r.view().restored, true); + assert.equal(r.view().hasMoreAfter, false); +}); + +test("a saved anchor does not supersede in-flight navigation when both rows are loaded", async () => { + const focused = { + ...transcriptViewFromSession(page([message("old"), message("target")]).session, { + ...target("target"), requestId: 7, + }), + loading: "target", + }; + const r = reader(async () => { throw new Error("automatic restore must not read"); }, { + messages: [message("old"), message("target")], + selectingSessionId: "s", + transcriptViews: { s: focused }, + }); + + assert.equal(await r.restoreTranscriptReading("s", bookmark, new AbortController().signal), false); + assert.equal(r.view(), focused, "explicit navigation keeps ownership of the reading view"); +}); diff --git a/docs/spec/06-delivery/04-e2e-test-plan.md b/docs/spec/06-delivery/04-e2e-test-plan.md index f5dd1e4d52..422678c89c 100644 --- a/docs/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/spec/06-delivery/04-e2e-test-plan.md @@ -6157,7 +6157,9 @@ must keep splitting are covered by `markdown-blocks.test.mjs`. an error toast, or a blank chat area. - **Expected**: Only the visible pane and the two most recently visited ones are retained; visiting beyond that budget evicts the oldest pane. A warm return - paints the retained frame and offset immediately. Returning to an evicted + paints the retained frame and offset immediately. The returned pane keeps the + page that already holds its anchor, so a warm return never offers to load later + messages for rows that page already had. Returning to an evicted session restores the message and viewport offset recorded before eviction, including explicit expanded content, using a bounded historical read if the latest page no longer contains the anchor. If the anchor is already loaded but diff --git a/scripts/e2e/session-reading-position.tsx b/scripts/e2e/session-reading-position.tsx index 928952260f..f146ad0b69 100644 --- a/scripts/e2e/session-reading-position.tsx +++ b/scripts/e2e/session-reading-position.tsx @@ -122,6 +122,11 @@ globalThis.sessionReadingPositionProbe = async () => { await select("A"); await until(() => Boolean(row(id)) && Math.abs(offset(id) - expectedOffset) < 2, "warm production pane preserves reading through revalidation"); + // Revalidation must not narrow the retained page to a window around the + // bookmark: that window reports newer rows and paints the load-later + // affordance on a conversation the reader never left. + assert(!host.querySelector('[data-visible="true"] .transcript-load-later'), + "warm return must not offer to load later messages"); for (const id of ["B", "C", "D", "E"]) await select(id); assert(!host.querySelector('[data-session-pane="real-A"]'), "A was not evicted"); await select("A");