Skip to content

Fix inline suggestion false negatives #325269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ulugbekna wants to merge 1 commit into from
Closed

Fix inline suggestion false negatives #325269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
177 changes: 169 additions & 8 deletions extensions/copilot/src/extension/inlineEdits/test/vscode-node/isInlineSuggestion.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
*--------------------------------------------------------------------------------------------*/

import { assert, suite, test } from 'vitest';
import { Position, Range, Uri } from 'vscode';
import { Position, Range, type TextDocument, Uri } from 'vscode';
import { createTextDocumentData } from '../../../../util/common/test/shims/textDocument';
import { toInlineSuggestion } from '../../vscode-node/isInlineSuggestion';
import { isSubword, toInlineSuggestion } from '../../vscode-node/isInlineSuggestion';

suite('toInlineSuggestion', () => {

Expand All @@ -22,6 +22,42 @@ suite('toInlineSuggestion', () => {
return { document, completionInsertionPoint, replaceRange, replaceText };
}

function applyEdit(document: TextDocument, range: Range, newText: string): string {
const documentText = document.getText();
return documentText.substring(0, document.offsetAt(range.start)) + newText + documentText.substring(document.offsetAt(range.end));
}

function canRepresentAsInlineSuggestion(document: TextDocument, cursorPosition: Position, range: Range, newText: string): boolean {
const documentText = document.getText();
const editedText = applyEdit(document, range, newText);
const lineLength = document.lineAt(cursorPosition.line).text.length;

for (let startCharacter = 0; startCharacter <= cursorPosition.character; startCharacter++) {
for (let endCharacter = cursorPosition.character; endCharacter <= lineLength; endCharacter++) {
const candidateRange = new Range(cursorPosition.line, startCharacter, cursorPosition.line, endCharacter);
const prefix = documentText.substring(0, document.offsetAt(candidateRange.start));
const suffix = documentText.substring(document.offsetAt(candidateRange.end));
const candidateNewTextEnd = editedText.length - suffix.length;
if (candidateNewTextEnd < prefix.length || !editedText.startsWith(prefix) || !editedText.endsWith(suffix)) {
continue;
}

const candidateNewText = editedText.substring(prefix.length, candidateNewTextEnd);
const replacedText = document.getText(candidateRange);
const cursorOffset = cursorPosition.character - startCharacter;
if (
applyEdit(document, candidateRange, candidateNewText) === editedText
&& replacedText.substring(0, cursorOffset) === candidateNewText.substring(0, cursorOffset)
&& isSubword(replacedText, candidateNewText)
) {
return true;
}
}
}

return false;
}

test('line before completion', () => {
const { document, completionInsertionPoint, replaceRange, replaceText } = getBaseCompletionScenario();

Expand Down Expand Up @@ -257,14 +293,18 @@ function createDocumentSymbol(

// --- Branch 1 regression: next-line insertion edge cases ---

test('next-line: cursor mid-line rejects even with valid next-line edit', () => {
test('next-line: cursor mid-line extends through the unchanged line suffix', () => {
const document = createMockDocument(['function foo(bar', '', 'other']);
const cursorPosition = new Position(0, 8); // middle of "function foo(bar"
const replaceRange = new Range(1, 0, 1, 0);
const replaceText = ' param1,\n param2\n';

// Cursor not at end of line → rejected
assert.isUndefined(toInlineSuggestion(cursorPosition, document, replaceRange, replaceText));
const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);
assert.deepStrictEqual(result, {
range: new Range(0, 8, 0, 16),
newText: ' foo(bar\n param1,\n param2',
});
assert.strictEqual(applyEdit(document, result!.range, result!.newText), applyEdit(document, replaceRange, replaceText));
});

test('next-line: non-empty range on next line falls through and is rejected', () => {
Expand Down Expand Up @@ -347,16 +387,137 @@ function createDocumentSymbol(
assert.isUndefined(toInlineSuggestion(cursorPosition, document, replaceRange, replaceText));
});

suite('equivalent edit rebasing', () => {

test('rebases a next-line replacement preserving the closing line', () => {
const document = createMockDocument([
'\tprivate gatherNeighborSnippets(',
'\t\tactiveDocument: StatelessNextEditDocument,',
'\t) {',
'\t\treturn undefined;',
]);
const cursorPosition = document.lineAt(1).range.end;
const replaceRange = new Range(2, 0, 2, 4);
const replaceText = [
'\t\tcurrentDocument: StatelessNextEditDocument,',
'\t\tpromptOptions: ModelConfig,',
'\t\tdelaySession: DelaySession,',
'\t\ttracer: RequestTracingContext,',
'\t\tcancellationToken: CancellationToken,',
'\t) {',
].join('\n');

const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);

assert.deepStrictEqual(result, {
range: new Range(cursorPosition, cursorPosition),
newText: [
'',
'\t\tcurrentDocument: StatelessNextEditDocument,',
'\t\tpromptOptions: ModelConfig,',
'\t\tdelaySession: DelaySession,',
'\t\ttracer: RequestTracingContext,',
'\t\tcancellationToken: CancellationToken,',
].join('\n'),
});
assert.strictEqual(applyEdit(document, result!.range, result!.newText), applyEdit(document, replaceRange, replaceText));
});

test('rebases a replacement starting after indentation on the next line', () => {
const document = createMockDocument(['function foo(', ' close', 'other']);
const cursorPosition = new Position(0, 13);
const replaceRange = new Range(1, 2, 1, 7);
const replaceText = 'item\n close';

const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);

assert.deepStrictEqual(result, {
range: new Range(cursorPosition, cursorPosition),
newText: '\n item',
});
assert.strictEqual(applyEdit(document, result!.range, result!.newText), applyEdit(document, replaceRange, replaceText));
});

test('does not rebase when the replacement changes the closing line', () => {
const document = createMockDocument(['function foo(', ' close', 'other']);
const cursorPosition = new Position(0, 13);
const replaceRange = new Range(1, 2, 1, 7);
const replaceText = 'item\n changed';

assert.isUndefined(toInlineSuggestion(cursorPosition, document, replaceRange, replaceText));
});

test('collapses a multi-line edit around the cursor using unchanged prefix and suffix', () => {
const document = createMockDocument(['header', 'abc', 'close']);
const cursorPosition = new Position(1, 1);
const replaceRange = new Range(0, 0, 2, 5);
const replaceText = 'header\naXbc\nclose';

const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);

assert.deepStrictEqual(result, {
range: new Range(1, 1, 1, 3),
newText: 'Xbc',
});
assert.strictEqual(applyEdit(document, result!.range, result!.newText), applyEdit(document, replaceRange, replaceText));
});

test('finds every representable edit in a finite exhaustive corpus', () => {
const documents: readonly { text: string; eol: '\n' | '\r\n' }[] = [
{ text: '', eol: '\n' },
{ text: 'a', eol: '\n' },
{ text: 'ab', eol: '\n' },
{ text: 'a\n', eol: '\n' },
{ text: 'a\nb', eol: '\n' },
{ text: 'ab\nc', eol: '\n' },
{ text: 'a\nb\nc', eol: '\n' },
{ text: 'a\r\nb', eol: '\r\n' },
{ text: 'ab\r\nc', eol: '\r\n' },
];
const newTexts = ['', 'a', 'b', 'ab', '\n', '\r\n', 'a\n', 'a\r\n', '\nb', '\r\nb', 'a\nb', 'a\r\nb', 'b\na', 'a\nb\nc'];

for (const { text: documentText, eol } of documents) {
const document = createTextDocumentData(Uri.from({ scheme: 'test', path: '/test/file.ts' }), documentText, 'typescript', eol).document;
const positions: Position[] = [];
for (let line = 0; line < document.lineCount; line++) {
for (let character = 0; character <= document.lineAt(line).text.length; character++) {
positions.push(new Position(line, character));
}
}

for (const cursorPosition of positions) {
for (let startIndex = 0; startIndex < positions.length; startIndex++) {
for (let endIndex = startIndex; endIndex < positions.length; endIndex++) {
const range = new Range(positions[startIndex], positions[endIndex]);
for (const newText of newTexts) {
if (!canRepresentAsInlineSuggestion(document, cursorPosition, range, newText)) {
continue;
}

const result = toInlineSuggestion(cursorPosition, document, range, newText);
assert.isDefined(result, JSON.stringify({ documentText, cursorPosition, range, newText }));
assert.strictEqual(applyEdit(document, result!.range, result!.newText), applyEdit(document, range, newText));
}
}
}
}
}
});
});

// --- Branch 2 regression: same-line edit edge cases ---

test('same-line: cursor before range start rejects', () => {
test('same-line: edit after cursor is extended back to cursor', () => {
const document = createMockDocument(['abcdef']);
const cursorPosition = new Position(0, 1);
const replaceRange = new Range(0, 3, 0, 6); // replaces "def"
const replaceText = 'defgh';

// cursorOffsetInReplacedText < 0
assert.isUndefined(toInlineSuggestion(cursorPosition, document, replaceRange, replaceText));
const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);
assert.deepStrictEqual(result, {
range: new Range(0, 1, 0, 6),
newText: 'bcdefgh',
});
});

test('same-line: text before cursor differs rejects', () => {
Expand Down
79 changes: 44 additions & 35 deletions extensions/copilot/src/extension/inlineEdits/vscode-node/isInlineSuggestion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,59 @@ export interface InlineSuggestionEdit {
* which is required for VS Code to render ghost text.
*/
export function toInlineSuggestion(cursorPos: Position, doc: TextDocument, range: Range, newText: string, advanced: boolean = true): InlineSuggestionEdit | undefined {
// Special case: a multi-line insertion that starts on the line *after* the cursor
// can be re-expressed as a pure insertion at the cursor.
if (range.start.line === range.end.line && range.start.line === cursorPos.line) {
const sameLineEdit = validateSameLineGhostText(cursorPos, doc, range, newText);
if (sameLineEdit) {
return sameLineEdit;
}
}

if (advanced) {
const cursorEdit = tryRebaseAsCursorEdit(cursorPos, doc, range, newText);
if (cursorEdit) {
return cursorEdit;
}
}

// Preserve the established behavior for insertions into an empty next line.
const nextLineInsertion = tryAdjustNextLineInsertion(cursorPos, doc, range, newText);
if (nextLineInsertion) {
return nextLineInsertion;
}

// If the range spans multiple lines, try to collapse it to a single line by
// trimming a shared prefix up to a newline boundary.
if (advanced && range.start.line !== range.end.line) {
({ range, newText } = stripCommonLinePrefix(doc, range, newText));
}
return undefined;
}

/**
* Re-express an edit as an equivalent edit from the cursor to the end of its
* line. If any equivalent inline suggestion exists, one exists in this form.
*/
function tryRebaseAsCursorEdit(cursorPos: Position, doc: TextDocument, range: Range, newText: string): InlineSuggestionEdit | undefined {
const cursorOffset = doc.offsetAt(cursorPos);
const lineEnd = doc.lineAt(cursorPos.line).range.end;
const lineEndOffset = doc.offsetAt(lineEnd);
const rangeStartOffset = doc.offsetAt(range.start);
const rangeEndOffset = doc.offsetAt(range.end);
const affectedStart = doc.positionAt(Math.min(cursorOffset, rangeStartOffset));
const affectedEnd = doc.positionAt(Math.max(lineEndOffset, rangeEndOffset));

// Ghost text requires the edit to be on the cursor's line.
if (range.start.line !== range.end.line || range.start.line !== cursorPos.line) {
const editedText = doc.getText(new Range(affectedStart, range.start)) + newText + doc.getText(new Range(range.end, affectedEnd));
const unchangedPrefix = doc.getText(new Range(affectedStart, cursorPos));
const unchangedSuffix = doc.getText(new Range(lineEnd, affectedEnd));
const cursorEditTextEnd = editedText.length - unchangedSuffix.length;
if (
cursorEditTextEnd < unchangedPrefix.length
|| !editedText.startsWith(unchangedPrefix)
|| !editedText.endsWith(unchangedSuffix)
) {
return undefined;
}

return validateSameLineGhostText(cursorPos, doc, range, newText);
const cursorEdit = {
range: new Range(cursorPos, lineEnd),
newText: editedText.substring(unchangedPrefix.length, cursorEditTextEnd),
};
return validateSameLineGhostText(cursorPos, doc, cursorEdit.range, cursorEdit.newText);
}

/**
Expand Down Expand Up @@ -74,30 +108,6 @@ function tryAdjustNextLineInsertion(cursorPos: Position, doc: TextDocument, rang
return { range: new Range(cursorPos, cursorPos), newText: lineBreak + trimmedNewText };
}

/**
* Strip the longest shared prefix that ends on a newline boundary from both sides
* of a multi-line edit. This often shrinks the range so it fits on a single line,
* which is required for ghost text rendering.
*/
function stripCommonLinePrefix(doc: TextDocument, range: Range, newText: string): { range: Range; newText: string } {
const replacedText = doc.getText(range);
const maxLen = Math.min(replacedText.length, newText.length);
let commonLen = 0;
while (commonLen < maxLen && replacedText[commonLen] === newText[commonLen]) {
commonLen++;
}
if (commonLen === 0) {
return { range, newText };
}
const lastNewline = replacedText.lastIndexOf('\n', commonLen - 1);
if (lastNewline < 0) {
return { range, newText };
}
const strippedLen = lastNewline + 1;
const newStart = doc.positionAt(doc.offsetAt(range.start) + strippedLen);
return { range: new Range(newStart, range.end), newText: newText.substring(strippedLen) };
}

/**
* Validate that a single-line edit can be rendered as ghost text at the cursor:
* - the cursor is at or after `range.start`
Expand Down Expand Up @@ -133,4 +143,3 @@ export function isSubword(a: string, b: string): boolean {
}
return true;
}

Loading