From 946ae1a5a5976ae2d3ff6145527c6abbd9f38207 Mon Sep 17 00:00:00 2001 From: Thorsten Sommer Date: Fri, 18 Sep 2026 18:04:43 +0200 Subject: [PATCH] Compare each side of a data source edit with its own provider --- .../Tools/DataSourceReindexWarning.cs | 15 +++- .../Tools/Services/EmbeddingChangeImpact.cs | 15 +++- app/Tests/Tools/EmbeddingChangeImpactTests.cs | 84 +++++++++++++++++-- 3 files changed, 98 insertions(+), 16 deletions(-) diff --git a/app/MindWork AI Studio/Tools/DataSourceReindexWarning.cs b/app/MindWork AI Studio/Tools/DataSourceReindexWarning.cs index 5f8908fd..9f896de3 100644 --- a/app/MindWork AI Studio/Tools/DataSourceReindexWarning.cs +++ b/app/MindWork AI Studio/Tools/DataSourceReindexWarning.cs @@ -72,14 +72,23 @@ public static class DataSourceReindexWarning IInternalDataSource before, IInternalDataSource after, CancellationToken token = default) { // Without a provider nothing is embedded at all, so nothing can be lost: - if (!DataSourceEmbeddingProviders.TryResolve(settingsManager, after, out var embeddingProvider)) + if (!DataSourceEmbeddingProviders.TryResolve(settingsManager, after, out var afterProvider)) return true; - if (!EmbeddingChangeImpact.AffectsStoredIndex(embeddingProvider, before, after)) + // + // The provider a data source points at today may be gone -- somebody deleted it, and this + // edit is how the source is put back to work. Standing in for it with NONE gives a signature + // of its own, so that edit is asked about as well, which is right: what is stored was made by + // a provider nobody can reach any more. + // + DataSourceEmbeddingProviders.TryResolve(settingsManager, before, out var resolvedBeforeProvider); + var beforeProvider = resolvedBeforeProvider ?? EmbeddingProvider.NONE; + + if (!EmbeddingChangeImpact.AffectsStoredIndex(before, beforeProvider, after, afterProvider)) return true; var affected = await embeddingService.GetDataSourcesWithStoredIndexAsync([after], token); - return await ConfirmAsync(dialogService, affected, !embeddingProvider.IsSelfHosted); + return await ConfirmAsync(dialogService, affected, !afterProvider.IsSelfHosted); } /// diff --git a/app/MindWork AI Studio/Tools/Services/EmbeddingChangeImpact.cs b/app/MindWork AI Studio/Tools/Services/EmbeddingChangeImpact.cs index 2f552c91..4e2b925f 100644 --- a/app/MindWork AI Studio/Tools/Services/EmbeddingChangeImpact.cs +++ b/app/MindWork AI Studio/Tools/Services/EmbeddingChangeImpact.cs @@ -28,13 +28,20 @@ internal static class EmbeddingChangeImpact /// /// Whether an edited data source invalidates what is stored for it. /// - /// The embedding provider, which the edit leaves alone. + /// + /// Each side is asked with the embedding provider it points at, never both with the same one. A + /// data source carries only the id of its provider, while the signature carries what that provider + /// is, so comparing both sides against one of them would report a changed embedding as no change + /// at all -- and the next indexing run would then rebuild everything unannounced. + /// /// The data source as it is stored. + /// The embedding provider it points at today. /// The data source as it would be stored. + /// The embedding provider it would point at. /// True when the stored index would be discarded. - public static bool AffectsStoredIndex(EmbeddingProvider embeddingProvider, IDataSource before, IDataSource after) => + public static bool AffectsStoredIndex(IDataSource before, EmbeddingProvider beforeProvider, IDataSource after, EmbeddingProvider afterProvider) => !string.Equals( - DataSourceEmbeddingService.BuildEmbeddingSignature(before, embeddingProvider), - DataSourceEmbeddingService.BuildEmbeddingSignature(after, embeddingProvider), + DataSourceEmbeddingService.BuildEmbeddingSignature(before, beforeProvider), + DataSourceEmbeddingService.BuildEmbeddingSignature(after, afterProvider), StringComparison.Ordinal); } \ No newline at end of file diff --git a/app/Tests/Tools/EmbeddingChangeImpactTests.cs b/app/Tests/Tools/EmbeddingChangeImpactTests.cs index e9301df1..dcea6626 100644 --- a/app/Tests/Tools/EmbeddingChangeImpactTests.cs +++ b/app/Tests/Tools/EmbeddingChangeImpactTests.cs @@ -94,8 +94,8 @@ public sealed class EmbeddingChangeImpactTests Assert.Multiple(() => { - Assert.That(EmbeddingChangeImpact.AffectsStoredIndex(embeddingProvider, stored, stored with { MaxChunkTokenLength = 256 }), Is.True, "Other chunk boundaries mean other vectors."); - Assert.That(EmbeddingChangeImpact.AffectsStoredIndex(embeddingProvider, stored, stored with { ChunkOverlapTokenLength = 50 }), Is.True, "Another overlap changes what every chunk starts with."); + Assert.That(EditKeepingTheProvider(embeddingProvider, stored, stored with { MaxChunkTokenLength = 256 }), Is.True, "Other chunk boundaries mean other vectors."); + Assert.That(EditKeepingTheProvider(embeddingProvider, stored, stored with { ChunkOverlapTokenLength = 50 }), Is.True, "Another overlap changes what every chunk starts with."); }); } @@ -117,12 +117,12 @@ public sealed class EmbeddingChangeImpactTests Assert.Multiple(() => { Assert.That( - EmbeddingChangeImpact.AffectsStoredIndex(embeddingProvider, followingTheProvider, followingTheProvider with { MaxChunkTokenLength = 8192 }), + EditKeepingTheProvider(embeddingProvider, followingTheProvider, followingTheProvider with { MaxChunkTokenLength = 8192 }), Is.False, "The provider limit typed into the field is the cut the data source already had."); Assert.That( - EmbeddingChangeImpact.AffectsStoredIndex(embeddingProvider, followingTheProvider, followingTheProvider with { MaxChunkTokenLength = 4096 }), + EditKeepingTheProvider(embeddingProvider, followingTheProvider, followingTheProvider with { MaxChunkTokenLength = 4096 }), Is.True, "Anything below the provider limit really does cut the text elsewhere."); }); @@ -142,7 +142,7 @@ public sealed class EmbeddingChangeImpactTests var stored = StoredDataSource() with { MaxChunkTokenLength = 512, ChunkOverlapTokenLength = 600 }; Assert.That( - EmbeddingChangeImpact.AffectsStoredIndex(embeddingProvider, stored, stored with { ChunkOverlapTokenLength = 700 }), + EditKeepingTheProvider(embeddingProvider, stored, stored with { ChunkOverlapTokenLength = 700 }), Is.False, "Both overlaps are capped to the chunk size, so the text is cut identically."); } @@ -155,13 +155,76 @@ public sealed class EmbeddingChangeImpactTests Assert.Multiple(() => { - Assert.That(EmbeddingChangeImpact.AffectsStoredIndex(embeddingProvider, stored, stored with { Name = "Another name" }), Is.False, "The name is how the data source is offered, not how it was read."); - Assert.That(EmbeddingChangeImpact.AffectsStoredIndex(embeddingProvider, stored, stored with { Description = "Another description" }), Is.False, "The description is there for the agent which picks data sources."); - Assert.That(EmbeddingChangeImpact.AffectsStoredIndex(embeddingProvider, stored, stored with { MaxMatches = 42 }), Is.False, "How many matches an answer may use is decided per query."); - Assert.That(EmbeddingChangeImpact.AffectsStoredIndex(embeddingProvider, stored, stored with { ConfidenceLevel = ConfidenceLevel.HIGH }), Is.False, "The confidence level is enforced live on every request and changes no vector."); + Assert.That(EditKeepingTheProvider(embeddingProvider, stored, stored with { Name = "Another name" }), Is.False, "The name is how the data source is offered, not how it was read."); + Assert.That(EditKeepingTheProvider(embeddingProvider, stored, stored with { Description = "Another description" }), Is.False, "The description is there for the agent which picks data sources."); + Assert.That(EditKeepingTheProvider(embeddingProvider, stored, stored with { MaxMatches = 42 }), Is.False, "How many matches an answer may use is decided per query."); + Assert.That(EditKeepingTheProvider(embeddingProvider, stored, stored with { ConfidenceLevel = ConfidenceLevel.HIGH }), Is.False, "The confidence level is enforced live on every request and changes no vector."); }); } + /// + /// Checks what changing the embedding of a data source costs. + /// + /// + /// This is why each side has to be asked with its own provider: a data source carries only the id + /// of its embedding provider, and that id is nowhere in the signature. Asking both sides with the + /// same provider would call this edit harmless, while the next indexing run throws everything away. + /// + [Test] + public void ChangingTheEmbeddingOfADataSourceDropsItsStoredIndex() + { + var storedProvider = StoredEmbeddingProvider(); + var anotherProvider = AnotherEmbeddingProvider(); + var stored = StoredDataSource(); + var moved = stored with { EmbeddingId = anotherProvider.Id }; + + Assert.That( + EmbeddingChangeImpact.AffectsStoredIndex(stored, storedProvider, moved, anotherProvider), + Is.True, + "Another embedding provider means another vector space, so nothing stored survives it."); + } + + /// + /// Putting a data source back to work after its provider was deleted is a rebuild as well. + /// + /// + /// The provider a data source points at can be gone. What is stored was made by it, so pointing + /// the source at any provider at all discards that -- and nobody may be surprised by it. + /// + [Test] + public void RepointingADataSourceWhoseProviderIsGoneDropsItsStoredIndex() + { + var stored = StoredDataSource(); + var anotherProvider = AnotherEmbeddingProvider(); + + Assert.That( + EmbeddingChangeImpact.AffectsStoredIndex(stored, EmbeddingProvider.NONE, stored with { EmbeddingId = anotherProvider.Id }, anotherProvider), + Is.True, + "A provider which cannot be resolved stands in as NONE, which is a signature of its own."); + } + + [Test] + public void KeepingTheEmbeddingKeepsTheStoredIndex() + { + var storedProvider = StoredEmbeddingProvider(); + var stored = StoredDataSource(); + + Assert.That( + EmbeddingChangeImpact.AffectsStoredIndex(stored, storedProvider, stored with { Name = "Another name" }, storedProvider with { Name = "Renamed provider" }), + Is.False, + "Neither name reaches a vector, and the data source still points at the same provider."); + } + + /// + /// Asks the question for an edit which leaves the embedding provider of the data source alone. + /// + /// The provider both sides point at. + /// The data source as it is stored. + /// The data source as it would be stored. + /// True when the stored index would be discarded. + private static bool EditKeepingTheProvider(EmbeddingProvider embeddingProvider, IDataSource before, IDataSource after) => + EmbeddingChangeImpact.AffectsStoredIndex(before, embeddingProvider, after, embeddingProvider); + private static DataSourceLocalDirectory StoredDataSource() => new() { Num = 1, @@ -178,4 +241,7 @@ public sealed class EmbeddingChangeImpactTests private static EmbeddingProvider StoredEmbeddingProvider() => new(1, "b0a4c4d2-1f3e-4f0a-8c9d-5a6b7c8d9e01", "Test embeddings", LLMProviders.OPEN_AI, new("text-embedding-3-small", "text-embedding-3-small")); + + private static EmbeddingProvider AnotherEmbeddingProvider() => + new(2, "c1b5d5e3-2a4f-4b1b-9dae-6b7c8d9e0f12", "Other embeddings", LLMProviders.MISTRAL, new("mistral-embed", "mistral-embed")); } \ No newline at end of file