Skip to content

Commit 3ca9398

Browse files
erikdarlingdataerikdarlingdata
authored
Merge pull request #219 from erikdarlingdata/feature/benefit-scoring
Add maximum benefit scoring for plan analysis findings
2 parents e763623 + 65170fd commit 3ca9398

File tree

14 files changed

+475
-25
lines changed

14 files changed

+475
-25
lines changed

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

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ public void LoadPlan(string planXml, string label, string? queryText = null)
275275

276276
_currentPlan = ShowPlanParser.Parse(planXml);
277277
PlanAnalyzer.Analyze(_currentPlan, ConfigLoader.Load());
278+
BenefitScorer.Score(_currentPlan);
278279

279280
var allStatements = _currentPlan.Batches
280281
.SelectMany(b => b.Statements)
@@ -1725,14 +1726,21 @@ private void ShowPropertiesPanel(PlanNode node)
17251726
if (s.PlanWarnings.Count > 0)
17261727
{
17271728
var planWarningsPanel = new StackPanel();
1728-
foreach (var w in s.PlanWarnings)
1729+
var sortedPlanWarnings = s.PlanWarnings
1730+
.OrderByDescending(w => w.MaxBenefitPercent ?? -1)
1731+
.ThenByDescending(w => w.Severity)
1732+
.ThenBy(w => w.WarningType);
1733+
foreach (var w in sortedPlanWarnings)
17291734
{
17301735
var warnColor = w.Severity == PlanWarningSeverity.Critical ? "#E57373"
17311736
: w.Severity == PlanWarningSeverity.Warning ? "#FFB347" : "#6BB5FF";
17321737
var warnPanel = new StackPanel { Margin = new Thickness(10, 2, 10, 2) };
1738+
var planWarnHeader = w.MaxBenefitPercent.HasValue
1739+
? $"\u26A0 {w.WarningType} \u2014 up to {w.MaxBenefitPercent:N0}% benefit"
1740+
: $"\u26A0 {w.WarningType}";
17331741
warnPanel.Children.Add(new TextBlock
17341742
{
1735-
Text = $"\u26A0 {w.WarningType}",
1743+
Text = planWarnHeader,
17361744
FontWeight = FontWeight.SemiBold,
17371745
FontSize = 11,
17381746
Foreground = new SolidColorBrush(Color.Parse(warnColor))
@@ -1788,14 +1796,21 @@ private void ShowPropertiesPanel(PlanNode node)
17881796
if (node.HasWarnings)
17891797
{
17901798
var warningsPanel = new StackPanel();
1791-
foreach (var w in node.Warnings)
1799+
var sortedNodeWarnings = node.Warnings
1800+
.OrderByDescending(w => w.MaxBenefitPercent ?? -1)
1801+
.ThenByDescending(w => w.Severity)
1802+
.ThenBy(w => w.WarningType);
1803+
foreach (var w in sortedNodeWarnings)
17921804
{
17931805
var warnColor = w.Severity == PlanWarningSeverity.Critical ? "#E57373"
17941806
: w.Severity == PlanWarningSeverity.Warning ? "#FFB347" : "#6BB5FF";
17951807
var warnPanel = new StackPanel { Margin = new Thickness(10, 2, 10, 2) };
1808+
var nodeWarnHeader = w.MaxBenefitPercent.HasValue
1809+
? $"\u26A0 {w.WarningType} \u2014 up to {w.MaxBenefitPercent:N0}% benefit"
1810+
: $"\u26A0 {w.WarningType}";
17961811
warnPanel.Children.Add(new TextBlock
17971812
{
1798-
Text = $"\u26A0 {w.WarningType}",
1813+
Text = nodeWarnHeader,
17991814
FontWeight = FontWeight.SemiBold,
18001815
FontSize = 11,
18011816
Foreground = new SolidColorBrush(Color.Parse(warnColor))
@@ -2140,18 +2155,21 @@ private object BuildNodeTooltipContent(PlanNode node, List<PlanWarning>? allWarn
21402155

21412156
if (allWarnings != null)
21422157
{
2143-
// Root node: show distinct warning type names only
2158+
// Root node: show distinct warning type names only, sorted by max benefit
21442159
var distinct = warnings
21452160
.GroupBy(w => w.WarningType)
2146-
.Select(g => (Type: g.Key, MaxSeverity: g.Max(w => w.Severity), Count: g.Count()))
2147-
.OrderByDescending(g => g.MaxSeverity)
2161+
.Select(g => (Type: g.Key, MaxSeverity: g.Max(w => w.Severity), Count: g.Count(),
2162+
MaxBenefit: g.Max(w => w.MaxBenefitPercent ?? -1)))
2163+
.OrderByDescending(g => g.MaxBenefit)
2164+
.ThenByDescending(g => g.MaxSeverity)
21482165
.ThenBy(g => g.Type);
21492166

2150-
foreach (var (type, severity, count) in distinct)
2167+
foreach (var (type, severity, count, maxBenefit) in distinct)
21512168
{
21522169
var warnColor = severity == PlanWarningSeverity.Critical ? "#E57373"
21532170
: severity == PlanWarningSeverity.Warning ? "#FFB347" : "#6BB5FF";
2154-
var label = count > 1 ? $"\u26A0 {type} ({count})" : $"\u26A0 {type}";
2171+
var benefitSuffix = maxBenefit >= 0 ? $" \u2014 up to {maxBenefit:N0}%" : "";
2172+
var label = count > 1 ? $"\u26A0 {type} ({count}){benefitSuffix}" : $"\u26A0 {type}{benefitSuffix}";
21552173
stack.Children.Add(new TextBlock
21562174
{
21572175
Text = label,

‎src/PlanViewer.App/Mcp/McpQueryStoreTools.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ public static async Task<string> GetQueryStoreTop(
124124
.Replace("encoding=\"utf-16\"", "encoding=\"utf-8\"");
125125
var parsed = ShowPlanParser.Parse(xml);
126126
PlanAnalyzer.Analyze(parsed);
127+
BenefitScorer.Score(parsed);
127128

128129
var allStatements = parsed.Batches.SelectMany(b => b.Statements).ToList();
129130

‎src/PlanViewer.App/PlanViewer.App.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<ApplicationManifest>app.manifest</ApplicationManifest>
77
<ApplicationIcon>EDD.ico</ApplicationIcon>
88
<AvaloniaUseCompiledBindingsByDefault>true</AvaloniaUseCompiledBindingsByDefault>
9-
<Version>1.4.3</Version>
9+
<Version>1.5.0</Version>
1010
<Authors>Erik Darling</Authors>
1111
<Company>Darling Data LLC</Company>
1212
<Product>Performance Studio</Product>

‎src/PlanViewer.Cli/Commands/AnalyzeCommand.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ private static async Task RunAsync(FileInfo? file, bool stdin, string output, bo
207207

208208
var plan = ShowPlanParser.Parse(planXml);
209209
PlanAnalyzer.Analyze(plan, analyzerConfig);
210+
BenefitScorer.Score(plan);
210211

211212
if (plan.Batches.Count == 0)
212213
{
@@ -400,6 +401,7 @@ private static async Task RunLiveAsync(
400401
// Parse, analyze, map result
401402
var plan = ShowPlanParser.Parse(planXml);
402403
PlanAnalyzer.Analyze(plan, analyzerConfig);
404+
BenefitScorer.Score(plan);
403405
var result = ResultMapper.Map(plan, $"{name}.sql");
404406

405407
if (warningsOnly)

‎src/PlanViewer.Cli/Commands/QueryStoreCommand.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ private static async Task RunAsync(
273273
// Parse, analyze, map
274274
var plan = ShowPlanParser.Parse(qsPlan.PlanXml);
275275
PlanAnalyzer.Analyze(plan, analyzerConfig);
276+
BenefitScorer.Score(plan);
276277
var result = ResultMapper.Map(plan, $"{label}.sqlplan");
277278

278279
if (warningsOnly)

‎src/PlanViewer.Core/Models/PlanModels.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,17 @@ public class PlanWarning
373373
public string Message { get; set; } = "";
374374
public PlanWarningSeverity Severity { get; set; }
375375
public SpillDetail? SpillDetails { get; set; }
376+
377+
/// <summary>
378+
/// Maximum percentage of elapsed time that could be saved by addressing this finding.
379+
/// null = not quantifiable, 0 = calculated as negligible.
380+
/// </summary>
381+
public double? MaxBenefitPercent { get; set; }
382+
383+
/// <summary>
384+
/// Short actionable fix suggestion (e.g., "Add INCLUDE (columns) to index").
385+
/// </summary>
386+
public string? ActionableFix { get; set; }
376387
}
377388

378389
public enum PlanWarningSeverity { Info, Warning, Critical }

‎src/PlanViewer.Core/Output/AnalysisResult.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,12 @@ public class WarningResult
208208

209209
[JsonPropertyName("node_id")]
210210
public int? NodeId { get; set; }
211+
212+
[JsonPropertyName("max_benefit_percent")]
213+
public double? MaxBenefitPercent { get; set; }
214+
215+
[JsonPropertyName("actionable_fix")]
216+
public string? ActionableFix { get; set; }
211217
}
212218

213219
public class MissingIndexResult

‎src/PlanViewer.Core/Output/HtmlExporter.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ .card h3 {
185185
.sev-info { color: var(--info); }
186186
.warn-op { font-size: 0.75rem; font-weight: 500; color: var(--text-secondary); }
187187
.warn-type { font-size: 0.75rem; font-weight: 600; }
188+
.warn-benefit { font-size: 0.7rem; font-weight: 600; color: var(--text-muted); padding: 0.05rem 0.3rem; border-radius: 3px; background: rgba(0,0,0,0.04); }
188189
.warn-msg { font-size: 0.8rem; color: var(--text); flex-basis: 100%; }
189190
190191
/* Query text */
@@ -428,14 +429,22 @@ private static void WriteWarnings(StringBuilder sb, StatementResult stmt)
428429
if (infoCount > 0) sb.Append($" <span class=\"warn-badge info\">{infoCount}</span>");
429430
sb.AppendLine("</h3>");
430431

431-
foreach (var w in allWarnings)
432+
// Sort by benefit descending (nulls last), then severity, then type
433+
var sorted = allWarnings
434+
.OrderByDescending(w => w.MaxBenefitPercent ?? -1)
435+
.ThenBy(w => w.Severity switch { "Critical" => 0, "Warning" => 1, _ => 2 })
436+
.ThenBy(w => w.Type);
437+
438+
foreach (var w in sorted)
432439
{
433440
var sevLower = w.Severity.ToLower();
434441
sb.AppendLine($"<div class=\"warning-item {sevLower}\">");
435442
sb.AppendLine($"<span class=\"sev sev-{sevLower}\">{Encode(w.Severity)}</span>");
436443
if (w.Operator != null)
437444
sb.AppendLine($"<span class=\"warn-op\">{Encode(w.Operator)}</span>");
438445
sb.AppendLine($"<span class=\"warn-type\">{Encode(w.Type)}</span>");
446+
if (w.MaxBenefitPercent.HasValue)
447+
sb.AppendLine($"<span class=\"warn-benefit\">up to {w.MaxBenefitPercent:N0}% benefit</span>");
439448
sb.AppendLine($"<span class=\"warn-msg\">{Encode(w.Message)}</span>");
440449
sb.AppendLine("</div>");
441450
}

‎src/PlanViewer.Core/Output/ResultMapper.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,9 @@ private static StatementResult MapStatement(PlanStatement stmt)
158158
{
159159
Type = w.WarningType,
160160
Severity = w.Severity.ToString(),
161-
Message = w.Message
161+
Message = w.Message,
162+
MaxBenefitPercent = w.MaxBenefitPercent,
163+
ActionableFix = w.ActionableFix
162164
});
163165
}
164166

@@ -259,7 +261,9 @@ private static OperatorResult MapNode(PlanNode node)
259261
Severity = w.Severity.ToString(),
260262
Message = w.Message,
261263
Operator = $"{node.PhysicalOp} (Node {node.NodeId})",
262-
NodeId = node.NodeId
264+
NodeId = node.NodeId,
265+
MaxBenefitPercent = w.MaxBenefitPercent,
266+
ActionableFix = w.ActionableFix
263267
});
264268
}
265269

‎src/PlanViewer.Core/Output/TextFormatter.cs‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,17 @@ public static void WriteText(AnalysisResult result, TextWriter writer)
151151
writer.WriteLine("Plan warnings:");
152152
var hasDetailedMemoryGrant = stmt.Warnings.Any(w =>
153153
w.Type == "Excessive Memory Grant" || w.Type == "Large Memory Grant");
154-
foreach (var w in stmt.Warnings)
154+
var sortedWarnings = stmt.Warnings
155+
.Where(w => !(w.Type == "Memory Grant" && hasDetailedMemoryGrant))
156+
.OrderByDescending(w => w.MaxBenefitPercent ?? -1)
157+
.ThenBy(w => w.Severity switch { "Critical" => 0, "Warning" => 1, _ => 2 })
158+
.ThenBy(w => w.Type);
159+
foreach (var w in sortedWarnings)
155160
{
156-
// Skip raw XML "Memory Grant" when analyzer provides better context
157-
if (w.Type == "Memory Grant" && hasDetailedMemoryGrant)
158-
continue;
159-
writer.WriteLine($" [{w.Severity}] {w.Type}: {EscapeNewlines(w.Message)}");
161+
var benefitTag = w.MaxBenefitPercent.HasValue
162+
? $" (up to {w.MaxBenefitPercent:N0}% benefit)"
163+
: "";
164+
writer.WriteLine($" [{w.Severity}] {w.Type}{benefitTag}: {EscapeNewlines(w.Message)}");
160165
}
161166
}
162167

@@ -272,10 +277,17 @@ private static void CollectNodeWarnings(OperatorResult node, List<WarningResult>
272277

273278
private static void WriteGroupedOperatorWarnings(List<WarningResult> warnings, TextWriter writer)
274279
{
280+
// Sort by benefit descending (nulls last), then severity, then type
281+
var sorted = warnings
282+
.OrderByDescending(w => w.MaxBenefitPercent ?? -1)
283+
.ThenBy(w => w.Severity switch { "Critical" => 0, "Warning" => 1, _ => 2 })
284+
.ThenBy(w => w.Type)
285+
.ToList();
286+
275287
// Split each message into "data | explanation" at the last sentence boundary
276288
// that starts with "The " (the harm assessment). Group by shared explanation.
277-
var entries = new List<(string Severity, string Operator, string Data, string? Explanation)>();
278-
foreach (var w in warnings)
289+
var entries = new List<(string Severity, string Operator, string Data, string? Explanation, double? Benefit)>();
290+
foreach (var w in sorted)
279291
{
280292
var msg = w.Message;
281293
string data;
@@ -293,14 +305,13 @@ private static void WriteGroupedOperatorWarnings(List<WarningResult> warnings, T
293305
data = msg;
294306
}
295307

296-
entries.Add((w.Severity, w.Operator ?? "?", data, explanation));
308+
entries.Add((w.Severity, w.Operator ?? "?", data, explanation, w.MaxBenefitPercent));
297309
}
298310

299311
// Group entries that share the same severity, type, and explanation
300-
// Sort criticals before warnings before info
312+
// Preserve the pre-sorted order (benefit desc, severity, type)
301313
var grouped = entries
302314
.GroupBy(e => (e.Severity, e.Explanation ?? ""))
303-
.OrderBy(g => g.Key.Item1 switch { "Critical" => 0, "Warning" => 1, _ => 2 })
304315
.ToList();
305316

306317
foreach (var group in grouped)
@@ -310,7 +321,10 @@ private static void WriteGroupedOperatorWarnings(List<WarningResult> warnings, T
310321
{
311322
// Multiple operators with the same explanation — list compactly
312323
foreach (var item in items)
313-
writer.WriteLine($" [{item.Severity}] {item.Operator}: {EscapeNewlines(item.Data)}");
324+
{
325+
var benefitTag = item.Benefit.HasValue ? $" (up to {item.Benefit:N0}% benefit)" : "";
326+
writer.WriteLine($" [{item.Severity}] {item.Operator}{benefitTag}: {EscapeNewlines(item.Data)}");
327+
}
314328
writer.WriteLine($" -> {group.Key.Item2}");
315329
}
316330
else
@@ -319,7 +333,8 @@ private static void WriteGroupedOperatorWarnings(List<WarningResult> warnings, T
319333
foreach (var item in items)
320334
{
321335
var full = item.Explanation != null ? $"{item.Data}. {item.Explanation}" : item.Data;
322-
writer.WriteLine($" [{item.Severity}] {item.Operator}: {EscapeNewlines(full)}");
336+
var benefitTag = item.Benefit.HasValue ? $" (up to {item.Benefit:N0}% benefit)" : "";
337+
writer.WriteLine($" [{item.Severity}] {item.Operator}{benefitTag}: {EscapeNewlines(full)}");
323338
}
324339
}
325340
}

0 commit comments

Comments
 (0)