fix(pi-tui): make the viewport anchor follow above-viewport content shifts#1364
fix(pi-tui): make the viewport anchor follow above-viewport content shifts#1364liruifengv wants to merge 1 commit into
Conversation
…hifts The anchor pins a buffer row index, but an above-viewport length change shifts the content living at every index below it. The pinned window then suddenly showed content further along (visible upward creep), and the rows that slid above the window top were lost: never committed to scrollback, which holds older bytes at those indices. During streaming, every above-viewport net shrink (a finished agent row collapsing, a merged step, a spinner line disappearing) permanently swallowed that many rows, and the blank area under the input box kept growing. doRender now scores two hypotheses for such frames — window stayed put vs window content shifted by the length delta — and when the shift explains the frame strictly better, moves the anchor with the content: a pure shift re-anchors with no painting at all (the screen already shows exactly that content), and a shift with local in-window changes (spinner/timer rows) repaints the window at the shifted anchor. Commit order stays continuous, so the exactly-once scrollback invariant is preserved: no loss, no duplication. Covered by e2e case07 (above-viewport shift), which fails on the previous revision; the stale-content unit test now asserts the follow-the-content window instead of the old swallowed-row behavior.
🦋 Changeset detectedLatest commit: 8d631b0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d631b063f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const lengthDelta = newLines.length - this.previousLines.length; | ||
| const shiftedTop = prevViewportTop + lengthDelta; | ||
| if (lengthDelta !== 0 && shiftedTop >= 0) { |
There was a problem hiding this comment.
When a single streaming frame both removes rows above the viewport and appends the same number of rows near the tail, the total lengthDelta is 0, so this guard skips the new shift-following path entirely. The later clamped diff then repaints the shifted tail in place without scrolling the old top viewport rows into scrollback, recreating the row-loss bug for frames where a status/activity block collapses while new output arrives. The shift amount needs to be derived from the above-viewport edit/alignment, not only from the net buffer length.
Useful? React with 👍 / 👎.
1 participant
Related Issue
No linked issue; follow-up to #1353, the problem is explained below.
Problem
The viewport anchor introduced in #1353 pins a buffer row index. But when content above the viewport changes length — a finished agent row collapsing, steps merging into a summary line, a spinner line disappearing — every line below the change shifts, while the pinned window keeps showing the same row indices. Two symptoms, observed live during long streaming turns:
Every above-viewport net shrink swallowed that many rows, repeatedly, for the whole turn (debug logs showed single frames shrinking by 9–20 lines).
What changed
doRendernow scores two hypotheses for a frame whose diff starts above the viewport and whose length changed, by counting mismatched window rows under each alignment:When the shift explains the frame strictly better, the anchor moves with the content:
Scrollback stays exactly-once: committed content ends precisely where the shifted anchor begins, so later growth commits rows in content order — no loss, no duplication. Frames better explained by the pinned hypothesis (appends, tail deletions, oscillation) take the existing paths unchanged.
Validation
case07-above-viewport-shift(in thee2e/rendering-bug ledger): the visible window must not move when above-viewport lines are removed, the shift must apply even when a live status row changes in the same frame, and later growth must commit each row exactly once. Fails on the previous revision, passes now.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.