-
Notifications
You must be signed in to change notification settings - Fork 928
Support python expressions in prompt settings #1422
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
base: dev
Are you sure you want to change the base?
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to ba318c3 in 1 minute and 18 seconds. Click for details.
- Reviewed
14
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
3
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. agents-api/agents_api/activities/task_steps/prompt_step.py:110
- Draft comment:
Ensure base_evaluate safely handles arbitrary code execution when evaluating settings. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While security around code evaluation is important, this comment violates our rules by starting with "Ensure that..." and asking the author to verify something. It's also speculative - we don't have evidence that base_evaluate is unsafe. Without seeing the implementation of base_evaluate, we can't make strong claims about its safety. The security concern could be valid and important. Arbitrary code execution is a serious security risk. However, the comment is phrased as a verification request rather than pointing out a specific issue. We don't have evidence that base_evaluate is actually unsafe. The comment should be deleted because it asks for verification rather than pointing out a specific issue, and we don't have strong evidence that there's actually a security problem.
2. agents-api/agents_api/activities/task_steps/prompt_step.py:110
- Draft comment:
Consider adding error handling for base_evaluate failures during settings evaluation. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
3. agents-api/agents_api/activities/task_steps/prompt_step.py:110
- Draft comment:
Typographical suggestion: In the comment on line 110, consider capitalizing 'python' to 'Python' for consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative and suggests a typographical change that doesn't impact the functionality or logic of the code. It doesn't align with the rules for useful comments, which should focus on code logic, potential issues, or improvements.
Workflow ID: wflow_I4zvZozKwEjAy0fa
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
User description
Summary
base_evaluate
Testing
poetry run poe check
(fails:Failed to fetch https://pypi.org/simple/poethepoet/
)PR Type
Enhancement
Description
Evaluate Python expressions in prompt step settings using
base_evaluate
Ensure settings are processed before LLM invocation
Changes walkthrough 📝
prompt_step.py
Evaluate and process prompt step settings before LLM call
agents-api/agents_api/activities/task_steps/prompt_step.py
passed_settings
viabase_evaluate
Important
Evaluate Python expressions in
prompt_step
settings usingbase_evaluate
inprompt_step.py
.prompt_step()
inprompt_step.py
, evaluate Python expressions inpassed_settings
usingbase_evaluate
before calling the language model.poetry run poe check
fails due to a dependency fetch issue.This description was created by
for ba318c3. You can customize this summary. It will automatically update as commits are pushed.