Skip to content

Rewrite terminal redraw: pipe PTY output straight to xterm#173

Merged
jsgrrchg merged 3 commits into
jsgrrchg:mainfrom
spamsch:terminal-redraw-pipeline
May 30, 2026
Merged

Rewrite terminal redraw: pipe PTY output straight to xterm#173
jsgrrchg merged 3 commits into
jsgrrchg:mainfrom
spamsch:terminal-redraw-pipeline

Conversation

@spamsch
Copy link
Copy Markdown
Contributor

@spamsch spamsch commented May 29, 2026

Why

The terminal reconstructed a byte delta on every render by string-diffing an accumulated rawOutput buffer against its previous value, then writing that guess to xterm. For a redraw-heavy TUI like Claude Code this fell apart:

  • once rawOutput crossed the 2 MB cap it was front-trimmed, so it no longer matched the previous value;
  • the truncation probe that tried to recover the delta was defeated by the repeated escape sequences TUIs emit (spinners, token counters);
  • the fallback was terminal.reset() + a full rewrite — a full-screen flush, i.e. the flicker.

The earlier attempt to patch this in-place (#156: Shift+Enter, immediate resize, and a truncation probe) did not solve the redraw problem — the probe was a workaround for a self-inflicted wound. That PR is now closed. The Shift+Enter and immediate-resize fixes from it already landed on main and are preserved here; the probe is removed entirely.

What

Adopt Hyper's model: xterm owns the screen buffer, and PTY chunks are written to it directly, in order, once.

  • Store delivers output through a per-terminal command channel ({write,clear}) with a small pre-mount backlog. rawOutput is no longer the screen source of truth; a hasOutput flag drives empty-state UI.
  • Viewport subscribes and calls terminal.write() directly. The diff/probe/reset effect and the local watermark write-queue are deleted.
  • Reattach uses @xterm/addon-serialize: the snapshot is restored on mount, saved on unmount (and debounced), and mirrored to safeStorage so content survives a full reload.
  • Backend decodes PTY reads with a carry buffer (Node StringDecoder-style) so a multi-byte UTF-8 sequence split across a 4 KB read boundary no longer corrupts box-drawing / emoji / powerline glyphs into U+FFFD.
  • AIAuthTerminalModal adopts the new TerminalSessionView interface with a local emitter, keeping its text accumulator only for sign-in success scanning.

Result

Confirmed working in a packaged build against a live Claude Code session: no flicker on long, redraw-heavy output; glyphs intact; tab-switch and reload restore screen state.

Tests

  • Removed the obsolete truncation-probe and watermark-backlog tests.
  • Added coverage for the output channel (live ordering, pre-subscribe flush), clear, replay-snapshot round-trip + persistence, channel teardown on close, and the snapshot-on-unmount path.
  • Backend: 5 unit tests for the UTF-8 carry decoder (2/3/4-byte splits, invalid-byte replacement, complete-input no-carry).
  • Full desktop suite green (1687 passed); lint clean.

Known limitation

After a full reload, output emitted by the PTY while the renderer was down is not recovered — only the pre-reload screen snapshot is restored. This is inherent to the renderer-side serialize-addon approach.

Tracking the upstream Ink cursor desync that underlies the original redraw bug: anthropics/claude-code#62740

@spamsch spamsch marked this pull request as draft May 29, 2026 13:27
@spamsch spamsch force-pushed the terminal-redraw-pipeline branch from b3aefbf to d93ed50 Compare May 29, 2026 13:29
The terminal reconstructed a byte delta on every render by string-diffing
an accumulated rawOutput buffer against its previous value, then writing
that guess to xterm. For a redraw-heavy TUI like Claude Code this fell
apart: once rawOutput crossed the 2MB cap it was front-trimmed, the
truncation probe was defeated by repeated escape sequences, and the
fallback was terminal.reset() + a full rewrite — a full-screen flush,
i.e. flicker.

Switch to Hyper's model: xterm owns the screen buffer and PTY chunks are
written to it directly, in order, once.

- Store delivers output through a per-terminal command channel
  ({write,clear}) with a small pre-mount backlog; rawOutput is no longer
  the screen source of truth (a hasOutput flag drives empty-state UI).
- Viewport subscribes and calls terminal.write() directly; deletes the
  diff/probe/reset effect and the watermark write-queue.
- Reattach uses @xterm/addon-serialize: snapshot restored on mount, saved
  on unmount and debounced, mirrored to safeStorage for reload survival.
- Backend decodes PTY reads with a carry buffer (StringDecoder-style) so
  multi-byte UTF-8 split across a 4KB read boundary no longer corrupts
  box-drawing/emoji/powerline glyphs into U+FFFD.
- AIAuthTerminalModal adopts the new TerminalSessionView interface with a
  local emitter, keeping its text accumulator only for success scanning.

This also folds in prior terminal work that had not reached upstream:
Shift+Enter inserting a newline, the immediate first PTY resize, and the
dictation overlay. The earlier in-place patch (truncation probe) is
removed — it was a workaround for the accumulator the rewrite deletes.

Tests: remove the obsolete probe/watermark tests; add coverage for the
output channel, clear, replay snapshot persistence, channel teardown, and
the UTF-8 carry decoder. Full desktop suite green; lint clean.
@spamsch spamsch force-pushed the terminal-redraw-pipeline branch from d93ed50 to 1ae94a4 Compare May 29, 2026 13:41
@spamsch
Copy link
Copy Markdown
Contributor Author

spamsch commented May 29, 2026

Finally this one feels right. No artifacts all features working.

@spamsch spamsch marked this pull request as ready for review May 29, 2026 14:01
@jsgrrchg
Copy link
Copy Markdown
Owner

Thank you Simon, this looks like a much cleaner direction.

I really appreciate the detailed explanation and the care you put into replacing the truncation-probe approach instead of trying to patch around it. The direct xterm pipeline makes a lot more sense, especially for Claude Code and other redraw-heavy TUIs.

I’ll test this throughout the day in real usage and report back if I find anything weird. Thanks again 😊

@jsgrrchg
Copy link
Copy Markdown
Owner

Reviewed the approach, and the tradeoff looks right to me. It is honestly wild how hard it is to make a good terminal. I tested it heavily yesterday, and it is working very well, I'm impressed.

I also pushed a very small fix: the mocked unlisten callback is now typed as () => void, matching the runtime listen contract. vi.fn() worked at runtime, but TypeScript inferred a broader Vitest mock type, which caused the build to fail.

Moving screen ownership to xterm simplifies the pipeline a lot and removes the most fragile part of the previous model, accumulating rawOutput, trimming it, and then trying to reconstruct byte deltas from it.

The full-reload limitation also seems acceptable and clearly explained. Restoring the latest snapshot covers the important remount/tab-switching/reload visual cases, while recovering output emitted while the renderer is down would require persistent backend buffering/replay and quite a bit more complexity.

Looks very good to me, I'll be merging when checks go green.

@jsgrrchg jsgrrchg merged commit f774282 into jsgrrchg:main May 30, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants