Feature/alert muting

[HannahVernon]

What does this PR do?

Adds configurable muting of Alerts. For full description, plan, and concise change-log see #493

Which component(s) does this affect?

• [✅] Full Dashboard
• [✅] Lite Dashboard
• Lite Tests
• SQL collection scripts
• CLI Installer
• GUI Installer
• [✅] Documentation

How was this tested?

Manually tested both Dashboard and Lite per screenshots shown in #493

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
• [✅] I have not introduced any hardcoded credentials or server names

[erikdarlingdata]

Code Review: Alert Muting

Overall this is well-structured with full parity across both apps. Nice work on the context-sensitive dialog, the auditability (muted alerts still logged), and the MCP integration. A few things to address:

---

Bugs

1. [BLOCKER] DuckDB migration NOT NULL on ALTER TABLE
Lite/Database/DuckDbInitializer.cs:563 — The v20 migration uses BOOLEAN NOT NULL DEFAULT false, but DuckDB doesn't support NOT NULL on ALTER TABLE ADD COLUMN. There's even a comment in the v9 migration (line 424) warning about this exact issue. The catch block swallows the error so the column never gets added, and subsequent INSERTs that reference muted will fail.

Fix: Change to BOOLEAN DEFAULT false (drop the NOT NULL). The NOT NULL in Schema.CreateAlertLogTable for fresh installs is fine — this only affects the ALTER TABLE migration path.

2. "Mute This Alert" dialog title says "Edit Mute Rule"
Both Dashboard/Controls/AlertsHistoryContent.xaml.cs and Lite/Controls/AlertsHistoryTab.xaml.cs pass a pre-filled MuteRule to the MuteRuleDialog(MuteRule existingRule) constructor. Since existingRule != null, the dialog sets its title to "Edit Mute Rule" instead of "Create Mute Rule".

Fix: Either use the no-arg constructor and set fields after, add a bool isNew parameter, or use the AlertMuteContext constructor path.

3. "Mute This Alert" doesn't pre-fill context fields
Right-clicking an alert only pre-fills server name + metric name, not database/query text/wait type/job name. The AlertMuteContext constructor exists for rich pre-fill but is never called from the UI — the structured fields aren't on the display model. Users have to manually type pattern fields even though the detail info is visible in the alert.

This could be deferred to a follow-up, but worth noting the AlertMuteContext constructor is currently dead code.

---

Medium

4. Lite MuteRuleService skips the LockedConnection pattern
All DuckDB operations in Lite/Services/MuteRuleService.cs call _dbInitializer.CreateConnection() directly without AcquireReadLock()/AcquireWriteLock(). The established pattern in LocalDataService uses locks to coordinate with CHECKPOINT and archive operations. A concurrent CHECKPOINT during a mute rule write could cause issues.

5. Cache-first, persist-second risks silent data loss
Lite's MuteRuleService mutates the in-memory cache under lock, then does a best-effort async DuckDB write. If the write fails silently, the user's changes are lost on next app restart since LoadAsync() reloads from DuckDB.

Consider: attempt the DB write first, update the cache only on success. Or at minimum log a visible warning when persistence fails.

6. Poison wait mute only checks worst wait type
Lite/MainWindow.xaml.cs:1195 — The mute context only includes worst.WaitType. If a user mutes RESOURCE_SEMAPHORE but THREADPOOL is the worst wait, the mute check evaluates THREADPOOL and the alert fires. If the muted wait happens to be worst, other unmuted waits get suppressed too. This is a design choice but may surprise users.

7. MCP tools gap

• get_alert_history in both apps doesn't include the new muted or detail_text columns
• get_mute_rules is missing from the README tools table (line 411)
• README tool counts (lines 49, 278) need incrementing

---

Low / Cosmetic

8. Hardcoded color #33D97706 on MutedBanner in AlertDetailWindow — verify it looks right on Light/Cool Breeze themes.

9. Dashboard AlertDetailWindow missing Icon attribute (Lite sets /EDD.ico).

10. Dashboard MuteRuleService.Save() swallows all exceptions silently — should at least Logger.Error() them.

11. No automatic purge of expired rules — they accumulate until manual purge. Consider purging on startup in LoadAsync().

---

What's Done Well

• All 7 alert metrics covered with consistent mute check pattern
• Muted alerts still logged for full auditability
• Thread safety via lock(_lock) with defensive ToList() copies
• All DuckDB queries properly parameterized (no SQL injection)
• Schema migrations idempotent with IF NOT EXISTS
• config_mute_rules correctly excluded from archivable tables
• Context-sensitive dialog hides irrelevant fields per metric type
• Clean settings integration entry point
• Schema test updated correctly

Bug #1 is a blocker for existing Lite users upgrading. The rest can be follow-up items if needed.

[HannahVernon]

PR Review Feedback — Changes Made (commit 7dc5744)

All 11 items from the review have been addressed. Here's a summary organized by severity:

Bug Fixes

1. [BLOCKER] DuckDB NOT NULL on ALTER TABLE (DuckDbInitializer.cs:563)
Changed v20 migration from BOOLEAN NOT NULL DEFAULT false to BOOLEAN DEFAULT false. DuckDB does not support NOT NULL on ALTER TABLE ADD COLUMN — this would crash on upgrade. Matches the pattern established in the v9 migration (line 424) which already had a comment about this limitation.

2. Dialog title showing "Edit" instead of "Create" (both editions)
Changed MuteThisAlert_Click in both AlertsHistoryContent.xaml.cs (Dashboard) and AlertsHistoryTab.xaml.cs (Lite) to construct MuteRuleDialog with an AlertMuteContext instead of a MuteRule. This uses the context-aware constructor which sets the title to "Create Mute Rule" and pre-fills available fields.

3. Context pre-fill / dead code (both editions)
By switching to the AlertMuteContext constructor (item #2 above), the pre-fill code path is now exercised — ServerName and MetricName are populated from the alert row. Richer field extraction from DetailText (database, query text, etc.) is deferred to a follow-up as suggested.

Medium Priority

4. Lite MuteRuleService — missing locked connections
Added AcquireReadLock() / AcquireWriteLock() to all DuckDB operations in Lite's MuteRuleService.cs, matching the LockedConnection pattern used throughout LocalDataService. This prevents CHECKPOINT from reorganizing the database file while mute rule operations are in progress.

5. Persist-first pattern (Lite MuteRuleService)
Restructured all mutation methods (AddRuleAsync, RemoveRuleAsync, UpdateRuleAsync, SetRuleEnabledAsync) to attempt the DB write first, then update the in-memory cache. Failures are logged via AppLogger.Warn() so the feature still works for the current session even if DuckDB persistence fails.

6. Poison wait mute limitation comment (both editions)
Added explanatory comment in both MainWindow.xaml.cs files (Dashboard line ~1390, Lite line ~1195) documenting that the mute check uses the worst triggered wait type, and that muting a non-worst type won't suppress the alert.

7. MCP tool gaps (both editions + README)

• Added muted and detail_text columns to get_alert_history results in both McpAlertTools.cs files
• Added get_mute_rules to the Alerts row in the README tools table
• Updated tool counts: Dashboard 27→28, Lite 31→32

Low Priority / Cosmetic

8. MutedBanner theme color (both editions)
Replaced hardcoded #33D97706 background with a theme-aware SolidColorBrush using {DynamicResource WarningColor} at 0.2 opacity in both AlertDetailWindow.xaml files. This ensures the muted banner renders correctly across Dark, Light, and CoolBreeze themes.

9. Dashboard AlertDetailWindow icon
Added Icon="/EDD.ico" to Dashboard/AlertDetailWindow.xaml to match the application icon shown on other windows.

10. Dashboard MuteRuleService.Save() error logging
Replaced silent catch {} with catch (Exception ex) { Helpers.Logger.Error(...) } in the Dashboard MuteRuleService.Save() method so persistence failures are logged.

11. Auto-purge expired rules on startup

• Dashboard: PurgeExpiredRules() is called after Load() in the MuteRuleService constructor
• Lite: PurgeExpiredRulesAsync() is called at the end of LoadAsync()

Files Changed (12 files, +112 / -65)

File	Changes
Dashboard/AlertDetailWindow.xaml	Icon, theme-aware MutedBanner
Dashboard/Controls/AlertsHistoryContent.xaml.cs	Use AlertMuteContext constructor
Dashboard/MainWindow.xaml.cs	Poison wait limitation comment
Dashboard/Mcp/McpAlertTools.cs	Add muted + detail_text columns
Dashboard/Services/MuteRuleService.cs	Error logging in Save(), auto-purge
Lite/Controls/AlertsHistoryTab.xaml.cs	Use AlertMuteContext constructor
Lite/Database/DuckDbInitializer.cs	Fix NOT NULL on v20 migration
Lite/MainWindow.xaml.cs	Poison wait limitation comment
Lite/Mcp/McpAlertTools.cs	Add muted + detail_text columns
Lite/Services/MuteRuleService.cs	Locked connections, persist-first, logging, auto-purge
Lite/Windows/AlertDetailWindow.xaml	Theme-aware MutedBanner
README.md	Tool counts, get_mute_rules in tools table

Build: 0 errors, 0 new warnings (all warnings are pre-existing). All 6 unit tests pass.

[erikdarlingdata]

Code Review — Feature/alert muting

Nice work on this feature, Hannah — the architecture is clean and the first round of fixes in 7dc5744 addressed a lot. Here's what I found in the current state:

---

BLOCKERS (4)

1. DynamicResource on SolidColorBrush.Color will crash at runtime
Both Dashboard/AlertDetailWindow.xaml:78 and Lite/Windows/AlertDetailWindow.xaml:78:

<SolidColorBrush Color="{DynamicResource WarningColor}" Opacity="0.2"/>

WPF doesn't support DynamicResource on Freezable.Color — this throws InvalidOperationException when the MutedBanner becomes visible. Change to StaticResource.

2. CheckBox Enabled toggle persists the wrong value
Lite/Windows/ManageMuteRulesWindow.xaml:38 and Dashboard/ManageMuteRulesWindow.xaml:37 — the Click handler fires before the WPF binding pushes IsChecked back to rule.Enabled, so SetRuleEnabled(rule.Id, rule.Enabled) persists the pre-toggle value. Fix: read cb.IsChecked == true directly, or negate rule.Enabled in the handler.

3. Lite get_alert_history MCP tool queries config_alert_log instead of v_config_alert_log
Lite/Mcp/McpAlertTools.cs:40 — misses all archived alert history. Also omits AND dismissed = FALSE, so dismissed alerts leak into results. Should call dataService.GetAlertHistoryAsync() instead of duplicating raw SQL.

4. Dashboard get_alert_settings MCP tool — missing DI registration
Dashboard/Mcp/McpAlertTools.cs:22 — GetAlertSettings takes UserPreferencesService via DI, but it's never registered in McpHostService.cs. Will throw InvalidOperationException when called.

---

MEDIUM (9)

5. Edit dialog mutates the original MuteRule in-place (Dashboard)
Dashboard/MuteRuleDialog.xaml.cs:24-29 — Rule = existingRule stores a direct reference. If a future change adds intermediate writes or the user cancels after partial state, the live rule is corrupted. Should clone before editing.

6. "Mute Similar Alerts" opens in Edit mode (Lite)
Lite/Controls/AlertsHistoryTab.xaml.cs:463-468 — passes a new MuteRule{...} to the existingRule constructor, so the dialog title says "Edit Mute Rule" instead of "Create." Should use the AlertMuteContext constructor.

7. AddRuleAsync adds to memory even when DB persist fails (Lite)
Lite/Services/MuteRuleService.cs:101-116 — rule works until restart, then silently vanishes. User has no indication it didn't persist. Consider not adding to _rules on failure, or surfacing a warning.

8. LogAlertAsync fallback writes to DuckDB without AcquireWriteLock()
Lite/Services/EmailAlertService.cs:210-238 — the INSERT runs without lock coordination, can conflict with CHECKPOINT or archive operations.

9. High CPU alert missing detailText (Lite)
Lite/MainWindow.xaml.cs (CPU alert section) — all other alert types pass detailText to TrySendAlertEmailAsync, but High CPU omits it. The "View Details" dialog for muted CPU alerts shows no details.

10. No guard against "mute everything" rule
Both MuteRule.Matches() — a rule with all filter fields null matches every alert. Summary shows "(matches all alerts)" but there's no validation or confirmation dialog preventing accidental creation.

11. PurgeExpiredRulesAsync removes from memory before confirming DB delete (Lite)
Lite/Services/MuteRuleService.cs:205-234 — if DB delete fails, rules reappear as zombies on restart.

12. Null-forgiving contextMenu! (Dashboard)
Dashboard/Controls/AlertsHistoryContent.xaml.cs:465,477,499 — menuItem.Parent as ContextMenu can return null. Should add null checks instead of !.

13. Inconsistent get_alert_settings JSON shapes between Dashboard and Lite
Field names differ: notifications_enabled vs alerts_enabled, notify_connection_lost vs notify_connection_changes, threshold field names vary. MCP clients switching editions need different parsing logic.

---

LOW (7)

14. AlertDetailWindow has no Close button or Escape key support (both editions)
15. Editing a rule resets its expiration timer from "now" (both editions)
16. Schema difference: fresh installs have NOT NULL on muted/dismissed columns; migrations create them nullable (Lite) — reader code handles both, but schema diverges
17. Auto-purge on startup silently removes expired rules with no trace
18. Hidden pattern fields retain stale text when metric changes and field re-appears
19. MuteRule model duplicated between Dashboard and Lite (future sync burden)
20. Grid rows 14-15 defined but unused in Dashboard/MuteRuleDialog.xaml

---

Summary

The 4 blockers need fixes before merge — #1 and #2 will crash or behave incorrectly at runtime, #3 and #4 break MCP tools. The medium items are worth addressing but aren't ship-stoppers individually. The lows are cleanup/hardening.

Great feature overall — looking forward to the next round! 🎯

[HannahVernon]

Blockers fixed

1. DynamicResource on Freezable.Color crash risk

• Changed DynamicResource WarningColor to StaticResource WarningColor on SolidColorBrush.Color in both AlertDetailWindow.xaml files — WPF doesn't support DynamicResource on Freezable properties

2. CheckBox Enabled toggle persisting wrong value

• EnabledCheckBox_Click handler now reads cb.IsChecked == true instead of rule.Enabled, which hadn't been updated by the WPF binding at Click time

3. Lite MCP get_alert_history queries wrong table

• Replaced raw SQL against config_alert_log with call to dataService.GetAlertHistoryAsync(), which queries v_config_alert_log (includes archived parquet data) and filters dismissed = FALSE

4. Dashboard get_alert_settings MCP tool — missing DI registration

• Added UserPreferencesService to McpHostService constructor and DI container registration

5. Migration guard test improvement

• Updated Migrations_DoNotUseNotNullOnAlterTableAddColumn test to detect multi-line SQL statements using regex with Singleline mode, not just per-line scanning

Bug Fixes (9 issues addressed)

6. Edit dialog mutates original MuteRule in-place (Dashboard & Lite)

• Added MuteRule.Clone() method to both editions
• Edit dialog now clones the rule before modifying, preventing corruption of the live rule if the dialog is cancelled after partial state changes

7. "Mute Similar Alerts" opens in Edit mode (Lite & Dashboard)

• Changed from MuteRuleDialog(new MuteRule{...}) (which shows "Edit Mute Rule") to MuteRuleDialog(new AlertMuteContext{...}) (which shows "Create Mute Rule")

8. AddRuleAsync adds to memory even when DB persist fails (Lite)

• Now returns early on persist failure instead of adding to memory — rule won't silently vanish on restart

9. LogAlertAsync missing AcquireWriteLock (Lite)

• EmailAlertService.LogAlertAsync INSERT now acquires write lock from DuckDbInitializer, preventing conflicts with CHECKPOINT or archive operations

10. High CPU alert missing detailText (Lite)

• CPU alert now passes detailText to TrySendAlertEmailAsync, consistent with all other alert types

11. No guard against "mute everything" rule (Dashboard & Lite)

• Added confirmation dialog in Save_Click when all filter fields are null: "This rule has no filters and will mute ALL alerts. Are you sure?"

12. PurgeExpiredRulesAsync removes from memory before DB delete (Lite)

• Reordered: DB delete now happens first, memory removal only on success — prevents zombie rules reappearing after restart on DB failure

13. Null-forgiving contextMenu! (Dashboard)

• Added proper null checks for menuItem.Parent as ContextMenu in ViewAlertDetails_Click, MuteThisAlert_Click, and MuteSimilarAlerts_Click

14. Inconsistent get_alert_settings JSON shapes (Lite vs Dashboard)

• Aligned Lite field names to match Dashboard: alerts_enabled → notifications_enabled, threshold → threshold_percent (CPU) / threshold_seconds (blocking)

Files changed (44 files, +3006 / -222 lines total across all commits)

[erikdarlingdata]

Follow-up Review — All Fix Commits Verified

Checked all 5 new commits (8b2efd3, 40e5819, 8a20b75, 5cf7fce, 5a543cc). Every item from the original review has been addressed:

#	Item	Status
1	StaticResource on MutedBanner	✅ Fixed
2	CheckBox reads cb.IsChecked directly	✅ Fixed
3	Lite MCP uses dataService.GetAlertHistoryAsync()	✅ Fixed
4	UserPreferencesService DI registration	✅ Fixed
5	Clone-on-edit in MuteRuleDialog	✅ Fixed
6	Mute Similar uses AlertMuteContext constructor	✅ Fixed
7	AddRuleAsync returns early on persist failure	✅ Fixed
8	LogAlertAsync acquires write lock	✅ Fixed
9	CPU alert passes detailText	✅ Fixed
10	Mute-all guard with confirmation dialog	✅ Fixed
11	Purge deletes from DB before memory	✅ Fixed
12	Null checks on ContextMenu cast	✅ Fixed
13	JSON field alignment	✅ Not a bug — Dashboard and Lite have genuinely different config models (separate lost/restored toggles vs single connection-changes toggle, Dashboard-only SMTP health tracking). The MCP output correctly reflects each app's actual settings.

One minor note

EmailAlertService.LogAlertAsync acquires the write lock after CreateConnection(), while MuteRuleService consistently acquires the lock before creating the connection. Not a functional issue (the connection object is inert until opened), just a pattern inconsistency. Optional cleanup.

Looks good to merge. Nice work on the thorough fixes! 👍

[HannahVernon]

Changes since last review

Self-review pass (code review + concurrency audit + cross-edition parity)

Ran three automated review passes against the full diff. Found and fixed two real issues; dismissed two false positives.

---

Bugs fixed

Lock ordering in Lite/Services/EmailAlertService.cs:LogAlertAsync

• Write lock was acquired AFTER creating the DuckDB connection, violating the lock-then-connect pattern used everywhere else in the codebase
• Swapped the two lines so AcquireWriteLock() comes before CreateConnection()

Dashboard CPU alert missing detailText

• Same bug we fixed in Lite (fix [FEATURE] Make the application "High DPI" aware #9) — RecordAlert() for High CPU wasn't passing detailText
• Alert detail window for muted CPU alerts now shows CPU percentage and threshold

False positives investigated and dismissed

Dashboard MuteRuleService.Save() "race condition"

• Automated review flagged Save() as unprotected, but all callers invoke it from inside lock(_lock) — _rules is fully protected during JSON serialization. Not a bug.

Missing ApplyDefaultExpiration() in AlertMuteContext constructor

• That feature lives on feature/alert-muting-part-2, not this PR. Not applicable here.

Minor parity note (not a bug)

• Lite MCP get_alert_settings uses notify_connection_changes (single toggle) vs Dashboard uses notify_connection_lost + notify_connection_restored (two toggles). This reflects a genuine feature difference between editions — Lite has a single setting, Dashboard has granular control.