Skip to content

Commit 961e6ca

Browse files
fix(amazonq): send full finding details to plugin, partial to agent (aws#2356)
* fix(amazonq): send full finding details to plugin, only send key fields to agent * fix(amazonq): add unit test --------- Co-authored-by: Blake Lazarine <[email protected]>
1 parent e7aa2a6 commit 961e6ca

File tree

5 files changed

+137
-19
lines changed

5 files changed

+137
-19
lines changed

server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2097,6 +2097,28 @@ export class AgenticChatController implements ChatHandlers {
20972097

20982098
let toolResultContent: ToolResultContentBlock
20992099

2100+
if (toolUse.name === CodeReview.toolName) {
2101+
// no need to write tool result for code review, this is handled by model via chat
2102+
// Push result in message so that it is picked by IDE plugin to show in issues panel
2103+
const codeReviewResult = result as InvokeOutput
2104+
if (
2105+
codeReviewResult?.output?.kind === 'json' &&
2106+
codeReviewResult.output.success &&
2107+
(codeReviewResult.output.content as any)?.findingsByFile
2108+
) {
2109+
await chatResultStream.writeResultBlock({
2110+
type: 'tool',
2111+
messageId: toolUse.toolUseId + CODE_REVIEW_FINDINGS_MESSAGE_SUFFIX,
2112+
body: (codeReviewResult.output.content as any).findingsByFile,
2113+
})
2114+
codeReviewResult.output.content = {
2115+
codeReviewId: (codeReviewResult.output.content as any).codeReviewId,
2116+
message: (codeReviewResult.output.content as any).message,
2117+
findingsByFileSimplified: (codeReviewResult.output.content as any).findingsByFileSimplified,
2118+
}
2119+
}
2120+
}
2121+
21002122
if (typeof result === 'string') {
21012123
toolResultContent = { text: result }
21022124
} else if (Array.isArray(result)) {
@@ -2182,20 +2204,6 @@ export class AgenticChatController implements ChatHandlers {
21822204
await chatResultStream.writeResultBlock(chatResult)
21832205
break
21842206
case CodeReview.toolName:
2185-
// no need to write tool result for code review, this is handled by model via chat
2186-
// Push result in message so that it is picked by IDE plugin to show in issues panel
2187-
const codeReviewResult = result as InvokeOutput
2188-
if (
2189-
codeReviewResult?.output?.kind === 'json' &&
2190-
codeReviewResult.output.success &&
2191-
(codeReviewResult.output.content as any)?.findingsByFile
2192-
) {
2193-
await chatResultStream.writeResultBlock({
2194-
type: 'tool',
2195-
messageId: toolUse.toolUseId + CODE_REVIEW_FINDINGS_MESSAGE_SUFFIX,
2196-
body: (codeReviewResult.output.content as any).findingsByFile,
2197-
})
2198-
}
21992207
break
22002208
case DisplayFindings.toolName:
22012209
// no need to write tool result for code review, this is handled by model via chat

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReview.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,92 @@ describe('CodeReview', () => {
146146
expect(result.output.kind).to.equal('json')
147147
})
148148

149+
it('should return both full and simplified findings', async () => {
150+
const mockFindings = [
151+
{
152+
findingId: '1',
153+
title: 'Test Issue',
154+
description: { text: 'Test description', markdown: 'Test **description**' },
155+
startLine: 10,
156+
endLine: 15,
157+
severity: 'HIGH',
158+
filePath: '/test/file.js',
159+
detectorId: 'detector1',
160+
detectorName: 'Test Detector',
161+
ruleId: 'rule1',
162+
relatedVulnerabilities: [],
163+
recommendation: { text: 'Fix this', url: null },
164+
suggestedFixes: [],
165+
comment: 'Test Issue: Test description',
166+
scanJobId: 'job-123',
167+
language: 'javascript',
168+
autoDetected: false,
169+
findingContext: 'Full',
170+
},
171+
]
172+
173+
mockCodeWhispererClient.createUploadUrl.resolves({
174+
uploadUrl: 'https://upload.com',
175+
uploadId: 'upload-123',
176+
requestHeaders: {},
177+
})
178+
179+
mockCodeWhispererClient.startCodeAnalysis.resolves({
180+
jobId: 'job-123',
181+
status: 'Pending',
182+
})
183+
184+
mockCodeWhispererClient.getCodeAnalysis.resolves({
185+
status: 'Completed',
186+
})
187+
188+
mockCodeWhispererClient.listCodeAnalysisFindings.resolves({
189+
codeAnalysisFindings: JSON.stringify(mockFindings),
190+
nextToken: undefined,
191+
})
192+
193+
sandbox.stub(CodeReviewUtils, 'uploadFileToPresignedUrl').resolves()
194+
sandbox.stub(codeReview as any, 'prepareFilesAndFoldersForUpload').resolves({
195+
zipBuffer: Buffer.from('test'),
196+
md5Hash: 'hash123',
197+
isCodeDiffPresent: false,
198+
programmingLanguages: new Set(['javascript']),
199+
numberOfFilesInCustomerCodeZip: 1,
200+
codeDiffFiles: new Set(),
201+
filePathsInZip: new Set(['/test/file.js']),
202+
})
203+
sandbox.stub(codeReview as any, 'parseFindings').returns(mockFindings)
204+
sandbox.stub(codeReview as any, 'resolveFilePath').returns('/test/file.js')
205+
206+
const result = await codeReview.execute(validInput, context)
207+
208+
expect(result.output.success).to.be.true
209+
expect(result.output.content).to.have.property('findingsByFile')
210+
expect(result.output.content).to.have.property('findingsByFileSimplified')
211+
212+
const fullFindings = JSON.parse((result.output.content as any).findingsByFile)
213+
const simplifiedFindings = JSON.parse((result.output.content as any).findingsByFileSimplified)
214+
215+
expect(fullFindings).to.have.length(1)
216+
expect(simplifiedFindings).to.have.length(1)
217+
218+
// Verify full findings structure
219+
expect(fullFindings[0].issues[0]).to.have.property('findingId')
220+
expect(fullFindings[0].issues[0]).to.have.property('description')
221+
expect(fullFindings[0].issues[0]).to.have.property('detectorId')
222+
223+
// Verify simplified findings structure (only 5 fields)
224+
const simplifiedIssue = simplifiedFindings[0].issues[0]
225+
expect(Object.keys(simplifiedIssue)).to.have.length(5)
226+
expect(simplifiedIssue).to.have.property('filePath', '/test/file.js')
227+
expect(simplifiedIssue).to.have.property('startLine', 10)
228+
expect(simplifiedIssue).to.have.property('endLine', 15)
229+
expect(simplifiedIssue).to.have.property('title', 'Test Issue')
230+
expect(simplifiedIssue).to.have.property('severity', 'HIGH')
231+
expect(simplifiedIssue).to.not.have.property('findingId')
232+
expect(simplifiedIssue).to.not.have.property('description')
233+
})
234+
149235
it('should execute successfully and pass languageModelId and clientType to startCodeAnalysis', async () => {
150236
const inputWithModelId = {
151237
...validInput,

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReview.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
CodeReviewFinding,
3030
FailedMetricName,
3131
SuccessMetricName,
32+
CodeReviewFindingSimplified,
3233
} from './codeReviewTypes'
3334
import { CancellationError } from '@aws/lsp-core'
3435
import { Origin } from '@amzn/codewhisperer-streaming'
@@ -44,7 +45,7 @@ export class CodeReview {
4445
private static readonly POLLING_INTERVAL_MS = 10000 // 10 seconds
4546
private static readonly UPLOAD_INTENT = 'AGENTIC_CODE_REVIEW'
4647
private static readonly SCAN_SCOPE = 'AGENTIC'
47-
private static readonly MAX_FINDINGS_COUNT = 30
48+
private static readonly MAX_FINDINGS_COUNT = 300
4849

4950
private static readonly ERROR_MESSAGES = {
5051
MISSING_CLIENT: 'CodeWhisperer client not available',
@@ -477,9 +478,18 @@ export class CodeReview {
477478
)
478479

479480
this.logging.info('Findings count grouped by file')
481+
let aggregatedCodeScanIssueListSimplified: { filePath: string; issues: CodeReviewFindingSimplified[] }[] = []
480482
aggregatedCodeScanIssueList.forEach(item => {
481483
this.logging.info(`File path - ${item.filePath} Findings count - ${item.issues.length}`)
482-
item.issues.forEach(issue =>
484+
let simplifiedIssues: CodeReviewFindingSimplified[] = []
485+
item.issues.forEach(issue => {
486+
simplifiedIssues.push({
487+
filePath: issue.filePath,
488+
startLine: issue.startLine,
489+
endLine: issue.endLine,
490+
title: issue.title,
491+
severity: issue.severity,
492+
})
483493
CodeReviewUtils.emitMetric(
484494
{
485495
reason: SuccessMetricName.IssuesDetected,
@@ -496,7 +506,11 @@ export class CodeReview {
496506
this.logging,
497507
this.telemetry
498508
)
499-
)
509+
})
510+
aggregatedCodeScanIssueListSimplified.push({
511+
filePath: item.filePath,
512+
issues: simplifiedIssues,
513+
})
500514
})
501515

502516
let scopeMessage = this.overrideDiffScan
@@ -505,8 +519,9 @@ export class CodeReview {
505519

506520
return {
507521
codeReviewId: jobId,
508-
message: `${CODE_REVIEW_TOOL_NAME} tool completed successfully. ${scopeMessage} ${findingsExceededLimit ? ` Inform the user that we are limiting findings to top ${CodeReview.MAX_FINDINGS_COUNT} based on severity.` : ''}`,
522+
message: `${CODE_REVIEW_TOOL_NAME} tool completed successfully. Please inform the user to use the explain and fix buttons in the Code Issues Panel to get the best information about particular findings. ${scopeMessage} ${findingsExceededLimit ? ` Inform the user that we are limiting findings to top ${CodeReview.MAX_FINDINGS_COUNT} based on severity.` : ''}`,
509523
findingsByFile: JSON.stringify(aggregatedCodeScanIssueList),
524+
findingsByFileSimplified: JSON.stringify(aggregatedCodeScanIssueListSimplified),
510525
}
511526
}
512527

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewConstants.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ export const CODE_REVIEW_TOOL_DESCRIPTION = [
181181
'The tool will generate some findings grouped by file',
182182
'Use following format STRICTLY to display the result of this tool for different scenarios:',
183183
'- When findings are present, you must inform user that you have completed the review of {file name / folder name / workspace} and found several issues that need attention. To inspect the details, and get fixes for those issues use the Code Issues panel.',
184-
' - When tool output message tells that findings were limited due to high count, you must inform the user that since there were lots of findings, you have included the top 30 findings only.',
184+
' - When tool output message tells that findings were limited due to high count, you must inform the user that since there were lots of findings, you have included the top 300 findings only.',
185185
'- When no findings are generated by the tool, you must tell user that you have completed the review of {file name / folder name / workspace} and found no issues.',
186186
].join('\n')
187187

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewTypes.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export type CodeReviewResult = {
4343
codeReviewId: string
4444
message: string
4545
findingsByFile: string
46+
findingsByFileSimplified: string
4647
}
4748

4849
export type CodeReviewFinding = {
@@ -66,6 +67,14 @@ export type CodeReviewFinding = {
6667
findingContext: string | null | undefined
6768
}
6869

70+
export type CodeReviewFindingSimplified = {
71+
filePath: string
72+
startLine: number
73+
endLine: number
74+
title: string
75+
severity: string
76+
}
77+
6978
export type CodeReviewMetric =
7079
| {
7180
reason: SuccessMetricName

0 commit comments

Comments
 (0)