From 56b2d5fcdeb4100d0d1667a2d793605cc18668d0 Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Thu, 8 Jan 2026 06:36:41 +0800 Subject: [PATCH] edit markdown parts in collapsible part (#286301) * edit markdown parts in collapsible part * remove whitespace * address some comments and remove a lot of unnecessary code * remove some unused functions * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * extra whitespace * Update src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../chatThinkingContentPart.ts | 76 +++++++---- .../media/chatThinkingContent.css | 16 +++ .../chat/browser/widget/chatListRenderer.ts | 120 +++++++++++------- .../contrib/chat/common/widget/annotations.ts | 4 + 4 files changed, 142 insertions(+), 74 deletions(-) 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 90b776bfee5..e1d2dfd7f24 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { $, clearNode, hide } from '../../../../../../base/browser/dom.js'; -import { IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized } from '../../../common/chatService/chatService.js'; +import { IChatMarkdownContent, IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized } from '../../../common/chatService/chatService.js'; import { IChatContentPartRenderContext, IChatContentPart } from './chatContentParts.js'; import { IChatRendererContent } from '../../../common/model/chatViewModel.js'; import { ChatConfiguration, ThinkingDisplayMode } from '../../../common/constants.js'; @@ -14,6 +14,8 @@ import { IConfigurationService } from '../../../../../../platform/configuration/ import { MarkdownString } from '../../../../../../base/common/htmlContent.js'; import { IRenderedMarkdown } from '../../../../../../base/browser/markdownRenderer.js'; import { IMarkdownRenderer } from '../../../../../../platform/markdown/browser/markdownRenderer.js'; +import { extractCodeblockUrisFromText } from '../../../common/widget/annotations.js'; +import { basename } from '../../../../../../base/common/resources.js'; import { ChatCollapsibleContentPart } from './chatCollapsibleContentPart.js'; import { localize } from '../../../../../../nls.js'; import { Codicon } from '../../../../../../base/common/codicons.js'; @@ -85,7 +87,7 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen private content: IChatThinkingPart; private currentThinkingValue: string; private currentTitle: string; - private defaultTitle = localize('chat.thinking.header', 'Thinking...'); + private defaultTitle = localize('chat.thinking.header', 'Working...'); private textContainer!: HTMLElement; private markdownResult: IRenderedMarkdown | undefined; private wrapper!: HTMLElement; @@ -93,10 +95,11 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen private lastExtractedTitle: string | undefined; private extractedTitles: string[] = []; private toolInvocationCount: number = 0; + private appendedItemCount: number = 0; private streamingCompleted: boolean = false; private isActive: boolean = true; private toolInvocations: (IChatToolInvocation | IChatToolInvocationSerialized)[] = []; - private singleToolItemInfo: { element: HTMLElement; originalParent: HTMLElement; originalNextSibling: Node | null } | undefined; + private singleItemInfo: { element: HTMLElement; originalParent: HTMLElement; originalNextSibling: Node | null } | undefined; constructor( content: IChatThinkingPart, @@ -110,7 +113,7 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen ) { const initialText = extractTextFromPart(content); const extractedTitle = extractTitleFromThinkingContent(initialText) - ?? 'Thinking...'; + ?? 'Working...'; super(extractedTitle, context, undefined, hoverService); @@ -358,9 +361,9 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen return; } - // case where we only have one tool in the thinking container and no thinking parts, we want to move it back to its original position - if (this.toolInvocationCount === 1 && this.currentThinkingValue.trim() === '' && this.singleToolItemInfo) { - this.restoreSingleToolToOriginalPosition(); + // 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() === '' && this.singleItemInfo) { + this.restoreSingleItemToOriginalPosition(); return; } @@ -455,12 +458,18 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen this.setFallbackTitle(); } - private restoreSingleToolToOriginalPosition(): void { - if (!this.singleToolItemInfo) { + private restoreSingleItemToOriginalPosition(): void { + if (!this.singleItemInfo) { return; } - const { element, originalParent, originalNextSibling } = this.singleToolItemInfo; + const { element, originalParent, originalNextSibling } = this.singleItemInfo; + + // don't restore it to original position - it contains multiple rendered elements + if (element.childElementCount > 1) { + this.singleItemInfo = undefined; + return; + } if (originalNextSibling && originalNextSibling.parentNode === originalParent) { originalParent.insertBefore(element, originalNextSibling); @@ -469,13 +478,13 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen } hide(this.domNode); - this.singleToolItemInfo = undefined; + this.singleItemInfo = undefined; } private setFallbackTitle(): void { const finalLabel = this.toolInvocationCount > 0 - ? localize('chat.thinking.finished.withTools', 'Finished thinking and invoked {0} tool{1}', this.toolInvocationCount, this.toolInvocationCount === 1 ? '' : 's') - : localize('chat.thinking.finished', 'Finished Thinking'); + ? localize('chat.thinking.finished.withTools', 'Finished working and invoked {0} tool{1}', this.toolInvocationCount, this.toolInvocationCount === 1 ? '' : 's') + : localize('chat.thinking.finished', 'Finished Working'); this.currentTitle = finalLabel; this.wrapper.classList.remove('chat-thinking-streaming'); @@ -489,20 +498,27 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen this.updateDropdownClickability(); } - public appendItem(content: HTMLElement, toolInvocationId?: string, toolInvocation?: IChatToolInvocation | IChatToolInvocationSerialized, originalParent?: HTMLElement): void { - // save the first tool item info for potential restoration later - if (this.toolInvocationCount === 0 && originalParent) { - this.singleToolItemInfo = { + public appendItem(content: HTMLElement, toolInvocationId?: string, toolInvocationOrMarkdown?: IChatToolInvocation | IChatToolInvocationSerialized | IChatMarkdownContent, originalParent?: HTMLElement): void { + if (!content.hasChildNodes() || content.textContent?.trim() === '') { + return; + } + + // save the first item info for potential restoration later + if (this.appendedItemCount === 0 && originalParent) { + this.singleItemInfo = { element: content, originalParent, originalNextSibling: this.domNode }; } else { - this.singleToolItemInfo = undefined; + this.singleItemInfo = undefined; } + this.appendedItemCount++; + const itemWrapper = $('.chat-thinking-tool-wrapper'); - const icon = toolInvocationId ? getToolInvocationIcon(toolInvocationId) : Codicon.tools; + const isMarkdownEdit = toolInvocationOrMarkdown?.kind === 'markdownContent'; + const icon = isMarkdownEdit ? Codicon.pencil : (toolInvocationId ? getToolInvocationIcon(toolInvocationId) : Codicon.tools); const iconElement = createThinkingIcon(icon); itemWrapper.appendChild(iconElement); itemWrapper.appendChild(content); @@ -512,15 +528,23 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen this.toolInvocationCount++; let toolCallLabel: string; - if (toolInvocation?.invocationMessage) { - const message = typeof toolInvocation.invocationMessage === 'string' ? toolInvocation.invocationMessage : toolInvocation.invocationMessage.value; + const isToolInvocation = toolInvocationOrMarkdown && (toolInvocationOrMarkdown.kind === 'toolInvocation' || toolInvocationOrMarkdown.kind === 'toolInvocationSerialized'); + if (isToolInvocation && toolInvocationOrMarkdown.invocationMessage) { + const message = typeof toolInvocationOrMarkdown.invocationMessage === 'string' ? toolInvocationOrMarkdown.invocationMessage : toolInvocationOrMarkdown.invocationMessage.value; toolCallLabel = message; + + this.toolInvocations.push(toolInvocationOrMarkdown); + } else if (toolInvocationOrMarkdown?.kind === 'markdownContent') { + const codeblockInfo = extractCodeblockUrisFromText(toolInvocationOrMarkdown.content.value); + if (codeblockInfo?.uri) { + const filename = basename(codeblockInfo.uri); + toolCallLabel = localize('chat.thinking.editedFile', 'Edited {0}', filename); + } else { + toolCallLabel = localize('chat.thinking.editingFile', 'Edited file'); + } } else { toolCallLabel = `Invoked \`${toolInvocationId}\``; } - if (toolInvocation) { - this.toolInvocations.push(toolInvocation); - } // Add tool call to extracted titles for LLM title generation if (!this.extractedTitles.includes(toolCallLabel)) { @@ -562,14 +586,14 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen this.currentTitle = title; return; } - const thinkingLabel = `Thinking: ${title}`; + const thinkingLabel = `Working: ${title}`; this.lastExtractedTitle = title; this.currentTitle = thinkingLabel; this.setTitleWithWidgets(new MarkdownString(thinkingLabel), this.instantiationService, this.chatMarkdownAnchorService, this.chatContentMarkdownRenderer); } hasSameContent(other: IChatRendererContent, _followingContent: IChatRendererContent[], _element: ChatTreeItem): boolean { - if (other.kind === 'toolInvocation' || other.kind === 'toolInvocationSerialized') { + if (other.kind === 'toolInvocation' || other.kind === 'toolInvocationSerialized' || other.kind === 'markdownContent') { return true; } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatThinkingContent.css b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatThinkingContent.css index f5c7919691a..9d05f50f96d 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatThinkingContent.css +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatThinkingContent.css @@ -67,6 +67,22 @@ } } + .chat-thinking-tool-wrapper .chat-markdown-part.rendered-markdown { + padding: 5px 12px 4px 20px; + + .status-icon.codicon-check { + display: none; + } + + .code:has(.chat-codeblock-pill-container) { + margin-bottom: 0px; + + .chat-codeblock-pill-container { + margin-bottom: 0px; + } + } + } + /* chain of thought lines */ .chat-thinking-tool-wrapper, .chat-thinking-item.markdown-content { diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index 6a41a8438ec..3766518105d 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -20,7 +20,7 @@ import { toErrorMessage } from '../../../../../base/common/errorMessage.js'; import { canceledName } from '../../../../../base/common/errors.js'; import { Emitter, Event } from '../../../../../base/common/event.js'; import { FuzzyScore } from '../../../../../base/common/filters.js'; -import { IMarkdownString, MarkdownString } from '../../../../../base/common/htmlContent.js'; +import { MarkdownString } from '../../../../../base/common/htmlContent.js'; import { Iterable } from '../../../../../base/common/iterator.js'; import { KeyCode } from '../../../../../base/common/keyCodes.js'; import { Disposable, DisposableStore, IDisposable, dispose, thenIfNotDisposed, toDisposable } from '../../../../../base/common/lifecycle.js'; @@ -47,7 +47,7 @@ import { IThemeService } from '../../../../../platform/theme/common/themeService import { IChatEntitlementService } from '../../../../services/chat/common/chatEntitlementService.js'; import { IWorkbenchIssueService } from '../../../issue/common/issue.js'; import { CodiconActionViewItem } from '../../../notebook/browser/view/cellParts/cellActionView.js'; -import { annotateSpecialMarkdownContent } from '../../common/widget/annotations.js'; +import { annotateSpecialMarkdownContent, hasCodeblockUriTag } from '../../common/widget/annotations.js'; import { checkModeOption } from '../../common/chat.js'; import { IChatAgentMetadata } from '../../common/participants/chatAgents.js'; import { ChatContextKeys } from '../../common/actions/chatContextKeys.js'; @@ -78,7 +78,7 @@ import { ChatElicitationContentPart } from './chatContentParts/chatElicitationCo import { ChatErrorConfirmationContentPart } from './chatContentParts/chatErrorConfirmationPart.js'; import { ChatErrorContentPart } from './chatContentParts/chatErrorContentPart.js'; import { ChatExtensionsContentPart } from './chatContentParts/chatExtensionsContentPart.js'; -import { ChatMarkdownContentPart } from './chatContentParts/chatMarkdownContentPart.js'; +import { ChatMarkdownContentPart, codeblockHasClosingBackticks } from './chatContentParts/chatMarkdownContentPart.js'; import { ChatMcpServersInteractionContentPart } from './chatContentParts/chatMcpServersInteractionContentPart.js'; import { ChatMultiDiffContentPart } from './chatContentParts/chatMultiDiffContentPart.js'; import { ChatProgressContentPart, ChatWorkingProgressContentPart } from './chatContentParts/chatProgressContentPart.js'; @@ -1045,7 +1045,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer('chat.agent.thinking.collapsedTools'); + // thinking and working content are always pinned (they are the thinking container itself) + if (part.kind === 'thinking' || part.kind === 'working') { + return true; + } + + // should not finalize thinking + if (part.kind === 'undoStop') { + return true; + } + if (collapsedToolsMode === CollapsedToolsDisplayMode.Off) { return false; } + // is an edit related part + if (this.hasCodeblockUri(part) || part.kind === 'textEditGroup') { + return true; + } + // Don't pin MCP tools const isMcpTool = (part.kind === 'toolInvocation' || part.kind === 'toolInvocationSerialized') && part.source.type === 'mcp'; if (isMcpTool) { @@ -1260,26 +1288,6 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer { - if (!value) { - return false; - } - const text = typeof value === 'string' ? value : value.value; - return text.toLowerCase().includes('create'); - }; - - if (containsCreate(content.invocationMessage) || containsCreate(content.pastTenseMessage)) { - return true; - } - - return content.toolId.toLowerCase().includes('create'); - } - private getLastThinkingPart(renderedParts: ReadonlyArray | undefined): ChatThinkingContentPart | undefined { if (!renderedParts || renderedParts.length === 0) { return undefined; @@ -1312,41 +1320,23 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer('chat.agent.thinking.collapsedTools'); // if we get an empty thinking part, mark thinking as finished - if (content.kind === 'thinking' && (Array.isArray(content.value) ? content.value.length === 0 : !content.value)) { + if (content.kind === 'thinking' && (Array.isArray(content.value) ? content.value.length === 0 : content.value === '')) { const lastThinking = this.getLastThinkingPart(templateData.renderedParts); lastThinking?.resetId(); return this.renderNoContent(other => content.kind === other.kind); } - const lastRenderedPart = context.preceedingContentParts.length ? context.preceedingContentParts[context.preceedingContentParts.length - 1] : undefined; - const previousContent = context.contentIndex > 0 ? context.content[context.contentIndex - 1] : undefined; - - // Special handling for "create" tool invocations- do not end thinking if previous part is a create tool invocation and config is set. - const shouldKeepThinkingForCreateTool = collapsedToolsMode !== CollapsedToolsDisplayMode.Off && lastRenderedPart instanceof ChatToolInvocationPart && this.isCreateToolInvocationContent(previousContent); - - const lastThinking = this.getLastThinkingPart(templateData.renderedParts); const isResponseElement = isResponseVM(context.element); - const isThinkingContent = content.kind === 'working' || content.kind === 'thinking'; - const isToolStreamingContent = isResponseElement && this.shouldPinPart(content, isResponseElement ? context.element : undefined); - if (!shouldKeepThinkingForCreateTool && lastThinking && lastThinking.getIsActive()) { - if (!isThinkingContent && !isToolStreamingContent) { - const followsThinkingPart = previousContent?.kind === 'thinking' || previousContent?.kind === 'toolInvocation' || previousContent?.kind === 'prepareToolInvocation' || previousContent?.kind === 'toolInvocationSerialized'; - - if (content.kind !== 'textEditGroup' && (context.element.isComplete || followsThinkingPart)) { - this.finalizeCurrentThinkingPart(context, templateData); - } - } - } + const shouldPin = this.shouldPinPart(content, isResponseElement ? context.element : undefined); // sometimes content is rendered out of order on re-renders so instead of looking at the current chat content part's // context and templateData, we have to look globally to find the active thinking part. - if (context.element.isComplete && !isThinkingContent && !this.shouldPinPart(content, isResponseElement ? context.element : undefined)) { + if (context.element.isComplete && !shouldPin) { for (const templateData of this.templateDataByRequestId.values()) { if (templateData.renderedParts) { const lastThinking = this.getLastThinkingPart(templateData.renderedParts); - if (content.kind !== 'textEditGroup' && lastThinking?.getIsActive()) { + if (lastThinking?.getIsActive()) { this.finalizeCurrentThinkingPart(context, templateData); } } @@ -1654,7 +1644,9 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer('chat.agent.thinking.collapsedTools'); + if (isResponseVM(context.element) && collapsedToolsMode !== CollapsedToolsDisplayMode.Off) { + + // append to thinking part when the codeblock is complete + const isComplete = this.isCodeblockComplete(markdown, context.element); + + // create thinking part if it doesn't exist yet + const lastThinking = this.getLastThinkingPart(templateData.renderedParts); + if (!lastThinking && markdownPart?.domNode && this.shouldPinPart(markdown, context.element) && collapsedToolsMode === CollapsedToolsDisplayMode.Always && isComplete) { + const thinkingPart = this.renderThinkingPart({ + kind: 'thinking', + }, context, templateData); + + if (thinkingPart instanceof ChatThinkingContentPart) { + thinkingPart.appendItem(markdownPart.domNode, markdownPart.codeblocksPartId, markdown, templateData.value); + thinkingPart.addDisposable(markdownPart.onDidChangeHeight(() => { + this.updateItemHeight(templateData); + })); + } + + return thinkingPart; + } + + if (this.shouldPinPart(markdown, context.element) && isComplete) { + if (lastThinking && markdownPart?.domNode) { + lastThinking.appendItem(markdownPart.domNode, markdownPart.codeblocksPartId, markdown, templateData.value); + } + } else if (!this.shouldPinPart(markdown, context.element)) { + this.finalizeCurrentThinkingPart(context, templateData); + } + } + return markdownPart; } diff --git a/src/vs/workbench/contrib/chat/common/widget/annotations.ts b/src/vs/workbench/contrib/chat/common/widget/annotations.ts index fad9bf57793..600decb9f36 100644 --- a/src/vs/workbench/contrib/chat/common/widget/annotations.ts +++ b/src/vs/workbench/contrib/chat/common/widget/annotations.ts @@ -117,6 +117,10 @@ export function extractCodeblockUrisFromText(text: string): { uri: URI; isEdit?: return undefined; } +export function hasCodeblockUriTag(text: string): boolean { + return text.includes('