Five ready-made dashboards for the empty Custom Views page - #2482
Conversation
No custom view ships seeded, so a new user meets a blank page and a blank canvas over an 82-read catalog. The first-run hero already knew what would help and rendered it as three inert chips — "Top waits by server", "CPU trend over time", "Slowest procedures by database" — with no handler and no href. That is the #2437 defect shape on the one page nobody arrives at with context. Those chips are the real templates now: Server health at a glance, CPU investigation, Blocking and deadlocks, Memory pressure, Configuration review. They also join the existing "New from template" menu beside the notebook seeds rather than living in a second affordance. Templates, not seeded rows. Seeding needs a migration rung, a 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 the user deletes it. A template is created only when asked for, and what lands is an ordinary view the user owns. The two halves of the menu differ on purpose: a notebook template links to the composer pre-filled because its composed panels re-scope live, while a dashboard template is v1 read panels whose server param is static, so it is created against the server picked in the menu. The server name goes in the view NAME so two servers' copies of one template do not collide on the unique-name constraint. Empty and failure states are stated, not blank: a fleet with no servers says why, and a refused create surfaces the backend's own message — flattening it would hide the line naming the bad panel. Pinned by the same invariant the built-in pages carry, and each of the five definitions was fed to the service's own ValidateDefinition, the authority that would refuse the POST. Three negative controls run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A starter dashboard is the first screen a UAT tester opens, on the store
with the least data it will ever have — a fresh install has collected
nothing for its first cycles. A wall of unexplained blank rectangles
there is a worse first impression than the feature not existing.
Every table and chart panel now carries its own empty-state sentence,
pinned as an exact count rather than a spot-check: emptyText appears
nowhere else in the module, so one dropped sentence is one panel that
renders blank in front of a new user.
The chart case bites without looking like it. get_blocking_trend and
get_deadlock_trend answer an idle server with `trend: []` and NO
{status,message} envelope, so the panel has real data in hand and falls
through to the chart's "Not enough data points to chart yet" — a
warming-up message on a server that simply never blocked. Those panels
now say an empty trend means none happened.
And the two reads that cannot be honest on day one are deliberately
absent rather than merely unused: analysis findings (the pass writes
nothing for 24 hours) and Query Store (a target with it off has nothing,
ever). Both are on the built-in server page, where the reader arrived
looking for that specific thing rather than for a first impression.
Negative control run: removing one sentence fails the pin at 18 of 19.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| viz: "line", | ||
| rowsKey: "trend", | ||
| xKey: "time", | ||
| emptyText: "No blocking events in this window — an empty trend here means none happened, not that nothing was collected.", |
There was a problem hiding this comment.
emptyText is dead on every viz: "line" panel (8 across these 5 templates, this one included). panels.js's vizLine (function vizLine(data, desc), ~line 164) never reads desc.emptyText — it only forwards points/xKey/series/formatValue/clampMax/unit into renderLineChart (charts.js). renderLineChart hardcodes "Not enough data points to chart yet." whenever rows.length < 2 (charts.js:75-77), with no way for a caller to override it. Compare vizTable and vizBandlist in panels.js, which both do desc.emptyText || "..." — vizLine is the one viz that drops the field.
That makes this exact sentence unreachable:
"No blocking events in this window — an empty trend here means none happened, not that nothing was collected."
which is the specific distinction the PR description calls the one that "bites without looking like it." A fresh/idle server will render the generic "Not enough data points to chart yet." here instead — the "warming up" framing this panel was written specifically to avoid.
ViewTemplatesTests.EveryTemplateDataPanel_ExplainsItsOwnEmptyState only pins that the string exists in the JS source (an emptyText: count + a literal Assert.Contains); it never exercises the render path, so this doesn't fail CI. Worth wiring desc.emptyText through vizLine/renderLineChart (or dropping the field from line panels until it's wired up, so it doesn't read as functional).
Review summaryScope: this PR is Darling-web-only (new Verified correct:
Found one real bug (posted inline on Everything else (test regex robustness, CSS button-vs-anchor reuse in the template menu, copyright headers, CHANGELOG) looked solid. |
|
Correct as read, and it names a merge-ordering hazard I created by splitting these into two PRs — so let me state the safe order explicitly rather than leave it in the PR body.
Safe merge order: #2477 first, then this. The dependency is one-directional and the failure mode is benign — if this merges first, the eight line-panel sentences sit dead and the charts render today's On dropping the field until it is wired: I would rather not. It would have to be re-added verbatim by #2477, and a template authored without the sentence is how the sentence never comes back — the panels that need it most ( On the test only pinning the source string — that is a fair hit and worth being precise about. |
#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>
| a template that ever named a removed read is refused with its own message rather than saved broken — which is | ||
| why the failure is surfaced verbatim here instead of being flattened to "could not create". A 409 means the name | ||
| is taken, which on a second click of the same template is the likely case, so it gets its own sentence. */ | ||
| async function createFromTemplate(template, server, status) { |
There was a problem hiding this comment.
Minor: unlike the composer's own save flow (editor.js's saveBtn.disabled = true around the request), the template button/chip that calls this isn't disabled while the request is in flight. A fast double-click fires two concurrent POST /api/views calls with the same generated name. The backend's unique-name constraint means this can't create a real duplicate (the loser gets a 409), so it's not a data-integrity issue — but it's a small inconsistency with the double-submit guard this codebase already uses elsewhere for the same kind of action, and the loser's error can flash into a status div right as the winner navigates away. Worth a disabled toggle on the clicked control for consistency, though not blocking.
Review summaryWent through the full diff (CHANGELOG, Correctness verification — cross-checked every one of the 21 read panels across the five templates against Security — all user/server-derived text (server names, template labels/descriptions) goes through One minor nit posted inline: the template create buttons don't disable themselves while the Otherwise this is a tightly self-verified change — the new |
Closes #2480.
No custom view ships seeded, so the first thing a new user sees at
#/viewsis an empty page. The hero (#1563 M4) sells the feature well and then hands over two buttons onto a blank canvas over an 82-read catalog. It also ended with three chips reading "Top waits by server", "CPU trend over time" and "Slowest procedures by database" —<span>s, no handler, no href. That is #2437's defect shape (a promise rendered as a caption) on the one page nobody arrives at with context.What lands
Five starter dashboards in a new
wwwroot/js/view-templates.js:The hero's chips are now those templates, rendered from the registry rather than from a second hardcoded list, and they also join the existing "New from template" menu beside the notebook seeds — one affordance, two groups, not a new one bolted on.
Templates, not seeded rows
Seeding curated views as data would need a migration rung, a
StorageVersionbump, four pinned test files and a viewer probe sentinel — for content that is not schema. Worse, a seeded row resurrects itself on the next upgrade after the user deletes it, and there is no reset path if an editing seat breaks one. A template is created only when someone asks for it, and what lands is an ordinary view the user owns, edits, and can delete for good.The trade-off is worth stating: a stored definition is validated once at write time and never again, so a definition naming a later-renamed read becomes a permanently broken panel that nothing greps. These templates being code is exactly what makes that fixable — see the pins below.
One wrinkle that shaped the design
A v1 read panel's
serverparam is static:renderViewthreads variables and range into composed panels only. So a dashboard template cannot re-scope itself the way a notebook template can, and the menu carries a server picker rather than a bare link. The chosen server also goes into the view's name, so two servers' copies of one template do not collide on the unique-name constraint and turn the second click into a 409.Degrading out loud
<select>.Verification
No JS test runner here, so this follows the #2447 / #2451 idiom.
ViewTemplatesTestscarries the same invariant the built-in pages do — every read named exists inBuildReadDispatch(), every parameter key is one that read actually binds perCatalogDescriptors, every viz is inKnownVizList— plus the structural rulesValidateDefinitionenforces (no rawpath, span ∈ {1,2}, no non-hex series colour), unique keys, and that the hero renders from the registry rather than a second list. Both sides come from the shipped artifacts.Behaviour was verified two ways. Every template was rendered through the shipped
renderPanelunder a minimal DOM shim across two response shapes (empty envelope, error) — zero throws, 21 distinct requests, every one checked againstCatalogDescriptors. And every definition was dumped from the shipped module and fed to the service's ownDarlingWebEndpoints.ValidateDefinition— the authority that would refuse the POST. All five come back storable.Three negative controls, all run and each restored afterwards: a bogus read name fails the dispatch pin;
limitswapped fortoponget_top_queries_by_cpufails the parameter pin, naming both the key and the allowed set; an unknown viz fails the storability pin.Darling.Testsbuilds (EnableWindowsTargeting) but cannot execute on macOS, so the six facts were compiled into a throwawaynet10.0console project against the real service build with a minimal xUnit shim — the assertions that ran are the shipped ones, not a retyped copy. All six pass.🤖 Generated with Claude Code
Since opening: honest on day one
A starter dashboard is the first screen a UAT tester opens, on the store with the least data it will ever have — a fresh install has collected nothing for its first cycles. A wall of unexplained blank rectangles there is a worse first impression than the feature not existing.
All 19 data panels (12 tables, 7 charts) now carry their own empty-state sentence, pinned as an exact count rather than a spot-check:
emptyTextappears nowhere else in the module, so one dropped sentence is one panel that renders blank in front of a new user. Negative control run — removing one fails the pin at 18 of 19.The chart case is the one that bites without looking like it.
get_blocking_trendandget_deadlock_trendanswer an idle server withtrend: []and no{status,message}envelope, so the panel has real data in hand and falls through to the chart's "Not enough data points to chart yet" — a warming-up message on a server that simply never blocked. Those panels now say an empty trend means none happened. (The renderer change that makesemptyTextwin at exactly zero rows is in #2477; these sentences are inert without it and correct with it, so there is no ordering dependency between the two PRs.)Two reads are deliberately absent rather than merely unused, for the same reason: analysis findings (the pass writes nothing for 24 hours) and Query Store (a target with it switched off has nothing, ever). Both are on the built-in server page, where the reader arrived looking for that specific thing rather than for a first impression.