-
Notifications
You must be signed in to change notification settings - Fork 927
fix(tui): prevent viewport jump when thinking finalizes above viewport #1141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
6e658e0
284f22c
2bcf0fb
8930933
27bab7f
8e9d3c2
5265028
c2f01c5
bb81d31
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Fix viewport jumps when thinking output finalizes above the visible transcript. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,7 @@ export class StreamingUIController { | |
| private _thinkingDraft = ''; | ||
| private _streamingBlock: { component: AssistantMessageComponent; entry: TranscriptEntry } | null = null; | ||
| private _activeThinkingComponent: ThinkingComponent | undefined = undefined; | ||
| private _pendingThinkingCompact = false; | ||
| private _activeCompactionBlock: CompactionComponent | undefined = undefined; | ||
| private _activeToolCalls = new Map<string, ToolCallBlockData>(); | ||
| private _streamingToolCallArguments = new Map< | ||
|
|
@@ -315,6 +316,7 @@ export class StreamingUIController { | |
| existingComponent.updateToolCall(toolCall); | ||
| } else if (existing === undefined) { | ||
| this.finalizeLiveTextBuffers('tool'); | ||
| this.compactPendingThinking(); | ||
| if (toolCall.name !== 'Agent' && toolCall.name !== 'AgentSwarm') { | ||
| this.onToolCallStart(toolCall); | ||
| } | ||
|
|
@@ -522,6 +524,7 @@ export class StreamingUIController { | |
| resetLiveText(): void { | ||
| this.pendingAssistantFlush = false; | ||
| this.pendingThinkingFlush = false; | ||
| this._pendingThinkingCompact = false; | ||
| this.clearFlushTimerIfIdle(); | ||
| this._assistantDraft = ''; | ||
| this._streamingBlock = null; | ||
|
|
@@ -555,6 +558,10 @@ export class StreamingUIController { | |
| const completedTurnKey = | ||
| this._currentTurnId ?? `local:${String(state.appState.streamingStartTime)}`; | ||
| this.finalizeLiveTextBuffers('idle'); | ||
| // After finalizeLiveTextBuffers, onThinkingEnd may have set | ||
| // _pendingThinkingCompact. Compact now so the thinking block | ||
| // reaches its final compact form before the turn ends. | ||
| this.compactPendingThinking(); | ||
| this.resetToolCallState(); | ||
| this._currentTurnId = undefined; | ||
|
|
||
|
|
@@ -580,6 +587,34 @@ export class StreamingUIController { | |
| // Live Render Hooks | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| /** | ||
| * Compact a stable-mode thinking component to its minimal finalized form. | ||
| * | ||
| * Called after the first visible assistant text update so that the thinking | ||
| * line-count reduction and the assistant content addition happen in the | ||
| * same pi-tui render cycle. The assistant content growing below offsets | ||
| * the destructive fullRender, making the transition invisible. | ||
| * | ||
| * Also called as a fallback in `finalizeTurn()` for the edge case where | ||
| * no assistant text follows the thinking block. | ||
| */ | ||
| compactPendingThinking(): void { | ||
| if (!this._pendingThinkingCompact) return; | ||
| this._pendingThinkingCompact = false; | ||
| // Walk in reverse to find the most recent stable-mode ThinkingComponent, | ||
| // not an older one from a previous turn. | ||
| const children = this.host.state.transcriptContainer.children; | ||
| for (let i = children.length - 1; i >= 0; i--) { | ||
| const child = children[i]; | ||
| if (child instanceof ThinkingComponent) { | ||
| if ((child as ThinkingComponent).compact()) { | ||
| this.host.state.ui.requestRender(); | ||
| } | ||
| break; | ||
|
Comment on lines
+609
to
+613
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
| } | ||
|
|
||
| onStreamingTextStart(): void { | ||
| const { state } = this.host; | ||
| this._pendingAgentGroup = null; | ||
|
|
@@ -601,8 +636,12 @@ export class StreamingUIController { | |
| onStreamingTextUpdate(fullText: string): void { | ||
| const block = this._streamingBlock; | ||
| if (block !== null) { | ||
| const hasVisibleAssistantText = fullText.trim().length > 0; | ||
| block.entry.content = fullText; | ||
| block.component.updateContent(fullText, { transient: true }); | ||
| if (hasVisibleAssistantText) { | ||
| this.compactPendingThinking(); | ||
| } | ||
| this.host.state.ui.requestRender(); | ||
| } | ||
| } | ||
|
|
@@ -637,7 +676,11 @@ export class StreamingUIController { | |
|
|
||
| onThinkingEnd(): void { | ||
| if (this._activeThinkingComponent === undefined) return; | ||
| // Enter stable mode: spinner stops but rendered line count stays | ||
| // 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a thinking block is finalized, this flag now defers the actual compacting, but several existing finalizers never drain it. For example, Useful? React with 👍 / 👎. |
||
| this._activeThinkingComponent = undefined; | ||
| this.host.state.ui.requestRender(); | ||
| this.host.mergeCurrentTurnSteps(); | ||
|
|
@@ -764,6 +807,7 @@ export class StreamingUIController { | |
| if (this._thinkingDraft.length > 0 || this._streamingBlock !== null) { | ||
| this.finalizeLiveTextBuffers('tool'); | ||
| } | ||
| this.compactPendingThinking(); | ||
|
|
||
| const existingComponent = this._pendingToolComponents.get(id); | ||
| if (existingComponent !== undefined) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a persisted assistant record has thinking plus a whitespace-only text part, this
elsebranch is skipped, andonStreamingTextUpdate()also refuses to compact becausefullText.trim()is empty. If that replayed message is followed by tool calls,renderToolCalls()adds the tool card without compacting, so resumed transcripts keep the thinking block in stablethoughtmode with the live tail preview and no expand footer, unlike the live tool-boundary path that compacts before rendering tools.Useful? React with 👍 / 👎.