Skip to content

perf: cleanup/reduce de/ser cycles introduced in #5970 #5992

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 27, 2025
Merged

perf: cleanup/reduce de/ser cycles introduced in #5970 #5992

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cdc4564
chore: cleanup/reduce de/ser cycles introduced in #5970
rli Aug 26, 2025
c3b2b47
tst
rli Aug 26, 2025
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
68 changes: 40 additions & 28 deletions ...ommunity/src/software/aws/toolkits/jetbrains/services/amazonq/webview/BrowserConnector.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package software.aws.toolkits.jetbrains.services.amazonq.webview

import com.fasterxml.jackson.databind.JsonNode
import com.fasterxml.jackson.databind.node.ObjectNode
import com.fasterxml.jackson.module.kotlin.readValue
import com.fasterxml.jackson.module.kotlin.treeToValue
import com.google.gson.Gson
import com.intellij.ide.BrowserUtil
Expand Down Expand Up @@ -34,6 +35,7 @@ import org.cef.browser.CefBrowser
import org.eclipse.lsp4j.TextDocumentIdentifier
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException
import org.eclipse.lsp4j.jsonrpc.messages.ResponseErrorCode
import org.intellij.lang.annotations.Language
import software.aws.toolkits.core.utils.error
import software.aws.toolkits.core.utils.getLogger
import software.aws.toolkits.core.utils.info
Expand Down Expand Up @@ -581,6 +583,21 @@ class BrowserConnector(
}
}

/**
* Solely used to extract the aggregated findings from the response
*/
// TODO: move to software.aws.toolkits.jetbrains.services.amazonq.lsp.model
data class FlareAdditionalMessages(
val additionalMessages: List<FlareAggregatedFindings>?,
)

// TODO: move to software.aws.toolkits.jetbrains.services.amazonq.lsp.model
data class FlareAggregatedFindings(
val messageId: String,
val body: List<AggregatedCodeScanIssue>,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Only findings messages have this format, others do not. I am worried if we force this typing, it will cause errors

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 is handled


// TODO: move to software.aws.toolkits.jetbrains.services.amazonq.lsp.model
data class FlareCodeScanIssue(
val startLine: Int,
val endLine: Int,
Expand All @@ -602,6 +619,7 @@ class BrowserConnector(
val findingContext: String,
)

// TODO: move to software.aws.toolkits.jetbrains.services.amazonq.lsp.model
data class AggregatedCodeScanIssue(
val filePath: String,
val issues: List<FlareCodeScanIssue>,
Expand All @@ -620,14 +638,13 @@ class BrowserConnector(
throw error
}
chatCommunicationManager.removePartialChatMessage(partialResultToken)
val decryptedMessage = Gson().fromJson(value?.let { encryptionManager?.decrypt(it) }.orEmpty(), Map::class.java)
as Map<String, *>
val decryptedMessage = value?.let { encryptionManager?.decrypt(it) }.orEmpty()
parseFindingsMessages(decryptedMessage)

val messageToChat = ChatCommunicationManager.convertToJsonToSendToChat(
SEND_CHAT_COMMAND_PROMPT,
tabId,
Gson().toJson(decryptedMessage),
decryptedMessage,
isPartialResult = false
)
browser.postChat(messageToChat)
Expand All @@ -641,26 +658,18 @@ class BrowserConnector(
}
}

fun parseFindingsMessages(messagesMap: Map<String, *>) {
fun parseFindingsMessages(@Language("JSON") responsePayload: String) {
try {
val additionalMessages = messagesMap["additionalMessages"] as? MutableList<Map<String, Any>>
val additionalMessages = serializer.objectMapper.readValue<FlareAdditionalMessages>(responsePayload).additionalMessages
val findingsMessages = additionalMessages?.filter { message ->
if (message.contains("messageId")) {
(message["messageId"] as String).endsWith(CODE_REVIEW_FINDINGS_SUFFIX) ||
(message["messageId"] as String).endsWith(DISPLAY_FINDINGS_SUFFIX)
} else {
false
}
}
message.messageId.endsWith(CODE_REVIEW_FINDINGS_SUFFIX) ||
message.messageId.endsWith(DISPLAY_FINDINGS_SUFFIX)
} ?: return

val scannedFiles = mutableListOf<VirtualFile>()
if (findingsMessages != null) {
val mappedFindings = buildList {
for (findingsMessage in findingsMessages) {
additionalMessages.remove(findingsMessage)
val gson = Gson()
val jsonFindings = gson.fromJson(findingsMessage["body"] as String, List::class.java)
val mappedFindings = mutableListOf<CodeWhispererCodeScanIssue>()
for (aggregatedIssueUnformatted in jsonFindings) {
val aggregatedIssue = gson.fromJson(gson.toJson(aggregatedIssueUnformatted), AggregatedCodeScanIssue::class.java)
for (aggregatedIssue in findingsMessage.body) {
val file = LocalFileSystem.getInstance().findFileByIoFile(
Path.of(aggregatedIssue.filePath).toFile()
)
Expand All @@ -677,7 +686,8 @@ class BrowserConnector(
if (isIssueIgnored) {
continue
}
mappedFindings.add(

add(
CodeWhispererCodeScanIssue(
startLine = issue.startLine,
startCol = 1,
Expand Down Expand Up @@ -705,16 +715,18 @@ class BrowserConnector(
}
}
}

CodeWhispererCodeScanManager.getInstance(project)
.addOnDemandIssues(
mappedFindings,
scannedFiles,
CodeWhispererConstants.CodeAnalysisScope.AGENTIC
)
CodeWhispererCodeScanManager.getInstance(project).showCodeScanUI()
}
}

if (mappedFindings.isNotEmpty()) {
CodeWhispererCodeScanManager.getInstance(project)
.addOnDemandIssues(
mappedFindings,
scannedFiles,
CodeWhispererConstants.CodeAnalysisScope.AGENTIC
)
CodeWhispererCodeScanManager.getInstance(project).showCodeScanUI()
}
} catch (e: Exception) {
LOG.error { "Failed to parse findings message $e" }
}
Expand Down
147 changes: 88 additions & 59 deletions ...nity/tst/software/aws/toolkits/jetbrains/services/amazonq/webview/BrowserConnectorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,54 +53,76 @@ class BrowserConnectorTest : AmazonQTestBase() {
}

@Test
fun `parseFindingsMessages should handle empty additionalMessages`() {
val messagesMap = mapOf<String, Any>()

browserConnector.parseFindingsMessages(messagesMap)
fun `parseFindingsMessages should handle no additionalMessages`() {
browserConnector.parseFindingsMessages("""""")

verify(mockCodeScanManager, never()).addOnDemandIssues(any(), any(), any())
}

@Test
fun `parseFindingsMessages should handle null additionalMessages`() {
val messagesMap = mapOf("additionalMessages" to null)

browserConnector.parseFindingsMessages(messagesMap)
browserConnector.parseFindingsMessages(
"""
{
"additionalMessages": null
}
""".trimIndent()
)

verify(mockCodeScanManager, never()).addOnDemandIssues(any(), any(), any())
}

@Test
fun `parseFindingsMessages should filter messages with CODE_REVIEW_FINDINGS_SUFFIX`() {
val findingsMessage = mapOf(
"messageId" to "test_codeReviewFindings",
"body" to """[{"filePath": "/test/file.kt", "issues": []}]"""
)
val otherMessage = mapOf(
"messageId" to "other_message",
"body" to "other content"
fun `parseFindingsMessages should handle empty additionalMessages`() {
browserConnector.parseFindingsMessages(
"""
{
"additionalMessages": []
}
""".trimIndent()
)
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage, otherMessage)
val messagesMap = mapOf("additionalMessages" to additionalMessages)

browserConnector.parseFindingsMessages(messagesMap)
verify(mockCodeScanManager, never()).addOnDemandIssues(any(), any(), any())
}

assertThat(additionalMessages.size == 1)
assertThat(additionalMessages[0] == otherMessage)
@Test
fun `parseFindingsMessages should filter messages with CODE_REVIEW_FINDINGS_SUFFIX`() {
val findingsMessage = """
{
"additionalMessages": [
{
"messageId": "test_codeReviewFindings",
"body": [{"filePath": "/test/file.kt", "issues": []}]
},
{
"messageId": "other_message",
"body": "other content"
}
]
}
""".trimIndent()

browserConnector.parseFindingsMessages(findingsMessage)
}

@Test
fun `parseFindingsMessages should filter messages with DISPLAY_FINDINGS_SUFFIX`() {
val findingsMessage = mapOf(
"messageId" to "test_displayFindings",
"body" to """[{"filePath": "/test/file.kt", "issues": []}]"""
)
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
val messagesMap = mapOf("additionalMessages" to additionalMessages)

browserConnector.parseFindingsMessages(messagesMap)
val findingsMessage = """
{
"additionalMessages": [
{
"messageId": "test_displayFindings",
"body": [{"filePath": "/test/file.kt", "issues": []}]
},
{
"messageId": "other_message",
"body": "other content"
}
]
}
""".trimIndent()

assertThat(additionalMessages).isEmpty()
browserConnector.parseFindingsMessages(findingsMessage)
}

@Test
Expand Down Expand Up @@ -135,14 +157,18 @@ class BrowserConnectorTest : AmazonQTestBase() {
)

val aggregatedIssue = BrowserConnector.AggregatedCodeScanIssue("/test/file.kt", listOf(issue))
val findingsMessage = mapOf(
"messageId" to "test_codeReviewFindings",
"body" to jacksonObjectMapper().writeValueAsString(listOf(aggregatedIssue))
)
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
val messagesMap = mapOf("additionalMessages" to additionalMessages)

browserConnector.parseFindingsMessages(messagesMap)
val findingsMessage = """
{
"additionalMessages": [
{
"messageId": "test_codeReviewFindings",
"body": ${jacksonObjectMapper().writeValueAsString(listOf(aggregatedIssue))}
}
]
}
""".trimIndent()

browserConnector.parseFindingsMessages(findingsMessage)

val issuesCaptor = argumentCaptor<List<CodeWhispererCodeScanIssue>>()
verify(mockCodeScanManager).addOnDemandIssues(
Expand All @@ -151,7 +177,6 @@ class BrowserConnectorTest : AmazonQTestBase() {
eq(CodeWhispererConstants.CodeAnalysisScope.AGENTIC)
)

assertThat(additionalMessages).isEmpty()
assertThat(issuesCaptor.firstValue.isNotEmpty())
assertThat(issuesCaptor.firstValue[0].title == "Test Issue")
}
Expand All @@ -177,38 +202,42 @@ class BrowserConnectorTest : AmazonQTestBase() {
)

val aggregatedIssue = BrowserConnector.AggregatedCodeScanIssue("/test/directory", listOf(issue))
val findingsMessage = mapOf(
"messageId" to "test_displayFindings",
"body" to jacksonObjectMapper().writeValueAsString(listOf(aggregatedIssue))
)
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
val messagesMap = mapOf("additionalMessages" to additionalMessages)
val findingsMessage = """
{
"additionalMessages": [
{
"messageId": "test_displayFindings",
"body": ${jacksonObjectMapper().writeValueAsString(listOf(aggregatedIssue))}
}
]
}
""".trimIndent()

browserConnector.parseFindingsMessages(messagesMap)
browserConnector.parseFindingsMessages(findingsMessage)

val issuesCaptor = argumentCaptor<List<CodeWhispererCodeScanIssue>>()
verify(mockCodeScanManager).addOnDemandIssues(
issuesCaptor.capture(),
verify(mockCodeScanManager, never()).addOnDemandIssues(
any(),
any(),
eq(CodeWhispererConstants.CodeAnalysisScope.AGENTIC)
)

assertThat(issuesCaptor.firstValue.isEmpty())
assertThat(additionalMessages).isEmpty()
}
}

@Test
fun `parseFindingsMessages should handle invalid JSON gracefully`() {
val findingsMessage = mapOf(
"messageId" to "test_codeReviewFindings",
"body" to "invalid json"
)
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
val messagesMap = mapOf("additionalMessages" to additionalMessages)
val findingsMessage = """
{
"additionalMessages": [
{
"messageId": "test_codeReviewFindings",
"body": "invalid json"
}
]
}
""".trimIndent()

browserConnector.parseFindingsMessages(messagesMap)
browserConnector.parseFindingsMessages(findingsMessage)

assertThat(additionalMessages).isEmpty()
verify(mockCodeScanManager, never()).addOnDemandIssues(any(), any(), any())
}
}
Loading