diff --git a/AGENTS.md b/AGENTS.md index 08aef0e4b..27a0cad10 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,6 @@ - Follow @docs/AGENTS.md instructions without exceptions - Ignore the "archived" directory +- During development don't EVER write to my personal ~/.km dir - For any task with multiple steps, create a todo list FIRST, then execute. - Avoid destructive operations like "get reset", "git clean", etc. diff --git a/docs b/docs index 6c3214d43..5565295b2 160000 --- a/docs +++ b/docs @@ -1 +1 @@ -Subproject commit 6c3214d43b8e30a43ad7d36bfad741f4ab922ff8 +Subproject commit 5565295b23b6932fa5f71192fba9fd8ea395b191 diff --git a/src/Core/Config/AppConfig.cs b/src/Core/Config/AppConfig.cs index 36999a53d..792f0e1d1 100644 --- a/src/Core/Config/AppConfig.cs +++ b/src/Core/Config/AppConfig.cs @@ -57,24 +57,31 @@ public void Validate(string path = "") /// /// Creates a default configuration with a single "personal" node - /// using local SQLite storage + /// using local SQLite storage in the user's home directory /// public static AppConfig CreateDefault() { var homeDir = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); var kmDir = Path.Combine(homeDir, ".km"); - var personalNodeDir = Path.Combine(kmDir, "nodes", "personal"); + return CreateDefault(kmDir); + } + + /// + /// Creates a default configuration with a single "personal" node + /// using local SQLite storage in the specified base directory + /// + /// Base directory for data storage + public static AppConfig CreateDefault(string baseDir) + { + var personalNodeDir = Path.Combine(baseDir, "nodes", "personal"); return new AppConfig { Nodes = new Dictionary { ["personal"] = NodeConfig.CreateDefaultPersonalNode(personalNodeDir) - }, - EmbeddingsCache = CacheConfig.CreateDefaultSqliteCache( - Path.Combine(kmDir, "embeddings-cache.db") - ), - LLMCache = null + } + // EmbeddingsCache and LLMCache intentionally omitted - add when features are implemented }; } } diff --git a/src/Core/Config/ConfigParser.cs b/src/Core/Config/ConfigParser.cs index c04cfa5bb..f20bd2c40 100644 --- a/src/Core/Config/ConfigParser.cs +++ b/src/Core/Config/ConfigParser.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. using System.Text.Json; +using System.Text.Json.Serialization.Metadata; using KernelMemory.Core.Config.Cache; using KernelMemory.Core.Config.ContentIndex; using KernelMemory.Core.Config.SearchIndex; @@ -19,17 +20,33 @@ public static class ConfigParser /// - Case insensitive property names /// - Comments allowed /// - Trailing commas allowed + /// - Supports polymorphic deserialization /// private static readonly JsonSerializerOptions s_jsonOptions = new() { PropertyNameCaseInsensitive = true, ReadCommentHandling = JsonCommentHandling.Skip, AllowTrailingCommas = true, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + TypeInfoResolver = new DefaultJsonTypeInfoResolver() }; /// - /// Loads configuration from a file, or returns default config if file doesn't exist + /// JSON serializer options for writing config files + /// - Indented formatting + /// - Camel case property names + /// - Omit null values + /// + private static readonly JsonSerializerOptions s_writeJsonOptions = new() + { + WriteIndented = true, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + DefaultIgnoreCondition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull + }; + + /// + /// Loads configuration from a file, or creates default config if file doesn't exist. + /// The config file is always ensured to exist on disk after loading. /// Performs tilde expansion on paths (~/ → home directory) /// /// Path to configuration file @@ -37,10 +54,21 @@ public static class ConfigParser /// Thrown when file exists but parsing or validation fails public static AppConfig LoadFromFile(string filePath) { - // If file doesn't exist, return default configuration + AppConfig config; + + // If file doesn't exist, create default configuration relative to config file location if (!File.Exists(filePath)) { - return AppConfig.CreateDefault(); + var configDir = Path.GetDirectoryName(filePath); + var baseDir = string.IsNullOrEmpty(configDir) ? "." : configDir; + + // Create default config relative to config file location + config = AppConfig.CreateDefault(baseDir); + + // Write the config file + WriteConfigFile(filePath, config); + + return config; } try @@ -49,11 +77,14 @@ public static AppConfig LoadFromFile(string filePath) var json = File.ReadAllText(filePath); // Parse and validate - var config = ParseFromString(json); + config = ParseFromString(json); // Expand tilde paths ExpandTildePaths(config); + // Always ensure the config file exists (recreate if deleted between load and save) + WriteConfigFileIfMissing(filePath, config); + return config; } catch (ConfigException) @@ -251,4 +282,38 @@ private static string ExpandTilde(string path, string homeDir) return path; } + /// + /// Writes the config file to disk if it doesn't exist. + /// Used to ensure config file is always present after any operation. + /// + /// Path to configuration file + /// Configuration to write + private static void WriteConfigFileIfMissing(string filePath, AppConfig config) + { + if (File.Exists(filePath)) + { + return; + } + + WriteConfigFile(filePath, config); + } + + /// + /// Writes the config file to disk, creating directories if needed. + /// + /// Path to configuration file + /// Configuration to write + private static void WriteConfigFile(string filePath, AppConfig config) + { + // Create the directory if needed + var configDir = Path.GetDirectoryName(filePath); + if (!string.IsNullOrEmpty(configDir) && !Directory.Exists(configDir)) + { + Directory.CreateDirectory(configDir); + } + + // Write the config file + var json = System.Text.Json.JsonSerializer.Serialize(config, s_writeJsonOptions); + File.WriteAllText(filePath, json); + } } diff --git a/src/Core/Config/ContentIndex/ContentIndexConfig.cs b/src/Core/Config/ContentIndex/ContentIndexConfig.cs index e36631f61..ccf99fa6b 100644 --- a/src/Core/Config/ContentIndex/ContentIndexConfig.cs +++ b/src/Core/Config/ContentIndex/ContentIndexConfig.cs @@ -9,7 +9,7 @@ namespace KernelMemory.Core.Config.ContentIndex; /// Base class for content index configurations /// Content index is the source of truth, backed by Entity Framework /// -[JsonPolymorphic(TypeDiscriminatorPropertyName = "$type")] +[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")] [JsonDerivedType(typeof(SqliteContentIndexConfig), typeDiscriminator: "sqlite")] [JsonDerivedType(typeof(PostgresContentIndexConfig), typeDiscriminator: "postgres")] public abstract class ContentIndexConfig : IValidatable diff --git a/src/Core/Config/Embeddings/EmbeddingsConfig.cs b/src/Core/Config/Embeddings/EmbeddingsConfig.cs index 780e136c3..154864d48 100644 --- a/src/Core/Config/Embeddings/EmbeddingsConfig.cs +++ b/src/Core/Config/Embeddings/EmbeddingsConfig.cs @@ -8,7 +8,7 @@ namespace KernelMemory.Core.Config.Embeddings; /// /// Base class for embeddings provider configurations /// -[JsonPolymorphic(TypeDiscriminatorPropertyName = "$type")] +[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")] [JsonDerivedType(typeof(OllamaEmbeddingsConfig), typeDiscriminator: "ollama")] [JsonDerivedType(typeof(OpenAIEmbeddingsConfig), typeDiscriminator: "openai")] [JsonDerivedType(typeof(AzureOpenAIEmbeddingsConfig), typeDiscriminator: "azureOpenAI")] diff --git a/src/Core/Config/NodeConfig.cs b/src/Core/Config/NodeConfig.cs index 69a25caf2..6ba521d86 100644 --- a/src/Core/Config/NodeConfig.cs +++ b/src/Core/Config/NodeConfig.cs @@ -71,10 +71,24 @@ public void Validate(string path) this.FileStorage?.Validate($"{path}.FileStorage"); this.RepoStorage?.Validate($"{path}.RepoStorage"); + // Validate search indexes for (int i = 0; i < this.SearchIndexes.Count; i++) { this.SearchIndexes[i].Validate($"{path}.SearchIndexes[{i}]"); } + + // Ensure all search index IDs are unique within this node + var duplicateIds = this.SearchIndexes + .GroupBy(idx => idx.Id) + .Where(g => g.Count() > 1) + .Select(g => g.Key) + .ToList(); + + if (duplicateIds.Count > 0) + { + throw new ConfigException($"{path}.SearchIndexes", + $"Duplicate search index IDs found: {string.Join(", ", duplicateIds)}. Each search index must have a unique ID within a node."); + } } /// @@ -97,16 +111,10 @@ internal static NodeConfig CreateDefaultPersonalNode(string nodeDir) { new FtsSearchIndexConfig { + Id = "sqlite-fts", Type = SearchIndexTypes.SqliteFTS, Path = Path.Combine(nodeDir, "fts.db"), EnableStemming = true - }, - new VectorSearchIndexConfig - { - Type = SearchIndexTypes.SqliteVector, - Path = Path.Combine(nodeDir, "vectors.db"), - Dimensions = 768, - Metric = VectorMetrics.Cosine } } }; diff --git a/src/Core/Config/SearchIndex/FtsSearchIndexConfig.cs b/src/Core/Config/SearchIndex/FtsSearchIndexConfig.cs index 5844c3675..2615c0b1a 100644 --- a/src/Core/Config/SearchIndex/FtsSearchIndexConfig.cs +++ b/src/Core/Config/SearchIndex/FtsSearchIndexConfig.cs @@ -33,6 +33,12 @@ public sealed class FtsSearchIndexConfig : SearchIndexConfig /// public override void Validate(string path) { + // Validate ID is provided + if (string.IsNullOrWhiteSpace(this.Id)) + { + throw new ConfigException($"{path}.Id", "Search index ID is required"); + } + this.Embeddings?.Validate($"{path}.Embeddings"); var isSqlite = this.Type == SearchIndexTypes.SqliteFTS; diff --git a/src/Core/Config/SearchIndex/GraphSearchIndexConfig.cs b/src/Core/Config/SearchIndex/GraphSearchIndexConfig.cs index 097c00026..acafd81e6 100644 --- a/src/Core/Config/SearchIndex/GraphSearchIndexConfig.cs +++ b/src/Core/Config/SearchIndex/GraphSearchIndexConfig.cs @@ -27,6 +27,12 @@ public sealed class GraphSearchIndexConfig : SearchIndexConfig /// public override void Validate(string path) { + // Validate ID is provided + if (string.IsNullOrWhiteSpace(this.Id)) + { + throw new ConfigException($"{path}.Id", "Search index ID is required"); + } + this.Embeddings?.Validate($"{path}.Embeddings"); var hasPath = !string.IsNullOrWhiteSpace(this.Path); diff --git a/src/Core/Config/SearchIndex/SearchIndexConfig.cs b/src/Core/Config/SearchIndex/SearchIndexConfig.cs index 57414b039..d6e4a5dc7 100644 --- a/src/Core/Config/SearchIndex/SearchIndexConfig.cs +++ b/src/Core/Config/SearchIndex/SearchIndexConfig.cs @@ -9,12 +9,20 @@ namespace KernelMemory.Core.Config.SearchIndex; /// /// Base class for search index configurations /// -[JsonPolymorphic(TypeDiscriminatorPropertyName = "$type")] -[JsonDerivedType(typeof(FtsSearchIndexConfig), typeDiscriminator: "fts")] -[JsonDerivedType(typeof(VectorSearchIndexConfig), typeDiscriminator: "vector")] +[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")] +[JsonDerivedType(typeof(FtsSearchIndexConfig), typeDiscriminator: "sqliteFTS")] +[JsonDerivedType(typeof(VectorSearchIndexConfig), typeDiscriminator: "sqliteVector")] [JsonDerivedType(typeof(GraphSearchIndexConfig), typeDiscriminator: "graph")] public abstract class SearchIndexConfig : IValidatable { + /// + /// Unique identifier for this search index instance. + /// Must be unique within a node's SearchIndexes array. + /// Used to identify index instances in operations pipeline (stable across config changes). + /// + [JsonPropertyName("id")] + public string Id { get; set; } = string.Empty; + /// /// Type of search index /// diff --git a/src/Core/Config/SearchIndex/VectorSearchIndexConfig.cs b/src/Core/Config/SearchIndex/VectorSearchIndexConfig.cs index 04eb88dad..f320c12f9 100644 --- a/src/Core/Config/SearchIndex/VectorSearchIndexConfig.cs +++ b/src/Core/Config/SearchIndex/VectorSearchIndexConfig.cs @@ -40,6 +40,12 @@ public sealed class VectorSearchIndexConfig : SearchIndexConfig /// public override void Validate(string path) { + // Validate ID is provided + if (string.IsNullOrWhiteSpace(this.Id)) + { + throw new ConfigException($"{path}.Id", "Search index ID is required"); + } + this.Embeddings?.Validate($"{path}.Embeddings"); var isSqlite = this.Type == SearchIndexTypes.SqliteVector; diff --git a/src/Core/Config/Storage/StorageConfig.cs b/src/Core/Config/Storage/StorageConfig.cs index 4135a59ef..ee3150e40 100644 --- a/src/Core/Config/Storage/StorageConfig.cs +++ b/src/Core/Config/Storage/StorageConfig.cs @@ -8,7 +8,7 @@ namespace KernelMemory.Core.Config.Storage; /// /// Base class for storage configurations (files, repositories) /// -[JsonPolymorphic(TypeDiscriminatorPropertyName = "$type")] +[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")] [JsonDerivedType(typeof(DiskStorageConfig), typeDiscriminator: "disk")] [JsonDerivedType(typeof(AzureBlobStorageConfig), typeDiscriminator: "azureBlobs")] public abstract class StorageConfig : IValidatable diff --git a/src/Core/Search/FtsMatch.cs b/src/Core/Search/FtsMatch.cs new file mode 100644 index 000000000..153af2480 --- /dev/null +++ b/src/Core/Search/FtsMatch.cs @@ -0,0 +1,25 @@ +// Copyright (c) Microsoft. All rights reserved. +namespace KernelMemory.Core.Search; + +/// +/// Represents a match from a full-text search query. +/// +public sealed class FtsMatch +{ + /// + /// The content ID that matched the search query. + /// + public required string ContentId { get; init; } + + /// + /// The relevance score (higher is more relevant). + /// FTS5 rank is negative (closer to 0 is better), so we negate it. + /// + public required double Score { get; init; } + + /// + /// A snippet of the matched text with highlights. + /// Highlights are marked with configurable markers (default: no markers). + /// + public required string Snippet { get; init; } +} diff --git a/src/Core/Search/IFtsIndex.cs b/src/Core/Search/IFtsIndex.cs new file mode 100644 index 000000000..2fc488e11 --- /dev/null +++ b/src/Core/Search/IFtsIndex.cs @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft. All rights reserved. +namespace KernelMemory.Core.Search; + +/// +/// Interface for Full-Text Search index operations. +/// Extends ISearchIndex with search-specific capabilities. +/// Implementations include SQLite FTS5 and PostgreSQL FTS. +/// +public interface IFtsIndex : ISearchIndex +{ + /// + /// Searches the full-text index for matching content. + /// + /// The search query (FTS5 syntax supported). + /// Maximum number of results to return. + /// Cancellation token. + /// List of matches ordered by relevance (highest score first). + Task> SearchAsync(string query, int limit = 10, CancellationToken cancellationToken = default); +} diff --git a/src/Core/Search/ISearchIndex.cs b/src/Core/Search/ISearchIndex.cs new file mode 100644 index 000000000..c5ce9a1a7 --- /dev/null +++ b/src/Core/Search/ISearchIndex.cs @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft. All rights reserved. +namespace KernelMemory.Core.Search; + +/// +/// Base interface for all search index types. +/// Implementations include FTS, vector search, graph search, etc. +/// +public interface ISearchIndex +{ + /// + /// Updates this index when content is created or updated. + /// + /// The content ID to index. + /// The text content to index. + /// Cancellation token. + Task IndexAsync(string contentId, string text, CancellationToken cancellationToken = default); + + /// + /// Removes content from this index. + /// Idempotent - no error if content doesn't exist. + /// + /// The content ID to remove. + /// Cancellation token. + Task RemoveAsync(string contentId, CancellationToken cancellationToken = default); + + /// + /// Clears all entries from this index. + /// Used for rebuilding from content storage. + /// + /// Cancellation token. + Task ClearAsync(CancellationToken cancellationToken = default); +} diff --git a/src/Core/Search/SqliteFtsIndex.cs b/src/Core/Search/SqliteFtsIndex.cs new file mode 100644 index 000000000..a09447087 --- /dev/null +++ b/src/Core/Search/SqliteFtsIndex.cs @@ -0,0 +1,194 @@ +// Copyright (c) Microsoft. All rights reserved. +using Microsoft.Data.Sqlite; +using Microsoft.Extensions.Logging; + +namespace KernelMemory.Core.Search; + +/// +/// SQLite FTS5 implementation of IFtsIndex. +/// Uses a contentless FTS5 table for efficient full-text search. +/// +public sealed class SqliteFtsIndex : IFtsIndex, IDisposable +{ + private const string TableName = "km_fts"; + private readonly string _connectionString; + private readonly bool _enableStemming; + private readonly ILogger _logger; + private SqliteConnection? _connection; + private bool _disposed; + + /// + /// Initializes a new instance of SqliteFtsIndex. + /// + /// Path to the SQLite database file. + /// Enable Porter stemming for better search results. + /// Logger instance. + public SqliteFtsIndex(string dbPath, bool enableStemming, ILogger logger) + { + this._connectionString = $"Data Source={dbPath}"; + this._enableStemming = enableStemming; + this._logger = logger; + } + + /// + /// Ensures the database connection is open and tables exist. + /// + /// Cancellation token. + public async Task InitializeAsync(CancellationToken cancellationToken = default) + { + if (this._connection != null) + { + return; + } + + this._connection = new SqliteConnection(this._connectionString); + await this._connection.OpenAsync(cancellationToken).ConfigureAwait(false); + + // Create FTS5 virtual table if it doesn't exist + // Using regular FTS5 table (stores content) to support snippets + var tokenizer = this._enableStemming ? "porter unicode61" : "unicode61"; + var createTableSql = $""" + CREATE VIRTUAL TABLE IF NOT EXISTS {TableName} USING fts5( + content_id UNINDEXED, + text, + tokenize='{tokenizer}' + ); + """; + + var command = this._connection.CreateCommand(); + await using (command.ConfigureAwait(false)) + { + command.CommandText = createTableSql; + await command.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false); + } + + this._logger.LogDebug("FTS5 index initialized at {ConnectionString}", this._connectionString); + } + + /// + public async Task IndexAsync(string contentId, string text, CancellationToken cancellationToken = default) + { + await this.InitializeAsync(cancellationToken).ConfigureAwait(false); + + // Remove existing entry first (upsert semantics) + await this.RemoveAsync(contentId, cancellationToken).ConfigureAwait(false); + + // Insert new entry + var insertSql = $"INSERT INTO {TableName}(content_id, text) VALUES (@contentId, @text)"; + + var insertCommand = this._connection!.CreateCommand(); + await using (insertCommand.ConfigureAwait(false)) + { + insertCommand.CommandText = insertSql; + insertCommand.Parameters.AddWithValue("@contentId", contentId); + insertCommand.Parameters.AddWithValue("@text", text); + await insertCommand.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false); + } + + this._logger.LogDebug("Indexed content {ContentId} in FTS", contentId); + } + + /// + public async Task RemoveAsync(string contentId, CancellationToken cancellationToken = default) + { + await this.InitializeAsync(cancellationToken).ConfigureAwait(false); + + var deleteSql = $"DELETE FROM {TableName} WHERE content_id = @contentId"; + + var deleteCommand = this._connection!.CreateCommand(); + await using (deleteCommand.ConfigureAwait(false)) + { + deleteCommand.CommandText = deleteSql; + deleteCommand.Parameters.AddWithValue("@contentId", contentId); + var rowsAffected = await deleteCommand.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false); + + if (rowsAffected > 0) + { + this._logger.LogDebug("Removed content {ContentId} from FTS index", contentId); + } + } + } + + /// + public async Task> SearchAsync(string query, int limit = 10, CancellationToken cancellationToken = default) + { + await this.InitializeAsync(cancellationToken).ConfigureAwait(false); + + if (string.IsNullOrWhiteSpace(query)) + { + return []; + } + + // Search using FTS5 MATCH operator + // rank is negative (closer to 0 is better), so we negate it for Score + // snippet() generates highlighted text excerpts + var searchSql = $""" + SELECT + content_id, + -rank as score, + snippet({TableName}, 1, '', '', '...', 32) as snippet + FROM {TableName} + WHERE {TableName} MATCH @query + ORDER BY rank + LIMIT @limit + """; + + var searchCommand = this._connection!.CreateCommand(); + await using (searchCommand.ConfigureAwait(false)) + { + searchCommand.CommandText = searchSql; + searchCommand.Parameters.AddWithValue("@query", query); + searchCommand.Parameters.AddWithValue("@limit", limit); + + var results = new List(); + var reader = await searchCommand.ExecuteReaderAsync(cancellationToken).ConfigureAwait(false); + await using (reader.ConfigureAwait(false)) + { + while (await reader.ReadAsync(cancellationToken).ConfigureAwait(false)) + { + results.Add(new FtsMatch + { + ContentId = reader.GetString(0), + Score = reader.GetDouble(1), + Snippet = reader.GetString(2) + }); + } + } + + this._logger.LogDebug("FTS search for '{Query}' returned {Count} results", query, results.Count); + return results; + } + } + + /// + public async Task ClearAsync(CancellationToken cancellationToken = default) + { + await this.InitializeAsync(cancellationToken).ConfigureAwait(false); + + var deleteSql = $"DELETE FROM {TableName}"; + + var clearCommand = this._connection!.CreateCommand(); + await using (clearCommand.ConfigureAwait(false)) + { + clearCommand.CommandText = deleteSql; + await clearCommand.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false); + } + + this._logger.LogInformation("Cleared all entries from FTS index"); + } + + /// + /// Disposes the database connection. + /// + public void Dispose() + { + if (this._disposed) + { + return; + } + + this._connection?.Dispose(); + this._connection = null; + this._disposed = true; + } +} diff --git a/src/Core/Storage/ContentStorageService.cs b/src/Core/Storage/ContentStorageService.cs index 1ae249aa9..e3f7612b0 100644 --- a/src/Core/Storage/ContentStorageService.cs +++ b/src/Core/Storage/ContentStorageService.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System.Text; using System.Text.Json; +using KernelMemory.Core.Search; using KernelMemory.Core.Storage.Entities; using KernelMemory.Core.Storage.Models; using Microsoft.EntityFrameworkCore; @@ -11,30 +12,56 @@ namespace KernelMemory.Core.Storage; /// /// Implementation of IContentStorage using SQLite with queue-based execution. /// Follows two-phase write pattern with distributed locking. +/// Integrates with multiple search indexes (FTS, vector, graph, etc.). /// public class ContentStorageService : IContentStorage { private readonly ContentStorageDbContext _context; private readonly ICuidGenerator _cuidGenerator; private readonly ILogger _logger; + private readonly IReadOnlyDictionary _searchIndexById; + /// + /// Initializes ContentStorageService without search indexes. + /// + /// Database context for content storage. + /// Generator for unique content IDs. + /// Logger instance. public ContentStorageService( ContentStorageDbContext context, ICuidGenerator cuidGenerator, ILogger logger) + : this(context, cuidGenerator, logger, new Dictionary()) + { + } + + /// + /// Initializes ContentStorageService with search indexes. + /// + /// Database context for content storage. + /// Generator for unique content IDs. + /// Logger instance. + /// Dictionary of index ID to ISearchIndex instance. + public ContentStorageService( + ContentStorageDbContext context, + ICuidGenerator cuidGenerator, + ILogger logger, + IReadOnlyDictionary searchIndexById) { this._context = context; this._cuidGenerator = cuidGenerator; this._logger = logger; + this._searchIndexById = searchIndexById; } /// /// Upserts content following the two-phase write pattern. + /// Never throws after queue succeeds - returns WriteResult with completion status. /// /// /// [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Best-effort error handling for phase 2 and processing - operation is queued successfully in phase 1")] - public async Task UpsertAsync(UpsertRequest request, CancellationToken cancellationToken = default) + public async Task UpsertAsync(UpsertRequest request, CancellationToken cancellationToken = default) { // Generate ID if not provided var contentId = string.IsNullOrWhiteSpace(request.Id) @@ -43,7 +70,7 @@ public async Task UpsertAsync(UpsertRequest request, CancellationToken c this._logger.LogInformation("Starting upsert operation for content ID: {ContentId}", contentId); - // Phase 1: Queue the operation (MUST succeed) + // Phase 1: Queue the operation (MUST succeed - throws if fails) var operationId = await this.QueueUpsertOperationAsync(contentId, request, cancellationToken).ConfigureAwait(false); this._logger.LogDebug("Phase 1 complete: Operation {OperationId} queued for content {ContentId}", operationId, contentId); @@ -64,27 +91,28 @@ public async Task UpsertAsync(UpsertRequest request, CancellationToken c { await this.TryProcessNextOperationAsync(contentId, cancellationToken).ConfigureAwait(false); this._logger.LogDebug("Processing complete for content {ContentId}", contentId); + return WriteResult.Success(contentId); } catch (Exception ex) { // Log but don't fail - operation is queued and will be processed eventually this._logger.LogWarning(ex, "Failed to process operation synchronously for content {ContentId} - will be processed by background worker", contentId); + return WriteResult.QueuedOnly(contentId, ex.Message); } - - return contentId; } /// /// Deletes content following the two-phase write pattern. + /// Never throws after queue succeeds - returns WriteResult with completion status. /// /// /// [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Best-effort error handling for phase 2 and processing - operation is queued successfully in phase 1")] - public async Task DeleteAsync(string id, CancellationToken cancellationToken = default) + public async Task DeleteAsync(string id, CancellationToken cancellationToken = default) { this._logger.LogInformation("Starting delete operation for content ID: {ContentId}", id); - // Phase 1: Queue the operation (MUST succeed) + // Phase 1: Queue the operation (MUST succeed - throws if fails) var operationId = await this.QueueDeleteOperationAsync(id, cancellationToken).ConfigureAwait(false); this._logger.LogDebug("Phase 1 complete: Operation {OperationId} queued for content {ContentId}", operationId, id); @@ -105,11 +133,13 @@ public async Task DeleteAsync(string id, CancellationToken cancellationToken = d { await this.TryProcessNextOperationAsync(id, cancellationToken).ConfigureAwait(false); this._logger.LogDebug("Processing complete for content {ContentId}", id); + return WriteResult.Success(id); } catch (Exception ex) { // Log but don't fail - operation is queued and will be processed eventually this._logger.LogWarning(ex, "Failed to process operation synchronously for content {ContentId} - will be processed by background worker", id); + return WriteResult.QueuedOnly(id, ex.Message); } } @@ -196,6 +226,15 @@ public async Task> ListAsync(int skip, int take, CancellationTo /// private async Task QueueUpsertOperationAsync(string contentId, UpsertRequest request, CancellationToken cancellationToken) { + // Build steps list - always upsert, then add index update for each search index + var steps = new List { "upsert" }; + + // Add step for each configured search index using its ID + foreach (var indexId in this._searchIndexById.Keys) + { + steps.Add($"index:{indexId}"); + } + var operation = new OperationRecord { Id = this._cuidGenerator.Generate(), @@ -203,9 +242,9 @@ private async Task QueueUpsertOperationAsync(string contentId, UpsertReq Cancelled = false, ContentId = contentId, Timestamp = DateTimeOffset.UtcNow, - PlannedSteps = ["upsert"], + PlannedSteps = steps.ToArray(), CompletedSteps = [], - RemainingSteps = ["upsert"], + RemainingSteps = steps.ToArray(), PayloadJson = JsonSerializer.Serialize(request), LastFailureReason = string.Empty, LastAttemptTimestamp = null @@ -214,7 +253,7 @@ private async Task QueueUpsertOperationAsync(string contentId, UpsertReq this._context.Operations.Add(operation); await this._context.SaveChangesAsync(cancellationToken).ConfigureAwait(false); - this._logger.LogDebug("Queued upsert operation {OperationId} for content {ContentId}", operation.Id, contentId); + this._logger.LogDebug("Queued upsert operation {OperationId} for content {ContentId} with steps: {Steps}", operation.Id, contentId, string.Join(", ", steps)); return operation.Id; } @@ -225,6 +264,15 @@ private async Task QueueUpsertOperationAsync(string contentId, UpsertReq /// private async Task QueueDeleteOperationAsync(string contentId, CancellationToken cancellationToken) { + // Build steps list - always delete, then remove from each search index + var steps = new List { "delete" }; + + // Add delete step for each configured search index using its ID + foreach (var indexId in this._searchIndexById.Keys) + { + steps.Add($"index:{indexId}:delete"); + } + var operation = new OperationRecord { Id = this._cuidGenerator.Generate(), @@ -232,9 +280,9 @@ private async Task QueueDeleteOperationAsync(string contentId, Cancellat Cancelled = false, ContentId = contentId, Timestamp = DateTimeOffset.UtcNow, - PlannedSteps = ["delete"], + PlannedSteps = steps.ToArray(), CompletedSteps = [], - RemainingSteps = ["delete"], + RemainingSteps = steps.ToArray(), PayloadJson = JsonSerializer.Serialize(new { Id = contentId }), LastFailureReason = string.Empty, LastAttemptTimestamp = null @@ -243,7 +291,7 @@ private async Task QueueDeleteOperationAsync(string contentId, Cancellat this._context.Operations.Add(operation); await this._context.SaveChangesAsync(cancellationToken).ConfigureAwait(false); - this._logger.LogDebug("Queued delete operation {OperationId} for content {ContentId}", operation.Id, contentId); + this._logger.LogDebug("Queued delete operation {OperationId} for content {ContentId} with steps: {Steps}", operation.Id, contentId, string.Join(", ", steps)); return operation.Id; } @@ -468,16 +516,39 @@ private async Task ExecuteStepsAsync(OperationRecord operation, CancellationToke { this._logger.LogDebug("Executing step '{Step}' for operation {OperationId}", step, operation.Id); - switch (step) + // Parse step name and execute + if (step == "upsert") + { + await this.ExecuteUpsertStepAsync(operation, cancellationToken).ConfigureAwait(false); + } + else if (step == "delete") + { + await this.ExecuteDeleteStepAsync(operation, cancellationToken).ConfigureAwait(false); + } + else if (step.StartsWith("index:", StringComparison.Ordinal)) + { + // Parse: "index:fts-stemmed" or "index:fts-exact:delete" + var parts = step.Split(':'); + if (parts.Length < 2) + { + throw new InvalidOperationException($"Invalid index step format: {step}"); + } + + var indexId = parts[1]; + var isDelete = parts.Length > 2 && parts[2] == "delete"; + + if (isDelete) + { + await this.ExecuteIndexDeleteStepAsync(operation, indexId, cancellationToken).ConfigureAwait(false); + } + else + { + await this.ExecuteIndexStepAsync(operation, indexId, cancellationToken).ConfigureAwait(false); + } + } + else { - case "upsert": - await this.ExecuteUpsertStepAsync(operation, cancellationToken).ConfigureAwait(false); - break; - case "delete": - await this.ExecuteDeleteStepAsync(operation, cancellationToken).ConfigureAwait(false); - break; - default: - throw new InvalidOperationException($"Unknown step type: {step}"); + throw new InvalidOperationException($"Unknown step type: {step}"); } // Move step from Remaining to Completed @@ -557,6 +628,60 @@ private async Task ExecuteDeleteStepAsync(OperationRecord operation, Cancellatio } } + /// + /// Execute index step: update content in specific search index. + /// Throws if index ID not found in current configuration (operation remains locked for recovery). + /// + /// The operation record. + /// The search index identifier (from config). + /// Cancellation token. + private async Task ExecuteIndexStepAsync(OperationRecord operation, string indexId, CancellationToken cancellationToken) + { + // Fail hard if index ID not found in current config + if (!this._searchIndexById.TryGetValue(indexId, out var searchIndex)) + { + this._logger.LogError("Search index '{IndexId}' not found in current configuration for operation {OperationId}. Operation will remain locked until index is restored or manually recovered.", indexId, operation.Id); + throw new InvalidOperationException($"Search index '{indexId}' not found in current configuration. Cannot process operation {operation.Id}."); + } + + // Get the content from the database + var content = await this._context.Content + .AsNoTracking() + .FirstOrDefaultAsync(c => c.Id == operation.ContentId, cancellationToken) + .ConfigureAwait(false); + + if (content == null) + { + this._logger.LogWarning("Content {ContentId} not found for indexing in index {IndexId} - skipping", operation.ContentId, indexId); + return; + } + + // Update the search index + await searchIndex.IndexAsync(operation.ContentId, content.Content, cancellationToken).ConfigureAwait(false); + this._logger.LogDebug("Indexed content {ContentId} in search index {IndexId}", operation.ContentId, indexId); + } + + /// + /// Execute index delete step: remove content from specific search index. + /// Throws if index ID not found in current configuration (operation remains locked for recovery). + /// + /// The operation record. + /// The search index identifier (from config). + /// Cancellation token. + private async Task ExecuteIndexDeleteStepAsync(OperationRecord operation, string indexId, CancellationToken cancellationToken) + { + // Fail hard if index ID not found in current config + if (!this._searchIndexById.TryGetValue(indexId, out var searchIndex)) + { + this._logger.LogError("Search index '{IndexId}' not found in current configuration for operation {OperationId}. Operation will remain locked until index is restored or manually recovered.", indexId, operation.Id); + throw new InvalidOperationException($"Search index '{indexId}' not found in current configuration. Cannot process operation {operation.Id}."); + } + + // Remove from search index (idempotent) + await searchIndex.RemoveAsync(operation.ContentId, cancellationToken).ConfigureAwait(false); + this._logger.LogDebug("Removed content {ContentId} from search index {IndexId}", operation.ContentId, indexId); + } + /// /// Step 4: Complete operation and unlock content. /// diff --git a/src/Core/Storage/IContentStorage.cs b/src/Core/Storage/IContentStorage.cs index 1bbce7e1f..da31c0880 100644 --- a/src/Core/Storage/IContentStorage.cs +++ b/src/Core/Storage/IContentStorage.cs @@ -12,23 +12,25 @@ public interface IContentStorage { /// /// Upserts content. Creates new record if ID is empty, replaces existing if ID is provided. - /// Operation is queued and processed asynchronously. + /// Operation is queued and processed synchronously (best-effort). + /// Never throws after queue succeeds - returns WriteResult with completion status. /// /// The upsert request containing content and metadata. /// Cancellation token. - /// The ID of the content record (newly generated or existing). - /// Thrown if queueing the operation fails. - Task UpsertAsync(UpsertRequest request, CancellationToken cancellationToken = default); + /// WriteResult with ID and completion status. + /// Thrown only if queueing the operation fails. + Task UpsertAsync(UpsertRequest request, CancellationToken cancellationToken = default); /// /// Deletes content by ID. Idempotent - no error if record doesn't exist. - /// Operation is queued and processed asynchronously. + /// Operation is queued and processed synchronously (best-effort). + /// Never throws after queue succeeds - returns WriteResult with completion status. /// /// The content ID to delete. /// Cancellation token. - /// Task representing the async operation. - /// Thrown if queueing the operation fails. - Task DeleteAsync(string id, CancellationToken cancellationToken = default); + /// WriteResult with ID and completion status. + /// Thrown only if queueing the operation fails. + Task DeleteAsync(string id, CancellationToken cancellationToken = default); /// /// Retrieves content by ID. diff --git a/src/Core/Storage/Models/WriteResult.cs b/src/Core/Storage/Models/WriteResult.cs new file mode 100644 index 000000000..98a0d0526 --- /dev/null +++ b/src/Core/Storage/Models/WriteResult.cs @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft. All rights reserved. +namespace KernelMemory.Core.Storage.Models; + +/// +/// Result of a write operation (upsert or delete). +/// Never throws after queue succeeds - always returns this result. +/// +public sealed class WriteResult +{ + /// + /// The content ID (newly generated or existing). + /// Always populated when queue succeeds. + /// + public required string Id { get; init; } + + /// + /// True if all steps completed successfully. + /// False if operation is queued but not yet fully processed. + /// + public required bool Completed { get; init; } + + /// + /// True if operation was queued but not fully completed. + /// Inverse of Completed for convenience. + /// + public bool Queued => !this.Completed; + + /// + /// Error message if any step failed. + /// Empty string if no error occurred. + /// + public string Error { get; init; } = string.Empty; + + /// + /// Creates a successful result (all steps completed). + /// + /// The content ID. + public static WriteResult Success(string id) => new() + { + Id = id, + Completed = true, + Error = string.Empty + }; + + /// + /// Creates a queued result (operation accepted but not fully completed). + /// + /// The content ID. + /// Optional error message describing what failed. + public static WriteResult QueuedOnly(string id, string error = "") => new() + { + Id = id, + Completed = false, + Error = error + }; +} diff --git a/src/Main/CLI/CliApplicationBuilder.cs b/src/Main/CLI/CliApplicationBuilder.cs index 81ae4fc74..f5d2760d8 100644 --- a/src/Main/CLI/CliApplicationBuilder.cs +++ b/src/Main/CLI/CliApplicationBuilder.cs @@ -15,9 +15,9 @@ namespace KernelMemory.Main.CLI; public sealed class CliApplicationBuilder { // Static readonly arrays for command examples (CA1861 compliance) - private static readonly string[] s_upsertExample1 = new[] { "upsert", "\"Hello, world!\"" }; - private static readonly string[] s_upsertExample2 = new[] { "upsert", "\"Some content\"", "--id", "my-id-123" }; - private static readonly string[] s_upsertExample3 = new[] { "upsert", "\"Tagged content\"", "--tags", "important,todo" }; + private static readonly string[] s_upsertExample1 = new[] { "put", "\"Hello, world!\"" }; + private static readonly string[] s_upsertExample2 = new[] { "put", "\"Some content\"", "--id", "my-id-123" }; + private static readonly string[] s_upsertExample3 = new[] { "put", "\"Tagged content\"", "--tags", "important,todo" }; private static readonly string[] s_getExample1 = new[] { "get", "abc123" }; private static readonly string[] s_getExample2 = new[] { "get", "abc123", "--full" }; private static readonly string[] s_getExample3 = new[] { "get", "abc123", "-f", "json" }; @@ -31,6 +31,7 @@ public sealed class CliApplicationBuilder private static readonly string[] s_configExample1 = new[] { "config" }; private static readonly string[] s_configExample2 = new[] { "config", "--show-nodes" }; private static readonly string[] s_configExample3 = new[] { "config", "--show-cache" }; + private static readonly string[] s_configExample4 = new[] { "config", "--create" }; /// /// Creates and configures a CommandApp with all CLI commands. @@ -50,6 +51,9 @@ public CommandApp Build(string[]? args = null) var services = new ServiceCollection(); services.AddSingleton(config); + // Also register the config path so commands can access it + services.AddSingleton(new ConfigPathService(configPath)); + // 4. Create type registrar for Spectre.Console.Cli DI integration var registrar = new TypeRegistrar(services); @@ -94,8 +98,8 @@ public void Configure(CommandApp app) { config.SetApplicationName("km"); - // Upsert command - config.AddCommand("upsert") + // Put command (HTTP-style naming for upsert operation) + config.AddCommand("put") .WithDescription("Upload or update content") .WithExample(s_upsertExample1) .WithExample(s_upsertExample2) @@ -132,7 +136,8 @@ public void Configure(CommandApp app) .WithDescription("Query configuration") .WithExample(s_configExample1) .WithExample(s_configExample2) - .WithExample(s_configExample3); + .WithExample(s_configExample3) + .WithExample(s_configExample4); config.ValidateExamples(); }); diff --git a/src/Main/CLI/Commands/BaseCommand.cs b/src/Main/CLI/Commands/BaseCommand.cs index 6435c5341..3d5c56374 100644 --- a/src/Main/CLI/Commands/BaseCommand.cs +++ b/src/Main/CLI/Commands/BaseCommand.cs @@ -122,8 +122,16 @@ protected ContentService CreateContentService(NodeConfig node, bool readonlyMode var cuidGenerator = new CuidGenerator(); var logger = this.CreateLogger(); - // Create storage service - var storage = new ContentStorageService(context, cuidGenerator, logger); + // Create search indexes from node configuration + var loggerFactory = LoggerFactory.Create(builder => + { + builder.AddConsole(); + builder.SetMinimumLevel(LogLevel.Warning); + }); + var searchIndexes = Services.SearchIndexFactory.CreateIndexes(node.SearchIndexes, loggerFactory); + + // Create storage service with search indexes + var storage = new ContentStorageService(context, cuidGenerator, logger, searchIndexes); // Create and return content service return new ContentService(storage, node.Id); diff --git a/src/Main/CLI/Commands/ConfigCommand.cs b/src/Main/CLI/Commands/ConfigCommand.cs index 76e024b13..97e66705a 100644 --- a/src/Main/CLI/Commands/ConfigCommand.cs +++ b/src/Main/CLI/Commands/ConfigCommand.cs @@ -1,7 +1,10 @@ // Copyright (c) Microsoft. All rights reserved. using System.ComponentModel; using System.Diagnostics.CodeAnalysis; +using System.Text.Json; +using KernelMemory.Main.CLI.Infrastructure; using KernelMemory.Main.CLI.Models; +using Spectre.Console; using Spectre.Console.Cli; namespace KernelMemory.Main.CLI.Commands; @@ -18,6 +21,10 @@ public class ConfigCommandSettings : GlobalOptions [CommandOption("--show-cache")] [Description("Show cache configuration")] public bool ShowCache { get; init; } + + [CommandOption("--create")] + [Description("Create configuration file on disk")] + public bool Create { get; init; } } /// @@ -25,12 +32,25 @@ public class ConfigCommandSettings : GlobalOptions /// public class ConfigCommand : BaseCommand { + private static readonly JsonSerializerOptions s_jsonOptions = new() + { + WriteIndented = true, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + DefaultIgnoreCondition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull + }; + + private readonly ConfigPathService _configPathService; + /// /// Initializes a new instance of the class. /// /// Application configuration (injected by DI). - public ConfigCommand(KernelMemory.Core.Config.AppConfig config) : base(config) + /// Service providing the config file path (injected by DI). + public ConfigCommand( + KernelMemory.Core.Config.AppConfig config, + ConfigPathService configPathService) : base(config) { + this._configPathService = configPathService ?? throw new ArgumentNullException(nameof(configPathService)); } [SuppressMessage("Design", "CA1031:Do not catch general exception types", @@ -44,6 +64,22 @@ public override async Task ExecuteAsync( // ConfigCommand doesn't need node selection - it queries the entire configuration // So we skip Initialize() and just use the injected config directly var formatter = CLI.OutputFormatters.OutputFormatterFactory.Create(settings); + var configPath = this._configPathService.Path; + var configFileExists = File.Exists(configPath); + + // Handle --create flag + if (settings.Create) + { + return this.HandleCreateConfig(configPath, configFileExists, formatter); + } + + // Show warning if using default config without file + if (!configFileExists && !settings.NoColor) + { + AnsiConsole.MarkupLine("[yellow]Warning: Using default configuration. Config file does not exist: {0}[/]", Markup.Escape(configPath)); + AnsiConsole.MarkupLine("[yellow]Run 'km config --create' to save the current configuration to disk.[/]"); + AnsiConsole.WriteLine(); + } // Determine what to show object output; @@ -96,4 +132,48 @@ public override async Task ExecuteAsync( return this.HandleError(ex, formatter); } } + + /// + /// Handles the --create flag to write the configuration to disk. + /// + /// Path to write the config file. + /// Whether the config file already exists. + /// Output formatter for messages. + /// Exit code. + [SuppressMessage("Design", "CA1031:Do not catch general exception types", + Justification = "Config creation must catch all exceptions to return appropriate exit codes")] + private int HandleCreateConfig(string configPath, bool configFileExists, CLI.OutputFormatters.IOutputFormatter formatter) + { + try + { + // Warn if file already exists + if (configFileExists) + { + formatter.FormatError($"Configuration file already exists: {configPath}"); + return Constants.ExitCodeUserError; + } + + // Ensure directory exists + var configDir = Path.GetDirectoryName(configPath); + if (!string.IsNullOrEmpty(configDir) && !Directory.Exists(configDir)) + { + Directory.CreateDirectory(configDir); + } + + // Serialize config with nice formatting + var json = JsonSerializer.Serialize(this.Config, s_jsonOptions); + + // Write to file + File.WriteAllText(configPath, json); + + formatter.Format(new { Message = $"Configuration file created: {configPath}" }); + + return Constants.ExitCodeSuccess; + } + catch (Exception ex) + { + formatter.FormatError($"Failed to create configuration file: {ex.Message}"); + return Constants.ExitCodeSystemError; + } + } } diff --git a/src/Main/CLI/Commands/DeleteCommand.cs b/src/Main/CLI/Commands/DeleteCommand.cs index 91e6ee3f6..413b11f23 100644 --- a/src/Main/CLI/Commands/DeleteCommand.cs +++ b/src/Main/CLI/Commands/DeleteCommand.cs @@ -57,16 +57,22 @@ public override async Task ExecuteAsync( var service = this.CreateContentService(node); // Delete is idempotent - no error if not found - await service.DeleteAsync(settings.Id, CancellationToken.None).ConfigureAwait(false); + var result = await service.DeleteAsync(settings.Id, CancellationToken.None).ConfigureAwait(false); // Output result based on verbosity if (settings.Verbosity.Equals("quiet", StringComparison.OrdinalIgnoreCase)) { - formatter.Format(settings.Id); + formatter.Format(result.Id); } else if (!settings.Verbosity.Equals("silent", StringComparison.OrdinalIgnoreCase)) { - formatter.Format(new { id = settings.Id, status = "deleted" }); + formatter.Format(new + { + id = result.Id, + completed = result.Completed, + queued = result.Queued, + error = string.IsNullOrEmpty(result.Error) ? null : result.Error + }); } return Constants.ExitCodeSuccess; diff --git a/src/Main/CLI/Commands/GetCommand.cs b/src/Main/CLI/Commands/GetCommand.cs index 8b8c23496..420bf8fd6 100644 --- a/src/Main/CLI/Commands/GetCommand.cs +++ b/src/Main/CLI/Commands/GetCommand.cs @@ -110,7 +110,7 @@ private void ShowFirstRunMessage(GetCommandSettings settings) AnsiConsole.MarkupLine("[dim]No content database exists yet. This is your first run.[/]"); AnsiConsole.WriteLine(); AnsiConsole.MarkupLine("[bold]Create content first:[/]"); - AnsiConsole.MarkupLine($" [cyan]km upsert \"Your content here\" --id {settings.Id}[/]"); + AnsiConsole.MarkupLine($" [cyan]km put \"Your content here\" --id {settings.Id}[/]"); AnsiConsole.WriteLine(); AnsiConsole.MarkupLine("[bold]Or list available content:[/]"); AnsiConsole.MarkupLine(" [cyan]km list[/]"); diff --git a/src/Main/CLI/Commands/ListCommand.cs b/src/Main/CLI/Commands/ListCommand.cs index c2f4716e1..dec7cef99 100644 --- a/src/Main/CLI/Commands/ListCommand.cs +++ b/src/Main/CLI/Commands/ListCommand.cs @@ -114,10 +114,10 @@ private void ShowFirstRunMessage(ListCommandSettings settings) AnsiConsole.MarkupLine("[dim]No content found yet. This is your first run.[/]"); AnsiConsole.WriteLine(); AnsiConsole.MarkupLine("[bold]To get started:[/]"); - AnsiConsole.MarkupLine(" [cyan]km upsert \"Your content here\"[/]"); + AnsiConsole.MarkupLine(" [cyan]km put \"Your content here\"[/]"); AnsiConsole.WriteLine(); AnsiConsole.MarkupLine("[bold]Example:[/]"); - AnsiConsole.MarkupLine(" [cyan]km upsert \"Hello, world!\" --id greeting[/]"); + AnsiConsole.MarkupLine(" [cyan]km put \"Hello, world!\" --id greeting[/]"); AnsiConsole.WriteLine(); } } diff --git a/src/Main/CLI/Commands/UpsertCommand.cs b/src/Main/CLI/Commands/UpsertCommand.cs index 93951f9fa..b039a2c1d 100644 --- a/src/Main/CLI/Commands/UpsertCommand.cs +++ b/src/Main/CLI/Commands/UpsertCommand.cs @@ -96,16 +96,22 @@ public override async Task ExecuteAsync( }; // Perform upsert - var contentId = await service.UpsertAsync(request, CancellationToken.None).ConfigureAwait(false); + var result = await service.UpsertAsync(request, CancellationToken.None).ConfigureAwait(false); // Output result based on verbosity if (settings.Verbosity.Equals("quiet", StringComparison.OrdinalIgnoreCase)) { - formatter.Format(contentId); + formatter.Format(result.Id); } else { - formatter.Format(new { id = contentId, status = "success" }); + formatter.Format(new + { + id = result.Id, + completed = result.Completed, + queued = result.Queued, + error = string.IsNullOrEmpty(result.Error) ? null : result.Error + }); } return Constants.ExitCodeSuccess; diff --git a/src/Main/CLI/Infrastructure/ConfigPathService.cs b/src/Main/CLI/Infrastructure/ConfigPathService.cs new file mode 100644 index 000000000..af89522d4 --- /dev/null +++ b/src/Main/CLI/Infrastructure/ConfigPathService.cs @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace KernelMemory.Main.CLI.Infrastructure; + +/// +/// Service that holds the configuration file path. +/// Registered in DI to allow commands to access the config path. +/// +public sealed class ConfigPathService +{ + public ConfigPathService(string path) + { + this.Path = path; + } + + public string Path { get; } +} diff --git a/src/Main/Services/ContentService.cs b/src/Main/Services/ContentService.cs index d33fa2a5d..e46f928d4 100644 --- a/src/Main/Services/ContentService.cs +++ b/src/Main/Services/ContentService.cs @@ -30,12 +30,12 @@ public ContentService(IContentStorage storage, string nodeId) public string NodeId => this._nodeId; /// - /// Upserts content and returns the content ID. + /// Upserts content and returns the write result. /// /// The upsert request. /// Cancellation token. - /// The content ID. - public async Task UpsertAsync(UpsertRequest request, CancellationToken cancellationToken = default) + /// WriteResult with ID and completion status. + public async Task UpsertAsync(UpsertRequest request, CancellationToken cancellationToken = default) { return await this._storage.UpsertAsync(request, cancellationToken).ConfigureAwait(false); } @@ -56,9 +56,10 @@ public async Task UpsertAsync(UpsertRequest request, CancellationToken c /// /// The content ID. /// Cancellation token. - public async Task DeleteAsync(string id, CancellationToken cancellationToken = default) + /// WriteResult with ID and completion status. + public async Task DeleteAsync(string id, CancellationToken cancellationToken = default) { - await this._storage.DeleteAsync(id, cancellationToken).ConfigureAwait(false); + return await this._storage.DeleteAsync(id, cancellationToken).ConfigureAwait(false); } /// diff --git a/src/Main/Services/SearchIndexFactory.cs b/src/Main/Services/SearchIndexFactory.cs new file mode 100644 index 000000000..cdcc23d6e --- /dev/null +++ b/src/Main/Services/SearchIndexFactory.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft. All rights reserved. +using KernelMemory.Core.Config.SearchIndex; +using KernelMemory.Core.Search; +using Microsoft.Extensions.Logging; + +namespace KernelMemory.Main.Services; + +/// +/// Factory for creating search index instances from configuration. +/// +public static class SearchIndexFactory +{ + /// + /// Creates search index instances from node configuration. + /// + /// Search index configurations from node. + /// Logger factory for creating loggers. + /// Dictionary mapping index ID to ISearchIndex instance. + public static IReadOnlyDictionary CreateIndexes( + IReadOnlyList searchIndexConfigs, + ILoggerFactory loggerFactory) + { + var indexes = new Dictionary(); + + foreach (var config in searchIndexConfigs) + { + var index = CreateIndex(config, loggerFactory); + if (index != null) + { + indexes[config.Id] = index; + } + } + + return indexes; + } + + /// + /// Creates a single search index instance from configuration. + /// + /// Search index configuration. + /// Logger factory. + /// ISearchIndex instance, or null if type not supported. + private static SqliteFtsIndex? CreateIndex(SearchIndexConfig config, ILoggerFactory loggerFactory) + { + return config switch + { + FtsSearchIndexConfig ftsConfig when !string.IsNullOrWhiteSpace(ftsConfig.Path) => + new SqliteFtsIndex( + ftsConfig.Path, + ftsConfig.EnableStemming, + loggerFactory.CreateLogger()), + + // Vector and Graph indexes not yet implemented + // VectorSearchIndexConfig vectorConfig => ..., + // GraphSearchIndexConfig graphConfig => ..., + + _ => null + }; + } +} diff --git a/tests/Core.Tests/Config/AppConfigTests.cs b/tests/Core.Tests/Config/AppConfigTests.cs index bae7770c6..2f3ec61ae 100644 --- a/tests/Core.Tests/Config/AppConfigTests.cs +++ b/tests/Core.Tests/Config/AppConfigTests.cs @@ -23,7 +23,8 @@ public void CreateDefault_ShouldCreateValidConfiguration() Assert.NotNull(config); Assert.Single(config.Nodes); Assert.True(config.Nodes.ContainsKey("personal")); - Assert.NotNull(config.EmbeddingsCache); + // Cache configs intentionally null - only created when features are implemented + Assert.Null(config.EmbeddingsCache); Assert.Null(config.LLMCache); // Verify personal node structure @@ -34,31 +35,16 @@ public void CreateDefault_ShouldCreateValidConfiguration() Assert.IsType(personalNode.ContentIndex); Assert.Null(personalNode.FileStorage); Assert.Null(personalNode.RepoStorage); - Assert.Equal(2, personalNode.SearchIndexes.Count); + Assert.Single(personalNode.SearchIndexes); - // Verify search indexes + // Verify search indexes (only FTS for now - vectors not yet implemented) Assert.IsType(personalNode.SearchIndexes[0]); - Assert.IsType(personalNode.SearchIndexes[1]); var ftsIndex = (FtsSearchIndexConfig)personalNode.SearchIndexes[0]; Assert.Equal(SearchIndexTypes.SqliteFTS, ftsIndex.Type); Assert.True(ftsIndex.EnableStemming); Assert.NotNull(ftsIndex.Path); Assert.Contains("fts.db", ftsIndex.Path); - - var vectorIndex = (VectorSearchIndexConfig)personalNode.SearchIndexes[1]; - Assert.Equal(SearchIndexTypes.SqliteVector, vectorIndex.Type); - Assert.Equal(768, vectorIndex.Dimensions); - Assert.Equal(VectorMetrics.Cosine, vectorIndex.Metric); - Assert.NotNull(vectorIndex.Path); - Assert.Contains("vectors.db", vectorIndex.Path); - - // Verify embeddings cache - Assert.Equal(CacheTypes.Sqlite, config.EmbeddingsCache.Type); - Assert.True(config.EmbeddingsCache.AllowRead); - Assert.True(config.EmbeddingsCache.AllowWrite); - Assert.NotNull(config.EmbeddingsCache.Path); - Assert.Contains("embeddings-cache.db", config.EmbeddingsCache.Path); } [Fact] diff --git a/tests/Core.Tests/Config/ConfigParserAutoCreateTests.cs b/tests/Core.Tests/Config/ConfigParserAutoCreateTests.cs new file mode 100644 index 000000000..ee348b553 --- /dev/null +++ b/tests/Core.Tests/Config/ConfigParserAutoCreateTests.cs @@ -0,0 +1,203 @@ +// Copyright (c) Microsoft. All rights reserved. + +using KernelMemory.Core.Config; +using KernelMemory.Core.Config.ContentIndex; +using KernelMemory.Core.Config.SearchIndex; + +namespace KernelMemory.Core.Tests.Config; + +/// +/// Tests that ConfigParser automatically creates config files when they don't exist +/// +public sealed class ConfigParserAutoCreateTests : IDisposable +{ + private readonly string _tempDir; + + public ConfigParserAutoCreateTests() + { + this._tempDir = Path.Combine(Path.GetTempPath(), $"km-autoconfig-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(this._tempDir); + } + + public void Dispose() + { + try + { + if (Directory.Exists(this._tempDir)) + { + Directory.Delete(this._tempDir, recursive: true); + } + } + catch (IOException) + { + // Best effort cleanup - ignore IO errors during test cleanup + } + catch (UnauthorizedAccessException) + { + // Best effort cleanup - ignore permission errors during test cleanup + } + } + + [Fact] + public void LoadFromFile_WhenFileDoesNotExist_CreatesConfigFile() + { + // Arrange + var configPath = Path.Combine(this._tempDir, "config.json"); + Assert.False(File.Exists(configPath), "Config file should not exist before test"); + + // Act + var config = ConfigParser.LoadFromFile(configPath); + + // Assert + Assert.True(File.Exists(configPath), "Config file should be created"); + Assert.NotNull(config); + Assert.NotEmpty(config.Nodes); + } + + [Fact] + public void LoadFromFile_WhenFileDoesNotExist_ConfigHasPathsRelativeToConfigFile() + { + // Arrange + var configPath = Path.Combine(this._tempDir, "config.json"); + + // Act + var config = ConfigParser.LoadFromFile(configPath); + + // Assert - paths should be relative to config directory + var personalNode = config.Nodes["personal"]; + + // Content index should be SQLite + var sqliteContent = Assert.IsType(personalNode.ContentIndex); + Assert.Contains(this._tempDir, sqliteContent.Path); + + // Should follow structure: {tempDir}/nodes/personal/content.db + var expectedContentPath = Path.Combine(this._tempDir, "nodes", "personal", "content.db"); + Assert.Equal(expectedContentPath, sqliteContent.Path); + + // FTS index path should also be under temp dir + var ftsIndex = Assert.IsType(personalNode.SearchIndexes.First()); + Assert.Contains(this._tempDir, ftsIndex.Path!); + + var expectedFtsPath = Path.Combine(this._tempDir, "nodes", "personal", "fts.db"); + Assert.Equal(expectedFtsPath, ftsIndex.Path); + } + + [Fact] + public void LoadFromFile_WhenFileDoesNotExist_CreatedConfigIsValid() + { + // Arrange + var configPath = Path.Combine(this._tempDir, "config.json"); + + // Act + var config = ConfigParser.LoadFromFile(configPath); + + // Assert - should not throw on validation + config.Validate(); + + // Verify structure + Assert.Single(config.Nodes); + Assert.True(config.Nodes.ContainsKey("personal")); + // Cache configs intentionally null - only created when features are implemented + Assert.Null(config.EmbeddingsCache); + Assert.Null(config.LLMCache); + } + + [Fact] + public void LoadFromFile_WhenFileDoesNotExist_CreatedJsonIsValidAndParseable() + { + // Arrange + var configPath = Path.Combine(this._tempDir, "config.json"); + + // Act + var config1 = ConfigParser.LoadFromFile(configPath); + + // Read it back and parse again + var config2 = ConfigParser.LoadFromFile(configPath); + + // Assert - should be able to reload the created config + Assert.NotNull(config2); + Assert.Equal(config1.Nodes.Count, config2.Nodes.Count); + Assert.Equal(config1.Nodes["personal"].Id, config2.Nodes["personal"].Id); + } + + [Fact] + public void LoadFromFile_WhenFileDoesNotExist_CreatedJsonHasNoNullFields() + { + // Arrange + var configPath = Path.Combine(this._tempDir, "config.json"); + + // Act + ConfigParser.LoadFromFile(configPath); + var json = File.ReadAllText(configPath); + + // Assert - no null fields should be serialized + Assert.DoesNotContain("\"connectionString\": null", json); + Assert.DoesNotContain("\"embeddings\": null", json); + Assert.DoesNotContain("\"fileStorage\": null", json); + Assert.DoesNotContain("\"repoStorage\": null", json); + Assert.DoesNotContain("\"llmCache\": null", json); + } + + [Fact] + public void LoadFromFile_WhenDirectoryDoesNotExist_CreatesDirectory() + { + // Arrange + var subDir = Path.Combine(this._tempDir, "nested", "deep"); + var configPath = Path.Combine(subDir, "config.json"); + Assert.False(Directory.Exists(subDir), "Directory should not exist before test"); + + // Act + var config = ConfigParser.LoadFromFile(configPath); + + // Assert + Assert.True(Directory.Exists(subDir), "Directory should be created"); + Assert.True(File.Exists(configPath), "Config file should be created"); + Assert.NotNull(config); + } + [Fact] + public void LoadFromFile_WhenConfigDeletedBetweenLoads_RecreatesFile() + { + // Arrange + var configPath = Path.Combine(this._tempDir, "config.json"); + + // First load - creates config + var config1 = ConfigParser.LoadFromFile(configPath); + Assert.True(File.Exists(configPath), "Config should exist after first load"); + + // Delete the config file (simulating accidental deletion) + File.Delete(configPath); + Assert.False(File.Exists(configPath), "Config should be deleted"); + + // Act - Second load should recreate the file + var config2 = ConfigParser.LoadFromFile(configPath); + + // Assert + Assert.True(File.Exists(configPath), "Config should be recreated on second load"); + Assert.NotNull(config2); + Assert.Equal(config1.Nodes.Count, config2.Nodes.Count); + } + + [Fact] + public void LoadFromFile_WhenConfigDeletedAfterManyWrites_RecreatesFile() + { + // Arrange + var configPath = Path.Combine(this._tempDir, "config.json"); + + // Simulate many operations + for (int i = 0; i < 10; i++) + { + var config = ConfigParser.LoadFromFile(configPath); + Assert.True(File.Exists(configPath), $"Config should exist after load {i}"); + + // Simulate accidental deletion in the middle + if (i == 5) + { + File.Delete(configPath); + Assert.False(File.Exists(configPath), "Config should be deleted"); + } + } + + // Assert - config should still exist after all operations + Assert.True(File.Exists(configPath), "Config should exist after all operations"); + } +} diff --git a/tests/Core.Tests/Config/ConfigParserTests.cs b/tests/Core.Tests/Config/ConfigParserTests.cs index 42086034d..afaec9b1f 100644 --- a/tests/Core.Tests/Config/ConfigParserTests.cs +++ b/tests/Core.Tests/Config/ConfigParserTests.cs @@ -22,7 +22,8 @@ public void LoadFromFile_WhenFileMissing_ShouldReturnDefaultConfig() Assert.NotNull(config); Assert.Single(config.Nodes); Assert.True(config.Nodes.ContainsKey("personal")); - Assert.NotNull(config.EmbeddingsCache); + // Cache configs intentionally null in default config + Assert.Null(config.EmbeddingsCache); } [Fact] @@ -36,7 +37,7 @@ public void LoadFromFile_WithValidJson_ShouldReturnParsedConfig() ""id"": ""mynode"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" } } @@ -127,7 +128,7 @@ public void LoadFromFile_WithTildeInPath_ShouldExpandToHomeDirectory() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""~/.km/test.db"" } } @@ -170,7 +171,7 @@ public void LoadFromFile_WithCommentsInJson_ShouldParseSuccessfully() comment */ ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" } } @@ -209,7 +210,7 @@ public void LoadFromFile_WithCaseInsensitiveProperties_ShouldParseSuccessfully() ""Id"": ""test"", ""Access"": ""Full"", ""ContentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""Path"": ""test.db"" } } @@ -247,7 +248,7 @@ public void ParseFromString_WithValidJson_ShouldReturnParsedConfig() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" } } @@ -285,7 +286,7 @@ public void LoadFromFile_WithCacheTildeExpansion_ShouldExpandPaths() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" } } @@ -307,7 +308,7 @@ public void LoadFromFile_WithCacheTildeExpansion_ShouldExpandPaths() // Act var config = ConfigParser.LoadFromFile(tempFile); - // Assert + // Assert - verify tilde expansion works for cache configs when present var homeDir = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); Assert.NotNull(config.EmbeddingsCache); Assert.NotNull(config.LLMCache); @@ -334,7 +335,7 @@ public void LoadFromFile_WithCacheBothPathAndConnectionString_ShouldThrowConfigE ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" } } diff --git a/tests/Core.Tests/Config/ContentIndexConfigTests.cs b/tests/Core.Tests/Config/ContentIndexConfigTests.cs index a7c6cda6c..371ce7838 100644 --- a/tests/Core.Tests/Config/ContentIndexConfigTests.cs +++ b/tests/Core.Tests/Config/ContentIndexConfigTests.cs @@ -20,7 +20,7 @@ public void LoadFromFile_WithPostgresContentIndex_ShouldValidate() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""postgres"", + ""type"": ""postgres"", ""connectionString"": ""Host=localhost;Database=testdb;Username=test;Password=test"" } } @@ -60,7 +60,7 @@ public void LoadFromFile_WithPostgresContentIndexMissingConnectionString_ShouldT ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""postgres"", + ""type"": ""postgres"", ""connectionString"": """" } } diff --git a/tests/Core.Tests/Config/EmbeddingsConfigTests.cs b/tests/Core.Tests/Config/EmbeddingsConfigTests.cs index 04347b439..d014be6b1 100644 --- a/tests/Core.Tests/Config/EmbeddingsConfigTests.cs +++ b/tests/Core.Tests/Config/EmbeddingsConfigTests.cs @@ -20,17 +20,17 @@ public void LoadFromFile_WithOllamaEmbeddings_ShouldValidate() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""dimensions"": 384, ""embeddings"": { - ""$type"": ""ollama"", + ""type"": ""ollama"", ""model"": ""all-minilm"", ""baseUrl"": ""http://localhost:11434"" } @@ -75,17 +75,17 @@ public void LoadFromFile_WithOpenAIEmbeddings_ShouldValidate() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""dimensions"": 1536, ""embeddings"": { - ""$type"": ""openai"", + ""type"": ""openai"", ""model"": ""text-embedding-ada-002"", ""apiKey"": ""test-key"", ""baseUrl"": ""https://api.openai.com/v1"" @@ -131,17 +131,17 @@ public void LoadFromFile_WithOllamaEmbeddingsMissingModel_ShouldThrowConfigExcep ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""dimensions"": 384, ""embeddings"": { - ""$type"": ""ollama"", + ""type"": ""ollama"", ""model"": """", ""baseUrl"": ""http://localhost:11434"" } @@ -179,17 +179,17 @@ public void LoadFromFile_WithOllamaEmbeddingsMissingBaseUrl_ShouldThrowConfigExc ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""dimensions"": 384, ""embeddings"": { - ""$type"": ""ollama"", + ""type"": ""ollama"", ""model"": ""all-minilm"", ""baseUrl"": """" } @@ -227,17 +227,17 @@ public void LoadFromFile_WithOpenAIEmbeddingsMissingApiKey_ShouldThrowConfigExce ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""dimensions"": 1536, ""embeddings"": { - ""$type"": ""openai"", + ""type"": ""openai"", ""model"": ""text-embedding-ada-002"", ""apiKey"": """" } @@ -275,17 +275,17 @@ public void LoadFromFile_WithAzureOpenAIEmbeddingsMissingModel_ShouldThrowConfig ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""dimensions"": 1536, ""embeddings"": { - ""$type"": ""azureOpenAI"", + ""type"": ""azureOpenAI"", ""model"": """", ""endpoint"": ""https://test.openai.azure.com"", ""deployment"": ""test-deployment"", @@ -325,17 +325,17 @@ public void LoadFromFile_WithAzureOpenAIEmbeddingsMissingEndpoint_ShouldThrowCon ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""dimensions"": 1536, ""embeddings"": { - ""$type"": ""azureOpenAI"", + ""type"": ""azureOpenAI"", ""model"": ""text-embedding-ada-002"", ""endpoint"": """", ""deployment"": ""test-deployment"", @@ -375,17 +375,17 @@ public void LoadFromFile_WithAzureOpenAIEmbeddingsMissingDeployment_ShouldThrowC ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""dimensions"": 1536, ""embeddings"": { - ""$type"": ""azureOpenAI"", + ""type"": ""azureOpenAI"", ""model"": ""text-embedding-ada-002"", ""endpoint"": ""https://test.openai.azure.com"", ""deployment"": """", @@ -425,17 +425,17 @@ public void LoadFromFile_WithAzureOpenAIEmbeddingsNoAuth_ShouldThrowConfigExcept ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""dimensions"": 1536, ""embeddings"": { - ""$type"": ""azureOpenAI"", + ""type"": ""azureOpenAI"", ""model"": ""text-embedding-ada-002"", ""endpoint"": ""https://test.openai.azure.com"", ""deployment"": ""test-deployment"" @@ -474,17 +474,17 @@ public void LoadFromFile_WithAzureOpenAIEmbeddings_ShouldValidate() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""dimensions"": 1536, ""embeddings"": { - ""$type"": ""azureOpenAI"", + ""type"": ""azureOpenAI"", ""model"": ""text-embedding-ada-002"", ""endpoint"": ""https://test.openai.azure.com"", ""deployment"": ""text-embedding-ada-002"", diff --git a/tests/Core.Tests/Config/SearchIndexConfigTests.cs b/tests/Core.Tests/Config/SearchIndexConfigTests.cs index 3ba40651b..3842fcfd6 100644 --- a/tests/Core.Tests/Config/SearchIndexConfigTests.cs +++ b/tests/Core.Tests/Config/SearchIndexConfigTests.cs @@ -20,12 +20,13 @@ public void LoadFromFile_WithGraphSearchIndex_ShouldExpandTildePath() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""graph"", + ""type"": ""graph"", + ""id"": ""graph-test"", ""path"": ""~/graph-index.db"" } ] @@ -69,13 +70,13 @@ public void LoadFromFile_WithFtsSearchIndexBothPathAndConnection_ShouldThrowConf ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""fts"", - ""type"": ""SqliteFTS"", + ""type"": ""sqliteFTS"", + ""id"": ""fts-test"", ""path"": ""fts.db"", ""connectionString"": ""Host=localhost"" } @@ -88,7 +89,7 @@ public void LoadFromFile_WithFtsSearchIndexBothPathAndConnection_ShouldThrowConf { File.WriteAllText(tempFile, json); var exception = Assert.Throws(() => ConfigParser.LoadFromFile(tempFile)); - Assert.Contains("specify either Path", exception.Message); + Assert.Contains("FTS index: specify either Path (SQLite) or ConnectionString (Postgres), not both", exception.Message); } finally { @@ -110,13 +111,13 @@ public void LoadFromFile_WithVectorSearchIndexBothPathAndConnection_ShouldThrowC ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-test"", ""path"": ""vector.db"", ""connectionString"": ""Host=localhost"", ""dimensions"": 384 @@ -130,7 +131,7 @@ public void LoadFromFile_WithVectorSearchIndexBothPathAndConnection_ShouldThrowC { File.WriteAllText(tempFile, json); var exception = Assert.Throws(() => ConfigParser.LoadFromFile(tempFile)); - Assert.Contains("specify either Path", exception.Message); + Assert.Contains("Vector index: specify either Path (SQLite) or ConnectionString (Postgres), not both", exception.Message); } finally { @@ -152,13 +153,13 @@ public void LoadFromFile_WithVectorSearchIndexInvalidDimensions_ShouldThrowConfi ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""searchIndexes"": [ { - ""$type"": ""vector"", - ""type"": ""SqliteVector"", + ""type"": ""sqliteVector"", + ""id"": ""vector-invalid"", ""path"": ""vector.db"", ""dimensions"": 0 } @@ -171,7 +172,7 @@ public void LoadFromFile_WithVectorSearchIndexInvalidDimensions_ShouldThrowConfi { File.WriteAllText(tempFile, json); var exception = Assert.Throws(() => ConfigParser.LoadFromFile(tempFile)); - Assert.Contains("must be positive", exception.Message); + Assert.Contains("Vector dimensions must be positive (got 0)", exception.Message); } finally { diff --git a/tests/Core.Tests/Config/StorageConfigTests.cs b/tests/Core.Tests/Config/StorageConfigTests.cs index eb78220ed..3e751a702 100644 --- a/tests/Core.Tests/Config/StorageConfigTests.cs +++ b/tests/Core.Tests/Config/StorageConfigTests.cs @@ -20,15 +20,15 @@ public void LoadFromFile_WithDiskStorage_ShouldExpandTildePath() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""fileStorage"": { - ""$type"": ""disk"", + ""type"": ""disk"", ""path"": ""~/test-files"" }, ""repoStorage"": { - ""$type"": ""disk"", + ""type"": ""disk"", ""path"": ""~/test-repos"" } } @@ -74,11 +74,11 @@ public void LoadFromFile_WithDiskStorageMissingPath_ShouldThrowConfigException() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""fileStorage"": { - ""$type"": ""disk"", + ""type"": ""disk"", ""path"": """" } } @@ -113,11 +113,11 @@ public void LoadFromFile_WithAzureBlobStorageConnectionString_ShouldValidate() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""fileStorage"": { - ""$type"": ""azureBlobs"", + ""type"": ""azureBlobs"", ""connectionString"": ""DefaultEndpointsProtocol=https;AccountName=test;AccountKey=key;EndpointSuffix=core.windows.net"" } } @@ -155,11 +155,11 @@ public void LoadFromFile_WithAzureBlobStorageApiKey_ShouldValidate() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""fileStorage"": { - ""$type"": ""azureBlobs"", + ""type"": ""azureBlobs"", ""account"": ""testaccount"", ""apiKey"": ""test-api-key"" } @@ -198,11 +198,11 @@ public void LoadFromFile_WithAzureBlobStorageNoAuth_ShouldThrowConfigException() ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""fileStorage"": { - ""$type"": ""azureBlobs"", + ""type"": ""azureBlobs"", ""account"": ""testaccount"" } } @@ -237,11 +237,11 @@ public void LoadFromFile_WithAzureBlobStorageMultipleAuth_ShouldThrowConfigExcep ""id"": ""test"", ""access"": ""Full"", ""contentIndex"": { - ""$type"": ""sqlite"", + ""type"": ""sqlite"", ""path"": ""test.db"" }, ""fileStorage"": { - ""$type"": ""azureBlobs"", + ""type"": ""azureBlobs"", ""connectionString"": ""DefaultEndpointsProtocol=https;AccountName=test;AccountKey=key"", ""apiKey"": ""test-api-key"" } diff --git a/tests/Core.Tests/Search/FtsIntegrationTests.cs b/tests/Core.Tests/Search/FtsIntegrationTests.cs new file mode 100644 index 000000000..bfa656540 --- /dev/null +++ b/tests/Core.Tests/Search/FtsIntegrationTests.cs @@ -0,0 +1,264 @@ +// Copyright (c) Microsoft. All rights reserved. +using KernelMemory.Core.Search; +using KernelMemory.Core.Storage; +using KernelMemory.Core.Storage.Models; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; +using Moq; + +namespace KernelMemory.Core.Tests.Search; + +/// +/// Integration tests for FTS with ContentStorageService. +/// Tests the full pipeline: upsert content -> FTS indexed -> search returns results. +/// +public sealed class FtsIntegrationTests : IDisposable +{ + private readonly SqliteConnection _contentConnection; + private readonly ContentStorageDbContext _context; + private readonly Mock _mockCuidGenerator; + private readonly Mock> _mockStorageLogger; + private readonly Mock> _mockFtsLogger; + private readonly string _ftsDbPath; + private readonly SqliteFtsIndex _ftsIndex; + private readonly ContentStorageService _service; + private int _cuidCounter; + + public FtsIntegrationTests() + { + // Content storage - in-memory SQLite + this._contentConnection = new SqliteConnection("DataSource=:memory:"); + this._contentConnection.Open(); + + var options = new DbContextOptionsBuilder() + .UseSqlite(this._contentConnection) + .Options; + + this._context = new ContentStorageDbContext(options); + this._context.Database.EnsureCreated(); + + // FTS index - temp file (FTS5 needs persistent storage for some operations) + this._ftsDbPath = Path.Combine(Path.GetTempPath(), $"fts_integration_{Guid.NewGuid()}.db"); + this._mockFtsLogger = new Mock>(); + this._ftsIndex = new SqliteFtsIndex(this._ftsDbPath, enableStemming: true, this._mockFtsLogger.Object); + + // Mock CUID generator with predictable IDs + this._mockCuidGenerator = new Mock(); + this._cuidCounter = 0; + this._mockCuidGenerator + .Setup(x => x.Generate()) + .Returns(() => $"fts_test_id_{++this._cuidCounter:D5}"); + + this._mockStorageLogger = new Mock>(); + + // Create service with FTS index integrated + var searchIndexById = new Dictionary + { + ["sqlite-fts"] = this._ftsIndex + }; + this._service = new ContentStorageService( + this._context, + this._mockCuidGenerator.Object, + this._mockStorageLogger.Object, + searchIndexById); + } + + public void Dispose() + { + this._ftsIndex.Dispose(); + this._context.Dispose(); + this._contentConnection.Dispose(); + + if (File.Exists(this._ftsDbPath)) + { + File.Delete(this._ftsDbPath); + } + + GC.SuppressFinalize(this); + } + + [Fact] + public async Task UpsertAsync_WithFtsIndex_IndexesContentAutomatically() + { + // Arrange + var request = new UpsertRequest + { + Content = "This document is about machine learning and artificial intelligence.", + MimeType = "text/plain", + Title = "ML Document" + }; + + // Act + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); + await Task.Delay(100).ConfigureAwait(false); // Allow processing + + // Assert - WriteResult should indicate completion + Assert.True(result.Completed); + Assert.Equal("fts_test_id_00001", result.Id); + + // Verify FTS index contains the content + var searchResults = await this._ftsIndex.SearchAsync("machine learning").ConfigureAwait(false); + Assert.Single(searchResults); + Assert.Equal(result.Id, searchResults[0].ContentId); + } + + [Fact] + public async Task UpsertAsync_WithFtsIndex_IncludesFtsStep() + { + // Arrange + var request = new UpsertRequest + { + Content = "Test content for FTS step verification", + MimeType = "text/plain" + }; + + // Act + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); + + // Assert - Check that operation includes index step (index:0 for first search index) + var operation = await this._context.Operations + .FirstOrDefaultAsync(o => o.ContentId == result.Id) + .ConfigureAwait(false); + + Assert.NotNull(operation); + Assert.Contains("upsert", operation.PlannedSteps); + Assert.Contains("index:sqlite-fts", operation.PlannedSteps); // SQLite FTS index + } + + [Fact] + public async Task DeleteAsync_WithFtsIndex_RemovesFromFtsIndex() + { + // Arrange - Create content first + var request = new UpsertRequest + { + Id = "delete_fts_test", + Content = "Content to be deleted from FTS index", + MimeType = "text/plain" + }; + await this._service.UpsertAsync(request).ConfigureAwait(false); + await Task.Delay(100).ConfigureAwait(false); + + // Verify it's in FTS + var beforeDelete = await this._ftsIndex.SearchAsync("deleted").ConfigureAwait(false); + Assert.Single(beforeDelete); + + // Act - Delete the content + var result = await this._service.DeleteAsync("delete_fts_test").ConfigureAwait(false); + await Task.Delay(100).ConfigureAwait(false); + + // Assert + Assert.True(result.Completed); + Assert.Equal("delete_fts_test", result.Id); + + // Verify removed from FTS + var afterDelete = await this._ftsIndex.SearchAsync("deleted").ConfigureAwait(false); + Assert.Empty(afterDelete); + } + + [Fact] + public async Task DeleteAsync_WithFtsIndex_IncludesFtsDeleteStep() + { + // Arrange + const string contentId = "fts_delete_step_test"; + + // Act + var result = await this._service.DeleteAsync(contentId).ConfigureAwait(false); + + // Assert - Check that operation includes index delete step (index:0:delete for first search index) + var operation = await this._context.Operations + .FirstOrDefaultAsync(o => o.ContentId == contentId) + .ConfigureAwait(false); + + Assert.NotNull(operation); + Assert.Contains("delete", operation.PlannedSteps); + Assert.Contains("index:sqlite-fts:delete", operation.PlannedSteps); // SQLite FTS index + } + + [Fact] + public async Task UpsertAsync_MultipleDocuments_AllSearchable() + { + // Arrange & Act + var requests = new[] + { + new UpsertRequest { Content = "Python programming language", MimeType = "text/plain" }, + new UpsertRequest { Content = "JavaScript web development", MimeType = "text/plain" }, + new UpsertRequest { Content = "C# dotnet programming", MimeType = "text/plain" } + }; + + var ids = new List(); + foreach (var request in requests) + { + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); + ids.Add(result.Id); + await Task.Delay(50).ConfigureAwait(false); + } + + await Task.Delay(200).ConfigureAwait(false); // Allow all to process + + // Assert - Search for "programming" should find 2 documents + var searchResults = await this._ftsIndex.SearchAsync("programming").ConfigureAwait(false); + Assert.Equal(2, searchResults.Count); + Assert.Contains(searchResults, r => r.ContentId == ids[0]); // Python + Assert.Contains(searchResults, r => r.ContentId == ids[2]); // C# + } + + [Fact] + public async Task UpsertAsync_UpdateContent_FtsIndexUpdated() + { + // Arrange - Create initial content + var initialRequest = new UpsertRequest + { + Id = "update_fts_test", + Content = "Original content about cats", + MimeType = "text/plain" + }; + await this._service.UpsertAsync(initialRequest).ConfigureAwait(false); + await Task.Delay(100).ConfigureAwait(false); + + // Verify initial indexing + var catResults = await this._ftsIndex.SearchAsync("cats").ConfigureAwait(false); + Assert.Single(catResults); + + // Act - Update content + var updateRequest = new UpsertRequest + { + Id = "update_fts_test", + Content = "Updated content about dogs", + MimeType = "text/plain" + }; + await this._service.UpsertAsync(updateRequest).ConfigureAwait(false); + await Task.Delay(100).ConfigureAwait(false); + + // Assert - Old content not searchable + var catResultsAfter = await this._ftsIndex.SearchAsync("cats").ConfigureAwait(false); + Assert.Empty(catResultsAfter); + + // New content is searchable + var dogResults = await this._ftsIndex.SearchAsync("dogs").ConfigureAwait(false); + Assert.Single(dogResults); + Assert.Equal("update_fts_test", dogResults[0].ContentId); + } + + [Fact] + public async Task WriteResult_ReturnsQueuedOnlyWhenFtsFails() + { + // This test verifies the response contract when FTS step fails. + // We can't easily force FTS to fail, but we verify the normal success path. + + // Arrange + var request = new UpsertRequest + { + Content = "Content for success verification", + MimeType = "text/plain" + }; + + // Act + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); + + // Assert - Should complete successfully + Assert.True(result.Completed); + Assert.False(result.Queued); + Assert.Empty(result.Error); + } +} diff --git a/tests/Core.Tests/Search/SqliteFtsIndexTests.cs b/tests/Core.Tests/Search/SqliteFtsIndexTests.cs new file mode 100644 index 000000000..8b72e8320 --- /dev/null +++ b/tests/Core.Tests/Search/SqliteFtsIndexTests.cs @@ -0,0 +1,310 @@ +// Copyright (c) Microsoft. All rights reserved. +using KernelMemory.Core.Search; +using Microsoft.Extensions.Logging; +using Moq; + +namespace KernelMemory.Core.Tests.Search; + +/// +/// Unit tests for SqliteFtsIndex using in-memory SQLite database. +/// Tests cover indexing, searching, and removal operations. +/// +public sealed class SqliteFtsIndexTests : IDisposable +{ + private readonly string _dbPath; + private readonly Mock> _mockLogger; + private readonly SqliteFtsIndex _ftsIndex; + + public SqliteFtsIndexTests() + { + // Use temp file for SQLite (FTS5 doesn't work well with :memory: across connections) + this._dbPath = Path.Combine(Path.GetTempPath(), $"fts_test_{Guid.NewGuid()}.db"); + this._mockLogger = new Mock>(); + this._ftsIndex = new SqliteFtsIndex(this._dbPath, enableStemming: true, this._mockLogger.Object); + } + + public void Dispose() + { + this._ftsIndex.Dispose(); + + // Clean up temp file + if (File.Exists(this._dbPath)) + { + File.Delete(this._dbPath); + } + + GC.SuppressFinalize(this); + } + + [Fact] + public async Task IndexAsync_IndexesContentSuccessfully() + { + // Arrange + const string contentId = "test-id-1"; + const string text = "This is a test document for full text search."; + + // Act + await this._ftsIndex.IndexAsync(contentId, text).ConfigureAwait(false); + + // Assert - Search should find it + var results = await this._ftsIndex.SearchAsync("test document").ConfigureAwait(false); + Assert.Single(results); + Assert.Equal(contentId, results[0].ContentId); + } + + [Fact] + public async Task IndexAsync_ReplacesExistingContent() + { + // Arrange + const string contentId = "test-id-replace"; + await this._ftsIndex.IndexAsync(contentId, "original content about cats").ConfigureAwait(false); + + // Act - Replace with new content + await this._ftsIndex.IndexAsync(contentId, "updated content about dogs").ConfigureAwait(false); + + // Assert - Old content should not be found + var catResults = await this._ftsIndex.SearchAsync("cats").ConfigureAwait(false); + Assert.Empty(catResults); + + // New content should be found + var dogResults = await this._ftsIndex.SearchAsync("dogs").ConfigureAwait(false); + Assert.Single(dogResults); + Assert.Equal(contentId, dogResults[0].ContentId); + } + + [Fact] + public async Task SearchAsync_ReturnsEmptyForNoMatches() + { + // Arrange + await this._ftsIndex.IndexAsync("id1", "content about programming").ConfigureAwait(false); + + // Act + var results = await this._ftsIndex.SearchAsync("nonexistent xyz123").ConfigureAwait(false); + + // Assert + Assert.Empty(results); + } + + [Fact] + public async Task SearchAsync_ReturnsEmptyForEmptyQuery() + { + // Arrange + await this._ftsIndex.IndexAsync("id1", "some content").ConfigureAwait(false); + + // Act + var results = await this._ftsIndex.SearchAsync("").ConfigureAwait(false); + + // Assert + Assert.Empty(results); + } + + [Fact] + public async Task SearchAsync_ReturnsMultipleMatches() + { + // Arrange + await this._ftsIndex.IndexAsync("id1", "The quick brown fox jumps").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("id2", "A quick rabbit runs fast").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("id3", "Slow turtle walks slowly").ConfigureAwait(false); + + // Act + var results = await this._ftsIndex.SearchAsync("quick").ConfigureAwait(false); + + // Assert + Assert.Equal(2, results.Count); + Assert.Contains(results, r => r.ContentId == "id1"); + Assert.Contains(results, r => r.ContentId == "id2"); + } + + [Fact] + public async Task SearchAsync_RespectsLimit() + { + // Arrange - Create many documents + for (int i = 0; i < 20; i++) + { + await this._ftsIndex.IndexAsync($"id{i}", $"Document number {i} with common word test").ConfigureAwait(false); + } + + // Act + var results = await this._ftsIndex.SearchAsync("test", limit: 5).ConfigureAwait(false); + + // Assert + Assert.Equal(5, results.Count); + } + + [Fact] + public async Task SearchAsync_OrdersByRelevance() + { + // Arrange - Create documents with different relevance + await this._ftsIndex.IndexAsync("low", "This document mentions search once.").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("high", "Search search search - this document is all about search optimization for search engines.").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("medium", "Search results and search queries are important for search.").ConfigureAwait(false); + + // Act + var results = await this._ftsIndex.SearchAsync("search").ConfigureAwait(false); + + // Assert - Higher relevance (more occurrences) should come first + Assert.Equal(3, results.Count); + Assert.Equal("high", results[0].ContentId); // Most occurrences + } + + [Fact] + public async Task SearchAsync_ReturnsSnippets() + { + // Arrange + const string longText = "This is a very long document. It contains many words and sentences. " + + "The important keyword appears here. More text follows after the keyword. " + + "And even more text to make this document quite lengthy."; + await this._ftsIndex.IndexAsync("id1", longText).ConfigureAwait(false); + + // Act + var results = await this._ftsIndex.SearchAsync("keyword").ConfigureAwait(false); + + // Assert + Assert.Single(results); + Assert.NotEmpty(results[0].Snippet); + Assert.Contains("keyword", results[0].Snippet, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task SearchAsync_WithStemming_FindsRelatedWords() + { + // Arrange - Stemming should match "running" with "run" + // Note: Porter stemmer handles regular inflections (running->run, runs->run) + // but NOT irregular verbs (ran does NOT stem to run) + await this._ftsIndex.IndexAsync("id1", "The athlete is running fast").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("id2", "She runs every morning").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("id3", "The runner finished the race").ConfigureAwait(false); + + // Act - Search for "run" should find variations via stemming + var results = await this._ftsIndex.SearchAsync("run").ConfigureAwait(false); + + // Assert - With Porter stemming: running->run, runs->run, runner->runner (partial match) + Assert.Equal(2, results.Count); // running and runs stem to run + Assert.Contains(results, r => r.ContentId == "id1"); + Assert.Contains(results, r => r.ContentId == "id2"); + } + + [Fact] + public async Task RemoveAsync_RemovesIndexedContent() + { + // Arrange + const string contentId = "test-remove"; + await this._ftsIndex.IndexAsync(contentId, "content to be removed").ConfigureAwait(false); + + // Verify it exists + var beforeRemove = await this._ftsIndex.SearchAsync("removed").ConfigureAwait(false); + Assert.Single(beforeRemove); + + // Act + await this._ftsIndex.RemoveAsync(contentId).ConfigureAwait(false); + + // Assert + var afterRemove = await this._ftsIndex.SearchAsync("removed").ConfigureAwait(false); + Assert.Empty(afterRemove); + } + + [Fact] + public async Task RemoveAsync_IsIdempotent() + { + // Arrange + const string contentId = "non-existent-id"; + + // Act - Should not throw for non-existent content + await this._ftsIndex.RemoveAsync(contentId).ConfigureAwait(false); + await this._ftsIndex.RemoveAsync(contentId).ConfigureAwait(false); + + // Assert - No exception thrown + Assert.True(true); + } + + [Fact] + public async Task ClearAsync_RemovesAllContent() + { + // Arrange + await this._ftsIndex.IndexAsync("id1", "first document").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("id2", "second document").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("id3", "third document").ConfigureAwait(false); + + // Verify content exists + var beforeClear = await this._ftsIndex.SearchAsync("document").ConfigureAwait(false); + Assert.Equal(3, beforeClear.Count); + + // Act + await this._ftsIndex.ClearAsync().ConfigureAwait(false); + + // Assert + var afterClear = await this._ftsIndex.SearchAsync("document").ConfigureAwait(false); + Assert.Empty(afterClear); + } + + [Fact] + public async Task SearchAsync_HandlesFts5Syntax() + { + // Arrange + await this._ftsIndex.IndexAsync("id1", "apple banana cherry").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("id2", "apple orange grape").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("id3", "banana mango papaya").ConfigureAwait(false); + + // Act - FTS5 AND query + var results = await this._ftsIndex.SearchAsync("apple AND banana").ConfigureAwait(false); + + // Assert - Only id1 has both apple and banana + Assert.Single(results); + Assert.Equal("id1", results[0].ContentId); + } + + [Fact] + public async Task SearchAsync_HandlesSpecialCharacters() + { + // Arrange + await this._ftsIndex.IndexAsync("id1", "C# programming with .NET framework").ConfigureAwait(false); + await this._ftsIndex.IndexAsync("id2", "JavaScript ES6+ features").ConfigureAwait(false); + + // Act + var results = await this._ftsIndex.SearchAsync("programming").ConfigureAwait(false); + + // Assert + Assert.Single(results); + Assert.Equal("id1", results[0].ContentId); + } + + [Fact] + public async Task IndexAsync_HandlesUnicodeContent() + { + // Arrange + const string unicodeContent = "日本語テキスト and English mixed content"; + await this._ftsIndex.IndexAsync("unicode-id", unicodeContent).ConfigureAwait(false); + + // Act + var results = await this._ftsIndex.SearchAsync("English").ConfigureAwait(false); + + // Assert + Assert.Single(results); + Assert.Equal("unicode-id", results[0].ContentId); + } + + [Fact] + public async Task IndexAsync_HandlesEmptyContent() + { + // Arrange & Act - Should not throw + await this._ftsIndex.IndexAsync("empty-id", "").ConfigureAwait(false); + + // Assert - Search should return nothing for empty content + var results = await this._ftsIndex.SearchAsync("anything").ConfigureAwait(false); + Assert.Empty(results); + } + + [Fact] + public async Task ScoreProperty_IsPositive() + { + // Arrange + await this._ftsIndex.IndexAsync("id1", "test content for scoring").ConfigureAwait(false); + + // Act + var results = await this._ftsIndex.SearchAsync("test").ConfigureAwait(false); + + // Assert - Score should be positive (we negate FTS5 rank) + Assert.Single(results); + Assert.True(results[0].Score > 0, "Score should be positive"); + } +} diff --git a/tests/Core.Tests/Storage/ContentStorageIntegrationTests.cs b/tests/Core.Tests/Storage/ContentStorageIntegrationTests.cs index 151b5c877..0f66799bc 100644 --- a/tests/Core.Tests/Storage/ContentStorageIntegrationTests.cs +++ b/tests/Core.Tests/Storage/ContentStorageIntegrationTests.cs @@ -105,11 +105,11 @@ public async Task FullWorkflow_UpsertRetrieveDeleteAsync() }; // Act 1: Upsert - var contentId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(200).ConfigureAwait(false); // Wait for processing // Assert 1: Content exists - var content = await this._service.GetByIdAsync(contentId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal("Integration test content", content.Content); Assert.Equal("text/plain", content.MimeType); @@ -121,7 +121,7 @@ public async Task FullWorkflow_UpsertRetrieveDeleteAsync() // Act 2: Update var updateRequest = new UpsertRequest { - Id = contentId, + Id = result.Id, Content = "Updated content", MimeType = "text/html", Title = "Updated Title" @@ -130,18 +130,18 @@ public async Task FullWorkflow_UpsertRetrieveDeleteAsync() await Task.Delay(200).ConfigureAwait(false); // Wait for processing // Assert 2: Content is updated - var updatedContent = await this._service.GetByIdAsync(contentId).ConfigureAwait(false); + var updatedContent = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(updatedContent); Assert.Equal("Updated content", updatedContent.Content); Assert.Equal("text/html", updatedContent.MimeType); Assert.Equal("Updated Title", updatedContent.Title); // Act 3: Delete - await this._service.DeleteAsync(contentId).ConfigureAwait(false); + await this._service.DeleteAsync(result.Id).ConfigureAwait(false); await Task.Delay(200).ConfigureAwait(false); // Wait for processing // Assert 3: Content is deleted - var deletedContent = await this._service.GetByIdAsync(contentId).ConfigureAwait(false); + var deletedContent = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.Null(deletedContent); } @@ -152,12 +152,12 @@ public async Task RealCuidGenerator_GeneratesValidIdsAsync() var ids = new List(); for (int i = 0; i < 10; i++) { - var id = await this._service.UpsertAsync(new UpsertRequest + var result = await this._service.UpsertAsync(new UpsertRequest { Content = $"Content {i}", MimeType = "text/plain" }).ConfigureAwait(false); - ids.Add(id); + ids.Add(result.Id); } // Assert - All IDs should be unique and non-empty @@ -268,11 +268,11 @@ public async Task DateTimeOffset_IsStoredAndRetrievedCorrectlyAsync() }; // Act - var contentId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(200).ConfigureAwait(false); // Wait for processing // Assert - var content = await this._service.GetByIdAsync(contentId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal(specificDate, content.ContentCreatedAt); } @@ -296,11 +296,11 @@ public async Task JsonSerialization_HandlesComplexMetadataAsync() }; // Act - var contentId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(200).ConfigureAwait(false); // Wait for processing // Assert - var content = await this._service.GetByIdAsync(contentId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal(3, content.Tags.Length); Assert.Equal("tag with spaces", content.Tags[0]); @@ -355,11 +355,11 @@ public async Task EmptyStringFields_AreHandledCorrectlyAsync() }; // Act - var contentId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(200).ConfigureAwait(false); // Wait for processing // Assert - var content = await this._service.GetByIdAsync(contentId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal(string.Empty, content.Title); Assert.Equal(string.Empty, content.Description); diff --git a/tests/Core.Tests/Storage/ContentStorageServiceTests.cs b/tests/Core.Tests/Storage/ContentStorageServiceTests.cs index 50b134cea..607d29f5d 100644 --- a/tests/Core.Tests/Storage/ContentStorageServiceTests.cs +++ b/tests/Core.Tests/Storage/ContentStorageServiceTests.cs @@ -65,13 +65,15 @@ public async Task UpsertAsync_WithEmptyId_GeneratesNewIdAsync() }; // Act - var resultId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); // Assert - Assert.Equal("test_id_00001", resultId); // First generated ID + Assert.Equal("test_id_00001", result.Id); // First generated ID + Assert.True(result.Completed); + Assert.False(result.Queued); // Verify content was created - var content = await this._service.GetByIdAsync(resultId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal("Test content", content.Content); Assert.Equal("text/plain", content.MimeType); @@ -90,13 +92,14 @@ public async Task UpsertAsync_WithProvidedId_UsesProvidedIdAsync() }; // Act - var resultId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); // Assert - Assert.Equal("custom_id_123", resultId); + Assert.Equal("custom_id_123", result.Id); + Assert.True(result.Completed); // Verify content was created - var content = await this._service.GetByIdAsync(resultId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal("Test content", content.Content); } @@ -150,11 +153,11 @@ public async Task UpsertAsync_StoresTagsAsync() }; // Act - var resultId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(100).ConfigureAwait(false); // Wait for processing // Assert - var content = await this._service.GetByIdAsync(resultId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal(3, content.Tags.Length); Assert.Contains("tag1", content.Tags); @@ -178,11 +181,11 @@ public async Task UpsertAsync_StoresMetadataAsync() }; // Act - var resultId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(100).ConfigureAwait(false); // Wait for processing // Assert - var content = await this._service.GetByIdAsync(resultId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal(2, content.Metadata.Count); Assert.Equal("value1", content.Metadata["key1"]); @@ -201,11 +204,11 @@ public async Task UpsertAsync_CalculatesByteSizeAsync() }; // Act - var resultId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(100).ConfigureAwait(false); // Wait for processing // Assert - var content = await this._service.GetByIdAsync(resultId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal(System.Text.Encoding.UTF8.GetByteCount(testContent), content.ByteSize); } @@ -223,11 +226,11 @@ public async Task UpsertAsync_UsesCustomContentCreatedAtAsync() }; // Act - var resultId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(100).ConfigureAwait(false); // Wait for processing // Assert - var content = await this._service.GetByIdAsync(resultId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal(customDate, content.ContentCreatedAt); } @@ -312,11 +315,11 @@ public async Task UpsertAsync_QueuesOperationSuccessfullyAsync() }; // Act - var resultId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); // Assert - Operation should be queued var operation = await this._context.Operations - .FirstOrDefaultAsync(o => o.ContentId == resultId).ConfigureAwait(false); + .FirstOrDefaultAsync(o => o.ContentId == result.Id).ConfigureAwait(false); Assert.NotNull(operation); Assert.False(operation.Complete); @@ -466,12 +469,12 @@ public async Task RecordTimestamps_AreSetCorrectlyAsync() }; // Act - var resultId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(100).ConfigureAwait(false); // Wait for processing var afterCreate = DateTimeOffset.UtcNow.AddSeconds(1); // Assert - var content = await this._service.GetByIdAsync(resultId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.InRange(content.RecordCreatedAt, beforeCreate, afterCreate); Assert.InRange(content.RecordUpdatedAt, beforeCreate, afterCreate); @@ -488,11 +491,11 @@ public async Task EmptyContent_IsAllowedAsync() }; // Act - var resultId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(100).ConfigureAwait(false); // Wait for processing // Assert - var content = await this._service.GetByIdAsync(resultId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal(string.Empty, content.Content); Assert.Equal(0, content.ByteSize); @@ -510,11 +513,11 @@ public async Task UpsertAsync_HandlesLargeContentAsync() }; // Act - var resultId = await this._service.UpsertAsync(request).ConfigureAwait(false); + var result = await this._service.UpsertAsync(request).ConfigureAwait(false); await Task.Delay(1000).ConfigureAwait(false); // Wait longer for large content processing // Assert - var content = await this._service.GetByIdAsync(resultId).ConfigureAwait(false); + var content = await this._service.GetByIdAsync(result.Id).ConfigureAwait(false); Assert.NotNull(content); Assert.Equal(largeContent.Length, content.Content.Length); Assert.True(content.ByteSize >= 1024 * 1024); // Should be at least 1MB (UTF-8 encoding) @@ -535,11 +538,11 @@ public async Task ListAsync_ReturnsEmptyListWhenNoContent() public async Task ListAsync_ReturnsContentOrderedByCreationTimeDescending() { // Arrange - Create multiple content items - var id1 = await this._service.UpsertAsync(new UpsertRequest { Content = "First", MimeType = "text/plain" }).ConfigureAwait(false); + var result1 = await this._service.UpsertAsync(new UpsertRequest { Content = "First", MimeType = "text/plain" }).ConfigureAwait(false); await Task.Delay(100).ConfigureAwait(false); - var id2 = await this._service.UpsertAsync(new UpsertRequest { Content = "Second", MimeType = "text/plain" }).ConfigureAwait(false); + var result2 = await this._service.UpsertAsync(new UpsertRequest { Content = "Second", MimeType = "text/plain" }).ConfigureAwait(false); await Task.Delay(100).ConfigureAwait(false); - var id3 = await this._service.UpsertAsync(new UpsertRequest { Content = "Third", MimeType = "text/plain" }).ConfigureAwait(false); + var result3 = await this._service.UpsertAsync(new UpsertRequest { Content = "Third", MimeType = "text/plain" }).ConfigureAwait(false); await Task.Delay(1000).ConfigureAwait(false); // Wait for processing // Act @@ -548,9 +551,9 @@ public async Task ListAsync_ReturnsContentOrderedByCreationTimeDescending() // Assert Assert.NotNull(result); Assert.Equal(3, result.Count); - Assert.Equal(id3, result[0].Id); // Most recent first - Assert.Equal(id2, result[1].Id); - Assert.Equal(id1, result[2].Id); + Assert.Equal(result3.Id, result[0].Id); // Most recent first + Assert.Equal(result2.Id, result[1].Id); + Assert.Equal(result1.Id, result[2].Id); } [Fact] diff --git a/tests/Main.Tests/Integration/CliIntegrationTests.cs b/tests/Main.Tests/Integration/CliIntegrationTests.cs index 85fad0a07..194915313 100644 --- a/tests/Main.Tests/Integration/CliIntegrationTests.cs +++ b/tests/Main.Tests/Integration/CliIntegrationTests.cs @@ -10,7 +10,7 @@ namespace KernelMemory.Main.Tests.Integration; /// /// Integration tests for CLI commands with real SQLite database. -/// These tests cover end-to-end workflows: upsert → get → list → delete. +/// These tests cover end-to-end workflows: put → get → list → delete. /// public sealed class CliIntegrationTests : IDisposable { @@ -72,7 +72,7 @@ public async Task UpsertCommand_WithMinimalOptions_CreatesContent() }; var command = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); // Act var exitCode = await command.ExecuteAsync(context, settings).ConfigureAwait(false); @@ -97,7 +97,7 @@ public async Task UpsertCommand_WithCustomId_UsesProvidedId() }; var command = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); // Act var exitCode = await command.ExecuteAsync(context, settings).ConfigureAwait(false); @@ -135,7 +135,7 @@ public async Task UpsertCommand_WithAllMetadata_StoresAllFields() }; var command = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); // Act var exitCode = await command.ExecuteAsync(context, settings).ConfigureAwait(false); @@ -159,7 +159,7 @@ public async Task GetCommand_ExistingId_ReturnsContent() }; var upsertCommand = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings).ConfigureAwait(false); // Act - Get the content @@ -191,7 +191,7 @@ public async Task GetCommand_NonExistentId_ReturnsUserError() Content = "Some content to create the DB" }; var upsertCommand = new UpsertCommand(config); - await upsertCommand.ExecuteAsync(CreateTestContext("upsert"), upsertSettings).ConfigureAwait(false); + await upsertCommand.ExecuteAsync(CreateTestContext("put"), upsertSettings).ConfigureAwait(false); // Now try to get non-existent ID from existing DB var settings = new GetCommandSettings @@ -227,7 +227,7 @@ public async Task GetCommand_WithFullFlag_ReturnsAllDetails() }; var upsertCommand = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings).ConfigureAwait(false); // Act - Get with full flag @@ -259,7 +259,7 @@ public async Task ListCommand_EmptyDatabase_ReturnsEmptyList() Id = "temp-id" }; var upsertCommand = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings).ConfigureAwait(false); // Delete the content to have empty database @@ -306,7 +306,7 @@ public async Task Bug3_ListCommand_EmptyDatabase_HumanFormat_ShouldHandleGracefu Id = "temp-id-human" }; var upsertCommand = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings).ConfigureAwait(false); // Delete the content to have empty database @@ -351,7 +351,7 @@ public async Task ListCommand_WithContent_ReturnsList() }; var upsertCommand = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings).ConfigureAwait(false); // Act - List content @@ -374,7 +374,7 @@ public async Task ListCommand_WithPagination_RespectsSkipAndTake() // Arrange - Insert multiple items var config = ConfigParser.LoadFromFile(this._configPath); var upsertCommand = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); for (int i = 0; i < 5; i++) { @@ -418,7 +418,7 @@ public async Task DeleteCommand_ExistingId_DeletesSuccessfully() }; var upsertCommand = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings).ConfigureAwait(false); // Act - Delete the content @@ -463,7 +463,7 @@ public async Task DeleteCommand_WithQuietVerbosity_SucceedsWithMinimalOutput() }; var upsertCommand = new UpsertCommand(config); - var context = CreateTestContext("upsert"); + var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings).ConfigureAwait(false); // Act - Delete with quiet verbosity @@ -593,7 +593,8 @@ public async Task ConfigCommand_Default_ShowsCurrentNode() Format = "json" }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = CreateTestContext("config"); // Act @@ -615,7 +616,8 @@ public async Task ConfigCommand_WithShowNodes_ShowsAllNodes() ShowNodes = true }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = CreateTestContext("config"); // Act @@ -637,7 +639,8 @@ public async Task ConfigCommand_WithShowCache_ShowsCacheConfig() ShowCache = true }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = CreateTestContext("config"); // Act @@ -689,7 +692,8 @@ public async Task Bug2_ConfigCommand_HumanFormat_ShouldNotLeakTypeNames() Format = "human" }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = CreateTestContext("config"); // Act diff --git a/tests/Main.Tests/Integration/CommandExecutionTests.cs b/tests/Main.Tests/Integration/CommandExecutionTests.cs index 035900af0..8c3285060 100644 --- a/tests/Main.Tests/Integration/CommandExecutionTests.cs +++ b/tests/Main.Tests/Integration/CommandExecutionTests.cs @@ -66,7 +66,7 @@ public async Task UpsertCommand_WithValidContent_ReturnsSuccess() Content = "Test content" }; var command = new UpsertCommand(config); - var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "upsert", null); + var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "put", null); var result = await command.ExecuteAsync(context, settings).ConfigureAwait(false); Assert.Equal(0, result); @@ -78,15 +78,15 @@ public async Task GetCommand_WithNonExistentId_ReturnsError() // Load config and inject into command var config = ConfigParser.LoadFromFile(this._configPath); - // First create the database by upserting some content - var upsertSettings = new UpsertCommandSettings + // First create the database by puting some content + var putSettings = new UpsertCommandSettings { ConfigPath = this._configPath, Content = "Test content to create DB" }; - var upsertCommand = new UpsertCommand(config); - var upsertContext = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "upsert", null); - await upsertCommand.ExecuteAsync(upsertContext, upsertSettings).ConfigureAwait(false); + var putCommand = new UpsertCommand(config); + var putContext = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "put", null); + await putCommand.ExecuteAsync(putContext, putSettings).ConfigureAwait(false); // Now try to get non-existent ID from existing DB var settings = new GetCommandSettings @@ -126,15 +126,15 @@ public async Task ListCommand_WithEmptyDatabase_ReturnsSuccess() // Load config and inject into commands var config = ConfigParser.LoadFromFile(this._configPath); - // First create the database by upserting, then deleting to have empty database - var upsertSettings = new UpsertCommandSettings + // First create the database by puting, then deleting to have empty database + var putSettings = new UpsertCommandSettings { ConfigPath = this._configPath, Content = "Temp content to create database" }; - var upsertCommand = new UpsertCommand(config); - var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "upsert", null); - await upsertCommand.ExecuteAsync(context, upsertSettings).ConfigureAwait(false); + var putCommand = new UpsertCommand(config); + var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "put", null); + await putCommand.ExecuteAsync(context, putSettings).ConfigureAwait(false); // Delete to make it empty var deleteSettings = new DeleteCommandSettings @@ -183,7 +183,8 @@ public async Task ConfigCommand_WithoutFlags_ReturnsSuccess() { ConfigPath = this._configPath }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "config", null); var result = await command.ExecuteAsync(context, settings).ConfigureAwait(false); @@ -201,7 +202,8 @@ public async Task ConfigCommand_WithShowNodes_ReturnsSuccess() ConfigPath = this._configPath, ShowNodes = true }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "config", null); var result = await command.ExecuteAsync(context, settings).ConfigureAwait(false); @@ -219,7 +221,8 @@ public async Task ConfigCommand_WithShowCache_ReturnsSuccess() ConfigPath = this._configPath, ShowCache = true }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "config", null); var result = await command.ExecuteAsync(context, settings).ConfigureAwait(false); @@ -232,15 +235,15 @@ public async Task GetCommand_WithFullFlag_ReturnsSuccess() // Load config and inject into commands var config = ConfigParser.LoadFromFile(this._configPath); - // First upsert - var upsertSettings = new UpsertCommandSettings + // First put + var putSettings = new UpsertCommandSettings { ConfigPath = this._configPath, Content = "Test content for full flag" }; - var upsertCommand = new UpsertCommand(config); - var upsertContext = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "upsert", null); - await upsertCommand.ExecuteAsync(upsertContext, upsertSettings).ConfigureAwait(false); + var putCommand = new UpsertCommand(config); + var putContext = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "put", null); + await putCommand.ExecuteAsync(putContext, putSettings).ConfigureAwait(false); // Then get with full flag - will fail because we don't know the ID // But this still exercises the code path diff --git a/tests/Main.Tests/Integration/ConfigCommandTests.cs b/tests/Main.Tests/Integration/ConfigCommandTests.cs index 98706933d..6344ec0d0 100644 --- a/tests/Main.Tests/Integration/ConfigCommandTests.cs +++ b/tests/Main.Tests/Integration/ConfigCommandTests.cs @@ -13,6 +13,13 @@ namespace KernelMemory.Main.Tests.Integration; /// These tests verify that config command shows the entire configuration, /// not just a single node. /// +/// +/// These tests capture Console.Out, which is a global shared resource. +/// Running them in parallel with other tests that write to Console.Out +/// (like HumanOutputFormatterTests) causes output contamination. +/// The [Collection] attribute ensures these tests run serially. +/// +[Collection("ConsoleOutputTests")] public sealed class ConfigCommandTests : IDisposable { private readonly string _tempDir; @@ -75,7 +82,8 @@ public void ConfigCommand_WithoutFlags_ShouldShowEntireConfiguration() Format = "json" // Use JSON format for easier assertion }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = new CommandContext( new[] { "--config", this._configPath }, new EmptyRemainingArguments(), @@ -127,7 +135,8 @@ public void ConfigCommand_OutputStructure_ShouldMatchAppConfigFormat() Format = "json" }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = new CommandContext( new[] { "--config", this._configPath }, new EmptyRemainingArguments(), @@ -184,7 +193,8 @@ public void ConfigCommand_WithShowNodesFlag_ShouldShowAllNodesSummary() ShowNodes = true }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = new CommandContext( new[] { "--config", this._configPath }, new EmptyRemainingArguments(), @@ -217,6 +227,245 @@ public void ConfigCommand_WithShowNodesFlag_ShouldShowAllNodesSummary() } } + [Fact] + public void ConfigCommand_WithCreate_CreatesConfigFile() + { + // Arrange: Use a non-existent config path + var newConfigPath = Path.Combine(this._tempDir, "new-config.json"); + Assert.False(File.Exists(newConfigPath)); + + var config = ConfigParser.LoadFromFile(this._configPath); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(newConfigPath); + var command = new ConfigCommand(config, configPathService); + + var settings = new ConfigCommandSettings + { + ConfigPath = newConfigPath, + Create = true, + Format = "json" + }; + + var context = new CommandContext( + new[] { "--config", newConfigPath, "--create" }, + new EmptyRemainingArguments(), + "config", + null); + + // Act + var exitCode = command.ExecuteAsync(context, settings).GetAwaiter().GetResult(); + + // Assert + Assert.Equal(Constants.ExitCodeSuccess, exitCode); + Assert.True(File.Exists(newConfigPath), "Config file should be created"); + + // Verify the file content is valid JSON + var createdJson = File.ReadAllText(newConfigPath); + var createdConfig = System.Text.Json.JsonDocument.Parse(createdJson); + Assert.NotNull(createdConfig); + } + + [Fact] + public void ConfigCommand_WithCreate_WhenFileExists_ReturnsError() + { + // Arrange: Config file already exists (from constructor) + Assert.True(File.Exists(this._configPath)); + + var config = ConfigParser.LoadFromFile(this._configPath); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); + + var settings = new ConfigCommandSettings + { + ConfigPath = this._configPath, + Create = true, + Format = "json" + }; + + var context = new CommandContext( + new[] { "--config", this._configPath, "--create" }, + new EmptyRemainingArguments(), + "config", + null); + + // Suppress console output + using var outputCapture = new StringWriter(); + using var errorCapture = new StringWriter(); + var originalOutput = Console.Out; + var originalError = Console.Error; + Console.SetOut(outputCapture); + Console.SetError(errorCapture); + + try + { + // Act + var exitCode = command.ExecuteAsync(context, settings).GetAwaiter().GetResult(); + + // Assert + Assert.Equal(Constants.ExitCodeUserError, exitCode); + + // Error message goes to Console.Error + var errorOutput = errorCapture.ToString(); + Assert.Contains("already exists", errorOutput); + } + finally + { + Console.SetOut(originalOutput); + Console.SetError(originalError); + } + } + + [Fact] + public void ConfigCommand_WithoutConfigFile_StillSucceeds() + { + // Arrange: Use a non-existent config path (default config will be used) + var missingConfigPath = Path.Combine(this._tempDir, "missing-config.json"); + Assert.False(File.Exists(missingConfigPath)); + + var config = AppConfig.CreateDefault(); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(missingConfigPath); + var command = new ConfigCommand(config, configPathService); + + var settings = new ConfigCommandSettings + { + ConfigPath = missingConfigPath, + Format = "json", + NoColor = false // Enable colors + }; + + var context = new CommandContext( + new[] { "--config", missingConfigPath }, + new EmptyRemainingArguments(), + "config", + null); + + // Suppress console output (warning goes to AnsiConsole which is hard to capture in tests) + using var outputCapture = new StringWriter(); + var originalOutput = Console.Out; + Console.SetOut(outputCapture); + + try + { + // Act + var exitCode = command.ExecuteAsync(context, settings).GetAwaiter().GetResult(); + + // Assert + // The key behavior: command succeeds even without config file + Assert.Equal(Constants.ExitCodeSuccess, exitCode); + + // Should still output valid JSON config + var output = outputCapture.ToString(); + var json = System.Text.Json.JsonDocument.Parse(output); + Assert.NotNull(json); + Assert.True(json.RootElement.TryGetProperty("nodes", out _)); + } + finally + { + Console.SetOut(originalOutput); + } + } + + [Fact] + public void ConfigCommand_OutputJson_DoesNotContainNullFields() + { + // Arrange + var config = ConfigParser.LoadFromFile(this._configPath); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); + + var settings = new ConfigCommandSettings + { + ConfigPath = this._configPath, + Format = "json" + }; + + var context = new CommandContext( + new[] { "--config", this._configPath }, + new EmptyRemainingArguments(), + "config", + null); + + // Capture stdout + using var outputCapture = new StringWriter(); + var originalOutput = Console.Out; + Console.SetOut(outputCapture); + + try + { + // Act + var exitCode = command.ExecuteAsync(context, settings).GetAwaiter().GetResult(); + + // Assert + Assert.Equal(Constants.ExitCodeSuccess, exitCode); + + var output = outputCapture.ToString(); + + // Verify no null fields are serialized + Assert.DoesNotContain("\"connectionString\": null", output); + Assert.DoesNotContain("\"embeddings\": null", output); + Assert.DoesNotContain("\"fileStorage\": null", output); + Assert.DoesNotContain("\"repoStorage\": null", output); + Assert.DoesNotContain("\"llmCache\": null", output); + } + finally + { + Console.SetOut(originalOutput); + } + } + + [Fact] + public void ConfigCommand_OutputJson_ContainsCorrectDiscriminators() + { + // Arrange + var config = ConfigParser.LoadFromFile(this._configPath); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); + + var settings = new ConfigCommandSettings + { + ConfigPath = this._configPath, + Format = "json" + }; + + var context = new CommandContext( + new[] { "--config", this._configPath }, + new EmptyRemainingArguments(), + "config", + null); + + // Capture stdout + using var outputCapture = new StringWriter(); + var originalOutput = Console.Out; + Console.SetOut(outputCapture); + + try + { + // Act + var exitCode = command.ExecuteAsync(context, settings).GetAwaiter().GetResult(); + + // Assert + Assert.Equal(Constants.ExitCodeSuccess, exitCode); + + var output = outputCapture.ToString(); + var outputJson = System.Text.Json.JsonDocument.Parse(output); + + // Verify content index uses "sqlite" (not a generic name) + var contentIndex = outputJson.RootElement.GetProperty("nodes").GetProperty("personal").GetProperty("contentIndex"); + Assert.Equal("sqlite", contentIndex.GetProperty("type").GetString()); + + // Verify search index uses "sqliteFTS" (not generic "fts") + var searchIndex = outputJson.RootElement.GetProperty("nodes").GetProperty("personal").GetProperty("searchIndexes")[0]; + Assert.Equal("sqliteFTS", searchIndex.GetProperty("type").GetString()); + + // Verify cache uses "Sqlite" + var embeddingsCache = outputJson.RootElement.GetProperty("embeddingsCache"); + Assert.Equal("Sqlite", embeddingsCache.GetProperty("type").GetString()); + } + finally + { + Console.SetOut(originalOutput); + } + } + /// /// Helper class to provide empty remaining arguments for CommandContext. /// diff --git a/tests/Main.Tests/Integration/ReadonlyCommandTests.cs b/tests/Main.Tests/Integration/ReadonlyCommandTests.cs index 13c503d90..db8abe4a0 100644 --- a/tests/Main.Tests/Integration/ReadonlyCommandTests.cs +++ b/tests/Main.Tests/Integration/ReadonlyCommandTests.cs @@ -188,7 +188,8 @@ public async Task BugA_ConfigCommand_NonExistentDatabase_ShouldNotCreateDirector Format = "json" }; - var command = new ConfigCommand(config); + var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); + var command = new ConfigCommand(config, configPathService); var context = CreateTestContext("config"); // Act diff --git a/tests/Main.Tests/Integration/RealConfigAutoCreationTest.cs b/tests/Main.Tests/Integration/RealConfigAutoCreationTest.cs new file mode 100644 index 000000000..ff196e7c7 --- /dev/null +++ b/tests/Main.Tests/Integration/RealConfigAutoCreationTest.cs @@ -0,0 +1,92 @@ +// Copyright (c) Microsoft. All rights reserved. + +using Xunit; + +namespace KernelMemory.Main.Tests.Integration; + +/// +/// Real end-to-end test: Config file must exist after ANY write, even the millionth. +/// This tests the actual CLI behavior, not just ConfigParser in isolation. +/// +public sealed class RealConfigAutoCreationTest : IDisposable +{ + private readonly string _tempDir; + private readonly string _configPath; + + public RealConfigAutoCreationTest() + { + this._tempDir = Path.Combine(Path.GetTempPath(), $"km-real-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(this._tempDir); + this._configPath = Path.Combine(this._tempDir, "config.json"); + } + + public void Dispose() + { + try + { + if (Directory.Exists(this._tempDir)) + { + Directory.Delete(this._tempDir, recursive: true); + } + } + catch (IOException) + { + // Best effort - ignore IO errors during test cleanup + } + catch (UnauthorizedAccessException) + { + // Best effort - ignore permission errors during test cleanup + } + } + + [Fact] + public void AfterAnyWrite_ConfigFileMustExist() + { + // Test the ACTUAL requirement: + // After running km with a config path, the config file MUST exist + + // Use the actual CLI application builder (how the real app works) + var builder = new CLI.CliApplicationBuilder(); + + // Scenario 1: First write + { + Assert.False(File.Exists(this._configPath), "Config should not exist before first write"); + + var app = builder.Build(new[] { "put", "First", "--config", this._configPath }); + var exitCode = app.RunAsync(new[] { "put", "First", "--config", this._configPath }).GetAwaiter().GetResult(); + + Assert.Equal(0, exitCode); + Assert.True(File.Exists(this._configPath), "Config MUST exist after first write"); + } + + // Scenario 2: Config deleted, second write + { + File.Delete(this._configPath); + Assert.False(File.Exists(this._configPath), "Config deleted"); + + var app = builder.Build(new[] { "put", "Second", "--config", this._configPath }); + var exitCode = app.RunAsync(new[] { "put", "Second", "--config", this._configPath }).GetAwaiter().GetResult(); + + Assert.Equal(0, exitCode); + Assert.True(File.Exists(this._configPath), "Config MUST be recreated after second write"); + } + + // Scenario 3: Config deleted after 10 writes + { + for (int i = 0; i < 10; i++) + { + if (i == 5) + { + File.Delete(this._configPath); + Assert.False(File.Exists(this._configPath), "Config deleted at iteration 5"); + } + + var app = builder.Build(new[] { "put", $"Record {i}", "--config", this._configPath }); + var exitCode = app.RunAsync(new[] { "put", $"Record {i}", "--config", this._configPath }).GetAwaiter().GetResult(); + + Assert.Equal(0, exitCode); + Assert.True(File.Exists(this._configPath), $"Config MUST exist after write {i}"); + } + } + } +} diff --git a/tests/Main.Tests/Integration/UserDataProtectionTests.cs b/tests/Main.Tests/Integration/UserDataProtectionTests.cs index b9fab3a46..d8738107f 100644 --- a/tests/Main.Tests/Integration/UserDataProtectionTests.cs +++ b/tests/Main.Tests/Integration/UserDataProtectionTests.cs @@ -93,7 +93,7 @@ public async Task CriticalBug_CommandExecutionTests_MustNotTouchUserData() var context = new CommandContext( new[] { "--config", this._configPath }, new EmptyRemainingArguments(), - "upsert", + "put", null); // Act - This WILL write to ~/.km if bug exists @@ -151,7 +151,7 @@ public async Task Fixed_SettingsWithConfigPath_MustUseTestDirectory() var context = new CommandContext( new[] { "--config", this._configPath }, new EmptyRemainingArguments(), - "upsert", + "put", null); // Act diff --git a/tests/Main.Tests/TestCollections.cs b/tests/Main.Tests/TestCollections.cs new file mode 100644 index 000000000..b813c151a --- /dev/null +++ b/tests/Main.Tests/TestCollections.cs @@ -0,0 +1,15 @@ +// Copyright (c) Microsoft. All rights reserved. +using Xunit; + +namespace KernelMemory.Main.Tests; + +/// +/// Collection definition for tests that use Console.Out or Console.Error. +/// Tests in this collection will not run in parallel with each other, +/// preventing output contamination when some tests capture Console output +/// and others write to it directly. +/// +[CollectionDefinition("ConsoleOutputTests", DisableParallelization = true)] +public class ConsoleOutputTestGroup +{ +} diff --git a/tests/Main.Tests/Unit/CLI/ModeRouterTests.cs b/tests/Main.Tests/Unit/CLI/ModeRouterTests.cs index 4063c55e2..51252d07a 100644 --- a/tests/Main.Tests/Unit/CLI/ModeRouterTests.cs +++ b/tests/Main.Tests/Unit/CLI/ModeRouterTests.cs @@ -50,7 +50,7 @@ public void DetectMode_RpcArguments_ReturnsRpc(string arg) } [Theory] - [InlineData("upsert", "cli")] + [InlineData("put", "cli")] [InlineData("get", "cli")] [InlineData("list", "cli")] [InlineData("unknown", "cli")] diff --git a/tests/Main.Tests/Unit/OutputFormatters/HumanOutputFormatterTests.cs b/tests/Main.Tests/Unit/OutputFormatters/HumanOutputFormatterTests.cs index a052caf31..930ab5380 100644 --- a/tests/Main.Tests/Unit/OutputFormatters/HumanOutputFormatterTests.cs +++ b/tests/Main.Tests/Unit/OutputFormatters/HumanOutputFormatterTests.cs @@ -9,6 +9,7 @@ namespace KernelMemory.Main.Tests.Unit.OutputFormatters; /// Unit tests for HumanOutputFormatter. /// Tests exercise the formatting logic with various data types. /// +[Collection("ConsoleOutputTests")] public sealed class HumanOutputFormatterTests { [Fact] diff --git a/tests/Main.Tests/Unit/OutputFormatters/JsonOutputFormatterTests.cs b/tests/Main.Tests/Unit/OutputFormatters/JsonOutputFormatterTests.cs index d8fddd907..c11a7f44a 100644 --- a/tests/Main.Tests/Unit/OutputFormatters/JsonOutputFormatterTests.cs +++ b/tests/Main.Tests/Unit/OutputFormatters/JsonOutputFormatterTests.cs @@ -9,6 +9,7 @@ namespace KernelMemory.Main.Tests.Unit.OutputFormatters; /// Unit tests for JsonOutputFormatter. /// Tests focus on behavior, not console output. /// +[Collection("ConsoleOutputTests")] public sealed class JsonOutputFormatterTests { [Fact] diff --git a/tests/Main.Tests/Unit/OutputFormatters/YamlOutputFormatterTests.cs b/tests/Main.Tests/Unit/OutputFormatters/YamlOutputFormatterTests.cs index bfd882090..74db014fe 100644 --- a/tests/Main.Tests/Unit/OutputFormatters/YamlOutputFormatterTests.cs +++ b/tests/Main.Tests/Unit/OutputFormatters/YamlOutputFormatterTests.cs @@ -8,6 +8,7 @@ namespace KernelMemory.Main.Tests.Unit.OutputFormatters; /// /// Unit tests for YamlOutputFormatter. /// +[Collection("ConsoleOutputTests")] public sealed class YamlOutputFormatterTests { [Fact] diff --git a/tests/Main.Tests/Unit/Services/ContentServiceTests.cs b/tests/Main.Tests/Unit/Services/ContentServiceTests.cs index 714c6da89..5a276f26b 100644 --- a/tests/Main.Tests/Unit/Services/ContentServiceTests.cs +++ b/tests/Main.Tests/Unit/Services/ContentServiceTests.cs @@ -32,8 +32,9 @@ public async Task UpsertAsync_CallsStorageUpsert() // Arrange var mockStorage = new Mock(); const string expectedId = "generated-id-123"; + var expectedResult = WriteResult.Success(expectedId); mockStorage.Setup(s => s.UpsertAsync(It.IsAny(), It.IsAny())) - .ReturnsAsync(expectedId); + .ReturnsAsync(expectedResult); var service = new ContentService(mockStorage.Object, "test-node"); var request = new UpsertRequest @@ -46,7 +47,8 @@ public async Task UpsertAsync_CallsStorageUpsert() var result = await service.UpsertAsync(request, CancellationToken.None).ConfigureAwait(false); // Assert - Assert.Equal(expectedId, result); + Assert.Equal(expectedId, result.Id); + Assert.True(result.Completed); mockStorage.Verify(s => s.UpsertAsync(request, CancellationToken.None), Times.Once); } @@ -57,8 +59,9 @@ public async Task UpsertAsync_WithCancellationToken_PassesTokenToStorage() var mockStorage = new Mock(); using var cts = new CancellationTokenSource(); const string expectedId = "id-456"; + var expectedResult = WriteResult.Success(expectedId); mockStorage.Setup(s => s.UpsertAsync(It.IsAny(), cts.Token)) - .ReturnsAsync(expectedId); + .ReturnsAsync(expectedResult); var service = new ContentService(mockStorage.Object, "test-node"); var request = new UpsertRequest { Content = "Content" }; @@ -67,7 +70,8 @@ public async Task UpsertAsync_WithCancellationToken_PassesTokenToStorage() var result = await service.UpsertAsync(request, cts.Token).ConfigureAwait(false); // Assert - Assert.Equal(expectedId, result); + Assert.Equal(expectedId, result.Id); + Assert.True(result.Completed); mockStorage.Verify(s => s.UpsertAsync(request, cts.Token), Times.Once); } @@ -116,15 +120,18 @@ public async Task DeleteAsync_CallsStorageDelete() // Arrange const string contentId = "delete-id"; var mockStorage = new Mock(); + var expectedResult = WriteResult.Success(contentId); mockStorage.Setup(s => s.DeleteAsync(contentId, It.IsAny())) - .Returns(Task.CompletedTask); + .ReturnsAsync(expectedResult); var service = new ContentService(mockStorage.Object, "test-node"); // Act - await service.DeleteAsync(contentId, CancellationToken.None).ConfigureAwait(false); + var result = await service.DeleteAsync(contentId, CancellationToken.None).ConfigureAwait(false); // Assert + Assert.Equal(contentId, result.Id); + Assert.True(result.Completed); mockStorage.Verify(s => s.DeleteAsync(contentId, CancellationToken.None), Times.Once); }