no undoStops between streaming edits, esp not between last and second to last round (microsoft#196708)

fixes microsoft/vscode-copilot#2403

Author: jrieken

src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts

@@ -596,7 +596,6 @@ export class InlineChatController implements IEditorContribution {
 		const progressiveEditsCts = new CancellationTokenSource(requestCts.token);
 		const progressiveEditsClock = StopWatch.create();
 		const progressiveEditsQueue = new Queue();
-		let round = 0;
 
 		const progress = new AsyncProgress<IInlineChatProgressItem>(async data => {
 			this._log('received chunk', data, request);
@@ -632,7 +631,7 @@ export class InlineChatController implements IEditorContribution {
 					// become infinitely fast
 					await this._makeChanges(data.edits!, data.editsShouldBeInstant
 						? undefined
-						: { duration: progressiveEditsAvgDuration.value, round: round++, token: progressiveEditsCts.token }
+						: { duration: progressiveEditsAvgDuration.value, token: progressiveEditsCts.token }
 					);
 
 					// reshow the widget if the start position changed or shows at the wrong position

src/vs/workbench/contrib/inlineChat/browser/inlineChatStrategies.ts

@@ -225,7 +225,6 @@ class InlineDiffDecorations {
 
 export interface ProgressingEditsOptions {
 	duration: number;
-	round: number;
 	token: CancellationToken;
 }
 
@@ -328,8 +327,9 @@ export class LiveStrategy extends EditModeStrategy {
 
 	override async makeProgressiveChanges(edits: ISingleEditOperation[], opts: ProgressingEditsOptions): Promise<void> {
 
-		if (opts.round === 0) {
-			this._session.textModelN.pushStackElement();
+		// push undo stop before first edit
+		if (++this._editCount === 1) {
+			this._editor.pushUndoStop();
 		}
 
 		const durationInSec = opts.duration / 1000;
@@ -591,14 +591,16 @@ export function asProgressiveEdit(edit: IIdentifiedSingleEditOperation, wordsPer
 		if (r.isFullString) {
 			clearInterval(handle);
 			stream.resolve();
+			d.dispose();
 		}
 
 	}, 1000 / wordsPerSec);
 
 	// cancel ASAP
-	token.onCancellationRequested(() => {
+	const d = token.onCancellationRequested(() => {
 		clearTimeout(handle);
 		stream.resolve();
+		d.dispose();
 	});
 
 	return {

src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts

@@ -10,7 +10,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle';
 import { mock } from 'vs/base/test/common/mock';
 import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
 import { TestDiffProviderFactoryService } from 'vs/editor/browser/diff/testDiffProviderFactoryService';
-import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
+import { IActiveCodeEditor } from 'vs/editor/browser/editorBrowser';
 import { IDiffProviderFactoryService } from 'vs/editor/browser/widget/diffEditor/diffProviderFactoryService';
 import { Range } from 'vs/editor/common/core/range';
 import { ITextModel } from 'vs/editor/common/model';
@@ -81,7 +81,7 @@ suite('InteractiveChatController', function () {
 	}
 
 	const store = new DisposableStore();
-	let editor: ICodeEditor;
+	let editor: IActiveCodeEditor;
 	let model: ITextModel;
 	let ctrl: TestController;
 	// let contextKeys: MockContextKeyService;
@@ -327,4 +327,44 @@ suite('InteractiveChatController', function () {
 		await r;
 		assert.strictEqual(ctrl.getWidgetPosition(), undefined);
 	});
+
+	test('[Bug] Inline Chat\'s streaming pushed broken iterations to the undo stack #2403', async function () {
+
+		const d = inlineChatService.addProvider({
+			debugName: 'Unit Test',
+			label: 'Unit Test',
+			prepareInlineChatSession() {
+				return {
+					id: Math.random(),
+					wholeRange: new Range(3, 1, 3, 3)
+				};
+			},
+			async provideResponse(session, request, progress) {
+
+				progress.report({ edits: [{ range: new Range(1, 1, 1, 1), text: 'hEllo1\n' }] });
+				progress.report({ edits: [{ range: new Range(2, 1, 2, 1), text: 'hEllo2\n' }] });
+
+				return {
+					id: Math.random(),
+					type: InlineChatResponseType.EditorEdit,
+					edits: [{ range: new Range(1, 1, 1000, 1), text: 'Hello1\nHello2\n' }]
+				};
+			}
+		});
+
+		const valueThen = editor.getModel().getValue();
+
+		store.add(d);
+		ctrl = instaService.createInstance(TestController, editor);
+		const p = ctrl.waitFor([...TestController.INIT_SEQUENCE, State.MAKE_REQUEST, State.APPLY_RESPONSE, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
+		const r = ctrl.run({ message: 'Hello', autoSend: true });
+		await p;
+		ctrl.acceptSession();
+		await r;
+
+		assert.strictEqual(editor.getModel().getValue(), 'Hello1\nHello2\n');
+
+		editor.getModel().undo();
+		assert.strictEqual(editor.getModel().getValue(), valueThen);
+	});
 });