feat(appkit): relocate typegen cache to committed .appkit/ dir - #502
Conversation
Introduce getCommittedCacheDir() -> <cwd>/.appkit and getEphemeralStateDir() -> <cwd>/node_modules/.databricks/appkit as the single source of truth for typegen cache/state locations, co-located with spawn-lock.ts and re-exported from the shared root barrel. Rewire getSpawnLockPath to compose from getEphemeralStateDir (behavior-preserving, same resolved path). Foundation for relocating the typegen cache out of node_modules so it survives pnpm install on warehouse-less CI/CD. xavier loop: iteration 1 -- Phase 1 (resolvers) Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Move the query/metric cache (types-cache.json) and serving cache (serving-types-cache.json) out of node_modules/.databricks/appkit into the committed per-app .appkit/ directory via getCommittedCacheDir(), so they survive `pnpm install` and let a blocking CI/CD build succeed with no warehouse access. Flat filenames (the dir namespaces them). Both writers now sort top-level record keys for deterministic, merge-friendly diffs. The serving plugin's runtime read now resolves through the same exported getServingCachePath() the writer uses (reader == writer by construction, fixing the inline-path drift risk). Add queryCacheFileExists() for the blocking-mode bootstrap-vs-drift message (consumed next). Route the ephemeral ui-variant choices file through getEphemeralStateDir() (path unchanged; stays gitignored). Document .appkit/ as a committed artifact in the template. xavier loop: iteration 3 -- Phase 2 Wave 1 (2A, 2B+2C, 2D) + Phase 3A Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
… fatal
When blocking-mode typegen fails fatally (a cache MISS with no reachable/
authorized warehouse), branch the fatal remedy on whether the committed
.appkit/ cache file exists: bootstrap (never initialized → run
`generate-types --wait` against a warehouse and commit .appkit/) vs drift
(cache present but stale/missing the failing key → regenerate it). Same
crash, clearer next step. queryCacheFileExists() is called only on the fatal
path, so a warm all-hit run does no extra work. The write-then-throw contract
(always emit the .d.ts, then throw to fail the build) is unchanged.
Tests: all-hit blocking run makes ZERO warehouse round-trips (getWarehouse
never probed); both fatal message branches asserted.
Verify-at-implementation findings (no fix needed): (A) new WorkspaceClient({})
in query-registry constructs unconditionally but is lazy (no eager
auth/network) — all-hit runs make zero calls, confirmed by test; (B) SDK auth
errors (401/403) classify as fatal (not connectivity) via isConnectivityError,
so the authz cutover crashes the build loud as intended.
xavier loop: iteration 4 -- Phase 2 Wave 2 (2E)
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
…yground Fix a bootstrap-vs-drift misclassification found in live warehouse testing: generateFromEntryPoint read queryCacheFileExists() at the fatal throw site, but generateQueriesFromDescribe and syncMetricViewsTypes both persist the cache during the run — so the file always existed by then and the fatal message always said "drift", making the bootstrap branch dead code. Capture the existence snapshot at function entry (before any generation writes) and thread it to the throw. The mocked unit test passed while the bug was live because it stubbed the predicate directly; the test now ties the mock to the in-memory cache file so a regression (moving the check back) fails. Exclude .appkit/ from Biome: the committed cache is a generated artifact whose serialization saveCache owns, so the formatter must not govern it (a regen would otherwise trip lint). Seed apps/dev-playground/.appkit/ from a live blocking run against a warehouse (16 queries + 2 metrics + 1 serving endpoint). Verified end-to-end: a warm all-hit blocking re-run makes ZERO warehouse round-trips (0 new, 16 from cache, 0.00s) and the cache is byte-stable across a no-op regen (deterministic diff). xavier loop: iteration 5 -- Phase 3B integration + 2E ordering bugfix Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
📦 Bundle size reportCompared against
|
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 822 KB (+31 KB) | 287 KB (+11 KB) |
| Type declarations | 302 KB (+17 KB) | 103 KB (+5.9 KB) |
| Source maps | 1.6 MB (+70 KB) | 537 KB (+23 KB) |
| Other | 11 KB | 3.7 KB |
| Total | 2.7 MB (+118 KB) | 931 KB (+39 KB) |
Per-entry composition (own code — deps external (as shipped))
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
. |
86 KB (+322 B) | 2.5 KB | 89 KB (+322 B) | external | 282 KB (+1.0 KB) |
./beta |
44 KB (+4.6 KB) | 429 B (+198 B) | 45 KB (+4.8 KB) | external | 129 KB (+9.5 KB) |
./type-generator |
19 KB (+291 B) | 0 B | 19 KB (+291 B) | external | 55 KB (+1.0 KB) |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
. |
index.js |
initial | 82 KB |
. |
utils.js |
initial | 4.0 KB |
. |
remote-tunnel-manager.js |
lazy | 2.5 KB |
./beta |
beta.js |
initial | 29 KB |
./beta |
stream-manager.js |
initial | 5.8 KB |
./beta |
wide-event-emitter.js |
initial | 3.2 KB |
./beta |
databricks.js |
initial | 3.0 KB |
./beta |
configuration.js |
initial | 2.1 KB |
./beta |
service-context.js |
initial | 1.3 KB |
./beta |
client-options.js |
initial | 220 B |
./beta |
supervisor-api.js |
lazy | 184 B |
./beta |
databricks.js |
lazy | 132 B |
./beta |
index.js |
lazy | 113 B |
./type-generator |
index.js |
initial | 19 KB |
@databricks/appkit-ui
npm tarball (packed): 305 KB (+101 B) — gzipped download (dist + bin; excludes release-only docs/NOTICE).
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 360 KB (+390 B) | 119 KB (+60 B) |
| Type declarations | 205 KB | 74 KB |
| Source maps | 686 KB (+390 B) | 224 KB (+66 B) |
| CSS | 16 KB | 3.3 KB |
| Total | 1.2 MB (+780 B) | 422 KB (+126 B) |
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
./js |
4.3 KB | 49 KB | 54 KB | 208 KB | 12 KB |
./js/beta |
20 B | 0 B | 20 B | 0 B | 0 B |
./react |
429 KB (+55 B) | 49 KB | 478 KB (+55 B) | 1.3 MB | 168 KB (+390 B) |
./react/beta |
20 B | 0 B | 20 B | 0 B | 0 B |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
./js |
index.js |
initial | 4.2 KB |
./js |
chunk |
initial | 120 B |
./js |
apache-arrow |
lazy | 49 KB |
./js/beta |
beta.js |
initial | 20 B |
./react |
index.js |
initial | 427 KB |
./react |
tslib |
initial | 2.1 KB |
./react |
apache-arrow |
lazy | 49 KB |
./react/beta |
beta.js |
initial | 20 B |
…ared barrel CI surfaced two failures from routing the cache-path resolvers through the shared root barrel: 1. Docs Build (+ PR Template): the root barrel `shared/src/index.ts` is client-safe (docs webpack aliases shared->src and pulls it via appkit-ui), but cache-paths imports `node:path`, so the docs client bundle died with UnhandledSchemeError. Move the resolvers off the root barrel onto a Node-only subpath and import them there from appkit's cache/serving/ choice-sink modules. tsdown owns shared's generated `exports` (devExports), so the subpath is added as a tsdown entry and keyed by its src-relative path: `shared/cli/commands/cache-paths`. 2. Unit Tests: the new bootstrap-vs-drift message assertions matched raw text, but picocolors emits ANSI codes when CI is set, splitting the strings. Strip ANSI (incl. the ESC byte) before matching. Also repoint the cache serialization test's `vi.mock` to the new subpath specifier. Verified with the gates CI runs: `CI=true pnpm test` (3725 pass), `pnpm docs:build` (client compiles, no node: error), typecheck, build, check, knip. xavier loop: iteration 6 -- CI fixes (docs node: leak + ANSI-brittle tests) Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
🤖 AppKit PR bot🔬 Run evalsStart an eval for this PR from the evals-monitor app: Go to Evals Monitor → 📦 Try this PR's app templateScaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh run download 30639519621 -R databricks/appkit -n appkit-template-0.48.0-pr.c439f84-move-typegen-502 -D appkit-pr-502 \
&& unzip -o "appkit-pr-502/appkit-template-0.48.0-pr.c439f84-move-typegen-502.zip" -d "appkit-pr-502" \
&& databricks apps init --template "appkit-pr-502"The template pins |
The cache-path resolvers are an internal monorepo helper, not part of the shared package's public API. The previous commit exposed them as a "./cli/commands/cache-paths" export (a side effect of adding a tsdown entry, which makes the exports plugin generate the mapping) — leaking shared's internal file layout for no benefit. appkit resolves the import via its tsconfig `paths` alias and bundles shared inline (noExternal), so the export is not load-bearing: dropping the tsdown entry (and thus the generated export) keeps typecheck, appkit build, vitest, and docs:build all green. shared stays private; cache-paths stays internal. xavier loop: iteration 7 -- keep cache-paths internal Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Reverts the previous "keep cache-paths internal" change, which broke the Playground Integration Tests with ERR_PACKAGE_PATH_NOT_EXPORTED. The export IS load-bearing on a path the earlier verification missed: dev-playground runs its server via `tsx` under the `development` export condition, so @databricks/appkit resolves to SOURCE, whose `import "shared/cli/commands/cache-paths"` is a bare cross-package specifier that tsx resolves through shared's package.json exports map at runtime — tsconfig `paths` do not apply there. Without the export the server never boots and Playwright times out. Re-add the tsdown entry (which regenerates the export) with a comment explaining the runtime dependency so it isn't dropped again. The export is monorepo-internal only: shared is private and appkit bundles it inline (noExternal), so it never reaches published-appkit consumers. Verified by booting the dev-playground server under the development condition (starts clean, no export error). xavier loop: iteration 8 -- restore runtime-required cache-paths export Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
…ration Tests Expand the tsdown entry comment to explicitly warn that removing the cache-paths export breaks the "Playground Integration Tests" CI job (the dev-playground server fails to boot with ERR_PACKAGE_PATH_NOT_EXPORTED and Playwright times out), and that build/typecheck/unit tests do NOT catch it. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
…tion Route the ui-variants choices JSONL path through getEphemeralStateDir() without the incidental churn: keep the single UI_CHOICES_FILE constant and the original constructor signature, and restore the main docstrings (with only the accuracy fix the now-absolute path requires). Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
There was a problem hiding this comment.
Pull request overview
Relocates AppKit type generation caches from an ephemeral node_modules/.databricks/appkit/ location into a committed, per-app .appkit/ directory so CI/CD can build with warm caches even when warehouse access is unavailable. This also centralizes cache path resolution and makes cache JSON serialization deterministic to reduce diff/merge noise.
Changes:
- Introduces shared cache-path resolvers and updates typegen/serving/runtime code to use committed
.appkit/caches (while keeping ephemeral coordination state undernode_modules/.databricks/appkit/). - Makes cache output deterministic by sorting top-level cache keys (queries/metrics/endpoints) and adds tests around cache behavior and “all-hit” warehouse avoidance.
- Updates tooling/docs/config: tsconfig path aliases, Biome includes exclusions, template README note, and commits dev-playground caches.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds TS path alias for shared/cli/commands/cache-paths. |
| template/README.md | Documents .appkit/ as a committed type-cache directory. |
| packages/shared/tsdown.config.ts | Adds cache-paths.ts as a dedicated build entry/export for runtime resolution. |
| packages/shared/src/cli/commands/spawn-lock.ts | Moves spawn-lock location to getEphemeralStateDir() (still under node_modules/.databricks/appkit/). |
| packages/shared/src/cli/commands/cache-paths.ts | Adds committed vs ephemeral directory resolvers (.appkit/ vs node_modules/.databricks/appkit/). |
| packages/shared/src/cli/commands/cache-paths.test.ts | Tests the new cache path resolvers and defaults. |
| packages/shared/package.json | Exports ./cli/commands/cache-paths for dev/runtime consumers. |
| packages/appkit/tsconfig.json | Adds TS path alias for shared/cli/commands/cache-paths. |
| packages/appkit/src/type-generator/tests/index.test.ts | Adds tests for bootstrap vs drift fatal messaging and mocks cache existence. |
| packages/appkit/src/type-generator/tests/generate-queries.test.ts | Adds “all cache hit → zero warehouse calls” blocking-mode test. |
| packages/appkit/src/type-generator/tests/cache-serialization.test.ts | Verifies deterministic (sorted) cache serialization and cache-file existence checks. |
| packages/appkit/src/type-generator/serving/tests/cache.test.ts | Verifies serving cache serialization sorts endpoints and uses new filename. |
| packages/appkit/src/type-generator/serving/cache.ts | Moves serving cache path into committed .appkit/ and adds deterministic endpoint sorting + shared path getter. |
| packages/appkit/src/type-generator/index.ts | Adds cache-existence snapshotting to improve fatal messaging for bootstrap vs drift. |
| packages/appkit/src/type-generator/cache.ts | Moves query/metric cache to committed .appkit/types-cache.json, adds deterministic serialization and queryCacheFileExists(). |
| packages/appkit/src/plugins/ui-variants/choice-sink.ts | Uses shared getEphemeralStateDir() for the JSONL UI-choices file path. |
| packages/appkit/src/plugins/serving/serving.ts | Uses shared getServingCachePath() to load serving schema allowlists. |
| biome.json | Excludes .appkit from Biome’s configured includes. |
| apps/dev-playground/.appkit/types-cache.json | Commits the dev-playground query/metric types cache into .appkit/. |
| apps/dev-playground/.appkit/serving-types-cache.json | Commits the dev-playground serving types cache into .appkit/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Make TypegenFatalError's cacheInitialized snapshot optional. A cache-unrelated fatal (e.g. a malformed definitions.json) now omits it and gets generic "Fix the FATAL error(s) above, then re-run generate-types" wording, instead of the bootstrap/drift message telling the operator to regenerate and commit .appkit/ (which only applies to failures tied to the type cache). Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
pkosiec
left a comment
There was a problem hiding this comment.
LGTM - but please see the comments before merge. Thanks!
* docs: document committed .appkit/ type cache and warehouse-less deploys
Reflect the typegen cache relocation in the docs:
- type-generation.md: add a "Type cache" section (cache now lives in the
committed .appkit/ dir, not node_modules; what to commit; deterministic
serialization) and a "Deploying without warehouse access" subsection
covering the rollout ordering (seed + commit .appkit/ while the warehouse is
reachable, then cut access) and the bootstrap-vs-drift failure messages.
Update the --wait section to describe hit-skips-warehouse / miss-attempts /
unresolved-miss-fails semantics.
- project-setup.md: add .appkit/ to the canonical layout as a committed
artifact (do not gitignore).
- app-management.mdx: deploy-guide admonition that a warehouse-less deploy
needs a committed .appkit/ covering every query.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* docs: match typegen cache-failure labels to actual CLI messages
Align the warehouse-less deploy failure bullets with the strings the
build actually prints ('No committed type cache found' vs 'The committed
.appkit/ cache is missing or stale') so they are greppable in CI logs,
and broaden the drift case to cover added/missing queries, not just
changed ones.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
---------
Signed-off-by: Atila Fassina <atila@fassina.eu>
What & why
AppKit's analytics typegen depends on a per-project cache of
DESCRIBEresults. Today it lives undernode_modules/.databricks/appkit/, so everypnpm installdestroys it and git can't see it. An upstream authorization change is coming that will leave CI/CD without warehouse access — at which point a cache wiped byinstallmeans every deploy is an all-MISS build with no warehouse to satisfy it, and every deploy crashes.This relocates the two build-input caches to a committed, per-app
.appkit/directory so a freshgit checkout+pnpm installon CI arrives with a warm cache. The committed cache acts as a types-lockfile: an all-hit blocking build needs no warehouse, and a genuine miss that can't be resolved fails loud instead of shipping degraded (result: unknown) types.The blocking/non-blocking typegen architecture is unchanged — blocking mode already provides HIT-skip / MISS-attempt / unresolved-MISS-fatal. The only thing defeating it was the cache location.
Changes
git diff)node_modules/.databricksto.appkit/.appkit/**on Biome linterNote
Also check #503 for the docs