code-review-graph/code-review-graph-main/skills/project-review/references/review-checklist.md

Unified Review — Generic Checklist

Reference for the Layer-1 chain decomposition and gstack CRITICAL sub-pass. Load the language-specific checklist when available (java-spring.md, python-django.md, python-fastapi.md, node-express.md, go-gin.md, csharp-dotnet.md, rust.md, php-laravel.md, ruby-rails.md); otherwise use this generic list.

Layer 1 — eight categories

For each changed area mark ✅ Clean / ⚠️ Issues Found / — N/A.

1. Interface — parameter validation, response conventions, HTTP status codes, rate limiting, API versioning, protocol correctness
2. Business — logic aligns with requirements, state machine correctness, idempotency design, distributed locks
3. Data — SQL injection, query performance, index usage, transaction boundaries, cache invalidation
4. Utility — input validity, no side effects, error return values, date/time timezone handling
5. Error handling — exception classification, fallback logic, error message sanitization, retry with backoff
6. Security — AuthN/AuthZ, sensitive data masking, permission control, CSRF/XSS prevention
7. Performance — N+1 queries, caching strategy, connection pooling, batch operations, blocking in async paths
8. Observability — structured logging with correlation IDs, metrics, health checks

gstack CRITICAL sub-pass (highest severity)

SQL & Data Safety

• String interpolation in SQL — use parameterized queries
• TOCTOU check-then-set — use atomic WHERE + update
• Bypassing model validations for direct DB writes
• N+1 queries — missing eager loading

Race Conditions & Concurrency

• Read-check-write without uniqueness constraint / duplicate-key retry
• find-or-create without a unique DB index
• Status transitions not atomic (WHERE old_status = ? UPDATE ...)
• Unsafe HTML rendering on user-controlled data

LLM Output Trust Boundary

• LLM-generated values (emails, URLs, names) written to DB without format validation
• Structured tool output accepted without type/shape checks
• LLM-generated URLs fetched without an allowlist (SSRF)
• LLM output stored in knowledge bases without sanitization (stored prompt injection)

Shell Injection

• subprocess with shell=True AND interpolated command strings
• os.system() with variable interpolation
• eval()/exec() on LLM-generated code without sandboxing

Enum & Value Completeness

• New enum/status/tier values: read (not just grep) every consumer that switches/filters/displays the value
• Check allowlists and case/if-elsif chains for fall-through

Suppressions — do NOT flag

• Harmless redundancy that aids readability
• "Add a comment explaining a threshold" — thresholds drift
• Consistency-only changes
• Anything already addressed in the diff

Severity

• 🔴 blocker — must fix before merge (injection, secrets, missing transaction, auth bypass) → verdict FAIL
• 🟡 major — should fix before merge (missing validation, missing fallback, N+1, unmasked data)
• 🔵 minor — can optimize later (naming, duplicate code, comments)

Confidence calibration

• 9-10 verified by reading specific code
• 7-8 high-confidence pattern match
• 5-6 medium — show with caveat
• 3-4 low — move to appendix
• 1-2 speculation — suppress unless severity would be P0

Every finding: [SEVERITY] (confidence: N/10) file:line — problem → fix.