fix: address PR review comments for embedding cache

Changes:
- Allow null text in EmbeddingCacheKey.Create(), normalize to empty string
- Remove unnecessary indexes (idx_timestamp, idx_provider, idx_model)
- Rename 'normalized' column to 'is_normalized' for clarity
- Rename 'timestamp' column to 'created_at' for clarity
- Simplify timestamp documentation (debugging only, no invalidation)

Author: dluc

docs

@@ -0,0 +1,13 @@
+{
+  "version": 1,
+  "isRoot": true,
+  "tools": {
+    "dotnet-reportgenerator-globaltool": {
+      "version": "5.5.0",
+      "commands": [
+        "reportgenerator"
+      ],
+      "rollForward": false
+    }
+  }
+}

dotnet-tools.json

@@ -25,6 +25,7 @@ public sealed class CachedEmbedding
 
     /// <summary>
     /// Timestamp when this embedding was stored in the cache.
+    /// Useful for debugging and diagnostics.
     /// </summary>
     public required DateTimeOffset Timestamp { get; init; }
 }

src/Core/Embeddings/Cache/CachedEmbedding.cs

@@ -53,20 +53,22 @@ public sealed class EmbeddingCacheKey
     /// <param name="isNormalized">Whether vectors are normalized.</param>
     /// <param name="text">The text to hash.</param>
     /// <returns>A new EmbeddingCacheKey instance.</returns>
-    /// <exception cref="ArgumentNullException">When provider, model, or text is null.</exception>
+    /// <exception cref="ArgumentNullException">When provider or model is null.</exception>
     /// <exception cref="ArgumentOutOfRangeException">When vectorDimensions is less than 1.</exception>
     public static EmbeddingCacheKey Create(
         string provider,
         string model,
         int vectorDimensions,
         bool isNormalized,
-        string text)
+        string? text)
     {
         ArgumentNullException.ThrowIfNull(provider, nameof(provider));
         ArgumentNullException.ThrowIfNull(model, nameof(model));
-        ArgumentNullException.ThrowIfNull(text, nameof(text));
         ArgumentOutOfRangeException.ThrowIfLessThan(vectorDimensions, 1, nameof(vectorDimensions));
 
+        // Normalize null to empty string for consistent hashing
+        text ??= string.Empty;
+
         return new EmbeddingCacheKey
         {
             Provider = provider,

src/Core/Embeddings/Cache/EmbeddingCacheKey.cs

@@ -18,30 +18,27 @@ CREATE TABLE IF NOT EXISTS embeddings_cache (
             provider TEXT NOT NULL,
             model TEXT NOT NULL,
             dimensions INTEGER NOT NULL,
-            normalized INTEGER NOT NULL,
+            is_normalized INTEGER NOT NULL,
             text_length INTEGER NOT NULL,
             text_hash TEXT NOT NULL,
             vector BLOB NOT NULL,
             token_count INTEGER NULL,
-            timestamp TEXT NOT NULL,
-            PRIMARY KEY (provider, model, dimensions, normalized, text_hash)
+            created_at TEXT NOT NULL,
+            PRIMARY KEY (provider, model, dimensions, is_normalized, text_hash)
         );
-        CREATE INDEX IF NOT EXISTS idx_timestamp ON embeddings_cache(timestamp);
-        CREATE INDEX IF NOT EXISTS idx_provider ON embeddings_cache(provider);
-        CREATE INDEX IF NOT EXISTS idx_model ON embeddings_cache(provider, model);
         """;
 
     private const string SelectSql = """
-        SELECT vector, token_count, timestamp FROM embeddings_cache
+        SELECT vector, token_count, created_at FROM embeddings_cache
         WHERE provider = @provider AND model = @model AND dimensions = @dimensions
-        AND normalized = @normalized AND text_hash = @textHash
+        AND is_normalized = @isNormalized AND text_hash = @textHash
         """;
 
     private const string UpsertSql = """
-        INSERT INTO embeddings_cache (provider, model, dimensions, normalized, text_length, text_hash, vector, token_count, timestamp)
-        VALUES (@provider, @model, @dimensions, @normalized, @textLength, @textHash, @vector, @tokenCount, @timestamp)
-        ON CONFLICT(provider, model, dimensions, normalized, text_hash)
-        DO UPDATE SET vector = @vector, token_count = @tokenCount, timestamp = @timestamp
+        INSERT INTO embeddings_cache (provider, model, dimensions, is_normalized, text_length, text_hash, vector, token_count, created_at)
+        VALUES (@provider, @model, @dimensions, @isNormalized, @textLength, @textHash, @vector, @tokenCount, @createdAt)
+        ON CONFLICT(provider, model, dimensions, is_normalized, text_hash)
+        DO UPDATE SET vector = @vector, token_count = @tokenCount, created_at = @createdAt
         """;
 
     private readonly SqliteConnection _connection;
@@ -122,7 +119,7 @@ public SqliteEmbeddingCache(string dbPath, CacheModes mode, ILogger<SqliteEmbedd
             command.Parameters.AddWithValue("@provider", key.Provider);
             command.Parameters.AddWithValue("@model", key.Model);
             command.Parameters.AddWithValue("@dimensions", key.VectorDimensions);
-            command.Parameters.AddWithValue("@normalized", key.IsNormalized ? 1 : 0);
+            command.Parameters.AddWithValue("@isNormalized", key.IsNormalized ? 1 : 0);
             command.Parameters.AddWithValue("@textHash", key.TextHash);
 
             var reader = await command.ExecuteReaderAsync(ct).ConfigureAwait(false);
@@ -139,7 +136,7 @@ public SqliteEmbeddingCache(string dbPath, CacheModes mode, ILogger<SqliteEmbedd
                 var vector = BytesToFloatArray(vectorBlob);
 
                 int? tokenCount = reader["token_count"] == DBNull.Value ? null : Convert.ToInt32(reader["token_count"], CultureInfo.InvariantCulture);
-                var timestamp = DateTimeOffset.Parse((string)reader["timestamp"], CultureInfo.InvariantCulture);
+                var createdAt = DateTimeOffset.Parse((string)reader["created_at"], CultureInfo.InvariantCulture);
 
                 this._logger.LogTrace("Cache hit for {Provider}/{Model} hash: {HashPrefix}..., dimensions: {Dimensions}",
                     key.Provider, key.Model, key.TextHash[..Math.Min(16, key.TextHash.Length)], vector.Length);
@@ -148,7 +145,7 @@ public SqliteEmbeddingCache(string dbPath, CacheModes mode, ILogger<SqliteEmbedd
                 {
                     Vector = vector,
                     TokenCount = tokenCount,
-                    Timestamp = timestamp
+                    Timestamp = createdAt
                 };
             }
         }
@@ -167,7 +164,7 @@ public async Task StoreAsync(EmbeddingCacheKey key, float[] vector, int? tokenCo
         }
 
         var vectorBlob = FloatArrayToBytes(vector);
-        var timestamp = DateTimeOffset.UtcNow.ToString("o", CultureInfo.InvariantCulture);
+        var createdAt = DateTimeOffset.UtcNow.ToString("o", CultureInfo.InvariantCulture);
 
         var command = this._connection.CreateCommand();
         await using (command.ConfigureAwait(false))
@@ -176,12 +173,12 @@ public async Task StoreAsync(EmbeddingCacheKey key, float[] vector, int? tokenCo
             command.Parameters.AddWithValue("@provider", key.Provider);
             command.Parameters.AddWithValue("@model", key.Model);
             command.Parameters.AddWithValue("@dimensions", key.VectorDimensions);
-            command.Parameters.AddWithValue("@normalized", key.IsNormalized ? 1 : 0);
+            command.Parameters.AddWithValue("@isNormalized", key.IsNormalized ? 1 : 0);
             command.Parameters.AddWithValue("@textLength", key.TextLength);
             command.Parameters.AddWithValue("@textHash", key.TextHash);
             command.Parameters.AddWithValue("@vector", vectorBlob);
             command.Parameters.AddWithValue("@tokenCount", tokenCount.HasValue ? tokenCount.Value : DBNull.Value);
-            command.Parameters.AddWithValue("@timestamp", timestamp);
+            command.Parameters.AddWithValue("@createdAt", createdAt);
 
             await command.ExecuteNonQueryAsync(ct).ConfigureAwait(false);
 

src/Core/Embeddings/Cache/SqliteEmbeddingCache.cs

@@ -189,11 +189,14 @@ public void Create_WithLongText_ShouldCaptureCorrectLength()
     }
 
     [Fact]
-    public void Create_WithNullText_ShouldThrowArgumentNullException()
+    public void Create_WithNullText_ShouldNormalizeToEmptyString()
     {
-        // Arrange & Act & Assert
-        Assert.Throws<ArgumentNullException>(() =>
-            EmbeddingCacheKey.Create("OpenAI", "model", 1536, true, null!));
+        // Arrange & Act
+        var key = EmbeddingCacheKey.Create("OpenAI", "model", 1536, true, null);
+
+        // Assert - null should be normalized to empty string
+        Assert.Equal(0, key.TextLength);
+        Assert.NotEmpty(key.TextHash); // Hash of empty string
     }
 
     [Fact]