diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 3002a5607..dd68e9138 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -2328,6 +2328,28 @@ deriving an older display status from `workflowStatusHistory`. ## Current State +Finding identity is the LLM-assigned key (resource+category+key hash → ID), +and key collisions are surfaced, not forked: when a same-ID re-detection's +title shares essentially no keywords with the existing finding's title +(`keywordOverlap` at or below `findingIdentityShiftMaxTitleOverlap`, +`internal/ai/findings.go`), the merge proceeds — the latest report still owns +the text, because key forking would split LLM rephrasings of one real issue +into duplicate findings — but `FindingsStore.Add` appends a +`content_replaced` lifecycle event preserving the previous and new titles in +metadata and logs the collision. This keeps the operator timeline honest +when a distinct issue reuses an existing finding's identity (including the +resolved case, where the reactivation otherwise reads as a regression of the +old issue: both `content_replaced` and `regressed` events appear). +Rephrasings and identical re-detections stay event-free per the +heartbeat-not-transition rule. The shared lifecycle presentation +(`frontend-modern/src/utils/aiFindingPresentation.ts`, +patrol-intelligence subsystem) renders unknown lifecycle types through the +identifier-formatter fallback, so the event reads "Content replaced" today; +a dedicated label is patrol-intelligence UI work and lands with that +subsystem's own ceremony. +`TestFindingsStore_KeyCollisionRecordsContentReplacedEvent` and its +companion tests (`internal/ai/findings_lifecycle_test.go`) pin the behavior. + The interaction-quality scenario corpus (`internal/ai/chat/interaction_scenario_corpus_test.go`) is the canonical regression home for chat-feel promises, mirroring the Discovery corpus diff --git a/internal/ai/findings.go b/internal/ai/findings.go index 97a4e83f5..c96af5c2e 100644 --- a/internal/ai/findings.go +++ b/internal/ai/findings.go @@ -8,6 +8,8 @@ import ( "sync" "time" + "github.com/rs/zerolog/log" + "github.com/rcourtman/pulse-go-rewrite/pkg/aicontracts" ) @@ -73,6 +75,13 @@ func CategorySupportsStaleAutoResolve(category FindingCategory) bool { } } +// findingIdentityShiftMaxTitleOverlap is the keyword-overlap (Jaccard) bar +// below which a same-ID re-detection's title counts as a substantially +// different report (a key collision) rather than a rephrasing of the same +// issue. Rephrasings share core keywords (resource names, symptom nouns) and +// land well above this; genuinely distinct issues rarely share any. +const findingIdentityShiftMaxTitleOverlap = 0.2 + // DismissReasonWillFixLater marks a finding as "I will fix this later" — an // operational commitment with an implicit deadline. See Finding.RemindAt. const DismissReasonWillFixLater = "will_fix_later" @@ -1454,6 +1463,32 @@ func (s *FindingsStore) Add(f *Finding) bool { existing.AcknowledgedAt = nil } + // Same-key re-detections normally describe the same issue, but the + // LLM-assigned key can collide: a substantially different report + // arriving under an existing finding's identity overwrites its text + // below, silently absorbing a distinct issue into this finding's + // history (and, on a resolved finding, counting it as a regression + // of the old issue). Key forking would be worse — LLM rephrasings + // would split one real issue into duplicate findings — so the merge + // stands, and the identity shift is recorded honestly in the + // lifecycle with the previous title preserved. Detection compares + // title keyword overlap: resource/category/key are equal by + // construction, so the text is the only discriminating signal, and + // plain rephrasings of the same issue share core keywords. + if existing.Title != "" && f.Title != "" && existing.Title != f.Title && + keywordOverlap(existing.Title, f.Title) <= findingIdentityShiftMaxTitleOverlap { + log.Info(). + Str("finding_id", existing.ID). + Str("key", existing.Key). + Str("previous_title", existing.Title). + Str("new_title", f.Title). + Msg("Finding key collision: re-detection replaced this finding's content with a substantially different report") + s.appendLifecycleLocked(existing, "content_replaced", + "Re-detected with substantially different details; the previous report's title is preserved in this event", + existing.LoopState, existing.LoopState, + map[string]string{"previous_title": existing.Title, "new_title": f.Title}) + } + // Update existing finding existing.LastSeenAt = time.Now() existing.Description = f.Description diff --git a/internal/ai/findings_lifecycle_test.go b/internal/ai/findings_lifecycle_test.go index 82220db8b..58fca8240 100644 --- a/internal/ai/findings_lifecycle_test.go +++ b/internal/ai/findings_lifecycle_test.go @@ -273,3 +273,129 @@ func TestFindingsStore_BlocksInvalidLoopStateTransition(t *testing.T) { t.Fatalf("expected loop_transition_violation, got %q", last.Type) } } + +// --- Key-collision identity-shift events ---------------------------------- +// +// The LLM-assigned finding key can collide: a substantially different report +// for the same resource+category+key lands on the existing finding's ID and +// overwrites its text. The merge is intentional (key forking would split LLM +// rephrasings of one issue into duplicate findings), but the identity shift +// must be recorded honestly as a content_replaced lifecycle event carrying +// the previous title. + +func newCollisionFinding(title, description string) *Finding { + return &Finding{ + ID: "lf-collision", + Key: "restart-loop", + ResourceID: "vm-100", + ResourceName: "vm-100", + Severity: FindingSeverityWarning, + Category: FindingCategoryReliability, + Title: title, + Description: description, + } +} + +func findLifecycleEvents(f *Finding, typ string) []FindingLifecycleEvent { + var events []FindingLifecycleEvent + for _, e := range f.Lifecycle { + if e.Type == typ { + events = append(events, e) + } + } + return events +} + +func TestFindingsStore_KeyCollisionRecordsContentReplacedEvent(t *testing.T) { + store := NewFindingsStore() + store.Add(newCollisionFinding( + "frigate service stuck in restart loop", + "frigate restarts every 30s after OOM kill", + )) + + // A distinct issue arrives under the same resource+category+key. + store.Add(newCollisionFinding( + "homeassistant zigbee bridge crashing repeatedly", + "zigbee2mqtt bridge exits with USB disconnect errors", + )) + + got := store.Get("lf-collision") + if got == nil { + t.Fatal("expected merged finding to exist") + } + events := findLifecycleEvents(got, "content_replaced") + if len(events) != 1 { + t.Fatalf("expected exactly one content_replaced event, got %d (lifecycle=%+v)", len(events), got.Lifecycle) + } + if events[0].Metadata["previous_title"] != "frigate service stuck in restart loop" { + t.Fatalf("expected previous title preserved in event meta, got %q", events[0].Metadata["previous_title"]) + } + if events[0].Metadata["new_title"] != "homeassistant zigbee bridge crashing repeatedly" { + t.Fatalf("expected new title in event meta, got %q", events[0].Metadata["new_title"]) + } + // Merge semantics are unchanged: the latest report owns the text. + if got.Title != "homeassistant zigbee bridge crashing repeatedly" { + t.Fatalf("expected merge to keep overwriting the title, got %q", got.Title) + } +} + +func TestFindingsStore_RephrasedRedetectionDoesNotRecordContentReplaced(t *testing.T) { + store := NewFindingsStore() + store.Add(newCollisionFinding( + "frigate service stuck in restart loop", + "frigate restarts every 30s after OOM kill", + )) + + // Same issue, rephrased — shares the core keywords (frigate, restart). + store.Add(newCollisionFinding( + "frigate container keeps hitting a restart loop", + "the frigate container restarts continuously after an OOM kill", + )) + + got := store.Get("lf-collision") + if events := findLifecycleEvents(got, "content_replaced"); len(events) != 0 { + t.Fatalf("rephrasing of the same issue must not record content_replaced, got %+v", events) + } +} + +func TestFindingsStore_IdenticalRedetectionDoesNotRecordContentReplaced(t *testing.T) { + store := NewFindingsStore() + store.Add(newCollisionFinding("frigate service stuck in restart loop", "d")) + store.Add(newCollisionFinding("frigate service stuck in restart loop", "d")) + + got := store.Get("lf-collision") + if events := findLifecycleEvents(got, "content_replaced"); len(events) != 0 { + t.Fatalf("identical re-detection must not record content_replaced, got %+v", events) + } +} + +func TestFindingsStore_KeyCollisionOnResolvedFindingRecordsBothEvents(t *testing.T) { + // The worst collision damage: a distinct new issue reusing a RESOLVED + // finding's identity counts as a regression of the old issue. The + // regression accounting stands (merge semantics), but the lifecycle must + // show the content shift so the "regression" can be read for what it is. + store := NewFindingsStore() + store.Add(newCollisionFinding( + "frigate service stuck in restart loop", + "frigate restarts every 30s after OOM kill", + )) + if !store.Resolve("lf-collision", false) { + t.Fatal("expected resolve to succeed") + } + + store.Add(newCollisionFinding( + "homeassistant zigbee bridge crashing repeatedly", + "zigbee2mqtt bridge exits with USB disconnect errors", + )) + + got := store.Get("lf-collision") + if got.ResolvedAt != nil { + t.Fatal("expected finding to be reactivated") + } + if len(findLifecycleEvents(got, "content_replaced")) != 1 { + t.Fatalf("expected content_replaced on resolved-finding collision, lifecycle=%+v", got.Lifecycle) + } + if len(findLifecycleEvents(got, "regressed")) != 1 { + t.Fatalf("expected regressed event to remain, lifecycle=%+v", got.Lifecycle) + } +}