Add text to image support to AP #44405
Conversation
Changes SummaryThis PR extends the Advanced Paste module to support text-to-image generation capabilities alongside the existing chat completion functionality. It introduces a new PasteAIUsage enum to differentiate between ChatCompletion and TextToImage operations, adds UI components to display image results, and implements the necessary backend logic to handle both modalities through the Semantic Kernel framework. Type: feature Components Affected: AdvancedPaste Module - Core Service, SemanticKernelPasteProvider - AI Provider Implementation, OptionsViewModel - UI Logic, Settings UI - Advanced Paste Configuration, XAML UI Components - PromptBox and AdvancedPastePage, Settings Library - Provider Definition Files Changed
Architecture Impact
Risk Areas: Image data URI parsing: Uses simple Split(',')[1] which could fail if data URI format is unexpected - no validation of base64 format, Memory management: MemoryStream created in CustomFormatImageResult getter without explicit disposal using 'using' statement - potential resource leak on repeated image conversions, Network operations: HttpClient in ProcessTextToImageAsync is created fresh each call - should use shared HttpClient or factory pattern, Exception handling: Broad catch-all exceptions in image parsing/creation could mask underlying issues; errors are logged but silently fail to user, Settings persistence: New provider fields (Usage, ImageWidth, ImageHeight) must be properly serialized/deserialized - no validation shown for image dimensions, Semantic Kernel versioning: Uses experimental APIs (#pragma warning SKEXP0001, SKEXP0010) which may change in future versions Suggestions
Full review in progress... | Powered by diffray |
| var base64Data = CustomFormatResult.Split(',')[1]; | ||
| var bytes = Convert.FromBase64String(base64Data); | ||
| var stream = new System.IO.MemoryStream(bytes); | ||
| var image = new BitmapImage(); | ||
| image.SetSource(stream.AsRandomAccessStream()); | ||
| return image; |
There was a problem hiding this comment.
🟠 HIGH - Split operation on string without bounds check
Agent: csharp
Category: bug
Description:
CustomFormatResult.Split(',')[1] is accessed without verifying the split produces at least 2 elements. If the data URI is malformed (missing comma), this will throw IndexOutOfRangeException.
Suggestion:
Use TrySplit or check array length before accessing index 1: var parts = CustomFormatResult.Split(','); if (parts.Length > 1) { var base64Data = parts[1]; }
Confidence: 85%
Rule: bug_collection_access_csharp
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| var base64Data = text.Split(',')[1]; | ||
| var bytes = Convert.FromBase64String(base64Data); | ||
| var stream = new System.IO.MemoryStream(bytes); | ||
| var dataPackage = DataPackageHelpers.CreateFromImage(Windows.Storage.Streams.RandomAccessStreamReference.CreateFromStream(stream.AsRandomAccessStream())); | ||
| await CopyPasteAndHideAsync(dataPackage); | ||
| } |
There was a problem hiding this comment.
🟠 HIGH - Split operation on string without bounds check
Agent: csharp
Category: bug
Description:
text.Split(',')[1] is accessed in PasteCustomAsync without verifying the split produces at least 2 elements. If a data URI is malformed, this will throw IndexOutOfRangeException.
Suggestion:
Validate the split result: var parts = text.Split(','); if (parts.Length > 1) { var base64Data = parts[1]; } else { Logger.LogError(...); return; }
Confidence: 85%
Rule: bug_collection_access_csharp
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| { | ||
| OnPropertyChanged(nameof(DisplayName)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM - Implicit default value in setter
Agent: csharp
Category: quality
Description:
ServiceType property setter silently changes null/whitespace to 'OpenAI' without documentation. This behavior should be documented.
Suggestion:
Add XML documentation to explain the default 'OpenAI' behavior, or consider making the default behavior more explicit.
Confidence: 60%
Rule: qual_misleading_names_csharp
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| public static class PasteAIUsageExtensions | ||
| { | ||
| public static string ToConfigString(this PasteAIUsage usage) | ||
| { | ||
| return usage switch | ||
| { | ||
| PasteAIUsage.ChatCompletion => "ChatCompletion", | ||
| PasteAIUsage.TextToImage => "TextToImage", | ||
| _ => "ChatCompletion", | ||
| }; | ||
| } | ||
|
|
||
| public static PasteAIUsage FromConfigString(string usage) | ||
| { | ||
| return usage switch | ||
| { | ||
| "TextToImage" => PasteAIUsage.TextToImage, | ||
| _ => PasteAIUsage.ChatCompletion, | ||
| }; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM - Missing XML documentation for public class and methods
Agent: csharp
Category: docs
Description:
The public static class PasteAIUsageExtensions and its public methods ToConfigString() and FromConfigString() lack XML documentation.
Suggestion:
Add
, , and tags to the class and all public methods.
Why this matters: Enables IntelliSense and consistent API usage across assemblies.
Confidence: 75%
Rule: cs_xml_documentation_for_public_apis
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
Rule:
cs_xml_documentation_for_public_apisReview ID:
0e2fcd21-993a-49b1-b1ab-9480f10905b9Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| public enum PasteAIUsage | ||
| { | ||
| ChatCompletion, | ||
| TextToImage, | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM - Missing XML documentation for public enum
Agent: csharp
Category: docs
Description:
The public enum PasteAIUsage lacks XML documentation. Enum members ChatCompletion and TextToImage are undocumented.
Suggestion:
Add
tag to the enum and tags to each enum member.
Why this matters: Enables IntelliSense and consistent API usage across assemblies.
Confidence: 70%
Rule: cs_xml_documentation_for_public_apis
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
Why this matters: Enables IntelliSense and consistent API usage across assemblies.
Confidence: 70%
Rule: cs_xml_documentation_for_public_apis
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| var width = _config.ImageWidth > 0 ? _config.ImageWidth : 1024; | ||
| var height = _config.ImageHeight > 0 ? _config.ImageHeight : 1024; |
There was a problem hiding this comment.
🟡 MEDIUM - Magic number 1024 for image dimensions
Agent: csharp
Category: quality
Description:
Magic numbers 1024 used for default image width and height. Should be extracted to named constants for clarity and maintainability.
Suggestion:
Extract to constants: private const int DefaultImageWidth = 1024; private const int DefaultImageHeight = 1024;
Why this matters: Magic numbers obscure intent and make changes error-prone when the same value is used in multiple places.
Confidence: 75%
Rule: qual_magic_numbers_csharp
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| return (serviceType.Equals("OpenAI", StringComparison.OrdinalIgnoreCase) || | ||
| serviceType.Equals("AzureOpenAI", StringComparison.OrdinalIgnoreCase)) | ||
| ? Visibility.Visible | ||
| : Visibility.Collapsed; |
There was a problem hiding this comment.
🔵 LOW - Magic string comparisons for service types
Agent: csharp
Category: quality
Description:
String literals 'OpenAI' and 'AzureOpenAI' used in string comparisons instead of using the AIServiceType enum directly.
Suggestion:
Use AIServiceType enum comparison via ServiceTypeKind property or convert using ToAIServiceType() method first.
Why this matters: Using enums improves type safety and enables compile-time checking.
Confidence: 65%
Rule: cs_use_enums_instead_of_magic_strings
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🟡 MEDIUM - Magic number 30 for cleanup delay
Agent: csharp
Category: quality
Description:
Magic number 30 used for temporary file cleanup delay timeout.
Suggestion:
Extract to constant: private const int CleanupDelaySeconds = 30;
Why this matters: Magic numbers obscure intent and make changes error-prone.
Confidence: 70%
Rule: qual_magic_numbers_csharp
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| private int _imageWidth = 1024; | ||
| private int _imageHeight = 1024; |
There was a problem hiding this comment.
🔵 LOW - Magic numbers for image dimensions
Agent: csharp
Category: quality
Description:
Hard-coded 1024 values for default image width and height. Should be extracted to constants.
Suggestion:
Extract to constants for reusability and maintainability
Why this matters: Magic numbers obscure intent and make changes error-prone.
Confidence: 70%
Rule: qual_magic_numbers_csharp
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| public class PasteAIProviderDefinition : INotifyPropertyChanged | ||
| { | ||
| private string _id = Guid.NewGuid().ToString("N"); | ||
| private string _serviceType = "OpenAI"; |
There was a problem hiding this comment.
🔵 LOW - Magic string 'OpenAI' as default service type
Agent: csharp
Category: quality
Description:
String literal 'OpenAI' used as default service type in field initialization. This duplicates the default found in AdvancedPasteViewModel.
Suggestion:
Extract to a constant or use AIServiceType enum for consistency
Why this matters: Improves type safety and consistency.
Confidence: 62%
Rule: cs_use_enums_instead_of_magic_strings
Review ID: 0e2fcd21-993a-49b1-b1ab-9480f10905b9
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
Review Summary
Validated 64 issues: 30 kept, 34 filtered Issues Found: 30💬 See 12 individual line comment(s) for details. 📊 13 unique issue type(s) across 30 location(s) 📋 Full issue list (click to expand)🟠 HIGH - Split operation on string without bounds check (2 occurrences)Agent: csharp Category: bug 📍 View all locations
Rule: 🟡 MEDIUM - Unsafe registry value casting without type validation (2 occurrences)Agent: security Category: security Why this matters: Avoids exceptions as control flow and handles invalid input gracefully. 📍 View all locations
Rule: 🟡 MEDIUM - Magic strings duplicated across multiple methods (2 occurrences)Agent: quality Category: quality Why this matters: Makes code harder to change safely. 📍 View all locations
Rule: 🟡 MEDIUM - Missing XML documentation for public class and methods (4 occurrences)Agent: csharp Category: docs Why this matters: Enables IntelliSense and consistent API usage across assemblies. 📍 View all locations
Rule: 🟡 MEDIUM - Event handler subscription without unsubscriptionAgent: csharp Category: quality Why this matters: This pattern commonly causes memory leaks in UI components. File: Description: ViewModel.PropertyChanged and ViewModel.PreviewRequested events are subscribed in constructor but may not be unsubscribed if the UserControl is reused or disposed. Suggestion: Implement IDisposable or override OnUnloaded to unsubscribe from ViewModel.PropertyChanged and ViewModel.PreviewRequested events. Confidence: 65% Rule: 🟡 MEDIUM - Empty catch block in Dispose method (4 occurrences)Agent: csharp Category: quality 📍 View all locations
Rule: 🟡 MEDIUM - Potential null reference in RestoreFoundrySelectionAgent: csharp Category: bug File: Description: FirstOrDefault can return null but matchingModel is correctly initialized to null and the assignment at line 628 handles it properly. This is actually safe code. Suggestion: Code is actually safe - matchingModel can be null and FoundryLocalPicker.SelectedModel can accept null. Consider keeping pattern for clarity. Confidence: 60% Rule: 🟡 MEDIUM - Implicit default value in setterAgent: csharp Category: quality File: Description: ServiceType property setter silently changes null/whitespace to 'OpenAI' without documentation. This behavior should be documented. Suggestion: Add XML documentation to explain the default 'OpenAI' behavior, or consider making the default behavior more explicit. Confidence: 60% Rule: 🟡 MEDIUM - FirstOrDefault used on query results with proper null checkAgent: csharp Category: bug File: Description: FirstOrDefault() is called on providers collection. The code at line 896 properly checks for null before further operations, and line 901 uses null-conditional access. Suggestion: Code is actually safe due to null check at line 896 and null-coalescing at line 901. Confidence: 65% Rule: 🟡 MEDIUM - FirstOrDefault() used where SingleOrDefault() might be appropriateAgent: csharp Category: quality File: Description: FirstOrDefault() is used to find a custom action by ID, which should be unique. SingleOrDefault() would document and enforce the uniqueness assumption. Suggestion: Consider using SingleOrDefault() instead of FirstOrDefault() if the ID is expected to be unique. Confidence: 60% Rule: 🟡 MEDIUM - Catching generic ExceptionAgent: csharp Category: quality Why this matters: Catching generic exceptions can mask bugs and make debugging difficult. File: Description: Catching broad Exception type when retrieving chat service by model ID. This masks specific failures that might need different handling. Suggestion: Catch specific exception types (InvalidOperationException, KeyNotFoundException) instead of base Exception, or add logging to understand what exceptions occur. Confidence: 65% Rule: 🟡 MEDIUM - Magic number 1024 for image dimensions (6 occurrences)Agent: csharp Category: quality Why this matters: Magic numbers obscure intent and make changes error-prone when the same value is used in multiple places. 📍 View all locations
Rule: 🔵 LOW - Magic string comparisons for service types (4 occurrences)Agent: csharp Category: quality Why this matters: Using enums improves type safety and enables compile-time checking. 📍 View all locations
Rule: ℹ️ 18 issue(s) outside PR diff (click to expand)
🟡 MEDIUM - Unsafe registry value casting without type validation (2 occurrences)Agent: security Category: security Why this matters: Avoids exceptions as control flow and handles invalid input gracefully. 📍 View all locations
Rule: 🟡 MEDIUM - Magic strings duplicated across multiple methods (2 occurrences)Agent: quality Category: quality Why this matters: Makes code harder to change safely. 📍 View all locations
Rule: 🟡 MEDIUM - Event handler subscription without unsubscriptionAgent: csharp Category: quality Why this matters: This pattern commonly causes memory leaks in UI components. File: Description: ViewModel.PropertyChanged and ViewModel.PreviewRequested events are subscribed in constructor but may not be unsubscribed if the UserControl is reused or disposed. Suggestion: Implement IDisposable or override OnUnloaded to unsubscribe from ViewModel.PropertyChanged and ViewModel.PreviewRequested events. Confidence: 65% Rule: 🟡 MEDIUM - Empty catch block in Dispose method (4 occurrences)Agent: csharp Category: quality 📍 View all locations
Rule: 🟡 MEDIUM - Potential null reference in RestoreFoundrySelectionAgent: csharp Category: bug File: Description: FirstOrDefault can return null but matchingModel is correctly initialized to null and the assignment at line 628 handles it properly. This is actually safe code. Suggestion: Code is actually safe - matchingModel can be null and FoundryLocalPicker.SelectedModel can accept null. Consider keeping pattern for clarity. Confidence: 60% Rule: 🟡 MEDIUM - FirstOrDefault used on query results with proper null checkAgent: csharp Category: bug File: Description: FirstOrDefault() is called on providers collection. The code at line 896 properly checks for null before further operations, and line 901 uses null-conditional access. Suggestion: Code is actually safe due to null check at line 896 and null-coalescing at line 901. Confidence: 65% Rule: 🟡 MEDIUM - FirstOrDefault() used where SingleOrDefault() might be appropriateAgent: csharp Category: quality File: Description: FirstOrDefault() is used to find a custom action by ID, which should be unique. SingleOrDefault() would document and enforce the uniqueness assumption. Suggestion: Consider using SingleOrDefault() instead of FirstOrDefault() if the ID is expected to be unique. Confidence: 60% Rule: 🟡 MEDIUM - Missing XML documentation for public class and constructorAgent: csharp Category: docs Why this matters: Enables IntelliSense and consistent API usage across assemblies. File: Description: The public sealed class CustomActionTransformService and its public constructor lack XML documentation. Suggestion: Add tag to the class and Confidence: 65% Rule: 🟡 MEDIUM - Magic string 'FoundryLocal' in equality check (2 occurrences)Agent: csharp Category: quality Why this matters: Improves type safety and discoverability. 📍 View all locations
Rule: 🟡 MEDIUM - Magic number 1 for timer interval (3 occurrences)Agent: csharp Category: quality Why this matters: Magic numbers obscure intent and make changes error-prone. 📍 View all locations
Rule: |
There was a problem hiding this comment.
Pull request overview
This pull request adds text-to-image generation support to Advanced Paste's AI capabilities, enabling users to generate images from text descriptions using OpenAI and Azure OpenAI providers.
Key Changes:
- Introduces
PasteAIUsageenum to distinguish between chat completion and text-to-image operations - Adds configurable image dimensions (width/height) with default 1024x1024 resolution
- Implements image display and pasting functionality for generated images returned as data URIs
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/settings-ui/Settings.UI.Library/PasteAIUsage.cs |
Defines new enum for AI usage types |
src/settings-ui/Settings.UI.Library/PasteAIUsageExtensions.cs |
Provides conversion methods between enum and config strings |
src/settings-ui/Settings.UI.Library/PasteAIProviderDefinition.cs |
Adds Usage, ImageWidth, and ImageHeight properties to provider configuration |
src/settings-ui/Settings.UI/ViewModels/AdvancedPasteViewModel.cs |
Updates provider synchronization to include new properties |
src/settings-ui/Settings.UI/SettingsXAML/Views/AdvancedPastePage.xaml |
Adds UI controls for usage selection and image resolution configuration |
src/settings-ui/Settings.UI/SettingsXAML/Views/AdvancedPastePage.xaml.cs |
Implements visibility logic and event handlers for text-to-image settings |
src/settings-ui/Settings.UI/Converters/PasteAIUsageToStringConverter.cs |
Converts usage enum to localized display strings in Settings UI |
src/settings-ui/Settings.UI/Strings/en-us/Resources.resw |
Adds localized strings for usage types and image dimensions |
src/modules/AdvancedPaste/AdvancedPaste/ViewModels/OptionsViewModel.cs |
Implements image display and pasting logic for data URI images |
src/modules/AdvancedPaste/AdvancedPaste/Services/CustomActions/SemanticKernelPasteProvider.cs |
Implements text-to-image generation using Semantic Kernel |
src/modules/AdvancedPaste/AdvancedPaste/Services/CustomActions/PasteAIConfig.cs |
Extends config with Usage, ImageWidth, and ImageHeight properties |
src/modules/AdvancedPaste/AdvancedPaste/Services/CustomActions/CustomActionTransformService.cs |
Populates new config properties when building provider config |
src/modules/AdvancedPaste/AdvancedPaste/Helpers/DataPackageHelpers.cs |
Adds helper method to create DataPackage from image stream |
src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Converters/PasteAIUsageToStringConverter.cs |
Converts usage enum to localized display strings in AdvancedPaste UI |
src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Controls/PromptBox.xaml |
Updates UI to display images and show usage badges |
src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Controls/PromptBox.xaml.cs |
Simplifies provider selection logic |
src/modules/AdvancedPaste/AdvancedPaste/Strings/en-us/Resources.resw |
Adds localized strings for usage types |
| case PasteAIUsage.TextToImage: | ||
| var imageDescription = string.IsNullOrWhiteSpace(prompt) ? inputText : $"{inputText}. {prompt}"; | ||
| return await ProcessTextToImageAsync(kernel, imageDescription, cancellationToken); |
There was a problem hiding this comment.
If a provider is configured with TextToImage usage for non-OpenAI/AzureOpenAI service types (e.g., through manual config file editing), this will attempt to retrieve ITextToImageService from the kernel but no such service will be registered (see lines 227-242 in CreateKernel), causing a runtime exception. Consider adding validation to ensure TextToImage usage is only allowed for OpenAI and AzureOpenAI providers, or add error handling to provide a clear error message.
| { | ||
| try | ||
| { | ||
| var base64Data = CustomFormatResult.Split(',')[1]; |
There was a problem hiding this comment.
The data URI parsing assumes the format 'data:type;base64,data' and directly accesses index [1] after splitting on comma. This will throw an IndexOutOfRangeException if the data URI doesn't contain a comma. Add validation to ensure the split result has at least 2 elements before accessing index [1].
| var base64Data = CustomFormatResult.Split(',')[1]; | |
| var parts = CustomFormatResult.Split(','); | |
| if (parts.Length < 2) | |
| { | |
| Logger.LogWarning("CustomFormatResult does not contain expected data URI payload segment."); | |
| return null; | |
| } | |
| var base64Data = parts[1]; |
| var base64Data = text.Split(',')[1]; | ||
| var bytes = Convert.FromBase64String(base64Data); |
There was a problem hiding this comment.
The data URI parsing assumes the format 'data:type;base64,data' and directly accesses index [1] after splitting on comma. This will throw an IndexOutOfRangeException if the data URI doesn't contain a comma. Add validation to ensure the split result has at least 2 elements before accessing index [1].
| { | ||
| var base64Data = CustomFormatResult.Split(',')[1]; | ||
| var bytes = Convert.FromBase64String(base64Data); | ||
| var stream = new System.IO.MemoryStream(bytes); |
There was a problem hiding this comment.
The MemoryStream created here is not being disposed. This can lead to resource leaks. Consider using a 'using' statement or calling Dispose() to ensure proper cleanup of the stream.
| var stream = new System.IO.MemoryStream(bytes); | |
| using var stream = new System.IO.MemoryStream(bytes); |
| } | ||
| else if (imageContent.Uri != null) | ||
| { | ||
| using var client = new HttpClient(); |
There was a problem hiding this comment.
Creating HttpClient with 'using' is an anti-pattern that can lead to socket exhaustion. HttpClient is designed to be reused and should be instantiated once and reused throughout the life of an application. Consider using a static HttpClient instance or IHttpClientFactory.
| using var client = new HttpClient(); | |
| var client = new HttpClient(); |
| var width = _config.ImageWidth > 0 ? _config.ImageWidth : 1024; | ||
| var height = _config.ImageHeight > 0 ? _config.ImageHeight : 1024; | ||
| var settings = new OpenAITextToImageExecutionSettings | ||
| { | ||
| Size = (width, height), | ||
| }; |
There was a problem hiding this comment.
The fallback values for image dimensions only check if they're greater than 0, but don't validate if they're within reasonable bounds. OpenAI's image generation APIs have specific size constraints. Consider adding validation to ensure width and height are within acceptable ranges for the specific model being used, and providing clear error messages if invalid dimensions are provided.
| var stream = new System.IO.MemoryStream(bytes); | ||
| var dataPackage = DataPackageHelpers.CreateFromImage(Windows.Storage.Streams.RandomAccessStreamReference.CreateFromStream(stream.AsRandomAccessStream())); | ||
| await CopyPasteAndHideAsync(dataPackage); |
There was a problem hiding this comment.
The MemoryStream created here is not being disposed. This can lead to resource leaks. Consider using a 'using' statement or calling Dispose() to ensure proper cleanup of the stream.
| public string Usage | ||
| { | ||
| get => _usage; | ||
| set => SetProperty(ref _usage, string.IsNullOrWhiteSpace(value) ? "ChatCompletion" : value); // TODO: Localization support |
There was a problem hiding this comment.
This TODO comment is misleading. The Usage property stores the configuration string value ("ChatCompletion" or "TextToImage"), which should not be localized. Localization is already properly handled in the UI converters (PasteAIUsageToStringConverter) that display these values to users. Consider removing this comment.
| set => SetProperty(ref _usage, string.IsNullOrWhiteSpace(value) ? "ChatCompletion" : value); // TODO: Localization support | |
| set => SetProperty(ref _usage, string.IsNullOrWhiteSpace(value) ? "ChatCompletion" : value); |
| <TextBox | ||
| x:Name="PasteAIImageWidthTextBox" | ||
| x:Uid="AdvancedPaste_ImgOutputWidth" | ||
| MinWidth="96" | ||
| Text="{x:Bind ViewModel.PasteAIProviderDraft.ImageWidth, Mode=TwoWay}" /> | ||
| <TextBlock | ||
| Margin="0,0,0,8" | ||
| VerticalAlignment="Bottom" | ||
| Text="x" /> | ||
| <TextBox | ||
| x:Name="PasteAIImageHeightTextBox" | ||
| x:Uid="AdvancedPaste_ImgOutputHeight" | ||
| MinWidth="96" | ||
| Text="{x:Bind ViewModel.PasteAIProviderDraft.ImageHeight, Mode=TwoWay}" /> |
There was a problem hiding this comment.
The TextBox controls for ImageWidth and ImageHeight lack input validation. Users can enter non-numeric values, negative numbers, zero, or extremely large values, which could cause binding errors or runtime issues. Consider adding input validation to ensure only positive integers within reasonable bounds (e.g., 64-4096) are accepted.
| var comboBox = (ComboBox)sender; | ||
| if (comboBox.SelectedValue is string usage && usage == "TextToImage") | ||
| { | ||
| ViewModel.PasteAIProviderDraft.EnableAdvancedAI = false; | ||
| PasteAIEnableAdvancedAICheckBox.IsEnabled = false; | ||
| } | ||
| else | ||
| { | ||
| PasteAIEnableAdvancedAICheckBox.IsEnabled = true; | ||
| } |
There was a problem hiding this comment.
There is duplicated logic between this event handler and the UpdatePasteAIUIVisibility method (lines 389-397). Both are setting EnableAdvancedAI to false and disabling the checkbox when usage is TextToImage. Consider consolidating this logic to avoid maintenance issues and potential inconsistencies.
| var comboBox = (ComboBox)sender; | |
| if (comboBox.SelectedValue is string usage && usage == "TextToImage") | |
| { | |
| ViewModel.PasteAIProviderDraft.EnableAdvancedAI = false; | |
| PasteAIEnableAdvancedAICheckBox.IsEnabled = false; | |
| } | |
| else | |
| { | |
| PasteAIEnableAdvancedAICheckBox.IsEnabled = true; | |
| } | |
| UpdatePasteAIUIVisibility(); |
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed