Repository navigation
Conversation
Three merge-blocking mistakes that signal a contributor hasn't read AGENTS.md and the docs it links to. Non-exhaustive — failing one is enough to send a PR back, but passing them all doesn't make a PR good. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
yiyixuxu
commented
May 21, 2026
| it links to. Failing one of these is enough to send a PR back; passing them | ||
| all doesn't make the PR good, it just means it's worth a proper review. | ||
|
|
||
| 1. **New mandatory dependency.** Any new import outside the existing |
Collaborator
Author
Contributor
There was a problem hiding this comment.
Claude finished @yiyixuxu's task in 3m 7s —— View job
Added 6 additional tripwires to review-rules.md
- Read current
.ai/review-rules.mdand PR diff - Explore the agents/tripwires codebase for context (read AGENTS.md, models.md, pipelines.md, modular.md, parity-testing docs)
- Draft additional tripwires based on patterns from the docs
- Edit
.ai/review-rules.mdwith new tripwires
After reading through all the .ai/ documentation files, I identified 6 additional merge-blocking patterns that are well-documented as gotchas but weren't yet in the Tripwires section. Here's what I added (tripwires 4–9):
| # | Tripwire | Why it blocks | Source |
|---|---|---|---|
| 4 | Unconditional torch.float64 / .double() |
Breaks MPS and NPU entirely | models.md gotcha #5 |
| 5 | Missing @torch.no_grad() on pipeline __call__ |
GPU OOM from gradient accumulation | pipelines.md gotcha #2 |
| 6 | Missing import registration in __init__.py |
ImportError for users |
models.md gotcha #1 |
| 7 | Graph-breaking ops in model forward |
Breaks torch.compile with fullgraph=True |
models.md coding style |
| 8 | Modular blocks importing from diffusers.pipelines.* |
Core architecture violation | modular.md gotcha #1 |
| 9 | Hardcoded dtype in model forward |
Breaks GGUF / quantized loading | models.md gotcha #4 |
Each entry follows the same format as the existing tripwires: what to look for, why it matters, and a link back to the detailed documentation.
Patterns I considered but left out (better as review feedback than hard tripwires):
- Subclassing existing pipelines for variants (pipelines.md gotcha Convert glide weights #4) — a convention violation but not always a contributor-ignorance signal
- Capability flags without implementation (models.md gotcha make from hub import work #3) — important but more subtle, covered well in the Common mistakes path
- Test model paths in contributor repos (modular.md gotcha add unet ldm in init #6) — easy to fix, not a sign of deeper misunderstanding
Contributor
|
❌ Failed to open follow-up PR with the Claude edits — see workflow run. |
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
1 participant
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.