chat rendering fix for working + cli specific issues (#301015)

* chat rendering fix for working + cli specific issues

* address comments
This commit is contained in:
Justin Chen
2026-03-11 23:39:46 -07:00
committed by GitHub
parent cabe52dcbe
commit 9c8a42f3ec
7 changed files with 52 additions and 9 deletions
@@ -164,7 +164,7 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen
private toolInvocations: (IChatToolInvocation | IChatToolInvocationSerialized)[] = [];
private allThinkingParts: IChatThinkingPart[] = [];
private hookCount: number = 0;
private singleItemInfo: { element: HTMLElement; originalParent: HTMLElement; originalNextSibling: Node | null } | undefined;
private singleItemInfo: { element: HTMLElement; originalParent: HTMLElement; originalNextSibling: Node | null; toolInvocation?: IChatToolInvocation | IChatToolInvocationSerialized } | undefined;
private lazyItems: ILazyItem[] = [];
private hasExpandedOnce: boolean = false;
private workingSpinnerElement: HTMLElement | undefined;
@@ -754,6 +754,11 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen
this.workingSpinnerElement = undefined;
this.workingSpinnerLabel = undefined;
}
// Clear the attached-to-thinking flag on all tool invocations
for (const toolInvocation of this.toolInvocations) {
toolInvocation.isAttachedToThinking = false;
}
}
public finalizeTitleIfDefault(): void {
@@ -804,23 +809,27 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen
}
// case where we only have one item (tool or edit) in the thinking container and no thinking parts, we want to move it back to its original position
if (this.appendedItemCount === 1 && this.currentThinkingValue.trim() === '') {
if (this.toolInvocationCount === 1 && this.hookCount === 0 && this.currentThinkingValue.trim() === '') {
// If singleItemInfo wasn't set (item was lazy/deferred), materialize it now
if (!this.singleItemInfo) {
const lazyItem = this.lazyItems.find(item => item.kind === 'tool' && item.originalParent);
if (lazyItem && lazyItem.kind === 'tool') {
const toolInvocation = lazyItem.toolInvocationOrMarkdown && (lazyItem.toolInvocationOrMarkdown.kind === 'toolInvocation' || lazyItem.toolInvocationOrMarkdown.kind === 'toolInvocationSerialized') ? lazyItem.toolInvocationOrMarkdown : undefined;
const result = lazyItem.lazy.value;
this.singleItemInfo = {
element: result.domNode,
originalParent: lazyItem.originalParent!,
originalNextSibling: this.domNode
originalNextSibling: this.domNode,
toolInvocation
};
if (result.disposable) {
this._register(result.disposable);
}
}
}
if (this.singleItemInfo && this.restoreSingleItemToOriginalPosition()) {
// Only restore if the tool is complete so the progress spinner is resolved
const toolIsComplete = !this.singleItemInfo?.toolInvocation || IChatToolInvocation.isComplete(this.singleItemInfo.toolInvocation);
if (toolIsComplete && this.singleItemInfo && this.restoreSingleItemToOriginalPosition()) {
return;
}
}
@@ -1027,7 +1036,7 @@ ${this.hookCount > 0 ? `EXAMPLES WITH BLOCKED CONTENT (from hooks):
return false;
}
const { element, originalParent, originalNextSibling } = this.singleItemInfo;
const { element, originalParent, originalNextSibling, toolInvocation } = this.singleItemInfo;
// don't restore it to original position - it contains multiple rendered elements
if (element.childElementCount > 1) {
@@ -1041,6 +1050,10 @@ ${this.hookCount > 0 ? `EXAMPLES WITH BLOCKED CONTENT (from hooks):
originalParent.appendChild(element);
}
if (toolInvocation) {
toolInvocation.isAttachedToThinking = false;
}
hide(this.domNode);
this.singleItemInfo = undefined;
return true;
@@ -1138,6 +1151,11 @@ ${this.hookCount > 0 ? `EXAMPLES WITH BLOCKED CONTENT (from hooks):
this.toolInvocationCount--;
}
// Clear the attached-to-thinking flag on the removed tool invocation
if (removedItem.kind === 'tool' && removedItem.toolInvocationOrMarkdown && (removedItem.toolInvocationOrMarkdown.kind === 'toolInvocation' || removedItem.toolInvocationOrMarkdown.kind === 'toolInvocationSerialized')) {
removedItem.toolInvocationOrMarkdown.isAttachedToThinking = false;
}
const toolInvocationsIndex = this.toolInvocations.findIndex(t =>
(t.kind === 'toolInvocation' || t.kind === 'toolInvocationSerialized') && t.toolId === toolInvocationId
);
@@ -1198,6 +1216,10 @@ ${this.hookCount > 0 ? `EXAMPLES WITH BLOCKED CONTENT (from hooks):
item.toolInvocationOrMarkdown.toolCallId === toolCallId
);
if (lazyIndex !== -1) {
const removedLazyItem = this.lazyItems[lazyIndex];
if (removedLazyItem.kind === 'tool' && removedLazyItem.toolInvocationOrMarkdown && (removedLazyItem.toolInvocationOrMarkdown.kind === 'toolInvocation' || removedLazyItem.toolInvocationOrMarkdown.kind === 'toolInvocationSerialized')) {
removedLazyItem.toolInvocationOrMarkdown.isAttachedToThinking = false;
}
this.lazyItems.splice(lazyIndex, 1);
}
@@ -1214,7 +1236,6 @@ ${this.hookCount > 0 ? `EXAMPLES WITH BLOCKED CONTENT (from hooks):
if (titleIndex !== -1) {
this.extractedTitles.splice(titleIndex, 1);
}
this.updateDropdownClickability();
this._onDidChangeHeight.fire();
}
@@ -1373,11 +1394,13 @@ ${this.hookCount > 0 ? `EXAMPLES WITH BLOCKED CONTENT (from hooks):
}
// Save the first item info for potential restoration later
if (this.appendedItemCount === 1 && originalParent) {
if (this.toolInvocationCount === 1 && this.hookCount === 0 && originalParent) {
const toolInvocation = toolInvocationOrMarkdown && (toolInvocationOrMarkdown.kind === 'toolInvocation' || toolInvocationOrMarkdown.kind === 'toolInvocationSerialized') ? toolInvocationOrMarkdown : undefined;
this.singleItemInfo = {
element: content,
originalParent,
originalNextSibling: this.domNode
originalNextSibling: this.domNode,
toolInvocation
};
} else {
this.singleItemInfo = undefined;
@@ -1030,6 +1030,18 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
return false;
}
// Never show working when the last part is a tool invocation that is attached to thinking,
// or *will be* attached to thinking during the upcoming render pass
if (lastPart && (lastPart.kind === 'toolInvocation' || lastPart.kind === 'toolInvocationSerialized')) {
if (lastPart.isAttachedToThinking) {
return false;
}
const collapsedToolsMode = this.configService.getValue<CollapsedToolsDisplayMode>('chat.agent.thinking.collapsedTools');
if (collapsedToolsMode !== CollapsedToolsDisplayMode.Off && this.shouldPinPart(lastPart, isResponseVM(element) ? element : undefined)) {
return false;
}
}
const hasRenderedThinkingPart = (templateData.renderedParts ?? []).some(part => part instanceof ChatThinkingContentPart);
@@ -1970,6 +1982,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
if (thinkingPart instanceof ChatThinkingContentPart) {
// Append using factory - thinking part decides whether to render lazily
toolInvocation.isAttachedToThinking = true;
thinkingPart.appendItem(createToolPart, toolInvocation.toolId, toolInvocation, templateData.value);
this.setupConfirmationTransitionWatcher(toolInvocation, thinkingPart, () => lazilyCreatedPart, createToolPart, context, templateData);
}
@@ -1980,6 +1993,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
if (this.shouldPinPart(toolInvocation, context.element)) {
if (lastThinking && toolInvocation.presentation !== 'hidden') {
// Append using factory - thinking part decides whether to render lazily
toolInvocation.isAttachedToThinking = true;
lastThinking.appendItem(createToolPart, toolInvocation.toolId, toolInvocation, templateData.value);
this.setupConfirmationTransitionWatcher(toolInvocation, lastThinking, () => lazilyCreatedPart, createToolPart, context, templateData);
return this.renderNoContent((other, followingContent, element) => lazilyCreatedPart ?
@@ -2019,6 +2033,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
const removeConfirmationWidget = () => {
const createdPart = getCreatedPart();
// move the created part out of thinking and into the main template
toolInvocation.isAttachedToThinking = false;
if (createdPart?.domNode) {
const wrapper = createdPart.domNode.parentElement;
if (wrapper?.classList.contains('chat-thinking-tool-wrapper')) {
@@ -2407,7 +2407,7 @@ have to be updated for changes to the rules above, or to support more deeply nes
display: flex;
align-items: center;
gap: 4px;
margin: 0 0 6px 0;
margin: 0 0 14px 0;
font-size: 13px;
/* Tool calls transition from a progress to a collapsible list part, which needs to have this top padding.
@@ -605,6 +605,7 @@ export interface IChatToolInvocation {
readonly subAgentInvocationId?: string;
readonly state: IObservable<IChatToolInvocation.State>;
generatedTitle?: string;
isAttachedToThinking: boolean;
kind: 'toolInvocation';
@@ -868,6 +869,7 @@ export interface IChatToolInvocationSerialized {
source: ToolDataSource | undefined; // undefined on pre-1.104 versions
readonly subAgentInvocationId?: string;
generatedTitle?: string;
isAttachedToThinking?: boolean;
kind: 'toolInvocationSerialized';
}
@@ -32,6 +32,7 @@ export class ChatToolInvocation implements IChatToolInvocation {
public parameters: unknown;
public generatedTitle?: string;
public readonly chatRequestId?: string;
public isAttachedToThinking: boolean = false;
public toolSpecificData?: IChatTerminalToolInvocationData | IChatToolInputInvocationData | IChatExtensionsContent | IChatTodoListContent | IChatSubagentToolInvocationData | IChatSimpleToolInvocationData | IChatModifiedFilesConfirmationData;
@@ -32,6 +32,7 @@ function makeToolInvocationPart(options: {
toolCallId: 'call-1',
state: observableValue('toolState', options.state),
toolSpecificData: options.toolSpecificData,
isAttachedToThinking: false,
toJSON: () => undefined!,
};
}
@@ -151,6 +151,7 @@ suite('ChatSubagentContentPart', () => {
toolCallId: toolCallId,
subAgentInvocationId: options.subAgentInvocationId,
state: observableValue('state', stateValue),
isAttachedToThinking: false,
kind: 'toolInvocation',
toJSON: () => createMockSerializedToolInvocation({
toolId: options.toolId ?? RunSubagentTool.Id,