UX polish: advice, warnings, efficiency, node rendering

src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs

@@ -442,9 +442,9 @@ private Border CreateNodeVisual(PlanNode node, int totalWarningCount = -1)
             HorizontalAlignment = HorizontalAlignment.Center
         });
 
-        // Cost percentage
-        IBrush costColor = node.CostPercent >= 50 ? OrangeRedBrush
-            : node.CostPercent >= 25 ? OrangeBrush
+        // Cost percentage — only highlight in estimated plans; actual plans use duration/CPU colors
+        IBrush costColor = !node.HasActualStats && node.CostPercent >= 50 ? OrangeRedBrush
+            : !node.HasActualStats && node.CostPercent >= 25 ? OrangeBrush
             : fgBrush;
 
         stack.Children.Add(new TextBlock
@@ -499,7 +499,7 @@ private Border CreateNodeVisual(PlanNode node, int totalWarningCount = -1)
             stack.Children.Add(new TextBlock
             {
                 Text = $"{node.ActualRows:N0} of {estRows:N0}{accuracy}",
-                FontSize = 9,
+                FontSize = 10,
                 Foreground = rowBrush,
                 TextAlignment = TextAlignment.Center,
                 HorizontalAlignment = HorizontalAlignment.Center,
@@ -514,7 +514,7 @@ private Border CreateNodeVisual(PlanNode node, int totalWarningCount = -1)
             var objBlock = new TextBlock
             {
                 Text = node.FullObjectName ?? node.ObjectName,
-                FontSize = 9,
+                FontSize = 10,
                 Foreground = fgBrush,
                 TextAlignment = TextAlignment.Center,
                 TextWrapping = TextWrapping.Wrap,
@@ -1778,6 +1778,17 @@ private object BuildNodeTooltipContent(PlanNode node, List<PlanWarning>? allWarn
             AddTooltipRow(stack, "Actual Executions", $"{node.ActualExecutions:N0}");
         }
 
+        // Rebinds/Rewinds (spools and other operators with rebind/rewind data)
+        if (node.EstimateRebinds > 0 || node.EstimateRewinds > 0
+            || node.ActualRebinds > 0 || node.ActualRewinds > 0)
+        {
+            AddTooltipSection(stack, "Rebinds / Rewinds");
+            if (node.EstimateRebinds > 0) AddTooltipRow(stack, "Est. Rebinds", $"{node.EstimateRebinds:N1}");
+            if (node.EstimateRewinds > 0) AddTooltipRow(stack, "Est. Rewinds", $"{node.EstimateRewinds:N1}");
+            if (node.ActualRebinds > 0) AddTooltipRow(stack, "Actual Rebinds", $"{node.ActualRebinds:N0}");
+            if (node.ActualRewinds > 0) AddTooltipRow(stack, "Actual Rewinds", $"{node.ActualRewinds:N0}");
+        }
+
         // I/O and CPU estimates
         if (node.EstimateIO > 0 || node.EstimateCPU > 0 || node.EstimatedRowSize > 0)
         {
@@ -2491,10 +2502,13 @@ static string EfficiencyColor(double pct) => pct >= 80 ? "#E4E6EB"
             string? dopColor = null;
             if (statement.QueryTimeStats != null &&
                 statement.QueryTimeStats.ElapsedTimeMs > 0 &&
-                statement.QueryTimeStats.CpuTimeMs > 0)
+                statement.QueryTimeStats.CpuTimeMs > 0 &&
+                statement.DegreeOfParallelism > 1)
             {
-                var idealCpu = statement.QueryTimeStats.ElapsedTimeMs * statement.DegreeOfParallelism;
-                var efficiency = Math.Min(100.0, statement.QueryTimeStats.CpuTimeMs * 100.0 / idealCpu);
+                // Speedup ratio: CPU/elapsed = 1.0 means serial, = DOP means perfect parallelism
+                var speedup = (double)statement.QueryTimeStats.CpuTimeMs / statement.QueryTimeStats.ElapsedTimeMs;
+                var efficiency = Math.Min(100.0, (speedup - 1.0) / (statement.DegreeOfParallelism - 1.0) * 100.0);
+                efficiency = Math.Max(0.0, efficiency);
                 dopText += $" ({efficiency:N0}% efficient)";
                 dopColor = EfficiencyColor(efficiency);
             }

src/PlanViewer.App/Services/AdviceContentBuilder.cs

@@ -74,6 +74,7 @@ public static StackPanel Build(string content)
                 panel.Children.Add(new Border { Height = 6 });
                 inCodeBlock = false;
                 isStatementText = false;
+                inSubSection = false;
                 continue;
             }
 
@@ -422,6 +423,15 @@ private static Border CreateWarningBlock(string line, SolidColorBrush severityBr
                             tb.Inlines.Add(new Run(part[2..])
                                 { Foreground = ValueBrush });
                         }
+                        else if (part.StartsWith("CREATE ", StringComparison.OrdinalIgnoreCase)
+                            || part.StartsWith("ON ", StringComparison.OrdinalIgnoreCase)
+                            || part.StartsWith("INCLUDE ", StringComparison.OrdinalIgnoreCase)
+                            || part.StartsWith("WHERE ", StringComparison.OrdinalIgnoreCase))
+                        {
+                            // SQL DDL lines (CREATE INDEX, ON, INCLUDE, WHERE)
+                            tb.Inlines.Add(new Run("\n" + part)
+                                { Foreground = CodeBrush });
+                        }
                         else
                         {
                             // Other detail lines

src/PlanViewer.Core/Output/TextFormatter.cs

@@ -42,15 +42,16 @@ public static void WriteText(AnalysisResult result, TextWriter writer)
             writer.WriteLine($"=== Statement {i + 1}: ===");
             writer.WriteLine(stmt.StatementText);
             writer.WriteLine();
-            writer.WriteLine($"Estimated cost: {stmt.EstimatedCost:F4}");
+            writer.WriteLine($"Estimated cost: {stmt.EstimatedCost:N2}");
 
             if (stmt.DegreeOfParallelism > 0)
             {
                 var dopLine = $"DOP: {stmt.DegreeOfParallelism}";
-                if (stmt.QueryTime != null && stmt.QueryTime.ElapsedTimeMs > 0 && stmt.QueryTime.CpuTimeMs > 0)
+                if (stmt.QueryTime != null && stmt.QueryTime.ElapsedTimeMs > 0
+                    && stmt.QueryTime.CpuTimeMs > 0 && stmt.DegreeOfParallelism > 1)
                 {
-                    var idealCpu = stmt.QueryTime.ElapsedTimeMs * stmt.DegreeOfParallelism;
-                    var efficiency = Math.Min(100.0, stmt.QueryTime.CpuTimeMs * 100.0 / idealCpu);
+                    var speedup = (double)stmt.QueryTime.CpuTimeMs / stmt.QueryTime.ElapsedTimeMs;
+                    var efficiency = Math.Clamp((speedup - 1.0) / (stmt.DegreeOfParallelism - 1.0) * 100.0, 0, 100);
                     dopLine += $" ({efficiency:N0}% efficient)";
                 }
                 writer.WriteLine(dopLine);
@@ -294,8 +295,10 @@ private static void WriteGroupedOperatorWarnings(List<WarningResult> warnings, T
         }
 
         // Group entries that share the same severity, type, and explanation
+        // Sort criticals before warnings before info
         var grouped = entries
             .GroupBy(e => (e.Severity, e.Explanation ?? ""))
+            .OrderBy(g => g.Key.Item1 switch { "Critical" => 0, "Warning" => 1, _ => 2 })
             .ToList();
 
         foreach (var group in grouped)

src/PlanViewer.Core/Services/PlanAnalyzer.cs

@@ -672,7 +672,10 @@
 
         // Rule 14: Lazy Table Spool unfavorable rebind/rewind ratio
         // Rebinds = cache misses (child re-executes), rewinds = cache hits (reuse cached result)
-        if (!cfg.IsRuleDisabled(14) && node.LogicalOp == "Lazy Spool")
+        // Exclude Lazy Index Spools: they cache by correlated parameter value (like a hash table)
+        // so rebind/rewind counts are unreliable. See https://www.sql.kiwi/2025/02/lazy-index-spool/
+        if (!cfg.IsRuleDisabled(14) && node.LogicalOp == "Lazy Spool"
+            && !node.PhysicalOp.Contains("Index", StringComparison.OrdinalIgnoreCase))
         {
             var rebinds = node.HasActualStats ? (double)node.ActualRebinds : node.EstimateRebinds;
             var rewinds = node.HasActualStats ? (double)node.ActualRewinds : node.EstimateRewinds;
@@ -880,7 +883,7 @@
         // Rule 28: Row Count Spool — NOT IN with nullable column
         // Pattern: Row Count Spool with high rewinds, child scan has IS NULL predicate,
         // and statement text contains NOT IN
         if (!cfg.IsRuleDisabled(28) && node.PhysicalOp.Contains("Row Count Spool"))
         {
             var rewinds = node.HasActualStats ? (double)node.ActualRewinds : node.EstimateRewinds;
             if (rewinds > 10000 && HasNotInPattern(node, stmt))
@@ -1327,8 +1330,8 @@
         if (node.LogicalOp.Contains("Join", StringComparison.OrdinalIgnoreCase) && !node.IsAdaptive)
         {
             return ratio >= 10.0
-                ? "The underestimate may have caused the optimizer to choose a suboptimal join strategy."
-                : "The overestimate may have caused the optimizer to choose a suboptimal join strategy.";
+                ? "The underestimate may have caused the optimizer to make poor choices."
+                : "The overestimate may have caused the optimizer to make poor choices.";
         }
 
         // Walk up to check if a parent was harmed by this bad estimate
@@ -1355,8 +1358,8 @@
                     return null; // Adaptive join self-corrects — no harm
 
                 return ratio >= 10.0
-                    ? $"The underestimate may have caused the optimizer to choose {ancestor.PhysicalOp} when a different join type would be more efficient."
-                    : $"The overestimate may have caused the optimizer to choose {ancestor.PhysicalOp} when a different join type would be more efficient.";
+                    ? $"The underestimate may have caused the optimizer to make poor choices."
+                    : $"The overestimate may have caused the optimizer to make poor choices.";
             }
 
             // Parent Sort/Hash that spilled — downstream bad estimate caused the spill

src/PlanViewer.Core/Services/ShowPlanParser.cs

@@ -1475,8 +1475,8 @@ private static List<PlanWarning> ParseWarningsFromElement(XElement warningsEl)
             result.Add(new PlanWarning
             {
                 WarningType = "No Join Predicate",
-                Message = "This join has no join predicate (possible cross join)",
-                Severity = PlanWarningSeverity.Critical
+                Message = "This join triggered a no join predicate warning, which is worth checking on, but is often misleading. The optimizer may have removed a redundant predicate after simplification.",
+                Severity = PlanWarningSeverity.Warning
             });
         }
 

tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs

@@ -321,14 +321,15 @@ public void Rule13_DataTypeMismatch_DetectedInMismatchedPlan()
     // ---------------------------------------------------------------
 
     [Fact]
-    public void Rule14_LazySpoolIneffective_DetectsUnfavorableRatio()
+    public void Rule14_LazySpoolIneffective_SkipsLazyIndexSpools()
     {
+        // Lazy Index Spools cache by correlated parameter value (like a hash table)
+        // so rebind/rewind counts are unreliable — Rule 14 should not fire.
+        // See https://www.sql.kiwi/2025/02/lazy-index-spool/
         var plan = PlanTestHelper.LoadAndAnalyze("lazy_spool_plan.sqlplan");
         var warnings = PlanTestHelper.WarningsOfType(plan, "Lazy Spool Ineffective");
 
-        Assert.Single(warnings);
-        Assert.Contains("rebinds", warnings[0].Message);
-        Assert.Contains("rewinds", warnings[0].Message);
+        Assert.Empty(warnings);
     }
 
     // ---------------------------------------------------------------