-
-
Notifications
You must be signed in to change notification settings - Fork 260
Replace bit Boilerplate's SmartComponents.LocalEmbeddings.SemanticKernel with text-embeddings-inference:cpu-1.8 (#11916) #11917
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
…nel with text-embeddings-inference:cpu-1.6 (bitfoundation#11916)
WalkthroughRemoves the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)
494-503: Add validation for embedding service configuration to prevent startup failures.The fallback
LocalTextEmbeddingGenerationServicehas been removed, creating a hard dependency on one of three external embedding services (OpenAI, AzureOpenAI, or HuggingFace). However, if none of these services are configured, theIEmbeddingGeneratordependency inProductEmbeddingServicewill fail to resolve at startup or when the service is injected, resulting in aDependencyInjectionExceptionrather than a clear error message.The error handling in
ProductEmbeddingService(lines 26-27, 61-62) only addresses the case where embeddings are registered but disabled viaAppDbContext.IsEmbeddingEnabled. There is no validation inProgram.Services.csto warn users if none of the three external embedding services are configured.Add an explicit check after the if-else-if chain to validate that at least one embedding service is configured, with a clear exception message guiding users to configure one of the three supported services.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (6)
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.propssrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csprojsrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cssrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.jsonsrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Realms/dev-realm.jsonsrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/appsettings.json
💤 Files with no reviewable changes (2)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props
- src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj
🔇 Additional comments (3)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (2)
8-11: LGTM: Conditional compilation directives properly restructured.The using directive for
Npgsqlis now correctly placed within the PostgreSQL conditional compilation block, which is necessary for the vector extension support added later in the file.
281-288: LGTM: PostgreSQL vector support properly configured.The PostgreSQL data source builder is correctly configured with:
UseVector()to enable pgvector extension supportEnableDynamicJson()for dynamic JSON handling- The EF Core options properly call
UseVector()on the database optionsThis configuration is essential for the vector embedding operations that will use the new text-embeddings-inference service.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json (1)
67-68: Documentation quality confirmed with verified compatibility.The
EmbeddingModel__Commentprovides clear setup instructions, and verification confirms both points:
- HuggingFaceEmbeddingGenerator accepts the Uri endpoint format directly (Program.Services.cs:496), making it fully compatible with the self-hosted TEI service.
- BAAI/bge-small-en-v1.5 produces 384-dimensional embeddings, which matches the vector database schema configured as
vector(384)(ProductConfiguration.cs:22).
...ates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Realms/dev-realm.json
Outdated
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/appsettings.json
Show resolved
Hide resolved
Signed-off-by: Yas Moradi <ysmoradi@outlook.com>
closes #11916
Summary by CodeRabbit
New Features
Configuration
Chores
✏️ Tip: You can customize this high-level summary in your review settings.