Skip to content

Conversation

@filintod
Copy link
Contributor

@filintod filintod commented Jul 29, 2025

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:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

filintod added 2 commits July 28, 2025 21:32
Signed-off-by: Filinto Duran <[email protected]>
@filintod filintod marked this pull request as ready for review August 1, 2025 00:04
@filintod filintod requested review from a team as code owners August 1, 2025 00:04
@filintod
Copy link
Contributor Author

filintod commented Aug 6, 2025

@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

Copy link
Contributor

@elena-kolevska elena-kolevska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@filintod
Copy link
Contributor Author

@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

@elena-kolevska
Copy link
Contributor

@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! 🙏

@elena-kolevska
Copy link
Contributor

@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 <[email protected]>
Signed-off-by: Filinto Duran <[email protected]>
@filintod
Copy link
Contributor Author

@acroca @elena-kolevska this should be ready now

@filintod filintod requested a review from acroca August 14, 2025 17:13
Some example README refactor/lint

Signed-off-by: Filinto Duran <[email protected]>
Signed-off-by: Filinto Duran <[email protected]>
@filintod
Copy link
Contributor Author

ran tox -e ruff to fix the format count mismatch shown on the lint error. I thought tox -e flake8 green meant that it was ok, but now I know.

@codecov
Copy link

codecov bot commented Sep 3, 2025

Codecov Report

❌ Patch coverage is 91.83246% with 78 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.33%. Comparing base (bffb749) to head (bc3dcf0).
⚠️ Report is 40 commits behind head on main.

Files with missing lines Patch % Lines
dapr/clients/grpc/_conversation_helpers.py 85.29% 75 Missing ⚠️
dapr/clients/grpc/conversation.py 99.03% 3 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@elena-kolevska elena-kolevska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@elena-kolevska elena-kolevska added this to the v1.16 milestone Sep 3, 2025
@elena-kolevska
Copy link
Contributor

Is this planned to be included in 1.16? If so, we would need to cherrypick it in the 1.16 branch.

@filintod filintod changed the base branch from main to release-1.16 September 3, 2025 14:23
@filintod filintod changed the base branch from release-1.16 to main September 3, 2025 14:24
@filintod
Copy link
Contributor Author

filintod commented Sep 3, 2025

@elena-kolevska ptal, thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, it was just getting too big. I'll merge and check the error on python 3.13 shown in CI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok please take another look

Signed-off-by: Elena Kolevska <[email protected]>
Copy link
Contributor

@elena-kolevska elena-kolevska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a quick fix of the linter, but there are still some typing errors

@filintod
Copy link
Contributor Author

filintod commented Sep 4, 2025

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.

@elena-kolevska elena-kolevska merged commit 41f44c5 into dapr:main Sep 4, 2025
19 of 20 checks passed
@elena-kolevska
Copy link
Contributor

Merged! 🎉 Thanks for your contribution and your patience @filintod :)

filintod added a commit to filintod/python-sdk that referenced this pull request Sep 4, 2025
* initial

Signed-off-by: Filinto Duran <[email protected]>

* fixes after proto change upstream

Signed-off-by: Filinto Duran <[email protected]>

* minor name changes and cleanup unused function

Signed-off-by: Filinto Duran <[email protected]>

* refactors, updates to readme, linting

Signed-off-by: Filinto Duran <[email protected]>

* feedback

Signed-off-by: Filinto Duran <[email protected]>

* feedback, updates

Signed-off-by: Filinto Duran <[email protected]>

* fix import in examples

Signed-off-by: Filinto Duran <[email protected]>

* cleanup, import, lint, more conversation helpers

Signed-off-by: Filinto Duran <[email protected]>

* clarify README, minor test import changes, copyright

Signed-off-by: Filinto Duran <[email protected]>

* feedback DRY test_conversation file

Signed-off-by: Filinto Duran <[email protected]>

* lint

Signed-off-by: Filinto Duran <[email protected]>

* move conversation classes in _response module to conversation module.

Some example README refactor/lint

Signed-off-by: Filinto Duran <[email protected]>

* minor readme change

Signed-off-by: Filinto Duran <[email protected]>

* Update daprdocs/content/en/python-sdk-docs/python-client.md

Co-authored-by: Albert Callarisa <[email protected]>
Signed-off-by: Filinto Duran <[email protected]>

* lint

Signed-off-by: Filinto Duran <[email protected]>

* 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 <[email protected]>

* coalesce conv helper tests, fix typing lint

Signed-off-by: Filinto Duran <[email protected]>

* make indent line method doc more dev friendly

Co-authored-by: Elena Kolevska <[email protected]>
Signed-off-by: Filinto Duran <[email protected]>

* tackle some feedback, still missing unit tests

Signed-off-by: Filinto Duran <[email protected]>

* add unit test to convert_value_to_struct

Signed-off-by: Filinto Duran <[email protected]>

* more unit tests per feedback

Signed-off-by: Filinto Duran <[email protected]>

* make async version of unit test conversation

Signed-off-by: Filinto Duran <[email protected]>

* add some information how to run markdown tests with a different runtime

Signed-off-by: Filinto Duran <[email protected]>

* ran tox -e ruff, even though tox -e flake8 was fine

Signed-off-by: Filinto Duran <[email protected]>

* add tests to increase coverage in conversation and conversation_helpers that codecov pointed out

Signed-off-by: Filinto Duran <[email protected]>

* add more information on execute registered tools, also added more tests for them to validate

Signed-off-by: Filinto Duran <[email protected]>

* fix test failing on py 1.13. Merge two unit test files per feedback

Signed-off-by: Filinto Duran <[email protected]>

* Linter

Signed-off-by: Elena Kolevska <[email protected]>

* fix typing issue with UnionType in py3.9

Signed-off-by: Filinto Duran <[email protected]>

---------

Signed-off-by: Filinto Duran <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
Co-authored-by: Albert Callarisa <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>
filintod added a commit to filintod/python-sdk that referenced this pull request Sep 4, 2025
* initial

Signed-off-by: Filinto Duran <[email protected]>

* fixes after proto change upstream

Signed-off-by: Filinto Duran <[email protected]>

* minor name changes and cleanup unused function

Signed-off-by: Filinto Duran <[email protected]>

* refactors, updates to readme, linting

Signed-off-by: Filinto Duran <[email protected]>

* feedback

Signed-off-by: Filinto Duran <[email protected]>

* feedback, updates

Signed-off-by: Filinto Duran <[email protected]>

* fix import in examples

Signed-off-by: Filinto Duran <[email protected]>

* cleanup, import, lint, more conversation helpers

Signed-off-by: Filinto Duran <[email protected]>

* clarify README, minor test import changes, copyright

Signed-off-by: Filinto Duran <[email protected]>

* feedback DRY test_conversation file

Signed-off-by: Filinto Duran <[email protected]>

* lint

Signed-off-by: Filinto Duran <[email protected]>

* move conversation classes in _response module to conversation module.

Some example README refactor/lint

Signed-off-by: Filinto Duran <[email protected]>

* minor readme change

Signed-off-by: Filinto Duran <[email protected]>

* Update daprdocs/content/en/python-sdk-docs/python-client.md

Co-authored-by: Albert Callarisa <[email protected]>
Signed-off-by: Filinto Duran <[email protected]>

* lint

Signed-off-by: Filinto Duran <[email protected]>

* 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 <[email protected]>

* coalesce conv helper tests, fix typing lint

Signed-off-by: Filinto Duran <[email protected]>

* make indent line method doc more dev friendly

Co-authored-by: Elena Kolevska <[email protected]>
Signed-off-by: Filinto Duran <[email protected]>

* tackle some feedback, still missing unit tests

Signed-off-by: Filinto Duran <[email protected]>

* add unit test to convert_value_to_struct

Signed-off-by: Filinto Duran <[email protected]>

* more unit tests per feedback

Signed-off-by: Filinto Duran <[email protected]>

* make async version of unit test conversation

Signed-off-by: Filinto Duran <[email protected]>

* add some information how to run markdown tests with a different runtime

Signed-off-by: Filinto Duran <[email protected]>

* ran tox -e ruff, even though tox -e flake8 was fine

Signed-off-by: Filinto Duran <[email protected]>

* add tests to increase coverage in conversation and conversation_helpers that codecov pointed out

Signed-off-by: Filinto Duran <[email protected]>

* add more information on execute registered tools, also added more tests for them to validate

Signed-off-by: Filinto Duran <[email protected]>

* fix test failing on py 1.13. Merge two unit test files per feedback

Signed-off-by: Filinto Duran <[email protected]>

* Linter

Signed-off-by: Elena Kolevska <[email protected]>

* fix typing issue with UnionType in py3.9

Signed-off-by: Filinto Duran <[email protected]>

---------

Signed-off-by: Filinto Duran <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
Co-authored-by: Albert Callarisa <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>
elena-kolevska added a commit that referenced this pull request Sep 7, 2025
)

* 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 <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
Co-authored-by: Albert Callarisa <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Oct 31, 2025
* 1.16.0-rc1

Signed-off-by: Elena Kolevska <[email protected]>

* [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 <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
Co-authored-by: Albert Callarisa <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>

* update docs with tool calling helpers info (#838)

Signed-off-by: Filinto Duran <[email protected]>

* 1.16.0rc2

Signed-off-by: Elena Kolevska <[email protected]>

* use latest durabletask (#840)

Signed-off-by: Cassandra Coyle <[email protected]>

* 1.16.0

Signed-off-by: Elena Kolevska <[email protected]>

* Adds support for interceptors and concurrency_options arguments in the workflow engine (#841)

Signed-off-by: Albert Callarisa <[email protected]>

* Implement multi-app workflows (#844)

* feat: Adds support for cross-app calls.

Signed-off-by: Albert Callarisa <[email protected]>

* Use durabletask alpha.9

Signed-off-by: Albert Callarisa <[email protected]>

* Added examples for error scenarios in multi-app workflow

Signed-off-by: Albert Callarisa <[email protected]>

* Remove unnecessary hardcoded ports

Signed-off-by: Albert Callarisa <[email protected]>

---------

Signed-off-by: Albert Callarisa <[email protected]>

* chore: Rename wait_until_ready to wait_for_sidecar (#843)

Signed-off-by: Albert Callarisa <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>

* 1.16.1rc1 (#846)

Signed-off-by: Albert Callarisa <[email protected]>

---------

Signed-off-by: Elena Kolevska <[email protected]>
Signed-off-by: Filinto Duran <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Albert Callarisa <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>
Co-authored-by: Filinto Duran <[email protected]>
Co-authored-by: Albert Callarisa <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]>
Co-authored-by: Albert Callarisa <[email protected]>
@marcduiker
Copy link
Contributor

@holopin-bot @filintod Thank you! Here's a digital badge as a small token of appreciation.

@holopin-bot
Copy link

holopin-bot bot commented Nov 18, 2025

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.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

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.

4 participants