experimental: import ai-dev-kit skills into experimental/ directory#73
Conversation
jamesbroadhead
requested review from
a team,
lennartkats-db and
simonfaltum
as code owners
jamesbroadhead
force-pushed
the
experimental-aidevkit
branch
from
2035bab to
2a45b83
Compare
jamesbroadhead
force-pushed
the
experimental-aidevkit
branch
from
8387654 to
a0fbb21
Compare
The default DATABRICKS_SKILLS_REF pin is a release tag that pre-dates the experimental_skills manifest section (see databricks/databricks-agent-skills#73). Users who pass --experimental against that ref today silently get no experimental skills installed. Log a Warnf at install time pointing them at the env var override (=main, or a future release that includes the section). Helper: manifestHasExperimental(), unit-tested in source_test.go. Co-authored-by: Isaac
jamesbroadhead
changed the title
jamesbroadhead
changed the title
Replaces the previous import (a-d-k commit 2228c3e on add_appkit) with the head of a-d-k PR #533 (commit 9c7a5b3 on appkit-on-experimental), which targets a-d-k's experimental branch. Changes: - Refresh 23 experimental skill directories from the new source. - Drop databricks-lakebase-provisioned — removed on a-d-k experimental. - databricks-apps-python: rename + SKILL.md now leads with AppKit (TypeScript + React SDK) and demotes Python frameworks to alternatives; 6-mcp-approach.md replaced with 6-cli-approach.md. - databricks-lakebase-autoscale/references/connection-patterns.md: change placeholder `user:password` to `<user>:<password>` so the secret scanner doesn't flag the doc-only example. Cosmetic only. - Continue to exclude databricks-model-serving and databricks-spark-declarative-pipelines (PR #73 TODOs #1b and #5). - Regenerate manifest.json and agents/openai.yaml stubs via scripts/skills.py generate. - Update experimental/README.md provenance section with the new SHA, branch, and divergence notes. Co-authored-by: Isaac
Merges the comprehensive jobs reference content from experimental/databricks-jobs/ into skills/databricks-jobs/ and removes the experimental copy. What's new in stable databricks-jobs (v0.2.0): - Full task-types reference (9 types: notebook, spark_python, python_wheel, sql, dbt, pipeline, spark_jar, run_job, for_each) - All 6 trigger types with examples (cron, periodic, file_arrival, table_update, continuous, manual) + combining + pause/resume - Notifications + health rules + retries + timeouts + queues - 7 end-to-end worked examples (ETL, warehouse refresh, event-driven, ML training, multi-env, streaming, cross-job orchestration) - run_if conditions, environments (serverless deps), permissions What's retained from the prior stable skill: - parent: databricks-core hierarchy - Compatibility note + version metadata (bumped 0.1.0 → 0.2.0) - Scaffolding workflow (databricks bundle init + CLAUDE.md/AGENTS.md template + project structure) - Unit testing + development workflow sections - agents/openai.yaml + assets/ Cleanups during the merge: - Replaced the trigger-spam description with a terse one - Normalized hard-coded /Workspace/Users/user@example.com/ paths in the imported reference files to /Workspace/Shared/ scripts/skills.py: updated SKILL_METADATA description for jobs to reflect the broader scope. Manifest regenerated; experimental count drops from 23 to 22. Resolves PR #73 TODO #1a. Co-authored-by: Isaac
Adds an experimental/ directory containing the 26 agent skills from databricks-solutions/ai-dev-kit. These are imported as a snapshot on a best-effort basis — they are not officially supported skills and follow a looser contract than skills/ (no agents/openai.yaml, no shared-asset sync, no SKILL_METADATA gate). The manifest now exposes them under a new top-level experimental_skills map so consumers can distinguish them from stable skills and skip them by default. scripts/skills.py handles the new directory; the existing generate / validate flow is unchanged for stable skills. Co-authored-by: Isaac
Owners are the top contributors to databricks-solutions/ai-dev-kit (>=10 commits at import time). The cross-org team @databricks-solutions/ai-dev-kit-maintainers is the canonical owner; this line can be replaced with that team handle if the team gets write access to this repo. Co-authored-by: Isaac
Adds a Provenance & sync model section to experimental/README.md: - Transition phase: source of truth is upstream ai-dev-kit; this dir gets periodic manual re-syncs. - Post-lock: source of truth is this repo; ai-dev-kit's databricks-skills/ becomes read-only. Co-authored-by: Isaac
Adds two helpers to scripts/skills.py that run as part of `generate`:
- `ensure_experimental_codex_metadata` copies the shared
assets/databricks.{svg,png} into each experimental skill (mirroring
what stable skills get via sync_assets).
- `synthesize_openai_yaml` writes agents/openai.yaml from each
experimental skill's SKILL.md frontmatter (display_name from the
skill name, short_description from the first sentence of the
frontmatter description, brand_color and icon paths fixed).
Both run only when the destination file is absent, so upstream
ai-dev-kit can override by shipping its own openai.yaml or assets.
This closes the cosmetic gap that experimental skills installed into
Codex CLI would render without an icon or marketplace metadata.
Co-authored-by: Isaac
Same pattern as databricks-model-serving: the experimental version covers a different surface than the stable databricks-pipelines skill (workflow / scaffolding / DLT-migration / per-language performance vs feature reference / decision tree / common traps / format options). The DAB-coupled scaffolding workflow is also the specific concern Dustin flagged in the Apr 28 Slack thread for demo-generator flows. Removed experimental/databricks-spark-declarative-pipelines/; manifest regenerated (24 experimental skills). Follow-up TODO: port the high-value pieces (DLT migration guide, workflow A/B/C decision matrix, per-language performance reference, language-selection rules) into skills/databricks-pipelines/, stripping MCP-tool refs. Co-authored-by: Isaac
Adds the actual a-d-k commit hash (2228c3e on the add_appkit branch, 5 commits ahead of origin/main at import time) along with a note about the local deltas vs public main. Surfaces the key one: the databricks-app-python -> databricks-apps-python rename hadn't merged upstream, and pulling from the renamed version is what avoids a 3rd skill-name collision with d-a-s's stable databricks-apps. Co-authored-by: Isaac
extract_version_from_skill() falls back to a synthetic version when a skill's SKILL.md frontmatter has no version: field. The previous fallback was 0.0.0, which several install tools treat as "unset" rather than "first release". Bump to 0.0.1. Affects all 24 experimental skills (imported from ai-dev-kit without versions) plus the stable databricks-dabs skill. Skills with an explicit version are unchanged. Co-authored-by: Isaac
- experimental/README.md: install examples now use the -experimental suffix on the skill name + the --experimental flag (matching the install-path behaviour landed in databricks/cli#5243). Adds a short note explaining why the in-repo dir name and the install dir name differ. - experimental/README.md: drop databricks-model-serving from the collision example (it was removed from this PR earlier). - experimental/README.md: update the (also available as stable skill) note for databricks-jobs to point at the open TODO #1a. - Root README: clarify the suffixed install name in the by-name install example. Co-authored-by: Isaac
Replaces the previous import (a-d-k commit 2228c3e on add_appkit) with the head of a-d-k PR #533 (commit 9c7a5b3 on appkit-on-experimental), which targets a-d-k's experimental branch. Changes: - Refresh 23 experimental skill directories from the new source. - Drop databricks-lakebase-provisioned — removed on a-d-k experimental. - databricks-apps-python: rename + SKILL.md now leads with AppKit (TypeScript + React SDK) and demotes Python frameworks to alternatives; 6-mcp-approach.md replaced with 6-cli-approach.md. - databricks-lakebase-autoscale/references/connection-patterns.md: change placeholder `user:password` to `<user>:<password>` so the secret scanner doesn't flag the doc-only example. Cosmetic only. - Continue to exclude databricks-model-serving and databricks-spark-declarative-pipelines (PR #73 TODOs #1b and #5). - Regenerate manifest.json and agents/openai.yaml stubs via scripts/skills.py generate. - Update experimental/README.md provenance section with the new SHA, branch, and divergence notes. Co-authored-by: Isaac
The previous regex-only parser in extract_description_from_skill() captured the YAML block-scalar indicator (`>-`) verbatim, so any SKILL.md that wrote `description: >-\n multi-line content` produced a manifest entry of `">-"`. The new ai-dev-kit import (PR #533) brought two such files — databricks-dbsql and databricks-execution-compute — which landed corrupted descriptions in manifest.json and corrupted short_description / default_prompt in agents/openai.yaml. Walk the frontmatter line by line: if the value is a block-scalar indicator (|, |-, |+, >, >-, >+), aggregate the indented continuation lines (folded with spaces for `>`-style, newlines for `|`-style). Regenerate manifest.json and the two affected agents/openai.yaml stubs. Co-authored-by: Isaac
Replaces the hand-rolled block-scalar walker (added one commit ago) with PyYAML's safe_load. PyYAML's default SafeLoader is pure-Python — no C extension required — and handles every YAML edge case for free instead of reimplementing them. Side-benefit: also fixes a second latent bug. The regex parser stripped the outer YAML quotes but left inner `\"` escapes intact as literal backslash-quote characters, so descriptions like `"... mentions \"switch workspace\"..."` ended up in manifest.json with the backslashes preserved. yaml.safe_load resolves these correctly. Regenerated manifest reflects the fix for databricks-config. Co-authored-by: Isaac
Merges the comprehensive jobs reference content from experimental/databricks-jobs/ into skills/databricks-jobs/ and removes the experimental copy. What's new in stable databricks-jobs (v0.2.0): - Full task-types reference (9 types: notebook, spark_python, python_wheel, sql, dbt, pipeline, spark_jar, run_job, for_each) - All 6 trigger types with examples (cron, periodic, file_arrival, table_update, continuous, manual) + combining + pause/resume - Notifications + health rules + retries + timeouts + queues - 7 end-to-end worked examples (ETL, warehouse refresh, event-driven, ML training, multi-env, streaming, cross-job orchestration) - run_if conditions, environments (serverless deps), permissions What's retained from the prior stable skill: - parent: databricks-core hierarchy - Compatibility note + version metadata (bumped 0.1.0 → 0.2.0) - Scaffolding workflow (databricks bundle init + CLAUDE.md/AGENTS.md template + project structure) - Unit testing + development workflow sections - agents/openai.yaml + assets/ Cleanups during the merge: - Replaced the trigger-spam description with a terse one - Normalized hard-coded /Workspace/Users/user@example.com/ paths in the imported reference files to /Workspace/Shared/ scripts/skills.py: updated SKILL_METADATA description for jobs to reflect the broader scope. Manifest regenerated; experimental count drops from 23 to 22. Resolves PR #73 TODO #1a. Co-authored-by: Isaac
PR #533 has merged into upstream a-d-k experimental. The
databricks-skills/ tree is byte-identical between the previous
import SHA (9c7a5b3) and the merge commit (7b07f18) — only
install.{sh,ps1} changed, which we don't import. README updated
to point at the now-authoritative branch + SHA and drop the "one
commit ahead of origin/experimental" caveat.
Co-authored-by: Isaac
jamesbroadhead
force-pushed
the
experimental-aidevkit
branch
from
50467c3 to
5719156
Compare
Stable ↔ experimental skill overlapSurfacing the overlap explicitly per Quentin's question. There are 6 stable/experimental pairs with non-trivial topical overlap; the remaining 16 experimental skills cover surfaces with no stable equivalent.
No stable equivalent (16 skills — no overlap)
TL;DRThree pairs are real duplicates that should converge over time: dabs/bundles (drop experimental), lakebase/lakebase-autoscale (drop experimental, port connection-patterns content first), core/config (drop experimental). Two pairs are complementary and worth keeping side-by-side: apps/apps-python (TS vs Python audiences), serverless-migration/execution-compute (migration vs runtime selection). One pair is mostly disjoint: core/python-sdk (CLI vs SDK). Intentional for this PR per the rationale in the description — port adk mostly intact, then resolve the duplicates as a series of single-skill PRs against stable, with the experimental copies removed in the same change. Happy to start that series if folks agree on the deltas above. This comment was written by Claude. |
Removes three experimental skills that duplicate stable equivalents without adding net new content: - databricks-bundles → use stable `databricks-dabs` - databricks-lakebase-autoscale → use stable `databricks-lakebase` - databricks-config → use stable `databricks-core` Cross-references in the surviving experimental skills (apps-python, python-sdk, zerobus-ingest, synthetic-data-gen, 4-deployment.md) now point at the stable names by bare name — matching the convention already used in stable skills' "Related Skills" / "Product Skills" sections. `experimental/README.md` records the removals and points readers at the stable replacements. Manifest regenerated — experimental skill count drops from 22 to 19. Constraint per James: root must not depend on experimental, but experimental may depend on root. Honored — all new references go in that direction, and no stable skill content was changed by this commit. Co-authored-by: Isaac
|
👋 @lennartkats-db @fjakobs @camielstee-db — Claude here on James's behalf, just refreshing review attention. Quite a lot has changed since this PR opened on May 12 and the description is long, so a tl;dr: Resolved since opening:
Current scope: 19 experimental skills under Still open: this is the only blocker on the cli side (#5243) — that PR can't go end-to-end-testable in a release until this lands and a d-a-s tag is cut containing Happy to walk through any specific concern (skill-by-skill overlap analysis is in the May 15 comment; the merge-direction caveat re: Dustin's Apr 28 thread is still in the description). Otherwise looking for a review pass. (comment posted by Claude) |
The previous fix added `pip install pyyaml` to the validate workflow, but the databricks-protected-runner can't reach pypi.org (SSL errors on every retry). Restoring a stdlib-only description parser sidesteps the network requirement entirely. The hand-rolled parser is the same shape as the pre-db9b2e5 version: handles plain, quoted, and block-scalar (>- / |-) values. db9b2e5 moved to yaml.safe_load to also unescape inner \" sequences, but no current SKILL.md description uses that escape, so the regression risk is nil. Re-introducing pyyaml is a one-line change if a future description ever needs it. Manifest regenerated; no content drift, only updated_at moved. Co-authored-by: Isaac
dustinvannoy-db
left a comment
dustinvannoy-db
left a comment
There was a problem hiding this comment.
A few things for us to consider which I commented on.
Higher level feedback:
- It sounds like this will add
-experimentalon the end of each skill name during install and I don't think we should. - Are all the skills being added to experimental ones you actually want in here? I want to confirm everything we add is expected to be included here long term, even if we modify skill content with follow up PRs.
- Experimental README might be too specific on the plan for future. I think we can leave out more of the idea of deprecation from AI Dev Kit and we will address that in AI Dev Kit repo once we are ready, likely next few weeks for experimental installs.
- We should follow up with a PR to move reference files into
references/directory. We weren't consistent in AI Dev Kit but should follow that standard going forward.
|
|
||
| ## `ai_fix_grammar` | ||
|
|
||
| **Docs:** https://docs.databricks.com/aws/en/sql/language-manual/functions/ai_fix_grammar |
There was a problem hiding this comment.
Do the current skills have references to web address? We could try removing these if we expect it to be an issue. I have only heard a few concerns about references that require the agent to use access internet.
There was a problem hiding this comment.
Assuming this is ok.
There was a problem hiding this comment.
This is ok, we just need to strip off the language and cloud provdier
| **Docs:** https://docs.databricks.com/aws/en/sql/language-manual/functions/ai_fix_grammar | |
| **Docs:** https://docs.databricks.com/sql/language-manual/functions/ai_fix_grammar |
@jamesbroadhead can you make sure this change happens throughout?
There was a problem hiding this comment.
@jamesbroadhead / Claude: please take another look at this; it's indeed our policy and documentation team guidance to strip these prefixes. We do the same in the CLI.
There was a problem hiding this comment.
@lennartkats-db this skill is to create a Genie Space, is that ok to include?
There was a problem hiding this comment.
This uses all public APIs, which is good, but please save this one for the next version.
There was a problem hiding this comment.
@jamesbroadhead / Claude I saw an agent response to the review saying
"This uses all public APIs, which is good, but please save this one for the next version" — I couldn't tell which file/skill it referred to. Mind clarifying?
The comment refers to the experimental/databricks-genie directory. That entire directory (and any references to it) should be left out for now; we'll revisit.
- Drop the -experimental suffix model entirely. Skills install under their plain names; the install dir is `~/.claude/skills/<name>/`. Stable+experimental collisions are resolved upstream by renaming (not by suffixing), and the manifest generator raises if a collision shows up. - Update README install instructions to the new top-level `databricks aitools install [name] --experimental` surface (was the legacy `databricks experimental aitools skills install ...` path). - Trim the experimental/README "Provenance & sync model" section per Dustin's L113/L126 asks — the long-term deprecation plan lives in ai-dev-kit when it's ready, not here. - Apply Dustin's L78 wording suggestion for the "previously experimental" callout. - Add @cankoklu-db and @yogesh-dbx as Code Owners on /experimental/. - Fix the databricks-pipelines manifest description: "Databricks Spark Declarative Pipelines (SDP) for ETL and streaming". - Drop the broken n8n-to-databricks cross-reference in databricks-ai-functions/4-document-processing-pipeline.md (the referenced skill does not exist). Paired with the cli-side suffix removal in databricks/cli#5243. Co-authored-by: Isaac
|
👋 @dustinvannoy-db @lennartkats-db @fjakobs @camielstee-db — Claude here on James's behalf. Addressing Dustin's review in Big-ticket item — drop the Inline nits applied:
Items left for you:
(comment posted by Claude) |
## Summary The skills manifest in `databricks/databricks-agent-skills` is gaining experimental skills sourced from a new `experimental/` directory in the repo (see paired [d-a-s PR databricks#73](databricks/databricks-agent-skills#73), which imports the ai-dev-kit skill catalog into `experimental/`). This wires the parsing through the aitools installer: - `Manifest.Skills` is a **single map** holding both stable and experimental entries; the per-skill `repo_dir` field ("skills" or "experimental") is the source of truth for whether a skill is experimental. `SkillMeta.IsExperimental()` derives state from `RepoDir`. - Experimental skills get a `-experimental` suffix on their install-side key during `normalizeManifest`; `SourceName` preserves the unsuffixed name for fetch URLs. - The existing `--experimental` flag (already wired in `cmd/skills.go`) now has experimental skills to install; without it, `resolveSkills` filters them out as before. ## UX ``` # default — only stable skills databricks experimental aitools skills install # all experimental skills, plus stable databricks experimental aitools skills install --experimental # one experimental skill by name (--experimental still required by resolveSkills) databricks experimental aitools skills install databricks-iceberg-experimental --experimental ``` ## TODOs / caveats for iteration 1. ~~**`DATABRICKS_SKILLS_REF` pin.**~~ **Partially resolved.** The default ref is still the latest stable release tag (sourced from `experimental/aitools/lib/installer/SKILLS_VERSION`); experimental entries won't exist there until d-a-s cuts a release with [PR databricks#73](databricks/databricks-agent-skills#73) merged. The default ref bump is a follow-up automated by the SKILLS_VERSION file. **UX fix shipped in this PR**: if `--experimental` is passed but the manifest at the resolved ref exposes no experimental skills, a warning is logged pointing users at `DATABRICKS_SKILLS_REF=main`. 2. ~~**Collision handling is naive.**~~ **Resolved.** Every experimental skill gets a `-experimental` suffix on its install-side key during `normalizeManifest`. The manifest key + install dir both carry the suffix; the `SourceName` field on `SkillMeta` preserves the upstream repo dir name for fetch URLs. Users see at a glance which installed skills are experimental. Also handled: **experimental↔stable transitions**. If a skill flips its experimental status upstream (the same logical skill changes manifest key), `install` removes the stale variant on disk + state before installing the new one, and `uninstall` accepts either variant name (and removes both if both are present). Helper: `alternateVariantKey()`. Covered by tests `TestInstallReplacesAlternateVariant`, `TestUninstallByEitherVariantRemovesBoth`, `TestUninstallByAlternateNameWhenOnlyOneVariantInstalled`. 3. ~~**`list` UX.**~~ **Resolved.** `aitools skills list` shows experimental skills with an `[experimental]` tag in the NAME column (driven by `meta.IsExperimental()`). Combined with the TODO databricks#2 resolution (`-experimental` suffix in the manifest key), every experimental row reads e.g. `databricks-iceberg-experimental [experimental]` — slightly redundant but a clear visual anchor. Hide-by-default was considered but rejected: users running `list` are usually looking for what's available, and silently omitting experimental skills makes them un-discoverable. 4. ~~**State tracking.**~~ **Resolved — kept additive semantics.** `InstallState.IncludeExperimental` records what was last requested but is not used to drive retroactive removal. Running `install` without `--experimental` leaves previously-installed experimental skills in place. Rationale: (a) users running `install` are typically adding/updating, not declaring set membership; (b) silently uninstalling things the user previously asked for is surprising; (c) the transition cleanup shipped under TODO databricks#2 handles the actual drift case (skill's experimental status flipping upstream). Removal is what `uninstall` is for. 5. ~~**No acceptance test yet.**~~ **Resolved.** Added acceptance tests under `acceptance/experimental/aitools/skills/install*/` covering the install flow against a mocked manifest server: - Stable-only install (no flag) → 1 skill installed - `--experimental` install adds the experimental skill (with `-experimental` suffix per the install-path model) → 2 skills total - Re-running `--experimental` is idempotent - Specific-skill install (`install --skills <name>`) for both stable and experimental - `--experimental` against a manifest with no experimental entries logs a nudge To make these reachable, exposed a new env-var override `DATABRICKS_SKILLS_BASE_URL` that overrides the hard-coded `raw.githubusercontent.com` base URL used by `GitHubManifestSource.FetchManifest` and `fetchSkillFile`. Defaults to the canonical URL when unset, so no production behavior change. Updated `Taskfile.yml`'s `test-exp-aitools` task to include `acceptance/experimental/aitools/**`. Variants left as follow-up acceptance tests (the structure is now in place): - Variant transition cleanup (stable → experimental, experimental → stable) - Uninstall flow (with both variants installed) 6. ~~**`--experimental` flag scope.**~~ **Resolved — kept current scope.** Each command has internally consistent behavior: - `install --experimental` → explicit opt-in (required to install experimental skills). - `update` → state-driven (honors `InstallState.IncludeExperimental` from the last `install`). If you opted in once, future updates refresh experimentals; otherwise they're skipped. - `list` → shows all skills with an `[experimental]` tag (no filtering — discovery first, opt-in to install). Adding `--experimental` / `--no-experimental` to `update` for one-off overrides was considered but rejected: the natural workflow is to re-run `install --experimental` (or just `install`), which already sets the desired state. Follow-up if real users hit a use case for the override. 7. ~~**Manifest shape.**~~ **Resolved.** Replaced the original two-map design (`skills` + `experimental_skills` + a per-skill `experimental` bool) with a single `skills` map where each entry's `repo_dir` (`"skills"` or `"experimental"`) is the source of truth. The cli derives experimental state from `RepoDir` via `SkillMeta.IsExperimental()`. Collisions between stable and experimental skills with the same repo dir name must be resolved upstream in d-a-s (which they already are — d-a-s PR databricks#73's TODO #1a merged the only known collision into stable). The d-a-s manifest generator should be updated to emit `repo_dir` per skill; until then `normalizeManifest` defaults a missing `RepoDir` to `"skills"` so older manifests still parse. ## Test plan - [x] `go build ./...` passes. - [x] `go test ./experimental/aitools/...` passes (`source_test.go` covers the normalize/IsExperimental cases). - [x] `go test ./acceptance -run TestAccept/experimental/aitools` passes (a pre-existing flake intermittently surfaces an `lstat` warning during copyDir, ~10% of multi-test runs; unrelated to this refactor). - [ ] Run `./task lint` and `./task fmt` before merge. - [ ] Manual: against a d-a-s ref containing experimental entries with `repo_dir`, verify the four UX cases above behave correctly. This pull request and its description were written by Claude. --------- Co-authored-by: simon <4305831+simonfaltum@users.noreply.github.com> Co-authored-by: simon <simon.faltum@databricks.com>
dustinvannoy-db
left a comment
dustinvannoy-db
left a comment
There was a problem hiding this comment.
I'm good with this one once @lennartkats-db confirms all these skills are ok to include.
Re-sync from databricks-solutions/ai-dev-kit:experimental (now at 20a92a3, 6 commits ahead of the previously-synced f9b404b). Upstream commits since f9b404b: - 0ebc38b "Surface silent failures in installer + dashboard skill" - f9b404b "Replace mas_manager.py with native supervisor-agents CLI" - 27ebae4 "SDP skill: continuous=false default, single canonical create example" - 7044312 "Rename databricks-model-serving → databricks-ml-training-serving" - f81f90e "ml-training-serving: densify SKILL.md, add jobs submit pattern" - 20a92a3 "tests: outcome-oriented rewrite across 7 skills + strip MCP tool_modules from all manifests" Most upstream drift was confined to "Related Skills" sections and was rename-only (databricks-model-serving → databricks-ml-training-serving, databricks-bundles → databricks-config, etc.). Per the convention from c4daa14 / 2996315, those references are translated back to bare stable- analogue names (databricks-model-serving, databricks-pipelines, databricks-dabs, databricks-core, databricks-lakebase, databricks-jobs) rather than linked into experimental/<name>/ paths that don't exist here. Substantive content deltas after translation: - experimental/databricks-apps-python/SKILL.md: adds a "databricks-app-apx" related-skills entry (no stable analogue yet — kept as a bare name) and tightens the lakebase description. - experimental/databricks-python-sdk/SKILL.md: minor wording tweak on databricks-core ("profile and authentication setup"). - experimental/README.md: source SHA pin bumped 7b07f18 → 20a92a3. Note: the prior 10baa35 sync from f9b404b did not bump the README pin — it stayed on 7b07f18 (an oversight). This commit corrects that and brings the pin to the latest synced SHA. The rename in 7044312 (databricks-model-serving → databricks-ml-training- serving) and the SDP / installer changes from upstream are outside the import scope: those skills have stable equivalents in skills/ and were excluded from experimental/ originally. Manifest unchanged (only narrative content moved). Co-authored-by: Isaac
|
|
||
| ## `ai_fix_grammar` | ||
|
|
||
| **Docs:** https://docs.databricks.com/aws/en/sql/language-manual/functions/ai_fix_grammar |
There was a problem hiding this comment.
This is ok, we just need to strip off the language and cloud provdier
| **Docs:** https://docs.databricks.com/aws/en/sql/language-manual/functions/ai_fix_grammar | |
| **Docs:** https://docs.databricks.com/sql/language-manual/functions/ai_fix_grammar |
@jamesbroadhead can you make sure this change happens throughout?
## Summary The skills manifest in `databricks/databricks-agent-skills` is gaining experimental skills sourced from a new `experimental/` directory in the repo (see paired [d-a-s PR databricks#73](databricks/databricks-agent-skills#73), which imports the ai-dev-kit skill catalog into `experimental/`). This wires the parsing through the aitools installer: - `Manifest.Skills` is a **single map** holding both stable and experimental entries; the per-skill `repo_dir` field ("skills" or "experimental") is the source of truth for whether a skill is experimental. `SkillMeta.IsExperimental()` derives state from `RepoDir`. - Experimental skills get a `-experimental` suffix on their install-side key during `normalizeManifest`; `SourceName` preserves the unsuffixed name for fetch URLs. - The existing `--experimental` flag (already wired in `cmd/skills.go`) now has experimental skills to install; without it, `resolveSkills` filters them out as before. ## UX ``` # default — only stable skills databricks experimental aitools skills install # all experimental skills, plus stable databricks experimental aitools skills install --experimental # one experimental skill by name (--experimental still required by resolveSkills) databricks experimental aitools skills install databricks-iceberg-experimental --experimental ``` ## TODOs / caveats for iteration 1. ~~**`DATABRICKS_SKILLS_REF` pin.**~~ **Partially resolved.** The default ref is still the latest stable release tag (sourced from `experimental/aitools/lib/installer/SKILLS_VERSION`); experimental entries won't exist there until d-a-s cuts a release with [PR databricks#73](databricks/databricks-agent-skills#73) merged. The default ref bump is a follow-up automated by the SKILLS_VERSION file. **UX fix shipped in this PR**: if `--experimental` is passed but the manifest at the resolved ref exposes no experimental skills, a warning is logged pointing users at `DATABRICKS_SKILLS_REF=main`. 2. ~~**Collision handling is naive.**~~ **Resolved.** Every experimental skill gets a `-experimental` suffix on its install-side key during `normalizeManifest`. The manifest key + install dir both carry the suffix; the `SourceName` field on `SkillMeta` preserves the upstream repo dir name for fetch URLs. Users see at a glance which installed skills are experimental. Also handled: **experimental↔stable transitions**. If a skill flips its experimental status upstream (the same logical skill changes manifest key), `install` removes the stale variant on disk + state before installing the new one, and `uninstall` accepts either variant name (and removes both if both are present). Helper: `alternateVariantKey()`. Covered by tests `TestInstallReplacesAlternateVariant`, `TestUninstallByEitherVariantRemovesBoth`, `TestUninstallByAlternateNameWhenOnlyOneVariantInstalled`. 3. ~~**`list` UX.**~~ **Resolved.** `aitools skills list` shows experimental skills with an `[experimental]` tag in the NAME column (driven by `meta.IsExperimental()`). Combined with the TODO databricks#2 resolution (`-experimental` suffix in the manifest key), every experimental row reads e.g. `databricks-iceberg-experimental [experimental]` — slightly redundant but a clear visual anchor. Hide-by-default was considered but rejected: users running `list` are usually looking for what's available, and silently omitting experimental skills makes them un-discoverable. 4. ~~**State tracking.**~~ **Resolved — kept additive semantics.** `InstallState.IncludeExperimental` records what was last requested but is not used to drive retroactive removal. Running `install` without `--experimental` leaves previously-installed experimental skills in place. Rationale: (a) users running `install` are typically adding/updating, not declaring set membership; (b) silently uninstalling things the user previously asked for is surprising; (c) the transition cleanup shipped under TODO databricks#2 handles the actual drift case (skill's experimental status flipping upstream). Removal is what `uninstall` is for. 5. ~~**No acceptance test yet.**~~ **Resolved.** Added acceptance tests under `acceptance/experimental/aitools/skills/install*/` covering the install flow against a mocked manifest server: - Stable-only install (no flag) → 1 skill installed - `--experimental` install adds the experimental skill (with `-experimental` suffix per the install-path model) → 2 skills total - Re-running `--experimental` is idempotent - Specific-skill install (`install --skills <name>`) for both stable and experimental - `--experimental` against a manifest with no experimental entries logs a nudge To make these reachable, exposed a new env-var override `DATABRICKS_SKILLS_BASE_URL` that overrides the hard-coded `raw.githubusercontent.com` base URL used by `GitHubManifestSource.FetchManifest` and `fetchSkillFile`. Defaults to the canonical URL when unset, so no production behavior change. Updated `Taskfile.yml`'s `test-exp-aitools` task to include `acceptance/experimental/aitools/**`. Variants left as follow-up acceptance tests (the structure is now in place): - Variant transition cleanup (stable → experimental, experimental → stable) - Uninstall flow (with both variants installed) 6. ~~**`--experimental` flag scope.**~~ **Resolved — kept current scope.** Each command has internally consistent behavior: - `install --experimental` → explicit opt-in (required to install experimental skills). - `update` → state-driven (honors `InstallState.IncludeExperimental` from the last `install`). If you opted in once, future updates refresh experimentals; otherwise they're skipped. - `list` → shows all skills with an `[experimental]` tag (no filtering — discovery first, opt-in to install). Adding `--experimental` / `--no-experimental` to `update` for one-off overrides was considered but rejected: the natural workflow is to re-run `install --experimental` (or just `install`), which already sets the desired state. Follow-up if real users hit a use case for the override. 7. ~~**Manifest shape.**~~ **Resolved.** Replaced the original two-map design (`skills` + `experimental_skills` + a per-skill `experimental` bool) with a single `skills` map where each entry's `repo_dir` (`"skills"` or `"experimental"`) is the source of truth. The cli derives experimental state from `RepoDir` via `SkillMeta.IsExperimental()`. Collisions between stable and experimental skills with the same repo dir name must be resolved upstream in d-a-s (which they already are — d-a-s PR databricks#73's TODO #1a merged the only known collision into stable). The d-a-s manifest generator should be updated to emit `repo_dir` per skill; until then `normalizeManifest` defaults a missing `RepoDir` to `"skills"` so older manifests still parse. ## Test plan - [x] `go build ./...` passes. - [x] `go test ./experimental/aitools/...` passes (`source_test.go` covers the normalize/IsExperimental cases). - [x] `go test ./acceptance -run TestAccept/experimental/aitools` passes (a pre-existing flake intermittently surfaces an `lstat` warning during copyDir, ~10% of multi-test runs; unrelated to this refactor). - [ ] Run `./task lint` and `./task fmt` before merge. - [ ] Manual: against a d-a-s ref containing experimental entries with `repo_dir`, verify the four UX cases above behave correctly. This pull request and its description were written by Claude. --------- Co-authored-by: simon <4305831+simonfaltum@users.noreply.github.com> Co-authored-by: simon <simon.faltum@databricks.com>
lennartkats-db
left a comment
lennartkats-db
left a comment
There was a problem hiding this comment.
Awesome work!!! Minor suggested changes inline, please take a look
| #!/usr/bin/env python3 | ||
| """Compute CLI - Execute code and manage compute resources on Databricks. |
There was a problem hiding this comment.
Note, not blocking review: for experimental this is fine but for getting this out of experimental I think we need to just rely on the Databricks CLI. And if that somehow has gaps then we should fix them!
There was a problem hiding this comment.
This uses all public APIs, which is good, but please save this one for the next version.
| @@ -0,0 +1,7 @@ | |||
| interface: | |||
| display_name: "Databricks Mlflow Evaluation" | |||
There was a problem hiding this comment.
| display_name: "Databricks Mlflow Evaluation" | |
| display_name: "Databricks MLflow Evaluation" |
| @@ -0,0 +1,7 @@ | |||
| interface: | |||
| display_name: "Databricks Mlflow Evaluation" | |||
There was a problem hiding this comment.
Non-blocking, but we should review how this compares to the MLflow skills from the MLflow repository? cc @dustinvannoy-db
| @@ -0,0 +1,276 @@ | |||
| #!/usr/bin/env python3 | |||
| """ | |||
| PDF Generator - Convert HTML files to PDF locally. | |||
There was a problem hiding this comment.
Non-blocking, but this is an odd one, it is almost not very Databricks-specific at all?
| @@ -1,17 +1,26 @@ | |||
| --- | |||
| name: databricks-jobs | |||
There was a problem hiding this comment.
@simonfaltum These non-experimental skill addition we should also get into Genie Code
| @@ -0,0 +1,335 @@ | |||
| --- | |||
| name: databricks-vector-search | |||
There was a problem hiding this comment.
Non-blocking: I believe this one is actually no longer considered experimental in the Genie Code world? Then it shouldn't be here either.
I don't think this applies to other skills here.
@simonfaltum could you take a look at this one?
# Conflicts: # manifest.json
|
👋 Claude here on James's behalf. Merged The branch-protection setting dismisses stale approvals on new commits, so @dustinvannoy-db your earlier approval (May 21 02:21) was invalidated by the merge — sorry for the churn. Nothing substantive changed since: this push is just the merge commit plus the regenerated manifest. Mind re-approving when you have a moment? Still need approvals from one of @lennartkats-db / @fjakobs / @camielstee-db (CODEOWNERS for (comment posted by Claude) |
- CODEOWNERS: shrink the /experimental/ reviewer list from 13+ contributors to the four most-active maintainers per Lennart's request (lennartkats-db, simonfaltum, calreynolds, dustinvannoy-db). Drop the explanatory comment block too. - experimental/README.md: replace em-dash with colon in the warning header per the no-em-dashes-in-READMEs convention. - scripts/skills.py: add DISPLAY_NAME_OVERRIDES for skills whose hyphen-titlecase rendering breaks well-known capitalization (AI, AI/BI, MLflow, PDF). Regenerated the four affected agents/openai.yaml files. - databricks-execution-compute/SKILL.md: add a DABs-first note before the Decision Flow per Lennart's option (2) so users with a databricks.yml are pointed at `databricks bundle run` ahead of the three execution modes documented here. Non-blocking items left for follow-up: compute.py / pdf_generator.py "is this Databricks-specific enough?" questions, MLflow-evaluation overlap with the MLflow repo (pinged @simonfaltum), vector-search experimental status review. Co-authored-by: Isaac
|
👋 @lennartkats-db — Claude here on James's behalf. Addressing your review in Applied as suggested:
Non-blocking items, left as follow-ups (noted in the commit message):
Ambiguous comment, would appreciate a pointer: the May 22 15:34 "This uses all public APIs, which is good, but please save this one for the next version" — I couldn't tell which file/skill it referred to. Mind clarifying? CI is green; merge state is BLOCKED on review-decision until you / Dustin re-approve. Thanks for the thorough pass! (comment posted by Claude) |
|
|
||
| ## `ai_fix_grammar` | ||
|
|
||
| **Docs:** https://docs.databricks.com/aws/en/sql/language-manual/functions/ai_fix_grammar |
There was a problem hiding this comment.
@jamesbroadhead / Claude: please take another look at this; it's indeed our policy and documentation team guidance to strip these prefixes. We do the same in the CLI.
There was a problem hiding this comment.
@jamesbroadhead / Claude I saw an agent response to the review saying
"This uses all public APIs, which is good, but please save this one for the next version" — I couldn't tell which file/skill it referred to. Mind clarifying?
The comment refers to the experimental/databricks-genie directory. That entire directory (and any references to it) should be left out for now; we'll revisit.
…ricks-genie - Per Lennart's docs-team policy: strip the cloud/language prefix (`/aws/en/`) from `docs.databricks.com` URLs across experimental skills so links resolve cloud-agnostically. 19 files touched. - Per Lennart: remove `experimental/databricks-genie` entirely (Genie-Space-creation skill) — deferred to a future revision. Cleaned up references in `experimental/README.md` (skill list) and `experimental/databricks-agent-bricks/SKILL.md` + `agents/openai.yaml` (cross-references to the genie skill in the description, intro paragraph, and brick-comparison table). Manifest regenerated (26 skills now). Co-authored-by: Isaac
|
👋 @lennartkats-db — Claude here on James's behalf. Both follow-up items addressed in
Thanks for the clarification, and for the thorough review pass throughout. (comment posted by Claude) |
|
👋 Claude here on James's behalf — per-skill audit of the 5 non-blocking items from this PR's review threads. Each item gets a finding + recommendation; owners can decide what (if anything) becomes a follow-up PR. #7 — Inspected the script: 743 lines, 19 functions. Vast majority shadows what's already in the Databricks CLI:
The one differentiator is Recommendation: when promoting #8 — Searched Recommendation: defer until the MLflow-repo skill source is identified (internal repo? planned upstream?). #9 — Confirmed: the skill is ~80% Databricks-agnostic HTML → PDF tooling (weasyprint / wkhtmltopdf, HTML template patterns), ~20% Unity Catalog volume upload ( Recommendation: keep the skill, but either (a) trim to just the UC-volume upload step and link to standard HTML→PDF tooling externally, or (b) rename to something like #10 — Confirmed: there's no stable Recommendation: promote #11 — This isn't a d-a-s repo change — Genie Code consumes d-a-s via Recommendation: action is in the Genie Code product, not in d-a-s. @simonfaltum to verify the install path picks up Bonus finding (during audit): (comment posted by Claude) |
4 participants
Summary
Adds an
experimental/directory containing 18 agent skills from databricks-solutions/ai-dev-kitdatabricks-skills/, imported as a snapshot on a best-effort basis. Excluded:databricks-model-serving(TODO #1b — different surface than stable, heavy MCP coupling)databricks-spark-declarative-pipelines(TODO Fix skill inconsistencies and add required reading sections #5 — different surface than stabledatabricks-pipelines)databricks-genie— removed during review per @lennartkats-db; deferred to a future revisiondatabricks-lakebase-provisioned— not in the upstreamexperimentalbranch eitherThe manifest exposes both stable and experimental skills in a single
skillsmap. Each entry carries arepo_dirfield ("skills"or"experimental") that points to the directory the skill lives in. Consumers derive experimental state fromrepo_dir— there is no parallelexperimental_skillsmap and no per-skillexperimentalbool.Paired with databricks/cli#5243 which teaches
databricks aitools install(top-level) to:repo_dirand skip experimental entries by default,--experimental,--experimentalrequired.Experimental and stable skills install under their plain names (e.g.
~/.claude/skills/databricks-iceberg/); the upstream repo enforces name-uniqueness across both directories, so no install-side suffix is needed.Source
Final sync at merge time was
20a92a3ondatabricks-solutions/ai-dev-kit:experimental("tests: outcome-oriented rewrite across 7 skills + strip MCP tool_modules from all manifests").Initial import was
9c7a5b3(head of a-d-k PR #533 on theappkit-on-experimentalbranch). PR #533 has since merged intoexperimental(7b07f18). The branch went through periodic re-syncs during review to pull in upstream updates.The rename (
databricks-app-python→databricks-apps-python) is preserved in the merged version, which is what prevents a 3rd skill-name collision with d-a-s's stabledatabricks-apps.Direction caveat
In the Apr 28 thread (Slack link), Dustin's stated plan was to move
databricks-agent-skillsskills intoai-dev-kit'sexperimentalbranch as defaults. This PR went the other direction (a-d-k content → d-a-s/experimental). The plan post-merge is to invert the direction viagit subtree— see TODO #3 below and a-d-k RFC PR #530.TODOs / caveats for iteration
Name collisions. Resolved in this PR:
1a.
databricks-jobs— merged into stable. Imported the comprehensive reference content from a-d-k'sdatabricks-jobsskill intoskills/databricks-jobs/, bumping version to0.2.0. The merged skill keeps stable's scaffolding workflow +parent: databricks-corehierarchy + Codexagents/openai.yaml+ compatibility note, and adds the experimental's full task-types reference (9 types), trigger types (6), notifications/health/retries/queues, and 7 worked end-to-end examples. Layered structure: SKILL.md as overview + four reference files (task-types.md,triggers-schedules.md,notifications-monitoring.md,examples.md). The experimental copy is removed.With the single-map manifest shape, collisions are no longer possible —
_add_skillraises if the same skill name shows up under bothskills/andexperimental/, so any future drift fails generation loudly.1b.
databricks-model-serving— dropped from this PR. After a deep compare, the two skills cover almost entirely different surfaces: stable is ops-focused (manage existing endpoints via CLI); experimental is dev-focused (build & ship MLflow models / GenAI agents with autolog →mlflow.pyfunc.log_model→databricks.agents.deploy()→ query, with full Classical ML / Custom PyFunc /ResponsesAgent+ LangGraph / UCFunctionToolkit / VectorSearchRetrieverTool coverage). Near-zero content overlap. Experimental version also has heavy MCP-tool dependency (60+ refs to ai-dev-kit'smanage_serving_endpoint, etc., that don't exist in the d-a-s/databricks aitoolsflow). Follow-up: port the high-value dev-side content into the stable skill — classical-ml autolog patterns, Custom PyFunc signatures,ResponsesAgentpattern with thecreate_text_output_itemhelper-method gotcha,UCFunctionToolkit+VectorSearchRetrieverToolwith resource passthrough for auth, the Foundation Model API endpoint table. Strip MCP refs; replace with CLI/SDK equivalents. Owners: @databricks/eng-apps-devex (per CODEOWNERS).CODEOWNERS forResolved. Per @lennartkats-db review,experimental//experimental/owners are@lennartkats-db @simonfaltum @calreynolds @dustinvannoy-db(kept compact for now; broad participation invited via the broader collaborator set).No sync mechanism with upstream a-d-k.Resolved with a paired RFC. Two-part plan:ai-dev-kitintoexperimental/. Documented inexperimental/README.md.databricks-skills/imported/in a-d-k is agit subtreeof this repo'sexperimental/. RFC PR opened against a-d-k: RFC: subtree-sync skills from databricks-agent-skills/experimental databricks-solutions/ai-dev-kit#530 (draft). To make subtree work, d-a-s needs to publish anexperimental-onlybranch viagit subtree split --prefix=experimentalafter every push to main — that's a small workflow to add here in a follow-up PR. A one-shot preview branchexperimental-only-previewwas pushed to this repo to enable the RFC demo and should be deleted once the auto-publish workflow lands.No agent metadata.Resolved.scripts/skills.pyauto-generatesagents/openai.yaml+ copies shared assets for each experimental skill ongenerate, using SKILL.md frontmatter. ADISPLAY_NAME_OVERRIDESmap handles names whose hyphen-titlecase rendering breaks well-known capitalisation (AI Functions, AI/BI Dashboards, MLflow Evaluation, Unstructured PDF Generation — fixed per @lennartkats-db review). Stubs are only written when missing so upstream a-d-k can override by shipping its own files.databricks-pipelineswas deliberately excluded.databricks-pipelinesskill under that name, but it does shipdatabricks-spark-declarative-pipelinescovering the same product. After a deep compare, that experimental version covers a different surface than stable. Removedexperimental/databricks-spark-declarative-pipelines/from this PR. Follow-up TODO (post-merge): port the high-value pieces into stableskills/databricks-pipelines/— DLT migration guide, workflow A/B/C decision matrix, per-language performance reference, language-selection rules. Strip MCP-tool refs. Owners: @lennartkats-db / @camielstee-db (per CODEOWNERS).spark-python-data-sourcenaming exception.Versioning.Resolved. Bumped theextract_version_from_skillfallback inscripts/skills.pyfrom0.0.0→0.0.1so the manifest never reports0.0.0. Applies to skills with no explicitversion:in their SKILL.md frontmatter. Sync-safe: when upstream a-d-k eventually adds version fields, those win; until then, the manifest reports the floor.installed_dirfor experimental skills.~/.claude/skills/<name>-experimental/. Replaced with: experimental skills install under their plain name (e.g.~/.claude/skills/databricks-iceberg/). Upstream guarantees name-uniqueness acrossskills/andexperimental/, and the manifest generator (scripts/skills.py _add_skill) raises on any future collision so drift fails loudly rather than silently overwriting. Cli-side change shipped in databricks/cli#5243 (6d9c479f— dropSourceNameand the suffixing innormalizeManifest).Excluded a-d-k content.Confirmed scope. Excluded:TEMPLATE/(template, not a skill),install_skills.sh+install_genie_code_skills.py(a-d-k's installers — we use the cli installer instead),databricks-builder-app/(a Python app for a-d-k's builder UI),databricks-mcp-server/(the a-d-k MCP server — separate concern from skills),databricks-tools-core/(Python lib used by a-d-k tooling — no experimental skill references it),hooks/hooks.json(a-d-k plugin lifecycle hooks tied to${CLAUDE_PLUGIN_ROOT}/.claude-plugin/setup.sh/check_update.sh— plugin-specific, not skill content), plus top-level repo metadata (.github/,LICENSE.md,README.md,VERSION,install.{sh,ps1}, etc.). Verified no experimental skill cross-references any excluded path.README placement.Verified.experimental/README.mdretains the adapted a-d-k skill list with a top warning block; the rootREADME.mdhas an "Experimental Skills" section with an install-by-name example. Install commands use the new top-level surface:databricks aitools install [name] --experimental.Manifest shape.Resolved. Replaced the original two-map design (top-levelskills+experimental_skillsplus per-skillexperimentalbool) with a singleskillsmap where each entry'srepo_dirfield is the source of truth. The manifest generator (scripts/skills.py) raises a clear error if the same skill name appears under bothskills/andexperimental/.Test plan
python3 scripts/skills.py generateregenerates the manifest cleanly.python3 scripts/skills.py validatepasses.databricks aitools install(no flag) installs only stable skills.databricks aitools install --experimentalinstalls both.databricks aitools install databricks-icebergerrors because it's experimental.databricks aitools install databricks-iceberg --experimentalinstalls that one skill.Post-merge follow-ups (reviewer-flagged)
references/directories for consistency across skills (Dustin + Lennart).databricks-execution-compute/scripts/compute.py: when promoting out of experimental, rely on the Databricks CLI rather than the bundled compute script (Lennart, non-blocking).databricks-mlflow-evaluation: review overlap with MLflow-repo skills (Lennart, non-blocking; cc'd @simonfaltum).databricks-unstructured-pdf-generation: re-evaluate inclusion — "not very Databricks-specific" (Lennart, non-blocking; cc'd @dustinvannoy-db).databricks-vector-search: "no longer experimental in Genie Code world" — move to stable or out (Lennart, non-blocking; cc'd @simonfaltum).skills/databricks-jobs/SKILL.md: non-experimental skill additions should also propagate into Genie Code (Lennart, for @simonfaltum).This pull request and its description were written by Claude.