diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts index 7ccb6a71164bb..259fde69a8404 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts @@ -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(); + } + } } })); @@ -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; } diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts index c4ecc2652eb44..b360c83a016d2 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts @@ -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]; } @@ -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; } diff --git a/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts b/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts index f88d7a9904249..5742290cc5a2a 100644 --- a/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts +++ b/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts @@ -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'; @@ -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'; @@ -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 () { @@ -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( @@ -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(id: string, focus?: boolean | undefined): Promise { + return { widget: chatWidget ?? null } as any; + } + }()], [IWorkspaceContextService, new TestContextService()], [IChatWidgetHistoryService, new SyncDescriptor(ChatWidgetHistoryService)], [IChatWidgetService, new SyncDescriptor(ChatWidgetService)], @@ -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, }, { @@ -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() { + 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() { + 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; + }); });