Skip to content

Commit

Permalink
On request-removal undo till the point at which the request was made (#…
Browse files Browse the repository at this point in the history 
…213902)

* On request-removal undo till the point at which the request was made

fixes microsoft/vscode-copilot#5736

* fixed EOL sequence which (hopefully) fixes tests

* give up, don't use new lines...
  • Loading branch information
@jrieken
jrieken authored May 30, 2024
1 parent e545871 commit 4dde670
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 11 deletions.
17 changes: 13 additions & 4 deletions src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,9 +439,18 @@ export class InlineChatController implements IEditorContribution {
}
});
} else if (e.kind === 'removeRequest') {
// TODO@jrieken this is buggy/weird when having code changes from multiple turns because
// logically they are all the same hunks
this._strategy?.cancel();
// TODO@jrieken there is still some work left for when a request "in the middle"
// is removed. We will undo all changes till that point but not remove those
// later request
const exchange = this._session!.exchanges.find(candidate => candidate.prompt.request.id === e.requestId);
if (exchange && this._editor.hasModel()) {
// undo till this point
const model = this._editor.getModel();
const targetAltVersion = exchange.prompt.modelAltVersionId;
while (targetAltVersion < model.getAlternativeVersionId() && model.canUndo()) {
await model.undo();
}
}
}
}));

Expand Down Expand Up @@ -601,7 +610,7 @@ export class InlineChatController implements IEditorContribution {
const input = request.message.text;
this._ui.value.zone.widget.value = input;

this._session.addInput(new SessionPrompt(request));
this._session.addInput(new SessionPrompt(request, this._editor.getModel()!.getAlternativeVersionId()));

return State.SHOW_REQUEST;
}
Expand Down
7 changes: 6 additions & 1 deletion src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ export class Session {
// this._teldata.responseTypes += `${exchange.response instanceof ReplyResponse ? exchange.response.responseType : InlineChatResponseTypes.Empty}|`;
}

get exchanges(): readonly SessionExchange[] {
return this._exchange;
}

get lastExchange(): SessionExchange | undefined {
return this._exchange[this._exchange.length - 1];
}
Expand Down Expand Up @@ -273,7 +277,8 @@ export class SessionPrompt {
readonly value: string;

constructor(
readonly request: IChatRequestModel
readonly request: IChatRequestModel,
readonly modelAltVersionId: number,
) {
this.value = request.message.text;
}
Expand Down
160 changes: 154 additions & 6 deletions src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { IActiveCodeEditor } from 'vs/editor/browser/editorBrowser';
import { IDiffProviderFactoryService } from 'vs/editor/browser/widget/diffEditor/diffProviderFactoryService';
import { EditOperation } from 'vs/editor/common/core/editOperation';
import { Range } from 'vs/editor/common/core/range';
import { ITextModel } from 'vs/editor/common/model';
import { EndOfLineSequence, ITextModel } from 'vs/editor/common/model';
import { IEditorWorkerService } from 'vs/editor/common/services/editorWorker';
import { IModelService } from 'vs/editor/common/services/model';
import { TestDiffProviderFactoryService } from 'vs/editor/test/browser/diff/testDiffProviderFactoryService';
Expand All @@ -27,16 +27,16 @@ import { ServiceCollection } from 'vs/platform/instantiation/common/serviceColle
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
import { IEditorProgressService, IProgressRunner } from 'vs/platform/progress/common/progress';
import { IViewDescriptorService } from 'vs/workbench/common/views';
import { IView, IViewDescriptorService } from 'vs/workbench/common/views';
import { AccessibilityVerbositySettingId } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration';
import { IAccessibleViewService } from 'vs/platform/accessibility/browser/accessibleView';
import { IChatAccessibilityService, IChatWidgetService } from 'vs/workbench/contrib/chat/browser/chat';
import { IChatAccessibilityService, IChatWidget, IChatWidgetService } from 'vs/workbench/contrib/chat/browser/chat';
import { ChatAgentLocation, ChatAgentService, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
import { IChatResponseViewModel } from 'vs/workbench/contrib/chat/common/chatViewModel';
import { InlineChatController, InlineChatRunOptions, State } from 'vs/workbench/contrib/inlineChat/browser/inlineChatController';
import { Session } from 'vs/workbench/contrib/inlineChat/browser/inlineChatSession';
import { CTX_INLINE_CHAT_USER_DID_EDIT, EditMode, InlineChatConfigKeys } from 'vs/workbench/contrib/inlineChat/common/inlineChat';
import { workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
import { TestViewsService, workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
import { IInlineChatSavingService } from '../../browser/inlineChatSavingService';
import { IInlineChatSessionService } from '../../browser/inlineChatSessionService';
import { InlineChatSessionServiceImpl } from '../../browser/inlineChatSessionServiceImpl';
Expand All @@ -61,6 +61,7 @@ import { ICommandService } from 'vs/platform/commands/common/commands';
import { TestCommandService } from 'vs/editor/test/browser/editorTestServices';
import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/services/notebookEditorService';
import { RerunAction } from 'vs/workbench/contrib/inlineChat/browser/inlineChatActions';
import { CancellationToken } from 'vs/base/common/cancellation';

suite('InteractiveChatController', function () {

Expand Down Expand Up @@ -127,10 +128,13 @@ suite('InteractiveChatController', function () {
let model: ITextModel;
let ctrl: TestController;
let contextKeyService: MockContextKeyService;
let chatService: IChatService;
let chatAgentService: IChatAgentService;
let inlineChatSessionService: IInlineChatSessionService;
let instaService: TestInstantiationService;

let chatWidget: IChatWidget;

setup(function () {

const serviceCollection = new ServiceCollection(
Expand All @@ -141,7 +145,11 @@ suite('InteractiveChatController', function () {
[IHoverService, NullHoverService],
[IExtensionService, new TestExtensionService()],
[IContextKeyService, new MockContextKeyService()],
[IViewsService, new TestExtensionService()],
[IViewsService, new class extends TestViewsService {
override async openView<T extends IView>(id: string, focus?: boolean | undefined): Promise<T | null> {
return { widget: chatWidget ?? null } as any;
}
}()],
[IWorkspaceContextService, new TestContextService()],
[IChatWidgetHistoryService, new SyncDescriptor(ChatWidgetHistoryService)],
[IChatWidgetService, new SyncDescriptor(ChatWidgetService)],
Expand Down Expand Up @@ -193,12 +201,13 @@ suite('InteractiveChatController', function () {
configurationService.setUserConfiguration('editor', {});

contextKeyService = instaService.get(IContextKeyService) as MockContextKeyService;

chatService = instaService.get(IChatService);
chatAgentService = instaService.get(IChatAgentService);

inlineChatSessionService = store.add(instaService.get(IInlineChatSessionService));

model = store.add(instaService.get(IModelService).createModel('Hello\nWorld\nHello Again\nHello World\n', null));
model.setEOL(EndOfLineSequence.LF);
editor = store.add(instantiateTestCodeEditor(instaService, model));

store.add(chatAgentService.registerDynamicAgent({ id: 'testEditorAgent', ...agentData, }, {
Expand Down Expand Up @@ -494,4 +503,143 @@ suite('InteractiveChatController', function () {
ctrl.finishExistingSession();
await r;
});

test('Retry undoes all changes, not just those from the request#5736', async function () {

const text = [
'eins-',
'zwei-',
'drei-'
];

store.add(chatAgentService.registerDynamicAgent({
id: 'testEditorAgent2',
...agentData
}, {
async invoke(request, progress, history, token) {
progress({ kind: 'textEdit', uri: model.uri, edits: [{ range: new Range(1, 1, 1, 1), text: text.shift() ?? '' }] });
return {};
},
}));

ctrl = instaService.createInstance(TestController, editor);
const rerun = new RerunAction();

model.setValue('');

// REQUEST 1
const p = ctrl.waitFor([...TestController.INIT_SEQUENCE, State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
const r = ctrl.run({ message: '1', autoSend: true });
await p;

assert.strictEqual(model.getValue(), 'eins-');

// REQUEST 2
const p2 = ctrl.waitFor([State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
await ctrl.acceptInput();
await p2;

assert.strictEqual(model.getValue(), 'zwei-eins-');

// REQUEST 2 - RERUN
const p3 = ctrl.waitFor([State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
await instaService.invokeFunction(rerun.runInlineChatCommand, ctrl, editor);
await p3;

assert.strictEqual(model.getValue(), 'drei-eins-');

ctrl.finishExistingSession();
await r;

});

test('moving inline chat to another model undoes changes', async function () {
const text = [
'eins\n',
'zwei\n'
];

store.add(chatAgentService.registerDynamicAgent({
id: 'testEditorAgent2',
...agentData
}, {
async invoke(request, progress, history, token) {
progress({ kind: 'textEdit', uri: model.uri, edits: [{ range: new Range(1, 1, 1, 1), text: text.shift() ?? '' }] });
return {};
},
}));
ctrl = instaService.createInstance(TestController, editor);

// REQUEST 1
const p = ctrl.waitFor([...TestController.INIT_SEQUENCE, State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
ctrl.run({ message: '1', autoSend: true });
await p;


assert.strictEqual(model.getValue(), 'eins\nHello\nWorld\nHello Again\nHello World\n');

const targetModel = chatService.startSession(ChatAgentLocation.Editor, CancellationToken.None)!;
store.add(targetModel);
chatWidget = new class extends mock<IChatWidget>() {
override get viewModel() {
return { model: targetModel } as any;
}
override focusLastMessage() { }
};

const r = ctrl.joinCurrentRun();
await ctrl.viewInChat();

assert.strictEqual(model.getValue(), 'Hello\nWorld\nHello Again\nHello World\n');
await r;
});

test('moving inline chat to another model undoes changes (2 requests)', async function () {
const text = [
'eins\n',
'zwei\n'
];

store.add(chatAgentService.registerDynamicAgent({
id: 'testEditorAgent2',
...agentData
}, {
async invoke(request, progress, history, token) {
progress({ kind: 'textEdit', uri: model.uri, edits: [{ range: new Range(1, 1, 1, 1), text: text.shift() ?? '' }] });
return {};
},
}));
ctrl = instaService.createInstance(TestController, editor);

// REQUEST 1
const p = ctrl.waitFor([...TestController.INIT_SEQUENCE, State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
ctrl.run({ message: '1', autoSend: true });
await p;

assert.strictEqual(model.getValue(), 'eins\nHello\nWorld\nHello Again\nHello World\n');

// REQUEST 2
const p2 = ctrl.waitFor([State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
await ctrl.acceptInput();
await p2;

assert.strictEqual(model.getValue(), 'zwei\neins\nHello\nWorld\nHello Again\nHello World\n');

const targetModel = chatService.startSession(ChatAgentLocation.Editor, CancellationToken.None)!;
store.add(targetModel);
chatWidget = new class extends mock<IChatWidget>() {
override get viewModel() {
return { model: targetModel } as any;
}
override focusLastMessage() { }
};

const r = ctrl.joinCurrentRun();

await ctrl.viewInChat();

assert.strictEqual(model.getValue(), 'Hello\nWorld\nHello Again\nHello World\n');

await r;
});
});

0 comments on commit 4dde670

Please sign in to comment.