Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various improvements in IMemoryStore, IMemoryManager and IMemoryStoreHandler. #49

Merged
merged 1 commit into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,14 @@ Previous classification is not required if changes are simple or all belong to t

## [8.1.2]

### 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.

### 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.

### 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-3</VersionSuffix>
<VersionSuffix>preview-4</VersionSuffix>
</PropertyGroup>

<!--
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using Microsoft.SemanticKernel.Memory;
// Ignore Spelling: Upsert

using Microsoft.SemanticKernel.Memory;

namespace Encamina.Enmarcha.SemanticKernel.Abstractions;

Expand All @@ -7,6 +9,11 @@
/// </summary>
public interface IMemoryManager
{
/// <summary>
/// Gets the instance of the memory store manage by this manager.
/// </summary>
IMemoryStore MemoryStore { get; }

/// <summary>
/// Upserts the memory content into a collection.
/// </summary>
Expand All @@ -16,7 +23,7 @@
/// <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>
public 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, CancellationToken cancellationToken, IDictionary<string, string> metadata = null);

Check warning on line 26 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 26 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)

/// <summary>
/// Deletes the memory content from a collection.
Expand All @@ -25,7 +32,7 @@
/// <param name="collectionName">Name of the collection from where the content will be deleted.</param>
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
public Task DeleteMemoryAsync(string memoryId, string collectionName, CancellationToken cancellationToken);
Task DeleteMemoryAsync(string memoryId, string collectionName, CancellationToken cancellationToken);

/// <summary>
/// Gets the memory content from a collection.
Expand All @@ -34,15 +41,15 @@
/// <param name="collectionName">Name of the collection where the content will be saved.</param>
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
/// <returns>A <see cref="Task"/> containing the <see cref="MemoryContent"/>, or <see langword="null"/> if the content could not be found.</returns>
public Task<MemoryContent> GetMemoryAsync(string memoryId, string collectionName, CancellationToken cancellationToken);
Task<MemoryContent> GetMemoryAsync(string memoryId, string collectionName, CancellationToken cancellationToken);

/// <summary>
/// Upserts a batch of memory contents into a collection.
/// </summary>
/// <param name="collectionName">Name of the collection where the content will be saved.</param>
/// <param name="memoryContents">
/// Dictionary with the memory contents to upsert.
/// The <c>key</c> in the dictionary must containt a unique identifier for the content of the memory,
/// The <c>key</c> in the dictionary must contain a unique identifier for the content of the memory,
/// and the <c>value</c> of the dictionary must provide the memory content (chunks and metadata).
/// </param>
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
namespace Encamina.Enmarcha.SemanticKernel.Abstractions;

/// <summary>
/// Represents types that helps working with memory stores (i.e., <see cref="IMemoryStore"/>), when sometimes
/// it is necessary to centrally manage how some operations or actions are done.
/// Represents types that helps working with memory stores (i.e., <see cref="IMemoryStore"/>) through a memory handler (<see cref="IMemoryManager"/>),
/// when sometimes it is necessary to centrally manage how some operations or actions are done.
/// </summary>
public interface IMemoryStoreHandler
{
Expand All @@ -18,6 +18,11 @@ public interface IMemoryStoreHandler
/// </summary>
string CollectionNamePrefix { get; init; }

/// <summary>
/// Gets the <see cref="IMemoryManager"/> that manages the memory stored handled by this instance.
/// </summary>
IMemoryManager MemoryManager { get; }

/// <summary>
/// Gets the name of a collection from its unique identifier.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,20 @@
/// <summary>
/// Base class for memory stores (i.e., <see cref="IMemoryStore"/>) handlers.
/// </summary>
public abstract class MemoryStoreHandlerBase : IMemoryStoreHandler

Check warning on line 12 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/MemoryStoreHandlerBase.cs

View workflow job for this annotation

GitHub Actions / CI

Convert this 'abstract' class to a concrete type with a protected constructor. (https://rules.sonarsource.com/csharp/RSPEC-1694)

Check warning on line 12 in src/Encamina.Enmarcha.SemanticKernel.Abstractions/MemoryStoreHandlerBase.cs

View workflow job for this annotation

GitHub Actions / CI

Convert this 'abstract' class to a concrete type with a protected constructor. (https://rules.sonarsource.com/csharp/RSPEC-1694)
{
/// <summary>
/// Initializes a new instance of the <see cref="MemoryStoreHandlerBase"/> class.
/// </summary>
/// <param name="memoryStore">The <see cref="IMemoryStore"/> to handle.</param>
#pragma warning disable SKEXP0003 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
protected MemoryStoreHandlerBase(IMemoryStore memoryStore)
#pragma warning restore SKEXP0003 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
/// <param name="memoryManager">A valid instance of <see cref="IMemoryManager"/> that manages the memory store handled by this instance.</param>
protected MemoryStoreHandlerBase(IMemoryManager memoryManager)
{
MemoryStore = memoryStore;
MemoryManager = memoryManager;
}

/// <inheritdoc/>
public virtual IMemoryManager MemoryManager { get; }

/// <inheritdoc/>
public virtual string CollectionNamePostfix { get; init; }

Expand All @@ -33,11 +34,6 @@
/// </summary>
protected IDictionary<string, MemoryStoreCollection> MemoryStoreCollectionInfo { get; } = new ConcurrentDictionary<string, MemoryStoreCollection>();

/// <summary>
/// Gets the current <see cref="IMemoryStore"/> handled by this instance.
/// </summary>
protected IMemoryStore MemoryStore { get; }

/// <inheritdoc/>
public virtual async Task<string> GetCollectionNameAsync(string collectionId, CancellationToken cancellationToken)
{
Expand All @@ -55,9 +51,9 @@
LastAccessUtc = DateTime.UtcNow,
};

if (!await MemoryStore.DoesCollectionExistAsync(collectionName, cancellationToken))
if (!await MemoryManager.MemoryStore.DoesCollectionExistAsync(collectionName, cancellationToken))
{
await MemoryStore.CreateCollectionAsync(collectionName, cancellationToken);
await MemoryManager.MemoryStore.CreateCollectionAsync(collectionName, cancellationToken);
}

return MemoryStoreCollectionInfo[collectionId].CollectionName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ internal sealed class EphemeralMemoryStoreHandler : MemoryStoreHandlerBase
/// <summary>
/// Initializes a new instance of the <see cref="EphemeralMemoryStoreHandler"/> class.
/// </summary>
/// <param name="memoryStore">A valid instance of an <see cref="IMemoryStore"/> handled by this memory store handler.</param>
/// <param name="memoryManager">A valid instance of <see cref="IMemoryManager"/> that manages the memory store handled by this instance.</param>
/// <param name="sessionManagementOptions">Configuration options for this memory store handler.</param>
/// <param name="logger">A logger for this memory store handler.</param>
public EphemeralMemoryStoreHandler(IMemoryStore memoryStore, IOptionsMonitor<EphemeralMemoryStoreHandlerOptions> sessionManagementOptions, ILogger<EphemeralMemoryStoreHandler> logger)
: base(memoryStore)
public EphemeralMemoryStoreHandler(IMemoryManager memoryManager, IOptionsMonitor<EphemeralMemoryStoreHandlerOptions> sessionManagementOptions, ILogger<EphemeralMemoryStoreHandler> logger)
: base(memoryManager)
{
this.logger = logger;

Expand Down Expand Up @@ -54,7 +54,7 @@ private async Task RemoveOutdatedCollectionsAsync(CancellationToken cancellation
foreach (var memoryStoreInfo in MemoryStoreCollectionInfo.Where(i => i.Value.LastAccessUtc < date).ToList())
{
MemoryStoreCollectionInfo.Remove(memoryStoreInfo.Key);
await MemoryStore.DeleteCollectionAsync(memoryStoreInfo.Value.CollectionName, cancellationToken);
await MemoryManager.MemoryStore.DeleteCollectionAsync(memoryStoreInfo.Value.CollectionName, cancellationToken);
}

Thread.Sleep(TimeSpan.FromMinutes(options.InactivePollingTimeMinutes));
Expand Down
41 changes: 25 additions & 16 deletions src/Encamina.Enmarcha.SemanticKernel/MemoryManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,30 @@
/// <summary>
/// Manager that provides some CRUD operations over memories with multiple chunks that need to be managed by an <see cref="IMemoryStore"/>, using batch operations.
/// </summary>
/// <remarks>
/// Initializes a new instance of the <see cref="MemoryManager"/> class.
/// </remarks>
/// <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 class MemoryManager(Kernel kernel, IMemoryStore memoryStore) : IMemoryManager
public class MemoryManager : IMemoryManager
{
private const string ChunkSize = @"chunkSize";

private readonly Kernel kernel = kernel;
private readonly Kernel kernel;

private readonly ILogger logger = kernel.LoggerFactory.CreateLogger<MemoryManager>();
private readonly IMemoryStore memoryStore = memoryStore;
private readonly ILogger logger;

/// <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)
{
this.kernel = kernel;
logger = kernel.LoggerFactory.CreateLogger<MemoryManager>();
MemoryStore = memoryStore;
}

/// <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)
Expand Down Expand Up @@ -66,7 +75,7 @@
return null;
}

var memoryRecords = await memoryStore.GetBatchAsync(collectionName, Enumerable.Range(0, chunkSize).Select(i => BuildMemoryIdentifier(memoryId, i)), cancellationToken: cancellationToken)
var memoryRecords = await MemoryStore.GetBatchAsync(collectionName, Enumerable.Range(0, chunkSize).Select(i => BuildMemoryIdentifier(memoryId, i)), cancellationToken: cancellationToken)
.ToListAsync(cancellationToken);

return new MemoryContent
Expand All @@ -77,7 +86,7 @@
}

/// <inheritdoc/>
public virtual async IAsyncEnumerable<string> BatchUpsertMemoriesAsync(string collectionName, IDictionary<string, MemoryContent> memoryContents, [EnumeratorCancellation] CancellationToken cancellationToken)

Check warning on line 89 in src/Encamina.Enmarcha.SemanticKernel/MemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

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

Check warning on line 89 in src/Encamina.Enmarcha.SemanticKernel/MemoryManager.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 210 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)
{
var memoryRecords = new Collection<MemoryRecord>();

Expand All @@ -100,7 +109,7 @@
}
}

var memoryRecordsUniqueIdentifiers = memoryStore.UpsertBatchAsync(collectionName, memoryRecords, cancellationToken);
var memoryRecordsUniqueIdentifiers = MemoryStore.UpsertBatchAsync(collectionName, memoryRecords, cancellationToken);

await foreach (var item in memoryRecordsUniqueIdentifiers)
{
Expand All @@ -113,7 +122,7 @@

private async Task<int> GetChunkSize(string memoryId, string collectionName, CancellationToken cancellationToken)
{
var fistMemoryChunk = await memoryStore.GetAsync(collectionName, BuildMemoryIdentifier(memoryId, 0), cancellationToken: cancellationToken);
var fistMemoryChunk = await MemoryStore.GetAsync(collectionName, BuildMemoryIdentifier(memoryId, 0), cancellationToken: cancellationToken);

if (fistMemoryChunk == null)
{
Expand All @@ -127,7 +136,7 @@

private async Task DeleteMemoryAsync(string memoryId, string collectionName, int chunkSize, CancellationToken cancellationToken)
{
await memoryStore.RemoveBatchAsync(collectionName, Enumerable.Range(0, chunkSize).Select(i => BuildMemoryIdentifier(memoryId, i)), cancellationToken);
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)
Expand All @@ -149,6 +158,6 @@
memoryRecords.Add(MemoryRecord.LocalRecord(BuildMemoryIdentifier(memoryid, i), chunk, null, embedding, metadataJson));
}

await memoryStore.UpsertBatchAsync(collectionName, memoryRecords, cancellationToken).ToListAsync(cancellationToken: cancellationToken);
await MemoryStore.UpsertBatchAsync(collectionName, memoryRecords, cancellationToken).ToListAsync(cancellationToken: cancellationToken);
}
}