Skip to content

nes: make tryRebase conflict limit exp-configurable#312708

Merged
ulugbekna merged 2 commits into
mainfrom
ulugbekna/agents/exp-configure-tryrebase-conflict-limit
Apr 27, 2026
Merged

nes: make tryRebase conflict limit exp-configurable#312708
ulugbekna merged 2 commits into
mainfrom
ulugbekna/agents/exp-configure-tryrebase-conflict-limit

Conversation

@ulugbekna
Copy link
Copy Markdown
Contributor

Summary

The tryRebase logic uses a hard-coded limit of 5 characters (maxImperfectAgreementLength) to decide whether a rebase with an imperfect agreement (i.e. the user's typed text is found in the suggestion but not at offset 0) should be accepted or rejected. This PR makes that limit configurable via an experiment setting so we can tune it without a code change.

Changes

NesRebaseConfigs interface (editRebase.ts)

  • Added maxImperfectAgreementLength: number as a required field (non-optional, so call sites always supply an explicit value — no hidden fallbacks).
  • Both places that enforce the strict limit now read directly from nesConfigs.maxImperfectAgreementLength:
    • Normal agreement path in agreementIndexOf
    • Reverse agreement path in tryRebaseEdits
  • Default parameter values for the two public helpers (tryRebase, tryRebaseStringEdits) are set to { maxImperfectAgreementLength } (the existing constant, value 5), preserving current behaviour for callers that don't pass configs.

Config key (configurationService.ts)

  • Added ConfigKey.TeamInternal.InlineEditsMaxImperfectAgreementLength — an experiment-based number setting with key chat.advanced.inlineEdits.maxImperfectAgreementLength and default 5.

Wire-up (nextEditCache.ts)

  • _getNesRebaseConfigs() now reads the new key and includes it in the returned NesRebaseConfigs.

Telemetry / debug code (rebaseResult.ts)

  • toReproductionTestCode() emits maxImperfectAgreementLength in the generated nesConfigs snippet when it is set.

Tests (editRebase.spec.ts)

  • All existing nesConfigs object literals updated to include maxImperfectAgreementLength (using the imported constant) to satisfy the now-required field.

Add `maxImperfectAgreementLength` to `NesRebaseConfigs` as a required field (no fallback needed at call sites). Wire it up through a new experiment-based `ConfigKey.TeamInternal.InlineEditsMaxImperfectAgreementLength` setting (default 5, key `chat.advanced.inlineEdits.maxImperfectAgreementLength`).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 27, 2026 09:54
@ulugbekna ulugbekna enabled auto-merge (squash) April 27, 2026 10:00
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 makes the tryRebase “imperfect agreement” conflict threshold configurable via an experiment-backed setting, replacing the previously hard-coded maxImperfectAgreementLength = 5 limit so it can be tuned without code changes.

Changes:

  • Adds an experiment-based config key chat.advanced.inlineEdits.maxImperfectAgreementLength (default 5).
  • Wires the setting into NES rebase logic by making maxImperfectAgreementLength a required field of NesRebaseConfigs and using it in both agreement paths.
  • Updates rebase debugging output and unit tests to include the new required config field.
Show a summary per file
File Description
extensions/copilot/src/platform/configuration/common/configurationService.ts Adds new experiment-based team-internal config key for the threshold.
extensions/copilot/src/extension/inlineEdits/node/nextEditCache.ts Reads the new config and supplies it in NesRebaseConfigs.
extensions/copilot/src/extension/inlineEdits/common/editRebase.ts Makes the threshold required in NesRebaseConfigs and uses it in strict rejection logic; adds defaulting in public helpers.
extensions/copilot/src/extension/inlineEdits/node/rebaseResult.ts Includes the new config in auto-generated reproduction test output.
extensions/copilot/src/extension/inlineEdits/test/common/editRebase.spec.ts Updates test config objects to include maxImperfectAgreementLength.

Copilot's findings

Comments suppressed due to low confidence (2)

extensions/copilot/src/extension/inlineEdits/node/rebaseResult.ts:136

  • Same as above: since maxImperfectAgreementLength is required, this ternary will always append , nesConfigs even when no optional flags are enabled. If you only want to pass nesConfigs when it meaningfully differs from defaults, gate on that instead of !== undefined.
		const configsArg = (this.nesRebaseConfigs.absorbSubsequenceTyping || this.nesRebaseConfigs.reverseAgreement || this.nesRebaseConfigs.maxImperfectAgreementLength !== undefined) ? ', nesConfigs' : '';
		lines.push(`\texpect(tryRebase(originalDocument, editWindow, originalEdits, [], userEditSince, currentDocumentContent, currentSelection, 'strict', logger${configsArg})).toMatchInlineSnapshot();`);

extensions/copilot/src/extension/inlineEdits/common/editRebase.ts:144

  • tryRebaseStringEdits has the same issue as tryRebase: the default nesConfigs = { maxImperfectAgreementLength } can throw at runtime in circular-import scenarios because maxImperfectAgreementLength is initialized later in the module. Define the constant before these exported functions (or otherwise ensure it's initialized before use).
export function tryRebaseStringEdits<T extends IEditData<T>>(content: string, ours: StringEdit, base: StringEdit, resolution: 'strict' | 'lenient', nesConfigs: NesRebaseConfigs = { maxImperfectAgreementLength }): StringEdit | undefined {
	return tryRebaseEdits(content, ours.mapData(r => new VoidEditData()), base, resolution, nesConfigs)?.toStringEdit();
  • Files reviewed: 5/5 changed files
  • Comments generated: 5

Comment thread extensions/copilot/src/extension/inlineEdits/node/nextEditCache.ts Outdated
Comment thread extensions/copilot/src/extension/inlineEdits/node/rebaseResult.ts Outdated
Comment thread extensions/copilot/src/extension/inlineEdits/common/editRebase.ts Outdated
Comment thread extensions/copilot/src/extension/inlineEdits/common/editRebase.ts
Comment thread extensions/copilot/src/platform/configuration/common/configurationService.ts Outdated
- Move maxAgreementOffset/maxImperfectAgreementLength constants before the interface and function declarations to eliminate the TDZ risk in default parameter values
- Clarify JSDoc: the field is required on NesRebaseConfigs; the default only applies when the whole nesConfigs param is omitted
- Clamp maxImperfectAgreementLength to >= 0 in _getNesRebaseConfigs() to guard against negative experiment values
- Add vNumber() validator to InlineEditsMaxImperfectAgreementLength config key
- Simplify rebaseResult.ts: always emit maxImperfectAgreementLength (required field); drop redundant !== undefined guards

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ulugbekna ulugbekna merged commit e46ea1e into main Apr 27, 2026
40 of 41 checks passed
@ulugbekna ulugbekna deleted the ulugbekna/agents/exp-configure-tryrebase-conflict-limit branch April 27, 2026 13:03
@vs-code-engineering vs-code-engineering Bot added this to the 1.119.0 milestone Apr 27, 2026
@ulugbekna ulugbekna added the release-cherry-pick Automated cherry-pick between release and main branches label Apr 27, 2026
@justschen justschen added the ~release-cherry-pick Trigger: cherry-pick this PR to the latest release branch label Apr 27, 2026
@vs-code-engineering vs-code-engineering Bot removed the ~release-cherry-pick Trigger: cherry-pick this PR to the latest release branch label Apr 27, 2026
dmitrivMS pushed a commit that referenced this pull request Apr 27, 2026
nes: make tryRebase conflict limit exp-configurable (#312708)

Add `maxImperfectAgreementLength` to `NesRebaseConfigs` as a required field (no fallback needed at call sites). Wire it up through a new experiment-based `ConfigKey.TeamInternal.InlineEditsMaxImperfectAgreementLength` setting (default 5, key `chat.advanced.inlineEdits.maxImperfectAgreementLength`).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

4 participants