feat(review): make review mandatory across pipeline skills (#433)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -245,9 +245,17 @@ This command takes a work document (plan, specification, or todo file) and execu
|
||||
# Use linting-agent before pushing to origin
|
||||
```
|
||||
|
||||
2. **Consider Code Review** (Optional)
|
||||
2. **Code Review**
|
||||
|
||||
Use for complex, risky, or large changes. Load the `ce:review` skill with `mode:autofix` to fix safe issues and flag the rest before shipping. When the plan file path is known, pass it as `plan:<path>`.
|
||||
Every change gets reviewed before shipping. The depth scales with the change's risk profile.
|
||||
|
||||
**Tier 1: Inline self-review** — Read the diff and check for logic errors, missed edge cases, and plan drift. No specialized agents. Use this tier only when **all four** criteria are true:
|
||||
- Purely additive (new files only, no existing behavior modified)
|
||||
- Single concern (one skill, one component — not cross-cutting)
|
||||
- Pattern-following (implementation mirrors an existing example with no novel logic)
|
||||
- Plan-faithful (no scope growth, no deferred questions resolved with surprising answers)
|
||||
|
||||
**Tier 2: Full review (default)** — Load the `ce:review` skill with `mode:autofix` to run specialized reviewer agents, auto-apply safe fixes, and surface residual work as todos. When the plan file path is known, pass it as `plan:<path>`. This is the default when working from a plan — use it unless all four Tier 1 criteria are met.
|
||||
|
||||
3. **Final Validation**
|
||||
- All tasks marked completed
|
||||
@@ -271,44 +279,9 @@ This command takes a work document (plan, specification, or todo file) and execu
|
||||
|
||||
### Phase 4: Ship It
|
||||
|
||||
1. **Create Commit**
|
||||
1. **Capture and Upload Screenshots for UI Changes** (REQUIRED for any UI work)
|
||||
|
||||
```bash
|
||||
git add .
|
||||
git status # Review what's being committed
|
||||
git diff --staged # Check the changes
|
||||
|
||||
# Commit with conventional format
|
||||
git commit -m "$(cat <<'EOF'
|
||||
feat(scope): description of what and why
|
||||
|
||||
Brief explanation if needed.
|
||||
|
||||
🤖 Generated with [MODEL] via [HARNESS](HARNESS_URL) + Compound Engineering v[VERSION]
|
||||
|
||||
Co-Authored-By: [MODEL] ([CONTEXT] context, [THINKING]) <noreply@anthropic.com>
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
**Fill in at commit/PR time:**
|
||||
|
||||
| Placeholder | Value | Example |
|
||||
|-------------|-------|---------|
|
||||
| Placeholder | Value | Example |
|
||||
|-------------|-------|---------|
|
||||
| `[MODEL]` | Model name | Claude Opus 4.6, GPT-5.4 |
|
||||
| `[CONTEXT]` | Context window (if known) | 200K, 1M |
|
||||
| `[THINKING]` | Thinking level (if known) | extended thinking |
|
||||
| `[HARNESS]` | Tool running you | Claude Code, Codex, Gemini CLI |
|
||||
| `[HARNESS_URL]` | Link to that tool | `https://claude.com/claude-code` |
|
||||
| `[VERSION]` | `plugin.json` → `version` | 2.40.0 |
|
||||
|
||||
Subagents creating commits/PRs are equally responsible for accurate attribution.
|
||||
|
||||
2. **Capture and Upload Screenshots for UI Changes** (REQUIRED for any UI work)
|
||||
|
||||
For **any** design changes, new views, or UI modifications, you MUST capture and upload screenshots:
|
||||
For **any** design changes, new views, or UI modifications, capture and upload screenshots before creating the PR:
|
||||
|
||||
**Step 1: Start dev server** (if not running)
|
||||
```bash
|
||||
@@ -336,65 +309,29 @@ This command takes a work document (plan, specification, or todo file) and execu
|
||||
- **Modified screens**: Before AND after screenshots
|
||||
- **Design implementation**: Screenshot showing Figma design match
|
||||
|
||||
**IMPORTANT**: Always include uploaded image URLs in PR description. This provides visual context for reviewers and documents the change.
|
||||
2. **Commit and Create Pull Request**
|
||||
|
||||
3. **Create Pull Request**
|
||||
Load the `git-commit-push-pr` skill to handle committing, pushing, and PR creation. The skill handles convention detection, branch safety, logical commit splitting, adaptive PR descriptions, and attribution badges.
|
||||
|
||||
```bash
|
||||
git push -u origin feature-branch-name
|
||||
When providing context for the PR description, include:
|
||||
- The plan's summary and key decisions
|
||||
- Testing notes (tests added/modified, manual testing performed)
|
||||
- Screenshot URLs from step 1 (if applicable)
|
||||
- Figma design link (if applicable)
|
||||
- The Post-Deploy Monitoring & Validation section (see Phase 3 Step 4)
|
||||
|
||||
gh pr create --title "Feature: [Description]" --body "$(cat <<'EOF'
|
||||
## Summary
|
||||
- What was built
|
||||
- Why it was needed
|
||||
- Key decisions made
|
||||
If the user prefers to commit without creating a PR, load the `git-commit` skill instead.
|
||||
|
||||
## Testing
|
||||
- Tests added/modified
|
||||
- Manual testing performed
|
||||
|
||||
## Post-Deploy Monitoring & Validation
|
||||
- **What to monitor/search**
|
||||
- Logs:
|
||||
- Metrics/Dashboards:
|
||||
- **Validation checks (queries/commands)**
|
||||
- `command or query here`
|
||||
- **Expected healthy behavior**
|
||||
- Expected signal(s)
|
||||
- **Failure signal(s) / rollback trigger**
|
||||
- Trigger + immediate action
|
||||
- **Validation window & owner**
|
||||
- Window:
|
||||
- Owner:
|
||||
- **If no operational impact**
|
||||
- `No additional operational monitoring required: <reason>`
|
||||
|
||||
## Before / After Screenshots
|
||||
| Before | After |
|
||||
|--------|-------|
|
||||
|  |  |
|
||||
|
||||
## Figma Design
|
||||
[Link if applicable]
|
||||
|
||||
---
|
||||
|
||||
[![Compound Engineering v[VERSION]](https://img.shields.io/badge/Compound_Engineering-v[VERSION]-6366f1)](https://github.com/EveryInc/compound-engineering-plugin)
|
||||
🤖 Generated with [MODEL] ([CONTEXT] context, [THINKING]) via [HARNESS](HARNESS_URL)
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
4. **Update Plan Status**
|
||||
3. **Update Plan Status**
|
||||
|
||||
If the input document has YAML frontmatter with a `status` field, update it to `completed`:
|
||||
```
|
||||
status: active → status: completed
|
||||
```
|
||||
|
||||
5. **Notify User**
|
||||
4. **Notify User**
|
||||
- Summarize what was completed
|
||||
- Link to PR
|
||||
- Link to PR (if one was created)
|
||||
- Note any follow-up work needed
|
||||
- Suggest next steps if applicable
|
||||
|
||||
@@ -452,7 +389,7 @@ Most plans should use subagent dispatch from standard mode. Agent teams add sign
|
||||
- Follow existing patterns
|
||||
- Write tests for new code
|
||||
- Run linting before pushing
|
||||
- Use reviewer agents for complex/risky changes only
|
||||
- Review every change — inline for simple additive work, full review for everything else
|
||||
|
||||
### Ship Complete Features
|
||||
|
||||
@@ -473,20 +410,21 @@ Before creating PR, verify:
|
||||
- [ ] Before/after screenshots captured and uploaded (for UI changes)
|
||||
- [ ] Commit messages follow conventional format
|
||||
- [ ] PR description includes Post-Deploy Monitoring & Validation section (or explicit no-impact rationale)
|
||||
- [ ] Code review completed (inline self-review or full `ce:review`)
|
||||
- [ ] PR description includes summary, testing notes, and screenshots
|
||||
- [ ] PR description includes Compound Engineered badge with accurate model, harness, and version
|
||||
|
||||
## When to Use Reviewer Agents
|
||||
## Code Review Tiers
|
||||
|
||||
**Don't use by default.** Use reviewer agents only when:
|
||||
Every change gets reviewed. The tier determines depth, not whether review happens.
|
||||
|
||||
- Large refactor affecting many files (10+)
|
||||
- Security-sensitive changes (authentication, permissions, data access)
|
||||
- Performance-critical code paths
|
||||
- Complex algorithms or business logic
|
||||
- User explicitly requests thorough review
|
||||
**Tier 1 (inline self-review)** — all four must be true:
|
||||
- Purely additive (new files only, no existing behavior modified)
|
||||
- Single concern (one skill, one component — not cross-cutting)
|
||||
- Pattern-following (mirrors an existing example, no novel logic)
|
||||
- Plan-faithful (no scope growth, no surprising deferred-question resolutions)
|
||||
|
||||
For most features: tests + linting + following patterns is sufficient.
|
||||
**Tier 2 (full review)** — the default when working from a plan. Invoke `ce:review mode:autofix` with `plan:<path>` when available. Safe fixes are applied automatically; residual work surfaces as todos.
|
||||
|
||||
## Common Pitfalls to Avoid
|
||||
|
||||
@@ -496,4 +434,4 @@ For most features: tests + linting + following patterns is sufficient.
|
||||
- **Testing at the end** - Test continuously or suffer later
|
||||
- **Forgetting to track progress** - Update task status as you go or lose track of what's done
|
||||
- **80% done syndrome** - Finish the feature, don't move on early
|
||||
- **Over-reviewing simple changes** - Save reviewer agents for complex work
|
||||
- **Skipping review** - Every change gets reviewed; only the depth varies
|
||||
|
||||
Reference in New Issue
Block a user