-
Notifications
You must be signed in to change notification settings - Fork 241
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
feat(amazonq): Prefetch next inline recommendation #5290
base: main
Are you sure you want to change the base?
Changes from all commits
d3ba908
2b4656e
e0ddf0b
b779722
f9284bd
75ff2a8
7a4693f
88347ed
3c886d2
4a946b9
f54fc34
43fc8d5
af913a2
a57190f
84aa418
142c94f
a87d570
da8644b
f3096cc
cf53dce
99d07db
cd084e1
397812b
cc908ef
0f1cbde
8704181
73a77f1
4fa59f2
0f84fc9
a558734
eb58762
a797dd4
fb8785b
c0f22e2
ed30848
18a3a1a
88f63b4
8f13841
4a9595b
f0659dc
e0819c2
ea4c279
3e817df
07741c1
d9795fb
b1b5c4d
2f801e6
7c119b6
a38455d
6e19c00
e44dc3a
8bd524f
b8f5ca9
8f38495
2806ffb
3f0a5e7
60b7039
59a79a6
fd901db
fba3832
aab84a7
53ea267
9499ea0
9978b8e
1e28cd1
4942279
221ba69
daf9688
3a430cc
9723e2a
639db28
d9f4083
0128423
e38b3cf
1b1f8db
4b8e310
3e5a6e9
1f62c49
4b9993a
629e929
b6564fa
df3cab4
d50a7bb
30168ae
fe1bf11
b8ab355
cc8288f
14ff542
c2b0d56
f117d68
e2e056e
38799b2
b4cbaab
45fccd2
73b2505
ed941b6
c7a8cce
a49dc99
e6e79c1
c545e48
c3ddd40
33e8530
a6bc09c
7f85f6d
8144e20
adca8f3
eb5d551
62749e6
5bed72d
f806491
2b6ce05
9c9fa32
cad3f25
957e329
d6f3a2d
5219ed9
917ce5d
15d7cd1
e604b8b
d2667f1
29205ed
1d11b7d
7877e7b
df5deae
33a1b8e
cb0e587
0d02b1b
3d75a4f
7e2e46a
5dd4c0c
a64d783
054f88f
71bd5a2
81c41d1
0968121
ac3404f
aefeabd
fa58868
6fee511
56f575f
6a3353b
d80b020
e070457
7a9d69a
fcd8bd6
e569d09
2470cf7
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 |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"type" : "feature", | ||
"description" : "Amazon Q: Instantly show the next recommendation after the current recommendation is accepted" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ import com.intellij.openapi.editor.Editor | |
import com.intellij.openapi.editor.VisualPosition | ||
import com.intellij.openapi.project.Project | ||
import com.intellij.openapi.ui.popup.JBPopup | ||
import com.intellij.openapi.ui.popup.JBPopupFactory | ||
import com.intellij.openapi.util.Disposer | ||
import com.intellij.psi.PsiDocumentManager | ||
import com.intellij.psi.PsiFile | ||
|
@@ -24,6 +25,7 @@ import com.intellij.util.messages.Topic | |
import kotlinx.coroutines.CoroutineScope | ||
import kotlinx.coroutines.Deferred | ||
import kotlinx.coroutines.Job | ||
import kotlinx.coroutines.SupervisorJob | ||
import kotlinx.coroutines.async | ||
import kotlinx.coroutines.delay | ||
import kotlinx.coroutines.isActive | ||
|
@@ -98,6 +100,7 @@ import java.util.concurrent.TimeUnit | |
class CodeWhispererService(private val cs: CoroutineScope) : Disposable { | ||
private val codeInsightSettingsFacade = CodeInsightsSettingsFacade() | ||
private var refreshFailure: Int = 0 | ||
private var nextInvocationContext: InvocationContext? = null | ||
|
||
init { | ||
Disposer.register(this, codeInsightSettingsFacade) | ||
|
@@ -209,7 +212,118 @@ class CodeWhispererService(private val cs: CoroutineScope) : Disposable { | |
invokeCodeWhispererInBackground(requestContext) | ||
} | ||
|
||
internal suspend fun invokeCodeWhispererInBackground(requestContext: RequestContext): Job { | ||
internal suspend fun invokeCodeWhispererInBackground( | ||
requestContext: RequestContext, | ||
currStates: InvocationContext? = null, | ||
): Job { | ||
// current states != null means that it's prefetch | ||
if (currStates != null) { | ||
val firstValidRecommendation = currStates.recommendationContext.details | ||
.firstOrNull { | ||
!it.isDiscarded && it.recommendation.content().isNotEmpty() | ||
} ?: return SupervisorJob().apply { complete() } | ||
val job = cs.launch(getCoroutineBgContext()) { | ||
val latencyContext = LatencyContext().apply { | ||
codewhispererPreprocessingStart = System.nanoTime() | ||
codewhispererEndToEndStart = System.nanoTime() | ||
} | ||
|
||
val nextCaretPosition = CaretPosition( | ||
line = requestContext.caretPosition.line + firstValidRecommendation.recommendation.content().count { it == '\n' }, | ||
offset = requestContext.caretPosition.offset + firstValidRecommendation.recommendation.content().length | ||
) | ||
|
||
val nextFileContextInfo = requestContext.fileContextInfo.copy( | ||
caretContext = requestContext.fileContextInfo.caretContext.copy( | ||
leftFileContext = requestContext.fileContextInfo.caretContext.leftFileContext + firstValidRecommendation.recommendation.content() | ||
) | ||
) | ||
|
||
val nextRequestContext = requestContext.copy( | ||
caretPosition = nextCaretPosition, | ||
fileContextInfo = nextFileContextInfo, | ||
latencyContext = latencyContext | ||
) | ||
val newVisualPosition = withContext(EDT) { | ||
runReadAction { | ||
nextRequestContext.editor.offsetToVisualPosition(nextRequestContext.caretPosition.offset) | ||
} | ||
} | ||
try { | ||
val nextResponse = CodeWhispererClientAdaptor | ||
.getInstance(nextRequestContext.project) | ||
.generateCompletions( | ||
buildCodeWhispererRequest( | ||
nextRequestContext.fileContextInfo, | ||
nextRequestContext.awaitSupplementalContext(), | ||
nextRequestContext.customizationArn | ||
) | ||
) | ||
val startTime = System.nanoTime() | ||
nextRequestContext.latencyContext.codewhispererPreprocessingEnd = System.nanoTime() | ||
nextRequestContext.latencyContext.paginationAllCompletionsStart = System.nanoTime() | ||
CodeWhispererInvocationStatus.getInstance().setInvocationStart() | ||
nextResponse.let { | ||
val endTime = System.nanoTime() | ||
val latency = TimeUnit.NANOSECONDS.toMillis(endTime - startTime).toDouble() | ||
val requestId = nextResponse.responseMetadata().requestId() | ||
val sessionId = nextResponse.sdkHttpResponse().headers().getOrDefault(KET_SESSION_ID, listOf(requestId))[0] | ||
|
||
nextRequestContext.latencyContext.apply { | ||
codewhispererPostprocessingStart = System.nanoTime() | ||
paginationFirstCompletionTime = (endTime - codewhispererEndToEndStart).toDouble() | ||
firstRequestId = requestId | ||
} | ||
|
||
CodeWhispererInvocationStatus.getInstance().setInvocationSessionId(sessionId) | ||
|
||
val nextResponseContext = ResponseContext(sessionId) | ||
CodeWhispererTelemetryService.getInstance().sendServiceInvocationEvent( | ||
nextResponse.responseMetadata().requestId(), | ||
nextRequestContext, | ||
nextResponseContext, | ||
nextResponse.completions().size, | ||
invocationSuccess = true, | ||
latency, | ||
null | ||
) | ||
val validatedResponse = validateResponse(it) | ||
val detailContexts = withContext(EDT) { | ||
runReadAction { | ||
CodeWhispererRecommendationManager.getInstance().buildDetailContext( | ||
nextRequestContext, | ||
"", | ||
validatedResponse.completions(), | ||
validatedResponse.responseMetadata().requestId() | ||
) | ||
} | ||
} | ||
val nextRecommendationContext = RecommendationContext(detailContexts, "", "", newVisualPosition) | ||
val newPopup = withContext(EDT) { | ||
JBPopupFactory.getInstance().createMessage("Dummy popup") | ||
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. Is this ever shown to the user? Should this be a more user-friendly message? 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. This dummy popup is not shown to the user. I created it as a placeholder since it's required for the nextInvocationContext variable. If there's a need to display a popup in the future, a new user-friendly popup will be created accordingly. 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. can't we just handle the case where there is no prior popup there? 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. The If the creation of this dummy popup seems unusual, an alternative would be to make the popup field nullable. However, this would require adding null checks at every location where To minimize the impact, maintain consistency and ensure this feature can be launched this week, I opted for this approach. If there are better suggestions for handling this scenario, I'm happy to discuss them! |
||
} | ||
|
||
// send userDecision and trigger decision when next recommendation haven't been seen | ||
if (currStates.popup.isDisposed) { | ||
CodeWhispererTelemetryService.getInstance().sendUserDecisionEventForAll( | ||
nextRequestContext, | ||
nextResponseContext, | ||
nextRecommendationContext, | ||
SessionContext(), | ||
false | ||
) | ||
} else { | ||
nextInvocationContext = InvocationContext(nextRequestContext, nextResponseContext, nextRecommendationContext, newPopup) | ||
} | ||
LOG.debug { "Prefetched next invocation stored in nextInvocationContext" } | ||
} | ||
} catch (ex: Exception) { | ||
LOG.warn { "Failed to prefetch next codewhisperer invocation: ${ex.message}" } | ||
} | ||
} | ||
return job | ||
} | ||
|
||
val popup = withContext(EDT) { | ||
CodeWhispererPopupManager.getInstance().initPopup().also { | ||
Disposer.register(it) { CodeWhispererInvocationStatus.getInstance().finishInvocation() } | ||
|
@@ -491,6 +605,9 @@ class CodeWhispererService(private val cs: CoroutineScope) : Disposable { | |
CodeWhispererPopupManager.getInstance().cancelPopup(popup) | ||
return null | ||
} | ||
cs.launch(getCoroutineBgContext()) { | ||
invokeCodeWhispererInBackground(requestContext, nextStates) | ||
} | ||
} else { | ||
// subsequent responses | ||
nextStates = updateStates(currStates, response) | ||
|
@@ -616,6 +733,34 @@ class CodeWhispererService(private val cs: CoroutineScope) : Disposable { | |
CodeWhispererPopupManager.getInstance().changeStates(states, 0, recommendationAdded) | ||
} | ||
|
||
fun promoteNextInvocationIfAvailable() { | ||
val nextStates = nextInvocationContext ?: run { | ||
LOG.debug { "No nextInvocationContext found, nothing to promote." } | ||
return | ||
} | ||
nextInvocationContext?.popup?.let { Disposer.dispose(it) } | ||
nextInvocationContext = null | ||
|
||
runInEdt { | ||
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. if we're already using coroutines, we do not need to use runInEdt 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. Thank you for pointing that out! I use 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.
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. Thanks for the clarification! Just to confirm—are you suggesting that I replace |
||
val newPopup = CodeWhispererPopupManager.getInstance().initPopup() | ||
val updatedNextStates = nextStates.copy(popup = newPopup).also { | ||
addPopupChildDisposables(it.requestContext.project, it.requestContext.editor, it.popup) | ||
Disposer.register(newPopup, it) | ||
} | ||
CodeWhispererPopupManager.getInstance().initPopupListener(updatedNextStates) | ||
CodeWhispererPopupManager.getInstance().changeStates( | ||
updatedNextStates, | ||
0, | ||
recommendationAdded = false | ||
) | ||
cs.launch(getCoroutineBgContext()) { | ||
invokeCodeWhispererInBackground(updatedNextStates.requestContext, updatedNextStates) | ||
} | ||
} | ||
|
||
LOG.debug { "Promoted nextInvocationContext to current session and displayed next recommendation." } | ||
} | ||
|
||
private fun sendDiscardedUserDecisionEventForAll( | ||
requestContext: RequestContext, | ||
responseContext: ResponseContext, | ||
|
@@ -782,6 +927,8 @@ class CodeWhispererService(private val cs: CoroutineScope) : Disposable { | |
|
||
override fun dispose() {} | ||
|
||
fun getNextInvocationContext(): InvocationContext? = nextInvocationContext | ||
|
||
companion object { | ||
private val LOG = getLogger<CodeWhispererService>() | ||
private const val MAX_REFRESH_ATTEMPT = 3 | ||
|
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.
does this actually need to return a job?
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.
The function's return type is explicitly defined as Job, and there is an existing test that relies on this return value. So I return a
SupervisorJob
to maintain consistency with current codebase.https://github.com/evanliu048/aws-toolkit-jetbrains/blob/prefechNextInlineRecommendation/plugins/amazonq/codewhisperer/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/codewhisperer/CodeWhispererServiceTest.kt#L219