mirror of
https://github.com/NCBM/plyngent.git
synced 2026-11-07 07:17:54 +08:00
cli/display: close an interrupted pretty line instead of erasing it
This commit is contained in:
+44
-18
@@ -181,7 +181,10 @@ class PrettyLine:
|
||||
|
||||
``prefix`` is written as soon as the call starts — a blocking tool call is
|
||||
then visible while it runs — and ``detail`` is appended once its result
|
||||
lands. Together they equal the single line the whole-line path prints.
|
||||
lands, so the two land on one line. Together they equal the single line the
|
||||
whole-line path prints. When something else is printed in between (a
|
||||
parallel call, a confirm/ask prompt) the prefix stays as its own line and
|
||||
the result is printed as a complete line instead.
|
||||
"""
|
||||
|
||||
prefix: Callable[[str], str]
|
||||
@@ -867,14 +870,14 @@ def _echo_stream(text: str) -> None:
|
||||
_ = sys.stdout.flush()
|
||||
|
||||
|
||||
def _clear_streamed_lines(line_count: int, *, resume_above: bool = False) -> None:
|
||||
def _clear_streamed_lines(line_count: int) -> None:
|
||||
"""Clear exactly *line_count* terminal lines ending at the cursor line.
|
||||
|
||||
The cursor ends at column 0 of the topmost cleared line, so a re-render
|
||||
overwrites the erased block in place instead of appending below it. With
|
||||
``resume_above`` the cursor steps one line further up: the next
|
||||
``"\\n"``-prefixed write then reuses the last cleared line — needed by the
|
||||
parallel-batch fallback, which must not touch the line above the prefix.
|
||||
overwrites the erased block in place instead of appending below it. Only the
|
||||
assistant markdown re-render uses this: pretty tool lines never move the
|
||||
cursor, because an interrupted prefix is *closed* as its own line instead
|
||||
(see :class:`_PrettyToolStream`).
|
||||
|
||||
``line_count`` counts *physical* rows; callers fold wrapped lines via
|
||||
:func:`_line_count_for_clear`.
|
||||
@@ -885,8 +888,6 @@ def _clear_streamed_lines(line_count: int, *, resume_above: bool = False) -> Non
|
||||
for _ in range(line_count - 1):
|
||||
_ = sys.stdout.write("\r\033[2K\033[1A")
|
||||
_ = sys.stdout.write("\r\033[2K")
|
||||
if resume_above:
|
||||
_ = sys.stdout.write("\033[1A")
|
||||
with contextlib.suppress(OSError):
|
||||
_ = sys.stdout.flush()
|
||||
|
||||
@@ -926,10 +927,12 @@ class _PrettyToolStream:
|
||||
|
||||
Writing the prefix immediately keeps a blocking tool call visible; appending
|
||||
the detail later reproduces the single whole-line write byte for byte, so
|
||||
non-interactive output is unchanged. The calls of one batch are yielded
|
||||
before their results, so a second call while a prefix is open (parallel
|
||||
tools) cannot append its detail in order: that prefix is erased and the
|
||||
batch falls back to whole lines.
|
||||
non-interactive output is unchanged. Nothing ever moves the cursor: when
|
||||
something else is printed in between — a second call of the batch (parallel
|
||||
tools) or a confirm/ask prompt — the open prefix is *closed* as its own line
|
||||
and the rest of the batch prints whole lines (``prefix + detail``). A prefix
|
||||
wider than the terminal therefore degrades cleanly instead of leaving a
|
||||
half-erased row behind.
|
||||
"""
|
||||
|
||||
_interactive: bool
|
||||
@@ -946,17 +949,15 @@ class _PrettyToolStream:
|
||||
if not (self._interactive and prefix) or self._whole_lines:
|
||||
return
|
||||
if self._open:
|
||||
# Parallel batch: the open prefix could never receive its detail.
|
||||
# Erase only the prefix line — the line above may be streamed
|
||||
# reasoning — and let the fallback's leading newline reuse it.
|
||||
_clear_streamed_lines(1, resume_above=True)
|
||||
self._open = False
|
||||
self._whole_lines = True
|
||||
# Parallel batch: the open prefix can never receive its detail, so
|
||||
# end it as its own line and print whole lines from here on.
|
||||
self.close_line()
|
||||
return
|
||||
click.echo(f"\n{prefix}", nl=False)
|
||||
with contextlib.suppress(OSError):
|
||||
_ = sys.stdout.flush()
|
||||
self._open = True
|
||||
_mark_open_prefix(self)
|
||||
|
||||
def finish_call(self, detail: str) -> bool:
|
||||
"""Append *detail* to the open prefix; False when a whole line is needed."""
|
||||
@@ -964,6 +965,7 @@ class _PrettyToolStream:
|
||||
return False
|
||||
click.echo(f"{detail}\n", nl=False)
|
||||
self._open = False
|
||||
_mark_open_prefix(None)
|
||||
return True
|
||||
|
||||
def close_line(self) -> None:
|
||||
@@ -973,6 +975,7 @@ class _PrettyToolStream:
|
||||
click.echo()
|
||||
self._open = False
|
||||
self._whole_lines = True
|
||||
_mark_open_prefix(None)
|
||||
|
||||
def end_batch(self) -> None:
|
||||
"""Reset batch-scoped state once every call of the batch has a result."""
|
||||
@@ -980,6 +983,29 @@ class _PrettyToolStream:
|
||||
self._whole_lines = False
|
||||
|
||||
|
||||
# The stream with a prefix line still open (cursor mid-line), or None. A confirm
|
||||
# box or ``ask_user_*`` prompt is printed from a worker thread while the tool
|
||||
# runs, so it needs a way to end that line before writing its own.
|
||||
_open_prefix_stream: _PrettyToolStream | None = None
|
||||
|
||||
|
||||
def _mark_open_prefix(stream: _PrettyToolStream | None) -> None:
|
||||
global _open_prefix_stream # noqa: PLW0603
|
||||
_open_prefix_stream = stream
|
||||
|
||||
|
||||
def close_open_pretty_line() -> None:
|
||||
"""End an open pretty prefix line so other output starts on its own line.
|
||||
|
||||
Wired into the prompt backend (:func:`plyngent.prompting.set_prompt_output_hook`):
|
||||
without it a confirm/ask prompt raised while a tool runs is written onto the
|
||||
still-open prefix line, and the result would later be appended to that mess.
|
||||
"""
|
||||
stream = _open_prefix_stream
|
||||
if stream is not None:
|
||||
stream.close_line()
|
||||
|
||||
|
||||
def print_markdown(text: str, *, label: str = "assistant:") -> None:
|
||||
"""Render *text* as markdown via Rich; *label* on its own line when set."""
|
||||
from rich.console import Console
|
||||
|
||||
@@ -951,11 +951,11 @@ async def test_pretty_verbose_keeps_full_result_output(
|
||||
assert "* Read 'a.txt' " not in out
|
||||
|
||||
|
||||
async def test_pretty_parallel_batch_erases_open_prefix(
|
||||
async def test_pretty_parallel_batch_closes_the_open_prefix(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
) -> None:
|
||||
"""A second call in the batch erases the open prefix; results print whole."""
|
||||
"""A second call in the batch ends the open prefix; results print whole lines."""
|
||||
monkeypatch.setattr("plyngent.cli.display.interactive_terminal", lambda: True)
|
||||
await render_events(
|
||||
_aiter(
|
||||
@@ -968,20 +968,20 @@ async def test_pretty_parallel_batch_erases_open_prefix(
|
||||
)
|
||||
)
|
||||
out = capsys.readouterr().out
|
||||
assert "\x1b[2K\x1b[1A" in out # the in-flight prefix was erased
|
||||
assert "* Read 'a.txt' L1-4 (done)\n" in out
|
||||
assert "* Read 'b.txt' L1-9 (done)\n" in out
|
||||
# The prefix survives as its own line; whole lines take over from there.
|
||||
assert out == ("\n* Read 'a.txt' \n\n* Read 'a.txt' L1-4 (done)\n\n* Read 'b.txt' L1-9 (done)\n\n")
|
||||
assert "\x1b[" not in out # the cursor is never moved
|
||||
|
||||
|
||||
async def test_pretty_parallel_batch_keeps_the_line_above_the_prefix(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
) -> None:
|
||||
"""Regression: erasing the open prefix must not eat the line above it.
|
||||
"""Ending the open prefix must not touch the line above it.
|
||||
|
||||
Reasoning streams without a trailing newline, so the prefix line sits right
|
||||
below the reasoning text; the erase used to clear one row too many and wiped
|
||||
that reasoning line out of the screen.
|
||||
below the reasoning text; moving the cursor up used to wipe that reasoning
|
||||
line out of the screen.
|
||||
"""
|
||||
monkeypatch.setattr("plyngent.cli.display.interactive_terminal", lambda: True)
|
||||
await render_events(
|
||||
@@ -997,13 +997,9 @@ async def test_pretty_parallel_batch_keeps_the_line_above_the_prefix(
|
||||
)
|
||||
out = capsys.readouterr().out
|
||||
assert out == (
|
||||
"\nreasoning:\nthink"
|
||||
"\n* Read 'a.txt' "
|
||||
"\r\x1b[2K\x1b[1A"
|
||||
"\n* Read 'a.txt' L1-4 (done)\n"
|
||||
"\n* Read 'b.txt' L1-9 (done)\n"
|
||||
"\n\n"
|
||||
"\nreasoning:\nthink\n* Read 'a.txt' \n\n* Read 'a.txt' L1-4 (done)\n\n* Read 'b.txt' L1-9 (done)\n\n\n"
|
||||
)
|
||||
assert "\x1b[" not in out
|
||||
|
||||
|
||||
def test_clear_streamed_lines_clears_exactly_the_given_rows(capsys: pytest.CaptureFixture[str]) -> None:
|
||||
@@ -1012,12 +1008,6 @@ def test_clear_streamed_lines_clears_exactly_the_given_rows(capsys: pytest.Captu
|
||||
assert capsys.readouterr().out == "\r\x1b[2K\x1b[1A\r\x1b[2K"
|
||||
|
||||
|
||||
def test_clear_streamed_lines_resume_above_steps_back_one_row(capsys: pytest.CaptureFixture[str]) -> None:
|
||||
"""``resume_above`` clears the cursor row only, then steps above it."""
|
||||
_clear_streamed_lines(1, resume_above=True)
|
||||
assert capsys.readouterr().out == "\r\x1b[2K\x1b[1A"
|
||||
|
||||
|
||||
def test_clear_streamed_lines_ignores_non_positive_counts(capsys: pytest.CaptureFixture[str]) -> None:
|
||||
_clear_streamed_lines(0)
|
||||
_clear_streamed_lines(-1)
|
||||
|
||||
Reference in New Issue
Block a user