Harden raw patch rendering in the TUI
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
6c88df28b389f4aae70986f0d05c28f8852ff32e- Parents
-
5943c43 - Tree
94eb2e9
6c88df2
6c88df28b389f4aae70986f0d05c28f8852ff32e5943c43
94eb2e9| Status | File | + | - |
|---|---|---|---|
| M |
src/loader/ui/app.py
|
9 | 3 |
| M |
src/loader/ui/widgets/approval_bar.py
|
30 | 25 |
| M |
src/loader/ui/widgets/confirmation.py
|
6 | 3 |
| M |
src/loader/ui/widgets/diff_widget.py
|
1 | 0 |
| M |
src/loader/ui/widgets/tool_widget.py
|
7 | 1 |
| M |
src/loader/utils/file_mutations.py
|
7 | 5 |
| M |
tests/test_bash_operator_surfaces.py
|
124 | 1 |
src/loader/ui/app.pymodified@@ -157,10 +157,16 @@ class LoaderApp(App): | ||
| 157 | 157 | "[dim]Commands: /help, /model, /jobs, /wait, /kill, /clear, /exit[/dim]" |
| 158 | 158 | ) |
| 159 | 159 | |
| 160 | - def _add_message(self, content: str, classes: str = "") -> None: | |
| 160 | + def _add_message( | |
| 161 | + self, | |
| 162 | + content: str, | |
| 163 | + classes: str = "", | |
| 164 | + *, | |
| 165 | + markup: bool = True, | |
| 166 | + ) -> None: | |
| 161 | 167 | """Add a message to the message area.""" |
| 162 | 168 | msg_area = self.query_one("#message-area", ScrollableContainer) |
| 163 | - widget = Static(content, classes=classes) | |
| 169 | + widget = Static(content, classes=classes, markup=markup) | |
| 164 | 170 | msg_area.mount(widget) |
| 165 | 171 | msg_area.scroll_end(animate=False) |
| 166 | 172 | |
@@ -868,7 +874,7 @@ class LoaderApp(App): | ||
| 868 | 874 | # If no content was streamed but we have a response, display it |
| 869 | 875 | # This handles cases like empty LLM responses with fallback messages |
| 870 | 876 | if not self._streamed_content and message.content.strip(): |
| 871 | - self._add_message(message.content) | |
| 877 | + self._add_message(message.content, markup=False) | |
| 872 | 878 | |
| 873 | 879 | def on_definition_of_done_updated(self, message: DefinitionOfDoneUpdated) -> None: |
| 874 | 880 | """Handle definition-of-done status changes.""" |
src/loader/ui/widgets/approval_bar.pymodified@@ -72,7 +72,7 @@ class ApprovalBar(Widget, can_focus=True): | ||
| 72 | 72 | self.can_focus = True |
| 73 | 73 | |
| 74 | 74 | def compose(self) -> ComposeResult: |
| 75 | - yield Static("", id="approval-content") | |
| 75 | + yield Static("", id="approval-content", markup=False) | |
| 76 | 76 | |
| 77 | 77 | def show_approval( |
| 78 | 78 | self, |
@@ -93,19 +93,11 @@ class ApprovalBar(Widget, can_focus=True): | ||
| 93 | 93 | self._full_command = details |
| 94 | 94 | self._preview = preview |
| 95 | 95 | |
| 96 | - preview = details if details else message | |
| 96 | + preview_text = details if details else message | |
| 97 | 97 | content = self.query_one("#approval-content", Static) |
| 98 | 98 | if tool_name == "bash": |
| 99 | 99 | header = Text("Bash", style="bold yellow") |
| 100 | - command = Text(preview or "(empty command)") | |
| 101 | - controls = Text.assemble( | |
| 102 | - ("[Y]", "bold green"), | |
| 103 | - ("es ",), | |
| 104 | - ("[n]", "bold red"), | |
| 105 | - ("o ",), | |
| 106 | - ("[e]", "bold"), | |
| 107 | - ("dit",), | |
| 108 | - ) | |
| 100 | + command = Text(preview_text or "(empty command)") | |
| 109 | 101 | content.update( |
| 110 | 102 | Group( |
| 111 | 103 | header, |
@@ -116,19 +108,11 @@ class ApprovalBar(Widget, can_focus=True): | ||
| 116 | 108 | box=box.SQUARE, |
| 117 | 109 | expand=True, |
| 118 | 110 | ), |
| 119 | - controls, | |
| 111 | + _approval_controls(), | |
| 120 | 112 | ) |
| 121 | 113 | ) |
| 122 | 114 | elif self._preview: |
| 123 | 115 | header = Text(f"Approve {_label_for_tool(tool_name)}", style="bold yellow") |
| 124 | - controls = Text.assemble( | |
| 125 | - ("[Y]", "bold green"), | |
| 126 | - ("es ",), | |
| 127 | - ("[n]", "bold red"), | |
| 128 | - ("o ",), | |
| 129 | - ("[e]", "bold"), | |
| 130 | - ("dit",), | |
| 131 | - ) | |
| 132 | 116 | content.update( |
| 133 | 117 | Group( |
| 134 | 118 | header, |
@@ -139,15 +123,25 @@ class ApprovalBar(Widget, can_focus=True): | ||
| 139 | 123 | max_lines=20, |
| 140 | 124 | max_chars=2_500, |
| 141 | 125 | ), |
| 142 | - controls, | |
| 126 | + _approval_controls(), | |
| 143 | 127 | ) |
| 144 | 128 | ) |
| 145 | 129 | else: |
| 146 | - if len(preview) > 70: | |
| 147 | - preview = preview[:67] + "..." | |
| 130 | + if len(preview_text) > 400: | |
| 131 | + preview_text = preview_text[:397] + "..." | |
| 132 | + header = Text(f"Approve {_label_for_tool(tool_name)}", style="bold yellow") | |
| 148 | 133 | content.update( |
| 149 | - f"[bold $warning]\\[{tool_name}][/] {preview} " | |
| 150 | - f"[bold green]\\[Y][/]es [bold red]\\[n][/]o [bold]\\[e][/]dit" | |
| 134 | + Group( | |
| 135 | + header, | |
| 136 | + Panel( | |
| 137 | + Text(preview_text or "(no details)"), | |
| 138 | + title="Details", | |
| 139 | + border_style="yellow", | |
| 140 | + box=box.SQUARE, | |
| 141 | + expand=True, | |
| 142 | + ), | |
| 143 | + _approval_controls(), | |
| 144 | + ) | |
| 151 | 145 | ) |
| 152 | 146 | |
| 153 | 147 | # Show the bar |
@@ -225,3 +219,14 @@ def _label_for_tool(tool_name: str) -> str: | ||
| 225 | 219 | "patch": "Patch", |
| 226 | 220 | "bash": "Bash", |
| 227 | 221 | }.get(tool_name, tool_name) |
| 222 | + | |
| 223 | + | |
| 224 | +def _approval_controls() -> Text: | |
| 225 | + return Text.assemble( | |
| 226 | + ("[Y]", "bold green"), | |
| 227 | + ("es ",), | |
| 228 | + ("[n]", "bold red"), | |
| 229 | + ("o ",), | |
| 230 | + ("[e]", "bold"), | |
| 231 | + ("dit",), | |
| 232 | + ) | |
src/loader/ui/widgets/confirmation.pymodified@@ -1,5 +1,6 @@ | ||
| 1 | 1 | """Confirmation modal for destructive tool operations.""" |
| 2 | 2 | |
| 3 | +from rich.text import Text | |
| 3 | 4 | from textual.app import ComposeResult |
| 4 | 5 | from textual.binding import Binding |
| 5 | 6 | from textual.containers import Horizontal, Vertical |
@@ -80,14 +81,16 @@ class ConfirmationModal(ModalScreen[bool]): | ||
| 80 | 81 | def compose(self) -> ComposeResult: |
| 81 | 82 | with Vertical(id="confirmation-dialog"): |
| 82 | 83 | yield Static( |
| 83 | - f"[bold yellow]⚠ Confirm {self.tool_name}[/bold yellow]", | |
| 84 | + Text(f"⚠ Confirm {self.tool_name}", style="bold yellow"), | |
| 84 | 85 | id="confirmation-title", |
| 86 | + markup=False, | |
| 85 | 87 | ) |
| 86 | - yield Static(self.message, id="confirmation-message") | |
| 88 | + yield Static(Text(self.message), id="confirmation-message", markup=False) | |
| 87 | 89 | if self.details: |
| 88 | 90 | yield Static( |
| 89 | - f"[dim]{self.details}[/dim]", | |
| 91 | + Text(self.details, style="dim"), | |
| 90 | 92 | id="confirmation-details", |
| 93 | + markup=False, | |
| 91 | 94 | ) |
| 92 | 95 | with Horizontal(id="confirmation-buttons"): |
| 93 | 96 | yield Button("Yes (y)", id="btn-yes", variant="success") |
src/loader/ui/widgets/diff_widget.pymodified@@ -24,4 +24,5 @@ class DiffWidget(Vertical): | ||
| 24 | 24 | max_chars=6_000, |
| 25 | 25 | ), |
| 26 | 26 | classes="diff-content", |
| 27 | + markup=False, | |
| 27 | 28 | ) |
src/loader/ui/widgets/tool_widget.pymodified@@ -64,8 +64,14 @@ class ToolCallWidget(Vertical): | ||
| 64 | 64 | self._header_renderable(), |
| 65 | 65 | id="tool-header", |
| 66 | 66 | classes="tool-header", |
| 67 | + markup=False, | |
| 68 | + ) | |
| 69 | + yield Static( | |
| 70 | + self._build_initial_summary(), | |
| 71 | + id="tool-summary", | |
| 72 | + classes="tool-summary", | |
| 73 | + markup=False, | |
| 67 | 74 | ) |
| 68 | - yield Static(self._build_initial_summary(), id="tool-summary", classes="tool-summary") | |
| 69 | 75 | |
| 70 | 76 | def _format_args(self) -> str: |
| 71 | 77 | """Format tool arguments for display.""" |
src/loader/utils/file_mutations.pymodified@@ -330,8 +330,8 @@ def _iter_rendered_patch_lines( | ||
| 330 | 330 | old_line = hunk.old_start |
| 331 | 331 | new_line = hunk.new_start |
| 332 | 332 | for raw_line in hunk.lines: |
| 333 | - prefix = raw_line[:1] | |
| 334 | - content = raw_line[1:] if raw_line else "" | |
| 333 | + prefix = raw_line[:1] if raw_line[:1] in {" ", "+", "-"} else "" | |
| 334 | + content = raw_line[1:] if prefix else raw_line | |
| 335 | 335 | display = Text() |
| 336 | 336 | old_label = " " |
| 337 | 337 | new_label = " " |
@@ -353,10 +353,12 @@ def _iter_rendered_patch_lines( | ||
| 353 | 353 | elif prefix == "-": |
| 354 | 354 | display.append("- ", style="red") |
| 355 | 355 | display.append(content, style="red") |
| 356 | + elif prefix == " ": | |
| 357 | + display.append(" ", style="dim") | |
| 358 | + display.append(content, style="dim") | |
| 356 | 359 | else: |
| 357 | - marker = " " if prefix == " " else f"{prefix} " | |
| 358 | - display.append(marker, style="dim") | |
| 359 | - display.append(content, style="dim" if prefix == " " else "") | |
| 360 | + display.append(" ", style="dim") | |
| 361 | + display.append(content) | |
| 360 | 362 | display.append("\n") |
| 361 | 363 | rendered.append(display) |
| 362 | 364 | return rendered |
tests/test_bash_operator_surfaces.pymodified@@ -12,7 +12,12 @@ from textual.widgets import Static | ||
| 12 | 12 | import loader.cli.main as cli_main_module |
| 13 | 13 | from loader.runtime.events import AgentEvent |
| 14 | 14 | from loader.tools import BashTool |
| 15 | -from loader.ui.adapter import EventAdapter, ToolCallCompleted, ToolCallStarted | |
| 15 | +from loader.ui.adapter import ( | |
| 16 | + EventAdapter, | |
| 17 | + ResponseComplete, | |
| 18 | + ToolCallCompleted, | |
| 19 | + ToolCallStarted, | |
| 20 | +) | |
| 16 | 21 | from loader.ui.app import LoaderApp |
| 17 | 22 | from loader.ui.widgets import ApprovalBar, DiffWidget |
| 18 | 23 | from loader.ui.widgets.tool_widget import ToolCallWidget |
@@ -63,6 +68,50 @@ def _patch_tool_args() -> dict[str, object]: | ||
| 63 | 68 | } |
| 64 | 69 | |
| 65 | 70 | |
| 71 | +def _raw_patch_tool_args() -> dict[str, object]: | |
| 72 | + return { | |
| 73 | + "file_path": "animals/index.html", | |
| 74 | + "hunks": [ | |
| 75 | + { | |
| 76 | + "old_start": 53, | |
| 77 | + "old_lines": 1, | |
| 78 | + "new_start": 53, | |
| 79 | + "new_lines": 5, | |
| 80 | + "lines": [ | |
| 81 | + "</body>", | |
| 82 | + "</html>", | |
| 83 | + "", | |
| 84 | + "<!-- New animal entries -->", | |
| 85 | + '<div class="animal-card">', | |
| 86 | + '<h2><a href="wolf.html">Wolf</a></h2>', | |
| 87 | + ( | |
| 88 | + "<p>Wolves are wild canines that live in packs and are known " | |
| 89 | + "for their intelligence and social behavior.</p>" | |
| 90 | + ), | |
| 91 | + "</div>", | |
| 92 | + "", | |
| 93 | + '<div class="animal-card">', | |
| 94 | + '<h2><a href="bear.html">Bear</a></h2>', | |
| 95 | + ( | |
| 96 | + "<p>Bears are large mammals that are found in various parts " | |
| 97 | + "of the world, known for their strength and omnivorous diet.</p>" | |
| 98 | + ), | |
| 99 | + "</div>", | |
| 100 | + "", | |
| 101 | + '<div class="animal-card">', | |
| 102 | + '<h2><a href="penguin.html">Penguin</a></h2>', | |
| 103 | + ( | |
| 104 | + "<p>Penguins are flightless birds that live in the Southern " | |
| 105 | + "Hemisphere, known for their distinctive waddle and swimming " | |
| 106 | + "abilities.</p>" | |
| 107 | + ), | |
| 108 | + "</div>", | |
| 109 | + ], | |
| 110 | + } | |
| 111 | + ], | |
| 112 | + } | |
| 113 | + | |
| 114 | + | |
| 66 | 115 | def _render_text(renderable, *, width: int = 100) -> str: |
| 67 | 116 | console = Console(record=True, width=width) |
| 68 | 117 | console.print(renderable) |
@@ -293,6 +342,32 @@ async def test_approval_bar_renders_file_mutation_preview() -> None: | ||
| 293 | 342 | assert "Patch(index.html)" in rendered |
| 294 | 343 | |
| 295 | 344 | |
| 345 | +@pytest.mark.asyncio | |
| 346 | +async def test_approval_bar_fallback_handles_raw_patch_details() -> None: | |
| 347 | + app = _ApprovalHost() | |
| 348 | + raw_details = ( | |
| 349 | + "patch(file_path=\"animals/index.html\", hunks=" | |
| 350 | + f"{_raw_patch_tool_args()['hunks']})" | |
| 351 | + ) | |
| 352 | + | |
| 353 | + async with app.run_test() as pilot: | |
| 354 | + bar = app.query_one(ApprovalBar) | |
| 355 | + bar.show_approval( | |
| 356 | + "patch", | |
| 357 | + "Patch file: animals/index.html", | |
| 358 | + raw_details, | |
| 359 | + preview=None, | |
| 360 | + ) | |
| 361 | + await pilot.pause() | |
| 362 | + | |
| 363 | + content = bar.query_one("#approval-content", Static) | |
| 364 | + rendered = _render_text(content.content, width=120) | |
| 365 | + | |
| 366 | + assert "Approve Patch" in rendered | |
| 367 | + assert "Details" in rendered | |
| 368 | + assert "wolf.html" in rendered | |
| 369 | + | |
| 370 | + | |
| 296 | 371 | @pytest.mark.asyncio |
| 297 | 372 | async def test_loader_app_replaces_patch_tool_widget_with_diff_widget() -> None: |
| 298 | 373 | tool_args = _patch_tool_args() |
@@ -332,6 +407,32 @@ async def test_loader_app_replaces_patch_tool_widget_with_diff_widget() -> None: | ||
| 332 | 407 | assert len(list(app.query(ToolCallWidget))) == 0 |
| 333 | 408 | |
| 334 | 409 | |
| 410 | +@pytest.mark.asyncio | |
| 411 | +async def test_loader_app_mounts_raw_patch_preview_without_markup_crash() -> None: | |
| 412 | + app = LoaderApp(shell_owner=_FakeShellOwner()) | |
| 413 | + | |
| 414 | + async with app.run_test() as pilot: | |
| 415 | + app.post_message( | |
| 416 | + ToolCallStarted( | |
| 417 | + tool_name="patch", | |
| 418 | + tool_args=_raw_patch_tool_args(), | |
| 419 | + tool_call_id="patch-call-raw", | |
| 420 | + phase="assistant", | |
| 421 | + ) | |
| 422 | + ) | |
| 423 | + await pilot.pause() | |
| 424 | + | |
| 425 | + widget = next(iter(app.query(ToolCallWidget))) | |
| 426 | + summary = widget.query_one("#tool-summary", Static) | |
| 427 | + rendered = _render_text(summary.content, width=120) | |
| 428 | + | |
| 429 | + assert "Preview" in rendered | |
| 430 | + assert "<h2><a href=\"wolf.html\">Wolf</a></h2>" in rendered | |
| 431 | + assert "wolf.html" in rendered | |
| 432 | + assert "penguin.html" in rendered | |
| 433 | + assert "< /body>" not in rendered | |
| 434 | + | |
| 435 | + | |
| 335 | 436 | @pytest.mark.asyncio |
| 336 | 437 | async def test_loader_app_matches_repeated_tool_results_by_tool_call_id() -> None: |
| 337 | 438 | app = LoaderApp(shell_owner=_FakeShellOwner()) |
@@ -375,6 +476,28 @@ async def test_loader_app_matches_repeated_tool_results_by_tool_call_id() -> Non | ||
| 375 | 476 | assert "/tmp/penguins.html" in second._header_renderable().plain |
| 376 | 477 | |
| 377 | 478 | |
| 479 | +@pytest.mark.asyncio | |
| 480 | +async def test_loader_app_renders_plain_response_without_markup_parsing() -> None: | |
| 481 | + app = LoaderApp(shell_owner=_FakeShellOwner()) | |
| 482 | + | |
| 483 | + async with app.run_test() as pilot: | |
| 484 | + app.post_message( | |
| 485 | + ResponseComplete( | |
| 486 | + content=( | |
| 487 | + "patch(file_path=\"animals/index.html\", hunks=" | |
| 488 | + f"{_raw_patch_tool_args()['hunks']})" | |
| 489 | + ) | |
| 490 | + ) | |
| 491 | + ) | |
| 492 | + await pilot.pause() | |
| 493 | + | |
| 494 | + message_area = app.query_one("#message-area") | |
| 495 | + last_widget = list(message_area.children)[-1] | |
| 496 | + rendered = _render_text(last_widget.render(), width=120) | |
| 497 | + assert "patch(file_path=" in rendered | |
| 498 | + assert "hunks=" in rendered | |
| 499 | + | |
| 500 | + | |
| 378 | 501 | def test_cli_parse_local_bash_commands_supports_slash_aliases() -> None: |
| 379 | 502 | assert cli_main_module._parse_local_bash_command("/jobs 5") == ("bash_jobs", {"limit": 5}) |
| 380 | 503 | assert cli_main_module._parse_local_bash_command("/wait bash-7 2.5") == ( |