Skip to content

Commit b967ca8

Browse files
committed
nes: joint: fix ghost-text provider returning no-op edits to UI (#2479)
1 parent 4457cff commit b967ca8

File tree

2 files changed

+42
-8
lines changed

2 files changed

+42
-8
lines changed

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

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
423423
tracer.trace(`no last NES suggestion to consider`);
424424
const completionsP = this._invokeCompletionsProvider(tracer, document, position, context, tokens, sw);
425425
const nesP = this._invokeNESProvider(tracer, document, position, true, context, tokens, sw);
426-
return this._returnCompletionsOrOtherwiseNES(completionsP, nesP, sw, tracer, tokens);
426+
return this._returnCompletionsOrOtherwiseNES(completionsP, nesP, docSnapshot, sw, tracer, tokens);
427427
}
428428

429429
tracer.trace(`last NES suggestion is for the current document, checking if it agrees with the current suggestion`);
@@ -433,7 +433,7 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
433433
if (!nesP) {
434434
tracer.trace(`no NES provider`);
435435
const completionsP = this._invokeCompletionsProvider(tracer, document, position, context, tokens, sw);
436-
return this._returnCompletionsOrOtherwiseNES(completionsP, nesP, sw, tracer, tokens);
436+
return this._returnCompletionsOrOtherwiseNES(completionsP, nesP, docSnapshot, sw, tracer, tokens);
437437
}
438438

439439
const NES_CACHE_WAIT_MS = 10;
@@ -494,7 +494,7 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
494494
}
495495

496496
tracer.trace('falling back to the default because completions came first or NES disagreed');
497-
return this._returnCompletionsOrOtherwiseNES(completionsP, nesP, sw, tracer, tokens);
497+
return this._returnCompletionsOrOtherwiseNES(completionsP, nesP, docSnapshot, sw, tracer, tokens);
498498
}
499499

500500
private _invokeNESProvider(tracer: ITracer, document: vscode.TextDocument, position: vscode.Position, enforceCacheDelay: boolean, context: vscode.InlineCompletionContext, tokens: { coreToken: CancellationToken; completionsCts: CancellationTokenSource; nesCts: CancellationTokenSource }, sw: StopWatch) {
@@ -549,6 +549,7 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
549549
private async _returnCompletionsOrOtherwiseNES(
550550
completionsP: Promise<vscode.InlineCompletionList | undefined> | undefined,
551551
nesP: Promise<NesCompletionList | undefined> | undefined,
552+
docSnapshot: StringText,
552553
sw: StopWatch,
553554
tracer: ITracer,
554555
tokens: { coreToken: CancellationToken; completionsCts: CancellationTokenSource; nesCts: CancellationTokenSource },
@@ -559,16 +560,26 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
559560
tracer.trace(`completions response received`);
560561

561562
if (completionsR && completionsR.items.length > 0) {
562-
tracer.trace(`using completions response, cancelling NES provider`);
563-
return this._returnCompletions(completionsR, { kind: vscode.InlineCompletionsDisposeReasonKind.LostRace }, nesP, sw, tracer, tokens);
563+
const filteredCompletionR = JointCompletionsProvider.retainOnlyMeaningfulEdits(docSnapshot, completionsR);
564+
if (filteredCompletionR.items.length === 0) {
565+
tracer.trace(`all completions edits are no-op, ignoring completions response`);
566+
} else {
567+
tracer.trace(`using completions response, cancelling NES provider`);
568+
return this._returnCompletions(filteredCompletionR, { kind: vscode.InlineCompletionsDisposeReasonKind.LostRace }, nesP, sw, tracer, tokens);
569+
}
564570
}
565571

566572
const nesR = nesP ? await nesP : undefined;
567573
tracer.trace(`NES response received`);
568574

569575
if (nesR && nesR.items.length > 0) {
570-
tracer.trace(`using NES response`);
571-
return this._returnNES(nesR, { kind: vscode.InlineCompletionsDisposeReasonKind.NotTaken }, completionsP, sw, tracer, tokens);
576+
const filteredNesR = JointCompletionsProvider.retainOnlyMeaningfulEdits(docSnapshot, nesR);
577+
if (filteredNesR.items.length === 0) {
578+
tracer.trace(`all NES edits are no-op, ignoring NES response`);
579+
} else {
580+
tracer.trace(`using NES response`);
581+
return this._returnNES(filteredNesR, { kind: vscode.InlineCompletionsDisposeReasonKind.NotTaken }, completionsP, sw, tracer, tokens);
582+
}
572583
}
573584

574585
// return empty completions
@@ -613,6 +624,29 @@ class JointCompletionsProvider extends Disposable implements vscode.InlineComple
613624
return applied === docWithNesEditApplied.getValue();
614625
}
615626

627+
private static retainOnlyMeaningfulEdits<T extends vscode.InlineCompletionList>(docSnapshot: StringText, list: T): T {
628+
// meaningful = not noop
629+
function isMeaningfulEdit(item: vscode.InlineCompletionItem): boolean {
630+
if (item.range === undefined || // must be a completion with a side-effect, eg a command invocation or something
631+
typeof item.insertText !== 'string' // shouldn't happen
632+
) {
633+
return true;
634+
}
635+
const originalSnippet = docSnapshot.getValueOfRange(new Range(
636+
item.range.start.line + 1,
637+
item.range.start.character + 1,
638+
item.range.end.line + 1,
639+
item.range.end.character + 1,
640+
));
641+
return originalSnippet.replace(/\r\n/g, '\n') !== item.insertText.replace(/\r\n/g, '\n');
642+
}
643+
const filteredEdits = list.items.filter(isMeaningfulEdit);
644+
if (filteredEdits.length === list.items.length) {
645+
return list;
646+
}
647+
return { ...list, items: filteredEdits };
648+
}
649+
616650
public handleDidShowCompletionItem?(completionItem: SingularCompletionItem, updatedInsertText: string): void {
617651
switch (completionItem.source) {
618652
case 'completions':

src/platform/configuration/common/configurationService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,7 +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);
772+
export const InlineEditsJointCompletionsProviderTriggerChangeStrategy = defineTeamInternalSetting<JointCompletionsProviderTriggerChangeStrategy>('chat.advanced.inlineEdits.jointCompletionsProvider.triggerChangeStrategy', ConfigType.ExperimentBased, JointCompletionsProviderTriggerChangeStrategy.NoTriggerOnRequestInFlight);
773773
export const InstantApplyModelName = defineTeamInternalSetting<string>('chat.advanced.instantApply.modelName', ConfigType.ExperimentBased, 'gpt-4o-instant-apply-full-ft-v66');
774774
export const VerifyTextDocumentChanges = defineTeamInternalSetting<boolean>('chat.advanced.inlineEdits.verifyTextDocumentChanges', ConfigType.ExperimentBased, false);
775775

0 commit comments

Comments
 (0)