Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import com.intellij.openapi.util.Disposer
import com.intellij.openapi.util.UserDataHolderBase
import com.intellij.util.concurrency.annotations.RequiresEdt
import kotlinx.coroutines.channels.Channel
import software.amazon.awssdk.services.codewhispererruntime.model.IdeDiagnostic
import software.aws.toolkits.jetbrains.core.credentials.ToolkitConnection
import software.aws.toolkits.jetbrains.services.amazonq.lsp.model.aws.textDocument.InlineCompletionItem
import software.aws.toolkits.jetbrains.services.amazonq.lsp.model.aws.textDocument.InlineCompletionListWithReferences
Expand Down Expand Up @@ -244,6 +245,7 @@ data class InlineCompletionSessionContext(
var sessionId: String = "",
val triggerOffset: Int,
var counter: Int = 0,
val diagnostics: List<IdeDiagnostic>? = emptyList(),
)

data class InlineCompletionItemContext(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import software.aws.toolkits.jetbrains.services.codewhisperer.service.CodeWhispe
import software.aws.toolkits.jetbrains.services.codewhisperer.telemetry.CodeWhispererTelemetryService
import software.aws.toolkits.jetbrains.services.codewhisperer.toolwindow.CodeWhispererCodeReferenceManager
import software.aws.toolkits.jetbrains.services.codewhisperer.util.CodeWhispererConstants
import software.aws.toolkits.jetbrains.services.codewhisperer.util.getDocumentDiagnostics
import software.aws.toolkits.jetbrains.utils.isQConnected
import software.aws.toolkits.resources.message
import software.aws.toolkits.telemetry.CodewhispererTriggerType
Expand Down Expand Up @@ -332,6 +333,7 @@ class QInlineCompletionProvider(private val cs: CoroutineScope) : InlineCompleti
latencyContext,
sessionContext,
triggerSessionId,
editor.document
)
activeTriggerSessions.remove(triggerSessionId)
}
Expand Down Expand Up @@ -398,6 +400,7 @@ class QInlineCompletionProvider(private val cs: CoroutineScope) : InlineCompleti
val triggerSessionId = triggerSessionId++
val latencyContext = LatencyContext(codewhispererEndToEndStart = System.nanoTime())
val triggerTypeInfo = getTriggerTypeInfo(request)
val diagnostics = getDocumentDiagnostics(editor.document, project)

CodeWhispererInvocationStatus.getInstance().setIsInvokingQInline(session, true)
Disposer.register(session) {
Expand All @@ -412,7 +415,7 @@ class QInlineCompletionProvider(private val cs: CoroutineScope) : InlineCompleti
return InlineCompletionSuggestion.Empty
}

val sessionContext = InlineCompletionSessionContext(triggerOffset = request.endOffset)
val sessionContext = InlineCompletionSessionContext(triggerOffset = request.endOffset, diagnostics = diagnostics)

// Pagination workaround: Always return exactly 5 variants
// Create channel placeholder for upcoming pagination results
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@ package software.aws.toolkits.jetbrains.services.codewhisperer.telemetry

import com.intellij.openapi.components.Service
import com.intellij.openapi.components.service
import com.intellij.openapi.editor.Document
import com.intellij.openapi.project.Project
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import software.aws.toolkits.core.utils.debug
import software.aws.toolkits.core.utils.getLogger
import software.aws.toolkits.jetbrains.core.credentials.sono.isInternalUser
import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLspService
import software.aws.toolkits.jetbrains.services.amazonq.lsp.model.aws.InlineCompletionStates
import software.aws.toolkits.jetbrains.services.amazonq.lsp.model.aws.LogInlineCompletionSessionResultsParams
Expand All @@ -24,6 +27,10 @@ import software.aws.toolkits.jetbrains.services.codewhisperer.service.CodeWhispe
import software.aws.toolkits.jetbrains.services.codewhisperer.util.CodeWhispererConstants
import software.aws.toolkits.jetbrains.services.codewhisperer.util.CodeWhispererUtil.getCodeWhispererStartUrl
import software.aws.toolkits.jetbrains.services.codewhisperer.util.CodeWhispererUtil.getConnectionStartUrl
import software.aws.toolkits.jetbrains.services.codewhisperer.util.DiagnosticDifferences
import software.aws.toolkits.jetbrains.services.codewhisperer.util.getDiagnosticDifferences
import software.aws.toolkits.jetbrains.services.codewhisperer.util.getDocumentDiagnostics
import software.aws.toolkits.jetbrains.services.cwc.controller.chat.telemetry.getStartUrl
import software.aws.toolkits.jetbrains.settings.AwsSettings
import software.aws.toolkits.telemetry.CodeFixAction
import software.aws.toolkits.telemetry.CodewhispererCodeScanScope
Expand Down Expand Up @@ -75,6 +82,7 @@ class CodeWhispererTelemetryService(private val cs: CoroutineScope) {
latencyContext: LatencyContext,
sessionContext: InlineCompletionSessionContext,
triggerSessionId: Int,
document: Document,
) {
if (sessionContext.sessionId.isEmpty()) {
QInlineCompletionProvider.logInline(triggerSessionId) {
Expand All @@ -96,6 +104,18 @@ class CodeWhispererTelemetryService(private val cs: CoroutineScope) {
"total session display time: ${CodeWhispererInvocationStatus.getInstance().completionShownTime?.let { Duration.between(it, Instant.now()) }
?.toMillis()?.toDouble()}"
}
var diffDiagnostics = DiagnosticDifferences(
added = emptyList(),
removed = emptyList()
)

if (isInternalUser(getStartUrl(project))) {
val oldDiagnostics = sessionContext.diagnostics.orEmpty()
// wait for the IDE itself to update its diagnostics for current file
delay(500)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will reflect in metrics

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if diagnostics run takes longer than 500ms?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was based on @leigaol 's PR earlier 2720828 to let the LSP have time to have new diagnostics

This should only be reflected in this telemetry path right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no callback provided by JetBrains for "diagnostic updated complete" once the acceptance is done. Meanwhile, user can also do quick keyboard input immediately after acceptance.

We ack two issues with the implementation:

  1. If IDE itself does not provide diagnostics (like VSC), we will return [] in diagnostics.
  2. If IDE cannot finish within 500ms, we will return [] in diagnostics.

There is no workaround for #1 because we cannot force user to install the LSP for that language.
For #2, 500ms is a best middle ground between "too fast to see any diagnostic showing up" and "too slow to be interrupted by next user input".

val newDiagnostics = getDocumentDiagnostics(document, project)
diffDiagnostics = getDiagnosticDifferences(oldDiagnostics, newDiagnostics)
}
val params = LogInlineCompletionSessionResultsParams(
sessionId = sessionContext.sessionId,
completionSessionResult = sessionContext.itemContexts.filter { it.item != null }.associate {
Expand All @@ -110,7 +130,9 @@ class CodeWhispererTelemetryService(private val cs: CoroutineScope) {
?.toMillis()?.toDouble(),
// no userInput in JB inline completion API, every new char input will discard the previous trigger so
// user input is always 0
typeaheadLength = 0
typeaheadLength = 0,
addedDiagnostics = diffDiagnostics.added,
removedDiagnostics = diffDiagnostics.removed,
)
AmazonQLspService.executeAsyncIfRunning(project) { server ->
server.logInlineCompletionSessionResults(params)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@

package software.aws.toolkits.jetbrains.services.codewhisperer.util

import com.intellij.codeInsight.daemon.impl.HighlightInfo
import com.intellij.codeInsight.lookup.LookupManager
import com.intellij.ide.BrowserUtil
import com.intellij.lang.annotation.HighlightSeverity
import com.intellij.notification.NotificationAction
import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.application.runInEdt
import com.intellij.openapi.editor.Document
import com.intellij.openapi.editor.Editor
import com.intellij.openapi.editor.impl.DocumentMarkupModel
import com.intellij.openapi.editor.impl.EditorImpl
import com.intellij.openapi.project.Project
import com.intellij.openapi.vfs.VfsUtil
Expand All @@ -21,7 +25,11 @@ import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import kotlinx.coroutines.yield
import software.amazon.awssdk.services.codewhispererruntime.model.DiagnosticSeverity
import software.amazon.awssdk.services.codewhispererruntime.model.IdeDiagnostic
import software.amazon.awssdk.services.codewhispererruntime.model.OptOutPreference
import software.amazon.awssdk.services.codewhispererruntime.model.Position
import software.amazon.awssdk.services.codewhispererruntime.model.Range
import software.aws.toolkits.core.utils.getLogger
import software.aws.toolkits.core.utils.warn
import software.aws.toolkits.jetbrains.core.credentials.AwsBearerTokenConnection
Expand Down Expand Up @@ -347,3 +355,87 @@ object CodeWhispererUtil {
enum class CaretMovement {
NO_CHANGE, MOVE_FORWARD, MOVE_BACKWARD
}

val diagnosticPatterns = mapOf(
"TYPE_ERROR" to listOf("type", "cast"),
"SYNTAX_ERROR" to listOf("expected", "indent", "syntax"),
"REFERENCE_ERROR" to listOf("undefined", "not defined", "undeclared", "reference", "symbol"),
"BEST_PRACTICE" to listOf("deprecated", "unused", "uninitialized", "not initialized"),
"SECURITY" to listOf("security", "vulnerability")
)

fun getDiagnosticsType(message: String): String {
val lowercaseMessage = message.lowercase()
return diagnosticPatterns
.entries
.firstOrNull { (_, keywords) ->
keywords.any { lowercaseMessage.contains(it) }
}
?.key ?: "OTHER"
}

fun convertSeverity(severity: HighlightSeverity): DiagnosticSeverity = when {
severity == HighlightSeverity.ERROR -> DiagnosticSeverity.ERROR
severity == HighlightSeverity.WARNING ||
severity == HighlightSeverity.WEAK_WARNING -> DiagnosticSeverity.WARNING
severity == HighlightSeverity.INFORMATION -> DiagnosticSeverity.INFORMATION
severity == HighlightSeverity.TEXT_ATTRIBUTES -> DiagnosticSeverity.HINT
severity == HighlightSeverity.INFO -> DiagnosticSeverity.INFORMATION
// For severities that might indicate performance issues
severity.toString().contains("PERFORMANCE", ignoreCase = true) -> DiagnosticSeverity.WARNING
// For deprecation warnings
severity.toString().contains("DEPRECATED", ignoreCase = true) -> DiagnosticSeverity.WARNING
// Default case
else -> DiagnosticSeverity.INFORMATION
}

fun getDocumentDiagnostics(document: Document, project: Project): List<IdeDiagnostic> = runCatching {
DocumentMarkupModel.forDocument(document, project, true)
.allHighlighters
.mapNotNull { it.errorStripeTooltip as? HighlightInfo }
.filter { !it.description.isNullOrEmpty() }
.map { info ->
val startLine = document.getLineNumber(info.startOffset)
val endLine = document.getLineNumber(info.endOffset)

IdeDiagnostic.builder()
.ideDiagnosticType(getDiagnosticsType(info.description))
.severity(convertSeverity(info.severity))
.source(info.inspectionToolId)
.range(
Range.builder()
.start(
Position.builder()
.line(startLine)
.character(document.getLineStartOffset(startLine))
.build()
)
.end(
Position.builder()
.line(endLine)
.character(document.getLineStartOffset(endLine))
.build()
)
.build()
)
.build()
}
}.getOrElse { e ->
getLogger<CodeWhispererUtil>().warn { "Failed to get document diagnostics ${e.message}" }
emptyList()
}

data class DiagnosticDifferences(
val added: List<IdeDiagnostic>,
val removed: List<IdeDiagnostic>,
)

fun serializeDiagnostics(diagnostic: IdeDiagnostic): String = "${diagnostic.source()}-${diagnostic.severity()}-${diagnostic.ideDiagnosticType()}"

fun getDiagnosticDifferences(oldDiagnostic: List<IdeDiagnostic>, newDiagnostic: List<IdeDiagnostic>): DiagnosticDifferences {
val oldSet = oldDiagnostic.map { i -> serializeDiagnostics(i) }.toSet()
val newSet = newDiagnostic.map { i -> serializeDiagnostics(i) }.toSet()
val added = newDiagnostic.filter { i -> !oldSet.contains(serializeDiagnostics(i)) }.distinctBy { serializeDiagnostics(it) }
val removed = oldDiagnostic.filter { i -> !newSet.contains(serializeDiagnostics(i)) }.distinctBy { serializeDiagnostics(it) }
return DiagnosticDifferences(added, removed)
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package software.aws.toolkits.jetbrains.services.codewhisperer

import com.intellij.lang.annotation.HighlightSeverity
import com.intellij.openapi.util.SimpleModificationTracker
import com.intellij.testFramework.fixtures.CodeInsightTestFixture
import kotlinx.coroutines.runBlocking
Expand All @@ -12,7 +13,11 @@ import org.junit.Before
import org.junit.Rule
import org.junit.Test
import software.amazon.awssdk.regions.Region
import software.amazon.awssdk.services.codewhispererruntime.model.DiagnosticSeverity
import software.amazon.awssdk.services.codewhispererruntime.model.IdeDiagnostic
import software.amazon.awssdk.services.codewhispererruntime.model.OptOutPreference
import software.amazon.awssdk.services.codewhispererruntime.model.Position
import software.amazon.awssdk.services.codewhispererruntime.model.Range
import software.amazon.awssdk.services.ssooidc.SsoOidcClient
import software.aws.toolkits.core.utils.test.aStringWithLineCount
import software.aws.toolkits.jetbrains.core.MockClientManagerRule
Expand All @@ -23,6 +28,9 @@ import software.aws.toolkits.jetbrains.core.credentials.sono.SONO_URL
import software.aws.toolkits.jetbrains.core.region.MockRegionProviderRule
import software.aws.toolkits.jetbrains.services.codewhisperer.util.CodeWhispererUtil.getCompletionType
import software.aws.toolkits.jetbrains.services.codewhisperer.util.CodeWhispererUtil.getTelemetryOptOutPreference
import software.aws.toolkits.jetbrains.services.codewhisperer.util.convertSeverity
import software.aws.toolkits.jetbrains.services.codewhisperer.util.getDiagnosticDifferences
import software.aws.toolkits.jetbrains.services.codewhisperer.util.getDiagnosticsType
import software.aws.toolkits.jetbrains.services.codewhisperer.util.isWithin
import software.aws.toolkits.jetbrains.services.codewhisperer.util.runIfIdcConnectionOrTelemetryEnabled
import software.aws.toolkits.jetbrains.services.codewhisperer.util.toCodeChunk
Expand Down Expand Up @@ -263,4 +271,126 @@ class CodeWhispererUtilTest {
val file = fixture.addFileToProject("workspace/projectA1/src/Sample.java", "").virtualFile
assertThat(file.isWithin(projectRoot)).isFalse()
}

@Test
fun `getDiagnosticsType correctly identifies syntax errors`() {
val messages = listOf(
"Expected semicolon at end of line",
"Incorrect indent level",
"Syntax error in expression"
)

messages.forEach { message ->
assertThat(getDiagnosticsType(message)).isEqualTo("SYNTAX_ERROR")
}
}

@Test
fun `getDiagnosticsType correctly identifies type errors`() {
val messages = listOf(
"Cannot cast String to Int",
"Type mismatch: expected String but got Int"
)

messages.forEach { message ->
assertThat(getDiagnosticsType(message)).isEqualTo("TYPE_ERROR")
}
}

@Test
fun `getDiagnosticsType returns OTHER for unrecognized patterns`() {
val message = "Some random message"
assertThat(getDiagnosticsType(message)).isEqualTo("OTHER")
}

@Test
fun `convertSeverity correctly maps severity levels`() {
assertThat(convertSeverity(HighlightSeverity.ERROR)).isEqualTo(DiagnosticSeverity.ERROR)
assertThat(convertSeverity(HighlightSeverity.WARNING)).isEqualTo(DiagnosticSeverity.WARNING)
assertThat(convertSeverity(HighlightSeverity.TEXT_ATTRIBUTES)).isEqualTo(DiagnosticSeverity.HINT)
assertThat(convertSeverity(HighlightSeverity.INFORMATION)).isEqualTo(DiagnosticSeverity.INFORMATION)
assertThat(convertSeverity(HighlightSeverity.INFO)).isEqualTo(DiagnosticSeverity.INFORMATION)
}

@Test
fun `getDiagnosticDifferences correctly identifies added and removed diagnostics`() {
val diagnostic1 = IdeDiagnostic.builder()
.ideDiagnosticType("SYNTAX_ERROR")
.severity("ERROR")
.source("inspection1")
.range(
Range.builder()
.start(Position.builder().line(0).character(0).build())
.end(Position.builder().line(0).character(10).build())
.build()
)
.build()

val diagnostic2 = IdeDiagnostic.builder()
.ideDiagnosticType("TYPE_ERROR")
.severity("WARNING")
.source("inspection2")
.range(
Range.builder()
.start(Position.builder().line(1).character(0).build())
.end(Position.builder().line(1).character(10).build())
.build()
)
.build()

val oldList = listOf(diagnostic1)
val newList = listOf(diagnostic2)

val differences = getDiagnosticDifferences(oldList, newList)

assertThat(differences.added).containsExactly(diagnostic2)
assertThat(differences.removed).containsExactly(diagnostic1)
}

@Test
fun `getDiagnosticDifferences handles empty lists`() {
val diagnostic = IdeDiagnostic.builder()
.ideDiagnosticType("SYNTAX_ERROR")
.severity("ERROR")
.source("inspection1")
.range(
Range.builder()
.start(Position.builder().line(0).character(0).build())
.end(Position.builder().line(0).character(10).build())
.build()
)
.build()

val emptyList = emptyList<IdeDiagnostic>()
val nonEmptyList = listOf(diagnostic)

val differencesWithEmptyOld = getDiagnosticDifferences(emptyList, nonEmptyList)
assertThat(differencesWithEmptyOld.added).containsExactly(diagnostic)
assertThat(differencesWithEmptyOld.removed).isEmpty()

val differencesWithEmptyNew = getDiagnosticDifferences(nonEmptyList, emptyList)
assertThat(differencesWithEmptyNew.added).isEmpty()
assertThat(differencesWithEmptyNew.removed).containsExactly(diagnostic)
}

@Test
fun `getDiagnosticDifferences handles identical lists`() {
val diagnostic = IdeDiagnostic.builder()
.ideDiagnosticType("SYNTAX_ERROR")
.severity("ERROR")
.source("inspection1")
.range(
Range.builder()
.start(Position.builder().line(0).character(0).build())
.end(Position.builder().line(0).character(10).build())
.build()
)
.build()

val list = listOf(diagnostic)
val differences = getDiagnosticDifferences(list, list)

assertThat(differences.added).isEmpty()
assertThat(differences.removed).isEmpty()
}
}
Loading
Loading