Fix PR review comments: health endpoint test, AppInsights config, FileProvider disposal, retry tests

- Fix CI failure: update FunctionalTests.cs to use /health and /alive instead of removed /healthz
- Fix AppInsights silent failure: check APPLICATIONINSIGHTS_CONNECTION_STRING,
  ApplicationInsights:ConnectionString, and GetConnectionString('ApplicationInsights') fallback;
  pass resolved string explicitly to UseAzureMonitor(options => ...) since the no-arg overload
  only auto-reads the flat env var. Deployment workflow sets ApplicationInsights__ConnectionString
  which maps to the hierarchical key, not the flat env var.
- Fix PhysicalFileProvider IDisposable leak: ListingSourceCodeService implements IDisposable with
  _OwnsFileProvider tracking; only disposes the provider it creates, not the framework-managed
  ContentRootFileProvider; GC.SuppressFinalize included per CA1816.
- Add AISearchService retry unit tests: 4 tests covering happy path (no retry), 28000 retry
  success, non-28000 pass-through, and both-attempts-fail propagation. PostgresException has a
  public constructor so no reflection or seams are needed.

Author: BenjaminMichaelis

EssentialCSharp.Chat.Tests/AISearchServiceTests.cs

@@ -0,0 +1,124 @@
+using EssentialCSharp.Chat.Common.Models;
+using EssentialCSharp.Chat.Common.Services;
+using Microsoft.Extensions.AI;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.VectorData;
+using Moq;
+using Npgsql;
+
+namespace EssentialCSharp.Chat.Tests;
+
+public class AISearchServiceTests
+{
+    private static readonly BookContentChunk _TestChunk = new() { Id = "test-1", ChunkText = "test" };
+
+    private static (AISearchService svc, Mock<VectorStoreCollection<string, BookContentChunk>> collectionMock)
+        CreateService()
+    {
+        var collectionMock = new Mock<VectorStoreCollection<string, BookContentChunk>>();
+
+        var vectorStoreMock = new Mock<VectorStore>();
+        vectorStoreMock
+            .Setup(vs => vs.GetCollection<string, BookContentChunk>(It.IsAny<string>(), It.IsAny<VectorStoreCollectionDefinition?>()))
+            .Returns(collectionMock.Object);
+
+        // IEmbeddingGenerator<string, Embedding<float>>.GenerateAsync is the batch interface method
+        // called internally by the single-value extension used in EmbeddingService.GenerateEmbeddingAsync.
+        var embGenMock = new Mock<IEmbeddingGenerator<string, Embedding<float>>>();
+        embGenMock
+            .Setup(g => g.GenerateAsync(
+                It.IsAny<IEnumerable<string>>(),
+                It.IsAny<EmbeddingGenerationOptions?>(),
+                It.IsAny<CancellationToken>()))
+            .ReturnsAsync(new GeneratedEmbeddings<Embedding<float>>([new Embedding<float>(new float[1536])]));
+
+        var embeddingService = new EmbeddingService(vectorStoreMock.Object, embGenMock.Object);
+        var loggerMock = new Mock<ILogger<AISearchService>>();
+
+        return (new AISearchService(vectorStoreMock.Object, embeddingService, loggerMock.Object), collectionMock);
+    }
+
+    private static async IAsyncEnumerable<VectorSearchResult<BookContentChunk>> OneResultStream()
+    {
+        yield return new VectorSearchResult<BookContentChunk>(_TestChunk, 0.9f);
+        await Task.CompletedTask;
+    }
+
+    [Test]
+    public async Task ExecuteVectorSearch_HappyPath_ReturnsResultsWithoutRetry()
+    {
+        var (svc, collectionMock) = CreateService();
+        int callCount = 0;
+
+        collectionMock
+            .Setup(c => c.SearchAsync(
+                It.IsAny<ReadOnlyMemory<float>>(),
+                It.IsAny<int>(),
+                It.IsAny<VectorSearchOptions<BookContentChunk>?>(),
+                It.IsAny<CancellationToken>()))
+            .Returns(() => { callCount++; return OneResultStream(); });
+
+        var results = await svc.ExecuteVectorSearch("test query");
+
+        await Assert.That(results.Count).IsEqualTo(1);
+        await Assert.That(callCount).IsEqualTo(1);
+    }
+
+    [Test]
+    public async Task ExecuteVectorSearch_RetriesOnce_WhenFirstAttemptThrows28000()
+    {
+        var (svc, collectionMock) = CreateService();
+        int callCount = 0;
+
+        collectionMock
+            .Setup(c => c.SearchAsync(
+                It.IsAny<ReadOnlyMemory<float>>(),
+                It.IsAny<int>(),
+                It.IsAny<VectorSearchOptions<BookContentChunk>?>(),
+                It.IsAny<CancellationToken>()))
+            .Returns(() =>
+            {
+                callCount++;
+                if (callCount == 1)
+                    throw new PostgresException("auth token expired", "FATAL", "FATAL", "28000");
+                return OneResultStream();
+            });
+
+        var results = await svc.ExecuteVectorSearch("test query");
+
+        await Assert.That(results.Count).IsEqualTo(1);
+        await Assert.That(callCount).IsEqualTo(2);
+    }
+
+    [Test]
+    public async Task ExecuteVectorSearch_DoesNotRetry_WhenSqlStateIsNot28000()
+    {
+        var (svc, collectionMock) = CreateService();
+
+        collectionMock
+            .Setup(c => c.SearchAsync(
+                It.IsAny<ReadOnlyMemory<float>>(),
+                It.IsAny<int>(),
+                It.IsAny<VectorSearchOptions<BookContentChunk>?>(),
+                It.IsAny<CancellationToken>()))
+            .Returns(() => throw new PostgresException("table not found", "ERROR", "ERROR", "42P01"));
+
+        await Assert.ThrowsAsync<PostgresException>(() => svc.ExecuteVectorSearch("test query"));
+    }
+
+    [Test]
+    public async Task ExecuteVectorSearch_PropagatesException_WhenBothAttemptsFail28000()
+    {
+        var (svc, collectionMock) = CreateService();
+
+        collectionMock
+            .Setup(c => c.SearchAsync(
+                It.IsAny<ReadOnlyMemory<float>>(),
+                It.IsAny<int>(),
+                It.IsAny<VectorSearchOptions<BookContentChunk>?>(),
+                It.IsAny<CancellationToken>()))
+            .Returns(() => throw new PostgresException("auth failed", "FATAL", "FATAL", "28000"));
+
+        await Assert.ThrowsAsync<PostgresException>(() => svc.ExecuteVectorSearch("test query"));
+    }
+}

EssentialCSharp.Web.Tests/FunctionalTests.cs

@@ -11,7 +11,8 @@ public class FunctionalTests(WebApplicationFactory factory)
     [Arguments("/hello-world")]
     [Arguments("/hello-world#hello-world")]
     [Arguments("/guidelines")]
-    [Arguments("/healthz")]
+    [Arguments("/health")]
+    [Arguments("/alive")]
     public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl)
     {
         HttpClient client = factory.CreateClient();

EssentialCSharp.Web/Program.cs

@@ -38,7 +38,15 @@ private static void Main(string[] args)
         //   Production:  Azure Monitor (Application Insights) via APPLICATIONINSIGHTS_CONNECTION_STRING
         //   Local/Aspire: OTLP to Aspire Dashboard via OTEL_EXPORTER_OTLP_ENDPOINT
         // Never both simultaneously — that would cause duplicate telemetry in App Insights.
-        bool useAzureMonitor = !string.IsNullOrEmpty(builder.Configuration["APPLICATIONINSIGHTS_CONNECTION_STRING"]);
+        // Resolve from all config locations:
+        //   1. Standard Azure Monitor flat env var (SDK's own default key)
+        //   2. Double-underscore (hierarchical) format set by the deployment workflow
+        //   3. GetConnectionString fallback
+        string? appInsightsConnectionString =
+            builder.Configuration["APPLICATIONINSIGHTS_CONNECTION_STRING"]
+            ?? builder.Configuration["ApplicationInsights:ConnectionString"]
+            ?? builder.Configuration.GetConnectionString("ApplicationInsights");
+        bool useAzureMonitor = !string.IsNullOrWhiteSpace(appInsightsConnectionString);
         bool useOtlp = !string.IsNullOrWhiteSpace(builder.Configuration["OTEL_EXPORTER_OTLP_ENDPOINT"]);
 
         builder.Logging.AddOpenTelemetry(logging =>
@@ -80,7 +88,7 @@ private static void Main(string[] args)
             });
 
         if (useAzureMonitor)
-            otel.UseAzureMonitor();
+            otel.UseAzureMonitor(options => options.ConnectionString = appInsightsConnectionString);
         else if (useOtlp)
             otel.UseOtlpExporter();
 

EssentialCSharp.Web/Services/ListingSourceCodeService.cs

@@ -5,9 +5,10 @@
 
 namespace EssentialCSharp.Web.Services;
 
-public partial class ListingSourceCodeService : IListingSourceCodeService
+public partial class ListingSourceCodeService : IListingSourceCodeService, IDisposable
 {
     private readonly IFileProvider _FileProvider;
+    private readonly bool _OwnsFileProvider;
     private readonly string _ChapterDirectoryPrefix;
     private readonly ILogger<ListingSourceCodeService> _Logger;
 
@@ -19,6 +20,7 @@ public ListingSourceCodeService(IWebHostEnvironment webHostEnvironment, ILogger<
         if (!string.IsNullOrEmpty(listingSourceCodePath) && Directory.Exists(listingSourceCodePath))
         {
             _FileProvider = new PhysicalFileProvider(listingSourceCodePath);
+            _OwnsFileProvider = true;
             _ChapterDirectoryPrefix = "src";
             _Logger.LogInformation("Using listing source code from: {Path}", listingSourceCodePath);
         }
@@ -29,6 +31,13 @@ public ListingSourceCodeService(IWebHostEnvironment webHostEnvironment, ILogger<
         }
     }
 
+    public void Dispose()
+    {
+        if (_OwnsFileProvider && _FileProvider is IDisposable disposable)
+            disposable.Dispose();
+        GC.SuppressFinalize(this);
+    }
+
     public async Task<ListingSourceCodeResponse?> GetListingAsync(int chapterNumber, int listingNumber)
     {
         string chapterDirectory = $"{_ChapterDirectoryPrefix}/Chapter{chapterNumber:D2}";