diff --git a/cmd/engram/conflicts.go b/cmd/engram/conflicts.go index aa1799f6..48159cbe 100644 --- a/cmd/engram/conflicts.go +++ b/cmd/engram/conflicts.go @@ -134,10 +134,11 @@ func cmdConflictsList(cfg store.Config) { defer s.Close() opts := store.ListRelationsOptions{ - Project: proj, - Status: statusFlag, - SinceTime: sinceTime, - Limit: limit, + Project: proj, + Status: statusFlag, + ExcludeNotConflict: true, + SinceTime: sinceTime, + Limit: limit, } items, err := s.ListRelations(opts) diff --git a/internal/mcp/mcp.go b/internal/mcp/mcp.go index 336f334e..827b99a4 100644 --- a/internal/mcp/mcp.go +++ b/internal/mcp/mcp.go @@ -881,11 +881,11 @@ PARAMS: BEHAVIOR: - Persists the verdict via JudgeBySemantic with system provenance (marked_by_actor="engram"). - - not_conflict: no row is inserted; tool returns success with empty sync_id (the verdict is recorded but not stored — it means "we evaluated these and they do not conflict"). + - not_conflict: inserts a row tracking that the pair was evaluated and found non-conflicting. This resolves any pending relation annotation and prevents ScanProject from re-inserting the pair. - Idempotent: calling again for the same pair updates the existing row. - Cross-project pairs are rejected. -SUCCESS: Returns { "sync_id": "rel-..." } on persist, { "sync_id": "" } on not_conflict. +SUCCESS: Returns { "sync_id": "rel-..." } on persist. ERROR: Returns IsError=true if IDs are unknown, relation is invalid, or cross-project pair.`), mcp.WithTitleAnnotation("Compare Memory Pair (Persist Semantic Verdict)"), mcp.WithReadOnlyHintAnnotation(false), @@ -2143,7 +2143,6 @@ func handleCompare(s *store.Store, _ *SessionActivity) server.ToolHandlerFunc { return mcp.NewToolResultError(err.Error()), nil } - // syncID is "" when relation == "not_conflict" (JudgeBySemantic no-op). envelope := map[string]any{ "sync_id": syncID, } diff --git a/internal/mcp/mcp_compare_test.go b/internal/mcp/mcp_compare_test.go index 512d6c12..b2eb1fae 100644 --- a/internal/mcp/mcp_compare_test.go +++ b/internal/mcp/mcp_compare_test.go @@ -86,9 +86,9 @@ func TestHandleCompare_HappyPath(t *testing.T) { } } -// TestHandleCompare_NotConflict_NoRow — not_conflict returns success without inserting a row. -// REQ-011 | Design §9 (not_conflict is still persisted but JudgeBySemantic handles it as no-op) -func TestHandleCompare_NotConflict_NoRow(t *testing.T) { +// TestHandleCompare_NotConflict_Persists — not_conflict inserts a row and returns non-empty sync_id. +// REQ-011 | Design §9 (not_conflict is persisted to track evaluated pairs) +func TestHandleCompare_NotConflict_Persists(t *testing.T) { s := newMCPTestStore(t) idA, idB := seedCompareFixture(t, s) @@ -115,10 +115,10 @@ func TestHandleCompare_NotConflict_NoRow(t *testing.T) { t.Fatalf("response not valid JSON: %v", err) } - // not_conflict is a no-op — sync_id should be empty string + // not_conflict should now persist — sync_id must be non-empty syncID, _ := envelope["sync_id"].(string) - if syncID != "" { - t.Fatalf("expected empty sync_id for not_conflict, got %q", syncID) + if syncID == "" { + t.Fatalf("expected non-empty sync_id for not_conflict, got empty") } } diff --git a/internal/server/server.go b/internal/server/server.go index 244d3b38..6fd91481 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -979,10 +979,11 @@ func (s *Server) handleListConflicts(w http.ResponseWriter, r *http.Request) { offset := queryInt(r, "offset", 0) opts := store.ListRelationsOptions{ - Project: project, - Status: status, - Limit: limit, - Offset: offset, + Project: project, + Status: status, + ExcludeNotConflict: true, + Limit: limit, + Offset: offset, } sinceStr := r.URL.Query().Get("since") @@ -1003,9 +1004,10 @@ func (s *Server) handleListConflicts(w http.ResponseWriter, r *http.Request) { // Count without limit/offset for the total field. countOpts := store.ListRelationsOptions{ - Project: project, - Status: status, - SinceTime: opts.SinceTime, + Project: project, + Status: status, + ExcludeNotConflict: true, + SinceTime: opts.SinceTime, } total, err := s.store.CountRelations(countOpts) if err != nil { diff --git a/internal/store/judge_by_semantic_test.go b/internal/store/judge_by_semantic_test.go index 84c4fe31..ff3cddfa 100644 --- a/internal/store/judge_by_semantic_test.go +++ b/internal/store/judge_by_semantic_test.go @@ -139,11 +139,11 @@ func TestJudgeBySemantic_UpsertIdempotency(t *testing.T) { } } -// ─── C.2c — TestJudgeBySemantic_NotConflictIsNoOp ──────────────────────────── +// ─── C.2c — TestJudgeBySemantic_NotConflictPersists ───────────────────────── -// TestJudgeBySemantic_NotConflictIsNoOp verifies that passing Relation="not_conflict" -// inserts no row and returns an empty sync_id without error. -func TestJudgeBySemantic_NotConflictIsNoOp(t *testing.T) { +// TestJudgeBySemantic_NotConflictPersists verifies that passing Relation="not_conflict" +// inserts a row and returns a non-empty sync_id, so the pair is tracked as evaluated. +func TestJudgeBySemantic_NotConflictPersists(t *testing.T) { s := setupRelationsStore(t) _, syncA := addTestObs(t, s, "Unrelated auth decision", "decision", "testproject", "project") @@ -160,20 +160,20 @@ func TestJudgeBySemantic_NotConflictIsNoOp(t *testing.T) { if err != nil { t.Fatalf("JudgeBySemantic(not_conflict): unexpected error: %v", err) } - if syncID != "" { - t.Errorf("JudgeBySemantic(not_conflict): expected empty sync_id; got %q", syncID) + if syncID == "" { + t.Errorf("JudgeBySemantic(not_conflict): expected non-empty sync_id; got empty") } - // No row must exist for this pair. + // A row must exist for this pair with relation = 'not_conflict'. var count int if err := s.db.QueryRow( - `SELECT count(*) FROM memory_relations WHERE source_id = ? OR target_id = ?`, - syncA, syncA, + `SELECT count(*) FROM memory_relations WHERE source_id = ? AND target_id = ? AND relation = 'not_conflict'`, + syncA, syncB, ).Scan(&count); err != nil { t.Fatalf("count query: %v", err) } - if count != 0 { - t.Errorf("expected 0 rows for not_conflict pair; got %d", count) + if count != 1 { + t.Errorf("expected 1 not_conflict row for pair; got %d", count) } } @@ -312,8 +312,8 @@ func TestJudgeBySemantic_CrossProjectRejected(t *testing.T) { // ─── C.2f — TestJudgeBySemantic_AllValidRelations ──────────────────────────── -// TestJudgeBySemantic_AllValidRelations verifies that all 5 non-not_conflict -// relation verbs are accepted by JudgeBySemantic. +// TestJudgeBySemantic_AllValidRelations verifies that all 6 relation verbs +// (including not_conflict) are accepted and persisted by JudgeBySemantic. func TestJudgeBySemantic_AllValidRelations(t *testing.T) { validRelations := []string{ RelationRelated, @@ -321,6 +321,7 @@ func TestJudgeBySemantic_AllValidRelations(t *testing.T) { RelationScoped, RelationConflictsWith, RelationSupersedes, + RelationNotConflict, } for _, rel := range validRelations { diff --git a/internal/store/relations.go b/internal/store/relations.go index 1adb6ad1..c71479c1 100644 --- a/internal/store/relations.go +++ b/internal/store/relations.go @@ -26,13 +26,13 @@ var ErrSemanticPromptBuilderRequired = errors.New("semantic scan requires a non- // Valid relation type values. Type compatibility is NOT enforced in Phase 1; // the agent does that judgment. const ( - RelationPending = "pending" - RelationRelated = "related" - RelationCompatible = "compatible" - RelationScoped = "scoped" + RelationPending = "pending" + RelationRelated = "related" + RelationCompatible = "compatible" + RelationScoped = "scoped" RelationConflictsWith = "conflicts_with" - RelationSupersedes = "supersedes" - RelationNotConflict = "not_conflict" + RelationSupersedes = "supersedes" + RelationNotConflict = "not_conflict" ) // Valid judgment_status values. @@ -93,6 +93,9 @@ type ListRelationsOptions struct { Project string // Status filters by judgment_status. Empty means no status filter. Status string + // ExcludeNotConflict removes rows with relation='not_conflict' from results. + // Set to true for conflict-focused views (dashboard, CLI conflicts list). + ExcludeNotConflict bool // SinceTime filters to rows created_at >= SinceTime. Zero value means no filter. SinceTime time.Time // Limit caps the number of rows returned. 0 or negative means no limit. @@ -118,11 +121,11 @@ type RelationListItem struct { // RelationStats holds aggregate counts of relations for a project. type RelationStats struct { - Project string `json:"project"` - ByRelation map[string]int `json:"by_relation"` + Project string `json:"project"` + ByRelation map[string]int `json:"by_relation"` ByJudgmentStatus map[string]int `json:"by_judgment_status"` - DeferredCount int `json:"deferred"` - DeadCount int `json:"dead"` + DeferredCount int `json:"deferred"` + DeadCount int `json:"dead"` } // DeferredRow represents a row in sync_apply_deferred with the payload decoded. @@ -141,13 +144,13 @@ type DeferredRow struct { // ScanResult holds the output of a ScanProject call. type ScanResult struct { - Project string `json:"project"` - Inspected int `json:"inspected"` - CandidatesFound int `json:"candidates_found"` - AlreadyRelated int `json:"already_related"` - RelationsInserted int `json:"inserted"` - Capped bool `json:"capped"` - DryRun bool `json:"dry_run"` + Project string `json:"project"` + Inspected int `json:"inspected"` + CandidatesFound int `json:"candidates_found"` + AlreadyRelated int `json:"already_related"` + RelationsInserted int `json:"inserted"` + Capped bool `json:"capped"` + DryRun bool `json:"dry_run"` // Semantic counters — populated only when ScanOptions.Semantic is true. // Zero-value is safe for existing JSON consumers. @@ -203,7 +206,8 @@ type JudgeBySemanticParams struct { // TargetID is the TEXT sync_id of the target observation (required). TargetID string // Relation is the verdict verb (required); must be in validRelationVerbs. - // Passing "not_conflict" is a no-op: no row is inserted and no error is returned. + // Including "not_conflict" which persists a row to track that the pair was + // evaluated and found non-conflicting. Relation string // Confidence is the LLM's self-reported confidence score [0.0, 1.0]. Confidence float64 @@ -244,31 +248,31 @@ type Candidate struct { // Relation represents a row in memory_relations. type Relation struct { - ID int64 `json:"id"` - SyncID string `json:"sync_id"` - SourceID string `json:"source_id"` - TargetID string `json:"target_id"` - Relation string `json:"relation"` - Reason *string `json:"reason,omitempty"` - Evidence *string `json:"evidence,omitempty"` - Confidence *float64 `json:"confidence,omitempty"` - JudgmentStatus string `json:"judgment_status"` - MarkedByActor *string `json:"marked_by_actor,omitempty"` - MarkedByKind *string `json:"marked_by_kind,omitempty"` - MarkedByModel *string `json:"marked_by_model,omitempty"` - SessionID *string `json:"session_id,omitempty"` - CreatedAt string `json:"created_at"` - UpdatedAt string `json:"updated_at"` + ID int64 `json:"id"` + SyncID string `json:"sync_id"` + SourceID string `json:"source_id"` + TargetID string `json:"target_id"` + Relation string `json:"relation"` + Reason *string `json:"reason,omitempty"` + Evidence *string `json:"evidence,omitempty"` + Confidence *float64 `json:"confidence,omitempty"` + JudgmentStatus string `json:"judgment_status"` + MarkedByActor *string `json:"marked_by_actor,omitempty"` + MarkedByKind *string `json:"marked_by_kind,omitempty"` + MarkedByModel *string `json:"marked_by_model,omitempty"` + SessionID *string `json:"session_id,omitempty"` + CreatedAt string `json:"created_at"` + UpdatedAt string `json:"updated_at"` // Annotation fields — populated by GetRelationsForObservations via LEFT JOIN. // Excluded from JSON output (used only for in-process annotation building). // REQ-005, REQ-012 | Design §7, §8. - SourceIntID int64 `json:"-"` // integer primary key of source observation - SourceTitle string `json:"-"` // title of source observation; empty if missing/deleted - SourceMissing bool `json:"-"` // true if source is soft-deleted or not found - TargetIntID int64 `json:"-"` // integer primary key of target observation - TargetTitle string `json:"-"` // title of target observation; empty if missing/deleted - TargetMissing bool `json:"-"` // true if target is soft-deleted or not found + SourceIntID int64 `json:"-"` // integer primary key of source observation + SourceTitle string `json:"-"` // title of source observation; empty if missing/deleted + SourceMissing bool `json:"-"` // true if source is soft-deleted or not found + TargetIntID int64 `json:"-"` // integer primary key of target observation + TargetTitle string `json:"-"` // title of target observation; empty if missing/deleted + TargetMissing bool `json:"-"` // true if target is soft-deleted or not found } // ObservationRelations groups relations for a single observation, split by role. @@ -282,7 +286,7 @@ type ObservationRelations struct { // SaveRelationParams holds the inputs for SaveRelation. type SaveRelationParams struct { // SyncID is the unique identifier for this relation row (format: rel-<16hex>). - SyncID string + SyncID string // SourceID is the TEXT sync_id of the source observation. SourceID string // TargetID is the TEXT sync_id of the target observation. @@ -292,23 +296,23 @@ type SaveRelationParams struct { // JudgeRelationParams holds the inputs for JudgeRelation. type JudgeRelationParams struct { // JudgmentID is the sync_id of the relation row to update (required). - JudgmentID string + JudgmentID string // Relation is the verdict verb (required); must be one of validRelationVerbs. - Relation string + Relation string // Reason is an optional free-text explanation. - Reason *string + Reason *string // Evidence is optional free-form JSON or text evidence. - Evidence *string + Evidence *string // Confidence is optional 0..1 confidence score. - Confidence *float64 + Confidence *float64 // MarkedByActor is the actor identifier (e.g. "agent:claude-sonnet-4-6" or "user"). MarkedByActor string // MarkedByKind is the actor kind ("agent", "human", "system"). - MarkedByKind string + MarkedByKind string // MarkedByModel is the model ID (may be empty for human actors). MarkedByModel string // SessionID is the session in which the judgment was made (optional). - SessionID string + SessionID string } // ─── FindCandidates ─────────────────────────────────────────────────────────── @@ -719,8 +723,10 @@ func validateCrossProjectGuard(tx *sql.Tx, sourceID, targetID string) error { // the memory_relations table with system provenance (marked_by_kind="system", // marked_by_actor="engram", marked_by_model=params.Model). // -// When params.Relation is "not_conflict" the call is a no-op: no row is inserted -// and an empty sync_id is returned without error. +// When params.Relation is "not_conflict", a row is still inserted (or an +// existing pending row is updated) so the pair is tracked as evaluated. +// This resolves any pending relation annotation and prevents ScanProject +// from re-inserting the pair on subsequent scans. // // Idempotency: if a row already exists for (source_id, target_id) in either // direction, the existing row is updated (UPSERT). The returned sync_id is @@ -744,11 +750,6 @@ func (s *Store) JudgeBySemantic(p JudgeBySemanticParams) (string, error) { return "", fmt.Errorf("JudgeBySemantic: confidence %v is out of range [0.0, 1.0]", p.Confidence) } - // not_conflict is a no-op. - if p.Relation == RelationNotConflict { - return "", nil - } - var resultSyncID string if err := s.withTx(func(tx *sql.Tx) error { @@ -1058,6 +1059,9 @@ func buildRelationsQuery(opts ListRelationsOptions, countOnly bool) (string, []a query += ` AND r.judgment_status = ?` args = append(args, opts.Status) } + if opts.ExcludeNotConflict { + query += ` AND r.relation != 'not_conflict'` + } if !opts.SinceTime.IsZero() { query += ` AND r.created_at >= ?` args = append(args, opts.SinceTime.UTC().Format("2006-01-02T15:04:05Z")) @@ -1091,6 +1095,7 @@ func (s *Store) GetRelationStats(project string) (RelationStats, error) { } // Build query: when project is non-empty, filter via JOIN to observations. + // not_conflict rows are excluded — this is a conflict stats view. var q string var args []any if project != "" { @@ -1099,7 +1104,8 @@ func (s *Store) GetRelationStats(project string) (RelationStats, error) { FROM memory_relations r LEFT JOIN observations src ON src.sync_id = r.source_id AND src.deleted_at IS NULL LEFT JOIN observations tgt ON tgt.sync_id = r.target_id AND tgt.deleted_at IS NULL - WHERE ifnull(src.project,'') = ? OR ifnull(tgt.project,'') = ? + WHERE (ifnull(src.project,'') = ? OR ifnull(tgt.project,'') = ?) + AND r.relation != 'not_conflict' GROUP BY r.relation, r.judgment_status ` args = []any{project, project} @@ -1107,6 +1113,7 @@ func (s *Store) GetRelationStats(project string) (RelationStats, error) { q = ` SELECT relation, judgment_status, count(*) AS cnt FROM memory_relations + WHERE relation != 'not_conflict' GROUP BY relation, judgment_status ` } @@ -1149,8 +1156,8 @@ func (s *Store) GetRelationStats(project string) (RelationStats, error) { // // Phase 4 extension: when ScanOptions.Semantic is true, after the FTS5 candidate // collection a bounded worker pool calls Runner.Compare on each pair and persists -// non-"not_conflict" verdicts via JudgeBySemantic. Semantic=false (zero value) -// preserves Phase 3 behaviour exactly. +// all verdicts (including not_conflict) via JudgeBySemantic. Semantic=false (zero +// value) preserves Phase 3 behaviour exactly. // // Returns a ScanResult with counts of inspected observations, candidates found, // already-related pairs skipped, relations inserted, and whether the cap was hit. @@ -1389,14 +1396,8 @@ func (s *Store) ScanProject(opts ScanOptions) (ScanResult, error) { return } - if verdict.Relation == RelationNotConflict { - mu.Lock() - result.SemanticSkipped++ - mu.Unlock() - return - } - - // Persist non-not_conflict verdict. + // Persist verdict (including not_conflict — it resolves the + // pending row so the pair is tracked as evaluated). _, judgeErr := s.JudgeBySemantic(JudgeBySemanticParams{ SourceID: pair.sourceSnippet.SyncID, TargetID: pair.candidateSnippet.SyncID, diff --git a/internal/store/scan_semantic_test.go b/internal/store/scan_semantic_test.go index 42c4982f..9b04ac31 100644 --- a/internal/store/scan_semantic_test.go +++ b/internal/store/scan_semantic_test.go @@ -30,8 +30,8 @@ func (r *errorRunner) Compare(_ context.Context, _ string) (SemanticVerdict, err // routingRunner routes calls to different runners based on the call index. type routingRunner struct { - runners []SemanticRunner - callIdx int + runners []SemanticRunner + callIdx int } func (r *routingRunner) Compare(ctx context.Context, prompt string) (SemanticVerdict, error) { @@ -144,11 +144,12 @@ func TestScanProject_Semantic_HappyPath(t *testing.T) { } } -// ─── C.5b — TestScanProject_Semantic_NotConflictSkipped ────────────────────── +// ─── C.5b — TestScanProject_Semantic_NotConflictPersisted ─────────────────── -// TestScanProject_Semantic_NotConflictSkipped verifies that verdicts of -// "not_conflict" are counted in SemanticSkipped and do NOT produce relation rows. -func TestScanProject_Semantic_NotConflictSkipped(t *testing.T) { +// TestScanProject_Semantic_NotConflictPersisted verifies that verdicts of +// "not_conflict" from automated scans are now persisted (counted in +// SemanticJudged) and produce a relation row with relation='not_conflict'. +func TestScanProject_Semantic_NotConflictPersisted(t *testing.T) { s := newTestStore(t) seedSimilarPair(t, s, "sem-skip-project") @@ -175,19 +176,17 @@ func TestScanProject_Semantic_NotConflictSkipped(t *testing.T) { t.Fatalf("ScanProject: %v", err) } - if result.SemanticSkipped == 0 { - t.Errorf("SemanticSkipped: want > 0; got 0 (CandidatesFound=%d)", result.CandidatesFound) - } - if result.SemanticJudged != 0 { - t.Errorf("SemanticJudged: want 0; got %d", result.SemanticJudged) + if result.SemanticJudged == 0 { + t.Errorf("SemanticJudged: want > 0; got 0 (not_conflict should now be persisted)") } var count int _ = s.db.QueryRow( - `SELECT count(*) FROM memory_relations WHERE marked_by_actor = 'engram'`, + `SELECT count(*) FROM memory_relations + WHERE marked_by_actor = 'engram' AND relation = 'not_conflict'`, ).Scan(&count) - if count != 0 { - t.Errorf("expected no 'engram' relation rows for not_conflict; got %d", count) + if count == 0 { + t.Errorf("expected not_conflict relation rows from ScanProject; got 0") } }