diff --git a/code_puppy/messaging/rich_renderer.py b/code_puppy/messaging/rich_renderer.py index 6a2fc295..697e633f 100644 --- a/code_puppy/messaging/rich_renderer.py +++ b/code_puppy/messaging/rich_renderer.py @@ -98,6 +98,17 @@ async def stop(self) -> None: "context": "dim", } +GROUPABLE_TYPES = frozenset( + { + FileContentMessage, + GrepResultMessage, + DiffMessage, + FileListingMessage, + ShellStartMessage, + } +) +TRANSPARENT_TYPES = frozenset({SpinnerControl}) + # ============================================================================= # Rich Console Renderer @@ -111,6 +122,12 @@ class RichConsoleRenderer: It uses a background thread for synchronous compatibility with the main loop. """ + # Message types that support consecutive grouping under a single banner + _GROUPABLE_TYPES = GROUPABLE_TYPES + + # Message types that are "transparent" - they don't break an active group + _TRANSPARENT_TYPES = TRANSPARENT_TYPES + def __init__( self, bus: MessageBus, @@ -132,6 +149,8 @@ def __init__( self._running = False self._thread: Optional[threading.Thread] = None self._spinners: Dict[str, object] = {} # spinner_id -> status context + # Grouping: track last rendered message type for consecutive grouping + self._last_rendered_type: Optional[type] = None @property def console(self) -> Console: @@ -172,6 +191,16 @@ def _should_suppress_subagent_output(self) -> bool: """ return is_subagent() and not get_subagent_verbose() + def _is_continuation(self, msg_type: type) -> bool: + """Check if this message should be rendered as a grouped child (no banner). + + Returns True if the previous rendered message was the same groupable type, + meaning the banner was already printed and we just need the child line. + """ + return ( + msg_type in self._GROUPABLE_TYPES and self._last_rendered_type == msg_type + ) + # ========================================================================= # Lifecycle (Synchronous - for compatibility with main.py) # ========================================================================= @@ -226,9 +255,12 @@ def _render_sync(self, message: AnyMessage) -> None: """Render a message synchronously with error handling.""" try: self._do_render(message) + # Track type for grouping (transparent types don't break groups) + msg_type = type(message) + if msg_type not in self._TRANSPARENT_TYPES: + self._last_rendered_type = msg_type except Exception as e: - # Don't let rendering errors crash the loop - # Escape the error message to prevent nested markup errors + self._last_rendered_type = None safe_error = escape_rich_markup(str(e)) self._console.print(f"[dim red]Render error: {safe_error}[/dim red]") @@ -327,8 +359,8 @@ async def render(self, message: AnyMessage) -> None: elif isinstance(message, SelectionRequest): await self._render_selection_request(message) else: - # Use sync render for everything else - self._do_render(message) + # Use sync renderer for shared rendering and grouping behavior + self._render_sync(message) # ========================================================================= # Text Messages @@ -383,12 +415,13 @@ def _render_file_listing(self, msg: FileListingMessage) -> None: import os from collections import defaultdict - # Header on single line rec_flag = f"(recursive={msg.recursive})" - banner = self._format_banner("directory_listing", "DIRECTORY LISTING") + if not self._is_continuation(FileListingMessage): + banner = self._format_banner("directory_listing", "DIRECTORY LISTING") + self._console.print(f"\n{banner}") + self._console.print( - f"\n{banner} " - f"📂 [bold cyan]{msg.directory}[/bold cyan] [dim]{rec_flag}[/dim]\n" + f" ├─ 📂 [bold cyan]{msg.directory}[/bold cyan] [dim]{rec_flag}[/dim]\n" ) # Build a tree structure: {parent_path: {files: [], dirs: set(), size: int}} @@ -498,11 +531,7 @@ def get_recursive_file_count(d: str) -> int: ) def _render_file_content(self, msg: FileContentMessage) -> None: - """Render a file read - just show the header, not the content. - - The file content is for the LLM only, not for display in the UI. - """ - # Skip for sub-agents unless verbose mode + """Render a file read - just show the header, not the content.""" if self._should_suppress_subagent_output(): return @@ -512,11 +541,13 @@ def _render_file_content(self, msg: FileContentMessage) -> None: end_line = msg.start_line + msg.num_lines - 1 line_info = f" [dim](lines {msg.start_line}-{end_line})[/dim]" - # Just print the header - content is for LLM only - banner = self._format_banner("read_file", "READ FILE") - self._console.print( - f"\n{banner} 📂 [bold cyan]{msg.path}[/bold cyan]{line_info}" - ) + # Print banner only if this is NOT a continuation of the same type + if not self._is_continuation(FileContentMessage): + banner = self._format_banner("read_file", "READ FILE") + self._console.print(f"\n{banner}") + + # Always print as tree child + self._console.print(f" ├─ 📂 [bold cyan]{msg.path}[/bold cyan]{line_info}") def _render_grep_result(self, msg: GrepResultMessage) -> None: """Render grep results grouped by file matching old format.""" @@ -526,15 +557,17 @@ def _render_grep_result(self, msg: GrepResultMessage) -> None: import re - # Header - banner = self._format_banner("grep", "GREP") + if not self._is_continuation(GrepResultMessage): + banner = self._format_banner("grep", "GREP") + self._console.print(f"\n{banner}") + self._console.print( - f"\n{banner} 📂 [dim]{msg.directory} for '{msg.search_term}'[/dim]" + f" ├─ 📂 [dim]{msg.directory} for '{msg.search_term}'[/dim]" ) if not msg.matches: self._console.print( - f"[dim]No matches found for '{msg.search_term}' " + f" │ [dim]No matches found for '{msg.search_term}' " f"in {msg.directory}[/dim]" ) return @@ -551,7 +584,7 @@ def _render_grep_result(self, msg: GrepResultMessage) -> None: file_matches = by_file[file_path] match_word = "match" if len(file_matches) == 1 else "matches" self._console.print( - f"\n[dim]📄 {file_path} ({len(file_matches)} {match_word})[/dim]" + f" │ [dim]📄 {file_path} ({len(file_matches)} {match_word})[/dim]" ) # Show each match with line number and content @@ -577,15 +610,16 @@ def _render_grep_result(self, msg: GrepResultMessage) -> None: highlighted_line = line ln = match.line_number - self._console.print(f" [dim]{ln:4d}[/dim] │ {highlighted_line}") + self._console.print( + f" │ [dim]{ln:4d}[/dim] │ {highlighted_line}" + ) else: # Concise mode (default): Show only file summaries - self._console.print("") for file_path in sorted(by_file.keys()): file_matches = by_file[file_path] match_word = "match" if len(file_matches) == 1 else "matches" self._console.print( - f"[dim]📄 {file_path} ({len(file_matches)} {match_word})[/dim]" + f" │ [dim]📄 {file_path} ({len(file_matches)} {match_word})[/dim]" ) # Summary - subtle @@ -593,13 +627,10 @@ def _render_grep_result(self, msg: GrepResultMessage) -> None: file_word = "file" if len(by_file) == 1 else "files" num_files = len(by_file) self._console.print( - f"[dim]Found {msg.total_matches} {match_word} " + f" │ [dim]Found {msg.total_matches} {match_word} " f"across {num_files} {file_word}[/dim]" ) - # Trailing newline for spinner separation - self._console.print() - # ========================================================================= # Diff # ========================================================================= @@ -616,11 +647,12 @@ def _render_diff(self, msg: DiffMessage) -> None: icon = op_icons.get(msg.operation, "📄") op_color = op_colors.get(msg.operation, "white") - # Header on single line - banner = self._format_banner("edit_file", "EDIT FILE") + if not self._is_continuation(DiffMessage): + banner = self._format_banner("edit_file", "EDIT FILE") + self._console.print(f"\n{banner}") + self._console.print( - f"\n{banner} " - f"{icon} [{op_color}]{msg.operation.upper()}[/{op_color}] " + f" ├─ {icon} [{op_color}]{msg.operation.upper()}[/{op_color}] " f"[bold cyan]{msg.path}[/bold cyan]" ) @@ -654,33 +686,33 @@ def _render_diff(self, msg: DiffMessage) -> None: def _render_shell_start(self, msg: ShellStartMessage) -> None: """Render shell command start notification.""" - # Skip for sub-agents unless verbose mode if self._should_suppress_subagent_output(): return - # Escape command to prevent Rich markup injection safe_command = escape_rich_markup(msg.command) - # Header showing command is starting - banner = self._format_banner("shell_command", "SHELL COMMAND") - # Add background indicator if running in background mode + if not self._is_continuation(ShellStartMessage): + banner = self._format_banner("shell_command", "SHELL COMMAND") + self._console.print(f"\n{banner}") + + # Tree child with command if msg.background: self._console.print( - f"\n{banner} 🚀 [dim]$ {safe_command}[/dim] [bold magenta][BACKGROUND 🌙][/bold magenta]" + f" ├─ 🚀 [dim]$ {safe_command}[/dim] [bold magenta][BACKGROUND 🌙][/bold magenta]" ) else: - self._console.print(f"\n{banner} 🚀 [dim]$ {safe_command}[/dim]") + self._console.print(f" ├─ 🚀 [dim]$ {safe_command}[/dim]") # Show working directory if specified if msg.cwd: safe_cwd = escape_rich_markup(msg.cwd) - self._console.print(f"[dim]📂 Working directory: {safe_cwd}[/dim]") + self._console.print(f" │ [dim]📂 Working directory: {safe_cwd}[/dim]") # Show timeout or background status if msg.background: - self._console.print("[dim]⏱ Runs detached (no timeout)[/dim]") + self._console.print(" │ [dim]⏱ Runs detached (no timeout)[/dim]") else: - self._console.print(f"[dim]⏱ Timeout: {msg.timeout}s[/dim]") + self._console.print(f" │ [dim]⏱ Timeout: {msg.timeout}s[/dim]") def _render_shell_line(self, msg: ShellLineMessage) -> None: """Render shell output line preserving ANSI codes and carriage returns.""" diff --git a/tests/test_rich_renderer.py b/tests/test_rich_renderer.py index 0ddbb37b..5afc57b7 100644 --- a/tests/test_rich_renderer.py +++ b/tests/test_rich_renderer.py @@ -444,3 +444,299 @@ def test_start_stop_and_consume_buffer() -> None: renderer.stop() assert renderer._running is False + + +def test_consecutive_file_content_groups_under_single_banner() -> None: + """Multiple consecutive FileContentMessage should share one banner.""" + renderer, console = _make_renderer() + renderer._get_banner_color = Mock(return_value="blue") + + msgs = [ + FileContentMessage( + path=f"/src/file{i}.ts", + content=f"content{i}", + start_line=i * 50 + 1, + num_lines=50, + total_lines=100, + num_tokens=10, + ) + for i in range(3) + ] + + # Render all three via _render_sync so grouping state is tracked + for msg in msgs: + renderer._render_sync(msg) + + # Collect all printed strings + printed = [ + call.args[0] + for call in console.print.call_args_list + if call.args and isinstance(call.args[0], str) + ] + + # Should have exactly ONE banner line (containing "READ FILE") + banner_lines = [p for p in printed if "READ FILE" in p] + assert len(banner_lines) == 1, ( + f"Expected 1 banner, got {len(banner_lines)}: {banner_lines}" + ) + + # Should have exactly THREE tree-child lines (containing "├─") + tree_lines = [p for p in printed if "├─" in p] + assert len(tree_lines) == 3, ( + f"Expected 3 tree lines, got {len(tree_lines)}: {tree_lines}" + ) + + # Each tree line should contain the file path + for i in range(3): + assert any(f"file{i}.ts" in line for line in tree_lines), ( + f"file{i}.ts not found in tree lines" + ) + + +def test_single_file_content_still_has_banner_and_tree() -> None: + """A single FileContentMessage should still get banner + tree child.""" + renderer, console = _make_renderer() + renderer._get_banner_color = Mock(return_value="blue") + + msg = FileContentMessage( + path="/src/app.py", + content="hello", + start_line=None, + num_lines=None, + total_lines=10, + num_tokens=5, + ) + + renderer._render_sync(msg) + + printed = [ + call.args[0] + for call in console.print.call_args_list + if call.args and isinstance(call.args[0], str) + ] + + # Should have banner + assert any("READ FILE" in p for p in printed), "Missing READ FILE banner" + + # Should have tree child + assert any("├─" in p and "app.py" in p for p in printed), "Missing tree child line" + + +def test_different_type_breaks_grouping() -> None: + """A different message type between same types should reset grouping.""" + renderer, console = _make_renderer() + renderer._get_banner_color = Mock(return_value="blue") + + msg1 = FileContentMessage( + path="/src/a.py", + content="a", + start_line=None, + num_lines=None, + total_lines=1, + num_tokens=1, + ) + interruption = TextMessage(level=MessageLevel.INFO, text="some info") + msg2 = FileContentMessage( + path="/src/b.py", + content="b", + start_line=None, + num_lines=None, + total_lines=1, + num_tokens=1, + ) + + renderer._render_sync(msg1) + renderer._render_sync(interruption) + renderer._render_sync(msg2) + + printed = [ + call.args[0] + for call in console.print.call_args_list + if call.args and isinstance(call.args[0], str) + ] + + # Should have TWO banners (grouping was broken by TextMessage) + banner_lines = [p for p in printed if "READ FILE" in p] + assert len(banner_lines) == 2, ( + f"Expected 2 banners, got {len(banner_lines)}: {banner_lines}" + ) + + +def test_spinner_does_not_break_grouping() -> None: + """SpinnerControl is transparent and should not break grouping.""" + renderer, console = _make_renderer() + renderer._get_banner_color = Mock(return_value="blue") + + msg1 = FileContentMessage( + path="/src/a.py", + content="a", + start_line=None, + num_lines=None, + total_lines=1, + num_tokens=1, + ) + spinner = SpinnerControl(action="start", spinner_id="s1", text="loading") + msg2 = FileContentMessage( + path="/src/b.py", + content="b", + start_line=None, + num_lines=None, + total_lines=1, + num_tokens=1, + ) + + renderer._render_sync(msg1) + renderer._render_sync(spinner) + renderer._render_sync(msg2) + + printed = [ + call.args[0] + for call in console.print.call_args_list + if call.args and isinstance(call.args[0], str) + ] + + # Should have only ONE banner (spinner didn't break the group) + banner_lines = [p for p in printed if "READ FILE" in p] + assert len(banner_lines) == 1, ( + f"Expected 1 banner, got {len(banner_lines)}: {banner_lines}" + ) + + # Should have TWO tree-child lines + tree_lines = [p for p in printed if "├─" in p] + assert len(tree_lines) == 2, ( + f"Expected 2 tree lines, got {len(tree_lines)}: {tree_lines}" + ) + + +def test_consecutive_diff_groups() -> None: + """Multiple consecutive DiffMessage should share one banner.""" + renderer, console = _make_renderer() + renderer._get_banner_color = Mock(return_value="blue") + + msgs = [ + DiffMessage( + path=f"/src/file{i}.py", + operation="modify", + diff_lines=[DiffLine(line_number=1, type="add", content=f"line{i}")], + ) + for i in range(3) + ] + + for msg in msgs: + renderer._render_sync(msg) + + printed = [ + call.args[0] + for call in console.print.call_args_list + if call.args and isinstance(call.args[0], str) + ] + + banner_lines = [p for p in printed if "EDIT FILE" in p] + assert len(banner_lines) == 1, ( + f"Expected 1 banner, got {len(banner_lines)}: {banner_lines}" + ) + + tree_lines = [p for p in printed if "├─" in p] + assert len(tree_lines) == 3, ( + f"Expected 3 tree lines, got {len(tree_lines)}: {tree_lines}" + ) + + +def test_consecutive_shell_start_groups() -> None: + """Multiple consecutive ShellStartMessage should share one banner.""" + renderer, console = _make_renderer() + renderer._get_banner_color = Mock(return_value="blue") + + msgs = [ + ShellStartMessage(command=f"echo {i}", cwd=None, timeout=30, background=False) + for i in range(3) + ] + + for msg in msgs: + renderer._render_sync(msg) + + printed = [ + call.args[0] + for call in console.print.call_args_list + if call.args and isinstance(call.args[0], str) + ] + + banner_lines = [p for p in printed if "SHELL COMMAND" in p] + assert len(banner_lines) == 1, ( + f"Expected 1 banner, got {len(banner_lines)}: {banner_lines}" + ) + + tree_lines = [p for p in printed if "├─" in p] + assert len(tree_lines) == 3, ( + f"Expected 3 tree lines, got {len(tree_lines)}: {tree_lines}" + ) + + +def test_grouping_resets_across_different_groupable_types() -> None: + """Switching between different groupable types should print new banners.""" + renderer, console = _make_renderer() + renderer._get_banner_color = Mock(return_value="blue") + + file_msg = FileContentMessage( + path="/src/a.py", + content="a", + start_line=None, + num_lines=None, + total_lines=1, + num_tokens=1, + ) + grep_msg = GrepResultMessage( + search_term="needle", + directory="/src", + matches=[], + total_matches=0, + files_searched=1, + ) + + renderer._render_sync(file_msg) + renderer._render_sync(grep_msg) + + printed = [ + call.args[0] + for call in console.print.call_args_list + if call.args and isinstance(call.args[0], str) + ] + + # Should have both banners + assert any("READ FILE" in p for p in printed), "Missing READ FILE banner" + assert any("GREP" in p for p in printed), "Missing GREP banner" + + +@pytest.mark.asyncio +async def test_async_render_grouping_resets_across_different_groupable_types() -> None: + """Async render path should also reset grouping when type changes.""" + renderer, console = _make_renderer() + renderer._get_banner_color = Mock(return_value="blue") + + file_msg = FileContentMessage( + path="/src/a.py", + content="a", + start_line=None, + num_lines=None, + total_lines=1, + num_tokens=1, + ) + grep_msg = GrepResultMessage( + search_term="needle", + directory="/src", + matches=[], + total_matches=0, + files_searched=1, + ) + + await renderer.render(file_msg) + await renderer.render(grep_msg) + + printed = [ + call.args[0] + for call in console.print.call_args_list + if call.args and isinstance(call.args[0], str) + ] + + assert any("READ FILE" in p for p in printed), "Missing READ FILE banner" + assert any("GREP" in p for p in printed), "Missing GREP banner"