fix(tui): prevent viewport jump when thinking finalizes above viewport#1141
fix(tui): prevent viewport jump when thinking finalizes above viewport#1141starSumi wants to merge 6 commits into
Conversation
When the ThinkingComponent transitions from 'live' to 'finalized' mode while above the visible viewport, its rendered line count changes (4→3 lines). This triggers pi-tui's `firstChanged < prevViewportTop` branch, which falls back to a destructive fullRender (clear-screen), causing the terminal to jump to the top. Introduce a stable transition mechanism: - `finalize()` now enters 'stable mode' that keeps the same line count as live mode (spinner replaced by static bullet, same content region) - `compact()` defers the actual line-count reduction to a safe window - StreamingUIController calls compactThinkingIfPending() at the start of assistant text streaming, so both changes land in the same render cycle and the fullRender is masked by the new content below Fixes MoonshotAI#981 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 56662c0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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: 3cfd3a6df6
ℹ️ 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".
| if (child instanceof ThinkingComponent) { | ||
| if ((child as ThinkingComponent).compact()) { | ||
| this.host.state.ui.requestRender(); | ||
| } | ||
| break; |
There was a problem hiding this comment.
When transcriptContainer already contains an older ThinkingComponent (for example after a previous turn or replayed message), this loop stops on that older finalized block; compact() returns false, but _pendingThinkingCompact has already been cleared and the loop still breaks. The just-finalized stable-mode block later in the transcript is then never compacted, leaving it in the live-shaped thought view instead of the finalized preview.
Useful? React with 👍 / 👎.
| if (state.appState.streamingPhase === 'idle') return; | ||
| this.host.deferUserMessages = false; | ||
| // Last-chance compact in case no assistant text triggered it | ||
| this.compactThinkingIfPending(); |
There was a problem hiding this comment.
For a turn that ends while thinking is still active and no assistant text has started, _pendingThinkingCompact is still false at this point, so this call is a no-op; finalizeLiveTextBuffers() below then calls onThinkingEnd() and sets the pending flag after the last-chance compact has already passed. That leaves no-assistant thinking blocks in stable/live-shaped rendering after turn completion and leaks the pending compact into a later event.
Useful? React with 👍 / 👎.
Two bugs caught by Codex automated review: 1. compactThinkingIfPending() found the FIRST ThinkingComponent in transcript, which could be an older block from a previous turn. Now walks in reverse to find the most recent stable-mode instance. 2. Last-chance compaction in finalizeTurn() was called BEFORE finalizeLiveTextBuffers(), so _pendingThinkingCompact was still false when thinking was still active. Moved after finalizeLiveTextBuffers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add controller-level regressions for thinking-only turn compaction and latest pending thinking block selection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Follow-up pushed in
Validation:
Transport note: HTTPS and explicit SSH @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb139392d2
ℹ️ 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".
| // After finalizeLiveTextBuffers, onThinkingEnd may have set | ||
| // _pendingThinkingCompact. Compact now so the thinking block | ||
| // reaches its final compact form before the turn ends. | ||
| this.compactThinkingIfPending(); |
There was a problem hiding this comment.
When a turn goes from thinking.delta directly to tool.call.started or tool.call.delta, the tool-call paths first call finalizeLiveTextBuffers('tool'), which now sets _pendingThinkingCompact, but they never consume it before adding the tool UI. The first compaction then happens here in finalizeTurn() after the tool/result has already rendered, so the thinking block can shrink above the viewport with no newly-added content below and recreate the same scroll-jump this patch is trying to avoid. Please also compact the pending thinking block before rendering/flushing the tool component.
Useful? React with 👍 / 👎.
| // identical to live mode, preventing a destructive fullRender when | ||
| // this component is above the viewport (fixes #981). | ||
| this._activeThinkingComponent.finalize(); | ||
| this._pendingThinkingCompact = true; |
There was a problem hiding this comment.
onThinkingEnd() is also used by SessionReplayRenderer.flushAssistant(). For replayed assistant messages that contain only think parts, or think plus tool calls with no text, replay never calls onStreamingTextStart() or finalizeTurn(), so this pending flag is never consumed and the transcript keeps rendering the block in stable live-format (thought header with only the tail lines) instead of the normal compact finalized preview. Resume/replay should either compact immediately in that path or expose a finalized mode that does not leave pending live state behind.
Useful? React with 👍 / 👎.
|
Follow-up pushed in
Regression coverage added:
Validation:
Transport note: I first tried a normal push while my local branch still had the pre-API SHA for the earlier test-only follow-up. I fetched the fork head, rebased this commit onto the actual remote @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a92658eff5
ℹ️ 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".
| if (this._thinkingDraft.length > 0 || this._streamingBlock !== null) { | ||
| this.finalizeLiveTextBuffers('tool'); | ||
| this.compactPendingThinking(); |
There was a problem hiding this comment.
When a thinking block has already been finalized by another boundary (for example handleStepBegin() calls finalizeLiveTextBuffers('waiting')), _pendingThinkingCompact can be true while both _thinkingDraft and _streamingBlock are empty. If the next event is tool.call.delta, this condition is skipped, so the tool preview is added under the stable thought live-shape block and compaction is delayed until a later turn boundary, reintroducing the stale preview/viewport-jump risk for that sequence. Call compactPendingThinking() before rendering the preview regardless of whether there is current draft/assistant text.
Useful? React with 👍 / 👎.
|
Follow-up pushed in
Validation:
Transport note: the first HTTPS push picked up an invalid stored credential. Retrying with a one-shot @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56662c0a53
ℹ️ 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".
| // identical to live mode, preventing a destructive fullRender when | ||
| // this component is above the viewport (fixes #981). | ||
| this._activeThinkingComponent.finalize(); | ||
| this._pendingThinkingCompact = true; |
There was a problem hiding this comment.
When a thinking block is finalized, this flag now defers the actual compacting, but several existing finalizers never drain it. For example, handleHookResult() calls flushThinkingToTranscript() and then appends the hook-result assistant entry without calling compactPendingThinking(), and interruption/error/compaction paths also call finalizeLiveTextBuffers() without a later assistant/tool/finalizeTurn drain. In those flows a long thinking block is left indefinitely in stable live-shape (thought, tail lines, no expand hint, and Ctrl+O has no effect), whereas before it finalized into the normal compact/expandable transcript entry. Please compact in those terminal/non-tool paths before adding the next transcript/status entry or before returning to idle.
Useful? React with 👍 / 👎.
1 participant
Summary
When
ThinkingComponenttransitions from live to finalized above the viewport, its line count changes (4→3), triggering pi-tui's destructivefullRender(true)which clears the screen and jumps to the top.Fix: Stable transition mechanism —
finalize()keeps live-format line count,compact()defers the reduction to a safe render cycle when assistant content also changes.Root Cause
Solution
thinking.ts— Stable modefinalize()enters stable mode: stops spinner, keeps same line shape as livecompact()defers actual line-count reduction, returns booleanrender()usesmode === 'live' || stableModefor identical outputstreaming-ui.ts— Deferred compactiononThinkingEnd()→finalize()(stable, not compact)compactThinkingIfPending()called atonStreamingTextStart()— both changes in same render cyclefinalizeTurn(), cleanup inresetLiveText()thinking.test.ts— Updated + 3 new testsWhy it works
Testing
Related