diff --git a/docs/plans/418-watcher-deltas-chat.md b/docs/plans/418-watcher-deltas-chat.md new file mode 100644 index 00000000..6b456b72 --- /dev/null +++ b/docs/plans/418-watcher-deltas-chat.md @@ -0,0 +1,118 @@ +# 418 — Watcher Deltas + Chat Sessions + +## Problem + +The watcher re-investigates every incident on every PagerDuty refresh, +even when nothing has changed. This wastes AI tokens, floods the user +with duplicate assessments, and makes the investigation log noisy. +Additionally, investigations are scoped to the UI-selected incident +(`m.selectedIncident`) rather than the incident that actually triggered +the observation, creating identity mismatches. + +## Approach + +Incident-state diffing as the primary investigation gate, scoped +investigations, and the Chat interface abstraction. + +### Key decisions + +- **In-memory only.** No TurnStore, JSONL persistence, or file I/O. + Restarting srepd = fresh start. Keeps Diff pure and storage-agnostic. +- **Design choice (c) for context:** triggering incidents are + foregrounded in the observation context; sibling alerts appear as + background in a queue summary. +- **Delta gate is primary, cooldown is secondary.** If incident state + hasn't changed (`len(changes) == 0`), detectors don't run at all. + Cooldown-based dedup remains as a secondary rate limiter for cases + where an unrelated field changes. +- **First-sighting semantics:** a new incident with no prior snapshot + counts as changed (IncidentNew), so the first poll always triggers + investigation. + +## Deliverables + +| ID | Summary | Files | Commit | +|----|---------|-------|--------| +| D1a | Pure `Diff(prev, curr)` function, `Narrate`, `Snapshot` types | `pkg/delta/delta.go`, `pkg/delta/delta_test.go` | `df22984` | +| D1b | Wire delta into watcher, gate investigation on changes | `pkg/tui/watcher.go`, `pkg/tui/tui.go`, `pkg/tui/model.go`, `pkg/tui/watcher_integration_test.go` | `5883b99` | +| D2 | Scope investigations to triggering incident, not UI selection | `pkg/tui/watcher.go`, `pkg/tui/investigation.go`, `pkg/tui/model.go`, `pkg/tui/tui.go`, `pkg/tui/watcher_test.go`, `pkg/tui/investigation_test.go`, `pkg/tui/ask_wiring_test.go`, `pkg/tui/approvals_update_test.go` | `e0c41c2` | +| D3 | Feed delta changes into investigation seed context | `pkg/tui/watcher.go` (via `delta.Narrate`) | `5883b99` | +| D4 | Land Chat interface in pkg/ai (optional-interface pattern) | `pkg/ai/provider.go`, `pkg/ai/provider_test.go` | `4c56dc0` | +| D5 | Add `get_recent_events` tool as ClassRead | `pkg/ai/tools/handlers.go`, `pkg/ai/tools/handlers_test.go`, `pkg/tui/model.go` | `47046e9` | + +## Delta change kinds + +| Kind | When | +|------|------| +| IncidentNew | ID exists in current but not previous | +| IncidentResolved | ID exists in previous but not current | +| StatusChanged | Status field differs | +| UrgencyChanged | Urgency field differs | +| NoteAdded | Note count increased (skipped when previous count unknown) | +| AlertAdded | Alert count increased (skipped when previous count unknown) | +| IncidentUpdated | Title or service changed | + +**Removed:** `Escalated` — the PagerDuty `Incident` struct on the list +response has no `escalation_level` field (that field exists only on +`IncidentAlert`). The level was hardcoded to 0, so `Escalated` could +never fire. + +## Chat interface (D4) + +Follows the optional-interface pattern established by `StreamingProvider`: + +```go +type Chat interface { + Send(ctx context.Context, userMsg string) (string, error) + History() []Turn +} +``` + +Helper functions `SupportsChat(p Provider)` and `AsChat(p Provider)` +use type assertions — no provider is forced to implement Chat. + +## Test coverage + +- `pkg/delta`: 16 table-driven subtests covering all change kinds, + first sighting, reordering, empty inputs, escalation decrease + (no-change), and Narrate formatting +- `pkg/tui`: `TestDeltaGate_IdenticalRefreshesProduceOneInvestigation`, + `TestBuildObservationContext_ScopedToTriggeringIncident`, + `TestBuildObservationContext_MultipleTriggering`, + `TestBuildObservationContext_EmptyIncidentIDs`, + `TestBuildAskFromVerdict_UsesOriginatingIncident` +- `pkg/ai`: `TestSupportsChat`, `TestAsChat` +- `pkg/ai/tools`: `TestGetRecentEvents_{HappyPath,EmptyChanges,WithLimit,InvalidInput,IsClassRead}` + +## Revert-check properties + +1. **D1 delta gate:** identical refreshes produce zero changes → + `runDetectors` returns nil (test: `TestDeltaGate_IdenticalRefreshesProduceOneInvestigation`) +2. **D2 scoping:** observation context foregrounds triggering incidents, + not UI selection (test: `TestBuildObservationContext_ScopedToTriggeringIncident`) +3. **D5 get_recent_events:** tool is ClassRead and returns expected + JSON (test: `TestGetRecentEvents_IsClassRead`) + +## Post-review fixes (PR #427) + +| Defect | Fix | Tests | +|--------|-----|-------| +| M1: delta gate suppression untested | Added `TestRunDetectors_DeltaGateBothDirections` exercising both directions | Revert check: `if false &&` → test fails | +| M2: lazy cache false-change burst | Changed `NoteCount`/`AlertCount` to `*int`; nil = unknown, skip comparison | `TestToSnapshots_UnloadedCacheSuppressesFalseChanges`, `TestToSnapshots_GenuineNoteAdditionAfterCacheLoad`, `TestDiff_NilNoteCountSkipsComparison`, `TestDiff_ZeroToNonZeroNoteCountDetected` | +| M3: `incidentList < 2` guard untested | Added `TestRunDetectors_SingleIncidentNoInvestigation` | Guard is redundant with detector thresholds (defense in depth) | +| N1: EscalationLevel hardcoded to 0 | Removed `Escalated` kind + `EscalationLevel` from Snapshot | PagerDuty Incident has no escalation_level on list response | +| N2: ClusterID set but never compared | Removed from Snapshot | Was never populated by toSnapshots | +| N3: watcherDedup.seen unbounded | Added eviction when map exceeds 100 entries | `TestWatcherDedup/evicts_expired_entries_when_threshold_exceeded` | +| N4: Title/Service never compared | Added comparison, new `IncidentUpdated` kind | `TestDiff_TitleChanged`, `TestDiff_ServiceChanged` | + +**Dedup + delta coexistence:** the delta layer gates on whether incident +STATE changed; the dedup layer gates on whether the same OBSERVATION TEXT +was already investigated within the cooldown window. Both are needed. + +## Constraints + +- No new Go modules added +- No `//nolint` directives +- No `replace` directives or vendoring +- Pre-existing `cmd/` test failure (missing config keys) is not caused + by these changes diff --git a/pkg/ai/provider.go b/pkg/ai/provider.go index 75418ce3..c26c13d9 100644 --- a/pkg/ai/provider.go +++ b/pkg/ai/provider.go @@ -86,6 +86,42 @@ func ResolvedModel(p Provider) string { return mr.Model() } +// Chat is an optional interface a Provider may implement to support multi-turn +// conversations with accumulated history. Providers that do not implement Chat +// fall back to single-shot Query calls. The watcher uses this to maintain +// session continuity ("this is the same cluster I flagged 20 minutes ago"). +type Chat interface { + Send(ctx context.Context, userMsg string) (string, error) + History() []Turn +} + +// Turn represents a single message in a Chat history. +type Turn struct { + Role string // "user" or "assistant" + Content string +} + +// SupportsChat reports whether p implements the Chat interface. +func SupportsChat(p Provider) bool { + if p == nil { + return false + } + _, ok := p.(Chat) + return ok +} + +// AsChat returns p as a Chat if it implements the interface, or nil. +func AsChat(p Provider) Chat { + if p == nil { + return nil + } + c, ok := p.(Chat) + if !ok { + return nil + } + return c +} + // Config holds the configuration for an LLM API provider. type Config struct { Provider string `mapstructure:"provider"` diff --git a/pkg/ai/provider_test.go b/pkg/ai/provider_test.go index a3266676..fe905f8f 100644 --- a/pkg/ai/provider_test.go +++ b/pkg/ai/provider_test.go @@ -101,6 +101,45 @@ func TestResolvedModel(t *testing.T) { }) } +// chatProvider embeds a non-streaming provider and implements Chat. +type chatProvider struct{ nonStreamingProvider } + +func (chatProvider) Send(_ context.Context, _ string) (string, error) { + return "reply", nil +} +func (chatProvider) History() []Turn { return nil } + +func TestSupportsChat(t *testing.T) { + t.Run("provider without Chat is not chatty", func(t *testing.T) { + assert.False(t, SupportsChat(nonStreamingProvider{})) + }) + + t.Run("provider with Chat is chatty", func(t *testing.T) { + assert.True(t, SupportsChat(chatProvider{})) + }) + + t.Run("nil provider is not chatty", func(t *testing.T) { + assert.False(t, SupportsChat(nil)) + }) +} + +func TestAsChat(t *testing.T) { + t.Run("returns Chat for implementing provider", func(t *testing.T) { + c := AsChat(chatProvider{}) + assert.NotNil(t, c) + }) + + t.Run("returns nil for non-implementing provider", func(t *testing.T) { + c := AsChat(nonStreamingProvider{}) + assert.Nil(t, c) + }) + + t.Run("returns nil for nil provider", func(t *testing.T) { + c := AsChat(nil) + assert.Nil(t, c) + }) +} + func TestRealProviders_SupportStreaming(t *testing.T) { // All shipped providers implement real streaming; they must advertise it so the // TUI turns streaming on for them. diff --git a/pkg/ai/tools/handlers.go b/pkg/ai/tools/handlers.go index 3fd79623..76176caa 100644 --- a/pkg/ai/tools/handlers.go +++ b/pkg/ai/tools/handlers.go @@ -9,6 +9,7 @@ import ( "github.com/PagerDuty/go-pagerduty" "github.com/charmbracelet/log" "github.com/clcollins/srepd/pkg/ai/policy" + "github.com/clcollins/srepd/pkg/delta" "github.com/clcollins/srepd/pkg/ocm" "github.com/clcollins/srepd/pkg/pd" ) @@ -211,6 +212,64 @@ func newGetLimitedSupportTool(client ocm.OCMClient) Tool { } } +// RegisterDeltaTools registers the get_recent_events tool, which exposes +// recent incident-state changes to the AI. The getChanges function is called +// at handler time to read the current in-memory change log. +func RegisterDeltaTools(reg *Registry, getChanges func() []delta.Change) error { + return reg.Register(newGetRecentEventsTool(getChanges)) +} + +func newGetRecentEventsTool(getChanges func() []delta.Change) Tool { + return Tool{ + Name: "get_recent_events", + Description: "Get recent incident-state change events (new, resolved, status/urgency changes, new alerts/notes)", + Class: policy.ClassRead, + Schema: []byte(`{"type":"object","properties":{"limit":{"type":"integer","description":"Maximum number of recent events to return (default 50, max 200)"}}}`), + Handler: func(_ context.Context, input json.RawMessage) (string, error) { + var params struct { + Limit int `json:"limit"` + } + if len(input) > 0 { + if err := json.Unmarshal(input, ¶ms); err != nil { + return formatError("invalid input", err), nil + } + } + if params.Limit <= 0 { + params.Limit = 50 + } + if params.Limit > 200 { + params.Limit = 200 + } + + changes := getChanges() + if len(changes) == 0 { + return "[]", nil + } + + start := 0 + if len(changes) > params.Limit { + start = len(changes) - params.Limit + } + recent := changes[start:] + + type eventJSON struct { + Kind string `json:"kind"` + IncidentID string `json:"incident_id"` + Summary string `json:"summary"` + } + events := make([]eventJSON, 0, len(recent)) + for _, c := range recent { + events = append(events, eventJSON{ + Kind: c.Kind.String(), + IncidentID: c.IncidentID, + Summary: c.Summary, + }) + } + return marshalResult(events) + }, + } +} + func marshalResult(v any) (string, error) { data, err := json.Marshal(v) if err != nil { diff --git a/pkg/ai/tools/handlers_test.go b/pkg/ai/tools/handlers_test.go index 205ab13b..275dee51 100644 --- a/pkg/ai/tools/handlers_test.go +++ b/pkg/ai/tools/handlers_test.go @@ -3,10 +3,13 @@ package tools_test import ( "context" "encoding/json" + "fmt" "strings" "testing" + "github.com/clcollins/srepd/pkg/ai/policy" "github.com/clcollins/srepd/pkg/ai/tools" + "github.com/clcollins/srepd/pkg/delta" "github.com/clcollins/srepd/pkg/ocm" "github.com/clcollins/srepd/pkg/pd" "github.com/stretchr/testify/assert" @@ -249,6 +252,72 @@ func TestHandler_FormatErrorIncludesClass(t *testing.T) { assert.Contains(t, result, "(", "error string should include a parenthesized error class") } +func TestGetRecentEvents_HappyPath(t *testing.T) { + changes := []delta.Change{ + {Kind: delta.IncidentNew, IncidentID: "P1", Summary: "New incident: Alert A"}, + {Kind: delta.StatusChanged, IncidentID: "P2", Summary: "Status changed: triggered → acknowledged"}, + } + reg := tools.NewRegistry() + require.NoError(t, tools.RegisterDeltaTools(reg, func() []delta.Change { return changes })) + + tool := findTool(t, reg, "get_recent_events") + result, err := tool.Handler(context.Background(), json.RawMessage(`{}`)) + require.NoError(t, err) + assert.Contains(t, result, "P1") + assert.Contains(t, result, "P2") + assert.Contains(t, result, "new") + assert.Contains(t, result, "status_changed") +} + +func TestGetRecentEvents_EmptyChanges(t *testing.T) { + reg := tools.NewRegistry() + require.NoError(t, tools.RegisterDeltaTools(reg, func() []delta.Change { return nil })) + + tool := findTool(t, reg, "get_recent_events") + result, err := tool.Handler(context.Background(), json.RawMessage(`{}`)) + require.NoError(t, err) + assert.Equal(t, "[]", result) +} + +func TestGetRecentEvents_WithLimit(t *testing.T) { + var changes []delta.Change + for i := 0; i < 10; i++ { + changes = append(changes, delta.Change{ + Kind: delta.IncidentNew, + IncidentID: fmt.Sprintf("P%d", i), + Summary: fmt.Sprintf("Event %d", i), + }) + } + reg := tools.NewRegistry() + require.NoError(t, tools.RegisterDeltaTools(reg, func() []delta.Change { return changes })) + + tool := findTool(t, reg, "get_recent_events") + result, err := tool.Handler(context.Background(), json.RawMessage(`{"limit":3}`)) + require.NoError(t, err) + + var parsed []map[string]string + require.NoError(t, json.Unmarshal([]byte(result), &parsed)) + assert.Len(t, parsed, 3, "limit must cap returned events") + assert.Equal(t, "P7", parsed[0]["incident_id"], "must return most recent events") +} + +func TestGetRecentEvents_InvalidInput(t *testing.T) { + reg := tools.NewRegistry() + require.NoError(t, tools.RegisterDeltaTools(reg, func() []delta.Change { return nil })) + + tool := findTool(t, reg, "get_recent_events") + result, err := tool.Handler(context.Background(), json.RawMessage(`{invalid}`)) + require.NoError(t, err) + assert.Contains(t, result, "invalid input") +} + +func TestGetRecentEvents_IsClassRead(t *testing.T) { + reg := tools.NewRegistry() + require.NoError(t, tools.RegisterDeltaTools(reg, func() []delta.Change { return nil })) + tool := findTool(t, reg, "get_recent_events") + assert.Equal(t, policy.ClassRead, tool.Class) +} + // findTool finds a tool by name in the registry. func findTool(t *testing.T, reg *tools.Registry, name string) tools.Tool { t.Helper() diff --git a/pkg/delta/delta.go b/pkg/delta/delta.go new file mode 100644 index 00000000..9db78b99 --- /dev/null +++ b/pkg/delta/delta.go @@ -0,0 +1,188 @@ +package delta + +import ( + "fmt" + "strings" + "time" +) + +// ChangeKind classifies a state transition between consecutive polls. +type ChangeKind int + +const ( + IncidentNew ChangeKind = iota // first sighting — no prior state + IncidentResolved // was present, now absent + StatusChanged + UrgencyChanged + NoteAdded + AlertAdded + IncidentUpdated // title or service changed +) + +func (k ChangeKind) String() string { + switch k { + case IncidentNew: + return "new" + case IncidentResolved: + return "resolved" + case StatusChanged: + return "status_changed" + case UrgencyChanged: + return "urgency_changed" + case NoteAdded: + return "note_added" + case AlertAdded: + return "alert_added" + case IncidentUpdated: + return "incident_updated" + default: + return "unknown" + } +} + +// Change represents a single state transition for an incident. +type Change struct { + Kind ChangeKind + IncidentID string + Summary string +} + +// Snapshot captures the fingerprint-relevant fields of an incident at a point +// in time. Pure value type — no I/O. +// +// NoteCount and AlertCount use *int to distinguish "unknown/not yet loaded" +// (nil) from "loaded and genuinely zero" (&0). Diff skips note/alert +// comparisons when the previous value is nil, preventing false-change bursts +// when the lazy enrichment cache loads between polls. +type Snapshot struct { + ID string + Title string + Service string + Status string + Urgency string + NoteCount *int + AlertCount *int +} + +// SnapshotFromFields constructs a Snapshot from individual fields, avoiding a +// dependency on any PagerDuty type in this package. +func SnapshotFromFields(id, title, service, status, urgency string, noteCount, alertCount *int) Snapshot { + return Snapshot{ + ID: id, + Title: title, + Service: service, + Status: status, + Urgency: urgency, + NoteCount: noteCount, + AlertCount: alertCount, + } +} + +// Diff computes changes between prev and curr snapshots. Pure function: values +// in, values out, no I/O. First-sighting semantics: a snapshot in curr with no +// match in prev produces IncidentNew. A snapshot in prev with no match in curr +// produces IncidentResolved. +func Diff(prev, curr []Snapshot) []Change { + prevMap := make(map[string]Snapshot, len(prev)) + for _, s := range prev { + prevMap[s.ID] = s + } + + currMap := make(map[string]Snapshot, len(curr)) + for _, s := range curr { + currMap[s.ID] = s + } + + var changes []Change + + // Detect new and changed incidents (iterate curr in order for determinism) + for _, c := range curr { + p, existed := prevMap[c.ID] + if !existed { + changes = append(changes, Change{ + Kind: IncidentNew, + IncidentID: c.ID, + Summary: fmt.Sprintf("New incident: %s (%s)", c.Title, c.Service), + }) + continue + } + if p.Title != c.Title { + changes = append(changes, Change{ + Kind: IncidentUpdated, + IncidentID: c.ID, + Summary: fmt.Sprintf("Title changed: %s → %s", p.Title, c.Title), + }) + } + if p.Service != c.Service { + changes = append(changes, Change{ + Kind: IncidentUpdated, + IncidentID: c.ID, + Summary: fmt.Sprintf("Service changed: %s → %s", p.Service, c.Service), + }) + } + if p.Status != c.Status { + changes = append(changes, Change{ + Kind: StatusChanged, + IncidentID: c.ID, + Summary: fmt.Sprintf("Status changed: %s → %s", p.Status, c.Status), + }) + } + if p.Urgency != c.Urgency { + changes = append(changes, Change{ + Kind: UrgencyChanged, + IncidentID: c.ID, + Summary: fmt.Sprintf("Urgency changed: %s → %s", p.Urgency, c.Urgency), + }) + } + if p.NoteCount != nil && c.NoteCount != nil && *c.NoteCount > *p.NoteCount { + added := *c.NoteCount - *p.NoteCount + changes = append(changes, Change{ + Kind: NoteAdded, + IncidentID: c.ID, + Summary: fmt.Sprintf("%d new note(s)", added), + }) + } + if p.AlertCount != nil && c.AlertCount != nil && *c.AlertCount > *p.AlertCount { + added := *c.AlertCount - *p.AlertCount + changes = append(changes, Change{ + Kind: AlertAdded, + IncidentID: c.ID, + Summary: fmt.Sprintf("%d new alert(s)", added), + }) + } + } + + // Detect resolved incidents (iterate prev in order for determinism) + for _, p := range prev { + if _, exists := currMap[p.ID]; !exists { + changes = append(changes, Change{ + Kind: IncidentResolved, + IncidentID: p.ID, + Summary: fmt.Sprintf("Resolved: %s", p.Title), + }) + } + } + + return changes +} + +// Narrate formats changes into a compact narrative block for the LLM. +// Pure function: changes + reference time in, string out. +func Narrate(changes []Change, now time.Time) string { + if len(changes) == 0 { + return "" + } + + var lines []string + for _, c := range changes { + lines = append(lines, fmt.Sprintf("- [%s] %s: %s", c.IncidentID, c.Kind, c.Summary)) + } + + const maxLines = 20 + if len(lines) > maxLines { + lines = lines[:maxLines] + lines = append(lines, fmt.Sprintf("... and %d more changes", len(changes)-maxLines)) + } + + return fmt.Sprintf("Changes since last check:\n%s", strings.Join(lines, "\n")) +} diff --git a/pkg/delta/delta_test.go b/pkg/delta/delta_test.go new file mode 100644 index 00000000..7833636a --- /dev/null +++ b/pkg/delta/delta_test.go @@ -0,0 +1,260 @@ +package delta + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func intPtr(n int) *int { return &n } + +func TestDiff_NoChange(t *testing.T) { + snaps := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high"}, + {ID: "P2", Title: "Alert B", Service: "svc-b", Status: "acknowledged", Urgency: "low"}, + } + changes := Diff(snaps, snaps) + assert.Empty(t, changes, "identical snapshots must produce no changes") +} + +func TestDiff_NewIncident(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high"}, + } + curr := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high"}, + {ID: "P2", Title: "New Alert", Service: "svc-b", Status: "triggered", Urgency: "high"}, + } + changes := Diff(prev, curr) + require.Len(t, changes, 1) + assert.Equal(t, IncidentNew, changes[0].Kind) + assert.Equal(t, "P2", changes[0].IncidentID) + assert.Contains(t, changes[0].Summary, "New Alert") +} + +func TestDiff_IncidentResolved(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high"}, + {ID: "P2", Title: "Alert B", Service: "svc-b", Status: "triggered", Urgency: "high"}, + } + curr := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high"}, + } + changes := Diff(prev, curr) + require.Len(t, changes, 1) + assert.Equal(t, IncidentResolved, changes[0].Kind) + assert.Equal(t, "P2", changes[0].IncidentID) +} + +func TestDiff_StatusChange(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high"}, + } + curr := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "acknowledged", Urgency: "high"}, + } + changes := Diff(prev, curr) + require.Len(t, changes, 1) + assert.Equal(t, StatusChanged, changes[0].Kind) + assert.Contains(t, changes[0].Summary, "triggered") + assert.Contains(t, changes[0].Summary, "acknowledged") +} + +func TestDiff_UrgencyChange(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "low"}, + } + curr := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high"}, + } + changes := Diff(prev, curr) + require.Len(t, changes, 1) + assert.Equal(t, UrgencyChanged, changes[0].Kind) + assert.Contains(t, changes[0].Summary, "low") + assert.Contains(t, changes[0].Summary, "high") +} + +func TestDiff_TitleChanged(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "Old Title", Service: "svc-a", Status: "triggered", Urgency: "high"}, + } + curr := []Snapshot{ + {ID: "P1", Title: "New Title", Service: "svc-a", Status: "triggered", Urgency: "high"}, + } + changes := Diff(prev, curr) + require.Len(t, changes, 1) + assert.Equal(t, IncidentUpdated, changes[0].Kind) + assert.Contains(t, changes[0].Summary, "Old Title") + assert.Contains(t, changes[0].Summary, "New Title") +} + +func TestDiff_ServiceChanged(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "Alert", Service: "svc-old", Status: "triggered", Urgency: "high"}, + } + curr := []Snapshot{ + {ID: "P1", Title: "Alert", Service: "svc-new", Status: "triggered", Urgency: "high"}, + } + changes := Diff(prev, curr) + require.Len(t, changes, 1) + assert.Equal(t, IncidentUpdated, changes[0].Kind) + assert.Contains(t, changes[0].Summary, "svc-old") + assert.Contains(t, changes[0].Summary, "svc-new") +} + +func TestDiff_NoteAdded(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high", NoteCount: intPtr(2)}, + } + curr := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high", NoteCount: intPtr(4)}, + } + changes := Diff(prev, curr) + require.Len(t, changes, 1) + assert.Equal(t, NoteAdded, changes[0].Kind) + assert.Contains(t, changes[0].Summary, "2 new note(s)") +} + +func TestDiff_AlertAdded(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high", AlertCount: intPtr(1)}, + } + curr := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high", AlertCount: intPtr(3)}, + } + changes := Diff(prev, curr) + require.Len(t, changes, 1) + assert.Equal(t, AlertAdded, changes[0].Kind) + assert.Contains(t, changes[0].Summary, "2 new alert(s)") +} + +func TestDiff_MultipleChanges(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "low", NoteCount: intPtr(1), AlertCount: intPtr(1)}, + } + curr := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "acknowledged", Urgency: "high", NoteCount: intPtr(3), AlertCount: intPtr(2)}, + } + changes := Diff(prev, curr) + assert.Len(t, changes, 4, "status + urgency + notes + alerts") +} + +func TestDiff_NilNoteCountSkipsComparison(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "A", Service: "svc", Status: "triggered", Urgency: "high", NoteCount: nil}, + } + curr := []Snapshot{ + {ID: "P1", Title: "A", Service: "svc", Status: "triggered", Urgency: "high", NoteCount: intPtr(5)}, + } + changes := Diff(prev, curr) + for _, c := range changes { + assert.NotEqual(t, NoteAdded, c.Kind, "nil→known must not produce NoteAdded") + } +} + +func TestDiff_NilAlertCountSkipsComparison(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "A", Service: "svc", Status: "triggered", Urgency: "high", AlertCount: nil}, + } + curr := []Snapshot{ + {ID: "P1", Title: "A", Service: "svc", Status: "triggered", Urgency: "high", AlertCount: intPtr(3)}, + } + changes := Diff(prev, curr) + for _, c := range changes { + assert.NotEqual(t, AlertAdded, c.Kind, "nil→known must not produce AlertAdded") + } +} + +func TestDiff_ZeroToNonZeroNoteCountDetected(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "A", Service: "svc", Status: "triggered", Urgency: "high", NoteCount: intPtr(0)}, + } + curr := []Snapshot{ + {ID: "P1", Title: "A", Service: "svc", Status: "triggered", Urgency: "high", NoteCount: intPtr(1)}, + } + changes := Diff(prev, curr) + require.Len(t, changes, 1) + assert.Equal(t, NoteAdded, changes[0].Kind, "genuine 0→1 must be detected") +} + +func TestDiff_FirstSighting(t *testing.T) { + curr := []Snapshot{ + {ID: "P1", Title: "Alert A", Service: "svc-a", Status: "triggered", Urgency: "high"}, + {ID: "P2", Title: "Alert B", Service: "svc-b", Status: "triggered", Urgency: "low"}, + } + changes := Diff(nil, curr) + assert.Len(t, changes, 2, "every incident on first sighting must produce IncidentNew") + for _, c := range changes { + assert.Equal(t, IncidentNew, c.Kind) + } +} + +func TestDiff_ReorderingOnly(t *testing.T) { + prev := []Snapshot{ + {ID: "P1", Title: "A", Service: "svc", Status: "triggered", Urgency: "high"}, + {ID: "P2", Title: "B", Service: "svc", Status: "triggered", Urgency: "high"}, + } + curr := []Snapshot{ + {ID: "P2", Title: "B", Service: "svc", Status: "triggered", Urgency: "high"}, + {ID: "P1", Title: "A", Service: "svc", Status: "triggered", Urgency: "high"}, + } + changes := Diff(prev, curr) + assert.Empty(t, changes, "reordering without field changes must produce no changes") +} + +func TestDiff_EmptyBoth(t *testing.T) { + changes := Diff(nil, nil) + assert.Empty(t, changes) +} + +func TestNarrate_Empty(t *testing.T) { + result := Narrate(nil, time.Now()) + assert.Equal(t, "", result) +} + +func TestNarrate_SingleChange(t *testing.T) { + changes := []Change{ + {Kind: IncidentNew, IncidentID: "P1", Summary: "New incident: Alert A (svc-a)"}, + } + result := Narrate(changes, time.Now()) + assert.Contains(t, result, "Changes since last check") + assert.Contains(t, result, "P1") + assert.Contains(t, result, "new") + assert.Contains(t, result, "Alert A") +} + +func TestNarrate_MultipleChanges(t *testing.T) { + changes := []Change{ + {Kind: IncidentNew, IncidentID: "P1", Summary: "New incident"}, + {Kind: StatusChanged, IncidentID: "P2", Summary: "Status changed"}, + } + result := Narrate(changes, time.Now()) + assert.Contains(t, result, "P1") + assert.Contains(t, result, "P2") +} + +func TestNarrate_CapsAtMaxLines(t *testing.T) { + var changes []Change + for i := 0; i < 25; i++ { + changes = append(changes, Change{ + Kind: IncidentNew, + IncidentID: "P" + string(rune('A'+i)), + Summary: "New incident", + }) + } + result := Narrate(changes, time.Now()) + assert.Contains(t, result, "... and 5 more changes") +} + +func TestChangeKind_String(t *testing.T) { + assert.Equal(t, "new", IncidentNew.String()) + assert.Equal(t, "resolved", IncidentResolved.String()) + assert.Equal(t, "status_changed", StatusChanged.String()) + assert.Equal(t, "urgency_changed", UrgencyChanged.String()) + assert.Equal(t, "note_added", NoteAdded.String()) + assert.Equal(t, "alert_added", AlertAdded.String()) + assert.Equal(t, "incident_updated", IncidentUpdated.String()) + assert.Equal(t, "unknown", ChangeKind(99).String()) +} diff --git a/pkg/tui/approvals_update_test.go b/pkg/tui/approvals_update_test.go index 0035f905..550b994b 100644 --- a/pkg/tui/approvals_update_test.go +++ b/pkg/tui/approvals_update_test.go @@ -105,7 +105,7 @@ func TestUpdate_ApprovalsEnter_ReturnsCmdThatPostsNote(t *testing.T) { Tier: tools.TierActionable, Summary: "Post investigation note", Action: noteContent, - }) + }, nil) m.approvals.Add(ask) // Simulate user browsing to a different incident after ask creation diff --git a/pkg/tui/ask_wiring_test.go b/pkg/tui/ask_wiring_test.go index 8c38d0c6..1d808a7a 100644 --- a/pkg/tui/ask_wiring_test.go +++ b/pkg/tui/ask_wiring_test.go @@ -24,7 +24,7 @@ func TestBuildAskFromVerdict_DraftNote_ActionCallsPDAddNote(t *testing.T) { Action: "The cluster error rate is above threshold — posting investigation note", } - ask := m.buildAskFromVerdict(verdict) + ask := m.buildAskFromVerdict(verdict, nil) assert.Equal(t, AskDraftNote, ask.Kind) require.NotNil(t, ask.Action, "DraftNote Action must not be nil") @@ -52,7 +52,7 @@ func TestBuildAskFromVerdict_SuggestedCommand_CopiesNotExecutes(t *testing.T) { Action: cmdText, } - ask := m.buildAskFromVerdict(verdict) + ask := m.buildAskFromVerdict(verdict, nil) assert.Equal(t, AskSuggestedCommand, ask.Kind) require.NotNil(t, ask.Action, "SuggestedCommand Action must not be nil") @@ -83,7 +83,7 @@ func TestBuildAskFromVerdict_EscalationSuggestion_ReEscalates(t *testing.T) { Action: "Re-escalate this incident to the on-call team", } - ask := m.buildAskFromVerdict(verdict) + ask := m.buildAskFromVerdict(verdict, nil) assert.Equal(t, AskEscalationSuggestion, ask.Kind) require.NotNil(t, ask.Action, "EscalationSuggestion Action must not be nil") @@ -113,7 +113,7 @@ func TestBuildAskFromVerdict_UnknownText_FallbackHasAction(t *testing.T) { Action: "Something happened that does not match any known pattern", } - ask := m.buildAskFromVerdict(verdict) + ask := m.buildAskFromVerdict(verdict, nil) assert.Equal(t, AskDraftNote, ask.Kind, "inferAskKind fallback must return AskDraftNote, not zero-value") @@ -169,7 +169,7 @@ func TestBuildAskFromVerdict_DraftNote_TargetsOriginalIncident(t *testing.T) { Summary: "Post note", Action: "Note content for incident A", } - ask := m.buildAskFromVerdict(verdict) + ask := m.buildAskFromVerdict(verdict, nil) assert.Equal(t, "INC-A", ask.IncidentID, "Ask must snapshot the incident ID at creation time") @@ -210,7 +210,7 @@ func TestBuildAskFromVerdict_Escalation_TargetsOriginalIncident(t *testing.T) { Summary: "Re-escalate", Action: "Re-escalate this incident", } - ask := m.buildAskFromVerdict(verdict) + ask := m.buildAskFromVerdict(verdict, nil) assert.Equal(t, "INC-A", ask.IncidentID) @@ -237,7 +237,7 @@ func TestBuildAskFromVerdict_NilSelectedIncident_NoAction(t *testing.T) { Summary: "Re-escalate", Action: "Re-escalate this incident", } - ask := m.buildAskFromVerdict(verdict) + ask := m.buildAskFromVerdict(verdict, nil) assert.Empty(t, ask.IncidentID, "no incident selected means empty IncidentID") require.NotNil(t, ask.Action) @@ -263,7 +263,7 @@ func TestBuildAskFromVerdict_SanitizesControlSequences(t *testing.T) { Action: "Injected\x1b[2Jaction\x07text", } - ask := m.buildAskFromVerdict(verdict) + ask := m.buildAskFromVerdict(verdict, nil) assert.Equal(t, "Cleantitle", ask.Title, "Ask.Title must have control sequences stripped") @@ -285,7 +285,7 @@ func TestBuildAskFromVerdict_UnhandledKind_FallbackAction(t *testing.T) { Action: "Permission to run write_note tool", } - ask := m.buildAskFromVerdict(verdict) + ask := m.buildAskFromVerdict(verdict, nil) require.NotNil(t, ask.Action, "even if the AskKind switch has no matching case, Action must not be nil") diff --git a/pkg/tui/investigation.go b/pkg/tui/investigation.go index c8d48fd8..a36a0a22 100644 --- a/pkg/tui/investigation.go +++ b/pkg/tui/investigation.go @@ -36,6 +36,7 @@ type investigationMsg struct { verdict tools.Verdict err error toolAsks []toolAsk + incidentIDs []string // triggering incident IDs for scoped actions } // investigationConfig holds the settings for a watcher investigation. @@ -65,12 +66,14 @@ func watcherInvestigateCmd( contextStr string, model string, onAsk func(toolName string, input json.RawMessage), + incidentIDs []string, ) tea.Cmd { return func() tea.Msg { if runner == nil || registry == nil { return investigationMsg{ observation: observation, err: fmt.Errorf("tool runner or registry not configured"), + incidentIDs: incidentIDs, } } @@ -117,6 +120,7 @@ func watcherInvestigateCmd( return investigationMsg{ observation: observation, err: fmt.Errorf("investigation: model not configured; set llm_api.model or use a provider with a default"), + incidentIDs: incidentIDs, } } @@ -144,6 +148,7 @@ func watcherInvestigateCmd( return investigationMsg{ observation: observation, err: err, + incidentIDs: incidentIDs, } } if msg == nil { @@ -168,6 +173,7 @@ func watcherInvestigateCmd( observation: observation, verdict: verdict, toolAsks: asks, + incidentIDs: incidentIDs, } } } diff --git a/pkg/tui/investigation_test.go b/pkg/tui/investigation_test.go index b8bbe7cb..032c5c41 100644 --- a/pkg/tui/investigation_test.go +++ b/pkg/tui/investigation_test.go @@ -95,6 +95,7 @@ func TestWatcherInvestigateCmd_DenyEverything_ProductionPath(t *testing.T) { "Test context", "claude-sonnet-4-6", nil, + nil, ) msg := cmd() @@ -175,6 +176,7 @@ func TestWatcherInvestigateCmd_AllowPath_HandlerRuns(t *testing.T) { "Test context", "claude-sonnet-4-6", nil, + nil, ) msg := cmd() @@ -229,7 +231,7 @@ func TestWatcherInvestigateCmd_ModelPlumbing_ExplicitModel(t *testing.T) { } configuredModel := "us.anthropic.claude-sonnet-4-6" - cmd := watcherInvestigateCmd(factory, reg, cfg, "test prompt", "obs", "ctx", configuredModel, nil) + cmd := watcherInvestigateCmd(factory, reg, cfg, "test prompt", "obs", "ctx", configuredModel, nil, nil) msg := cmd() result, ok := msg.(investigationMsg) require.True(t, ok) @@ -257,7 +259,7 @@ func TestWatcherInvestigateCmd_ModelPlumbing_EmptyModelReturnsError(t *testing.T option.WithBaseURL(server.URL), ) - cmd := watcherInvestigateCmd(&client.Beta.Messages, reg, cfg, "prompt", "obs", "ctx", "", nil) + cmd := watcherInvestigateCmd(&client.Beta.Messages, reg, cfg, "prompt", "obs", "ctx", "", nil, nil) msg := cmd() result, ok := msg.(investigationMsg) require.True(t, ok) @@ -323,7 +325,7 @@ func TestWatcherInvestigateCmd_AskDedup_SameToolAndInput(t *testing.T) { cmd := watcherInvestigateCmd( &client.Beta.Messages, reg, cfg, - "test prompt", "obs", "ctx", "claude-sonnet-4-6", nil, + "test prompt", "obs", "ctx", "claude-sonnet-4-6", nil, nil, ) msg := cmd() diff --git a/pkg/tui/model.go b/pkg/tui/model.go index 4ef85592..179a9c99 100644 --- a/pkg/tui/model.go +++ b/pkg/tui/model.go @@ -26,6 +26,7 @@ import ( "github.com/clcollins/srepd/pkg/ai/tools" "github.com/clcollins/srepd/pkg/backplane" pkgconfig "github.com/clcollins/srepd/pkg/config" + "github.com/clcollins/srepd/pkg/delta" "github.com/clcollins/srepd/pkg/docs" "github.com/clcollins/srepd/pkg/launcher" "github.com/clcollins/srepd/pkg/ocm" @@ -170,6 +171,8 @@ type model struct { watcherMarker string agentMarker string watcherDedup *watcherDedup + prevSnapshots []delta.Snapshot // previous poll's snapshots for diffing + recentChanges []delta.Change // bounded log of recent changes (max 200) watcherAnalyzing bool watcherQueryStart time.Time watcherQueryTimeout time.Duration @@ -954,7 +957,7 @@ func defaultLogFilePath() string { return logFilePathForOS(runtime.GOOS) } -func (m *model) buildAskFromVerdict(verdict tools.Verdict) Ask { +func (m *model) buildAskFromVerdict(verdict tools.Verdict, originatingIncidentIDs []string) Ask { kind := inferAskKind(verdict.Action) ask := Ask{ Kind: kind, @@ -962,10 +965,17 @@ func (m *model) buildAskFromVerdict(verdict tools.Verdict) Ask { Body: stripControl(verdict.Action), } - // Snapshot the incident identity at creation time so that actions - // always target the incident that seeded the investigation, never - // whichever incident happens to be selected when the user accepts. - if m.selectedIncident != nil { + // Use the investigation's originating incident rather than whichever + // incident happens to be selected in the UI. This fixes D2: an ambient + // watcher must not depend on UI selection state. + var originInc *pagerduty.Incident + if len(originatingIncidentIDs) > 0 { + originInc = findIncidentByID(m.incidentList, originatingIncidentIDs[0]) + } + if originInc != nil { + ask.IncidentID = originInc.ID + ask.IncidentTitle = originInc.Title + } else if m.selectedIncident != nil { ask.IncidentID = m.selectedIncident.ID ask.IncidentTitle = m.selectedIncident.Title } @@ -1062,6 +1072,11 @@ func initToolRegistryForModel(m *model) { log.Warn("ai.tools", "msg", "failed to register OCM tools", "error", err) } } + if err := tools.RegisterDeltaTools(reg, func() []delta.Change { + return m.recentChanges + }); err != nil { + log.Warn("ai.tools", "msg", "failed to register delta tools", "error", err) + } m.toolRegistry = reg log.Info("ai.tools", "msg", "tool registry initialized", "tools", len(reg.Tools())) } diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index b2e5edda..c1959da5 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -454,10 +454,9 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { msg.condition.ID = m.flagNextID m.flagConditions = append(m.flagConditions, msg.condition) m.rebuildFlagMatchCache() - watcherCmds := m.runDetectors() flashCmd := m.flashNotification(fmt.Sprintf("flag #%d added: %s", msg.condition.ID, msg.condition.Label)) rebuildCmd := func() tea.Msg { return updatedIncidentListMsg{m.incidentList, nil} } - return m, tea.Batch(append(watcherCmds, flashCmd, rebuildCmd)...) + return m, tea.Batch(flashCmd, rebuildCmd) case removeFlagConditionMsg: for i, c := range m.flagConditions { @@ -467,10 +466,9 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } } m.rebuildFlagMatchCache() - watcherCmds := m.runDetectors() flashCmd := m.flashNotification(fmt.Sprintf("flag #%d removed", msg.id)) rebuildCmd := func() tea.Msg { return updatedIncidentListMsg{m.incidentList, nil} } - return m, tea.Batch(append(watcherCmds, flashCmd, rebuildCmd)...) + return m, tea.Batch(flashCmd, rebuildCmd) case clearFlagConditionsMsg: m.flagConditions = nil @@ -734,7 +732,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case tools.TierActionable: m.watcherBuffer.Append("") if msg.verdict.Action != "" { - ask := m.buildAskFromVerdict(msg.verdict) + ask := m.buildAskFromVerdict(msg.verdict, msg.incidentIDs) m.approvals.Add(ask) } return m, m.startTypewriter(m.watcherMarker, msg.verdict.Summary) @@ -1255,7 +1253,8 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { cmds = append(cmds, cmd) } - cmds = append(cmds, m.runDetectors()...) + changes := m.computeAndStoreDeltas() + cmds = append(cmds, m.runDetectors(changes)...) case parseTemplateForNoteMsg: if m.selectedIncident == nil { diff --git a/pkg/tui/watcher.go b/pkg/tui/watcher.go index 642f994a..a72b6ac8 100644 --- a/pkg/tui/watcher.go +++ b/pkg/tui/watcher.go @@ -11,6 +11,7 @@ import ( "github.com/charmbracelet/lipgloss" "github.com/charmbracelet/log" "github.com/clcollins/srepd/pkg/ai" + "github.com/clcollins/srepd/pkg/delta" ) const ( @@ -189,7 +190,8 @@ func (m *model) advanceTypewriter() tea.Cmd { } type watcherObservation struct { - Summary string + Summary string + IncidentIDs []string // triggering incident IDs for scoped context } type watcherDedup struct { @@ -204,44 +206,66 @@ func newWatcherDedup(cooldown time.Duration) *watcherDedup { } } +const watcherDedupEvictThreshold = 100 + func (d *watcherDedup) IsNew(observation string) bool { h := fmt.Sprintf("%x", sha256.Sum256([]byte(observation))) if last, ok := d.seen[h]; ok && time.Since(last) < d.cooldown { return false } d.seen[h] = time.Now() + if len(d.seen) > watcherDedupEvictThreshold { + d.evictExpired() + } return true } -func (m *model) runDetectors() []tea.Cmd { +func (d *watcherDedup) evictExpired() { + for k, ts := range d.seen { + if time.Since(ts) >= d.cooldown { + delete(d.seen, k) + } + } +} + +func (m *model) runDetectors(changes []delta.Change) []tea.Cmd { if len(m.incidentList) < 2 { return nil } + // D1 gate: only investigate when the incident state actually changed. + // On first poll (no previous state), every incident is a first-sighting + // and produces IncidentNew changes — that is correct. + if len(changes) == 0 { + return nil + } + observations := detectAll(m.incidentList, m.incidentClusterMap) var cmds []tea.Cmd added := false for _, obs := range observations { + // Secondary rate limit: suppress re-investigation of the same + // observation text within the cooldown window, even if the delta + // gate fires (e.g. an unrelated field changed). if !m.watcherDedup.IsNew(obs.Summary) { continue } log.Debug("watcher.runDetectors", "observation", obs.Summary) - // Ambient synthesis runs unless the provider is in a known-error - // state — unverified providers get their first health signal from - // this very query. if m.aiProvider != nil && m.aiHealth != aiHealthError && !m.watcherAnalyzing { m.watcherAnalyzing = true m.watcherQueryStart = time.Now() m.watcherQueryTimeout = watcherSynthesisTimeout - // If the provider supports tools (Anthropic family) and a tool - // registry is available, run a tool-using investigation. if m.toolRunnerFactory != nil && m.toolRegistry != nil && isAnthropicFamily(m.aiProvider.Name()) { m.watcherQueryTimeout = m.investigationCfg.timeout - contextStr := buildWatcherContext(m) + contextStr := buildObservationContext(m, obs) + changesNarrative := delta.Narrate(changes, time.Now()) + if changesNarrative != "" { + contextStr = changesNarrative + "\n\n" + contextStr + } cmds = append(cmds, watcherInvestigateCmd( m.toolRunnerFactory, m.toolRegistry, @@ -250,7 +274,8 @@ func (m *model) runDetectors() []tea.Cmd { obs.Summary, contextStr, ai.ResolvedModel(m.aiProvider), - nil, // collected by wrappedOnAsk inside watcherInvestigateCmd + nil, + obs.IncidentIDs, )) } else { summary := buildIncidentSummary(m.incidentList) @@ -273,6 +298,46 @@ func (m *model) runDetectors() []tea.Cmd { return cmds } +const maxRecentChanges = 200 + +func toSnapshots(incidents []pagerduty.Incident, cache map[string]*cachedIncidentData) []delta.Snapshot { + snaps := make([]delta.Snapshot, 0, len(incidents)) + for _, inc := range incidents { + var noteCount, alertCount *int + if c, ok := cache[inc.ID]; ok { + if c.notesLoaded { + n := len(c.notes) + noteCount = &n + } + if c.alertsLoaded { + a := len(c.alerts) + alertCount = &a + } + } + snaps = append(snaps, delta.SnapshotFromFields( + inc.ID, inc.Title, inc.Service.Summary, + inc.Status, inc.Urgency, + noteCount, alertCount, + )) + } + return snaps +} + +func (m *model) computeAndStoreDeltas() []delta.Change { + curr := toSnapshots(m.incidentList, m.incidentCache) + changes := delta.Diff(m.prevSnapshots, curr) + m.prevSnapshots = curr + + if len(changes) > 0 { + m.recentChanges = append(m.recentChanges, changes...) + if len(m.recentChanges) > maxRecentChanges { + m.recentChanges = m.recentChanges[len(m.recentChanges)-maxRecentChanges:] + } + } + + return changes +} + func buildIncidentSummary(incidents []pagerduty.Incident) string { var lines []string for _, inc := range incidents { @@ -290,16 +355,19 @@ func detectAll(incidents []pagerduty.Incident, clusterMap map[string][]string) [ } func detectServiceStorm(incidents []pagerduty.Incident) []watcherObservation { + serviceIncidents := make(map[string][]string) serviceCounts := make(map[string]int) for _, inc := range incidents { serviceCounts[inc.Service.Summary]++ + serviceIncidents[inc.Service.Summary] = append(serviceIncidents[inc.Service.Summary], inc.ID) } var observations []watcherObservation for svc, count := range serviceCounts { if count >= 3 { observations = append(observations, watcherObservation{ - Summary: fmt.Sprintf("Service storm: %d incidents on %s", count, svc), + Summary: fmt.Sprintf("Service storm: %d incidents on %s", count, svc), + IncidentIDs: serviceIncidents[svc], }) } } @@ -312,9 +380,11 @@ func detectClusterStorm(incidents []pagerduty.Incident, clusterMap map[string][] } clusterCounts := make(map[string]int) + clusterIncidents := make(map[string][]string) for _, inc := range incidents { for _, clusterID := range clusterMap[inc.ID] { clusterCounts[clusterID]++ + clusterIncidents[clusterID] = append(clusterIncidents[clusterID], inc.ID) } } @@ -322,7 +392,8 @@ func detectClusterStorm(incidents []pagerduty.Incident, clusterMap map[string][] for cluster, count := range clusterCounts { if count >= 2 { observations = append(observations, watcherObservation{ - Summary: fmt.Sprintf("Cluster storm: %d incidents on cluster %s", count, cluster), + Summary: fmt.Sprintf("Cluster storm: %d incidents on cluster %s", count, cluster), + IncidentIDs: clusterIncidents[cluster], }) } } @@ -331,15 +402,18 @@ func detectClusterStorm(incidents []pagerduty.Incident, clusterMap map[string][] func detectUrgencyShift(incidents []pagerduty.Incident) []watcherObservation { highCount := 0 + var highIDs []string for _, inc := range incidents { if inc.Urgency == "high" { highCount++ + highIDs = append(highIDs, inc.ID) } } if highCount >= 3 { return []watcherObservation{{ - Summary: fmt.Sprintf("High urgency cluster: %d/%d incidents are high urgency", highCount, len(incidents)), + Summary: fmt.Sprintf("High urgency cluster: %d/%d incidents are high urgency", highCount, len(incidents)), + IncidentIDs: highIDs, }} } return nil @@ -358,6 +432,83 @@ func parseWatcherQuery(input string) string { return strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(input), ":watcher")) } +// buildObservationContext derives context from the observation's triggering +// incidents, never from m.selectedIncident. Implements design choice (c): +// triggering incidents are foregrounded with sibling alerts labelled as background. +func buildObservationContext(m *model, obs watcherObservation) string { + var parts []string + + for i, incID := range obs.IncidentIDs { + inc := findIncidentByID(m.incidentList, incID) + if inc == nil { + continue + } + + label := "Triggering incident" + if i > 0 { + label = "Related incident" + } + parts = append(parts, fmt.Sprintf("%s: %s (%s)", label, inc.Title, inc.ID)) + parts = append(parts, fmt.Sprintf("Service: %s", inc.Service.Summary)) + parts = append(parts, fmt.Sprintf("Status: %s, Urgency: %s", inc.Status, inc.Urgency)) + + var alerts []pagerduty.IncidentAlert + if cached, ok := m.incidentCache[inc.ID]; ok && cached.alertsLoaded { + alerts = cached.alerts + } + + for _, alert := range alerts { + if details, ok := alert.Body["details"].(map[string]interface{}); ok { + if name, ok := details["alert_name"].(string); ok { + parts = append(parts, fmt.Sprintf("Alert: %s", name)) + } + if sopURL, ok := details["firing"].(string); ok && sopURL != "" { + parts = append(parts, fmt.Sprintf("SOP: %s", sopURL)) + } + if cluster, ok := details["cluster_id"].(string); ok { + parts = append(parts, fmt.Sprintf("Cluster: %s", cluster)) + parts = append(parts, buildClusterContext(m, cluster)...) + } + } + } + + var notes []pagerduty.IncidentNote + if cached, ok := m.incidentCache[inc.ID]; ok && cached.notesLoaded { + notes = cached.notes + } + + if len(notes) > 0 { + parts = append(parts, fmt.Sprintf("Notes: %d", len(notes))) + for j, n := range notes { + if j >= 5 { + break + } + content := n.Content + if r := []rune(content); len(r) > 300 { + content = string(r[:300]) + "..." + } + parts = append(parts, fmt.Sprintf(" - %s", content)) + } + } + } + + if len(m.incidentList) > 0 { + parts = append(parts, fmt.Sprintf("\nFull incident queue (%d incidents):", len(m.incidentList))) + parts = append(parts, buildIncidentSummary(m.incidentList)) + } + + return strings.Join(parts, "\n") +} + +func findIncidentByID(incidents []pagerduty.Incident, id string) *pagerduty.Incident { + for i := range incidents { + if incidents[i].ID == id { + return &incidents[i] + } + } + return nil +} + func buildWatcherContext(m *model) string { var parts []string diff --git a/pkg/tui/watcher_integration_test.go b/pkg/tui/watcher_integration_test.go index 8e94d58d..5470a4e5 100644 --- a/pkg/tui/watcher_integration_test.go +++ b/pkg/tui/watcher_integration_test.go @@ -950,7 +950,11 @@ func TestRunDetectors_ModelPlumbing(t *testing.T) { {APIObject: pagerduty.APIObject{ID: "P003"}, Service: pagerduty.APIObject{Summary: "svc-a"}, Urgency: "low"}, } - cmds := m.runDetectors() + // Simulate first-sighting: no prior state → all incidents are new changes + changes := m.computeAndStoreDeltas() + require.NotEmpty(t, changes, "first poll must produce IncidentNew changes") + + cmds := m.runDetectors(changes) require.NotEmpty(t, cmds, "runDetectors must produce commands for a service-storm with a healthy Anthropic provider") // Execute the first command to trigger the investigation path. diff --git a/pkg/tui/watcher_test.go b/pkg/tui/watcher_test.go index 746cb653..3fbed34c 100644 --- a/pkg/tui/watcher_test.go +++ b/pkg/tui/watcher_test.go @@ -1,11 +1,16 @@ package tui import ( + "fmt" "testing" "time" "github.com/PagerDuty/go-pagerduty" + "github.com/clcollins/srepd/pkg/ai/tools" + "github.com/clcollins/srepd/pkg/delta" + "github.com/clcollins/srepd/pkg/pd" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestWatcherBuffer_Append(t *testing.T) { @@ -239,6 +244,15 @@ func TestWatcherDedup(t *testing.T) { d.IsNew("first thing") assert.True(t, d.IsNew("second thing")) }) + + t.Run("evicts expired entries when threshold exceeded", func(t *testing.T) { + d := newWatcherDedup(0) // zero cooldown → all entries are immediately expired + for i := 0; i < watcherDedupEvictThreshold+10; i++ { + d.IsNew(fmt.Sprintf("obs-%d", i)) + } + assert.LessOrEqual(t, len(d.seen), watcherDedupEvictThreshold, + "expired entries must be evicted when threshold is exceeded") + }) } func TestDetectAll(t *testing.T) { @@ -410,3 +424,232 @@ func TestBuildWatcherContext_WithIncident(t *testing.T) { assert.Contains(t, ctx, "triggered") assert.Contains(t, ctx, "high") } + +// D2 test: an observation for alert A, with a DIFFERENT incident selected, +// must produce context naming A — and would FAIL if it reverted to m.selectedIncident. +func TestBuildObservationContext_ScopedToTriggeringIncident(t *testing.T) { + m := createTestModel() + + incidentA := pagerduty.Incident{ + APIObject: pagerduty.APIObject{ID: "INC-A"}, + Title: "Alert on cluster-xyz", + Status: "triggered", + Urgency: "high", + Service: pagerduty.APIObject{Summary: "svc-alpha"}, + } + incidentB := pagerduty.Incident{ + APIObject: pagerduty.APIObject{ID: "INC-B"}, + Title: "Unrelated alert", + Status: "acknowledged", + Urgency: "low", + Service: pagerduty.APIObject{Summary: "svc-beta"}, + } + m.incidentList = []pagerduty.Incident{incidentA, incidentB} + + // User has selected incident B, but the observation is about A + m.selectedIncident = &incidentB + + obs := watcherObservation{ + Summary: "Service storm on svc-alpha", + IncidentIDs: []string{"INC-A"}, + } + + ctx := buildObservationContext(&m, obs) + + // Must contain triggering incident A's details + assert.Contains(t, ctx, "INC-A") + assert.Contains(t, ctx, "Alert on cluster-xyz") + assert.Contains(t, ctx, "svc-alpha") + assert.Contains(t, ctx, "triggered") + + // The triggering section must not label incident B as "Triggering" or "Related" + assert.NotContains(t, ctx, "Triggering incident: Unrelated alert") + assert.NotContains(t, ctx, "Related incident: Unrelated alert") + // Queue summary includes all incidents (expected — design choice c) + assert.Contains(t, ctx, "Full incident queue") +} + +func TestBuildObservationContext_MultipleTriggering(t *testing.T) { + m := createTestModel() + m.incidentList = []pagerduty.Incident{ + {APIObject: pagerduty.APIObject{ID: "P1"}, Title: "First", Service: pagerduty.APIObject{Summary: "svc-a"}, Status: "triggered", Urgency: "high"}, + {APIObject: pagerduty.APIObject{ID: "P2"}, Title: "Second", Service: pagerduty.APIObject{Summary: "svc-a"}, Status: "triggered", Urgency: "high"}, + } + + obs := watcherObservation{ + Summary: "Service storm", + IncidentIDs: []string{"P1", "P2"}, + } + + ctx := buildObservationContext(&m, obs) + assert.Contains(t, ctx, "P1") + assert.Contains(t, ctx, "P2") + assert.Contains(t, ctx, "Triggering incident") + assert.Contains(t, ctx, "Related incident") +} + +func TestBuildObservationContext_EmptyIncidentIDs(t *testing.T) { + m := createTestModel() + m.incidentList = []pagerduty.Incident{ + makeIncident("P1", "svc-a", "high"), + } + obs := watcherObservation{Summary: "Something happened"} + + ctx := buildObservationContext(&m, obs) + assert.Contains(t, ctx, "P1", "queue summary still included") +} + +// M1: Headline integration test exercising BOTH directions of the delta gate +// in runDetectors. FAILS if the suppression gate is stubbed with `if false &&`. +func TestRunDetectors_DeltaGateBothDirections(t *testing.T) { + m := createTestModel() + m.incidentCache = make(map[string]*cachedIncidentData) + m.incidentClusterMap = make(map[string][]string) + m.watcherDedup = newWatcherDedup(0) // disable cooldown to isolate delta gate + + // 3 incidents on the same service → triggers service storm detector. + // Use low urgency to avoid triggering urgency-shift detector. + m.incidentList = []pagerduty.Incident{ + makeIncident("P1", "svc-x", "low"), + makeIncident("P2", "svc-x", "low"), + makeIncident("P3", "svc-x", "low"), + } + + // --- Direction 1: changes present → detectors MUST fire --- + changes1 := m.computeAndStoreDeltas() + require.NotEmpty(t, changes1, "first poll must produce IncidentNew changes") + + beforeLen := m.watcherBuffer.Len() + cmds1 := m.runDetectors(changes1) + // With no AI provider, observations go to the buffer + assert.True(t, m.watcherBuffer.Len() > beforeLen || len(cmds1) > 0, + "non-empty changes must trigger detector observations") + firstPollBufLen := m.watcherBuffer.Len() + + // --- Direction 2: no changes → detectors MUST NOT fire --- + changes2 := m.computeAndStoreDeltas() + assert.Empty(t, changes2, "identical second poll must produce zero changes") + + cmds2 := m.runDetectors(changes2) + assert.Empty(t, cmds2, "runDetectors must return nil when changes are empty") + assert.Equal(t, firstPollBufLen, m.watcherBuffer.Len(), + "buffer must not grow when delta gate suppresses") + + // --- Changed data re-enables detectors --- + m.incidentList[0] = makeIncident("P1", "svc-x", "high") // urgency change + changes3 := m.computeAndStoreDeltas() + require.NotEmpty(t, changes3, "changed data must produce changes") + + cmds3 := m.runDetectors(changes3) + assert.True(t, m.watcherBuffer.Len() > firstPollBufLen || len(cmds3) > 0, + "changed data must re-enable detector observations") +} + +// M3: the incidentList < 2 guard must suppress detectors for a single incident. +// FAILS if the guard is changed to `< 0`. +func TestRunDetectors_SingleIncidentNoInvestigation(t *testing.T) { + m := createTestModel() + m.incidentCache = make(map[string]*cachedIncidentData) + m.incidentClusterMap = make(map[string][]string) + + m.incidentList = []pagerduty.Incident{ + makeIncident("P1", "svc-a", "high"), + } + + changes := m.computeAndStoreDeltas() + require.NotEmpty(t, changes, "first-sighting must produce changes") + + cmds := m.runDetectors(changes) + assert.Empty(t, cmds, "single incident must not trigger investigation") + assert.Equal(t, 0, m.watcherBuffer.Len(), + "single incident must not produce buffer entries") +} + +// M2: cache loading between polls must not produce false NoteAdded/AlertAdded. +// Poll 1 with unloaded cache → Poll 2 with loaded cache (same data) → zero +// note/alert changes. This test FAILS if toSnapshots treats "not loaded" as 0. +func TestToSnapshots_UnloadedCacheSuppressesFalseChanges(t *testing.T) { + incidents := []pagerduty.Incident{ + makeIncident("P1", "svc-a", "high"), + } + + // Poll 1: cache entry exists but notes/alerts not yet loaded + cache1 := map[string]*cachedIncidentData{ + "P1": {notesLoaded: false, alertsLoaded: false}, + } + snap1 := toSnapshots(incidents, cache1) + + // Between polls: lazy enrichment loads 5 notes and 3 alerts + cache2 := map[string]*cachedIncidentData{ + "P1": { + notesLoaded: true, + notes: make([]pagerduty.IncidentNote, 5), + alertsLoaded: true, + alerts: make([]pagerduty.IncidentAlert, 3), + }, + } + snap2 := toSnapshots(incidents, cache2) + + changes := delta.Diff(snap1, snap2) + for _, c := range changes { + assert.NotEqual(t, delta.NoteAdded, c.Kind, + "cache loading must not produce false NoteAdded") + assert.NotEqual(t, delta.AlertAdded, c.Kind, + "cache loading must not produce false AlertAdded") + } +} + +// M2 counterpart: a genuine note addition AFTER cache load must still be detected. +func TestToSnapshots_GenuineNoteAdditionAfterCacheLoad(t *testing.T) { + incidents := []pagerduty.Incident{ + makeIncident("P1", "svc-a", "high"), + } + + cache1 := map[string]*cachedIncidentData{ + "P1": {notesLoaded: true, notes: make([]pagerduty.IncidentNote, 2)}, + } + snap1 := toSnapshots(incidents, cache1) + + cache2 := map[string]*cachedIncidentData{ + "P1": {notesLoaded: true, notes: make([]pagerduty.IncidentNote, 3)}, + } + snap2 := toSnapshots(incidents, cache2) + + changes := delta.Diff(snap1, snap2) + found := false + for _, c := range changes { + if c.Kind == delta.NoteAdded { + found = true + } + } + assert.True(t, found, "genuine note addition must be detected") +} + +func TestBuildAskFromVerdict_UsesOriginatingIncident(t *testing.T) { + mock := &pd.MockPagerDutyClient{} + m := createTestModel() + m.config = &pd.Config{Client: mock} + + incidentA := pagerduty.Incident{ + APIObject: pagerduty.APIObject{ID: "INC-ORIGIN"}, + Title: "Originating Alert", + } + incidentB := pagerduty.Incident{ + APIObject: pagerduty.APIObject{ID: "INC-SELECTED"}, + Title: "UI Selected Alert", + } + m.incidentList = []pagerduty.Incident{incidentA, incidentB} + m.selectedIncident = &incidentB + + verdict := tools.Verdict{ + Tier: tools.TierActionable, + Summary: "Post note", + Action: "Investigation note content", + } + + ask := m.buildAskFromVerdict(verdict, []string{"INC-ORIGIN"}) + + assert.Equal(t, "INC-ORIGIN", ask.IncidentID, + "must use originating incident, not m.selectedIncident") + assert.Equal(t, "Originating Alert", ask.IncidentTitle) +}