Make the landing snow actually render on an idle screen (#428)

* Make the landing snow actually render on an idle screen

Three independent gates kept the falling-snow decoration over the landing
mountain from ever drawing: the mount-time paint was still=true, which the
old snowOn check tied directly to snow visibility; paintLanding early-returned
whenever it wasn't re-entered with animating=true, which never happens while
idle; and its only caller was the turn monitor, which deliberately stops
ticking once idle.

Fix: snow visibility no longer depends on `still` (that flag now only freezes
the mountain's own draw/fill/fade timeline, not the flakes over it), the
early-return is gone so idle repaints actually happen, and the shell's
createAppShell now repaints the landing off the renderer's own FRAME event
whenever the landing is up and not mid-turn-animation. That event is already
scoped to shell lifetime (wired at construction, unwired in dispose) and
paintLanding already no-ops once the landing tears down, so this needed no
new timer to arm or leak, and it doesn't touch the turn monitor's cadence at
all. The alternative — a separate timer armed from createLandingAbove and
stopped in the landing teardown — was rejected: it would duplicate the
monitor's own cadence-management responsibility for no real gain, since the
FRAME event already has the right lifetime.

Added a real-mount-path regression test in landing.test.ts that goes through
createAppShell and lets the renderer's FRAME event drive the repaint, unlike
every existing test in that file, which drives the mark by calling
paintLanding directly with a hand-picked clock. That gap is exactly why this
shipped broken and nobody caught it.

This supersedes PR #380, which added the snow-drawing code but never made it
reachable; that PR should stay open until this one is reviewed and can then
be closed in favor of this one.

* Throttle the idle landing repaint to ~8fps

FRAME fires from inside the on-demand render loop, and paintLanding
reassigns a fresh StyledText to every row every call regardless of
whether content changed, which unconditionally dirties renderables and
requests another render. Left unthrottled that turned the idle landing
into a perpetual ~60fps render loop instead of riding an existing one.
Guard the onFrame-driven repaint with a stored last-paint timestamp so
it only actually repaints once every ~125ms, and drop the comment that
falsely claimed only changed rows get touched.

Also thread an explicit reducedMotion input through the mark renderer
(renderMark/markChunks/paintLandingMark/paintLanding), separate from
still, so a future reduced-motion setting has a real plumbing path
instead of overloading still (which only freezes the mountain's own
draw/fill/fade timeline). Defaults to false everywhere; no behavior
change until something sets it. Updates the mark-anim.ts and
landing.ts docblocks that stated still suppressed snow, which PR #428
made no longer true.

* Drive the idle landing repaint off a mount-scoped timer, not a throttled FRAME hook

The FRAME-driven throttle added in the previous commit killed the landing's
self-driving loop entirely: the renderer only keeps rendering because each
paint dirties a row, which schedules the next FRAME; skipping a paint on a
throttled tick breaks that chain on the very next frame and the snow freezes
after the first paint. Any throttle above zero frames has the same effect,
since the throttle and the frame source were the same mechanism.

Replace it with a plain ~125ms interval armed when the shell mounts (the
landing exists for the lifetime of the shell until the first transcript row
tears it down) and cleared on whichever teardown happens first: the landing
going away, or the shell disposing. The timer self-cancels once the renderer
reports destroyed, so headless test harnesses that skip explicit
shell.dispose() don't leave it firing against torn-down renderables.

Also replace the test's manual renderOnce-loop clock with a real wall-clock
wait and no frame pumping at all, so the test can no longer stay green while
production's self-driving mechanism is dead — that blind spot is exactly how
the frozen throttle shipped in the first place.

Author: TheGreatAxios

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<string>()
         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, {

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]

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", () => {

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++) {

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) {