Skip to content

Commit f99d878

Browse files
ctlai95kevluu-aws
authored andcommitted
feat(amazonq): grouping options for code issues aws#6330
## Problem Code issues are grouped by severity and while this is useful in some cases, it can be hard to navigate. ## Solution Let the user decide how to group the issues. Grouping strategies to start off with: - Severity (existing behavior) - File Location (issues in the same file are grouped together)
1 parent 0ed68be commit f99d878

File tree

15 files changed

+388
-15
lines changed

15 files changed

+388
-15
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Feature",
3+
"description": "/review: Code issues can be grouped by file location or severity"
4+
}

packages/amazonq/package.json

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,10 +365,15 @@
365365
"when": "view == aws.AmazonQChatView || view == aws.amazonq.AmazonCommonAuth",
366366
"group": "y_toolkitMeta@2"
367367
},
368+
{
369+
"command": "aws.amazonq.codescan.showGroupingStrategy",
370+
"when": "view == aws.amazonq.SecurityIssuesTree",
371+
"group": "navigation@1"
372+
},
368373
{
369374
"command": "aws.amazonq.security.showFilters",
370375
"when": "view == aws.amazonq.SecurityIssuesTree",
371-
"group": "navigation"
376+
"group": "navigation@2"
372377
}
373378
],
374379
"view/item/context": [
@@ -724,6 +729,12 @@
724729
{
725730
"command": "aws.amazonq.security.showFilters",
726731
"title": "%AWS.command.amazonq.filterIssues%",
732+
"icon": "$(filter)",
733+
"enablement": "view == aws.amazonq.SecurityIssuesTree"
734+
},
735+
{
736+
"command": "aws.amazonq.codescan.showGroupingStrategy",
737+
"title": "%AWS.command.amazonq.groupIssues%",
727738
"icon": "$(list-filter)",
728739
"enablement": "view == aws.amazonq.SecurityIssuesTree"
729740
},

packages/amazonq/test/unit/codewhisperer/models/model.test.ts

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@
44
*/
55
import assert from 'assert'
66
import sinon from 'sinon'
7-
import { SecurityIssueFilters, SecurityTreeViewFilterState } from 'aws-core-vscode/codewhisperer'
7+
import {
8+
CodeIssueGroupingStrategy,
9+
CodeIssueGroupingStrategyState,
10+
SecurityIssueFilters,
11+
SecurityTreeViewFilterState,
12+
} from 'aws-core-vscode/codewhisperer'
813
import { globals } from 'aws-core-vscode/shared'
914

1015
describe('model', function () {
@@ -70,4 +75,100 @@ describe('model', function () {
7075
assert.deepStrictEqual(hiddenSeverities, ['High', 'Low'])
7176
})
7277
})
78+
79+
describe('CodeIssueGroupingStrategyState', function () {
80+
let sandbox: sinon.SinonSandbox
81+
let state: CodeIssueGroupingStrategyState
82+
83+
beforeEach(function () {
84+
sandbox = sinon.createSandbox()
85+
state = CodeIssueGroupingStrategyState.instance
86+
})
87+
88+
afterEach(function () {
89+
sandbox.restore()
90+
})
91+
92+
describe('instance', function () {
93+
it('should return the same instance when called multiple times', function () {
94+
const instance1 = CodeIssueGroupingStrategyState.instance
95+
const instance2 = CodeIssueGroupingStrategyState.instance
96+
assert.strictEqual(instance1, instance2)
97+
})
98+
})
99+
100+
describe('getState', function () {
101+
it('should return fallback when no state is stored', function () {
102+
const result = state.getState()
103+
104+
assert.equal(result, CodeIssueGroupingStrategy.Severity)
105+
})
106+
107+
it('should return stored state when valid', async function () {
108+
const validStrategy = CodeIssueGroupingStrategy.FileLocation
109+
await state.setState(validStrategy)
110+
111+
const result = state.getState()
112+
113+
assert.equal(result, validStrategy)
114+
})
115+
116+
it('should return fallback when stored state is invalid', async function () {
117+
const invalidStrategy = 'invalid'
118+
await state.setState(invalidStrategy)
119+
120+
const result = state.getState()
121+
122+
assert.equal(result, CodeIssueGroupingStrategy.Severity)
123+
})
124+
})
125+
126+
describe('setState', function () {
127+
it('should update state and fire change event for valid strategy', async function () {
128+
const validStrategy = CodeIssueGroupingStrategy.FileLocation
129+
130+
// Create a spy to watch for event emissions
131+
const eventSpy = sandbox.spy()
132+
state.onDidChangeState(eventSpy)
133+
134+
await state.setState(validStrategy)
135+
136+
sinon.assert.calledWith(eventSpy, validStrategy)
137+
})
138+
139+
it('should use fallback and fire change event for invalid strategy', async function () {
140+
const invalidStrategy = 'invalid'
141+
142+
// Create a spy to watch for event emissions
143+
const eventSpy = sandbox.spy()
144+
state.onDidChangeState(eventSpy)
145+
146+
await state.setState(invalidStrategy)
147+
148+
sinon.assert.calledWith(eventSpy, CodeIssueGroupingStrategy.Severity)
149+
})
150+
})
151+
152+
describe('reset', function () {
153+
it('should set state to fallback value', async function () {
154+
const setStateStub = sandbox.stub(state, 'setState').resolves()
155+
156+
await state.reset()
157+
158+
sinon.assert.calledWith(setStateStub, CodeIssueGroupingStrategy.Severity)
159+
})
160+
})
161+
162+
describe('onDidChangeState', function () {
163+
it('should allow subscribing to state changes', async function () {
164+
const listener = sandbox.spy()
165+
const disposable = state.onDidChangeState(listener)
166+
167+
await state.setState(CodeIssueGroupingStrategy.Severity)
168+
169+
sinon.assert.calledWith(listener, CodeIssueGroupingStrategy.Severity)
170+
disposable.dispose()
171+
})
172+
})
173+
})
73174
})

packages/amazonq/test/unit/codewhisperer/service/securityIssueTreeViewProvider.test.ts

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,24 @@ import {
1010
SecurityTreeViewFilterState,
1111
SecurityIssueProvider,
1212
SeverityItem,
13+
CodeIssueGroupingStrategyState,
14+
CodeIssueGroupingStrategy,
1315
} from 'aws-core-vscode/codewhisperer'
1416
import { createCodeScanIssue } from 'aws-core-vscode/test'
1517
import assert from 'assert'
1618
import sinon from 'sinon'
19+
import path from 'path'
1720

1821
describe('SecurityIssueTreeViewProvider', function () {
19-
let securityIssueProvider: SecurityIssueProvider
2022
let securityIssueTreeViewProvider: SecurityIssueTreeViewProvider
2123

2224
beforeEach(function () {
23-
securityIssueProvider = SecurityIssueProvider.instance
25+
SecurityIssueProvider.instance.issues = [
26+
{ filePath: 'file/path/a', issues: [createCodeScanIssue(), createCodeScanIssue()] },
27+
{ filePath: 'file/path/b', issues: [createCodeScanIssue(), createCodeScanIssue()] },
28+
{ filePath: 'file/path/c', issues: [createCodeScanIssue(), createCodeScanIssue()] },
29+
{ filePath: 'file/path/d', issues: [createCodeScanIssue(), createCodeScanIssue()] },
30+
]
2431
securityIssueTreeViewProvider = new SecurityIssueTreeViewProvider()
2532
})
2633

@@ -44,13 +51,6 @@ describe('SecurityIssueTreeViewProvider', function () {
4451

4552
describe('getChildren', function () {
4653
it('should return sorted list of severities if element is undefined', function () {
47-
securityIssueProvider.issues = [
48-
{ filePath: 'file/path/c', issues: [createCodeScanIssue(), createCodeScanIssue()] },
49-
{ filePath: 'file/path/d', issues: [createCodeScanIssue(), createCodeScanIssue()] },
50-
{ filePath: 'file/path/a', issues: [createCodeScanIssue(), createCodeScanIssue()] },
51-
{ filePath: 'file/path/b', issues: [createCodeScanIssue(), createCodeScanIssue()] },
52-
]
53-
5454
const element = undefined
5555
const result = securityIssueTreeViewProvider.getChildren(element) as SeverityItem[]
5656
assert.strictEqual(result.length, 5)
@@ -102,5 +102,55 @@ describe('SecurityIssueTreeViewProvider', function () {
102102
const result = securityIssueTreeViewProvider.getChildren(element) as IssueItem[]
103103
assert.strictEqual(result.length, 0)
104104
})
105+
106+
it('should return severity-grouped items when grouping strategy is Severity', function () {
107+
sinon.stub(CodeIssueGroupingStrategyState.instance, 'getState').returns(CodeIssueGroupingStrategy.Severity)
108+
109+
const severityItems = securityIssueTreeViewProvider.getChildren() as SeverityItem[]
110+
for (const [index, [severity, expectedIssueCount]] of [
111+
['Critical', 0],
112+
['High', 8],
113+
['Medium', 0],
114+
['Low', 0],
115+
['Info', 0],
116+
].entries()) {
117+
const currentSeverityItem = severityItems[index]
118+
assert.strictEqual(currentSeverityItem.label, severity)
119+
assert.strictEqual(currentSeverityItem.issues.length, expectedIssueCount)
120+
121+
const issueItems = securityIssueTreeViewProvider.getChildren(currentSeverityItem) as IssueItem[]
122+
assert.ok(issueItems.every((item) => item.iconPath === undefined))
123+
assert.ok(
124+
issueItems.every((item) => item.description?.toString().startsWith(path.basename(item.filePath)))
125+
)
126+
}
127+
})
128+
129+
it('should return file-grouped items when grouping strategy is FileLocation', function () {
130+
sinon
131+
.stub(CodeIssueGroupingStrategyState.instance, 'getState')
132+
.returns(CodeIssueGroupingStrategy.FileLocation)
133+
134+
const result = securityIssueTreeViewProvider.getChildren() as FileItem[]
135+
for (const [index, [fileName, expectedIssueCount]] of [
136+
['a', 2],
137+
['b', 2],
138+
['c', 2],
139+
['d', 2],
140+
].entries()) {
141+
const currentFileItem = result[index]
142+
assert.strictEqual(currentFileItem.label, fileName)
143+
assert.strictEqual(currentFileItem.issues.length, expectedIssueCount)
144+
assert.strictEqual(currentFileItem.description, 'file/path')
145+
146+
const issueItems = securityIssueTreeViewProvider.getChildren(currentFileItem) as IssueItem[]
147+
assert.ok(
148+
issueItems.every((item) =>
149+
item.iconPath?.toString().includes(`${item.issue.severity.toLowerCase()}.svg`)
150+
)
151+
)
152+
assert.ok(issueItems.every((item) => item.description?.toString().startsWith('[Ln ')))
153+
}
154+
})
105155
})
106156
})
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
import { createQuickPickPrompterTester, QuickPickPrompterTester } from 'aws-core-vscode/test'
6+
import {
7+
CodeIssueGroupingStrategy,
8+
CodeIssueGroupingStrategyState,
9+
createCodeIssueGroupingStrategyPrompter,
10+
} from 'aws-core-vscode/codewhisperer'
11+
import sinon from 'sinon'
12+
import assert from 'assert'
13+
import vscode from 'vscode'
14+
15+
const severity = { data: CodeIssueGroupingStrategy.Severity, label: 'Severity' }
16+
const fileLocation = { data: CodeIssueGroupingStrategy.FileLocation, label: 'File Location' }
17+
18+
describe('createCodeIssueGroupingStrategyPrompter', function () {
19+
let tester: QuickPickPrompterTester<CodeIssueGroupingStrategy>
20+
21+
beforeEach(function () {
22+
tester = createQuickPickPrompterTester(createCodeIssueGroupingStrategyPrompter())
23+
})
24+
25+
afterEach(function () {
26+
sinon.restore()
27+
})
28+
29+
it('should list grouping strategies', async function () {
30+
tester.assertItems([severity, fileLocation])
31+
tester.hide()
32+
await tester.result()
33+
})
34+
35+
it('should update state on selection', async function () {
36+
const originalState = CodeIssueGroupingStrategyState.instance.getState()
37+
assert.equal(originalState, CodeIssueGroupingStrategy.Severity)
38+
39+
tester.selectItems(fileLocation)
40+
tester.addCallback(() => vscode.commands.executeCommand('workbench.action.acceptSelectedQuickOpenItem'))
41+
42+
await tester.result()
43+
assert.equal(CodeIssueGroupingStrategyState.instance.getState(), fileLocation.data)
44+
})
45+
})

packages/core/package.nls.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@
133133
"AWS.command.amazonq.acceptFix": "Accept Fix",
134134
"AWS.command.amazonq.regenerateFix": "Regenerate Fix",
135135
"AWS.command.amazonq.filterIssues": "Filter Issues",
136+
"AWS.command.amazonq.groupIssues": "Group Issues",
136137
"AWS.command.deploySamApplication": "Deploy SAM Application",
137138
"AWS.command.aboutToolkit": "About",
138139
"AWS.command.downloadLambda": "Download...",
@@ -309,6 +310,10 @@
309310
"AWS.amazonq.scans.projectScanInProgress": "Workspace review is in progress...",
310311
"AWS.amazonq.scans.fileScanInProgress": "File review is in progress...",
311312
"AWS.amazonq.scans.noGitRepo": "Your workspace is not in a git repository. I'll review your project files for security issues, and your in-flight changes for code quality issues.",
313+
"AWS.amazonq.scans.severity": "Severity",
314+
"AWS.amazonq.scans.fileLocation": "File Location",
315+
"AWS.amazonq.scans.groupIssues": "Group Issues",
316+
"AWS.amazonq.scans.groupIssues.placeholder": "Select how to group code issues",
312317
"AWS.amazonq.featureDev.error.conversationIdNotFoundError": "Conversation id must exist before starting code generation",
313318
"AWS.amazonq.featureDev.error.contentLengthError": "The folder you selected is too large for me to use as context. Please choose a smaller folder to work on. For more information on quotas, see the <a href=\"https://docs.aws.amazon.com/amazonq/latest/qdeveloper-ug/software-dev.html#quotas\" target=\"_blank\">Amazon Q Developer documentation.</a>",
314319
"AWS.amazonq.featureDev.error.illegalStateTransition": "Illegal transition between states, restart the conversation",

packages/core/src/codewhisperer/activation.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
SecurityTreeViewFilterState,
1919
AggregatedCodeScanIssue,
2020
CodeScanIssue,
21+
CodeIssueGroupingStrategyState,
2122
} from './models/model'
2223
import { invokeRecommendation } from './commands/invokeRecommendation'
2324
import { acceptSuggestion } from './commands/onInlineAcceptance'
@@ -60,6 +61,7 @@ import {
6061
ignoreAllIssues,
6162
focusIssue,
6263
showExploreAgentsView,
64+
showCodeIssueGroupingQuickPick,
6365
} from './commands/basicCommands'
6466
import { sleep } from '../shared/utilities/timeoutUtils'
6567
import { ReferenceLogViewProvider } from './service/referenceLogViewProvider'
@@ -289,6 +291,8 @@ export async function activate(context: ExtContext): Promise<void> {
289291
listCodeWhispererCommands.register(),
290292
// quick pick with security issues tree filters
291293
showSecurityIssueFilters.register(),
294+
// quick pick code issue grouping strategy
295+
showCodeIssueGroupingQuickPick.register(),
292296
// reset security issue filters
293297
clearFilters.register(),
294298
// handle security issues tree item clicked
@@ -297,6 +301,10 @@ export async function activate(context: ExtContext): Promise<void> {
297301
SecurityTreeViewFilterState.instance.onDidChangeState((e) => {
298302
SecurityIssueTreeViewProvider.instance.refresh()
299303
}),
304+
// refresh treeview when grouping strategy changes
305+
CodeIssueGroupingStrategyState.instance.onDidChangeState((e) => {
306+
SecurityIssueTreeViewProvider.instance.refresh()
307+
}),
300308
// show a no match state
301309
SecurityIssueTreeViewProvider.instance.onDidChangeTreeData((e) => {
302310
const noMatches =

packages/core/src/codewhisperer/client/codewhisperer.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ export interface CodeWhispererConfig {
3131
}
3232

3333
export const defaultServiceConfig: CodeWhispererConfig = {
34-
region: 'us-east-1',
35-
endpoint: 'https://codewhisperer.us-east-1.amazonaws.com/',
34+
region: 'us-west-2',
35+
endpoint: 'https://rts.alpha-us-west-2.codewhisperer.ai.aws.dev/',
3636
}
3737

3838
export function getCodewhispererConfig(): CodeWhispererConfig {

packages/core/src/codewhisperer/commands/basicCommands.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ import { DefaultAmazonQAppInitContext } from '../../amazonq/apps/initContext'
6868
import path from 'path'
6969
import { UserWrittenCodeTracker } from '../tracker/userWrittenCodeTracker'
7070
import { parsePatch } from 'diff'
71+
import { createCodeIssueGroupingStrategyPrompter } from '../ui/prompters'
7172

7273
const MessageTimeOut = 5_000
7374

@@ -887,6 +888,14 @@ export const showSecurityIssueFilters = Commands.declare({ id: 'aws.amazonq.secu
887888
}
888889
})
889890

891+
export const showCodeIssueGroupingQuickPick = Commands.declare(
892+
{ id: 'aws.amazonq.codescan.showGroupingStrategy' },
893+
() => async () => {
894+
const prompter = createCodeIssueGroupingStrategyPrompter()
895+
await prompter.prompt()
896+
}
897+
)
898+
890899
export const focusIssue = Commands.declare(
891900
{ id: 'aws.amazonq.security.focusIssue' },
892901
() => async (issue: CodeScanIssue, filePath: string) => {

packages/core/src/codewhisperer/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,5 @@ export * as CodeWhispererConstants from '../codewhisperer/models/constants'
102102
export { getSelectedCustomization, setSelectedCustomization, baseCustomization } from './util/customizationUtil'
103103
export { Container } from './service/serviceContainer'
104104
export * from './util/gitUtil'
105+
export * from './ui/prompters'
105106
export { UserWrittenCodeTracker } from './tracker/userWrittenCodeTracker'

0 commit comments

Comments
 (0)