feat(agents-api): Transitions stream SSE endpoint #485

creatorrr · 2024-09-03T04:18:19Z

wip: Set/Get step implementation
feat(agents-api): Set/get steps based on workflow state
feat(agents-api): Transitions stream SSE endpoint

🚀	This description was created by Ellipsis for commit `93b4bb1`

Summary:

This PR adds a Transitions stream SSE endpoint, implements set/get steps in workflows, updates temporal data handling, and modifies the TypeScript SDK and tests.

Key points:

Added Transitions stream SSE endpoint in agents-api/agents_api/routers/tasks/stream_transitions_events.py.
Implemented set/get steps in workflows in agents-api/agents_api/workflows/task_execution/__init__.py.
Updated temporal data handling with lookup_temporal_data in agents-api/agents_api/models/execution/lookup_temporal_data.py.
Modified create_temporal_lookup to use execution ID in agents-api/agents_api/models/execution/create_temporal_lookup.py.
Added set_value_step for setting values in workflows.
Updated TaskExecutionWorkflow to handle SetStep and GetStep.
Migration script to make transition output optional.
Updated TypeScript SDK models and schemas for SetStep.
Added tests for new set/get steps and transitions stream functionality in agents-api/tests/test_execution_queries.py.

Generated with ❤️ by ellipsis.dev

Signed-off-by: Diwank Singh Tomer <[email protected]>

ellipsis-dev

❌ Changes requested. Reviewed everything up to ed4d2ec in 1 minute and 6 seconds

More details

Looked at 1346 lines of code in 29 files
Skipped 2 files when reviewing.
Skipped posting 3 drafted comments based on config settings.

1. agents-api/agents_api/activities/task_steps/set_value_step.py:35

Draft comment:
Reassigning set_value_step to a new definition with activity.defn is unnecessary and can be confusing. Consider defining the function once and using activity.defn directly on it.

@activity.defn(name="set_value_step")
async def set_value_step(

Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment addresses a change made in the diff, specifically the reassignment of set_value_step. The suggestion to use activity.defn directly on the function is a valid code quality improvement, as it simplifies the code and reduces potential confusion.
The reassignment might have a specific purpose, such as conditional logic for testing, which the comment does not acknowledge. The comment could be missing context about why the reassignment is done this way.
Even if there is a specific reason for the reassignment, the comment still highlights a potential area for simplification and clarity, which is valuable feedback.
The comment is about a change made in the diff and provides a valid suggestion for code improvement. It should be kept.

2. agents-api/agents_api/workflows/task_execution/transition.py:21

Draft comment:
The match-case statement for setting state.type can be simplified. Consider using a more concise conditional approach.

if context.is_last_step:
    state.type = "finish" if context.cursor.workflow == "main" else "finish_branch"
else:
    state.type = "step"

Reason this comment was not posted:
Confidence changes required: 50%
The transition function in transition.py uses a match-case statement that could be simplified. The match-case is used to determine the type of transition based on the context, but it could be more concise by directly setting the state type based on conditions.

3. agents-api/agents_api/models/execution/create_temporal_lookup.py:38

Draft comment:
Converting UUID to str for developer_id and execution_id may be redundant if these values are already strings or if the function can handle UUID types directly. Consider removing unnecessary conversions.
Reason this comment was not posted:
Confidence changes required: 40%
The create_temporal_lookup function in create_temporal_lookup.py has a redundant conversion of UUID to str for developer_id and execution_id. This conversion is unnecessary if these values are already strings or if the function that uses them can handle UUID types.

Workflow ID: wflow_aRcH7zdrWaH109pL

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.

ellipsis-dev · 2024-09-03T04:19:31Z

agents-api/agents_api/activities/task_steps/set_value_step.py

+@beartype
+async def set_value_step(
+    context: StepContext,
+    additional_values: dict[str, Any] = {},


Using mutable default arguments like {} for additional_values can lead to unexpected behavior. Consider using None as the default value and initializing it inside the function.

Suggested change

additional_values: dict[str, Any] = {},

additional_values: dict[str, Any] = None,

agents-api/agents_api/routers/tasks/stream_transitions_events.py

Signed-off-by: Diwank Singh Tomer <[email protected]>

ellipsis-dev

👍 Looks good to me! Incremental review on ab427f0 in 44 seconds

More details

Looked at 436 lines of code in 11 files
Skipped 0 files when reviewing.
Skipped posting 5 drafted comments based on config settings.

1. agents-api/agents_api/clients/temporal.py:61

Draft comment:
Consider adding type hints for the parameters and return type of this function for better readability and maintainability.
Reason this comment was not posted:
Confidence changes required: 50%
The get_workflow_handle function in temporal.py is missing type hints for its parameters and return type. Adding these will improve code readability and maintainability.

2. agents-api/agents_api/models/execution/create_temporal_lookup.py:32

Draft comment:
Ensure that the @cozo_query decorator is properly defined and imported, as it is not shown in the provided code. This might lead to runtime errors if not handled correctly.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is speculative and not directly related to any changes made in the diff. It suggests verifying something that is not part of the code changes, which violates the rules for review comments. The decorator's presence in the unchanged code indicates it is likely already handled.
I might be overlooking the possibility that the decorator could be incorrectly defined elsewhere, but that is outside the scope of this diff review.
The focus should remain on the changes made in the diff, and the comment does not address any of those changes.
The comment should be deleted as it is speculative and not related to the changes made in the diff.

3. agents-api/agents_api/models/execution/lookup_temporal_data.py:30

Draft comment:
Ensure that the @cozo_query decorator is properly defined and imported, as it is not shown in the provided code. This might lead to runtime errors if not handled correctly.
Reason this comment was not posted:
Marked as duplicate.

4. agents-api/agents_api/routers/tasks/create_task_execution.py:124

Draft comment:
Consider catching more specific exceptions instead of a general Exception to avoid masking unexpected errors.
Reason this comment was not posted:
Confidence changes required: 50%
The create_task_execution function in create_task_execution.py has a try-except block that catches a general Exception. It's a good practice to catch more specific exceptions to avoid masking unexpected errors.

5. agents-api/agents_api/routers/tasks/stream_transitions_events.py:100

Draft comment:
Ensure that next_page_token is always a valid base64 string to prevent runtime errors during decoding.
Reason this comment was not posted:
Confidence changes required: 50%
The stream_transitions_events function in stream_transitions_events.py uses a next_page_token parameter that is base64 decoded. Ensure that the input is always a valid base64 string to prevent runtime errors.

Workflow ID: wflow_FWNS6DmNCkI0GVg3

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev

👍 Looks good to me! Incremental review on 93b4bb1 in 33 seconds

More details

Looked at 436 lines of code in 11 files
Skipped 0 files when reviewing.
Skipped posting 5 drafted comments based on config settings.

1. agents-api/agents_api/routers/tasks/stream_transitions_events.py:1

Draft comment:
The test_set_get_workflow.py file is empty and should be removed if not needed.
Reason this comment was not posted:
Confidence changes required: 50%
The test_set_get_workflow.py file is empty and should be removed if not needed.

2. agents-api/agents_api/models/execution/create_temporal_lookup.py:2

Draft comment:
The uuid4 import is unused and should be removed.
Reason this comment was not posted:
Confidence changes required: 50%
The create_temporal_lookup function has an unused import uuid4 which should be removed.

3. agents-api/agents_api/models/execution/create_temporal_lookup.py:1

Draft comment:
The TypeVar import is unused and should be removed.
Reason this comment was not posted:
Confidence changes required: 50%
The create_temporal_lookup function has an unused import TypeVar which should be removed.

4. agents-api/agents_api/models/execution/lookup_temporal_data.py:1

Draft comment:
The Any import is unused and should be removed.
Reason this comment was not posted:
Confidence changes required: 50%
The lookup_temporal_data function has an unused import Any which should be removed.

5. agents-api/agents_api/models/execution/lookup_temporal_data.py:18

Draft comment:
The ModelT import is unused and should be removed.
Reason this comment was not posted:
Confidence changes required: 50%
The lookup_temporal_data function has an unused import ModelT which should be removed.

Workflow ID: wflow_HdQm5nWXvqbSAoVW

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

creatorrr added 2 commits September 2, 2024 20:58

wip: Set/Get step implementation

cb8ce3e

Signed-off-by: Diwank Singh Tomer <[email protected]>

feat(agents-api): Set/get steps based on workflow state

3b933b5

Signed-off-by: Diwank Singh Tomer <[email protected]>

ellipsis-dev bot reviewed Sep 3, 2024

View reviewed changes

creatorrr force-pushed the f/transition-stream branch from ed4d2ec to ab427f0 Compare September 3, 2024 04:35

feat(agents-api): Transitions stream SSE endpoint

93b4bb1

Signed-off-by: Diwank Singh Tomer <[email protected]>

creatorrr force-pushed the f/transition-stream branch from ab427f0 to 93b4bb1 Compare September 3, 2024 04:35

ellipsis-dev bot reviewed Sep 3, 2024

View reviewed changes

creatorrr changed the title ~~feat(agents-api): Transitions stream SSE endpoint~~ wip(agents-api): Transitions stream SSE endpoint Sep 3, 2024

Merge branch 'dev-tasks' into f/transition-stream

eaeafb9

creatorrr changed the title ~~wip(agents-api): Transitions stream SSE endpoint~~ feat(agents-api): Transitions stream SSE endpoint Sep 3, 2024

creatorrr merged commit 19e02e2 into dev-tasks Sep 3, 2024

creatorrr deleted the f/transition-stream branch September 3, 2024 14:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(agents-api): Transitions stream SSE endpoint #485

feat(agents-api): Transitions stream SSE endpoint #485

Uh oh!

creatorrr commented Sep 3, 2024 •

edited by ellipsis-dev bot

Loading

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

ellipsis-dev bot Sep 3, 2024

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	additional_values: dict[str, Any] = {},
	additional_values: dict[str, Any] = None,

feat(agents-api): Transitions stream SSE endpoint #485

feat(agents-api): Transitions stream SSE endpoint #485

Uh oh!

Conversation

creatorrr commented Sep 3, 2024 • edited by ellipsis-dev bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

ellipsis-dev bot Sep 3, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

creatorrr commented Sep 3, 2024 •

edited by ellipsis-dev bot

Loading