Release v1.26.0 - #533
Merged
Merged
Release v1.26.0#533
Conversation
Optimizer costs arrive from showplan at full float precision and every display site formatted them itself, so the properties panel and the node tooltips printed "16.765900", "3526.210000" and "0.000000" -- six decimals of noise, and a zero cost dressed up as a measurement. Durations had the same problem from the other end: the statements grid carried its own private ms/s/m ladder that nothing else could reach. MetricFormatter is the one place both shapes live now. Costs: at most four decimals, trailing zeros stripped, thousands separators like the rest of the UI, and exactly zero is "0". The interesting case is a cost small enough to round away at four decimals but not actually zero -- that reads "<0.0001" rather than "0", because a cost that exists must never display as free. The format string is "#,##0.####" rather than "G"/"R" specifically so a huge cost can never fall back to scientific notation in a panel. Durations keep the ladder the statements grid already used (ms under a second, seconds with one decimal under a minute, m+s beyond) rather than inventing a third scale style. Both take an optional IFormatProvider so the tests can pin a culture without mutating global state; production formats in the caller's culture, which is what the "N0"/"N1" rows next to these values do. Human display only. The Robot Advice JSON, MCP tool JSON and plan XML round-tripping keep raw values and do not come through here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
The sites that were printing raw: - Properties panel: "Operator Cost 16.765900 (0%)", "Subtree Cost 3526.210000", "I/O Cost 0.000000", "CPU Cost 0.000000" were all :F6. They now read "16.7659", "3,526.21" and "0", percentage suffix unchanged. - Node tooltips: the same four values, same :F6, same fix. - Statements grid: dropped its private copy of the duration ladder in favour of the shared one, and its cost column moved off :F2 so the grid and the panel beside it group and round the same number the same way instead of showing "3526.21" next to "3,526.21". - Comparison report: estimated cost was :F4 with no separators while runtime and wait times sat next to it as unscaled milliseconds. Cost now goes through FormatCost and the three duration lines through FormatDuration, so a 20-minute runtime reads "20m 35s" rather than "1,235,000ms". The percentage deltas are still computed from the raw values, so nothing about the comparison arithmetic moved. - Web viewer: the operator properties panel and the tooltip builder were on :N4, which is not the six-decimal defect but is a different answer to the same question, and a zero cost still read "0.0000" there. Same formatter now, so both viewers agree. MetricFormatter.cs needed a linked Compile entry in Web.csproj -- the web project links Core sources file by file rather than referencing the project. WriteMetricLine took a format string plus a unit suffix; it now takes a Func<double, string>, since "scale this to seconds" is not something a format string can express. Memory grant deliberately stays fixed at MB on both sides: a per-side scale would put "512 KB" opposite "2.1 GB" on the one line whose entire job is a side-by-side. Untouched on purpose: the Robot Advice JSON, the MCP tool JSON output, plan XML round-tripping and Save .sqlplan all keep full raw precision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
A grouped parent row in the Query Store grid is an aggregate over many plans, so the synthetic QueryStorePlan behind it never gets a QueryId or a PlanId and both come back 0. Query Store ids start at 1, so a column of zeros next to real ids reads like an id rather than "not applicable" -- and both grouping modes produce them at two levels, the root aggregate and the plan-hash/query-hash intermediate. QueryIdDisplay and PlanIdDisplay blank anything at or below zero and the two columns bind to those. Leaf rows are unaffected; they have real ids. The blanking is display only. SortMemberPath still points at the numeric QueryId/PlanId and the column filters still read them through NumericAccessors, so sorting and filtering behave exactly as before -- there is a test pinning that, because binding a column to a string and leaving it to sort as one is the obvious way to break this later. Copy Query ID / Copy Plan ID now disable on a row with no id, following the rule Copy Query Hash and Copy Module Name already use when their value is missing, and the tab-separated clipboard row leaves the two fields empty rather than writing 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
The panel opened at a hard 320px and every subsequent node selection re-ran that assignment, so any width the user dragged out was thrown away on the next click. Set the width only on the transition from hidden to visible, and remember whatever the splitter writes onto the column for the rest of the session, the way the minimap already remembers its size. Default open width is now 380 with the column bounded to 280-800 while open. Those bounds are cleared on close because MinWidth clamps a column regardless of its Width, and a 280px strip on a closed panel is not a panel, it is a bug. The splitter itself was a 5px grey hairline painted in BorderBrush, which at 3840x2400 is both invisible and nearly ungrabbable. It is now 6px and fills with the accent color while the pointer is over it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Seek predicates, probe residuals and output column lists were rendered in the value column of a label|value grid. At a 180px value column a fully qualified column list wraps into a tower of [Database].[schema].[fragment] pieces, one or two per line, and reading one meant reassembling it by eye. Rows flagged isCode now put the label on its own line with the value as a monospace block beneath it spanning all three columns, so the text gets the entire panel width and wraps on something closer to a word boundary. Two supporting changes: The label/value drag handle moves from "created with the section's first row" to "created with the section", because a section can now open with a full-width row that has no label column for the handle to sit beside. It sits at a negative ZIndex so full-width rows own their strip of it; nothing else is ever in the gap column, so it still takes the press over an ordinary row. Values become SelectableTextBlock rather than read-only TextBox, and the panel now records a small row model - label, value, searchable text, and the controls each row occupies - while it builds. The panel is raw controls with no bindings behind it, so once a row is in the visual tree its text is the only thing left to work from, and the filter and copy work landing next both need it. The warning lists register through the same model even though they are prose panels rather than label/value grids. Also gives Template Plan Guide the section header it never had; its two rows were landing in whichever grid happened to be built last. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Every actual metric emitted one indented "Thread N" row per thread, inline, right under its own summary row: rows, rows read, executions, elapsed, CPU, logical reads, physical reads, scans, read-aheads. A DOP 4 hash match already produced about thirty of those rows and DOP 8 produces hundreds, so the handful of summary numbers people actually open this panel for were buried in a scroll marathon. The summary rows are untouched. The per-thread numbers now live in one "Per-thread breakdown" sub-expander per affected section (Actual Statistics, Actual Timing, Actual I/O), collapsed by default, with the threads grouped under a small header per metric. Metrics with no per-thread data are skipped, so a section only grows a breakdown when there is something in it. Rows and executions list idle threads too, since a thread sitting at zero while its siblings work is exactly what someone opens the breakdown to see. The breakdown header carries the skew when there is any: the share test mirrors PlanAnalyzer's Rule 8 so the header never contradicts the Parallel Skew warning the same plan raises, plus an idle-thread test Rule 8 does not make, because a thread that returned nothing at all while its siblings did real work reads as skew on sight even when the busiest thread is under Rule 8's share threshold. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Right-clicking a value produced the default TextBox menu: Cut and Copy greyed out because nothing was selected, and Paste enabled, on read-only data. There was no way to copy a value, a row, or the panel. Every row now carries a menu of its own - Copy value, Copy name and value, and Copy all properties - shared between the row's label and value controls so a right-click anywhere on the row hits it. The warning panels get the same menu. Any context flyout the theme attached is cleared, so the stock menu cannot come back. Copy all properties renders the whole panel from the row model rather than walking the visual tree: the operator header, then a line per section with its rows indented under it, and code values verbatim on their own lines so a predicate or a CREATE INDEX pastes as-is instead of arriving re-indented. Clipboard writes go through the existing SetClipboardTextAsync, which carries the retry for a clipboard another process has locked (#415). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
An operator with actual stats runs well past sixty rows across a dozen sections, and the only way to find one of them was to scroll and read. A filter box sits under the panel header and matches, case-insensitively, against both the label and the value of every row, code blocks included. Non-matching rows hide, and a section hides outright once nothing in it is left showing. A section whose own title matches keeps all of its rows, so typing a section name jumps to that section instead of emptying it. The text is sticky across node selections - it lives in the header chrome, which the rebuild does not touch - and is re-applied every time the panel rebuilds. Esc clears it while the box has focus. Empty sections now hide with no filter text at all. A few can be built with no rows (Operator Details when the only property that qualified the node contributes no row of its own, for one), and an expander with nothing inside it was never worth a line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Two defects an Avalonia review of this branch turned up, both verified against 11.3.20 in a headless harness rather than by reading docs. Ctrl+C on a property value was no longer crash-safe. SelectableTextBlock registers its own CopyingToClipboard routed event rather than sharing TextBox's, so the app-wide TextBoxClipboardGuard stopped covering these values the moment they stopped being read-only TextBoxes, and SelectableTextBlock.Copy() is async void over an unguarded SetTextAsync - the exact shape that crashed the app when another process held the clipboard (#415). Every value now handles that event and goes through ClipboardHelper, so the panel's Ctrl+C and its copy menu take the same guarded path. The splitter's hover was wired in code-behind, which pinned Background to a plain brush at local-value priority the first time the pointer left it, permanently severing the {DynamicResource BorderBrush} the AXAML asked for, and hard-coded the accent color a second time. Both states are Setters on the splitter now, resting and pointer-over, resolving BorderBrush and AccentBrush from the theme. The Background attribute had to go with it: a local value outranks a Style setter, and with it in place the hover never fires at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
"A task was canceled." — the raw message off an OperationCanceledException —
was landing in the query session's toolbar strip with autoClear: false and
staying there. Switching sub-tabs mid-fetch is the ordinary way to produce
it (the fetch is torn down, the exception comes back, the catch prints it),
and the message then followed the user across every view they visited
afterwards, reading as a complaint about whichever one they had moved to.
It only went away when some later action happened to overwrite it.
Three changes, all in QuerySessionControl:
- Cancellation is never reported. SetStatusFromException drops
OperationCanceledException (TaskCanceledException derives from it) and
shows nothing; the Query Store, schema and format catch sites go through
it. The two explicit cancel catches in the execution paths lose their
"Cancelled" message for the same reason — the cancel was the user's own
(Escape, the Cancel button, or starting the next query) and the spinner
tab vanishing already says so. That line was also about to become
invisible anyway: removing the selected tab moves the selection, which
now empties the strip.
- Failures look like failures and do not last forever. SetErrorStatus paints
the strip with a new ErrorBrush token (#E06C75, the muted red the plan
viewer's load error already uses) and clears after 12 seconds instead of
never. Every former autoClear: false error site uses it, as do the error
messages that were already auto-clearing at 3s, so the strip now reads
one way: red means the session could not do what you asked. Progress and
success messages ("Formatting...", "plan captured", "Loaded Indexes for
X") keep the ordinary foreground and the 3s clear.
- A message cannot outlive the view it was about. The strip empties when
the active sub-tab changes, and when the session leaves the visual tree
(a top-level tab switch) — the latter is where the old code cancelled the
pending clear without taking the text down, so anything showing at that
moment was parked permanently by construction.
Two deliberate exceptions. The schema lookup's "Fetching Indexes for X..."
keeps autoClear: false: it is progress for an operation that may take a
while and is always replaced by its own completion or error line. And the
Query Store read-only replica notice is now red rather than persistent — it
explains why nothing opened, which is a failure the user needs to read, and
12 seconds beats both the old 3 and never.
QueryStoreOverview's "Loading..." moved to after its tab is selected, since
selecting a sub-tab now clears the strip and would have wiped a message set
ahead of the switch.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
The Compare Plans button in the toolbar over every window-level plan tab was built enabled and never re-decided. With one plan open, clicking it ran ShowCompareDialog's early return — no dialog, no message, no cursor change, nothing — which reads as a broken feature rather than as a missing precondition. The query session's copy of the button had been taught to count plans window-wide in #447; this one was left out of that fix. Both are now driven from the same count. RefreshComparePlanAvailability — already called by the tab watcher whenever a tab, sub-tab, or tab's content changes anywhere in the window — walks plan tabs as well as sessions, and ComparePlansButtonState applies the answer to either shape, including the tooltip: "Open a second plan to compare" when it is disabled, the old "Compare any two plans open in this window" when it is not. A disabled control that does not say what would enable it is only half an improvement over one that silently does nothing. Details worth naming: - The code-built button carries a Name so the refresh can find it again; this toolbar is rebuilt per plan tab, so there is no field to hold. - It is also born with the honest answer, because LoadPlanFile builds the toolbar before adding the tab that would trigger a refresh. - Detached plan windows are refreshed too: their button still opens the main window's picker, which cannot see the plan that left, so a window detached while a pair existed used to keep an enabled button afterwards. Detached query sessions stay out of that loop on purpose — #447 has them answer from their own plans, since their button falls back to their own picker. - The early return stays as a guard and is now unreachable from the UI, which the comment there says. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
The "Query Editor" header sets no FontSize, so it inherited Avalonia Fluent's TabItem header default of about 24px, while every sibling sub-tab header — plan tabs, Query Store, QS Overview, schema tabs — is built in code with FontSize 12 (CreateSubTab and AddPlanTab). The strip read as two controls jammed together: one outsized word, then a row of small ones. Pinned at 12 to match, which is the whole change. Vertical alignment was already Center on both, and the ZoomBox sharing that header sets its own FontSize of 11 and is centered too, so the row still lines up. The tab model itself is untouched — restructuring it is a later phase. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
A plan opened as a sub-tab inside a query session rendered its own toolbar row with a Reconnect button, a server label and a Database combo — directly underneath the session toolbar that already carries those three for the same connection. Two stacked toolbars saying the same thing, and the inner Database combo permanently disabled, because an embedded viewer inherits ConnectionString from its session and never populates a database list of its own. PlanViewerControl gets a HostedInSession flag that collapses that group. QuerySessionControl sets it at both places it builds an embedded viewer (AddPlanTab, and ShowCapturedPlan for an executed query). Standalone .sqlplan tabs and pasted plans are unchanged: nothing sits above them, so their connection controls are the only ones there are. Everything plan-scoped stays put in both modes — zoom in/out, Fit, the zoom readout, Save .sqlplan, Statements — and schema lookups from the embedded viewer's context menu still work, since they read ConnectionString rather than the hidden controls. The trailing separator moved inside the collapsed group so hiding it does not leave an orphan pipe at the left edge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
A new Query tab was a wall of empty editor. Everything it can do sat behind a menu, and nothing on screen said so — the three things a user opens that tab for (open a plan, paste plan XML, connect to a server) were invisible from the place they are wanted. The editor tab now carries an empty state over the editor, centred and quiet: a "Get started" heading, those three actions with the shortcuts the File menu actually binds, and up to five recent plans with their folders. Every row is wired to the existing handler rather than a second copy of it — OpenFile_Click and PasteXml_Click (now internal, like NewQuery_Click already was), the session's own Connect_Click, and a new OpenRecentPlan extracted from the Recent Plans menu handler so a file that has been moved is reported and forgotten identically from both places. It cannot trap typing, which was the design constraint. The editor keeps focus underneath, a click anywhere on the panel hands focus back to it, and the panel is gone the moment the session holds anything: RefreshEmptyState shows it only while the editor is empty AND no sub-tab but Query Editor exists, and runs from the editor's TextChanged and the sub-tab watcher so it re-decides in both directions. Deleting the last character with nothing else open brings it back, which is the same state a fresh tab is in. Two things worth knowing for later: - Every clickable row sets Background="Transparent" itself. A control with a null background hit-tests only the glyphs it draws, so presses in the gaps between words would fall through. That local value outranks a style setter, so hover shows in the text colour rather than behind it. - The session is told its window by CreateTab rather than looking one up. FindLogicalAncestorOfType only answers once a session has been realised, and a tab that opens behind the selected one never is — its empty state would have been built with no recent plans and never rebuilt. Detaching unsets it, which also hides the two File actions that would have had no window to act on; redocking passes back through CreateTab. The File menu's two plan items gained x:Names so a test can hold their gestures against the shortcuts the panel prints, since that is a copy and copies drift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Opening Settings and clicking Cancel always asked "You have unsaved changes. Discard them?" even when nothing was touched. ShowSection builds each section's controls and subscribes dirty handlers to ValueChanged/SelectionChanged/TextChanged/PropertyChanged. Those events also fire while the controls initialize themselves: NumericUpDown syncs text and value when it is templated, and the format DataGrid's two-way cell bindings write back into FormatOptionRow as rows are realized. Both happen on the layout pass after ShowSection returns, so the dialog was dirty before the user did anything. Add a _building guard that the handlers (now routed through MarkDirty) honor. It is set around the section build and released by a Background priority dispatcher post, once templating and binding have settled; a build token keeps an older section's post from releasing a newer one. Reset Section and Reset All still set _isDirty explicitly, so cancelling after a reset still prompts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Settings was opened with Show(this), which parents it to the main window but leaves the main window fully interactive. With Settings open you could still drive the menu bar behind it, and the unsaved-changes prompt that Settings owns was drawn under a Help menu opened from the main window. Open it with ShowDialog(this) instead. The discard prompt inside SettingsWindow was already ShowDialog-ed onto SettingsWindow, so making the parent modal closes the whole interaction leak. Nothing needs Settings to be modeless - it only reports back through SettingsSaved, which fires on Save - and the existing single-instance guard stays as a belt-and-braces check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
The Query Store grid passed the CLR property name to the filter popup, so filtering the "Query Hash" column opened a popup headed "Filter: QueryHash", and "Module" read "Filter: ModuleName". SetColumnFilterButton already knows both the column id and the header label it renders, so record the label in _columnLabels and hand it to ColumnFilterPopup.Initialize as a separate display name. The column id still drives _activeFilters keying and RowMatchesAllFilters, so filter behaviour and the server-search promotion are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Connect was disabled until Test Connection had been clicked, because only the test enumerated databases and enabled the button. The disabled button gave no reason, and in a live session the two-step order was not guessable - people filled the form, clicked Connect, and nothing happened. Connect is now enabled by the fields a connection actually needs: a server name, plus login and password when SQL Server authentication is selected. Clicking it runs the connect and database enumeration inline through the same ConnectAndLoadDatabasesAsync the test button uses, then saves the credentials and connection and closes with ResultDatabase set from the Database dropdown, the typed initial database, or master. Failures land in StatusText and the dialog stays open, so the error is attached to the attempt. Test Connection keeps its old behaviour as an optional way to browse databases first, and credentials are now only persisted once a connection has actually succeeded. The dialog stays editable while a connection opens, so Connect captures the connection, login, password and typed database it is about to validate before awaiting, and checks the dialog is still open afterwards. Without that, cancelling mid-connect still wrote a credential, and editing the server name mid-connect would save a server that was never tested. The handlers that XAML can raise during loading also guard the named fields they touch, since a selection set in XAML (EncryptBox already has one) fires before those fields exist. Callers that reconnect an already-connected session (QuerySessionControl and PlanViewerControl) pass their current database, which is pre-selected once the list loads so a reconnect returns to the database in use instead of dropping to master. The dialog still never connects on its own. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Double-clicking a results row only expanded or selected it, so loading a plan meant right-click then "Load Plan", or ticking a checkbox and using Load Selected. Double-click is what people try first. ResultsGrid_DoubleTapped now hands off to LoadHighlightedPlan_Click, the same handler the context menu's "Load Plan" item is wired to, so both routes load the same plans through the same PlansSelected event. Grouped header rows have no plan of their own and keep expanding and collapsing on double-click. The existing guard still applies: double-clicks that land on a Button are ignored, which covers the expand chevron and the select checkbox (CheckBox derives from Button), so single-click selection, checkbox multi-select and the Load Selected button are untouched. That guard now searches with includeSelf, so a press that lands on the button itself rather than inside its template still bails. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Two defects in the close path the modal Settings change leans on, found reviewing that change. TryClose is async void and had no re-entrancy latch, so two close attempts stacked two discard dialogs and two pending awaits. On Windows the nested ShowDialog disables the Settings window and hides it; X11 and macOS modality is weaker and this app ships there. Add the same _closeWalkInProgress latch MainWindow already carries for its own cancel-close-then-reclose walk, cleared in a finally so a cancelled prompt leaves Cancel and the X able to start a fresh one. OnClosing keeps cancelling every close unconditionally; the walk's own Close clears _isDirty first, so it sails through rather than being swallowed. The discard dialog also completed its TaskCompletionSource from Closing, which an owner-driven teardown never raises - that left the await suspended forever. Complete it from Closed instead, which always fires. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
SelectableTextBlock registers its own CopyingToClipboard routed event rather than sharing TextBox's, so the #415 guard never covered it - and its built-in Copy() is the same async void over an unguarded SetTextAsync that crashed the app when another process held the clipboard. One class handler covers every SelectableTextBlock in the app, including the unguarded ones in the advice views, execution results, and missing-index panels, and any created later. Found by the properties-panel workstream while fixing the same gap inside its own fence; this is the app-wide half it could not reach. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Two changes in this branch composed into silent error loss: plan load failures became auto-clearing error statuses, and sub-tab selection began clearing the status strip. A bad plan in the middle of a multi-select batch reported its error, the next plan's tab selection wiped it, and the all-loaded summary was suppressed because not all plans loaded - two tabs where three were asked for, and an empty strip. The batch now collects each failure and writes one authoritative status after the loop - the only message that can survive the selections the loop itself makes. Found by the integration review's Avalonia pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
- The posted status-clear now re-checks that its own timer is still the live one, so a clear that lost the race to a newer message (possibly a 12s error) can no longer wipe it the moment it was posted. - The Query Store results grid is IsReadOnly: its bound columns only ever showed get-only display properties, but with double-click now the primary load gesture, the second click was also starting a useless cell edit over the navigation. The chevron and checkbox live in cell templates and stay interactive. - The empty-state action rows only respond to the left button; a right- or middle-click opening the file picker is a surprise, not a shortcut. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Initialize called Focus() before the popup opened, and a control with no visual root focuses as a silent no-op - the filter popup opened with a dead keyboard every time. The dialogs workstream flagged this rather than fixing it blind, since a wrong fix is the same no-op with extra steps and only a live session can tell them apart. Verified at runtime both ways: keystrokes after the funnel click went nowhere before, and land in the value box now. Focus is posted at Loaded priority after IsOpen = true, so the popup's child has a visual root by the time it runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Opening the same .sqlplan repeatedly stacked identical tabs - the live review ended up with three tabs all named another-parallel-spill.sqlplan and no way to tell them apart, and the tab strip wraps into extra rows that much sooner. Reopening now selects the tab that already shows the file (matched on SourceFilePath, case-insensitive full path) and still bumps it in Recent Plans. Session restore comes through the same path, so a restore list that accumulated duplicates heals to one tab per file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
ShowError built a window with no way out but the titlebar X: no button, no Esc handling. It is owner-modal, but Alt+Tab can activate the owner past it, at which point it parks over the app indefinitely - observed live with the pasted-plan XML error. IsDefault/IsCancel on the one OK button makes Enter and Esc both close it, and it takes focus on open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Phase 1 UI/UX fixes from the live review
Colour in this app had no single home. Controls reached for inline hex
(#4FA3FF, #9B9BFF, #7BCF7B and #E06C75 are all hardcoded in
PlanViewerControl.axaml), code-behind reached for Brushes.LimeGreen, and
AdviceContentBuilder built its own private SolidColorBrush fields. The
result is the same intent painted three slightly different ways with no
way to re-theme any of it, and no way for a reader to tell a deliberate
one-off from a copy-paste.
Add the tokens those ad-hoc colours were standing in for, and group and
comment the file so it reads as a registry rather than a pile:
SurfaceHoverBrush one step above BackgroundLightBrush, hover fills
AccentSoftBrush AccentBrush at 12% alpha, for pressed/selected
WarningBrush the amber already used ad hoc for warnings
SuccessBrush a calmer green than LimeGreen that still reads "ok"
ErrorBrush the red already inlined in the plan surface
Insight{Server,Index,Params,Waits}Brush the four insight-panel hues
Every pre-existing key keeps its name and value byte for byte, so this
commit cannot move a pixel on its own. Migrating the call sites onto
these keys is the plan-surface and dialog work, not this file's.
The header says the part that matters for next time: inline hex in a
control is a defect, and new shades get added here first.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
AppButton painted its whole face with AccentBrush on :pointerover and flipped the label to white. Every mouse pass across a toolbar flashed cyan, which is loud on its own, but the real cost showed up live: a button was seen holding an accent fill after it opened a view, looking for all the world like a toggle that had latched on. There is no toggle code anywhere near it. A solid accent fill simply reads as "selected", so any pseudo-class that outlives the gesture that set it - :pointerover surviving a pointer that left while a window was opening - invents a state the app does not have. The fix is to make no state capable of looking selected: pointerover SurfaceHoverBrush + accent border, foreground untouched pressed AccentSoftBrush (12% accent) + accent border focus-visible accent border, nothing else disabled unchanged at 0.4 opacity Nothing fills with a solid accent now, so a stuck hover degrades to a slightly lighter button with a lit edge - legible as hover, never as on. Dropping the white-foreground rules also removes a contrast trap: the label no longer has to work against two different backgrounds. The focus-visible rule is new rather than restored. This ControlTheme replaces Fluent's Button template outright and takes Fluent's focus adorner with it, so keyboard users previously got no focus visual at all on the app's only button style. Giving focus an explicit, bounded appearance is also what keeps the emergent-fill class of bug closed: every visual a button can wear is now written down here. Template structure and the 4px corner radius are untouched, so the code-built toolbars that pull this theme by FindResource inherit the new states without changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
The plan view was showing three horizontal scrollbars stacked at its bottom edge at once. That is not a bug in the plan view: Fluent reserves layout space for any scrollbar whose AllowAutoHide is false, the app nests scrollers, and this file had turned AllowAutoHide off globally. Every nested scroller therefore charged its pane a permanent 16px strip. This reverses the MECHANISM of #464 while honouring its motive, which is worth spelling out because the two complaints are mirror images. #464 reported that a scrollbar "is a couple of pixels tall until after you have found it". That is accurate and it is Fluent's fault: Fluent collapses an idle bar by scaling the thumb to 0.125 of a 16px bar, which leaves 2px of rail. The fix available at the time was to stop it collapsing at all, and always-expanded bars are certainly grabbable. They are also permanent furniture, and the stacking above is the bill. Neither "always expanded" nor "auto-hide" is the right answer, because the real defect was never auto-hide - it was 2px. So the geometry is redefined rather than inherited: ScrollBarSize 16 -> 14 idle thumb scale 0.125 -> 0.5 idle rail 2px -> 7px 7px of rail is drawn at all times and opens to the full 14px under the pointer. #464's reporter can still see and grab it - at 250% scaling it is 17 physical pixels - and with auto-hide back on, ScrollViewer's own theme spans the content presenter across both scrollbar cells, so bars overlay the content instead of displacing it. The panes get their strips back and nothing stacks. Those two resources are a pair: the rail is ScrollBarSize times the scale, and changing either alone walks straight back into #464. Both the comment and the test say so. ScrollBarVisibilityTests is rewritten to pin the new contract rather than the old one, and carries the whole history so a future reader can see why the pinned behaviour changed twice. It measures thickness off controls that have really been through layout, not off this file's text. Two limits are recorded there honestly: the pointer is not simulated, because headless hit-testing needs a renderer this suite has no Skia for, and the DataGrid case checks only the AllowAutoHide plumbing, because a headless DataGrid measures its rows to zero height, concludes it does not overflow, and never templates a bar to measure. Grab feel at high DPI is a live re-drive question and is flagged as one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Plan documents are opened by three paths - the Query Store/file path and the two execute paths - and only the first built the context menu. The execute paths built their own header and attached nothing, so a plan opened by running a query had no Rename, no Close Other Tabs and no Close All at all: right-clicking it did nothing. Nothing revealed it, because the header looks identical either way and the close button still worked, and executing a query is the commonest way to open a plan, so the menu was missing exactly where it was most expected. Pre-existing, not from this branch (dev has the same three headers and the same one menu). Found by right-clicking a plan tab during the wave-B re-drive; the header now comes from one builder, so a fourth path cannot forget it. Also gives the header a transparent background, which is what makes the whole header rect a target rather than just the label's glyphs - the trap the empty-state rows hit before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Session IA: views beside a documents-only strip
One decimal rounds anything from 99.95 up to "100.0", so a plan that went from 4.7s to 2ms reported "100.0% faster" - which reads as "took no time at all" rather than as the near-total win it was. A value below 100 now rounds down to 99.9 instead of up to a claim the numbers do not support; 100 and above are untouched, as is every other percentage. This is the wording Erik left open when the comparison diff shipped, now ruled toward accuracy. It changes the MCP compare_plans text as well as the window chip, because both read the same formatter - the frozen baseline moves 18 lines, every one of them a genuine sub-100 value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
…ounding Stop a 99.9% win printing as 100%
Two wave-B carry-forwards. ShowOverviewAsync has three ways in and all of them are fire-and-forget: two async void handlers and a discarded task. Its inner catch covers the load, but anything thrown around that - the connection dialog, building the view, the surface flip - escaped into an async void and took the process with it. Guarded once in the shared implementation rather than three times at the callers. The new test covers a plan arriving in a document whose header has been scrolled out of the strip, which is the state where a virtualising header panel would differ. Worth recording what measuring that found: swapping in a VirtualizingStackPanel does NOT pass the suite. Four surface pins fail on it, none of them written for it, so the panel choice was already guarded and the concern that it was invisible was unfounded. The test stays as a scenario, and says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
The view bar and the Overview Total/Avg toggle were the same forty lines of ControlTheme in two files - same template, same four style selectors, byte for byte - differing only in padding, font size and height. Those three are properties a caller can set on the RadioButton itself, so the shared theme moves to DarkTheme.axaml beside AppButton and each site sets its own three. Net thirty-two lines lighter, and a segmented control now has one place to change rather than two that can drift apart. Verified on screen as well as in the layout pins: both bars render exactly as before, latched segment included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Wave-B carry-forwards: Overview guard, scrolled-out coverage, one segmented button
The ladder started at whole milliseconds, so 0.4ms rounded to "0ms" - reporting a query that ran as one that did not. The Query Store Overview already knew this and kept its own copy of the fix, which is the usual sign the knowledge is in the wrong place: Query Store hands out microseconds, and anything else that starts reading them would have had to rediscover it. A double overload now carries the two rungs below a millisecond and hands everything else to the existing long ladder unchanged, so whole milliseconds read exactly as they always did. The Overview delegates and drops its copy. Pinned both ways, including that the two overloads agree on every whole number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Teach the shared duration ladder about sub-millisecond times
Twenty-three wait categories had colours chosen one at a time, and it showed. Three failed the 3:1 contrast floor for a graphical object against the page - Buffer IO sat at 2.00, a blue so dark it was effectively a hole in the chart. Buffer IO and Parallelism shared an identical hue. Three oranges sat within 9 degrees of each other, three pinks within 28, and six near-grey categories were separated by nothing but lightness, two of them closer than the eye reliably splits. Now hue carries the family - greens for compute, blues for storage and network, warm for contention, grey for the buckets that mean "no information" - and within a family the categories climb a lightness ladder. That second part is the one that matters for a red-green colour deficiency, where the hues inside a family collapse and lightness is all that is left. Every colour clears the contrast floor, nearest neighbours are 12 apart in CIELAB with normal vision and 6 under simulated deuteranopia, with no outliers below that. Twenty-three categories cannot all be distinct by colour alone under that condition, which is what the legend is for; the ladder keeps neighbours in one chart apart. The plan viewer had a second, smaller scheme of its own drawn from the status triad: Lock in the error brush, Network in the success brush, I/O in the warning brush. That judged - a lock wait is not an error - and it coupled a wait category to a token meaning something else, so repainting "error" would have repainted "Lock". It also put red and green on two categories in a tool for DBAs. Those six now use the identity palette, which is what that palette asked for: one wait type, one colour, everywhere. Verified on screen against SQL2022 as well as by measurement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Lay the wait palette out instead of picking it
FileAssociationService's summary still said macOS double-click "launches the app without the plan" because Avalonia 11.3.17 did not expose FileActivatedEventArgs. That stopped being true when App.OnAppActivated was wired up: the app subscribes to IActivatableLifetime.Activated and routes the activated paths into the same MainWindow.OpenFiles the Windows and Linux argv paths reach. A reader trusting the old comment would conclude the macOS open path is missing and either rebuild it or drop a working feature. Say what the code does, and keep the caveat that is still real: it has never been run on a Mac. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
…mment Correct the macOS file-association comment
The SegmentedButton ControlTheme declares no BasedOn, so it replaces Fluent's RadioButton theme outright rather than layering onto it. It defines pointerover, checked and focus-visible, but never disabled -- and there is nothing left underneath to supply one. A disabled segment would render at full opacity, indistinguishable from an enabled one. Nothing disables a segment today: the four instances (the session view bar's Editor/Overview, the Overview's Total/Avg) are never written to. That is why the gap is worth closing now rather than when someone adds the first IsEnabled binding and cannot see why the control lies. Mirrors AppButton's own disabled rule, which sits forty lines above it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Give SegmentedButton the disabled state it never had
Two findings from the UI review that the campaign never reached. V7, the minimap. It opened as a 400x400 panel pinned to the canvas's top-left, which is where a plan renders its root operator and first few nodes -- the part a reader most wants unobstructed. It now opens at 220x220 pinned bottom-right, inset far enough to clear the overlay scrollbar rails, with the panel sitting above the toggle that summons it so neither covers the other. The toggle was the literal word "minimap" at 9px, which read as a label rather than a control; it is now an icon button. AppIcons.Minimap was drawn for this button during the design-system pass and left unwired, so the icon already existed. Because the panel is pinned bottom-right, its free corner is the top-left one, so the resize grip moved there and the drag math inverted. The drag is also re-based onto the control rather than the panel: a panel pinned bottom-right moves its own origin as it grows, so in panel coordinates the grip would sit still while the pointer moved and the drag would fight itself. V11, settings consolidation. The MCP server port and the proxy configuration lived in the About box. They are settings, not facts about the build, and nobody goes looking for a port number behind "About". They are now a Settings > Integrations section and About is back to version, copyright, links and the update check. The section follows the dialog's Save/Cancel model rather than About's save-on-every-change, so nothing reaches settings.json or the credential store until Save, and Reset Section and Reset All both cover it. Unlike the other sections it holds its own pending state instead of being read back off its controls at save time, because the pending values have to survive navigating away and back, which an orphaned control's value does not. Two things found while doing it, both fixed here. Saving Settings without visiting Query History silently wiped both of its values. Sections build their controls only when first opened, and Save read them back unguarded, so a null combo read as "" and a null spinner as a hardcoded 10: changing a Query Store option and pressing Save reset a configured max-plans of 50 to 10 and blanked the default metric. Format Options already guarded against exactly this. Pinned by a test that fails with Expected "TotalCpuMs", Actual "" without the guard. SettingsFile had no test-host redirect, unlike AppSettingsService, which has one because tests once rewrote the developer's real config. This change puts a save path to that file behind a UI a test can drive, so it is redirected now too -- before something writes a real settings.json and credential store from a test run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Ten findings, nine acted on. The two that matter most were regressions this branch introduced. The resize grip covered the plan's root operator. Moving the panel to the bottom-right put its free corner -- and so the grip -- at the top-left, which is exactly where PlanLayoutEngine draws the root. The grip declares a transparent background and handles the press, so on any plan scaling below ~0.074 (roughly 23 leaves, which is routine) it swallowed the root's whole rect and click-to-center on it stopped working. The grip now lives in the panel header: same corner, no plan content underneath. A lost pointer capture left the drag armed forever, and inverting the math made that far worse. Nothing handled PointerCaptureLost, and the move handlers only ever checked the "am I dragging?" flag, so an alt-tab mid-drag left it set and a later plain hover resized the panel against a stale origin. Before the inversion a stale drag shrank the panel to its floor; after, it grew it to 500 and swallowed the canvas. Both the grip and the canvas now clear their flag on capture loss, and the move handler bails when the button is no longer down. Saving the proxy could delete a password nobody touched. ProxySettings.Load swallows a credential-store failure and reports no password, which is indistinguishable from there being none -- and the inherited rule "touch the credential unless the box is empty AND one is stored" then deleted it. Moving the settings here widened the trigger from a proxy field losing focus to any save at all, so ticking the MCP checkbox was enough. The store is now touched only on a positive instruction: a typed password, or a Reset asking for the stored one to go. Reset says so now rather than leaving the credential behind and the watermark claiming it is saved. Also: the MCP and proxy writes are split so one cannot drag the other to the credential store; the pending-state handlers go through a guard that drops build-time events instead of silently overwriting the value just loaded; four handlers read their own control rather than a field that gets reassigned on every rebuild; and Copy MCP command says so when it hands over a port that is not saved yet. Reset All wrote stale values back for every section not on screen -- the same root cause as the Query History bug this branch already fixed, and the review was right that it belonged in the same change. It replaced the settings object but rebuilt only the visible section, so the other sections' controls survived holding pre-reset values and Save read them back over the defaults. Every section read is guarded now, and Reset All drops the cached controls so the defaults survive. Pinned by a test that fails with Expected 30, Actual 90 without the fix. Tests: 700 passing. The dirty-flag test now asserts the dirty flag, and closes without leaving an unanswered modal in the shared headless app. The file's note about credential-store safety now says what is actually true -- the JSON is redirected, the credential store is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
The test-host redirect added on this branch only covered one direction. McpSettings.Load rebuilt the real ~/.planview/settings.json path itself instead of going through SettingsFile, so under test it read the developer's own file while every write went to the temp one. A split like that is worse than having no redirect at all: the isolation looks present, and the disagreement only surfaces when a value read back contradicts the one just written -- in exactly the section this branch adds coverage for. ProxySettings.Load, sitting beside it in the same dialog, already went through SettingsFile. Reading each key separately rather than in one try block, so a malformed or wrongly typed value costs only itself instead of taking its neighbour back to a default with it. Both behaviours are pinned. Found by the CI review on the second pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Reset All is one unconfirmed button reachable from every section, and the previous commit made it stage deletion of the saved proxy password as a side effect of resetting anything at all. Someone restoring the Query Store defaults would lose a credential they never came here for, with nothing on screen to say so. Deleting an OS credential is the one thing in this dialog that Cancel cannot really undo: the setting comes back, the password does not. So Reset All still returns the proxy configuration to defaults -- mode, address, username, and the MCP keys -- but leaves the credential where it is. Reset Section, pressed while Integrations is on screen, is the deliberate gesture that removes it. This is the same bug I introduced while fixing the review's earlier complaint that Reset left the credential behind: the fix was right for Reset Section and wrong for Reset All. Found by the CI review on the third pass. 703 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
Two JSON stores were redirected for tests; the third store was not. Proxy passwords live in the OS credential manager, and the Integrations save path can delete one, so the only thing standing between a test run and the developer's real saved password was nobody having written that test yet. The file's own doc comment said as much, which is a bad thing for a comment to have to say. CredentialServiceFactory now takes an in-memory service for the test host, reusing the implementation Linux already gets. One shared instance, so a save and a later load inside a run agree with each other. The app never calls it; unset, Create resolves by platform exactly as before. That makes the credential path safe to exercise, so it is exercised: an ordinary save -- MCP toggled, proxy address edited, password box left empty -- must leave the stored password where it is. Mutating TouchCredential to an unconditional true fails that test. Worth being precise about what it does not cover. The original bug needed a credential store that fails its reads, because a swallowed read failure is indistinguishable from an empty store and that is what made the old rule delete a live password. An in-memory store cannot fail, so the test guards the conclusion rather than reproducing the cause, and says so. Found by the CI review on the fourth pass. 704 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
…nd-integrations Move the minimap into a corner and settings out of About
Sections build their controls the first time you open them and throw them away on the way out, and Save read the values back off whatever controls happened to exist at the end. So an edit lived only in its control: leave the section, come back, and a fresh control was built from the unchanged settings with the edit gone -- while the dialog still believed itself dirty and offered to discard changes that no longer existed. The direction is inverted now. Every section writes its edits into the settings as they happen, and Save reads no control at all; its job is to validate, apply and persist what is already there. That retires the shape behind three separate bugs this branch has now fixed: values read from sections the user never opened, values read from sections rebuilt since, and edits lost by leaving a section. Commit() replaces MarkDirty and carries a build token. Reading at save time made a stale control harmless, because nobody asked it anything; writing on change does not, so a handler whose section has been rebuilt recognises itself and stays out of the way. Both constructors clone. The settings object is the draft now rather than a staging area touched once at the end, and the parameterless constructor was handing back AppSettingsService.Load() -- the instance cached process-wide -- so a single keystroke would already have changed the app's settings and left Cancel nothing to undo. Nothing uses that constructor today, which is why it was quietly waiting. Colours only ever grow. Trimming the stored list to the current database count looks like tidying and is data loss: the list is the only record of the palette, so dropping entry 8 because the count is 5 throws away a colour the user picked and raising the count back hands them a stock one. Entries past the count cost nothing and are what make the trip down and back free. Save judges only the rows in play, since a bad colour with no row on screen would be a refusal nobody could clear. Invalid colours also stop being a silent refusal. Validation reads the settings rather than the text boxes, so saving from another section brings the offending row forward and marks it, instead of reddening detached controls nobody can see and declining to close. Also here: the eight per-control fields the old save path needed are gone, along with four more that were already write-only, and one test that leaked a Manual proxy into the shared settings file now seeds its own starting state instead of depending on which order the suite ran in. 708 tests. The round trip, the colour round trip and the clone are each pinned by a test that fails without its fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
CommitFormatOptions rebuilt FormatOptions from a fresh SqlFormatSettings and skipped any row it could not parse, so a skipped row came back as the type's default rather than the value it had. At save time that was nearly unreachable: you had to press Save with a field mid-edit. Committing on every keystroke makes it ordinary. An int field is unreadable for exactly as long as it is empty, which it is the instant you select its contents to retype them -- so clearing it dropped the default into the draft right then, and navigating away rebuilt the section from that. A value silently reverting because you retyped it is the bug this whole change set exists to stop. Seeded from what is already stored instead, so a row that cannot be read right now keeps what it had and only rows that parse overwrite it. Found by the CI review. 709 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
…nd-trip Let a Settings section keep an edit you navigate away from
The last two findings from the UI review, both left half-done. U7. Phase 2 bounded the button states so nothing could look latched by accident -- a button that lost the pointer mid-click used to keep :pointerover, and a full accent fill read as "on" when no toggle state existed anywhere. That fixed the false positives and left no true ones: Statements and the minimap show and hide a panel, and neither said so. There is an "on" class now, and only the code that owns a panel's visibility sets it. That is the whole point of where it lives: the click is not the only way either panel closes, since both have their own close button and clearing the plan closes them too. Hung off the click, a panel dismissed any other way would leave its button lit over nothing. The mark is the accent on the label rather than on the button's ground. A wash and a lit border are what focus and hover already look like, so on their own "latched" and "the pointer is here" came down to a weight change that did not carry at normal size. Colouring the text settles it, and the icon follows because AppIcons lets it inherit. Nothing paints a solid accent background, so the thing phase 2 removed stays removed. U9. The editor's font-size picker and the plan canvas's zoom readout both rendered a bare percentage at the same size, a row apart, and neither said what it scaled -- so the fix for two controls that look identical is to stop them looking identical. The picker takes an "Aa" prefix, which names it as text size. The readout, which sits between its own zoom buttons, says what it is on hover; it also gets a background so the whole rect answers the pointer rather than just the glyphs, which is the hit-testing trap this repo has been bitten by before. Verified live, both states and both controls, including closing the minimap by its own button and watching the toggle go out. 711 tests; the minimap case fails if the class moves to the click handler. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DEJ8FiSKJg44Ew2GG2nQ7
…te-and-zoom-labels Say which panel is open, and which zoom is which
Four files carry a version and all four have to move together. Directory.Build.props is the only one anything checks: the release workflow reads it for the tag, and check-version-bump.yml compares it against main. The other three are on the honour system. CITATION.cff was two releases stale at 1.23.0 -- missed in both 1.24.0 and 1.25.0, and stale on main too. Nothing catches it: ci.yml and the review workflow both list it under paths-ignore, and release.yml never reads it. Caught this time only by auditing every version-carrying file rather than grepping for the current one, which by definition cannot find a file that stopped tracking it. The SSMS pair was in sync at 1.25.0 and stays in sync here. That project is non-SDK, so it does not inherit the props file, and release.yml only emits a warning on mismatch rather than failing -- which is how it drifted to 1.13.0 during the 1.14.x releases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bump to 1.26.0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
132 commits, 14 PRs (#518–#531). Test suite 574 → 711, zero failures, zero build warnings.
This is the UI/UX modernization campaign plus a pile of pre-existing bug fixes the campaign's reviews turned up.
Bug fixes — user-visible, nearly all pre-existing
Settings, four separate bugs. Opening Settings and clicking Cancel with zero interaction always prompted "Discard changes?" (#518). Saving without ever opening Query History silently wiped both its values — a configured max-plans of 50 reset to 10, default metric blanked (#529). Editing a section, navigating away and back lost the edit while the dialog still claimed to be dirty (#530). Invalid colours were a silent refusal on a control you couldn't see, and the stored palette could be trimmed, losing a colour you'd picked (#530).
Advice and repro. Copy Repro and Run Repro stayed dead after every execute. Advice could act on a plan you weren't looking at. Right-clicking a plan tab did nothing when the plan came from executing a query — the commonest way to open one (all #522).
Numbers that lied. A 4.7s → 2ms win printed as "100.0% faster" (#523). The duration ladder rounded 0.4ms to "0ms", reporting a query that ran as one that didn't (#525). Costs rendered as
3526.210000; a tiny-but-real cost now reads<0.0001instead of0(#518).A real Query Store correctness bug: Avg mode was summing per-execution averages for the "Others" aggregate — on a many-database instance that dominates every card's scale with a number that isn't a rate (#521).
Wait colours you couldn't see: 3 of 23 categories failed the 3:1 contrast floor;
Buffer IOat 2.00 was effectively a hole in the chart (#526).Plus: Compare Plans silently no-oping with one plan open; exception text parked in the status strip forever; grouped rows showing
0for Query ID / Plan ID; a connect-dialog race that could save an unvalidated server; a restored slicer range collapsing to zero width; an unguardedasync voidon the shared Overview path.Behaviour changes worth stating plainly
Users may file these as bugs:
compare_planstext changed in the 99.95–100% band only (Stop a 99.9% win printing as 100% #523). Everything else there, and the Robot Advice JSON, is byte-identical and golden-file pinned.UI/UX
Design tokens so every colour resolves to a documented name; 18 Fluent icons replacing the emoji/glyph mix; a single scrolling tab row with fixed-slot toolbars and an overflow chevron instead of chrome wrapping to three rows; the properties panel overhauled (per-thread stats collapsed, filter box, real copy menu); Advice for Humans as severity cards with byte-identical copy output; Plan Comparison as a real metric diff; QS Overview as small multiples; session IA splitting views from documents; the wait palette laid out by hue-family and lightness ladder; the minimap as a 220×220 corner overlay; MCP and proxy moved from the About box into Settings > Integrations.
Known issues shipping with this
Release checks
CITATION.cff, which had been stale at 1.23.0 since before 1.24.0.claude-*.ymlcherry-pick tomainis needed.main— no server deploy rides on this.Merging this publishes the release.
🤖 Generated with Claude Code