Skip to content

Conversation

danishsjjd
Copy link

@danishsjjd danishsjjd commented May 15, 2025

Description

Added rightbrain block and rightbrain_run_task tool

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Tested the rightbrain_run_task tool and rightbrain block manually.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally and in CI (npm test)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have updated version numbers as needed (if needed)
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Security Considerations:

  • My changes do not introduce any new security vulnerabilities
  • I have considered the security implications of my changes

Additional Information:

Added the @rightbrain/sdk package, but it's currently used only for TypeScript types.

Copy link

vercel bot commented May 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 15, 2025 7:22pm

Copy link

vercel bot commented May 15, 2025

@pete001 is attempting to deploy a commit to the Sim Studio Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added Rightbrain integration to enable running AI tasks through Rightbrain's API, allowing users to deploy LLMs for specific tasks across applications and workflows.

  • The URL validation in /apps/sim/tools/rightbrain/run_task.ts needs improvement - currently processes URLs without validating structure before accessing array indices
  • Response transformation in run_task.ts should validate data structure before property access
  • Generic object type used in /apps/sim/tools/rightbrain/types.ts could be more specific for better type safety
  • Inconsistencies exist between type definitions in documentation (rightbrain.mdx) and implementation
  • The URL processing function in run_task.ts discards its return value, making the validation ineffective

💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!

10 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile

outputs: {
response: {
type: {
charged_credits: 'any',
Copy link
Contributor

Choose a reason for hiding this comment

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

style: charged_credits type should be more specific than 'any' - based on the Rightbrain API docs this should be a number

task_id: 'string',
task_revision_id: 'string',
total_tokens: 'number',
is_error: 'any',
Copy link
Contributor

Choose a reason for hiding this comment

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

style: is_error should be typed as boolean rather than 'any' for better type safety

Suggested change
is_error: 'any',
is_error: 'boolean',

Comment on lines +478 to +492
| Parameter | Type |
| ------------------------ | ------ |
| `charged_credits` | string |
| `created` | string |
| `id` | string |
| `input_processor_timing` | string |
| `input_tokens` | string |
| `llm_call_timing` | string |
| `output_tokens` | string |
| `response` | string |
| `run_data` | string |
| `task_id` | string |
| `task_revision_id` | string |
| `total_tokens` | string |
| `is_error` | string |
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Type inconsistency between docs and implementation - output parameters are documented as 'string' but some are actually 'number' or 'any' according to the block configuration

Suggested change
| Parameter | Type |
| ------------------------ | ------ |
| `charged_credits` | string |
| `created` | string |
| `id` | string |
| `input_processor_timing` | string |
| `input_tokens` | string |
| `llm_call_timing` | string |
| `output_tokens` | string |
| `response` | string |
| `run_data` | string |
| `task_id` | string |
| `task_revision_id` | string |
| `total_tokens` | string |
| `is_error` | string |
| Parameter | Type |
| ------------------------ | ------ |
| `charged_credits` | any |
| `created` | string |
| `id` | string |
| `input_processor_timing` | any |
| `input_tokens` | number |
| `llm_call_timing` | any |
| `output_tokens` | number |
| `response` | json |
| `run_data` | json |
| `task_id` | string |
| `task_revision_id` | string |
| `total_tokens` | number |
| `is_error` | any |

@@ -65,6 +65,7 @@
"@radix-ui/react-toggle": "^1.1.2",
"@radix-ui/react-tooltip": "^1.1.6",
"@react-email/components": "^0.0.34",
"@rightbrain/sdk": "^0.1.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider pinning to exact version (0.1.2) instead of using caret (^0.1.2) since this is a very early SDK version that may have breaking changes

Comment on lines +48 to +51
url: (params) => {
processUrl(params.url)
return params.url
},
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: processUrl() result is unused but contains important parsed values that could be needed later

Comment on lines +10 to +13
const baseUrl = urlInstance.pathname.split('/')
const orgId = baseUrl[4]
const projectId = baseUrl[6]
const taskId = baseUrl[8]
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: array access without length validation could throw if URL doesn't have expected path segments

Suggested change
const baseUrl = urlInstance.pathname.split('/')
const orgId = baseUrl[4]
const projectId = baseUrl[6]
const taskId = baseUrl[8]
const baseUrl = urlInstance.pathname.split('/')
if (baseUrl.length < 9) {
throw new Error('Invalid URL format: missing required path segments')
}
const orgId = baseUrl[4]
const projectId = baseUrl[6]
const taskId = baseUrl[8]


export interface RightBrainRunTaskParams {
url: string
inputs: object
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Using generic object type loses type information. Consider using a more specific type or making this generic with constraints.

}

export interface RightBrainRunTaskResponse extends ToolResponse {
output: TaskRun<object, object>
Copy link
Contributor

Choose a reason for hiding this comment

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

style: TaskRun<object, object> is too generic. Consider using more specific types or making the interface generic with appropriate constraints.

@waleedlatif1 waleedlatif1 force-pushed the main branch 5 times, most recently from 1b3f825 to b9b662b Compare May 21, 2025 16:13
@waleedlatif1 waleedlatif1 force-pushed the main branch 2 times, most recently from 5fec813 to b42c43a Compare June 28, 2025 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants