Surface finding-key collisions as lifecycle events instead of silent identity hijack

Patrol finding identity is the LLM-assigned key (resource+category+key
hash -> ID), and recordFinding adds by plain ID merge: when the LLM
reuses a key for a genuinely different issue on the same resource, the
new report silently overwrites the existing finding's title,
description, and evidence while inheriting its lifecycle — and on a
resolved finding, the reactivation counts as a regression of the OLD
issue, inflating the regression counter with a fiction.

Forking the key on dissimilar content would be worse: LLM titles vary
run to run, and splitting one real issue into duplicate findings is a
bigger trust hit than a conflated history. So the merge semantics stand,
and the identity shift is recorded honestly: when a same-ID
re-detection's title shares essentially no keywords with the existing
title (keywordOverlap <= findingIdentityShiftMaxTitleOverlap, 0.2 —
resource/category/key are equal by construction so text is the only
discriminating signal), FindingsStore.Add appends a content_replaced
lifecycle event preserving both titles in metadata and logs the
collision for frequency observability. Rephrasings and identical
re-detections stay event-free per the heartbeat rule. The UI renders
the event through the existing identifier-formatter fallback ("Content
replaced"); a dedicated label is patrol-intelligence UI work and lands
with that subsystem's own ceremony.

Teeth in findings_lifecycle_test.go: distinct-issue collision records
exactly one event with both titles; rephrased and identical
re-detections record none; the resolved-finding collision shows BOTH
content_replaced and regressed so the regression can be read for what
it is. Contract: Current State entry in ai-runtime.md. Full internal/ai
tree green.
This commit is contained in:
rcourtman 2026-06-09 22:03:20 +01:00
parent 41e41be851
commit dad1152fc4
3 changed files with 183 additions and 0 deletions

View file

@ -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

View file

@ -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

View file

@ -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)
}
}