Skip to content

Commit

Permalink
Remove internal implementation of ChatAgentTask (#202908)
Browse files Browse the repository at this point in the history 
* Get rid of chat progress Task

* Remove internal implementation of ChatAgentTask

* Clean up more
  • Loading branch information
@roblourens
roblourens authored Jan 20, 2024
1 parent 68f4339 commit 564d6ae
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 114 deletions.
31 changes: 2 additions & 29 deletions src/vs/workbench/api/browser/mainThreadChatAgents2.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { DeferredPromise } from 'vs/base/common/async';
import { CancellationToken } from 'vs/base/common/cancellation';
import { IMarkdownString } from 'vs/base/common/htmlContent';
import { Disposable, DisposableMap, IDisposable } from 'vs/base/common/lifecycle';
import { revive } from 'vs/base/common/marshalling';
import { escapeRegExpCharacters } from 'vs/base/common/strings';
Expand All @@ -23,7 +21,7 @@ import { AddDynamicVariableAction, IAddDynamicVariableContext } from 'vs/workben
import { IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
import { ChatRequestAgentPart } from 'vs/workbench/contrib/chat/common/chatParserTypes';
import { ChatRequestParser } from 'vs/workbench/contrib/chat/common/chatRequestParser';
import { IChatFollowup, IChatProgress, IChatService, IChatTreeData } from 'vs/workbench/contrib/chat/common/chatService';
import { IChatFollowup, IChatProgress, IChatService } from 'vs/workbench/contrib/chat/common/chatService';
import { IExtHostContext, extHostNamedCustomer } from 'vs/workbench/services/extensions/common/extHostCustomers';

type AgentData = {
Expand All @@ -42,9 +40,6 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
private readonly _pendingProgress = new Map<string, (part: IChatProgress) => void>();
private readonly _proxy: ExtHostChatAgentsShape2;

private _responsePartHandlePool = 0;
private readonly _activeResponsePartPromises = new Map<string, DeferredPromise<string | IMarkdownString | IChatTreeData>>();

constructor(
extHostContext: IExtHostContext,
@IChatAgentService private readonly _chatAgentService: IChatAgentService,
Expand Down Expand Up @@ -123,29 +118,7 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
this._chatAgentService.updateAgent(data.name, revive(metadataUpdate));
}

async $handleProgressChunk(requestId: string, progress: IChatProgressDto, responsePartHandle?: number): Promise<number | void> {
if (progress.kind === 'asyncContent') {
const handle = ++this._responsePartHandlePool;
const responsePartId = `${requestId}_${handle}`;
const deferredContentPromise = new DeferredPromise<string | IMarkdownString | IChatTreeData>();
this._activeResponsePartPromises.set(responsePartId, deferredContentPromise);
this._pendingProgress.get(requestId)?.({ ...progress, resolvedContent: deferredContentPromise.p });
return handle;
} else if (typeof responsePartHandle === 'number') {
// Complete an existing deferred promise with resolved content
const responsePartId = `${requestId}_${responsePartHandle}`;
const deferredContentPromise = this._activeResponsePartPromises.get(responsePartId);
if (deferredContentPromise && progress.kind === 'treeData') {
const withRevivedUris = revive<IChatTreeData>(progress);
deferredContentPromise.complete(withRevivedUris);
this._activeResponsePartPromises.delete(responsePartId);
} else if (deferredContentPromise && progress.kind === 'content') {
deferredContentPromise.complete(progress.content);
this._activeResponsePartPromises.delete(responsePartId);
}
return responsePartHandle;
}

async $handleProgressChunk(requestId: string, progress: IChatProgressDto): Promise<number | void> {
const revivedProgress = revive(progress);
this._pendingProgress.get(requestId)?.(revivedProgress as IChatProgress);
}
Expand Down
12 changes: 4 additions & 8 deletions src/vs/workbench/api/common/extHost.protocol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ import { CallHierarchyItem } from 'vs/workbench/contrib/callHierarchy/common/cal
import { IChatAgentCommand, IChatAgentMetadata, IChatAgentRequest, IChatAgentResult } from 'vs/workbench/contrib/chat/common/chatAgents';
import { IChatProgressResponseContent } from 'vs/workbench/contrib/chat/common/chatModel';
import { IChatMessage, IChatResponseFragment, IChatResponseProviderMetadata } from 'vs/workbench/contrib/chat/common/chatProvider';
import { IChatAsyncContent, IChatDynamicRequest, IChatFollowup, IChatProgress, IChatReplyFollowup, IChatResponseErrorDetails, IChatUserActionEvent, InteractiveSessionVoteDirection } from 'vs/workbench/contrib/chat/common/chatService';
import { IChatDynamicRequest, IChatFollowup, IChatProgress, IChatReplyFollowup, IChatResponseErrorDetails, IChatUserActionEvent, InteractiveSessionVoteDirection } from 'vs/workbench/contrib/chat/common/chatService';
import { IChatRequestVariableValue, IChatVariableData } from 'vs/workbench/contrib/chat/common/chatVariables';
import { DebugConfigurationProviderTriggerKind, MainThreadDebugVisualization, IAdapterDescriptor, IConfig, IDebugSessionReplMode, IDebugVisualization, IDebugVisualizationContext } from 'vs/workbench/contrib/debug/common/debug';
import { IInlineChatBulkEditResponse, IInlineChatEditResponse, IInlineChatProgressItem, IInlineChatRequest, IInlineChatSession, InlineChatResponseFeedbackKind } from 'vs/workbench/contrib/inlineChat/common/inlineChat';
Expand Down Expand Up @@ -1195,7 +1195,7 @@ export interface MainThreadChatAgentsShape2 extends IDisposable {
$unregisterAgentCompletionsProvider(handle: number): void;
$updateAgent(handle: number, metadataUpdate: IExtensionChatAgentMetadata): void;
$unregisterAgent(handle: number): void;
$handleProgressChunk(requestId: string, chunk: IChatProgressDto, responsePartHandle?: number): Promise<number | void>;
$handleProgressChunk(requestId: string, chunk: IChatProgressDto): Promise<number | void>;
}

export interface IChatAgentCompletionItem {
Expand All @@ -1207,8 +1207,7 @@ export interface IChatAgentCompletionItem {
}

export type IChatContentProgressDto =
| Dto<Exclude<IChatProgressResponseContent, IChatAsyncContent>>
| IChatAsyncContentDto;
| Dto<IChatProgressResponseContent>;

export type IChatAgentHistoryEntryDto = {
request: IChatAgentRequest;
Expand Down Expand Up @@ -1293,11 +1292,8 @@ export type IDocumentContextDto = {
ranges: IRange[];
};

export type IChatAsyncContentDto = Dto<Omit<IChatAsyncContent, 'resolvedContent'>>;

export type IChatProgressDto =
| Dto<Exclude<IChatProgress, IChatAsyncContent>>
| IChatAsyncContentDto;
| Dto<IChatProgress>;

export interface MainThreadChatShape extends IDisposable {
$registerChatProvider(handle: number, id: string): Promise<void>;
Expand Down
26 changes: 3 additions & 23 deletions src/vs/workbench/contrib/chat/browser/chatListRenderer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,8 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
? this.renderTreeData(data.treeData, element, templateData, fileTreeIndex++)
: data.kind === 'markdownContent'
? this.renderMarkdown(data.content, element, templateData, fillInIncompleteTokens)
: data.kind === 'asyncContent' ? this.renderPlaceholder(new MarkdownString(data.content), templateData)
: onlyProgressMessagesAfterI(value, index) ? this.renderProgressMessage(data, false)
: undefined;
: onlyProgressMessagesAfterI(value, index) ? this.renderProgressMessage(data, false)
: undefined;
if (result) {
templateData.value.appendChild(result.element);
templateData.elementDisposables.add(result);
Expand Down Expand Up @@ -572,11 +571,6 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
}
}

// Did this part go from being a placeholder string to resolved tree data?
else if (part.kind === 'treeData' && !isInteractiveProgressTreeData(renderedPart)) {
partsToRender[index] = part.treeData;
}

// Did this part's content change?
else if (part.kind !== 'treeData' && !isInteractiveProgressTreeData(renderedPart) && !isProgressMessageRenderData(renderedPart)) {
const wordCountResult = this.getDataForProgressiveRender(element, contentToMarkdown(part.content), renderedPart);
Expand Down Expand Up @@ -638,9 +632,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
// Avoid doing progressive rendering for multiple markdown parts simultaneously
else if (!hasRenderedOneMarkdownBlock && wordCountResults[index]) {
const { value } = wordCountResults[index];
result = renderableResponse[index].kind === 'asyncContent'
? this.renderPlaceholder(new MarkdownString(value), templateData)
: this.renderMarkdown(new MarkdownString(value), element, templateData, true);
result = this.renderMarkdown(new MarkdownString(value), element, templateData, true);
hasRenderedOneMarkdownBlock = true;
}

Expand Down Expand Up @@ -820,18 +812,6 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
element.ariaLabel = expanded ? localize('usedReferencesExpanded', "{0}, expanded", label) : localize('usedReferencesCollapsed', "{0}, collapsed", label);
}

private renderPlaceholder(markdown: IMarkdownString, templateData: IChatListItemTemplate): IMarkdownRenderResult {
const codicon = $('.interactive-response-codicon-details', undefined, renderIcon({ id: 'sync~spin' }));
codicon.classList.add('interactive-response-placeholder-codicon');
const result = dom.append(templateData.value, codicon);

const content = this.renderer.render(markdown);
content.element.className = 'interactive-response-placeholder-content';
result.appendChild(content.element);

return { element: result, dispose: () => content.dispose() };
}

private renderProgressMessage(progress: IChatProgressMessage, showSpinner: boolean): IMarkdownRenderResult {
if (showSpinner) {
// this step is in progress, communicate it to SR users
Expand Down
31 changes: 3 additions & 28 deletions src/vs/workbench/contrib/chat/common/chatModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { OffsetRange } from 'vs/editor/common/core/offsetRange';
import { ILogService } from 'vs/platform/log/common/log';
import { IChatAgentCommand, IChatAgentData, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
import { ChatRequestTextPart, IParsedChatRequest, reviveParsedChatRequest } from 'vs/workbench/contrib/chat/common/chatParserTypes';
import { IChat, IChatAgentMarkdownContentWithVulnerability, IChatAsyncContent, IChatContent, IChatContentInlineReference, IChatContentReference, IChatFollowup, IChatMarkdownContent, IChatProgress, IChatProgressMessage, IChatReplyFollowup, IChatResponse, IChatResponseErrorDetails, IChatResponseProgressFileTreeData, IChatTreeData, IChatUsedContext, InteractiveSessionVoteDirection, isIUsedContext } from 'vs/workbench/contrib/chat/common/chatService';
import { IChat, IChatAgentMarkdownContentWithVulnerability, IChatContent, IChatContentInlineReference, IChatContentReference, IChatFollowup, IChatMarkdownContent, IChatProgress, IChatProgressMessage, IChatReplyFollowup, IChatResponse, IChatResponseErrorDetails, IChatResponseProgressFileTreeData, IChatTreeData, IChatUsedContext, InteractiveSessionVoteDirection, isIUsedContext } from 'vs/workbench/contrib/chat/common/chatService';
import { IChatRequestVariableValue } from 'vs/workbench/contrib/chat/common/chatVariables';

export interface IChatRequestVariableData {
Expand All @@ -42,7 +42,6 @@ export type IChatProgressResponseContent =
| IChatMarkdownContent
| IChatAgentMarkdownContentWithVulnerability
| IChatTreeData
| IChatAsyncContent
| IChatContentInlineReference
| IChatProgressMessage;

Expand Down Expand Up @@ -160,22 +159,6 @@ export class Response implements IResponse {
}

this._updateRepr(quiet);
} else if (progress.kind === 'asyncContent') {
// Add a new resolving part
const responsePosition = this._responseParts.push(progress) - 1;
this._updateRepr(quiet);

progress.resolvedContent?.then((content) => {
// Replace the resolving part's content with the resolved response
if (typeof content === 'string') {
this._responseParts[responsePosition] = { content: new MarkdownString(content), kind: 'markdownContent' };
} else if (isMarkdownString(content)) {
this._responseParts[responsePosition] = { content, kind: 'markdownContent' };
} else {
this._responseParts[responsePosition] = content;
}
this._updateRepr(quiet);
});
} else if (progress.kind === 'treeData' || progress.kind === 'inlineReference' || progress.kind === 'markdownVuln' || progress.kind === 'progressMessage') {
this._responseParts.push(progress);
this._updateRepr(quiet);
Expand All @@ -188,8 +171,6 @@ export class Response implements IResponse {
return '';
} else if (part.kind === 'inlineReference') {
return basename('uri' in part.inlineReference ? part.inlineReference.uri : part.inlineReference);
} else if (part.kind === 'asyncContent') {
return part.content;
} else {
return part.content.value;
}
Expand Down Expand Up @@ -303,15 +284,12 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel
/**
* Apply one of the progress updates that are not part of the actual response content.
*/
applyReference(progress: IChatUsedContext | IChatContentReference | IChatProgressMessage) {
applyReference(progress: IChatUsedContext | IChatContentReference) {
if (progress.kind === 'usedContext') {
this._usedContext = progress;
} else if (progress.kind === 'reference') {
this._contentReferences.push(progress);
this._onDidChange.fire();
} else if (progress.kind === 'progressMessage') {
// this._progressMessages.push(progress);
// this._onDidChange.fire();
}
}

Expand Down Expand Up @@ -667,9 +645,8 @@ export class ChatModel extends Disposable implements IChatModel {
}

if (progress.kind === 'vulnerability') {
// TODO@roblourens ChatModel should just work with strings
request.response.updateContent({ kind: 'markdownVuln', content: { value: progress.content }, vulnerabilities: progress.vulnerabilities }, quiet);
} else if (progress.kind === 'content' || progress.kind === 'markdownContent' || progress.kind === 'asyncContent' || progress.kind === 'treeData' || progress.kind === 'inlineReference' || progress.kind === 'markdownVuln' || progress.kind === 'progressMessage') {
} else if (progress.kind === 'content' || progress.kind === 'markdownContent' || progress.kind === 'treeData' || progress.kind === 'inlineReference' || progress.kind === 'markdownVuln' || progress.kind === 'progressMessage') {
request.response.updateContent(progress, quiet);
} else if (progress.kind === 'usedContext' || progress.kind === 'reference') {
request.response.applyReference(progress);
Expand Down Expand Up @@ -758,8 +735,6 @@ export class ChatModel extends Disposable implements IChatModel {
return item.treeData;
} else if (item.kind === 'markdownContent') {
return item.content;
} else if (item.kind === 'asyncContent') {
return new MarkdownString(item.content);
} else {
return item as any; // TODO
}
Expand Down
10 changes: 0 additions & 10 deletions src/vs/workbench/contrib/chat/common/chatService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,6 @@ export interface IChatTreeData {
kind: 'treeData';
}

export interface IChatAsyncContent {
/**
* The placeholder to show while the content is loading
*/
content: string;
resolvedContent: Promise<string | IMarkdownString | IChatTreeData>;
kind: 'asyncContent';
}

export interface IChatProgressMessage {
content: IMarkdownString;
kind: 'progressMessage';
Expand Down Expand Up @@ -157,7 +148,6 @@ export type IChatProgress =
| IChatAgentContentWithVulnerabilities
| IChatAgentMarkdownContentWithVulnerability
| IChatTreeData
| IChatAsyncContent
| IChatUsedContext
| IChatContentReference
| IChatContentInlineReference
Expand Down
17 changes: 1 addition & 16 deletions src/vs/workbench/contrib/chat/test/common/chatModel.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import * as assert from 'assert';
import { DeferredPromise, timeout } from 'vs/base/common/async';
import { timeout } from 'vs/base/common/async';
import { MarkdownString } from 'vs/base/common/htmlContent';
import { URI } from 'vs/base/common/uri';
import { assertSnapshot } from 'vs/base/test/common/snapshot';
Expand All @@ -17,7 +17,6 @@ import { IStorageService } from 'vs/platform/storage/common/storage';
import { ChatAgentService, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
import { ChatModel, Response } from 'vs/workbench/contrib/chat/common/chatModel';
import { ChatRequestTextPart } from 'vs/workbench/contrib/chat/common/chatParserTypes';
import { IChatTreeData } from 'vs/workbench/contrib/chat/common/chatService';
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { TestExtensionService, TestStorageService } from 'vs/workbench/test/common/workbenchTestServices';

Expand Down Expand Up @@ -140,20 +139,6 @@ suite('Response', () => {
await assertSnapshot(response.value);
});

test('async content', async () => {
const response = new Response([]);
const deferred = new DeferredPromise<string>();
const deferred2 = new DeferredPromise<IChatTreeData>();
response.updateContent({ resolvedContent: deferred.p, content: 'text', kind: 'asyncContent' });
response.updateContent({ resolvedContent: deferred2.p, content: 'text2', kind: 'asyncContent' });
await assertSnapshot(response.value);

await deferred2.complete({ kind: 'treeData', treeData: { label: 'label', uri: URI.parse('https://microsoft.com') } });
await deferred.complete('resolved');
await assertSnapshot(response.value);
});


test('inline reference', async () => {
const response = new Response([]);
response.updateContent({ content: 'text before', kind: 'content' });
Expand Down

0 comments on commit 564d6ae

Please sign in to comment.