Skip to content

Commit 8df4132

Browse files
fix: resolve critical FTS indexing and search bugs
This commit fixes multiple critical bugs that prevented full-text search from working properly: 1. **Wrong IndexAsync signature** (ContentStorageService.cs) - Was calling legacy 2-param IndexAsync(contentId, content) - Now properly calls 4-param IndexAsync(contentId, title, desc, content) - This ensures title and description fields are indexed for search 2. **Incorrect phrase search syntax** (NodeSearchService.cs) - Phrase searches now use "phrase" instead of field:"phrase" - Single-word searches use field:term syntax - Fixes FTS5 compatibility issues with multi-word queries 3. **SQLite connections not disposed** (ContentService.cs + Commands) - Made ContentService IDisposable to properly close FTS connections - Updated all CLI commands to use 'using' statements - Ensures writes are flushed before process exits 4. **Force synchronous writes** (SqliteFtsIndex.cs) - Added PRAGMA synchronous=FULL for immediate persistence - Added WAL checkpoint on disposal to flush pending writes - Prevents data loss in short-lived CLI processes All search functionality now working correctly with proper FTS indexing of title, description, and content fields. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <[email protected]>
1 parent ee2c004 commit 8df4132

File tree

10 files changed

+119
-24
lines changed

10 files changed

+119
-24
lines changed

src/Core/Search/NodeSearchService.cs

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -146,18 +146,29 @@ private string ExtractTerms(QueryNode node)
146146

147147
private string ExtractTextSearch(TextSearchNode node)
148148
{
149-
// Escape FTS5 special characters and quote the term
150-
var escapedText = this.EscapeFtsText(node.SearchText);
149+
// Check if this is a phrase search (contains spaces)
150+
var isPhrase = node.SearchText.Contains(' ', StringComparison.Ordinal);
151+
152+
if (isPhrase)
153+
{
154+
// Phrase searches: use quotes and no field prefix
155+
// FTS5 doesn't support field:phrase syntax well, so just search all fields
156+
var escapedPhrase = node.SearchText.Replace("\"", "\"\"", StringComparison.Ordinal);
157+
return $"\"{escapedPhrase}\"";
158+
}
159+
160+
// Single word searches: use field prefix WITHOUT quotes
161+
var escapedTerm = this.EscapeFtsSingleTerm(node.SearchText);
151162

152163
// If specific field, prefix with field name (SQLite FTS5 syntax)
153164
if (node.Field != null && this.IsFtsField(node.Field.FieldPath))
154165
{
155-
return $"{node.Field.FieldPath}:{escapedText}";
166+
return $"{node.Field.FieldPath}:{escapedTerm}";
156167
}
157168

158169
// Default field: search all FTS fields (title, description, content)
159170
// FTS5 syntax: {title description content}:term
160-
return $"{{title description content}}:{escapedText}";
171+
return $"{{title description content}}:{escapedTerm}";
161172
}
162173

163174
private string ExtractLogical(LogicalNode node)
@@ -191,8 +202,18 @@ private string ExtractComparison(ComparisonNode node)
191202
node.Value != null)
192203
{
193204
var searchText = node.Value.AsString();
194-
var escapedText = this.EscapeFtsText(searchText);
195-
return $"{node.Field.FieldPath}:{escapedText}";
205+
var isPhrase = searchText.Contains(' ', StringComparison.Ordinal);
206+
207+
if (isPhrase)
208+
{
209+
// Phrase search: use quotes without field prefix
210+
var escapedPhrase = searchText.Replace("\"", "\"\"", StringComparison.Ordinal);
211+
return $"\"{escapedPhrase}\"";
212+
}
213+
214+
// Single word: use field prefix without quotes
215+
var escapedTerm = this.EscapeFtsSingleTerm(searchText);
216+
return $"{node.Field.FieldPath}:{escapedTerm}";
196217
}
197218

198219
// Other comparison operators (==, !=, >=, etc.) are handled by LINQ filtering
@@ -211,12 +232,16 @@ private bool IsFtsField(string? fieldPath)
211232
return normalized == "title" || normalized == "description" || normalized == "content";
212233
}
213234

214-
private string EscapeFtsText(string text)
235+
private string EscapeFtsSingleTerm(string term)
215236
{
216-
// FTS5 special characters that need escaping: " * ( )
217-
// Wrap in quotes to handle spaces and special characters
218-
var escaped = text.Replace("\"", "\"\"", StringComparison.Ordinal); // Escape quotes by doubling
219-
return $"\"{escaped}\"";
237+
// For single-word searches with field prefix (e.g., content:call)
238+
// FTS5 does NOT support quotes after the colon: content:"call" is INVALID
239+
// We must use: content:call
240+
//
241+
// Escape FTS5 special characters: " *
242+
// For now, keep it simple: just remove quotes and wildcards that could break syntax
243+
return term.Replace("\"", string.Empty, StringComparison.Ordinal)
244+
.Replace("*", string.Empty, StringComparison.Ordinal);
220245
}
221246
}
222247
}

src/Core/Search/SqliteFtsIndex.cs

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ public async Task InitializeAsync(CancellationToken cancellationToken = default)
4444
this._connection = new SqliteConnection(this._connectionString);
4545
await this._connection.OpenAsync(cancellationToken).ConfigureAwait(false);
4646

47+
// Set synchronous=FULL to ensure writes are immediately persisted to disk
48+
// This prevents data loss when connections are disposed quickly (CLI scenario)
49+
using (var pragmaCmd = this._connection.CreateCommand())
50+
{
51+
pragmaCmd.CommandText = "PRAGMA synchronous=FULL;";
52+
await pragmaCmd.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false);
53+
}
54+
4755
// Create FTS5 virtual table if it doesn't exist
4856
// BREAKING CHANGE: New schema indexes title, description, content separately
4957
// This enables field-specific search (e.g., title:kubernetes vs content:kubernetes)
@@ -201,6 +209,7 @@ public async Task ClearAsync(CancellationToken cancellationToken = default)
201209

202210
/// <summary>
203211
/// Disposes the database connection.
212+
/// Ensures all pending writes are flushed to disk before closing.
204213
/// </summary>
205214
public void Dispose()
206215
{
@@ -209,8 +218,27 @@ public void Dispose()
209218
return;
210219
}
211220

212-
this._connection?.Dispose();
213-
this._connection = null;
221+
// Flush any pending writes before closing the connection
222+
// SQLite needs explicit close to ensure writes are persisted
223+
if (this._connection != null)
224+
{
225+
try
226+
{
227+
// Execute a checkpoint to flush WAL to disk (if WAL mode is enabled)
228+
using var cmd = this._connection.CreateCommand();
229+
cmd.CommandText = "PRAGMA wal_checkpoint(TRUNCATE);";
230+
cmd.ExecuteNonQuery();
231+
}
232+
catch (Exception ex)
233+
{
234+
this._logger.LogWarning(ex, "Failed to checkpoint WAL during FTS index disposal");
235+
}
236+
237+
this._connection.Close();
238+
this._connection.Dispose();
239+
this._connection = null;
240+
}
241+
214242
this._disposed = true;
215243
}
216244
}

src/Core/Storage/ContentStorageService.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -656,8 +656,18 @@ private async Task ExecuteIndexStepAsync(OperationRecord operation, string index
656656
return;
657657
}
658658

659-
// Update the search index
660-
await searchIndex.IndexAsync(operation.ContentId, content.Content, cancellationToken).ConfigureAwait(false);
659+
// Update the search index with title, description, and content
660+
// Use the 4-parameter signature to properly index all fields
661+
if (searchIndex is IFtsIndex ftsIndex)
662+
{
663+
await ftsIndex.IndexAsync(operation.ContentId, content.Title, content.Description, content.Content, cancellationToken).ConfigureAwait(false);
664+
}
665+
else
666+
{
667+
// Fallback for non-FTS indexes (vector, graph, etc.) - use legacy 2-parameter signature
668+
await searchIndex.IndexAsync(operation.ContentId, content.Content, cancellationToken).ConfigureAwait(false);
669+
}
670+
661671
this._logger.LogDebug("Indexed content {ContentId} in search index {IndexId}", operation.ContentId, indexId);
662672
}
663673

src/Main/CLI/Commands/BaseCommand.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ protected ContentService CreateContentService(NodeConfig node, bool readonlyMode
133133
// Create storage service with search indexes
134134
var storage = new ContentStorageService(context, cuidGenerator, logger, searchIndexes);
135135

136-
// Create and return content service
137-
return new ContentService(storage, node.Id);
136+
// Create and return content service, passing search indexes for proper disposal
137+
return new ContentService(storage, node.Id, searchIndexes);
138138
}
139139

140140
/// <summary>

src/Main/CLI/Commands/DeleteCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public override async Task<int> ExecuteAsync(
5454
try
5555
{
5656
var (config, node, formatter) = this.Initialize(settings);
57-
var service = this.CreateContentService(node);
57+
using var service = this.CreateContentService(node);
5858

5959
// Delete is idempotent - no error if not found
6060
var result = await service.DeleteAsync(settings.Id, CancellationToken.None).ConfigureAwait(false);

src/Main/CLI/Commands/GetCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public override async Task<int> ExecuteAsync(
5959
try
6060
{
6161
var (config, node, formatter) = this.Initialize(settings);
62-
var service = this.CreateContentService(node, readonlyMode: true);
62+
using var service = this.CreateContentService(node, readonlyMode: true);
6363

6464
var result = await service.GetAsync(settings.Id, CancellationToken.None).ConfigureAwait(false);
6565

src/Main/CLI/Commands/ListCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public override async Task<int> ExecuteAsync(
6666
try
6767
{
6868
var (config, node, formatter) = this.Initialize(settings);
69-
var service = this.CreateContentService(node, readonlyMode: true);
69+
using var service = this.CreateContentService(node, readonlyMode: true);
7070

7171
// Get total count
7272
var totalCount = await service.CountAsync(CancellationToken.None).ConfigureAwait(false);

src/Main/CLI/Commands/SearchCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ private SearchService CreateSearchService()
454454
foreach (var (nodeId, nodeConfig) in this.Config.Nodes)
455455
{
456456
// Create ContentService for this node
457-
var contentService = this.CreateContentService(nodeConfig, readonlyMode: true);
457+
using var contentService = this.CreateContentService(nodeConfig, readonlyMode: true);
458458

459459
// Get FTS index from search indexes
460460
var ftsIndex = Services.SearchIndexFactory.CreateFtsIndex(nodeConfig.SearchIndexes);

src/Main/CLI/Commands/UpsertCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public override async Task<int> ExecuteAsync(
7676
try
7777
{
7878
var (config, node, formatter) = this.Initialize(settings);
79-
var service = this.CreateContentService(node);
79+
using var service = this.CreateContentService(node);
8080

8181
// Parse tags if provided
8282
var tags = string.IsNullOrWhiteSpace(settings.Tags)

src/Main/Services/ContentService.cs

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Copyright (c) Microsoft. All rights reserved.
2+
using KernelMemory.Core.Search;
23
using KernelMemory.Core.Storage;
34
using KernelMemory.Core.Storage.Models;
45

@@ -7,21 +8,26 @@ namespace KernelMemory.Main.Services;
78
/// <summary>
89
/// Business logic layer for content operations.
910
/// Wraps IContentStorage and provides CLI-friendly interface.
11+
/// Implements IDisposable to ensure search indexes are properly disposed.
1012
/// </summary>
11-
public class ContentService
13+
public sealed class ContentService : IDisposable
1214
{
1315
private readonly IContentStorage _storage;
1416
private readonly string _nodeId;
17+
private readonly IReadOnlyDictionary<string, ISearchIndex>? _searchIndexes;
18+
private bool _disposed;
1519

1620
/// <summary>
1721
/// Initializes a new instance of ContentService.
1822
/// </summary>
1923
/// <param name="storage">The content storage implementation.</param>
2024
/// <param name="nodeId">The node ID this service operates on.</param>
21-
public ContentService(IContentStorage storage, string nodeId)
25+
/// <param name="searchIndexes">Optional search indexes to dispose when done.</param>
26+
public ContentService(IContentStorage storage, string nodeId, IReadOnlyDictionary<string, ISearchIndex>? searchIndexes = null)
2227
{
2328
this._storage = storage;
2429
this._nodeId = nodeId;
30+
this._searchIndexes = searchIndexes;
2531
}
2632

2733
/// <summary>
@@ -88,4 +94,30 @@ public async Task<long> CountAsync(CancellationToken cancellationToken = default
8894
{
8995
return await this._storage.CountAsync(cancellationToken).ConfigureAwait(false);
9096
}
97+
98+
/// <summary>
99+
/// Disposes the service and underlying search indexes.
100+
/// </summary>
101+
public void Dispose()
102+
{
103+
if (this._disposed)
104+
{
105+
return;
106+
}
107+
108+
// Dispose all search indexes (e.g., SqliteFtsIndex connections)
109+
if (this._searchIndexes != null)
110+
{
111+
foreach (var index in this._searchIndexes.Values)
112+
{
113+
if (index is IDisposable disposable)
114+
{
115+
disposable.Dispose();
116+
}
117+
}
118+
}
119+
120+
this._disposed = true;
121+
GC.SuppressFinalize(this);
122+
}
91123
}

0 commit comments

Comments
 (0)