Darling Viewer: Query Store grid DarkButton crash — Lite's #2114 on the other SKU (#2181, #2331)

[erikdarlingdata]

Closes #2181. #2331 is the fresh duplicate that exposed the gap (being closed as a dupe of #2181, which is reopened as the canonical Darling-side report).

What actually happened

• [BUG] PMLite crashes with 0xc00000fd stack overflow when opening Queries/Performance tab #2114: Lite crashed on the Query Store grid — the Query Store grid has no inline plan column - plan access is context-menu/double-click only, unlike Query Stats and Procedure Stats #1980 port carried the Viewer's DarkButton key into Lite. Fixed in Lite Query Store grid: drop the Viewer-only DarkButton style that stack-overflowed the app #2118 for Lite only, plus a hygiene test scanning for cross-app keys.
• [BUG] Client crashes when clicking the query store by duration #2181 then reported the same crash against the Darling Viewer and was closed as a duplicate of [BUG] PMLite crashes with 0xc00000fd stack overflow when opening Queries/Performance tab #2114 — wrong premise: the Darling Viewer has its own copy of the bug that Lite Query Store grid: drop the Viewer-only DarkButton style that stack-overflowed the app #2118 never touched. ViewerServerTab.xaml:1409 references DarkButton, which IS a Viewer key — defined in MainWindow.xaml's window resources, a scope a UserControl's templates cannot see (StaticResource resolves lexically at load, never through the runtime tree).
• The hygiene test passed because its model was per-APP key existence ("defined somewhere in the app") — exactly the case that still crashes.
• It survived dogfooding because an empty grid never applies its cell template — and the dogfood fleet's Query Store data is empty. The reporters' servers have rows, so it crashes every time for them.
• Timing: stable 3.4.0 was cut 2026-08-07 10:44 UTC; Lite Query Store grid: drop the Viewer-only DarkButton style that stack-overflowed the app #2118 merged 12 hours later. So 3.4.0 ships both SKUs' crashes; [BUG] #2331's reporter is on 3.4.0.

The fix

• Drop the explicit Style — default chrome, byte-for-byte Lite's Lite Query Store grid: drop the Viewer-only DarkButton style that stack-overflowed the app #2118 treatment. (Lite's IsEnabled=HasQueryPlan gate doesn't transplant: Darling's button fetches the stored plan on demand and shows "Plan Not Found" on a miss, by design.)
• The hygiene test gets the model it should have had: per-FILE resolution — own keys + transitively merged dictionaries (Source=, including ;component/ paths) + App.xaml scope — matching WPF's actual StaticResource lookup. The widened scan was proven exact before adoption: run over both apps it flagged exactly this one real crash and zero false positives.

🤖 Generated with Claude Code

[claude]

Nit: the doc comment on TransitiveDefinitions (line 111-112) says pack paths are "resolved against the file, falling back to the repo layout," but there's no repo-layout fallback here — a ;component/ path is always resolved relative to the referencing file's directory after the prefix is stripped. For a same-assembly relative Source (the only kind actually used in this repo today, confirmed via grep -rn "ResourceDictionary Source" across both apps) that's correct. But a genuine pack URI like /OtherAssembly;component/Themes/Foo.xaml — which points at a different project's root, not a path relative to the current file — would resolve to a nonexistent path here. TransitiveDefinitions degrades safely in that case (unresolvable file → empty key set → scan gets stricter, per the comment on line 113), so today this is inert, not a live bug. Just flagging so the comment doesn't overstate what the code does, in case a future merge actually uses cross-assembly ;component/ syntax.

[erikdarlingdata]

Fixed in 9be67af — the doc now says what the code does: prefix stripped, remainder resolved against the referencing file, correct for same-assembly sources (the only kind in the repo), and a cross-assembly pack URI degrades toward stricter. (Note for the record: the first version of this reply cited a hash before the fix had actually been committed — a scripted edit's anchor missed and the chain kept going; caught and corrected within minutes.)

[claude]

Reviewed. This is a well-scoped, well-verified fix — no blocking issues found.

Correctness check performed: I reimplemented the hygiene test's regex/traversal logic and ran it by hand against the actual XAML tree in both apps to confirm the "zero false positives" claim in the PR description. Two things stood out that I specifically checked and can confirm are not bugs:

• DarkGrid/DarkGridRow/DarkGridCell/DarkGridHeader are referenced from ViewerServerTab.xaml, FinOpsTab.xaml, and several *Window.xaml files that don't merge ViewerDarkTheme.xaml. At first glance this looks like the same class of bug as DarkButton (a MainWindow-local key leaking into a sibling file). It isn't — each of those files defines its own local copy of those four styles under its own UserControl.Resources/Window.Resources, so the per-file lexical model resolves them correctly. DarkButton was the one style that MainWindow-local set didn't get duplicated into ViewerServerTab.xaml, which is exactly why it's the one that broke.
• Confirmed via grep that Lite has zero StaticResource DarkButton references, so there's no parity drift to flag — the Lite side was already fixed under Lite Query Store grid: drop the Viewer-only DarkButton style that stack-overflowed the app #2118 and this PR correctly touches only the Darling Viewer.

The new TransitiveDefinitions per-file model (own keys + transitively merged Source= dictionaries + App.xaml scope) matches WPF's actual lexical StaticResource resolution, and is a real improvement over the old per-app model that missed this exact bug shape.

Left one non-blocking nit inline on the test file: a doc-comment/implementation mismatch around ;component/ pack-path resolution (currently dead code — no XAML in this repo uses that syntax — and it fails safe toward stricter, not toward missing bugs).

[claude]

Reviewed. This is a clean, well-scoped fix — verified the following and found no correctness, parity, security, or perf issues:

• Root cause checks out. DarkButton is defined only in MainWindow.xaml's window resources (line 167) and in FinOpsTab.xaml's local copy (line 110) — never in a merged dictionary. ViewerServerTab.xaml (a UserControl) doesn't merge either, and App.xaml only merges Themes/DarkTheme.xaml, which doesn't define the key. So the StaticResource miss is real, and dropping the explicit Style (matching Lite's [BUG] PMLite crashes with 0xc00000fd stack overflow when opening Queries/Performance tab #2114 fix exactly) is the right call. Grepped all remaining StaticResource DarkButton usages — every one left is inside MainWindow.xaml itself, so no other latent instance of this bug remains in the tree.
• IsEnabled gate correctly omitted. Lite's fix uses IsEnabled="{Binding HasQueryPlan}", but Darling's ViewQueryStorePlanInline_Click (ViewerServerTab.Plans.cs:260-289) already handles a missing plan gracefully via a "Plan Not Found" MessageBox — the PR description's stated reason for the divergence is accurate, not an unexplained parity drift.
• Hygiene test widening (per-app → per-file resolution) is a correct model of WPF's actual lexical StaticResource lookup (own file → transitively merged dictionaries via Source= incl. ;component/ pack paths → App.xaml scope). Traced TransitiveDefinitions' cycle guard (per-call-site seenFiles, fresh per top-level scope) and the Ordinal vs OrdinalIgnoreCase comparer split (file-path dedup case-insensitive, resource-key comparison case-sensitive) — both are deliberate and correct, not a latent bug.
• No Lite/Darling parity gap: this hygiene test lives only in Darling.Tests but scans both apps' XAML trees via the Scopes array, so there's no separate Lite-side copy to fall out of sync.
• Minor/non-blocking observation: MergeSource regex (<ResourceDictionary\s+Source\s*=\s*"...") requires Source to immediately follow the element name, so a merged dictionary written as <ResourceDictionary xmlns:x="..." Source="..."> wouldn't be picked up. Not a bug today — every current Source= merge in both trees has it as the sole/first attribute — but worth keeping in mind if that pattern shows up later, since it would silently under-count merged keys and make the scan too strict (false failures), not too permissive.

No SQL changes in this PR, so the T-SQL style guide doesn't apply here.