Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 3 additions & 9 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,17 @@ jobs:
gh release create "v${{ steps.version.outputs.VERSION }}" --title "v${{ steps.version.outputs.VERSION }}" --generate-notes --target main

- name: Setup .NET 8.0
if: steps.check.outputs.EXISTS == 'false'
uses: actions/setup-dotnet@v4
with:
dotnet-version: 8.0.x

- name: Build and test
if: steps.check.outputs.EXISTS == 'false'
run: |
dotnet restore
dotnet build -c Release
dotnet test tests/PlanViewer.Core.Tests/PlanViewer.Core.Tests.csproj -c Release --no-build --verbosity normal

- name: Publish App (all platforms)
if: steps.check.outputs.EXISTS == 'false'
run: |
dotnet publish src/PlanViewer.App/PlanViewer.App.csproj -c Release -r win-x64 --self-contained -o publish/win-x64
dotnet publish src/PlanViewer.App/PlanViewer.App.csproj -c Release -r linux-x64 --self-contained -o publish/linux-x64
Expand All @@ -68,7 +65,6 @@ jobs:

# ── SignPath code signing (Windows only, skipped if secret not configured) ──
- name: Check if signing is configured
if: steps.check.outputs.EXISTS == 'false'
id: signing
shell: bash
run: |
Expand All @@ -80,15 +76,15 @@ jobs:
fi

- name: Upload Windows build for signing
if: steps.check.outputs.EXISTS == 'false' && steps.signing.outputs.ENABLED == 'true'
if: steps.signing.outputs.ENABLED == 'true'
id: upload-unsigned
uses: actions/upload-artifact@v4
with:
name: App-unsigned
path: publish/win-x64/

- name: Sign Windows build
if: steps.check.outputs.EXISTS == 'false' && steps.signing.outputs.ENABLED == 'true'
if: steps.signing.outputs.ENABLED == 'true'
uses: signpath/github-action-submit-signing-request@v1
with:
api-token: '${{ secrets.SIGNPATH_API_TOKEN }}'
Expand All @@ -101,15 +97,14 @@ jobs:
output-artifact-directory: 'signed/win-x64'

- name: Replace unsigned Windows build with signed
if: steps.check.outputs.EXISTS == 'false' && steps.signing.outputs.ENABLED == 'true'
if: steps.signing.outputs.ENABLED == 'true'
shell: pwsh
run: |
Remove-Item -Recurse -Force publish/win-x64
Copy-Item -Recurse signed/win-x64 publish/win-x64

# ── Velopack (uses signed Windows binaries) ───────────────────────
- name: Create Velopack release (Windows)
if: steps.check.outputs.EXISTS == 'false'
shell: pwsh
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Expand All @@ -126,7 +121,6 @@ jobs:

# ── Package and upload ────────────────────────────────────────────
- name: Package and upload
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Idempotency check: gh release upload ... --clobber (line 184) and vpk upload ... --merge (line 187) handle re-runs against an existing release, so dropping the EXISTS == 'false' guards on the build/sign/package steps is safe from a failure-mode standpoint. The trade-off is that every re-run now submits a fresh SignPath signing request and a full Velopack pack — intentional for the "partial failure on the first run" recovery case, just flagging the cost side for awareness.


Generated by Claude Code

if: steps.check.outputs.EXISTS == 'false'
shell: pwsh
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Expand Down
12 changes: 12 additions & 0 deletions src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs
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)
});
}
planWarningsPanel.Children.Add(warnPanel);
}

Expand Down
2 changes: 1 addition & 1 deletion src/PlanViewer.App/PlanViewer.App.csproj
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
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))
sb.AppendLine($"<span class=\"warn-fix\">{Encode(w.ActionableFix)}</span>");
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

New warn-fix span branch has no coverage in tests/PlanViewer.Core.Tests/HtmlExporterTests.cs. A snapshot-style assertion that a warning with ActionableFix set emits the span (and one with it null/empty does not) would guard against a regression like the one this PR is fixing.


Generated by Claude Code

sb.AppendLine("</div>");
}
sb.AppendLine("</div>");
Expand Down
2 changes: 2 additions & 0 deletions src/PlanViewer.Core/Output/TextFormatter.cs
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))
writer.WriteLine($" Fix: {EscapeNewlines(w.ActionableFix)}");
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Same gap on the text side — no test in tests/PlanViewer.Core.Tests/ asserts the Fix: ... line renders when ActionableFix is set. Text output is the easiest of the four surfaces to golden-test.


Generated by Claude Code

}
}

Expand Down
13 changes: 12 additions & 1 deletion src/PlanViewer.Web/Pages/Index.razor
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))
Comment on lines +335 to +338
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Ordering here mirrors TextFormatter.cs:166-168, which is good — but the secondary key differs in direction. Text uses .ThenBy(w => w.Severity switch { "Critical" => 0, "Warning" => 1, _ => 2 }) (ascending over an ordinal where Critical is lowest). Web uses .ThenByDescending(x => x.Severity == "Critical" ? 3 : x.Severity == "Warning" ? 2 : 1). Both happen to yield Critical → Warning → Info, so behavior matches, but the inverted convention invites drift the next time someone touches one side and not the other. Worth normalizing to a single helper or identical expression.


Generated by Claude Code

{
<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>
}
<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
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);
padding-left: 0.5rem;
}

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