-
Notifications
You must be signed in to change notification settings - Fork 904
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: Apply various small fixes to task execution logic #436
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.
❌ Changes requested. Reviewed everything up to 277c168 in 1.0 minute and 2.5101609999999965 seconds
More details
- Looked at
586
lines of code in10
files - Skipped
1
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_4ON5NGRrPpUYdUL4
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.
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 72f6782 in 1.0 minute and 18.215592 seconds
More details
- Looked at
86
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. agents-api/agents_api/activities/task_steps/__init__.py:122
- Draft comment:
The change fromworkflow
togoto_workflow
in the return statement ofif_else_step
is correctly handled intask_execution.py
wheregoto_workflow
is accessed. This ensures consistency in data flow. - Reason this comment was not posted:
Confidence changes required:0%
The change in the return key fromworkflow
togoto_workflow
inif_else_step
function inagents_api/activities/task_steps/__init__.py
is reflected in thetask_execution.py
file where the output key is correctly accessed asgoto_workflow
. This ensures consistency in the data flow between the activity and the workflow that uses its output. This is a correct implementation and does not require any changes.
2. agents-api/agents_api/common/utils/json.py:41
- Draft comment:
TheCustomJSONEncoder
usesmodel_dump()
forBaseModel
instances. Ensure allBaseModel
subclasses implement this method or provide a fallback mechanism to avoidAttributeError
. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
3. agents-api/agents_api/routers/agents/create_agent.py:11
- Draft comment:
Removal ofutcnow
import is appropriate as there's no usage in this file. Ensure timestamps are handled correctly increate_agent_query
or by the database/model. - Reason this comment was not posted:
Confidence changes required:0%
Thecreate_agent
endpoint inagents_api/routers/agents/create_agent.py
has removed the import ofutcnow
from...common.utils.datetime
, but there is no usage ofutcnow
in the provided code snippet. This suggests that the removal is appropriate if the timestamp is handled elsewhere (likely within thecreate_agent_query
function or automatically by the database/model layer). This change seems justified and does not introduce any issues based on the provided code.
4. agents-api/agents_api/worker/__main__.py:80
- Draft comment:
Enablingyield_step
in the worker's main function aligns with the PR's intent to enable various task steps. Ensure that all necessary dependencies and configurations foryield_step
are correctly set up. - Reason this comment was not posted:
Confidence changes required:0%
Theyield_step
function inagents_api/worker/__main__.py
is enabled, and the corresponding activity is also enabled in the worker's main function. This is consistent with the PR's intent to enable and implement various task steps. The inclusion ofyield_step
in the worker's activity list ensures that it can be executed as part of the workflows that require it.
Workflow ID: wflow_3QwGNDvNjYyHFmuY
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Summary:
Applied various small fixes to task execution logic, including enabling and implementing workflow steps, updating models, adding logging, and error handling.
Key points:
yield_step
andif_else_step
inagents_api/activities/task_steps/__init__.py
.agents_api/autogen/openapi_model.py
to include new models and enums likeToolResponse
,ExecutionStatus
, andTransitionType
.data_converter
to Temporal client inagents_api/clients/temporal.py
.create_execution_query
inagents_api/models/execution/create_execution.py
to handle new statuses.create_agent
endpoint inagents_api/routers/agents/create_agent.py
to handle default values for optional fields.agents_api/routers/tasks/routers.py
.yield_step
inagents_api/worker/__main__.py
.if_else_step
andyield_step
inagents_api/workflows/task_execution.py
.agents-api/pyproject.toml
to includesimpleeval
.openapi.yaml
to reflect new models and endpoints.Generated with ❤️ by ellipsis.dev