Skip to content

docs(apps): forbid as unknown as double type assertions#65

Closed
keugenek wants to merge 1 commit intomainfrom
evgenii.kniazev/forbid-double-type-assertion
Closed

docs(apps): forbid as unknown as double type assertions#65
keugenek wants to merge 1 commit intomainfrom
evgenii.kniazev/forbid-double-type-assertion

Conversation

@keugenek
Copy link
Copy Markdown
Contributor

@keugenek keugenek commented May 5, 2026

Summary

databricks apps validate runs appkit lint, which enforces the no-double-type-assertion ast-grep rule (added in a recent @databricks/appkit version). Even one occurrence in a generated app fails the validate step, while build / typecheck / unit tests still pass — exactly the "validate flips Y→N, everything else stays Y" fingerprint observed in the apps-mcp-evals nightly's Mode-B regression (prod run 456555456546311, 2026-05-05; 11 of 16 clean-regress apps carried this pattern).

This adds an explicit guideline so the agent doesn't reach for as unknown as <T> when typing analytics-query results — that's the cast pattern the regressed apps were generating. Suggests proper alternatives (fix source type, runtime type guard, Zod narrowing, mapper function).

Independent confirmation

PromptingCo's CAO pilot (Apr 30, 25 DevHub templates, report draft) clusters the same family of failures as Cluster A — token passthrough / user_api_scopes — 5 of 9 documented Codex failures. Two independent benchmarks, same root cause.

Tracks

  • LKB-12428 (Mode B sub-task)
  • LKB-12426 (CLI 0.299.0 + AppKit 0.11.0 regression umbrella)

Verifying signal

Skill update only — verifying signal is "next apps-mcp-evals nightly's cb_* and analytics apps cleanly pass apps_validate_pass, with no as unknown as strings appearing in generated server.ts". The merged appkit_version capture in PR #1893815 means we can also slice the next nightly by AppKit version + skill version directly from MLflow.

Documentation safety checklist

  • Examples use least-privilege permissions (no unnecessary `ALL PRIVILEGES`, admin tokens, or broad scopes)
  • Elevated permissions are explicitly called out where required
  • Sensitive values are obfuscated (placeholder workspace IDs, URLs, no real tokens)
  • No insecure patterns introduced (e.g. disabled TLS verification, hardcoded credentials)

This is a one-line documentation addition with no examples or credentials — checklist applies trivially.

`databricks apps validate` runs `appkit lint`, which enforces the
`no-double-type-assertion` ast-grep rule (added in a recent
@databricks/appkit version). Even one occurrence in a generated app
fails the validate step, while build / typecheck / unit tests still
pass — that's the exact "validate flips Y→N, everything else stays Y"
fingerprint we saw in the apps-mcp-evals nightly's Mode-B regression
(2026-05-04 prod run, 11/16 clean-regress apps).

Add an explicit guideline so the agent doesn't reach for `as unknown
as <T>` when typing analytics-query results — that's the cast pattern
the regressed apps were generating.

Tracks LKB-12428.

Co-authored-by: Isaac
@keugenek keugenek requested a review from a team as a code owner May 5, 2026 16:19
@keugenek keugenek self-assigned this May 5, 2026
@keugenek keugenek requested review from MarioCadenas and pkosiec May 5, 2026 16:20
@keugenek
Copy link
Copy Markdown
Contributor Author

keugenek commented May 6, 2026

Friendly nudge — would appreciate a look when there's a moment.

This is the Mode-B fix for the apps-mcp-evals nightly: in last week's prod run (456555456546311, 2026-05-05) 11 of 16 Mode-B regressed apps shipped with this exact as unknown as pattern that appkit lint's no-double-type-assertion flags. Same family was independently identified as Cluster A in the PromptingCo CAO pilot across DevHub templates last week.

One-line skill text addition, CI green, no behavior change. Once it lands, the next nightly's cb_* and analytics apps should cleanly pass apps_validate_pass (slice-able by appkit_version + skill_version in MLflow now that databricks-eng/universe#1893815 merged).

cc @pkosiec @MarioCadenas

@keugenek
Copy link
Copy Markdown
Contributor Author

keugenek commented May 6, 2026

/merge

keugenek added a commit that referenced this pull request May 7, 2026
…tpicks

Subsumes PR #65 (TypeScript casts). Reorders smoke-test bullets together,
trims AppKit API bullet to avoid duplicating the Frameworks section,
and tightens wording per nitpicker review.

Co-authored-by: Isaac
@keugenek
Copy link
Copy Markdown
Contributor Author

keugenek commented May 7, 2026

Superseded by #67 which consolidates this guideline along with three additional codegen-quality guidelines, with nitpicker review feedback addressed.

@keugenek keugenek closed this May 7, 2026
keugenek added a commit that referenced this pull request May 7, 2026
…#67)

## Summary
Consolidates PR #65 and adds four codegen-quality guidelines to the
AppKit skill:

- **AppKit API surface**: run `npx @databricks/appkit docs` before
writing call sites — avoids invented signatures that fail `tsc --noEmit`
during validate
- **TypeScript casts**: forbid `as unknown as <T>` double-assertions —
`appkit lint` enforces `no-double-type-assertion` (from PR #65)
- **Smoke test selectors**: `getByLabel` (Playwright) not
`getByLabelText` (React Testing Library) — wrong API throws `TypeError`
at runtime
- **Smoke test data**: keep result sets under the 1 MB analytics-event
payload cap — unbounded queries cause `net::ERR_ABORTED` and missing UI
elements

Addresses nitpicker review feedback: reordered smoke-test bullets
together, trimmed AppKit API bullet to avoid duplicating the Frameworks
section, replaced magic `LIMIT 500` with generic `LIMIT`, concrete
aggregation example, Zod narrowing listed first.

Supersedes #65.

## Test plan
- [x] `skills.py validate` passes
- [x] Three sub-agent tests confirm guidelines are clear and actionable
- [ ] Review guidelines against actual AppKit docs and Playwright API

This pull request and its description were written by Isaac.
keugenek added a commit that referenced this pull request May 7, 2026
…ion pinning (LKB-12428, LKB-12465, LKB-12614) (#69)

## Summary

Three codegen-quality guidelines for the `databricks-apps` skill, each
addressing a distinct regression observed in apps-mcp-evals nightlies:

1. **AppKit API surface** (LKB-12465): run `npx @databricks/appkit docs
<section>` before writing server code. Training data has stale shapes; a
single invented signature fails `tsc --noEmit`.
2. **TypeScript casts** (LKB-12428): never use `as unknown as <T>` —
`appkit lint` enforces `no-double-type-assertion`. Use Zod, type guards,
or mapper functions.
3. **AppKit version pinning** (LKB-12614): never override
`@databricks/appkit` version in `package.json` — `databricks apps init`
sets the correct version. Agent was writing stale versions from training
data (0.11.0 when latest is 0.31.0).

Plus: smoke-test selector guidance (use Playwright locator APIs, not
React Testing Library), data-size cap for analytics payloads.

Supersedes PR #65 (which covered only the TypeScript-casts bullet).
Mario Cadenas's review feedback from #65 is incorporated (auto-gen
queryKey files carve-out, warehouse-unavailable `unknown` case).

## Documentation safety checklist

- [x] Examples use least-privilege permissions
- [x] Elevated permissions explicitly called out where required
- [x] Sensitive values are obfuscated
- [x] No insecure patterns introduced

Co-authored-by: Isaac
Sign up for free to 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.

1 participant