From ca57ab07b67e7b992db4653ac3246169456b7c0d Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Fri, 9 Oct 2026 23:57:14 -0400 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Reuse=20the=20reading?= =?UTF-8?q?=20geometry=20a=20keystroke=20did=20not=20change=20(#881)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/cli/src/repl/application.ts | 21 ++ packages/cli/src/repl/fitting.ts | 278 +++++++++++++++--- packages/cli/src/repl/program.ts | 86 ++++-- .../cli/tests/fixtures/repl/presentation.ts | 52 ++++ packages/cli/tests/repl-entries.test.ts | 81 +++++ packages/cli/tests/repl-journey.test.ts | 60 +++- .../cli/tests/repl-source-reading.test.ts | 133 ++++++++- 7 files changed, 643 insertions(+), 68 deletions(-) diff --git a/packages/cli/src/repl/application.ts b/packages/cli/src/repl/application.ts index 6eede758..cf2e9475 100644 --- a/packages/cli/src/repl/application.ts +++ b/packages/cli/src/repl/application.ts @@ -898,6 +898,27 @@ const EMPTY_MODEL: ReplModel = Object.freeze({ sessions: Object.freeze([]), }); +/** + * Whether answering this action needs the frame's measured admission. + * + * The four scrolls, and nothing else: every other branch of `reduceRepl` + * decides from the state, the model and this process's overlay alone. It + * lives here because the reducer is what reads `admission`, so a fifth + * branch that starts reading one is written next to the answer about it. + * + * The root measures a frame before the action when this is true. For a + * keystroke into the draft it is false, and measuring the reading beside that + * draft would be work for an answer nothing consults. + */ +export function readsGeometry(action: ReplAction): boolean { + return ( + action.kind === "scroll" || + action.kind === "scroll-sessions" || + action.kind === "scroll-entries" || + action.kind === "scroll-reading" + ); +} + /** * Answer one action. * diff --git a/packages/cli/src/repl/fitting.ts b/packages/cli/src/repl/fitting.ts index 7777f6f1..3688fd7d 100644 --- a/packages/cli/src/repl/fitting.ts +++ b/packages/cli/src/repl/fitting.ts @@ -113,6 +113,103 @@ export interface ReplPreparedReading { readonly rows: readonly ReplPreparedRow[]; } +/** + * Where one logical line was cut, and how wide the piece measured. + * + * The measured half of a row, and the only half an engine answer decides. + * Which text it holds comes from the line it belongs to, so this stays true + * for as long as that line's text and depth do. + */ +export interface ReplRowCut { + /** The offset into the logical line this piece starts at. */ + readonly from: number; + /** How many code units of it this piece holds. */ + readonly length: number; + /** Whether a previous piece already carried the start of this line. */ + readonly continuation: boolean; + /** The column this line was written at, carried onto its continuations. */ + readonly indent: number; + /** What the engine measured this exact piece as. */ + readonly width: number; +} + +/** + * Everything about a reading that measurement decides. + * + * The reservation and one cut list per logical line, in order. It is a + * function of the lines' text and depth, the room they were fitted to, and + * the terminal size — and of nothing else. A badge arriving, a phase + * settling, a selection moving and a position being inspected all leave it + * exactly as it was, which is why it can be kept and reused. + */ +export interface ReplFitGeometry { + readonly reservation: ReplFitReservation; + readonly cuts: readonly (readonly ReplRowCut[])[]; +} + +/** + * The geometry one running REPL is holding on to. + * + * Typing moves the draft, not the reading beside it, and a reading is the + * expensive thing to fit: a two-hundred-line entry costs an engine render per + * line plus a binary search per overflowing row, and a keystroke used to pay + * that twice. This keeps the last answer and the exact inputs it was computed + * from, so a frame whose reading has not moved reuses it. + * + * Inputs are compared, not digested. A few hundred short string comparisons + * are far below one render, and a digest would trade a guarantee for a + * cheaper comparison that is already cheap. + */ +export interface ReplFitStore { + /** The geometry retained for these exact inputs, or none. */ + held( + lines: readonly ReplReadingLine[], + room: number, + size: ReplTerminalSize, + ): ReplFitGeometry | undefined; + /** Retain this geometry as the answer for these inputs. */ + keep( + lines: readonly ReplReadingLine[], + room: number, + size: ReplTerminalSize, + geometry: ReplFitGeometry, + ): void; +} + +/** A store holding one reading's geometry, owned by whoever creates it. */ +export function createFitStore(): ReplFitStore { + let held: + | { + readonly shape: readonly { readonly text: string; readonly depth: number }[]; + readonly room: number; + readonly columns: number; + readonly rows: number; + readonly geometry: ReplFitGeometry; + } + | undefined; + const same = (lines: readonly ReplReadingLine[], room: number, size: ReplTerminalSize) => + held !== undefined && + held.room === room && + held.columns === size.columns && + held.rows === size.rows && + held.shape.length === lines.length && + held.shape.every((one, at) => one.text === lines[at].text && one.depth === lines[at].depth); + return { + held(lines, room, size) { + return same(lines, room, size) ? held?.geometry : undefined; + }, + keep(lines, room, size, geometry) { + held = { + shape: lines.map((one) => ({ text: one.text, depth: one.depth })), + room, + columns: size.columns, + rows: size.rows, + geometry, + }; + }, + }; +} + /** The rail glyph, and the gap between it and the text beside it. */ const RAIL = "│"; const SEPARATOR = " "; @@ -216,13 +313,40 @@ export function* prepareReading( size: ReplTerminalSize, lines: readonly ReplReadingLine[], room: number, + store?: ReplFitStore, ): Operation> { + const retained = store?.held(lines, room, size); + if (retained !== undefined) { + return Ok(decorate(lines, retained)); + } + const measured = yield* fitGeometry(renderer, size, lines, room); + if (!measured.ok) { + return measured; + } + store?.keep(lines, room, size, measured.value); + return Ok(decorate(lines, measured.value)); +} + +/** + * Everything measurement decides about a reading, and nothing else. + * + * Every engine render this file performs for a reading happens here. What + * comes back says where each line was cut and how wide each piece is; which + * rail, badge and roles those pieces are drawn with is decided afterwards, + * from the lines as they stand at the time. + */ +function* fitGeometry( + renderer: ReplRenderer, + size: ReplTerminalSize, + lines: readonly ReplReadingLine[], + room: number, +): Operation> { const reserved = yield* reserveFor(renderer, size, room); if (!reserved.ok) { return reserved; } const reservation = reserved.value; - const cut: ReplPreparedRow[] = []; + const cuts: (readonly ReplRowCut[])[] = []; // Nesting is drawn inside the content region rather than beside it, so a // deeply nested element loses text room rather than pushing the status column // off the pane it was measured against. @@ -232,34 +356,81 @@ export function* prepareReading( if (!fitted.ok) { return fitted; } - cut.push(...fitted.value); + cuts.push(fitted.value); } // The finished candidates, measured again as the text they ended up being. // The search above narrowed over prefixes; this is the row, and a row that // does not fit the region it was cut for is a frame refusal rather than // something to clip. - const measured = yield* widths( - renderer, - size, - cut.map((one) => one.text), + const flat = cuts.flatMap((one, at) => + one.map((cut) => lines[at].text.slice(cut.from, cut.from + cut.length)), ); - if (!measured.ok) { - return measured; + const widest = yield* widths(renderer, size, flat); + if (!widest.ok) { + return widest; } + const sized: (readonly ReplRowCut[])[] = []; + let index = 0; + for (const [at, one] of cuts.entries()) { + const line: ReplRowCut[] = []; + for (const cut of one) { + const width = widest.value[index]; + index += 1; + const indent = Math.min( + lines[at].depth * 2 + cut.indent, + Math.max(0, reservation.content - 1), + ); + if (width > reservation.content - indent) { + return Err( + new ReplFitRefusal( + `row ${keyOf(lines[at], cut)} measured ${width} columns in ${ + reservation.content - indent + }`, + ), + ); + } + line.push(Object.freeze({ ...cut, width })); + } + sized.push(Object.freeze(line)); + } + return Ok(Object.freeze({ reservation, cuts: Object.freeze(sized) })); +} + +/** + * The rows these lines are drawn as, over geometry already measured. + * + * The live half: rails, badges, roles and token runs come from the lines as + * they stand now, so an element settling or a position being inspected shows + * immediately over cuts that were measured once. + */ +function decorate( + lines: readonly ReplReadingLine[], + geometry: ReplFitGeometry, +): ReplPreparedReading { const rows: ReplPreparedRow[] = []; - for (const [index, one] of cut.entries()) { - const width = measured.value[index]; - const indent = Math.min(one.depth * 2 + one.indent, Math.max(0, reservation.content - 1)); - if (width > reservation.content - indent) { - return Err( - new ReplFitRefusal( - `row ${one.key} measured ${width} columns in ${reservation.content - indent}`, - ), + for (const [at, one] of geometry.cuts.entries()) { + const line = lines[at]; + for (const cut of one) { + rows.push( + Object.freeze({ + ...row( + line, + line.text.slice(cut.from, cut.from + cut.length), + cut.from, + cut.continuation, + cut.indent, + ), + width: cut.width, + }), ); } - rows.push(Object.freeze({ ...one, width })); } - return Ok(Object.freeze({ reservation, rows: Object.freeze(rows) })); + return Object.freeze({ reservation: geometry.reservation, rows: Object.freeze(rows) }); +} + +/** The key a cut of this line is drawn under. */ +function keyOf(line: ReplReadingLine, cut: ReplRowCut): string { + return cut.continuation ? `${line.key}+${cut.from}` : line.key; } /** One line of read-only text, fitted to a width. */ @@ -302,31 +473,34 @@ export function* fitPlain( const fitted: ReplFittedLine[] = []; for (const one of lines) { const runs = one.runs ?? tokenRuns([{ text: one.text, token: "source" }]); - const rows = yield* fitLine( - renderer, - size, - Object.freeze({ - key: one.key, - text: one.text, - runs, - rail: "rail-pending" as const, - depth: 0, - badge: undefined, - style: Object.freeze({ role: "source" as const, selected: false, inspected: false }), - }), - room, - ); - if (!rows.ok) { - return rows; + const line = Object.freeze({ + key: one.key, + text: one.text, + runs, + rail: "rail-pending" as const, + depth: 0, + badge: undefined, + style: Object.freeze({ role: "source" as const, selected: false, inspected: false }), + }); + const cuts = yield* fitLine(renderer, size, line, room); + if (!cuts.ok) { + return cuts; } - for (const row of rows.value) { + for (const cut of cuts.value) { + const drawn = row( + line, + line.text.slice(cut.from, cut.from + cut.length), + cut.from, + cut.continuation, + cut.indent, + ); fitted.push( Object.freeze({ - key: row.key, - text: row.text, - runs: row.runs, - continuation: row.continuation, - indent: row.indent, + key: drawn.key, + text: drawn.text, + runs: drawn.runs, + continuation: drawn.continuation, + indent: drawn.indent, }), ); } @@ -334,24 +508,29 @@ export function* fitPlain( return Ok(Object.freeze(fitted)); } -/** One logical line as its rows. */ +/** + * One logical line as the pieces it is cut into. + * + * Widths are filled in by the pass that measures every finished piece at once; + * nothing reads them in between. + */ function* fitLine( renderer: ReplRenderer, size: ReplTerminalSize, line: ReplReadingLine, room: number, -): Operation> { +): Operation> { // A blank line is a row. Measuring it would ask the engine the width of // nothing, and dropping it would edit the shape of somebody's document. if (line.text.length === 0) { - return Ok([row(line, "", 0, false, 0)]); + return Ok([piece(0, 0, false, 0)]); } const whole = yield* widths(renderer, size, [line.text]); if (!whole.ok) { return whole; } if (whole.value[0] <= room) { - return Ok([row(line, line.text, 0, false, 0)]); + return Ok([piece(0, line.text.length, false, 0)]); } // Where this line begins, so its continuations begin there too. Bounded well // inside the region: an indent that left no room for text would make a row @@ -360,7 +539,7 @@ function* fitLine( line.text.length - line.text.trimStart().length, Math.max(0, Math.floor(room / 2)), ); - const rows: ReplPreparedRow[] = []; + const cuts: ReplRowCut[] = []; let at = 0; while (at < line.text.length) { const rest = line.text.slice(at); @@ -368,10 +547,15 @@ function* fitLine( if (!cut.ok) { return cut; } - rows.push(row(line, rest.slice(0, cut.value), at, at > 0, hanging)); + cuts.push(piece(at, cut.value, at > 0, at > 0 ? hanging : 0)); at += cut.value; } - return Ok(Object.freeze(rows)); + return Ok(Object.freeze(cuts)); +} + +/** One cut, before the pass that measures it. */ +function piece(from: number, length: number, continuation: boolean, indent: number): ReplRowCut { + return Object.freeze({ from, length, continuation, indent, width: 0 }); } /** diff --git a/packages/cli/src/repl/program.ts b/packages/cli/src/repl/program.ts index b1566b72..4f48fbf8 100644 --- a/packages/cli/src/repl/program.ts +++ b/packages/cli/src/repl/program.ts @@ -56,6 +56,7 @@ import { presentationFor, readinessOf, readingOf, + readsGeometry, reduceRepl, refusedView, stateFor, @@ -83,7 +84,8 @@ import type { ReplReading, ReplSession } from "./session.ts"; import { useReplFrames } from "./frame.ts"; import type { ReplFrames } from "./frame.ts"; import { committedOps, flatten, profileFor, skeletonOps } from "./layout.ts"; -import { fitPlain, prepareReading } from "./fitting.ts"; +import { createFitStore, fitPlain, prepareReading } from "./fitting.ts"; +import type { ReplFitStore } from "./fitting.ts"; import { prepareRail } from "./history-rail.ts"; import { readingLines } from "./source-reading.ts"; import { sourceRuns } from "./presentation-text.ts"; @@ -420,6 +422,10 @@ function* drive( // separately. Everything below assigns both halves together or neither. let reading: ReplReading = { model: session.model, navigation: session.navigation }; let rendered: ReplRendered | undefined; + // The reading geometry this command has already measured. Owned here, so it + // lives exactly as long as the command does and no frame outside it can + // reach one process's cuts. + const fits = createFitStore(); const outcome = yield* scoped(function* (): Operation> { const screen = yield* useReplScreen(); @@ -474,6 +480,13 @@ function* drive( function* act(event: ReplInputEvent): Operation { const dispatched = yield* tree.dispatch(event); if (!dispatched.ok || dispatched.value.outcome !== "action") { + // A traversal has already moved focus in the tree. Taking it now means + // the frame below is composed with the focus that is actually held — + // the alternative is a frame marking the control the reader has just + // left, followed by a second whole frame correcting it. + if (dispatched.ok && dispatched.value.outcome === "focus") { + focused = keyOfFocus(tree); + } return false; } // Measured for the size and the reading this action is being answered at, @@ -481,19 +494,28 @@ function* drive( // showing, so the capacity it moves within has to be this frame's and not // the one a resize or a filter left behind. This preparation subscribes to // no frame and acknowledges nothing — it is a question about geometry. - const ready = yield* prepareFrame( - renderer, - yield* build(state, reading, current, yield* screen.size(), focused), - ); - if (!ready.ok) { - throw ready.error; + // + // Only the actions that ask the question pay for the answer. Typing into + // the draft moves no window, and measuring the reading beside it was the + // single most expensive thing a keystroke did. + let geometry = NOTHING_ADMITTED; + if (readsGeometry(dispatched.value.action)) { + const ready = yield* prepareFrame( + renderer, + yield* build(state, reading, current, yield* screen.size(), focused), + fits, + ); + if (!ready.ok) { + throw ready.error; + } + geometry = ready.value.admission; } const transition = reduceRepl( state, dispatched.value.action, reading.model, liveOf(current), - ready.value.admission, + geometry, ); state = transition.state; const performed = yield* perform(transition.intent, current, execution, options, wakes); @@ -545,10 +567,14 @@ function* drive( ? yield* build(state, reading, current, at, focused) : refusedView(state, broken, at, focused); }; - rendered = (yield* paint(frames, tree, renderer, screen, compose)).rendered; - focused = keyOfFocus(tree); - // The first frame has the same obligation as every other one. - rendered = (yield* paint(frames, tree, renderer, screen, compose)).rendered; + rendered = (yield* paint(frames, tree, renderer, screen, compose, fits)).rendered; + // The first frame has the same obligation as every other one: draw again + // only if the tree settled focus somewhere the frame did not mark. + const settled = keyOfFocus(tree); + if (settled !== focused) { + focused = settled; + rendered = (yield* paint(frames, tree, renderer, screen, compose, fits)).rendered; + } while (true) { const taken = yield* wakes.take(); @@ -736,7 +762,7 @@ function* drive( reading = back.reading; } } - const painted = yield* paint(frames, tree, renderer, screen, compose); + const painted = yield* paint(frames, tree, renderer, screen, compose, fits); rendered = painted.rendered; // Focus is read against the view that was committed, which a resize may // have rebuilt from the one resolved above. @@ -753,7 +779,7 @@ function* drive( state = focusSettled(view, settledFocus, painted.admission); if (settledFocus !== focused) { focused = settledFocus; - const again = yield* paint(frames, tree, renderer, screen, compose); + const again = yield* paint(frames, tree, renderer, screen, compose, fits); rendered = again.rendered; settledFocus = keyOfFocus(tree); state = focusSettled(again.view, settledFocus, again.admission); @@ -1127,9 +1153,10 @@ function* paint( renderer: ReplRenderer, screen: ReplScreen, compose: ReplCompose, + store?: ReplFitStore, ): Operation { while (true) { - const painted = yield* prepared(frames, tree, renderer, screen, compose); + const painted = yield* prepared(frames, tree, renderer, screen, compose, store); if (painted !== undefined) { return painted; } @@ -1152,6 +1179,7 @@ function* prepared( renderer: ReplRenderer, screen: ReplScreen, compose: ReplCompose, + store?: ReplFitStore, ): Operation { return yield* scoped(function* (): Operation { const held = yield* frames.subscribe({ owner: "repl", participants: [["repl"]] }); @@ -1173,8 +1201,14 @@ function* prepared( // that skips it: a frame drawn from a measurement the terminal has // already invalidated publishes targets naming rows nobody can see, and // presenting it would put that on the screen as though it were current. - const committed = yield* commitReplFrame(tree, renderer, view, tick.delta, undefined, () => - screen.size(), + const committed = yield* commitReplFrame( + tree, + renderer, + view, + tick.delta, + undefined, + () => screen.size(), + store, ); if (committed.ok) { // 5. Present the copied bytes. 6. The caller retains the returned map. @@ -1296,11 +1330,12 @@ export function* commitReplFrame( * one exact size — leaves it out. */ sizeNow?: () => Operation, + store?: ReplFitStore, ): Operation> { // Measure before anything is mounted: this asks the engine how much room each // region has, so what a window shows is the frame's answer rather than this // boundary's guess. It mounts nothing and publishes nothing. - const prepared = yield* prepareFrame(renderer, view); + const prepared = yield* prepareFrame(renderer, view, store); if (!prepared.ok) { return prepared; } @@ -1438,6 +1473,13 @@ function measuringAt( export function* prepareFrame( renderer: ReplRenderer, view: ReplView, + /** + * Where this process keeps the reading geometry it has already measured. + * + * Absent for a caller holding one frame — a test, or a measurement taken + * for its own sake — which measures everything every time. + */ + store?: ReplFitStore, ): Operation> { // No widths at all for the pass that answers what the widths are. Every row // whose text has to fit a region is left out of it, because a growing column @@ -1483,7 +1525,13 @@ export function* prepareFrame( const reading = widths.surface < 1 ? Ok(undefined) - : yield* prepareReading(renderer, view.size, readingLines(readingOf(view)), widths.surface); + : yield* prepareReading( + renderer, + view.size, + readingLines(readingOf(view)), + widths.surface, + store, + ); if (!reading.ok) { return reading; } diff --git a/packages/cli/tests/fixtures/repl/presentation.ts b/packages/cli/tests/fixtures/repl/presentation.ts index ed6d96d8..b2109dbb 100644 --- a/packages/cli/tests/fixtures/repl/presentation.ts +++ b/packages/cli/tests/fixtures/repl/presentation.ts @@ -682,6 +682,58 @@ export function useMeasuringRenderer(size: ReplTerminalSize): Operation { + const renderer = yield* useReplRenderer(size); + let measures = 0; + let texts = 0; + let draws = 0; + return { + *measure(ops, measured) { + measures += 1; + // Each `text` op is one string the engine has to lay out. It is the unit + // the expensive paths differ in: a binary search over one long line and a + // batch over fifty short ones are both one render. + texts += ops.filter((op) => "content" in op && typeof op.content === "string").length; + return yield* renderer.measure(ops, measured); + }, + *draw(input) { + draws += 1; + return yield* renderer.draw(input); + }, + resize: (next) => renderer.resize(next), + last: () => renderer.last(), + engines: () => renderer.engines(), + measures: () => measures, + texts: () => texts, + draws: () => draws, + reset() { + measures = 0; + texts = 0; + draws = 0; + }, + }; +} + /** * A measuring context, for a test asking what a view *offers* rather than what * one frame admitted. diff --git a/packages/cli/tests/repl-entries.test.ts b/packages/cli/tests/repl-entries.test.ts index 0a6838bf..f49ae54c 100644 --- a/packages/cli/tests/repl-entries.test.ts +++ b/packages/cli/tests/repl-entries.test.ts @@ -63,6 +63,7 @@ import { focusSettled, initialState, NO_AGENT, + readsGeometry, reduceRepl, presentationFor, viewFor, @@ -3411,3 +3412,83 @@ describe("REPL entries: what a failed entry says it failed with", () => { }); }); }); + +/** + * One representative action of every kind the vocabulary offers. + * + * The referents are arbitrary on purpose: what is compared below is two + * reductions of the *same* action, so whether a marker exists or a field is + * mounted changes both answers identically. + */ +const EVERY_ACTION: readonly ReplAction[] = Object.freeze([ + { kind: "type", text: "x" }, + { kind: "type", text: "x", field: "project" }, + { kind: "erase" }, + { kind: "erase", field: "project" }, + { kind: "submit" }, + { kind: "select-surface", surface: "entries" }, + { kind: "select-scope", scopes: ["entry-1"] }, + { kind: "open-drawer", drawer: { kind: "history" } }, + { kind: "close-drawer" }, + { kind: "select-marker", marker: "yield:root:0" }, + { kind: "go-live" }, + { kind: "pause" }, + { kind: "continue" }, + { kind: "answer" }, + { kind: "select-field", field: "project" }, + { kind: "choose", field: "project", option: "Approve" }, + { kind: "select-session", session: "xmd:v1:one" }, + { kind: "all-sessions" }, + { kind: "select-permission", request: "permission-1" }, + { kind: "choose-permission", request: "permission-1", option: "once" }, + { kind: "dismiss-permission", request: "permission-1" }, + { kind: "exit" }, + { kind: "scroll", delta: 1 }, + { kind: "scroll-sessions", delta: 1 }, + { kind: "scroll-entries", delta: 1 }, + { kind: "scroll-reading", delta: 1 }, +]); + +describe("EU2 only the actions that move a window are answered with measured geometry", () => { + it("answers every other action identically with and without an admission", () => + scoped(function* () { + const engine = yield* useReplRenderer(NARROW); + const model = projected(yield* manyEntries(6)); + const state = selecting(initialState(EXECUTION), "entry-1"); + const view = reading(state, model, NOTHING_LIVE, NARROW); + const admission = (yield* contextOf(view, engine)).admission; + // The frame really did admit something, or every comparison below would + // be two reductions against nothing. + expect(admission.windows.size).toBeGreaterThan(0); + + const covered = new Set(); + const moved: string[] = []; + for (const action of EVERY_ACTION) { + covered.add(action.kind); + const measured = reduceRepl(state, action, model, NOTHING_LIVE, admission); + const blind = reduceRepl(state, action, model, NOTHING_LIVE); + if (JSON.stringify(measured) === JSON.stringify(blind)) { + continue; + } + // Whatever the measurement changed the answer to is an action the root + // has to measure before. If a kind outside the predicate ever appears + // here, the root is skipping a measurement that action needed — which + // is the defect this guard exists for. + moved.push(action.kind); + expect([action.kind, readsGeometry(action)]).toEqual([action.kind, true]); + } + // And the predicate is tight where it has to be. The whole reason the + // root may skip a measurement is that these two never need one, and a + // predicate answering yes for them would be correct while giving back + // every millisecond this costs to save. + expect(readsGeometry({ kind: "type", text: "x" })).toBe(false); + expect(readsGeometry({ kind: "erase" })).toBe(false); + // And the admission really did change an answer, so the agreement above + // is a fact about the other kinds rather than about a measurement + // nothing consulted. + expect(moved.length).toBeGreaterThan(0); + // Every kind the union offers is represented, so a kind added later + // without a row here is a kind this guard never looked at. + expect(covered.size).toBe(24); + })); +}); diff --git a/packages/cli/tests/repl-journey.test.ts b/packages/cli/tests/repl-journey.test.ts index 3368d6d8..edd031ba 100644 --- a/packages/cli/tests/repl-journey.test.ts +++ b/packages/cli/tests/repl-journey.test.ts @@ -2552,8 +2552,17 @@ describe("REPL journey: resizing while a frame is being prepared", () => { // against this program rather than promised by it — what the row asserts is // that the measured frame and the terminal were out of step, and the two // controls for it are what keep that honest. + // + // Three, because that is where the boundary is. One keystroke reads the + // size at the too-small check, again to resolve the frame, a third time + // for the measurement the frame is built from, and a fourth to + // revalidate it. The third is the measurement, so a drag armed there + // answers it with the old size and leaves the new one for the + // revalidation to find. It was four while every action also measured a + // frame before reducing; typing no longer does, and the read that + // preparation performed is gone. const from = terminal.presented.length; - terminal.resizeAfterRead(4, { columns: 72, rows: 20 }); + terminal.resizeAfterRead(3, { columns: 72, rows: 20 }); terminal.feed("!"); yield* drawing(); @@ -4738,6 +4747,55 @@ describe("REPL journey: a cold process over a multi-entry journal", () => { * evidence about anything. Top level, each of these can be run, and reddened, on * its own. */ +describe("REPL responsiveness: a focus move is one frame", () => { + it("RS1: Tab moves the cue and the focus, and presents once", function* () { + const { terminal, install } = recordingTerminal(); + yield* scoped(function* (): Operation { + yield* install(); + yield* immediateClock(); + yield* useTempFileCompiler(); + yield* useTemporaryHost(); + + const running = yield* spawn(function* (): Operation { + const ran = yield* runReplProgram({ profile: PROFILE }); + if (!ran.ok) { + throw ran.error; + } + }); + yield* untilDrawn(terminal); + // Something to traverse, and a settled screen to count from. + yield* submitted(terminal, COLD_ONE); + yield* settledEntry(terminal, 1, "ok"); + yield* focusDraft(terminal); + yield* settled(40); + + // Where focus is now, read off the screen rather than from the program. + const before = screenOf(terminal).findIndex((line) => line.includes(">> ")); + expect(before).toBeGreaterThanOrEqual(0); + + const from = terminal.presented.length; + terminal.feed("\t"); + yield* settled(40); + + // The cue moved, so the traversal happened. + expect(screenOf(terminal).findIndex((line) => line.includes(">> "))).not.toBe(before); + // And it cost one frame. It used to cost two: the first drew the control + // the reader had just left and the second corrected it, so the marker was + // always one keystroke behind what the tree already held. + expect(terminal.presented.length - from).toBe(1); + + // Back again, the same way and for the same price. + const back = terminal.presented.length; + terminal.feed("\x1b[Z"); + yield* settled(40); + expect(screenOf(terminal).findIndex((line) => line.includes(">> "))).toBe(before); + expect(terminal.presented.length - back).toBe(1); + + terminal.end(); + yield* running; + }); + }); +}); describe("REPL first use: UI2", () => { it("UI2: Enter on [exit] ends the command without touching the history", function* () { const { terminal, install } = recordingTerminal(); diff --git a/packages/cli/tests/repl-source-reading.test.ts b/packages/cli/tests/repl-source-reading.test.ts index 5e9b8671..44474d08 100644 --- a/packages/cli/tests/repl-source-reading.test.ts +++ b/packages/cli/tests/repl-source-reading.test.ts @@ -29,8 +29,10 @@ import type { Operation } from "effection"; import { entryReading, NO_READING, READING_STATUSES } from "../src/repl/source-reading.ts"; import type { ReplReadingLine } from "../src/repl/source-reading.ts"; -import { prepareReading, readingRuns, reserveFor } from "../src/repl/fitting.ts"; +import { createFitStore, prepareReading, readingRuns, reserveFor } from "../src/repl/fitting.ts"; import type { ReplPreparedReading } from "../src/repl/fitting.ts"; +import { useCountingRenderer } from "./fixtures/repl/presentation.ts"; +import type { ReplCountingRenderer } from "./fixtures/repl/presentation.ts"; import { useReplRenderer } from "../src/repl/renderer.ts"; import type { ReplRenderer } from "../src/repl/renderer.ts"; import { runText } from "../src/repl/description.ts"; @@ -42,6 +44,7 @@ import { LIFECYCLE, RAIL } from "./fixtures/repl/reference-style.ts"; import { REPL_PALETTE, runStyleOf } from "../src/repl/presentation-style.ts"; const WIDE: ReplTerminalSize = { columns: 160, rows: 36 }; +const NARROW_SIZE: ReplTerminalSize = { columns: 72, rows: 20 }; function withRenderer(body: (renderer: ReplRenderer) => Operation): Operation { return scoped(function* () { @@ -888,3 +891,131 @@ describe("the reading one entry is shown as", () => { })); }); }); + +/** + * A reading of this many lines, long enough that measuring it is the + * expensive thing a frame does. + */ +function longSource(lines: number): string { + return Array.from({ length: lines }, (_, at) => + at % 4 === 3 + ? "" + : ``, + ).join("\n"); +} + +/** The lines a reading of this source is drawn from, at one lifecycle. */ +function linesOf(source: string, lifecycle = NO_LIFECYCLE, inspected = false) { + return entryReading({ entry: entryOf(source), lifecycle, live: "", inspected }).source.lines; +} + +function withCounting(body: (renderer: ReplCountingRenderer) => Operation): Operation { + return scoped(function* () { + return yield* body(yield* useCountingRenderer(WIDE)); + }); +} + +describe("S5 a reading is measured once for the inputs it was measured for", () => { + it("measures nothing again when only the draft beside it moved", () => + withCounting(function* (renderer) { + const store = createFitStore(); + const lines = linesOf(longSource(222)); + const first = yield* prepareReading(renderer, WIDE, lines, 100, store); + expect(first.ok).toBe(true); + // The reading really is the expensive thing: this is the work a + // keystroke used to repeat. + expect(renderer.measures()).toBeGreaterThan(100); + + // Three more frames, as three keystrokes into the draft would ask for. + // The reading is rebuilt from the model every frame, so these are fresh + // line objects carrying the same text — a store keyed on identity would + // miss every time and prove nothing. + renderer.reset(); + for (let press = 0; press < 3; press += 1) { + const again = yield* prepareReading(renderer, WIDE, linesOf(longSource(222)), 100, store); + expect(again.ok).toBe(true); + if (!again.ok || !first.ok) { + return; + } + expect(again.value.rows.map((one) => one.text)).toEqual( + first.value.rows.map((one) => one.text), + ); + expect(again.value.rows.map((one) => one.width)).toEqual( + first.value.rows.map((one) => one.width), + ); + } + expect(renderer.measures()).toBe(0); + })); + + it("measures again when the source, the depth or the room actually changed", () => + withCounting(function* (renderer) { + const store = createFitStore(); + const lines = linesOf(longSource(40)); + expect((yield* prepareReading(renderer, WIDE, lines, 100, store)).ok).toBe(true); + + // A different room is a different fit, and says so. + renderer.reset(); + const narrower = yield* prepareReading(renderer, WIDE, lines, 60, store); + expect(renderer.measures()).toBeGreaterThan(0); + expect(narrower.ok).toBe(true); + + // Back to the first room: the store holds one answer, so this is a miss + // too, and it is still the right answer. + renderer.reset(); + const back = yield* prepareReading(renderer, WIDE, lines, 100, store); + expect(renderer.measures()).toBeGreaterThan(0); + if (!back.ok) { + throw back.error; + } + + // One character of one line, and the whole reading is measured again. + renderer.reset(); + const edited = yield* prepareReading( + renderer, + WIDE, + linesOf(longSource(40).replace("line 0 of", "line 0 OF")), + 100, + store, + ); + expect(renderer.measures()).toBeGreaterThan(0); + if (!edited.ok) { + throw edited.error; + } + expect(edited.value.rows.some((one) => one.text.includes("line 0 OF"))).toBe(true); + + // A different terminal size, at the same room. + renderer.reset(); + expect((yield* prepareReading(renderer, NARROW_SIZE, lines, 100, store)).ok).toBe(true); + expect(renderer.measures()).toBeGreaterThan(0); + })); + + it("draws a phase that arrived without measuring the source again", () => + withCounting(function* (renderer) { + const store = createFitStore(); + const source = longSource(40); + const settled = yield* prepareReading(renderer, WIDE, linesOf(source), 100, store); + expect(settled.ok).toBe(true); + + // The same text, observed differently: one element is now waiting. The + // badge, the rail and the role are live, and none of them is a cut. + renderer.reset(); + const observed = linesOf( + source, + observing([ + { phase: "active", position: at(0), waiting: Object.freeze(["question" as const]) }, + ]), + ); + const held = yield* prepareReading(renderer, WIDE, observed, 100, store); + expect(renderer.measures()).toBe(0); + if (!held.ok || !settled.ok) { + throw held.ok ? new Error("the settled reading refused") : held.error; + } + // Same cuts. + expect(held.value.rows.map((one) => one.text)).toEqual( + settled.value.rows.map((one) => one.text), + ); + // And the new reading is on the row, immediately. + expect(held.value.rows.some((one) => one.badge !== undefined)).toBe(true); + expect(settled.value.rows.every((one) => one.badge === undefined)).toBe(true); + })); +}); From 22b9d2e2bbd3893c55a2149acfa12b5a5e1bca1f Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Sat, 10 Oct 2026 05:01:06 -0400 Subject: [PATCH 2/2] =?UTF-8?q?=E2=9C=85=20Prove=20the=20reuse=20through?= =?UTF-8?q?=20a=20complete=20frame,=20and=20invalidate=20one=20input=20at?= =?UTF-8?q?=20a=20time=20(#881)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/cli/tests/repl-journey.test.ts | 81 ++++++++++ .../cli/tests/repl-presentation-style.test.ts | 98 +++++++++++ .../cli/tests/repl-source-reading.test.ts | 152 +++++++++++++++--- 3 files changed, 305 insertions(+), 26 deletions(-) diff --git a/packages/cli/tests/repl-journey.test.ts b/packages/cli/tests/repl-journey.test.ts index edd031ba..2a9a3ade 100644 --- a/packages/cli/tests/repl-journey.test.ts +++ b/packages/cli/tests/repl-journey.test.ts @@ -4747,6 +4747,87 @@ describe("REPL journey: a cold process over a multi-entry journal", () => { * evidence about anything. Top level, each of these can be run, and reddened, on * its own. */ +/** An entry of this many lines, for measuring what an interaction costs beside it. */ +function longEntry(lines: number): string { + return `${Array.from({ length: lines }, (_, at) => + at % 4 === 3 + ? "" + : ``, + ).join("\n")}\n`; +} + +/** + * What one interaction costs in terminal size reads, through the real loop. + * + * The root reads the size at the too-small check, again to resolve the frame, + * again for the measurement the frame is built from, and again to revalidate + * it. A fifth read appears the moment an action is answered with a freshly + * measured frame — which is what the gate exists to avoid for a keystroke. + * The recording terminal counts those reads, so this is the production + * dispatch, preparation and reconciliation path observed from outside. + */ +function* costOf(terminal: Terminal, send: () => void): Operation { + const from = terminal.sized; + send(); + yield* settled(40); + return terminal.sized - from; +} + +describe("REPL responsiveness: an interaction beside a long reading", () => { + for (const lines of [12, 222]) { + it(`RS3: typing and traversing beside a ${lines}-line entry read the size a fixed number of times`, function* () { + const { terminal, install } = recordingTerminal(); + yield* scoped(function* (): Operation { + yield* install(); + yield* immediateClock(); + yield* useTempFileCompiler(); + yield* useTemporaryHost(); + + const running = yield* spawn(function* (): Operation { + const ran = yield* runReplProgram({ profile: PROFILE }); + if (!ran.ok) { + throw ran.error; + } + }); + yield* untilDrawn(terminal); + yield* submitted(terminal, longEntry(lines)); + yield* settledEntry(terminal, 1, "ok"); + // The long reading is the selected one, so it is what a frame would + // have to fit if anything made it fit again. + yield* click(terminal, "1. [ok] entry-1"); + yield* focusDraft(terminal); + yield* settled(40); + + // Warmup: the first keystroke after a selection still measures the + // reading once, because this is the first frame that reading is in. + yield* costOf(terminal, () => terminal.feed("a")); + + const typed = [ + yield* costOf(terminal, () => terminal.feed("b")), + yield* costOf(terminal, () => terminal.feed("c")), + ]; + const erased = yield* costOf(terminal, () => terminal.feed("\x7f")); + const forward = yield* costOf(terminal, () => terminal.feed("\t")); + const backward = yield* costOf(terminal, () => terminal.feed("\x1b[Z")); + + // Four reads each: the too-small check, the resolution, the frame's + // measurement and its revalidation. A fifth would be an action + // answered with a frame measured before it ran — which a keystroke + // does not need, and which is what this count is here to catch. + expect([lines, typed]).toEqual([lines, [4, 4]]); + expect([lines, erased]).toEqual([lines, 4]); + // A traversal is answered by the tree, not the reducer, so it reads + // the size for its one frame and nothing else. + expect([lines, forward]).toEqual([lines, 4]); + expect([lines, backward]).toEqual([lines, 4]); + + terminal.end(); + yield* running; + }); + }); + } +}); + describe("REPL responsiveness: a focus move is one frame", () => { it("RS1: Tab moves the cue and the focus, and presents once", function* () { const { terminal, install } = recordingTerminal(); diff --git a/packages/cli/tests/repl-presentation-style.test.ts b/packages/cli/tests/repl-presentation-style.test.ts index 3c8e6f03..553b4a11 100644 --- a/packages/cli/tests/repl-presentation-style.test.ts +++ b/packages/cli/tests/repl-presentation-style.test.ts @@ -82,6 +82,10 @@ import { viewportBounds, } from "./fixtures/repl/presentation.ts"; import { referenceEvents } from "./fixtures/repl/reference.ts"; +import { useCountingRenderer } from "./fixtures/repl/presentation.ts"; +import { useReplTree } from "../src/repl/reconcile.ts"; +import { createFitStore } from "../src/repl/fitting.ts"; +import { commitReplFrame } from "../src/repl/program.ts"; import { ordinaryEvaluationProfile } from "../src/evaluation-profile.ts"; import { submitReplEntry } from "../src/repl/session.ts"; import type { ReplSession } from "../src/repl/session.ts"; @@ -2364,3 +2368,97 @@ describe("REPL presentation: a row the frame cannot hold whole", () => { ); }); }); + +/** An entry whose source is this many lines, for measuring what a frame costs. */ +function sourceOf(lines: number): string { + return Array.from({ length: lines }, (_, at) => + at % 4 === 3 + ? "" + : ``, + ).join("\n"); +} + +/** + * What one complete frame asks the engine for, with the draft moved. + * + * The production commit path — measure, admit, reconcile, draw — against one + * real tree, one real engine pair and the store a running REPL owns. The + * first frame is warmup; the number returned is what the *second* frame costs + * when the only thing that changed is the draft beside the reading. + */ +function* frameCost(lines: number): Operation<{ warmup: number; typed: number }> { + return yield* scoped(function* () { + const renderer = yield* useCountingRenderer(WIDE); + const tree = yield* useReplTree(); + const store = createFitStore(); + const model = yield* scoped(function* () { + const physical = new InMemoryStream(); + yield* runEntry(physical, `${sourceOf(lines)}\n`); + return projected(yield* physical.readAll()); + }); + const entry = model.entries[0]; + if (entry === undefined) { + throw new Error("the fixture admitted no entry"); + } + const drafted = (draft: string) => + reading( + Object.freeze({ ...selecting(stateWith({}), entry.key), draft }), + model, + NOTHING_LIVE, + WIDE, + ); + + const first = yield* commitReplFrame( + tree, + renderer, + drafted("a"), + 0, + undefined, + undefined, + store, + ); + if (!first.ok) { + throw first.error; + } + const warmup = renderer.measures(); + renderer.reset(); + const second = yield* commitReplFrame( + tree, + renderer, + drafted("ab"), + 0, + undefined, + undefined, + store, + ); + if (!second.ok) { + throw second.error; + } + return { warmup, typed: renderer.measures() }; + }); +} + +describe("REPL responsiveness: a complete frame after a keystroke", () => { + it("RS2: costs the same whether the selected reading is short or long", () => + scoped(function* () { + const short = yield* frameCost(12); + const long = yield* frameCost(222); + + // The first frame of each pays for its own reading, and the long one + // pays far more — so these two fixtures really do differ in the work + // the cache is about. + expect(long.warmup).toBeGreaterThan(short.warmup * 4); + + // The second frame changed only the draft. What it costs is the fixed + // measurement a complete frame always performs — three structural + // passes, the rail and the drawer — and *not* the source fitting, so it + // is the same number for twelve lines and for two hundred and + // twenty-two. + expect(long.typed).toBe(short.typed); + // That floor is real work, not zero: this is a whole frame, not the + // fitter on its own. + expect(short.typed).toBeGreaterThan(0); + // And it is a fraction of what the reading cost to fit. + expect(long.typed).toBeLessThan(long.warmup / 10); + })); +}); diff --git a/packages/cli/tests/repl-source-reading.test.ts b/packages/cli/tests/repl-source-reading.test.ts index 44474d08..92c6e782 100644 --- a/packages/cli/tests/repl-source-reading.test.ts +++ b/packages/cli/tests/repl-source-reading.test.ts @@ -35,7 +35,7 @@ import { useCountingRenderer } from "./fixtures/repl/presentation.ts"; import type { ReplCountingRenderer } from "./fixtures/repl/presentation.ts"; import { useReplRenderer } from "../src/repl/renderer.ts"; import type { ReplRenderer } from "../src/repl/renderer.ts"; -import { runText } from "../src/repl/description.ts"; +import { runText, tokenRuns } from "../src/repl/description.ts"; import { NO_LIFECYCLE } from "../src/repl/lifecycle.ts"; import type { ReplLifecycleReading, ReplOccurrence } from "../src/repl/lifecycle.ts"; import type { ReplEntry, ReplScope, ReplTerminal } from "../src/repl/model.ts"; @@ -947,46 +947,146 @@ describe("S5 a reading is measured once for the inputs it was measured for", () expect(renderer.measures()).toBe(0); })); - it("measures again when the source, the depth or the room actually changed", () => + /** + * One logical line, built here so a single input can move on its own. + * + * A reading built from source carries its depth from the nesting its text + * has, so changing depth alone means stating the line rather than deriving + * it. The text wraps at the rooms below, which is what makes a change of + * depth or room visible in the cuts. + */ + function line(key: string, text: string, depth: number): ReplReadingLine { + return Object.freeze({ + key, + text, + runs: tokenRuns([{ text, token: "source" }]), + rail: "rail-pending" as const, + depth, + badge: undefined, + style: Object.freeze({ role: "source" as const, selected: false, inspected: false }), + }); + } + + const WRAPPING = [ + line( + "l1", + '', + 0, + ), + line( + "l2", + " a second line, indented, also long enough that the room decides where it ends", + 0, + ), + ]; + + const deeper = (lines: readonly ReplReadingLine[]) => + lines.map((one) => line(one.key, one.text, one.depth + 3)); + + /** The cuts, widths and recovered text one prepared reading produced. */ + function shapeOf(prepared: ReplPreparedReading) { + return { + rows: prepared.rows.map((one) => ({ key: one.key, text: one.text, width: one.width })), + recovered: [...recovered(prepared).entries()], + }; + } + + /** + * The same inputs fitted with no store at all, which is the answer the + * cached path has to agree with. + */ + function* fresh( + renderer: ReplCountingRenderer, + size: ReplTerminalSize, + lines: readonly ReplReadingLine[], + room: number, + ): Operation { + const measured = yield* prepareReading(renderer, size, lines, room); + if (!measured.ok) { + throw measured.error; + } + return measured.value; + } + + it("measures again when only the depth changed, and cuts for the new depth", () => withCounting(function* (renderer) { const store = createFitStore(); - const lines = linesOf(longSource(40)); - expect((yield* prepareReading(renderer, WIDE, lines, 100, store)).ok).toBe(true); - - // A different room is a different fit, and says so. + // Warmed on the exact inputs this pair is about, immediately before. + const warm = yield* prepareReading(renderer, WIDE, WRAPPING, 60, store); + if (!warm.ok) { + throw warm.error; + } renderer.reset(); - const narrower = yield* prepareReading(renderer, WIDE, lines, 60, store); + + // Only the depth moves: same text, same room, same size. + const nested = deeper(WRAPPING); + const cached = yield* prepareReading(renderer, WIDE, nested, 60, store); expect(renderer.measures()).toBeGreaterThan(0); - expect(narrower.ok).toBe(true); + if (!cached.ok) { + throw cached.error; + } + // It is the fit this depth deserves, not the one the store was holding. + expect(shapeOf(cached.value)).toEqual(shapeOf(yield* fresh(renderer, WIDE, nested, 60))); + // And a deeper reading really is cut differently, or the comparison + // above would hold for a store that ignored depth entirely. + expect(shapeOf(cached.value)).not.toEqual(shapeOf(warm.value)); + })); - // Back to the first room: the store holds one answer, so this is a miss - // too, and it is still the right answer. + it("measures again when only the room changed, and cuts for the new room", () => + withCounting(function* (renderer) { + const store = createFitStore(); + const warm = yield* prepareReading(renderer, WIDE, WRAPPING, 60, store); + if (!warm.ok) { + throw warm.error; + } renderer.reset(); - const back = yield* prepareReading(renderer, WIDE, lines, 100, store); + + const cached = yield* prepareReading(renderer, WIDE, WRAPPING, 40, store); expect(renderer.measures()).toBeGreaterThan(0); - if (!back.ok) { - throw back.error; + if (!cached.ok) { + throw cached.error; } + expect(shapeOf(cached.value)).toEqual(shapeOf(yield* fresh(renderer, WIDE, WRAPPING, 40))); + expect(shapeOf(cached.value)).not.toEqual(shapeOf(warm.value)); + })); - // One character of one line, and the whole reading is measured again. + it("measures again when only the terminal size changed", () => + withCounting(function* (renderer) { + const store = createFitStore(); + const warm = yield* prepareReading(renderer, WIDE, WRAPPING, 60, store); + if (!warm.ok) { + throw warm.error; + } renderer.reset(); - const edited = yield* prepareReading( - renderer, - WIDE, - linesOf(longSource(40).replace("line 0 of", "line 0 OF")), - 100, - store, - ); + + // Only the size moves: same lines, same room. + const cached = yield* prepareReading(renderer, NARROW_SIZE, WRAPPING, 60, store); expect(renderer.measures()).toBeGreaterThan(0); - if (!edited.ok) { - throw edited.error; + if (!cached.ok) { + throw cached.error; } - expect(edited.value.rows.some((one) => one.text.includes("line 0 OF"))).toBe(true); + expect(shapeOf(cached.value)).toEqual( + shapeOf(yield* fresh(renderer, NARROW_SIZE, WRAPPING, 60)), + ); + })); - // A different terminal size, at the same room. + it("measures again when only one line's text changed", () => + withCounting(function* (renderer) { + const store = createFitStore(); + const warm = yield* prepareReading(renderer, WIDE, WRAPPING, 60, store); + if (!warm.ok) { + throw warm.error; + } renderer.reset(); - expect((yield* prepareReading(renderer, NARROW_SIZE, lines, 100, store)).ok).toBe(true); + + const edited = [line("l1", `${WRAPPING[0].text} and one more clause`, 0), WRAPPING[1]]; + const cached = yield* prepareReading(renderer, WIDE, edited, 60, store); expect(renderer.measures()).toBeGreaterThan(0); + if (!cached.ok) { + throw cached.error; + } + expect(shapeOf(cached.value)).toEqual(shapeOf(yield* fresh(renderer, WIDE, edited, 60))); + expect(shapeOf(cached.value)).not.toEqual(shapeOf(warm.value)); })); it("draws a phase that arrived without measuring the source again", () =>