From story-to-ship
Use when adding new classes, refactoring code, or reviewing PRs for Particle-Viewer.
How this skill is triggered — by the user, by Claude, or both
Slash command
/story-to-ship:architecture-reviewThe summary Claude sees in its skill listing — used to decide when to auto-load this skill
```
DEPENDENCIES FLOW INWARD -- INNER LAYERS NEVER DEPEND ON OUTER LAYERS
YOU MUST verify dependency direction for every new class and every refactor. A layer violation in a PR means the PR is NOT mergeable until it is fixed. No exceptions.
Violating the letter of this rule is violating the spirit of this rule.
Announce at start: "I am using the architecture-review skill to review [component/file]."
For the Particle-Viewer 4-layer inward-dependency model, file-to-layer mapping, and dependency table, see references/PV_LAYER_ARCHITECTURE.md.
"With a sufficient number of users of an API, it does not matter what you promise in the contract: all observable behaviors of your system will be depended upon by somebody."
Before modifying any observable behavior of an existing component -- including undocumented behaviors, error messages, return value nuances, timing, side effects -- ask:
"What else could currently be depending on this?"
This includes:
Gate: If you cannot enumerate all call sites that might depend on the behavior being changed, dispatch an explore agent to find them before proceeding. Changing behavior without knowing who relies on it is a silent breaking change.
This does not mean "never change behavior." It means: be deliberate. Know what you're breaking. Break it intentionally, with all dependents updated in the same commit.
The codebase has a dirty zone (data that has not been validated) and a clean zone (data that has passed all validation). The barricade is the boundary between them.
Rules:
| Zone | Contains | May assume |
|---|---|---|
| Dirty | File bytes, user input strings, socket data | Nothing -- validate everything |
| Barricade | Parser, validator, loader functions | Converts dirty -> clean |
| Clean | ViewerApp state, Camera, Shader, Particle | Data is valid -- no defensive checks needed |
Violation signal: A class in Layer 2 (Camera, Shader, Particle) checking for null or empty data it received from ViewerApp. That check belongs at the barricade, not in the clean zone.
Run every item for each file under review:
ViewerApp orchestrate or implement? (must orchestrate only -- rendering logic belongs in Shader/Particle classes)glXxx(), glXxx_ext()) appear outside of IOpenGLContext implementations? (VIOLATION)src/ files import from tests/? (VIOLATION -- production code must never depend on test code)src/testing/PixelComparator acquire OpenGL state directly, rather than receiving an Image? (VIOLATION)ui/) reach into graphics/ internals beyond IOpenGLContext? (VIOLATION)#include dependencies between any two files in the same layer?[+] All pass -> verdict: CLEAN [-] Any fail -> verdict: VIOLATIONS FOUND -- document every failure in the Review Report
For Particle-Viewer specific violation examples and fixes, see references/PV_LAYER_ARCHITECTURE.md.
For the report table template and per-file dispatch instructions, see references/ARCH_REVIEW_TEMPLATES.md.
A verdict of VIOLATIONS FOUND means the PR is NOT mergeable until every row in the violations table is resolved.
If you catch yourself thinking any of the following, STOP before writing your verdict:
IOpenGLContext is a violation. Flag it.| Excuse | Reality |
|---|---|
| "Just a small dependency, doesn't affect architecture" | Architecture violations compound. Small ones become load-bearing. Fix them now. |
| "ViewerApp needed to call GL directly for performance" | Use IOpenGLContext. That abstraction exists precisely for this case. |
| "The test utility is tiny, no harm putting it in src/" | Test-only code in production is a maintenance trap and blurs the test boundary. |
| "Circular dependency is fine since they're in the same layer" | Same layer does not mean circular is acceptable. It is still a design smell requiring resolution. |
| "I'll refactor it properly later" | Later never comes. Fix the boundary violation before this code ships. |
| "The PR is urgent, we can clean up the architecture next sprint" | Urgency is the most common rationalization for shipping violations. The law applies under urgency too. |
code-quality -- naming conventions and C++ patterns; architecture-review checks structure, code-quality checks formtesting -- governs what lives in tests/ vs src/testing/; architecture-review enforces the boundaryinfrastructure-review -- CMake build structure; architecture-review checks source structureoop-principles -- sub-domain skill; run Is-A / Has-A and SOLID gate for every class hierarchy change reviewed hereDesign patterns and architectural principles: references/DESIGN_PATTERNS.md and references/ANTIPATTERNS.md
npx claudepluginhub jpegthedev/story-to-ship --plugin story-to-shipProvides behavioral guidelines to reduce common LLM coding mistakes, focusing on simplicity, surgical changes, assumption surfacing, and verifiable success criteria.
Searches, retrieves, and installs Agent Skills from prompts.chat registry using MCP tools like search_skills and get_skill. Activates for finding skills, browsing catalogs, or extending Claude.
Creates, edits, and optimizes skills for Claude Code, including drafting, evaluating with test prompts, iterating on performance, and improving skill descriptions for better triggering accuracy.