diff --git a/clients/terminal/tui/settings.py b/clients/terminal/tui/settings.py index a7e4e7c..0b566c8 100644 --- a/clients/terminal/tui/settings.py +++ b/clients/terminal/tui/settings.py @@ -18,6 +18,11 @@ mouse: bool = True scroll_speed: int = 1 diff_style: str = "unified" + # Maximum number of chat items rendered into the conversation panel. Older + # items stay in the in-memory model (non-destructive) but are not laid out, + # so per-token refresh cost is bounded regardless of history length. The + # render cache makes even the visible window cheap to rebuild. + max_visible_items: int = 200 keybinds: dict[str, str] = field(default_factory=dict) def to_dict(self) -> dict[str, Any]: @@ -36,6 +41,11 @@ mouse=_coerce_bool(merged.get("mouse"), field_defaults["mouse"]), scroll_speed=_coerce_int(merged.get("scroll_speed"), field_defaults["scroll_speed"]), diff_style=_coerce_str(merged.get("diff_style"), field_defaults["diff_style"]), + max_visible_items=_coerce_int_bounded( + merged.get("max_visible_items"), + field_defaults["max_visible_items"], + minimum=10, + ), keybinds=dict(keybinds), ) @@ -116,6 +126,19 @@ return default +def _coerce_int_bounded(value: Any, default: int, minimum: int) -> int: + """Like _coerce_int but clamps the result to at least ``minimum``. + + A stored value below the floor (or a non-numeric one) falls back to + ``default`` so a bad config can never silently shrink the window to + something unusable. + """ + coerced = _coerce_int(value, default) + if coerced < minimum: + return default + return coerced + + def _coerce_str(value: Any, default: str) -> str: """Return a string value if it is a non-empty string, else default.""" if isinstance(value, str) and value: diff --git a/clients/terminal/tui/widgets/chat_panel.py b/clients/terminal/tui/widgets/chat_panel.py index f4b9b0a..3d53a38 100644 --- a/clients/terminal/tui/widgets/chat_panel.py +++ b/clients/terminal/tui/widgets/chat_panel.py @@ -2,13 +2,67 @@ from __future__ import annotations -from rich.console import Group +import json +from typing import Any + +from rich.console import Group, RenderableType +from rich.text import Text from textual.app import ComposeResult from textual.containers import ScrollableContainer from textual.widgets import Static -from clients.terminal.tui.chat_model import ChatModel +from clients.terminal.tui.chat_model import ChatItem, ChatModel from clients.terminal.tui.renderers import default_registry +from clients.terminal.tui.settings import get_tui_settings +from clients.terminal.tui.themes import get_active_theme + + +def _item_msg(item: ChatItem) -> dict[str, Any]: + """Map a ChatItem to the msg dict its renderer expects.""" + kind = item.kind + if kind == "user_message": + return {"type": "user_message", "content": item.content} + if kind == "assistant_message": + return {"type": "assistant_message", "content": item.content} + if kind == "thinking_block": + return { + "type": "thinking_block", + "content": item.content, + "is_subagent": item.meta.get("is_subagent", False), + } + if kind == "tool_started": + return {"type": "tool_started", **item.meta} + if kind == "tool_call": + return {"type": "tool_call", **item.meta} + if kind == "error": + return {"type": "error", "message": item.content} + if kind == "status": + return {"type": "status", "content": item.content} + if kind == "planning_status": + return { + "type": "planning_status", + "label": item.content, + "phase": item.meta.get("phase"), + "is_subagent": item.meta.get("is_subagent", False), + } + if kind == "plan_ready": + return { + "type": "plan_ready", + "plan": item.content, + "is_subagent": item.meta.get("is_subagent", False), + } + return {"type": "plain", "content": item.content} + + +def _signature(msg: dict[str, Any]) -> str: + """Stable serialized form of a renderer msg, for cache invalidation. + + Only an item whose rendered inputs changed should miss the cache; this + captures content + meta so in-place mutations (e.g. a streaming assistant + bubble growing token by token, or a rolling planning_status line) invalidate + exactly their own entry. + """ + return json.dumps(msg, sort_keys=True, default=str) class ChatPanel(ScrollableContainer): @@ -33,6 +87,11 @@ self._model = ChatModel() self._registry = default_registry() self._items_container = Static("") + # Renderable cache keyed by id(item) -> (signature, renderable). On a + # per-token stream_delta only the streaming item's signature changes, + # so every other visible item is reused from here instead of being + # rebuilt (and re-parsed as markdown) on every token. + self._chat_render_cache: dict[int, tuple[str, RenderableType]] = {} def compose(self) -> ComposeResult: yield self._items_container @@ -53,66 +112,48 @@ self._model.items.clear() self._model._current_assistant = None self._model._current_thinking = None + self._chat_render_cache.clear() self._refresh() + def _max_visible_items(self) -> int: + return get_tui_settings().max_visible_items + + def _truncation_hint(self, hidden: int) -> RenderableType: + theme = get_active_theme() + return Text( + f"… {hidden} earlier messages not shown", + style=theme.text.hex, + ) + def _refresh(self) -> None: - renderables = [] - for item in self._model.items: - if item.kind == "user_message": - renderables.append( - self._registry.render({"type": "user_message", "content": item.content}) - ) - elif item.kind == "assistant_message": - renderables.append( - self._registry.render({"type": "assistant_message", "content": item.content}) - ) - elif item.kind == "thinking_block": - renderables.append( - self._registry.render( - { - "type": "thinking_block", - "content": item.content, - "is_subagent": item.meta.get("is_subagent", False), - } - ) - ) - elif item.kind == "tool_started": - renderables.append(self._registry.render({"type": "tool_started", **item.meta})) - elif item.kind == "tool_call": - renderables.append(self._registry.render({"type": "tool_call", **item.meta})) - elif item.kind == "error": - renderables.append( - self._registry.render({"type": "error", "message": item.content}) - ) - elif item.kind == "status": - renderables.append( - self._registry.render({"type": "status", "content": item.content}) - ) - elif item.kind == "planning_status": - renderables.append( - self._registry.render( - { - "type": "planning_status", - "label": item.content, - "phase": item.meta.get("phase"), - "is_subagent": item.meta.get("is_subagent", False), - } - ) - ) - elif item.kind == "plan_ready": - renderables.append( - self._registry.render( - { - "type": "plan_ready", - "plan": item.content, - "is_subagent": item.meta.get("is_subagent", False), - } - ) - ) + items = self._model.items + limit = self._max_visible_items() + hidden = max(0, len(items) - limit) + # Slice the last `limit` items when over the cap; otherwise keep all. + visible = items[-limit:] if hidden else items + + renderables: list[RenderableType] = [] + if hidden: + renderables.append(self._truncation_hint(hidden)) + + seen_ids: set[int] = set() + for item in visible: + seen_ids.add(id(item)) + msg = _item_msg(item) + sig = _signature(msg) + cached = self._chat_render_cache.get(id(item)) + if cached is not None and cached[0] == sig: + renderables.append(cached[1]) else: - renderables.append( - self._registry.render({"type": "plain", "content": item.content}) - ) + rendered = self._registry.render(msg) + self._chat_render_cache[id(item)] = (sig, rendered) + renderables.append(rendered) + + # Drop cache entries for items no longer in the model so memory stays + # bounded by the live history, not by everything ever shown. + stale = [key for key in self._chat_render_cache if key not in seen_ids] + for key in stale: + self._chat_render_cache.pop(key, None) self._items_container.update(Group(*renderables)) - self.scroll_end(animate=False) + self.scroll_end(animate=False) \ No newline at end of file diff --git a/tests/clients/test_chat_panel.py b/tests/clients/test_chat_panel.py new file mode 100644 index 0000000..5557542 --- /dev/null +++ b/tests/clients/test_chat_panel.py @@ -0,0 +1,160 @@ +"""Tests for ChatPanel render caching and the visible-history window cap. + +The panel rebuilds every renderable on each WS event (including per-token +stream_delta). Caching makes only the changed item miss, and the window cap +bounds how many items are laid out regardless of total history length. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from clients.terminal.tui.tui_app import NaviCodeTui + + +@pytest.fixture(autouse=True) +def tmp_state_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + from clients.terminal import config + import clients.terminal.tui.settings as settings_module + + original = config.settings.state_dir + config.settings.state_dir = tmp_path + settings_module._tui_settings = None + yield tmp_path + config.settings.state_dir = original + settings_module._tui_settings = None + + +@pytest.fixture(autouse=True) +def mock_tui_api(monkeypatch: pytest.MonkeyPatch) -> None: + import clients.terminal.api as api_module + + monkeypatch.setattr(api_module, "create_session", lambda *a, **k: {"session_id": "s", "profile_id": "navi_code"}) + monkeypatch.setattr(api_module, "get_session", lambda sid: {"session_id": sid, "profile_id": "navi_code"}) + monkeypatch.setattr(api_module, "list_sessions", lambda: []) + monkeypatch.setattr(api_module, "get_profile_model", lambda pid: "configured-model") + + +def _capture_groups(chat) -> list: + """Record each Group passed to the items container's update().""" + captured: list = [] + original_update = chat._items_container.update + + def _record(group, *args, **kwargs): + captured.append(group) + return original_update(group, *args, **kwargs) + + chat._items_container.update = _record # type: ignore[method-assign] + return captured + + +def _wrap_render(chat) -> list: + """Wrap the panel's renderer registry.render to count calls.""" + calls: list[dict] = [] + original_render = chat._registry.render + + def _record(msg, *args, **kwargs): + calls.append(msg) + return original_render(msg, *args, **kwargs) + + chat._registry.render = _record # type: ignore[method-assign] + return calls + + +@pytest.mark.anyio +async def test_per_token_refresh_renders_only_the_streaming_item() -> None: + """A stream_delta changes only the assistant bubble's signature, so the + registry is called exactly once per token — every other visible item is + reused from the cache instead of being rebuilt/re-parsed.""" + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + chat = pilot.app.query_one("ChatPanel") + calls = _wrap_render(chat) + + chat.add_user_message("hi") # renders the user bubble once + chat.handle_ws_event({"type": "stream_start"}) + chat.handle_ws_event({"type": "stream_delta", "delta": "a"}) # creates + renders assistant + + baseline = len(calls) + # Two more tokens — only the streaming assistant item should be rebuilt. + chat.handle_ws_event({"type": "stream_delta", "delta": "b"}) + after_one = len(calls) + chat.handle_ws_event({"type": "stream_delta", "delta": "c"}) + after_two = len(calls) + + assert after_one - baseline == 1 + assert after_two - baseline == 2 + + +@pytest.mark.anyio +async def test_window_cap_renders_only_last_n_with_hint(monkeypatch) -> None: + """Beyond the cap, only the last N items are laid out and a hint is shown.""" + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + chat = pilot.app.query_one("ChatPanel") + monkeypatch.setattr(chat, "_max_visible_items", lambda: 3) + captured = _capture_groups(chat) + chat.clear() # drop the startup cwd/connected status items + + for i in range(5): + chat.add_user_message(f"msg {i}") + + group = captured[-1] + # 3 visible items + 1 truncation hint. + assert len(group.renderables) == 4 + hint = group.renderables[0] + assert "2 earlier messages not shown" in str(hint) + + +@pytest.mark.anyio +async def test_no_hint_when_within_limit(monkeypatch) -> None: + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + chat = pilot.app.query_one("ChatPanel") + monkeypatch.setattr(chat, "_max_visible_items", lambda: 10) + captured = _capture_groups(chat) + chat.clear() # drop the startup cwd/connected status items + + for i in range(3): + chat.add_user_message(f"msg {i}") + + group = captured[-1] + assert len(group.renderables) == 3 + assert "not shown" not in str(group.renderables[0]) + + +@pytest.mark.anyio +async def test_clear_empties_chat_render_cache() -> None: + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + chat = pilot.app.query_one("ChatPanel") + chat.add_user_message("hi") + chat.handle_ws_event({"type": "stream_start"}) + chat.handle_ws_event({"type": "stream_delta", "delta": "a"}) + assert chat._chat_render_cache # populated during refresh + + chat.clear() + assert chat._chat_render_cache == {} + assert chat._model.items == [] + + +@pytest.mark.anyio +async def test_removed_item_is_pruned_from_cache() -> None: + """stream_end purges empty assistant bubbles; their cache entries must go + too so the cache stays bounded by live items, not by everything shown.""" + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + chat = pilot.app.query_one("ChatPanel") + + chat.handle_ws_event({"type": "stream_start"}) + chat.handle_ws_event({"type": "stream_delta", "delta": ""}) # empty bubble + empty_ids = {id(it) for it in chat._model.items if it.kind == "assistant_message"} + assert empty_ids # the empty bubble exists before stream_end + + chat.handle_ws_event({"type": "stream_end", "content": ""}) + cached_ids = set(chat._chat_render_cache.keys()) + assert not (cached_ids & empty_ids) + # And the empty bubble is no longer in the model. + assert not any(it.kind == "assistant_message" for it in chat._model.items) \ No newline at end of file diff --git a/tests/clients/test_tui_settings.py b/tests/clients/test_tui_settings.py index 457bbfc..0f32ccb 100644 --- a/tests/clients/test_tui_settings.py +++ b/tests/clients/test_tui_settings.py @@ -36,6 +36,7 @@ assert s.mouse is True assert s.scroll_speed == 1 assert s.diff_style == "unified" + assert s.max_visible_items == 200 assert s.keybinds == {} @@ -112,3 +113,25 @@ assert loaded.scroll_speed == 1 assert loaded.theme == "gnexus-dark" assert loaded.diff_style == "unified" + + +def test_settings_coerces_numeric_max_visible_items(tmp_state_dir: Path) -> None: + data = {"max_visible_items": "150"} + (tmp_state_dir / "tui.json").write_text(json.dumps(data), encoding="utf-8") + loaded = TuiSettings().load() + assert loaded.max_visible_items == 150 + + +def test_settings_rejects_too_small_max_visible_items(tmp_state_dir: Path) -> None: + """A value below the floor falls back to the default, not the floor.""" + data = {"max_visible_items": 5} + (tmp_state_dir / "tui.json").write_text(json.dumps(data), encoding="utf-8") + loaded = TuiSettings().load() + assert loaded.max_visible_items == 200 + + +def test_settings_rejects_bad_max_visible_items(tmp_state_dir: Path) -> None: + data = {"max_visible_items": "all"} + (tmp_state_dir / "tui.json").write_text(json.dumps(data), encoding="utf-8") + loaded = TuiSettings().load() + assert loaded.max_visible_items == 200