Skip to content

fix(amazonq): block completion before active edit is accepted/rejected #8141

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
Oct 6, 2025
Merged

fix(amazonq): block completion before active edit is accepted/rejected #8141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9d0b54
fix(amazonq): block completion before active edit is accepted/rejected
bywang56 Oct 3, 2025
1371364
Merge branch 'master' into feature/inlinebug
bywang56 Oct 6, 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
21 changes: 17 additions & 4 deletions packages/amazonq/src/app/inline/EditRendering/displayImage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,11 @@ export class EditDecorationManager {
newCode: string,
originalCodeHighlightRanges: Array<{ line: number; start: number; end: number }>
): Promise<void> {
await this.clearDecorations(editor)

await setContext('aws.amazonq.editSuggestionActive' as any, true)
EditSuggestionState.setEditSuggestionActive(true)
// Clear old decorations but don't reset state (state is already set in displaySvgDecoration)
Copy link
Contributor

@atonaamz atonaamz Oct 6, 2025

Choose a reason for hiding this comment

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

Why do we need to move the logic to displaySvgDecoration? I think we clear decoration after making sure that all condition passes which is in displayEditSuggestion? should we just need the condition in recommendationService.ts?

if (!isTriggerByDeletion && !request.partialResultToken && !EditSuggestionState.isEditSuggestionActive()) {

Copy link
Contributor Author

@bywang56 bywang56 Oct 6, 2025

Choose a reason for hiding this comment

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

The check in recommendationService.ts alone isn't sufficient. Without setting the state early in displaySvgDecoration(), there's a race condition window between when the edit flow starts and when the state is finally set. During this window (which includes async operations like isCompletionActive() and applyPatch()), completion requests can check the state, see it as false, and proceed. Both changes are needed for proper mutual exclusion.

editor.setDecorations(this.imageDecorationType, [])
editor.setDecorations(this.removedCodeDecorationType, [])
this.currentImageDecoration = undefined
this.currentRemovedCodeDecorations = []

this.acceptHandler = onAccept
this.rejectHandler = onReject
Expand Down Expand Up @@ -313,8 +314,16 @@ export async function displaySvgDecoration(
) {
const originalCode = editor.document.getText()

// Set edit state immediately to prevent race condition with completion requests
Copy link
Contributor

@atonaamz atonaamz Oct 6, 2025

Choose a reason for hiding this comment

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

what if the state is set to false later and we already skip completion request?

Copy link
Contributor Author

@bywang56 bywang56 Oct 6, 2025

Choose a reason for hiding this comment

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

Setting state to TRUE early and then potentially setting it to FALSE later (if edit fails) is the correct behavior. When we set state to TRUE, we're indicating "an edit operation is in progress" - completion should be skipped during this time regardless of whether the edit ultimately succeeds or fails. If the edit fails, we clean up the state, and future completion requests will work normally. The alternative (not setting state early) is what causes the bug where both suggestions appear simultaneously.

await setContext('aws.amazonq.editSuggestionActive' as any, true)
EditSuggestionState.setEditSuggestionActive(true)

// Check if a completion suggestion is currently active - if so, discard edit suggestion
if (inlineCompletionProvider && (await inlineCompletionProvider.isCompletionActive())) {
// Clean up state since we're not showing the edit
await setContext('aws.amazonq.editSuggestionActive' as any, false)
EditSuggestionState.setEditSuggestionActive(false)

// Emit DISCARD telemetry for edit suggestion that can't be shown due to active completion
const params = createDiscardTelemetryParams(session, item)
languageClient.sendNotification('aws/logInlineCompletionSessionResults', params)
Expand All @@ -326,6 +335,10 @@ export async function displaySvgDecoration(

const isPatchValid = applyPatch(editor.document.getText(), item.insertText as string)
if (!isPatchValid) {
// Clean up state since we're not showing the edit
await setContext('aws.amazonq.editSuggestionActive' as any, false)
EditSuggestionState.setEditSuggestionActive(false)

const params = createDiscardTelemetryParams(session, item)
// TODO: this session is closed on flare side hence discarded is not emitted in flare
languageClient.sendNotification('aws/logInlineCompletionSessionResults', params)
Expand Down
2 changes: 1 addition & 1 deletion packages/amazonq/src/app/inline/recommendationService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export class RecommendationService {
* Completions use PartialResultToken with single 1 call of [getAllRecommendations].
* Edits leverage partialResultToken to achieve EditStreak such that clients can pull all continuous suggestions generated by the model within 1 EOS block.
*/
if (!isTriggerByDeletion && !request.partialResultToken) {
if (!isTriggerByDeletion && !request.partialResultToken && !EditSuggestionState.isEditSuggestionActive()) {
const completionPromise: Promise<InlineCompletionListWithReferences> = languageClient.sendRequest(
inlineCompletionWithReferencesRequestType.method,
request,
Expand Down
8 changes: 5 additions & 3 deletions packages/amazonq/test/unit/amazonq/apps/inline/recommendationService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ describe('RecommendationService', () => {
}
})

it('should make completion request when edit suggestion is active', async () => {
it('should not make completion request when edit suggestion is active', async () => {
// Mock EditSuggestionState to return true (edit suggestion is active)
sandbox.stub(EditSuggestionState, 'isEditSuggestionActive').returns(true)

Expand All @@ -360,13 +360,15 @@ describe('RecommendationService', () => {
const completionCalls = cs.filter((c) => c.firstArg === completionApi)
const editCalls = cs.filter((c) => c.firstArg === editApi)

assert.strictEqual(cs.length, 2) // Only edit call
assert.strictEqual(completionCalls.length, 1) // No completion calls
assert.strictEqual(cs.length, 1) // Only edit call
assert.strictEqual(completionCalls.length, 0) // No completion calls
assert.strictEqual(editCalls.length, 1) // One edit call
})

it('should make completion request when edit suggestion is not active', async () => {
// Mock EditSuggestionState to return false (no edit suggestion active)
sandbox.stub(EditSuggestionState, 'isEditSuggestionActive').returns(false)

const mockResult = {
sessionId: 'test-session',
items: [mockInlineCompletionItemOne],
Expand Down
Loading