---
type: review
work_item_id: ET-005
title: "Code Review: Переключение единиц измерения расстояний (км/мили)"
version: 1
status: approved
verdict: APPROVED
created_at: 2026-05-21
authors:
  - "agent:reviewer"
---

# Code Review — ET-005

## Вердикт

**APPROVED** (с комментариями).

P0/P1-findings нет. Зафиксировано 1×P2 и 2×P3 — все некритичные, не
блокируют merge. P2 — побочный эффект оркестратора (сброс выбора варианта
связки при переключении единиц), не нарушает требований ТЗ.

## Объект ревью

- Ветка: `feature/ET-005-`
- Код-коммит: `2fe5cfe` `feat(web): переключатель единиц измерения расстояний (км/мили)`
- Изменённые файлы кода:
  - `src/web/units.js` (новый, 190 строк)
  - `src/web/index.html` (+11 строк)
  - `src/web/app.js` (+118 / −24 строк)
  - `src/web/app.css` (+20 строк)
  - `tests/unit/units.test.js` (новый, 219 строк)
  - `tests/unit/test_unit_toggle.py` (новый, 211 строк)
  - `CHANGELOG.md` (+5 строк)
- Прочитано: `02-trz.md`, `03-acceptance-criteria.md`, `04-test-plan.yaml`,
  `06-adr/adr-0001-unit-toggle-client-side.md`, `10-tech-risks.md`,
  `CLAUDE.md`.

## 1. Соответствие ТЗ

### Функциональные требования

| Требование | Статус | Комментарий |
|------------|--------|-------------|
| ФТ-1 Кнопка-toggle в панели настроек карты | ✅ | `index.html:62-69` — сегментированный `.seg-control` в попапе `#terrain-popup`, после чекбокса POI, отделён `<hr>` |
| ФТ-2 Два состояния: km (default) / mi | ✅ | `units.js` `DEFAULT_UNIT = 'km'`, `VALID_UNITS = ['km','mi']` |
| ФТ-3 Пересчёт всех видимых расстояний | ✅ | Единый оркестратор `onUnitChange()` — карточки маршрутов, мини-карточка, лист точек, линейка, scale-bar, связка, «красивый» маршрут. См. R-01 (P2) — побочный сброс выбора варианта связки |
| ФТ-4 Сохранение выбора в `localStorage` (`distance_unit`) | ✅ | `units.js` `setUnit()` → `writeStored()`, ключ `distance_unit` |
| ФТ-5 При загрузке — чтение из `localStorage` | ✅ | `getUnit()` лениво читает ключ; `syncUnitToggleUI()` на `DOMContentLoaded` восстанавливает вид кнопок |

### Нефункциональные требования

| Требование | Статус | Комментарий |
|------------|--------|-------------|
| Пересчёт < 100 мс | ✅ | Чисто клиентская конвертация, нет ни сети, ни перезапроса тайлов |
| Кнопка доступна на всех размерах экрана | ✅ | Переиспользован адаптивный `.seg-control`; статически подтверждено `test_unit_toggle_reuses_seg_control_component`. Браузерный e2e на 375px (TP-05) — на этап тестирования |
| Не блокирует другие UI элементы | ✅ | Аддитивный ряд `.terrain-unit-row` с разделителем `<hr>`; `margin-bottom:0` для последнего элемента попапа |

### Технический дизайн ТЗ

Два пункта раздела «Технический дизайн» ТЗ скорректированы в ADR-0001 —
оба отклонения **обоснованы и легитимны**, дефектом не являются:

- путь `src/web/static/js/units.js` → `src/web/units.js` (ADR-0001 п.2:
  каталога `static/js/` нет, сборщика нет, `app.js` — классический
  скрипт; Вариант C отклонён);
- «все компоненты слушают `unitchange`» → единый оркестратор
  `onUnitChange()` (ADR-0001 п.6; Вариант B отклонён ради низкой
  связности).

Согласно CLAUDE.md ADR обладает архитектурным мандатом; раздел
«Технический дизайн» ТЗ носит рекомендательный характер. Реализация
следует ADR — это корректно.

Все функциональные и нефункциональные требования ТЗ выполнены.

## 2. Соответствие ADR-0001

| Пункт решения ADR | Статус | Комментарий |
|-------------------|--------|-------------|
| п.1 Только клиент; backend/БД/инфра без изменений | ✅ | Затронут только `src/web/` + тесты |
| п.2 Модуль `src/web/units.js`, подключён перед `app.js` | ✅ | `index.html:415` `units.js` строго до `app.js`; статически подтверждено `test_units_js_loaded_before_app_js` (R7) |
| п.3 Неймспейс `window.Units` (`getUnit`/`setUnit`/`formatDistance`/`KM_TO_MI`) | ✅ | Публичный контракт собран; дополнительно `module.exports` для Node-тестов |
| п.4 Каноническая единица — метры; конвертация только при форматировании | ✅ | `formatDistance(meters,...)` принимает метры; внутреннее состояние (`route.distance_m`, `rulerTotal`) не трогается |
| п.5 `localStorage` ключ `distance_unit`, дефолт `km` при отсутствии/мусоре | ✅ | `getUnit()` через `isValidUnit()` деградирует к `DEFAULT_UNIT` |
| п.6 Единый оркестратор — ровно одна подписка на `unitchange` | ✅ | `document.addEventListener('unitchange', onUnitChange)` единственная; подтверждено `test_app_js_single_unitchange_subscription` |
| п.7 Все 13 мест форматирования → `Units.formatDistance()`; GPX остаётся метрическим | ✅ | grep-аудит: вне `generateGPX()` захардкоженного `(m/1000).toFixed()+' км'` не осталось; GPX-экспорт намеренно метрический (R6), подтверждено `test_app_js_gpx_export_stays_metric` |
| п.8 UI на готовом `.seg-control` в `#terrain-popup` | ✅ | Новый CSS-компонент не вводится; добавлены только обёрточные стили `.terrain-unit-row` |
| п.9 C4 без изменений | ✅ | Состав компонентов не меняется |

Реализация полностью соответствует принятому ADR-0001. Риски R1–R8 из
`10-tech-risks.md` отработаны: R1 (call-sites) — централизация + аудит;
R2 (суб-км значения) — явная политика в `formatDistance()`; R3
(scale-bar) — отдельная unit-aware ветка + включение в оркестратор; R4
(разделитель) — единая запятая в `units.js` (по scale-bar см. R-02); R5
(точность) — параметр `precision`; R6 — GPX исключён; R7 — порядок
скриптов; R8 — переиспользование `.seg-control`.

## 3. Acceptance Criteria

| AC | Покрытие |
|----|----------|
| AC-1 Кнопка km/mi в панели, показывает выбор, клик переключает | ✅ `test_unit_toggle_present_in_html`; `syncUnitToggleUI()` переключает класс `.active` |
| AC-2 Пересчёт всех расстояний, коэф. 0.621371, округление до 1 знака | ✅ JS `TP-02`, `AC-2: KM_TO_MI`, `точность по умолчанию — 1 знак`. Карточки маршрутов — `precision:0` (R5, осознанная трактовка), суб-км в милях — 2 знака (R2) |
| AC-3 Сохранение и восстановление из `localStorage`, дефолт km | ✅ JS `TP-01`, `TP-03`, `недоступный localStorage` |
| AC-4 Кнопка не перекрывает элементы, mobile, переключение < 100мс | ✅ (структурно) `test_unit_toggle_has_styles`; e2e mobile (TP-05) — этап тестирования |

Поведенческое ядро (модуль `units.js`) покрыто полностью. Замечание по
покрытию слоя `app.js` — см. R-03 (P3).

## 4. Качество кода

Сильные стороны:

- **Единый источник истины.** `KM_TO_MI` объявлен ровно один раз;
  форматирование расстояний централизовано в `units.js`; 13 call-sites
  в `app.js` сведены к `Units.formatDistance()`. Дублирования нет.
- **Защитное программирование.** `readStored()`/`writeStored()` обёрнуты
  в `try/catch` (private mode); `dispatchEvent` защищён от headless-среды;
  `formatDistance()` корректно отдаёт `'-'` для `null/undefined/NaN`;
  `setUnit()` валидирует вход и не шлёт событие при неизменной единице.
- **Каноническое состояние в метрах.** Конвертация — только в момент
  форматирования; тест `TP-04: многократное переключение` подтверждает
  отсутствие дрейфа округления.
- **Консистентность с кодовой базой.** Глобальный неймспейс и
  классический скрипт соответствуют стилю `app.js`; паттерн
  `window._updateScaleZoom` зеркалит существующий `window._map`;
  блок-маркеры `>>> ET-005 ... >>>` повторяют приём ET-002.
- **Документированность.** JSDoc на всех публичных и приватных функциях
  со ссылками на ADR/риски; запись в `CHANGELOG.md`; commit —
  Conventional Commits (`feat(web):`).
- **Оркестратор аккуратен.** `renderLinkCards()`/`drawScenicRoutes()`
  идемпотентны (remove-before-add), повторный вызов из `onUnitChange()`
  не вызывает «layer already exists»; перерисовка связки/«красивого»
  маршрута огорожена флагами `linkMode`/`scenicMode`.

Замечания — см. findings R-01 (P2), R-02/R-03 (P3).

## 5. Качество тестов

- **Unit (`units.test.js`)** — высокое качество: тесты исполняют
  **реальный** `src/web/units.js` (сброс `require.cache` + инъекция
  моков `window`/`document`/`localStorage` перед каждым тестом).
  Покрыты TP-01..TP-04, AC-2, AC-3, граница 1000 м, недоступный
  `localStorage`, валидация `setUnit()`, публикация неймспейса.
- **Python (`test_unit_toggle.py`)** — статические проверки структуры
  `units.js`/`index.html`/`app.js` (ADR-0001, риски R1/R3/R6/R7) +
  запуск JS-раннера через `node --test` со `skip` при отсутствии `node`
  (по аналогии с `test_poi_toggle.py`).
- e2e TP-05 (mobile responsive) не реализован — Playwright-инфраструктуры
  в репозитории нет; отклонение задокументировано в шапке
  `test_unit_toggle.py`. На merge не влияет, относится к этапу
  тестирования.

## Findings

### R-01 — Переключение единиц сбрасывает выбор варианта связки (P2)

`onUnitChange()` (`app.js:2937-2938`) при активном режиме связки
вызывает `renderLinkCards(linkRoutes)`. В отличие от
`drawScenicRoutes(routes, activeIdx)`, функция `renderLinkCards()` **не
принимает индекс активного варианта** и всегда отрисовывает «Вариант 1»
как активный: карточке `i===0` присваивается класс `active`, линии
`link-line-0` — `line-width:5`. В коде нет переменной `activeLinkIdx` —
выбор существует только как класс в DOM и ширина линии на карте.

Следствие: пользователь, выбравший вариант 2/3 связки через
`selectLinkRoute()`, при каждом переключении км/мили теряет выбор —
подсветка карточки и акцент линии откатываются на вариант 1. Требования
ТЗ (ФТ-3 «пересчёт расстояний») формально выполнены — расстояния
пересчитываются корректно; это побочный дефект UX, ограниченный режимом
связки, поэтому P2, а не P1.

**Рекомендация:** ввести `activeLinkIdx` (по аналогии с
`activeScenicIdx`), обновлять его в `selectLinkRoute()` и после
повторного `renderLinkCards()` в `onUnitChange()` вызывать
`selectLinkRoute(activeLinkIdx)` для восстановления подсветки. Можно
поправить в этом же PR либо вынести в техдолг.

### R-02 — Scale-bar в режиме «mi» использует точку и латиницу (P3)

Масштабная линейка (`updateScaleZoom`, `app.js:1455`) формирует подпись
как `distance + ' ' + unit`, где `unit` — латинские `'mi'`/`'km'`/`'m'`,
а суб-единичные значения выводятся стандартным `Number.toString()` с
точкой (`'0.5 mi'`). Это расходится с `units.js`, который по R4 задаёт
запятую и русские подписи (`'0,5 ми'`). Митигация R4 в `10-tech-risks.md`
предписывает «единый разделитель для всех поверхностей».

Замечание ограничено P3: латиница и точка в scale-bar — **поведение,
существовавшее до ET-005** (в режиме «km» линейка и раньше показывала
`'30 km'`/`'0.5 km'`); ET-005 лишь расширил тот же стиль на мили и
консистентен внутри компонента. Полная унификация требует отдельного
форматирования подписи scale-bar и выходит за scope ET-005.
**Рекомендация:** при желании привести подпись scale-bar к запятой и
русским единицам — отдельной косметической правкой/техдолгом.

### R-03 — Слой `app.js` (оркестратор, scale-bar) покрыт только статически (P3)

Поведенческими тестами покрыт изолированный модуль `units.js`. Логика
`app.js` — `onUnitChange()` (перерисовка поверхностей), unit-aware ветка
scale-bar — проверяется лишь строковыми ассертами наличия в
`test_unit_toggle.py` (`test_app_js_*`). Поведение «после переключения
каждая видимая поверхность пересчиталась» (TP-02) не верифицируется
автоматически.

Это приемлемо: в репозитории нет DOM/MapLibre-харнесса для `app.js`
(монолитный non-module скрипт), и подход повторяет принятый в ET-002.
Сквозную проверку TP-02 закрывает этап тестирования. Фиксируется как
технический долг — учесть при будущей модуляризации фронтенда.

## Заключение

Реализация ET-005 корректна и полна: все функциональные и
нефункциональные требования ТЗ выполнены, ADR-0001 (пп.1–9) соблюдён
полностью, риски R1–R8 отработаны. Архитектурные отклонения от раздела
«Технический дизайн» ТЗ легитимны и зафиксированы в ADR. Дублирования
нет, форматирование централизовано, защитное программирование на месте,
модуль `units.js` тщательно покрыт unit-тестами на реальном коде.

Блокирующих замечаний нет.

**Вердикт: APPROVED.** R-01 (P2) рекомендуется поправить в этом же PR
либо осознанно вынести в техдолг; R-02/R-03 (P3) — на усмотрение
разработчика. Ни одно замечание merge не блокирует.