-
Notifications
You must be signed in to change notification settings - Fork 6.9k
[ci] claude in ci. #13297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[ci] claude in ci. #13297
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| # PR Review Rules | ||
|
|
||
| Rules for Claude to check during PR reviews. Focus on correctness — style is handled by ruff. | ||
|
|
||
| ## Code style | ||
| - Inline logic — minimize small helper/utility functions. A reader should follow the full flow without jumping between functions. | ||
| - No defensive code or unused code paths — no fallback paths, safety checks, or config options "just in case". | ||
| - No silent fallbacks — raise a concise error for unsupported cases rather than guessing user intent. | ||
|
|
||
| ## Dependencies | ||
| - No new mandatory dependencies without prior discussion. | ||
| - Optional deps must be guarded with `is_X_available()` and have a dummy in `utils/dummy_*.py`. | ||
| - Never use `einops` — rewrite with native PyTorch (`reshape`, `permute`, `unflatten`). | ||
|
|
||
| ## Models | ||
| - All layer calls must be visible directly in `forward()` — no helper functions hiding `nn.Module` calls. | ||
| - No NumPy operations in `forward()` — breaks `torch.compile` with `fullgraph=True`. | ||
| - No hardcoded dtypes (e.g. `torch.float32`, `torch.bfloat16`) in forward — use input tensor dtype or `self.dtype`. | ||
| - Attention must use `dispatch_attention_fn`, not `F.scaled_dot_product_attention` directly. | ||
| - Every `__init__` parameter in a `ModelMixin` subclass must be captured by `register_to_config`. | ||
| - New classes must be registered in `__init__.py` with lazy imports (both `_import_structure` and `_lazy_modules`). | ||
|
|
||
| ## Pipelines | ||
| - Must inherit from `DiffusionPipeline`. | ||
| - `@torch.no_grad()` on pipeline `__call__` — forgetting this causes OOM from gradient accumulation. | ||
| - Do NOT subclass an existing pipeline for a variant (e.g. don't subclass `FluxPipeline` for `FluxImg2ImgPipeline`). | ||
| - Support `output_type="latent"` for skipping VAE decode. | ||
| - Support `generator` parameter for reproducibility. | ||
|
|
||
| ## Copied code | ||
| - Never edit a `# Copied from` block directly — run `make fix-copies` to propagate changes from the source. | ||
| - Remove the `# Copied from` header to intentionally break the sync link. | ||
|
|
||
| ## Common mistakes (add new rules below this line) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| name: Claude PR Review | ||
|
|
||
| on: | ||
| issue_comment: | ||
| types: [created] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we also be able to @claude on the pr review comment? (not just issue comments) this gives it more context for stuff like we want it to add a rule based on this pattern eetc |
||
|
|
||
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
| issues: read | ||
|
|
||
| jobs: | ||
| claude-review: | ||
| if: | | ||
| github.event.issue.pull_request && | ||
| github.event.issue.state == 'open' && | ||
| contains(github.event.comment.body, '@claude') && | ||
| (github.event.comment.author_association == 'MEMBER' || | ||
| github.event.comment.author_association == 'OWNER' || | ||
| github.event.comment.author_association == 'COLLABORATOR') | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: anthropics/claude-code-action@v1 | ||
| with: | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| claude_args: | | ||
| --append-system-prompt "Review this PR against the rules in .ai/review-rules.md. Focus on correctness, not style (ruff handles style). Only review changes under src/diffusers/." | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add something like |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are a lot of overlaps from https://github.com/huggingface/diffusers/blob/main/.ai/skills/model-integration/SKILL.md
I think maybe we have a separate subdoc for
models.mdthat contains the gotchas, common conventions, rules, structures etc and we can link it from everywhere: for now bothAGENTS.mdand the model-integration skillswhat do you think? cc @stevhliu here too
these contents that are already in
AGENTS.mddoes not need to be included here again, I think claude CI would have read the AGENTS.md. anyways no?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i agree here, and i think it'd be cleaner and easier to maintain to provide a path to content that is already in
AGENTS.md