fix(tui): heal focus regain without a separate screen clear (supersedes #88596) - #89623
Conversation
The focus-in handler must repaint from scratch: some emulators throttle hidden-tab output, so Ink's virtual frame can claim a row is already blank while the physical screen still shows the old status/progress text. A buffer-only reset does not fix that — the cell diff skips blank-over-blank, so the stale row survives. But the clear must not be its own write. forceRedraw() emits stdout.write(ERASE_SCREEN + CURSOR_HOME) and then the frame, so an ordinary tab or pane switch flashes an empty screen between the two. Queue the clear via needsEraseBeforePaint instead of writing it directly. That folds it into the frame's patch list, so clear+paint reach the terminal in a single write and no blank frame can be presented. The alt screen already had this mechanism for resize; extend it to the main screen (INLINE_MODE / Termux), which previously had no in-band erase path at all. The flag is always consumed but only emitted when the frame actually repaints, so a queued erase can never ride a later incremental frame (spinner tick) and wipe content that frame does not redraw. Replaces the emitted-bytes assertions with screen-state ones: the tests now replay the emitted ANSI into a terminal model and assert what the user sees — stale row gone, content present and not duplicated, exactly one erase and it shares a write with the repaint — across both the alt screen and the main screen. Asserting "no ERASE_SCREEN" is what let the regression through: it passes precisely when the healing is removed. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
૮ >ﻌ< ა ci reviewran on 074c2b1 — fix(tui): scope CSI 3J to resize and re-assert modes on focu
|
There was a problem hiding this comment.
Pull request overview
This PR updates the Hermes Ink renderer’s focus-regain (“focus-in”) recovery path to preserve the existing stale-row “healing” behavior while eliminating visible flicker by ensuring the clear-screen sequence is emitted atomically with the repaint (i.e., in a single stdout.write).
Changes:
- Reworks focus-regain recovery to queue an erase via
needsEraseBeforePaintand repaint immediately (instead of usingforceRedraw()’s standalone erase write). - Adds a main-screen (
INLINE_MODE/ Termux)onRenderbranch that injectsERASE_THEN_HOMEinto the patch list when a full repaint is requested. - Replaces prior byte-level assertions with screen-state assertions via a minimal ANSI replay “terminal model” test harness.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ui-tui/packages/hermes-ink/src/ink/ink.tsx | Changes focus-regain recovery to do an in-band erase+paint and adds main-screen patch injection for atomic clear+repaint. |
| ui-tui/packages/hermes-ink/src/ink/ink-focus-redraw.test.ts | Adds regression tests that validate “healing” and “atomicity” by replaying emitted ANSI into a terminal model. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Two defects from the review of the previous commit. Scrollback erase leaked to focus regain. Reusing needsEraseBeforePaint routed focus-in through the same erase selection as resize healing, whose heuristic is TERM_PROGRAM == 'Apple_Terminal' — so on Apple Terminal an ordinary tab or pane switch emitted CSI 3J and wiped the user's scrollback. That erase exists to clear alt-screen reflow artifacts after a resize, which is the only case worth discarding history for. Track the deep erase behind its own flag, set only by resize healing; every other requester gets 2J. Terminal modes were never re-asserted. #88596 dropped reassertTerminalModes(false) from the focus path and the previous commit did not restore it, leaving one caller (onStdinResume). An emulator that cleared the DEC mouse modes while the pane was hidden then stayed dead until the DECRQM watchdog's next 2s probe. Restore the non-destructive call — extended keys plus mouse preset, no alt-screen re-entry, no erase — so it costs a few idempotent bytes and no flicker. Both are covered: reverting either fix fails its test. The mode assertion is checked on the alt screen only, since mouse tracking is alt-screen-scoped and reassertTerminalModes returns early on main. Reported-by: Copilot
…-regain-atomic-repaint fix(tui): heal focus regain without a separate screen clear (supersedes NousResearch#88596)
Summary
Supersedes #88596 (@helix4u), whose report is correct: focus regain flashes the screen on an ordinary tab or pane switch. The flicker is real and measurable —
forceRedraw()writesERASE_SCREEN + CURSOR_HOMEand then the frame, so the terminal can present a blank screen between the two writes.The fix in #88596 removes the clear entirely, which removes the healing the handler exists for (#86332). This PR keeps @helix4u's commit and framing and fixes the flicker the other way: the clear stays, but it stops being its own write.
Root cause
Two separate properties were conflated:
stdout.write().needsEraseBeforePaintalready folds an erase into the frame's patch list so clear+paint land in one write; the alt screen used it for resize, and the main screen (INLINE_MODE/ Termux) had no in-band erase path at all.Changes
needsEraseBeforePaintinstead of callingforceRedraw(), on both screens. Clear+paint now reach the terminal in a single write, so no blank frame can be presented.onRenderthat unshiftsERASE_THEN_HOMEinto the patch list. No cursor park (main-screen cursor position is meaningful — it's the prompt row — and log-update already restores it) and noCSI 3J(scrollback is the user's history here, not a resize artifact).reassertTerminalModes(false)is restored on the focus path (it had been dropped by fix(tui): avoid destructive redraw on focus regain #88596): it also re-arms mouse tracking, which the DECRQM watchdog (fix(tui): recover mouse tracking without a resize via DECRQM watchdog #66080) otherwise only recovers on its 2s probe.CSI 3J) is scoped to resize healing via its own flag. ReusingneedsEraseBeforePaintalone would have inherited resize'sApple_Terminalheuristic and wiped scrollback on an ordinary tab switch.Tests
The previous test asserted on emitted bytes (
not.toContain(ERASE_SCREEN)). That contract is what let the regression through — it passes precisely when the healing is removed, and it would block this fix.Replaced with screen-state assertions: the tests replay the emitted ANSI into a terminal model and check what the user actually sees, across both the alt screen and the main screen.
write()with the repaint.The suite is a real guard, not a rubber stamp:
mainValidation
vitest(packages/hermes-ink/src/ink/)npm run typecheck(ui-tui)eslint(changed files)prettier --checkAlso verified
prepareAltScreenResizeRepaintis the only other writer ofneedsEraseBeforePaintand is alt-screen-gated, so resize behavior is unchanged and the new main-screen branch is reachable only from focus regain.Python tests not run — TypeScript-only change.
Supersedes #88596.
Co-authored-by: Gille 4317663+helix4u@users.noreply.github.com