From 823e2210170c1d1574383885b289c6ee2fe9abdc Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Wed, 10 Jun 2026 18:43:45 +0200 Subject: [PATCH 01/10] feat(appkit): metric-registry type generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build-time type generator for UC Metric Views — reads config/queries/metric-views.json (entity-first metricViews map per the #429 schema; executor app_service_principal|user -> internal sp/obo lane), runs DESCRIBE TABLE EXTENDED ... AS JSON per declared view, and emits two artifacts: the MetricRegistry .d.ts augmentation (metric.d.ts: MeasureKey/DimensionKey/MetricRow/TimeGrain) and the metrics.metadata.json semantic-metadata bundle (entries { measures, dimensions }, frontend-safe). Absent config = fully dormant (zero artifacts, zero logs). Per-key DESCRIBE failures warn-and-continue and ship empty allowlists (arms the runtime fail-closed gate, next PR in chain); broken config throws loudly. Vite plugin: metricOutFile/metricMetadataOutFile options + watcher regen on metric-views.json edits. Re-implemented from #341 (a7bd6982) on current main (post-#406 non-blocking preflight surface); 3rd PR of the #341 decomposition after #427/#429. Co-authored-by: Isaac Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/index.ts | 110 +- .../src/type-generator/metric-registry.ts | 1094 +++++++++++++++ .../metric-registry.test.ts.snap | 207 +++ .../src/type-generator/tests/index.test.ts | 136 ++ .../tests/metric-registry.test.ts | 1194 +++++++++++++++++ .../type-generator/tests/vite-plugin.test.ts | 119 ++ .../appkit/src/type-generator/vite-plugin.ts | 36 +- 7 files changed, 2891 insertions(+), 5 deletions(-) create mode 100644 packages/appkit/src/type-generator/metric-registry.ts create mode 100644 packages/appkit/src/type-generator/tests/__snapshots__/metric-registry.test.ts.snap create mode 100644 packages/appkit/src/type-generator/tests/metric-registry.test.ts diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index 42b8c324..1ae9f4fb 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -3,6 +3,20 @@ import path from "node:path"; import dotenv from "dotenv"; import pc from "picocolors"; import { createLogger } from "../logging/logger"; +import { + createWorkspaceDescribeFetcher, + type DescribeFetcher, + generateMetricsMetadataJson, + generateMetricTypeDeclarations, + type MetricColumnMetadata, + type MetricLane, + type MetricSchema, + type MetricSyncFailure, + type MetricSyncResult, + readMetricConfig, + resolveMetricConfig, + syncMetrics, +} from "./metric-registry"; import { migrateProjectConfig, removeOldGeneratedTypes, @@ -199,7 +213,16 @@ declare module "@databricks/appkit-ui/react" { * @param options - the options for the generation * @param options.entryPoint - the entry point file * @param options.outFile - the output file - * @param options.querySchemaFile - optional path to query schema file (e.g. config/queries/schema.ts) + * @param options.metricOutFile - optional output file for the MetricRegistry + * augmentation. Defaults to a sibling `metric.d.ts` file under the same + * directory as `outFile`. Skipped entirely if `metric-views.json` is absent. + * @param options.metricMetadataOutFile - optional output file for the + * build-time semantic metadata JSON bundle (`metrics.metadata.json`). + * Defaults to a sibling of `metricOutFile`. Skipped entirely if + * `metric-views.json` is absent. + * @param options.metricFetcher - optional DescribeFetcher used by + * {@link syncMetrics}. Tests inject a mock; production builds let the + * default WorkspaceClient-backed fetcher be created lazily. */ export async function generateFromEntryPoint(options: { outFile: string; @@ -207,6 +230,9 @@ export async function generateFromEntryPoint(options: { warehouseId: string; noCache?: boolean; mode?: PreflightMode; + metricOutFile?: string; + metricMetadataOutFile?: string; + metricFetcher?: DescribeFetcher; }) { const { outFile, @@ -214,6 +240,9 @@ export async function generateFromEntryPoint(options: { warehouseId, noCache, mode = "non-blocking", + metricOutFile, + metricMetadataOutFile, + metricFetcher, } = options; const projectRoot = resolveProjectRoot(outFile); @@ -237,6 +266,59 @@ export async function generateFromEntryPoint(options: { await fs.mkdir(path.dirname(outFile), { recursive: true }); await fs.writeFile(outFile, typeDeclarations, "utf-8"); + // Metric-view types: only emit when metric-views.json exists. The path is + // purely additive — apps that never adopt metric views must not produce + // empty noise. + if (queryFolder) { + const metricConfig = await readMetricConfig(queryFolder); + if (metricConfig) { + const resolution = resolveMetricConfig(metricConfig); + const fetcher = + metricFetcher ?? createWorkspaceDescribeFetcher(warehouseId); + const { schemas: metricSchemas, failures } = await syncMetrics( + resolution, + fetcher, + ); + + // Surface DESCRIBE failures loudly so a misconfigured metric-views.json + // or a workspace-side typo doesn't silently ship an empty bundle entry. + // The route's runtime fail-closed gate would 503 these in production — + // catching the issue at type-gen time is the cheaper signal. + if (failures.length > 0) { + for (const f of failures) { + logger.warn( + "metric sync failed for %s (%s): %s", + f.key, + f.source, + f.reason, + ); + } + } + + const metricFile = + metricOutFile ?? path.join(path.dirname(outFile), METRIC_TYPES_FILE); + const metricDeclarations = generateMetricTypeDeclarations(metricSchemas); + await fs.mkdir(path.dirname(metricFile), { recursive: true }); + await fs.writeFile(metricFile, metricDeclarations, "utf-8"); + + // Emit the semantic-metadata JSON bundle alongside the .d.ts. The hook + // imports this artifact (via a registration call from the consuming + // app) and exposes the per-metric subset on its return value. + const metadataFile = + metricMetadataOutFile ?? + path.join(path.dirname(metricFile), METRIC_METADATA_FILE); + const metadataJson = generateMetricsMetadataJson(metricSchemas); + await fs.mkdir(path.dirname(metadataFile), { recursive: true }); + await fs.writeFile(metadataFile, metadataJson, "utf-8"); + + logger.debug( + "Wrote MetricRegistry augmentation + metadata bundle for %d metric(s)%s", + metricSchemas.length, + failures.length > 0 ? ` (${failures.length} failure(s))` : "", + ); + } + } + // One-time migration: remove old generated file and patch project configs await removeOldGeneratedTypes(projectRoot, "appKitTypes.d.ts"); await migrateProjectConfig(projectRoot); @@ -260,9 +342,35 @@ export async function generateFromEntryPoint(options: { // mirroring how generateFromEntryPoint (also defined here) is preserved via the analytics vite plugin. export const generateServingTypes = generateServingTypesImpl; +// Re-export the metric-registry types so consumers (CLI, the type-generator +// .d.ts shim in `packages/shared`) can pick them up from this entry point — +// the .d.ts shim documents these as part of the package's public surface. +export type { + MetricColumnMetadata, + MetricLane, + MetricSchema, + MetricSyncFailure, + MetricSyncResult, +}; + /** Directory name for generated AppKit type declaration files. */ export const TYPES_DIR = "appkit-types"; /** Default filename for analytics query type declarations. */ export const ANALYTICS_TYPES_FILE = "analytics.d.ts"; /** Default filename for serving endpoint type declarations. */ export const SERVING_TYPES_FILE = "serving.d.ts"; +/** Default filename for metric-view registry type declarations. */ +export const METRIC_TYPES_FILE = "metric.d.ts"; +/** + * Default filename for the build-time semantic-metadata JSON bundle. + * + * Sibling of {@link METRIC_TYPES_FILE}. The JSON shape is + * `Record` — see `MetricsMetadataBundle` + * in `metric-registry.ts` (UC FQN and execution lane are server-side concerns + * and deliberately not part of this client-shipped artifact). The consuming + * app imports this file at build time (via Vite's JSON loader / Webpack's + * `import` etc.) and registers it through `@databricks/appkit-ui/format`'s + * `registerMetricsMetadata()` so the React hook can return per-metric + * `metadata` without a second network round-trip. + */ +export const METRIC_METADATA_FILE = "metrics.metadata.json"; diff --git a/packages/appkit/src/type-generator/metric-registry.ts b/packages/appkit/src/type-generator/metric-registry.ts new file mode 100644 index 00000000..17947255 --- /dev/null +++ b/packages/appkit/src/type-generator/metric-registry.ts @@ -0,0 +1,1094 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { WorkspaceClient } from "@databricks/sdk-experimental"; +import { createLogger } from "../logging/logger"; +import type { DatabricksStatementExecutionResponse } from "./types"; + +const logger = createLogger("type-generator:metric-registry"); + +/** + * Default filename for the metric source declarations. + * Lives at config/queries/metric-views.json by convention. + * + * Absence of the file means the metric-view path is dormant — + * {@link readMetricConfig} returns `null` silently (no fallback to any legacy + * filename, no log noise). + */ +const METRIC_CONFIG_FILE = "metric-views.json"; + +/** + * The lane an entry sits in: `sp` (service principal, shared cache) + * or `obo` (on-behalf-of, per-user cache). + * + * Lanes are internal vocabulary — the config speaks `executor` + * ("app_service_principal" | "user") and {@link resolveMetricConfig} derives + * the lane at the parse boundary. + */ +export type MetricLane = "sp" | "obo"; + +/** + * Single entry in the `metricViews` map of metric-views.json. + * + * v1 allows `source` plus the optional `executor`. Object form (rather than + * bare string) is the forward-compat seam for future per-entry options + * (cacheTtl, defaultFilter, ...) — `executor` is the first such option. + */ +interface MetricEntryConfig { + source: string; + executor?: "app_service_principal" | "user"; +} + +/** + * Shape of metric-views.json (mirrors `metricSourceSchema` in + * `packages/shared/src/schemas/metric-source.ts`). Inlined here so the + * type-generator does not pull in the shared schema package at runtime. + */ +interface MetricSourceConfig { + $schema?: string; + metricViews?: Record; +} + +/** + * Resolved entry consumed by the rest of the metric-view pipeline. + * Lane is denormalized onto the entry so downstream code does not have to + * re-derive it from the config's `executor` field. + */ +interface ResolvedMetricEntry { + /** Stable map key shared across route, hook, registry, and cache. */ + key: string; + /** Three-part Unity Catalog FQN of the metric view. */ + source: string; + /** Execution lane — sp = service principal, obo = on-behalf-of. */ + lane: MetricLane; +} + +/** + * Per-column metadata extracted from DESCRIBE TABLE EXTENDED ... AS JSON. + * + * Phase 1 captured measure flags + types. Phase 2 widens to time-typed + * dimensions: grain qualification is inferred from the column's SQL type + * (TIMESTAMP* / DATE) — the UC metric-view YAML schema has no per-column + * `time_grain` attribute, so the type is the only signal available. + * + * Phase 5 captures the YAML 1.1 semantic-metadata fields so the build-time + * artifact is a complete record of what the metric view declares: display name + * (used by `formatLabel` to render axis titles / legend entries / tooltips), + * format spec (printf-like string consumed by `formatValue` and `toD3Format`), + * and description (column-level documentation). All three are optional in the + * YAML; the extractor leaves the field undefined when absent. + */ +export interface MetricColumnMetadata { + name: string; + type: string; + /** UC marks columns produced by `MEASURE()` as measures; everything else is a dimension. */ + isMeasure: boolean; + /** Optional column comment / display description (best-effort). */ + description?: string; + /** + * Human-readable display name from the YAML 1.1 `display_name` attribute. + * Used by `formatLabel` as the canonical axis / legend / tooltip text; + * absent → callers fall back to camelCase / snake_case humanization of `name`. + */ + displayName?: string; + /** + * Printf-style format spec from the YAML 1.1 `format` attribute (e.g. + * `"$#,##0.00"`, `"0.0%"`, `"#,##0"`). `formatValue` and `toD3Format` + * consume this passthrough — the framework deliberately does not invent a + * format DSL; we forward the YAML's verbatim string and fall back to + * sensible defaults when the spec is absent or unrecognized. + */ + format?: string; + /** + * Standard time-grain set for this column, inferred from the SQL data type: + * TIMESTAMP* → 7 grains (minute..year); DATE → 5 grains (day..year). + * Undefined means the column is not time-typed. Measures never get grains. + */ + timeGrains?: string[]; +} + +/** + * Per-metric schema captured at type-generation time. + * + * The full row type is the union of measure + dimension column types. Phase 1 + * uses only `measures`; Phase 2 widens to `dimensions` and `timeGrains`. + */ +export interface MetricSchema { + /** Stable metric key (the map key under `metricViews` in metric-views.json). */ + key: string; + /** Three-part FQN of the metric view. */ + source: string; + /** Execution lane this metric was registered under. */ + lane: MetricLane; + /** Measure columns (those exposed by MEASURE()). */ + measures: MetricColumnMetadata[]; + /** Dimension columns (everything that is not a measure). */ + dimensions: MetricColumnMetadata[]; +} + +/** + * Result of reading and resolving metric-views.json — a flat entries list + * with the lane denormalized for iteration. + */ +interface MetricConfigResolution { + entries: ResolvedMetricEntry[]; +} + +/** + * Read metric-views.json from a queries folder. + * + * Returns `null` if the file does not exist (the metric-view path is + * additive — apps without metric-views.json must not be penalized). There is + * deliberately no fallback to the legacy `metric.json` filename. + * + * Throws on JSON parse errors so misconfiguration surfaces loudly. + */ +export async function readMetricConfig( + queryFolder: string, +): Promise { + const metricPath = path.join(queryFolder, METRIC_CONFIG_FILE); + let raw: string; + try { + raw = await fs.readFile(metricPath, "utf8"); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") { + return null; + } + throw err; + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + throw new Error( + `Failed to parse metric-views.json at ${metricPath}: ${(err as Error).message}`, + ); + } + + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + throw new Error( + `Invalid metric-views.json at ${metricPath}: expected an object with a 'metricViews' map.`, + ); + } + + return parsed as MetricSourceConfig; +} + +/** + * Validate a key against the JSON Schema's metricKey pattern. Kept + * lightweight — the shared Zod schema (`metricSourceSchema`) is the canonical + * contract for IDE/CI; this regex is identical to its `metricKeySchema`. + */ +function isValidMetricKey(key: string): boolean { + return /^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key); +} + +/** + * Validate a UC FQN against the shared schema's source pattern. + */ +function isValidFqn(fqn: string): boolean { + return /^[a-zA-Z0-9_][a-zA-Z0-9_-]*\.[a-zA-Z0-9_][a-zA-Z0-9_-]*\.[a-zA-Z0-9_][a-zA-Z0-9_-]*$/.test( + fqn, + ); +} + +/** + * Resolve the `metricViews` map into a flat list of entries. + * + * The internal lane is derived from each entry's `executor` at this parse + * boundary: `"user"` → `obo`; `"app_service_principal"` or absent → `sp`. + * Downstream consumers only ever see lanes. + * + * Throws on unknown top-level fields, invalid keys, non-object entries, + * unknown entry fields, invalid FQNs, or invalid executors. A single map + * makes duplicate metric keys unrepresentable by construction. Stable + * ordering: alphabetical by key. + */ +export function resolveMetricConfig( + config: MetricSourceConfig, +): MetricConfigResolution { + // v1 explicitly rejects unknown top-level fields so the legacy sp/obo lane + // shape (and future additions) cannot be silently consumed today. + const allowedTopLevel = new Set(["$schema", "metricViews"]); + for (const field of Object.keys(config)) { + if (!allowedTopLevel.has(field)) { + throw new Error( + `Invalid top-level field "${field}" in metric-views.json: only '$schema' and 'metricViews' are allowed.`, + ); + } + } + + const metricViews = config.metricViews ?? {}; + if ( + typeof metricViews !== "object" || + metricViews === null || + Array.isArray(metricViews) + ) { + throw new Error( + `Invalid 'metricViews' in metric-views.json: expected an object map of metric entries.`, + ); + } + + const entries: ResolvedMetricEntry[] = []; + const sortedKeys = Object.keys(metricViews).sort(); + for (const key of sortedKeys) { + if (!isValidMetricKey(key)) { + throw new Error( + `Invalid metric key "${key}" in metricViews: must match /^[a-zA-Z_][a-zA-Z0-9_]*$/.`, + ); + } + + const entry = metricViews[key]; + if (!entry || typeof entry !== "object" || Array.isArray(entry)) { + throw new Error( + `Invalid metric entry "${key}": expected an object with a 'source' field.`, + ); + } + + // v1 explicitly rejects unknown entry fields so future additions cannot + // be silently consumed today. + const allowed = new Set(["source", "executor"]); + for (const field of Object.keys(entry)) { + if (!allowed.has(field)) { + throw new Error( + `Invalid field "${field}" on metric entry "${key}": only 'source' and 'executor' are allowed at v1.`, + ); + } + } + + if (typeof entry.source !== "string" || entry.source.trim() === "") { + throw new Error( + `Invalid metric entry "${key}": 'source' must be a non-empty string.`, + ); + } + + if (!isValidFqn(entry.source)) { + throw new Error( + `Invalid metric source "${entry.source}" for "${key}": expected a three-part UC FQN ...`, + ); + } + + const executor = entry.executor; + if ( + executor !== undefined && + executor !== "app_service_principal" && + executor !== "user" + ) { + throw new Error( + `Invalid executor "${String(executor)}" on metric entry "${key}": must be "app_service_principal" or "user".`, + ); + } + + const lane: MetricLane = executor === "user" ? "obo" : "sp"; + + entries.push({ key, source: entry.source, lane }); + } + + return { entries }; +} + +/** + * Parse the JSON payload returned by DESCRIBE TABLE EXTENDED ... AS JSON. + * + * The Statement Execution API returns a single string cell — this normalizer + * unwraps it. Handles both the production (real warehouse) shape and the + * shape produced by mocked test responses. + */ +export function parseDescribeTableExtendedJson( + response: DatabricksStatementExecutionResponse, +): unknown { + if (response.status?.state === "FAILED") { + const msg = response.status.error?.message ?? "DESCRIBE failed"; + throw new Error(`DESCRIBE TABLE EXTENDED failed: ${msg}`); + } + + const rows = response.result?.data_array ?? []; + if (rows.length === 0) { + throw new Error( + "DESCRIBE TABLE EXTENDED returned no rows. Verify the FQN points to a metric view.", + ); + } + + const cell = rows[0]?.[0]; + if (typeof cell !== "string") { + throw new Error( + "DESCRIBE TABLE EXTENDED first cell was not a JSON string. Confirm the AS JSON suffix is supported.", + ); + } + + try { + return JSON.parse(cell); + } catch (err) { + throw new Error( + `Failed to parse DESCRIBE TABLE EXTENDED JSON: ${(err as Error).message}`, + ); + } +} + +/** + * Pure function: turn the parsed DESCRIBE JSON into structured column metadata. + * + * Tolerant of multiple JSON shapes (the field may be `columns` or `schema.fields`, + * type may be a string or `{ name }` object, the measure marker may be `is_measure` + * or under `metadata.is_measure`). Phase 1's job is to find names + measure flags; + * later phases can tighten this if a more authoritative shape stabilizes. + */ +export function extractMetricColumns(parsed: unknown): MetricColumnMetadata[] { + if (!parsed || typeof parsed !== "object") { + return []; + } + + const root = parsed as Record; + const columnsCandidate = (root.columns ?? + (root.schema && typeof root.schema === "object" + ? (root.schema as Record).fields + : undefined)) as unknown; + + if (!Array.isArray(columnsCandidate)) { + return []; + } + + const columns: MetricColumnMetadata[] = []; + for (const raw of columnsCandidate) { + if (!raw || typeof raw !== "object") continue; + const obj = raw as Record; + const name = + typeof obj.name === "string" + ? obj.name + : typeof obj.column_name === "string" + ? obj.column_name + : undefined; + if (!name) continue; + + const typeRaw = obj.type ?? obj.data_type ?? obj.type_name; + let type = "STRING"; + if (typeof typeRaw === "string") { + type = typeRaw; + } else if (typeRaw && typeof typeRaw === "object") { + const inner = (typeRaw as Record).name; + if (typeof inner === "string") type = inner; + } + + let isMeasure = false; + if (typeof obj.is_measure === "boolean") { + isMeasure = obj.is_measure; + } else if ( + obj.metadata && + typeof obj.metadata === "object" && + typeof (obj.metadata as Record).is_measure === "boolean" + ) { + isMeasure = (obj.metadata as Record) + .is_measure as boolean; + } else if (obj.kind === "measure" || obj.role === "measure") { + isMeasure = true; + } + + const description = + typeof obj.comment === "string" + ? obj.comment + : typeof obj.description === "string" + ? obj.description + : undefined; + + const displayName = extractStringFromAny(obj, [ + "display_name", + "displayName", + ]); + const format = extractFormatString(obj); + + // Time-grain inference is type-driven, not YAML-attribute-driven. + // Earlier versions of this code looked for a `time_grain` field on each + // column, but that field does not exist in UC's metric-view schema — + // the Rust serde at universe/reyden/metric-view-serde/src/v11/column.rs + // enumerates the 7 known column properties (window, expr, format, + // display_name, name, comment, synonyms). CREATE rejects `time_grain` + // with "Unrecognized field". Measures don't get grouped, so skip them. + const timeGrains = isMeasure ? undefined : inferTimeGrains(type); + + columns.push({ + name, + type, + isMeasure, + description, + ...(displayName ? { displayName } : {}), + ...(format ? { format } : {}), + ...(timeGrains ? { timeGrains } : {}), + }); + } + + return columns; +} + +/** + * Read a non-empty string attribute from a DESCRIBE column entry, tolerating + * the multiple shapes UC has shipped for this metadata over time. + * + * For each candidate name, we check the column object directly, then under + * `metadata.`. The first non-empty trimmed string wins. Empty / missing + * → undefined (the caller leaves the field off the emitted artifact). + */ +function extractStringFromAny( + obj: Record, + candidates: readonly string[], +): string | undefined { + for (const key of candidates) { + const direct = obj[key]; + if (typeof direct === "string" && direct.trim().length > 0) { + return direct; + } + const meta = obj.metadata; + if (meta && typeof meta === "object" && !Array.isArray(meta)) { + const nested = (meta as Record)[key]; + if (typeof nested === "string" && nested.trim().length > 0) { + return nested; + } + } + } + return undefined; +} + +/** + * Read the column's `format` attribute from a DESCRIBE entry and return a + * printf-like format string suitable for `formatValue` and `toD3Format`. + * + * Tolerates two source shapes: + * + * 1. **Legacy / hand-authored** — `format: "$#,##0.00"` (already a printf + * string). Returned as-is. + * + * 2. **YAML 1.1 structured** — DESCRIBE TABLE EXTENDED ... AS JSON for a + * UC Metric View wraps the column's format type as the outer key: + * + * ``` + * { "currency": { "decimal_places": { "places": 2 }, "currency_code": "USD" } } + * { "percent": { "decimal_places": { "places": 1 } } } + * { "number": { "decimal_places": { "places": 0 } } } + * ``` + * + * Both shapes are checked at top-level (`obj.format` / `obj.format_spec`) + * and under `metadata.` for parity with extractStringFromAny. + * + * Unrecognized objects return undefined; downstream consumers fall back to + * default locale formatting. + */ +function extractFormatString(obj: Record): string | undefined { + for (const key of ["format", "format_spec"]) { + const direct = obj[key]; + const fromDirect = formatStringFromValue(direct); + if (fromDirect) return fromDirect; + + const meta = obj.metadata; + if (meta && typeof meta === "object" && !Array.isArray(meta)) { + const nested = (meta as Record)[key]; + const fromMeta = formatStringFromValue(nested); + if (fromMeta) return fromMeta; + } + } + return undefined; +} + +function formatStringFromValue(value: unknown): string | undefined { + if (typeof value === "string" && value.trim().length > 0) return value.trim(); + if (value && typeof value === "object" && !Array.isArray(value)) { + return translateStructuredFormat(value as Record); + } + return undefined; +} + +/** + * Translate the structured `format` object emitted by DESCRIBE TABLE EXTENDED + * AS JSON into a printf-like format string. Recognizes the three YAML 1.1 + * shapes; returns undefined for anything else. + */ +function translateStructuredFormat( + spec: Record, +): string | undefined { + if (spec.currency && typeof spec.currency === "object") { + return currencyFormatString(spec.currency as Record); + } + if (spec.percent && typeof spec.percent === "object") { + return percentFormatString(spec.percent as Record); + } + if (spec.number && typeof spec.number === "object") { + return numberFormatString(spec.number as Record); + } + return undefined; +} + +function currencyFormatString(c: Record): string { + const places = readDecimalPlaces(c) ?? 2; + const codeRaw = c.currency_code; + const code = + typeof codeRaw === "string" && codeRaw.trim().length > 0 + ? codeRaw.toUpperCase() + : "USD"; + const symbol = currencySymbol(code); + return `${symbol}#,##0${fractionalSuffix(places)}`; +} + +function percentFormatString(p: Record): string { + const places = readDecimalPlaces(p) ?? 0; + return `0${fractionalSuffix(places)}%`; +} + +function numberFormatString(n: Record): string { + const places = readDecimalPlaces(n) ?? 0; + return `#,##0${fractionalSuffix(places)}`; +} + +function fractionalSuffix(places: number): string { + return places > 0 ? `.${"0".repeat(places)}` : ""; +} + +function readDecimalPlaces(obj: Record): number | undefined { + const dp = obj.decimal_places; + if (typeof dp === "number" && Number.isFinite(dp) && dp >= 0) { + return Math.floor(dp); + } + if (dp && typeof dp === "object" && !Array.isArray(dp)) { + const places = (dp as Record).places; + if (typeof places === "number" && Number.isFinite(places) && places >= 0) { + return Math.floor(places); + } + } + return undefined; +} + +/** + * Map ISO currency codes to their conventional prefix symbol. Unknown codes + * fall back to the literal code + space (e.g., "AUD #,##0.00") so the value + * is never lost — `formatValue` and `toD3Format` will still render correctly, + * just without a single-character glyph. + */ +function currencySymbol(code: string): string { + switch (code) { + case "USD": + return "$"; + case "EUR": + return "€"; + case "GBP": + return "£"; + case "JPY": + case "CNY": + return "¥"; + case "INR": + return "₹"; + case "BRL": + return "R$"; + default: + return `${code} `; + } +} + +/** + * Infer the standard set of valid time grains for a dimension based on its + * SQL data type. + * + * TIMESTAMP / TIMESTAMP_LTZ / TIMESTAMP_NTZ → all 7 standard grains + * DATE → [day, week, month, quarter, year] (no sub-day grains) + * anything else → undefined (not time-typed) + * + * Earlier code looked for a `time_grain` attribute on the YAML column. That + * field does not exist in the UC metric-view schema (see the v11 Rust serde + * — Column has 7 known properties: window, expr, format, display_name, + * name, comment, synonyms; CREATE fails with "Unrecognized field + * 'time_grain'"). So grain qualification has to come from the column's + * resolved SQL type instead. + */ +function inferTimeGrains(type: string): string[] | undefined { + // Strip parameterized suffixes ("TIMESTAMP(6)" → "TIMESTAMP") and trim. + const normalized = type + .toLowerCase() + .replace(/\(.*\)$/, "") + .trim(); + if ( + normalized === "timestamp" || + normalized === "timestamp_ltz" || + normalized === "timestamp_ntz" + ) { + return ["day", "hour", "minute", "month", "quarter", "week", "year"]; + } + if (normalized === "date") { + return ["day", "month", "quarter", "week", "year"]; + } + return undefined; +} + +/** + * Map a Databricks SQL type to a TypeScript primitive. + * Centralized here (not imported from query-registry) so this module + * stays self-contained at Phase 1. + */ +function tsTypeFor(sqlType: string): string { + const normalized = sqlType + .toUpperCase() + .replace(/\(.*\)$/, "") + .replace(/<.*>$/, "") + .split(" ")[0]; + + switch (normalized) { + case "BOOLEAN": + return "boolean"; + case "TINYINT": + case "SMALLINT": + case "INT": + case "INTEGER": + case "BIGINT": + case "FLOAT": + case "DOUBLE": + case "DECIMAL": + case "NUMERIC": + return "number"; + default: + return "string"; + } +} + +/** + * Render a MetricRegistry interface entry from a MetricSchema. + */ +function renderMetricEntry(schema: MetricSchema): string { + const indent = " "; + const measures = + schema.measures.length > 0 + ? schema.measures + .map( + (m) => `${indent}/** @sqlType ${m.type} */ +${indent}${JSON.stringify(m.name)}: ${tsTypeFor(m.type)}`, + ) + .join(";\n") + : ""; + const dimensions = + schema.dimensions.length > 0 + ? schema.dimensions + .map((d) => { + const grainComment = d.timeGrains?.length + ? ` @timeGrain ${d.timeGrains.join("|")}` + : ""; + return `${indent}/** @sqlType ${d.type}${grainComment} */ +${indent}${JSON.stringify(d.name)}: ${tsTypeFor(d.type)}`; + }) + .join(";\n") + : ""; + + const measureKeys = schema.measures.map((m) => JSON.stringify(m.name)); + const dimensionKeys = schema.dimensions.map((d) => JSON.stringify(d.name)); + + const measuresBlock = measures + ? `{ +${measures}; + }` + : "Record"; + + const dimensionsBlock = dimensions + ? `{ +${dimensions}; + }` + : "Record"; + + const measureUnion = + measureKeys.length > 0 ? measureKeys.join(" | ") : "never"; + const dimensionUnion = + dimensionKeys.length > 0 ? dimensionKeys.join(" | ") : "never"; + + // Union of allowed time-grains across every time-typed dimension. The PRD + // documents the v1 contract: a single top-level `timeGrain` applies to all + // time-typed dims. Therefore the type-level constraint is the union (any of + // the dim-allowed grains is acceptable; per-dim narrowing is a future + // widening to `TimeGrain | Record, TimeGrain>`). + const timeGrainSet = new Set(); + for (const d of schema.dimensions) { + for (const g of d.timeGrains ?? []) { + timeGrainSet.add(g); + } + } + const timeGrainUnion = + timeGrainSet.size > 0 + ? [...timeGrainSet] + .sort() + .map((g) => JSON.stringify(g)) + .join(" | ") + : "never"; + + const measureMetadata = renderMetadataMap(schema.measures, indent); + const dimensionMetadata = renderMetadataMap(schema.dimensions, indent, true); + + return ` ${JSON.stringify(schema.key)}: { + key: ${JSON.stringify(schema.key)}; + source: ${JSON.stringify(schema.source)}; + lane: ${JSON.stringify(schema.lane)}; + measures: ${measuresBlock}; + dimensions: ${dimensionsBlock}; + measureKeys: ${measureUnion}; + dimensionKeys: ${dimensionUnion}; + timeGrains: ${timeGrainUnion}; + metadata: { + measures: ${measureMetadata}; + dimensions: ${dimensionMetadata}; + }; + }`; +} + +/** + * Render the type-level shape of a column's semantic-metadata map for the + * `metadata` field of a MetricRegistry entry. + * + * The shape mirrors {@link MetricColumnSemanticMetadata}: each column emits an + * object literal with `type` (string literal) plus optional `display_name`, + * `format`, `description` (string literals when known, dropped when absent), + * and — for dimensions only — `time_grain` (the column's allowed-grain tuple + * literal). + * + * When the column list is empty, the type collapses to `Record` + * so consumers can still index into `metadata.measures` / `metadata.dimensions` + * without TypeScript errors. + */ +function renderMetadataMap( + cols: MetricColumnMetadata[], + indent: string, + includeTimeGrain = false, +): string { + if (cols.length === 0) return "Record"; + + const inner = cols + .map((col) => { + const fields: string[] = [`type: ${JSON.stringify(col.type)}`]; + if (col.displayName) { + fields.push(`display_name: ${JSON.stringify(col.displayName)}`); + } + if (col.format) { + fields.push(`format: ${JSON.stringify(col.format)}`); + } + if (col.description) { + fields.push(`description: ${JSON.stringify(col.description)}`); + } + if (includeTimeGrain && col.timeGrains && col.timeGrains.length > 0) { + const grainTuple = col.timeGrains + .map((g) => JSON.stringify(g)) + .join(", "); + fields.push(`time_grain: readonly [${grainTuple}]`); + } + const fieldsBlock = fields.map((f) => `${indent} ${f}`).join(";\n"); + return `${indent}${JSON.stringify(col.name)}: { +${fieldsBlock}; +${indent}}`; + }) + .join(";\n"); + + return `{ +${inner}; + }`; +} + +/** + * Render the augmentation block for the appkit-ui MetricRegistry interface. + * + * Mirrors the pattern in `generateTypeDeclarations` for QueryRegistry — emits + * a `declare module` block that consumers in `@databricks/appkit-ui/react` + * pick up via TypeScript module augmentation. + */ +function renderMetricRegistry(schemas: MetricSchema[]): string { + if (schemas.length === 0) { + return `declare module "@databricks/appkit-ui/react" { + interface MetricRegistry {} +} +`; + } + const entries = schemas.map(renderMetricEntry).join(";\n"); + return `declare module "@databricks/appkit-ui/react" { + interface MetricRegistry { +${entries}; + } +} +`; +} + +/** + * Default header for the generated metric.d.ts file. The file is consumed by + * TypeScript via module augmentation only, so no runtime import is needed. + */ +function metricFileHeader(): string { + return `// Auto-generated by AppKit - DO NOT EDIT +// Generated by 'npx @databricks/appkit generate-types' or Vite plugin during build +import "@databricks/appkit-ui/react"; +`; +} + +/** + * Build the full metric.d.ts file from a list of metric schemas. + */ +export function generateMetricTypeDeclarations( + schemas: MetricSchema[], +): string { + return metricFileHeader() + renderMetricRegistry(schemas); +} + +/** + * Per-column metadata as emitted into the build-time JSON artifact. + * + * The shape is deliberately narrow — we forward what the YAML 1.1 declared + * (type, display name, format spec, description) plus the time-grain list for + * dimensions. Consumers (the React hook, the format utilities) destructure + * only the fields they need; absent fields stay absent rather than carrying + * empty-string sentinels so JSON.stringify output is minimal. + * + * Internal — exposed via the {@link buildMetricsMetadataBundle} return shape. + * Library consumers see this shape mirrored verbatim in + * `@databricks/appkit-ui/format`'s `ColumnMetadata` (they import there, not + * here). + */ +interface MetricColumnSemanticMetadata { + type: string; + display_name?: string; + format?: string; + description?: string; + /** Only emitted on dimension entries that resolved to a TIMESTAMP* or DATE SQL type (grain set inferred from type). */ + time_grain?: readonly string[]; +} + +/** + * One metric's complete semantic-metadata bundle. + * + * Splits cleanly into measures + dimensions so the consuming hook can return + * the exact subset for the queried metric without scanning the rest of the + * registry. + * + * Server-side concerns — UC FQN (`source`) and execution lane (`lane`) — are + * deliberately NOT part of this artifact. They live in metric-views.json and + * are consumed by the server only. The bundle ships to the client in + * `metrics.metadata.json` and must contain frontend-safe metadata only + * (display names, format specs, descriptions, time-grain hints). + */ +interface MetricSemanticMetadataEntry { + measures: Record; + dimensions: Record; +} + +/** + * Top-level shape of `metrics.metadata.json` — keyed by metric key. + * + * Loaded by: + * - the server-side `loadMetricRegistry` (for body-validator awareness of + * display names + types in error messages, when wired up in a follow-on) + * - the client-side `useMetricView` hook (returned in the `metadata` field) + * - any chart-library glue code that wants direct access to format specs / + * display names (Plotly tickformat, ECharts valueFormatter, table cells, ...) + */ +type MetricsMetadataBundle = Record; + +/** + * Pure function: turn a list of metric schemas into the JSON metadata bundle. + * + * Deterministic key order: outer object keys are sorted alphabetically; + * measures and dimensions are emitted in the order they appeared in DESCRIBE + * (Phase 1's preserved-from-YAML order), but each per-column object's fields + * follow a fixed declaration order so snapshot diffs are stable. + * + * The output is `JSON.stringify`'d with two-space indentation by the file + * emitter — keeping the data structure pure here lets unit tests assert on the + * structure without parsing. + */ +export function buildMetricsMetadataBundle( + schemas: MetricSchema[], +): MetricsMetadataBundle { + const bundle: MetricsMetadataBundle = {}; + const sortedSchemas = [...schemas].sort((a, b) => a.key.localeCompare(b.key)); + + for (const schema of sortedSchemas) { + const measures: Record = {}; + for (const m of schema.measures) { + measures[m.name] = buildColumnMetadata(m); + } + + const dimensions: Record = {}; + for (const d of schema.dimensions) { + dimensions[d.name] = buildColumnMetadata(d); + } + + bundle[schema.key] = { + measures, + dimensions, + }; + } + + return bundle; +} + +/** + * Render one column's emitted semantic-metadata object. + * + * Field order is fixed (`type`, `display_name`, `format`, `description`, + * `time_grain`) and absent fields are simply not included, so the snapshot + * diff is always minimal — consumers receive only what the YAML declared. + * + * `time_grain` is only emitted on dimensions whose SQL type is TIMESTAMP* or + * DATE — measures never receive a grain since they aren't grouped on. The + * caller (extractMetricColumns) skips inference for `isMeasure: true` columns. + */ +function buildColumnMetadata( + col: MetricColumnMetadata, +): MetricColumnSemanticMetadata { + const entry: MetricColumnSemanticMetadata = { type: col.type }; + if (col.displayName) entry.display_name = col.displayName; + if (col.format) entry.format = col.format; + if (col.description) entry.description = col.description; + if (!col.isMeasure && col.timeGrains && col.timeGrains.length > 0) { + entry.time_grain = [...col.timeGrains]; + } + return entry; +} + +/** + * Serialize the metadata bundle to a stable, human-readable JSON string. + * + * Uses two-space indentation and a trailing newline so file diffs are clean + * across regenerations; the bundle's own key order is already sorted by + * {@link buildMetricsMetadataBundle}. + */ +export function generateMetricsMetadataJson(schemas: MetricSchema[]): string { + const bundle = buildMetricsMetadataBundle(schemas); + return `${JSON.stringify(bundle, null, 2)}\n`; +} + +/** + * Optional dependency-injection seam: the function used to fetch DESCRIBE + * results for a given FQN. Production wires this through the WorkspaceClient; + * tests inject a mock that returns a representative DESCRIBE response. + */ +export type DescribeFetcher = ( + fqn: string, +) => Promise; + +/** + * Build a DescribeFetcher from a real WorkspaceClient + warehouseId. + * + * Kept narrow so it does not require importing the SDK at test time. + * + * `wait_timeout: "30s"` makes the API wait synchronously for the statement + * to complete (matching the SDK's own example pattern). Without an explicit + * wait, the call can return while the statement is still PENDING/RUNNING — + * the response carries no `data_array` yet, `parseDescribeTableExtendedJson` + * reads that as "returned no rows", and the registry ships empty. The + * runtime fail-closed gate then 503s every metric request, which is exactly + * the symptom we hit on a cold warehouse. + */ +export function createWorkspaceDescribeFetcher( + warehouseId: string, +): DescribeFetcher { + const client = new WorkspaceClient({}); + return async (fqn: string) => { + const result = (await client.statementExecution.executeStatement({ + statement: `DESCRIBE TABLE EXTENDED ${fqn} AS JSON`, + warehouse_id: warehouseId, + wait_timeout: "30s", + })) as DatabricksStatementExecutionResponse; + return result; + }; +} + +/** + * One per-entry sync failure recorded by {@link syncMetrics}. Failures are + * surfaced to the caller (CLI / Vite plugin) so they can decide whether to + * exit non-zero. Without this, a silently-empty bundle would ship to + * production and the route's runtime fail-closed gate would 503 every + * affected metric. + */ +export interface MetricSyncFailure { + /** Stable metric key — matches the key under `metricViews` in metric-views.json. */ + key: string; + /** Three-part FQN that failed to resolve. */ + source: string; + /** Single human-readable reason (DESCRIBE failed, parse failed, zero columns). */ + reason: string; +} + +/** + * Result shape from {@link syncMetrics}: the schemas (one per entry, possibly + * empty if the entry failed) plus a list of per-entry failures so the caller + * can emit a non-zero exit / build error when something didn't resolve. + */ +export interface MetricSyncResult { + schemas: MetricSchema[]; + failures: MetricSyncFailure[]; +} + +/** + * Run schema synchronization for every entry in `metric-views.json`. + * + * `fetcher` is injected so the same code path serves Vite, the CLI, and unit + * tests with a mock that returns a representative DESCRIBE response. + * + * Returns `{ schemas, failures }`. The schemas array always carries one + * entry per registered metric (even on failure — the entry has empty + * measures/dimensions). The failures array is populated for any entry that + * (a) the DESCRIBE call rejected, (b) the response could not be parsed, or + * (c) extraction yielded zero columns. Callers (the CLI, the Vite plugin) + * inspect `failures` to decide whether to exit non-zero. + */ +export async function syncMetrics( + resolution: MetricConfigResolution, + fetcher: DescribeFetcher, +): Promise { + const schemas: MetricSchema[] = []; + const failures: MetricSyncFailure[] = []; + + for (const entry of resolution.entries) { + let response: DatabricksStatementExecutionResponse; + try { + response = await fetcher(entry.source); + } catch (err) { + const reason = `DESCRIBE TABLE EXTENDED failed: ${(err as Error).message}`; + logger.warn("%s for %s", reason, entry.source); + failures.push({ key: entry.key, source: entry.source, reason }); + schemas.push({ + key: entry.key, + source: entry.source, + lane: entry.lane, + measures: [], + dimensions: [], + }); + continue; + } + + let columns: MetricColumnMetadata[] = []; + let parseError: string | null = null; + try { + const parsed = parseDescribeTableExtendedJson(response); + columns = extractMetricColumns(parsed); + } catch (err) { + parseError = `Failed to extract columns from DESCRIBE response: ${(err as Error).message}`; + logger.warn("%s for %s", parseError, entry.source); + } + + const measures = columns.filter((c) => c.isMeasure); + const dimensions = columns.filter((c) => !c.isMeasure); + + if (parseError) { + failures.push({ + key: entry.key, + source: entry.source, + reason: parseError, + }); + } else if (columns.length === 0) { + // Extraction succeeded but yielded no columns. The most common cause + // is a DESCRIBE response shape that `extractMetricColumns` doesn't + // recognize. Treat as a failure so CI catches it instead of letting an + // empty bundle entry ship — the route's fail-closed gate would then + // 503 every request to this metric in production. + const reason = + "DESCRIBE response yielded zero columns — check the response shape (top-level `columns` array or `schema.fields`)."; + logger.warn("%s for %s", reason, entry.source); + failures.push({ key: entry.key, source: entry.source, reason }); + } + + schemas.push({ + key: entry.key, + source: entry.source, + lane: entry.lane, + measures, + dimensions, + }); + } + + return { schemas, failures }; +} diff --git a/packages/appkit/src/type-generator/tests/__snapshots__/metric-registry.test.ts.snap b/packages/appkit/src/type-generator/tests/__snapshots__/metric-registry.test.ts.snap new file mode 100644 index 00000000..71b34e28 --- /dev/null +++ b/packages/appkit/src/type-generator/tests/__snapshots__/metric-registry.test.ts.snap @@ -0,0 +1,207 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`generateMetricTypeDeclarations — snapshot > emits TimeGrain union for a metric view with time-typed + regular dimensions 1`] = ` +"// Auto-generated by AppKit - DO NOT EDIT +// Generated by 'npx @databricks/appkit generate-types' or Vite plugin during build +import "@databricks/appkit-ui/react"; +declare module "@databricks/appkit-ui/react" { + interface MetricRegistry { + "revenue": { + key: "revenue"; + source: "appkit_demo.public.revenue_metrics_v2"; + lane: "sp"; + measures: { + /** @sqlType DECIMAL(38,2) */ + "arr": number; + }; + dimensions: { + /** @sqlType TIMESTAMP @timeGrain day|hour|minute|month|quarter|week|year */ + "created_at": string; + /** @sqlType STRING */ + "region": string; + /** @sqlType STRING */ + "segment": string; + }; + measureKeys: "arr"; + dimensionKeys: "created_at" | "region" | "segment"; + timeGrains: "day" | "hour" | "minute" | "month" | "quarter" | "week" | "year"; + metadata: { + measures: { + "arr": { + type: "DECIMAL(38,2)"; + description: "Annual recurring revenue"; + }; + }; + dimensions: { + "created_at": { + type: "TIMESTAMP"; + time_grain: readonly ["day", "hour", "minute", "month", "quarter", "week", "year"]; + }; + "region": { + type: "STRING"; + }; + "segment": { + type: "STRING"; + }; + }; + }; + }; + } +} +" +`; + +exports[`generateMetricTypeDeclarations — snapshot > emits a stable MetricRegistry augmentation for a mixed sp + obo input 1`] = ` +"// Auto-generated by AppKit - DO NOT EDIT +// Generated by 'npx @databricks/appkit generate-types' or Vite plugin during build +import "@databricks/appkit-ui/react"; +declare module "@databricks/appkit-ui/react" { + interface MetricRegistry { + "customer_metrics": { + key: "customer_metrics"; + source: "appkit_demo.public.customer_metrics"; + lane: "obo"; + measures: { + /** @sqlType DOUBLE */ + "churn_rate": number; + }; + dimensions: { + /** @sqlType STRING */ + "csm_email": string; + /** @sqlType DATE @timeGrain day|month|quarter|week|year */ + "billing_date": string; + }; + measureKeys: "churn_rate"; + dimensionKeys: "csm_email" | "billing_date"; + timeGrains: "day" | "month" | "quarter" | "week" | "year"; + metadata: { + measures: { + "churn_rate": { + type: "DOUBLE"; + display_name: "Churn Rate"; + format: "0.0%"; + }; + }; + dimensions: { + "csm_email": { + type: "STRING"; + }; + "billing_date": { + type: "DATE"; + time_grain: readonly ["day", "month", "quarter", "week", "year"]; + }; + }; + }; + }; + "revenue": { + key: "revenue"; + source: "appkit_demo.public.revenue_metrics"; + lane: "sp"; + measures: { + /** @sqlType DECIMAL(38,2) */ + "arr": number; + /** @sqlType DECIMAL(38,2) */ + "mrr": number; + }; + dimensions: { + /** @sqlType STRING */ + "region": string; + /** @sqlType TIMESTAMP @timeGrain day|hour|minute|month|quarter|week|year */ + "created_at": string; + }; + measureKeys: "arr" | "mrr"; + dimensionKeys: "region" | "created_at"; + timeGrains: "day" | "hour" | "minute" | "month" | "quarter" | "week" | "year"; + metadata: { + measures: { + "arr": { + type: "DECIMAL(38,2)"; + display_name: "Annual Recurring Revenue"; + format: "$#,##0.00"; + description: "Annual recurring revenue"; + }; + "mrr": { + type: "DECIMAL(38,2)"; + description: "Monthly recurring revenue"; + }; + }; + dimensions: { + "region": { + type: "STRING"; + }; + "created_at": { + type: "TIMESTAMP"; + time_grain: readonly ["day", "hour", "minute", "month", "quarter", "week", "year"]; + }; + }; + }; + }; + } +} +" +`; + +exports[`generateMetricTypeDeclarations — snapshot > emits an empty MetricRegistry interface when no metrics are registered 1`] = ` +"// Auto-generated by AppKit - DO NOT EDIT +// Generated by 'npx @databricks/appkit generate-types' or Vite plugin during build +import "@databricks/appkit-ui/react"; +declare module "@databricks/appkit-ui/react" { + interface MetricRegistry {} +} +" +`; + +exports[`generateMetricsMetadataJson — snapshot > serializes a representative metric view with display_name + format + time_grain 1`] = ` +"{ + "customer_metrics": { + "measures": { + "churn_rate": { + "type": "DOUBLE", + "display_name": "Churn Rate", + "format": "0.0%" + } + }, + "dimensions": { + "csm_email": { + "type": "STRING", + "display_name": "CSM Email" + } + } + }, + "revenue": { + "measures": { + "arr": { + "type": "DECIMAL(38,2)", + "display_name": "Annual Recurring Revenue", + "format": "$#,##0.00", + "description": "ARR per quarter" + }, + "growth_rate": { + "type": "DOUBLE", + "display_name": "Growth Rate", + "format": "0.0%" + } + }, + "dimensions": { + "region": { + "type": "STRING", + "display_name": "Region" + }, + "created_at": { + "type": "TIMESTAMP", + "display_name": "Period", + "time_grain": [ + "day", + "hour", + "minute", + "month", + "quarter", + "week", + "year" + ] + } + } + } +} +" +`; diff --git a/packages/appkit/src/type-generator/tests/index.test.ts b/packages/appkit/src/type-generator/tests/index.test.ts index 4c37699a..d37525aa 100644 --- a/packages/appkit/src/type-generator/tests/index.test.ts +++ b/packages/appkit/src/type-generator/tests/index.test.ts @@ -9,6 +9,7 @@ import { test, vi, } from "vitest"; +import type { DatabricksStatementExecutionResponse } from "../types"; const mocks = vi.hoisted(() => ({ generateQueriesFromDescribe: vi.fn(), @@ -198,3 +199,138 @@ describe("generateFromEntryPoint — query failure handling", () => { expect(fs.readFileSync(outFile, "utf-8")).toContain("bad_auth"); }); }); + +describe("generateFromEntryPoint — metric-view emission", () => { + const metricsDir = path.join(__dirname, "__output_metrics__"); + const queryFolder = path.join(metricsDir, "queries"); + const outFile = path.join(metricsDir, "generated", "analytics.d.ts"); + // Defaults: metric artifacts are siblings of `outFile`. + const metricFile = path.join(metricsDir, "generated", "metric.d.ts"); + const metadataFile = path.join( + metricsDir, + "generated", + "metrics.metadata.json", + ); + + const describeResponse: DatabricksStatementExecutionResponse = { + statement_id: "stmt-mock", + status: { state: "SUCCEEDED" }, + result: { + data_array: [ + [ + JSON.stringify({ + columns: [ + { + name: "total_revenue", + type: "DECIMAL(38,2)", + is_measure: true, + }, + { name: "region", type: "STRING", is_measure: false }, + ], + }), + ], + ], + }, + }; + + const writeMetricConfig = () => { + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + JSON.stringify({ + metricViews: { revenue: { source: "demo.sales.revenue" } }, + }), + ); + }; + + beforeEach(() => { + vi.clearAllMocks(); + fs.rmSync(metricsDir, { recursive: true, force: true }); + fs.mkdirSync(queryFolder, { recursive: true }); + mocks.generateQueriesFromDescribe.mockResolvedValue({ + schemas: [], + syntaxErrors: [], + fatalErrors: [], + }); + }); + + afterAll(() => { + fs.rmSync(metricsDir, { recursive: true, force: true }); + }); + + test("writes metric.d.ts and metrics.metadata.json when metric-views.json exists", async () => { + writeMetricConfig(); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + metricFetcher: async () => describeResponse, + }), + ).resolves.toBeUndefined(); + + const declarations = fs.readFileSync(metricFile, "utf-8"); + expect(declarations).toContain("interface MetricRegistry"); + expect(declarations).toContain('"revenue"'); + expect(declarations).toContain('"total_revenue": number'); + expect(declarations).toContain('"region": string'); + + const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + expect(bundle.revenue.measures.total_revenue.type).toBe("DECIMAL(38,2)"); + expect(bundle.revenue.dimensions.region.type).toBe("STRING"); + }); + + test("emits no metric artifacts and no errors when metric-views.json is absent", async () => { + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + }), + ).resolves.toBeUndefined(); + + // Query types are still written; the metric path stays fully dormant. + expect(fs.existsSync(outFile)).toBe(true); + expect(fs.existsSync(metricFile)).toBe(false); + expect(fs.existsSync(metadataFile)).toBe(false); + }); + + test("a failing metric fetcher warns but query type generation still succeeds", async () => { + writeMetricConfig(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + try { + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + metricFetcher: async () => { + throw new Error("DESCRIBE exploded"); + }, + }), + ).resolves.toBeUndefined(); + + // The failure is surfaced per key (key/source/reason) ... + const warned = warnSpy.mock.calls.flat().map(String).join("\n"); + expect(warned).toContain( + "metric sync failed for revenue (demo.sales.revenue)", + ); + expect(warned).toContain("DESCRIBE exploded"); + } finally { + warnSpy.mockRestore(); + } + + // ... query types are unaffected by the metric failure ... + expect(fs.existsSync(outFile)).toBe(true); + expect(fs.readFileSync(outFile, "utf-8")).toContain( + "interface QueryRegistry", + ); + + // ... and both artifacts still ship, with the failed key carrying an + // empty schema rather than poisoning the build. + expect(fs.readFileSync(metricFile, "utf-8")).toContain('"revenue"'); + const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); + }); +}); diff --git a/packages/appkit/src/type-generator/tests/metric-registry.test.ts b/packages/appkit/src/type-generator/tests/metric-registry.test.ts new file mode 100644 index 00000000..48b042a6 --- /dev/null +++ b/packages/appkit/src/type-generator/tests/metric-registry.test.ts @@ -0,0 +1,1194 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { metricSourceSchema } from "../../../../shared/src/schemas/metric-source"; +import { + buildMetricsMetadataBundle, + extractMetricColumns, + generateMetricsMetadataJson, + generateMetricTypeDeclarations, + parseDescribeTableExtendedJson, + readMetricConfig, + resolveMetricConfig, + syncMetrics, +} from "../metric-registry"; +import type { DatabricksStatementExecutionResponse } from "../types"; + +/** + * Build a representative DESCRIBE TABLE EXTENDED ... AS JSON response. + * + * The Statement Execution API returns one row, one cell — a JSON string + * payload. The shape is broadly: + * + * ```json + * { + * "table_name": "...", + * "columns": [ + * { "name": "arr", "type": "DECIMAL(38,2)", "is_measure": true, "comment": "..." }, + * { "name": "region", "type": "STRING", "is_measure": false } + * ] + * } + * ``` + * + * Phase 1 mocks this. Live integration ships in Phase 7. + */ +function mockDescribeResponse( + payload: unknown, +): DatabricksStatementExecutionResponse { + return { + statement_id: "stmt-mock", + status: { state: "SUCCEEDED" }, + result: { + data_array: [[JSON.stringify(payload)]], + }, + }; +} + +/** + * Cast helper for fixtures that intentionally violate the config type + * (invalid executors, unknown fields, legacy shapes, ...). + */ +const resolveUnchecked = (config: unknown) => + resolveMetricConfig(config as Parameters[0]); + +describe("readMetricConfig", () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "appkit-metric-typegen-")); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + test("returns null when metric-views.json is absent", async () => { + expect(await readMetricConfig(tmpDir)).toBeNull(); + }); + + test("ignores a legacy metric.json file (no fallback)", async () => { + await fs.writeFile( + path.join(tmpDir, "metric.json"), + JSON.stringify({ + metricViews: { revenue: { source: "demo.public.revenue" } }, + }), + ); + expect(await readMetricConfig(tmpDir)).toBeNull(); + }); + + test("parses a valid metric-views.json", async () => { + await fs.writeFile( + path.join(tmpDir, "metric-views.json"), + JSON.stringify({ + metricViews: { revenue: { source: "demo.public.revenue" } }, + }), + ); + const cfg = await readMetricConfig(tmpDir); + expect(cfg?.metricViews?.revenue.source).toBe("demo.public.revenue"); + }); + + test("throws on malformed JSON", async () => { + await fs.writeFile(path.join(tmpDir, "metric-views.json"), "{not json"); + await expect(readMetricConfig(tmpDir)).rejects.toThrowError( + /parse metric-views\.json/, + ); + }); +}); + +describe("resolveMetricConfig", () => { + test("flattens metricViews into a sorted entries list with derived lanes", () => { + const cfg = { + metricViews: { + b_metric: { source: "a.b.c" }, + a_metric: { source: "a.b.d" }, + c_metric: { source: "a.b.e", executor: "user" as const }, + }, + }; + const { entries } = resolveMetricConfig(cfg); + expect(entries.map((e) => e.key)).toEqual([ + "a_metric", + "b_metric", + "c_metric", + ]); + expect(entries[0].lane).toBe("sp"); + expect(entries[1].lane).toBe("sp"); + expect(entries[2].lane).toBe("obo"); + }); + + test("defaults an absent executor to the sp lane", () => { + const { entries } = resolveMetricConfig({ + metricViews: { revenue: { source: "a.b.c" } }, + }); + expect(entries).toEqual([{ key: "revenue", source: "a.b.c", lane: "sp" }]); + }); + + test("maps executor 'app_service_principal' to the sp lane", () => { + const { entries } = resolveMetricConfig({ + metricViews: { + revenue: { source: "a.b.c", executor: "app_service_principal" }, + }, + }); + expect(entries[0].lane).toBe("sp"); + }); + + test("maps executor 'user' to the obo lane", () => { + const { entries } = resolveMetricConfig({ + metricViews: { revenue: { source: "a.b.c", executor: "user" } }, + }); + expect(entries[0].lane).toBe("obo"); + }); + + test("rejects invalid executor values", () => { + for (const executor of ["sp", "obo", "service_principal", "USER", null]) { + expect(() => + resolveUnchecked({ + metricViews: { revenue: { source: "a.b.c", executor } }, + }), + ).toThrowError(/Invalid executor/); + } + }); + + test("rejects unknown entry fields", () => { + expect(() => + resolveUnchecked({ + metricViews: { revenue: { source: "a.b.c", cacheTtl: 60 } }, + }), + ).toThrowError(/'source' and 'executor' are allowed/); + }); + + test("rejects unknown top-level fields (legacy sp/obo lane shape)", () => { + expect(() => + resolveUnchecked({ sp: { revenue: { source: "a.b.c" } } }), + ).toThrowError(/Invalid top-level field "sp"/); + expect(() => + resolveUnchecked({ metricViews: {}, unknown: {} }), + ).toThrowError(/Invalid top-level field "unknown"/); + }); + + test("accepts $schema at the top level", () => { + const { entries } = resolveMetricConfig({ + $schema: + "https://databricks.github.io/appkit/schemas/metric-source.schema.json", + metricViews: { revenue: { source: "a.b.c" } }, + }); + expect(entries).toHaveLength(1); + }); + + test("resolves to no entries when metricViews is absent", () => { + expect(resolveMetricConfig({}).entries).toEqual([]); + }); + + test("resolves to no entries when metricViews is empty", () => { + expect(resolveMetricConfig({ metricViews: {} }).entries).toEqual([]); + }); + + test("rejects a non-object entry", () => { + expect(() => + resolveUnchecked({ metricViews: { revenue: "a.b.c" } }), + ).toThrowError(/expected an object/); + }); + + test("rejects bad FQN format", () => { + const cfg = { + metricViews: { revenue: { source: "not.three.part.parts" } }, + }; + expect(() => resolveMetricConfig(cfg)).toThrowError(/three-part UC FQN/); + }); + + test("rejects a metric key starting with a digit", () => { + const cfg = { metricViews: { "1revenue": { source: "a.b.c" } } }; + expect(() => resolveMetricConfig(cfg)).toThrowError(/Invalid metric key/); + }); +}); + +// ── Parity: the inline config validation must agree with the canonical +// shared Zod schema (packages/shared/src/schemas/metric-source.ts). +// The regexes and allowlists are copied, not imported (locked dependency-graph +// ruling: the type-generator must not pull the shared schema package into the +// runtime path) — this block is the drift alarm. TEST-ONLY import. +describe("resolveMetricConfig — parity with shared metricSourceSchema", () => { + const accepts: Array<{ name: string; config: Record }> = [ + { + name: "single entry with default executor", + config: { metricViews: { revenue: { source: "demo.public.revenue" } } }, + }, + { + name: "explicit app_service_principal executor", + config: { + metricViews: { + revenue: { + source: "demo.public.revenue", + executor: "app_service_principal", + }, + }, + }, + }, + { + name: "user executor", + config: { + metricViews: { + customer_metrics: { + source: "appkit_demo.public.customer_metrics", + executor: "user", + }, + }, + }, + }, + { + name: "$schema plus multiple entries", + config: { + $schema: + "https://databricks.github.io/appkit/schemas/metric-source.schema.json", + metricViews: { + a_metric: { source: "a.b.c" }, + b_metric: { source: "a.b.d", executor: "user" }, + }, + }, + }, + { name: "empty config (no metricViews)", config: {} }, + { name: "empty metricViews map", config: { metricViews: {} } }, + ]; + + const rejects: Array<{ name: string; config: Record }> = [ + { + name: "metric key starting with a digit", + config: { metricViews: { "1revenue": { source: "a.b.c" } } }, + }, + { + name: "non-three-part FQN", + config: { metricViews: { revenue: { source: "not.three.part.parts" } } }, + }, + { + name: "invalid executor value", + config: { + metricViews: { revenue: { source: "a.b.c", executor: "admin" } }, + }, + }, + { + name: "unknown entry field", + config: { + metricViews: { revenue: { source: "a.b.c", cacheTtl: 60 } }, + }, + }, + { + name: "unknown top-level field (legacy sp lane shape)", + config: { sp: { revenue: { source: "a.b.c" } } }, + }, + { + name: "non-object entry", + config: { metricViews: { revenue: "a.b.c" } }, + }, + ]; + + for (const fixture of accepts) { + test(`both accept: ${fixture.name}`, () => { + expect(metricSourceSchema.safeParse(fixture.config).success).toBe(true); + expect(() => resolveUnchecked(fixture.config)).not.toThrow(); + }); + } + + for (const fixture of rejects) { + test(`both reject: ${fixture.name}`, () => { + expect(metricSourceSchema.safeParse(fixture.config).success).toBe(false); + expect(() => resolveUnchecked(fixture.config)).toThrow(); + }); + } +}); + +describe("parseDescribeTableExtendedJson", () => { + test("parses the JSON payload from the first cell", () => { + const response = mockDescribeResponse({ + columns: [{ name: "arr", type: "DECIMAL", is_measure: true }], + }); + const parsed = parseDescribeTableExtendedJson(response); + expect(parsed).toMatchObject({ + columns: [{ name: "arr", type: "DECIMAL", is_measure: true }], + }); + }); + + test("throws on a FAILED status", () => { + expect(() => + parseDescribeTableExtendedJson({ + statement_id: "x", + status: { state: "FAILED", error: { message: "no such table" } }, + }), + ).toThrowError(/no such table/); + }); + + test("throws when the response is empty", () => { + expect(() => + parseDescribeTableExtendedJson({ + statement_id: "x", + status: { state: "SUCCEEDED" }, + result: { data_array: [] }, + }), + ).toThrowError(/no rows/); + }); + + test("throws when the cell is not a JSON string", () => { + expect(() => + parseDescribeTableExtendedJson({ + statement_id: "x", + status: { state: "SUCCEEDED" }, + result: { data_array: [[null]] }, + }), + ).toThrowError(/JSON string/); + }); +}); + +describe("extractMetricColumns", () => { + test("extracts measures and dimensions from the standard shape", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "arr", + type: "DECIMAL(38,2)", + is_measure: true, + comment: "Annual recurring revenue", + }, + { name: "region", type: "STRING", is_measure: false }, + ], + }); + expect(cols).toHaveLength(2); + expect(cols[0]).toMatchObject({ + name: "arr", + type: "DECIMAL(38,2)", + isMeasure: true, + description: "Annual recurring revenue", + }); + expect(cols[1]).toMatchObject({ + name: "region", + type: "STRING", + isMeasure: false, + }); + }); + + test("falls back to schema.fields shape", () => { + const cols = extractMetricColumns({ + schema: { + fields: [ + { + name: "mrr", + type: { name: "DOUBLE" }, + metadata: { is_measure: true }, + }, + ], + }, + }); + expect(cols).toHaveLength(1); + expect(cols[0]).toMatchObject({ + name: "mrr", + type: "DOUBLE", + isMeasure: true, + }); + }); + + test("returns empty array on unrecognized shape", () => { + expect(extractMetricColumns({ unrelated: true })).toEqual([]); + }); + + // ── Phase 2: time-typed dimensions ──────────────────────────────────── + test("infers all 7 standard grains for a TIMESTAMP dimension", () => { + const cols = extractMetricColumns({ + columns: [ + { name: "created_at", type: "TIMESTAMP", is_measure: false }, + { name: "region", type: "STRING", is_measure: false }, + ], + }); + expect(cols).toHaveLength(2); + expect(cols[0]).toMatchObject({ + name: "created_at", + isMeasure: false, + timeGrains: ["day", "hour", "minute", "month", "quarter", "week", "year"], + }); + // Non-time dim has no timeGrains key. + expect(cols[1].timeGrains).toBeUndefined(); + }); + + test("infers 5 standard grains (no sub-day) for a DATE dimension", () => { + const cols = extractMetricColumns({ + columns: [{ name: "billing_date", type: "DATE", is_measure: false }], + }); + expect(cols[0].timeGrains).toEqual([ + "day", + "month", + "quarter", + "week", + "year", + ]); + }); + + test("recognizes TIMESTAMP_LTZ and TIMESTAMP_NTZ aliases", () => { + const cols = extractMetricColumns({ + columns: [ + { name: "ts_ltz", type: "TIMESTAMP_LTZ", is_measure: false }, + { name: "ts_ntz", type: "TIMESTAMP_NTZ", is_measure: false }, + ], + }); + expect(cols[0].timeGrains).toEqual([ + "day", + "hour", + "minute", + "month", + "quarter", + "week", + "year", + ]); + expect(cols[1].timeGrains).toEqual([ + "day", + "hour", + "minute", + "month", + "quarter", + "week", + "year", + ]); + }); + + test("type matching is case-insensitive", () => { + const cols = extractMetricColumns({ + columns: [ + { name: "a", type: "timestamp", is_measure: false }, + { name: "b", type: "Timestamp", is_measure: false }, + { name: "c", type: "DATE", is_measure: false }, + { name: "d", type: "date", is_measure: false }, + ], + }); + expect(cols[0].timeGrains?.length).toBe(7); + expect(cols[1].timeGrains?.length).toBe(7); + expect(cols[2].timeGrains?.length).toBe(5); + expect(cols[3].timeGrains?.length).toBe(5); + }); + + test("strips parameterized type suffixes like TIMESTAMP(6)", () => { + const cols = extractMetricColumns({ + columns: [{ name: "ts", type: "TIMESTAMP(6)", is_measure: false }], + }); + expect(cols[0].timeGrains?.length).toBe(7); + }); + + test("does not infer grains for non-temporal types", () => { + const cols = extractMetricColumns({ + columns: [ + { name: "id", type: "BIGINT", is_measure: false }, + { name: "name", type: "STRING", is_measure: false }, + { name: "amount", type: "DECIMAL(38,2)", is_measure: false }, + ], + }); + for (const col of cols) { + expect(col.timeGrains).toBeUndefined(); + } + }); + + test("does not infer grains on measures even if their type is TIMESTAMP", () => { + // Measures are aggregated, never grouped on — grain inference is + // dimension-only. Defends against an unusual MEASURE() expression + // resolving to a temporal type. + const cols = extractMetricColumns({ + columns: [{ name: "last_event_at", type: "TIMESTAMP", is_measure: true }], + }); + expect(cols[0].timeGrains).toBeUndefined(); + }); +}); + +describe("syncMetrics", () => { + test("returns one schema per resolved entry, columns split by measure flag", async () => { + const resolution = resolveMetricConfig({ + metricViews: { revenue: { source: "demo.public.revenue" } }, + }); + + const fetcher = async () => + mockDescribeResponse({ + columns: [ + { name: "arr", type: "DECIMAL(38,2)", is_measure: true }, + { name: "mrr", type: "DECIMAL(38,2)", is_measure: true }, + { name: "region", type: "STRING", is_measure: false }, + ], + }); + + const { schemas } = await syncMetrics(resolution, fetcher); + expect(schemas).toHaveLength(1); + const [schema] = schemas; + expect(schema.key).toBe("revenue"); + expect(schema.measures.map((m) => m.name)).toEqual(["arr", "mrr"]); + expect(schema.dimensions.map((d) => d.name)).toEqual(["region"]); + }); + + test("falls back to empty columns when DESCRIBE throws (does not crash typegen)", async () => { + const resolution = resolveMetricConfig({ + metricViews: { revenue: { source: "demo.public.revenue" } }, + }); + + const fetcher = async () => { + throw new Error("warehouse unreachable"); + }; + + const { schemas, failures } = await syncMetrics(resolution, fetcher); + expect(schemas[0].measures).toEqual([]); + expect(schemas[0].dimensions).toEqual([]); + expect(failures).toHaveLength(1); + expect(failures[0]).toMatchObject({ + key: "revenue", + source: "demo.public.revenue", + }); + expect(failures[0].reason).toMatch(/warehouse unreachable/); + }); +}); + +describe("generateMetricTypeDeclarations — snapshot", () => { + test("emits a stable MetricRegistry augmentation for a mixed sp + obo input", async () => { + const resolution = resolveMetricConfig({ + metricViews: { + revenue: { source: "appkit_demo.public.revenue_metrics" }, + customer_metrics: { + source: "appkit_demo.public.customer_metrics", + executor: "user", + }, + }, + }); + + const fetcher = async (fqn: string) => + fqn.endsWith("revenue_metrics") + ? mockDescribeResponse({ + columns: [ + { + name: "arr", + type: "DECIMAL(38,2)", + is_measure: true, + display_name: "Annual Recurring Revenue", + format: "$#,##0.00", + comment: "Annual recurring revenue", + }, + { + name: "mrr", + type: "DECIMAL(38,2)", + is_measure: true, + comment: "Monthly recurring revenue", + }, + { name: "region", type: "STRING", is_measure: false }, + { name: "created_at", type: "TIMESTAMP", is_measure: false }, + ], + }) + : mockDescribeResponse({ + columns: [ + { + name: "churn_rate", + type: "DOUBLE", + is_measure: true, + display_name: "Churn Rate", + format: "0.0%", + }, + { name: "csm_email", type: "STRING", is_measure: false }, + { name: "billing_date", type: "DATE", is_measure: false }, + ], + }); + + const { schemas } = await syncMetrics(resolution, fetcher); + const output = generateMetricTypeDeclarations(schemas); + expect(output).toMatchSnapshot(); + + // Sanity assertions in addition to the snapshot, so future drift surfaces + // even when snapshots are blindly updated. The executor→lane derivation + // must hold end-to-end: default → sp, "user" → obo. + expect(output).toContain('lane: "sp"'); + expect(output).toContain('lane: "obo"'); + expect(output).toContain('format: "$#,##0.00"'); + expect(output).toContain('format: "0.0%"'); + }); + + test("emits an empty MetricRegistry interface when no metrics are registered", () => { + const output = generateMetricTypeDeclarations([]); + expect(output).toMatchSnapshot(); + }); + + // ── Phase 2: time-typed dim + multiple non-time dims fixture ───────── + test("emits TimeGrain union for a metric view with time-typed + regular dimensions", async () => { + const resolution = resolveMetricConfig({ + metricViews: { + revenue: { source: "appkit_demo.public.revenue_metrics_v2" }, + }, + }); + + const fetcher = async () => + mockDescribeResponse({ + columns: [ + { + name: "arr", + type: "DECIMAL(38,2)", + is_measure: true, + comment: "Annual recurring revenue", + }, + { name: "created_at", type: "TIMESTAMP", is_measure: false }, + { name: "region", type: "STRING", is_measure: false }, + { name: "segment", type: "STRING", is_measure: false }, + ], + }); + + const { schemas } = await syncMetrics(resolution, fetcher); + const output = generateMetricTypeDeclarations(schemas); + expect(output).toMatchSnapshot(); + + // Sanity assertions in addition to the snapshot, so future drift surfaces + // even when snapshots are blindly updated. TIMESTAMP → all 7 standard grains. + expect(output).toContain( + 'timeGrains: "day" | "hour" | "minute" | "month" | "quarter" | "week" | "year"', + ); + expect(output).toContain( + "@timeGrain day|hour|minute|month|quarter|week|year", + ); + expect(output).toContain('"created_at": string'); + expect(output).toContain('"region": string'); + }); +}); + +// ── Phase 5: semantic-metadata extraction (display_name + format) ───────── +describe("extractMetricColumns — Phase 5 semantic metadata", () => { + test("captures display_name from a measure column", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "arr", + type: "DECIMAL(38,2)", + is_measure: true, + display_name: "Annual Recurring Revenue", + comment: "ARR for the period", + }, + ], + }); + expect(cols[0]).toMatchObject({ + name: "arr", + type: "DECIMAL(38,2)", + isMeasure: true, + displayName: "Annual Recurring Revenue", + description: "ARR for the period", + }); + }); + + test("captures format spec from a measure column", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "arr", + type: "DECIMAL(38,2)", + is_measure: true, + format: "$#,##0.00", + }, + ], + }); + expect(cols[0].format).toBe("$#,##0.00"); + }); + + test("captures display_name + format on a dimension column", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "region", + type: "STRING", + is_measure: false, + display_name: "Region", + format: undefined, + }, + ], + }); + expect(cols[0]).toMatchObject({ + name: "region", + isMeasure: false, + displayName: "Region", + }); + expect(cols[0].format).toBeUndefined(); + }); + + test("falls back to displayName camelCase variant", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "mrr", + type: "DECIMAL", + is_measure: true, + displayName: "Monthly Recurring Revenue", + }, + ], + }); + expect(cols[0].displayName).toBe("Monthly Recurring Revenue"); + }); + + test("reads display_name + format from metadata. (DESCRIBE wrap)", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "arr", + type: "DECIMAL(38,2)", + metadata: { + is_measure: true, + display_name: "ARR", + format: "$#,##0.00", + }, + }, + ], + }); + expect(cols[0]).toMatchObject({ + isMeasure: true, + displayName: "ARR", + format: "$#,##0.00", + }); + }); + + test("treats empty / whitespace display_name as absent", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "arr", + type: "DECIMAL", + is_measure: true, + display_name: " ", + format: "", + }, + ], + }); + expect(cols[0].displayName).toBeUndefined(); + expect(cols[0].format).toBeUndefined(); + }); + + test("captures format from format_spec alias", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "arr", + type: "DECIMAL", + is_measure: true, + format_spec: "$#,##0.00", + }, + ], + }); + expect(cols[0].format).toBe("$#,##0.00"); + }); + + // ── Structured-format translation (UC YAML 1.1 → printf string) ──────── + // DESCRIBE TABLE EXTENDED ... AS JSON wraps the format type as the outer + // key: { currency: { ... } } / { percent: { ... } } / { number: { ... } }. + // The extractor translates these into printf strings consumable by + // formatValue / toD3Format. + test("translates structured currency format with USD", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "arr", + type: "DOUBLE", + is_measure: true, + metadata: { + format: { + currency: { + decimal_places: { type: "EXACT", places: 2 }, + currency_code: "USD", + }, + }, + }, + }, + ], + }); + expect(cols[0].format).toBe("$#,##0.00"); + }); + + test("translates structured currency format with EUR + 0 decimal places", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "ticket_price", + type: "DOUBLE", + is_measure: true, + metadata: { + format: { + currency: { + decimal_places: { places: 0 }, + currency_code: "EUR", + }, + }, + }, + }, + ], + }); + expect(cols[0].format).toBe("€#,##0"); + }); + + test("falls back to ISO code as literal prefix for unknown currencies", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "amount", + type: "DOUBLE", + is_measure: true, + metadata: { + format: { + currency: { + decimal_places: { places: 2 }, + currency_code: "AUD", + }, + }, + }, + }, + ], + }); + expect(cols[0].format).toBe("AUD #,##0.00"); + }); + + test("translates structured percent format with 1 decimal place", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "churn_rate", + type: "DECIMAL", + is_measure: true, + metadata: { + format: { + percent: { decimal_places: { places: 1 } }, + }, + }, + }, + ], + }); + expect(cols[0].format).toBe("0.0%"); + }); + + test("translates structured percent with 0 decimal places", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "rate", + type: "DECIMAL", + is_measure: true, + metadata: { format: { percent: { decimal_places: { places: 0 } } } }, + }, + ], + }); + expect(cols[0].format).toBe("0%"); + }); + + test("translates structured number format with comma grouping", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "active_accounts", + type: "BIGINT", + is_measure: true, + metadata: { + format: { number: { decimal_places: { places: 0 } } }, + }, + }, + ], + }); + expect(cols[0].format).toBe("#,##0"); + }); + + test("returns undefined for unrecognized structured format shapes", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "weirdo", + type: "DOUBLE", + is_measure: true, + metadata: { + format: { custom_thing: { whatever: 1 } }, + }, + }, + ], + }); + expect(cols[0].format).toBeUndefined(); + }); + + test("currency format defaults to USD + 2 places when fields are missing", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "amount", + type: "DOUBLE", + is_measure: true, + metadata: { format: { currency: {} } }, + }, + ], + }); + expect(cols[0].format).toBe("$#,##0.00"); + }); + + test("accepts decimal_places as a bare number (legacy shape)", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "amount", + type: "DOUBLE", + is_measure: true, + metadata: { + format: { currency: { decimal_places: 4, currency_code: "USD" } }, + }, + }, + ], + }); + expect(cols[0].format).toBe("$#,##0.0000"); + }); +}); + +// ── Phase 5: metadata bundle generation ─────────────────────────────────── +describe("buildMetricsMetadataBundle", () => { + test("emits per-metric measures + dimensions records keyed by name", async () => { + const resolution = resolveMetricConfig({ + metricViews: { + revenue: { source: "appkit_demo.public.revenue_metrics" }, + }, + }); + + const fetcher = async () => + mockDescribeResponse({ + columns: [ + { + name: "arr", + type: "DECIMAL(38,2)", + is_measure: true, + display_name: "Annual Recurring Revenue", + format: "$#,##0.00", + comment: "ARR for the period", + }, + { name: "region", type: "STRING", is_measure: false }, + { name: "created_at", type: "TIMESTAMP", is_measure: false }, + ], + }); + + const { schemas } = await syncMetrics(resolution, fetcher); + const bundle = buildMetricsMetadataBundle(schemas); + + expect(bundle.revenue).toMatchObject({ + measures: { + arr: { + type: "DECIMAL(38,2)", + display_name: "Annual Recurring Revenue", + format: "$#,##0.00", + description: "ARR for the period", + }, + }, + dimensions: { + region: { + type: "STRING", + }, + created_at: { + type: "TIMESTAMP", + time_grain: [ + "day", + "hour", + "minute", + "month", + "quarter", + "week", + "year", + ], + }, + }, + }); + // Defense-in-depth: the client-shipped bundle must not carry server-side + // concerns (UC FQN, execution lane). They live in metric-views.json + // server-side. + expect(bundle.revenue).not.toHaveProperty("source"); + expect(bundle.revenue).not.toHaveProperty("lane"); + }); + + test("preserves stable alphabetical key order across metrics", async () => { + const resolution = resolveMetricConfig({ + metricViews: { + z_metric: { source: "demo.public.z_metric" }, + a_metric: { source: "demo.public.a_metric" }, + }, + }); + + const fetcher = async () => + mockDescribeResponse({ + columns: [{ name: "v", type: "DECIMAL", is_measure: true }], + }); + + const { schemas } = await syncMetrics(resolution, fetcher); + const bundle = buildMetricsMetadataBundle(schemas); + expect(Object.keys(bundle)).toEqual(["a_metric", "z_metric"]); + }); + + test("omits absent fields rather than emitting null/empty placeholders", async () => { + const resolution = resolveMetricConfig({ + metricViews: { revenue: { source: "demo.public.revenue" } }, + }); + + const fetcher = async () => + mockDescribeResponse({ + columns: [{ name: "arr", type: "DECIMAL", is_measure: true }], + }); + + const { schemas } = await syncMetrics(resolution, fetcher); + const bundle = buildMetricsMetadataBundle(schemas); + const arr = bundle.revenue.measures.arr; + expect(arr.type).toBe("DECIMAL"); + expect(arr.display_name).toBeUndefined(); + expect(arr.format).toBeUndefined(); + expect(arr.description).toBeUndefined(); + expect(arr.time_grain).toBeUndefined(); + }); + + test("only emits time_grain on time-typed dimensions, never on measures", async () => { + const resolution = resolveMetricConfig({ + metricViews: { revenue: { source: "demo.public.revenue" } }, + }); + + const fetcher = async () => + mockDescribeResponse({ + columns: [ + // Even when a measure resolves to a temporal type (rare but possible + // for MEASURE() expressions like MAX(event_at)), no grains should be + // emitted — measures aren't grouped on. Grain inference is gated on + // is_measure: false in extractMetricColumns. + { name: "last_event_at", type: "TIMESTAMP", is_measure: true }, + { name: "ts", type: "TIMESTAMP", is_measure: false }, + ], + }); + + const { schemas } = await syncMetrics(resolution, fetcher); + const bundle = buildMetricsMetadataBundle(schemas); + expect(bundle.revenue.measures.last_event_at.time_grain).toBeUndefined(); + expect(bundle.revenue.dimensions.ts.time_grain).toEqual([ + "day", + "hour", + "minute", + "month", + "quarter", + "week", + "year", + ]); + }); +}); + +// ── Phase 5: metadata JSON serialization ────────────────────────────────── +describe("generateMetricsMetadataJson — snapshot", () => { + test("serializes a representative metric view with display_name + format + time_grain", async () => { + const resolution = resolveMetricConfig({ + metricViews: { + revenue: { source: "appkit_demo.public.revenue_metrics" }, + customer_metrics: { + source: "appkit_demo.public.customer_metrics", + executor: "user", + }, + }, + }); + + const fetcher = async (fqn: string) => + fqn.endsWith("revenue_metrics") + ? mockDescribeResponse({ + columns: [ + { + name: "arr", + type: "DECIMAL(38,2)", + is_measure: true, + display_name: "Annual Recurring Revenue", + format: "$#,##0.00", + comment: "ARR per quarter", + }, + { + name: "growth_rate", + type: "DOUBLE", + is_measure: true, + display_name: "Growth Rate", + format: "0.0%", + }, + { + name: "region", + type: "STRING", + is_measure: false, + display_name: "Region", + }, + { + name: "created_at", + type: "TIMESTAMP", + is_measure: false, + display_name: "Period", + }, + ], + }) + : mockDescribeResponse({ + columns: [ + { + name: "churn_rate", + type: "DOUBLE", + is_measure: true, + display_name: "Churn Rate", + format: "0.0%", + }, + { + name: "csm_email", + type: "STRING", + is_measure: false, + display_name: "CSM Email", + }, + ], + }); + + const { schemas } = await syncMetrics(resolution, fetcher); + const json = generateMetricsMetadataJson(schemas); + expect(json).toMatchSnapshot(); + + // Guard against snapshot blind-update: structural assertions on the parsed JSON. + const parsed = JSON.parse(json); + expect(Object.keys(parsed)).toEqual(["customer_metrics", "revenue"]); + expect(parsed.revenue.measures.arr.format).toBe("$#,##0.00"); + expect(parsed.revenue.measures.arr.display_name).toBe( + "Annual Recurring Revenue", + ); + // Time grains are inferred from the SQL type and ordered lexicographically. + // TIMESTAMP → all 7 standard grains. + expect(parsed.revenue.dimensions.created_at.time_grain).toEqual([ + "day", + "hour", + "minute", + "month", + "quarter", + "week", + "year", + ]); + // The client-shipped artifact must not carry server-side concerns: + // UC FQN (`source`) and execution lane (`lane`) live in metric-views.json + // and are consumed only on the server. Asserting their absence catches + // accidental re-introduction in code review or refactors. + expect(parsed.revenue).not.toHaveProperty("source"); + expect(parsed.revenue).not.toHaveProperty("lane"); + expect(parsed.customer_metrics).not.toHaveProperty("source"); + expect(parsed.customer_metrics).not.toHaveProperty("lane"); + }); + + test("emits `{}` when no metrics are registered", () => { + expect(generateMetricsMetadataJson([])).toBe("{}\n"); + }); +}); + +// ── Phase 2: syncMetrics propagates timeGrains end-to-end ──────────────── +describe("syncMetrics — time-typed dimension propagation", () => { + test("propagates inferred grains onto the resulting MetricSchema", async () => { + const resolution = resolveMetricConfig({ + metricViews: { revenue: { source: "demo.public.revenue" } }, + }); + + const fetcher = async () => + mockDescribeResponse({ + columns: [ + { name: "arr", type: "DECIMAL", is_measure: true }, + { name: "ts", type: "TIMESTAMP", is_measure: false }, + { name: "region", type: "STRING", is_measure: false }, + ], + }); + + const { schemas } = await syncMetrics(resolution, fetcher); + expect(schemas[0].dimensions).toHaveLength(2); + const tsDim = schemas[0].dimensions.find((d) => d.name === "ts"); + expect(tsDim?.timeGrains).toEqual([ + "day", + "hour", + "minute", + "month", + "quarter", + "week", + "year", + ]); + const regionDim = schemas[0].dimensions.find((d) => d.name === "region"); + expect(regionDim?.timeGrains).toBeUndefined(); + }); +}); diff --git a/packages/appkit/src/type-generator/tests/vite-plugin.test.ts b/packages/appkit/src/type-generator/tests/vite-plugin.test.ts index 16153257..b9623113 100644 --- a/packages/appkit/src/type-generator/tests/vite-plugin.test.ts +++ b/packages/appkit/src/type-generator/tests/vite-plugin.test.ts @@ -37,6 +37,11 @@ vi.mock("@databricks/sdk-experimental", () => ({ })); const { appKitTypesPlugin } = await import("../vite-plugin"); +// Real constant values: the "../index" mock spreads the actual module, so these +// are the genuine defaults the plugin resolves its metric out-paths from. +const { METRIC_METADATA_FILE, METRIC_TYPES_FILE, TYPES_DIR } = await import( + "../index" +); // The plugin hooks are loosely typed on Vite's Plugin; cast to the shapes we // actually drive so we can call them directly without a Vite build. @@ -258,6 +263,120 @@ describe("appKitTypesPlugin — single-flight generate", () => { await flush(); expect(mocks.generateFromEntryPoint).toHaveBeenCalledTimes(2); }); + + test("a metric-views.json change triggers a regeneration like a .sql edit", async () => { + mocks.generateFromEntryPoint.mockResolvedValue(undefined); + + const plugin = makeConfiguredPlugin(); + const { server, watcher } = makeFakeServer(); + getHook(plugin, "configureServer")(server); + const buildStart = getHook(plugin, "buildStart"); + + await buildStart(); + await flush(); + expect(mocks.generateFromEntryPoint).toHaveBeenCalledTimes(1); + + // The metric-view config rides the exact same watcher → single-flight + // regenerate flow as a .sql edit (no separate machinery). + watcher.emit( + "change", + path.join(process.cwd(), "config", "queries", "metric-views.json"), + ); + await flush(); + expect(mocks.generateFromEntryPoint).toHaveBeenCalledTimes(2); + }); + + test("an unrelated file change does not trigger a regeneration", async () => { + mocks.generateFromEntryPoint.mockResolvedValue(undefined); + + const plugin = makeConfiguredPlugin(); + const { server, watcher } = makeFakeServer(); + getHook(plugin, "configureServer")(server); + const buildStart = getHook(plugin, "buildStart"); + + await buildStart(); + await flush(); + expect(mocks.generateFromEntryPoint).toHaveBeenCalledTimes(1); + + // Inside the watched folder, but neither .sql nor metric-views.json. + watcher.emit( + "change", + path.join(process.cwd(), "config", "queries", "foo.txt"), + ); + await flush(); + expect(mocks.generateFromEntryPoint).toHaveBeenCalledTimes(1); + }); +}); + +describe("appKitTypesPlugin — metric option plumbing", () => { + const savedNodeEnv = process.env.NODE_ENV; + const savedWarehouseId = process.env.DATABRICKS_WAREHOUSE_ID; + + beforeEach(() => { + vi.clearAllMocks(); + mocks.generateFromEntryPoint.mockResolvedValue(undefined); + // Keep the warehouse watch inert (DELETED no-ops) so only the foreground + // generate's arguments are asserted. + mocks.getWarehouseState.mockResolvedValue("DELETED" as WarehouseState); + mocks.startWarehouse.mockResolvedValue(undefined); + mocks.waitUntilRunning.mockResolvedValue("RUNNING" as WarehouseState); + process.env.NODE_ENV = "development"; + process.env.DATABRICKS_WAREHOUSE_ID = "wh-test"; + }); + + afterEach(() => { + if (savedNodeEnv === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = savedNodeEnv; + + if (savedWarehouseId === undefined) + delete process.env.DATABRICKS_WAREHOUSE_ID; + else process.env.DATABRICKS_WAREHOUSE_ID = savedWarehouseId; + }); + + test("defaults the metric out files to shared/ siblings of outFile", async () => { + await runPlugin(); + await flush(); + + // configResolved resolves both metric paths against projectRoot + // (config.root/..) exactly like outFile. + expect(mocks.generateFromEntryPoint).toHaveBeenCalledWith( + expect.objectContaining({ + metricOutFile: path.resolve( + process.cwd(), + `shared/${TYPES_DIR}/${METRIC_TYPES_FILE}`, + ), + metricMetadataOutFile: path.resolve( + process.cwd(), + `shared/${TYPES_DIR}/${METRIC_METADATA_FILE}`, + ), + }), + ); + }); + + test("custom metricOutFile/metricMetadataOutFile reach generateFromEntryPoint", async () => { + const plugin = appKitTypesPlugin({ + metricOutFile: "custom/types/metric.d.ts", + metricMetadataOutFile: "custom/types/metrics.metadata.json", + }); + getHook( + plugin, + "configResolved", + )({ + root: path.join(process.cwd(), "client"), + }); + await getHook(plugin, "buildStart")(); + await flush(); + + expect(mocks.generateFromEntryPoint).toHaveBeenCalledWith( + expect.objectContaining({ + metricOutFile: path.resolve(process.cwd(), "custom/types/metric.d.ts"), + metricMetadataOutFile: path.resolve( + process.cwd(), + "custom/types/metrics.metadata.json", + ), + }), + ); + }); }); describe("appKitTypesPlugin — background warehouse watch", () => { diff --git a/packages/appkit/src/type-generator/vite-plugin.ts b/packages/appkit/src/type-generator/vite-plugin.ts index c869acdf..52996e75 100644 --- a/packages/appkit/src/type-generator/vite-plugin.ts +++ b/packages/appkit/src/type-generator/vite-plugin.ts @@ -6,6 +6,8 @@ import { createLogger } from "../logging/logger"; import { ANALYTICS_TYPES_FILE, generateFromEntryPoint, + METRIC_METADATA_FILE, + METRIC_TYPES_FILE, TYPES_DIR, TypegenFatalError, TypegenSyntaxError, @@ -33,6 +35,14 @@ const DEV_WAREHOUSE_WATCH_MAX_MS = 60_000; interface AppKitTypesPluginOptions { /* Path to the output d.ts file (relative to client folder). */ outFile?: string; + /** Path to the metric registry d.ts file (relative to client folder). */ + metricOutFile?: string; + /** + * Path to the metric semantic-metadata JSON file (relative to client folder). + * Build-time artifact — sibling of {@link metricOutFile}. Skipped + * automatically when `metric-views.json` is absent. + */ + metricMetadataOutFile?: string; /** Folders to watch for changes. */ watchFolders?: string[]; } @@ -45,6 +55,8 @@ interface AppKitTypesPluginOptions { */ export function appKitTypesPlugin(options?: AppKitTypesPluginOptions): Plugin { let outFile: string; + let metricOutFile: string; + let metricMetadataOutFile: string; let watchFolders: string[]; // Single-flight state for runGenerate(). `inFlight` is the promise of the @@ -94,6 +106,8 @@ export function appKitTypesPlugin(options?: AppKitTypesPluginOptions): Plugin { warehouseId, noCache: false, mode, + metricOutFile, + metricMetadataOutFile, }); } catch (error) { // TypegenSyntaxError / TypegenFatalError carry a complete, actionable @@ -296,6 +310,15 @@ export function appKitTypesPlugin(options?: AppKitTypesPluginOptions): Plugin { projectRoot, options?.outFile ?? `shared/${TYPES_DIR}/${ANALYTICS_TYPES_FILE}`, ); + metricOutFile = path.resolve( + projectRoot, + options?.metricOutFile ?? `shared/${TYPES_DIR}/${METRIC_TYPES_FILE}`, + ); + metricMetadataOutFile = path.resolve( + projectRoot, + options?.metricMetadataOutFile ?? + `shared/${TYPES_DIR}/${METRIC_METADATA_FILE}`, + ); watchFolders = options?.watchFolders ?? [ path.join(process.cwd(), "config", "queries"), ]; @@ -326,13 +349,18 @@ export function appKitTypesPlugin(options?: AppKitTypesPluginOptions): Plugin { changedFile.startsWith(folder), ); - if (isWatchedFile && changedFile.endsWith(".sql")) { + if ( + isWatchedFile && + (changedFile.endsWith(".sql") || + changedFile.endsWith("metric-views.json")) + ) { // Route through the single-flight runner (was fire-and-forget // generate(), which could race the initial build / watch). This is a // dev-only hook, so degrade instantly (non-blocking), then re-arm the - // warehouse watch so the edited query is re-described in the background - // against the running warehouse (or once a still-starting one warms - // up), landing fresh blocking-described types. + // warehouse watch so the edited query or metric-view source is + // re-described in the background against the running warehouse (or + // once a still-starting one warms up), landing fresh + // blocking-described types. void runGenerate("non-blocking"); armWarehouseWatch(); } From f55d4f994d0b156a38916e36006b51140fc20020 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Wed, 10 Jun 2026 20:46:29 +0200 Subject: [PATCH 02/10] fix(appkit): respect non-blocking mode in metric type-gen; dedupe sync failure logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review response (#433): (1) the metric emit block now gates DESCRIBEs on warehouse state in non-blocking mode — one read-only status GET (never starts a warehouse); when not RUNNING (or the probe fails) it skips all DESCRIBEs and emits degraded artifacts (every configured key with empty measures/dimensions) that the vite plugin's warehouse-watch regen (blocking mode) refreshes once the warehouse is up. Blocking mode and injected metricFetcher bypass the gate. (2) syncMetrics is now log-free: the three internal per-failure warns are removed and the generateFromEntryPoint caller owns surfacing failures exactly once. Co-authored-by: Isaac Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/index.ts | 106 +++++++++-- .../src/type-generator/metric-registry.ts | 38 ++-- .../src/type-generator/tests/index.test.ts | 177 ++++++++++++++++++ 3 files changed, 289 insertions(+), 32 deletions(-) diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index 1ae9f4fb..28871d00 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -1,11 +1,13 @@ import fs from "node:fs/promises"; import path from "node:path"; +import { WorkspaceClient } from "@databricks/sdk-experimental"; import dotenv from "dotenv"; import pc from "picocolors"; import { createLogger } from "../logging/logger"; import { createWorkspaceDescribeFetcher, type DescribeFetcher, + emptyMetricSchema, generateMetricsMetadataJson, generateMetricTypeDeclarations, type MetricColumnMetadata, @@ -26,6 +28,7 @@ import type { PreflightMode } from "./preflight"; import { generateQueriesFromDescribe } from "./query-registry"; import { generateServingTypes as generateServingTypesImpl } from "./serving/generator"; import type { QueryFatalError, QuerySchema, QuerySyntaxError } from "./types"; +import { getWarehouseState } from "./warehouse-status"; dotenv.config(); @@ -208,11 +211,37 @@ declare module "@databricks/appkit-ui/react" { `; } +/** + * Status-only probe for the metric-view gate in {@link generateFromEntryPoint}: + * is the warehouse RUNNING right now? + * + * Uses {@link getWarehouseState} (`warehouses.get`) — a read-only GET that can + * never start the warehouse — unlike the metric DESCRIBE statements it guards, + * whose statement execution auto-starts a stopped warehouse and waits on it. + * Probe failures (connectivity, auth, bad id) read as "not running": in + * non-blocking mode typegen must never block on, or fail because of, the + * warehouse, so the caller degrades and a later blocking run (e.g. the Vite + * plugin's warehouse watch) lands the real schemas. + */ +async function isWarehouseRunning(warehouseId: string): Promise { + try { + const client = new WorkspaceClient({}); + return (await getWarehouseState(client, warehouseId)) === "RUNNING"; + } catch { + return false; + } +} + /** * Entry point for generating type declarations from all imported files * @param options - the options for the generation * @param options.entryPoint - the entry point file * @param options.outFile - the output file + * @param options.mode - preflight policy (see {@link PreflightMode}). For + * queries, `"non-blocking"` never probes or describes the warehouse. For + * metric views, `"non-blocking"` makes one status-only probe and DESCRIBEs + * only when the warehouse is already RUNNING (otherwise degraded artifacts + * are emitted); `"blocking"` always DESCRIBEs. Defaults to `"non-blocking"`. * @param options.metricOutFile - optional output file for the MetricRegistry * augmentation. Defaults to a sibling `metric.d.ts` file under the same * directory as `outFile`. Skipped entirely if `metric-views.json` is absent. @@ -222,7 +251,10 @@ declare module "@databricks/appkit-ui/react" { * `metric-views.json` is absent. * @param options.metricFetcher - optional DescribeFetcher used by * {@link syncMetrics}. Tests inject a mock; production builds let the - * default WorkspaceClient-backed fetcher be created lazily. + * default WorkspaceClient-backed fetcher be created lazily. An injected + * fetcher always runs — it bypasses the non-blocking warehouse gate, since + * it does not hit a warehouse and skipping it would only blind the tests + * and CI runs that inject it. */ export async function generateFromEntryPoint(options: { outFile: string; @@ -273,26 +305,62 @@ export async function generateFromEntryPoint(options: { const metricConfig = await readMetricConfig(queryFolder); if (metricConfig) { const resolution = resolveMetricConfig(metricConfig); - const fetcher = - metricFetcher ?? createWorkspaceDescribeFetcher(warehouseId); - const { schemas: metricSchemas, failures } = await syncMetrics( - resolution, - fetcher, - ); - // Surface DESCRIBE failures loudly so a misconfigured metric-views.json - // or a workspace-side typo doesn't silently ship an empty bundle entry. - // The route's runtime fail-closed gate would 503 these in production — - // catching the issue at type-gen time is the cheaper signal. - if (failures.length > 0) { - for (const f of failures) { - logger.warn( - "metric sync failed for %s (%s): %s", - f.key, - f.source, - f.reason, - ); + // Honor the non-blocking preflight contract (#406) for metric DESCRIBEs + // too: each `DESCRIBE TABLE EXTENDED ... AS JSON` waits up to 30s per + // key and auto-starts a stopped warehouse — exactly what "non-blocking" + // promises never to do. One status-only probe (a GET that can never + // start the warehouse) decides whether to describe now or emit degraded + // artifacts that a later blocking run refreshes. An injected + // metricFetcher always runs: it doesn't hit a warehouse (tests/CI + // inject mocks), so gating it would only skip meaningful work. An empty + // metricViews map needs no probe either — nothing would be described + // in any mode. + const describeNow = + metricFetcher !== undefined || + mode !== "non-blocking" || + resolution.entries.length === 0 || + (await isWarehouseRunning(warehouseId)); + + let metricSchemas: MetricSchema[]; + let failures: MetricSyncFailure[] = []; + if (describeNow) { + const fetcher = + metricFetcher ?? createWorkspaceDescribeFetcher(warehouseId); + ({ schemas: metricSchemas, failures } = await syncMetrics( + resolution, + fetcher, + )); + + // Surface DESCRIBE failures loudly so a misconfigured metric-views.json + // or a workspace-side typo doesn't silently ship an empty bundle entry. + // The route's runtime fail-closed gate would 503 these in production — + // catching the issue at type-gen time is the cheaper signal. + // syncMetrics itself is log-free; this caller is the single owner of + // failure logging. + if (failures.length > 0) { + for (const f of failures) { + logger.warn( + "metric sync failed for %s (%s): %s", + f.key, + f.source, + f.reason, + ); + } } + } else { + // Deliberately un-probed DESCRIBEs, not failures: emit every + // configured key with empty measure/dimension allowlists so both + // artifacts always exist, and say so once — no per-key warnings + // (nothing failed). The dev warehouse watch (or the next blocking + // run) re-enters this path with the warehouse RUNNING and lands the + // real schemas. + metricSchemas = resolution.entries.map(emptyMetricSchema); + logger.info( + "Warehouse %s is not running — wrote degraded metric types (empty schemas) for %d metric view(s); they will refresh once the warehouse is available.", + warehouseId, + resolution.entries.length, + ); } const metricFile = diff --git a/packages/appkit/src/type-generator/metric-registry.ts b/packages/appkit/src/type-generator/metric-registry.ts index 17947255..11246f2d 100644 --- a/packages/appkit/src/type-generator/metric-registry.ts +++ b/packages/appkit/src/type-generator/metric-registry.ts @@ -1,11 +1,8 @@ import fs from "node:fs/promises"; import path from "node:path"; import { WorkspaceClient } from "@databricks/sdk-experimental"; -import { createLogger } from "../logging/logger"; import type { DatabricksStatementExecutionResponse } from "./types"; -const logger = createLogger("type-generator:metric-registry"); - /** * Default filename for the metric source declarations. * Lives at config/queries/metric-views.json by convention. @@ -1012,6 +1009,26 @@ export interface MetricSyncResult { failures: MetricSyncFailure[]; } +/** + * Build the degraded schema emitted when an entry's columns are not + * available — same key/source/lane as a real schema, with empty + * measure/dimension allowlists. Shared by {@link syncMetrics}' per-entry + * failure path and by callers that skip DESCRIBE entirely (the non-blocking + * warehouse gate in `generateFromEntryPoint`), so "entry present but empty" + * has exactly one definition. + */ +export function emptyMetricSchema( + entry: Pick, +): MetricSchema { + return { + key: entry.key, + source: entry.source, + lane: entry.lane, + measures: [], + dimensions: [], + }; +} + /** * Run schema synchronization for every entry in `metric-views.json`. * @@ -1024,6 +1041,10 @@ export interface MetricSyncResult { * (a) the DESCRIBE call rejected, (b) the response could not be parsed, or * (c) extraction yielded zero columns. Callers (the CLI, the Vite plugin) * inspect `failures` to decide whether to exit non-zero. + * + * This function is deliberately log-free: callers own surfacing `failures` + * (logging, exit codes), so each failure is reported exactly once at the call + * site instead of once in here and again by the caller. */ export async function syncMetrics( resolution: MetricConfigResolution, @@ -1038,15 +1059,8 @@ export async function syncMetrics( response = await fetcher(entry.source); } catch (err) { const reason = `DESCRIBE TABLE EXTENDED failed: ${(err as Error).message}`; - logger.warn("%s for %s", reason, entry.source); failures.push({ key: entry.key, source: entry.source, reason }); - schemas.push({ - key: entry.key, - source: entry.source, - lane: entry.lane, - measures: [], - dimensions: [], - }); + schemas.push(emptyMetricSchema(entry)); continue; } @@ -1057,7 +1071,6 @@ export async function syncMetrics( columns = extractMetricColumns(parsed); } catch (err) { parseError = `Failed to extract columns from DESCRIBE response: ${(err as Error).message}`; - logger.warn("%s for %s", parseError, entry.source); } const measures = columns.filter((c) => c.isMeasure); @@ -1077,7 +1090,6 @@ export async function syncMetrics( // 503 every request to this metric in production. const reason = "DESCRIBE response yielded zero columns — check the response shape (top-level `columns` array or `schema.fields`)."; - logger.warn("%s for %s", reason, entry.source); failures.push({ key: entry.key, source: entry.source, reason }); } diff --git a/packages/appkit/src/type-generator/tests/index.test.ts b/packages/appkit/src/type-generator/tests/index.test.ts index d37525aa..a173378c 100644 --- a/packages/appkit/src/type-generator/tests/index.test.ts +++ b/packages/appkit/src/type-generator/tests/index.test.ts @@ -13,6 +13,8 @@ import type { DatabricksStatementExecutionResponse } from "../types"; const mocks = vi.hoisted(() => ({ generateQueriesFromDescribe: vi.fn(), + getWarehouseState: vi.fn(), + executeStatement: vi.fn(), })); // Mock only the warehouse-describe step; index.ts owns the throw decision we @@ -25,6 +27,27 @@ vi.mock("../query-registry", async (importOriginal) => { }; }); +// The metric gate's status-only probe resolves through getWarehouseState; stub +// it so tests dictate the observed warehouse state. Keep the rest of the module +// actual — query-registry imports startWarehouse/waitUntilRunning from it. +vi.mock("../warehouse-status", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getWarehouseState: mocks.getWarehouseState, + }; +}); + +// index.ts constructs `new WorkspaceClient({})` for the metric probe and (via +// createWorkspaceDescribeFetcher) for the default DESCRIBE fetcher. Stub the +// SDK so neither needs real credentials; `executeStatement` doubles as the +// "was any metric DESCRIBE actually issued?" spy. +vi.mock("@databricks/sdk-experimental", () => ({ + WorkspaceClient: vi.fn(() => ({ + statementExecution: { executeStatement: mocks.executeStatement }, + })), +})); + const { generateFromEntryPoint, TypegenFatalError, TypegenSyntaxError } = await import("../index"); @@ -333,4 +356,158 @@ describe("generateFromEntryPoint — metric-view emission", () => { const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); }); + + // ── Non-blocking warehouse gate: metric DESCRIBEs honor the #406 contract ── + + test("non-blocking + warehouse not running: skips all DESCRIBEs but still emits degraded artifacts", async () => { + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + JSON.stringify({ + metricViews: { + revenue: { source: "demo.sales.revenue" }, + churn: { source: "demo.sales.churn", executor: "user" }, + }, + }), + ); + mocks.getWarehouseState.mockResolvedValue("STOPPED"); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + try { + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "non-blocking", + }), + ).resolves.toBeUndefined(); + + // One status-only probe, zero DESCRIBE statements against the stopped + // warehouse (a DESCRIBE would block ~30s per key and auto-start it). + expect(mocks.getWarehouseState).toHaveBeenCalledTimes(1); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + + // Both artifacts still ship with EVERY configured key present — + // degraded (empty allowlists), key/source/lane intact. + const declarations = fs.readFileSync(metricFile, "utf-8"); + expect(declarations).toContain('"revenue"'); + expect(declarations).toContain('"churn"'); + expect(declarations).toContain('lane: "obo"'); + const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); + expect(bundle.churn).toEqual({ measures: {}, dimensions: {} }); + + // Nothing failed (we deliberately didn't probe each key), so no + // per-key "metric sync failed" warnings — just the single + // degraded-emit info line. (Unrelated warns, e.g. the migration + // helper's project-root notice in this temp dir, are not in scope.) + const warned = warnSpy.mock.calls.flat().map(String).join("\n"); + expect(warned).not.toContain("metric sync failed"); + const logged = logSpy.mock.calls.flat().map(String).join("\n"); + expect(logged).toContain("degraded metric types"); + + // Query typegen is unaffected. + expect(fs.existsSync(outFile)).toBe(true); + } finally { + warnSpy.mockRestore(); + logSpy.mockRestore(); + } + }); + + test("non-blocking + RUNNING warehouse: DESCRIBEs run and land full schemas", async () => { + writeMetricConfig(); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue(describeResponse); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "non-blocking", + }), + ).resolves.toBeUndefined(); + + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + expect(mocks.executeStatement).toHaveBeenCalledWith( + expect.objectContaining({ + statement: "DESCRIBE TABLE EXTENDED demo.sales.revenue AS JSON", + warehouse_id: "wh-1", + }), + ); + expect(fs.readFileSync(metricFile, "utf-8")).toContain( + '"total_revenue": number', + ); + const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + expect(bundle.revenue.measures.total_revenue.type).toBe("DECIMAL(38,2)"); + }); + + test("blocking mode: DESCRIBEs are attempted without probing warehouse state", async () => { + writeMetricConfig(); + // Even a stopped warehouse is never probed in blocking mode — the + // DESCRIBE itself owns warehouse readiness (it waits / auto-starts). + mocks.getWarehouseState.mockResolvedValue("STOPPED"); + mocks.executeStatement.mockResolvedValue(describeResponse); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "blocking", + }), + ).resolves.toBeUndefined(); + + expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + expect(fs.readFileSync(metricFile, "utf-8")).toContain( + '"total_revenue": number', + ); + }); + + test("an injected metricFetcher bypasses the gate even when non-blocking + stopped", async () => { + writeMetricConfig(); + mocks.getWarehouseState.mockResolvedValue("STOPPED"); + const fetcher = vi.fn(async () => describeResponse); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "non-blocking", + metricFetcher: fetcher, + }), + ).resolves.toBeUndefined(); + + // The injected fetcher doesn't hit a warehouse, so it always runs — no + // probe, no skip. This keeps test/CI injections meaningful. + expect(fetcher).toHaveBeenCalledTimes(1); + expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + expect(fs.readFileSync(metricFile, "utf-8")).toContain( + '"total_revenue": number', + ); + }); + + test("non-blocking: a failed status probe degrades instead of throwing", async () => { + writeMetricConfig(); + mocks.getWarehouseState.mockRejectedValue( + new Error("connect ECONNREFUSED"), + ); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "non-blocking", + }), + ).resolves.toBeUndefined(); + + expect(mocks.executeStatement).not.toHaveBeenCalled(); + const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); + }); }); From 35d18009dbbd89c325604334186a9f622edff2e3 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Thu, 11 Jun 2026 18:11:06 +0200 Subject: [PATCH 03/10] feat(appkit): classify non-terminal DESCRIBE as degraded; render degraded metric types permissively Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/index.ts | 30 ++- .../src/type-generator/metric-registry.ts | 132 +++++++++++-- .../metric-registry.test.ts.snap | 47 +++++ .../src/type-generator/tests/index.test.ts | 63 ++++++- .../tests/metric-registry.test.ts | 175 ++++++++++++++++++ 5 files changed, 421 insertions(+), 26 deletions(-) diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index 28871d00..677012bc 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -348,18 +348,36 @@ export async function generateFromEntryPoint(options: { ); } } + + // Degraded-but-not-failed keys: the warehouse answered with a + // non-terminal state (stopped / cold-starting), so their schemas are + // unknown — not errors. One summary line, no per-key warns; failed + // keys are excluded (the warn loop above already reported them). + const failedKeys = new Set(failures.map((f) => f.key)); + const degradedKeys = metricSchemas + .filter((s) => s.degraded && !failedKeys.has(s.key)) + .map((s) => s.key); + if (degradedKeys.length > 0) { + logger.info( + "Warehouse %s did not return schemas for %d metric view(s) (%s) — wrote degraded metric types (permissive); they will refresh once the warehouse is available.", + warehouseId, + degradedKeys.length, + degradedKeys.join(", "), + ); + } } else { // Deliberately un-probed DESCRIBEs, not failures: emit every - // configured key with empty measure/dimension allowlists so both - // artifacts always exist, and say so once — no per-key warnings - // (nothing failed). The dev warehouse watch (or the next blocking - // run) re-enters this path with the warehouse RUNNING and lands the - // real schemas. + // configured key as a degraded schema (permissive types, empty + // runtime allowlists) so both artifacts always exist, and say so + // once — no per-key warnings (nothing failed). The dev warehouse + // watch (or the next blocking run) re-enters this path with the + // warehouse RUNNING and lands the real schemas. metricSchemas = resolution.entries.map(emptyMetricSchema); logger.info( - "Warehouse %s is not running — wrote degraded metric types (empty schemas) for %d metric view(s); they will refresh once the warehouse is available.", + "Warehouse %s is not running — wrote degraded metric types (permissive) for %d metric view(s) (%s); they will refresh once the warehouse is available.", warehouseId, resolution.entries.length, + resolution.entries.map((e) => e.key).join(", "), ); } diff --git a/packages/appkit/src/type-generator/metric-registry.ts b/packages/appkit/src/type-generator/metric-registry.ts index 11246f2d..aca81812 100644 --- a/packages/appkit/src/type-generator/metric-registry.ts +++ b/packages/appkit/src/type-generator/metric-registry.ts @@ -120,6 +120,21 @@ export interface MetricSchema { measures: MetricColumnMetadata[]; /** Dimension columns (everything that is not a measure). */ dimensions: MetricColumnMetadata[]; + /** + * `true` when the schema is unknown — the warehouse couldn't tell us + * (DESCRIBE was skipped, returned a non-terminal state, was rejected, or + * its response couldn't be parsed into columns). Absent/`false` means the + * measures/dimensions are a real DESCRIBE result, including a genuinely + * column-light view (e.g. dimensions only). + * + * Degraded entries render permissive types (`string` unions, permissive + * row) so an app still compiles while the warehouse is unavailable; + * non-degraded entries keep exact (possibly `never`) unions. Orthogonal to + * {@link MetricSyncFailure}: `failures` drives loud reporting, `degraded` + * drives permissive rendering. Plain JSON-safe boolean so the schema can be + * serialized into a future typegen cache verbatim. + */ + degraded?: boolean; } /** @@ -290,6 +305,13 @@ export function resolveMetricConfig( * The Statement Execution API returns a single string cell — this normalizer * unwraps it. Handles both the production (real warehouse) shape and the * shape produced by mocked test responses. + * + * Precondition: the statement reached a terminal state. {@link syncMetrics} + * classifies non-terminal responses (PENDING/RUNNING — a stopped or + * cold-starting warehouse that outlived `wait_timeout`) as degraded before + * calling this, so the "returned no rows" error below only ever describes a + * SUCCEEDED statement that genuinely produced no rows (a wrong FQN), never + * warehouse readiness. */ export function parseDescribeTableExtendedJson( response: DatabricksStatementExecutionResponse, @@ -643,8 +665,17 @@ function tsTypeFor(sqlType: string): string { /** * Render a MetricRegistry interface entry from a MetricSchema. + * + * Degraded schemas (see {@link MetricSchema.degraded}) render permissive + * types instead of exact unions: the schema is unknown, so `never`-style + * empty unions would reject every measure/dimension/grain and block the app + * from compiling until the warehouse comes back. Non-degraded schemas — + * including genuinely column-light views — keep accurate unions. */ function renderMetricEntry(schema: MetricSchema): string { + if (schema.degraded) { + return renderDegradedMetricEntry(schema); + } const indent = " "; const measures = schema.measures.length > 0 @@ -726,6 +757,39 @@ ${dimensions}; }`; } +/** + * Render the permissive ("degraded-open") entry for a schema the warehouse + * could not describe. Key/source/lane stay exact (they come from + * metric-views.json, not the warehouse); everything schema-derived opens up: + * + * - `measureKeys` / `dimensionKeys` / `timeGrains` become `string` so any + * helper-type union derived from them accepts arbitrary identifiers; + * - `measures` / `dimensions` become `Record` so the row + * type they feed is permissive instead of `Record`; + * - `metadata` stays `Record` — it mirrors the runtime + * bundle, which emits `{ measures: {}, dimensions: {} }` for this key. + * + * The next successful run (warehouse RUNNING) replaces this entry with exact + * unions; a confirmed-empty view never takes this path. + */ +function renderDegradedMetricEntry(schema: MetricSchema): string { + return ` /** Degraded: schema unavailable at type-generation time — permissive types until a successful DESCRIBE refreshes them. */ + ${JSON.stringify(schema.key)}: { + key: ${JSON.stringify(schema.key)}; + source: ${JSON.stringify(schema.source)}; + lane: ${JSON.stringify(schema.lane)}; + measures: Record; + dimensions: Record; + measureKeys: string; + dimensionKeys: string; + timeGrains: string; + metadata: { + measures: Record; + dimensions: Record; + }; + }`; +} + /** * Render the type-level shape of a column's semantic-metadata map for the * `metadata` field of a MetricRegistry entry. @@ -1012,10 +1076,13 @@ export interface MetricSyncResult { /** * Build the degraded schema emitted when an entry's columns are not * available — same key/source/lane as a real schema, with empty - * measure/dimension allowlists. Shared by {@link syncMetrics}' per-entry - * failure path and by callers that skip DESCRIBE entirely (the non-blocking - * warehouse gate in `generateFromEntryPoint`), so "entry present but empty" - * has exactly one definition. + * measure/dimension allowlists and `degraded: true` (see + * {@link MetricSchema.degraded}: the schema is unknown, so renderers emit + * permissive types instead of `never`-style empty unions). Shared by + * {@link syncMetrics}' per-entry failure + non-terminal paths and by callers + * that skip DESCRIBE entirely (the non-blocking warehouse gate in + * `generateFromEntryPoint`), so "entry present but unknown" has exactly one + * definition. */ export function emptyMetricSchema( entry: Pick, @@ -1026,6 +1093,7 @@ export function emptyMetricSchema( lane: entry.lane, measures: [], dimensions: [], + degraded: true, }; } @@ -1036,15 +1104,27 @@ export function emptyMetricSchema( * tests with a mock that returns a representative DESCRIBE response. * * Returns `{ schemas, failures }`. The schemas array always carries one - * entry per registered metric (even on failure — the entry has empty - * measures/dimensions). The failures array is populated for any entry that - * (a) the DESCRIBE call rejected, (b) the response could not be parsed, or - * (c) extraction yielded zero columns. Callers (the CLI, the Vite plugin) - * inspect `failures` to decide whether to exit non-zero. + * entry per registered metric. Classification mirrors the query path + * (query-registry's describe flow): + * + * - FAILED statement, rejected fetch, unparseable response, or zero + * extracted columns → a genuine failure: recorded in `failures` AND the + * schema is `degraded: true` (its columns are unknown). + * - Non-terminal statement state (PENDING/RUNNING — warehouse reachable but + * not ready) → degraded, never an error: schema is `degraded: true`, NOT + * in `failures`. The next run with a ready warehouse lands the real + * schema. + * - SUCCEEDED with extracted columns → real schema, `degraded` unset (a + * genuinely column-light view keeps its accurate empty unions). + * + * Callers (the CLI, the Vite plugin) inspect `failures` to decide whether to + * exit non-zero; renderers inspect `degraded` to emit permissive types. The + * two are orthogonal: failures drive loud reporting, degraded drives + * permissive rendering. * * This function is deliberately log-free: callers own surfacing `failures` - * (logging, exit codes), so each failure is reported exactly once at the call - * site instead of once in here and again by the caller. + * (logging, exit codes) and degraded summaries, so each is reported exactly + * once at the call site instead of once in here and again by the caller. */ export async function syncMetrics( resolution: MetricConfigResolution, @@ -1064,6 +1144,19 @@ export async function syncMetrics( continue; } + // Non-terminal statement state (PENDING/RUNNING): the warehouse is + // reachable but not ready — stopped or cold-starting, with the DESCRIBE's + // `wait_timeout` elapsed before completion. Degraded, never an error + // (precedent: query-registry treats anything that is neither FAILED nor + // SUCCEEDED as "unavailable"). The response carries no rows yet, so + // falling through would misclassify this as the "returned no rows" / + // wrong-FQN failure. + const state = response.status?.state; + if (state !== "SUCCEEDED" && state !== "FAILED") { + schemas.push(emptyMetricSchema(entry)); + continue; + } + let columns: MetricColumnMetadata[] = []; let parseError: string | null = null; try { @@ -1073,26 +1166,33 @@ export async function syncMetrics( parseError = `Failed to extract columns from DESCRIBE response: ${(err as Error).message}`; } - const measures = columns.filter((c) => c.isMeasure); - const dimensions = columns.filter((c) => !c.isMeasure); - if (parseError) { failures.push({ key: entry.key, source: entry.source, reason: parseError, }); - } else if (columns.length === 0) { + schemas.push(emptyMetricSchema(entry)); + continue; + } + + if (columns.length === 0) { // Extraction succeeded but yielded no columns. The most common cause // is a DESCRIBE response shape that `extractMetricColumns` doesn't // recognize. Treat as a failure so CI catches it instead of letting an // empty bundle entry ship — the route's fail-closed gate would then - // 503 every request to this metric in production. + // 503 every request to this metric in production. The schema is also + // degraded: its real columns are unknown. const reason = "DESCRIBE response yielded zero columns — check the response shape (top-level `columns` array or `schema.fields`)."; failures.push({ key: entry.key, source: entry.source, reason }); + schemas.push(emptyMetricSchema(entry)); + continue; } + const measures = columns.filter((c) => c.isMeasure); + const dimensions = columns.filter((c) => !c.isMeasure); + schemas.push({ key: entry.key, source: entry.source, diff --git a/packages/appkit/src/type-generator/tests/__snapshots__/metric-registry.test.ts.snap b/packages/appkit/src/type-generator/tests/__snapshots__/metric-registry.test.ts.snap index 71b34e28..5f10f29a 100644 --- a/packages/appkit/src/type-generator/tests/__snapshots__/metric-registry.test.ts.snap +++ b/packages/appkit/src/type-generator/tests/__snapshots__/metric-registry.test.ts.snap @@ -151,6 +151,53 @@ declare module "@databricks/appkit-ui/react" { " `; +exports[`generateMetricTypeDeclarations — snapshot > emits permissive types for a degraded entry and accurate empty unions for a confirmed-empty entry 1`] = ` +"// Auto-generated by AppKit - DO NOT EDIT +// Generated by 'npx @databricks/appkit generate-types' or Vite plugin during build +import "@databricks/appkit-ui/react"; +declare module "@databricks/appkit-ui/react" { + interface MetricRegistry { + /** Degraded: schema unavailable at type-generation time — permissive types until a successful DESCRIBE refreshes them. */ + "cold_metric": { + key: "cold_metric"; + source: "appkit_demo.public.cold_metric"; + lane: "sp"; + measures: Record; + dimensions: Record; + measureKeys: string; + dimensionKeys: string; + timeGrains: string; + metadata: { + measures: Record; + dimensions: Record; + }; + }; + "dims_only": { + key: "dims_only"; + source: "appkit_demo.public.dims_only"; + lane: "obo"; + measures: Record; + dimensions: { + /** @sqlType STRING */ + "region": string; + }; + measureKeys: never; + dimensionKeys: "region"; + timeGrains: never; + metadata: { + measures: Record; + dimensions: { + "region": { + type: "STRING"; + }; + }; + }; + }; + } +} +" +`; + exports[`generateMetricsMetadataJson — snapshot > serializes a representative metric view with display_name + format + time_grain 1`] = ` "{ "customer_metrics": { diff --git a/packages/appkit/src/type-generator/tests/index.test.ts b/packages/appkit/src/type-generator/tests/index.test.ts index a173378c..a86f001c 100644 --- a/packages/appkit/src/type-generator/tests/index.test.ts +++ b/packages/appkit/src/type-generator/tests/index.test.ts @@ -350,9 +350,61 @@ describe("generateFromEntryPoint — metric-view emission", () => { "interface QueryRegistry", ); - // ... and both artifacts still ship, with the failed key carrying an - // empty schema rather than poisoning the build. - expect(fs.readFileSync(metricFile, "utf-8")).toContain('"revenue"'); + // ... and both artifacts still ship, with the failed key carrying a + // degraded schema (permissive types — its real columns are unknown) + // rather than poisoning the build. + const declarations = fs.readFileSync(metricFile, "utf-8"); + expect(declarations).toContain('"revenue"'); + expect(declarations).toContain("measureKeys: string"); + const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); + }); + + test("a non-terminal DESCRIBE response degrades without failing: no warn, one info line, permissive types", async () => { + writeMetricConfig(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + try { + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + // Stopped/cold warehouse: the DESCRIBE's wait_timeout elapsed with + // the statement still PENDING — no rows yet. Previously this fell + // into the "returned no rows" failure with per-key warns. + metricFetcher: async () => ({ + statement_id: "stmt-mock", + status: { state: "PENDING" }, + }), + }), + ).resolves.toBeUndefined(); + + // Degraded, never an error: no per-key failure warns ... + const warned = warnSpy.mock.calls.flat().map(String).join("\n"); + expect(warned).not.toContain("metric sync failed"); + // ... exactly one info summary line, naming the degraded key. + const degradedLines = logSpy.mock.calls + .map((call) => call.map(String).join(" ")) + .filter((line) => line.includes("degraded metric types")); + expect(degradedLines).toHaveLength(1); + expect(degradedLines[0]).toContain("revenue"); + expect(degradedLines[0]).toContain( + "refresh once the warehouse is available", + ); + } finally { + warnSpy.mockRestore(); + logSpy.mockRestore(); + } + + // Degraded-open rendering: permissive unions for the unknown schema. + const declarations = fs.readFileSync(metricFile, "utf-8"); + expect(declarations).toContain('"revenue"'); + expect(declarations).toContain("measureKeys: string"); + expect(declarations).toContain("dimensionKeys: string"); + expect(declarations).toContain("timeGrains: string"); + // The metadata bundle keeps its locked frontend-safe shape. const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); }); @@ -389,11 +441,14 @@ describe("generateFromEntryPoint — metric-view emission", () => { expect(mocks.executeStatement).not.toHaveBeenCalled(); // Both artifacts still ship with EVERY configured key present — - // degraded (empty allowlists), key/source/lane intact. + // degraded (permissive types, empty bundle allowlists), key/source/lane + // intact. const declarations = fs.readFileSync(metricFile, "utf-8"); expect(declarations).toContain('"revenue"'); expect(declarations).toContain('"churn"'); expect(declarations).toContain('lane: "obo"'); + expect(declarations).toContain("measureKeys: string"); + expect(declarations).toContain("timeGrains: string"); const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); expect(bundle.churn).toEqual({ measures: {}, dimensions: {} }); diff --git a/packages/appkit/src/type-generator/tests/metric-registry.test.ts b/packages/appkit/src/type-generator/tests/metric-registry.test.ts index 48b042a6..3a94d9c4 100644 --- a/packages/appkit/src/type-generator/tests/metric-registry.test.ts +++ b/packages/appkit/src/type-generator/tests/metric-registry.test.ts @@ -513,6 +513,8 @@ describe("syncMetrics", () => { expect(schema.key).toBe("revenue"); expect(schema.measures.map((m) => m.name)).toEqual(["arr", "mrr"]); expect(schema.dimensions.map((d) => d.name)).toEqual(["region"]); + // A successful parse is a real schema — never marked degraded. + expect(schema.degraded).toBeUndefined(); }); test("falls back to empty columns when DESCRIBE throws (does not crash typegen)", async () => { @@ -527,6 +529,9 @@ describe("syncMetrics", () => { const { schemas, failures } = await syncMetrics(resolution, fetcher); expect(schemas[0].measures).toEqual([]); expect(schemas[0].dimensions).toEqual([]); + // Both flags, orthogonally: the failure drives loud reporting, the + // degraded marker drives permissive rendering (the schema is unknown). + expect(schemas[0].degraded).toBe(true); expect(failures).toHaveLength(1); expect(failures[0]).toMatchObject({ key: "revenue", @@ -536,6 +541,109 @@ describe("syncMetrics", () => { }); }); +// ── Parity with the query path's state machine (query-registry): FAILED → +// genuine error, SUCCEEDED → proceed, anything else (PENDING/RUNNING) → +// degraded, never an error. A stopped/cold warehouse that outlives the +// DESCRIBE's `wait_timeout` returns a non-terminal state with no rows — +// previously misclassified as the "returned no rows" wrong-FQN failure. +describe("syncMetrics — DESCRIBE state classification", () => { + const singleEntryResolution = () => + resolveMetricConfig({ + metricViews: { revenue: { source: "demo.public.revenue" } }, + }); + + for (const state of ["PENDING", "RUNNING"] as const) { + test(`a non-terminal ${state} response degrades the schema without recording a failure`, async () => { + const fetcher = + async (): Promise => ({ + statement_id: "stmt-mock", + status: { state }, + }); + + const { schemas, failures } = await syncMetrics( + singleEntryResolution(), + fetcher, + ); + expect(failures).toEqual([]); + expect(schemas).toHaveLength(1); + expect(schemas[0]).toMatchObject({ + key: "revenue", + source: "demo.public.revenue", + lane: "sp", + measures: [], + dimensions: [], + degraded: true, + }); + }); + } + + test("a FAILED response stays a genuine failure (and its schema is degraded)", async () => { + const fetcher = + async (): Promise => ({ + statement_id: "stmt-mock", + status: { state: "FAILED", error: { message: "no such table" } }, + }); + + const { schemas, failures } = await syncMetrics( + singleEntryResolution(), + fetcher, + ); + expect(failures).toHaveLength(1); + expect(failures[0].reason).toMatch(/no such table/); + expect(schemas[0].degraded).toBe(true); + }); + + test("a SUCCEEDED response with zero rows stays the wrong-FQN failure", async () => { + // Non-terminal states are classified before parsing, so "returned no + // rows" is reserved for a statement that genuinely completed empty — a + // wrong FQN, not warehouse readiness. + const fetcher = + async (): Promise => ({ + statement_id: "stmt-mock", + status: { state: "SUCCEEDED" }, + result: { data_array: [] }, + }); + + const { schemas, failures } = await syncMetrics( + singleEntryResolution(), + fetcher, + ); + expect(failures).toHaveLength(1); + expect(failures[0].reason).toMatch(/returned no rows/); + expect(failures[0].reason).toMatch(/Verify the FQN/); + expect(schemas[0].degraded).toBe(true); + }); + + test("a SUCCEEDED response yielding zero extracted columns stays a failure with a degraded schema", async () => { + const fetcher = async () => mockDescribeResponse({ unrelated: true }); + + const { schemas, failures } = await syncMetrics( + singleEntryResolution(), + fetcher, + ); + expect(failures).toHaveLength(1); + expect(failures[0].reason).toMatch(/zero columns/); + expect(schemas[0].degraded).toBe(true); + }); + + test("a confirmed-empty view (SUCCEEDED, dimensions only) is NOT degraded", async () => { + const resolution = resolveMetricConfig({ + metricViews: { dims_only: { source: "demo.public.dims_only" } }, + }); + + const fetcher = async () => + mockDescribeResponse({ + columns: [{ name: "region", type: "STRING", is_measure: false }], + }); + + const { schemas, failures } = await syncMetrics(resolution, fetcher); + expect(failures).toEqual([]); + expect(schemas[0].degraded).toBeUndefined(); + expect(schemas[0].measures).toEqual([]); + expect(schemas[0].dimensions.map((d) => d.name)).toEqual(["region"]); + }); +}); + describe("generateMetricTypeDeclarations — snapshot", () => { test("emits a stable MetricRegistry augmentation for a mixed sp + obo input", async () => { const resolution = resolveMetricConfig({ @@ -602,6 +710,50 @@ describe("generateMetricTypeDeclarations — snapshot", () => { expect(output).toMatchSnapshot(); }); + // ── Degraded-open rendering: a degraded entry opens up (string unions, + // permissive row) while a confirmed-empty entry keeps accurate `never` + // unions — the two must never be conflated. + test("emits permissive types for a degraded entry and accurate empty unions for a confirmed-empty entry", async () => { + const resolution = resolveMetricConfig({ + metricViews: { + cold_metric: { source: "appkit_demo.public.cold_metric" }, + dims_only: { + source: "appkit_demo.public.dims_only", + executor: "user", + }, + }, + }); + + const fetcher = async ( + fqn: string, + ): Promise => + fqn.endsWith("cold_metric") + ? // Stopped/cold warehouse: wait_timeout elapsed → non-terminal, no rows. + { statement_id: "stmt-mock", status: { state: "PENDING" } } + : // Genuinely measure-less view: SUCCEEDED with dimension columns only. + mockDescribeResponse({ + columns: [{ name: "region", type: "STRING", is_measure: false }], + }); + + const { schemas, failures } = await syncMetrics(resolution, fetcher); + expect(failures).toEqual([]); + const output = generateMetricTypeDeclarations(schemas); + expect(output).toMatchSnapshot(); + + // Sanity assertions in addition to the snapshot, so future drift surfaces + // even when snapshots are blindly updated. Degraded entry → permissive: + // unions accept any string, row contributions are Record. + expect(output).toContain("measureKeys: string"); + expect(output).toContain("dimensionKeys: string"); + expect(output).toContain("timeGrains: string"); + expect(output).toContain("measures: Record"); + expect(output).toContain("dimensions: Record"); + // Confirmed-empty entry → accurate: zero measures stay `never`-style. + expect(output).toContain("measureKeys: never"); + expect(output).toContain('dimensionKeys: "region"'); + expect(output).toContain("measures: Record"); + }); + // ── Phase 2: time-typed dim + multiple non-time dims fixture ───────── test("emits TimeGrain union for a metric view with time-typed + regular dimensions", async () => { const resolution = resolveMetricConfig({ @@ -1027,6 +1179,29 @@ describe("buildMetricsMetadataBundle", () => { expect(arr.time_grain).toBeUndefined(); }); + test("degraded schemas emit empty maps and never leak the degraded flag", async () => { + const resolution = resolveMetricConfig({ + metricViews: { cold_metric: { source: "demo.public.cold_metric" } }, + }); + + // Non-terminal DESCRIBE (cold warehouse) → degraded schema. + const fetcher = + async (): Promise => ({ + statement_id: "stmt-mock", + status: { state: "PENDING" }, + }); + + const { schemas } = await syncMetrics(resolution, fetcher); + expect(schemas[0].degraded).toBe(true); + + const bundle = buildMetricsMetadataBundle(schemas); + // The frontend-safe artifact has a locked shape: degraded keys emit + // empty maps... + expect(bundle.cold_metric).toEqual({ measures: {}, dimensions: {} }); + // ...and the degraded marker is a build-time concern that must NOT leak. + expect(bundle.cold_metric).not.toHaveProperty("degraded"); + }); + test("only emits time_grain on time-typed dimensions, never on measures", async () => { const resolution = resolveMetricConfig({ metricViews: { revenue: { source: "demo.public.revenue" } }, From 5cee6ec9aeed8f582775547ba3e8d55912c8f434 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Thu, 11 Jun 2026 18:24:40 +0200 Subject: [PATCH 04/10] feat(appkit): single WorkspaceClient per pass + blocking-mode metric preflight Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/index.ts | 132 +++++++-- .../src/type-generator/metric-registry.ts | 10 +- .../src/type-generator/tests/index.test.ts | 257 +++++++++++++++++- 3 files changed, 369 insertions(+), 30 deletions(-) diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index 677012bc..5e5461ff 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -24,16 +24,30 @@ import { removeOldGeneratedTypes, resolveProjectRoot, } from "./migration"; -import type { PreflightMode } from "./preflight"; +import { decidePreflight, type PreflightMode } from "./preflight"; import { generateQueriesFromDescribe } from "./query-registry"; import { generateServingTypes as generateServingTypesImpl } from "./serving/generator"; import type { QueryFatalError, QuerySchema, QuerySyntaxError } from "./types"; -import { getWarehouseState } from "./warehouse-status"; +import { + getWarehouseState, + startWarehouse, + waitUntilRunning, +} from "./warehouse-status"; dotenv.config(); const logger = createLogger("type-generator"); +/** + * Upper bound on how long the metric path's `blocking`-mode preflight waits + * for a warehouse to reach RUNNING (~5 min). Mirrors the query path's + * (unexported) `PREFLIGHT_WAIT_MAX_MS` in query-registry.ts; kept as a + * separate metric-local constant because the two preflights are deliberately + * split — queries and metric views may bind to different warehouses in the + * future. + */ +const METRIC_PREFLIGHT_WAIT_MAX_MS = 300_000; + type TypegenFailure = QuerySyntaxError | QueryFatalError; function plural(count: number, singular: string, pluralForm = `${singular}s`) { @@ -218,15 +232,20 @@ declare module "@databricks/appkit-ui/react" { * Uses {@link getWarehouseState} (`warehouses.get`) — a read-only GET that can * never start the warehouse — unlike the metric DESCRIBE statements it guards, * whose statement execution auto-starts a stopped warehouse and waits on it. - * Probe failures (connectivity, auth, bad id) read as "not running": in - * non-blocking mode typegen must never block on, or fail because of, the - * warehouse, so the caller degrades and a later blocking run (e.g. the Vite - * plugin's warehouse watch) lands the real schemas. + * + * Takes the metric path's lazy client *getter* (not a constructed client) so + * the probe's failure semantics cover client construction too: any failure to + * observe a state — connectivity, auth, bad id, or SDK construction — reads + * as "not running". In non-blocking mode typegen must never block on, or fail + * because of, the warehouse, so the caller degrades and a later blocking run + * (e.g. the Vite plugin's warehouse watch) lands the real schemas. */ -async function isWarehouseRunning(warehouseId: string): Promise { +async function isWarehouseRunning( + getClient: () => WorkspaceClient, + warehouseId: string, +): Promise { try { - const client = new WorkspaceClient({}); - return (await getWarehouseState(client, warehouseId)) === "RUNNING"; + return (await getWarehouseState(getClient(), warehouseId)) === "RUNNING"; } catch { return false; } @@ -241,7 +260,11 @@ async function isWarehouseRunning(warehouseId: string): Promise { * queries, `"non-blocking"` never probes or describes the warehouse. For * metric views, `"non-blocking"` makes one status-only probe and DESCRIBEs * only when the warehouse is already RUNNING (otherwise degraded artifacts - * are emitted); `"blocking"` always DESCRIBEs. Defaults to `"non-blocking"`. + * are emitted); `"blocking"` first ensures the warehouse is running — it + * waits for a starting warehouse and starts (then waits for) a stopped one, + * failing the build only for a deleted/deleting warehouse, exactly like the + * query path's fatal preflight — and then DESCRIBEs. Defaults to + * `"non-blocking"`. * @param options.metricOutFile - optional output file for the MetricRegistry * augmentation. Defaults to a sibling `metric.d.ts` file under the same * directory as `outFile`. Skipped entirely if `metric-views.json` is absent. @@ -252,9 +275,10 @@ async function isWarehouseRunning(warehouseId: string): Promise { * @param options.metricFetcher - optional DescribeFetcher used by * {@link syncMetrics}. Tests inject a mock; production builds let the * default WorkspaceClient-backed fetcher be created lazily. An injected - * fetcher always runs — it bypasses the non-blocking warehouse gate, since - * it does not hit a warehouse and skipping it would only blind the tests - * and CI runs that inject it. + * fetcher always runs — it bypasses the non-blocking warehouse gate AND the + * blocking-mode preflight, since it does not hit a warehouse: skipping it + * would only blind the tests and CI runs that inject it, and preflighting + * for it would construct an SDK client nothing needs. */ export async function generateFromEntryPoint(options: { outFile: string; @@ -306,6 +330,64 @@ export async function generateFromEntryPoint(options: { if (metricConfig) { const resolution = resolveMetricConfig(metricConfig); + // At most ONE WorkspaceClient per generation pass for the whole metric + // path: the non-blocking status probe, the blocking preflight, and the + // default DESCRIBE fetcher all share this lazily-created instance. A + // pass that never contacts the warehouse constructs zero clients: an + // injected metricFetcher covers fetching (and skips probe/preflight), + // and an empty metricViews map has nothing to describe in any mode. + let metricClient: WorkspaceClient | undefined; + const getMetricClient = (): WorkspaceClient => { + metricClient ??= new WorkspaceClient({}); + return metricClient; + }; + + // Blocking-mode preflight: ensure the warehouse is running before the + // DESCRIBE batch, mirroring the query path's flow (probe → decide → + // wait / start+wait; only DELETED/DELETING is fatal). Deliberately + // SPLIT from the query path's preflight rather than shared — queries + // and metric views may bind to different warehouses in the future. + // Two deliberate softenings versus the query preflight: a failed probe + // and a timed-out (or non-RUNNING-ending) wait are NOT fatal here. We + // fall through to syncMetrics, whose DESCRIBEs classify a still-not- + // ready warehouse as degraded (permissive types, refreshed by a later + // run) rather than failing the build. An injected metricFetcher needs + // no warehouse, so it skips the preflight entirely. + let preflightFatalMessage: string | undefined; + if ( + mode === "blocking" && + metricFetcher === undefined && + resolution.entries.length > 0 + ) { + try { + const state = await getWarehouseState(getMetricClient(), warehouseId); + const decision = decidePreflight(state, mode); + if (decision === "fatal") { + preflightFatalMessage = `warehouse ${warehouseId} is ${state}`; + } else if (decision === "startWaitProceed") { + // Stopped/stopping: nudge it awake, then poll to RUNNING. + // treatStoppedAsTransient rides out the stale pre-start + // STOPPED/STOPPING reading, same as the query preflight. + await startWarehouse(getMetricClient(), warehouseId); + await waitUntilRunning(getMetricClient(), warehouseId, { + maxMs: METRIC_PREFLIGHT_WAIT_MAX_MS, + treatStoppedAsTransient: true, + }); + } else if (decision === "waitThenProceed") { + await waitUntilRunning(getMetricClient(), warehouseId, { + maxMs: METRIC_PREFLIGHT_WAIT_MAX_MS, + }); + } + // "proceed" — and a wait that resolved non-RUNNING — falls through + // to syncMetrics below. + } catch { + // Probe/start failure or a wait that timed out: fall through to + // syncMetrics. DESCRIBEs against a not-ready warehouse come back + // non-terminal and classify as degraded — never thrown — so the + // build still writes both artifacts. + } + } + // Honor the non-blocking preflight contract (#406) for metric DESCRIBEs // too: each `DESCRIBE TABLE EXTENDED ... AS JSON` waits up to 30s per // key and auto-starts a stopped warehouse — exactly what "non-blocking" @@ -320,13 +402,31 @@ export async function generateFromEntryPoint(options: { metricFetcher !== undefined || mode !== "non-blocking" || resolution.entries.length === 0 || - (await isWarehouseRunning(warehouseId)); + (await isWarehouseRunning(getMetricClient, warehouseId)); let metricSchemas: MetricSchema[]; let failures: MetricSyncFailure[] = []; - if (describeNow) { + if (preflightFatalMessage !== undefined) { + // Fatal preflight (deleted/deleting warehouse): fail exactly like the + // query path's fatal preflight — skip the DESCRIBE batch, emit + // degraded schemas so both artifacts are still written, and record + // one fatal error per key. The shared end-of-run throw below + // (TypegenFatalError, or TypegenSyntaxError's fatalQueries when + // syntax errors coexist) surfaces them after the writes, identically + // to query fatals. + metricSchemas = resolution.entries.map(emptyMetricSchema); + for (const entry of resolution.entries) { + fatalErrors.push({ name: entry.key, message: preflightFatalMessage }); + } + } else if (resolution.entries.length === 0) { + // Nothing configured to describe: syncMetrics would be a no-op, and + // building its default fetcher would construct a client for nothing. + // Emit the (empty) artifacts directly. + metricSchemas = []; + } else if (describeNow) { const fetcher = - metricFetcher ?? createWorkspaceDescribeFetcher(warehouseId); + metricFetcher ?? + createWorkspaceDescribeFetcher(getMetricClient(), warehouseId); ({ schemas: metricSchemas, failures } = await syncMetrics( resolution, fetcher, diff --git a/packages/appkit/src/type-generator/metric-registry.ts b/packages/appkit/src/type-generator/metric-registry.ts index aca81812..6bfdd2a0 100644 --- a/packages/appkit/src/type-generator/metric-registry.ts +++ b/packages/appkit/src/type-generator/metric-registry.ts @@ -1,6 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { WorkspaceClient } from "@databricks/sdk-experimental"; +import type { WorkspaceClient } from "@databricks/sdk-experimental"; import type { DatabricksStatementExecutionResponse } from "./types"; /** @@ -1023,7 +1023,11 @@ export type DescribeFetcher = ( /** * Build a DescribeFetcher from a real WorkspaceClient + warehouseId. * - * Kept narrow so it does not require importing the SDK at test time. + * The client is supplied by the caller rather than constructed here: + * `generateFromEntryPoint` keeps at most ONE WorkspaceClient per generation + * pass for the whole metric path (status probe, blocking preflight, and this + * fetcher all share it). Type-only SDK import keeps this module free of the + * SDK at test time. * * `wait_timeout: "30s"` makes the API wait synchronously for the statement * to complete (matching the SDK's own example pattern). Without an explicit @@ -1034,9 +1038,9 @@ export type DescribeFetcher = ( * the symptom we hit on a cold warehouse. */ export function createWorkspaceDescribeFetcher( + client: WorkspaceClient, warehouseId: string, ): DescribeFetcher { - const client = new WorkspaceClient({}); return async (fqn: string) => { const result = (await client.statementExecution.executeStatement({ statement: `DESCRIBE TABLE EXTENDED ${fqn} AS JSON`, diff --git a/packages/appkit/src/type-generator/tests/index.test.ts b/packages/appkit/src/type-generator/tests/index.test.ts index a86f001c..297d12fa 100644 --- a/packages/appkit/src/type-generator/tests/index.test.ts +++ b/packages/appkit/src/type-generator/tests/index.test.ts @@ -14,6 +14,8 @@ import type { DatabricksStatementExecutionResponse } from "../types"; const mocks = vi.hoisted(() => ({ generateQueriesFromDescribe: vi.fn(), getWarehouseState: vi.fn(), + startWarehouse: vi.fn(), + waitUntilRunning: vi.fn(), executeStatement: vi.fn(), })); @@ -27,27 +29,33 @@ vi.mock("../query-registry", async (importOriginal) => { }; }); -// The metric gate's status-only probe resolves through getWarehouseState; stub -// it so tests dictate the observed warehouse state. Keep the rest of the module -// actual — query-registry imports startWarehouse/waitUntilRunning from it. +// The metric gate's status-only probe and the metric blocking preflight +// resolve through getWarehouseState/startWarehouse/waitUntilRunning; stub all +// three so tests dictate the observed warehouse lifecycle. (query-registry +// also imports these, but its describe step is fully mocked above, so the +// stubs only ever serve the metric path here.) vi.mock("../warehouse-status", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, getWarehouseState: mocks.getWarehouseState, + startWarehouse: mocks.startWarehouse, + waitUntilRunning: mocks.waitUntilRunning, }; }); -// index.ts constructs `new WorkspaceClient({})` for the metric probe and (via -// createWorkspaceDescribeFetcher) for the default DESCRIBE fetcher. Stub the -// SDK so neither needs real credentials; `executeStatement` doubles as the -// "was any metric DESCRIBE actually issued?" spy. +// index.ts lazily constructs at most ONE `new WorkspaceClient({})` per pass +// for the whole metric path (status probe + blocking preflight + default +// DESCRIBE fetcher share it). Stub the SDK so no real credentials are needed; +// `executeStatement` doubles as the "was any metric DESCRIBE actually +// issued?" spy and the constructor mock doubles as the client-count spy. vi.mock("@databricks/sdk-experimental", () => ({ WorkspaceClient: vi.fn(() => ({ statementExecution: { executeStatement: mocks.executeStatement }, })), })); +const { WorkspaceClient } = await import("@databricks/sdk-experimental"); const { generateFromEntryPoint, TypegenFatalError, TypegenSyntaxError } = await import("../index"); @@ -498,11 +506,36 @@ describe("generateFromEntryPoint — metric-view emission", () => { expect(bundle.revenue.measures.total_revenue.type).toBe("DECIMAL(38,2)"); }); - test("blocking mode: DESCRIBEs are attempted without probing warehouse state", async () => { + // ── Blocking-mode preflight: mirrors the query path's ensure-running flow ── + + test("blocking + RUNNING: one preflight probe, no start/wait, DESCRIBEs run", async () => { + writeMetricConfig(); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue(describeResponse); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "blocking", + }), + ).resolves.toBeUndefined(); + + expect(mocks.getWarehouseState).toHaveBeenCalledTimes(1); + expect(mocks.startWarehouse).not.toHaveBeenCalled(); + expect(mocks.waitUntilRunning).not.toHaveBeenCalled(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + expect(fs.readFileSync(metricFile, "utf-8")).toContain( + '"total_revenue": number', + ); + }); + + test("blocking + STOPPED: preflight starts the warehouse and waits for RUNNING before DESCRIBEs", async () => { writeMetricConfig(); - // Even a stopped warehouse is never probed in blocking mode — the - // DESCRIBE itself owns warehouse readiness (it waits / auto-starts). mocks.getWarehouseState.mockResolvedValue("STOPPED"); + mocks.startWarehouse.mockResolvedValue(undefined); + mocks.waitUntilRunning.mockResolvedValue("RUNNING"); mocks.executeStatement.mockResolvedValue(describeResponse); await expect( @@ -514,13 +547,215 @@ describe("generateFromEntryPoint — metric-view emission", () => { }), ).resolves.toBeUndefined(); - expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + // Same flow as the query preflight: probe → start → wait → DESCRIBE, + // with the start-induced stale STOPPED reading treated as transient. + expect(mocks.getWarehouseState).toHaveBeenCalledTimes(1); + expect(mocks.startWarehouse).toHaveBeenCalledWith( + expect.anything(), + "wh-1", + ); + expect(mocks.waitUntilRunning).toHaveBeenCalledWith( + expect.anything(), + "wh-1", + expect.objectContaining({ + maxMs: 300_000, + treatStoppedAsTransient: true, + }), + ); expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + + const order = [ + mocks.getWarehouseState.mock.invocationCallOrder[0], + mocks.startWarehouse.mock.invocationCallOrder[0], + mocks.waitUntilRunning.mock.invocationCallOrder[0], + mocks.executeStatement.mock.invocationCallOrder[0], + ]; + expect(order).toEqual([...order].sort((a, b) => a - b)); + expect(fs.readFileSync(metricFile, "utf-8")).toContain( '"total_revenue": number', ); }); + test("blocking + DELETED: fails through the query path's fatal pathway (TypegenFatalError after artifacts are written)", async () => { + writeMetricConfig(); + mocks.getWarehouseState.mockResolvedValue("DELETED"); + + const error = await generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "blocking", + }).then( + () => { + throw new Error("expected generateFromEntryPoint to reject"); + }, + (err: unknown) => err, + ); + + // Identical surfacing to a query-path fatal preflight: same error class, + // same per-name fatal entries, same message template. + expect(error).toBeInstanceOf(TypegenFatalError); + expect((error as InstanceType).queries).toEqual([ + { name: "revenue", message: "warehouse wh-1 is DELETED" }, + ]); + + // A deleted warehouse is never started, waited on, or described. + expect(mocks.startWarehouse).not.toHaveBeenCalled(); + expect(mocks.waitUntilRunning).not.toHaveBeenCalled(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + + // Write-first semantics match query fatals: degraded artifacts exist. + const declarations = fs.readFileSync(metricFile, "utf-8"); + expect(declarations).toContain('"revenue"'); + expect(declarations).toContain("measureKeys: string"); + const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); + }); + + test.each([ + [ + "rejects with a timeout", + () => + mocks.waitUntilRunning.mockRejectedValue( + new Error( + "Warehouse wh-1 did not reach RUNNING within 300000ms (last state: STARTING)", + ), + ), + ], + [ + "resolves non-RUNNING", + () => mocks.waitUntilRunning.mockResolvedValue("STOPPED"), + ], + ])( + "blocking + preflight wait %s: generation does not throw, keys degrade", + async (_label, armWait) => { + writeMetricConfig(); + mocks.getWarehouseState.mockResolvedValue("STARTING"); + armWait(); + // The fall-through DESCRIBE hits a still-cold warehouse: non-terminal + // response, which classifies as degraded (never an error). + mocks.executeStatement.mockResolvedValue({ + statement_id: "stmt-mock", + status: { state: "PENDING" }, + }); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + try { + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "blocking", + }), + ).resolves.toBeUndefined(); + + // Degraded, not failed: no per-key warns, one info summary line. + const warned = warnSpy.mock.calls.flat().map(String).join("\n"); + expect(warned).not.toContain("metric sync failed"); + const degradedLines = logSpy.mock.calls + .map((call) => call.map(String).join(" ")) + .filter((line) => line.includes("degraded metric types")); + expect(degradedLines).toHaveLength(1); + } finally { + warnSpy.mockRestore(); + logSpy.mockRestore(); + } + + // STARTING → wait-only (no start), without treatStoppedAsTransient. + expect(mocks.startWarehouse).not.toHaveBeenCalled(); + expect(mocks.waitUntilRunning).toHaveBeenCalledWith( + expect.anything(), + "wh-1", + expect.objectContaining({ maxMs: 300_000 }), + ); + expect( + mocks.waitUntilRunning.mock.calls[0][2].treatStoppedAsTransient, + ).toBeUndefined(); + // The DESCRIBE batch still ran (fall-through), and its non-terminal + // answer degraded the key per Phase 1 semantics. + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); + expect(fs.readFileSync(metricFile, "utf-8")).toContain( + "measureKeys: string", + ); + }, + ); + + test("blocking + injected metricFetcher: no preflight calls and no WorkspaceClient construction", async () => { + writeMetricConfig(); + mocks.getWarehouseState.mockResolvedValue("STOPPED"); + const fetcher = vi.fn(async () => describeResponse); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "blocking", + metricFetcher: fetcher, + }), + ).resolves.toBeUndefined(); + + // The injected fetcher needs no warehouse: zero preflight round-trips + // and zero SDK clients for the whole pass. + expect(fetcher).toHaveBeenCalledTimes(1); + expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + expect(mocks.startWarehouse).not.toHaveBeenCalled(); + expect(mocks.waitUntilRunning).not.toHaveBeenCalled(); + expect(vi.mocked(WorkspaceClient)).not.toHaveBeenCalled(); + expect(fs.readFileSync(metricFile, "utf-8")).toContain( + '"total_revenue": number', + ); + }); + + // ── One WorkspaceClient per generation pass ── + + test("non-blocking + RUNNING with the default fetcher: probe and DESCRIBEs share exactly one client", async () => { + writeMetricConfig(); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue(describeResponse); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "non-blocking", + }), + ).resolves.toBeUndefined(); + + // The pass both probed AND described — one shared client total. + expect(mocks.getWarehouseState).toHaveBeenCalledTimes(1); + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + expect(vi.mocked(WorkspaceClient)).toHaveBeenCalledTimes(1); + }); + + test("empty metricViews map: no probe, no preflight, no client — empty artifacts still ship", async () => { + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + JSON.stringify({ metricViews: {} }), + ); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "blocking", + }), + ).resolves.toBeUndefined(); + + expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + expect(vi.mocked(WorkspaceClient)).not.toHaveBeenCalled(); + expect(fs.existsSync(metricFile)).toBe(true); + expect(JSON.parse(fs.readFileSync(metadataFile, "utf-8"))).toEqual({}); + }); + test("an injected metricFetcher bypasses the gate even when non-blocking + stopped", async () => { writeMetricConfig(); mocks.getWarehouseState.mockResolvedValue("STOPPED"); From f02fe5a73a38d5dce91f3856fbbd73df0d0cfc66 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Thu, 11 Jun 2026 18:34:44 +0200 Subject: [PATCH 05/10] feat(appkit): bounded-concurrency metric DESCRIBEs in syncMetrics Signed-off-by: Atila Fassina --- .../src/type-generator/metric-registry.ts | 140 +++++++++-- .../tests/metric-registry.test.ts | 225 ++++++++++++++++++ 2 files changed, 339 insertions(+), 26 deletions(-) diff --git a/packages/appkit/src/type-generator/metric-registry.ts b/packages/appkit/src/type-generator/metric-registry.ts index 6bfdd2a0..74cfe3d2 100644 --- a/packages/appkit/src/type-generator/metric-registry.ts +++ b/packages/appkit/src/type-generator/metric-registry.ts @@ -1101,12 +1101,43 @@ export function emptyMetricSchema( }; } +/** + * Maximum number of in-flight DESCRIBE statements per {@link syncMetrics} + * pass. Mirrors the query path's (unexported) default `concurrency = 10` in + * query-registry.ts (`generateQueriesFromDescribe`); kept as a separate + * metric-local constant because the two describe pipelines are deliberately + * split — queries and metric views may bind to different warehouses in the + * future. + */ +const METRIC_DESCRIBE_CONCURRENCY = 10; + +/** + * Outcome of describing a single metric entry, tagged with the entry's + * position in `resolution.entries` so chunked, out-of-order completion can + * be reassembled into config order. `failure` is present only for genuine + * failures (rejected fetch, FAILED statement, unparseable response, zero + * columns) — a degraded-but-not-failed schema (non-terminal state) carries + * no failure. + */ +interface MetricDescribeOutcome { + index: number; + schema: MetricSchema; + failure?: MetricSyncFailure; +} + /** * Run schema synchronization for every entry in `metric-views.json`. * * `fetcher` is injected so the same code path serves Vite, the CLI, and unit * tests with a mock that returns a representative DESCRIBE response. * + * Entries are described with bounded concurrency: chunks of + * {@link METRIC_DESCRIBE_CONCURRENCY} run via `Promise.allSettled`, the next + * chunk starting only after the previous one fully settles (the query path's + * batching in query-registry). Results are placed by entry index, so + * `schemas` (and `failures`) always come back in `resolution.entries` order + * regardless of completion order. + * * Returns `{ schemas, failures }`. The schemas array always carries one * entry per registered metric. Classification mirrors the query path * (query-registry's describe flow): @@ -1134,18 +1165,27 @@ export async function syncMetrics( resolution: MetricConfigResolution, fetcher: DescribeFetcher, ): Promise { - const schemas: MetricSchema[] = []; - const failures: MetricSyncFailure[] = []; - - for (const entry of resolution.entries) { + const { entries } = resolution; + // Index-keyed slots: every entry writes exactly one schema slot (and at + // most one failure slot), so output order equals config order no matter + // which DESCRIBE settles first. + const schemas = new Array(entries.length); + const failureSlots = new Array(entries.length); + + const describeOne = async ( + entry: ResolvedMetricEntry, + index: number, + ): Promise => { let response: DatabricksStatementExecutionResponse; try { response = await fetcher(entry.source); } catch (err) { const reason = `DESCRIBE TABLE EXTENDED failed: ${(err as Error).message}`; - failures.push({ key: entry.key, source: entry.source, reason }); - schemas.push(emptyMetricSchema(entry)); - continue; + return { + index, + schema: emptyMetricSchema(entry), + failure: { key: entry.key, source: entry.source, reason }, + }; } // Non-terminal statement state (PENDING/RUNNING): the warehouse is @@ -1157,8 +1197,7 @@ export async function syncMetrics( // wrong-FQN failure. const state = response.status?.state; if (state !== "SUCCEEDED" && state !== "FAILED") { - schemas.push(emptyMetricSchema(entry)); - continue; + return { index, schema: emptyMetricSchema(entry) }; } let columns: MetricColumnMetadata[] = []; @@ -1171,13 +1210,11 @@ export async function syncMetrics( } if (parseError) { - failures.push({ - key: entry.key, - source: entry.source, - reason: parseError, - }); - schemas.push(emptyMetricSchema(entry)); - continue; + return { + index, + schema: emptyMetricSchema(entry), + failure: { key: entry.key, source: entry.source, reason: parseError }, + }; } if (columns.length === 0) { @@ -1189,22 +1226,73 @@ export async function syncMetrics( // degraded: its real columns are unknown. const reason = "DESCRIBE response yielded zero columns — check the response shape (top-level `columns` array or `schema.fields`)."; - failures.push({ key: entry.key, source: entry.source, reason }); - schemas.push(emptyMetricSchema(entry)); - continue; + return { + index, + schema: emptyMetricSchema(entry), + failure: { key: entry.key, source: entry.source, reason }, + }; } const measures = columns.filter((c) => c.isMeasure); const dimensions = columns.filter((c) => !c.isMeasure); - schemas.push({ - key: entry.key, - source: entry.source, - lane: entry.lane, - measures, - dimensions, - }); + return { + index, + schema: { + key: entry.key, + source: entry.source, + lane: entry.lane, + measures, + dimensions, + }, + }; + }; + + for ( + let offset = 0; + offset < entries.length; + offset += METRIC_DESCRIBE_CONCURRENCY + ) { + // The final slice is naturally partial: slice() clamps to entries.length. + const slice = entries.slice(offset, offset + METRIC_DESCRIBE_CONCURRENCY); + const settled = await Promise.allSettled( + slice.map((entry, i) => describeOne(entry, offset + i)), + ); + + for (let i = 0; i < settled.length; i++) { + const result = settled[i]; + if (result.status === "fulfilled") { + const { index, schema, failure } = result.value; + schemas[index] = schema; + if (failure) { + failureSlots[index] = failure; + } + } else { + // describeOne catches every expected failure internally, so a + // rejected settlement should be impossible — but handle it + // defensively (the query path's processBatchResults does the same) + // so one entry's surprise throw degrades only that entry, never its + // siblings. + const index = offset + i; + const entry = entries[index]; + const message = + result.reason instanceof Error + ? result.reason.message + : String(result.reason); + schemas[index] = emptyMetricSchema(entry); + failureSlots[index] = { + key: entry.key, + source: entry.source, + reason: `DESCRIBE TABLE EXTENDED failed: ${message}`, + }; + } + } } + // Compact the slots: failures come out ordered by entry index. + const failures = failureSlots.filter( + (failure): failure is MetricSyncFailure => failure !== undefined, + ); + return { schemas, failures }; } diff --git a/packages/appkit/src/type-generator/tests/metric-registry.test.ts b/packages/appkit/src/type-generator/tests/metric-registry.test.ts index 3a94d9c4..d8790c87 100644 --- a/packages/appkit/src/type-generator/tests/metric-registry.test.ts +++ b/packages/appkit/src/type-generator/tests/metric-registry.test.ts @@ -644,6 +644,231 @@ describe("syncMetrics — DESCRIBE state classification", () => { }); }); +// ── Bounded-concurrency scheduling (parity with query-registry's chunked +// DESCRIBE batching): entries run in waves of at most 10 via +// Promise.allSettled, the next wave starting only once the previous one has +// fully settled, and results are reassembled into config order by entry +// index. Per-entry classification semantics are unchanged — only the +// scheduling moved from a sequential loop to chunks. +describe("syncMetrics — bounded-concurrency scheduling", () => { + /** Manually-resolvable gate for controlling fetcher completion order. */ + function gate() { + let open: () => void = () => {}; + const opened = new Promise((resolve) => { + open = resolve; + }); + return { opened, open }; + } + + /** Drain microtasks + one macrotask so settled fetches fully propagate. */ + const flush = () => new Promise((resolve) => setTimeout(resolve, 0)); + + /** Build a resolution with `count` entries m01, m02, ... (config order). */ + function manyEntries(count: number) { + const keys = Array.from( + { length: count }, + (_, i) => `m${String(i + 1).padStart(2, "0")}`, + ); + const resolution = resolveMetricConfig({ + metricViews: Object.fromEntries( + keys.map((key) => [key, { source: `demo.public.${key}` }]), + ), + }); + // Precondition: resolution preserves the sorted key order. + expect(resolution.entries.map((e) => e.key)).toEqual(keys); + return { keys, resolution }; + } + + const keyOf = (fqn: string) => fqn.split(".")[2]; + + test("schemas come back in config order even when DESCRIBEs resolve out of order", async () => { + const { keys, resolution } = manyEntries(12); + + const gates = new Map>(); + const fetcher = async (fqn: string) => { + const g = gate(); + gates.set(keyOf(fqn), g); + await g.opened; + return mockDescribeResponse({ + columns: [ + { name: `${keyOf(fqn)}_total`, type: "BIGINT", is_measure: true }, + ], + }); + }; + + const resultPromise = syncMetrics(resolution, fetcher); + + // Wave 1 (first 10 entries) is in flight; release it back-to-front so + // later entries complete before earlier ones. + await flush(); + expect([...gates.keys()]).toEqual(keys.slice(0, 10)); + for (const key of [...keys.slice(0, 10)].reverse()) { + gates.get(key)?.open(); + await flush(); // let this fetch fully settle before releasing the next + } + + // Wave 2 (the partial final slice) — release back-to-front too. + await flush(); + expect([...gates.keys()].slice(10)).toEqual(keys.slice(10)); + for (const key of [...keys.slice(10)].reverse()) { + gates.get(key)?.open(); + await flush(); + } + + const { schemas, failures } = await resultPromise; + expect(failures).toEqual([]); + // Output order equals config order, not completion order. + expect(schemas.map((s) => s.key)).toEqual(keys); + // Each slot carries its own entry's schema, not merely the right key. + expect(schemas.map((s) => s.measures[0]?.name)).toEqual( + keys.map((key) => `${key}_total`), + ); + }); + + test("in-flight DESCRIBEs are capped at 10; a second wave starts only after the first settles", async () => { + const { keys, resolution } = manyEntries(12); + + let inFlight = 0; + let maxInFlight = 0; + const started: string[] = []; + const gates: Array> = []; + + const fetcher = async (fqn: string) => { + started.push(keyOf(fqn)); + inFlight++; + maxInFlight = Math.max(maxInFlight, inFlight); + const g = gate(); + gates.push(g); + try { + await g.opened; + return mockDescribeResponse({ + columns: [{ name: "total", type: "BIGINT", is_measure: true }], + }); + } finally { + inFlight--; + } + }; + + const resultPromise = syncMetrics(resolution, fetcher); + + // All 10 wave-1 gates held: calls 11-12 must NOT have started yet. + await flush(); + await flush(); + expect(started).toEqual(keys.slice(0, 10)); + expect(inFlight).toBe(10); + + // Release wave 1 → only then does wave 2 (the partial slice) start. + for (const g of gates.slice(0, 10)) { + g.open(); + } + await flush(); + expect(started).toEqual(keys); + + for (const g of gates.slice(10)) { + g.open(); + } + const { schemas, failures } = await resultPromise; + + expect(failures).toEqual([]); + expect(schemas).toHaveLength(12); + // The full wave ran in parallel, but never beyond the bound. + expect(maxInFlight).toBe(10); + }); + + test("one entry's rejection or non-terminal state never affects its siblings", async () => { + const { keys, resolution } = manyEntries(12); + const rejecting = new Set(["m02", "m06", "m11"]); + const nonTerminal = "m04"; + + const fetcher = async ( + fqn: string, + ): Promise => { + const key = keyOf(fqn); + if (rejecting.has(key)) { + throw new Error(`boom ${key}`); + } + if (key === nonTerminal) { + return { statement_id: "stmt-mock", status: { state: "PENDING" } }; + } + return mockDescribeResponse({ + columns: [ + { name: `${key}_total`, type: "BIGINT", is_measure: true }, + { name: "region", type: "STRING", is_measure: false }, + ], + }); + }; + + const { schemas, failures } = await syncMetrics(resolution, fetcher); + + // Order preserved across the mixed batch. + expect(schemas.map((s) => s.key)).toEqual(keys); + + // Rejected entries land in `failures` (stable entry order) AND are + // degraded — the Phase-1 matrix, unchanged by chunking. + expect(failures.map((f) => f.key)).toEqual(["m02", "m06", "m11"]); + for (const failure of failures) { + expect(failure.source).toBe(`demo.public.${failure.key}`); + expect(failure.reason).toBe( + `DESCRIBE TABLE EXTENDED failed: boom ${failure.key}`, + ); + } + + for (const schema of schemas) { + if (rejecting.has(schema.key) || schema.key === nonTerminal) { + // Degraded: empty allowlists drive permissive rendering downstream. + expect(schema.degraded).toBe(true); + expect(schema.measures).toEqual([]); + expect(schema.dimensions).toEqual([]); + } else { + // Siblings keep their real DESCRIBE results. + expect(schema.degraded).toBeUndefined(); + expect(schema.measures.map((m) => m.name)).toEqual([ + `${schema.key}_total`, + ]); + expect(schema.dimensions.map((d) => d.name)).toEqual(["region"]); + } + } + }); + + test("a per-entry helper throw (poisoned response object) degrades only that entry", async () => { + // Fetcher rejections are caught inside the per-entry helper, so a + // rejected settlement is normally impossible. Force one anyway: a + // response whose property access throws blows up AFTER the fetch + // try/catch, rejecting the helper's promise — the defensive + // rejected-settlement branch must contain it to this entry alone. + const { keys, resolution } = manyEntries(3); + + const poisoned = new Proxy({} as DatabricksStatementExecutionResponse, { + get(_target, prop) { + if (prop === "then") { + return undefined; // keep the object await-able + } + throw new Error("poisoned response object"); + }, + }); + + const fetcher = async (fqn: string) => + keyOf(fqn) === "m02" + ? poisoned + : mockDescribeResponse({ + columns: [{ name: "total", type: "BIGINT", is_measure: true }], + }); + + const { schemas, failures } = await syncMetrics(resolution, fetcher); + + expect(schemas.map((s) => s.key)).toEqual(keys); + expect(schemas[0].degraded).toBeUndefined(); + expect(schemas[1].degraded).toBe(true); + expect(schemas[2].degraded).toBeUndefined(); + expect(failures).toHaveLength(1); + expect(failures[0]).toMatchObject({ + key: "m02", + source: "demo.public.m02", + }); + expect(failures[0].reason).toMatch(/poisoned response object/); + }); +}); + describe("generateMetricTypeDeclarations — snapshot", () => { test("emits a stable MetricRegistry augmentation for a mixed sp + obo input", async () => { const resolution = resolveMetricConfig({ From 3f426e77d9265cbb62737302a145432ae1368d6b Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Thu, 11 Jun 2026 19:00:16 +0200 Subject: [PATCH 06/10] feat(appkit): metrics describe-cache with retry-driven convergence and last-known-good degradation Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/cache.ts | 26 ++ packages/appkit/src/type-generator/index.ts | 148 ++++++-- .../src/type-generator/tests/index.test.ts | 354 ++++++++++++++++++ 3 files changed, 504 insertions(+), 24 deletions(-) diff --git a/packages/appkit/src/type-generator/cache.ts b/packages/appkit/src/type-generator/cache.ts index 1a91daf5..c6fb6005 100644 --- a/packages/appkit/src/type-generator/cache.ts +++ b/packages/appkit/src/type-generator/cache.ts @@ -2,6 +2,7 @@ import crypto from "node:crypto"; import fs from "node:fs/promises"; import path from "node:path"; import { createLogger } from "../logging/logger"; +import type { MetricSchema } from "./metric-registry"; const logger = createLogger("type-generator:cache"); @@ -16,14 +17,39 @@ interface CacheEntry { retry: boolean; } +/** + * One cached metric-view DESCRIBE outcome. + * + * `hash` is md5 over `"|"` — the two config inputs that + * determine a DESCRIBE — so editing either invalidates the entry. `schema` + * is the full {@link MetricSchema} persisted verbatim (it is JSON-safe by + * design), letting a warm pass regenerate both metric artifacts without a + * single warehouse call. `retry: true` marks a degraded outcome (DESCRIBE + * skipped, unanswered, or failed): the cached schema still renders + * artifacts, but the next eligible pass re-describes exactly these keys so + * degraded schemas converge to real ones. + */ +export interface MetricCacheEntry { + hash: string; + schema: MetricSchema; + retry: boolean; +} + /** * Cache interface * @property version - the version of the cache * @property queries - the queries in the cache + * @property metrics - cached metric-view schemas keyed by metric key. + * OPTIONAL on purpose: version "3" files written before this section + * existed load unchanged (absent ⇒ treated as empty by the metric path), + * and the query path's `noCache` reinit literal stays valid as-is. The + * section rides through the query path's load → mutate → save cycle as a + * plain sibling key, so query-side saves preserve it byte-for-byte. */ interface Cache { version: string; queries: Record; + metrics?: Record; } export const CACHE_VERSION = "3"; diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index 5e5461ff..a54ca6fc 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -4,6 +4,7 @@ import { WorkspaceClient } from "@databricks/sdk-experimental"; import dotenv from "dotenv"; import pc from "picocolors"; import { createLogger } from "../logging/logger"; +import { hashSQL, loadCache, type MetricCacheEntry, saveCache } from "./cache"; import { createWorkspaceDescribeFetcher, type DescribeFetcher, @@ -256,6 +257,10 @@ async function isWarehouseRunning( * @param options - the options for the generation * @param options.entryPoint - the entry point file * @param options.outFile - the output file + * @param options.noCache - skip the typegen cache entirely: every query is + * re-described, and the metric path ignores its cached schemas (every + * configured key becomes describe-needed) and overwrites the cache's + * `metrics` section with this pass's results. * @param options.mode - preflight policy (see {@link PreflightMode}). For * queries, `"non-blocking"` never probes or describes the warehouse. For * metric views, `"non-blocking"` makes one status-only probe and DESCRIBEs @@ -330,12 +335,61 @@ export async function generateFromEntryPoint(options: { if (metricConfig) { const resolution = resolveMetricConfig(metricConfig); + // Metric schemas persist in the shared typegen cache as a `metrics` + // section (sibling of `queries`, same file, same version) keyed by + // metric key with md5("|") as the change detector. The + // cache is (re)loaded here — strictly AFTER generateQueriesFromDescribe + // above has finished its own load → mutate → save cycle — so the single + // metric-side save below re-serializes the exact `queries` object it + // just read and can never clobber a query entry. + const cache = await loadCache(); + + // The section is consumed through a null-prototype copy: metric keys + // are user-controlled config input and "__proto__" passes the metric + // key regex — on a plain object, writing it would hit the + // Object.prototype setter (mutating the object's prototype and silently + // dropping the entry) instead of storing data. A null prototype also + // keeps partition reads from resolving inherited names ("constructor", + // "toString", ...) as phantom entries. + const metricsSection: Record = + Object.create(null); + if (!noCache && cache.metrics) { + for (const key of Object.keys(cache.metrics)) { + metricsSection[key] = cache.metrics[key]; + } + } + + // Partition BEFORE any gate/preflight decision: a hit (hash match and + // not flagged for retry) is served from cache no matter what the + // warehouse is doing — a degraded-mode pass falls back to + // last-known-good schemas exactly like queries degrade to cached + // types. Only the remainder — new keys, edited entries, and + // retry-flagged degraded entries — is eligible for DESCRIBE, so a + // fully-warm pass makes zero warehouse calls and constructs zero + // clients. `noCache` left the section empty above, which makes every + // configured key describe-needed here. + const hitSchemas = new Map(); + const describeNeeded: typeof resolution.entries = []; + // Parallel to describeNeeded: the config hash to persist per key. + const neededHashes: string[] = []; + for (const entry of resolution.entries) { + const hash = hashSQL(`${entry.source}|${entry.lane}`); + const prior = metricsSection[entry.key]; + if (prior && prior.hash === hash && !prior.retry) { + hitSchemas.set(entry.key, prior.schema); + } else { + describeNeeded.push(entry); + neededHashes.push(hash); + } + } + // At most ONE WorkspaceClient per generation pass for the whole metric // path: the non-blocking status probe, the blocking preflight, and the // default DESCRIBE fetcher all share this lazily-created instance. A // pass that never contacts the warehouse constructs zero clients: an // injected metricFetcher covers fetching (and skips probe/preflight), - // and an empty metricViews map has nothing to describe in any mode. + // and a pass with nothing describe-needed — fully-warm cache or an + // empty metricViews map — has nothing to describe in any mode. let metricClient: WorkspaceClient | undefined; const getMetricClient = (): WorkspaceClient => { metricClient ??= new WorkspaceClient({}); @@ -357,7 +411,7 @@ export async function generateFromEntryPoint(options: { if ( mode === "blocking" && metricFetcher === undefined && - resolution.entries.length > 0 + describeNeeded.length > 0 ) { try { const state = await getWarehouseState(getMetricClient(), warehouseId); @@ -395,40 +449,43 @@ export async function generateFromEntryPoint(options: { // start the warehouse) decides whether to describe now or emit degraded // artifacts that a later blocking run refreshes. An injected // metricFetcher always runs: it doesn't hit a warehouse (tests/CI - // inject mocks), so gating it would only skip meaningful work. An empty - // metricViews map needs no probe either — nothing would be described + // inject mocks), so gating it would only skip meaningful work. A pass + // with nothing describe-needed — fully-warm cache or an empty + // metricViews map — needs no probe either: nothing would be described // in any mode. const describeNow = metricFetcher !== undefined || mode !== "non-blocking" || - resolution.entries.length === 0 || + describeNeeded.length === 0 || (await isWarehouseRunning(getMetricClient, warehouseId)); - let metricSchemas: MetricSchema[]; + let described: MetricSchema[]; let failures: MetricSyncFailure[] = []; if (preflightFatalMessage !== undefined) { // Fatal preflight (deleted/deleting warehouse): fail exactly like the // query path's fatal preflight — skip the DESCRIBE batch, emit // degraded schemas so both artifacts are still written, and record - // one fatal error per key. The shared end-of-run throw below + // one fatal error per describe-needed key (cache hits are unaffected: + // they serve their cached schemas). The shared end-of-run throw below // (TypegenFatalError, or TypegenSyntaxError's fatalQueries when // syntax errors coexist) surfaces them after the writes, identically // to query fatals. - metricSchemas = resolution.entries.map(emptyMetricSchema); - for (const entry of resolution.entries) { + described = describeNeeded.map(emptyMetricSchema); + for (const entry of describeNeeded) { fatalErrors.push({ name: entry.key, message: preflightFatalMessage }); } - } else if (resolution.entries.length === 0) { - // Nothing configured to describe: syncMetrics would be a no-op, and - // building its default fetcher would construct a client for nothing. - // Emit the (empty) artifacts directly. - metricSchemas = []; + } else if (describeNeeded.length === 0) { + // Nothing left to describe — every configured key (if any) was a + // cache hit. syncMetrics would be a no-op, and building its default + // fetcher would construct a client for nothing. The artifacts below + // regenerate from cached schemas alone. + described = []; } else if (describeNow) { const fetcher = metricFetcher ?? createWorkspaceDescribeFetcher(getMetricClient(), warehouseId); - ({ schemas: metricSchemas, failures } = await syncMetrics( - resolution, + ({ schemas: described, failures } = await syncMetrics( + { entries: describeNeeded }, fetcher, )); @@ -454,7 +511,7 @@ export async function generateFromEntryPoint(options: { // unknown — not errors. One summary line, no per-key warns; failed // keys are excluded (the warn loop above already reported them). const failedKeys = new Set(failures.map((f) => f.key)); - const degradedKeys = metricSchemas + const degradedKeys = described .filter((s) => s.degraded && !failedKeys.has(s.key)) .map((s) => s.key); if (degradedKeys.length > 0) { @@ -467,20 +524,63 @@ export async function generateFromEntryPoint(options: { } } else { // Deliberately un-probed DESCRIBEs, not failures: emit every - // configured key as a degraded schema (permissive types, empty + // describe-needed key as a degraded schema (permissive types, empty // runtime allowlists) so both artifacts always exist, and say so - // once — no per-key warnings (nothing failed). The dev warehouse - // watch (or the next blocking run) re-enters this path with the - // warehouse RUNNING and lands the real schemas. - metricSchemas = resolution.entries.map(emptyMetricSchema); + // once — no per-key warnings (nothing failed). Cache hits keep + // serving their last-known-good schemas — only the remainder + // degrades. The dev warehouse watch (or the next blocking run) + // re-enters this path with the warehouse RUNNING and lands the real + // schemas. + described = describeNeeded.map(emptyMetricSchema); logger.info( "Warehouse %s is not running — wrote degraded metric types (permissive) for %d metric view(s) (%s); they will refresh once the warehouse is available.", warehouseId, - resolution.entries.length, - resolution.entries.map((e) => e.key).join(", "), + describeNeeded.length, + describeNeeded.map((e) => e.key).join(", "), ); } + // Persist this pass's outcomes for exactly the keys it owned (the + // describe-needed set): a successful DESCRIBE caches `retry: false`; + // every degraded outcome — syncMetrics failures and non-terminal + // states, the gate-skip path, and the fatal-preflight path (the last + // two never entered syncMetrics) — caches its degraded schema with + // `retry: true` so the next eligible pass re-describes only these + // keys. Hits were partitioned out above and are never rewritten, which + // is what lets a warehouse-down pass keep last-known-good entries + // intact. One save per pass; with `noCache` the section was started + // empty, so saving overwrites it with this pass's results alone. + if (describeNeeded.length > 0 || noCache) { + for (let i = 0; i < describeNeeded.length; i++) { + // syncMetrics (and both .map(emptyMetricSchema) branches) return + // one schema per entry in entry order, so described[i] always + // belongs to describeNeeded[i] / neededHashes[i]. + metricsSection[describeNeeded[i].key] = { + hash: neededHashes[i], + schema: described[i], + retry: described[i].degraded === true, + }; + } + cache.metrics = metricsSection; + await saveCache(cache); + } + + // Merge cached hits with fresh results back into config order + // (resolution.entries order — the renderers sort internally where + // determinism matters). + const describedByKey = new Map(); + for (const schema of described) { + describedByKey.set(schema.key, schema); + } + const metricSchemas = resolution.entries.map( + (entry) => + hitSchemas.get(entry.key) ?? + describedByKey.get(entry.key) ?? + // Unreachable: every entry is either a hit or describe-needed, and + // every describe-needed entry yields exactly one schema above. + emptyMetricSchema(entry), + ); + const metricFile = metricOutFile ?? path.join(path.dirname(outFile), METRIC_TYPES_FILE); const metricDeclarations = generateMetricTypeDeclarations(metricSchemas); diff --git a/packages/appkit/src/type-generator/tests/index.test.ts b/packages/appkit/src/type-generator/tests/index.test.ts index 297d12fa..15027920 100644 --- a/packages/appkit/src/type-generator/tests/index.test.ts +++ b/packages/appkit/src/type-generator/tests/index.test.ts @@ -17,6 +17,12 @@ const mocks = vi.hoisted(() => ({ startWarehouse: vi.fn(), waitUntilRunning: vi.fn(), executeStatement: vi.fn(), + // In-memory stand-in for the on-disk typegen cache file. `undefined` means + // "no file yet"; otherwise it holds the serialized JSON exactly as + // saveCache would have written it, so load/save round-trips behave like + // the real implementation (string parse, own-property semantics, unknown + // sibling keys preserved) without touching node_modules/.databricks. + cacheFile: { contents: undefined as string | undefined }, })); // Mock only the warehouse-describe step; index.ts owns the throw decision we @@ -29,6 +35,38 @@ vi.mock("../query-registry", async (importOriginal) => { }; }); +// The metric path persists schemas in the shared typegen cache; redirect +// loadCache/saveCache to the in-memory `cacheFile` above so tests control +// cache state per test and nothing leaks to the real cache file (which would +// make DESCRIBE-count assertions order- and rerun-dependent). hashSQL and +// CACHE_VERSION pass through unmocked. +vi.mock("../cache", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + loadCache: vi.fn(async () => { + const raw = mocks.cacheFile.contents; + if (raw !== undefined) { + try { + const parsed = JSON.parse(raw) as Awaited< + ReturnType + >; + if (parsed.version === actual.CACHE_VERSION) { + return parsed; + } + } catch { + // Corrupted "file": fall through to the fresh-cache default, same + // as the real loadCache. + } + } + return { version: actual.CACHE_VERSION, queries: {} }; + }), + saveCache: vi.fn(async (cache: unknown) => { + mocks.cacheFile.contents = JSON.stringify(cache, null, 2); + }), + }; +}); + // The metric gate's status-only probe and the metric blocking preflight // resolve through getWarehouseState/startWarehouse/waitUntilRunning; stub all // three so tests dictate the observed warehouse lifecycle. (query-registry @@ -275,6 +313,7 @@ describe("generateFromEntryPoint — metric-view emission", () => { beforeEach(() => { vi.clearAllMocks(); + mocks.cacheFile.contents = undefined; fs.rmSync(metricsDir, { recursive: true, force: true }); fs.mkdirSync(queryFolder, { recursive: true }); mocks.generateQueriesFromDescribe.mockResolvedValue({ @@ -801,3 +840,318 @@ describe("generateFromEntryPoint — metric-view emission", () => { expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); }); }); + +describe("generateFromEntryPoint — metric cache section", () => { + const cacheTestDir = path.join(__dirname, "__output_metric_cache__"); + const queryFolder = path.join(cacheTestDir, "queries"); + const outFile = path.join(cacheTestDir, "generated", "analytics.d.ts"); + const metricFile = path.join(cacheTestDir, "generated", "metric.d.ts"); + const metadataFile = path.join( + cacheTestDir, + "generated", + "metrics.metadata.json", + ); + + const describeResponseFor = ( + measure: string, + ): DatabricksStatementExecutionResponse => ({ + statement_id: "stmt-mock", + status: { state: "SUCCEEDED" }, + result: { + data_array: [ + [ + JSON.stringify({ + columns: [ + { name: measure, type: "DECIMAL(38,2)", is_measure: true }, + { name: "region", type: "STRING", is_measure: false }, + ], + }), + ], + ], + }, + }); + + const writeConfig = ( + metricViews: Record< + string, + { source: string; executor?: "app_service_principal" | "user" } + >, + ) => { + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + JSON.stringify({ metricViews }), + ); + }; + + // Parse the in-memory "cache file" the way the next pass's loadCache would. + const savedCache = () => JSON.parse(mocks.cacheFile.contents ?? "{}"); + + const run = ( + overrides: Partial[0]> = {}, + ) => + generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "non-blocking", + ...overrides, + }); + + beforeEach(() => { + vi.clearAllMocks(); + mocks.cacheFile.contents = undefined; + fs.rmSync(cacheTestDir, { recursive: true, force: true }); + fs.mkdirSync(queryFolder, { recursive: true }); + mocks.generateQueriesFromDescribe.mockResolvedValue({ + schemas: [], + syntaxErrors: [], + fatalErrors: [], + }); + }); + + afterAll(() => { + fs.rmSync(cacheTestDir, { recursive: true, force: true }); + }); + + test("warm pass: unchanged config makes zero DESCRIBEs, zero probes, zero clients — artifacts rewritten byte-identical from cache", async () => { + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + const firstDeclarations = fs.readFileSync(metricFile, "utf-8"); + const firstBundle = fs.readFileSync(metadataFile, "utf-8"); + + // Wipe the artifacts so pass 2 provably rewrites them from cache alone. + fs.rmSync(metricFile); + fs.rmSync(metadataFile); + vi.clearAllMocks(); + + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + // All keys were hits, so the gate never even probed the warehouse ... + expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + // ... and the whole pass constructed zero SDK clients. + expect(vi.mocked(WorkspaceClient)).not.toHaveBeenCalled(); + expect(fs.readFileSync(metricFile, "utf-8")).toBe(firstDeclarations); + expect(fs.readFileSync(metadataFile, "utf-8")).toBe(firstBundle); + }); + + test("single-entry edit: only the edited key is re-described", async () => { + writeConfig({ + churn: { source: "demo.sales.churn" }, + revenue: { source: "demo.sales.revenue" }, + }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(2); + + vi.clearAllMocks(); + writeConfig({ + churn: { source: "demo.sales.churn" }, + revenue: { source: "demo.sales.revenue_v2" }, + }); + + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + expect(mocks.executeStatement).toHaveBeenCalledWith( + expect.objectContaining({ + statement: "DESCRIBE TABLE EXTENDED demo.sales.revenue_v2 AS JSON", + }), + ); + const metrics = savedCache().metrics; + expect(metrics.churn.retry).toBe(false); + expect(metrics.revenue.retry).toBe(false); + expect(metrics.revenue.schema.source).toBe("demo.sales.revenue_v2"); + }); + + test("retry convergence: a degraded key is re-described — and only that key — once a blocking pass reaches a RUNNING warehouse", async () => { + // Pass 0: revenue lands a real schema in the cache. + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + await expect(run()).resolves.toBeUndefined(); + + // Pass 1: churn added while the warehouse is down. The gate skips its + // DESCRIBE; churn is cached degraded with retry: true. revenue stays a + // hit and its good entry is NOT overwritten. + vi.clearAllMocks(); + mocks.getWarehouseState.mockResolvedValue("STOPPED"); + writeConfig({ + churn: { source: "demo.sales.churn" }, + revenue: { source: "demo.sales.revenue" }, + }); + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + expect(savedCache().metrics.churn.retry).toBe(true); + expect(savedCache().metrics.churn.schema.degraded).toBe(true); + expect(savedCache().metrics.revenue.retry).toBe(false); + // Artifacts mix the cached real schema with the degraded newcomer. + expect(fs.readFileSync(metricFile, "utf-8")).toContain( + '"total_revenue": number', + ); + + // Pass 2: blocking with the warehouse RUNNING. Only the retry-flagged + // key is described; the hit is untouched. + vi.clearAllMocks(); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("monthly_churn"), + ); + await expect(run({ mode: "blocking" })).resolves.toBeUndefined(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + expect(mocks.executeStatement).toHaveBeenCalledWith( + expect.objectContaining({ + statement: "DESCRIBE TABLE EXTENDED demo.sales.churn AS JSON", + }), + ); + expect(savedCache().metrics.churn.retry).toBe(false); + + const refreshed = fs.readFileSync(metricFile, "utf-8"); + expect(refreshed).toContain('"monthly_churn": number'); + expect(refreshed).toContain('"total_revenue": number'); + expect(refreshed).not.toContain("measureKeys: string"); + }); + + test("last-known-good: warehouse down serves cached real schemas, not degraded ones", async () => { + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + await expect(run()).resolves.toBeUndefined(); + + vi.clearAllMocks(); + mocks.getWarehouseState.mockResolvedValue("STOPPED"); + fs.rmSync(metricFile); + fs.rmSync(metadataFile); + + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + + // The artifacts carry the cached REAL unions — not degraded-open types. + const declarations = fs.readFileSync(metricFile, "utf-8"); + expect(declarations).toContain('"total_revenue": number'); + expect(declarations).not.toContain("measureKeys: string"); + const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + expect(bundle.revenue.measures.total_revenue.type).toBe("DECIMAL(38,2)"); + // The good entry survived the warehouse-down pass un-overwritten. + expect(savedCache().metrics.revenue.retry).toBe(false); + }); + + test("noCache: true re-describes every key despite a warm cache and overwrites the section", async () => { + writeConfig({ + churn: { source: "demo.sales.churn" }, + legacy: { source: "demo.sales.legacy" }, + revenue: { source: "demo.sales.revenue" }, + }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(3); + + // Drop `legacy` from the config and rerun with noCache: every remaining + // key is described again and the section is rebuilt from results only — + // the stale `legacy` entry does not survive. + vi.clearAllMocks(); + writeConfig({ + churn: { source: "demo.sales.churn" }, + revenue: { source: "demo.sales.revenue" }, + }); + + await expect(run({ noCache: true })).resolves.toBeUndefined(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(2); + expect(Object.keys(savedCache().metrics).sort()).toEqual([ + "churn", + "revenue", + ]); + }); + + test("metric-path save preserves the queries section byte-for-byte, and a metrics-less cache file loads fine", async () => { + const seededQueries = { + my_query: { + hash: "abc123", + type: '{ name: "my_query"; parameters: Record; result: unknown; }', + retry: false, + }, + }; + // Pre-metrics cache file: version "3" with no `metrics` section at all. + mocks.cacheFile.contents = JSON.stringify( + { version: "3", queries: seededQueries }, + null, + 2, + ); + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + + const saved = savedCache(); + // Same version (no bump), queries byte-identical, metrics added beside. + expect(saved.version).toBe("3"); + expect(JSON.stringify(saved.queries)).toBe(JSON.stringify(seededQueries)); + expect(saved.metrics.revenue.retry).toBe(false); + expect(saved.metrics.revenue.schema.measures[0].name).toBe("total_revenue"); + }); + + test("a cached metric key named __proto__ neither pollutes prototypes nor vanishes on save", async () => { + const protoEntry = { + hash: "deadbeef", + schema: { + key: "__proto__", + source: "demo.evil.proto", + lane: "sp", + measures: [], + dimensions: [], + degraded: true, + }, + retry: true, + }; + // Computed key keeps "__proto__" an own property of the literal, so the + // serialized cache file genuinely contains a "__proto__" metrics key. + mocks.cacheFile.contents = JSON.stringify({ + version: "3", + queries: {}, + metrics: { ["__proto__"]: protoEntry }, + }); + expect(mocks.cacheFile.contents).toContain('"__proto__"'); + + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + + await expect(run()).resolves.toBeUndefined(); + + // No prototype pollution: the entry's fields never leaked onto plain + // objects via an Object.prototype mutation. + expect(({} as Record).hash).toBeUndefined(); + expect(({} as Record).retry).toBeUndefined(); + expect(Object.prototype).not.toHaveProperty("hash"); + + // The entry survived load → null-prototype copy → save as an OWN key of + // the section (a plain-object copy would have hit the __proto__ setter + // and silently dropped it from the serialized output). + expect(mocks.cacheFile.contents).toContain('"__proto__"'); + const metrics = savedCache().metrics; + expect(Object.hasOwn(metrics, "__proto__")).toBe(true); + expect(metrics.revenue.retry).toBe(false); + }); +}); From 6d0953ae3fadf59bedc98fb00f730c4f6cbbf302 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Thu, 11 Jun 2026 19:04:35 +0200 Subject: [PATCH 07/10] docs(appkit): document metric typegen convergence contract Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/index.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index a54ca6fc..52da2a9c 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -264,12 +264,16 @@ async function isWarehouseRunning( * @param options.mode - preflight policy (see {@link PreflightMode}). For * queries, `"non-blocking"` never probes or describes the warehouse. For * metric views, `"non-blocking"` makes one status-only probe and DESCRIBEs - * only when the warehouse is already RUNNING (otherwise degraded artifacts - * are emitted); `"blocking"` first ensures the warehouse is running — it - * waits for a starting warehouse and starts (then waits for) a stopped one, - * failing the build only for a deleted/deleting warehouse, exactly like the - * query path's fatal preflight — and then DESCRIBEs. Defaults to - * `"non-blocking"`. + * only when the warehouse is already RUNNING; otherwise permissive degraded + * metric types are emitted immediately and the affected keys are cached + * with `retry: true`, converging to real schemas on the next + * describe-capable pass — in dev the Vite plugin's warehouse watch triggers + * that pass automatically, while one-shot CLI runs (e.g. postinstall) leave + * no background waiter and converge on their next run. `"blocking"` first + * ensures the warehouse is running — it waits for a starting warehouse and + * starts (then waits for) a stopped one, failing the build only for a + * deleted/deleting warehouse, exactly like the query path's fatal + * preflight — and then DESCRIBEs. Defaults to `"non-blocking"`. * @param options.metricOutFile - optional output file for the MetricRegistry * augmentation. Defaults to a sibling `metric.d.ts` file under the same * directory as `outFile`. Skipped entirely if `metric-views.json` is absent. From 1370c233f234a91b9d705eaa3f45507b1994bc44 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Fri, 12 Jun 2026 15:30:16 +0200 Subject: [PATCH 08/10] =?UTF-8?q?fix(appkit):=20metric=20cache=20hygiene?= =?UTF-8?q?=20=E2=80=94=20sticky=20failures,=20pruning,=20revival=20valida?= =?UTF-8?q?tion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit D′ retry semantics (transient vs sticky failure classes via MetricSyncFailure.transient), sticky-hit warn notice, state-aware non-blocking gate (DELETED becomes sticky), DELETED-mid-wait rides TypegenFatalError, cache pruning with forced save on config shrink, structural validation of revived cache entries. Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/cache.ts | 14 +- packages/appkit/src/type-generator/index.ts | 300 ++++++++--- .../src/type-generator/metric-registry.ts | 48 +- .../src/type-generator/tests/index.test.ts | 503 +++++++++++++++++- .../tests/metric-registry.test.ts | 73 +++ 5 files changed, 846 insertions(+), 92 deletions(-) diff --git a/packages/appkit/src/type-generator/cache.ts b/packages/appkit/src/type-generator/cache.ts index c6fb6005..27424380 100644 --- a/packages/appkit/src/type-generator/cache.ts +++ b/packages/appkit/src/type-generator/cache.ts @@ -24,10 +24,16 @@ interface CacheEntry { * determine a DESCRIBE — so editing either invalidates the entry. `schema` * is the full {@link MetricSchema} persisted verbatim (it is JSON-safe by * design), letting a warm pass regenerate both metric artifacts without a - * single warehouse call. `retry: true` marks a degraded outcome (DESCRIBE - * skipped, unanswered, or failed): the cached schema still renders - * artifacts, but the next eligible pass re-describes exactly these keys so - * degraded schemas converge to real ones. + * single warehouse call. `retry: true` marks a SELF-CONVERGING degraded + * outcome (DESCRIBE skipped behind a not-running warehouse, unanswered, or + * transiently failed): the cached schema still renders artifacts, but the + * next eligible pass re-describes exactly these keys so degraded schemas + * converge to real ones. A degraded schema with `retry: false` is a STICKY + * failure — a deterministic DESCRIBE failure (bad FQN, unparseable + * response, zero columns) or a deleted warehouse — that re-describing the + * unchanged entry cannot fix; it hits like any cached entry until the + * config hash changes or the cache is bypassed, and the type generator + * warns about it on every pass that serves it. */ export interface MetricCacheEntry { hash: string; diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index 52da2a9c..4053fb64 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -32,6 +32,7 @@ import type { QueryFatalError, QuerySchema, QuerySyntaxError } from "./types"; import { getWarehouseState, startWarehouse, + type WarehouseState, waitUntilRunning, } from "./warehouse-status"; @@ -228,28 +229,76 @@ declare module "@databricks/appkit-ui/react" { /** * Status-only probe for the metric-view gate in {@link generateFromEntryPoint}: - * is the warehouse RUNNING right now? + * what state is the warehouse in right now? * * Uses {@link getWarehouseState} (`warehouses.get`) — a read-only GET that can * never start the warehouse — unlike the metric DESCRIBE statements it guards, * whose statement execution auto-starts a stopped warehouse and waits on it. * + * Returns the observed state so the gate can distinguish a transient + * not-running state (STOPPED/STARTING/... → degraded entries that retry) from + * a terminal one (DELETED/DELETING → degraded entries pinned sticky: they can + * never self-converge). + * * Takes the metric path's lazy client *getter* (not a constructed client) so * the probe's failure semantics cover client construction too: any failure to - * observe a state — connectivity, auth, bad id, or SDK construction — reads - * as "not running". In non-blocking mode typegen must never block on, or fail - * because of, the warehouse, so the caller degrades and a later blocking run - * (e.g. the Vite plugin's warehouse watch) lands the real schemas. + * observe a state — connectivity, auth, bad id, or SDK construction — returns + * `undefined`, which the gate reads as a transient not-running state. In + * non-blocking mode typegen must never block on, or fail because of, the + * warehouse, so the caller degrades and a later blocking run (e.g. the Vite + * plugin's warehouse watch) lands the real schemas. */ -async function isWarehouseRunning( +async function probeWarehouseState( getClient: () => WorkspaceClient, warehouseId: string, -): Promise { +): Promise { try { - return (await getWarehouseState(getClient(), warehouseId)) === "RUNNING"; + return await getWarehouseState(getClient(), warehouseId); } catch { + return undefined; + } +} + +/** + * Structural gate for reviving a cached metric entry at partition time. + * + * The cache file lives in `node_modules/.databricks` and is plain JSON — + * hand-edits, truncation, or a stale writer can leave entries whose shape no + * longer matches {@link MetricCacheEntry}. A malformed entry must read as a + * cache MISS (re-describe) rather than crash the pass or render revived + * garbage into the artifacts. Checks exactly what the renderers and the + * metadata bundle consume: `hash` string, `retry` boolean, and a schema with + * `key`/`source` strings, a valid lane, an optional boolean `degraded`, and + * measure/dimension arrays whose elements carry `name`/`type` strings + * (other column fields are optional). Deliberately inline — the shared Zod + * schemas must not enter the type-generator's runtime path. + */ +function isRevivableMetricCacheEntry(entry: MetricCacheEntry): boolean { + if (typeof entry.hash !== "string" || typeof entry.retry !== "boolean") { + return false; + } + const schema = entry.schema as unknown; + if (typeof schema !== "object" || schema === null || Array.isArray(schema)) { return false; } + const s = schema as Record; + const isColumnArray = (value: unknown): boolean => + Array.isArray(value) && + value.every( + (col) => + typeof col === "object" && + col !== null && + typeof (col as Record).name === "string" && + typeof (col as Record).type === "string", + ); + return ( + typeof s.key === "string" && + typeof s.source === "string" && + (s.lane === "sp" || s.lane === "obo") && + (s.degraded === undefined || typeof s.degraded === "boolean") && + isColumnArray(s.measures) && + isColumnArray(s.dimensions) + ); } /** @@ -269,10 +318,13 @@ async function isWarehouseRunning( * with `retry: true`, converging to real schemas on the next * describe-capable pass — in dev the Vite plugin's warehouse watch triggers * that pass automatically, while one-shot CLI runs (e.g. postinstall) leave - * no background waiter and converge on their next run. `"blocking"` first - * ensures the warehouse is running — it waits for a starting warehouse and - * starts (then waits for) a stopped one, failing the build only for a - * deleted/deleting warehouse, exactly like the query path's fatal + * no background waiter and converge on their next run. (A probe that reads + * DELETED/DELETING instead caches the keys sticky — `retry: false` — since + * they can never converge; the sticky-hit notice surfaces them on later + * passes.) `"blocking"` first ensures the warehouse is running — it waits + * for a starting warehouse and starts (then waits for) a stopped one, + * failing the build only for a deleted/deleting warehouse (observed at the + * first check or mid-wait), exactly like the query path's fatal * preflight — and then DESCRIBEs. Defaults to `"non-blocking"`. * @param options.metricOutFile - optional output file for the MetricRegistry * augmentation. Defaults to a sibling `metric.d.ts` file under the same @@ -363,30 +415,53 @@ export async function generateFromEntryPoint(options: { } } - // Partition BEFORE any gate/preflight decision: a hit (hash match and - // not flagged for retry) is served from cache no matter what the - // warehouse is doing — a degraded-mode pass falls back to - // last-known-good schemas exactly like queries degrade to cached - // types. Only the remainder — new keys, edited entries, and - // retry-flagged degraded entries — is eligible for DESCRIBE, so a - // fully-warm pass makes zero warehouse calls and constructs zero - // clients. `noCache` left the section empty above, which makes every - // configured key describe-needed here. + // Partition BEFORE any gate/preflight decision: a hit (structurally + // valid entry, hash match, and not flagged for retry) is served from + // cache no matter what the warehouse is doing — a degraded-mode pass + // falls back to last-known-good schemas exactly like queries degrade + // to cached types. Only the remainder — new keys, edited entries, + // retry-flagged degraded entries, and malformed (unrevivable) entries + // — is eligible for DESCRIBE, so a fully-warm pass makes zero + // warehouse calls and constructs zero clients. `noCache` left the + // section empty above, which makes every configured key + // describe-needed here. const hitSchemas = new Map(); const describeNeeded: typeof resolution.entries = []; // Parallel to describeNeeded: the config hash to persist per key. const neededHashes: string[] = []; + // Hits whose cached schema is degraded are STICKY failures: a previous + // pass pinned them with `retry: false` because re-describing the + // unchanged entry can't succeed (deterministic DESCRIBE failure, or a + // deleted warehouse). They serve their permissive schemas like any hit, + // but silently doing so forever would hide the misconfiguration — + // collect them for the single notice below. + const stickyDegradedHits: string[] = []; for (const entry of resolution.entries) { const hash = hashSQL(`${entry.source}|${entry.lane}`); const prior = metricsSection[entry.key]; - if (prior && prior.hash === hash && !prior.retry) { + if ( + prior !== undefined && + isRevivableMetricCacheEntry(prior) && + prior.hash === hash && + !prior.retry + ) { hitSchemas.set(entry.key, prior.schema); + if (prior.schema.degraded === true) { + stickyDegradedHits.push(entry.key); + } } else { describeNeeded.push(entry); neededHashes.push(hash); } } + if (stickyDegradedHits.length > 0) { + logger.warn( + "cached failure for %s — fix the entry in metric-views.json or run with --no-cache to retry.", + stickyDegradedHits.join(", "), + ); + } + // At most ONE WorkspaceClient per generation pass for the whole metric // path: the non-blocking status probe, the blocking preflight, and the // default DESCRIBE fetcher all share this lazily-created instance. A @@ -402,15 +477,15 @@ export async function generateFromEntryPoint(options: { // Blocking-mode preflight: ensure the warehouse is running before the // DESCRIBE batch, mirroring the query path's flow (probe → decide → - // wait / start+wait; only DELETED/DELETING is fatal). Deliberately - // SPLIT from the query path's preflight rather than shared — queries - // and metric views may bind to different warehouses in the future. - // Two deliberate softenings versus the query preflight: a failed probe - // and a timed-out (or non-RUNNING-ending) wait are NOT fatal here. We - // fall through to syncMetrics, whose DESCRIBEs classify a still-not- - // ready warehouse as degraded (permissive types, refreshed by a later - // run) rather than failing the build. An injected metricFetcher needs - // no warehouse, so it skips the preflight entirely. + // wait / start+wait; only DELETED/DELETING is fatal — at decision time + // OR observed mid-wait). Deliberately SPLIT from the query path's + // preflight rather than shared — queries and metric views may bind to + // different warehouses in the future. Two deliberate softenings versus + // the query preflight: a failed probe and a timed-out wait (thrown) + // are NOT fatal here. We fall through to syncMetrics, whose DESCRIBEs + // classify a still-not-ready warehouse as degraded (permissive types, + // refreshed by a later run) rather than failing the build. An injected + // metricFetcher needs no warehouse, so it skips the preflight entirely. let preflightFatalMessage: string | undefined; if ( mode === "blocking" && @@ -427,17 +502,38 @@ export async function generateFromEntryPoint(options: { // treatStoppedAsTransient rides out the stale pre-start // STOPPED/STOPPING reading, same as the query preflight. await startWarehouse(getMetricClient(), warehouseId); - await waitUntilRunning(getMetricClient(), warehouseId, { - maxMs: METRIC_PREFLIGHT_WAIT_MAX_MS, - treatStoppedAsTransient: true, - }); + const settled = await waitUntilRunning( + getMetricClient(), + warehouseId, + { + maxMs: METRIC_PREFLIGHT_WAIT_MAX_MS, + treatStoppedAsTransient: true, + }, + ); + if (settled !== "RUNNING") { + // With treatStoppedAsTransient, a non-RUNNING resolve is + // exactly DELETED/DELETING — the warehouse was deleted while + // we waited. Fatal, same as catching it at decision time. + preflightFatalMessage = `warehouse ${warehouseId} is ${settled}`; + } } else if (decision === "waitThenProceed") { - await waitUntilRunning(getMetricClient(), warehouseId, { - maxMs: METRIC_PREFLIGHT_WAIT_MAX_MS, - }); + const settled = await waitUntilRunning( + getMetricClient(), + warehouseId, + { + maxMs: METRIC_PREFLIGHT_WAIT_MAX_MS, + }, + ); + if (settled === "DELETED" || settled === "DELETING") { + // Deleted mid-wait: fatal. A STOPPED/STOPPING resolve (this + // wait runs without treatStoppedAsTransient) stays a soft + // fall-through — a stopped warehouse is startable, so it + // degrades and converges rather than failing the build. + preflightFatalMessage = `warehouse ${warehouseId} is ${settled}`; + } } - // "proceed" — and a wait that resolved non-RUNNING — falls through - // to syncMetrics below. + // "proceed" — and a wait that resolved into a startable state — + // falls through to syncMetrics below. } catch { // Probe/start failure or a wait that timed out: fall through to // syncMetrics. DESCRIBEs against a not-ready warehouse come back @@ -451,30 +547,44 @@ export async function generateFromEntryPoint(options: { // key and auto-starts a stopped warehouse — exactly what "non-blocking" // promises never to do. One status-only probe (a GET that can never // start the warehouse) decides whether to describe now or emit degraded - // artifacts that a later blocking run refreshes. An injected - // metricFetcher always runs: it doesn't hit a warehouse (tests/CI - // inject mocks), so gating it would only skip meaningful work. A pass - // with nothing describe-needed — fully-warm cache or an empty - // metricViews map — needs no probe either: nothing would be described - // in any mode. - const describeNow = + // artifacts that a later blocking run refreshes. The probe keeps the + // observed state (not just a boolean) so the skip below can tell a + // transient not-running state from a terminal DELETED/DELETING one. An + // injected metricFetcher always runs: it doesn't hit a warehouse + // (tests/CI inject mocks), so gating it would only skip meaningful + // work. A pass with nothing describe-needed — fully-warm cache or an + // empty metricViews map — needs no probe either: nothing would be + // described in any mode. + let gateState: WarehouseState | undefined; + let describeNow = metricFetcher !== undefined || mode !== "non-blocking" || - describeNeeded.length === 0 || - (await isWarehouseRunning(getMetricClient, warehouseId)); + describeNeeded.length === 0; + if (!describeNow) { + gateState = await probeWarehouseState(getMetricClient, warehouseId); + describeNow = gateState === "RUNNING"; + } let described: MetricSchema[]; let failures: MetricSyncFailure[] = []; + // True when this pass skipped the DESCRIBE batch for a reason that can + // never self-converge — a deleted/deleting warehouse (fatal preflight + // or gate skip). The write site pins those degraded outcomes sticky + // (`retry: false`) instead of re-describing them forever. + let terminalSkip = false; if (preflightFatalMessage !== undefined) { - // Fatal preflight (deleted/deleting warehouse): fail exactly like the - // query path's fatal preflight — skip the DESCRIBE batch, emit - // degraded schemas so both artifacts are still written, and record - // one fatal error per describe-needed key (cache hits are unaffected: - // they serve their cached schemas). The shared end-of-run throw below - // (TypegenFatalError, or TypegenSyntaxError's fatalQueries when - // syntax errors coexist) surfaces them after the writes, identically - // to query fatals. + // Fatal preflight (deleted/deleting warehouse — at decision time or + // mid-wait): fail exactly like the query path's fatal preflight — + // skip the DESCRIBE batch, emit degraded schemas so both artifacts + // are still written, and record one fatal error per describe-needed + // key (cache hits are unaffected: they serve their cached schemas). + // The shared end-of-run throw below (TypegenFatalError, or + // TypegenSyntaxError's fatalQueries when syntax errors coexist) + // surfaces them after the writes, identically to query fatals. The + // skip is terminal — these keys can never converge against a deleted + // warehouse — so their cache entries are pinned sticky. described = describeNeeded.map(emptyMetricSchema); + terminalSkip = true; for (const entry of describeNeeded) { fatalErrors.push({ name: entry.key, message: preflightFatalMessage }); } @@ -532,10 +642,14 @@ export async function generateFromEntryPoint(options: { // runtime allowlists) so both artifacts always exist, and say so // once — no per-key warnings (nothing failed). Cache hits keep // serving their last-known-good schemas — only the remainder - // degrades. The dev warehouse watch (or the next blocking run) - // re-enters this path with the warehouse RUNNING and lands the real - // schemas. + // degrades. For a transient state (stopped/starting/probe failure) + // the dev warehouse watch (or the next blocking run) re-enters this + // path with the warehouse RUNNING and lands the real schemas. A + // DELETED/DELETING probe is terminal: those keys are pinned sticky + // below (non-blocking never fails the build, so the sticky-hit + // notice on later passes is the loud signal). described = describeNeeded.map(emptyMetricSchema); + terminalSkip = gateState === "DELETED" || gateState === "DELETING"; logger.info( "Warehouse %s is not running — wrote degraded metric types (permissive) for %d metric view(s) (%s); they will refresh once the warehouse is available.", warehouseId, @@ -546,25 +660,55 @@ export async function generateFromEntryPoint(options: { // Persist this pass's outcomes for exactly the keys it owned (the // describe-needed set): a successful DESCRIBE caches `retry: false`; - // every degraded outcome — syncMetrics failures and non-terminal - // states, the gate-skip path, and the fatal-preflight path (the last - // two never entered syncMetrics) — caches its degraded schema with - // `retry: true` so the next eligible pass re-describes only these - // keys. Hits were partitioned out above and are never rewritten, which - // is what lets a warehouse-down pass keep last-known-good entries - // intact. One save per pass; with `noCache` the section was started - // empty, so saving overwrites it with this pass's results alone. - if (describeNeeded.length > 0 || noCache) { - for (let i = 0; i < describeNeeded.length; i++) { - // syncMetrics (and both .map(emptyMetricSchema) branches) return - // one schema per entry in entry order, so described[i] always - // belongs to describeNeeded[i] / neededHashes[i]. - metricsSection[describeNeeded[i].key] = { - hash: neededHashes[i], - schema: described[i], - retry: described[i].degraded === true, - }; + // degraded outcomes split by whether re-describing the unchanged entry + // can ever succeed. Self-converging degradation — non-terminal states, + // transient fetch failures, and the gate-skip / preflight paths for a + // merely not-running warehouse — caches `retry: true` so the next + // eligible pass re-describes only these keys. Deterministic failures + // (FAILED statement, zero rows, unparseable response, zero columns) + // and terminal skips (deleted/deleting warehouse) are pinned STICKY: + // `retry: false` with the degraded schema cached, so they hit on later + // passes (surfacing through the sticky-hit notice) instead of + // re-failing every describe-capable run. Hits were partitioned out + // above and are never rewritten, which is what lets a warehouse-down + // pass keep last-known-good entries intact. Keys dropped from the + // config are pruned so the section tracks metric-views.json exactly. + // One save per pass; with `noCache` the section was started empty, so + // saving overwrites it with this pass's results alone. + const failureByKey = new Map(); + for (const failure of failures) { + failureByKey.set(failure.key, failure); + } + for (let i = 0; i < describeNeeded.length; i++) { + // syncMetrics (and both .map(emptyMetricSchema) branches) return + // one schema per entry in entry order, so described[i] always + // belongs to describeNeeded[i] / neededHashes[i]. + const failure = failureByKey.get(describeNeeded[i].key); + metricsSection[describeNeeded[i].key] = { + hash: neededHashes[i], + schema: described[i], + retry: + described[i].degraded === true && + !terminalSkip && + (failure === undefined || failure.transient === true), + }; + } + + // Prune entries whose key is no longer configured, so a removed metric + // doesn't haunt the cache file forever. + const configuredKeys = new Set(resolution.entries.map((e) => e.key)); + let prunedCount = 0; + for (const key of Object.keys(metricsSection)) { + if (!configuredKeys.has(key)) { + delete metricsSection[key]; + prunedCount++; } + } + + // Save when this pass produced outcomes, bypassed the cache, or pruned + // — a warm pass over a shrunk config has nothing to describe but must + // still shrink the file. + if (describeNeeded.length > 0 || noCache || prunedCount > 0) { cache.metrics = metricsSection; await saveCache(cache); } diff --git a/packages/appkit/src/type-generator/metric-registry.ts b/packages/appkit/src/type-generator/metric-registry.ts index 74cfe3d2..00ec1b12 100644 --- a/packages/appkit/src/type-generator/metric-registry.ts +++ b/packages/appkit/src/type-generator/metric-registry.ts @@ -1065,6 +1065,19 @@ export interface MetricSyncFailure { source: string; /** Single human-readable reason (DESCRIBE failed, parse failed, zero columns). */ reason: string; + /** + * Whether the failure is expected to self-converge on a later pass without + * a config change. `true` for failures whose cause lives outside the + * entry's definition — a rejected fetch (transport/auth blip) or an + * unexplained settlement rejection — so retrying the same DESCRIBE can + * succeed. `false` for deterministic warehouse answers (FAILED statement, + * SUCCEEDED with zero rows, unparseable response, zero extracted columns): + * re-describing an unchanged entry would fail identically, so the caller's + * cache pins these sticky (`retry: false`) until the config (hash) changes + * or the cache is bypassed. Additive field — existing fields are consumed + * by the CLI via dynamic import and must not change shape. + */ + transient: boolean; } /** @@ -1144,7 +1157,9 @@ interface MetricDescribeOutcome { * * - FAILED statement, rejected fetch, unparseable response, or zero * extracted columns → a genuine failure: recorded in `failures` AND the - * schema is `degraded: true` (its columns are unknown). + * schema is `degraded: true` (its columns are unknown). Each failure also + * carries `transient` (see {@link MetricSyncFailure.transient}): rejected + * fetches are transient, deterministic warehouse answers are not. * - Non-terminal statement state (PENDING/RUNNING — warehouse reachable but * not ready) → degraded, never an error: schema is `degraded: true`, NOT * in `failures`. The next run with a ready warehouse lands the real @@ -1180,11 +1195,18 @@ export async function syncMetrics( try { response = await fetcher(entry.source); } catch (err) { + // The fetcher itself threw — a transport/auth blip, not a warehouse + // verdict on the entry. Transient: a later pass may succeed unchanged. const reason = `DESCRIBE TABLE EXTENDED failed: ${(err as Error).message}`; return { index, schema: emptyMetricSchema(entry), - failure: { key: entry.key, source: entry.source, reason }, + failure: { + key: entry.key, + source: entry.source, + reason, + transient: true, + }, }; } @@ -1210,10 +1232,18 @@ export async function syncMetrics( } if (parseError) { + // Deterministic warehouse answer (FAILED statement, SUCCEEDED with zero + // rows, unparseable payload): re-describing the same entry would fail + // identically, so this failure is non-transient (sticky in the cache). return { index, schema: emptyMetricSchema(entry), - failure: { key: entry.key, source: entry.source, reason: parseError }, + failure: { + key: entry.key, + source: entry.source, + reason: parseError, + transient: false, + }, }; } @@ -1229,7 +1259,14 @@ export async function syncMetrics( return { index, schema: emptyMetricSchema(entry), - failure: { key: entry.key, source: entry.source, reason }, + // Deterministic answer for this entry — non-transient, like the + // parse failures above. + failure: { + key: entry.key, + source: entry.source, + reason, + transient: false, + }, }; } @@ -1280,10 +1317,13 @@ export async function syncMetrics( ? result.reason.message : String(result.reason); schemas[index] = emptyMetricSchema(entry); + // Unknown cause — prefer convergence: mark transient so the next + // describe-capable pass retries instead of pinning a surprise. failureSlots[index] = { key: entry.key, source: entry.source, reason: `DESCRIBE TABLE EXTENDED failed: ${message}`, + transient: true, }; } } diff --git a/packages/appkit/src/type-generator/tests/index.test.ts b/packages/appkit/src/type-generator/tests/index.test.ts index 15027920..91b352c9 100644 --- a/packages/appkit/src/type-generator/tests/index.test.ts +++ b/packages/appkit/src/type-generator/tests/index.test.ts @@ -96,6 +96,9 @@ vi.mock("@databricks/sdk-experimental", () => ({ const { WorkspaceClient } = await import("@databricks/sdk-experimental"); const { generateFromEntryPoint, TypegenFatalError, TypegenSyntaxError } = await import("../index"); +// The "../cache" mock spreads the actual module, so this is the real hashSQL — +// used to seed cache entries whose hash genuinely matches the config. +const { hashSQL } = await import("../cache"); const outputDir = path.join(__dirname, "__output__"); @@ -650,6 +653,14 @@ describe("generateFromEntryPoint — metric-view emission", () => { expect(declarations).toContain("measureKeys: string"); const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); + + // D′: the fatal skip is terminal — a deleted warehouse can never serve + // these keys, so the degraded entries are pinned sticky (retry: false) + // and later passes surface them via the sticky-hit notice instead of + // re-describing forever. + const metrics = JSON.parse(mocks.cacheFile.contents ?? "{}").metrics; + expect(metrics.revenue.retry).toBe(false); + expect(metrics.revenue.schema.degraded).toBe(true); }); test.each([ @@ -721,6 +732,67 @@ describe("generateFromEntryPoint — metric-view emission", () => { expect(fs.readFileSync(metricFile, "utf-8")).toContain( "measureKeys: string", ); + + // D′: a still-startable warehouse is transient degradation — cached + // with retry: true so the next describe-capable pass converges it. + const metrics = JSON.parse(mocks.cacheFile.contents ?? "{}").metrics; + expect(metrics.revenue.retry).toBe(true); + expect(metrics.revenue.schema.degraded).toBe(true); + }, + ); + + test.each<[string, boolean]>([ + // STOPPED probe → start + wait (treatStoppedAsTransient: a non-RUNNING + // resolve is necessarily DELETED/DELETING). + ["STOPPED", true], + // STARTING probe → wait-only; a DELETED resolve is fatal there too. + ["STARTING", false], + ])( + "blocking + warehouse deleted mid-wait (probe read %s): fatal after artifacts, sticky cache entry", + async (probedState, startsWarehouse) => { + writeMetricConfig(); + mocks.getWarehouseState.mockResolvedValue(probedState); + mocks.startWarehouse.mockResolvedValue(undefined); + // The warehouse was deleted while the preflight waited: the wait + // RESOLVES (does not throw) with the terminal state. + mocks.waitUntilRunning.mockResolvedValue("DELETED"); + + const error = await generateFromEntryPoint({ + outFile, + queryFolder, + warehouseId: "wh-1", + mode: "blocking", + }).then( + () => { + throw new Error("expected generateFromEntryPoint to reject"); + }, + (err: unknown) => err, + ); + + // Same fatal pathway as the decision-time DELETED: per-key entries + // with the query path's message template, thrown after the writes. + expect(error).toBeInstanceOf(TypegenFatalError); + expect((error as InstanceType).queries).toEqual( + [{ name: "revenue", message: "warehouse wh-1 is DELETED" }], + ); + + expect(mocks.startWarehouse).toHaveBeenCalledTimes( + startsWarehouse ? 1 : 0, + ); + // The DESCRIBE batch is skipped — nothing can answer it. + expect(mocks.executeStatement).not.toHaveBeenCalled(); + + // Degraded artifacts are still written before the throw. + const declarations = fs.readFileSync(metricFile, "utf-8"); + expect(declarations).toContain('"revenue"'); + expect(declarations).toContain("measureKeys: string"); + const bundle = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + expect(bundle.revenue).toEqual({ measures: {}, dimensions: {} }); + + // D′: terminal skip — sticky, like the decision-time fatal. + const metrics = JSON.parse(mocks.cacheFile.contents ?? "{}").metrics; + expect(metrics.revenue.retry).toBe(false); + expect(metrics.revenue.schema.degraded).toBe(true); }, ); @@ -1110,7 +1182,7 @@ describe("generateFromEntryPoint — metric cache section", () => { expect(saved.metrics.revenue.schema.measures[0].name).toBe("total_revenue"); }); - test("a cached metric key named __proto__ neither pollutes prototypes nor vanishes on save", async () => { + test("a configured metric key named __proto__ neither pollutes prototypes nor vanishes on save", async () => { const protoEntry = { hash: "deadbeef", schema: { @@ -1132,7 +1204,13 @@ describe("generateFromEntryPoint — metric cache section", () => { }); expect(mocks.cacheFile.contents).toContain('"__proto__"'); - writeConfig({ revenue: { source: "demo.sales.revenue" } }); + // "__proto__" passes the metric key regex, so a config can genuinely + // declare it. Keeping it CONFIGURED is what exempts it from pruning — + // the unconfigured-key case is covered by the prune tests. + writeConfig({ + ["__proto__"]: { source: "demo.evil.proto" }, + revenue: { source: "demo.sales.revenue" }, + }); mocks.getWarehouseState.mockResolvedValue("RUNNING"); mocks.executeStatement.mockResolvedValue( describeResponseFor("total_revenue"), @@ -1140,18 +1218,431 @@ describe("generateFromEntryPoint — metric cache section", () => { await expect(run()).resolves.toBeUndefined(); + // The seeded hash mismatches the configured source, so the key was + // re-described alongside revenue. + expect(mocks.executeStatement).toHaveBeenCalledTimes(2); + // No prototype pollution: the entry's fields never leaked onto plain - // objects via an Object.prototype mutation. + // objects via an Object.prototype mutation — neither on the load copy + // nor on the describe-result write into the section. expect(({} as Record).hash).toBeUndefined(); expect(({} as Record).retry).toBeUndefined(); expect(Object.prototype).not.toHaveProperty("hash"); - // The entry survived load → null-prototype copy → save as an OWN key of - // the section (a plain-object copy would have hit the __proto__ setter - // and silently dropped it from the serialized output). + // The entry survived load → null-prototype copy → write → save as an + // OWN key of the section (a plain-object section would have hit the + // __proto__ setter and silently dropped it from the serialized output). expect(mocks.cacheFile.contents).toContain('"__proto__"'); const metrics = savedCache().metrics; expect(Object.hasOwn(metrics, "__proto__")).toBe(true); + const protoSaved = Object.getOwnPropertyDescriptor( + metrics, + "__proto__", + )?.value; + expect(protoSaved.retry).toBe(false); + expect(protoSaved.schema.measures[0].name).toBe("total_revenue"); expect(metrics.revenue.retry).toBe(false); }); + + // ── D′ sticky/transient retry semantics ─────────────────────────────── + + test("D′ write matrix: transient failures and non-terminal states retry, deterministic failures stick", async () => { + writeConfig({ + failed_stmt: { source: "demo.sales.failed_stmt" }, + fetch_reject: { source: "demo.sales.fetch_reject" }, + good: { source: "demo.sales.good" }, + no_columns: { source: "demo.sales.no_columns" }, + no_rows: { source: "demo.sales.no_rows" }, + pending: { source: "demo.sales.pending" }, + }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockImplementation( + async ({ statement }: { statement: string }) => { + if (statement.includes("fetch_reject")) { + throw new Error("socket hang up"); + } + if (statement.includes("failed_stmt")) { + return { + statement_id: "stmt-mock", + status: { state: "FAILED", error: { message: "no such table" } }, + }; + } + if (statement.includes("no_rows")) { + return { + statement_id: "stmt-mock", + status: { state: "SUCCEEDED" }, + result: { data_array: [] }, + }; + } + if (statement.includes("no_columns")) { + return { + statement_id: "stmt-mock", + status: { state: "SUCCEEDED" }, + result: { data_array: [[JSON.stringify({ unrelated: true })]] }, + }; + } + if (statement.includes("pending")) { + return { statement_id: "stmt-mock", status: { state: "PENDING" } }; + } + return describeResponseFor("total_revenue"); + }, + ); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + try { + await expect(run()).resolves.toBeUndefined(); + } finally { + warnSpy.mockRestore(); + logSpy.mockRestore(); + } + + const metrics = savedCache().metrics; + // Transient fetch rejection → re-describe next eligible pass. + expect(metrics.fetch_reject.retry).toBe(true); + expect(metrics.fetch_reject.schema.degraded).toBe(true); + // Non-terminal statement state (not a failure at all) → retry. + expect(metrics.pending.retry).toBe(true); + expect(metrics.pending.schema.degraded).toBe(true); + // Deterministic failures → STICKY: degraded schema cached, no retry. + for (const key of ["failed_stmt", "no_rows", "no_columns"]) { + expect(metrics[key].retry).toBe(false); + expect(metrics[key].schema.degraded).toBe(true); + } + // Success → real schema, no retry. + expect(metrics.good.retry).toBe(false); + expect(metrics.good.schema.degraded).toBeUndefined(); + }); + + test.each<[string, boolean]>([ + // Startable / transient states converge on a later pass → retry. + ["STOPPED", true], + ["STARTING", true], + // A deleted warehouse can never converge → sticky. + ["DELETED", false], + ["DELETING", false], + ])( + "D′ gate skip: a %s probe caches the skipped keys with retry: %s", + async (state, retry) => { + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue(state); + + // Non-blocking never throws — even for a deleted warehouse the pass + // degrades; only the cache disposition differs. + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + + const metrics = savedCache().metrics; + expect(metrics.revenue.retry).toBe(retry); + expect(metrics.revenue.schema.degraded).toBe(true); + }, + ); + + test("D′ gate skip on DELETED: the sticky entry hits on the next pass and surfaces via the notice", async () => { + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("DELETED"); + await expect(run()).resolves.toBeUndefined(); + expect(savedCache().metrics.revenue.retry).toBe(false); + + // Warm pass: the sticky entry is a HIT — zero describes, zero probes — + // and the notice names it. + vi.clearAllMocks(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + const stickyLines = warnSpy.mock.calls + .map((call) => call.map(String).join(" ")) + .filter((line) => line.includes("cached failure")); + expect(stickyLines).toHaveLength(1); + expect(stickyLines[0]).toContain("revenue"); + } finally { + warnSpy.mockRestore(); + } + }); + + test("sticky-hit notice: a warm pass over a sticky entry describes nothing and warns once naming the key", async () => { + // Pass 1: a deterministic DESCRIBE failure pins the key sticky. + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue({ + statement_id: "stmt-mock", + status: { state: "FAILED", error: { message: "no such table" } }, + }); + const firstWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(run()).resolves.toBeUndefined(); + // The describing pass reports the failure itself — the cached-failure + // notice is reserved for passes that merely SERVE the sticky entry. + const warned = firstWarnSpy.mock.calls.flat().map(String).join("\n"); + expect(warned).toContain("metric sync failed for revenue"); + expect(warned).not.toContain("cached failure"); + } finally { + firstWarnSpy.mockRestore(); + } + expect(savedCache().metrics.revenue.retry).toBe(false); + + // Pass 2 (warm): hash match + retry: false ⇒ HIT. No describes, no + // probes, exactly one warn naming the key and the escape hatches. + vi.clearAllMocks(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + + const warnedLines = warnSpy.mock.calls.map((call) => + call.map(String).join(" "), + ); + const stickyLines = warnedLines.filter((line) => + line.includes("cached failure"), + ); + expect(stickyLines).toHaveLength(1); + expect(stickyLines[0]).toContain("revenue"); + expect(stickyLines[0]).toContain("metric-views.json"); + expect(stickyLines[0]).toContain("--no-cache"); + // Nothing was described, so no fresh per-key failure warns. + expect(warnedLines.join("\n")).not.toContain("metric sync failed"); + } finally { + warnSpy.mockRestore(); + } + + // The sticky degraded schema still renders permissive artifacts. + expect(fs.readFileSync(metricFile, "utf-8")).toContain( + "measureKeys: string", + ); + }); + + test("no sticky-hit notice when the warm pass serves only good entries", async () => { + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + await expect(run()).resolves.toBeUndefined(); + + vi.clearAllMocks(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + expect(warnSpy.mock.calls.flat().map(String).join("\n")).not.toContain( + "cached failure", + ); + } finally { + warnSpy.mockRestore(); + } + }); + + test("sticky convergence: editing the source (hash change) re-describes a sticky key", async () => { + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue({ + statement_id: "stmt-mock", + status: { state: "FAILED", error: { message: "no such table" } }, + }); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(run()).resolves.toBeUndefined(); + expect(savedCache().metrics.revenue.retry).toBe(false); + + // The user fixes the FQN: hash changes, the sticky entry is + // invalidated, and the key converges to a real schema. + vi.clearAllMocks(); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + writeConfig({ revenue: { source: "demo.sales.revenue_v2" } }); + + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + expect(mocks.executeStatement).toHaveBeenCalledWith( + expect.objectContaining({ + statement: "DESCRIBE TABLE EXTENDED demo.sales.revenue_v2 AS JSON", + }), + ); + } finally { + warnSpy.mockRestore(); + } + + const metrics = savedCache().metrics; + expect(metrics.revenue.retry).toBe(false); + expect(metrics.revenue.schema.degraded).toBeUndefined(); + expect(fs.readFileSync(metricFile, "utf-8")).toContain( + '"total_revenue": number', + ); + }); + + test("sticky convergence: noCache re-describes a sticky key despite the matching hash", async () => { + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue({ + statement_id: "stmt-mock", + status: { state: "FAILED", error: { message: "no such table" } }, + }); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(run()).resolves.toBeUndefined(); + expect(savedCache().metrics.revenue.retry).toBe(false); + + vi.clearAllMocks(); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + + await expect(run({ noCache: true })).resolves.toBeUndefined(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + } finally { + warnSpy.mockRestore(); + } + + const metrics = savedCache().metrics; + expect(metrics.revenue.retry).toBe(false); + expect(metrics.revenue.schema.degraded).toBeUndefined(); + }); + + // ── Pruning + forced save ────────────────────────────────────────────── + + test("prune: a warm pass over a shrunk config drops the stale key and force-saves", async () => { + writeConfig({ + churn: { source: "demo.sales.churn" }, + revenue: { source: "demo.sales.revenue" }, + }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + await expect(run()).resolves.toBeUndefined(); + expect(Object.keys(savedCache().metrics).sort()).toEqual([ + "churn", + "revenue", + ]); + + // Drop churn. revenue is a hit, so nothing is described or probed — but + // the save must STILL run (forced by the prune) so the file shrinks. + vi.clearAllMocks(); + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + expect(Object.keys(savedCache().metrics)).toEqual(["revenue"]); + expect(savedCache().metrics.revenue.retry).toBe(false); + }); + + test("prune: a describing pass also drops stale keys", async () => { + writeConfig({ + churn: { source: "demo.sales.churn" }, + revenue: { source: "demo.sales.revenue" }, + }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + await expect(run()).resolves.toBeUndefined(); + + // Drop churn AND edit revenue: the pass describes revenue (hash change) + // and prunes churn in the same save. + vi.clearAllMocks(); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + writeConfig({ revenue: { source: "demo.sales.revenue_v2" } }); + + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + expect(Object.keys(savedCache().metrics)).toEqual(["revenue"]); + }); + + // ── Revival validation: malformed cache entries are misses, not crashes ── + + const revivableSchema = { + key: "revenue", + source: "demo.sales.revenue", + lane: "sp", + measures: [{ name: "m", type: "BIGINT", isMeasure: true }], + dimensions: [{ name: "region", type: "STRING", isMeasure: false }], + }; + + test("revival control: a well-formed seeded entry with a matching hash is served without describing", async () => { + // Control for the malformed matrix below: same hash/retry mechanics, + // valid shape ⇒ HIT. Proves the matrix's misses come from validation, + // not from a hash mismatch. + mocks.cacheFile.contents = JSON.stringify({ + version: "3", + queries: {}, + metrics: { + revenue: { + hash: hashSQL("demo.sales.revenue|sp"), + retry: false, + schema: revivableSchema, + }, + }, + }); + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + + await expect(run()).resolves.toBeUndefined(); + expect(mocks.executeStatement).not.toHaveBeenCalled(); + expect(mocks.getWarehouseState).not.toHaveBeenCalled(); + expect(fs.readFileSync(metricFile, "utf-8")).toContain('"m": number'); + }); + + test.each<[string, Record]>([ + ["schema is null", { schema: null }], + ["schema is an array", { schema: [] }], + [ + "schema missing measures", + { schema: { ...revivableSchema, measures: undefined } }, + ], + ["invalid lane", { schema: { ...revivableSchema, lane: "x" } }], + [ + "measures not an array", + { schema: { ...revivableSchema, measures: "nope" } }, + ], + [ + "column element missing type", + { schema: { ...revivableSchema, measures: [{ name: "m" }] } }, + ], + [ + "non-boolean degraded", + { schema: { ...revivableSchema, degraded: "yep" } }, + ], + ["non-string hash", { hash: 42 }], + ["non-boolean retry", { retry: "yes" }], + ])( + "revival validation: %s is a cache miss (re-described), never a crash", + async (_label, overrides) => { + mocks.cacheFile.contents = JSON.stringify({ + version: "3", + queries: {}, + metrics: { + revenue: { + hash: hashSQL("demo.sales.revenue|sp"), + retry: false, + schema: revivableSchema, + ...overrides, + }, + }, + }); + writeConfig({ revenue: { source: "demo.sales.revenue" } }); + mocks.getWarehouseState.mockResolvedValue("RUNNING"); + mocks.executeStatement.mockResolvedValue( + describeResponseFor("total_revenue"), + ); + + await expect(run()).resolves.toBeUndefined(); + // The malformed entry was not revived: the key was re-described and + // the cache healed with the fresh result. + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + expect(savedCache().metrics.revenue.retry).toBe(false); + expect(savedCache().metrics.revenue.schema.measures[0].name).toBe( + "total_revenue", + ); + // The artifacts render the fresh schema — never the revived garbage. + expect(fs.readFileSync(metricFile, "utf-8")).toContain( + '"total_revenue": number', + ); + }, + ); }); diff --git a/packages/appkit/src/type-generator/tests/metric-registry.test.ts b/packages/appkit/src/type-generator/tests/metric-registry.test.ts index d8790c87..7367a7af 100644 --- a/packages/appkit/src/type-generator/tests/metric-registry.test.ts +++ b/packages/appkit/src/type-generator/tests/metric-registry.test.ts @@ -536,11 +536,84 @@ describe("syncMetrics", () => { expect(failures[0]).toMatchObject({ key: "revenue", source: "demo.public.revenue", + // A rejected fetch is a transport blip, not a warehouse verdict — + // transient, so the caller's cache retries it. + transient: true, }); expect(failures[0].reason).toMatch(/warehouse unreachable/); }); }); +// ── D′ transience classification: every failure says whether retrying the +// unchanged entry can succeed. Rejected fetches (and the defensive +// settlement-rejection backstop) are transient; deterministic warehouse +// answers — FAILED, zero rows, unparseable payload, zero columns — are not, +// and the caller's cache pins them sticky until the config hash changes. +describe("syncMetrics — failure transience (D′)", () => { + const singleEntryResolution = () => + resolveMetricConfig({ + metricViews: { revenue: { source: "demo.public.revenue" } }, + }); + + test("a rejected fetch is transient", async () => { + const fetcher = async (): Promise => { + throw new Error("socket hang up"); + }; + const { failures } = await syncMetrics(singleEntryResolution(), fetcher); + expect(failures).toHaveLength(1); + expect(failures[0].transient).toBe(true); + }); + + test.each<[string, DatabricksStatementExecutionResponse]>([ + [ + "a FAILED statement", + { + statement_id: "stmt-mock", + status: { state: "FAILED", error: { message: "no such table" } }, + }, + ], + [ + "a SUCCEEDED statement with zero rows", + { + statement_id: "stmt-mock", + status: { state: "SUCCEEDED" }, + result: { data_array: [] }, + }, + ], + [ + "an unparseable payload", + { + statement_id: "stmt-mock", + status: { state: "SUCCEEDED" }, + result: { data_array: [["{not json"]] }, + }, + ], + ["zero extracted columns", mockDescribeResponse({ unrelated: true })], + ])("%s is non-transient (deterministic)", async (_label, response) => { + const fetcher = async () => response; + const { failures } = await syncMetrics(singleEntryResolution(), fetcher); + expect(failures).toHaveLength(1); + expect(failures[0].transient).toBe(false); + }); + + test("a defensive rejected settlement is transient (unknown cause — prefer convergence)", async () => { + // Same poisoned-response trick as the scheduling suite: blow up after + // the fetch try/catch so the settlement itself rejects. + const poisoned = new Proxy({} as DatabricksStatementExecutionResponse, { + get(_target, prop) { + if (prop === "then") { + return undefined; // keep the object await-able + } + throw new Error("poisoned response object"); + }, + }); + const fetcher = async () => poisoned; + const { failures } = await syncMetrics(singleEntryResolution(), fetcher); + expect(failures).toHaveLength(1); + expect(failures[0].transient).toBe(true); + }); +}); + // ── Parity with the query path's state machine (query-registry): FAILED → // genuine error, SUCCEEDED → proceed, anything else (PENDING/RUNNING) → // degraded, never an error. A stopped/cold warehouse that outlives the From c4777d4741377bdbf1836b89adc907246771aaa2 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Fri, 12 Jun 2026 15:45:38 +0200 Subject: [PATCH 09/10] fix(appkit): harden metric config validation, FQN quoting, and artifacts Caps (200 metric views, 255/segment + 767 FQN, 100 decimal places clamp), explicit metricViews:null rejection, backtick-quoted DESCRIBE segments (hyphen-legal FQNs now work; '--' neutralized), null-prototype bundle builder, basename-exact watcher match, shared code-unit key comparator for deterministic artifact ordering. Signed-off-by: Atila Fassina --- .../src/type-generator/metric-registry.ts | 130 +++++++- .../src/type-generator/tests/index.test.ts | 10 +- .../tests/metric-registry.test.ts | 296 ++++++++++++++++++ .../type-generator/tests/vite-plugin.test.ts | 30 ++ .../appkit/src/type-generator/vite-plugin.ts | 5 +- 5 files changed, 453 insertions(+), 18 deletions(-) diff --git a/packages/appkit/src/type-generator/metric-registry.ts b/packages/appkit/src/type-generator/metric-registry.ts index 00ec1b12..28c6cbfa 100644 --- a/packages/appkit/src/type-generator/metric-registry.ts +++ b/packages/appkit/src/type-generator/metric-registry.ts @@ -13,6 +13,37 @@ import type { DatabricksStatementExecutionResponse } from "./types"; */ const METRIC_CONFIG_FILE = "metric-views.json"; +/** + * Input caps enforced by {@link resolveMetricConfig}. + * + * Inline-only at v1: the canonical Zod schema + * (`packages/shared/src/schemas/metric-source.ts`) carries no caps yet — + * aligning it is a PR4 rider, so the parity suite deliberately excludes cap + * fixtures until then. + * + * - `MAX_METRIC_VIEWS` bounds the `metricViews` map so a pathological config + * cannot fan out thousands of DESCRIBE statements per generation pass. + * - `MAX_FQN_SEGMENT_LENGTH` mirrors Unity Catalog's 255-character + * identifier limit per FQN part. + * - `MAX_FQN_LENGTH` bounds the full dotted name (3 × 255 + 2 separators). + */ +const MAX_METRIC_VIEWS = 200; +const MAX_FQN_SEGMENT_LENGTH = 255; +const MAX_FQN_LENGTH = 767; + +/** + * Locale-independent comparator (UTF-16 code-unit order) shared by BOTH + * artifact key orderings: {@link resolveMetricConfig}'s entry sort (which the + * `.d.ts` renderer preserves) and {@link buildMetricsMetadataBundle}'s key + * sort. `localeCompare` (ICU-backed collation) can order mixed-case keys + * differently across machines and locales; code-unit order cannot — so the + * emitted `metric.d.ts` and `metrics.metadata.json` key order is always + * identical. + */ +function compareKeys(a: string, b: string): number { + return a < b ? -1 : a > b ? 1 : 0; +} + /** * The lane an entry sits in: `sp` (service principal, shared cache) * or `obo` (on-behalf-of, per-user cache). @@ -212,9 +243,11 @@ function isValidFqn(fqn: string): boolean { * Downstream consumers only ever see lanes. * * Throws on unknown top-level fields, invalid keys, non-object entries, - * unknown entry fields, invalid FQNs, or invalid executors. A single map - * makes duplicate metric keys unrepresentable by construction. Stable - * ordering: alphabetical by key. + * unknown entry fields, invalid FQNs, invalid executors, or inputs exceeding + * the v1 caps ({@link MAX_METRIC_VIEWS} entries, {@link MAX_FQN_LENGTH} / + * {@link MAX_FQN_SEGMENT_LENGTH} FQN bounds). A single map makes duplicate + * metric keys unrepresentable by construction. Stable ordering: by key in + * locale-independent code-unit order (see {@link compareKeys}). */ export function resolveMetricConfig( config: MetricSourceConfig, @@ -230,7 +263,11 @@ export function resolveMetricConfig( } } - const metricViews = config.metricViews ?? {}; + // Default ONLY a genuinely-absent `metricViews`. `null` must fall through + // to the type check below and throw — the canonical Zod schema rejects null + // (`.optional()` admits undefined only) and the inline validator agrees. + const metricViews = + config.metricViews === undefined ? {} : config.metricViews; if ( typeof metricViews !== "object" || metricViews === null || @@ -242,7 +279,12 @@ export function resolveMetricConfig( } const entries: ResolvedMetricEntry[] = []; - const sortedKeys = Object.keys(metricViews).sort(); + const sortedKeys = Object.keys(metricViews).sort(compareKeys); + if (sortedKeys.length > MAX_METRIC_VIEWS) { + throw new Error( + `Invalid 'metricViews' in metric-views.json: ${sortedKeys.length} metric views exceed the maximum of ${MAX_METRIC_VIEWS}.`, + ); + } for (const key of sortedKeys) { if (!isValidMetricKey(key)) { throw new Error( @@ -274,12 +316,33 @@ export function resolveMetricConfig( ); } + // Total-length cap BEFORE the regex so the pattern only ever runs on + // bounded input. The offending FQN is reported by length, not echoed — + // it can be arbitrarily long. + if (entry.source.length > MAX_FQN_LENGTH) { + throw new Error( + `Invalid metric source for "${key}": FQN is ${entry.source.length} characters, exceeding the maximum of ${MAX_FQN_LENGTH}.`, + ); + } + if (!isValidFqn(entry.source)) { throw new Error( `Invalid metric source "${entry.source}" for "${key}": expected a three-part UC FQN ...`, ); } + // The regex guarantees exactly three dot-joined segments; cap each at + // UC's identifier limit. + const segments = entry.source.split("."); + const segmentNames = ["catalog", "schema", "metric_view"]; + for (let i = 0; i < segments.length; i++) { + if (segments[i].length > MAX_FQN_SEGMENT_LENGTH) { + throw new Error( + `Invalid metric source for "${key}": the ${segmentNames[i]} segment is ${segments[i].length} characters, exceeding the maximum of ${MAX_FQN_SEGMENT_LENGTH} per segment.`, + ); + } + } + const executor = entry.executor; if ( executor !== undefined && @@ -559,15 +622,25 @@ function fractionalSuffix(places: number): string { return places > 0 ? `.${"0".repeat(places)}` : ""; } +/** + * Maximum decimal places honored from a format spec. `Number#toFixed` (the + * digit-count primitive downstream formatters render fractional suffixes + * with) throws a RangeError above 100 fraction digits, and the emitted + * printf string would carry a pathological zero-run. Clamp, do NOT throw: + * format specs are workspace-authored column metadata, not app config — a + * wild value must degrade gracefully, never fail the build. + */ +const MAX_DECIMAL_PLACES = 100; + function readDecimalPlaces(obj: Record): number | undefined { const dp = obj.decimal_places; if (typeof dp === "number" && Number.isFinite(dp) && dp >= 0) { - return Math.floor(dp); + return Math.min(Math.floor(dp), MAX_DECIMAL_PLACES); } if (dp && typeof dp === "object" && !Array.isArray(dp)) { const places = (dp as Record).places; if (typeof places === "number" && Number.isFinite(places) && places >= 0) { - return Math.floor(places); + return Math.min(Math.floor(places), MAX_DECIMAL_PLACES); } } return undefined; @@ -940,7 +1013,8 @@ type MetricsMetadataBundle = Record; /** * Pure function: turn a list of metric schemas into the JSON metadata bundle. * - * Deterministic key order: outer object keys are sorted alphabetically; + * Deterministic key order: outer object keys are sorted in locale-independent + * code-unit order (see {@link compareKeys} — identical to the .d.ts order); * measures and dimensions are emitted in the order they appeared in DESCRIBE * (Phase 1's preserved-from-YAML order), but each per-column object's fields * follow a fixed declaration order so snapshot diffs are stable. @@ -952,16 +1026,28 @@ type MetricsMetadataBundle = Record; export function buildMetricsMetadataBundle( schemas: MetricSchema[], ): MetricsMetadataBundle { - const bundle: MetricsMetadataBundle = {}; - const sortedSchemas = [...schemas].sort((a, b) => a.key.localeCompare(b.key)); + // Null-prototype maps, same guard as the typegen cache section in + // index.ts: metric keys are user-controlled config input and column names + // are workspace-controlled DESCRIBE output — "__proto__" passes the metric + // key regex and is a legal column name. On a plain object that write would + // hit the Object.prototype setter (swapping the object's prototype and + // silently dropping the entry from the emitted JSON) instead of storing + // data. + const bundle: MetricsMetadataBundle = Object.create(null); + // compareKeys (code-unit), NOT localeCompare: the bundle's key order must + // be byte-identical to the .d.ts entry order (resolveMetricConfig's sort) + // on every machine and locale. + const sortedSchemas = [...schemas].sort((a, b) => compareKeys(a.key, b.key)); for (const schema of sortedSchemas) { - const measures: Record = {}; + const measures: Record = + Object.create(null); for (const m of schema.measures) { measures[m.name] = buildColumnMetadata(m); } - const dimensions: Record = {}; + const dimensions: Record = + Object.create(null); for (const d of schema.dimensions) { dimensions[d.name] = buildColumnMetadata(d); } @@ -1042,8 +1128,26 @@ export function createWorkspaceDescribeFetcher( warehouseId: string, ): DescribeFetcher { return async (fqn: string) => { + // Defense-in-depth: every caller passes a source that already cleared + // resolveMetricConfig, but this fetcher is an exported seam — re-check + // before interpolating into SQL. + if (!isValidFqn(fqn)) { + throw new Error( + `Invalid metric source "${fqn}": expected a three-part UC FQN ...`, + ); + } + // Backtick-quote each segment. The segment charset + // ([a-zA-Z0-9_][a-zA-Z0-9_-]*, enforced by the regex above) cannot + // contain backticks (or dots), so the quoting cannot be escaped from — + // while the SQL metacharacter the charset DOES allow is neutralized + // inside the quotes (a hyphenated segment like "c--x" would otherwise + // open a `--` line comment mid-statement). + const quotedFqn = fqn + .split(".") + .map((segment) => `\`${segment}\``) + .join("."); const result = (await client.statementExecution.executeStatement({ - statement: `DESCRIBE TABLE EXTENDED ${fqn} AS JSON`, + statement: `DESCRIBE TABLE EXTENDED ${quotedFqn} AS JSON`, warehouse_id: warehouseId, wait_timeout: "30s", })) as DatabricksStatementExecutionResponse; diff --git a/packages/appkit/src/type-generator/tests/index.test.ts b/packages/appkit/src/type-generator/tests/index.test.ts index 91b352c9..bcff9fe8 100644 --- a/packages/appkit/src/type-generator/tests/index.test.ts +++ b/packages/appkit/src/type-generator/tests/index.test.ts @@ -537,7 +537,7 @@ describe("generateFromEntryPoint — metric-view emission", () => { expect(mocks.executeStatement).toHaveBeenCalledTimes(1); expect(mocks.executeStatement).toHaveBeenCalledWith( expect.objectContaining({ - statement: "DESCRIBE TABLE EXTENDED demo.sales.revenue AS JSON", + statement: "DESCRIBE TABLE EXTENDED `demo`.`sales`.`revenue` AS JSON", warehouse_id: "wh-1", }), ); @@ -1035,7 +1035,8 @@ describe("generateFromEntryPoint — metric cache section", () => { expect(mocks.executeStatement).toHaveBeenCalledTimes(1); expect(mocks.executeStatement).toHaveBeenCalledWith( expect.objectContaining({ - statement: "DESCRIBE TABLE EXTENDED demo.sales.revenue_v2 AS JSON", + statement: + "DESCRIBE TABLE EXTENDED `demo`.`sales`.`revenue_v2` AS JSON", }), ); const metrics = savedCache().metrics; @@ -1083,7 +1084,7 @@ describe("generateFromEntryPoint — metric cache section", () => { expect(mocks.executeStatement).toHaveBeenCalledTimes(1); expect(mocks.executeStatement).toHaveBeenCalledWith( expect.objectContaining({ - statement: "DESCRIBE TABLE EXTENDED demo.sales.churn AS JSON", + statement: "DESCRIBE TABLE EXTENDED `demo`.`sales`.`churn` AS JSON", }), ); expect(savedCache().metrics.churn.retry).toBe(false); @@ -1460,7 +1461,8 @@ describe("generateFromEntryPoint — metric cache section", () => { expect(mocks.executeStatement).toHaveBeenCalledTimes(1); expect(mocks.executeStatement).toHaveBeenCalledWith( expect.objectContaining({ - statement: "DESCRIBE TABLE EXTENDED demo.sales.revenue_v2 AS JSON", + statement: + "DESCRIBE TABLE EXTENDED `demo`.`sales`.`revenue_v2` AS JSON", }), ); } finally { diff --git a/packages/appkit/src/type-generator/tests/metric-registry.test.ts b/packages/appkit/src/type-generator/tests/metric-registry.test.ts index 7367a7af..3612e620 100644 --- a/packages/appkit/src/type-generator/tests/metric-registry.test.ts +++ b/packages/appkit/src/type-generator/tests/metric-registry.test.ts @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { metricSourceSchema } from "../../../../shared/src/schemas/metric-source"; import { buildMetricsMetadataBundle, + createWorkspaceDescribeFetcher, extractMetricColumns, generateMetricsMetadataJson, generateMetricTypeDeclarations, @@ -183,6 +184,14 @@ describe("resolveMetricConfig", () => { expect(resolveMetricConfig({ metricViews: {} }).entries).toEqual([]); }); + test("rejects metricViews: null (only a genuinely-absent field defaults)", () => { + // The canonical Zod schema rejects null (`.optional()` admits undefined + // only) — the inline validator must not coalesce null into an empty map. + expect(() => resolveUnchecked({ metricViews: null })).toThrowError( + /expected an object map of metric entries/, + ); + }); + test("rejects a non-object entry", () => { expect(() => resolveUnchecked({ metricViews: { revenue: "a.b.c" } }), @@ -202,11 +211,68 @@ describe("resolveMetricConfig", () => { }); }); +// ── Input caps (inline-only at v1): the canonical Zod schema has no caps +// yet — aligning it is a PR4 rider, so these fixtures deliberately do NOT +// run through metricSourceSchema (they'd pass it) and stay out of the +// parity suite below. +describe("resolveMetricConfig — input caps", () => { + const manyViews = (count: number) => + Object.fromEntries( + Array.from({ length: count }, (_, i) => [`m${i}`, { source: "a.b.c" }]), + ); + + test("accepts exactly 200 metricViews entries", () => { + const { entries } = resolveMetricConfig({ metricViews: manyViews(200) }); + expect(entries).toHaveLength(200); + }); + + test("rejects 201 metricViews entries, naming the limit and the count", () => { + expect(() => + resolveMetricConfig({ metricViews: manyViews(201) }), + ).toThrowError(/201 metric views exceed the maximum of 200/); + }); + + test("accepts FQN segments of exactly 255 characters (full FQN at the 767 cap)", () => { + const seg = "a".repeat(255); + const fqn = `${seg}.${seg}.${seg}`; // 3 × 255 + 2 = 767 — at the cap. + expect(fqn).toHaveLength(767); + const { entries } = resolveMetricConfig({ + metricViews: { revenue: { source: fqn } }, + }); + expect(entries[0].source).toBe(fqn); + }); + + test("rejects a 256-character FQN segment, naming the key, segment, and limit", () => { + const fqn = `${"a".repeat(256)}.b.c`; + expect(() => + resolveMetricConfig({ metricViews: { revenue: { source: fqn } } }), + ).toThrowError( + /Invalid metric source for "revenue": the catalog segment is 256 characters, exceeding the maximum of 255/, + ); + }); + + test("rejects a full FQN over 767 characters, naming the key and limit", () => { + const seg = "a".repeat(300); + const fqn = `${seg}.${seg}.${seg}`; // 902 — total cap fires before segment caps. + expect(() => + resolveMetricConfig({ metricViews: { revenue: { source: fqn } } }), + ).toThrowError( + /Invalid metric source for "revenue": FQN is 902 characters, exceeding the maximum of 767/, + ); + }); +}); + // ── Parity: the inline config validation must agree with the canonical // shared Zod schema (packages/shared/src/schemas/metric-source.ts). // The regexes and allowlists are copied, not imported (locked dependency-graph // ruling: the type-generator must not pull the shared schema package into the // runtime path) — this block is the drift alarm. TEST-ONLY import. +// +// Caps divergence: the inline validator enforces v1 input caps (≤200 entries, +// ≤255 per FQN segment, ≤767 full FQN) that the canonical schema does not +// carry yet — aligning the Zod schema is a PR4 rider. Cap fixtures therefore +// live in the dedicated caps suite above and are asserted on the inline side +// only; do NOT add them here expecting metricSourceSchema to reject them. describe("resolveMetricConfig — parity with shared metricSourceSchema", () => { const accepts: Array<{ name: string; config: Record }> = [ { @@ -279,6 +345,11 @@ describe("resolveMetricConfig — parity with shared metricSourceSchema", () => name: "non-object entry", config: { metricViews: { revenue: "a.b.c" } }, }, + { + // `.optional()` admits undefined only — null must throw on both sides. + name: "metricViews: null", + config: { metricViews: null }, + }, ]; for (const fixture of accepts) { @@ -337,6 +408,95 @@ describe("parseDescribeTableExtendedJson", () => { }); }); +// ── DESCRIBE statement construction: every FQN segment is backtick-quoted. +// The segment charset ([a-zA-Z0-9_][a-zA-Z0-9_-]*) cannot contain backticks, +// so the quoting cannot be escaped from — and the one SQL metacharacter the +// charset does allow (`-`, which unquoted can open a `--` line comment) is +// neutralized inside the quotes. +describe("createWorkspaceDescribeFetcher", () => { + /** Stub WorkspaceClient capturing executeStatement requests. */ + function stubClient(payload: unknown = { columns: [] }) { + const statements: Array> = []; + const client = { + statementExecution: { + executeStatement: async (req: Record) => { + statements.push(req); + return mockDescribeResponse(payload); + }, + }, + } as unknown as Parameters[0]; + return { client, statements }; + } + + test("emits a backtick-quoted three-part FQN with warehouse id and wait timeout", async () => { + const { client, statements } = stubClient(); + const fetcher = createWorkspaceDescribeFetcher(client, "wh-1"); + + await fetcher("demo.sales.revenue"); + + expect(statements).toHaveLength(1); + expect(statements[0]).toMatchObject({ + statement: "DESCRIBE TABLE EXTENDED `demo`.`sales`.`revenue` AS JSON", + warehouse_id: "wh-1", + wait_timeout: "30s", + }); + }); + + test("a hyphenated FQN round-trips: validated by resolveMetricConfig, quoted in the statement, response parsed", async () => { + // Hyphenated catalogs are valid per the shared source regex; unquoted + // they would be a SQL syntax error against a real warehouse. + const { entries } = resolveMetricConfig({ + metricViews: { revenue: { source: "prod-data.analytics.revenue" } }, + }); + expect(entries[0].source).toBe("prod-data.analytics.revenue"); + + const { client, statements } = stubClient({ + columns: [{ name: "arr", type: "DECIMAL", is_measure: true }], + }); + const fetcher = createWorkspaceDescribeFetcher(client, "wh-1"); + + const response = await fetcher(entries[0].source); + + expect(statements[0].statement).toBe( + "DESCRIBE TABLE EXTENDED `prod-data`.`analytics`.`revenue` AS JSON", + ); + const cols = extractMetricColumns(parseDescribeTableExtendedJson(response)); + expect(cols).toHaveLength(1); + expect(cols[0]).toMatchObject({ + name: "arr", + type: "DECIMAL", + isMeasure: true, + }); + }); + + test("a segment containing `--` is quoted so the comment introducer is neutralized", async () => { + const { client, statements } = stubClient(); + const fetcher = createWorkspaceDescribeFetcher(client, "wh-1"); + + await fetcher("a.b.c--x"); + + // `--` sits inside backticks — an identifier character sequence, not a + // line comment that would truncate ` AS JSON` off the statement. + expect(statements[0].statement).toBe( + "DESCRIBE TABLE EXTENDED `a`.`b`.`c--x` AS JSON", + ); + expect(statements[0].statement).toContain("`c--x`"); + }); + + test("rejects an FQN that fails validation without issuing a statement", async () => { + const { client, statements } = stubClient(); + const fetcher = createWorkspaceDescribeFetcher(client, "wh-1"); + + // Backticks (and anything else outside the segment charset) fail the + // defense-in-depth re-validation at the fetcher seam. + await expect(fetcher("a.b.`c`")).rejects.toThrowError(/three-part UC FQN/); + await expect(fetcher("not.three.part.parts")).rejects.toThrowError( + /three-part UC FQN/, + ); + expect(statements).toHaveLength(0); + }); +}); + describe("extractMetricColumns", () => { test("extracts measures and dimensions from the standard shape", () => { const cols = extractMetricColumns({ @@ -1375,6 +1535,44 @@ describe("extractMetricColumns — Phase 5 semantic metadata", () => { }); expect(cols[0].format).toBe("$#,##0.0000"); }); + + test("clamps structured decimal places to 100 (Number#toFixed RangeError bound)", () => { + // Format specs are workspace-authored column metadata, not app config — + // a wild `places` is clamped (never thrown) so the build still succeeds + // and downstream toFixed-style formatters stay inside their 100-digit + // RangeError bound. + const cols = extractMetricColumns({ + columns: [ + { + name: "huge", + type: "DOUBLE", + is_measure: true, + metadata: { + format: { number: { decimal_places: { places: 1000 } } }, + }, + }, + ], + }); + expect(cols[0].format).toBe(`#,##0.${"0".repeat(100)}`); + }); + + test("clamps a bare-number decimal_places to 100 as well", () => { + const cols = extractMetricColumns({ + columns: [ + { + name: "huge", + type: "DOUBLE", + is_measure: true, + metadata: { + format: { + currency: { decimal_places: 500, currency_code: "USD" }, + }, + }, + }, + ], + }); + expect(cols[0].format).toBe(`$#,##0.${"0".repeat(100)}`); + }); }); // ── Phase 5: metadata bundle generation ─────────────────────────────────── @@ -1530,6 +1728,62 @@ describe("buildMetricsMetadataBundle", () => { "year", ]); }); + + test("a __proto__ metric key and a __proto__ column name are emitted as own enumerable properties (no prototype pollution)", async () => { + // "__proto__" passes the metric key regex, so a config can genuinely + // declare it — and a workspace column can genuinely be named it. The + // bundle (and its per-entry maps) are null-prototype, so the write + // stores data instead of hitting the Object.prototype setter (which + // would swap the map's prototype and silently drop the key from the + // emitted JSON). Object-literal syntax would set the prototype at + // construction, so the config arrives via JSON.parse — exactly like the + // real metric-views.json read. + const config = JSON.parse( + '{"metricViews":{"__proto__":{"source":"demo.evil.proto"},"revenue":{"source":"demo.sales.revenue"}}}', + ); + const resolution = resolveMetricConfig(config); + expect(resolution.entries.map((e) => e.key)).toEqual([ + "__proto__", + "revenue", + ]); + + const fetcher = async () => + mockDescribeResponse({ + columns: [ + { name: "__proto__", type: "DOUBLE", is_measure: true }, + { name: "region", type: "STRING", is_measure: false }, + ], + }); + + const { schemas, failures } = await syncMetrics(resolution, fetcher); + expect(failures).toEqual([]); + + // Pre-serialization: own keys on the null-prototype maps. + const bundle = buildMetricsMetadataBundle(schemas); + expect(Object.hasOwn(bundle, "__proto__")).toBe(true); + expect(Object.hasOwn(bundle, "revenue")).toBe(true); + + // The emitted metrics.metadata.json carries both as own enumerable + // properties (JSON.parse creates own data properties for __proto__). + const parsed = JSON.parse(generateMetricsMetadataJson(schemas)); + expect(Object.keys(parsed)).toEqual(["__proto__", "revenue"]); + expect(Object.hasOwn(parsed, "__proto__")).toBe(true); + const protoEntry = Object.getOwnPropertyDescriptor( + parsed, + "__proto__", + )?.value; + expect(Object.hasOwn(protoEntry.measures, "__proto__")).toBe(true); + expect(Object.hasOwn(parsed.revenue.measures, "__proto__")).toBe(true); + expect( + Object.getOwnPropertyDescriptor(parsed.revenue.measures, "__proto__") + ?.value, + ).toEqual({ type: "DOUBLE" }); + + // And no global prototype pollution leaked out of the build. + expect(({} as Record).polluted).toBeUndefined(); + expect(({} as Record).measures).toBeUndefined(); + expect(Object.prototype).not.toHaveProperty("measures"); + }); }); // ── Phase 5: metadata JSON serialization ────────────────────────────────── @@ -1633,6 +1887,48 @@ describe("generateMetricsMetadataJson — snapshot", () => { }); }); +// ── Key-order determinism across artifacts: both emitters sort with ONE +// shared locale-independent (code-unit) comparator. localeCompare-style +// collation would interleave mixed-case keys ("ARPU", "churn", "Revenue") +// and could vary by machine/locale, drifting the .d.ts from the bundle. +describe("artifact key-order determinism", () => { + test("mixed-case keys order identically (code-unit) in metric.d.ts and metrics.metadata.json", async () => { + const resolution = resolveMetricConfig({ + metricViews: { + Revenue: { source: "a.b.r" }, + churn: { source: "a.b.c" }, + ARPU: { source: "a.b.a" }, + }, + }); + // Code-unit order puts uppercase before lowercase — NOT the + // case-insensitive interleaving locale collation would produce. + expect(resolution.entries.map((e) => e.key)).toEqual([ + "ARPU", + "Revenue", + "churn", + ]); + + const fetcher = async () => + mockDescribeResponse({ + columns: [{ name: "v", type: "DOUBLE", is_measure: true }], + }); + const { schemas } = await syncMetrics(resolution, fetcher); + + // Entry keys in the .d.ts appear as ` "": {` lines (4-space + // indent — metadata column maps sit deeper and don't match). + const declarations = generateMetricTypeDeclarations(schemas); + const dtsKeys = [...declarations.matchAll(/^ {4}"([^"]+)": \{$/gm)].map( + (m) => m[1], + ); + const bundleKeys = Object.keys( + JSON.parse(generateMetricsMetadataJson(schemas)), + ); + + expect(dtsKeys).toEqual(["ARPU", "Revenue", "churn"]); + expect(bundleKeys).toEqual(dtsKeys); + }); +}); + // ── Phase 2: syncMetrics propagates timeGrains end-to-end ──────────────── describe("syncMetrics — time-typed dimension propagation", () => { test("propagates inferred grains onto the resulting MetricSchema", async () => { diff --git a/packages/appkit/src/type-generator/tests/vite-plugin.test.ts b/packages/appkit/src/type-generator/tests/vite-plugin.test.ts index b9623113..5d6a6761 100644 --- a/packages/appkit/src/type-generator/tests/vite-plugin.test.ts +++ b/packages/appkit/src/type-generator/tests/vite-plugin.test.ts @@ -306,6 +306,36 @@ describe("appKitTypesPlugin — single-flight generate", () => { await flush(); expect(mocks.generateFromEntryPoint).toHaveBeenCalledTimes(1); }); + + test("a legacy-metric-views.json change does NOT regenerate; metric-views.json still does (basename match, not suffix)", async () => { + mocks.generateFromEntryPoint.mockResolvedValue(undefined); + + const plugin = makeConfiguredPlugin(); + const { server, watcher } = makeFakeServer(); + getHook(plugin, "configureServer")(server); + const buildStart = getHook(plugin, "buildStart"); + + await buildStart(); + await flush(); + expect(mocks.generateFromEntryPoint).toHaveBeenCalledTimes(1); + + // Suffix-matches "metric-views.json" but is a different file — the + // basename check must not fire for it. + watcher.emit( + "change", + path.join(process.cwd(), "config", "queries", "legacy-metric-views.json"), + ); + await flush(); + expect(mocks.generateFromEntryPoint).toHaveBeenCalledTimes(1); + + // The real config file still triggers. + watcher.emit( + "change", + path.join(process.cwd(), "config", "queries", "metric-views.json"), + ); + await flush(); + expect(mocks.generateFromEntryPoint).toHaveBeenCalledTimes(2); + }); }); describe("appKitTypesPlugin — metric option plumbing", () => { diff --git a/packages/appkit/src/type-generator/vite-plugin.ts b/packages/appkit/src/type-generator/vite-plugin.ts index 52996e75..a46cfcac 100644 --- a/packages/appkit/src/type-generator/vite-plugin.ts +++ b/packages/appkit/src/type-generator/vite-plugin.ts @@ -352,7 +352,10 @@ export function appKitTypesPlugin(options?: AppKitTypesPluginOptions): Plugin { if ( isWatchedFile && (changedFile.endsWith(".sql") || - changedFile.endsWith("metric-views.json")) + // Basename equality, not endsWith: a sibling like + // "legacy-metric-views.json" must not trigger a regenerate — + // only the real config file does. + path.basename(changedFile) === "metric-views.json") ) { // Route through the single-flight runner (was fire-and-forget // generate(), which could race the initial build / watch). This is a From d9b5546f84dcbd904dd5d59aac2755b5112371cb Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Fri, 12 Jun 2026 16:42:07 +0200 Subject: [PATCH 10/10] refactor(appkit): simplify metric typegen internals Behavior-preserving cleanup (snapshots byte-identical): failure-outcome helper tables D' classification; unified renderer block builders; currency map; parse-flag collapse; revival validator + cache-hash helper move into cache.ts; parallel-array removal; hoisted allowlists; stale phase-comment sweep. One semantic alignment: the Vite plugin now defers metric artifact defaults to the generator (sibling-of-outFile) instead of resolving its own, so plugin- and CLI-driven runs agree when outFile is customized. Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/cache.ts | 54 +++++ packages/appkit/src/type-generator/index.ts | 65 ++---- .../src/type-generator/metric-registry.ts | 215 ++++++++---------- .../tests/metric-registry.test.ts | 2 +- .../type-generator/tests/vite-plugin.test.ts | 25 +- .../appkit/src/type-generator/vite-plugin.ts | 43 ++-- 6 files changed, 197 insertions(+), 207 deletions(-) diff --git a/packages/appkit/src/type-generator/cache.ts b/packages/appkit/src/type-generator/cache.ts index 27424380..6f78deb4 100644 --- a/packages/appkit/src/type-generator/cache.ts +++ b/packages/appkit/src/type-generator/cache.ts @@ -10,6 +10,9 @@ const logger = createLogger("type-generator:cache"); * Cache types * @property hash - the hash of the SQL query * @property type - the type of the query + * @property retry - when true the entry never satisfies a cache hit, so the + * query is re-described on the next pass; fresh successful describes + * persist `retry: false` */ interface CacheEntry { hash: string; @@ -41,6 +44,48 @@ export interface MetricCacheEntry { retry: boolean; } +/** + * Structural gate for reviving a cached metric entry at partition time. + * + * The cache file lives in `node_modules/.databricks` and is plain JSON — + * hand-edits, truncation, or a stale writer can leave entries whose shape no + * longer matches {@link MetricCacheEntry}. A malformed entry must read as a + * cache MISS (re-describe) rather than crash the pass or render revived + * garbage into the artifacts. Checks exactly what the renderers and the + * metadata bundle consume: `hash` string, `retry` boolean, and a schema with + * `key`/`source` strings, a valid lane, an optional boolean `degraded`, and + * measure/dimension arrays whose elements carry `name`/`type` strings + * (other column fields are optional). Deliberately inline — the shared Zod + * schemas must not enter the type-generator's runtime path. + */ +export function isRevivableMetricCacheEntry(entry: MetricCacheEntry): boolean { + if (typeof entry.hash !== "string" || typeof entry.retry !== "boolean") { + return false; + } + const schema = entry.schema as unknown; + if (typeof schema !== "object" || schema === null || Array.isArray(schema)) { + return false; + } + const s = schema as Record; + const isColumnArray = (value: unknown): boolean => + Array.isArray(value) && + value.every( + (col) => + typeof col === "object" && + col !== null && + typeof (col as Record).name === "string" && + typeof (col as Record).type === "string", + ); + return ( + typeof s.key === "string" && + typeof s.source === "string" && + (s.lane === "sp" || s.lane === "obo") && + (s.degraded === undefined || typeof s.degraded === "boolean") && + isColumnArray(s.measures) && + isColumnArray(s.dimensions) + ); +} + /** * Cache interface * @property version - the version of the cache @@ -77,6 +122,15 @@ export function hashSQL(sql: string): string { return crypto.createHash("md5").update(sql).digest("hex"); } +/** + * Change detector stored on {@link MetricCacheEntry.hash}: md5 over + * `"|"` — the two config inputs that determine a DESCRIBE — + * so editing either invalidates the entry. + */ +export function metricCacheHash(source: string, lane: string): string { + return hashSQL(`${source}|${lane}`); +} + /** * Load the cache from the file system * If the cache is not found, run the query explain diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index 4053fb64..d8f770b3 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -4,7 +4,13 @@ import { WorkspaceClient } from "@databricks/sdk-experimental"; import dotenv from "dotenv"; import pc from "picocolors"; import { createLogger } from "../logging/logger"; -import { hashSQL, loadCache, type MetricCacheEntry, saveCache } from "./cache"; +import { + isRevivableMetricCacheEntry, + loadCache, + type MetricCacheEntry, + metricCacheHash, + saveCache, +} from "./cache"; import { createWorkspaceDescribeFetcher, type DescribeFetcher, @@ -259,48 +265,6 @@ async function probeWarehouseState( } } -/** - * Structural gate for reviving a cached metric entry at partition time. - * - * The cache file lives in `node_modules/.databricks` and is plain JSON — - * hand-edits, truncation, or a stale writer can leave entries whose shape no - * longer matches {@link MetricCacheEntry}. A malformed entry must read as a - * cache MISS (re-describe) rather than crash the pass or render revived - * garbage into the artifacts. Checks exactly what the renderers and the - * metadata bundle consume: `hash` string, `retry` boolean, and a schema with - * `key`/`source` strings, a valid lane, an optional boolean `degraded`, and - * measure/dimension arrays whose elements carry `name`/`type` strings - * (other column fields are optional). Deliberately inline — the shared Zod - * schemas must not enter the type-generator's runtime path. - */ -function isRevivableMetricCacheEntry(entry: MetricCacheEntry): boolean { - if (typeof entry.hash !== "string" || typeof entry.retry !== "boolean") { - return false; - } - const schema = entry.schema as unknown; - if (typeof schema !== "object" || schema === null || Array.isArray(schema)) { - return false; - } - const s = schema as Record; - const isColumnArray = (value: unknown): boolean => - Array.isArray(value) && - value.every( - (col) => - typeof col === "object" && - col !== null && - typeof (col as Record).name === "string" && - typeof (col as Record).type === "string", - ); - return ( - typeof s.key === "string" && - typeof s.source === "string" && - (s.lane === "sp" || s.lane === "obo") && - (s.degraded === undefined || typeof s.degraded === "boolean") && - isColumnArray(s.measures) && - isColumnArray(s.dimensions) - ); -} - /** * Entry point for generating type declarations from all imported files * @param options - the options for the generation @@ -427,8 +391,6 @@ export async function generateFromEntryPoint(options: { // describe-needed here. const hitSchemas = new Map(); const describeNeeded: typeof resolution.entries = []; - // Parallel to describeNeeded: the config hash to persist per key. - const neededHashes: string[] = []; // Hits whose cached schema is degraded are STICKY failures: a previous // pass pinned them with `retry: false` because re-describing the // unchanged entry can't succeed (deterministic DESCRIBE failure, or a @@ -437,12 +399,11 @@ export async function generateFromEntryPoint(options: { // collect them for the single notice below. const stickyDegradedHits: string[] = []; for (const entry of resolution.entries) { - const hash = hashSQL(`${entry.source}|${entry.lane}`); const prior = metricsSection[entry.key]; if ( prior !== undefined && isRevivableMetricCacheEntry(prior) && - prior.hash === hash && + prior.hash === metricCacheHash(entry.source, entry.lane) && !prior.retry ) { hitSchemas.set(entry.key, prior.schema); @@ -451,7 +412,6 @@ export async function generateFromEntryPoint(options: { } } else { describeNeeded.push(entry); - neededHashes.push(hash); } } @@ -682,10 +642,11 @@ export async function generateFromEntryPoint(options: { for (let i = 0; i < describeNeeded.length; i++) { // syncMetrics (and both .map(emptyMetricSchema) branches) return // one schema per entry in entry order, so described[i] always - // belongs to describeNeeded[i] / neededHashes[i]. - const failure = failureByKey.get(describeNeeded[i].key); - metricsSection[describeNeeded[i].key] = { - hash: neededHashes[i], + // belongs to describeNeeded[i]. + const entry = describeNeeded[i]; + const failure = failureByKey.get(entry.key); + metricsSection[entry.key] = { + hash: metricCacheHash(entry.source, entry.lane), schema: described[i], retry: described[i].degraded === true && diff --git a/packages/appkit/src/type-generator/metric-registry.ts b/packages/appkit/src/type-generator/metric-registry.ts index 28c6cbfa..54ac0e39 100644 --- a/packages/appkit/src/type-generator/metric-registry.ts +++ b/packages/appkit/src/type-generator/metric-registry.ts @@ -93,12 +93,12 @@ interface ResolvedMetricEntry { /** * Per-column metadata extracted from DESCRIBE TABLE EXTENDED ... AS JSON. * - * Phase 1 captured measure flags + types. Phase 2 widens to time-typed - * dimensions: grain qualification is inferred from the column's SQL type - * (TIMESTAMP* / DATE) — the UC metric-view YAML schema has no per-column - * `time_grain` attribute, so the type is the only signal available. + * Beyond measure flags + types, time-typed dimensions carry grain + * qualification inferred from the column's SQL type (TIMESTAMP* / DATE) — + * the UC metric-view YAML schema has no per-column `time_grain` attribute, + * so the type is the only signal available. * - * Phase 5 captures the YAML 1.1 semantic-metadata fields so the build-time + * The YAML 1.1 semantic-metadata fields are captured so the build-time * artifact is a complete record of what the metric view declares: display name * (used by `formatLabel` to render axis titles / legend entries / tooltips), * format spec (printf-like string consumed by `formatValue` and `toD3Format`), @@ -137,8 +137,8 @@ export interface MetricColumnMetadata { /** * Per-metric schema captured at type-generation time. * - * The full row type is the union of measure + dimension column types. Phase 1 - * uses only `measures`; Phase 2 widens to `dimensions` and `timeGrains`. + * The full row type is the union of measure + dimension column types; + * time-typed dimensions additionally carry their inferred `timeGrains`. */ export interface MetricSchema { /** Stable metric key (the map key under `metricViews` in metric-views.json). */ @@ -235,6 +235,15 @@ function isValidFqn(fqn: string): boolean { ); } +/** + * Field allowlists enforced by {@link resolveMetricConfig}. v1 explicitly + * rejects unknown top-level fields (so the legacy sp/obo lane shape — and + * future additions — cannot be silently consumed today) and unknown entry + * fields (same rationale, per entry). + */ +const ALLOWED_TOP_LEVEL_FIELDS = new Set(["$schema", "metricViews"]); +const ALLOWED_ENTRY_FIELDS = new Set(["source", "executor"]); + /** * Resolve the `metricViews` map into a flat list of entries. * @@ -252,11 +261,8 @@ function isValidFqn(fqn: string): boolean { export function resolveMetricConfig( config: MetricSourceConfig, ): MetricConfigResolution { - // v1 explicitly rejects unknown top-level fields so the legacy sp/obo lane - // shape (and future additions) cannot be silently consumed today. - const allowedTopLevel = new Set(["$schema", "metricViews"]); for (const field of Object.keys(config)) { - if (!allowedTopLevel.has(field)) { + if (!ALLOWED_TOP_LEVEL_FIELDS.has(field)) { throw new Error( `Invalid top-level field "${field}" in metric-views.json: only '$schema' and 'metricViews' are allowed.`, ); @@ -299,11 +305,8 @@ export function resolveMetricConfig( ); } - // v1 explicitly rejects unknown entry fields so future additions cannot - // be silently consumed today. - const allowed = new Set(["source", "executor"]); for (const field of Object.keys(entry)) { - if (!allowed.has(field)) { + if (!ALLOWED_ENTRY_FIELDS.has(field)) { throw new Error( `Invalid field "${field}" on metric entry "${key}": only 'source' and 'executor' are allowed at v1.`, ); @@ -412,8 +415,8 @@ export function parseDescribeTableExtendedJson( * * Tolerant of multiple JSON shapes (the field may be `columns` or `schema.fields`, * type may be a string or `{ name }` object, the measure marker may be `is_measure` - * or under `metadata.is_measure`). Phase 1's job is to find names + measure flags; - * later phases can tighten this if a more authoritative shape stabilizes. + * or under `metadata.is_measure`). The job here is to find names + measure + * flags; this can be tightened if a more authoritative shape stabilizes. */ export function extractMetricColumns(parsed: unknown): MetricColumnMetadata[] { if (!parsed || typeof parsed !== "object") { @@ -652,24 +655,18 @@ function readDecimalPlaces(obj: Record): number | undefined { * is never lost — `formatValue` and `toD3Format` will still render correctly, * just without a single-character glyph. */ +const CURRENCY_SYMBOLS: Record = { + USD: "$", + EUR: "€", + GBP: "£", + JPY: "¥", + CNY: "¥", + INR: "₹", + BRL: "R$", +}; + function currencySymbol(code: string): string { - switch (code) { - case "USD": - return "$"; - case "EUR": - return "€"; - case "GBP": - return "£"; - case "JPY": - case "CNY": - return "¥"; - case "INR": - return "₹"; - case "BRL": - return "R$"; - default: - return `${code} `; - } + return CURRENCY_SYMBOLS[code] ?? `${code} `; } /** @@ -709,7 +706,7 @@ function inferTimeGrains(type: string): string[] | undefined { /** * Map a Databricks SQL type to a TypeScript primitive. * Centralized here (not imported from query-registry) so this module - * stays self-contained at Phase 1. + * stays self-contained. */ function tsTypeFor(sqlType: string): string { const normalized = sqlType @@ -750,47 +747,36 @@ function renderMetricEntry(schema: MetricSchema): string { return renderDegradedMetricEntry(schema); } const indent = " "; - const measures = - schema.measures.length > 0 - ? schema.measures - .map( - (m) => `${indent}/** @sqlType ${m.type} */ -${indent}${JSON.stringify(m.name)}: ${tsTypeFor(m.type)}`, - ) - .join(";\n") - : ""; - const dimensions = - schema.dimensions.length > 0 - ? schema.dimensions - .map((d) => { - const grainComment = d.timeGrains?.length - ? ` @timeGrain ${d.timeGrains.join("|")}` - : ""; - return `${indent}/** @sqlType ${d.type}${grainComment} */ -${indent}${JSON.stringify(d.name)}: ${tsTypeFor(d.type)}`; - }) - .join(";\n") - : ""; - - const measureKeys = schema.measures.map((m) => JSON.stringify(m.name)); - const dimensionKeys = schema.dimensions.map((d) => JSON.stringify(d.name)); - - const measuresBlock = measures - ? `{ -${measures}; - }` - : "Record"; - - const dimensionsBlock = dimensions - ? `{ -${dimensions}; - }` - : "Record"; - - const measureUnion = - measureKeys.length > 0 ? measureKeys.join(" | ") : "never"; - const dimensionUnion = - dimensionKeys.length > 0 ? dimensionKeys.join(" | ") : "never"; + // One builder serves measures AND dimensions: the grain-comment leg is a + // no-op for measures, which never carry `timeGrains` (extractMetricColumns + // skips grain inference for them), so the rendered output is identical to + // a measure-specific variant for every reachable input. + const colsBlock = (cols: MetricColumnMetadata[]): string => { + if (cols.length === 0) return "Record"; + const fields = cols + .map((col) => { + const grainComment = col.timeGrains?.length + ? ` @timeGrain ${col.timeGrains.join("|")}` + : ""; + return `${indent}/** @sqlType ${col.type}${grainComment} */ +${indent}${JSON.stringify(col.name)}: ${tsTypeFor(col.type)}`; + }) + .join(";\n"); + return `{ +${fields}; + }`; + }; + const unionOf = (keys: string[]): string => + keys.length > 0 ? keys.join(" | ") : "never"; + + const measuresBlock = colsBlock(schema.measures); + const dimensionsBlock = colsBlock(schema.dimensions); + const measureUnion = unionOf( + schema.measures.map((m) => JSON.stringify(m.name)), + ); + const dimensionUnion = unionOf( + schema.dimensions.map((d) => JSON.stringify(d.name)), + ); // Union of allowed time-grains across every time-typed dimension. The PRD // documents the v1 contract: a single top-level `timeGrain` applies to all @@ -1016,7 +1002,7 @@ type MetricsMetadataBundle = Record; * Deterministic key order: outer object keys are sorted in locale-independent * code-unit order (see {@link compareKeys} — identical to the .d.ts order); * measures and dimensions are emitted in the order they appeared in DESCRIBE - * (Phase 1's preserved-from-YAML order), but each per-column object's fields + * (the preserved-from-YAML order), but each per-column object's fields * follow a fixed declaration order so snapshot diffs are stable. * * The output is `JSON.stringify`'d with two-space indentation by the file @@ -1291,6 +1277,20 @@ export async function syncMetrics( const schemas = new Array(entries.length); const failureSlots = new Array(entries.length); + // Shared shape for every failed outcome: degraded schema plus one failure + // record. Reason wording and transient classification stay at the call + // sites — this only removes the structural repetition. + const failedOutcome = ( + index: number, + entry: ResolvedMetricEntry, + reason: string, + transient: boolean, + ): MetricDescribeOutcome => ({ + index, + schema: emptyMetricSchema(entry), + failure: { key: entry.key, source: entry.source, reason, transient }, + }); + const describeOne = async ( entry: ResolvedMetricEntry, index: number, @@ -1302,16 +1302,7 @@ export async function syncMetrics( // The fetcher itself threw — a transport/auth blip, not a warehouse // verdict on the entry. Transient: a later pass may succeed unchanged. const reason = `DESCRIBE TABLE EXTENDED failed: ${(err as Error).message}`; - return { - index, - schema: emptyMetricSchema(entry), - failure: { - key: entry.key, - source: entry.source, - reason, - transient: true, - }, - }; + return failedOutcome(index, entry, reason, true); } // Non-terminal statement state (PENDING/RUNNING): the warehouse is @@ -1326,29 +1317,16 @@ export async function syncMetrics( return { index, schema: emptyMetricSchema(entry) }; } - let columns: MetricColumnMetadata[] = []; - let parseError: string | null = null; + let columns: MetricColumnMetadata[]; try { const parsed = parseDescribeTableExtendedJson(response); columns = extractMetricColumns(parsed); } catch (err) { - parseError = `Failed to extract columns from DESCRIBE response: ${(err as Error).message}`; - } - - if (parseError) { // Deterministic warehouse answer (FAILED statement, SUCCEEDED with zero // rows, unparseable payload): re-describing the same entry would fail // identically, so this failure is non-transient (sticky in the cache). - return { - index, - schema: emptyMetricSchema(entry), - failure: { - key: entry.key, - source: entry.source, - reason: parseError, - transient: false, - }, - }; + const reason = `Failed to extract columns from DESCRIBE response: ${(err as Error).message}`; + return failedOutcome(index, entry, reason, false); } if (columns.length === 0) { @@ -1357,21 +1335,11 @@ export async function syncMetrics( // recognize. Treat as a failure so CI catches it instead of letting an // empty bundle entry ship — the route's fail-closed gate would then // 503 every request to this metric in production. The schema is also - // degraded: its real columns are unknown. + // degraded: its real columns are unknown. Deterministic answer for + // this entry — non-transient, like the parse failures above. const reason = "DESCRIBE response yielded zero columns — check the response shape (top-level `columns` array or `schema.fields`)."; - return { - index, - schema: emptyMetricSchema(entry), - // Deterministic answer for this entry — non-transient, like the - // parse failures above. - failure: { - key: entry.key, - source: entry.source, - reason, - transient: false, - }, - }; + return failedOutcome(index, entry, reason, false); } const measures = columns.filter((c) => c.isMeasure); @@ -1420,15 +1388,16 @@ export async function syncMetrics( result.reason instanceof Error ? result.reason.message : String(result.reason); - schemas[index] = emptyMetricSchema(entry); // Unknown cause — prefer convergence: mark transient so the next // describe-capable pass retries instead of pinning a surprise. - failureSlots[index] = { - key: entry.key, - source: entry.source, - reason: `DESCRIBE TABLE EXTENDED failed: ${message}`, - transient: true, - }; + const { schema, failure } = failedOutcome( + index, + entry, + `DESCRIBE TABLE EXTENDED failed: ${message}`, + true, + ); + schemas[index] = schema; + failureSlots[index] = failure; } } } diff --git a/packages/appkit/src/type-generator/tests/metric-registry.test.ts b/packages/appkit/src/type-generator/tests/metric-registry.test.ts index 3612e620..c8ff9b52 100644 --- a/packages/appkit/src/type-generator/tests/metric-registry.test.ts +++ b/packages/appkit/src/type-generator/tests/metric-registry.test.ts @@ -32,7 +32,7 @@ import type { DatabricksStatementExecutionResponse } from "../types"; * } * ``` * - * Phase 1 mocks this. Live integration ships in Phase 7. + * Unit tests mock this; live warehouse integration is exercised separately. */ function mockDescribeResponse( payload: unknown, diff --git a/packages/appkit/src/type-generator/tests/vite-plugin.test.ts b/packages/appkit/src/type-generator/tests/vite-plugin.test.ts index 5d6a6761..0c6a7fcf 100644 --- a/packages/appkit/src/type-generator/tests/vite-plugin.test.ts +++ b/packages/appkit/src/type-generator/tests/vite-plugin.test.ts @@ -38,10 +38,8 @@ vi.mock("@databricks/sdk-experimental", () => ({ const { appKitTypesPlugin } = await import("../vite-plugin"); // Real constant values: the "../index" mock spreads the actual module, so these -// are the genuine defaults the plugin resolves its metric out-paths from. -const { METRIC_METADATA_FILE, METRIC_TYPES_FILE, TYPES_DIR } = await import( - "../index" -); +// are the genuine defaults the plugin resolves outFile from. +const { ANALYTICS_TYPES_FILE, TYPES_DIR } = await import("../index"); // The plugin hooks are loosely typed on Vite's Plugin; cast to the shapes we // actually drive so we can call them directly without a Vite build. @@ -363,22 +361,23 @@ describe("appKitTypesPlugin — metric option plumbing", () => { else process.env.DATABRICKS_WAREHOUSE_ID = savedWarehouseId; }); - test("defaults the metric out files to shared/ siblings of outFile", async () => { + test("passes unset metric out files through as undefined so the generator defaults them to siblings of outFile", async () => { await runPlugin(); await flush(); - // configResolved resolves both metric paths against projectRoot - // (config.root/..) exactly like outFile. + // configResolved resolves only outFile (and explicitly-provided metric + // paths) against projectRoot (config.root/..). Unset metric options stay + // undefined so generateFromEntryPoint computes its sibling-of-outFile + // defaults — identical final paths in the all-defaults case, since the + // default outFile below lives in shared//. expect(mocks.generateFromEntryPoint).toHaveBeenCalledWith( expect.objectContaining({ - metricOutFile: path.resolve( + outFile: path.resolve( process.cwd(), - `shared/${TYPES_DIR}/${METRIC_TYPES_FILE}`, - ), - metricMetadataOutFile: path.resolve( - process.cwd(), - `shared/${TYPES_DIR}/${METRIC_METADATA_FILE}`, + `shared/${TYPES_DIR}/${ANALYTICS_TYPES_FILE}`, ), + metricOutFile: undefined, + metricMetadataOutFile: undefined, }), ); }); diff --git a/packages/appkit/src/type-generator/vite-plugin.ts b/packages/appkit/src/type-generator/vite-plugin.ts index a46cfcac..d5c462a4 100644 --- a/packages/appkit/src/type-generator/vite-plugin.ts +++ b/packages/appkit/src/type-generator/vite-plugin.ts @@ -6,8 +6,6 @@ import { createLogger } from "../logging/logger"; import { ANALYTICS_TYPES_FILE, generateFromEntryPoint, - METRIC_METADATA_FILE, - METRIC_TYPES_FILE, TYPES_DIR, TypegenFatalError, TypegenSyntaxError, @@ -35,11 +33,15 @@ const DEV_WAREHOUSE_WATCH_MAX_MS = 60_000; interface AppKitTypesPluginOptions { /* Path to the output d.ts file (relative to client folder). */ outFile?: string; - /** Path to the metric registry d.ts file (relative to client folder). */ + /** + * Path to the metric registry d.ts file (relative to client folder). + * Defaults to a sibling of `outFile`, computed by the generator. + */ metricOutFile?: string; /** * Path to the metric semantic-metadata JSON file (relative to client folder). - * Build-time artifact — sibling of {@link metricOutFile}. Skipped + * Build-time artifact — defaults to a sibling of {@link metricOutFile} + * (itself a sibling of `outFile`), computed by the generator. Skipped * automatically when `metric-views.json` is absent. */ metricMetadataOutFile?: string; @@ -55,8 +57,8 @@ interface AppKitTypesPluginOptions { */ export function appKitTypesPlugin(options?: AppKitTypesPluginOptions): Plugin { let outFile: string; - let metricOutFile: string; - let metricMetadataOutFile: string; + let metricOutFile: string | undefined; + let metricMetadataOutFile: string | undefined; let watchFolders: string[]; // Single-flight state for runGenerate(). `inFlight` is the promise of the @@ -196,9 +198,9 @@ export function appKitTypesPlugin(options?: AppKitTypesPluginOptions): Plugin { * * Post-probe behaviour by state: * - RUNNING → describe right away (the dev foreground degraded, so a running - * warehouse would otherwise never get real types — this is the case Phase 3 - * restores). `waitUntilRunning` returns immediately for an already-running - * warehouse, then the blocking regenerate fires. + * warehouse would otherwise never get real types). `waitUntilRunning` + * returns immediately for an already-running warehouse, then the blocking + * regenerate fires. * - STARTING → it's already coming up; just wait for RUNNING, then describe. * - STOPPED / STOPPING → kick off a start, wait for RUNNING, then describe. * - DELETED / DELETING → return (a deleted warehouse can't be started, and @@ -310,15 +312,20 @@ export function appKitTypesPlugin(options?: AppKitTypesPluginOptions): Plugin { projectRoot, options?.outFile ?? `shared/${TYPES_DIR}/${ANALYTICS_TYPES_FILE}`, ); - metricOutFile = path.resolve( - projectRoot, - options?.metricOutFile ?? `shared/${TYPES_DIR}/${METRIC_TYPES_FILE}`, - ); - metricMetadataOutFile = path.resolve( - projectRoot, - options?.metricMetadataOutFile ?? - `shared/${TYPES_DIR}/${METRIC_METADATA_FILE}`, - ); + // Metric out-paths resolve against projectRoot only when explicitly + // provided; unset options pass through as undefined so the generator + // computes its sibling-of-outFile defaults. In the all-defaults case + // the final paths are identical (the default outFile above lives in + // shared//), and a customized outFile now keeps its metric + // siblings next to it instead of pinning them under shared/. + metricOutFile = + options?.metricOutFile !== undefined + ? path.resolve(projectRoot, options.metricOutFile) + : undefined; + metricMetadataOutFile = + options?.metricMetadataOutFile !== undefined + ? path.resolve(projectRoot, options.metricMetadataOutFile) + : undefined; watchFolders = options?.watchFolders ?? [ path.join(process.cwd(), "config", "queries"), ];