The operator can maintain the twelve palette colors of each theme from Settings, with a measured contrast readout and a reset to default (#3577, arm B)

[erikdarlingdata]

Arm (B) of #3577 — the request itself. The report asked for two things: that the Cool Breeze selected tab and Dark's row marks stop being hard to read, and that the operator be able to maintain the colors themselves, with a reset to default. Arm (A) (#3589) did the first: it measured every text-on-accent pair and Dark's markers, gave the accent a per-theme ink key (AccentForegroundColor) that every accent surface shares, and made the row marks theme brushes. This PR does the second, on top of that. No Closes keyword — the coordinator closes #3577 when this merges.

Everything below is either built and measured, or stated as what could not be run here (no Windows on this lane; CI is the first execution of the tests).

What the operator gets

Settings → expand Colors (a collapsed section directly beneath the Color theme drop-down, in both Lite and the Darling viewer). For the theme currently on screen — it follows the combo's live preview — one row per adjustable palette color, grouped Backgrounds / Text / Accent / Rows / Status: a swatch, the hex, a picker button, and the measured WCAG contrast ratio against the surface that color renders on (page ↔ text, raised panel ↔ text, accent ↔ its ink, each status color ↔ page, alternating row ↔ text). Green ≥ 4.50:1, amber 3.00–4.49, red below 3.00, with a one-line note under the rows when anything is below 4.50. Apply writes the file and re-applies live (no restart). Reset to default removes the current theme's block and re-applies. Open file hands the JSON to the shell's editor; a FileSystemWatcher re-applies what is saved there.

The file: Lite %LOCALAPPDATA%\PerformanceMonitorLite-Data\config\theme-overrides.json (beside settings.json); the viewer %APPDATA%\PerformanceMonitorDarling\theme-overrides.json (beside viewer-settings.json — the viewer's existing per-user root is the roaming one, so the file goes where its siblings already are rather than inventing a second root). Keyed by theme, then by color, holding only what the operator changed:

{ "Dark": { "AccentColor": "#3A7BD5", "AccentForegroundColor": "#FFFFFF" }, "CoolBreeze": { "WarningColor": "#B45309" } }

The settled choices, and why each is what it is

Per theme. The base palettes differ — Dark's accent is a sky blue on near-black, Cool Breeze's a steel blue on white — so one override set cannot suit all three. A Dark override never touches Light; the tests pin the isolation.

A closed list of twelve of the eighteen palette colors — BackgroundColor, BackgroundLightColor, BackgroundDarkColor; ForegroundColor, ForegroundDimColor; AccentColor, AccentForegroundColor; AlternatingRowColor; SuccessColor, WarningColor, ErrorColor, InfoColor. Declared once, ThemeColorOverrides.Slots in PerformanceMonitor.Ui, with label, group, meaning and contrast partner per key; the loader, the panel, the README table and the tests all read it from there. The other six (AccentHoverColor, AccentPressedColor, BackgroundLighterColor, ForegroundMutedColor, BorderColor, BorderLightColor) are withheld on purpose and the doc comment says why: they are secondary tones each theme derives from a primary — a hover is the accent lightened a step, a border is the background lifted a step — and exposing them invites pulling a derived tone away from the one it derives from, which is how a hover ends up darker than its rest state. A test proves all eighteen exist in every theme file of both apps, so the exclusion is a choice and not an accident. The list is also the answer to the next request — "can I change just the selected tab?" That is the style layer, a setter inside one control template, and it is not on offer: arm (A) fixed the selected tab by giving the accent an ink key every accent surface shares, not by special-casing the tab, and the same discipline holds here. The operator adjusts the accent and its ink; every accent surface follows.

Warn, don't refuse. A pair under 4.5:1 shows amber or red and the note; Apply still works. The operator asked to maintain their own colors; an app that overrules them on a threshold it measured is not giving them that. The one thing Apply declines is a value that is not a color at all — there is nothing to apply.

The file never takes the app down. It is read at startup, before the first window. Absent is a first run and silent. Malformed costs the whole file, an unknown theme block costs that block, an unknown key or bad value costs that one entry — each said once through the app's logger and skipped, and everything else in the file still applies. This reuses SettingsFileGuard (#2425) rather than re-inventing its absent/unreadable discipline: a save over an unreadable file copies it aside first, and refuses when no copy can be made. A save is a read-modify-write of the document, so a block the loader skipped with a warning survives a save it had nothing to do with.

The regeneration — and the hypothesis that turned out to be half right

The brief's hypothesis was that the theme's brushes are declared Color="{StaticResource XColor}" and resolved at load, so overriding a Color after load would not move its brushes and the brushes would have to be regenerated. True — and not the whole story. Reading the dictionaries: every one of the 51 (Lite) / 59 (viewer) brushes is declared that way, and so is every control style and template. Window, TextBlock, Button, TabItem, DataGrid, ComboBox, ScrollBar, Menu — 184–200 {StaticResource XBrush} setters per file against two DynamicResource. WPF resolves a StaticResource inside a compiled dictionary to the object: the Window style's Background setter holds the very SolidColorBrush instance BackgroundBrush resolved to, and a merged dictionary that redefines the key changes what the key means, not what the setter holds. So the design as written — load the theme, merge a generated dictionary of overridden Colors and re-emitted Brushes — would have recolored the ~700 {DynamicResource} references in the app's own XAML and the code-behind FindResource calls, and left every styled control in the stock palette. A two-tone app. Mutating the loaded brushes in place does not work either: a value realized from a dictionary that belongs to Application.Resources is sealed on the way out (ResourceDictionary.SealValue freezes Freezables once the dictionary has an application owner), and a brush in a style setter is frozen again when the style seals.

So the override is applied where it can reach everything: in the text, before the dictionary is parsed. Each app's project embeds its three theme files a second time as plain text (<EmbeddedResource Include="Themes\*.xaml" LogicalName="Themes/%(Filename).xaml"/>, ~150 KB in a 277 MB tree). A theme with no overrides loads exactly as before — the compiled BAML, unchanged path, unchanged cost; the deprecated Dashboard and the tests never set a file path and get pre-#3577 behaviour byte for byte. A theme with overrides is regenerated: ThemeXamlRewriter rewrites the overridden <Color x:Key="…"> declarations in the text, ThemeManager parses the result with XamlReader.Parse, and every {StaticResource XColor} in the file — the brushes, the two ColorAnimation To= targets, anything added later — resolves against the new value because it is resolved inside the dictionary being built. One dictionary lands in MergedDictionaries, the same live swap the theme combo has always done. Any failure in the chain (text missing, parse error) is a logged warning and the stock dictionary.

The dependency map is derived, not maintained. The loader needs no color→brush map at all — the re-parse follows every dependency the XAML declares. ThemeXamlRewriter.BrushDependencies reads the map out of the text (<SolidColorBrush x:Key="B" Color="{StaticResource C}") so the tests can prove the regenerated dictionary honours it: RegeneratingATheme_MovesEveryBrushTheXamlDerivesFromAnOverriddenColor runs against all six shipped theme files, overrides all twelve exposed colors to distinct sentinels (so a brush that picked up the wrong override fails, not just one that picked up none), rebuilds the dictionary the way Apply does, and asserts every derived brush — 26 in Dark, 35–42 in the light themes, including the {x:Static SystemColors.*Key} grid-selection brushes — carries exactly its color's sentinel, while every brush derived from a withheld color has not moved. The counts differ per theme (Light derives its border and muted brushes from the palette where Dark paints them literally), which is exactly the drift a hand-maintained map would have carried.

What does not follow an override, by construction and documented in the README: the handful of component-specific brushes each theme paints with a fixed value — the plan viewer's panels, the time-range slicer, the deadlock graph, WarningTextBrush, ApexBorderBrush (25 of Dark's 51). They were never derived from the palette.

The GUI, and the picker choice

One shared ThemeColorsPanel (UserControl in PerformanceMonitor.Ui), hosted by one line in each Settings window, so the two apps cannot drift. The rows are built in code from ThemeColorOverrides.Slots — twelve rows of five controls whose labels, groups and partners are already declared once on the slot list. Typing a hex or moving the picker updates the row's swatch and every readout that depends on it (a new page background re-measures every text and status row), but nothing is applied until Apply, because each Apply is a dictionary regeneration and the operator should choose when it happens. Only what differs from the stock palette is written, so a row typed back to its default is not stored and the file only ever says what the operator changed.

Picker: three sliders (hue / saturation / brightness) over a small pure HsvColor record plus a live swatch, in a Popup beside the row's button, writing the hex back into the row's text box. WPF has no color picker; a hue wheel, an eyedropper or a saved-swatches strip would each be more control than twelve rows justify, and the text box takes any #RRGGBB brought from elsewhere. The sliders are stock WPF (the themes carry no Slider style) — functional, and visually plainer than the themed controls around them; noted, not hidden.

No Alt mnemonics on Apply / Reset / Open file, on purpose. The panel shares its host window's access-key scope and both Settings windows have spent every natural letter (A, C, R, S, O, D, F, E, T, L, P, Y). A key on a letter nobody would guess is a worse affordance than none — the same reasoning as Pause Co_llection's comment — and a duplicate would turn Alt+letter into "move focus". Pinned by TheColorsPanel_CarriesNoAccessKeys. The Expander header is Colors, no underscore.

Parity

• Darling viewer: yes. Same panel, same Ui code, its own file under its own per-user root, its own embedded theme text. Viewer-local by design — a color is a preference of the person at the screen, not a fact about the fleet, so it goes to neither the store nor the control plane; a second seat keeps its own. Darling.Tests/ViewerThemeColorOverridesTests pins the location, the absence from ViewerAppSettings, the Storage and Service projects, and that the viewer hosts the shared panel with no private copy of the rows.
• deprecated/Dashboard: no port. Frozen twin, bug-fix only. It shares ThemeManager, never sets OverridesFilePath, and so takes the compiled-dictionary path unchanged. Its ThemeParityTests compare palette keys and values Lite↔Dashboard; this PR adds no keys and changes no values, and the six theme files are untouched.

Tests (CI is their first run)

Lite.Tests/ThemeColorOverrideTests — the twelve spelled out; the six withheld spelled out; exposed ∪ withheld = exactly the 18 declarations of every theme file in both apps; every slot's partner is an exposed key and the brief's pairs are pinned by name. Hex: accepts #RRGGBB / RRGGBB / #AARRGGBB, rejects #FFF, names, junk; ToHex writes upper-case and alpha only when translucent. Loader: a file with two withheld keys, an unknown key, three bad values, an unknown theme and a non-object block yields the two good entries and exactly eight warnings, no throw; per-theme isolation; missing file → empty and silent; five malformed shapes (trailing comma, array root, null, empty, prose) → empty, one warning, file untouched. Save: replaces only the named block, preserves an unknown sibling verbatim, writes keys in slot order; empty set removes exactly that block and leaves siblings; save over an unreadable file quarantines it and says so. Rewriter: byte-for-byte except the named declaration; warns on an undeclared key; the declaration regex matches all 18 in every file exactly once and every declared value parses. Regeneration: the sentinel test above on all six files; no-override regeneration reproduces the stock palette; unparseable text → null plus one warning, never a throw. Contrast: white/black 21.00, the arm-A fixtures (2.71, 5.39, 4.56, 1.99, 7.52, 9.49) computed by the class the readout uses and order-independent; band edges at 3.00 and 4.50. HSV: primaries decompose, six palette colors round-trip. Wiring: both projects embed the text under the name ThemeManager.ThemeTextResourceName asks for; both App.xaml.cs set the path and the log hooks before the first Apply and start the watcher; both Settings windows host the panel beneath the theme combo. Every cross-app read sits inside trees the lite filter already reaches (Darling/PerformanceMonitor.Darling.Viewer/**).

Verification here: dotnet build -c Release -p:EnableWindowsTargeting=true on PerformanceMonitor.Ui, Lite, Lite.Tests, PerformanceMonitor.Darling.Viewer, Darling.Tests, Dashboard and Dashboard.Tests — 0 warnings, 0 errors each, after rebasing onto current dev. The built assemblies were inspected for both the compiled BAML (themes/darktheme.baml) and the embedded text (Themes/DarkTheme.xaml) under the loader's exact names. The rewriter's and dependency-map regexes were emulated in Python against all six theme files (18 declarations each; 26–42 derived brushes; every exposed key depended on in every file). The contrast fixtures and the HSV round-trips were computed independently before being pinned. Screenshots are impossible from this lane.

What it does not do

No style-layer overrides (one tab, one button). No store or control-plane persistence. No Dashboard port. No live preview before Apply. No repainting of the component-specific literal brushes named above. Not adjustable: the six withheld tones.

Out-of-lane findings (reported, not fixed)

• The light themes' stock status colors measure below the text threshold against their page, and the new readout shows it. Light: WarningColor #F57F17 on #F5F7FA = 2.47, InfoColor #2eaef1 on #F5F7FA = 2.32. Cool Breeze: WarningColor 2.09, InfoColor 4.25, SuccessColor 4.04, ErrorColor 4.43. Dark passes all twelve. Arm (A)'s sweep measured Dark's markers and Cool Breeze's accent surfaces, not the light themes' status colors on the page; WarningTextBrush exists precisely because the warning amber is not legible as text on a light page, but the status brushes are also used as markers (dots, borders) where 3:1 is the target — and Light's two are below even that. Not repainted here: choosing new warning/info tones for two themes is a palette-design call, and the values must land in the Dashboard's copies too or ThemeParityTests fails. A fresh Light install will show two red rows in the Colors section; that is the measurement. The tests pin the eight surface/text/accent pairs green everywhere and all twelve green in Dark, and say why the light-theme status rows are not pinned.
• The themes carry no Slider style, so the picker's sliders render in stock WPF chrome inside the popup.
• ThemeManager.Apply("<anything else>") has always fallen back to Dark's dictionary while recording the unknown name as CurrentTheme; the overrides path normalizes the name for its own lookups and leaves that behaviour alone.

Claims in the brief that did not survive contact

• "A merged generated dictionary that redefines the Colors and re-emits the Brushes" would not have reached the theme's own styles (see the regeneration section); the user-facing design is exactly as approved, the mechanism is a text regeneration rather than a merge.
• The viewer's existing per-user root is roaming %APPDATA%\PerformanceMonitorDarling, not %LOCALAPPDATA%; the file follows the root that exists.
• The Settings windows are not tabbed; the section lives beneath the theme combo as the brief's fallback said.

CHANGELOG.md untouched; the entry is handed to the coordinator separately.

[claude]

ReloadFromDiskIfChanged only retries on IOException. Any other exception File.ReadAllText can throw for a file that still passes File.Exists — most notably UnauthorizedAccessException (an ACL change, an AV scan, a sync client like OneDrive/Dropbox briefly locking the file in a way that doesn't surface as IOException) — propagates uncaught out of this method.

This runs from ScheduleReloadFromDisk's DispatcherTimer.Tick handler, on the UI dispatcher thread (dispatcher.BeginInvoke(ScheduleReloadFromDisk)), so an unhandled exception here can crash the app rather than just skip a reload.

This directly contradicts the class's own stated contract in the header doc: "Nothing here can take the app down... An operator who breaks the file gets the stock theme and a log line, not a crash." Every other file read added in this PR (SettingsFileGuard.ReadText, SaveTheme, EnsureOverridesFileExists) catches the broad Exception, consistent with that contract — this is the one spot that doesn't.

Suggest widening the catch here to Exception (or at least adding UnauthorizedAccessException) and reporting through LogWarning the same way the sibling paths do, since a watcher failure is explicitly documented elsewhere as "a convenience... a failure... is a log line and nothing else."

[erikdarlingdata]

Agreed — it was the one read in the class outside the never-crash contract, and it sat on a Tick with nobody above it. Fixed in d92892f: the read is behind ThemeManager.OverridesFileReader, IOException still retries up to MaxReloadRetries, and anything else (or retries exhausted) is a LogWarning with the theme left exactly as it was; the Tick handler also carries a broad catch as the last line of defence. Pinned by ReloadFromDisk_WhenTheReadThrowsUnauthorizedAccess_LogsAWarningAndKeepsTheLoadedOverrides (plus the IOException-cap, unchanged-text and no-file cases) in Lite.Tests/ThemeColorOverrideTests.

[claude]

Reviewed the theme-color-overrides feature (arm B of #3577): the loader/save/regeneration path in PerformanceMonitor.Ui (ThemeColorOverrides, ThemeXamlRewriter, ThemeManager, WcagContrast, HsvColor), the shared ThemeColorsPanel, and the Lite/Darling wiring (App.xaml.cs, csproj embeds, SettingsWindow.xaml, README updates, and both test suites). No T-SQL in this PR. Overall this is very carefully built — the never-crash-on-a-bad-file discipline, the closed key list, the derived (not hand-maintained) brush-dependency map, and the extensive Lite/Darling parity tests are all solid, and I couldn't find a Lite/Darling drift issue or a security problem (the one Process.Start(UseShellExecute=true) call opens a fixed, app-owned config path, not attacker-controlled input; regex-built-from-user-keys is guarded by Regex.Escape and a closed key list).

1. Correctness — one exception path breaks the feature's own "never crashes" contract. ThemeManager.ReloadFromDiskIfChanged (PerformanceMonitor.Ui/ThemeManager.cs) only retries on IOException when re-reading theme-overrides.json after an outside edit. Any other exception File.ReadAllText can throw for a file that still passes File.Exists (most plausibly UnauthorizedAccessException from an AV scan, ACL change, or a sync client's lock) propagates uncaught out of a DispatcherTimer.Tick callback running on the UI thread — i.e., it can crash the app. Every other file read this PR adds (SettingsFileGuard.ReadText, SaveTheme, EnsureOverridesFileExists) catches broadly and logs, consistent with the class's own documented guarantee that "an operator who breaks the file gets... a log line, not a crash." This one path doesn't follow that pattern. Left an inline comment with the specific line.

Nothing else rose to blocking; happy to re-review once that's addressed.

Remediated in d92892f: the watcher's reload reads through ThemeManager.OverridesFileReader and catches broadly (IOException retries to MaxReloadRetries, everything else is LogWarning and the theme stays as it was); the Tick handler is the last line of defence with its own broad catch. Pinned by ReloadFromDisk_WhenTheReadThrowsUnauthorizedAccess_LogsAWarningAndKeepsTheLoadedOverrides in Lite.Tests/ThemeColorOverrideTests.

[claude]

Reviewed the diff (PerformanceMonitor.Ui: ThemeColorOverrides/ThemeXamlRewriter/ThemeManager/WcagContrast/HsvColor/ThemeColorsPanel, plus both apps' wiring, tests and READMEs). No T-SQL in this PR, so the collector-query conventions don't apply here.

Correctness — traced the regeneration path (text rewrite → XamlReader.Parse → one MergedDictionaries swap), the loader's per-theme/per-key skip semantics, SaveTheme's read-modify-write over SettingsFileGuard, and the watcher's debounce/retry/fingerprint logic (ReloadFromDiskIfChanged). All hold up, including the edge cases: file deleted mid-session correctly clears to empty overrides (SourceText both null → Unchanged, non-null → text differs → reapply-to-empty), IOException retries capped at MaxReloadRetries before warning, UnauthorizedAccessException and everything else falls into the broad catch this PR's second commit added. WcagContrast and HsvColor math check out (checked the linearization formula, the band edges, the HSV round-trip) and match the arm-A fixtures cited in the tests.

Lite/Darling parity — verified directly: both App.xaml.cs files set OverridesFilePath/LogWarning/LogInfo before the first Apply() and call WatchOverridesFile() right after, both .csproj files embed Themes\*.xaml under the identical LogicalName pattern, both Settings windows host one <ui:ThemeColorsPanel> beneath the theme combo. The Darling test file (ViewerThemeColorOverridesTests) independently pins that the override machinery stays out of ViewerAppSettings, Storage and Service. Lite's README path (%LOCALAPPDATA%\PerformanceMonitorLite-Data\config\theme-overrides.json) matches DataRootMigration.DataRootName + App.xaml.cs's ConfigDirectory construction.

Security — theme-overrides.json's path is always host-constructed (AppData-derived), never influenced by file contents or network input, so Process.Start(..., UseShellExecute = true) on "Open file" isn't attacker-reachable. JSON is parsed with System.Text.Json/JsonNode, no injection surface. The two new regexes (ColorDeclaration, BrushFromColor) run only over the app's own embedded theme text, not external input, so no ReDoS concern in practice.

Nothing substantive found — this is a clean, well-tested addition that reuses existing infra (SettingsFileGuard) rather than re-inventing it.

[claude]

LGTM — reviewed the theme-override loader, XAML regeneration path, watcher reload logic, WCAG/HSV math, and Lite/Darling wiring parity; no correctness, security, or parity issues found. No T-SQL in this PR.

[claude]

Minor, non-blocking: s_stockPalettes caches by theme name and is never invalidated when ThemeXamlTextProvider is reassigned. In production this is harmless (the provider is set once, at startup, and never swapped), but it's a latent trap for any future test that calls StockPalette after overriding ThemeXamlTextProvider — the cached palette from an earlier call (possibly with a different provider) will be returned silently instead of being recomputed. Might be worth clearing the cache (or keying it off the provider instance) if StockPalette ever becomes something tests exercise directly.

[erikdarlingdata]

Agreed, and worth closing while the context is warm: #3617 makes the provider setter empty the cache (the doc comment carries this note's reasoning) and pins it with StockPalette_FollowsTheProvider_WhenTheProviderIsSwapped. No behaviour change in the apps, where the provider is set once.

[claude]

LGTM — reviewed the theme-color-overrides implementation (ThemeColorOverrides, ThemeXamlRewriter, ThemeManager's regeneration/watcher/reload path, WcagContrast, HsvColor, ThemeColorsPanel), the Lite/Darling wiring in both App.xaml.cs files and both SettingsWindow.xaml files, and both READMEs.

Checked specifically:

• Correctness: hex parsing/round-trip, WCAG luminance/ratio/band math, HSV round-trip, the regex-based XAML rewrite (exact x:Key match, first-occurrence-only replace), the file watcher's debounce/retry/fingerprint logic (own-write vs. outside-edit detection), and the loader's absent/unreadable/per-entry warning semantics via SettingsFileGuard — all consistent with the pinned test fixtures and each other.
• Lite/Darling parity: ThemeColorsPanel is hosted exactly once in each SettingsWindow.xaml, in the same position beneath the theme combo; both apps embed theme XAML text under the same resource name; both set OverridesFilePath/LogWarning/LogInfo before their first ThemeManager.Apply and call WatchOverridesFile() after, matching the "first paint carries overrides" claim. deprecated/Dashboard never sets OverridesFilePath, so it stays on the pre-#3577 compiled-dictionary path unchanged, as documented.
• Security: the overrides file path is always derived from the app's own per-user settings directory plus a fixed filename, never from user input; Process.Start in "Open file" only ever targets that fixed path.
• Docs: Lite's and Darling's README sections agree with each other and with the code on file location, contrast thresholds, and Apply/Reset/Open-file semantics.

Left one non-blocking inline note about ThemeManager.StockPalette's cache not invalidating if ThemeXamlTextProvider is ever swapped at runtime — latent, not currently reachable in production or tests.

No T-SQL/collector changes in this PR (pure WPF/C#), so the T-SQL style conventions don't apply here.