Skip to content

Commit c09b09a

Browse files
authored
feat(dev): File-level accepts of generated code #5928
## Problem For the current /dev experience for code review, user needs to review the entire file list of generated code, reject the changes they do not want, and then accept the remaining changes in bulk at the end. This PR introduces improvements in user experience around the code review experience in which user can now review the file changes and accept them one at a time. User can start editing the file right away after accepting the changes without having to go through the entire generated file list. ## Solution - introduces a new action button for accepting file change for each change suggested on the file list - accepted changes are now highlighted in green - when accepting changes in bulk, the file list is updated to reflect the status if a change is rejected or accepted - minor text changes / improvements
1 parent 82ee006 commit c09b09a

File tree

27 files changed

+698
-142
lines changed

27 files changed

+698
-142
lines changed

package-lock.json

Lines changed: 8 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Feature",
3+
"description": "Amazon Q /dev: Add an action to accept individual files"
4+
}

packages/amazonq/test/e2e/amazonq/featureDev.test.ts

Lines changed: 126 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ describe('Amazon Q Feature Dev', function () {
1717
let tab: Messenger
1818

1919
const prompt = 'Add blank.txt file with empty content'
20-
const codegenApproachPrompt = prompt + ' and add a readme that describes the changes'
20+
const codegenApproachPrompt = `${prompt} and add a readme that describes the changes`
21+
const fileLevelAcceptPrompt = `${prompt} and add a license, and a contributing file`
2122
const tooManyRequestsWaitTime = 100000
2223

2324
function waitForButtons(buttons: FollowUpTypes[]) {
@@ -50,6 +51,14 @@ describe('Amazon Q Feature Dev', function () {
5051
)
5152
}
5253

54+
async function clickActionButton(filePath: string, actionName: string) {
55+
tab.clickFileActionButton(filePath, actionName)
56+
await tab.waitForEvent(() => !tab.hasAction(filePath, actionName), {
57+
waitIntervalInMs: 500,
58+
waitTimeoutInMs: 600000,
59+
})
60+
}
61+
5362
/**
5463
* Wait for the original request to finish.
5564
* If the response has a retry button or encountered a guardrails error, continue retrying
@@ -216,4 +225,120 @@ describe('Amazon Q Feature Dev', function () {
216225
await waitForButtons([FollowUpTypes.NewTask, FollowUpTypes.CloseSession])
217226
})
218227
})
228+
229+
describe('file-level accepts', async () => {
230+
beforeEach(async function () {
231+
tab.addChatMessage({ command: '/dev', prompt: fileLevelAcceptPrompt })
232+
await retryIfRequired(
233+
async () => {
234+
await tab.waitForChatFinishesLoading()
235+
},
236+
() => {
237+
tab.addChatMessage({ prompt })
238+
}
239+
)
240+
await retryIfRequired(async () => {
241+
await Promise.any([
242+
waitForButtons([FollowUpTypes.InsertCode, FollowUpTypes.ProvideFeedbackAndRegenerateCode]),
243+
waitForButtons([FollowUpTypes.Retry]),
244+
])
245+
})
246+
})
247+
248+
describe('fileList', async () => {
249+
it('has both accept-change and reject-change action buttons for file', async () => {
250+
const filePath = tab.getFilePaths()[0]
251+
assert.ok(tab.getActionsByFilePath(filePath).length === 2)
252+
assert.ok(tab.hasAction(filePath, 'accept-change'))
253+
assert.ok(tab.hasAction(filePath, 'reject-change'))
254+
})
255+
256+
it('has only revert-rejection action button for rejected file', async () => {
257+
const filePath = tab.getFilePaths()[0]
258+
await clickActionButton(filePath, 'reject-change')
259+
260+
assert.ok(tab.getActionsByFilePath(filePath).length === 1)
261+
assert.ok(tab.hasAction(filePath, 'revert-rejection'))
262+
})
263+
264+
it('does not have any of the action buttons for accepted file', async () => {
265+
const filePath = tab.getFilePaths()[0]
266+
await clickActionButton(filePath, 'accept-change')
267+
268+
assert.ok(tab.getActionsByFilePath(filePath).length === 0)
269+
})
270+
271+
it('disables all action buttons when new task is clicked', async () => {
272+
tab.clickButton(FollowUpTypes.InsertCode)
273+
await waitForButtons([FollowUpTypes.NewTask, FollowUpTypes.CloseSession])
274+
tab.clickButton(FollowUpTypes.NewTask)
275+
await waitForText('What new task would you like to work on?')
276+
277+
const filePaths = tab.getFilePaths()
278+
for (const filePath of filePaths) {
279+
assert.ok(tab.getActionsByFilePath(filePath).length === 0)
280+
}
281+
})
282+
283+
it('disables all action buttons when close session is clicked', async () => {
284+
tab.clickButton(FollowUpTypes.InsertCode)
285+
await waitForButtons([FollowUpTypes.NewTask, FollowUpTypes.CloseSession])
286+
tab.clickButton(FollowUpTypes.CloseSession)
287+
await waitForText(
288+
"Okay, I've ended this chat session. You can open a new tab to chat or start another workflow."
289+
)
290+
291+
const filePaths = tab.getFilePaths()
292+
for (const filePath of filePaths) {
293+
assert.ok(tab.getActionsByFilePath(filePath).length === 0)
294+
}
295+
})
296+
})
297+
298+
describe('accept button', async () => {
299+
describe('button text', async () => {
300+
it('shows "Accept all changes" when no files are accepted or rejected, and "Accept remaining changes" otherwise', async () => {
301+
let insertCodeButton = tab.getFollowUpButton(FollowUpTypes.InsertCode)
302+
assert.ok(insertCodeButton.pillText === 'Accept all changes')
303+
304+
const filePath = tab.getFilePaths()[0]
305+
await clickActionButton(filePath, 'reject-change')
306+
307+
insertCodeButton = tab.getFollowUpButton(FollowUpTypes.InsertCode)
308+
assert.ok(insertCodeButton.pillText === 'Accept remaining changes')
309+
310+
await clickActionButton(filePath, 'revert-rejection')
311+
312+
insertCodeButton = tab.getFollowUpButton(FollowUpTypes.InsertCode)
313+
assert.ok(insertCodeButton.pillText === 'Accept all changes')
314+
315+
await clickActionButton(filePath, 'accept-change')
316+
317+
insertCodeButton = tab.getFollowUpButton(FollowUpTypes.InsertCode)
318+
assert.ok(insertCodeButton.pillText === 'Accept remaining changes')
319+
})
320+
321+
it('shows "Continue" when all files are either accepted or rejected, with at least one of them rejected', async () => {
322+
const filePaths = tab.getFilePaths()
323+
for (const filePath of filePaths) {
324+
await clickActionButton(filePath, 'reject-change')
325+
}
326+
327+
const insertCodeButton = tab.getFollowUpButton(FollowUpTypes.InsertCode)
328+
assert.ok(insertCodeButton.pillText === 'Continue')
329+
})
330+
})
331+
332+
it('disappears and automatically moves on to the next step when all changes are accepted', async () => {
333+
const filePaths = tab.getFilePaths()
334+
for (const filePath of filePaths) {
335+
await clickActionButton(filePath, 'accept-change')
336+
}
337+
await waitForButtons([FollowUpTypes.NewTask, FollowUpTypes.CloseSession])
338+
339+
assert.ok(tab.hasButton(FollowUpTypes.InsertCode) === false)
340+
assert.ok(tab.hasButton(FollowUpTypes.ProvideFeedbackAndRegenerateCode) === false)
341+
})
342+
})
343+
})
219344
})

packages/amazonq/test/e2e/amazonq/framework/messenger.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ export class Messenger {
5959
this.mynahUIProps.onFollowUpClicked(this.tabID, lastChatItem?.messageId ?? '', option[0])
6060
}
6161

62+
clickFileActionButton(filePath: string, actionName: string) {
63+
if (!this.mynahUIProps.onFileActionClick) {
64+
assert.fail('onFileActionClick must be defined to use it in the tests')
65+
}
66+
67+
this.mynahUIProps.onFileActionClick(this.tabID, this.getFileListMessageId(), filePath, actionName)
68+
}
69+
6270
findCommand(command: string) {
6371
return this.getCommands()
6472
.map((groups) => groups.commands)
@@ -78,6 +86,52 @@ export class Messenger {
7886
return this.getStore().promptInputPlaceholder
7987
}
8088

89+
getFollowUpButton(type: FollowUpTypes) {
90+
const followUpButton = this.getChatItems()
91+
.pop()
92+
?.followUp?.options?.find((action) => action.type === type)
93+
if (!followUpButton) {
94+
assert.fail(`Could not find follow up button with type ${type}`)
95+
}
96+
return followUpButton
97+
}
98+
99+
getFileList() {
100+
const chatItems = this.getChatItems()
101+
const fileList = chatItems.find((item) => 'fileList' in item)
102+
if (!fileList) {
103+
assert.fail('Could not find file list')
104+
}
105+
return fileList
106+
}
107+
108+
getFileListMessageId() {
109+
const fileList = this.getFileList()
110+
const messageId = fileList?.messageId
111+
if (!messageId) {
112+
assert.fail('Could not find file list message id')
113+
}
114+
return messageId
115+
}
116+
117+
getFilePaths() {
118+
const fileList = this.getFileList()
119+
const filePaths = fileList?.fileList?.filePaths
120+
if (!filePaths) {
121+
assert.fail('Could not find file paths')
122+
}
123+
if (filePaths.length === 0) {
124+
assert.fail('File paths list is empty')
125+
}
126+
return filePaths
127+
}
128+
129+
getActionsByFilePath(filePath: string) {
130+
const fileList = this.getFileList()
131+
const actions = fileList?.fileList?.actions
132+
return actions?.[filePath] ?? []
133+
}
134+
81135
hasButton(type: FollowUpTypes) {
82136
return (
83137
this.getChatItems()
@@ -87,6 +141,10 @@ export class Messenger {
87141
)
88142
}
89143

144+
hasAction(filePath: string, actionName: string) {
145+
return this.getActionsByFilePath(filePath).some((action) => action.name === actionName)
146+
}
147+
90148
async waitForChatFinishesLoading() {
91149
return this.waitForEvent(() => this.getStore().loadingChat === false || this.hasButton(FollowUpTypes.Retry))
92150
}

packages/amazonq/test/unit/amazonqFeatureDev/session/session.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ describe('session', () => {
8383
rejected: false,
8484
virtualMemoryUri: uri,
8585
workspaceFolder: controllerSetup.workspaceFolder,
86+
changeApplied: false,
8687
},
8788
{
8889
zipFilePath: 'rejectedFile.js',
@@ -91,6 +92,7 @@ describe('session', () => {
9192
rejected: true,
9293
virtualMemoryUri: generateVirtualMemoryUri(uploadID, 'rejectedFile.js'),
9394
workspaceFolder: controllerSetup.workspaceFolder,
95+
changeApplied: false,
9496
},
9597
],
9698
[],

packages/core/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@
470470
"@aws-sdk/property-provider": "3.46.0",
471471
"@aws-sdk/smithy-client": "^3.46.0",
472472
"@aws-sdk/util-arn-parser": "^3.46.0",
473-
"@aws/mynah-ui": "^4.15.11",
473+
"@aws/mynah-ui": "^4.18.0",
474474
"@gerhobbelt/gitignore-parser": "^0.2.0-9",
475475
"@iarna/toml": "^2.2.5",
476476
"@smithy/middleware-retry": "^2.3.1",

packages/core/package.nls.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,9 @@
312312
"AWS.amazonq.featureDev.pillText.generatingCode": "Generating code...",
313313
"AWS.amazonq.featureDev.pillText.requestingChanges": "Requesting changes ...",
314314
"AWS.amazonq.featureDev.pillText.insertCode": "Accept code",
315+
"AWS.amazonq.featureDev.pillText.continue": "Continue",
316+
"AWS.amazonq.featureDev.pillText.acceptAllChanges": "Accept all changes",
317+
"AWS.amazonq.featureDev.pillText.acceptRemainingChanges": "Accept remaining changes",
315318
"AWS.amazonq.featureDev.pillText.stoppingCodeGeneration": "Stopping code generation...",
316319
"AWS.amazonq.featureDev.pillText.sendFeedback": "Send feedback",
317320
"AWS.amazonq.featureDev.pillText.selectFiles": "Select files for context",

packages/core/src/amazonq/commons/diff.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ export async function openDiff(leftPath: string, rightPath: string, tabId: strin
1313
}
1414

1515
export async function openDeletedDiff(filePath: string, name: string, tabId: string) {
16-
const fileUri = await getOriginalFileUri(filePath, tabId)
17-
await vscode.commands.executeCommand('vscode.open', fileUri, {}, `${name} (Deleted)`)
16+
const left = await getOriginalFileUri(filePath, tabId)
17+
const right = createAmazonQUri('empty', tabId)
18+
await vscode.commands.executeCommand('vscode.diff', left, right, `${name} (Deleted)`)
1819
}
1920

2021
export async function getOriginalFileUri(fullPath: string, tabId: string) {
@@ -32,3 +33,11 @@ export function createAmazonQUri(path: string, tabId: string) {
3233
// TODO change the featureDevScheme to a more general amazon q scheme
3334
return vscode.Uri.from({ scheme: featureDevScheme, path, query: `tabID=${tabId}` })
3435
}
36+
37+
export async function openFile(path: string) {
38+
if (!(await fs.exists(path))) {
39+
return
40+
}
41+
const fileUri = vscode.Uri.file(path)
42+
await vscode.commands.executeCommand('vscode.diff', fileUri, fileUri)
43+
}

packages/core/src/amazonq/index.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,14 @@ export { amazonQHelpUrl } from '../shared/constants'
2727
export { listCodeWhispererCommandsWalkthrough } from '../codewhisperer/ui/statusBarMenu'
2828
export { focusAmazonQPanel, focusAmazonQPanelKeybinding } from '../codewhispererChat/commands/registerCommands'
2929
export { TryChatCodeLensProvider, tryChatCodeLensCommand } from '../codewhispererChat/editor/codelens'
30-
export { createAmazonQUri, openDiff, openDeletedDiff, getOriginalFileUri, getFileDiffUris } from './commons/diff'
30+
export {
31+
createAmazonQUri,
32+
openDiff,
33+
openDeletedDiff,
34+
getOriginalFileUri,
35+
getFileDiffUris,
36+
openFile,
37+
} from './commons/diff'
3138
export { CodeReference } from '../codewhispererChat/view/connector/connector'
3239
export { AuthMessageDataMap, AuthFollowUpType } from './auth/model'
3340
export { extractAuthFollowUp } from './util/authUtils'

0 commit comments

Comments
 (0)