Render only the incidents outside their own #1154 cooldown window

[erikdarlingdata]

Closes #3313.

IncidentCooldown answers whether an alert posts, and #1154 makes that answer "yes if any of its fingerprints is outside its own window" so a genuinely distinct incident is never throttled by an unrelated prior one. Nothing answered the second question — which incidents the resulting card contains — so the channel builders rendered every entry in AlertContext.Incidents, including ones still inside their own window that had been delivered minutes earlier. One fresh fingerprint dragged every co-resident stale one back into the channel with it, which is why raising the cooldown cannot fix it: the repeat is a passenger on another fingerprint's card, not a second firing of its own.

IncidentCooldown.Decision now reports the fingerprints that were outside their own window — a member of the same value as ShouldSend, because both come from one pass over one key set and a render deciding freshness for itself would not be answering the question the send decision answered. IncidentDeliveryFilter.ForDelivery narrows the rendered context to them: each suppressed incident's own detail item is dropped, every other item is kept, and a one-line Other Incidents / Still Open: N other incident(s) still open (already reported) footer states what was held back.

Applied once per send path, which is one place per channel family rather than four: the webhook fan-out filters once for Teams, Slack, PagerDuty and the generic channel (alongside triageUrl, and for the same reason — four channels describing one firing must not disagree about what it covers), and EmailSendCore's email branch filters from its own decision. Deliberately not shared between the two: email and webhook hold separate cooldown key spaces, so an email that failed to send last cycle is still owed a delivery where the webhook's is not. Lite runs the same PerformanceMonitor.Notifications send core, so this is one fix for both SKUs.

Delivery filters; persistence does not

ForDelivery returns a copy and never mutates its input, so the config_alert_log row the caller writes after the send still carries every incident in both detail_text and the context JSON — what the MCP reader, the triage page, the Viewer's detail pane and AlertMuteContext.PopulateFromDetailText read is unchanged. Same split #3317 shipped for analysis prose.

The #2109 remediation prose is structurally out of reach

AgAlertContexts.ForDatabase builds one plain detail item and never touches AlertIncidentRenderer, so an AG alert carries no fingerprint, is evaluated on the metric-level fallback key, and reports a null deliverable set — "nothing to filter", which returns the input instance unchanged. The filter cannot reach that prose even in principle. The existing pins on both SKUs stay green, and the one shape no pin covered — prose and fingerprinted incidents on the same alert — is now pinned on the wire.

That last shape is also why the prose is resolved against the unfiltered context. Every engine alert's detailText is AlertDetailText.Flatten of its own context, which is how ProseForDelivery recognises it as a restatement and suppresses it; comparing against the filtered copy would find them unequal and print the whole flattened alert as prose, resurrecting exactly the incidents just removed from the structure. Five channel builders re-run ProseForDelivery on whatever they are handed, so rather than leave that composition argued, the one shape that would break it takes the unfiltered path.

Per-event mode does not have this defect, and is not changed

PerEventNotification.Split already gives each message a single-incident context and each goes through its own send, so an in-window fingerprint fails its own ShouldSend and never posts at all. Its trailing +N more overflow message is the one per-event payload carrying several fingerprints, and it is filtered by the same call because that call sits at the delivery boundary every message crosses. Established from the code and pinned: PerEventMode_PostsTheFreshFingerprintOnly_AndRecordsBoth stays green with the filter disabled, and it fails if the filter is ever moved ahead of the split (which would cost the suppressed incident its history row).

Two consequences worth naming

PagerDuty's dedup_key now anchors on the first deliverable incident, not the first one the alert happens to carry, because DerivePagerDutyDedupKey reads the rendered context (as does the triage link, so link, token and PD all still correlate). That is the behaviour #1154 was after: a batch of {in-window, fresh} previously triggered under the in-window incident's key and folded into its existing PD alert, so the new incident never surfaced there at all.

The forensic attachment is carried over unchanged and is not per-incident. BuildDeadlockContext sets one AttachmentXml from the first deadlock in the window, with no mapping from graph to fingerprint, so the filter has nothing to select on. Carrying it is the right direction — dropping it would lose the graph entirely — but the card's attachment may belong to an incident the card no longer lists. Pre-existing and unchanged by this PR; noted rather than left to be discovered.

Verification

Darling.Tests / Lite.Tests target net10.0-windows and cannot run on macOS, so the actual test .cs files were compiled into a net10.0 console harness with an xunit shim and run: 37 checks, including the 13 pre-existing AlertIncidentRenderTests / IncidentCooldownTests pins that share this seam. Four mutations, each reverted afterwards:

mutation	result
filter disabled (return the input unchanged)	5 red, the all-fresh control green
ShouldSend requires every fingerprint fresh (#1154 crippled)	4 different red, incl. both SummaryWithMixedFreshness and the deliverable-key pin
ShouldSend fold replaced by a plain assignment (last-key-wins)	green — the fixtures were order-biased. Both fixtures now evaluate both orders, and it reddens
filter keeps by position instead of set membership	red, on the flipped-order half

The third row is the reason for the second commit. Both members are reductions over an ordered key list and both fixtures put the fresh fingerprint last, so a last-one-wins reduction was right on the one order they exercised.

CI is the arbiter for the suites themselves.

[claude]

Nit: two consecutive <summary> blocks are attached to CapturingWebhookEndpoint (lines 22-28 and 29-34). Only one <summary> per doc comment is valid — most XML-doc tooling (IntelliSense, DocFX, etc.) only surfaces the first one, so the actual per-class summary ("A loopback endpoint that records the bodies posted to it...") would be silently dropped by anything that renders docs, and the file-level rationale ("Shared rather than nested per test class...") ends up mis-attached to this one class instead of the file/namespace.

Consider making the first block a plain // file-banner comment (or moving it above a namespace-level doc target) so it isn't parsed as this class's XML doc.

[claude]

Reviewed the delivery-filter change (IncidentDeliveryFilter, the IncidentCooldown.Decision.DeliverableDedupKeys addition, and the Email/Webhook integration).

Correctness: traced the core logic carefully — the null-vs-empty distinction for DeliverableDedupKeys (no fingerprints vs. every fingerprint suppressed), the "kept.Count == 0 → render unfiltered" fallback for a mismatched decision/context pairing, blank-DedupKey incidents never being filterable, the persistence-vs-delivery split (ForDelivery never mutates its input), and the deliberate choice to resolve prose against the unfiltered context so ProseForDelivery's redundancy check doesn't resurrect suppressed incidents in text form. All of it checks out, and the accompanying tests (IncidentDeliveryFilterTests, the IncidentCooldownTests additions, the cross-vehicle Lite render pin) already cover the edge cases I went looking for — including the two-order fixture that catches an ordered-reduction mutation, and the explicit pin that per-event mode is untouched.

Lite/Darling parity: the fix lives entirely in the shared PerformanceMonitor.Notifications project (EmailSendCore, WebhookAlertService), which both SKUs consume unchanged — confirmed Lite/Services/EmailAlertService.cs delegates straight to EmailSendCore.TrySendAsync with no separate incident-rendering logic to drift. Both Darling.Tests and Lite.Tests gained coverage, so this looks correctly parity-safe.

Security/perf: no new I/O, secrets, or injection surface; the new loopback TCP test doubles are appropriately test-only. No performance concerns — this is a small filtering pass over an already-small incident list on the alert-send path, not a collector query.

One minor doc nit left as an inline comment on CapturingAlertEndpoints.cs (a duplicated <summary> block). Otherwise this looks solid — nice work threading the persistence/delivery split and the prose-redundancy interaction.

[claude]

Reviewed the diff (all four commits). Summary: no correctness, Lite/Darling parity, or security issues found.

Scope: all production changes live in the shared PerformanceMonitor.Notifications project (IncidentCooldown.cs, AlertIncidentRenderer.cs, EmailSendCore.cs, WebhookAlertService.cs, new IncidentDeliveryFilter.cs). No T-SQL touched, so the style-guide section doesn't apply here. Since Lite and Darling both consume this library directly, there's no drift risk by construction — and the PR still mirrors test coverage in both Lite.Tests and Darling.Tests, consistent with the CONTRIBUTING.md two-store parity guidance.

Things I specifically checked and found correct:

• IncidentCooldown.BuildKeys never mixes fingerprinted and fallback keys, so the fingerprinted flag in EvaluateAsync is never ambiguous, and Decision.DeliverableDedupKeys correctly distinguishes null ("nothing to filter" — metric-fallback/AG alerts) from empty ("everything suppressed").
• IncidentDeliveryFilter.ForDelivery never mutates the input AlertContext (verified AlertContext has exactly 4 members, all copied into rendered), so config_alert_log's detail_text/context JSON still carry every incident — confirmed at the call site in DarlingAlertDeliverer.SendAndRecordAsync, which records from the original unfiltered context/detailText, not the filtered render.
• Blank-DedupKey incidents (unresolved objects) are correctly never filtered out, since IncidentCooldown.BuildKeys excludes them from cooldown keys entirely and the filter special-cases IsNullOrEmpty(DedupKey) to always keep.
• The prose-resolved-against-unfiltered-context reasoning checks out against AlertDetailText.Flatten/ProseForDelivery — resolving against the filtered copy would indeed make five downstream channel builders' own ProseForDelivery re-run see prose == flatten(filtered context) and wrongly suppress genuine remediation prose (the [QUESTION] Since upgrading to 3.7.0, I get an email every hour #3296 regression this is careful to avoid).
• AgAlertContexts.ForDatabase confirmed to never populate Incidents, so the filter's null-fast-path correctly leaves it untouched.
• PerEventNotification.Split confirmed to build single-incident contexts per message, so per-event sends correctly bypass the filter's footer path (an in-window fingerprint just fails its own ShouldSend and never posts).
• The test-file split (CapturingAlertEndpoints.cs carved out of AlertDeliveryChannelTests.cs) is a byte-for-byte move (private → internal for cross-class use), and the 4th commit's fix for the stacked-<summary> doc-hygiene violation is applied correctly in the final state.

No findings to post inline.