feat(conversation): add prompt cache + usage metrics to test cache#9265
feat(conversation): add prompt cache + usage metrics to test cache#9265sicoyle wants to merge 4 commits into
Conversation
Signed-off-by: Samantha Coyle <[email protected]>
Signed-off-by: Samantha Coyle <[email protected]>
| // Set of 16 key-value pairs that can be attached to the conversation. | ||
| // This can be useful for storing additional information about the object in a structured format, | ||
| // and querying for objects via API or the dashboard. | ||
| // Keys are strings with a maximum length of 64 characters. | ||
| // Values are strings with a maximum length of 512 characters. |
There was a problem hiding this comment.
Rather than this being a field on the request message, should the information which will be set here, go into the HTTP or gRPC request header itself? This is with the assumption that users of Dapr aren't expected to add anything here, and the presence of this field is confusing.
There was a problem hiding this comment.
I agree headers would be better. However, this field is already implemented in the SDKs, used in docs/quickstarts, and matches OpenAI's ChatCompletion API metadata field (https://github.com/openai/openai-go/blob/main/chatcompletion.go#L3010), which is passed through to LLM providers. To avoid breaking Alpha2 clients, I'd prefer to keep it for now with a comment noting it will move to headers in a future version. This preserves backward compatibility while aligning with OpenAI's API structure.
| // inspired by openai.ChatCompletion.Usage of type CompletionUsage | ||
| message ConversationResultAlpha2CompletionUsage { | ||
| // Number of tokens in the generated completion. | ||
| int64 completion_tokens = 1; |
There was a problem hiding this comment.
Should all these count fields be unsigned rather than signed?
There was a problem hiding this comment.
I matched openai directly here but will update.
| if err != nil { | ||
| return nil, err | ||
| } | ||
| request.Metadata = req.GetMetadata() |
There was a problem hiding this comment.
Here, we would take the metadata from the ctx, rather than from the request field.
Signed-off-by: Samantha Coyle <[email protected]>
3 participants
Description
This PR breaks out work from a larger contrib PR into a smaller, more focused change: #9241
It reviews the Conversation API from a Dapr Agents perspective, evaluates which fields are required, and aligns the API more closely with OpenAI’s where appropriate (including adding fields now and documenting candidates for future support).
Specifically, this PR:
Clarifies the semantics of the metadata field to represent request-level metadata passed through to the LLM provider, rather than component metadata as some documentation currently implies.
Adds a field to enable prompt caching. Dapr currently supports response caching only; this change mirrors OpenAI’s API, allows per-request configuration, and raises the question of whether prompt caching should also be supported as a global, component-level setting.
Adds token usage metrics as a result of testing that prompt caching indeed works.
Updates the LangChain dependency.
Updates integration tests to reflect the API changes.
Note: This PR depends on the corresponding components-contrib change being merged first for the build to pass: dapr/components-contrib#4154
Issue reference
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: