nes: joint: don't trigger joint provider's onDidChange on request in flight or completions request in flight (#2443)

Author: ulugbekna

src/extension/inlineEdits/vscode-node/jointInlineCompletionProvider.ts

@@ -10,7 +10,7 @@ import { IAuthenticationService, IExperimentationService } from '../../../lib/no
 import { ConfigKey, IConfigurationService } from '../../../platform/configuration/common/configurationService';
 import { IEnvService } from '../../../platform/env/common/envService';
 import { IVSCodeExtensionContext } from '../../../platform/extContext/common/extensionContext';
-import { JointCompletionsProviderStrategy } from '../../../platform/inlineEdits/common/dataTypes/jointCompletionsProviderOptions';
+import { JointCompletionsProviderStrategy, JointCompletionsProviderTriggerChangeStrategy } from '../../../platform/inlineEdits/common/dataTypes/jointCompletionsProviderOptions';
 import { InlineEditRequestLogContext } from '../../../platform/inlineEdits/common/inlineEditLogContext';
 import { ObservableGit } from '../../../platform/inlineEdits/common/observableGit';
 import { shortenOpportunityId } from '../../../platform/inlineEdits/common/utils/utils';
@@ -243,7 +243,21 @@ type LastNesSuggestion = {
 
 class JointCompletionsProvider extends Disposable implements vscode.InlineCompletionItemProvider {
 
-	public onDidChange?: vscode.Event<void> | undefined;
+	private readonly _onDidChangeEmitter = this._register(new vscode.EventEmitter<void>());
+	public readonly onDidChange?: vscode.Event<void> | undefined = this._onDidChangeEmitter.event;
+
+	private _requestsInFlight = new Set<CancellationToken>();
+	private _completionsRequestsInFlight = new Set<CancellationToken>();
+
+	private get _isRequestInFlight(): boolean {
+		return this._requestsInFlight.size > 0;
+	}
+
+	private get _isCompletionsRequestInFlight(): boolean {
+		return this._completionsRequestsInFlight.size > 0;
+	}
+
+	private _tracer = createTracer(['NES', 'JointCompletionsProvider'], (msg) => this._logService.trace(msg));
 
 	//#region Model picker
 	public readonly onDidChangeModelInfo = this._inlineEditProvider?.onDidChangeModelInfo;
@@ -261,15 +275,42 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
 		@ILogService private readonly _logService: ILogService,
 	) {
 		super();
-		this.onDidChange = _inlineEditProvider?.onDidChange;
+
+		const disp = this._inlineEditProvider?.onDidChange?.(() => {
+			const strategy = this._configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsJointCompletionsProviderTriggerChangeStrategy, this._expService);
+			switch (strategy) {
+				case JointCompletionsProviderTriggerChangeStrategy.AlwaysTrigger:
+					break;
+				case JointCompletionsProviderTriggerChangeStrategy.NoTriggerOnRequestInFlight:
+					if (this._isRequestInFlight) {
+						this._tracer.trace('Skipping onDidChange event firing because request is in flight');
+						return;
+					}
+					break;
+				case JointCompletionsProviderTriggerChangeStrategy.NoTriggerOnCompletionsRequestInFlight:
+					if (this._isCompletionsRequestInFlight) {
+						this._tracer.trace('Skipping onDidChange event firing because completions request is in flight');
+						return;
+					}
+					break;
+				default:
+					assertNever(strategy);
+			}
+			this._tracer.trace('Firing onDidChange event');
+			this._onDidChangeEmitter.fire();
+		});
+		if (disp) {
+			this._register(disp);
+		}
+
 		softAssert(
 			_completionsProvider?.onDidChange === undefined,
 			'CompletionsProvider does not implement onDidChange'
 		);
 	}
 
 	public async provideInlineCompletionItems(document: vscode.TextDocument, position: vscode.Position, context: vscode.InlineCompletionContext, token: vscode.CancellationToken): Promise<SingularCompletionList | undefined> {
-		const tracer = createTracer(['JointCompletionsProvider', shortenOpportunityId(context.requestUuid), 'provideInlineCompletionItems'], (msg) => this._logService.trace(msg));
+		const tracer = this._tracer.sub([shortenOpportunityId(context.requestUuid), 'provideInlineCompletionItems']);
 
 		const strategy = this._configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsJointCompletionsProviderStrategy, this._expService);
 
@@ -290,6 +331,13 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
 		const completionsCts = new CancellationTokenSource(token);
 		const nesCts = new CancellationTokenSource(token);
 
+		this._requestsInFlight.add(token);
+		const disp1 = token.onCancellationRequested(() => {
+			tracer.trace(`invocation #${invocationId}: request in flight: false -- due to cancellation`);
+			this._requestsInFlight.delete(token);
+		});
+		tracer.trace(`invocation #${invocationId} started; request in flight: true`);
+
 		let saveLastNesSuggestion: null | LastNesSuggestion = null;
 		try {
 			const docSnapshot = new StringText(document.getText());
@@ -329,6 +377,11 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
 
 			return list;
 		} finally {
+			if (!token.isCancellationRequested) {
+				tracer.trace(`invocation #${invocationId}: request in flight: false -- due to provider finishing`);
+				this._requestsInFlight.delete(token);
+			}
+			disp1.dispose();
 
 			// Only save the last NES suggestion if this is the latest invocation
 			if (invocationId === this.provideInlineCompletionItemsInvocationCount) {
@@ -465,13 +518,27 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
 	private _invokeCompletionsProvider(tracer: ITracer, document: vscode.TextDocument, position: vscode.Position, context: vscode.InlineCompletionContext, tokens: { coreToken: CancellationToken; completionsCts: CancellationTokenSource; nesCts: CancellationTokenSource }, sw: StopWatch) {
 		let completionsP: Promise<vscode.InlineCompletionList | undefined> | undefined;
 		if (this._completionsProvider) {
-			tracer.trace(`- requesting completions provideInlineCompletionItems`);
-			completionsP = this._completionsProvider.provideInlineCompletionItems(document, position, context, tokens.completionsCts.token);
-			completionsP.then((completionsR) => {
-				tracer.trace(`got completions response in ${sw.elapsed()}ms -- ${completionsR === undefined ? 'undefined' : `with ${completionsR.items.length} items`}`);
-			}).catch((e) => {
-				tracer.trace(`completions provider errored after ${sw.elapsed()}ms -- ${errors.toString(errors.fromUnknown(e))}`);
-			});
+			this._completionsRequestsInFlight.add(tokens.completionsCts.token);
+			const disp = tokens.completionsCts.token.onCancellationRequested(() => this._completionsRequestsInFlight.delete(tokens.completionsCts.token));
+			const cleanup = () => {
+				this._completionsRequestsInFlight.delete(tokens.completionsCts.token);
+				disp.dispose();
+			};
+			try { // in case the provider throws synchronously
+				tracer.trace(`- requesting completions provideInlineCompletionItems`);
+				completionsP = this._completionsProvider.provideInlineCompletionItems(document, position, context, tokens.completionsCts.token);
+				completionsP.then((completionsR) => {
+					tracer.trace(`got completions response in ${sw.elapsed()}ms -- ${completionsR === undefined ? 'undefined' : `with ${completionsR.items.length} items`}`);
+				}).catch((e) => {
+					tracer.trace(`completions provider errored after ${sw.elapsed()}ms -- ${errors.toString(errors.fromUnknown(e))}`);
+				}).finally(() => {
+					cleanup();
+				});
+			} catch (e) {
+				cleanup();
+				tracer.trace(`completions provider threw synchronously after ${sw.elapsed()}ms -- ${errors.toString(errors.fromUnknown(e))}`);
+				throw e;
+			}
 		} else {
 			tracer.trace(`- no completions provider`);
 			completionsP = undefined;

src/platform/configuration/common/configurationService.ts

@@ -13,7 +13,7 @@ import { IObservable, observableFromEventOpts } from '../../../util/vs/base/comm
 import * as types from '../../../util/vs/base/common/types';
 import { ICopilotTokenStore } from '../../authentication/common/copilotTokenStore';
 import { isPreRelease, packageJson } from '../../env/common/packagejson';
-import { JointCompletionsProviderStrategy } from '../../inlineEdits/common/dataTypes/jointCompletionsProviderOptions';
+import { JointCompletionsProviderStrategy, JointCompletionsProviderTriggerChangeStrategy } from '../../inlineEdits/common/dataTypes/jointCompletionsProviderOptions';
 import { NextCursorLinePrediction } from '../../inlineEdits/common/dataTypes/nextCursorLinePrediction';
 import * as xtabPromptOptions from '../../inlineEdits/common/dataTypes/xtabPromptOptions';
 import { LANGUAGE_CONTEXT_ENABLED_LANGUAGES, LanguageContextLanguages } from '../../inlineEdits/common/dataTypes/xtabPromptOptions';
@@ -769,6 +769,7 @@ export namespace ConfigKey {
 		export const InlineEditsIgnoreWhenSuggestVisible = defineTeamInternalSetting<boolean>('chat.advanced.inlineEdits.ignoreWhenSuggestVisible', ConfigType.ExperimentBased, false);
 		export const InlineEditsJointCompletionsProviderEnabled = defineTeamInternalSetting<boolean>('chat.advanced.inlineEdits.jointCompletionsProvider.enabled', ConfigType.ExperimentBased, false);
 		export const InlineEditsJointCompletionsProviderStrategy = defineTeamInternalSetting<JointCompletionsProviderStrategy>('chat.advanced.inlineEdits.jointCompletionsProvider.strategy', ConfigType.ExperimentBased, JointCompletionsProviderStrategy.Regular);
+		export const InlineEditsJointCompletionsProviderTriggerChangeStrategy = defineTeamInternalSetting<JointCompletionsProviderTriggerChangeStrategy>('chat.advanced.inlineEdits.jointCompletionsProvider.triggerChangeStrategy', ConfigType.ExperimentBased, JointCompletionsProviderTriggerChangeStrategy.NoTriggerOnCompletionsRequestInFlight);
 		export const InstantApplyModelName = defineTeamInternalSetting<string>('chat.advanced.instantApply.modelName', ConfigType.ExperimentBased, 'gpt-4o-instant-apply-full-ft-v66');
 		export const VerifyTextDocumentChanges = defineTeamInternalSetting<boolean>('chat.advanced.inlineEdits.verifyTextDocumentChanges', ConfigType.ExperimentBased, false);
 

src/platform/inlineEdits/common/dataTypes/jointCompletionsProviderOptions.ts

@@ -6,3 +6,9 @@
 export enum JointCompletionsProviderStrategy {
 	Regular = 'regular',
 }
+
+export enum JointCompletionsProviderTriggerChangeStrategy {
+	NoTriggerOnRequestInFlight = 'noTriggerOnRequestInFlight',
+	NoTriggerOnCompletionsRequestInFlight = 'noTriggerOnCompletionsRequestInFlight',
+	AlwaysTrigger = 'alwaysTrigger',
+}