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 prompt render, codec and task execution #478

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

creatorrr
Copy link
Contributor

@creatorrr creatorrr commented Aug 29, 2024

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


🚀 This description was created by Ellipsis for commit 19c0f13

Summary:

This PR refactors template rendering, improves exception handling, and updates test configurations in the agents-api module.

Key points:

  • agents-api/agents_api/common/utils/template.py: Replaced render_template_chatml and render_template_parts with render_template_nested to handle nested lists/dicts.
  • agents-api/agents_api/dependencies/developer_id.py: Removed assertion for x_developer_id in non-multi-tenant mode.
  • agents-api/agents_api/models/agent/delete_agent.py: Updated exception handling for QueryException to check e.resp["display"].
  • agents-api/agents_api/models/agent/get_agent.py: Similar exception handling update as delete_agent.py.
  • agents-api/agents_api/models/user/get_user.py: Similar exception handling update as delete_agent.py.
  • agents-api/agents_api/worker/codec.py: Added logging for encoding errors in PydanticEncodingPayloadConverter.
  • agents-api/agents_api/workflows/task_execution.py: Improved error handling in run function by converting exceptions to strings.
  • agents-api/pyproject.toml: Added PYTHONPATH to test environment variables.
  • agents-api/tests/fixtures.py: Adjusted test fixtures to respect multi_tenant_mode.
  • agents-api/tests/test_execution_workflow.py: Updated test to check for choices in result.

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.

👍 Looks good to me! Reviewed everything up to 19c0f13 in 44 seconds

More details
  • Looked at 357 lines of code in 10 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 drafted comments based on config settings.
1. agents-api/agents_api/common/utils/template.py:5
  • Draft comment:
    Consider using the standard re module instead of re2 for regex operations unless there's a specific need for re2.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of re2 for regex operations is not standard in Python, where re is more common. This could lead to compatibility issues or unexpected behavior if re2 does not fully support all re features. Consider using re unless there's a specific need for re2.
2. agents-api/agents_api/common/utils/template.py:70
  • Draft comment:
    Ensure that the project's Python version requirement is compatible with the use of match-case statements, which are available from Python 3.10 onwards.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The render_template_nested function uses a match-case statement which is only available in Python 3.10 and later. Ensure that the project's Python version requirement is compatible with this feature.
3. agents-api/agents_api/dependencies/developer_id.py:16
  • Draft comment:
    Consider adding validation for the developer ID even when multi_tenant_mode is false to prevent potential security issues.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The get_developer_id function does not verify the developer ID if multi_tenant_mode is false. This could lead to potential security issues if the developer ID is not validated in single-tenant mode.
4. agents-api/agents_api/models/agent/delete_agent.py:31
  • Draft comment:
    Ensure that e.resp is a dictionary with a "display" key before accessing it to avoid potential KeyError.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The delete_agent function uses e.resp["display"] to check for a specific error message. This assumes that e.resp is always a dictionary with a "display" key, which might not be the case for all exceptions. This could lead to a KeyError.
5. agents-api/agents_api/worker/codec.py:92
  • Draft comment:
    Consider raising an exception or ensuring the caller handles None appropriately when encoding fails and None is returned.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PydanticEncodingPayloadConverter class logs a warning and returns None if encoding fails. This could lead to silent failures if the caller does not check for None. Consider raising an exception or handling None appropriately.
6. agents-api/agents_api/workflows/task_execution.py:171
  • Draft comment:
    Good practice to convert exceptions to strings for logging: output=dict(error=str(e)).
  • Reason this comment was not posted:
    Confidence changes required: 0%
    In the run function, the transition function is called with output=dict(error=str(e)). This ensures that the error message is always a string, which is a good practice for logging and debugging.

Workflow ID: wflow_Y4xukZWjiypUu4C6


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

@whiterabbit1983 whiterabbit1983 merged commit f8a8f08 into dev-tasks Aug 29, 2024
2 of 5 checks passed
@whiterabbit1983 whiterabbit1983 deleted the x/fix-task-execution branch August 29, 2024 18:42
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