Skip to content

Conversation

@bywang56
Copy link
Contributor

@bywang56 bywang56 commented Oct 9, 2025

Problem

When pressing Enter to trigger inline suggestions, ghost text fails to appear in the editor despite the language server returning valid suggestions.

Root Cause: VS Code triggers two provideInlineCompletionItems calls simultaneously when Enter is pressed:

  1. Automatic trigger (t=0ms) - sends LSP request, receives valid suggestions after ~380ms
  2. Invoke trigger (t=2ms) - sends duplicate LSP request, receives empty response after ~6ms (language server skips concurrent requests)
  3. VS Code uses the most recent provider response, which is the empty result from the second call, causing the first call's valid suggestions to be ignored.

Evidence from logs:

[info] Sending inline completion request (474)
[info] Sending inline completion request (476)
[info] Received response (476): { sessionId: '', items: [] }  ← Used by VS Code
[info] Received response (474): { items: [{ itemId: '...', insertText: '...' }] }  ← Ignored

Solution

I am not able to have a working fix to prevent either automatic or invoke trigger, which is controlled in toolkit. So I implement request deduplication on Q side by tracking pending requests in the inline completion provider:

  • Shared Request Pattern: When a request is in progress, subsequent concurrent calls reuse the same pending promise instead of creating new LSP requests

  • Independent Cancellation: Each call checks its own CancellationToken after the shared request completes, allowing independent cancellation semantics

Key Changes:

  • Added pendingRequest field to track in-flight requests
  • Wrapped provideInlineCompletionItems to check for and reuse pending requests
  • Added per-call cancellation check when reusing requests
  • Ensured pendingRequest is cleared after completion for fresh subsequent requests

Tests:

Tested on the same scenarios, both concurrent calls now return the same valid suggestions, ghost text appears reliably when pressing Enter

Logs after fix:

[info] _provideInlineCompletionItems called (Automatic)
[info] _provideInlineCompletionItems called (Invoke)
[info] Reusing pending inline completion request to avoid race condition
[info] Received response: { sessionId: '...', itemCount: 2, items: [...] }

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

@amazon-inspector-ohio
Copy link

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.
  • This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
    • Note: beta or "experiment" features that have active users should announce fixes in the changelog.
    • If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

@amazon-inspector-ohio
Copy link

✅ I finished the code review, and didn't find any security or code quality issues.

@bywang56
Copy link
Contributor Author

/retryBuilds

@bywang56 bywang56 marked this pull request as ready for review October 13, 2025 15:28
@bywang56 bywang56 requested a review from a team as a code owner October 13, 2025 15:28
@Will-ShaoHua
Copy link
Contributor

I am not able to have a working fix to prevent either automatic or invoke trigger, which is controlled in toolkit. So I implement request deduplication on Q side by tracking pending requests in the inline completion provider:

you mean controled by vscode right?

@Will-ShaoHua
Copy link
Contributor

umm, test failure seems to be not related to this change, try push empty commit to have CI rerun

@rli rli merged commit 663f626 into aws:master Oct 14, 2025
42 of 46 checks passed
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.

4 participants