code-review

在完成实施后，于创建PR之前，对代码质量、用户影响、测试覆盖率以及文档进行审查时使用此功能。

SKILL.md

--- frontmatter

name: code-review
description: Use after completing implementation to review code quality, user impact, test coverage, and documentation before creating a PR

Code Review

Overview

Review code for quality, user impact, tests, and documentation. Balance technical excellence with practical simplicity.

Core principle: Clean code should serve users, not just developers.

When to Use

•After completing a feature or bug fix
•Before creating a pull request
•When reviewing changes before merge

When NOT to Use

•Trivial changes (typo fixes)
•Documentation-only changes
•Initial exploration/prototyping

Review Process

Phase	Focus	Key Question
1. Identify	What changed?	`git diff --name-only develop`
2. User Impact	How does this affect users?	Is UX better or worse?
3. Code Quality	Does it follow standards?	KISS + no anti-patterns?
4. Tests	Is it covered?	New code = new tests?
5. Docs	What needs updating?	llm/state-*.md current?

Phase 1: Identify Changes

Categorize changed files:

•Backend: lib/, app.py
•Frontend: frontend/src/
•Tests: tests/
•Docs: llm/, *.md

Note change type: new feature | bug fix | refactoring | enhancement

Phase 2: User Impact

Ask for each change:

•Does this affect what users see or do?
•Are error messages user-friendly (not technical jargon)?
•Are loading states shown?
•Can users recover from errors?
•Is this the simplest UX possible?

Red flags:

•Silent failures (user doesn't know something failed)
•Lost work on errors
•Unclear feedback ("Error: 500" vs "Could not save")
•Unnecessary complexity exposed to users

Phase 3: Code Quality

KISS Check

Can each function be explained in one sentence? If not, it's too complex.

Backend Anti-patterns (blocking)

• Silent failures (empty except blocks)
• God functions (>30 lines, >3 params)
• SQL injection (f-strings in queries)
• Missing error context
• Walrus operators / complex one-liners

Frontend Anti-patterns (blocking)

• Empty catch blocks
• Inline fetch (not in service layer)
• Missing useEffect cleanup
• any types or as assertions
• Hardcoded colors (use theme: fg., canvas.)
• Prop drilling (>5 props)

Security

• Inputs validated at API boundary
• SQL parameterized (? placeholders)
• No secrets in code/logs

Phase 4: Test Coverage

Change Type	Required Test
New API endpoint	Unit test
New block	`tests/blocks/test_*.py`
Bug fix	Regression test
User workflow change	E2E test
Refactoring	Existing tests pass

Test quality:

•Naming: test_<method>_<scenario>_<expected>
•One behavior per test
•Error cases tested, not just happy path

Phase 5: Documentation

Update llm/state-*.md when:

•New API endpoint → state-backend.md
•New block → state-backend.md
•New component/page → state-frontend.md
•Architecture change → state-project.md

Code comments: explain WHY, not what. Lowercase, concise.

Output Format

markdown

### User Impact
[UX improvements or issues found]

### Anti-patterns
[location + violation + fix, or "none"]

### Code Quality Issues
[severity + location + fix, or "none"]

### Test Coverage
[required: present/missing | gaps if any]

### Documentation Updates
[files needing update, or "none"]

### Verdict
[BLOCK | REQUEST CHANGES | APPROVE]
Reason: [brief explanation]

Verdict Rules

Condition	Verdict
Anti-patterns found	BLOCK
Security issues	BLOCK
Missing required tests	REQUEST CHANGES
Needs doc updates	REQUEST CHANGES
All checks pass	APPROVE

Golden Rules

•Anti-patterns are blocking - always reject
•User experience matters - clean code that hurts UX is bad code
•KISS wins - one sentence explanation or it's too complex
•Tests are not optional - new code needs tests
•Fail loudly - silent failures are never acceptable