From 7ef96919d362cf7cf89a918713b3fe99ca8c2189 Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Thu, 11 Dec 2025 00:03:17 +0000
Subject: [PATCH] fix(ai): Make LLM finding IDs stable across patrol runs

The main issue was that finding IDs included the title, which the LLM
generates differently each time. 'High CPU on minipc' vs 'Node minipc
experiencing high CPU load' got different IDs, making dismissals useless.

Changes:
1. LLM findings now get IDs based on resource+category only, not title
2. Add() now checks if finding is suppressed before adding as new
3. Add() now checks dismissed findings and only reactivates on severity escalation
4. IsSuppressed() now matches by resource+category only, not title
5. Added isSuppressedInternal() for use when lock is already held

Now when you dismiss 'performance issues on minipc', any future patrol finding
about performance on minipc will be recognized as the same issue and stay dismissed.
---
 internal/ai/findings.go | 46 +++++++++++++++++++++++++++++++++++++++--
 internal/ai/patrol.go   |  6 ++++--
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/internal/ai/findings.go b/internal/ai/findings.go
index ee0a7df..fd29435 100644
--- a/internal/ai/findings.go
+++ b/internal/ai/findings.go
@@ -201,17 +201,47 @@ func (s *FindingsStore) ForceSave() error {
 
 // Add adds or updates a finding
 // If a finding with the same ID exists, it updates LastSeenAt and increments TimesRaised
+// If the finding is suppressed or dismissed, it may be skipped
 // Returns true if this is a new finding
 func (s *FindingsStore) Add(f *Finding) bool {
 	s.mu.Lock()
 
 	existing, exists := s.findings[f.ID]
 	if exists {
+		// Check if it's permanently suppressed - don't update at all
+		if existing.Suppressed {
+			s.mu.Unlock()
+			return false
+		}
+		
+		// Check if dismissed - only update if severity has escalated
+		if existing.DismissedReason != "" {
+			severityOrder := map[FindingSeverity]int{
+				FindingSeverityInfo:     0,
+				FindingSeverityWatch:    1,
+				FindingSeverityWarning:  2,
+				FindingSeverityCritical: 3,
+			}
+			// If new severity is same or lower, don't reactivate
+			if severityOrder[f.Severity] <= severityOrder[existing.Severity] {
+				existing.LastSeenAt = time.Now()
+				existing.TimesRaised++
+				s.mu.Unlock()
+				s.scheduleSave()
+				return false
+			}
+			// Severity escalated - clear dismissal and reactivate
+			existing.DismissedReason = ""
+			existing.UserNote = "" // Clear note since situation changed
+			existing.AcknowledgedAt = nil
+		}
+		
 		// Update existing finding
 		existing.LastSeenAt = time.Now()
 		existing.Description = f.Description
 		existing.Recommendation = f.Recommendation
 		existing.Evidence = f.Evidence
+		existing.Title = f.Title // Update title in case LLM phrased it better
 		existing.Severity = f.Severity
 		existing.TimesRaised++ // Track recurrence
 		s.mu.Unlock()
@@ -219,6 +249,12 @@ func (s *FindingsStore) Add(f *Finding) bool {
 		return false
 	}
 
+	// New finding - check if resource+category is suppressed
+	if s.isSuppressedInternal(f.ResourceID, f.Category) {
+		s.mu.Unlock()
+		return false
+	}
+
 	// New finding
 	if f.DetectedAt.IsZero() {
 		f.DetectedAt = time.Now()
@@ -377,14 +413,20 @@ func (s *FindingsStore) Suppress(id string) bool {
 }
 
 // IsSuppressed checks if findings of this type for this resource are suppressed
-func (s *FindingsStore) IsSuppressed(resourceID string, category FindingCategory, title string) bool {
+// Checks by resource+category only (not title, since LLM titles vary)
+func (s *FindingsStore) IsSuppressed(resourceID string, category FindingCategory) bool {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
+	return s.isSuppressedInternal(resourceID, category)
+}
 
+// isSuppressedInternal checks suppression without locking (caller must hold lock)
+func (s *FindingsStore) isSuppressedInternal(resourceID string, category FindingCategory) bool {
 	ids := s.byResource[resourceID]
 	for _, id := range ids {
 		if f, exists := s.findings[id]; exists {
-			if f.Suppressed && f.Category == category && f.Title == title {
+			// Match by resource+category only
+			if f.Suppressed && f.Category == category {
 				return true
 			}
 		}
diff --git a/internal/ai/patrol.go b/internal/ai/patrol.go
index ea39717..25ec632 100644
--- a/internal/ai/patrol.go
+++ b/internal/ai/patrol.go
@@ -1772,8 +1772,10 @@ func (p *PatrolService) parseFindingBlock(block string) *Finding {
 		cat = FindingCategoryPerformance
 	}
 
-	// Generate stable ID from content
-	id := generateFindingID(resource, string(cat), title)
+	// Generate stable ID from resource and category ONLY (not title)
+	// This ensures the same issue on the same resource gets the same ID even if
+	// the LLM phrases it differently each time
+	id := generateFindingID(resource, string(cat), "llm-finding")
 
 	return &Finding{
 		ID:             id,