Release v1.6.0 by erikdarlingdata · Pull Request #243 · erikdarlingdata/PerformanceStudio

erikdarlingdata · 2026-04-20T15:59:04Z

v1.6.0 Release

Highlights

Wait stats benefit scoring (Stage 2 of [FEATURE] Add system to assign a "maximum benefit" for plan analysis rules #215) — wait stats now contribute to the max-benefit calculation in PlanAnalyzer findings
CPU:Elapsed ratio replaces the old parallelism-efficiency warnings for clearer parallel-plan diagnosis
SQL Formatter in the Query Editor (Feature/query editor sql formater #238) with persistent format options (parameters dialog, async formatting, parse-error popup)
Query Store group-by changer (Feature/query store group by changer #218) — group plans by QueryHash or Module, with auto-expand, golden headers, and reordered columns
Built-in analytics for PlanShare and stats dashboard
Joe's plan-review feedback — false-positive fixes, improved messaging, Rule 34 narrowing, Serial Plan / adaptive join math, MAXDOP 1 wording, Scalar UDF redundancy, parallel-skew + key-lookup NL join scoring

Other

PlanViewer toolbars / Query Store headers / tab accent polish
Query Editor no longer loses syntax highlighting on tab switch (Fix: Query Editor losing syntax highlighting on tab switch #234)
Auto-scrolling improvements ([FEATURE] Enable Auto-Scrolling (panning) on Grids #225, thanks @ClaudioESSilva)
Wait-stats benefit wired into the web plan viewer

🤖 Generated with Claude Code

* fixes #233 * Improve the fix

- Query Store grid: dropped "(Local)" from Last Executed (the Time display dropdown already controls timezone), shortened "Physical Reads" to "Phys Reads", and widened CPU / Duration / Memory / Phys Reads / Executions columns by 5-15px so the "(ms)" and "(MB)" unit suffixes no longer truncate in the default layout. - Toolbar density: reduced button padding 10,0 -> 8,0, StackPanel Spacing 6 -> 4, and pipe-separator margins 4,0 -> 2,0 (and 12,0 -> 6,0) across both QuerySessionControl and PlanViewerControl toolbars. Dropped redundant per-button margins now that Spacing handles them. - Wait palette: Worker Thread #19ff25 -> #52E3B5 (teal, distinct from CPU green), Parallelism #fd01d3 -> #7B4FFF (vivid purple, out of the pink/magenta cluster) so neighboring wait categories read as different colors on the stacked wait profile bar. - Added SystemAccentColor (+ Dark1/Dark2/Light1/Light2 variants) to DarkTheme. Fluent theme's TabItem selected pipe reads from SystemAccentColor, so Plan 1 / Plan 2 tabs now get the brand-blue underline consistently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Polish PlanViewer toolbars, Query Store headers, and tab accent

Clears CS8933 and CS8019 — System is already covered by the SDK's implicit global usings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…sing-serverconnection Remove redundant System using in ServerConnection

* Query Editor SQL Formater with parameters options - init working * improve parameter format option grid ux * undo possible after format * Dynamic brushes instead of hard coded color * remove unused using lib * Fix Reviewer remarks phase 1 * Fix Second-opinion addendum * fix query editor content disapear if format have a syntax error. Now a popup message appears with the syntax error * 1. Hardcoded colors in error dialog → Now uses (IBrush)this.FindResource("BackgroundBrush")! 2. Close without dirty-state prompt : fixed 3. Tooltip Ctrl+K,Ctrl+D : removed * dynamic brush for formatoptions * Build passes. All three callers of Load(out string?)/Save(SqlFormatSettings, out string?) now surface errors:

The format error dialog was launched fire-and-forget, causing status text to update and the method to return before the user dismissed it. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

erikdarlingdata

PR #243 — v1.6.0 release review

What it does
Merges dev → main for v1.6.0. The 12 files in this diff cover: the new SQL Formatter (SqlFormattingService + SqlFormatSettingsService + FormatOptionsWindow + Format/Format Options buttons in QuerySessionControl), a fix for Query Editor losing syntax highlighting on tab switch (#234), PlanViewer / Query Store toolbar polish (padding/spacing/column width tweaks and header renames), two WaitCategory color swaps in DarkTheme.axaml, a Fluent SystemAccentColor* override, a removed unused using System; in ServerConnection.cs, the v1.6.0 csproj bump, and THIRD-PARTY-NOTICES.md for SqlFormatter attribution.

Base branch: main. Per the standard rule, PRs target dev — but this is the release merge, so it's intentional. Calling it out for the record.

PR description vs. diff mismatch. The description claims wait-stats benefit scoring (Stage 2 of #215), CPU:Elapsed ratio, Query Store group-by changer (#218), PlanShare analytics, Joe's plan-review feedback (Rule 34 narrowing, Serial Plan / adaptive join math, Scalar UDF, parallel-skew + key-lookup NL scoring), etc. None of those are in this diff — there are no PlanViewer.Core rule/scoring changes here. Presumably they landed via earlier dev→main merges. Worth reconciling the release notes before tagging, otherwise the changelog will mislead users scanning the commit range.

PlanAnalyzer sync check: Since no PlanAnalyzer rules are touched here, nothing to sync to PerformanceMonitor Dashboard / Lite for this PR. If any rule changes did land on dev since the last release, verify those were mirrored at the time.

What's good

SQL Formatter is correctly implemented as a Task.Run wrapper so parsing doesn't block the UI thread.
QueryEditor.Document.BeginUpdate() / EndUpdate() around Replace preserves undo and avoids intermediate re-rendering; caret offset is clamped to new length.
SetupSyntaxHighlighting re-install on re-attach is a clean fix for the #234 tab-switch issue.
PlaceholderText (not Watermark) is used correctly on ComboBoxes.
New WaitCategory colors (#52E3B5, #7B4FFF) are bright/saturated — not in the rejected #6B7280/dim-grey range.
SqlFormatSettingsService round-trips to LocalApplicationData with reasonable error surfaces.

What needs attention (see inline comments)

FormatOptionRow : INotifyPropertyChanged — MVVM creep that the repo explicitly avoids.
Save_Click silently swallows bad int input and generic exceptions; the user gets no feedback that their edit was dropped.
OnClosing fire-and-forgets an async void TryClose() that can crash the app on exception. Format_Click has the same async void exposure.
Revert_Click flips _isDirty=true unnecessarily, triggering a spurious "Unsaved Changes" prompt.
SqlFormattingService.Format generates the script even when parse errors exist and callers discard it.
SetupSyntaxHighlighting re-install — quick perf/leak sanity check on heavy multi-tab sessions.
No validation on KeywordCasing string at save time (low risk given the ComboBox UI).

Test coverage. New logic in src/PlanViewer.App/Services/ (SqlFormattingService, SqlFormatSettingsService) has no corresponding tests. tests/PlanViewer.Core.Tests/ is Core-scoped so it isn't the natural home — but if there's no PlanViewer.App.Tests, the settings round-trip and format-happy-path at minimum are easy to cover and would catch regressions on the ScriptDom dependency bump.

Comments only — not approving or requesting changes. Maintainer's call.

Generated by Claude Code

erikdarlingdata · 2026-04-20T16:03:21Z

+    }
+}
+
+public class FormatOptionRow : INotifyPropertyChanged


FormatOptionRow : INotifyPropertyChanged is MVVM creep. Repo convention is code-behind only — no INotifyPropertyChanged, no bindings-to-VM. The DataGrid needs change notification only for the BoolValue⇄CurrentValue sync; that can be handled imperatively from the code-behind (e.g., wire ToggleSwitch.IsCheckedChanged in cell templates, or rebuild the rows after edits). If you keep INPC here, it sets a precedent that will leak into the rest of the app.

Generated by Claude Code

erikdarlingdata · 2026-04-20T16:03:34Z

+                    if (!int.TryParse(row.CurrentValue, out var intVal))
+                    {
+                        Debug.WriteLine($"FormatOptions: invalid int value '{row.CurrentValue}' for {row.Name}, using default");
+                        continue;
+                    }
+                    value = intVal;
+                }
+                else
+                    value = row.CurrentValue;
+
+                prop.SetValue(settings, value);
+            }
+            catch (Exception ex)
+            {
+                Debug.WriteLine($"FormatOptions: failed to set {row.Name}: {ex.Message}");
+            }


Both error paths here silently drop the user's edit with only a Debug.WriteLine:

L109-113: invalid int → continue, the previously-saved value is silently kept.

L121-124: blanket catch (Exception) → same.

From the user's perspective, they typed something, clicked Save, the dialog closed, and the setting didn't change. There's no feedback. Either surface a validation error (reuse ShowErrorPopup with return; so nothing saves) or constrain the editor so a bad value can't be typed in the first place. Also consider int.TryParse(row.CurrentValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out …) since the JSON file is invariant.

Generated by Claude Code

erikdarlingdata · 2026-04-20T16:03:43Z

+    protected override void OnClosing(WindowClosingEventArgs e)
+    {
+        if (_isDirty)
+        {
+            e.Cancel = true;
+            TryClose();
+            return;
+        }
+        base.OnClosing(e);
+    }
+
+    private async void TryClose()


OnClosing fires-and-forgets an async void TryClose(). Any exception from ShowDiscardDialog (e.g., FindResource miss on a themed resource, dialog owner already closed) propagates to the SynchronizationContext as an unhandled exception and crashes the app. Wrap the body of TryClose in try/catch and log, or make it return Task and await it from a single-purpose async void shim. Same concern applies to Format_Click in QuerySessionControl.axaml.cs:2009 — the try/finally there doesn't catch, so SqlFormattingService.Format / ShowDialog exceptions bubble out of an async void.

Generated by Claude Code

erikdarlingdata · 2026-04-20T16:03:57Z

+    private void Revert_Click(object? sender, RoutedEventArgs e)
+    {
+        foreach (var row in _rows)
+        {
+            row.CurrentValue = row.DefaultValue;
+            if (row.IsBool)
+                row.BoolValue = row.DefaultBoolValue;
+        }
+    }


Revert_Click mutates CurrentValue / BoolValue, which fires the PropertyChanged handler added in LoadSettings (L91) and flips _isDirty = true, even when the user reverts to defaults and never had pending edits. Net effect: click Revert → Close → "Unsaved Changes" prompt appears unnecessarily. Consider suppressing the dirty flag during revert, or comparing against defaults before setting _isDirty.

Generated by Claude Code

erikdarlingdata · 2026-04-20T16:04:10Z

+
+        if (fragment == null)
+            return (sql, errors);
+


If parser.Parse returns a non-null fragment but errors is non-empty (it's designed to recover), you still run GenerateScript against the partially-valid AST and return it as formattedText. Callers in QuerySessionControl.Format_Click (L2026) treat any non-null errors as "format failed, show dialog and bail" — so the generated output is discarded. That's the safer path, but it means this method is doing work that gets thrown away. Either bail here too when errors are present, or let callers decide without double-generating.

Generated by Claude Code

erikdarlingdata · 2026-04-20T16:04:19Z

        AttachedToVisualTree += (_, _) =>
        {
+            if (_textMateInstallation == null)
+                SetupSyntaxHighlighting();
+
            QueryEditor.Focus();
            QueryEditor.TextArea.Focus();
        };

-        DetachedFromVisualTree += (_, _) => _textMateInstallation?.Dispose();
+        // Dispose TextMate when detached (e.g. tab switch) to release renderers/transformers
+        DetachedFromVisualTree += (_, _) =>
+        {
+            _textMateInstallation?.Dispose();
+            _textMateInstallation = null;
+        };


Fix for #234 looks right. Two notes:

SetupSyntaxHighlighting() is called on every re-attach; confirm it doesn't leak the RegistryOptions / grammar objects (the old _textMateInstallation?.Dispose() only releases the installation, not whatever InstallTextMate allocated on the editor). If it's cheap, fine.

Tab-switch churn now runs a full TextMate reinstall every time — worth a quick perf sanity check with a busy session that has many tabs.

Generated by Claude Code

erikdarlingdata · 2026-04-20T16:04:27Z

+            IndentSetClause = IndentSetClause,
+            IndentViewBody = IndentViewBody,
+            IndentationSize = IndentationSize,
+            KeywordCasing = Enum.TryParse<Microsoft.SqlServer.TransactSql.ScriptDom.KeywordCasing>(KeywordCasing, true, out var kc)


KeywordCasing round-trips via ToString() / Enum.TryParse. Persisting the string form is fine, but there's no validation on Save — a user can type any text in the options grid (if the combo cell is edited as text), it'll get saved to JSON, and on next Load the Enum.TryParse silently falls back to Uppercase. Not a real risk with the current ComboBox-backed editor, but worth validating on Save so bad state never hits disk.

Generated by Claude Code

ClaudioESSilva and others added 8 commits April 17, 2026 16:28

Fix: Query Editor losing syntax highlighting on tab switch (#234)

f2eb1c1

* fixes #233 * Improve the fix

Merge pull request #239 from erikdarlingdata/ui/planviewer-polish

2c239e1

Polish PlanViewer toolbars, Query Store headers, and tab accent

Remove redundant System using in ServerConnection

6ec4a17

Clears CS8933 and CS8019 — System is already covered by the SDK's implicit global usings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Merge pull request #240 from erikdarlingdata/chore/remove-redundant-u…

4db4bc8

…sing-serverconnection Remove redundant System using in ServerConnection

Await ShowDialog in Format_Click to fix CS4014 (#241)

f03f565

The format error dialog was launched fire-and-forget, causing status text to update and the method to return before the user dismissed it. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Bump version to 1.6.0 (#242)

78d3a46

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

erikdarlingdata commented Apr 20, 2026

View reviewed changes

erikdarlingdata merged commit 2958568 into main Apr 20, 2026
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release v1.6.0#243

Release v1.6.0#243
erikdarlingdata merged 8 commits intomainfrom
dev

erikdarlingdata commented Apr 20, 2026

Uh oh!

erikdarlingdata left a comment

Uh oh!

erikdarlingdata Apr 20, 2026

Uh oh!

erikdarlingdata Apr 20, 2026

Uh oh!

erikdarlingdata Apr 20, 2026

Uh oh!

erikdarlingdata Apr 20, 2026

Uh oh!

erikdarlingdata Apr 20, 2026

Uh oh!

erikdarlingdata Apr 20, 2026

Uh oh!

erikdarlingdata Apr 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

erikdarlingdata commented Apr 20, 2026

v1.6.0 Release

Highlights

Other

Uh oh!

erikdarlingdata left a comment

Choose a reason for hiding this comment

PR #243 — v1.6.0 release review

Uh oh!

erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants