Refactor vector embedding in bit Boilerplate (#11448) #11450
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 Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRenames product search API/action and updates client call. Introduces embedding-based search with configurable dimensions, local fallback embedding generator, and database-specific vector setup and indexing. Narrows feature inclusion to PostgreSQL/SqlServer in several template conditions. Adds embedding-related packages and appsettings. Minor formatting-only changes elsewhere. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client as Client (ProductsPage)
participant API as Server API (ProductController)
participant EmbedSvc as ProductEmbeddingService
participant DB as Database
User->>Client: Enter search text
Client->>API: SearchProducts(searchQuery)
API->>EmbedSvc: SearchProducts(searchQuery, odata, ct)
alt Embeddings enabled and supported DB
EmbedSvc->>EmbedSvc: Generate embedding vector for query
EmbedSvc->>DB: Vector similarity query (cosine distance <= threshold)
DB-->>EmbedSvc: Paged products
EmbedSvc-->>API: PagedResult<ProductDto>
API-->>Client: Results
else Embeddings disabled/unsupported DB
API-->>Client: Error (NotImplemented / InvalidOperation)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Pull Request Overview
This PR refactors vector embedding functionality in the bit Boilerplate project to improve implementation and make it more flexible. The refactor focuses on simplifying the embedding service, adding proper configuration support, and making the code more maintainable.
Key Changes:
- Refactored ProductEmbeddingService to use weighted multi-text embeddings and improved vector generation
- Added EmbeddingOptions configuration to appsettings.json files for better configurability
- Updated method names from
GetProductsBySearchQuerytoSearchProductsfor better clarity
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| IProductController.cs | Renamed method from GetProductsBySearchQuery to SearchProducts |
| appsettings.json (both) | Added EmbeddingOptions configuration with dimensions setting |
| AppHub.Chatbot.cs | Updated conditional compilation directive for Sales module |
| ProductEmbeddingService.cs | Major refactor with weighted embedding generation and simplified API usage |
| Program.Services.cs | Added local embedding support and configuration binding |
| ProductConfiguration.cs | Updated vector dimensions and added PostgreSQL index configuration |
| SystemPromptConfiguration.cs | Updated conditional compilation for car recommendation rules |
| ProductController.cs | Updated method name and conditional compilation directives |
| Boilerplate.Server.Api.csproj | Added new package references for embedding support |
| Directory.Packages.props | Added version specifications for new embedding packages |
| ProductsPage.razor.cs | Updated method call to use new SearchProducts name |
| template.json | Removed SignalR dependency from ProductEmbeddingService exclusion condition |
...rplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ProductEmbeddingService.cs
Show resolved
Hide resolved
...rplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ProductEmbeddingService.cs
Show resolved
Hide resolved
...rplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ProductEmbeddingService.cs
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json (1)
46-55: Use AzureAIInference client for Azure AI Inference endpoint
TheOpenAIsection in appsettings.json targets models.inference.ai.azure.com (Azure AI Inference); you must register the AzureAIInference client (Microsoft.Extensions.AI.AzureAIInference) instead of the OpenAI client (Microsoft.Extensions.AI.OpenAI), or authentication/routing will fail. Update your DI setup (e.g., in Program.cs/Startup) to use AddAzureAIInferenceClient for that section.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Products/ProductController.cs (3)
86-97: Mismatched compile guards can break builds (property injection vs. call site)Embed calls are included when (database PG/SqlServer OR signalR), but the injected productEmbeddingService exists only for (database PG/SqlServer). Templates with signalR=true and a non‑PG/SqlServer DB will fail to compile.
Apply:
-//#if (database == "PostgreSQL" || database == "SqlServer" || signalR == true) +//#if (database == "PostgreSQL" || database == "SqlServer")Do the same for the matching block in Update below.
119-130: Same guard mismatch hereAlign with the injection guard to avoid missing-field compile errors.
Apply:
-//#if (database == "PostgreSQL" || database == "SqlServer" || signalR == true) +//#if (database == "PostgreSQL" || database == "SqlServer")
49-63: Use RestException for 501 and add empty-query guard
- Early-return when
searchQueryis null or whitespace by callingGetProducts(odataQuery, cancellationToken).- Instead of throwing
NotSupportedException(which the global handler maps to 500), throw aRestExceptionwithHttpStatusCode.NotImplementedand clear message (or change the action to returnActionResult<PagedResult<ProductDto>>andreturn StatusCode(501, "...")) so the handler emits a 501.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Configurations/Chatbot/SystemPromptConfiguration.cs (1)
177-197: Align all template guard quoting to single quotes
Replace every occurrence of//#if (... module == "Sales" | "Admin" | database == "PostgreSQL" | "SqlServer" ...)with single-quote syntax (e.g.module == 'Sales',database == 'PostgreSQL', etc.) across the codebase to ensure consistency and prevent template-eval regressions.
🧹 Nitpick comments (11)
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (2)
81-85: Preview Microsoft.Extensions.AI packages pinned.9.9.0-preview.* is fine for templates, but consider tracking the same “train” as your ASP.NET Core/EF versions to reduce churn for consumers when .NET 10 GA drops.
37-44: Unify OpenTelemetry package versions to the same release train. EF Core, Hangfire and resource detector packages currently only ship as prereleases (1.12.0-beta.*), while core and AspNetCore instrumentation are on stable 1.12.0. Either pin everything to 1.12.0-beta (e.g. beta.2) or wait for GA before mixing stable and prereleases.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Configurations/Product/ProductConfiguration.cs (1)
4-4: Namespace mismatch.The file configures products but lives under
...Configurations.Identity. Rename for consistency and discoverability.Apply:
-namespace Boilerplate.Server.Api.Data.Configurations.Identity; +namespace Boilerplate.Server.Api.Data.Configurations.Product;src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs (1)
101-105: Use appropriate log level.Saving a user’s email/conversation isn’t an error. Prefer
LogInformation(orLogWarningif needed).- .LogError("Chat reported issue: User email: {emailAddress}, Conversation history: {conversationHistory}", emailAddress, conversationHistory); + .LogInformation("Chat reported issue: User email: {emailAddress}, Conversation history: {conversationHistory}", emailAddress, conversationHistory);src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Products/ProductsPage.razor.cs (1)
70-78: Use culture-invariant lowercasing in OData filters.Avoid locale surprises (e.g., Turkish ‘i’).
- query.Filter = $"contains(tolower({nameof(ProductDto.Name)}),'{ProductNameFilter.ToLower()}')"; + query.Filter = $"contains(tolower({nameof(ProductDto.Name)}),'{ProductNameFilter.ToLowerInvariant()}')"; ... - query.AndFilter = $"contains(tolower({nameof(ProductDto.CategoryName)}),'{CategoryNameFilter.ToLower()}')"; + query.AndFilter = $"contains(tolower({nameof(ProductDto.CategoryName)}),'{CategoryNameFilter.ToLowerInvariant()}')";src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Products/ProductController.cs (1)
53-55: OData interplay with vector searchIgnoring OrderBy is right; consider also ignoring $filter on vector-only fields to avoid unexpected DB plans. Optional.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (3)
439-444: Bind embedding options here is good; add early validation to catch dimension mismatchConsider validating AI:EmbeddingOptions on startup (e.g., Dimensions > 0) and logging the resolved dimension to match DB vector column.
455-461: Same: option binding for Azure pathMirror the validation/logging here.
401-475: Resiliency for AI HTTP callsAdd transient error handling (retry/backoff) to the "AI" HttpClient and set sensible timeouts; current code relies on defaults.
services.AddHttpClient("AI") + .SetHandlerLifetime(TimeSpan.FromMinutes(5)) + .AddStandardResilienceHandler(); // .NET 8+ or add Polly policies if earliersrc/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ProductEmbeddingService.cs (2)
19-19: Make DISTANCE_THRESHOLD configurableHardcoding 0.65 risks mismatch across environments. Bind from configuration (e.g., AI:EmbeddingOptions:DistanceThreshold) or inject via options.
26-32: Add guard for trivial/empty search queriesAvoid embedding calls for empty/whitespace; return an empty page or reuse GetProducts with default sorting.
- if (AppDbContext.IsEmbeddingEnabled is false) + if (AppDbContext.IsEmbeddingEnabled is false) throw new InvalidOperationException("Embeddings are not enabled. Please enable them to use this feature."); + if (string.IsNullOrWhiteSpace(searchQuery)) + return dbContext.Products.AsQueryable(); + // It would be a good idea to try finding products using full-text search first, and if not enough results are found, then use the vector-based search.
📜 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 (13)
src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Products/ProductsPage.razor.cs(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/Controllers/Products/ProductController.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Configurations/Chatbot/SystemPromptConfiguration.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Configurations/Product/ProductConfiguration.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs(4 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ProductEmbeddingService.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs(1 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/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Controllers/Products/IProductController.cs(1 hunks)
⏰ 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). (1)
- GitHub Check: build and test
🔇 Additional comments (12)
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (3)
81-85: Gating for AI/embeddings is consistent with template changes.Including the AI packages when either SignalR is enabled or the DB is PostgreSQL/SqlServer matches the updated template condition. Looks good.
Please generate the template once with database=Sqlite and once with database=PostgreSQL to ensure no unintended AI deps leak into Sqlite output.
79-80: SignalR package compatibility confirmed. Microsoft.Azure.SignalR 1.32.0 targets .NETStandard2.0 and is compatible with net10.0, matching ASP.NET Core SignalR.Client 10.0.0-rc.1—no changes needed.
86-86: Verify Pgvector.EntityFrameworkCore 0.2.2 compatibility with EF Core 10 RC
0.2.2 (released Mar 26, 2025) is the latest and supports HNSW/IVFFlat indexing but doesn’t officially target .NET/EF Core 10. Test this setup against your EF Core 10 RC; upgrade to a newer release once official EF Core 10 support is available.src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json (1)
529-533: Embedding service gating change looks right.Dropping the SignalR check and gating solely on PostgreSQL/SqlServer matches the new embeddings story. Ensure any DB-specific config/migrations (vector type, indexes) are also gated the same way.
If possible, smoke‑test scaffolding:
- database=Sqlite → ProductEmbeddingService.cs absent
- database=PostgreSQL → present and compiles
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs (2)
116-136: Ignore outdated rename suggestion
TheProductEmbeddingServicestill definesGetProductsBySearchQuery(...)(see ProductEmbeddingService.cs:24), so no renaming toSearchProductsis required.Likely an incorrect or invalid review comment.
107-107: Gating matches supported databases
Confirmed that ProductEmbeddingService is registered and invoked only under PostgreSQL and SqlServer directives in both Program.Services.cs and AppHub.Chatbot.cs.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Products/ProductsPage.razor.cs (1)
81-84: Approve client call rename
UsingSearchProductsaligns with the server interface.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Configurations/Product/ProductConfiguration.cs (1)
13-36: Templating block comments may hide required mapping.
Lines 16–33 in the template are wrapped in a/* … */block, so if your engine doesn’t strip those comments when instantiating a project, EF will never see theEmbeddingmapping. Generate a new project from this template and inspectProductConfiguration.csto confirm thatHasColumnType("vector(384)")and the related index configuration are present.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/appsettings.json (1)
38-40: Verify EmbeddingOptions binding and remove duplicate config in Web
• Confirm that theAddEmbeddingGeneratorregistration inProgram.Services.csactually reads and applies theAI:EmbeddingOptions:Dimensionsvalue.
• Remove the redundantEmbeddingOptionsblock in the Web project’sappsettings.jsonand rely on the API’s config to prevent drift.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Products/ProductController.cs (1)
24-26: Good: scoped injection gated only for PostgreSQL/SQL ServerThis aligns DI surface with supported embedding backends.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)
72-75: Good: register ProductEmbeddingService only for PG/SQL ServerMatches persistence support surface.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ProductEmbeddingService.cs (1)
34-47: LGTM: server-side cosine distance ordering and thresholdCorrect use of vector_cosine_ops semantics (lower is more similar) with ascending ordering.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json
Show resolved
Hide resolved
.../Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj
Show resolved
Hide resolved
...rplate/src/Server/Boilerplate.Server.Api/Data/Configurations/Product/ProductConfiguration.cs
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs
Show resolved
Hide resolved
...rplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ProductEmbeddingService.cs
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Controllers/Products/IProductController.cs
Show resolved
Hide resolved
1 participant
closes #11448
Summary by CodeRabbit