Skip to content

Commit c9498b4

Browse files
fix: AI, lifecycle improvements, etc (#990)
## Description Fix the AI issues. Need to consider a different store as semantic kernel and postgres are a bit behind.... Fixes #886 ### Ensure that your pull request has followed all the steps below: - [ ] Code compilation - [ ] Created tests which fail without the change (if possible) - [ ] All tests passing - [ ] Extended the README / documentation, if necessary
1 parent d2a7126 commit c9498b4

16 files changed

Lines changed: 369 additions & 145 deletions

.github/workflows/Build-Test-And-Deploy.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ jobs:
166166
az containerapp update --name $CONTAINER_APP_NAME --resource-group $RESOURCEGROUP --replace-env-vars Authentication__github__clientId=secretref:github-clientid Authentication__github__clientSecret=secretref:github-clientsecret \
167167
Authentication__microsoft__clientId=secretref:msft-clientid Authentication__microsoft__clientSecret=secretref:msft-clientsecret AuthMessageSender__ApiKey=secretref:emailsender-apikey AuthMessageSender__SecretKey=secretref:emailsender-secret \
168168
AuthMessageSender__SendFromName=secretref:emailsender-name AuthMessageSender__SendFromEmail=secretref:emailsender-email ConnectionStrings__EssentialCSharpWebContextConnection=secretref:connectionstring ASPNETCORE_ENVIRONMENT=Staging \
169-
AZURE_CLIENT_ID=$AZURECLIENTID HCaptcha__SiteKey=secretref:captcha-sitekey HCaptcha__SecretKey=secretref:captcha-secretkey ApplicationInsights__ConnectionString=secretref:appinsights-connectionstring \
169+
AZURE_CLIENT_ID=$AZURECLIENTID HCaptcha__SiteKey=secretref:captcha-sitekey HCaptcha__SecretKey=secretref:captcha-secretkey APPLICATIONINSIGHTS_CONNECTION_STRING=secretref:appinsights-connectionstring \
170170
AIOptions__Endpoint=secretref:ai-endpoint AIOptions__ApiKey=secretref:ai-apikey AIOptions__VectorGenerationDeploymentName=secretref:ai-vectordeployment AIOptions__ChatDeploymentName=secretref:ai-chatdeployment \
171171
AIOptions__SystemPrompt=secretref:ai-systemprompt ConnectionStrings__PostgresVectorStore=secretref:postgres-vectorstore-connectionstring \
172172
TryDotNet__Origin=$TRYDOTNET_ORIGIN
@@ -263,7 +263,7 @@ jobs:
263263
az containerapp update --name $CONTAINER_APP_NAME --resource-group $RESOURCEGROUP --replace-env-vars Authentication__github__clientId=secretref:github-clientid Authentication__github__clientSecret=secretref:github-clientsecret \
264264
Authentication__microsoft__clientId=secretref:msft-clientid Authentication__microsoft__clientSecret=secretref:msft-clientsecret AuthMessageSender__ApiKey=secretref:emailsender-apikey AuthMessageSender__SecretKey=secretref:emailsender-secret \
265265
AuthMessageSender__SendFromName=secretref:emailsender-name AuthMessageSender__SendFromEmail=secretref:emailsender-email ConnectionStrings__EssentialCSharpWebContextConnection=secretref:connectionstring ASPNETCORE_ENVIRONMENT=Production \
266-
AZURE_CLIENT_ID=$AZURECLIENTID HCaptcha__SiteKey=secretref:captcha-sitekey HCaptcha__SecretKey=secretref:captcha-secretkey ApplicationInsights__ConnectionString=secretref:appinsights-connectionstring \
266+
AZURE_CLIENT_ID=$AZURECLIENTID HCaptcha__SiteKey=secretref:captcha-sitekey HCaptcha__SecretKey=secretref:captcha-secretkey APPLICATIONINSIGHTS_CONNECTION_STRING=secretref:appinsights-connectionstring \
267267
AIOptions__Endpoint=secretref:ai-endpoint AIOptions__ApiKey=secretref:ai-apikey AIOptions__VectorGenerationDeploymentName=secretref:ai-vectordeployment AIOptions__ChatDeploymentName=secretref:ai-chatdeployment \
268268
AIOptions__SystemPrompt=secretref:ai-systemprompt ConnectionStrings__PostgresVectorStore=secretref:postgres-vectorstore-connectionstring \
269269
TryDotNet__Origin=$TRYDOTNET_ORIGIN

Directory.Packages.props

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
<PackageVersion Include="Azure.Extensions.AspNetCore.Configuration.Secrets" Version="1.5.0" />
2424
<PackageVersion Include="Azure.Identity" Version="1.21.0" />
2525
<PackageVersion Include="Azure.Monitor.OpenTelemetry.AspNetCore" Version="1.4.0" />
26-
<PackageVersion Include="Microsoft.ApplicationInsights.Profiler.AspNetCore" Version="3.0.2" />
2726
<PackageVersion Include="TUnit" Version="1.33.0" />
2827
<PackageVersion Include="EssentialCSharp.Shared.Models" Version="$(ToolingPackagesVersion)" />
2928
<PackageVersion Include="HtmlAgilityPack" Version="1.12.4" />
@@ -39,9 +38,13 @@
3938
<PackageVersion Include="Microsoft.EntityFrameworkCore.Sqlite" Version="10.0.5" />
4039
<PackageVersion Include="Microsoft.EntityFrameworkCore.SqlServer" Version="10.0.5" />
4140
<PackageVersion Include="Microsoft.EntityFrameworkCore.Tools" Version="10.0.3" />
41+
<PackageVersion Include="Microsoft.Extensions.Http.Resilience" Version="10.2.0" />
4242
<PackageVersion Include="Microsoft.SemanticKernel" Version="$(SemanticKernelVersion)" />
4343
<PackageVersion Include="Microsoft.SemanticKernel.Connectors.PgVector" Version="$(SemanticKernelVersion)-preview" />
44-
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="10.0.201" />
44+
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="10.0.202" />
45+
<!-- Pin to patched versions; NuGet.Protocol 6.12.5+ fixes GHSA-g4vj-cjjj-v7hg (pulled in by Microsoft.VisualStudio.Web.CodeGeneration.Design) -->
46+
<PackageVersion Include="NuGet.Packaging" Version="6.12.5" />
47+
<PackageVersion Include="NuGet.Protocol" Version="6.12.5" />
4548
<PackageVersion Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.23.0" />
4649
<PackageVersion Include="Microsoft.VisualStudio.Web.CodeGeneration.Design" Version="10.0.2" />
4750
<PackageVersion Include="ModelContextProtocol" Version="0.3.0-preview.4" />
@@ -51,6 +54,12 @@
5154
<PackageVersion Include="System.CommandLine" Version="2.0.5" />
5255
<PackageVersion Include="Newtonsoft.Json" Version="13.0.4" />
5356
<PackageVersion Include="Octokit" Version="14.0.0" />
57+
<PackageVersion Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" Version="1.15.2" />
58+
<PackageVersion Include="OpenTelemetry.Extensions.Hosting" Version="1.15.2" />
59+
<PackageVersion Include="OpenTelemetry.Instrumentation.AspNetCore" Version="1.15.1" />
60+
<PackageVersion Include="OpenTelemetry.Instrumentation.Http" Version="1.15.0" />
61+
<PackageVersion Include="OpenTelemetry.Instrumentation.Runtime" Version="1.15.0" />
62+
<PackageVersion Include="OpenTelemetry.Instrumentation.SqlClient" Version="1.15.1" />
5463
<PackageVersion Include="DotnetSitemapGenerator" Version="2.0.0" />
5564
</ItemGroup>
5665
</Project>

EssentialCSharp.Chat.Shared/Extensions/ServiceCollectionExtensions.cs

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,10 @@ public static IServiceCollection AddAzureOpenAIServices(
101101

102102
/// <summary>
103103
/// Adds PostgreSQL vector store with managed identity authentication support.
104-
/// NOTE: Token is obtained once at startup and will expire after ~1 hour.
105-
/// For long-running applications, consider implementing token refresh logic.
104+
/// Uses per-connection token refresh via <c>UsePasswordProvider</c>, which calls
105+
/// <see cref="TokenCredential.GetTokenAsync"/> on every new physical connection.
106+
/// <see cref="DefaultAzureCredential"/> caches tokens internally and auto-refreshes
107+
/// ~5 minutes before expiry, so this does not add Azure AD overhead.
106108
/// </summary>
107109
/// <param name="services">The service collection to add services to</param>
108110
/// <param name="connectionString">The PostgreSQL connection string (without password)</param>
@@ -115,36 +117,51 @@ private static IServiceCollection AddPostgresVectorStoreWithManagedIdentity(
115117
{
116118
credential ??= new DefaultAzureCredential();
117119

118-
// Parse the connection string to extract host, database, and username
119-
var builder = new NpgsqlConnectionStringBuilder(connectionString);
120-
121-
// Check if this is an Azure PostgreSQL connection (contains .postgres.database.azure.com)
122-
bool isAzurePostgres = builder.Host?.Contains(".postgres.database.azure.com", StringComparison.OrdinalIgnoreCase) ?? false;
123-
124-
if (isAzurePostgres && string.IsNullOrEmpty(builder.Password))
125-
{
126-
// Get access token for Azure PostgreSQL using managed identity
127-
var tokenRequestContext = new TokenRequestContext(_PostgresScopes);
128-
var accessToken = credential.GetToken(tokenRequestContext, default);
129-
130-
// Set the password to the access token
131-
builder.Password = accessToken.Token;
132-
133-
// Ensure SSL is enabled for Azure
134-
if (builder.SslMode == SslMode.Disable)
135-
{
136-
builder.SslMode = SslMode.Require;
137-
}
138-
139-
connectionString = builder.ToString();
140-
}
141-
142120
// Register NpgsqlDataSource with UseVector() enabled - this is critical for pgvector support
143121
services.AddSingleton<NpgsqlDataSource>(sp =>
144122
{
123+
var connBuilder = new NpgsqlConnectionStringBuilder(connectionString);
124+
bool isAzurePostgres = connBuilder.Host?.Contains(".postgres.database.azure.com",
125+
StringComparison.OrdinalIgnoreCase) ?? false;
126+
145127
var dataSourceBuilder = new NpgsqlDataSourceBuilder(connectionString);
146128
// IMPORTANT: UseVector() must be called to enable pgvector support
147129
dataSourceBuilder.UseVector();
130+
131+
if (isAzurePostgres && string.IsNullOrEmpty(connBuilder.Password))
132+
{
133+
// Ensure SSL is enabled for Azure PostgreSQL
134+
if (dataSourceBuilder.ConnectionStringBuilder.SslMode < SslMode.Require)
135+
{
136+
dataSourceBuilder.ConnectionStringBuilder.SslMode = SslMode.Require;
137+
}
138+
139+
var tokenRequestContext = new TokenRequestContext(_PostgresScopes);
140+
141+
// UsePasswordProvider is called for every new physical connection.
142+
// DefaultAzureCredential caches tokens internally and auto-refreshes ~5 min before
143+
// expiry — no extra Azure AD load. This is the approach recommended by the Npgsql
144+
// docs for cloud providers that implement their own caching (Azure MI does).
145+
// UsePeriodicPasswordProvider is only for token sources without built-in caching.
146+
// See: https://www.npgsql.org/doc/security.html
147+
// See: https://github.com/npgsql/npgsql/issues/5186
148+
//
149+
// Note: The username is expected to be set in the connection string already
150+
// (Aspire sets it during deployment for Azure PostgreSQL Flexible Server).
151+
// If a standalone username-extraction fallback is ever needed, use the
152+
// Microsoft.Azure.PostgreSQL.Auth package (UseEntraAuthentication extension)
153+
// once it ships on NuGet.
154+
dataSourceBuilder.UsePasswordProvider(
155+
passwordProvider: _ => credential.GetToken(tokenRequestContext, default).Token,
156+
passwordProviderAsync: async (_, ct) =>
157+
(await credential.GetTokenAsync(tokenRequestContext, ct)).Token);
158+
159+
// Recycle pooled connections after 50 min, well before the 60-min JWT token TTL.
160+
// Combined with UsePasswordProvider (called on every new physical connection),
161+
// this ensures no pooled connection ever holds an expired token.
162+
dataSourceBuilder.ConnectionStringBuilder.ConnectionLifetime = 3000;
163+
}
164+
148165
return dataSourceBuilder.Build();
149166
});
150167

EssentialCSharp.Chat.Shared/Services/AIChatService.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,14 @@ private async Task<string> EnrichPromptWithContext(string prompt, bool enableCon
118118
return prompt;
119119
}
120120

121-
var searchResults = await _SearchService.ExecuteVectorSearch(prompt);
121+
var searchResults = await _SearchService.ExecuteVectorSearch(prompt, cancellationToken: cancellationToken);
122122
var contextualInfo = new System.Text.StringBuilder();
123123

124124
contextualInfo.AppendLine("## Contextual Information");
125125
contextualInfo.AppendLine("The following information might be relevant to your question:");
126126
contextualInfo.AppendLine();
127127

128-
await foreach (var result in searchResults)
128+
foreach (var result in searchResults)
129129
{
130130
contextualInfo.AppendLine(System.Globalization.CultureInfo.InvariantCulture, $"**From: {result.Record.Heading}**");
131131
contextualInfo.AppendLine(result.Record.ChunkText);
Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,54 @@
1-
using EssentialCSharp.Chat.Common.Models;
1+
using System.Diagnostics;
2+
using EssentialCSharp.Chat.Common.Models;
3+
using Microsoft.Extensions.Logging;
24
using Microsoft.Extensions.VectorData;
5+
using Npgsql;
36

47
namespace EssentialCSharp.Chat.Common.Services;
58

6-
public class AISearchService(VectorStore vectorStore, EmbeddingService embeddingService)
9+
public class AISearchService(
10+
VectorStore vectorStore,
11+
EmbeddingService embeddingService,
12+
ILogger<AISearchService> logger)
713
{
814
// TODO: Implement Hybrid Search functionality, may need to switch db providers to support full text search?
915

10-
public async Task<IAsyncEnumerable<VectorSearchResult<BookContentChunk>>> ExecuteVectorSearch(string query, string? collectionName = null)
16+
public async Task<IReadOnlyList<VectorSearchResult<BookContentChunk>>> ExecuteVectorSearch(
17+
string query, string? collectionName = null, CancellationToken cancellationToken = default)
1118
{
1219
collectionName ??= EmbeddingService.CollectionName;
1320

1421
VectorStoreCollection<string, BookContentChunk> collection = vectorStore.GetCollection<string, BookContentChunk>(collectionName);
1522

16-
ReadOnlyMemory<float> searchVector = await embeddingService.GenerateEmbeddingAsync(query);
23+
ReadOnlyMemory<float> searchVector = await embeddingService.GenerateEmbeddingAsync(query, cancellationToken);
1724

1825
var vectorSearchOptions = new VectorSearchOptions<BookContentChunk>
1926
{
2027
VectorProperty = x => x.TextEmbedding,
2128
};
2229

23-
var searchResults = collection.SearchAsync(searchVector, options: vectorSearchOptions, top: 3);
24-
25-
return searchResults;
30+
for (int attempt = 0; attempt <= 1; attempt++)
31+
{
32+
try
33+
{
34+
var results = new List<VectorSearchResult<BookContentChunk>>();
35+
await foreach (var result in collection.SearchAsync(searchVector, options: vectorSearchOptions, top: 3, cancellationToken: cancellationToken))
36+
{
37+
results.Add(result);
38+
}
39+
return results;
40+
}
41+
catch (PostgresException ex) when (ex.SqlState == "28000" && attempt == 0)
42+
{
43+
// The pooled connection held an expired Entra ID token. Npgsql automatically
44+
// removes the broken connection from the pool on error — no manual pool clearing
45+
// needed (clearing would evict all healthy connections, hurting concurrent users).
46+
// The retry opens a fresh physical connection, which calls UsePasswordProvider
47+
// and gets a new token from DefaultAzureCredential.
48+
logger.LogWarning(ex, "Entra ID token expired on pooled connection (SqlState 28000); retrying once.");
49+
}
50+
}
51+
52+
throw new UnreachableException("Retry loop exited without returning or throwing.");
2653
}
2754
}
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
using EssentialCSharp.Chat.Common.Models;
2+
using EssentialCSharp.Chat.Common.Services;
3+
using Microsoft.Extensions.AI;
4+
using Microsoft.Extensions.Logging;
5+
using Microsoft.Extensions.VectorData;
6+
using Moq;
7+
using Moq.Language.Flow;
8+
using Npgsql;
9+
10+
namespace EssentialCSharp.Chat.Tests;
11+
12+
public class AISearchServiceTests
13+
{
14+
private static readonly BookContentChunk _TestChunk = new() { Id = "test-1", ChunkText = "test" };
15+
16+
private static (AISearchService svc, Mock<VectorStoreCollection<string, BookContentChunk>> collectionMock)
17+
CreateService()
18+
{
19+
var collectionMock = new Mock<VectorStoreCollection<string, BookContentChunk>>();
20+
21+
var vectorStoreMock = new Mock<VectorStore>();
22+
vectorStoreMock
23+
.Setup(vs => vs.GetCollection<string, BookContentChunk>(It.IsAny<string>(), It.IsAny<VectorStoreCollectionDefinition?>()))
24+
.Returns(collectionMock.Object);
25+
26+
// IEmbeddingGenerator<string, Embedding<float>>.GenerateAsync is the batch interface method
27+
// called internally by the single-value extension used in EmbeddingService.GenerateEmbeddingAsync.
28+
var embGenMock = new Mock<IEmbeddingGenerator<string, Embedding<float>>>();
29+
embGenMock
30+
.Setup(g => g.GenerateAsync(
31+
It.IsAny<IEnumerable<string>>(),
32+
It.IsAny<EmbeddingGenerationOptions?>(),
33+
It.IsAny<CancellationToken>()))
34+
.ReturnsAsync(new GeneratedEmbeddings<Embedding<float>>([new Embedding<float>(new float[1536])]));
35+
36+
var embeddingService = new EmbeddingService(vectorStoreMock.Object, embGenMock.Object);
37+
var loggerMock = new Mock<ILogger<AISearchService>>();
38+
39+
return (new AISearchService(vectorStoreMock.Object, embeddingService, loggerMock.Object), collectionMock);
40+
}
41+
42+
private static async IAsyncEnumerable<VectorSearchResult<BookContentChunk>> OneResultStream()
43+
{
44+
yield return new VectorSearchResult<BookContentChunk>(_TestChunk, 0.9f);
45+
await Task.CompletedTask;
46+
}
47+
48+
private static ISetup<VectorStoreCollection<string, BookContentChunk>, IAsyncEnumerable<VectorSearchResult<BookContentChunk>>>
49+
SetupSearch(Mock<VectorStoreCollection<string, BookContentChunk>> mock) =>
50+
mock.Setup(c => c.SearchAsync(
51+
It.IsAny<ReadOnlyMemory<float>>(),
52+
It.IsAny<int>(),
53+
It.IsAny<VectorSearchOptions<BookContentChunk>?>(),
54+
It.IsAny<CancellationToken>()));
55+
56+
[Test]
57+
public async Task ExecuteVectorSearch_HappyPath_ReturnsResultsWithoutRetry()
58+
{
59+
var (svc, collectionMock) = CreateService();
60+
int callCount = 0;
61+
62+
SetupSearch(collectionMock).Returns(() => { callCount++; return OneResultStream(); });
63+
64+
var results = await svc.ExecuteVectorSearch("test query");
65+
66+
await Assert.That(results.Count).IsEqualTo(1);
67+
await Assert.That(callCount).IsEqualTo(1);
68+
}
69+
70+
[Test]
71+
public async Task ExecuteVectorSearch_RetriesOnce_WhenFirstAttemptThrows28000()
72+
{
73+
var (svc, collectionMock) = CreateService();
74+
int callCount = 0;
75+
76+
SetupSearch(collectionMock).Returns(() =>
77+
{
78+
callCount++;
79+
if (callCount == 1)
80+
throw new PostgresException("auth token expired", "FATAL", "FATAL", "28000");
81+
return OneResultStream();
82+
});
83+
84+
var results = await svc.ExecuteVectorSearch("test query");
85+
86+
await Assert.That(results.Count).IsEqualTo(1);
87+
await Assert.That(callCount).IsEqualTo(2);
88+
}
89+
90+
[Test]
91+
public async Task ExecuteVectorSearch_DoesNotRetry_WhenSqlStateIsNot28000()
92+
{
93+
var (svc, collectionMock) = CreateService();
94+
95+
SetupSearch(collectionMock).Returns(() => throw new PostgresException("table not found", "ERROR", "ERROR", "42P01"));
96+
97+
await Assert.ThrowsAsync<PostgresException>(() => svc.ExecuteVectorSearch("test query"));
98+
}
99+
100+
[Test]
101+
public async Task ExecuteVectorSearch_PropagatesException_WhenBothAttemptsFail28000()
102+
{
103+
var (svc, collectionMock) = CreateService();
104+
105+
SetupSearch(collectionMock).Returns(() => throw new PostgresException("auth failed", "FATAL", "FATAL", "28000"));
106+
107+
await Assert.ThrowsAsync<PostgresException>(() => svc.ExecuteVectorSearch("test query"));
108+
}
109+
}

EssentialCSharp.Web.Tests/FunctionalTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ public class FunctionalTests(WebApplicationFactory factory)
1111
[Arguments("/hello-world")]
1212
[Arguments("/hello-world#hello-world")]
1313
[Arguments("/guidelines")]
14-
[Arguments("/healthz")]
14+
[Arguments("/health")]
15+
[Arguments("/alive")]
1516
public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl)
1617
{
1718
HttpClient client = factory.CreateClient();

0 commit comments

Comments
 (0)