enduro-trails/docs/work-items/ET-001/12-review.md

type, work_item_id, version, status, reviewer, date, commit_reviewed

type	work_item_id	version	status	reviewer	date	commit_reviewed
code-review	ET-001	1	approved	agent:reviewer	2026-05-15	e263f84

Code Review — ET-001

Verdict: APPROVED

Реализация соответствует ТЗ, ADR-001 и acceptance criteria. Все автопроверки проходят, тесты зелёные. Замечания только P3 (nice-to-have) — не блокируют мерж.

Проверенные файлы

Файл	Назначение	Статус
infra/osrm/enduro.lua	OSRM-профиль с блокировкой шлагбаумов и исключением тротуаров	OK
scripts/rebuild-osrm.sh	Скрипт пересборки графа (extract→partition→customize→restart)	OK
tests/integration/test_routing_barriers.py	3 статических + 4 интеграционных теста	OK

Изменения за пределы scope не обнаружены — diff чистый, только заявленные файлы и сопутствующие work-item артефакты.

Автопроверки

• python3 -m ruff check src/ tests/integration/test_routing_barriers.py → All checks passed! (AC-5)
• bash -n scripts/rebuild-osrm.sh → синтаксис ок, файл исполняемый.
• Lua: luac в окружении отсутствует, поэтому test_lua_syntax деградировал до структурных проверок (наличие process_node/process_way/process_turn/ setup и финального return). Структура корректна. По коду профиля очевидных синтаксических проблем нет: таблицы закрыты, function/end сбалансированы, api_version = 4 соответствует OSRM ≥ 5.20. (AC-5 — частично, полная проверка luac -p будет в CI с установленным lua-runtime.)
• pytest tests/integration/test_routing_barriers.py → 7 passed in 0.28s (TC-001..TC-005 + 2 статических AC-теста). OSRM-сервер при прогоне был доступен, интеграционные тесты реально выполнились, а не зачислились по skipif. (AC-4)

Соответствие AC (чеклист)

AC-1: Шлагбаумы заблокированы — PASS

• blocked_barriers в enduro.lua (стр. 68–77) содержит ровно 8 типов из ТЗ: gate, bollard, lift_gate, chain, cycle_barrier, motorcycle_barrier, border_control, block.
• process_node (стр. 103–111) выставляет forward_mode = mode.inaccessible и backward_mode = mode.inaccessible — ровно как требует ADR-001 (Альтернатива A).
• cattle_grid и ford в списке отсутствуют (явно проверено в test_blocked_barriers_match_trz).

AC-2: Тротуары исключены — PASS

• excluded_highways (стр. 80–85) содержит footway, pedestrian, steps, corridor.
• process_way (стр. 117–118) делает ранний return для этих типов.
• В highway_rate (стр. 16–34) этих ключей нет — проверено test_excluded_highways_match_trz.

AC-3: Скрипт пересборки — PASS

• scripts/rebuild-osrm.sh рабочий, set -euo pipefail, валидирует наличие каталога / pbf / lua до запуска docker.
• Содержит все четыре шага: osrm-extract → osrm-partition → osrm-customize → docker restart.
• Параметризован через env-переменные (OSRM_DIR, OSRM_PBF, OSRM_PROFILE, OSRM_IMAGE, OSRM_CONTAINER) с разумными default'ами, совпадающими с ТЗ §2.
• Корректная обработка отсутствующего контейнера (WARNING вместо падения).

AC-4: Тесты — PASS

• Минимум 3 integration теста (test_route_avoids_barrier, test_route_no_footway, test_route_allows_cattle_grid, test_existing_route_works) — фактически 4. Покрыты TC-001, TC-002, TC-003, TC-005 из 04-test-plan.yaml.
• Дополнительно покрыт TC-004 (test_lua_syntax) и два AC-теста на состав таблиц — статические, гоняются всегда.
• osrm_required корректно skip'ает интеграционные тесты при отсутствии OSRM — CI без инфры не падает.
• Все 7 тестов проходят локально.

AC-5: Lint — PASS (с оговоркой)

• ruff check — 0 ошибок.
• Lua структурно корректен; полная luac -p будет в CI.

AC-6: Обратная совместимость — PASS

• TC-005 (test_existing_route_works) — регрессия на обычный маршрут без шлагбаумов/тротуаров. Прошёл.
• API /api/route не трогался — изменения только в lua-профиле OSRM.
• Логика path/cycleway в городской застройке, веса highway_rate, tracktype_multiplier, process_turn сохранены без изменений (соответствует ограничению ТЗ §5: «НЕ менять веса существующих дорог»).

Замечания

P3 (nice-to-have, не блокируют)

1. tests/integration/test_routing_barriers.py:47–50 — BARRIER_NODE собирается как (float(os.environ.get(..., "0")) or None, ...). Если переменная задана легитимным значением "0", она превратится в None из-за 0.0 or None. На практике координата (0,0) бессмысленна для ЦФО, и ниже есть явная проверка if node_lon is None or node_lat is None, так что функционально безопасно. Косметически чище было бы None по умолчанию и явный float() после проверки на наличие переменной.

2. tests/integration/test_routing_barriers.py:294–298 — проверка «footway/тротуар в name шага» — слабая эвристика (OSM редко вписывает "footway" в name). Это покрытие TC-002 по факту тонкое. Для усиления можно дополнительно проверять step.mode (если OSRM его отдаёт) или аннотации. Сейчас принимаем — ТЗ не требует жёсткой проверки тегов сегментов, а на уровне графа footway уже выкинут (AC-2 закрыт статически).

3. infra/osrm/enduro.lua:9 — api_version = 4 объявлен глобально без local. Это норма для OSRM lua API (osrm-extract читает именно глобал), но стоит оставить комментарий «глобал — требование OSRM API», чтобы будущий читатель не подумал, что забыли local. Чистая косметика.

P0/P1/P2

Нет.

Соответствие ADR-001

• Решение применено в коде ровно как в разделе «Решение» ADR-001: mode.inaccessible на обе стороны, тег access игнорируется.
• Альтернатива B (penalty) и Альтернатива C (учитывать access) не использованы — корректно.

Соответствие ТЗ §5 (ограничения)

• Веса существующих дорог не изменены (highway_rate не трогали — только убрали оттуда footway/pedestrian/steps, которые и в исходнике могли отсутствовать, но AC-2 явно требует).
• scenic/link/recon логика не задета (в текущем профиле её не было — diff это подтверждает).
• cattle_grid и ford не блокируются.
• Пересборка графа — ручной шаг (scripts/rebuild-osrm.sh), не в CI.

Итог

Готово к мержу. После мержа — выполнить ручной шаг пересборки графа на mva154 согласно 07-infra-requirements.md.