Skip to content

Commit

Permalink
edits context key cleanup (#239337)
Browse files Browse the repository at this point in the history 
* debt - remove `isApplyingChatEdits` context key, it is the same as "requestInProgress"

#238742

* rename to xyzServiceImpl

* move edits context keys into widget, not inside service

* fix init order bug
  • Loading branch information
@jrieken
jrieken authored Jan 31, 2025
1 parent 241c38e commit d04d2d3
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 161 deletions.
19 changes: 5 additions & 14 deletions src/vs/workbench/contrib/chat/browser/actions/chatExecuteActions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { KeybindingWeight } from '../../../../../platform/keybinding/common/keyb
import { IViewsService } from '../../../../services/views/common/viewsService.js';
import { ChatAgentLocation, IChatAgentService } from '../../common/chatAgents.js';
import { ChatContextKeys } from '../../common/chatContextKeys.js';
import { applyingChatEditsContextKey, IChatEditingService, WorkingSetEntryState } from '../../common/chatEditingService.js';
import { IChatEditingService, WorkingSetEntryState } from '../../common/chatEditingService.js';
import { chatAgentLeader, extractAgentAndCommand } from '../../common/chatParserTypes.js';
import { IChatService } from '../../common/chatService.js';
import { EditsViewId, IChatWidget, IChatWidgetService } from '../chat.js';
Expand Down Expand Up @@ -218,7 +218,7 @@ export class ChatEditingSessionSubmitAction extends SubmitAction {
// if the input has prompt instructions attached, allow submitting requests even
// without text present - having instructions is enough context for a request
ContextKeyExpr.or(ChatContextKeys.inputHasText, ChatContextKeys.instructionsAttached),
applyingChatEditsContextKey.toNegated(),
ChatContextKeys.requestInProgress.negate(),
ChatContextKeys.location.isEqualTo(ChatAgentLocation.EditingSession),
);

Expand All @@ -238,13 +238,13 @@ export class ChatEditingSessionSubmitAction extends SubmitAction {
{
id: MenuId.ChatExecuteSecondary,
group: 'group_1',
when: ContextKeyExpr.and(ContextKeyExpr.or(ChatContextKeys.isRequestPaused, ChatContextKeys.requestInProgress.negate()), ChatContextKeys.location.isEqualTo(ChatAgentLocation.EditingSession), applyingChatEditsContextKey.toNegated()),
when: ContextKeyExpr.and(ContextKeyExpr.or(ChatContextKeys.isRequestPaused, ChatContextKeys.requestInProgress.negate()), ChatContextKeys.location.isEqualTo(ChatAgentLocation.EditingSession)),
order: 1
},
{
id: MenuId.ChatExecute,
order: 4,
when: ContextKeyExpr.and(ContextKeyExpr.or(ChatContextKeys.isRequestPaused, ChatContextKeys.requestInProgress.negate()), ChatContextKeys.location.isEqualTo(ChatAgentLocation.EditingSession), applyingChatEditsContextKey.toNegated()),
when: ContextKeyExpr.and(ContextKeyExpr.or(ChatContextKeys.isRequestPaused, ChatContextKeys.requestInProgress.negate()), ChatContextKeys.location.isEqualTo(ChatAgentLocation.EditingSession)),
group: 'navigation',
},
]
Expand Down Expand Up @@ -517,10 +517,7 @@ export class CancelAction extends Action2 {
icon: Codicon.stopCircle,
menu: {
id: MenuId.ChatExecute,
when: ContextKeyExpr.or(
ContextKeyExpr.and(ChatContextKeys.isRequestPaused.negate(), ChatContextKeys.requestInProgress),
ContextKeyExpr.and(ChatContextKeys.location.isEqualTo(ChatAgentLocation.EditingSession), applyingChatEditsContextKey)
),
when: ContextKeyExpr.and(ChatContextKeys.isRequestPaused.negate(), ChatContextKeys.requestInProgress),
order: 4,
group: 'navigation',
},
Expand All @@ -545,12 +542,6 @@ export class CancelAction extends Action2 {
if (widget.viewModel) {
chatService.cancelCurrentRequestForSession(widget.viewModel.sessionId);
}

const chatEditingService = accessor.get(IChatEditingService);
const currentEditingSession = chatEditingService.currentEditingSession;
if (currentEditingSession && currentEditingSession?.chatSessionId === widget.viewModel?.sessionId) {
chatEditingService.currentAutoApplyOperation?.cancel();
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/chat/browser/chat.contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import { IChatAccessibilityService, IChatCodeBlockContextProviderService, IChatW
import { ChatAccessibilityService } from './chatAccessibilityService.js';
import './chatAttachmentModel.js';
import { ChatMarkdownAnchorService, IChatMarkdownAnchorService } from './chatContentParts/chatMarkdownAnchorService.js';
import { ChatEditingService } from './chatEditing/chatEditingService.js';
import { ChatEditingService } from './chatEditing/chatEditingServiceImpl.js';
import { ChatEditor, IChatEditorOptions } from './chatEditor.js';
import { registerChatEditorActions } from './chatEditorActions.js';
import { ChatEditorController } from './chatEditorController.js';
Expand Down
202 changes: 76 additions & 126 deletions ...browser/chatEditing/chatEditingService.ts → ...ser/chatEditing/chatEditingServiceImpl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { coalesce, compareBy, delta } from '../../../../../base/common/arrays.js';
import { AsyncIterableSource } from '../../../../../base/common/async.js';
import { CancellationToken, CancellationTokenSource } from '../../../../../base/common/cancellation.js';
import { CancellationToken } from '../../../../../base/common/cancellation.js';
import { Codicon } from '../../../../../base/common/codicons.js';
import { BugIndicatingError, ErrorNoTelemetry } from '../../../../../base/common/errors.js';
import { Emitter, Event } from '../../../../../base/common/event.js';
Expand All @@ -22,11 +22,10 @@ import { URI } from '../../../../../base/common/uri.js';
import { TextEdit } from '../../../../../editor/common/languages.js';
import { ITextModelService } from '../../../../../editor/common/services/resolverService.js';
import { localize } from '../../../../../nls.js';
import { IContextKey, IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js';
import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js';
import { IFileService } from '../../../../../platform/files/common/files.js';
import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';
import { ILogService } from '../../../../../platform/log/common/log.js';
import { bindContextKey } from '../../../../../platform/observable/common/platformObservableUtils.js';
import { IProductService } from '../../../../../platform/product/common/productService.js';
import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js';
import { IWorkbenchAssignmentService } from '../../../../services/assignment/common/assignmentService.js';
Expand All @@ -37,8 +36,7 @@ import { ILifecycleService } from '../../../../services/lifecycle/common/lifecyc
import { IMultiDiffSourceResolver, IMultiDiffSourceResolverService, IResolvedMultiDiffSource, MultiDiffEditorItem } from '../../../multiDiffEditor/browser/multiDiffSourceResolverService.js';
import { CellUri } from '../../../notebook/common/notebookCommon.js';
import { ChatAgentLocation, IChatAgentService } from '../../common/chatAgents.js';
import { ChatContextKeys } from '../../common/chatContextKeys.js';
import { applyingChatEditsContextKey, applyingChatEditsFailedContextKey, CHAT_EDITING_MULTI_DIFF_SOURCE_RESOLVER_SCHEME, chatEditingAgentSupportsReadonlyReferencesContextKey, chatEditingMaxFileAssignmentName, chatEditingResourceContextKey, ChatEditingSessionState, decidedChatEditingResourceContextKey, defaultChatEditingMaxFileLimit, hasAppliedChatEditsContextKey, hasUndecidedChatEditingResourceContextKey, IChatEditingService, IChatEditingSession, IChatEditingSessionStream, IChatRelatedFile, IChatRelatedFilesProvider, IModifiedFileEntry, inChatEditingSessionContextKey, WorkingSetEntryState } from '../../common/chatEditingService.js';
import { CHAT_EDITING_MULTI_DIFF_SOURCE_RESOLVER_SCHEME, chatEditingAgentSupportsReadonlyReferencesContextKey, chatEditingMaxFileAssignmentName, chatEditingResourceContextKey, ChatEditingSessionState, defaultChatEditingMaxFileLimit, IChatEditingService, IChatEditingSession, IChatEditingSessionStream, IChatRelatedFile, IChatRelatedFilesProvider, IModifiedFileEntry, inChatEditingSessionContextKey, WorkingSetEntryState } from '../../common/chatEditingService.js';
import { IChatResponseModel, IChatTextEditGroup } from '../../common/chatModel.js';
import { IChatService } from '../../common/chatService.js';
import { ChatEditingModifiedFileEntry } from './chatEditingModifiedFileEntry.js';
Expand Down Expand Up @@ -66,11 +64,6 @@ export class ChatEditingService extends Disposable implements IChatEditingServic
return result;
});

private readonly _currentAutoApplyOperationObs = observableValue<CancellationTokenSource | null>(this, null);
get currentAutoApplyOperation(): CancellationTokenSource | null {
return this._currentAutoApplyOperationObs.get();
}

get currentEditingSession(): IChatEditingSession | null {
return this._currentSessionObs.get();
}
Expand All @@ -91,8 +84,6 @@ export class ChatEditingService extends Disposable implements IChatEditingServic

private _restoringEditingSession: Promise<IChatEditingSession> | undefined;

private _applyingChatEditsFailedContextKey: IContextKey<boolean | undefined>;

private _chatRelatedFilesProviders = new Map<number, IChatRelatedFilesProvider>();

constructor(
Expand All @@ -113,54 +104,14 @@ export class ChatEditingService extends Disposable implements IChatEditingServic
@IProductService productService: IProductService,
) {
super();
this._applyingChatEditsFailedContextKey = applyingChatEditsFailedContextKey.bindTo(contextKeyService);
this._applyingChatEditsFailedContextKey.set(false);
this._register(decorationsService.registerDecorationsProvider(_instantiationService.createInstance(ChatDecorationsProvider, this._currentSessionObs)));
this._register(multiDiffSourceResolverService.registerResolver(_instantiationService.createInstance(ChatEditingMultiDiffSourceResolver, this._currentSessionObs)));
this._register(textModelService.registerTextModelContentProvider(ChatEditingTextModelContentProvider.scheme, _instantiationService.createInstance(ChatEditingTextModelContentProvider, this._currentSessionObs)));
this._register(textModelService.registerTextModelContentProvider(ChatEditingSnapshotTextModelContentProvider.scheme, _instantiationService.createInstance(ChatEditingSnapshotTextModelContentProvider, this._currentSessionObs)));
this._register(bindContextKey(decidedChatEditingResourceContextKey, contextKeyService, (reader) => {
const currentSession = this._currentSessionObs.read(reader);
if (!currentSession) {
return;
}
const entries = currentSession.entries.read(reader);
const decidedEntries = entries.filter(entry => entry.state.read(reader) !== WorkingSetEntryState.Modified);
return decidedEntries.map(entry => entry.entryId);
}));
this._register(bindContextKey(hasUndecidedChatEditingResourceContextKey, contextKeyService, (reader) => {

for (const session of this.editingSessionsObs.read(reader)) {
const entries = session.entries.read(reader);
const decidedEntries = entries.filter(entry => entry.state.read(reader) === WorkingSetEntryState.Modified);
return decidedEntries.length > 0;
}

return false;
}));
this._register(bindContextKey(hasAppliedChatEditsContextKey, contextKeyService, (reader) => {
const currentSession = this._currentSessionObs.read(reader);
if (!currentSession) {
return false;
}
const entries = currentSession.entries.read(reader);
return entries.length > 0;
}));
this._register(bindContextKey(inChatEditingSessionContextKey, contextKeyService, (reader) => {
return this._currentSessionObs.read(reader) !== null;
}));
this._register(bindContextKey(applyingChatEditsContextKey, contextKeyService, (reader) => {
return this._currentAutoApplyOperationObs.read(reader) !== null;
}));
this._register(bindContextKey(ChatContextKeys.chatEditingCanUndo, contextKeyService, (r) => {
return this._currentSessionObs.read(r)?.canUndo.read(r) || false;
}));
this._register(bindContextKey(ChatContextKeys.chatEditingCanRedo, contextKeyService, (r) => {
return this._currentSessionObs.read(r)?.canRedo.read(r) || false;
}));
this._register(this._chatService.onDidDisposeSession((e) => {
if (e.reason === 'cleared' && this._currentSessionObs.get()?.chatSessionId === e.sessionId) {
this._applyingChatEditsFailedContextKey.set(false);
void this._currentSessionObs.get()?.stop();
}
}));
Expand Down Expand Up @@ -306,7 +257,6 @@ export class ChatEditingService extends Disposable implements IChatEditingServic
if (responseModel.result?.errorDetails && !responseModel.result.errorDetails.responseIsIncomplete) {
// Roll back everything
session.restoreSnapshot(responseModel.requestId);
this._applyingChatEditsFailedContextKey.set(true);
}

editsSource?.resolve();
Expand All @@ -317,94 +267,98 @@ export class ChatEditingService extends Disposable implements IChatEditingServic


const handleResponseParts = async (responseModel: IChatResponseModel) => {

if (responseModel.isCanceled) {
return;
}

for (const part of responseModel.response.value) {
if (part.kind === 'codeblockUri' || part.kind === 'textEditGroup') {
// ensure editor is open asap
if (!editedFilesExist.get(part.uri)) {
const uri = part.uri.scheme === Schemas.vscodeNotebookCell ? CellUri.parse(part.uri)?.notebook ?? part.uri : part.uri;
editedFilesExist.set(part.uri, this._fileService.exists(uri).then((e) => {
if (e) {
this._editorService.openEditor({ resource: uri, options: { inactive: true, preserveFocus: true, pinned: true } });
}
return e;
}));
}
if (part.kind !== 'codeblockUri' && part.kind !== 'textEditGroup') {
continue;
}
// ensure editor is open asap
if (!editedFilesExist.get(part.uri)) {
const uri = part.uri.scheme === Schemas.vscodeNotebookCell ? CellUri.parse(part.uri)?.notebook ?? part.uri : part.uri;
editedFilesExist.set(part.uri, this._fileService.exists(uri).then((e) => {
if (e) {
this._editorService.openEditor({ resource: uri, options: { inactive: true, preserveFocus: true, pinned: true } });
}
return e;
}));
}

// get new edits and start editing session
const first = editsSeen.size === 0;
let entry = editsSeen.get(part.uri);
if (!entry) {
entry = { seen: 0 };
editsSeen.set(part.uri, entry);
}
// get new edits and start editing session
const first = editsSeen.size === 0;
let entry = editsSeen.get(part.uri);
if (!entry) {
entry = { seen: 0 };
editsSeen.set(part.uri, entry);
}

const allEdits: TextEdit[][] = part.kind === 'textEditGroup' ? part.edits : [];
const newEdits = allEdits.slice(entry.seen);
entry.seen += newEdits.length;
const allEdits: TextEdit[][] = part.kind === 'textEditGroup' ? part.edits : [];
const newEdits = allEdits.slice(entry.seen);
entry.seen += newEdits.length;

if (newEdits.length > 0 || entry.seen === 0) {
// only allow empty edits when having just started, ignore otherwise to avoid unneccessary work
editsSource ??= new AsyncIterableSource();
editsSource.emitOne({ uri: part.uri, edits: newEdits, kind: 'textEditGroup', done: part.kind === 'textEditGroup' && part.done });
}
if (newEdits.length > 0 || entry.seen === 0) {
// only allow empty edits when having just started, ignore otherwise to avoid unneccessary work
editsSource ??= new AsyncIterableSource();
editsSource.emitOne({ uri: part.uri, edits: newEdits, kind: 'textEditGroup', done: part.kind === 'textEditGroup' && part.done });
}

if (first) {

if (first) {

await editsPromise;

editsPromise = this._continueEditingSession(session, async (builder, token) => {
for await (const item of editsSource!.asyncIterable) {
if (responseModel.isCanceled) {
break;
}
if (token.isCancellationRequested) {
break;
}
if (item.edits.length === 0) {
// EMPTY edit, just signal via empty edits that work is starting
builder.textEdits(item.uri, [], item.done ?? false, responseModel);
continue;
}
for (let i = 0; i < item.edits.length; i++) {
const group = item.edits[i];
const isLastGroup = i === item.edits.length - 1;
builder.textEdits(item.uri, group, isLastGroup && (item.done ?? false), responseModel);
}
await editsPromise;

editsPromise = this._continueEditingSession(session, async (builder) => {
for await (const item of editsSource!.asyncIterable) {
if (responseModel.isCanceled) {
break;
}
}).finally(() => {
editsPromise = undefined;
});
}
if (item.edits.length === 0) {
// EMPTY edit, just signal via empty edits that work is starting
builder.textEdits(item.uri, [], item.done ?? false, responseModel);
continue;
}
for (let i = 0; i < item.edits.length; i++) {
const group = item.edits[i];
const isLastGroup = i === item.edits.length - 1;
builder.textEdits(item.uri, group, isLastGroup && (item.done ?? false), responseModel);
}
}
}).finally(() => {
editsPromise = undefined;
});
}
}
};

observerDisposables.add(chatModel.onDidChange(async e => {
if (e.kind === 'addRequest') {
session.createSnapshot(e.request.id);
this._applyingChatEditsFailedContextKey.set(false);
const responseModel = e.request.response;
if (responseModel) {
if (e.kind !== 'addRequest') {
return;
}
session.createSnapshot(e.request.id);
const responseModel = e.request.response;
if (!responseModel) {
return;
}
if (responseModel.isComplete) {
await handleResponseParts(responseModel);
onResponseComplete(responseModel);
} else {
const disposable = observerDisposables.add(responseModel.onDidChange(async () => {
await handleResponseParts(responseModel);
if (responseModel.isComplete) {
await handleResponseParts(responseModel);
onResponseComplete(responseModel);
} else {
const disposable = responseModel.onDidChange(async () => {
await handleResponseParts(responseModel);
if (responseModel.isComplete) {
onResponseComplete(responseModel);
disposable.dispose();
}
});
observerDisposables.delete(disposable);
}
}
}));
}
}));
observerDisposables.add(chatModel.onDidDispose(() => observerDisposables.dispose()));
return observerDisposables;
}

private async _continueEditingSession(session: ChatEditingSession, builder: (stream: IChatEditingSessionStream, token: CancellationToken) => Promise<void>): Promise<void> {
private async _continueEditingSession(session: ChatEditingSession, builder: (stream: IChatEditingSessionStream) => Promise<void>): Promise<void> {
if (session.state.get() === ChatEditingSessionState.StreamingEdits) {
throw new BugIndicatingError('Cannot continue session that is still streaming');
}
Expand All @@ -415,13 +369,9 @@ export class ChatEditingService extends Disposable implements IChatEditingServic
}
};
session.acceptStreamingEditsStart();
const cancellationTokenSource = new CancellationTokenSource();
this._currentAutoApplyOperationObs.set(cancellationTokenSource, undefined);
try {
await builder(stream, cancellationTokenSource.token);
await builder(stream);
} finally {
cancellationTokenSource.dispose();
this._currentAutoApplyOperationObs.set(null, undefined);
session.resolve();
}
}
Expand Down
Loading

0 comments on commit d04d2d3

Please sign in to comment.