-
Notifications
You must be signed in to change notification settings - Fork 817
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
make observer thought artifacts work #1423
Conversation
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 29268b6 in 25 seconds
More details
- Looked at
112
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. skyvern/forge/sdk/db/client.py:2007
- Draft comment:
Consider includingorganization_id
in theNotFoundError
message for better debugging context. - Reason this comment was not posted:
Confidence changes required:50%
Theupdate_observer_thought
method inclient.py
raises aNotFoundError
with a message that doesn't include theorganization_id
. This could make debugging difficult if the error is raised because theorganization_id
doesn't match. Including theorganization_id
in the error message would provide more context.
2. skyvern/forge/sdk/services/observer_service.py:231
- Draft comment:
Consider defaultingint_max_iterations_override
toDEFAULT_MAX_ITERATIONS
ifmax_iterations_override
is not set or is invalid. - Reason this comment was not posted:
Confidence changes required:50%
Therun_observer_cruise
function inobserver_service.py
has a potential issue with theint_max_iterations_override
variable. Ifmax_iterations_override
is not provided or is not an integer,int_max_iterations_override
remainsNone
, which could lead to unexpected behavior in the loop. It would be safer to defaultint_max_iterations_override
toDEFAULT_MAX_ITERATIONS
if it's not set.
Workflow ID: wflow_y3RMqz5yPAAWt2nk
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! Reviewed everything up to 29268b6 in 1 minute and 21 seconds
More details
- Looked at
112
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. skyvern/forge/sdk/db/client.py:2007
- Draft comment:
Ensureorganization_id
is checked before raisingNotFoundError
to prevent unauthorized access to observer thoughts from different organizations. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_DAITk1myuvsHmqaE
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
<!-- ELLIPSIS_HIDDEN --> > [!IMPORTANT] > Enhance observer thought handling by adding `OBSERVER_THOUGHT` log entity type and updating observer thought processes in `client.py` and `observer_service.py`. > > - **Behavior**: > - Add `OBSERVER_THOUGHT` to `LogEntityType` in `models.py`. > - Implement `update_observer_thought()` in `client.py` to update observer thoughts with fields like `workflow_run_block_id`, `observation`, `thought`, and `answer`. > - Modify `run_observer_cruise()` in `observer_service.py` to create and update observer thoughts during the observer cruise process. > - **Database**: > - Add `update_observer_thought()` method in `client.py` to handle updates to `ObserverThoughtModel`. > - **Service**: > - Update `run_observer_cruise()` in `observer_service.py` to use `create_observer_thought()` and `update_observer_thought()` for handling observer thoughts. > - **Misc**: > - Add `OBSERVER_THOUGHT` to `EntityType` in `agent_protocol.py`. > > <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=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for fd92b851c149a2d27cec0fc4121e6e020c950c99. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
29268b6
to
44f93d1
Compare
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 44f93d1 in 23 seconds
More details
- Looked at
112
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. skyvern/forge/sdk/artifact/models.py:87
- Draft comment:
AddOBSERVER_THOUGHT
toLogEntityType
to match the PR description and ensure consistency.
OBSERVER_THOUGHT = "observer_thought"
- Reason this comment was not posted:
Comment position is out of bounds for file skyvern/forge/sdk/artifact/models.py; line 88 but file only has 87 lines
Workflow ID: wflow_SpyWcc03TqrTc0r1
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Important
Enhance observer thought handling by adding
OBSERVER_THOUGHT
log entity type and updating observer thought processes inclient.py
andobserver_service.py
.OBSERVER_THOUGHT
toLogEntityType
inmodels.py
.update_observer_thought()
inclient.py
to update observer thoughts with fields likeworkflow_run_block_id
,observation
,thought
, andanswer
.run_observer_cruise()
inobserver_service.py
to create and update observer thoughts during the observer cruise process.update_observer_thought()
method inclient.py
to handle updates toObserverThoughtModel
.run_observer_cruise()
inobserver_service.py
to usecreate_observer_thought()
andupdate_observer_thought()
for handling observer thoughts.OBSERVER_THOUGHT
toEntityType
inagent_protocol.py
.This description was created by for 44f93d1. It will automatically update as commits are pushed.