Skip to content

Commit e690809

Browse files
authored
nes: joint: don't trigger joint provider's onDidChange on request in flight or completions request in flight (#2443)
1 parent a914151 commit e690809

File tree

3 files changed

+73
-12
lines changed

3 files changed

+73
-12
lines changed

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

Lines changed: 65 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { IAuthenticationService, IExperimentationService } from '../../../lib/no
1010
import { ConfigKey, IConfigurationService } from '../../../platform/configuration/common/configurationService';
1111
import { IEnvService } from '../../../platform/env/common/envService';
1212
import { IVSCodeExtensionContext } from '../../../platform/extContext/common/extensionContext';
13-
import { JointCompletionsProviderStrategy } from '../../../platform/inlineEdits/common/dataTypes/jointCompletionsProviderOptions';
13+
import { JointCompletionsProviderStrategy, JointCompletionsProviderTriggerChangeStrategy } from '../../../platform/inlineEdits/common/dataTypes/jointCompletionsProviderOptions';
1414
import { InlineEditRequestLogContext } from '../../../platform/inlineEdits/common/inlineEditLogContext';
1515
import { ObservableGit } from '../../../platform/inlineEdits/common/observableGit';
1616
import { shortenOpportunityId } from '../../../platform/inlineEdits/common/utils/utils';
@@ -243,7 +243,21 @@ type LastNesSuggestion = {
243243

244244
class JointCompletionsProvider extends Disposable implements vscode.InlineCompletionItemProvider {
245245

246-
public onDidChange?: vscode.Event<void> | undefined;
246+
private readonly _onDidChangeEmitter = this._register(new vscode.EventEmitter<void>());
247+
public readonly onDidChange?: vscode.Event<void> | undefined = this._onDidChangeEmitter.event;
248+
249+
private _requestsInFlightCount = 0;
250+
private _completionsRequestsInFlightCount = 0;
251+
252+
private get _isRequestInFlight(): boolean {
253+
return this._requestsInFlightCount > 0;
254+
}
255+
256+
private get _isCompletionsRequestInFlight(): boolean {
257+
return this._completionsRequestsInFlightCount > 0;
258+
}
259+
260+
private _tracer = createTracer(['NES', 'JointCompletionsProvider'], (msg) => this._logService.trace(msg));
247261

248262
//#region Model picker
249263
public readonly onDidChangeModelInfo = this._inlineEditProvider?.onDidChangeModelInfo;
@@ -261,15 +275,42 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
261275
@ILogService private readonly _logService: ILogService,
262276
) {
263277
super();
264-
this.onDidChange = _inlineEditProvider?.onDidChange;
278+
279+
const disp = this._inlineEditProvider?.onDidChange?.(() => {
280+
const strategy = this._configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsJointCompletionsProviderTriggerChangeStrategy, this._expService);
281+
switch (strategy) {
282+
case JointCompletionsProviderTriggerChangeStrategy.AlwaysTrigger:
283+
break;
284+
case JointCompletionsProviderTriggerChangeStrategy.NoTriggerOnRequestInFlight:
285+
if (this._isRequestInFlight) {
286+
this._tracer.trace('Skipping onDidChange event firing because request is in flight');
287+
return;
288+
}
289+
break;
290+
case JointCompletionsProviderTriggerChangeStrategy.NoTriggerOnCompletionsRequestInFlight:
291+
if (this._isCompletionsRequestInFlight) {
292+
this._tracer.trace('Skipping onDidChange event firing because completions request is in flight');
293+
return;
294+
}
295+
break;
296+
default:
297+
assertNever(strategy);
298+
}
299+
this._tracer.trace('Firing onDidChange event');
300+
this._onDidChangeEmitter.fire();
301+
});
302+
if (disp) {
303+
this._register(disp);
304+
}
305+
265306
softAssert(
266307
_completionsProvider?.onDidChange === undefined,
267308
'CompletionsProvider does not implement onDidChange'
268309
);
269310
}
270311

271312
public async provideInlineCompletionItems(document: vscode.TextDocument, position: vscode.Position, context: vscode.InlineCompletionContext, token: vscode.CancellationToken): Promise<SingularCompletionList | undefined> {
272-
const tracer = createTracer(['JointCompletionsProvider', shortenOpportunityId(context.requestUuid), 'provideInlineCompletionItems'], (msg) => this._logService.trace(msg));
313+
const tracer = this._tracer.sub([shortenOpportunityId(context.requestUuid), 'provideInlineCompletionItems']);
273314

274315
const strategy = this._configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsJointCompletionsProviderStrategy, this._expService);
275316

@@ -290,6 +331,9 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
290331
const completionsCts = new CancellationTokenSource(token);
291332
const nesCts = new CancellationTokenSource(token);
292333

334+
this._requestsInFlightCount++;
335+
tracer.trace(`invocation #${invocationId} started; request in flight: true`);
336+
293337
let saveLastNesSuggestion: null | LastNesSuggestion = null;
294338
try {
295339
const docSnapshot = new StringText(document.getText());
@@ -329,6 +373,7 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
329373

330374
return list;
331375
} finally {
376+
this._requestsInFlightCount--;
332377

333378
// Only save the last NES suggestion if this is the latest invocation
334379
if (invocationId === this.provideInlineCompletionItemsInvocationCount) {
@@ -465,13 +510,22 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
465510
private _invokeCompletionsProvider(tracer: ITracer, document: vscode.TextDocument, position: vscode.Position, context: vscode.InlineCompletionContext, tokens: { coreToken: CancellationToken; completionsCts: CancellationTokenSource; nesCts: CancellationTokenSource }, sw: StopWatch) {
466511
let completionsP: Promise<vscode.InlineCompletionList | undefined> | undefined;
467512
if (this._completionsProvider) {
468-
tracer.trace(`- requesting completions provideInlineCompletionItems`);
469-
completionsP = this._completionsProvider.provideInlineCompletionItems(document, position, context, tokens.completionsCts.token);
470-
completionsP.then((completionsR) => {
471-
tracer.trace(`got completions response in ${sw.elapsed()}ms -- ${completionsR === undefined ? 'undefined' : `with ${completionsR.items.length} items`}`);
472-
}).catch((e) => {
473-
tracer.trace(`completions provider errored after ${sw.elapsed()}ms -- ${errors.toString(errors.fromUnknown(e))}`);
474-
});
513+
this._completionsRequestsInFlightCount++;
514+
try {
515+
tracer.trace(`- requesting completions provideInlineCompletionItems`);
516+
completionsP = this._completionsProvider.provideInlineCompletionItems(document, position, context, tokens.completionsCts.token);
517+
completionsP.then((completionsR) => {
518+
tracer.trace(`got completions response in ${sw.elapsed()}ms -- ${completionsR === undefined ? 'undefined' : `with ${completionsR.items.length} items`}`);
519+
}).catch((e) => {
520+
tracer.trace(`completions provider errored after ${sw.elapsed()}ms -- ${errors.toString(errors.fromUnknown(e))}`);
521+
}).finally(() => {
522+
this._completionsRequestsInFlightCount--;
523+
});
524+
} catch (e) {
525+
this._completionsRequestsInFlightCount--;
526+
tracer.trace(`completions provider threw synchronously after ${sw.elapsed()}ms -- ${errors.toString(errors.fromUnknown(e))}`);
527+
throw e;
528+
}
475529
} else {
476530
tracer.trace(`- no completions provider`);
477531
completionsP = undefined;

src/platform/configuration/common/configurationService.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { IObservable, observableFromEventOpts } from '../../../util/vs/base/comm
1313
import * as types from '../../../util/vs/base/common/types';
1414
import { ICopilotTokenStore } from '../../authentication/common/copilotTokenStore';
1515
import { isPreRelease, packageJson } from '../../env/common/packagejson';
16-
import { JointCompletionsProviderStrategy } from '../../inlineEdits/common/dataTypes/jointCompletionsProviderOptions';
16+
import { JointCompletionsProviderStrategy, JointCompletionsProviderTriggerChangeStrategy } from '../../inlineEdits/common/dataTypes/jointCompletionsProviderOptions';
1717
import { NextCursorLinePrediction } from '../../inlineEdits/common/dataTypes/nextCursorLinePrediction';
1818
import * as xtabPromptOptions from '../../inlineEdits/common/dataTypes/xtabPromptOptions';
1919
import { LANGUAGE_CONTEXT_ENABLED_LANGUAGES, LanguageContextLanguages } from '../../inlineEdits/common/dataTypes/xtabPromptOptions';
@@ -769,6 +769,7 @@ export namespace ConfigKey {
769769
export const InlineEditsIgnoreWhenSuggestVisible = defineTeamInternalSetting<boolean>('chat.advanced.inlineEdits.ignoreWhenSuggestVisible', ConfigType.ExperimentBased, false);
770770
export const InlineEditsJointCompletionsProviderEnabled = defineTeamInternalSetting<boolean>('chat.advanced.inlineEdits.jointCompletionsProvider.enabled', ConfigType.ExperimentBased, false);
771771
export const InlineEditsJointCompletionsProviderStrategy = defineTeamInternalSetting<JointCompletionsProviderStrategy>('chat.advanced.inlineEdits.jointCompletionsProvider.strategy', ConfigType.ExperimentBased, JointCompletionsProviderStrategy.Regular);
772+
export const InlineEditsJointCompletionsProviderTriggerChangeStrategy = defineTeamInternalSetting<JointCompletionsProviderTriggerChangeStrategy>('chat.advanced.inlineEdits.jointCompletionsProvider.triggerChangeStrategy', ConfigType.ExperimentBased, JointCompletionsProviderTriggerChangeStrategy.NoTriggerOnCompletionsRequestInFlight);
772773
export const InstantApplyModelName = defineTeamInternalSetting<string>('chat.advanced.instantApply.modelName', ConfigType.ExperimentBased, 'gpt-4o-instant-apply-full-ft-v66');
773774
export const VerifyTextDocumentChanges = defineTeamInternalSetting<boolean>('chat.advanced.inlineEdits.verifyTextDocumentChanges', ConfigType.ExperimentBased, false);
774775

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,9 @@
66
export enum JointCompletionsProviderStrategy {
77
Regular = 'regular',
88
}
9+
10+
export enum JointCompletionsProviderTriggerChangeStrategy {
11+
NoTriggerOnRequestInFlight = 'noTriggerOnRequestInFlight',
12+
NoTriggerOnCompletionsRequestInFlight = 'noTriggerOnCompletionsRequestInFlight',
13+
AlwaysTrigger = 'alwaysTrigger',
14+
}

0 commit comments

Comments
 (0)