Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions extensions/copilot/src/extension/xtab/common/promptCrafting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ ${PromptTags.EDIT_HISTORY.end}`;
case PromptingStrategy.PatchBased01:
mainPrompt = basePrompt;
break;
case PromptingStrategy.PatchBased02: {
case PromptingStrategy.PatchBased02:
case PromptingStrategy.PatchBased02WithRecentLineNumbers:
case PromptingStrategy.PatchBased02WithoutRecentLineNumbers: {
const currentDocument = promptPieces.currentDocument;
const cursorLine = currentDocument.lineWithCursor();
const cursorLineWithTag = cursorLine.substring(0, currentDocument.cursorPosition.column - 1) + PromptTags.CURSOR + cursorLine.substring(currentDocument.cursorPosition.column - 1);
Expand Down Expand Up @@ -144,7 +146,9 @@ ${PromptTags.EDIT_HISTORY.end}`;
const includeBackticks = opts.promptingStrategy !== PromptingStrategy.Nes41Miniv3 &&
opts.promptingStrategy !== PromptingStrategy.Codexv21NesUnified &&
opts.promptingStrategy !== PromptingStrategy.PatchBased01 &&
opts.promptingStrategy !== PromptingStrategy.PatchBased02;
opts.promptingStrategy !== PromptingStrategy.PatchBased02 &&
opts.promptingStrategy !== PromptingStrategy.PatchBased02WithRecentLineNumbers &&
opts.promptingStrategy !== PromptingStrategy.PatchBased02WithoutRecentLineNumbers;

const packagedPrompt = includeBackticks ? wrapInBackticks(mainPrompt) : mainPrompt;
const packagedPromptWithRelatedInfo = addRelatedInformation(relatedInformation, packagedPrompt, opts.languageContext.traitPosition);
Expand Down Expand Up @@ -331,6 +335,8 @@ function getPostScript(strategy: PromptingStrategy | undefined, currentFilePath:
case PromptingStrategy.Codexv21NesUnified:
break;
case PromptingStrategy.PatchBased02:
case PromptingStrategy.PatchBased02WithRecentLineNumbers:
case PromptingStrategy.PatchBased02WithoutRecentLineNumbers:
postScript = `The developer was working on a section of code within the \`current_file_content\` - carefully note their \`cursor_location\` marked with \`<|cursor|>\`. Using the given \`recently_viewed_code_snippets\`, \`current_file_content\`, \`edit_diff_history\`, and \`cursor_location\`, please continue the developer's work. Output a modified diff format with a sequence of intuitive next changes, where each patch must start with \`<filename>:<line number>\`. Order changes by priority and flow; for instance, edits adjacent to the user's cursor should always be prioritized, followed by lines near the cursor, followed by lines farther away. If there are no good edit candidates, output the empty string "". Avoid undoing or reverting the developer's last change unless there are obvious typos or errors. Adhere meticulously to the diff format.`;
break;
case PromptingStrategy.UnifiedModel:
Expand Down
7 changes: 3 additions & 4 deletions extensions/copilot/src/extension/xtab/node/xtabProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1451,10 +1451,7 @@ export class XtabProvider implements IStatelessNextEditProvider {
};

const selectedModelConfig = this.modelService.selectedModelConfiguration();
// proxy /models doesn't know about includeTagsInCurrentFile field as of now, so hard code it to true for CopilotNesXtab strategy
const modelConfig: xtabPromptOptions.ModelConfiguration = selectedModelConfig.promptingStrategy === xtabPromptOptions.PromptingStrategy.CopilotNesXtab
? { ...selectedModelConfig, includeTagsInCurrentFile: true }
: selectedModelConfig;
const modelConfig = xtabPromptOptions.applyStrategyConfig(selectedModelConfig);
return {
promptOptions: overrideModelConfig(sourcedModelConfig, modelConfig),
modelServiceConfig: modelConfig
Expand Down Expand Up @@ -1654,6 +1651,8 @@ export function pickSystemPrompt(promptingStrategy: xtabPromptOptions.PromptingS
case xtabPromptOptions.PromptingStrategy.PatchBased:
case xtabPromptOptions.PromptingStrategy.PatchBased01:
case xtabPromptOptions.PromptingStrategy.PatchBased02:
case xtabPromptOptions.PromptingStrategy.PatchBased02WithRecentLineNumbers:
case xtabPromptOptions.PromptingStrategy.PatchBased02WithoutRecentLineNumbers:
case xtabPromptOptions.PromptingStrategy.Xtab275:
case xtabPromptOptions.PromptingStrategy.XtabAggressiveness:
case xtabPromptOptions.PromptingStrategy.Xtab275Aggressiveness:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ describe('pickSystemPrompt', () => {
PromptingStrategy.PatchBased,
PromptingStrategy.PatchBased01,
PromptingStrategy.PatchBased02,
PromptingStrategy.PatchBased02WithRecentLineNumbers,
PromptingStrategy.PatchBased02WithoutRecentLineNumbers,
PromptingStrategy.Xtab275,
PromptingStrategy.XtabAggressiveness,
PromptingStrategy.Xtab275Aggressiveness,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,10 @@ export enum PromptingStrategy {
PatchBased = 'patchBased',
PatchBased01 = 'patchBased01',
PatchBased02 = 'patchBased02',
/** PatchBased02 variant: line numbers on recent docs. */
PatchBased02WithRecentLineNumbers = 'patchBased02WithRecentLineNumbers',
/** PatchBased02 variant: no line numbers on recent docs. */
PatchBased02WithoutRecentLineNumbers = 'patchBased02WithoutRecentLineNumbers',
/**
* Xtab275-based strategy with edit intent tag parsing.
* Response format: <|edit_intent|>low|medium|high|no_edit<|/edit_intent|>
Expand Down Expand Up @@ -393,6 +397,8 @@ export namespace ResponseFormat {
case PromptingStrategy.PatchBased:
case PromptingStrategy.PatchBased01:
case PromptingStrategy.PatchBased02:
case PromptingStrategy.PatchBased02WithRecentLineNumbers:
case PromptingStrategy.PatchBased02WithoutRecentLineNumbers:
return ResponseFormat.CustomDiffPatch;
case PromptingStrategy.Xtab275EditIntent:
return ResponseFormat.EditWindowWithEditIntent;
Expand Down Expand Up @@ -473,6 +479,50 @@ export interface ModelConfiguration {
supportsNextCursorLinePrediction?: boolean;
}

/**
* Per-strategy configuration baked into the strategy itself. When a strategy
* declares values here, those values override anything provided by the upstream
* model configuration. A strategy without an entry contributes no overrides.
*/
const STRATEGY_CONFIG: Partial<Record<PromptingStrategy, Partial<ModelConfiguration>>> = {
// proxy /models doesn't know about includeTagsInCurrentFile field as of now, so hard-code it for CopilotNesXtab
[PromptingStrategy.CopilotNesXtab]: {
includeTagsInCurrentFile: true,
},
[PromptingStrategy.PatchBased02WithRecentLineNumbers]: {
includeTagsInCurrentFile: false,
includePostScript: true,
currentFile: { includeLineNumbers: IncludeLineNumbersOption.WithoutSpace },
recentlyViewedDocuments: { includeLineNumbers: IncludeLineNumbersOption.WithoutSpace },
supportsNextCursorLinePrediction: false,
},
[PromptingStrategy.PatchBased02WithoutRecentLineNumbers]: {
includeTagsInCurrentFile: false,
includePostScript: true,
currentFile: { includeLineNumbers: IncludeLineNumbersOption.WithoutSpace },
recentlyViewedDocuments: { includeLineNumbers: IncludeLineNumbersOption.None },
supportsNextCursorLinePrediction: false,
},
};

/** Apply per-strategy baked-in config; strategy values override `config`. */
export function applyStrategyConfig(config: ModelConfiguration): ModelConfiguration {
const overrides = config.promptingStrategy === undefined ? undefined : STRATEGY_CONFIG[config.promptingStrategy];
if (!overrides) {
return config;
}
const hasCurrentFile = config.currentFile !== undefined || overrides.currentFile !== undefined;
const hasRecentlyViewed = config.recentlyViewedDocuments !== undefined || overrides.recentlyViewedDocuments !== undefined;
const hasLintOptions = config.lintOptions !== undefined || overrides.lintOptions !== undefined;
return {
...config,
...overrides,
currentFile: hasCurrentFile ? { ...config.currentFile, ...overrides.currentFile } : undefined,
recentlyViewedDocuments: hasRecentlyViewed ? { ...config.recentlyViewedDocuments, ...overrides.recentlyViewedDocuments } : undefined,
lintOptions: hasLintOptions ? { ...config.lintOptions, ...overrides.lintOptions } : undefined,
};
}

export const LINT_OPTIONS_VALIDATOR: IValidator<Partial<LintOptions>> = vObj({
'tagName': vString(),
'warnings': vEnum(LintOptionWarning.YES, LintOptionWarning.NO, LintOptionWarning.YES_IF_NO_ERRORS),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { describe, expect, it } from 'vitest';
import { applyStrategyConfig, IncludeLineNumbersOption, ModelConfiguration, PromptingStrategy } from '../../common/dataTypes/xtabPromptOptions';

function baseConfig(overrides: Partial<ModelConfiguration> = {}): ModelConfiguration {
return {
modelName: 'test-model',
promptingStrategy: undefined,
includeTagsInCurrentFile: true,
lintOptions: undefined,
...overrides,
};
}

describe('applyStrategyConfig', () => {

it('returns config unchanged when strategy has no entry', () => {
const config = baseConfig({ promptingStrategy: PromptingStrategy.Xtab275 });
expect(applyStrategyConfig(config)).toBe(config);
});

it('returns config unchanged when strategy is undefined', () => {
const config = baseConfig();
expect(applyStrategyConfig(config)).toBe(config);
});

it('forces includeTagsInCurrentFile=true for CopilotNesXtab', () => {
const result = applyStrategyConfig(baseConfig({
promptingStrategy: PromptingStrategy.CopilotNesXtab,
includeTagsInCurrentFile: false,
}));
expect(result.includeTagsInCurrentFile).toBe(true);
});

it('forces baked-in fields for PatchBased02WithRecentLineNumbers', () => {
const result = applyStrategyConfig(baseConfig({
promptingStrategy: PromptingStrategy.PatchBased02WithRecentLineNumbers,
includeTagsInCurrentFile: true,
includePostScript: false,
currentFile: { includeLineNumbers: IncludeLineNumbersOption.None, maxTokens: 42 },
recentlyViewedDocuments: { includeLineNumbers: IncludeLineNumbersOption.None, maxTokens: 99 },
supportsNextCursorLinePrediction: true,
}));
expect(result).toMatchObject({
includeTagsInCurrentFile: false,
includePostScript: true,
currentFile: { includeLineNumbers: IncludeLineNumbersOption.WithoutSpace, maxTokens: 42 },
recentlyViewedDocuments: { includeLineNumbers: IncludeLineNumbersOption.WithoutSpace, maxTokens: 99 },
supportsNextCursorLinePrediction: false,
});
});

it('forces recentlyViewedDocuments.includeLineNumbers=None for PatchBased02WithoutRecentLineNumbers', () => {
const result = applyStrategyConfig(baseConfig({
promptingStrategy: PromptingStrategy.PatchBased02WithoutRecentLineNumbers,
recentlyViewedDocuments: { includeLineNumbers: IncludeLineNumbersOption.WithSpaceAfter },
}));
expect(result.recentlyViewedDocuments?.includeLineNumbers).toBe(IncludeLineNumbersOption.None);
expect(result.currentFile?.includeLineNumbers).toBe(IncludeLineNumbersOption.WithoutSpace);
});

it('preserves undefined for option bags neither side specifies', () => {
const result = applyStrategyConfig(baseConfig({
promptingStrategy: PromptingStrategy.CopilotNesXtab,
}));
// CopilotNesXtab only sets includeTagsInCurrentFile; nested option bags should stay undefined.
expect(result.currentFile).toBeUndefined();
expect(result.recentlyViewedDocuments).toBeUndefined();
expect(result.lintOptions).toBeUndefined();
});
});
13 changes: 9 additions & 4 deletions extensions/copilot/src/platform/parser/node/docGenParsing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Node, TreeSitterOffsetRange } from './nodes';
import { _parse } from './parserWithCaching';
import { _getNodeMatchingSelection } from './selectionParsing';
import { WASMLanguage } from './treeSitterLanguages';
import { extractIdentifier, isDocumentableNode } from './util';
import { extractIdentifier, isDocumentableNode, unwrapPythonDecoratedDefinition } from './util';


export type NodeToDocumentContext = {
Expand Down Expand Up @@ -53,10 +53,11 @@ export async function _getNodeToDocument(
const selectionMatchedNode = isSelectionEmpty ? undefined : _getNodeMatchingSelection(treeRef.tree, selection, language);

if (selectionMatchedNode) {
const nodeIdentifier = extractIdentifier(selectionMatchedNode, language);
const unwrapped = unwrapPythonDecoratedDefinition(selectionMatchedNode, language);
const nodeIdentifier = extractIdentifier(unwrapped, language);
return {
nodeIdentifier,
nodeToDocument: Node.ofSyntaxNode(selectionMatchedNode),
nodeToDocument: Node.ofSyntaxNode(unwrapped),
nodeSelectionBy: 'matchingSelection'
};
}
Expand All @@ -72,6 +73,8 @@ export async function _getNodeToDocument(
++nNodesClimbedUp;
}

nodeToDocument = unwrapPythonDecoratedDefinition(nodeToDocument, language);

const nodeIdentifier = extractIdentifier(nodeToDocument, language);
return {
nodeIdentifier,
Expand All @@ -96,7 +99,9 @@ export async function _getDocumentableNodeIfOnIdentifier(
if (smallestNodeContainingRange.type.match(/identifier/) &&
(smallestNodeContainingRange.parent === null || isDocumentableNode(smallestNodeContainingRange.parent, language))
) {
const parent = smallestNodeContainingRange.parent;
const parent = smallestNodeContainingRange.parent === null
? null
: unwrapPythonDecoratedDefinition(smallestNodeContainingRange.parent, language);

const parentNodeRange = parent === null
? undefined
Expand Down
30 changes: 30 additions & 0 deletions extensions/copilot/src/platform/parser/node/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,37 @@ export function isDocumentableNode(node: SyntaxNode, language: WASMLanguage) {
return node.type.match(/definition|declaration|class_specifier/);
case WASMLanguage.Ruby:
return node.type.match(/module|class|method|assignment/);
case WASMLanguage.Python:
// Match `function_definition`/`class_definition`, plus the wrapping
// `decorated_definition` so that selections/cursors *on the decorator
// line itself* still resolve to the function/class (rather than
// climbing all the way up to `module`). Callers must unwrap a matched
// `decorated_definition` to its inner definition via
// {@link unwrapPythonDecoratedDefinition} so that downstream consumers
// (e.g. docstring generation) treat the `def`/`class` line — not the
// `@decorator` line — as the start of the definition. Otherwise
// docstrings end up *before* the decorator, which is a syntax error.
// See https://github.com/microsoft/vscode/issues/283165.
return node.type.match(/^(function_definition|class_definition|decorated_definition)$/);
default:
return node.type.match(/definition|declaration|declarator/);
}
}

/**
* In Python, a `decorated_definition` wraps a `function_definition` or
* `class_definition` whenever decorators are present. Its range starts at the
* `@decorator` line, which is *not* what callers want as "the node to
* document" — placing a docstring at that range would put it before the
* decorator (a syntax error). This helper unwraps a `decorated_definition` to
* its inner `function_definition`/`class_definition` so the range starts at
* the `def`/`class` keyword.
*
* See {@link isDocumentableNode} and https://github.com/microsoft/vscode/issues/283165.
*/
export function unwrapPythonDecoratedDefinition(node: SyntaxNode, language: WASMLanguage): SyntaxNode {
if (language !== WASMLanguage.Python || node.type !== 'decorated_definition') {
return node;
}
return node.children.find(c => c.type === 'function_definition' || c.type === 'class_definition') ?? node;
}
Loading
Loading