Skip to content

Commit 76a1cd0

Browse files
authored
refactor(amazonq): separate edits from completion code path (#7793)
## Problem related aws/language-servers#2058 Edits as a newly introduced feature, previously the implementation was living in the same flow as Completion. However the 2 suggestion types have few different product requirements, thus we decided to completely separate these 2 suggestions out in terms of their code path. Key difference - Language server will process Completion request on receiving request from IDE clients, and will BLOCK following requests if in-flight request is not fulfilled yet. Edits, on the other hand, it debounces/delay the execution time for the sake of "latest" file context to ensure the suggestion reflects the CORRECT changes users attempt to make. - Triggering heuristic. For example, Completion is not supposed to be invoked when users are deleting the code, whereas Edits allow such scenario to go through. ## Solution - Introduce a new `onEditCompletion` language server API which is purely for Edits suggestion. - Invoke 2 Flare API and serve the response which first arrives (First come first served). - Allow Edits suggestion on code deletion --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 7ff0659 commit 76a1cd0

File tree

3 files changed

+104
-34
lines changed

3 files changed

+104
-34
lines changed

packages/amazonq/src/app/inline/completion.ts

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -276,12 +276,6 @@ export class AmazonQInlineCompletionItemProvider implements InlineCompletionItem
276276

277277
// yield event loop to let the document listen catch updates
278278
await sleep(1)
279-
// prevent user deletion invoking auto trigger
280-
// this is a best effort estimate of deletion
281-
if (this.documentEventListener.isLastEventDeletion(document.uri.fsPath)) {
282-
getLogger().debug('Skip auto trigger when deleting code')
283-
return []
284-
}
285279

286280
let logstr = `GenerateCompletion metadata:\\n`
287281
try {
@@ -370,8 +364,8 @@ export class AmazonQInlineCompletionItemProvider implements InlineCompletionItem
370364
},
371365
token,
372366
isAutoTrigger,
373-
getAllRecommendationsOptions,
374-
this.documentEventListener.getLastDocumentChangeEvent(document.uri.fsPath)?.event
367+
this.documentEventListener,
368+
getAllRecommendationsOptions
375369
)
376370
// get active item from session for displaying
377371
const items = this.sessionManager.getActiveRecommendation()
@@ -404,21 +398,24 @@ ${itemLog}
404398

405399
const cursorPosition = document.validatePosition(position)
406400

407-
if (position.isAfter(editor.selection.active)) {
408-
const params: LogInlineCompletionSessionResultsParams = {
409-
sessionId: session.sessionId,
410-
completionSessionResult: {
411-
[itemId]: {
412-
seen: false,
413-
accepted: false,
414-
discarded: true,
401+
// Completion will not be rendered if users cursor moves to a position which is before the position when the service is invoked
402+
if (items.length > 0 && !items[0].isInlineEdit) {
403+
if (position.isAfter(editor.selection.active)) {
404+
const params: LogInlineCompletionSessionResultsParams = {
405+
sessionId: session.sessionId,
406+
completionSessionResult: {
407+
[itemId]: {
408+
seen: false,
409+
accepted: false,
410+
discarded: true,
411+
},
415412
},
416-
},
413+
}
414+
this.languageClient.sendNotification(this.logSessionResultMessageName, params)
415+
this.sessionManager.clear()
416+
logstr += `- cursor moved behind trigger position. Discarding completion suggestion...`
417+
return []
417418
}
418-
this.languageClient.sendNotification(this.logSessionResultMessageName, params)
419-
this.sessionManager.clear()
420-
logstr += `- cursor moved behind trigger position. Discarding suggestion...`
421-
return []
422419
}
423420

424421
// delay the suggestion rendeing if user is actively typing

packages/amazonq/src/app/inline/recommendationService.ts

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
5-
import * as vscode from 'vscode'
65
import {
76
InlineCompletionListWithReferences,
87
InlineCompletionWithReferencesParams,
98
inlineCompletionWithReferencesRequestType,
109
TextDocumentContentChangeEvent,
10+
editCompletionRequestType,
1111
} from '@aws/language-server-runtimes/protocol'
1212
import { CancellationToken, InlineCompletionContext, Position, TextDocument } from 'vscode'
1313
import { LanguageClient } from 'vscode-languageclient'
@@ -21,6 +21,7 @@ import {
2121
import { TelemetryHelper } from './telemetryHelper'
2222
import { ICursorUpdateRecorder } from './cursorUpdateManager'
2323
import { getLogger } from 'aws-core-vscode/shared'
24+
import { DocumentEventListener } from './documentEventListener'
2425
import { getOpenFilesInWindow } from 'aws-core-vscode/utils'
2526
import { asyncCallWithTimeout } from '../../util/timeoutUtil'
2627

@@ -66,9 +67,11 @@ export class RecommendationService {
6667
context: InlineCompletionContext,
6768
token: CancellationToken,
6869
isAutoTrigger: boolean,
69-
options: GetAllRecommendationsOptions = { emitTelemetry: true, showUi: true },
70-
documentChangeEvent?: vscode.TextDocumentChangeEvent
70+
documentEventListener: DocumentEventListener,
71+
options: GetAllRecommendationsOptions = { emitTelemetry: true, showUi: true }
7172
) {
73+
const documentChangeEvent = documentEventListener?.getLastDocumentChangeEvent(document.uri.fsPath)?.event
74+
7275
// Record that a regular request is being made
7376
this.cursorUpdateRecorder?.recordCompletionRequest()
7477
const documentChangeParams = documentChangeEvent
@@ -119,7 +122,51 @@ export class RecommendationService {
119122
})
120123
const t0 = performance.now()
121124

122-
const result = await this.getRecommendationsWithTimeout(languageClient, request, token)
125+
// Best effort estimate of deletion
126+
const isTriggerByDeletion = documentEventListener.isLastEventDeletion(document.uri.fsPath)
127+
128+
const ps: Promise<InlineCompletionListWithReferences>[] = []
129+
/**
130+
* IsTriggerByDeletion is to prevent user deletion invoking Completions.
131+
* PartialResultToken is not a hack for now since only Edits suggestion use partialResultToken across different calls of [getAllRecommendations],
132+
* Completions use PartialResultToken with single 1 call of [getAllRecommendations].
133+
* Edits leverage partialResultToken to achieve EditStreak such that clients can pull all continuous suggestions generated by the model within 1 EOS block.
134+
*/
135+
if (!isTriggerByDeletion && !request.partialResultToken) {
136+
const completionPromise: Promise<InlineCompletionListWithReferences> = languageClient.sendRequest(
137+
inlineCompletionWithReferencesRequestType.method,
138+
request,
139+
token
140+
)
141+
ps.push(completionPromise)
142+
}
143+
144+
/**
145+
* Though Edit request is sent on keystrokes everytime, the language server will execute the request in a debounced manner so that it won't be immediately executed.
146+
*/
147+
const editPromise: Promise<InlineCompletionListWithReferences> = languageClient.sendRequest(
148+
editCompletionRequestType.method,
149+
request,
150+
token
151+
)
152+
ps.push(editPromise)
153+
154+
/**
155+
* First come first serve, ideally we should simply return the first response returned. However there are some caviar here because either
156+
* (1) promise might be returned early without going through service
157+
* (2) some users are not enabled with edits suggestion, therefore service will return empty result without passing through the model
158+
* With the scenarios listed above or others, it's possible that 1 promise will ALWAYS win the race and users will NOT get any suggestion back.
159+
* This is the hack to return first "NON-EMPTY" response
160+
*/
161+
let result = await Promise.race(ps)
162+
if (ps.length > 1 && result.items.length === 0) {
163+
for (const p of ps) {
164+
const r = await p
165+
if (r.items.length > 0) {
166+
result = r
167+
}
168+
}
169+
}
123170

124171
getLogger().info('Received inline completion response from LSP: %O', {
125172
sessionId: result.sessionId,

packages/amazonq/test/unit/amazonq/apps/inline/recommendationService.test.ts

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ import { createMockDocument } from 'aws-core-vscode/test'
1414
import { CursorUpdateManager } from '../../../../../src/app/inline/cursorUpdateManager'
1515
import { CodeWhispererStatusBarManager } from 'aws-core-vscode/codewhisperer'
1616
import { globals } from 'aws-core-vscode/shared'
17+
import { DocumentEventListener } from '../../../../../src/app/inline/documentEventListener'
18+
19+
const completionApi = 'aws/textDocument/inlineCompletionWithReferences'
20+
const editApi = 'aws/textDocument/editCompletion'
1721

1822
describe('RecommendationService', () => {
1923
let languageClient: LanguageClient
@@ -28,6 +32,10 @@ describe('RecommendationService', () => {
2832
const mockPosition = { line: 0, character: 0 } as Position
2933
const mockContext = { triggerKind: InlineCompletionTriggerKind.Automatic, selectedCompletionInfo: undefined }
3034
const mockToken = { isCancellationRequested: false } as CancellationToken
35+
const mockDocumentEventListener = {
36+
isLastEventDeletion: (filepath: string) => false,
37+
getLastDocumentChangeEvent: (filepath: string) => undefined,
38+
} as DocumentEventListener
3139
const mockInlineCompletionItemOne = {
3240
insertText: 'ItemOne',
3341
} as InlineCompletionItem
@@ -134,12 +142,19 @@ describe('RecommendationService', () => {
134142
mockPosition,
135143
mockContext,
136144
mockToken,
137-
true
145+
true,
146+
mockDocumentEventListener
138147
)
139148

140149
// Verify sendRequest was called with correct parameters
141-
assert(sendRequestStub.calledOnce)
142-
const requestArgs = sendRequestStub.firstCall.args[1]
150+
const cs = sendRequestStub.getCalls()
151+
const completionCalls = cs.filter((c) => c.firstArg === completionApi)
152+
const editCalls = cs.filter((c) => c.firstArg === editApi)
153+
assert.strictEqual(cs.length, 2)
154+
assert.strictEqual(completionCalls.length, 1)
155+
assert.strictEqual(editCalls.length, 1)
156+
157+
const requestArgs = completionCalls[0].args[1]
143158
assert.deepStrictEqual(requestArgs, {
144159
textDocument: {
145160
uri: 'file:///test.py',
@@ -177,12 +192,19 @@ describe('RecommendationService', () => {
177192
mockPosition,
178193
mockContext,
179194
mockToken,
180-
true
195+
true,
196+
mockDocumentEventListener
181197
)
182198

183199
// Verify sendRequest was called with correct parameters
184-
assert(sendRequestStub.calledTwice)
185-
const firstRequestArgs = sendRequestStub.firstCall.args[1]
200+
const cs = sendRequestStub.getCalls()
201+
const completionCalls = cs.filter((c) => c.firstArg === completionApi)
202+
const editCalls = cs.filter((c) => c.firstArg === editApi)
203+
assert.strictEqual(cs.length, 3)
204+
assert.strictEqual(completionCalls.length, 2)
205+
assert.strictEqual(editCalls.length, 1)
206+
207+
const firstRequestArgs = completionCalls[0].args[1]
186208
const expectedRequestArgs = {
187209
textDocument: {
188210
uri: 'file:///test.py',
@@ -192,7 +214,7 @@ describe('RecommendationService', () => {
192214
documentChangeParams: undefined,
193215
openTabFilepaths: [],
194216
}
195-
const secondRequestArgs = sendRequestStub.secondCall.args[1]
217+
const secondRequestArgs = completionCalls[1].args[1]
196218
assert.deepStrictEqual(firstRequestArgs, expectedRequestArgs)
197219
assert.deepStrictEqual(secondRequestArgs, {
198220
...expectedRequestArgs,
@@ -218,7 +240,8 @@ describe('RecommendationService', () => {
218240
mockPosition,
219241
mockContext,
220242
mockToken,
221-
true
243+
true,
244+
mockDocumentEventListener
222245
)
223246

224247
// Verify recordCompletionRequest was called
@@ -235,6 +258,7 @@ describe('RecommendationService', () => {
235258
mockContext,
236259
mockToken,
237260
true,
261+
mockDocumentEventListener,
238262
{
239263
showUi: false,
240264
emitTelemetry: true,
@@ -254,7 +278,8 @@ describe('RecommendationService', () => {
254278
mockPosition,
255279
mockContext,
256280
mockToken,
257-
true
281+
true,
282+
mockDocumentEventListener
258283
)
259284

260285
// Verify UI methods were called
@@ -286,6 +311,7 @@ describe('RecommendationService', () => {
286311
mockContext,
287312
mockToken,
288313
true,
314+
mockDocumentEventListener,
289315
options
290316
)
291317

0 commit comments

Comments
 (0)