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 33 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
10895d3
feat: add support for llamaindex (TS)
abhishekpatil4 Jan 10, 2025
f33976d
fix: llamaindex tests
abhishekpatil4 Jan 10, 2025
b7a8481
feat: add LlamaIndex TS framework support
abhishekpatil4 Jan 10, 2025
769477f
fix: added type to schema
abhishekpatil4 Jan 10, 2025
57535ff
fix: lint
abhishekpatil4 Jan 10, 2025
d602522
fix: add llamaindex to peerDependencies
abhishekpatil4 Jan 10, 2025
2d8a8ba
feat: SP
abhishekpatil4 Jan 20, 2025
3a8fb8e
Feat/docs monitor logs (#1172)
abhishekpatil4 Jan 9, 2025
8e6f898
fix: trend finder agent added and existing python examples were fixed…
Prat011 Jan 9, 2025
10775fd
feat: remove trailing slash (#1176)
himanshu-dixit Jan 10, 2025
42ff618
feat: add docs on how to setup redirect URL (#1177)
abhishekpatil4 Jan 10, 2025
d545188
feat: add plugin tests (#1096)
tushar-composio Jan 13, 2025
3734266
feat: support custom actions in vercel toolkit (with type safety impr…
nicolas-angelo Jan 13, 2025
3401f19
feat: prettier and fail test
himanshu-dixit Jan 13, 2025
4b539a3
fix: use app filter when fetching integrations for `initiate_connecti…
angrybayblade Jan 13, 2025
f73d58e
fix: ENG-3490 (#1186)
himanshu-dixit Jan 14, 2025
3c8aeb7
fix: langchain and remove failing test check (#1187)
himanshu-dixit Jan 14, 2025
d562d3f
Add autogen tools new version (#1190)
kaavee315 Jan 14, 2025
1c65530
Update release (#1191)
kaavee315 Jan 14, 2025
6905ceb
feat: add refresh_token in AuthConnectionParamsModel (#1192)
utkarsh-dixit Jan 15, 2025
54d08ce
fix: Ensure Sentry is initialized for non logged in users too (#1183)
tushar-composio Jan 15, 2025
d34e19c
feat/update readme (#1188)
abhishekpatil4 Jan 15, 2025
d444e54
feat: make test env agnostic (#1197)
himanshu-dixit Jan 15, 2025
c61edc4
feat: change TEST_ENVIRONMENT -> TEST_ENV and log base url for CI (#…
himanshu-dixit Jan 15, 2025
0eb60c7
feat: integration list, reinitiate and update schema (#1189)
himanshu-dixit Jan 16, 2025
37c7054
feat: Added helicone for caching openai response (#1202)
plxity Jan 16, 2025
8647237
fix: enforce specific NodeJS version (#943)
plxity Jan 16, 2025
ee56d22
feat: crypto kit agents added (#1205)
Prat011 Jan 17, 2025
9b5db7f
feat: triggerId -> triggerName, triggerInstanceId, add methods for co…
himanshu-dixit Jan 18, 2025
bd7906e
fix: allow auto reconnection in pusher client (#1199)
kaavee315 Jan 20, 2025
058121e
fix: filter connections by entity [ENG-2523] (#1200)
kaavee315 Jan 20, 2025
b08fcd1
fix: session ID delegation (#1212)
angrybayblade Jan 20, 2025
2cc4b51
Merge branch 'master' into feat/llamaindex-ts
abhishekpatil4 Jan 20, 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
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