mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-07-16 03:11:51 +00:00
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:
parent
41e41be851
commit
dad1152fc4
3 changed files with 183 additions and 0 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue