Skip to content

fix: gate typegen fallback per surface, not across surfaces - #511

Merged
atilafassina merged 1 commit into
mainfrom
fix/typegen-per-surface-gate
Aug 5, 2026
Merged

fix: gate typegen fallback per surface, not across surfaces#511
atilafassina merged 1 commit into
mainfrom
fix/typegen-per-surface-gate

Conversation

@atilafassina

Copy link
Copy Markdown
Contributor

The --wait committed-types fallback tracked a single hadEnvironmentalFailure flag for both analytics queries and metric views, and satisfied the gate if either committed artifact existed. When only one surface failed, an unrelated committed artifact could stand in for the missing one: queries failing environmentally with no committed analytics.d.ts still exited 0 as long as a metric-views.d.ts happened to be present, leaving CI green with types that were never generated.

Track the failure per surface and require each failing surface to have its own committed artifact. Metric-view degradation now participates in the gate the same way degraded queries already did, and the fatal error names the missing artifacts so the remedy points at the right surface.

The `--wait` committed-types fallback tracked a single
`hadEnvironmentalFailure` flag for both analytics queries and metric
views, and satisfied the gate if *either* committed artifact existed.
When only one surface failed, an unrelated committed artifact could
stand in for the missing one: queries failing environmentally with no
committed `analytics.d.ts` still exited 0 as long as a
`metric-views.d.ts` happened to be present, leaving CI green with types
that were never generated.
Track the failure per surface and require each failing surface to have
its own committed artifact. Metric-view degradation now participates in
the gate the same way degraded queries already did, and the fatal error
names the missing artifacts so the remedy points at the right surface.
Signed-off-by: Atila Fassina <atila@fassina.eu>
CopilotAI review requested due to automatic review settings August 4, 2026 15:12
@atilafassina
atilafassina requested a review from a team as a code ownerAugust 4, 2026 15:12
@github-actions

Copy link
Copy Markdown
Contributor

📦 Bundle size report

Compared against bundle-size-baseline.json (main).

@databricks/appkit

npm tarball (packed): 809 KB (+6.5 KB) — gzipped download (dist + bin; excludes release-only docs/NOTICE).

distrawgzip
JS (runtime)832 KB (+7.8 KB)290 KB (+1.8 KB)
Type declarations306 KB105 KB (-9 B)
Source maps1.6 MB (+18 KB)544 KB (+4.7 KB)
Other11 KB3.7 KB
Total2.7 MB (+26 KB)943 KB (+6.4 KB)
Per-entry composition (own code — deps external (as shipped))
EntryInitial (gz)Lazy (gz)Total (gz)node_modules (min)Own code (min)
.87 KB (+702 B)2.5 KB89 KB (+702 B)external284 KB (+2.4 KB)
./beta45 KB458 B45 KBexternal129 KB
./type-generator20 KB (+723 B)0 B20 KB (+723 B)external57 KB (+2.4 KB)

Chunks:

EntryChunkLoadSize (gz)
.index.jsinitial83 KB
.utils.jsinitial4.0 KB
.remote-tunnel-manager.jslazy2.5 KB
./betabeta.jsinitial29 KB
./betastream-manager.jsinitial5.8 KB
./betawide-event-emitter.jsinitial3.2 KB
./betadatabricks.jsinitial3.0 KB
./betaconfiguration.jsinitial2.1 KB
./betaservice-context.jsinitial1.3 KB
./betaclient.jsinitial431 B
./betaclient-options.jsinitial220 B
./betasupervisor-api.jslazy193 B
./betadatabricks.jslazy142 B
./betaindex.jslazy123 B
./type-generatorindex.jsinitial20 KB

@databricks/appkit-ui

npm tarball (packed): 305 KB — gzipped download (dist + bin; excludes release-only docs/NOTICE).

distrawgzip
JS (runtime)360 KB119 KB
Type declarations205 KB74 KB
Source maps686 KB224 KB
CSS16 KB3.3 KB
Total1.2 MB422 KB
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
EntryInitial (gz)Lazy (gz)Total (gz)node_modules (min)Own code (min)
./js4.3 KB49 KB54 KB208 KB12 KB
./js/beta20 B0 B20 B0 B0 B
./react429 KB49 KB478 KB1.3 MB168 KB
./react/beta20 B0 B20 B0 B0 B

Chunks:

EntryChunkLoadSize (gz)
./jsindex.jsinitial4.2 KB
./jschunkinitial120 B
./jsapache-arrowlazy49 KB
./js/betabeta.jsinitial20 B
./reactindex.jsinitial427 KB
./reacttslibinitial2.1 KB
./reactapache-arrowlazy49 KB
./react/betabeta.jsinitial20 B

CopilotAI 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

This PR fixes the --wait (blocking) typegen committed-types fallback gate so that environmental failures are tracked per surface (analytics queries vs metric views) and each failing surface must have its own committed .d.ts artifact present—preventing a committed artifact for one surface from incorrectly satisfying the fallback for the other.

Changes:

  • Split the single environmental-failure flag into per-surface flags and enforce a per-surface committed artifact check in blocking mode.
  • Improve the blocking-mode fatal error to name exactly which committed artifact(s) are missing.
  • Add/adjust tests to cover “cross-surface artifact cannot satisfy fallback” cases and verify committed metric types are preserved byte-for-byte when degraded writes are suppressed.

Reviewed changes

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

FileDescription
packages/appkit/src/type-generator/index.tsTracks environmental failures per surface and requires the corresponding committed .d.ts artifact(s) when blocking-mode fallbacks are used; improves fatal error messaging.
packages/appkit/src/type-generator/tests/unreachable-warehouse-gate.test.tsAdds end-to-end coverage ensuring committed metric types do not satisfy missing query fallback under environmental query failure.
packages/appkit/src/type-generator/tests/index.test.tsRefactors committed metric type setup and adds tests ensuring generated analytics types can’t satisfy missing metric fallback; tightens assertions that committed metric types are preserved.

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

@github-actions

Copy link
Copy Markdown
Contributor

🤖 AppKit PR bot

🔬 Run evals

Start an eval for this PR from the evals-monitor app: Go to Evals Monitor →

📦 Try this PR's app template

Scaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh auth login — and the Databricks CLI):

gh run download 30922962109 -R databricks/appkit -n appkit-template-0.52.0-pr.6fa3c49-fix-typegen-per-surface-gate-511 -D appkit-pr-511 \
&& unzip -o "appkit-pr-511/appkit-template-0.52.0-pr.6fa3c49-fix-typegen-per-surface-gate-511.zip" -d "appkit-pr-511" \
&& databricks apps init --template "appkit-pr-511"

The template pins @databricks/appkit and @databricks/appkit-ui to tarballs built from this branch, so the scaffolded app runs against this PR's code.

@atilafassina
atilafassina enabled auto-merge (squash) August 4, 2026 15:23
@atilafassina
atilafassina merged commit d55de0f into mainAug 5, 2026
11 checks passed
@atilafassina
atilafassina deleted the fix/typegen-per-surface-gate branch August 5, 2026 09:34
atilafassina added a commit that referenced this pull request Aug 5, 2026
PR #511 landed the same committed-types gate this branch had reworked.
Both wanted the metric surface to participate in the gate; #511's version
is strictly more precise, tracking the failure per surface so a committed
artifact for one surface can never stand in for a missing one elsewhere.
Resolve to #511's design and drop this branch's all-artifacts variant
(hasRequiredCommittedTypes and the now-unread metricTypesRequired flag).
Keep the .ts rename on top: the gate message says "type files" rather
than ".d.ts files", the two fixtures point at metric-views.ts, and the
committed-metric-types fixture carries the runtime const so a preserved
fallback stays a loadable module.
Signed-off-by: Atila Fassina <atila@fassina.eu>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@atilafassina@calvarjorge