Skip to content

Conversation

@roblourens
Copy link
Member

Fix an issue where only the first token of thinking text would be rendered in a collapsed thinking block

Copilot AI review requested due to automatic review settings January 24, 2026 00:24
@roblourens roblourens enabled auto-merge (squash) January 24, 2026 00:24
@roblourens roblourens self-assigned this Jan 24, 2026
@vs-code-engineering vs-code-engineering bot added this to the January 2026 milestone Jan 24, 2026
@justschen
Copy link
Collaborator

Screenshot 2026-01-23 at 4 32 43 PM

we all agree this is a great PR thanks @roblourens 😆

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes collapsed/lazy “thinking” rendering so that streaming updates are reflected correctly when the user expands the thinking block.

Changes:

  • Update ChatThinkingContentPart to keep pending lazy thinking items up-to-date during streaming and avoid duplicate/stale containers on expand.
  • Add tests covering lazy thinking + streaming updates and a non-streaming edge case.
  • Remove an unused ESLint disable directive from the test file.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatThinkingContentPart.test.ts Adds regression tests for lazy thinking behavior (streaming and non-streaming) and removes an unused ESLint disable.
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts Updates lazy-thinking materialization and streaming update propagation to prevent stale/duplicate thinking content after expansion.

Comment on lines +254 to +258
// Only create textContainer here if there's no pending lazy thinking item.
// If there's a lazy thinking item, it will be rendered via materializeLazyItem
// with the latest streaming content.
const hasLazyThinkingItems = this.lazyItems.some(item => item.kind === 'thinking');
if (this.currentThinkingValue && !hasLazyThinkingItems) {
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initContent skips creating/rendering the initial textContainer whenever any lazy thinking item exists. This can drop the constructor’s initial thinking text when another thinking section is queued via setupThinkingContainer before the part is expanded (initial content non-empty + later lazy thinking item). Consider preserving the initial thinking content (e.g. store it as a lazy thinking item when collapsed, or only skip when that initial content is already represented by a pending lazy item).

Suggested change
// Only create textContainer here if there's no pending lazy thinking item.
// If there's a lazy thinking item, it will be rendered via materializeLazyItem
// with the latest streaming content.
const hasLazyThinkingItems = this.lazyItems.some(item => item.kind === 'thinking');
if (this.currentThinkingValue && !hasLazyThinkingItems) {
// Render any initial thinking content captured before expansion.
if (this.currentThinkingValue) {

Copilot uses AI. Check for mistakes.
@roblourens
Copy link
Member Author

And you were all wrong
image

@roblourens roblourens merged commit 9900e97 into main Jan 24, 2026
22 checks passed
@roblourens roblourens deleted the roblou/progressive-prawn branch January 24, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants