Plan viewer: a shared MetricFormatter for costs and durations

Darling/Darling.Tests/Viewer4577Tests.cs

@@ -0,0 +1,202 @@
+using System;
+using System.Globalization;
+using System.IO;
+using PerformanceMonitor.PlanAnalysis;
+using Xunit;
+
+namespace Darling.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.
+//
+// Ported from erikdarlingdata/PerformanceStudio@caa57ce's MetricFormatterTests.
+public class Viewer4577Tests
+{
+    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));
+    }
+
+    [Theory]
+    // Below a millisecond, where the whole-ms ladder would report a query that ran as one
+    // that did not. Two rungs: the number itself, then a floor for the vanishingly small.
+    [InlineData(0.4, "0.4ms")]
+    [InlineData(0.42, "0.42ms")]
+    [InlineData(0.425, "0.43ms")]
+    [InlineData(0.01, "0.01ms")]
+    [InlineData(0.009, "<0.01ms")]
+    [InlineData(0.0000001, "<0.01ms")]
+    // Zero is zero, and a duration is never negative - but if one ever arrives it reads as 0ms
+    // rather than as a fraction with a minus sign.
+    [InlineData(0.0, "0ms")]
+    [InlineData(-0.4, "0ms")]
+    // At a millisecond and above it is the same ladder, reached by rounding rather than truncating.
+    [InlineData(1.0, "1ms")]
+    [InlineData(1.6, "2ms")]
+    [InlineData(999.4, "999ms")]
+    [InlineData(1000.0, "1.0s")]
+    public void FormatDuration_HasTwoMoreRungsBelowAMillisecond(double ms, string expected)
+    {
+        Assert.Equal(expected, MetricFormatter.FormatDuration(ms, Invariant));
+    }
+
+    [Fact]
+    public void FormatDuration_AgreesWithItselfWhicheverOverloadIsCalled()
+    {
+        /* The Query Store Overview kept its own copy of the sub-millisecond rungs because this
+           helper had none. It delegates now, so the two must not drift: a whole number of
+           milliseconds reads the same whether it arrives as a long or as a double. */
+        foreach (var ms in new long[] { 0, 1, 7, 999, 1000, 59_900, 60_000, 1_235_000 })
+            Assert.Equal(
+                MetricFormatter.FormatDuration(ms, Invariant),
+                MetricFormatter.FormatDuration((double)ms, Invariant));
+    }
+
+    /// <summary>
+    /// Census pin: no cost display site in PerformanceMonitor.Ui still hand-formats with a raw
+    /// ":F6" or ":F4" numeric format string for a cost. MemoryFractionInput/Output keep their own
+    /// ":F4" (a different metric family PerformanceStudio leaves alone), so this only forbids
+    /// ":F6", which was exclusively the unrouted cost sites this issue replaces.
+    /// </summary>
+    [Fact]
+    public void NoRawCostFormatStringSurvivesInTheUi()
+    {
+        var uiDir = Path.Combine(RepoRoot(), "PerformanceMonitor.Ui");
+        Assert.True(Directory.Exists(uiDir), $"PerformanceMonitor.Ui not found at {uiDir}");
+
+        var offenders = new System.Collections.Generic.List<string>();
+        var filesScanned = 0;
+
+        foreach (var file in Directory.EnumerateFiles(uiDir, "*.cs", SearchOption.AllDirectories))
+        {
+            filesScanned++;
+            var text = File.ReadAllText(file);
+            if (text.Contains(":F6"))
+                offenders.Add(file);
+        }
+
+        /* Floor over the walk itself, same idiom as ViewerButtonAccessKeyTests.RepoRoot: a walk that
+           silently found nothing would satisfy an empty-offenders assertion for the wrong reason. */
+        Assert.True(filesScanned >= 5, $"only {filesScanned} .cs files found under {uiDir}; the walk is not finding the source");
+
+        Assert.True(offenders.Count == 0, "Raw \":F6\" cost formatting left in: " + string.Join(", ", offenders));
+    }
+
+    /// <summary>Same walk-up idiom as <c>ViewerButtonAccessKeyTests.RepoRoot</c>.</summary>
+    private static string RepoRoot()
+    {
+        var directory = new DirectoryInfo(AppContext.BaseDirectory);
+        for (var i = 0; i < 10 && directory is not null; i++)
+        {
+            if (File.Exists(Path.Combine(directory.FullName, "PerformanceMonitor.sln")))
+                return directory.FullName;
+
+            directory = directory.Parent;
+        }
+
+        throw new DirectoryNotFoundException(
+            "Could not locate the repository root (walked up from the test binary looking for "
+          + "PerformanceMonitor.sln). This test scans the source tree, so it cannot run without it.");
+    }
+}

PerformanceMonitor.PlanAnalysis/MetricFormatter.cs

@@ -0,0 +1,86 @@
+using System;
+using System.Globalization;
+
+namespace PerformanceMonitor.PlanAnalysis;
+
+/// <summary>
+/// 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 — 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.
+/// </summary>
+public static class MetricFormatter
+{
+    /// <summary>
+    /// 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".
+    /// </summary>
+    private const double SmallestShownCost = 0.00005;
+
+    /// <summary>
+    /// 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 "&lt;0.0001" rather than
+    /// "0", so a cost that exists never displays as free. Never scientific notation.
+    /// </summary>
+    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);
+    }
+
+    /// <summary>
+    /// The same ladder the statements grid already uses (under a second stays in
+    /// milliseconds like "847ms", under a minute scales to seconds with one decimal like
+    /// "3.5s", and anything longer splits into minutes and seconds like "20m 35s"), for a
+    /// duration that can be finer than a millisecond.
+    /// </summary>
+    /// <remarks>
+    /// The ladder below starts at whole milliseconds, so an average CPU of 0.4ms — an ordinary
+    /// number on a healthy OLTP database — rounds to "0ms" and reports a query that ran as one that
+    /// did not. Query Store keeps microseconds, so its numbers arrive here as fractions and get two
+    /// extra rungs below 1ms; anything already counted in whole milliseconds climbs exactly the
+    /// rungs it always did, because that is what the overload below hands it.
+    /// </remarks>
+    public static string FormatDuration(double ms, IFormatProvider? provider = null)
+    {
+        provider ??= CultureInfo.CurrentCulture;
+
+        if (ms > 0 && ms < 1)
+            return ms < 0.01 ? "<0.01ms" : ms.ToString("0.##", provider) + "ms";
+
+        return FormatDuration((long)Math.Round(ms), provider);
+    }
+
+    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";
+    }
+}

PerformanceMonitor.Ui/PlanViewerControl.Properties.cs

@@ -307,10 +307,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");

PerformanceMonitor.Ui/PlanViewerControl.Tooltips.cs

@@ -48,8 +48,8 @@ private ToolTip BuildNodeTooltip(PlanNode node, List<PlanWarning>? allWarnings =
 
         // 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");
@@ -66,8 +66,8 @@ private ToolTip BuildNodeTooltip(PlanNode node, List<PlanWarning>? allWarnings =
         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");
         }
 

PerformanceMonitor.Ui/PlanViewerControl.xaml.cs

@@ -396,15 +396,8 @@ public class StatementRow
     public PlanStatement Statement { get; set; } = null!;
 
     // Display helpers — grid binds to these, sorting uses the raw properties via SortMemberPath
-    public string CpuDisplay => FormatDuration(CpuMs);
-    public string ElapsedDisplay => FormatDuration(ElapsedMs);
-    public string UdfDisplay => UdfMs > 0 ? FormatDuration(UdfMs) : "";
+    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 ? $"{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";
-    }
 }