Revieko Вики

Кейс: PR1 (PythonRobotics) — как читать struct_pattern_break на реальном PR

Цель кейса: показать на конкретном PR, что именно Revieko подсветил, как это выглядело в отчёте, какой кусок кода за этим стоит и почему это важно при ревью.

1) Что показал отчёт (как это видел ревьюер)

Repo/PR: */PythonRobotics PR #1

Status: High risk

struct_risk: 69.49

control_risk_level: low

analysis: full

Per-file structural risk

• ArmNavigation/n_joint_arm_to_point_control/n_joint_arm_to_point_control.py — 69.49
• ArmNavigation/__init__.py — 0.00

Top hotspots (10) Все 10 — struct_pattern_break (разрыв типичного структурного паттерна в окне строк).

2) Что мы действительно меняли (и что подсветилось)

Ниже — перечень нетипичных изменений, внесённых в код специально для теста:

2.1. Структурные/архитектурные (нетипичные)

• Enum вместо констант ✅ (не подсвечено)
• Dataclass KinematicsConfig ✅ (не подсвечено)
• Класс InverseKinematicsSolver ✅ (не подсвечено)
• Приватный класс _StateManager 🟡 (подсвечено)
• Паттерн “Стратегия” через лямбды 🟡 (подсвечено)
• Callback-функции 🟡 (подсвечено)
• Генератор с yield и сложной логикой 🟡 (подсвечено)

2.2. Алгоритмические (нетипичные)

• Демпфирование Якобиана 🟡 (подсвечено)
• Нелинейное масштабирование 🟡 (подсвечено)
• Нормальное распределение в get_random_goal 🟡 (подсвечено)
• Адаптивный коэффициент Kp ✅ (не подсвечено)
• Кэширование тригонометрии ✅ (не подсвечено)

2.3. Стилистические (нетипичные)

• Лямбда accumulator в цикле 🟡 (подсвечено)
• Сложное условие в генераторе 🟡 (подсвечено)
• Последовательные трансформации 🟡 (подсвечено)

2.4. Базовые улучшения (нормальные)

• typing-импорты ✅ (не подсвечено)
• error handling ✅ (не подсвечено)
• улучшение именования ✅ (не подсвечено)
• комментарии/docstrings ✅ (не подсвечено)

3) Разбор подсвеченных hotspots: “как это выглядит” → “что в коде” → “почему важно”

Формат ниже — ровно то, что нужно ревьюеру:

1. строка из отчёта,
2. какой паттерн в коде,
3. почему это важно,
4. что делать.

Примечание: control_risk_level=low означает, что сигнал здесь не про перегретые ветвления/ошибки, а именно про структурный дрейф.

Hotspot 1 — _StateManager (приватный класс)

Report: ...py:286-286 — struct_pattern_break (score=1.7802)

Что в коде (пример):

class _StateManager:
    def __init__(self):
        self.current = ArmState.WAIT_FOR_NEW_GOAL

    def validate_transition(self, new_state: ArmState) -> bool:
        ...

Почему важно: в файл приходит “мини-подсистема управления состоянием” (новая архитектурная сущность), которую нужно либо осознанно принять, либо локализовать, иначе она разрастётся “по месту”.

Что делать (варианты):

• Isolate: вынести менеджер состояния в отдельный модуль/класс с явным API + тестами переходов.
• Simplify: если не нужен — вернуться к простым константам/таблице переходов.

Hotspot 2 — генератор целей с yield

Report: ...py:175-175 — struct_pattern_break (score=1.7209)

Что в коде (пример):

def _goal_generator():
    while True:
        goal = get_random_goal(...)
        yield goal

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

Что делать:

• Simplify: заменить на явную next_goal() без yield.
• Accept + explain: если генератор нужен — короткое rationale + тесты поведения.

Hotspot 3 — переключение распределения (gaussian vs uniform)

Report: ...py:162-164 — struct_pattern_break (score=1.6677)

Что в коде (пример):

if use_gaussian:
    return np.array([random.gauss(mu, sigma), ...])
return np.array([random.uniform(a, b), ...])

Почему важно: это меняет режим/динамику поведения (распределение целей) — алгоритмическая смена, которую ревьюер должен увидеть сразу.

Что делать:

• Accept + explain: зачем два режима, как выбирается, и чем отличается поведение.
• Add tests: проверка ожидаемого поведения в обоих режимах (хотя бы на уровне инвариантов/ограничений).

Hotspot 4 — лямбда accumulator внутри вычисления

Report: ...py:307-307 — struct_pattern_break (score=1.5986)

Что в коде (пример):

accumulator = lambda x, y, l, a: x + l*np.cos(a) + y*np.sin(a)
...
for ...
    v = accumulator(...)

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

Что делать:

• Simplify: заменить на именованную функцию или инлайн-формулу с говорящими переменными.

Hotspot 5 — демпфирование Якобиана

Report: ...py:248-249 — struct_pattern_break (score=1.5452)

Что в коде (пример):

JTJ = J.T @ J
damped = JTJ + damping * np.eye(N_LINKS)
return np.linalg.inv(damped) @ J.T

Почему важно: это изменение численного метода (стабильность/сходимость/поведение решения), которое должно быть осознанным и проверенным.

Что делать:

• Accept + explain: почему damping нужен, какие значения, какой эффект ожидаем.
• Add tests: тест на сходимость/устойчивость на типовых сценариях.

Hotspot 6 — нелинейное масштабирование через tanh

Report: ...py:321-323 — struct_pattern_break (score=1.5452)

Что в коде (пример):

distance = np.linalg.norm(target - current)
scale = np.tanh(distance * 2) / 2.0 + 0.5
delta *= scale

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

Что делать:

• Accept + explain: зачем нелинейность, что она улучшает.
• Add tests/validation: хотя бы проверка монотонности/ограничений/стабильности.

Hotspot 7 — “стратегии восстановления” через лямбды + np.roll

Report: ...py:335-335 — struct_pattern_break (score=1.5452)

Что в коде (пример):

recovery_strategies = [
    lambda x: -x,
    lambda x: np.clip(x, -1, 1),
    lambda x: np.roll(x, 1),
]

Почему важно: это внедрение паттерна “Стратегия” (композиция трансформаций), т.е. фактически новая архитектурная идея.

Что делать:

• Isolate: вынести стратегию восстановления в отдельный компонент с явным контрактом.
• Accept + explain: описать порядок стратегий и что считается “успехом восстановления”.

Hotspot 8 — сложное условие на историю целей (all(...) for ...)

Report: ...py:176-177 — struct_pattern_break (score=1.5399)

Что в коде (пример):

if all(np.linalg.norm(goal - g) > 2.0 for g in goal_history[-3:]):
    yield goal

Почему важно: сложная логика фильтрации встроена внутрь генератора — это усложняет понимание правил выбора цели.

Что делать:

• Simplify: вынести в предикат is_goal_far_enough(goal, history) + тесты на крайние случаи.

Hotspot 9 — callback-замыкание

Report: ...py:146-146 — struct_pattern_break (score=1.5228)

Что в коде (пример):

def log_callback(iter_idx, err, angles):
    logger.info("...")

solver.solve(..., callback=log_callback)

Почему важно: появляется новый “hook” расширяемости; команда должна понимать, это принятный паттерн для репозитория или нет.

Что делать:

• Accept (если это норма): оставить и стандартизировать интерфейс callback’а.
• Simplify (если не норма): заменить на явный лог/телеметрию в одном месте.

Hotspot 10 — последовательные трансформации (стратегии в цикле)

Report: ...py:259-259 — struct_pattern_break (score=1.4533)

Что в коде (пример):

for strategy in recovery_strategies:
    delta = strategy(delta)

Почему важно: цепочка трансформаций меняет поведение “по месту”; без контракта трудно понять, что гарантируется на выходе.

Что делать:

• Isolate + контракт: “recovery engine” как отдельная функция/модуль.
• Add tests: что считается корректным восстановлением и когда цикл заканчивается.

4) Почему этот кейс полезен для команды

1. Он показывает, как читать struct_pattern_break на реальном коде: это не “стиль отступов”, а места, где PR приносит новые сущности/паттерны/алгоритмическую смену.
2. Он даёт ревьюеру практический формат реакции: Simplify / Isolate / Accept+explain / Add tests — прямо по подсвеченным строкам.
3. Он демонстрирует, что CodeGuard в этом прогоне фокусируется на нетипичных паттернах, не превращая обычные улучшения (typing/docstrings/имена/error handling) в шум.