Skip to content

test: check for required params on tool defs #6501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
17 changes: 17 additions & 0 deletions core/tools/definitions/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
export { codebaseTool } from "./codebaseTool";
export { createNewFileTool } from "./createNewFile";
export { createRuleBlock } from "./createRuleBlock";
export { editFileTool } from "./editFile";
export { fetchUrlContentTool } from "./fetchUrlContent";
export { globSearchTool } from "./globSearch";
export { grepSearchTool } from "./grepSearch";
export { lsTool } from "./lsTool";
export { readCurrentlyOpenFileTool } from "./readCurrentlyOpenFile";
export { readFileTool } from "./readFile";
export { requestRuleTool } from "./requestRule";
export { runTerminalCommandTool } from "./runTerminalCommand";
export { searchAndReplaceInFileTool } from "./searchAndReplaceInFile";
export { searchWebTool } from "./searchWeb";
export { viewDiffTool } from "./viewDiff";
export { viewRepoMapTool } from "./viewRepoMap";
export { viewSubdirectoryTool } from "./viewSubdirectory";
54 changes: 54 additions & 0 deletions core/tools/definitions/toolDefinitions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { describe, expect, it } from "@jest/globals";
import { ConfigDependentToolParams, GetTool, Tool } from "../..";
import * as toolDefinitions from "./index";

describe("Tool Definitions", () => {
// Mock params for tools that need them
const mockParams: ConfigDependentToolParams = {
rules: [],
enableExperimentalTools: false,
};

// Helper function to get the actual tool object
const getToolObject = (toolDefinition: Tool | GetTool): Tool => {
if (typeof toolDefinition === "function") {
return toolDefinition(mockParams);
}
return toolDefinition;
};

it("should have all required parameters defined in properties for each tool", () => {
const exportedTools = Object.values(toolDefinitions);

exportedTools.forEach((toolDefinition) => {
const tool = getToolObject(toolDefinition);

// Each tool should have the required structure
expect(tool).toHaveProperty("type", "function");
expect(tool).toHaveProperty("function");
expect(tool.function).toHaveProperty("parameters");

const parameters = tool.function.parameters;

// If there are required parameters, they should be defined in properties
if (
parameters &&
parameters.required &&
Array.isArray(parameters.required)
) {
expect(parameters).toHaveProperty("properties");
expect(typeof parameters.properties).toBe("object");

// Check each required parameter is defined in properties
parameters.required.forEach((requiredParam: string) => {
expect(parameters.properties).toHaveProperty(requiredParam);

// Each property should have at least a type
const property = parameters.properties[requiredParam];
expect(property).toHaveProperty("type");
expect(typeof property.type).toBe("string");
});
}
});
});
});
53 changes: 20 additions & 33 deletions core/tools/index.ts
Original file line number Diff line number Diff line change
@@ -1,50 +1,37 @@
import { ConfigDependentToolParams, IDE, Tool } from "..";
import { codebaseTool } from "./definitions/codebaseTool";
import { createNewFileTool } from "./definitions/createNewFile";
import { createRuleBlock } from "./definitions/createRuleBlock";
import { editFileTool } from "./definitions/editFile";
import { fetchUrlContentTool } from "./definitions/fetchUrlContent";
import { globSearchTool } from "./definitions/globSearch";
import { grepSearchTool } from "./definitions/grepSearch";
import { lsTool } from "./definitions/lsTool";
import { readCurrentlyOpenFileTool } from "./definitions/readCurrentlyOpenFile";
import { readFileTool } from "./definitions/readFile";
import { requestRuleTool } from "./definitions/requestRule";
import { runTerminalCommandTool } from "./definitions/runTerminalCommand";
import { searchAndReplaceInFileTool } from "./definitions/searchAndReplaceInFile";
import { searchWebTool } from "./definitions/searchWeb";
import { viewDiffTool } from "./definitions/viewDiff";
import { viewRepoMapTool } from "./definitions/viewRepoMap";
import { viewSubdirectoryTool } from "./definitions/viewSubdirectory";

import * as toolDefinitions from "./definitions";
// I'm writing these as functions because we've messed up 3 TIMES by pushing to const, causing duplicate tool definitions on subsequent config loads.

// missing support for remote os calls: https://github.com/microsoft/vscode/issues/252269
const getLocalOnlyToolDefinitions = () => [grepSearchTool];
const getLocalOnlyToolDefinitions = () => [toolDefinitions.grepSearchTool];

const getBaseToolDefinitions = () => [
readFileTool,
createNewFileTool,
runTerminalCommandTool,
globSearchTool,
searchWebTool,
viewDiffTool,
readCurrentlyOpenFileTool,
lsTool,
createRuleBlock,
fetchUrlContentTool,
toolDefinitions.readFileTool,
toolDefinitions.createNewFileTool,
toolDefinitions.runTerminalCommandTool,
toolDefinitions.globSearchTool,
toolDefinitions.searchWebTool,
toolDefinitions.viewDiffTool,
toolDefinitions.readCurrentlyOpenFileTool,
toolDefinitions.lsTool,
toolDefinitions.createRuleBlock,
toolDefinitions.fetchUrlContentTool,
];

export const getConfigDependentToolDefinitions = (
params: ConfigDependentToolParams,
): Tool[] => [
requestRuleTool(params),
toolDefinitions.requestRuleTool(params),
// Search and replace is now generally available
searchAndReplaceInFileTool,
toolDefinitions.searchAndReplaceInFileTool,
// Keep edit file tool available for models that need it
editFileTool,
toolDefinitions.editFileTool,
...(params.enableExperimentalTools
? [viewRepoMapTool, viewSubdirectoryTool, codebaseTool]
? [
toolDefinitions.viewRepoMapTool,
toolDefinitions.viewSubdirectoryTool,
toolDefinitions.codebaseTool,
]
: []),
];

Expand Down
Loading