fix: clear in place on CSI 2J at home position (#9181)#10743
Conversation
When a primary-screen TUI emits CSI 2J with the cursor already at the home position (0, 0), treat the sequence as a full-frame redraw and clear in place rather than pushing the prior frame into block-level scrollback via `clear_viewport`. This mirrors xterm/ghostty/iTerm2 semantics for CSI 2J and addresses the "Terminal Screen Replicates Throughout Session" symptom reported for Claude Code (and other inline-rendering CLI agents that do not opt into the alt screen and are not yet detected as CLI agents by Warp). Without this guard each redraw cycle duplicated the visible region into the block's scrollback, growing unboundedly over the life of the session. The legacy scroll-into-scrollback path is preserved for non-home CSI 2J (notably the shell `clear` command path), so existing user expectations there are unchanged. Closes warpdotdev#9181
github-actions
Bot
added
the
external-contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR changes primary-screen CSI 2J handling so a full-screen erase at cursor home clears visible rows in place, with regression tests for repeated inline TUI redraws.
Concerns
⚠️ The cursor-at-home heuristic also matches common shellclearsequences such asxterm-256colorterminfo'sESC[H ESC[2J, so runningclearfrom a non-home prompt would take the new in-place path after the preceding home command and stop preserving the visible region in scrollback.⚠️ This is a user-visible terminal behavior change, but the PR does not include screenshots or a screen recording showing the behavior end to end, nor a justification for why manual visual validation is not possible.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| self.grid.region_mut(..).each(|cell| *cell = bg.into()); | ||
| } else if self.full_grid_clear_behavior == FullGridClearBehavior::Clear { | ||
| self.clear_visible_rows_in_place(bg); | ||
| } else if self.is_tui_redraw_clear() { |
There was a problem hiding this comment.
clear output because terminfo entries like xterm-256color use ESC[H ESC[2J; after the preceding home command, this branch runs and stops preserving the visible region in scrollback despite the stated legacy behavior. Distinguish redraws from shell clear using more than the current cursor position, or update the behavior/tests intentionally.
…ve shell clear (warpdotdev#9181) The previous fix used cursor-at-home as the sole signal to switch CSI 2J to the in-place clear path. As flagged by oz-for-oss review, xterm-256color terminfo's `clear` capability emits `CSI [H` followed by `CSI [2J` — so the cursor IS at home when 2J fires for the shell `clear` command, and the cursor-at-home heuristic alone would silently destroy the visible region on every `clear` (no scrollback preserved). Add a cadence check: an at-home CSI 2J only takes the in-place path when a prior at-home CSI 2J fired within `TUI_REDRAW_CADENCE_WINDOW` (500ms). Inline-rendering CLI agents (Claude Code et al.) emit redraws many frames per second, so all redraws after the first frame fall inside the window; an interactive `clear` invocation happens at human speed and never does. The first at-home 2J in a session therefore behaves like the legacy shell-`clear` path (scrollback preserved). The unbounded duplication symptom from GH warpdotdev#9181 is gated on the second-onward frames staying in place, which the existing repeated-redraw test asserts and the new shell-clear test pins from the opposite direction. Test plan: - Adds `test_shell_clear_command_preserves_scrollback_when_not_in_cli_agent_redraw_loop` reproducing the xterm-256color `clear` byte sequence and asserting legacy scrollback behavior. - Updates the two existing TUI-redraw tests to seed the cadence via a new `#[cfg(test)] seed_tui_redraw_cadence_for_test()` helper so they isolate the in-place clear behavior from cadence detection. - Non-home and shell-clear tests both continue to preserve scrollback.
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
oz-for-oss
Bot
dismissed
their stale review
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR updates primary-screen CSI 2J handling to detect rapid at-home redraw loops and clear the visible grid in place instead of pushing every frame into scrollback. It preserves the existing scrollback-producing path for non-home clears and one-off at-home shell clears, and adds regression coverage for the TUI redraw and shell clear cases.
Concerns
- No blocking correctness or security concerns found in the annotated diff.
- The changed terminal behavior is user-visible, but the PR body includes a manual testing limitation for this worktree and explains the available validation path.
- No matching individual stakeholder was listed for
app/src/terminal/model/grid/; only the catch-all team rule applies, sorecommended_reviewersis left empty for the workflow fallback.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
oz-for-oss
Bot
requested review from
a team and
peicodes
and removed request for
a team
1 participant
Closes #9181
Summary
CSI 2Jhandler taking theclear_viewport()path, which scrolls the prior visible region into block-level scrollback before re-rendering.CSI [H(cursor home) followed byCSI 2Jmany times per second to redraw their frame. Each redraw cycle duplicates the previous frame into the block's scrollback, producing the unbounded "replication" the user reports.ansi_handler::clear_screenforansi::ClearMode::All: when the cursor is already at the home position (VisibleRow(0), col0) and a prior at-homeCSI 2Jfired within the recent cadence window (TUI_REDRAW_CADENCE_WINDOW = 500ms), and the alt-screen /FullGridClearBehavior::Clearpaths are not engaged, clear the visible rows in place via the existingclear_visible_rows_in_placehelper instead ofclear_viewport().CSI 2Jand for the first at-homeCSI 2Jin a long while — the latter case is what the shellclearcommand produces underxterm-256colorterminfo, which emitsCSI [Hfollowed byCSI [2J.Why this is the right shape of fix
Implement full-frame clear for active block for CLI Agents.), which solved the same family of bug for the resize code path once Warp had already detected the CLI agent. This change covers the redraw code path before detection registers (or when the agent is not detected at all — e.g. third-party agents orclaudeinvoked without the Warp plugin), which matches both the symptom timeline and the "Yes, this issue prevents me from using Warp daily" severity in the report.xterm/ghostty/iTerm2semantics forCSI 2J: those terminals do not push the prior frame into scrollback on full-screen erase. Warp's behavior of conflating "user clear screen" with "TUI redraw" is the non-standard interpretation that caused the regression in the first place.xterm-256colorterminfo'sclearcapability isCSI [HthenCSI [2J, so the cursor is at home when2Jfires for the shellclearcommand too. The cadence check is what cleanly disambiguates the two cases: inline-rendering CLI agents emit many at-homeCSI 2Jper second, while an interactive user runsclearat human speed. The first at-home2Jin a long while is therefore treated as a one-offclearand preserves the visible region in scrollback; subsequent rapid ones are treated as TUI redraws and clear in place.Test plan
Added four regression tests in
app/src/terminal/model/grid/grid_handler_tests.rs:test_clear_screen_all_primary_tui_redraw_at_home_clears_in_place— the core regression: home +CSI 2Jon the default primary-screen path (with active TUI redraw cadence) must leavehistory_size() == 0.test_clear_screen_all_primary_repeated_tui_redraws_do_not_duplicate— drives 10 redraw cycles and asserts the scrollback does not grow.test_clear_screen_all_primary_non_home_cursor_preserves_scrollback— the non-homeCSI 2Jpath must continue to push the visible region into scrollback.test_shell_clear_command_preserves_scrollback_when_not_in_cli_agent_redraw_loop— new: directly addresses the bot review concern. Reproduces the xterm-256color shellclearbyte sequence (CSI [Hfollowed byCSI [2J) with no active TUI redraw cadence and asserts that scrollback is preserved (legacy behavior).The existing tests
test_clear_screen_all_primary_preserves_visible_rows_in_history_by_default,test_clear_screen_all_primary_with_full_grid_clear_behavior_clears_in_place, andtest_clear_screen_all_alt_screen_clears_in_placecontinue to cover the unchanged paths and were not modified.Manual testing note
This worktree environment lacks the Xcode
metaltoolchain (xcrun: error: unable to find utility "metal"), socargo check -p warp --libandcargo check -p warp_terminalcould not complete locally — both transitively depend onwarpui's Metal-shader build step. The change was validated by direct review of the surroundingclear_screen/clear_visible_rows_in_place/clear_viewportpaths and the new unit tests above (which exercise only theGridHandlerand have no GPU dependency). CI on the upstream repo will exercise the full crate-scoped build and test suite.The cadence window of 500ms is chosen to be a generous upper bound on inline-CLI-agent frame intervals (Claude Code emits multiple redraws per second under any non-trivial streaming load) while leaving a wide margin above any plausible human-interactive
clearcadence (a user mashingclearat the keyboard typically achieves ~3-5 invocations per second at most, and crucially does not chain them with no intervening prompt — but even back-to-backclearinvocations within 500ms would simply behave like an in-place redraw, which is visually indistinguishable from the legacy scroll-into-scrollback path when the prior frame is already blank from the firstclear).🤖 Generated with Claude Code