Skip to content
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.

Sign up for GitHub

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

feat: switch from client.actions to getToolsSchema #1174

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+58 −21
Open

feat: switch from client.actions to getToolsSchema #1174

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8e4f421
feat: switch from client.actions to getToolsSchema
himanshu-dixit Jan 9, 2025
0eda3b1
Merge branch 'master' into ft-vercel-get-tool-schema
himanshu-dixit Jan 9, 2025
806f9e1
feat: remove trailing slash if exist
himanshu-dixit Jan 10, 2025
755d346
Merge branch 'ft-vercel-get-tool-schema' of github.com:ComposioHQ/com…
himanshu-dixit Jan 10, 2025
d42dfda
Merge branch 'master' into ft-vercel-get-tool-schema
himanshu-dixit Jan 10, 2025
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
32 changes: 32 additions & 0 deletions js/src/frameworks/vercel.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { beforeAll, describe, expect, it } from "@jest/globals";
import { z } from "zod";
import { getTestConfig } from "../../config/getTestConfig";
import { ActionExecuteResponse } from "../sdk/models/actions";
import { VercelAIToolSet } from "./vercel";

describe("Apps class tests", () => {
Expand All @@ -25,4 +27,34 @@ describe("Apps class tests", () => {
});
expect(Object.keys(tools).length).toBe(1);
});

it("Should create custom action to star a repository", async () => {
await vercelAIToolSet.createAction({
actionName: "starRepositoryCustomAction",
toolName: "github",
Comment on lines 27 to +34
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Quality: The added test case for creating a custom action to star a repository is a good addition. However, ensure that the test case is complete and follows the existing test structure. Consider adding assertions to verify the expected behavior of the createAction method. This will enhance the test's robustness and ensure it validates the intended functionality effectively.

🔧 Suggested Code Diff: 
  it("Should create custom action to star a repository", async () => {
    await vercelAIToolSet.createAction({
      actionName: "starRepositoryCustomAction",
      toolName: "github",
+     // Add assertions to verify the action was created successfully
+     const action = tools.getAction("starRepositoryCustomAction");
+     expect(action).toBeDefined();
+     expect(action.toolName).toBe("github");
    });
  });
📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
});
expect(Object.keys(tools).length).toBe(1);
});
it("Should create custom action to star a repository", async () => {
await vercelAIToolSet.createAction({
actionName: "starRepositoryCustomAction",
toolName: "github",
it("Should create custom action to star a repository", async () => {
await vercelAIToolSet.createAction({
actionName: "starRepositoryCustomAction",
toolName: "github"
});
// Add assertions to verify the action was created successfully
const action = tools.getAction("starRepositoryCustomAction");
expect(action).toBeDefined();
expect(action.toolName).toBe("github");
});

description: "This action stars a repository",
inputParams: z.object({
owner: z.string(),
repo: z.string(),
}),
callback: async (
inputParams,
_authCredentials,
executeRequest
): Promise<ActionExecuteResponse> => {
const res = await executeRequest({
endpoint: `/user/starred/${inputParams.owner}/${inputParams.repo}`,
method: "PUT",
parameters: [],
});
return res;
},
});

const tools = await vercelAIToolSet.getTools({
actions: ["starRepositoryCustomAction"],
});

await expect(Object.keys(tools).length).toBe(1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test case could benefit from additional assertions to verify the created action's properties. Consider adding checks for:

  1. The returned tool's description
  2. The input parameter schema structure
  3. The actual functionality by making a mock request

This would ensure the action is created correctly with all expected properties.

});
});
39 changes: 19 additions & 20 deletions js/src/frameworks/vercel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { jsonSchema, tool } from "ai";
import { CoreTool, jsonSchema, tool } from "ai";
import { z } from "zod";
import { ComposioToolSet as BaseComposioToolSet } from "../sdk/base.toolset";
import { TELEMETRY_LOGGER } from "../sdk/utils/telemetry";
Expand Down Expand Up @@ -62,7 +62,7 @@ export class VercelAIToolSet extends BaseComposioToolSet {
useCase?: Optional<string>;
usecaseLimit?: Optional<number>;
filterByAvailableApps?: Optional<boolean>;
}): Promise<{ [key: string]: RawActionData }> {
}): Promise<{ [key: string]: CoreTool }> {
TELEMETRY_LOGGER.manualTelemetry(TELEMETRY_EVENTS.SDK_METHOD_INVOKED, {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of tools should be CoreTool[] instead of an object, as the getToolsSchema returns an array of CoreTool.

method: "getTools",
file: this.fileName,
Expand All @@ -78,25 +78,24 @@ export class VercelAIToolSet extends BaseComposioToolSet {
actions,
} = ZExecuteToolCallParams.parse(filters);

const actionsList = await this.client.actions.list({
...(apps && { apps: apps?.join(",") }),
...(tags && { tags: tags?.join(",") }),
...(useCase && { useCase: useCase }),
...(actions && { actions: actions?.join(",") }),
...(usecaseLimit && { usecaseLimit: usecaseLimit }),
filterByAvailableApps: filterByAvailableApps ?? undefined,
});

const tools = {};
actionsList.items?.forEach((actionSchema) => {
// @ts-ignore
tools[actionSchema.name!] = this.generateVercelTool(
// @ts-ignore
actionSchema as ActionData
);
});
const tools = await this.getToolsSchema(
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding error handling for the getToolsSchema call. If the API call fails, it could lead to an unhandled rejection. Something like:

const tools = await this.getToolsSchema(
  {
    apps,
    tags,
    useCase,
    filterByAvailableApps,
    actions,
    useCaseLimit: usecaseLimit,
  },
  this.entityId
).catch(error => {
  TELEMETRY_LOGGER.error('Failed to fetch tools schema', { error });
  throw error;
});

apps,
tags,
useCase,
filterByAvailableApps,
actions,
useCaseLimit: usecaseLimit,
},
this.entityId
);

return tools;
return Object.fromEntries(
tools.map((tool) => [
tool.name,
this.generateVercelTool(tool as RawActionData),
])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type cast to RawActionData here could be unsafe. Consider adding a type guard or validation to ensure the tool object matches the RawActionData interface before casting. This would prevent potential runtime errors if the tool schema doesn't match expected format.

);
}

async executeToolCall(
Expand Down
Loading