Skip to content
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

[Langchain-AWS-67] Refactor ChatBedrock._combine_llm_outputs() to mat… #68

Conversation

adarmiento
Copy link

…ch other Langchain model output

Changes

  • Replaced usage with token_usage
  • Replaced model_id with model_name

These change make the Runnable output of Bedrock hosted models consistent with other models supported by Langchain, such as OpenAI chat models.

…ch other Langchain model output

 # Changes

* Replaced `usage` with `token_usage`
* Replaced `model_id` with `model_name`

These change make the Runnable output of Bedrock hosted models consistent with other models supported by Langchain, such as OpenAI chat models.
final_usage[token_type] += token_count
final_output.update(output)
final_output["usage"] = final_usage
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a breaking change.

what we probably want to do is use the new AIMessage.token_usage attribute which is meant to be the standardized way to record token usage: langchain-ai/langchain#21944

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, the standard Langchain is moving toward is the usage_metadata attribute on AIMessage, which is just a dict with keys "input_tokens", "output_tokens", "total_tokens". If you're able to access the usage data within the _generate method, you can construct this dict and then populate usage_metadata on the AIMessage it generates. Note that we'd need to bump langchain-core >= 0.2.2 in the dependencies.

@baskaryan baskaryan requested a review from ccurme June 4, 2024 17:27
@ccurme
Copy link
Contributor

ccurme commented Aug 23, 2024

Believe usage_metadata is now correctly populated on output messages. Please let me know if more development is needed here.

@ccurme ccurme closed this Aug 23, 2024
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.

3 participants