Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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;
});
});
Loading