mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-09 13:21:42 +00:00
opentui(v6): windowing S2 — pin selected rows instead of freezing on a lingering highlight
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).
This commit is contained in:
parent
eaa069e322
commit
c04aaecb51
3 changed files with 90 additions and 10 deletions
|
|
@ -65,6 +65,10 @@ export interface RenderProbe {
|
|||
readonly scroll: (x: number, y: number, direction: 'up' | 'down') => Promise<void>
|
||||
/** 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<void>
|
||||
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()
|
||||
|
|
|
|||
|
|
@ -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[] = []
|
||||
|
|
|
|||
|
|
@ -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<number, boolean>()
|
||||
// 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<number, BoxRenderable>()
|
||||
const wrapperKeys = new WeakMap<object, number>()
|
||||
// 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<number>()
|
||||
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}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue