gendesign/.claude/agents/deep-review-phases/phase-5-verdict.md

Deep Review — Phase 5: Verdict + merge

Supporting reference. Verdict шаблон + auto-merge policy.

Verdict format (строго)

## Deep Code Review — verdict

### Summary
- **Status**: ✅ APPROVE / ⚠️ MINOR / 🟡 MEDIUM / 🟠 HIGH / 🔴 BLOCK
- **Files reviewed**: N (P0: a, P1: b, P2: c, P3: d)
- **Lines**: +X / -Y
- **Time spent**: ~M min
- **PR**: #N (если есть)

### 🔴 Critical (BLOCK push/merge)
- [ ] `path/file.py:42` — описание + конкретный fix (code snippet)
- [ ] `data/sql/95_foo.sql:12` — `ALTER TABLE ... NOT NULL DEFAULT 0` на 50M rows заблокирует таблицу. Split: ADD NULL → backfill batched → SET NOT NULL отдельным PR

### 🟠 High (должен быть пофиксен до merge)
- [ ] `path/file.ts:84` — race condition: state update в onSettled() при unmounted component
- [ ] `backend/app/api/v1/parcels.py:201` — N+1: цикл с `await db.execute(...)`, переделать на `IN (:ids)`

### 🟡 Medium (желательно fix, но не блокирующее)
- [ ] `file.py:120` — `except Exception: pass` swallow'ит ошибки

### 🟢 Low / nits (можно отдельным PR)
- [ ] `file.py:50` — naming `data` слишком generic, `parcels_by_district`

### Cross-file impact analysis
- `backend/app/services/site_finder/foo.py` изменил signature `find_parcels()` →
  callers обновлены: `api/v1/parcels.py:88` ✅, но `tasks.py:142` всё ещё передаёт старый kwarg ❌
- `frontend/src/types/site-finder.ts` regen нужен — `npm run codegen` не запущен в этом PR

### Vault cross-check
- Похожий fix был в `fixes/Bug_Worker_Ready_EarlyReturn.md` — паттерн использован корректно ✅
- Decision `decisions/2026-05-12-no-mutable-defaults.md` — этот PR не нарушает
- Если bug fix, но нет entry в `fixes/` → ⚠️ нужна docs (medium severity)

### Performance findings
- `EXPLAIN ANALYZE` на новом query `parcels.find_by_district`:
  - Seq Scan на `rosreestr_parcels` (5.2M rows) — нужен index `(district_id, status)`
  - Estimated cost 12.4k → с индексом 0.8

### Positive observations
- ✅ `ON CONFLICT DO UPDATE` использован корректно в bulk_upsert
- ✅ `CREATE INDEX CONCURRENTLY` в миграции

### Recommended next steps
1. Worker (backend-engineer): fix 🔴 #1 + 🟠 #1, push fixup commit
2. После fixup: run `npm run codegen` в frontend
3. Перед merge: проверить EXPLAIN на проде через MCP postgres
4. Создать vault entry `fixes/<this-bug>.md`

### Сomplexity / blast radius score
- **Risk**: Low / Medium / High / Critical
- **Reversibility**: Easy revert / DB migration needs manual rollback / Data loss on revert
- **Recommended merge window**: anytime / business hours only / pre-deploy freeze

Когда BLOCK (🔴) — обязательные триггеры

1. Hardcoded secret / token / password в commit (даже test files)
2. SQL injection vector
3. Auth bypass на admin endpoint
4. Migration без rollback plan (если меняет / удаляет данные)
5. Breaking API change без deprecation / migration path
6. Удаление prod данных без явного approval
7. --no-verify / --force / --amend в недавних коммитах ветки
8. Прямой push в main (HEAD == main, не feature branch)
9. ALTER TABLE ... NOT NULL DEFAULT на таблице >1M rows без batched backfill
10. CREATE INDEX (не CONCURRENTLY) на prod таблице >1M rows
11. DROP COLUMN / DROP TABLE без 2-stage rollout
12. Celery task без idempotency маркера (вернёт дубли при retry)
13. Scraper без dedup ключа в ON CONFLICT

Когда APPROVE (✅)

• Нет critical / high issues
• Cross-file impact проверен — все callers consistent
• Conventions OK (psycopg v3, httpx, ruff, TS strict)
• Backward compat сохранён
• Performance не регрессирует (или регрессия документирована и acceptable)
• Vault entry создана для нетривиальных fixes / decisions
• Tests есть для новой бизнес-логики (если applicable per scope)
• Pre-flight: на feature branch, нет force / no-verify в истории

Auto-merge policy (PR review mode)

При review открытых Forgejo PR — если verdict ✅ APPROVE (нет 🔴/🟠/🟡): мержи сам, любой scope (user override 2026-05-16: «после аппрув в ревью можешь мерджить все что угодно»).

Pre-merge checks (все обязательны)

1. curl -sH "$H" "$REPO/pulls/<N>" | jq '{state,mergeable,draft,head:.head.sha}' → state=open (не уже merged), mergeable=true (no conflicts), draft=false
2. Head SHA не изменился после твоего review (нет fixup push'ей с момента scan'а файлов)
3. CI status passing — Forgejo Actions UI или curl -sH "$H" "$REPO/commits/<sha>/status"

Merge команда (Forgejo)

# Через MCP (preferred):
mcp__forgejo__merge_pull_request  (Do=squash, delete_branch_after_merge=true)

# Или curl:
curl -sH "$H" -H "Content-Type: application/json" \
  -X POST "$REPO/pulls/<N>/merge" \
  -d '{"Do":"squash","delete_branch_after_merge":true}'

# Post-merge confirmation comment:
curl -sH "$H" -H "Content-Type: application/json" \
  -X POST "$REPO/issues/<N>/comments" \
  -d '{"body":"Merged via deep-code-reviewer — verdict ✅ APPROVE."}'

Forgejo API возвращает пустой body при успехе merge → verify через GET pulls/ что state == merged.

Когда НЕ мержить даже при APPROVE

• mergeable=false (conflicts) → comment "approved but has conflicts — rebase needed"
• CI failing → comment "approved but CI red — wait for green"
• Draft PR → comment "approved, ready when undrafted"
• Head SHA changed после твоего scan'а → НЕ мержь stale verdict, re-review нужен