Skip to content

Commit

Permalink
- Signature of UpsertMemoryAsync method has changed in `IMemoryMana…
Browse files Browse the repository at this point in the history
…ger` 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`.
  • Loading branch information
rliberoff committed Jan 19, 2024
1 parent d2a8f53 commit 991f297
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 17 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ Previous classification is not required if changes are simple or all belong to t
### Braking 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`.
Expand Down
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

<PropertyGroup>
<VersionPrefix>8.1.2</VersionPrefix>
<VersionSuffix>preview-6</VersionSuffix>
<VersionSuffix>preview-7</VersionSuffix>
</PropertyGroup>

<!--
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Ignore Spelling: Upsert

using Microsoft.SemanticKernel;
using Microsoft.SemanticKernel.Memory;

namespace Encamina.Enmarcha.SemanticKernel.Abstractions;
Expand All @@ -23,7 +24,7 @@ public interface IMemoryManager
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>

Check warning on line 24 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

The parameter documentation for 'cancellationToken' should be at position 6 (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1612.md)

Check warning on line 24 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

The parameter documentation for 'cancellationToken' should be at position 6 (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1612.md)
/// <param name="metadata">Metadata of the memory.</param>

Check warning on line 25 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

The parameter documentation for 'metadata' should be at position 4 (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1612.md)
/// <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);

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Parameter 'kernel' has no matching param tag in the XML comment for 'IMemoryManager.UpsertMemoryAsync(string, string, IEnumerable<string>, IDictionary<string, string>, Kernel, CancellationToken)' (but other parameters do)

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 209 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Use the overloading mechanism instead of the optional parameters. (https://rules.sonarsource.com/csharp/RSPEC-2360)

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Use the overloading mechanism instead of the optional parameters. (https://rules.sonarsource.com/csharp/RSPEC-2360)

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Use the overloading mechanism instead of the optional parameters. (https://rules.sonarsource.com/csharp/RSPEC-2360)

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Parameter 'kernel' has no matching param tag in the XML comment for 'IMemoryManager.UpsertMemoryAsync(string, string, IEnumerable<string>, IDictionary<string, string>, Kernel, CancellationToken)' (but other parameters do)

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 209 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Use the overloading mechanism instead of the optional parameters. (https://rules.sonarsource.com/csharp/RSPEC-2360)

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Use the overloading mechanism instead of the optional parameters. (https://rules.sonarsource.com/csharp/RSPEC-2360)

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Use the overloading mechanism instead of the optional parameters. (https://rules.sonarsource.com/csharp/RSPEC-2360)

Check warning on line 27 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI


/// <summary>
/// Deletes the memory content from a collection.
Expand Down Expand Up @@ -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);

Check warning on line 58 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Parameter 'kernel' has no matching param tag in the XML comment for 'IMemoryManager.BatchUpsertMemoriesAsync(string, IDictionary<string, MemoryContent>, Kernel, CancellationToken)' (but other parameters do)

Check warning on line 58 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Use the overloading mechanism instead of the optional parameters. (https://rules.sonarsource.com/csharp/RSPEC-2360)

Check warning on line 58 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Use the overloading mechanism instead of the optional parameters. (https://rules.sonarsource.com/csharp/RSPEC-2360)

Check warning on line 58 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Parameter 'kernel' has no matching param tag in the XML comment for 'IMemoryManager.BatchUpsertMemoriesAsync(string, IDictionary<string, MemoryContent>, Kernel, CancellationToken)' (but other parameters do)

Check warning on line 58 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Use the overloading mechanism instead of the optional parameters. (https://rules.sonarsource.com/csharp/RSPEC-2360)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -47,7 +47,7 @@ public static IServiceCollection AddEphemeralMemoryStoreHandler(this IServiceCol
.ValidateDataAnnotations()
.ValidateOnStart();

services.TryAddTransient<IMemoryStoreHandler, EphemeralMemoryStoreHandler>();
services.TryAddSingleton<IMemoryStoreHandler, EphemeralMemoryStoreHandler>();

return services;
}
Expand Down
23 changes: 12 additions & 11 deletions src/Encamina.Enmarcha.SemanticKernel/MemoryManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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/>
Expand Down Expand Up @@ -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>();

Expand All @@ -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)));
}
}
Expand Down Expand Up @@ -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>();

Expand All @@ -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));
}

Expand Down

0 comments on commit 991f297

Please sign in to comment.