Restructure skill files and reference guides for CLI based installation. - #14454
Restructure skill files and reference guides for CLI based installation. #14454DN6 wants to merge 5 commits into
Conversation
github-actions
Bot
added
size/L
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
DN6
changed the title
There was a problem hiding this comment.
thanks for the PR!
I left some questions in the PR
Additionally, I wonder, does it makes sense to add a utils script (e.g., check_ai.py or something), and then we can prompt the agents in AGENTS.md as well as each individual file to get them to check everything is set up correctly before it does anything? it would just check links etc
| 4. **Subclassing an existing pipeline for a variant.** Don't use an existing pipeline class (e.g. `FluxPipeline`) to override another (e.g. `FluxImg2ImgPipeline`) inside the core `src/` codebase. Each pipeline lives in its own file with its own class, even if it shares 90% of `__call__` with a sibling. Convention across diffusers — flux, sdxl, wan, qwenimage — is duplicated `__call__` between img2img / text2img / inpaint variants, not subclassing. Reuse private utilities (shared schedulers, prep functions) but not the pipeline class itself. | ||
|
|
||
| 5. **Copying a method from another pipeline without `# Copied from`.** When you reuse a method like `encode_prompt`, `prepare_latents`, `check_inputs`, or `_prepare_latent_image_ids` from another pipeline, add a `# Copied from` annotation so `make fix-copies` keeps the two in sync. Forgetting it means future refactors to the source drift away from your copy silently — and reviewers waste time spotting near-identical code that should have been linked. The annotation grammar (decorator placement, rename syntax with `with old->new`, etc.) is implemented in [`utils/check_copies.py`](../utils/check_copies.py) — read it for the exact rules. | ||
|
|
There was a problem hiding this comment.
I think links to the source like this utils/check_copies.py
would not work anymore when the skill is installed in cli
There was a problem hiding this comment.
Additionally, we could make it so that all the hyperlinks and reference links are checked and verified by check_ai.py and make it a part of our CI.
| 1. **Reference repo** — ask for the github link. If they've already set it up locally, ask for the path. Otherwise, ask what setup steps are needed (install deps, download checkpoints, set env vars, etc.) and run through them before proceeding. | ||
| 2. **Inference script** — ask for a runnable end-to-end script for a basic workflow first (e.g. T2V). Then ask what other workflows they want to support (I2V, V2V, etc.) and agree on the full implementation order together. | ||
| 3. **Standard vs modular** — **default to modular.** [Modular Diffusers](../../modular.md) is the preferred implementation for new pipelines; the standard `DiffusionPipeline` is still supported but no longer the default. We prefer modular especially for models that don't fit a fixed task-based structure (modality baked into the checkpoint) or that are actively evolving. | ||
| 3. **Standard vs modular** — **default to modular.** [Modular Diffusers](references/modular.md) is the preferred implementation for new pipelines; the standard `DiffusionPipeline` is still supported but no longer the default. We prefer modular especially for models that don't fit a fixed task-based structure (modality baked into the checkpoint) or that are actively evolving. |
There was a problem hiding this comment.
does this means we cannot click through to the linked guides (like the [Modular Diffusers] here) anymore when we browse the skill file on github? I don't think it's a deal breaker if it's the case, but wonder if there is an easy fix
There was a problem hiding this comment.
Yes, but it's set up this way so that we maintain a single set of reference guides. We could duplicate the guide files and place them as needed in the individual skill folders. Then the should links would work when browsing on github.
There was a problem hiding this comment.
Another point is the discoverability of the agents. Without this mechanism in place, they won't be able to do so. Cc: https://huggingface.slack.com/archives/C065E480NN9/p1786544533688239
| - `model-integration` — adding a new model or pipeline to diffusers end-to-end (file structure, integration checklist, testing layout, weight conversion) | ||
| - `self-review` — review your changes against the project rules before opening a PR | ||
| - **Setup commands**: | ||
| - `make codex` — wire up skills for OpenAI Codex (under `.agents/`) |
There was a problem hiding this comment.
what happens for people who already had claude set up with make claude and then did a pull to the latest diffusers (after the PR is merged)? would their current setting just silently stopped working correctly?
There was a problem hiding this comment.
Ah, yeah this is tricky. Since the make approach used symlinks, it might cause as issue with how the paths to the guides resolve in the skill.
Another problem is that since .claude/skills symlinks to .ai/skills, I think if someone were to run the install with the diffusers-cli, the skill files written to .claude/skills/diffusers would actually get written to .ai/skills/diffusers/<skill name> 🤔
I'm not sure what the best way to fix is. We could include instructions in AGENTS.md to remove the symlinks perhaps?
| @@ -0,0 +1,15 @@ | |||
| { | |||
There was a problem hiding this comment.
(curiosity): why is this needed?
| @@ -0,0 +1,14 @@ | |||
| { | |||
There was a problem hiding this comment.
Should there be one for Codex too?
| ## Related | ||
|
|
||
| - [`diffusers-cli` skill](../diffusers-cli/SKILL.md) — once your block is uploaded, `schema`/`run` | ||
| - the `diffusers-cli` skill — once your block is uploaded, `schema`/`run` |
There was a problem hiding this comment.
Without the link or reference, would it not leave the agent to guess?
| - Location: `tests/modular_pipelines/<model>/test_modular_pipeline_<model>.py` (one test class per blockset / pipeline variant). | ||
| - Subclass `ModularPipelineTesterMixin` (from `..test_modular_pipelines_common`) — it runs the pipeline end-to-end (call signature, batch consistency, float16, device placement) against a tiny checkpoint. | ||
| - Set `pipeline_class`, `pipeline_blocks_class`, `pretrained_model_name_or_path`, `params` / `batch_params`, and implement `get_dummy_inputs(seed=0)`. Set `expected_workflow_blocks` to pin the block name → class ordering per workflow. | ||
| - Set `pipeline_class`, `pipeline_blocks_class`, `pretrained_model_name_or_path`, `params` / `batch_params`, and implement `get_dummy_inputs(seed=0)`. Set `expected_workflow_blocks` to pin the block name → class ordering per workflow (only for blocksets with a `_workflow_map` — with a single workflow the list would just restate the class definition), and `expected_workflow_defaults` to pin each workflow's components, pipeline configs, and inputs — required ones by name, optional ones with their defaults. A pipeline without workflows pins its full blockset under the `None` key. An optional `component_configs` entry pins config values of `from_config` components against their creating spec (e.g. the guider scale that tells a base and a distilled preset apart); pretrained components take their config from the repo, so there is nothing block-level to pin. |
|
|
||
| - **Usage docs.** New or changed public behavior — a new pipeline/model, a new argument, changed defaults, a renamed API — should have matching updates in `docs/`, docstrings, and examples. Flag any that now describe outdated behavior or that are missing for the new surface. | ||
| - **Agent docs.** If the review turns up a rule, pattern, or common gotcha that isn't written down yet — especially one the author got wrong or that you had to reason out — propose adding it to the relevant agent guide ([AGENTS.md](AGENTS.md), [models.md](models.md), [pipelines.md](pipelines.md), [modular.md](modular.md), a skill, or this file) so the next contributor/agent gets it for free instead of repeating the mistake. Human review comments on the PR are a good source for these: if a human reviewer pointed something out and your review missed it, that usually indicates a doc gap — figure out what's missing and propose the addition. | ||
| - **Agent docs.** If the review turns up a rule, pattern, or common gotcha that isn't written down yet — especially one the author got wrong or that you had to reason out — propose adding it to the relevant agent guide (`AGENTS.md`, [models.md](models.md), [pipelines.md](pipelines.md), [modular.md](modular.md), a skill, or this file) so the next contributor/agent gets it for free instead of repeating the mistake. Human review comments on the PR are a good source for these: if a human reviewer pointed something out and your review missed it, that usually indicates a doc gap — figure out what's missing and propose the addition. |
There was a problem hiding this comment.
Why are we just removing it from AGENT.md? Additionally, wouldn't we have a similar problem like https://huggingface.slack.com/archives/C065E480NN9/p1786544533688239?
| 4. **Subclassing an existing pipeline for a variant.** Don't use an existing pipeline class (e.g. `FluxPipeline`) to override another (e.g. `FluxImg2ImgPipeline`) inside the core `src/` codebase. Each pipeline lives in its own file with its own class, even if it shares 90% of `__call__` with a sibling. Convention across diffusers — flux, sdxl, wan, qwenimage — is duplicated `__call__` between img2img / text2img / inpaint variants, not subclassing. Reuse private utilities (shared schedulers, prep functions) but not the pipeline class itself. | ||
|
|
||
| 5. **Copying a method from another pipeline without `# Copied from`.** When you reuse a method like `encode_prompt`, `prepare_latents`, `check_inputs`, or `_prepare_latent_image_ids` from another pipeline, add a `# Copied from` annotation so `make fix-copies` keeps the two in sync. Forgetting it means future refactors to the source drift away from your copy silently — and reviewers waste time spotting near-identical code that should have been linked. The annotation grammar (decorator placement, rename syntax with `with old->new`, etc.) is implemented in [`utils/check_copies.py`](../utils/check_copies.py) — read it for the exact rules. | ||
|
|
There was a problem hiding this comment.
Additionally, we could make it so that all the hyperlinks and reference links are checked and verified by check_ai.py and make it a part of our CI.
| - [modular.md](modular.md) — modular pipeline conventions, patterns, common mistakes | ||
| - [testing.md](testing.md) — test conventions: required test layers, tester mixins, dummy-component rules. When a PR adds or changes tests, check them against this guide. | ||
| - [skills/model-integration/pitfalls.md](skills/model-integration/pitfalls.md) — known pitfalls causing numerical discrepancies between the reference implementation and the diffusers port (dtype mismatches, config assumptions, etc.) | ||
| - [skills/model-integration/pitfalls.md](https://github.com/huggingface/diffusers/blob/main/.ai/skills/model-integration/pitfalls.md) — known pitfalls causing numerical discrepancies between the reference implementation and the diffusers port (dtype mismatches, config assumptions, etc.) |
There was a problem hiding this comment.
Why are we just removing it from AGENT.md? Additionally, wouldn't we have a similar problem like https://huggingface.slack.com/archives/C065E480NN9/p1786544533688239?
| - **Guidelines** — `.ai/AGENTS.md` is loaded into every agent session through the root `AGENTS.md` / `CLAUDE.md` | ||
| symlinks; the reference guides under `.ai/references/` are read on demand when a link is followed: |
There was a problem hiding this comment.
I thought we were getting rid of the symliking thing?
Makes diffusers-cli skills add the canonical way to install skills, replacing make claude / make codex.
| Without a target flag, the CLI installs for whichever agent launched it, or for every agent when it can't tell. | ||
| For Claude Code the skills are written as a plugin bundle at `.claude/skills/diffusers/`, so they are namespaced | ||
| as `/diffusers:<skill name>`; Codex and Cursor get `.agents/skills/<skill name>/`. | ||
|
|
There was a problem hiding this comment.
Include an example to update the skill?
| class TestSkillDescription: | ||
| def test_reads_folded_block(self): | ||
| skill_md = "---\nname: demo\ndescription: >\n First line\n second line.\n---\n\n# Body\n" | ||
| assert _skill_description(skill_md) == "First line second line." | ||
|
|
||
| def test_reads_single_line(self): | ||
| assert _skill_description("---\nname: demo\ndescription: Does a thing.\n---\n") == "Does a thing." | ||
|
|
||
| def test_returns_empty_without_frontmatter(self): | ||
| assert _skill_description("# Just a heading\n") == "" | ||
|
|
||
| def test_returns_empty_without_description(self): | ||
| assert _skill_description("---\nname: demo\n---\n") == "" |
There was a problem hiding this comment.
But is this a meaningful thing to test?
| instead. | ||
| 2. A guide a skill cites as `references/<guide>.md` exists in `.ai/references/`, since that is where the installer | ||
| copies it from. | ||
| 3. Every other relative link resolves on disk, and each skill's frontmatter `name` matches its directory. |
There was a problem hiding this comment.
And this should probably be done from the perspective of how an agent would access the link.
| Run `python utils/check_ai.py`. | ||
| """ | ||
|
|
||
| import re |
There was a problem hiding this comment.
Let's make it a part of our CI and expose through the Makefile.
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| """Check that the agent guides and skills under `.ai/` still hold together. |
There was a problem hiding this comment.
nice! can we also:
- in
AGENTS.md, teach the agent to run this check at the start of a session and let user know if there is an issue, what skills are installed etc - add a line to the contribution guide to ask the human user to always double check with their agent to confirm ther setup is correct
|
BTW I think we should update the README now with this PR (to include setup using agents) https://github.com/huggingface/diffusers#installation |
4 participants
What does this PR do?
Some housekeeping on skills, reference guides, and
AGENTS.md.Right now the guides and
AGENTS.mdpoint at each other. e.g.review-rules.mdandmodels.mdlinkback up to
AGENTS.md, which links back down to them. This means means a guide only reallyworks inside a clone of the repo. It's better to keep each guide as a standalone module that
AGENTS.mdand anySKILL.mdcan "import". Then skills can also ship the guides they need, and an agentcan use it without cloning diffusers first.
This PR:
.ai/references/soAGENTS.mdand the skills can point at the same files,and drops the links from the guides back up to
AGENTS.md.diffusers-cli skills addto fetch the references guides along with the skill files.diffusers-cli skills addthe canonical way to install skills, replacingmake claude/make codex.Adds
--claude/--codex/--cursorto pick a target, andskills listnow prints what eachskill is for.
Skills for Claude will now be namespaced by the plugin name. e.g.
/diffusers:self-reviewso they are easy to find when using the coding agent. Codex doesn't not add this plugin name prefix, so it is worth considering whether we want to prefix our skill names. e.gself-review -> diffusers-reviewAGENTS.mdsetup instructions to match.Fixes # (issue)
Before submitting
self-reviewskill on the diff?documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.