-
Notifications
You must be signed in to change notification settings - Fork 2k
refactor: enhance ChatGenerationMetadata with metadata Map and Builder #1806
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
Conversation
- ChatGenerationMetadata provides Map for additional metadata - Add Builder interface and DefaultChatGenerationMetadataBuilder - Add DefaultChatGenerationMetadata implementation - Update all AI model implementations to use the new builder pattern - Deprecate ChatGenerationMetadata.from() factory method Resolves spring-projects#1805
| chunk.amazonBedrockInvocationMetrics()); | ||
| chatGenerationMetadata = ChatGenerationMetadata.builder() | ||
| .finishReason(completionReason) | ||
| .metadata("usage", chunk.amazonBedrockInvocationMetrics()) |
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.
Should be "metrics" ?
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 class BedrockUsage has AmazonBedrockInvocationMetrics getUsage() so maybe it is the other way around, the class name should change? Prob not that important as we will remove this on the switch to bedrock converse.
| response.usage().outputTokens())); | ||
| chatGenerationMetadata = ChatGenerationMetadata.builder() | ||
| .finishReason(response.delta().stopReason()) | ||
| .metadata("usage", |
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.
Can we move all these common metadata keys to constants?
| } | ||
| }; | ||
| } | ||
| // /** |
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.
commented out code can be deleted.
|
removed commented out code and made other small changes. merged in f388f53 |
Resolves #1805