nes: speculative: allow reusing spec request more than once#311387
nes: speculative: allow reusing spec request more than once#311387
Conversation
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
Enables reusing a speculative NES request more than once by keeping the speculative pending request attached to SpeculativeRequestManager instead of detaching it on first reuse.
Changes:
- Removed
SpeculativeRequestManager.consumePending()(no longer clears_pendingon consumption). - Stopped detaching the speculative pending request when
NextEditProviderreuses it.
Show a summary per file
| File | Description |
|---|---|
| extensions/copilot/src/extension/inlineEdits/node/speculativeRequestManager.ts | Removes the API used to detach (clear) the pending speculative request without cancelling it. |
| extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.ts | Keeps speculative pending requests attached when reused, allowing subsequent reuse attempts. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 2
| // Nice! No need to make another request, we can reuse the result from a pending request. | ||
| if (speculativeRequest) { | ||
| logger.trace(`reusing speculative pending request (opportunityId=${speculativeRequest.opportunityId}, headerRequestId=${speculativeRequest.headerRequestId})`); | ||
| // Detach the speculative — caller is consuming it now. | ||
| this._specManager.consumePending(); | ||
| } else { |
There was a problem hiding this comment.
This change enables reusing a speculative pending request multiple times, but there’s no test exercising the new behavior (e.g. multiple getNextEdit calls in the same post-edit state should still reuse the same speculative request and not create an additional provider call). Please add/extend a Vitest case in nextEditProviderSpeculative.spec.ts to cover reusing the same speculative request more than once.
| // Nice! No need to make another request, we can reuse the result from a pending request. | ||
| if (speculativeRequest) { | ||
| logger.trace(`reusing speculative pending request (opportunityId=${speculativeRequest.opportunityId}, headerRequestId=${speculativeRequest.headerRequestId})`); | ||
| // Detach the speculative — caller is consuming it now. | ||
| this._specManager.consumePending(); | ||
| } else { |
There was a problem hiding this comment.
By no longer clearing _specManager.pending when a speculative request is reused, the pending entry (including postEditContent and trajectory strings) can now live much longer. Since onActiveDocumentChanged is currently not invoked (it’s commented out in the constructor), this pending state may stick around even after the document diverges, increasing retained memory per accepted suggestion. Consider adding an explicit cleanup strategy (e.g. clear pending after N reuses / after the request completes, or re-enable a safe divergence signal) to avoid holding large document snapshots unnecessarily.
Co-authored-by: Copilot copilot@github.com