diff --git a/cmd/memory/import_diff_test.go b/cmd/memory/import_diff_test.go index 2d34a51b..df6e5552 100644 --- a/cmd/memory/import_diff_test.go +++ b/cmd/memory/import_diff_test.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/mnemon-dev/mnemon/internal/memory/importdraft" "github.com/mnemon-dev/mnemon/internal/memory/model" @@ -155,3 +156,156 @@ func TestImportNoDiffStoresExactRepeats(t *testing.T) { result.Results[0].ID: content, result.Results[1].ID: content, }) } + +// Mainline now preserves every non-identical fact, including UPDATE suggestions. +// Keep the conflict review's persistence and edge checks under that write policy. +func TestImportDiffWriteOutcomes(t *testing.T) { + tests := []struct { + name string + existing string + newText string + noDiff bool + wantAction string + wantActive int + wantDeleted bool + }{ + {"exact duplicate", "Production deployment is allowed", "Production deployment is allowed", false, "skipped", 1, false}, + {"negated correction", "Production deployment is allowed", "Production deployment is not allowed", false, "added", 2, false}, + {"existing conflict signal", "Production deployment supports Python services", "Production deployment no longer supports Python services", false, "added", 2, false}, + {"ordinary update is preserved", "Production deployment uses PostgreSQL for persistent storage", "Production deployment uses SQLite for persistent storage", false, "added", 2, false}, + {"no diff inserts duplicate", "Production deployment is allowed", "Production deployment is allowed", true, "added", 2, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + db := setupImportDiffTest(t, tt.noDiff) + insertTestInsight(t, db, "original", tt.existing, "original-source", "2026-01-01T00:00:00Z") + summary := runImportDiffDraft(t, importdraft.MemoryDraft{ + SchemaVersion: "1", + Insights: []importdraft.DraftInsight{{Content: tt.newText, Importance: 5}}, + }) + if summary.Errors != 0 || len(summary.Results) != 1 || summary.Results[0].Action != tt.wantAction { + t.Fatalf("import summary = %+v, want one %s result", summary, tt.wantAction) + } + counts := map[string]int{"added": summary.Imported, "updated": summary.Updated, "skipped": summary.Skipped} + if counts[tt.wantAction] != 1 || summary.Imported+summary.Updated+summary.Skipped != 1 { + t.Fatalf("import counts = %v, want one %s", counts, tt.wantAction) + } + active, err := db.GetAllActiveInsights() + if err != nil || len(active) != tt.wantActive { + t.Fatalf("active insights = %d, error = %v; want %d", len(active), err, tt.wantActive) + } + original, err := db.GetInsightByIDIncludeDeleted("original") + if err != nil { + t.Fatal(err) + } + if deleted := original.DeletedAt != nil; deleted != tt.wantDeleted { + t.Fatalf("original deleted = %v, want %v", deleted, tt.wantDeleted) + } + resultID := summary.Results[0].ID + if (resultID == "original") != (tt.wantAction == "skipped") { + t.Fatalf("result ID = %q for action %s", resultID, tt.wantAction) + } + result, err := db.GetInsightByID(resultID) + if err != nil || result.Content != tt.newText { + t.Fatalf("imported insight = %+v, error = %v", result, err) + } + }) + } +} + +func TestImportConflictAndDuplicateResolveExplicitEdges(t *testing.T) { + db := setupImportDiffTest(t, false) + insertTestInsight(t, db, "original", "Production deployment is allowed", "original-source", "2026-01-01T00:00:00Z") + insertTestInsight(t, db, "context", "Security review approved the release plan", "review-source", "2026-01-01T00:00:00Z") + if err := db.InsertEdge(&model.Edge{ + SourceID: "original", TargetID: "context", EdgeType: model.EdgeCausal, + Weight: 0.7, CreatedAt: time.Now().UTC(), + }); err != nil { + t.Fatal(err) + } + summary := runImportDiffDraft(t, importdraft.MemoryDraft{ + SchemaVersion: "1", + Insights: []importdraft.DraftInsight{ + {Content: "Production deployment is allowed", Importance: 5}, + {Content: "Production deployment is not allowed", Importance: 5}, + }, + Edges: []importdraft.DraftEdge{{SourceIndex: 1, TargetIndex: 0, EdgeType: "causal", Weight: 0.8}}, + }) + if summary.Errors != 0 || summary.Imported != 1 || summary.Updated != 0 || summary.Skipped != 1 || summary.EdgesInserted != 1 || len(summary.Results) != 2 { + t.Fatalf("unexpected summary: %+v", summary) + } + if summary.Results[0].ID != "original" || summary.Results[0].Action != "skipped" || summary.Results[1].Action != "added" { + t.Fatalf("draft indices resolved incorrectly: %+v", summary.Results) + } + active, err := db.GetAllActiveInsights() + if err != nil || len(active) != 3 { + t.Fatalf("active insights = %d, error = %v; want 3", len(active), err) + } + for _, pair := range [][2]string{{summary.Results[1].ID, "original"}, {"original", "context"}} { + edges, err := db.GetEdgesBySourceAndType(pair[0], model.EdgeCausal) + if err != nil { + t.Fatal(err) + } + found := false + for _, edge := range edges { + if edge.TargetID == pair[1] { + found = true + } + } + if !found { + t.Fatalf("missing causal edge %s -> %s", pair[0], pair[1]) + } + } +} + +type importDiffSummary struct { + Imported int `json:"imported"` + Updated int `json:"updated"` + Skipped int `json:"skipped"` + Errors int `json:"errors"` + EdgesInserted int `json:"edges_inserted"` + Results []importResult `json:"results"` +} + +func setupImportDiffTest(t *testing.T, noDiff bool) *store.DB { + t.Helper() + t.Setenv("MNEMON_EMBED_ENDPOINT", "http://127.0.0.1:1") + t.Setenv("MNEMON_EMBED_PROTOCOL", "ollama") + t.Setenv("MNEMON_MAX_INSIGHTS", "1000") + oldDataDir, oldStoreName, oldReadOnly := dataDir, storeName, readOnly + oldImportNoDiff, oldImportDryRun := importNoDiff, importDryRun + t.Cleanup(func() { + dataDir, storeName, readOnly = oldDataDir, oldStoreName, oldReadOnly + importNoDiff, importDryRun = oldImportNoDiff, oldImportDryRun + }) + dataDir, storeName, readOnly = t.TempDir(), store.DefaultStoreName, false + importNoDiff, importDryRun = noDiff, false + db, err := store.Open(store.StoreDir(dataDir, storeName)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + return db +} + +func runImportDiffDraft(t *testing.T, draft importdraft.MemoryDraft) importDiffSummary { + t.Helper() + data, err := json.Marshal(draft) + if err != nil { + t.Fatal(err) + } + draftPath := filepath.Join(t.TempDir(), "draft.json") + if err := os.WriteFile(draftPath, data, 0o600); err != nil { + t.Fatal(err) + } + output := captureStdout(t, func() { + if err := importCmd.RunE(importCmd, []string{draftPath}); err != nil { + t.Fatal(err) + } + }) + var summary importDiffSummary + if err := json.Unmarshal([]byte(output), &summary); err != nil { + t.Fatalf("decode summary: %v\n%s", err, output) + } + return summary +} diff --git a/cmd/memory/remember_diff_test.go b/cmd/memory/remember_diff_test.go index 170bcdb2..86e25f62 100644 --- a/cmd/memory/remember_diff_test.go +++ b/cmd/memory/remember_diff_test.go @@ -100,6 +100,18 @@ func TestRememberPreservesDistinctContent(t *testing.T) { second: "Project Alpha no longer uses PostgreSQL database for persistent application storage", suggestion: search.DiffConflict, }, + { + name: "near duplicate negation remains advisory", + first: "Production deployment is allowed", + second: "Production deployment is not allowed", + suggestion: search.DiffConflict, + }, + { + name: "removing negation remains advisory", + first: "Production deployment is not allowed", + second: "Production deployment is allowed", + suggestion: search.DiffConflict, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/memory/search/diff.go b/internal/memory/search/diff.go index 32d8c7a1..521e5098 100644 --- a/internal/memory/search/diff.go +++ b/internal/memory/search/diff.go @@ -1,6 +1,7 @@ package search import ( + "regexp" "sort" "strings" @@ -195,6 +196,21 @@ var negationWords = []string{ "不再", "放弃", "替换", "取消", } +// negationMarkers matches explicit English negation markers in raw text. +// Stopword filtering removes "not"/"no" from the token set, so polarity must be +// read from the original text. Used only to tell a near-identical re-statement +// apart from its negation; it is deliberately NOT part of the >= 0.7 similarity +// conflict scan (bare "not" in scientific prose must not force CONFLICT). +// Unicode word boundaries avoid matching names such as "Noté". Both common +// apostrophes carry the same contraction. Individual CJK characters cannot +// establish negation: "非常" and "未来", for example, are not negative statements. +var negationMarkers = regexp.MustCompile(`(?i)(^|[^\p{L}\p{N}\p{M}_])(not|no|never|cannot|without|none)($|[^\p{L}\p{N}\p{M}_])|n['’]t($|[^\p{L}\p{N}\p{M}_])`) + +// hasNegation reports whether text carries an explicit negation marker. +func hasNegation(text string) bool { + return negationMarkers.MatchString(text) +} + func classifySuggestion(tokenSim, similarity float64, newText, existingText string) DiffSuggestion { if similarity < 0.5 { return DiffAdd @@ -205,10 +221,19 @@ func classifySuggestion(tokenSim, similarity float64, newText, existingText stri // classified DUPLICATE — a skip would silently drop the new content. isExtension := len(newText) > len(existingText)+len(existingText)/4 + // A near-identical token set can still flip meaning: stopwords strip + // "not"/"no", so "X is allowed" and "X is not allowed" tokenize identically. + // A polarity mismatch on an otherwise near-verbatim re-statement is a + // contradiction to surface (CONFLICT keeps both), never a duplicate to skip. + polarityMismatch := hasNegation(newText) != hasNegation(existingText) + // Near-verbatim re-statement measured by TOKENS (not just embeddings) is a // duplicate no matter what vocabulary it contains. Checked before the // negation scan so a text can never "conflict" with a copy of itself. if tokenSim > 0.9 && !isExtension { + if polarityMismatch { + return DiffConflict + } return DiffDuplicate } @@ -227,6 +252,9 @@ func classifySuggestion(tokenSim, similarity float64, newText, existingText stri } if similarity > 0.9 && !isExtension { + if polarityMismatch { + return DiffConflict + } return DiffDuplicate } return DiffUpdate diff --git a/internal/memory/search/diff_test.go b/internal/memory/search/diff_test.go index 037964a2..ca23991f 100644 --- a/internal/memory/search/diff_test.go +++ b/internal/memory/search/diff_test.go @@ -188,3 +188,81 @@ func TestDiff_LowerKeywordScoreUpdateNotMasked(t *testing.T) { "high-keyword-score ADD from insightA masked the UPDATE", result.Suggestion) } } + +func TestClassifySuggestion_NegationIsNotDuplicate(t *testing.T) { + // Issue #133: "not" is a stopword, so both texts tokenize identically. + // The negated correction must never be classified DUPLICATE (a skip would + // silently discard it); it must surface as CONFLICT so both facts are kept. + got := classifySuggestion(1.0, 1.0, "Production deployment is not allowed", "Production deployment is allowed") + if got != DiffConflict { + t.Errorf("negated re-statement: want CONFLICT, got %s", got) + } +} + +func TestClassifySuggestion_ExactRepetitionStillDuplicate(t *testing.T) { + // Control case: polarity is unchanged, so an exact repetition must still dedupe. + got := classifySuggestion(1.0, 1.0, "Production deployment is allowed", "Production deployment is allowed") + if got != DiffDuplicate { + t.Errorf("exact repetition: want DUPLICATE, got %s", got) + } +} + +func TestDiff_NegatedCorrectionIsNotSkipped(t *testing.T) { + // End-to-end through Diff(): the affirmative fact is already stored and the + // negated correction must not be reported as an overall DUPLICATE. + insights := []*model.Insight{ + {ID: "1", Content: "Production deployment is allowed"}, + } + result := Diff(insights, "Production deployment is not allowed", DiffOptions{}) + if result.Suggestion == DiffDuplicate { + t.Errorf("negated correction: overall suggestion must not be DUPLICATE, got %s", result.Suggestion) + } +} + +func TestDiff_NegationMarkerBoundaries(t *testing.T) { + const suffix = " for the regional production cluster following security review and automated compliance checks across all services while ensuring observability resilience capacity backups restoration health readiness throughout primary secondary environments" + const chineseSuffix = "，值班团队完成上线审核流程并记录服务状态以及所有关键指标，监控系统会持续观察业务运行情况和生产资源使用情况" + tests := []struct { + name string + existing string + newText string + want DiffSuggestion + }{ + {"straight contraction", "Production deployment is allowed" + suffix, "Production deployment isn't allowed" + suffix, DiffConflict}, + {"curly contraction", "Production deployment is allowed" + suffix, "Production deployment isn’t allowed" + suffix, DiffConflict}, + {"equivalent apostrophes", "Production deployment isn't allowed" + suffix, "Production deployment isn’t allowed" + suffix, DiffDuplicate}, + {"unicode word boundary", "Production deployment is allowed" + suffix, "Production deployment is allowed with Noté" + suffix, DiffDuplicate}, + {"noteworthy is not a marker", "Production deployment is allowed" + suffix, "Production deployment is noteworthy and allowed" + suffix, DiffDuplicate}, + {"nonetheless is not a marker", "Production deployment is allowed" + suffix, "Production deployment is nonetheless allowed" + suffix, DiffDuplicate}, + {"chinese intensifier", "生产部署状态稳定" + chineseSuffix, "生产部署状态非常稳定" + chineseSuffix, DiffDuplicate}, + {"chinese future word", "生产部署计划已经确认" + chineseSuffix, "未来生产部署计划已经确认" + chineseSuffix, DiffDuplicate}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if similarity := JaccardSimilarity(tt.newText, tt.existing); similarity <= 0.9 { + t.Fatalf("fixture must reach the near-duplicate branch, got %f", similarity) + } + result := Diff([]*model.Insight{{ID: "existing", Content: tt.existing}}, tt.newText, DiffOptions{}) + if result.Suggestion != tt.want { + t.Fatalf("suggestion = %s, want %s", result.Suggestion, tt.want) + } + }) + } +} + +func TestDiff_NegationInEmbeddingDuplicate(t *testing.T) { + result := Diff( + []*model.Insight{{ID: "existing", Content: "Production deployment is allowed"}}, + "Production rollout is not allowed", + DiffOptions{ + NewEmbedding: []float64{1, 0}, + ExistingEmbed: []EmbeddedItem{{ID: "existing", Embedding: []float64{0.95, 0.3122498999199199}}}, + }, + ) + if len(result.Matches) != 1 || result.Matches[0].TokenSimilarity > 0.9 || result.Matches[0].Similarity <= 0.9 { + t.Fatalf("fixture must reach the embedding near-duplicate branch: %+v", result) + } + if result.Suggestion != DiffConflict { + t.Fatalf("suggestion = %s, want CONFLICT", result.Suggestion) + } +}