Skip to content

Commit 042ce00

Browse files
HweinstockHweinstock
authored
fix(amazonq): inline suggestion scrolling now uses built-in VSC commands. (#7323)
## Problem The current implementation overwrites the `editor.action.inlineSuggest.showPrevious` and `editor.action.inlineSuggest.showNext` commands to swap out the inlineProvider. These custom overwrites cause a few issues: - forces us to maintain a ton of state [leads to bugs, some of which were observed in bug bash]. - causes the suggestions to "refresh" when going through them since we are re-instantiating the provider on "scrolls". - doesn't natively support "accept word"/"accept line". ## Solution - Remove the overwrite, remove reliance of `session` state. - Simplify implementation to use native VSC commands. ## Fixed Bugs - Scrolling through suggestions now works and doesn't "refresh". - Accept Word/Accept Line now work properly. - Significantly faster to scroll through suggestions. - Inline Reference Codelense no longer flashes when scrolling through suggestions. ## Tests and Verification: Testing this change it feels significantly 'smoother'. https://github.com/user-attachments/assets/631ee822-3cab-46b5-a07c-b3d4b8eb7d3b ## Future Work - with some more aggressive refactoring, and once telemetry is mostly in Flare, we can completely delete the sessionManager. - revisit reference codelense, and see if its the right choice. Maybe a message instead? --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent a814ee0 commit 042ce00

File tree

4 files changed

+25
-192
lines changed

4 files changed

+25
-192
lines changed

‎packages/amazonq/src/app/inline/completion.ts‎

Lines changed: 18 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import { RecommendationService } from './recommendationService'
2828
import {
2929
CodeWhispererConstants,
3030
ReferenceHoverProvider,
31-
ReferenceInlineProvider,
3231
ReferenceLogViewProvider,
3332
ImportAdderProvider,
3433
CodeSuggestionsState,
@@ -158,39 +157,6 @@ export class InlineCompletionManager implements Disposable {
158157
this.languageClient.sendNotification(this.logSessionResultMessageName, params)
159158
}
160159
commands.registerCommand('aws.amazonq.rejectCodeSuggestion', onInlineRejection)
161-
162-
/*
163-
We have to overwrite the prev. and next. commands because the inlineCompletionProvider only contained the current item
164-
To show prev. and next. recommendation we need to re-register a new provider with the previous or next item
165-
*/
166-
167-
const swapProviderAndShow = async () => {
168-
await commands.executeCommand('editor.action.inlineSuggest.hide')
169-
this.disposable.dispose()
170-
this.disposable = languages.registerInlineCompletionItemProvider(
171-
CodeWhispererConstants.platformLanguageIds,
172-
new AmazonQInlineCompletionItemProvider(
173-
this.languageClient,
174-
this.recommendationService,
175-
this.sessionManager,
176-
this.inlineTutorialAnnotation,
177-
false
178-
)
179-
)
180-
await commands.executeCommand('editor.action.inlineSuggest.trigger')
181-
}
182-
183-
const prevCommandHandler = async () => {
184-
this.sessionManager.decrementActiveIndex()
185-
await swapProviderAndShow()
186-
}
187-
commands.registerCommand('editor.action.inlineSuggest.showPrevious', prevCommandHandler)
188-
189-
const nextCommandHandler = async () => {
190-
this.sessionManager.incrementActiveIndex()
191-
await swapProviderAndShow()
192-
}
193-
commands.registerCommand('editor.action.inlineSuggest.showNext', nextCommandHandler)
194160
}
195161
}
196162

@@ -199,8 +165,7 @@ export class AmazonQInlineCompletionItemProvider implements InlineCompletionItem
199165
private readonly languageClient: LanguageClient,
200166
private readonly recommendationService: RecommendationService,
201167
private readonly sessionManager: SessionManager,
202-
private readonly inlineTutorialAnnotation: InlineTutorialAnnotation,
203-
private readonly isNewSession: boolean = true
168+
private readonly inlineTutorialAnnotation: InlineTutorialAnnotation
204169
) {}
205170

206171
provideInlineCompletionItems = debounce(
@@ -217,27 +182,24 @@ export class AmazonQInlineCompletionItemProvider implements InlineCompletionItem
217182
): Promise<InlineCompletionItem[]> {
218183
try {
219184
vsCodeState.isRecommendationsActive = true
220-
if (this.isNewSession) {
221-
const isAutoTrigger = context.triggerKind === InlineCompletionTriggerKind.Automatic
222-
if (isAutoTrigger && !CodeSuggestionsState.instance.isSuggestionsEnabled()) {
223-
// return early when suggestions are disabled with auto trigger
224-
return []
225-
}
226-
227-
// tell the tutorial that completions has been triggered
228-
await this.inlineTutorialAnnotation.triggered(context.triggerKind)
229-
TelemetryHelper.instance.setInvokeSuggestionStartTime()
230-
TelemetryHelper.instance.setTriggerType(context.triggerKind)
231-
232-
// make service requests if it's a new session
233-
await this.recommendationService.getAllRecommendations(
234-
this.languageClient,
235-
document,
236-
position,
237-
context,
238-
token
239-
)
185+
const isAutoTrigger = context.triggerKind === InlineCompletionTriggerKind.Automatic
186+
if (isAutoTrigger && !CodeSuggestionsState.instance.isSuggestionsEnabled()) {
187+
// return early when suggestions are disabled with auto trigger
188+
return []
240189
}
190+
191+
// tell the tutorial that completions has been triggered
192+
await this.inlineTutorialAnnotation.triggered(context.triggerKind)
193+
TelemetryHelper.instance.setInvokeSuggestionStartTime()
194+
TelemetryHelper.instance.setTriggerType(context.triggerKind)
195+
196+
await this.recommendationService.getAllRecommendations(
197+
this.languageClient,
198+
document,
199+
position,
200+
context,
201+
token
202+
)
241203
// get active item from session for displaying
242204
const items = this.sessionManager.getActiveRecommendation()
243205
const session = this.sessionManager.getActiveSession()
@@ -271,11 +233,6 @@ export class AmazonQInlineCompletionItemProvider implements InlineCompletionItem
271233
}
272234
item.range = new Range(cursorPosition, cursorPosition)
273235
item.insertText = typeof item.insertText === 'string' ? item.insertText : item.insertText.value
274-
ReferenceInlineProvider.instance.setInlineReference(
275-
cursorPosition.line,
276-
item.insertText,
277-
item.references
278-
)
279236
ImportAdderProvider.instance.onShowRecommendation(document, cursorPosition.line, item)
280237
}
281238
return items as InlineCompletionItem[]

‎packages/amazonq/src/app/inline/sessionManager.ts‎

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ interface CodeWhispererSession {
1717

1818
export class SessionManager {
1919
private activeSession?: CodeWhispererSession
20-
private activeIndex: number = 0
2120
private _acceptedSuggestionCount: number = 0
2221

2322
constructor() {}
@@ -35,7 +34,6 @@ export class SessionManager {
3534
requestStartTime,
3635
firstCompletionDisplayLatency,
3736
}
38-
this.activeIndex = 0
3937
}
4038

4139
public closeSession() {
@@ -56,45 +54,8 @@ export class SessionManager {
5654
this.activeSession.suggestions = [...this.activeSession.suggestions, ...suggestions]
5755
}
5856

59-
public incrementActiveIndex() {
60-
const suggestionCount = this.activeSession?.suggestions?.length
61-
if (!suggestionCount) {
62-
return
63-
}
64-
this.activeIndex === suggestionCount - 1 ? suggestionCount - 1 : this.activeIndex++
65-
}
66-
67-
public decrementActiveIndex() {
68-
this.activeIndex === 0 ? 0 : this.activeIndex--
69-
}
70-
71-
/*
72-
We have to maintain the active suggestion index ourselves because VS Code doesn't expose which suggestion it's currently showing
73-
In order to keep track of the right suggestion state, and for features such as reference tracker, this hack is still needed
74-
*/
75-
7657
public getActiveRecommendation(): InlineCompletionItemWithReferences[] {
77-
let suggestionCount = this.activeSession?.suggestions.length
78-
if (!suggestionCount) {
79-
return []
80-
}
81-
if (suggestionCount === 1 && this.activeSession?.isRequestInProgress) {
82-
suggestionCount += 1
83-
}
84-
85-
const activeSuggestion = this.activeSession?.suggestions[this.activeIndex]
86-
if (!activeSuggestion) {
87-
return []
88-
}
89-
const items = [activeSuggestion]
90-
// to make the total number of suggestions match the actual number
91-
for (let i = 1; i < suggestionCount; i++) {
92-
items.push({
93-
...activeSuggestion,
94-
insertText: `${i}`,
95-
})
96-
}
97-
return items
58+
return this.activeSession?.suggestions ?? []
9859
}
9960

10061
public get acceptedSuggestionCount(): number {
@@ -107,6 +68,5 @@ export class SessionManager {
10768

10869
public clear() {
10970
this.activeSession = undefined
110-
this.activeIndex = 0
11171
}
11272
}

‎packages/amazonq/test/unit/amazonq/apps/inline/completion.test.ts‎

Lines changed: 6 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,7 @@ import { AmazonQInlineCompletionItemProvider, InlineCompletionManager } from '..
2020
import { RecommendationService } from '../../../../../src/app/inline/recommendationService'
2121
import { SessionManager } from '../../../../../src/app/inline/sessionManager'
2222
import { createMockDocument, createMockTextEditor, getTestWindow, installFakeClock } from 'aws-core-vscode/test'
23-
import {
24-
noInlineSuggestionsMsg,
25-
ReferenceHoverProvider,
26-
ReferenceInlineProvider,
27-
ReferenceLogViewProvider,
28-
} from 'aws-core-vscode/codewhisperer'
23+
import { noInlineSuggestionsMsg, ReferenceHoverProvider, ReferenceLogViewProvider } from 'aws-core-vscode/codewhisperer'
2924
import { InlineGeneratingMessage } from '../../../../../src/app/inline/inlineGeneratingMessage'
3025
import { LineTracker } from '../../../../../src/app/inline/stateTracker/lineTracker'
3126
import { InlineTutorialAnnotation } from '../../../../../src/app/inline/tutorials/inlineTutorialAnnotation'
@@ -230,46 +225,6 @@ describe('InlineCompletionManager', () => {
230225
assert(registerProviderStub.calledTwice) // Once in constructor, once after rejection
231226
})
232227
})
233-
234-
describe('previous command', () => {
235-
it('should register and handle previous command correctly', async () => {
236-
const prevCommandCall = registerCommandStub
237-
.getCalls()
238-
.find((call) => call.args[0] === 'editor.action.inlineSuggest.showPrevious')
239-
240-
assert(prevCommandCall, 'Previous command should be registered')
241-
242-
if (prevCommandCall) {
243-
const handler = prevCommandCall.args[1]
244-
await handler()
245-
246-
assert(executeCommandStub.calledWith('editor.action.inlineSuggest.hide'))
247-
assert(disposableStub.calledOnce)
248-
assert(registerProviderStub.calledTwice)
249-
assert(executeCommandStub.calledWith('editor.action.inlineSuggest.trigger'))
250-
}
251-
})
252-
})
253-
254-
describe('next command', () => {
255-
it('should register and handle next command correctly', async () => {
256-
const nextCommandCall = registerCommandStub
257-
.getCalls()
258-
.find((call) => call.args[0] === 'editor.action.inlineSuggest.showNext')
259-
260-
assert(nextCommandCall, 'Next command should be registered')
261-
262-
if (nextCommandCall) {
263-
const handler = nextCommandCall.args[1]
264-
await handler()
265-
266-
assert(executeCommandStub.calledWith('editor.action.inlineSuggest.hide'))
267-
assert(disposableStub.calledOnce)
268-
assert(registerProviderStub.calledTwice)
269-
assert(executeCommandStub.calledWith('editor.action.inlineSuggest.trigger'))
270-
}
271-
})
272-
})
273228
})
274229

275230
describe('AmazonQInlineCompletionItemProvider', () => {
@@ -278,15 +233,13 @@ describe('InlineCompletionManager', () => {
278233
let provider: AmazonQInlineCompletionItemProvider
279234
let getAllRecommendationsStub: sinon.SinonStub
280235
let recommendationService: RecommendationService
281-
let setInlineReferenceStub: sinon.SinonStub
282236
let inlineTutorialAnnotation: InlineTutorialAnnotation
283237

284238
beforeEach(() => {
285239
const lineTracker = new LineTracker()
286240
const activeStateController = new InlineGeneratingMessage(lineTracker)
287241
inlineTutorialAnnotation = new InlineTutorialAnnotation(lineTracker, mockSessionManager)
288242
recommendationService = new RecommendationService(mockSessionManager, activeStateController)
289-
setInlineReferenceStub = sandbox.stub(ReferenceInlineProvider.instance, 'setInlineReference')
290243

291244
mockSessionManager = {
292245
getActiveSession: getActiveSessionStub,
@@ -320,48 +273,21 @@ describe('InlineCompletionManager', () => {
320273
assert(getAllRecommendationsStub.calledOnce)
321274
assert.deepStrictEqual(items, mockSuggestions)
322275
}),
323-
it('should not call recommendation service for existing sessions', async () => {
324-
provider = new AmazonQInlineCompletionItemProvider(
325-
languageClient,
326-
recommendationService,
327-
mockSessionManager,
328-
inlineTutorialAnnotation,
329-
false
330-
)
331-
const items = await provider.provideInlineCompletionItems(
332-
mockDocument,
333-
mockPosition,
334-
mockContext,
335-
mockToken
336-
)
337-
assert(getAllRecommendationsStub.notCalled)
338-
assert.deepStrictEqual(items, mockSuggestions)
339-
}),
340276
it('should handle reference if there is any', async () => {
341277
provider = new AmazonQInlineCompletionItemProvider(
342278
languageClient,
343279
recommendationService,
344280
mockSessionManager,
345-
inlineTutorialAnnotation,
346-
false
281+
inlineTutorialAnnotation
347282
)
348283
await provider.provideInlineCompletionItems(mockDocument, mockPosition, mockContext, mockToken)
349-
assert(setInlineReferenceStub.calledOnce)
350-
assert(
351-
setInlineReferenceStub.calledWithExactly(
352-
mockPosition.line,
353-
mockSuggestions[0].insertText,
354-
fakeReferences
355-
)
356-
)
357284
}),
358285
it('should add a range to the completion item when missing', async function () {
359286
provider = new AmazonQInlineCompletionItemProvider(
360287
languageClient,
361288
recommendationService,
362289
mockSessionManager,
363-
inlineTutorialAnnotation,
364-
true
290+
inlineTutorialAnnotation
365291
)
366292
getActiveRecommendationStub.returns([
367293
{
@@ -391,8 +317,7 @@ describe('InlineCompletionManager', () => {
391317
languageClient,
392318
recommendationService,
393319
mockSessionManager,
394-
inlineTutorialAnnotation,
395-
true
320+
inlineTutorialAnnotation
396321
)
397322
const expectedText = 'this is my text'
398323
getActiveRecommendationStub.returns([
@@ -415,8 +340,7 @@ describe('InlineCompletionManager', () => {
415340
languageClient,
416341
recommendationService,
417342
mockSessionManager,
418-
inlineTutorialAnnotation,
419-
true
343+
inlineTutorialAnnotation
420344
)
421345
getActiveRecommendationStub.returns([])
422346
const messageShown = new Promise((resolve) =>
@@ -449,8 +373,7 @@ describe('InlineCompletionManager', () => {
449373
languageClient,
450374
recommendationService,
451375
mockSessionManager,
452-
inlineTutorialAnnotation,
453-
false
376+
inlineTutorialAnnotation
454377
)
455378
const p1 = provider.provideInlineCompletionItems(mockDocument, mockPosition, mockContext, mockToken)
456379
const p2 = provider.provideInlineCompletionItems(mockDocument, mockPosition, mockContext, mockToken)

‎packages/amazonq/test/unit/amazonq/apps/inline/recommendationService.test.ts‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,6 @@ describe('RecommendationService', () => {
111111
...expectedRequestArgs,
112112
partialResultToken: mockPartialResultToken,
113113
})
114-
115-
// Verify session management
116-
const items = sessionManager.getActiveRecommendation()
117-
assert.deepStrictEqual(items, [mockInlineCompletionItemOne, { insertText: '1' } as InlineCompletionItem])
118-
sessionManager.incrementActiveIndex()
119-
const items2 = sessionManager.getActiveRecommendation()
120-
assert.deepStrictEqual(items2, [mockInlineCompletionItemTwo, { insertText: '1' } as InlineCompletionItem])
121114
})
122115
})
123116
})

0 commit comments

Comments
 (0)