feat(appkit): introduce workspace-client wrapper over the Databricks SDK by MarioCadenas · Pull Request #475 · databricks/appkit

MarioCadenas · 2026-07-10T13:20:54Z

What

Introduces packages/appkit/src/workspace-client/ — a facade over the legacy @databricks/sdk-experimental client that mirrors the modular Databricks SDK's multi-client shape. This prepares the library for an incremental, per-service migration to the modular SDK. Nothing is migrated yet — every service still delegates to the legacy client.

Supersedes the exploration in #414, which actually migrated services (files/warehouses/vectorSearch), added upstream patches, and introduced dual-error handling. This PR does only the seam.

Why introduce a wrapper that changes nothing?

Fair question — on its own this PR adds indirection that delegates every call straight back to the same SDK. The value is entirely in what it unblocks.

AppKit is on @databricks/sdk-experimental (one monolithic WorkspaceClient). The supported SDK is databricks/sdk-js — a modular architecture: a @databricks/sdk-core + -auth + -options foundation plus one @databricks/sdk-<service> package per API, each constructed like new FilesClient({ host, credentials }). We want to move onto it, but a big-bang migration is risky (that's what #414 was, and it was too large to land safely).

So the plan is migrate one service at a time, each in its own PR, with the app fully working after every step. This wrapper is the seam that makes that possible:

Every service goes through one accessor (wrapper.files, wrapper.genie, …), and a Biome boundary rule guarantees nothing else in appkit imports the SDK directly. So migrating a service is a localized change — one getter in client.ts, one accessor type, one connector — with a provably bounded blast radius.
Unmigrated services keep delegating to legacy, so main stays green between every migration PR — no half-broken state.
toLegacyWorkspaceClient() is the deliberate escape hatch for code still on the old SDK (notably @databricks/lakebase), so the seam can be complete even while the migration is partial.

That's why the boundary rule and escape hatch exist — they're the mechanism for a safe, incremental migration, not speculative abstraction.

Where this goes (for context, not in this PR)

Coverage was verified 2026-07-23: every service appkit uses now has a published modular package (sdk-files, sdk-warehouses, sdk-vectorsearch, sdk-statementexecution, sdk-scim for currentUser, sdk-modelserving for serving, sdk-genie, sdk-jobs), so full migration and eventual removal of sdk-experimental is a reachable end state. Rough sequence: clean services first (files/warehouses/vectorSearch, PoC-proven), then statementExecution + the raw-apiClient.request sites → sdk-core/http, then the divergent ones last (genie/jobs need a connector rewrite for the waiter idiom + camelCase fields). The honest caveat: the wrapper makes the wiring trivial but does not shrink the per-connector rewrite — that's real, case-by-case work per service.

Full plan lives in the AppKit roadmap under Pillar 4 → "SDK migration".

Design

legacy.ts is the only module that imports @databricks/sdk-experimental. It constructs the client and re-exports the SDK symbols AppKit uses (Context, ConfigError, Time, TimeUnits, type namespaces files/jobs/serving/sql, etc.).
The WorkspaceClient facade exposes per-service accessors (files, warehouses, genie, jobs, statementExecution, servingEndpoints, currentUser, config, apiClient), each legacy-typed and delegating to the underlying client. Migrating a service later is a localized swap: one getter + its accessor type + one connector — the rest of the codebase never touches the SDK.
createWorkspaceClient() replaces every new WorkspaceClient(...) site (runtime + build-time type-generator). toLegacyWorkspaceClient() is the escape hatch for @databricks/lakebase, which is still on the old SDK.
createApp({ client }) and the public package index now expose the wrapper type instead of the raw SDK client. (Breaking at the type level for anyone passing a raw SDK client to createApp — no known external users.)
A Biome noRestrictedImports boundary rule forbids importing @databricks/sdk-experimental outside workspace-client/, so the seam stays enforceable. Tests mock the wrapper rather than the SDK.

Note on `Time`

Time is sourced off the SDK namespace (SDK.Time ?? SDK.default.Time) rather than a static export { Time }. The SDK's CommonJS Time export is emitted as a getter calling __importDefault(...), which defeats Node's static ESM named-export detection — a direct re-export throws "does not provide an export named 'Time'" at link time. This mirrors the guard the genie connector already used.

Verification

pnpm build — clean, publint OK
pnpm -r typecheck — 0 errors, all packages
Biome boundary rule — 0 violations; @databricks/sdk-experimental imported only in workspace-client/
pnpm check — 0 errors, 63 warnings (identical to main baseline)
Full test suite — 3284 passed / 0 failed
Built playground boots past agent registration + plugin setup (fails only at Databricks auth, as expected without credentials) — confirms the ESM Time fix end-to-end

github-actions · 2026-07-23T14:26:19Z

📦 Bundle size report

Compared against bundle-size-baseline.json (main).

`@databricks/appkit`

npm tarball (packed): 803 KB (+5.5 KB) — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist	raw	gzip
JS (runtime)	824 KB (+4.3 KB)	288 KB (+1.6 KB)
Type declarations	306 KB (+4.4 KB)	105 KB (+2.0 KB)
Source maps	1.6 MB (+8.9 KB)	539 KB (+3.6 KB)
Other	11 KB	3.7 KB
Total	2.7 MB (+18 KB)	936 KB (+7.3 KB)

Per-entry composition (own code — deps external (as shipped))

Entry	Initial (gz)	Lazy (gz)	Total (gz)	node_modules (min)	Own code (min)
`.`	86 KB (+86 B)	2.5 KB	89 KB (+86 B)	external	281 KB (+123 B)
`./beta`	45 KB (+400 B)	458 B (+29 B)	45 KB (+429 B)	external	129 KB (+623 B)
`./type-generator`	19 KB (+227 B)	0 B	19 KB (+227 B)	external	54 KB (+654 B)

Chunks:

Entry	Chunk	Load	Size (gz)
`.`	`index.js`	initial	82 KB
`.`	`utils.js`	initial	4.0 KB
`.`	`remote-tunnel-manager.js`	lazy	2.5 KB
`./beta`	`beta.js`	initial	29 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	193 B
`./beta`	`databricks.js`	lazy	142 B
`./beta`	`index.js`	lazy	123 B
`./type-generator`	`index.js`	initial	19 KB

`@databricks/appkit-ui`

npm tarball (packed): 305 KB — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist	raw	gzip
JS (runtime)	360 KB	119 KB
Type declarations	205 KB	74 KB
Source maps	686 KB	224 KB
CSS	16 KB	3.3 KB
Total	1.2 MB	422 KB

Per-entry composition (consumer bundle — deps bundled, peerDeps external)

Entry	Initial (gz)	Lazy (gz)	Total (gz)	node_modules (min)	Own code (min)
`./js`	4.3 KB	49 KB	54 KB	208 KB	12 KB
`./js/beta`	20 B	0 B	20 B	0 B	0 B
`./react`	429 KB	49 KB	478 KB	1.3 MB	168 KB
`./react/beta`	20 B	0 B	20 B	0 B	0 B

Chunks:

Entry	Chunk	Load	Size (gz)
`./js`	`index.js`	initial	4.2 KB
`./js`	`chunk`	initial	120 B
`./js`	`apache-arrow`	lazy	49 KB
`./js/beta`	`beta.js`	initial	20 B
`./react`	`index.js`	initial	427 KB
`./react`	`tslib`	initial	2.1 KB
`./react`	`apache-arrow`	lazy	49 KB
`./react/beta`	`beta.js`	initial	20 B

github-actions · 2026-07-24T10:59:28Z

🤖 AppKit PR bot

🔬 Run evals

Start an eval for this PR from the evals-monitor app: Go to Evals Monitor →

📦 Try this PR's app template

Scaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh auth login — and the Databricks CLI):

gh run download 30833418762 -R databricks/appkit -n appkit-template-0.51.0-pr.17c2a49-feat-workspace-client-wrapper-475 -D appkit-pr-475 \
  && unzip -o "appkit-pr-475/appkit-template-0.51.0-pr.17c2a49-feat-workspace-client-wrapper-475.zip" -d "appkit-pr-475" \
  && databricks apps init --template "appkit-pr-475"

The template pins @databricks/appkit and @databricks/appkit-ui to tarballs built from this branch, so the scaffolded app runs against this PR's code.

Add packages/appkit/src/workspace-client/ — a facade over the legacy @databricks/sdk-experimental client that mirrors the modular Databricks SDK's multi-client shape. This prepares the library for an incremental, per-service migration to the modular SDK without migrating anything yet. - legacy.ts is the ONLY module importing @databricks/sdk-experimental; it constructs the client and re-exports the SDK symbols AppKit uses. - The WorkspaceClient facade exposes per-service accessors (files, warehouses, genie, jobs, statementExecution, servingEndpoints, currentUser, config, apiClient), each legacy-typed and delegating to the underlying client. Migrating a service later is a localized swap: one getter + its accessor type + one connector. - createWorkspaceClient() replaces every `new WorkspaceClient(...)` site (runtime + build-time type-generator); toLegacyWorkspaceClient() is the escape hatch for @databricks/lakebase. - createApp({ client }) and the public index now expose the wrapper type instead of the raw SDK client. - A Biome noRestrictedImports boundary rule forbids importing @databricks/sdk-experimental outside workspace-client/, keeping the seam enforceable. Tests mock the wrapper rather than the SDK. Time is sourced off the SDK namespace (SDK.Time ?? SDK.default.Time) because the SDK's CommonJS `Time` export is a getter that defeats Node's static ESM named-export detection — a direct `export { Time }` throws at link time. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>

…rehouse gate The new gate test replaced `@databricks/sdk-experimental` with a bare factory mock. After #475 the type-generator reaches the SDK through `../../workspace-client`, whose `legacy.ts` destructures `ConfigError`, `Context` and `TimeUnits` off that module at import time — so the factory-only mock starved module init and the suite failed to collect. Mock the wrapper instead, spreading `importOriginal` so the re-exported SDK values survive. This matches the sibling type-generator tests and keeps the test's intent: the wrapper is now the client boundary, so the real query path still classifies the failure and the real gate still decides. Verified by re-injecting the original bug (`isEnvironmental = false` on the connectivity branch) and confirming both gate tests fail. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>

* fix(appkit): --wait typegen never overwrites committed types on degrade 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> * feat(appkit): add classifyBlockingFailure two-bucket taxonomy 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> * feat(appkit): has-types gate for environmental typegen failures in --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> * docs: document CI-resilient --wait typegen behavior 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> * chore: remove implementation-phase narration and slop from typegen changes 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> * fix(appkit): recognize wrapped and response-carried auth status in cause labels `classifyEnvironmentalCause` read only `err.status`/`err.statusCode` on the top-level error, while `errors.ts` already resolved `response.status` and walked `cause`/`AggregateError` chains. A 403 reported under `response` or wrapped in a cause chain was therefore labeled "warehouse unavailable" instead of "auth blocked", pointing CI at the wrong remedy. Move the helper next to `classifyBlockingFailure` in errors.ts so both classifiers share one status-extraction path, and reuse the existing chain walk. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit): treat unreachable warehouses as environmental in --wait typegen The blocking-mode preflight set `decision = "degradeAll"` for connectivity failures but left `isEnvironmental` false, so `hadEnvironmentalFailure` never flipped and the has-types gate never ran for an unreachable warehouse. On a fresh CI checkout that meant: queries degrade to `result: unknown`, write suppression skips `analytics.d.ts` entirely, no fatal errors are recorded, and the run exits 0 having written no types — the build then fails later somewhere less legible. Flag connectivity failures (preflight and per-query DESCRIBE) as environmental so the gate decides: warn and fall back when committed types exist, crash with the run-locally remedy when they don't. This cannot turn a passing build red — with committed types the outcome is unchanged apart from the warning now being emitted. Also return `environmentalCause` from `generateQueriesFromDescribe`, which `QueryGenerationResult` already declared and the metric path already set. Without it every query-path environmental failure fell back to the default "warehouse unavailable" label, and the "warehouse unreachable" label was unreachable in practice. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * chore(appkit): correct write-suppression comments and drop test env mutation Two comments claimed blocking mode suppresses writes only for pure degradation and that degraded artifacts are still written before a throw. `suppressDegradedWrite: mode === "blocking"` is unconditional, so any degradation suppresses the write including on runs that then throw. The behavior is what the PR intends; the comments described the old shape. The ANSI-free warning test set `process.env.CI` and deleted it in `finally`, clobbering a pre-existing value for later tests. Nothing under `src/type-generator` reads `CI`, so the assignment never affected the assertion — it only risked perturbing third-party color detection, which is exactly what this test checks. Drop it rather than stub it. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * test(appkit): drive the unreachable-warehouse gate through the real query path The gate tests in index.test.ts mock `generateQueriesFromDescribe`, so they supply the `hadEnvironmentalFailure: true` they assert on. They pass whether or not the query path actually reports it — which is how the connectivity bug survived review: the preflight returned false and no test joined the two halves. Mock only the SDK boundary so the real query path classifies the failure and the real gate decides. Covers the fresh-checkout crash (previously exit 0 with no types written), the committed-types warn-and-preserve path with the "warehouse unreachable" label, and non-blocking staying silent. Verified these fail when the isEnvironmental assignment is reverted. Co-authored-by: Isaac * test(appkit): mock the workspace-client wrapper in the unreachable-warehouse gate The new gate test replaced `@databricks/sdk-experimental` with a bare factory mock. After #475 the type-generator reaches the SDK through `../../workspace-client`, whose `legacy.ts` destructures `ConfigError`, `Context` and `TimeUnits` off that module at import time — so the factory-only mock starved module init and the suite failed to collect. Mock the wrapper instead, spreading `importOriginal` so the re-exported SDK values survive. This matches the sibling type-generator tests and keeps the test's intent: the wrapper is now the client boundary, so the real query path still classifies the failure and the real gate still decides. Verified by re-injecting the original bug (`isEnvironmental = false` on the connectivity branch) and confirming both gate tests fail. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: preserve typegen fallback for warehouse outages Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit): couple degraded queries to typegen fallback gate Signed-off-by: Atila Fassina <atila@fassina.eu> * chore(appkit): drop redundant comments from the typegen fallback gate Six comment-only removals in the has-types gate: an orphaned section header that summarised the two decision comments immediately below it, two docblocks restating single-expression predicates (`isQueryDegraded`, `hasAnyDegradedMetrics`), two trailing comments restating their own assignment, and a redundant `@param warehouseId` plus a sentence duplicating the `@param cause` list. No behaviour change. The load-bearing comments stay: the two-bucket taxonomy contracts in errors.ts, the write-suppression invariant note, and the gate test's account of how the original bug evaded weaker coverage. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> --------- Signed-off-by: Atila Fassina <atila@fassina.eu>

* feat(analytics): freeze metric-view runtime contracts (PR5 phase 0) 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> * feat(appkit): metric-view hook, formatters, generator const + payload 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> * feat(playground): wire metric-view runtime end-to-end + docs (PR5 phase 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> * chore: metrics * refactor(appkit-ui): extract MetricFilter vocabulary + toMetricFilter 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> * feat(appkit-ui): add onDataClick + selected props to charts (phases 1-2) 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> * feat(playground): chart-click cross-filter on metric-views region + segment 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> * feat: export notify for write-back feat * fix: sound metric-view row/time typing, currency + cache correctness, 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> * chore: move files and update comments * docs: trim redundant comments and convert render-types doc to JSDoc 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> * docs: drop non-existent autoStart option from useMetricView table 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> * refactor(analytics): consolidate shared logic - 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> * docs: link Plotly/ECharts to their OSS docs; trim chart-datum comment - 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> * docs: trim leftover phase labels and history narration from comments Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit): --wait typegen never overwrites committed types on degrade 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> * feat(appkit): add classifyBlockingFailure two-bucket taxonomy 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> * feat(appkit): has-types gate for environmental typegen failures in --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> * docs: document CI-resilient --wait typegen behavior 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> * chore: remove implementation-phase narration and slop from typegen changes 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> * chore: regenerate bundle-size baseline against merged tree 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> * fix(appkit): recognize wrapped and response-carried auth status in cause labels `classifyEnvironmentalCause` read only `err.status`/`err.statusCode` on the top-level error, while `errors.ts` already resolved `response.status` and walked `cause`/`AggregateError` chains. A 403 reported under `response` or wrapped in a cause chain was therefore labeled "warehouse unavailable" instead of "auth blocked", pointing CI at the wrong remedy. Move the helper next to `classifyBlockingFailure` in errors.ts so both classifiers share one status-extraction path, and reuse the existing chain walk. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit): treat unreachable warehouses as environmental in --wait typegen The blocking-mode preflight set `decision = "degradeAll"` for connectivity failures but left `isEnvironmental` false, so `hadEnvironmentalFailure` never flipped and the has-types gate never ran for an unreachable warehouse. On a fresh CI checkout that meant: queries degrade to `result: unknown`, write suppression skips `analytics.d.ts` entirely, no fatal errors are recorded, and the run exits 0 having written no types — the build then fails later somewhere less legible. Flag connectivity failures (preflight and per-query DESCRIBE) as environmental so the gate decides: warn and fall back when committed types exist, crash with the run-locally remedy when they don't. This cannot turn a passing build red — with committed types the outcome is unchanged apart from the warning now being emitted. Also return `environmentalCause` from `generateQueriesFromDescribe`, which `QueryGenerationResult` already declared and the metric path already set. Without it every query-path environmental failure fell back to the default "warehouse unavailable" label, and the "warehouse unreachable" label was unreachable in practice. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * chore(appkit): correct write-suppression comments and drop test env mutation Two comments claimed blocking mode suppresses writes only for pure degradation and that degraded artifacts are still written before a throw. `suppressDegradedWrite: mode === "blocking"` is unconditional, so any degradation suppresses the write including on runs that then throw. The behavior is what the PR intends; the comments described the old shape. The ANSI-free warning test set `process.env.CI` and deleted it in `finally`, clobbering a pre-existing value for later tests. Nothing under `src/type-generator` reads `CI`, so the assignment never affected the assertion — it only risked perturbing third-party color detection, which is exactly what this test checks. Drop it rather than stub it. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * test(appkit): drive the unreachable-warehouse gate through the real query path The gate tests in index.test.ts mock `generateQueriesFromDescribe`, so they supply the `hadEnvironmentalFailure: true` they assert on. They pass whether or not the query path actually reports it — which is how the connectivity bug survived review: the preflight returned false and no test joined the two halves. Mock only the SDK boundary so the real query path classifies the failure and the real gate decides. Covers the fresh-checkout crash (previously exit 0 with no types written), the committed-types warn-and-preserve path with the "warehouse unreachable" label, and non-blocking staying silent. Verified these fail when the isEnvironmental assignment is reverted. Co-authored-by: Isaac * test(appkit): mock the workspace-client wrapper in the unreachable-warehouse gate The new gate test replaced `@databricks/sdk-experimental` with a bare factory mock. After #475 the type-generator reaches the SDK through `../../workspace-client`, whose `legacy.ts` destructures `ConfigError`, `Context` and `TimeUnits` off that module at import time — so the factory-only mock starved module init and the suite failed to collect. Mock the wrapper instead, spreading `importOriginal` so the re-exported SDK values survive. This matches the sibling type-generator tests and keeps the test's intent: the wrapper is now the client boundary, so the real query path still classifies the failure and the real gate still decides. Verified by re-injecting the original bug (`isEnvironmental = false` on the connectivity branch) and confirming both gate tests fail. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: preserve typegen fallback for warehouse outages Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: require complete committed types for fallback Signed-off-by: Atila Fassina <atila@fassina.eu> * feat(analytics): add metric view metadata pipeline Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit-ui): tighten metric view option inference Signed-off-by: Atila Fassina <atila@fassina.eu> * chore: trim metric view comment narration Condense explanatory comments on the metric view cross-filter demo and the metadata-stamping invariant, dropping development-phase framing in favour of stating the invariant directly. Comments only, no behaviour change. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * docs(analytics): state the absent-metadata contract directly Define the no-metadata case by its concrete equivalence (envelope-identical to /query) rather than by reference to a prior state of the route. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit-ui): restore identifier stripping in formatFieldLabel Delegating to formatLabel dropped the [^a-zA-Z0-9_-] strip the wrapper previously applied. data-table passes a raw column.id and defaultFilterColumn that never pass through SAFE_KEY_REGEX, so the label echoed arbitrary input; the accompanying test had encoded that as expected. Restore the strip and correct the expectation. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * docs(appkit-ui): drop duplicated triggerLineEvent rationale The `interactive` flag already carries this explanation where it is computed, and the ChartOptions prop owns the mechanism. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * chore: update bundle size baseline Signed-off-by: Atila Fassina <atila@fassina.eu> * docs(analytics): name the real metadata type in useMetricView The useMetricView result table and the format-utilities section both referred to `MetricColumnMeta`, which is not exported anywhere. The hook returns `Record<string, MetricViewColumnDisplay>`, so a reader importing the documented name got a compile error. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit-ui): correct click datum for heatmap and radar series mapToDatum treated every array-valued datum as an [x, y] tuple. Heatmap data items are [xIndex, yIndex, value] INDEX triples, so onDataClick received the y *index* as `value` and lost the actual cell value; radar items hold one value per indicator and were reduced to their second component. Branch on `params.seriesType` instead of the datum's shape: - heatmap reports the cell value, and resolves the two indices back to their axis category labels (the raw index when labels are absent), so a caller gets the labels the user sees rather than positions. BaseChart passes the normalized axis data for this, held in a ref so the click subscription still does not re-attach when the data changes. - radar reports `value: null` rather than an arbitrary component; the full vector stays reachable through `raw`. Line/scatter tuples and scalar bar/pie data are unchanged. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit-ui): keep large integer precision in formatValue The JSON_ARRAY wire path delivers every numeric cell as a string (the SQL connector copies data_array cells verbatim), so a BIGINT or large DECIMAL measure reaches formatValue as an integer-shaped string. Coercing that through Number() rounds it: formatValue("9007199254740993", "#,##0") rendered 9,007,199,254,740,992. Format oversized integer strings exactly via BigInt instead. This is also what the existing bigint branch was written for — JSON.parse cannot yield a bigint, so that branch had no reachable caller and the precision it protects was lost on the path that actually runs. Both now share one formatBigInt helper. Only plain optionally-signed digit strings qualify; fractions and exponents stay on the Number path, where float semantics are correct. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit-ui): filter NULL dimension values with IS NULL Selecting a NULL group key built a predicate that matched nothing. The value was stringified on the way into the filter, so a NULL region compiled to `region equals 'null'` — a literal string comparison against a column that is NULL. toMetricFilter now accepts `null` and compiles it to the grammar's `notSet`, which the server already renders as IS NULL. Note the asymmetry: `undefined` means no filter on that dimension, `null` means filter to the rows where it IS NULL. A guard in toMetricFilter alone is not enough, since the value was already a string by the time it arrived, so the playground's producers pass the real `null` through: table rows, the dropdown domains (NULL sorted last, labelled "(none)" behind a sentinel because Radix rejects empty item values), and the filter chips. Chart clicks needed it too — normalizeChartData maps a NULL category to "", which would otherwise have compiled to `equals ''`. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * docs(appkit-ui): trim comment narration on the metric-view surface Tighten the doc comments across the metric-view client surface: drop restated rationale, redundant per-field descriptions, and prose that repeated what the signature already says. Comments only — no behavior, type, or formatting changes. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit-ui): fire line chart stroke clicks Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit-ui): label empty and NULL dimension values in the MV filter UI Radix `Select` throws when an item value is the empty string, and `toDimensionOptions` keeps `""` as a real value, so a dimension carrying an empty string crashed the filter dropdown on render. Add an `EMPTY` sentinel alongside `ALL`/`NONE` and decode it back to `""` so it still compiles to an `equals ''` filter, distinct from the NULL group's `IS NULL`. `FilterBadge` also interpolated the raw selection, rendering "Region: null" for a NULL group key while the active-filter chip on the same page showed "(none)" for it. Route the badge through `toDisplayLabel` so both read the same. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * docs(appkit-ui): correct the useMetricView payload memo comment The note said the memo "enforces referencial equality" — misspelled, and backwards: serializing to a string is what lets `start`'s dependency check compare the request by value instead of by object identity. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: align metric view wire types and currency formats Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit): make metric-view LIMIT deterministic, add orderBy `buildMetricSql` emitted `LIMIT n` with no `ORDER BY`. In Spark that is a non-deterministic row sample, not "the top n": the engine returns whichever rows it produced first, which varies with partitioning, parallelism and cache state. Measured against a live warehouse, identical SQL returned 3 distinct result sets across 4 runs, and a KPI card built on such a request could render a different number run to run with nothing erroring. Two changes: - `orderBy?: Array<{ field, direction? }>` on the metric request. Multi-key, `ASC`/`DESC` as a closed enum. `field` must be one of the request's own measures or dimensions — validated in-request, never against the view's declared columns, so the documented no-name-allowlist security model is preserved. Entry order is semantic and is never sorted. - When `limit` is set, the grouped dimensions not already named in `orderBy` are appended as tie-breakers. Under `GROUP BY ALL` the dimension tuple is unique per row, so the ordering becomes total and `LIMIT` is deterministic. Skipped when there are no dimensions (a pure aggregate is one row) or when `limit` is absent. Measures are ordered by their SELECT alias: `ORDER BY MEASURE(`x`)` is rejected by Spark with METRIC_VIEW_INVALID_MEASURE_FUNCTION_INPUT. Verified against a live metric view, along with alias binding under `date_trunc`/`GROUP BY ALL` and the 4-run determinism claim. Note for callers: a request that sets `limit` without `orderBy` previously returned an arbitrary subset of rows and now returns a stable one, so the rows returned may differ from before. Callers who want a ranked top-N must pass `orderBy` explicitly — the automatic tie-breakers make the result stable, not ranked. This is not an API break: `orderBy` is optional and additive, and the prior unordered `LIMIT` had no documented row-selection contract to rely on. Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit-ui): keep DECIMAL strings exact when formatting 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> * fix(appkit-ui): let a reusable orderBy array type-check against useMetricView `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> * refactor(appkit): drop redundant comments from the orderBy validators 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> * fix(appkit-ui): keep empty-string and NULL distinct on chart clicks `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> * perf(appkit-ui): parse a format spec once, not once per cell `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> * docs(analytics): deduplicate comments and reinstate config rationale 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> * fix(appkit-ui): sort metric time series chronologically Signed-off-by: Atila Fassina <atila@fassina.eu> * fix(appkit-ui): polish metric view query experience Signed-off-by: Atila Fassina <atila@fassina.eu> * chore(playground): mark the line chart click as a write-back seam 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> * feat(appkit): discover metric-view metadata instead of injecting it `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> * chore: constants and filters * fix(appkit): scope the missing-metric-metadata warning to its source 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> * docs(appkit): trim metric-view comments to their load-bearing why 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> * fix(appkit): align the metric-views.d.ts header with its siblings `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> * chore: cleanup comments * fix(appkit-ui): export MetricViewColumnDisplay for consumers 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> * fix(appkit-ui): dedupe useMetricView requests under Strict Mode 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> * fix(appkit-ui): keep tiny pie slices pointer-selectable 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> * test(dev-playground): cover metric views cross-filtering end to end 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> --------- Signed-off-by: Atila Fassina <atila@fassina.eu>

MarioCadenas requested a review from a team as a code owner July 10, 2026 13:20

MarioCadenas requested a review from ditadi July 10, 2026 13:20

MarioCadenas marked this pull request as draft July 10, 2026 14:38

MarioCadenas force-pushed the feat/workspace-client-wrapper branch from 01e0549 to 395c4a2 Compare July 23, 2026 14:23

MarioCadenas force-pushed the feat/workspace-client-wrapper branch 2 times, most recently from f45fda6 to 9e0715b Compare July 23, 2026 16:02

MarioCadenas marked this pull request as ready for review July 23, 2026 16:33

MarioCadenas force-pushed the feat/workspace-client-wrapper branch from 9e0715b to 0fdddaf Compare July 24, 2026 10:51

MarioCadenas force-pushed the feat/workspace-client-wrapper branch from 0fdddaf to 2fa18c3 Compare August 3, 2026 10:37

atilafassina reviewed Aug 3, 2026

View reviewed changes

Comment thread packages/appkit/src/workspace-client/legacy.ts Outdated

MarioCadenas force-pushed the feat/workspace-client-wrapper branch from 2fa18c3 to e30ff52 Compare August 3, 2026 16:42

atilafassina approved these changes Aug 3, 2026

View reviewed changes

MarioCadenas merged commit f6e81eb into main Aug 4, 2026
10 checks passed

MarioCadenas deleted the feat/workspace-client-wrapper branch August 4, 2026 09:20

MarioCadenas mentioned this pull request Aug 4, 2026

feat(appkit): ship the ai-search plugin at beta #508

Merged

Navigation Menu

Uh oh!

feat(appkit): introduce workspace-client wrapper over the Databricks SDK - #475

feat(appkit): introduce workspace-client wrapper over the Databricks SDK#475
MarioCadenas merged 1 commit into
mainfrom
feat/workspace-client-wrapper

MarioCadenas commented Jul 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 23, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

MarioCadenas commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why introduce a wrapper that changes nothing?

Where this goes (for context, not in this PR)

Design

Note on Time

Verification

Uh oh!

github-actions Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Bundle size report

@databricks/appkit

@databricks/appkit-ui

Uh oh!

github-actions Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 AppKit PR bot

🔬 Run evals

📦 Try this PR's app template

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MarioCadenas commented Jul 10, 2026 •

edited

Loading

Note on `Time`

github-actions Bot commented Jul 23, 2026 •

edited

Loading

`@databricks/appkit`

`@databricks/appkit-ui`

github-actions Bot commented Jul 24, 2026 •

edited

Loading