feat: watcher deltas + Chat sessions (plan 418)

[clcollins]

Summary

• D1: Add pure Diff(prev, curr) function in pkg/delta/ that computes typed changes between PagerDuty polls. Wire into watcher as primary investigation gate — unchanged alerts are no longer re-investigated.
• D2: Scope investigations to the triggering alert/incident, not m.selectedIncident. Context now foregrounds triggering incidents with sibling alerts as background.
• D3: Feed delta narrative (delta.Narrate) into investigation seed context so the AI sees what changed.
• D4: Land Chat interface in pkg/ai following the optional-interface pattern (SupportsChat/AsChat).
• D5: Add get_recent_events tool as ClassRead — exposes recent incident-state changes to the AI agent.

Binding decision

In-memory only. No TurnStore, JSONL, or file persistence. Restarting srepd = fresh start.

Post-review fixes

Defect	Fix	Tests
M1: delta gate suppression completely untested	Added TestRunDetectors_DeltaGateBothDirections exercising both directions of the gate through runDetectors	Revert: if false && len(changes) == 0 → test FAILS
M2: lazy cache fires false-change burst on startup	Changed NoteCount/AlertCount to *int; nil = unknown/unloaded, Diff skips comparison when prev is nil	TestToSnapshots_UnloadedCacheSuppressesFalseChanges, 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 on list response has no escalation_level field	Removed TestDiff_Escalated, TestDiff_EscalationDecrease_NoChange
N2: ClusterID set but never compared	Removed from Snapshot (never populated by toSnapshots)
N3: watcherDedup.seen unbounded	Added eviction of expired entries when map exceeds 100	TestWatcherDedup/evicts_expired_entries_when_threshold_exceeded
N4: Title/Service never compared	Added comparison + new IncidentUpdated change 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 — delta catches "nothing changed at all", dedup catches "same observation text despite unrelated state change".

Traceability

Deliverable	Commit	Key Tests
D1a: Pure Diff	df22984	pkg/delta — TestDiff_{NoChange,NewIncident,IncidentResolved,StatusChange,UrgencyChange,NoteAdded,AlertAdded,MultipleChanges,FirstSighting,ReorderingOnly,EmptyBoth,NilNoteCountSkipsComparison,NilAlertCountSkipsComparison,ZeroToNonZeroNoteCountDetected,TitleChanged,ServiceChanged}
D1b: Wire + gate	5883b99	TestRunDetectors_DeltaGateBothDirections
D2: Scoped investigations	e0c41c2	TestBuildObservationContext_ScopedToTriggeringIncident, TestBuildAskFromVerdict_UsesOriginatingIncident
D3: Delta narrative	5883b99	Integrated into watcher context
D4: Chat interface	4c56dc0	TestSupportsChat, TestAsChat
D5: get_recent_events	47046e9	TestGetRecentEvents_{HappyPath,EmptyChanges,WithLimit,InvalidInput,IsClassRead}
M1: Delta gate test	0bd17ed	TestRunDetectors_DeltaGateBothDirections
M2: False-change fix	3041078→70e746f	TestToSnapshots_UnloadedCacheSuppressesFalseChanges, TestToSnapshots_GenuineNoteAdditionAfterCacheLoad
M3: Single-incident test	0bd17ed	TestRunDetectors_SingleIncidentNoInvestigation
N1: Remove Escalated	38d28e0	Removed tests for non-existent change kind
N2+N4: ClusterID/Title/Service	bc01128	TestDiff_TitleChanged, TestDiff_ServiceChanged
N3: Bound dedup	208ba1e	TestWatcherDedup/evicts_expired_entries_when_threshold_exceeded

Revert checks

M1 — if false && len(changes) == 0

=== RUN   TestRunDetectors_DeltaGateBothDirections
    watcher_test.go:535:
            Error Trace:    /workspace/.../pkg/tui/watcher_test.go:535
            Error:          Not equal:
                            expected: 1
                            actual  : 2
            Messages:       buffer must not grow when delta gate suppresses
--- FAIL: TestRunDetectors_DeltaGateBothDirections (0.00s)
FAIL

M2 — revert toSnapshots to always use &0 for unloaded cache

=== RUN   TestToSnapshots_UnloadedCacheSuppressesFalseChanges
    watcher_test.go:595:
            Error:          Should not be: 4
            Messages:       cache loading must not produce false NoteAdded
    watcher_test.go:597:
            Error:          Should not be: 5
            Messages:       cache loading must not produce false AlertAdded
--- FAIL: TestToSnapshots_UnloadedCacheSuppressesFalseChanges (0.00s)
FAIL

After restore — all pass

ok      github.com/clcollins/srepd/pkg/tui      0.036s
ok      github.com/clcollins/srepd/pkg/delta     0.005s

Test plan

• gofmt -s -l cmd pkg — no output (clean)
• go vet ./... — passes
• golangci-lint run — 0 issues
• go test ./pkg/... -count=1 — all pass
• go test -race ./pkg/... -count=1 — all pass
• deadcode ./... — no new entries (only pre-existing mock/config/test helpers)
• make quickstart-verify — docs/quickstart.md is up to date
• make plan-check — plan doc found
• Pre-existing: cmd/ tests fail on missing config keys (same on clean main)

Testing this live

1. make build
2. ./dist/srepd_linux_amd64_v1/srepd --dev
3. Wait for incidents to load (first poll)
4. EXPECT: watcher fires investigation on first sighting (service storm / urgency cluster)
5. Wait 15s for second poll (same data)
6. EXPECT: watcher does NOT re-fire investigation (delta gate suppresses)
7. Press w to toggle watcher pane visibility

Visual validation

tui-mcp is unavailable in the container environment (no Node.js/npx). Golden snapshots and view_render integration tests are the only visual evidence. The orchestrator should validate on the host.

🤖 Generated with Claude Code

[clcollins]

PR #427 Adversarial Review — Watcher Deltas + Chat

MUST-FIX

M1: Headline test does not test the gate — passes when the gate is removed

File: pkg/tui/watcher_test.go:493 (TestDeltaGate_IdenticalRefreshesProduceOneInvestigation)
File: pkg/tui/watcher.go:226

Mutation executed:

-  if len(changes) == 0 {
+  if false && len(changes) == 0 {

Result: TestDeltaGate_IdenticalRefreshesProduceOneInvestigation PASSES. The test only exercises computeAndStoreDeltas() and asserts its return values. It never calls runDetectors() with empty changes and asserts that no commands are produced. The comment on line 517 says the gate "would block" — but that is a reading claim, not a test assertion.

The reverse mutation (if true || len(changes) == 0) IS caught — by TestRunDetectors_ModelPlumbing, not by the headline test. So one direction of the gate is tested, but the critical suppression direction (the entire point of this PR) is not.

Failure scenario: Someone removes the len(changes) == 0 guard. Every poll re-investigates every incident, exactly the problem this PR exists to fix. No test breaks.

Fix: Add a test that calls m.runDetectors([]delta.Change{}) on a model with 3+ incidents on the same service (so detectors would fire) and asserts the return is empty/nil.

M2: Spurious NoteAdded/AlertAdded changes on cache population

File: pkg/tui/watcher.go:290-309 (toSnapshots)

Confirmed by reading (not executed as mutation — requires mocking the lazy enrichment pipeline).

toSnapshots reads noteCount and alertCount from m.incidentCache. On the first poll after startup, the cache is empty, so counts are 0. Between polls 1 and 2, lazy enrichment populates the cache. On poll 2, counts jump from 0 to actual values, producing spurious NoteAdded and AlertAdded changes.

Failure scenario: Incident P123 has 5 notes and 3 alerts. Poll 1: snapshot has noteCount=0, alertCount=0. Lazy enrichment loads cache. Poll 2: snapshot has noteCount=5, alertCount=3. Diff produces NoteAdded(5) + AlertAdded(3) even though nothing changed — the data was just fetched for the first time. This fires the delta gate and triggers an investigation on unchanged data, partially defeating the purpose of the gate.

This will happen for EVERY incident on startup as their caches populate, producing a burst of false-positive changes. The watcherDedup secondary gate may catch some of these (if the observation text matches), but the delta gate — the one this PR adds — is bypassed.

Fix: Either initialize note/alert counts from the PagerDuty list response (which includes NumberOfAlerts in the Incident struct) so the first snapshot already has correct counts, or only emit NoteAdded/AlertAdded when a previous non-zero count changes (i.e., skip if p.NoteCount == 0).

M3: incidentList < 2 guard in runDetectors is untested

File: pkg/tui/watcher.go:219

Mutation executed:

-  if len(m.incidentList) < 2 {
+  if len(m.incidentList) < 0 {

Result: All tests pass. No test exercises the single-incident case to prove the guard blocks.

Failure scenario: With a single incident, removing this guard would let detectors fire on every poll since detectServiceStorm and detectClusterStorm have their own thresholds. The practical risk is low (detectors already require 2-3 incidents), but this is the same "untested guard" pattern that bit #418 and #419. Since TestRunDetectors_ModelPlumbing uses 3 incidents, it incidentally avoids this code path rather than testing it.

NICE-TO-HAVE

N1: EscalationLevel is hardcoded to 0 in toSnapshots

File: pkg/tui/watcher.go:306

toSnapshots always passes 0 for escalationLevel. The Escalated change kind has full unit-test coverage in pkg/delta/delta_test.go, but can never fire in production because the wiring always supplies 0. The pagerduty.Incident struct does not directly expose escalation level (it's on the escalation policy response), so this may be intentionally deferred. But it means escalation events — something SREs very much care about — silently produce no diff.

Severity: Low. A real change an SRE cares about (escalation) produces no diff. But the escalation data isn't in the incident list response, so fixing it requires an additional API call.

N2: Snapshot.ClusterID is dead code

File: pkg/delta/delta.go:56

The field is declared in the Snapshot struct but never set (not in SnapshotFromFields, not anywhere else) and never compared in Diff. Pure dead weight.

N3: watcherDedup.seen grows unboundedly

File: pkg/tui/watcher.go:198-215

The seen map stores SHA-256 hashes with timestamps. Entries are added but never evicted, even after their cooldown expires. Over a long-running session (srepd runs all day for SREs), this map grows without bound. The growth rate is slow (~88 bytes per unique observation), so this won't cause problems in practice for hours of use, but a 24-hour session with many detector fires could accumulate thousands of entries.

The PR was "supposed to supersede" watcherDedup with the delta layer, but both mechanisms remain active — the dedup is a secondary rate-limiter on top of the delta gate.

N4: Title and Service changes produce no diff

File: pkg/delta/delta.go:107-143

Diff compares Status, Urgency, EscalationLevel (increase only), NoteCount (increase only), and AlertCount (increase only). It does NOT compare Title or Service, even though these are stored in Snapshot. If PagerDuty updates an incident's title (e.g., an operator renames it to reflect new findings), no diff is generated.

This is a design choice rather than a bug — titles rarely change and comparing them would fire on cosmetic edits. But it means a meaningful title update (e.g., "SilenceExpired" → "ClusterDown") is invisible to the delta layer.

CLEAN

• Plan doc numbering: 418-watcher-deltas-chat.md — no collision with anything on main. Confirmed.
• All cited test names exist. All 10 test names in the PR body match real func Test... declarations. No fabrications.
• Diff is pure. Takes values, returns values, no I/O, no globals, no time.Now(). Narrate takes time.Time as a parameter. Clean.
• D2 scoping is correct. buildObservationContext does NOT access m.selectedIncident. It iterates only over obs.IncidentIDs and calls findIncidentByID. buildWatcherContext (user-directed path) correctly reads m.selectedIncident. The two paths are cleanly separated.
• get_recent_events tool: ClassRead (tested), goes through the policy gate via GatedBetaTools, output bounded at 200 events and truncated at 8KB via Truncate(). Input validated (invalid JSON returns error). Clean.
• No new Go modules. go.mod/go.sum unchanged from main. No dependency additions to review.
• Race detection: Clean (no DATA RACE) with -race -count=2 on both pkg/delta and pkg/tui.
• go vet ./...: Clean.
• go test ./pkg/... -count=1: All pass.
• Deadcode: Only SupportsChat/AsChat from this PR appear in deadcode output — intentionally staged per plan (landed-but-not-yet-wired).
• No persistence added: Confirmed in-memory only. No file I/O, JSONL, or schema. prevSnapshots replaced each poll (bounded by incident list size). recentChanges capped at 200.
• First sighting: Diff(nil, curr) correctly produces IncidentNew for every incident. Tested in TestDiff_FirstSighting. In computeAndStoreDeltas, m.prevSnapshots starts as nil, so the first poll produces first-sighting changes. TestRunDetectors_ModelPlumbing confirms this flows through to runDetectors. First-sighting is not silently skipped.

Mutations executed

Mutation	Compiles?	Test result	Conclusion
Delta gate: if false && (never suppresses)	Yes	All pass	Gate suppression is UNTESTED
Delta gate: if true || (always suppresses)	Yes	TestRunDetectors_ModelPlumbing fails	Gate pass-through IS tested
incidentList < 0 (disable minimum)	Yes	All pass	Minimum guard is UNTESTED