Skip to content

Commit f0649e9

Browse files
committed
Phase 3 Stage 3.12: Bundle 3 quick wins (CB1, B2, N3, N2)
- CB1: Remove redundant <InternalsVisibleTo> in Core.csproj; the [assembly:] attribute in SqlMetadataProvider.cs:26 already covers it. - B2: Include batchResult.ErrorMessage in the embedding-failure throw so the provider's actual reason (quota, auth, etc.) isn't dropped. - N3: Add SubstituteEmbedParametersAsync(RuntimeConfig, entityName) overload; 3 engine call sites collapse from 5 lines to 1. - N2: Sort using-order in 4 files so Embeddings precedes MetadataProviders. Verified locally: 45 tests pass, 0 build warnings.
1 parent 4cd9225 commit f0649e9

6 files changed

Lines changed: 58 additions & 26 deletions

File tree

src/Core/Azure.DataApiBuilder.Core.csproj

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,6 @@
5858
<ProjectReference Include="..\Service.GraphQLBuilder\Azure.DataApiBuilder.Service.GraphQLBuilder.csproj" />
5959
</ItemGroup>
6060

61-
<ItemGroup>
62-
<InternalsVisibleTo Include="Azure.DataApiBuilder.Service.Tests" />
63-
</ItemGroup>
64-
6561
<ItemGroup>
6662
<None Include="..\..\nuget\nuget_core\README.md" Pack="true" PackagePath="\" />
6763
<None Include="..\..\nuget\nuget_icon.png" Pack="true" PackagePath="\" />

src/Core/Resolvers/Factories/MutationEngineFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
using Azure.DataApiBuilder.Config.ObjectModel;
88
using Azure.DataApiBuilder.Core.Configurations;
99
using Azure.DataApiBuilder.Core.Models;
10-
using Azure.DataApiBuilder.Core.Services.MetadataProviders;
1110
using Azure.DataApiBuilder.Core.Services.Embeddings;
11+
using Azure.DataApiBuilder.Core.Services.MetadataProviders;
1212
using Azure.DataApiBuilder.Service.Exceptions;
1313
using Microsoft.AspNetCore.Http;
1414
using static Azure.DataApiBuilder.Config.DabConfigEvents;

src/Core/Resolvers/Factories/QueryEngineFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
using Azure.DataApiBuilder.Core.Configurations;
99
using Azure.DataApiBuilder.Core.Models;
1010
using Azure.DataApiBuilder.Core.Services.Cache;
11-
using Azure.DataApiBuilder.Core.Services.MetadataProviders;
1211
using Azure.DataApiBuilder.Core.Services.Embeddings;
12+
using Azure.DataApiBuilder.Core.Services.MetadataProviders;
1313
using Azure.DataApiBuilder.Service.Exceptions;
1414
using Microsoft.AspNetCore.Http;
1515
using Microsoft.Extensions.Logging;

src/Core/Resolvers/SqlMutationEngine.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
using Azure.DataApiBuilder.Core.Resolvers.Factories;
1616
using Azure.DataApiBuilder.Core.Resolvers.Sql_Query_Structures;
1717
using Azure.DataApiBuilder.Core.Services;
18-
using Azure.DataApiBuilder.Core.Services.MetadataProviders;
1918
using Azure.DataApiBuilder.Core.Services.Embeddings;
19+
using Azure.DataApiBuilder.Core.Services.MetadataProviders;
2020
using Azure.DataApiBuilder.Service.Exceptions;
2121
using Azure.DataApiBuilder.Service.GraphQLBuilder;
2222
using Azure.DataApiBuilder.Service.GraphQLBuilder.Mutations;
@@ -352,12 +352,12 @@ private static bool IsPointMutation(IMiddlewareContext context)
352352
IQueryBuilder queryBuilder = _queryManagerFactory.GetQueryBuilder(sqlMetadataProvider.GetDatabaseType());
353353
IQueryExecutor queryExecutor = _queryManagerFactory.GetQueryExecutor(sqlMetadataProvider.GetDatabaseType());
354354

355-
// Phase 3: substitute embed:true parameters with embedding vectors (REST mutation path)
356-
// Helper handles the case where embed params exist but service is null (throws 503).
357-
Entity entity = _runtimeConfigProvider.GetConfig().Entities[context.EntityName];
355+
// Phase 3: substitute auto-embed:true parameters with embedding vectors (REST mutation path).
356+
// Helper handles entity lookup, null-service detection, and per-param validation.
358357
await ParameterEmbeddingHelper.SubstituteEmbedParametersAsync(
359358
context.ResolvedParameters,
360-
entity.Source.Parameters,
359+
_runtimeConfigProvider.GetConfig(),
360+
context.EntityName,
361361
_embeddingService,
362362
_httpContextAccessor.HttpContext?.RequestAborted ?? CancellationToken.None);
363363

src/Core/Resolvers/SqlQueryEngine.cs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
using Azure.DataApiBuilder.Core.Resolvers.Factories;
1212
using Azure.DataApiBuilder.Core.Services;
1313
using Azure.DataApiBuilder.Core.Services.Cache;
14-
using Azure.DataApiBuilder.Core.Services.MetadataProviders;
1514
using Azure.DataApiBuilder.Core.Services.Embeddings;
15+
using Azure.DataApiBuilder.Core.Services.MetadataProviders;
1616
using Azure.DataApiBuilder.Service.Exceptions;
1717
using Azure.DataApiBuilder.Service.GraphQLBuilder;
1818
using Azure.DataApiBuilder.Service.GraphQLBuilder.Queries;
@@ -144,12 +144,12 @@ await ExecuteAsync(structure, dataSourceName, isMultipleCreateOperation: true),
144144
ISqlMetadataProvider sqlMetadataProvider = _sqlMetadataProviderFactory.GetMetadataProvider(dataSourceName);
145145
if (sqlMetadataProvider.GraphQLStoredProcedureExposedNameToEntityNameMap.TryGetValue(context.Selection.Field.Name, out string? entityName))
146146
{
147-
// Phase 3: substitute embed:true parameters with embedding vectors (GraphQL path)
148-
// Helper handles the case where embed params exist but service is null (throws 503).
149-
Entity entity = _runtimeConfigProvider.GetConfig().Entities[entityName];
147+
// Phase 3: substitute auto-embed:true parameters with embedding vectors (GraphQL path).
148+
// Helper handles entity lookup, null-service detection, and per-param validation.
150149
await ParameterEmbeddingHelper.SubstituteEmbedParametersAsync(
151150
parameters,
152-
entity.Source.Parameters,
151+
_runtimeConfigProvider.GetConfig(),
152+
entityName,
153153
_embeddingService,
154154
_httpContextAccessor.HttpContext?.RequestAborted ?? CancellationToken.None);
155155

@@ -211,14 +211,14 @@ await ParameterEmbeddingHelper.SubstituteEmbedParametersAsync(
211211
/// </summary>
212212
public async Task<IActionResult> ExecuteAsync(StoredProcedureRequestContext context, string dataSourceName)
213213
{
214-
// Phase 3: substitute embed:true parameters with embedding vectors
215-
// before SqlExecuteStructure reads them. The helper replaces text values
216-
// (e.g., "wireless headphones") with vector JSON strings (e.g., "[0.012,...]").
217-
// Helper handles the case where embed params exist but service is null (throws 503).
218-
Entity entity = _runtimeConfigProvider.GetConfig().Entities[context.EntityName];
214+
// Phase 3: substitute auto-embed:true parameters with embedding vectors (REST query path).
215+
// The helper replaces text values (e.g., "wireless headphones") with vector JSON strings
216+
// (e.g., "[0.012,...]") before SqlExecuteStructure reads them. Helper handles entity
217+
// lookup, null-service detection, and per-param validation.
219218
await ParameterEmbeddingHelper.SubstituteEmbedParametersAsync(
220219
context.ResolvedParameters,
221-
entity.Source.Parameters,
220+
_runtimeConfigProvider.GetConfig(),
221+
context.EntityName,
222222
_embeddingService,
223223
_httpContextAccessor.HttpContext?.RequestAborted ?? CancellationToken.None);
224224

src/Core/Services/Embeddings/ParameterEmbeddingHelper.cs

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,37 @@ namespace Azure.DataApiBuilder.Core.Services.Embeddings;
2222
/// </summary>
2323
public static class ParameterEmbeddingHelper
2424
{
25+
/// <summary>
26+
/// Convenience overload that resolves the entity's <see cref="ParameterMetadata"/> from
27+
/// the runtime config by entity name, then delegates to the parameter-list overload.
28+
///
29+
/// All three engine call sites (SqlQueryEngine GraphQL path, SqlQueryEngine REST path,
30+
/// SqlMutationEngine REST path) follow the same lookup-then-substitute pattern; this
31+
/// overload centralizes it so the engines don't each carry the boilerplate.
32+
/// </summary>
33+
/// <param name="resolvedParams">
34+
/// The parameter dictionary from the request. Modified in-place: text values for
35+
/// auto-embed params are replaced with vector JSON strings.
36+
/// </param>
37+
/// <param name="runtimeConfig">The active runtime config (resolved via the provider at the call site).</param>
38+
/// <param name="entityName">Name of the stored-procedure entity whose parameters may need embedding.</param>
39+
/// <param name="embeddingService">The embedding service to call for text → vector conversion.</param>
40+
/// <param name="cancellationToken">Cancellation token from the HTTP request.</param>
41+
public static Task SubstituteEmbedParametersAsync(
42+
IDictionary<string, object?> resolvedParams,
43+
RuntimeConfig runtimeConfig,
44+
string entityName,
45+
IEmbeddingService? embeddingService,
46+
CancellationToken cancellationToken)
47+
{
48+
Entity entity = runtimeConfig.Entities[entityName];
49+
return SubstituteEmbedParametersAsync(
50+
resolvedParams,
51+
entity.Source.Parameters,
52+
embeddingService,
53+
cancellationToken);
54+
}
55+
2556
/// <summary>
2657
/// For each parameter marked auto-embed:true in config, replaces the text value in
2758
/// resolvedParams with a serialized vector string.
@@ -162,12 +193,17 @@ public static async Task SubstituteEmbedParametersAsync(
162193

163194
if (!batchResult.Success || batchResult.Embeddings is null)
164195
{
165-
// Batch failure: we lose per-param error specificity here, but the
166-
// batch result's ErrorMessage typically explains the underlying issue.
167-
// Naming all involved params helps the user identify the request context.
196+
// Batch failure: include the provider's ErrorMessage when available so the caller
197+
// sees the actual reason (e.g., quota exhausted, model not found, authentication
198+
// failed) rather than only the generic "Failed to generate embeddings" line.
199+
// Per-param specificity is lost at the batch level, so naming all involved params
200+
// helps identify the request context.
168201
string paramNames = string.Join(", ", embedRequests.Select(r => $"'{r.paramName}'"));
202+
string providerDetail = string.IsNullOrWhiteSpace(batchResult.ErrorMessage)
203+
? string.Empty
204+
: $" Provider error: {batchResult.ErrorMessage}";
169205
throw new DataApiBuilderException(
170-
message: $"Failed to generate embeddings for parameter(s) {paramNames}.",
206+
message: $"Failed to generate embeddings for parameter(s) {paramNames}.{providerDetail}",
171207
statusCode: HttpStatusCode.InternalServerError,
172208
subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError);
173209
}

0 commit comments

Comments
 (0)