diff --git a/KNOWN-ISSUES.md b/KNOWN-ISSUES.md index 65d00bb2b..9e976bb68 100644 --- a/KNOWN-ISSUES.md +++ b/KNOWN-ISSUES.md @@ -86,21 +86,7 @@ km search 'content:"user:password"' ## Configuration Limitations -### 4. Index ID Hardcoded - -**Status:** Known limitation - -**Issue:** Index ID is hardcoded as `"fts-main"` instead of being loaded from configuration. - -**Location:** `src/Core/Search/NodeSearchService.cs:77` - -**Impact:** Cannot configure multiple indexes per node. - -**Fix Required:** Load index configuration from node settings. - ---- - -### 5. Index Weights Not Configurable +### 4. Index Weights Not Configurable **Status:** Known limitation diff --git a/src/Core/Search/NodeSearchService.cs b/src/Core/Search/NodeSearchService.cs index 106fbb344..b2883d5ab 100644 --- a/src/Core/Search/NodeSearchService.cs +++ b/src/Core/Search/NodeSearchService.cs @@ -14,6 +14,7 @@ namespace KernelMemory.Core.Search; public sealed class NodeSearchService { private readonly string _nodeId; + private readonly string _indexId; private readonly IFtsIndex _ftsIndex; private readonly IContentStorage _contentStorage; @@ -23,9 +24,15 @@ public sealed class NodeSearchService /// The node ID this service operates on. /// The FTS index for this node. /// The content storage for loading full records. - public NodeSearchService(string nodeId, IFtsIndex ftsIndex, IContentStorage contentStorage) + /// Optional index ID for this FTS index. Defaults to SearchConstants.DefaultFtsIndexId. + public NodeSearchService( + string nodeId, + IFtsIndex ftsIndex, + IContentStorage contentStorage, + string indexId = SearchConstants.DefaultFtsIndexId) { this._nodeId = nodeId; + this._indexId = indexId; this._ftsIndex = ftsIndex; this._contentStorage = contentStorage; } @@ -74,7 +81,7 @@ public NodeSearchService(string nodeId, IFtsIndex ftsIndex, IContentStorage cont { RecordId = content.Id, NodeId = this._nodeId, - IndexId = "fts-main", // TODO: Get from index config + IndexId = this._indexId, ChunkId = null, BaseRelevance = (float)match.Score, Title = content.Title, diff --git a/src/Core/Search/SearchConstants.cs b/src/Core/Search/SearchConstants.cs index 90bc229c7..f6e3b2812 100644 --- a/src/Core/Search/SearchConstants.cs +++ b/src/Core/Search/SearchConstants.cs @@ -118,4 +118,10 @@ public static class SearchConstants /// Minimum relevance score. /// public const float MinRelevanceScore = 0.0f; + + /// + /// Default FTS index ID used when not specified in configuration. + /// This is the identifier assigned to search results from the full-text search index. + /// + public const string DefaultFtsIndexId = "fts-main"; } diff --git a/tests/Core.Tests/Search/NodeSearchServiceIndexIdTests.cs b/tests/Core.Tests/Search/NodeSearchServiceIndexIdTests.cs new file mode 100644 index 000000000..628326182 --- /dev/null +++ b/tests/Core.Tests/Search/NodeSearchServiceIndexIdTests.cs @@ -0,0 +1,233 @@ +// Copyright (c) Microsoft. All rights reserved. + +using KernelMemory.Core.Search; +using KernelMemory.Core.Search.Models; +using KernelMemory.Core.Search.Query.Ast; +using KernelMemory.Core.Storage; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; +using Moq; + +namespace KernelMemory.Core.Tests.Search; + +/// +/// Unit tests for NodeSearchService index ID configuration. +/// Tests that index ID is properly configurable and not hardcoded. +/// Issue: Known Issue #4 - Index ID was hardcoded as "fts-main". +/// +public sealed class NodeSearchServiceIndexIdTests : IDisposable +{ + private readonly string _tempDir; + private readonly Mock> _mockFtsLogger; + private readonly Mock> _mockStorageLogger; + private readonly List _contexts = []; + private readonly List _ftsIndexes = []; + + public NodeSearchServiceIndexIdTests() + { + this._tempDir = Path.Combine(Path.GetTempPath(), $"km-index-id-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(this._tempDir); + this._mockFtsLogger = new Mock>(); + this._mockStorageLogger = new Mock>(); + } + + public void Dispose() + { + // Dispose all tracked resources + foreach (var ftsIndex in this._ftsIndexes) + { + ftsIndex.Dispose(); + } + + foreach (var context in this._contexts) + { + context.Dispose(); + } + + try + { + if (Directory.Exists(this._tempDir)) + { + Directory.Delete(this._tempDir, true); + } + } + catch (IOException) + { + // Ignore cleanup errors + } + } + + /// + /// Tests that NodeSearchService uses the provided index ID in search results. + /// The index ID should be configurable, not hardcoded. + /// + [Fact] + public async Task SearchAsync_WithCustomIndexId_ResultsContainThatIndexId() + { + // Arrange + const string customIndexId = "custom-fts-index"; + const string nodeId = "test-node"; + + var (ftsIndex, storage) = this.CreateIndexAndStorage("custom_index"); + + // Insert content + await storage.UpsertAsync(new KernelMemory.Core.Storage.Models.UpsertRequest + { + Content = "Test content for custom index ID verification", + MimeType = "text/plain" + }, CancellationToken.None).ConfigureAwait(false); + + // Create NodeSearchService with custom index ID + var nodeService = new NodeSearchService(nodeId, ftsIndex, storage, customIndexId); + + var request = new SearchRequest + { + Query = "content", + Limit = 10, + MinRelevance = 0.0f + }; + + // Act - SearchIndexResult is returned by NodeSearchService.SearchAsync + var queryNode = new TextSearchNode { SearchText = "content" }; + var (results, _) = await nodeService.SearchAsync( + queryNode, + request, + CancellationToken.None).ConfigureAwait(false); + + // Assert - SearchIndexResult has IndexId property + Assert.NotEmpty(results); + Assert.All(results, r => Assert.Equal(customIndexId, r.IndexId)); + } + + /// + /// Tests that the default index ID constant is used when not specified. + /// Ensures backward compatibility. + /// + [Fact] + public async Task SearchAsync_WithDefaultIndexId_UsesSearchConstantsDefault() + { + // Arrange + const string nodeId = "test-node"; + + var (ftsIndex, storage) = this.CreateIndexAndStorage("default_index"); + + // Insert content + await storage.UpsertAsync(new KernelMemory.Core.Storage.Models.UpsertRequest + { + Content = "Test content for default index ID verification", + MimeType = "text/plain" + }, CancellationToken.None).ConfigureAwait(false); + + // Create NodeSearchService WITHOUT specifying index ID (uses default) + var nodeService = new NodeSearchService(nodeId, ftsIndex, storage); + + var request = new SearchRequest + { + Query = "content", + Limit = 10, + MinRelevance = 0.0f + }; + + // Act + var queryNode = new TextSearchNode { SearchText = "content" }; + var (results, _) = await nodeService.SearchAsync( + queryNode, + request, + CancellationToken.None).ConfigureAwait(false); + + // Assert + Assert.NotEmpty(results); + Assert.All(results, r => Assert.Equal(SearchConstants.DefaultFtsIndexId, r.IndexId)); + } + + /// + /// Tests that different nodes can have different index IDs. + /// Validates multi-node, multi-index scenarios. + /// + [Fact] + public async Task SearchAsync_MultipleNodesWithDifferentIndexIds_EachHasCorrectIndexId() + { + // Arrange + const string node1Id = "node1"; + const string node1IndexId = "node1-fts-index"; + const string node2Id = "node2"; + const string node2IndexId = "node2-fts-index"; + + var (ftsIndex1, storage1) = this.CreateIndexAndStorage("node1_db"); + var (ftsIndex2, storage2) = this.CreateIndexAndStorage("node2_db"); + + // Insert content into both nodes + await storage1.UpsertAsync(new KernelMemory.Core.Storage.Models.UpsertRequest + { + Content = "Content in first node with custom index", + MimeType = "text/plain" + }, CancellationToken.None).ConfigureAwait(false); + + await storage2.UpsertAsync(new KernelMemory.Core.Storage.Models.UpsertRequest + { + Content = "Content in second node with different index", + MimeType = "text/plain" + }, CancellationToken.None).ConfigureAwait(false); + + // Create NodeSearchServices with different index IDs + var node1Service = new NodeSearchService(node1Id, ftsIndex1, storage1, node1IndexId); + var node2Service = new NodeSearchService(node2Id, ftsIndex2, storage2, node2IndexId); + + var queryNode = new TextSearchNode { SearchText = "content" }; + var request = new SearchRequest + { + Query = "content", + Limit = 10, + MinRelevance = 0.0f + }; + + // Act - Test each node service directly + var (results1, _) = await node1Service.SearchAsync(queryNode, request, CancellationToken.None).ConfigureAwait(false); + var (results2, _) = await node2Service.SearchAsync(queryNode, request, CancellationToken.None).ConfigureAwait(false); + + // Assert - Each node returns results with its own index ID + Assert.NotEmpty(results1); + Assert.NotEmpty(results2); + Assert.All(results1, r => Assert.Equal(node1IndexId, r.IndexId)); + Assert.All(results2, r => Assert.Equal(node2IndexId, r.IndexId)); + } + + /// + /// Tests that SearchConstants.DefaultFtsIndexId constant has the expected value. + /// Validates the constant is properly defined. + /// + [Fact] + public void DefaultFtsIndexId_HasExpectedValue() + { + // Assert + Assert.Equal("fts-main", SearchConstants.DefaultFtsIndexId); + } + + /// + /// Helper method to create FTS index and storage for testing. + /// Tracks created resources for proper disposal. + /// + /// Prefix for database file names to ensure uniqueness. + /// Tuple of FTS index and storage service. + private (SqliteFtsIndex ftsIndex, ContentStorageService storage) CreateIndexAndStorage(string dbPrefix) + { + var ftsDbPath = Path.Combine(this._tempDir, $"{dbPrefix}_fts.db"); + var contentDbPath = Path.Combine(this._tempDir, $"{dbPrefix}_content.db"); + + var ftsIndex = new SqliteFtsIndex(ftsDbPath, enableStemming: true, this._mockFtsLogger.Object); + this._ftsIndexes.Add(ftsIndex); + + var options = new DbContextOptionsBuilder() + .UseSqlite($"Data Source={contentDbPath}") + .Options; + var context = new ContentStorageDbContext(options); + this._contexts.Add(context); + context.Database.EnsureCreated(); + + var cuidGenerator = new CuidGenerator(); + var searchIndexes = new Dictionary { ["fts"] = ftsIndex }; + var storage = new ContentStorageService(context, cuidGenerator, this._mockStorageLogger.Object, searchIndexes); + + return (ftsIndex, storage); + } +}