-
Notifications
You must be signed in to change notification settings - Fork 917
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): ALL TESTS PASS!! :D #459
Conversation
Signed-off-by: Diwank Tomer <diwank@julep.ai>
Signed-off-by: Diwank Tomer <diwank@julep.ai>
… test temporal envs Signed-off-by: Diwank Tomer <diwank@julep.ai>
Signed-off-by: Diwank Tomer <diwank@julep.ai>
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.
👍 Looks good to me! Reviewed everything up to 0194c73 in 1 minute and 36 seconds
More details
- Looked at
1105
lines of code in21
files - Skipped
0
files when reviewing. - Skipped posting
7
drafted comments based on config settings.
1. agents-api/agents_api/activities/demo.py:8
- Draft comment:
Ensure that thedemo_activity
is never called in production environments as it raises an exception. This is controlled by thetesting
flag, which should be properly set in non-production environments to avoid runtime errors. - Reason this comment was not posted:
Confidence changes required:33%
The code inagents-api/agents_api/activities/demo.py
is designed to switch between a real activity and a mock activity based on thetesting
flag. This is a common pattern in testing to avoid executing real code that could affect production systems. Thedemo_activity
raises an exception if called in production, which is a safeguard against accidental execution in a non-testing environment. Themock_demo_activity
simply returns the sum of two numbers, serving as a benign placeholder during tests.
2. agents-api/agents_api/activities/embed_docs.py:12
- Draft comment:
Theembed_docs
activity processes document snippets for embedding. Ensure that theembedder
client andembed_snippets_query
are correctly configured and that thetesting
flag is properly managed to switch between real and mock implementations. - Reason this comment was not posted:
Confidence changes required:33%
Theembed_docs
activity inagents-api/agents_api/activities/embed_docs.py
is designed to handle document embedding tasks. It uses theembedder
client to generate embeddings for document snippets and stores these embeddings using theembed_snippets_query
. Themock_embed_docs
function is a no-op, used for testing to bypass the actual embedding process. The activity definition at the end of the file switches between the real and mock functions based on thetesting
flag.
3. agents-api/agents_api/activities/task_steps/evaluate_step.py:16
- Draft comment:
Theevaluate_step
activity evaluates expressions and returns the results. It uses thesimple_eval_dict
utility function to perform the evaluation. Ensure that the expressions and context are correctly handled and that thetesting
flag is properly managed. - Reason this comment was not posted:
Confidence changes required:33%
Theevaluate_step
activity inagents-api/agents_api/activities/task_steps/evaluate_step.py
evaluates expressions based on the context provided. Themock_evaluate_step
is a direct reference toevaluate_step
, indicating that no special mocking behavior is needed for testing this activity. The activity definition uses thetesting
flag to decide whether to use the real or mock function.
4. agents-api/agents_api/activities/task_steps/transition_step.py:13
- Draft comment:
Thetransition_step
activity manages state transitions within task executions. It uses thecreate_execution_transition_query
to record transitions. Ensure that the transition logic is correctly implemented and that thetesting
flag is properly managed to switch between real and mock implementations. - Reason this comment was not posted:
Confidence changes required:33%
Thetransition_step
activity inagents-api/agents_api/activities/task_steps/transition_step.py
handles the logic for transitioning between different states in a workflow. It checks if a waiting step is needed and fetches the task token if so. Thecreate_execution_transition_query
is used to record the transition in the database. Themock_transition_step
does nothing, serving as a placeholder during testing. The activity definition switches between the real and mock functions based on thetesting
flag.
5. agents-api/agents_api/activities/task_steps/yield_step.py:15
- Draft comment:
Theyield_step
activity manages control flow by evaluating expressions and determining the next step or workflow to transition to. Ensure that the expressions and transitions are correctly handled and that thetesting
flag is properly managed. - Reason this comment was not posted:
Confidence changes required:33%
Theyield_step
activity inagents-api/agents_api/activities/task_steps/yield_step.py
handles the logic for yielding control to another step or workflow based on the evaluation of expressions. It uses thesimple_eval_dict
utility function to evaluate expressions and determine the next step. Themock_yield_step
is a direct reference toyield_step
, indicating that no special mocking behavior is needed for testing this activity. The activity definition uses thetesting
flag to decide whether to use the real or mock function.
6. agents-api/agents_api/common/protocol/tasks.py:26
- Draft comment:
Thevalid_transitions
dictionary is crucial for managing state transitions in task executions. Ensure that all transitions are correctly defined and that the state machine logic is robust and handles all expected and unexpected transitions appropriately. - Reason this comment was not posted:
Confidence changes required:33%
Thevalid_transitions
dictionary inagents-api/agents_api/common/protocol/tasks.py
defines valid state transitions for task executions. It's crucial for ensuring that tasks transition through states in a controlled and predictable manner. Theinit
state is noted as a placeholder for the start state of the state machine, which is a key piece of information for understanding the workflow logic.
7. agents-api/agents_api/env.py:91
- Draft comment:
Ensure that thedebug
andtesting
flags are never enabled in production environments as they could lead to the exposure of sensitive configuration details. The warning messages are a good practice but consider implementing additional safeguards to prevent these modes from being enabled in production. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is somewhat redundant because the code already includes a warning message for whendebug
ortesting
is enabled. Additionally, the suggestion to implement additional safeguards is vague and not actionable. The comment does not point out a specific code change that is required, which makes it less useful.
I might be underestimating the importance of the comment's suggestion for additional safeguards, but without specific guidance, it remains vague. The comment could be seen as a reminder for best practices, but it doesn't align with the rules for actionable comments.
While the suggestion for additional safeguards is a good practice, the lack of specificity makes it less actionable. The existing warning in the code already addresses the primary concern raised by the comment.
Remove the comment as it is redundant and not actionable. The code already includes a warning for the issue mentioned, and the suggestion for additional safeguards is too vague.
Workflow ID: wflow_bUHsgDoG3b1NR0AP
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
@@ -120,11 +122,12 @@ def _(make_request=make_request, task=test_task): | |||
assert response.status_code == 200 | |||
|
|||
|
|||
# FIXME: This test is failing |
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.
That might happen because the transition created in test_transition
does not belong to the execution created in test_execution
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.
No this is happening because of an invalid transition. It seems that at the time of running this fixture, there is already a transition with type "cancelled". And this fixture transition is of type "step".
Since "cancelled"->"step" is an invalid transition, this query fails
729a0aa
into
dev-tasks-disable-routes
Summary:
This PR adds a demo workflow, refactors activities for testing, updates environment configurations, and enhances test fixtures to ensure all tests pass in the
agents-api
module.Key points:
agents-api/agents_api/activities/demo.py
for demo activity with mock support.agents-api/agents_api/activities/embed_docs.py
to include mock activity for testing.agents-api/agents_api/activities/task_steps
to use mock activities when testing.agents-api/agents_api/env.py
to print environment variables and warn if testing is enabled.agents-api/agents_api/models/execution
to fix transition validation and update logic.agents-api/agents_api/workflows/demo.py
for demo workflow execution.agents-api/tests/
to support testing with Temporal.Generated with ❤️ by ellipsis.dev