-
-
Notifications
You must be signed in to change notification settings - Fork 254
Extend LLM and Embedding options across projects (#11494) #11495
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
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughMultiple projects are extended with OpenAI chat client and HuggingFace embedding support. GitHub workflows simplified to remove self-contained and Linux-specific runtime publishing. Sales website domain updated from bitservices.company to services.bitplatform.dev. GoogleTagManager configuration removed from platform and sales websites. Changes
Sequence Diagram(s)sequenceDiagram
participant Startup
participant Config as Configuration
participant Services as Dependency Container
participant ChatClient as ChatClient (OpenAI)
participant Cache as Distributed Cache
participant Logger as Logger
Startup->>Config: Load AppSettings
Config-->>Startup: OpenAI settings
alt Azure OpenAI Configured
Startup->>Services: Register Azure OpenAI ChatClient
else OpenAI API Key Present
rect rgb(220, 240, 255)
note right of Startup: New OpenAI Path
Startup->>Services: Register OpenAI ChatClient
Services->>ChatClient: Create with model, credentials, endpoint
Services->>Logger: Enable logging
Services->>Cache: Enable distributed cache
Services-->>Startup: ChatClient ready
end
else Fallback
Startup->>Services: Use alternative service
end
Startup-->>Services: Complete
sequenceDiagram
participant Startup
participant Config as Configuration
participant Services as Dependency Container
participant HF as HuggingFaceEmbeddingGenerator
participant OpenTelemetry as OpenTelemetry
Startup->>Config: Load AppSettings
Config-->>Startup: HuggingFace settings
alt OpenAI Configured
Startup->>Services: Register OpenAI Embeddings
else Azure OpenAI Configured
Startup->>Services: Register Azure OpenAI Embeddings
else HuggingFace Endpoint Present
rect rgb(240, 255, 240)
note right of Startup: New HuggingFace Path
Startup->>HF: Create HuggingFaceEmbeddingGenerator
HF->>HF: Configure with endpoint & API key
Services->>OpenTelemetry: Enable tracing
Services->>Logger: Enable logging
Services-->>Startup: Generator ready
end
else Fallback
Startup->>Services: Use LocalTextEmbeddingGenerationService
end
Startup-->>Services: Complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Changes span multiple projects with consistent patterns (configuration classes, service registration, package additions). Logic density is low to moderate—primarily conditional service registration. Heterogeneity is moderate: workflow simplifications are homogeneous, but OpenAI and HuggingFace integrations require separate reasoning for each context (BlazorUI Demo, Boilerplate, Platform). URL/domain updates are trivial. Review should verify conditional logic correctness, configuration consistency across projects, and package version alignment. Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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: 6
🧹 Nitpick comments (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/appsettings.json (1)
57-61: Consider documenting the asymmetric provider configuration structure.The HuggingFace configuration (lines 58-61) only includes embedding options, unlike OpenAI and AzureOpenAI which support both chat and embedding. While this may be intentional (if HuggingFace is only used for embeddings in this codebase), it could be clearer to developers if the appsettings structure or documentation explicitly states which providers support which capabilities.
Consider adding an inline comment documenting the intended use case:
"HuggingFace": { "EmbeddingApiKey": null, // HuggingFace Semantic Kernel connector - embeddings only "EmbeddingEndpoint": null }src/Websites/Platform/src/Bit.Websites.Platform.Server/AppSettings.cs (1)
32-37: Consider consolidating with AzureOpenAIOptions to reduce duplication.
OpenAIOptionsandAzureOpenAIOptions(lines 25-30) have identical properties. While keeping them separate provides clarity, you could optionally refactor to a shared base class or a singleChatClientOptionsclass with a provider discriminator if future requirements allow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (23)
.github/workflows/admin-sample.cd.yml(1 hunks).github/workflows/blazorui.demo.cd.yml(1 hunks).github/workflows/platform.website.cd.yml(1 hunks).github/workflows/sales-module-demo.cd.yml(1 hunks).github/workflows/sales.website.cd.yml(1 hunks).github/workflows/todo-sample.cd.yml(1 hunks)src/BlazorUI/Demo/Bit.BlazorUI.Demo.Server/AppSettings.cs(2 hunks)src/BlazorUI/Demo/Bit.BlazorUI.Demo.Server/Bit.BlazorUI.Demo.Server.csproj(1 hunks)src/BlazorUI/Demo/Bit.BlazorUI.Demo.Server/Startup/Services.cs(2 hunks)src/BlazorUI/Demo/Bit.BlazorUI.Demo.Server/appsettings.json(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/ServerApiSettings.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/appsettings.json(1 hunks)src/Websites/Platform/src/Bit.Websites.Platform.Server/AppSettings.cs(2 hunks)src/Websites/Platform/src/Bit.Websites.Platform.Server/Bit.Websites.Platform.Server.csproj(1 hunks)src/Websites/Platform/src/Bit.Websites.Platform.Server/Startup/Services.cs(2 hunks)src/Websites/Platform/src/Bit.Websites.Platform.Server/appsettings.json(1 hunks)src/Websites/Sales/src/Bit.Websites.Sales.Client/Components/PageOutlet.razor(2 hunks)src/Websites/Sales/src/Bit.Websites.Sales.Client/Shared/Footer.razor(1 hunks)src/Websites/Sales/src/Bit.Websites.Sales.Server/appsettings.json(0 hunks)
💤 Files with no reviewable changes (1)
- src/Websites/Sales/src/Bit.Websites.Sales.Server/appsettings.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: build api + blazor web
- GitHub Check: build api + blazor web
- GitHub Check: build blazor hybrid (windows)
- GitHub Check: build api + blazor web
- GitHub Check: build api + blazor web
- GitHub Check: build blazor hybrid (android)
- GitHub Check: build blazor hybrid (iOS-macOS)
- GitHub Check: build blazor hybrid (windows)
- GitHub Check: build and test
- GitHub Check: build blazor hybrid (android)
- GitHub Check: build blazor hybrid (iOS-macOS)
- GitHub Check: build blazor hybrid (windows)
- GitHub Check: build blazor wasm standalone
- GitHub Check: build api + blazor web
- GitHub Check: build and test
🔇 Additional comments (15)
src/Websites/Sales/src/Bit.Websites.Sales.Client/Shared/Footer.razor (1)
12-12: LGTM! Email contact updated correctly.The email link has been properly updated to align with the new domain migration.
src/Websites/Sales/src/Bit.Websites.Sales.Client/Components/PageOutlet.razor (1)
7-7: LGTM! Domain migration applied correctly.The Open Graph URL and default image URL have been properly updated to the new domain, ensuring consistent metadata across the sales website.
Also applies to: 23-23
.github/workflows/platform.website.cd.yml (1)
36-37: Verify deployment environment compatibility with framework-dependent publishing.The Publish step was changed from self-contained (
--self-contained -r linux-x64) to framework-dependent deployment. This means the target environment must have .NET 10.0 runtime pre-installed. While Azure App Service typically handles this automatically, you should confirm that:
- All deployment targets (especially any on-premises or edge deployments) have the required .NET 10.0 runtime installed
- Deployment automation scripts don't rely on self-contained runtime bundling
- Container images (if used) include the .NET 10.0 runtime
src/BlazorUI/Demo/Bit.BlazorUI.Demo.Server/Bit.BlazorUI.Demo.Server.csproj (1)
24-24: No action required — preview version is the only available release.The Microsoft.Extensions.AI.OpenAI package (9.10.0-preview.1.25513.3) has no stable release available; it is only published as prerelease. The version in use is the latest prerelease, and this is consistent with related packages in this file (e.g., Microsoft.Extensions.AI.AzureAIInference).
src/BlazorUI/Demo/Bit.BlazorUI.Demo.Server/appsettings.json (1)
25-30: LGTM!The OpenAI configuration block is properly structured and mirrors the existing AzureOpenAI pattern, providing a consistent configuration experience.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj (1)
68-68: LGTM!The HuggingFace connector package reference is properly added with conditional logic that matches the surrounding AI-related package patterns.
src/Websites/Platform/src/Bit.Websites.Platform.Server/appsettings.json (1)
24-29: LGTM!The OpenAI configuration block follows the same structure as the existing AzureOpenAI configuration, maintaining consistency across the application settings.
src/Websites/Platform/src/Bit.Websites.Platform.Server/Bit.Websites.Platform.Server.csproj (1)
13-13: LGTM!The OpenAI package reference is properly added with an explicit version, consistent with the versioning pattern used throughout this project file.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json (1)
65-69: LGTM!The HuggingFace configuration block is properly structured and follows the same pattern as OpenAI and AzureOpenAI options. Note that the
EmbeddingEndpointtype inconsistency flagged inServerApiSettings.csshould be addressed to ensure this configuration uses the correct type.src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (1)
89-89: LGTM!The HuggingFace connector package version is properly added to central package management with conditional logic that matches the surrounding AI-related package patterns.
src/BlazorUI/Demo/Bit.BlazorUI.Demo.Server/AppSettings.cs (2)
12-13: LGTM!The OpenAI property is properly added to the AppSettings class, extending AI provider support alongside the existing AzureOpenAI configuration.
33-39: LGTM!The OpenAIOptions class properly mirrors the structure of AzureOpenAIOptions with consistent property types, including the correct use of
Uri?for the endpoint property.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)
445-458: HuggingFace embedding integration looks good.The implementation correctly follows the established pattern for OpenAI and Azure OpenAI embedding generators. The else-if chain provides a clear fallback order, and all necessary middleware (logging, OpenTelemetry) are consistently applied.
src/Websites/Platform/src/Bit.Websites.Platform.Server/Startup/Services.cs (1)
7-7: Using directive correctly added.
System.ClientModel.Primitivesis required forHttpClientPipelineTransportused on line 64.src/BlazorUI/Demo/Bit.BlazorUI.Demo.Server/Startup/Services.cs (1)
1-2: Using directives correctly added.Both directives are required:
System.ClientModel.PrimitivesforHttpClientPipelineTransport(line 73), andSystem.IO.CompressionforCompressionLevel(lines 84, 94).
closes #11494
Summary by CodeRabbit
New Features
Chores