Skip to content

Web server page: twelve sub-tabs over the existing renderPanel seam - #2477

Merged
erikdarlingdata merged 7 commits into
devfrom
feat/web-viewer-depth
Aug 21, 2026
Merged

Web server page: twelve sub-tabs over the existing renderPanel seam#2477
erikdarlingdata merged 7 commits into
devfrom
feat/web-viewer-depth

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Closes #2475.

The service already dispatches 82 read handlers at GET /api/read/{name}, and panels.js has been a generic renderer over them since #1562renderPanel({read, params, viz, span}) with a four-kind viz registry whose own header says the seam is stable and nothing downstream of it is built. The built-in pages reached 23 of the 82 reads. #/server/{name} was one scroll of seven panels against the desktop viewer's 65 TabItems.

So none of this is backend work. It is descriptors.

What lands

pages/server.js becomes the shell — header, sub-tab bar, time-range picker, panel grid — and the new pages/server-tabs.js is the registry: eleven tabs, ~70 panels, reaching 61 of the 82 reads.

Tab What it carries
Overview server summary, properties, CPU, memory, blocking + deadlock trends, file I/O latency, analysis findings, daily summary
Wait Stats wait table with a trend for the wait you pick, waiting tasks, latches, spinlocks
CPU utilization, scheduler pressure, top queries, top procedures
Memory summary, trend, clerks, grants, resource semaphore, pressure events, plan cache + breakdown
Blocking blocking and deadlock trends, blocking events, deadlocks, deadlock graph XML, blocked-process report XML, object contention
File I/O latency per database, per-file stats, tempdb, database sizes, table/index sizes, PVS
Queries active queries, duration trend, top queries, top procedures, Query Store, long-query completions, plan corrections, automatic tuning
Configuration properties, config audit, sp_configure, database config, Query Store health, trace flags
Config Changes server, database and trace-flag change history
Activity perfmon counter picker + trend, perfmon snapshot, sessions, sessions by application, running jobs, index usage
System Events the eight system_health parse-on-read families and the default trace
Collection Health sweep pressure, collectors, heaviest collectors

The tab id rides in the hash — #/server/{name}/{tab} — so a section is deep-linkable and survives the 60s refresh, which re-renders the whole route. An unknown or absent id resolves to Overview rather than erroring, and that fallback is what keeps every pre-existing #/server/{name} link working. The router splits the hash before decoding, so an encoded / inside a server name still round-trips.

A page-level range picker (1h / 4h / 12h / 24h / 7d / 30d) is the web twin of ViewerServerTab.TimeRange.cs's toolbar presets. It is module state like the fleet page's sort, so it survives the refresh, and it is deliberately not persisted: a page that reopens on a 30-day window is slow for a reason the reader cannot see. Panels whose read takes no time window at all say "latest snapshot" in their own subtitle 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; both now have a picker. The wait picker is seeded from the rows of the table directly above it rather than from get_wait_types, so the reader is choosing from what they can already see — the full distinct set would offer wait types absent from the table and make the two disagree.

Two things deliberately not done

No fifth viz kind. The shape that tempted one is the property grid — a read returning a flat object with a dozen scalars. stat handles it (get_memory_stats, get_server_properties, get_cpu_scheduler_pressure), and table handles every read that already returns rows. A fifth kind added only to panels.js's VIZ would be a page-only special case, and adding it properly means KnownVizList (or the composer cannot offer it), derive.js's deriveVizConfig, and an editor config arm — composer surface area this change does not need.

Nothing seeded as stored views. Curated views as data would need a migration rung, StorageVersion bump, four pinned test files and a viewer probe sentinel, for content that is not schema — and a seeded row resurrects itself on the next upgrade after a user deletes it. Code descriptors are covered by the invariant below; a stored view's read name is validated once at write time and never again.

Honesty, in the two places it matters

Every table panel supplies its own empty-state sentence, and table() throws without one. renderPanel already shows a read's {status,message} envelope when the read has nothing — better wording than any descriptor could carry — but that does not cover the read returning data whose row array is empty, where vizTable falls back to a generic "No rows in this window". On a collector that is off, opt-in, or daily, that reads as a fault. The DOM-shim run builds every tab, so a missing sentence fails there rather than shipping as a blank rectangle.

A tab whose desktop twin does something the browser cannot do says so, in the tab. Plan analysis, the query heatmap, cached-plan retrieval and actual-plan re-execution need a plan renderer and a Stage-2 command back to the monitored server; the block-chain view and the interactive deadlock graph need a graph viewer. Blocking hands over the captured blocked-process-report and deadlock-graph XML verbatim rather than imitating the 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.

Verification

The repository has no JavaScript test runner, so this follows the #2447 / #2451 idiom: xUnit text-scan pins over the shipped module, plus running the shipped JS under a minimal DOM shim.

The pins are invariants, not spot-checks. With ~60 read names spread across eleven tabs, a typo is a 400 inside one panel at runtime and completely invisible to inspection. So ServerPageTabsTests asserts the category:

  • every read name the module mentions exists in BuildReadDispatch();
  • every parameter key it sends is one that read actually binds, per CatalogDescriptors — this is the half that fails silently, since an unknown query key is ignored and limit sent to a read that binds top quietly returns the default 20 rows;
  • every viz named is in KnownVizList, and panels.js's registry is that same vocabulary;
  • the sub-tab bar is built from the registry and the router carries the tab back;
  • no get_pg_* read appears — see below.

Both sides of each check come from the shipped artifacts, never a transcribed list.

The DOM-shim run exercised the shipped server-tabs.js and server.js under a stubbed fetch across four response shapes — the empty envelope, an error, data with no rows, and data with rows — at three time ranges, plus renderServer with an absent, valid and unknown tab id. Zero throws in all cases; 142 distinct requests issued, every one of them checked against CatalogDescriptors for parameter validity. Zero problems.

Negative controls, all three run. A bogus read name fails the dispatch pin; limit swapped for top on get_latch_stats fails the parameter pin naming both the key and the allowed set; a get_pg_* read fails the engine pin. Removing one emptyText fails the shim run. Each was restored afterwards.

The xUnit file builds (Darling.Tests, EnableWindowsTargeting) but cannot execute on macOS. To avoid deferring it to CI, all eight facts were compiled into a throwaway net10.0 console project against the real PerformanceMonitor.Darling.Service build with a minimal xUnit shim, so the assertions that ran are the shipped ones rather than a retyped copy. All eight pass.

The backend gap this change names but does not close

DarlingFleetReader.FleetServerCard carries engine_edition — the SQL Server SERVERPROPERTY value — and no CollectorTargetEngine discriminator. The browser therefore cannot tell a PostgreSQL target from a SQL Server one, so a get_pg_* panel added to these tabs would render on every SQL Server in the fleet, permanently empty. NoPostgresRead_IsOnTheServerPage_UntilTheFleetPayloadCanTellTheEngines fails until the discriminator exists, rather than letting the panel ship first. The other unreachable readers, and the nested-array shape get_database_scoped_config returns, are enumerated on #2475.

🤖 Generated with Claude Code


Since opening

Twelve tabs, not eleven. The prose said eleven while the registry held twelve. The count is now pinned exactly rather than as a floor, so it cannot drift again.

Two existing pins broke, and fixing only them would have been the scenario-shaped repair. DarlingEmptyEnumerationNoteTests and ViewerGridPayloadColumnOrderPinTests scan the web server page by filename for COLLECTOR_COLUMNS and ACTIVE_COLUMNS, and those arrays moved into server-tabs.js. Both now scan where the arrays live. But the query-text pin asserted #1949 ordering on one array because that was the only web list rendering query text — the sub-tabs added five more, and a rule enforced on one of six is a rule that gets broken on the other five. It is a [Theory] over all seven now, and the six new grids were reordered to obey it: Top Queries, Query Store, Blocking and Deadlocks had their text behind their metrics, and Plan Corrections had it dead last.

The server header's band badge said "Warning" and nothing else. Warning has three unrelated causes — a genuine metric breach, a server awaiting its first collection, a collector error — so that badge was #2422 rebuilt on a new surface. It now carries the fleet's own pre-computed reason when the server is in the worst-first ranking, over the fleet card's severity chips, rendered by fleet.js's own metricBands (exported, not reimplemented). A server outside the ranking gets the chips and no sentence: inventing one there is the second derivation the fix exists to refuse.

An empty trend is not a warming-up trend. get_blocking_trend and get_deadlock_trend answer an idle server with trend: [] and no {status,message} envelope — so renderPanel had real data in hand and vizLine fell through to the chart's "Not enough data points to chart yet". A server with no blocking was being told its blocking chart was still warming up. vizLine now renders a descriptor's emptyText at exactly zero rows; one row still gets the chart's own sentence (there it is the true one), and a descriptor with no emptyText still falls through, so every stored view authored before this is unchanged. line() requires the sentence the way table() does. Verified against the shipped vizLine across all three cases, negative control run.

docs/uat-onboarding.md §3.4 is updated here, not deferred. #2476 listed the wait-type picker, tempdb, the perfmon picker, Running Jobs and the configuration grids as missing; all five ship on this branch, and a UAT tester reading absences that are no longer absent is worse served than by no list. Two counts in it were also wrong and are corrected from the XAML: the viewer has nineteen top-level per-server tabs, not fourteen, and 65 TabItems counting the nested ones. What remains missing is now divided by why, because the three groups have different answers — reads served but unshown, viewer data with no read endpoint at all (now #2484), and desktop things a web imitation would be worse than. The exclusion bullets above that paragraph are untouched: no plan analysis, no analyze_server, no config_command enqueue, no alert tuning. Nothing here changes them.

…2475)

The service dispatches 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 TabItems, so the whole gap was unbuilt UI
over a stable seam rather than backend work.

pages/server.js becomes the shell (header, sub-tab bar, range picker,
panel grid) and pages/server-tabs.js is the registry: eleven tabs of
descriptors run through the unmodified renderPanel, reaching 61 reads.
The tab id rides in the hash so a section is deep-linkable and survives
the 60s refresh; an unknown or absent id resolves to Overview, which is
what keeps every existing #/server/{name} link working.

Two reads were unreachable from a browser because they need a parameter
no UI collected — get_wait_trend's wait_type and get_perfmon_trend's
counter_name — and both now have a picker.

No fifth viz kind. The property-grid shape that tempted one is served by
stat and table; a fifth kind living only in panels.js would be a
page-only special case, and doing it properly means KnownVizList,
derive.js and an editor config arm.

What the browser cannot do is said in the tab where a reader looks for
it. 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; the block-chain view and the deadlock graph need a
graph viewer. Blocking hands over the captured XML verbatim rather than
imitating the graph.

Guarded by invariants rather than spot-checks: every read name the
module mentions must exist in the dispatch, every parameter key must be
one its read binds (an unknown 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 the engines apart. All three negative controls were
run: each fails without its fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review

Went through server.js / server-tabs.js / app.js / the new ServerPageTabsTests.cs against CatalogDescriptors in DarlingWebEndpoints.cs. Manually cross-checked roughly 30 of the ~70 panel descriptors' read + rowsKey/stats paths against the actual C# reader return shapes (DarlingMcpDataTools, DarlingMcpBlockingTools, DarlingMcpMemoryGrantTools, DarlingMcpPlanCorrectionTools, DarlingMcpObjectStatsTools, ...) — all matched. The route encode/decode round-trip (navigateServer -> serverRoute) is sound, and there's no XSS surface (el()/text-node paths only, no innerHTML). No Lite-parity concern — this only touches the Darling web service, and doesn't correspond to anything in Lite/.

Two things worth a look, left as inline comments:

  1. Several tabs fetch the same expensive read multiple times per page load with no dedup layer. readTool/apiGet has no caching (confirmed in util.js), so every panel descriptor sharing a read triggers its own independent network round trip + backend execution. Concretely: Memory tab calls get_plan_cache_bloat twice, Queries tab calls get_plan_corrections twice (and the 'Automatic Tuning' panel's own window/limit params are pure waste — that field comes from an unconditional latest-snapshot query, not the windowed one), Activity tab calls both get_perfmon_stats and get_session_stats twice each, and Collection Health calls get_collection_health three times. That read in particular is documented as scanning 7 days of collector logs — tripling it every time that tab opens seems like an easy regression to avoid by fetching once per unique (read, params) and handing the payload to each panel.

  2. Possible out-of-order render race in the new wait-type/perfmon-counter pickers. drawWaitTrend/drawPerfmonTrend mount straight into a persistent chartSlot with no request-generation guard. If a reader flips the <select> quickly, a slower in-flight response for the previous selection can resolve after the newer one and overwrite the chart — so the chart can end up showing the wrong wait type/counter with no visible sign anything's wrong.

Nothing else stood out — the invariant tests (EveryReadTheServerPageNames_ExistsInTheDispatch, EveryParameterKeyTheServerPageSends_IsOneItsReadBinds, etc.) are a good defense against the read-name/param-key typo class of bug this file is most exposed to.

Comment on lines +633 to +640
table(
"Automatic Tuning",
"get_plan_corrections",
{ server, hours: ctx.hours, limit: 50 },
"automatic_tuning",
AUTO_TUNING_COLUMNS,
SNAPSHOT,
"No per-database FORCE_LAST_GOOD_PLAN state recorded.",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This panel re-issues get_plan_corrections with the same {server, hours, limit} as the "Plan Corrections" panel just above (line 623-630) — a second network round trip and a second GetPlanCorrectionsAsync execution to get data this panel doesn't even use. Looking at DarlingMcpPlanCorrectionTools.GetPlanCorrections, automatic_tuning comes from GetLatestAutomaticTuningAsync, which is unconditional and ignores hours_back/limit entirely — so the hours/limit params sent here are dead weight on top of being a duplicate fetch. Worth having both panels share one fetch of get_plan_corrections (e.g. build the response once per tab and pass slices to table()), same as the get_collection_health triple-fetch below in the Collection Health tab.

…#2475)

CI caught the exact defect class this repo keeps relearning — a call site
that never learned a concept moved. DarlingEmptyEnumerationNoteTests and
ViewerGridPayloadColumnOrderPinTests scan the web server page BY FILENAME
for COLLECTOR_COLUMNS and ACTIVE_COLUMNS, and those arrays moved into
pages/server-tabs.js when the page grew sub-tabs. Both now scan where the
arrays live, and say so.

Fixing only the instance would have been the scenario-shaped repair. The
query-text pin asserted #1949 ordering on ONE array because that was the
only web list rendering query text; the sub-tabs added five more, and a
rule enforced on one of six is a rule that gets broken on the other five.
It is a Theory over all seven now, and the six new grids were reordered
to obey it — Top Queries, Query Store, Plan Corrections, Blocking and
Deadlocks all had their text behind their metrics or (Plan Corrections)
dead last.

The server header's band badge said "Warning" and nothing else. Warning
has three unrelated causes — a real metric breach, a server awaiting its
first collection, a collector error — so that badge is #2422 rebuilt on a
new surface. It now carries the fleet's own reason string when the server
is in the ranking, over the fleet card's severity chips, rendered by
fleet.js's own metricBands so there is one implementation rather than
two. Neither is derived in the browser.

Twelve tabs, not eleven. The prose said eleven while the registry held
twelve, so the count is pinned exactly rather than as a floor.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@erikdarlingdata erikdarlingdata changed the title Web server page: eleven sub-tabs over the existing renderPanel seam Web server page: twelve sub-tabs over the existing renderPanel seam Aug 21, 2026
Comment on lines +883 to +902
stat("Sweep Pressure", "get_collection_health", { server }, SWEEP_STATS, "trailing 7 days", 2),
table(
"Collectors",
"get_collection_health",
{ server },
"collectors",
COLLECTOR_COLUMNS,
"trailing 7 days",
"No collection log rows for this server yet."
),
table(
"Heaviest Collectors",
"get_collection_health",
{ server },
"sweep_pressure.heaviest_collectors",
HEAVIEST_COLUMNS,
"trailing 7 days",
"No per-collector timings recorded yet."
),
],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three panels ("Sweep Pressure", "Collectors", "Heaviest Collectors") each independently call get_collection_health with identical {server} params — three separate fetches/backend executions of a read that's documented (in DarlingMcpDataTools) as rolling up 7 days of collector logs plus the sweep-pressure computation, just to render three different slices (sweep_pressure.*, collectors, sweep_pressure.heaviest_collectors) of the same payload. Since table()/stat() go through renderPanel independently with no shared cache (readTool/apiGet in util.js has none), opening this tab triples a query that's explicitly called out elsewhere in this PR's own docs as one of the heavier ones in the fleet. Might be worth fetching once and handing the parsed object to all three panel builders instead of three descriptor-level reads.

erikdarlingdata and others added 2 commits August 21, 2026 21:31
#2476's "What it will not do" listed the wait-type picker, tempdb, the
perfmon picker, Running Jobs and the configuration grids as missing. All
five ship on this branch, and a UAT tester reading a list of absences
that are no longer absent is worse served than by no list at all.

Two counts were also wrong and are corrected from the XAML: the viewer
has nineteen top-level per-server tabs, not fourteen, and 65 TabItems
counting the nested ones.

What remains missing is now divided by WHY, because the three groups have
different answers. Reads the service serves that no page shows (the eight
PostgreSQL ones, blocked by the fleet payload carrying no target-engine
discriminator; the nested-array scoped config; store-wide metrics).
Viewer data with no read endpoint at all, which no web work reaches.
And the desktop things a web imitation would be worse than — plan
viewer, heatmap, block chain, deadlock graph, and the grids' own
filter/export/repro/drill-down affordances.

The exclusion bullets above it are untouched: no plan analysis, no
analyze_server, no config_command enqueue, no alert tuning. Those are
the viewer role's grants and the web host having no write path, and
nothing here changes them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread CHANGELOG.md Outdated
## [Unreleased]

### Added
- **The web dashboard's server page gets the viewer's tabs: eleven 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 table panel supplies its own empty-state sentence and the helper THROWS without one, because vizTable's generic "No rows in this window" reads as a fault on a collector that is off, opt-in, or daily. 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Headline says "eleven sections" but the rest of the same bullet says "twelve sub-tabs" and lists all twelve (Overview, Wait Stats, CPU, Memory, Blocking, File I/O, Queries, Configuration, Config Changes, Activity, System Events, Collection Health) — matching SERVER_TABS and the new Assert.Equal(12, ids.Length) pin in ServerPageTabsTests.cs. This is exactly the "eleven vs twelve" drift ServerPageTabsTests.cs (line ~254) says it was written to catch — it just caught it in the CHANGELOG bullet's own bolded lead-in rather than the code. docs/uat-onboarding.md's "twelve sub-tabs" wording is correct; this line should match.

Suggested change
- **The web dashboard's server page gets the viewer's tabs: eleven 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 table panel supplies its own empty-state sentence and the helper THROWS without one, because vizTable's generic "No rows in this window" reads as a fault on a collector that is off, opt-in, or daily. 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.
- **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 table panel supplies its own empty-state sentence and the helper THROWS without one, because vizTable's generic "No rows in this window" reads as a fault on a collector that is off, opt-in, or daily. 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.

get_blocking_trend and get_deadlock_trend answer an idle server with
`trend: []` and NO {status,message} envelope, so renderPanel had real
data to hand and vizLine fell through to renderLineChart's "Not enough
data points to chart yet". A server with no blocking was being told its
blocking chart was still warming up — the exact confusion between "did
not happen" and "was not collected" this codebase keeps closing
everywhere else.

vizLine now renders a descriptor's emptyText at EXACTLY zero rows. One
row still gets the chart's own sentence, because there it is the true
statement; and a descriptor with no emptyText still falls through, so
every stored view authored before this is unchanged.

line() requires the sentence the way table() does, and every tab is
built during the DOM-shim run, so a chart panel that forgot one cannot
reach a browser. Verified against the shipped vizLine directly: zero
points renders the descriptor's sentence, one point renders the chart's,
zero-without-emptyText still renders the chart's. Negative control run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review

Purely a Darling-web-frontend change (pages/server-tabs.js new, pages/server.js split into a shell, fleet.js export, plus the C# text-scan pins). No T-SQL or Lite files touched, and this feature has no Lite counterpart to drift from (Lite is a separate WPF app with no web dashboard) — no parity concern.

Verified by hand against DarlingWebEndpoints.cs's BuildReadDispatch/CatalogDescriptors: every read name and parameter key used in the new tab registry (top vs limit per-read, hours vs the read's actual default, get_object_locking taking no hours, etc.) — all correct, and independently confirmed by the new ServerPageTabsTests pins. getPath() in panels.js already supports the dotted rowsKey used for sweep_pressure.heaviest_collectors, so that one isn't a landmine. el()/disclosure()/noticeStrip() stay on the textContent/setAttribute path — no innerHTML, no XSS surface from server-supplied strings (query text, wait types, perfmon counter names) reaching the DOM. The #/server/{name}/{tab} hash split correctly handles a %2F-encoded server name (decode-after-split), and an unknown tab id/absent id falls back to Overview as claimed.

One inline finding: the new CHANGELOG.md bullet's own bolded headline says "eleven sections" while the rest of the same sentence, docs/uat-onboarding.md, and the shipped code (Assert.Equal(12, ids.Length) in ServerPageTabsTests.cs) all say twelve. Left a suggestion inline.

Minor, non-blocking observation: redrawPanels() rebuilds and refetches every panel on the active tab whenever the page-level range picker changes, even for tabs whose panels are all latest-snapshot/no-window reads (e.g. Configuration, Collection Health) that ignore ctx.hours entirely. Not a bug — just a few redundant fetches when a reader flips the range on one of those tabs — and not something the PR needs to fix given the added complexity/benefit tradeoff.

No other correctness, security, or performance issues found.

The group of viewer data with no /api/read endpoint is genuine backend
scope, not a paragraph. #2484 enumerates it with the reader behind each
item and the finding that matters for scoping: the viewer reads the same
Postgres store the MCP does, so every one is a missing ENDPOINT rather
than missing collection.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review found two panels re-fetching a read a sibling panel on the same
tab had already fetched. There were six, across five tabs, so fixing the
two named would have been the scenario-shaped repair again.

readTool/apiGet have no cache, so a read feeding several panels ran
several times. The worst was get_collection_health — seven days of
collector-log rollup plus the sweep-pressure computation, rendered as
three slices of one payload, so opening that tab ran the page's heaviest
query three times. Also plan corrections (recommendations + automatic
tuning, where the second slice comes from an unconditional latest-
snapshot query that ignores hours/limit entirely), plan cache, sessions,
and system_health. The perfmon snapshot table folds into the composite
that had already fetched exactly that payload to build its picker.

fanout() takes one read and several ordinary panel descriptors, mapping
the same three response kinds renderPanel does, and carries the same
empty-state rule so routing a panel through it is never how the sentence
gets lost. Measured under the DOM shim: zero duplicate fetches across
all twelve tabs, Collection Health from three to one.

NoTab_FetchesTheSameReadTwice is the guard, because "fix the two review
named" is how the other four ship. It maps the composites' own reads
explicitly and asserts that map against the composites, so it cannot go
stale and start passing a tab it no longer describes. Negative control
run.

And the CHANGELOG headline still said eleven sections while its own body
said twelve — the drift the tab-count pin was written to catch, caught
in prose the pin cannot see.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@erikdarlingdata

erikdarlingdata commented Aug 21, 2026

Copy link
Copy Markdown
Owner Author

All three review findings addressed in 2c30e7c — and the first two were the visible part of a larger set.

The duplicate fetches: six, not two. readTool/apiGet have no cache, so a read feeding several panels on one tab ran several times. Scanning for the category rather than fixing the two named turned up get_plan_cache_bloat (memory), get_session_stats (activity), get_health_parser_system_health (events) and get_perfmon_stats (the composite had already fetched exactly what the standalone snapshot table re-fetched) alongside the two you found. New fanout() helper takes one read and several ordinary panel descriptors, mapping the same three response kinds renderPanel does and carrying the same empty-state rule, so routing a panel through it is never how the sentence gets lost.

Measured under a DOM shim, per tab: zero duplicate fetches across all twelve, Collection Health three fetches down to one. NoTab_FetchesTheSameReadTwice is the guard — it maps the composites' own reads explicitly and asserts that map against the composites, so it cannot go stale and start passing a tab it no longer describes. Negative control run: reintroducing one duplicate fails it by name.

You were right that automatic_tuning ignores hours_back/limit — with one shared fetch that is moot, and the comment now says so.

The CHANGELOG headline: fixed. Worth noting what happened there — the tab-count pin was written precisely to catch eleven-versus-twelve drift, and it did catch it in the code; it just cannot see prose, so the bolded lead-in kept the stale number while the body next to it was correct.

@erikdarlingdata
erikdarlingdata merged commit 974e9ab into dev Aug 21, 2026
6 checks passed
erikdarlingdata added a commit that referenced this pull request Aug 21, 2026
#2477 landed the server-page depth while this branch was open, so both
lanes had added an entry and its link refs at the top of the same section.
Both are kept -- they describe different work and neither supersedes the
other -- with the starter dashboards above the server page, newest first.

The two are related in one direction worth noting for anyone reading the
entries together: the chart sentences this branch adds are inert until
#2477's vizLine change makes emptyText win at zero rows. That is why the
merge order was 2477 first, and it is why both entries describe the same
get_blocking_trend defect from opposite ends.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant