diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts index ddbc257facf12..6899e66a113b5 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts @@ -255,10 +255,10 @@ export function registerChatCodeBlockActions() { // try applying workspace edit that was returned by a MappedEditsProvider, else simply insert at selection - let workspaceEdit: WorkspaceEdit | null = null; + let mappedEdits: WorkspaceEdit | null = null; if (mappedEditsProviders.length > 0) { - const mostRelevantProvider = mappedEditsProviders[0]; + const mostRelevantProvider = mappedEditsProviders[0]; // TODO@ulugbekna: should we try all providers? // 0th sub-array - editor selections array if there are any selections // 1st sub-array - array with documents used to get the chat reply @@ -287,21 +287,23 @@ export function registerChatCodeBlockActions() { const cancellationTokenSource = new CancellationTokenSource(); - workspaceEdit = await mostRelevantProvider.provideMappedEdits( + mappedEdits = await mostRelevantProvider.provideMappedEdits( activeModel, [chatCodeBlockActionContext.code], { documents: docRefs }, cancellationTokenSource.token); } - if (workspaceEdit) { - await bulkEditService.apply(workspaceEdit); + if (mappedEdits) { + await bulkEditService.apply(mappedEdits); } else { const activeSelection = codeEditor.getSelection() ?? new Range(activeModel.getLineCount(), 1, activeModel.getLineCount(), 1); - await bulkEditService.apply([new ResourceTextEdit(activeModel.uri, { - range: activeSelection, - text: chatCodeBlockActionContext.code, - })]); + await bulkEditService.apply([ + new ResourceTextEdit(activeModel.uri, { + range: activeSelection, + text: chatCodeBlockActionContext.code, + }), + ]); } codeEditorService.listCodeEditors().find(editor => editor.getModel()?.uri.toString() === activeModel.uri.toString())?.focus(); } diff --git a/src/vs/workbench/contrib/chat/common/chatModel.ts b/src/vs/workbench/contrib/chat/common/chatModel.ts index 2656e980d0515..5e62ae84d173e 100644 --- a/src/vs/workbench/contrib/chat/common/chatModel.ts +++ b/src/vs/workbench/contrib/chat/common/chatModel.ts @@ -13,7 +13,7 @@ import { OffsetRange } from 'vs/editor/common/core/offsetRange'; import { ILogService } from 'vs/platform/log/common/log'; import { IChatAgentData, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents'; import { ChatRequestTextPart, IParsedChatRequest, reviveParsedChatRequest } from 'vs/workbench/contrib/chat/common/chatParserTypes'; -import { IChat, IChatFollowup, IChatProgress, IChatReplyFollowup, IChatResponse, IChatResponseErrorDetails, IChatResponseProgressFileTreeData, IUsedContext, InteractiveSessionVoteDirection } from 'vs/workbench/contrib/chat/common/chatService'; +import { IChat, IChatFollowup, IChatProgress, IChatReplyFollowup, IChatResponse, IChatResponseErrorDetails, IChatResponseProgressFileTreeData, IUsedContext, InteractiveSessionVoteDirection, isIUsedContext } from 'vs/workbench/contrib/chat/common/chatService'; export interface IChatRequestModel { readonly id: string; @@ -62,14 +62,6 @@ export interface IChatResponseModel { setVote(vote: InteractiveSessionVoteDirection): void; } -export function isRequest(item: unknown): item is IChatRequestModel { - return !!item && typeof (item as IChatRequestModel).message !== 'undefined'; -} - -export function isResponse(item: unknown): item is IChatResponseModel { - return !isRequest(item); -} - export class ChatRequestModel implements IChatRequestModel { private static nextId = 0; @@ -355,6 +347,8 @@ export interface ISerializableChatRequestData { followups: IChatFollowup[] | undefined; isCanceled: boolean | undefined; vote: InteractiveSessionVoteDirection | undefined; + /** For backward compat: should be optional */ + usedContext?: IUsedContext; } export interface IExportableChatData { @@ -386,7 +380,10 @@ export function isSerializableSessionData(obj: unknown): obj is ISerializableCha const data = obj as ISerializableChatData; return isExportableSessionData(obj) && typeof data.creationDate === 'number' && - typeof data.sessionId === 'string'; + typeof data.sessionId === 'string' && + obj.requests.every((request: ISerializableChatRequestData) => + !request.usedContext /* for backward compat allow missing usedContext */ || isIUsedContext(request.usedContext) + ); } export type IChatChangeEvent = IChatAddRequestEvent | IChatAddResponseEvent | IChatInitEvent | IChatRemoveRequestEvent; @@ -523,12 +520,17 @@ export class ChatModel extends Disposable implements IChatModel { try { return requests.map((raw: ISerializableChatRequestData) => { - const parsedRequest = typeof raw.message === 'string' ? this.getParsedRequestFromString(raw.message) : - reviveParsedChatRequest(raw.message); + const parsedRequest = + typeof raw.message === 'string' + ? this.getParsedRequestFromString(raw.message) + : reviveParsedChatRequest(raw.message); const request = new ChatRequestModel(this, parsedRequest, raw.providerRequestId); if (raw.response || raw.responseErrorDetails) { const agent = raw.agent && this.chatAgentService.getAgents().find(a => a.id === raw.agent!.id); // TODO do something reasonable if this agent has disappeared since the last session request.response = new ChatResponseModel(raw.response ?? [new MarkdownString(raw.response)], this, agent, true, raw.isCanceled, raw.vote, raw.providerRequestId, raw.responseErrorDetails, raw.followups); + if (raw.usedContext) { // @ulugbekna: if this's a new vscode sessions, doc versions are incorrect anyway? + request.response.updateContent(raw.usedContext); + } } return request; }); @@ -708,6 +710,7 @@ export class ChatModel extends Disposable implements IChatModel { fullName: r.response.agent.metadata.fullName, icon: r.response.agent.metadata.icon } : undefined, + usedContext: r.response?.response.usedContext, }; }), providerId: this.providerId, diff --git a/src/vs/workbench/contrib/chat/common/chatService.ts b/src/vs/workbench/contrib/chat/common/chatService.ts index f36f6e6785468..22bf865b6ce7c 100644 --- a/src/vs/workbench/contrib/chat/common/chatService.ts +++ b/src/vs/workbench/contrib/chat/common/chatService.ts @@ -8,7 +8,7 @@ import { Event } from 'vs/base/common/event'; import { IMarkdownString } from 'vs/base/common/htmlContent'; import { IDisposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; -import { IRange } from 'vs/editor/common/core/range'; +import { Range, IRange } from 'vs/editor/common/core/range'; import { ProviderResult } from 'vs/editor/common/languages'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { IChatModel, ChatModel, ISerializableChatData } from 'vs/workbench/contrib/chat/common/chatModel'; @@ -59,10 +59,30 @@ export type IDocumentContext = { ranges: IRange[]; }; +export function isIDocumentContext(obj: unknown): obj is IDocumentContext { + return ( + !!obj && + typeof obj === 'object' && + 'uri' in obj && obj.uri instanceof URI && + 'version' in obj && typeof obj.version === 'number' && + 'ranges' in obj && Array.isArray(obj.ranges) && obj.ranges.every(Range.isIRange) + ); +} + export type IUsedContext = { documents: IDocumentContext[]; }; +export function isIUsedContext(obj: unknown): obj is IUsedContext { + return ( + !!obj && + typeof obj === 'object' && + 'documents' in obj && + Array.isArray(obj.documents) && + obj.documents.every(isIDocumentContext) + ); +} + export type IChatProgress = | { content: string | IMarkdownString } | { requestId: string } diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_deserialize.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_deserialize.0.snap new file mode 100644 index 0000000000000..8a055fa515789 --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_deserialize.0.snap @@ -0,0 +1,68 @@ +{ + requesterUsername: "test", + requesterAvatarIconUri: undefined, + responderUsername: "test", + responderAvatarIconUri: undefined, + welcomeMessage: undefined, + requests: [ + { + providerRequestId: undefined, + message: { + text: "test request", + parts: [ + { + range: { + start: 0, + endExclusive: 12 + }, + editorRange: { + startLineNumber: 1, + startColumn: 1, + endLineNumber: 1, + endColumn: 13 + }, + text: "test request", + kind: "text" + } + ] + }, + response: [ + { + value: "", + isTrusted: false, + supportThemeIcons: false, + supportHtml: false + } + ], + responseErrorDetails: undefined, + followups: undefined, + isCanceled: false, + vote: undefined, + agent: undefined, + usedContext: { documents: [ + { + uri: { + scheme: "file", + authority: "", + path: "/test/path/to/file", + query: "", + fragment: "", + _formatted: null, + _fsPath: null + }, + version: 3, + ranges: [ + { + startLineNumber: 1, + startColumn: 1, + endLineNumber: 2, + endColumn: 2 + } + ] + } + ] } + } + ], + providerId: "ChatProviderWithUsedContext", + providerState: undefined +} \ No newline at end of file diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.0.snap new file mode 100644 index 0000000000000..ba62978586c09 --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.0.snap @@ -0,0 +1,10 @@ +{ + requesterUsername: "", + requesterAvatarIconUri: undefined, + responderUsername: "", + responderAvatarIconUri: undefined, + welcomeMessage: undefined, + requests: [ ], + providerId: "ChatProviderWithUsedContext", + providerState: undefined +} \ No newline at end of file diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.1.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.1.snap new file mode 100644 index 0000000000000..4abd4c09014c3 --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/Chat_can_serialize.1.snap @@ -0,0 +1,68 @@ +{ + requesterUsername: "test", + requesterAvatarIconUri: undefined, + responderUsername: "test", + responderAvatarIconUri: undefined, + welcomeMessage: undefined, + requests: [ + { + providerRequestId: undefined, + message: { + parts: [ + { + range: { + start: 0, + endExclusive: 12 + }, + editorRange: { + startLineNumber: 1, + startColumn: 1, + endLineNumber: 1, + endColumn: 13 + }, + text: "test request", + kind: "text" + } + ], + text: "test request" + }, + response: [ + { + value: "", + isTrusted: false, + supportThemeIcons: false, + supportHtml: false + } + ], + responseErrorDetails: undefined, + followups: undefined, + isCanceled: false, + vote: undefined, + agent: undefined, + usedContext: { documents: [ + { + uri: { + scheme: "file", + authority: "", + path: "/test/path/to/file", + query: "", + fragment: "", + _formatted: null, + _fsPath: null + }, + version: 3, + ranges: [ + { + startLineNumber: 1, + startColumn: 1, + endLineNumber: 2, + endColumn: 2 + } + ] + } + ] } + } + ], + providerId: "ChatProviderWithUsedContext", + providerState: undefined +} \ No newline at end of file diff --git a/src/vs/workbench/contrib/chat/test/common/chatService.test.ts b/src/vs/workbench/contrib/chat/test/common/chatService.test.ts index 9cf2a92c98816..00a8133fbfaf2 100644 --- a/src/vs/workbench/contrib/chat/test/common/chatService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/chatService.test.ts @@ -7,7 +7,10 @@ import * as assert from 'assert'; import { CancellationToken } from 'vs/base/common/cancellation'; import { Emitter } from 'vs/base/common/event'; import { Disposable } from 'vs/base/common/lifecycle'; +import { URI } from 'vs/base/common/uri'; +import { assertSnapshot } from 'vs/base/test/common/snapshot'; import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils'; +import { Range } from 'vs/editor/common/core/range'; import { ProviderResult } from 'vs/editor/common/languages'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors'; @@ -22,6 +25,7 @@ import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace import { IViewsService } from 'vs/workbench/common/views'; import { ChatAgentService, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents'; import { IChatContributionService } from 'vs/workbench/contrib/chat/common/chatContributionService'; +import { ISerializableChatData } from 'vs/workbench/contrib/chat/common/chatModel'; import { IChat, IChatProgress, IChatProvider, IChatRequest, IChatResponse, IPersistedChatState, ISlashCommand } from 'vs/workbench/contrib/chat/common/chatService'; import { ChatService } from 'vs/workbench/contrib/chat/common/chatServiceImpl'; import { ChatSlashCommandService, IChatSlashCommandService } from 'vs/workbench/contrib/chat/common/chatSlashCommands'; @@ -57,11 +61,31 @@ class SimpleTestProvider extends Disposable implements IChatProvider { this._onDidChangeState.fire(state); } - async provideReply(request: IChatRequest) { + async provideReply(request: IChatRequest, progress: (progress: IChatProgress) => void): Promise<{ session: IChat; followups: never[] }> { return { session: request.session, followups: [] }; } } +/** Chat provider for testing that returns used context */ +class ChatProviderWithUsedContext extends SimpleTestProvider implements IChatProvider { + override provideReply(request: IChatRequest, progress: (progress: IChatProgress) => void): Promise<{ session: IChat; followups: never[] }> { + + progress({ + documents: [ + { + uri: URI.file('/test/path/to/file'), + version: 3, + ranges: [ + new Range(1, 1, 2, 2) + ] + } + ] + }); + + return super.provideReply(request, progress); + } +} + suite('Chat', () => { const testDisposables = ensureNoDisposablesAreLeakedInTestSuite(); @@ -223,4 +247,66 @@ suite('Chat', () => { assert.ok(model.getRequests()[0].response); assert.strictEqual(model.getRequests()[0].response?.response.asString(), 'test response'); }); + + test('can serialize', async () => { + + const testService = testDisposables.add(instantiationService.createInstance(ChatService)); + const providerId = 'ChatProviderWithUsedContext'; + + testDisposables.add(testService.registerProvider(testDisposables.add(new ChatProviderWithUsedContext(providerId)))); + + const model = testDisposables.add(testService.startSession(providerId, CancellationToken.None)); + assert.strictEqual(model.getRequests().length, 0); + + assertSnapshot(model.toExport()); + + const response = await testService.sendRequest(model.sessionId, 'test request'); + assert(response); + + await response.responseCompletePromise; + + assert.strictEqual(model.getRequests().length, 1); + + assertSnapshot(model.toExport()); + }); + + test('can deserialize', async () => { + + let serializedChatData: ISerializableChatData; + + const providerId = 'ChatProviderWithUsedContext'; + + // create the first service, send request, get response, and serialize the state + { // serapate block to not leak variables in outer scope + const testService = testDisposables.add(instantiationService.createInstance(ChatService)); + + testDisposables.add(testService.registerProvider(testDisposables.add(new ChatProviderWithUsedContext(providerId)))); + + const chatModel1 = testDisposables.add(testService.startSession(providerId, CancellationToken.None)); + assert.strictEqual(chatModel1.getRequests().length, 0); + + const response = await testService.sendRequest(chatModel1.sessionId, 'test request'); + assert(response); + + await response.responseCompletePromise; + + serializedChatData = chatModel1.toJSON(); + } + + // try deserializing the state into a new service + + const testService2 = testDisposables.add(instantiationService.createInstance(ChatService)); + + testDisposables.add(testService2.registerProvider(testDisposables.add(new ChatProviderWithUsedContext(providerId)))); + + const chatModel2 = testService2.loadSessionFromContent(serializedChatData); + assert(chatModel2); + + // should `loadSessionFromContent` return `ChatModel` that's disposable instead of `IChatModel`? + testDisposables.add({ + dispose: () => testService2.clearSession(serializedChatData.sessionId) + }); + + assertSnapshot(chatModel2.toExport()); + }); });