fix(tui-v2): repair NameError on exit-boundary replay in _on_stream - #757
Open
Kailigithub wants to merge 1 commit into
Open
fix(tui-v2): repair NameError on exit-boundary replay in _on_stream#757Kailigithub wants to merge 1 commit into
Kailigithub wants to merge 1 commit into
Conversation
# 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
frontends/tuiapp_v2.py:6594references a localrefresh_chromethat the surrounding function_on_streamnever receives:The function only takes
(agent_id, task_id, text, done);refresh_chromeis a parameter of_update_assistantand is not in scope here.The branch fires whenever exit-boundary replay starts a follow-up task before the original display queue emits its final
done— exactly the spinner-forever scenario the comment block warns about. The originalif refresh_chrome:guard would always evaluateFalse(orNameError), so the sidebar/topbar refresh is silently lost in the replay path.The regular
done=Truepath a few lines below (line ~6603) calls_update_assistant(..., refresh_chrome=True), so the fix mirrors that.Repro
The included
test_issue_6594_tui_v2_stale_replay_nameerror.pyexercises the replay path without needing a live Textual app:The test fails on the parent commit with the exact
NameError: name refresh_chrome is not definedand passes with this fix.Verification
python3 -m ruff check frontends/tuiapp_v2.py --select E9,F63,F7,F82is clean (8 prior F821 warnings reduced to 0; the 7List-under-from __future__ import annotationsF821s were false positives that pyflakes also flagged — fixed by addingListto the existingfrom typing importline sotyping.get_type_hintson the affected@dataclassand helper functions resolves cleanly).python3 -m compileallis clean across the whole repo.git diff --checkis clean.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 evaluatedFalseor raisedNameError.The same fix incidentally resolves a closely-related F821 noise class in this file:
from __future__ import annotationsis in effect, so the 7List[...]annotations are stored as strings and do not fail at runtime.@dataclass RenderRow, however, introspects annotations viatyping.get_type_hintsat class creation, andrender_treereturn annotation needsListto be in scope if any caller does the same. Adding the import is a 1-line change that matches the codebase other typing-aware modules.Files
frontends/tuiapp_v2.py— 1 logic line + 1 import line (8 insertions, 4 deletions)test_issue_6594_tui_v2_stale_replay_nameerror.py— new regression test (127 lines)