From c27f01a83931825800a9104235a066bc74b0f191 Mon Sep 17 00:00:00 2001 From: Johannes Date: Tue, 1 Apr 2025 10:42:04 +0200 Subject: [PATCH 1/3] clean up working set remains --- .../browser/chatEditing/chatEditingActions.ts | 6 +++--- .../chatEditingModifiedFileEntry.ts | 2 +- .../browser/chatEditing/chatEditingSession.ts | 19 ++++--------------- .../contrib/chat/common/chatEditingService.ts | 10 +--------- 4 files changed, 9 insertions(+), 28 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingActions.ts b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingActions.ts index 703e79b9eaa..6258f998f34 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingActions.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingActions.ts @@ -28,7 +28,7 @@ import { GroupsOrder, IEditorGroupsService } from '../../../../services/editor/c import { IEditorService } from '../../../../services/editor/common/editorService.js'; import { isChatViewTitleActionContext } from '../../common/chatActions.js'; import { ChatContextKeys } from '../../common/chatContextKeys.js'; -import { applyingChatEditsFailedContextKey, CHAT_EDITING_MULTI_DIFF_SOURCE_RESOLVER_SCHEME, chatEditingResourceContextKey, chatEditingWidgetFileStateContextKey, decidedChatEditingResourceContextKey, hasAppliedChatEditsContextKey, hasUndecidedChatEditingResourceContextKey, IChatEditingService, IChatEditingSession, WorkingSetEntryRemovalReason, ModifiedFileEntryState } from '../../common/chatEditingService.js'; +import { applyingChatEditsFailedContextKey, CHAT_EDITING_MULTI_DIFF_SOURCE_RESOLVER_SCHEME, chatEditingResourceContextKey, chatEditingWidgetFileStateContextKey, decidedChatEditingResourceContextKey, hasAppliedChatEditsContextKey, hasUndecidedChatEditingResourceContextKey, IChatEditingService, IChatEditingSession, ModifiedFileEntryState } from '../../common/chatEditingService.js'; import { IChatService } from '../../common/chatService.js'; import { isRequestVM, isResponseVM } from '../../common/chatViewModel.js'; import { ChatAgentLocation, ChatMode } from '../../common/constants.js'; @@ -146,7 +146,7 @@ registerAction2(class RemoveFileFromWorkingSet extends WorkingSetAction { // Remove from working set await currentEditingSession.reject(...uris); - currentEditingSession.remove(WorkingSetEntryRemovalReason.User, ...uris); + currentEditingSession.remove(...uris); // Remove from chat input part for (const uri of uris) { @@ -354,7 +354,7 @@ export class ChatEditingRemoveAllFilesAction extends EditingSessionAction { override async runEditingSessionAction(accessor: ServicesAccessor, editingSession: IChatEditingSession, chatWidget: IChatWidget, ...args: any[]): Promise { // Remove all files from working set const uris = [...editingSession.entries.get()].map((e) => e.modifiedURI); - editingSession.remove(WorkingSetEntryRemovalReason.User, ...uris); + editingSession.remove(...uris); // Remove all file attachments const fileAttachments = chatWidget.attachmentModel ? chatWidget.attachmentModel.fileAttachments : []; diff --git a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedFileEntry.ts b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedFileEntry.ts index cc693bb44fe..44cfbfa8293 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedFileEntry.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedFileEntry.ts @@ -50,7 +50,7 @@ export abstract class AbstractChatEditingModifiedFileEntry extends Disposable im protected readonly _onDidDelete = this._register(new Emitter()); readonly onDidDelete = this._onDidDelete.event; - protected readonly _stateObs = observableValue(this, ModifiedFileEntryState.Attached); + protected readonly _stateObs = observableValue(this, ModifiedFileEntryState.Modified); readonly state: IObservable = this._stateObs; protected readonly _isCurrentlyBeingModifiedByObs = observableValue(this, undefined); diff --git a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts index 52e4b6ed79b..18e58467a2f 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts @@ -41,7 +41,7 @@ import { MultiDiffEditor } from '../../../multiDiffEditor/browser/multiDiffEdito import { MultiDiffEditorInput } from '../../../multiDiffEditor/browser/multiDiffEditorInput.js'; import { CellUri, ICellEditOperation } from '../../../notebook/common/notebookCommon.js'; import { INotebookService } from '../../../notebook/common/notebookService.js'; -import { ChatEditingSessionChangeType, ChatEditingSessionState, ChatEditKind, getMultiDiffSourceUri, IChatEditingSession, IEditSessionEntryDiff, IModifiedFileEntry, IStreamingEdits, ModifiedFileEntryState, WorkingSetDisplayMetadata, WorkingSetEntryRemovalReason } from '../../common/chatEditingService.js'; +import { ChatEditingSessionChangeType, ChatEditingSessionState, ChatEditKind, getMultiDiffSourceUri, IChatEditingSession, IEditSessionEntryDiff, IModifiedFileEntry, IStreamingEdits, ModifiedFileEntryState, WorkingSetDisplayMetadata } from '../../common/chatEditingService.js'; import { ICellTextEditOperation, IChatRequestDisablement, IChatResponseModel, isCellTextEditOperation } from '../../common/chatModel.js'; import { IChatService } from '../../common/chatService.js'; import { ChatEditingModifiedDocumentEntry } from './chatEditingModifiedDocumentEntry.js'; @@ -138,8 +138,6 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio return this._entriesObs; } - private _workingSet = new ResourceMap(); - private _editorPane: MultiDiffEditor | undefined; get state(): IObservable { @@ -384,9 +382,6 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio public createSnapshot(requestId: string, undoStop: string | undefined): void { const snapshot = this._createSnapshot(requestId, undoStop); - for (const [uri, _] of this._workingSet) { - this._workingSet.set(uri, { state: ModifiedFileEntryState.Sent }); - } const linearHistoryPtr = this._linearHistoryIndex.get(); const newLinearHistory: IChatEditingSessionSnapshot[] = []; @@ -413,7 +408,7 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio } private _createSnapshot(requestId: string | undefined, undoStop: string | undefined): IChatEditingSessionStop { - const workingSet = new ResourceMap(this._workingSet); + const entries = new ResourceMap(); for (const entry of this._entriesObs.get()) { entries.set(entry.modifiedURI, entry.createSnapshot(requestId, undoStop)); @@ -421,7 +416,7 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio return { stopId: undoStop, - workingSet, + workingSet: new ResourceMap(), entries, }; } @@ -473,7 +468,6 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio } private async _restoreSnapshot({ workingSet, entries }: IChatEditingSessionStop, tx: ITransaction | undefined, restoreResolvedToDisk = true): Promise { - this._workingSet = new ResourceMap(workingSet); // Reset all the files which are modified in this session state // but which are not found in the snapshot @@ -497,7 +491,7 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio this._entriesObs.set(entriesArr, tx); } - remove(reason: WorkingSetEntryRemovalReason, ...uris: URI[]): void { + remove(...uris: URI[]): void { this._assertNotDisposed(); let didRemoveUris = false; @@ -511,10 +505,6 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio didRemoveUris = true; } - const state = this._workingSet.get(uri); - if (state !== undefined) { - didRemoveUris = this._workingSet.delete(uri) || didRemoveUris; - } } if (!didRemoveUris) { @@ -905,7 +895,6 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio const listener = entry.onDidDelete(() => { const newEntries = this._entriesObs.get().filter(e => !isEqual(e.modifiedURI, entry.modifiedURI)); this._entriesObs.set(newEntries, undefined); - this._workingSet.delete(entry.modifiedURI); this._editorService.closeEditors(this._editorService.findEditors(entry.modifiedURI)); if (!existingExternalEntry) { diff --git a/src/vs/workbench/contrib/chat/common/chatEditingService.ts b/src/vs/workbench/contrib/chat/common/chatEditingService.ts index 01c4725e3d9..7c2102f4761 100644 --- a/src/vs/workbench/contrib/chat/common/chatEditingService.ts +++ b/src/vs/workbench/contrib/chat/common/chatEditingService.ts @@ -85,7 +85,7 @@ export interface IChatEditingSession extends IDisposable { readonly state: IObservable; readonly entries: IObservable; show(): Promise; - remove(reason: WorkingSetEntryRemovalReason, ...uris: URI[]): void; + remove(...uris: URI[]): void; accept(...uris: URI[]): Promise; reject(...uris: URI[]): Promise; getEntry(uri: URI): IModifiedFileEntry | undefined; @@ -140,18 +140,10 @@ export interface IEditSessionEntryDiff { removed: number; } -export const enum WorkingSetEntryRemovalReason { - User, - Programmatic -} - export const enum ModifiedFileEntryState { Modified, Accepted, Rejected, - - Attached, // TODO@joyceerhl remove this - Sent, // TODO@joyceerhl remove this } export const enum ChatEditingSessionChangeType { From c00cf036f1332b727fbfda3e558ae7140bb9abbc Mon Sep 17 00:00:00 2001 From: Johannes Date: Tue, 1 Apr 2025 10:50:57 +0200 Subject: [PATCH 2/3] more workingSet removal --- .../browser/chatEditing/chatEditingSession.ts | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts index 18e58467a2f..03d5d2bfc7d 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts @@ -416,7 +416,6 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio return { stopId: undoStop, - workingSet: new ResourceMap(), entries, }; } @@ -467,7 +466,7 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio } } - private async _restoreSnapshot({ workingSet, entries }: IChatEditingSessionStop, tx: ITransaction | undefined, restoreResolvedToDisk = true): Promise { + private async _restoreSnapshot({ entries }: IChatEditingSessionStop, tx: ITransaction | undefined, restoreResolvedToDisk = true): Promise { // Reset all the files which are modified in this session state // but which are not found in the snapshot @@ -985,12 +984,6 @@ class ChatEditingSessionStorage { } return readPromise; }; - const deserializeResourceMap = (resourceMap: ResourceMapDTO, deserialize: (value: any) => T, result: ResourceMap): ResourceMap => { - resourceMap.forEach(([resourceURI, value]) => { - result.set(URI.parse(resourceURI), deserialize(value)); - }); - return result; - }; const deserializeSnapshotEntriesDTO = async (dtoEntries: ISnapshotEntryDTO[]): Promise> => { const entries = new ResourceMap(); for (const entryDTO of dtoEntries) { @@ -1001,14 +994,13 @@ class ChatEditingSessionStorage { }; const deserializeChatEditingStopDTO = async (stopDTO: IChatEditingSessionStopDTO | IChatEditingSessionSnapshotDTO): Promise => { const entries = await deserializeSnapshotEntriesDTO(stopDTO.entries); - const workingSet = deserializeResourceMap(stopDTO.workingSet, (value) => value, new ResourceMap()); - return { stopId: 'stopId' in stopDTO ? stopDTO.stopId : undefined, workingSet, entries }; + return { stopId: 'stopId' in stopDTO ? stopDTO.stopId : undefined, entries }; }; const normalizeSnapshotDtos = (snapshot: IChatEditingSessionSnapshotDTO | IChatEditingSessionSnapshotDTO2): IChatEditingSessionSnapshotDTO2 => { if ('stops' in snapshot) { return snapshot; } - return { requestId: snapshot.requestId, stops: [{ stopId: undefined, entries: snapshot.entries, workingSet: snapshot.workingSet }], postEdit: undefined }; + return { requestId: snapshot.requestId, stops: [{ stopId: undefined, entries: snapshot.entries }], postEdit: undefined }; }; const deserializeChatEditingSessionSnapshot = async (startIndex: number, snapshot: IChatEditingSessionSnapshotDTO2): Promise => { const stops = await Promise.all(snapshot.stops.map(deserializeChatEditingStopDTO)); @@ -1104,7 +1096,6 @@ class ChatEditingSessionStorage { const serializeChatEditingSessionStop = (stop: IChatEditingSessionStop): IChatEditingSessionStopDTO => { return { stopId: stop.stopId, - workingSet: serializeResourceMap(stop.workingSet, value => value), entries: Array.from(stop.entries.values()).map(serializeSnapshotEntry) }; }; @@ -1190,13 +1181,11 @@ interface IChatEditingSessionStop { /** Edit stop ID, first for a request is always undefined. */ stopId: string | undefined; - readonly workingSet: ResourceMap; readonly entries: ResourceMap; } interface IChatEditingSessionStopDTO { readonly stopId: string | undefined; - readonly workingSet: ResourceMapDTO; readonly entries: ISnapshotEntryDTO[]; } From 8db7ee64cc2ee281771dd2e98b569eb27715717f Mon Sep 17 00:00:00 2001 From: Johannes Date: Tue, 1 Apr 2025 11:48:22 +0200 Subject: [PATCH 3/3] document entry cannot assume it's always back by something on disk --- .../chatEditingModifiedDocumentEntry.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedDocumentEntry.ts b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedDocumentEntry.ts index 2bb67456864..876e96165ba 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedDocumentEntry.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedDocumentEntry.ts @@ -38,7 +38,7 @@ import { editorSelectionBackground } from '../../../../../platform/theme/common/ import { IUndoRedoElement, IUndoRedoService } from '../../../../../platform/undoRedo/common/undoRedo.js'; import { SaveReason, IEditorPane } from '../../../../common/editor.js'; import { IFilesConfigurationService } from '../../../../services/filesConfiguration/common/filesConfigurationService.js'; -import { IResolvedTextFileEditorModel, ITextFileService, stringToSnapshot } from '../../../../services/textfile/common/textfiles.js'; +import { isTextFileEditorModel, ITextFileService, stringToSnapshot } from '../../../../services/textfile/common/textfiles.js'; import { ICellEditOperation } from '../../../notebook/common/notebookCommon.js'; import { IModifiedFileEntry, ChatEditKind, ModifiedFileEntryState, IModifiedFileEntryEditorIntegration } from '../../common/chatEditingService.js'; import { IChatResponseModel } from '../../common/chatModel.js'; @@ -76,7 +76,7 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie private readonly originalModel: ITextModel; private readonly modifiedModel: ITextModel; - readonly docFileEditorModel: IResolvedTextFileEditorModel; + private readonly _docFileEditorModel: IResolvedTextEditorModel; private _edit: OffsetEdit = OffsetEdit.empty; private _isEditFromUs: boolean = false; @@ -128,7 +128,7 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie instantiationService ); - this.docFileEditorModel = this._register(resourceRef).object as IResolvedTextFileEditorModel; + this._docFileEditorModel = this._register(resourceRef).object; this.modifiedModel = resourceRef.object.textEditorModel; this.originalURI = ChatEditingTextModelContentProvider.getFileURI(telemetryInfo.sessionId, this.entryId, this.modifiedURI.path); @@ -451,15 +451,17 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie protected override async _doReject(tx: ITransaction | undefined): Promise { if (this.createdInRequestId === this._telemetryInfo.requestId) { - await this.docFileEditorModel.revert({ soft: true }); - await this._fileService.del(this.modifiedURI); + if (isTextFileEditorModel(this._docFileEditorModel)) { + await this._docFileEditorModel.revert({ soft: true }); + await this._fileService.del(this.modifiedURI); + } this._onDidDelete.fire(); } else { this._setDocValue(this.originalModel.getValue()); - if (this._allEditsAreFromUs) { + if (this._allEditsAreFromUs && isTextFileEditorModel(this._docFileEditorModel)) { // save the file after discarding so that the dirty indicator goes away // and so that an intermediate saved state gets reverted - await this.docFileEditorModel.save({ reason: SaveReason.EXPLICIT, skipSaveParticipants: true }); + await this._docFileEditorModel.save({ reason: SaveReason.EXPLICIT, skipSaveParticipants: true }); } await this._collapse(tx); }