-
Notifications
You must be signed in to change notification settings - Fork 19.4k
chore(perplexity): Added all keys for usage metadata #33480
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
base: master
Are you sure you want to change the base?
chore(perplexity): Added all keys for usage metadata #33480
Conversation
CodSpeed Performance ReportMerging #33480 will not alter performanceComparing Summary
Footnotes
|
citation_tokens = token_usage.get("citation_tokens", 0) | ||
num_search_queries = token_usage.get("num_search_queries", 0) | ||
reasoning_tokens = token_usage.get("reasoning_tokens", 0) | ||
return UsageMetadata( # type: ignore[typeddict-unknown-key] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the linter is complaining here because UsageMetadata
does not support extra keys.
We should follow the existing types where possible: https://python.langchain.com/api_reference/core/messages/langchain_core.messages.ai.UsageMetadata.html
reasoning_tokens
has a dedicated slot reasoning
under output_token_details
.
citation_tokens
we can also stash in output_token_details
.
num_search_queries
IMO can go in response_metadata since it isn't a token count but I don't feel strongly about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ccurme please check now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, could we add a test?
output_token_details: OutputTokenDetails = {} | ||
output_token_details["reasoning"] = token_usage.get("reasoning_tokens", 0) | ||
output_token_details["citation_tokens"] = token_usage.get("citation_tokens", 0) # type: ignore[typeddict-unknown-key] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit) might make sense to only populate the keys if they are present in token_usage
.
@ccurme done |
ChatPerplexity
—citation_tokens
,num_search_queries
, andreasoning_token
— which are required for accurate cost calculation, as noted in the issue. These have now been added. AlthoughOutputTokenDetails
has restricted keys, all of them have been included for now.