From df5b1c35f2a4fe3005437cce22585b66cab3d170 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Wed, 17 Jun 2026 16:52:27 -0400 Subject: [PATCH 1/8] fix(tui): avoid unnecessary viewport scroll in fallback live renderer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DiffLive grew the live frame with a bare "\n" on every appended row, which scrolls the terminal viewport up even when the new row still fits the visible region — the "text jumps up" symptom during non-interactive streaming. Use cursor-down when the next row provably fits (frame origin + target row vs height) and fall back to newline scroll only on genuine overflow. Adds PYTHINKER_DIFF_LIVE_LOG tracing and scroll-geometry tests. --- .../ui/shell/visualize/_diff_live.py | 105 ++++++- tests/ui_and_conv/test_diff_live_scroll.py | 288 ++++++++++++++++++ 2 files changed, 388 insertions(+), 5 deletions(-) create mode 100644 tests/ui_and_conv/test_diff_live_scroll.py diff --git a/src/pythinker_code/ui/shell/visualize/_diff_live.py b/src/pythinker_code/ui/shell/visualize/_diff_live.py index 15c5fa21..2bf11992 100644 --- a/src/pythinker_code/ui/shell/visualize/_diff_live.py +++ b/src/pythinker_code/ui/shell/visualize/_diff_live.py @@ -6,7 +6,9 @@ from __future__ import annotations +import os import sys +import time from collections.abc import Callable from dataclasses import dataclass from typing import IO, TYPE_CHECKING, TextIO, cast @@ -45,6 +47,21 @@ def _should_rewrite_growing_last_line( return bool(old_lines and len(lines) > len(old_lines) and first_diff == len(old_lines) - 1) +def _diff_live_trace(event: str) -> None: + """Append a DiffLive timeline event when ``PYTHINKER_DIFF_LIVE_LOG`` is set. + + Diagnostic only. No-op when unset; never raises. + """ + path = os.environ.get("PYTHINKER_DIFF_LIVE_LOG") + if not path: + return + try: + with open(path, "a", encoding="utf-8") as fh: + fh.write(f"{time.monotonic():.3f}\t{event}\n") + except OSError: + pass + + class DiffLive(RenderHook): """Minimal live-region renderer that updates changed lines in place.""" @@ -73,6 +90,8 @@ def __init__( self._console_state_active = False self._cursor_below_frame = False self._frame_truncated = False + self._frame_origin_row: int | None = None + self._last_terminal_height: int = 0 def __enter__(self) -> DiffLive: if not self._started: @@ -111,6 +130,15 @@ def refresh(self) -> None: lines = self._render_lines(renderable) max_visible = self.console.size.height + if max_visible != self._last_terminal_height: + self._last_terminal_height = max_visible + # A resize can invalidate a previously probed origin (e.g. shrink). + if ( + self._frame_origin_row is not None + and max_visible > 0 + and self._frame_origin_row >= max_visible + ): + self._frame_origin_row = None self._frame_truncated = False if max_visible > 0: if len(lines) > max_visible: @@ -119,10 +147,20 @@ def refresh(self) -> None: if len(self._lines) > max_visible: self._lines = self._lines[-max_visible:] + growing = bool(self._lines and len(lines) > len(self._lines)) if not self._lines: self._write_initial(lines) + _diff_live_trace( + f"DIFF_LIVE\tinitial\tlines={len(lines)}\theight={max_visible}" + f"\torigin={self._frame_origin_row}" + ) else: self._write_diff(lines) + if growing: + _diff_live_trace( + f"DIFF_LIVE\tgrowth\tlines={len(lines)}\theight={max_visible}" + f"\torigin={self._frame_origin_row}" + ) self._lines = lines def stop(self) -> None: @@ -138,6 +176,8 @@ def stop(self) -> None: self._restore_console_state() self._nested = False self._frame_truncated = False + self._frame_origin_row = None + self._last_terminal_height = 0 def _print_current_renderable(self) -> None: renderable = self.get_renderable() @@ -212,13 +252,55 @@ def _render_lines(self, renderable: RenderableType) -> list[_RenderedLine]: for line in rendered_lines ] + def _ensure_frame_origin_row(self) -> None: + if self._frame_origin_row is not None: + return + from pythinker_code.utils.term import get_cursor_row + + row = get_cursor_row() + if row is None: + return + origin = row - 1 + height = self.console.size.height + # CPR can time out, lie, or report a row outside the viewport — fail closed. + if origin < 0 or height <= 0 or origin >= height: + return + self._frame_origin_row = origin + + def _bump_origin_after_scroll(self) -> None: + if self._frame_origin_row is None: + return + self._frame_origin_row -= 1 + # Lost geometry after repeated viewport scrolls — stop guessing. + if self._frame_origin_row < 0: + self._frame_origin_row = None + + def _row_fits_without_scroll(self, target_row: int) -> bool: + """True only when frame row *target_row* is provably on-screen.""" + if target_row < 0: + return False + origin = self._frame_origin_row + height = self.console.size.height + if origin is None or origin < 0 or height <= 0: + return False + return origin + target_row < height + + def _append_row_transition(self, payload: list[str], target_row: int) -> None: + if self._row_fits_without_scroll(target_row): + payload.append(self._cursor_down_newline()) + return + payload.append(self._scroll_newline()) + self._bump_origin_after_scroll() + def _write_initial(self, lines: list[_RenderedLine]) -> None: if not lines: return + self._ensure_frame_origin_row() payload_parts: list[str] = [] for index, line in enumerate(lines): if index: payload_parts.append(self._scroll_newline()) + self._bump_origin_after_scroll() payload_parts.append(line.text) payload_parts.append(str(Control.move_to_column(0))) payload = "".join(payload_parts) @@ -231,7 +313,7 @@ def _write_diff(self, lines: list[_RenderedLine]) -> None: if first_diff == len(old_lines) == len(lines): return if first_diff == len(old_lines) and len(lines) > len(old_lines): - self._write_appended_lines(lines[first_diff:]) + self._write_appended_lines(lines[first_diff:], base_row=len(old_lines)) return if _should_rewrite_growing_last_line(old_lines, lines, first_diff): self._rewrite_growing_last_line(old_lines[-1], lines[first_diff:]) @@ -282,23 +364,31 @@ def _append_diff_row_transition( ) -> None: next_row = row + 1 if row >= last_old_row or next_row > last_old_row: - payload.append(self._scroll_newline()) + self._append_row_transition(payload, next_row) return payload.append(self._move_to_line_start(1)) - def _write_appended_lines(self, lines: list[_RenderedLine]) -> None: + def _write_appended_lines( + self, + lines: list[_RenderedLine], + *, + base_row: int, + ) -> None: if not lines: return payload_parts: list[str] = [] + target_row = base_row if self._cursor_below_frame: payload_parts.append(str(Control.move_to_column(0))) payload_parts.append(lines[0].text) remaining_lines = lines[1:] + target_row += 1 else: remaining_lines = lines for line in remaining_lines: - payload_parts.append(self._scroll_newline()) + self._append_row_transition(payload_parts, target_row) payload_parts.append(line.text) + target_row += 1 payload_parts.append(str(Control.move_to_column(0))) payload = "".join(payload_parts) self._write(payload) @@ -315,9 +405,11 @@ def _rewrite_growing_last_line( payload = [self._move_to_line_start(row_delta), new_lines[0].text] if old_last_line.cell_length > new_lines[0].cell_length: payload.append(str(Control((ControlType.ERASE_IN_LINE, 0)))) + target_row = len(self._lines) for line in new_lines[1:]: - payload.append(self._scroll_newline()) + self._append_row_transition(payload, target_row) payload.append(line.text) + target_row += 1 payload.append(str(Control.move_to_column(0))) self._write("".join(payload)) self._cursor_below_frame = False @@ -342,6 +434,9 @@ def _clear_region(self) -> None: def _move_to_line_start(self, row_delta: int) -> str: return str(Control.move_to_column(0, y=row_delta)) + def _cursor_down_newline(self) -> str: + return self._move_to_line_start(1) + def _scroll_newline(self) -> str: return f"{Control.move_to_column(0)}\n" diff --git a/tests/ui_and_conv/test_diff_live_scroll.py b/tests/ui_and_conv/test_diff_live_scroll.py new file mode 100644 index 00000000..b6e649e2 --- /dev/null +++ b/tests/ui_and_conv/test_diff_live_scroll.py @@ -0,0 +1,288 @@ +"""Geometry-safe scroll vs cursor-down transitions in DiffLive.""" + +from __future__ import annotations + +import io +from typing import cast + +import pytest +from rich.console import Console, ConsoleDimensions +from rich.control import Control +from rich.text import Text + +from pythinker_code.ui.shell.visualize._diff_live import ( + DiffLive, + _diff_live_trace, + _RenderedLine, +) + + +def _make_console(*, height: int, width: int = 80) -> Console: + console = Console( + file=io.StringIO(), + width=width, + height=height, + force_terminal=True, + color_system=None, + legacy_windows=False, + ) + object.__setattr__(console, "size", ConsoleDimensions(width=width, height=height)) + return console + + +def _set_console_height(console: Console, *, height: int, width: int = 80) -> None: + object.__setattr__(console, "size", ConsoleDimensions(width=width, height=height)) + + +def _line(text: str) -> _RenderedLine: + return _RenderedLine(text=text, cell_length=len(text)) + + +def _make_live(*, height: int, origin: int | None = 0) -> tuple[DiffLive, Console]: + console = _make_console(height=height) + live = DiffLive(console=console, transient=True) + live._is_interactive = True + live._started = True + live._frame_origin_row = origin + live._last_terminal_height = height + return live, console + + +def _payload(console: Console) -> str: + return cast("io.StringIO", console.file).getvalue() + + +def _cud_bytes() -> str: + return str(Control.move_to_column(0, y=1)) + + +def _scroll_bytes() -> str: + return f"{Control.move_to_column(0)}\n" + + +@pytest.mark.parametrize( + ("origin", "height", "base_row", "target_row", "expect_cud", "expect_scroll"), + [ + (0, 40, 1, 5, True, False), + (35, 40, 1, 5, False, True), + (None, 40, 1, 5, False, True), + (0, 40, 1, 40, False, True), + ], +) +def test_append_row_transition_geometry( + origin: int | None, + height: int, + base_row: int, + target_row: int, + *, + expect_cud: bool, + expect_scroll: bool, +) -> None: + live, console = _make_live(height=height, origin=origin) + payload_parts: list[str] = [] + live._append_row_transition(payload_parts, target_row) + live._write("".join(payload_parts)) + + payload = _payload(console) + if expect_cud: + assert _cud_bytes() in payload + else: + assert _cud_bytes() not in payload + if expect_scroll: + assert _scroll_bytes() in payload + else: + assert _scroll_bytes() not in payload + + +def test_write_appended_lines_uses_cud_when_rows_fit() -> None: + live, console = _make_live(height=40, origin=0) + live._lines = [_line("seed")] + + live._write_appended_lines( + [_line("a"), _line("b"), _line("c"), _line("d")], + base_row=1, + ) + + payload = _payload(console) + assert _cud_bytes() in payload + assert _scroll_bytes() not in payload + + +def test_write_appended_lines_scrolls_when_near_bottom() -> None: + live, console = _make_live(height=40, origin=35) + live._lines = [_line("seed")] + + live._write_appended_lines([_line("overflow")], base_row=5) + + payload = _payload(console) + assert _scroll_bytes() in payload + assert _cud_bytes() not in payload + + +def test_row_fits_without_scroll_unknown_origin_fails_closed() -> None: + live, _console = _make_live(height=40, origin=None) + assert live._row_fits_without_scroll(5) is False + + +def test_row_fits_without_scroll_near_bottom() -> None: + live, _console = _make_live(height=40, origin=35) + assert live._row_fits_without_scroll(4) is True + assert live._row_fits_without_scroll(5) is False + + +def test_bump_origin_after_scroll_updates_fit() -> None: + live, console = _make_live(height=40, origin=35) + live._lines = [_line("seed")] + + # target 5 does not fit at origin 35 (35+5=40). + live._write_appended_lines([_line("a")], base_row=5) + first_payload = _payload(console) + assert _scroll_bytes() in first_payload + assert live._frame_origin_row == 34 + + cast("io.StringIO", console.file).seek(0) + cast("io.StringIO", console.file).truncate(0) + live._lines = [_line("seed"), _line("a")] + # After one scroll, origin 34 + target 5 = 39 < 40 → CUD allowed. + live._write_appended_lines([_line("b")], base_row=5) + second_payload = _payload(console) + assert _cud_bytes() in second_payload + assert _scroll_bytes() not in second_payload + + +def test_height_shrink_re_evaluates_fit() -> None: + live, console = _make_live(height=40, origin=0) + live._lines = [_line("seed")] + + live._write_appended_lines([_line("grow")], base_row=1) + assert _cud_bytes() in _payload(console) + + _set_console_height(console, height=30) + live._last_terminal_height = 30 + cast("io.StringIO", console.file).seek(0) + cast("io.StringIO", console.file).truncate(0) + live._lines = [_line("seed"), _line("grow")] + + live._write_appended_lines([_line("more")], base_row=2) + payload = _payload(console) + # origin 0 + target 2 = 2 < 30 → still fits with CUD. + assert _cud_bytes() in payload + assert _scroll_bytes() not in payload + + +def test_rewrite_growing_last_line_uses_geometry_gate() -> None: + live, console = _make_live(height=40, origin=35) + live._lines = [_line("last")] + + live._rewrite_growing_last_line( + _line("last"), + [_line("last"), _line("extra")], + ) + + payload = _payload(console) + # target row 1: 35+1=36 < 40 → CUD for the extra line. + assert _cud_bytes() in payload + assert _scroll_bytes() not in payload + + +def test_append_diff_row_transition_growth_branch() -> None: + live, console = _make_live(height=40, origin=0) + live._lines = [_line("a"), _line("b")] + + payload_parts: list[str] = [] + live._append_diff_row_transition(payload_parts, row=1, last_old_row=1) + live._write("".join(payload_parts)) + + payload = _payload(console) + assert _cud_bytes() in payload + assert _scroll_bytes() not in payload + + +def test_diff_live_trace_noop_when_env_unset(monkeypatch: pytest.MonkeyPatch, tmp_path) -> None: + monkeypatch.delenv("PYTHINKER_DIFF_LIVE_LOG", raising=False) + _diff_live_trace("DIFF_LIVE\tinitial\tlines=1\theight=40\torigin=0") + assert not any(tmp_path.iterdir()) + + +def test_diff_live_trace_appends_when_enabled( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + log = tmp_path / "diff-live.log" + monkeypatch.setenv("PYTHINKER_DIFF_LIVE_LOG", str(log)) + _diff_live_trace("DIFF_LIVE\tgrowth\tlines=5\theight=40\torigin=0") + _diff_live_trace("DIFF_LIVE\tinitial\tlines=1\theight=40\torigin=None") + text = log.read_text(encoding="utf-8") + assert "DIFF_LIVE\tgrowth" in text + assert "DIFF_LIVE\tinitial" in text + assert text.count("\n") == 2 + + +def test_row_fits_rejects_negative_target_row() -> None: + live, _console = _make_live(height=40, origin=0) + assert live._row_fits_without_scroll(-1) is False + + +def test_bump_origin_after_scroll_invalidates_when_exhausted() -> None: + live, console = _make_live(height=40, origin=0) + live._frame_origin_row = 0 + for _ in range(2): + live._bump_origin_after_scroll() + assert live._frame_origin_row is None + + payload_parts: list[str] = [] + live._append_row_transition(payload_parts, 1) + live._write("".join(payload_parts)) + assert _scroll_bytes() in _payload(console) + + +def test_ensure_frame_origin_rejects_out_of_range_probe( + monkeypatch: pytest.MonkeyPatch, +) -> None: + console = _make_console(height=40) + live = DiffLive(console=console, transient=True) + live._is_interactive = True + monkeypatch.setattr("pythinker_code.utils.term.get_cursor_row", lambda: 50) + live._ensure_frame_origin_row() + assert live._frame_origin_row is None + + +def test_refresh_clears_origin_when_height_shrinks_below_it() -> None: + console = _make_console(height=40) + live = DiffLive(console=console, transient=True) + live._is_interactive = True + live._started = True + live._frame_origin_row = 35 + live._last_terminal_height = 40 + live._lines = [_line("seed")] + live._renderable = Text("seed\nmore") + + _set_console_height(console, height=30) + live.refresh() + + assert live._frame_origin_row is None + + +def test_refresh_logs_growth_when_env_enabled( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + log = tmp_path / "diff-live.log" + monkeypatch.setenv("PYTHINKER_DIFF_LIVE_LOG", str(log)) + monkeypatch.setattr( + "pythinker_code.utils.term.get_cursor_row", + lambda: 1, + ) + + console = _make_console(height=40) + live = DiffLive(console=console, transient=True) + live._is_interactive = True + live._started = True + live._renderable = Text("one") + live.refresh() + live._renderable = Text("one\ntwo") + live.refresh() + + text = log.read_text(encoding="utf-8") + assert "DIFF_LIVE\tinitial" in text + assert "DIFF_LIVE\tgrowth" in text From b3b9ac840b92a93452d3ed4700dd1793d51b6194 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Wed, 17 Jun 2026 16:52:36 -0400 Subject: [PATCH 2/8] fix(tui): prevent interactive preamble ghosting during scrollback handoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked "Vibing… (Ns)" rows and duplicate tips fossilized into permanent scrollback during streaming. Root cause: scrollback handoffs tear down and redraw the prompt app around run_in_terminal on every tool transition / turn end, and the height-varying transient preamble (agent stream body + elapsed verb spinner + tips) re-rendered in that window is left behind when the teardown erase miscounts. Suppress all transient preamble (body, spinner, tips) for the duration of a scrollback handoff via _suppress_transient_preamble, and treat a terminal resize as a hard invalidation boundary that briefly hides multi-row tips while prompt_toolkit settles at the new geometry. Committed prose is emitted by the handoff itself; the live tail returns once the handoff completes. Tests sample the preamble renderers from inside the real handoff emit window to prove suppression is live, not just a getter contract. --- CHANGELOG.md | 17 ++ .../ui/shell/visualize/_interactive.py | 108 ++++++- .../ui/shell/visualize/_live_view.py | 4 +- .../test_visualize_running_prompt.py | 269 +++++++++++++++++- 4 files changed, 387 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f53e335..1557ffd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,23 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- **TUI: interactive resize/handoff ghosting.** Scrollback handoffs in prompt mode + now fully suppress the transient preamble (agent stream body, verb spinner, and + tips) while ``run_in_terminal`` emits permanent scrollback, so stacked + ``Vibing…`` rows and duplicate tips no longer fossilize during tool transitions. + Terminal resize triggers a hard preamble invalidation and briefly hides tips + while prompt_toolkit settles at the new geometry. Handoffs defer during resize + recovery; failed emits leave scrollback queued for retry instead of dropping it. +- **DiffLive streaming scroll geometry.** Non-interactive live streaming now uses + cursor-down only when the next row provably fits the visible terminal region + (frame origin + target row vs height); otherwise it falls back to newline scroll, + preventing mid-viewport overwrite when the live frame starts below the top of the + screen. Set `PYTHINKER_DIFF_LIVE_LOG` to trace DiffLive refresh/growth ticks. +- **TUI: clearer collapsed ReadFile cards.** Collapsed reads now show a line-count + summary with the file name (e.g. `Read 140 lines from console.py`) plus a short, + width-capped preview of the leading lines, instead of the generic `Read 1 file` + that made it look like no content was returned. Empty/unknown reads stay truthful + (`Read 0 lines` / `Read file content`), and expanded mode still shows the full file. - **Cleaner terminal report rendering.** Structured ` ```report ` outputs now suppress duplicated trailing summaries, keep only artifact footers after the report, compact long finding locations, and switch large reports to a borderless dashboard layout for faster terminal scanning. - **Unknown subagent-type recovery hints.** Invalid types still fail loudly, but `Agent`/`RunAgents` errors now include best-effort suggestions for common diff --git a/src/pythinker_code/ui/shell/visualize/_interactive.py b/src/pythinker_code/ui/shell/visualize/_interactive.py index 3f8a0645..0f6ae4ac 100644 --- a/src/pythinker_code/ui/shell/visualize/_interactive.py +++ b/src/pythinker_code/ui/shell/visualize/_interactive.py @@ -80,6 +80,9 @@ _STATUS_REFRESH_INTERVAL_S = 0.22 _STATUS_REFRESH_REDUCED_INTERVAL_S = 1.0 +# Redraw ticks after a terminal resize before tips return — old wrapped rows may +# not be fully erased until prompt_toolkit settles at the new geometry. +_RESIZE_RECOVERY_FRAMES = 3 def _handoff_trace(event: str) -> None: @@ -156,9 +159,59 @@ def __init__( self._status_refresh_task: asyncio.Task[None] | None = None self._pending_scrollback: list[tuple[RenderableType, bool]] = [] self._scrollback_handoff_depth: int = 0 + self._last_terminal_size: tuple[int, int] | None = None + self._resize_recovery_remaining: int = 0 # -- Helpers ------------------------------------------------------------- + @property + def _suppress_transient_preamble(self) -> bool: + """True while scrollback handoff must not paint height-varying preamble.""" + return getattr(self, "_scrollback_handoff_depth", 0) > 0 + + @property + def _hide_working_tips(self) -> bool: + """True while resize recovery is settling — tips are multi-row and ghost easily.""" + return getattr(self, "_resize_recovery_remaining", 0) > 0 + + def _current_terminal_size(self) -> tuple[int, int] | None: + from prompt_toolkit.application import get_app_or_none + + app = get_app_or_none() + if app is None: + return None + size = app.output.get_size() + return (size.columns, size.rows) + + def _tick_resize_recovery(self) -> None: + """Detect terminal geometry changes and force a hard preamble invalidation.""" + size = self._current_terminal_size() + if size is None: + return + columns, rows = size + if columns < 1 or rows < 1: + _handoff_trace(f"RESIZE_IGNORE\t{columns}x{rows}") + return + if self._last_terminal_size != size: + self._last_terminal_size = size + self._resize_recovery_remaining = _RESIZE_RECOVERY_FRAMES + self._force_refresh = True + _handoff_trace(f"RESIZE\t{columns}x{rows}") + return + if self._resize_recovery_remaining > 0: + self._resize_recovery_remaining -= 1 + + def _defer_scrollback_handoff(self) -> bool: + """Backpressure: defer permanent scrollback while preamble geometry is unstable.""" + return self._resize_recovery_remaining > 0 + + def _safe_prompt_invalidate(self) -> None: + """Invalidate the prompt without letting teardown races take down the UI loop.""" + try: + self._prompt_session.invalidate() + except Exception as exc: # noqa: BLE001 — invalidate must never abort handoff cleanup + _handoff_trace(f"INVALIDATE_FAIL\t{type(exc).__name__}:{exc}") + def _prompt_is_finalizing(self) -> bool: """True while scrollback is queued or being emitted above the prompt.""" if ( @@ -182,15 +235,18 @@ def _finalizing_indicator(self) -> RenderableType: async def _run_scrollback_handoff(self, emit: Callable[[], None], *, reason: str = "?") -> None: _handoff_trace(f"HANDOFF\t{reason}") self._scrollback_handoff_depth += 1 - self._prompt_session.invalidate() + self._safe_prompt_invalidate() try: if console.is_terminal: await run_in_terminal(emit) else: emit() + except Exception as exc: + _handoff_trace(f"HANDOFF_FAIL\t{reason}\t{type(exc).__name__}:{exc}") + raise finally: self._scrollback_handoff_depth -= 1 - self._prompt_session.invalidate() + self._safe_prompt_invalidate() @property def _btw_active(self) -> bool: @@ -274,6 +330,7 @@ async def _status_refresh_loop(self) -> None: """ try: while True: + self._tick_resize_recovery() # Drain buffered paced text smoothly, even past TurnEnd, so the # tail flows out instead of popping when the block finally # commits. advance_stream_reveal() is a no-op unless a paced block @@ -338,20 +395,33 @@ async def _flush_pending_scrollback(self) -> None: is not fossilized into permanent transcript output. In piped/non-terminal mode run_in_terminal does not write to the captured stdout, so fall back to direct console.print() which matches the pre-preamble base-class behavior. + + Scrollback is removed from the queue only after a successful handoff emit. + Failed emits leave the queue intact for a later retry; handoffs are deferred + while terminal geometry is settling after a resize. """ if not self._pending_scrollback: return - to_print = self._pending_scrollback[:] - self._pending_scrollback.clear() + if self._defer_scrollback_handoff(): + _handoff_trace(f"HANDOFF_DEFER\tpending_scrollback({len(self._pending_scrollback)})") + return + batch = self._pending_scrollback[:] def emit() -> None: - for renderable, blank_row in to_print: + for renderable, blank_row in batch: console.print(renderable) if blank_row: console.print() - await self._run_scrollback_handoff(emit, reason=f"pending_scrollback({len(to_print)})") - self._prompt_session.invalidate() + try: + await self._run_scrollback_handoff( + emit, reason=f"pending_scrollback({len(batch)})" + ) + except Exception: + return + + del self._pending_scrollback[: len(batch)] + self._safe_prompt_invalidate() def _emit_final_scrollback(self, renderable: RenderableType) -> None: self._pending_scrollback.append((renderable, True)) @@ -707,6 +777,15 @@ def render_agent_status(self, columns: int) -> ANSI: """ if self._turn_ended and not self._prompt_is_finalizing(): return ANSI("") + # During a scrollback handoff the prompt app is torn down and redrawn + # around run_in_terminal (every tool transition / turn end). Re-rendering + # the multi-row live stream in that window is what gets left behind as + # fossilized scrollback when the teardown erase height drifts. The + # committed prose is emitted by the handoff itself, and the remaining + # tail is re-rendered once the handoff completes — so suppress the + # transient body for the duration of the handoff. + if self._suppress_transient_preamble: + return ANSI("") from prompt_toolkit.application import get_app_or_none from pythinker_code.ui.shell.prompt import _prompt_preamble_max_rows @@ -744,6 +823,13 @@ def render_pinned_status_tail(self, columns: int) -> ANSI: if not turn_active and not finalizing: return ANSI("") + # During scrollback handoff the prompt app is torn down around + # run_in_terminal. Any pinned spinner/tip row rendered in that window can + # be fossilized into permanent scrollback — suppress all transient tail + # content for the handoff duration. + if self._suppress_transient_preamble: + return ANSI("") + if finalizing and not turn_active: body = render_to_ansi(self._finalizing_indicator(), columns=columns).rstrip("\n") return ANSI(body if body else "") @@ -752,9 +838,15 @@ def render_pinned_status_tail(self, columns: int) -> ANSI: if content_block is not None and not content_block.is_think: body = render_to_ansi(content_block._compose_spinner(), columns=columns).rstrip("\n") else: - body = render_to_ansi(self._working_indicator(), columns=columns).rstrip("\n") + body = render_to_ansi( + self._working_indicator(hide_tips=self._hide_working_tips), + columns=columns, + ).rstrip("\n") return ANSI(body if body else "") + def _working_indicator(self, *, hide_tips: bool = False) -> RenderableType: + return super()._working_indicator(hide_tips=hide_tips) + def render_running_prompt_body(self, columns: int) -> ANSI: """Render the interactive part — transient command output + queued messages.""" parts: list[str] = [] diff --git a/src/pythinker_code/ui/shell/visualize/_live_view.py b/src/pythinker_code/ui/shell/visualize/_live_view.py index f93af638..9a11f817 100644 --- a/src/pythinker_code/ui/shell/visualize/_live_view.py +++ b/src/pythinker_code/ui/shell/visualize/_live_view.py @@ -896,7 +896,7 @@ def _print_turn_recap(self) -> None: console.print(block) console.print() - def _working_indicator(self) -> RenderableType: + def _working_indicator(self, *, hide_tips: bool = False) -> RenderableType: now = time.monotonic() elapsed = 0.0 if self._turn_start_time is None else now - self._turn_start_time width = current_console_width() @@ -930,7 +930,7 @@ def _working_indicator(self) -> RenderableType: width=width, ) # During longer waits, surface a rotating CLI-feature tip under the verb. - if elapsed < _WORKING_TIP_MIN_ELAPSED_S: + if hide_tips or elapsed < _WORKING_TIP_MIN_ELAPSED_S: return line tip_content = Text("Tip: ", style=tui_rich_style("dim")) tip_content.append(current_tip(now), style=tui_rich_style("dim")) diff --git a/tests/ui_and_conv/test_visualize_running_prompt.py b/tests/ui_and_conv/test_visualize_running_prompt.py index 2d2f8dc1..650e7772 100644 --- a/tests/ui_and_conv/test_visualize_running_prompt.py +++ b/tests/ui_and_conv/test_visualize_running_prompt.py @@ -358,7 +358,274 @@ def test_render_pinned_status_tail_finalizing_during_scrollback_handoff() -> Non view._scrollback_handoff_depth = 1 view._current_content_block = None - assert "Finalizing" in view.render_pinned_status_tail(80).value + assert view.render_pinned_status_tail(80).value == "" + + +def test_render_pinned_status_tail_no_elapsed_spinner_during_midturn_handoff() -> None: + """Mid-turn tool-transition handoffs (turn still active) must not render the + elapsed-time verb spinner or tips — they stack as fossilized scrollback when + the teardown erase drifts. + """ + import time as _time + + from pythinker_code.ui.shell.visualize._blocks import _ContentBlock + + view = object.__new__(_PromptLiveView) + view._turn_ended = False + view._active_turn_depth = 1 # turn IS active (mid-turn transition) + view._turn_start_time = _time.monotonic() + view._current_question_panel = None + view._current_approval_request_panel = None + view._pending_scrollback = [] + view._scrollback_handoff_depth = 1 # ...inside a scrollback handoff + block = _ContentBlock(is_think=False) + block.append("Streaming body.\n\ntail") + view._current_content_block = block + + out = view.render_pinned_status_tail(80).value + assert out == "" + assert "Composing" not in out + + +@pytest.mark.asyncio +async def test_transient_preamble_suppressed_inside_handoff_window(monkeypatch) -> None: + """The real proof: sample the preamble renderers from inside the emit window + of an actual ``_run_scrollback_handoff``. With the turn active and a content + block present, the agent-status body and the elapsed verb spinner must both + be suppressed while the handoff is in flight, so nothing height-varying is + rendered during the prompt-app teardown. + """ + import time as _time + + from pythinker_code.ui.shell.visualize._blocks import _ContentBlock + + class _PromptSession: + def invalidate(self) -> None: + pass + + view = _PromptLiveView( + StatusUpdate(), + prompt_session=cast(Any, _PromptSession()), + steer=lambda _content: None, + ) + view._turn_ended = False + view._active_turn_depth = 1 + view._turn_start_time = _time.monotonic() + block = _ContentBlock(is_think=False) + block.append("Live stream body.\n\ntail") + view._current_content_block = block + + # Sanity: outside any handoff, the body and a live spinner are rendered. + assert "Live stream body" in view.render_agent_status(80).value + assert "Composing" in view.render_pinned_status_tail(80).value + + samples: dict[str, str] = {} + + def _emit() -> None: + # In tests console.is_terminal is False, so emit() runs with the handoff + # depth already incremented — exactly the teardown window. + samples["body"] = view.render_agent_status(80).value + samples["tail"] = view.render_pinned_status_tail(80).value + + await view._run_scrollback_handoff(_emit, reason="test") + + assert samples["body"] == "" # multi-row stream suppressed during handoff + assert samples["tail"] == "" # no spinner/tips during handoff emit window + + # After the handoff completes the transient preamble comes back. + assert "Live stream body" in view.render_agent_status(80).value + assert "Composing" in view.render_pinned_status_tail(80).value + + +@pytest.mark.asyncio +async def test_multiple_handoffs_leave_no_transient_tail_snapshots(monkeypatch) -> None: + """Each handoff emit window must see an empty pinned tail — no stacked verbs.""" + import time as _time + + from pythinker_code.ui.shell.visualize._blocks import _ContentBlock + + class _PromptSession: + def invalidate(self) -> None: + pass + + view = _PromptLiveView( + StatusUpdate(), + prompt_session=cast(Any, _PromptSession()), + steer=lambda _content: None, + ) + view._turn_ended = False + view._active_turn_depth = 1 + view._turn_start_time = _time.monotonic() - 30.0 + view._current_content_block = _ContentBlock(is_think=False) + view._current_content_block.append("body\n\ntail") + + tails: list[str] = [] + + async def _emit(_label: str) -> None: + def _inner() -> None: + tails.append(view.render_pinned_status_tail(80).value) + + await view._run_scrollback_handoff(_inner, reason=_label) + + await _emit("first") + await _emit("second") + await _emit("third") + + assert tails == ["", "", ""] + + +def test_handoff_suppresses_working_tips(monkeypatch) -> None: + """Long-running turns show tips normally, but not during scrollback handoff.""" + import time as _time + + from pythinker_code.ui.shell.visualize._live_view import _WORKING_TIP_MIN_ELAPSED_S + + view = object.__new__(_PromptLiveView) + view._turn_ended = False + view._active_turn_depth = 1 + view._turn_start_time = _time.monotonic() - _WORKING_TIP_MIN_ELAPSED_S - 5.0 + view._current_question_panel = None + view._current_approval_request_panel = None + view._pending_scrollback = [] + view._scrollback_handoff_depth = 0 + view._current_content_block = None + view._pinned_todos_visible = True + view._latest_todos = () + view._resize_recovery_remaining = 0 + + monkeypatch.setattr(_live_view_mod, "current_tip", lambda _now: "do the thing") + + normal = view.render_pinned_status_tail(80).value + assert "Tip:" in normal + + view._scrollback_handoff_depth = 1 + during_handoff = view.render_pinned_status_tail(80).value + assert during_handoff == "" + + +def test_resize_triggers_recovery_and_hides_tips(monkeypatch) -> None: + import time as _time + + from pythinker_code.ui.shell.visualize._live_view import _WORKING_TIP_MIN_ELAPSED_S + + view = object.__new__(_PromptLiveView) + view._turn_ended = False + view._active_turn_depth = 1 + view._turn_start_time = _time.monotonic() - _WORKING_TIP_MIN_ELAPSED_S - 5.0 + view._current_question_panel = None + view._current_approval_request_panel = None + view._pending_scrollback = [] + view._scrollback_handoff_depth = 0 + view._current_content_block = None + view._pinned_todos_visible = True + view._latest_todos = () + view._last_terminal_size = (80, 24) + view._resize_recovery_remaining = 0 + view._force_refresh = False + + monkeypatch.setattr(_live_view_mod, "current_tip", lambda _now: "resize tip") + + current_size = [80, 24] + + def _size() -> tuple[int, int]: + return (current_size[0], current_size[1]) + + monkeypatch.setattr(view, "_current_terminal_size", _size) + + current_size[:] = [100, 30] + view._tick_resize_recovery() + assert view._force_refresh is True + assert view._resize_recovery_remaining == _interactive_mod._RESIZE_RECOVERY_FRAMES + assert "Tip:" not in view.render_pinned_status_tail(80).value + + view._force_refresh = False + for expected in ( + _interactive_mod._RESIZE_RECOVERY_FRAMES - 1, + _interactive_mod._RESIZE_RECOVERY_FRAMES - 2, + 0, + ): + view._tick_resize_recovery() + assert view._resize_recovery_remaining == expected + + assert "Tip:" in view.render_pinned_status_tail(80).value + + +@pytest.mark.asyncio +async def test_flush_pending_scrollback_deferred_during_resize_recovery(monkeypatch) -> None: + printed: list[object] = [] + + class _PromptSession: + def invalidate(self) -> None: + pass + + async def _run_in_terminal(func, *args, **kwargs): # noqa: ANN001, ANN002, ANN003 + func() + + monkeypatch.setattr(_interactive_mod, "run_in_terminal", _run_in_terminal) + monkeypatch.setattr( + _live_view_mod.console, + "print", + lambda *args, **kwargs: printed.extend(args) if args else None, + ) + + view = _PromptLiveView( + StatusUpdate(), + prompt_session=cast(Any, _PromptSession()), + steer=lambda _content: None, + ) + view._pending_scrollback.append((Text("queued block"), True)) + view._resize_recovery_remaining = 2 + + await view._flush_pending_scrollback() + + assert printed == [] + assert len(view._pending_scrollback) == 1 + + +@pytest.mark.asyncio +async def test_flush_pending_scrollback_retains_queue_on_handoff_failure(monkeypatch) -> None: + printed: list[object] = [] + + class _PromptSession: + def invalidate(self) -> None: + pass + + async def _run_in_terminal(func, *args, **kwargs): # noqa: ANN001, ANN002, ANN003 + raise RuntimeError("terminal suspended") + + monkeypatch.setattr(_interactive_mod, "run_in_terminal", _run_in_terminal) + monkeypatch.setattr(_live_view_mod.console, "_force_terminal", True) + monkeypatch.setattr( + _live_view_mod.console, + "print", + lambda *args, **kwargs: printed.extend(args) if args else None, + ) + + view = _PromptLiveView( + StatusUpdate(), + prompt_session=cast(Any, _PromptSession()), + steer=lambda _content: None, + ) + view._pending_scrollback.append((Text("must survive"), True)) + + await view._flush_pending_scrollback() + + assert printed == [] + assert len(view._pending_scrollback) == 1 + assert view._pending_scrollback[0][0].plain == "must survive" + + +def test_tick_resize_recovery_ignores_invalid_geometry() -> None: + view = object.__new__(_PromptLiveView) + view._last_terminal_size = (80, 24) + view._resize_recovery_remaining = 0 + view._force_refresh = False + + view._current_terminal_size = lambda: (0, 24) # type: ignore[method-assign] + view._tick_resize_recovery() + assert view._last_terminal_size == (80, 24) + assert view._resize_recovery_remaining == 0 + assert view._force_refresh is False def test_render_pinned_status_tail_finalizing_when_committed_blocks_pending() -> None: From 06f067bbcd7daf639663bc3fe981ca34c48cc151 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Wed, 17 Jun 2026 16:52:43 -0400 Subject: [PATCH 3/8] fix(tui): show line count and preview in collapsed ReadFile cards Collapsed reads said "Read 1 file (ctrl+o to expand)", hiding the line count and content so a successful read looked like it returned nothing. Render a line-count-aware summary with the file basename (e.g. "Read 140 lines from console.py") plus a short, cell-width-capped preview of the leading lines. Line count prefers the tool message, falls back to counting the body, and stays truthful when unknowable ("Read file content") or empty ("Read 0 lines"). Preview is ANSI-sanitized, capped to a few visual lines, width-capped per line, and skipped on narrow terminals. Expanded mode and the LLM-facing tool result are unchanged. --- .../ui/shell/tool_renderers/read.py | 104 ++++++++++++++++-- .../test_tui_card_tool_renderers.py | 93 +++++++++++++++- .../test_tui_transcript_enhancements.py | 2 +- 3 files changed, 185 insertions(+), 14 deletions(-) diff --git a/src/pythinker_code/ui/shell/tool_renderers/read.py b/src/pythinker_code/ui/shell/tool_renderers/read.py index 0dc07b22..29e72a80 100644 --- a/src/pythinker_code/ui/shell/tool_renderers/read.py +++ b/src/pythinker_code/ui/shell/tool_renderers/read.py @@ -7,11 +7,15 @@ from __future__ import annotations +import re +from pathlib import PurePosixPath, PureWindowsPath from typing import Any from rich.console import Group, RenderableType from rich.text import Text +from pythinker_code.ui.shell.components import sanitize_ansi +from pythinker_code.ui.shell.components.render_utils import truncate_to_width from pythinker_code.ui.shell.tool_renderers import ( ToolRenderContext, ToolRenderDefinition, @@ -27,10 +31,16 @@ pending_tool_call_header, running_spinner, shorten_path, + tab_to_spaces, tool_call_header, ) +from pythinker_code.ui.theme import tui_rich_style _TOOL_NAME = "ReadFile" +# Compact collapsed preview: a few leading lines so humans/models can confirm +# content was returned without dumping the file. Expanded mode shows it all. +_PREVIEW_MAX_LINES = 4 +_LINES_READ_RE = re.compile(r"(\d+)\s+lines?\s+read") def _format_line_range(args: dict[str, Any]) -> Text | None: @@ -98,6 +108,77 @@ def _friendly_error(text: str) -> str: return "Error reading file" +def _basename(path: Any) -> str | None: + """Display basename for a read path, or ``None`` when unavailable. + + The call row already shows the (shortened) path, so the result summary only + needs the leaf name — and never a fuller path that would leak more than the + call row already does. Handle both POSIX and Windows separators since the + path is model-supplied text, not a resolved local path. + """ + raw = as_str(path) + if raw is None or not raw.strip(): + return None + name = PureWindowsPath(PurePosixPath(raw).name).name + return name or None + + +def _line_count(message: str | None, output_text: str) -> int | None: + """Lines read in this call: prefer the tool message, else count the body. + + Returns ``None`` only when the count is genuinely unknowable (no message and + no body) — callers must then avoid asserting a count rather than lie with 0. + """ + if message: + match = _LINES_READ_RE.search(message) + if match: + return int(match.group(1)) + if "no lines read" in message.lower(): + return 0 + if output_text: + cleaned = output_text.rstrip("\n") + return cleaned.count("\n") + 1 if cleaned else 0 + return None + + +def _summary_text(count: int | None, basename: str | None) -> str: + if count is None: + head = "Read file content" + else: + head = f"Read {count} {'line' if count == 1 else 'lines'}" + if basename: + head += f" from {basename}" + return head + + +def _preview(output_text: str, width: int) -> Text | None: + """A few leading body lines, ANSI-stripped and width-capped per line. + + Caps both the number of visual lines (``_PREVIEW_MAX_LINES``) and each + line's width so a file with very long lines can never produce giant + collapsed output. Returns ``None`` when there is nothing friendly to show. + """ + cleaned = sanitize_ansi(output_text or "").rstrip("\n") + if not cleaned: + return None + # Width ceiling keyed off terminal cell width, leaving room for the card + # gutter so each preview line stays on a single visual row. On a terminal + # too narrow to show anything useful, skip the preview entirely. + limit = min(max(width - 6, 0), 200) + if limit < 12: + return None + out = Text(style=tui_rich_style("tool_output")) + for index, line in enumerate(cleaned.split("\n")[:_PREVIEW_MAX_LINES]): + if index: + out.append("\n") + # Cell-width aware: a wide-glyph / CJK / emoji line is truncated by the + # space it actually occupies, not its character count. + out.append(truncate_to_width(tab_to_spaces(line), limit)) + out.no_wrap = True + out.overflow = "ellipsis" + return out if out.plain else None + + def _render_result(ctx: ToolRenderContext, result: ToolResultPayload) -> RenderableType | None: ctx.state["__suppress_generic_expand_hint__"] = True if result.is_error: @@ -109,13 +190,24 @@ def _render_result(ctx: ToolRenderContext, result: ToolResultPayload) -> Rendera if isinstance(message, str) and message.startswith("Directory listing for `"): return fg("tool_output", "Listed 1 directory") + # An empty string is a valid (empty-file) body — only fall back to the + # flattened text when ``output`` is absent/non-string, so an empty read + # never inherits unrelated metadata from ``result.text``. output = result.details.get("output") - output_text = output if isinstance(output, str) and output else result.text + output_text = output if isinstance(output, str) else result.text + + count = _line_count(message if isinstance(message, str) else None, output_text) + basename = _basename(ctx.args.get("path")) + summary = _summary_text(count, basename) + if not output_text: - return fg("tool_output", "Read 1 file") + # Nothing to preview or expand (empty file / no body): truthful summary only. + return fg("tool_output", summary) if not ctx.expanded: - return fg("tool_output", "Read 1 file (ctrl+o to expand)") + collapsed = fg("tool_output", f"{summary} (ctrl+o to expand)") + preview = _preview(output_text, ctx.width) + return Group(collapsed, preview) if preview is not None else collapsed start_line = 1 offset = ctx.args.get("line_offset") @@ -128,11 +220,7 @@ def _render_result(ctx: ToolRenderContext, result: ToolResultPayload) -> Rendera start_line=start_line, style_token="tool_output", ) - return ( - Group(fg("tool_output", "Read 1 file"), body) - if body.plain - else fg("tool_output", "Read 1 file") - ) + return Group(fg("tool_output", summary), body) if body.plain else fg("tool_output", summary) READ_RENDERER = ToolRenderDefinition( diff --git a/tests/ui_and_conv/test_tui_card_tool_renderers.py b/tests/ui_and_conv/test_tui_card_tool_renderers.py index f3523d87..b5f49485 100644 --- a/tests/ui_and_conv/test_tui_card_tool_renderers.py +++ b/tests/ui_and_conv/test_tui_card_tool_renderers.py @@ -134,7 +134,9 @@ def test_read_renders_path_and_range(): assert "⏺ Read(" in rendered assert "src/foo.py" in rendered assert ":10-39" in rendered - assert "Read 1 file (ctrl+o to expand)" in rendered + # Line-count-aware summary with basename, not the generic "Read 1 file". + assert "Read 2 lines from foo.py (ctrl+o to expand)" in rendered + assert "Read 1 file" not in rendered def test_read_renders_negative_offset_as_tail(): @@ -158,12 +160,93 @@ def test_read_renders_negative_offset_with_limit(): assert ":tail 100 · limit 20" in rendered -def test_read_result_matches_reference_summary_only(): +def test_read_collapsed_shows_count_and_capped_preview(): body = "\n".join(f"line {i}" for i in range(20)) rendered = _render("ReadFile", {"path": "/repo/x.py"}, output=body) - assert "Read 1 file (ctrl+o to expand)" in rendered - assert "line 0" not in rendered - assert "more lines" not in rendered + # Line-count-aware summary replaces the generic "Read 1 file". + assert "Read 20 lines from x.py (ctrl+o to expand)" in rendered + assert "Read 1 file" not in rendered + # A short preview of leading lines is shown, but the preview is capped: + # the first line appears, a line past the cap does not. + assert "line 0" in rendered + assert "line 19" not in rendered + + +def test_read_collapsed_caps_preview_line_width_no_giant_output(): + # A single very long line must not blow up the collapsed card height. + long_line = "x" * 500 + rendered = _render("ReadFile", {"path": "/repo/big.py"}, output=long_line, width=80) + assert "Read 1 line from big.py (ctrl+o to expand)" in rendered + # Width-capped: the full 500-char line is truncated, so total output stays small. + assert "x" * 500 not in rendered + assert rendered.count("\n") < 6 + + +def test_read_preview_skips_on_narrow_terminal(): + from pythinker_code.ui.shell.tool_renderers.read import _preview + + # Too narrow to show anything useful → no preview at all. + assert _preview("some content here", width=10) is None + # Roomy terminal → preview present. + assert _preview("some content here", width=100) is not None + + +def test_read_preview_truncates_by_cell_width_not_char_count(): + # Wide (2-cell) glyphs must be measured by display width, so the preview + # line fits the column budget instead of overflowing on char count alone. + from pythinker_code.ui.shell.components.render_utils import cell_len + from pythinker_code.ui.shell.tool_renderers.read import _preview + + preview = _preview("世" * 200, width=60) + assert preview is not None + assert cell_len(preview.plain) <= 60 + + +def test_read_collapsed_uses_message_line_count(): + rendered = _render( + "ReadFile", + {"path": "/repo/src/_live_view.py", "line_offset": 1, "n_lines": 1000}, + details={ + "message": "140 lines read from file starting from line 1. Total lines in file: 320.", + "output": " 1\tdef view():\n 2\t return 1", + }, + ) + assert "Read 140 lines from _live_view.py (ctrl+o to expand)" in rendered + + +def test_read_collapsed_falls_back_when_count_unknown(): + # No message and no body text → count is unknowable; never assert a fake 0. + rendered = _render("ReadFile", {"path": "/repo/x.py"}, details={"message": ""}) + assert "Read file content" in rendered + assert "Read 0 lines" not in rendered + + +def test_read_collapsed_empty_file_is_truthful(): + rendered = _render( + "ReadFile", + {"path": "/repo/empty.py"}, + details={"message": "No lines read from file. Total lines in file: 0."}, + ) + assert "Read 0 lines from empty.py" in rendered + # Nothing to expand for an empty file. + assert "ctrl+o to expand" not in rendered + + +def test_read_collapsed_preview_sanitizes_control_sequences(): + body = "\x1b[31mred\x1b[0m\x07\nsecond" + rendered = _render("ReadFile", {"path": "/repo/x.py"}, output=body) + assert "red" in rendered + assert "\x1b" not in rendered + assert "\x07" not in rendered + + +def test_read_expanded_shows_full_content(): + body = "\n".join(f"line {i}" for i in range(20)) + rendered = _render("ReadFile", {"path": "/repo/x.py"}, output=body, expanded=True) + assert "Read 20 lines from x.py" in rendered + # Every line is present when expanded, including past the collapsed cap. + assert "line 0" in rendered + assert "line 19" in rendered def test_read_error_prefers_structured_message(): diff --git a/tests/ui_and_conv/test_tui_transcript_enhancements.py b/tests/ui_and_conv/test_tui_transcript_enhancements.py index 01a1c8c3..340cf1e5 100644 --- a/tests/ui_and_conv/test_tui_transcript_enhancements.py +++ b/tests/ui_and_conv/test_tui_transcript_enhancements.py @@ -71,7 +71,7 @@ def test_finished_expandable_tool_card_remains_available_after_flush(monkeypatch block = view._completed_expandable_tool_card() assert block is not None expanded = render_plain(block.render_expanded(), width=100) - assert "Read 1 file" in expanded + assert "Read 20 lines from big.py" in expanded assert "line 0" in expanded assert "line 19" in expanded From 7ec9dcd44d9565f0e4f01698823ea990e8f5312c Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Wed, 17 Jun 2026 16:57:49 -0400 Subject: [PATCH 4/8] fix(tui): strip ANSI/control sequences from diff card bodies render_diff rendered untrusted file content and model-supplied edit text verbatim, so a crafted edit could smuggle ANSI escapes (cursor movement, color) into the terminal through Update/Write diff cards and the approval/ pager diffs that share the renderer. Sanitize diff_text once at the top of render_diff; sanitize_ansi keeps newlines/tabs so +/- prefix and line-number parsing are unaffected, and visible text is preserved. --- CHANGELOG.md | 5 +++++ src/pythinker_code/ui/shell/components/diff.py | 8 ++++++++ tests/ui_and_conv/test_tui_card_tool_renderers.py | 10 ++++++++++ 3 files changed, 23 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1557ffd4..d4805a23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- **TUI: diff cards strip terminal control sequences.** Inline file-diff bodies + (Update/Write cards, approval and pager diffs) now sanitize ANSI/control escapes + from the untrusted file and model-supplied edit content before rendering, so a + crafted edit can no longer smuggle cursor-movement or color escapes into the + terminal through a diff card. Visible text is preserved. - **TUI: interactive resize/handoff ghosting.** Scrollback handoffs in prompt mode now fully suppress the transient preamble (agent stream body, verb spinner, and tips) while ``run_in_terminal`` emits permanent scrollback, so stacked diff --git a/src/pythinker_code/ui/shell/components/diff.py b/src/pythinker_code/ui/shell/components/diff.py index 80985e8f..6970d3e9 100644 --- a/src/pythinker_code/ui/shell/components/diff.py +++ b/src/pythinker_code/ui/shell/components/diff.py @@ -25,6 +25,7 @@ from rich.table import Table from rich.text import Text +from pythinker_code.ui.shell.components.render_utils import sanitize_ansi from pythinker_code.ui.shell.render_constants import ( DIFF_CONTEXT_LINES, DIFF_LINE_NUMBER_MIN_WIDTH, @@ -403,6 +404,13 @@ def render_diff(diff_text: str, *, path: str | None = None) -> RenderableType: if not diff_text: return Text("") + # Diff bodies carry untrusted file content and model-supplied edit text. Strip + # ANSI/control sequences before rendering so a crafted edit can't smuggle + # cursor-movement or color escapes into the terminal through the diff card. + # sanitize_ansi keeps newlines and tabs, so +/- prefix and line-number parsing + # below is unaffected. + diff_text = sanitize_ansi(diff_text) + colors = get_diff_colors() # Added/removed rows are distinguished by background tint only; line numbers, # +/- markers, and code content all use the terminal's default foreground diff --git a/tests/ui_and_conv/test_tui_card_tool_renderers.py b/tests/ui_and_conv/test_tui_card_tool_renderers.py index b5f49485..6f2e3e02 100644 --- a/tests/ui_and_conv/test_tui_card_tool_renderers.py +++ b/tests/ui_and_conv/test_tui_card_tool_renderers.py @@ -1082,6 +1082,16 @@ def test_render_diff_colorizes_added_removed(): assert "world" in plain +def test_render_diff_strips_ansi_control_sequences(): + """Diff bodies carry untrusted file/model content; a crafted edit must not + smuggle ANSI/control escapes into the terminal through the diff card.""" + diff = "- safe\n+ \x1b[31mRED\x1b[0m\x07evil" + plain = render_plain(render_diff(diff, path="/x.py"), width=80) + assert "\x1b" not in plain # CSI escape stripped + assert "\x07" not in plain # BEL stripped + assert "RED" in plain and "evil" in plain # visible text preserved + + def test_render_diff_spaces_marker_before_at_rule(): old = "@keyframes drawer-fade-in { from { opacity: 0; } to { opacity: 1); } }\n" new = "@keyframes drawer-fade-in { from { opacity: 0; } to { opacity: 1; } }\n" From 4cc080e9f449c0a15ab0a2d8dd8c47cbebc2d01a Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Wed, 17 Jun 2026 16:57:49 -0400 Subject: [PATCH 5/8] style(tui): wrap long scrollback-handoff call to satisfy formatter --- src/pythinker_code/ui/shell/visualize/_interactive.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pythinker_code/ui/shell/visualize/_interactive.py b/src/pythinker_code/ui/shell/visualize/_interactive.py index 0f6ae4ac..777d646b 100644 --- a/src/pythinker_code/ui/shell/visualize/_interactive.py +++ b/src/pythinker_code/ui/shell/visualize/_interactive.py @@ -414,9 +414,7 @@ def emit() -> None: console.print() try: - await self._run_scrollback_handoff( - emit, reason=f"pending_scrollback({len(batch)})" - ) + await self._run_scrollback_handoff(emit, reason=f"pending_scrollback({len(batch)})") except Exception: return From 4bba145bb45cfa2eb12bd9d275bd7f78eb1b3e0d Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Wed, 17 Jun 2026 17:13:18 -0400 Subject: [PATCH 6/8] fix(tui): flush scrollback on turn end during resize recovery Resize recovery could defer pending scrollback indefinitely when prompt geometry was unavailable, leaving PTY sessions stuck on Finalizing without emitting completed response text. Force flush on outermost turn end and always tick recovery down each status refresh frame. --- CHANGELOG.md | 2 + .../ui/shell/visualize/_interactive.py | 31 +++++++-------- .../test_visualize_running_prompt.py | 39 ++++++++++++++++++- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4805a23..30747d54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ GitHub Releases page; `0.8.0` is the new starting line. Terminal resize triggers a hard preamble invalidation and briefly hides tips while prompt_toolkit settles at the new geometry. Handoffs defer during resize recovery; failed emits leave scrollback queued for retry instead of dropping it. + Outermost turn end always flushes completed prose even when recovery is active, + so PTY sessions no longer stall on ``Finalizing…`` without emitting the response. - **DiffLive streaming scroll geometry.** Non-interactive live streaming now uses cursor-down only when the next row provably fits the visible terminal region (frame origin + target row vs height); otherwise it falls back to newline scroll, diff --git a/src/pythinker_code/ui/shell/visualize/_interactive.py b/src/pythinker_code/ui/shell/visualize/_interactive.py index 777d646b..ffbe1328 100644 --- a/src/pythinker_code/ui/shell/visualize/_interactive.py +++ b/src/pythinker_code/ui/shell/visualize/_interactive.py @@ -186,18 +186,16 @@ def _current_terminal_size(self) -> tuple[int, int] | None: def _tick_resize_recovery(self) -> None: """Detect terminal geometry changes and force a hard preamble invalidation.""" size = self._current_terminal_size() - if size is None: - return - columns, rows = size - if columns < 1 or rows < 1: - _handoff_trace(f"RESIZE_IGNORE\t{columns}x{rows}") - return - if self._last_terminal_size != size: - self._last_terminal_size = size - self._resize_recovery_remaining = _RESIZE_RECOVERY_FRAMES - self._force_refresh = True - _handoff_trace(f"RESIZE\t{columns}x{rows}") - return + if size is not None: + columns, rows = size + if columns >= 1 and rows >= 1: + if self._last_terminal_size != size: + self._last_terminal_size = size + self._resize_recovery_remaining = _RESIZE_RECOVERY_FRAMES + self._force_refresh = True + _handoff_trace(f"RESIZE\t{columns}x{rows}") + else: + _handoff_trace(f"RESIZE_IGNORE\t{columns}x{rows}") if self._resize_recovery_remaining > 0: self._resize_recovery_remaining -= 1 @@ -388,7 +386,7 @@ def emit_committed() -> None: async def _after_incremental_scrollback_emitted(self) -> None: self._prompt_session.invalidate() - async def _flush_pending_scrollback(self) -> None: + async def _flush_pending_scrollback(self, *, force: bool = False) -> None: """Drain queued scrollback to scrollback. In a real terminal, route through run_in_terminal so the prompt preamble @@ -398,11 +396,12 @@ async def _flush_pending_scrollback(self) -> None: Scrollback is removed from the queue only after a successful handoff emit. Failed emits leave the queue intact for a later retry; handoffs are deferred - while terminal geometry is settling after a resize. + while terminal geometry is settling after a resize unless ``force`` is set + (e.g. outermost turn end must not leave completed prose stuck finalizing). """ if not self._pending_scrollback: return - if self._defer_scrollback_handoff(): + if not force and self._defer_scrollback_handoff(): _handoff_trace(f"HANDOFF_DEFER\tpending_scrollback({len(self._pending_scrollback)})") return batch = self._pending_scrollback[:] @@ -542,7 +541,7 @@ async def visualize_loop(self, wire: WireUISide): else: self._turn_ended = False self._force_refresh = True - await self._flush_pending_scrollback() + await self._flush_pending_scrollback(force=turn_ended) self._flush_prompt_refresh() continue diff --git a/tests/ui_and_conv/test_visualize_running_prompt.py b/tests/ui_and_conv/test_visualize_running_prompt.py index 650e7772..07219316 100644 --- a/tests/ui_and_conv/test_visualize_running_prompt.py +++ b/tests/ui_and_conv/test_visualize_running_prompt.py @@ -535,12 +535,11 @@ def _size() -> tuple[int, int]: current_size[:] = [100, 30] view._tick_resize_recovery() assert view._force_refresh is True - assert view._resize_recovery_remaining == _interactive_mod._RESIZE_RECOVERY_FRAMES + assert view._resize_recovery_remaining == _interactive_mod._RESIZE_RECOVERY_FRAMES - 1 assert "Tip:" not in view.render_pinned_status_tail(80).value view._force_refresh = False for expected in ( - _interactive_mod._RESIZE_RECOVERY_FRAMES - 1, _interactive_mod._RESIZE_RECOVERY_FRAMES - 2, 0, ): @@ -582,6 +581,42 @@ async def _run_in_terminal(func, *args, **kwargs): # noqa: ANN001, ANN002, ANN0 assert len(view._pending_scrollback) == 1 +@pytest.mark.asyncio +async def test_flush_pending_scrollback_forced_on_turn_end_during_resize_recovery( + monkeypatch, +) -> None: + printed: list[object] = [] + + class _PromptSession: + def invalidate(self) -> None: + pass + + async def _run_in_terminal(func, *args, **kwargs): # noqa: ANN001, ANN002, ANN003 + func() + + monkeypatch.setattr(_interactive_mod, "run_in_terminal", _run_in_terminal) + monkeypatch.setattr( + _live_view_mod.console, + "print", + lambda *args, **kwargs: printed.extend(args) if args else None, + ) + + view = _PromptLiveView( + StatusUpdate(), + prompt_session=cast(Any, _PromptSession()), + steer=lambda _content: None, + ) + view._pending_scrollback.append((Text("Smoke turn one completed."), True)) + view._resize_recovery_remaining = 2 + + await view._flush_pending_scrollback(force=True) + + assert len(printed) == 1 + assert isinstance(printed[0], Text) + assert printed[0].plain == "Smoke turn one completed." + assert view._pending_scrollback == [] + + @pytest.mark.asyncio async def test_flush_pending_scrollback_retains_queue_on_handoff_failure(monkeypatch) -> None: printed: list[object] = [] From 2e34dd2f93875565f58216f598b85eec535d8e5a Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Wed, 17 Jun 2026 17:43:45 -0400 Subject: [PATCH 7/8] fix(tui,llm): address PR review findings and compat-proxy tool results Serialize pending scrollback flush with a lock, force flush on shutdown, and log handoff failures. Flatten tool results for non-native Anthropic/OpenAI hosts so compatibility proxies do not drop multi-part payloads. --- CHANGELOG.md | 10 ++ src/pythinker_code/llm.py | 59 +++++++++- .../ui/shell/visualize/_diff_live.py | 1 + .../ui/shell/visualize/_interactive.py | 50 +++++---- tests/core/test_create_llm.py | 103 ++++++++++++++++++ tests/ui_and_conv/test_diff_live_scroll.py | 11 +- 6 files changed, 206 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30747d54..5409a15f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,16 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- **Fix: tool outputs invisible on Anthropic-compatible proxies (GLM-5.2 via z.ai).** + `api.z.ai/api/anthropic` only surfaces the first content block of a multi-part + `tool_result`, so the leading `` summary reached GLM-5.2 while the actual tool + payload was dropped — every Shell/ReadFile/Grep result read as a "success" summary with + no output (reproduced from a live GLM-5.2 session transcript). Tool results are now + flattened to a single text block for non-native hosts via a transport-keyed resolver + (`resolve_tool_result_mode`), while genuine `api.anthropic.com` keeps the rich + multi-part form. The same single-string mode is applied defensively to non-native + OpenAI-compatible hosts (lossless for text), most relevant to GLM served over z.ai's + OpenAI endpoint; genuine `api.openai.com` is unchanged. - **TUI: diff cards strip terminal control sequences.** Inline file-diff bodies (Update/Write cards, approval and pager diffs) now sanitize ANSI/control escapes from the untrusted file and model-supplied edit content before rendering, so a diff --git a/src/pythinker_code/llm.py b/src/pythinker_code/llm.py index 296dcd92..7134489f 100644 --- a/src/pythinker_code/llm.py +++ b/src/pythinker_code/llm.py @@ -19,6 +19,8 @@ from pythinker_code.utils.logging import logger if TYPE_CHECKING: + from pythinker_core.contrib.chat_provider.common import ToolMessageConversion + from pythinker_code.auth.oauth import OAuthManager from pythinker_code.config import Config, LLMModel, LLMProvider @@ -61,6 +63,24 @@ def model_name(self) -> str: # through `api.anthropic.com` (see `auth/anthropic_direct.py:ANTHROPIC_BASE_URL`). _GENUINE_ANTHROPIC_HOSTS = frozenset({"api.anthropic.com"}) +# Hosts that serve the genuine OpenAI API (as opposed to the many +# OpenAI-compatible proxies that reuse the chat-completions wire format). +_GENUINE_OPENAI_HOSTS = frozenset({"api.openai.com"}) + + +def _normalize_host(base_url: str | None) -> str: + """Lowercased hostname of `base_url`, or "" when absent/unparseable. + + Single source of truth for the genuine-vs-proxy host checks so callers do not + re-implement URL parsing (and so trailing slashes, paths, and case never matter). + """ + if not base_url: + return "" + from urllib.parse import urlparse + + return (urlparse(base_url).hostname or "").lower() + + # Model-name substrings that do NOT support `tool_reference`. Haiku is the only # known unsupported pattern in the deferred tool-search workflow. _TOOL_REFERENCE_UNSUPPORTED_MODEL_PATTERNS = ("haiku",) @@ -102,15 +122,42 @@ def supports_deferred_tool_search(llm: LLM | None) -> bool: return False # type="anthropic" is necessary but NOT sufficient — the compat proxies above # share it. Only the genuine Anthropic host forwards the beta. - from urllib.parse import urlparse - - host = (urlparse(provider.base_url).hostname or "").lower() + host = _normalize_host(provider.base_url) if host not in _GENUINE_ANTHROPIC_HOSTS: return False model = llm.model_name.lower() return not any(pat in model for pat in _TOOL_REFERENCE_UNSUPPORTED_MODEL_PATTERNS) +def resolve_tool_result_mode( + *, api_family: Literal["anthropic", "openai"], base_url: str | None +) -> ToolMessageConversion | None: + """How `role="tool"` results should be serialized for a provider's transport. + + The split that matters is NATIVE endpoint vs COMPATIBILITY PROXY, not which model: + genuine `api.anthropic.com` / `api.openai.com` consume structured multi-part + `tool_result` content faithfully, but the many proxies that merely speak the same + wire format often do not. z.ai/GLM (`api.z.ai/api/anthropic`) honors only the FIRST + content block of an array-form `tool_result`, so the leading `` summary block + reaches the model while the actual tool OUTPUT block is silently dropped — every + Shell/ReadFile result reads as "success" with no payload (confirmed against GLM-5.2). + + For non-native hosts we flatten the tool result to a single text block + (`extract_text`), which puts the whole payload in that first block. The flatten is + lossless for text and is the lowest-common-denominator shape every proxy accepts; it + drops any non-text tool-result block, which a first-block-only proxy could not deliver + anyway. Native hosts keep the rich multi-part form (so tool-result images survive). + + Returns `None` to mean "native multi-part" (the provider default) and `"extract_text"` + to mean "flatten to one string". New families/modes plug in here, not in agent/tool code. + """ + host = _normalize_host(base_url) + native_hosts = _GENUINE_ANTHROPIC_HOSTS if api_family == "anthropic" else _GENUINE_OPENAI_HOSTS + if not host or host in native_hosts: + return None + return "extract_text" + + def model_display_name(model_name: str | None, model: LLMModel | None = None) -> str: if model is not None and model.display_name: return model.display_name @@ -293,6 +340,9 @@ def create_llm( reasoning_key=reasoning_key, default_headers=dict(provider.custom_headers) if provider.custom_headers else None, http_client=rl_http_client, + tool_message_conversion=resolve_tool_result_mode( + api_family="openai", base_url=provider.base_url + ), ) case "openai_responses": from pythinker_core.contrib.chat_provider.openai_responses import OpenAIResponses @@ -333,6 +383,9 @@ def create_llm( metadata={"user_id": session_id} if session_id else None, default_headers=dict(provider.custom_headers) if provider.custom_headers else None, http_client=rl_http_client, + tool_message_conversion=resolve_tool_result_mode( + api_family="anthropic", base_url=provider.base_url + ), ) case "google_genai" | "gemini": from pythinker_core.contrib.chat_provider.google_genai import GoogleGenAI diff --git a/src/pythinker_code/ui/shell/visualize/_diff_live.py b/src/pythinker_code/ui/shell/visualize/_diff_live.py index 2bf11992..790cf821 100644 --- a/src/pythinker_code/ui/shell/visualize/_diff_live.py +++ b/src/pythinker_code/ui/shell/visualize/_diff_live.py @@ -59,6 +59,7 @@ def _diff_live_trace(event: str) -> None: with open(path, "a", encoding="utf-8") as fh: fh.write(f"{time.monotonic():.3f}\t{event}\n") except OSError: + # Diagnostics-only: never let log I/O failures affect UI rendering. pass diff --git a/src/pythinker_code/ui/shell/visualize/_interactive.py b/src/pythinker_code/ui/shell/visualize/_interactive.py index ffbe1328..d4e5535d 100644 --- a/src/pythinker_code/ui/shell/visualize/_interactive.py +++ b/src/pythinker_code/ui/shell/visualize/_interactive.py @@ -51,6 +51,7 @@ ) from pythinker_code.ui.theme import tui_rich_style from pythinker_code.utils.aioqueue import QueueShutDown +from pythinker_code.utils.logging import logger from pythinker_code.utils.slashcmd import SlashCommandCall from pythinker_code.wire import WireUISide from pythinker_code.wire.types import ( @@ -159,6 +160,7 @@ def __init__( self._status_refresh_task: asyncio.Task[None] | None = None self._pending_scrollback: list[tuple[RenderableType, bool]] = [] self._scrollback_handoff_depth: int = 0 + self._scrollback_flush_lock = asyncio.Lock() self._last_terminal_size: tuple[int, int] | None = None self._resize_recovery_remaining: int = 0 @@ -209,6 +211,7 @@ def _safe_prompt_invalidate(self) -> None: self._prompt_session.invalidate() except Exception as exc: # noqa: BLE001 — invalidate must never abort handoff cleanup _handoff_trace(f"INVALIDATE_FAIL\t{type(exc).__name__}:{exc}") + logger.debug("Prompt invalidation failed during scrollback handoff: {}", exc) def _prompt_is_finalizing(self) -> bool: """True while scrollback is queued or being emitted above the prompt.""" @@ -399,26 +402,35 @@ async def _flush_pending_scrollback(self, *, force: bool = False) -> None: while terminal geometry is settling after a resize unless ``force`` is set (e.g. outermost turn end must not leave completed prose stuck finalizing). """ - if not self._pending_scrollback: - return - if not force and self._defer_scrollback_handoff(): - _handoff_trace(f"HANDOFF_DEFER\tpending_scrollback({len(self._pending_scrollback)})") - return - batch = self._pending_scrollback[:] + async with self._scrollback_flush_lock: + if not self._pending_scrollback: + return + if not force and self._defer_scrollback_handoff(): + _handoff_trace( + f"HANDOFF_DEFER\tpending_scrollback({len(self._pending_scrollback)})" + ) + return + batch = self._pending_scrollback[:] - def emit() -> None: - for renderable, blank_row in batch: - console.print(renderable) - if blank_row: - console.print() + def emit() -> None: + for renderable, blank_row in batch: + console.print(renderable) + if blank_row: + console.print() - try: - await self._run_scrollback_handoff(emit, reason=f"pending_scrollback({len(batch)})") - except Exception: - return + try: + await self._run_scrollback_handoff( + emit, reason=f"pending_scrollback({len(batch)})" + ) + except Exception: + logger.exception( + "Failed to flush pending scrollback; retaining {} queued blocks", + len(batch), + ) + return - del self._pending_scrollback[: len(batch)] - self._safe_prompt_invalidate() + del self._pending_scrollback[: len(batch)] + self._safe_prompt_invalidate() def _emit_final_scrollback(self, renderable: RenderableType) -> None: self._pending_scrollback.append((renderable, True)) @@ -517,14 +529,14 @@ async def visualize_loop(self, wire: WireUISide): self._flush_prompt_refresh() continue self.cleanup(is_interrupt=False) - await self._flush_pending_scrollback() + await self._flush_pending_scrollback(force=True) self._force_refresh = True self._flush_prompt_refresh() break if isinstance(msg, StepInterrupted): self.cleanup(is_interrupt=True) - await self._flush_pending_scrollback() + await self._flush_pending_scrollback(force=True) self._force_refresh = True self._flush_prompt_refresh() break diff --git a/tests/core/test_create_llm.py b/tests/core/test_create_llm.py index b9fdce74..4f4da6e3 100644 --- a/tests/core/test_create_llm.py +++ b/tests/core/test_create_llm.py @@ -4,6 +4,7 @@ from pydantic import SecretStr from pythinker_core.chat_provider.echo import EchoChatProvider from pythinker_core.chat_provider.pythinker import Pythinker +from pythinker_core.contrib.chat_provider.anthropic import Anthropic from pythinker_core.contrib.chat_provider.openai_legacy import OpenAILegacy from pythinker_core.contrib.chat_provider.openai_responses import OpenAIResponses @@ -13,6 +14,7 @@ clone_llm_with_model_alias, create_llm, derive_model_capabilities, + resolve_tool_result_mode, ) @@ -881,3 +883,104 @@ def test_create_llm_kimi_k2_thinking_ignores_thinking_off(): assert llm.chat_provider._generation_kwargs.get("extra_body") == { # pyright: ignore[reportPrivateUsage] "thinking": {"type": "enabled"} } + + +def test_resolve_tool_result_mode_native_vs_compat_proxy(): + # Native endpoints consume multi-part tool_result content; compatibility proxies + # get flattened to one string. Keyed on transport (host), never on model name. + assert resolve_tool_result_mode(api_family="anthropic", base_url=None) is None + assert ( + resolve_tool_result_mode(api_family="anthropic", base_url="https://api.anthropic.com") + is None + ) + assert resolve_tool_result_mode(api_family="openai", base_url="https://api.openai.com") is None + # Anthropic-compatible proxies (z.ai/GLM, MiniMax, Kimi) → flatten. + assert ( + resolve_tool_result_mode(api_family="anthropic", base_url="https://api.z.ai/api/anthropic") + == "extract_text" + ) + # OpenAI-compatible proxies (DeepSeek, xAI/Grok, …) → flatten. + assert ( + resolve_tool_result_mode(api_family="openai", base_url="https://api.deepseek.com/v1") + == "extract_text" + ) + assert ( + resolve_tool_result_mode(api_family="openai", base_url="https://api.x.ai/v1") + == "extract_text" + ) + + +def test_resolve_tool_result_mode_host_normalization(): + # Trailing slashes, paths, and case must not change the genuine-host verdict. + for url in ( + "https://api.anthropic.com/", + "https://API.Anthropic.com", + "https://api.anthropic.com/v1/messages", + ): + assert resolve_tool_result_mode(api_family="anthropic", base_url=url) is None + # A lookalike host must NOT be treated as genuine. + assert ( + resolve_tool_result_mode(api_family="anthropic", base_url="https://fake-anthropic.com") + == "extract_text" + ) + + +def test_create_llm_zai_anthropic_proxy_flattens_tool_results(): + # z.ai's Anthropic-compatible proxy only honors the first content block of an + # array-form tool_result, so multi-block results (system summary + output) must + # be flattened to a single text block or the model never sees the tool output. + provider = LLMProvider( + type="anthropic", + base_url="https://api.z.ai/api/anthropic", + api_key=SecretStr("test-key"), + ) + model = LLMModel( + provider="managed:z-ai", + model="glm-5.2", + max_context_size=200_000, + capabilities=None, + ) + llm = create_llm(provider, model) + assert llm is not None + assert isinstance(llm.chat_provider, Anthropic) + assert llm.chat_provider._tool_message_conversion == "extract_text" # pyright: ignore[reportPrivateUsage] + + +def test_create_llm_genuine_anthropic_keeps_array_tool_results(): + # Genuine api.anthropic.com handles array-form tool_result content (incl. images), + # so it must NOT be flattened. + provider = LLMProvider( + type="anthropic", + base_url="https://api.anthropic.com", + api_key=SecretStr("test-key"), + ) + model = LLMModel( + provider="anthropic", + model="claude-sonnet-4-6", + max_context_size=200_000, + capabilities=None, + ) + llm = create_llm(provider, model) + assert llm is not None + assert isinstance(llm.chat_provider, Anthropic) + assert llm.chat_provider._tool_message_conversion is None # pyright: ignore[reportPrivateUsage] + + +def test_create_llm_openai_compatible_proxy_flattens_tool_results(): + # OpenAI-compatible proxies share the chat-completions wire format but vary on + # multi-part tool_result support, so they get the single-string safe mode too. + provider = LLMProvider( + type="openai_legacy", + base_url="https://api.deepseek.com/v1", + api_key=SecretStr("test-key"), + ) + model = LLMModel( + provider="deepseek", + model="deepseek-chat", + max_context_size=64_000, + capabilities=None, + ) + llm = create_llm(provider, model) + assert llm is not None + assert isinstance(llm.chat_provider, OpenAILegacy) + assert llm.chat_provider._tool_message_conversion == "extract_text" # pyright: ignore[reportPrivateUsage] diff --git a/tests/ui_and_conv/test_diff_live_scroll.py b/tests/ui_and_conv/test_diff_live_scroll.py index b6e649e2..9112682b 100644 --- a/tests/ui_and_conv/test_diff_live_scroll.py +++ b/tests/ui_and_conv/test_diff_live_scroll.py @@ -61,18 +61,17 @@ def _scroll_bytes() -> str: @pytest.mark.parametrize( - ("origin", "height", "base_row", "target_row", "expect_cud", "expect_scroll"), + ("origin", "height", "target_row", "expect_cud", "expect_scroll"), [ - (0, 40, 1, 5, True, False), - (35, 40, 1, 5, False, True), - (None, 40, 1, 5, False, True), - (0, 40, 1, 40, False, True), + (0, 40, 5, True, False), + (35, 40, 5, False, True), + (None, 40, 5, False, True), + (0, 40, 40, False, True), ], ) def test_append_row_transition_geometry( origin: int | None, height: int, - base_row: int, target_row: int, *, expect_cud: bool, From 78a15211e46a989683953cea6456a6fd8b609dc3 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Wed, 17 Jun 2026 17:59:14 -0400 Subject: [PATCH 8/8] style(tui): fix ruff format on scrollback handoff call --- src/pythinker_code/ui/shell/visualize/_interactive.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pythinker_code/ui/shell/visualize/_interactive.py b/src/pythinker_code/ui/shell/visualize/_interactive.py index d4e5535d..f876b042 100644 --- a/src/pythinker_code/ui/shell/visualize/_interactive.py +++ b/src/pythinker_code/ui/shell/visualize/_interactive.py @@ -419,9 +419,7 @@ def emit() -> None: console.print() try: - await self._run_scrollback_handoff( - emit, reason=f"pending_scrollback({len(batch)})" - ) + await self._run_scrollback_handoff(emit, reason=f"pending_scrollback({len(batch)})") except Exception: logger.exception( "Failed to flush pending scrollback; retaining {} queued blocks",