Skip to content

Conversation

@sean-mcmanus
Copy link
Contributor

@sean-mcmanus sean-mcmanus commented Dec 3, 2024

Follow up to #12773 .

The previous/abandoned PR also ignored the ServerCancellation requests that occur if a user switches files and makes edits, but I added them back: #12988

@sean-mcmanus sean-mcmanus requested a review from a team as a code owner December 3, 2024 22:49
return result;
}

// This is used to avoid processing unnecessary LSP cancel requests during a Copilot completion
Copy link
Member

Choose a reason for hiding this comment

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

@sean-mcmanus the exact reason is that caching is happening on copilot-client, hence even if the token is cancelled, it means that the return value will NOT be used for the prompt construction, but it will be used for caching purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I know it's not used for the currently running completion (it doesn't block Copilot), but it's cached and used on the next one. Does my comment not explain that? i.e. should I re-word my comment?

Copy link
Member

Choose a reason for hiding this comment

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

I'd omit the 2-minute detail since it is an implementation detail not located in this codebase. Instead, I'd mention that upon cancellation, the result will not be used to build the prompt (since the copilot extension gave up on waiting for a value from this provider), but the returned value is cached by the copilot extension and used on a subsequent call when available, where the cache will be hit and the call to getProjectContext will be saved.

The 2 minutes is the cache entry eviction time defined by the copilot extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I tried to re-word it.

lukka
lukka previously approved these changes Dec 3, 2024
@sean-mcmanus sean-mcmanus deleted the seanmcm/avoidUnnecessaryCancelRetryWithGitHubCopilot2 branch March 27, 2025 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pull Request

Development

Successfully merging this pull request may close these issues.

3 participants