Skip to content

Commit 0f9570c

Browse files
alexr00alexr00
committed
parseToolCallDetails 100% coverage and remove unreachable code
1 parent 0beb87e commit 0f9570c

3 files changed

Lines changed: 197 additions & 13 deletions

File tree

‎common/sessionParsing.ts‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -192,15 +192,7 @@ export function parseToolCallDetails(
192192
const filePath = args.path;
193193
let fileLabel = filePath ? toFileLabel(filePath) : undefined;
194194

195-
if (fileLabel === undefined) {
196-
fileLabel = filePath;
197-
198-
return {
199-
toolName: fileLabel ? (`Read ${fileLabel}` + (parsedRange ? `, lines ${parsedRange.start} to ${parsedRange.end}` : '')) : 'Read repository',
200-
invocationMessage: fileLabel ? (`Read ${fileLabel}` + (parsedRange ? `, lines ${parsedRange.start} to ${parsedRange.end}` : '')) : 'Read repository',
201-
pastTenseMessage: fileLabel ? (`Read ${fileLabel}` + (parsedRange ? `, lines ${parsedRange.start} to ${parsedRange.end}` : '')) : 'Read repository',
202-
};
203-
} else if (fileLabel === '') {
195+
if (fileLabel === undefined || fileLabel === '') {
204196
return {
205197
toolName: 'Read repository',
206198
invocationMessage: 'Read repository',
@@ -250,7 +242,7 @@ export function parseToolCallDetails(
250242
filePath: filePath,
251243
fileLabel: fileLabel,
252244
} : undefined
253-
}
245+
};
254246
} else if (name === 'create') {
255247
const filePath = args.path;
256248
const fileLabel = filePath && toFileLabel(filePath);
@@ -264,7 +256,7 @@ export function parseToolCallDetails(
264256
filePath: filePath,
265257
fileLabel: fileLabel,
266258
} : undefined
267-
}
259+
};
268260
} else if (name === 'view') {
269261
const filePath = args.path;
270262
const fileLabel = filePath && toFileLabel(filePath);
@@ -319,12 +311,12 @@ export function parseToolCallDetails(
319311
return {
320312
toolName: 'read_bash',
321313
invocationMessage: 'Read logs from Bash session'
322-
}
314+
};
323315
} else if (name === 'stop_bash') {
324316
return {
325317
toolName: 'stop_bash',
326318
invocationMessage: 'Stop Bash session'
327-
}
319+
};
328320
} else {
329321
// Unknown tool type
330322
return {

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4179,6 +4179,7 @@
41794179
"clean": "rm -r dist/",
41804180
"compile": "webpack --mode development --env esbuild",
41814181
"compile:test": "tsc -p tsconfig.test.json",
4182+
"watch:test": "tsc -w -p tsconfig.test.json",
41824183
"compile:node": "webpack --mode development --config-name extension:node --config-name webviews",
41834184
"compile:web": "webpack --mode development --config-name extension:webworker --config-name webviews",
41844185
"lint": "eslint --fix --cache --config .eslintrc.js --ignore-pattern src/env/browser/**/* \"{src,webviews}/**/*.{ts,tsx}\"",

‎src/test/common/sessionParsing.test.ts‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,22 @@ another non-data line`;
6868
});
6969

7070
describe('parseToolCallDetails()', function () {
71+
it('should handle empty arguments string (covers ternary else)', function () {
72+
// forces the ternary at line ~165 in sessionParsing.ts to take the else branch
73+
const toolCall = {
74+
function: {
75+
name: 'str_replace_editor',
76+
arguments: '' // empty string -> falsy -> args stays {}
77+
},
78+
id: 'call_empty_args',
79+
type: 'function',
80+
index: 0
81+
};
82+
83+
const result = parseToolCallDetails(toolCall as any, '');
84+
assert.strictEqual(result.toolName, 'Edit');
85+
assert.strictEqual(result.invocationMessage, 'Edit');
86+
});
7187
it('should parse str_replace_editor tool calls with view command', function () {
7288
const toolCall = {
7389
function: {
@@ -148,6 +164,23 @@ another non-data line`;
148164
assert.strictEqual(result.invocationMessage, 'I need to analyze this code');
149165
});
150166

167+
it('should default think tool call to Thought when no args.thought and no content', function () {
168+
const toolCall = {
169+
function: {
170+
name: 'think',
171+
arguments: '{}' // no thought provided
172+
},
173+
id: 'call_790',
174+
type: 'function',
175+
index: 0
176+
};
177+
178+
// Pass empty string content so code falls back to 'Thought'
179+
const result = parseToolCallDetails(toolCall, '');
180+
assert.strictEqual(result.toolName, 'think');
181+
assert.strictEqual(result.invocationMessage, 'Thought');
182+
});
183+
151184
it('should parse report_progress tool calls', function () {
152185
const toolCall = {
153186
function: {
@@ -166,6 +199,41 @@ another non-data line`;
166199
assert.strictEqual(result.originMessage, 'Commit: feat: add new tests');
167200
});
168201

202+
it('report_progress falls back to content when prDescription empty string', function () {
203+
// prDescription provided but empty => falsy, so chain uses content
204+
const toolCall = {
205+
function: {
206+
name: 'report_progress',
207+
arguments: '{"prDescription": ""}'
208+
},
209+
id: 'call_102',
210+
type: 'function',
211+
index: 0
212+
};
213+
214+
const fallbackContent = 'Using content as progress update';
215+
const result = parseToolCallDetails(toolCall, fallbackContent);
216+
assert.strictEqual(result.toolName, 'Progress Update');
217+
assert.strictEqual(result.invocationMessage, fallbackContent);
218+
});
219+
220+
it('report_progress falls back to default when prDescription and content empty', function () {
221+
// Both prDescription (empty string) and content ('') are falsy => 'Progress Update'
222+
const toolCall = {
223+
function: {
224+
name: 'report_progress',
225+
arguments: '{"prDescription": ""}'
226+
},
227+
id: 'call_103',
228+
type: 'function',
229+
index: 0
230+
};
231+
232+
const result = parseToolCallDetails(toolCall, '');
233+
assert.strictEqual(result.toolName, 'Progress Update');
234+
assert.strictEqual(result.invocationMessage, 'Progress Update');
235+
});
236+
169237
it('should handle unknown tool types', function () {
170238
const toolCall = {
171239
function: {
@@ -251,6 +319,25 @@ another non-data line`;
251319
assert.ok(result.toolSpecificData && 'command' in result.toolSpecificData);
252320
});
253321

322+
it('handles str_replace_editor view with diff-parsed content and no range (parsedRange undefined)', function () {
323+
// This exercises the branch where parsedRange is falsy so no ", lines X to Y" suffix is appended
324+
const diff = [
325+
'diff --git a/home/runner/work/repo/repo/src/another/file.ts b/home/runner/work/repo/repo/src/another/file.ts',
326+
'index aaaaaaa..bbbbbbb 100644',
327+
'--- a/home/runner/work/repo/repo/src/another/file.ts',
328+
'+++ b/home/runner/work/repo/repo/src/another/file.ts',
329+
'@@ -1,2 +1,2 @@',
330+
'-old line',
331+
'+new line'
332+
].join('\n');
333+
const toolCall = makeToolCall('str_replace_editor', { command: 'view' }); // no view_range provided
334+
const result = parseToolCallDetails(toolCall, diff);
335+
assert.strictEqual(result.toolName, 'Read');
336+
assert.ok(result.invocationMessage.includes('src/another/file.ts'));
337+
assert.ok(!/lines \d+ to \d+/.test(result.invocationMessage), 'invocationMessage should not contain line range');
338+
assert.ok(result.pastTenseMessage && result.pastTenseMessage === result.invocationMessage);
339+
});
340+
254341
it('handles str_replace_editor view with path but unparsable diff content (no diff headers)', function () {
255342
const content = 'just some file content without diff headers';
256343
const toolCall = makeToolCall('str_replace_editor', { command: 'view', path: '/home/runner/work/repo/repo/src/other.ts' });
@@ -259,6 +346,38 @@ another non-data line`;
259346
assert.strictEqual(result.invocationMessage, 'Read src/other.ts');
260347
});
261348

349+
it('handles str_replace_editor view with path and range (parsedRange defined)', function () {
350+
// This covers the branch in sessionParsing.ts lines ~202-212 where parsedRange is defined
351+
// and a normal file path (no diff content) is provided so invocationMessage includes the lines suffix.
352+
const toolCall = makeToolCall('str_replace_editor', { command: 'view', path: '/home/runner/work/repo/repo/src/ranged.ts', view_range: [4, 9] });
353+
const result = parseToolCallDetails(toolCall, 'plain file content');
354+
assert.strictEqual(result.toolName, 'Read');
355+
assert.strictEqual(result.invocationMessage, 'Read src/ranged.ts, lines 4 to 9');
356+
assert.strictEqual(result.pastTenseMessage, 'Read src/ranged.ts, lines 4 to 9');
357+
assert.ok(result.toolSpecificData && 'viewRange' in result.toolSpecificData, 'Expected viewRange in toolSpecificData');
358+
if (result.toolSpecificData && 'viewRange' in result.toolSpecificData) {
359+
assert.strictEqual(result.toolSpecificData.viewRange?.start, 4);
360+
assert.strictEqual(result.toolSpecificData.viewRange?.end, 9);
361+
}
362+
});
363+
364+
it('handles str_replace_editor view with diff hunk but no diff header (fileA undefined)', function () {
365+
// This diff content has an @@ hunk so parseDiff returns an object, but no 'diff --git' header,
366+
// therefore fileA and fileB remain undefined. This exercises the fallback at line ~177 where
367+
// file is chosen via parsedContent.fileA ?? parsedContent.fileB resulting in undefined and thus
368+
// a repository-level read.
369+
const diffOnlyHunk = [
370+
'@@ -1,2 +1,2 @@',
371+
'-old line',
372+
'+new line'
373+
].join('\n');
374+
const toolCall = makeToolCall('str_replace_editor', { command: 'view', view_range: [1, 2] });
375+
const result = parseToolCallDetails(toolCall, diffOnlyHunk);
376+
// fileLabel is undefined so toolName is 'Read' but invocation message falls back to 'Read repository'
377+
assert.strictEqual(result.toolName, 'Read');
378+
assert.strictEqual(result.invocationMessage, 'Read repository');
379+
});
380+
262381
it('handles str_replace_editor view with undefined path (no label)', function () {
263382
const toolCall = makeToolCall('str_replace_editor', { command: 'view' });
264383
const result = parseToolCallDetails(toolCall, 'plain content');
@@ -279,6 +398,18 @@ another non-data line`;
279398
assert.ok(result.invocationMessage.includes('lines 5 to 15'));
280399
});
281400

401+
it('handles str_replace_editor edit when args.command is undefined (defaults to edit)', function () {
402+
// Covers sessionParsing.ts lines 220-230 where args.command || 'edit' supplies default
403+
const toolCall = makeToolCall('str_replace_editor', { path: '/home/runner/work/repo/repo/src/implicitEdit.ts' });
404+
const result = parseToolCallDetails(toolCall, '');
405+
assert.strictEqual(result.toolName, 'Edit');
406+
assert.strictEqual(result.invocationMessage, 'Edit [](src/implicitEdit.ts)');
407+
assert.ok(result.toolSpecificData && 'command' in result.toolSpecificData, 'Expected toolSpecificData for edit operation');
408+
if (result.toolSpecificData && 'command' in result.toolSpecificData) {
409+
assert.strictEqual(result.toolSpecificData.command, 'edit'); // default applied
410+
}
411+
});
412+
282413
it('handles str_replace (non-editor) path missing label fallback', function () {
283414
// Provide a path that toFileLabel will still shorten; assert structure
284415
const toolCall = makeToolCall('str_replace', { path: '/home/runner/work/repo/repo/src/x.ts' });
@@ -287,13 +418,32 @@ another non-data line`;
287418
assert.strictEqual(result.invocationMessage, 'Edit [](src/x.ts)');
288419
});
289420

421+
it('handles str_replace with undefined path (fileLabel undefined)', function () {
422+
// No path provided -> filePath undefined -> fileLabel undefined, should fall back to `Edit ${filePath}` which is 'Edit undefined'
423+
const toolCall = makeToolCall('str_replace', { /* no path */ });
424+
const result = parseToolCallDetails(toolCall, '');
425+
assert.strictEqual(result.toolName, 'Edit');
426+
assert.strictEqual(result.invocationMessage, 'Edit undefined');
427+
assert.strictEqual(result.pastTenseMessage, 'Edit undefined');
428+
assert.strictEqual(result.toolSpecificData, undefined);
429+
});
430+
290431
it('handles create tool call', function () {
291432
const toolCall = makeToolCall('create', { path: '/home/runner/work/repo/repo/new/file.txt' });
292433
const result = parseToolCallDetails(toolCall, '');
293434
assert.strictEqual(result.toolName, 'Create');
294435
assert.strictEqual(result.invocationMessage, 'Create [](new/file.txt)');
295436
});
296437

438+
it('handles create tool call without path (fileLabel undefined)', function () {
439+
const toolCall = makeToolCall('create', { /* no path provided */ });
440+
const result = parseToolCallDetails(toolCall, '');
441+
assert.strictEqual(result.toolName, 'Create');
442+
assert.strictEqual(result.invocationMessage, 'Create File undefined');
443+
assert.strictEqual(result.pastTenseMessage, 'Create File undefined');
444+
assert.strictEqual(result.toolSpecificData, undefined);
445+
});
446+
297447
it('handles view tool call (non str_replace_editor) with range and root path giving repository label', function () {
298448
const toolCall = makeToolCall('view', { path: '/home/runner/work/repo/repo/', view_range: [2, 3] });
299449
const result = parseToolCallDetails(toolCall, '');
@@ -308,6 +458,17 @@ another non-data line`;
308458
assert.ok(result.invocationMessage.includes('lines 3 to 7'));
309459
});
310460

461+
it('handles view tool call (non str_replace_editor) with file path and no range (parsedRange undefined)', function () {
462+
// Covers lines 261-275 in sessionParsing.ts where parsedRange is falsy, so
463+
// the ", lines X to Y" suffix should NOT be appended.
464+
const toolCall = makeToolCall('view', { path: '/home/runner/work/repo/repo/src/noRange.ts' });
465+
const result = parseToolCallDetails(toolCall, 'file content');
466+
assert.strictEqual(result.toolName, 'Read');
467+
assert.ok(result.invocationMessage === 'Read [](src/noRange.ts)', 'invocationMessage should not contain line range');
468+
assert.ok(result.pastTenseMessage === 'Read [](src/noRange.ts)', 'pastTenseMessage should not contain line range');
469+
assert.ok(result.toolSpecificData && 'viewRange' in result.toolSpecificData && !result.toolSpecificData.viewRange, 'viewRange should be undefined');
470+
});
471+
311472
it('handles bash tool call without command (only content)', function () {
312473
const toolCall = makeToolCall('bash', {});
313474
const result = parseToolCallDetails(toolCall, 'only output');
@@ -316,6 +477,14 @@ another non-data line`;
316477
assert.ok(!result.toolSpecificData); // no command so no toolSpecificData
317478
});
318479

480+
it('handles bash tool call without command and without content (fallback to default message)', function () {
481+
// Exercises bashContent empty so code uses 'Run Bash command' fallback (lines ~292-300)
482+
const toolCall = makeToolCall('bash', {});
483+
const result = parseToolCallDetails(toolCall, '');
484+
assert.strictEqual(result.toolName, 'Run Bash command');
485+
assert.strictEqual(result.invocationMessage, 'Run Bash command');
486+
});
487+
319488
it('handles read_bash tool call', function () {
320489
const toolCall = makeToolCall('read_bash', {});
321490
const result = parseToolCallDetails(toolCall, 'ignored');
@@ -337,6 +506,19 @@ another non-data line`;
337506
assert.strictEqual(result.invocationMessage, 'mystery_tool');
338507
});
339508

509+
it('handles unknown tool call with falsy name (empty string) returning unknown', function () {
510+
// Directly craft toolCall without using makeToolCall so we can force empty name
511+
const toolCall = {
512+
function: { name: '', arguments: '{}' },
513+
id: 'call_empty_name',
514+
type: 'function',
515+
index: 0
516+
};
517+
const result = parseToolCallDetails(toolCall as any, '');
518+
assert.strictEqual(result.toolName, 'unknown');
519+
assert.strictEqual(result.invocationMessage, 'unknown');
520+
});
521+
340522
it('gracefully handles invalid JSON arguments for non-view str_replace_editor (edit path undefined)', function () {
341523
const toolCall = {
342524
function: { name: 'str_replace_editor', arguments: '{"command": "edit", invalid' },
@@ -349,6 +531,15 @@ another non-data line`;
349531
assert.strictEqual(result.toolName, 'Edit');
350532
assert.strictEqual(result.invocationMessage, 'Edit');
351533
});
534+
535+
it('handles str_replace_editor view with no path and no range (fileLabel undefined branch)', function () {
536+
// Triggers the branch where args.path is undefined and thus fileLabel is undefined
537+
const toolCall = makeToolCall('str_replace_editor', { command: 'view', path: '' });
538+
const result = parseToolCallDetails(toolCall, 'plain non-diff content');
539+
assert.strictEqual(result.toolName, 'Read repository');
540+
assert.strictEqual(result.invocationMessage, 'Read repository');
541+
assert.strictEqual(result.pastTenseMessage, 'Read repository');
542+
});
352543
});
353544

354545
describe('parseDiff()', function () {

0 commit comments

Comments
 (0)