Skip to content

Commit c849a6d

Browse files
authored
nes: emitFastCursorChange: support additive only emitting (#3285)
* nes: emitFastCursorChange: support additive only emitting Improved isAdditiveEdit to use subsequence matching instead of simple substring containment. This correctly detects additive edits where text is inserted in the middle of a line. Example: 'function fib() {' → 'function fib(n: number) {' is now correctly identified as additive. The algorithm is O(n) with O(1) space - a single pass through the new line checking if all original characters appear in order. * camel case value * fix default
1 parent 3d37026 commit c849a6d

File tree

4 files changed

+152
-8
lines changed

4 files changed

+152
-8
lines changed

src/extension/xtab/node/xtabProvider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,7 @@ export class XtabProvider implements IStatelessNextEditProvider {
764764
}
765765

766766
const diffOptions: ResponseProcessor.DiffParams = {
767-
emitFastCursorLineChange: this.configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsXtabProviderEmitFastCursorLineChange, this.expService),
767+
emitFastCursorLineChange: ResponseProcessor.mapEmitFastCursorLineChange(this.configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsXtabProviderEmitFastCursorLineChange, this.expService)),
768768
nLinesToConverge: this.configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsXtabNNonSignificantLinesToConverge, this.expService),
769769
nSignificantLinesToConverge: this.configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsXtabNSignificantLinesToConverge, this.expService),
770770
};

src/extension/xtab/test/common/responseProcessor.spec.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { expect, suite, test } from 'vitest';
6+
import { describe, expect, it, suite, test } from 'vitest';
77
import { ResponseProcessor } from '../../../../platform/inlineEdits/common/responseProcessor';
88
import { AsyncIterableObject } from '../../../../util/vs/base/common/async';
99
import { LineEdit, LineReplacement } from '../../../../util/vs/editor/common/core/edits/lineEdit';
@@ -351,3 +351,78 @@ suite('stream diffing', () => {
351351

352352
});
353353
});
354+
355+
describe('isAdditiveEdit', () => {
356+
357+
it('should detect simple substring additions as additive', () => {
358+
expect(ResponseProcessor.isAdditiveEdit('hello', 'hello world')).toMatchInlineSnapshot(`true`);
359+
expect(ResponseProcessor.isAdditiveEdit('world', 'hello world')).toMatchInlineSnapshot(`true`);
360+
expect(ResponseProcessor.isAdditiveEdit('hello world', 'hello world')).toMatchInlineSnapshot(`true`);
361+
});
362+
363+
it('should detect insertions in the middle as additive', () => {
364+
// The key case: adding parameters to a function
365+
expect(ResponseProcessor.isAdditiveEdit('function fib() {', 'function fib(n: number) {')).toMatchInlineSnapshot(`true`);
366+
367+
// Adding type annotations
368+
expect(ResponseProcessor.isAdditiveEdit('const x = 5', 'const x: number = 5')).toMatchInlineSnapshot(`true`);
369+
370+
// Adding modifiers
371+
expect(ResponseProcessor.isAdditiveEdit('function foo() {}', 'async function foo() {}')).toMatchInlineSnapshot(`true`);
372+
});
373+
374+
it('should detect character insertions as additive', () => {
375+
expect(ResponseProcessor.isAdditiveEdit('abc', 'aXbYcZ')).toMatchInlineSnapshot(`true`);
376+
expect(ResponseProcessor.isAdditiveEdit('abc', 'XXXaYYYbZZZc')).toMatchInlineSnapshot(`true`);
377+
});
378+
379+
it('should detect deletions as non-additive', () => {
380+
expect(ResponseProcessor.isAdditiveEdit('hello world', 'hello')).toMatchInlineSnapshot(`false`);
381+
expect(ResponseProcessor.isAdditiveEdit('hello world', 'world')).toMatchInlineSnapshot(`false`);
382+
expect(ResponseProcessor.isAdditiveEdit('function fib(n: number) {', 'function fib() {')).toMatchInlineSnapshot(`false`);
383+
});
384+
385+
it('should detect replacements as non-additive', () => {
386+
// Changing name (f → g) is a replacement
387+
expect(ResponseProcessor.isAdditiveEdit('function fib() {', 'function gib() {')).toMatchInlineSnapshot(`false`);
388+
389+
// Changing value is a replacement
390+
expect(ResponseProcessor.isAdditiveEdit('const x = 5', 'const x = 10')).toMatchInlineSnapshot(`false`);
391+
});
392+
393+
it('should handle empty strings', () => {
394+
expect(ResponseProcessor.isAdditiveEdit('', '')).toMatchInlineSnapshot(`true`);
395+
expect(ResponseProcessor.isAdditiveEdit('', 'hello')).toMatchInlineSnapshot(`true`);
396+
expect(ResponseProcessor.isAdditiveEdit('hello', '')).toMatchInlineSnapshot(`false`);
397+
});
398+
399+
it('should handle whitespace changes', () => {
400+
// Adding whitespace
401+
expect(ResponseProcessor.isAdditiveEdit('a b', 'a b')).toMatchInlineSnapshot(`true`);
402+
expect(ResponseProcessor.isAdditiveEdit('ab', 'a b')).toMatchInlineSnapshot(`true`);
403+
404+
// Removing whitespace is not additive
405+
expect(ResponseProcessor.isAdditiveEdit('a b', 'a b')).toMatchInlineSnapshot(`false`);
406+
});
407+
408+
it('should handle complex code transformations', () => {
409+
// Adding optional chaining
410+
expect(ResponseProcessor.isAdditiveEdit('obj.prop', 'obj?.prop')).toMatchInlineSnapshot(`true`);
411+
412+
// Adding template literal
413+
expect(ResponseProcessor.isAdditiveEdit('`hello`', '`hello ${name}`')).toMatchInlineSnapshot(`true`);
414+
415+
// Adding array element
416+
expect(ResponseProcessor.isAdditiveEdit('[1, 2]', '[1, 2, 3]')).toMatchInlineSnapshot(`true`);
417+
418+
// Adding object property (subsequence still works)
419+
expect(ResponseProcessor.isAdditiveEdit('{ a: 1 }', '{ a: 1, b: 2 }')).toMatchInlineSnapshot(`true`);
420+
});
421+
422+
it('should handle repeated characters correctly', () => {
423+
// All 'a's from original must be matched in order
424+
expect(ResponseProcessor.isAdditiveEdit('aaa', 'aaaa')).toMatchInlineSnapshot(`true`);
425+
expect(ResponseProcessor.isAdditiveEdit('aaa', 'aXaYaZ')).toMatchInlineSnapshot(`true`);
426+
expect(ResponseProcessor.isAdditiveEdit('aaaa', 'aaa')).toMatchInlineSnapshot(`false`);
427+
});
428+
});

src/platform/configuration/common/configurationService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -794,7 +794,7 @@ export namespace ConfigKey {
794794
export const InlineEditsNextCursorPredictionRecentSnippetsIncludeLineNumbers = defineTeamInternalSetting<xtabPromptOptions.IncludeLineNumbersOption>('chat.advanced.inlineEdits.nextCursorPrediction.recentSnippets.includeLineNumbers', ConfigType.ExperimentBased, xtabPromptOptions.IncludeLineNumbersOption.None);
795795
export const InlineEditsXtabDiffNEntries = defineTeamInternalSetting<number>('chat.advanced.inlineEdits.xtabProvider.diffNEntries', ConfigType.ExperimentBased, xtabPromptOptions.DEFAULT_OPTIONS.diffHistory.nEntries);
796796
export const InlineEditsXtabDiffMaxTokens = defineTeamInternalSetting<number>('chat.advanced.inlineEdits.xtabProvider.diffMaxTokens', ConfigType.ExperimentBased, xtabPromptOptions.DEFAULT_OPTIONS.diffHistory.maxTokens);
797-
export const InlineEditsXtabProviderEmitFastCursorLineChange = defineTeamInternalSetting<boolean>('chat.advanced.inlineEdits.xtabProvider.emitFastCursorLineChange', ConfigType.ExperimentBased, true);
797+
export const InlineEditsXtabProviderEmitFastCursorLineChange = defineTeamInternalSetting<ResponseProcessor.EmitFastCursorLineChange>('chat.advanced.inlineEdits.xtabProvider.emitFastCursorLineChange', ConfigType.ExperimentBased, ResponseProcessor.EmitFastCursorLineChange.Always);
798798
export const InlineEditsXtabIncludeViewedFiles = defineTeamInternalSetting<boolean>('chat.advanced.inlineEdits.xtabProvider.includeViewedFiles', ConfigType.ExperimentBased, xtabPromptOptions.DEFAULT_OPTIONS.recentlyViewedDocuments.includeViewedFiles);
799799
export const InlineEditsXtabPageSize = defineTeamInternalSetting<number>('chat.advanced.inlineEdits.xtabProvider.pageSize', ConfigType.ExperimentBased, xtabPromptOptions.DEFAULT_OPTIONS.pagedClipping.pageSize);
800800
export const InlineEditsXtabEditWindowMaxTokens = defineTeamInternalSetting<number | undefined>('chat.advanced.inlineEdits.xtabProvider.editWindowMaxTokens', ConfigType.ExperimentBased, 2000);

src/platform/inlineEdits/common/responseProcessor.ts

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,47 @@ import { LineRange } from '../../../util/vs/editor/common/core/ranges/lineRange'
1010

1111
export namespace ResponseProcessor {
1212

13+
/**
14+
* Controls when to emit fast cursor line changes.
15+
* - `off`: Never emit fast cursor line changes
16+
* - `always`: Always emit when the cursor line changes (original behavior)
17+
* - `additiveOnly`: Only emit when the edit on the cursor line is additive (only adds text)
18+
*/
19+
export const enum EmitFastCursorLineChange {
20+
Off = 'off',
21+
Always = 'always',
22+
AdditiveOnly = 'additiveOnly',
23+
}
24+
1325
export type DiffParams = {
1426
/**
15-
* Whether to emit a fast cursor line change event.
16-
* Defaults to false.
27+
* Controls when to emit a fast cursor line change event.
1728
*/
18-
readonly emitFastCursorLineChange: boolean;
29+
readonly emitFastCursorLineChange: EmitFastCursorLineChange;
1930
readonly nSignificantLinesToConverge: number;
2031
readonly nLinesToConverge: number;
2132
};
2233

2334
export const DEFAULT_DIFF_PARAMS: DiffParams = {
24-
emitFastCursorLineChange: false,
35+
emitFastCursorLineChange: EmitFastCursorLineChange.Off,
2536
nSignificantLinesToConverge: 2,
2637
nLinesToConverge: 3,
2738
};
2839

40+
/**
41+
* Maps the `emitFastCursorLineChange` setting value to the new type,
42+
* preserving backward compatibility with the old boolean type.
43+
*/
44+
export function mapEmitFastCursorLineChange(value: boolean | EmitFastCursorLineChange): EmitFastCursorLineChange {
45+
if (value === true) {
46+
return EmitFastCursorLineChange.Always;
47+
}
48+
if (value === false) {
49+
return EmitFastCursorLineChange.Off;
50+
}
51+
return value;
52+
}
53+
2954
type DivergenceState =
3055
| { k: 'aligned' }
3156
| {
@@ -117,6 +142,43 @@ export namespace ResponseProcessor {
117142
return !!s.match(/[a-zA-Z1-9]+/);
118143
}
119144

145+
/**
146+
* Checks if a line edit is additive (only adds text without removing any).
147+
* An edit is additive if the original line is a subsequence of the new line,
148+
* meaning all characters from the original appear in the new line in the same order.
149+
*
150+
* Examples:
151+
* - "function fib() {" → "function fib(n: number) {" ✓ (additive)
152+
* - "hello world" → "hello" ✗ (not additive, removes " world")
153+
* - "abc" → "aXbYcZ" ✓ (additive)
154+
*/
155+
export function isAdditiveEdit(originalLine: string, newLine: string): boolean {
156+
return isSubsequence(originalLine, newLine);
157+
}
158+
159+
/**
160+
* Returns true if `subsequence` is a subsequence of `str`.
161+
* A subsequence means all characters appear in `str` in the same relative order,
162+
* but not necessarily consecutively.
163+
*/
164+
function isSubsequence(subsequence: string, str: string): boolean {
165+
if (subsequence.length === 0) {
166+
return true;
167+
}
168+
if (subsequence.length > str.length) {
169+
return false;
170+
}
171+
172+
let subIdx = 0;
173+
for (let i = 0; i < str.length && subIdx < subsequence.length; i++) {
174+
if (str[i] === subsequence[subIdx]) {
175+
subIdx++;
176+
}
177+
}
178+
179+
return subIdx === subsequence.length;
180+
}
181+
120182
function checkForConvergence(
121183
originalLines: string[],
122184
cursorOriginalLinesOffset: number,
@@ -136,12 +198,19 @@ export namespace ResponseProcessor {
136198
let candidates = lineToIndexes.get(state.newLines[newLinesIdx]).map((idx): [number, number] => [idx, idx]);
137199

138200
if (candidates.length === 0) {
139-
if (!params.emitFastCursorLineChange ||
201+
if (params.emitFastCursorLineChange === EmitFastCursorLineChange.Off ||
140202
editWindowIdx !== cursorOriginalLinesOffset || state.newLines.length > 1
141203
) {
142204
return;
143205
}
144206

207+
// Check if emit is allowed based on the setting
208+
const originalLine = originalLines[editWindowIdx];
209+
const newLine = state.newLines[0];
210+
if (params.emitFastCursorLineChange === EmitFastCursorLineChange.AdditiveOnly && !isAdditiveEdit(originalLine, newLine)) {
211+
return;
212+
}
213+
145214
// we detected that line with the cursor has changed, so we immediately emit an edit for it
146215
const zeroBasedLineRange = [editWindowIdx, editWindowIdx + 1];
147216
const lineRange = new LineRange(zeroBasedLineRange[0] + 1, zeroBasedLineRange[1] + 1);

0 commit comments

Comments
 (0)