diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts index d695b515e88..986cbc83271 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts @@ -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; diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index 15f811225e7..0c1769e1f70 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -1030,6 +1030,18 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer('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 lazilyCreatedPart, createToolPart, context, templateData); } @@ -1980,6 +1993,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer lazilyCreatedPart, createToolPart, context, templateData); return this.renderNoContent((other, followingContent, element) => lazilyCreatedPart ? @@ -2019,6 +2033,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer { 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')) { diff --git a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css index cfbf53ea6d8..1278d6c1e79 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css +++ b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css @@ -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. diff --git a/src/vs/workbench/contrib/chat/common/chatService/chatService.ts b/src/vs/workbench/contrib/chat/common/chatService/chatService.ts index a083e6212a5..9b89136688c 100644 --- a/src/vs/workbench/contrib/chat/common/chatService/chatService.ts +++ b/src/vs/workbench/contrib/chat/common/chatService/chatService.ts @@ -605,6 +605,7 @@ export interface IChatToolInvocation { readonly subAgentInvocationId?: string; readonly state: IObservable; 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'; } diff --git a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts index 6537d9a1eac..f128efd7e89 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts @@ -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; diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentSessionApprovalModel.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentSessionApprovalModel.test.ts index 321bac437d2..de54c6b1471 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentSessionApprovalModel.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentSessionApprovalModel.test.ts @@ -32,6 +32,7 @@ function makeToolInvocationPart(options: { toolCallId: 'call-1', state: observableValue('toolState', options.state), toolSpecificData: options.toolSpecificData, + isAttachedToThinking: false, toJSON: () => undefined!, }; } diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatSubagentContentPart.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatSubagentContentPart.test.ts index 5dc3df666c6..f141f1a1830 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatSubagentContentPart.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatSubagentContentPart.test.ts @@ -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,