From aaf50816561c4e55eb8403f91932ff2bc31c84c2 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 15 Jul 2026 20:22:47 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat(#240):=20Dashboard=20KPI=20bands=20?= =?UTF-8?q?=E2=80=94=20full-width=20flat=20card=20streams=20for=20explicit?= =?UTF-8?q?=20KPI=20favorites?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Explicit favorited panel.cfg.type==='kpi' queries now render as a flat, full-width KPI band (one shared, wrapping card stream per maximal run of consecutive explicit KPI favorites) instead of nested inside a generic gray tile, spanning every Dashboard layout mode. Loading/unfilled/error states render as compact in-band state cards naming their query; one source's failure never hides its band siblings; warnings render below the band. An auto-detected (unconfigured) one-row KPI result is unaffected — it remains an ordinary tile. - core/dashboard.js: pure partitionKpiBands() groups favorites into tile/kpi-band layout items, built before any query executes. - ui/kpi-panel.js: extracted renderKpiCards() (individual card nodes) from renderKpiPanel(), shared by the workbench preview and the new band. - ui/dashboard-kpi-band.js (new): band/source-slot construction, state cards, and warning aggregation. - ui/dashboard.js: dispatches per slot.kind through one shared runFavoriteSource() worker (extracted per CLAUDE.md's "extract a shared primitive on the second consumer" rule) rather than duplicating the tile/KPI-source streaming logic. - core/panel-execution.js: relocated explicitPanel() so eligibility is shared, never re-derived, between partitioning and execution. - styles.css: .dash-kpi-band/-stream/-source/-state-card/-warnings per the issue's sizing (160-320px, content-driven) and mobile full-row rules. No schema change, no new runtime dependency. Reconciles README's KPI section and CHANGELOG [Unreleased]. Files Altinity/altinity-sql-browser#246 (inbox) for a deferred manual real-Safari check of the new display:contents band member wrapper. Closes #240 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y --- CHANGELOG.md | 18 +++ README.md | 12 +- src/core/dashboard.js | 25 ++++ src/core/panel-execution.js | 10 ++ src/styles.css | 37 ++++++ src/ui/dashboard-kpi-band.js | 151 +++++++++++++++++++++++ src/ui/dashboard.js | 159 +++++++++++++++++++------ src/ui/kpi-panel.js | 30 ++++- tests/e2e/kpi.html | 15 +-- tests/e2e/kpi.spec.js | 31 +++-- tests/unit/dashboard-kpi-band.test.js | 165 ++++++++++++++++++++++++++ tests/unit/dashboard.test.js | 93 ++++++++++++++- tests/unit/kpi-panel.test.js | 35 +++++- tests/unit/panel-execution.test.js | 16 ++- 14 files changed, 726 insertions(+), 71 deletions(-) create mode 100644 src/ui/dashboard-kpi-band.js create mode 100644 tests/unit/dashboard-kpi-band.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index ebf2e35e..484746ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -120,6 +120,24 @@ auto-generated per-PR notes; this file is the curated, human-readable history. and the exception-code Filter and "Errors over time" panel are removed. ### Changed +- **Explicit, favorited KPI queries now render as full-width Dashboard KPI + bands instead of nested inside a generic gray tile** (#240). A KPI band + spans every Dashboard column regardless of the selected Full width/Report/ + 2-column/3-column layout; consecutive explicit `panel.cfg.type==='kpi'` + favorites merge into one flat, wrapping card stream (favorite order, then + result-column order), with no per-favorite name, description, or + rows/time/bytes footer. Loading, missing-parameter, and error states render + as compact in-stream state cards (naming the source query); one source's + failure never hides its band siblings; warnings render below the band, + each naming its query. Cards use controlled, content-driven widths + (160–320px desktop, full row under 520px). An auto-detected (unconfigured) + one-row KPI result is unaffected — it remains an ordinary tile, following + the selected layout, exactly as before. `src/ui/kpi-panel.js` gained a + lower-level `renderKpiCards()` primitive (the individual card nodes, + decoupled from the workbench's `.kpi-panel/.kpi-grid` wrapper) that both the + workbench preview and the new `src/ui/dashboard-kpi-band.js` module share; + `core/dashboard.js` gained the pure `partitionKpiBands()` grouping. No + schema change, no new runtime dependency. - **`src/core/clickhouse-type.js` is now the sole ClickHouse type-expression parser** (#238), replacing `param-type.js`'s independent regex parser; the latter is now a thin compatibility projection deriving everything from the diff --git a/README.md b/README.md index 9538f57a..dc99b282 100644 --- a/README.md +++ b/README.md @@ -83,9 +83,17 @@ The implemented **KPI** panel turns an exactly-one-row result into responsive cards: numeric scalar columns become simple KPIs, while named ClickHouse `Tuple(value numeric, delta Nullable(numeric))` columns add an optional delta. SQL owns the values; `panel.fieldConfig` owns labels, descriptions, units, -rounding, colors, NULL text, visibility, and delta semantics. The complete +rounding, colors, NULL text, visibility, and delta semantics. The card +rendering itself — labels, values, deltas, colors — is identical on both +surfaces; the surrounding composition differs by design (#240): the workbench +Panel preview and an unconfigured Dashboard KPI tile show the cards inside the +ordinary `.kpi-panel` grid, while a **favorited, explicitly-KPI-typed** Dashboard +query instead joins a full-width **KPI band** — a flat, wrapping card stream +with no per-favorite name, description, or statistics footer, spanning every +Dashboard layout (Full width/Report/2/3 columns). Consecutive explicit KPI +favorites merge into one shared band. The complete [`kpi-panel.json`](examples/kpi-panel.json) Library example can be opened from -**File ▾ → Open** and renders identically in the workbench and Dashboard. +**File ▾ → Open** to see both. When constructing a named tuple from expressions, either enable alias-derived member names for the query: diff --git a/src/core/dashboard.js b/src/core/dashboard.js index cc3313ff..d7203088 100644 --- a/src/core/dashboard.js +++ b/src/core/dashboard.js @@ -116,3 +116,28 @@ export const DASH_TABLE_DISPLAY_CAP = 1000; // `fieldControls(analysis)` in param-pipeline.js replaces the old // `dashboardParams(favorites)` union — the analysis view also sees params // confined to optional blocks, which readStatementParams never could.) + +/** + * Partition the ordered Panel-role favorites into Dashboard layout items + * (#240): a maximal consecutive run of explicit KPI favorites becomes one + * `{kind:'kpi-band', indices}` (a full-width shared card stream); every other + * favorite is its own `{kind:'tile', index}`. `isKpiFlags[i]` is true only for + * an EXPLICIT `panel.cfg.type === 'kpi'` favorite — an auto-detected one-row + * result must never join a band, so the caller derives this from the saved + * cfg, never from a query's executed result. Structural only (no query + * results involved), so bands are fixed before any tile issues a request. + */ +export function partitionKpiBands(isKpiFlags) { + const items = []; + let run = null; + isKpiFlags.forEach((isKpi, index) => { + if (isKpi) { + if (!run) { run = { kind: 'kpi-band', indices: [] }; items.push(run); } + run.indices.push(index); + } else { + run = null; + items.push({ kind: 'tile', index }); + } + }); + return items; +} diff --git a/src/core/panel-execution.js b/src/core/panel-execution.js index d2f0b038..104f270e 100644 --- a/src/core/panel-execution.js +++ b/src/core/panel-execution.js @@ -1,9 +1,19 @@ import { detectSqlFormat } from './format.js'; +import { queryPanel } from './saved-query.js'; export function isKpiPanel(panel) { return panel?.cfg?.type === 'kpi'; } +/** A saved query's explicit, known-typed panel payload, or null. Unknown + * panel-cfg shapes stay non-null-ish only through resolvePanel's diagnostic + * fallback. Shared by the Dashboard's ordinary-tile path and its KPI-band + * partitioning/execution (#240) so eligibility can never drift between them. */ +export function explicitPanel(query) { + const panel = queryPanel(query); + return panel && panel.cfg && typeof panel.cfg === 'object' ? panel : null; +} + /** Resolve the transport owned by an explicit panel without changing SQL. */ export function panelExecution(panel, sql, defaults = {}) { if (!isKpiPanel(panel)) return { ...defaults, owned: false, error: null, params: { ...(defaults.params || {}) } }; diff --git a/src/styles.css b/src/styles.css index c0ab8a17..fd0b16f4 100644 --- a/src/styles.css +++ b/src/styles.css @@ -2280,6 +2280,43 @@ table.res-table tbody tr:hover td.idx { background: var(--bg-hover); } border-top: 1px solid var(--border-faint); font-family: var(--mono); font-size: 10.5px; color: var(--fg-faint); } +/* KPI bands (#240): a full-width composition primitive, not a tile — spans + every Dashboard grid column regardless of the selected Full width/Report/ + 2 columns/3 columns layout, so several consecutive explicit KPI favorites + merge into one dense card stream instead of nested per-tile grids. */ +.dash-kpi-band { grid-column: 1 / -1; display: flex; flex-direction: column; gap: 8px; } +.dash-kpi-stream { + display: flex; flex-wrap: wrap; align-items: stretch; gap: 10px; +} +.dash-kpi-stream .kpi-card { + flex: 0 1 auto; inline-size: fit-content; min-inline-size: 160px; max-inline-size: 320px; +} +/* A source's stable host contributes its children (cards, or a state card) + directly to the stream's flex-wrap — it is never itself a visible box. */ +.dash-kpi-source { display: contents; } +.dash-kpi-state-card { + --kpi-accent: var(--accent); + min-width: 160px; max-width: 320px; padding: 14px 16px; + border: 1px solid var(--border); border-top: 3px solid var(--kpi-accent); + border-radius: 8px; background: var(--bg-modal); +} +.dash-kpi-state-label { + color: var(--fg-mute); font-size: 12px; font-weight: 600; letter-spacing: .02em; +} +.dash-kpi-state-message { + margin-top: 6px; display: flex; flex-direction: column; gap: 4px; + color: var(--fg); font-size: 13px; line-height: 1.4; +} +.dash-kpi-state-loading { display: flex; align-items: center; gap: 8px; } +.dash-kpi-state-card[role='alert'] .dash-kpi-state-message { color: var(--error-fg); } +.dash-kpi-warnings { display: grid; gap: 4px; } +.dash-kpi-warning { color: var(--fg-mute); font-size: 12px; } +@media (max-width: 520px) { + .dash-kpi-stream .kpi-card, .dash-kpi-state-card { + flex-basis: 100%; min-inline-size: 0; max-inline-size: none; inline-size: 100%; + min-width: 0; max-width: none; width: 100%; + } +} @media (max-width: 640px) { .dash-grid { grid-template-columns: 1fr; padding: 12px; } .dash-header { padding: 10px 12px; gap: 8px; } diff --git a/src/ui/dashboard-kpi-band.js b/src/ui/dashboard-kpi-band.js new file mode 100644 index 00000000..3055ae90 --- /dev/null +++ b/src/ui/dashboard-kpi-band.js @@ -0,0 +1,151 @@ +// The Dashboard KPI band (#240): consecutive explicit `panel.cfg.type==='kpi'` +// favorites render as one full-width, flat card stream instead of each being +// its own gray tile with a nested KPI grid. Isolated from `src/ui/dashboard.js` +// so its own branches (state cards, warning aggregation) get independent test +// coverage rather than inflating that file's already-100%-covered functions. +// +// A band owns `{ el, stream, warningHost, sources }`: `stream` is the flex-wrap +// `.dash-kpi-stream` card row (spans every Dashboard grid column via CSS, +// independent of the selected tile layout); `warningHost` is the band's shared +// `.dash-kpi-warnings` area below it. Each member favorite gets one stable +// `.dash-kpi-source` slot (`display:contents` — its children participate +// directly in the stream's flex-wrap without adding a visible box) appended to +// `stream` in favorite order at band-build time and never reordered; a source's +// own request lifecycle only ever replaces ITS host's children in place +// (loading → success cards | state card), mirroring the ordinary tile slot's +// "never remove/reappend" discipline (dashboard.js's buildTileSlot) so the +// #193 stable-slot-identity/generation/abort guarantees extend unchanged to +// KPI sources. + +import { h } from './dom.js'; +import { Icon } from './icons.js'; +import { resolvePanel } from '../core/panel-cfg.js'; +import { renderKpiCards, KPI_STREAM_ARIA } from './kpi-panel.js'; + +/** One compact white state card (loading/unfilled/error) — the query name + * plus a message, replacing the KPI success cards in a source's stable host + * while it has none to show. `role` drives assistive-tech behavior: `status` + * (+ `aria-live=polite` for loading) or `alert` for errors. */ +function kpiStateCard(name, role, live, ...messageChildren) { + const attrs = { class: 'dash-kpi-state-card', role, 'aria-label': name }; + if (live) attrs['aria-live'] = 'polite'; + return h('div', attrs, + h('div', { class: 'dash-kpi-state-label' }, name), + h('div', { class: 'dash-kpi-state-message' }, ...messageChildren)); +} + +/** Build one KPI band container: a full-width `.dash-kpi-band` holding the + * card `stream` and a `warningHost` for its shared warning area (rendered + * only while non-empty). `sources` accumulates this band's member slots in + * favorite order, read back by `refreshBandWarnings`. */ +export function buildKpiBand() { + const stream = h('div', { class: 'dash-kpi-stream', ...KPI_STREAM_ARIA }); + const warningHost = h('div', { class: 'dash-kpi-warnings', style: { display: 'none' } }); + const el = h('div', { class: 'dash-kpi-band' }, stream, warningHost); + return { el, stream, warningHost, sources: [] }; +} + +/** Build one favorite's stable KPI source slot, append its host into the + * band's stream (favorite order), and register it on the band for warning + * aggregation. `explicit` (the favorite's saved `cfg.type==='kpi'` panel) is + * cached on the slot once, here, at the same structural build time + * `partitionKpiBands` already establishes eligibility — so a later wave's + * dispatch (dashboard.js's runPlan) reads `slot.explicit` instead of + * re-deriving it every Refresh/filter-affected run. `abortController` mirrors + * buildTileSlot's field exactly, so runFavoriteSource's existing generation- + * guard/abort dispatch works unchanged over a KPI source. */ +export function buildKpiSourceSlot(band, explicit, name) { + const host = h('div', { class: 'dash-kpi-source' }); + const slot = { + kind: 'kpi-source', host, band, name, explicit, warnings: [], + gen: 0, status: null, abortController: null, loadLabel: null, + }; + band.sources.push(slot); + band.stream.appendChild(host); + return slot; +} + +/** Rebuild a band's shared warning area from every member source's current + * `warnings`, in source order (favorite order, set at band-build time) then + * diagnostic order — a stale rerun replaces it wholesale, never appends. + * Every entry is always `severity:'warning'` (renderKpiCards's `warnings` + * output is pre-filtered to that severity) so the role/class are fixed, + * not diagnostic-driven — a blocking (`error`) diagnostic is a state card, + * never a band warning. */ +export function refreshBandWarnings(band) { + const all = band.sources.flatMap((slot) => slot.warnings); + band.warningHost.style.display = all.length ? '' : 'none'; + band.warningHost.replaceChildren(...all.map((w) => h('div', { + class: 'dash-kpi-warning', role: 'status', + }, `${w.sourceName}: ${w.message}`))); +} + +/** One compact loading state card, in the source's stable position. Returns + * the live message text node so streamed row progress (onChunk, #193) can + * update just its text, exactly like the ordinary tile's loading label. + * Does NOT itself call `refreshBandWarnings` — a Refresh wave marks every + * affected source loading in one synchronous pass (dashboard.js's runPlan), + * and rebuilding the shared band DOM once per source in that pass would be + * N redundant O(N) rebuilds before the first ever paints; the caller + * refreshes each distinct touched band exactly once after the pass instead. */ +export function setKpiSourceLoading(slot) { + slot.status = 'loading'; + slot.warnings = []; + const label = h('span', null, 'Loading…'); + slot.loadLabel = label; + const row = h('div', { class: 'dash-kpi-state-loading' }, Icon.spinner(), label); + slot.host.replaceChildren(kpiStateCard(slot.name, 'status', true, row)); + return label; +} + +/** A KPI source blocked on an empty/invalid `{name:Type}` value (#170) — one + * filter value away from rendering, so it stays in its stable position with + * a neutral prompt rather than an error. */ +export function setKpiSourceUnfilled(slot, names) { + slot.status = 'unfilled'; + slot.warnings = []; + slot.host.replaceChildren(kpiStateCard(slot.name, 'status', false, 'Enter a value for: ' + names.join(', '))); + refreshBandWarnings(slot.band); +} + +/** Apply a completed (or errored) result to one KPI source: a transport/SQL + * error or a blocking KPI diagnostic (zero rows, wrong row count, no + * eligible fields) renders as one state card; otherwise the normalized KPI + * cards replace the source's host contents and its warnings feed the band's + * shared area. `explicit` is always a `cfg.type==='kpi'` panel here — band + * membership is gated on exactly that at partition time (core/dashboard.js's + * `partitionKpiBands`), so `resolvePanel`'s kpi branch is unconditional and + * its non-kpi fallback path can't be reached from a KPI source. Streamed row + * progress during the fetch updates `label.textContent` directly (see + * dashboard.js), never re-entering this function mid-stream. */ +export function applyKpiSourceResult(app, explicit, slot, r) { + const name = slot.name; + if (r.error != null) { + slot.status = 'error'; + slot.warnings = []; + slot.host.replaceChildren(kpiStateCard(name, 'alert', false, r.error)); + refreshBandWarnings(slot.band); + return; + } + const resolved = resolvePanel(explicit, { + columns: r.columns, rows: r.rows, fieldConfig: explicit.fieldConfig, serverVersion: app.state.serverVersion, + }); + const { cards, warnings, errors } = renderKpiCards(resolved.kpi); + if (errors.length) { + slot.status = 'error'; + slot.warnings = []; + // Every blocking diagnostic stacks as its own line in the ONE state card + // (never dropped) — the workbench's renderKpiPanel renders the same + // `errors` list in full (kpi-panel.js), so the two surfaces show + // identical diagnostic detail for identical data. + const role = errors.some((d) => d.severity === 'error') ? 'alert' : 'status'; + const lines = errors.map((d) => h('div', null, d.message)); + slot.host.replaceChildren(kpiStateCard(name, role, false, ...lines)); + refreshBandWarnings(slot.band); + return; + } + slot.status = 'panel'; + slot.warnings = warnings.map((w) => ({ ...w, sourceName: name })); + slot.host.replaceChildren(...cards); + refreshBandWarnings(slot.band); +} diff --git a/src/ui/dashboard.js b/src/ui/dashboard.js index 2f1b1efa..34b9750b 100644 --- a/src/ui/dashboard.js +++ b/src/ui/dashboard.js @@ -23,7 +23,7 @@ import { schemaKey } from '../core/chart-data.js'; import { resolvePanel, autoPanel } from '../core/panel-cfg.js'; import { DASH_TILE_ROW_CAP, DASH_TILE_BYTE_CAP, DASH_TABLE_DISPLAY_CAP, - activeDashboardView, dashboardViewSelection, + activeDashboardView, dashboardViewSelection, partitionKpiBands, } from '../core/dashboard.js'; import { formatBytes, formatRows, detectSqlFormat } from '../core/format.js'; import { newResult } from '../core/stream.js'; @@ -34,12 +34,16 @@ import { hasOptionalBlocks } from '../core/optional-blocks.js'; import { effectiveFilterActive, KEYS } from '../state.js'; import { buildFilterBar } from './filter-bar.js'; import { queryDescription, queryFavorite, queryName, queryPanel } from '../core/saved-query.js'; -import { isKpiPanel, panelExecution } from '../core/panel-execution.js'; +import { explicitPanel, isKpiPanel, panelExecution } from '../core/panel-execution.js'; import { effectiveDashboardRole } from '../core/result-choice.js'; import { filterExecution } from '../core/filter-execution.js'; import { readFilterOptions } from '../core/filter-options.js'; import { mergeDashboardFilterHelpers } from '../core/dashboard-filters.js'; import { diagnostic } from '../core/diagnostics.js'; +import { + buildKpiBand, buildKpiSourceSlot, setKpiSourceLoading, setKpiSourceUnfilled, applyKpiSourceResult, + refreshBandWarnings, +} from './dashboard-kpi-band.js'; // At most this many tile queries run at once, so a large favorites list doesn't // fire a thundering herd of concurrent reads at ClickHouse (saturating the @@ -150,6 +154,9 @@ async function runPool(items, limit, worker) { // contract) before it's replaced; `panelState` is the slot-persistent table-tile // state (#166 — sort + column widths, keyed by result schema); `loadLabel` is // the loading placeholder's live row-count text node (streamed progress, #193). +// An EXPLICIT KPI favorite never reaches this builder — it's routed to a KPI +// band slot instead (#240, see partitionKpiBands) — so `is-kpi` here only ever +// toggles on later for an AUTO-DETECTED one-row result (applyTileResult). function buildTileSlot(q) { const body = h('div', { class: 'dash-tile-body' }); const foot = h('div', { class: 'dash-tile-foot' }); @@ -160,9 +167,9 @@ function buildTileSlot(q) { const head = h('div', { class: 'dash-tile-head' }, h('span', { class: 'dash-tile-name', title: name }, name)); if (description) head.appendChild(h('div', { class: 'dash-tile-desc', title: description }, description)); - const card = h('div', { class: `dash-tile${isKpiPanel(queryPanel(q)) ? ' is-kpi' : ''}` }, head, body, foot); + const card = h('div', { class: 'dash-tile' }, head, body, foot); return { - card, body, foot, gen: 0, status: null, destroy: null, panelState: null, + kind: 'tile', card, body, foot, gen: 0, status: null, destroy: null, panelState: null, abortController: null, loadLabel: null, }; } @@ -185,13 +192,6 @@ function destroySlotChart(slot) { if (slot.destroy) { slot.destroy(); slot.destroy = null; } } -/** The favorite's explicit, known-typed panel payload, or null. Unknown types - * stay non-null-ish only through resolvePanel's diagnostic fallback below. */ -function explicitPanel(q) { - const panel = queryPanel(q); - return panel && panel.cfg && typeof panel.cfg === 'object' ? panel : null; -} - /** True for a text panel — the no-query partition (#166). */ function isTextFav(q) { const p = explicitPanel(q); @@ -262,7 +262,10 @@ function applyTileResult(app, q, slot, r) { } slot.status = 'panel'; slot.card.style.display = ''; - if (explicit && r.rows.length === 0 && !isKpiPanel(explicit)) { + // `explicit` here is never an explicit KPI panel — those are routed to a KPI + // band slot (#240) and never reach applyTileResult — so an explicit zero-row + // result is always a non-KPI panel's honest "0 rows" state. + if (explicit && r.rows.length === 0) { slot.body.replaceChildren(h('div', { class: 'dash-tile-empty' }, '0 rows')); slot.foot.replaceChildren(...tileFooter(r.meta)); return; @@ -296,62 +299,70 @@ function applyTileResult(app, q, slot, r) { slot.foot.replaceChildren(...tileFooter(r.meta)); } -// Run (or re-run) one favorite's tile into its slot, gated by its prepared +// Run (or re-run) one favorite's source into its slot, gated by its prepared // source from the wave's batch (#173): unfilled OR invalid (#170) `{name:Type}` // values show the placeholder (never issuing a request — an invalid value left // to reach the server would either error confusingly or, for Int/UInt, silently // wrap; see param-validate.js), a per-source error (e.g. a value that can't // serialize for this tile's declaration) shows an error card — blocking only -// this tile, never its siblings — otherwise stream the SQL read-only through the -// shared `app.runReadInto` seam (#193) and classify ONCE on completion. +// this source, never its siblings — otherwise stream the SQL read-only through +// the shared `app.runReadInto` seam (#193) and classify ONCE on completion. // `onSettled()` fires after every transition (unfilled, errored or fetched) so // the caller can recompute the live "N not shown" count. // +// Shared by an ordinary tile (`runSlotTile`) and an explicit KPI band source +// (`runKpiSourceTile`, #240) — the two differ only in which state-transition +// functions render each outcome, the client row cap, and whether an authored +// `FORMAT` needs the extra `detectSqlFormat` cross-check (a KPI's authored- +// FORMAT rejection is entirely `panelExecution`'s own); `hooks` supplies that +// difference so the streaming/gating/generation/abort discipline itself is +// written once (CLAUDE.md: extract a shared primitive on the second consumer +// of a pattern rather than copy it). +// // `generation` was reserved (and any prior in-flight request aborted) by // `supersedeSlot` at WAVE CREATION (#193 design req 3), not here: a queued // Refresh worker whose slot a newer wave has already re-reserved discards itself // up front without issuing, and a supersede mid-stream aborts this request and // makes the post-await guard drop it — so a stale wave can never overwrite a // newer one, even under the 6-way pool's queueing. -async function runSlotTile(app, q, slot, onSettled, src, generation) { - if (slot.gen !== generation) return; // a newer wave already superseded this queued tile +async function runFavoriteSource(app, q, slot, onSettled, src, generation, hooks) { + if (slot.gen !== generation) return; // a newer wave already superseded this queued source if (src.missing.length || src.invalid.length) { - setSlotUnfilled(slot, src.missing.concat(src.invalid)); + hooks.setUnfilled(slot, src.missing.concat(src.invalid)); onSettled(); return; } if (src.errors.length) { - applyTileResult(app, q, slot, { error: src.errors[0] }); + hooks.applyResult(slot, { error: src.errors[0] }); onSettled(); return; } // The wire text is the wave's materialized execution view (#165) — only when // the favorite actually is a template; block-free SQL keeps its exact bytes. const execSql = hasOptionalBlocks(q.sql) ? mergedSourceSql(src, q.sql) : q.sql; + const execution = panelExecution(hooks.explicit, execSql, { + format: 'Table', rowLimit: DASH_TILE_ROW_CAP + 1, + params: { readonly: 2, max_result_bytes: DASH_TILE_BYTE_CAP, ...mergedSourceArgs(src) }, + }); // #193 design req 5: the shared seam streams the structured // JSONStringsEachRowWithProgress format, so an explicit `FORMAT` clause would // silently corrupt the tile (an empty successful-looking result, or ignored // lines). Reject it with a clear error rather than mis-parse. - const explicit = explicitPanel(q); - const execution = panelExecution(explicit, execSql, { - format: 'Table', rowLimit: DASH_TILE_ROW_CAP + 1, - params: { readonly: 2, max_result_bytes: DASH_TILE_BYTE_CAP, ...mergedSourceArgs(src) }, - }); - if (execution.error || (!isKpiPanel(explicit) && detectSqlFormat(execSql))) { - applyTileResult(app, q, slot, { + if (execution.error || (hooks.checkFormat && detectSqlFormat(execSql))) { + hooks.applyResult(slot, { error: execution.error || 'Dashboard panels require structured streaming results. Remove the explicit FORMAT clause.', }); onSettled(); return; } - const label = setSlotLoading(slot); + const label = hooks.setLoading(slot); const ac = new AbortController(); slot.abortController = ac; const startedAt = app.now(); // Client row limit = CAP (newResult trims + flags `capped`); server cap = // CAP + 1 (the sentinel one past the client limit), so an exactly-CAP result // is NOT marked truncated and a >CAP result is trimmed AND flagged (#193 req 1). - const result = newResult(execution.format, isKpiPanel(explicit) ? 2 : DASH_TILE_ROW_CAP); + const result = newResult(execution.format, hooks.rowCap); await app.runReadInto(result, { sql: execSql, format: execution.format, @@ -373,15 +384,44 @@ async function runSlotTile(app, q, slot, onSettled, src, generation) { if (slot.gen !== generation) return; slot.abortController = null; const r = dashboardTileResult(result, startedAt, app.now()); - applyTileResult(app, q, slot, r); - // #171: this tile completed (current generation) — record its bound params on - // success only (the exact wave's boundParams snapshot, so a param confined to - // an inactive optional block — never in `src.statements[*].boundParams` — is - // never recorded). An errored tile records nothing. + hooks.applyResult(slot, r); + // #171: this source completed (current generation) — record its bound params + // on success only (the exact wave's boundParams snapshot, so a param confined + // to an inactive optional block — never in `src.statements[*].boundParams` — + // is never recorded). An errored source records nothing. if (r.error == null) app.recordBoundParams(src.statements.flatMap((s) => s.boundParams)); onSettled(); } +// `q` here is never an explicit KPI favorite — those run through +// runKpiSourceTile instead (#240) — so `explicitPanel(q)` (if non-null) is +// never `isKpiPanel`. +function runSlotTile(app, q, slot, onSettled, src, generation) { + return runFavoriteSource(app, q, slot, onSettled, src, generation, { + explicit: explicitPanel(q), rowCap: DASH_TILE_ROW_CAP, checkFormat: true, + setUnfilled: setSlotUnfilled, + setLoading: setSlotLoading, + applyResult: (s, r) => applyTileResult(app, q, s, r), + }); +} + +// The KPI-source counterpart of runSlotTile (#240), sharing its gating/ +// generation/abort discipline exactly via runFavoriteSource. `explicit` is +// always `cfg.type === 'kpi'` here (the caller only dispatches here for a +// `kind:'kpi-source'` slot, which partitionKpiBands only ever builds from an +// explicit KPI favorite) — so panelExecution always takes its KPI branch +// (owned typed transport, two-row sentinel) and the authored-FORMAT rejection +// is entirely panelExecution's own (no detectSqlFormat cross-check needed, +// unlike the ordinary-tile path). +function runKpiSourceTile(app, q, explicit, slot, onSettled, src, generation) { + return runFavoriteSource(app, q, slot, onSettled, src, generation, { + explicit, rowCap: 2, checkFormat: false, + setUnfilled: setKpiSourceUnfilled, + setLoading: setKpiSourceLoading, + applyResult: (s, r) => applyKpiSourceResult(app, explicit, s, r), + }); +} + /** Render the dashboard into `app.root`. */ export function renderDashboard(app) { const { document: doc, state } = app; @@ -401,6 +441,13 @@ export function renderDashboard(app) { else roleDiagnostics.push({ severity: 'error', message: `${queryName(query)} has unknown Dashboard role "${role}".` }); } + // KPI bands are built structurally, from the saved config alone, before any + // query executes (#240): an EXPLICIT `panel.cfg.type==='kpi'` favorite joins + // a band; an auto-detected one-row KPI result (no saved panel) never does — + // that distinction lives entirely in `explicitPanel`/`isKpiPanel`, never in a + // fetched result, so it can't drift with what a query happens to return. + const layoutItems = partitionKpiBands(panelFavorites.map((q) => isKpiPanel(explicitPanel(q)))); + // The favorites snapshot is fixed for this render, so the parameter analysis // (#173 phase 1 — structure only) runs once; each wave (runAll / a filter's // runAffected) prepares it against the current varValues with one wall-clock @@ -642,11 +689,24 @@ export function renderDashboard(app) { // Mark every planned slot loading up front — before the 6-way pool starts — // so tiles beyond TILE_CONCURRENCY's window don't linger on stale content // while queued. Applies to BOTH full Refresh and targeted affected waves - // (#193); runSlotTile re-marks its own slot loading when its worker starts - // (capturing the progress label), so filled tiles simply repaint identically. - plan.forEach(({ slot }) => setSlotLoading(slot)); + // (#193); runSlotTile/runKpiSourceTile re-mark their own slot loading when + // their worker starts (capturing the progress label), so filled tiles/cards + // simply repaint identically. Dispatch is by `slot.kind` (#240): an explicit + // KPI favorite's slot always came from buildKpiSourceSlot, never buildTileSlot. + // setKpiSourceLoading does NOT refresh its band's shared warning area itself + // (that would be one O(band size) DOM rebuild PER member, back to back, + // synchronously, with only the last ever visible) — collect every band this + // plan touches and refresh each exactly once after marking the whole batch. + const touchedBands = new Set(); + plan.forEach(({ slot }) => { + if (slot.kind === 'kpi-source') { setKpiSourceLoading(slot); touchedBands.add(slot.band); } + else setSlotLoading(slot); + }); + touchedBands.forEach(refreshBandWarnings); return runPool(plan, TILE_CONCURRENCY, - ({ q, slot, src, generation }) => runSlotTile(app, q, slot, updateSkipNote, src, generation)); + ({ q, slot, src, generation }) => (slot.kind === 'kpi-source' + ? runKpiSourceTile(app, q, slot.explicit, slot, updateSkipNote, src, generation) + : runSlotTile(app, q, slot, updateSkipNote, src, generation))); }; // Re-run only the favorites whose SQL references `name` (a filter field's @@ -679,8 +739,29 @@ export function renderDashboard(app) { if (!(await app.ensureFreshToken())) { app.chCtx.onSignedOut(); return; } refreshBtn.disabled = true; if (!slots.length) { - slots = panelFavorites.map((q) => buildTileSlot(q)); - slots.forEach((s) => grid.appendChild(s.card)); + // Build the grid from the structural layout items (#240): an ordinary + // tile appends its own card; a KPI band builds one full-width container + // and gives each of its member favorites a stable source slot inside its + // shared stream, in favorite order. `slots` stays flat over panelFavorites + // (the index space planWave/tileId/runAffected all key off), regardless + // of which favorites share a band. + slots = new Array(panelFavorites.length); + for (const item of layoutItems) { + if (item.kind === 'tile') { + const q = panelFavorites[item.index]; + slots[item.index] = buildTileSlot(q); + grid.appendChild(slots[item.index].card); + } else { + const band = buildKpiBand(); + for (const i of item.indices) { + // `explicit` is cached on the slot once, here (structural build + // time), so runPlan's dispatch reads `slot.explicit` on every later + // wave instead of re-deriving it from `q` on every Refresh/filter run. + slots[i] = buildKpiSourceSlot(band, explicitPanel(panelFavorites[i]), queryName(panelFavorites[i])); + } + grid.appendChild(band.el); + } + } } // Partition before execution (#166): text panels render right here — // synchronously, before any tile query is issued — and they never join diff --git a/src/ui/kpi-panel.js b/src/ui/kpi-panel.js index 50aa4be8..4885b919 100644 --- a/src/ui/kpi-panel.js +++ b/src/ui/kpi-panel.js @@ -1,6 +1,10 @@ import { formatKpiValue, kpiDeltaState } from '../core/kpi.js'; import { h } from './dom.js'; +/** The `.kpi-grid`/`.dash-kpi-stream` group's shared accessible name (#240) — + * one literal, not duplicated between kpi-panel.js and the dashboard band. */ +export const KPI_STREAM_ARIA = { role: 'group', 'aria-label': 'Key performance indicators' }; + function diagnosticNode(diagnostic) { return h('div', { class: `kpi-diagnostic is-${diagnostic.severity}`, @@ -14,10 +18,22 @@ function absoluteValue(value) { return Math.abs(value); } -export function renderKpiPanel(normalized) { +function isBlockingDiagnostic(item) { + return item.severity === 'error' || item.code === 'kpi-no-data'; +} + +/** + * Shared lower-level KPI card rendering (#240): the individual `.kpi-card` + * nodes, decoupled from the workbench's `.kpi-panel/.kpi-grid` wrapper, so the + * Dashboard KPI band can flatten cards from several consecutive favorites + * into one stream. `errors` mirrors the pre-#240 blocking-diagnostic rule + * exactly (an `kpi-no-data` info-severity diagnostic is still blocking) — + * changing that rule would regress the zero-row state for both callers. + */ +export function renderKpiCards(normalized) { const data = normalized || { items: [], diagnostics: [] }; - const errors = data.diagnostics.filter((item) => item.severity === 'error' || item.code === 'kpi-no-data'); - if (errors.length) return h('div', { class: 'kpi-state' }, ...data.diagnostics.map(diagnosticNode)); + const errors = data.diagnostics.filter(isBlockingDiagnostic); + if (errors.length) return { cards: [], warnings: [], errors: data.diagnostics }; const cards = data.items.map((item) => { const presentation = item.presentation; const label = h('div', { class: 'kpi-label' }, presentation.displayName); @@ -39,7 +55,13 @@ export function renderKpiPanel(normalized) { return card; }); const warnings = data.diagnostics.filter((item) => item.severity === 'warning'); + return { cards, warnings, errors: [] }; +} + +export function renderKpiPanel(normalized) { + const { cards, warnings, errors } = renderKpiCards(normalized); + if (errors.length) return h('div', { class: 'kpi-state' }, ...errors.map(diagnosticNode)); return h('div', { class: 'kpi-panel' }, - h('div', { class: 'kpi-grid', role: 'group', 'aria-label': 'Key performance indicators' }, ...cards), + h('div', { class: 'kpi-grid', ...KPI_STREAM_ARIA }, ...cards), ...(warnings.length ? [h('div', { class: 'kpi-warnings' }, ...warnings.map(diagnosticNode))] : [])); } diff --git a/tests/e2e/kpi.html b/tests/e2e/kpi.html index 60776338..baacb2e3 100644 --- a/tests/e2e/kpi.html +++ b/tests/e2e/kpi.html @@ -11,16 +11,17 @@
+
-
-
Service KPIs
-
-
1 row
-
+
+
+