Skip to content
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): Fix map reduce step and activity #466

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

creatorrr
Copy link
Contributor

@creatorrr creatorrr commented Aug 23, 2024

Signed-off-by: Diwank Tomer [email protected]


🚀 This description was created by Ellipsis for commit 2a12e3c

Summary:

Refactored task step evaluation logic using base_evaluate function and updated TypeScript SDK models to align with backend changes, improving consistency and error handling.

Key points:

  • Introduced base_evaluate in agents-api/agents_api/activities/task_steps/base_evaluate.py for expression evaluation.
  • Updated task step functions (evaluate_step, for_each_step, if_else_step, etc.) to use base_evaluate.
  • Removed simple_eval usage, replaced with base_evaluate.
  • Updated TypeScript SDK models and schemas to reflect backend changes.
  • Removed redundant TypeScript SDK model definitions related to task steps.
  • Enhanced error handling and logging in task step functions.
  • Adjusted map_reduce step logic to utilize base_evaluate for expression evaluation.

Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 2a12e3c in 40 seconds

More details
  • Looked at 2883 lines of code in 70 files
  • Skipped 2 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/activities/task_steps/evaluate_step.py:14
  • Draft comment:
    Using a mutable default argument (like a dictionary) is not recommended as it can lead to unexpected behavior. Consider using None as the default value and initializing the dictionary inside the function.
    additional_values: dict[str, Any] = None,
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_sApOb4iZkGrq7MJ0


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.

@beartype
async def base_evaluate(
exprs: str | list[str] | dict[str, str],
values: dict[str, Any] = {},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a mutable default argument (like a dictionary) is not recommended as it can lead to unexpected behavior. Consider using None as the default value and initializing the dictionary inside the function.

Suggested change
values: dict[str, Any] = {},
values: dict[str, Any] = None,

@whiterabbit1983 whiterabbit1983 merged commit 93a32bf into dev-tasks-disable-routes Aug 23, 2024
2 of 5 checks passed
@whiterabbit1983 whiterabbit1983 deleted the x/map-reduce-step branch August 23, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants