Skip to content

Commit e1526ec

Browse files
authored
Handle no modification scenario in chat editing (microsoft#239207)
* Remove/dispose modified files if they turn out to not contain modifications * Only start only accept flow when done streaming, reset when no edits come along fixes microsoft/vscode-copilot#10413
1 parent 76bfc91 commit e1526ec

File tree

3 files changed

+41
-23
lines changed

3 files changed

+41
-23
lines changed

src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedFileEntry.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -293,16 +293,12 @@ export class ChatEditingModifiedFileEntry extends Disposable implements IModifie
293293

294294
acceptStreamingEditsStart(tx: ITransaction) {
295295
this._resetEditsState(tx);
296+
this._autoAcceptCtrl.get()?.cancel();
296297
}
297298

298-
acceptStreamingEditsEnd(tx: ITransaction) {
299+
async acceptStreamingEditsEnd(tx: ITransaction) {
299300
this._resetEditsState(tx);
300-
}
301-
302-
private _resetEditsState(tx: ITransaction): void {
303-
this._isCurrentlyBeingModifiedObs.set(false, tx);
304-
this._rewriteRatioObs.set(0, tx);
305-
this._clearCurrentEditLineDecoration();
301+
await this._diffOperation;
306302

307303
// AUTO accept mode
308304
if (!this.reviewMode.get() && !this._autoAcceptCtrl.get()) {
@@ -333,6 +329,12 @@ export class ChatEditingModifiedFileEntry extends Disposable implements IModifie
333329
}
334330
}
335331

332+
private _resetEditsState(tx: ITransaction): void {
333+
this._isCurrentlyBeingModifiedObs.set(false, tx);
334+
this._rewriteRatioObs.set(0, tx);
335+
this._clearCurrentEditLineDecoration();
336+
}
337+
336338
private _mirrorEdits(event: IModelContentChangedEvent) {
337339
const edit = OffsetEdits.fromContentChanges(event.changes);
338340

src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { StringSHA1 } from '../../../../../base/common/hash.js';
1111
import { Disposable, DisposableMap, DisposableStore, dispose } from '../../../../../base/common/lifecycle.js';
1212
import { ResourceMap, ResourceSet } from '../../../../../base/common/map.js';
1313
import { Schemas } from '../../../../../base/common/network.js';
14-
import { autorun, derived, IObservable, IReader, ITransaction, observableValue, transaction } from '../../../../../base/common/observable.js';
14+
import { asyncTransaction, autorun, derived, IObservable, IReader, ITransaction, observableValue, transaction } from '../../../../../base/common/observable.js';
1515
import { autorunDelta, autorunIterableDelta } from '../../../../../base/common/observableInternal/autorun.js';
1616
import { isEqual, joinPath } from '../../../../../base/common/resources.js';
1717
import { URI } from '../../../../../base/common/uri.js';
@@ -735,9 +735,22 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio
735735
}
736736

737737
private async _resolve(): Promise<void> {
738-
transaction((tx) => {
738+
await asyncTransaction(async (tx) => {
739+
740+
const entriesWithoutChange = new ResourceMap<ChatEditingModifiedFileEntry>();
741+
739742
for (const entry of this._entriesObs.get()) {
740-
entry.acceptStreamingEditsEnd(tx);
743+
await entry.acceptStreamingEditsEnd(tx);
744+
if (entry.diffInfo.get().identical) {
745+
entriesWithoutChange.set(entry.modifiedURI, entry);
746+
}
747+
}
748+
749+
if (entriesWithoutChange.size > 0) {
750+
const newEntries = this._entriesObs.get().filter(e => !entriesWithoutChange.has(e.modifiedURI));
751+
this._entriesObs.set(newEntries, tx);
752+
753+
dispose(entriesWithoutChange.values());
741754
}
742755
this._state.set(ChatEditingSessionState.Idle, tx);
743756
});

src/vs/workbench/contrib/chat/browser/chatEditorController.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import { ChatAgentLocation, IChatAgentService } from '../common/chatAgents.js';
3737
import { EditorsOrder, IEditorIdentifier, isDiffEditorInput } from '../../../common/editor.js';
3838
import { ChatEditorOverlayController } from './chatEditorOverlay.js';
3939
import { IChatService } from '../common/chatService.js';
40+
import { StableEditorScrollState } from '../../../../editor/browser/stableEditorScroll.js';
4041

4142
export const ctxIsGlobalEditingSession = new RawContextKey<boolean>('chat.isGlobalEditingSession', undefined, localize('chat.ctxEditSessionIsGlobal', "The current editor is part of the global edit session"));
4243
export const ctxHasEditorModification = new RawContextKey<boolean>('chat.hasEditorModifications', undefined, localize('chat.hasEditorModifications', "The current editor contains chat modifications"));
@@ -114,9 +115,7 @@ export class ChatEditorController extends Disposable implements IEditorContribut
114115

115116
for (const session of _chatEditingService.editingSessionsObs.read(r)) {
116117
const entries = session.entries.read(r);
117-
const idx = model?.uri
118-
? entries.findIndex(e => isEqual(e.modifiedURI, model.uri))
119-
: -1;
118+
const idx = entries.findIndex(e => isEqual(e.modifiedURI, model.uri));
120119

121120
const chatModel = chatService.getSession(session.chatSessionId);
122121

@@ -129,7 +128,7 @@ export class ChatEditorController extends Disposable implements IEditorContribut
129128
});
130129

131130

132-
let didReval = false;
131+
let scrollState: StableEditorScrollState | undefined = undefined;
133132

134133
this._register(autorunWithStore((r, store) => {
135134

@@ -138,7 +137,7 @@ export class ChatEditorController extends Disposable implements IEditorContribut
138137
if (!currentEditorEntry) {
139138
this._ctxIsGlobalEditsSession.reset();
140139
this._clear();
141-
didReval = false;
140+
scrollState = undefined;
142141
return;
143142
}
144143

@@ -149,10 +148,10 @@ export class ChatEditorController extends Disposable implements IEditorContribut
149148

150149
const { session, chatModel, entries, idx, entry } = currentEditorEntry;
151150

152-
store.add(chatModel.onDidChange(e => {
153-
if (e.kind === 'addRequest') {
154-
didReval = false;
155-
}
151+
const lastRequestSignal = observableFromEvent(this, chatModel.onDidChange, () => chatModel.getRequests().at(-1));
152+
store.add(autorun(r => {
153+
lastRequestSignal.read(r);
154+
scrollState ??= StableEditorScrollState.capture(this._editor);
156155
}));
157156

158157
this._ctxIsGlobalEditsSession.set(session.isGlobalEditingSession);
@@ -193,7 +192,7 @@ export class ChatEditorController extends Disposable implements IEditorContribut
193192
fontInfoObs.read(r);
194193
lineHeightObs.read(r);
195194

196-
const diff = entry?.diffInfo.read(r);
195+
const diff = entry.diffInfo.read(r);
197196

198197
// Add line decorations (just markers, no UI) for diff navigation
199198
this._updateDiffLineDecorations(diff);
@@ -207,9 +206,13 @@ export class ChatEditorController extends Disposable implements IEditorContribut
207206
this._clearDiffRendering();
208207
}
209208

210-
if (!didReval && !diff.identical) {
211-
didReval = true;
212-
this._reveal(true, false, ScrollType.Immediate);
209+
if (lastRequestSignal.read(r)?.response?.isComplete) {
210+
if (!diff.identical) {
211+
this._reveal(true, false, ScrollType.Immediate);
212+
} else {
213+
scrollState?.restore(_editor);
214+
scrollState = undefined;
215+
}
213216
}
214217
}
215218
}));

0 commit comments

Comments
 (0)