Skip to content

Commit 6fbeba6

Browse files
committed
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.
1 parent 9165913 commit 6fbeba6

2 files changed

Lines changed: 68 additions & 70 deletions

File tree

src/tui-opentui/landing.test.ts

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -253,16 +253,17 @@ describe("landing screen", () => {
253253
})
254254

255255
test(
256-
"an idle mount keeps the snow drifting on the renderer's own frame event",
256+
"an idle mount keeps the snow drifting on its own, with nothing pumping frames by hand",
257257
async () => {
258258
// Regression for CL-5737: every other test in this file drives the mark
259259
// by calling `paintLanding` directly with a hand-picked clock. That is
260260
// exactly why the landing snow shipped completely unreachable — none of
261261
// those tests go through the real driver a running session actually
262-
// uses. This one mounts the shell for real and lets the renderer's own
263-
// FRAME event (wired in `createAppShell`, unwired in `shell.dispose`)
264-
// drive the repaint, the same as production, with no direct calls to
265-
// `paintLanding` or `renderMark`.
262+
// uses. This one mounts the shell for real and lets it repaint itself:
263+
// no `paintLanding`/`renderMark` calls, and critically no `renderOnce`
264+
// loop either while waiting — a test that pumps frames by hand can stay
265+
// green even when production's self-driving mechanism is dead, which is
266+
// exactly the blind spot that let the throttled build ship frozen snow.
266267
await withTestRenderer(async (h) => {
267268
const shell = createAppShell(h.renderer, {
268269
terminal: { columns: 80, rows: 24 },
@@ -273,24 +274,13 @@ describe("landing screen", () => {
273274
await settle(h)
274275
const before = markRows(h).join("\n")
275276

276-
// A burst of frames faster than the ~125ms idle-repaint throttle
277-
// must not each produce a distinct paint: the FRAME event fires
278-
// from inside the render loop, so an unthrottled repaint here is
279-
// exactly the uncapped render-loop regression this guards against.
280-
const burstStart = Date.now()
281-
while (Date.now() - burstStart < 60) {
282-
await h.renderOnce()
283-
}
284-
expect(markRows(h).join("\n")).toBe(before)
285-
286-
// Real elapsed time, not an injected clock: the production driver
287-
// reads the wall clock, so this is the only way to exercise it.
288-
// At the fall speed in mark-anim.ts a few real seconds is enough
289-
// for at least one active flake column to cross a row boundary.
290-
const start = Date.now()
291-
while (Date.now() - start < 5_000) {
292-
await h.renderOnce()
293-
}
277+
// Real wall-clock wait, no renderOnce in between: only the mount's
278+
// own idle-repaint timer can be advancing the snow here. `flush`
279+
// waits on the renderer's own scheduler settling rather than
280+
// forcing frames, so it does not manufacture the motion itself.
281+
await new Promise((resolve) => setTimeout(resolve, 3_000))
282+
283+
await h.flush()
294284
const after = markRows(h).join("\n")
295285

296286
expect(after).not.toBe(before)

src/tui-opentui/shell.ts

Lines changed: 55 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ function dispatchOverlayAccept(
471471
/** Renderer surface required by the shell (CliRenderer / createTestRenderer). */
472472
export type ShellRenderer = Pick<
473473
CliRenderer,
474-
"root" | "width" | "height" | "keyInput" | "on" | "off"
474+
"root" | "width" | "height" | "keyInput" | "on" | "off" | "isDestroyed"
475475
>
476476

477477
export type AppShellOptions = {
@@ -1931,11 +1931,12 @@ type ShellInternals = {
19311931
/** Clock of the last painted mark frame, so a resize can redraw in place. */
19321932
landingNowMs: number
19331933
/**
1934-
* Wall-clock time of the last idle-driven landing repaint (the FRAME-event
1935-
* path in `createAppShell`'s `onFrame`), so that path can throttle itself
1936-
* to ~8fps instead of repainting on every render pass.
1934+
* Cancels the mount-scoped idle repaint timer (see `armLandingIdleTimer`
1935+
* in `createAppShell`), or null while none is armed. Cleared by whichever
1936+
* teardown happens first — the landing going away (`clearLandingMark`) or
1937+
* the whole shell disposing (`dispose`) — so it can never outlive either.
19371938
*/
1938-
landingLastIdlePaintMs: number
1939+
landingIdleTimerCancel: (() => void) | null
19391940
/** Chrome content (empty array = zone off). */
19401941
chrome: {
19411942
/**
@@ -2539,6 +2540,8 @@ function clearLandingMark(shell: AppShell): void {
25392540
const landing = bag?.landing
25402541
if (bag === undefined || landing === null || landing === undefined) return
25412542
bag.landing = null
2543+
bag.landingIdleTimerCancel?.()
2544+
bag.landingIdleTimerCancel = null
25422545
shell.transcript.remove(landing.above.box)
25432546
destroySubtree(landing.above.box)
25442547
shell.root.remove(landing.below)
@@ -2562,10 +2565,9 @@ function clearLandingMark(shell: AppShell): void {
25622565
}
25632566

25642567
/**
2565-
* Minimum spacing between idle-driven landing repaints (~8fps). The snow
2566-
* only needs to advance about half a row per second, so this is well above
2567-
* the animation's actual needs while staying far below the render engine's
2568-
* uncapped max rate.
2568+
* Cadence of the mount-scoped idle repaint timer (see `armLandingIdleTimer`
2569+
* in `createAppShell`). The snow only needs to advance about half a row per
2570+
* second, so ~8fps is comfortably enough to read as motion.
25692571
*/
25702572
const LANDING_IDLE_REPAINT_INTERVAL_MS = 125
25712573

@@ -2576,11 +2578,10 @@ const LANDING_IDLE_REPAINT_INTERVAL_MS = 125
25762578
*
25772579
* Always repaints while the landing is up, even when `animating` is false:
25782580
* the landing is idle by definition (no turn processing), and snow still
2579-
* needs to drift across a frozen mountain. Every call reassigns a fresh
2580-
* `StyledText` to every row regardless of whether its content changed, which
2581-
* unconditionally dirties the renderables and requests another render — the
2582-
* idle-driven call site in `createAppShell`'s `onFrame` throttles how often
2583-
* it calls this rather than relying on this function to skip unchanged rows.
2581+
* needs to drift across a frozen mountain. Driven by the mount-scoped timer
2582+
* armed in `createAppShell` (see `armLandingIdleTimer`) rather than a render
2583+
* event, so the repaint cadence is independent of however often the renderer
2584+
* happens to paint.
25842585
*/
25852586
export function paintLanding(
25862587
shell: AppShell,
@@ -5617,38 +5618,6 @@ export function createAppShell(
56175618
// starves that pass of room to lay the row out in.
56185619
syncTranscriptSpacer(shell)
56195620
syncNoticeAfterLayout(shell)
5620-
// The landing's snow needs a frame source that keeps running while the
5621-
// turn monitor is deliberately quiet (idle, no session yet). The renderer
5622-
// FRAME event is already scoped to shell lifetime (wired here, unwired in
5623-
// `dispose` below) and `paintLanding` no-ops once the landing tears down,
5624-
// so riding it costs no extra timer to arm or leak.
5625-
//
5626-
// Only re-enters while idle (`landingAnimating` already false): while a
5627-
// turn is processing, `paintPhaseAt` in runtime-bridge.ts drives the
5628-
// mountain's own draw/fill/fade loop off the turn monitor's clock, and
5629-
// this re-entry must not stomp that with an unrelated real-clock value
5630-
// every render pass.
5631-
//
5632-
// FRAME fires from inside the render loop itself, and `paintLanding`
5633-
// always reassigns fresh row content (see its docblock), which
5634-
// unconditionally dirties the renderables and requests another render.
5635-
// Left unthrottled that turns into a perpetual render loop at the
5636-
// engine's max frame rate rather than the app's configured target, for
5637-
// as long as the landing sits idle on screen. The snow only needs to
5638-
// advance about half a row per second, so gating repaints to roughly
5639-
// every `LANDING_IDLE_REPAINT_INTERVAL_MS` keeps the animation smooth
5640-
// at a fraction of the render cost.
5641-
const landingBag = internals.get(shell)
5642-
if (landingBag?.landing != null && !landingBag.landingAnimating) {
5643-
const nowMs = Date.now()
5644-
if (
5645-
nowMs - landingBag.landingLastIdlePaintMs >=
5646-
LANDING_IDLE_REPAINT_INTERVAL_MS
5647-
) {
5648-
landingBag.landingLastIdlePaintMs = nowMs
5649-
paintLanding(shell, nowMs, false)
5650-
}
5651-
}
56525621
}
56535622

56545623
const onResize = (width: number, height: number): void => {
@@ -5737,6 +5706,7 @@ export function createAppShell(
57375706
}
57385707
renderer.off(CliRenderEvents.FRAME, onFrame)
57395708
renderer.off(CliRenderEvents.RESIZE, onResize)
5709+
internals.get(shell)?.landingIdleTimerCancel?.()
57405710
flashTimers.get(shell)?.()
57415711
flashTimers.delete(shell)
57425712
try {
@@ -5776,10 +5746,48 @@ export function createAppShell(
57765746
landingSuggestionsVisible: true,
57775747
landingAnimating: false,
57785748
landingNowMs: 0,
5779-
landingLastIdlePaintMs: 0,
5749+
landingIdleTimerCancel: null,
57805750
chrome: { task: [], tasksRaw: [], agents: [] },
57815751
tasksPanelHidden: false,
57825752
})
5753+
// The landing's snow needs a frame source that keeps running while the
5754+
// turn monitor is deliberately quiet (idle, no session yet). A plain timer
5755+
// armed at mount is that source: it does not depend on the renderer
5756+
// scheduling further frames, so it cannot stall the way riding the
5757+
// renderer's own FRAME event did (see CL-5737 history in the PR).
5758+
//
5759+
// Only repaints while idle (`landingAnimating` false): while a turn is
5760+
// processing, `paintPhaseAt` in runtime-bridge.ts drives the mountain's
5761+
// own draw/fill/fade loop off the turn monitor's clock, and this timer
5762+
// must not stomp that with an unrelated real-clock value.
5763+
//
5764+
// Cleared on whichever teardown happens first: the landing going away
5765+
// (`clearLandingMark`, first transcript row) or the whole shell disposing
5766+
// (`dispose` below, e.g. tests that never grow a transcript).
5767+
//
5768+
// Also self-cancels on `renderer.isDestroyed`: a real terminal session
5769+
// always disposes the shell, but headless test harnesses commonly destroy
5770+
// the renderer directly (`withTestRenderer`'s cleanup) without ever
5771+
// calling `shell.dispose()`. Without this check the timer would keep
5772+
// firing against renderables the harness already tore down.
5773+
const landingIdleHandle = setInterval(() => {
5774+
if (renderer.isDestroyed) {
5775+
clearInterval(landingIdleHandle)
5776+
return
5777+
}
5778+
const bag = internals.get(shell)
5779+
if (bag?.landing == null || bag.landingAnimating) return
5780+
paintLanding(shell, Date.now(), false)
5781+
}, LANDING_IDLE_REPAINT_INTERVAL_MS)
5782+
landingIdleHandle.unref?.()
5783+
{
5784+
const bag = internals.get(shell)
5785+
if (bag !== undefined) {
5786+
bag.landingIdleTimerCancel = () => clearInterval(landingIdleHandle)
5787+
} else {
5788+
clearInterval(landingIdleHandle)
5789+
}
5790+
}
57835791
transcriptSpacers.set(shell, transcriptSpacer)
57845792
if (onCommandOpt) setPaletteOnCommand(shell, onCommandOpt)
57855793
if (onObserveRequestOpt) {

0 commit comments

Comments
 (0)