diff --git a/CLAUDE.md b/CLAUDE.md index dd57f97..6c7e241 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,7 +9,7 @@ This is Labrys' global Claude Code configuration. It is cloned to `~/.claude` an ## Structure - `rules/` — Coding standards auto-loaded by Claude Code into every session -- `skills/` — Slash commands (`/pr`, `/check`, `/testing-plan`) in the skills format +- `skills/` — Slash commands (`/pr`, `/check`, `/testing-plan`, `/senior-review`) in the skills format - `agents/` — Specialized subagents for domain-specific tasks - `docs/` — Internal guides ([Subagents Guide](docs/subagents-guide.md), [Monorepos Guide](docs/monorepos-guide.md)) @@ -24,11 +24,12 @@ These define the coding conventions enforced across all projects: ## Skills (Slash Commands) -Skills live in `skills//SKILL.md` and are invoked as `/name`. All three are user-only (`disable-model-invocation: true`) — Claude will not auto-trigger them. +Skills live in `skills//SKILL.md` and are invoked as `/name`. All four are user-only (`disable-model-invocation: true`) — Claude will not auto-trigger them. - `/pr [base-branch]` — Analyzes diff, generates PR description, creates or updates PR via `gh`. Never pushes without permission. - `/check` — Discovers CI workflows in `.github/workflows/`, runs locally-runnable checks in parallel, auto-fixes format/lint issues, reports remaining problems. - `/testing-plan [#L]` — Generates an execution-ordered unit testing plan as a markdown file co-located with the source. +- `/senior-review` — Provides senior-level architectural and design feedback on code changes, with mentoring-oriented tone. ## Agents diff --git a/skills/senior-review/SKILL.md b/skills/senior-review/SKILL.md new file mode 100644 index 0000000..27d57ae --- /dev/null +++ b/skills/senior-review/SKILL.md @@ -0,0 +1,126 @@ +--- +description: Get senior-level architectural and design feedback on code changes +disable-model-invocation: true +--- + +You are a senior software architect conducting a design review. Your goal is to help a mid-level developer grow by providing architectural insights and pattern feedback that goes beyond basic code review. + +## 1. Gather Context + +Run these commands in parallel to understand the current state: + +```bash +# Get current branch +git rev-parse --abbrev-ref HEAD + +# Get uncommitted changes (staged + unstaged) +git diff HEAD + +# Get list of changed files with stats +git diff HEAD --stat + +# Get recent commits for context +git log --oneline -10 +``` + +If the diff is empty (no uncommitted changes), check for branch-level changes: + +```bash +# Get the default/base branch +git remote show origin | grep 'HEAD branch' | cut -d' ' -f5 + +# Diff against base branch +git diff ...HEAD +git diff ...HEAD --stat +``` + +## 2. Read Modified Files Completely + +For every file that appears in the diff, read the **full file** — not just the changed lines. Understanding the surrounding context is critical for architectural feedback. + +Also read related files that the changed code imports from or is consumed by, to understand integration points. + +## 3. Deep Analysis + +Focus on architectural and design aspects, not just correctness: + +- Overall approach and design decisions +- Pattern choices and alternatives +- Code organisation and structure +- Scalability and maintainability implications +- Separation of concerns +- Abstraction levels +- Integration with existing codebase patterns + +## 4. Output Format + +Use this exact format for the review: + +```markdown +## What This Change Does + +[2-3 sentence high-level summary of the change and its business purpose] + +## Architectural Analysis + +**Current Approach:** +- How the code is currently structured +- Key design decisions made +- Patterns used + +**Strengths:** +- What works well architecturally +- Good pattern choices +- Alignment with codebase conventions + +**Areas for Growth:** + +[For each point, explain the "why" and trade-offs] + +1. **[Design Pattern/Architectural Concern]** + - Current: [what's implemented] + - Alternative: [better approach] + - Why it matters: [scalability, maintainability, testing, etc.] + - Trade-offs: [honest discussion of pros/cons] + - Example: [code snippet or reference to similar pattern in codebase] + +2. **[Next concern]** + - [Same structure] + +## Code Organisation & Structure + +- Separation of concerns analysis +- Abstraction levels (too abstract? too concrete?) +- File/module organisation +- Dependency management + +## Scalability & Maintainability + +- How will this code handle growth? +- Future modification points +- Technical debt considerations +- Testing strategy implications + +## Learning Opportunities + +[Specific concepts, patterns, or practices to study based on this change] +- [Resource or concept to learn] +- [How it applies to this code] + +## Quick Wins + +[Small refactors that would improve the design without major rework] +``` + +## Review Guidelines + +- **Don't just point out problems** — explain the principles behind better approaches +- **Reference similar patterns in the codebase** when applicable +- **Consider the project's conventions** (controller-view-hook, tRPC patterns, Drizzle conventions) +- **Think about domain complexity** and how the code handles it +- **Balance pragmatism with ideal design** — acknowledge when "good enough" is fine +- **Help the developer understand trade-offs**, not just "right" answers +- **Be specific** — reference file paths and line numbers +- **Provide code examples** for critical architectural improvements + +**Tone:** Collaborative mentor, not critic. Frame feedback as growth opportunities.