diff --git a/src/CrestApps.Core.Docs/docs/changelog/2.0.0.md b/src/CrestApps.Core.Docs/docs/changelog/2.0.0.md index 2b81ecbd..6ecd79ee 100644 --- a/src/CrestApps.Core.Docs/docs/changelog/2.0.0.md +++ b/src/CrestApps.Core.Docs/docs/changelog/2.0.0.md @@ -864,3 +864,19 @@ microphone, desk speakers): docs version from patch tags only when that major/minor docs version does not already exist, and skips prerelease tags successfully after logging why no docs version PR was needed +- **Fixed: one of two AI provider connections whose names differ only by case disappeared without a + trace.** A connection's identifier hashes its lowercased client and connection name, so `Shared-Azure` + declared under `CrestApps:AI:Connections` and `shared-azure` declared under + `CrestApps:AI:Providers:Azure:Connections` resolved to the same identifier, and the second silently + replaced the first — taking its endpoint and its credentials with it. Every deployment naming that + connection then reached whichever resource happened to be read last, which surfaces as an unexplained + `DeploymentNotFound` when the model is deployed only on the other one. The first definition now wins, + matching how configured deployments are read, and the dropped entry is logged as a warning that names + the entry it collided with. A site that has such a pair today will switch to the earlier definition; + the warning names both, so the duplicate can be renamed. +- **A failed Azure OpenAI completion now names the deployment and endpoint that served it.** The + provider's own exception names neither, so a misrouted request — a deployment bound to the wrong + connection, or a model name that does not exist on the resource that connection points at — read as a + bare HTTP error with nothing to say where the request went. Both the streaming and non-streaming paths + now log the deployment name, the model name that forms the request URL, the connection name, and the + endpoint host. diff --git a/src/Primitives/CrestApps.Core.AI.OpenAI.Azure/Services/AzureOpenAICompletionClient.cs b/src/Primitives/CrestApps.Core.AI.OpenAI.Azure/Services/AzureOpenAICompletionClient.cs index 3ffc0543..fc9df0eb 100644 --- a/src/Primitives/CrestApps.Core.AI.OpenAI.Azure/Services/AzureOpenAICompletionClient.cs +++ b/src/Primitives/CrestApps.Core.AI.OpenAI.Azure/Services/AzureOpenAICompletionClient.cs @@ -11,6 +11,7 @@ using CrestApps.Core.AI.OpenAI.Azure.Models; using CrestApps.Core.AI.Services; using CrestApps.Core.Extensions; +using CrestApps.Core.Infrastructure; using CrestApps.Core.Templates.Services; using Microsoft.AspNetCore.DataProtection; using Microsoft.Extensions.DependencyInjection; @@ -177,7 +178,7 @@ public string ClientName } catch (Exception ex) { - _logger.LogError(ex, "Unable to get chat completion result from Azure OpenAI."); + LogCompletionFailure(ex, deployment, connectionProperties); } return null; @@ -251,8 +252,36 @@ public string ClientName while (iterations <= _defaultOptions.MaximumIterationsPerRequest) { var hasToolCalls = false; - await foreach (var update in chatClient.CompleteChatStreamingAsync(prompts, chatOptions, cancellationToken)) + + // Enumerated by hand rather than with `await foreach` so the call into the provider sits in a + // try block: a yield cannot appear inside one, and without it a failed request leaves no record + // of which deployment and endpoint produced it. + await using var updates = chatClient + .CompleteChatStreamingAsync(prompts, chatOptions, cancellationToken) + .GetAsyncEnumerator(cancellationToken); + + while (true) { + try + { + if (!await updates.MoveNextAsync()) + { + break; + } + } + catch (OperationCanceledException) + { + throw; + } + catch (Exception ex) + { + LogCompletionFailure(ex, deployment, connection); + + throw; + } + + var update = updates.Current; + // Accumulate tool call updates as they arrive. foreach (var toolCallUpdate in update.ToolCallUpdates) { @@ -474,6 +503,30 @@ private AzureOpenAIClient GetChatClient(AIProviderConnectionEntry connection) return AzureOpenAIClientFactory.Create(connection, _loggerFactory, _azureClientOptions); } + /// + /// Logs a failed completion together with the deployment and endpoint that served it. + /// + /// + /// The provider's own exception names neither. A misrouted request -- a deployment bound to the wrong + /// connection, or a model name that does not exist on the resource that connection points at -- reads + /// as a bare DeploymentNotFound without them, which says nothing about where the request went. + /// The model name is the one that forms the request URL, so it is reported even when it matches the + /// deployment name. + /// + /// The exception the request failed with. + /// The deployment that served the request. + /// The connection that served the request. + private void LogCompletionFailure(Exception exception, AIDeployment deployment, AIProviderConnectionEntry connection) + { + _logger.LogError( + exception, + "Unable to get chat completion result from Azure OpenAI using deployment '{DeploymentName}' (model '{ModelName}') on connection '{ConnectionName}' at '{Endpoint}'.", + deployment.Name, + deployment.ModelName, + deployment.ConnectionName, + connection.GetEndpoint(throwException: false)?.Host); + } + private static async ValueTask> ConfigureOptionsAsync(ChatCompletionOptions chatOptions, AICompletionContext context, List prompts) { var optionsContext = new AzureOpenAIChatOptionsContext(chatOptions, context, prompts); diff --git a/src/Primitives/CrestApps.Core.AI/Services/ConfigurationAIProviderConnectionSource.cs b/src/Primitives/CrestApps.Core.AI/Services/ConfigurationAIProviderConnectionSource.cs index 41e15eed..bc6340e1 100644 --- a/src/Primitives/CrestApps.Core.AI/Services/ConfigurationAIProviderConnectionSource.cs +++ b/src/Primitives/CrestApps.Core.AI/Services/ConfigurationAIProviderConnectionSource.cs @@ -194,6 +194,21 @@ private void AddConfiguredConnection( return; } + // The identifier hashes the lowercased provider and connection name, so two entries whose names + // differ only by case land on the same one. The first definition wins, matching how configured + // deployments are read; without this the later entry replaced the earlier one in silence and an + // entire connection -- its endpoint and its credentials -- disappeared with no diagnostic. + if (connections.TryGetValue(connection.ItemId, out var existingConnection)) + { + _logger.LogWarning( + "Skipping AI connection '{ConnectionName}' from '{SourceDescription}' because '{ExistingConnectionName}' resolves to the same identifier. Connection names are case-insensitive; rename one of them so both are read.", + connection.Name, + sourceDescription, + existingConnection.Name); + + return; + } + if (_logger.IsEnabled(LogLevel.Debug)) { _logger.LogDebug("Adding AI connection '{ConnectionName}' (ClientName: '{ClientName}', ItemId: '{ItemId}') from '{SourceDescription}'.", diff --git a/tests/CrestApps.Core.Tests/Framework/Mvc/AIProviderConnectionOptionsTests.cs b/tests/CrestApps.Core.Tests/Framework/Mvc/AIProviderConnectionOptionsTests.cs index 568681a3..2c39d4f4 100644 --- a/tests/CrestApps.Core.Tests/Framework/Mvc/AIProviderConnectionOptionsTests.cs +++ b/tests/CrestApps.Core.Tests/Framework/Mvc/AIProviderConnectionOptionsTests.cs @@ -609,6 +609,30 @@ public async Task ConfigurationAIProviderConnectionStore_ShouldSkipConfiguredCon Assert.Equal("ui-connection", connections.Single().ItemId); } + [Fact] + public async Task ConfigurationAIProviderConnectionStore_WhenTwoConfiguredNamesDifferOnlyByCase_ShouldKeepTheFirstOne() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["CrestApps:AI:Connections:0:Name"] = "Shared-Azure", + ["CrestApps:AI:Connections:0:ClientName"] = "Azure", + ["CrestApps:AI:Connections:0:Endpoint"] = "https://first.openai.azure.com/", + ["CrestApps:AI:Providers:Azure:Connections:shared-azure:Endpoint"] = "https://second.openai.azure.com/", + }) + .Build(); + + var store = CreateConnectionStore(configuration); + + var connections = await store.GetAllAsync(TestContext.Current.CancellationToken); + + // Both entries hash to the same identifier, so only one survives. The later one used to replace + // the earlier in silence, taking its endpoint and its credentials with it. + var connection = Assert.Single(connections); + Assert.Equal("Shared-Azure", connection.Name); + Assert.Equal("https://first.openai.azure.com/", connection.Properties["Endpoint"]?.ToString()); + } + private static DefaultAIDeploymentCapabilityService CreateCapabilityService() { return new DefaultAIDeploymentCapabilityService(Options.Create(new AIDeploymentCapabilityOptions()), Mock.Of());