Repository navigation
feat(analytics): add useMetricView client experience - #488
Merged
Merged
Conversation
Contributor
|
Contributor
📦 Bundle size reportCompared against
|
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 863 KB (-1.0 KB) | 301 KB (-552 B) |
| Type declarations | 314 KB | 109 KB |
| Source maps | 1.7 MB (-8.4 KB) | 562 KB (-2.6 KB) |
| Other | 11 KB | 3.7 KB |
| Total | 2.8 MB (-9.4 KB) | 975 KB (-3.2 KB) |
Per-entry composition (own code — deps external (as shipped))
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
. |
88 KB | 2.5 KB | 91 KB | external | 288 KB |
./beta |
48 KB | 459 B | 48 KB | external | 141 KB |
./type-generator |
21 KB | 0 B | 21 KB | external | 60 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
. |
index.js |
initial | 84 KB |
. |
utils.js |
initial | 4.0 KB |
. |
remote-tunnel-manager.js |
lazy | 2.5 KB |
./beta |
beta.js |
initial | 32 KB |
./beta |
stream-manager.js |
initial | 5.8 KB |
./beta |
wide-event-emitter.js |
initial | 3.2 KB |
./beta |
databricks.js |
initial | 3.0 KB |
./beta |
configuration.js |
initial | 2.1 KB |
./beta |
service-context.js |
initial | 1.3 KB |
./beta |
client.js |
initial | 431 B |
./beta |
client-options.js |
initial | 220 B |
./beta |
supervisor-api.js |
lazy | 194 B |
./beta |
databricks.js |
lazy | 142 B |
./beta |
index.js |
lazy | 123 B |
./type-generator |
index.js |
initial | 21 KB |
@databricks/appkit-ui
npm tarball (packed): 342 KB (+1003 B) — gzipped download (dist + bin; excludes release-only docs/NOTICE).
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 390 KB (+752 B) | 130 KB (+192 B) |
| Type declarations | 228 KB (+281 B) | 83 KB (+61 B) |
| Source maps | 753 KB (+2.1 KB) | 248 KB (+754 B) |
| CSS | 16 KB | 3.3 KB |
| Total | 1.4 MB (+3.1 KB) | 464 KB (+1007 B) |
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
./js |
5.3 KB | 49 KB | 55 KB | 208 KB | 14 KB |
./js/beta |
20 B | 0 B | 20 B | 0 B | 0 B |
./react |
432 KB (+99 B) | 49 KB | 480 KB (+99 B) | 1.3 MB | 175 KB (+265 B) |
./react/beta |
1.0 KB | 0 B | 1.0 KB | 0 B | 1.9 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
./js |
index.js |
initial | 5.2 KB |
./js |
chunk |
initial | 120 B |
./js |
apache-arrow |
lazy | 49 KB |
./js/beta |
beta.js |
initial | 20 B |
./react |
index.js |
initial | 430 KB |
./react |
tslib |
initial | 2.1 KB |
./react |
apache-arrow |
lazy | 49 KB |
./react/beta |
beta.js |
initial | 1.0 KB |
atilafassina
changed the title
useMetricViews
atilafassina
force-pushed
the
mv-hook
branch
2 times, most recently
from
July 29, 2026 08:24
7456890 to
613d5cb
Compare
Contributor
🤖 AppKit PR bot🔬 Run evalsStart an eval for this PR from the evals-monitor app: Go to Evals Monitor → 📦 Try this PR's app templateScaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh run download 31596951960 -R databricks/appkit -n appkit-template-0.58.0-pr.3330d85-mv-hook-488 -D appkit-pr-488 \
&& unzip -o "appkit-pr-488/appkit-template-0.58.0-pr.3330d85-mv-hook-488.zip" -d "appkit-pr-488" \
&& databricks apps init --template "appkit-pr-488"The template pins |
Freeze the three shared seams the metric-view hook/server/generator phases compile against: - S1: MetricColumnMeta + MetricViewsMetadata value types in packages/shared - S2: optional per-column metadata on the SSE result message + makeResultMessage - S3: base MetricRegistry, MetricKey, Infer* helpers, MetricFilter mirrors, and UseMetricViewOptions/UseMetricViewResult in appkit-ui hook types Types only (plus a makeResultMessage passthrough); existing /query callers are unchanged since metadata is optional. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
… metadata (PR5 1-4)
Implement the four parallel PR5 slices against the frozen phase-0 contracts:
- Generator (type-generator): emit metric-views.ts (was .d.ts) carrying both
the erasable declare-module MetricRegistry augmentation and a runtime
`export const metricViewsMetadata = {...} as const`. Header is a type-only
import (no runtime side-effect import on the Node server). Rename propagated
through METRIC_TYPES_FILE, mvOutFile, vite-plugin, CLI announce, and tests;
generated .ts added to Biome ignore.
- Server (analytics plugin): accept an injected `metricViewsMetadata` config
and stamp the responding metric's per-column slice (scoped to the requested
measures/dimensions) into the SSE result payload. Metadata is response
decoration — it never enters composeMetricCacheKey and never alters SQL.
- Hook (appkit-ui): `useMetricView(key, opts)` mirroring useAnalyticsQuery
(SSE, abort-on-arg-change, autoStart), returning
{ data, loading, error, errorCode, metadata }.
- Formatters (appkit-ui js): pure, React-free, tree-shakeable formatValue /
formatLabel / toD3Format taking the format spec / column metadata as args.
Also fix a pre-existing latent port collision: analytics.integration.test.ts
and server.integration.test.ts both hardcoded port 9879; under the added
metric-test weight they could bind concurrently in the shared vitest worker
pool, so an analytics request hit the server-plugin app and 404'd. Switch the
analytics integration test to an OS-assigned ephemeral port (port: 0), matching
the files plugin integration test.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
…se 5)
Integrate the metric-view runtime in dev-playground and document it:
- Regenerate the generated artifact as shared/appkit-types/metric-views.ts
(delete the legacy .d.ts). Verified byte-for-byte identical to a live
`generate-types` DESCRIBE against a real UC Metric View (warehouse
dd43ee29fedd958d, dogfood): display_name/format/description genuinely flow
from the UC YAML through typegen into the runtime metricViewsMetadata const.
- Inject the const server-side: analytics({ metricViewsMetadata }).
- Add a /metric-views demo route calling useMetricView("revenue", …) with
timeGrain/timeDimension, rendering a chart + table whose labels and value
formats come from the payload metadata (never hand-typed), degrading
gracefully when metadata is absent.
- Docs: extend plugins/analytics.md with the useMetricView + format-utility +
metricViewsMetadata injection story (Plotly + ECharts examples), and fix the
stale metric-views.d.ts references in development/type-generation.md.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
… builder to /js
Move the twelve-operator MetricFilter grammar out of react/hooks/types.ts into a
canonical, framework-agnostic js/metric-filter/ module and add a toMetricFilter
builder that compiles a { dimension -> value(s) } shorthand into a MetricFilter
(scalar -> equals, array -> in, omit undefined/empty). react/hooks/types.ts now
re-exports the types so the /react public surface and UseMetricViewOptions.filter
are unchanged. Wire the dev-playground metric-views route's buildFilter onto
toMetricFilter, keeping only the app-specific cross-filter facet-exclusion local.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
Adds two public chart props, inherited by every chart type via the factory:
- onDataClick?(datum: ChartClickDatum): fire-and-forget click callback. base.tsx
builds a memoized internal echarts onEvents={{ click }} only when the handler is
set (no idle listener), mapping raw params via the pure mapToDatum. Pointer-only
(canvas) — documented to require a keyboard-accessible equivalent.
- selected?: string | string[]: controlled, name-based visual emphasis. base.tsx
runs the pure applySelectionEmphasis transform over the built option so matching
bar/pie-donut categories stay prominent and the rest dim; no-op when unset.
ChartClickDatum is the only new public (barrel) symbol; mapToDatum,
applySelectionEmphasis and SelectionEmphasisOptions are internal. echarts types
stay out of the public API (datum.raw is unknown). Phases 1 and 2 are committed
together so the producer helpers have their consumer (satisfies knip).
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
…egment charts
Wire the new appkit-ui chart props into the metric-views demo: the region BarChart
and segment DonutChart get onDataClick={(d) => setDimension(dim, d.name)} — reusing
the same setDimension the table row-click uses — and selected={selection[dim]} so the
clicked category is emphasized. LineChart, Table, and the existing (keyboard-accessible)
table row-click are unchanged.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
… review cleanup Address adversarial-review findings on the useMetricView / metric-route branch: - Type soundness: infer rows from the selected measure/dimension tuples (PickMetricRow) and correlate timeDimension/timeGrain to temporal dims only. - Formatting: preserve every currency symbol the generator emits end-to-end and keep bigint precision (no Number() rounding). - Cache correctness: stamp fresh per-column metadata AFTER the cached execute() so a cache hit never serves stale labels/formats after a redeploy. - Charts: guard selected="" as a no-op, split [x,y] click tuples into x/y, and memoize onEvents on handler presence (no listener thrash per SSE tick). - Typegen: sweep a stale sibling metric-views.d.ts on upgrade and reject a .d.ts mvOutFile. - Drop the unused public notify export and the fake "Write back" demo; remove the dead autoStart option; align AnalyticsStreamMessage; tighten the biome ignore; add tests + comment cleanup. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Drop comments that restated adjacent JSDoc/functions: - base.tsx: the inline-handler re-subscribe rationale lived in both the `interactive` and `onEvents` comments; keep it once at `onEvents` (where the subscription happens) and point the `interactive` note at it. - use-metric-view.ts: result-branch comment re-explained metadata narrowing already documented on `asMetricMetadata`; defer to that doc. - js/format/index.ts: call-site comment restated `currencyPrefix`'s own JSDoc. - render-types.ts: convert `generateMetricTypeDeclarations`'s // block to /** */ so the exported function's rationale surfaces on IDE hover. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
useMetricView never implemented autoStart — the row was copied from useAnalyticsQuery's options table. The hook's effect calls start() unconditionally and UseMetricViewOptions has no such field. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
- Centralize analytics SSE parsing across React hooks - Share metric filter types and runtime operator vocabulary - Reuse metadata and label formatters; refresh size baseline Signed-off-by: Atila Fassina <atila@fassina.eu>
- analytics.md: make the Plotly and ECharts chart-library references links to plotly.com/javascript and echarts.apache.org; minor wording tidy (hardcode). - charts/types.ts: trim the ChartClickDatum doc comment. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Phase 1 of typegen-ci-resilient-describe. In blocking (`--wait`) mode the type generator now suppresses the `.d.ts` write on ANY degraded result (query `result: unknown` or degraded metric), leaving the committed types untouched as the CI fallback of record, then throws as before. The prior path wrote degraded (`unknown`) declarations first and threw after, which clobbered good committed types on a fresh CI checkout — including via the auth/timeout/bad-id/DELETED fatal-degrade path. Non-blocking mode is unchanged (still writes degraded types for the detached worker to refresh). Tests inverted to assert no-write-on-degrade while preserving throw + behavioral assertions; adds coverage for the query-side fatal-degrade clobber-prevention case. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Phase 2 of typegen-ci-resilient-describe. Adds a pure classifyBlockingFailure(error) → "deterministic" | "environmental" to type-generator/errors.ts, building on the existing getErrorStatus and isConnectivityError helpers. Deterministic (build must crash regardless of committed types): HTTP 404 (bad warehouse id) and 400 (malformed request), checked first and walked through cause/AggregateError chains. Environmental (has-types gate applies later): 401/403 auth, connectivity, DELETED/DELETING, wait-timeout, and any unrecognized failure (the default). The auth status set is a one-line change point for the auth-owning team. No behavior change to isConnectivityError. Adds tests/errors.test.ts. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
…wait Phase 3 of typegen-ci-resilient-describe (join point). Wires the classifyBlockingFailure taxonomy into the reordered blocking write path so `--wait` degrades gracefully on environmental failures instead of always crashing: - Deterministic failures (SQL syntax, HTTP 404/400) still crash the build. - Environmental failures (401/403 auth, connectivity, DELETED/DELETING, wait-timeout, unrecognized) now flow through a has-types gate: if committed analytics/metric-views .d.ts exist, skip the (already-suppressed) write, emit one loud greppable stderr warning naming the coarse cause (auth blocked / warehouse unreachable / warehouse unavailable) + warehouse id, and exit 0 using the committed types as the fallback of record. If no committed types exist, crash with a generic 'run generate-types --wait locally' remedy. Serving types are excluded from the gate (gitignored, degrade independently). Non-blocking mode is unchanged. Threads deterministic-vs-environmental and a coarse cause label out of the query + metric preflights. Adds gate-matrix coverage: environmental+present (per cause) → warning+exit0, environmental+ absent → crash, deterministic (404/400/syntax) → crash regardless of types, partial presence, serving-exclusion, and CI-safe (ANSI-free) warning output. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Phase 4 of typegen-ci-resilient-describe. Updates the type-generation docs to describe the committed-types fallback and two-bucket failure taxonomy for blocking (`--wait`) builds: committed .d.ts as the fallback of record, --wait never overwriting good types with degraded ones, deterministic failures (SQL syntax / 404 / 400) crashing vs. environmental failures (auth / connectivity / deleted / timeout) gating on committed-type presence, the loud stderr warning, and the run-locally remedy for a first build with no committed types. Notes the metric-views-only edge case (empty analytics.d.ts satisfies the gate). Refreshes the metric-view section to reference the same taxonomy instead of the old always-fail framing. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
…anges
Wash of the typegen-ci-resilient-describe branch: strips loop-process
"Phase N" labels from comments, test names, and describe titles (keeping
the semantic text), removes an unnecessary comment / empty else-block /
useless default parameter, rewrites two stale+duplicated write-suppression
comments to match the actual behavior, and converts errors.test.ts's
`(error as any)` casts to the sibling `Object.assign(new Error(...), { ... })`
idiom. Comments, names, and test-setup style only — no logic or assertion
changes (537 tests still pass).
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
Brings in main's revert of PR #502 (#509), which relocated the typegen cache back to node_modules/.databricks. This branch was cut from the #502 merge commit and inherited that surface without ever depending on it, so the sync is a clean subtraction: cache-paths.ts, the committed apps/dev-playground/.appkit/ fixtures, the cache-serialization tests, and the warehouse-less-deploy docs all drop out, while the metric-view runtime (useMetricView, formatters, metric-views.ts codegen) is untouched. bundle-size-baseline.json conflicted because both sides regenerated it; resolved by taking main's, to be regenerated against the merged tree. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
The merge took main's baseline to resolve the conflict; this remeasures against the post-merge build so the numbers reflect the actual tree. `size:compare` now reports no change. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
The exact-formatting path only recognised integer-shaped strings, so a
fractional value off the JSON_ARRAY wire fell through to `Number()` and lost
precision past ±2^53 — a cents-denominated total or a fixed-scale DECIMAL
rendered as a neighbouring value.
Parse plain decimal strings into a fixed-point `{coefficient, negative, scale}`
and format from that, so the value never passes through a JS number. Rounding
is half-away-from-zero, matching `Intl.NumberFormat`'s default. The percent
path now scales the coefficient by 100 exactly instead of refusing the input.
Exponent-form strings still take the float path, having no exact reading.
Signed-off-by: Atila Fassina <atila@fassina.eu>
…tricView `MetricOrderBy` was exported so callers could name a hoisted `orderBy` constant, but its `field` was `string` while `UseMetricViewOptions` required the selected-field literal union — so the exported type could not be used for the thing it was exported for. Hoisting is the pattern the metric-views route follows, since the hook's payload memo compares by value. Make `MetricOrderBy<Field extends string = string>` generic and instantiate it in `UseMetricViewOptions` at the selected measures/dimensions, replacing the inline structural duplicate that let the two definitions drift. The default instantiation stays `field: string`, so the server's wire type (`IAnalyticsMetricRequest`) is unchanged. A bare `MetricOrderBy[]` is still rejected by the hook, deliberately: `MetricOrderBy<string>` cannot prove its fields were selected, and accepting it would forfeit the compile-time guarantee that you cannot order by an unselected column. Hook callers parameterize with their selected fields; the JSDoc now says so and the type probe pins both the accepted and rejected forms. Signed-off-by: Atila Fassina <atila@fassina.eu>
The `Rule A:` / `Rule B:` labels in the metric request `superRefine` restated their own `addIssue` messages, and the sibling rules in that same function carry no such labels. The `renderOrderByClause` lead comment narrated the step rather than the intent; the ordering-is-caller-significant point it carried is stated where it is load-bearing, in the cache key. Comments only — no behaviour change. Signed-off-by: Atila Fassina <atila@fassina.eu>
`2226fde4` established that an empty-string dimension value is a real value
filtering on `equals ''`, distinct from a NULL group key's `IS NULL`, and gave
the filter dropdown an `EMPTY` sentinel to say so. The chart-click path still
collapsed the two: `fromChartName("")` returned `null`, so clicking an
empty-string category filtered for SQL NULL, and `toSelectionSet` drops `""`, so
that category could never render as selected.
The ambiguity starts upstream of the click — `ChartClickDatum.name` is a string,
and `mapToDatum` derives it from ECharts' own `p.name`, so a NULL group key and
a genuine `""` are already indistinguishable by the time a handler sees them.
Project the dimension into the sentinel space the dropdown already uses before
handing rows to the chart, and decode it back on click. `toSelectionSet`'s
documented "empty selection = no emphasis" guard is untouched: sentinels are
non-empty, so they pass through it.
A dimension with no filter at all stays `undefined` rather than being mapped to
the NONE sentinel, which would have emphasized the NULL bar whenever nothing
was selected.
Signed-off-by: Atila Fassina <atila@fassina.eu>
`formatValue` re-derived `isPercent`, `grouping`, `decimals` and `prefix` from the format string on every call — two of those run a regex — and callers invoke it per table cell, so a render redid the same parse rows x columns times. Memoize the parse keyed by the spec string. The realistic key space is metric-view catalog metadata, so it is tiny, but the function is public and takes an arbitrary string: cap the map and stop inserting once full rather than letting a caller that generates specs in a loop grow it without bound. A miss past the cap still parses and returns the right value, so correctness never depends on cache state. `toD3Format` parsed the same spec separately and now shares the cache. Per-value work (`parseExactDecimal`, thousands grouping) is unchanged — it cannot be cached, since it depends on the value rather than the spec. Signed-off-by: Atila Fassina <atila@fassina.eu>
pkosiec
reviewed
Aug 10, 2026
pkosiec
left a comment
pkosiec
left a comment
Member
There was a problem hiding this comment.
Review: useMetricView client experience
Solid PR — the hook, SSE consolidation, and format/filter utilities are well-built. I verified the feature live in the dev-playground against the dogfood warehouse: labels, currency formatting, all filter paths (dropdowns, table-row, chart-click), multi-dimension filters, chip removal, and Clear all all work.
One thing is broken: the time-series line chart scrambles into spaghetti after any filter (details inline). Everything else is UX polish or API consistency.
Findings
- 🔴 P1 — trend line chart renders unordered → scrambled (reproduced)
- 🟠 P2 — every filter blanks all visuals to skeletons (flash)
- 🟠 P2 —
useMetricViewdoesn't returnwarehouseStatus(no cold-start UI) nit× 6 — charts ignore metadata format (playground), empty-key throw,autoStartparity,selectedscope, leftoverconsole.log, dense comments
Third-party (not this PR, FYI): filtering spams the console with TypeError: Cannot read properties of undefined (reading 'disconnect') (×10 in one session). Traced to size-sensor (resizeObserver.js destroy()), a transitive dep of echarts-for-react — not our code. The rapid chart remount-on-filter (see the skeleton-flash finding) is what surfaces it. Fixing that flash likely quiets it; otherwise a separate ticket / dep note.
Non-blocking (COMMENT). Nothing needs to gate merge except your own call on the P1.
pkosiec
reviewed
Aug 10, 2026
pkosiec
left a comment
pkosiec
left a comment
Member
There was a problem hiding this comment.
Review: useMetricView client experience
Solid PR — the hook, SSE consolidation, and format/filter utilities are well-built. I verified the feature live in the dev-playground against the dogfood warehouse: labels, currency formatting, all filter paths (dropdowns, table-row, chart-click), multi-dimension filters, chip removal, and Clear all all work.
One thing is broken: the time-series line chart scrambles into spaghetti after any filter (details inline). Everything else is UX polish or API consistency.
Findings
- 🔴 P1 — trend line chart renders unordered → scrambled (reproduced)
- 🟠 P2 — every filter blanks all visuals to skeletons (flash)
- 🟠 P2 —
useMetricViewdoesn't returnwarehouseStatus(no cold-start UI) nit× 6 — charts ignore metadata format (playground), empty-key throw,autoStartparity,selectedscope, leftoverconsole.log, dense comments
Third-party (not this PR, FYI): filtering spams the console with TypeError: Cannot read properties of undefined (reading 'disconnect') (×10 in one session). Traced to size-sensor (resizeObserver.js destroy()), a transitive dep of echarts-for-react — not our code. The rapid chart remount-on-filter (see the skeleton-flash finding) is what surfaces it. Fixing that flash likely quiets it; otherwise a separate ticket / dep note.
Non-blocking (COMMENT). Nothing needs to gate merge except your own call on the P1.
Wash pass over the useMetricView branch: - drop the orderBy-is-not-sorted rationale duplicated in analytics.ts; it stays whole at its canonical site in mv/cache.ts - collapse the verbatim-duplicated `selected` JSDoc in BaseChartProps to a link to ChartBaseProps.selected - remove a dangling review-finding reference from a test name - bring back warehouseStartupTimeoutMs's description and autoStartWarehouse's cost-control rationale, which an earlier comment-trim pass had removed from the IDE hover Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
The handler only logged the clicked datum to the console. That log was residue from b9be4f1, which removed the `notify` export and the fake "Write back" demo that `dbef8f84` had added around this handler. Keep the handler as an intentional placeholder for the write-back flow and drop the console write. The empty body is deliberate: `BaseChart` derives `interactive` from handler presence, so removing the prop would switch off line-stroke click emission and the seam with it. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
`analytics({ metricViewsMetadata })` made every app import a generated const
and hand it to the plugin. The plugin already discovers metric views from
`config/metric-views/definitions.json` with no wiring at all, so display
metadata — the same data, from the same generator pass — took a second,
redundant channel into the same plugin.
The reason was the artifact's format, not a design choice: the metadata was
emitted as a TypeScript `as const`, and a value can only reach the server via
an import. That format also forced the type artifact to be a real `.ts`
(an object-literal const in an ambient declaration is TS1254), which is why
typegen had to reject a `.d.ts` `mvOutFile` and sweep a stale sibling.
The generator now writes the runtime half to
`config/metric-views/metadata.generated.json`, beside the hand-authored
`definitions.json` (separate file: a generator writing into a user-authored
one would clobber hand edits). The route reads it through
`AppManager.readMetricViewsConfig`, inheriting the dev tunnel and the
traversal guard, and caches the parse keyed on raw contents so a regenerated
bundle is picked up without a restart.
Unlike `definitions.json`, every failure mode here degrades instead of
throwing: metadata is pure response decoration, so an absent, malformed, or
future-versioned bundle yields unlabeled columns rather than failing a query
that would otherwise have succeeded. Malformed and version-mismatched cases
warn; a registered view with no metadata entry warns too, since that was
previously silent.
`metricViewsMetadata` stays as an explicit override that wins over the
discovered bundle, so this is not a breaking change and apps building
metadata another way keep working.
With the const gone the type artifact reverts to `metric-views.d.ts`, which
drops the `mvOutFile` `.d.ts` rejection, inverts the stale-sibling sweep, and
removes the special case in the `--wait` committed-types gate. Both artifacts
are written under one anti-clobber gate, so a degraded blocking pass never
pairs committed types with emptied-out metadata.
Verified against the built bundle, not just source: unbundle mode preserves
`METRIC_TYPES_FILE = "metric-views.d.ts"`, the `METRIC_METADATA_FILE` write
in the generator, and `loadMetricMetadata` on the mv barrel.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
The warning named one remedy — "regenerate types to refresh
metadata.generated.json" — regardless of where the metadata came from. On the
`analytics({ metricViewsMetadata })` override path that advice is wrong twice
over: regenerating cannot change an injected value, and the bundle is never
read there at all.
Hold the injected value in a variable so the source survives the `??`, then
warn per branch. Two separate calls rather than a conditional format string, so
neither carries a dangling argument.
Tests cover both branches. The discovered-bundle case needs a bundle on disk
that omits the queried key — an absent bundle resolves to `undefined`, which is
dormancy and skips the block entirely, so a fixture without one asserts
nothing.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
Review feedback: several comment blocks read as design docs rather than code comments. Keep the rationale, drop the worked-through reasoning. `mv/cache.ts` 7 comment lines to 2, `use-metric-view.ts` 8 to 4. Every mechanism a maintainer needs to evaluate the code survives: order-by sequence matters under LIMIT, commas are valid in identifiers so the key must be JSON-encoded, and shape changes clear rows while filter revalidations keep them. `js/metric-filter/index.ts` was also cited but left alone — its density is public-API JSDoc with `@example` blocks, which the same review exempted. Comments only; no code lines changed. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
`metric-views.d.ts` was the only generated artifact using
`import type {} from "@databricks/appkit-ui/react"`; `analytics.d.ts` and
`serving.d.ts` both use a bare side-effect import. Use the same form.
The type-only variant was correct while the artifact was a real `metric-views.ts`
carrying the runtime metadata const, where a side-effect import would have
executed the client package entry on the Node server. It became a `.d.ts` when
the metadata moved to a JSON bundle, and a `.d.ts` is never emitted to JS, so
the runtime concern the comment cited no longer exists. Verified: both forms
typecheck identically.
The import itself stays, and the comment now says why. It marks the file a
module, which is what makes `declare module` an augmentation that merges into
the real one. Without it the block is an ambient declaration that SHADOWS
`@databricks/appkit-ui/react` and hides every genuine export — confirmed by
removing it: `Module has no exported member 'useMetricView'`.
The tests asserted the previous form, so they inverted with it. They now pin the
invariant that matters (import present + augmentation present) rather than which
import spelling was used.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
# Conflicts: # bundle-size-baseline.json
pkosiec
approved these changes
Aug 12, 2026
pkosiec
left a comment
pkosiec
left a comment
Member
There was a problem hiding this comment.
LGTM, just one comment from an agentic review - PTAL before merge. Thanks!
useMetricView returns metadata as Record<string, MetricViewColumnDisplay> and the public formatLabel() takes it as a parameter, but the type only lived in the workspace-private shared package. Consumers had no importable name for it, so wrapping formatLabel or typing a column renderer was impossible without reaching into a private dist path. Re-export it from js/format alongside the sibling Metric* types, and surface it through the react hooks barrel. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
React Strict Mode runs setup/cleanup/setup on mount, which sent a POST, aborted it, and sent an identical second one. Track the active request key and defer teardown by a microtask so the second setup can claim the still open stream. A genuine unmount has no new lease, so the abort still fires. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
A sub-percent category rendered thinner than a CSS pixel, so it could not be clicked or hovered for cross-filtering. Set minAngle: 3 — ECharts still reports labels and tooltips from the true values. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Add a Playwright spec over the metric-views route with a mocked metric endpoint, plus the ariaLabel/testId hooks the charts need to be addressable from it. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Delivers the complete Metric Views client experience on top of #512: applications can query typed Metric Views, format results from catalog metadata, and build interactive cross-filtering charts without duplicating query or presentation logic.
Delivered
useMetricViewwith inferred selected-row types, metadata, loading/error state, cancellation, HMR refresh, and shared warehouse-readiness reporting.useMetricViewanduseAnalyticsQuery.formatValue,formatLabel,toD3Format, andtoMetricFilterutilities.onDataClickandselected) for cross-filter interactions.Stacked on #512.