Pre-flight Checks
Bug Description
mem_compare with relation="not_conflict" returns {"sync_id": ""} (success) but persists nothing to the database. The tool description at mcp.go:870 says "PERSIST that verdict into the relation store" and at line 884 says "the verdict is recorded but not stored" — these statements contradict each other.
The practical impact: the agent evaluates two memories, calls mem_compare, receives success, but the verdict is lost. On the next session, FindCandidates re-surfaces the same pair because no row was inserted into memory_relations. The agent must re-evaluate the same pair from scratch — its previous work is silently discarded.
Root cause: JudgeBySemantic at relations.go:747-749 returns "", nil for not_conflict without inserting any row. There is no "already evaluated" tracking mechanism — the only dedup is the memory_relations table itself.
Steps to Reproduce
- Start engram and save two related observations:
engram mem save "Auth uses JWT tokens" --type architecture --project engram
engram mem save "Auth uses session cookies" --type architecture --project engram
- Agent observes both observations and calls:
mem_compare(memory_id_a: 1, memory_id_b: 2, relation: "not_conflict", confidence: 0.9, reasoning: "Both describe auth approaches, no conflict")
- Agent receives:
{"sync_id": ""} — success response
- End the session and start a new one
- Agent calls
mem_search("auth") or saves a new auth-related observation
FindCandidates re-surfaces the same (1, 2) pair as a potential conflict
- Agent must re-evaluate the pair from scratch
Expected Behavior
mem_compare with relation="not_conflict" should either:
- (a) Insert a row into
memory_relations with relation="not_conflict" so the pair is tracked and not re-surfaced, OR
- (b) Return an error indicating that
not_conflict verdicts cannot be persisted (if the no-op is intentional)
Option (a) is preferred — it matches the tool description ("PERSIST that verdict") and prevents redundant re-evaluation.
Actual Behavior
Agent receives {"sync_id": ""} (success), but:
- No row is inserted into
memory_relations
FindCandidates (relations.go:364-376) has no filter for "already compared" pairs
- Same pair re-surfaces on every subsequent search/save
- Agent comparison work is silently lost between sessions
Relevant Code
| Location |
What |
internal/mcp/mcp.go:870-888 |
Tool description claims "PERSIST" but documents no-op |
internal/store/relations.go:747-749 |
JudgeBySemantic returns "", nil for not_conflict |
internal/store/relations.go:364-376 |
FindCandidates FTS5 query — no "already compared" filter |
internal/store/relations.go:1306-1320 |
ScanProject pre-check — only finds rows in memory_relations |
Follow-up Considerations
-
Backward compatibility: Existing not_conflict verdicts are not stored, so no migration needed — new verdicts will simply start being persisted.
-
FindCandidates update: After inserting not_conflict rows, FindCandidates should filter out pairs that already have a relation row (currently only ScanProject does this at line 1306).
-
mem_search annotations: Search results show relation annotations (supersedes, conflicts_with, etc.). Should not_conflict also appear as an annotation? Likely not — it means "no relationship" and would clutter results.
-
Test coverage: mcp_compare_test.go:89-122 (TestHandleCompare_NotConflict_NoRow) currently asserts the no-op behavior. This test needs to be updated to assert that a row IS inserted.
Proposed Approach
-
In JudgeBySemantic (relations.go:747-749), replace the no-op with an INSERT using the same upsert logic as other relation types.
-
Update the tool description (mcp.go:884) to remove the contradictory "recorded but not stored" language.
-
Update TestHandleCompare_NotConflict_NoRow to assert a row IS created.
-
Add a test in scan_semantic_test.go to verify ScanProject skips not_conflict pairs.
-
Optionally: add a FindCandidates filter to skip pairs with existing relation rows (currently only ScanProject does this).
Pre-flight Checks
status:approvedbefore a PR can be openedBug Description
mem_comparewithrelation="not_conflict"returns{"sync_id": ""}(success) but persists nothing to the database. The tool description atmcp.go:870says "PERSIST that verdict into the relation store" and at line 884 says "the verdict is recorded but not stored" — these statements contradict each other.The practical impact: the agent evaluates two memories, calls
mem_compare, receives success, but the verdict is lost. On the next session,FindCandidatesre-surfaces the same pair because no row was inserted intomemory_relations. The agent must re-evaluate the same pair from scratch — its previous work is silently discarded.Root cause:
JudgeBySemanticatrelations.go:747-749returns"", nilfornot_conflictwithout inserting any row. There is no "already evaluated" tracking mechanism — the only dedup is thememory_relationstable itself.Steps to Reproduce
{"sync_id": ""}— success responsemem_search("auth")or saves a new auth-related observationFindCandidatesre-surfaces the same (1, 2) pair as a potential conflictExpected Behavior
mem_comparewithrelation="not_conflict"should either:memory_relationswithrelation="not_conflict"so the pair is tracked and not re-surfaced, ORnot_conflictverdicts cannot be persisted (if the no-op is intentional)Option (a) is preferred — it matches the tool description ("PERSIST that verdict") and prevents redundant re-evaluation.
Actual Behavior
Agent receives
{"sync_id": ""}(success), but:memory_relationsFindCandidates(relations.go:364-376) has no filter for "already compared" pairsRelevant Code
internal/mcp/mcp.go:870-888internal/store/relations.go:747-749JudgeBySemanticreturns"", nilfor not_conflictinternal/store/relations.go:364-376FindCandidatesFTS5 query — no "already compared" filterinternal/store/relations.go:1306-1320ScanProjectpre-check — only finds rows inmemory_relationsFollow-up Considerations
Backward compatibility: Existing
not_conflictverdicts are not stored, so no migration needed — new verdicts will simply start being persisted.FindCandidatesupdate: After insertingnot_conflictrows,FindCandidatesshould filter out pairs that already have a relation row (currently onlyScanProjectdoes this at line 1306).mem_searchannotations: Search results show relation annotations (supersedes,conflicts_with, etc.). Shouldnot_conflictalso appear as an annotation? Likely not — it means "no relationship" and would clutter results.Test coverage:
mcp_compare_test.go:89-122(TestHandleCompare_NotConflict_NoRow) currently asserts the no-op behavior. This test needs to be updated to assert that a row IS inserted.Proposed Approach
In
JudgeBySemantic(relations.go:747-749), replace the no-op with an INSERT using the same upsert logic as other relation types.Update the tool description (mcp.go:884) to remove the contradictory "recorded but not stored" language.
Update
TestHandleCompare_NotConflict_NoRowto assert a row IS created.Add a test in
scan_semantic_test.goto verifyScanProjectskipsnot_conflictpairs.Optionally: add a
FindCandidatesfilter to skip pairs with existing relation rows (currently onlyScanProjectdoes this).