Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions apps/desktop/src/stores/runtime/transcript-reading-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
33 changes: 33 additions & 0 deletions apps/desktop/test/transcript-reading.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
4 changes: 3 additions & 1 deletion docs/spec/06-delivery/04-e2e-test-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions scripts/e2e/session-reading-position.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading