From 5b2dac5bf953247953555543ae30f62d20c43c47 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:41:04 -0400 Subject: [PATCH 001/119] Add MetricFormatter for costs and durations that reach a human Optimizer costs arrive from showplan at full float precision and every display site formatted them itself, so the properties panel and the node tooltips printed "16.765900", "3526.210000" and "0.000000" -- six decimals of noise, and a zero cost dressed up as a measurement. Durations had the same problem from the other end: the statements grid carried its own private ms/s/m ladder that nothing else could reach. MetricFormatter is the one place both shapes live now. Costs: at most four decimals, trailing zeros stripped, thousands separators like the rest of the UI, and exactly zero is "0". The interesting case is a cost small enough to round away at four decimals but not actually zero -- that reads "<0.0001" rather than "0", because a cost that exists must never display as free. The format string is "#,##0.####" rather than "G"/"R" specifically so a huge cost can never fall back to scientific notation in a panel. Durations keep the ladder the statements grid already used (ms under a second, seconds with one decimal under a minute, m+s beyond) rather than inventing a third scale style. Both take an optional IFormatProvider so the tests can pin a culture without mutating global state; production formats in the caller's culture, which is what the "N0"/"N1" rows next to these values do. Human display only. The Robot Advice JSON, MCP tool JSON and plan XML round-tripping keep raw values and do not come through here. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- src/PlanViewer.Core/Output/MetricFormatter.cs | 68 +++++++++++ .../MetricFormatterTests.cs | 115 ++++++++++++++++++ 2 files changed, 183 insertions(+) create mode 100644 src/PlanViewer.Core/Output/MetricFormatter.cs create mode 100644 tests/PlanViewer.Core.Tests/MetricFormatterTests.cs diff --git a/src/PlanViewer.Core/Output/MetricFormatter.cs b/src/PlanViewer.Core/Output/MetricFormatter.cs new file mode 100644 index 00000000..71984a98 --- /dev/null +++ b/src/PlanViewer.Core/Output/MetricFormatter.cs @@ -0,0 +1,68 @@ +using System.Globalization; + +namespace PlanViewer.Core.Output; + +/// +/// The shared shape for the two metric families that reach a human: optimizer costs +/// and durations. Costs come out of showplan at full float precision, which reads as +/// noise in a panel ("16.765900", "0.000000"), and durations come out as raw +/// milliseconds, which stop being readable somewhere past a few seconds. +/// +/// Human-display strings only. Machine-readable output — the Robot Advice JSON, MCP +/// tool results, plan XML round-tripping — must NOT come through here: those keep the +/// raw values so whatever reads them can do its own arithmetic. +/// +public static class MetricFormatter +{ + /// + /// Magnitude below which a cost rounds away to nothing at four decimals. Anything + /// under this is reported as "smaller than the smallest thing we show" rather than + /// rounded to "0". + /// + private const double SmallestShownCost = 0.00005; + + /// + /// An optimizer cost (operator, subtree, I/O, CPU) with at most four decimal places, + /// trailing zeros stripped, and thousands separators like the rest of the UI: + /// 3526.210000 becomes "3,526.21", 16.765900 becomes "16.7659", 0.028900 becomes + /// "0.0289". Exactly zero is "0", never "0.0000". + /// + /// A non-zero cost too small to survive four decimals reads "<0.0001" rather than + /// "0", so a cost that exists never displays as free. Never scientific notation. + /// + public static string FormatCost(double cost, IFormatProvider? provider = null) + { + provider ??= CultureInfo.CurrentCulture; + + if (cost == 0) + return "0"; + + if (Math.Abs(cost) < SmallestShownCost) + return cost > 0 ? "<0.0001" : ">-0.0001"; + + // "#,##0.####" both caps the decimals at four and drops the trailing zeros, and + // unlike "G"/"R" it can never fall back to scientific notation on a huge cost. + return cost.ToString("#,##0.####", provider); + } + + /// + /// A duration in milliseconds on the ladder the statements grid already uses: + /// under a second stays in milliseconds ("847ms"), under a minute scales to seconds + /// with one decimal ("3.5s"), and anything longer splits into minutes and seconds + /// ("20m 35s"). + /// + public static string FormatDuration(long ms, IFormatProvider? provider = null) + { + provider ??= CultureInfo.CurrentCulture; + + if (ms < 1000) + return ms.ToString("N0", provider) + "ms"; + + if (ms < 60_000) + return (ms / 1000.0).ToString("F1", provider) + "s"; + + var minutes = (ms / 60_000).ToString("N0", provider); + var seconds = (ms % 60_000 / 1000).ToString("N0", provider); + return $"{minutes}m {seconds}s"; + } +} diff --git a/tests/PlanViewer.Core.Tests/MetricFormatterTests.cs b/tests/PlanViewer.Core.Tests/MetricFormatterTests.cs new file mode 100644 index 00000000..902e192b --- /dev/null +++ b/tests/PlanViewer.Core.Tests/MetricFormatterTests.cs @@ -0,0 +1,115 @@ +using System.Globalization; +using PlanViewer.Core.Output; + +namespace PlanViewer.Core.Tests; + +// Locks in MetricFormatter — the one place optimizer costs and durations are shaped for a +// human. Contract for costs: at most four decimals, trailing zeros stripped, thousands +// separators, exactly zero is "0", a non-zero cost too small for four decimals is "<0.0001" +// and never "0", and never scientific notation. Contract for durations: the statements-grid +// ladder, ms under a second, seconds with one decimal under a minute, m+s beyond that. +// +// Every case passes InvariantCulture explicitly. Production formats in the caller's culture +// (the panels next to these values already use "N0"/"N1"), so pinning the provider here is +// what keeps the expected strings honest on a machine that separates numbers differently. +public class MetricFormatterTests +{ + private static readonly CultureInfo Invariant = CultureInfo.InvariantCulture; + + [Theory] + // Exactly zero is the whole point of the change: "0.000000" was the worst offender. + [InlineData(0.0, "0")] + // Trailing zeros come off, and four decimals is the cap. + [InlineData(3526.21, "3,526.21")] + [InlineData(16.7659, "16.7659")] + [InlineData(0.0289, "0.0289")] + [InlineData(0.5, "0.5")] + // Whole numbers lose the decimal point entirely rather than showing ".0000". + [InlineData(1.0, "1")] + [InlineData(42.0, "42")] + // Past four decimals it rounds, it does not truncate. + [InlineData(0.00123456, "0.0012")] + [InlineData(1.99999, "2")] + // Thousands separators, including a cost big enough that "G" would go exponential. + [InlineData(44940.5, "44,940.5")] + [InlineData(1234567.891, "1,234,567.891")] + [InlineData(1e12, "1,000,000,000,000")] + // Negative is not a cost showplan produces, but it must not turn into garbage if one arrives. + [InlineData(-16.7659, "-16.7659")] + public void FormatCost_ShapesTheNumber(double cost, string expected) + { + Assert.Equal(expected, MetricFormatter.FormatCost(cost, Invariant)); + } + + [Theory] + // The case that must NOT read "0": a cost this small is tiny, not absent. Everything + // below half of the last shown digit rounds away, so that is where the label starts. + [InlineData(0.000001)] + [InlineData(0.00001)] + [InlineData(0.000049)] + public void FormatCost_TinyButRealCostNeverReadsAsZero(double cost) + { + Assert.Equal("<0.0001", MetricFormatter.FormatCost(cost, Invariant)); + + // And the distinction is the point — a real zero still reads "0". + Assert.NotEqual(MetricFormatter.FormatCost(0, Invariant), MetricFormatter.FormatCost(cost, Invariant)); + } + + [Fact] + public void FormatCost_TinyNegativeKeepsItsSign() + { + Assert.Equal(">-0.0001", MetricFormatter.FormatCost(-0.000001, Invariant)); + } + + [Fact] + public void FormatCost_JustAboveTheThresholdShowsTheDigitInstead() + { + // 0.00006 survives rounding to four decimals, so it gets the digit rather than the label. + Assert.Equal("0.0001", MetricFormatter.FormatCost(0.00006, Invariant)); + } + + [Theory] + // No showplan cost is ever this small or this large, but a format string that falls back to + // scientific notation would corrupt the panel silently, so assert it cannot happen. + [InlineData(1e-30)] + [InlineData(1e20)] + [InlineData(double.MaxValue)] + public void FormatCost_NeverUsesScientificNotation(double cost) + { + var text = MetricFormatter.FormatCost(cost, Invariant); + + Assert.DoesNotContain("E", text, StringComparison.OrdinalIgnoreCase); + } + + [Theory] + // Under a second: milliseconds, no scaling. + [InlineData(0L, "0ms")] + [InlineData(1L, "1ms")] + [InlineData(847L, "847ms")] + [InlineData(999L, "999ms")] + // A second and up: seconds with one decimal. + [InlineData(1000L, "1.0s")] + [InlineData(3475L, "3.5s")] + [InlineData(59_999L, "60.0s")] + // A minute and up: minutes plus whole seconds. + [InlineData(60_000L, "1m 0s")] + [InlineData(1_235_000L, "20m 35s")] + [InlineData(3_600_000L, "60m 0s")] + public void FormatDuration_ClimbsTheLadder(long ms, string expected) + { + Assert.Equal(expected, MetricFormatter.FormatDuration(ms, Invariant)); + } + + [Fact] + public void FormatDuration_MatchesTheLadderTheStatementsGridAlreadyUsed() + { + /* The helper exists to replace a private copy of exactly this ladder in StatementRow. + If someone "improves" the boundaries here, the statements grid and every panel that + now shares this helper move together — which is the point — so the boundaries are + worth pinning rather than leaving to whoever edits next. */ + Assert.Equal("999ms", MetricFormatter.FormatDuration(999, Invariant)); + Assert.Equal("1.0s", MetricFormatter.FormatDuration(1000, Invariant)); + Assert.Equal("59.9s", MetricFormatter.FormatDuration(59_900, Invariant)); + Assert.Equal("1m 0s", MetricFormatter.FormatDuration(60_000, Invariant)); + } +} From 1b38c9a0c7e05f4b1b51f4029b17b65847e421b1 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:41:19 -0400 Subject: [PATCH 002/119] Route the cost and duration display sites through MetricFormatter The sites that were printing raw: - Properties panel: "Operator Cost 16.765900 (0%)", "Subtree Cost 3526.210000", "I/O Cost 0.000000", "CPU Cost 0.000000" were all :F6. They now read "16.7659", "3,526.21" and "0", percentage suffix unchanged. - Node tooltips: the same four values, same :F6, same fix. - Statements grid: dropped its private copy of the duration ladder in favour of the shared one, and its cost column moved off :F2 so the grid and the panel beside it group and round the same number the same way instead of showing "3526.21" next to "3,526.21". - Comparison report: estimated cost was :F4 with no separators while runtime and wait times sat next to it as unscaled milliseconds. Cost now goes through FormatCost and the three duration lines through FormatDuration, so a 20-minute runtime reads "20m 35s" rather than "1,235,000ms". The percentage deltas are still computed from the raw values, so nothing about the comparison arithmetic moved. - Web viewer: the operator properties panel and the tooltip builder were on :N4, which is not the six-decimal defect but is a different answer to the same question, and a zero cost still read "0.0000" there. Same formatter now, so both viewers agree. MetricFormatter.cs needed a linked Compile entry in Web.csproj -- the web project links Core sources file by file rather than referencing the project. WriteMetricLine took a format string plus a unit suffix; it now takes a Func, since "scale this to seconds" is not something a format string can express. Memory grant deliberately stays fixed at MB on both sides: a per-side scale would put "512 KB" opposite "2.1 GB" on the one line whose entire job is a side-by-side. Untouched on purpose: the Robot Advice JSON, the MCP tool JSON output, plan XML round-tripping and Save .sqlplan all keep full raw precision. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Controls/PlanViewerControl.Properties.cs | 8 ++--- .../Controls/PlanViewerControl.Tooltips.cs | 9 ++--- .../Controls/PlanViewerControl.axaml.cs | 18 ++++------ .../Output/ComparisonFormatter.cs | 34 ++++++++++++------- .../Pages/OperatorPropertiesPanel.razor | 8 ++--- src/PlanViewer.Web/PlanViewFormat.cs | 4 +-- src/PlanViewer.Web/PlanViewer.Web.csproj | 1 + 7 files changed, 44 insertions(+), 38 deletions(-) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs index 500514c4..027f9d87 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs @@ -304,10 +304,10 @@ private void ShowPropertiesPanel(PlanNode node) // === Estimated Costs Section === AddPropertySection("Estimated Costs"); - AddPropertyRow("Operator Cost", $"{node.EstimatedOperatorCost:F6} ({node.CostPercent}%)"); - AddPropertyRow("Subtree Cost", $"{node.EstimatedTotalSubtreeCost:F6}"); - AddPropertyRow("I/O Cost", $"{node.EstimateIO:F6}"); - AddPropertyRow("CPU Cost", $"{node.EstimateCPU:F6}"); + AddPropertyRow("Operator Cost", $"{MetricFormatter.FormatCost(node.EstimatedOperatorCost)} ({node.CostPercent}%)"); + AddPropertyRow("Subtree Cost", MetricFormatter.FormatCost(node.EstimatedTotalSubtreeCost)); + AddPropertyRow("I/O Cost", MetricFormatter.FormatCost(node.EstimateIO)); + AddPropertyRow("CPU Cost", MetricFormatter.FormatCost(node.EstimateCPU)); // === Estimated Rows Section === AddPropertySection("Estimated Rows"); diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Tooltips.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Tooltips.cs index 841283f8..e90b938f 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Tooltips.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Tooltips.cs @@ -6,6 +6,7 @@ using Avalonia.Layout; using Avalonia.Media; using PlanViewer.Core.Models; +using PlanViewer.Core.Output; namespace PlanViewer.App.Controls; @@ -40,8 +41,8 @@ private object BuildNodeTooltipContent(PlanNode node, List? allWarn // Cost AddTooltipSection(stack, "Costs"); - AddTooltipRow(stack, "Cost", $"{node.CostPercent}% of statement ({node.EstimatedOperatorCost:F6})"); - AddTooltipRow(stack, "Subtree Cost", $"{node.EstimatedTotalSubtreeCost:F6}"); + AddTooltipRow(stack, "Cost", $"{node.CostPercent}% of statement ({MetricFormatter.FormatCost(node.EstimatedOperatorCost)})"); + AddTooltipRow(stack, "Subtree Cost", MetricFormatter.FormatCost(node.EstimatedTotalSubtreeCost)); // Rows AddTooltipSection(stack, "Rows"); @@ -70,8 +71,8 @@ private object BuildNodeTooltipContent(PlanNode node, List? allWarn if (node.EstimateIO > 0 || node.EstimateCPU > 0 || node.EstimatedRowSize > 0) { AddTooltipSection(stack, "Estimates"); - if (node.EstimateIO > 0) AddTooltipRow(stack, "I/O Cost", $"{node.EstimateIO:F6}"); - if (node.EstimateCPU > 0) AddTooltipRow(stack, "CPU Cost", $"{node.EstimateCPU:F6}"); + if (node.EstimateIO > 0) AddTooltipRow(stack, "I/O Cost", MetricFormatter.FormatCost(node.EstimateIO)); + if (node.EstimateCPU > 0) AddTooltipRow(stack, "CPU Cost", MetricFormatter.FormatCost(node.EstimateCPU)); if (node.EstimatedRowSize > 0) AddTooltipRow(stack, "Avg Row Size", $"{node.EstimatedRowSize} B"); } diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs b/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs index 5df3f24e..507b809f 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs @@ -41,18 +41,12 @@ public class StatementRow public int Warnings { get; set; } public PlanStatement Statement { get; set; } = null!; - // Display helpers - public string CpuDisplay => FormatDuration(CpuMs); - public string ElapsedDisplay => FormatDuration(ElapsedMs); - public string UdfDisplay => UdfMs > 0 ? FormatDuration(UdfMs) : ""; - public string CostDisplay => EstCost > 0 ? $"{EstCost:F2}" : ""; - - private static string FormatDuration(long ms) - { - if (ms < 1000) return $"{ms}ms"; - if (ms < 60_000) return $"{ms / 1000.0:F1}s"; - return $"{ms / 60_000}m {(ms % 60_000) / 1000}s"; - } + // Display helpers. The duration ladder this grid used to carry privately is now + // MetricFormatter's, so the panels and tooltips scale the same numbers the same way. + public string CpuDisplay => MetricFormatter.FormatDuration(CpuMs); + public string ElapsedDisplay => MetricFormatter.FormatDuration(ElapsedMs); + public string UdfDisplay => UdfMs > 0 ? MetricFormatter.FormatDuration(UdfMs) : ""; + public string CostDisplay => EstCost > 0 ? MetricFormatter.FormatCost(EstCost) : ""; } public partial class PlanViewerControl : UserControl diff --git a/src/PlanViewer.Core/Output/ComparisonFormatter.cs b/src/PlanViewer.Core/Output/ComparisonFormatter.cs index bfead874..5e6565bc 100644 --- a/src/PlanViewer.Core/Output/ComparisonFormatter.cs +++ b/src/PlanViewer.Core/Output/ComparisonFormatter.cs @@ -75,28 +75,28 @@ private static void WriteStatementComparison( // Estimated metrics (always available) WriteMetricLine(writer, "Estimated cost", a.EstimatedCost, b.EstimatedCost, - "F4", "", "cheaper", lowerIsBetter: true); + FormatCost, "cheaper", lowerIsBetter: true); WriteMetricLine(writer, "Estimated rows", a.EstimatedRows, b.EstimatedRows, - "N0", "", "fewer", lowerIsBetter: true); + FormatCount, "fewer", lowerIsBetter: true); // Runtime (actual plans only) if (a.QueryTime != null || b.QueryTime != null) { WriteMetricLine(writer, "Runtime", a.QueryTime?.ElapsedTimeMs, b.QueryTime?.ElapsedTimeMs, - "N0", "ms", "faster", lowerIsBetter: true); + FormatDuration, "faster", lowerIsBetter: true); WriteMetricLine(writer, "CPU time", a.QueryTime?.CpuTimeMs, b.QueryTime?.CpuTimeMs, - "N0", "ms", "faster", lowerIsBetter: true); + FormatDuration, "faster", lowerIsBetter: true); } // I/O from operator tree var (aLR, aPR) = SumTreeIO(a.OperatorTree); var (bLR, bPR) = SumTreeIO(b.OperatorTree); if (aLR > 0 || bLR > 0) - WriteMetricLine(writer, "Logical reads", aLR, bLR, "N0", "", "fewer", lowerIsBetter: true); + WriteMetricLine(writer, "Logical reads", aLR, bLR, FormatCount, "fewer", lowerIsBetter: true); if (aPR > 0 || bPR > 0) - WriteMetricLine(writer, "Physical reads", aPR, bPR, "N0", "", "fewer", lowerIsBetter: true); + WriteMetricLine(writer, "Physical reads", aPR, bPR, FormatCount, "fewer", lowerIsBetter: true); // Memory grant if ((a.MemoryGrant != null && a.MemoryGrant.GrantedKB > 0) || @@ -104,7 +104,9 @@ private static void WriteStatementComparison( { var aGrantMB = a.MemoryGrant != null ? a.MemoryGrant.GrantedKB / 1024.0 : 0; var bGrantMB = b.MemoryGrant != null ? b.MemoryGrant.GrantedKB / 1024.0 : 0; - WriteMetricLine(writer, "Memory grant", aGrantMB, bGrantMB, "N1", " MB", "less", lowerIsBetter: true); + // Fixed at MB on both sides: a per-side scale would put "512 KB" opposite "2.1 GB" + // and make the one line whose whole job is a side-by-side unreadable. + WriteMetricLine(writer, "Memory grant", aGrantMB, bGrantMB, FormatMegabytes, "less", lowerIsBetter: true); } // DOP — show raw values, no percentage @@ -126,7 +128,7 @@ private static void WriteStatementComparison( { writer.WriteLine(" Plan A:"); foreach (var w in a.WaitStats.OrderByDescending(w => w.WaitTimeMs)) - writer.WriteLine($" - {w.WaitType} {w.WaitTimeMs:N0}ms"); + writer.WriteLine($" - {w.WaitType} {MetricFormatter.FormatDuration(w.WaitTimeMs)}"); } if (a.WaitStats.Count > 0 && b.WaitStats.Count > 0) writer.WriteLine(); @@ -134,23 +136,31 @@ private static void WriteStatementComparison( { writer.WriteLine(" Plan B:"); foreach (var w in b.WaitStats.OrderByDescending(w => w.WaitTimeMs)) - writer.WriteLine($" - {w.WaitType} {w.WaitTimeMs:N0}ms"); + writer.WriteLine($" - {w.WaitType} {MetricFormatter.FormatDuration(w.WaitTimeMs)}"); } } writer.WriteLine(); } + // The per-metric display shapes. Costs and durations go through the shared + // MetricFormatter so a number reads the same here as it does in the properties + // panel; counts and megabytes are local because nothing else displays them. + private static string FormatCost(double cost) => MetricFormatter.FormatCost(cost); + private static string FormatDuration(double ms) => MetricFormatter.FormatDuration((long)ms); + private static string FormatCount(double count) => count.ToString("N0"); + private static string FormatMegabytes(double mb) => mb.ToString("N1") + " MB"; + private static void WriteMetricLine( TextWriter writer, string label, double? valA, double? valB, - string format, string unit, string betterWord, + Func formatValue, string betterWord, bool lowerIsBetter) { if (!valA.HasValue && !valB.HasValue) return; - var aStr = valA.HasValue ? valA.Value.ToString(format) + unit : "N/A"; - var bStr = valB.HasValue ? valB.Value.ToString(format) + unit : "N/A"; + var aStr = valA.HasValue ? formatValue(valA.Value) : "N/A"; + var bStr = valB.HasValue ? formatValue(valB.Value) : "N/A"; var padded = $" {label}:".PadRight(22); diff --git a/src/PlanViewer.Web/Pages/OperatorPropertiesPanel.razor b/src/PlanViewer.Web/Pages/OperatorPropertiesPanel.razor index e19a8cf0..fdd5f07f 100644 --- a/src/PlanViewer.Web/Pages/OperatorPropertiesPanel.razor +++ b/src/PlanViewer.Web/Pages/OperatorPropertiesPanel.razor @@ -114,10 +114,10 @@
Costs
-
Operator Cost@Node.EstimatedOperatorCost.ToString("N4") (@(Node.CostPercent)%)
-
Subtree Cost@Node.EstimatedTotalSubtreeCost.ToString("N4")
-
I/O Cost@Node.EstimateIO.ToString("N4")
-
CPU Cost@Node.EstimateCPU.ToString("N4")
+
Operator Cost@MetricFormatter.FormatCost(Node.EstimatedOperatorCost) (@(Node.CostPercent)%)
+
Subtree Cost@MetricFormatter.FormatCost(Node.EstimatedTotalSubtreeCost)
+
I/O Cost@MetricFormatter.FormatCost(Node.EstimateIO)
+
CPU Cost@MetricFormatter.FormatCost(Node.EstimateCPU)
diff --git a/src/PlanViewer.Web/PlanViewFormat.cs b/src/PlanViewer.Web/PlanViewFormat.cs index 61f8ed11..8e6a92b0 100644 --- a/src/PlanViewer.Web/PlanViewFormat.cs +++ b/src/PlanViewer.Web/PlanViewFormat.cs @@ -102,8 +102,8 @@ public static string BuildTooltip(PlanNode node) parts.Add($"Estimated rows: {node.EstimateRows:N0}"); } - parts.Add($"Estimated cost: {node.EstimatedOperatorCost:N4}"); - parts.Add($"Subtree cost: {node.EstimatedTotalSubtreeCost:N4}"); + parts.Add($"Estimated cost: {MetricFormatter.FormatCost(node.EstimatedOperatorCost)}"); + parts.Add($"Subtree cost: {MetricFormatter.FormatCost(node.EstimatedTotalSubtreeCost)}"); if (!string.IsNullOrEmpty(node.ObjectName)) parts.Add($"Object: {node.FullObjectName ?? node.ObjectName}"); if (!string.IsNullOrEmpty(node.IndexName)) parts.Add($"Index: {node.IndexName}"); diff --git a/src/PlanViewer.Web/PlanViewer.Web.csproj b/src/PlanViewer.Web/PlanViewer.Web.csproj index bda01eaa..50c52ed0 100644 --- a/src/PlanViewer.Web/PlanViewer.Web.csproj +++ b/src/PlanViewer.Web/PlanViewer.Web.csproj @@ -40,6 +40,7 @@ + From 8910b5fe625bb504d2ed5c8341b2ac440c874fdb Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:41:31 -0400 Subject: [PATCH 003/119] Blank the Query Store ids on grouped rows instead of showing 0 A grouped parent row in the Query Store grid is an aggregate over many plans, so the synthetic QueryStorePlan behind it never gets a QueryId or a PlanId and both come back 0. Query Store ids start at 1, so a column of zeros next to real ids reads like an id rather than "not applicable" -- and both grouping modes produce them at two levels, the root aggregate and the plan-hash/query-hash intermediate. QueryIdDisplay and PlanIdDisplay blank anything at or below zero and the two columns bind to those. Leaf rows are unaffected; they have real ids. The blanking is display only. SortMemberPath still points at the numeric QueryId/PlanId and the column filters still read them through NumericAccessors, so sorting and filtering behave exactly as before -- there is a test pinning that, because binding a column to a string and leaving it to sort as one is the obvious way to break this later. Copy Query ID / Copy Plan ID now disable on a row with no id, following the rule Copy Query Hash and Copy Module Name already use when their value is missing, and the tab-separated clipboard row leaves the two fields empty rather than writing 0. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../QueryStoreGridControl.Selection.cs | 12 ++-- .../Controls/QueryStoreGridControl.axaml | 4 +- .../Controls/QueryStoreGridControl.axaml.cs | 11 ++++ .../QueryStoreGroupedRowIdDisplayTests.cs | 61 +++++++++++++++++++ 4 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 tests/PlanViewer.Core.Tests/QueryStoreGroupedRowIdDisplayTests.cs diff --git a/src/PlanViewer.App/Controls/QueryStoreGridControl.Selection.cs b/src/PlanViewer.App/Controls/QueryStoreGridControl.Selection.cs index 0d5b62c1..5ab5d115 100644 --- a/src/PlanViewer.App/Controls/QueryStoreGridControl.Selection.cs +++ b/src/PlanViewer.App/Controls/QueryStoreGridControl.Selection.cs @@ -134,8 +134,10 @@ private void ContextMenu_Opening(object? sender, System.ComponentModel.CancelEve var hasRow = row != null; ViewHistoryItem.IsEnabled = hasRow; - CopyQueryIdItem.IsEnabled = hasRow; - CopyPlanIdItem.IsEnabled = hasRow; + // Grouped parent rows have no id of their own, so there is nothing to copy — same + // rule the hash and module items already follow when their value is missing. + CopyQueryIdItem.IsEnabled = hasRow && row!.QueryId > 0; + CopyPlanIdItem.IsEnabled = hasRow && row!.PlanId > 0; CopyQueryHashItem.IsEnabled = hasRow && !string.IsNullOrEmpty(row!.QueryHash); CopyPlanHashItem.IsEnabled = hasRow && !string.IsNullOrEmpty(row!.QueryPlanHash); CopyModuleItem.IsEnabled = hasRow && !string.IsNullOrEmpty(row!.ModuleName); @@ -153,8 +155,8 @@ private void ContextMenu_Opening(object? sender, System.ComponentModel.CancelEve if (!hasRow) return; - CopyQueryIdItem.Tag = row!.QueryId.ToString(); - CopyPlanIdItem.Tag = row.PlanId.ToString(); + CopyQueryIdItem.Tag = row!.QueryIdDisplay; + CopyPlanIdItem.Tag = row.PlanIdDisplay; CopyQueryHashItem.Tag = row.QueryHash; CopyPlanHashItem.Tag = row.QueryPlanHash; CopyModuleItem.Tag = row.ModuleName; @@ -178,7 +180,7 @@ private async void CopyMenuItem_Click(object? sender, RoutedEventArgs e) /// One results row as a tab-separated line, shared by Copy Row and Ctrl+C. private static string FormatRowForClipboard(QueryStoreRow row) => - $"{row.QueryId}\t{row.PlanId}\t{row.QueryHash}\t{row.QueryPlanHash}\t{row.ModuleName}\t{row.LastExecutedLocal}\t{row.ExecsDisplay}\t{row.TotalCpuDisplay}\t{row.AvgCpuDisplay}\t{row.TotalDurDisplay}\t{row.AvgDurDisplay}\t{row.TotalReadsDisplay}\t{row.AvgReadsDisplay}\t{row.TotalWritesDisplay}\t{row.AvgWritesDisplay}\t{row.TotalPhysReadsDisplay}\t{row.AvgPhysReadsDisplay}\t{row.TotalMemDisplay}\t{row.AvgMemDisplay}\t{row.FullQueryText}"; + $"{row.QueryIdDisplay}\t{row.PlanIdDisplay}\t{row.QueryHash}\t{row.QueryPlanHash}\t{row.ModuleName}\t{row.LastExecutedLocal}\t{row.ExecsDisplay}\t{row.TotalCpuDisplay}\t{row.AvgCpuDisplay}\t{row.TotalDurDisplay}\t{row.AvgDurDisplay}\t{row.TotalReadsDisplay}\t{row.AvgReadsDisplay}\t{row.TotalWritesDisplay}\t{row.AvgWritesDisplay}\t{row.TotalPhysReadsDisplay}\t{row.AvgPhysReadsDisplay}\t{row.TotalMemDisplay}\t{row.AvgMemDisplay}\t{row.FullQueryText}"; private System.Threading.Tasks.Task SetClipboardTextAsync(string text) => ClipboardHelper.TrySetTextAsync(this, text); diff --git a/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml b/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml index 800110dc..e1e97fd4 100644 --- a/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml +++ b/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml @@ -310,8 +310,8 @@ - - + + diff --git a/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs b/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs index fbb393d9..9525070b 100644 --- a/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs +++ b/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs @@ -473,6 +473,17 @@ public double WaitMaxGrandTotal public long QueryId => Plan.QueryId; public long PlanId => Plan.PlanId; + + /// + /// The ids as the grid shows them. A grouped parent row is an aggregate over many + /// plans, so the synthetic plan behind it carries no id at all and both come back 0 — + /// and a column of zeros reads like a real Query Store id rather than "not applicable". + /// Query Store ids start at 1, so anything at or below zero is the aggregate case and + /// shows blank; leaf rows keep the ids they actually have. + /// + public string QueryIdDisplay => QueryId > 0 ? QueryId.ToString() : ""; + public string PlanIdDisplay => PlanId > 0 ? PlanId.ToString() : ""; + public string QueryHash => Plan.QueryHash; public string QueryPlanHash => Plan.QueryPlanHash; public string ModuleName => Plan.ModuleName; diff --git a/tests/PlanViewer.Core.Tests/QueryStoreGroupedRowIdDisplayTests.cs b/tests/PlanViewer.Core.Tests/QueryStoreGroupedRowIdDisplayTests.cs new file mode 100644 index 00000000..15adc7a4 --- /dev/null +++ b/tests/PlanViewer.Core.Tests/QueryStoreGroupedRowIdDisplayTests.cs @@ -0,0 +1,61 @@ +using PlanViewer.App.Controls; +using PlanViewer.Core.Models; + +namespace PlanViewer.Core.Tests; + +// Locks in what the Query Store grid puts in the QueryId / PlanId columns. A grouped +// parent row aggregates many plans, so the synthetic plan behind it never gets either id +// and both read 0 — which in a column of real Query Store ids looks like an id rather than +// "not applicable". The display strings blank those; the numeric properties keep the 0 +// because sorting, filtering and the bar ratios are all keyed on them. +public class QueryStoreGroupedRowIdDisplayTests +{ + private static QueryStoreRow LeafRow(long queryId, long planId) => + new(new QueryStorePlan { QueryId = queryId, PlanId = planId }); + + // How QueryStoreGridControl builds a grouped parent: AggregateGroupedRows returns a + // QueryStorePlan with the metric totals summed and no ids set at all. + private static QueryStoreRow AggregateRow(params QueryStoreRow[] children) => + new(new QueryStorePlan { QueryHash = "0x1A2B3C" }, 0, "0x1A2B3C", children.ToList()); + + [Fact] + public void LeafRowsShowTheIdsTheyHave() + { + var row = LeafRow(12345, 678); + + Assert.Equal("12345", row.QueryIdDisplay); + Assert.Equal("678", row.PlanIdDisplay); + } + + [Fact] + public void AggregateRowsShowNothingRatherThanZero() + { + var row = AggregateRow(LeafRow(12345, 678), LeafRow(12346, 679)); + + Assert.Equal("", row.QueryIdDisplay); + Assert.Equal("", row.PlanIdDisplay); + } + + [Fact] + public void BlankingIsDisplayOnlyAndLeavesTheSortAndFilterKeysAlone() + { + /* The columns sort on SortMemberPath="QueryId"/"PlanId" and filter through + NumericAccessors, both of which read the numeric properties. Blanking the text + must not reach them, or a grouped view would sort on a string. */ + var row = AggregateRow(LeafRow(12345, 678)); + + Assert.Equal(0, row.QueryId); + Assert.Equal(0, row.PlanId); + } + + [Fact] + public void ChildrenOfAGroupKeepTheirOwnIds() + { + // Only the aggregate loses its ids — the leaves under it are still real plans. + var leaf = LeafRow(12345, 678); + var group = AggregateRow(leaf); + + Assert.Equal("", group.QueryIdDisplay); + Assert.Equal("12345", Assert.Single(group.Children).QueryIdDisplay); + } +} From b5ba82e69bfa38d34af1df7a7defbba4b394594a Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:55:00 -0400 Subject: [PATCH 004/119] Keep the properties panel at the width the user dragged it to The panel opened at a hard 320px and every subsequent node selection re-ran that assignment, so any width the user dragged out was thrown away on the next click. Set the width only on the transition from hidden to visible, and remember whatever the splitter writes onto the column for the rest of the session, the way the minimap already remembers its size. Default open width is now 380 with the column bounded to 280-800 while open. Those bounds are cleared on close because MinWidth clamps a column regardless of its Width, and a 280px strip on a closed panel is not a panel, it is a bug. The splitter itself was a 5px grey hairline painted in BorderBrush, which at 3840x2400 is both invisible and nearly ungrabbable. It is now 6px and fills with the accent color while the pointer is over it. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Controls/PlanViewerControl.Properties.cs | 58 +++++++++++++++++-- .../Controls/PlanViewerControl.axaml | 5 +- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs index 027f9d87..143697e8 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs @@ -16,8 +16,21 @@ namespace PlanViewer.App.Controls; public partial class PlanViewerControl : UserControl { + // Properties panel sizing. The width is static so a width the user drags out survives + // closing the panel and switching plan tabs, the same way the minimap remembers its size. + private const double DefaultPropertiesWidth = 380; + private const double MinPropertiesWidth = 280; + private const double MaxPropertiesWidth = 800; + private const double PropertiesSplitterWidth = 6; + private static double _propertiesPanelWidth = DefaultPropertiesWidth; + private bool _propertiesChromeWired; + + // Accent fill for the properties splitter while the pointer is over it. + private static readonly SolidColorBrush SplitterHoverBrush = new(Color.FromRgb(0x2E, 0xAE, 0xF1)); + private void ShowPropertiesPanel(PlanNode node) { + EnsurePropertiesChrome(); PropertiesContent.Children.Clear(); _sectionLabelColumns.Clear(); _currentSectionGrid = null; @@ -1021,11 +1034,42 @@ murky origin - but until now the only way to see one was to already have clicked PropertiesContent.Children.Add(warningsExpander); } - // Show the panel - _propertiesColumn.Width = new GridLength(320); - _splitterColumn.Width = new GridLength(5); - PropertiesSplitter.IsVisible = true; - PropertiesPanel.IsVisible = true; + /* Show the panel. The width is set only when the panel is opening: setting it on every + selection threw away whatever width the user had dragged, on every single click. */ + if (!PropertiesPanel.IsVisible) + { + _propertiesColumn.MinWidth = MinPropertiesWidth; + _propertiesColumn.MaxWidth = MaxPropertiesWidth; + _propertiesColumn.Width = new GridLength( + Math.Clamp(_propertiesPanelWidth, MinPropertiesWidth, MaxPropertiesWidth)); + _splitterColumn.Width = new GridLength(PropertiesSplitterWidth); + PropertiesSplitter.IsVisible = true; + PropertiesPanel.IsVisible = true; + } + } + + /// + /// One-time wiring for the panel chrome that lives in AXAML: the splitter's hover + /// feedback, and remembering the width the user drags the panel to. + /// + private void EnsurePropertiesChrome() + { + if (_propertiesChromeWired) return; + _propertiesChromeWired = true; + + var splitterIdleBrush = PropertiesSplitter.Background ?? Brushes.Transparent; + PropertiesSplitter.PointerEntered += (_, _) => PropertiesSplitter.Background = SplitterHoverBrush; + PropertiesSplitter.PointerExited += (_, _) => PropertiesSplitter.Background = splitterIdleBrush; + + // The splitter writes the dragged size straight onto the column, so that is where the + // remembered width comes from - no drag tracking of our own. + _propertiesColumn.PropertyChanged += (_, args) => + { + if (args.Property.Name != "Width" || !PropertiesPanel.IsVisible) return; + var width = _propertiesColumn.Width; + if (width.IsAbsolute && width.Value > 0) + _propertiesPanelWidth = width.Value; + }; } private void AddPropertySection(string title) @@ -1146,6 +1190,10 @@ private void ClosePropertiesPanel() { PropertiesPanel.IsVisible = false; PropertiesSplitter.IsVisible = false; + // Clear the open-state bounds first: MinWidth clamps the column whatever its Width + // says, so leaving it set would hold a 280px strip open on a closed panel. + _propertiesColumn.MinWidth = 0; + _propertiesColumn.MaxWidth = double.PositiveInfinity; _propertiesColumn.Width = new GridLength(0); _splitterColumn.Width = new GridLength(0); diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.axaml b/src/PlanViewer.App/Controls/PlanViewerControl.axaml index b0a7c35c..a2772af6 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.axaml +++ b/src/PlanViewer.App/Controls/PlanViewerControl.axaml @@ -346,8 +346,9 @@ TextAlignment="Center" HorizontalAlignment="Center" Margin="0,8,0,0"/> - - + From 435e12db4ceb1f821fc2f3890fc72a928b99f2a6 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:56:59 -0400 Subject: [PATCH 005/119] Give code values the full panel width instead of a 180px column Seek predicates, probe residuals and output column lists were rendered in the value column of a label|value grid. At a 180px value column a fully qualified column list wraps into a tower of [Database].[schema].[fragment] pieces, one or two per line, and reading one meant reassembling it by eye. Rows flagged isCode now put the label on its own line with the value as a monospace block beneath it spanning all three columns, so the text gets the entire panel width and wraps on something closer to a word boundary. Two supporting changes: The label/value drag handle moves from "created with the section's first row" to "created with the section", because a section can now open with a full-width row that has no label column for the handle to sit beside. It sits at a negative ZIndex so full-width rows own their strip of it; nothing else is ever in the gap column, so it still takes the press over an ordinary row. Values become SelectableTextBlock rather than read-only TextBox, and the panel now records a small row model - label, value, searchable text, and the controls each row occupies - while it builds. The panel is raw controls with no bindings behind it, so once a row is in the visual tree its text is the only thing left to work from, and the filter and copy work landing next both need it. The warning lists register through the same model even though they are prose panels rather than label/value grids. Also gives Template Plan Guide the section header it never had; its two rows were landing in whichever grid happened to be built last. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Controls/PlanViewerControl.Properties.cs | 228 ++++++++++++++---- 1 file changed, 186 insertions(+), 42 deletions(-) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs index 143697e8..652a3701 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text; using System.Text.RegularExpressions; using Avalonia; using Avalonia.Controls; @@ -28,11 +29,49 @@ public partial class PlanViewerControl : UserControl // Accent fill for the properties splitter while the pointer is over it. private static readonly SolidColorBrush SplitterHoverBrush = new(Color.FromRgb(0x2E, 0xAE, 0xF1)); + private static readonly FontFamily CodeFontFamily = new("Consolas"); + + /// + /// One row of the properties panel: what it says, what the filter box matches it against, + /// and what the copy menu hands back. Recorded while the panel is built because the panel + /// is raw controls with no bindings behind them, so once a row is in the visual tree its + /// text is the only thing left to work from. + /// + private sealed class PropertyPanelRow + { + public string Label { get; init; } = ""; + public string Value { get; init; } = ""; + public bool IsCode { get; init; } + + /// + /// Plain text for rows that are not a label/value pair - the per-thread breakdown. + /// Null for ordinary rows, which the copy menu renders from Label and Value. + /// + public string? BlockText { get; init; } + + public string SearchText { get; init; } = ""; + + /// Every control the row occupies, so the filter can hide all of them. + public List Controls { get; } = new(); + } + + private sealed class PropertyPanelSection + { + public string Title { get; init; } = ""; + public Expander Expander { get; init; } = null!; + public List Rows { get; } = new(); + } + + private readonly List _propertySections = new(); + private PropertyPanelSection? _currentSection; + private void ShowPropertiesPanel(PlanNode node) { EnsurePropertiesChrome(); PropertiesContent.Children.Clear(); _sectionLabelColumns.Clear(); + _propertySections.Clear(); + _currentSection = null; _currentSectionGrid = null; _currentSectionRowIndex = 0; @@ -676,6 +715,9 @@ private void ShowPropertiesPanel(PlanNode node) // === Template Plan Guide === if (!string.IsNullOrEmpty(s.TemplatePlanGuideName)) { + // Without its own section these two rows land in whichever grid was built last, + // which reads as an unrelated section growing two mystery rows. + AddPropertySection("Template Plan Guide"); AddPropertyRow("Template Plan Guide", s.TemplatePlanGuideName); if (!string.IsNullOrEmpty(s.TemplatePlanGuideDB)) AddPropertyRow("Template Guide DB", s.TemplatePlanGuideDB); @@ -821,6 +863,7 @@ private void ShowPropertiesPanel(PlanNode node) if (s.PlanWarnings.Count > 0) { var planWarningsPanel = new StackPanel(); + var planWarningRows = new List(); var sortedPlanWarnings = s.PlanWarnings .OrderByDescending(w => w.MaxBenefitPercent ?? -1) .ThenByDescending(w => w.Severity) @@ -865,6 +908,7 @@ private void ShowPropertiesPanel(PlanNode node) }); } planWarningsPanel.Children.Add(warnPanel); + planWarningRows.Add(NewWarningRow(planWarnHeader, w.Message, w.ActionableFix, warnPanel)); } var planWarningsExpander = new Expander @@ -888,6 +932,7 @@ private void ShowPropertiesPanel(PlanNode node) HorizontalContentAlignment = HorizontalAlignment.Stretch }; PropertiesContent.Children.Add(planWarningsExpander); + RegisterPropertySection("Plan Warnings", planWarningsExpander).Rows.AddRange(planWarningRows); } /* === Operator Warnings (#440) === @@ -903,6 +948,7 @@ murky origin - but until now the only way to see one was to already have clicked if (operatorWarnings.Count > 0) { var operatorWarningsPanel = new StackPanel(); + var operatorWarningRows = new List(); foreach (var (originNode, w) in operatorWarnings .OrderByDescending(x => x.Warning.MaxBenefitPercent ?? -1) .ThenByDescending(x => x.Warning.Severity) @@ -933,6 +979,8 @@ murky origin - but until now the only way to see one was to already have clicked Margin = new Thickness(16, 0, 0, 0) }); operatorWarningsPanel.Children.Add(opWarnPanel); + operatorWarningRows.Add( + NewWarningRow(opHeaderText, OperatorOriginLabel(originNode), null, opWarnPanel)); } var operatorWarningsExpander = new Expander @@ -960,6 +1008,8 @@ murky origin - but until now the only way to see one was to already have clicked HorizontalContentAlignment = HorizontalAlignment.Stretch }; PropertiesContent.Children.Add(operatorWarningsExpander); + RegisterPropertySection($"Operator Warnings ({operatorWarnings.Count})", operatorWarningsExpander) + .Rows.AddRange(operatorWarningRows); } // === Missing Indexes === @@ -979,6 +1029,7 @@ murky origin - but until now the only way to see one was to already have clicked if (node.HasWarnings) { var warningsPanel = new StackPanel(); + var nodeWarningRows = new List(); var sortedNodeWarnings = node.Warnings .OrderByDescending(w => w.MaxBenefitPercent ?? -1) .ThenByDescending(w => w.Severity) @@ -1009,6 +1060,7 @@ murky origin - but until now the only way to see one was to already have clicked Margin = new Thickness(16, 0, 0, 0) }); warningsPanel.Children.Add(warnPanel); + nodeWarningRows.Add(NewWarningRow(nodeWarnHeader, w.Message, null, warnPanel)); } var warningsExpander = new Expander @@ -1032,6 +1084,7 @@ murky origin - but until now the only way to see one was to already have clicked HorizontalContentAlignment = HorizontalAlignment.Stretch }; PropertiesContent.Children.Add(warningsExpander); + RegisterPropertySection("Warnings", warningsExpander).Rows.AddRange(nodeWarningRows); } /* Show the panel. The width is set only when the panel is opening: setting it on every @@ -1072,6 +1125,40 @@ private void EnsurePropertiesChrome() }; } + /// + /// Wraps one already-built warning panel as a filterable, copyable row. + /// + private static PropertyPanelRow NewWarningRow( + string header, string body, string? fix, Control panel) + { + var text = new StringBuilder(); + text.Append(" ").AppendLine(header); + if (!string.IsNullOrEmpty(body)) text.Append(" ").AppendLine(body); + if (!string.IsNullOrEmpty(fix)) text.Append(" ").AppendLine(fix); + + var row = new PropertyPanelRow + { + Label = header, + Value = body, + BlockText = text.ToString().TrimEnd(), + SearchText = $"{header} {body} {fix}" + }; + row.Controls.Add(panel); + return row; + } + + /// + /// Registers an expander that was built by hand rather than through + /// - the warning lists, which are stacked panels of prose + /// rather than label/value grids - so the filter and the copy menu cover them too. + /// + private PropertyPanelSection RegisterPropertySection(string title, Expander expander) + { + var section = new PropertyPanelSection { Title = title, Expander = expander }; + _propertySections.Add(section); + return section; + } + private void AddPropertySection(string title) { var labelCol = new ColumnDefinition { Width = new GridLength(_propertyLabelWidth) }; @@ -1099,6 +1186,26 @@ private void AddPropertySection(string title) sectionGrid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(4) }); sectionGrid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + /* The label/value drag handle, in the 4px gap column. It used to be created with the + section's first row; it lives here now because a section can open with a full-width + code row, and that row has no label column for the handle to sit beside. + + ZIndex keeps it under its siblings so the full-width rows own their strip of it - + nothing else is ever in column 1, so over an ordinary row it still takes the press. */ + var labelSplitter = new GridSplitter + { + Width = 4, + Background = Brushes.Transparent, + Foreground = Brushes.Transparent, + BorderThickness = new Thickness(0), + ZIndex = -1, + Cursor = new Avalonia.Input.Cursor(Avalonia.Input.StandardCursorType.SizeWestEast) + }; + Grid.SetColumn(labelSplitter, 1); + Grid.SetRow(labelSplitter, 0); + Grid.SetRowSpan(labelSplitter, 100); + sectionGrid.Children.Add(labelSplitter); + _currentSectionGrid = sectionGrid; _currentSectionRowIndex = 0; @@ -1123,62 +1230,99 @@ private void AddPropertySection(string title) HorizontalContentAlignment = HorizontalAlignment.Stretch }; PropertiesContent.Children.Add(expander); + + _currentSection = new PropertyPanelSection { Title = title, Expander = expander }; + _propertySections.Add(_currentSection); } private void AddPropertyRow(string label, string value, bool isCode = false, bool indent = false) { - if (_currentSectionGrid == null) return; - - var row = _currentSectionRowIndex++; - _currentSectionGrid.RowDefinitions.Add(new RowDefinition { Height = GridLength.Auto }); + if (_currentSectionGrid == null || _currentSection == null) return; - var labelBlock = new TextBlock + var entry = new PropertyPanelRow { - Text = label, - FontSize = indent ? 10 : 11, - Foreground = TooltipFgBrush, - VerticalAlignment = VerticalAlignment.Top, - TextWrapping = TextWrapping.Wrap, - Margin = new Thickness(indent ? 16 : 4, 2, 0, 2) + Label = label, + Value = value, + IsCode = isCode, + SearchText = $"{label} {value}" }; - Grid.SetColumn(labelBlock, 0); - Grid.SetRow(labelBlock, row); - _currentSectionGrid.Children.Add(labelBlock); - // GridSplitter in column 1 (only in first row per section) - if (row == 0) + if (isCode) { - var splitter = new GridSplitter + /* Code values get the whole panel width, label on its own line above them. In the + label|value split a seek predicate or an output column list wraps inside a ~180px + column and comes out a tower of [Database].[schema].[fragment] pieces, one or two + per line, which is the least readable thing in this panel by a distance. */ + if (!string.IsNullOrEmpty(label)) + AddSectionRowControl(NewPropertyLabel(label, indent), entry, fullWidth: true); + + AddSectionRowControl(new SelectableTextBlock { - Width = 4, + Text = value, + FontFamily = CodeFontFamily, + FontSize = indent ? 10 : 11, + Foreground = TooltipFgBrush, + TextWrapping = TextWrapping.Wrap, + // Without a background a text block is only hit-testable where its glyphs + // landed, so presses in the margins never start a selection (#503). Background = Brushes.Transparent, - Foreground = Brushes.Transparent, - BorderThickness = new Thickness(0), - Cursor = new Avalonia.Input.Cursor(Avalonia.Input.StandardCursorType.SizeWestEast) - }; - Grid.SetColumn(splitter, 1); - Grid.SetRow(splitter, 0); - Grid.SetRowSpan(splitter, 100); // span all rows - _currentSectionGrid.Children.Add(splitter); + Margin = new Thickness(indent ? 20 : 10, 0, 4, 3) + }, entry, fullWidth: true); } + else + { + var row = NextSectionRow(); + AddSectionRowControl(NewPropertyLabel(label, indent), entry, fullWidth: false, row: row); + AddSectionRowControl(new SelectableTextBlock + { + Text = value, + FontSize = indent ? 10 : 11, + Foreground = TooltipFgBrush, + TextWrapping = TextWrapping.Wrap, + Background = Brushes.Transparent, + Margin = new Thickness(0, 2, 4, 2), + VerticalAlignment = VerticalAlignment.Top + }, entry, fullWidth: false, row: row, column: 2); + } + + _currentSection.Rows.Add(entry); + } - var valueBox = new TextBox + private static TextBlock NewPropertyLabel(string label, bool indent) => new() + { + Text = label, + FontSize = indent ? 10 : 11, + Foreground = TooltipFgBrush, + VerticalAlignment = VerticalAlignment.Top, + TextWrapping = TextWrapping.Wrap, + Background = Brushes.Transparent, + Margin = new Thickness(indent ? 16 : 4, 2, 0, 2) + }; + + private int NextSectionRow() + { + _currentSectionGrid!.RowDefinitions.Add(new RowDefinition { Height = GridLength.Auto }); + return _currentSectionRowIndex++; + } + + /// + /// Places a control in the current section's grid and records it on , + /// which is what lets the filter hide the row later. Full-width controls take a row of their + /// own spanning all three columns. + /// + private void AddSectionRowControl( + Control control, PropertyPanelRow entry, bool fullWidth, int row = -1, int column = 0) + { + if (fullWidth) { - Text = value, - FontSize = indent ? 10 : 11, - Foreground = TooltipFgBrush, - TextWrapping = TextWrapping.Wrap, - IsReadOnly = true, - BorderThickness = new Thickness(0), - Background = Brushes.Transparent, - Padding = new Thickness(0), - Margin = new Thickness(0, 2, 4, 2), - VerticalAlignment = VerticalAlignment.Top - }; - if (isCode) valueBox.FontFamily = new FontFamily("Consolas"); - Grid.SetColumn(valueBox, 2); - Grid.SetRow(valueBox, row); - _currentSectionGrid.Children.Add(valueBox); + row = NextSectionRow(); + Grid.SetColumnSpan(control, 3); + } + + Grid.SetRow(control, row); + Grid.SetColumn(control, column); + _currentSectionGrid!.Children.Add(control); + entry.Controls.Add(control); } private void CloseProperties_Click(object? sender, RoutedEventArgs e) From ff7f1d9d59851b2f7438bdade83d710e4528a411 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:58:55 -0400 Subject: [PATCH 006/119] Roll the per-thread stats into one collapsed breakdown per section Every actual metric emitted one indented "Thread N" row per thread, inline, right under its own summary row: rows, rows read, executions, elapsed, CPU, logical reads, physical reads, scans, read-aheads. A DOP 4 hash match already produced about thirty of those rows and DOP 8 produces hundreds, so the handful of summary numbers people actually open this panel for were buried in a scroll marathon. The summary rows are untouched. The per-thread numbers now live in one "Per-thread breakdown" sub-expander per affected section (Actual Statistics, Actual Timing, Actual I/O), collapsed by default, with the threads grouped under a small header per metric. Metrics with no per-thread data are skipped, so a section only grows a breakdown when there is something in it. Rows and executions list idle threads too, since a thread sitting at zero while its siblings work is exactly what someone opens the breakdown to see. The breakdown header carries the skew when there is any: the share test mirrors PlanAnalyzer's Rule 8 so the header never contradicts the Parallel Skew warning the same plan raises, plus an idle-thread test Rule 8 does not make, because a thread that returned nothing at all while its siblings did real work reads as skew on sight even when the busiest thread is under Rule 8's share threshold. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Controls/PlanViewerControl.Properties.cs | 223 +++++++++++++++--- 1 file changed, 184 insertions(+), 39 deletions(-) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs index 652a3701..d0d6476d 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs @@ -29,6 +29,9 @@ public partial class PlanViewerControl : UserControl // Accent fill for the properties splitter while the pointer is over it. private static readonly SolidColorBrush SplitterHoverBrush = new(Color.FromRgb(0x2E, 0xAE, 0xF1)); + // The amber this panel already uses for Warning-severity warnings. + private static readonly SolidColorBrush PropWarningBrush = new(Color.FromRgb(0xFF, 0xB3, 0x47)); + private static readonly FontFamily CodeFontFamily = new("Consolas"); /// @@ -382,20 +385,9 @@ private void ShowPropertiesPanel(PlanNode node) { AddPropertySection("Actual Statistics"); AddPropertyRow("Actual Rows", $"{node.ActualRows:N0}"); - if (node.PerThreadStats.Count > 1) - foreach (var t in node.PerThreadStats) - AddPropertyRow($" Thread {t.ThreadId}", $"{t.ActualRows:N0}", indent: true); if (node.ActualRowsRead > 0) - { AddPropertyRow("Actual Rows Read", $"{node.ActualRowsRead:N0}"); - if (node.PerThreadStats.Count > 1) - foreach (var t in node.PerThreadStats.Where(t => t.ActualRowsRead > 0)) - AddPropertyRow($" Thread {t.ThreadId}", $"{t.ActualRowsRead:N0}", indent: true); - } AddPropertyRow("Actual Executions", $"{node.ActualExecutions:N0}"); - if (node.PerThreadStats.Count > 1) - foreach (var t in node.PerThreadStats) - AddPropertyRow($" Thread {t.ThreadId}", $"{t.ActualExecutions:N0}", indent: true); if (node.ActualRebinds > 0) AddPropertyRow("Actual Rebinds", $"{node.ActualRebinds:N0}"); if (node.ActualRewinds > 0) @@ -409,29 +401,30 @@ private void ShowPropertiesPanel(PlanNode node) AddPropertyRow("Partition Ranges", node.PartitionRanges); } + // Rows and executions list every thread, idle ones included: a thread sitting at + // zero while its siblings work is the whole point of looking at the breakdown. + AddPerThreadBreakdown(node, + ("Rows", t => t.ActualRows, true, ""), + ("Rows Read", t => t.ActualRowsRead, false, ""), + ("Executions", t => t.ActualExecutions, true, "")); + // Timing if (node.ActualElapsedMs > 0 || node.ActualCPUMs > 0 || node.UdfCpuTimeMs > 0 || node.UdfElapsedTimeMs > 0) { AddPropertySection("Actual Timing"); if (node.ActualElapsedMs > 0) - { AddPropertyRow("Elapsed Time", $"{node.ActualElapsedMs:N0} ms"); - if (node.PerThreadStats.Count > 1) - foreach (var t in node.PerThreadStats.Where(t => t.ActualElapsedMs > 0)) - AddPropertyRow($" Thread {t.ThreadId}", $"{t.ActualElapsedMs:N0} ms", indent: true); - } if (node.ActualCPUMs > 0) - { AddPropertyRow("CPU Time", $"{node.ActualCPUMs:N0} ms"); - if (node.PerThreadStats.Count > 1) - foreach (var t in node.PerThreadStats.Where(t => t.ActualCPUMs > 0)) - AddPropertyRow($" Thread {t.ThreadId}", $"{t.ActualCPUMs:N0} ms", indent: true); - } if (node.UdfElapsedTimeMs > 0) AddPropertyRow("UDF Elapsed", $"{node.UdfElapsedTimeMs:N0} ms"); if (node.UdfCpuTimeMs > 0) AddPropertyRow("UDF CPU", $"{node.UdfCpuTimeMs:N0} ms"); + + AddPerThreadBreakdown(node, + ("Elapsed", t => t.ActualElapsedMs, false, " ms"), + ("CPU", t => t.ActualCPUMs, false, " ms")); } // I/O @@ -442,34 +435,22 @@ private void ShowPropertiesPanel(PlanNode node) { AddPropertySection("Actual I/O"); AddPropertyRow("Logical Reads", $"{node.ActualLogicalReads:N0}"); - if (node.PerThreadStats.Count > 1) - foreach (var t in node.PerThreadStats.Where(t => t.ActualLogicalReads > 0)) - AddPropertyRow($" Thread {t.ThreadId}", $"{t.ActualLogicalReads:N0}", indent: true); if (node.ActualPhysicalReads > 0) - { AddPropertyRow("Physical Reads", $"{node.ActualPhysicalReads:N0}"); - if (node.PerThreadStats.Count > 1) - foreach (var t in node.PerThreadStats.Where(t => t.ActualPhysicalReads > 0)) - AddPropertyRow($" Thread {t.ThreadId}", $"{t.ActualPhysicalReads:N0}", indent: true); - } if (node.ActualScans > 0) - { AddPropertyRow("Scans", $"{node.ActualScans:N0}"); - if (node.PerThreadStats.Count > 1) - foreach (var t in node.PerThreadStats.Where(t => t.ActualScans > 0)) - AddPropertyRow($" Thread {t.ThreadId}", $"{t.ActualScans:N0}", indent: true); - } if (node.ActualReadAheads > 0) - { AddPropertyRow("Read-Ahead Reads", $"{node.ActualReadAheads:N0}"); - if (node.PerThreadStats.Count > 1) - foreach (var t in node.PerThreadStats.Where(t => t.ActualReadAheads > 0)) - AddPropertyRow($" Thread {t.ThreadId}", $"{t.ActualReadAheads:N0}", indent: true); - } if (node.ActualSegmentReads > 0) AddPropertyRow("Segment Reads", $"{node.ActualSegmentReads:N0}"); if (node.ActualSegmentSkips > 0) AddPropertyRow("Segment Skips", $"{node.ActualSegmentSkips:N0}"); + + AddPerThreadBreakdown(node, + ("Logical Reads", t => t.ActualLogicalReads, false, ""), + ("Physical Reads", t => t.ActualPhysicalReads, false, ""), + ("Scans", t => t.ActualScans, false, ""), + ("Read-Ahead Reads", t => t.ActualReadAheads, false, "")); } // LOB I/O @@ -1125,6 +1106,170 @@ private void EnsurePropertiesChrome() }; } + /// + /// Moves a section's per-thread numbers out of the flat row list and into one collapsed + /// sub-expander, grouped under a small header per metric. + /// + /// Every actual metric used to emit one indented "Thread N" row per thread inline, + /// right under its own summary row. A DOP 4 hash match produced about thirty of them and + /// DOP 8 produces hundreds, so the summary numbers most people open this panel for were + /// buried in a scroll marathon of detail almost nobody wants expanded by default. + /// + /// Metrics with no per-thread data at all are skipped, so a section only shows the + /// breakdown when there is something in it. + /// + private void AddPerThreadBreakdown( + PlanNode node, + params (string Metric, Func Value, bool IncludeIdleThreads, string Unit)[] metrics) + { + if (_currentSectionGrid == null || _currentSection == null || node.PerThreadStats.Count <= 1) + return; + + var panel = new StackPanel { Margin = new Thickness(10, 2, 6, 4) }; + var copyText = new StringBuilder(); + var searchText = new StringBuilder(); + var groupCount = 0; + + foreach (var metric in metrics) + { + var threads = node.PerThreadStats + .Where(t => metric.IncludeIdleThreads || metric.Value(t) > 0) + .ToList(); + if (threads.Count == 0 || threads.All(t => metric.Value(t) == 0)) + continue; + + groupCount++; + panel.Children.Add(new TextBlock + { + Text = metric.Metric, + FontSize = 10, + FontWeight = FontWeight.SemiBold, + Foreground = SectionHeaderBrush, + Margin = new Thickness(0, groupCount == 1 ? 0 : 5, 0, 1) + }); + copyText.Append(" ").AppendLine(metric.Metric); + searchText.Append(metric.Metric).Append(' '); + + var threadGrid = new Grid { Margin = new Thickness(8, 0, 0, 0) }; + threadGrid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(72) }); + threadGrid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + + var rowIndex = 0; + foreach (var t in threads) + { + threadGrid.RowDefinitions.Add(new RowDefinition { Height = GridLength.Auto }); + var threadValue = $"{metric.Value(t):N0}{metric.Unit}"; + + var threadLabelBlock = new TextBlock + { + Text = $"Thread {t.ThreadId}", + FontSize = 10, + Foreground = TooltipFgBrush + }; + Grid.SetRow(threadLabelBlock, rowIndex); + Grid.SetColumn(threadLabelBlock, 0); + threadGrid.Children.Add(threadLabelBlock); + + var threadValueBlock = new TextBlock + { + Text = threadValue, + FontSize = 10, + Foreground = TooltipFgBrush, + TextWrapping = TextWrapping.Wrap + }; + Grid.SetRow(threadValueBlock, rowIndex); + Grid.SetColumn(threadValueBlock, 1); + threadGrid.Children.Add(threadValueBlock); + + copyText.Append(" Thread ").Append(t.ThreadId).Append(": ").AppendLine(threadValue); + searchText.Append("Thread ").Append(t.ThreadId).Append(' ').Append(threadValue).Append(' '); + rowIndex++; + } + + panel.Children.Add(threadGrid); + } + + if (groupCount == 0) return; + + var headerText = $"Per-thread breakdown ({node.PerThreadStats.Count} threads)"; + var (isSkewed, maxRows, minRows) = ThreadRowSkew(node); + var skewSuffix = isSkewed ? $"(skewed: {maxRows:N0} max / {minRows:N0} min)" : ""; + + var header = new StackPanel { Orientation = Orientation.Horizontal }; + header.Children.Add(new TextBlock + { + Text = headerText, + FontWeight = FontWeight.SemiBold, + FontSize = 11, + Foreground = SectionHeaderBrush, + VerticalAlignment = VerticalAlignment.Center + }); + if (isSkewed) + { + header.Children.Add(new TextBlock + { + Text = skewSuffix, + FontWeight = FontWeight.SemiBold, + FontSize = 11, + Foreground = PropWarningBrush, + VerticalAlignment = VerticalAlignment.Center, + Margin = new Thickness(6, 0, 0, 0) + }); + } + + var breakdown = new Expander + { + IsExpanded = false, + Header = header, + Content = panel, + Margin = new Thickness(0, 2, 0, 2), + Padding = new Thickness(0), + Foreground = SectionHeaderBrush, + BorderThickness = new Thickness(0), + HorizontalAlignment = HorizontalAlignment.Stretch, + HorizontalContentAlignment = HorizontalAlignment.Stretch + }; + + var entry = new PropertyPanelRow + { + Label = headerText, + BlockText = $" {headerText} {skewSuffix}".TrimEnd() + + Environment.NewLine + copyText.ToString().TrimEnd(), + SearchText = $"{headerText} {skewSuffix} {searchText}" + }; + AddSectionRowControl(breakdown, entry, fullWidth: true); + _currentSection.Rows.Add(entry); + } + + /// + /// Per-thread row skew, for the breakdown header. + /// + /// The share test mirrors PlanAnalyzer's Rule 8 (Parallel Skew) so this header never + /// disagrees with the warning the same plan raises. The idle-thread test is additional: a + /// thread that returned no rows at all while its siblings did real work is the shape people + /// read as skew on sight, and Rule 8 stays quiet about it whenever the busiest thread is + /// still under its share threshold. + /// + private static (bool IsSkewed, long MaxRows, long MinRows) ThreadRowSkew(PlanNode node) + { + // Thread 0 is the coordinator and normally moves no rows in a parallel operator. + var workers = node.PerThreadStats.Where(t => t.ThreadId > 0).ToList(); + if (workers.Count < 2) workers = node.PerThreadStats; + if (workers.Count < 2) return (false, 0, 0); + + var maxRows = workers.Max(t => t.ActualRows); + var minRows = workers.Min(t => t.ActualRows); + var totalRows = workers.Sum(t => t.ActualRows); + + // Below this there are too few rows to distribute for a split to mean anything. + if (totalRows < workers.Count * 1000L) return (false, maxRows, minRows); + + // At DOP 2 a 60/40 split is normal, so that case needs a higher bar. + var shareThreshold = workers.Count <= 2 ? 0.80 : 0.50; + var isSkewed = (double)maxRows / totalRows >= shareThreshold || minRows == 0; + return (isSkewed, maxRows, minRows); + } + /// /// Wraps one already-built warning panel as a filterable, copyable row. /// From 217c580e8c143f2ff9646936bde8cc553a38443d Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 16:00:01 -0400 Subject: [PATCH 007/119] Give property values a copy menu instead of the stock text-box one Right-clicking a value produced the default TextBox menu: Cut and Copy greyed out because nothing was selected, and Paste enabled, on read-only data. There was no way to copy a value, a row, or the panel. Every row now carries a menu of its own - Copy value, Copy name and value, and Copy all properties - shared between the row's label and value controls so a right-click anywhere on the row hits it. The warning panels get the same menu. Any context flyout the theme attached is cleared, so the stock menu cannot come back. Copy all properties renders the whole panel from the row model rather than walking the visual tree: the operator header, then a line per section with its rows indented under it, and code values verbatim on their own lines so a predicate or a CREATE INDEX pastes as-is instead of arriving re-indented. Clipboard writes go through the existing SetClipboardTextAsync, which carries the retry for a clipboard another process has locked (#415). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Controls/PlanViewerControl.Properties.cs | 87 ++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs index d0d6476d..dadec0fd 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs @@ -56,6 +56,16 @@ private sealed class PropertyPanelRow /// Every control the row occupies, so the filter can hide all of them. public List Controls { get; } = new(); + + /// The copy menu, shared by every control in the row. + public ContextMenu? Menu { get; set; } + + /// What "Copy value" hands back. + public string CopyValue => BlockText ?? Value; + + /// What "Copy name and value" hands back. + public string CopyLabelAndValue => BlockText + ?? (string.IsNullOrEmpty(Label) ? Value : $"{Label}: {Value}"); } private sealed class PropertyPanelSection @@ -1273,7 +1283,7 @@ private static (bool IsSkewed, long MaxRows, long MinRows) ThreadRowSkew(PlanNod /// /// Wraps one already-built warning panel as a filterable, copyable row. /// - private static PropertyPanelRow NewWarningRow( + private PropertyPanelRow NewWarningRow( string header, string body, string? fix, Control panel) { var text = new StringBuilder(); @@ -1289,6 +1299,7 @@ private static PropertyPanelRow NewWarningRow( SearchText = $"{header} {body} {fix}" }; row.Controls.Add(panel); + AttachPropertyRowMenu(panel, row); return row; } @@ -1466,10 +1477,84 @@ private void AddSectionRowControl( Grid.SetRow(control, row); Grid.SetColumn(control, column); + AttachPropertyRowMenu(control, entry); _currentSectionGrid!.Children.Add(control); entry.Controls.Add(control); } + /// + /// Gives a control the row's copy menu, and clears any context flyout the theme put on it. + /// A read-only value used to answer right-click with the stock text menu: Cut and Copy + /// greyed out, Paste enabled, on data nobody can edit. + /// + private void AttachPropertyRowMenu(Control control, PropertyPanelRow entry) + { + control.ContextMenu = entry.Menu ??= BuildPropertyRowMenu(entry); + control.ContextFlyout = null; + } + + private ContextMenu BuildPropertyRowMenu(PropertyPanelRow entry) + { + var menu = new ContextMenu(); + + var copyValueItem = new MenuItem { Header = "Copy value" }; + copyValueItem.Click += async (_, _) => await SetClipboardTextAsync(entry.CopyValue); + menu.Items.Add(copyValueItem); + + var copyRowItem = new MenuItem { Header = "Copy name and value" }; + copyRowItem.Click += async (_, _) => await SetClipboardTextAsync(entry.CopyLabelAndValue); + menu.Items.Add(copyRowItem); + + menu.Items.Add(new Separator()); + + var copyAllItem = new MenuItem { Header = "Copy all properties" }; + copyAllItem.Click += async (_, _) => await SetClipboardTextAsync(BuildPropertiesText()); + menu.Items.Add(copyAllItem); + + return menu; + } + + /// + /// The whole panel as plain text, for "Copy all properties": the operator header, then a + /// line per section title with " Label: Value" beneath it. Code values go out verbatim on + /// their own lines, so a predicate or a CREATE INDEX comes back out pasteable rather than + /// re-indented into something that has to be cleaned up first. + /// + private string BuildPropertiesText() + { + var text = new StringBuilder(); + text.AppendLine(PropertiesHeader.Text); + if (!string.IsNullOrEmpty(PropertiesSubHeader.Text)) + text.AppendLine(PropertiesSubHeader.Text); + + foreach (var section in _propertySections) + { + if (section.Rows.Count == 0) continue; + + text.AppendLine(); + text.AppendLine(section.Title); + foreach (var row in section.Rows) + { + if (row.BlockText != null) + { + text.AppendLine(row.BlockText); + } + else if (row.IsCode) + { + if (!string.IsNullOrEmpty(row.Label)) + text.Append(" ").Append(row.Label).AppendLine(":"); + text.AppendLine(row.Value); + } + else + { + text.Append(" ").Append(row.Label).Append(": ").AppendLine(row.Value); + } + } + } + + return text.ToString().TrimEnd(); + } + private void CloseProperties_Click(object? sender, RoutedEventArgs e) { ClosePropertiesPanel(); From e156e56bc9385da6dda0c6ed30619e43c91e4e37 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 16:03:00 -0400 Subject: [PATCH 008/119] Add a filter box to the properties panel An operator with actual stats runs well past sixty rows across a dozen sections, and the only way to find one of them was to scroll and read. A filter box sits under the panel header and matches, case-insensitively, against both the label and the value of every row, code blocks included. Non-matching rows hide, and a section hides outright once nothing in it is left showing. A section whose own title matches keeps all of its rows, so typing a section name jumps to that section instead of emptying it. The text is sticky across node selections - it lives in the header chrome, which the rebuild does not touch - and is re-applied every time the panel rebuilds. Esc clears it while the box has focus. Empty sections now hide with no filter text at all. A few can be built with no rows (Operator Details when the only property that qualified the node contributes no row of its own, for one), and an expander with nothing inside it was never worth a line. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Controls/PlanViewerControl.Properties.cs | 48 +++++++++++++++++++ .../Controls/PlanViewerControl.axaml | 12 +++++ 2 files changed, 60 insertions(+) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs index dadec0fd..aa5a4289 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs @@ -1078,6 +1078,9 @@ murky origin - but until now the only way to see one was to already have clicked RegisterPropertySection("Warnings", warningsExpander).Rows.AddRange(nodeWarningRows); } + // The filter box keeps its text across selections, so a rebuilt panel has to re-apply it. + ApplyPropertiesFilter(); + /* Show the panel. The width is set only when the panel is opening: setting it on every selection threw away whatever width the user had dragged, on every single click. */ if (!PropertiesPanel.IsVisible) @@ -1092,6 +1095,51 @@ murky origin - but until now the only way to see one was to already have clicked } } + private void PropertiesFilter_TextChanged(object? sender, TextChangedEventArgs e) + => ApplyPropertiesFilter(); + + private void PropertiesFilter_KeyDown(object? sender, Avalonia.Input.KeyEventArgs e) + { + if (e.Key != Avalonia.Input.Key.Escape) return; + PropertiesFilterBox.Text = ""; + e.Handled = true; + } + + /// + /// Hides every row whose label and value miss the filter text, and hides a section outright + /// once nothing in it is left showing. + /// + /// A section whose own title matches keeps all of its rows, so typing a section name + /// is a way to jump to that section rather than a way to empty it. + /// + /// An empty section is hidden even with no filter text. A few of them can be built + /// with no rows at all - Operator Details when the only thing that qualified the node + /// contributes no row of its own, for one - and an expander with nothing inside it is + /// noise whether or not anyone is filtering. + /// + private void ApplyPropertiesFilter() + { + var filter = PropertiesFilterBox.Text?.Trim() ?? ""; + + foreach (var section in _propertySections) + { + var sectionMatches = filter.Length == 0 + || section.Title.Contains(filter, StringComparison.OrdinalIgnoreCase); + + var anyVisible = false; + foreach (var row in section.Rows) + { + var visible = sectionMatches + || row.SearchText.Contains(filter, StringComparison.OrdinalIgnoreCase); + foreach (var control in row.Controls) + control.IsVisible = visible; + anyVisible |= visible; + } + + section.Expander.IsVisible = anyVisible; + } + } + /// /// One-time wiring for the panel chrome that lives in AXAML: the splitter's hover /// feedback, and remembering the width the user drags the panel to. diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.axaml b/src/PlanViewer.App/Controls/PlanViewerControl.axaml index a2772af6..2a6ede37 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.axaml +++ b/src/PlanViewer.App/Controls/PlanViewerControl.axaml @@ -378,6 +378,18 @@ + + + + + From 645716add8f00cafe2365d5e4618cad77d4bc658 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 16:31:03 -0400 Subject: [PATCH 009/119] Keep Ctrl+C guarded and the splitter hover themeable Two defects an Avalonia review of this branch turned up, both verified against 11.3.20 in a headless harness rather than by reading docs. Ctrl+C on a property value was no longer crash-safe. SelectableTextBlock registers its own CopyingToClipboard routed event rather than sharing TextBox's, so the app-wide TextBoxClipboardGuard stopped covering these values the moment they stopped being read-only TextBoxes, and SelectableTextBlock.Copy() is async void over an unguarded SetTextAsync - the exact shape that crashed the app when another process held the clipboard (#415). Every value now handles that event and goes through ClipboardHelper, so the panel's Ctrl+C and its copy menu take the same guarded path. The splitter's hover was wired in code-behind, which pinned Background to a plain brush at local-value priority the first time the pointer left it, permanently severing the {DynamicResource BorderBrush} the AXAML asked for, and hard-coded the accent color a second time. Both states are Setters on the splitter now, resting and pointer-over, resolving BorderBrush and AccentBrush from the theme. The Background attribute had to go with it: a local value outranks a Style setter, and with it in place the hover never fires at all. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Controls/PlanViewerControl.Properties.cs | 34 ++++++++++++++----- .../Controls/PlanViewerControl.axaml | 18 +++++++--- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs index aa5a4289..c96872dd 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs @@ -26,9 +26,6 @@ public partial class PlanViewerControl : UserControl private static double _propertiesPanelWidth = DefaultPropertiesWidth; private bool _propertiesChromeWired; - // Accent fill for the properties splitter while the pointer is over it. - private static readonly SolidColorBrush SplitterHoverBrush = new(Color.FromRgb(0x2E, 0xAE, 0xF1)); - // The amber this panel already uses for Warning-severity warnings. private static readonly SolidColorBrush PropWarningBrush = new(Color.FromRgb(0xFF, 0xB3, 0x47)); @@ -1141,18 +1138,14 @@ private void ApplyPropertiesFilter() } /// - /// One-time wiring for the panel chrome that lives in AXAML: the splitter's hover - /// feedback, and remembering the width the user drags the panel to. + /// One-time wiring for the panel chrome that lives in AXAML: remembering the width the + /// user drags the panel to. /// private void EnsurePropertiesChrome() { if (_propertiesChromeWired) return; _propertiesChromeWired = true; - var splitterIdleBrush = PropertiesSplitter.Background ?? Brushes.Transparent; - PropertiesSplitter.PointerEntered += (_, _) => PropertiesSplitter.Background = SplitterHoverBrush; - PropertiesSplitter.PointerExited += (_, _) => PropertiesSplitter.Background = splitterIdleBrush; - // The splitter writes the dragged size straight onto the column, so that is where the // remembered width comes from - no drag tracking of our own. _propertiesColumn.PropertyChanged += (_, args) => @@ -1492,6 +1485,27 @@ private void AddPropertyRow(string label, string value, bool isCode = false, boo _currentSection.Rows.Add(entry); } + /// + /// Routes a value's Ctrl+C through the guarded clipboard helper. + /// + /// SelectableTextBlock.Copy() is async void over an unguarded SetTextAsync - the same + /// shape that crashed the app when another process held the clipboard (#415). The app-wide + /// guard in hangs off TextBox's own routed events, and + /// SelectableTextBlock registers its own, so these values would have gone from guarded + /// (they were read-only TextBoxes) to unguarded. Handling the event stops the built-in path + /// before it reaches the clipboard. + /// + private static void GuardValueCopy(SelectableTextBlock value) + => value.AddHandler(SelectableTextBlock.CopyingToClipboardEvent, OnPropertyValueCopying); + + private static void OnPropertyValueCopying(object? sender, RoutedEventArgs e) + { + if (sender is not SelectableTextBlock value || !value.CanCopy) return; + + e.Handled = true; + _ = ClipboardHelper.TrySetTextAsync(value, value.SelectedText); + } + private static TextBlock NewPropertyLabel(string label, bool indent) => new() { Text = label, @@ -1539,6 +1553,8 @@ private void AttachPropertyRowMenu(Control control, PropertyPanelRow entry) { control.ContextMenu = entry.Menu ??= BuildPropertyRowMenu(entry); control.ContextFlyout = null; + if (control is SelectableTextBlock value) + GuardValueCopy(value); } private ContextMenu BuildPropertyRowMenu(PropertyPanelRow entry) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.axaml b/src/PlanViewer.App/Controls/PlanViewerControl.axaml index 2a6ede37..256c1ad9 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.axaml +++ b/src/PlanViewer.App/Controls/PlanViewerControl.axaml @@ -346,11 +346,21 @@ TextAlignment="Center" HorizontalAlignment="Center" Margin="0,8,0,0"/> - + + IsVisible="False"> + + + + + Date: Mon, 14 Sep 2026 15:35:49 -0400 Subject: [PATCH 010/119] Stop exception text from parking in the session status strip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "A task was canceled." — the raw message off an OperationCanceledException — was landing in the query session's toolbar strip with autoClear: false and staying there. Switching sub-tabs mid-fetch is the ordinary way to produce it (the fetch is torn down, the exception comes back, the catch prints it), and the message then followed the user across every view they visited afterwards, reading as a complaint about whichever one they had moved to. It only went away when some later action happened to overwrite it. Three changes, all in QuerySessionControl: - Cancellation is never reported. SetStatusFromException drops OperationCanceledException (TaskCanceledException derives from it) and shows nothing; the Query Store, schema and format catch sites go through it. The two explicit cancel catches in the execution paths lose their "Cancelled" message for the same reason — the cancel was the user's own (Escape, the Cancel button, or starting the next query) and the spinner tab vanishing already says so. That line was also about to become invisible anyway: removing the selected tab moves the selection, which now empties the strip. - Failures look like failures and do not last forever. SetErrorStatus paints the strip with a new ErrorBrush token (#E06C75, the muted red the plan viewer's load error already uses) and clears after 12 seconds instead of never. Every former autoClear: false error site uses it, as do the error messages that were already auto-clearing at 3s, so the strip now reads one way: red means the session could not do what you asked. Progress and success messages ("Formatting...", "plan captured", "Loaded Indexes for X") keep the ordinary foreground and the 3s clear. - A message cannot outlive the view it was about. The strip empties when the active sub-tab changes, and when the session leaves the visual tree (a top-level tab switch) — the latter is where the old code cancelled the pending clear without taking the text down, so anything showing at that moment was parked permanently by construction. Two deliberate exceptions. The schema lookup's "Fetching Indexes for X..." keeps autoClear: false: it is progress for an operation that may take a while and is always replaced by its own completion or error line. And the Query Store read-only replica notice is now red rather than persistent — it explains why nothing opened, which is a failure the user needs to read, and 12 seconds beats both the old 3 and never. QueryStoreOverview's "Loading..." moved to after its tab is selected, since selecting a sub-tab now clears the strip and would have wiped a message set ahead of the switch. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Controls/QuerySessionControl.Advice.cs | 6 +- .../Controls/QuerySessionControl.Editor.cs | 67 ++++++++++++++++--- .../Controls/QuerySessionControl.Execution.cs | 19 ++++-- .../Controls/QuerySessionControl.Format.cs | 10 +-- .../Controls/QuerySessionControl.Plans.cs | 4 +- .../QuerySessionControl.QueryStore.cs | 22 +++--- .../Controls/QuerySessionControl.Schema.cs | 2 +- .../Controls/QuerySessionControl.axaml.cs | 15 +++-- src/PlanViewer.App/Themes/DarkTheme.axaml | 2 + 9 files changed, 106 insertions(+), 41 deletions(-) diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Advice.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Advice.cs index 85f2e7d6..0c706ba5 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Advice.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Advice.cs @@ -38,7 +38,7 @@ public partial class QuerySessionControl : UserControl private void HumanAdvice_Click(object? sender, RoutedEventArgs e) { var (analysis, viewer) = GetCurrentAnalysisWithViewer(); - if (analysis == null) { SetStatus("No plan to analyze", autoClear: false); return; } + if (analysis == null) { SetErrorStatus("No plan to analyze"); return; } var text = TextFormatter.Format(analysis); ShowAdviceWindow("Advice for Humans", text, analysis, viewer); @@ -47,7 +47,7 @@ private void HumanAdvice_Click(object? sender, RoutedEventArgs e) private void RobotAdvice_Click(object? sender, RoutedEventArgs e) { var analysis = GetCurrentAnalysis(); - if (analysis == null) { SetStatus("No plan to analyze", autoClear: false); return; } + if (analysis == null) { SetErrorStatus("No plan to analyze"); return; } string json; try @@ -60,7 +60,7 @@ private void RobotAdvice_Click(object? sender, RoutedEventArgs e) process down with no dialog and nothing logged. AnalysisJson's depth ceiling makes this unreachable for any plan seen in the field — this catch is here so that "unreachable" is not the only thing standing between a deep plan and a silent crash. */ - SetStatus($"Could not build robot advice for this plan: {ex.Message}", autoClear: false); + SetErrorStatus($"Could not build robot advice for this plan: {ex.Message}"); return; } diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Editor.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Editor.cs index bad58e2b..ab2df8da 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Editor.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Editor.cs @@ -12,6 +12,7 @@ using Avalonia.Input; using Avalonia.Interactivity; using Avalonia.Layout; +using Avalonia.Markup.Xaml.MarkupExtensions; using Avalonia.Media; using AvaloniaEdit; using AvaloniaEdit.CodeCompletion; @@ -358,7 +359,50 @@ private string GetTextFromCursor() return text[batchStart..batchEnd].Trim(); } - private void SetStatus(string text, bool autoClear = true) + /// How long an ordinary message — progress, or something that worked — stays up. + private static readonly TimeSpan StatusClearDelay = TimeSpan.FromSeconds(3); + + /// + /// How long a failure stays up. Longer than an ordinary message, because an error is the one + /// thing on this strip worth reading twice, but still finite: a message that never clears + /// outlives the view it was about and ends up hanging over an unrelated one. + /// + private static readonly TimeSpan ErrorStatusClearDelay = TimeSpan.FromSeconds(12); + + private void SetStatus(string text, bool autoClear = true) => + ShowStatus(text, isError: false, autoClear ? StatusClearDelay : null); + + /// + /// Reports something the session could not do. Red, and gone on its own before long. + /// + private void SetErrorStatus(string text) => + ShowStatus(text, isError: true, ErrorStatusClearDelay); + + /// + /// Reports a failed operation — unless it failed because the user moved on. + /// + /// Cancellation is not a failure worth a word: switching sub-tabs, closing a view, or + /// starting the next thing tears down whatever was in flight, and the exception that comes + /// back says "A task was canceled." with no hint of which task or why. That string used to + /// land in the strip with autoClear: false and sit there across every view the user + /// visited afterwards. derives from + /// , so the one check covers both. + /// + private void SetStatusFromException(Exception ex, string prefix = "") + { + if (ex is OperationCanceledException) + return; + + SetErrorStatus(prefix + ex.Message); + } + + /// + /// Empties the strip. Called when the active sub-tab changes and when the session leaves the + /// visual tree, so a message never outlives what it was about. + /// + private void ClearStatus() => ShowStatus("", isError: false, clearAfter: null); + + private void ShowStatus(string text, bool isError, TimeSpan? clearAfter) { var old = _statusClearCts; _statusClearCts = null; @@ -367,19 +411,24 @@ private void SetStatus(string text, bool autoClear = true) StatusText.Text = text; + /* Bound rather than assigned so the strip keeps following the theme dictionary, the way + its XAML foreground always has. */ + StatusText[!TextBlock.ForegroundProperty] = + new DynamicResourceExtension(isError ? "ErrorBrush" : "ForegroundBrush"); + /* The bar is one line and trims with an ellipsis, so a long message - an error, usually - is readable only on hover. Setting the tip to the same text costs nothing when it fits and is the difference between a truncated error and a recoverable one when it does not. */ ToolTip.SetTip(StatusText, string.IsNullOrEmpty(text) ? null : text); - if (autoClear && !string.IsNullOrEmpty(text)) + if (clearAfter is not { } delay || string.IsNullOrEmpty(text)) + return; + + var cts = new CancellationTokenSource(); + _statusClearCts = cts; + _ = Task.Delay(delay, cts.Token).ContinueWith(_ => { - var cts = new CancellationTokenSource(); - _statusClearCts = cts; - _ = Task.Delay(3000, cts.Token).ContinueWith(_ => - { - Avalonia.Threading.Dispatcher.UIThread.Post(() => StatusText.Text = ""); - }, TaskContinuationOptions.OnlyOnRanToCompletion); - } + Avalonia.Threading.Dispatcher.UIThread.Post(ClearStatus); + }, TaskContinuationOptions.OnlyOnRanToCompletion); } } diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs index cc250454..3af54b59 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs @@ -44,7 +44,7 @@ private async Task CaptureAndShowPlan(bool estimated, string? queryTextOverride { if (_serverConnection == null || _selectedDatabase == null) { - SetStatus("Connect to a server first", autoClear: false); + SetErrorStatus("Connect to a server first"); return; } @@ -57,7 +57,7 @@ private async Task CaptureAndShowPlan(bool estimated, string? queryTextOverride ?? QueryEditor.Text?.Trim(); if (string.IsNullOrEmpty(queryText)) { - SetStatus("Enter a query", autoClear: false); + SetErrorStatus("Enter a query"); return; } @@ -203,7 +203,11 @@ failure is reported. A SQL error is the one string in this app a user most needs } catch (OperationCanceledException) { - SetStatus("Cancelled"); + /* Nothing in the strip. The user cancelled this themselves — Escape, the Cancel + button, or by starting the next query — and the spinner tab vanishing is the + answer to that. Saying so as well used to be harmless and is no longer even + visible: removing the selected tab moves the selection, and a selection change + now empties the strip. */ SubTabControl.Items.Remove(loadingTab); } catch (SqlException ex) @@ -277,13 +281,13 @@ private async void GetActualPlan_Click(object? sender, RoutedEventArgs e) var viewer = GetSelectedPlanViewer(); if (viewer == null) { - SetStatus("Select a plan tab first"); + SetErrorStatus("Select a plan tab first"); return; } if (_connectionString == null || _selectedDatabase == null) { - SetStatus("Connect to a server first", autoClear: false); + SetErrorStatus("Connect to a server first"); return; } @@ -292,7 +296,7 @@ private async void GetActualPlan_Click(object? sender, RoutedEventArgs e) if (string.IsNullOrEmpty(queryText)) { - SetStatus("No query text available for this plan"); + SetErrorStatus("No query text available for this plan"); return; } @@ -424,7 +428,8 @@ private async void GetActualPlan_Click(object? sender, RoutedEventArgs e) } catch (OperationCanceledException) { - SetStatus("Cancelled"); + // Same as the capture path above: the cancel was the user's own, and the tab going + // away says so. See that catch for why the message is gone. SubTabControl.Items.Remove(loadingTab); } catch (SqlException ex) diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Format.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Format.cs index 577504d0..da1be44c 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Format.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Format.cs @@ -35,7 +35,7 @@ private async void CopyRepro_Click(object? sender, RoutedEventArgs e) var viewer = GetSelectedPlanViewer(); if (viewer == null) { - SetStatus("Select a plan tab first"); + SetErrorStatus("Select a plan tab first"); return; } @@ -44,7 +44,7 @@ private async void CopyRepro_Click(object? sender, RoutedEventArgs e) if (string.IsNullOrEmpty(queryText) && string.IsNullOrEmpty(planXml)) { - SetStatus("No query or plan data available"); + SetErrorStatus("No query or plan data available"); return; } @@ -63,7 +63,7 @@ otherwise fall back to the currently selected database */ if (await ClipboardHelper.TrySetTextAsync(this, reproScript)) SetStatus("Repro script copied to clipboard"); else - SetStatus("Clipboard busy - could not copy repro script"); + SetErrorStatus("Clipboard busy - could not copy repro script"); } private async void Format_Click(object? sender, RoutedEventArgs e) @@ -115,7 +115,7 @@ private async void Format_Click(object? sender, RoutedEventArgs e) } }; await dialog.ShowDialog(GetParentWindow()); - SetStatus($"Format failed: {errors.Count} error(s)"); + SetErrorStatus($"Format failed: {errors.Count} error(s)"); return; } @@ -137,7 +137,7 @@ private async void Format_Click(object? sender, RoutedEventArgs e) catch (Exception ex) { // async void handler: an unhandled throw here would crash the app. - SetStatus($"Format failed: {ex.Message}"); + SetStatusFromException(ex, "Format failed: "); } finally { diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs index d6ce988b..1ce03262 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs @@ -50,7 +50,7 @@ private bool AddPlanTab(string planXml, string queryText, bool estimated, string // Blank XML or a parse failure. Don't navigate away from the current view // (e.g. the Query Store grid) to a blank tab — surface why and stay put. viewer.OpenInEditorRequested -= OnOpenInEditorRequested; - SetStatus($"Couldn't load {label}: {viewer.LastLoadError}", autoClear: false); + SetErrorStatus($"Couldn't load {label}: {viewer.LastLoadError}"); return false; } @@ -287,7 +287,7 @@ is the whole complaint. */ var planTabs = GetPlanTabs().ToList(); if (planTabs.Count < 2) { - SetStatus("Need at least 2 plans open to compare"); + SetErrorStatus("Need at least 2 plans open to compare"); return; } diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.QueryStore.cs b/src/PlanViewer.App/Controls/QuerySessionControl.QueryStore.cs index ff786cb6..60c99c77 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.QueryStore.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.QueryStore.cs @@ -108,8 +108,6 @@ private async void QueryStoreOverview_Click(object? sender, RoutedEventArgs e) return; } - SetStatus("Loading Query Store Overview..."); - var supportsWaitStats = _serverMetadata?.SupportsQueryStoreWaitStats ?? false; var overview = new QueryStoreOverviewControl(_serverConnection, _credentialService, supportsWaitStats: supportsWaitStats); @@ -125,14 +123,18 @@ private async void QueryStoreOverview_Click(object? sender, RoutedEventArgs e) SubTabControl.Items.Add(tab); SubTabControl.SelectedItem = tab; + /* After the tab is selected, not before: selecting a sub-tab clears the strip, so a + "loading" message set ahead of the switch would be wiped by its own tab arriving. */ + SetStatus("Loading Query Store Overview..."); + try { await overview.LoadAsync(); - SetStatus(""); + ClearStatus(); } catch (Exception ex) { - SetStatus(ex.Message, autoClear: false); + SetStatusFromException(ex); } } @@ -147,7 +149,7 @@ private async Task OpenQueryStoreForDatabaseAsync(string database, DateTime? ini var (enabled, state, readOnlyReplica) = await QueryStoreService.CheckEnabledAsync(connStr); if (!enabled) { - SetStatus(readOnlyReplica + SetErrorStatus(readOnlyReplica ? $"{database} is a read-only replica with no Query Store data ({state ?? "unknown"}); enable it on the primary" : $"Query Store not enabled on {database} ({state ?? "unknown"})"); return; @@ -155,11 +157,11 @@ private async Task OpenQueryStoreForDatabaseAsync(string database, DateTime? ini } catch (Exception ex) { - SetStatus(ex.Message, autoClear: false); + SetStatusFromException(ex); return; } - SetStatus(""); + ClearStatus(); // Check if wait stats are supported var supportsWaitStats = _serverMetadata?.SupportsQueryStoreWaitStats ?? false; @@ -208,7 +210,7 @@ private async void QueryStore_Click(object? sender, RoutedEventArgs e) var (enabled, state, readOnlyReplica) = await QueryStoreService.CheckEnabledAsync(_connectionString); if (!enabled) { - SetStatus(readOnlyReplica + SetErrorStatus(readOnlyReplica ? $"Read-only replica with no Query Store data ({state ?? "unknown"}); enable it on the primary" : $"Query Store not enabled ({state ?? "unknown"})"); return; @@ -221,11 +223,11 @@ private async void QueryStore_Click(object? sender, RoutedEventArgs e) the actual available width; doing it again in code just threw away text the control would have kept, and with it the tooltip that now carries the full message. Same family as #448. */ - SetStatus(ex.Message, autoClear: false); + SetStatusFromException(ex); return; } - SetStatus(""); + ClearStatus(); // Check if wait stats are supported (SQL 2017+ / Azure) and capture is enabled var supportsWaitStats = _serverMetadata?.SupportsQueryStoreWaitStats ?? false; diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs index 9682cda9..2ad6b701 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs @@ -72,7 +72,7 @@ private async Task ShowSchemaInfoAsync(SchemaInfoKind kind) } catch (Exception ex) { - SetStatus($"Error: {ex.Message}", autoClear: false); + SetStatusFromException(ex, "Error: "); Debug.WriteLine($"Schema lookup error: {ex}"); } } diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs b/src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs index 17db3009..f1830553 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs @@ -151,14 +151,16 @@ public QuerySessionControl(ICredentialService credentialService, ConnectionStore }; // Dispose TextMate when detached (e.g. tab switch) to release renderers/transformers. - // Also cancel any in-flight status-clear dispatch so it doesn't fire on a dead control. + /* Emptying the strip cancels the in-flight status-clear dispatch — it must not fire on a + dead control — and, since the timer is what would have taken the message down, it is + also the only thing that can: a session detaches when the user switches to another + top-level tab, and whatever the strip was saying would otherwise be waiting, timer + cancelled and therefore forever, when they came back. */ DetachedFromVisualTree += (_, _) => { _textMateInstallation?.Dispose(); _textMateInstallation = null; - _statusClearCts?.Cancel(); - _statusClearCts?.Dispose(); - _statusClearCts = null; + ClearStatus(); }; /* #447: a plan appearing in — or leaving — this session changes whether Compare Plans is @@ -177,6 +179,11 @@ public QuerySessionControl(ICredentialService credentialService, ConnectionStore QueryEditor.TextArea.Focus(); } UpdatePlanTabButtonState(); + + /* The strip sits above the sub-tabs and says nothing about which one it is talking + about, so a message that outlives its view reads as a complaint about the view the + user moved to. Whatever it was saying was about the view they just left. */ + ClearStatus(); }; } diff --git a/src/PlanViewer.App/Themes/DarkTheme.axaml b/src/PlanViewer.App/Themes/DarkTheme.axaml index 460f4d20..a99d69e0 100644 --- a/src/PlanViewer.App/Themes/DarkTheme.axaml +++ b/src/PlanViewer.App/Themes/DarkTheme.axaml @@ -8,6 +8,8 @@ + + #2eaef1 From 48145089e990cdcbdea84dcbaf80f008a9b57eb2 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:39:01 -0400 Subject: [PATCH 011/119] Disable Compare Plans when there is no second plan to compare MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Compare Plans button in the toolbar over every window-level plan tab was built enabled and never re-decided. With one plan open, clicking it ran ShowCompareDialog's early return — no dialog, no message, no cursor change, nothing — which reads as a broken feature rather than as a missing precondition. The query session's copy of the button had been taught to count plans window-wide in #447; this one was left out of that fix. Both are now driven from the same count. RefreshComparePlanAvailability — already called by the tab watcher whenever a tab, sub-tab, or tab's content changes anywhere in the window — walks plan tabs as well as sessions, and ComparePlansButtonState applies the answer to either shape, including the tooltip: "Open a second plan to compare" when it is disabled, the old "Compare any two plans open in this window" when it is not. A disabled control that does not say what would enable it is only half an improvement over one that silently does nothing. Details worth naming: - The code-built button carries a Name so the refresh can find it again; this toolbar is rebuilt per plan tab, so there is no field to hold. - It is also born with the honest answer, because LoadPlanFile builds the toolbar before adding the tab that would trigger a refresh. - Detached plan windows are refreshed too: their button still opens the main window's picker, which cannot see the plan that left, so a window detached while a pair existed used to keep an enabled button afterwards. Detached query sessions stay out of that loop on purpose — #447 has them answer from their own plans, since their button falls back to their own picker. - The early return stays as a guard and is now unreachable from the UI, which the comment there says. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Controls/QuerySessionControl.Plans.cs | 3 +- .../Controls/QuerySessionControl.axaml | 5 +- .../Helpers/ComparePlansButtonState.cs | 41 +++++++++++ src/PlanViewer.App/MainWindow.PlanViewer.cs | 16 ++++- src/PlanViewer.App/MainWindow.Tabs.cs | 6 ++ src/PlanViewer.App/MainWindow.axaml.cs | 45 +++++++++++- .../ComparePlansAvailabilityTests.cs | 69 +++++++++++++++++++ 7 files changed, 179 insertions(+), 6 deletions(-) create mode 100644 src/PlanViewer.App/Helpers/ComparePlansButtonState.cs diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs index 1ce03262..332df66a 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs @@ -251,7 +251,8 @@ Fall back to what this session can see rather than leaving the button in a stale SetCompareAvailability(CountOwnPlans() >= 2); } - internal void SetCompareAvailability(bool enabled) => ComparePlansButton.IsEnabled = enabled; + internal void SetCompareAvailability(bool enabled) => + Helpers.ComparePlansButtonState.Apply(ComparePlansButton, enabled); private int CountOwnPlans() { diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.axaml b/src/PlanViewer.App/Controls/QuerySessionControl.axaml index 0c40f54a..a597b6dc 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.axaml +++ b/src/PlanViewer.App/Controls/QuerySessionControl.axaml @@ -57,11 +57,14 @@ ToolTip.Tip="JSON analysis for LLMs and automation"/> + +internal static class ComparePlansButtonState +{ + /// Marks a code-built Compare button so the window can find it again to refresh. + internal const string Name = "ComparePlansButton"; + + internal const string EnabledTip = "Compare any two plans open in this window"; + + /// + /// What the button says when it cannot be clicked. The disabled state is only informative if + /// it names the thing that would fix it. + /// + internal const string DisabledTip = "Open a second plan to compare"; + + /// + /// Enables or disables one Compare button, and tells it why. + /// + internal static void Apply(Button button, bool comparable) + { + button.IsEnabled = comparable; + ToolTip.SetTip(button, comparable ? EnabledTip : DisabledTip); + } +} diff --git a/src/PlanViewer.App/MainWindow.PlanViewer.cs b/src/PlanViewer.App/MainWindow.PlanViewer.cs index a2166196..e7d63ab7 100644 --- a/src/PlanViewer.App/MainWindow.PlanViewer.cs +++ b/src/PlanViewer.App/MainWindow.PlanViewer.cs @@ -91,6 +91,11 @@ depth ceiling and the same guard. */ var compareBtn = new Button { + /* Named so RefreshComparePlanAvailability can find it again. This toolbar is built + fresh for every plan tab and for every detached plan window, so there is no field + to hold them in, and whether Compare is available is a fact about the whole + window that changes long after the button was made. */ + Name = Helpers.ComparePlansButtonState.Name, Content = "\u2194 Compare Plans", Height = 28, Padding = new Avalonia.Thickness(10, 0), @@ -101,6 +106,12 @@ depth ceiling and the same guard. */ Theme = (Avalonia.Styling.ControlTheme)this.FindResource("AppButton")! }; + /* Born with the honest answer rather than enabled: this toolbar is often built by the + very call that makes the second plan exist (LoadPlanFile assigns the tab's content + after this returns), so the watcher's refresh may have already run for a window that + did not yet contain this plan. */ + Helpers.ComparePlansButtonState.Apply(compareBtn, CollectAllPlanTabs().Count >= 2); + compareBtn.Click += (_, _) => ShowCompareDialog(); var separator1 = new TextBlock @@ -231,7 +242,10 @@ internal void ShowCompareDialog() var planTabs = CollectAllPlanTabs(); if (planTabs.Count < 2) { - // Not enough plans to compare + /* Belt and braces: every Compare button in the window is disabled while this is + true (see RefreshComparePlanAvailability), so no click can reach here. It used to + be the only thing standing between a click and a dialog, which is exactly how the + button came to answer a click with nothing at all. */ return; } diff --git a/src/PlanViewer.App/MainWindow.Tabs.cs b/src/PlanViewer.App/MainWindow.Tabs.cs index 6201cdec..1c90a947 100644 --- a/src/PlanViewer.App/MainWindow.Tabs.cs +++ b/src/PlanViewer.App/MainWindow.Tabs.cs @@ -432,6 +432,12 @@ changes none — so the pre-detach state stuck until the next plan landed. Recom needs no twin call: adding the tab back fires MainWindow's collection watcher. */ if (content is QuerySessionControl detachedSession) detachedSession.UpdateCompareButtonState(); + else + /* A detached plan window keeps its own Compare button, still wired to THIS window's + picker — and the plan that just left is no longer in it, so the pair it was + offering may no longer exist. The watcher fired on the tab's removal, which was + before the detached register knew about this window. */ + RefreshComparePlanAvailability(); return detachedWindow; } diff --git a/src/PlanViewer.App/MainWindow.axaml.cs b/src/PlanViewer.App/MainWindow.axaml.cs index e6e0c895..5169eb60 100644 --- a/src/PlanViewer.App/MainWindow.axaml.cs +++ b/src/PlanViewer.App/MainWindow.axaml.cs @@ -858,21 +858,60 @@ private void About_Click(object? sender, RoutedEventArgs e) /// - /// Re-decides whether Compare Plans is offered, for every query session in the window (#447). + /// Re-decides whether Compare Plans is offered, for every Compare button in the window (#447). /// /// The button used to be enabled from a session's OWN plan count, so two queries in two /// separate sessions — one plan each — left it disabled in both, even though comparing them is /// exactly what it is for. The plans were always reachable: /// spans sessions and is what the file-mode Compare button has always used, which is why saving /// a plan and reopening it worked around this. + /// + /// The plan-tab toolbar's own Compare button was left out of that fix and was never + /// disabled at all: with one plan open it looked available, and clicking it did nothing + /// whatsoever — returns early and silently. It is refreshed + /// here too now, from the same count, so the two toolbars cannot disagree. /// internal void RefreshComparePlanAvailability() { var comparable = CollectAllPlanTabs().Count >= 2; + foreach (var item in MainTabControl.Items) + ApplyCompareAvailability((item as TabItem)?.Content as Control, comparable); + + /* Detached PLAN windows keep their toolbar, and the button on it still opens THIS + window's picker — which cannot see the detached plan itself — so the window-wide + count is the honest answer there too. Left out, a plan window detached while a pair + existed kept an enabled button after the pair stopped existing. + + Detached SESSIONS are deliberately not in this loop: their button falls back to their + own picker over their own plans, and QuerySessionControl.UpdateCompareButtonState + answers for them from that count (#447). */ + foreach (var content in _detachedTabContents.OfType()) + ApplyCompareAvailability(content, comparable); + } + + /// + /// Applies the window-wide answer to whatever Compare button one tab's content owns: a query + /// session has a named one in its own toolbar, and a plan tab has the code-built one from + /// . + /// + private static void ApplyCompareAvailability(Control? content, bool comparable) + { + if (content is QuerySessionControl session) { - if (item is TabItem { Content: QuerySessionControl session }) - session.SetCompareAvailability(comparable); + session.SetCompareAvailability(comparable); + return; + } + + if (content is not DockPanel dock) return; + + foreach (var toolbar in dock.Children.OfType()) + { + foreach (var button in toolbar.Children.OfType public string? ConnectionString { get; set; } + /// + /// Whether this viewer is living as a sub-tab inside a query session, rather than as a + /// top-level tab of its own. + /// + /// Hosted, it drops the connection half of its toolbar — Reconnect, the server label + /// and the Database picker — because the session's toolbar is one row above showing the same + /// connection, and its own picker was permanently disabled there anyway: a plan inside a + /// session inherits from the session (see + /// QuerySessionControl.AddPlanTab), and never populates a database list of its own. Two + /// stacked toolbars, three of the controls duplicated, one of them dead. + /// + /// Everything plan-scoped stays: zoom, Fit, the zoom readout, Save .sqlplan and + /// Statements. And schema lookups keep working, since they read ConnectionString rather than + /// the controls. + /// + public bool HostedInSession + { + get => _hostedInSession; + set + { + _hostedInSession = value; + PlanConnectionControls.IsVisible = !value; + } + } + + private bool _hostedInSession; + // Connection state for plans that connect via the toolbar private ServerConnection? _planConnection; private ICredentialService? _planCredentialService; diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs index 3af54b59..a15dbd0a 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs @@ -236,6 +236,8 @@ now empties the strip. */ internal void ShowCapturedPlan(TabItem planTab, string planXml, string tabLabel, string queryText) { var viewer = new PlanViewerControl(); + // Sub-tab of this session: the session's toolbar above it owns the connection (#U5). + viewer.HostedInSession = true; viewer.Metadata = _serverMetadata; viewer.ConnectionString = _connectionString; viewer.SetConnectionServices(_credentialService, _connectionStore); diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs index 332df66a..75c47410 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Plans.cs @@ -38,6 +38,8 @@ private bool AddPlanTab(string planXml, string queryText, bool estimated, string var label = labelOverride ?? (estimated ? $"Est Plan {_planCounter}" : $"Plan {_planCounter}"); var viewer = new PlanViewerControl(); + // Sub-tab of this session: the session's toolbar above it owns the connection (#U5). + viewer.HostedInSession = true; viewer.Metadata = _serverMetadata; viewer.ConnectionString = _connectionString; viewer.SetConnectionServices(_credentialService, _connectionStore); From 179f4c09103897ad1f73a7ad347b1a2305a33a7e Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:50:31 -0400 Subject: [PATCH 014/119] Offer the primary actions on an empty query tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A new Query tab was a wall of empty editor. Everything it can do sat behind a menu, and nothing on screen said so — the three things a user opens that tab for (open a plan, paste plan XML, connect to a server) were invisible from the place they are wanted. The editor tab now carries an empty state over the editor, centred and quiet: a "Get started" heading, those three actions with the shortcuts the File menu actually binds, and up to five recent plans with their folders. Every row is wired to the existing handler rather than a second copy of it — OpenFile_Click and PasteXml_Click (now internal, like NewQuery_Click already was), the session's own Connect_Click, and a new OpenRecentPlan extracted from the Recent Plans menu handler so a file that has been moved is reported and forgotten identically from both places. It cannot trap typing, which was the design constraint. The editor keeps focus underneath, a click anywhere on the panel hands focus back to it, and the panel is gone the moment the session holds anything: RefreshEmptyState shows it only while the editor is empty AND no sub-tab but Query Editor exists, and runs from the editor's TextChanged and the sub-tab watcher so it re-decides in both directions. Deleting the last character with nothing else open brings it back, which is the same state a fresh tab is in. Two things worth knowing for later: - Every clickable row sets Background="Transparent" itself. A control with a null background hit-tests only the glyphs it draws, so presses in the gaps between words would fall through. That local value outranks a style setter, so hover shows in the text colour rather than behind it. - The session is told its window by CreateTab rather than looking one up. FindLogicalAncestorOfType only answers once a session has been realised, and a tab that opens behind the selected one never is — its empty state would have been built with no recent plans and never rebuilt. Detaching unsets it, which also hides the two File actions that would have had no window to act on; redocking passes back through CreateTab. The File menu's two plan items gained x:Names so a test can hold their gestures against the shortcuts the panel prints, since that is a copy and copies drift. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../QuerySessionControl.EmptyState.cs | 166 +++++++++++++++++ .../Controls/QuerySessionControl.axaml | 109 +++++++++-- .../Controls/QuerySessionControl.axaml.cs | 34 +++- src/PlanViewer.App/MainWindow.FileOps.cs | 9 +- src/PlanViewer.App/MainWindow.RecentPlans.cs | 18 ++ src/PlanViewer.App/MainWindow.Tabs.cs | 14 ++ src/PlanViewer.App/MainWindow.axaml | 6 +- .../EmptyQueryTabStateTests.cs | 174 ++++++++++++++++++ 8 files changed, 507 insertions(+), 23 deletions(-) create mode 100644 src/PlanViewer.App/Controls/QuerySessionControl.EmptyState.cs create mode 100644 tests/PlanViewer.Core.Tests/EmptyQueryTabStateTests.cs diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.EmptyState.cs b/src/PlanViewer.App/Controls/QuerySessionControl.EmptyState.cs new file mode 100644 index 00000000..7b3aeb02 --- /dev/null +++ b/src/PlanViewer.App/Controls/QuerySessionControl.EmptyState.cs @@ -0,0 +1,166 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Avalonia.Controls; +using Avalonia.Input; +using Avalonia.Interactivity; +using Avalonia.Layout; +using Avalonia.Media; + +namespace PlanViewer.App.Controls; + +public partial class QuerySessionControl : UserControl +{ + /// + /// How many recent plans the empty state offers. The File menu lists them all; this is a + /// starting point, not a second copy of that menu. + /// + private const int EmptyStateRecentPlanLimit = 5; + + /// + /// Decides whether the editor's empty state is showing, and rebuilds it when it is. + /// + /// Shown only when this session holds nothing at all: no text, and no sub-tab beyond + /// the Query Editor. Both halves matter — a session whose editor is empty because the user + /// is reading the plan they just ran must not have an overlay waiting behind that plan. + /// + /// Called from the editor's TextChanged and from the sub-tab watcher, so it re-decides + /// in both directions: delete every character with no plan open and the panel comes back, + /// which is the same state a fresh tab is in and deserves the same offer. + /// + private void RefreshEmptyState() + { + var empty = QueryEditor.Text.Length == 0 && SubTabControl.Items.Count <= 1; + + if (empty) + { + /* Rebuilt on the way in rather than once at construction: the recent list changes + while the app runs, and a session can sit empty across a dozen plans being + opened. At most five rows, and only while the buffer is empty. */ + EmptyStateFileActions.IsVisible = _owningWindow != null; + PopulateEmptyStateRecentPlans(_owningWindow); + } + + EmptyStateOverlay.IsVisible = empty; + } + + /// + /// The window this session belongs to, or null while it has none — a session detached into + /// its own window, where the two File menu actions below have nothing to act on. + /// + private MainWindow? _owningWindow; + + /// + /// Told to the session by , which every top-level session + /// passes through exactly when it gains a tab, and unset by a detach. + /// + /// Handed over rather than walked up to: the tree answer (FindLogicalAncestorOfType, + /// as uses) is only true once the session has been + /// realised, and a tab that opens behind the selected one never is — so the empty state on + /// the tab you have not looked at yet would be built with no recent plans in it and never + /// rebuilt. + /// + internal void SetOwningWindow(MainWindow? window) + { + _owningWindow = window; + RefreshEmptyState(); + } + + private void PopulateEmptyStateRecentPlans(MainWindow? owner) + { + EmptyStateRecentPlans.Children.Clear(); + + var recent = owner?.RecentPlans ?? (IReadOnlyList)Array.Empty(); + foreach (var path in recent.Take(EmptyStateRecentPlanLimit)) + EmptyStateRecentPlans.Children.Add(BuildRecentPlanRow(path)); + + EmptyStateRecentSection.IsVisible = EmptyStateRecentPlans.Children.Count > 0; + } + + /// + /// One clickable recent plan: file name over its folder, the full path on hover. + /// + private Border BuildRecentPlanRow(string path) + { + /* Size and colour come from the overlay's styles via these classes, not from a resource + lookup here: a row is built before this session has been attached to anything, and + FindResource on an unattached control hands back UnsetValue rather than a brush. */ + var fileName = new TextBlock + { + Text = Path.GetFileName(path), + Classes = { "label" }, + TextTrimming = TextTrimming.CharacterEllipsis + }; + + var directory = new TextBlock + { + Text = Path.GetDirectoryName(path) ?? "", + Classes = { "path" }, + TextTrimming = TextTrimming.CharacterEllipsis + }; + + var row = new Border + { + /* The same hit-testing reason the XAML rows give: without a background this answers + clicks on its glyphs only. The Classes entry supplies the hover and the padding + from the overlay's styles. */ + Background = Brushes.Transparent, + Classes = { "action" }, + Tag = path, + Child = new StackPanel + { + Orientation = Orientation.Vertical, + Children = { fileName, directory } + } + }; + + ToolTip.SetTip(row, path); + row.PointerPressed += EmptyStateRecentPlan_PointerPressed; + + return row; + } + + /// + /// A click on the panel itself, rather than on one of its rows, puts the caret back where + /// the user expects it. The editor is underneath and usually already has focus — this is + /// for the case where something else took it. + /// + private void EmptyStateBackground_PointerPressed(object? sender, PointerPressedEventArgs e) + { + FocusEditor(); + } + + private void EmptyStateOpenPlan_PointerPressed(object? sender, PointerPressedEventArgs e) + { + e.Handled = true; // else this bubbles to the background handler above + _owningWindow?.OpenFile_Click(this, new RoutedEventArgs()); + } + + private void EmptyStatePastePlan_PointerPressed(object? sender, PointerPressedEventArgs e) + { + e.Handled = true; + _owningWindow?.PasteXml_Click(this, new RoutedEventArgs()); + } + + private void EmptyStateConnect_PointerPressed(object? sender, PointerPressedEventArgs e) + { + e.Handled = true; + // The toolbar's own Connect handler, so the two entry points cannot diverge. + Connect_Click(this, new RoutedEventArgs()); + } + + private void EmptyStateRecentPlan_PointerPressed(object? sender, PointerPressedEventArgs e) + { + e.Handled = true; + + if (sender is Border { Tag: string path }) + _owningWindow?.OpenRecentPlan(path); + } + + private void FocusEditor() + { + QueryEditor.Focus(); + QueryEditor.TextArea.Focus(); + } +} diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.axaml b/src/PlanViewer.App/Controls/QuerySessionControl.axaml index ffb5d6f3..76aeeb6a 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.axaml +++ b/src/PlanViewer.App/Controls/QuerySessionControl.axaml @@ -126,17 +126,104 @@ - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs b/src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs index f1830553..ebe41edc 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs @@ -137,7 +137,13 @@ public QuerySessionControl(ICredentialService credentialService, ConnectionStore // #462: every edit is a chance for the tab's modified marker to change, in both // directions — an undo back to the saved text clears it again. - QueryEditor.TextChanged += (_, _) => DirtyStateChanged?.Invoke(this, EventArgs.Empty); + QueryEditor.TextChanged += (_, _) => + { + DirtyStateChanged?.Invoke(this, EventArgs.Empty); + // The first keystroke is what takes the empty state away, and deleting the last + // one is what brings it back. + RefreshEmptyState(); + }; // Focus the editor when the control is attached to the visual tree // Re-install TextMate if it was disposed on detach (tab switching disposes it) @@ -146,8 +152,11 @@ public QuerySessionControl(ICredentialService credentialService, ConnectionStore if (_textMateInstallation == null) SetupSyntaxHighlighting(); - QueryEditor.Focus(); - QueryEditor.TextArea.Focus(); + FocusEditor(); + + /* The empty state's recent plans come off the owning window, which a session built + moments ago cannot see yet. Attaching is when it can. */ + RefreshEmptyState(); }; // Dispose TextMate when detached (e.g. tab switch) to release renderers/transformers. @@ -168,16 +177,19 @@ public QuerySessionControl(ICredentialService credentialService, ConnectionStore called at each site that produces a plan, because the sites that produce a plan are the ones nobody remembers: executing a query fills in a tab that already exists, which is neither an Add nor a Remove and is exactly the case the first fix missed. */ - TabContentWatcher.Watch(SubTabControl, UpdateCompareButtonState); + TabContentWatcher.Watch(SubTabControl, () => + { + UpdateCompareButtonState(); + // A plan, Query Store grid or schema tab opening or closing decides the other half + // of whether this session is empty. + RefreshEmptyState(); + }); // Focus the editor when the Editor tab is selected; toggle plan-dependent buttons SubTabControl.SelectionChanged += (_, _) => { if (SubTabControl.SelectedIndex == 0) - { - QueryEditor.Focus(); - QueryEditor.TextArea.Focus(); - } + FocusEditor(); UpdatePlanTabButtonState(); /* The strip sits above the sub-tabs and says nothing about which one it is talking @@ -185,6 +197,12 @@ public QuerySessionControl(ICredentialService credentialService, ConnectionStore user moved to. Whatever it was saying was about the view they just left. */ ClearStatus(); }; + + /* A brand new session is the empty state's whole reason for existing, and neither the + watcher (which only reports changes) nor a keystroke (there has been none) would say + so. The attach above refreshes it again once there is a window to read recent plans + from. */ + RefreshEmptyState(); } diff --git a/src/PlanViewer.App/MainWindow.FileOps.cs b/src/PlanViewer.App/MainWindow.FileOps.cs index 4ecb5ce2..9a3d864f 100644 --- a/src/PlanViewer.App/MainWindow.FileOps.cs +++ b/src/PlanViewer.App/MainWindow.FileOps.cs @@ -40,7 +40,11 @@ internal void NewQuery_Click(object? sender, RoutedEventArgs e) UpdateEmptyOverlay(); } - private async void OpenFile_Click(object? sender, RoutedEventArgs e) + /// + /// Internal, like : the empty state on a fresh query tab offers + /// this action and calls the menu's own handler rather than growing a second copy of it. + /// + internal async void OpenFile_Click(object? sender, RoutedEventArgs e) { var storage = StorageProvider; var files = await storage.OpenFilePickerAsync(new FilePickerOpenOptions @@ -220,7 +224,8 @@ same way. */ } } - private async void PasteXml_Click(object? sender, RoutedEventArgs e) + /// Internal for the same reason as . + internal async void PasteXml_Click(object? sender, RoutedEventArgs e) { await PasteXmlAsync(); } diff --git a/src/PlanViewer.App/MainWindow.RecentPlans.cs b/src/PlanViewer.App/MainWindow.RecentPlans.cs index 6c288725..6f53c92f 100644 --- a/src/PlanViewer.App/MainWindow.RecentPlans.cs +++ b/src/PlanViewer.App/MainWindow.RecentPlans.cs @@ -27,6 +27,12 @@ namespace PlanViewer.App; public partial class MainWindow : Window { + /// + /// The recent plan paths, most recent first — what the File menu lists, and what a query + /// session's empty state offers as a way back into the last few plans. + /// + internal IReadOnlyList RecentPlans => _appSettings.RecentPlans; + /// /// Adds a file path to the recent plans list, saves settings, and rebuilds the menu. /// @@ -88,6 +94,18 @@ private void RecentPlanItem_Click(object? sender, RoutedEventArgs e) if (sender is not MenuItem item || item.Tag is not string path) return; + OpenRecentPlan(path); + } + + /// + /// Opens one remembered plan, or explains why it cannot and forgets it. + /// + /// Split out of the menu handler because the empty state on a fresh query tab offers + /// the same list: a file that has been moved or deleted has to be reported and dropped there + /// too, not silently do nothing. + /// + internal void OpenRecentPlan(string path) + { if (!File.Exists(path)) { // File was moved or deleted — remove from the list and notify the user diff --git a/src/PlanViewer.App/MainWindow.Tabs.cs b/src/PlanViewer.App/MainWindow.Tabs.cs index 1c90a947..cd75a37c 100644 --- a/src/PlanViewer.App/MainWindow.Tabs.cs +++ b/src/PlanViewer.App/MainWindow.Tabs.cs @@ -115,6 +115,12 @@ The pointer handlers below live and die with the button itself. */ persistence — same single-subscription argument as the #495 tab watcher. Idempotent, because redock passes the same living session through again. */ HookScratchPersistence(querySession); + + /* And, for the same "every session, exactly here" reason, the moment it gains a + window. Its empty state lists this window's recent plans and runs this window's + File actions, and it cannot find the window by looking: a tab that opens behind + the selected one is never realised, so there is no tree to walk up. */ + querySession.SetOwningWindow(this); } // Middle-click to close @@ -431,13 +437,21 @@ changes none — so the pre-detach state stuck until the next plan landed. Recom session's plans, which inside a detached window is the only honest answer. Redock needs no twin call: adding the tab back fires MainWindow's collection watcher. */ if (content is QuerySessionControl detachedSession) + { detachedSession.UpdateCompareButtonState(); + + /* Off the tab strip, its empty state has no window to open files into. Redock hands + it back through CreateTab. */ + detachedSession.SetOwningWindow(null); + } else + { /* A detached plan window keeps its own Compare button, still wired to THIS window's picker — and the plan that just left is no longer in it, so the pair it was offering may no longer exist. The watcher fired on the tab's removal, which was before the detached register knew about this window. */ RefreshComparePlanAvailability(); + } return detachedWindow; } diff --git a/src/PlanViewer.App/MainWindow.axaml b/src/PlanViewer.App/MainWindow.axaml index fc63ed8e..ac6c6b5d 100644 --- a/src/PlanViewer.App/MainWindow.axaml +++ b/src/PlanViewer.App/MainWindow.axaml @@ -21,9 +21,11 @@ - + - diff --git a/tests/PlanViewer.Core.Tests/EmptyQueryTabStateTests.cs b/tests/PlanViewer.Core.Tests/EmptyQueryTabStateTests.cs new file mode 100644 index 00000000..84a27835 --- /dev/null +++ b/tests/PlanViewer.Core.Tests/EmptyQueryTabStateTests.cs @@ -0,0 +1,174 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Avalonia.Controls; +using Avalonia.Interactivity; +using Avalonia.LogicalTree; +using PlanViewer.App; +using PlanViewer.App.Controls; +using PlanViewer.Core.Models; + +namespace PlanViewer.Core.Tests; + +/// +/// A new Query tab used to be a wall of empty editor: no sign of what the app can do, and the +/// three things a user actually wants there — open a plan, paste plan XML, connect — reachable +/// only from a menu they had no reason to look in. The empty state offers them, and gets out of +/// the way the instant the session holds anything. +/// +/// What is pinned here is the appearing and disappearing, because that is what can trap +/// typing if it is wrong, plus the two things that rot quietly: the hit-testing of clickable +/// text (a null background answers clicks on glyphs only — the trap this repo has fallen into +/// before), and the printed shortcuts, which are a second copy of the File menu's gestures. +/// +public class EmptyQueryTabStateTests +{ + [Fact] + public void AFreshQueryTabShowsTheEmptyState_AndTypingTakesItAway() + { + HeadlessUi.Run(() => + { + var window = new MainWindow(); + var session = NewSession(window); + + Assert.True(Overlay(session).IsVisible, "nothing typed, no sub-tabs — a fresh tab"); + + session.QueryEditor.Text = "select 1;"; + Assert.False(Overlay(session).IsVisible, "the editor is in use"); + + session.QueryEditor.Text = ""; + Assert.True(Overlay(session).IsVisible, + "a buffer emptied back out is the same state a fresh tab is in"); + }); + } + + /// + /// The half that is easy to forget: an empty editor is not an empty session. Run a query, + /// read the plan, come back to the editor tab having typed nothing — an overlay waiting + /// there would be offering to get started on a session that already has. + /// + [Fact] + public void AnOpenPlanKeepsTheEmptyStateAwayEvenWithAnEmptyEditor() + { + HeadlessUi.Run(() => + { + var window = new MainWindow(); + var session = NewSession(window); + + session.OnQueryStorePlansSelected(null, new List + { + new() + { + QueryId = 1, + PlanId = 1, + QueryText = "select 1;", + PlanXml = PlanXml("row_goal_plan.sqlplan") + } + }); + + Assert.Equal("", session.QueryEditor.Text); + Assert.False(Overlay(session).IsVisible, "this session holds a plan"); + }); + } + + /// + /// Every row is clickable across its whole width, not just where its glyphs happen to fall. + /// A control with a null background hit-tests only what it draws, so a press in the gap + /// between two words would sail past the row and land on the panel behind it. + /// + [Fact] + public void EveryClickableRowHasABackgroundToBeHitOn() + { + HeadlessUi.Run(() => + { + var window = new MainWindow(); + window.LoadPlanFile(PlanPath("row_goal_plan.sqlplan")); + + var session = NewSession(window); + var rows = ActionRows(session).ToList(); + + /* Open a plan, Paste plan XML, Connect to a server, and at least one recent plan — + the one loaded above. */ + Assert.True(rows.Count >= 4, $"expected the three actions and a recent plan, found {rows.Count}"); + Assert.All(rows, row => Assert.NotNull(row.Background)); + }); + } + + [Fact] + public void RecentPlansAreOfferedByPath_AndCappedAtFive() + { + HeadlessUi.Run(() => + { + var window = new MainWindow(); + + var path = PlanPath("key_lookup_plan.sqlplan"); + window.LoadPlanFile(path); + + var session = NewSession(window); + var recent = RecentPlanRows(session).ToList(); + + Assert.Contains(path, recent.Select(row => row.Tag as string)); + Assert.True(recent.Count <= 5, "the empty state is a starting point, not the whole File menu"); + }); + } + + /// + /// The shortcuts printed next to the two File menu actions are a copy of that menu's + /// gestures, and a copy drifts. Changing either gesture should fail here rather than leave + /// the empty state quietly teaching the wrong keys. + /// + [Fact] + public void ThePrintedShortcutsAreTheOnesTheFileMenuBinds() + { + HeadlessUi.Run(() => + { + var window = new MainWindow(); + var session = NewSession(window); + + var printed = session.FindControl("EmptyStateFileActions")! + .GetLogicalDescendants() + .OfType() + .Where(text => text.Classes.Contains("gesture")) + .Select(text => text.Text) + .ToList(); + + Assert.Equal( + new[] + { + window.FindControl("OpenPlanMenuItem")!.InputGesture!.ToString(), + window.FindControl("PastePlanXmlMenuItem")!.InputGesture!.ToString() + }, + printed); + }); + } + + private static QuerySessionControl NewSession(MainWindow window) + { + window.NewQuery_Click(window, new RoutedEventArgs()); + return window.FindControl("MainTabControl")!.Items + .OfType() + .Select(tab => tab.Content) + .OfType() + .Last(); + } + + private static Border Overlay(QuerySessionControl session) => + session.FindControl("EmptyStateOverlay")!; + + /// Every clickable row in the panel: the three actions, then the recent plans. + private static IEnumerable ActionRows(QuerySessionControl session) => + Overlay(session).GetLogicalDescendants() + .OfType() + .Where(border => border.Classes.Contains("action")); + + private static IEnumerable RecentPlanRows(QuerySessionControl session) => + session.FindControl("EmptyStateRecentPlans")!.Children.OfType(); + + private static string PlanPath(string name) => + Path.Combine(System.AppContext.BaseDirectory, "Plans", name); + + /// SSMS writes plan files as UTF-16 and declares it; the parser wants the declaration + /// to match what it is handed. Same substitution the other plan-loading tests make. + private static string PlanXml(string name) => + File.ReadAllText(PlanPath(name)).Replace("encoding=\"utf-16\"", "encoding=\"utf-8\""); +} From da1c3108ccafa9f00e535125ed5ed3059edd90ff Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:32:24 -0400 Subject: [PATCH 015/119] Stop the Settings dialog from starting out dirty Opening Settings and clicking Cancel always asked "You have unsaved changes. Discard them?" even when nothing was touched. ShowSection builds each section's controls and subscribes dirty handlers to ValueChanged/SelectionChanged/TextChanged/PropertyChanged. Those events also fire while the controls initialize themselves: NumericUpDown syncs text and value when it is templated, and the format DataGrid's two-way cell bindings write back into FormatOptionRow as rows are realized. Both happen on the layout pass after ShowSection returns, so the dialog was dirty before the user did anything. Add a _building guard that the handlers (now routed through MarkDirty) honor. It is set around the section build and released by a Background priority dispatcher post, once templating and binding have settled; a build token keeps an older section's post from releasing a newer one. Reset Section and Reset All still set _isDirty explicitly, so cancelling after a reset still prompts. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Dialogs/SettingsWindow.axaml.cs | 54 +++++++++++++++---- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs b/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs index 420746fc..9f0e0ff1 100644 --- a/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs +++ b/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs @@ -14,6 +14,7 @@ using Avalonia.Interactivity; using Avalonia.Layout; using Avalonia.Media; +using Avalonia.Threading; using PlanViewer.App.Services; using PlanViewer.Core.Services; @@ -24,6 +25,18 @@ internal partial class SettingsWindow : Window private AppSettings _settings; private bool _isDirty; + /// + /// Set while a section is being built and initialized. Giving a control its starting value + /// raises ValueChanged/SelectionChanged/TextChanged/PropertyChanged - some synchronously, + /// some later when the control is templated and its bindings run on the layout pass - so the + /// dirty handlers no-op while this is set. Without it, merely opening Settings looks like an + /// edit and Cancel always asks to discard changes. + /// + private bool _building; + + /// Identifies the most recent build so an older one cannot release the guard early. + private int _buildToken; + // QueryStore controls private NumericUpDown? _slicerDaysBox; private ComboBox? _defaultMetricBox; @@ -70,6 +83,9 @@ private void SectionList_SelectionChanged(object? sender, SelectionChangedEventA private void ShowSection(int index) { + var token = ++_buildToken; + _building = true; + DetailPanel.Content = index switch { 0 => BuildQueryStoreSection(), @@ -77,6 +93,22 @@ private void ShowSection(int index) 2 => BuildScriptOptionsSection(), _ => null }; + + // Controls keep initializing after this returns: templates are applied and cell bindings + // (the format DataGrid writes back through them) run on the layout pass. Release the guard + // once the UI has settled, so only real user edits mark the dialog dirty. + Dispatcher.UIThread.Post(() => + { + if (_buildToken == token) + _building = false; + }, DispatcherPriority.Background); + } + + /// Marks the dialog dirty unless a section is still being built and initialized. + private void MarkDirty() + { + if (!_building) + _isDirty = true; } // ── Query Store Section ────────────────────────────────────────── @@ -116,27 +148,27 @@ private Control BuildQueryStoreSection() panel.Children.Add(CreateChapterHeader("Query Store")); _slicerDaysBox = CreateNumericUpDown(_settings.QueryStoreSlicerDays, 1, 365); - _slicerDaysBox.ValueChanged += (_, _) => _isDirty = true; + _slicerDaysBox.ValueChanged += (_, _) => MarkDirty(); panel.Children.Add(CreateRow("Default history length (days)", _slicerDaysBox)); _defaultMetricBox = CreateTagComboBox(MetricOptions, _settings.QueryStoreDefaultMetric); - _defaultMetricBox.SelectionChanged += (_, _) => _isDirty = true; + _defaultMetricBox.SelectionChanged += (_, _) => MarkDirty(); panel.Children.Add(CreateRow("Default metric for top", _defaultMetricBox)); _topLimitBox = CreateNumericUpDown(_settings.QueryStoreTopLimit, 1, 200); - _topLimitBox.ValueChanged += (_, _) => _isDirty = true; + _topLimitBox.ValueChanged += (_, _) => MarkDirty(); panel.Children.Add(CreateRow("Top elements limit", _topLimitBox)); _defaultTimeRangeBox = CreateTagComboBox(TimeRangeOptions, _settings.QueryStoreDefaultTimeRange); - _defaultTimeRangeBox.SelectionChanged += (_, _) => _isDirty = true; + _defaultTimeRangeBox.SelectionChanged += (_, _) => MarkDirty(); panel.Children.Add(CreateRow("Default time range", _defaultTimeRangeBox)); _defaultTimeDisplayBox = CreateTagComboBox(TimeDisplayOptions, _settings.QueryStoreDefaultTimeDisplay); - _defaultTimeDisplayBox.SelectionChanged += (_, _) => _isDirty = true; + _defaultTimeDisplayBox.SelectionChanged += (_, _) => MarkDirty(); panel.Children.Add(CreateRow("Default time display", _defaultTimeDisplayBox)); _defaultGroupByBox = CreateTagComboBox(GroupByOptions, _settings.QueryStoreDefaultGroupBy); - _defaultGroupByBox.SelectionChanged += (_, _) => _isDirty = true; + _defaultGroupByBox.SelectionChanged += (_, _) => MarkDirty(); panel.Children.Add(CreateRow("Default group by", _defaultGroupByBox)); // Chapter 2: Multi QS Overview @@ -145,7 +177,7 @@ private Control BuildQueryStoreSection() _topDbCountBox = CreateNumericUpDown(_settings.MultiQsTopDbCount, 2, 20); _topDbCountBox.ValueChanged += (_, e) => { - _isDirty = true; + MarkDirty(); RebuildColorList(); }; panel.Children.Add(CreateRow("Number of top databases", _topDbCountBox)); @@ -185,7 +217,7 @@ private void RebuildColorList() var index = i; textBox.TextChanged += (_, _) => { - _isDirty = true; + MarkDirty(); if (index < _colorPreviews.Count) _colorPreviews[index].Fill = TryParseBrush(textBox.Text ?? ""); }; @@ -247,11 +279,11 @@ private Control BuildQueryHistorySection() panel.Children.Add(CreateChapterHeader("Query History")); _historyMetricBox = CreateTagComboBox(HistoryMetricOptions, _settings.QueryHistoryDefaultMetric); - _historyMetricBox.SelectionChanged += (_, _) => _isDirty = true; + _historyMetricBox.SelectionChanged += (_, _) => MarkDirty(); panel.Children.Add(CreateRow("Default chart metric", _historyMetricBox)); _historyMaxPlansBox = CreateNumericUpDown(_settings.QueryHistoryMaxPlans, 1, 100); - _historyMaxPlansBox.ValueChanged += (_, _) => _isDirty = true; + _historyMaxPlansBox.ValueChanged += (_, _) => MarkDirty(); panel.Children.Add(CreateRow("Max plans fetched per query", _historyMaxPlansBox)); return panel; @@ -314,7 +346,7 @@ private Control BuildScriptOptionsSection() ChoiceOptions = choiceOptions, PropertyInfo = prop }; - row.PropertyChanged += (_, _) => _isDirty = true; + row.PropertyChanged += (_, _) => MarkDirty(); _formatRows.Add(row); } From fe86bedfc3248b9d489da7747549fee0d510a241 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:33:05 -0400 Subject: [PATCH 016/119] Show the Settings window as a modal dialog Settings was opened with Show(this), which parents it to the main window but leaves the main window fully interactive. With Settings open you could still drive the menu bar behind it, and the unsaved-changes prompt that Settings owns was drawn under a Help menu opened from the main window. Open it with ShowDialog(this) instead. The discard prompt inside SettingsWindow was already ShowDialog-ed onto SettingsWindow, so making the parent modal closes the whole interaction leak. Nothing needs Settings to be modeless - it only reports back through SettingsSaved, which fires on Save - and the existing single-instance guard stays as a belt-and-braces check. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- src/PlanViewer.App/MainWindow.axaml.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/PlanViewer.App/MainWindow.axaml.cs b/src/PlanViewer.App/MainWindow.axaml.cs index 5169eb60..52ab0431 100644 --- a/src/PlanViewer.App/MainWindow.axaml.cs +++ b/src/PlanViewer.App/MainWindow.axaml.cs @@ -827,7 +827,7 @@ private void Exit_Click(object? sender, RoutedEventArgs e) Close(); } - private void Settings_Click(object? sender, RoutedEventArgs e) + private async void Settings_Click(object? sender, RoutedEventArgs e) { if (_settingsWindow != null) { @@ -841,7 +841,10 @@ private void Settings_Click(object? sender, RoutedEventArgs e) _appSettings = settings; }; _settingsWindow.Closed += (_, _) => _settingsWindow = null; - _settingsWindow.Show(this); + + // Modal: Settings owns the interaction until it closes, so its own child prompts + // (the unsaved-changes discard dialog) cannot be talked over from this window. + await _settingsWindow.ShowDialog(this); } private void About_Click(object? sender, RoutedEventArgs e) From 3ca13c8e45aaa48532c98079eb0f159d5adf7f0f Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:33:45 -0400 Subject: [PATCH 017/119] Title the column filter popup with the column header The Query Store grid passed the CLR property name to the filter popup, so filtering the "Query Hash" column opened a popup headed "Filter: QueryHash", and "Module" read "Filter: ModuleName". SetColumnFilterButton already knows both the column id and the header label it renders, so record the label in _columnLabels and hand it to ColumnFilterPopup.Initialize as a separate display name. The column id still drives _activeFilters keying and RowMatchesAllFilters, so filter behaviour and the server-search promotion are unchanged. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- src/PlanViewer.App/Controls/ColumnFilterPopup.axaml.cs | 9 +++++++-- .../Controls/QueryStoreGridControl.Filters.cs | 6 +++++- .../Controls/QueryStoreGridControl.axaml.cs | 2 ++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/PlanViewer.App/Controls/ColumnFilterPopup.axaml.cs b/src/PlanViewer.App/Controls/ColumnFilterPopup.axaml.cs index dd5ced32..dfaec930 100644 --- a/src/PlanViewer.App/Controls/ColumnFilterPopup.axaml.cs +++ b/src/PlanViewer.App/Controls/ColumnFilterPopup.axaml.cs @@ -36,10 +36,15 @@ public ColumnFilterPopup() OperatorComboBox.SelectedIndex = 0; } - public void Initialize(string columnName, ColumnFilterState? existingFilter, bool canSearchServer) + /// + /// Prepares the popup for one column. is the internal column id + /// the filter is keyed and evaluated by; is the grid header the + /// user actually sees, and is all that is shown in the popup. + /// + public void Initialize(string columnName, string displayName, ColumnFilterState? existingFilter, bool canSearchServer) { _currentColumnName = columnName; - HeaderText.Text = $"Filter: {columnName}"; + HeaderText.Text = $"Filter: {(string.IsNullOrEmpty(displayName) ? columnName : displayName)}"; SearchServerButton.IsVisible = canSearchServer; if (existingFilter?.IsActive == true) diff --git a/src/PlanViewer.App/Controls/QueryStoreGridControl.Filters.cs b/src/PlanViewer.App/Controls/QueryStoreGridControl.Filters.cs index ea203b9a..d32ba096 100644 --- a/src/PlanViewer.App/Controls/QueryStoreGridControl.Filters.cs +++ b/src/PlanViewer.App/Controls/QueryStoreGridControl.Filters.cs @@ -153,6 +153,8 @@ private void SetupColumnHeaders() private void SetColumnFilterButton(DataGridColumn col, string columnId, string label) { + _columnLabels[columnId] = label; + var icon = new TextBlock { Text = "▽", @@ -215,7 +217,9 @@ private void ColumnFilter_Click(object? sender, RoutedEventArgs e) EnsureFilterPopup(); _activeFilters.TryGetValue(columnId, out var existing); var canSearchServer = MapColumnToServerKind(columnId) is not null; - _filterPopupContent!.Initialize(columnId, existing, canSearchServer); + // Title the popup with the grid header ("Query Hash"), not the property name ("QueryHash"). + var label = _columnLabels.TryGetValue(columnId, out var header) ? header : columnId; + _filterPopupContent!.Initialize(columnId, label, existing, canSearchServer); _filterPopup!.PlacementTarget = button; _filterPopup.IsOpen = true; } diff --git a/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs b/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs index 9525070b..7ac00c06 100644 --- a/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs +++ b/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs @@ -30,6 +30,8 @@ public partial class QueryStoreGridControl : UserControl private ObservableCollection _rows = new(); private ObservableCollection _filteredRows = new(); private readonly Dictionary _activeFilters = new(); + /// Column id → the header text shown in the grid, for user-facing filter labels. + private readonly Dictionary _columnLabels = new(); private Popup? _filterPopup; private ColumnFilterPopup? _filterPopupContent; private string? _sortedColumnTag; From ac968a5583e91f7774b579b53bd8734294aedc42 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:35:50 -0400 Subject: [PATCH 018/119] Make Connect a single step in the connection dialog Connect was disabled until Test Connection had been clicked, because only the test enumerated databases and enabled the button. The disabled button gave no reason, and in a live session the two-step order was not guessable - people filled the form, clicked Connect, and nothing happened. Connect is now enabled by the fields a connection actually needs: a server name, plus login and password when SQL Server authentication is selected. Clicking it runs the connect and database enumeration inline through the same ConnectAndLoadDatabasesAsync the test button uses, then saves the credentials and connection and closes with ResultDatabase set from the Database dropdown, the typed initial database, or master. Failures land in StatusText and the dialog stays open, so the error is attached to the attempt. Test Connection keeps its old behaviour as an optional way to browse databases first, and credentials are now only persisted once a connection has actually succeeded. The dialog stays editable while a connection opens, so Connect captures the connection, login, password and typed database it is about to validate before awaiting, and checks the dialog is still open afterwards. Without that, cancelling mid-connect still wrote a credential, and editing the server name mid-connect would save a server that was never tested. The handlers that XAML can raise during loading also guard the named fields they touch, since a selection set in XAML (EncryptBox already has one) fires before those fields exist. Callers that reconnect an already-connected session (QuerySessionControl and PlanViewerControl) pass their current database, which is pre-selected once the list loads so a reconnect returns to the database in use instead of dropping to master. The dialog still never connects on its own. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7 --- .../Controls/PlanViewerControl.axaml.cs | 3 +- .../QuerySessionControl.Connection.cs | 3 +- .../Dialogs/ConnectionDialog.axaml | 13 +- .../Dialogs/ConnectionDialog.axaml.cs | 147 +++++++++++++++--- 4 files changed, 140 insertions(+), 26 deletions(-) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs b/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs index 85b529cb..108a05a1 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs @@ -504,7 +504,8 @@ private async void PlanConnect_Click(object? sender, RoutedEventArgs e) { if (_planCredentialService == null || _planConnectionStore == null) return; - var dialog = new ConnectionDialog(_planCredentialService, _planConnectionStore); + // Pass the current database so a reconnect comes back to it rather than master. + var dialog = new ConnectionDialog(_planCredentialService, _planConnectionStore, _planSelectedDatabase); var topLevel = TopLevel.GetTopLevel(this); if (topLevel is not Window parentWindow) return; diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Connection.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Connection.cs index 85d4b710..5a2d379a 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Connection.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Connection.cs @@ -37,7 +37,8 @@ private async void Connect_Click(object? sender, RoutedEventArgs e) private async Task ShowConnectionDialogAsync() { - var dialog = new ConnectionDialog(_credentialService, _connectionStore); + // Pass the session's current database so a reconnect comes back to it rather than master. + var dialog = new ConnectionDialog(_credentialService, _connectionStore, _selectedDatabase); var result = await dialog.ShowDialog(GetParentWindow()); if (result == true && dialog.ResultConnection != null) diff --git a/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml b/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml index ce477f13..d6c02969 100644 --- a/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml +++ b/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml @@ -16,6 +16,7 @@