From 41bc8854d337eb8f86c5b1dc1e3308fdd9ff870f Mon Sep 17 00:00:00 2001 From: Andrew Yu Date: Mon, 3 Feb 2025 12:20:17 -0800 Subject: [PATCH 1/4] bug fix(amazonq): fix an issue where user action is accept but telemetry is reject --- .../popup/CodeWhispererPopupManager.kt | 19 +++++++++++----- ...hispererPopupIntelliSenseAcceptListener.kt | 22 +++++++++++++++---- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt index 86549103c5..cfea9f331e 100644 --- a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt +++ b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt @@ -85,7 +85,8 @@ import javax.swing.JLabel class CodeWhispererPopupManager { val popupComponents = CodeWhispererPopupComponents() - var shouldListenerCancelPopup: Boolean = true + // Act like a semaphore: one increment only corresponds to one decrement + var shouldEditorChangeCancelPopup: Int = 0 var sessionContext = SessionContext() private set @@ -247,10 +248,16 @@ class CodeWhispererPopupManager { fun dontClosePopupAndRun(runnable: () -> Unit) { try { - shouldListenerCancelPopup = false + shouldEditorChangeCancelPopup++ + LOG.debug { "Incrementing shouldListenerCancelPopup semaphore value" } runnable() } finally { - shouldListenerCancelPopup = true + if (shouldEditorChangeCancelPopup <= 0) { + LOG.error("shouldListenerCancelPopup semaphore is not updated correctly") + } else { + shouldEditorChangeCancelPopup-- + LOG.debug { "Decrementing shouldListenerCancelPopup semaphore value" } + } } } @@ -496,7 +503,7 @@ class CodeWhispererPopupManager { val editor = states.requestContext.editor val codewhispererSelectionListener: SelectionListener = object : SelectionListener { override fun selectionChanged(event: SelectionEvent) { - if (shouldListenerCancelPopup) { + if (shouldEditorChangeCancelPopup == 0) { cancelPopup(states.popup) } super.selectionChanged(event) @@ -512,7 +519,7 @@ class CodeWhispererPopupManager { if (!delete) return if (editor.caretModel.offset == event.offset) { changeStates(states, 0) - } else if (shouldListenerCancelPopup) { + } else if (shouldEditorChangeCancelPopup == 0) { cancelPopup(states.popup) } } @@ -521,7 +528,7 @@ class CodeWhispererPopupManager { val codewhispererCaretListener: CaretListener = object : CaretListener { override fun caretPositionChanged(event: CaretEvent) { - if (shouldListenerCancelPopup) { + if (shouldEditorChangeCancelPopup == 0) { cancelPopup(states.popup) } super.caretPositionChanged(event) diff --git a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt index 6a6eb4b888..32e3e11dcc 100644 --- a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt +++ b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt @@ -8,8 +8,11 @@ import com.intellij.codeInsight.lookup.LookupEvent import com.intellij.codeInsight.lookup.LookupListener import com.intellij.codeInsight.lookup.LookupManagerListener import com.intellij.codeInsight.lookup.impl.LookupImpl +import software.aws.toolkits.core.utils.debug +import software.aws.toolkits.core.utils.getLogger import software.aws.toolkits.jetbrains.services.codewhisperer.model.InvocationContext import software.aws.toolkits.jetbrains.services.codewhisperer.popup.CodeWhispererPopupManager +import software.aws.toolkits.jetbrains.services.codewhisperer.popup.listeners.CodeWhispererPopupIntelliSenseAcceptListener.Companion.LOG import software.aws.toolkits.jetbrains.services.codewhisperer.service.CodeWhispererInvocationStatus class CodeWhispererPopupIntelliSenseAcceptListener(private val states: InvocationContext) : LookupManagerListener { @@ -18,14 +21,20 @@ class CodeWhispererPopupIntelliSenseAcceptListener(private val states: Invocatio addIntelliSenseAcceptListener(newLookup, states) } + + companion object { + val LOG = getLogger() + } } fun addIntelliSenseAcceptListener(lookup: Lookup, states: InvocationContext) { lookup.addLookupListener(object : LookupListener { - override fun beforeItemSelected(event: LookupEvent): Boolean { - CodeWhispererPopupManager.getInstance().shouldListenerCancelPopup = false - return super.beforeItemSelected(event) + override fun lookupShown(event: LookupEvent) { + CodeWhispererPopupManager.getInstance().shouldEditorChangeCancelPopup++ + LOG.debug { "Incrementing shouldListenerCancelPopup semaphore value" } + super.lookupShown(event) } + override fun itemSelected(event: LookupEvent) { if (!CodeWhispererInvocationStatus.getInstance().isDisplaySessionActive() || !(event.lookup as LookupImpl).isShown @@ -46,7 +55,12 @@ fun addIntelliSenseAcceptListener(lookup: Lookup, states: InvocationContext) { private fun cleanup() { lookup.removeLookupListener(this) - CodeWhispererPopupManager.getInstance().shouldListenerCancelPopup = true + if (CodeWhispererPopupManager.getInstance().shouldEditorChangeCancelPopup <= 0) { + LOG.error("shouldListenerCancelPopup semaphore is not updated correctly") + } else { + CodeWhispererPopupManager.getInstance().shouldEditorChangeCancelPopup-- + LOG.debug { "Decrementing shouldListenerCancelPopup semaphore value" } + } } }) } From 5bd12605e2e6fcf369787c5954df971f8124515a Mon Sep 17 00:00:00 2001 From: Andrew Yu Date: Mon, 3 Feb 2025 13:05:02 -0800 Subject: [PATCH 2/4] detekt --- .../services/codewhisperer/popup/CodeWhispererPopupManager.kt | 3 ++- .../listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt index cfea9f331e..b79c4e24a2 100644 --- a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt +++ b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt @@ -44,6 +44,7 @@ import com.intellij.util.ui.UIUtil import software.amazon.awssdk.services.codewhispererruntime.model.Import import software.amazon.awssdk.services.codewhispererruntime.model.Reference import software.aws.toolkits.core.utils.debug +import software.aws.toolkits.core.utils.error import software.aws.toolkits.core.utils.getLogger import software.aws.toolkits.jetbrains.services.codewhisperer.editor.CodeWhispererEditorManager import software.aws.toolkits.jetbrains.services.codewhisperer.layout.CodeWhispererLayoutConfig.addHorizontalGlue @@ -253,7 +254,7 @@ class CodeWhispererPopupManager { runnable() } finally { if (shouldEditorChangeCancelPopup <= 0) { - LOG.error("shouldListenerCancelPopup semaphore is not updated correctly") + LOG.error { "shouldListenerCancelPopup semaphore is not updated correctly" } } else { shouldEditorChangeCancelPopup-- LOG.debug { "Decrementing shouldListenerCancelPopup semaphore value" } diff --git a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt index 32e3e11dcc..5ec56ac3e8 100644 --- a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt +++ b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt @@ -9,6 +9,7 @@ import com.intellij.codeInsight.lookup.LookupListener import com.intellij.codeInsight.lookup.LookupManagerListener import com.intellij.codeInsight.lookup.impl.LookupImpl import software.aws.toolkits.core.utils.debug +import software.aws.toolkits.core.utils.error import software.aws.toolkits.core.utils.getLogger import software.aws.toolkits.jetbrains.services.codewhisperer.model.InvocationContext import software.aws.toolkits.jetbrains.services.codewhisperer.popup.CodeWhispererPopupManager @@ -56,7 +57,7 @@ fun addIntelliSenseAcceptListener(lookup: Lookup, states: InvocationContext) { private fun cleanup() { lookup.removeLookupListener(this) if (CodeWhispererPopupManager.getInstance().shouldEditorChangeCancelPopup <= 0) { - LOG.error("shouldListenerCancelPopup semaphore is not updated correctly") + LOG.error { "shouldListenerCancelPopup semaphore is not updated correctly" } } else { CodeWhispererPopupManager.getInstance().shouldEditorChangeCancelPopup-- LOG.debug { "Decrementing shouldListenerCancelPopup semaphore value" } From 3b3c081d2ac48db51579f95a7eb446e2355a97d6 Mon Sep 17 00:00:00 2001 From: Andrew Yu Date: Mon, 3 Feb 2025 15:33:31 -0800 Subject: [PATCH 3/4] fix increment logic when IntelliSense is showing up --- .../CodeWhispererPopupIntelliSenseAcceptListener.kt | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt index 5ec56ac3e8..936d371fab 100644 --- a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt +++ b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt @@ -29,13 +29,9 @@ class CodeWhispererPopupIntelliSenseAcceptListener(private val states: Invocatio } fun addIntelliSenseAcceptListener(lookup: Lookup, states: InvocationContext) { + CodeWhispererPopupManager.getInstance().shouldEditorChangeCancelPopup++ + LOG.debug { "Incrementing shouldListenerCancelPopup semaphore value" } lookup.addLookupListener(object : LookupListener { - override fun lookupShown(event: LookupEvent) { - CodeWhispererPopupManager.getInstance().shouldEditorChangeCancelPopup++ - LOG.debug { "Incrementing shouldListenerCancelPopup semaphore value" } - super.lookupShown(event) - } - override fun itemSelected(event: LookupEvent) { if (!CodeWhispererInvocationStatus.getInstance().isDisplaySessionActive() || !(event.lookup as LookupImpl).isShown From a1ab2418b632cf9431cf296de6fb0915aef5206e Mon Sep 17 00:00:00 2001 From: Andrew Yu Date: Tue, 4 Feb 2025 04:46:58 -0800 Subject: [PATCH 4/4] Change to use a semaphore --- .../popup/CodeWhispererPopupManager.kt | 32 +++++++++++-------- ...hispererPopupIntelliSenseAcceptListener.kt | 15 ++++----- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt index b79c4e24a2..152498d4ed 100644 --- a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt +++ b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupManager.kt @@ -41,6 +41,7 @@ import com.intellij.ui.popup.AbstractPopup import com.intellij.ui.popup.PopupFactoryImpl import com.intellij.util.messages.Topic import com.intellij.util.ui.UIUtil +import kotlinx.coroutines.sync.Semaphore import software.amazon.awssdk.services.codewhispererruntime.model.Import import software.amazon.awssdk.services.codewhispererruntime.model.Reference import software.aws.toolkits.core.utils.debug @@ -87,7 +88,7 @@ class CodeWhispererPopupManager { val popupComponents = CodeWhispererPopupComponents() // Act like a semaphore: one increment only corresponds to one decrement - var shouldEditorChangeCancelPopup: Int = 0 + var allowEditsDuringSuggestionPreview = Semaphore(MAX_EDIT_SOURCE_DURING_SUGGESTION_PREVIEW) var sessionContext = SessionContext() private set @@ -247,18 +248,20 @@ class CodeWhispererPopupManager { } } + // Don't want to block or throw any kinds of exceptions here if it can continue to provide suggestions fun dontClosePopupAndRun(runnable: () -> Unit) { - try { - shouldEditorChangeCancelPopup++ - LOG.debug { "Incrementing shouldListenerCancelPopup semaphore value" } - runnable() - } finally { - if (shouldEditorChangeCancelPopup <= 0) { - LOG.error { "shouldListenerCancelPopup semaphore is not updated correctly" } - } else { - shouldEditorChangeCancelPopup-- - LOG.debug { "Decrementing shouldListenerCancelPopup semaphore value" } + if (allowEditsDuringSuggestionPreview.tryAcquire()) { + try { + runnable() + } finally { + try { + allowEditsDuringSuggestionPreview.release() + } catch (e: Exception) { + LOG.error(e) { "Failed to release allowEditsDuringSuggestionPreview semaphore" } + } } + } else { + LOG.error { "Failed to acquire allowEditsDuringSuggestionPreview semaphore" } } } @@ -504,7 +507,7 @@ class CodeWhispererPopupManager { val editor = states.requestContext.editor val codewhispererSelectionListener: SelectionListener = object : SelectionListener { override fun selectionChanged(event: SelectionEvent) { - if (shouldEditorChangeCancelPopup == 0) { + if (allowEditsDuringSuggestionPreview.availablePermits == MAX_EDIT_SOURCE_DURING_SUGGESTION_PREVIEW) { cancelPopup(states.popup) } super.selectionChanged(event) @@ -520,7 +523,7 @@ class CodeWhispererPopupManager { if (!delete) return if (editor.caretModel.offset == event.offset) { changeStates(states, 0) - } else if (shouldEditorChangeCancelPopup == 0) { + } else if (allowEditsDuringSuggestionPreview.availablePermits == MAX_EDIT_SOURCE_DURING_SUGGESTION_PREVIEW) { cancelPopup(states.popup) } } @@ -529,7 +532,7 @@ class CodeWhispererPopupManager { val codewhispererCaretListener: CaretListener = object : CaretListener { override fun caretPositionChanged(event: CaretEvent) { - if (shouldEditorChangeCancelPopup == 0) { + if (allowEditsDuringSuggestionPreview.availablePermits == MAX_EDIT_SOURCE_DURING_SUGGESTION_PREVIEW) { cancelPopup(states.popup) } super.caretPositionChanged(event) @@ -710,6 +713,7 @@ class CodeWhispererPopupManager { "CodeWhisperer user action performed", CodeWhispererUserActionListener::class.java ) + const val MAX_EDIT_SOURCE_DURING_SUGGESTION_PREVIEW = 2 } } diff --git a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt index 936d371fab..1d401f4fcd 100644 --- a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt +++ b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/popup/listeners/CodeWhispererPopupIntelliSenseAcceptListener.kt @@ -8,7 +8,6 @@ import com.intellij.codeInsight.lookup.LookupEvent import com.intellij.codeInsight.lookup.LookupListener import com.intellij.codeInsight.lookup.LookupManagerListener import com.intellij.codeInsight.lookup.impl.LookupImpl -import software.aws.toolkits.core.utils.debug import software.aws.toolkits.core.utils.error import software.aws.toolkits.core.utils.getLogger import software.aws.toolkits.jetbrains.services.codewhisperer.model.InvocationContext @@ -29,8 +28,9 @@ class CodeWhispererPopupIntelliSenseAcceptListener(private val states: Invocatio } fun addIntelliSenseAcceptListener(lookup: Lookup, states: InvocationContext) { - CodeWhispererPopupManager.getInstance().shouldEditorChangeCancelPopup++ - LOG.debug { "Incrementing shouldListenerCancelPopup semaphore value" } + if (!CodeWhispererPopupManager.getInstance().allowEditsDuringSuggestionPreview.tryAcquire()) { + LOG.error { "Failed to acquire allowEditsDuringSuggestionPreview semaphore" } + } lookup.addLookupListener(object : LookupListener { override fun itemSelected(event: LookupEvent) { if (!CodeWhispererInvocationStatus.getInstance().isDisplaySessionActive() || @@ -52,11 +52,10 @@ fun addIntelliSenseAcceptListener(lookup: Lookup, states: InvocationContext) { private fun cleanup() { lookup.removeLookupListener(this) - if (CodeWhispererPopupManager.getInstance().shouldEditorChangeCancelPopup <= 0) { - LOG.error { "shouldListenerCancelPopup semaphore is not updated correctly" } - } else { - CodeWhispererPopupManager.getInstance().shouldEditorChangeCancelPopup-- - LOG.debug { "Decrementing shouldListenerCancelPopup semaphore value" } + try { + CodeWhispererPopupManager.getInstance().allowEditsDuringSuggestionPreview.release() + } catch (e: Exception) { + LOG.error(e) { "Failed to release allowEditsDuringSuggestionPreview semaphore" } } } })