From c04aaecb513fb9175c7d4888afc3f1e058b77832 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Thu, 11 Jun 2026 12:17:47 +0530 Subject: [PATCH] =?UTF-8?q?opentui(v6):=20windowing=20S2=20=E2=80=94=20pin?= =?UTF-8?q?=20selected=20rows=20instead=20of=20freezing=20on=20a=20lingeri?= =?UTF-8?q?ng=20highlight?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial-review follow-up to the S2 slice. The S1 rule froze ALL window recomputes while renderer.getSelection()?.isActive — but a finished mouse selection persists by design (boundary/renderer.ts keeps the highlight so Ctrl+C can re-copy), so a long streaming turn behind a lingering highlight ballooned the mounted set exactly like pre-windowing (and permanently ratcheted the Yoga-WASM high-water). Refinement: - full freeze only while selection.isDragging (the native walk touches the live tree on every drag update — destroying a row mid-walk corrupts the highlight; unchanged from S1 where it matters), - a finished highlight instead PINS the rows containing selection.selectedRenderables (parent-climb to the row wrapper via a WeakMap) as neverWindow — the highlight and a later Ctrl+C copy stay byte-exact while everything else keeps windowing, - an active highlight counts as activity (no idle measure churn under it). Test (headless mock-mouse drag): finished selection persists (isActive, !isDragging) → 300-row burst keeps peakMounted < 120 AND getSelectedText() returns the identical text afterward, the selected rows having been pinned while long scrolled past the margin. Verified on this build: gate digest otui-capped d5e9558583159eac… (2/2), mem2000 otui-capped windowing-ON vmhwm 312MB (target ≤ 350), scroll2000 otui-capped p50 2.0ms / p99 6.0ms (gate ≤ 17ms). check exit 0 (648 tests). Review verdicts on the remaining findings (verified against core source): - "scrollTop compensation race": rejected — scrollTop is an imperative scrollbar property (no signal staleness); records fire in document order, each compensation immediately visible to the next. - "heights map leak on /new": rejected — the countChanged cleanup prunes every per-key map against the live key set (test-verified). - remount-in-viewport estimate shift: only reachable when one frame jumps past the margin (> 1 viewport); the design's accepted "remounted for view" path — documented in the header. - expanded tool/reasoning re-collapse on far-remount: S1-accepted, deferred (component-local state; out of S2 file ownership). --- ui-opentui/src/test/lib/render.ts | 6 +++ ui-opentui/src/test/transcriptWindow.test.tsx | 41 ++++++++++++++ ui-opentui/src/view/transcript.tsx | 53 +++++++++++++++---- 3 files changed, 90 insertions(+), 10 deletions(-) diff --git a/ui-opentui/src/test/lib/render.ts b/ui-opentui/src/test/lib/render.ts index d4bd8638527..37e53473396 100644 --- a/ui-opentui/src/test/lib/render.ts +++ b/ui-opentui/src/test/lib/render.ts @@ -65,6 +65,10 @@ export interface RenderProbe { readonly scroll: (x: number, y: number, direction: 'up' | 'down') => Promise /** The mock keyboard (typeText / pressArrow / pressEnter / …) — pair with `settle()`. */ readonly keys: TestRendererSetup['mockInput'] + /** The full mock mouse (drag / pressDown / release / …) — selection tests. */ + readonly mouse: TestRendererSetup['mockMouse'] + /** The live test renderer (e.g. `getSelection()` assertions). */ + readonly renderer: TestRendererSetup['renderer'] /** Run a render pass + flush so simulated input lands in the next `frame()`. */ readonly settle: () => Promise readonly destroy: () => void @@ -121,6 +125,8 @@ export async function renderProbe( await setup.flush() }, keys: setup.mockInput, + mouse: setup.mockMouse, + renderer: setup.renderer, settle: async () => { await setup.renderOnce() await setup.flush() diff --git a/ui-opentui/src/test/transcriptWindow.test.tsx b/ui-opentui/src/test/transcriptWindow.test.tsx index 668be60258a..31f941f7a96 100644 --- a/ui-opentui/src/test/transcriptWindow.test.tsx +++ b/ui-opentui/src/test/transcriptWindow.test.tsx @@ -196,6 +196,47 @@ describe('transcript windowing — S2 append-time adjudication', () => { }, 120_000) }) +describe('transcript windowing — S2 selection: drag freezes, a finished highlight only pins its rows', () => { + test('a persisting (finished) selection does not freeze windowing; its rows stay mounted for copy', async () => { + const store = seedRows(60) + const on = await mountTranscript(store, '1') + try { + // drag-select across a visible row's TEXT line (rows interleave with + // margin lines — find one from the frame), then release — the highlight + // PERSISTS by design (boundary/renderer.ts keeps it so Ctrl+C re-copies). + const textY = on.probe + .frame() + .split('\n') + .findIndex(line => line.includes('marker')) + expect(textY).toBeGreaterThanOrEqual(0) + await on.probe.mouse.drag(3, textY, 30, textY + 2) + await on.probe.settle() + const selection = on.probe.renderer.getSelection() + expect(selection?.isActive).toBe(true) + expect(selection?.isDragging).toBe(false) + const copied = selection?.getSelectedText() ?? '' + expect(copied).toContain('marker') + + // burst 300 appends while the highlight lingers: windowing must keep + // adjudicating (the S1 full freeze would balloon the mounted set)… + resetWindowRowStats() + for (let i = 0; i < 300; i++) { + store.pushSystem(`late-${i} marker`) + if (i % 50 === 49) await on.probe.settle() + } + for (let i = 0; i < 6; i++) await on.probe.settle() + expect(windowRowStats().peakMounted).toBeLessThan(120) + + // …while the selected row — long scrolled out past the margin — stays + // PINNED: the highlight's renderables are alive and Ctrl+C still copies + // the exact same text. + expect(on.probe.renderer.getSelection()?.getSelectedText()).toBe(copied) + } finally { + on.probe.destroy() + } + }, 60_000) +}) + describe('transcript windowing — S2 windowed resume (commitSnapshot)', () => { function snapshot(n: number): Message[] { const out: Message[] = [] diff --git a/ui-opentui/src/view/transcript.tsx b/ui-opentui/src/view/transcript.tsx index a7c0c14738a..35b6930e51a 100644 --- a/ui-opentui/src/view/transcript.tsx +++ b/ui-opentui/src/view/transcript.tsx @@ -43,9 +43,12 @@ * BOTTOM_ALWAYS_MOUNTED of the transcript or streaming (live rows paint * instantly with zero added latency); a row created deep in a bulk snapshot * (resume `commitSnapshot`) starts as an ESTIMATE spacer — a resumed 2k - * session mounts only the bottom window. While a mouse selection is live the - * window FREEZES (no swaps — a swap would destroy highlighted renderables out - * from under the native selection walk). + * session mounts only the bottom window. While a mouse selection is being + * DRAGGED the window FREEZES (no swaps — a swap would destroy highlighted + * renderables out from under the native selection walk); once the drag + * finishes, the persisting highlight only PINS the rows containing selected + * renderables (highlight + later Ctrl+C copy stay exact) so a streaming turn + * can't balloon the mounted set behind a lingering selection. * * Spacer corrections (S2, the zero-jank rule): when a remount/measure lands a * height different from what the spacer occupied, the wrapper's onSizeChange @@ -75,7 +78,11 @@ * * Known S2 limits (documented, deferred): /compact·/details toggles and width * resizes leave out-of-window spacer heights stale until remount or the idle - * march (resize invalidation is S3, design §5). + * march (resize invalidation is S3, design §5). A discrete scroll jump larger + * than the margin in one frame remounts a mis-estimated row already inside + * the viewport — the in-viewport correction is forbidden (jank rule), so that + * single user-caused frame absorbs the estimate error (the design's accepted + * "remounted for view" path). */ import type { BoxRenderable, ScrollBoxRenderable } from '@opentui/core' import { useRenderer } from '@opentui/solid' @@ -183,8 +190,10 @@ export function Transcript(props: { store: SessionStore }) { // anything deeper (a bulk resume snapshot) starts as an estimate spacer. const defaults = new Map() // key → the row's live measuring wrapper (the idle measure pull below reads - // post-layout heights for batch rows whose mount changed nothing). + // post-layout heights for batch rows whose mount changed nothing); and the + // reverse map (wrapper → key) for pinning rows under a persisting selection. const wrappers = new Map() + const wrapperKeys = new WeakMap() // key → cached settled-row height estimate (estimateFor scans the row text — // caching keeps the per-append adjudication O(rows), not O(text)). Tagged // with the /compact flag it was computed under. @@ -245,13 +254,36 @@ export function Transcript(props: { store: SessionStore }) { const tick = (force = false): void => { const sb = scroll() if (!sb) return - // Selection freeze: the native selection walks the LIVE tree — swapping a - // row out (destroying its renderables) mid-selection would corrupt the - // highlight/copy. Frozen ≠ broken: unseen new rows default to mounted. - if (renderer.getSelection()?.isActive) { + // Selection handling (S2 refinement of the S1 full freeze): + // - while DRAGGING, the native selection walks the LIVE tree on every + // update — swapping a row out (destroying its renderables) mid-walk + // would corrupt the highlight/copy. Full freeze, as in S1. + // - a FINISHED highlight persists by design (boundary/renderer.ts keeps + // it so Ctrl+C can re-copy). An indefinite full freeze would let a + // burst-streaming turn balloon the mounted set (the S1 hole this slice + // closes), so instead only the rows that CONTAIN selected renderables + // are pinned (never windowed) — the highlight and a later Ctrl+C copy + // stay exact, and everything else keeps windowing. + const selection = renderer.getSelection() + if (selection?.isActive && selection.isDragging) { lastActivityAt = Date.now() return } + const pinned = new Set() + if (selection?.isActive) { + lastActivityAt = Date.now() // a live highlight is activity: no idle pulses + for (const renderable of selection.selectedRenderables) { + let node: unknown = renderable + while (node && typeof node === 'object') { + const key = wrapperKeys.get(node) + if (key !== undefined) { + pinned.add(key) + break + } + node = (node as { parent?: unknown }).parent + } + } + } const viewportHeight = sb.viewport.height if (viewportHeight <= 0) return const messages = props.store.state.messages @@ -303,7 +335,7 @@ export function Transcript(props: { store: SessionStore }) { // here (the override lives in component-local signals — toolPart.tsx/ // reasoningPart.tsx); expanded rows far above the viewport may // re-collapse on remount. Accepted for S1. - neverWindow: (message.streaming ?? false) || (running && i === messages.length - 1) + neverWindow: (message.streaming ?? false) || (running && i === messages.length - 1) || pinned.has(key) } }) // Pinned to the bottom (sticky region): anchor the window to the content @@ -428,6 +460,7 @@ export function Transcript(props: { store: SessionStore }) { ref={el => { wrapper = el wrappers.set(key, el) + wrapperKeys.set(el, key) }} style={{ flexDirection: 'column', flexShrink: 0 }} onSizeChange={record}