From 1ae4fa2741b800601f28f260608665d84035fa35 Mon Sep 17 00:00:00 2001 From: Valery Bugakov Date: Wed, 18 Dec 2024 12:34:09 +0800 Subject: [PATCH 1/8] fix(autoedit): fix shrink prediction logic --- vscode/src/autoedits/autoedits-provider.ts | 6 +- .../src/autoedits/shrink-prediction.test.ts | 105 +++++++----------- vscode/src/autoedits/shrink-prediction.ts | 34 ++---- 3 files changed, 54 insertions(+), 91 deletions(-) diff --git a/vscode/src/autoedits/autoedits-provider.ts b/vscode/src/autoedits/autoedits-provider.ts index 96a54c3a547d..b2472fee61fb 100644 --- a/vscode/src/autoedits/autoedits-provider.ts +++ b/vscode/src/autoedits/autoedits-provider.ts @@ -39,7 +39,7 @@ import { extractAutoEditResponseFromCurrentDocumentCommentTemplate, shrinkReplacerTextToCodeToReplaceRange, } from './renderer/renderer-testing' -// import { shrinkPredictionUntilSuffix } from './shrink-prediction' +import { shrinkPredictionUntilSuffix } from './shrink-prediction' const AUTOEDITS_CONTEXT_STRATEGY = 'auto-edits' const INLINE_COMPLETION_DEFAULT_DEBOUNCE_INTERVAL_MS = 150 @@ -219,8 +219,8 @@ export class AutoeditsProvider implements vscode.InlineCompletionItemProvider, v return null } - const { prediction, codeToReplaceData } = autoeditResponse - // prediction = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) + let { prediction, codeToReplaceData } = autoeditResponse + prediction = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) if (prediction === codeToReplaceData.codeToRewrite) { return null diff --git a/vscode/src/autoedits/shrink-prediction.test.ts b/vscode/src/autoedits/shrink-prediction.test.ts index 3f2f8f9c216d..0214fd4df128 100644 --- a/vscode/src/autoedits/shrink-prediction.test.ts +++ b/vscode/src/autoedits/shrink-prediction.test.ts @@ -1,6 +1,5 @@ import dedent from 'dedent' import { describe, expect, it } from 'vitest' - import { getCurrentDocContext } from '../completions/get-current-doc-context' import { documentAndPosition } from '../completions/test-helpers' @@ -8,78 +7,50 @@ import { type CodeToReplaceData, getCurrentFilePromptComponents } from './prompt import { shrinkPredictionUntilSuffix } from './shrink-prediction' describe('shrinkPredictionUntilSuffix', () => { - it('middle of file, no overlap, 4-line prediction', () => { - const codeToReplaceData = createCodeToReplaceData`const a = 1 - const b = 2 - const c = 3 - console.log(a, b, c)█ - function greet() { console.log("Hello") } - const x = 10 - console.log(x) - console.log("end") - ` + it('returns code to rewrite if the prediction does not change anything', () => { + const codeToReplaceData = createCodeToReplaceData` + import { RecentEditsTracker } from '../completions/context/retrievers/recent-user-actions/recent-edits-tracker' - const prediction = dedent`const c = 999 - console.log(a + b, c) - let y = 42 - function greet() { console.log("Changed!") } - ` + export class FilterPredictionEditsBasedOnRecentEdits { - const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) - expect(result.trimEnd()).toBe(prediction) - }) + private readonly recentEditsTracker: RecentEditsTracker - it('middle of file, partial overlap, 4-line prediction', () => { - const codeToReplaceData = createCodeToReplaceData`const a = 1 - const b = 2 - const c = 3 - console.log(a, b, c)█ - function greet() { console.log("Hello") } - console.log(a) - console.log("end") - ` + constructor(recentEditsTracker: RecentEditsTracker) { + this.recentEditsTracker = █ + } - // 4-line prediction. The last line "console.log(a)" is a suffix line and should be overlapped and removed. - const prediction = dedent`const c = 999 - console.log(a * b * c) - function greet() { console.log("Modified hello") } - console.log(a) + }\n\n ` - const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) + const prediction = dedent` constructor(recentEditsTracker: RecentEditsTracker) { + this.recentEditsTracker = recentEditsTracker + }\n\n` - // After removing overlap (console.log(a)), we have 3 lines left. - // This matches the original codeToReplace line count (3 lines). - expect(result.trimEnd()).toBe(withoutLastLines(prediction, 1)) + const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) + expect(result).toBe(prediction) }) - it('middle of file, full overlap, 4-line prediction', () => { - const codeToReplaceData = createCodeToReplaceData`const a = 1 - const b = 2 - const c = 3 - console.log(a, b, c)█ - function greet() { console.log("Hello") } - const x = 10 - console.log(x) - console.log("end") + it('if prediction suggests line additions which duplicate the existing document suffix, remove them from prediction', () => { + const codeToReplaceData = createCodeToReplaceData`class ContactForm: + def __init__(self█, name, message): + self.name = name + self.message = message + self.email = ` - // 4-line prediction that ends with both suffix lines: "const x = 10" and "console.log(x)" - const prediction = dedent`const c = 1000 - console.log(a - b - c) - const x = 10 - console.log(x) + // 4-line prediction. The last line "console.log(a)" is a suffix line and should be overlapped and removed. + const prediction = dedent`class ContactForm: + def __init__(self, name, message, email): + self.name = name + self.message = message + self.email = email ` const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) - // After removing the two overlapping suffix lines ("const x = 10" and "console.log(x)"), - // we have only 2 lines left from prediction. - // Original codeToReplace is 3 lines. The function should append original lines to reach 3 lines total. - expect(result.trimEnd()).toMatchInlineSnapshot(` - "const c = 1000 - console.log(a - b - c) - const c = 3" - `) + + // After removing overlap (sel.email = email), we have 4 lines left. + // This matches the original codeToReplace line count (4 lines). + expect(result.trimEnd()).toBe(withoutLastLines(prediction, 1)) }) it('cursor at end of file, no overlap, 4-line prediction', () => { @@ -102,13 +73,15 @@ describe('shrinkPredictionUntilSuffix', () => { it('cursor near start, partial overlap, 4-line prediction', () => { const codeToReplaceData = createCodeToReplaceData`console.log("start")█ - let val = 123 + let value = 123 + console.log(value) console.log("end") ` // 4-line prediction tries to rewrite "console.log("start")" and includes "console.log("end")" at the end for overlap. const prediction = dedent`console.log("modified start") - let val = 999 + let value = 999 + console.log(value) extra_line_here console.log("end") ` @@ -141,18 +114,18 @@ function createCodeToReplaceData(code: TemplateStringsArray, ...values: unknown[ const docContext = getCurrentDocContext({ document, position, - maxPrefixLength: 100, - maxSuffixLength: 100, + maxPrefixLength: 1000, + maxSuffixLength: 1000, }) return getCurrentFilePromptComponents({ docContext, position, document, - maxPrefixLinesInArea: 2, - maxSuffixLinesInArea: 2, + maxPrefixLinesInArea: 5, + maxSuffixLinesInArea: 5, codeToRewritePrefixLines: 1, - codeToRewriteSuffixLines: 1, + codeToRewriteSuffixLines: 2, }).codeToReplace } diff --git a/vscode/src/autoedits/shrink-prediction.ts b/vscode/src/autoedits/shrink-prediction.ts index 760a2a4223c1..6e0c99f8a7e2 100644 --- a/vscode/src/autoedits/shrink-prediction.ts +++ b/vscode/src/autoedits/shrink-prediction.ts @@ -4,8 +4,6 @@ import type { CodeToReplaceData } from './prompt/prompt-utils' /** * Shrinks the prediction by removing overlapping lines with the suffix. - * If the prediction becomes smaller than the original code to replace, - * appends the missing original lines to maintain the line count. */ export function shrinkPredictionUntilSuffix( prediction: string, @@ -16,9 +14,8 @@ export function shrinkPredictionUntilSuffix( const suffix = codeToReplaceData.suffixInArea + codeToReplaceData.suffixAfterArea // Split the prediction and suffix into arrays of lines - const predictionLines = lines(prediction) + const predictionLines = lines(stripLastEmptyLineIfExists(prediction)) const suffixLines = lines(suffix) - const originalLines = lines(codeToReplaceData.codeToRewrite.trimEnd()) // Determine the maximum possible overlap const maxOverlap = Math.min(predictionLines.length, suffixLines.length) @@ -26,17 +23,16 @@ export function shrinkPredictionUntilSuffix( // Iterate over possible overlap lengths for (let i = 1; i <= maxOverlap; i++) { - // Get the last 'i' lines of the prediction const predictionSlice = predictionLines.slice(-i) - // Get the first 'i' lines of the suffix const suffixSlice = suffixLines.slice(0, i) - // Assume the lines match until proven otherwise - let matches = true + let matches = false for (let j = 0; j < i; j++) { - // Compare lines after trimming whitespace - if (!suffixSlice[j].trim().startsWith(predictionSlice[j].trim())) { - matches = false + if ( + (suffixSlice[j].length > 0 && predictionSlice[j].startsWith(suffixSlice[j])) || + (suffixSlice[j].length === 0 && suffixSlice === predictionSlice) + ) { + matches = true break } } @@ -52,16 +48,10 @@ export function shrinkPredictionUntilSuffix( predictionLines.splice(-overlap, overlap) } - const originalLineCount = originalLines.length - const adjustedPredictionLineCount = predictionLines.length - - // If the prediction has fewer lines than the original, append missing original lines - if (adjustedPredictionLineCount < originalLineCount) { - const missingLineCount = originalLineCount - adjustedPredictionLineCount - const linesToAppend = originalLines.slice(0, missingLineCount) - predictionLines.push(...linesToAppend) - } - - // Return the final adjusted prediction return predictionLines.join(newLineChar) + newLineChar } + +function stripLastEmptyLineIfExists(value: string) { + const newLineChar = getNewLineChar(value) + return value.endsWith(newLineChar) ? value.slice(0, -newLineChar.length) : value +} From 8aa254f09086675a3c5141e840a14b3d53113607 Mon Sep 17 00:00:00 2001 From: Valery Bugakov Date: Wed, 18 Dec 2024 15:23:34 +0800 Subject: [PATCH 2/8] feat(audoedit): fix the condition --- vscode/src/autoedits/shrink-prediction.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/vscode/src/autoedits/shrink-prediction.ts b/vscode/src/autoedits/shrink-prediction.ts index 6e0c99f8a7e2..f998f0d0a56c 100644 --- a/vscode/src/autoedits/shrink-prediction.ts +++ b/vscode/src/autoedits/shrink-prediction.ts @@ -26,13 +26,14 @@ export function shrinkPredictionUntilSuffix( const predictionSlice = predictionLines.slice(-i) const suffixSlice = suffixLines.slice(0, i) - let matches = false + // Assume the lines match until proven otherwise + let matches = true for (let j = 0; j < i; j++) { if ( - (suffixSlice[j].length > 0 && predictionSlice[j].startsWith(suffixSlice[j])) || - (suffixSlice[j].length === 0 && suffixSlice === predictionSlice) + (suffixSlice[j].length > 0 && !predictionSlice[j].startsWith(suffixSlice[j])) || + (suffixSlice[j].length === 0 && suffixSlice !== predictionSlice) ) { - matches = true + matches = false break } } From 7ed4de303317e427a06edf5dc715cb730332db11 Mon Sep 17 00:00:00 2001 From: Valery Bugakov Date: Wed, 18 Dec 2024 15:25:37 +0800 Subject: [PATCH 3/8] feat(audoedit): add a comment --- vscode/src/autoedits/shrink-prediction.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/vscode/src/autoedits/shrink-prediction.ts b/vscode/src/autoedits/shrink-prediction.ts index f998f0d0a56c..280bc5ab4c54 100644 --- a/vscode/src/autoedits/shrink-prediction.ts +++ b/vscode/src/autoedits/shrink-prediction.ts @@ -13,7 +13,10 @@ export function shrinkPredictionUntilSuffix( const newLineChar = getNewLineChar(prediction) const suffix = codeToReplaceData.suffixInArea + codeToReplaceData.suffixAfterArea - // Split the prediction and suffix into arrays of lines + // Remove the last empty line from the prediction because it always ends + // with an extra empty line. This extra line is technically the first line + // of the suffix. Stripping it ensures we can accurately compare the last + // lines of the prediction to the first lines of the suffix. const predictionLines = lines(stripLastEmptyLineIfExists(prediction)) const suffixLines = lines(suffix) From 86fbe0c4ce4150337fcf35f268cf87aed482e951 Mon Sep 17 00:00:00 2001 From: Valery Bugakov Date: Wed, 18 Dec 2024 15:28:45 +0800 Subject: [PATCH 4/8] feat(audoedit): use `codeToRewrite` to get the new line char --- vscode/src/autoedits/shrink-prediction.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/vscode/src/autoedits/shrink-prediction.ts b/vscode/src/autoedits/shrink-prediction.ts index 280bc5ab4c54..a3bd6adc371e 100644 --- a/vscode/src/autoedits/shrink-prediction.ts +++ b/vscode/src/autoedits/shrink-prediction.ts @@ -7,17 +7,20 @@ import type { CodeToReplaceData } from './prompt/prompt-utils' */ export function shrinkPredictionUntilSuffix( prediction: string, - codeToReplaceData: CodeToReplaceData + { suffixInArea, suffixAfterArea, codeToRewrite }: CodeToReplaceData ): string { - // Combine the suffixInArea and suffixAfterArea to get the full suffix - const newLineChar = getNewLineChar(prediction) - const suffix = codeToReplaceData.suffixInArea + codeToReplaceData.suffixAfterArea + const newLineChar = getNewLineChar(codeToRewrite) + const suffix = suffixInArea + suffixAfterArea // Remove the last empty line from the prediction because it always ends // with an extra empty line. This extra line is technically the first line // of the suffix. Stripping it ensures we can accurately compare the last // lines of the prediction to the first lines of the suffix. - const predictionLines = lines(stripLastEmptyLineIfExists(prediction)) + const predictionWithoutLastEmptyLine = prediction.endsWith(newLineChar) + ? prediction.slice(0, -newLineChar.length) + : prediction + + const predictionLines = lines(predictionWithoutLastEmptyLine) const suffixLines = lines(suffix) // Determine the maximum possible overlap @@ -54,8 +57,3 @@ export function shrinkPredictionUntilSuffix( return predictionLines.join(newLineChar) + newLineChar } - -function stripLastEmptyLineIfExists(value: string) { - const newLineChar = getNewLineChar(value) - return value.endsWith(newLineChar) ? value.slice(0, -newLineChar.length) : value -} From e159e229c5b2b67e3876c9cbeab542d18f5b4bc5 Mon Sep 17 00:00:00 2001 From: hitesh-1997 Date: Wed, 18 Dec 2024 21:51:50 +0530 Subject: [PATCH 5/8] repro issue because of partial suffix match with predictions repro issue because of partial suffix match with predictions repro issue because of partial suffix match with predictions --- .../src/autoedits/shrink-prediction.test.ts | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/vscode/src/autoedits/shrink-prediction.test.ts b/vscode/src/autoedits/shrink-prediction.test.ts index 0214fd4df128..a4be712a77a6 100644 --- a/vscode/src/autoedits/shrink-prediction.test.ts +++ b/vscode/src/autoedits/shrink-prediction.test.ts @@ -7,6 +7,44 @@ import { type CodeToReplaceData, getCurrentFilePromptComponents } from './prompt import { shrinkPredictionUntilSuffix } from './shrink-prediction' describe('shrinkPredictionUntilSuffix', () => { + + it.only('repro issue because of partial suffix match', () => { + + // Note: Assume if instead of p there was nothing just the whitespace, then also the additional lines pred_line_1 and pred_line_2 would get trimmed + const codeToReplaceData = createCodeToReplaceData` + import { RecentEditsTracker } from '../completions/context/retrievers/recent-user-actions/recent-edits-tracker' + + export class FilterPredictionEditsBasedOnRecentEdits { + + private readonly recentEditsTracker: RecentEditsTracker + + constructor(recentEditsTracker: RecentEditsTracker) { + this.recentEditsTracker = █ + + + + + + + + + + // some code + ` + + const prediction = dedent` constructor(recentEditsTracker: RecentEditsTracker) { + this.recentEditsTracker = recentEditsTracker + pred_line_1 + pred_line_2 + ` + + const wrongExpectedResult = "constructor(recentEditsTracker: RecentEditsTracker) {\n" + + const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) + expect(result).toBe(wrongExpectedResult) + }) + + it('returns code to rewrite if the prediction does not change anything', () => { const codeToReplaceData = createCodeToReplaceData` import { RecentEditsTracker } from '../completions/context/retrievers/recent-user-actions/recent-edits-tracker' From a7bdee32c4ce3118d76557605f16f83dcea85ef2 Mon Sep 17 00:00:00 2001 From: Valery Bugakov Date: Thu, 19 Dec 2024 10:59:47 +0800 Subject: [PATCH 6/8] fix(audoedit): use exact match --- .../src/autoedits/shrink-prediction.test.ts | 138 +++++++++++++----- vscode/src/autoedits/shrink-prediction.ts | 12 +- 2 files changed, 112 insertions(+), 38 deletions(-) diff --git a/vscode/src/autoedits/shrink-prediction.test.ts b/vscode/src/autoedits/shrink-prediction.test.ts index a4be712a77a6..b3fefadf18b1 100644 --- a/vscode/src/autoedits/shrink-prediction.test.ts +++ b/vscode/src/autoedits/shrink-prediction.test.ts @@ -1,5 +1,6 @@ import dedent from 'dedent' import { describe, expect, it } from 'vitest' + import { getCurrentDocContext } from '../completions/get-current-doc-context' import { documentAndPosition } from '../completions/test-helpers' @@ -7,10 +8,7 @@ import { type CodeToReplaceData, getCurrentFilePromptComponents } from './prompt import { shrinkPredictionUntilSuffix } from './shrink-prediction' describe('shrinkPredictionUntilSuffix', () => { - - it.only('repro issue because of partial suffix match', () => { - - // Note: Assume if instead of p there was nothing just the whitespace, then also the additional lines pred_line_1 and pred_line_2 would get trimmed + it('does not trim the prediction lines that start with the same indentation as the following suffix empty lines', () => { const codeToReplaceData = createCodeToReplaceData` import { RecentEditsTracker } from '../completions/context/retrievers/recent-user-actions/recent-edits-tracker' @@ -20,31 +18,28 @@ describe('shrinkPredictionUntilSuffix', () => { constructor(recentEditsTracker: RecentEditsTracker) { this.recentEditsTracker = █ - - - - - - - - - - // some code + + + + + + + + + + // some code ` const prediction = dedent` constructor(recentEditsTracker: RecentEditsTracker) { this.recentEditsTracker = recentEditsTracker pred_line_1 - pred_line_2 + pred_line_2\n ` - const wrongExpectedResult = "constructor(recentEditsTracker: RecentEditsTracker) {\n" - const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) - expect(result).toBe(wrongExpectedResult) + expect(result).toBe(prediction) }) - it('returns code to rewrite if the prediction does not change anything', () => { const codeToReplaceData = createCodeToReplaceData` import { RecentEditsTracker } from '../completions/context/retrievers/recent-user-actions/recent-edits-tracker' @@ -71,23 +66,21 @@ describe('shrinkPredictionUntilSuffix', () => { it('if prediction suggests line additions which duplicate the existing document suffix, remove them from prediction', () => { const codeToReplaceData = createCodeToReplaceData`class ContactForm: def __init__(self█, name, message): - self.name = name - self.message = message - self.email = + pass + pass + self.email = email ` - // 4-line prediction. The last line "console.log(a)" is a suffix line and should be overlapped and removed. + // Prediction with 4 lines; the last line exactly matches the suffix line "self.email = email". const prediction = dedent`class ContactForm: def __init__(self, name, message, email): - self.name = name - self.message = message + pass + pass self.email = email ` const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) - - // After removing overlap (sel.email = email), we have 4 lines left. - // This matches the original codeToReplace line count (4 lines). + // We expect that last line to be removed (overlap is 1 line). expect(result.trimEnd()).toBe(withoutLastLines(prediction, 1)) }) @@ -97,7 +90,6 @@ describe('shrinkPredictionUntilSuffix', () => { line3█ ` - // 4-line prediction rewriting line3 and adding more lines. const prediction = dedent`line3_modified extra_line1 extra_line2 @@ -105,8 +97,8 @@ describe('shrinkPredictionUntilSuffix', () => { ` const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) - // codeToReplace is smaller, but we have more lines in prediction. No overlap removal needed. - expect(result.trimEnd()).toBe(prediction) + // No overlap to remove, so the prediction remains. + expect(result.trimEnd()).toBe(prediction.trimEnd()) }) it('cursor near start, partial overlap, 4-line prediction', () => { @@ -116,7 +108,7 @@ describe('shrinkPredictionUntilSuffix', () => { console.log("end") ` - // 4-line prediction tries to rewrite "console.log("start")" and includes "console.log("end")" at the end for overlap. + // The last line of prediction "console.log('end')" exactly matches the first line in the suffix "console.log('end')". const prediction = dedent`console.log("modified start") let value = 999 console.log(value) @@ -125,7 +117,6 @@ describe('shrinkPredictionUntilSuffix', () => { ` const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) - // Removing overlap "console.log("end")", leaves us with 3 lines. expect(result.trimEnd()).toBe(withoutLastLines(prediction, 1)) }) @@ -145,6 +136,85 @@ describe('shrinkPredictionUntilSuffix', () => { const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) expect(result).toBe(codeToReplaceData.codeToRewrite) }) + + it('handles empty suffix (no overlap possible)', () => { + const codeToReplaceData = createCodeToReplaceData`test code█` + const prediction = dedent` + test code changed + more lines\n + ` + + const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) + expect(result).toBe(prediction) + }) + + it('handles empty prediction', () => { + const codeToReplaceData = createCodeToReplaceData`some code█ + suffix line1 + suffix line2 + ` + + const prediction = '' + + const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) + expect(result).toBe(prediction) + }) + + it('handles partial line mismatch properly (no partial/startsWith overlap)', () => { + const codeToReplaceData = createCodeToReplaceData`console.log("foo")█ + console.log("bar") + ` + + // The predicted line "console.log("barbaz")" is not an exact match, so no overlap is removed. + const prediction = dedent`console.log("foo changed") + console.log("barbaz")\n + ` + + const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) + expect(result).toBe(prediction) + }) + + it('removes all lines if prediction fully matches suffix line-by-line', () => { + // codeToRewrite is a single line; suffix has 2 lines; the prediction is exactly those 2 lines. + const codeToReplaceData = createCodeToReplaceData` + foo█ + + + line1 + line2\n + ` + + const prediction = dedent` + + line1 + line2\n + ` + + const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) + // Entire prediction is removed => only a single newline remains. + expect(result).toBe('\n') + }) + + it('removes overlapping lines even if they are empty', () => { + const codeToReplaceData = createCodeToReplaceData`line1█ + line2 + + + line3 + line4 + ` + + const prediction = dedent`line1 changed + line2 + + + line3 + line4\n + ` + + const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) + expect(result).toBe(withoutLastLines(prediction, 3)) + }) }) function createCodeToReplaceData(code: TemplateStringsArray, ...values: unknown[]): CodeToReplaceData { @@ -170,6 +240,6 @@ function createCodeToReplaceData(code: TemplateStringsArray, ...values: unknown[ function withoutLastLines(text: string, n: number): string { return text .split('\n') - .slice(0, n > 0 ? -n : 0) + .slice(0, n > 0 ? -n : undefined) .join('\n') } diff --git a/vscode/src/autoedits/shrink-prediction.ts b/vscode/src/autoedits/shrink-prediction.ts index a3bd6adc371e..6dd601049f59 100644 --- a/vscode/src/autoedits/shrink-prediction.ts +++ b/vscode/src/autoedits/shrink-prediction.ts @@ -9,6 +9,10 @@ export function shrinkPredictionUntilSuffix( prediction: string, { suffixInArea, suffixAfterArea, codeToRewrite }: CodeToReplaceData ): string { + if (prediction.length === 0) { + return prediction + } + const newLineChar = getNewLineChar(codeToRewrite) const suffix = suffixInArea + suffixAfterArea @@ -35,10 +39,10 @@ export function shrinkPredictionUntilSuffix( // Assume the lines match until proven otherwise let matches = true for (let j = 0; j < i; j++) { - if ( - (suffixSlice[j].length > 0 && !predictionSlice[j].startsWith(suffixSlice[j])) || - (suffixSlice[j].length === 0 && suffixSlice !== predictionSlice) - ) { + // Using a partial match predictionSlice[j].startWith(suffixSlice[j] here + // gives us too many false positive, so if we encounter cases where the model + // suggests modified suffix lines, we should address it on a different stage. + if (predictionSlice[j] !== suffixSlice[j]) { matches = false break } From c4592faee9baa15ad25c3f09285fb8912f24d6d4 Mon Sep 17 00:00:00 2001 From: Valery Bugakov Date: Wed, 25 Dec 2024 10:47:06 +0800 Subject: [PATCH 7/8] feat(audoedit): restore `isPredictedTextAlreadyInSuffix` --- vscode/src/autoedits/autoedits-provider.ts | 27 +++++++++++++++++++--- vscode/src/autoedits/utils.ts | 23 ++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/vscode/src/autoedits/autoedits-provider.ts b/vscode/src/autoedits/autoedits-provider.ts index 10693d02f699..d3f50b4ef888 100644 --- a/vscode/src/autoedits/autoedits-provider.ts +++ b/vscode/src/autoedits/autoedits-provider.ts @@ -43,6 +43,7 @@ import { shrinkReplacerTextToCodeToReplaceRange, } from './renderer/renderer-testing' import { shrinkPredictionUntilSuffix } from './shrink-prediction' +import { isPredictedTextAlreadyInSuffix } from './utils' const AUTOEDITS_CONTEXT_STRATEGY = 'auto-edits' const INLINE_COMPLETION_DEFAULT_DEBOUNCE_INTERVAL_MS = 150 @@ -224,18 +225,24 @@ export class AutoeditsProvider implements vscode.InlineCompletionItemProvider, v } let { prediction, codeToReplaceData } = autoeditResponse + const { codeToRewrite } = codeToReplaceData + const shouldFilterPredictionBasedRecentEdits = this.filterPrediction.shouldFilterPrediction( document.uri, prediction, - codeToReplaceData.codeToRewrite + codeToRewrite ) if (shouldFilterPredictionBasedRecentEdits) { + autoeditsLogger.logDebug('Autoedits', 'Skipping autoedit - based on recent edits') return null } prediction = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) - - if (prediction === codeToReplaceData.codeToRewrite) { + if (prediction === codeToRewrite) { + autoeditsLogger.logDebug( + 'Autoedits', + 'Skipping autoedit - prediction equals to code to rewrite' + ) return null } @@ -247,6 +254,20 @@ export class AutoeditsProvider implements vscode.InlineCompletionItemProvider, v const decorationInfo = getDecorationInfo(currentFileText, predictedFileText) + if ( + isPredictedTextAlreadyInSuffix({ + codeToRewrite, + decorationInfo, + suffix: codeToReplaceData.suffixInArea + codeToReplaceData.suffixAfterArea, + }) + ) { + autoeditsLogger.logDebug( + 'Autoedits', + 'Skipping autoedit - predicted text already exists in suffix' + ) + return null + } + const { inlineCompletions } = await this.rendererManager.maybeRenderDecorationsAndTryMakeInlineCompletionResponse( prediction, diff --git a/vscode/src/autoedits/utils.ts b/vscode/src/autoedits/utils.ts index 68e52c1d2253..fc5889ea1710 100644 --- a/vscode/src/autoedits/utils.ts +++ b/vscode/src/autoedits/utils.ts @@ -1,5 +1,7 @@ import { getNewLineChar, lines } from '../completions/text-processing' +import type { DecorationInfo } from './renderer/decorators/base' + export function fixFirstLineIndentation(source: string, target: string): string { // Check the first line indentation of source string and replaces in target string. const codeToRewriteLines = lines(source) @@ -56,6 +58,27 @@ function getNumberOfNewLineCharsAtSuffix(text: string): number { return match ? match[0].length : 0 } +export function isPredictedTextAlreadyInSuffix({ + codeToRewrite, + decorationInfo: { addedLines }, + suffix, +}: { + codeToRewrite: string + decorationInfo: DecorationInfo + suffix: string +}): boolean { + if (addedLines.length === 0) { + return false + } + + const allAddedLinesText = addedLines + .sort((a, b) => a.modifiedLineNumber - b.modifiedLineNumber) + .map(line => line.text) + .join(getNewLineChar(codeToRewrite)) + + return suffix.startsWith(allAddedLinesText) +} + /** * Adjusts the prediction to enable inline completion when possible. * From 8331acfc574bfd94f50ae98549c1344ed131a383 Mon Sep 17 00:00:00 2001 From: Valery Bugakov Date: Wed, 25 Dec 2024 10:50:08 +0800 Subject: [PATCH 8/8] feat(audoedit): restore `isPredictedTextAlreadyInSuffix` tests --- vscode/src/autoedits/utils.test.ts | 40 ++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/vscode/src/autoedits/utils.test.ts b/vscode/src/autoedits/utils.test.ts index ab9413626683..91141b48a532 100644 --- a/vscode/src/autoedits/utils.test.ts +++ b/vscode/src/autoedits/utils.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' +import { getDecorationInfo } from './renderer/diff-utils' import * as utils from './utils' describe('fixFirstLineIndentation', () => { @@ -647,3 +648,42 @@ describe('countNewLineCharsStart', () => { expect(utils.countNewLineCharsStart('')).toBe(0) }) }) + +describe('isPredictedTextAlreadyInSuffix', () => { + it('should return false when there are no added lines', () => { + const codeToRewrite = 'const x = 1;\nconst y = 2;' + const prediction = 'const x = 1;\nconst y = 2;' + + const result = utils.isPredictedTextAlreadyInSuffix({ + codeToRewrite, + decorationInfo: getDecorationInfo(codeToRewrite, prediction), + suffix: '', + }) + expect(result).toBe(false) + }) + + it('should return false when predicted text is different from suffix', () => { + const codeToRewrite = 'function test() {\n \n}' + const prediction = 'function test() {\n console.log("hello");\n}' + + const result = utils.isPredictedTextAlreadyInSuffix({ + codeToRewrite, + decorationInfo: getDecorationInfo(codeToRewrite, prediction), + suffix: 'return true;\n}', + }) + expect(result).toBe(false) + }) + + it('should handle multiline predictions correctly', () => { + const codeToRewrite = 'function test() {\n' + const prediction = + 'function test() {\n const a = 1;\n const b = 2;\n console.log(a + b);\n}\n' + + const result = utils.isPredictedTextAlreadyInSuffix({ + codeToRewrite, + decorationInfo: getDecorationInfo(codeToRewrite, prediction), + suffix: ' const a = 1;\n const b = 2;\n console.log(a + b);\n}\n', + }) + expect(result).toBe(true) + }) +})