Revieko Вики

Revieko — Ревью плейбук (PR)

Этот документ — набор плейбуков “что делать”, когда Revieko подсвечивает:

• высокий структурный риск (high struct),
• высокий control риск (high control),
• эффектные хвосты (IO/NET/DB/EXEC/LOG),
• связку USER_INPUT → (NET/DB/EXEC/LOG),
• partial analysis из-за лимитов/фолбэков.

Цель плейбуков — не заменить ревью, а быстро выбрать правильный тип реакции:

• Accept + explain
• Simplify
• Isolate
• Add tests / invariants
• Split PR / rerun with full analysis

Сначала прочитай маршрут: docs/PR_РЕВЬЮ_БЫСТРЫЙ_СТАРТ.

Справочник полей: docs/СПРАВОЧНИК_ПО_ОТЧЕТУ.

0) Общие принципы реакции

0.1. Важное: сигналы Revieko — не “вердикт”, а “карта внимания”

• Высокий риск не означает, что код неверный.
• Низкий риск не означает, что код безопасный или без багов.
• Смысл отчёта: дать 3–10 мест, куда человеку стоит посмотреть в первую очередь.

0.2. Три базовых действия ревьюера (и когда что выбирать)

• Simplify — когда сложность возникла “по пути”, без явной архитектурной причины.
• Isolate — когда изменение похоже на новую подсистему/паттерн/алгоритм.
• Accept + explain — когда сложность оправдана, но должна быть проговорена и зафиксирована (контракт/тест/комментарий).

0.3. Если есть invariant_violations — это “жёсткий сигнал”

Если включены командные правила (semantics/invariants), нарушения трактуются как:

• “либо исправить”,
• либо “осознанно исключить/переопределить правило”, но не игнорировать молча.

1) Быстрый выбор плейбука (30 секунд)

Смотри Summary:

1. analysis=partial → Playbook: Partial analysis (scope)
2. risk_level=high и hotspots в основном struct_pattern_break/depth_spike → Playbook: High struct
3. control_risk_level=high или hotspots control_outlier → Playbook: High control
4. В hotspots есть Effects/Taint, в JSON виден effect_density_tail/dangerous_flow_score →
   • если хвост “плотно эффектный” → Playbook: Effect tails
   • если есть USER_INPUT рядом с NET/DB/EXEC/LOG → Playbook: USER_INPUT → Effects

2) Playbook: High struct (структурная аномалия)

Сигналы

• risk_level=high и/или высокий struct_risk
• hotspots в основном:
  • struct_pattern_break
  • depth_spike
  • mixed (если доминирует форма/глубина)
• control_risk_level может быть low — это нормально: “форма нетипична, но ветвления не перегреты”.

Риск, который снижаем

• рост когнитивной нагрузки,
• “скрытая архитектура” внутри файла,
• появление нетипичных паттернов без контракта (потом это сложно сопровождать).

Быстрый чек (60 секунд)

1. Какой файл главный в Per-file structural risk?
2. Top-3 hotspots: что добавилось?
   • новые сущности (класс/подсистема/мини-DSL),
   • генераторы/yield,
   • лямбды/функциональные конструкции,
   • нетипичная математика/алгоритм,
   • резкий рост вложенности.
3. Ответь: это фича-архитектура или случайное усложнение?

Варианты действий

A) Accept + explain (сложность оправдана)

Подходит, если:

• изменение действительно приносит ценность (новый алгоритм/режим),
• есть тесты/контракт,
• место локализовано, читаемо и объяснено.

Действия:

• добавить короткий комментарий “почему этот паттерн нужен”,
• добавить тест(ы) на контракт/крайние случаи,
• если это “режим” — сделать явный флаг/конфиг и задокументировать.

Шаблон PR-комментария:

“CodeGuard подсветил структурный разрыв в <file:lines>. Изменение осознанное: <зачем>. Добавлены тесты <что проверяем> / комментарий к контракту.”

B) Simplify (снизить архитектурную массу)

Подходит, если:

• паттерн “красивый”, но не обязателен,
• можно выразить проще и ближе к стилю репозитория.

Техники упрощения:

• заменить лямбды на именованные функции,
• убрать генераторы: сделать простую функцию next_*,
• уменьшить вложенность (guard clauses, ранние return),
• разнести “вычисление” и “эффекты” (IO отдельно),
• сделать шаги алгоритма читаемыми (переменные, имена).

Шаблон PR-комментария:

“Есть структурный разрыв относительно нормы репозитория. Давай перепишем участок более императивно/плоско (без <lambda/yield/…>), чтобы снизить когнитивную нагрузку.”

C) Isolate (новая подсистема/паттерн)

Подходит, если:

• в файл “приехала” мини-архитектура (strategy/state/manager),
• появился новый алгоритм, который хочется переиспользовать,
• правка меняет режим работы и требует явного интерфейса.

Действия:

• вынести в отдельный модуль/класс,
• дать явный API,
• добавить минимальную документацию/README к модулю,
• добавить тесты на интерфейс, а не на внутренности.

Шаблон PR-комментария:

“Похоже на внедрение новой подсистемы. Давай вынесем в отдельный модуль с явным интерфейсом и тестами — так изменение станет локализованным и понятным.”

3) Playbook: High control (перегретые ветвления / error-path / return-tail)

Сигналы

• control_risk_level=high (даже если struct_risk средний/низкий)
• hotspots типа control_outlier
• В control_summary высокие значения:
  • branch_cond_risk
  • error_path_risk
  • return_path_risk
• Усилитель сигнала: высокий _effect_density
  • branch_cond_effect_density
  • error_path_effect_density
  • return_path_effect_density

Риск, который снижаем

• ошибки из-за сложных условий,
• “программы внутри обработки ошибок”,
• неожиданные side-effects на выходе (перед return/raise),
• тестируемость и предсказуемость поведения.

Быстрый чек (60 секунд)

1. Какая ось перегрета?
   • BRANCH_COND: условия слишком сложные
   • ERROR_PATH: обработка ошибок стала сложной
   • RETURN_PATH: перед return/raise происходит логика/эффекты
2. Есть ли эффекты внутри этих путей? (если да — это приоритет №1).

Рецепты рефакторинга (по оси)

A) Перегретые условия (BRANCH_COND)

Типовые проблемы:

• длинные if с вычислениями и IO,
• цепочки elif как mini-state-machine,
• сложные условия без имен.

Что делать:

• вынести условия в именованные функции (is_*, should_*),
• разорвать на guard clauses,
• “таблица решений” вместо цепочек elif,
• перенос эффектов (IO/NET/DB) вне условных выражений.

PR-комментарий:

“Control-слой показывает перегретые условия. Предлагаю вынести условия в именованные предикаты и убрать эффекты из условий — так снизим риск ошибок и улучшим читаемость.”

B) Сложный error-path (ERROR_PATH)

Типовые проблемы:

• except делает половину бизнес-логики,
• много эффектов в обработчиках ошибок,
• смешаны компенсации/логирование/ретраи.

Что делать:

• минимизировать работу в except: собрать контекст → вызвать handle_error(...),
• унифицировать обработку ошибок (один путь),
• явные политики retry/backoff,
• тесты на ошибочные сценарии (обязательны, если меняется поведение).

PR-комментарий:

“Error-path стал сложным. Давай вынесем обработку ошибок в отдельные функции/модуль, чтобы except не превращался в программу внутри программы.”

C) Тяжёлые return/raise-хвосты (RETURN_PATH)

Типовые проблемы:

• перед return выполняются эффекты (лог/запись/сеть),
• хвост меняет контракт функции “на выходе”.

Что делать:

• разделить: вычислить результат → отдельно выполнить эффекты → вернуть,
• явно документировать контракт (что гарантируется до return),
• тесты на контракт “на выходе”.

PR-комментарий:

“В return-path появилась тяжёлая логика/эффекты. Опасно менять контракт на выходе. Предлагаю разделить вычисление результата и эффекты и покрыть контракт тестами.”

4) Playbook: Effect tails (плотный хвост с NET/DB/FILE/EXEC/LOG)

Сигналы

• В JSON/Full report высокий effect_density_tail (по файлу или по PR)
• TL;DR/подсказки уровня “Хвост с NET/DB/FILE-эффектами…”
• В hotspots есть Effects (например NET_IO+DB+LOG) без явных Taint

Риск, который снижаем

• сложная, плохо тестируемая “эффектная” концовка функций/модулей,
• скрытая зависимость от порядка операций,
• неявная транзакционность (частичный успех),
• нестабильность на ошибках (частичное состояние).

Быстрый чек (2 минуты)

1. Что это за эффекты в хвосте?
   • DB / NET_IO / EXEC — высокий приоритет
   • LOG — зависит от контекста, но при большом объёме может быть “шумом”
2. Есть ли явная граница “pure → effects”?
3. Что происходит при ошибке в середине хвоста?
4. Есть ли идемпотентность / транзакционность / компенсации?

Рекомендуемые действия

• Вынести эффекты в отдельный слой/функцию (persist_*, send_*, exec_*)
• Сделать порядок операций явным (и задокументировать)
• Добавить тесты:
  • на частичные ошибки,
  • на повторный запуск (идемпотентность),
  • на таймауты/ретраи (если сеть).

PR-комментарий:

“В хвосте файла/функции концентрируются IO/NET/DB эффекты. Давай выделим ‘эффектный слой’ и добавим тесты на ошибки/идемпотентность, чтобы поведение было предсказуемым.”

5) Playbook: USER_INPUT → (NET/DB/EXEC/LOG) (подозрительные потоки)

Сигналы

• dangerous_flow_score > 0
• hotspot имеет taint_hint=USER_INPUT и effect_hint включает NET_IO/DB/EXEC/LOG
• TL;DR/подсказки “USER_INPUT рядом с NET/DB/LOG…”

Риск, который снижаем

Это не полноценный dataflow-анализ, но практический риск понятен:

• входные данные могут попадать в сеть/БД/exec/log без проверок,
• риск инъекций/утечек/лог-спама/неожиданных side-effects.

Быстрый чек (2–3 минуты)

1. Где граница доверия?
   • валидация, нормализация, ограничения, escaping
2. Есть ли allowlist (параметры/команды/поля)?
3. Логируются ли входные данные? Если да — нет ли PII/секретов?
4. Для DB/NET/EXEC: есть ли явный слой, который отвечает за безопасность/контракты?

Рекомендуемые действия (по типу эффекта)

A) USER_INPUT → DB

• параметризованные запросы/ORM
• строгая схема валидации
• ограничения длины/формата
• тесты на “плохие” входы

B) USER_INPUT → NET_IO

• allowlist доменов/эндпоинтов
• timeouts/retries policy
• ограничение размеров payload
• логирование без утечек

C) USER_INPUT → EXEC

• запрет или строгий allowlist
• никакой сборки shell-команд строками
• отдельный слой “исполнитель”, покрытый тестами

D) USER_INPUT → LOG

• маскирование/редакция
• ограничение объёма
• исключение секретов/PII

PR-комментарий:

“Есть участок, где USER_INPUT находится рядом с NET/DB/EXEC/LOG. Давай явно зафиксируем границы: валидацию/allowlist/маскирование и добавим тесты на небезопасные входы.”

6) Playbook: Partial analysis (scope: PR содержит непрофильные файлы)

Сигналы

• analysis=partial

Что это означает

PR содержит файлы вне области анализа CodeGuard (на текущем этапе — не Python: YAML/CSV/Markdown и т.п.). Для Python-части отчёт остаётся полноценным.

Риск, который снижаем

• пропустить важные изменения в конфигурации/данных/документации, потому что “инструмент ничего не сказал”.

Что делать ревьюеру (быстрый сценарий)

1. Python-изменения ревьюим по CodeGuard: per-file risk → top hotspots → решение.
2. Непрофильные файлы ревьюим вручную:
   • конфиги: валидность схемы, backward compatibility, секреты, окружения
   • данные/CSV: формат, миграции, размер/дифф-шум
   • Markdown/доки: корректность инструкций/примеров/команд

Комментарий в PR (корректная формулировка)

“analysis=partial — в PR есть непрофильные файлы (конфиги/данные/доки), CodeGuard анализирует только Python-часть. По этим файлам делаем обычное ручное ревью.”

7) Мини-шаблоны “что попросить в PR” (коротко, но по делу)

7.1. Когда просить “обоснование”

“Это осознанная архитектурная идея или случайное усложнение? Если осознанная — добавь краткое rationale и тест на контракт.”

7.2. Когда просить “локализацию”

“Похоже на новую подсистему. Давай вынесем в отдельный модуль/класс с явным API.”

7.3. Когда просить “тесты”

“Изменение меняет режим/контракт/эффектный хвост. Нужны тесты на ошибки/крайние случаи/идемпотентность.”

7.4. Когда просить “split PR”

“Сейчас analysis partial. Разбей PR или уменьшай hunks, чтобы отчёт был полным.”

8) Полезная привычка команды

Если CodeGuard регулярно подсвечивает одни и те же классы проблем, команда может:

• формализовать правила в .revieko/semantics.yaml (invariants),
• закрепить стиль “эффекты отдельно от вычислений”,
• договориться: “если внедряем новый паттерн — isolate + контракт + тест”.

Это превращает “одноразовый сигнал” в системное улучшение архитектуры.