mapped edits: support de/serialization of mapped edits context so that it can be used across vscode sessions (#194572)

* refactor(chatModel): remove unused code

especially because `isResponse` may've been incorrect because it would return true on a falsy value while `isRequest` checked for it

* refactor(mapped edits): better names

* mapped edits: support de/serialization of mapped edits context so that it can be used across vscode sessions

reservation/concern: the document versions are obsolete/incorrect, what should we do with them?

* mapped edits: fix test failing remotely but not locally
This commit is contained in:
Ulugbek Abdullaev 2023-10-03 14:41:02 +02:00 committed by GitHub
parent 01edf51e0f
commit 2bdd4af2d0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 280 additions and 23 deletions

View file

@ -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();
}

View file

@ -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,

View file

@ -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 }

View file

@ -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
}

View file

@ -0,0 +1,10 @@
{
requesterUsername: "",
requesterAvatarIconUri: undefined,
responderUsername: "",
responderAvatarIconUri: undefined,
welcomeMessage: undefined,
requests: [ ],
providerId: "ChatProviderWithUsedContext",
providerState: undefined
}

View file

@ -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
}

View file

@ -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 { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
@ -21,6 +24,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();
@ -222,4 +246,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());
});
});