diff --git a/src/bootSnapshot.ts b/src/bootSnapshot.ts index 23179780d..bd387b38c 100644 --- a/src/bootSnapshot.ts +++ b/src/bootSnapshot.ts @@ -86,6 +86,7 @@ export function saveBootSnapshot(snap: Omit): void { */ export function loadCachedThreadDetail(id: string | null): ThreadDetail | null { if (!id) return null; + if (pendingDetail?.thread.id === id) return pendingDetail; try { const raw = window.localStorage.getItem(DETAIL_KEY); if (!raw || raw.length > MAX_SNAPSHOT_CHARS) return null; @@ -99,11 +100,34 @@ export function loadCachedThreadDetail(id: string | null): ThreadDetail | null { } } +/** Detail waiting for an idle slot; undefined when no write is scheduled. */ +let pendingDetail: ThreadDetail | null | undefined; +/** Longest an idle write may wait on a renderer that never goes idle. */ +const IDLE_WRITE_TIMEOUT_MS = 2000; + +/** + * Stringifying a big transcript costs ~17 ms (#1475), so the write waits for + * an idle slot. The key holds one thread, so saves before that slot coalesce + * into one write of the latest detail. + */ export function saveCachedThreadDetail(d: ThreadDetail | null): void { - try { - if (d == null) window.localStorage.removeItem(DETAIL_KEY); - else window.localStorage.setItem(DETAIL_KEY, JSON.stringify(d)); - } catch { - // Quota/private mode: a switch just shows the empty pane until the fetch. + const scheduled = pendingDetail !== undefined; + pendingDetail = d; + if (scheduled) return; + const write = () => { + const next = pendingDetail; + pendingDetail = undefined; + try { + if (next == null) window.localStorage.removeItem(DETAIL_KEY); + else window.localStorage.setItem(DETAIL_KEY, JSON.stringify(next)); + } catch { + // Quota/private mode: a switch just shows the empty pane until the fetch. + } + }; + // Safari (web mode) has no requestIdleCallback. + if (typeof window.requestIdleCallback === "function") { + window.requestIdleCallback(write, { timeout: IDLE_WRITE_TIMEOUT_MS }); + } else { + window.setTimeout(write, 0); } } diff --git a/src/components/ThreadView.tsx b/src/components/ThreadView.tsx index 16a7e527e..2baff5384 100644 --- a/src/components/ThreadView.tsx +++ b/src/components/ThreadView.tsx @@ -87,7 +87,7 @@ import { import { clampWindowStart, ensureVisibleStart, - initialWindowStart, + tailWindowStart, } from "../transcriptWindow"; import { lastUserMessage } from "../retryTurn"; import { @@ -985,12 +985,14 @@ export const ThreadView = memo(function ThreadView({ /** * One integer: index of the first mounted timeline entry. Thread switches - * reset to the tail window; streaming appends leave it alone so the top - * does not creep; Show earlier / revealMessageId only move it down. + * reset to the tail window. Appends while stuck to the bottom advance it so + * the mounted tail stays bounded (#1475); scrolled up, it stays put so the + * content being read does not move. Show earlier / revealMessageId only + * move it down. */ const [windowThreadId, setWindowThreadId] = useState(threadId); const [windowStart, setWindowStart] = useState(() => - initialWindowStart(timeline.length), + tailWindowStart(timeline), ); const revealTargetId = revealMessageId ?? jumpMessageId; const revealIndex = useMemo(() => { @@ -1004,8 +1006,10 @@ export const ThreadView = memo(function ThreadView({ const start = clampWindowStart( ensureVisibleStart( threadId !== windowThreadId - ? initialWindowStart(timeline.length) - : windowStart, + ? tailWindowStart(timeline) + : stickToBottom.current + ? Math.max(windowStart, tailWindowStart(timeline)) + : windowStart, revealIndex, ), timeline.length, @@ -1014,7 +1018,7 @@ export const ThreadView = memo(function ThreadView({ setWindowThreadId(threadId); setWindowStart(start); setJumpMessageId(null); - } else if (start < windowStart) { + } else if (start !== windowStart) { setWindowStart(start); } diff --git a/src/components/thread/useStickToBottom.ts b/src/components/thread/useStickToBottom.ts index 4837ec30a..3d690708a 100644 --- a/src/components/thread/useStickToBottom.ts +++ b/src/components/thread/useStickToBottom.ts @@ -85,12 +85,15 @@ export function useStickToBottom({ /** * Pin before paint so a remounted body (thread switch) and a newly - * inserted permission card never flash at the wrong scrollTop. #408's - * ResizeObserver still covers post-paint growth. + * inserted permission card never flash at the wrong scrollTop. Plain + * appends skip this: reading scrollHeight here forces a synchronous + * layout on every streamed push (#1475), and #408's ResizeObserver pins + * them once the browser has laid out anyway. */ useLayoutEffect(() => { const id = detail?.thread.id ?? null; - if (id !== prevLayoutThreadId.current) { + const opened = id !== prevLayoutThreadId.current; + if (opened) { const switching = prevLayoutThreadId.current !== null && id !== null && @@ -114,7 +117,7 @@ export function useStickToBottom({ forceStick.current = true; } prevPermReq.current = req; - pinIfStuck(); + if (opened || forceStick.current) pinIfStuck(); }, [ timeline, isWorking, diff --git a/src/transcriptWindow.ts b/src/transcriptWindow.ts index 290633061..fb8cacd1e 100644 --- a/src/transcriptWindow.ts +++ b/src/transcriptWindow.ts @@ -3,10 +3,38 @@ * at the bottom, so we mount the last N timeline entries and grow upward * on demand. State is one integer (the first visible index). */ +import type { TimelineEntry } from "./timeline"; /** Last N entries on first paint. Covers several screens of a typical thread. */ export const TRANSCRIPT_WINDOW = 120; +/** + * Message text the tail window keeps mounted (#1475). A count alone let six + * streamed 45 KB answers grow one open transcript to 62k DOM nodes; rendered + * markdown runs ~140 nodes per KB, so this caps the window near 15k nodes. + * Tool output is not counted: tool cards mount collapsed. + */ +export const TRANSCRIPT_CHAR_BUDGET = 100_000; + +/** + * First index of the tail window: at most `windowSize` entries and at most + * `budget` characters of message text, but always the last entry. + */ +export function tailWindowStart( + timeline: readonly TimelineEntry[], + windowSize = TRANSCRIPT_WINDOW, + budget = TRANSCRIPT_CHAR_BUDGET, +): number { + const min = initialWindowStart(timeline.length, windowSize); + let chars = 0; + for (let i = timeline.length - 1; i >= min; i--) { + const entry = timeline[i]!; + if (entry.kind === "message") chars += entry.message.text.length; + if (chars > budget && i < timeline.length - 1) return i + 1; + } + return min; +} + /** First index to mount so the tail window is N entries (0 when it all fits). */ export function initialWindowStart( length: number, @@ -26,8 +54,7 @@ export function extendWindowStart( } /** - * Raise the window so `index` is included. Streaming appends do not call - * this — they keep the start index and grow the tail. + * Lower the window start so `index` is included. */ export function ensureVisibleStart(start: number, index: number): number { if (!Number.isFinite(index) || index < 0) return start; diff --git a/test/renderFirstBoot.test.tsx b/test/renderFirstBoot.test.tsx index eb1fe4ccd..88daf98d8 100644 --- a/test/renderFirstBoot.test.tsx +++ b/test/renderFirstBoot.test.tsx @@ -304,3 +304,48 @@ describe("render-first boot (#364)", () => { } }); }); + +describe("cached thread detail write (#1475)", () => { + it("writes on idle, coalescing saves into one write of the latest detail", async () => { + const shell = await mount(
); + const w = window as unknown as { + requestIdleCallback?: (cb: () => void, opts?: { timeout?: number }) => number; + }; + const prevIdle = w.requestIdleCallback; + const idle: Array<{ cb: () => void; timeout?: number }> = []; + w.requestIdleCallback = (cb, opts) => idle.push({ cb, timeout: opts?.timeout }); + // Assigning onto the Storage instance would just store an item. + const store = Object.getPrototypeOf(window.localStorage) as Storage; + const prevSet = store.setItem; + const writes: string[] = []; + store.setItem = function (key: string, value: string) { + if (key === DETAIL_KEY) writes.push(value); + return prevSet.call(this, key, value); + }; + try { + const { saveCachedThreadDetail, loadCachedThreadDetail } = await import( + "../src/bootSnapshot" + ); + const first = detail({ thread: thread({ id: "t-idle" }), messages: [marker("FIRST")] }); + const second = detail({ thread: thread({ id: "t-idle" }), messages: [marker("SECOND")] }); + saveCachedThreadDetail(first); + saveCachedThreadDetail(second); + assert.equal(writes.length, 0, "no synchronous setItem on the save path"); + assert.equal(idle.length, 1, "saves before the idle slot share one callback"); + assert.ok(idle[0]!.timeout! > 0, "the idle write has a timeout fallback"); + assert.equal( + loadCachedThreadDetail("t-idle")?.messages[0]?.text, + "SECOND", + "a load before the write sees the pending detail", + ); + idle[0]!.cb(); + assert.equal(writes.length, 1); + assert.ok(writes[0]!.includes("SECOND") && !writes[0]!.includes("FIRST")); + } finally { + store.setItem = prevSet; + if (prevIdle) w.requestIdleCallback = prevIdle; + else delete w.requestIdleCallback; + shell.unmount(); + } + }); +}); diff --git a/test/threadView.test.tsx b/test/threadView.test.tsx index b5022c32b..b11d4d186 100644 --- a/test/threadView.test.tsx +++ b/test/threadView.test.tsx @@ -2371,6 +2371,97 @@ describe("ThreadView stick-to-bottom on content resize (issue #408)", () => { ); m.unmount(); }); + + /** + * Issue #1475: reading scrollHeight in the pin layout effect forced a + * synchronous layout on every streamed push. Plain appends now leave the + * pin to the ResizeObserver; #607's two cases still pin before paint. + */ + it("pins thread switches and permission cards before paint but leaves appends to the ResizeObserver (#1475)", async () => { + const pending: PendingPermissionInfo = { + requestId: "req-1475", + toolName: "Bash", + summary: "Bash: npm test", + input: '{"command":"npm test"}', + command: "npm test", + }; + const threadB = detail({ + thread: thread({ id: "t2", title: "other thread" }), + messages: [msg({ id: "b1", role: "user", text: "THREAD_B", createdAt: 10 })], + }); + + function Harness() { + const [open, setOpen] = useState(sizedDetail()); + return ( +
+ + + + {view({ detail: open })} +
+ ); + } + + const m = await mount(); + const body = m.query(".body") as HTMLElement; + const layout = { clientHeight: 400, scrollHeight: 1000, scrollTop: 600 }; + fakeScrollMetrics(body, layout); + let reads = 0; + Object.defineProperty(body, "scrollHeight", { + configurable: true, + get: () => { + reads++; + return layout.scrollHeight; + }, + }); + + layout.scrollHeight = 1400; + await inAct(async () => { + (m.query("[data-append]") as HTMLButtonElement).click(); + }); + assert.ok(m.text().includes("APPENDED")); + assert.equal(reads, 0, "a plain append must not force a layout read"); + fireObservedResizes(); + assert.equal(layout.scrollTop, 1400, "the ResizeObserver pins the append"); + + layout.scrollHeight = 2400; + await inAct(async () => { + (m.query("[data-perm]") as HTMLButtonElement).click(); + }); + assert.ok(m.query("[data-permission-card]")); + assert.equal(layout.scrollTop, 2400, "a permission card pins before paint"); + + layout.scrollHeight = 3500; + await inAct(async () => { + (m.query("[data-switch]") as HTMLButtonElement).click(); + }); + assert.ok(m.text().includes("THREAD_B")); + assert.equal(m.query(".body"), body, "a direct switch keeps the scroll body"); + assert.equal(layout.scrollTop, 3500, "a thread switch pins before paint"); + m.unmount(); + }); }); /** @@ -3011,55 +3102,112 @@ describe("ThreadView transcript windowing (issue #564)", () => { m.unmount(); }); - it("appends a streamed message at the tail without revealing earlier entries", async () => { - const n = 500; + /** Thread of `n` bulk messages plus a button that appends one streamed tail message. */ + function streamHarness(n: number) { const initial = bulkMessages(n); - - function StreamHarness() { + let appended = 0; + return function StreamHarness() { const [messages, setMessages] = useState(initial); return (
{view({ detail: detail({ messages }) })}
); - } + }; + } - const m = await mount(); - assert.ok(!m.html().includes("#0#"), "pre-stream: oldest is windowed out"); - assert.ok(m.html().includes("#499#"), "pre-stream: tail is mounted"); + it("advances the window start on appends while stuck to the bottom (#1475)", async () => { + const n = 500; + const Harness = streamHarness(n); + const m = await mount(); + assert.ok(m.html().includes("#380#"), "pre-stream: head of the window mounted"); await m.click(m.query("[data-append-stream]")); + await m.click(m.query("[data-append-stream]")); + assert.ok(m.text().includes("STREAMED_TAIL_2"), "the streamed message mounts at the tail"); assert.ok( - m.text().includes("STREAMED_TAIL"), - "the streamed message mounts at the tail", + !m.html().includes("#380#") && !m.html().includes("#381#"), + "a pinned append drops the oldest mounted entries", ); assert.ok( - !m.html().includes("#0#"), - "append must not extend the top of the window", + m.text().includes("Show earlier — 382 messages"), + "the mounted tail stays at TRANSCRIPT_WINDOW entries", ); + m.unmount(); + }); + + it("keeps the window start on appends after the user scrolled up (#1475)", async () => { + const n = 500; + const Harness = streamHarness(n); + const m = await mount(); + const body = m.query(".body") as HTMLElement; + fakeScrollMetrics(body, { clientHeight: 400, scrollHeight: 20_000, scrollTop: 100 }); + body.dispatchEvent(new Event("scroll")); + await m.click(m.query("[data-append-stream]")); + assert.ok(m.text().includes("STREAMED_TAIL_1")); + assert.ok( + m.html().includes("#380#"), + "content above a reader who scrolled up must not be unmounted", + ); + assert.ok(m.text().includes("Show earlier — 380 messages")); + m.unmount(); + }); + + it("Show earlier still pages older entries in after the head was trimmed (#1475)", async () => { + const n = 500; + const Harness = streamHarness(n); + const m = await mount(); + await m.click(m.query("[data-append-stream]")); + assert.ok(m.text().includes("Show earlier — 381 messages")); + await m.click(m.query("[data-show-earlier]")); + assert.ok(m.text().includes("Show earlier — 261 messages")); + assert.ok(m.html().includes("#261#") && m.html().includes("#380#")); + // Show earlier unsticks, so the next append must not trim it away again. + await m.click(m.query("[data-append-stream]")); + assert.ok(m.text().includes("STREAMED_TAIL_2")); assert.ok( - m.text().includes("Show earlier — 380 messages"), - "hidden count stays put across a tail append", + m.text().includes("Show earlier — 261 messages"), + "paged-in entries survive an append", ); m.unmount(); }); + it("bounds the mounted tail by message text, not just entry count (#1475)", async () => { + const answers = Array.from({ length: 6 }, (_, i) => + msg({ + id: `long-${i}`, + role: "assistant", + text: `LONG_ANSWER_${i} ${"word ".repeat(9_000)}`, + createdAt: 10 + i, + }), + ); + const html = render({ detail: detail({ messages: answers }) }); + assert.ok(html.includes("LONG_ANSWER_5") && html.includes("LONG_ANSWER_4")); + assert.ok( + !html.includes("LONG_ANSWER_0"), + "six 45 KB answers must not all mount", + ); + assert.ok(html.includes("data-show-earlier")); + }); + it("extends the window to include a jump-to-anchor above it", async () => { const n = 500; const messages = bulkMessages(n); diff --git a/test/transcriptWindow.test.ts b/test/transcriptWindow.test.ts index 6ac321d0b..de6fe6ad4 100644 --- a/test/transcriptWindow.test.ts +++ b/test/transcriptWindow.test.ts @@ -5,12 +5,22 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; import { + TRANSCRIPT_CHAR_BUDGET, TRANSCRIPT_WINDOW, clampWindowStart, ensureVisibleStart, extendWindowStart, initialWindowStart, + tailWindowStart, } from "../src/transcriptWindow.ts"; +import type { TimelineEntry } from "../src/timeline.ts"; + +const entries = (sizes: number[]): TimelineEntry[] => + sizes.map((n, i) => ({ + kind: "message", + timestamp: i, + message: { id: `m${i}`, role: "assistant", text: "x".repeat(n), createdAt: i }, + })); describe("transcriptWindow", () => { it("starts at 0 when the timeline fits in one window", () => { @@ -40,4 +50,18 @@ describe("transcriptWindow", () => { assert.equal(clampWindowStart(380, 10), 0); assert.equal(clampWindowStart(380, 0), 0); }); + + it("tail window stops at the character budget (#1475)", () => { + // Six 45 KB streamed answers: a 120-entry window would mount all six. + const big = entries([...Array(200).fill(10), ...Array(6).fill(45_000)]); + const start = tailWindowStart(big); + assert.equal(big.length - start, Math.floor(TRANSCRIPT_CHAR_BUDGET / 45_000)); + }); + + it("tail window keeps the count cap for small entries and always the last entry", () => { + assert.equal(tailWindowStart(entries(Array(500).fill(10))), 500 - TRANSCRIPT_WINDOW); + assert.equal(tailWindowStart(entries(Array(40).fill(10))), 0); + const huge = entries([10, 10, TRANSCRIPT_CHAR_BUDGET * 3]); + assert.equal(tailWindowStart(huge), 2); + }); });