Skip to content

Render benefit % and actionable fix in all warning surfaces (v1.7.1) #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erikdarlingdata merged 1 commit into from
Apr 21, 2026
Merged

Render benefit % and actionable fix in all warning surfaces (v1.7.1) #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1754,6 +1754,18 @@ private void ShowPropertiesPanel(PlanNode node)
TextWrapping = TextWrapping.Wrap,
Margin = new Thickness(16, 0, 0, 0)
});
if (!string.IsNullOrEmpty(w.ActionableFix))
{
warnPanel.Children.Add(new TextBlock
{
Text = w.ActionableFix,
FontSize = 11,
FontStyle = FontStyle.Italic,
Foreground = TooltipFgBrush,
TextWrapping = TextWrapping.Wrap,
Margin = new Thickness(16, 2, 0, 0)
});
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed TooltipFgBrush = #E4E6EB (line 90) — fine on the dark properties-panel surface, well out of the rejected #6B7280/#808080 dim range. Left margin (16) matches the message block above, indent reads correctly. No concerns here — leaving this so the inline record matches the summary.

Generated by Claude Code

}
planWarningsPanel.Children.Add(warnPanel);
}

Expand Down
2 changes: 1 addition & 1 deletion src/PlanViewer.App/PlanViewer.App.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<ApplicationManifest>app.manifest</ApplicationManifest>
<ApplicationIcon>EDD.ico</ApplicationIcon>
<AvaloniaUseCompiledBindingsByDefault>true</AvaloniaUseCompiledBindingsByDefault>
<Version>1.7.0</Version>
<Version>1.7.1</Version>
<Authors>Erik Darling</Authors>
<Company>Darling Data LLC</Company>
<Product>Performance Studio</Product>
Expand Down
3 changes: 3 additions & 0 deletions src/PlanViewer.Core/Output/HtmlExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ .card h3 {
.warn-type { font-size: 0.75rem; font-weight: 600; }
.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); }
.warn-msg { font-size: 0.8rem; color: var(--text); flex-basis: 100%; }
.warn-fix { font-size: 0.75rem; color: var(--text-secondary); font-style: italic; flex-basis: 100%; border-left: 2px solid var(--border); padding-left: 0.5rem; margin-top: 0.15rem; }

/* Query text */
details { margin-bottom: 0.75rem; }
Expand Down Expand Up @@ -459,6 +460,8 @@ private static void WriteWarnings(StringBuilder sb, StatementResult stmt)
if (w.MaxBenefitPercent.HasValue)
sb.AppendLine($"<span class=\"warn-benefit\">up to {w.MaxBenefitPercent:N0}% benefit</span>");
sb.AppendLine($"<span class=\"warn-msg\">{Encode(w.Message)}</span>");
if (!string.IsNullOrEmpty(w.ActionableFix))
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test coverage added for the new warn-benefit / warn-fix spans. tests/PlanViewer.Core.Tests/HtmlExporterTests.cs already exercises Export_ProducesValidHtml_WithWarnings using key_lookup_plan.sqlplan; a one-liner Assert.Contains("warn-benefit", html) (and "warn-fix" if that fixture produces an ActionableFix) would lock in this regression — the whole reason for this PR is the strip silently dropped these fields.

Generated by Claude Code

sb.AppendLine($"<span class=\"warn-fix\">{Encode(w.ActionableFix)}</span>");
sb.AppendLine("</div>");
}
sb.AppendLine("</div>");
Expand Down
2 changes: 2 additions & 0 deletions src/PlanViewer.Core/Output/TextFormatter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ public static void WriteText(AnalysisResult result, TextWriter writer)
? $" (up to {w.MaxBenefitPercent:N0}% benefit)"
: "";
writer.WriteLine($" [{w.Severity}] {w.Type}{benefitTag}: {EscapeNewlines(w.Message)}");
if (!string.IsNullOrEmpty(w.ActionableFix))
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No TextFormatterTests.cs exists under tests/PlanViewer.Core.Tests/ — the text output is effectively untested. Not a blocker for this PR, but a TextFormatter_Warning_IncludesBenefitAndFix test would be cheap insurance for the output the CLI/MCP surface depends on.

Generated by Claude Code

writer.WriteLine($" Fix: {EscapeNewlines(w.ActionableFix)}");
}
}

Expand Down
13 changes: 12 additions & 1 deletion src/PlanViewer.Web/Pages/Index.razor
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,10 @@ else
@if (infoCount > 0) { <span class="warn-count-badge info">@infoCount</span> }
</h4>
<div class="warnings-list">
@foreach (var w in GetAllWarnings(ActiveStmt!))
@foreach (var w in GetAllWarnings(ActiveStmt!)
.OrderByDescending(x => x.MaxBenefitPercent ?? -1)
.ThenByDescending(x => x.Severity == "Critical" ? 3 : x.Severity == "Warning" ? 2 : 1)
.ThenBy(x => x.Type))
{
<div class="warning @w.Severity.ToLower()">
<span class="severity">@w.Severity</span>
Expand All @@ -341,7 +344,15 @@ else
<span class="warning-op">@w.Operator</span>
}
<span class="warning-type">@w.Type</span>
@if (w.MaxBenefitPercent.HasValue)
{
<span class="warn-benefit">up to @w.MaxBenefitPercent.Value.ToString("N0")% benefit</span>
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Class-name nit: the neighboring spans in this strip are severity, warning-op, warning-type, warning-msg (full warning- prefix), but this PR introduces warn-benefit (short prefix) alongside warning-fix (full prefix) at line 354. The short form matches the HtmlExporter copy but is inconsistent with every other class in this Razor block. Consider renaming to warning-benefit for consistency with the siblings in Index.razor — HtmlExporter.cs is a separate CSS namespace and doesn't need to match.

Generated by Claude Code

}
<span class="warning-msg">@w.Message</span>
@if (!string.IsNullOrEmpty(w.ActionableFix))
{
<span class="warning-fix">@w.ActionableFix</span>
}
</div>
}
</div>
Expand Down
20 changes: 20 additions & 0 deletions src/PlanViewer.Web/wwwroot/css/app.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,26 @@ textarea::placeholder {
font-size: 0.75rem;
}

.warn-benefit {
font-size: 0.7rem;
font-weight: 600;
color: var(--text-muted);
padding: 0.05rem 0.35rem;
border-radius: 3px;
background: rgba(0, 0, 0, 0.05);
margin-right: 0.4rem;
}

.warning-fix {
color: var(--text-secondary);
display: block;
margin-top: 0.25rem;
font-size: 0.75rem;
font-style: italic;
border-left: 2px solid var(--border);
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--border: #e0e0e0 (defined at line 15) on the default light surface is barely visible — a 2px vertical rule at that color on white/near-white will effectively disappear, defeating the purpose of the quote-style indent. The HtmlExporter copy (src/PlanViewer.Core/Output/HtmlExporter.cs line 190) uses the same var(--border) and has the same issue. Consider var(--text-muted) for the border so the fix block actually reads as a distinct quoted region.

Generated by Claude Code

padding-left: 0.5rem;
}

/* === Query Text === */
.stmt-text-section {
margin-bottom: 0.75rem;
Expand Down
Loading