diff --git a/vscode/src/autoedits/autoedits-provider.ts b/vscode/src/autoedits/autoedits-provider.ts index 37cb1babe19a..d3f50b4ef888 100644 --- a/vscode/src/autoedits/autoedits-provider.ts +++ b/vscode/src/autoedits/autoedits-provider.ts @@ -13,12 +13,14 @@ import { isDotComAuthed, tokensToChars, } from '@sourcegraph/cody-shared' + import { ContextRankingStrategy } from '../completions/context/completions-context-ranker' import { ContextMixer } from '../completions/context/context-mixer' import { DefaultContextStrategyFactory } from '../completions/context/context-strategy' import { RetrieverIdentifier } from '../completions/context/utils' import { getCurrentDocContext } from '../completions/get-current-doc-context' import { getConfiguration } from '../configuration' + import type { AutoeditsModelAdapter, AutoeditsPrompt } from './adapters/base' import { CodyGatewayAdapter } from './adapters/cody-gateway' import { FireworksAdapter } from './adapters/fireworks' @@ -40,7 +42,8 @@ import { extractAutoEditResponseFromCurrentDocumentCommentTemplate, shrinkReplacerTextToCodeToReplaceRange, } from './renderer/renderer-testing' -// import { shrinkPredictionUntilSuffix } from './shrink-prediction' +import { shrinkPredictionUntilSuffix } from './shrink-prediction' +import { isPredictedTextAlreadyInSuffix } from './utils' const AUTOEDITS_CONTEXT_STRATEGY = 'auto-edits' const INLINE_COMPLETION_DEFAULT_DEBOUNCE_INTERVAL_MS = 150 @@ -221,19 +224,25 @@ export class AutoeditsProvider implements vscode.InlineCompletionItemProvider, v return null } - const { prediction, codeToReplaceData } = autoeditResponse + 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) { + prediction = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) + if (prediction === codeToRewrite) { + autoeditsLogger.logDebug( + 'Autoedits', + 'Skipping autoedit - prediction equals to code to rewrite' + ) return null } @@ -245,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/shrink-prediction.test.ts b/vscode/src/autoedits/shrink-prediction.test.ts index 3f2f8f9c216d..b3fefadf18b1 100644 --- a/vscode/src/autoedits/shrink-prediction.test.ts +++ b/vscode/src/autoedits/shrink-prediction.test.ts @@ -8,78 +8,80 @@ 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('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' + + export class FilterPredictionEditsBasedOnRecentEdits { + + private readonly recentEditsTracker: RecentEditsTracker + + constructor(recentEditsTracker: RecentEditsTracker) { + this.recentEditsTracker = █ + + + + + + + + + + // some code ` - const prediction = dedent`const c = 999 - console.log(a + b, c) - let y = 42 - function greet() { console.log("Changed!") } + const prediction = dedent` constructor(recentEditsTracker: RecentEditsTracker) { + this.recentEditsTracker = recentEditsTracker + pred_line_1 + pred_line_2\n ` const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) - expect(result.trimEnd()).toBe(prediction) + expect(result).toBe(prediction) }) - 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") - ` + 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' + + export class FilterPredictionEditsBasedOnRecentEdits { - // 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) + private readonly recentEditsTracker: RecentEditsTracker + + constructor(recentEditsTracker: RecentEditsTracker) { + this.recentEditsTracker = █ + } + + }\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): + pass + pass + self.email = 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) + // 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): + pass + pass + 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" - `) + // We expect that last line to be removed (overlap is 1 line). + expect(result.trimEnd()).toBe(withoutLastLines(prediction, 1)) }) it('cursor at end of file, no overlap, 4-line prediction', () => { @@ -88,7 +90,6 @@ describe('shrinkPredictionUntilSuffix', () => { line3█ ` - // 4-line prediction rewriting line3 and adding more lines. const prediction = dedent`line3_modified extra_line1 extra_line2 @@ -96,25 +97,26 @@ 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', () => { 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. + // 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 val = 999 + let value = 999 + console.log(value) extra_line_here console.log("end") ` const result = shrinkPredictionUntilSuffix(prediction, codeToReplaceData) - // Removing overlap "console.log("end")", leaves us with 3 lines. expect(result.trimEnd()).toBe(withoutLastLines(prediction, 1)) }) @@ -134,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 { @@ -141,24 +222,24 @@ 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 } 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 760a2a4223c1..6dd601049f59 100644 --- a/vscode/src/autoedits/shrink-prediction.ts +++ b/vscode/src/autoedits/shrink-prediction.ts @@ -4,21 +4,28 @@ 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, - 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 + if (prediction.length === 0) { + return prediction + } + + 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 predictionWithoutLastEmptyLine = prediction.endsWith(newLineChar) + ? prediction.slice(0, -newLineChar.length) + : prediction - // Split the prediction and suffix into arrays of lines - const predictionLines = lines(prediction) + const predictionLines = lines(predictionWithoutLastEmptyLine) const suffixLines = lines(suffix) - const originalLines = lines(codeToReplaceData.codeToRewrite.trimEnd()) // Determine the maximum possible overlap const maxOverlap = Math.min(predictionLines.length, suffixLines.length) @@ -26,16 +33,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 for (let j = 0; j < i; j++) { - // Compare lines after trimming whitespace - if (!suffixSlice[j].trim().startsWith(predictionSlice[j].trim())) { + // 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 } @@ -52,16 +59,5 @@ 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 } 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) + }) +}) 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. *