Fix merging chat anchors in markdown content (#213114)

* Fix merging multiple chat anchors inside markdown content * Simplify types * Fix tests
microsoft · May 21, 2024 · 601018a · 601018a
1 parent 51591f8
commit 601018a
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 16 deletions.
diff --git a/src/vs/workbench/contrib/chat/common/annotations.ts b/src/vs/workbench/contrib/chat/common/annotations.ts
@@ -6,32 +6,35 @@ import { MarkdownString } from 'vs/base/common/htmlContent';
 import { basename } from 'vs/base/common/resources';
 import { URI } from 'vs/base/common/uri';
 import { IRange } from 'vs/editor/common/core/range';
-import { IChatProgressRenderableResponseContent, IChatProgressResponseContent, canMergeMarkdownStrings } from 'vs/workbench/contrib/chat/common/chatModel';
-import { IChatAgentMarkdownContentWithVulnerability, IChatAgentVulnerabilityDetails, IChatContentInlineReference, IChatMarkdownContent, IChatTask } from 'vs/workbench/contrib/chat/common/chatService';
+import { IChatProgressRenderableResponseContent, IChatProgressResponseContent, appendMarkdownString, canMergeMarkdownStrings } from 'vs/workbench/contrib/chat/common/chatModel';
+import { IChatAgentVulnerabilityDetails, IChatMarkdownContent } from 'vs/workbench/contrib/chat/common/chatService';
 
 export const contentRefUrl = 'http://_vscodecontentref_'; // must be lowercase for URI
 
 export function annotateSpecialMarkdownContent(response: ReadonlyArray<IChatProgressResponseContent>): ReadonlyArray<IChatProgressRenderableResponseContent> {
-	const result: Exclude<IChatProgressResponseContent | IChatTask, IChatContentInlineReference | IChatAgentMarkdownContentWithVulnerability>[] = [];
+	const result: IChatProgressRenderableResponseContent[] = [];
 	for (const item of response) {
 		const previousItem = result[result.length - 1];
 		if (item.kind === 'inlineReference') {
 			const location = 'uri' in item.inlineReference ? item.inlineReference : { uri: item.inlineReference };
 			const printUri = URI.parse(contentRefUrl).with({ fragment: JSON.stringify(location) });
 			const markdownText = `[${item.name || basename(location.uri)}](${printUri.toString()})`;
 			if (previousItem?.kind === 'markdownContent') {
-				result[result.length - 1] = { content: new MarkdownString(previousItem.content.value + markdownText), kind: 'markdownContent' };
+				const merged = appendMarkdownString(previousItem.content, new MarkdownString(markdownText));
+				result[result.length - 1] = { content: merged, kind: 'markdownContent' };
 			} else {
 				result.push({ content: new MarkdownString(markdownText), kind: 'markdownContent' });
 			}
 		} else if (item.kind === 'markdownContent' && previousItem?.kind === 'markdownContent' && canMergeMarkdownStrings(previousItem.content, item.content)) {
-			result[result.length - 1] = { content: new MarkdownString(previousItem.content.value + item.content.value), kind: 'markdownContent' };
+			const merged = appendMarkdownString(previousItem.content, item.content);
+			result[result.length - 1] = { content: merged, kind: 'markdownContent' };
 		} else if (item.kind === 'markdownVuln') {
 			const vulnText = encodeURIComponent(JSON.stringify(item.vulnerabilities));
 			const markdownText = `<vscode_annotation details='${vulnText}'>${item.content.value}</vscode_annotation>`;
 			if (previousItem?.kind === 'markdownContent') {
 				// Since this is inside a codeblock, it needs to be merged into the previous markdown content.
-				result[result.length - 1] = { content: new MarkdownString(previousItem.content.value + markdownText, { isTrusted: previousItem.content.isTrusted }), kind: 'markdownContent' };
+				const merged = appendMarkdownString(previousItem.content, new MarkdownString(markdownText));
+				result[result.length - 1] = { content: merged, kind: 'markdownContent' };
 			} else {
 				result.push({ content: new MarkdownString(markdownText), kind: 'markdownContent' });
 			}

diff --git a/src/vs/workbench/contrib/chat/common/chatModel.ts b/src/vs/workbench/contrib/chat/common/chatModel.ts
@@ -185,13 +185,7 @@ export class Response implements IResponse {
 				// The last part can't be merged with- not markdown, or markdown with different permissions
 				this._responseParts.push(progress);
 			} else {
-				lastResponsePart.content = {
-					value: lastResponsePart.content.value + progress.content.value,
-					isTrusted: lastResponsePart.content.isTrusted,
-					supportThemeIcons: lastResponsePart.content.supportThemeIcons,
-					supportHtml: lastResponsePart.content.supportHtml,
-					baseUri: lastResponsePart.content.baseUri
-				} satisfies IMarkdownString;
+				lastResponsePart.content = appendMarkdownString(lastResponsePart.content, progress.content);
 			}
 			this._updateRepr(quiet);
 		} else if (progress.kind === 'textEdit') {
@@ -1018,3 +1012,14 @@ export function canMergeMarkdownStrings(md1: IMarkdownString, md2: IMarkdownStri
 		md1.supportHtml === md2.supportHtml &&
 		md1.supportThemeIcons === md2.supportThemeIcons;
 }
+
+export function appendMarkdownString(md1: IMarkdownString, md2: IMarkdownString | string): IMarkdownString {
+	const appendedValue = typeof md2 === 'string' ? md2 : md2.value;
+	return {
+		value: md1.value + appendedValue,
+		isTrusted: md1.isTrusted,
+		supportThemeIcons: md1.supportThemeIcons,
+		supportHtml: md1.supportHtml,
+		baseUri: md1.baseUri
+	};
+}
diff --git a/...hat/test/common/__snapshots__/Annotations_extractVulnerabilitiesFromText_multiline.0.snap b/...hat/test/common/__snapshots__/Annotations_extractVulnerabilitiesFromText_multiline.0.snap
@@ -4,7 +4,8 @@
       value: "some code\nover\nmultiple lines <vscode_annotation details='%5B%7B%22title%22%3A%22title%22%2C%22description%22%3A%22vuln%22%7D%5D'>content with vuln\nand\nnewlines</vscode_annotation>more code\nwith newline",
       isTrusted: false,
       supportThemeIcons: false,
-      supportHtml: false
+      supportHtml: false,
+      baseUri: undefined
     },
     kind: "markdownContent"
   }

diff --git a/...est/common/__snapshots__/Annotations_extractVulnerabilitiesFromText_multiple_vulns.0.snap b/...est/common/__snapshots__/Annotations_extractVulnerabilitiesFromText_multiple_vulns.0.snap
@@ -4,7 +4,8 @@
       value: "some code\nover\nmultiple lines <vscode_annotation details='%5B%7B%22title%22%3A%22title%22%2C%22description%22%3A%22vuln%22%7D%5D'>content with vuln\nand\nnewlines</vscode_annotation>more code\nwith newline<vscode_annotation details='%5B%7B%22title%22%3A%22title%22%2C%22description%22%3A%22vuln%22%7D%5D'>content with vuln\nand\nnewlines</vscode_annotation>",
       isTrusted: false,
       supportThemeIcons: false,
-      supportHtml: false
+      supportHtml: false,
+      baseUri: undefined
     },
     kind: "markdownContent"
   }

diff --git a/...t/test/common/__snapshots__/Annotations_extractVulnerabilitiesFromText_single_line.0.snap b/...t/test/common/__snapshots__/Annotations_extractVulnerabilitiesFromText_single_line.0.snap
@@ -4,7 +4,8 @@
       value: "some code <vscode_annotation details='%5B%7B%22title%22%3A%22title%22%2C%22description%22%3A%22vuln%22%7D%5D'>content with vuln</vscode_annotation> after",
       isTrusted: false,
       supportThemeIcons: false,
-      supportHtml: false
+      supportHtml: false,
+      baseUri: undefined
     },
     kind: "markdownContent"
   }