[.ai] add self-review skill#13917
Open
yiyixuxu wants to merge 7 commits into
Open
Conversation
… the agent guides - New `self-review` skill mirroring the `@claude` CI review (rubric from review-rules.md, call-path dead-code analysis), report-only, with the report flagging what to fix before submitting (blocking + dead code) vs what to leave for the actual review. - Remove the WIP `parity-testing` skill; preserve its pitfalls as `model-integration/pitfalls.md` (numerical-discrepancy reference). - model-integration: restructure around a grouped checklist, default-to-modular, an overall file-structure sketch (details deferred to the guides), a fresh-conversion `Model parity test` example (internal, not shipped), and a filled-in weight/checkpoint-conversion section. - Centralize the loading rule (from_pretrained / from_single_file, no custom loaders) in models.md; add per-folder File structure sections to models.md / pipelines.md; default-to-modular note in pipelines.md. - AGENTS.md: dedicated 'Self-review before a PR' and 'Reference guides' sections. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t entries Trim pitfall #6 to the essential point (small dtype diffs compound into a large final difference), remove the `/tmp` model-storage and incomplete-injection-test pitfalls, and renumber 1-16 with cross-references updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
With the parity-testing skill gone, remove the stale-test-fixtures pitfall (saved tensors / cross-pipeline fixtures no longer apply) and de-jargon the noise-dtype detection note. Keeps the pitfalls list generic to numerical discrepancy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the variable-shadowing and decoder-config pitfalls and the noise-dtype 'Detection' aside, tighten the remaining entries, renumber 1-12 (cross-refs updated), and reframe the intro as a non-checklist reference list of possible causes to consult only when outputs don't match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
yiyixuxu
commented
Jun 11, 2026
| @@ -1,172 +0,0 @@ | |||
| --- | |||
| name: testing-parity | |||
Collaborator
Author
There was a problem hiding this comment.
i removed this skill for now since it is a WIP
yiyixuxu
commented
Jun 11, 2026
yiyixuxu
commented
Jun 11, 2026
yiyixuxu
commented
Jun 11, 2026
| @@ -0,0 +1,48 @@ | |||
| --- | |||
| name: self-review | |||
Collaborator
Author
There was a problem hiding this comment.
added a self-review skill here
mostly doing the samething our claude CI do, but for self-review I think they should focus on blocking issues & clean up dead code
we may ask contributor to provide the self-review summary in PR submission as well
Replace the retired parity-testing skill with self-review in the skills list, and add a 'Self-review before opening' step to the AI-assisted contributions section: run the self-review skill / review-rules, fix blocking issues + dead code, and treat the @claude CI review as a non-authoritative helper (note any intentional skips in the PR for the reviewer). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
the
documentation
|
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. |
sayakpaul
approved these changes
Jun 11, 2026
| ## Integration checklist | ||
|
|
||
| ### File structure for a new model | ||
| - [ ] **Transformer model** |
Member
There was a problem hiding this comment.
Since it can be a VAE, too.
Suggested change
| - [ ] **Transformer model** | |
| A pipeline in Diffusers (be it standard or modular) will have multiple components. These components can be models, schedulers, processors, etc. | |
| - [ ] **Transformer model** |
| - [ ] **Scheduler** — reuse an existing scheduler, or add a custom one | ||
| - [ ] **Pipeline** | ||
| - [ ] Implement the pipeline — see [modular.md](../../modular.md) for modular pipeline, or [pipelines.md](../../pipelines.md) for standard pipeline | ||
| - [ ] Add a LoRA mixin if applicable |
Member
There was a problem hiding this comment.
Maybe better if this is skipped in the initial implementation. Easier to review and test.
| All weights load through the standard paths — `from_pretrained`, or `from_single_file` (add `FromSingleFileMixin` + a weight-mapping) for an original-format single checkpoint. No custom `from_pretrained`, no manual runtime loading. See the loading rule in [models.md](../../models.md). | ||
|
|
||
| ### Testing | ||
| Common conversion patterns to watch for: |
Member
There was a problem hiding this comment.
Suggested change
| Common conversion patterns to watch for: | |
| Common conversion patterns to watch for model-level components: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.