Features/refine model settings

[iceljc]

PR Type

Enhancement

---

Description

• Refactor file storage service interface and implementation

• Consolidate settings classes and improve configuration structure

• Add new convenience properties to dialog models

• Update file handling methods with improved options pattern

---

Diagram Walkthrough

flowchart LR
  A["File Storage Interface"] --> B["Options Pattern"]
  B --> C["Consolidated Settings"]
  C --> D["Dialog Model Enhancements"]
  D --> E["Updated Function Callbacks"]

Loading

File Walkthrough

	Relevant files
Enhancement	19 files RoleDialogModel.cs Add role convenience properties and JSON ignore                    +7/-0      FileSource.cs Rename FileSourceType to FileSource                                            +1/-1      IFileStorageService.cs Refactor methods to use options pattern                                    +4/-6      MessageFileModel.cs Update FileSource reference                                                            +1/-1      MessageFileOptions.cs Add new options classes for file operations                            +20/-0    KnowledgeBaseSettings.cs Remove duplicate SettingBase class                                              +0/-5      LlmConfigBase.cs Refactor LLM configuration hierarchy                                          +13/-10  SettingBase.cs Create centralized SettingBase class                                          +6/-0      Using.cs Add Settings namespace import                                                        +2/-1      FileInstructService.SelectFile.cs Update file selection with options pattern                              +42/-27  LocalFileStorageService.Conversation.cs Implement options pattern for file operations                        +68/-118 ConversationController.cs Update file retrieval calls                                                            +6/-6      HandleAudioRequestFn.cs Update audio file retrieval method                                              +6/-2      EditImageFn.cs Update settings injection and file saving                                +4/-5      GenerateImageFn.cs Add settings injection and update configuration                    +7/-7      ReadImageFn.cs Update image file handling with options                                    +29/-14  ReadPdfFn.cs Enhance PDF reading with conditional image conversion        +47/-8    FileHandlerSettings.cs Refactor settings structure and inheritance                            +10/-14  TencentCosService.Conversation.cs Implement options pattern for COS file operations                +71/-101
Formatting	1 files TokenStatsModel.cs Organize token properties with regions                                      +7/-1
Configuration changes	2 files FileCoreSettings.cs Remove PDF converter settings                                                        +0/-7      appsettings.json Update configuration structure for file handling                  +11/-14
Miscellaneous	1 files ImageCompletionProvider.Generation.cs Add missing using statement                                                            +1/-0
Tests	1 files NullFileStorageService.cs Update test service interface implementation                          +2/-12

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue MergeMessageFiles builds a merged list but returns the original collection instead of the merged one, likely dropping ordering and merge logic. private IEnumerable<MessageFileModel> MergeMessageFiles(IEnumerable<string> messageIds, IEnumerable<MessageFileModel> files) { var mergedFiles = new List<MessageFileModel>(); if (messageIds.IsNullOrEmpty()) { return mergedFiles; } var userFiles = files.Where(x => x.FileSource.IsEqualTo(FileSource.User)); var botFiles = files.Where(x => x.FileSource.IsEqualTo(FileSource.Bot)); foreach (var messageId in messageIds) { var users = userFiles.Where(x => x.MessageId == messageId).OrderBy(x => x.FileIndex, new MessageFileIndexComparer()).ToList(); var bots = botFiles.Where(x => x.MessageId == messageId).OrderBy(x => x.FileIndex, new MessageFileIndexComparer()).ToList(); if (!users.IsNullOrEmpty()) { mergedFiles.AddRange(users); } if (!bots.IsNullOrEmpty()) { mergedFiles.AddRange(bots); } } return files; Config Lookup GetLlmProviderModel reads both provider and model from the same state key 'image_edit_llm_provider'; likely the model should use a different state key. { var state = _services.GetRequiredService<IConversationStateService>(); var llmProviderService = _services.GetRequiredService<ILlmProviderService>(); var provider = state.GetState("image_edit_llm_provider"); var model = state.GetState("image_edit_llm_provider"); Path Construction GetMessageFile constructs dir without including messageId segment, which may lead to incorrect file lookups in COS. { var dir = $"{CONVERSATION_FOLDER}/{conversationId}/{FILE_FOLDER}/{source}/{index}"; var fileList = _cosClient.BucketClient.GetDirFiles(dir); var found = fileList.FirstOrDefault(f => Path.GetFileNameWithoutExtension(f).IsEqualTo(fileName)); return found;

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consolidate settings for file handling The ImageConverter setting in FileCoreSettings is now redundant due to refactoring. It should be removed to simplify the configuration structure and avoid potential conflicts with the more specific settings in FileHandlerSettings. Examples: src/Infrastructure/BotSharp.Abstraction/Files/FileCoreSettings.cs [5-9] public class FileCoreSettings { public string Storage { get; set; } = FileStorageEnum.LocalFileStorage; public SettingBase? ImageConverter { get; set; } } src/Plugins/BotSharp.Plugin.FileHandler/Settings/FileHandlerSettings.cs [30-33] public class ImageEditSettings : LlmBase { public SettingBase? ImageConverter { get; set; } } Solution Walkthrough: Before: // src/Infrastructure/BotSharp.Abstraction/Files/FileCoreSettings.cs public class FileCoreSettings { public string Storage { get; set; } public SettingBase? ImageConverter { get; set; } // Redundant setting } // src/Plugins/BotSharp.Plugin.FileHandler/Settings/FileHandlerSettings.cs public class FileHandlerSettings { public ImageSettings? Image { get; set; } public PdfSettings? Pdf { get; set; } } public class ImageEditSettings : LlmBase { public SettingBase? ImageConverter { get; set; } } After: // src/Infrastructure/BotSharp.Abstraction/Files/FileCoreSettings.cs public class FileCoreSettings { public string Storage { get; set; } // ImageConverter setting is removed } // src/Plugins/BotSharp.Plugin.FileHandler/Settings/FileHandlerSettings.cs public class FileHandlerSettings { public ImageSettings? Image { get; set; } public PdfSettings? Pdf { get; set; } } public class ImageEditSettings : LlmBase { public SettingBase? ImageConverter { get; set; } } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a redundant ImageConverter setting in FileCoreSettings that was orphaned by the refactoring, and its removal would improve configuration clarity and prevent future confusion.	Medium
Possible issue	Prevent potential null reference exception Replace Last() with LastOrDefault() when splitting directory paths to prevent a potential exception if the split results in an empty sequence. src/Infrastructure/BotSharp.Core/Files/Services/Storage/LocalFileStorageService.Conversation.cs [79-81] var sources = options?.Sources != null ? options.Sources - : Directory.GetDirectories(baseDir).Select(x => x.Split(Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries).Last()); + : Directory.GetDirectories(baseDir) + .Select(x => x.Split(Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries).LastOrDefault()) + .Where(x => !string.IsNullOrEmpty(x)) + .ToList(); Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a potential InvalidOperationException from using Last() on a sequence that could be empty in an edge case, and provides a more robust solution using LastOrDefault().	Low
More