Add new tool calling capability#822
Conversation
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
|
@elena-kolevska not sure if you are still working on this to review? I kind of redid the proto changes you had before, there has also been some changes on dapr master about it |
elena-kolevska
left a comment
There was a problem hiding this comment.
Thanks for the contribution @filintod :)
I left few initial comments, will try to do a deeper review of the actual implementation as soon as possible.
|
@elena-kolevska @acroca , as I was closing the feedback items, I was wondering if leaving all those converstation-specific classes in _request.py is ok or should I refactor the conversation ones to a _conversation.py file? lmk |
We already started separating domain-specific stuff in their own files (crypto, jobs, streaming subscriptions...), and I think it's a good practice we should continue. Thanks for suggesting it, Filinto! 🙏 |
|
@filintod there are some test and lint failures, pls ping me when they're fixed so I can rerun them. |
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
|
@acroca @elena-kolevska this should be ready now |
Some example README refactor/lint Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
|
ran |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #822 +/- ##
==========================================
+ Coverage 86.63% 87.33% +0.70%
==========================================
Files 84 93 +9
Lines 4473 6143 +1670
==========================================
+ Hits 3875 5365 +1490
- Misses 598 778 +180 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
elena-kolevska
left a comment
There was a problem hiding this comment.
Last change request.. Small, but I believe it's really important
| def execute_registered_tool(name: str, params: Union[Params, str] = None) -> Any: | ||
| """Execute a registered tool.""" | ||
| if isinstance(params, str): | ||
| params = json.loads(params) |
There was a problem hiding this comment.
This is a vulnerable part of our code, so we need to add at least some basic validation, and probably consider sanitisation for the future
There was a problem hiding this comment.
Also, let's please add a detailed docstring explaining the vulnerability and giving some suggestions to developers on writing safe functions that validate the LLM input.
I know you said you'd be sending a docs PR, but I think it's important to have this here as well.
|
Is this planned to be included in 1.16? If so, we would need to cherrypick it in the 1.16 branch. |
…rs that codecov pointed out Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
…ts for them to validate Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
|
@elena-kolevska ptal, thanks |
There was a problem hiding this comment.
Why do we need two files for this? Let's either merge them or try to find a more descriptive name than one ending in _more
There was a problem hiding this comment.
ok, it was just getting too big. I'll merge and check the error on python 3.13 shown in CI
There was a problem hiding this comment.
ok please take another look
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
elena-kolevska
left a comment
There was a problem hiding this comment.
I did a quick fix of the linter, but there are still some typing errors
Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com>
|
sorry @elena-kolevska , I did changed something to fix an issue in a test breaking on 3.13, but looks like it broke 3.9. I need to have pre-commit for the future. It "must" be good now. |
|
Merged! 🎉 Thanks for your contribution and your patience @filintod :) |
* initial Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * fixes after proto change upstream Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * minor name changes and cleanup unused function Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * refactors, updates to readme, linting Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * feedback Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * feedback, updates Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * fix import in examples Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * cleanup, import, lint, more conversation helpers Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * clarify README, minor test import changes, copyright Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * feedback DRY test_conversation file Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * lint Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * move conversation classes in _response module to conversation module. Some example README refactor/lint Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * minor readme change Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * Update daprdocs/content/en/python-sdk-docs/python-client.md Co-authored-by: Albert Callarisa <albert@acroca.com> Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * lint Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * updates to fix issue with tool calling helper when dealing with classes instead of dataclasses, and also with serializatin output of the tool back to the LLM Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * coalesce conv helper tests, fix typing lint Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * make indent line method doc more dev friendly Co-authored-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * tackle some feedback, still missing unit tests Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * add unit test to convert_value_to_struct Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * more unit tests per feedback Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * make async version of unit test conversation Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * add some information how to run markdown tests with a different runtime Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * ran tox -e ruff, even though tox -e flake8 was fine Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * add tests to increase coverage in conversation and conversation_helpers that codecov pointed out Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * add more information on execute registered tools, also added more tests for them to validate Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * fix test failing on py 1.13. Merge two unit test files per feedback Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * Linter Signed-off-by: Elena Kolevska <elena@kolevska.com> * fix typing issue with UnionType in py3.9 Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> --------- Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> Signed-off-by: Elena Kolevska <elena@kolevska.com> Co-authored-by: Albert Callarisa <albert@acroca.com> Co-authored-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> Co-authored-by: Elena Kolevska <elena@kolevska.com>
* initial Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * fixes after proto change upstream Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * minor name changes and cleanup unused function Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * refactors, updates to readme, linting Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * feedback Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * feedback, updates Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * fix import in examples Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * cleanup, import, lint, more conversation helpers Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * clarify README, minor test import changes, copyright Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * feedback DRY test_conversation file Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * lint Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * move conversation classes in _response module to conversation module. Some example README refactor/lint Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * minor readme change Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * Update daprdocs/content/en/python-sdk-docs/python-client.md Co-authored-by: Albert Callarisa <albert@acroca.com> Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * lint Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * updates to fix issue with tool calling helper when dealing with classes instead of dataclasses, and also with serializatin output of the tool back to the LLM Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * coalesce conv helper tests, fix typing lint Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * make indent line method doc more dev friendly Co-authored-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * tackle some feedback, still missing unit tests Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * add unit test to convert_value_to_struct Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * more unit tests per feedback Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * make async version of unit test conversation Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * add some information how to run markdown tests with a different runtime Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * ran tox -e ruff, even though tox -e flake8 was fine Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * add tests to increase coverage in conversation and conversation_helpers that codecov pointed out Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * add more information on execute registered tools, also added more tests for them to validate Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * fix test failing on py 1.13. Merge two unit test files per feedback Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * Linter Signed-off-by: Elena Kolevska <elena@kolevska.com> * fix typing issue with UnionType in py3.9 Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> --------- Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> Signed-off-by: Elena Kolevska <elena@kolevska.com> Co-authored-by: Albert Callarisa <albert@acroca.com> Co-authored-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> Co-authored-by: Elena Kolevska <elena@kolevska.com>
) * initial * fixes after proto change upstream * minor name changes and cleanup unused function * refactors, updates to readme, linting * feedback * feedback, updates * fix import in examples * cleanup, import, lint, more conversation helpers * clarify README, minor test import changes, copyright * feedback DRY test_conversation file * lint * move conversation classes in _response module to conversation module. Some example README refactor/lint * minor readme change * Update daprdocs/content/en/python-sdk-docs/python-client.md * lint * updates to fix issue with tool calling helper when dealing with classes instead of dataclasses, and also with serializatin output of the tool back to the LLM * coalesce conv helper tests, fix typing lint * make indent line method doc more dev friendly * tackle some feedback, still missing unit tests * add unit test to convert_value_to_struct * more unit tests per feedback * make async version of unit test conversation * add some information how to run markdown tests with a different runtime * ran tox -e ruff, even though tox -e flake8 was fine * add tests to increase coverage in conversation and conversation_helpers that codecov pointed out * add more information on execute registered tools, also added more tests for them to validate * fix test failing on py 1.13. Merge two unit test files per feedback * Linter * fix typing issue with UnionType in py3.9 --------- Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> Signed-off-by: Elena Kolevska <elena@kolevska.com> Co-authored-by: Albert Callarisa <albert@acroca.com> Co-authored-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> Co-authored-by: Elena Kolevska <elena@kolevska.com>
* 1.16.0-rc1 Signed-off-by: Elena Kolevska <elena@kolevska.com> * [Conversation API - Alpha2] Add new tool calling capability (#822) (#832) * initial * fixes after proto change upstream * minor name changes and cleanup unused function * refactors, updates to readme, linting * feedback * feedback, updates * fix import in examples * cleanup, import, lint, more conversation helpers * clarify README, minor test import changes, copyright * feedback DRY test_conversation file * lint * move conversation classes in _response module to conversation module. Some example README refactor/lint * minor readme change * Update daprdocs/content/en/python-sdk-docs/python-client.md * lint * updates to fix issue with tool calling helper when dealing with classes instead of dataclasses, and also with serializatin output of the tool back to the LLM * coalesce conv helper tests, fix typing lint * make indent line method doc more dev friendly * tackle some feedback, still missing unit tests * add unit test to convert_value_to_struct * more unit tests per feedback * make async version of unit test conversation * add some information how to run markdown tests with a different runtime * ran tox -e ruff, even though tox -e flake8 was fine * add tests to increase coverage in conversation and conversation_helpers that codecov pointed out * add more information on execute registered tools, also added more tests for them to validate * fix test failing on py 1.13. Merge two unit test files per feedback * Linter * fix typing issue with UnionType in py3.9 --------- Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> Signed-off-by: Elena Kolevska <elena@kolevska.com> Co-authored-by: Albert Callarisa <albert@acroca.com> Co-authored-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> Co-authored-by: Elena Kolevska <elena@kolevska.com> * update docs with tool calling helpers info (#838) Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> * 1.16.0rc2 Signed-off-by: Elena Kolevska <elena@kolevska.com> * use latest durabletask (#840) Signed-off-by: Cassandra Coyle <cassie@diagrid.io> * 1.16.0 Signed-off-by: Elena Kolevska <elena@kolevska.com> * Adds support for interceptors and concurrency_options arguments in the workflow engine (#841) Signed-off-by: Albert Callarisa <albert@diagrid.io> * Implement multi-app workflows (#844) * feat: Adds support for cross-app calls. Signed-off-by: Albert Callarisa <albert@diagrid.io> * Use durabletask alpha.9 Signed-off-by: Albert Callarisa <albert@diagrid.io> * Added examples for error scenarios in multi-app workflow Signed-off-by: Albert Callarisa <albert@diagrid.io> * Remove unnecessary hardcoded ports Signed-off-by: Albert Callarisa <albert@diagrid.io> --------- Signed-off-by: Albert Callarisa <albert@diagrid.io> * chore: Rename wait_until_ready to wait_for_sidecar (#843) Signed-off-by: Albert Callarisa <albert@diagrid.io> Co-authored-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> * 1.16.1rc1 (#846) Signed-off-by: Albert Callarisa <albert@diagrid.io> --------- Signed-off-by: Elena Kolevska <elena@kolevska.com> Signed-off-by: Filinto Duran <1373693+filintod@users.noreply.github.com> Signed-off-by: Cassandra Coyle <cassie@diagrid.io> Signed-off-by: Albert Callarisa <albert@diagrid.io> Co-authored-by: Elena Kolevska <elena@kolevska.com> Co-authored-by: Filinto Duran <1373693+filintod@users.noreply.github.com> Co-authored-by: Albert Callarisa <albert@acroca.com> Co-authored-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> Co-authored-by: Albert Callarisa <albert@diagrid.io>
|
@holopin-bot @filintod Thank you! Here's a digital badge as a small token of appreciation. |
|
Congratulations @filintod, the maintainer of this repository has issued you a badge! Here it is: https://holopin.io/claim/cmi4hz7rd000djs04wn4tt3ro This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account. |
Description
Implement conversation new alpha2 version with tool calling. As new protos are more complex, added DevEx helpers and tests.
Issue reference
Part of 1.16 dapr/dapr#8553
Please reference the issue this PR will close: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: