pre-commit-review

Pure Analysis & Reporting - Generates report, doesn't fix anything or invoke skills.

Reference: See reference.md for complete detection checklist with examples. Examples: See examples.md for real-world review scenarios. </objective>

<quick_start>

1. •Read files under review (all staged or specific files)
2. •Apply design principles checklist from reference.md (LLM reasoning)
3. •Search usage patterns with Grep tool
4. •Categorize findings: Bugs, Design Debt, Readability Debt, Polish
5. •Generate structured report with file:line locations and fix recommendations
6. •Return report to caller (no fixes, no skill invocations) </quick_start>

<input_output> <input>

• •Files to review (specific files or all staged changes)
• •Review mode: full (first run) or incremental (subsequent runs)
• •Previous findings (optional, for incremental mode)
• •Context (invoked by refactoring, orchestrator, subagent, or user) </input>

<invocation_modes> <direct_skill_invocation context="User or Orchestrator">

• •Full control, can invoke other skills
• •Can make changes based on findings
• •Interactive mode with user feedback </direct_skill_invocation>

<subagent_mode context="Task tool with go-code-reviewer">

• •Read-only analysis, returns report only
• •Cannot invoke other skills
• •Used for parallel execution by orchestrator
• •Designed for speed and focused analysis </subagent_mode> </invocation_modes>

<who_invokes>

1. •go-code-reviewer agent - Loads this skill for design analysis guidance during parallel quality analysis
2. •@refactoring skill - After applying patterns, validates design quality remains high
3. •User - Manual code review before commit </who_invokes>

<detection_capabilities> What Reviewer Detects (That Linters Can't):

• •Primitive obsession (with juiciness scoring)
• •Unstorified functions (mixed abstraction levels)
• •Missing domain concepts (implicit types that should be explicit)
• •Non-self-validating types (defensive code in methods)
• •Poor comment quality (explaining what instead of why)
• •File structure issues (too long, too many types)
• •Generic package extraction opportunities
• •Design bugs (nil deref, ignored errors, resource leaks)
• •Test quality (weak assertions, missing use cases, mock overuse, conditionals in tests)

Division of Labor:

• •Linter handles: Complexity metrics, line counts, formatting, syntax
• •Reviewer handles: Design patterns, domain modeling, conceptual issues

See reference.md for complete detection checklist with examples. </detection_capabilities>

<full_review_mode context="First Run">

1. •Read all files under review (using Read tool)
2. •Apply design principles checklist from reference.md (LLM reasoning)
3. •Search for usage patterns across codebase (using Grep tool)
4. •Categorize findings:
   • •Bugs (nil deref, ignored errors, resource leaks)
   • •Design Debt (types, architecture, validation)
   • •Readability Debt (abstraction, flow, clarity)
   • •Polish (naming, docs, minor improvements)
5. •Generate structured report with recommendations
6. •Return report to caller (doesn't invoke other skills or make fixes) </full_review_mode>

<incremental_review_mode context="Subsequent Runs"> Used after fixes have been applied to verify resolution and detect new issues.

1. •Read ONLY changed files since last review (using git diff)
2. •Compare against previous findings:
   • •Mark resolved issues as Fixed
   • •Identify issues that still exist
3. •Analyze changed code for NEW issues introduced by fixes
4. •Generate delta report:
   • •Fixed: Issues from previous run that are now resolved
   • •Remaining: Issues that still need attention
   • •New: Issues introduced by recent changes
5. •Return concise delta report (not full analysis)

When to Use Incremental Mode:

• •After @refactoring skill applies fixes
• •During iterative fix loop in Phase 4 of autopilot workflow
• •User requests re-review after making changes

Benefits:

• •Faster execution (only analyzes changed files)
• •Clear feedback on what was fixed vs what remains
• •Detects regressions introduced by fixes </incremental_review_mode>

<detection_approach> LLM-Powered Analysis (not AST parsing or metrics calculation):

The reviewer reads code like a senior developer and applies design principles:

• •Reads files with Read tool
• •Searches patterns with Grep tool (find usages, duplications)
• •Applies checklist from reference.md using LLM reasoning
• •Pattern matches against anti-patterns
• •Counts occurrences and calculates juiciness scores
• •Generates findings with specific locations and fix guidance </detection_approach>

<report_format>

<full_report context="First Run">

📊 CODE REVIEW REPORT
Scope: [files reviewed]
Mode: FULL

SUMMARY

Total findings: 18
🐛 Bugs: 2 (fix immediately)
🔴 Design Debt: 5 (fix before commit)
🟡 Readability Debt: 8 (improves maintainability)
🟢 Polish: 3 (nice to have)

Estimated fix effort: 3.5 hours

[Detailed findings by category]
[Recommendations by priority]
[Skills to use for fixes]

</full_report>

<incremental_report context="Subsequent Runs">

📊 CODE REVIEW DELTA REPORT
Scope: [changed files only]
Mode: INCREMENTAL

SUMMARY

✅ Fixed: 4 (resolved from previous run)
⚠️ Remaining: 2 (still need attention)
🆕 New: 1 (introduced by recent changes)

[Detailed delta findings]

</incremental_report>

<structured_output context="For Orchestrator Parsing"> When invoked as subagent for combined analysis, output follows strict format:

🐛 BUGS
────────────────────────────────────────────────
file:line | Issue description | Why it matters | Fix strategy | Effort: [Trivial/Moderate/Significant]

🔴 DESIGN DEBT
────────────────────────────────────────────────
file:line | Issue description | Why it matters | Fix strategy | Effort: [Trivial/Moderate/Significant]

🟡 READABILITY DEBT
────────────────────────────────────────────────
file:line | Issue description | Why it matters | Fix strategy | Effort: [Trivial/Moderate/Significant]

🟢 POLISH
────────────────────────────────────────────────
file:line | Issue description | Why it matters | Fix strategy | Effort: [Trivial/Moderate/Significant]

Effort Estimates:

• •Trivial: <5 minutes (extract constant, rename variable)
• •Moderate: 5-20 minutes (extract function, storifying, create simple type)
• •Significant: >20 minutes (architectural refactoring, complex type extraction)

file:line Format: Must be exact for orchestrator to correlate with linter errors

• •Example: pkg/parser.go:45
• •NOT: parser.go line 45 or pkg/parser.go (line 45) </structured_output>

</report_format>

<invoked_by_refactoring> Refactoring completes → invoke reviewer → analyze report:

• •Bugs found? → Fix immediately, re-run linter
• •Design debt found? → Apply another refactoring pattern
• •All clean? → Return success to orchestrator </invoked_by_refactoring>

<invoked_by_go_code_reviewer> During Phase 2 (Parallel Quality Analysis):

1. •go-code-reviewer agent automatically loads this skill for guidance
2. •Agent applies detection checklist from this skill
3. •Agent returns structured report to quality-analyzer
4. •quality-analyzer combines with linter/test results
5. •Orchestrator routes based on combined findings </invoked_by_go_code_reviewer>

<invoked_by_user> Manual review request:

1. •User invokes: @pre-commit-review on path/to/file.go
2. •Receive detailed report
3. •User decides how to proceed
4. •User may invoke @refactoring or @code-designing for fixes </invoked_by_user>

<review_scope> Primary Scope: Changed code in commit

• •All modified lines
• •All new files
• •Specific focus on design principle adherence

Secondary Scope: Context around changes

• •Entire files containing modifications
• •Flag patterns/issues outside commit scope (in BROADER CONTEXT section)
• •Suggest broader refactoring opportunities if valuable </review_scope>

<advisory_nature> This review does NOT block commits.

Purpose:

• •Provide visibility into design quality
• •Offer concrete improvement suggestions with examples
• •Help maintain coding principles
• •Guide refactoring decisions

Caller (or user) decides:

• •Commit as-is (accept debt knowingly)
• •Fix critical debt before commit (bugs, major design issues)
• •Fix all debt before commit (comprehensive cleanup)
• •Expand scope to broader refactor (when broader context issues found) </advisory_nature>

<finding_categories>

<design_debt severity="high"> Will cause pain when extending/modifying code

• •Primitive obsession, missing domain types
• •Non-self-validating types
• •Wrong architecture (horizontal vs vertical)
• •Fix before commit recommended </design_debt>

<readability_debt severity="medium"> Makes code harder to understand and work with

• •Mixed abstraction levels, not storified
• •Functions too long or complex
• •Poor naming, unclear intent
• •Fix improves team productivity </readability_debt>

See reference.md for detailed principles and examples for each category. </finding_categories>

<success_criteria> Code Quality Analysis Performed:

• • Read every function - does it read like a story? (single abstraction level)
• • Checked all functions for mixed abstraction levels (storifying needed?)
• • Evaluated primitives for primitive obsession (juiciness test applied)
• • Assessed types for self-validation (defensive code in methods?)
• • Reviewed comment quality (explaining WHY not WHAT?)
• • Checked file structure (too long? too many types?)
• • Searched for missing domain concepts (implicit types that should be explicit)
• • Validated test quality (weak assertions? conditionals in tests? mock overuse?)
• • Scanned for design bugs (nil deref, ignored errors, resource leaks)

Report Quality:

• • All findings categorized by severity (Bugs, Design Debt, Readability Debt, Polish)
• • Each finding includes: file:line, issue, why it matters, fix strategy, effort estimate
• • Structured format parseable by orchestrator
• • Clear distinction between bugs (fix immediately) and advisory findings
• • Incremental mode accurately tracks fixed, remaining, and new issues </success_criteria>