feat(convo): add fields to conversation API#736
Conversation
|
Hi @WhitWaldo @sicoyle, could you please review the PR and share your feedback when you get a chance? |
|
It'll probably be next week. |
|
Hi! 👋 thanks!! if you can run these commands pls and be sure to always sign your commits with |
There was a problem hiding this comment.
Pull request overview
Adds the Conversation API client to the Dapr JS SDK, supporting both HTTP and gRPC protocols (gRPC throws GRPCNotSupportedError). The implementation includes two new request fields (responseFormat, promptCacheRetention) and two new response fields (model, usage) as specified in issue #732.
Changes:
- New HTTP and gRPC conversation client implementations with camelCase-to-snake_case mapping for the Dapr API
- New types for conversation messages, inputs, options, and responses including token usage tracking
- Public exports and
DaprClientintegration with unit tests for both protocols
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/implementation/Client/HTTPClient/conversation.ts | HTTP conversation client with request/response field mapping |
| src/implementation/Client/GRPCClient/conversation.ts | gRPC stub that throws GRPCNotSupportedError |
| src/implementation/Client/DaprClient.ts | Wires conversation client into DaprClient for both protocols |
| src/interfaces/Client/IClientConversation.ts | Interface for conversation client |
| src/types/conversation/ConversationMessage.type.ts | Message types with role-based union |
| src/types/conversation/ConversationInput.type.ts | Input type with messages and scrubPII |
| src/types/conversation/ConversationOptions.type.ts | Options type with new responseFormat and promptCacheRetention fields |
| src/types/conversation/ConversationResponse.type.ts | Response type with new model and usage fields |
| src/index.ts | Public exports for all new conversation types |
| test/unit/protocols/http/conversation.test.ts | HTTP conversation unit tests |
| test/unit/protocols/grpc/conversation.test.ts | gRPC conversation unit test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #736 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 6 6
Branches 1 1
=========================================
Hits 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9249254 to
bfb89ba
Compare
|
@sicoyle I've updated pubsub.ts to address CI failure. Please review and let me know your feedback. |
|
@1Ninad You're going to ultimately need to sign your commits before we can merge this as well. I'll try to get this reviewed this week. |
Signed-off-by: Ninad Kale <ninadkale200@gmail.com>
Fix by Copilot Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ninad Kale <145228622+1Ninad@users.noreply.github.com> Signed-off-by: Ninad Kale <ninadkale200@gmail.com>
Signed-off-by: Ninad Kale <ninadkale200@gmail.com>
…wire format Signed-off-by: Ninad Kale <ninadkale200@gmail.com>
ea6f8dc to
177b248
Compare
Done |
|
Hi @sicoyle @WhitWaldo, could you please review this and let me know if any changes are required? Thanks. |
If you could add some tests to make the codecov build step happy then that would be great @1Ninad 🙏 |
Signed-off-by: Ninad Kale <ninadkale200@gmail.com>
Signed-off-by: Ninad Kale <ninadkale200@gmail.com>
e826ba6 to
16202ef
Compare
Signed-off-by: Ninad Kale <ninadkale200@gmail.com>
Signed-off-by: Ninad Kale <ninadkale200@gmail.com>
|
Hi @sicoyle @WhitWaldo, looks like the E2E failures might not be related to this PR. |
|
It's probably flaky - one of my many goals this release. I'll review and get this merged soon. |
Thank you 👍 |
WhitWaldo
left a comment
There was a problem hiding this comment.
First of all - thank you for putting this PR together!
Several of the types do not match what's in the prototypes. I've linked to their relevant bits for your convenience. While I'm perfectly fine referring to the statistics as input/output tokens instead of prom[t/completion tokens, we shouldn't seek to flatten any of the options or responses at this time until the API is in a stable place.
Unfortunately, this means the test will also need some changes, so I didn't review that until it's synced up.
Signed-off-by: Ninad Kale <ninadkale200@gmail.com>
Description
Adds the Conversation API client to the JS SDK. Includes the two new request fields (responseFormat, promptCacheRetention) and two new response fields (model, usage). Unit tests cover all new fields for both HTTP and gRPC protocols.
Issue reference
closes #732
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: