feat(agents-api): Set/get steps based on workflow state #484
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![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 cb8ce3e in 1 minute and 12 seconds
More details
- Looked at
2148lines of code in38files - Skipped
2files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. agents-api/agents_api/activities/task_steps/set_value_step.py:26
- Draft comment:
Consider raising the exception after logging it to ensure the caller is aware of the failure. - Reason this comment was not posted:
Marked as duplicate.
2. agents-api/agents_api/workflows/task_execution/__init__.py:141
- Draft comment:
Consider raising the exception after logging it to ensure the caller is aware of the failure. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The code already raises an exception after logging, which is exactly what the comment suggests. Therefore, the comment is redundant and not needed.
I might be overlooking a subtlety in the comment's suggestion, such as a specific type of exception or additional context that should be included.
The comment is generic and does not specify any additional context or exception type, so it seems redundant given the current code implementation.
The comment is unnecessary because the code already raises an exception after logging, fulfilling the comment's suggestion. The comment should be removed.
3. agents-api/agents_api/worker/codec.py:41
- Draft comment:
Remove or address the commented-out code for type checking to avoid confusion. - Reason this comment was not posted:
Confidence changes required:50%
Thefrom_payload_datafunction incodec.pyhas commented-out code for type checking. This should be addressed or removed to avoid confusion.
4. agents-api/agents_api/routers/tasks/create_or_update_task.py:31
- Draft comment:
Address the FIXME comment indicating a known bug to ensure the function works as intended. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is about a FIXME note, which is a clear indication of a code issue that needs to be addressed. This aligns with the rule that allows comments suggesting actionable code quality refactors. The comment is not asking for verification or explanation, but rather pointing out a necessary action.
The comment might be seen as redundant since the FIXME already indicates a known issue. However, it emphasizes the importance of addressing the bug, which could be useful for prioritization.
While the FIXME note already indicates an issue, the comment serves as a reminder to address it, which can be beneficial in ensuring the bug is not overlooked.
Keep the comment as it highlights the need to address a known bug, which is relevant to the changes made in the PR.
5. agents-api/agents_api/routers/sessions/chat.py:64
- Draft comment:
Address the FIXME comment indicating unimplemented features to ensure the function works as intended. - Reason this comment was not posted:
Marked as duplicate.
6. agents-api/agents_api/routers/tasks/stream_transitions_events.py:20
- Draft comment:
Address the FIXME comment indicating missing support for page tokens to ensure the function works as intended. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_OeRLe8VXSADvPhIM
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 fdc7bfc in 25 seconds
More details
- Looked at
173lines of code in4files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. agents-api/agents_api/workflows/task_execution/__init__.py:579
- Draft comment:
Avoid using print statements for debugging. Use logging with appropriate log levels instead. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is relevant because the diff includes print statements that are likely used for debugging. The suggestion to use logging instead is a valid code quality improvement. The comment is actionable and clear, as it suggests a specific change to improve the code.
The comment might be considered obvious to experienced developers, but it is still a valid suggestion for improving code quality. The print statements are indeed part of the changes in the diff, so the comment is relevant.
Even if the suggestion is somewhat obvious, it is still a useful reminder to maintain good coding practices. The comment is relevant to the changes made in the diff, so it should be kept.
The comment is relevant and suggests a valid code quality improvement related to the changes in the diff. It should be kept.
2. agents-api/agents_api/clients/temporal.py:57
- Draft comment:
TODO: Consider implementing search_attributes for queryability to enhance workflow querying capabilities. - Reason this comment was not posted:
Confidence changes required:50%
The TODO comments indicate areas that need further attention or implementation. It's important to track these and ensure they are addressed in future updates.
3. agents-api/agents_api/workflows/task_execution/__init__.py:159
- Draft comment:
TODO: Consider implementing search_attributes for queryability to enhance workflow querying capabilities. - Reason this comment was not posted:
Confidence changes required:50%
The TODO comments indicate areas that need further attention or implementation. It's important to track these and ensure they are addressed in future updates.
4. agents-api/agents_api/workflows/task_execution/__init__.py:611
- Draft comment:
TODO: Consider using a continue_as_new workflow if history grows too large to manage workflow history size effectively. - Reason this comment was not posted:
Confidence changes required:50%
The TODO comments indicate areas that need further attention or implementation. It's important to track these and ensure they are addressed in future updates.
Workflow ID: wflow_kusWMHgykNkAZrRK
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
creatorrr
changed the title
creatorrr
force-pushed
the
f/set-get-step
branch
from
8079f53
to
511842f
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 511842f in 26 seconds
More details
- Looked at
192lines of code in4files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/workflows/task_execution/__init__.py:580
- Draft comment:
Avoid using print statements for debugging. Consider using a logging framework instead. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is relevant because it addresses the use of print statements in the code, which is a change made in the diff. The rest of the file uses a logging framework, so the comment is suggesting a consistent approach to logging. This is a valid code quality suggestion.
The comment might be considered obvious since using logging frameworks is a common practice, but it is still a valid suggestion for code quality improvement.
Even if the suggestion is somewhat obvious, it is still a valid point for improving code consistency and quality.
The comment is relevant to the changes made in the diff and provides a valid suggestion for improving code quality by using a logging framework instead of print statements.
Workflow ID: wflow_G5HszCYdcukXZcs6
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
creatorrr
force-pushed
the
f/set-get-step
branch
from
511842f
to
85fccc4
Compare
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 85fccc4 in 29 seconds
More details
- Looked at
312lines of code in6files - Skipped
0files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. agents-api/agents_api/common/utils/template.py:44
- Draft comment:
The TODO comment indicates an issue with the regex not working. Consider addressing this or removing the comment if it's no longer relevant. - Reason this comment was not posted:
Confidence changes required:50%
The TODO comment indicates a known issue with the regex not working as expected. This should be addressed or removed if no longer relevant.
2. agents-api/agents_api/clients/temporal.py:57
- Draft comment:
Consider implementing search attributes for queryability as suggested by the TODO comment. - Reason this comment was not posted:
Confidence changes required:30%
The TODO comment suggests adding search attributes for queryability, which is a potential improvement for the workflow execution.
3. agents-api/agents_api/workflows/task_execution/__init__.py:122
- Draft comment:
Consider implementing search attributes for queryability as suggested by the TODO comment. - Reason this comment was not posted:
Confidence changes required:30%
The TODO comment suggests adding search attributes for queryability, which is a potential improvement for the workflow execution.
4. agents-api/agents_api/workflows/task_execution/__init__.py:126
- Draft comment:
Consider reviewing and optimizing the current user state storage method for efficiency as suggested by the TODO comment. - Reason this comment was not posted:
Confidence changes required:30%
The TODO comment suggests reviewing the current user state storage method for efficiency. This is a potential area for improvement.
5. agents-api/agents_api/workflows/task_execution/__init__.py:135
- Draft comment:
Consider adding endpoints for getting and setting user state as suggested by the TODO comment. - Reason this comment was not posted:
Confidence changes required:30%
The TODO comment suggests adding endpoints for getting and setting user state, which could enhance functionality.
6. agents-api/agents_api/workflows/task_execution/__init__.py:577
- Draft comment:
Consider using a continue_as_new workflow if history grows too large as suggested by the TODO comment. - Reason this comment was not posted:
Confidence changes required:30%
The TODO comment suggests using a continue_as_new workflow if history grows too large, which is a potential improvement for handling large histories.
Workflow ID: wflow_WBpYtmOosEHL73W2
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.
creatorrr
force-pushed
the
f/set-get-step
branch
from
85fccc4
to
3b933b5
Compare
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 3b933b5 in 26 seconds
More details
- Looked at
328lines of code in6files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. agents-api/tests/test_set_get_workflow.py:1
- Draft comment:
The file is empty. It should contain tests for the new set/get workflow functionality introduced in this PR. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is relevant because it addresses the fact that the new file added in the diff is empty and should contain tests. This is a valid point since the purpose of the file is to test new functionality introduced in the PR.
The comment could be seen as obvious since an empty test file is clearly not useful. However, it is still a valid point that needs to be addressed.
Even though the issue might seem obvious, it is important to highlight it to ensure that the necessary tests are added.
The comment should be kept because it highlights a valid issue with the new file added in the diff, which is currently empty and should contain tests.
2. agents-api/agents_api/common/utils/template.py:44
- Draft comment:
The TODO comment is vague. Consider providing more context or details about the issue with the regex. - Reason this comment was not posted:
Confidence changes required:50%
The TODO comment indicates a known issue with the regex, but it doesn't provide any actionable information or context. It would be more helpful to describe the specific problem or remove the comment if it's not useful.
Workflow ID: wflow_TQClmZ30RplXLIml
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.
|
Very inefficient. A better implementation would be to create activities that get a patent workflow handle from child workflows and then do temporal queries and signals to it |
creatorrr
changed the title
creatorrr
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This PR introduces
set_value_stepandget_value_stepin workflows, updates models, and adds tests for new functionality.Key points:
set_value_stepandget_value_stepin workflows.base_evaluatefor mutable values usingBoxwithconversion_box=True.set_value_stepinto workflow execution logic inagents-api/agents_api/workflows/task_execution/__init__.py.SetStepmodel to use a dictionary for expressions.outputfield in transitions optional.SetStep.agents-api/tests/test_set_get_workflow.pyto cover new functionality.Generated with ❤️ by ellipsis.dev