diff --git a/src/Netclaw.Actors.Tests/Memory/CurationPromptBuilderTests.cs b/src/Netclaw.Actors.Tests/Memory/CurationPromptBuilderTests.cs index f2cb5c69c..aa756ba85 100644 --- a/src/Netclaw.Actors.Tests/Memory/CurationPromptBuilderTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/CurationPromptBuilderTests.cs @@ -48,6 +48,8 @@ public void ParseResponse_parses_CONSOLIDATE_with_multiple_ids() Assert.Equal(2, decision.ConsolidationTargetIds.Count); Assert.Equal("doc-abc123", decision.ConsolidationTargetIds[0]); Assert.Equal("doc-def456", decision.ConsolidationTargetIds[1]); + // First listed id doubles as the primary write target for the collapse write. + Assert.Equal("doc-abc123", decision.TargetDocumentId); } [Fact] diff --git a/src/Netclaw.Actors.Tests/Memory/CurationRulesEvaluatorTests.cs b/src/Netclaw.Actors.Tests/Memory/CurationRulesEvaluatorTests.cs index 1e0eac47e..212c82dac 100644 --- a/src/Netclaw.Actors.Tests/Memory/CurationRulesEvaluatorTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/CurationRulesEvaluatorTests.cs @@ -164,6 +164,8 @@ public void Evaluate_returns_Consolidate_for_fuzzy_match_with_high_overlap() Assert.Equal(CurationDecisionKind.Consolidate, decision.Kind); Assert.NotNull(decision.ConsolidationTargetIds); Assert.Contains("doc-456", decision.ConsolidationTargetIds); + // Best match doubles as the primary write target for the collapse write. + Assert.Equal("doc-456", decision.TargetDocumentId); } // ── Fuzzy match + ambiguous overlap -> Ambiguous ──────────────── diff --git a/src/Netclaw.Actors.Tests/Memory/MemoryCurationEvaluatorParityTests.cs b/src/Netclaw.Actors.Tests/Memory/MemoryCurationEvaluatorParityTests.cs index cc7012240..5d036ddd6 100644 --- a/src/Netclaw.Actors.Tests/Memory/MemoryCurationEvaluatorParityTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/MemoryCurationEvaluatorParityTests.cs @@ -4,6 +4,7 @@ // // ----------------------------------------------------------------------- using Akka.Event; +using Microsoft.Data.Sqlite; using Microsoft.Extensions.AI; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -122,6 +123,42 @@ await SeedDocumentAsync( Assert.Equal(CurationDecisionKind.Consolidate, fromActor.Kind); Assert.NotNull(fromActor.ConsolidationTargetIds); Assert.Contains("doc-akka", fromActor.ConsolidationTargetIds!); + // The primary write target: ApplyDecisionAsync sets MemoryId from this so the + // store takes the explicit-target overwrite path (collapse), not dedup-append. + Assert.Equal("doc-akka", fromActor.TargetDocumentId); + } + + // ── Consolidate end-to-end: existing document is REPLACED, not appended ── + + [Fact] + public async Task ConsolidateDecision_appliedThroughStore_replacesExistingDocumentInsteadOfAppending() + { + var ct = TestContext.Current.CancellationToken; + await _store.InitializeAsync(ct); + await SeedDocumentAsync( + "akka-net-latest-release", "doc-akka", "Akka.NET latest release version is 1.5.62", freshnessAtMs: 1000, ct); + + // One extra token ("now") keeps Jaccard overlap at 0.9 (> 0.8 threshold) while making + // the proposal body distinct from the seed, so replacement vs append is observable. + var operation = MakeOperation( + "akka-net-release", "Akka.NET latest release version is now 1.5.62", freshnessAtMs: 2000); + + var evaluator = new MemoryCurationEvaluator(_store, (ILoggingAdapter)NoLogger.Instance); + var decision = await evaluator.EvaluateAsync(operation, TestSessionId, ct); + Assert.Equal(CurationDecisionKind.Consolidate, decision.Kind); + + var writeOp = await evaluator.ApplyDecisionAsync(operation, decision, ct); + Assert.NotNull(writeOp); + Assert.Equal("doc-akka", writeOp!.MemoryId); + + await _store.ApplyInlineCurationBatchAsync([writeOp], ct); + + var (body, updateSemantics) = await ReadDocumentBodyAndSemanticsAsync("doc-akka", ct); + // Collapse semantics: the near-duplicate body is replaced outright — no dated + // append marker, no doubled content. + Assert.Equal("Akka.NET latest release version is now 1.5.62", body); + Assert.DoesNotContain("_[merged", body); + Assert.Equal("merge-document", updateSemantics); } // ── gray zone (0.4-0.8), no LLM -> deterministic auto-resolve ─── @@ -329,6 +366,18 @@ private static SQLiteMemoryCurationOperation MakeOperation( FreshnessAtMs: freshnessAtMs, ExpiresAtMs: null); + private async Task<(string Body, string UpdateSemantics)> ReadDocumentBodyAndSemanticsAsync(string documentId, CancellationToken ct) + { + await using var conn = new SqliteConnection($"Data Source={_dbPath}"); + await conn.OpenAsync(ct); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = "SELECT markdown_body, update_semantics FROM memory_documents WHERE document_id = $id"; + cmd.Parameters.AddWithValue("$id", documentId); + await using var reader = await cmd.ExecuteReaderAsync(ct); + Assert.True(await reader.ReadAsync(ct), $"Expected document row '{documentId}' to exist."); + return (reader.GetString(0), reader.GetString(1)); + } + private static void AssertSameDecision(CurationDecision expected, CurationDecision actual) { Assert.Equal(expected.Kind, actual.Kind); diff --git a/src/Netclaw.Actors.Tests/Memory/SqliteMemoryStoreDedupCollisionTests.cs b/src/Netclaw.Actors.Tests/Memory/SqliteMemoryStoreDedupCollisionTests.cs new file mode 100644 index 000000000..6ea367984 --- /dev/null +++ b/src/Netclaw.Actors.Tests/Memory/SqliteMemoryStoreDedupCollisionTests.cs @@ -0,0 +1,297 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Microsoft.Data.Sqlite; +using Microsoft.Extensions.Time.Testing; +using Netclaw.Actors.Memory; +using Netclaw.Configuration; +using Xunit; + +namespace Netclaw.Actors.Tests.Memory; + +/// +/// Regression coverage for the anchor-based dedup collision bug: a curation Create +/// decision (or any no-MemoryId, merge-document proposal) that lands on an anchor which +/// already has a document used to be written via a blind INSERT ... ON CONFLICT DO +/// UPDATE, silently overwriting the existing row's title/body/classification with the +/// new proposal's raw values. The rules tier () can +/// legitimately emit Create for "different topic, similar anchor name" (fuzzy anchor match, +/// low content overlap), so this collision is a real, reachable production path — not a +/// theoretical one (audit: 88 silent overwrites/14 days, including a destroyed +/// LLM-merged memory). Both batch appliers (, +/// used by the daemon checkpoint worker, and , +/// used by the inline per-session actor) share the identical dedup logic and must both +/// preserve the existing document by appending instead of overwriting. Also covers the +/// idempotency guard on that path: a collision whose content is already present verbatim +/// is a logged no-op (curation_dedup_duplicate_skipped), not a repeated append. +/// +public sealed class SqliteMemoryStoreDedupCollisionTests : IAsyncDisposable +{ + private readonly string _baseDir = Path.Combine(Path.GetTempPath(), "netclaw-dedup-collision-tests", Guid.NewGuid().ToString("N")); + private readonly string _dbPath; + private readonly FakeTimeProvider _timeProvider; + private readonly SQLiteMemoryStore _store; + + public SqliteMemoryStoreDedupCollisionTests() + { + Directory.CreateDirectory(_baseDir); + _dbPath = Path.Combine(_baseDir, "netclaw.db"); + _timeProvider = new FakeTimeProvider(DateTimeOffset.Parse("2026-06-01T09:00:00Z")); + _store = new SQLiteMemoryStore(_dbPath, _timeProvider); + } + + public async ValueTask DisposeAsync() => await SqliteTempDirectoryCleanup.TryDeleteDirectoryAsync(_baseDir); + + [Fact] + public async Task ApplyCurationBatchAsync_CreateCollidesWithExistingAnchorDocument_AppendsInsteadOfOverwriting() + { + var ct = TestContext.Current.CancellationToken; + await _store.InitializeAsync(ct); + + var createdAt = await SeedExistingDocumentAsync("doc-existing", "project-quasar", ct); + + // Time advances between the seed write and the colliding batch, so updated_at + // must move while created_at (excluded from the ON CONFLICT SET clause) does not. + _timeProvider.Advance(TimeSpan.FromHours(3)); + + await _store.ApplyCurationBatchAsync( + "cp-collision-1", + [MakeColliderOperation("project-quasar")], + ct); + + var row = await ReadDocumentAsync("doc-existing", ct); + + Assert.Contains("Quasar uses a Postgres 15 read replica for reporting.", row.MarkdownBody); + Assert.Contains("Quasar's on-call rotation moved to PagerDuty last sprint.", row.MarkdownBody); + Assert.Contains("---", row.MarkdownBody); + Assert.Matches(@"_\[merged \d{4}-\d{2}-\d{2}\]_", row.MarkdownBody); + + // Identity/classification of the existing row must survive the collision verbatim. + Assert.Equal("Project Quasar datastore", row.Title); + Assert.Equal(MemorySensitivity.Secret.ToWireValue(), row.Sensitivity); + Assert.Equal(TrustAudience.Personal.ToWireValue(), row.Audience); + Assert.Equal(TrustBoundary.PersonalValue, row.Boundary); + + Assert.Equal(MemoryUpdateSemantics.AppendDocument.ToWireValue(), row.UpdateSemantics); + Assert.Equal(createdAt, row.CreatedAtMs); + Assert.True(row.UpdatedAtMs > createdAt); + } + + [Fact] + public async Task ApplyInlineCurationBatchAsync_CreateCollidesWithExistingAnchorDocument_AppendsInsteadOfOverwriting() + { + var ct = TestContext.Current.CancellationToken; + await _store.InitializeAsync(ct); + + var createdAt = await SeedExistingDocumentAsync("doc-existing-inline", "project-nebula", ct); + + _timeProvider.Advance(TimeSpan.FromHours(3)); + + await _store.ApplyInlineCurationBatchAsync( + [MakeColliderOperation("project-nebula")], + ct); + + var row = await ReadDocumentAsync("doc-existing-inline", ct); + + Assert.Contains("Quasar uses a Postgres 15 read replica for reporting.", row.MarkdownBody); + Assert.Contains("Quasar's on-call rotation moved to PagerDuty last sprint.", row.MarkdownBody); + Assert.Matches(@"_\[merged \d{4}-\d{2}-\d{2}\]_", row.MarkdownBody); + + Assert.Equal("Project Quasar datastore", row.Title); + Assert.Equal(MemorySensitivity.Secret.ToWireValue(), row.Sensitivity); + Assert.Equal(TrustAudience.Personal.ToWireValue(), row.Audience); + Assert.Equal(TrustBoundary.PersonalValue, row.Boundary); + + Assert.Equal(MemoryUpdateSemantics.AppendDocument.ToWireValue(), row.UpdateSemantics); + Assert.Equal(createdAt, row.CreatedAtMs); + Assert.True(row.UpdatedAtMs > createdAt); + } + + [Fact] + public async Task ApplyCurationBatchAsync_CollisionWithVerbatimDuplicateContent_LeavesRowUnchanged() + { + var ct = TestContext.Current.CancellationToken; + await _store.InitializeAsync(ct); + + var createdAt = await SeedExistingDocumentAsync("doc-dup", "project-vega", ct); + _timeProvider.Advance(TimeSpan.FromHours(3)); + + // Content is verbatim already present in the seeded body: appending it would be + // pure bloat, so the write must be a logged no-op — the row stays byte-identical. + await _store.ApplyCurationBatchAsync( + "cp-dup-1", + [MakeColliderOperation("project-vega", content: "Quasar uses a Postgres 15 read replica for reporting.")], + ct); + + var row = await ReadDocumentAsync("doc-dup", ct); + Assert.Equal("Quasar uses a Postgres 15 read replica for reporting.", row.MarkdownBody); + Assert.Equal("Project Quasar datastore", row.Title); + Assert.Equal(MemoryUpdateSemantics.MergeDocument.ToWireValue(), row.UpdateSemantics); + Assert.Equal(createdAt, row.CreatedAtMs); + Assert.Equal(createdAt, row.UpdatedAtMs); + } + + [Fact] + public async Task ApplyInlineCurationBatchAsync_CollisionWithVerbatimDuplicateContent_LeavesRowUnchanged() + { + var ct = TestContext.Current.CancellationToken; + await _store.InitializeAsync(ct); + + var createdAt = await SeedExistingDocumentAsync("doc-dup-inline", "project-lyra", ct); + _timeProvider.Advance(TimeSpan.FromHours(3)); + + await _store.ApplyInlineCurationBatchAsync( + [MakeColliderOperation("project-lyra", content: "Quasar uses a Postgres 15 read replica for reporting.")], + ct); + + var row = await ReadDocumentAsync("doc-dup-inline", ct); + Assert.Equal("Quasar uses a Postgres 15 read replica for reporting.", row.MarkdownBody); + Assert.Equal("Project Quasar datastore", row.Title); + Assert.Equal(MemoryUpdateSemantics.MergeDocument.ToWireValue(), row.UpdateSemantics); + Assert.Equal(createdAt, row.CreatedAtMs); + Assert.Equal(createdAt, row.UpdatedAtMs); + } + + [Fact] + public async Task ApplyCurationBatchAsync_CreateWithNoCollision_InsertsFreshDocumentUnchanged() + { + var ct = TestContext.Current.CancellationToken; + await _store.InitializeAsync(ct); + + // No document exists under "project-comet" yet, so this Create-shaped proposal + // (no MemoryId, merge-document semantics) must be a plain insert: the dedup lookup + // finds nothing, so none of the collision/append machinery should engage. + var operation = MakeColliderOperation("project-comet"); + await _store.ApplyCurationBatchAsync("cp-no-collision", [operation], ct); + + var anchorId = MemoryTypedId.AnchorId("project-comet"); + var documentId = await ReadDocumentIdForAnchorAsync(anchorId, ct); + Assert.NotNull(documentId); + + var row = await ReadDocumentAsync(documentId!, ct); + Assert.Equal(operation.Title, row.Title); + Assert.Equal(operation.Content, row.MarkdownBody); + Assert.DoesNotContain("merged", row.MarkdownBody); + Assert.Equal(MemoryUpdateSemantics.MergeDocument.ToWireValue(), row.UpdateSemantics); + Assert.Equal(operation.Sensitivity, row.Sensitivity); + } + + // ── helpers ────────────────────────────────────────────────────── + + /// + /// Seeds an existing document under with a + /// distinct title/body/classification so the collision test can prove every one of + /// those values survives the append (rather than being replaced by the colliding + /// proposal's own values). Returns the seeded row's created_at for the created_at- + /// unchanged assertion. + /// + private async Task SeedExistingDocumentAsync(string documentId, string anchorCanonicalName, CancellationToken ct) + { + var anchor = _store.CreateDefaultAnchor(anchorCanonicalName); + var createdAt = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds(); + + await _store.UpsertDocumentAsync(new SQLiteMemoryDocument( + DocumentId: documentId, + Anchor: anchor, + MemoryClass: "durable_fact", + Title: "Project Quasar datastore", + MarkdownBody: "Quasar uses a Postgres 15 read replica for reporting.", + AliasesJson: null, + FacetsJson: null, + SlotsJson: null, + UpdateSemantics: MemoryUpdateSemantics.MergeDocument.ToWireValue(), + Sensitivity: MemorySensitivity.Secret.ToWireValue(), + RecallMode: "auto", + Confidence: 0.9, + FreshnessAtMs: createdAt, + ExpiresAtMs: null, + CreatedAtMs: createdAt, + UpdatedAtMs: createdAt, + Boundary: TrustBoundary.PersonalValue, + Audience: TrustAudience.Personal.ToWireValue()), ct); + + return createdAt; + } + + /// + /// A no-MemoryId, merge-document-semantics proposal — the shape a Create decision + /// takes by the time it reaches the store (see MemoryCurationEvaluator.ApplyDecisionAsync, + /// case CurationDecisionKind.Create: returns the operation unchanged). Content deliberately + /// shares no words with the seeded body, characterizing "fuzzy anchor match but low content + /// overlap": a genuinely different topic that happens to land on the same anchor. + /// Sensitivity/audience/boundary are deliberately public/normal — the OPPOSITE of the + /// seeded row's secret/personal/personal-boundary — so a test that only passed by + /// coincidence (both sides equal) would not slip through. + /// + private static SQLiteMemoryCurationOperation MakeColliderOperation( + string anchorCanonicalName, + string content = "Quasar's on-call rotation moved to PagerDuty last sprint.") => + new( + Kind: "document", + MemoryClass: "durable_fact", + MemoryId: null, + AnchorCanonicalName: anchorCanonicalName, + AnchorType: "concept", + Title: "On-call rotation", + Content: content, + AliasesJson: null, + FacetsJson: null, + SlotsJson: null, + Relations: null, + UpdateSemantics: MemoryUpdateSemantics.MergeDocument.ToWireValue(), + Boundary: TrustBoundary.PublicValue, + Audience: TrustAudience.Public, + Sensitivity: MemorySensitivity.Normal.ToWireValue(), + RecallMode: "auto", + Confidence: 0.9, + FreshnessAtMs: null, + ExpiresAtMs: null); + + private sealed record DocumentRow( + string Title, + string MarkdownBody, + string UpdateSemantics, + string? Boundary, + string? Audience, + string Sensitivity, + long CreatedAtMs, + long UpdatedAtMs); + + private async Task ReadDocumentAsync(string documentId, CancellationToken ct) + { + await using var conn = new SqliteConnection($"Data Source={_dbPath}"); + await conn.OpenAsync(ct); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = """ + SELECT title, markdown_body, update_semantics, boundary, audience, sensitivity, created_at, updated_at + FROM memory_documents + WHERE document_id = $id; + """; + cmd.Parameters.AddWithValue("$id", documentId); + await using var reader = await cmd.ExecuteReaderAsync(ct); + var found = await reader.ReadAsync(ct); + Assert.True(found, $"Expected document row '{documentId}' to exist."); + + return new DocumentRow( + reader.GetString(0), + reader.GetString(1), + reader.GetString(2), + reader.IsDBNull(3) ? null : reader.GetString(3), + reader.IsDBNull(4) ? null : reader.GetString(4), + reader.GetString(5), + reader.GetInt64(6), + reader.GetInt64(7)); + } + + private async Task ReadDocumentIdForAnchorAsync(string anchorId, CancellationToken ct) + { + await using var conn = new SqliteConnection($"Data Source={_dbPath}"); + await conn.OpenAsync(ct); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = "SELECT document_id FROM memory_documents WHERE anchor_id = $anchorId"; + cmd.Parameters.AddWithValue("$anchorId", anchorId); + return (string?)await cmd.ExecuteScalarAsync(ct); + } +} diff --git a/src/Netclaw.Actors/Memory/CurationPromptBuilder.cs b/src/Netclaw.Actors/Memory/CurationPromptBuilder.cs index 17c686c00..a22df1124 100644 --- a/src/Netclaw.Actors/Memory/CurationPromptBuilder.cs +++ b/src/Netclaw.Actors/Memory/CurationPromptBuilder.cs @@ -165,9 +165,12 @@ public static string BuildUserMessage( .ToArray(); if (ids.Length > 0) { + // First listed id is the primary write target (same role as + // EvaluateFuzzyMatch's best-match TargetDocumentId). No relevance scoring + // exists at this layer — the LLM's ordering is the only signal. return new CurationDecision( CurationDecisionKind.Consolidate, - null, + ids[0], ids, null, $"LLM decision: CONSOLIDATE {string.Join(" ", ids)}"); diff --git a/src/Netclaw.Actors/Memory/CurationRulesEvaluator.cs b/src/Netclaw.Actors/Memory/CurationRulesEvaluator.cs index c9b60c9e8..02d59168d 100644 --- a/src/Netclaw.Actors/Memory/CurationRulesEvaluator.cs +++ b/src/Netclaw.Actors/Memory/CurationRulesEvaluator.cs @@ -152,13 +152,17 @@ private static CurationDecision EvaluateFuzzyMatch( var overlap = AnchorNameMatcher.ComputeContentOverlap(proposal.Content, best.Content); - // High content overlap with fuzzy anchor match -> consolidation (auto-merge) + // High content overlap with fuzzy anchor match -> consolidation (auto-merge). + // TargetDocumentId names the primary document the collapsed content is written + // INTO (explicit-target overwrite, like Update): without it the store's anchor + // dedup treats the write as a Create collision and appends, doubling the + // near-duplicate body instead of collapsing it. if (overlap > HighOverlapThreshold) { var targetIds = fuzzyMatches.Select(c => c.DocumentId).ToArray(); return new CurationDecision( CurationDecisionKind.Consolidate, - null, + best.DocumentId, targetIds, best.AnchorCanonicalName, $"fuzzy anchor match + high content overlap ({overlap:P0})"); diff --git a/src/Netclaw.Actors/Memory/MemoryCurationEvaluator.cs b/src/Netclaw.Actors/Memory/MemoryCurationEvaluator.cs index a7ebc3a4f..2152903aa 100644 --- a/src/Netclaw.Actors/Memory/MemoryCurationEvaluator.cs +++ b/src/Netclaw.Actors/Memory/MemoryCurationEvaluator.cs @@ -361,9 +361,14 @@ public async Task EvaluateAsync( decision.Reason); await ExecuteConsolidationAsync(operation, decision, ct); - // After consolidation, write the new proposal under the canonical anchor + // After consolidation, write the proposal INTO the primary consolidated + // document (explicit target => the store's overwrite path, like Update): + // Consolidate means near-duplicate content, so the designed outcome is a + // collapse, not an append. A null TargetDocumentId (no construction site + // should produce one) flows through with MemoryId null and lands on the + // store's lossless dedup-append path, which logs curation_dedup_append. var canonicalAnchor = decision.CanonicalAnchorName ?? operation.AnchorCanonicalName; - return operation with { AnchorCanonicalName = canonicalAnchor }; + return operation with { AnchorCanonicalName = canonicalAnchor, MemoryId = decision.TargetDocumentId }; case CurationDecisionKind.Create: _log.Info( diff --git a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs index 0a0044dbe..73073068d 100644 --- a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs +++ b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs @@ -1340,6 +1340,34 @@ UPDATE memory_documents }, ct); } + /// + /// An existing document row found by the anchor-based dedup lookup in + /// /. + /// Carries the values that must survive a dedup collision so ON CONFLICT DO UPDATE + /// cannot silently replace this row's identity/classification with an unrelated + /// proposal's raw values (title stays, boundary/audience/sensitivity stay — a widened + /// audience or loosened sensitivity here would be a silent visibility escalation). + /// + private sealed record DedupCollision(string Title, string MarkdownBody, string? Boundary, string? Audience, string Sensitivity); + + /// + /// Builds the anchor-dedup append body for a proposal that collides with an existing + /// document under the same anchor. The rules tier can legitimately emit a Create + /// decision for "different topic, similar anchor name" (see + /// 's fuzzy/low-overlap branch); when that proposal's + /// anchor happens to already hold a document, appending instead of overwriting is + /// unconditionally lossless — plain concatenation can never drop the prior content the + /// way the old ON CONFLICT DO UPDATE overwrite did (audit: 88 silent overwrites/14 days, + /// including a carefully LLM-merged memory). Matches the dated-separator convention used + /// by MemoryCurationEvaluator.BuildAppendedBody on feature/memory-embeddings so the two + /// branches stay convention-compatible when merged. + /// + private string BuildDedupAppendedBody(string existingBody, string proposalContent) + { + var isoDate = _timeProvider.GetUtcNow().ToString("yyyy-MM-dd", System.Globalization.CultureInfo.InvariantCulture); + return $"{existingBody}\n\n---\n_[merged {isoDate}]_\n{proposalContent}"; + } + /// /// Write a batch of curation operations without an associated checkpoint. /// Used by the inline curation actor path where proposals are sent directly @@ -1419,8 +1447,13 @@ INSERT INTO memory_records( ? MemoryRecallMode.Never.ToWireValue() : operation.RecallMode; - // Anchor-based dedup: same logic as ApplyCurationBatchAsync + // Anchor-based dedup: same logic as ApplyCurationBatchAsync. Reusing an existing + // document_id here (documentId set, collision left null) is safe: it means the + // operation already carries an explicit target (Update decision). Finding an + // existing row via the anchor lookup below (collision set) is a genuine Create + // collision — see DedupCollision's remarks — and must append, not overwrite. string documentId; + DedupCollision? collision = null; if (!string.IsNullOrWhiteSpace(operation.MemoryId)) { documentId = operation.MemoryId; @@ -1430,20 +1463,69 @@ INSERT INTO memory_records( await using var lookupCmd = conn.CreateCommand(); lookupCmd.Transaction = tx; lookupCmd.CommandText = """ - SELECT document_id FROM memory_documents + SELECT document_id, title, markdown_body, boundary, audience, sensitivity + FROM memory_documents WHERE anchor_id = $anchorId ORDER BY updated_at DESC LIMIT 1; """; lookupCmd.Parameters.AddWithValue("$anchorId", anchor.AnchorId); - documentId = (string?)await lookupCmd.ExecuteScalarAsync(ct) - ?? MemoryTypedId.NewDocumentId().Value; + await using var lookupReader = await lookupCmd.ExecuteReaderAsync(ct); + if (await lookupReader.ReadAsync(ct)) + { + documentId = lookupReader.GetString(0); + collision = new DedupCollision( + lookupReader.GetString(1), + lookupReader.GetString(2), + lookupReader.IsDBNull(3) ? null : lookupReader.GetString(3), + lookupReader.IsDBNull(4) ? null : lookupReader.GetString(4), + lookupReader.GetString(5)); + } + else + { + documentId = MemoryTypedId.NewDocumentId().Value; + } } else { documentId = MemoryTypedId.NewDocumentId().Value; } + if (collision is not null) + { + // Idempotency guard: a colliding proposal whose content the existing body + // already holds verbatim adds nothing — appending it would bloat the + // document on every repeat (the inverse failure of the overwrite bug this + // path fixes). Logged no-op: the document row stays byte-identical. + if (collision.MarkdownBody.Contains(operation.Content, StringComparison.Ordinal)) + { + _logger.LogInformation( + "curation_dedup_duplicate_skipped anchor={AnchorCanonicalName} targetDoc={DocumentId}", + canonicalName, + documentId); + continue; + } + + _logger.LogInformation( + "curation_dedup_append anchor={AnchorCanonicalName} targetDoc={DocumentId}", + canonicalName, + documentId); + } + + // Preserve the colliding row's identity/classification; only the body grows + // (appended) and update_semantics flips to append-document to record that this + // write did not overwrite. Non-collision path is the pre-existing behavior. + var effectiveTitle = collision is not null ? collision.Title : operation.Title; + var effectiveBody = collision is not null + ? BuildDedupAppendedBody(collision.MarkdownBody, operation.Content) + : operation.Content; + var effectiveBoundary = collision is not null ? collision.Boundary : resolvedBoundary; + var effectiveAudience = collision is not null ? collision.Audience : operation.Audience.ToWireValue(); + var effectiveSensitivity = collision is not null ? collision.Sensitivity : operation.Sensitivity; + var effectiveSemantics = collision is not null + ? MemoryUpdateSemantics.AppendDocument.ToWireValue() + : operation.UpdateSemantics; + await using var documentCmd = conn.CreateCommand(); documentCmd.Transaction = tx; documentCmd.CommandText = """ @@ -1474,15 +1556,15 @@ ON CONFLICT(document_id) DO UPDATE SET documentCmd.Parameters.AddWithValue("$id", documentId); documentCmd.Parameters.AddWithValue("$anchorId", anchor.AnchorId); documentCmd.Parameters.AddWithValue("$memoryClass", operation.MemoryClass); - documentCmd.Parameters.AddWithValue("$title", operation.Title); - documentCmd.Parameters.AddWithValue("$body", operation.Content); + documentCmd.Parameters.AddWithValue("$title", effectiveTitle); + documentCmd.Parameters.AddWithValue("$body", effectiveBody); documentCmd.Parameters.AddWithValue("$aliasesJson", (object?)operation.AliasesJson ?? DBNull.Value); documentCmd.Parameters.AddWithValue("$facetsJson", (object?)operation.FacetsJson ?? DBNull.Value); documentCmd.Parameters.AddWithValue("$slotsJson", (object?)operation.SlotsJson ?? DBNull.Value); - documentCmd.Parameters.AddWithValue("$semantics", operation.UpdateSemantics); - documentCmd.Parameters.AddWithValue("$boundary", resolvedBoundary); - documentCmd.Parameters.AddWithValue("$audience", operation.Audience.ToWireValue()); - documentCmd.Parameters.AddWithValue("$sensitivity", operation.Sensitivity); + documentCmd.Parameters.AddWithValue("$semantics", effectiveSemantics); + documentCmd.Parameters.AddWithValue("$boundary", (object?)effectiveBoundary ?? DBNull.Value); + documentCmd.Parameters.AddWithValue("$audience", (object?)effectiveAudience ?? DBNull.Value); + documentCmd.Parameters.AddWithValue("$sensitivity", effectiveSensitivity); documentCmd.Parameters.AddWithValue("$recallMode", resolvedRecallMode); documentCmd.Parameters.AddWithValue("$confidence", operation.Confidence); documentCmd.Parameters.AddWithValue("$freshnessAt", (object?)operation.FreshnessAtMs ?? DBNull.Value); @@ -1492,7 +1574,7 @@ ON CONFLICT(document_id) DO UPDATE SET await documentCmd.ExecuteNonQueryAsync(ct); if (IsSearchableRecallMode(resolvedRecallMode)) - await UpsertDocumentFtsAsync(conn, tx, documentId, operation.Title, operation.Content, operation.AliasesJson, operation.FacetsJson, ct); + await UpsertDocumentFtsAsync(conn, tx, documentId, effectiveTitle, effectiveBody, operation.AliasesJson, operation.FacetsJson, ct); } await tx.CommitAsync(ct); @@ -1577,8 +1659,13 @@ INSERT INTO memory_records( // Anchor-based dedup: for merge-document semantics, find existing document // by anchor_id and reuse its ID so the ON CONFLICT UPDATE fires instead of // creating a duplicate. This catches same-anchor duplicates like 10 copies - // of "favorite color is blue" under the same anchor. + // of "favorite color is blue" under the same anchor. Reusing an existing + // document_id here (documentId set, collision left null) is safe when the + // operation already carries an explicit target (Update decision). Finding an + // existing row via the anchor lookup below (collision set) is a genuine Create + // collision — see DedupCollision's remarks — and must append, not overwrite. string documentId; + DedupCollision? collision = null; if (!string.IsNullOrWhiteSpace(operation.MemoryId)) { documentId = operation.MemoryId; @@ -1588,20 +1675,69 @@ INSERT INTO memory_records( await using var lookupCmd = conn.CreateCommand(); lookupCmd.Transaction = tx; lookupCmd.CommandText = """ - SELECT document_id FROM memory_documents + SELECT document_id, title, markdown_body, boundary, audience, sensitivity + FROM memory_documents WHERE anchor_id = $anchorId ORDER BY updated_at DESC LIMIT 1; """; lookupCmd.Parameters.AddWithValue("$anchorId", anchor.AnchorId); - documentId = (string?)await lookupCmd.ExecuteScalarAsync(ct) - ?? MemoryTypedId.NewDocumentId().Value; + await using var lookupReader = await lookupCmd.ExecuteReaderAsync(ct); + if (await lookupReader.ReadAsync(ct)) + { + documentId = lookupReader.GetString(0); + collision = new DedupCollision( + lookupReader.GetString(1), + lookupReader.GetString(2), + lookupReader.IsDBNull(3) ? null : lookupReader.GetString(3), + lookupReader.IsDBNull(4) ? null : lookupReader.GetString(4), + lookupReader.GetString(5)); + } + else + { + documentId = MemoryTypedId.NewDocumentId().Value; + } } else { documentId = MemoryTypedId.NewDocumentId().Value; } + if (collision is not null) + { + // Idempotency guard: a colliding proposal whose content the existing body + // already holds verbatim adds nothing — appending it would bloat the + // document on every repeat (the inverse failure of the overwrite bug this + // path fixes). Logged no-op: the document row stays byte-identical. + if (collision.MarkdownBody.Contains(operation.Content, StringComparison.Ordinal)) + { + _logger.LogInformation( + "curation_dedup_duplicate_skipped anchor={AnchorCanonicalName} targetDoc={DocumentId}", + canonicalName, + documentId); + continue; + } + + _logger.LogInformation( + "curation_dedup_append anchor={AnchorCanonicalName} targetDoc={DocumentId}", + canonicalName, + documentId); + } + + // Preserve the colliding row's identity/classification; only the body grows + // (appended) and update_semantics flips to append-document to record that this + // write did not overwrite. Non-collision path is the pre-existing behavior. + var effectiveTitle = collision is not null ? collision.Title : operation.Title; + var effectiveBody = collision is not null + ? BuildDedupAppendedBody(collision.MarkdownBody, operation.Content) + : operation.Content; + var effectiveBoundary = collision is not null ? collision.Boundary : resolvedBoundary; + var effectiveAudience = collision is not null ? collision.Audience : operation.Audience.ToWireValue(); + var effectiveSensitivity = collision is not null ? collision.Sensitivity : operation.Sensitivity; + var effectiveSemantics = collision is not null + ? MemoryUpdateSemantics.AppendDocument.ToWireValue() + : operation.UpdateSemantics; + await using var documentCmd = conn.CreateCommand(); documentCmd.Transaction = tx; documentCmd.CommandText = """ @@ -1632,15 +1768,15 @@ ON CONFLICT(document_id) DO UPDATE SET documentCmd.Parameters.AddWithValue("$id", documentId); documentCmd.Parameters.AddWithValue("$anchorId", anchor.AnchorId); documentCmd.Parameters.AddWithValue("$memoryClass", operation.MemoryClass); - documentCmd.Parameters.AddWithValue("$title", operation.Title); - documentCmd.Parameters.AddWithValue("$body", operation.Content); + documentCmd.Parameters.AddWithValue("$title", effectiveTitle); + documentCmd.Parameters.AddWithValue("$body", effectiveBody); documentCmd.Parameters.AddWithValue("$aliasesJson", (object?)operation.AliasesJson ?? DBNull.Value); documentCmd.Parameters.AddWithValue("$facetsJson", (object?)operation.FacetsJson ?? DBNull.Value); documentCmd.Parameters.AddWithValue("$slotsJson", (object?)operation.SlotsJson ?? DBNull.Value); - documentCmd.Parameters.AddWithValue("$semantics", operation.UpdateSemantics); - documentCmd.Parameters.AddWithValue("$boundary", resolvedBoundary); - documentCmd.Parameters.AddWithValue("$audience", operation.Audience.ToWireValue()); - documentCmd.Parameters.AddWithValue("$sensitivity", operation.Sensitivity); + documentCmd.Parameters.AddWithValue("$semantics", effectiveSemantics); + documentCmd.Parameters.AddWithValue("$boundary", (object?)effectiveBoundary ?? DBNull.Value); + documentCmd.Parameters.AddWithValue("$audience", (object?)effectiveAudience ?? DBNull.Value); + documentCmd.Parameters.AddWithValue("$sensitivity", effectiveSensitivity); documentCmd.Parameters.AddWithValue("$recallMode", resolvedRecallMode); documentCmd.Parameters.AddWithValue("$confidence", operation.Confidence); documentCmd.Parameters.AddWithValue("$freshnessAt", (object?)operation.FreshnessAtMs ?? DBNull.Value); @@ -1650,7 +1786,7 @@ ON CONFLICT(document_id) DO UPDATE SET await documentCmd.ExecuteNonQueryAsync(ct); if (IsSearchableRecallMode(resolvedRecallMode)) - await UpsertDocumentFtsAsync(conn, tx, documentId, operation.Title, operation.Content, operation.AliasesJson, operation.FacetsJson, ct); + await UpsertDocumentFtsAsync(conn, tx, documentId, effectiveTitle, effectiveBody, operation.AliasesJson, operation.FacetsJson, ct); } await using var markDone = conn.CreateCommand();