Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(agents-api): Add wait_for_input step to the acceptable steps inside foreach step #625

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

HamadaSalhab
Copy link
Contributor

@HamadaSalhab HamadaSalhab commented Oct 9, 2024

Important

Add WaitForInputStep to foreach step in agents API, updating Tasks.py, steps.tsp, OpenAPI specs, and tests.

  • Behavior:
    • Add WaitForInputStep to ForeachDo and ForeachDoUpdateItem in Tasks.py, allowing it as a valid step in foreach.
    • Update SequentialWorkflowStep alias in steps.tsp to include WaitForInputStep.
  • OpenAPI:
    • Update openapi-0.4.0.yaml and openapi-1.0.0.yaml to include WaitForInputStep in ForeachDo and ForeachDoUpdateItem schemas.
  • Tests:
    • Add test for foreach with wait_for_input in test_execution_workflow.py.

This description was created by Ellipsis for 7b5b4a6. It will automatically update as commits are pushed.

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.

👍 Looks good to me! Reviewed everything up to ac9b7c1 in 27 seconds

More details
  • Looked at 91 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. typespec/tsp-output/@typespec/openapi3/openapi-1.0.0.yaml:4169
  • Draft comment:
    WaitForInputStep is added to the foreach step but lacks documentation in the OpenAPI spec. Consider adding a description for better clarity.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR adds WaitForInputStep to the foreach step, which is reflected in the OpenAPI spec changes. However, the WaitForInputStep is not documented in the OpenAPI spec, which could lead to confusion for API consumers.

Workflow ID: wflow_tDM3gxy961TeFsNb


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

@HamadaSalhab HamadaSalhab marked this pull request as draft October 10, 2024 15:21
@creatorrr creatorrr marked this pull request as ready for review October 10, 2024 22:45
@creatorrr creatorrr merged commit 3291120 into dev Oct 10, 2024
9 of 13 checks passed
@creatorrr creatorrr deleted the f/wait-for-input-in-foreach branch October 10, 2024 22:45
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.

👍 Looks good to me! Reviewed everything up to 7b5b4a6 in 25 seconds

More details
  • Looked at 240 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. typespec/tsp-output/@typespec/openapi3/openapi-1.0.0.yaml:4169
  • Draft comment:
    Ensure that the addition of WaitForInputStep to the ForeachDo schema is consistent with the intended functionality and does not introduce any unintended side effects. This change is also present in ForeachDoUpdateItem schema.
  • Reason this comment was not posted:
    Confidence changes required: 20%
    The OpenAPI specification has been updated to include the WaitForInputStep in the ForeachDo and ForeachDoUpdateItem schemas. This aligns with the PR's intent to add WaitForInputStep to the foreach step. The changes seem consistent with the PR description.

Workflow ID: wflow_RDgyaUfUCyfhVTf1


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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants