feat: update backend types #1082
Conversation
himanshu-dixit
requested review from
plxity and
utkarsh-dixit
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
This comment was generated by github-actions[bot]! JS SDK Coverage Report📊 Coverage report for JS SDK can be found at the following URL: 📁 Test report folder can be found at the following URL: |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 09c5f08 in 1 minute and 48 seconds
More details
- Looked at
6480lines of code in10files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. js/src/sdk/base.toolset.spec.ts:100
- Draft comment:
Typo: The propertysuccessfullis misspelled. It should besuccessful. This typo is also present in thetoolResponseobject. - Reason this comment was not posted:
Comment was on unchanged code.
2. js/src/sdk/client/client.ts:7
- Draft comment:
The change fromclientAuth: SDKClient.AuthServicetoclientAuth: SDKClient.ClientServicemight break existing code that relies onAuthService. Ensure that all references toAuthServiceare updated accordingly. - Reason this comment was not posted:
Comment did not seem useful.
3. js/src/cli/connections.ts:26
- Draft comment:
The methodgetConnectionshas been replaced withlistConnections. Ensure that this change is consistent across the codebase and that all references togetConnectionsare updated. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_GXGF6gSCr3abQWJf
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
js/src/sdk/client/types.gen.ts
Outdated
| * Whether the action execution was successfully executed or not. If this is false, error field will be populated with the error message. | ||
| * @deprecated | ||
| */ | ||
| successfull: boolean; |
There was a problem hiding this comment.
The successfull field (with double 'l') is marked as deprecated but still required in the type definition. This could cause confusion. Consider:
- Making it optional if it's deprecated
- Adding a migration guide for users to switch to
successful - Adding a timeline for when
successfullwill be removed
| @@ -1213,7 +2019,7 @@ export type AdvancedUseCaseSearchQueryDTO = { | |||
| * Use case is deprecated. Please provide this in the body instead to avoid max-uri-length error. | |||
| * @deprecated | |||
| */ | |||
| useCase: string; | |||
| useCase?: string; | |||
There was a problem hiding this comment.
The useCase parameter is marked as deprecated in the comment but the type definition doesn't reflect this. Consider:
- Adding
@deprecatedJSDoc tag - Making it optional in the type definition
- Adding a note about what to use instead
| /** | ||
| * The yaml of the app | ||
| */ | ||
| yaml?: unknown; | ||
| }; |
There was a problem hiding this comment.
The yaml field has been removed from SingleAppInfoResDTO but there's no deprecation notice or migration guide. If this is a breaking change, consider:
- Adding a deprecation cycle first
- Providing migration instructions
- Documenting alternative ways to access this information
| /** | ||
| * The type of error | ||
| */ | ||
| export type type = "BadRequestError"; |
There was a problem hiding this comment.
The type enum is defined multiple times with different values (type, type2, type3, type4, type5). This could lead to confusion and type safety issues. Consider:
- Using more descriptive names for these types (e.g.,
ErrorType,RequestBodyType, etc.) - Consolidating similar enums where possible
- Adding documentation to clarify the purpose of each type
Code Review SummaryPositive Aspects
Areas for Improvement
Recommendations
Overall, this is a solid improvement to the type system, but needs some refinement in deprecation handling and documentation. |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on f9b8813 in 1 minute and 42 seconds
More details
- Looked at
527lines of code in10files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. js/src/frameworks/vercel.ts:37
- Draft comment:
The type casting toRawActionDatais unnecessary here sinceactionSchemais already of typeRawActionData. Consider removing theas RawActionDatacast for cleaner code. - Reason this comment was not posted:
Confidence changes required:50%
The change fromTRawActionDatatoRawActionDatais consistent across the files, but the type casting ingenerateVercelToolandgetToolsmethods is unnecessary and could be removed for cleaner code.
2. js/src/sdk/actionRegistry.ts:61
- Draft comment:
The type casting toRawActionDatais unnecessary here sincecomposioSchemais already of typeRawActionData. Consider removing theas unknown as RawActionDatacast for cleaner code. - Reason this comment was not posted:
Confidence changes required:50%
The change fromTRawActionDatatoRawActionDatais consistent across the files, but the type casting ingenerateVercelToolandgetToolsmethods is unnecessary and could be removed for cleaner code.
3. js/src/sdk/actionRegistry.ts:95
- Draft comment:
The type casting toRawActionDatais unnecessary here sincecomposioSchemais already of typeRawActionData. Consider removing theas unknown as RawActionDatacast for cleaner code. - Reason this comment was not posted:
Confidence changes required:50%
The change fromTRawActionDatatoRawActionDatais consistent across the files, but the type casting ingenerateVercelToolandgetToolsmethods is unnecessary and could be removed for cleaner code.
4. js/src/sdk/actionRegistry.ts:106
- Draft comment:
The type casting toRawActionDatais unnecessary here sinceaction!.schemais already of typeRawActionData. Consider removing theas RawActionDatacast for cleaner code. - Reason this comment was not posted:
Confidence changes required:50%
The change fromTRawActionDatatoRawActionDatais consistent across the files, but the type casting ingenerateVercelToolandgetToolsmethods is unnecessary and could be removed for cleaner code.
Workflow ID: wflow_29qVsstx1tWdi94y
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 6375402 in 58 seconds
More details
- Looked at
66lines of code in5files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/frameworks/cloudflare.ts:11
- Draft comment:
Ensure that the import path forOptionalandSequenceis updated consistently across all files to../types/utilto avoid import errors. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_8lQvohpFmg8uezNf
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 52712ed in 1 minute and 33 seconds
More details
- Looked at
226lines of code in8files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. js/src/sdk/actionRegistry.spec.ts:91
- Draft comment:
The expected result should match theActionExecuteResponsetype. Adjust the test to expect the correct structure. - Reason this comment was not posted:
Marked as duplicate.
2. js/src/sdk/actionRegistry.spec.ts:124
- Draft comment:
The expected result should match theActionExecuteResponsetype. Adjust the test to expect the correct structure. - Reason this comment was not posted:
Marked as duplicate.
3. js/src/sdk/actionRegistry.spec.ts:27
- Draft comment:
The expected result should match theActionExecuteResponsetype. Adjust the test to expect the correct structure. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_1GwNrGdz19a2u08p
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -76,7 +76,7 @@ describe("ActionRegistry", () => { | |||
| description: "This is a test action", | |||
| inputParams: params, | |||
| callback: async function () { | |||
| return { success: true }; | |||
| return { data: { success: true }, successful: true }; | |||
There was a problem hiding this comment.
The expected result should match the ActionExecuteResponse type. Adjust the test to expect the correct structure.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on fdfe3ea in 1 minute and 15 seconds
More details
- Looked at
50lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/frameworks/langchain.spec.ts:73
- Draft comment:
There's a typo in the test expectation. It should besuccessfulinstead ofsuccessfull.
expect(actionOuput).toHaveProperty("successful", true);
- Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_LmyCjgpiA3LqsSBC
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 290e7ad in 2 minutes and 22 seconds
More details
- Looked at
47lines of code in4files - Skipped
1files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/examples/market_research_agent/package.json:2
- Draft comment:
Inconsistent versioning ofcomposio-coreacross different examples. Ensure consistent versions to avoid compatibility issues. This is also applicable innewsletter_twitter_threads/package.json,scheduling-agent/package.json, andvercel/nextjs-vercel-demo/package.json. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_7RQSPDEWdGau8Qou
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on de804f7 in 1 minute and 6 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/sdk/base.toolset.ts:50
- Draft comment:
The default value forapiKeyhas been changed tonull. Ensure this change is intentional and does not affect the expected behavior whenapiKeyis not provided. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_11RebFKSe5cxhCeV
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 9a71bfe in 1 minute and 3 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. .github/workflows/examples_js.yml:66
- Draft comment:
Consider updating the lockfile to ensure consistency in dependencies across environments. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_2wzP62zXVQ4qXAxS
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on bd22018 in 2 minutes and 39 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. .github/workflows/examples_js.yml:66
- Draft comment:
Including 'langchain' alongside '@langchain/core' and '@langchain/openai' may be redundant and could lead to version conflicts. Consider removing 'langchain' if it's not needed separately. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_sfsiG6XrdrSeDfQF
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
…io into ft-update-backend-type
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on b45089d in 1 minute and 16 seconds
More details
- Looked at
98lines of code in5files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/sdk/base.toolset.spec.ts:42
- Draft comment:
TheappNameparameter is removed from theTSchemaProcessortype inbase_toolset.tsbut is still present here. This can lead to type errors. Please removeappNamefrom theTSchemaProcessorusage here. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_Av6oG16XJuTfwAM8
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
getConnectionstolistConnectionsinconnections.ts.Rename
executeActionProxyV2toexecuteWithHttpClientinactionRegistry.ts.Rename
AuthServicetoClientServiceinclient.ts.HttpError,BadRequestErrorinschemas.gen.ts.Update DTOs with new fields and descriptions in
schemas.gen.ts.Update types to reflect new API changes in
types.gen.ts.services.gen.tsto align with new API endpoints.Entity.ts,actions.ts, andconnectedAccounts.tsto use updated API methods.This description was created by
<https://www.ellipsis.dev?ref=ComposioHQ%2Fcomposio&utm_source=github&utm_medium=referral%7Chttps://img.shields.io/badge/Ellipsis-blue?color=175173%7CEllipsis><sup> for 52712ed. It will automatically update as commits are pushed.
Important
Refactor backend types and update API methods to align with new specifications, including renaming functions and updating models.
getConnectionstolistConnectionsinconnections.ts.executeActionProxyV2toexecuteWithHttpClientinactionRegistry.ts.AuthServicetoClientServiceinclient.ts.HttpError,BadRequestErrorinschemas.gen.ts.schemas.gen.ts.types.gen.ts.services.gen.tsto align with new API endpoints.Entity.ts,actions.ts, andconnectedAccounts.tsto use updated API methods.This description was created by
for b45089d. It will automatically update as commits are pushed.