fix(tui): anchor offscreen expansion replay - #714
Open
devkade wants to merge 1 commit into
Open
Conversation
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.
Summary
Fix incomplete and mismatched tool output when expanding collapsed results with
Ctrl+O.When multiple completed tool calls were rendered in collapsed form, expanding them could leave some entries showing only headers or error summaries while another entry displayed only a trailing portion of its output.
This happens when an offscreen expansion changes the structure above the current viewport. The renderer's cached line state and the terminal's actual scrollback can then become inconsistent.
This PR:
Root cause
Ctrl+Ochanges the height of tool-rendering components that may already be above the current viewport.Rows that have already entered terminal scrollback cannot be edited in place. The renderer therefore needs to either:
Previously, the replay path could begin from the renderer's calculated hardware-cursor position rather than the physical top of the screen. The replay could therefore be offset from the expected terminal position and produce mixed output such as:
The renderer's internal state could appear correct even though the terminal contained mismatched rows.
What changed
The scrollback replay path now explicitly anchors at:
before writing the canonical transcript.
This makes replay independent of the cursor position used by the previous differential-rendering frame.
Multiplexer and Herdr sessions continue to use bounded viewport repaint rather than canonical scrollback replay. Those environments own their pane history, so replaying the entire transcript would append duplicate content and corrupt the user's pane scrollback.
Known limitation
For regular, non-multiplexer terminals, canonical replay may clear the terminal scrollback before rewriting the transcript:
As a result, expanding an offscreen tool result can remove content that the user had previously scrolled up to inspect. This may include:
A regular terminal does not provide a reliable way for the application to rewrite arbitrary historical rows or restore the previous native scroll position after the layout changes above the viewport.
This means there is a broader design tradeoff that should be decided explicitly.
Two possible policies for regular terminals
Option 1: Preserve native scrollback where possible
Senpi can avoid clearing the terminal scrollback and replay or repaint the current viewport without destructive history clearing.
Advantages:
Risks:
Option 2: Follow Pi's full-clear and redraw approach
Pi handles this structural-rendering problem in regular/main-screen mode by clearing the terminal scrollback and redrawing the current canonical transcript.
Advantages:
Risks:
Relevant Pi implementation and discussion:
TuiMainScreenPi's rationale is that a regular terminal cannot set the native scroll position after content above the viewport changes. Clearing and rebuilding the transcript avoids leaving stale collapsed rows mixed with newly expanded content.
Question for review
Which behavior should Senpi prefer for regular terminals?
My current understanding is:
This PR fixes the incorrect and mismatched expansion rendering without silently hiding that tradeoff. Reviewer guidance on the preferred long-term policy would be appreciated.
Multiplexer and Herdr behavior remains separate: those environments own pane scrollback and should continue using bounded viewport repaint rather than destructive replay.
Verification
npm --prefix packages/tui testnpm run checkCtrl+OFixes #701
Summary by cubic
Anchors scrollback replay to the terminal’s top-left to fix mixed or truncated tool output when expanding offscreen results with Ctrl+O, while keeping multiplexer/Herdr on bounded viewport repaint. Includes regression tests; fixes #701.
CSI 1;1Hinstead of a cached hardware-cursor row.Written for commit ae42a11. Summary will update on new commits.