-
Notifications
You must be signed in to change notification settings - Fork 746
refactor(amazonq): separate edits from completion code path #7793
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.
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
Changes from 25 commits
704e81e
4f7b38a
34d7ee9
94c3ceb
3847638
62b3bab
0631329
339832f
d915d27
dd9d602
fc89521
c15744c
6b0d3a7
3d46491
fd81f2e
bcd9125
c60ecca
927578a
157c615
b9ecf20
8128b12
be2da2b
fbde320
cb99bc8
acd1f06
6b88956
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,12 @@ | |
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| import * as vscode from 'vscode' | ||
| import { | ||
| InlineCompletionListWithReferences, | ||
| InlineCompletionWithReferencesParams, | ||
| inlineCompletionWithReferencesRequestType, | ||
| TextDocumentContentChangeEvent, | ||
| editCompletionRequestType, | ||
| } from '@aws/language-server-runtimes/protocol' | ||
| import { CancellationToken, InlineCompletionContext, Position, TextDocument } from 'vscode' | ||
| import { LanguageClient } from 'vscode-languageclient' | ||
|
|
@@ -20,7 +20,8 @@ import { | |
| } from 'aws-core-vscode/codewhisperer' | ||
| import { TelemetryHelper } from './telemetryHelper' | ||
| import { ICursorUpdateRecorder } from './cursorUpdateManager' | ||
| import { getLogger } from 'aws-core-vscode/shared' | ||
| import { getLogger, sleep } from 'aws-core-vscode/shared' | ||
| import { DocumentEventListener } from './documentEventListener' | ||
| import { getOpenFilesInWindow } from 'aws-core-vscode/utils' | ||
| import { asyncCallWithTimeout } from '../../util/timeoutUtil' | ||
|
|
||
|
|
@@ -66,9 +67,11 @@ export class RecommendationService { | |
| context: InlineCompletionContext, | ||
| token: CancellationToken, | ||
| isAutoTrigger: boolean, | ||
| options: GetAllRecommendationsOptions = { emitTelemetry: true, showUi: true }, | ||
| documentChangeEvent?: vscode.TextDocumentChangeEvent | ||
| documentEventListener: DocumentEventListener, | ||
| options: GetAllRecommendationsOptions = { emitTelemetry: true, showUi: true } | ||
| ) { | ||
| const documentChangeEvent = documentEventListener?.getLastDocumentChangeEvent(document.uri.fsPath)?.event | ||
|
|
||
| // Record that a regular request is being made | ||
| this.cursorUpdateRecorder?.recordCompletionRequest() | ||
| const documentChangeParams = documentChangeEvent | ||
|
|
@@ -119,7 +122,51 @@ export class RecommendationService { | |
| }) | ||
| const t0 = performance.now() | ||
|
|
||
| const result = await this.getRecommendationsWithTimeout(languageClient, request, token) | ||
| // Best effort estimate of deletion | ||
| const isTriggerByDeletion = documentEventListener.isLastEventDeletion(document.uri.fsPath) | ||
|
|
||
| const ps: Promise<InlineCompletionListWithReferences>[] = [] | ||
| /** | ||
| * IsTriggerByDeletion is to prevent user deletion invoking Completions. | ||
| * PartialResultToken is not a hack for now since only Edits suggestion use partialResultToken across different calls of [getAllRecommendations], | ||
| * Completions use PartialResultToken with single 1 call of [getAllRecommendations]. | ||
| * Edits leverage partialResultToken to achieve EditStreak such that clients can pull all continuous suggestions generated by the model within 1 EOS block. | ||
| */ | ||
| if (!isTriggerByDeletion && !request.partialResultToken) { | ||
| const completionPromise: Promise<InlineCompletionListWithReferences> = languageClient.sendRequest( | ||
| inlineCompletionWithReferencesRequestType.method, | ||
| request, | ||
| token | ||
| ) | ||
| ps.push(completionPromise) | ||
| } | ||
|
|
||
| /** | ||
| * 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. | ||
| */ | ||
| const editPromise: Promise<InlineCompletionListWithReferences> = languageClient.sendRequest( | ||
| editCompletionRequestType.method, | ||
| request, | ||
| token | ||
| ) | ||
| ps.push(editPromise) | ||
|
|
||
| /** | ||
| * First come first serve, ideally we should simply return the first response returned. However there are some caviar here because either | ||
| * (1) promise might be returned early without going through service | ||
| * (2) some users are not enabled with edits suggestion, therefore service will return empty result without passing through the model | ||
| * 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. | ||
| * This is the hack to return first "NON-EMPTY" response | ||
| */ | ||
| let result = await Promise.race(ps) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. donee |
||
| if (ps.length > 1 && result.items.length === 0) { | ||
| for (const p of ps) { | ||
| const r = await p | ||
| if (r.items.length > 0) { | ||
| result = r | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+162
to
+169
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ugly, but this is the best effort we can do atm
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add another field |
||
|
|
||
| getLogger().info('Received inline completion response from LSP: %O', { | ||
| sessionId: result.sessionId, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leigaol fyi, logic to handle trigger when users are deleting the code is moved here