AgentSkillsCN

reviewing-code

像高级工程师一样审查代码变更。当进行代码审查、检查PR质量、审计提交前的变更,或提供实现反馈时使用。

SKILL.md
--- frontmatter
name: reviewing-code
description: Reviewing code changes like a senior engineer. Use when doing code reviews, checking PR quality, auditing changes before commit, or providing feedback on implementations.

Code Review

Review code with precision. Flag real issues, skip noise.

Severity Levels

🔴 Critical (Block merge)

  • Security vulnerabilities (injection, auth bypass, secrets exposure)
  • Data loss or corruption risks
  • Breaking changes to public APIs
  • Race conditions with data integrity impact
  • Unhandled errors that crash the system

🟡 Important (Should fix)

  • Bugs that affect functionality
  • Performance issues (N+1 queries, unbounded loops, memory leaks)
  • Missing error handling for likely failure cases
  • Violation of established patterns/architecture
  • Missing input validation on external data
  • Hardcoded values that should be configurable

🟢 Minor (Consider fixing)

  • Naming improvements for clarity
  • Code duplication that could be extracted
  • Missing edge case tests
  • Documentation gaps
  • Style inconsistencies not caught by linter

⚪ Skip (Don't mention)

  • Issues a linter/formatter would catch
  • Personal style preferences not in guidelines
  • Pre-existing issues not introduced by this change
  • Nitpicks that don't improve the code meaningfully

Review Checklist

Correctness

  • Does the code do what it claims?
  • Are edge cases handled?
  • Are error conditions caught and handled appropriately?

Security

  • User input sanitized before use?
  • Auth/authz checks in place?
  • Secrets kept out of code?
  • SQL/command injection prevented?

Performance

  • Database queries efficient? (indexes, N+1)
  • Appropriate caching?
  • Unbounded operations paginated?
  • Expensive operations async where appropriate?

Maintainability

  • Clear naming and structure?
  • Appropriate abstractions?
  • Tests cover the important paths?
  • Would a new team member understand this?

Architecture

  • Follows established patterns?
  • Correct layer boundaries?
  • Dependencies flow in the right direction?
  • No circular dependencies introduced?

Giving Feedback

Format

code
🔴 path/to/file.ts:42 - Brief issue description
   → Suggested fix or approach

Good Feedback

  • Specific: Points to exact location and problem
  • Actionable: Suggests a concrete fix
  • Educational: Explains why it matters
  • Proportional: Severity matches actual impact

Avoid

  • Vague complaints ("this could be better")
  • Rewrites without explanation
  • Blocking on style preferences
  • Piling on - one comment per issue type is enough

Confidence Filter

Before reporting an issue, ask:

  1. Is this a real problem or a preference?
  2. Would a senior engineer call this out?
  3. Is this introduced by this change (not pre-existing)?
  4. Does it violate explicit guidelines or just implicit taste?

Only report issues where you're 80%+ confident they matter.

Review Output Structure

code
**Summary:** One-line assessment of the change

**Issues:**
🔴 Critical issues (if any)
🟡 Important issues (if any)  
🟢 Minor suggestions (limit to 2-3)

**Good:** What the code does well (always include something)

Anti-patterns to Watch

PatternProblemLook For
God objectToo many responsibilitiesClass with 10+ methods, mixed concerns
Primitive obsessionStringly-typed dataIDs as strings, status as raw strings
Deep nestingHard to follow logic4+ levels of indentation
Magic valuesUnclear intentNumbers/strings without constants
Silent failuresBugs hideEmpty catch blocks, ignored return values
Temporal couplingOrder-dependent callsInit must be called before use
Feature envyWrong home for logicMethod uses more of another class

Positive Signals

Also call out good patterns:

  • Clean separation of concerns
  • Good test coverage on critical paths
  • Defensive error handling
  • Clear naming that documents intent
  • Appropriate use of types
  • Thoughtful API design