---
name: review-checklist
description: "Чеклист и формат code review: severity levels (CRITICAL/MAJOR/MINOR/NIT), формат вердикта (APPROVED/CHANGES_REQUESTED/BLOCKED), Django anti-patterns, N+1 detection, dbt checks, verification gate, протокол обработки фидбэка, risk-first review. Загружать при проведении code review, при запуске Reviewer агента, при форматировании результатов ревью, или когда Builder получает CHANGES_REQUESTED."
user-invocable: false
---

# Review Checklist — Audit Platform

Формат и критерии code review для Reviewer агента.

## Severity Levels

| Level | Вердикт | Когда |
|-------|---------|-------|
| **CRITICAL** | → BLOCKED | Security vulnerability, data loss risk, требует человека |
| **MAJOR** | → CHANGES_REQUESTED | N+1, race condition, missing permissions, логика в view |
| **MINOR** | → CHANGES_REQUESTED | print() вместо logger, missing select_related, naming |
| **NIT** | → APPROVED + nits | Стилистика, можно упростить но работает |

## Формат замечания

```
[file:line] SEVERITY: Описание проблемы → Конкретное решение
```

Примеры:
```
[views.py:15] MAJOR: Бизнес-логика в view → Вынести в CompanyArchiveService
[models.py:42] MINOR: save() без update_fields → save(update_fields=["status", "updated_at"])
[api.ts:8] NIT: Можно использовать optional chaining → data?.name
```

## Формат вердикта

```markdown
## Результат ревью

**Задача:** task-XXX-slug
**Вердикт:** APPROVED | CHANGES_REQUESTED | BLOCKED

### Проверки:
- Scope: ok
- Security: ok
- Standards: ok
- Logic: ok
- Coverage: ok

### Замечания:
[file:line] SEVERITY: Issue → Fix

### Nits (не блокируют):
- [file:line] Suggestion

### Что хорошо:
- Positive note
```

## Standards Check (что проверять в diff)

| Что | Красный флаг | Правильно |
|-----|--------------|-----------|
| Модели | Не от TimestampedModel | Наследование от TimestampedModel |
| Логика | Бизнес-логика в views | В services.py |
| Queries | N+1 без select_related | select_related / prefetch_related |
| TypeScript | Использование `any` | Explicit types |
| Logging | `print()` / `console.log` | `logging.getLogger()` / structured |
| TODO | TODO / mock-данные в коде | Чистый финальный код |

### Assertion Quality (в тестовых файлах — MAJOR если нарушено)

| Что | Красный флаг | Правильно |
|-----|--------------|-----------|
| Backend truthy | `assert response is not None` / `assert result` | `assert response.status_code == 201` |
| Backend range | `assert status in [200, 201]` | `assert status == 201` |
| Backend count | `assert len(items) > 0` | `assert len(items) == 3` |
| Frontend count | `expect(items.length).toBeGreaterThan(0)` | `expect(items).toHaveLength(3)` |
| Frontend query | `queryAllByText('Save')` для кнопки | `getByRole('button', { name: 'Save' })` |
| Frontend DOM | `container.textContent).toContain(` | `within(container).getByText()` |
| Frontend state | Zustand test без afterEach reset | `afterEach(() => store.setState(initial))` |
| React Query | `useMutation` без invalidation test | Тест на refresh после мутации |

## Logic Check (что проверять в diff)

| Что | Красный флаг | Решение |
|-----|--------------|---------|
| N+1 queries | Цикл с ORM-запросом | prefetch_related, bulk operations |
| Race conditions | Shared state без transaction | `transaction.atomic()` / `select_for_update()` |
| Empty except | `except: pass` | Log + raise или handle конкретно |
| Edge cases | Нет проверки пустых данных | Guard clauses, default values |
| Idempotency | POST создаёт дубли | get_or_create или unique constraint |

## Примеры вердиктов

### APPROVED
```
Вердикт: APPROVED
Проверки: Scope ok, Security ok, Standards ok, Logic ok, Coverage ok
Что хорошо: логика в services.py, select_related, structured logging
Nits: [models.py:15] Можно добавить help_text
```

### CHANGES_REQUESTED
```
Вердикт: CHANGES_REQUESTED
[views.py:3] MAJOR: Бизнес-логика в view → Вынести в CompanyArchiveService
[views.py:5] MINOR: save() без update_fields → save(update_fields=["status", "updated_at"])
[views.py:9-11] MAJOR: N+1 — цикл с save() → Contact.objects.filter(...).update(...)
```

### BLOCKED
```
Вердикт: BLOCKED
1. CRITICAL: Hardcoded password — views.py:5 → секреты в .env
2. CRITICAL: SQL injection — views.py:6 → ORM вместо raw()
3. CRITICAL: Empty permission_classes → добавить [IsAuthenticated]
```

## Правила ревью

1. Ты **не пишешь код** — только указываешь проблемы
2. Конкретность обязательна: файл, строка, проблема, решение
3. Справедливость: не блокируй без реальной причины
4. Max 1 раунд CHANGES_REQUESTED (бесконечные ревью — анти-паттерн)
5. Проверяй **только diff** — не весь проект
6. Не предлагай рефакторинг вне Scope
7. **Scope check ПЕРВЫМ шагом:** `git diff --name-only` vs Scope задачи. Файлы вне Scope = auto CHANGES_REQUESTED
7. Проверяй Django anti-patterns в diff:
   - `request.data` / `request.query_params` без serializer → MAJOR (см. rules/backend/django.md "Parse, don't validate")
   - `related_name` на каждом FK/M2M (нет → MINOR)
   - `TextChoices` вместо строковых кортежей (нет → NIT)
   - `get_queryset()` для фильтрации, не override `list()` (нет → MINOR)
   - `assertNumQueries` в тестах с ORM-логикой (нет → NIT)
   - `.exists()` вместо `.count() > 0` (нет → NIT)
   - `SerializerMethodField` — можно ли заменить на annotate? (если да → NIT)

## dbt проверки (если diff содержит dbt/)

Если изменены файлы в `dbt/models/`:

```bash
# 1. Применить
docker compose exec web bash -c "dbt run --select <model> --project-dir ./dbt --profiles-dir ./dbt 2>&1 | tail -10"

# 2. Проверить колонку
docker compose exec web python manage.py shell -c "
from django.db import connection
cursor = connection.cursor()
cursor.execute(\"SELECT column_name FROM information_schema.columns WHERE table_schema='bi' AND table_name='<view>'\")
cols = [r[0] for r in cursor.fetchall()]
assert '<new_column>' in cols, 'MISSING'
print('OK')
" 2>&1 | tail -5
```

Без `dbt run` → новая колонка не в VIEW → Django OperationalError в prod.
Это **BLOCKED**, не "риск задокументирован".

## Verification Gate (перед любым заявлением о готовности)

```
ПЕРЕД заявлением о статусе или готовности:

1. IDENTIFY: Какая команда доказывает это заявление?
2. RUN: Выполни ПОЛНУЮ команду (свежую, не из кеша)
3. READ: Полный вывод, exit code, количество failures
4. VERIFY: Вывод подтверждает заявление?
   - НЕТ → Покажи реальный статус с evidence
   - ДА → Заяви с evidence
5. ТОЛЬКО ТОГДА: Делай заявление

Пропустил шаг = ложь, не верификация
```

| Заявление | Требуется | Недостаточно |
|-----------|----------|-------------|
| Тесты проходят | Вывод тестов: 0 failures | «должны пройти», прошлый запуск |
| Lint чистый | Вывод линтера: 0 errors | Частичная проверка |
| Build OK | Build exit 0 | «линтер прошёл» |
| Баг исправлен | Тест оригинального симптома | Код изменён, «должно работать» |

**Red Flags — СТОП:** слова «должно», «наверное», «вроде», «seems to», «probably»,
выражения удовлетворения ДО верификации («Готово!», «Сделано!»)

## Протокол обработки фидбэка (Builder после CHANGES_REQUESTED)

```
КОГДА получен фидбэк от Reviewer:

1. ПРОЧИТАЙ: Весь фидбэк целиком без реакции
2. ПОЙМИ: Переформулируй требование своими словами (или спроси)
3. ПРОВЕРЬ: Корректно ли для ЭТОГО кодовой базы?
4. ОЦЕНИ: Не ломает ли существующую функциональность?
5. РЕАГИРУЙ: Техническое подтверждение или аргументированный pushback
6. ИСПРАВЛЯЙ: По одному пункту, тестируй каждый
```

**Порядок исправлений:** blocking → простые фиксы → сложные

**Когда pushback:** suggestion ломает существующий код, reviewer не видит полный
контекст, нарушает YAGNI, технически некорректно для нашего стека

**Запрещено:** «Вы абсолютно правы!», «Отличное замечание!» — перформативное согласие.
Просто исправь и покажи результат.

## Risk-First Review (по файлам в diff)

Классифицируй файлы в diff по уровню риска и фокусируйся на HIGH первыми.

| Уровень | Триггеры в diff | Глубина проверки |
|---------|----------------|-----------------|
| **HIGH** | auth, permissions, crypto, external calls, validation удалена, денежные операции | Полный анализ + git blame удалённого кода |
| **MEDIUM** | Бизнес-логика, state changes, новые public API, services.py | 1-hop зависимости, input/output |
| **LOW** | Комментарии, тесты, UI, логирование | Беглый просмотр |

### Blast Radius

Для HIGH risk изменений оцени радиус поражения:
```bash
# Сколько мест вызывает изменённую функцию?
grep -rn "function_name" --include="*.py" | wc -l
```

- **50+ callers** + HIGH risk → обязательно adversarial analysis
- **10-50** → проверь ключевых потребителей
- **< 10** → стандартная проверка

### Red Flags — немедленная эскалация

Если видишь в diff:
- Удалён код из коммита с «security», «CVE», «fix» в сообщении
- Удалены `permission_classes` или `@login_required`
- Validation удалена без замены
- External calls добавлены без проверки ответа
- `.raw()` или `cursor.execute()` с f-string

→ **BLOCKED** до расследования, даже если остальное выглядит нормально.