diff --git a/openhands_cli/tui/widgets/richlog_visualizer.py b/openhands_cli/tui/widgets/richlog_visualizer.py index 322aacec..6a97be9d 100644 --- a/openhands_cli/tui/widgets/richlog_visualizer.py +++ b/openhands_cli/tui/widgets/richlog_visualizer.py @@ -5,10 +5,11 @@ import re import threading +from dataclasses import dataclass, field from typing import TYPE_CHECKING from rich.text import Text -from textual.widgets import Markdown +from textual.widgets import Markdown, Static from openhands.sdk.conversation.visualizer.base import ConversationVisualizerBase from openhands.sdk.event import ( @@ -48,11 +49,22 @@ # Maximum line length for truncating titles/commands in collapsed view MAX_LINE_LENGTH = 70 ELLIPSIS = "..." +DEFAULT_MAX_RENDERED_EVENTS = 250 # Default agent name - don't show prefix for this agent DEFAULT_AGENT_NAME = "OpenHands Agent" +@dataclass +class _RenderCapState: + max_rendered_events: int = DEFAULT_MAX_RENDERED_EVENTS + hidden_events_count: int = 0 + hidden_events_indicator: object | None = None + pending_actions: dict[str, tuple[ActionEvent, "Collapsible"]] = field( + default_factory=dict + ) + + if TYPE_CHECKING: from textual.containers import VerticalScroll from textual.widget import Widget @@ -100,6 +112,7 @@ def __init__( container: "VerticalScroll", app: "OpenHandsApp", name: str | None = None, + _render_cap_state: _RenderCapState | None = None, ) -> None: """Initialize the visualizer. @@ -113,12 +126,41 @@ def __init__( self._container = container self._app = app self._name = name + self._render_cap_state = ( + _render_cap_state if _render_cap_state is not None else _RenderCapState() + ) # Store the main thread ID for thread safety checks self._main_thread_id = threading.get_ident() # Cache CLI settings to avoid repeated file system reads self._cli_settings: CliSettings | None = None - # Track pending actions by tool_call_id for action-observation pairing - self._pending_actions: dict[str, tuple[ActionEvent, Collapsible]] = {} + + @property + def _max_rendered_events(self) -> int: + return self._render_cap_state.max_rendered_events + + @_max_rendered_events.setter + def _max_rendered_events(self, value: int) -> None: + self._render_cap_state.max_rendered_events = value + + @property + def _hidden_events_count(self) -> int: + return self._render_cap_state.hidden_events_count + + @_hidden_events_count.setter + def _hidden_events_count(self, value: int) -> None: + self._render_cap_state.hidden_events_count = value + + @property + def _hidden_events_indicator(self) -> object | None: + return self._render_cap_state.hidden_events_indicator + + @_hidden_events_indicator.setter + def _hidden_events_indicator(self, value: object | None) -> None: + self._render_cap_state.hidden_events_indicator = value + + @property + def _pending_actions(self) -> dict[str, tuple[ActionEvent, Collapsible]]: + return self._render_cap_state.pending_actions @property def cli_settings(self) -> CliSettings: @@ -147,6 +189,7 @@ def create_sub_visualizer(self, agent_id: str) -> "ConversationVisualizer": container=self._container, app=self._app, name=agent_id, + _render_cap_state=self._render_cap_state, ) @staticmethod @@ -302,9 +345,78 @@ def on_event(self, event: Event) -> None: def _add_widget_to_ui(self, widget: "Widget") -> None: """Add a widget to the UI (must be called from main thread).""" self._container.mount(widget) + self._prune_rendered_widgets() if self._container.is_vertical_scroll_end: self._container.scroll_end(animate=False) + def _is_hidden_events_indicator(self, widget: "Widget") -> bool: + """Check whether this widget is the hidden-events indicator.""" + try: + return widget.has_class("older-events-indicator") + except Exception: + return False + + def _remove_hidden_events_indicator(self) -> None: + """Remove the hidden-events indicator widget if present.""" + indicator = self._hidden_events_indicator + self._hidden_events_indicator = None + if indicator is None: + return + if indicator in list(self._container.children): + indicator.remove() + + def _ensure_hidden_events_indicator(self) -> None: + """Create/update the hidden-events indicator and pin it to the top.""" + if self._hidden_events_count <= 0: + self._remove_hidden_events_indicator() + return + + label = f"… {self._hidden_events_count} older events hidden" + if self._hidden_events_indicator is None: + self._hidden_events_indicator = Static( + label, classes="older-events-indicator" + ) + self._container.mount(self._hidden_events_indicator, before=0) + return + + if self._hidden_events_indicator not in list(self._container.children): + self._container.mount(self._hidden_events_indicator, before=0) + + self._hidden_events_indicator.update(label) + + def _remove_pending_action_if_widget_removed(self, widget: "Widget") -> None: + """Drop pending action state for widgets that are removed from the UI.""" + stale_actions = [ + tool_call_id + for tool_call_id, (_, pending_widget) in self._pending_actions.items() + if pending_widget is widget + ] + for tool_call_id in stale_actions: + self._pending_actions.pop(tool_call_id, None) + + def _prune_rendered_widgets(self) -> None: + """Cap number of rendered widgets and show hidden-events indicator.""" + if self._max_rendered_events <= 0: + self._hidden_events_count = 0 + self._remove_hidden_events_indicator() + return + + children = list(self._container.children) + visible_widgets = [ + w for w in children if not self._is_hidden_events_indicator(w) + ] + excess = len(visible_widgets) - self._max_rendered_events + if excess <= 0: + self._ensure_hidden_events_indicator() + return + + for widget in visible_widgets[:excess]: + widget.remove() + self._remove_pending_action_if_widget_removed(widget) + self._hidden_events_count += 1 + + self._ensure_hidden_events_indicator() + def _handle_critic_result(self, critic_result: "CriticResult") -> None: """Handle a critic result by displaying widgets and notifying controller. diff --git a/tests/tui/widgets/test_richlog_visualizer.py b/tests/tui/widgets/test_richlog_visualizer.py index 1630400a..d053275b 100644 --- a/tests/tui/widgets/test_richlog_visualizer.py +++ b/tests/tui/widgets/test_richlog_visualizer.py @@ -111,6 +111,61 @@ def create_terminal_action_event( ) +class _TestContainer: + """Simple container used to validate render tree trimming behavior.""" + + def __init__(self) -> None: + self.children: list[object] = [] + self.is_vertical_scroll_end = True + self._scroll_end_calls = 0 + + def mount(self, widget: object, before: int | None = None) -> None: + if before is None: + self.children.append(widget) + else: + self.children.insert(before, widget) + if hasattr(widget, "__dict__"): + setattr(widget, "_container", self) + + def scroll_end(self, animate: bool = False) -> None: + self._scroll_end_calls += 1 + + def _remove_child(self, widget: object) -> None: + if widget in self.children: + self.children.remove(widget) + + def query(self, _selector) -> list[object]: + return [] + + +class _StubWidget: + """Minimal renderable-like widget for container pruning tests.""" + + def __init__(self, text: str, classes: str = "") -> None: + self._text = text + self._classes = set(classes.split()) if classes else set() + self.removed = False + + @property + def classes(self) -> str: + return " ".join(sorted(self._classes)) + + def has_class(self, class_name: str) -> bool: + return class_name in self._classes + + def remove(self) -> None: + self.removed = True + container = getattr(self, "_container", None) + if container is not None: + container._remove_child(self) + + def update(self, text: str) -> None: + self._text = text + + def render(self) -> str: + return self._text + + class TestChineseCharacterMarkupHandling: """Tests for handling Chinese characters with special markup symbols.""" @@ -870,6 +925,73 @@ def test_create_sub_visualizer_shares_container_and_app(self, visualizer): assert sub_vis._container is visualizer._container assert sub_vis._app is visualizer._app + def test_create_sub_visualizer_shares_render_cap_state(self) -> None: + """Parent and sub-visualizer share cap state when using same container.""" + container = _TestContainer() + app = App() + parent = ConversationVisualizer( + container, + app, # type: ignore[arg-type] + name="parent_agent", + ) + child = parent.create_sub_visualizer("child_agent") + + parent._max_rendered_events = 3 + child._max_rendered_events = 3 + + parent._add_widget_to_ui(_StubWidget("parent-1")) + parent._add_widget_to_ui(_StubWidget("parent-2")) + child._add_widget_to_ui(_StubWidget("child-1")) + child._add_widget_to_ui(_StubWidget("child-2")) + child._add_widget_to_ui(_StubWidget("child-3")) + + rendered = [w for w in container.children if hasattr(w, "has_class")] + event_widgets = [ + w for w in rendered if not w.has_class("older-events-indicator") + ] + indicator_widgets = [ + w for w in rendered if w.has_class("older-events-indicator") + ] + + assert len(event_widgets) == 3 + assert len(indicator_widgets) == 1 + assert parent._hidden_events_count == 2 + assert child._hidden_events_count == 2 + assert child._render_cap_state is parent._render_cap_state + assert parent._hidden_events_indicator is child._hidden_events_indicator + + def test_shared_pending_actions_are_pruned_consistently(self) -> None: + """Parent and child share pending-action state when pruning shared widgets.""" + container = _TestContainer() + app = App() + parent = ConversationVisualizer( + container, + app, # type: ignore[arg-type] + name="parent_agent", + ) + child = parent.create_sub_visualizer("child_agent") + + action_event = create_terminal_action_event("echo shared action") + pending_widget = _StubWidget("parent-action") + parent._pending_actions[action_event.tool_call_id] = ( + action_event, + pending_widget, + ) + + parent._add_widget_to_ui(pending_widget) + assert child._pending_actions is parent._pending_actions + assert action_event.tool_call_id in parent._pending_actions + + parent._max_rendered_events = 1 + child._add_widget_to_ui(_StubWidget("child-event")) + + assert parent._max_rendered_events == 1 + assert len(container.children) == 2 + assert action_event.tool_call_id not in parent._pending_actions + assert action_event.tool_call_id not in child._pending_actions + assert pending_widget not in container.children + assert isinstance(container.children[0], Static) + class TestMessageEventDelegation: """Tests for MessageEvent handling in delegation context.""" @@ -1121,6 +1243,83 @@ def test_create_event_collapsible_uses_shared_fallback_titles( assert expected_title in str(collapsible.title) +class TestRenderedEventCap: + """Tests for keeping a bounded number of rendered event widgets.""" + + def test_rendered_event_cap_shows_hidden_indicator(self) -> None: + """When limit is exceeded, oldest widgets are removed and indicator is shown.""" + container = _TestContainer() + app = App() + visualizer = ConversationVisualizer(container, app) # type: ignore[arg-type] + visualizer._max_rendered_events = 3 + + for i in range(5): + visualizer._add_widget_to_ui(_StubWidget(f"event-{i}")) + + rendered = [w for w in container.children if hasattr(w, "has_class")] + event_widgets = [ + w for w in rendered if not w.has_class("older-events-indicator") + ] + indicator_widgets = [ + w for w in rendered if w.has_class("older-events-indicator") + ] + + assert len(event_widgets) == 3 + assert len(indicator_widgets) == 1 + assert visualizer._hidden_events_count == 2 + + event_texts = [str(widget.render()) for widget in event_widgets] + assert all("event-0" not in text for text in event_texts) + assert all("event-1" not in text for text in event_texts) + assert any("event-2" in text for text in event_texts) + assert any("event-3" in text for text in event_texts) + assert any("event-4" in text for text in event_texts) + + indicator = indicator_widgets[0] + assert str(indicator.render()).endswith("older events hidden") + assert "2" in str(indicator.render()) + assert container.children[0] is indicator + + def test_rendered_event_cap_no_trim_when_under_limit(self) -> None: + """Under the cap, all widgets are retained and no indicator is shown.""" + container = _TestContainer() + app = App() + visualizer = ConversationVisualizer(container, app) # type: ignore[arg-type] + visualizer._max_rendered_events = 10 + + for i in range(5): + visualizer._add_widget_to_ui(_StubWidget(f"event-{i}")) + + rendered = [w for w in container.children if hasattr(w, "has_class")] + event_widgets = [ + w for w in rendered if not w.has_class("older-events-indicator") + ] + + assert len(event_widgets) == 5 + assert visualizer._hidden_events_count == 0 + assert not any(w.has_class("older-events-indicator") for w in rendered) + + def test_rendered_event_indicator_updates_as_more_events_stream(self) -> None: + """Hidden-events indicator increments as more events exceed the cap.""" + container = _TestContainer() + app = App() + visualizer = ConversationVisualizer(container, app) # type: ignore[arg-type] + visualizer._max_rendered_events = 3 + + for i in range(6): + visualizer._add_widget_to_ui(_StubWidget(f"event-{i}")) + + rendered = [w for w in container.children if hasattr(w, "has_class")] + event_widgets = [ + w for w in rendered if not w.has_class("older-events-indicator") + ] + indicator = next(w for w in rendered if w.has_class("older-events-indicator")) + + assert len(event_widgets) == 3 + assert visualizer._hidden_events_count == 3 + assert "3" in str(indicator.render()) + + class TestAgentMessageEventDisplay: """Tests for agent MessageEvent display in non-delegation context.