Skip to content

Commit b4e37c5

Browse files
swapnil-nagarswapnil-nagar
committed
Code review comments
1 parent d227818 commit b4e37c5

File tree

5 files changed

+52
-35
lines changed

5 files changed

+52
-35
lines changed

‎src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export { InvocationContext } from './InvocationContext';
1616
export * as output from './output';
1717
export * as trigger from './trigger';
1818
export { Disposable } from './utils/Disposable';
19-
export { toolProp } from './utils/toolProperties';
19+
export { toolProperty } from './utils/toolProperties';
2020

2121
export enum SqlChangeOperation {
2222
Insert = 0,

‎src/utils/toolProperties.ts‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,19 @@ export class ToolPropertyBuilder implements McpToolProperty {
7474
return this;
7575
}
7676

77+
/**
78+
* Set the property type to double
79+
*/
80+
double(): ToolPropertyBuilder {
81+
this.property.propertyType = 'double';
82+
return this;
83+
}
84+
7785
/**
7886
* Set the description for the property
7987
* @param description - Description of the property's purpose
8088
*/
81-
desc(description: string): ToolPropertyBuilder {
89+
describe(description: string): ToolPropertyBuilder {
8290
this.property.description = description;
8391
return this;
8492
}
@@ -126,16 +134,16 @@ export class ToolPropertyBuilder implements McpToolProperty {
126134
* const toolProperties = {
127135
* snippetName: toolProp
128136
* .string()
129-
* .desc("Some Description"),
137+
* .describe("Some Description"),
130138
*
131139
* optionalField: toolProp
132140
* .number()
133-
* .desc("Optional number field")
141+
* .describe("Optional number field")
134142
* .optional(),
135143
* };
136144
* ```
137145
*/
138-
export const toolProp = {
146+
export const toolProperty = {
139147
/**
140148
* Start building a string property
141149
*/
@@ -170,6 +178,13 @@ export const toolProp = {
170178
long(): ToolPropertyBuilder {
171179
return new ToolPropertyBuilder().long();
172180
},
181+
182+
/**
183+
* Start building a double property
184+
*/
185+
double(): ToolPropertyBuilder {
186+
return new ToolPropertyBuilder().double();
187+
},
173188
};
174189

175190
/**

‎test/converters/toMcpToolTriggerOptionsToRpc.test.ts‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import 'mocha';
55
import { expect } from 'chai';
66
import { converToMcpToolTriggerOptionsToRpc } from '../../src/converters/toMcpToolTriggerOptionsToRpc';
7-
import { toolProp } from '../../src/utils/toolProperties';
7+
import { toolProperty } from '../../src/utils/toolProperties';
88
import { McpToolProperty, McpToolTriggerOptions, ToolProps } from '../../types/mcpTool';
99

1010
describe('converToMcpToolTriggerOptionsToRpc', () => {
@@ -90,8 +90,8 @@ describe('converToMcpToolTriggerOptionsToRpc', () => {
9090
describe('toolProps object format', () => {
9191
it('should handle toolProps object format', () => {
9292
const toolProperties: ToolProps = {
93-
name: toolProp.string().desc('The name of the item'),
94-
age: toolProp.number().desc('The age of the person').optional(),
93+
name: toolProperty.string().describe('The name of the item'),
94+
age: toolProperty.number().describe('The age of the person').optional(),
9595
};
9696

9797
const input: McpToolTriggerOptions = {
@@ -116,11 +116,11 @@ describe('converToMcpToolTriggerOptionsToRpc', () => {
116116

117117
it('should handle all supported property types', () => {
118118
const toolProperties: ToolProps = {
119-
stringProp: toolProp.string().desc('A string property'),
120-
numberProp: toolProp.number().desc('A number property').optional(),
121-
booleanProp: toolProp.boolean().desc('A boolean property'),
122-
objectProp: toolProp.object().desc('An object property').optional(),
123-
longProp: toolProp.long().desc('A long property'),
119+
stringProp: toolProperty.string().describe('A string property'),
120+
numberProp: toolProperty.number().describe('A number property').optional(),
121+
booleanProp: toolProperty.boolean().describe('A boolean property'),
122+
objectProp: toolProperty.object().describe('An object property').optional(),
123+
doubleProp: toolProperty.double().describe('A double property'),
124124
};
125125

126126
const input: McpToolTriggerOptions = {
@@ -140,8 +140,8 @@ describe('converToMcpToolTriggerOptionsToRpc', () => {
140140

141141
it('should handle array properties correctly', () => {
142142
const toolProperties: ToolProps = {
143-
stringArray: toolProp.string().desc('A string array').asArray().optional(),
144-
numberArray: toolProp.number().desc('A number array').asArray(),
143+
stringArray: toolProperty.string().describe('A string array').asArray().optional(),
144+
numberArray: toolProperty.number().describe('A number array').asArray(),
145145
};
146146

147147
const input: McpToolTriggerOptions = {
@@ -312,7 +312,7 @@ describe('converToMcpToolTriggerOptionsToRpc', () => {
312312
describe('JSON serialization', () => {
313313
it('should produce valid JSON in toolProperties', () => {
314314
const toolProperties: ToolProps = {
315-
test: toolProp.string().desc('Test property'),
315+
test: toolProperty.string().describe('Test property'),
316316
};
317317

318318
const input: McpToolTriggerOptions = {
@@ -352,7 +352,9 @@ describe('converToMcpToolTriggerOptionsToRpc', () => {
352352
describe('edge cases', () => {
353353
it('should handle properties with special characters', () => {
354354
const toolProperties: ToolProps = {
355-
'special-field_with$symbols': toolProp.string().desc('A property with special characters in name'),
355+
'special-field_with$symbols': toolProperty
356+
.string()
357+
.describe('A property with special characters in name'),
356358
};
357359

358360
const input: McpToolTriggerOptions = {
@@ -372,7 +374,7 @@ describe('converToMcpToolTriggerOptionsToRpc', () => {
372374

373375
it('should handle properties with non-empty descriptions', () => {
374376
const toolProperties: ToolProps = {
375-
validDesc: toolProp.string().desc('A valid description'),
377+
validDesc: toolProperty.string().describe('A valid description'),
376378
};
377379

378380
const input: McpToolTriggerOptions = {
@@ -391,7 +393,7 @@ describe('converToMcpToolTriggerOptionsToRpc', () => {
391393
});
392394
it('should handle whitespace in names and descriptions', () => {
393395
const toolProperties: ToolProps = {
394-
' spaced name ': toolProp.string().desc(' spaced description ').optional(),
396+
' spaced name ': toolProperty.string().describe(' spaced description ').optional(),
395397
};
396398

397399
const input: McpToolTriggerOptions = {

‎test/toolArrayProperties.test.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,21 @@ import { expect } from 'chai';
66
import {
77
convertToolProperties,
88
normalizeToolProperties,
9-
toolProp,
9+
toolProperty,
1010
ToolPropertyBuilder,
1111
} from '../src/utils/toolProperties';
1212

1313
describe('isArray property support', () => {
1414
it('asArray() method sets isArray to true', () => {
15-
const stringArrayProp = toolProp.string().asArray().desc('String array');
15+
const stringArrayProp = toolProperty.string().asArray().describe('String array');
1616
expect(stringArrayProp.isArray).to.equal(true);
1717
expect(stringArrayProp.propertyType).to.equal('string');
1818
expect(stringArrayProp.description).to.equal('String array');
1919
expect(stringArrayProp.isRequired).to.equal(true);
2020
});
2121

2222
it('regular properties have isArray set to false', () => {
23-
const regularProp = toolProp.string().desc('Regular string').optional();
23+
const regularProp = toolProperty.string().describe('Regular string').optional();
2424
expect(regularProp.isArray).to.equal(false);
2525
expect(regularProp.propertyType).to.equal('string');
2626
expect(regularProp.description).to.equal('Regular string');
@@ -29,9 +29,9 @@ describe('isArray property support', () => {
2929

3030
it('convertToolProperties preserves isArray property', () => {
3131
const toolProps = {
32-
stringArray: toolProp.string().asArray().desc('String array'),
33-
numberArray: toolProp.number().asArray().desc('Number array').optional(),
34-
regularProp: toolProp.boolean().desc('Regular boolean'),
32+
stringArray: toolProperty.string().asArray().describe('String array'),
33+
numberArray: toolProperty.number().asArray().describe('Number array').optional(),
34+
regularProp: toolProperty.boolean().describe('Regular boolean'),
3535
};
3636

3737
const converted = convertToolProperties(toolProps);
@@ -79,11 +79,11 @@ describe('isArray property support', () => {
7979
});
8080

8181
it('all property types support asArray()', () => {
82-
const stringArray = toolProp.string().asArray().desc('String array');
83-
const numberArray = toolProp.number().asArray().desc('Number array');
84-
const booleanArray = toolProp.boolean().asArray().desc('Boolean array');
85-
const objectArray = toolProp.object().asArray().desc('Object array');
86-
const longArray = toolProp.long().asArray().desc('Long array');
82+
const stringArray = toolProperty.string().asArray().describe('String array');
83+
const numberArray = toolProperty.number().asArray().describe('Number array');
84+
const booleanArray = toolProperty.boolean().asArray().describe('Boolean array');
85+
const objectArray = toolProperty.object().asArray().describe('Object array');
86+
const longArray = toolProperty.long().asArray().describe('Long array');
8787

8888
expect(stringArray.isArray).to.equal(true);
8989
expect(numberArray.isArray).to.equal(true);
@@ -95,8 +95,8 @@ describe('isArray property support', () => {
9595
it('supports seamless property access after desc()', () => {
9696
// Test the specific pattern from user's example - this should work seamlessly
9797
const toolProperties = {
98-
name: toolProp.string().desc('Required property to identify the caller.').optional(),
99-
arrayT: toolProp.string().asArray().desc('An array of strings property.'),
98+
name: toolProperty.string().describe('Required property to identify the caller.').optional(),
99+
arrayT: toolProperty.string().asArray().describe('An array of strings property.'),
100100
};
101101

102102
expect(toolProperties.name).to.have.property('propertyType', 'string');
@@ -112,12 +112,12 @@ describe('isArray property support', () => {
112112
expect(() => {
113113
// Missing propertyType should throw when propertyType getter is accessed
114114
const builder = new ToolPropertyBuilder();
115-
builder.desc('Some description');
115+
builder.describe('Some description');
116116
return builder.propertyType; // This should throw
117117
}).to.throw('Property type must be specified');
118118

119119
// Missing description should default to empty string when description getter is accessed
120-
const builder = toolProp.string();
120+
const builder = toolProperty.string();
121121
expect(builder.description).to.equal(''); // Should default to empty string
122122
});
123123
});

‎types/index.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export * from './warmup';
3131
export * from './webpubsub';
3232

3333
// Export toolProp for fluent tool properties API
34-
export { toolProp } from '../src/utils/toolProperties';
34+
export { toolProperty } from '../src/utils/toolProperties';
3535

3636
/**
3737
* Void if no `return` output is registered

0 commit comments

Comments
 (0)