fix(pi-tui): handle above-viewport changes without a destructive redraw - #2133
fix(pi-tui): handle above-viewport changes without a destructive redraw#2133UoMaster wants to merge 1 commit into
Conversation
A change above the viewport (e.g. a subagent status row ticking during streaming) triggered a full redraw emitting ESC[2J/ESC[3J, which wiped scrollback and yanked the user's scroll position to the bottom on every frame. Clamp in-place edits to the visible window, and re-anchor the viewport on above-window length changes with only the mismatched window rows repainted in place — no scrolling, no scrollback clear. Collapses, out-of-range anchors, and kitty image frames keep the destructive fallback. Closes MoonshotAI#2098
🦋 Changeset detectedLatest commit: d552357 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d55235737b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // by delta rows. The terminal already displays the shifted rows, | ||
| // so re-anchor the viewport and repaint only the window rows | ||
| // whose content actually differs — in place, with no scrolling. | ||
| const newViewportTop = prevViewportTop + delta; |
There was a problem hiding this comment.
When a frame both edits a row above the window and appends output at the tail, this line treats the net total length delta as if the insertion happened above the viewport. In that mixed frame (for example, a ticking subagent status row plus a streamed line in the same render batch), the branch repaints the new tail in place and returns without emitting the newline scroll that would commit the old top viewport row to scrollback, so the renderer's previousViewportTop advances while the terminal buffer does not and one history row is lost from scrollback on subsequent appends. The non-destructive re-anchor rule is scoped to length changes above the window, so this path needs to distinguish where the insertion/deletion occurred before re-anchoring.
AGENTS.md reference: packages/pi-tui/AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| const delta = newLines.length - this.previousLines.length; | ||
| if (delta === 0) { |
There was a problem hiding this comment.
For same-height changes above the viewport, this branch can skip or clamp past changed Kitty image lines without ever calling deleteChangedKittyImages or doing the documented full-redraw fallback. If an off-screen image placement is replaced/removed while the line count stays the same, previousKittyImageIds is then overwritten with the new union, so the old image ID is no longer tracked for later deletion and stale Kitty graphics can remain in scrollback. The package rule explicitly keeps destructive fallback for frames involving Kitty image lines.
AGENTS.md reference: packages/pi-tui/AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
1 participant
Related Issue
Resolve #2098
Problem
While a subagent runs, status rows above the visible window (the subagent header's spinner/elapsed timer ticking at 80ms, live progress lines) kept marking content above the viewport as changed. Every such frame took the "first changed line is above the viewport" branch and did a destructive full redraw (
ESC[2J/ESC[3J), which cleared scrollback and snapped the terminal back to the bottom — making it impossible to scroll up and read history during subagent execution (macOS and Windows).What changed
In
packages/pi-tui's differential renderer, the above-viewport branch no longer falls back to a destructive full redraw:delta === 0, e.g. the subagent header tick): the diff is clamped to the visible window; when nothing visible changed, nothing is painted and only the frame caches are updated.ESC[3J./clear, compaction), out-of-range anchors, and frames involving kitty image lines.This is deliberately narrower than the fork's earlier viewport/scrollback patches (reverted in
23daf0f3): it never scroll-commits and never repaints a rewound window, which is where the blank-screen / duplicated-scrollback edge cases came from. Accepted trade-offs, documented in code andpackages/pi-tui/AGENTS.md: scrollback keeps stale bytes for changed above-window rows (a terminal cannot edit scrollback without clearing), and a shrink rewind may later re-commit a bounded span.Tests: a new
TUI above-viewport changessuite pins the behavior — in-place edits, mixed visible edits, a 20-frame subagent-style ticking simulation (asserts zero full redraws, noESC[3J, byte-identical scrollback), insertion/shrink re-anchoring with commit-continuity checks, mixed shift+edit frames, and the collapse fallback. The pre-existing transient-component test now asserts the non-destructive behavior. Fullpi-tuisuite (743 tests) and thekimi-codevitest suite (2380 tests) pass.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.