diff --git a/CHANGELOG.md b/CHANGELOG.md index e50c7e0fd2..dfbbdd0062 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- **The web dashboard's server page gets the viewer's tabs: twelve sections, 61 of the 82 served reads, and a time range** ([#2475]) - the service dispatched **82** reads at `GET /api/read/{name}` and the built-in pages reached **23** of them; `#/server/{name}` was one scroll of seven panels against the desktop viewer's **65** `TabItem`s. The gap was never backend work - `panels.js` has been a generic renderer over those reads since #1562 - so this is descriptors over the unchanged `renderPanel` seam: twelve sub-tabs (Overview, Wait Stats, CPU, Memory, Blocking, File I/O, Queries, Configuration, Config Changes, Activity, System Events, Collection Health) carrying ~70 panels, reaching **61** reads. The header carries the WHY beneath the band badge - `Warning` has three unrelated causes (a real metric breach, a server awaiting its first collection, a collector error), so a badge reading "Warning" with no way to ask why is [#2422] rebuilt on a new surface; the fleet's own reason string and the fleet card's severity chips are rendered there, the chips through `fleet.js`'s own `metricBands` so there is one implementation rather than two, and neither is re-derived in the browser (R1). Every web grid that renders query text now puts it immediately right of its time/identity anchor ([#1949]) and the pin that enforced that on one array enforces it on all seven. The tab id rides in the hash (`#/server/{name}/{tab}`) so a section is deep-linkable and survives the 60s refresh, and an unknown or absent id resolves to Overview, which is what keeps every existing `#/server/{name}` link working. A page-level range picker (1h/4h/12h/24h/7d/30d) is the twin of the viewer's toolbar presets - **not** persisted, because a page that reopens on a 30-day window is slow for a reason the reader cannot see; panels whose read takes no window at all say "latest snapshot" rather than inheriting a label that would misdescribe them. Two reads were previously **unreachable from a browser** because they require a parameter no UI collected - `get_wait_trend` needs a `wait_type` and `get_perfmon_trend` a `counter_name` - and both now have a picker, the wait one seeded from the rows of the table directly above it so the picker and the table cannot disagree. **No fifth viz kind was added**: the property-grid shape that tempted one is served by `stat` (the reads returning a flat object) and `table` (the ones already returning rows), and a fifth kind that lived only in `panels.js` would be a page-only special case, while doing it properly means `KnownVizList`, `derive.js` and an editor config arm - composer surface this change does not need. What the browser genuinely cannot do is **stated in the tab where a reader goes looking for it** rather than left as a page that quietly lacks a feature: plan analysis, the query heatmap, cached-plan retrieval and actual-plan re-execution need a plan renderer and a command back to the monitored server, and the block-chain view and interactive deadlock graph need a graph viewer - the Blocking tab hands over the captured blocked-process-report and deadlock-graph XML verbatim instead of pretending. Every data panel supplies its own empty-state sentence and both helpers THROW without one. vizTable's generic "No rows in this window" reads as a fault on a collector that is off, opt-in, or daily; and the chart case was worse - `get_blocking_trend` and `get_deadlock_trend` answer an IDLE server with `trend: []` and no `{status,message}` envelope at all, so a perfectly healthy server was told its blocking chart did not have "enough data points to chart yet". `vizLine` now renders a descriptor's `emptyText` at exactly ZERO rows and still falls through at one (where the chart's own sentence is the true one) and when no `emptyText` was authored, so every stored view predating this is unchanged. A read feeding several panels on one tab is fetched ONCE (`fanout`) rather than per descriptor - `readTool` has no cache, so `get_collection_health`, which rolls up seven days of collector logs and computes sweep pressure, was running three times to open its own tab; six such duplicates existed across five tabs and a pin now refuses a seventh. Guarded by an invariant rather than by spot-checks: every read name the module mentions must exist in the shipped dispatch, every parameter key must be one its read actually binds (an unknown query key is silently ignored, so `limit` sent to a read binding `top` quietly returns the default), every viz must be in the shipped vocabulary, and no `get_pg_*` read may appear until the fleet payload can tell a PostgreSQL target from a SQL Server one - it carries `engine_edition`, not a `CollectorTargetEngine`, so a PostgreSQL panel today would render on all 42 SQL Servers, permanently empty. + ### Changed - **`get_collection_health` serves what a HEAVY run costs, not only what runs cost on average** ([#2460]) - `query_store` on one dogfood server reported `avg_duration_ms: 13,834` over 1,155 runs. 958 of those runs carried the `enumeration yielded 0 items` note, and an empty enumeration costs **36 ms** - measured on a control server that yields nothing on all 1,551 of its runs and pays 36 ms for every one. Back that out and the remaining 197 PRODUCTIVE runs cost **~80,900 ms EACH**: more than the entire 60,000 ms sweep budget, on their own, once every few cycles. 13,834 ms describes neither population - it is an 83/17 blend that happens to land in a range reading like a plausible single number, and everything downstream inherited it, [#2459]'s brand-new `peak_cycle_ms` included, which understated that server's worst body by ~67,000 ms. Every collector row now carries `p95_duration_ms` and `max_duration_ms` beside `avg_duration_ms`, and `peak_cycle_ms` is built from the p95 rather than the mean. **The store already held this** - `duration_ms` has been written per run since the schema's second rung, and nothing had ever read it as anything but a mean - so this is two aggregates over a table already being grouped, no new collection and no new column. p95 rather than max for the number a decision is made from, because a max is one run and a single pathological cycle would make a collector read as permanently terrible for a week; the max is served BESIDE it as a fact, and comparing the two is what tells a routine tail from a one-off (avg ~ p95 ~ max is one population, avg << p95 is two, p95 << max is one bad run). p95 also scales itself to the sample: over 3,500 runs it discards the outlier, and over the six runs a daily collector gets in a week it lands on the max, which is right, because with six samples there is no outlier anyone can afford to throw away. Each collector is charged the p95 **floored at its mean**, so the aligned cycle can only ever rise - a p95 CAN sit below a mean (99 runs at 10 ms and one at 1,000,000 ms), and taking it unconditionally could have retracted a `BODY_OVERRUN` [#2446] correctly caught. **The verdict is untouched and still amortizes the mean**: sustained demand over a window IS the mean, and a rate built from a tail claims work the server never sustains. On the measured server the aligned body goes from 73,408 ms to 140,507 ms and `peak_collector` changes from `index_object_stats` (the larger MEAN, once a day) to `query_store` (the larger heavy RUN, every five minutes) - which is the collector that was actually overrunning those bodies. `peak_cycle_note` now states the mean beside the heavy run and the gap between them; `heaviest_collectors` rows carry both new statistics and take `pct_of_sweep_budget_per_run` from the heavy run, since a "per run" percentage computed from a mean that describes no run is the defect itself. Both SKUs, one query shape, and the same fixture pinned against live DuckDB and live Postgres so the two engines have to agree - including that both ignore a NULL `duration_ms` in an ordered-set aggregate, which neither query states. - **`sweep_pressure` answers the single-sweep question as well as the sustained one** ([#2446]) - a server logging six "collection body has not completed after 60-69s of execution - skipping relaunch" warnings in three hours reported `busy_percent: 20.4`, `verdict: OK`, every collector HEALTHY - and both numbers were right about what they measured. [#2296]'s amortized model asks "does this server's total demand fit its cadence on average"; an operator reading a skipped relaunch is asking "did THIS sweep overrun". Those diverge exactly when one collector's single run approaches the budget while its amortized cost is negligible: `index_object_stats` took 37,207 ms of a 60,000 ms body and, at a 1440-minute cadence, contributed 26 ms/min to the verdict. The block now also carries `peak_cycle_ms` / `peak_cycle_percent` - what the body costs on the cycle where every scheduled cadence comes due together, the collectors' averages added WITHOUT being amortized - and `peak_cycle_risk` (FITS / BODY_OVERRUN). That cycle is not a hypothetical worst case: the shipped cadences are strictly nested (1 | 5 | 60 | 1440), so alignment is a guaranteed periodic event, and on that server it costs 73,408 ms against the 60,000 ms budget, which is what the pinned fixture reproduces. `peak_collector` names the collector that owns the most of one sweep and `peak_cycle_note` explains it, because `heaviest_collectors` ranks by amortized contribution and therefore ranks the offending collector out of sight by construction - that list now also carries `amortized_ms_per_minute` and `pct_of_sweep_budget_per_run` per row, so the two costs sit side by side. **The verdict is deliberately unchanged.** A once-daily 37-second collector is not saturation; calling it SATURATED would spend the word on a case whose lever is the schedule's shape rather than the capacity that verdict recommends, and an operator who learns to discount SATURATED loses the signal [#2296] built. Separate field and separate vocabulary, so neither can be read as the other and a fleet scan can filter on either. Measured across the dogfood fleet: the two servers that logged skipped relaunches read OK/BODY_OVERRUN (122% and 109% of budget) while a quiet one read OK/FITS at 19%. The decision stays in the shared SweepPressureClassifier (PerformanceMonitor.Common) with the same table pinned in both suites, and both SKUs' tools serve the identical shape. @@ -2845,6 +2848,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#2312]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2312 [#2306]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2306 [#2302]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2302 +[#2475]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2475 +[#2422]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2422 [#2460]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2460 [#2459]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/2459 [#2446]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2446 diff --git a/Darling/Darling.Tests/DarlingEmptyEnumerationNoteTests.cs b/Darling/Darling.Tests/DarlingEmptyEnumerationNoteTests.cs index aee2b7fb35..5dd6b91ff5 100644 --- a/Darling/Darling.Tests/DarlingEmptyEnumerationNoteTests.cs +++ b/Darling/Darling.Tests/DarlingEmptyEnumerationNoteTests.cs @@ -195,13 +195,16 @@ public void The_Web_Dashboards_Collection_Health_Table_Shows_The_Note_Too() /* Darling has THREE Collection Health surfaces, not two: the WPF Viewer grid, the MCP tool, and the web dashboard's table, which renders whatever COLLECTOR_COLUMNS lists from that same tool's payload. A field added to the tool but not to that list is silently dropped, leaving the - browser as the one surface still hiding what #1837 exists to show. */ + browser as the one surface still hiding what #1837 exists to show. + + The array moved from pages/server.js to pages/server-tabs.js when the web server page grew + sub-tabs (#2475) - server.js is the shell now and the tab registry owns every column array. */ var source = ReadRepoFile(Path.Combine( - "Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "pages", "server.js")); + "Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "pages", "server-tabs.js")); /* The DEFINITION, not the earlier `columns: COLLECTOR_COLUMNS` use site. */ var start = source.IndexOf("const COLLECTOR_COLUMNS", System.StringComparison.Ordinal); - Assert.True(start >= 0, "server.js must still define COLLECTOR_COLUMNS"); + Assert.True(start >= 0, "server-tabs.js must still define COLLECTOR_COLUMNS"); var columns = source[start..]; columns = columns[..columns.IndexOf("];", System.StringComparison.Ordinal)]; diff --git a/Darling/Darling.Tests/ServerPageTabsTests.cs b/Darling/Darling.Tests/ServerPageTabsTests.cs new file mode 100644 index 0000000000..ef67e7d185 --- /dev/null +++ b/Darling/Darling.Tests/ServerPageTabsTests.cs @@ -0,0 +1,386 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; +using PerformanceMonitor.Darling.Service; +using Xunit; + +namespace Darling.Tests; + +/// +/// The web server page's sub-tabs — the port of the desktop viewer's per-server TabControl. +/// +/// The invariant these pins exist for. A panel descriptor names its data source as a STRING +/// (read: "get_wait_stats"). A typo, or a read renamed on the C# side, produces a 400 inside one panel at +/// runtime and is completely invisible to inspection — the JS still parses, the page still renders, one panel +/// just says "unknown". With ~60 reads named across twelve tabs that is not a defect you find by reading. So +/// rather than pinning individual panels, this asserts the CATEGORY: every read name the shipped module mentions +/// exists in the shipped dispatch table, and every viz it names exists in the shipped viz vocabulary. Both sides +/// come from the artifacts themselves, never a transcribed copy, so the check cannot drift into agreeing with a +/// stale list of its own. +/// +/// This repository carries no JavaScript test runner, so the scan is a text scan over the shipped module +/// (the / +/// pattern). Behaviour was verified separately by running the shipped modules under a minimal DOM shim with a +/// stubbed fetch across four response shapes (empty envelope, error, data-with-no-rows, data-with-rows): every +/// tab built without throwing at three time ranges, and every request the page actually issued was checked +/// against CatalogDescriptors for parameter-key validity. See the PR. +/// +public sealed class ServerPageTabsTests +{ + private static string ServerTabsJs => ReadRepoFile(Path.Combine( + "Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "pages", "server-tabs.js")); + + private static string ServerJs => ReadRepoFile(Path.Combine( + "Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "pages", "server.js")); + + private static string AppJs => ReadRepoFile(Path.Combine( + "Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "app.js")); + + /// + /// Every read name the tab module mentions is a read the service actually serves. + /// + /// The scan is deliberately over the whole file rather than over parsed descriptors: read names live in + /// the get_* namespace and nothing else in this module does, so a literal in a comment is caught too — + /// which is the point. A comment that names a read the dispatch no longer has is stale documentation about + /// the one thing here that is impossible to verify by eye. + /// + [Fact] + public void EveryReadTheServerPageNames_ExistsInTheDispatch() + { + var dispatch = DarlingWebEndpoints.BuildReadDispatch().Keys.ToHashSet(StringComparer.Ordinal); + var named = ReadNamesIn(ServerTabsJs); + + Assert.NotEmpty(named); + + var unknown = named.Where(n => !dispatch.Contains(n)).OrderBy(n => n, StringComparer.Ordinal).ToArray(); + Assert.True( + unknown.Length == 0, + "server-tabs.js names reads that GET /api/read/{name} does not serve — each renders as a broken " + + "panel at runtime and looks fine on inspection: " + string.Join(", ", unknown)); + } + + /// + /// Every parameter key the page sends is one its read actually binds. + /// + /// This is the half that fails SILENTLY rather than loudly: an unknown query key is ignored, so a panel + /// asking limit=10 of a read that binds top quietly returns the read's default 20 rows and + /// nothing anywhere says so. CatalogDescriptors is the authority for a read's real wire keys (the + /// dispatch lambdas bind string literals imperatively, so the C# parameter names are not those keys), and the + /// two documented aliases the dispatch also accepts are allowed explicitly rather than by accident. + /// + [Fact] + public void EveryParameterKeyTheServerPageSends_IsOneItsReadBinds() + { + var js = ServerTabsJs; + var problems = new List(); + + foreach (var (read, keys) in ParamsSentIn(js)) + { + if (!DarlingWebEndpoints.CatalogDescriptors.TryGetValue(read, out var descriptor)) + { + continue; // the read-existence pin above owns this failure and names it better. + } + + var allowed = descriptor.Params.Select(p => p.Name).ToHashSet(StringComparer.Ordinal); + + /* The dispatch's two documented aliases: Hours() reads ?hours= then ?hours_back=, and Server() reads + ?server= then ?server_name=. Named here rather than assumed, so removing one breaks this test. */ + if (allowed.Contains("hours")) allowed.Add("hours_back"); + if (allowed.Contains("server")) allowed.Add("server_name"); + + foreach (var key in keys.Where(k => !allowed.Contains(k))) + { + problems.Add($"{read} is sent '{key}' but binds only [{string.Join(", ", allowed.OrderBy(a => a, StringComparer.Ordinal))}]"); + } + } + + Assert.True(problems.Count == 0, string.Join("; ", problems)); + } + + /// + /// The page speaks the catalog's canonical time key. Both hours and hours_back reach the same + /// binding, so this is a consistency rule rather than a correctness one — but it is the rule that keeps the + /// pin above meaningful: with two spellings in play, a reviewer cannot tell a deliberate alias from a typo + /// that happened to land on the alias. + /// + [Fact] + public void TheServerPage_UsesTheCatalogsCanonicalTimeKey() + { + Assert.DoesNotContain("hours_back:", ServerTabsJs, StringComparison.Ordinal); + Assert.Contains("hours: ctx.hours", ServerTabsJs, StringComparison.Ordinal); + } + + /// + /// Every viz a descriptor names is in the shipped vocabulary. The four kinds are the whole registry; a fifth + /// would have to be added to panels.js's VIZ, to KnownVizList (or the composer could not offer it), to + /// derive.js's deriveVizConfig and to the editor's config arms — so a page quietly introducing one + /// would be a page-only special case, which is exactly what the seam exists to prevent. + /// + [Fact] + public void EveryVizTheServerPageNames_IsInTheShippedVocabulary() + { + var vocabulary = DarlingWebEndpoints.KnownVizList.ToHashSet(StringComparer.Ordinal); + var named = Regex.Matches(ServerTabsJs, "viz:\\s*\"([a-z]+)\"") + .Select(m => m.Groups[1].Value) + .ToHashSet(StringComparer.Ordinal); + + Assert.NotEmpty(named); + Assert.All(named, v => Assert.Contains(v, vocabulary)); + + /* And the registry in panels.js is that same vocabulary — the C# validator and the browser renderer + agreeing is what lets a stored view and a built-in page share one seam. */ + var panels = ReadRepoFile(Path.Combine( + "Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "panels.js")); + foreach (var v in vocabulary) + { + Assert.Contains(" " + v + ": viz", panels, StringComparison.Ordinal); + } + } + + /// + /// No PostgreSQL read appears on the server page, and this is a guard rather than an oversight. + /// + /// The web dashboard's per-server surface is fed by /api/fleet, whose card + /// (DarlingFleetReader.FleetServerCard) carries engine_edition — the SQL Server + /// SERVERPROPERTY value — and NO CollectorTargetEngine discriminator. So the browser cannot tell a + /// PostgreSQL target from a SQL Server one, and a get_pg_* panel added to these tabs would render for + /// every server in the fleet, permanently empty on the ~all of them that are SQL Server. Adding the + /// discriminator to the fleet payload is the prerequisite; this fails until then rather than letting the + /// panel ship first. + /// + [Fact] + public void NoPostgresRead_IsOnTheServerPage_UntilTheFleetPayloadCanTellTheEngines() + { + var pg = ReadNamesIn(ServerTabsJs).Where(n => n.StartsWith("get_pg_", StringComparison.Ordinal)).ToArray(); + Assert.True( + pg.Length == 0, + "the fleet payload carries no target-engine discriminator, so these would render on every SQL Server " + + "too: " + string.Join(", ", pg)); + } + + /// + /// The shell links only to tabs that exist, and every tab is reachable. A sub-tab link is a real href, so a + /// stale id is a page that renders Overview while claiming to be something else — the fallback that makes old + /// #/server/{name} links keep working is the same fallback that would hide this. + /// + [Fact] + public void TheSubTabBar_IsBuiltFromTheRegistry_AndTheRouterCarriesTheTab() + { + /* One source of truth for the bar: it maps the registry rather than listing ids. */ + Assert.Contains("SERVER_TABS.map((t) =>", ServerJs, StringComparison.Ordinal); + Assert.Contains("\"#/server/\" + encodeURIComponent(server) + \"/\" + t.id", ServerJs, StringComparison.Ordinal); + + /* And the router parses that second segment back out and hands it to the page. */ + Assert.Contains("function serverRoute(rest)", AppJs, StringComparison.Ordinal); + Assert.Contains("renderServer(main, r.param, r.tab)", AppJs, StringComparison.Ordinal); + + /* The name is decoded AFTER the split, so an encoded '/' inside a server name survives the tab segment + being introduced — the one way this change could have broken existing links. */ + Assert.Contains("decodeURIComponent(rest.slice(0, slash))", AppJs, StringComparison.Ordinal); + Assert.Contains("if (slash < 0) return { name: \"server\", param: decodeURIComponent(rest) };", AppJs, StringComparison.Ordinal); + + /* An unknown id resolves to a tab rather than throwing, which is what keeps a stale bookmark working. */ + Assert.Contains("SERVER_TABS.find((t) => t.id === id) || SERVER_TABS[0]", ServerTabsJs, StringComparison.Ordinal); + + /* Ids are unique — two tabs sharing one id makes the second unreachable and the bar's active state lie. */ + var ids = Regex.Matches(ServerTabsJs, "^\\s{4}id: \"([a-z-]+)\",$", RegexOptions.Multiline) + .Select(m => m.Groups[1].Value) + .ToArray(); + /* An exact count, not a floor. A floor would have let the prose in the CHANGELOG, the commit and this + file drift from the registry — which it did, at "eleven" against twelve, before this pin existed. */ + Assert.Equal(12, ids.Length); + Assert.Equal(ids.Length, ids.Distinct(StringComparer.Ordinal).Count()); + Assert.Equal("overview", ids[0]); // the fallback tab must be the first one + } + + /// + /// The tabs whose desktop twin does something the browser cannot say so, in the tab itself. + /// + /// Plan analysis, the query heatmap, cached-plan retrieval and actual-plan re-execution all need either + /// a plan renderer or a command back to the monitored server, and this read-only seat has neither. The same + /// goes for the block-chain view and the interactive deadlock graph. A reader told to open the desktop viewer + /// is better served than one given a web page that looks like plan analysis and is not — so the absence is + /// stated where they go looking for it, not left as a page that simply lacks the feature. + /// + [Fact] + public void TheTabsThatCannotDoWhatTheDesktopDoes_SaySo() + { + var js = ServerTabsJs; + + Assert.Contains("desktop-viewer features", js, StringComparison.Ordinal); + Assert.Contains("Execution-plan analysis, the query heatmap", js, StringComparison.Ordinal); + Assert.Contains("block-chain view", js, StringComparison.Ordinal); + + /* The note renders — a `note` field with no renderer is the same silence in a different place. */ + Assert.Contains("return tab.note ? noticeStrip(tab.note) : null;", js, StringComparison.Ordinal); + Assert.Contains("tabNote(tab)", ServerJs, StringComparison.Ordinal); + } + + /// + /// Every data panel says why it could be empty — charts as well as tables. + /// + /// renderPanel already shows a read's own {status,message} envelope when the read has nothing, + /// and that sentence is better than anything a descriptor could carry. What it does NOT cover is the read + /// returning DATA whose row array is empty, and both renderers had a wrong generic for that case. vizTable + /// falls back to "No rows in this window", which on a collector that is off, opt-in, or daily reads as a + /// fault. vizLine fell through to the chart's "Not enough data points to chart yet", which is right while + /// collection is warming up and wrong for the two reads whose empty array means the thing did not happen: + /// get_blocking_trend and get_deadlock_trend answer an idle server with trend: [] and no + /// envelope at all, so a healthy server was told its blocking chart was still warming up. + /// + /// Both helpers THROW without a sentence, and every tab is built during the DOM-shim run, so a panel + /// that forgot one cannot reach a browser. The zero-versus-one distinction was verified against the shipped + /// vizLine: zero points with an emptyText renders the descriptor's sentence, one point still renders the + /// chart's own (which is the true statement there), and zero points WITHOUT one still falls through — so a + /// stored view authored before this existed is unchanged. + /// + [Fact] + public void EveryDataPanel_ExplainsItsOwnEmptyState() + { + var js = ServerTabsJs; + + /* Not a count of sentences — a structural guard. Counting "No ..." literals would pass vacuously the + moment a comment happened to contain one, which is the shape of check that converts an open question + into false confidence. The helper THROWS without an emptyText, and every tab is built during the + DOM-shim run, so a table panel that forgot one cannot reach a browser. */ + Assert.Contains("function table(title, read, params, rowsKey, columns, subtitle, emptyText, span = 2)", js, StringComparison.Ordinal); + Assert.Contains( + "if (!emptyText) throw new Error(\"table(\" + title + \"): a table panel must explain its own empty state.\");", + js, + StringComparison.Ordinal); + + Assert.Contains("function line(title, read, params, rowsKey, xKey, series, opts = {})", js, StringComparison.Ordinal); + Assert.Contains( + "if (!opts.emptyText) throw new Error(\"line(\" + title + \"): a chart panel must explain its own empty state.\");", + js, + StringComparison.Ordinal); + + /* And renderPanel is what renders both, from the descriptor field the helpers set. The line guard fires + at EXACTLY zero rows: at one row the chart's own sentence is the true one, and a descriptor that never + had an emptyText (every stored view authored before this) still falls through unchanged. */ + var panels = ReadRepoFile(Path.Combine( + "Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "panels.js")); + Assert.Contains("desc.emptyText || \"No rows in this window.\"", panels, StringComparison.Ordinal); + Assert.Contains("if (!points.length && desc.emptyText) return emptyStrip(desc.emptyText);", panels, StringComparison.Ordinal); + } + + /// + /// No tab fetches the same read twice. + /// + /// A descriptor owning its own fetch is the right default and is what makes the seam composable — but + /// readTool/apiGet have no cache, so a read feeding two or three panels on ONE tab ran two or + /// three times. Review caught two; there were six, and the worst was get_collection_health, which + /// rolls up seven days of collector logs and computes sweep pressure, rendered as three slices of one + /// payload — so opening that tab ran the page's heaviest query three times. fanout() is the fix, and + /// this is the guard, because "fix the two review named" is how the other four ship. + /// + /// Composites name their reads inside their own function bodies rather than in a tab, so their reads + /// are mapped here explicitly — and the map is asserted against the functions, so it cannot quietly go + /// stale and start passing a tab it no longer describes. + /// + [Fact] + public void NoTab_FetchesTheSameReadTwice() + { + var js = ServerTabsJs; + + /* The composite -> reads map, verified against the composites themselves before it is trusted. */ + var composites = new Dictionary(StringComparer.Ordinal) + { + ["waitsPanel("] = new[] { "get_wait_stats", "get_wait_trend" }, + ["fileIoPanel("] = new[] { "get_file_io_trend" }, + ["perfmonPanel("] = new[] { "get_perfmon_stats", "get_perfmon_trend" }, + }; + foreach (var (call, reads) in composites) + { + var fn = "function " + call.TrimEnd('('); + var at = js.IndexOf(fn, StringComparison.Ordinal); + Assert.True(at > 0, "composite " + call + " is gone — remap it before editing this test"); + /* Its reads and its helpers' reads: take everything from the definition to the descriptor section. */ + var region = js[at..js.IndexOf("/* ─────────────────────────── descriptor helpers", StringComparison.Ordinal)]; + foreach (var read in reads) Assert.Contains("\"" + read + "\"", region, StringComparison.Ordinal); + } + + var problems = new List(); + var ids = Regex.Matches(js, "^ id: \"([a-z-]+)\",$", RegexOptions.Multiline).ToArray(); + + for (var i = 0; i < ids.Length; i++) + { + var start = ids[i].Index; + var end = i + 1 < ids.Length + ? ids[i + 1].Index + : js.IndexOf("/** The tab for an id", StringComparison.Ordinal); + var block = js[start..end]; + + var reads = Regex.Matches(block, "\"(get_[a-z0-9_]+|audit_config)\"").Select(m => m.Groups[1].Value).ToList(); + foreach (var (call, composed) in composites) + { + if (block.Contains(call, StringComparison.Ordinal)) reads.AddRange(composed); + } + + foreach (var dupe in reads.GroupBy(r => r, StringComparer.Ordinal).Where(g => g.Count() > 1)) + { + problems.Add($"tab '{ids[i].Groups[1].Value}' fetches {dupe.Key} {dupe.Count()} times"); + } + } + + Assert.True(problems.Count == 0, + string.Join("; ", problems) + " — several panels over one read is what fanout() is for."); + + /* And fanout carries the same empty-state rule the two descriptor helpers do, so routing a panel through + it is never the way to lose the sentence. */ + Assert.Contains("function fanout(read, params, specs)", js, StringComparison.Ordinal); + Assert.Contains("a data panel must explain its own empty state.", js, StringComparison.Ordinal); + } + + // ── helpers ──────────────────────────────────────────────────────────────────────────────────── + + /// Every read name the module mentions. Read names live in their own get_* namespace plus + /// three one-off verbs; nothing else in this module is a string literal of that shape. + private static HashSet ReadNamesIn(string js) => + Regex.Matches(js, "\"(get_[a-z0-9_]+|audit_config|list_servers|compare_analysis)\"") + .Select(m => m.Groups[1].Value) + .ToHashSet(StringComparer.Ordinal); + + /// + /// The (read, param-keys) pairs the module sends. Both descriptor forms are covered: the positional helpers + /// (table("T", "read", { ... })) and the direct readTool("read", { ... }) calls in the + /// composites. The params object is matched non-greedily up to its closing brace, which is exact here because + /// no params object in this file nests another. + /// + private static IEnumerable<(string Read, string[] Keys)> ParamsSentIn(string js) + { + foreach (Match m in Regex.Matches(js, "\"(get_[a-z0-9_]+|audit_config)\",\\s*\\{([^{}]*)\\}", RegexOptions.Singleline)) + { + var keys = Regex.Matches(m.Groups[2].Value, @"(?:^|[,{]\s*)([a-z_][a-z0-9_]*)\s*(?::|,|\})") + .Select(k => k.Groups[1].Value) + .Distinct(StringComparer.Ordinal) + .ToArray(); + yield return (m.Groups[1].Value, keys); + } + } + + private static string ReadRepoFile(string relative, [CallerFilePath] string thisFile = "") + { + for (var dir = new DirectoryInfo(Path.GetDirectoryName(thisFile)!); dir is not null; dir = dir.Parent) + { + var candidate = Path.Combine(dir.FullName, relative); + if (File.Exists(candidate)) + { + return File.ReadAllText(candidate).Replace("\r\n", "\n", StringComparison.Ordinal); + } + } + + throw new FileNotFoundException($"Could not locate {relative} walking up from {thisFile}"); + } +} diff --git a/Darling/Darling.Tests/ViewerGridPayloadColumnOrderPinTests.cs b/Darling/Darling.Tests/ViewerGridPayloadColumnOrderPinTests.cs index 34920f25d5..8c75d1ad71 100644 --- a/Darling/Darling.Tests/ViewerGridPayloadColumnOrderPinTests.cs +++ b/Darling/Darling.Tests/ViewerGridPayloadColumnOrderPinTests.cs @@ -25,8 +25,9 @@ namespace Darling.Tests; /// and assert the collector WRITE payload into the store, never a DataGrid. Worst case before the /// fix was QueryStoreGrid, where the query text was column 55 of 55. /// -/// The pin is the table below, transcribed from the #1949 census, plus the web server page's -/// ACTIVE_COLUMNS array — the one web column list in the service that renders query text. For each +/// The pin is the table below, transcribed from the #1949 census, plus EVERY web column list in the service +/// that renders query text — one when this pin was written, six since the web server page grew sub-tabs +/// (#2475), which is why the web half is now a table of its own rather than a single hardcoded array. For each /// grid it records the time (or identity) ANCHOR and the payload columns that must follow it immediately. /// Adding a grid to this table whose payload columns still sit at the back FAILS — that is the point. /// @@ -63,8 +64,12 @@ private sealed record GridPin( private static readonly string ViewerQueryStatsHistory = Path.Combine(ViewerDir, "QueryStatsHistoryWindow.xaml"); private static readonly string ViewerQueryStoreHistory = Path.Combine(ViewerDir, "QueryStoreHistoryWindow.xaml"); + /* The web server page's descriptor arrays moved out of pages/server.js and into pages/server-tabs.js when + the page grew sub-tabs (#2475): server.js is now the shell (header, tab bar, range picker, panel grid) + and the tab registry owns every column array. The pin follows the array, not the filename it used to + sit in. */ private static readonly string ServerPageJs = Path.Combine( - "Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "pages", "server.js"); + "Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "pages", "server-tabs.js"); /* The #1949 move list for the Darling viewer. Comparison and FinOps grids carry no time column, so their anchor is the identity block the grid is ranked by (Database, or Score+Database on High Impact). @@ -162,20 +167,34 @@ public void PayloadColumnsFollowTheAnchor(string relativePath, string gridName) $"not behind the metrics. Full order: [{string.Join(", ", headers)}]"); } - [Fact] - public void WebServerPageShowsQueryTextRightOfCollectionTime() + /// + /// Every web column list that renders query text puts it immediately right of the anchor. + /// + /// This used to pin ACTIVE_COLUMNS alone, because it was the only such list in the service. The + /// web server page's sub-tabs (#2475) added five more, and a rule enforced on one of six lists is a rule that + /// will be broken on the other five — the same reason the XAML half is a table rather than a spot-check. A new + /// grid whose text sits behind its metrics FAILS here, which is the point. + /// + [Theory] + [InlineData("ACTIVE_COLUMNS", "collection_time", "query_text", 9)] + [InlineData("TOP_QUERY_COLUMNS", "database_name", "query_text", 10)] + [InlineData("QUERY_STORE_COLUMNS", "database_name", "query_text", 8)] + [InlineData("LONG_QUERY_COLUMNS", "event_time", "statement", 8)] + [InlineData("PLAN_CORRECTION_COLUMNS", "collection_time", "query_text", 8)] + [InlineData("BLOCKING_COLUMNS", "event_time", "blocked_sql_text", 9)] + [InlineData("DEADLOCK_COLUMNS", "deadlock_time", "victim_sql_text", 5)] + public void EveryWebGridWithQueryText_PutsItRightOfTheAnchor(string array, string anchor, string text, int minKeys) { - // The web twin of the Active Queries grid. ACTIVE_COLUMNS array order IS the rendered column order - // (the server page hands the array straight to the table renderer); query_text was index 8 of 9. - var keys = JsColumnKeys(ReadRepoFile(ServerPageJs), "ACTIVE_COLUMNS"); - Assert.True(keys.Count >= 9, $"only {keys.Count} ACTIVE_COLUMNS keys parsed — the scan is broken."); - Assert.Equal("collection_time", keys[0]); - Assert.Equal("query_text", keys[1]); - - // Without this, re-adding a second query_text at the BACK would leave the pin green — the same hole + // Array order IS the rendered column order — the page hands the array straight to the table renderer. + var keys = JsColumnKeys(ReadRepoFile(ServerPageJs), array); + Assert.True(keys.Count >= minKeys, $"only {keys.Count} {array} keys parsed — the scan is broken."); + Assert.Equal(anchor, keys[0]); + Assert.Equal(text, keys[1]); + + // Without this, re-adding a second text column at the BACK would leave the pin green — the same hole // EveryPinnedPayloadColumnIsUniqueInItsGrid closes on the XAML side. - Assert.True(keys.Count(k => k == "query_text") == 1, - $"query_text appears {keys.Count(k => k == "query_text")} times in ACTIVE_COLUMNS; expected once."); + Assert.True(keys.Count(k => k == text) == 1, + $"{text} appears {keys.Count(k => k == text)} times in {array}; expected once."); } [Fact] @@ -261,7 +280,7 @@ private static List JsColumnKeys(string js, string arrayName) { var marker = $"const {arrayName} = ["; var at = js.IndexOf(marker, StringComparison.Ordinal); - Assert.True(at >= 0, $"{arrayName} not found in server.js"); + Assert.True(at >= 0, $"{arrayName} not found in the scanned module"); var end = js.IndexOf("\n];", at, StringComparison.Ordinal); Assert.True(end >= 0, $"{arrayName} is unterminated"); return JsKey.Matches(js[(at + marker.Length)..end]).Select(m => m.Groups[1].Value).ToList(); diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/css/app.css b/Darling/PerformanceMonitor.Darling.Service/wwwroot/css/app.css index a5f4339102..b5960dae2a 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/css/app.css +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/css/app.css @@ -907,3 +907,58 @@ pre.code { color: var(--dim); margin-left: 0.3rem; } + +/* ─────────────────────────── server page: sub-tabs + range ─────────────────────────── */ + +/* The web port of ViewerServerTab.xaml's per-server TabControl. Real links (the tab id rides in the + hash), so a tab is bookmarkable and middle-clickable; the strip scrolls horizontally on a narrow window + rather than wrapping into a block that pushes the panels off-screen. */ +.subtabs { + display: flex; + gap: 0.15rem; + overflow-x: auto; + border-bottom: 1px solid var(--border); + margin-bottom: 1rem; + scrollbar-width: thin; +} +.subtab { + flex: 0 0 auto; + padding: 0.45rem 0.8rem; + font-size: 0.85rem; + color: var(--muted); + text-decoration: none; + border-bottom: 2px solid transparent; + white-space: nowrap; +} +.subtab:hover { color: var(--fg); background: rgba(127, 127, 127, 0.08); } +.subtab.active { color: var(--fg); font-weight: 600; border-bottom-color: var(--accent); } +.subtab:focus-visible { outline: 2px solid var(--muted); outline-offset: -2px; } + +/* The tab's honesty note sits between the tab strip and the panels — it describes what this tab does NOT do, + so it belongs above the panels it qualifies rather than buried under them. */ +.subtabs + .strip.notice { margin-bottom: 1rem; } + +/* The page-level time-range preset picker (the web twin of the viewer's toolbar window presets). */ +.range-control { display: inline-flex; align-items: center; gap: 0.4rem; font-size: 0.78rem; color: var(--muted); } +.range-select-inline { + background: var(--card); + color: var(--fg); + border: 1px solid var(--border); + border-radius: 4px; + padding: 0.2rem 0.4rem; + font-size: 0.78rem; +} + +/* The in-panel picker row (wait-type / perfmon-counter composites) — the single-select web twin of the desktop + viewer's checkbox pickers. Sits between a panel's table and its chart. */ +.picker-row { display: flex; align-items: center; gap: 0.5rem; margin: 0.85rem 0 0.4rem; } + +/* The server header's "why": the pre-banded reason sentence (when the fleet ranked this server) over the same + metric chips the fleet card carries. A band word with no way to ask why is #2422 on a new surface. */ +.server-why { margin: -0.4rem 0 1rem; } +.server-reason { font-size: 0.85rem; margin-bottom: 0.5rem; } +.server-reason.band-Critical { color: var(--err); } +.server-reason.band-Warning { color: var(--warn); } +.server-reason.band-Offline { color: var(--err); } +.server-reason.band-Healthy { color: var(--ok); } +.server-why .metric-bands { max-width: 900px; } diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/app.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/app.js index 5cf1ac6250..4dbae0b589 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/app.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/app.js @@ -11,7 +11,8 @@ * refresh loop. Routes: * #/fleet — Fleet Overview (default) * #/ag — Availability Group topology (#991; nav entry revealed only when the store has AG data) - * #/server/{name} — one server's detail + * #/server/{name} — one server's detail (Overview) + * #/server/{name}/{tab} — one server's detail, opened on a named sub-tab (pages/server-tabs.js) * #/alerts — fleet-wide Alert History * #/views — Custom Views list (#1563) * #/view/{id} — a saved custom view, rendered (#1563) @@ -48,7 +49,7 @@ const statusbar = document.getElementById("statusbar"); function currentRoute() { const h = location.hash || "#/fleet"; - if (h.startsWith("#/server/")) return { name: "server", param: decodeURIComponent(h.slice("#/server/".length)) }; + if (h.startsWith("#/server/")) return serverRoute(h.slice("#/server/".length)); if (h === "#/ag" || h === "#/ag/") return { name: "ag" }; if (h.startsWith("#/alerts")) return { name: "alerts" }; /* #/views (list) is checked before the #/view/ forms; and the /edit form is tested before the bare /view/. */ @@ -70,10 +71,25 @@ function currentRoute() { return { name: "fleet" }; } +/* #/server/{name} and #/server/{name}/{tab} — the sub-tab id rides in the hash so a tab is deep-linkable and + survives the 60s refresh (which re-renders the whole route). The name is always encodeURIComponent'd by + navigateServer, so a '/' inside a server name arrives as %2F and this split is unambiguous; a hand-typed hash + carrying a RAW '/' in the name was already ambiguous before the tab segment existed. An unknown tab id is not + an error — findServerTab falls back to Overview — so every pre-existing #/server/{name} link still works. */ +function serverRoute(rest) { + const slash = rest.indexOf("/"); + if (slash < 0) return { name: "server", param: decodeURIComponent(rest) }; + return { + name: "server", + param: decodeURIComponent(rest.slice(0, slash)), + tab: decodeURIComponent(rest.slice(slash + 1)), + }; +} + function route() { const r = currentRoute(); setActiveNav(r); - if (r.name === "server") renderServer(main, r.param); + if (r.name === "server") renderServer(main, r.param, r.tab); else if (r.name === "ag") renderAg(main); else if (r.name === "alerts") renderAlerts(main); else if (r.name === "views") renderViewList(main); diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/fleet.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/fleet.js index 85d2ef3258..3b76669f30 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/fleet.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/fleet.js @@ -498,8 +498,14 @@ function serverCard(c) { /* Enriched metric chips (M1): each carries a secondary detail line from fields /api/fleet already returns — the SQL-vs-total CPU split, threads available/max, memory + buffer-pool GB, blocking max wait, deadlocks - last-seen, and collectors healthy/failing. */ -function metricBands(c) { + last-seen, and collectors healthy/failing. + + EXPORTED because the server detail page's header renders the same chips under its band badge. `Warning` has + three unrelated causes — a real metric breach, awaiting-first-collection, and a collector error — and a badge + that says only "Warning" is #2422 rebuilt on a new surface. These chips are the answer, and they are the + SERVER's severities read off the card (R1), so the two surfaces cannot drift into different opinions the way + a second derivation would. */ +export function metricBands(c) { const threadsValue = c.threads_severity === "Unknown" ? "n/a" diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js new file mode 100644 index 0000000000..05c42b3ceb --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js @@ -0,0 +1,1646 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +/* + * The server page's TAB REGISTRY — the web port of the desktop viewer's per-server TabControl + * (Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml, ~65 TabItems across 38 partials). + * + * Every entry is `{ id, label, note?, build(server, ctx) }` and `build` returns an array of nodes, almost all of + * them PANEL DESCRIPTORS run through the unmodified renderPanel (the #1563 seam): a `read` naming an MCP tool + * served at GET /api/read/{read}, `params`, and a `viz` from the four-kind registry in panels.js. No fifth viz + * kind was added — see the PR for why the property-grid shape that tempted one is served honestly by `stat` + * (the reads that return a flat object) and `table` (the reads that already return rows). + * + * `ctx.hours` is the page's time range, the web twin of ViewerServerTab.TimeRange.cs's preset picker. Panels + * whose read takes no time window ignore it and say so in their subtitle, because a panel labelled "last 6 + * hours" that is really a latest-snapshot read is a lie the reader cannot see. + * + * TWO HONESTY RULES run through this file: + * 1. Every panel carries an `emptyText` saying WHY it could be empty in the reader's own terms. A read that + * returns the {status,message} envelope supplies its own (better) sentence and renderPanel shows that + * instead; emptyText only covers the data-arrived-but-zero-rows case, which would otherwise read as a + * blank rectangle. + * 2. A tab whose desktop twin does something the browser genuinely cannot do carries a `note` naming it and + * pointing at the desktop viewer. A reader told "open the desktop viewer for plan analysis" is better + * served than one given a web page that looks like plan analysis and is not. + * + * R4 (XSS): every value reaches the DOM through renderPanel/util.el's text path. The two custom cell renderers + * here (query text, XML) build a 
 through el() and never touch innerHTML.
+ */
+
+import { el, readTool, mount, loadingStrip, errorStrip, emptyStrip, disclosure, noticeStrip } from "../util.js";
+import { renderPanel, VIZ } from "../panels.js";
+import { renderLineChart, SERIES_COLORS } from "../charts.js";
+
+/* ─────────────────────────── shared cell renderers ─────────────────────────── */
+
+/** Query-text cell: a truncated one-liner that expands to the full statement (mono) — the B2 disclosure. */
+function codeDisclosure(text) {
+  if (text == null || text === "") return document.createTextNode("—");
+  return disclosure(text, el("pre", { class: "code" }, [text]), { max: 100 });
+}
+
+/** XML cell: the same disclosure over a captured payload (blocked-process report, deadlock graph). The desktop
+ *  viewer renders these as a graph; the browser has no graph viewer, so it hands over the payload verbatim
+ *  rather than pretending. The tab's note says where the graph lives. */
+function xmlDisclosure(text) {
+  if (text == null || text === "") return document.createTextNode("—");
+  return disclosure("XML capture (" + String(text).length.toLocaleString() + " chars)", el("pre", { class: "code" }, [text]), {
+    max: 60,
+  });
+}
+
+/* ─────────────────────────── composites ─────────────────────────── */
+
+/* Two panels chain reads or reshape rows, so they are built by hand rather than declared. Both were already on
+   the page before the tabs existed; they keep their behaviour and move into the tab that owns them. */
+
+function panelShell(title, subtitle, span = 2) {
+  const body = el("div", { class: "panel-body" }, [loadingStrip()]);
+  const panel = el("div", { class: "panel card" + (span === 2 ? " span-2" : "") }, [
+    el("h3", {}, [title, subtitle ? el("span", { class: "panel-sub", text: " " + subtitle }) : null]),
+    body,
+  ]);
+  return { panel, body };
+}
+
+/**
+ * Several panels over ONE fetch of one read.
+ *
+ * A descriptor owns its own fetch, which is the right default and is what makes the seam composable — but a
+ * read that feeds two or three panels on the SAME tab then runs two or three times, and `readTool`/`apiGet` have
+ * no cache to absorb it. That is a real cost rather than a tidiness point: `get_collection_health` rolls up
+ * seven days of collector logs and computes sweep pressure, and the Collection Health tab renders three slices
+ * of one payload (`sweep_pressure.*`, `collectors`, `sweep_pressure.heaviest_collectors`), so opening it was
+ * running that query three times. Same shape for plan corrections (recommendations + automatic tuning), plan
+ * cache (summary + per-type), sessions (summary + per-application) and system_health (chart + entries).
+ *
+ * Each spec is an ordinary panel descriptor minus `read`/`params` — the same viz registry, the same three-kind
+ * response mapping renderPanel does — so nothing about the seam changes except how many times the wire is used.
+ */
+function fanout(read, params, specs) {
+  for (const spec of specs) {
+    if ((spec.viz === "table" || spec.viz === "line") && !spec.emptyText) {
+      throw new Error("fanout(" + spec.title + "): a data panel must explain its own empty state.");
+    }
+  }
+  const shells = specs.map((s) => panelShell(s.title, s.subtitle, s.span ?? 2));
+  (async () => {
+    const res = await readTool(read, params);
+    specs.forEach((spec, i) => {
+      const body = shells[i].body;
+      if (res.kind === "error") return mount(body, errorStrip(res.message));
+      if (res.kind === "empty") return mount(body, emptyStrip(res.message));
+      try {
+        mount(body, VIZ[spec.viz](res.data, spec));
+      } catch (e) {
+        mount(body, errorStrip("Could not render this panel: " + (e && e.message ? e.message : String(e))));
+      }
+    });
+  })();
+  return shells.map((s) => s.panel);
+}
+
+/**
+ * Wait Stats table + a trend for ONE wait type, chosen from a picker seeded with the heaviest.
+ *
+ * The desktop viewer's Wait Stats tab is a checkbox list of wait types over a multi-series chart. This is the
+ * single-select version of the same idea: the picker's options are the rows of the table directly above it,
+ * heaviest first, so the reader is choosing from what they can already see rather than from a second list that
+ * may disagree with it. That is also why get_wait_types is NOT read here — it returns the full distinct set,
+ * which would offer wait types absent from the table and make the two disagree.
+ */
+export function waitsPanel(server, ctx) {
+  const { panel, body } = panelShell("Wait Stats", ctx.label + ", with a trend for the wait you pick");
+  (async () => {
+    const res = await readTool("get_wait_stats", { server, hours: ctx.hours, limit: 20 });
+    if (res.kind === "error") return mount(body, errorStrip(res.message));
+    if (res.kind === "empty") return mount(body, emptyStrip(res.message));
+
+    const waits = res.data.waits || [];
+    const parts = [VIZ.table(res.data, { rowsKey: "waits", columns: WAIT_COLUMNS })];
+
+    if (waits.length) {
+      const chartSlot = el("div", {}, [loadingStrip()]);
+      const picker = pickerControl(
+        "Trend",
+        waits.map((w) => w.wait_type),
+        (waitType) => drawWaitTrend(chartSlot, server, ctx, waitType)
+      );
+      parts.push(el("div", { class: "picker-row" }, [picker]), chartSlot);
+      drawWaitTrend(chartSlot, server, ctx, waits[0].wait_type);
+    }
+    mount(body, parts);
+  })();
+  return panel;
+}
+
+async function drawWaitTrend(slot, server, ctx, waitType) {
+  mount(slot, loadingStrip());
+  const trend = await readTool("get_wait_trend", { server, wait_type: waitType, hours: ctx.hours });
+  if (trend.kind !== "data") {
+    mount(slot, trend.kind === "empty" ? emptyStrip(trend.message) : errorStrip(trend.message));
+    return;
+  }
+  mount(
+    slot,
+    renderLineChart({
+      points: trend.data.trend || [],
+      xKey: "time",
+      series: [
+        { key: "wait_time_ms_per_second", label: "Wait ms/s", color: SERIES_COLORS[0] },
+        { key: "signal_wait_time_ms_per_second", label: "Signal ms/s", color: SERIES_COLORS[1] },
+      ],
+      formatValue: (v) => Math.round(v).toLocaleString(),
+      unit: "ms/s",
+    })
+  );
+}
+
+/**
+ * Perfmon counters: a picker over the counters this server actually collects, charting the chosen one.
+ *
+ * The desktop viewer's Perfmon tab is a searchable counter list over a multi-series chart. get_perfmon_trend
+ * REQUIRES a counter_name, so without a picker the read is unreachable from the browser — which is why the
+ * options come from get_perfmon_stats (the latest snapshot's counter list) rather than being hardcoded: a
+ * hardcoded name is exactly how you end up charting a counter this server does not collect.
+ *
+ * When the trend read still comes back empty it can carry hints.collected_counters. Its message tells the
+ * reader to "see hints.collected_counters", which is a JSON path no browser reader can open, so the hint list
+ * is rendered here instead of being dropped.
+ */
+export function perfmonPanel(server, ctx) {
+  const { panel, body } = panelShell("Perfmon Counters", "latest snapshot, with a trend for the counter you pick");
+  (async () => {
+    const res = await readTool("get_perfmon_stats", { server });
+    if (res.kind === "error") return mount(body, errorStrip(res.message));
+    if (res.kind === "empty") return mount(body, emptyStrip(res.message));
+
+    const names = [...new Set((res.data.counters || []).map((c) => c.counter_name).filter(Boolean))].sort();
+    if (!names.length) return mount(body, emptyStrip("The latest snapshot holds no perfmon counters."));
+
+    /* The snapshot table lives HERE rather than in its own descriptor: this composite has already fetched the
+       exact payload it would render, and a second panel would have paid for get_perfmon_stats twice to show the
+       list the picker above it is built from. */
+    const chartSlot = el("div", {}, [loadingStrip()]);
+    const picker = pickerControl("Counter", names, (name) => drawPerfmonTrend(chartSlot, server, ctx, name));
+    mount(body, [
+      VIZ.table(res.data, {
+        rowsKey: "counters",
+        columns: PERFMON_COLUMNS,
+        emptyText: "No perfmon counters in the latest snapshot.",
+      }),
+      el("div", { class: "picker-row" }, [picker]),
+      chartSlot,
+    ]);
+    drawPerfmonTrend(chartSlot, server, ctx, names[0]);
+  })();
+  return panel;
+}
+
+async function drawPerfmonTrend(slot, server, ctx, counterName) {
+  mount(slot, loadingStrip());
+  const trend = await readTool("get_perfmon_trend", { server, counter_name: counterName, hours: ctx.hours });
+  if (trend.kind === "error") return mount(slot, errorStrip(trend.message));
+  if (trend.kind === "empty") {
+    const hinted = trend.hints && Array.isArray(trend.hints.collected_counters) ? trend.hints.collected_counters : null;
+    mount(slot, [
+      emptyStrip(trend.message),
+      hinted && hinted.length
+        ? el("div", { class: "muted", style: "margin-top:0.4rem", text: "Collected here: " + hinted.join(", ") })
+        : null,
+    ]);
+    return;
+  }
+  mount(
+    slot,
+    renderLineChart({
+      points: trend.data.trend || [],
+      xKey: "time",
+      series: [
+        { key: "value", label: "Value", color: SERIES_COLORS[0] },
+        { key: "delta_value", label: "Delta", color: SERIES_COLORS[1] },
+      ],
+      formatValue: (v) => Number(v).toLocaleString(undefined, { maximumFractionDigits: 2 }),
+    })
+  );
+}
+
+/** A labelled