From 2ee31a48c6df8277ac1c250b82d3192307858a33 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Wed, 15 Apr 2026 11:59:58 +0200 Subject: [PATCH 1/4] nes: cache: add a test to show how rebasing fails --- .../test/node/nextEditCacheRebase.spec.ts | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts diff --git a/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts b/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts new file mode 100644 index 0000000000000..2d710ee53ab24 --- /dev/null +++ b/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts @@ -0,0 +1,179 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +import { assert, beforeEach, describe, it } from 'vitest'; +import { DefaultsOnlyConfigurationService } from '../../../../platform/configuration/common/defaultsOnlyConfigurationService'; +import { InMemoryConfigurationService } from '../../../../platform/configuration/test/common/inMemoryConfigurationService'; +import { DocumentId } from '../../../../platform/inlineEdits/common/dataTypes/documentId'; +import { InlineEditRequestLogContext } from '../../../../platform/inlineEdits/common/inlineEditLogContext'; +import { MutableObservableWorkspace } from '../../../../platform/inlineEdits/common/observableWorkspace'; +import { LogServiceImpl } from '../../../../platform/log/common/logService'; +import { NullExperimentationService } from '../../../../platform/telemetry/common/nullExperimentationService'; +import { URI } from '../../../../util/vs/base/common/uri'; +import { generateUuid } from '../../../../util/vs/base/common/uuid'; +import { StringEdit, StringReplacement } from '../../../../util/vs/editor/common/core/edits/stringEdit'; +import { OffsetRange } from '../../../../util/vs/editor/common/core/ranges/offsetRange'; +import { StringText } from '../../../../util/vs/editor/common/core/text/abstractText'; +import { NextEditCache } from '../../node/nextEditCache'; +import { NextEditFetchRequest } from '../../node/nextEditProvider'; + +/** + * Regression test from a real scenario: + * + * User typed `class Fibonacci {\n\t` character by character. Two NES requests + * were made at different points during typing: + * + * - Request #6 (early): doc ended with `class `, model predicted `class FibonacciCalculator {` + * - Request #18 (later): doc ended with `class Fibonacci `, model predicted `class Fibonacci {` + * + * When `lookupNextEdit` runs, it should find and rebase the compatible cached edit + * from request #18 (whose prediction matches the user's typing). + */ +describe('NextEditCache rebase — Fibonacci scenario', () => { + + let configService: InMemoryConfigurationService; + let obsWorkspace: MutableObservableWorkspace; + let logService: LogServiceImpl; + let expService: NullExperimentationService; + let cache: NextEditCache; + let docId: DocumentId; + + // Common prefix of all document states — everything before the class declaration + const docPrefix = + 'import * as vscode from \'vscode\';\n' + + 'import { ASTNodeWithOffset } from \'./nodeTypes\';\n' + + 'import { NodeTypesIndex } from \'./nodeTypesIndex\';\n' + + 'import { Result } from \'./util/common/result\';\n' + + 'import { LRUCache } from \'./util/vs/base/common/map\';\n' + + '\n' + + 'export class NodeTypesDefinitionProvider implements vscode.DefinitionProvider {\n' + + '\n' + + '\tprivate _cache: LRUCache;\n' + + '\tprivate _definitions: Map;\n' + + '\n' + + '\tconstructor() {\n' + + '\t\tthis._definitions = new Map();\n' + + '\t\tthis._cache = new LRUCache(10);\n' + + '\t}\n' + + '\n' + + '\tasync provideDefinition(\n' + + '\t\tdocument: vscode.TextDocument,\n' + + '\t\tposition: vscode.Position,\n' + + '\t\ttoken: vscode.CancellationToken\n' + + '\t): Promise {\n' + + '\t\tconst word = NodeTypesDefinitionProvider.positionToSymbol(document, position);\n' + + '\t\tif (!word) {\n' + + '\t\t\treturn null;\n' + + '\t\t}\n' + + '\t\tconst def = this.computeDefForSymbol(document, word);\n' + + '\t\tif (!def) {\n' + + '\t\t\treturn null;\n' + + '\t\t}\n' + + '\t\treturn [{\n' + + '\t\t\ttargetUri: document.uri,\n' + + '\t\t\ttargetRange: new vscode.Range(document.positionAt(def.offset), document.positionAt(def.offset + def.length))\n' + + '\t\t}];\n' + + '\t}\n' + + '\n' + + '\tprivate computeDefForSymbol(document: vscode.TextDocument, symbol: string) {\n' + + '\t\tconst index = new NodeTypesIndex(document);\n' + + '\t\tconst astNodes = index.nodes;\n' + + '\t\tif (Result.isErr(astNodes)) {\n' + + '\t\t\treturn null;\n' + + '\t\t}\n' + + '\t\tthis.recomputeDefinitions(astNodes.val);\n' + + '\t\treturn this._definitions.get(symbol) || null;\n' + + '\t}\n' + + '\n' + + '\tprivate recomputeDefinitions(nodes: ASTNodeWithOffset[]) {\n' + + '\t\tif (this._cache.has(nodes)) {\n' + + '\t\t\treturn;\n' + + '\t\t}\n' + + '\t\tfor (const node of nodes) {\n' + + '\t\t\tthis._definitions.set(node.type.value, node);\n' + + '\t\t}\n' + + '\t\tthis._cache.set(nodes, true);\n' + + '\t}\n' + + '\n' + + '\tprivate static positionToSymbol(document: vscode.TextDocument, position: vscode.Position) {\n' + + '\t\tconst wordRange = document.getWordRangeAtPosition(position);\n' + + '\t\treturn wordRange ? document.getText(wordRange) : null;\n' + + '\t}\n' + + '}\n' + + '\n' + + 'function fibonacci(n: number): number {\n' + + '\tif (n <= 1) {\n' + + '\t\treturn n;\n' + + '\t}\n' + + '\treturn fibonacci(n - 1) + fibonacci(n - 2);\n' + + '}\n' + + '\n'; + + // Document states at different points in time + const docAtRequest6 = docPrefix + 'class '; // offset 1944 → "class " ends at 1950 + const docAtRequest18 = docPrefix + 'class Fibonacci '; // offset 1944 → "class Fibonacci " ends at 1960 + const currentDoc = docPrefix + 'class Fibonacci {\n\t'; // offset 1944 → "class Fibonacci {\n\t" ends at 1963 + + function makeSource(): NextEditFetchRequest { + const logContext = new InlineEditRequestLogContext('test', 0, undefined); + return new NextEditFetchRequest(generateUuid(), logContext, undefined, false); + } + + beforeEach(() => { + configService = new InMemoryConfigurationService(new DefaultsOnlyConfigurationService()); + obsWorkspace = new MutableObservableWorkspace(); + logService = new LogServiceImpl([]); + expService = new NullExperimentationService(); + + docId = DocumentId.create(URI.file('/Users/ulugbekna/code/tsq/src/nodeTypesDefinitionProvider.ts').toString()); + // Initialize workspace doc with the CURRENT document state + // (so checkEditConsistency(documentBeforeEdit + userEditSince = currentDoc) passes) + obsWorkspace.addDocument({ id: docId, initialValue: currentDoc }); + + cache = new NextEditCache(obsWorkspace, logService, configService, expService); + }); + + it('rebases cached edit when model predicted class Fibonacci { and user typed the same', () => { + // Scenario from real usage: + // documentBeforeEdit (at cache time): ...class Fibonacci \n (ends at offset 1960) + // Model's edit: replace [1944,1960) "class Fibonacci " → "class Fibonacci {" + // Model also has a 2nd edit: insert at 1961 → class body + // User then typed "{\n\t" → userEditSince: [1944,1960) → "class Fibonacci {\n\t" + // + // The user's typing is a superset of the model's first edit (model: "{", user: "{\n\t"), + // so rebase should succeed and the 2nd edit (class body) should be offered. + const cachedEdit = cache.setKthNextEdit( + docId, + new StringText(docAtRequest18), + new OffsetRange(1944, 1960), // editWindow + new StringReplacement(new OffsetRange(1944, 1960), 'class Fibonacci {'), + 0, + [ + new StringReplacement(new OffsetRange(1944, 1960), 'class Fibonacci {'), + new StringReplacement(OffsetRange.emptyAt(1961), '\n\tprivate memo: Map;\n\n\tconstructor() {\n\t\tthis.memo = new Map();\n\t}\n\n\tcalc(n: number): number {\n\t\tif (n <= 1) {\n\t\t\treturn n;\n\t\t}\n\t\tif (this.memo.has(n)) {\n\t\t\treturn this.memo.get(n)!;\n\t\t}\n\t\tconst result = this.calc(n - 1) + this.calc(n - 2);\n\t\tthis.memo.set(n, result);\n\t\treturn result;\n\t}\n}'), + ], + StringEdit.single(new StringReplacement(new OffsetRange(1944, 1960), 'class Fibonacci {\n\t')), + makeSource(), + { isFromCursorJump: false, cursorOffset: 1960 }, + ); + + assert(cachedEdit !== undefined, 'setKthNextEdit should return the cached edit'); + assert(cachedEdit.userEditSince !== undefined, 'userEditSince should be set'); + + const rebaseResult = cache.tryRebaseCacheEntry( + cachedEdit, + new StringText(currentDoc), + [new OffsetRange(1963, 1963)], + ); + + // TODO: rebase currently fails for this scenario because the user's typing + // ("{\n\t") is a strict superset of the model's first edit ("{") but the + // rebase engine treats this as a conflict. Once fixed, change this assertion: + assert(rebaseResult.edit === undefined, 'rebase currently fails — expected to succeed after fix'); + + // When the rebase is fixed, this should be the assertion: + // assert(rebaseResult.edit !== undefined, 'should rebase successfully'); + // assert(rebaseResult.edit.rebasedEdit !== undefined, 'should have a rebased edit for the class body'); + }); +}); From 4064fcf4d14129d885b7f980057c76278b1fdce4 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Thu, 16 Apr 2026 18:35:25 +0200 Subject: [PATCH 2/4] feat: add reverse-agreement rebase for NES cache behind experiment flag When the user types more text than the model predicted at the same position (e.g., model: '{', user: '{\n\t'), the rebase engine now recognizes that the model's edit is consumed by the user's typing and offers the remaining unconsumed model edits as rebased suggestions. The new behavior is gated behind the 'reverseAgreement' experiment flag in NesRebaseConfigs (config key: chat.advanced.inlineEdits.reverseAgreement, default: false). Adds 10 unit tests covering positive, negative, and edge cases. --- .../inlineEdits/common/editRebase.ts | 77 ++++++ .../inlineEdits/node/nextEditCache.ts | 1 + .../inlineEdits/node/rebaseResult.ts | 13 +- .../test/common/editRebase.spec.ts | 241 ++++++++++++++++++ .../test/node/nextEditCacheRebase.spec.ts | 12 +- .../common/configurationService.ts | 1 + 6 files changed, 334 insertions(+), 11 deletions(-) diff --git a/extensions/copilot/src/extension/inlineEdits/common/editRebase.ts b/extensions/copilot/src/extension/inlineEdits/common/editRebase.ts index 2a86014f85c0a..dd9ae35aca565 100644 --- a/extensions/copilot/src/extension/inlineEdits/common/editRebase.ts +++ b/extensions/copilot/src/extension/inlineEdits/common/editRebase.ts @@ -22,6 +22,14 @@ export interface NesRebaseConfigs { * the typed pair instead of failing. */ readonly absorbSubsequenceTyping?: boolean; + /** + * When enabled, allows rebase to succeed when the user typed more text + * than the model predicted at the same position (reverse agreement). + * Model edits consumed by the user's typing are absorbed, and any + * unconsumed portion of subsequent model edits is offered as the + * rebased suggestion. + */ + readonly reverseAgreement?: boolean; } export class EditDataWithIndex implements IEditData { @@ -209,6 +217,75 @@ function tryRebaseEdits>(content: string, ours: Annotated )); ourIdx++; offset += delta; + } else if (nesConfigs.reverseAgreement && ourEdit.replaceRange.equals(baseEdit.replaceRange)) { + // Reverse agreement: user's edit (base) covers model's edit (ours) + // at the same range. The user typed more than the model predicted. + // Use ourEdit (pre-shift) to avoid false matches from shift alignment. + // Iterate over consecutive our-edits consumed by this base edit. + let baseNewTextOffset = 0; + let previousOurE: AnnotatedStringReplacement | undefined; + + while (ourIdx < ours.replacements.length && baseEdit.replaceRange.containsRange(ours.replacements[ourIdx].replaceRange)) { + const curOurE = ours.replacements[ourIdx]; + + // Account for gap content between previous our-edit end and current our-edit start + const gapStart = previousOurE ? previousOurE.replaceRange.endExclusive : baseEdit.replaceRange.start; + const gapText = gapStart < curOurE.replaceRange.start ? content.substring(gapStart, curOurE.replaceRange.start) : ''; + const effectiveText = gapText + curOurE.newText; + + // Try full consumption: model text found entirely within user text + const j = baseEdit.newText.indexOf(effectiveText, baseNewTextOffset); + const strictRejected = j !== -1 && resolution === 'strict' && ( + j - baseNewTextOffset > maxAgreementOffset || + (j - baseNewTextOffset > 0 && effectiveText.length > maxImperfectAgreementLength) + ); + + if (j !== -1 && !strictRejected) { + // Full consumption — model edit absorbed by user typing + baseNewTextOffset = j + effectiveText.length; + previousOurE = curOurE; + ourIdx++; + continue; + } + + // Try partial consumption: remaining user text is a prefix of model text + const remainingBase = baseEdit.newText.substring(baseNewTextOffset); + if (remainingBase.length > 0 && effectiveText.startsWith(remainingBase)) { + const consumedFromNewText = Math.max(0, remainingBase.length - gapText.length); + const unconsumedNewText = curOurE.newText.substring(consumedFromNewText); + if (unconsumedNewText.length > 0) { + newEdits.push(new AnnotatedStringReplacement( + OffsetRange.emptyAt(baseEdit.replaceRange.start + offset + baseEdit.newText.length), + unconsumedNewText, + curOurE.data, + )); + } + baseNewTextOffset = baseEdit.newText.length; + previousOurE = curOurE; + ourIdx++; + break; + } + + // Conflicting + return undefined; + } + + // Verify trailing gap in strict mode: any original content between the + // last consumed our-edit and the end of the base range must be preserved. + // Remaining user text beyond the gap is the user's own typing and is fine. + if (baseNewTextOffset < baseEdit.newText.length && resolution === 'strict') { + const lastOurEnd = previousOurE ? previousOurE.replaceRange.endExclusive : baseEdit.replaceRange.start; + const trailingGap = content.substring(lastOurEnd, baseEdit.replaceRange.endExclusive); + if (trailingGap.length > 0) { + const remainingBase = baseEdit.newText.substring(baseNewTextOffset); + if (!remainingBase.startsWith(trailingGap)) { + return undefined; + } + } + } + + baseIdx++; + offset += baseEdit.newText.length - baseEdit.replaceRange.length; } else { // Conflicting return undefined; diff --git a/extensions/copilot/src/extension/inlineEdits/node/nextEditCache.ts b/extensions/copilot/src/extension/inlineEdits/node/nextEditCache.ts index 0a3a27444a102..bcaa766ee382d 100644 --- a/extensions/copilot/src/extension/inlineEdits/node/nextEditCache.ts +++ b/extensions/copilot/src/extension/inlineEdits/node/nextEditCache.ts @@ -127,6 +127,7 @@ export class NextEditCache extends Disposable { private _getNesRebaseConfigs(): NesRebaseConfigs { return { absorbSubsequenceTyping: this._configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsAbsorbSubsequenceTyping, this._expService), + reverseAgreement: this._configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsReverseAgreement, this._expService), }; } diff --git a/extensions/copilot/src/extension/inlineEdits/node/rebaseResult.ts b/extensions/copilot/src/extension/inlineEdits/node/rebaseResult.ts index 8d7f4e82cbf8e..bc278d8feea61 100644 --- a/extensions/copilot/src/extension/inlineEdits/node/rebaseResult.ts +++ b/extensions/copilot/src/extension/inlineEdits/node/rebaseResult.ts @@ -114,15 +114,22 @@ export class RebaseFailureInfo implements MarkdownLoggable { lines.push(`\tconst currentSelection = [${this.currentSelection.map(s => `new OffsetRange(${s.start}, ${s.endExclusive})`).join(', ')}];`); - if (this.nesRebaseConfigs.absorbSubsequenceTyping) { - lines.push(`\tconst nesConfigs = { absorbSubsequenceTyping: ${this.nesRebaseConfigs.absorbSubsequenceTyping} };`); + if (this.nesRebaseConfigs.absorbSubsequenceTyping || this.nesRebaseConfigs.reverseAgreement) { + const configEntries: string[] = []; + if (this.nesRebaseConfigs.absorbSubsequenceTyping) { + configEntries.push(`absorbSubsequenceTyping: ${this.nesRebaseConfigs.absorbSubsequenceTyping}`); + } + if (this.nesRebaseConfigs.reverseAgreement) { + configEntries.push(`reverseAgreement: ${this.nesRebaseConfigs.reverseAgreement}`); + } + lines.push(`\tconst nesConfigs = { ${configEntries.join(', ')} };`); } lines.push(''); lines.push('\tconst logger = new TestLogService();'); lines.push('\texpect(userEditSince.apply(originalDocument)).toBe(currentDocumentContent);'); - const configsArg = this.nesRebaseConfigs.absorbSubsequenceTyping ? ', nesConfigs' : ''; + const configsArg = (this.nesRebaseConfigs.absorbSubsequenceTyping || this.nesRebaseConfigs.reverseAgreement) ? ', nesConfigs' : ''; lines.push(`\texpect(tryRebase(originalDocument, editWindow, originalEdits, [], userEditSince, currentDocumentContent, currentSelection, 'strict', logger${configsArg})).toMatchInlineSnapshot();`); lines.push('});'); diff --git a/extensions/copilot/src/extension/inlineEdits/test/common/editRebase.spec.ts b/extensions/copilot/src/extension/inlineEdits/test/common/editRebase.spec.ts index 701ac20559dfd..45790da6b1fa1 100644 --- a/extensions/copilot/src/extension/inlineEdits/test/common/editRebase.spec.ts +++ b/extensions/copilot/src/extension/inlineEdits/test/common/editRebase.spec.ts @@ -1091,4 +1091,245 @@ class Point3D { expect(lenient2?.apply(current2)).toStrictEqual(applied); expect(lenient2?.removeCommonSuffixAndPrefix(current2).replacements.toString()).toMatchInlineSnapshot(`"[7, ${7 + maxImperfectAgreementLength + 1}) -> "x${'h'.repeat(maxImperfectAgreementLength + 2)}x""`); }); + + test('reverse agreement: user typed more than model predicted at same position', () => { + // Model predicts two edits: insert "{" and insert body. + // User typed "{\n\t" which covers the first edit and the start of the second. + // Rebase should succeed, offering the unconsumed portion of the second edit. + const originalDocument = 'class Fibonacci \n'; + const originalEdits = [ + StringReplacement.replace(new OffsetRange(0, 16), 'class Fibonacci {'), + StringReplacement.replace(OffsetRange.emptyAt(17), '\n\tprivate memo: Map;\n}'), + ]; + const userEditSince = StringEdit.create([ + StringReplacement.replace(new OffsetRange(0, 16), 'class Fibonacci {\n\t'), + ]); + const currentDocumentContent = 'class Fibonacci {\n\t\n'; + const nesConfigs = { reverseAgreement: true }; + + const logger = new TestLogService(); + // Without flag: rebase fails + expect(tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger)).toBe('rebaseFailed'); + // With flag: rebase succeeds + const res = tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger, nesConfigs); + expect(res).toBeTypeOf('object'); + const result = res as Exclude; + expect(result.length).toBe(1); + expect(result[0].rebasedEditIndex).toBe(1); + // The unconsumed portion of the body edit should be offered + expect(result[0].rebasedEdit.newText).toContain('private memo'); + }); + + test('reverse agreement: user typed exactly the first model edit', () => { + // User typed exactly "{" which is the model's first edit. + // The second edit (body) should be offered in full. + // Note: this case is actually handled by the existing forward agreement path + // (user text length == model text length), so it works regardless of the flag. + const originalDocument = 'class Foo \n'; + const originalEdits = [ + StringReplacement.replace(new OffsetRange(0, 10), 'class Foo {'), + StringReplacement.replace(OffsetRange.emptyAt(12), '\n\tbar(): void {}\n}'), + ]; + const userEditSince = StringEdit.create([ + StringReplacement.replace(new OffsetRange(0, 10), 'class Foo {'), + ]); + const currentDocumentContent = 'class Foo {\n'; + + const logger = new TestLogService(); + // Works without reverse agreement flag (handled by forward agreement) + const res = tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger); + expect(res).toBeTypeOf('object'); + const result = res as Exclude; + expect(result.length).toBe(1); + expect(result[0].rebasedEditIndex).toBe(1); + expect(result[0].rebasedEdit.newText).toContain('bar(): void {}'); + }); + + test('reverse agreement: user typed completely different text — should conflict', () => { + // Model: "class Foo " → "class Foo {" + // User: "class Foo " → "class Foo XYZ" + // "XYZ" is NOT found in "{", so this should fail. + const originalDocument = 'class Foo \n'; + const originalEdits = [ + StringReplacement.replace(new OffsetRange(0, 10), 'class Foo {'), + ]; + const userEditSince = StringEdit.create([ + StringReplacement.replace(new OffsetRange(0, 10), 'class Foo XYZ'), + ]); + const currentDocumentContent = 'class Foo XYZ\n'; + const nesConfigs = { reverseAgreement: true }; + + const logger = new TestLogService(); + expect(tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger, nesConfigs)).toBe('rebaseFailed'); + expect(tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'lenient', logger, nesConfigs)).toBe('rebaseFailed'); + }); + + test('reverse agreement: user typed text that accidentally contains model text as substring', () => { + // Model: replace [0,5) "hello" → "hello{" (diff: insert "{" at 5), then insert body at 6. + // User: replace [0,5) "hello" → "helloXX{YY" (diff: insert "XX{YY" at 5). + // The model's first diff ("{") IS found in user's "XX{YY" at offset 2, so it's consumed. + // But the model's second edit ("\n\tworld\n}") can't be found in the remaining + // user text "YY" — partial consumption also fails ("YY" doesn't start with "\n\tworld\n}"). + // So the rebase correctly fails for the second edit. + const originalDocument = 'hello\n'; + const originalEdits = [ + StringReplacement.replace(new OffsetRange(0, 5), 'hello{'), + StringReplacement.replace(OffsetRange.emptyAt(6), '\n\tworld\n}'), + ]; + const userEditSince = StringEdit.create([ + StringReplacement.replace(new OffsetRange(0, 5), 'helloXX{YY'), + ]); + const currentDocumentContent = 'helloXX{YY\n'; + const nesConfigs = { reverseAgreement: true }; + + const logger = new TestLogService(); + // Fails because user's remaining text "YY" doesn't match model's second edit + expect(tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger, nesConfigs)).toBe('rebaseFailed'); + expect(tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'lenient', logger, nesConfigs)).toBe('rebaseFailed'); + }); + + test('reverse agreement: user typed text with model text at large offset — strict rejects', () => { + // Model: "a" → "a{" + // User: "a" → "a" + "X".repeat(15) + "{" + // The "{" is at offset 15 into the user text, which exceeds maxAgreementOffset (10). + // Strict should reject; lenient should also fail since there's no lenient fallback + // in the reverse branch. + const pad = 'X'.repeat(maxAgreementOffset + 1); + const originalDocument = 'a\n'; + const originalEdits = [ + StringReplacement.replace(new OffsetRange(0, 1), 'a{'), + ]; + const userEditSince = StringEdit.create([ + StringReplacement.replace(new OffsetRange(0, 1), 'a' + pad + '{'), + ]); + const currentDocumentContent = 'a' + pad + '{\n'; + const nesConfigs = { reverseAgreement: true }; + + const logger = new TestLogService(); + expect(tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger, nesConfigs)).toBe('rebaseFailed'); + }); + + test('reverse agreement: user typed long text at small offset — strict rejects imperfect agreement', () => { + // Model: "a" → "a{" + // User: "a" → "aX" + "{".repeat(maxImperfectAgreementLength + 1) + // The model text "{" is found at offset 1 (> 0) and the effective text length + // is 1 (≤ maxImperfectAgreementLength), so this should pass strict. + // But if effectiveText were longer... + const longText = 'Z'.repeat(maxImperfectAgreementLength + 1); + const originalDocument = 'a\n'; + const originalEdits = [ + StringReplacement.replace(new OffsetRange(0, 1), 'a' + longText), + ]; + const userEditSince = StringEdit.create([ + StringReplacement.replace(new OffsetRange(0, 1), 'aX' + longText), + ]); + const currentDocumentContent = 'aX' + longText + '\n'; + const nesConfigs = { reverseAgreement: true }; + + const logger = new TestLogService(); + // offset = 1 > 0, effectiveText.length = longText.length > maxImperfectAgreementLength + // → strict rejected + expect(tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger, nesConfigs)).toBe('rebaseFailed'); + }); + + test('reverse agreement: all model edits fully consumed by user — no rebased edit emitted', () => { + // Model predicts single edit: insert "{\n\t" + // User typed "{\n\tfoo\n}" which fully contains "{\n\t" + // All model edits consumed → nothing to offer + const originalDocument = 'fn \n'; + const originalEdits = [ + StringReplacement.replace(new OffsetRange(0, 3), 'fn {\n\t'), + ]; + const userEditSince = StringEdit.create([ + StringReplacement.replace(new OffsetRange(0, 3), 'fn {\n\tfoo\n}'), + ]); + const currentDocumentContent = 'fn {\n\tfoo\n}\n'; + const nesConfigs = { reverseAgreement: true }; + + const logger = new TestLogService(); + // Without flag: rebase fails + expect(tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger)).toBe('rebaseFailed'); + // With flag: succeeds with no edits to offer + const res = tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger, nesConfigs); + expect(res).toBeTypeOf('object'); + const result = res as Exclude; + // The single model edit was fully consumed — nothing left to suggest + expect(result.length).toBe(0); + }); + + test('reverse agreement: consistency check — rebased edit applied to current doc produces expected result', () => { + // This is the key correctness check: applying the rebased edit to the current + // document should produce the same result as applying the original edits to + // the original document. + const originalDocument = 'class Fibonacci \n'; + const originalEdits = [ + StringReplacement.replace(new OffsetRange(0, 16), 'class Fibonacci {'), + StringReplacement.replace(OffsetRange.emptyAt(17), '\n\tprivate memo: Map;\n}'), + ]; + const userEditSince = StringEdit.create([ + StringReplacement.replace(new OffsetRange(0, 16), 'class Fibonacci {\n\t'), + ]); + const currentDocumentContent = 'class Fibonacci {\n\t\n'; + const nesConfigs = { reverseAgreement: true }; + + // Expected final: apply both model edits in sequence to original + const expectedFinal = new StringEdit([originalEdits[0]]).apply(originalDocument); + const expectedFinal2 = new StringEdit([originalEdits[1]]).apply(expectedFinal); + + const logger = new TestLogService(); + const res = tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger, nesConfigs); + expect(res).toBeTypeOf('object'); + const result = res as Exclude; + expect(result.length).toBe(1); + + // Apply rebased edit to current document + const actualFinal = StringEdit.single(result[0].rebasedEdit).apply(currentDocumentContent); + expect(actualFinal).toBe(expectedFinal2); + }); + + test('reverse agreement: pure inserts at same position — user insert is superset of model insert', () => { + // Both edits are pure inserts at position 5. + // Model inserts "X", user inserts "XY". + // After removeCommonSuffixAndPrefix on user edit: + // user edit: insert at 5 → "XY", model edit: insert at 5 → "X" + // These have equal replaceRange (both emptyAt(5)). + // The reverse branch should fire: "X" found in "XY" at offset 0 → consumed. + // Nothing left to suggest from this model edit. + const originalDocument = 'hello world\n'; + const suggestedEdit = StringEdit.create([ + StringReplacement.replace(OffsetRange.emptyAt(5), 'X'), + ]); + const userEdit = StringEdit.create([ + StringReplacement.replace(OffsetRange.emptyAt(5), 'XY'), + ]); + const current = userEdit.apply(originalDocument); + expect(current).toBe('helloXY world\n'); + + // Without flag: rebase fails + expect(tryRebaseStringEdits(originalDocument, suggestedEdit, userEdit, 'strict')).toBeUndefined(); + // With flag: model edit fully consumed → empty result + const nesConfigs = { reverseAgreement: true }; + const res = tryRebaseStringEdits(originalDocument, suggestedEdit, userEdit, 'strict', nesConfigs); + expect(res).toBeDefined(); + expect(res!.replacements.length).toBe(0); + }); + + test('reverse agreement: does NOT fire when ranges differ', () => { + // Model replaces [0,3), user replaces [0,5) — different ranges. + // The reverse branch requires equal ranges, so this should NOT trigger it. + // Instead, this falls through to the conflict branch. + const originalDocument = 'abcde\n'; + const originalEdits = [ + StringReplacement.replace(new OffsetRange(0, 3), 'XYZ'), + ]; + const userEditSince = StringEdit.create([ + StringReplacement.replace(new OffsetRange(0, 5), 'XYZWV'), + ]); + const currentDocumentContent = 'XYZWV\n'; + const nesConfigs = { reverseAgreement: true }; + + const logger = new TestLogService(); + // The ranges don't match after removeCommonSuffixAndPrefix, so this conflicts + expect(tryRebase(originalDocument, undefined, originalEdits, [], userEditSince, currentDocumentContent, [], 'strict', logger, nesConfigs)).toBe('rebaseFailed'); + }); }); diff --git a/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts b/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts index 2d710ee53ab24..04f3bca43f1f3 100644 --- a/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts +++ b/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ import { assert, beforeEach, describe, it } from 'vitest'; +import { ConfigKey } from '../../../../platform/configuration/common/configurationService'; import { DefaultsOnlyConfigurationService } from '../../../../platform/configuration/common/defaultsOnlyConfigurationService'; import { InMemoryConfigurationService } from '../../../../platform/configuration/test/common/inMemoryConfigurationService'; import { DocumentId } from '../../../../platform/inlineEdits/common/dataTypes/documentId'; @@ -122,6 +123,7 @@ describe('NextEditCache rebase — Fibonacci scenario', () => { beforeEach(() => { configService = new InMemoryConfigurationService(new DefaultsOnlyConfigurationService()); + configService.setConfig(ConfigKey.TeamInternal.InlineEditsReverseAgreement, true); obsWorkspace = new MutableObservableWorkspace(); logService = new LogServiceImpl([]); expService = new NullExperimentationService(); @@ -167,13 +169,7 @@ describe('NextEditCache rebase — Fibonacci scenario', () => { [new OffsetRange(1963, 1963)], ); - // TODO: rebase currently fails for this scenario because the user's typing - // ("{\n\t") is a strict superset of the model's first edit ("{") but the - // rebase engine treats this as a conflict. Once fixed, change this assertion: - assert(rebaseResult.edit === undefined, 'rebase currently fails — expected to succeed after fix'); - - // When the rebase is fixed, this should be the assertion: - // assert(rebaseResult.edit !== undefined, 'should rebase successfully'); - // assert(rebaseResult.edit.rebasedEdit !== undefined, 'should have a rebased edit for the class body'); + assert(rebaseResult.edit !== undefined, 'should rebase successfully'); + assert(rebaseResult.edit.rebasedEdit !== undefined, 'should have a rebased edit for the class body'); }); }); diff --git a/extensions/copilot/src/platform/configuration/common/configurationService.ts b/extensions/copilot/src/platform/configuration/common/configurationService.ts index 548b3ab8febb5..526b37e3bc23d 100644 --- a/extensions/copilot/src/platform/configuration/common/configurationService.ts +++ b/extensions/copilot/src/platform/configuration/common/configurationService.ts @@ -777,6 +777,7 @@ export namespace ConfigKey { export const InlineEditsSpeculativeRequestDelay = defineTeamInternalSetting('chat.advanced.inlineEdits.speculativeRequestDelay', ConfigType.ExperimentBased, 0); export const InlineEditsRebasedCacheDelay = defineTeamInternalSetting('chat.advanced.inlineEdits.rebasedCacheDelay', ConfigType.ExperimentBased, 0); export const InlineEditsAbsorbSubsequenceTyping = defineTeamInternalSetting('chat.advanced.inlineEdits.absorbSubsequenceTyping', ConfigType.ExperimentBased, false); + export const InlineEditsReverseAgreement = defineTeamInternalSetting('chat.advanced.inlineEdits.reverseAgreement', ConfigType.ExperimentBased, false); export const InlineEditsBackoffDebounceEnabled = defineTeamInternalSetting('chat.advanced.inlineEdits.backoffDebounceEnabled', ConfigType.ExperimentBased, true); export const InlineEditsExtraDebounceEndOfLine = defineTeamInternalSetting('chat.advanced.inlineEdits.extraDebounceEndOfLine', ConfigType.ExperimentBased, 2000); export const InlineEditsSpeculativeRequests = defineTeamInternalSetting('chat.advanced.inlineEdits.speculativeRequests', ConfigType.ExperimentBased, SpeculativeRequestsEnablement.Off, SpeculativeRequestsEnablement.VALIDATOR); From e4e41df68e0ef2d4de4fb5bf0d3c9aa2a9884404 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Thu, 16 Apr 2026 18:41:57 +0200 Subject: [PATCH 3/4] remove unused var --- .../extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts b/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts index 04f3bca43f1f3..8b03c4f87342c 100644 --- a/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts +++ b/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts @@ -112,7 +112,6 @@ describe('NextEditCache rebase — Fibonacci scenario', () => { '\n'; // Document states at different points in time - const docAtRequest6 = docPrefix + 'class '; // offset 1944 → "class " ends at 1950 const docAtRequest18 = docPrefix + 'class Fibonacci '; // offset 1944 → "class Fibonacci " ends at 1960 const currentDoc = docPrefix + 'class Fibonacci {\n\t'; // offset 1944 → "class Fibonacci {\n\t" ends at 1963 From 9b1a83d2d9fd6f8a8e99c3c635502cdebc17662f Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Thu, 16 Apr 2026 18:52:05 +0200 Subject: [PATCH 4/4] address review: computed offsets, await setConfig, generic path --- .../test/node/nextEditCacheRebase.spec.ts | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts b/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts index 8b03c4f87342c..6020bf4716d15 100644 --- a/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts +++ b/extensions/copilot/src/extension/inlineEdits/test/node/nextEditCacheRebase.spec.ts @@ -111,23 +111,26 @@ describe('NextEditCache rebase — Fibonacci scenario', () => { '}\n' + '\n'; - // Document states at different points in time - const docAtRequest18 = docPrefix + 'class Fibonacci '; // offset 1944 → "class Fibonacci " ends at 1960 - const currentDoc = docPrefix + 'class Fibonacci {\n\t'; // offset 1944 → "class Fibonacci {\n\t" ends at 1963 + // Document states at different points in time — offsets derived from docPrefix.length + const classStart = docPrefix.length; // where "class " begins + const docAtRequest18 = docPrefix + 'class Fibonacci '; // "class Fibonacci " ends at classStart + 16 + const classEndAtRequest18 = classStart + 'class Fibonacci '.length; // = classStart + 16 + const currentDoc = docPrefix + 'class Fibonacci {\n\t'; // "class Fibonacci {\n\t" ends at classStart + 19 + const cursorOffset = classStart + 'class Fibonacci {\n\t'.length; // = classStart + 19 function makeSource(): NextEditFetchRequest { const logContext = new InlineEditRequestLogContext('test', 0, undefined); return new NextEditFetchRequest(generateUuid(), logContext, undefined, false); } - beforeEach(() => { + beforeEach(async () => { configService = new InMemoryConfigurationService(new DefaultsOnlyConfigurationService()); - configService.setConfig(ConfigKey.TeamInternal.InlineEditsReverseAgreement, true); + await configService.setConfig(ConfigKey.TeamInternal.InlineEditsReverseAgreement, true); obsWorkspace = new MutableObservableWorkspace(); logService = new LogServiceImpl([]); expService = new NullExperimentationService(); - docId = DocumentId.create(URI.file('/Users/ulugbekna/code/tsq/src/nodeTypesDefinitionProvider.ts').toString()); + docId = DocumentId.create(URI.file('/test/nodeTypesDefinitionProvider.ts').toString()); // Initialize workspace doc with the CURRENT document state // (so checkEditConsistency(documentBeforeEdit + userEditSince = currentDoc) passes) obsWorkspace.addDocument({ id: docId, initialValue: currentDoc }); @@ -147,16 +150,16 @@ describe('NextEditCache rebase — Fibonacci scenario', () => { const cachedEdit = cache.setKthNextEdit( docId, new StringText(docAtRequest18), - new OffsetRange(1944, 1960), // editWindow - new StringReplacement(new OffsetRange(1944, 1960), 'class Fibonacci {'), + new OffsetRange(classStart, classEndAtRequest18), // editWindow + new StringReplacement(new OffsetRange(classStart, classEndAtRequest18), 'class Fibonacci {'), 0, [ - new StringReplacement(new OffsetRange(1944, 1960), 'class Fibonacci {'), - new StringReplacement(OffsetRange.emptyAt(1961), '\n\tprivate memo: Map;\n\n\tconstructor() {\n\t\tthis.memo = new Map();\n\t}\n\n\tcalc(n: number): number {\n\t\tif (n <= 1) {\n\t\t\treturn n;\n\t\t}\n\t\tif (this.memo.has(n)) {\n\t\t\treturn this.memo.get(n)!;\n\t\t}\n\t\tconst result = this.calc(n - 1) + this.calc(n - 2);\n\t\tthis.memo.set(n, result);\n\t\treturn result;\n\t}\n}'), + new StringReplacement(new OffsetRange(classStart, classEndAtRequest18), 'class Fibonacci {'), + new StringReplacement(OffsetRange.emptyAt(classStart + 'class Fibonacci {'.length), '\n\tprivate memo: Map;\n\n\tconstructor() {\n\t\tthis.memo = new Map();\n\t}\n\n\tcalc(n: number): number {\n\t\tif (n <= 1) {\n\t\t\treturn n;\n\t\t}\n\t\tif (this.memo.has(n)) {\n\t\t\treturn this.memo.get(n)!;\n\t\t}\n\t\tconst result = this.calc(n - 1) + this.calc(n - 2);\n\t\tthis.memo.set(n, result);\n\t\treturn result;\n\t}\n}'), ], - StringEdit.single(new StringReplacement(new OffsetRange(1944, 1960), 'class Fibonacci {\n\t')), + StringEdit.single(new StringReplacement(new OffsetRange(classStart, classEndAtRequest18), 'class Fibonacci {\n\t')), makeSource(), - { isFromCursorJump: false, cursorOffset: 1960 }, + { isFromCursorJump: false, cursorOffset: classEndAtRequest18 }, ); assert(cachedEdit !== undefined, 'setKthNextEdit should return the cached edit'); @@ -165,7 +168,7 @@ describe('NextEditCache rebase — Fibonacci scenario', () => { const rebaseResult = cache.tryRebaseCacheEntry( cachedEdit, new StringText(currentDoc), - [new OffsetRange(1963, 1963)], + [new OffsetRange(cursorOffset, cursorOffset)], ); assert(rebaseResult.edit !== undefined, 'should rebase successfully');