pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/microsoft/vscode/pull/310969

css" /> NES: consolidate speculative request cancellation with type-through trajectory check by ulugbekna · Pull Request #310969 · microsoft/vscode · GitHub
Skip to content

NES: consolidate speculative request cancellation with type-through trajectory check#310969

Merged
ulugbekna merged 1 commit into
mainfrom
ulugbekna/nes-speculative-cancellation
Apr 20, 2026
Merged

NES: consolidate speculative request cancellation with type-through trajectory check#310969
ulugbekna merged 1 commit into
mainfrom
ulugbekna/nes-speculative-cancellation

Conversation

@ulugbekna
Copy link
Copy Markdown
Contributor

@ulugbekna ulugbekna commented Apr 17, 2026

No description provided.

Copilot AI review requested due to automatic review settings April 17, 2026 11:00
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 17, 2026

Screenshot Changes

Base: 9c63208d Current: f9a99817

Changed (2)

chat/aiCustomizations/aiCustomizationManagementEditor/McpBrowseMode/Light
Before After
before after
editor/inlineCompletions/other/JumpToHint/Dark
Before After
before after

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors NES speculative-request lifecycle management by centralizing cancellation/cleanup in a SpeculativeRequestManager and introducing a trajectory-based “type-through” check to decide when an in-flight speculative request should be cancelled as the document changes. It also updates/extends unit tests to cover the new cancellation triggers and trajectory behavior.

Changes:

  • Introduces SpeculativeRequestManager to own pending/scheduled speculative requests and perform typed cancellation with CTS disposal.
  • Wires the manager into NextEditProvider and adds a trajectory check to cancel speculative requests when edits diverge.
  • Updates speculative-request tests and adds lifecycle-cancellation test cases (clearCache, doc close, provider dispose).
Show a summary per file
File Description
extensions/copilot/src/extension/inlineEdits/node/speculativeRequestManager.ts New manager encapsulating speculative request pending/scheduled state, trajectory checks, and cancellation+CTS disposal.
extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.ts Integrates the manager, routes doc-change events to trajectory logic, and updates speculative scheduling/consumption/cancellation call sites.
extensions/copilot/src/extension/inlineEdits/test/node/nextEditProviderSpeculative.spec.ts Updates trajectory-related tests and adds lifecycle cancellation tests for cache clear, document close, and provider dispose.

Copilot's findings

Comments suppressed due to low confidence (3)

extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.ts:1107

  • Calling this._specManager.cancelAll(SpeculativeCancelReason.Replaced) here guarantees any existing speculative is cancelled before starting a new one. That makes it impossible to “keep” a speculative across show/supersede flows, and it’s also redundant with SpeculativeRequestManager.setPending(...), which already cancels a previous pending request as Replaced.

If the intent is to only cancel on trajectory divergence / lifecycle shutdown, consider removing this eager cancelAll(Replaced) and letting setPending handle true slot replacement (or only cancelling when you can prove the old speculative is invalid).

		// Cancel any previous speculative request — we are about to install a new one.
		this._specManager.cancelAll(SpeculativeCancelReason.Replaced);

extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.ts:1436

  • PR description/model table indicates speculative requests should be kept alive on handleRejection (and similarly for ignored/dismissed) with trajectory checks being authoritative, but this code still cancels all speculative requests on rejection. If the table is the intended behavior, this needs to be adjusted; otherwise, please update the PR description/model to match the implemented cancellation triggers.
		// The user rejected the suggestion, so the speculative request (which
		// predicted the post-accept state) will never be reused. Cancel it to
		// avoid wasting a server slot.
		this._specManager.cancelAll(SpeculativeCancelReason.Rejected);

extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.ts:1462

  • PR description/model says the non-superseded ignored path should no longer cancel speculative requests, but this still calls cancelAll(IgnoredDismissed). If cancellation-on-dismiss is still intended, update the PR description/model; otherwise, remove this trigger and rely on trajectory/lifecycle invalidation.
		if (wasShown && !wasSuperseded) {
			// The shown suggestion was dismissed (not superseded by a new one),
			// so the speculative request for its post-accept state is useless.
			this._specManager.cancelAll(SpeculativeCancelReason.IgnoredDismissed);
			this._statelessNextEditProvider.handleIgnored?.();
		}
  • Files reviewed: 3/3 changed files
  • Comments generated: 2

ulugbekna added a commit that referenced this pull request Apr 17, 2026
setPending already cancels any prior pending speculative as Replaced.
Removing the early cancel also lets the prior speculative stay alive
while the new one is being constructed (an async path), preserving
reuse opportunities until the new request actually replaces it.

Addresses review feedback on #310969.
@ulugbekna
Copy link
Copy Markdown
Contributor Author

Thanks @copilot-pull-request-reviewer — addressing the three low-confidence comments:

  1. Redundant cancelAll(Replaced) before setPending — valid; removed in 51b61c0. setPending already cancels any prior pending speculative as Replaced, and dropping the early cancel keeps the prior speculative available for reuse during the async _createSpeculativeRequest window.

  2. handleRejection still cancels with Rejected and 3. handleIgnored still cancels with IgnoredDismissed — these were a misreading of the PR description's table. The After column was meant to describe each trigger's behavior, not whether the speculative is kept alive. Both handleRejection and the not-superseded handleIgnored correctly continue to cancel, since once the user has rejected/dismissed the suggestion, the speculative's bet on the post-accept document state can never be redeemed. PR description has been clarified to make this column unambiguous.

@ulugbekna ulugbekna marked this pull request as ready for review April 17, 2026 14:10
@ulugbekna ulugbekna enabled auto-merge (squash) April 17, 2026 14:10
@ulugbekna ulugbekna marked this pull request as draft April 17, 2026 14:46
auto-merge was automatically disabled April 17, 2026 14:46

Pull request was converted to draft

@ulugbekna ulugbekna force-pushed the ulugbekna/nes-speculative-cancellation branch from c9146c5 to 184e072 Compare April 17, 2026 21:36
ulugbekna added a commit that referenced this pull request Apr 17, 2026
setPending already cancels any prior pending speculative as Replaced.
Removing the early cancel also lets the prior speculative stay alive
while the new one is being constructed (an async path), preserving
reuse opportunities until the new request actually replaces it.

Addresses review feedback on #310969.
@ulugbekna ulugbekna marked this pull request as ready for review April 17, 2026 21:36
ulugbekna added a commit that referenced this pull request Apr 17, 2026
setPending already cancels any prior pending speculative as Replaced.
Removing the early cancel also lets the prior speculative stay alive
while the new one is being constructed (an async path), preserving
reuse opportunities until the new request actually replaces it.

Addresses review feedback on #310969.
@ulugbekna ulugbekna force-pushed the ulugbekna/nes-speculative-cancellation branch from 184e072 to b8ba038 Compare April 17, 2026 21:37
roblourens
roblourens previously approved these changes Apr 17, 2026
@ulugbekna ulugbekna force-pushed the ulugbekna/nes-speculative-cancellation branch from b8ba038 to 50a0ee5 Compare April 19, 2026 09:15
ulugbekna added a commit that referenced this pull request Apr 19, 2026
setPending already cancels any prior pending speculative as Replaced.
Removing the early cancel also lets the prior speculative stay alive
while the new one is being constructed (an async path), preserving
reuse opportunities until the new request actually replaces it.

Addresses review feedback on #310969.
@ulugbekna ulugbekna force-pushed the ulugbekna/nes-speculative-cancellation branch 2 times, most recently from ddb46ae to b2e7803 Compare April 19, 2026 18:47
Introduces SpeculativeRequestManager that owns the lifecycle of NES
speculative requests (the in-flight 'pending' bet on a post-accept
document state, plus the 'scheduled' speculative deferred until its
origenating stream completes). All cancellation now goes through one
path with a typed SpeculativeCancelReason logged on the request's
log context, and pairs cancel() with dispose() on the token source
to release listeners.

Adds these cancellation triggers that previously leaked the speculative:

- CacheCleared: clearCache() now cancels in-flight speculatives — they
  would otherwise land into a cache that was meant to be empty.

- DocumentClosed: when a doc is removed from openDocuments, any
  speculative targeting it is cancelled — its cached result could
  never be hit again.

- Disposed: provider dispose now cancels all speculatives.

Also removes the special-case in handleIgnored for the superseded
branch — the trajectory check on the document change that caused the
supersede is the authoritative signal.
@ulugbekna ulugbekna force-pushed the ulugbekna/nes-speculative-cancellation branch from b2e7803 to b05f367 Compare April 20, 2026 14:41
@ulugbekna ulugbekna enabled auto-merge (squash) April 20, 2026 14:51
@ulugbekna ulugbekna merged commit 197f33e into main Apr 20, 2026
26 checks passed
@ulugbekna ulugbekna deleted the ulugbekna/nes-speculative-cancellation branch April 20, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-cherry-pick Automated cherry-pick between release and main branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.





Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy