Skip to content

fix(amazonq): inline suggestion scrolling now uses built-in VSC commands. #7323

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.

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 7 commits into from
May 16, 2025

Conversation

Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented May 15, 2025

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'.

suggestionScrollingFIx.mov

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.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

This comment was marked as resolved.

@Hweinstock Hweinstock changed the title fix(amazonq): inline suggestion scrolling now uses built-in VSC features. fix(amazonq): inline suggestion scrolling now uses built-in VSC commands. May 15, 2025
@Hweinstock Hweinstock marked this pull request as ready for review May 15, 2025 22:20
@Hweinstock Hweinstock requested a review from a team as a code owner May 15, 2025 22:20
@Hweinstock Hweinstock marked this pull request as draft May 15, 2025 22:22
@Hweinstock Hweinstock marked this pull request as ready for review May 15, 2025 23:40
@Hweinstock
Copy link
Contributor Author

lint-duplicate-code failure is unrelated:

[
  {
    first: 'https://github.com/aws/aws-toolkit-vscode/blob/ab045d280548db0929d2958ad96a39ceacbcaea1/packages/core/src/codewhisperer/util/runtimeLanguageContext.ts#L262-L276',
    second: 'https://github.com/aws/aws-toolkit-vscode/blob/ab045d280548db0929d2958ad96a39ceacbcaea1/packages/core/src/codewhisperer/util/runtimeLanguageContext.ts#L141-L155',
    numberOfLines: [15](https://github.com/aws/aws-toolkit-vscode/actions/runs/15057354954/job/42325918841?pr=7323#step:10:16)
  }
]

To show prev. and next. recommendation we need to re-register a new provider with the previous or next item
*/

const swapProviderAndShow = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @zixlin7 was this stuff added to support inline completions that come after the initial request? or is it no longer needed? As far as I can see from the demo we still have pagination

Copy link
Contributor Author

@Hweinstock Hweinstock May 16, 2025

Choose a reason for hiding this comment

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

My understanding of the old implementation is that we fetch all the suggestions, add them to the session, and then request them from the session one at a time (by triggering the provideInlineCompletion via overwriting the scroll commands). This is where the state tracking comes in to manage what the "active" suggestion is. We also add 'placeholder' items so that VSC knows we can scroll.

Since we're fetching all the suggestions anyway, we can return those directly and avoid 'placeholder' items, and additional triggers of provideInlineCompletion.

However, I don't have all the context on the original reason for this decision.

When testing this, I also noticed a slight bug on current prod where it always shows were on suggestion 1, even when we scroll.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checked with Zoe and sounds like this is a hack to handle the codelenses for references and imports.

  • references are confirmed working in fix(amazonq): code reference codelense works #7331.
  • import codelense is unknown. Note: There weren't any reported issue in first three bug bashes, so either this is not a common case, or already works. However, still worth following up to confirm this.

@Hweinstock Hweinstock merged commit 042ce00 into aws:feature/flare-inline May 16, 2025
21 of 22 checks passed
@Hweinstock Hweinstock deleted the fix/suggestions branch May 16, 2025 22:14
Hweinstock added a commit that referenced this pull request May 16, 2025
builds off #7323


## Problem
- Code reference codelense doesn't open the code reference panel, it
opens problems.
- Sometimes the codelense never goes away. 

(Note: both of these bugs are on prod)

## Solution
- Hide the codelense after 5 seconds. 
- Properly link the codelense to code reference log panel. 

## Verification


https://github.com/user-attachments/assets/3a231ab4-1e25-4746-864a-d66f5e852686



---

- 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants