Release v1.25.0 - #517
Merged
Merged
Conversation
Optional GCF output for the MCP server (PLANVIEWER_OUTPUT_FORMAT=gcf)
…hort SQL Server caps StatementText at 4,000 characters when it writes showplan XML. Everything that read it inherited the cap: Copy Query Text, Open in Query Editor, the Ctrl+C copy guard, and the advice built from the parsed plan. The reporter's symptom was the second-order one -- a statement cut mid-token is not valid T-SQL, so re-running or formatting it failed with a syntax error that pointed nowhere near the truncation. Two halves. Say so. Rule 39 raises an Info warning when a statement hits the cap, so the plan reports it instead of leaving the user to work out why their query stops early. Because it is a plan warning it reaches the Statements grid count, Human Advice, and the Robot Advice JSON -- the advice still analyzes the short text, but it is no longer silent about it. The 3990 literal that Rule 3 already used for the same test moved onto PlanStatement as one definition. Hand back the real query. PlanViewerControl already holds the text a plan was captured from; GetQueryTextFromPlan preferred it, the statement menu did not. It now does, for a truncated single-statement plan. Single-statement only, deliberately: the captured text is the whole batch and showplan records no statement offsets, so there is no way to tell which slice belongs to the selected row, and a confidently wrong statement is worse than a short one. Rule 39 still fires either way. RunnableStatementText carried a comment saying the plan is the only source because the editor buffer is wrong for plans opened from a file or Query Store. That reasoning is sound but it is about the live buffer; this is the text captured at LoadPlan, which is null for a file-opened plan and the stored query for Query Store. The comment now draws that distinction rather than being quietly contradicted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nD83xZyWPzKWhs7Gg9Vyq
The advice pane added one control per line. Avalonia gives every SelectableTextBlock its own selection -- the whole API is SelectionStart/SelectionEnd/SelectedText/Copy on the individual control -- and nothing coordinates a drag that starts in one and ends in another. So a selection could never be longer than a line. The reporter could grab a one-line query and nothing else: Server Context, Parameters and Missing indexes are several lines each, so dragging over them produced nothing usable. Copying everything into Notepad was the only way to get a piece of it. BodyTextAccumulator collects consecutive body lines into one SelectableTextBlock built from Inlines, so a drag across those lines is an ordinary selection. Every text path in Build routes through it -- statement SQL, key/value pairs, bullets, code blocks, the -> explanations, the SNIFFING marker, sub-section labels, plain text -- and flushes before anything structural. Blank lines no longer break the block. They used to emit an 8px spacer Border, which would have split every section at its first paragraph break; they are now a LineBreak inside the same block. Indentation moved from control margins to leading spaces. That only works because the pane is monospace throughout. The limit is a section, not the window. Warning blocks, operator groups, wait-stat bars and the triage card are Bordered composites, not text runs, so they cannot join a text block and still interrupt a drag. Going further needs a cross-control selection manager. A test pins that ceiling so it does not get quietly assumed away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nD83xZyWPzKWhs7Gg9Vyq
Review catch on #505: the statement-SQL path passed 0 where BuildSqlHighlightedLine had been carrying Margin(8, 1, 0, 1), so merging the runs into a shared block silently dropped the SQL indent. Every other converted call site passed the right number; this one did not. Making the caller restate a number the helper already knows is the bug. The SelectableTextBlock overload now reads Margin.Left off the block it was handed, so a helper's indent cannot be lost by a caller that forgets it, and a test pins the indent rather than trusting it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nD83xZyWPzKWhs7Gg9Vyq
… copy alone Two review catches on #504. IsSingleStatementPlan summed _currentPlan.Batches, which stops at the outer batch, while the grid lists statements via EnumerateAllWithContainer and descends into stored procedure and UDF bodies. A plan captured around EXEC dbo.SomeProc therefore has one batch statement and several rows: the guard called it single-statement and handed the outer EXEC back for a truncated body statement. That is the confidently-wrong-statement case the guard exists to prevent, so the bug defeated the design exactly where it mattered most. Now _allStatements?.Count == 1, which is what the grid shows. CopyParameterizedStatementText_Click was also swapping in the captured text. That entry exists to hand over the plan's placeholder form, and the captured text carries literals where the plan carries parameters, so substituting there silently collapsed the distinction #467 added it for. Reverted, along with the menu-label decision that goes with it. Rule 39 still reports the truncation. The new test uses the exec_stored_procedure_plan fixture and asserts Batches.Sum(...) == 1 before the behavior, so it is pinned to exercise the bug rather than pass for an unrelated reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nD83xZyWPzKWhs7Gg9Vyq
…text Fix #502: hand back the full query when the plan's copy is truncated
Two more review catches on #505. The [SNIFFING] branch still built a SelectableTextBlock that nothing used once the runs moved into the accumulator. Removed. The Node-link hit test is the real one, and it is a regression this PR caused. WireNodeClickHandler maps a hit-test character index back to a Run by walking inlines and summing Run.Text.Length, advancing only past Runs. That was correct while every block held one line and contained no LineBreaks. Merging interleaves them, and a LineBreak takes up a position in the laid-out text while exposing no Text, so every offset past the first line slid backwards: a "Node N" on a later line resolved to the wrong run, or to none. The hover handler carried an identical copy of the same loop, so both are now one RunAtCharIndex helper rather than the same fix applied twice. How much a non-Run inline contributes is derived from InlineCollection.Text against the summed Run lengths instead of hardcoded, so the walk cannot drift if the framework changes what it writes for one. The test checks that arithmetic directly; driving a real click would place a pointer at a laid-out coordinate and test font metrics rather than the part that was wrong. Worth noting: the selection tests added earlier would never have caught this. They assert on selection, and this broke clicking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nD83xZyWPzKWhs7Gg9Vyq
Review note on #505, taken further than asked. The offset walk inferred how many positions a non-Run inline occupies by dividing the leftover characters evenly across them. That is exact today -- a LineBreak emits one newline and the accumulator inserts nothing else -- but it holds only while every non-Run inline is the same kind, and the day one is not it goes quietly wrong rather than loudly. The review suggested a comment saying so; a comment does not stop the breakage. Each Run is now found in the text the collection actually laid out, searching forward from the end of the previous one. That is exact for any interleaving, so there is no assumption left to document. Searching forward rather than from zero matters on its own: two runs can carry the same text, such as the same node referenced on two lines, and a from-zero search resolves the second to the first. Covered by a test, because the arithmetic being replaced got that case right and a naive rewrite would not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nD83xZyWPzKWhs7Gg9Vyq
Review housekeeping note on #505, and the cause is mine rather than an editor: the scripted edits read with utf-8-sig and wrote back with utf-8-sig, which strips a BOM on the way in and adds one on the way out. Every file touched that way gained a BOM that its siblings in the same partial class do not have. Harmless to the compiler, but it is noise on the first line of every future diff of these files. The same thing reached dev through #504 on three more files; that is a separate follow-up rather than something to smuggle into this PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nD83xZyWPzKWhs7Gg9Vyq
Fix #503: let a selection cover a section of Human Advice, not just one line
Same cause as the strip in #505, on the files that reached dev before it was noticed: the scripted edits read with utf-8-sig and wrote back with utf-8-sig, which strips a BOM going in and adds one coming out. All three files had none before, and their neighbours still have none. No behavior change -- the compiler does not care. It is the first line of every future diff of these files that does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nD83xZyWPzKWhs7Gg9Vyq
Drop the BOMs #504 added to three files
Part of #507. The custom range popup had two silent failures, and both read as the picker ignoring what you typed, which is the conclusion the reporter came to. An end at or before the start was rewritten to an hour after the start (endUtc = startUtc.AddHours(1)) and applied. A missing date just closed the popup. Neither said anything. Now both refuse, name the problem, and leave the popup open on the entered values so they can be seen and corrected. Equal bounds get their own message. "The end is before the start" is untrue there, and equal bounds are exactly the case the old code turned into a silent one-hour range. The rule is a static DescribeRangeProblem rather than more branches inside the click handler, so it can be tested without standing up a popup and driving two CalendarDatePickers, and so the next rule is one method to change. DarkTheme had no error colour, so SlicerValidationBrush is added next to the other slicer brushes, reusing the red the advice pane already uses for Critical. Deliberately unchanged: a sub-hour range still widens to one hourly bucket when applied. That is Query Store's aggregation interval, not bad input, and the range label already shows what was applied. A test pins a one-minute range as valid input so that does not later get "fixed" into an error. The other half of #507, the pickers supposedly being limited to the loaded window, turned out not to exist -- quick filters only move the selection inside data that is already loaded, so the calendar always spans the full slicer window. Corrected on the issue rather than fixed here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nD83xZyWPzKWhs7Gg9Vyq
…idation Say why a custom time range was rejected instead of quietly changing it
Bumps Meziantou.Framework.Win32.CredentialManager from 3.0.1 to 3.0.3 Bumps Microsoft.Testing.Extensions.HangDump from 2.3.3 to 2.4.0 --- updated-dependencies: - dependency-name: Meziantou.Framework.Win32.CredentialManager dependency-version: 3.0.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-and-minor - dependency-name: Meziantou.Framework.Win32.CredentialManager dependency-version: 3.0.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-and-minor - dependency-name: Meziantou.Framework.Win32.CredentialManager dependency-version: 3.0.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-and-minor - dependency-name: Meziantou.Framework.Win32.CredentialManager dependency-version: 3.0.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-and-minor - dependency-name: Microsoft.Testing.Extensions.HangDump dependency-version: 2.4.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: patch-and-minor ... Signed-off-by: dependabot[bot] <support@github.com>
--- updated-dependencies: - dependency-name: BlackwellSystems.Gcf dependency-version: 1.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
…nViewer.App/dev/patch-and-minor-4aeccb4785 deps: Bump the patch-and-minor group with 2 updates
…nViewer.App/dev/BlackwellSystems.Gcf-1.0.0 deps: Bump BlackwellSystems.Gcf from 0.2.1 to 1.0.0
#504 recovered the captured query for Copy Query Text and Open in Query Editor but left Human Advice, Robot Advice, and the MCP tools analyzing the plan's 4,000-character stub - the surface the reporter actually screenshotted, so the nightly read as no fix at all. ResultMapper.Map now takes the captured query text and substitutes it for a single-statement plan whose text hit the showplan cap, under the same guard as the viewer's PlanOrCapturedStatementText. The model is untouched: Rule 39 and the properties panel keep reporting what the plan records; only the mapped output carries the recovery, and its truncation warning then says the text was recovered instead of contradicting the complete query beside it. Every caller that holds the text passes it: query-editor advice (selected tab and fallback, which also regains the server metadata it was silently dropping), file-mode advice, both compare dialogs, the MCP plan and Query Store tools, PlanOperations session analysis, and the CLI analyze (SQL input) and query-store commands. File/stream paths pass nothing - a .sqlplan alone has nothing to recover from. Verified end to end against SQL Server 2022: a 10,692-character single-statement query whose plan records 3,999 characters now shows the full query in the advice output. Six new tests; full suite 531 passed, 1 pre-existing skip, 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013tW5wVxgbtcCegPuC6Zbqe
#505 merged a section's lines into one SelectableTextBlock, but a text block with no background is only hit-testable where its glyphs rendered. A press in the leading indent, past the end of a short line, or in any empty run - the natural way to start a section drag - fell through to the panel, and no selection ever started. The statement's wall-to-wall wrapped SQL catches any press, which is exactly the reporter's "I can select a query" but nothing else, so the merged blocks read as no fix at all. Build() now ends with a recursive pass giving every text block in the pane, composites included, a transparent background: the whole rectangle accepts the press, nothing changes visually, and a block added next month is covered without anyone remembering why. Verified with real input against a Skia-rendering headless session (the suite's session cannot hit-test glyphs at all, which is how the SelectAll-based tests passed while the drag was broken): a margin press that selected nothing now selects the whole section. Selection still cannot cross a Border-wrapped composite - #505's documented ceiling stands. New test walks the logical tree rather than mirroring the traversal, so a container shape the fix misses fails the test instead of hiding from it. Full suite 531 passed, 1 pre-existing skip, 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013tW5wVxgbtcCegPuC6Zbqe
Fix #502 follow-up: the advice reads the captured query too
Fix #503 follow-up: accept a press anywhere in an advice text block
- Bump patch/minor packages: Microsoft.Data.SqlClient 7.0.3, SqlClient.Extensions.Azure 7.0.3, ScriptDom 180.107.0, CredentialManager 3.0.4, System.CommandLine 2.0.12, AspNetCore WebAssembly + DevServer 10.0.12, Test.Sdk 18.10.0, xunit.v3 4.0.1, Microsoft.Data.Sqlite 10.0.12 (PlanShare) - Fix the 9 MCP9005 warnings: the MCP spec deprecated Roots (SEP-2577) but Roots is still the contract the plan-path sandbox enforces and McpSmokeTests must keep exercising it; suppress with rationale, migration tracked in #514 - Drop an unused using in McpSmokeTests - Refresh the SQLitePCLRaw pin comment in PlanShare: the Microsoft.Data.Sqlite 10.0.12 floor moved to the CVE-fixed 2.1.12, so the pin now exists for the newer 3.x-bundled SQLite, not the CVE - Not bumped (parked): Avalonia 12 family + ScottPlot 5.1.59 + SkiaSharp 4.x (branch upgrade/avalonia-12); VS SDK held on 17.x Validated: solution + PlanShare + SSMS installer build with zero warnings; 531 tests pass; linux-x64 and osx-arm64 publish smoke OK with libSkiaSharp.so hash-matching the 3.119.4 pin (issue #139 guard). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbaAfAZPVqXbZVPy79AL81
- PlanShare /api/stats: accept the token via the X-Stats-Token header only. The ?token= fallback wrote the secret into nginx access logs - the exact leak the dashboard's fragment-based bootstrap avoids, and nothing used it (the dashboard sends the header). - deploy-planshare.yml: count 401 from /api/stats as a healthy deploy. Requiring 200 meant that setting STATS_TOKEN would fail verification and auto-roll-back every subsequent good deploy. /health stays out of reach because nginx does not route it. - dependabot.yml: stop auto-bumping BlackwellSystems.Gcf. It runs against every MCP tool result when enabled, it is maintained by the contributor who introduced it (#501), and dependabot PRs skip the AI review gate - so bumps now arrive only via hand-reviewed PRs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbaAfAZPVqXbZVPy79AL81
Quarterly maintenance 2026-09: dependency bumps, zero-warning build
All four spots: Directory.Build.props plus the SSMS extension pair (vsixmanifest + AssemblyInfo), which do not inherit the props version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbaAfAZPVqXbZVPy79AL81
Bump version to 1.25.0 for release
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
2 participants
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.
Promotes dev to main and cuts the v1.25.0 release on merge (release.yml builds all platforms, signs via SignPath, packages with Velopack, and publishes).
Shipping since v1.24.0 (27 commits):
Pre-release audit: READY - version bumps consistent everywhere, vpk pin matches Velopack 1.2.0 (unchanged since v1.24.0), sign-before-publish flow intact, no schema or update-path changes. The main push after this merge also auto-deploys PlanShare (first live run of the 401-accepting verify step, rollback in place) and redeploys the web viewer.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UbaAfAZPVqXbZVPy79AL81