epam / epam/ai-dial-client-python

Support DIAL agent skills (/v2/skills) in the Python client

Open
#135 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
7
Forks
2
Avg merge
1d 18h
Merged PRs (30d)
8

Description

### Name and Version

aidial-client 0.16.1

### What is the problem this feature will solve?

DIAL Core shipped **"DIAL Folder As Resource"** ([epam/ai-dial-core#1633](https://github.com/epam/ai-dial-core/issues/1633), closed). Agent skills are now a first-class `SKILL` resource type in Core: a *folder* — marked by a `.dial-resource` JSON marker that points at an immutable `v/{versionId}/` subtree — containing a mandatory `SKILL.md` plus an arbitrary file hierarchy. Core serves it through a new `/v2/skills/**` route family with whole-resource, grouping-folder, single-file and metadata operations, server-side frontmatter validation, and integration with the existing sharing and publication flows.

The Python client cannot reach any of it. `Dial` / `AsyncDial` expose `files`, `prompts`, `metadata`, `application`, `toolset`, `model`, `bucket` and more, but nothing for `/v2/skills` — and the library assumes `/v1/` throughout, so this is not a matter of adding one more resource class:

- `API_PREFIX = "v1/"` is a module constant in `aidial_client/_constants.py` that `resources/files.py`, `resources/prompts.py` and `helpers/storage_resource.py` `urljoin` directly. Skills live under `/v2/`, so the prefix has to become per-resource.
- `StorageResourceType = Literal["files", "conversations", "prompts"]` gates `safe_parse_storage_resource`, which rejects anything else outright — `skills/{bucket}/{path}` cannot currently be parsed at all.
- `safe_parse_storage_resource` also rejects a bucket-root URL: `files/mybucket` and `files/mybucket/` both raise `InvalidDialURLError("Missing bucket in URL")`, because the guard `len(api_path.parents) < 3` computes `bucket_path` from a *parent* and so cannot represent a path that **is** the bucket. Core's children-metadata route explicitly allows an empty `{path}` to list the bucket root, so this blocks the natural first call.
- `Metadata.get` is `@overload`-typed over that same `Literal` and closes with `assert_never` in `_get_cast_to`, and `METADATA_PREFIX` is *derived* from `API_PREFIX` — so `/v2/metadata/skills/...` needs its own entry point and its own prefix, not an extra overload.
- Two payload shapes the library has not needed before: a streamed `application/zip` reader for the whole-resource GET, and a `multipart/form-data` writer with **one part per file** for the whole-resource PUT.

Without this, every consumer of DIAL skills has to drive `httpx` off the injected client's `base_url` and `auth_headers()`, forking transport concerns the library already solves once for everyone — retries, timeout policy, error translation into the `DialException` hierarchy, and the percent-encoding just fixed in #124.

### What is the feature you are proposing to solve the problem?

A `skills` resource (`Skills` / `AsyncSkills`) on `Dial` / `AsyncDial` covering Core's skill API, delivered as two child issues over shared groundwork.

**Endpoint matrix** (Core `docs/open_api_core.yaml`, tag `Skills`):

| # | Method | Path | Child |
|---|--------|------|-------|
| 1 | `GET` | `/v2/metadata/skills/{bucket}/{path}` — list skills + grouping folders | Read |
| 2 | `GET` | `/v2/metadata/skills/{bucket}/{path}/files/{filePath}` — list files in a skill | Read |
| 3 | `GET` | `/v2/skills/{bucket}/{path}` — whole skill as `application/zip` | Read |
| 4 | `GET` | `/v2/skills/{bucket}/{path}/files/{filePath}` — one file | Read |
| 5 | `PUT` | `/v2/skills/{bucket}/{path}` — replace whole skill (multipart) | Write |
| 6 | `DELETE` | `/v2/skills/{bucket}/{path}` — delete whole skill | Write |
| 7 | `PUT` | `/v2/skills/{bucket}/{path}/` — create grouping folder | Write |
| 8 | `DELETE` | `/v2/skills/{bucket}/{path}/` — delete grouping folder (only if empty) | Write |
| 9 | `PUT` | `/v2/skills/{bucket}/{path}/files/{filePath}` — add/replace one file | Write |
| 10 | `DELETE` | `/v2/skills/{bucket}/{path}/files/{filePath}` — remove one file | Write |
| 11 | `GET` | `/v2/skills/{bucket}/{path}/` — answers `400` by design; use the metadata listing | n/a |

**Shared groundwork** (lands with whichever child ships first):

1. `_constants.py` — `API_V2_PREFIX = "v2/"` and `METADATA_V2_PREFIX = "v2/metadata/"`.
2. `helpers/storage_resource.py` — add `V2StorageResourceType = Literal["skills"]` and `AnyStorageResourceType`, keeping the v1 union intact so `Metadata._get_cast_to`'s `assert_never` stays exhaustive; give `safe_parse_storage_resource` and `DialStorageResourceMixin` an `api_prefix` (defaulting to `API_PREFIX`, also used by `_prepare_download_request`); and add an opt-in `allow_bucket_root` flag that lets a `{type}/{bucket}` path parse with `bucket_path == ""` and `filename is None`. The flag is opt-in rather than automatic because a two-segment path is ambiguous — `files/my-file.txt` has the same shape and must stay a missing-bucket error (`test_get_api_path_missing_bucket` pins this) — so only callers whose endpoint accepts an empty path enable it.
3. `_client.py` — an `api_v2_url` property mirroring `api_url`, `self.skills` wired into both `_init_resources`, and `my_skills_home()` alongside `my_files_home()` / `my_prompts_home()`.
4. `resources/__init__.py` — export `Skills` / `AsyncSkills`.

**Tracker**

| Child issue | Scope |
|---|---|
| #136 — Read side | `get_metadata`, `list_files`, `get_file`, `download` + the groundwork above |
| #137 — Write side | whole-resource `PUT`/`DELETE`, grouping folders, single-file `PUT`/`DELETE` |
| #140 — Follow-up | drop the `node_type` workaround #136 ships, once the DIAL Core fix is in a release |

**Why split this way:** the four GETs are a complete, independently useful capability — browse a bucket, enumerate a skill, fetch one file, export the whole skill — and they carry no unresolved API-shape questions. The writes do: multipart part-naming, responses that are an empty body plus an `ETag` header only, and Core's unusual `If-Match` semantics where *omitting* the header means create-only-if-absent while `*` overwrites. Those deserve their own discussion rather than holding up the reads.

**Notes for whoever picks this up**

- Every `/v2/skills` operation is marked `x-preview: true` in Core's `open_api_core.yaml`. Say so in the README.
- Core's `open_api_core.yaml` is lossy for the whole-resource `PUT`: it advertises a single `file` binary part, but `ComplexResourceController.put` keys uploads by `upload.filename()`, i.e. **one part per file, part filename = the relative path inside the skill**.
- Path segments `files` and `v` are **reserved** by Core — no skill or grouping folder may be named either — so the `{path}/files/{filePath}` grammar stays unambiguous after percent-encoding.
- Per `CLAUDE.md`'s PR checklist: public-API additions must also update `README.md` with sync + async usage and sample response objects, and export the new classes from `resources/__init__.py`.

### What alternatives have you considered?

**Extend `Files` / `Metadata` with a `"skills"` resource type instead of a new resource.** Rejected. `Metadata.get`'s `@overload`s and `assert_never` are v1-shaped, `METADATA_PREFIX` derives from `API_PREFIX`, and `/v2/metadata/skills/...` returns a different container than `FileMetadata`. More fundamentally the payloads do not fit v1 blob semantics: the whole-resource GET is a ZIP archive and the whole-resource PUT is multipart-with-one-part-per-file, neither of which `Files` models.

**Let each consumer drive `httpx` directly off `base_url` + `auth_headers()`.** Rejected. It forks retries, timeout policy, error translation into `DialException` / `ResourceNotFoundError` / `EtagMismatchError`, and percent-encoding (#124) into every consumer, which is exactly what this library exists to centralise.

**Expose only the whole-resource ZIP endpoint and let callers unpack it.** Rejected as the primary path. It materialises every bundled file — including binaries a caller will never open — and forces the archive through memory. The metadata listing plus a per-file GET is strictly cheaper for the common "read the manifest, then fetch what's actually referenced" access pattern. The ZIP endpoint is still worth having for export, which is why it is in the read child.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.