-
Notifications
You must be signed in to change notification settings - Fork 35
update MEAI image generation support and basic usage tracking for streaming #83
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
WalkthroughThis pull request adds image generation configuration support to the GenerativeAI.Microsoft SDK by introducing response modalities and image aspect ratio properties. Two new constant keys are added to map these properties, new configuration classes are defined in the core generation config, and extension methods are updated to handle the additional properties. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/GenerativeAI.Microsoft/Extensions/MicrosoftExtensions.cs (1)
345-349: Consider adding aspect ratio format validation.The implementation correctly instantiates
ImageConfigand sets the aspect ratio. However, there's no validation for the aspect ratio format. Consider validating against expected patterns like"16:9","9:16","1:1","4:3","3:4"to catch configuration errors early.Example validation:
if (options.AdditionalProperties.TryGetValue(AdditionalPropertiesKeys.ImageConfigAspectRatio, out string? aspectRatio)) { + // Validate aspect ratio format (optional but recommended) + if (!string.IsNullOrEmpty(aspectRatio) && !System.Text.RegularExpressions.Regex.IsMatch(aspectRatio, @"^\d+:\d+$")) + { + throw new ArgumentException($"Invalid aspect ratio format: {aspectRatio}. Expected format: 'width:height' (e.g., '16:9')"); + } config.ImageConfig ??= new ImageConfig(); config.ImageConfig.AspectRatio = aspectRatio; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/GenerativeAI.Microsoft/AdditionalPropertyKeys.cs(1 hunks)src/GenerativeAI.Microsoft/Extensions/MicrosoftExtensions.cs(2 hunks)src/GenerativeAI/Types/ContentGeneration/Config/GenerationConfig.cs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/GenerativeAI.Microsoft/Extensions/MicrosoftExtensions.cs (4)
src/GenerativeAI.Microsoft/AdditionalPropertyKeys.cs (1)
AdditionalPropertiesKeys(6-27)src/GenerativeAI/Types/ContentGeneration/Config/GenerationConfig.cs (1)
ImageConfig(335-342)src/GenerativeAI/Types/ContentGeneration/Inputs/Content.cs (4)
Content(11-58)Content(16-19)Content(25-29)Content(40-44)src/GenerativeAI/Types/ContentGeneration/Inputs/Part.cs (3)
Part(12-96)Part(69-72)Part(79-82)
🔇 Additional comments (6)
src/GenerativeAI.Microsoft/AdditionalPropertyKeys.cs (1)
18-26: LGTM! Clean constant additions for image generation support.The new constants are well-documented and integrate properly with the generation config. The key name
"AspectRatio"is generic but acceptable given it's namespaced withinAdditionalPropertiesKeys.ImageConfig*.src/GenerativeAI.Microsoft/Extensions/MicrosoftExtensions.cs (3)
340-343: LGTM! Response modalities properly configured.The implementation correctly reads the modalities list from additional properties and assigns it to the generation config.
408-419: Good refactoring for streaming usage tracking.The change to pre-build the contents collection and append
UsageContentwhen usage data is available is well-implemented and enables the framework to aggregate usage information during streaming. This aligns with the PR objectives for basic usage tracking.
546-556: Review verified: Implementation is correct.The Vertex AI API's output token count does not include any thinking tokens; those are provided in a separate field. The implementation correctly uses
CandidatesTokenCountdirectly forOutputTokenCount, which already excludes thinking tokens as required by the PR objectives. The separateThoughtsTokenCountfield tracks thinking tokens independently, so no additional filtering is needed here.src/GenerativeAI/Types/ContentGeneration/Config/GenerationConfig.cs (2)
213-217: LGTM! ImageConfig property properly integrated.The new property follows the existing patterns in
GenerationConfigwith appropriate JSON serialization attributes and clear documentation.
332-342: LGTM! Well-defined ImageConfig class.The
ImageConfigclass is cleanly implemented with:
- Clear documentation including common aspect ratio examples
- Proper JSON property naming convention (
aspectRatio)- Nullable string type for optional configuration
Summary
Note
This provides basic usage support for streaming responses. Unlike other MEAI implementations:
Tested with Vertex AI.
Summary by CodeRabbit
Release Notes