Skip to content

Commit

Permalink
Enable roaming of inflight chat requests (microsoft#213239)
Browse files Browse the repository at this point in the history
This enables to remove the special handling for `/explain`

- fixes https://github.com/microsoft/vscode-copilot/issues/2192
- fixes microsoft#208706 because inline chat now closes on "view in chat"
- fixes https://github.com/microsoft/vscode-copilot/issues/5288
  • Loading branch information
jrieken authored and andremmsilva committed May 26, 2024
1 parent 52a4242 commit de93a5e
Show file tree
Hide file tree
Showing 12 changed files with 178 additions and 148 deletions.
10 changes: 10 additions & 0 deletions src/vs/base/common/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,16 @@ export class DisposableMap<K, V extends IDisposable = IDisposable> implements ID
this._store.delete(key);
}

/**
* Delete the value stored for `key` from this map but return it. The caller is
* responsible for disposing of the value.
*/
deleteAndLeak(key: K): V | undefined {
const value = this._store.get(key);
this._store.delete(key);
return value;
}

keys(): IterableIterator<K> {
return this._store.keys();
}
Expand Down
53 changes: 42 additions & 11 deletions src/vs/workbench/contrib/chat/common/chatModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti
import { ILogService } from 'vs/platform/log/common/log';
import { ChatAgentLocation, IChatAgentCommand, IChatAgentData, IChatAgentHistoryEntry, IChatAgentRequest, IChatAgentResult, IChatAgentService, reviveSerializedAgent } from 'vs/workbench/contrib/chat/common/chatAgents';
import { ChatRequestTextPart, IParsedChatRequest, getPromptText, reviveParsedChatRequest } from 'vs/workbench/contrib/chat/common/chatParserTypes';
import { IChatAgentMarkdownContentWithVulnerability, IChatCommandButton, IChatConfirmation, IChatContentInlineReference, IChatContentReference, IChatFollowup, IChatMarkdownContent, IChatProgress, IChatProgressMessage, IChatResponseProgressFileTreeData, IChatTask, IChatTextEdit, IChatTreeData, IChatUsedContext, IChatWarningMessage, ChatAgentVoteDirection, isIUsedContext } from 'vs/workbench/contrib/chat/common/chatService';
import { IChatAgentMarkdownContentWithVulnerability, IChatCommandButton, IChatConfirmation, IChatContentInlineReference, IChatContentReference, IChatFollowup, IChatMarkdownContent, IChatProgressMessage, IChatResponseProgressFileTreeData, IChatTask, IChatTextEdit, IChatTreeData, IChatUsedContext, IChatWarningMessage, ChatAgentVoteDirection, isIUsedContext, IChatProgress } from 'vs/workbench/contrib/chat/common/chatService';
import { IChatRequestVariableValue } from 'vs/workbench/contrib/chat/common/chatVariables';

export interface IChatRequestVariableEntry {
Expand Down Expand Up @@ -109,9 +109,10 @@ export class ChatRequestModel implements IChatRequestModel {

public response: ChatResponseModel | undefined;

private _id: string;
public get id(): string {
return this._id;
public readonly id: string;

public get session() {
return this._session;
}

public get username(): string {
Expand All @@ -135,12 +136,16 @@ export class ChatRequestModel implements IChatRequestModel {
}

constructor(
public readonly session: ChatModel,
private _session: ChatModel,
public readonly message: IParsedChatRequest,
private _variableData: IChatRequestVariableData,
private _attempt: number = 0
) {
this._id = 'request_' + ChatRequestModel.nextId++;
this.id = 'request_' + ChatRequestModel.nextId++;
}

adoptTo(session: ChatModel) {
this._session = session;
}
}

Expand Down Expand Up @@ -267,9 +272,10 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel

private static nextId = 0;

private _id: string;
public get id(): string {
return this._id;
public readonly id: string;

public get session() {
return this._session;
}

public get isComplete(): boolean {
Expand Down Expand Up @@ -342,7 +348,7 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel

constructor(
_response: IMarkdownString | ReadonlyArray<IMarkdownString | IChatResponseProgressFileTreeData | IChatContentInlineReference | IChatAgentMarkdownContentWithVulnerability>,
public readonly session: ChatModel,
private _session: ChatModel,
private _agent: IChatAgentData | undefined,
private _slashCommand: IChatAgentCommand | undefined,
public readonly requestId: string,
Expand All @@ -360,7 +366,7 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel
this._followups = followups ? [...followups] : undefined;
this._response = new Response(_response);
this._register(this._response.onDidChangeValue(() => this._onDidChange.fire()));
this._id = 'response_' + ChatResponseModel.nextId++;
this.id = 'response_' + ChatResponseModel.nextId++;
}

/**
Expand Down Expand Up @@ -430,6 +436,11 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel
this._onDidChange.fire();
return true;
}

adoptTo(session: ChatModel) {
this._session = session;
this._onDidChange.fire();
}
}

export interface IChatModel {
Expand Down Expand Up @@ -773,6 +784,26 @@ export class ChatModel extends Disposable implements IChatModel {
return request;
}

adoptRequest(request: ChatRequestModel): void {

// this doesn't use `removeRequest` because it must not dispose the request object
const oldOwner = request.session;
const index = oldOwner._requests.findIndex(candidate => candidate.id === request.id);

if (index === -1) {
return;
}

oldOwner._requests.splice(index, 1);

request.adoptTo(this);
request.response?.adoptTo(this);
this._requests.push(request);

oldOwner._onDidChange.fire({ kind: 'removeRequest', requestId: request.id, responseId: request.response?.id });
this._onDidChange.fire({ kind: 'addRequest', request });
}

acceptResponseProgress(request: ChatRequestModel, progress: IChatProgress, quiet?: boolean): void {
if (!request.response) {
request.response = new ChatResponseModel([], this, undefined, undefined, request.id);
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/chat/common/chatService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ export interface IChatService {
sendRequest(sessionId: string, message: string, options?: IChatSendRequestOptions): Promise<IChatSendRequestData | undefined>;

resendRequest(request: IChatRequestModel, options?: IChatSendRequestOptions): Promise<void>;

adoptRequest(sessionId: string, request: IChatRequestModel): Promise<void>;
removeRequest(sessionid: string, requestId: string): Promise<void>;
cancelCurrentRequestForSession(sessionId: string): void;
clearSession(sessionId: string): void;
Expand Down
22 changes: 22 additions & 0 deletions src/vs/workbench/contrib/chat/common/chatServiceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,28 @@ export class ChatService extends Disposable implements IChatService {
model.removeRequest(requestId);
}

async adoptRequest(sessionId: string, request: IChatRequestModel) {
if (!(request instanceof ChatRequestModel)) {
throw new TypeError('Can only adopt requests of type ChatRequestModel');
}
const target = this._sessionModels.get(sessionId);
if (!target) {
throw new Error(`Unknown session: ${sessionId}`);
}

await target.waitForInitialization();

const oldOwner = request.session;
target.adoptRequest(request);

if (request.response && !request.response.isComplete) {
const cts = this._pendingRequests.deleteAndLeak(oldOwner.sessionId);
if (cts) {
this._pendingRequests.set(target.sessionId, cts);
}
}
}

async addCompleteRequest(sessionId: string, message: IParsedChatRequest | string, variableData: IChatRequestVariableData | undefined, attempt: number | undefined, response: IChatCompleteResponse): Promise<void> {
this.trace('addCompleteRequest', `message: ${message}`);

Expand Down
30 changes: 30 additions & 0 deletions src/vs/workbench/contrib/chat/test/common/chatModel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,36 @@ suite('ChatModel', () => {
model.removeRequest(requests[0].id);
assert.strictEqual(model.getRequests().length, 0);
});

test('adoptRequest', async function () {
const model1 = testDisposables.add(instantiationService.createInstance(ChatModel, undefined, ChatAgentLocation.Editor));
const model2 = testDisposables.add(instantiationService.createInstance(ChatModel, undefined, ChatAgentLocation.Panel));

model1.startInitialize();
model1.initialize(undefined);

model2.startInitialize();
model2.initialize(undefined);

const text = 'hello';
const request1 = model1.addRequest({ text, parts: [new ChatRequestTextPart(new OffsetRange(0, text.length), new Range(1, text.length, 1, text.length), text)] }, { variables: [] }, 0);

assert.strictEqual(model1.getRequests().length, 1);
assert.strictEqual(model2.getRequests().length, 0);
assert.ok(request1.session === model1);
assert.ok(request1.response?.session === model1);

model2.adoptRequest(request1);

assert.strictEqual(model1.getRequests().length, 0);
assert.strictEqual(model2.getRequests().length, 1);
assert.ok(request1.session === model2);
assert.ok(request1.response?.session === model2);

model2.acceptResponseProgress(request1, { content: new MarkdownString('Hello'), kind: 'markdownContent' });

assert.strictEqual(request1.response.response.asString(), 'Hello');
});
});

suite('Response', () => {
Expand Down
3 changes: 3 additions & 0 deletions src/vs/workbench/contrib/chat/test/common/mockChatService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ export class MockChatService implements IChatService {
resendRequest(request: IChatRequestModel, options?: IChatSendRequestOptions | undefined): Promise<void> {
throw new Error('Method not implemented.');
}
adoptRequest(sessionId: string, request: IChatRequestModel): Promise<void> {
throw new Error('Method not implemented.');
}
removeRequest(sessionid: string, requestId: string): Promise<void> {
throw new Error('Method not implemented.');
}
Expand Down
13 changes: 7 additions & 6 deletions src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { registerIcon } from 'vs/platform/theme/common/iconRegistry';
import { IPreferencesService } from 'vs/workbench/services/preferences/common/preferences';
import { ILogService } from 'vs/platform/log/common/log';
import { IChatService } from 'vs/workbench/contrib/chat/common/chatService';
import { CONTEXT_CHAT_REQUEST_IN_PROGRESS } from 'vs/workbench/contrib/chat/common/chatContextKeys';

CommandsRegistry.registerCommandAlias('interactiveEditor.start', 'inlineChat.start');
CommandsRegistry.registerCommandAlias('interactive.acceptChanges', ACTION_ACCEPT_CHANGES);
Expand Down Expand Up @@ -482,15 +483,14 @@ export class ViewInChatAction extends AbstractInlineChatAction {
icon: Codicon.commentDiscussion,
precondition: CTX_INLINE_CHAT_VISIBLE,
menu: {
id: MENU_INLINE_CHAT_WIDGET_STATUS,
when: CTX_INLINE_CHAT_RESPONSE_TYPES.isEqualTo(InlineChatResponseTypes.OnlyMessages),
group: '0_main',
order: 1
id: MENU_INLINE_CHAT_WIDGET,
group: 'navigation',
order: 5
}
});
}
override runInlineChatCommand(_accessor: ServicesAccessor, ctrl: InlineChatController, _editor: ICodeEditor, ..._args: any[]): void {
ctrl.viewInChat();
override runInlineChatCommand(_accessor: ServicesAccessor, ctrl: InlineChatController, _editor: ICodeEditor, ..._args: any[]) {
return ctrl.viewInChat();
}
}

Expand All @@ -501,6 +501,7 @@ export class RerunAction extends AbstractInlineChatAction {
title: localize2('chat.rerun.label', "Rerun Request"),
f1: false,
icon: Codicon.refresh,
precondition: CONTEXT_CHAT_REQUEST_IN_PROGRESS.negate(),
menu: {
id: MENU_INLINE_CHAT_WIDGET_STATUS,
group: '0_main',
Expand Down
Loading

0 comments on commit de93a5e

Please sign in to comment.