SQL Server's Poison Wait alert measures accumulated starvation over a ten-minute window like its PostgreSQL twin, so one slow wait no longer pages and a THREADPOOL storm no longer sleeps (#3539 A4)

[erikdarlingdata]

What does this PR do?

SQL Server's Poison Wait alert now measures accumulated starvation over a rolling ten-minute window and grades it WARNING/CRITICAL — the shape its PostgreSQL twin has fired on since #2711 — so one slow wait no longer pages and a THREADPOOL storm no longer sleeps. Partial for #3539: checklist item A4 ("SQL Poison Wait shape blindness") and the parity-table gap it names (the undocumented always-CRITICAL-vs-graded severity divergence under one shared mute key). References #3539; does not close it.

Root cause

AlertEngine.CheckPoisonWaitsAsync asked the read adapter for the newest three wait_stats rows (with delta_waiting_tasks > 0) and judged each row's avg_ms_per_wait >= PoisonWaitThresholdMs (500), presence-flat CRITICAL through the AlertSeverity map. That is a statement about how long ONE wait lasted in ONE collector interval, with no volume floor. It has both failure modes at once:

• False page: a single 600 ms wait is 600 ms of accumulated wait — 0.001 tasks stuck on average — and it paged CRITICAL.
• Blind to the storm: 10,000 THREADPOOL waits of 50 ms is 500 seconds of worker starvation per minute, and it averages 50 ms. An average cannot see volume by construction.

Meanwhile "Poison Wait" is ONE alert name on both engines — one PoisonWaitEnabled switch, one mute key, one history filter — and the PostgreSQL side (#2711) already fired on TOTAL accumulated wait over a ten-minute window normalized to "average backends continuously stuck", graded Warning at 1.0 / Critical at 10.0, fleet-calibrated ~160× above the worst healthy baseline. A mute rule written against one meaning silently governed the other, and the divergence was documented nowhere.

What the fleet said before a line changed

One production store class, 43 SQL Server primaries, 4 days of wait_stats, 24,757 server-ten-minute buckets (sanitized figures; the measurement document lives with the lane):

• The worst ten-minute bucket anywhere accumulated 5,795 ms of THREADPOOL — 0.0097 average waiters. The PG WARNING bar (1.0) sits ~100× above it, the same order as the ~160× margin the Postgres fleet (pgmon) has no per-target operational alerting — only tool-health and 3 outage-predictor alerts fire #2711 research measured for the PostgreSQL events. RESOURCE_SEMAPHORE accrued zero wait in four days.
• The old avg >= 500 trigger fired on no row in four days — no observed false positive, and nothing poison happened either. But its near-misses show both wrong shapes: THREADPOOL rows at 247.5 ms avg over 2 tasks and 163 ms over 1 task (the one-slow-wait class — a 600 ms sibling would have paged), and the largest THREADPOOL row at 703 tasks × 8.2 ms (the storm shape, invisible to an average). One server's daily compile burst (8 tasks, ~3 s, 356–394 ms avg) sat just under the old bar.
• Rows exist with delta_wait_time_ms > 0 and delta_waiting_tasks = 0 (304, 151, 149, 146 ms in one interval) — a task still waiting across the interval boundary. The old read's task filter dropped them; accumulation counts them.

Nothing measured would fire the new shape, which is the point: these waits are near-zero when healthy, and a full task pinned for ten straight minutes is categorically not that.

What changed, and why this shape

One shared home for the bars. New PoisonWaitEvaluator (Alerting) owns WindowMinutes = 10, WarningAvgWaiters = 1.0, CriticalAvgWaiters = 10.0, the shared Grade, and the SQL Server evaluation. PostgresAlertEvaluator's three poison constants are now aliases of those (= PoisonWaitEvaluator.X) under their old names, so the PG read adapter, the PG host and the PG tests keep compiling untouched — and a test reads the source to pin that they are aliases, not equal literals (two equal literals are exactly the state that drifts). Both engines' margins are documented on the shared constants. No engine-specific reason to diverge was found; the rationale for sharing (D9-style: one name under one mute key means one thing) is in the class comment, with the instruction that a future SQL-specific bar gets its own documented constant beside these, not a silent local number.

The read is a dumb window sum. IAlertReadAdapter.GetPoisonWaitDeltasAsync(serverKey, thresholdMs) → GetPoisonWaitAccumulationAsync(serverKey, windowMinutes) returning PoisonWaitAccumulation(WaitType, AccumulatedWaitMs, AccumulatedWaits, ObservedIntervals, NewestCollectionTime) — the SQL Server twin of PostgresPoisonWaitAlertInfo. Both SKUs' SQL: SUM(delta_wait_time_ms), SUM(delta_waiting_tasks), COUNT(*), MAX(collection_time) per poison wait type over collection_time >= now − window, no delta_waiting_tasks > 0 filter (that wait is evidence), no LIMIT (a limit on a sum is an undercount), no threshold (the engine grades; and it needs the under-bar rows — see the clear arm). The window is passed by the engine rather than read from the constant in the adapter so the read's cutoff and the evaluator's denominator are one number by construction. (0, 0) rows — the delta calculator's "no delta is knowable" marker, indistinguishable in wait_stats from an idle interval — contribute nothing to the sum, and the denominator is the window's wall clock, not a row count, so a fabricated zero neither inflates nor deflates the figure and is never itself a claim of quiet. Deliberately no interval handling: the V128 keystone lane owns sample_interval_seconds and rebases onto this text.

Read cost under the alert pass's 10-second deadline. The alert pass on the largest store has to survive the hourly interval CAGG refresh (4–7 minutes, #3597): a read that is sub-second by shape survives, a 2-second one dies. This read is cheap by SHAPE, not by any index the planner may or may not choose — no claim is made about the (server_id, collection_time) composite, which the planner has refused on a sibling table with the same interleaved write pattern (#3573). The WHERE is predicate-identical to the retired read's (one server_id, the three wait_type literals, collection_time >= now − 10 min), so it touches exactly the rows the old read touched and aggregates them instead of ordering them for a LIMIT 3: at the collector's one-minute cadence, at most ~10 collections × 3 types ≈ 30 rows per server, all in the last ten minutes of the current one-day chunk (the uncompressed head; chunk exclusion keeps compressed history out of the plan). The PostgreSQL twin has run this exact aggregate shape over pg_wait_stats under the same deadline since #2711. Stated in the SQL's own doc comment.

The engine grades and fires ONE alert per server, as before. Per wait type, worst-first (severity, then accumulated ms); the alert's severity is the worst type's, carried on BOTH AlertOutcome.Severity and AlertContext.SeverityOverride (channel builders read the context — the #1136 low-disk path; Darling's deliverer folds the outcome's in only when the context has none — #2090; setting both makes both SKUs agree by construction). Mute keys on the worst wait type (Lite's documented limitation, unchanged). Detail items per firing type carry accumulated seconds, avg tasks stuck, waits completed, the tier and a per-type remedy (SqlServerRemedyFor — the sibling of PG's PoisonWaitRemedyFor, expanding the README's own one-line characterizations). Numerics are the worst type's accumulated ms against the breached bar in ms — the PG twin's exact numeric pair and the unit the history formatter already renders this metric in. Message shape mirrors PG clause-for-clause with the engine's noun (task(s) vs backend(s)): [THREADPOOL] 600s of wait accumulated in the last 10 minutes across 300,000 waits — on average 1.0 task(s) continuously stuck. Not one alert per wait type like the PG host: changing the per-server delivery contract (metric-level cooldown fallback, no incidents, mute-on-worst) is a separate decision from changing what the number means.

The clear arm requires an observation. A standing alert clears when the window holds at least one poison-type row AND no type is over the bar — which, because the sum is rolling, happens only once enough wait has aged out, up to a full window late (the PG twin accepts the same lateness). An empty read holds the flag where it is: no wait_stats rows for any poison type in ten minutes is the collector not delivering, not the server going quiet (THREADPOOL has lifetime wait on any server up long enough to fire this, so its row lands every cycle). The retired shape announced "Poison Waits Cleared" on that same silence. This is the #3282 rule for a CPU reading that stops arriving, applied here; it is NOT the AlertPersistenceGate — the rolling window is already this alert's persistence in both directions (600 s must accumulate to fire; the sum must fall back under the bar to clear), and A5 (gate rollout) is a separate #3539 item this PR does not touch.

The #2704 re-fire guard stays, keyed on the newest collection_time of the FIRING types (a quiet sibling's newer row is not a new observation of the type over the bar).

PoisonWaitThresholdMs retired honestly. It is persisted on both SKUs (Lite settings JSON; Darling's darling.json and the store control-plane row) and surfaced by both Settings windows and get_alert_settings. It stays on IAlertEngineSettings, stays readable/writable/reported, and is no longer consulted — the interface member's doc says why (no migration rung for a value nothing evaluates; a per-wait average has no honest meaning to repurpose it to). Pass-throughs on DarlingAlertSettings/AppAlertEngineSettings/DarlingConfig carry a one-line pointer. Both READMEs say so; the README's alert table (which claimed "100 ms avg" — a stale default on top of a stale shape) now states the windowed bars, the clear rule, and that SQL and PG share the shape.

AlertSeverity map arm for "Poison Wait" keeps CRITICAL with a new comment: every live fire now carries its grade on the context, so the arm styles only override-less renders — alert-history replays of rows written BEFORE this change, every one of which WAS a presence-flat CRITICAL fire. CRITICAL is therefore the faithful replay, not a default.

Alternatives rejected

• Keep the avg bar and add a volume floor. Still cannot see the storm (703 × 8 ms fails any per-wait floor), and would leave the two engines' "Poison Wait" meaning two things.
• Reference PG's constants from the SQL side (instead of a shared home). Leaves a SQL Server alert keyed off a class named PostgresAlertEvaluator; the shared class is the honest home, with aliases so nothing PG-side moves.
• AlertPersistenceGate for the clear. The window is the persistence; gating it would count breaches in collector cycles and clears in sweeps (the units mismatch the field comment already names) on top of a measure that already holds.
• Clear on an empty read, like the PG host does. On PG an empty read is the normal healthy state on non-Aurora targets (the source is Aurora-only) — and those never fire, so hold-on-empty there is inert; on SQL Server an empty read is collector silence. Flagged below as a residual for the PG host's own arm.

Blast radius

• IAlertReadAdapter member replaced (no default implementations by rule): compiler named every implementer and fake — DarlingAlertReadAdapter, LiteAlertReadAdapter (+ LocalDataService.WaitStats, where Lite's DuckDB text lives by the adapter's own "engine stays free of DuckDB" convention), AlertEngineTests (two fakes), DarlingSelfAlertTests, LiteAlertForwardingTests. All updated.
• PoisonWaitDelta and AlertContextBuilders.BuildPoisonWaitContext are kept — the deprecated Dashboard's own loop uses them; PoisonWaitDelta's comment now says the shared engine no longer does.
• AlertReadFailureSurfaceTests census: catch (Exception) count in AlertEngine.cs unchanged (20 total / 14 counted), every await in the poison try block still followed by readClock.Restart().
• Alert text changes for SQL Poison Wait (CurrentValue, ThresholdValue, ShortMessage, resolution message). History dedup/mute keys unchanged (metric name, server, wait type).
• Two gated live-PG E2Es re-planted: DarlingAlertReadAdapterTests now seeds the measured shapes (703 × 8 ms row, time-with-no-tasks row, (0, 0) row, storm row, out-of-window row, sub-bar sibling) and asserts the sums; DarlingAlertingTests plants 36,000 × 20 ms = 720 s (the old shape could not fire on a 20 ms average).

What it does NOT do

• Does not close Alert-condition semantics campaign: window-invariance, persistence gates, engine parity (8 findings + parity table) #3539 (A2/A3/A5/A8c/A8d and the other parity gaps remain).
• Does not add a migration rung, a knob, or touch CHANGELOG.md (entry handed to the coordinator).
• Does not change the PG host's evaluation, delivery or clear arm; only its constants became aliases.
• Does not gate any other condition (A5).
• Does not add sample_interval_seconds handling (V128 lane).
• Does not touch get_alert_settings.poison_wait.threshold_ms (still reports the unconsulted value), AlertMetricClassifier.IsCritical's by-name "Poison" emphasis in the history grids, the deprecated Dashboard's own avg-based loop, or the PG host's clear-on-empty arm — outside this lane's file boundary and listed for the coordinator. (The Settings-window labels and save-preview strings on Lite and the Viewer WERE fixed after review — a label this change falsified is part of this change; see the review disposition comment.)

Test plan

New / changed tests — all compile-verified here (Darling.Tests, Lite.Tests: 0 warnings) and first EXECUTED in CI (net10.0-windows). Before trusting them, the engine scenarios were run on this machine against the real AlertEngine with AlertEngineTests' own fakes in a throwaway console under a hostile (de-DE) culture: every assertion below passed; and both SQL texts were executed against a throwaway PG18 + TimescaleDB 2.28.1 container (desktop-linux, torn down) and an in-memory DuckDB 1.5.5 with the real DuckDBParameter binding — both returned THREADPOOL 600083 / 30403 / 4 and RESOURCE_SEMAPHORE 3154 / 8 / 1 on the seeded shapes, with the out-of-window and other-server rows excluded.

Darling.Tests/AlertEngineTests (poison-wait section, rewritten):

• PoisonWait_FiresOnAccumulatedStarvation_NotPerWaitAverage — 300,000 × 2 ms = 600 s fires Warning; exact CurrentValue/ThresholdValue/ShortMessage; numerics 600,000 / 600,000; severity on outcome AND context; one detail with remedy; no incidents; window asked = 10.
• PoisonWait_OneSlowWait_AndEveryMeasuredFleetBucket_StaySilent — 1 × 600 ms, 5,795, 5,779 (703 tasks), 3,154 (compile burst), 599,999: all silent.
• PoisonWait_GradesCritical_AtTenTasksContinuouslyStuck — 6,000,000 ms → Critical, threshold text names the 10-task bar.
• PoisonWait_JudgesEachWaitTypeIndependently_WorstFirst_MutesOnTheWorst — Critical RS leads Warning TP (5.9M), sub-bar third type absent from value and details; mute asked for the worst.
• PoisonWait_HoldsOnAnEmptyRead_AndClearsOnlyOnAnObservedQuietWindow — empty read: no fire, no clear; zero-sum observed rows: Cleared with the windowed message; Cleared is an edge.
• PoisonWait_ClearsOnASubBarWindow_RegardlessOfTheRetiredKnob — knob = 1 ms changes nothing.
• PoisonWait_DoesNotRefire_OnTheSameCollectionTime_EvenAfterCooldownElapses (Fix Poison Wait alert re-firing on an unrefreshed wait_stats row #2704, re-shaped) and PoisonWait_FreshnessFollowsTheFiringTypes_NotAQuietSibling.
• PoisonWait_Suppressed_TracksTheConditionWithoutDelivering.

Darling.Tests/PostgresAlertEvaluatorTests (new SQL-port section): constants are ONE definition (value + source-alias pin with positive control); both engines grade identically at 0 / 599,999 / 600,000 / 5,999,999 / 6,000,000 with the same numeric pair; storm fires / one slow wait does not; fleet-quiet pin; (0, 0) is no evidence; worst-first ordering; message shape mirrors PG; remedy per type.

Darling.Tests/DarlingAlertReadAdapterTests: PoisonWaitsSql_IsAWindowAccumulation_NotTheNewestDeltas (absences: task filter, LIMIT, threshold, sample_interval; presences: the four aggregates, GROUP BY; wait-type list equals the evaluator's); the live E2E re-planted as above.

Lite.Tests: PoisonWait_ToastBody_UsesTheWorstWait_GradedByAccumulation_NotTheRetiredKnob (knob = 999, which under the old shape silenced both rows); PoisonWaitAccumulationSql_IsTheDarlingTextInDuckDbDialect (clause-for-clause parity read from both sources via ParitySource, with the only permitted dialect differences named); reflection pins moved to the new member.

Builds (all zero warnings): PerformanceMonitor.Alerting, PerformanceMonitor.Darling.Service, Lite (-p:EnableWindowsTargeting=true), Darling.Tests, Lite.Tests.

Which component(s) does this affect?

• Lite
• Darling
• Lite Tests
• Darling Tests
• SQL collection scripts
• Documentation
• Full Dashboard (deprecated)
• CLI Installer (deprecated)

How was this tested?

See the test plan: compile-verified on macOS, evaluator/engine scenarios executed locally against the real engine, SQL executed against throwaway PG18+Timescale and DuckDB. No SQL Server was touched; the read runs against the store, not a target.

Checklist

• I have read the contributing guide
• My code builds with zero warnings (dotnet build -c Debug)
• I have tested my changes against at least one SQL Server version (store-side change; verified against the store engines it reads — PG18/Timescale and DuckDB)
• I have not introduced any hardcoded credentials or server names

[claude]

Nice, thorough port — the shared PoisonWaitEvaluator, the alias-not-just-equal constants (with the source-text pin so they can't drift apart), the read-side rationale for dropping delta_waiting_tasks > 0 and LIMIT, and the freshness/clear-arm edge cases are all well reasoned and well tested (PoisonWait_FreshnessFollowsTheFiringTypes_NotAQuietSibling, PoisonWait_HoldsOnAnEmptyRead_AndClearsOnlyOnAnObservedQuietWindow, etc.). The DuckDB/PostgreSQL SQL text is pinned identical via PoisonWaitAccumulationSql_IsTheDarlingTextInDuckDbDialect, so that parity is enforced, not just asserted in prose.

One gap: both live Settings UIs still describe the retired knob as if it's the live threshold, and nothing in this PR updates them:

• Lite/Windows/SettingsWindow.xaml:249 — checkbox label "Poison waits (THREADPOOL, etc.) avg wait ≥" next to an editable ms box
• Lite/Windows/SettingsWindow.xaml.cs:950 — alert preview line "poison waits >= {threshold}ms avg"
• Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml:357 — identical checkbox label
• Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml.cs:1106 — identical preview line

After this change, PoisonWaitThresholdMs is explicitly "read and reported but no longer consulted" (per the new IAlertEngineSettings.PoisonWaitThresholdMs doc and the README table this PR did update). But an operator opening either Settings window still sees a checkbox reading "avg wait ≥ [500] ms" and a preview line confirming "poison waits >= 500ms avg" — both now false statements about what triggers the alert. Someone who raises or lowers that number to tune sensitivity gets no effect at all, with no indication in the UI that the field is inert. Since the README explicitly documents the retirement, it'd be good to at least relabel the control (e.g. "no longer used — see Poison Wait accumulation bars") or grey it out with a tooltip, on both SKUs.

This isn't a Lite-vs-Darling drift (both apps have the identical gap), but it is a real user-facing correctness issue introduced by redefining what the setting does without updating where it's surfaced.

[claude]

Solid port of the accumulation-based Poison Wait shape to SQL Server — the shared evaluator, alias-not-duplicate constants (with a source-text pin against drift), and the SQL parity pin between Lite's DuckDB text and Darling's Postgres text are all well done, and the edge-case test coverage (freshness gating on firing types only, hold-vs-clear on empty vs. observed-quiet reads, worst-first mute keying) is thorough and correct as written.

1. User-facing correctness bug (both SKUs): Neither Lite's nor Darling Viewer's Settings window was updated for the semantic change. Both still show a checkbox labeled "Poison waits (THREADPOOL, etc.) avg wait ≥" next to an editable ms threshold, and both build an alert-preview line reading "poison waits >= {threshold}ms avg" (Lite/Windows/SettingsWindow.xaml:249 / SettingsWindow.xaml.cs:950; Darling/PerformanceMonitor.Darling.Service/../Viewer/SettingsWindow.xaml:357 / SettingsWindow.xaml.cs:1106). Since PoisonWaitThresholdMs is now explicitly unconsulted by the engine (per this PR's own IAlertEngineSettings doc and README update), these controls actively mislead an operator into believing they can tune poison-wait sensitivity via that box, with zero effect and no UI indication that it's dead. Not a Lite/Darling parity drift (both apps have the identical gap) — a correctness gap left by the semantic change itself.

Everything else checked out: SQL is parameterized (no injection risk), the shared constants/tests correctly enforce cross-engine grading parity, and the freshness/mute/clear state machine changes are consistent with the codebase's existing patterns and covered by new tests.

[erikdarlingdata]

Review finding 1 (Settings windows still offering the retired ms bar) — fixed in 67e97aa, both twins identically:

• Lite/Windows/SettingsWindow.xaml:249 and Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml:357: the checkbox now reads "Poison waits (THREADPOOL, etc.) — accumulated wait over a 10-minute window; legacy avg-ms bar (no longer used):"; the AlertPoisonWaitThresholdBox is IsEnabled="False" with a tooltip stating the live bars (600 s / 6,000 s in 10 min) and why the box survives (saved settings round-trip). Kept rather than removed so the settings JSON / control-plane row / get_alert_settings shape is unchanged — the honest state is "visible, disabled, explained", not "gone" with a stale value still persisted underneath.
• Lite/Windows/SettingsWindow.xaml.cs:950 and Viewer/SettingsWindow.xaml.cs:1106: the save-preview line now states the live bar from the shared constants (poison waits >= 600s accumulated in 10min) instead of the box's text.

I had listed these surfaces as outside the lane's declared file boundary in the PR body; the review is right that a label my change falsifies is part of my change, so the boundary extension is taken and the "does NOT do" paragraph is stale on that point (the residuals that remain out of lane: get_alert_settings.poison_wait.threshold_ms reporting an unconsulted value, AlertMetricClassifier.IsCritical's by-name "Poison" emphasis, the deprecated Dashboard's own avg loop, and the PG host's clear-on-empty arm — all handed to the coordinator).

Same push also fixes the one CI failure, mine: EndToEnd_AllSevenFeeds… compared a seeded DateTime (100 ns ticks) to the value PostgreSQL stored at microsecond precision (.9353066 vs .9353060). It now compares at the store's precision and separately asserts the in-window row's clock won over the −30 min row's. Lite + Viewer + Darling.Tests rebuilt: 0 warnings.

Finding 1 (Settings windows offering the retired ms bar) remediated in 67e97aa on both SKUs; see the PR comment for the disposition.

[claude]

Reviewed the full diff (both commits). This is a well-scoped, thoroughly-tested port of the PostgreSQL Poison Wait accumulation shape to SQL Server. Notes:

• Correctness: PoisonWaitEvaluator grade boundaries, worst-first ordering, the freshness guard restricted to firing types only (findings.Max(f => f.NewestCollectionTime)), and the "observed-and-quiet vs. not-observed" distinction (accumulated.Count > 0 gating the clear arm) all match the accompanying tests and the documented intent. The (0,0) calculator-marker row is handled correctly — it contributes nothing to the sum and produces no finding, while still counting as an observed interval.
• Lite/Darling parity: LocalDataService.PoisonWaitAccumulationSql (DuckDB) and DarlingAlertReadAdapter.PoisonWaitsSql (Postgres-backed) carry the same clauses (no LIMIT, no delta_waiting_tasks > 0 filter, same GROUP BY/ORDER BY), and this is pinned by a cross-SKU parity test (PoisonWaitAccumulationSql_IsTheDarlingTextInDuckDbDialect) rather than left to eyeballing. Both Settings windows (Lite + Darling Viewer) got the identical XAML/code-behind treatment for disabling the retired ms box, and the second commit already fixed a Lite/Darling drift a reviewer would otherwise have flagged (settings-window labels lagging the engine change).
• Shared constants: PostgresAlertEvaluator's three poison constants are aliases of PoisonWaitEvaluator's (not just equal literals), and that's pinned via a source-text assertion (PoisonWaitConstantsAreOneDefinition_SharedByBothEngines) specifically to prevent future drift — a good pattern given the shared mute-key/alert-name contract.
• Security: all reads remain parameterized ($1/$2, DuckDB DuckDBParameter); no string-built SQL introduced.
• No T-SQL collector files are touched by this PR (the "SQL Server" read here is against the Postgres-backed Darling store / DuckDB Lite store, not a live DMV query), so the T-SQL style conventions in CONTRIBUTING.md don't apply to this diff.

Nothing rises to a blocking finding.

[claude]

LGTM — reviewed the SQL Server Poison Wait accumulation-shape port (AlertEngine, PoisonWaitEvaluator, both read adapters, both Settings windows) for correctness, Lite/Darling parity, and SQL injection; no substantive issues found. Details in the review comment.

[claude]

This IsEnabled="False" is undone at runtime: UpdateAlertControlStates() in SettingsWindow.xaml.cs sets AlertPoisonWaitThresholdBox.IsEnabled = enabled (where enabled tracks the master AlertsEnabledCheckBox), and it's called both on window load and on every toggle of that checkbox. With alerts enabled — the default — the box re-enables itself as soon as the window opens, silently defeating the point of this change (letting an operator tune a setting the engine no longer reads). Same issue in the Darling Viewer's SettingsWindow.xaml.cs.

AlertPoisonWaitThresholdBox should be excluded from that loop (or forced disabled unconditionally there too).

[erikdarlingdata]

Fixed: UpdateAlertControlStates() now sets this box IsEnabled = false unconditionally (both SKUs), so the XAML disablement holds on load and on every master-switch toggle.

[claude]

Same issue as the Lite twin: IsEnabled="False" here is overridden at runtime by UpdateAlertControlStates() in the code-behind, which sets AlertPoisonWaitThresholdBox.IsEnabled = enabled on window load and on every AlertsEnabledCheckBox toggle. With alerts enabled (the default), the box re-enables as soon as the window opens, so the "retired, don't edit" disablement doesn't actually hold in the common case.

[erikdarlingdata]

Fixed: UpdateAlertControlStates() now sets this box IsEnabled = false unconditionally (both SKUs), so the XAML disablement holds on load and on every master-switch toggle.

[claude]

Reviewed the SQL Poison Wait accumulation-shape port. The evaluation logic (PoisonWaitEvaluator, the AlertEngine fire/clear/re-fire arms, the shared-constant aliasing with PostgresAlertEvaluator, and the DuckDB/Postgres SQL parity) all check out against the stated design — boundary grading, worst-first ordering, the observed-vs-empty clear rule, and the Lite/Darling SQL text parity all match their tests and each other.

1. UI regression: the "retired" poison-wait ms box doesn't actually stay disabled. Both Settings windows (Lite/Windows/SettingsWindow.xaml and Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml) set AlertPoisonWaitThresholdBox to IsEnabled="False" specifically so operators can't tune a value the engine no longer consults. But in both SettingsWindow.xaml.cs files, UpdateAlertControlStates() unconditionally does AlertPoisonWaitThresholdBox.IsEnabled = enabled (where enabled mirrors the master "Alerts Enabled" checkbox), and that method runs both on window load and on every toggle of that checkbox. With alerts enabled — the default — the box re-enables itself as soon as the Settings window opens, silently undoing the disablement this PR added and defeating its own stated intent ("the box is disabled ... so nobody tunes a number nothing reads"). Present identically in both SKUs, so it's a shared bug rather than a parity drift, but it does need a fix (exclude that control from the loop, or force it disabled unconditionally after).

Everything else — the evaluator math, the read-adapter SQL, the clear/re-fire state machine, and the extensive test coverage — looks correct and consistent between Lite and Darling.

[erikdarlingdata]

Round-3 finding (the retired ms box re-enabled by UpdateAlertControlStates()) — fixed in the commit just pushed, both twins identically: Lite/Windows/SettingsWindow.xaml.cs:988 and Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml.cs:1150 now set AlertPoisonWaitThresholdBox.IsEnabled = false unconditionally inside the master-switch loop, with a comment saying why the box is the one control that ignores the switch. The save path still parses the box's persisted text (always valid, never user-edited now), so settings round-trip unchanged. Lite + Viewer rebuilt, 0 warnings. Good catch — the XAML attribute alone was a disablement that lasted until the window's Loaded handler.

Round-3 finding (master-switch loop re-enabling the retired ms box) remediated on both SKUs; see the PR comment and inline replies.

[claude]

Non-blocking, since the PR body already discloses this under "What it does NOT do" and hands it to the coordinator — flagging for visibility since it's a real (if pre-existing-shaped) behavior change.

AlertMetricClassifier.IsCritical (a few lines up in this file) classifies "Poison Wait" by name, unconditionally, for the Lite and Darling Viewer Alert History grids. Before this PR that was accurate, because the SQL Server engine only ever fired Poison Wait presence-flat CRITICAL. After this PR, SQL Server (and PostgreSQL, unchanged) can genuinely fire Poison Wait at Warning severity (1.0–9.99 avg waiters, a real fraction of the fired range, not an edge case). The email/webhook paths correctly consult Context.SeverityOverride via AlertSeverity.ForMetric, but the History grids' IsCritical/IsWarning have no override mechanism — so a Warning-graded poison-wait row will still render red/CRITICAL in both apps' history grids, silently overstating its severity relative to what the toast/email/webhook just said for the same event.

[erikdarlingdata]

Agreed, and deliberately out of this lane: IsCritical/IsWarning are by-name for every metric and have no override channel, so the honest fix is a grid-side severity column (the history row already serializes Context.SeverityOverride in its context JSON on Darling; Lite's row does not carry it) — a surface change across both apps' Alert History grids, not a one-arm edit here. It has been a real mismatch for PG Warning poison fires since #2711; this PR widens the population it affects. Handed to the coordinator as a named out-of-lane finding with the mechanism above.

[claude]

Reviewed the full diff (production code, test changes, and both Settings-window/README parity edits).

Correctness. The new accumulation shape checks out: the shared PoisonWaitEvaluator bars (Grade, boundary-inclusive at the Warning/Critical bars), the worst-first sort (severity then accumulated ms, consistent with PostgresAlertEvaluator.EvaluatePoisonWaits's sort), the (0,0)-row handling, the "observed vs. not-observed" clear-arm distinction, and the #2704 freshness guard (now scoped to firing types only) all read correctly against the tests exercising them. Ran through the boundary arithmetic (599,999 / 600,000 / 5,999,999 / 6,000,000 ms) by hand — no floating-point surprises at these magnitudes since the divisions are exact.

Lite/Darling parity. IAlertReadAdapter.GetPoisonWaitAccumulationAsync is implemented identically in shape on both SKUs (DuckDB vs. Npgsql), the SQL text is asserted clause-for-clause equal via ParitySource/RepoFile.ReadRepoFile pins, and the Settings-window edits (label, disabled+tooltip box, preview line, UpdateAlertControlStates keeping the box disabled through the master switch) are identical on Lite and the Darling Viewer. All IAlertReadAdapter implementers and fakes (Darling service, Lite, and every test double) were updated — no leftover GetPoisonWaitDeltasAsync/AvgMsPerWait callers in the alert path.

One residual worth a second look (left as an inline comment on AlertMetricClassifier.cs:165, non-blocking since it's explicitly disclosed in the PR body's "What it does NOT do"): AlertMetricClassifier.IsCritical still classifies "Poison Wait" as CRITICAL by name for the Lite/Darling Viewer Alert History grids, with no path to the Warning severity this PR now makes real and common. Email/webhook rendering is fine (AlertSeverity.ForMetric consults Context.SeverityOverride), so this is scoped to the history-grid row color only — but since Warning-graded poison waits are now a normal outcome rather than an edge case, it may be worth more than a coordinator note.

Security/T-SQL style. No SQL Server T-SQL touched by this PR (Darling's PostgreSQL adapter text and Lite's DuckDB text are the only SQL surfaces changed); both remain parameterized ($1/$2, DuckDBParameter), no string concatenation into SQL. No missing-index recommendations made, per repo convention.

[claude]

LGTM — reviewed the accumulation-shape port for correctness (grading boundaries, worst-first sort, clear-arm observed/quiet distinction, #2704 freshness guard), Lite/Darling parity across the read adapters and both Settings windows, and SQL parameterization. One residual noted inline (AlertMetricClassifier.IsCritical's by-name Poison Wait styling in the history grids no longer matches the now-real Warning tier) but it's explicitly disclosed in the PR body as deferred, so non-blocking.