Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 72 additions & 1 deletion src/extension/inlineEdits/node/nextEditProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ export interface INextEditProvider<T extends INextEditResult, TTelemetry, TData
lastRejectionTime: number;
lastTriggerTime: number;
lastOutcome: NesOutcome | undefined;
/**
* If the last shown NES targets the given document and the cursor landed
* at the expected position (the start of the edit's replace range, or the
* jump-to position), consumes the suppress state and returns `true`.
*/
consumeShouldSuppressSelectionChangeTrigger(docId: DocumentId, cursorLine: number, cursorColumn: number): boolean;
}

interface ProcessedDoc {
Expand Down Expand Up @@ -148,6 +154,20 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
/** The requestId of the last shown suggestion. We store only the requestId (not the object) to avoid preventing garbage collection. */
private _lastShownSuggestionId: number | undefined = undefined;

/**
* Info about the last shown NES, used to suppress the selection-change
* trigger that VS Code fires as a side-effect of acceptance / cursor jump.
* Set in {@link handleShown}, consumed on the first matching check,
* cleared on rejection/ignore.
*/
private _shownNesInfo: {
readonly targetDocumentId: DocumentId;
/** 0-based line where the cursor is expected to land. */
readonly expectedCursorLine: number;
/** 0-based column where the cursor is expected to land. */
readonly expectedCursorColumn: number;
} | undefined;

private _lastRejectionTime = 0;
public get lastRejectionTime() {
return this._lastRejectionTime;
Expand Down Expand Up @@ -371,7 +391,7 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
logContext.markAsNoSuggestions();
} else {
telemetryBuilder.setStatus('emptyEditsButHasNextCursorPosition');
return new NextEditResult(logContext.requestId, req, { jumpToPosition: error.nextCursorPosition, documentBeforeEdits: documentAtInvocationTime, isFromCursorJump: false, isSubsequentEdit: false });
return new NextEditResult(logContext.requestId, req, { jumpToPosition: error.nextCursorPosition, documentBeforeEdits: documentAtInvocationTime, isFromCursorJump: false, isSubsequentEdit: false, targetDocumentId: docId });
}
}

Expand Down Expand Up @@ -972,6 +992,11 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
this._lastOutcome = undefined; // clear so that outcome is "pending" until resolved
this._scheduledSpeculativeRequest = null; // clear any previously scheduled speculative

// Track the shown NES so the triggerer can suppress the selection-change
// event that VS Code fires when the user accepts the NES or jumps to it.
this._shownNesInfo = NextEditProvider._computeShownNesInfo(suggestion);
this._logger.trace(`handleShown: _shownNesInfo=${this._shownNesInfo ? `doc=${this._shownNesInfo.targetDocumentId.uri}, line=${this._shownNesInfo.expectedCursorLine}, col=${this._shownNesInfo.expectedCursorColumn}` : 'none'}`);

// Trigger speculative request for the post-edit document state
const speculativeRequestsEnablement = this._configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsSpeculativeRequests, this._expService);
if (speculativeRequestsEnablement === SpeculativeRequestsEnablement.On) {
Expand Down Expand Up @@ -1356,7 +1381,26 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
return shiftedSelection;
}

public consumeShouldSuppressSelectionChangeTrigger(docId: DocumentId, cursorLine: number, cursorColumn: number): boolean {
const info = this._shownNesInfo;
if (!info) {
return false;
}
if (info.targetDocumentId !== docId || info.expectedCursorLine !== cursorLine || info.expectedCursorColumn !== cursorColumn) {
console.log(
`consumeShouldSuppressSelectionChangeTrigger: not suppressing (doc=${docId.uri}, expectedLine=${info.expectedCursorLine}, expectedColumn=${info.expectedCursorColumn}, line=${cursorLine}, col=${cursorColumn})`
);
return false;
}
this._logger.trace(`consumeShouldSuppressSelectionChangeTrigger: suppressing (doc=${docId.uri}, line=${cursorLine}, col=${cursorColumn})`);
this._shownNesInfo = undefined;
return true;
}

public handleAcceptance(docId: DocumentId, suggestion: NextEditResult) {
// Re-arm suppression in case it was consumed between handleShown and now.
this._shownNesInfo = NextEditProvider._computeShownNesInfo(suggestion);
this._logger.trace(`handleAcceptance: _shownNesInfo=${this._shownNesInfo ? `doc=${this._shownNesInfo.targetDocumentId.uri}, line=${this._shownNesInfo.expectedCursorLine}, col=${this._shownNesInfo.expectedCursorColumn}` : 'none'}`);
this.runSnippy(docId, suggestion);
this._statelessNextEditProvider.handleAcceptance?.();
this._lastOutcome = NesOutcome.Accepted;
Expand All @@ -1371,6 +1415,7 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
}

public handleRejection(docId: DocumentId, suggestion: NextEditResult) {
this._shownNesInfo = undefined;
assertType(suggestion.result, '@ulugbekna: undefined edit cannot be rejected?');

// The user rejected the suggestion, so the speculative request (which
Expand All @@ -1393,6 +1438,7 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
}

public handleIgnored(docId: DocumentId, suggestion: NextEditResult, supersededBy: INextEditResult | undefined): void {
this._shownNesInfo = undefined;
this._lastOutcome = NesOutcome.Ignored;

// Check if this was the last shown suggestion
Expand All @@ -1417,6 +1463,31 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
this._nextEditCache.clear();
this._rejectionCollector.clear();
}

/**
* Computes the expected cursor position after the user accepts the shown
* NES. Returns `undefined` for jump-to-position NES (no edit) and for
* suggestions without a result, since those should not suppress triggers.
*/
private static _computeShownNesInfo(suggestion: NextEditResult): {
readonly targetDocumentId: DocumentId;
readonly expectedCursorLine: number;
readonly expectedCursorColumn: number;
} | undefined {
const result = suggestion.result;
if (!result?.edit) {
return undefined;
}

// Eliminate common prefix/suffix to find where the actual change starts.
const preciseEdit = result.edit.removeCommonSuffixPrefix(result.documentBeforeEdits.value);
const position = result.documentBeforeEdits.getTransformer().getPosition(preciseEdit.replaceRange.start);
return {
targetDocumentId: result.targetDocumentId,
expectedCursorLine: position.lineNumber - 1, // 1-based → 0-based
expectedCursorColumn: position.column - 1, // 1-based → 0-based
};
}
}

function assertDefined<T>(value: T | undefined): T {
Expand Down
2 changes: 1 addition & 1 deletion src/extension/inlineEdits/node/nextEditResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export class NextEditResult implements INextEditResult {
edit?: StringReplacement;
documentBeforeEdits: StringText;
displayLocation?: INextEditDisplayLocation;
targetDocumentId?: DocumentId;
targetDocumentId: DocumentId;
action?: Command;
isFromCursorJump: boolean;
jumpToPosition?: Position;
Expand Down
150 changes: 150 additions & 0 deletions src/extension/inlineEdits/test/vscode-node/inlineEditTriggerer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,16 @@ suite('InlineEditTriggerer', () => {
public lastRejectionTime: number = Date.now();
public lastTriggerTime: number = Date.now();
public lastOutcome: NesOutcome | undefined = undefined;
public shownNesInfo: { targetDocumentId: DocumentId; expectedCursorLine: number; expectedCursorColumn: number } | undefined;

public consumeShouldSuppressSelectionChangeTrigger(docId: DocumentId, cursorLine: number, cursorColumn: number): boolean {
const info = this.shownNesInfo;
if (info && info.targetDocumentId === docId && info.expectedCursorLine === cursorLine && info.expectedCursorColumn === cursorColumn) {
this.shownNesInfo = undefined;
return true;
}
return false;
}
}

class MockVSCodeWorkspace {
Expand Down Expand Up @@ -1309,4 +1319,144 @@ suite('InlineEditTriggerer', () => {
});

// #endregion

// #region Post-acceptance trigger suppression

suite('Post-acceptance trigger suppression', () => {
test('No signal when NES target doc and cursor position match', () => {
const { document, textEditor } = createTextDocument(undefined, undefined, 'line1\nline2\nline3');
nextEditProvider.lastRejectionTime = Date.now() - TRIGGER_INLINE_EDIT_REJECTION_COOLDOWN - 1;

triggerTextChange(document);
triggerTextSelectionChange(textEditor, new Selection(0, 0, 0, 0));
const countAfterFirst = firedEvents.length;
assert.isAtLeast(countAfterFirst, 1, 'First trigger should fire');

// Simulate: NES shown targeting this document, cursor expected at (2, 3)
nextEditProvider.shownNesInfo = {
targetDocumentId: DocumentId.create(document.uri.toString()),
expectedCursorLine: 2,
expectedCursorColumn: 3,
};

// Selection changes to the expected position
triggerTextSelectionChange(textEditor, new Selection(2, 3, 2, 3));

assert.strictEqual(firedEvents.length, countAfterFirst,
'Signal should not fire when doc and cursor position match');
});

test('Suppression is consumed and only suppresses once', () => {
const { document, textEditor } = createTextDocument(undefined, undefined, 'line1\nline2\nline3');
nextEditProvider.lastRejectionTime = Date.now() - TRIGGER_INLINE_EDIT_REJECTION_COOLDOWN - 1;

triggerTextChange(document);
triggerTextSelectionChange(textEditor, new Selection(0, 0, 0, 0));
const countAfterFirst = firedEvents.length;

// Simulate NES shown
nextEditProvider.shownNesInfo = {
targetDocumentId: DocumentId.create(document.uri.toString()),
expectedCursorLine: 2,
expectedCursorColumn: 0,
};

// First selection change on matching position — suppressed
triggerTextSelectionChange(textEditor, new Selection(2, 0, 2, 0));
assert.strictEqual(firedEvents.length, countAfterFirst,
'First selection change after acceptance should be suppressed');

// Info should be consumed
assert.strictEqual(nextEditProvider.shownNesInfo, undefined,
'shownNesInfo should be consumed after match');

// Second selection change — NOT suppressed
triggerTextChange(document);
triggerTextSelectionChange(textEditor, new Selection(1, 0, 1, 0));
assert.isAtLeast(firedEvents.length, countAfterFirst + 1,
'Second selection change should trigger normally');
});

test('No suppression when cursor column does not match', () => {
const { document, textEditor } = createTextDocument(undefined, undefined, 'line1\nline2\nline3');
nextEditProvider.lastRejectionTime = Date.now() - TRIGGER_INLINE_EDIT_REJECTION_COOLDOWN - 1;

triggerTextChange(document);
triggerTextSelectionChange(textEditor, new Selection(0, 0, 0, 0));
const countAfterFirst = firedEvents.length;

// NES expects cursor at (2, 3)
nextEditProvider.shownNesInfo = {
targetDocumentId: DocumentId.create(document.uri.toString()),
expectedCursorLine: 2,
expectedCursorColumn: 3,
};

// Cursor lands on correct line but wrong column
triggerTextSelectionChange(textEditor, new Selection(2, 0, 2, 0));

assert.isAtLeast(firedEvents.length, countAfterFirst + 1,
'Signal should fire when cursor column does not match');
});

test('No suppression when cursor line does not match', () => {
const { document, textEditor } = createTextDocument(undefined, undefined, 'line1\nline2\nline3');
nextEditProvider.lastRejectionTime = Date.now() - TRIGGER_INLINE_EDIT_REJECTION_COOLDOWN - 1;

triggerTextChange(document);
triggerTextSelectionChange(textEditor, new Selection(0, 0, 0, 0));
const countAfterFirst = firedEvents.length;

// NES expects cursor at (2, 0)
nextEditProvider.shownNesInfo = {
targetDocumentId: DocumentId.create(document.uri.toString()),
expectedCursorLine: 2,
expectedCursorColumn: 0,
};

// Cursor lands on wrong line
triggerTextSelectionChange(textEditor, new Selection(1, 0, 1, 0));

assert.isAtLeast(firedEvents.length, countAfterFirst + 1,
'Signal should fire when cursor line does not match');
});

test('No suppression when NES target doc does not match', () => {
const doc1 = createTextDocument(undefined, Uri.file('file1.py'), 'line1\nline2');
const doc2 = createTextDocument(undefined, Uri.file('file2.py'), 'line1\nline2');
nextEditProvider.lastRejectionTime = Date.now() - TRIGGER_INLINE_EDIT_REJECTION_COOLDOWN - 1;

// NES targets doc2 at (1, 0)
nextEditProvider.shownNesInfo = {
targetDocumentId: DocumentId.create(doc2.document.uri.toString()),
expectedCursorLine: 1,
expectedCursorColumn: 0,
};

// Selection change in doc1 at same position — should NOT be suppressed
triggerTextChange(doc1.document);
triggerTextSelectionChange(doc1.textEditor, new Selection(1, 0, 1, 0));

assert.isAtLeast(firedEvents.length, 1,
'Signal should fire when NES target doc does not match');
// Info should NOT be consumed since doc didn't match
assert.isDefined(nextEditProvider.shownNesInfo,
'shownNesInfo should not be consumed when doc does not match');
});

test('No suppression when no NES info is set', () => {
const { document, textEditor } = createTextDocument(undefined, undefined, 'line1\nline2');
nextEditProvider.lastRejectionTime = Date.now() - TRIGGER_INLINE_EDIT_REJECTION_COOLDOWN - 1;

nextEditProvider.shownNesInfo = undefined;

triggerTextChange(document);
triggerTextSelectionChange(textEditor, new Selection(1, 0, 1, 0));

assert.isAtLeast(firedEvents.length, 1,
'Signal should fire normally when no NES info is set');
});
});

// #endregion
});
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ export class DiagnosticsNextEditProvider extends Disposable implements INextEdit

handleShown(suggestion: DiagnosticsNextEditResult): void { }

consumeShouldSuppressSelectionChangeTrigger(_docId: DocumentId, _cursorLine: number, _cursorColumn: number): boolean {
return false; // diagnostics NES doesn't need trigger suppression
}

handleAcceptance(docId: DocumentId, suggestion: DiagnosticsNextEditResult): void {
const completionResult = suggestion.result;
if (!completionResult) {
Expand Down
5 changes: 5 additions & 0 deletions src/extension/inlineEdits/vscode-node/inlineEditTriggerer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,11 @@ export class InlineEditTriggerer extends Disposable {

const selectionLine = range.start.line;

if (this.nextEditProvider.consumeShouldSuppressSelectionChangeTrigger(doc.id, selectionLine, range.start.character)) {
logger.trace('Return: suppressed after NES acceptance');
return;
}

if (this._isSameLineCooldownActive(mostRecentChange, selectionLine, e.textEditor.document)) {
logger.trace('Return: same line cooldown');
return;
Expand Down
Loading