| cypilot | true |
|---|---|
| type | requirement |
| name | Code Quality Expert Checklist |
| version | 1.0 |
| purpose | Generic (kit-agnostic) quality checklist for code changes and reviews |
Generic (kit-agnostic) checklist for reviewing code changes. Use it to produce an issues-only report and to drive consistent, high-signal code reviews across any language and project.
Scope: Any codebase (language-agnostic). This file contains only generic code-quality criteria.
Output contract:
- Produce an issues-only report using the formats in Reporting.
- For each issue: include checklist ID, severity, location, evidence, why it matters, and a concrete fix.
- Prerequisites
- Applicability Context
- Severity Dictionary
- Review Scope Selection
- MUST HAVE
- MUST NOT HAVE
- Validation Summary
- Conflict Resolution
- Reporting
Before starting the review, confirm:
- I understand this checklist validates CODE implementations
- I will follow the Applicability Context rules below
- I selected a Review Mode and will check all items required by that mode
- I will document any violations found
- I will provide specific feedback for each failed check
- I will complete the Final Checklist and provide a review report
Before evaluating each checklist item, the expert MUST:
-
Understand the code's domain — What kind of code is this? (e.g., business logic, infrastructure, tests, utilities, configuration)
-
Determine applicability for each requirement — Not all checklist items apply to all code:
- Test code may have different error handling requirements
- Configuration code may not need complex algorithms
- Utility code may not need full observability
-
Require explicit handling — For each checklist item:
- If applicable: The code MUST satisfy it
- If not applicable: Document why (in review notes)
- If missing without explanation: Report as violation
-
Use explicit evaluation states — For each checklist item in your report, use one of:
- FAIL: Applicable and violated
- PASS: Applicable and satisfied
- N/A: Not applicable, with a written rationale
- NOT REVIEWED: Not checked due to selected Review Mode scope
-
Never skip silently — The expert MUST NOT skip a requirement just because it seems irrelevant
Key principle: Code quality is about fitness for purpose, not checkbox compliance
- CRITICAL: Unsafe/broken/security vulnerability; blocks merge.
- HIGH: Major quality issue; should be fixed before merge.
- MEDIUM: Meaningful improvement; fix when feasible.
- LOW: Minor improvement; optional.
Select review depth based on change size and risk:
| Change Size | Risk Level | Review Mode | Items to Check |
|---|---|---|---|
| <50 LOC, single file | Low | Quick | Critical-tier only |
| 50-200 LOC, few files | Medium | Standard | Critical + High-tier |
| >200 LOC or architectural | High | Full | All items |
For small, low-risk changes, check only these items:
MUST CHECK (blocking):
- SEC-CODE-001: Injection Prevention
- SEC-CODE-002: Authentication & Authorization
- SEC-CODE-003: Data Protection
- SEC-CODE-NO-001: No Hardcoded Secrets
- SEC-CODE-NO-002: No Dangerous Patterns
- ERR-CODE-001: Explicit Error Handling
- ERR-CODE-003: Input Validation
- ERR-CODE-NO-001: No Silent Failures
- QUAL-CODE-NO-002: No Placeholder Implementations
SPOT CHECK (sample):
- ENG-CODE-001: TDD — at least one test for new behavior
- QUAL-CODE-001: Readability — naming is clear
Note: Quick review: {N} LOC, checked Critical-tier items; other items = NOT REVIEWED
For medium-sized changes, check all CRITICAL and HIGH severity items:
All Quick Review items PLUS:
- All HIGH severity items from MUST HAVE sections
- All items from MUST NOT HAVE sections
For large or architectural changes, check ALL items in this checklist.
When time-constrained during full review, prioritize in this order:
- First pass: All CRITICAL items (security, error handling)
- Second pass: All HIGH items (quality, testing, SOLID)
- Third pass: All MEDIUM items (DRY, OCP, performance)
- Fourth pass: All LOW items (metrics, tracing)
Severity: HIGH
- New behavior has corresponding tests
- Tests were written before or alongside implementation
- Tests fail when implementation is removed
- Tests verify expected outcomes (not just "no crash")
- Test names describe the behavior being tested
- Tests are independent and can run in any order
Severity: HIGH
- Each module/class/function has one reason to change
- Functions do one thing and do it well
- Classes have a single, clear purpose
- No "god objects" or "kitchen sink" modules
- Responsibilities are separated by concern (UI, business logic, data access)
Severity: MEDIUM
- Behavior extended via composition/configuration, not modification
- New functionality doesn't require changing existing code
- Extension points are clear and intentional
- No modification of working code to add unrelated specs
Severity: HIGH
- Implementations honor interface contracts
- Subtypes are substitutable for their base types
- No surprising behavior when using polymorphism
- Preconditions are not strengthened in subtypes
- Postconditions are not weakened in subtypes
Severity: MEDIUM
- Interfaces are small and purpose-driven
- No "fat" interfaces with methods clients don't use
- Clients depend only on what they need
- Role interfaces preferred over header interfaces
Severity: HIGH
- High-level modules don't depend on low-level modules
- Both depend on abstractions
- Dependencies are injectable
- Core logic is testable without heavy integration setup
- External dependencies are behind interfaces
Severity: MEDIUM
- No copy-paste duplication
- Shared logic extracted with clear ownership
- Duplication removed only when patterns are clear (rule of three)
- Constants defined once, not scattered
- Common patterns abstracted appropriately
Severity: HIGH
- Simplest correct solution chosen
- No unnecessary complexity
- Code is readable without extensive documentation
- Clever tricks avoided in favor of clarity
- Standard patterns preferred over novel approaches
Severity: HIGH
- No speculative specs
- No unused abstractions
- No configuration for hypothetical scenarios
- No extension points without current use cases
- Specs added only when needed, not "just in case"
Severity: MEDIUM
- Refactoring done only after tests pass
- Behavior unchanged during refactoring
- Structure improved without adding specs
- Small, incremental refactoring steps
- No mixing refactoring with spec work in same commit
Severity: HIGH
- Clear, descriptive naming (variables, functions, classes)
- Consistent naming conventions
- Code reads like well-written prose
- Complex logic has explanatory comments
- No misleading names or abbreviations
Severity: HIGH
- Code is easy to modify
- Changes are localized (no ripple effects)
- Dependencies are explicit and minimal
- No hidden coupling between modules
- Clear module boundaries
Severity: HIGH
- Core logic testable without external dependencies
- Dependencies injectable for testing
- Side effects isolated and mockable
- Deterministic behavior (no random/time dependencies in logic)
- Observable outcomes (not just internal state)
Severity: CRITICAL
- Errors fail explicitly, not silently
- Error conditions identified and handled
- No swallowed exceptions
- Error messages are clear and actionable
- Stack traces available for debugging (not in production UI)
Severity: HIGH
- Partial failures handled appropriately
- Recovery actions documented
- Fallback behavior defined
- User-facing errors are friendly
- System-facing errors are detailed
Severity: CRITICAL
- All external inputs validated at system boundaries
- Validation rules clear and consistent
- Invalid input rejected early
- Validation errors are specific and helpful
- No trusting of internal code (validate at boundaries only)
Severity: CRITICAL
- No string concatenation for queries (parameterized queries)
- No command injection vulnerabilities
- No XSS vulnerabilities (output encoding)
- No path traversal vulnerabilities
- User input never used directly in dangerous contexts
Severity: CRITICAL
- Authentication checks present where required by the design/spec (routes, handlers, entrypoints)
- Authorization checks present for operations that mutate or access protected resources (as defined by design/spec)
- No privilege escalation vulnerabilities
- Session management secure
- Credentials not hardcoded
Severity: CRITICAL
- Sensitive data not logged
- PII handled appropriately
- Secrets not in code
- Encryption used where required
- Secure transmission for sensitive data
Severity: MEDIUM
- No obvious performance anti-patterns
- N+1 query patterns avoided
- Unnecessary allocations avoided
- Resources cleaned up properly
- Appropriate data structures chosen
Severity: MEDIUM
- Algorithm complexity appropriate for data size
- No blocking operations in hot paths
- Caching used where beneficial
- Batch operations used where appropriate
- Pagination implemented for large datasets
Severity: MEDIUM
- Meaningful events logged at integration boundaries
- Log levels used appropriately (DEBUG, INFO, WARN, ERROR)
- No secrets in logs
- Correlation IDs propagated
- Sufficient context for debugging
Severity: LOW (when applicable)
- Key operations have metrics
- Tracing integrated where beneficial
- Health checks implemented
- Alertable conditions identified
- Performance baselines established
Applicability note: Mark this item N/A unless the service is long-running or has operational SLO/SLA requirements.
Severity: HIGH
- All public APIs have tests
- Happy path tested
- Error paths tested
- Edge cases tested
- Boundary conditions tested
Severity: HIGH
- Tests are fast
- Tests are reliable (no flaky tests)
- Tests are independent
- Tests are readable
- Tests have clear assertions
Severity: MEDIUM
- Unit tests for business logic
- Integration tests for external dependencies
- E2E tests for critical paths (when applicable)
- Tests cover regression scenarios
- Tests document expected behavior
Severity: HIGH
What to check:
- No TODO in production code (without ticket reference)
- No FIXME in production code
- No XXX markers
- No HACK markers
- No "temporary" solutions that became permanent
Action: Either complete the work or create a tracked issue
Severity: CRITICAL
What to check:
- No
unimplemented!()/todo!()in business logic - No
throw new NotImplementedException()in production paths - No
passwith# TODOin Python - No empty catch blocks
- No stub methods that do nothing
Action: Implement or remove
Severity: CRITICAL
What to check:
- No empty catch blocks
- No swallowed exceptions
- No ignored return values for fallible operations
- No
_ = might_fail()patterns without handling - No
try: ... except: passpatterns
Action: Handle or propagate errors explicitly
Severity: HIGH
What to check:
- No bare
unwrap()on Results/Options in production code - No bare
panic!()in production code - No
expect()without meaningful message - No force-unwrapping optionals without guards
- No assertions in production code paths
Action: Use proper error handling
Severity: MEDIUM
What to check:
- No
#[ignore]without documented reason - No
@Disabledwithout documented reason - No
skipmarkers without explanation - No commented-out tests
- No placeholder tests (
assert!(true),assertTrue(true))
Action: Fix or remove the test
Severity: CRITICAL
What to check:
- No API keys in code
- No passwords in code
- No tokens in code
- No connection strings with credentials
- No private keys in code
Action: Use environment variables or secret management
Severity: CRITICAL
What to check:
- No
eval()with user input - No
exec()with user input - No
system()with user input - No
innerHTMLwith user input - No SQL string concatenation
Action: Use safe alternatives
Confirm before reporting results:
- I checked ALL items in MUST HAVE sections
- I verified ALL items in MUST NOT HAVE sections
- I ran all tests and they pass
- I ran linters and they pass
- I documented all violations found
- I provided specific feedback for each failed check
- All critical issues have been reported
- Code compiles without errors
- Code compiles without warnings (or warnings are acceptable)
- All unit tests pass
- All integration tests pass
- All E2E tests pass (if applicable)
- Linter passes
- Coverage meets requirements (if specified)
When checklist items appear to conflict, use this resolution order:
- Security (SEC) — Always takes precedence
- Error Handling (ERR) — Explicit failures over silent issues
- Correctness (QUAL, TEST) — Working code over elegant code
- Maintainability (ENG, QUAL) — Clarity over cleverness
- Performance (PERF) — Only after correctness established
- Observability (OBS) — Useful but not at expense of security
| Conflict | Resolution |
|---|---|
| DRY vs KISS | Prefer KISS — duplication is better than wrong abstraction |
| YAGNI vs OCP | Prefer YAGNI — don't add extension points "just in case" |
| Performance vs Readability | Prefer readability; optimize only with evidence |
| Test coverage vs Test speed | Prefer coverage for critical paths; speed for CI |
| Verbose errors vs User experience | Detailed logs, friendly user messages |
- Ask: "What failure mode is worse?"
- Security/data loss > inconvenience > performance
- Document the trade-off decision in code comments
- I will report every identified issue (no omissions)
- I will report only issues (no "everything looks good" sections)
- Each reported issue will include Evidence (code location/quote)
- Each reported issue will include Why it matters (impact)
- Each reported issue will include a Proposal (concrete fix)
- I will avoid vague statements and use precise, verifiable language
Report only problems (do not list what is OK).
| Review Mode | Report Format |
|---|---|
| Quick | Compact (table) |
| Standard | Compact or Full |
| Full | Full (detailed) |
## Code Review: {scope}
| # | ID | Sev | Location | Issue | Fix |
|---|-----|-----|----------|-------|-----|
| 1 | SEC-001 | CRIT | file.py:42 | SQL injection | Use parameterized query |
| 2 | ERR-001 | HIGH | file.py:87 | Silent catch | Add logging |
**Review mode**: Quick ({N} LOC)For each issue include:
- Issue: What is wrong
- Location: File path and line number(s)
- Evidence: Code snippet or description
- Why it matters: Impact (risk, maintainability, security)
- Proposal: Concrete fix
Full output format:
## Code Review Report (Issues Only)
### 1. {Short issue title}
**Checklist Item**: `{CHECKLIST-ID}` — {Checklist item title}
**Severity**: CRITICAL|HIGH|MEDIUM|LOW
#### Location
`{file_path}:{line_number}`
#### Issue
{What is wrong}
#### Evidence
```{language}
{code snippet}{Impact: risk, maintainability, security, performance}
{Concrete fix: what to change}
...
---
## Reporting Commitment
- [ ] I reported all issues I found
- [ ] I used the exact report format defined in this checklist
- [ ] I included evidence and impact for each issue
- [ ] I proposed concrete fixes for each issue
- [ ] I did not hide or omit known problems
- [ ] I am ready to iterate on the proposals and re-review after changes