diff --git a/src/tui-opentui/landing.test.ts b/src/tui-opentui/landing.test.ts index a5f3430b8..20fe2e938 100644 --- a/src/tui-opentui/landing.test.ts +++ b/src/tui-opentui/landing.test.ts @@ -38,6 +38,7 @@ import { import { LOCKUP_WORDMARK } from "./lockup" import pkg from "../../package.json" with { type: "json" } import { MARK_LARGE, MARK_MID, MARK_SMALL } from "./mark-shape" +import { SNOW_CHAR } from "./mark-anim" import { UI } from "./theme" const SIZE = { width: 80, height: 24 } as const @@ -229,11 +230,14 @@ describe("landing screen", () => { try { await settle(h) const still = markRows(h).join("\n") + const stripSnow = (text: string) => text.replaceAll(SNOW_CHAR, " ") - // Idle re-entry holds the filled frame however far the clock moves. + // Idle re-entry holds the mountain's filled frame however far the + // clock moves — but the snow drifting over it is not still, since the + // idle landing screen is exactly where it needs to animate. paintLanding(shell, 1_700, false) await settle(h) - expect(markRows(h).join("\n")).toBe(still) + expect(stripSnow(markRows(h).join("\n"))).toBe(stripSnow(still)) const frames = new Set() for (const nowMs of [0, 500, 1_100, 1_900, 2_600, 3_400]) { @@ -248,6 +252,49 @@ describe("landing screen", () => { }, SIZE) }) + test( + "an idle mount keeps the snow drifting on its own, with nothing pumping frames by hand", + async () => { + // Regression for CL-5737: every other test in this file drives the mark + // by calling `paintLanding` directly with a hand-picked clock. That is + // exactly why the landing snow shipped completely unreachable — none of + // those tests go through the real driver a running session actually + // uses. This one mounts the shell for real and lets it repaint itself: + // no `paintLanding`/`renderMark` calls, and critically no `renderOnce` + // loop either while waiting — a test that pumps frames by hand can stay + // green even when production's self-driving mechanism is dead, which is + // exactly the blind spot that let the throttled build ship frozen snow. + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + run: "idle", + }) + try { + await settle(h) + const before = markRows(h).join("\n") + + // Real wall-clock wait, no renderOnce in between: only the mount's + // own idle-repaint timer can be advancing the snow here. `flush` + // waits on the renderer's own scheduler settling rather than + // forcing frames, so it does not manufacture the motion itself. + await new Promise((resolve) => setTimeout(resolve, 3_000)) + + await h.flush() + const after = markRows(h).join("\n") + + expect(after).not.toBe(before) + + const stripSnow = (text: string) => text.replaceAll(SNOW_CHAR, " ") + expect(stripSnow(after)).toBe(stripSnow(before)) + } finally { + shell.dispose() + } + }, SIZE) + }, + 15_000, + ) + test("a starter key fills the prompt; a typed prompt keeps its digits", async () => { await withTestRenderer(async (h) => { const shell = createAppShell(h.renderer, { diff --git a/src/tui-opentui/landing.ts b/src/tui-opentui/landing.ts index e58c03ff2..9f5bd1813 100644 --- a/src/tui-opentui/landing.ts +++ b/src/tui-opentui/landing.ts @@ -271,8 +271,9 @@ function markChunks( grid: MarkGrid, nowMs: number, still: boolean, + reducedMotion = false, ): readonly TextChunk[][] { - return renderMark({ nowMs, still, grid }).map((row) => + return renderMark({ nowMs, still, grid, reducedMotion }).map((row) => row.map((cell) => fgChunk(cell.fg)(cell.char)), ) } @@ -403,17 +404,20 @@ export function fitLandingMark(above: LandingAbove, grid: MarkGrid | null): void } /** - * Repaint the mark for the given clock. `still` holds the fully-filled frame — - * the idle state, and the reduced-motion state. + * Repaint the mark for the given clock. `still` holds the mountain's + * draw/fill/fade timeline on its fully-filled frame — the idle state. + * `reducedMotion` is the separate hook that suppresses snow; it does not + * affect `still`'s mountain framing. */ export function paintLandingMark( above: LandingAbove, nowMs: number, still: boolean, + reducedMotion = false, ): void { const grid = above.grid if (grid === null) return - const chunks = markChunks(grid, nowMs, still) + const chunks = markChunks(grid, nowMs, still, reducedMotion) const offset = MARK_LARGE.rows - grid.rows above.markRows.forEach((line, index) => { const row = chunks[index - offset] diff --git a/src/tui-opentui/mark-anim.test.ts b/src/tui-opentui/mark-anim.test.ts index cb321f7f5..e634e2a9d 100644 --- a/src/tui-opentui/mark-anim.test.ts +++ b/src/tui-opentui/mark-anim.test.ts @@ -106,19 +106,38 @@ describe("renderMark", () => { const grid = renderMark({ nowMs: 0, still: true, grid: MARK_LARGE }) grid.forEach((row, y) => { row.forEach((cell, x) => { - // Still mode has no snow — only space or mountain blocks. - expect(` ${MOUNTAIN_CHARS}`).toContain(cell.char) + // Still mode freezes the mountain, but snow still drifts over the sky. + expect(` ${MOUNTAIN_CHARS}${SNOW_CHAR}`).toContain(cell.char) if ((MARK_LARGE.coverage[y]?.[x] ?? 0) === 1) expect(cell.char).toBe("█") }) }) }) - test("the still frame is clock-independent and has no snow", () => { - const a = markText(renderMark({ nowMs: 0, still: true })) - const b = markText(renderMark({ nowMs: 987_654, still: true })) + test("still holds the mountain fixed while the clock advances", () => { + // Snow moves with the clock even in still mode (the idle landing screen), + // so isolate the mountain by stripping snow before comparing. + const stripSnow = (text: string) => text.replaceAll(SNOW_CHAR, " ") + const a = stripSnow(markText(renderMark({ nowMs: 0, still: true }))) + const b = stripSnow(markText(renderMark({ nowMs: 987_654, still: true }))) expect(b).toBe(a) expect(a.replace(/[\s\n]/g, "").length).toBeGreaterThan(0) - expect(a.includes(SNOW_CHAR)).toBe(false) + }) + + test("snow keeps drifting in still mode while the mountain stays frozen", () => { + const times = [0, 1500, 3000, 4500, 6000, 7500] + const snowSets = times.map((nowMs) => { + const grid = renderMark({ nowMs, still: true, grid: MARK_LARGE }) + const snow: string[] = [] + grid.forEach((row, y) => { + row.forEach((cell, x) => { + if (isSnow(cell.char)) snow.push(`${y},${x}`) + }) + }) + return snow.join("|") + }) + const withSnow = snowSets.filter((s) => s.length > 0) + expect(withSnow.length).toBeGreaterThan(1) + expect(new Set(withSnow).size).toBeGreaterThan(1) }) test("the animated frame advances with the injected clock", () => { @@ -203,11 +222,12 @@ describe("renderMark", () => { expect(mountains).toBeGreaterThan(flakes) }) - test("still mode freezes the mark with no snow motion", () => { + test("still mode freezes the mountain but not the snow", () => { const a = renderMark({ nowMs: 0, still: true, grid: MARK_SMALL }) const b = renderMark({ nowMs: 50_000, still: true, grid: MARK_SMALL }) - expect(markText(b)).toBe(markText(a)) - expect(a.flat().some((cell) => isSnow(cell.char))).toBe(false) + const mountainText = (grid: typeof a) => + grid.map((row) => row.map((cell) => (isMountain(cell.char) ? cell.char : " ")).join("")).join("\n") + expect(mountainText(b)).toBe(mountainText(a)) }) test("snow drops out during the fade-out phase, matching the mark", () => { diff --git a/src/tui-opentui/mark-anim.ts b/src/tui-opentui/mark-anim.ts index a3a62dafe..450cbc9f2 100644 --- a/src/tui-opentui/mark-anim.ts +++ b/src/tui-opentui/mark-anim.ts @@ -9,8 +9,11 @@ * * Over the sky (zero-coverage cells) a sparse field of pixel snow falls on the * same injected clock. Density and speed stay low so the ridgeline keeps its - * silhouette; `still` (idle or reduced motion) freezes the mark and drops the - * snow entirely. Mountain cells always win over flakes. + * silhouette. `still` freezes the mountain's own draw/fill/fade timeline to + * its fully-filled frame but leaves snow drifting — the landing screen is + * idle by definition, so tying snow to the same flag that freezes the + * mountain would mean it never falls. `reducedMotion` is the separate hook + * that does suppress snow. Mountain cells always win over flakes. * * Everything here is pure and clock-injected: `nowMs` is the only time source, * so the caller's existing 250 ms status tick drives the animation and tests @@ -94,8 +97,20 @@ export type MarkCell = { export type MarkInput = { readonly nowMs: number - /** Hold the mark still: idle session, or reduced motion. */ + /** + * Hold the mountain's draw/fill/fade timeline on its fully-filled frame: + * idle session, or reduced motion. Snow is not gated by this — see + * `snowOn` in `renderMark`. + */ readonly still: boolean + /** + * Reduced-motion hook: suppresses snow regardless of `still`. Nothing + * wires a live setting into this yet, but the parameter exists so a + * future reduced-motion setting has a real path to gate motion, rather + * than overloading `still` (which only ever freezes the mountain's + * draw/fill/fade timeline). Defaults to off. + */ + readonly reducedMotion?: boolean /** Which baked rasterization to composite. Defaults to the compact grid. */ readonly grid?: MarkGrid } @@ -138,7 +153,8 @@ function snowflakeAt( * the mark is a mountain, and a mountain is opaque. * * Sky cells (zero coverage) may hold a single falling snow pixel. Flakes never - * overwrite mountain coverage, and `still` suppresses them entirely. + * overwrite mountain coverage; `reducedMotion` suppresses them, `still` does + * not (see `snowOn` below). * * `alpha` has no terminal equivalent, so it scales the block height instead: * the mark sinks toward empty rather than blending to black. @@ -149,9 +165,11 @@ export function renderMark(input: MarkInput): readonly (readonly MarkCell[])[] { const { drawProg, fillProg, alpha } = markFrame(seconds, input.still) const revealed = drawProg * shape.cols const fillLine = shape.rows * (1 - fillProg) - // Fade out drops the snow too so the decoration doesn't outlast the mark - // it drifts over. - const snowOn = !input.still && alpha === 1 + // Independent of `still`: the mountain can be frozen full while snow still + // drifts (the idle landing screen). `reducedMotion` is the actual + // motion-suppression hook. Fade out drops the snow too so the decoration + // doesn't outlast the mark it drifts over. + const snowOn = alpha === 1 && !input.reducedMotion const grid: MarkCell[][] = [] for (let row = 0; row < shape.rows; row++) { diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index 9e87951c9..458695141 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -471,7 +471,7 @@ function dispatchOverlayAccept( /** Renderer surface required by the shell (CliRenderer / createTestRenderer). */ export type ShellRenderer = Pick< CliRenderer, - "root" | "width" | "height" | "keyInput" | "on" | "off" + "root" | "width" | "height" | "keyInput" | "on" | "off" | "isDestroyed" > export type AppShellOptions = { @@ -1930,6 +1930,13 @@ type ShellInternals = { landingAnimating: boolean /** Clock of the last painted mark frame, so a resize can redraw in place. */ landingNowMs: number + /** + * Cancels the mount-scoped idle repaint timer (see `armLandingIdleTimer` + * in `createAppShell`), or null while none is armed. Cleared by whichever + * teardown happens first — the landing going away (`clearLandingMark`) or + * the whole shell disposing (`dispose`) — so it can never outlive either. + */ + landingIdleTimerCancel: (() => void) | null /** Chrome content (empty array = zone off). */ chrome: { /** @@ -2533,6 +2540,8 @@ function clearLandingMark(shell: AppShell): void { const landing = bag?.landing if (bag === undefined || landing === null || landing === undefined) return bag.landing = null + bag.landingIdleTimerCancel?.() + bag.landingIdleTimerCancel = null shell.transcript.remove(landing.above.box) destroySubtree(landing.above.box) shell.root.remove(landing.below) @@ -2556,26 +2565,36 @@ function clearLandingMark(shell: AppShell): void { } /** - * Repaint the landing mark for `nowMs`. `animating` runs the draw/fill/fade - * timeline; anything else holds the filled frame. No-op once the landing is - * gone, so the caller can drive it unconditionally. + * Cadence of the mount-scoped idle repaint timer (see `armLandingIdleTimer` + * in `createAppShell`). The snow only needs to advance about half a row per + * second, so ~8fps is comfortably enough to read as motion. + */ +const LANDING_IDLE_REPAINT_INTERVAL_MS = 125 + +/** + * Repaint the landing mark for `nowMs`. `animating` runs the mountain's + * draw/fill/fade timeline; anything else holds its filled frame. No-op once + * the landing is gone, so the caller can drive it unconditionally. * - * A still mark draws the same frame for every clock value, so repainting it - * only dirties renderables; the guard mirrors `setLockupFrame` and lets an idle - * session sit without touching the paint tree. + * Always repaints while the landing is up, even when `animating` is false: + * the landing is idle by definition (no turn processing), and snow still + * needs to drift across a frozen mountain. Driven by the mount-scoped timer + * armed in `createAppShell` (see `armLandingIdleTimer`) rather than a render + * event, so the repaint cadence is independent of however often the renderer + * happens to paint. */ export function paintLanding( shell: AppShell, nowMs: number, animating: boolean, + reducedMotion = false, ): void { const bag = internals.get(shell) const landing = bag?.landing if (bag === undefined || landing === null || landing === undefined) return - if (!animating && !bag.landingAnimating) return bag.landingAnimating = animating bag.landingNowMs = nowMs - paintLandingMark(landing.above, nowMs, !animating) + paintLandingMark(landing.above, nowMs, !animating, reducedMotion) } /** True while the landing composition is still mounted. */ @@ -5687,6 +5706,7 @@ export function createAppShell( } renderer.off(CliRenderEvents.FRAME, onFrame) renderer.off(CliRenderEvents.RESIZE, onResize) + internals.get(shell)?.landingIdleTimerCancel?.() flashTimers.get(shell)?.() flashTimers.delete(shell) try { @@ -5726,9 +5746,48 @@ export function createAppShell( landingSuggestionsVisible: true, landingAnimating: false, landingNowMs: 0, + landingIdleTimerCancel: null, chrome: { task: [], tasksRaw: [], agents: [] }, tasksPanelHidden: false, }) + // The landing's snow needs a frame source that keeps running while the + // turn monitor is deliberately quiet (idle, no session yet). A plain timer + // armed at mount is that source: it does not depend on the renderer + // scheduling further frames, so it cannot stall the way riding the + // renderer's own FRAME event did (see CL-5737 history in the PR). + // + // Only repaints while idle (`landingAnimating` false): while a turn is + // processing, `paintPhaseAt` in runtime-bridge.ts drives the mountain's + // own draw/fill/fade loop off the turn monitor's clock, and this timer + // must not stomp that with an unrelated real-clock value. + // + // Cleared on whichever teardown happens first: the landing going away + // (`clearLandingMark`, first transcript row) or the whole shell disposing + // (`dispose` below, e.g. tests that never grow a transcript). + // + // Also self-cancels on `renderer.isDestroyed`: a real terminal session + // always disposes the shell, but headless test harnesses commonly destroy + // the renderer directly (`withTestRenderer`'s cleanup) without ever + // calling `shell.dispose()`. Without this check the timer would keep + // firing against renderables the harness already tore down. + const landingIdleHandle = setInterval(() => { + if (renderer.isDestroyed) { + clearInterval(landingIdleHandle) + return + } + const bag = internals.get(shell) + if (bag?.landing == null || bag.landingAnimating) return + paintLanding(shell, Date.now(), false) + }, LANDING_IDLE_REPAINT_INTERVAL_MS) + landingIdleHandle.unref?.() + { + const bag = internals.get(shell) + if (bag !== undefined) { + bag.landingIdleTimerCancel = () => clearInterval(landingIdleHandle) + } else { + clearInterval(landingIdleHandle) + } + } transcriptSpacers.set(shell, transcriptSpacer) if (onCommandOpt) setPaletteOnCommand(shell, onCommandOpt) if (onObserveRequestOpt) {