Query history improvments by rferraton · Pull Request #336 · erikdarlingdata/PerformanceStudio

rferraton · 2026-05-17T21:51:38Z

Query Store History Improvements

Summary

This PR adds a Query Store History feature that lets users view execution history for a query, visualize trends on a chart, and load plans directly from historical data. It also includes several UX improvements for tab management.

New Features

Query Store History Viewer

New QueryStoreHistoryControl UserControl — displays a ScottPlot chart and DataGrid showing query execution metrics over time (CPU, duration, reads, memory, etc.)
Numeric sorting — DataGrid columns bind to numeric properties with StringFormat so sorting is numeric, not lexicographic
Interactive chart ↔ grid linking — clicking chart dots highlights corresponding grid rows and vice versa; hover tooltip shows metric details
Context menu: "Load First/Last Plan" — right-click a selected row or chart point to fetch and open the oldest or newest execution plan for that plan hash via QueryStoreService.FetchPlanByHashAsync

Sub-tab Hosting

History views open as sub-tabs inside the QuerySessionControl rather than cluttering the main tab bar
Sub-tabs have their own close button

Long-press to Detach

Long-press (500 ms) any tab header to detach it into a free-floating window
6 px dead-zone prevents accidental detach while clicking
Minimizing or closing the detached window re-docks the content back to its tab
Extracted shared TabHeaderLongPressBehavior helper used by both MainWindow tabs and session sub-tabs

Window Lifecycle

Closing the main window now closes all detached windows — iterates IClassicDesktopStyleApplicationLifetime.Windows on OnClosed

Bug Fixes

Shared ContextMenu — each control (DataGrid, Chart) now gets its own ContextMenu instance (Avalonia can only parent a menu to one control)
Silent error swallowing — plan load errors are now shown in the status bar instead of being silently caught
Wrong type check in Close_Click — changed from IClassicDesktopStyleApplicationLifetime to MainWindow
Event handler leak on re-dock — PlanLoadRequested is now unsubscribed before re-subscribing when a history tab is re-docked

Improvements

Loading indicator ("Loading plan…") shown during async plan fetch
Context menu items disabled when nothing is selected
ScrollIntoView moved after ItemsSource reset so the target row actually exists
HistoryPlanLoadEventArgs moved to its own file
Unnecessary null-conditional on non-nullable parameter removed
Bare catch blocks replaced with catch (Exception) / catch (Exception ex)

Files Changed

File	Change
`src/PlanViewer.App/Controls/QueryStoreHistoryControl.axaml`	New — AXAML layout for history viewer
`src/PlanViewer.App/Controls/QueryStoreHistoryControl.axaml.cs`	New — chart, grid, tooltips, context menu, plan loading
`src/PlanViewer.App/Controls/HistoryPlanLoadEventArgs.cs`	New — event args for plan load requests
`src/PlanViewer.App/Controls/QuerySessionControl.QueryStore.cs`	Sub-tab hosting, long-press detach, event leak fix
`src/PlanViewer.App/Helpers/TabHeaderLongPressBehavior.cs`	New — reusable long-press helper
`src/PlanViewer.App/MainWindow.Tabs.cs`	Use `TabHeaderLongPressBehavior`, middle-click support
`src/PlanViewer.App/MainWindow.axaml.cs`	Close detached windows on main window close
`src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs`	Thin shell window for detached history
`src/PlanViewer.Core/Services/QueryStoreService.cs`	New `FetchPlanByHashAsync` method

Which component(s) does this affect?

How was this tested?

Describe the testing you've done. Include:

Plan files tested : Query Store
Platforms tested : Windows Only

Checklist

I have read the contributing guide
My code builds with zero warnings (dotnet build -c Debug)
All tests pass (dotnet test)
I have not introduced any hardcoded credentials or server names

1. QueryStoreGridControl.Selection.cs — ViewHistory_Click now walks up the visual tree to find the parent QuerySessionControl and calls AddHistorySubTab(), placing the history as a sub-tab alongside "Query Store — DB" and "QS Overview" (instead of at the top-level MainTabControl). 2. QuerySessionControl.QueryStore.cs — Added two new methods: • AddHistorySubTab() — creates a closeable sub-tab with the history control, including long-press (~500ms) to detach into a free-floating window • DetachHistorySubTabToWindow() — pops the history content into a standalone non-modal window (movable to another screen). When that window is minimized or closed, the content automatically re-docks back as a sub-tab 3. MainWindow.Tabs.cs — Removed the now-unused AddHistoryTab() method since history tabs no longer live at the top level User flow • View History → opens as a sub-tab next to Query Store / QS Overview • Long-press the history sub-tab → detaches into a free window (can move to another screen) • Minimize or close the free window → content returns to the sub-tab

1. QueryStoreService.FetchPlanByHashAsync() — New method that fetches a plan XML from sys.query_store_plan by query_plan_hash. The oldest parameter controls whether it returns the first (smallest plan_id) or last (largest plan_id) plan. 2. Context menu on QueryStoreHistoryControl — Right-clicking on the DataGrid or chart shows: • "Load the First Plan" — fetches the oldest plan for the selected plan hash • "Load the Last Plan" — fetches the most recent plan for the selected plan hash The selected plan hash is determined from either the grid selection or chart selection. 3. PlanLoadRequested event — Raised when the user picks a plan from the context menu. The parent QuerySessionControl subscribes to this event and opens the plan XML as a new sub-tab (using the existing AddPlanTab mechanism). User flow • Select a row in the history grid (or a dot on the chart) • Right-click → "Load the First Plan" or "Load the Last Plan" • The plan opens as a new sub-tab (e.g., "QS 42 / 17")

Bugs Fixed 1. Shared ContextMenu — BuildContextMenu() now creates separate ContextMenu instances for DataGrid and Chart via CreatePlanContextMenu() 2. Silent catch — LoadPlanFromSelection now shows errors in StatusText with catch (Exception ex) 3. Wrong type check — Close_Click now checks is not PlanViewer.App.MainWindow instead of IClassicDesktopStyleApplicationLifetime 4. Event handler leak — AddHistorySubTab now unsubscribes before subscribing: -= OnHistoryPlanLoadRequested before += Improvements 5. Loading feedback — Shows "Loading plan…" / "Plan not found" / error in StatusText 6. Disable menu when no selection — Opening handler disables items when GetSelectedPlanHash() is null 7. IndexOf — Added clarifying comment (list is <500 items, no better API available) 8. ScrollIntoView — Moved after ItemsSource reset so the target row exists 9. Long-press duplication — Extracted TabHeaderLongPressBehavior helper, used in both MainWindow.Tabs.cs and QuerySessionControl.QueryStore.cs 10. HistoryPlanLoadEventArgs — Moved to its own file Nits 11. Removed unnecessary ?. on non-nullable orderBy parameter 12. Changed bare catch to catch (Exception) in tooltip handler 13. Changed bare catch to catch (Exception ex) in LoadPlanFromSelection

…ssicDesktopStyleApplicationLifetime.Windows and closes any that aren't the MainWindow itself. This ensures all detached free-floating windows (from tab detach, history detach, advice windows, etc.) are closed when the app's main window is closed.

erikdarlingdata

Review

Substantial new feature (~1600 lines). Core design is sound; flagging real bugs and UX gaps before merge.

Must-fix

DataGrid numeric sort is broken on every formatted column. QueryStoreHistoryControl.axaml:129-141 — StringFormat='{}{0:N2}' on a DataGridTextColumn with CanUserSortColumns=True causes lexicographic sort on the formatted string ("1,234.50" sorts before "9.10"). The PR description says numeric sort works because the binding is to a numeric property, but Avalonia's DataGrid sorts on the displayed text when StringFormat is in play. Commit 4d4ceff claims to fix this — please verify; if not, remove StringFormat and apply formatting via a converter or template column with SortMemberPath pointing at the unformatted property.
Shutdown race: closing MainWindow → detached window Closing → Dispatcher.UIThread.Post(AddHistorySubTab/CreateTab) runs against a torn-down SubTabControl/MainTabControl. MainWindow.axaml.cs:204-208 closes every other window during OnClosed; the Closing handlers in QuerySessionControl.QueryStore.cs:420 and MainWindow.Tabs.cs:302 re-dock via a deferred dispatcher post. Set a "shutting down" flag before iterating, or unsubscribe the Closing handlers first.
OnClosed is async void with awaits before the window-close loop. MainWindow.axaml.cs:189-211 — exceptions become unhandled, and the close loop runs on a continuation after OnClosed returns. Wrap the body in try/catch.
LoadPlanFromSelection (async void) has no re-entrancy guard. QueryStoreHistoryControl.axaml.cs:1038 — rapid double-click fires two FetchPlanByHashAsync calls and two PlanLoadRequested invocations → duplicate tabs. Disable menu items or set a guard while the fetch is pending.
Long-press timer leak when header is unparented mid-press. Helpers/TabHeaderLongPressBehavior.cs:42 — if the tab is closed (via X button) between PointerPressed and PointerReleased, the timer keeps ticking and fires onLongPress() on an orphan TabItem. Also stop the timer on PointerCaptureLost and header.DetachedFromVisualTree.
_fetchCts only disposed on explicit sub-tab close. QueryStoreHistoryControl.axaml.cs:28,205-210 — leaks the CTS and any in-flight SqlConnection if the host window/session closes without the X being clicked. Hook DetachedFromVisualTree.

UX must-fix

Long-press detach has zero discoverability. MainWindow.Tabs.cs:79-86 is the only entry point — no tooltip, no context-menu item. Add "Detach to Window" to the tab right-click menu (which already has Rename/Copy Path/Close — perfect spot). Same for the QS sub-tabs.
Minimize-as-redock will read as "the app ate my window." MainWindow.Tabs.cs:305-316 — Windows users expect minimize → taskbar, not auto-restore into another window. Recommend dropping the minimize handler and relying on Close + an explicit "Re-dock" action.
Closing MainWindow force-closes detached windows with no session save. MainWindow.axaml.cs:189-211 saves file-based plans but not detached window state. Consider re-docking detached content back into tabs before SaveOpenPlans runs.
Error messages truncated to 80 chars in QueryStoreHistoryControl.axaml.cs:278 and QuerySessionControl.QueryStore.cs:110,131. SQL error context usually lives past char 80 ("...network-related or instance-specific error..."). Drop the truncation or move to a "click for details" link.

Nice-to-have

Tab-header creation is duplicated 5 times across QuerySessionControl.QueryStore.cs (4 blocks) and MainWindow.Tabs.cs. The QS variants are missing Background = Brushes.Transparent on the header StackPanel, which MainWindow.Tabs.cs:70 has — long-press hit-testing on empty header space depends on it.
Detach inconsistency: MainWindow.Tabs.cs:289-292 calls hc.ShowCloseButton(false) on re-dock; QuerySessionControl.QueryStore.cs:407-418 does not. The two paths should match.
"Load the First/Last Plan" → rename to "Load Oldest Plan for This Hash" / "Load Newest Plan for This Hash". Matches the oldest: parameter and removes ambiguity ("first in selection" vs "first in time").
No spinner during loads, no UI Cancel. Add an indeterminate ProgressBar next to StatusText during fetches.
HighlightGridRows does ItemsSource = null; ItemsSource = source (line 769-770) — wipes column sort and scroll position. Iterate visible rows and update Background directly instead.
PixelToCoordinates uses X scaling for both axes (lines 677-682, duplicated at 688-691 and 851-854). Breaks on non-uniform DPI. Extract and fix once.
StatusText "Loading plan…" → "" on success clobbers the data-load summary. Use a separate label or restore the summary.
No keyboard path: no KeyBinding to detach; detached windows don't respond to Esc/Ctrl+W; context menu has no accelerators.
AutomationProperties.Name missing on LegendToggleButton and the color-indicator column.
FetchPlanByHashAsync: orderDir interpolation is safe (literal "ASC"/"DESC"); @planHash is parameterized. Note: when one query_plan_hash maps to multiple query_id rows, TOP 1 ORDER BY plan_id picks arbitrarily — rare but real.

Clean

AvaloniaEdit style include present.
No MVVM creep, no muted color literals, no %LOCALAPPDATA%.
ContextMenu fix is real (two distinct instances).
ScrollIntoView reordering fix is real.
PlanLoadRequested unsubscribe-before-subscribe in AddHistorySubTab is correct.
Designer-only parameterless ctor of QueryStoreHistoryControl is safe.

rferraton · 2026-05-18T18:34:46Z

Sort is working with this PR, the grid was made lexicographic sort before but now it is fixed. May be IA hallucination ?

All the rest is done except point 9 (nothing to save in query history windows)

erikdarlingdata

Re-review after the 4 follow-up commits

Most of the must-fix and UX items are addressed. A few notes below.

Must-fix — status

#	Item	Status
1	DataGrid numeric sort	Verify by clicking — see note below
2	Shutdown race	✅ `IsShuttingDown` flag, checked both before scheduling the Post and inside it (`MainWindow.axaml.cs:44`, `MainWindow.Tabs.cs:285,293`, `QuerySessionControl.QueryStore.cs:339-347` in the redock click handler)
3	`OnClosed` async void	✅ wrapped in try/catch (`MainWindow.axaml.cs:194-224`)
4	`LoadPlanFromSelection` re-entrancy	✅ `_isLoadingPlan` with try/finally (`QueryStoreHistoryControl.axaml.cs:1043,1047,1085`)
5	Long-press timer leak	✅ moot — long-press is gone entirely (replaced by explicit button + menu item)
6	`_fetchCts` disposal	✅ `DetachedFromVisualTree += (_, _) => CancelFetch()` (`QueryStoreHistoryControl.axaml.cs:194`)

On #1 — DataGrid sort: I want to retract some of the certainty in my first review. On closer reading, Avalonia's DataGrid sorts on the Binding's Path via reflection, not on the formatted display string — StringFormat is only the display converter. So the current pattern ({ReflectionBinding AvgDurationMs, StringFormat='{}{0:N2}'}) should sort numerically. That contradicts what my first review said. Worth a quick smoke test: click each column header on a result set where lexicographic vs numeric ordering would differ (e.g., values mixing 9.10 and 1,234.50) and confirm.

UX must-fix — status

#	Item	Status
7	Detach discoverability	✅ "Detach to Window" in tab context menu (`MainWindow.Tabs.cs:101`); `↗` button on QS sub-tabs (`QuerySessionControl.QueryStore.cs:279-294`)
8	Minimize-as-redock confusion	✅ Minimize behavior removed entirely; explicit `📌 Re-dock` button in detached window toolbar
9	Force-close detached windows on app exit	Acceptable design: Close now destroys (with `CancelFetch`), user must Re-dock before quit. Document if you care.
10	80-char error truncation	✅ Removed (`QuerySessionControl.QueryStore.cs:109,130`; `QueryStoreHistoryControl.axaml.cs:281`)

Nice-to-have — status

✅ Tab-header duplication: extracted CreateSubTab helper (QuerySessionControl.QueryStore.cs:42-101); also picks up the missing Background = Brushes.Transparent on the header StackPanel.
✅ PixelToCoordinates Y scaling: fixed; PointToScaledPixel helper extracts the duplication.
✅ HighlightGridRows: no longer resets ItemsSource; iterates GetVisualDescendants().OfType<DataGridRow>() and updates Background directly. Sort + scroll state preserved.
✅ Context menu rename: "Load Oldest/Newest Plan for This Hash".
✅ StatusText data summary: captured in _dataSummaryText and restored after plan load (QueryStoreHistoryControl.axaml.cs:50,266,1078).

New issues introduced

📌 glyph in the Re-dock button content — QuerySessionControl.QueryStore.cs:332 and MainWindow.Tabs.cs:276. Hardcoded pushpin emoji renders unevenly across Windows font fallbacks (and is monochrome on some) and screen readers will announce "pushpin emoji." Consider a text label like "⤵ Re-dock" (already-used arrow style) or just "Re-dock".
Two "Re-dock" code paths are near-duplicates — MainWindow.Tabs.cs:272-340 and QuerySessionControl.QueryStore.cs:332-374 build the same wrapper DockPanel + Re-dock button + Closing handler with a few-line difference. Same kind of duplication you just fixed for sub-tab headers. Worth factoring into a DetachableWindow helper later.
Re-dock click order is fine but subtle — in both paths the click handler does: wrapper.Children.Remove(content) → detachedWindow.Content = null → detachedWindow.Close() → AddHistorySubTab/CreateTab. The Close() synchronously fires the Closing handler which calls content.CancelFetch(). By the time AttachedToVisualTree re-fires on the new parent, any in-flight fetch was already cancelled — the if (_historyData.Count == 0) guard prevents a redundant reload when data is present, but if the user re-docks during the first load, they'll lose progress and the load won't auto-restart. Probably acceptable; flag for awareness.

Still open from prior review

No loading spinner / no in-UI Cancel during fetches.
No KeyBinding for detach; detached windows don't respond to Esc/Ctrl+W.
AutomationProperties.Name missing on LegendToggleButton and the color-indicator column.

Looks good overall. The detach UX is much cleaner now.

rferraton · 2026-05-18T18:40:46Z

rferraton · 2026-05-18T18:55:50Z

emoji removed and spinner + cancel added.
The rest is, i think, decorative

erikdarlingdata

Re-review after `2d6f4ed`

Single commit, three of the prior open items knocked off cleanly.

Addressed

Loading spinner + Cancel — LoadingPanel with an indeterminate ProgressBar and a Cancel button in QueryStoreHistoryControl.axaml:72-77. Visibility toggled in LoadHistoryAsync (set true on entry, false in finally), so it always clears even on exceptions. Cancel_Click → CancelFetch(), which cancels _fetchCts; the existing OperationCanceledException catch sets StatusText.Text = "Cancelled.". Clean.
Detach code duplication — extracted into Helpers/DetachedWindowHelper.cs with onRedock / onClosing callbacks. Both MainWindow.Tabs.cs and QuerySessionControl.QueryStore.cs are now ~15-line call sites. The redocked flag inside the helper correctly prevents onClosing from firing when the close was initiated by Re-dock.
📌 glyph — removed; button is now plain "Re-dock".

New minor notes

LoadPlanFromSelection doesn't use the spinner or pass a CancelToken. QueryStoreHistoryControl.axaml.cs:1056-1063 calls FetchPlanByHashAsync(_connectionString, planHash, oldest) with default token, and doesn't toggle LoadingPanel. So the "Loading plan…" status has no spinner and no Cancel path. Plan fetch is normally quick; flag only if you want full parity with the history fetch.
DetachedWindowHelper.ShowDetached doesn't guard against a Closing-cancelled window. If something external were to set e.Cancel = true on the window's Closing, the Re-dock click handler has already removed content from wrapper.Children, set detachedWindow.Content = null, and would proceed to onRedock(content) — leaving an empty detached window still visible. No code in this PR cancels Closing, so practically harmless; flag for future hardening.

Still open (unchanged)

No KeyBinding to detach; detached windows don't respond to Esc/Ctrl+W.
AutomationProperties.Name missing on LegendToggleButton and the color-indicator column.
DataGrid numeric-sort verification (no change since prior comment — should work but worth a column-header click on real data).

LGTM otherwise.

- Replace deprecated DataGridRow.GetIndex() with the Index property - Remove dead _suppressGridSelectionEvent field; the suppression was only needed for the old ItemsSource-reset highlight path, which was refactored away Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

rferraton added 5 commits May 17, 2026 22:11

Fix sort issues on the QueryHistory

4d4ceff

erikdarlingdata reviewed May 18, 2026

View reviewed changes

rferraton added 4 commits May 18, 2026 19:55

Must-fix fixed

f6ab97c

use pin/unpin button to fix ux-must-fix

a1f8112

fix error truncation to 80 chars : truncation removed

c7e85a4

fix most remaining nice to have

edd9961

erikdarlingdata reviewed May 18, 2026

View reviewed changes

add spinner and cancel for history fetches

2d6f4ed

erikdarlingdata reviewed May 18, 2026

View reviewed changes

erikdarlingdata merged commit a093055 into erikdarlingdata:dev May 18, 2026
2 checks passed

erikdarlingdata mentioned this pull request May 18, 2026

Clean up two warnings from PR #336 #339

Merged

3 tasks

erikdarlingdata mentioned this pull request May 19, 2026

Release v1.11.0 #341

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Query history improvments#336

Query history improvments#336
erikdarlingdata merged 10 commits into
erikdarlingdata:devfrom
rferraton:query-history-improvments

rferraton commented May 17, 2026

Uh oh!

erikdarlingdata left a comment

Uh oh!

rferraton commented May 18, 2026

Uh oh!

erikdarlingdata left a comment

Uh oh!

rferraton commented May 18, 2026

Uh oh!

rferraton commented May 18, 2026

Uh oh!

erikdarlingdata left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

rferraton commented May 17, 2026

Query Store History Improvements

Summary

New Features

Query Store History Viewer

Sub-tab Hosting

Long-press to Detach

Window Lifecycle

Bug Fixes

Improvements

Files Changed

Which component(s) does this affect?

How was this tested?

Checklist

Uh oh!

erikdarlingdata left a comment

Choose a reason for hiding this comment

Review

Must-fix

UX must-fix

Nice-to-have

Clean

Uh oh!

rferraton commented May 18, 2026

Uh oh!

erikdarlingdata left a comment

Choose a reason for hiding this comment

Re-review after the 4 follow-up commits

Must-fix — status

UX must-fix — status

Nice-to-have — status

New issues introduced

Still open from prior review

Uh oh!

rferraton commented May 18, 2026

Uh oh!

rferraton commented May 18, 2026

Uh oh!

erikdarlingdata left a comment

Choose a reason for hiding this comment

Re-review after 2d6f4ed

Addressed

New minor notes

Still open (unchanged)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Re-review after `2d6f4ed`