Skip to content

fix(diff): stop destroying memories on CONFLICT and cosine-only UPDATE #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
astrologist wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(diff): stop destroying memories on CONFLICT and cosine-only UPDATE #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions cmd/remember.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,24 @@ var rememberCmd = &cobra.Command{
if len(result.Matches) > 0 {
replacedID = result.Matches[0].ID
}
case search.DiffConflict, search.DiffUpdate:
diffAction = "updated"
if len(result.Matches) > 0 {
case search.DiffConflict:
// A CONFLICT means the two texts appear to disagree. Silently
// soft-deleting one side is destructive and has repeatedly
// clobbered unrelated same-domain memories (long technical
// notes share vocabulary at >=0.7 similarity, and change-log
// words like "replaced"/"no longer" appear in almost all of
// them). Keep both; the caller sees diff_suggestion=CONFLICT
// and can merge or delete deliberately.
diffAction = "added"
case search.DiffUpdate:
// Only auto-replace when the texts overlap heavily by TOKENS.
// Cosine similarity alone (same-domain embeddings cluster at
// 0.85+) is not enough evidence to destroy an existing memory.
if len(result.Matches) > 0 && result.Matches[0].TokenSimilarity >= 0.6 {
diffAction = "updated"
replacedID = result.Matches[0].ID
} else {
diffAction = "added"
}
default:
diffAction = "added"
Expand Down
20 changes: 16 additions & 4 deletions internal/search/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func Diff(insights []*model.Insight, newContent string, opts DiffOptions) DiffRe
similarity = cosineSim
}

suggestion := classifySuggestion(similarity, newContent, c.Insight.Content)
suggestion := classifySuggestion(tokenSim, similarity, newContent, c.Insight.Content)
matches = append(matches, DiffMatch{
ID: c.Insight.ID,
Content: c.Insight.Content,
Expand Down Expand Up @@ -143,7 +143,7 @@ func Diff(insights []*model.Insight, newContent string, opts DiffOptions) DiffRe
if cp.sim >= 0.85 && cp.sim > similarity {
similarity = cp.sim
}
suggestion := classifySuggestion(similarity, newContent, ins.Content)
suggestion := classifySuggestion(tokenSim, similarity, newContent, ins.Content)

// Only add if this match is meaningful (not ADD)
if suggestion != DiffAdd {
Expand Down Expand Up @@ -195,11 +195,23 @@ var negationWords = []string{
"不再", "放弃", "替换", "取消",
}

func classifySuggestion(similarity float64, newText, existingText string) DiffSuggestion {
func classifySuggestion(tokenSim, similarity float64, newText, existingText string) DiffSuggestion {
if similarity < 0.5 {
return DiffAdd
}

// isExtension: the new text is meaningfully longer than the existing one,
// i.e. it carries additional information. An extension must never be
// classified DUPLICATE — a skip would silently drop the new content.
isExtension := len(newText) > len(existingText)+len(existingText)/4

// 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 {
return DiffDuplicate
}

// Only check for conflict signals when texts are substantially similar.
// At borderline similarity (0.5–0.7) texts may share domain vocabulary
// without being about the same subject (e.g. two survey records from
Expand All @@ -214,7 +226,7 @@ func classifySuggestion(similarity float64, newText, existingText string) DiffSu
}
}

if similarity > 0.9 {
if similarity > 0.9 && !isExtension {
return DiffDuplicate
}
return DiffUpdate
Expand Down
16 changes: 8 additions & 8 deletions internal/search/diff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ import (
)

func TestClassifySuggestion_Add(t *testing.T) {
got := classifySuggestion(0.3, "completely new content", "existing different content")
got := classifySuggestion(0.3, 0.3, "completely new content", "existing different content")
if got != DiffAdd {
t.Errorf("low similarity: want ADD, got %s", got)
}
}

func TestClassifySuggestion_Duplicate(t *testing.T) {
got := classifySuggestion(0.95, "very similar content here", "very similar content here indeed")
got := classifySuggestion(0.95, 0.95, "very similar content here", "very similar content here indeed")
if got != DiffDuplicate {
t.Errorf("high similarity: want DUPLICATE, got %s", got)
}
}

func TestClassifySuggestion_Update(t *testing.T) {
got := classifySuggestion(0.7, "Go uses SQLite for storage", "Go uses PostgreSQL for storage")
got := classifySuggestion(0.7, 0.7, "Go uses SQLite for storage", "Go uses PostgreSQL for storage")
if got != DiffUpdate {
t.Errorf("medium similarity: want UPDATE, got %s", got)
}
Expand All @@ -40,7 +40,7 @@ func TestClassifySuggestion_ConflictNegation(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Similarity >= 0.7 so negation check is active.
got := classifySuggestion(0.7, tt.newText, tt.existing)
got := classifySuggestion(0.7, 0.7, tt.newText, tt.existing)
if got != DiffConflict {
t.Errorf("want CONFLICT, got %s", got)
}
Expand All @@ -52,7 +52,7 @@ func TestClassifySuggestion_NotWordNoConflict(t *testing.T) {
// "not" alone must NOT trigger CONFLICT — it appears constantly in
// scientific text ("species not previously recorded") and would cause
// false replacements of distinct survey records.
got := classifySuggestion(0.7, "species not recorded at this site", "species recorded at Kinabalu")
got := classifySuggestion(0.7, 0.7, "species not recorded at this site", "species recorded at Kinabalu")
if got == DiffConflict {
t.Error("bare 'not' should not trigger CONFLICT")
}
Expand All @@ -62,21 +62,21 @@ func TestClassifySuggestion_ConflictBelowThreshold(t *testing.T) {
// Negation words must NOT trigger CONFLICT when similarity < 0.7.
// Two survey records from different locations may share domain vocabulary
// and contain "no longer" or "replaced" in unrelated sentences.
got := classifySuggestion(0.6, "no longer present at Raub site", "butterfly survey Kinabalu")
got := classifySuggestion(0.6, 0.6, "no longer present at Raub site", "butterfly survey Kinabalu")
if got == DiffConflict {
t.Error("negation below similarity 0.7 should not trigger CONFLICT")
}
}

func TestClassifySuggestion_Boundary(t *testing.T) {
// Exactly 0.5 should not be ADD (it's >= 0.5)
got := classifySuggestion(0.5, "some content here", "other content here")
got := classifySuggestion(0.5, 0.5, "some content here", "other content here")
if got == DiffAdd {
t.Error("similarity=0.5: should not be ADD (threshold is < 0.5)")
}

// Exactly 0.9 should not be DUPLICATE (threshold is > 0.9)
got = classifySuggestion(0.9, "some content here", "other content here")
got = classifySuggestion(0.9, 0.9, "some content here", "other content here")
if got == DiffDuplicate {
t.Error("similarity=0.9: should not be DUPLICATE (threshold is > 0.9)")
}
Expand Down