Skip to content

Commit f56d2b0

Browse files
authored
fix: improvements to next edit (#8129)
* refactor: autocomplete feedback * fix: don't hardcode outcome to false on aborted completion * fix: correctly parse out backticks * fix: correctly parse out backticks * fix: next edit diff formatting * fix: reduce context lines * improv: track profile type for autocomplete * fix: lint err * fix: test
1 parent 21ed2ad commit f56d2b0

File tree

16 files changed

+286
-154
lines changed

16 files changed

+286
-154
lines changed

core/autocomplete/CompletionProvider.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,8 @@ export class CompletionProvider {
268268
gitRepo: await this.ide.getRepoName(helper.filepath),
269269
uniqueId: await this.ide.getUniqueId(),
270270
timestamp: new Date().toISOString(),
271+
profileType:
272+
this.configHandler.currentProfile?.profileDescription.profileType,
271273
...helper.options,
272274
};
273275

core/autocomplete/postprocessing/index.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ function isBlank(completion: string): boolean {
5454

5555
/**
5656
* Removes markdown code block delimiters from completion.
57-
* Removes the first line if it starts with "```" and the last line if it is exactly "```".
57+
* Removes the first line if it starts with backticks (with optional language name).
58+
* Removes the last line if it contains only backticks.
5859
*/
5960
function removeBackticks(completion: string): string {
6061
const lines = completion.split("\n");
@@ -66,14 +67,18 @@ function removeBackticks(completion: string): string {
6667
let startIdx = 0;
6768
let endIdx = lines.length;
6869

69-
// Remove first line if it starts with ```
70-
if (lines[0].trimStart().startsWith("```")) {
70+
// Remove first line if it starts with backticks (``` or ```language)
71+
const firstLineTrimmed = lines[0].trim();
72+
if (firstLineTrimmed.startsWith("```")) {
7173
startIdx = 1;
7274
}
7375

74-
// Remove last line if it is exactly ```
75-
if (lines.length > startIdx && lines[lines.length - 1].trim() === "```") {
76-
endIdx = lines.length - 1;
76+
// Remove last line if it contains only backticks (one or more)
77+
if (lines.length > startIdx) {
78+
const lastLineTrimmed = lines[lines.length - 1].trim();
79+
if (lastLineTrimmed.length > 0 && /^`+$/.test(lastLineTrimmed)) {
80+
endIdx = lines.length - 1;
81+
}
7782
}
7883

7984
// If we removed lines, return the modified completion

core/autocomplete/util/AutocompleteLoggingService.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ export class AutocompleteLoggingService {
119119
time: restOfOutcome.time,
120120
useRecentlyEdited: restOfOutcome.useRecentlyEdited,
121121
numLines: restOfOutcome.numLines,
122+
profileType: restOfOutcome.profileType,
122123
};
123124

124125
outcome.enabledStaticContextualization

core/autocomplete/util/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,5 @@ export interface AutocompleteOutcome extends TabAutocompleteOptions {
4343
uniqueId: string;
4444
timestamp: string;
4545
enabledStaticContextualization?: boolean;
46+
profileType?: "local" | "platform" | "control-plane";
4647
}

core/config/ConfigHandler.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,12 +537,19 @@ export class ConfigHandler {
537537
// Track config loading telemetry
538538
const endTime = performance.now();
539539
const duration = endTime - startTime;
540-
void Telemetry.capture("config_reload", {
540+
541+
const profileDescription = this.currentProfile.profileDescription;
542+
const telemetryData: Record<string, any> = {
541543
duration,
542544
reason,
543545
totalConfigLoads: this.totalConfigReloads,
544546
configLoadInterrupted,
545-
});
547+
profileType: profileDescription.profileType,
548+
isPersonalOrg: this.currentOrg?.id === this.PERSONAL_ORG_DESC.id,
549+
errorCount: errors.length,
550+
};
551+
552+
void Telemetry.capture("config_reload", telemetryData);
546553

547554
return {
548555
config,

core/nextEdit/NextEditLoggingService.ts

Lines changed: 8 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,6 @@ export class NextEditLoggingService {
9595
outcome.accepted = true;
9696
outcome.aborted = false;
9797
this.logNextEditOutcome(outcome);
98-
if (outcome.requestId) {
99-
void this.logAcceptReject(outcome.requestId, true);
100-
}
10198
this._outcomes.delete(completionId);
10299
return outcome;
103100
}
@@ -116,9 +113,6 @@ export class NextEditLoggingService {
116113
outcome.accepted = false;
117114
outcome.aborted = false;
118115
this.logNextEditOutcome(outcome);
119-
if (outcome.requestId) {
120-
void this.logAcceptReject(outcome.requestId, false);
121-
}
122116
this._outcomes.delete(completionId);
123117
return outcome;
124118
}
@@ -151,9 +145,6 @@ export class NextEditLoggingService {
151145
outcome.accepted = false;
152146
outcome.aborted = false;
153147
this.logNextEditOutcome(outcome);
154-
if (outcome.requestId) {
155-
void this.logAcceptReject(outcome.requestId, false);
156-
}
157148
this._logRejectionTimeouts.delete(completionId);
158149
this._outcomes.delete(completionId);
159150
}, COUNT_COMPLETION_REJECTED_AFTER);
@@ -197,44 +188,15 @@ export class NextEditLoggingService {
197188
this._logRejectionTimeouts.delete(completionId);
198189
}
199190

200-
// If we have the full outcome, log it as aborted.
191+
// Only log if the completion was displayed to the user.
192+
// This aligns with Autocomplete behavior and prevents logging
193+
// of cancelled requests that never reached the user.
201194
if (this._outcomes.has(completionId)) {
202195
const outcome = this._outcomes.get(completionId)!;
203-
outcome.accepted = false;
196+
// outcome.accepted = false;
204197
outcome.aborted = true;
205198
this.logNextEditOutcome(outcome);
206199
this._outcomes.delete(completionId);
207-
} else {
208-
// Log minimal abort event for requests that never got displayed.
209-
const pendingData = this._pendingCompletions.get(completionId);
210-
211-
const minimalAbortOutcome: Partial<NextEditOutcome> = {
212-
completionId,
213-
accepted: false,
214-
aborted: true,
215-
timestamp: Date.now(),
216-
uniqueId: completionId,
217-
elapsed: pendingData ? Date.now() - pendingData.startTime : 0,
218-
modelName: pendingData?.modelName || "unknown",
219-
modelProvider: pendingData?.modelProvider || "unknown",
220-
fileUri: pendingData?.filepath || "unknown",
221-
222-
// Empty/default values for fields we don't have.
223-
completion: "",
224-
prompt: "",
225-
userEdits: "",
226-
userExcerpts: "",
227-
originalEditableRange: "",
228-
workspaceDirUri: "",
229-
cursorPosition: { line: -1, character: -1 },
230-
finalCursorPosition: { line: -1, character: -1 },
231-
editableRegionStartLine: -1,
232-
editableRegionEndLine: -1,
233-
diffLines: [],
234-
completionOptions: {},
235-
};
236-
237-
this.logNextEditOutcome(minimalAbortOutcome as NextEditOutcome);
238200
}
239201

240202
// Clean up.
@@ -255,6 +217,9 @@ export class NextEditLoggingService {
255217
});
256218

257219
// const { prompt, completion, prefix, suffix, ...restOfOutcome } = outcome;
220+
if (outcome.requestId && outcome.accepted !== undefined) {
221+
void this.logAcceptReject(outcome.requestId, outcome.accepted);
222+
}
258223
void Telemetry.capture("nextEditOutcome", outcome, true);
259224
}
260225

@@ -268,7 +233,7 @@ export class NextEditLoggingService {
268233
}
269234

270235
const controlPlaneEnv = getControlPlaneEnvSync("production");
271-
await fetchwithRequestOptions(
236+
const resp = await fetchwithRequestOptions(
272237
new URL("model-proxy/v1/feedback", controlPlaneEnv.CONTROL_PLANE_URL),
273238
{
274239
method: "POST",

core/nextEdit/NextEditPrefetchQueue.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ export class PrefetchQueue {
140140
const count = Math.min(3, this.processedQueue.length);
141141
const firstThree = this.processedQueue.slice(0, count);
142142
firstThree.forEach((item, index) => {
143-
console.log(
143+
console.debug(
144144
`Item ${index + 1}: ${item.location.range.start.line} to ${item.location.range.end.line}`,
145145
);
146146
});

core/nextEdit/NextEditProvider.ts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { ContextRetrievalService } from "../autocomplete/context/ContextRetrieva
88

99
import { BracketMatchingService } from "../autocomplete/filtering/BracketMatchingService.js";
1010
import { CompletionStreamer } from "../autocomplete/generation/CompletionStreamer.js";
11+
import { postprocessCompletion } from "../autocomplete/postprocessing/index.js";
1112
import { shouldPrefilter } from "../autocomplete/prefiltering/index.js";
1213
import { getAllSnippetsWithoutRace } from "../autocomplete/snippets/index.js";
1314
import { AutocompleteCodeSnippet } from "../autocomplete/snippets/types.js";
@@ -415,6 +416,7 @@ export class NextEditProvider {
415416
filePath: helper.filepath,
416417
diffType: DiffFormatType.Unified,
417418
contextLines: 3,
419+
workspaceDir: workspaceDirs[0], // Use first workspace directory
418420
}),
419421
};
420422

@@ -468,33 +470,53 @@ export class NextEditProvider {
468470
}
469471

470472
// Extract completion using model-specific logic.
471-
const nextCompletion = this.modelProvider.extractCompletion(msg.content);
473+
let nextCompletion = this.modelProvider.extractCompletion(msg.content);
474+
475+
// Postprocess the completion (same as autocomplete).
476+
const postprocessed = postprocessCompletion({
477+
completion: nextCompletion,
478+
llm,
479+
prefix: helper.prunedPrefix,
480+
suffix: helper.prunedSuffix,
481+
});
482+
483+
// Return early if postprocessing filtered out the completion.
484+
if (!postprocessed) {
485+
return undefined;
486+
}
487+
488+
nextCompletion = postprocessed;
472489

473490
let outcome: NextEditOutcome | undefined;
474491

475492
// Handle based on diff type.
493+
const profileType =
494+
this.configHandler.currentProfile?.profileDescription.profileType;
495+
476496
if (opts?.usingFullFileDiff === false || !opts?.usingFullFileDiff) {
477-
outcome = await this.modelProvider.handlePartialFileDiff(
497+
outcome = await this.modelProvider.handlePartialFileDiff({
478498
helper,
479499
editableRegionStartLine,
480500
editableRegionEndLine,
481501
startTime,
482502
llm,
483503
nextCompletion,
484-
this.promptMetadata!,
485-
this.ide,
486-
);
504+
promptMetadata: this.promptMetadata!,
505+
ide: this.ide,
506+
profileType,
507+
});
487508
} else {
488-
outcome = await this.modelProvider.handleFullFileDiff(
509+
outcome = await this.modelProvider.handleFullFileDiff({
489510
helper,
490511
editableRegionStartLine,
491512
editableRegionEndLine,
492513
startTime,
493514
llm,
494515
nextCompletion,
495-
this.promptMetadata!,
496-
this.ide,
497-
);
516+
promptMetadata: this.promptMetadata!,
517+
ide: this.ide,
518+
profileType,
519+
});
498520
}
499521

500522
if (outcome) {

core/nextEdit/context/diffFormatting.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createPatch } from "diff";
2+
import { getUriPathBasename } from "../../util/uri";
23

34
export enum DiffFormatType {
45
Unified = "unified",
@@ -18,6 +19,7 @@ export interface CreateDiffArgs {
1819
filePath: string;
1920
diffType: DiffFormatType;
2021
contextLines: number;
22+
workspaceDir?: string;
2123
}
2224

2325
export const createDiff = ({
@@ -26,6 +28,7 @@ export const createDiff = ({
2628
filePath,
2729
diffType,
2830
contextLines,
31+
workspaceDir,
2932
}: CreateDiffArgs) => {
3033
switch (diffType) {
3134
case DiffFormatType.Unified:
@@ -34,6 +37,7 @@ export const createDiff = ({
3437
afterContent,
3538
filePath,
3639
contextLines,
40+
workspaceDir,
3741
);
3842
case DiffFormatType.TokenLineDiff:
3943
return createTokenLineDiff(beforeContent, afterContent, filePath);
@@ -46,6 +50,7 @@ const createUnifiedDiff = (
4650
afterContent: string,
4751
filePath: string,
4852
contextLines: number,
53+
workspaceDir?: string,
4954
) => {
5055
const normalizedBefore = beforeContent.endsWith("\n")
5156
? beforeContent
@@ -54,12 +59,21 @@ const createUnifiedDiff = (
5459
? afterContent
5560
: afterContent + "\n";
5661

62+
// Use relative path if workspace directory is provided
63+
let displayPath = filePath;
64+
if (workspaceDir && filePath.startsWith(workspaceDir)) {
65+
displayPath = filePath.slice(workspaceDir.length).replace(/^[\/]/, "");
66+
} else if (workspaceDir) {
67+
// Fallback to just the basename if we can't determine relative path
68+
displayPath = getUriPathBasename(filePath);
69+
}
70+
5771
const patch = createPatch(
58-
filePath,
72+
displayPath,
5973
normalizedBefore,
6074
normalizedAfter,
61-
"before",
62-
"after",
75+
undefined,
76+
undefined,
6377
{ context: contextLines },
6478
);
6579

0 commit comments

Comments
 (0)