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

feat(agents-api): Add python expression support to prompt step #795

Merged
merged 7 commits into from
Oct 31, 2024

Conversation

creatorrr
Copy link
Contributor

@creatorrr creatorrr commented Oct 31, 2024

  • wip(agents-api): Auto-run tools in prompt steps
  • refactor: Lint integrations-service (CI)
  • feat(agents-api): Add python expression support to prompt step
  • feat(agents-api): Default prompt_step.tools = 'all'

Important

Add Python expression support to prompt steps in agents API, with default tool settings and refactoring.

  • Behavior:
    • Add Python expression evaluation for prompts starting with $_ in prompt_step() in prompt_step.py.
    • Default tools in PromptStep to 'all' in Tasks.py.
    • Default auto_run_tools to True for PromptStep and False for sessions in Sessions.py and Tasks.py.
  • Refactoring:
    • Remove unused import Developer from execute_system.py.
    • Remove unused import RootModel from Tools.py.
    • Linting changes in integrations-service.
  • Testing:
    • Add test for prompt step with Python expression in test_execution_workflow.py.

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

creatorrr and others added 4 commits October 31, 2024 15:46
Signed-off-by: Diwank Singh Tomer <diwank.singh@gmail.com>
Signed-off-by: Diwank Singh Tomer <diwank.singh@gmail.com>
Signed-off-by: Diwank Singh Tomer <diwank.singh@gmail.com>
Copy link
Contributor

sweep-ai bot commented Oct 31, 2024

Hey @creatorrr, here is an example of how you can ask me to improve this pull request:

@Sweep Add unit tests for `base_evaluate` function in `prompt_step.py` to test Python expression evaluation with different types of expressions and context data.

📖 For more information on how to use Sweep, please read our documentation.

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 d768c7f in 35 seconds

More details
  • Looked at 1049 lines of code in 17 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. agents-api/agents_api/workflows/task_execution/__init__.py:390
  • Draft comment:
    Consider handling unsupported tool call types explicitly to avoid unexpected behavior.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code is checking for a specific type of tool call but does not handle other types. This could lead to unexpected behavior if an unsupported tool call type is encountered.
2. agents-api/agents_api/workflows/task_execution/__init__.py:619
  • Draft comment:
    Consider handling unsupported tool types more gracefully instead of raising an ApplicationError.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code is raising an ApplicationError for unsupported tool types, but it might be better to handle this more gracefully.

Workflow ID: wflow_F3UOFcAdEDztG4bU


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

Signed-off-by: Diwank Singh Tomer <diwank.singh@gmail.com>
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! Incremental review on ce6f80c in 58 seconds

More details
  • Looked at 221 lines of code in 8 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. agents-api/agents_api/activities/task_steps/prompt_step.py:161
  • Draft comment:
    The comment about reformatting the prompt for Anthropic is now redundant since the prompt is already formatted in the line above. Consider removing it.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change in prompt_step.py from new_prompt to prompt is correct, but the comment above it is now redundant and should be removed.
2. agents-api/agents_api/models/session/create_or_update_session.py:53
  • Draft comment:
    Ensure that excluding disable_cache is intentional and consistent with the new attribute added to PromptStep.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The change in create_or_update_session.py to exclude disable_cache is consistent with the new attribute added to PromptStep.
3. agents-api/agents_api/models/session/create_session.py:60
  • Draft comment:
    Ensure that excluding disable_cache is intentional and consistent with the new attribute added to PromptStep.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The change in create_session.py to exclude disable_cache is consistent with the new attribute added to PromptStep.
4. agents-api/agents_api/workflows/task_execution/__init__.py:403
  • Draft comment:
    Consider refactoring the repeated pattern message := response["choices"][0]["message"] to avoid redundancy. This applies to lines 430, 445, 460, and 474 as well.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The repeated pattern in task_execution/__init__.py for checking message["choices"][0]["message"] can be refactored to avoid redundancy.

Workflow ID: wflow_4U7F4zZxrF1D7Xcc


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

Signed-off-by: Diwank Singh Tomer <diwank.singh@gmail.com>
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! Incremental review on d73eb7f in 16 seconds

More details
  • Looked at 80 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/workflows/task_execution/__init__.py:401
  • Draft comment:
    Consider refactoring the repeated pattern for handling different tool call types (function, integration, api_call, system) to reduce redundancy and improve maintainability. This pattern appears multiple times in the code.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code has multiple instances where the same pattern is repeated for different tool call types. This can be refactored to reduce redundancy and improve maintainability.

Workflow ID: wflow_OPqbUbsNf2ADgIBc


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

Signed-off-by: Diwank Singh Tomer <diwank.singh@gmail.com>
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! Incremental review on 4254083 in 30 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/workflows/task_execution/__init__.py:407
  • Draft comment:
    The condition "type" not in ["integration", "api_call", "system"] should be == "function" to match the intended logic. This issue is also present in lines 437, 452, and 467.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment suggests a change in logic that might not align with the intended functionality. The current condition seems to be checking for types that are not 'integration', 'api_call', or 'system', which might be intentional to handle a broader range of types, including 'function'.
    I might be missing the broader context of how 'function' is used elsewhere in the code. The current condition might be intentionally broad to handle more cases than just 'function'.
    The current condition seems to be intentionally broad, and changing it to '== "function"' might limit its functionality. Without strong evidence that the change is necessary, the comment should be deleted.
    Delete the comment as it suggests a change that might not align with the intended functionality and lacks strong evidence for its necessity.

Workflow ID: wflow_TOaMrERfFs5jfRKz


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

@creatorrr creatorrr merged commit 7264881 into dev Oct 31, 2024
14 of 15 checks passed
@creatorrr creatorrr deleted the f/py-expr-prompt-step branch October 31, 2024 22:32
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.

1 participant