From 6bfea62e4b33dfde8018df7a7c5cd93ff65622c0 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 5 Nov 2026 00:31:47 +0100 Subject: [PATCH] fix(p2): TUI transcript render survives a sync from its own callbacks onTop/onMissingAnchor/onVisibleGap load history, which syncs the transcript synchronously through runtime.changed(). The gap loop then indexed the new order against the old line starts and crashed the TUI sandbox when switching sessions. Render now works on the views it started with. --- apps/p2/src/tui/components/transcript.ts | 12 +++-- apps/p2/test/p2/tui-transcript.test.ts | 57 ++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 apps/p2/test/p2/tui-transcript.test.ts diff --git a/apps/p2/src/tui/components/transcript.ts b/apps/p2/src/tui/components/transcript.ts index 45be086c7..c4dcae641 100644 --- a/apps/p2/src/tui/components/transcript.ts +++ b/apps/p2/src/tui/components/transcript.ts @@ -437,13 +437,17 @@ export class TranscriptView implements Component { const lines: string[] = this.header ? [...this.header.render(width)] : []; const starts = new Map(); const rendered: { key: string; start: number; entryId?: number }[] = []; - this.order.forEach((view, index) => { - const key = this.keys[index]!; + // The callbacks below may sync synchronously (store change -> runtime.changed()) and replace `this.order`: + // everything after this point works on the views that were actually rendered. + const order = this.order; + const keys = this.keys; + order.forEach((view, index) => { + const key = keys[index]!; starts.set(key, lines.length); rendered.push({ key, start: lines.length, entryId: itemEntryId(view.item) }); lines.push(...view.component.render(width)); }); - if (this.order.length === 0 && this.empty && !this.tail) lines.push("", ` ${this.empty}`); + if (order.length === 0 && this.empty && !this.tail) lines.push("", ` ${this.empty}`); if (this.tail) lines.push(...this.tail.render(width)); // One empty line between the transcript and the dock (DTUI). lines.push(""); @@ -472,7 +476,7 @@ export class TranscriptView implements Component { // At the top of a laid-out viewport (or everything fits): older history may continue above. if (scroll && scroll.viewportHeight > 0 && scroll.scrollTop === 0 && !this.pendingAnchor) this.onTop?.(); if (scroll && scroll.viewportHeight > 0 && !this.pendingAnchor) - this.order.forEach((view, index) => { + order.forEach((view, index) => { const start = rendered[index]!.start; const end = rendered[index + 1]?.start ?? lines.length; const visible = end > scroll.scrollTop && start < scroll.scrollTop + scroll.viewportHeight; diff --git a/apps/p2/test/p2/tui-transcript.test.ts b/apps/p2/test/p2/tui-transcript.test.ts new file mode 100644 index 000000000..021ccc0d2 --- /dev/null +++ b/apps/p2/test/p2/tui-transcript.test.ts @@ -0,0 +1,57 @@ +// The TUI transcript component: a render whose callbacks (onTop, onMissingAnchor, onVisibleGap) sync the transcript +// synchronously (store change -> runtime.changed() -> sync) must finish on the views it rendered. +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { type EntryId, type EntryRecord, ROOT_CONVERSATION_ID } from "@earendil-works/pi-durable"; +import { ScrollView } from "@earendil-works/pi-tui"; +import { TranscriptView } from "../../src/tui/components/transcript.ts"; +import { ThemeRef } from "../../src/tui/theme/ref.ts"; +import { BUILTIN_THEMES, createTheme } from "../../src/tui/theme/theme.ts"; +import { buildTranscript } from "../../src/web/transcript.ts"; +import { wholeView } from "../fixtures/whole-view.ts"; + +const user = (id: number): EntryRecord => + ({ + id: id as EntryId, + conversationId: ROOT_CONVERSATION_ID, + kind: "pi.user", + model: [{ role: "user", content: `message ${id}`, timestamp: id }], + }) as EntryRecord; +const transcriptOf = (ids: number[]) => + buildTranscript( + wholeView({ + conversation: { id: ROOT_CONVERSATION_ID } as Parameters[0]["conversation"], + entries: ids.map(user), + docs: {}, + }), + ); + +test("a render whose onTop syncs a longer transcript does not crash", () => { + const view = new TranscriptView( + { + theme: new ThemeRef(createTheme(BUILTIN_THEMES.dark!, "truecolor")), + keyText: () => "ctrl+o", + requestRender: () => {}, + cwd: "/work", + markdownTransformers: [], + toolRenderers: () => undefined, + entryRenderers: new Map(), + }, + { expanded: false, showThinking: false }, + ); + const scroll = new ScrollView(view, { follow: "end" }); + view.attach(scroll); + view.sync(transcriptOf([1])); + // Everything fits: the viewport is at the top, so render asks for older history. + scroll.updateLayout(1, 40, () => {}); + let synced = false; + view.onTop = () => { + if (synced) return; + synced = true; + // As `loadEarlier` does through the store and runtime listeners, before the render returns. + view.sync(transcriptOf([1, 2, 3, 4])); + }; + view.render(80); + assert.equal(synced, true); + assert.match(view.render(80).join("\n"), /message 4/); +});