From f7484812477b347489a5010be29613495728fe58 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:34:08 -0700 Subject: [PATCH] fix(cli): refill only the viewport on redraw and erase it without CSI 2J The bounded replay still stacked a copy per resize/Ctrl+L: counting logical lines against the terminal height ignores soft-wrapped rows and the prompt chrome, and CSI 2J makes scroll-on-clear terminals (tmux, VTE) copy the whole visible screen into scrollback before the replay repaints it. - _clear_prompt_toolkit_screen erases the viewport row by row (EL) when scrollback is kept and returns the room above the chrome: terminal rows minus the layout's preferred height, at the current width. The 3J rebuild path (display.cli_rebuild_scrollback_on_redraw) keeps 2J+3J and replays the whole history, which is what rebuilds scrollback. - _replay_output_history(fit) keeps the newest lines whose wrapped height fits that room (_output_tail_fitting in cli_render). - the resize path budgets against the full chrome: the status bar/rules hidden while the reflow settles come back and pushed the top replayed rows into scrollback. Tests: the two tests that pinned CSI 2J on the keep-scrollback path now assert the row erase; replay stubs take the fit argument; the salvaged visible-height tests collapse into one wrapped-height invariant. Fixes #95375 --- cli.py | 28 +---- hermes_cli/cli_render.py | 14 +++ hermes_cli/cli_terminal_mixin.py | 41 +++++-- tests/hermes_cli/test_cli_force_redraw.py | 130 ++++++++-------------- tests/hermes_cli/test_cli_pet_pane.py | 2 +- 5 files changed, 97 insertions(+), 118 deletions(-) diff --git a/cli.py b/cli.py index 296ac5f899..ae6a483b44 100644 --- a/cli.py +++ b/cli.py @@ -109,6 +109,7 @@ from hermes_cli.cli_render import ( # noqa: F401,E402 _luminance_from_hex, _maybe_remap_for_light_mode, _output_history_recording, + _output_tail_fitting, _panel_box_width, _post_stream_transform_output, _prepend_note_to_message, @@ -646,22 +647,11 @@ def _suspend_output_history(): _OUTPUT_HISTORY_SUPPRESSED = old_value -def _visible_terminal_rows() -> int: - """Return the terminal's current visible row count (fallback 24).""" - try: - return shutil.get_terminal_size((80, 24)).lines - except Exception: - return 24 - - -def _replay_output_history() -> None: +def _replay_output_history(fit=None) -> None: """Repaint recent output above the prompt after a full screen clear. - The pre-replay clear only wipes the *visible* viewport (CSI 2J), not the - scrollback, so re-printing the full ``_OUTPUT_HISTORY`` buffer on every - resize/redraw would append a duplicate copy of the history to scrollback - (#95375). Bound the replay to the visible terminal height so a redraw - restores the visible transcript without stacking duplicate blocks. + ``fit=(rows, columns)`` replays only the newest lines whose wrapped height fits + ``rows`` — the older ones are still in scrollback (#95375). """ global _OUTPUT_HISTORY_REPLAYING if not _OUTPUT_HISTORY_ENABLED or not _OUTPUT_HISTORY: @@ -679,15 +669,9 @@ def _replay_output_history() -> None: if isinstance(lines, str): lines = lines.splitlines() rendered_lines.extend(str(line) for line in lines) + if fit is not None: + rendered_lines = _output_tail_fitting(rendered_lines, *fit) if rendered_lines: - # Only re-print what fits on the visible screen. The clear that - # precedes this replay wipes the viewport (CSI 2J) but not the - # scrollback, so re-printing the whole buffer would append a - # duplicate block to scrollback on every resize/redraw. Bounding - # the replay to the visible row count keeps the transcript intact - # without stacking duplicates. - visible_rows = max(1, _visible_terminal_rows()) - rendered_lines = rendered_lines[-visible_rows:] # One payload: per-line pt prints each force a sync redraw (a waterfall of old output). _pt_print(_PT_ANSI("\n".join(rendered_lines))) except Exception: diff --git a/hermes_cli/cli_render.py b/hermes_cli/cli_render.py index 64001dfd0e..6ce661b16e 100644 --- a/hermes_cli/cli_render.py +++ b/hermes_cli/cli_render.py @@ -497,6 +497,20 @@ def _record_output_history(text: str) -> None: _cli()._OUTPUT_HISTORY.extend(str(text).replace("\r", "").rstrip("\n").splitlines()) +def _output_tail_fitting(lines: list[str], max_rows: int, columns: int) -> list[str]: + """Newest ``lines`` whose soft-wrapped height at ``columns`` fits in ``max_rows``.""" + from prompt_toolkit.formatted_text import ANSI, fragment_list_width, to_formatted_text + kept, used = [], 0 + for line in reversed(lines): + width = fragment_list_width(to_formatted_text(ANSI(line))) + used += max(1, -(-width // columns)) if columns > 0 else 1 + if used > max_rows: + break + kept.append(line) + kept.reverse() + return kept + + def _pt_print_ansi(text: str) -> None: """``_pt_print(ANSI(text))``, falling back to ``print`` when stdout is not a real console.""" from cli import _PT_ANSI, _pt_print diff --git a/hermes_cli/cli_terminal_mixin.py b/hermes_cli/cli_terminal_mixin.py index 3115df2b16..d1b1b592af 100644 --- a/hermes_cli/cli_terminal_mixin.py +++ b/hermes_cli/cli_terminal_mixin.py @@ -128,10 +128,10 @@ class CLITerminalMixin: app = getattr(self, "_app", None) if not app: return - self._clear_prompt_toolkit_screen(app, rebuild_scrollback=self._redraw_rebuilds_scrollback()) + fit = self._clear_prompt_toolkit_screen(app, rebuild_scrollback=self._redraw_rebuilds_scrollback()) if getattr(self, "_terminal_io_broken", False): return - _replay_output_history() + _replay_output_history(fit) self._pet_queue_kitty_frame() self._app_invalidate(app, "force_full_redraw", swallow=True) @@ -186,30 +186,48 @@ class CLITerminalMixin: pass self._force_full_redraw() - def _clear_prompt_toolkit_screen(self, app, *, rebuild_scrollback: bool = False) -> None: - """Clear the terminal and reset prompt_toolkit renderer state.""" + def _clear_prompt_toolkit_screen(self, app, *, rebuild_scrollback: bool = False): + """Clear the terminal and reset prompt_toolkit renderer state. + + Returns ``(rows, columns)`` of transcript room above the prompt chrome for the + replay that follows, or ``None`` when the whole history should be replayed + (scrollback wiped by CSI 3J, or the clear failed). Without 3J the older transcript + stays in scrollback, so the replay may only refill the viewport (#95375); the + viewport is erased row by row because CSI 2J makes scroll-on-clear terminals + (tmux, VTE) copy the whole screen into scrollback first, stacking a duplicate. + """ if getattr(self, "_terminal_io_broken", False): - return + return None try: renderer = app.renderer out = renderer.output out.reset_attributes() - out.erase_screen() + fit = None if rebuild_scrollback: + out.erase_screen() try: out.write_raw("\x1b[3J") except Exception: pass + else: + size = out.get_size() + for row in range(size.rows): + out.cursor_goto(row, 0) + out.erase_end_of_line() + chrome = app.layout.container.preferred_height(size.columns, size.rows).preferred + fit = (max(0, size.rows - chrome), size.columns) out.cursor_goto(0, 0) out.flush() # Drop cached screen + cursor state so the next _redraw() starts from a # known (0, 0) origin and re-renders every cell instead of diffing stale. renderer.reset(leave_alternate_screen=False) + return fit except OSError as exc: if _is_eio(exc): self._mark_terminal_io_broken("clear_screen") except Exception: pass + return None def _recover_after_resize(self, app, original_on_resize) -> None: """Recover a resized classic CLI without desynchronizing cursor state. @@ -221,7 +239,7 @@ class CLITerminalMixin: already-painted rows into scrollback first, so a fresh bar looks duplicated (#19280, #22976). Suppression cannot erase the already-reflowed OLD bar (``renderer.erase()`` uses ``_cursor_pos.y`` cached at the OLD width), so on an - OBSERVED width change we wipe the viewport (CSI 2J, banner-safe; 3J only via + OBSERVED width change we wipe the viewport (banner-safe; 3J only via ``display.cli_rebuild_scrollback_on_redraw``) and replay the transcript first. Same-width SIGWINCH (tmux attach, GNOME tab bar, focus) and the first signal without a seeded baseline are left alone — 2J+replay against preserved scrollback @@ -234,7 +252,6 @@ class CLITerminalMixin: if getattr(getattr(self, '_subagent_monitor', None), 'opening', False): return from cli import _replay_output_history - self._status_bar_suppressed_after_resize = True try: new_width = self._get_tui_terminal_width() except Exception: @@ -242,12 +259,16 @@ class CLITerminalMixin: prev_width = getattr(self, "_last_resize_width", None) width_changed = new_width is not None and prev_width is not None and new_width != prev_width if width_changed: + # Budget the replay against the full chrome: the bar/rules hidden while the + # reflow settles come back and would push the top replayed rows into scrollback. + self._status_bar_suppressed_after_resize = False try: - self._clear_prompt_toolkit_screen( + fit = self._clear_prompt_toolkit_screen( app, rebuild_scrollback=self._redraw_rebuilds_scrollback()) - _replay_output_history() + _replay_output_history(fit) except Exception: pass + self._status_bar_suppressed_after_resize = True if new_width is not None: self._last_resize_width = new_width if width_changed: diff --git a/tests/hermes_cli/test_cli_force_redraw.py b/tests/hermes_cli/test_cli_force_redraw.py index 4e9942cdb8..89d654e65f 100644 --- a/tests/hermes_cli/test_cli_force_redraw.py +++ b/tests/hermes_cli/test_cli_force_redraw.py @@ -24,6 +24,16 @@ def bare_cli(): return cli +def _fake_app(*, rows, columns, chrome): + """MagicMock app whose output reports a real size and whose layout is ``chrome`` rows tall.""" + from prompt_toolkit.data_structures import Size + + app = MagicMock() + app.renderer.output.get_size.return_value = Size(rows=rows, columns=columns) + app.layout.container.preferred_height.return_value.preferred = chrome + return app + + class TestForceFullRedraw: def test_no_app_is_safe(self, bare_cli): # _force_full_redraw must be a no-op when the TUI isn't running. @@ -34,17 +44,17 @@ class TestForceFullRedraw: def test_resize_recovery_clears_viewport_on_width_change(self, bare_cli, monkeypatch): - """A WIDTH change must wipe the visible viewport (CSI 2J) and replay. + """A WIDTH change must wipe the visible viewport and replay. On column shrink the terminal reflows the old full-width chrome into extra rows that prompt_toolkit's stale-cursor erase cannot reach, leaving a duplicated status bar (#19280/#5474 class). We route through - the same recovery as Ctrl+L: erase_screen (2J) + replay transcript. + the same recovery as Ctrl+L: erase the viewport + replay transcript. It must be banner-safe — CSI 3J (write_raw) must NOT fire. """ - app = MagicMock() + app = _fake_app(rows=30, columns=90, chrome=5) events = [] - app.renderer.output.erase_screen.side_effect = lambda: events.append("erase") + app.renderer.output.erase_end_of_line.side_effect = lambda: events.append("erase") app.renderer.output.write_raw.side_effect = lambda *_: events.append("scrollback_wipe") original_on_resize = lambda: events.append("original_resize") @@ -52,7 +62,7 @@ class TestForceFullRedraw: bare_cli._last_resize_width = 200 monkeypatch.setattr(bare_cli, "_get_tui_terminal_width", lambda: 90) monkeypatch.setattr(bare_cli, "_schedule_status_bar_unsuppress", lambda *_: None) - monkeypatch.setattr(cli_mod, "_replay_output_history", lambda: events.append("replay")) + monkeypatch.setattr(cli_mod, "_replay_output_history", lambda *_: events.append("replay")) monkeypatch.setattr( cli_mod, "CLI_CONFIG", @@ -71,9 +81,14 @@ class TestForceFullRedraw: assert bare_cli._last_resize_width == 90 assert bare_cli._status_bar_suppressed_after_resize is True - def test_force_redraw_uses_full_screen_clear_without_scrollback_clear(self, bare_cli, monkeypatch): - app = MagicMock() + def test_force_redraw_refills_only_the_viewport_without_a_screen_clear(self, bare_cli, monkeypatch): + """#95375: scrollback already holds the older transcript, so a redraw must + neither emit CSI 2J (scroll-on-clear terminals — tmux, VTE — copy the whole + screen into scrollback first) nor replay more than fits above the chrome.""" + app = _fake_app(rows=30, columns=100, chrome=6) bare_cli._app = app + fits = [] + monkeypatch.setattr(cli_mod, "_replay_output_history", lambda fit=None: fits.append(fit)) monkeypatch.setattr( cli_mod, "CLI_CONFIG", @@ -82,13 +97,17 @@ class TestForceFullRedraw: bare_cli._force_full_redraw() - app.renderer.output.erase_screen.assert_called_once() - app.renderer.output.cursor_goto.assert_called_once_with(0, 0) - app.renderer.output.write_raw.assert_not_called() + out = app.renderer.output + out.erase_screen.assert_not_called() + out.write_raw.assert_not_called() + assert out.erase_end_of_line.call_count == 30 + assert fits == [(24, 100)] def test_force_redraw_can_clear_scrollback_when_configured(self, bare_cli, monkeypatch): app = MagicMock() bare_cli._app = app + fits = [] + monkeypatch.setattr(cli_mod, "_replay_output_history", lambda fit=None: fits.append(fit)) monkeypatch.setattr( cli_mod, "CLI_CONFIG", @@ -99,6 +118,8 @@ class TestForceFullRedraw: app.renderer.output.erase_screen.assert_called_once() app.renderer.output.write_raw.assert_called_once_with("\x1b[3J") + # Scrollback was wiped, so the whole history is replayed to rebuild it. + assert fits == [None] def test_resize_recovery_can_clear_scrollback_when_configured(self, bare_cli, monkeypatch): app = MagicMock() @@ -111,7 +132,7 @@ class TestForceFullRedraw: bare_cli._last_resize_width = 200 monkeypatch.setattr(bare_cli, "_get_tui_terminal_width", lambda: 90) monkeypatch.setattr(bare_cli, "_schedule_status_bar_unsuppress", lambda *_: None) - monkeypatch.setattr(cli_mod, "_replay_output_history", lambda: events.append("replay")) + monkeypatch.setattr(cli_mod, "_replay_output_history", lambda *_: events.append("replay")) monkeypatch.setattr( cli_mod, "CLI_CONFIG", @@ -139,7 +160,7 @@ class TestForceFullRedraw: bare_cli._last_resize_width = 120 monkeypatch.setattr(bare_cli, "_get_tui_terminal_width", lambda: 120) monkeypatch.setattr(bare_cli, "_schedule_status_bar_unsuppress", lambda *_: None) - monkeypatch.setattr(cli_mod, "_replay_output_history", lambda: events.append("replay")) + monkeypatch.setattr(cli_mod, "_replay_output_history", lambda *_: events.append("replay")) bare_cli._recover_after_resize(app, original_on_resize) @@ -299,7 +320,7 @@ class TestFirstSigwinchBaseline: monkeypatch.setattr(bare_cli, "_get_tui_terminal_width", lambda: 120) monkeypatch.setattr(bare_cli, "_schedule_status_bar_unsuppress", lambda *_: None) monkeypatch.setattr( - cli_mod, "_replay_output_history", lambda: events.append("replay") + cli_mod, "_replay_output_history", lambda *_: events.append("replay") ) bare_cli._recover_after_resize(app, original_on_resize) @@ -313,10 +334,10 @@ class TestFirstSigwinchBaseline: def test_real_width_change_after_baseline_still_replays( self, bare_cli, monkeypatch ): - """The #49120 recovery (2J + replay) must still fire on a real change.""" - app = MagicMock() + """The #49120 recovery (viewport erase + replay) must still fire on a real change.""" + app = _fake_app(rows=30, columns=90, chrome=5) events = [] - app.renderer.output.erase_screen.side_effect = lambda: events.append("erase") + app.renderer.output.erase_end_of_line.side_effect = lambda: events.append("erase") original_on_resize = lambda: events.append("original_resize") bare_cli._status_bar_suppressed_after_resize = False @@ -324,7 +345,7 @@ class TestFirstSigwinchBaseline: monkeypatch.setattr(bare_cli, "_get_tui_terminal_width", lambda: 90) monkeypatch.setattr(bare_cli, "_schedule_status_bar_unsuppress", lambda *_: None) monkeypatch.setattr( - cli_mod, "_replay_output_history", lambda: events.append("replay") + cli_mod, "_replay_output_history", lambda *_: events.append("replay") ) bare_cli._recover_after_resize(app, original_on_resize) @@ -391,83 +412,22 @@ class TestFirstSigwinchBaseline: assert getattr(bare_cli, "_last_resize_width", None) is None -class TestReplayBoundedToVisibleHeight: - """Bug #95375: persistent_output replay must not append a duplicate block - to scrollback on resize/redraw. +class TestReplayFitsViewport: + """#95375: a redraw that keeps scrollback replays only what the viewport holds.""" - The pre-replay clear only wipes the visible viewport (CSI 2J), not the - scrollback, so re-printing the full ``_OUTPUT_HISTORY`` buffer on every - resize/redraw stacked a fresh copy of the history below the old content. - The replay must be bounded to the terminal's visible row count. - """ - - def test_replay_emits_at_most_visible_rows(self, monkeypatch): - """With 200 history lines and a 10-row terminal, replay emits ~10 lines, - not all 200.""" + def test_replay_keeps_newest_lines_that_fit_wrapped(self, monkeypatch): cli_mod._configure_output_history(True, 200) for i in range(200): cli_mod._record_output_history(f"history line {i}") - + cli_mod._record_output_history("x" * 150) # soft-wraps to 2 rows at 100 cols printed = [] monkeypatch.setattr(cli_mod, "_pt_print", lambda x: printed.append(x)) monkeypatch.setattr(cli_mod, "_PT_ANSI", lambda t: t) - monkeypatch.setattr( - cli_mod.shutil, - "get_terminal_size", - lambda *a, **k: __import__("os").terminal_size((80, 10)), - ) - cli_mod._replay_output_history() + cli_mod._replay_output_history((10, 100)) - assert len(printed) == 1, "replay must emit a single ANSI payload" - replayed = printed[0].split("\n") - assert len(replayed) <= 10, ( - f"replay emitted {len(replayed)} lines, expected at most 10 " - "(visible terminal height) — full 200-line buffer was re-printed" - ) - # The LAST visible lines are replayed, not the first. - assert replayed == [f"history line {i}" for i in range(190, 200)] - - def test_replay_keeps_full_history_buffer(self, monkeypatch): - """Bounding the replay must NOT truncate _OUTPUT_HISTORY itself — the - deque keeps the full history for other consumers.""" - cli_mod._configure_output_history(True, 200) - for i in range(200): - cli_mod._record_output_history(f"history line {i}") - - monkeypatch.setattr(cli_mod, "_pt_print", lambda x: None) - monkeypatch.setattr(cli_mod, "_PT_ANSI", lambda t: t) - monkeypatch.setattr( - cli_mod.shutil, - "get_terminal_size", - lambda *a, **k: __import__("os").terminal_size((80, 10)), - ) - - cli_mod._replay_output_history() - - assert len(cli_mod._OUTPUT_HISTORY) == 200, ( - "_OUTPUT_HISTORY must keep the full buffer after a bounded replay" - ) - - def test_replay_emits_all_when_history_fits(self, monkeypatch): - """When the history is shorter than the visible height, everything is - replayed (no regression for small transcripts).""" - cli_mod._configure_output_history(True, 200) - for i in range(5): - cli_mod._record_output_history(f"line {i}") - - printed = [] - monkeypatch.setattr(cli_mod, "_pt_print", lambda x: printed.append(x)) - monkeypatch.setattr(cli_mod, "_PT_ANSI", lambda t: t) - monkeypatch.setattr( - cli_mod.shutil, - "get_terminal_size", - lambda *a, **k: __import__("os").terminal_size((80, 10)), - ) - - cli_mod._replay_output_history() - - assert printed[0].split("\n") == [f"line {i}" for i in range(5)] + assert printed[0].split("\n") == [f"history line {i}" for i in range(192, 200)] + ["x" * 150] + assert len(cli_mod._OUTPUT_HISTORY) == 200 # the buffer itself is untouched class TestFocusRegainRedraw: diff --git a/tests/hermes_cli/test_cli_pet_pane.py b/tests/hermes_cli/test_cli_pet_pane.py index 9f42508b0d..d257205434 100644 --- a/tests/hermes_cli/test_cli_pet_pane.py +++ b/tests/hermes_cli/test_cli_pet_pane.py @@ -258,7 +258,7 @@ def test_force_full_redraw_requeues_kitty_frame(boba_like, monkeypatch): self.invalidated = True cli_obj._app = App() - monkeypatch.setattr("cli._replay_output_history", lambda: None) + monkeypatch.setattr("cli._replay_output_history", lambda *_: None) cli_obj._force_full_redraw()