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
62 changes: 58 additions & 4 deletions src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,38 @@ interface ICopilotFileToolArgs {
path: string;
}

/**
* Parameters for the `view` tool. The Copilot CLI accepts an optional
* `view_range: [startLine, endLine]` (1-based, inclusive). `endLine` may be
* `-1` to mean "to end of file".
*/
interface ICopilotViewToolArgs extends ICopilotFileToolArgs {
view_range?: number[];
}

/**
* Normalizes a `view_range` array. Returns `undefined` unless the array has
* exactly two integer elements with `startLine >= 0`. `endLine === -1` is
* preserved as the "to end of file" sentinel; otherwise `endLine` must be
* `>= startLine`.
*/
function formatViewRange(view_range: number[] | undefined): { startLine: number; endLine: number } | undefined {
if (!Array.isArray(view_range) || view_range.length !== 2) {
return undefined;
}
const [startLine, endLine] = view_range;
if (!Number.isInteger(startLine) || !Number.isInteger(endLine)) {
return undefined;
}
if (startLine < 0) {
return undefined;
}
if (endLine !== -1 && endLine < startLine) {
return undefined;
}
return { startLine, endLine };
}

/** Parameters for the `grep` tool. */
interface ICopilotGrepToolArgs {
pattern: string;
Expand Down Expand Up @@ -212,9 +244,20 @@ export function getInvocationMessage(toolName: string, displayName: string, para

switch (toolName) {
case CopilotToolName.View: {
const args = parameters as ICopilotFileToolArgs | undefined;
const args = parameters as ICopilotViewToolArgs | undefined;
if (args?.path) {
return md(localize('toolInvoke.viewFile', "Reading {0}", formatPathAsMarkdownLink(args.path)));
const link = formatPathAsMarkdownLink(args.path);
const range = formatViewRange(args.view_range);
if (range) {
if (range.endLine === -1) {
return md(localize('toolInvoke.viewFileFromLine', "Reading {0}, line {1} to the end", link, range.startLine));
}
if (range.endLine !== range.startLine) {
return md(localize('toolInvoke.viewFileRange', "Reading {0}, lines {1} to {2}", link, range.startLine, range.endLine));
}
return md(localize('toolInvoke.viewFileLine', "Reading {0}, line {1}", link, range.startLine));
}
return md(localize('toolInvoke.viewFile', "Reading {0}", link));
Comment thread
roblourens marked this conversation as resolved.
}
return localize('toolInvoke.view', "Reading file");
}
Expand Down Expand Up @@ -267,9 +310,20 @@ export function getPastTenseMessage(toolName: string, displayName: string, param

switch (toolName) {
case CopilotToolName.View: {
const args = parameters as ICopilotFileToolArgs | undefined;
const args = parameters as ICopilotViewToolArgs | undefined;
if (args?.path) {
return md(localize('toolComplete.viewFile', "Read {0}", formatPathAsMarkdownLink(args.path)));
const link = formatPathAsMarkdownLink(args.path);
const range = formatViewRange(args.view_range);
if (range) {
if (range.endLine === -1) {
return md(localize('toolComplete.viewFileFromLine', "Read {0}, line {1} to the end", link, range.startLine));
}
if (range.endLine !== range.startLine) {
return md(localize('toolComplete.viewFileRange', "Read {0}, lines {1} to {2}", link, range.startLine, range.endLine));
}
return md(localize('toolComplete.viewFileLine', "Read {0}, line {1}", link, range.startLine));
}
return md(localize('toolComplete.viewFile', "Read {0}", link));
}
return localize('toolComplete.view', "Read file");
}
Expand Down
51 changes: 50 additions & 1 deletion src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import assert from 'assert';
import { URI } from '../../../../base/common/uri.js';
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
import { getPermissionDisplay, type ITypedPermissionRequest } from '../../node/copilot/copilotToolDisplay.js';
import { getInvocationMessage, getPastTenseMessage, getPermissionDisplay, type ITypedPermissionRequest } from '../../node/copilot/copilotToolDisplay.js';

suite('getPermissionDisplay — cd-prefix stripping', () => {

Expand Down Expand Up @@ -75,3 +75,52 @@ suite('getPermissionDisplay — cd-prefix stripping', () => {
assert.strictEqual(display.toolInput, 'dir');
});
});

suite('view tool — view_range display', () => {

ensureNoDisposablesAreLeakedInTestSuite();

function invocation(parameters: Record<string, unknown> | undefined): string {
const result = getInvocationMessage('view', 'View File', parameters);
return typeof result === 'string' ? result : result.markdown;
}

function pastTense(parameters: Record<string, unknown> | undefined): string {
const result = getPastTenseMessage('view', 'View File', parameters, true);
return typeof result === 'string' ? result : result.markdown;
}

test('renders path-only when view_range is absent', () => {
assert.ok(invocation({ path: '/repo/file.ts' }).startsWith('Reading ['));
assert.ok(pastTense({ path: '/repo/file.ts' }).startsWith('Read ['));
});

test('renders "lines X to Y" for a valid two-element range', () => {
assert.ok(invocation({ path: '/repo/file.ts', view_range: [10, 20] }).endsWith(', lines 10 to 20'));
assert.ok(pastTense({ path: '/repo/file.ts', view_range: [10, 20] }).endsWith(', lines 10 to 20'));
});

test('renders "line X" when start === end', () => {
assert.ok(invocation({ path: '/repo/file.ts', view_range: [10, 10] }).endsWith(', line 10'));
assert.ok(pastTense({ path: '/repo/file.ts', view_range: [10, 10] }).endsWith(', line 10'));
});

test('renders "line X to the end" for the -1 EOF sentinel', () => {
assert.ok(invocation({ path: '/repo/file.ts', view_range: [10, -1] }).endsWith(', line 10 to the end'));
assert.ok(pastTense({ path: '/repo/file.ts', view_range: [10, -1] }).endsWith(', line 10 to the end'));
});

test('falls back to path-only for invalid ranges', () => {
// end < start (and not -1)
assert.ok(!invocation({ path: '/repo/file.ts', view_range: [20, 10] }).includes(','));
// negative start
assert.ok(!invocation({ path: '/repo/file.ts', view_range: [-5, 10] }).includes(','));
// non-integer
assert.ok(!invocation({ path: '/repo/file.ts', view_range: [1.5, 10] }).includes(','));
// wrong arity
assert.ok(!invocation({ path: '/repo/file.ts', view_range: [10] }).includes(','));
assert.ok(!invocation({ path: '/repo/file.ts', view_range: [10, 20, 30] }).includes(','));
// non-array
assert.ok(!invocation({ path: '/repo/file.ts', view_range: 'whatever' }).includes(','));
});
});
Loading