-
Notifications
You must be signed in to change notification settings - Fork 900
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
fix(agents-api & integrations-service): Fix Browserbase/RemoteBrowser integrations #784
Conversation
Hey @HamadaSalhab, here is an example of how you can ask me to improve this pull request: @Sweep Add unit tests for the `get_browserbase_client` function in `browserbase.py` to verify it correctly initializes the Browserbase client with the new optional parameters (project_id, api_url, connect_url). 📖 For more information on how to use Sweep, please read our documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 90d7b30 in 46 seconds
More details
- Looked at
877
lines of code in12
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. agents-api/agents_api/autogen/Tools.py:1147
- Draft comment:
Changingconnect_url
fromAnyUrl
tostr
removes automatic URL validation. Ensure URL validation is handled elsewhere to prevent invalid URLs. - Reason this comment was not posted:
Comment did not seem useful.
2. integrations-service/integrations/autogen/Tools.py:1147
- Draft comment:
Changingconnect_url
fromAnyUrl
tostr
removes automatic URL validation. Ensure URL validation is handled elsewhere to prevent invalid URLs. - Reason this comment was not posted:
Marked as duplicate.
3. integrations-service/integrations/utils/integrations/browserbase.py:110
- Draft comment:
Consider logging the exception details in theexcept
block for better debugging and traceability. - Reason this comment was not posted:
Confidence changes required:50%
The exception handling incomplete_session
should log the exception details for better debugging and traceability.
Workflow ID: wflow_wJIVgWLKllCCvs4C
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Made some fixes. Tested |
Made changes so that a fully typed JSON object is returned by Brave Search . This fixes issue #771 <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Parse Brave Search JSON output into `SearchResult` objects and update `BraveSearchOutput` to return a list of these objects. > > - **Behavior**: > - `search()` in `brave.py` now parses JSON results into `SearchResult` objects. > - `BraveSearchOutput` now returns a list of `SearchResult` instead of a single string. > - **Models**: > - Adds `SearchResult` model in `brave.py` with `title`, `link`, and `snippet` fields. > - Updates `BraveSearchOutput` in `brave.py` to use `List[SearchResult]` for `result`. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=julep-ai%2Fjulep&utm_source=github&utm_medium=referral)<sup> for 61e813a. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN --> --------- Co-authored-by: Diwank Singh Tomer <[email protected]> Co-authored-by: vedantsahai18 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 9ab7f86 in 22 seconds
More details
- Looked at
31
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. integrations-service/integrations/models/execution.py:32
- Draft comment:
BrowserbaseContextOutput
is commented out here and inExecutionResponse
. Ensure this is intentional and not an oversight, as the PR description mentions adding it tobrowserbase.py
. - Reason this comment was not posted:
Confidence changes required:50%
The comment is about the removal ofBrowserbaseContextOutput
from the imports and usage inExecutionResponse
. This is consistent with the PR description, which mentions addingBrowserbaseContextOutput
tobrowserbase.py
, but it seems to be commented out here. This might be intentional, but it's worth pointing out in case it was overlooked.
Workflow ID: wflow_ETLdqnE3i3CsipE0
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Signed-off-by: Diwank Singh Tomer <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 4e77450 in 30 seconds
More details
- Looked at
373
lines of code in8
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_AGUarFvuxTQA0tp6
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on a18f853 in 25 seconds
More details
- Looked at
77
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. integrations-service/integrations/models/browserbase.py:6
- Draft comment:
The import ofSession
frombrowserbase
is unused and can be removed to clean up the code. - Reason this comment was not posted:
Confidence changes required:10%
The import statement forSession
frombrowserbase
is unnecessary inintegrations-service/integrations/models/browserbase.py
since it is not used in the file.
2. integrations-service/integrations/utils/integrations/browserbase.py:28
- Draft comment:
The import ofSession
frombrowserbase
is unused and can be removed to clean up the code. - Reason this comment was not posted:
Confidence changes required:10%
The import statement forSession
frombrowserbase
is unnecessary inintegrations-service/integrations/utils/integrations/browserbase.py
since it is not used in the file.
Workflow ID: wflow_AcF09iyyckkfStS4
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
…-ai/julep into x/browserbase-integrations
…-ai/julep into x/browserbase-integrations
…-ai/julep into x/browserbase-integrations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on 40734ad in 32 seconds
More details
- Looked at
34
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_VxPIDIggKNwYAjRo
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.
Important
Fixes Browserbase and RemoteBrowser integrations by adding new fields, making arguments optional, and changing URL types.
project_id
,api_url
, andconnect_url
fields toBrowserbaseSetup
andBrowserbaseSetupUpdate
inTools.py
.arguments
optional in severalIntegrationDef
classes inTools.py
.connect_url
type fromAnyUrl
tostr
inRemoteBrowserSetup
andRemoteBrowserSetupUpdate
inTools.py
.SessionInfo
,BrowserbaseCreateSessionOutput
, andBrowserbaseGetSessionOutput
inbrowserbase.py
to allowNone
for several fields.BrowserbaseContextOutput
tobrowserbase.py
.ExecutionArguments
andExecutionResponse
inexecution.py
to include new Browserbase and RemoteBrowser models.get_integrations.py
andexecute_integration.py
.project_id
,api_url
, andconnect_url
toget_browserbase_client()
inbrowserbase.py
.This description was created by for 40734ad. It will automatically update as commits are pushed.