feat: CI-resilient --wait typegen with committed-types fallback - #510
Merged
Conversation
Phase 1 of typegen-ci-resilient-describe. In blocking (`--wait`) mode the type generator now suppresses the `.d.ts` write on ANY degraded result (query `result: unknown` or degraded metric), leaving the committed types untouched as the CI fallback of record, then throws as before. The prior path wrote degraded (`unknown`) declarations first and threw after, which clobbered good committed types on a fresh CI checkout — including via the auth/timeout/bad-id/DELETED fatal-degrade path. Non-blocking mode is unchanged (still writes degraded types for the detached worker to refresh). Tests inverted to assert no-write-on-degrade while preserving throw + behavioral assertions; adds coverage for the query-side fatal-degrade clobber-prevention case. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Phase 2 of typegen-ci-resilient-describe. Adds a pure classifyBlockingFailure(error) → "deterministic" | "environmental" to type-generator/errors.ts, building on the existing getErrorStatus and isConnectivityError helpers. Deterministic (build must crash regardless of committed types): HTTP 404 (bad warehouse id) and 400 (malformed request), checked first and walked through cause/AggregateError chains. Environmental (has-types gate applies later): 401/403 auth, connectivity, DELETED/DELETING, wait-timeout, and any unrecognized failure (the default). The auth status set is a one-line change point for the auth-owning team. No behavior change to isConnectivityError. Adds tests/errors.test.ts. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
…wait Phase 3 of typegen-ci-resilient-describe (join point). Wires the classifyBlockingFailure taxonomy into the reordered blocking write path so `--wait` degrades gracefully on environmental failures instead of always crashing: - Deterministic failures (SQL syntax, HTTP 404/400) still crash the build. - Environmental failures (401/403 auth, connectivity, DELETED/DELETING, wait-timeout, unrecognized) now flow through a has-types gate: if committed analytics/metric-views .d.ts exist, skip the (already-suppressed) write, emit one loud greppable stderr warning naming the coarse cause (auth blocked / warehouse unreachable / warehouse unavailable) + warehouse id, and exit 0 using the committed types as the fallback of record. If no committed types exist, crash with a generic 'run generate-types --wait locally' remedy. Serving types are excluded from the gate (gitignored, degrade independently). Non-blocking mode is unchanged. Threads deterministic-vs-environmental and a coarse cause label out of the query + metric preflights. Adds gate-matrix coverage: environmental+present (per cause) → warning+exit0, environmental+ absent → crash, deterministic (404/400/syntax) → crash regardless of types, partial presence, serving-exclusion, and CI-safe (ANSI-free) warning output. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Phase 4 of typegen-ci-resilient-describe. Updates the type-generation docs to describe the committed-types fallback and two-bucket failure taxonomy for blocking (`--wait`) builds: committed .d.ts as the fallback of record, --wait never overwriting good types with degraded ones, deterministic failures (SQL syntax / 404 / 400) crashing vs. environmental failures (auth / connectivity / deleted / timeout) gating on committed-type presence, the loud stderr warning, and the run-locally remedy for a first build with no committed types. Notes the metric-views-only edge case (empty analytics.d.ts satisfies the gate). Refreshes the metric-view section to reference the same taxonomy instead of the old always-fail framing. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
…anges
Wash of the typegen-ci-resilient-describe branch: strips loop-process
"Phase N" labels from comments, test names, and describe titles (keeping
the semantic text), removes an unnecessary comment / empty else-block /
useless default parameter, rewrites two stale+duplicated write-suppression
comments to match the actual behavior, and converts errors.test.ts's
`(error as any)` casts to the sibling `Object.assign(new Error(...), { ... })`
idiom. Comments, names, and test-setup style only — no logic or assertion
changes (537 tests still pass).
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
atilafassina
changed the title
--wait typegen with committed-types fallback
Contributor
📦 Bundle size reportCompared against
|
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 832 KB (+7.8 KB) | 290 KB (+1.8 KB) |
| Type declarations | 306 KB | 105 KB |
| Source maps | 1.6 MB (+19 KB) | 544 KB (+4.7 KB) |
| Other | 11 KB | 3.7 KB |
| Total | 2.7 MB (+26 KB) | 943 KB (+6.5 KB) |
Per-entry composition (own code — deps external (as shipped))
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
. |
87 KB (+652 B) | 2.5 KB | 89 KB (+652 B) | external | 284 KB (+2.3 KB) |
./beta |
45 KB | 458 B | 45 KB | external | 129 KB |
./type-generator |
20 KB (+673 B) | 0 B | 20 KB (+673 B) | external | 57 KB (+2.3 KB) |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
. |
index.js |
initial | 83 KB |
. |
utils.js |
initial | 4.0 KB |
. |
remote-tunnel-manager.js |
lazy | 2.5 KB |
./beta |
beta.js |
initial | 29 KB |
./beta |
stream-manager.js |
initial | 5.8 KB |
./beta |
wide-event-emitter.js |
initial | 3.2 KB |
./beta |
databricks.js |
initial | 3.0 KB |
./beta |
configuration.js |
initial | 2.1 KB |
./beta |
service-context.js |
initial | 1.3 KB |
./beta |
client.js |
initial | 431 B |
./beta |
client-options.js |
initial | 220 B |
./beta |
supervisor-api.js |
lazy | 193 B |
./beta |
databricks.js |
lazy | 142 B |
./beta |
index.js |
lazy | 123 B |
./type-generator |
index.js |
initial | 20 KB |
@databricks/appkit-ui
npm tarball (packed): 305 KB — gzipped download (dist + bin; excludes release-only docs/NOTICE).
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 360 KB | 119 KB |
| Type declarations | 205 KB | 74 KB |
| Source maps | 686 KB | 224 KB |
| CSS | 16 KB | 3.3 KB |
| Total | 1.2 MB | 422 KB |
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
./js |
4.3 KB | 49 KB | 54 KB | 208 KB | 12 KB |
./js/beta |
20 B | 0 B | 20 B | 0 B | 0 B |
./react |
429 KB | 49 KB | 478 KB | 1.3 MB | 168 KB |
./react/beta |
20 B | 0 B | 20 B | 0 B | 0 B |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
./js |
index.js |
initial | 4.2 KB |
./js |
chunk |
initial | 120 B |
./js |
apache-arrow |
lazy | 49 KB |
./js/beta |
beta.js |
initial | 20 B |
./react |
index.js |
initial | 427 KB |
./react |
tslib |
initial | 2.1 KB |
./react |
apache-arrow |
lazy | 49 KB |
./react/beta |
beta.js |
initial | 20 B |
Contributor
🤖 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 30919671743 -R databricks/appkit -n appkit-template-0.52.0-pr.9b3eb86-fix-shared-subpath-bundling-510 -D appkit-pr-510 \
&& unzip -o "appkit-pr-510/appkit-template-0.52.0-pr.9b3eb86-fix-shared-subpath-bundling-510.zip" -d "appkit-pr-510" \
&& databricks apps init --template "appkit-pr-510"The template pins |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens AppKit’s type-generator blocking (--wait) path for CI/CD by preventing degraded output from overwriting committed .d.ts artifacts, and by introducing a deterministic-vs-environmental failure taxonomy so CI can fall back to committed types (with a loud warning) on transient/environmental issues.
Changes:
- Add anti-clobber write suppression in blocking mode to preserve committed
analytics.d.ts/metric-views.d.tswhen generation degrades. - Introduce
classifyBlockingFailureand plumb “environmental failure” signals into the entrypoint so it can decide warn+fallback vs crash. - Expand/adjust test coverage for the new gating and warning behavior, and update docs to describe CI-resilient
--wait.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/appkit/src/type-generator/types.ts | Extends result types to carry environmental-failure signals used by the blocking-mode gate. |
| packages/appkit/src/type-generator/index.ts | Implements blocking-mode anti-clobber, has-types gating, and warning-message generation. |
| packages/appkit/src/type-generator/query-registry.ts | Updates query preflight/failure handling to separate deterministic fatals from environmental degradation and track it for the gate. |
| packages/appkit/src/type-generator/errors.ts | Adds classifyBlockingFailure to bucket errors into deterministic vs environmental. |
| packages/appkit/src/type-generator/tests/index.test.ts | Adds extensive integration-style tests for write suppression, warning output, and the has-types gate. |
| packages/appkit/src/type-generator/tests/generate-queries.test.ts | Updates query-registry tests to align with environmental degradation + gate signaling. |
| packages/appkit/src/type-generator/tests/errors.test.ts | Adds unit tests for classifyBlockingFailure. |
| docs/docs/development/type-generation.md | Documents CI-resilient --wait semantics, including warning behavior and the two-bucket taxonomy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…use labels `classifyEnvironmentalCause` read only `err.status`/`err.statusCode` on the top-level error, while `errors.ts` already resolved `response.status` and walked `cause`/`AggregateError` chains. A 403 reported under `response` or wrapped in a cause chain was therefore labeled "warehouse unavailable" instead of "auth blocked", pointing CI at the wrong remedy. Move the helper next to `classifyBlockingFailure` in errors.ts so both classifiers share one status-extraction path, and reuse the existing chain walk. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
…typegen The blocking-mode preflight set `decision = "degradeAll"` for connectivity failures but left `isEnvironmental` false, so `hadEnvironmentalFailure` never flipped and the has-types gate never ran for an unreachable warehouse. On a fresh CI checkout that meant: queries degrade to `result: unknown`, write suppression skips `analytics.d.ts` entirely, no fatal errors are recorded, and the run exits 0 having written no types — the build then fails later somewhere less legible. Flag connectivity failures (preflight and per-query DESCRIBE) as environmental so the gate decides: warn and fall back when committed types exist, crash with the run-locally remedy when they don't. This cannot turn a passing build red — with committed types the outcome is unchanged apart from the warning now being emitted. Also return `environmentalCause` from `generateQueriesFromDescribe`, which `QueryGenerationResult` already declared and the metric path already set. Without it every query-path environmental failure fell back to the default "warehouse unavailable" label, and the "warehouse unreachable" label was unreachable in practice. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
…utation Two comments claimed blocking mode suppresses writes only for pure degradation and that degraded artifacts are still written before a throw. `suppressDegradedWrite: mode === "blocking"` is unconditional, so any degradation suppresses the write including on runs that then throw. The behavior is what the PR intends; the comments described the old shape. The ANSI-free warning test set `process.env.CI` and deleted it in `finally`, clobbering a pre-existing value for later tests. Nothing under `src/type-generator` reads `CI`, so the assignment never affected the assertion — it only risked perturbing third-party color detection, which is exactly what this test checks. Drop it rather than stub it. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
…uery path The gate tests in index.test.ts mock `generateQueriesFromDescribe`, so they supply the `hadEnvironmentalFailure: true` they assert on. They pass whether or not the query path actually reports it — which is how the connectivity bug survived review: the preflight returned false and no test joined the two halves. Mock only the SDK boundary so the real query path classifies the failure and the real gate decides. Covers the fresh-checkout crash (previously exit 0 with no types written), the committed-types warn-and-preserve path with the "warehouse unreachable" label, and non-blocking staying silent. Verified these fail when the isEnvironmental assignment is reverted. Co-authored-by: Isaac
…rehouse gate The new gate test replaced `@databricks/sdk-experimental` with a bare factory mock. After #475 the type-generator reaches the SDK through `../../workspace-client`, whose `legacy.ts` destructures `ConfigError`, `Context` and `TimeUnits` off that module at import time — so the factory-only mock starved module init and the suite failed to collect. Mock the wrapper instead, spreading `importOriginal` so the re-exported SDK values survive. This matches the sibling type-generator tests and keeps the test's intent: the wrapper is now the client boundary, so the real query path still classifies the failure and the real gate still decides. Verified by re-injecting the original bug (`isEnvironmental = false` on the connectivity branch) and confirming both gate tests fail. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
MarioCadenas
approved these changes
Aug 4, 2026
calvarjorge
reviewed
Aug 4, 2026
calvarjorge
left a comment
calvarjorge
left a comment
Contributor
There was a problem hiding this comment.
If I add new analytics query but don't commit the types, can this cause the build to succeed when the SQL warehouse is not available? Even if no types are there? In this scenario, we might want to fail, given that types are clearly missing
Collaborator
either the build will fail or the app will just normally run, but this would be a strange scenario because this would be caught on deploy time |
Signed-off-by: Atila Fassina <atila@fassina.eu>
Six comment-only removals in the has-types gate: an orphaned section header that summarised the two decision comments immediately below it, two docblocks restating single-expression predicates (`isQueryDegraded`, `hasAnyDegradedMetrics`), two trailing comments restating their own assignment, and a redundant `@param warehouseId` plus a sentence duplicating the `@param cause` list. No behaviour change. The load-bearing comments stay: the two-bucket taxonomy contracts in errors.ts, the write-suppression invariant note, and the gate test's account of how the original bug evaded weaker coverage. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Makes the blocking (
--wait) type-generation path resilient for CI/CD. On a fresh CI checkout, the committed.d.tsfiles become the fallback of record:--waitnever overwrites good committed types with degraded (result: unknown) ones, and a two-bucket failure taxonomy decides crash-vs-fall-back. The non-blocking dev/install path is untouched.Behavior
Blocking (
--wait) mode:generate-types --waitlocally and commit the.d.ts" remedy.serving.d.ts) are excluded from the gate (gitignored, degrade independently).Non-blocking mode: unchanged — still writes best-effort/degraded types immediately and refreshes via the detached worker.
Commits (one per phase)
fix(appkit)— anti-clobber:--waitnever overwrites committed types on any degradefeat(appkit)—classifyBlockingFailuretwo-bucket taxonomy (pure, inerrors.ts)feat(appkit)— has-types gate + loud warning + CLI terminal wiring (the join)docs— document the CI-resilient--waitbehaviorchore— remove implementation-phase narration and slopTesting
pnpm test— 3763 passed / 1 skipped / 0 failed (type-generator suite: 537 across 19 files, incl. newerrors.test.ts+ gate-matrix coverage: environmental+present→warn, environmental+absent→crash, deterministic→crash, serving-exclusion, partial-presence, CI-safe warning).pnpm -r typecheck,pnpm check(0 errors),pnpm build,pnpm docs:build— all green.--conditions=developmentwith noERR_PACKAGE_PATH_NOT_EXPORTED; no survivingshared/<subpath>bare imports.Notes
config/queries/) writes an emptyanalytics.d.tson a fresh run, which satisfies the has-types gate — so an environmental failure falls back + warns rather than crashing even on a true first build. Documented in the type-generation docs..d.tsoutput is the fallback).This pull request and its description were written by Isaac.