From 31cc1cfaf643d63d03bbf779db4360a169a6ec68 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Mon, 22 Jun 2026 20:32:13 +0200 Subject: [PATCH 1/6] feat(shared): appkit mv sync CLI (metric views) Adds an appkit mv sync command that fetches Unity Catalog metric-view schemas and emits metric.d.ts plus metrics.metadata.json outside the Vite dev loop (CI, non-Vite builds, manual refresh). The command lives in shared and reaches appkit's sync core via dynamic import of the type-generator entry with an ambient declaration and a graceful appkit-absent fallback, so shared keeps no static appkit dependency. A new appkit syncMetricViewsTypes export reuses the existing metric writers, adaptive describe fetcher and persistent cache helpers, so the emitted bundle matches the Vite plugin output. Config is validated against metricSourceSchema before sync, an absent default file exits zero for dormancy while error modes exit non-zero with distinct messages, and interactive and non-interactive flows mirror plugin create. Flags are --warehouse-id, --metric-views-json-path, --output-dir and --no-cache. Fourth change in the metric-views decomposition after #427, #429 and #433. Co-authored-by: Isaac Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/index.ts | 796 +++++++++++------- .../tests/sync-metric-views-types.test.ts | 350 ++++++++ packages/appkit/tsdown.config.ts | 9 +- .../src/cli/commands/metric-views/index.ts | 20 + .../commands/metric-views/sync/sync.test.ts | 646 ++++++++++++++ .../cli/commands/metric-views/sync/sync.ts | 422 ++++++++++ .../validate-metric-views-source.test.ts | 185 ++++ .../validate-metric-views-source.ts | 79 ++ .../src/cli/commands/type-generator.d.ts | 51 ++ packages/shared/src/cli/index.ts | 2 + 10 files changed, 2241 insertions(+), 319 deletions(-) create mode 100644 packages/appkit/src/type-generator/tests/sync-metric-views-types.test.ts create mode 100644 packages/shared/src/cli/commands/metric-views/index.ts create mode 100644 packages/shared/src/cli/commands/metric-views/sync/sync.test.ts create mode 100644 packages/shared/src/cli/commands/metric-views/sync/sync.ts create mode 100644 packages/shared/src/cli/commands/metric-views/validate-metric-views-source.test.ts create mode 100644 packages/shared/src/cli/commands/metric-views/validate-metric-views-source.ts diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index 90dcaae7d..83fe4def5 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -333,348 +333,508 @@ export async function generateFromEntryPoint(options: { // 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. + // empty noise. Delegate to the unified metric pipeline in + // syncMetricViewsTypes, forwarding this run's mode verbatim: `non-blocking` + // keeps its status-only #406 gate, `blocking` keeps its preflight, and both + // keep last-known-good cache serving + the sticky-degraded notice. The + // unified fn returns early with `noConfig: true` when metric-views.json is + // absent, so the additive "only when it exists" behavior is preserved here by + // simply ignoring that flag. Fatal preflight errors come back in + // `fatalErrors` (empty except for a deleted/deleting warehouse in blocking + // mode) so the end-of-run throw below surfaces them after the writes, exactly + // as the inline block did. if (queryFolder) { - const mvConfig = await readMetricConfig(queryFolder); - if (mvConfig) { - const resolution = resolveMetricConfig(mvConfig); - - // Metric schemas persist in the shared typegen cache as a `metrics` - // section (sibling of `queries`, same file/version), keyed by metric key - // with md5("|") as the change detector. Loaded strictly - // AFTER the query path's own load → mutate → save cycle, so the single - // metric-side save below 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 mvCacheSection: Record = - Object.create(null); - if (!noCache && cache.metrics) { - for (const key of Object.keys(cache.metrics)) { - mvCacheSection[key] = cache.metrics[key]; - } - } + const mvFile = + mvOutFile ?? path.join(path.dirname(outFile), METRIC_TYPES_FILE); + const mvMetadataFile = + mvMetadataOutFile ?? + path.join(path.dirname(mvFile), METRIC_METADATA_FILE); + const mvResult = await syncMetricViewsTypes({ + queryFolder, + warehouseId, + metricOutFile: mvFile, + metricMetadataOutFile: mvMetadataFile, + cache: !noCache, + metricFetcher, + mode, + }); + for (const fe of mvResult.fatalErrors) { + fatalErrors.push(fe); + } + } - // Partition BEFORE any gate/preflight decision: a hit (structurally valid - // entry, hash match, not retry-flagged) is served from cache no matter - // what the warehouse is doing — a degraded pass falls back to - // last-known-good schemas, exactly like queries degrade to cached types. - // Only the remainder (new, edited, retry-flagged, or unrevivable entries) - // is eligible for DESCRIBE, so a fully-warm pass makes zero warehouse - // calls and constructs zero clients. - const hitSchemas = new Map(); - const describeNeeded: typeof resolution.entries = []; - // Degraded cached schemas pinned `retry: false` are sticky failures: they - // serve their permissive schema like any hit, but are collected here for - // the single notice below so the misconfiguration isn't silently hidden. - const stickyDegradedHits: string[] = []; - for (const entry of resolution.entries) { - const prior = mvCacheSection[entry.key]; - if ( - prior !== undefined && - isRevivableMetricCacheEntry(prior) && - prior.hash === metricCacheHash(entry.source, entry.lane) && - !prior.retry - ) { - hitSchemas.set(entry.key, prior.schema); - if (prior.schema.degraded === true) { - stickyDegradedHits.push(entry.key); - } - } else { - describeNeeded.push(entry); - } - } + // One-time migration: remove old generated file and patch project configs + await removeOldGeneratedTypes(projectRoot, "appKitTypes.d.ts"); + await migrateProjectConfig(projectRoot); - 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(", "), - ); - } + // Types are always written above — including `result: unknown` for any query + // that could not be described. Connectivity failures pass silently so a + // transient warehouse outage never blocks a build; genuine SQL errors and + // non-connectivity fatal request failures surface after the file write. + if (syntaxErrors.length > 0) { + throw new TypegenSyntaxError(syntaxErrors, warehouseId, fatalErrors); + } + if (fatalErrors.length > 0) { + throw new TypegenFatalError(fatalErrors, warehouseId); + } - // At most ONE WorkspaceClient per pass for the whole metric path: the - // status probe, the blocking preflight, and the default DESCRIBE fetcher - // share this lazily-created instance, so a pass that never contacts the - // warehouse constructs zero clients. - let mvClient: WorkspaceClient | undefined; - const getMvClient = (): WorkspaceClient => { - mvClient ??= new WorkspaceClient({}); - return mvClient; - }; - - // Blocking-mode preflight: ensure the warehouse is running before the - // DESCRIBE batch (probe → decide → wait / start+wait; only - // DELETED/DELETING is fatal). Deliberately split from the query path's - // preflight — metric views may bind a different warehouse in future. Two - // softenings vs the query preflight: a failed probe and a timed-out wait - // are NOT fatal here — we fall through to syncMetrics, which classifies a - // still-not-ready warehouse as degraded rather than failing the build. - let preflightFatalMessage: string | undefined; - if ( - mode === "blocking" && - metricFetcher === undefined && - describeNeeded.length > 0 - ) { - try { - const state = await getWarehouseState(getMvClient(), warehouseId); - const decision = decidePreflight(state, mode); - if (decision === "fatal") { - preflightFatalMessage = `warehouse ${warehouseId} is ${state}`; - } else if (decision === "startWaitProceed") { - // treatStoppedAsTransient rides out the stale pre-start - // STOPPED/STOPPING reading, same as the query preflight. - await startWarehouse(getMvClient(), warehouseId); - const settled = await waitUntilRunning(getMvClient(), warehouseId, { - maxMs: MV_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") { - const settled = await waitUntilRunning(getMvClient(), warehouseId, { - maxMs: MV_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}`; - } - } - } catch (err) { - // Connectivity blip: fall through to syncMetrics, whose DESCRIBEs - // degrade a not-ready / unreachable warehouse rather than throwing. A - // deterministic failure (auth, bad warehouse id, a timed-out start) - // is fatal — surface it instead of stalling ~5 min against a - // not-ready warehouse, mirroring the query path's preflight catch. - if (!isConnectivityError(err)) { - preflightFatalMessage = `warehouse ${warehouseId}: ${getErrorDiagnostic(err)}`; - } - } - } + logger.debug("Type generation complete!"); +} - // Honor the non-blocking preflight contract (#406) for metric DESCRIBEs: - // a `DESCRIBE TABLE EXTENDED ... AS JSON` waits up to 30s per key and - // auto-starts a stopped warehouse — exactly what "non-blocking" promises - // not to do. So one status-only probe (which can't start the warehouse) - // decides whether to DESCRIBE now or emit degraded artifacts for a later - // blocking run; it keeps the observed state so the skip can tell a - // transient not-running warehouse from a terminal DELETED/DELETING one. - let gateState: WarehouseState | undefined; - let describeNow = - metricFetcher !== undefined || - mode !== "non-blocking" || - describeNeeded.length === 0; - if (!describeNow) { - try { - gateState = await probeWarehouseState(getMvClient, warehouseId); - } catch (err) { - // probeWarehouseState only throws on a deterministic failure (auth, - // bad warehouse id) — a connectivity blip already returned undefined. - // Pin it fatal through the same path as a fatal blocking preflight. - preflightFatalMessage = `warehouse ${warehouseId}: ${getErrorDiagnostic(err)}`; - } - describeNow = gateState === "RUNNING"; - } +/** + * Result of a {@link syncMetricViewsTypes} run, returned to the caller (the CLI + * directly, or {@link generateFromEntryPoint} which delegates to it) so it can + * report what happened and decide its exit code. + */ +export interface SyncMetricViewsTypesResult { + /** Absolute path the MetricRegistry `.d.ts` was written to (undefined when no config). */ + metricOutFile?: string; + /** Absolute path the semantic-metadata JSON bundle was written to (undefined when no config). */ + metricMetadataOutFile?: string; + /** Schemas emitted, one per configured metric key (empty when no config). */ + schemas: MetricSchema[]; + /** Per-entry DESCRIBE failures surfaced by {@link syncMetrics}. */ + failures: MetricSyncFailure[]; + /** + * `true` when no `metric-views.json` was found in the query folder, so nothing + * was synced. The metric path is additive — its absence is not an error. + */ + noConfig: boolean; + /** + * Per-key fatal preflight errors (empty except in the `blocking`-mode + * deleted/deleting-warehouse and deterministic-preflight-failure cases). The + * artifacts are still written; {@link generateFromEntryPoint} surfaces these + * by throwing {@link TypegenFatalError} after the writes. The CLI never sets + * `mode`, so for `"describe-now"` this is always empty. + */ + fatalErrors: Array<{ name: string; message: string }>; +} - let described: MetricSchema[]; - let failures: MetricSyncFailure[] = []; - // True when this pass skipped DESCRIBE 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. - let terminalSkip = false; - if (preflightFatalMessage !== undefined) { - // Fatal preflight (deleted/deleting warehouse): fail like the query - // path — skip DESCRIBE, emit degraded schemas so both artifacts are - // still written, and record one fatal error per describe-needed key - // (cache hits are unaffected). The end-of-run throw below surfaces them - // after the writes. Terminal, so these entries are pinned sticky. - described = describeNeeded.map(emptyMetricSchema); - terminalSkip = true; - for (const entry of describeNeeded) { - fatalErrors.push({ name: entry.key, message: preflightFatalMessage }); - } - } else if (describeNeeded.length === 0) { - // Nothing left to describe — every configured key was a cache hit. - // syncMetrics would be a no-op (and building its fetcher would - // construct a client for nothing); artifacts regenerate from cache. - described = []; - } else if (describeNow) { - const fetcher = - metricFetcher ?? - createWorkspaceDescribeFetcher(getMvClient(), warehouseId); - ({ schemas: described, failures } = await syncMetrics( - { entries: describeNeeded }, - fetcher, - )); - - // Surface DESCRIBE failures loudly: a misconfigured metric-views.json - // would otherwise silently ship an empty entry that the runtime - // fail-closed gate 503s in production. syncMetrics 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, - ); - } - } +/** + * Unified metric-view type-generation pipeline. Backs BOTH the `appkit mv sync` + * CLI (default `"describe-now"` mode) and {@link generateFromEntryPoint}'s + * metric section (which forwards its dev `"non-blocking"`/`"blocking"` mode). + * + * It does the focused metric pipeline ONLY — it never describes analytics + * queries and never writes `analytics.d.ts` / `serving.d.ts`. The pipeline: + * read config ({@link readMetricConfig}) → resolve ({@link resolveMetricConfig}) + * → partition cache hits vs describe-needed → optional warehouse preflight / + * #406 status gate → describe ({@link syncMetrics} over + * {@link createWorkspaceDescribeFetcher}) → persist + prune the `metrics` + * cache section → merge → write `metric.d.ts` + * ({@link generateMetricTypeDeclarations}) and `metrics.metadata.json` + * ({@link generateMetricsMetadataJson}). + * + * The shared typegen cache (the `metrics` section of `.appkit-types-cache.json`, + * same {@link metricCacheHash} change-detector and {@link MetricCacheEntry} + * shape) means a second run over an unchanged, healthy config makes zero + * warehouse calls. `cache === false` (the CLI's `--no-cache`) ignores the cached + * section entirely (every key becomes describe-needed) and overwrites it with + * this pass's results. + * + * The `mode` toggle is the ONLY axis that differs between callers: + * - `"describe-now"` (default, the CLI): no preflight, no #406 status probe — + * DESCRIBE every key that isn't a clean cache hit. The hit predicate is + * STRICTER here: a degraded/sticky cached entry is NEVER served (it is + * re-described), so a focused `mv sync` always converges to correct types, + * and the sticky-degraded notice never fires (nothing degraded is served). + * - `"non-blocking"` (dev/Vite default): honor the #406 contract — one + * status-only probe, DESCRIBE only when the warehouse is already RUNNING, + * else emit degraded artifacts immediately. Degraded cache hits ARE served + * (last-known-good) and surfaced via the sticky-degraded notice. + * - `"blocking"`: wait for / start the warehouse first (only a + * deleted/deleting one is fatal), then DESCRIBE. Degraded cache hits are + * served, same as non-blocking. A fatal preflight is reported via + * {@link SyncMetricViewsTypesResult.fatalErrors} (the artifacts are still + * written) so the caller can throw after the writes. + * + * An injected `metricFetcher` always runs — it hits no warehouse, so it bypasses + * both the blocking preflight and the non-blocking gate regardless of mode. + * + * @param options.queryFolder - folder that holds `metric-views.json` + * (conventionally `/config/queries`). Returns early with + * `noConfig: true` when the file is absent — additive, never an error. + * @param options.warehouseId - SQL warehouse used for `DESCRIBE TABLE EXTENDED`. + * @param options.metricOutFile - output path for the MetricRegistry `.d.ts`. + * @param options.metricMetadataOutFile - output path for the semantic-metadata + * JSON bundle. + * @param options.cache - cache toggle, default ON. Only `cache === false` + * disables it (so `undefined`/`true` keep caching). Mirrors the `noCache` + * convention on {@link generateFromEntryPoint}: gate the cache READ + * (`!noCache`) and overwrite the `metrics` section on SAVE. + * @param options.metricFetcher - optional injected {@link DescribeFetcher} + * (tests pass a mock; production lazily builds a WorkspaceClient-backed one). + * @param options.mode - preflight/gate policy, default `"describe-now"`. See + * above; the CLI omits it (taking `"describe-now"`), + * {@link generateFromEntryPoint} forwards its own {@link PreflightMode}. + */ +export async function syncMetricViewsTypes(options: { + queryFolder: string; + warehouseId: string; + metricOutFile: string; + metricMetadataOutFile: string; + cache?: boolean; + metricFetcher?: DescribeFetcher; + mode?: "describe-now" | "non-blocking" | "blocking"; +}): Promise { + const { + queryFolder, + warehouseId, + metricOutFile, + metricMetadataOutFile, + cache: cacheEnabled, + metricFetcher, + mode = "describe-now", + } = 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 = described - .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 { - // Un-probed DESCRIBEs deliberately skipped, not failures: emit each - // describe-needed key as a degraded schema (permissive types) so both - // artifacts exist; cache hits keep serving last-known-good. A transient - // state refreshes on a later RUNNING pass; a DELETED/DELETING probe is - // terminal, so those keys are pinned sticky below. - 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, - describeNeeded.length, - describeNeeded.map((e) => e.key).join(", "), - ); - } + // Only `cache === false` disables caching; `undefined`/`true` keep it on. + const noCache = cacheEnabled === false; - // Persist outcomes for exactly the keys this pass owned (the - // describe-needed set); hits were partitioned out above and are never - // rewritten, so a warehouse-down pass keeps last-known-good entries. A - // successful DESCRIBE caches `retry: false`; a degraded outcome caches - // `retry: true` only when re-describing could later succeed (non-terminal - // state or transient failure), else sticky `retry: false`. One save per - // pass; with `noCache` the section started empty, so it's overwritten. - 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]. - const entry = describeNeeded[i]; - const failure = failureByKey.get(entry.key); - mvCacheSection[entry.key] = { - hash: metricCacheHash(entry.source, entry.lane), - schema: described[i], - retry: - described[i].degraded === true && - !terminalSkip && - (failure === undefined || failure.transient === true), - }; + const mvConfig = await readMetricConfig(queryFolder); + if (!mvConfig) { + // No metric-views.json — additive path stays dormant. The CLI turns this + // into a friendly "nothing to sync" message and exits 0; + // generateFromEntryPoint simply ignores `noConfig`. + return { schemas: [], failures: [], fatalErrors: [], noConfig: true }; + } + + const resolution = resolveMetricConfig(mvConfig); + + const fatalErrors: Array<{ name: string; message: string }> = []; + + // Load the shared typegen cache and copy its `metrics` section into a + // null-prototype map. Metric keys are user-controlled config and + // "__proto__"/"constructor" pass the metric key regex — a null prototype + // keeps a malicious/edge key from hitting an Object.prototype setter on write + // or resolving inherited names as phantom entries on read. With `noCache`, the + // section starts empty (every entry describe-needed) and is overwritten on + // save below. + const cache = await loadCache(); + const mvCacheSection: Record = Object.create(null); + if (!noCache && cache.metrics) { + for (const key of Object.keys(cache.metrics)) { + mvCacheSection[key] = cache.metrics[key]; + } + } + + // Dev modes (`non-blocking`/`blocking`) serve degraded cache hits as + // last-known-good — exactly like queries degrade to cached types — and + // surface them via the sticky-degraded notice. `describe-now` (the CLI) is an + // explicit "make my types correct now" action, so it NEVER serves a + // degraded/sticky entry: that entry is re-described instead, and no degraded + // hit is served, so the notice never fires. + const serveDegraded = mode !== "describe-now"; + + // Partition BEFORE any gate/preflight decision: a hit (structurally valid + // entry, hash match, not retry-flagged, and — unless serving degraded — not + // degraded) is served from cache no matter what the warehouse is doing. Only + // the remainder (new, edited, retry-flagged, unrevivable, or — in + // `describe-now` — degraded entries) is eligible for DESCRIBE, so a + // fully-warm pass makes zero warehouse calls and constructs zero clients. + const hitSchemas = new Map(); + const describeNeeded: typeof resolution.entries = []; + // Degraded cached schemas pinned `retry: false` that are SERVED as hits are + // sticky failures: they serve their permissive schema, but are collected here + // for the single notice below so the misconfiguration isn't silently hidden. + // (Empty in `describe-now`, which never serves a degraded hit.) + const stickyDegradedHits: string[] = []; + for (const entry of resolution.entries) { + const prior = mvCacheSection[entry.key]; + if ( + prior !== undefined && + isRevivableMetricCacheEntry(prior) && + prior.hash === metricCacheHash(entry.source, entry.lane) && + !prior.retry && + (serveDegraded || prior.schema.degraded !== true) + ) { + hitSchemas.set(entry.key, prior.schema); + if (prior.schema.degraded === true) { + stickyDegradedHits.push(entry.key); } + } else { + describeNeeded.push(entry); + } + } - // 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(mvCacheSection)) { - if (!configuredKeys.has(key)) { - delete mvCacheSection[key]; - prunedCount++; + 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 pass for the whole metric path: the status + // probe, the blocking preflight, and the default DESCRIBE fetcher share this + // lazily-created instance, so a pass that never contacts the warehouse + // constructs zero clients. + let mvClient: WorkspaceClient | undefined; + const getMvClient = (): WorkspaceClient => { + mvClient ??= new WorkspaceClient({}); + return mvClient; + }; + + // Blocking-mode preflight: ensure the warehouse is running before the DESCRIBE + // batch (probe → decide → wait / start+wait; only DELETED/DELETING is fatal). + // Two softenings vs the query preflight: a failed probe and a timed-out wait + // are NOT fatal here — we fall through to syncMetrics, which classifies a + // still-not-ready warehouse as degraded rather than failing the build. Skipped + // for `describe-now`/`non-blocking` (only `mode === "blocking"` enters here). + let preflightFatalMessage: string | undefined; + if ( + mode === "blocking" && + metricFetcher === undefined && + describeNeeded.length > 0 + ) { + try { + const state = await getWarehouseState(getMvClient(), warehouseId); + const decision = decidePreflight(state, mode); + if (decision === "fatal") { + preflightFatalMessage = `warehouse ${warehouseId} is ${state}`; + } else if (decision === "startWaitProceed") { + // treatStoppedAsTransient rides out the stale pre-start STOPPED/STOPPING + // reading, same as the query preflight. + await startWarehouse(getMvClient(), warehouseId); + const settled = await waitUntilRunning(getMvClient(), warehouseId, { + maxMs: MV_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") { + const settled = await waitUntilRunning(getMvClient(), warehouseId, { + maxMs: MV_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}`; } } - - // 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 = mvCacheSection; - await saveCache(cache); + } catch (err) { + // Connectivity blip: fall through to syncMetrics, whose DESCRIBEs degrade + // a not-ready / unreachable warehouse rather than throwing. A + // deterministic failure (auth, bad warehouse id, a timed-out start) is + // fatal — surface it instead of stalling ~5 min against a not-ready + // warehouse, mirroring the query path's preflight catch. + if (!isConnectivityError(err)) { + preflightFatalMessage = `warehouse ${warehouseId}: ${getErrorDiagnostic(err)}`; } + } + } - // 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 mvSchemas = resolution.entries.map((entry) => { - const schema = - hitSchemas.get(entry.key) ?? describedByKey.get(entry.key); - if (schema !== undefined) return schema; - // Defensive: every entry is either a cache hit or describe-needed (and - // every describe-needed entry yields exactly one schema above), so this - // should be unreachable. If the invariant ever breaks, warn loudly but - // still emit a permissive degraded schema — the metric path never - // crashes a build over a single entry. + // Honor the non-blocking preflight contract (#406) for metric DESCRIBEs: a + // `DESCRIBE TABLE EXTENDED ... AS JSON` waits up to 30s per key and auto-starts + // a stopped warehouse — exactly what "non-blocking" promises not to do. So one + // status-only probe (which can't start the warehouse) decides whether to + // DESCRIBE now or emit degraded artifacts for a later blocking run; it keeps + // the observed state so the skip can tell a transient not-running warehouse + // from a terminal DELETED/DELETING one. `describe-now` and `blocking` both + // start `describeNow = true` (`mode !== "non-blocking"`), so this gate is + // skipped for them — `describe-now` describes directly, `blocking` already ran + // its preflight above. + let gateState: WarehouseState | undefined; + let describeNow = + metricFetcher !== undefined || + mode !== "non-blocking" || + describeNeeded.length === 0; + if (!describeNow) { + try { + gateState = await probeWarehouseState(getMvClient, warehouseId); + } catch (err) { + // probeWarehouseState only throws on a deterministic failure (auth, bad + // warehouse id) — a connectivity blip already returned undefined. Pin it + // fatal through the same path as a fatal blocking preflight. + preflightFatalMessage = `warehouse ${warehouseId}: ${getErrorDiagnostic(err)}`; + } + describeNow = gateState === "RUNNING"; + } + + let described: MetricSchema[]; + let failures: MetricSyncFailure[] = []; + // True when this pass skipped DESCRIBE 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. Never set in + // `describe-now` (no preflight/gate runs there). + let terminalSkip = false; + if (preflightFatalMessage !== undefined) { + // Fatal preflight (deleted/deleting warehouse): fail like the query path — + // skip DESCRIBE, emit degraded schemas so both artifacts are still written, + // and record one fatal error per describe-needed key (cache hits are + // unaffected). The caller surfaces them after the writes. Terminal, so these + // entries are pinned sticky. + described = describeNeeded.map(emptyMetricSchema); + terminalSkip = true; + for (const entry of describeNeeded) { + fatalErrors.push({ name: entry.key, message: preflightFatalMessage }); + } + } else if (describeNeeded.length === 0) { + // Nothing left to describe — every configured key was a cache hit. + // syncMetrics would be a no-op (and building its fetcher would construct a + // client for nothing); artifacts regenerate from cache. + described = []; + } else if (describeNow) { + const fetcher = + metricFetcher ?? + createWorkspaceDescribeFetcher(getMvClient(), warehouseId); + ({ schemas: described, failures } = await syncMetrics( + { entries: describeNeeded }, + fetcher, + )); + + // Surface DESCRIBE failures loudly: a misconfigured metric-views.json would + // otherwise silently ship an empty entry that the runtime fail-closed gate + // 503s in production. syncMetrics is log-free; this caller is the single + // owner of failure logging. + if (failures.length > 0) { + for (const f of failures) { logger.warn( - "no schema resolved for metric key %s — emitting degraded types (should not happen)", - entry.key, + "metric sync failed for %s (%s): %s", + f.key, + f.source, + f.reason, ); - return emptyMetricSchema(entry); - }); - - const mvFile = - mvOutFile ?? path.join(path.dirname(outFile), METRIC_TYPES_FILE); - const mvDeclarations = generateMetricTypeDeclarations(mvSchemas); - await fs.mkdir(path.dirname(mvFile), { recursive: true }); - await fs.writeFile(mvFile, mvDeclarations, "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 mvMetadataFile = - mvMetadataOutFile ?? - path.join(path.dirname(mvFile), METRIC_METADATA_FILE); - const metadataJson = generateMetricsMetadataJson(mvSchemas); - await fs.mkdir(path.dirname(mvMetadataFile), { recursive: true }); - await fs.writeFile(mvMetadataFile, metadataJson, "utf-8"); - - logger.debug( - "Wrote MetricRegistry augmentation + metadata bundle for %d metric(s)%s", - mvSchemas.length, - failures.length > 0 ? ` (${failures.length} failure(s))` : "", + } + } + + // 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 = described + .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 { + // Un-probed DESCRIBEs deliberately skipped, not failures: emit each + // describe-needed key as a degraded schema (permissive types) so both + // artifacts exist; cache hits keep serving last-known-good. A transient + // state refreshes on a later RUNNING pass; a DELETED/DELETING probe is + // terminal, so those keys are pinned sticky below. (Only reachable in + // `non-blocking` mode.) + 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, + describeNeeded.length, + describeNeeded.map((e) => e.key).join(", "), + ); } - // One-time migration: remove old generated file and patch project configs - await removeOldGeneratedTypes(projectRoot, "appKitTypes.d.ts"); - await migrateProjectConfig(projectRoot); + // Persist outcomes for exactly the keys this pass owned (the describe-needed + // set); hits were partitioned out above and are never rewritten, so a + // warehouse-down pass keeps last-known-good entries. A successful DESCRIBE + // caches `retry: false`; a degraded outcome caches `retry: true` only when + // re-describing could later succeed (non-terminal state or transient failure), + // else sticky `retry: false`. In `describe-now` there is no preflight/gate, so + // `terminalSkip` is always false and this reduces to "retry a degraded outcome + // unless it was a deterministic DESCRIBE failure" — a deterministic failure + // won't loop forever, and the stricter hit rule re-describes it next run + // anyway. One save per pass; with `noCache` the section started empty, so it's + // overwritten. + 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]. + const entry = describeNeeded[i]; + const failure = failureByKey.get(entry.key); + mvCacheSection[entry.key] = { + hash: metricCacheHash(entry.source, entry.lane), + schema: described[i], + retry: + described[i].degraded === true && + !terminalSkip && + (failure === undefined || failure.transient === true), + }; + } - // Types are always written above — including `result: unknown` for any query - // that could not be described. Connectivity failures pass silently so a - // transient warehouse outage never blocks a build; genuine SQL errors and - // non-connectivity fatal request failures surface after the file write. - if (syntaxErrors.length > 0) { - throw new TypegenSyntaxError(syntaxErrors, warehouseId, fatalErrors); + // 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(mvCacheSection)) { + if (!configuredKeys.has(key)) { + delete mvCacheSection[key]; + prunedCount++; + } } - if (fatalErrors.length > 0) { - throw new TypegenFatalError(fatalErrors, warehouseId); + + // 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. With `noCache` the section started empty, so it's overwritten. + if (describeNeeded.length > 0 || noCache || prunedCount > 0) { + cache.metrics = mvCacheSection; + await saveCache(cache); } - logger.debug("Type generation complete!"); + // Merge cached hits with fresh results back into config order (renderers sort + // internally where determinism matters). Every describe-needed entry yields + // exactly one schema above, so the final fallback is defensive only. + const describedByKey = new Map(); + for (const schema of described) { + describedByKey.set(schema.key, schema); + } + const schemas = resolution.entries.map((entry) => { + const schema = hitSchemas.get(entry.key) ?? describedByKey.get(entry.key); + if (schema !== undefined) return schema; + // Defensive: every entry is either a cache hit or describe-needed (and every + // describe-needed entry yields exactly one schema above), so this should be + // unreachable. If the invariant ever breaks, warn loudly but still emit a + // permissive degraded schema — the metric path never crashes a build over a + // single entry. + logger.warn( + "no schema resolved for metric key %s — emitting degraded types (should not happen)", + entry.key, + ); + return emptyMetricSchema(entry); + }); + + await fs.mkdir(path.dirname(metricOutFile), { recursive: true }); + await fs.writeFile( + metricOutFile, + generateMetricTypeDeclarations(schemas), + "utf-8", + ); + + await fs.mkdir(path.dirname(metricMetadataOutFile), { recursive: true }); + await fs.writeFile( + metricMetadataOutFile, + generateMetricsMetadataJson(schemas), + "utf-8", + ); + + logger.debug( + "Wrote MetricRegistry augmentation + metadata bundle for %d metric(s)%s", + schemas.length, + failures.length > 0 ? ` (${failures.length} failure(s))` : "", + ); + + return { + metricOutFile, + metricMetadataOutFile, + schemas, + failures, + fatalErrors, + noConfig: false, + }; } // Rolldown tree-shaking only preserves "own exports" (locally defined) — not re-exports. diff --git a/packages/appkit/src/type-generator/tests/sync-metric-views-types.test.ts b/packages/appkit/src/type-generator/tests/sync-metric-views-types.test.ts new file mode 100644 index 000000000..dae4ec3af --- /dev/null +++ b/packages/appkit/src/type-generator/tests/sync-metric-views-types.test.ts @@ -0,0 +1,350 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import type { DescribeFetcher } from "../mv-registry/types"; +import type { DatabricksStatementExecutionResponse } from "../types"; + +/** + * Unit tests for the metric-only `syncMetricViewsTypes` export that backs the + * `appkit mv sync` CLI. A mock {@link DescribeFetcher} is injected so the + * pipeline (read config → resolve → [cache partition] → syncMetrics → write + * artifacts) runs without a warehouse, asserting BOTH artifacts land for a + * mixed fixture (a service-principal metric + an OBO metric; measures + a + * time-typed dimension + a format spec) and that the shared typegen cache is + * honored (default) / bypassed (`cache: false`). + */ + +// In-memory stand-in for the on-disk typegen cache file so the focused metric +// sync's loadCache/saveCache never touch node_modules/.databricks and each test +// controls cache state. hashSQL / metricCacheHash / isRevivableMetricCacheEntry +// / CACHE_VERSION pass through unmocked (mirrors index.test.ts). +const mocks = vi.hoisted(() => ({ + cacheFile: { contents: undefined as string | undefined }, +})); + +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. + } + } + return { version: actual.CACHE_VERSION, queries: {} }; + }), + saveCache: vi.fn(async (cache: unknown) => { + mocks.cacheFile.contents = JSON.stringify(cache, null, 2); + }), + }; +}); + +const { syncMetricViewsTypes } = await import("../index"); + +/** + * Build a representative DESCRIBE TABLE EXTENDED ... AS JSON response: one row, + * one cell, a JSON-string payload (the Statement Execution API shape). + */ +function mockDescribeResponse( + payload: unknown, +): DatabricksStatementExecutionResponse { + return { + statement_id: "stmt-mock", + status: { state: "SUCCEEDED" }, + result: { data_array: [[JSON.stringify(payload)]] }, + }; +} + +// Per-FQN DESCRIBE payloads for the mixed fixture. `revenue` (SP lane) exercises +// a currency `format` spec on its measure; `churn` (OBO lane) exercises a +// time-typed dimension (TIMESTAMP → time grains inferred from the SQL type). +const DESCRIBE_BY_FQN: Record = { + "demo.sales.revenue": { + columns: [ + { + name: "total_revenue", + type: "DECIMAL(38,2)", + is_measure: true, + format: "$#,##0.00", + }, + { name: "region", type: "STRING", is_measure: false }, + ], + }, + "demo.sales.churn": { + columns: [ + { name: "churn_rate", type: "DOUBLE", is_measure: true }, + { name: "event_time", type: "TIMESTAMP", is_measure: false }, + ], + }, +}; + +describe("syncMetricViewsTypes", () => { + let tmpRoot: string; + let queryFolder: string; + let metricOutFile: string; + let metricMetadataOutFile: string; + + // A spy fetcher so cache tests can assert which FQNs were (re)described. + const fetcher = vi.fn(async (fqn) => { + const payload = DESCRIBE_BY_FQN[fqn]; + if (payload === undefined) { + throw new Error(`unexpected FQN in test fetcher: ${fqn}`); + } + return mockDescribeResponse(payload); + }); + + const writeMixedConfig = () => { + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + JSON.stringify({ + metricViews: { + // SP lane (default executor). + revenue: { source: "demo.sales.revenue" }, + // OBO lane (executor: "user"). + churn: { source: "demo.sales.churn", executor: "user" }, + }, + }), + ); + }; + + beforeEach(() => { + fetcher.mockClear(); + mocks.cacheFile.contents = undefined; + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), "sync-metric-types-")); + queryFolder = path.join(tmpRoot, "config", "queries"); + fs.mkdirSync(queryFolder, { recursive: true }); + metricOutFile = path.join(tmpRoot, "shared", "appkit-types", "metric.d.ts"); + metricMetadataOutFile = path.join( + tmpRoot, + "shared", + "appkit-types", + "metrics.metadata.json", + ); + }); + + afterEach(() => { + fs.rmSync(tmpRoot, { recursive: true, force: true }); + }); + + test("writes BOTH artifacts for a mixed SP + OBO fixture", async () => { + writeMixedConfig(); + + const result = await syncMetricViewsTypes({ + queryFolder, + warehouseId: "wh-1", + metricOutFile, + metricMetadataOutFile, + metricFetcher: fetcher, + }); + + // Both artifacts exist on disk. + expect(fs.existsSync(metricOutFile)).toBe(true); + expect(fs.existsSync(metricMetadataOutFile)).toBe(true); + + // Result reports both keys, no failures, config present. + expect(result.noConfig).toBe(false); + expect(result.failures).toEqual([]); + expect(result.schemas.map((s) => s.key).sort()).toEqual([ + "churn", + "revenue", + ]); + expect(result.metricOutFile).toBe(metricOutFile); + expect(result.metricMetadataOutFile).toBe(metricMetadataOutFile); + + // --- metric.d.ts: MetricRegistry augmentation for both metrics --- + const declarations = fs.readFileSync(metricOutFile, "utf-8"); + expect(declarations).toContain("interface MetricRegistry"); + expect(declarations).toContain('"revenue"'); + expect(declarations).toContain('"churn"'); + // Measure + dimension column types render as TS primitives. + expect(declarations).toContain('"total_revenue": number'); + expect(declarations).toContain('"region": string'); + expect(declarations).toContain('"churn_rate": number'); + // The OBO metric's lane is captured in its entry. + expect(declarations).toContain('lane: "obo"'); + expect(declarations).toContain('lane: "sp"'); + // The TIMESTAMP dimension carries inferred time grains in its @timeGrain tag. + expect(declarations).toContain("@timeGrain"); + + // --- metrics.metadata.json: per-metric semantic bundle --- + const bundle = JSON.parse(fs.readFileSync(metricMetadataOutFile, "utf-8")); + // SP metric: currency format spec is preserved on the measure. + expect(bundle.revenue.measures.total_revenue.type).toBe("DECIMAL(38,2)"); + expect(bundle.revenue.measures.total_revenue.format).toBe("$#,##0.00"); + expect(bundle.revenue.dimensions.region.type).toBe("STRING"); + // OBO metric: time-typed dimension carries its inferred time_grain set. + expect(bundle.churn.measures.churn_rate.type).toBe("DOUBLE"); + expect(bundle.churn.dimensions.event_time.type).toBe("TIMESTAMP"); + expect(bundle.churn.dimensions.event_time.time_grain).toEqual( + expect.arrayContaining(["day", "hour", "minute", "month", "year"]), + ); + }); + + test("returns noConfig and writes nothing when metric-views.json is absent", async () => { + const result = await syncMetricViewsTypes({ + queryFolder, + warehouseId: "wh-1", + metricOutFile, + metricMetadataOutFile, + metricFetcher: fetcher, + }); + + expect(result.noConfig).toBe(true); + expect(result.schemas).toEqual([]); + expect(result.failures).toEqual([]); + expect(fs.existsSync(metricOutFile)).toBe(false); + expect(fs.existsSync(metricMetadataOutFile)).toBe(false); + }); + + // --- cache behavior (default ON) ------------------------------------------- + + test("default (cache on): a warm second run over an unchanged config serves cache hits and describes nothing", async () => { + writeMixedConfig(); + + // First run: both keys are cache misses → both described, results persisted. + await syncMetricViewsTypes({ + queryFolder, + warehouseId: "wh-1", + metricOutFile, + metricMetadataOutFile, + metricFetcher: fetcher, + }); + expect(fetcher).toHaveBeenCalledTimes(2); + + fetcher.mockClear(); + + // Second run, same config: both keys hit the cache → zero DESCRIBE calls, + // and the artifacts are still regenerated from the cached schemas. + const result = await syncMetricViewsTypes({ + queryFolder, + warehouseId: "wh-1", + metricOutFile, + metricMetadataOutFile, + metricFetcher: fetcher, + }); + expect(fetcher).not.toHaveBeenCalled(); + expect(result.failures).toEqual([]); + expect(result.schemas.map((s) => s.key).sort()).toEqual([ + "churn", + "revenue", + ]); + // Cached schemas still render the real (non-degraded) types. + const declarations = fs.readFileSync(metricOutFile, "utf-8"); + expect(declarations).toContain('"total_revenue": number'); + expect(declarations).toContain('"churn_rate": number'); + }); + + test("cache: false (--no-cache) re-describes every key even when a warm cache exists", async () => { + writeMixedConfig(); + + // Warm the cache. + await syncMetricViewsTypes({ + queryFolder, + warehouseId: "wh-1", + metricOutFile, + metricMetadataOutFile, + metricFetcher: fetcher, + }); + expect(fetcher).toHaveBeenCalledTimes(2); + + fetcher.mockClear(); + + // cache: false ignores the warm section → both keys re-described. + await syncMetricViewsTypes({ + queryFolder, + warehouseId: "wh-1", + metricOutFile, + metricMetadataOutFile, + cache: false, + metricFetcher: fetcher, + }); + expect(fetcher).toHaveBeenCalledTimes(2); + }); + + test("a degraded/failed cached entry is re-described, not served (stricter hit rule)", async () => { + // Config with one entry whose first DESCRIBE fails (degraded), warming a + // sticky cache entry; the second run must re-describe it rather than ship + // the degraded schema. + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + JSON.stringify({ + metricViews: { revenue: { source: "demo.sales.revenue" } }, + }), + ); + + // First run: fetcher throws → degraded schema + a failure, cached retry:true. + fetcher.mockRejectedValueOnce(new Error("TABLE_OR_VIEW_NOT_FOUND")); + const first = await syncMetricViewsTypes({ + queryFolder, + warehouseId: "wh-1", + metricOutFile, + metricMetadataOutFile, + metricFetcher: fetcher, + }); + expect(first.failures).toHaveLength(1); + expect(fetcher).toHaveBeenCalledTimes(1); + + fetcher.mockClear(); + + // Second run, unchanged config, cache ON: the degraded entry is NOT a hit + // (degraded !== true clause + retry:true) → re-described, now succeeds. + const second = await syncMetricViewsTypes({ + queryFolder, + warehouseId: "wh-1", + metricOutFile, + metricMetadataOutFile, + metricFetcher: fetcher, + }); + expect(fetcher).toHaveBeenCalledTimes(1); + expect(second.failures).toEqual([]); + const declarations = fs.readFileSync(metricOutFile, "utf-8"); + expect(declarations).toContain('"total_revenue": number'); + }); + + test("a removed metric key is pruned from the cache section", async () => { + writeMixedConfig(); + + // Warm both keys. + await syncMetricViewsTypes({ + queryFolder, + warehouseId: "wh-1", + metricOutFile, + metricMetadataOutFile, + metricFetcher: fetcher, + }); + const afterFirst = JSON.parse(mocks.cacheFile.contents ?? "{}"); + expect(Object.keys(afterFirst.metrics).sort()).toEqual([ + "churn", + "revenue", + ]); + + // Shrink the config to a single key. + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + JSON.stringify({ + metricViews: { revenue: { source: "demo.sales.revenue" } }, + }), + ); + + await syncMetricViewsTypes({ + queryFolder, + warehouseId: "wh-1", + metricOutFile, + metricMetadataOutFile, + metricFetcher: fetcher, + }); + + const afterSecond = JSON.parse(mocks.cacheFile.contents ?? "{}"); + expect(Object.keys(afterSecond.metrics)).toEqual(["revenue"]); + }); +}); diff --git a/packages/appkit/tsdown.config.ts b/packages/appkit/tsdown.config.ts index d61e8c534..730757e71 100644 --- a/packages/appkit/tsdown.config.ts +++ b/packages/appkit/tsdown.config.ts @@ -4,7 +4,14 @@ export default defineConfig([ { publint: true, name: "@databricks/appkit", - entry: ["src/index.ts", "src/beta.ts"], + // `./type-generator` is a public subpath export consumed cross-package by the + // `appkit` CLI (`appkit mv sync` / `generate-types`) via a dynamic import + // Rolldown can't see. It must be its own entry so its declared public API + // (syncMetricViewsTypes + METRIC_TYPES_FILE / METRIC_METADATA_FILE, alongside + // generateFromEntryPoint / generateServingTypes) is preserved under unbundle + // tree-shaking. Without it, the subpath's runtime exports collapse to only the + // names appkit's own Vite plugins import — silently dropping the CLI's. + entry: ["src/index.ts", "src/beta.ts", "src/type-generator/index.ts"], outDir: "dist", hash: false, format: "esm", diff --git a/packages/shared/src/cli/commands/metric-views/index.ts b/packages/shared/src/cli/commands/metric-views/index.ts new file mode 100644 index 000000000..7e03dd2fc --- /dev/null +++ b/packages/shared/src/cli/commands/metric-views/index.ts @@ -0,0 +1,20 @@ +import { Command } from "commander"; +import { metricViewsSyncCommand } from "./sync/sync"; + +/** + * Parent command for UC Metric View operations. + * + * Phase 1 exposes a single subcommand (`sync`). Future subcommands + * (`list` / `validate` / `describe`) plug in here so users have one top-level + * surface for everything related to Metric Views. Sibling of `plugin`, + * `setup`, `generate-types`, `lint`, `docs`, `codemod`. + */ +export const metricViewsCommand = new Command("mv") + .description("Metric-view management commands (UC Metric Views)") + .addCommand(metricViewsSyncCommand) + .addHelpText( + "after", + ` +Examples: + $ appkit mv sync --warehouse-id 1234abcd5678efgh`, + ); diff --git a/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts b/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts new file mode 100644 index 000000000..bbd8b183b --- /dev/null +++ b/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts @@ -0,0 +1,646 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + afterEach, + beforeEach, + describe, + expect, + type Mock, + test, + vi, +} from "vitest"; + +// --- Module mocks ----------------------------------------------------------- +// vi.mock factories are hoisted above the file, so the spies they return must be +// created in a hoisted block too. Mirrors generate-types.test.ts. +const { syncMetricViewsTypes, METRIC_TYPES_FILE, METRIC_METADATA_FILE } = + vi.hoisted(() => ({ + // The mock stands in for the appkit export: it WRITES both artifacts (so + // the test can assert they land in the temp dir) and reports its inputs. + syncMetricViewsTypes: vi.fn( + async (opts: { + queryFolder: string; + warehouseId: string; + metricOutFile: string; + metricMetadataOutFile: string; + cache?: boolean; + }) => { + const nodeFs = require("node:fs") as typeof import("node:fs"); + const nodePath = require("node:path") as typeof import("node:path"); + nodeFs.mkdirSync(nodePath.dirname(opts.metricOutFile), { + recursive: true, + }); + nodeFs.writeFileSync(opts.metricOutFile, "// metric.d.ts\n"); + nodeFs.writeFileSync(opts.metricMetadataOutFile, "{}\n"); + // Annotate the array element types so the inferred return type is wide + // enough for `mockResolvedValueOnce` overrides that populate `failures` + // (an empty literal would otherwise infer `never[]`). + const schemas: Array<{ key: string; source: string; lane: string }> = [ + { key: "revenue", source: "demo.sales.revenue", lane: "sp" }, + ]; + const failures: Array<{ + key: string; + source: string; + reason: string; + transient: boolean; + }> = []; + return { + metricOutFile: opts.metricOutFile, + metricMetadataOutFile: opts.metricMetadataOutFile, + schemas, + failures, + noConfig: false, + }; + }, + ), + METRIC_TYPES_FILE: "metric.d.ts", + METRIC_METADATA_FILE: "metrics.metadata.json", + })); + +// The library type-generator is an optional/ambient module; mock it so the +// command's `await import("@databricks/appkit/type-generator")` resolves to +// spies and never touches a warehouse. +vi.mock("@databricks/appkit/type-generator", () => ({ + syncMetricViewsTypes, + METRIC_TYPES_FILE, + METRIC_METADATA_FILE, +})); + +// --- @clack/prompts mock ---------------------------------------------------- +// Drive the interactive path deterministically: each `text` prompt returns the +// next queued answer; `isCancel` recognizes the shared CANCEL symbol so a queued +// cancel triggers the graceful-exit branch. intro/outro/cancel/spinner are +// no-op spies (the spinner object exposes start/stop). +const clackMocks = vi.hoisted(() => { + const CANCEL = Symbol("clack:cancel"); + return { + CANCEL, + // Answers consumed in prompt order (warehouse id, config path, output dir). + textAnswers: [] as Array, + text: vi.fn(), + intro: vi.fn(), + outro: vi.fn(), + cancel: vi.fn(), + spinnerStart: vi.fn(), + spinnerStop: vi.fn(), + }; +}); + +vi.mock("@clack/prompts", () => ({ + intro: clackMocks.intro, + outro: clackMocks.outro, + cancel: clackMocks.cancel, + isCancel: (value: unknown) => value === clackMocks.CANCEL, + text: (...args: unknown[]) => { + clackMocks.text(...args); + return Promise.resolve( + clackMocks.textAnswers.length > 0 + ? clackMocks.textAnswers.shift() + : undefined, + ); + }, + spinner: () => ({ + start: clackMocks.spinnerStart, + stop: clackMocks.spinnerStop, + }), +})); + +import { metricViewsSyncCommand } from "./sync"; + +/** + * Drive the real commander command the way the bin does. `metricViewsSyncCommand` + * is a module-level singleton, so commander retains option values parsed by a + * previous call (absent options are NOT reset between `parseAsync` calls); + * clear that stored state first so each invocation parses from a clean slate. + * Resetting `_optionValueSources` to `{}` also clears the per-option `default` + * source bookkeeping, so a no-flag parse leaves every source `undefined` — the + * interactive-detection check keys on the `cli` source, so that reads correctly + * as "no user flag". + */ +async function runCli(args: string[]): Promise { + const cmd = metricViewsSyncCommand as unknown as { + _optionValues: Record; + _optionValueSources: Record; + }; + cmd._optionValues = {}; + cmd._optionValueSources = {}; + await metricViewsSyncCommand.parseAsync(args, { from: "user" }); +} + +describe("appkit mv sync", () => { + let tmpRoot: string; + let queryFolder: string; + let consoleLog: Mock; + let consoleError: Mock; + let originalCwd: string; + const prevWarehouse = process.env.DATABRICKS_WAREHOUSE_ID; + + beforeEach(() => { + vi.clearAllMocks(); + clackMocks.textAnswers = []; + originalCwd = process.cwd(); + tmpRoot = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), "metric-sync-cli-")), + ); + queryFolder = path.join(tmpRoot, "config", "queries"); + fs.mkdirSync(queryFolder, { recursive: true }); + delete process.env.DATABRICKS_WAREHOUSE_ID; + // `--root-dir` was dropped in Phase 3; the command resolves cwd-relative + // paths against process.cwd(), so anchor cwd at the temp root (mirrors + // promote.test.ts). + process.chdir(tmpRoot); + + consoleLog = vi.spyOn(console, "log").mockImplementation(() => {}) as Mock; + consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => {}) as Mock; + }); + + afterEach(() => { + process.chdir(originalCwd); + vi.restoreAllMocks(); + fs.rmSync(tmpRoot, { recursive: true, force: true }); + if (prevWarehouse === undefined) { + delete process.env.DATABRICKS_WAREHOUSE_ID; + } else { + process.env.DATABRICKS_WAREHOUSE_ID = prevWarehouse; + } + }); + + const writeConfig = () => { + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + JSON.stringify({ + metricViews: { revenue: { source: "demo.sales.revenue" } }, + }), + ); + }; + + // --- Non-interactive: flag parsing + option mapping ------------------------ + + test("calls the appkit entry with resolved paths and writes both artifacts", async () => { + writeConfig(); + + await runCli(["--warehouse-id", "wh-123"]); + + const expectedMetricOut = path.join( + tmpRoot, + "shared", + "appkit-types", + "metric.d.ts", + ); + const expectedMetadataOut = path.join( + tmpRoot, + "shared", + "appkit-types", + "metrics.metadata.json", + ); + + // The appkit entry was called once with the resolved options. Cache is the + // commander default; after the test harness reset it is undefined (no flag, + // source cleared) — i.e. caching stays ON downstream. + expect(syncMetricViewsTypes).toHaveBeenCalledTimes(1); + expect(syncMetricViewsTypes).toHaveBeenCalledWith({ + queryFolder, + warehouseId: "wh-123", + metricOutFile: expectedMetricOut, + metricMetadataOutFile: expectedMetadataOut, + cache: undefined, + }); + + // Both artifacts landed in the temp dir. + expect(fs.existsSync(expectedMetricOut)).toBe(true); + expect(fs.existsSync(expectedMetadataOut)).toBe(true); + }); + + test("falls back to DATABRICKS_WAREHOUSE_ID when --warehouse-id is omitted (and stays non-interactive via another flag)", async () => { + writeConfig(); + process.env.DATABRICKS_WAREHOUSE_ID = "wh-env"; + + // Pass --output-dir so the env var alone doesn't have to force + // non-interactive (it must not — see the dedicated interactive test). + await runCli(["--output-dir", "shared/appkit-types"]); + + expect(syncMetricViewsTypes).toHaveBeenCalledWith( + expect.objectContaining({ warehouseId: "wh-env" }), + ); + }); + + test("honors --metric-views-json-path / --output-dir overrides for path resolution", async () => { + const customConfigDir = path.join(tmpRoot, "custom", "cfg"); + fs.mkdirSync(customConfigDir, { recursive: true }); + fs.writeFileSync( + path.join(customConfigDir, "metric-views.json"), + JSON.stringify({ metricViews: {} }), + ); + + await runCli([ + "--warehouse-id", + "wh-123", + "--metric-views-json-path", + "custom/cfg/metric-views.json", + "--output-dir", + "build/types", + ]); + + expect(syncMetricViewsTypes).toHaveBeenCalledWith({ + queryFolder: customConfigDir, + warehouseId: "wh-123", + metricOutFile: path.join(tmpRoot, "build", "types", "metric.d.ts"), + metricMetadataOutFile: path.join( + tmpRoot, + "build", + "types", + "metrics.metadata.json", + ), + cache: undefined, + }); + }); + + test("absolute --metric-views-json-path / --output-dir are used as-is", async () => { + const absConfig = path.join( + tmpRoot, + "config", + "queries", + "metric-views.json", + ); + writeConfig(); + const absOut = path.join(tmpRoot, "abs-out"); + + await runCli([ + "--warehouse-id", + "wh-123", + "--metric-views-json-path", + absConfig, + "--output-dir", + absOut, + ]); + + expect(syncMetricViewsTypes).toHaveBeenCalledWith( + expect.objectContaining({ + queryFolder: path.dirname(absConfig), + metricOutFile: path.join(absOut, "metric.d.ts"), + metricMetadataOutFile: path.join(absOut, "metrics.metadata.json"), + }), + ); + }); + + test("friendly message + no appkit call when metric-views.json is absent", async () => { + await runCli(["--warehouse-id", "wh-123"]); + + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + const logged = consoleLog.mock.calls.flat().map(String).join("\n"); + expect(logged).toContain("Nothing to sync"); + }); + + test("appkit absent: recognizable error message + non-zero exit", async () => { + writeConfig(); + // Model the dynamic import failing as it does when @databricks/appkit + // isn't installed. + syncMetricViewsTypes.mockRejectedValueOnce( + new Error("Cannot find module '@databricks/appkit/type-generator'"), + ); + + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never); + + await runCli(["--warehouse-id", "wh-123"]); + + const errored = consoleError.mock.calls.flat().map(String).join("\n"); + expect(errored).toContain( + "appkit mv sync is only available with @databricks/appkit installed", + ); + expect(exitSpy).toHaveBeenCalledWith(1); + + exitSpy.mockRestore(); + }); + + // --- --no-cache propagation ------------------------------------------------ + + test("--no-cache forwards cache: false to syncMetricViewsTypes", async () => { + writeConfig(); + + await runCli(["--warehouse-id", "wh-123", "--no-cache"]); + + expect(syncMetricViewsTypes).toHaveBeenCalledWith( + expect.objectContaining({ cache: false }), + ); + }); + + test("without --no-cache, cache is not disabled (default ON downstream)", async () => { + writeConfig(); + + await runCli(["--warehouse-id", "wh-123"]); + + const call = syncMetricViewsTypes.mock.calls[0][0] as { cache?: boolean }; + // Either undefined (harness reset clears the default source) or true (live + // commander default) — the load-bearing invariant is that it is NOT false. + expect(call.cache).not.toBe(false); + }); + + // --- Phase 2: error taxonomy ------------------------------------------------ + // Every error mode exits non-zero (1) with a distinct, recognizable message. + // The command always `return`s right after `process.exit`, so a no-op exit + // spy lets execution stop cleanly and we assert the captured code + message. + + /** + * Drive the CLI with `process.exit` spied to a no-op (the command returns + * immediately after calling it), returning the spy so the test can assert the + * exit code and the captured stderr. + */ + async function runCliCapturingExit(args: string[]): Promise { + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never) as unknown as Mock; + await runCli(args); + return exitSpy; + } + + const erroredText = () => + consoleError.mock.calls.flat().map(String).join("\n"); + + test("explicit --metric-views-json-path to a missing file: non-zero + recognizable message", async () => { + const missing = path.join(tmpRoot, "nowhere", "metric-views.json"); + + const exitSpy = await runCliCapturingExit([ + "--warehouse-id", + "wh-123", + "--metric-views-json-path", + missing, + ]); + + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(erroredText()).toContain("metric-views.json not found"); + }); + + test("explicit --metric-views-json-path with a non-metric-views.json basename: rejected before syncing", async () => { + // A valid config that EXISTS but is not named metric-views.json. The appkit + // reader resolves `/metric-views.json`, so without the basename guard + // the CLI would validate this file but sync a different (sibling/absent) one. + const customDir = path.join(tmpRoot, "custom"); + fs.mkdirSync(customDir, { recursive: true }); + fs.writeFileSync( + path.join(customDir, "my-config.json"), + JSON.stringify({ + metricViews: { revenue: { source: "demo.sales.revenue" } }, + }), + ); + + const exitSpy = await runCliCapturingExit([ + "--warehouse-id", + "wh-123", + "--metric-views-json-path", + "custom/my-config.json", + ]); + + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(erroredText()).toContain( + "must point to a file named metric-views.json", + ); + }); + + test("malformed JSON: non-zero + 'not valid JSON' message, never imports appkit", async () => { + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + "{ this is not json", + ); + + const exitSpy = await runCliCapturingExit(["--warehouse-id", "wh-123"]); + + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(erroredText()).toContain("is not valid JSON"); + }); + + test("schema-invalid config (bad FQN): non-zero + path:message list, never imports appkit", async () => { + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + // Two-part FQN — fails the three-part UC FQN grammar. + JSON.stringify({ metricViews: { revenue: { source: "main.cm" } } }), + ); + + const exitSpy = await runCliCapturingExit(["--warehouse-id", "wh-123"]); + + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + const errored = erroredText(); + expect(errored).toContain("invalid"); + // Humanized path of the failing field. + expect(errored).toContain("metricViews.revenue.source"); + }); + + test("schema-invalid config (unknown executor): non-zero + path:message list", async () => { + fs.writeFileSync( + path.join(queryFolder, "metric-views.json"), + JSON.stringify({ + metricViews: { revenue: { source: "main.a.cm", executor: "robot" } }, + }), + ); + + const exitSpy = await runCliCapturingExit(["--warehouse-id", "wh-123"]); + + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(erroredText()).toContain("metricViews.revenue.executor"); + }); + + test("missing warehouse id (after a valid config): non-zero + recognizable message", async () => { + writeConfig(); + // No --warehouse-id and no DATABRICKS_WAREHOUSE_ID (cleared in beforeEach). + // Pass --output-dir so the run is non-interactive (no flag → interactive). + + const exitSpy = await runCliCapturingExit([ + "--output-dir", + "shared/appkit-types", + ]); + + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(erroredText()).toContain("no warehouse ID"); + }); + + test("absent DEFAULT config with no warehouse id: still exits 0 (dormancy invariant)", async () => { + // No config file, no warehouse id — the additive path must stay dormant and + // NOT error on the missing warehouse. Pass a flag so this stays + // non-interactive (the dormancy decision is path-independent). + const exitSpy = await runCliCapturingExit([ + "--output-dir", + "shared/appkit-types", + ]); + + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + // Dormancy takes the early-return path, so exit() is never called at all. + expect(exitSpy).not.toHaveBeenCalled(); + const logged = consoleLog.mock.calls.flat().map(String).join("\n"); + expect(logged).toContain("Nothing to sync"); + }); + + test("unreachable warehouse / auth failure (syncMetricViewsTypes throws): non-zero + message surfaced", async () => { + writeConfig(); + syncMetricViewsTypes.mockRejectedValueOnce( + new Error("warehouse wh-123 is unreachable: connection refused"), + ); + + const exitSpy = await runCliCapturingExit(["--warehouse-id", "wh-123"]); + + expect(syncMetricViewsTypes).toHaveBeenCalledTimes(1); + expect(exitSpy).toHaveBeenCalledWith(1); + // The action wrapper prints the thrown error's message verbatim. + expect(erroredText()).toContain( + "warehouse wh-123 is unreachable: connection refused", + ); + }); + + test("per-entry DESCRIBE failure (unreachable FQN): non-zero + lists the failed metric", async () => { + writeConfig(); + // The appkit export writes degraded types and returns `failures` rather + // than throwing for a missing/unreachable metric view FQN. + syncMetricViewsTypes.mockResolvedValueOnce({ + metricOutFile: path.join( + tmpRoot, + "shared", + "appkit-types", + "metric.d.ts", + ), + metricMetadataOutFile: path.join( + tmpRoot, + "shared", + "appkit-types", + "metrics.metadata.json", + ), + schemas: [], + failures: [ + { + key: "revenue", + source: "demo.sales.revenue", + reason: "TABLE_OR_VIEW_NOT_FOUND", + transient: false, + }, + ], + noConfig: false, + }); + + const exitSpy = await runCliCapturingExit(["--warehouse-id", "wh-123"]); + + expect(syncMetricViewsTypes).toHaveBeenCalledTimes(1); + expect(exitSpy).toHaveBeenCalledWith(1); + const errored = erroredText(); + expect(errored).toContain("could not be described"); + expect(errored).toContain("revenue"); + expect(errored).toContain("TABLE_OR_VIEW_NOT_FOUND"); + }); + + // --- Phase 3: interactive flow ---------------------------------------------- + + test("no flags → interactive: prompts fire and resolved values reach syncMetricViewsTypes", async () => { + writeConfig(); + // Answers in prompt order: warehouse id, config path (blank → default), + // output dir (blank → default). + clackMocks.textAnswers = ["wh-interactive", "", ""]; + + await runCli([]); + + // intro/outro + three text prompts fired. + expect(clackMocks.intro).toHaveBeenCalledTimes(1); + expect(clackMocks.text).toHaveBeenCalledTimes(3); + expect(clackMocks.outro).toHaveBeenCalledTimes(1); + // Spinner wrapped the sync. + expect(clackMocks.spinnerStart).toHaveBeenCalledTimes(1); + expect(clackMocks.spinnerStop).toHaveBeenCalledTimes(1); + + // The interactive answer reached the appkit entry; blank path answers fell + // back to the canonical defaults (cwd-anchored). + expect(syncMetricViewsTypes).toHaveBeenCalledWith({ + queryFolder, + warehouseId: "wh-interactive", + metricOutFile: path.join( + tmpRoot, + "shared", + "appkit-types", + "metric.d.ts", + ), + metricMetadataOutFile: path.join( + tmpRoot, + "shared", + "appkit-types", + "metrics.metadata.json", + ), + cache: true, + }); + }); + + test("interactive: a non-blank config path / output dir answer is honored", async () => { + const customConfigDir = path.join(tmpRoot, "alt", "cfg"); + fs.mkdirSync(customConfigDir, { recursive: true }); + fs.writeFileSync( + path.join(customConfigDir, "metric-views.json"), + JSON.stringify({ metricViews: {} }), + ); + clackMocks.textAnswers = [ + "wh-interactive", + "alt/cfg/metric-views.json", + "alt/out", + ]; + + await runCli([]); + + expect(syncMetricViewsTypes).toHaveBeenCalledWith( + expect.objectContaining({ + queryFolder: customConfigDir, + warehouseId: "wh-interactive", + metricOutFile: path.join(tmpRoot, "alt", "out", "metric.d.ts"), + }), + ); + }); + + test("interactive: env var alone does NOT force non-interactive (prompts still run)", async () => { + writeConfig(); + process.env.DATABRICKS_WAREHOUSE_ID = "wh-env"; + // Blank warehouse answer → falls back to the env var downstream. + clackMocks.textAnswers = ["", "", ""]; + + await runCli([]); + + expect(clackMocks.intro).toHaveBeenCalledTimes(1); + expect(clackMocks.text).toHaveBeenCalledTimes(3); + expect(syncMetricViewsTypes).toHaveBeenCalledWith( + expect.objectContaining({ warehouseId: "wh-env" }), + ); + }); + + test("interactive cancel (first prompt): graceful cancel + non-zero exit, no appkit call", async () => { + writeConfig(); + // First prompt returns the cancel symbol. + clackMocks.textAnswers = [clackMocks.CANCEL]; + + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never) as unknown as Mock; + + await runCli([]); + + expect(clackMocks.cancel).toHaveBeenCalledTimes(1); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + }); + + test("interactive cancel (later prompt): graceful cancel + non-zero exit", async () => { + writeConfig(); + // First answer ok, second prompt cancels. + clackMocks.textAnswers = ["wh-1", clackMocks.CANCEL]; + + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never) as unknown as Mock; + + await runCli([]); + + expect(clackMocks.cancel).toHaveBeenCalledTimes(1); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/shared/src/cli/commands/metric-views/sync/sync.ts b/packages/shared/src/cli/commands/metric-views/sync/sync.ts new file mode 100644 index 000000000..677a986ed --- /dev/null +++ b/packages/shared/src/cli/commands/metric-views/sync/sync.ts @@ -0,0 +1,422 @@ +import fs from "node:fs"; +import path from "node:path"; +import process from "node:process"; +import { cancel, intro, isCancel, outro, spinner, text } from "@clack/prompts"; +import { Command } from "commander"; +import { + formatMetricViewsSourceErrors, + validateMetricViewsSource, +} from "../validate-metric-views-source"; + +/** + * Options parsed by commander for `appkit mv sync`. + * + * Phase 3 locks the flag surface to exactly four options and adds the + * interactive clack flow: + * - `--warehouse-id` (+ `DATABRICKS_WAREHOUSE_ID` fallback) + * - `--metric-views-json-path` (canonical config path) + * - `--output-dir` (artifact output directory; replaces Phase 1's `--out-dir`) + * - `--no-cache` (commander negation → `cache === false` disables the + * metric type-generation cache) + * + * Phase 1's interim `--root-dir` is dropped: relative `--metric-views-json-path` + * / `--output-dir` resolve against `process.cwd()`, mirroring how + * `generate-types` anchors its defaults at the current directory. + */ +export interface MetricViewsSyncOptions { + warehouseId?: string; + /** + * Path to metric-views.json. Default: + * `/config/queries/metric-views.json`. Canonical flag name in the + * locked spec. + */ + metricViewsJsonPath?: string; + /** Output directory for metric.d.ts + metrics.metadata.json (default: /shared/appkit-types). */ + outputDir?: string; + /** + * Caching toggle. Commander's `--no-cache` sets this to `false` (and leaves it + * `true`/absent otherwise); `cache === false` is the single signal that + * disables the metric type-generation cache when forwarded to + * `syncMetricViewsTypes`. + */ + cache?: boolean; +} + +/** Default filename for the metric source config (post-#433 name). */ +const METRIC_VIEWS_CONFIG_FILE = "metric-views.json"; + +/** + * Non-zero exit code for `appkit mv sync` failure modes. Every error mode + * exits with this same code and a distinct, recognizable message (the failure + * mode is identified by that message, not by a bespoke per-mode code — keeping + * the single Phase-1 exit mechanism). The dormant (no-config) and success cases + * take an early `return`, exiting 0 naturally. + */ +const EXIT_FAILURE = 1; + +/** Resolved, absolute paths the sync run operates on. */ +interface ResolvedPaths { + /** Folder that holds metric-views.json (the appkit export reads from a folder). */ + queryFolder: string; + /** Absolute path to metric-views.json. */ + configPath: string; + /** Whether the config path came from an explicit `--metric-views-json-path`. */ + explicitConfigPath: boolean; + /** Output directory for the generated artifacts. */ + outDir: string; +} + +/** + * Resolve config + output paths from the (interactive- or flag-supplied) + * options. Relative paths resolve against `process.cwd()` (Phase 1's + * `--root-dir` was dropped in Phase 3). + */ +function resolvePaths(options: { + metricViewsJsonPath?: string; + outputDir?: string; +}): ResolvedPaths { + const cwd = process.cwd(); + + // metric-views.json: --metric-views-json-path > /config/queries/metric-views.json. + const explicitConfigPath = options.metricViewsJsonPath !== undefined; + const configPath = options.metricViewsJsonPath + ? path.isAbsolute(options.metricViewsJsonPath) + ? options.metricViewsJsonPath + : path.resolve(cwd, options.metricViewsJsonPath) + : path.join(cwd, "config", "queries", METRIC_VIEWS_CONFIG_FILE); + + // Output paths under shared/appkit-types — matches how generate-types + // resolves its output directory. + const outDir = options.outputDir + ? path.isAbsolute(options.outputDir) + ? options.outputDir + : path.resolve(cwd, options.outputDir) + : path.join(cwd, "shared", "appkit-types"); + + return { + queryFolder: path.dirname(configPath), + configPath, + explicitConfigPath, + outDir, + }; +} + +/** + * The shared sync core for BOTH the interactive and non-interactive paths: + * resolve paths → existence check (dormancy vs missing) → read + `JSON.parse` + * → schema-validate → require warehouse → ONLY THEN dynamic-import appkit + + * `syncMetricViewsTypes`. Reaches the appkit metric-sync core through a dynamic + * `import("@databricks/appkit/type-generator")` — the exact pattern + * `generate-types.ts` uses — so the `shared` CLI package carries NO static + * dependency on `@databricks/appkit` and compiles without it. + * + * Validation runs entirely before the dynamic import, so a misconfigured + * `metric-views.json` fails fast with a precise message and never touches a + * warehouse (or even requires appkit to be installed). + * + * `onProgress` lets the interactive path drive a clack spinner around the + * appkit call; the non-interactive path passes nothing (plain console logs). + */ +async function runMetricViewsSync( + options: MetricViewsSyncOptions, + onProgress?: { start(): void; succeed(msg: string): void; fail(): void }, +): Promise { + try { + const { queryFolder, configPath, explicitConfigPath, outDir } = + resolvePaths(options); + + // `--metric-views-json-path` selects WHICH metric-views.json to sync, but the + // appkit reader resolves `/metric-views.json` from the folder, so + // a differently-named file would be validated here yet never synced (appkit + // would read a sibling metric-views.json, or none). Reject the mismatch + // explicitly instead of silently validating one file and syncing another. + if ( + explicitConfigPath && + path.basename(configPath) !== METRIC_VIEWS_CONFIG_FILE + ) { + console.error( + `Error: --metric-views-json-path must point to a file named ${METRIC_VIEWS_CONFIG_FILE} (got "${path.basename(configPath)}").`, + ); + process.exit(EXIT_FAILURE); + return; + } + + // Existence is checked before anything else (including the warehouse-id + // requirement) so the dormancy invariant holds unconditionally: + // - DEFAULT path absent → additive path is dormant, exit 0. An opt-in + // project that never adopted metric views must NOT error, even without a + // warehouse configured. + // - EXPLICIT --metric-views-json-path absent → the user named a file that + // isn't there; that's a real error, exit non-zero. + if (!fs.existsSync(configPath)) { + if (explicitConfigPath) { + console.error(`Error: metric-views.json not found at ${configPath}.`); + process.exit(EXIT_FAILURE); + return; + } + console.log( + `No ${METRIC_VIEWS_CONFIG_FILE} found at ${configPath}. Nothing to sync.`, + ); + return; + } + + // Read + parse the config before touching appkit. A malformed file is a + // user error with a precise location, not an appkit/warehouse failure. + const rawConfig = fs.readFileSync(configPath, "utf-8"); + let parsedConfig: unknown; + try { + parsedConfig = JSON.parse(rawConfig); + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + console.error(`Error: ${configPath} is not valid JSON: ${reason}`); + process.exit(EXIT_FAILURE); + return; + } + + // Schema-validate against the canonical metricSourceSchema (single source + // of truth) BEFORE the dynamic import. Bad FQN grammar, an unknown + // executor, an unrecognized key, or a bad metric key fail here with the + // `path: message` list — never as an opaque downstream error. + const validation = validateMetricViewsSource(parsedConfig); + if (!validation.valid) { + console.error(`Error: invalid ${configPath}:`); + console.error(formatMetricViewsSourceErrors(validation.errors)); + process.exit(EXIT_FAILURE); + return; + } + + // The warehouse is only needed once we have a valid config to sync; require + // it here (after dormancy + validation) so a dormant/invalid project never + // trips on a missing warehouse. + const warehouseId = + options.warehouseId || process.env.DATABRICKS_WAREHOUSE_ID; + if (!warehouseId) { + console.error( + "Error: no warehouse ID. Pass --warehouse-id or set DATABRICKS_WAREHOUSE_ID.", + ); + process.exit(EXIT_FAILURE); + return; + } + + const typeGen = await import("@databricks/appkit/type-generator"); + + const metricOutFile = path.join(outDir, typeGen.METRIC_TYPES_FILE); + const metricMetadataOutFile = path.join( + outDir, + typeGen.METRIC_METADATA_FILE, + ); + + onProgress?.start(); + let result: Awaited>; + try { + result = await typeGen.syncMetricViewsTypes({ + queryFolder, + metricOutFile, + metricMetadataOutFile, + warehouseId, + // `--no-cache` → cache === false disables the metric typegen cache; + // absent/true keeps the default (cache on). Forwarded verbatim. + cache: options.cache, + }); + } catch (err) { + onProgress?.fail(); + throw err; + } + + if (result.noConfig) { + // Defensive: the existence check above already handled the dormant case, + // but syncMetricViewsTypes re-checks the folder, so honor its verdict too. + onProgress?.fail(); + console.log( + `No ${METRIC_VIEWS_CONFIG_FILE} found at ${configPath}. Nothing to sync.`, + ); + return; + } + + // Per-entry DESCRIBE failures (missing/unreachable FQN, type errors against + // a reachable warehouse) come back in `failures` rather than thrown — the + // appkit export writes permissive/degraded types and returns. Surface them + // as a hard failure so a misconfigured FQN does not silently ship. + if (result.failures.length > 0) { + onProgress?.fail(); + console.error( + `Error: ${result.failures.length} metric view(s) could not be described:`, + ); + for (const failure of result.failures) { + console.error( + ` ${failure.key} (${failure.source}): ${failure.reason}`, + ); + } + process.exit(EXIT_FAILURE); + return; + } + + onProgress?.succeed(`Generated metric types: ${metricOutFile}`); + console.log(`Generated metric types: ${metricOutFile}`); + console.log(`Generated metric metadata: ${metricMetadataOutFile}`); + } catch (error) { + if ( + error instanceof Error && + error.message.includes("Cannot find module") + ) { + console.error( + "Error: appkit mv sync is only available with @databricks/appkit installed.", + ); + console.error("Please install @databricks/appkit to use this command."); + process.exit(EXIT_FAILURE); + return; + } + // Errors thrown by syncMetricViewsTypes — an unreachable warehouse, an auth + // failure, or a fatal DESCRIBE setup problem (TypegenFatalError) — carry + // their own recognizable message. Re-throw so the action wrapper prints it + // and exits non-zero, keeping the message verbatim. + throw error; + } +} + +/** + * Interactive flow (mirrors `plugin create`'s `runInteractive`): `intro` → + * `text` prompts (warehouse id, config path, output dir, each guarded by + * `isCancel`) → `spinner` around the sync → `outro`. Each prompt's value is + * folded back into {@link MetricViewsSyncOptions} and handed to {@link runMetricViewsSync} + * — the SAME validation + taxonomy + appkit call the flag path uses. + * + * Empty answers fall through as `undefined`, so the warehouse prompt's blank + * input still lets the `DATABRICKS_WAREHOUSE_ID` fallback apply, and blank + * path prompts use the canonical defaults. + */ +async function runInteractive(): Promise { + intro("Sync UC Metric View types"); + + // A cancelled prompt (Ctrl-C) is a graceful, non-zero exit. The explicit + // `return` after `process.exit` keeps control flow correct under a no-op exit + // (tests) — without it, a cancelled flow would fall through to the next + // prompt and eventually run the sync. + const cancelled = (): never => { + cancel("Cancelled."); + process.exit(1); + }; + + const warehouseId = await text({ + message: "SQL Warehouse ID", + placeholder: process.env.DATABRICKS_WAREHOUSE_ID + ? `${process.env.DATABRICKS_WAREHOUSE_ID} (from DATABRICKS_WAREHOUSE_ID)` + : "1234abcd5678efgh", + // Optional: a blank value defers to DATABRICKS_WAREHOUSE_ID (validated + // downstream by runMetricViewsSync, which errors if neither is set). + }); + if (isCancel(warehouseId)) return cancelled(); + + const metricViewsJsonPath = await text({ + message: "Path to metric-views.json", + placeholder: "config/queries/metric-views.json", + // Optional: blank uses the canonical default path. + }); + if (isCancel(metricViewsJsonPath)) return cancelled(); + + const outputDir = await text({ + message: "Output directory for generated types", + placeholder: "shared/appkit-types", + // Optional: blank uses the canonical default output dir. + }); + if (isCancel(outputDir)) return cancelled(); + + const trimmed = (value: string | symbol): string | undefined => { + if (typeof value !== "string") return undefined; + const t = value.trim(); + return t.length > 0 ? t : undefined; + }; + + const options: MetricViewsSyncOptions = { + warehouseId: trimmed(warehouseId), + metricViewsJsonPath: trimmed(metricViewsJsonPath), + outputDir: trimmed(outputDir), + // Interactive runs use the default cache behavior (cache on); --no-cache is + // a non-interactive flag. + cache: true, + }; + + const s = spinner(); + await runMetricViewsSync(options, { + start: () => s.start("Describing metric views…"), + succeed: (msg) => s.stop(msg), + fail: () => s.stop("Failed."), + }); + + outro("Metric types synced."); +} + +/** + * Entry point for the `metric sync` action. Detection mirrors `plugin create`'s + * interactive-vs-non-interactive split, but keys on commander's per-option value + * SOURCE (`getOptionValueSource(name) === "cli"`) rather than presence: + * - NO user-provided flag (every option's source is `default`/absent) → + * interactive prompts. + * - ANY user-provided flag → the flag-driven (non-interactive) path. + * + * Keying on the `cli` source (not value presence) is deliberate: a + * `DATABRICKS_WAREHOUSE_ID` env default does NOT populate any CLI option, so it + * never forces non-interactive; and `--no-cache`'s default (`cache: true`, + * source `default`) is correctly ignored, while an explicit `--no-cache` + * (source `cli`) does select non-interactive. + */ +async function runMetricViewsSyncAction( + options: MetricViewsSyncOptions, + command: Command, +): Promise { + const FLAG_OPTION_NAMES = [ + "warehouseId", + "metricViewsJsonPath", + "outputDir", + "cache", + ] as const; + const hasUserFlag = FLAG_OPTION_NAMES.some( + (name) => command.getOptionValueSource(name) === "cli", + ); + + if (hasUserFlag) { + await runMetricViewsSync(options); + } else { + await runInteractive(); + } +} + +export const metricViewsSyncCommand = new Command("sync") + .description( + "Sync UC Metric View schemas: DESCRIBE every entry in metric-views.json, then emit metric.d.ts + metrics.metadata.json.", + ) + .option( + "--warehouse-id ", + "Databricks SQL Warehouse ID (overrides DATABRICKS_WAREHOUSE_ID env var)", + ) + .option( + "--metric-views-json-path ", + "Path to metric-views.json (default: config/queries/metric-views.json)", + ) + .option( + "--output-dir ", + "Output directory for metric.d.ts and metrics.metadata.json (default: shared/appkit-types)", + ) + .option("--no-cache", "Disable the metric type-generation cache") + .addHelpText( + "after", + ` +Run with no flags for an interactive prompt; pass any flag for non-interactive mode. + +Examples: + $ appkit mv sync + $ appkit mv sync --warehouse-id 1234abcd5678efgh + $ appkit mv sync --warehouse-id 1234abcd5678efgh --metric-views-json-path config/queries/metric-views.json + $ appkit mv sync --warehouse-id 1234abcd5678efgh --output-dir shared/appkit-types + $ appkit mv sync --warehouse-id 1234abcd5678efgh --no-cache + +Environment variables: + DATABRICKS_WAREHOUSE_ID SQL warehouse ID (used when --warehouse-id is omitted) + DATABRICKS_HOST Databricks workspace URL (consumed by the SDK)`, + ) + .action((opts: MetricViewsSyncOptions, command: Command) => + runMetricViewsSyncAction(opts, command).catch((err: unknown) => { + console.error(err instanceof Error ? err.message : String(err)); + process.exit(EXIT_FAILURE); + }), + ); diff --git a/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.test.ts b/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.test.ts new file mode 100644 index 000000000..bd4d4fc27 --- /dev/null +++ b/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.test.ts @@ -0,0 +1,185 @@ +import { describe, expect, test } from "vitest"; +import { + formatMetricViewsSourceErrors, + humanizeMetricViewsPath, + validateMetricViewsSource, +} from "./validate-metric-views-source"; + +/** + * Fixtures are derived from the ACTUAL canonical schema + * (`packages/shared/src/schemas/metric-source.ts`) and the UC FQN grammar + * (`packages/shared/src/schemas/metric-fqn.ts`): + * - FQN: exactly three dot-separated segments; each segment may contain any + * character EXCEPT ASCII control chars (U+0000-U+001F), space (U+0020), + * forward slash, period, or DELETE (U+007F). Non-ASCII letters and hyphens + * are explicitly legal. + * - executor: enum "app_service_principal" (default) | "user". + * - metric key (record key): /^[a-zA-Z_][a-zA-Z0-9_]*$/. + * - root + entry objects are .strict() — unknown keys are rejected. + */ + +describe("validateMetricViewsSource", () => { + describe("accepts", () => { + test("a valid three-part FQN with the default executor", () => { + const result = validateMetricViewsSource({ + metricViews: { + revenue: { source: "main.analytics.customer_metrics" }, + }, + }); + expect(result.valid).toBe(true); + }); + + test('executor: "user"', () => { + const result = validateMetricViewsSource({ + metricViews: { + revenue: { source: "main.analytics.cm", executor: "user" }, + }, + }); + expect(result.valid).toBe(true); + }); + + test('executor: "app_service_principal"', () => { + const result = validateMetricViewsSource({ + metricViews: { + revenue: { + source: "main.analytics.cm", + executor: "app_service_principal", + }, + }, + }); + expect(result.valid).toBe(true); + }); + + test("FQN segments with non-ASCII letters and hyphens (UC delimited-identifier grammar)", () => { + const result = validateMetricViewsSource({ + metricViews: { + // café (combining acute), prod-data (hyphen), métricas (non-ASCII). + rev: { source: "café.prod-data.métricas" }, + }, + }); + expect(result.valid).toBe(true); + }); + + test("a $schema key alongside metricViews", () => { + const result = validateMetricViewsSource({ + $schema: + "https://databricks.github.io/appkit/schemas/metric-source.schema.json", + metricViews: { revenue: { source: "main.a.cm" } }, + }); + expect(result.valid).toBe(true); + }); + + test("an empty metricViews map", () => { + const result = validateMetricViewsSource({ metricViews: {} }); + expect(result.valid).toBe(true); + }); + + test("a completely empty object (metricViews is optional)", () => { + const result = validateMetricViewsSource({}); + expect(result.valid).toBe(true); + }); + }); + + describe("rejects", () => { + test("an FQN segment containing a space", () => { + const result = validateMetricViewsSource({ + metricViews: { rev: { source: "main.bad name.cm" } }, + }); + expect(result.valid).toBe(false); + if (result.valid) throw new Error("expected invalid"); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].path).toBe("metricViews.rev.source"); + }); + + test("an FQN segment containing a forward slash", () => { + const result = validateMetricViewsSource({ + metricViews: { rev: { source: "main.a/b.cm" } }, + }); + expect(result.valid).toBe(false); + if (result.valid) throw new Error("expected invalid"); + expect(result.errors[0].path).toBe("metricViews.rev.source"); + }); + + test("a two-part FQN (a literal dot inside what should be one segment)", () => { + const result = validateMetricViewsSource({ + metricViews: { rev: { source: "main.cm" } }, + }); + expect(result.valid).toBe(false); + if (result.valid) throw new Error("expected invalid"); + expect(result.errors[0].path).toBe("metricViews.rev.source"); + }); + + test("an unknown executor value", () => { + const result = validateMetricViewsSource({ + metricViews: { rev: { source: "main.a.cm", executor: "robot" } }, + }); + expect(result.valid).toBe(false); + if (result.valid) throw new Error("expected invalid"); + expect(result.errors[0].path).toBe("metricViews.rev.executor"); + }); + + test("an unknown entry key (entries are .strict())", () => { + const result = validateMetricViewsSource({ + metricViews: { rev: { source: "main.a.cm", ttl: 5 } }, + }); + expect(result.valid).toBe(false); + if (result.valid) throw new Error("expected invalid"); + // Unrecognized-keys issues attach to the containing object. + expect(result.errors[0].path).toBe("metricViews.rev"); + expect(result.errors[0].message.toLowerCase()).toContain("ttl"); + }); + + test("a metric key that is not a valid identifier", () => { + const result = validateMetricViewsSource({ + metricViews: { "1bad": { source: "main.a.cm" } }, + }); + expect(result.valid).toBe(false); + if (result.valid) throw new Error("expected invalid"); + expect(result.errors[0].path).toBe("metricViews.1bad"); + }); + + test("an unknown top-level key (root is .strict())", () => { + const result = validateMetricViewsSource({ + metricViews: {}, + unexpected: true, + }); + expect(result.valid).toBe(false); + if (result.valid) throw new Error("expected invalid"); + // Root-level issue → humanized as "(root)". + expect(result.errors[0].path).toBe("(root)"); + }); + }); +}); + +describe("humanizeMetricViewsPath", () => { + test("empty path renders as (root)", () => { + expect(humanizeMetricViewsPath([])).toBe("(root)"); + expect(humanizeMetricViewsPath(undefined)).toBe("(root)"); + }); + + test("nested object keys join with dots", () => { + expect(humanizeMetricViewsPath(["metricViews", "revenue", "source"])).toBe( + "metricViews.revenue.source", + ); + }); + + test("numeric segments render as bracket indices", () => { + expect(humanizeMetricViewsPath(["a", 0, "b"])).toBe("a[0].b"); + }); +}); + +describe("formatMetricViewsSourceErrors", () => { + test("renders each issue as an indented `path: message` line", () => { + const out = formatMetricViewsSourceErrors([ + { path: "metricViews.revenue.source", message: "Invalid string" }, + { path: "(root)", message: 'Unrecognized key: "foo"' }, + ]); + expect(out).toBe( + ' metricViews.revenue.source: Invalid string\n (root): Unrecognized key: "foo"', + ); + }); + + test("an empty issue list renders as an empty string", () => { + expect(formatMetricViewsSourceErrors([])).toBe(""); + }); +}); diff --git a/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.ts b/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.ts new file mode 100644 index 000000000..fd0aa8b0d --- /dev/null +++ b/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.ts @@ -0,0 +1,79 @@ +import { + type MetricSource, + metricSourceSchema, +} from "../../../schemas/metric-source"; + +/** + * A single metric-source validation issue. `path` is a humanized property path + * (e.g. `metricViews.revenue.source`) suitable for direct CLI output, and + * `message` is the schema's own diagnostic. Mirrors the `SemanticIssue` shape + * used by the plugin-manifest validator so CLI output stays uniform across + * commands. + */ +export interface MetricViewsSourceIssue { + path: string; + message: string; +} + +/** Result of {@link validateMetricViewsSource}. */ +export type ValidateMetricViewsSourceResult = + | { valid: true; source: MetricSource } + | { valid: false; errors: MetricViewsSourceIssue[] }; + +/** + * Humanize a Zod issue path (array of object keys / array indices) into a + * single string like `metricViews.revenue.source`. Numeric segments render as + * `[n]`; an empty path (a root-level issue, e.g. an unrecognized top-level key) + * renders as `(root)`. Mirrors `humanizePath` in the plugin-manifest validator. + */ +export function humanizeMetricViewsPath( + path: ReadonlyArray | undefined, +): string { + if (!path || path.length === 0) return "(root)"; + + let out = ""; + for (const key of path) { + if (typeof key === "number") { + out += `[${key}]`; + } else { + const str = String(key); + out += out.length === 0 ? str : `.${str}`; + } + } + return out.length === 0 ? "(root)" : out; +} + +/** + * Validate a parsed `metric-views.json` object against the canonical + * {@link metricSourceSchema}. The schema is the single source of truth (it also + * backs the generated JSON schema and the type-generator runtime); this helper + * only adapts its `safeParse` result into a CLI-friendly issue list. + * + * On success the original input is returned (typed as {@link MetricSource}), + * not a re-emitted copy. + */ +export function validateMetricViewsSource( + obj: unknown, +): ValidateMetricViewsSourceResult { + const result = metricSourceSchema.safeParse(obj); + if (result.success) { + return { valid: true, source: result.data }; + } + const errors = result.error.issues.map((issue) => ({ + path: humanizeMetricViewsPath(issue.path as ReadonlyArray), + message: issue.message, + })); + return { valid: false, errors }; +} + +/** + * Format metric-source validation issues for CLI output. Each issue renders on + * its own line indented by two spaces as ` : `. Mirrors + * `formatValidationErrors` in the plugin-manifest validator so the two commands + * present schema errors identically. + */ +export function formatMetricViewsSourceErrors( + issues: MetricViewsSourceIssue[], +): string { + return issues.map((issue) => ` ${issue.path}: ${issue.message}`).join("\n"); +} diff --git a/packages/shared/src/cli/commands/type-generator.d.ts b/packages/shared/src/cli/commands/type-generator.d.ts index d03dd547a..b9576cc3d 100644 --- a/packages/shared/src/cli/commands/type-generator.d.ts +++ b/packages/shared/src/cli/commands/type-generator.d.ts @@ -25,4 +25,55 @@ declare module "@databricks/appkit/type-generator" { outFile: string; noCache?: boolean; }): Promise; + + /** Execution lane: `sp` (service principal) or `obo` (on-behalf-of). */ + type MetricLane = "sp" | "obo"; + + /** Per-metric schema captured by {@link syncMetricViewsTypes}. */ + interface MetricSchema { + key: string; + source: string; + lane: MetricLane; + measures: unknown[]; + dimensions: unknown[]; + degraded?: boolean; + } + + /** One per-entry DESCRIBE failure surfaced by the metric sync. */ + interface MetricSyncFailure { + key: string; + source: string; + reason: string; + transient: boolean; + } + + /** Result of {@link syncMetricViewsTypes}. */ + interface SyncMetricViewsTypesResult { + metricOutFile?: string; + metricMetadataOutFile?: string; + schemas: MetricSchema[]; + failures: MetricSyncFailure[]; + /** `true` when no metric-views.json was found — nothing was synced. */ + noConfig: boolean; + } + + /** + * Metric-only sync entry: read metric-views.json from `queryFolder`, DESCRIBE + * every entry (minus clean cache hits), and write `metric.d.ts` + + * `metrics.metadata.json`. Does NOT generate analytics query types. Backs + * `appkit mv sync`. `cache` defaults to ON; only `cache === false` (the + * CLI's `--no-cache`) disables the shared metric type-generation cache. + */ + export function syncMetricViewsTypes(options: { + queryFolder: string; + warehouseId: string; + metricOutFile: string; + metricMetadataOutFile: string; + cache?: boolean; + }): Promise; + + /** Default filename for the generated MetricRegistry declarations. */ + export const METRIC_TYPES_FILE: string; + /** Default filename for the build-time semantic-metadata JSON bundle. */ + export const METRIC_METADATA_FILE: string; } diff --git a/packages/shared/src/cli/index.ts b/packages/shared/src/cli/index.ts index aa60157c8..764c90c1a 100644 --- a/packages/shared/src/cli/index.ts +++ b/packages/shared/src/cli/index.ts @@ -8,6 +8,7 @@ import { codemodCommand } from "./commands/codemod/index.js"; import { docsCommand } from "./commands/docs.js"; import { generateTypesCommand } from "./commands/generate-types.js"; import { lintCommand } from "./commands/lint.js"; +import { metricViewsCommand } from "./commands/metric-views/index.js"; import { pluginCommand } from "./commands/plugin/index.js"; import { setupCommand } from "./commands/setup.js"; @@ -28,5 +29,6 @@ cmd.addCommand(lintCommand); cmd.addCommand(docsCommand); cmd.addCommand(pluginCommand); cmd.addCommand(codemodCommand); +cmd.addCommand(metricViewsCommand); await cmd.parseAsync(); From 13b04603930dbd78ab46f367018ebe7fac31bb5f Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Wed, 24 Jun 2026 10:44:58 +0200 Subject: [PATCH 2/6] refactor(shared): rename humanizeMetricViewsPath to formatMetricViewsPath MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "format" describes what the helper does — render a Zod issue path as a CLI string — without the "humanize" framing it borrowed from the plugin-manifest validator's humanizePath. Update its caller, tests, and doc comments to match. Also trim redundant comments in the type-generator .d.ts shim and align the syncMetricViewsTypes doc with the appkit source. Co-authored-by: Isaac Signed-off-by: Atila Fassina --- .../commands/metric-views/sync/sync.test.ts | 2 +- .../validate-metric-views-source.test.ts | 14 ++++++------- .../validate-metric-views-source.ts | 20 +++++++++---------- .../src/cli/commands/type-generator.d.ts | 20 +++++++++---------- 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts b/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts index bbd8b183b..97b20d136 100644 --- a/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts +++ b/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts @@ -429,7 +429,7 @@ describe("appkit mv sync", () => { expect(exitSpy).toHaveBeenCalledWith(1); const errored = erroredText(); expect(errored).toContain("invalid"); - // Humanized path of the failing field. + // CLI path of the failing field. expect(errored).toContain("metricViews.revenue.source"); }); diff --git a/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.test.ts b/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.test.ts index bd4d4fc27..f6b88c9d4 100644 --- a/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.test.ts +++ b/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "vitest"; import { + formatMetricViewsPath, formatMetricViewsSourceErrors, - humanizeMetricViewsPath, validateMetricViewsSource, } from "./validate-metric-views-source"; @@ -145,26 +145,26 @@ describe("validateMetricViewsSource", () => { }); expect(result.valid).toBe(false); if (result.valid) throw new Error("expected invalid"); - // Root-level issue → humanized as "(root)". + // Root-level issues render as "(root)". expect(result.errors[0].path).toBe("(root)"); }); }); }); -describe("humanizeMetricViewsPath", () => { +describe("formatMetricViewsPath", () => { test("empty path renders as (root)", () => { - expect(humanizeMetricViewsPath([])).toBe("(root)"); - expect(humanizeMetricViewsPath(undefined)).toBe("(root)"); + expect(formatMetricViewsPath([])).toBe("(root)"); + expect(formatMetricViewsPath(undefined)).toBe("(root)"); }); test("nested object keys join with dots", () => { - expect(humanizeMetricViewsPath(["metricViews", "revenue", "source"])).toBe( + expect(formatMetricViewsPath(["metricViews", "revenue", "source"])).toBe( "metricViews.revenue.source", ); }); test("numeric segments render as bracket indices", () => { - expect(humanizeMetricViewsPath(["a", 0, "b"])).toBe("a[0].b"); + expect(formatMetricViewsPath(["a", 0, "b"])).toBe("a[0].b"); }); }); diff --git a/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.ts b/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.ts index fd0aa8b0d..5ff0f0ae3 100644 --- a/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.ts +++ b/packages/shared/src/cli/commands/metric-views/validate-metric-views-source.ts @@ -4,11 +4,10 @@ import { } from "../../../schemas/metric-source"; /** - * A single metric-source validation issue. `path` is a humanized property path - * (e.g. `metricViews.revenue.source`) suitable for direct CLI output, and - * `message` is the schema's own diagnostic. Mirrors the `SemanticIssue` shape - * used by the plugin-manifest validator so CLI output stays uniform across - * commands. + * A single metric-source validation issue. `path` is formatted for direct CLI + * output (e.g. `metricViews.revenue.source`), and `message` is the schema's own + * diagnostic. Mirrors the `SemanticIssue` shape used by the plugin-manifest + * validator so CLI output stays uniform across commands. */ export interface MetricViewsSourceIssue { path: string; @@ -21,12 +20,11 @@ export type ValidateMetricViewsSourceResult = | { valid: false; errors: MetricViewsSourceIssue[] }; /** - * Humanize a Zod issue path (array of object keys / array indices) into a - * single string like `metricViews.revenue.source`. Numeric segments render as - * `[n]`; an empty path (a root-level issue, e.g. an unrecognized top-level key) - * renders as `(root)`. Mirrors `humanizePath` in the plugin-manifest validator. + * Format a Zod issue path (array of object keys / array indices) as a CLI path + * like `metricViews.revenue.source`. Numeric segments render as `[n]`; an empty + * path (a root-level issue, e.g. an unrecognized top-level key) renders as `(root)`. */ -export function humanizeMetricViewsPath( +export function formatMetricViewsPath( path: ReadonlyArray | undefined, ): string { if (!path || path.length === 0) return "(root)"; @@ -60,7 +58,7 @@ export function validateMetricViewsSource( return { valid: true, source: result.data }; } const errors = result.error.issues.map((issue) => ({ - path: humanizeMetricViewsPath(issue.path as ReadonlyArray), + path: formatMetricViewsPath(issue.path as ReadonlyArray), message: issue.message, })); return { valid: false, errors }; diff --git a/packages/shared/src/cli/commands/type-generator.d.ts b/packages/shared/src/cli/commands/type-generator.d.ts index b9576cc3d..155ec2ce5 100644 --- a/packages/shared/src/cli/commands/type-generator.d.ts +++ b/packages/shared/src/cli/commands/type-generator.d.ts @@ -1,4 +1,3 @@ -// Type declarations for optional @databricks/appkit/type-generator module declare module "@databricks/appkit/type-generator" { export function generateFromEntryPoint(options: { queryFolder?: string; @@ -26,7 +25,6 @@ declare module "@databricks/appkit/type-generator" { noCache?: boolean; }): Promise; - /** Execution lane: `sp` (service principal) or `obo` (on-behalf-of). */ type MetricLane = "sp" | "obo"; /** Per-metric schema captured by {@link syncMetricViewsTypes}. */ @@ -53,16 +51,20 @@ declare module "@databricks/appkit/type-generator" { metricMetadataOutFile?: string; schemas: MetricSchema[]; failures: MetricSyncFailure[]; - /** `true` when no metric-views.json was found — nothing was synced. */ + // `true` when no metric-views.json was found — nothing was synced. noConfig: boolean; } /** - * Metric-only sync entry: read metric-views.json from `queryFolder`, DESCRIBE - * every entry (minus clean cache hits), and write `metric.d.ts` + - * `metrics.metadata.json`. Does NOT generate analytics query types. Backs - * `appkit mv sync`. `cache` defaults to ON; only `cache === false` (the - * CLI's `--no-cache`) disables the shared metric type-generation cache. + * Generate the metric-view type artifacts used by `appkit mv sync`. + * + * Reads `metric-views.json` from `queryFolder`, DESCRIBEs any metric views + * that are missing from the cache, then writes `metric.d.ts` and + * `metrics.metadata.json`. This only syncs metric-view types; analytics query + * types are generated separately. + * + * The cache is enabled by default. Pass `cache: false` to force fresh + * DESCRIBE results, matching the CLI's `--no-cache` flag. */ export function syncMetricViewsTypes(options: { queryFolder: string; @@ -72,8 +74,6 @@ declare module "@databricks/appkit/type-generator" { cache?: boolean; }): Promise; - /** Default filename for the generated MetricRegistry declarations. */ export const METRIC_TYPES_FILE: string; - /** Default filename for the build-time semantic-metadata JSON bundle. */ export const METRIC_METADATA_FILE: string; } From f96f5f0fdf8c73c9d5e77675fdda73543893428b Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Thu, 25 Jun 2026 08:46:06 +0200 Subject: [PATCH 3/6] fix(shared): warn when mv sync writes degraded metric types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `appkit mv sync` runs in describe-now mode, where a not-ready (cold or starting) warehouse can return no schema for a metric view WITHOUT a hard DESCRIBE failure: the appkit export writes permissive (degraded) types and reports them via `schemas[].degraded` with an empty `failures` list. The CLI only inspected `failures`, so this path fell through to the success message and exited 0 with no signal, silently shipping permissive `unknown` types. Add a degraded-schema check after the failures gate: name the affected views, tell the user to rerun once the warehouse is available, and exit 0 — a not-ready warehouse is transient, not a hard failure (genuine DESCRIBE failures still exit non-zero, unchanged). Also align the ambient `@databricks/appkit/type-generator` shim's `SyncMetricViewsTypesResult` with the real export by adding `fatalErrors` (always empty for the CLI's describe-now mode; declared to keep the cross-package contract honest). Co-authored-by: Isaac Signed-off-by: Atila Fassina --- .../commands/metric-views/sync/sync.test.ts | 59 ++++++++++++++++++- .../cli/commands/metric-views/sync/sync.ts | 18 ++++++ .../src/cli/commands/type-generator.d.ts | 4 ++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts b/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts index 97b20d136..4aeadf073 100644 --- a/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts +++ b/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts @@ -36,9 +36,12 @@ const { syncMetricViewsTypes, METRIC_TYPES_FILE, METRIC_METADATA_FILE } = // Annotate the array element types so the inferred return type is wide // enough for `mockResolvedValueOnce` overrides that populate `failures` // (an empty literal would otherwise infer `never[]`). - const schemas: Array<{ key: string; source: string; lane: string }> = [ - { key: "revenue", source: "demo.sales.revenue", lane: "sp" }, - ]; + const schemas: Array<{ + key: string; + source: string; + lane: string; + degraded?: boolean; + }> = [{ key: "revenue", source: "demo.sales.revenue", lane: "sp" }]; const failures: Array<{ key: string; source: string; @@ -133,6 +136,7 @@ describe("appkit mv sync", () => { let queryFolder: string; let consoleLog: Mock; let consoleError: Mock; + let consoleWarn: Mock; let originalCwd: string; const prevWarehouse = process.env.DATABRICKS_WAREHOUSE_ID; @@ -155,6 +159,9 @@ describe("appkit mv sync", () => { consoleError = vi .spyOn(console, "error") .mockImplementation(() => {}) as Mock; + consoleWarn = vi + .spyOn(console, "warn") + .mockImplementation(() => {}) as Mock; }); afterEach(() => { @@ -361,6 +368,8 @@ describe("appkit mv sync", () => { const erroredText = () => consoleError.mock.calls.flat().map(String).join("\n"); + const warnedText = () => consoleWarn.mock.calls.flat().map(String).join("\n"); + test("explicit --metric-views-json-path to a missing file: non-zero + recognizable message", async () => { const missing = path.join(tmpRoot, "nowhere", "metric-views.json"); @@ -534,6 +543,50 @@ describe("appkit mv sync", () => { expect(errored).toContain("TABLE_OR_VIEW_NOT_FOUND"); }); + test("degraded-but-not-failed (warehouse not ready): warns and exits 0 with permissive types", async () => { + writeConfig(); + // A not-ready warehouse returns no schema for a key WITHOUT a hard failure: + // `syncMetricViewsTypes` writes permissive (degraded) types and reports them + // via `schemas[].degraded` with an empty `failures` list. Unlike a per-entry + // DESCRIBE failure, the CLI must treat this as a WARNING (exit 0), not a hard + // failure — the entries refresh on a rerun once the warehouse is available. + syncMetricViewsTypes.mockResolvedValueOnce({ + metricOutFile: path.join( + tmpRoot, + "shared", + "appkit-types", + "metric.d.ts", + ), + metricMetadataOutFile: path.join( + tmpRoot, + "shared", + "appkit-types", + "metrics.metadata.json", + ), + schemas: [ + { + key: "revenue", + source: "demo.sales.revenue", + lane: "sp", + degraded: true, + }, + ], + failures: [], + noConfig: false, + }); + + const exitSpy = await runCliCapturingExit(["--warehouse-id", "wh-123"]); + + expect(syncMetricViewsTypes).toHaveBeenCalledTimes(1); + // Degraded is a warning, NOT a hard failure — the command exits 0. + expect(exitSpy).not.toHaveBeenCalled(); + const warned = warnedText(); + expect(warned).toContain("could not be described"); + expect(warned).toContain("revenue"); + expect(warned).toContain("permissive"); + expect(warned).toContain("Rerun"); + }); + // --- Phase 3: interactive flow ---------------------------------------------- test("no flags → interactive: prompts fire and resolved values reach syncMetricViewsTypes", async () => { diff --git a/packages/shared/src/cli/commands/metric-views/sync/sync.ts b/packages/shared/src/cli/commands/metric-views/sync/sync.ts index 677a986ed..282b2ab00 100644 --- a/packages/shared/src/cli/commands/metric-views/sync/sync.ts +++ b/packages/shared/src/cli/commands/metric-views/sync/sync.ts @@ -251,6 +251,24 @@ async function runMetricViewsSync( return; } + // Degraded-but-not-failed (e.g. a not-ready warehouse returned no schema for + // some keys): the permissive types ARE written, so unlike `result.failures` + // above this is a warning — not a hard failure — and the command still exits + // 0. The degraded entries refresh on a rerun once the warehouse is available. + const degradedKeys = result.schemas + .filter((schema) => schema.degraded) + .map((schema) => schema.key); + if (degradedKeys.length > 0) { + onProgress?.succeed( + `Generated permissive metric types: ${metricOutFile}`, + ); + console.warn( + `Warning: ${degradedKeys.length} metric view(s) (${degradedKeys.join(", ")}) could not be described — the warehouse wasn't ready, so permissive types were written. Rerun \`appkit mv sync\` once the warehouse is available.`, + ); + console.log(`Generated metric metadata: ${metricMetadataOutFile}`); + return; + } + onProgress?.succeed(`Generated metric types: ${metricOutFile}`); console.log(`Generated metric types: ${metricOutFile}`); console.log(`Generated metric metadata: ${metricMetadataOutFile}`); diff --git a/packages/shared/src/cli/commands/type-generator.d.ts b/packages/shared/src/cli/commands/type-generator.d.ts index 155ec2ce5..1782a495b 100644 --- a/packages/shared/src/cli/commands/type-generator.d.ts +++ b/packages/shared/src/cli/commands/type-generator.d.ts @@ -53,6 +53,10 @@ declare module "@databricks/appkit/type-generator" { failures: MetricSyncFailure[]; // `true` when no metric-views.json was found — nothing was synced. noConfig: boolean; + // Per-key fatal preflight errors. Always empty for `mv sync` (the CLI uses + // the default `describe-now` mode); populated only by the dev/Vite blocking + // path. Declared to match the real export's result contract. + fatalErrors: Array<{ name: string; message: string }>; } /** From 2992331182bc62a66ab14f86d45bb0ec11a8f170 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Thu, 25 Jun 2026 12:15:39 +0200 Subject: [PATCH 4/6] chore: rename metric typegen artifacts to metric-views.* Rename the generated metric-view artifacts for naming consistency with the `metric-views.json` source config (and to fix the metric/metrics singular-vs- plural split): - METRIC_TYPES_FILE: metric.d.ts -> metric-views.d.ts - METRIC_METADATA_FILE: metrics.metadata.json -> metric-views.metadata.json Touches the constants, the dev/Vite generate path, the ambient type-generator shim, the shared `mv sync` CLI, and all test assertions across appkit + shared. The Metric Views feature is pre-release (not shipped), so no migration is needed. Co-authored-by: Isaac Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/index.ts | 12 +++---- .../mv-registry/render-types.ts | 2 +- .../src/type-generator/tests/index.test.ts | 10 +++--- .../type-generator/tests/mv-registry.test.ts | 4 +-- .../tests/sync-metric-views-types.test.ts | 13 +++++--- .../type-generator/tests/vite-plugin.test.ts | 11 ++++--- .../commands/metric-views/sync/sync.test.ts | 32 +++++++++---------- .../cli/commands/metric-views/sync/sync.ts | 6 ++-- .../src/cli/commands/type-generator.d.ts | 4 +-- 9 files changed, 51 insertions(+), 43 deletions(-) diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index 83fe4def5..bd78cdf9a 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -277,10 +277,10 @@ async function probeWarehouseState( * degraded types immediately. `"blocking"` waits for / starts the warehouse * first, failing the build only for a deleted/deleting one. * @param options.mvOutFile - optional output file for the MetricRegistry - * augmentation. Defaults to a sibling `metric.d.ts` file under the same + * augmentation. Defaults to a sibling `metric-views.d.ts` file under the same * directory as `outFile`. Skipped entirely if `metric-views.json` is absent. * @param options.mvMetadataOutFile - optional output file for the - * build-time semantic metadata JSON bundle (`metrics.metadata.json`). + * build-time semantic metadata JSON bundle (`metric-views.metadata.json`). * Defaults to a sibling of `mvOutFile`. Skipped entirely if * `metric-views.json` is absent. * @param options.metricFetcher - optional DescribeFetcher used by @@ -421,8 +421,8 @@ export interface SyncMetricViewsTypesResult { * → partition cache hits vs describe-needed → optional warehouse preflight / * #406 status gate → describe ({@link syncMetrics} over * {@link createWorkspaceDescribeFetcher}) → persist + prune the `metrics` - * cache section → merge → write `metric.d.ts` - * ({@link generateMetricTypeDeclarations}) and `metrics.metadata.json` + * cache section → merge → write `metric-views.d.ts` + * ({@link generateMetricTypeDeclarations}) and `metric-views.metadata.json` * ({@link generateMetricsMetadataJson}). * * The shared typegen cache (the `metrics` section of `.appkit-types-cache.json`, @@ -860,7 +860,7 @@ 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"; +export const METRIC_TYPES_FILE = "metric-views.d.ts"; /** * Default filename for the build-time semantic-metadata JSON bundle, sibling of * {@link METRIC_TYPES_FILE}. Shape is `Record { 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 metricFile = path.join(metricsDir, "generated", "metric-views.d.ts"); const metadataFile = path.join( metricsDir, "generated", - "metrics.metadata.json", + "metric-views.metadata.json", ); const describeResponse: DatabricksStatementExecutionResponse = { @@ -330,7 +330,7 @@ describe("generateFromEntryPoint — metric-view emission", () => { fs.rmSync(metricsDir, { recursive: true, force: true }); }); - test("writes metric.d.ts and metrics.metadata.json when metric-views.json exists", async () => { + test("writes metric-views.d.ts and metric-views.metadata.json when metric-views.json exists", async () => { writeMetricConfig(); await expect( @@ -998,11 +998,11 @@ 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 metricFile = path.join(cacheTestDir, "generated", "metric-views.d.ts"); const metadataFile = path.join( cacheTestDir, "generated", - "metrics.metadata.json", + "metric-views.metadata.json", ); const describeResponseFor = ( diff --git a/packages/appkit/src/type-generator/tests/mv-registry.test.ts b/packages/appkit/src/type-generator/tests/mv-registry.test.ts index d1a5cd864..ae074014c 100644 --- a/packages/appkit/src/type-generator/tests/mv-registry.test.ts +++ b/packages/appkit/src/type-generator/tests/mv-registry.test.ts @@ -2135,7 +2135,7 @@ describe("buildMetricsMetadataBundle", () => { expect(Object.hasOwn(bundle, "__proto__")).toBe(true); expect(Object.hasOwn(bundle, "revenue")).toBe(true); - // The emitted metrics.metadata.json carries both as own enumerable + // The emitted metric-views.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"]); @@ -2264,7 +2264,7 @@ describe("generateMetricsMetadataJson — snapshot", () => { // 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 () => { + test("mixed-case keys order identically (code-unit) in metric-views.d.ts and metric-views.metadata.json", async () => { const resolution = resolveMetricConfig({ metricViews: { Revenue: { source: "a.b.r" }, diff --git a/packages/appkit/src/type-generator/tests/sync-metric-views-types.test.ts b/packages/appkit/src/type-generator/tests/sync-metric-views-types.test.ts index dae4ec3af..3037f0036 100644 --- a/packages/appkit/src/type-generator/tests/sync-metric-views-types.test.ts +++ b/packages/appkit/src/type-generator/tests/sync-metric-views-types.test.ts @@ -123,12 +123,17 @@ describe("syncMetricViewsTypes", () => { tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), "sync-metric-types-")); queryFolder = path.join(tmpRoot, "config", "queries"); fs.mkdirSync(queryFolder, { recursive: true }); - metricOutFile = path.join(tmpRoot, "shared", "appkit-types", "metric.d.ts"); + metricOutFile = path.join( + tmpRoot, + "shared", + "appkit-types", + "metric-views.d.ts", + ); metricMetadataOutFile = path.join( tmpRoot, "shared", "appkit-types", - "metrics.metadata.json", + "metric-views.metadata.json", ); }); @@ -161,7 +166,7 @@ describe("syncMetricViewsTypes", () => { expect(result.metricOutFile).toBe(metricOutFile); expect(result.metricMetadataOutFile).toBe(metricMetadataOutFile); - // --- metric.d.ts: MetricRegistry augmentation for both metrics --- + // --- metric-views.d.ts: MetricRegistry augmentation for both metrics --- const declarations = fs.readFileSync(metricOutFile, "utf-8"); expect(declarations).toContain("interface MetricRegistry"); expect(declarations).toContain('"revenue"'); @@ -176,7 +181,7 @@ describe("syncMetricViewsTypes", () => { // The TIMESTAMP dimension carries inferred time grains in its @timeGrain tag. expect(declarations).toContain("@timeGrain"); - // --- metrics.metadata.json: per-metric semantic bundle --- + // --- metric-views.metadata.json: per-metric semantic bundle --- const bundle = JSON.parse(fs.readFileSync(metricMetadataOutFile, "utf-8")); // SP metric: currency format spec is preserved on the measure. expect(bundle.revenue.measures.total_revenue.type).toBe("DECIMAL(38,2)"); 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 b6092587f..b93a96181 100644 --- a/packages/appkit/src/type-generator/tests/vite-plugin.test.ts +++ b/packages/appkit/src/type-generator/tests/vite-plugin.test.ts @@ -384,8 +384,8 @@ describe("appKitTypesPlugin — metric option plumbing", () => { test("custom mvOutFile/mvMetadataOutFile reach generateFromEntryPoint", async () => { const plugin = appKitTypesPlugin({ - mvOutFile: "custom/types/metric.d.ts", - mvMetadataOutFile: "custom/types/metrics.metadata.json", + mvOutFile: "custom/types/metric-views.d.ts", + mvMetadataOutFile: "custom/types/metric-views.metadata.json", }); getHook( plugin, @@ -398,10 +398,13 @@ describe("appKitTypesPlugin — metric option plumbing", () => { expect(mocks.generateFromEntryPoint).toHaveBeenCalledWith( expect.objectContaining({ - mvOutFile: path.resolve(process.cwd(), "custom/types/metric.d.ts"), + mvOutFile: path.resolve( + process.cwd(), + "custom/types/metric-views.d.ts", + ), mvMetadataOutFile: path.resolve( process.cwd(), - "custom/types/metrics.metadata.json", + "custom/types/metric-views.metadata.json", ), }), ); diff --git a/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts b/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts index 4aeadf073..5108fc534 100644 --- a/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts +++ b/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts @@ -31,7 +31,7 @@ const { syncMetricViewsTypes, METRIC_TYPES_FILE, METRIC_METADATA_FILE } = nodeFs.mkdirSync(nodePath.dirname(opts.metricOutFile), { recursive: true, }); - nodeFs.writeFileSync(opts.metricOutFile, "// metric.d.ts\n"); + nodeFs.writeFileSync(opts.metricOutFile, "// metric-views.d.ts\n"); nodeFs.writeFileSync(opts.metricMetadataOutFile, "{}\n"); // Annotate the array element types so the inferred return type is wide // enough for `mockResolvedValueOnce` overrides that populate `failures` @@ -57,8 +57,8 @@ const { syncMetricViewsTypes, METRIC_TYPES_FILE, METRIC_METADATA_FILE } = }; }, ), - METRIC_TYPES_FILE: "metric.d.ts", - METRIC_METADATA_FILE: "metrics.metadata.json", + METRIC_TYPES_FILE: "metric-views.d.ts", + METRIC_METADATA_FILE: "metric-views.metadata.json", })); // The library type-generator is an optional/ambient module; mock it so the @@ -195,13 +195,13 @@ describe("appkit mv sync", () => { tmpRoot, "shared", "appkit-types", - "metric.d.ts", + "metric-views.d.ts", ); const expectedMetadataOut = path.join( tmpRoot, "shared", "appkit-types", - "metrics.metadata.json", + "metric-views.metadata.json", ); // The appkit entry was called once with the resolved options. Cache is the @@ -254,12 +254,12 @@ describe("appkit mv sync", () => { expect(syncMetricViewsTypes).toHaveBeenCalledWith({ queryFolder: customConfigDir, warehouseId: "wh-123", - metricOutFile: path.join(tmpRoot, "build", "types", "metric.d.ts"), + metricOutFile: path.join(tmpRoot, "build", "types", "metric-views.d.ts"), metricMetadataOutFile: path.join( tmpRoot, "build", "types", - "metrics.metadata.json", + "metric-views.metadata.json", ), cache: undefined, }); @@ -287,8 +287,8 @@ describe("appkit mv sync", () => { expect(syncMetricViewsTypes).toHaveBeenCalledWith( expect.objectContaining({ queryFolder: path.dirname(absConfig), - metricOutFile: path.join(absOut, "metric.d.ts"), - metricMetadataOutFile: path.join(absOut, "metrics.metadata.json"), + metricOutFile: path.join(absOut, "metric-views.d.ts"), + metricMetadataOutFile: path.join(absOut, "metric-views.metadata.json"), }), ); }); @@ -513,13 +513,13 @@ describe("appkit mv sync", () => { tmpRoot, "shared", "appkit-types", - "metric.d.ts", + "metric-views.d.ts", ), metricMetadataOutFile: path.join( tmpRoot, "shared", "appkit-types", - "metrics.metadata.json", + "metric-views.metadata.json", ), schemas: [], failures: [ @@ -555,13 +555,13 @@ describe("appkit mv sync", () => { tmpRoot, "shared", "appkit-types", - "metric.d.ts", + "metric-views.d.ts", ), metricMetadataOutFile: path.join( tmpRoot, "shared", "appkit-types", - "metrics.metadata.json", + "metric-views.metadata.json", ), schemas: [ { @@ -614,13 +614,13 @@ describe("appkit mv sync", () => { tmpRoot, "shared", "appkit-types", - "metric.d.ts", + "metric-views.d.ts", ), metricMetadataOutFile: path.join( tmpRoot, "shared", "appkit-types", - "metrics.metadata.json", + "metric-views.metadata.json", ), cache: true, }); @@ -645,7 +645,7 @@ describe("appkit mv sync", () => { expect.objectContaining({ queryFolder: customConfigDir, warehouseId: "wh-interactive", - metricOutFile: path.join(tmpRoot, "alt", "out", "metric.d.ts"), + metricOutFile: path.join(tmpRoot, "alt", "out", "metric-views.d.ts"), }), ); }); diff --git a/packages/shared/src/cli/commands/metric-views/sync/sync.ts b/packages/shared/src/cli/commands/metric-views/sync/sync.ts index 282b2ab00..514ee12a7 100644 --- a/packages/shared/src/cli/commands/metric-views/sync/sync.ts +++ b/packages/shared/src/cli/commands/metric-views/sync/sync.ts @@ -31,7 +31,7 @@ export interface MetricViewsSyncOptions { * locked spec. */ metricViewsJsonPath?: string; - /** Output directory for metric.d.ts + metrics.metadata.json (default: /shared/appkit-types). */ + /** Output directory for metric-views.d.ts + metric-views.metadata.json (default: /shared/appkit-types). */ outputDir?: string; /** * Caching toggle. Commander's `--no-cache` sets this to `false` (and leaves it @@ -401,7 +401,7 @@ async function runMetricViewsSyncAction( export const metricViewsSyncCommand = new Command("sync") .description( - "Sync UC Metric View schemas: DESCRIBE every entry in metric-views.json, then emit metric.d.ts + metrics.metadata.json.", + "Sync UC Metric View schemas: DESCRIBE every entry in metric-views.json, then emit metric-views.d.ts + metric-views.metadata.json.", ) .option( "--warehouse-id ", @@ -413,7 +413,7 @@ export const metricViewsSyncCommand = new Command("sync") ) .option( "--output-dir ", - "Output directory for metric.d.ts and metrics.metadata.json (default: shared/appkit-types)", + "Output directory for metric-views.d.ts and metric-views.metadata.json (default: shared/appkit-types)", ) .option("--no-cache", "Disable the metric type-generation cache") .addHelpText( diff --git a/packages/shared/src/cli/commands/type-generator.d.ts b/packages/shared/src/cli/commands/type-generator.d.ts index 1782a495b..23f80d534 100644 --- a/packages/shared/src/cli/commands/type-generator.d.ts +++ b/packages/shared/src/cli/commands/type-generator.d.ts @@ -63,8 +63,8 @@ declare module "@databricks/appkit/type-generator" { * Generate the metric-view type artifacts used by `appkit mv sync`. * * Reads `metric-views.json` from `queryFolder`, DESCRIBEs any metric views - * that are missing from the cache, then writes `metric.d.ts` and - * `metrics.metadata.json`. This only syncs metric-view types; analytics query + * that are missing from the cache, then writes `metric-views.d.ts` and + * `metric-views.metadata.json`. This only syncs metric-view types; analytics query * types are generated separately. * * The cache is enabled by default. Pass `cache: false` to force fresh From 64449e4499715ce22f207476bc1534d4fe32b74b Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Thu, 25 Jun 2026 13:52:58 +0200 Subject: [PATCH 5/6] chore: cleanup tsdoc comments --- packages/appkit/src/type-generator/index.ts | 167 ++++-------------- .../src/cli/commands/metric-views/index.ts | 7 +- .../cli/commands/metric-views/sync/sync.ts | 64 +------ 3 files changed, 44 insertions(+), 194 deletions(-) diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index bd78cdf9a..118b50df7 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -46,7 +46,7 @@ dotenv.config(); const logger = createLogger("type-generator"); /** - * Upper bound (~5 min) on how long the metric path's `blocking`-mode preflight + * Upper bound (~5 min) on how long the Metric Views path's `blocking`-mode preflight * waits for a warehouse to reach RUNNING. Mirrors the query path's (unexported) * `PREFLIGHT_WAIT_MAX_MS` in query-registry.ts. */ @@ -78,7 +78,6 @@ function formatFailureRows( const tag = color(label.padEnd(7)); const rows: string[] = []; for (const [message, names] of byMessage) { - // Unique message → keep the compact one-line `tag name message` form. if (names.length === 1) { rows.push( ` ${tag} ${pc.bold(names[0].padEnd(maxNameLen))} ${pc.dim(message)}`, @@ -363,14 +362,10 @@ export async function generateFromEntryPoint(options: { } } - // One-time migration: remove old generated file and patch project configs await removeOldGeneratedTypes(projectRoot, "appKitTypes.d.ts"); await migrateProjectConfig(projectRoot); - // Types are always written above — including `result: unknown` for any query - // that could not be described. Connectivity failures pass silently so a - // transient warehouse outage never blocks a build; genuine SQL errors and - // non-connectivity fatal request failures surface after the file write. + // Types are always written above — including `result: unknown` for any Metric View that could not be described. if (syntaxErrors.length > 0) { throw new TypegenSyntaxError(syntaxErrors, warehouseId, fatalErrors); } @@ -387,13 +382,9 @@ export async function generateFromEntryPoint(options: { * report what happened and decide its exit code. */ export interface SyncMetricViewsTypesResult { - /** Absolute path the MetricRegistry `.d.ts` was written to (undefined when no config). */ metricOutFile?: string; - /** Absolute path the semantic-metadata JSON bundle was written to (undefined when no config). */ metricMetadataOutFile?: string; - /** Schemas emitted, one per configured metric key (empty when no config). */ schemas: MetricSchema[]; - /** Per-entry DESCRIBE failures surfaced by {@link syncMetrics}. */ failures: MetricSyncFailure[]; /** * `true` when no `metric-views.json` was found in the query folder, so nothing @@ -415,58 +406,23 @@ export interface SyncMetricViewsTypesResult { * CLI (default `"describe-now"` mode) and {@link generateFromEntryPoint}'s * metric section (which forwards its dev `"non-blocking"`/`"blocking"` mode). * - * It does the focused metric pipeline ONLY — it never describes analytics - * queries and never writes `analytics.d.ts` / `serving.d.ts`. The pipeline: - * read config ({@link readMetricConfig}) → resolve ({@link resolveMetricConfig}) - * → partition cache hits vs describe-needed → optional warehouse preflight / - * #406 status gate → describe ({@link syncMetrics} over - * {@link createWorkspaceDescribeFetcher}) → persist + prune the `metrics` - * cache section → merge → write `metric-views.d.ts` - * ({@link generateMetricTypeDeclarations}) and `metric-views.metadata.json` - * ({@link generateMetricsMetadataJson}). - * - * The shared typegen cache (the `metrics` section of `.appkit-types-cache.json`, - * same {@link metricCacheHash} change-detector and {@link MetricCacheEntry} - * shape) means a second run over an unchanged, healthy config makes zero - * warehouse calls. `cache === false` (the CLI's `--no-cache`) ignores the cached - * section entirely (every key becomes describe-needed) and overwrites it with - * this pass's results. + * The shared typegen cache (the `metrics` section of `.appkit-types-cache.json`, same {@link metricCacheHash} change-detector and {@link MetricCacheEntry} shape) means a second run over an unchanged, healthy config makes zero warehouse calls. `cache === false` (the CLI's `--no-cache`) ignores the cached section entirely (every key becomes describe-needed) and overwrites it with this pass's results. * * The `mode` toggle is the ONLY axis that differs between callers: - * - `"describe-now"` (default, the CLI): no preflight, no #406 status probe — - * DESCRIBE every key that isn't a clean cache hit. The hit predicate is - * STRICTER here: a degraded/sticky cached entry is NEVER served (it is - * re-described), so a focused `mv sync` always converges to correct types, - * and the sticky-degraded notice never fires (nothing degraded is served). - * - `"non-blocking"` (dev/Vite default): honor the #406 contract — one - * status-only probe, DESCRIBE only when the warehouse is already RUNNING, - * else emit degraded artifacts immediately. Degraded cache hits ARE served - * (last-known-good) and surfaced via the sticky-degraded notice. - * - `"blocking"`: wait for / start the warehouse first (only a - * deleted/deleting one is fatal), then DESCRIBE. Degraded cache hits are - * served, same as non-blocking. A fatal preflight is reported via - * {@link SyncMetricViewsTypesResult.fatalErrors} (the artifacts are still - * written) so the caller can throw after the writes. + * - `"describe-now"` (default, the CLI): no preflight, no status probe — DESCRIBE every key that isn't a clean cache hit. The hit predicate is STRICTER here: a degraded/sticky cached entry is NEVER served (it is re-described), so a focused `mv sync` always converges to correct types, and the sticky-degraded notice never fires (nothing degraded is served). + * - `"non-blocking"` (dev/Vite default): one status-only probe, DESCRIBE only when the warehouse is already RUNNING, else emit degraded artifacts immediately. Degraded cache hits ARE served (last-known-good) and surfaced via the sticky-degraded notice. + * - `"blocking"`: wait for / start the warehouse first (only a deleted/deleting one is fatal), then DESCRIBE. Degraded cache hits are served, same as non-blocking. A fatal preflight is reported via {@link SyncMetricViewsTypesResult.fatalErrors} (the artifacts are still written) so the caller can throw after the writes. * - * An injected `metricFetcher` always runs — it hits no warehouse, so it bypasses - * both the blocking preflight and the non-blocking gate regardless of mode. + * An injected `metricFetcher` always runs — it hits no warehouse, so it bypasses both the blocking preflight and the non-blocking gate regardless of mode. * - * @param options.queryFolder - folder that holds `metric-views.json` - * (conventionally `/config/queries`). Returns early with - * `noConfig: true` when the file is absent — additive, never an error. + * @param options.queryFolder - folder that holds `metric-views.json` (conventionally `/config/queries`). Returns early with `noConfig: true` when the file is absent — additive, never an error. * @param options.warehouseId - SQL warehouse used for `DESCRIBE TABLE EXTENDED`. * @param options.metricOutFile - output path for the MetricRegistry `.d.ts`. - * @param options.metricMetadataOutFile - output path for the semantic-metadata - * JSON bundle. - * @param options.cache - cache toggle, default ON. Only `cache === false` - * disables it (so `undefined`/`true` keep caching). Mirrors the `noCache` - * convention on {@link generateFromEntryPoint}: gate the cache READ - * (`!noCache`) and overwrite the `metrics` section on SAVE. + * @param options.metricMetadataOutFile - output path for the semantic-metadata JSON bundle. + * @param options.cache - cache toggle, default ON. Only `cache === false` disables it (so `undefined`/`true` keep caching). Mirrors the `noCache` convention on {@link generateFromEntryPoint}: gate the cache READ (`!noCache`) and overwrite the `metrics` section on SAVE. * @param options.metricFetcher - optional injected {@link DescribeFetcher} * (tests pass a mock; production lazily builds a WorkspaceClient-backed one). - * @param options.mode - preflight/gate policy, default `"describe-now"`. See - * above; the CLI omits it (taking `"describe-now"`), - * {@link generateFromEntryPoint} forwards its own {@link PreflightMode}. + * @param options.mode - preflight/gate policy, default `"describe-now"`. See above; the CLI omits it (taking `"describe-now"`), {@link generateFromEntryPoint} forwards its own {@link PreflightMode}. */ export async function syncMetricViewsTypes(options: { queryFolder: string; @@ -502,13 +458,7 @@ export async function syncMetricViewsTypes(options: { const fatalErrors: Array<{ name: string; message: string }> = []; - // Load the shared typegen cache and copy its `metrics` section into a - // null-prototype map. Metric keys are user-controlled config and - // "__proto__"/"constructor" pass the metric key regex — a null prototype - // keeps a malicious/edge key from hitting an Object.prototype setter on write - // or resolving inherited names as phantom entries on read. With `noCache`, the - // section starts empty (every entry describe-needed) and is overwritten on - // save below. + // Load the shared typegen cache and copy its `metrics` section into a null-prototype map. const cache = await loadCache(); const mvCacheSection: Record = Object.create(null); if (!noCache && cache.metrics) { @@ -517,12 +467,9 @@ export async function syncMetricViewsTypes(options: { } } - // Dev modes (`non-blocking`/`blocking`) serve degraded cache hits as - // last-known-good — exactly like queries degrade to cached types — and - // surface them via the sticky-degraded notice. `describe-now` (the CLI) is an - // explicit "make my types correct now" action, so it NEVER serves a - // degraded/sticky entry: that entry is re-described instead, and no degraded - // hit is served, so the notice never fires. + // Dev modes (`non-blocking`/`blocking`) serve degraded cache hits as last-known-good — exactly like queries degrade to cached types — + // and surface them via the sticky-degraded notice. `describe-now` (the CLI) is an explicit "make my types correct now" action, + // so it NEVER serves a degraded/sticky entry: that entry is re-described instead, and no degraded hit is served, so the notice never fires. const serveDegraded = mode !== "describe-now"; // Partition BEFORE any gate/preflight decision: a hit (structurally valid @@ -563,22 +510,14 @@ export async function syncMetricViewsTypes(options: { ); } - // At most ONE WorkspaceClient per pass for the whole metric path: the status - // probe, the blocking preflight, and the default DESCRIBE fetcher share this - // lazily-created instance, so a pass that never contacts the warehouse - // constructs zero clients. let mvClient: WorkspaceClient | undefined; const getMvClient = (): WorkspaceClient => { mvClient ??= new WorkspaceClient({}); return mvClient; }; - // Blocking-mode preflight: ensure the warehouse is running before the DESCRIBE - // batch (probe → decide → wait / start+wait; only DELETED/DELETING is fatal). - // Two softenings vs the query preflight: a failed probe and a timed-out wait - // are NOT fatal here — we fall through to syncMetrics, which classifies a - // still-not-ready warehouse as degraded rather than failing the build. Skipped - // for `describe-now`/`non-blocking` (only `mode === "blocking"` enters here). + // Blocking-mode preflight: ensure the warehouse is running before the MV DESCRIBE + // batch (probe → decide → wait / start+wait; only DELETED/DELETING is fatal). Two softenings vs the query preflight: a failed probe and a timed-out wait are NOT fatal here — we fall through to syncMetrics, which classifies a still-not-ready warehouse as degraded rather than failing the build. Skipped for `describe-now`/`non-blocking` (only `mode === "blocking"` enters here). let preflightFatalMessage: string | undefined; if ( mode === "blocking" && @@ -600,8 +539,7 @@ export async function syncMetricViewsTypes(options: { }); 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. + // DELETED/DELETING — the warehouse was deleted while we waited. preflightFatalMessage = `warehouse ${warehouseId} is ${settled}`; } } else if (decision === "waitThenProceed") { @@ -609,35 +547,25 @@ export async function syncMetricViewsTypes(options: { maxMs: MV_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. + // Deleted mid-wait: fatal. preflightFatalMessage = `warehouse ${warehouseId} is ${settled}`; } } } catch (err) { // Connectivity blip: fall through to syncMetrics, whose DESCRIBEs degrade - // a not-ready / unreachable warehouse rather than throwing. A - // deterministic failure (auth, bad warehouse id, a timed-out start) is - // fatal — surface it instead of stalling ~5 min against a not-ready - // warehouse, mirroring the query path's preflight catch. + // a not-ready / unreachable warehouse rather than throwing. if (!isConnectivityError(err)) { preflightFatalMessage = `warehouse ${warehouseId}: ${getErrorDiagnostic(err)}`; } } } - // Honor the non-blocking preflight contract (#406) for metric DESCRIBEs: a + // Honor the non-blocking preflight contract for metric DESCRIBEs: a // `DESCRIBE TABLE EXTENDED ... AS JSON` waits up to 30s per key and auto-starts - // a stopped warehouse — exactly what "non-blocking" promises not to do. So one - // status-only probe (which can't start the warehouse) decides whether to - // DESCRIBE now or emit degraded artifacts for a later blocking run; it keeps - // the observed state so the skip can tell a transient not-running warehouse - // from a terminal DELETED/DELETING one. `describe-now` and `blocking` both - // start `describeNow = true` (`mode !== "non-blocking"`), so this gate is - // skipped for them — `describe-now` describes directly, `blocking` already ran - // its preflight above. + // a stopped warehouse. So one status-only probe decides whether to DESCRIBE now or + // emit degraded artifacts for a later blocking run; + // it keeps the observed state so the skip can tell a transient not-running warehouse + // from a terminal DELETED/DELETING one. let gateState: WarehouseState | undefined; let describeNow = metricFetcher !== undefined || @@ -703,9 +631,7 @@ export async function syncMetricViewsTypes(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). + // state (stopped / cold-starting), so their schemas are unknown. const failedKeys = new Set(failures.map((f) => f.key)); const degradedKeys = described .filter((s) => s.degraded && !failedKeys.has(s.key)) @@ -720,11 +646,8 @@ export async function syncMetricViewsTypes(options: { } } else { // Un-probed DESCRIBEs deliberately skipped, not failures: emit each - // describe-needed key as a degraded schema (permissive types) so both - // artifacts exist; cache hits keep serving last-known-good. A transient - // state refreshes on a later RUNNING pass; a DELETED/DELETING probe is - // terminal, so those keys are pinned sticky below. (Only reachable in - // `non-blocking` mode.) + // describe-needed key as a degraded schema so both artifacts exist; cache hits keep serving last-known-good. + // A transient state refreshes on a later RUNNING pass; a DELETED/DELETING probe is terminal, so those keys are pinned sticky below. described = describeNeeded.map(emptyMetricSchema); terminalSkip = gateState === "DELETED" || gateState === "DELETING"; logger.info( @@ -737,23 +660,13 @@ export async function syncMetricViewsTypes(options: { // Persist outcomes for exactly the keys this pass owned (the describe-needed // set); hits were partitioned out above and are never rewritten, so a - // warehouse-down pass keeps last-known-good entries. A successful DESCRIBE - // caches `retry: false`; a degraded outcome caches `retry: true` only when - // re-describing could later succeed (non-terminal state or transient failure), - // else sticky `retry: false`. In `describe-now` there is no preflight/gate, so - // `terminalSkip` is always false and this reduces to "retry a degraded outcome - // unless it was a deterministic DESCRIBE failure" — a deterministic failure - // won't loop forever, and the stricter hit rule re-describes it next run - // anyway. One save per pass; with `noCache` the section started empty, so it's - // overwritten. + // warehouse-down pass keeps last-known-good entries. 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]. + // syncMetrics return one schema per entry in entry order, so described[i] always belongs to describeNeeded[i]. const entry = describeNeeded[i]; const failure = failureByKey.get(entry.key); mvCacheSection[entry.key] = { @@ -766,8 +679,7 @@ export async function syncMetricViewsTypes(options: { }; } - // Prune entries whose key is no longer configured so a removed metric doesn't - // haunt the cache file forever. + // Prune entries whose key is no longer configured const configuredKeys = new Set(resolution.entries.map((e) => e.key)); let prunedCount = 0; for (const key of Object.keys(mvCacheSection)) { @@ -777,17 +689,13 @@ export async function syncMetricViewsTypes(options: { } } - // 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. With `noCache` the section started empty, so it's overwritten. + // Save when this pass produced outcomes, bypassed the cache, or pruned. if (describeNeeded.length > 0 || noCache || prunedCount > 0) { cache.metrics = mvCacheSection; await saveCache(cache); } - // Merge cached hits with fresh results back into config order (renderers sort - // internally where determinism matters). Every describe-needed entry yields - // exactly one schema above, so the final fallback is defensive only. + // Merge cached hits with fresh results back into config order. const describedByKey = new Map(); for (const schema of described) { describedByKey.set(schema.key, schema); @@ -795,11 +703,6 @@ export async function syncMetricViewsTypes(options: { const schemas = resolution.entries.map((entry) => { const schema = hitSchemas.get(entry.key) ?? describedByKey.get(entry.key); if (schema !== undefined) return schema; - // Defensive: every entry is either a cache hit or describe-needed (and every - // describe-needed entry yields exactly one schema above), so this should be - // unreachable. If the invariant ever breaks, warn loudly but still emit a - // permissive degraded schema — the metric path never crashes a build over a - // single entry. logger.warn( "no schema resolved for metric key %s — emitting degraded types (should not happen)", entry.key, @@ -853,13 +756,9 @@ export type { 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-views.d.ts"; /** * Default filename for the build-time semantic-metadata JSON bundle, sibling of diff --git a/packages/shared/src/cli/commands/metric-views/index.ts b/packages/shared/src/cli/commands/metric-views/index.ts index 7e03dd2fc..334176003 100644 --- a/packages/shared/src/cli/commands/metric-views/index.ts +++ b/packages/shared/src/cli/commands/metric-views/index.ts @@ -4,10 +4,9 @@ import { metricViewsSyncCommand } from "./sync/sync"; /** * Parent command for UC Metric View operations. * - * Phase 1 exposes a single subcommand (`sync`). Future subcommands - * (`list` / `validate` / `describe`) plug in here so users have one top-level - * surface for everything related to Metric Views. Sibling of `plugin`, - * `setup`, `generate-types`, `lint`, `docs`, `codemod`. + * Exposes a single subcommand (`sync`). + * Future subcommands (`list` / `validate` / `describe`) plug in here so users have one top-level surface for everything related to Metric Views. + * Sibling of `plugin`, `setup`, `generate-types`, `lint`, `docs`, `codemod`. */ export const metricViewsCommand = new Command("mv") .description("Metric-view management commands (UC Metric Views)") diff --git a/packages/shared/src/cli/commands/metric-views/sync/sync.ts b/packages/shared/src/cli/commands/metric-views/sync/sync.ts index 514ee12a7..438bafeee 100644 --- a/packages/shared/src/cli/commands/metric-views/sync/sync.ts +++ b/packages/shared/src/cli/commands/metric-views/sync/sync.ts @@ -11,47 +11,21 @@ import { /** * Options parsed by commander for `appkit mv sync`. * - * Phase 3 locks the flag surface to exactly four options and adds the * interactive clack flow: * - `--warehouse-id` (+ `DATABRICKS_WAREHOUSE_ID` fallback) * - `--metric-views-json-path` (canonical config path) * - `--output-dir` (artifact output directory; replaces Phase 1's `--out-dir`) - * - `--no-cache` (commander negation → `cache === false` disables the - * metric type-generation cache) - * - * Phase 1's interim `--root-dir` is dropped: relative `--metric-views-json-path` - * / `--output-dir` resolve against `process.cwd()`, mirroring how - * `generate-types` anchors its defaults at the current directory. + * - `--no-cache` (commander negation → `cache === false` disables the metric type-generation cache) */ export interface MetricViewsSyncOptions { warehouseId?: string; - /** - * Path to metric-views.json. Default: - * `/config/queries/metric-views.json`. Canonical flag name in the - * locked spec. - */ metricViewsJsonPath?: string; - /** Output directory for metric-views.d.ts + metric-views.metadata.json (default: /shared/appkit-types). */ outputDir?: string; - /** - * Caching toggle. Commander's `--no-cache` sets this to `false` (and leaves it - * `true`/absent otherwise); `cache === false` is the single signal that - * disables the metric type-generation cache when forwarded to - * `syncMetricViewsTypes`. - */ cache?: boolean; } -/** Default filename for the metric source config (post-#433 name). */ const METRIC_VIEWS_CONFIG_FILE = "metric-views.json"; -/** - * Non-zero exit code for `appkit mv sync` failure modes. Every error mode - * exits with this same code and a distinct, recognizable message (the failure - * mode is identified by that message, not by a bespoke per-mode code — keeping - * the single Phase-1 exit mechanism). The dormant (no-config) and success cases - * take an early `return`, exiting 0 naturally. - */ const EXIT_FAILURE = 1; /** Resolved, absolute paths the sync run operates on. */ @@ -77,7 +51,6 @@ function resolvePaths(options: { }): ResolvedPaths { const cwd = process.cwd(); - // metric-views.json: --metric-views-json-path > /config/queries/metric-views.json. const explicitConfigPath = options.metricViewsJsonPath !== undefined; const configPath = options.metricViewsJsonPath ? path.isAbsolute(options.metricViewsJsonPath) @@ -85,8 +58,6 @@ function resolvePaths(options: { : path.resolve(cwd, options.metricViewsJsonPath) : path.join(cwd, "config", "queries", METRIC_VIEWS_CONFIG_FILE); - // Output paths under shared/appkit-types — matches how generate-types - // resolves its output directory. const outDir = options.outputDir ? path.isAbsolute(options.outputDir) ? options.outputDir @@ -105,7 +76,8 @@ function resolvePaths(options: { * The shared sync core for BOTH the interactive and non-interactive paths: * resolve paths → existence check (dormancy vs missing) → read + `JSON.parse` * → schema-validate → require warehouse → ONLY THEN dynamic-import appkit + - * `syncMetricViewsTypes`. Reaches the appkit metric-sync core through a dynamic + * `syncMetricViewsTypes`. + * Reaches the appkit metric-sync core through a dynamic * `import("@databricks/appkit/type-generator")` — the exact pattern * `generate-types.ts` uses — so the `shared` CLI package carries NO static * dependency on `@databricks/appkit` and compiles without it. @@ -141,13 +113,8 @@ async function runMetricViewsSync( return; } - // Existence is checked before anything else (including the warehouse-id - // requirement) so the dormancy invariant holds unconditionally: - // - DEFAULT path absent → additive path is dormant, exit 0. An opt-in - // project that never adopted metric views must NOT error, even without a - // warehouse configured. - // - EXPLICIT --metric-views-json-path absent → the user named a file that - // isn't there; that's a real error, exit non-zero. + // Check existence before requiring a warehouse. Missing default config means + // metric views are unused; a missing explicit path is an error. if (!fs.existsSync(configPath)) { if (explicitConfigPath) { console.error(`Error: metric-views.json not found at ${configPath}.`); @@ -160,8 +127,6 @@ async function runMetricViewsSync( return; } - // Read + parse the config before touching appkit. A malformed file is a - // user error with a precise location, not an appkit/warehouse failure. const rawConfig = fs.readFileSync(configPath, "utf-8"); let parsedConfig: unknown; try { @@ -173,10 +138,7 @@ async function runMetricViewsSync( return; } - // Schema-validate against the canonical metricSourceSchema (single source - // of truth) BEFORE the dynamic import. Bad FQN grammar, an unknown - // executor, an unrecognized key, or a bad metric key fail here with the - // `path: message` list — never as an opaque downstream error. + // Schema-validate against the canonical metricSourceSchema BEFORE the dynamic import. const validation = validateMetricViewsSource(parsedConfig); if (!validation.valid) { console.error(`Error: invalid ${configPath}:`); @@ -185,9 +147,7 @@ async function runMetricViewsSync( return; } - // The warehouse is only needed once we have a valid config to sync; require - // it here (after dormancy + validation) so a dormant/invalid project never - // trips on a missing warehouse. + // The warehouse is only needed once we have a valid config to sync; require it here (after dormancy + validation) so a dormant/invalid project never trips on a missing warehouse. const warehouseId = options.warehouseId || process.env.DATABRICKS_WAREHOUSE_ID; if (!warehouseId) { @@ -214,8 +174,6 @@ async function runMetricViewsSync( metricOutFile, metricMetadataOutFile, warehouseId, - // `--no-cache` → cache === false disables the metric typegen cache; - // absent/true keeps the default (cache on). Forwarded verbatim. cache: options.cache, }); } catch (err) { @@ -319,23 +277,19 @@ async function runInteractive(): Promise { message: "SQL Warehouse ID", placeholder: process.env.DATABRICKS_WAREHOUSE_ID ? `${process.env.DATABRICKS_WAREHOUSE_ID} (from DATABRICKS_WAREHOUSE_ID)` - : "1234abcd5678efgh", - // Optional: a blank value defers to DATABRICKS_WAREHOUSE_ID (validated - // downstream by runMetricViewsSync, which errors if neither is set). + : "your-warehouse-id", }); if (isCancel(warehouseId)) return cancelled(); const metricViewsJsonPath = await text({ message: "Path to metric-views.json", placeholder: "config/queries/metric-views.json", - // Optional: blank uses the canonical default path. }); if (isCancel(metricViewsJsonPath)) return cancelled(); const outputDir = await text({ message: "Output directory for generated types", placeholder: "shared/appkit-types", - // Optional: blank uses the canonical default output dir. }); if (isCancel(outputDir)) return cancelled(); @@ -349,8 +303,6 @@ async function runInteractive(): Promise { warehouseId: trimmed(warehouseId), metricViewsJsonPath: trimmed(metricViewsJsonPath), outputDir: trimmed(outputDir), - // Interactive runs use the default cache behavior (cache on); --no-cache is - // a non-interactive flag. cache: true, }; From 120eb94431fb01a6102e735304b9a52b169c2ade Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Tue, 30 Jun 2026 11:20:28 +0200 Subject: [PATCH 6/6] chore(shared): address mv sync review findings - interactive mode no longer double-prints the success line - narrow appkit-missing detection so real sync errors surface verbatim - match the interactive outro to the run outcome (dormant != success) - exit 0 on interactive cancel (consistent with plugin create) - canonical command `metric-views` with `mv` alias - add `--wait` (blocking preflight) to fail instead of degrade in CI - document the ambient type-generator shim's intentional narrowing Co-authored-by: Isaac Signed-off-by: Atila Fassina --- .../src/cli/commands/metric-views/index.ts | 8 +- .../commands/metric-views/sync/sync.test.ts | 181 +++++++++++++++++- .../cli/commands/metric-views/sync/sync.ts | 127 +++++++++--- .../src/cli/commands/type-generator.d.ts | 22 +++ 4 files changed, 302 insertions(+), 36 deletions(-) diff --git a/packages/shared/src/cli/commands/metric-views/index.ts b/packages/shared/src/cli/commands/metric-views/index.ts index 334176003..fd1613a57 100644 --- a/packages/shared/src/cli/commands/metric-views/index.ts +++ b/packages/shared/src/cli/commands/metric-views/index.ts @@ -8,12 +8,16 @@ import { metricViewsSyncCommand } from "./sync/sync"; * Future subcommands (`list` / `validate` / `describe`) plug in here so users have one top-level surface for everything related to Metric Views. * Sibling of `plugin`, `setup`, `generate-types`, `lint`, `docs`, `codemod`. */ -export const metricViewsCommand = new Command("mv") +export const metricViewsCommand = new Command("metric-views") + // `metric-views` is the canonical name shown in --help (full-word, consistent + // with `plugin` / `generate-types`); `mv` is the ergonomic shorthand alias. + .alias("mv") .description("Metric-view management commands (UC Metric Views)") .addCommand(metricViewsSyncCommand) .addHelpText( "after", ` Examples: - $ appkit mv sync --warehouse-id 1234abcd5678efgh`, + $ appkit metric-views sync --warehouse-id 1234abcd5678efgh + $ appkit mv sync --warehouse-id 1234abcd5678efgh # 'mv' is a shorthand alias`, ); diff --git a/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts b/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts index 5108fc534..7ea2eb796 100644 --- a/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts +++ b/packages/shared/src/cli/commands/metric-views/sync/sync.test.ts @@ -25,6 +25,7 @@ const { syncMetricViewsTypes, METRIC_TYPES_FILE, METRIC_METADATA_FILE } = metricOutFile: string; metricMetadataOutFile: string; cache?: boolean; + mode?: "describe-now" | "non-blocking" | "blocking"; }) => { const nodeFs = require("node:fs") as typeof import("node:fs"); const nodePath = require("node:path") as typeof import("node:path"); @@ -35,7 +36,7 @@ const { syncMetricViewsTypes, METRIC_TYPES_FILE, METRIC_METADATA_FILE } = nodeFs.writeFileSync(opts.metricMetadataOutFile, "{}\n"); // Annotate the array element types so the inferred return type is wide // enough for `mockResolvedValueOnce` overrides that populate `failures` - // (an empty literal would otherwise infer `never[]`). + // or `fatalErrors` (an empty literal would otherwise infer `never[]`). const schemas: Array<{ key: string; source: string; @@ -48,11 +49,13 @@ const { syncMetricViewsTypes, METRIC_TYPES_FILE, METRIC_METADATA_FILE } = reason: string; transient: boolean; }> = []; + const fatalErrors: Array<{ name: string; message: string }> = []; return { metricOutFile: opts.metricOutFile, metricMetadataOutFile: opts.metricMetadataOutFile, schemas, failures, + fatalErrors, noConfig: false, }; }, @@ -303,11 +306,17 @@ describe("appkit mv sync", () => { test("appkit absent: recognizable error message + non-zero exit", async () => { writeConfig(); - // Model the dynamic import failing as it does when @databricks/appkit - // isn't installed. - syncMetricViewsTypes.mockRejectedValueOnce( - new Error("Cannot find module '@databricks/appkit/type-generator'"), + // Model a module-resolution failure reaching the catch, as the dynamic + // `import("@databricks/appkit/type-generator")` throws when @databricks/appkit + // isn't installed: a structured ERR_MODULE_NOT_FOUND naming the package. Only + // THIS shape maps to the "not installed" guidance (see the next test). + const moduleNotFound = Object.assign( + new Error( + "Cannot find package '@databricks/appkit' imported from /app/cli.js", + ), + { code: "ERR_MODULE_NOT_FOUND" }, ); + syncMetricViewsTypes.mockRejectedValueOnce(moduleNotFound); const exitSpy = vi .spyOn(process, "exit") @@ -317,7 +326,33 @@ describe("appkit mv sync", () => { const errored = consoleError.mock.calls.flat().map(String).join("\n"); expect(errored).toContain( - "appkit mv sync is only available with @databricks/appkit installed", + "appkit metric-views sync is only available with @databricks/appkit installed", + ); + expect(exitSpy).toHaveBeenCalledWith(1); + + exitSpy.mockRestore(); + }); + + test("a non-module sync error propagates verbatim (not misreported as missing appkit)", async () => { + writeConfig(); + // A real failure from syncMetricViewsTypes — e.g. an unreachable warehouse — + // carries its own message and no module-resolution code. It must surface + // verbatim with a non-zero exit, NOT be rewritten as "install appkit" just + // because some unrelated message might mention a module. + syncMetricViewsTypes.mockRejectedValueOnce( + new Error("Warehouse wh-123 is unreachable (timed out after 30s)"), + ); + + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never); + + await runCli(["--warehouse-id", "wh-123"]); + + const errored = consoleError.mock.calls.flat().map(String).join("\n"); + expect(errored).toContain("Warehouse wh-123 is unreachable"); + expect(errored).not.toContain( + "only available with @databricks/appkit installed", ); expect(exitSpy).toHaveBeenCalledWith(1); @@ -530,6 +565,7 @@ describe("appkit mv sync", () => { transient: false, }, ], + fatalErrors: [], noConfig: false, }); @@ -572,6 +608,7 @@ describe("appkit mv sync", () => { }, ], failures: [], + fatalErrors: [], noConfig: false, }); @@ -587,6 +624,105 @@ describe("appkit mv sync", () => { expect(warned).toContain("Rerun"); }); + // --- --wait (blocking preflight): CI opt-in to fail instead of degrade ------ + + test("--wait forwards mode: 'blocking' to syncMetricViewsTypes", async () => { + writeConfig(); + + await runCli(["--warehouse-id", "wh-123", "--wait"]); + + expect(syncMetricViewsTypes).toHaveBeenCalledWith( + expect.objectContaining({ mode: "blocking" }), + ); + }); + + test("without --wait, mode stays describe-now (never blocking)", async () => { + writeConfig(); + + await runCli(["--warehouse-id", "wh-123"]); + + const call = syncMetricViewsTypes.mock.calls[0][0] as { mode?: string }; + // describe-now is appkit's default; the CLI omits the key entirely. + expect(call.mode).not.toBe("blocking"); + }); + + test("--wait + still-degraded result: hard failure (exit 1), not a warning", async () => { + writeConfig(); + // Even after waiting, a key came back degraded (e.g. a served degraded cache + // hit). Under --wait the CLI must fail rather than ship permissive types. + syncMetricViewsTypes.mockResolvedValueOnce({ + metricOutFile: path.join( + tmpRoot, + "shared", + "appkit-types", + "metric-views.d.ts", + ), + metricMetadataOutFile: path.join( + tmpRoot, + "shared", + "appkit-types", + "metric-views.metadata.json", + ), + schemas: [ + { + key: "revenue", + source: "demo.sales.revenue", + lane: "sp", + degraded: true, + }, + ], + failures: [], + fatalErrors: [], + noConfig: false, + }); + + const exitSpy = await runCliCapturingExit([ + "--warehouse-id", + "wh-123", + "--wait", + ]); + + expect(exitSpy).toHaveBeenCalledWith(1); + const errored = erroredText(); + expect(errored).toContain("could not be described even after waiting"); + expect(errored).toContain("revenue"); + }); + + test("--wait + fatal preflight (e.g. deleted warehouse): exit 1 with the reason", async () => { + writeConfig(); + // blocking mode reports a deleted/deleting warehouse via fatalErrors (the + // artifacts are still written); the CLI surfaces it and exits non-zero. + syncMetricViewsTypes.mockResolvedValueOnce({ + metricOutFile: path.join( + tmpRoot, + "shared", + "appkit-types", + "metric-views.d.ts", + ), + metricMetadataOutFile: path.join( + tmpRoot, + "shared", + "appkit-types", + "metric-views.metadata.json", + ), + schemas: [], + failures: [], + fatalErrors: [{ name: "wh-123", message: "Warehouse is DELETED" }], + noConfig: false, + }); + + const exitSpy = await runCliCapturingExit([ + "--warehouse-id", + "wh-123", + "--wait", + ]); + + expect(exitSpy).toHaveBeenCalledWith(1); + const errored = erroredText(); + expect(errored).toContain("warehouse preflight failed"); + expect(errored).toContain("Warehouse is DELETED"); + }); + // --- Phase 3: interactive flow ---------------------------------------------- test("no flags → interactive: prompts fire and resolved values reach syncMetricViewsTypes", async () => { @@ -624,6 +760,14 @@ describe("appkit mv sync", () => { ), cache: true, }); + + // #1: in interactive mode the spinner carries the success line; the plain + // "Generated metric types" console.log must NOT also fire (no double print). + expect(clackMocks.spinnerStop).toHaveBeenCalledWith( + expect.stringContaining("Generated metric types"), + ); + const logged = consoleLog.mock.calls.flat().map(String).join("\n"); + expect(logged).not.toContain("Generated metric types"); }); test("interactive: a non-blank config path / output dir answer is honored", async () => { @@ -665,7 +809,22 @@ describe("appkit mv sync", () => { ); }); - test("interactive cancel (first prompt): graceful cancel + non-zero exit, no appkit call", async () => { + test("interactive + no metric-views.json: outro matches the no-op (not a false 'synced')", async () => { + // No writeConfig() → the default config path is absent (dormant). + clackMocks.textAnswers = ["wh-interactive", "", ""]; + + await runCli([]); + + expect(syncMetricViewsTypes).not.toHaveBeenCalled(); + // A dormant run must not claim success — the outro reflects the no-op. + expect(clackMocks.outro).toHaveBeenCalledTimes(1); + expect(clackMocks.outro).toHaveBeenCalledWith("Nothing to sync."); + expect(clackMocks.outro).not.toHaveBeenCalledWith("Metric types synced."); + const logged = consoleLog.mock.calls.flat().map(String).join("\n"); + expect(logged).toContain("Nothing to sync"); + }); + + test("interactive cancel (first prompt): graceful cancel + exit 0, no appkit call", async () => { writeConfig(); // First prompt returns the cancel symbol. clackMocks.textAnswers = [clackMocks.CANCEL]; @@ -677,11 +836,12 @@ describe("appkit mv sync", () => { await runCli([]); expect(clackMocks.cancel).toHaveBeenCalledTimes(1); - expect(exitSpy).toHaveBeenCalledWith(1); + // Cancel is graceful: exit 0, like every other interactive command. + expect(exitSpy).toHaveBeenCalledWith(0); expect(syncMetricViewsTypes).not.toHaveBeenCalled(); }); - test("interactive cancel (later prompt): graceful cancel + non-zero exit", async () => { + test("interactive cancel (later prompt): graceful cancel + exit 0", async () => { writeConfig(); // First answer ok, second prompt cancels. clackMocks.textAnswers = ["wh-1", clackMocks.CANCEL]; @@ -693,7 +853,8 @@ describe("appkit mv sync", () => { await runCli([]); expect(clackMocks.cancel).toHaveBeenCalledTimes(1); - expect(exitSpy).toHaveBeenCalledWith(1); + // Cancel is graceful: exit 0, like every other interactive command. + expect(exitSpy).toHaveBeenCalledWith(0); expect(syncMetricViewsTypes).not.toHaveBeenCalled(); }); }); diff --git a/packages/shared/src/cli/commands/metric-views/sync/sync.ts b/packages/shared/src/cli/commands/metric-views/sync/sync.ts index 438bafeee..991da7610 100644 --- a/packages/shared/src/cli/commands/metric-views/sync/sync.ts +++ b/packages/shared/src/cli/commands/metric-views/sync/sync.ts @@ -9,18 +9,21 @@ import { } from "../validate-metric-views-source"; /** - * Options parsed by commander for `appkit mv sync`. + * Options parsed by commander for `appkit metric-views sync` (alias `mv`). * - * interactive clack flow: + * Flags: * - `--warehouse-id` (+ `DATABRICKS_WAREHOUSE_ID` fallback) * - `--metric-views-json-path` (canonical config path) * - `--output-dir` (artifact output directory; replaces Phase 1's `--out-dir`) + * - `--wait` (wait for the warehouse instead of emitting permissive types on a + * cold warehouse; exit non-zero if any view still can't be described) * - `--no-cache` (commander negation → `cache === false` disables the metric type-generation cache) */ export interface MetricViewsSyncOptions { warehouseId?: string; metricViewsJsonPath?: string; outputDir?: string; + wait?: boolean; cache?: boolean; } @@ -28,6 +31,14 @@ const METRIC_VIEWS_CONFIG_FILE = "metric-views.json"; const EXIT_FAILURE = 1; +/** + * What a sync run actually did, returned so the interactive caller can print an + * outro that matches the outcome (a dormant run must not claim "synced"). + * `undefined` means a hard-failure branch already called `process.exit` — it + * only "returns" under the test no-op exit spy. + */ +type SyncOutcome = "synced" | "degraded" | "noop"; + /** Resolved, absolute paths the sync run operates on. */ interface ResolvedPaths { /** Folder that holds metric-views.json (the appkit export reads from a folder). */ @@ -92,7 +103,7 @@ function resolvePaths(options: { async function runMetricViewsSync( options: MetricViewsSyncOptions, onProgress?: { start(): void; succeed(msg: string): void; fail(): void }, -): Promise { +): Promise { try { const { queryFolder, configPath, explicitConfigPath, outDir } = resolvePaths(options); @@ -124,7 +135,7 @@ async function runMetricViewsSync( console.log( `No ${METRIC_VIEWS_CONFIG_FILE} found at ${configPath}. Nothing to sync.`, ); - return; + return "noop"; } const rawConfig = fs.readFileSync(configPath, "utf-8"); @@ -175,6 +186,10 @@ async function runMetricViewsSync( metricMetadataOutFile, warehouseId, cache: options.cache, + // `--wait` → blocking preflight (wait for / start the warehouse). The + // default omits `mode` so appkit uses "describe-now" (DESCRIBE now, + // degrade on a cold warehouse) — and the default call shape is unchanged. + ...(options.wait ? { mode: "blocking" as const } : {}), }); } catch (err) { onProgress?.fail(); @@ -188,6 +203,21 @@ async function runMetricViewsSync( console.log( `No ${METRIC_VIEWS_CONFIG_FILE} found at ${configPath}. Nothing to sync.`, ); + return "noop"; + } + + // Fatal preflight errors only arise under `--wait` (blocking mode) — e.g. + // the warehouse is deleted/deleting. The artifacts are still written, but + // the run did not converge, so exit non-zero with the underlying reason. + if (result.fatalErrors.length > 0) { + onProgress?.fail(); + console.error( + "Error: could not sync metric views (warehouse preflight failed):", + ); + for (const fe of result.fatalErrors) { + console.error(` ${fe.name}: ${fe.message}`); + } + process.exit(EXIT_FAILURE); return; } @@ -217,26 +247,56 @@ async function runMetricViewsSync( .filter((schema) => schema.degraded) .map((schema) => schema.key); if (degradedKeys.length > 0) { + // `--wait` promised to wait for the warehouse and emit correct types, so a + // still-degraded result is a hard failure — never silently ship permissive + // types in CI. Without `--wait` (describe-now) the same state is a warning. + if (options.wait) { + onProgress?.fail(); + console.error( + `Error: ${degradedKeys.length} metric view(s) (${degradedKeys.join(", ")}) could not be described even after waiting for the warehouse. Check the warehouse state and the metric view definitions, then rerun.`, + ); + process.exit(EXIT_FAILURE); + return; + } onProgress?.succeed( `Generated permissive metric types: ${metricOutFile}`, ); console.warn( - `Warning: ${degradedKeys.length} metric view(s) (${degradedKeys.join(", ")}) could not be described — the warehouse wasn't ready, so permissive types were written. Rerun \`appkit mv sync\` once the warehouse is available.`, + `Warning: ${degradedKeys.length} metric view(s) (${degradedKeys.join(", ")}) could not be described — the warehouse wasn't ready, so permissive types were written. Rerun \`appkit metric-views sync\` once the warehouse is available.`, ); - console.log(`Generated metric metadata: ${metricMetadataOutFile}`); - return; + // In interactive mode the spinner's success line is the user-facing + // output; emit the plain metadata path only on the flag-driven path. + if (!onProgress) { + console.log(`Generated metric metadata: ${metricMetadataOutFile}`); + } + return "degraded"; } onProgress?.succeed(`Generated metric types: ${metricOutFile}`); - console.log(`Generated metric types: ${metricOutFile}`); - console.log(`Generated metric metadata: ${metricMetadataOutFile}`); + // Avoid a double print in interactive mode: `onProgress.succeed` already + // stopped the spinner with the "Generated metric types" line. The plain logs + // are the flag-driven path's only output, so gate them on its absence. + if (!onProgress) { + console.log(`Generated metric types: ${metricOutFile}`); + console.log(`Generated metric metadata: ${metricMetadataOutFile}`); + } + return "synced"; } catch (error) { + // Only a genuinely missing/unresolvable @databricks/appkit maps to the + // "not installed" guidance. The dynamic import throws a structured + // ERR_MODULE_NOT_FOUND naming the package when appkit is absent; match THAT + // rather than any message containing "Cannot find module". Otherwise a real + // failure from syncMetricViewsTypes — an unreachable warehouse, an auth + // error, a bad FQN — whose message happens to mention a module would be + // misreported here, sending the user to reinstall a package that is present. + const err = error as { code?: unknown; message?: unknown }; if ( - error instanceof Error && - error.message.includes("Cannot find module") + err?.code === "ERR_MODULE_NOT_FOUND" && + typeof err.message === "string" && + err.message.includes("@databricks/appkit") ) { console.error( - "Error: appkit mv sync is only available with @databricks/appkit installed.", + "Error: appkit metric-views sync is only available with @databricks/appkit installed.", ); console.error("Please install @databricks/appkit to use this command."); process.exit(EXIT_FAILURE); @@ -264,13 +324,15 @@ async function runMetricViewsSync( async function runInteractive(): Promise { intro("Sync UC Metric View types"); - // A cancelled prompt (Ctrl-C) is a graceful, non-zero exit. The explicit - // `return` after `process.exit` keeps control flow correct under a no-op exit - // (tests) — without it, a cancelled flow would fall through to the next - // prompt and eventually run the sync. + // A cancelled prompt (Ctrl-C) is a graceful exit (code 0) — matching + // `plugin create` / `add-resource` and clack's own convention, so a wrapper + // script that treats non-zero as failure doesn't misread a deliberate cancel. + // The explicit `return` after `process.exit` keeps control flow correct under + // a no-op exit (tests) — without it, a cancelled flow would fall through to + // the next prompt and eventually run the sync. const cancelled = (): never => { cancel("Cancelled."); - process.exit(1); + process.exit(0); }; const warehouseId = await text({ @@ -307,13 +369,22 @@ async function runInteractive(): Promise { }; const s = spinner(); - await runMetricViewsSync(options, { + const outcome = await runMetricViewsSync(options, { start: () => s.start("Describing metric views…"), succeed: (msg) => s.stop(msg), fail: () => s.stop("Failed."), }); - outro("Metric types synced."); + // Close with an outro that matches what actually happened — a dormant run + // logged "Nothing to sync", so a blanket "Metric types synced." would + // contradict it. `undefined` means a hard-failure branch already exited. + if (outcome === "noop") { + outro("Nothing to sync."); + } else if (outcome === "degraded") { + outro("Metric types synced with warnings."); + } else if (outcome === "synced") { + outro("Metric types synced."); + } } /** @@ -338,6 +409,7 @@ async function runMetricViewsSyncAction( "warehouseId", "metricViewsJsonPath", "outputDir", + "wait", "cache", ] as const; const hasUserFlag = FLAG_OPTION_NAMES.some( @@ -367,18 +439,25 @@ export const metricViewsSyncCommand = new Command("sync") "--output-dir ", "Output directory for metric-views.d.ts and metric-views.metadata.json (default: shared/appkit-types)", ) + .option( + "--wait", + "Wait for the SQL warehouse to start instead of emitting permissive types on a cold warehouse; exit non-zero if any metric view still can't be described", + ) .option("--no-cache", "Disable the metric type-generation cache") .addHelpText( "after", ` Run with no flags for an interactive prompt; pass any flag for non-interactive mode. +'mv' is a shorthand alias, so 'appkit mv sync' is equivalent to 'appkit metric-views sync'. Examples: - $ appkit mv sync - $ appkit mv sync --warehouse-id 1234abcd5678efgh - $ appkit mv sync --warehouse-id 1234abcd5678efgh --metric-views-json-path config/queries/metric-views.json - $ appkit mv sync --warehouse-id 1234abcd5678efgh --output-dir shared/appkit-types - $ appkit mv sync --warehouse-id 1234abcd5678efgh --no-cache + $ appkit metric-views sync + $ appkit metric-views sync --warehouse-id 1234abcd5678efgh + $ appkit metric-views sync --warehouse-id 1234abcd5678efgh --metric-views-json-path config/queries/metric-views.json + $ appkit metric-views sync --warehouse-id 1234abcd5678efgh --output-dir shared/appkit-types + $ appkit metric-views sync --warehouse-id 1234abcd5678efgh --wait + $ appkit metric-views sync --warehouse-id 1234abcd5678efgh --no-cache + $ appkit mv sync --warehouse-id 1234abcd5678efgh # 'mv' alias Environment variables: DATABRICKS_WAREHOUSE_ID SQL warehouse ID (used when --warehouse-id is omitted) diff --git a/packages/shared/src/cli/commands/type-generator.d.ts b/packages/shared/src/cli/commands/type-generator.d.ts index 23f80d534..8d6bf7a94 100644 --- a/packages/shared/src/cli/commands/type-generator.d.ts +++ b/packages/shared/src/cli/commands/type-generator.d.ts @@ -1,3 +1,20 @@ +/** + * Ambient, intentionally NARROWED mirror of `@databricks/appkit/type-generator`. + * + * `shared` must not statically depend on `appkit` (it is a leaf package), so the + * CLI reaches appkit's type-generator through a dynamic + * `import("@databricks/appkit/type-generator")`; this declaration types that + * import without a build-time dependency on appkit. + * + * The mirror is deliberately narrower than the real export: array element types + * are widened to `unknown[]`, options the CLI never passes (e.g. `metricFetcher`) + * are omitted, and only the surface the CLI actually uses is declared. + * + * DRIFT WARNING: there is NO compile-time link to appkit's real types — if the + * real `generateFromEntryPoint` / `syncMetricViewsTypes` (or their result + * shapes) change, this declaration will NOT fail to compile and must be + * re-synced by hand against `packages/appkit/src/type-generator/index.ts`. + */ declare module "@databricks/appkit/type-generator" { export function generateFromEntryPoint(options: { queryFolder?: string; @@ -76,6 +93,11 @@ declare module "@databricks/appkit/type-generator" { metricOutFile: string; metricMetadataOutFile: string; cache?: boolean; + // Preflight policy (mirrors the real export). The CLI omits it for the + // default `"describe-now"` (DESCRIBE now, degrade on a cold warehouse) and + // passes `"blocking"` under `--wait` (wait for / start the warehouse; only a + // deleted one is fatal). `"non-blocking"` is the dev/Vite path. + mode?: "describe-now" | "non-blocking" | "blocking"; }): Promise; export const METRIC_TYPES_FILE: string;