Skip to content

Commit 197f33e

Browse files
authored
NES: consolidate speculative request cancellation with type-through trajectory check (#310969)
nes: consolidate speculative request cancellation 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 originating 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.
1 parent 334c5b7 commit 197f33e

3 files changed

Lines changed: 382 additions & 89 deletions

File tree

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

Lines changed: 80 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import { INesConfigs } from './nesConfigs';
4646
import { CachedOrRebasedEdit, NextEditCache } from './nextEditCache';
4747
import { LlmNESTelemetryBuilder, ReusedRequestKind } from './nextEditProviderTelemetry';
4848
import { INextEditResult, NextEditResult } from './nextEditResult';
49+
import { SpeculativeCancelReason, SpeculativeRequestManager } from './speculativeRequestManager';
4950

5051
/**
5152
* Computes a reduced window range that encompasses both the original window (shrunk by one line
@@ -167,28 +168,7 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
167168

168169
private _pendingStatelessNextEditRequest: StatelessNextEditRequest<CachedOrRebasedEdit> | null = null;
169170

170-
/**
171-
* Tracks a speculative request for the post-edit document state.
172-
* When a suggestion is shown, we speculatively fetch the next edit as if the user had already accepted.
173-
* This allows reusing the in-flight request when the user actually accepts the suggestion.
174-
*/
175-
private _speculativePendingRequest: {
176-
request: StatelessNextEditRequest<CachedOrRebasedEdit>;
177-
docId: DocumentId;
178-
postEditContent: string;
179-
} | null = null;
180-
181-
/**
182-
* A speculative request that is deferred until the originating stream completes.
183-
* When a suggestion is shown while its stream is still running, we schedule the
184-
* speculative request here instead of firing immediately. If more edits arrive
185-
* from the stream, the schedule is cleared (the shown edit wasn't the last one).
186-
* When the stream ends, if the schedule is still present, the speculative fires.
187-
*/
188-
private _scheduledSpeculativeRequest: {
189-
suggestion: NextEditResult;
190-
headerRequestId: string;
191-
} | null = null;
171+
private readonly _specManager: SpeculativeRequestManager;
192172

193173
private _lastShownTime = 0;
194174
/** The requestId of the last shown suggestion. We store only the requestId (not the object) to avoid preventing garbage collection. */
@@ -230,35 +210,48 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
230210

231211
this._logger = this._logService.createSubLogger(['NES', 'NextEditProvider']);
232212
this._nextEditCache = new NextEditCache(this._workspace, this._logService, this._configService, this._expService);
213+
this._specManager = this._register(new SpeculativeRequestManager(this._logger.createSubLogger('SpeculativeRequestManager')));
233214

234215
mapObservableArrayCached(this, this._workspace.openDocuments, (doc, store) => {
235216
store.add(runOnChange(doc.value, (value) => {
236217
this._cancelPendingRequestDueToDocChange(doc.id, value);
218+
// FIXME: don't invoke before fixing false positive cancellations
219+
// this._specManager.onActiveDocumentChanged(doc.id, value.value);
237220
}));
221+
// When the per-doc store is disposed, the document was removed from
222+
// openDocuments. Cancel any speculative targeting it — its cached result
223+
// would never be hit again.
224+
store.add(toDisposable(() => this._specManager.onDocumentClosed(doc.id)));
238225
}).recomputeInitiallyAndOnChange(this._store);
239226
}
240227

241-
private _cancelSpeculativeRequest(): void {
242-
this._scheduledSpeculativeRequest = null;
243-
if (this._speculativePendingRequest) {
244-
this._speculativePendingRequest.request.cancellationTokenSource.cancel();
245-
this._speculativePendingRequest = null;
246-
}
247-
}
248-
228+
/**
229+
* Cancels the in-flight stateless next-edit request when the document it
230+
* was issued for has diverged from the request's expected post-edit state.
231+
*
232+
* Invoked from the per-document `runOnChange` autorun in the constructor
233+
* whenever an open document's value changes. The pending request was built
234+
* against a specific snapshot (`documentAfterEdits`); if the user has since
235+
* typed something that makes the current value differ from that snapshot,
236+
* the result would no longer be applicable and is cancelled eagerly.
237+
*
238+
* Skipped when:
239+
* - the `InlineEditsAsyncCompletions` experiment is enabled (that path
240+
* tolerates divergence and rebases later), or
241+
* - there is no pending request, or
242+
* - the changed document is not the one the pending request targets.
243+
*
244+
* Note: this only handles the regular pending stateless request. Speculative
245+
* requests have their own divergence handling via
246+
* `SpeculativeRequestManager.onActiveDocumentChanged` (trajectory check).
247+
*/
249248
private _cancelPendingRequestDueToDocChange(docId: DocumentId, docValue: StringText) {
250-
// Note: we intentionally do NOT cancel the speculative request here.
251-
// The speculative request's postEditContent represents a *future* document state
252-
// (after the user would accept the suggestion), so it will almost never match the
253-
// current document value while the user is still typing. Cancelling here would
254-
// wastefully kill and recreate the speculative request on every keystroke.
255-
// Instead, speculative requests are cancelled by the appropriate lifecycle handlers:
256-
// handleRejection, handleIgnored, _triggerSpeculativeRequest, and _executeNewNextEditRequest.
257-
258249
const isAsyncCompletions = this._configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsAsyncCompletions, this._expService);
250+
259251
if (isAsyncCompletions || this._pendingStatelessNextEditRequest === null) {
260252
return;
261253
}
254+
262255
const activeDoc = this._pendingStatelessNextEditRequest.getActiveDocument();
263256
if (activeDoc.id === docId && activeDoc.documentAfterEdits.value !== docValue.value) {
264257
this._pendingStatelessNextEditRequest.cancellationTokenSource.cancel();
@@ -546,11 +539,12 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
546539
&& this._pendingStatelessNextEditRequest || undefined;
547540

548541
// Check if we can reuse the speculative pending request (from when a suggestion was shown)
549-
const speculativeRequestMatches = this._speculativePendingRequest?.docId === curDocId
550-
&& this._speculativePendingRequest?.postEditContent === documentAtInvocationTime.value
551-
&& !this._speculativePendingRequest.request.cancellationTokenSource.token.isCancellationRequested
552-
&& cursorInRequestEditWindow(this._speculativePendingRequest.request);
553-
const speculativeRequest = speculativeRequestMatches ? this._speculativePendingRequest?.request : undefined;
542+
const specPending = this._specManager.pending;
543+
const speculativeRequestMatches = specPending?.docId === curDocId
544+
&& specPending?.postEditContent === documentAtInvocationTime.value
545+
&& !specPending.request.cancellationTokenSource.token.isCancellationRequested
546+
&& cursorInRequestEditWindow(specPending.request);
547+
const speculativeRequest = speculativeRequestMatches ? specPending?.request : undefined;
554548

555549
// Prefer speculative request if it matches (it was specifically created for this post-edit state)
556550
const requestToReuse = speculativeRequest ?? existingNextEditRequest;
@@ -559,8 +553,8 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
559553
// Nice! No need to make another request, we can reuse the result from a pending request.
560554
if (speculativeRequest) {
561555
logger.trace(`reusing speculative pending request (opportunityId=${speculativeRequest.opportunityId}, headerRequestId=${speculativeRequest.headerRequestId})`);
562-
// Clear the speculative request since we're using it
563-
this._speculativePendingRequest = null;
556+
// Detach the speculative — caller is consuming it now.
557+
this._specManager.consumePending();
564558
} else {
565559
logger.trace(`reusing in-flight pending request (opportunityId=${requestToReuse.opportunityId}, headerRequestId=${requestToReuse.headerRequestId})`);
566560
}
@@ -741,17 +735,12 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
741735
// Clear any scheduled (but not yet triggered) speculative request tied to the
742736
// old stream — it would otherwise fire stale when the old stream's background
743737
// loop calls handleStreamEnd after the stream has already been superseded.
744-
this._scheduledSpeculativeRequest = null;
738+
this._specManager.clearScheduled();
745739
}
746740

747741
// Cancel speculative request if it doesn't match the document/state
748742
// of this new request — it was built for a different document or post-edit state.
749-
if (this._speculativePendingRequest
750-
&& (this._speculativePendingRequest.docId !== curDocId
751-
|| this._speculativePendingRequest.postEditContent !== nextEditRequest.documentBeforeEdits.value)
752-
) {
753-
this._cancelSpeculativeRequest();
754-
}
743+
this._specManager.cancelIfMismatch(curDocId, nextEditRequest.documentBeforeEdits.value, SpeculativeCancelReason.Superseded);
755744

756745
this._pendingStatelessNextEditRequest = nextEditRequest;
757746

@@ -888,9 +877,8 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
888877

889878
// Fire any scheduled speculative request — the last shown edit
890879
// was indeed the last edit from this stream.
891-
if (this._scheduledSpeculativeRequest?.headerRequestId === nextEditRequest.headerRequestId) {
892-
const scheduled = this._scheduledSpeculativeRequest;
893-
this._scheduledSpeculativeRequest = null;
880+
const scheduled = this._specManager.consumeScheduled(nextEditRequest.headerRequestId);
881+
if (scheduled) {
894882
void this._triggerSpeculativeRequest(scheduled.suggestion);
895883
}
896884

@@ -920,9 +908,7 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
920908

921909
// A new edit arrived from the stream — the previously-shown
922910
// edit was not the last one. Clear the scheduled speculative.
923-
if (this._scheduledSpeculativeRequest?.headerRequestId === nextEditRequest.headerRequestId) {
924-
this._scheduledSpeculativeRequest = null;
925-
}
911+
this._specManager.consumeScheduled(nextEditRequest.headerRequestId);
926912

927913
res = await editStream.next();
928914
}
@@ -1030,7 +1016,7 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
10301016
this._lastShownTime = Date.now();
10311017
this._lastShownSuggestionId = suggestion.requestId;
10321018
this._lastOutcome = undefined; // clear so that outcome is "pending" until resolved
1033-
this._scheduledSpeculativeRequest = null; // clear any previously scheduled speculative
1019+
this._specManager.clearScheduled(); // clear any previously scheduled speculative
10341020

10351021
// Trigger speculative request for the post-edit document state
10361022
const speculativeRequestsEnablement = this._configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsSpeculativeRequests, this._expService);
@@ -1041,10 +1027,10 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
10411027
// request only fires when the stream ends with the shown edit as the last one.
10421028
const originatingRequest = this._pendingStatelessNextEditRequest;
10431029
if (originatingRequest && originatingRequest.headerRequestId === suggestion.source.headerRequestId) {
1044-
this._scheduledSpeculativeRequest = {
1030+
this._specManager.schedule({
10451031
suggestion,
10461032
headerRequestId: originatingRequest.headerRequestId,
1047-
};
1033+
});
10481034
} else {
10491035
void this._triggerSpeculativeRequest(suggestion);
10501036
}
@@ -1115,7 +1101,8 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
11151101
}
11161102

11171103
// Check if we already have a speculative request for this post-edit state
1118-
if (this._speculativePendingRequest?.docId === docId && this._speculativePendingRequest?.postEditContent === postEditContent) {
1104+
const existingSpec = this._specManager.pending;
1105+
if (existingSpec?.docId === docId && existingSpec?.postEditContent === postEditContent) {
11191106
logger.trace('already have speculative request for post-edit state');
11201107
return;
11211108
}
@@ -1129,8 +1116,11 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
11291116
return;
11301117
}
11311118

1132-
// Cancel any previous speculative request
1133-
this._cancelSpeculativeRequest();
1119+
// Note: any previous speculative request will be cancelled (as `Replaced`)
1120+
// by `_specManager.setPending` once the new request is actually installed —
1121+
// see the `setPending` call at the end of this method. We deliberately do
1122+
// not cancel earlier so the prior speculative stays available for reuse
1123+
// while the new one is being constructed.
11341124

11351125
const historyContext = this._historyContextProvider.getHistoryContext(docId);
11361126
if (!historyContext) {
@@ -1159,11 +1149,24 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
11591149
);
11601150

11611151
if (speculativeRequest) {
1162-
this._speculativePendingRequest = {
1152+
// Capture trajectory data: while the user is typing in `docId`, the
1153+
// document is on a "type-through" trajectory iff:
1154+
// doc = preEdit[0..editStart] + newText[0..k] + preEdit[editEnd..]
1155+
// for some 0 <= k <= newText.length. Storing the prefix/suffix/newText
1156+
// (already-CRLF-normalized via `result.edit.newText` whose newlines
1157+
// match the original document) lets us check this in O(|cur|) on doc changes.
1158+
const preEditValue = result.documentBeforeEdits.value;
1159+
const trajectoryPrefix = preEditValue.slice(0, preciseEdit.replaceRange.start);
1160+
const trajectorySuffix = preEditValue.slice(preciseEdit.replaceRange.endExclusive);
1161+
const trajectoryNewText = preciseEdit.newText;
1162+
this._specManager.setPending({
11631163
request: speculativeRequest,
11641164
docId,
11651165
postEditContent,
1166-
};
1166+
trajectoryPrefix,
1167+
trajectorySuffix,
1168+
trajectoryNewText,
1169+
});
11671170
}
11681171
} catch (e) {
11691172
logger.trace(`speculative request failed: ${ErrorUtils.toString(e)}`);
@@ -1445,7 +1448,7 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
14451448
// The user rejected the suggestion, so the speculative request (which
14461449
// predicted the post-accept state) will never be reused. Cancel it to
14471450
// avoid wasting a server slot.
1448-
this._cancelSpeculativeRequest();
1451+
this._specManager.cancelAll(SpeculativeCancelReason.Rejected);
14491452

14501453
const shownDuration = Date.now() - this._lastShownTime;
14511454
if (shownDuration > 1000 && suggestion.result.edit) {
@@ -1470,9 +1473,15 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
14701473
if (wasShown && !wasSuperseded) {
14711474
// The shown suggestion was dismissed (not superseded by a new one),
14721475
// so the speculative request for its post-accept state is useless.
1473-
this._cancelSpeculativeRequest();
1476+
this._specManager.cancelAll(SpeculativeCancelReason.IgnoredDismissed);
14741477
this._statelessNextEditProvider.handleIgnored?.();
14751478
}
1479+
// Note: the superseded case is intentionally NOT handled here. The trajectory
1480+
// check on `_specManager.onActiveDocumentChanged` already cancels the
1481+
// speculative iff the user's edit moved off the type-through trajectory; if
1482+
// the new (superseding) suggestion is just a continuation of the old one
1483+
// (e.g. typed `i` while `ibonacci` was shown → now `bonacci` is shown), the
1484+
// speculative's `postEditContent` is still the right bet and we keep it.
14761485
}
14771486

14781487
private async runSnippy(docId: DocumentId, suggestion: NextEditResult) {
@@ -1497,6 +1506,9 @@ export class NextEditProvider extends Disposable implements INextEditProvider<Ne
14971506
public clearCache() {
14981507
this._nextEditCache.clear();
14991508
this._rejectionCollector.clear();
1509+
// Any in-flight speculative would land its result into a cache that's
1510+
// meant to be empty (and may be based on a now-stale model/auth/prompt).
1511+
this._specManager.cancelAll(SpeculativeCancelReason.CacheCleared);
15001512
}
15011513
}
15021514

0 commit comments

Comments
 (0)