From e543423904f8c4e09b8012cf93b99d3de1465a9b Mon Sep 17 00:00:00 2001 From: hitesh-1997 Date: Fri, 10 Jan 2025 03:59:46 +0530 Subject: [PATCH 1/7] feat(auto-edits): fix the partial decoration issue when not enough lines in the editor --- .../src/autoedits/renderer/decorators/base.ts | 10 +++ .../renderer/decorators/default-decorator.ts | 86 ++++++++++++++++--- .../decorators/inline-diff-decorator.ts | 5 ++ vscode/src/autoedits/renderer/manager.ts | 10 +++ 4 files changed, 101 insertions(+), 10 deletions(-) diff --git a/vscode/src/autoedits/renderer/decorators/base.ts b/vscode/src/autoedits/renderer/decorators/base.ts index b9acf0c18bf3..00a1082a41a7 100644 --- a/vscode/src/autoedits/renderer/decorators/base.ts +++ b/vscode/src/autoedits/renderer/decorators/base.ts @@ -31,6 +31,16 @@ export interface AutoEditsDecorator extends vscode.Disposable { * and how they should be decorated in the editor. */ setDecorations(decorationInfo: DecorationInfo): void + + /** + * Checks if the decorator can render decorations for the given decoration information. + * + * This method verifies if the current editor state allows for the decorations to be + * rendered properly. Some conditions that might prevent rendering include: + * - Insufficient lines in the editor + * @returns true if decorations can be rendered, false otherwise + */ + canRenderDecoration(decorationInfo: DecorationInfo): boolean } /** diff --git a/vscode/src/autoedits/renderer/decorators/default-decorator.ts b/vscode/src/autoedits/renderer/decorators/default-decorator.ts index 6035c935288d..cf3757e91367 100644 --- a/vscode/src/autoedits/renderer/decorators/default-decorator.ts +++ b/vscode/src/autoedits/renderer/decorators/default-decorator.ts @@ -11,15 +11,41 @@ interface AddedLinesDecorationInfo { lineText: string } +interface DiffDecorationAddedLinesInfo { + /** Information about lines that have been added */ + addedLinesDecorationInfo: AddedLinesDecorationInfo[] + /** Starting line number for the decoration */ + startLine: number + /** Column position for the replacement text */ + replacerCol: number +} + +/** + * Information about diff decorations to be applied to lines in the editor + */ +interface DiffDecorationInfo { + /** Ranges of text that have been removed */ + removedRangesInfo: vscode.Range[] + /** Information about lines that have been added */ + addedLinesInfo?: DiffDecorationAddedLinesInfo +} + export class DefaultDecorator implements AutoEditsDecorator { private readonly decorationTypes: vscode.TextEditorDecorationType[] + private readonly editor: vscode.TextEditor + + // Decoration types private readonly removedTextDecorationType: vscode.TextEditorDecorationType private readonly modifiedTextDecorationType: vscode.TextEditorDecorationType private readonly suggesterType: vscode.TextEditorDecorationType - private readonly hideRemainderDecorationType: vscode.TextEditorDecorationType private readonly addedLinesDecorationType: vscode.TextEditorDecorationType private readonly insertMarkerDecorationType: vscode.TextEditorDecorationType - private readonly editor: vscode.TextEditor + + /** + * Pre-computed information about diff decorations to be applied to lines in the editor + */ + + private diffDecorationInfo: DiffDecorationInfo | undefined constructor(editor: vscode.TextEditor) { this.editor = editor @@ -34,9 +60,6 @@ export class DefaultDecorator implements AutoEditsDecorator { before: { color: GHOST_TEXT_COLOR }, after: { color: GHOST_TEXT_COLOR }, }) - this.hideRemainderDecorationType = vscode.window.createTextEditorDecorationType({ - opacity: '0', - }) this.addedLinesDecorationType = vscode.window.createTextEditorDecorationType({ backgroundColor: 'red', // SENTINEL (should not actually appear) before: { @@ -55,12 +78,28 @@ export class DefaultDecorator implements AutoEditsDecorator { this.removedTextDecorationType, this.modifiedTextDecorationType, this.suggesterType, - this.hideRemainderDecorationType, this.addedLinesDecorationType, this.insertMarkerDecorationType, ] } + public canRenderDecoration(decorationInfo: DecorationInfo): boolean { + if (!this.diffDecorationInfo) { + this.diffDecorationInfo = this.getDiffDecorationsInfo(decorationInfo) + } + const { addedLinesInfo } = this.diffDecorationInfo + if (addedLinesInfo) { + // Check if there are enough lines in the editor to render the diff decorations + if ( + addedLinesInfo.startLine + addedLinesInfo.addedLinesDecorationInfo.length >= + this.editor.document.lineCount + ) { + return false + } + } + return true + } + private clearDecorations(): void { for (const decorationType of this.decorationTypes) { this.editor.setDecorations(decorationType, []) @@ -90,6 +129,27 @@ export class DefaultDecorator implements AutoEditsDecorator { } private renderDiffDecorations(decorationInfo: DecorationInfo): void { + if (!this.diffDecorationInfo) { + this.diffDecorationInfo = this.getDiffDecorationsInfo(decorationInfo) + } + this.editor.setDecorations( + this.modifiedTextDecorationType, + this.diffDecorationInfo.removedRangesInfo + ) + const addedLinesInfo = this.diffDecorationInfo.addedLinesInfo + + if (!addedLinesInfo) { + return + } + + this.renderAddedLinesDecorations( + addedLinesInfo.addedLinesDecorationInfo, + addedLinesInfo.startLine, + addedLinesInfo.replacerCol + ) + } + + private getDiffDecorationsInfo(decorationInfo: DecorationInfo): DiffDecorationInfo { const { modifiedLines, addedLines, unchangedLines } = decorationInfo // Display the removed range decorations @@ -119,7 +179,6 @@ export class DefaultDecorator implements AutoEditsDecorator { }) } } - this.editor.setDecorations(this.modifiedTextDecorationType, removedRanges) // Handle fully added lines for (const addedLine of addedLines) { @@ -151,13 +210,20 @@ export class DefaultDecorator implements AutoEditsDecorator { // Sort addedLinesInfo by line number in ascending order addedLinesInfo.sort((a, b) => a.afterLine - b.afterLine) if (addedLinesInfo.length === 0) { - return + return { removedRangesInfo: removedRanges } } - // todo (hitesh): handle case when too many lines to fit in the editor const oldLines = addedLinesInfo.map(info => this.editor.document.lineAt(info.afterLine)) const replacerCol = Math.max(...oldLines.map(line => line.range.end.character)) const startLine = Math.min(...oldLines.map(line => line.lineNumber)) - this.renderAddedLinesDecorations(addedLinesInfo, startLine, replacerCol) + + return { + removedRangesInfo: removedRanges, + addedLinesInfo: { + addedLinesDecorationInfo: addedLinesInfo, + startLine, + replacerCol, + }, + } } private renderAddedLinesDecorations( diff --git a/vscode/src/autoedits/renderer/decorators/inline-diff-decorator.ts b/vscode/src/autoedits/renderer/decorators/inline-diff-decorator.ts index 400512beefdb..b855bb95136b 100644 --- a/vscode/src/autoedits/renderer/decorators/inline-diff-decorator.ts +++ b/vscode/src/autoedits/renderer/decorators/inline-diff-decorator.ts @@ -19,6 +19,11 @@ export class InlineDiffDecorator implements vscode.Disposable, AutoEditsDecorato this.editor.setDecorations(this.addedTextDecorationType, added) } + public canRenderDecoration(decorationInfo: DecorationInfo): boolean { + // Inline decorator can render any decoration, so it should always return true. + return true + } + /** * Process modified lines to create decorations for inserted and deleted text within those lines. */ diff --git a/vscode/src/autoedits/renderer/manager.ts b/vscode/src/autoedits/renderer/manager.ts index b32f0bda523b..abb259567ff2 100644 --- a/vscode/src/autoedits/renderer/manager.ts +++ b/vscode/src/autoedits/renderer/manager.ts @@ -184,6 +184,16 @@ export class AutoEditsDefaultRendererManager implements AutoEditsRendererManager this.activeRequestId = requestId this.decorator = this.createDecorator(vscode.window.activeTextEditor!) + if ( + 'decorationInfo' in request && + request.decorationInfo && + !this.decorator.canRenderDecoration(request.decorationInfo) + ) { + // If the decorator cannot render the decoration properly, dispose of it and return early. + this.decorator.dispose() + return + } + autoeditAnalyticsLogger.markAsSuggested(requestId) // Clear any existing timeouts, only one suggestion can be shown at a time From 6fb246014b46c6a3c82deb52630307cbbe0d1dda Mon Sep 17 00:00:00 2001 From: hitesh-1997 Date: Fri, 10 Jan 2025 04:02:48 +0530 Subject: [PATCH 2/7] feat(auto-edits): make decorator null if cannot render deocrations --- vscode/src/autoedits/renderer/manager.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/vscode/src/autoedits/renderer/manager.ts b/vscode/src/autoedits/renderer/manager.ts index abb259567ff2..6a04e2fc119c 100644 --- a/vscode/src/autoedits/renderer/manager.ts +++ b/vscode/src/autoedits/renderer/manager.ts @@ -191,6 +191,7 @@ export class AutoEditsDefaultRendererManager implements AutoEditsRendererManager ) { // If the decorator cannot render the decoration properly, dispose of it and return early. this.decorator.dispose() + this.decorator = null return } From 164315aedf92deb5944e0fcdafe2f5e6f011e82d Mon Sep 17 00:00:00 2001 From: hitesh-1997 Date: Fri, 10 Jan 2025 04:04:33 +0530 Subject: [PATCH 3/7] fix order --- vscode/src/autoedits/renderer/manager.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vscode/src/autoedits/renderer/manager.ts b/vscode/src/autoedits/renderer/manager.ts index 6a04e2fc119c..3afffa8d7dbb 100644 --- a/vscode/src/autoedits/renderer/manager.ts +++ b/vscode/src/autoedits/renderer/manager.ts @@ -181,9 +181,7 @@ export class AutoEditsDefaultRendererManager implements AutoEditsRendererManager return } - this.activeRequestId = requestId this.decorator = this.createDecorator(vscode.window.activeTextEditor!) - if ( 'decorationInfo' in request && request.decorationInfo && @@ -195,6 +193,7 @@ export class AutoEditsDefaultRendererManager implements AutoEditsRendererManager return } + this.activeRequestId = requestId autoeditAnalyticsLogger.markAsSuggested(requestId) // Clear any existing timeouts, only one suggestion can be shown at a time From 414ac1f462cca83ee845e61c8bcad4594bb2bd9c Mon Sep 17 00:00:00 2001 From: hitesh-1997 Date: Fri, 10 Jan 2025 14:34:54 +0530 Subject: [PATCH 4/7] add discard reason for dismissing decorations --- .../analytics-logger/analytics-logger.ts | 2 ++ .../renderer/decorators/default-decorator.ts | 3 +-- vscode/src/autoedits/renderer/manager.ts | 22 ++++++++++++++----- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/vscode/src/autoedits/analytics-logger/analytics-logger.ts b/vscode/src/autoedits/analytics-logger/analytics-logger.ts index af4b7fd5b65d..404edc793329 100644 --- a/vscode/src/autoedits/analytics-logger/analytics-logger.ts +++ b/vscode/src/autoedits/analytics-logger/analytics-logger.ts @@ -173,6 +173,8 @@ export const autoeditDiscardReason = { suffixOverlap: 5, emptyPredictionAfterInlineCompletionExtraction: 6, noActiveEditor: 7, + conflictingDecorationWithEdits: 8, + noEnoughLinesEditor: 9, } as const /** We use numeric keys to send these to the analytics backend */ diff --git a/vscode/src/autoedits/renderer/decorators/default-decorator.ts b/vscode/src/autoedits/renderer/decorators/default-decorator.ts index cf3757e91367..4482b128f9d0 100644 --- a/vscode/src/autoedits/renderer/decorators/default-decorator.ts +++ b/vscode/src/autoedits/renderer/decorators/default-decorator.ts @@ -42,9 +42,8 @@ export class DefaultDecorator implements AutoEditsDecorator { private readonly insertMarkerDecorationType: vscode.TextEditorDecorationType /** - * Pre-computed information about diff decorations to be applied to lines in the editor + * Pre-computed information about diff decorations to be applied to lines in the editor. */ - private diffDecorationInfo: DiffDecorationInfo | undefined constructor(editor: vscode.TextEditor) { diff --git a/vscode/src/autoedits/renderer/manager.ts b/vscode/src/autoedits/renderer/manager.ts index 3afffa8d7dbb..e890244bef70 100644 --- a/vscode/src/autoedits/renderer/manager.ts +++ b/vscode/src/autoedits/renderer/manager.ts @@ -7,7 +7,11 @@ import { getLatestVisibilityContext, isCompletionVisible, } from '../../completions/is-completion-visible' -import { type AutoeditRequestID, autoeditAnalyticsLogger } from '../analytics-logger' +import { + type AutoeditRequestID, + autoeditAnalyticsLogger, + autoeditDiscardReason, +} from '../analytics-logger' import { autoeditsProviderConfig } from '../autoedits-config' import { autoeditsOutputChannelLogger } from '../output-channel-logger' import type { CodeToReplaceData } from '../prompt/prompt-utils' @@ -174,10 +178,14 @@ export class AutoEditsDefaultRendererManager implements AutoEditsRendererManager await this.rejectActiveEdit() const request = autoeditAnalyticsLogger.getRequest(requestId) - if ( - !request || - this.hasConflictingDecorations(request.document, request.codeToReplaceData.range) - ) { + if (!request) { + return + } + if (this.hasConflictingDecorations(request.document, request.codeToReplaceData.range)) { + autoeditAnalyticsLogger.markAsDiscarded({ + requestId, + discardReason: autoeditDiscardReason.conflictingDecorationWithEdits, + }) return } @@ -190,6 +198,10 @@ export class AutoEditsDefaultRendererManager implements AutoEditsRendererManager // If the decorator cannot render the decoration properly, dispose of it and return early. this.decorator.dispose() this.decorator = null + autoeditAnalyticsLogger.markAsDiscarded({ + requestId, + discardReason: autoeditDiscardReason.noEnoughLinesEditor, + }) return } From cd86320aca7a16b42c0fa2187aa6305c45ccec52 Mon Sep 17 00:00:00 2001 From: hitesh-1997 Date: Fri, 10 Jan 2025 14:44:17 +0530 Subject: [PATCH 5/7] fix condition --- vscode/src/autoedits/renderer/decorators/default-decorator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vscode/src/autoedits/renderer/decorators/default-decorator.ts b/vscode/src/autoedits/renderer/decorators/default-decorator.ts index 4482b128f9d0..4c31b0604ebf 100644 --- a/vscode/src/autoedits/renderer/decorators/default-decorator.ts +++ b/vscode/src/autoedits/renderer/decorators/default-decorator.ts @@ -90,7 +90,7 @@ export class DefaultDecorator implements AutoEditsDecorator { if (addedLinesInfo) { // Check if there are enough lines in the editor to render the diff decorations if ( - addedLinesInfo.startLine + addedLinesInfo.addedLinesDecorationInfo.length >= + addedLinesInfo.startLine + addedLinesInfo.addedLinesDecorationInfo.length > this.editor.document.lineCount ) { return false From 4f6c6aa0bdfc43eb9f8b8a4da8c08e3614565166 Mon Sep 17 00:00:00 2001 From: hitesh-1997 Date: Fri, 10 Jan 2025 15:19:52 +0530 Subject: [PATCH 6/7] add state transition --- vscode/src/autoedits/analytics-logger/analytics-logger.ts | 2 +- vscode/src/autoedits/renderer/decorators/default-decorator.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/vscode/src/autoedits/analytics-logger/analytics-logger.ts b/vscode/src/autoedits/analytics-logger/analytics-logger.ts index 404edc793329..05527e3c630e 100644 --- a/vscode/src/autoedits/analytics-logger/analytics-logger.ts +++ b/vscode/src/autoedits/analytics-logger/analytics-logger.ts @@ -86,7 +86,7 @@ const validRequestTransitions = { started: ['contextLoaded', 'discarded'], contextLoaded: ['loaded', 'discarded'], loaded: ['postProcessed', 'discarded'], - postProcessed: ['suggested'], + postProcessed: ['suggested', 'discarded'], suggested: ['read', 'accepted', 'rejected'], read: ['accepted', 'rejected'], accepted: [], diff --git a/vscode/src/autoedits/renderer/decorators/default-decorator.ts b/vscode/src/autoedits/renderer/decorators/default-decorator.ts index 4c31b0604ebf..05b2ee0327af 100644 --- a/vscode/src/autoedits/renderer/decorators/default-decorator.ts +++ b/vscode/src/autoedits/renderer/decorators/default-decorator.ts @@ -211,7 +211,9 @@ export class DefaultDecorator implements AutoEditsDecorator { if (addedLinesInfo.length === 0) { return { removedRangesInfo: removedRanges } } - const oldLines = addedLinesInfo.map(info => this.editor.document.lineAt(info.afterLine)) + const oldLines = addedLinesInfo + .filter(info => info.afterLine < this.editor.document.lineCount) + .map(info => this.editor.document.lineAt(info.afterLine)) const replacerCol = Math.max(...oldLines.map(line => line.range.end.character)) const startLine = Math.min(...oldLines.map(line => line.lineNumber)) From 847e4bc45fa4efb097b0a4edbe880f7b785e7544 Mon Sep 17 00:00:00 2001 From: hitesh-1997 Date: Mon, 13 Jan 2025 16:16:54 +0530 Subject: [PATCH 7/7] address pr review comments --- vscode/src/autoedits/analytics-logger/analytics-logger.ts | 2 +- vscode/src/autoedits/renderer/manager.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vscode/src/autoedits/analytics-logger/analytics-logger.ts b/vscode/src/autoedits/analytics-logger/analytics-logger.ts index 05527e3c630e..120cb5350fe5 100644 --- a/vscode/src/autoedits/analytics-logger/analytics-logger.ts +++ b/vscode/src/autoedits/analytics-logger/analytics-logger.ts @@ -174,7 +174,7 @@ export const autoeditDiscardReason = { emptyPredictionAfterInlineCompletionExtraction: 6, noActiveEditor: 7, conflictingDecorationWithEdits: 8, - noEnoughLinesEditor: 9, + notEnoughLinesEditor: 9, } as const /** We use numeric keys to send these to the analytics backend */ diff --git a/vscode/src/autoedits/renderer/manager.ts b/vscode/src/autoedits/renderer/manager.ts index e890244bef70..1ebc29b6d8a3 100644 --- a/vscode/src/autoedits/renderer/manager.ts +++ b/vscode/src/autoedits/renderer/manager.ts @@ -200,7 +200,7 @@ export class AutoEditsDefaultRendererManager implements AutoEditsRendererManager this.decorator = null autoeditAnalyticsLogger.markAsDiscarded({ requestId, - discardReason: autoeditDiscardReason.noEnoughLinesEditor, + discardReason: autoeditDiscardReason.notEnoughLinesEditor, }) return }