Merge pull request #53 from Encamina/@rliberoff/improvements_on_ephem…

…aral_memory_handler

Fixing DI lifecycles for Memory Managers and update related methods.

CHANGELOG.md

@@ -18,16 +18,18 @@ Previous classification is not required if changes are simple or all belong to t
 
 ## [8.1.2]
 
-### Braking Changes
+### Breaking Changes
 - Replace dependency with `IMemoryStore` for `IMemoryManager` in abstract class `MemoryStoreHandlerBase`. This affects internal types like the `EphemeralMemoryStoreHandler`.
 - Removed visibility modifiers in `IMemoryManager` interface.
+- Signature of `UpsertMemoryAsync` method has changed in `IMemoryManager` interface.
+- Signature of `BatchUpsertMemoriesAsync` method has changed in `IMemoryManager` interface.
+- Dependency with `Kernel` has been replace with dependency to `ITextEmbeddingGenerationService` in `MemoryManager` class. Also, added dependency with `ILogger`.
 
 ### Major change
 - Method `GetDocumentConnector` in `DocumentContentExtractorBase` is now `public` instead of `protected`.
 - New `MemoryManager` property of type `IMemoryManager` in `IMemoryStoreHandler` interface to get read-only access to the underlaying memory manager.
 - New `MemoryStore` property of type `IMemoryStore` in `IMemoryManager` interface to get read-only access to the underlaying memory store.
 - Removed unnecessary `Guards` when adding a Memory Manager and the Ephemeral Memory Store Handler. The exceptions will be thrown by the DI engine itself.
-- Modified dependency injection lifetime for `MemoryManager` and `EphemeralMemoryStoreHandler` services.
 
 ### Minor Changes
 - Properties `CollectionNamePostfix` and `CollectionNamePrefix` from `MemoryStoreHandlerBase` are now `virtual` instead of `abstract`.
@@ -65,7 +67,7 @@ Sadly, some features from `Semantic Kernel` that we might have been using, are m
 
  More information about these warnings is available here: https://github.com/microsoft/semantic-kernel/blob/main/dotnet/docs/EXPERIMENTS.md
 
-### Braking Changes
+### Breaking Changes
 
 - Replaced class `Completition` for `Completion` in `Encamina.Enmarcha.AI.OpenAI.Abstractions`. It was misspelled.
 - Class `SemanticKernelOptions` does not exists anymore. It has been replaced by `AzureOpenAIOptions` from `Encamina.Enmarcha.AI.OpenAI.Abstractions`. 

Directory.Build.props

@@ -17,7 +17,7 @@
 
   <PropertyGroup>
     <VersionPrefix>8.1.2</VersionPrefix>
-    <VersionSuffix>preview-6</VersionSuffix>
+    <VersionSuffix>preview-7</VersionSuffix>
   </PropertyGroup>
 
   <!--

src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

@@ -1,5 +1,6 @@
 // Ignore Spelling: Upsert
 
+using Microsoft.SemanticKernel;
 using Microsoft.SemanticKernel.Memory;
 
 namespace Encamina.Enmarcha.SemanticKernel.Abstractions;
@@ -23,7 +24,7 @@ public interface IMemoryManager
     /// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
     /// <param name="metadata">Metadata of the memory.</param>
     /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
-    Task UpsertMemoryAsync(string memoryId, string collectionName, IEnumerable<string> chunks, CancellationToken cancellationToken, IDictionary<string, string> metadata = null);
+    Task UpsertMemoryAsync(string memoryId, string collectionName, IEnumerable<string> chunks, IDictionary<string, string> metadata = null, Kernel kernel = null, CancellationToken cancellationToken = default);
 
     /// <summary>
     /// Deletes the memory content from a collection.
@@ -54,5 +55,5 @@ public interface IMemoryManager
     /// </param>
     /// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
     /// <returns>The unique identifiers for the memory records (not necessarily the same as the unique identifier of the memory content).</returns>
-    IAsyncEnumerable<string> BatchUpsertMemoriesAsync(string collectionName, IDictionary<string, MemoryContent> memoryContents, CancellationToken cancellationToken);
+    IAsyncEnumerable<string> BatchUpsertMemoriesAsync(string collectionName, IDictionary<string, MemoryContent> memoryContents, Kernel kernel = null, CancellationToken cancellationToken = default);
 }

src/Encamina.Enmarcha.SemanticKernel/Extensions/IServiceCollectionExtensions.cs

@@ -24,7 +24,7 @@ public static class IServiceCollectionExtensions
     /// <returns>The <see cref="IServiceCollection"/> so that additional calls can be chained.</returns>
     public static IServiceCollection AddMemoryManager(this IServiceCollection services)
     {
-        services.TryAddTransient<IMemoryManager, MemoryManager>();
+        services.TryAddSingleton<IMemoryManager, MemoryManager>();
 
         return services;
     }
@@ -47,7 +47,7 @@ public static IServiceCollection AddEphemeralMemoryStoreHandler(this IServiceCol
                 .ValidateDataAnnotations()
                 .ValidateOnStart();
 
-        services.TryAddTransient<IMemoryStoreHandler, EphemeralMemoryStoreHandler>();
+        services.TryAddSingleton<IMemoryStoreHandler, EphemeralMemoryStoreHandler>();
 
         return services;
     }

src/Encamina.Enmarcha.SemanticKernel/MemoryManager.cs

@@ -20,29 +20,30 @@ public class MemoryManager : IMemoryManager
 {
     private const string ChunkSize = @"chunkSize";
 
-    private readonly Kernel kernel;
-
     private readonly ILogger logger;
 
+    private readonly ITextEmbeddingGenerationService textEmbeddingGenerationService;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="MemoryManager"/> class.
     /// </summary>
     /// <param name="kernel">
     /// A valid instance of <see cref="Kernel"/>, used to get the configured text embeddings generation service (<see cref="ITextEmbeddingGenerationService"/>) required by this manager.
     /// </param>
     /// <param name="memoryStore">A valid instance of a <see cref="IMemoryStore"/> to manage.</param>
-    public MemoryManager(Kernel kernel, IMemoryStore memoryStore)
+    public MemoryManager(IMemoryStore memoryStore, ITextEmbeddingGenerationService textEmbeddingGenerationService, ILogger<MemoryManager> logger)
     {
-        this.kernel = kernel;
-        logger = kernel.LoggerFactory.CreateLogger<MemoryManager>();
+        this.logger = logger;
         MemoryStore = memoryStore;
+
+        this.textEmbeddingGenerationService = textEmbeddingGenerationService;
     }
 
     /// <inheritdoc/>
     public IMemoryStore MemoryStore { get; init; }
 
     /// <inheritdoc/>
-    public virtual async Task UpsertMemoryAsync(string memoryId, string collectionName, IEnumerable<string> chunks, CancellationToken cancellationToken, IDictionary<string, string> metadata = null)
+    public virtual async Task UpsertMemoryAsync(string memoryId, string collectionName, IEnumerable<string> chunks, IDictionary<string, string> metadata = null, Kernel kernel = null, CancellationToken cancellationToken = default)
     {
         var memoryChunkSize = await GetChunkSize(memoryId, collectionName, cancellationToken);
 
@@ -51,7 +52,7 @@ public virtual async Task UpsertMemoryAsync(string memoryId, string collectionNa
             await DeleteMemoryAsync(memoryId, collectionName, memoryChunkSize, cancellationToken);
         }
 
-        await SaveChunks(memoryId, collectionName, chunks, metadata, cancellationToken);
+        await SaveChunks(memoryId, collectionName, chunks, metadata, kernel, cancellationToken);
     }
 
     /// <inheritdoc/>
@@ -86,7 +87,7 @@ public virtual async Task<MemoryContent> GetMemoryAsync(string memoryId, string
     }
 
     /// <inheritdoc/>
-    public virtual async IAsyncEnumerable<string> BatchUpsertMemoriesAsync(string collectionName, IDictionary<string, MemoryContent> memoryContents, [EnumeratorCancellation] CancellationToken cancellationToken)
+    public virtual async IAsyncEnumerable<string> BatchUpsertMemoriesAsync(string collectionName, IDictionary<string, MemoryContent> memoryContents, Kernel kernel = null, [EnumeratorCancellation] CancellationToken cancellationToken = default)
     {
         var memoryRecords = new Collection<MemoryRecord>();
 
@@ -103,7 +104,7 @@ public virtual async IAsyncEnumerable<string> BatchUpsertMemoriesAsync(string co
                 for (var i = 0; i < totalChunks; i++)
                 {
                     var chunk = memoryContent.Chunks.ElementAt(i);
-                    var embedding = await kernel.GetRequiredService<ITextEmbeddingGenerationService>().GenerateEmbeddingAsync(chunk, kernel, cancellationToken);
+                    var embedding = await textEmbeddingGenerationService.GenerateEmbeddingAsync(chunk, kernel, cancellationToken);
                     memoryRecords.Add(MemoryRecord.LocalRecord($@"{memoryContentId}-{i}", chunk, null, embedding, JsonSerializer.Serialize(memoryContent.Metadata)));
                 }
             }
@@ -139,7 +140,7 @@ private async Task DeleteMemoryAsync(string memoryId, string collectionName, int
         await MemoryStore.RemoveBatchAsync(collectionName, Enumerable.Range(0, chunkSize).Select(i => BuildMemoryIdentifier(memoryId, i)), cancellationToken);
     }
 
-    private async Task SaveChunks(string memoryid, string collectionName, IEnumerable<string> chunks, IDictionary<string, string> metadata, CancellationToken cancellationToken)
+    private async Task SaveChunks(string memoryid, string collectionName, IEnumerable<string> chunks, IDictionary<string, string> metadata, Kernel kernel = null, CancellationToken cancellationToken = default)
     {
         metadata ??= new Dictionary<string, string>();
 
@@ -154,7 +155,7 @@ private async Task SaveChunks(string memoryid, string collectionName, IEnumerabl
         for (var i = 0; i < chunksCount; i++)
         {
             var chunk = chunks.ElementAt(i);
-            var embedding = await kernel.GetRequiredService<ITextEmbeddingGenerationService>().GenerateEmbeddingAsync(chunk, kernel, cancellationToken);
+            var embedding = await textEmbeddingGenerationService.GenerateEmbeddingAsync(chunk, kernel, cancellationToken);
             memoryRecords.Add(MemoryRecord.LocalRecord(BuildMemoryIdentifier(memoryid, i), chunk, null, embedding, metadataJson));
         }