diff --git a/CHANGELOG.md b/CHANGELOG.md index 98a9a821..25dec41a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **The force-plan recommendation now warns when the regressed query is parameter-sensitive** ([#2138] gap 3) - each regressed_queries row carries a `parameter_sensitivity_cofired` flag, computed inside the drill-down with the PARAMETER_SENSITIVITY detector's own thresholds (one cached plan whose per-execution cost varies >= 10x across parameter values, same floors, same window) joined by query hash - so the flag can never claim evidence the detector would not report. A flagged target's force-plan preview gains a caution block naming the risk (forcing pins ONE shape for every parameter value; the population that preferred the other plan inherits the wrong one permanently, quietly, because a forced plan no longer recompiles away) and the gentler first levers (statistics updates; PSP optimization / Query Store hints on 2022+), and the copy-paste surface gets a compact two-line version of the same warning. Unflagged targets render byte-identically to before. This flag is also the standing gate for the future auto-force bot: a flagged target is never auto-forced. Both SKUs, pinned by live tests in both stores. ### Fixed +- **Darling Viewer crash on Queries -> Query Store by Duration** ([#2181], [#2331]) - the same uncatchable crash class as Lite's #2114, on the OTHER SKU: the grid's inline View Plan button referenced `DarkButton`, a key that IS defined in the Viewer - in `MainWindow.xaml`'s window resources, a scope a UserControl's templates cannot see, because StaticResource resolves lexically at load rather than through the runtime tree. The miss inside a cell template stack-overflows the process the moment the grid renders a row, which is also why it survived dogfooding: an EMPTY Query Store grid never applies its cell template. #2181 reported this against the Darling Viewer and was closed as a duplicate of the Lite fix on a wrong premise; #2331 re-proved it on 3.4.0. The button uses default chrome now (Lite's exact fix), and the XAML hygiene test's model is widened from per-app to per-FILE resolution (own keys + merged dictionaries + App.xaml scope - WPF's actual lookup), which flags exactly this class and produced zero false positives across both apps. - **The store self-metrics sweep's ~5-a-day "Exception while reading from stream" ERRORs were command timeouts in a network-fault costume** ([#2317]) - the sweep's sizing queries (`hypertable_detailed_size` across every hypertable - its inner `hypertable_local_size` is the frame the server log names - and `pg_database_size` over the whole store) ran on Npgsql's default 30-second timeout, which they outgrew under load on a 141-object store with a 100+ GB plan dimension. Npgsql enforces its deadline by cancelling the statement (the store side logs `canceling statement due to user request` - confirmed at the exact failure timestamps in the managed server's own log) and the client is left holding a torn stream, so the ERROR read as "the network broke" - the same misdirection #2294 named on the baseline path, one layer over. Every sweep statement now carries a five-minute timeout, and the worker caps the WHOLE sweep at the same five minutes through a linked cancellation (the sweep is awaited on the main loop, so five sequential statement timeouts must not stack into a 25-minute stall of per-server dispatch), the statement count is pinned to the timeout count so a sixth statement cannot ride the default back in, and the worker's catch names a timeout as a timeout - a sweep that still cannot finish skips the tick and the series gains a self-healing one-hour gap, deliberately NOT retrying into the same load. - **Gapped charts no longer bury their neighbours under opaque black fill** ([#2324]) - the #1944 gap markers (a NaN Y injected mid-gap so lines break across an outage) shipped first in 3.4.0, and they collide with the gradient area fill: reproduced headlessly against ScottPlot 5.1.59, one NaN in a FillY + ColorPositions series renders the ribbon as opaque black polygons with straight chord edges crossing the gap - the fill path closes its contours through the break, and its fill paint under ColorPositions is hardcoded black with the gradient shader expected to paint over it, which a NaN-bearing series defeats. On the reporter's dark theme that black buried every other series on every tab whose data had a collection gap; the one healthy tab was the one with gapless data. A gap-marked series now renders line-only - the break stays visible, nothing is buried - and continuous series keep the full gradient ribbon, pinned in both directions so the fix cannot quietly repeal the fill feature. - **The plan fetch's adaptive candidate sizing is finally wired** ([#2312] Finding 1) - QueryStorePlanXmlState.CandidatePlanCount was designed to size each pass's decompression window from the database's OWN observed average plan size (the doc argues an 11x fleet spread makes any constant wrong somewhere), and the single call site passed null - so every pass on every database sized its window from the 160KB first-contact seed, K ~ 116, always. The runner now carries a per-database estimate in memory (like the adaptive-shrink counters: a restart costs one seed-sized pass) folded by a pure, pinned Learn: an empty pass proves the walk caught up and clears the catch-up flag while keeping the average; a pass cut by either bound (window consumed or byte budget reached) proves a backlog and sets it; an ordinary pass learns its average. Downstream effect on the measured fleet shapes: a genuine 15KB-average database re-sizes from 116 to ~1,259 candidates per pass and walks its backlog an order of magnitude faster, while the catch-up floor still pins the window at seed size during exactly the biased-sample window the overload's doc describes. @@ -2796,6 +2797,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#2158]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2158 [#2246]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2246 [#2319]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2319 +[#2331]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2331 +[#2181]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2181 [#2317]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2317 [#2316]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2316 [#2324]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2324 diff --git a/Darling/Darling.Tests/XamlStaticResourceHygieneTests.cs b/Darling/Darling.Tests/XamlStaticResourceHygieneTests.cs index dbe7085c..0e6ebf48 100644 --- a/Darling/Darling.Tests/XamlStaticResourceHygieneTests.cs +++ b/Darling/Darling.Tests/XamlStaticResourceHygieneTests.cs @@ -16,16 +16,24 @@ namespace Darling.Tests; /// -/// #2114: a {StaticResource Key} whose key is not defined anywhere in the SAME app's XAML is -/// not a style nit — inside a DataGrid cell template it throws XamlParseException during -/// measure, and WPF's re-attempted template realization stack-overflows the process -/// (0xc00000fd, uncatchable, no log). The field crash was exactly that: #1980 ported the -/// Query Store grid's inline plan button into Lite still carrying the VIEWER's DarkButton -/// key. This scan enforces the cross-app boundary: every StaticResource key an app references must -/// be defined in that app's own XAML tree. Scope is per-APP, not per-file — WPF resolves through -/// merged dictionaries and control ancestry that a text scan cannot model, so this deliberately -/// catches only the definitely-broken class (key defined NOWHERE in the app) and never false-fails -/// a key that lives in another file of the same app. +/// #2114 / #2181 / #2331: a {StaticResource Key} whose key is not resolvable from the file that +/// references it is not a style nit — inside a DataGrid cell template it throws +/// XamlParseException during measure, and WPF's re-attempted template realization stack-overflows +/// the process (0xc00000fd, uncatchable, no error dialog). The first version of this scan modeled +/// the failure as CROSS-APP (a key defined nowhere in the same app), because that was #2114's shape — +/// and #2331 proved that model too generous: the Darling Viewer's own Query Store grid referenced +/// DarkButton, a key that IS defined in the app… in MainWindow.xaml's window resources, +/// which a UserControl's templates cannot see. StaticResource resolves LEXICALLY at load (own file, then +/// merged dictionaries, then App.xaml) — never through the runtime element tree, which is +/// DynamicResource's job. It shipped in 3.4.0 and stayed invisible on the dogfood box only because an +/// EMPTY grid never applies its cell template. +/// +/// So the model here is per-FILE, matching WPF's actual lookup: a reference in file F must resolve +/// from F's own definitions, dictionaries F merges (transitively, via +/// <ResourceDictionary Source="…"/>), or the app scope (App.xaml plus everything IT merges). +/// The scan proved exact before it was adopted: run over both apps it flagged exactly the one real crash +/// and zero false positives. A key that must be shared across files belongs in App.xaml or a merged +/// dictionary — moving it there is the fix this test demands, never widening the model back. /// public sealed class XamlStaticResourceHygieneTests { @@ -43,8 +51,11 @@ private static readonly (string App, string[] Roots)[] Scopes = private static readonly Regex Definition = new( @"x:Key\s*=\s*""(?[A-Za-z0-9_.]+)""", RegexOptions.Compiled); + private static readonly Regex MergeSource = new( + @"[^""]+)""", RegexOptions.Compiled); + [Fact] - public void EveryStaticResourceKey_IsDefinedInTheSameAppsXamlTree() + public void EveryStaticResourceKey_ResolvesFromTheFileThatReferencesIt() { var root = FindRepoRoot(); Assert.True(root is not null, @@ -63,31 +74,72 @@ public void EveryStaticResourceKey_IsDefinedInTheSameAppsXamlTree() .ToList(); Assert.NotEmpty(files); - var defined = new HashSet(StringComparer.Ordinal); - foreach (var file in files) - { - foreach (Match m in Definition.Matches(File.ReadAllText(file))) - defined.Add(m.Groups["key"].Value); - } + /* App scope: App.xaml's own keys plus everything it merges — visible everywhere in the app, + because Application resources are the last stop of every StaticResource lookup. */ + var appXaml = files.FirstOrDefault(f => Path.GetFileName(f) == "App.xaml"); + var appScope = appXaml is null + ? new HashSet(StringComparer.Ordinal) + : TransitiveDefinitions(appXaml, new HashSet(StringComparer.OrdinalIgnoreCase)); /* System-supplied keys referenced by name, never defined in app XAML. */ - defined.Add("SystemParameters.VerticalScrollBarWidthKey"); + appScope.Add("SystemParameters.VerticalScrollBarWidthKey"); foreach (var file in files) { + var visible = TransitiveDefinitions(file, new HashSet(StringComparer.OrdinalIgnoreCase)); + visible.UnionWith(appScope); + foreach (Match m in Reference.Matches(File.ReadAllText(file))) { var key = m.Groups["key"].Value; - if (!defined.Contains(key)) + if (!visible.Contains(key)) offenders.Add($"{Path.GetRelativePath(root!, file)}: StaticResource {key} ({app} scope)"); } } } Assert.True(offenders.Count == 0, - "StaticResource keys referenced but defined nowhere in the same app's XAML — inside a cell " + - "template this is the #2114 uncatchable stack-overflow crash, not a cosmetic miss. Define the key " + - "in the app, use an app-local style, or drop the explicit Style:\n" + string.Join("\n", offenders)); + "StaticResource keys that do not resolve from the file referencing them (own definitions + " + + "merged dictionaries + App.xaml scope). StaticResource is LEXICAL — a key defined in another " + + "window's or control's resources is invisible no matter who hosts whom at runtime, and inside a " + + "cell template the miss is the #2114/#2331 uncatchable stack-overflow crash. Define the key in " + + "the same file, move it to App.xaml / a merged dictionary, or drop the explicit Style:\n" + + string.Join("\n", offenders)); + } + + /// The keys defined in a file plus, transitively, in every dictionary it merges via + /// Source= (relative paths, and ;component/ pack paths with the prefix stripped and the + /// remainder resolved against the referencing file — correct for same-assembly sources, the only kind + /// this repo uses; a cross-assembly pack URI would not resolve here). A Source that cannot be resolved + /// contributes nothing, which only ever makes the scan stricter. + private static HashSet TransitiveDefinitions(string file, HashSet seenFiles) + { + var keys = new HashSet(StringComparer.Ordinal); + if (!seenFiles.Add(file) || !File.Exists(file)) + { + return keys; + } + + var text = File.ReadAllText(file); + foreach (Match m in Definition.Matches(text)) + { + keys.Add(m.Groups["key"].Value); + } + + foreach (Match m in MergeSource.Matches(text)) + { + var src = m.Groups["src"].Value; + var componentIndex = src.IndexOf(";component/", StringComparison.OrdinalIgnoreCase); + if (componentIndex >= 0) + { + src = src[(componentIndex + ";component/".Length)..]; + } + + var candidate = Path.GetFullPath(Path.Combine(Path.GetDirectoryName(file)!, src.Replace('/', Path.DirectorySeparatorChar))); + keys.UnionWith(TransitiveDefinitions(candidate, seenFiles)); + } + + return keys; } /// Same walk-up idiom as DocCommentHygieneTests.FindRepoRoot. diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml index 6c8d5fe1..d3afe94a 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml @@ -1405,8 +1405,14 @@ +