Conversation
5f4b42e to
c1f0045
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the model configuration system to address issues #113 and #96 by introducing a type-safe, capability-based model architecture. The changes replace the string-based model selection with concrete model classes and interfaces.
Changes:
- Introduced abstract
AIModelbase class withLocalModelandCloudModelsubclasses, plus capability interfaces (IReasoningModel,IVisionModel) - Added
ModelRegistryto replace the deprecatedKnownModelsclass with thread-safe registration and lookup - Created concrete model classes for 30+ local models and 12+ cloud models with proper metadata
- Updated
Chatentity withModelId(string) andModelInstance(AIModel) properties for backward compatibility - Refactored all service layers, contexts, examples, and tests to use the new type-safe model system
Reviewed changes
Copilot reviewed 58 out of 58 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
src/MaIN.Domain/Models/Abstract/AIModel.cs |
Defines base model classes and generic variants for runtime model registration |
src/MaIN.Domain/Models/Abstract/IModelCapabilities.cs |
Defines capability interfaces for vision, reasoning, embedding, and TTS |
src/MaIN.Domain/Models/Abstract/ModelRegistry.cs |
Thread-safe registry for model registration and lookup with reflection-based initialization |
src/MaIN.Domain/Models/Concrete/LocalModels.cs |
30+ concrete local model definitions with download URLs and metadata |
src/MaIN.Domain/Models/Concrete/CloudModels.cs |
12+ cloud model definitions for OpenAI, Anthropic, Gemini, xAI, GroqCloud, DeepSeek, Ollama |
src/MaIN.Domain/Entities/Chat.cs |
Added ModelInstance property with lazy loading from ModelId |
src/MaIN.Services/Services/LLMService/LLMService.cs |
Updated to use LocalModel with type checking and interface-based capabilities |
src/MaIN.Core/Hub/Contexts/ChatContext.cs |
Added type-safe WithModel overloads accepting AIModel or generic type parameters |
src/MaIN.Core/Hub/Contexts/ModelContext.cs |
Updated download and registry operations to work with new model system |
Examples/**/*.cs |
Updated 15+ examples to use strongly-typed model classes |
| Test files | Updated unit and integration tests to use new model IDs and types |
Comments suppressed due to low confidence (2)
src/MaIN.Domain/Exceptions/Models/MissingModelIdException.cs:8
- The PublicErrorMessage still says "Model name cannot be null or empty" but should be updated to say "Model id cannot be null or empty" to match the renamed exception class and parameter.
src/MaIN.InferPage/Components/Pages/Home.razor:215 - Call to obsolete method WithModel.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/MaIN.Domain/Exceptions/Models/MissingModelInstanceException.cs
Outdated
Show resolved
Hide resolved
|
@srebrek Overall, great job. I left a few comments, but they were mostly minor things. I just wonder if keeping the old approach in all Agent Examples or MCP Examples was intentional. I mean, there's still no "new type-safe model system". For example, in |
Yes, agents should also be changed but I dont want to make this PR too bloated. In the next PR I will thoroughly analyze agents and make them use the new approach. |
…ration #119 - Make TTS models implement ITTSModel - Fix and add tests - Cleanup
- register test model before each test
Closes #113
Closes #96
Add Concrete classes for every supported model. Models can be either Local or Cloud and can implement various capability interfaces such as IReasoning or IVision... (more will be added soon). It is a foundation for safer runtime behavior eg. text to text model to generate an image. It is recommended to create concrete classes for not supported models by yourself and use generic types only for runtime cases eg. dynamically load modal as a plugin.
Tested on local examples and gemini examples.
Will add ImagaGeneration and ToolCalling interfaces in the future as this PR is already big.
Tested with examples for local models and gemini.