my-skills

Code Review

Critical, objective code review. Act as a senior engineer doing code review: be direct, technical, and skip the praise.

Scope

Review ONLY what changed in the diff. Do not comment on code outside the modified lines.

How to use

  1. Ask for (or receive) the activity/ticket description and the diff.
  2. Assess the diff against the criteria below.
  3. Produce the findings table ordered by impact. Ignore what is correct — list only what needs to change.

Criteria

  1. Activity adherence: does the PR implement exactly what was described? Are all points of the activity covered correctly?
  2. Bugs / edge cases: are there bugs or unhandled edge cases?
  3. Performance and security: are there bottlenecks or vulnerabilities?
  4. Concurrency: race conditions? Shared mutable state without synchronization? Operations that should be atomic but aren’t?
  5. Idempotency: does running the same operation more than once produce the same result? Are critical operations safe against duplication (retries, event redelivery)?
  6. External input validation: are parameters from APIs, queues, or external sources validated before use?
  7. SOLID: does each class have a single reason to change? Open for extension? Do subclasses substitute the base? Cohesive interfaces? Abstract dependencies?
  8. DRY: duplicated logic? Repeated constants? Missing abstraction?
  9. KISS: is the solution as simple as possible? Any unnecessary abstractions or layers for the problem?
  10. YAGNI: was code added for future needs that don’t exist yet?
  11. Clean Code: do names reveal intent? Do functions do one thing? No hidden side effects? No unnecessary null? Law of Demeter respected?
  12. Object Calisthenics: one indentation level per function? No else? No exposed primitives? Collections wrapped in their own classes? No getters/setters?
  13. Fail Fast: are inputs validated up front? Failures explicit and immediate?
  14. Tests: is what was added tested? Do the tests cover edge cases and read cleanly without conditional logic?
  15. Complexity: functions with many independent paths (nested if/switch) that hurt understanding and testing?
  16. Observability: are errors logged with enough context to diagnose in production?

Output format

List only what needs to change, ordered by impact:

# Severity Principle Comment Rationale
1 🔴 Critical Security Unsanitized input on line 42 Allows SQL injection via the userId parameter
2 🟠 High SOLID/SRP OrderService does parsing, validation, and persistence Three reasons to change; should be split
… … … … …

Severities: 🔴 Critical (bug/security), 🟠 High (design/SOLID), 🟡 Medium (Clean Code/maintainability), 🔵 Low (style/suggestion).