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.

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: add LlamaIndex TS support to JS SDK #1178

Open
wants to merge 6 commits into
base: master
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
1 change: 1 addition & 0 deletions js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"@langchain/openai": "^0.2.5",
"ai": "^3.2.22",
"langchain": "^0.2.11",
"llamaindex": "^0.8.31",

Choose a reason for hiding this comment

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

The plus signs in the added line are redundant and should be removed.

📝 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
"llamaindex": "^0.8.31",
"llamaindex": "^0.8.31",

"openai": "^4.50.0"
},
"dependencies": {
Expand Down
75 changes: 75 additions & 0 deletions js/src/frameworks/llamaindex.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { beforeAll, describe, expect, it } from "@jest/globals";
import { z } from "zod";
import { getTestConfig } from "../../config/getTestConfig";
import { ActionExecuteResponse } from "../sdk/models/actions";
import { LlamaIndexToolSet } from "./llamaindex";

describe("Apps class tests", () => {
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 suite description Apps class tests is misleading as it's testing the LlamaIndexToolSet class. Consider renaming it to LlamaIndexToolSet tests for better clarity.

let llamaindexToolSet: LlamaIndexToolSet;
beforeAll(() => {
llamaindexToolSet = new LlamaIndexToolSet({
apiKey: getTestConfig().COMPOSIO_API_KEY,
baseUrl: getTestConfig().BACKEND_HERMES_URL,
});
});

it("getools", async () => {
const tools = await llamaindexToolSet.getTools({
apps: ["github"],
});
expect(tools).toBeInstanceOf(Array);
});
Comment on lines +16 to +21

Choose a reason for hiding this comment

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

The test getools in llamaindex.spec.ts doesn't assert anything meaningful. It only checks the type of the return, not the content.

Comment on lines +16 to +21

Choose a reason for hiding this comment

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

The test getools in llamaindex.spec.ts doesn't assert anything meaningful. It only checks if the returned value is an array, but not its contents. This test should verify the expected tools are returned.

it("check if tools are coming", async () => {
const tools = await llamaindexToolSet.getTools({
actions: ["GITHUB_GITHUB_API_ROOT"],
});
expect(tools.length).toBe(1);
});
it("check if getTools, actions are coming", async () => {
const tools = await llamaindexToolSet.getTools({
actions: ["GITHUB_GITHUB_API_ROOT"],
});

expect(tools.length).toBe(1);
});
Comment on lines +22 to +34

Choose a reason for hiding this comment

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

Redundant tests in llamaindex.spec.ts. The tests "check if tools are coming" and "check if getTools, actions are coming" are effectively identical. Consolidate these into a single test.

it("Should create custom action to star a repository", async () => {
await llamaindexToolSet.createAction({
actionName: "starRepositoryCustomAction",
toolName: "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;
},
});
Comment on lines +36 to +56

Choose a reason for hiding this comment

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

The createAction call in the test Should create custom action to star a repository lacks proper error handling. If the action creation fails, the test will not fail and the subsequent getTools and executeAction calls might behave unexpectedly.


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

await expect(tools.length).toBe(1);
const actionOuput = await llamaindexToolSet.executeAction({
action: "starRepositoryCustomAction",
params: {
owner: "composioHQ",
repo: "composio",
},
entityId: "default",
connectedAccountId: "9442cab3-d54f-4903-976c-ee67ef506c9b",
Comment on lines +66 to +70

Choose a reason for hiding this comment

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

The test Should create custom action to star a repository in llamaindex.spec.ts uses hardcoded values for connectedAccountId and the repository details. This makes the test less robust and prone to failures if these values change.

});

expect(actionOuput).toHaveProperty("successful", true);
});
Comment on lines +35 to +74

Choose a reason for hiding this comment

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

The test suite in llamaindex.spec.ts is missing a test to verify that the createAction method adds a new action correctly. The current test only checks if the number of tools increased but not the properties of the new tool/action.

});
Comment on lines +1 to +75

Choose a reason for hiding this comment

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

Redundant plus signs should be removed.

91 changes: 91 additions & 0 deletions js/src/frameworks/llamaindex.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { FunctionTool, type JSONValue } from "llamaindex";
import { z } from "zod";
import { ComposioToolSet as BaseComposioToolSet } from "../sdk/base.toolset";
import { COMPOSIO_BASE_URL } from "../sdk/client/core/OpenAPI";
import { TELEMETRY_LOGGER } from "../sdk/utils/telemetry";
import { TELEMETRY_EVENTS } from "../sdk/utils/telemetry/events";
import { ZToolSchemaFilter } from "../types/base_toolset";
import { Optional, Sequence } from "../types/util";

type ToolSchema = {
name: string;
description: string;
parameters: Record<string, unknown>;
};

export class LlamaIndexToolSet extends BaseComposioToolSet {
/**
* Composio toolset for LlamaIndex framework.
*
*/
static FRAMEWORK_NAME = "llamaindex";
static DEFAULT_ENTITY_ID = "default";
fileName: string = "js/src/frameworks/llamaindex.ts";

constructor(
config: {
apiKey?: Optional<string>;
baseUrl?: Optional<string>;
entityId?: string;
runtime?: string;
} = {}
) {
super({
apiKey: config.apiKey || null,
baseUrl: config.baseUrl || COMPOSIO_BASE_URL,
runtime: config?.runtime || null,
entityId: config.entityId || LlamaIndexToolSet.DEFAULT_ENTITY_ID,
});
}

private _wrapTool(
Copy link
Collaborator

Choose a reason for hiding this comment

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

The _wrapTool method accepts schema: any as a parameter type. Consider creating a proper interface for better type safety:

interface ToolSchema {
  name: string;
  description: string;
  parameters: {
    properties?: Record<string, unknown>;
    required?: string[];
  };
}

schema: ToolSchema,
entityId: Optional<string> = null
): FunctionTool<Record<string, unknown>, JSONValue | Promise<JSONValue>> {
return FunctionTool.from(
async (params: Record<string, unknown>) => {
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 JSON parsing operation:

try {
  return JSON.parse(JSON.stringify(result)) as JSONValue;
} catch (error) {
  throw new Error(`Failed to parse tool result: ${error.message}`);
}

This would provide better error messages and make debugging easier.

const result = await this.executeAction({
action: schema.name,
params,
entityId: entityId || this.entityId,
});
return JSON.parse(JSON.stringify(result)) as JSONValue;
},
{
name: schema.name,
description: schema.description,
parameters: {
type: "object",
properties: schema.parameters.properties || {},
required: schema.parameters.required || [],
Comment on lines +57 to +60

Choose a reason for hiding this comment

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

The getTools function in llamaindex.ts does not correctly handle the case where schema.parameters is undefined. This can lead to a runtime error when trying to access properties or required.

},
Comment on lines +57 to +61

Choose a reason for hiding this comment

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

The getTools function in llamaindex.ts does not correctly handle the case when schema.parameters is null or undefined, which could lead to a runtime error.

Comment on lines +57 to +61

Choose a reason for hiding this comment

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

The getTools function in llamaindex.ts does not correctly handle the case where parameters is undefined in the tool schema. This could lead to a runtime error when trying to access schema.parameters.properties or schema.parameters.required.

}
);
}

async getTools(
filters: z.infer<typeof ZToolSchemaFilter> = {},
entityId: Optional<string> = null
): Promise<
Sequence<
FunctionTool<Record<string, unknown>, JSONValue | Promise<JSONValue>>
>
> {
TELEMETRY_LOGGER.manualTelemetry(TELEMETRY_EVENTS.SDK_METHOD_INVOKED, {
method: "getTools",
file: this.fileName,
params: { filters, entityId },
});

const tools = await this.getToolsSchema(filters, entityId);
return tools.map((tool) => {
const wrappedTool = this._wrapTool(tool, entityId || this.entityId);
Object.assign(wrappedTool, {
name: tool.name,
description: tool.description,
parameters: tool.parameters,
});
return wrappedTool;
});
}
}
Comment on lines +1 to +91

Choose a reason for hiding this comment

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

Redundant plus signs should be removed.

2 changes: 2 additions & 0 deletions js/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { CloudflareToolSet } from "./frameworks/cloudflare";
import { LangchainToolSet } from "./frameworks/langchain";
import { LangGraphToolSet } from "./frameworks/langgraph";
import { LlamaIndexToolSet } from "./frameworks/llamaindex";
import { OpenAIToolSet } from "./frameworks/openai";
import { VercelAIToolSet } from "./frameworks/vercel";
import { ComposioToolSet } from "./sdk/base.toolset";
Expand All @@ -26,6 +27,7 @@ export {
ConnectionRequest,
LangGraphToolSet,
LangchainToolSet,
LlamaIndexToolSet,
OpenAIToolSet,
VercelAIToolSet,
};
Loading