Skip to content

chore: CLI typegen for Metric Views#459

Open
atilafassina wants to merge 6 commits into
mainfrom
mv-cli
Open

chore: CLI typegen for Metric Views#459
atilafassina wants to merge 6 commits into
mainfrom
mv-cli

Conversation

@atilafassina

@atilafassina atilafassina commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

appkit mv sync CLI that runs the metric-view schema-fetch + type-emission outside the Vite dev loop — for CI, non-Vite builds, and manual refresh after pulling a teammate's metric-views.json.

Note

Dormant feature, still not user facing.

Design

The CLI lives in shared and reaches appkit's metric-sync core via dynamic import("@databricks/appkit/type-generator") — the same pattern generate-types uses — backed by an ambient declaration and a graceful appkit-absent fallback, so shared keeps no static dependency on appkit.

A new appkit syncMetricTypes export runs readMetricConfig → resolveMetricConfig → syncMetrics → write metric.d.ts + metrics.metadata.json, reusing the existing metric writers, the adaptive DESCRIBE fetcher, and the post-#433 persistent cache helpers (loadCache / saveCache / metricCacheHash). Because it is the same code path, the emitted bundle is structurally identical to the Vite plugin output. It deliberately does not generate query types or route through generateFromEntryPoint.

Behaviour

  • Validation against metricSourceSchema (post-chore: Metric Views typegen #433 UC FQN grammar) before any sync, with issues formatted as path: message.
  • Exit-code taxonomy — an absent default config/queries/metric-views.json exits 0 (dormancy invariant); explicit-missing-path / JSON-parse / schema-validation / missing-FQN / unreachable-warehouse / auth each exit non-zero with a distinct message.
  • Interactive vs non-interactive — no flags → @clack/prompts flow (mirrors plugin create); any flag → non-interactive (detected via getOptionValueSource() === "cli").
  • Flags--warehouse-id (+ DATABRICKS_WAREHOUSE_ID fallback), --metric-views-json-path, --output-dir, --no-cache (→ cache: false: ignore + overwrite the metric cache).

@github-actions

Copy link
Copy Markdown
Contributor

🔬  Run evals on this PR  ·  Go to Evals Monitor →

@atilafassina atilafassina force-pushed the mv-cli branch 4 times, most recently from b0ca822 to bef7429 Compare June 23, 2026 14:05
@atilafassina atilafassina changed the title feat(shared): appkit metric sync CLI chore: CLI typegen for Metric Views Jun 25, 2026
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 <atila@fassina.eu>
…Path

"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 <atila@fassina.eu>
`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 <atila@fassina.eu>
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 <atila@fassina.eu>
@atilafassina atilafassina marked this pull request as ready for review June 25, 2026 11:55
@atilafassina atilafassina requested a review from a team as a code owner June 25, 2026 11:55
@atilafassina atilafassina requested review from Copilot and pkosiec June 25, 2026 11:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new appkit mv sync CLI surface (in packages/shared) that can generate Metric View type artifacts outside the Vite dev loop, backed by a new/expanded syncMetricViewsTypes export in @databricks/appkit/type-generator and a refactor to reuse the same unified metric sync pipeline from both the CLI and generateFromEntryPoint.

Changes:

  • Add appkit mv sync command with interactive/non-interactive flows, schema validation, dormancy behavior, and cache controls.
  • Introduce/extend syncMetricViewsTypes in the appkit type-generator and refactor generateFromEntryPoint to delegate Metric View emission to it.
  • Standardize Metric View artifact filenames to metric-views.d.ts and metric-views.metadata.json, updating tests accordingly.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/shared/src/cli/index.ts Wires the new mv parent command into the shared CLI.
packages/shared/src/cli/commands/type-generator.d.ts Extends the ambient optional @databricks/appkit/type-generator declaration with syncMetricViewsTypes + metric artifact constants.
packages/shared/src/cli/commands/metric-views/index.ts Adds the mv parent command and attaches the sync subcommand.
packages/shared/src/cli/commands/metric-views/validate-metric-views-source.ts Adds Zod-based validation and CLI-friendly formatting for metric-views.json.
packages/shared/src/cli/commands/metric-views/validate-metric-views-source.test.ts Unit tests for metric source validation + formatting.
packages/shared/src/cli/commands/metric-views/sync/sync.ts Implements appkit mv sync (path resolution, dormancy, dynamic import, error taxonomy, interactive prompts).
packages/shared/src/cli/commands/metric-views/sync/sync.test.ts Tests CLI behavior (flag vs interactive, error taxonomy, cache flag propagation).
packages/appkit/tsdown.config.ts Ensures ./type-generator is built as its own entry so dynamic-import consumers see required exports.
packages/appkit/src/type-generator/index.ts Refactors metric emission to a unified syncMetricViewsTypes pipeline and updates exported metric artifact constants.
packages/appkit/src/type-generator/mv-registry/render-types.ts Updates comments to reflect the new artifact filename.
packages/appkit/src/type-generator/tests/vite-plugin.test.ts Updates expected default/custom Metric View artifact filenames.
packages/appkit/src/type-generator/tests/index.test.ts Updates expected Metric View artifact filenames in type-generator tests.
packages/appkit/src/type-generator/tests/mv-registry.test.ts Updates metadata bundle naming references in tests.
packages/appkit/src/type-generator/tests/sync-metric-views-types.test.ts Adds focused unit tests for the new syncMetricViewsTypes export, including cache behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/shared/src/cli/commands/metric-views/sync/sync.ts Outdated
Comment thread packages/shared/src/cli/commands/metric-views/sync/sync.ts
Comment thread packages/appkit/src/type-generator/index.ts
Comment thread packages/shared/src/cli/commands/metric-views/index.ts Outdated
Comment thread packages/shared/src/cli/commands/metric-views/sync/sync.ts

@pkosiec pkosiec left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the mv-cli typegen changes — solid refactor, and behavior parity for the dev/Vite (non-blocking/blocking) paths reads as preserved 1:1 (the extracted syncMetricViewsTypes keeps serveDegraded = mode !== "describe-now", the preflight, the #406 gate, the sticky-degraded notice, cache save/prune, and the fatalErrors→throw ordering). Findings inline below — mostly P3 polish; nothing blocks correctness of the extraction. Verified the --no-cache / getOptionValueSource handling and the @clack/prompts usage against their docs — both correct.

Two nits not worth their own threads:

  • Stale metric.d.ts / metrics.metadata.json aren't cleaned up on the rename to metric-views.* (removeOldGeneratedTypes only removes appKitTypes.d.ts) — low impact for a pre-release feature, but an upgraded app could keep an orphaned metric.d.ts.
  • The CLI's Zod pre-validation doesn't mirror resolveMetricConfig's 200-entry / 255-char-segment caps, so an over-cap config passes the "fail fast before the dynamic import" stage and throws only after it (still exit 1, just later).

Verdict: ready with minor fixes.

Comment thread packages/shared/src/cli/commands/metric-views/sync/sync.ts
Comment thread packages/shared/src/cli/commands/metric-views/sync/sync.ts Outdated
Comment thread packages/shared/src/cli/commands/type-generator.d.ts
Comment thread packages/shared/src/cli/commands/metric-views/sync/sync.ts Outdated
Comment thread packages/shared/src/cli/commands/metric-views/sync/sync.ts Outdated
Comment thread packages/shared/src/cli/commands/metric-views/sync/sync.ts Outdated
Comment thread packages/shared/src/cli/commands/metric-views/index.ts Outdated
Comment thread packages/shared/src/cli/commands/metric-views/sync/sync.ts
- 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 <atila@fassina.eu>
@atilafassina atilafassina enabled auto-merge (squash) June 30, 2026 10:21
@atilafassina atilafassina disabled auto-merge June 30, 2026 11:40
* 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should move the cli to the appkit package?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants