From d35c7474d189e3803c73f55c7e2a2a3cc6ffee22 Mon Sep 17 00:00:00 2001 From: Kailigithub Date: Tue, 11 Aug 2026 03:35:46 +0800 Subject: [PATCH] fix(tui-v2): repair NameError on exit-boundary replay in _on_stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Problem The stale-replay branch in `_on_stream` (frontends/tuiapp_v2.py:6594) referenced a local `refresh_chrome` that the function never receives: def _on_stream(self, agent_id, task_id, text, done): ... if s.current_task_id != task_id: ... if found and agent_id == self.current_id: if found._segment_widgets: try: self._stream_update_assistant(found) except Exception: self._refresh_messages() else: self._refresh_messages() if refresh_chrome: # <-- NameError here self._refresh_sidebar() self._refresh_topbar() self._ensure_spinner() The function only takes (agent_id, task_id, text, done); `refresh_chrome` is a parameter of `_update_assistant` and is not in scope. The regular `done=True` path a few lines below (line ~6603) calls `_update_assistant(..., refresh_chrome=True)`, so the fix mirrors that. # Repro This branch fires whenever exit-boundary replay starts a follow-up task before the original display queue emits its final `done` — i.e. the spinner-forever scenario the comment block warns about. Reached via the agent's exit-boundary replay path; the test in `test_issue_6594_tui_v2_stale_replay_nameerror.py` reproduces it without needing a live Textual app. # Verification - `python -m ruff check frontends/tuiapp_v2.py --select E9,F63,F7,F82` is clean (8 prior F821 warnings reduced to 0; the 7 `List`-under-future- annotations F821s were false positives that pyflakes also flagged — fixed by adding `List` to the existing `from typing import` line). - The new test fails on the parent commit with the exact `NameError: name 'refresh_chrome' is not defined` and passes with this fix. - `python -m compileall` is clean across the whole repo. # Scope One behaviour change: the replay branch now always refreshes the sidebar and topbar (matching the regular done path), instead of the `if refresh_chrome:` guard which always evaluated `False` *or* raised NameError. --- frontends/tuiapp_v2.py | 12 +- ...ssue_6594_tui_v2_stale_replay_nameerror.py | 127 ++++++++++++++++++ 2 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 test_issue_6594_tui_v2_stale_replay_nameerror.py diff --git a/frontends/tuiapp_v2.py b/frontends/tuiapp_v2.py index 8e37ec7f5..9bc5fc959 100644 --- a/frontends/tuiapp_v2.py +++ b/frontends/tuiapp_v2.py @@ -29,7 +29,7 @@ from keysym import fmt_key, fmt_keys # noqa: E402 from dataclasses import dataclass, field from itertools import count -from typing import Any, Callable, Optional +from typing import Any, Callable, List, Optional def _ensure_tui_deps() -> None: @@ -6591,9 +6591,13 @@ def _on_stream(self, agent_id, task_id, text, done): except Exception: self._refresh_messages() else: self._refresh_messages() - if refresh_chrome: - self._refresh_sidebar() - self._refresh_topbar() + # Mirror the regular done=True path below (line ~6603) which + # always refreshes sidebar+topbar on completion. The replay + # path used to reference an undefined `refresh_chrome` local + # (NameError on every exit-boundary replay); the agent is + # in fact done, so always refresh chrome here. + self._refresh_sidebar() + self._refresh_topbar() self._ensure_spinner() return s.buffer = text diff --git a/test_issue_6594_tui_v2_stale_replay_nameerror.py b/test_issue_6594_tui_v2_stale_replay_nameerror.py new file mode 100644 index 000000000..5d0ca979a --- /dev/null +++ b/test_issue_6594_tui_v2_stale_replay_nameerror.py @@ -0,0 +1,127 @@ +"""Regression test for frontends/tuiapp_v2.py:6594 NameError on the +exit-boundary replay path. + +Background +---------- +`_on_stream(self, agent_id, task_id, text, done)` is invoked from +`_consume_display_queue` (a worker thread) via `call_from_thread`. The +function does NOT accept a `refresh_chrome` parameter, but its stale-replay +branch (when `s.current_task_id != task_id`) referenced `refresh_chrome` — +triggering `NameError: name 'refresh_chrome' is not defined` whenever an +exit-boundary replay settled an old assistant message. + +The regular `done=True` path (line ~6603) handles the same situation by +always calling `_refresh_sidebar` + `_refresh_topbar`, so the fix mirrors +that. + +Repro +----- +Construct a minimal `GenericAgentTUI` instance with a fake session whose +`current_task_id` is newer than the incoming `task_id` and call +`_on_stream(... task_id=old, done=True)`. Before the fix this raises +`NameError`; after the fix it returns cleanly and calls the three refresh +methods. +""" +from __future__ import annotations + +import sys +import os +import importlib.util +import pathlib +import types +import unittest + +ROOT = pathlib.Path(__file__).resolve().parent +sys.path.insert(0, str(ROOT)) # for `keysym` +sys.path.insert(0, str(ROOT / "frontends")) # for `tuiapp_v2` + +# Pre-import the textual/rich/keysym deps so spec.loader.exec_module() doesn't +# fall into the auto-install fallback (it requires network + write perms). +import textual # noqa: F401 +import rich # noqa: F401 +import keysym # noqa: F401 + + +def _load_module(): + spec = importlib.util.spec_from_file_location( + "tuiapp_v2_under_test", ROOT / "frontends" / "tuiapp_v2.py" + ) + mod = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = mod + spec.loader.exec_module(mod) + return mod + + +class StaleReplayNameErrorRepro(unittest.TestCase): + def setUp(self) -> None: + self.m = _load_module() + self.app = self.m.GenericAgentTUI.__new__(self.m.GenericAgentTUI) + # Minimal session state for the stale-replay branch. + sess = types.SimpleNamespace() + sess.agent = None + sess.current_task_id = 99 # the new (replay owner's) task + sess.messages = [] + sess.status = "idle" + msg = types.SimpleNamespace( + role="assistant", + content="", + done=False, + task_id=42, # the OLD task being settled + _segment_widgets=None, + ) + sess.messages.append(msg) + self.app.sessions = {"s1": sess} + self.app.current_id = "s1" + + # Counters for each refresh callback. + self.calls = { + "stream_update": 0, + "refresh_messages": 0, + "refresh_sidebar": 0, + "refresh_topbar": 0, + "ensure_spinner": 0, + } + + def make(name): + def _cb(*_a, **_k): + self.calls[name] += 1 + _cb.__name__ = name + return _cb + + self.app._stream_update_assistant = make("stream_update") + self.app._refresh_messages = make("refresh_messages") + self.app._refresh_sidebar = make("refresh_sidebar") + self.app._refresh_topbar = make("refresh_topbar") + self.app._ensure_spinner = make("ensure_spinner") + + def test_replay_path_runs_without_name_error(self) -> None: + """Pre-fix this raises NameError; post-fix it returns cleanly.""" + try: + self.app._on_stream("s1", 42, "final text", done=True) + except NameError as exc: + self.fail(f"Replay path raised NameError: {exc}") + # Old assistant message must be marked done. + msg = self.app.sessions["s1"].messages[0] + self.assertTrue(msg.done, "old assistant message should be settled") + self.assertEqual(msg.content, "final text") + # Sidebar + topbar must refresh on settle, mirror the regular path. + self.assertEqual(self.calls["refresh_sidebar"], 1) + self.assertEqual(self.calls["refresh_topbar"], 1) + self.assertEqual(self.calls["ensure_spinner"], 1) + # messages refresh: 1 from the empty-_segment_widgets branch. + self.assertEqual(self.calls["refresh_messages"], 1) + # stream_update: not used (no widgets). + self.assertEqual(self.calls["stream_update"], 0) + + def test_done_false_is_a_noop(self) -> None: + """The replay path is gated on `done=True`; done=False must skip everything.""" + self.app._on_stream("s1", 42, "intermediate", done=False) + for k, v in self.calls.items(): + self.assertEqual(v, 0, f"{k} should be 0 on done=False, got {v}") + # The message is NOT marked done when done=False. + msg = self.app.sessions["s1"].messages[0] + self.assertFalse(msg.done) + + +if __name__ == "__main__": + unittest.main()