fix(tui): make streamed assistant rendering redraw-safe via an active draft - #632
Open
rian-dolphin wants to merge 2 commits into
Open
fix(tui): make streamed assistant rendering redraw-safe via an active draft#632rian-dolphin wants to merge 2 commits into
rian-dolphin wants to merge 2 commits into
Conversation
Replace the split provisional state (assistant_buffer, thinking rows in TuiState.items, per-widget liveness pointers) with one explicit ActiveAssistantDraft holding the cumulative assistant snapshot and a monotonic stream id. TuiState.items now contains finalized history only, and focused lifecycle methods (begin/update/finish/discard/interrupt) own every draft transition. TranscriptView projects the draft through a pure ordered-block helper (interleaved thinking/text preserved, one placeholder per contiguous hidden thinking run, matching Pi) and owns all live tail widgets under one _ActiveAssistantRender. Full redraws reconstruct that ownership, so Ctrl+T, slash commands, theme changes, resizes, and other refreshes can no longer orphan or duplicate streamed blocks. Incremental updates flow through sync_active_assistant, which writes only missing suffixes via MarkdownStream, mounts only additively-new blocks, and rebuilds the tail for corrections or replacement streams. Completion is one idempotent finish_active_assistant that finalizes and rebinds owned widgets in place when topology matches; append_item is identity-idempotent and inserts new rows before the active tail. A render generation counter plus stream ids make stale async work abort instead of mounting or registering removed widgets. Cancellation and flush now retain the whole partial turn consistently (thinking and text alike), matching the aborted message the session records. Deterministic race tests gate MarkdownStream write/stop to interleave redraws at exact await points.
…m PR 631 Add two deterministic scenarios the earlier fix attempt covered: a thinking toggle processed while finalization is suspended on a gated MarkdownStream.stop, and an interrupted stream's projected rows surviving the next turn's completion.
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
1 participant
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
The TUI can show streamed thinking text or answer text two times. This occurs when the app rebuilds the transcript during a response. Ctrl+T, slash commands, a terminal resize, or a theme change can start a rebuild.
The old code kept one live turn in three places:
TuiState.assistant_buffer.TuiState.items.TranscriptView.A rebuild made the live thinking rows into ordinary history rows. It also removed the live widget pointers. The next delta then made a second live widget. Completion removed only one copy. The other copy stayed on the screen. The session file was always correct. Only the TUI display was incorrect.
Solution
This change uses the same ownership model as Pi.
TuiState.itemsnow holds only finalized history.ActiveAssistantDraftobject holds the full in-flight turn. It keeps the last cumulative message snapshot and a stream ID.MessageUpdateEvent.messageas the truth. It does not add deltas together._ActiveAssistantRenderobject owns all live tail widgets. A full redraw makes this owner again from the draft. A redraw can no longer make live rows into rows without an owner.sync_active_assistant()does the incremental updates. The usual path writes only the missing text suffix throughMarkdownStream. New blocks mount without a rebuild of the other blocks. A correction or a replacement stream causes a full rebuild of the tail.finish_active_assistant()completes the turn one time only. When the block layout agrees, it binds the live widgets to the canonical items in place. Unrelated history widgets keep their identity.Tests
MarkdownStream.write()andstop()withasyncio.Event. They then start a redraw at an exact await point.The performance behavior does not change. The usual path writes only a suffix for each delta. It does not parse the full markdown again. It does not remount unrelated history.
Validation
uv run pytest tests/test_tui_adapter.py tests/test_tui_app.pypasses (432 tests).uv run ruff check .anduv run mypy srcpass.mainalso, because of local environment tools.A manual TUI smoke test with slow streaming is still necessary before merge.
Documentation
See
dev-notes/tui-active-assistant-draft.md. It explains the draft model, the redraw behavior, and the map to Pi's design.Example of the problem
If I ran
/sessionor some other command while the model was thinking, it would duplicate the thinking block. I also had cases where the full output block was duplicated as well, but found that harder to reproduce. You can see below that the first thinking block is duplicated. After the fix, I couldn't reproduce that.