reviewer(ET): auto-commit from reviewer run_id=32

docs/work-items/ET-007/12-review.md

@@ -1,416 +1,355 @@
 ---
 type: code-review
 work_item_id: ET-007
-title: "Review: Спутниковая карта (Схема / Спутник) — артефакты анализа и архитектуры"
-version: 1
-status: REQUEST_CHANGES
+title: "Review v2: Спутниковая карта (Схема / Спутник) — артефакты + код"
+version: 2
+status: APPROVED_WITH_COMMENTS
 created_at: 2026-05-31
 updated_at: 2026-05-31
 authors:
   - "agent:reviewer"
 branch: feature/ET-007-et-005
-stage_reviewed: analysis + architecture
+stage_reviewed: analysis + architecture + development (code + tests)
+previous_review: v1 (REQUEST_CHANGES, 6 P1 / 6 P2 / 3 P3)
 ---
 
-# Code Review — ET-007: Спутниковая карта (Схема / Спутник)
+# Code Review v2 — ET-007: Спутниковая карта (Схема / Спутник)
 
 ## Вердикт
 
-**REQUEST_CHANGES** — найдено 6 P1-блокирующих findings и 6 P2/3
-nice-to-have. Кода в ветке ещё нет (только документация анализа и
-архитектуры); цель ревью — не пропустить эти расхождения в
-разработку, иначе разработчик будет вынужден угадывать намерения
-или возвращаться за уточнениями.
+**APPROVED with comments.**
 
-Сами решения (Esri World Imagery; ленивый raster-source; восстановление
-через `rebuildMapOverlays()`; гибридное halo) — обоснованы и
-непротиворечивы внутри ADR-004. Проблема в нестыковках между ТЗ /
-ADR / Data / Tech-Risks и в нескольких ссылках на сущности в коде
-(`trails-grade1..5`, `paths-bridleway`, baseline-цвета), которых в
-реальном `src/web/` нет.
+Все 6 P1-блокеров из v1 закрыты в спецификации **и** в коде. Реализация
+корректна, прошли все 22 pytest-проверки (`tests/unit/test_base_layer.py`)
+и все 33 поведенческих JS-теста (`tests/unit/base_layer.test.js`),
+запускаемых под `node --test`. Архитектурные решения (Esri, M-A, S-B,
+O-A, H-B) соблюдены в коде один-в-один; никаких отклонений от ADR-004
+не обнаружено.
+
+Остаются 4 не-блокирующих замечания P2 (часть из них — несвёрнутые
+концы v1 P2) и 4 nice-to-have P3, включая мелкие расхождения между
+текстом TRZ и фактической реализацией (код в одной точке делает чуть
+больше, чем требует ТЗ — добавляет защитный `beforeId`). Эти правки
+рекомендуется внести следующим коммитом, но они не препятствуют
+переходу в `testing` / merge.
 
 ## Что проверено
 
-- `docs/work-items/ET-007/01-brd.md` (v1, status draft)
-- `docs/work-items/ET-007/02-trz.md` (v1, status draft)
-- `docs/work-items/ET-007/03-acceptance-criteria.md` (v1, status draft)
-- `docs/work-items/ET-007/04-test-plan.yaml` (v1)
-- `docs/work-items/ET-007/04b-ui-test-cases.md` (v1) — просмотрено
-- `docs/work-items/ET-007/06-adr/ADR-004-satellite-base-layer.md` (accepted)
-- `docs/work-items/ET-007/07-infra-requirements.md` (approved)
-- `docs/work-items/ET-007/08-data-requirements.md` (approved)
-- `docs/work-items/ET-007/10-tech-risks.md` (approved)
-- `docs/architecture/README.md` (обновлён ET-007)
-- `docs/architecture/adr/README.md` (добавлен ADR-004)
-- Diff `origin/main...HEAD`: 12 файлов, 2020 вставок, 1 удаление —
-  только `docs/`. Кода в ветке нет; ревью на этапе разработки
-  понадобится повторно.
-- Сверка с фактическим кодом: `src/web/app.js` (3132 строки),
-  `src/web/style.json` (136), `src/web/style-dark.json` (136).
-- CLAUDE.md
+### Артефакты (вторая итерация)
+- `01-brd.md` v2 — P1-3 закрыт (риск №4: «авто-выключение hillshade
+  не вводится»).
+- `02-trz.md` v2 — P1-1, P1-2, P1-4, P1-5, P1-6 закрыты; добавлены
+  §5.6 (контракт с `layerState.basemap`) и §5.7 (синхронизация halo).
+- `03-acceptance-criteria.md` v2 — добавлены сценарии под P1-2/P1-5/P1-6.
+- `06-adr/ADR-004-satellite-base-layer.md` (accepted) — добавлены
+  §8 и §9 под P1-5 и P1-6; §5 переписан под реальные halo-id;
+  §6 единая константа `#2a2a2a`; baseline dark исправлен `#1a1a2e`.
+- `08-data-requirements.md` v2 — таблицы 5.1 / 5.2 (baseline POI per-
+  theme и satellite-значения), `_savedBasemapState`, исправлен dark
+  baseline на `#1a1a2e`.
+- `10-tech-risks.md` v2 — R-1 переписан под реальные id; R-5
+  обновлён с baseline `#1a1a2e`.
+- `04-test-plan.yaml`, `04b-ui-test-cases.md`, `07-infra-requirements.md`
+  — без изменений (v1 не требовали правок по тем findings).
+
+### Код (новое в ветке)
+- `src/web/index.html` (+11 строк) — блок `#base-seg` в `#terrain-popup`.
+- `src/web/app.css` (+30 строк) — стили `.terrain-base-row`,
+  `.base-seg`, CSS-hook `body.satellite-active #btn-basemap`.
+- `src/web/app.js` (+368 строк) — блок ET-007, хук в
+  `rebuildMapOverlays()`, синхронизация halo в `onTrailsCheckbox()` /
+  `restoreTrailsState()`, инициализация в обеих ветках IIFE.
+- `src/web/style.json` (+30) / `src/web/style-dark.json` (+30) — два
+  halo-underlay-слоя `trails-track-halo-satellite` и
+  `trails-path-bridleway-halo-satellite`, оба с `visibility: none` и
+  размещены **перед** соответствующим базовым trails-слоем (z-order).
+
+### Тесты
+- `tests/unit/test_base_layer.py` (+301 строк) — 22 статических теста
+  (HTML/CSS/JS-структура, halo-слои в обоих стилях, порядок halo
+  перед базовым слоем, `restoreBaseLayerState()` первым в
+  `rebuildMapOverlays()`, ≥4 вызова в init-путях) + сабпроцесс
+  `node --test` для JS-suite.
+- `tests/unit/base_layer.test.js` (+468 строк) — 33 поведенческих
+  unit-теста через `new Function`-загрузку блока ET-007, с мок-DOM,
+  мок-localStorage и мок-картой; покрывают U-01..U-05, U-10..U-11,
+  I-01..I-04, I-07, halo, POI text-color/halo (P1-2), background P1-4
+  обе темы, валидацию входа, отсутствие window._map, недоступный
+  localStorage, z-order.
+
+### Прогон
+```
+$ python -m pytest tests/unit/test_base_layer.py -v
+22 passed in 0.16s
+
+$ node --test tests/unit/base_layer.test.js
+# tests 33, pass 33, fail 0
+```
+Полный `pytest tests/` падает только на `tests/unit/test_health.py`
+из-за отсутствующего `shapely` в окружении — это инфраструктурная
+проблема, не относится к ET-007.
 
 ---
 
-## P1 — must-fix перед началом разработки
+## P1 — must-fix
 
-### P1-1 — TRZ / ADR / Data / Tech-Risks ссылаются на несуществующие слои
+**Нет.** Все 6 P1-findings из v1 закрыты.
 
-**Где:**
-- TRZ §1 REQ-F-04 таблица — строки «Trails (grade1..5)» и «Paths/bridleway».
-- ADR-004 §5 пункт 1 — «Для линий **grade1..5** и paths/bridleway… парные
-  underlay-слои `*-halo-satellite`».
-- Data §6 таблица — `trails-grade1`, `trails-grade2`, `paths-bridleway`,
-  и т.д.
-- Tech-Risks R-1 — «массив `['trails-grade1', 'trails-grade2', ...]` с
-  производным правилом `<base>-halo-satellite`».
+| v1 ID | Категория | Где закрыто |
+|-------|-----------|-------------|
+| P1-1 | Несуществующие слои grade1..5 | TRZ §1 REQ-F-04, ADR-004 §5, Data §6, Tech-Risks R-1 — реальные id `trails-track-halo-satellite`, `trails-path-bridleway-halo-satellite` |
+| P1-2 | POI text-color на спутнике | TRZ §1 REQ-F-04-POI, ADR-004 §5, Data §5.2, код `_applyPoiSatellitePaint()`, JS-тесты «P1-2» |
+| P1-3 | BRD vs TRZ hillshade | BRD §5 риск 4 переписан под TRZ/ADR/AC: авто-выключение не вводится |
+| P1-4 | background-color 3 источника | ADR-004 §6, TRZ §1 REQ-F-03, Data §5 — единая `#2a2a2a` для обеих тем; baseline dark `#1a1a2e`; JS-тесты обе темы |
+| P1-5 | Контракт с `layerState.basemap` | TRZ §5.6, AC-02/AC-03 новые сценарии, ADR-004 §8, код `_savedBasemapState` + `_setBodyClass('satellite-active', …)`, CSS `body.satellite-active #btn-basemap { display:none !important }` |
+| P1-6 | halo не синхронизирован с чекбоксами | TRZ §5.7, AC-04 новые сценарии, ADR-004 §9, код `_applyTrailHaloVisibility(map, base)` + хуки в `onTrailsCheckbox()` и `restoreTrailsState()` |
 
-**Что в коде:** в `src/web/style.json` (стр. 42–92) и `style-dark.json`
-определены только три line-слоя поверх OSM:
-
-| id в style.json | filter | как обрабатываются grade |
-|---|---|---|
-| `trails-asphalt`         | `highway in primary/secondary/tertiary/residential` | без grade |
-| `trails-track`           | `highway == track`                                   | **одно** paint-выражение `match` по `tracktype` (`grade1..5`) — см. style.json:64–72 |
-| `trails-path-bridleway`  | `highway in path/bridleway/footway`                  | без grade |
-
-Слоёв `trails-grade1`, `trails-grade2`, …, `trails-grade5`,
-`paths-bridleway` **нет** ни в одном из стилей. Поэтому:
-
-- Halo-стратегия H-B (пара underlay-слоёв на каждый base-слой) на
-  практике даёт 2–3 пары (`trails-track-halo-satellite`,
-  `trails-path-bridleway-halo-satellite`, опционально
-  `trails-asphalt-halo-satellite`), а **не** 5+ пар, как написано.
-- Чтобы получить «разный halo по grade», halo-слой `trails-track-halo-satellite`
-  должен сам содержать `match` по `tracktype` (если halo должен
-  зависеть от grade) — либо halo единого цвета на весь `trails-track`.
-  ADR не оговаривает.
-- Tech-Risks R-1 митигация «массив `['trails-grade1', …, 'trails-grade5']`»
-  введёт разработчика в заблуждение и приведёт либо к `addLayer` от
-  несуществующих base, либо к молчаливому `getLayer === undefined`.
-
-**Действие:** обновить TRZ §1 REQ-F-04, ADR-004 §5, Data §6, Tech-Risks
-R-1, чтобы они оперировали реальными id (`trails-track`,
-`trails-path-bridleway`, при необходимости `trails-asphalt`) и явно
-зафиксировали, нужен ли halo, чувствительный к `tracktype`, или
-единый halo на весь `trails-track`.
-
-### P1-2 — Контраст POI labels на спутнике спецификацией сломан
-
-**Где:**
-- TRZ §1 REQ-F-04 таблица, строка «POI labels» — «`text-halo-color: #000000`,
-  `text-halo-width: 2px` для читаемости на спутнике».
-- ADR-004 §5 пункт 2 — «`text-halo-color` (`#000000` на спутнике …) и
-  `text-halo-width` (`2` на спутнике …) — через `setPaintProperty`».
-- TestPlan I-24 — «`text-halo-color` === `#000000`».
-
-**Что в коде:** `style.json:128–133` для `poi-labels`:
-```
-"text-color": "#333333",
-"text-halo-color": "#ffffff",
-"text-halo-width": 1.5
-```
-В `style-dark.json` аналогично, `text-color` тёмный.
-
-Меняем halo с белого на чёрный, при этом текст остаётся `#333333` —
-**тёмный текст на тёмном halo не читаем**. Это прямо нарушает цель
-требования («для читаемости на спутнике»).
-
-**Действие:** в TRZ/ADR явно поменять на спутнике **`text-color`**
-тоже (например, `#ffffff` или `#f0ede6`) или зафиксировать другую
-схему контраста (инвертированная пара). Если на схеме `text-color`
-должен оставаться `#333333` — это нужно вернуть в `applyBaseLayer('schematic')`,
-т.е. baseline POI text-color должен попасть в перечень «исходных
-значений» Data §5.
-
-### P1-3 — Прямое противоречие BRD ↔ TRZ/ADR/AC насчёт hillshade
-
-**Где:**
-- BRD §5 риск 4 (hillshade): «По умолчанию hillshade **отключается**
-  при включении спутника — поведение фиксируется в TRZ».
-- TRZ §1 REQ-F-04 таблица, строка «Hillshade»: «Включается/выключается
-  чекбоксом как раньше; **по умолчанию НЕ авто-выключается**».
-- ADR-004 §«Открытые вопросы» / Раздел «контекст / 1.5»: «оставить
-  как есть».
-- AC-04 сценарий «Hillshade поверх спутника»: ожидается одновременный
-  показ обоих слоёв (т.е. поведение без авто-выключения).
-
-BRD говорит «отключать», все нижестоящие документы — «не отключать».
-Развилка проектная, нужно выбрать одну версию.
-
-**Действие:** обновить BRD §5 риск 4, чтобы он совпадал с TRZ/ADR/AC
-(митигация = «hillshade продолжает работать поверх спутника как и
-поверх схемы; визуальную проверку даёт UI-тест AC-04»). Если же
-правильное решение всё-таки «авто-выключать» — нужно
-синхронно править TRZ, ADR-004, AC-04 и TestPlan.
-
-### P1-4 — Цвет фона `background-color` в режиме «Спутник» — три расходящихся источника
-
-**Где:**
-- TRZ §1 REQ-F-03: «цвет фона `#2a2a2a` для тёмной темы и **`#1a1a1a`
-  для светлой темы** в режиме «Спутник»» — две константы, причём
-  светлая получает **более тёмный** фон (`#1a1a1a` < `#2a2a2a`), что
-  выглядит как опечатка / перестановка.
-- ADR-004 §6: одна константа `#2a2a2a` для обеих тем
-  (`setPaintProperty('background', 'background-color', '#2a2a2a')`).
-- Data §5 строка «baseline-значения `background-color`»: `#f0ede6`
-  (light), **`#1a1a1a`** (dark) — но фактическое значение в
-  `style-dark.json:28` — `"#1a1a2e"` (а не `#1a1a1a`).
-
-Итого: четыре цвета упомянуто, два не совпадают с фактическим стилем
-и сам ADR противоречит ТЗ по количеству констант.
-
-**Действие:**
-1. Выбрать модель: «одна satellite-bg-константа на обе темы» (ADR-вариант,
-   проще) или «по константе на тему» (TRZ-вариант, эстетичнее под тёмные
-   снимки). Зафиксировать в TRZ и ADR одинаково.
-2. Если TRZ-вариант — проверить, что светлая тема не получает более
-   тёмный фон, чем тёмная (исправить вероятную опечатку
-   `#1a1a1a`/`#2a2a2a`).
-3. В Data §5 baseline тёмной темы заменить `#1a1a1a` → `#1a1a2e`
-   (фактическое значение из `style-dark.json:28`).
-
-### P1-5 — В `app.js:380–386` уже есть `layerState.basemap: ['osm-base']` и `toggleLayer('basemap')`; контракт взаимодействия не описан
-
-**Где (код, app.js:380):**
-```js
-const layerState = { tracks: true, paths: true, poi: true, basemap: true };
-const layerGroups = { …, basemap: ['osm-base'] };
-function toggleLayer(group) { …setLayoutProperty('osm-base', 'visibility', …) }
-```
-Это существующий UI-механизм, способный включать/выключать `osm-base`
-независимо от подложки. Ни TRZ, ни ADR-004 его не упоминают.
-
-**Открытые вопросы (не отвечены ни в одном артефакте):**
-- При `applyBaseLayer('satellite')` мы выставляем `osm-base.visibility=none`.
-  Что делать с `layerState.basemap`? Сбрасывать в `false` (тогда после
-  возврата на «Схему» пользователь обнаружит, что схема скрыта) или
-  оставлять `true` (тогда `layerState` рассинхронизирован с реальной
-  видимостью)?
-- При возврате на «Схему» (`applyBaseLayer('schematic')`) мы безусловно
-  выставляем `osm-base.visibility=visible` (TRZ §5.2 шаг 3.1). Если у
-  пользователя `layerState.basemap === false`, это нарушит его выбор.
-- В UI существует ли кнопка `btn-basemap` (упоминается в `toggleLayer`)?
-  Что с ней делает «Спутник»: скрыть, заблокировать, оставить как
-  «выключатель схемы поверх спутника»?
-
-**Действие:** в TRZ §5.2 (алгоритм `applyBaseLayer`) и в AC-02/AC-03
-добавить раздел «Взаимодействие с существующим toggleLayer('basemap')».
-Рекомендация — на спутнике переключатель «Базовая карта» либо скрыт
-из UI, либо означает «показывать схему поверх спутника» (гибрид) —
-но это уже out of scope (BRD §3). Самый простой контракт: при входе
-в «Спутник» запомнить `layerState.basemap`, при выходе — восстановить.
-
-### P1-6 — `restoreTrailsState()` и обработчики «Грунтовки»/«Тропы» не управляют halo-слоями
-
-**Где (код, app.js:2798–2821):** `restoreTrailsState()` выставляет
-`visibility` только для `trails-track` и `trails-path-bridleway`.
-Обработчики чекбоксов «Грунтовки» (`onTrailsTrackCb` / аналогично) — тоже.
-
-В режиме «Спутник» при выключении чекбокса «Грунтовки» базовый слой
-скрывается, а **halo-underlay-слой `trails-track-halo-satellite`
-останется видимым** (его никто не трогает). Получим «фантом» белой
-линии на спутнике без основной линии сверху.
-
-Ни TRZ, ни ADR-004 не правят `restoreTrailsState()` и обработчики
-чекбоксов. Контракт неполный.
-
-**Действие:** в TRZ §5 (либо отдельный пункт «синхронизация halo с
-чекбоксами слоёв») задать, что:
-- `restoreTrailsState()` должен учитывать текущий base layer и
-  одновременно править видимость пары halo-слоя;
-- обработчики чекбоксов «Грунтовки» / «Тропы» — то же самое;
-- источник истины: «halo видим ⇔ (base === satellite) AND (соответствующий
-  чекбокс ON)».
-
-То же замечание про POI: при выключении чекбокса «POI» в «Спутник»
-динамические правки halo текста (P1-2) тоже не нужны — но это будет
-автоматически, потому что меняется `visibility` всей группы; явно
-зафиксировать в TRZ.
+Спецификация и реализация на уровне поведения согласованы. Z-order
+проверен JS-тестом «Z-order: satellite-base вставляется beforeId=первый
+terrain/trails/poi слой» и Python-тестом
+`test_halo_layers_below_real_trails`.
 
 ---
 
 ## P2 — should-fix
 
-### P2-1 — TRZ §5.2 алгоритм противоречит решению ADR-004 «Вариант O»
+### P2-1 — `00-business-request.md` остался с `TBD` и неверным заголовком
 
-**Где:** TRZ §5.2 шаг 2.2:
+**Где:** `docs/work-items/ET-007/00-business-request.md`:
+```
+# Business Request: ET-005: Спутниковая карта (Схема/Спутник)
+Work Item ID: ET-007
+## Description
+TBD
+```
 
-> 2.2. Если layer 'satellite-base' отсутствует — addLayer (см. 4.2),
-> вставлять **beforeId** = id первого слоя trails-* или terrain-*
-> (первый из существующих) — чтобы спутник оказался под terrain и trails.
+v1 пункты P2-4 и P2-5 не закрыты. Заголовок всё ещё «ET-005»
+(пересекается с фактической ET-005 «единицы измерения»), Description
+— пустой. BRD/TRZ/AC уже содержат целевую формулировку, поэтому
+блокировать поставку этим нельзя, но формальное основание для
+возврата остаётся.
 
-В ADR-004 §«Вариант O» именно вариант **O-B** («через явный `beforeId`
-первого trails-слоя») **отклонён** как ненадёжный («terrain/trails
-добавляются асинхронно; использовать `beforeId` слоёв, которых ещё
-нет, нельзя»). Выбран **O-A** — «вызвать `restoreBaseLayerState()`
-первым в `rebuildMapOverlays()`», без `beforeId`.
+**Действие:** заменить заголовок на «Business Request: ET-007:
+Спутниковая карта (Схема / Спутник)»; в Description вставить 2–3
+предложения из BRD §1.
 
-Алгоритм в TRZ остаётся в варианте O-B. Разработчик возьмёт TRZ.
+### P2-2 — Tech-Risks R-2 митигация частично противоречит BRD F-02
 
-**Действие:** в TRZ §5.2 шаг 2.2 убрать `beforeId` или поменять
-формулировку на «addLayer без beforeId; порядок гарантируется тем,
-что restoreBaseLayerState вызывается первым в rebuildMapOverlays
-(ADR-004 §«Вариант O», O-A)».
+**Где:** `10-tech-risks.md` R-2 «Альтернативные провайдеры …
+быстрый switch на следующего по приоритету — Mapbox или MapTiler —
+потребует только введения переменной окружения для API-ключа (это
+уже инфра-изменение, выходящее за scope ET-007)».
 
-### P2-2 — Tech-Risks R-2 митигация противоречит BRD F-02 / ADR §«Вариант P»
+Caveat «выходящее за scope ET-007» добавлен в v2 — это улучшение.
+Но формулировка «потребует только введения переменной окружения для
+API-ключа» по-прежнему противоречит BRD F-02 («без API-ключа»),
+ADR-004 §«Вариант P» (Mapbox/MapTiler отклонены именно по этому
+критерию) и описанию out-of-scope в BRD §3.
 
-**Где:** Tech-Risks R-2: «быстрый switch на следующего по приоритету —
-Mapbox или MapTiler — потребует только введения переменной окружения
-для API-ключа».
+**Действие:** в R-2 переписать митигацию честно: «при деградации Esri
+без-API-ключевых публичных альтернатив с глобальным покрытием в
+момент написания нет; реакция требует либо пересмотра BRD F-02
+(возврат в Анализ), либо архитектурного решения о self-hosted
+satellite tiles (новый ADR)».
 
-Но BRD F-02 явно требует «без API-ключа», ADR-004 §«Вариант P»
-отклоняет Mapbox/MapTiler именно по этому критерию. Значит, такой
-switch — **не** «один объект source-spec», а пересмотр функциональных
-требований BRD (и инфра-требований: введение переменной окружения,
-секретов, ротации).
+### P2-3 — TRZ §3.2 оставлено двусмысленное указание про позицию блока
 
-**Действие:** в R-2 заменить митигацию на честную: «при деградации
-Esri единственный безапи-ключевой кандидат — отсутствует; реакция
-требует пересмотра BRD F-02 (вход в Анализ) либо архитектурного
-решения о self-hosted-тайлах».
+**Где:** TRZ §3.2: «в начале `#terrain-popup`, **сразу после**
+`<div class="terrain-popup-title">Эндуро</div>` ИЛИ выше него — по
+выбору разработчика».
 
-### P2-3 — Baseline `background-color` тёмной темы в Data §5 неверный
+Реализация выбрала «выше заголовка» (совпадает с диаграммой §3.1 и
+закреплено тестом `test_base_toggle_placed_at_top_of_terrain_popup`).
+ТЗ нужно привести в соответствие, иначе следующая итерация фичи
+может молча вернуть блок ниже заголовка.
 
-**Где:** Data §5: «`#1a1a1a` (dark) — задублированы из `style*.json`».
-**Что в коде:** `style-dark.json:28` — `"#1a1a2e"`.
+**Действие:** в §3.2 убрать ветку «сразу после» и оставить только
+«в самом верху попапа, выше заголовка «Эндуро»».
 
-(Дубль с P1-4 в части цифры, но это узкая правка — поправить значение
-в Data §5.)
+### P2-4 — Отсутствует автотест для CSS-hook `body.satellite-active #btn-basemap` и для контракта halo-синхронизации с чекбоксами
 
-### P2-4 — `00-business-request.md` содержит только `TBD`
+**Где:** `tests/unit/test_base_layer.py` и `base_layer.test.js`.
 
-**Где:** `00-business-request.md` — секция Description пустая (TBD).
-По формальным правилам репозитория BR должен содержать первичное
-формулирование запроса; всё остальное (BRD, TRZ, AC) уже опирается
-на это как на «источник правды». Анализ-этап де-факто содержит
-правду в BRD §1, но BR оставлен незаполненным.
+Покрытие P1-5 и P1-6 на уровне поведения присутствует в JS-suite
+**только** для частей, лежащих внутри блока ET-007 (применение
+satellite-active класса делегировано на `_setBodyClass`, который в
+мок-DOM деградирует в no-op — см. `_setBodyClass` lines 2989–2997).
+В итоге не тестируется:
 
-**Действие:** заполнить BR двумя-тремя предложениями (можно
-конспектом из BRD §1) — это снимет одно из формальных оснований
-для возврата.
+1. **CSS-rule** `body.satellite-active #btn-basemap { display:none }`
+   — Python-тест `test_base_toggle_styles_defined` проверяет только
+   `.terrain-base-row` / `.terrain-base-label` / `.base-seg`. AC-02
+   сценарий «Кнопка «Базовая карта» скрывается на спутнике (P1-5)»
+   формально не покрыт автоматическим тестом.
+2. **Вызов `_applyTrailHaloVisibility(...)` из `onTrailsCheckbox`** —
+   функция `onTrailsCheckbox` живёт ВНЕ блока ET-007 и в JS-suite не
+   подгружается через `new Function`. Python-сторона лишь проверяет
+   присутствие id `trails-*-halo-satellite` в `app.js`, но не сам
+   вызов хука. AC-04 сценарии «Выключение «Грунтовки» скрывает и halo»
+   формально не покрыты регресс-тестом.
+3. **`_savedBasemapState` save/restore цикл** — поведение
+   реализовано, но в JS-suite нет теста, который бы выставил
+   `layerState.basemap = false` до перехода в спутник и проверил,
+   что после возврата `osm-base.visibility === 'none'`. AC-02/03
+   сценарий «Запоминание выбора Базовая карта» формально не покрыт.
 
-### P2-5 — Несогласованный заголовок: «ET-005» при `work_item_id: ET-007`
+Гэп ровно по тем границам, по которым были претензии v1 (P1-5/P1-6).
+Код корректен (проверено вручную при ревью), но без регрессионных
+тестов будущий рефакторинг `onTrailsCheckbox` или `applyBaseLayer`
+может молча сломать контракт.
 
-**Где:** `00-business-request.md` — `# Business Request: ET-005: Спутниковая карта (Схема/Спутник)` / `Work Item ID: ET-007`. Аналогично `.task.md` title.
+**Действие (минимум):**
+- Python-тест: assert `'body.satellite-active'` и `'#btn-basemap'` в
+  `app.css`.
+- Python-тест: внутри `function onTrailsCheckbox(` тело содержит
+  `_applyTrailHaloVisibility`; то же для `restoreTrailsState`.
+- JS-тест: один кейс на цикл «schematic (basemap=false) → satellite
+  → schematic», проверяющий, что `osm-base` остаётся `none` после
+  возврата. Сейчас в moc-окружении нет `layerState` (он вне блока
+  ET-007) — потребуется либо экспортировать `_savedBasemapState`,
+  либо добавить тонкую заглушку `layerState` в `loadBaseLayerModule`.
 
-ET-005 — это уже сделанная задача про единицы измерения (km/mi,
-ADR-0001). Двойное использование «ET-005» в заголовке этой задачи
-сбивает с толку.
-
-**Действие:** заменить заголовок на «ET-007: Спутниковая карта
-(Схема / Спутник)» — как и в BRD/TRZ/AC.
-
-### P2-6 — TRZ §3.2 даёт две взаимоисключающие инструкции про позицию блока
-
-**Где:**
-- §3.1 диаграмма: «Подложка» в **самом верху** попапа (над «Эндуро»).
-- §3.2 текст: «в начале `#terrain-popup`, **сразу после**
-  `<div class="terrain-popup-title">Эндуро</div>` ИЛИ **выше него** —
-  по выбору разработчика».
-
-Диаграмма и текст не совпадают. AC-01 не уточняет позицию.
-
-**Действие:** в §3.2 зафиксировать одну позицию (рекомендуется «выше
-заголовка Эндуро», что соответствует §3.1).
+Эту работу можно сделать одним PR. Не блокирует merge ET-007, но
+блокирует «зрелость» теста.
 
 ---
 
 ## P3 — nice-to-have
 
-### P3-1 — TRZ §5.5: избыточная защита `typeof restoreBaseLayerState === 'function'`
+### P3-1 — Расхождение TRZ §5.2 vs код в части `beforeId`
 
-**Где:** TRZ §5.5 показывает пример встраивания:
+TRZ v2 §5.2 шаг 2.2: «addLayer (см. 4.2) **без beforeId**. Корректный
+z-order гарантируется тем, что restoreBaseLayerState вызывается
+ПЕРВЫМ в rebuildMapOverlays».
+
+Код `app.js` `applyBaseLayer()`:
+```js
+const before = _firstOverlayLayerId(map);
+map.addLayer({ id: SATELLITE_LAYER_ID, ... }, before);
+```
+
+Код **дополнительно** вычисляет `beforeId` через `_firstOverlayLayerId`
+(ищет первый слой с префиксом `terrain-` / `trails-` / `poi-`). Это
+защита от случая, когда `restoreBaseLayerState` вызван не первым (на
+старте приложения, например). Поведение **более устойчивое**, чем
+требует ТЗ, и тесты I-02 и Z-order это подтверждают. Но формально
+spec ↔ code расходятся.
+
+**Действие:** либо обновить TRZ §5.2 шаг 2.2 (добавить «с
+опциональным `beforeId` от `_firstOverlayLayerId(map)` как
+страховкой — не обязателен, поскольку порядок гарантирован O-A»),
+либо снять защиту из кода и положиться чисто на O-A. Первый вариант
+проще и оставляет код более устойчивым.
+
+### P3-2 — Мёртвая константа `SATELLITE_HALO_LAYER_IDS`
+
+`app.js:2914–2917`:
+```js
+const SATELLITE_HALO_LAYER_IDS = [
+  'trails-track-halo-satellite',
+  'trails-path-bridleway-halo-satellite',
+];
+```
+
+Константа объявлена и экспортируется из фабрики юнит-тестов, но в
+рантайме не используется ни единого раза — `_applyTrailHaloVisibility`
+работает по локальному `pairs`. Либо использовать константу
+(`pairs.forEach((p) => …)` → читать halo-id из неё), либо удалить.
+
+### P3-3 — Мёртвая функция `_toggleSatelliteHalo`
+
+`app.js:3107–3110`:
+```js
+function _toggleSatelliteHalo(map, enabled) {
+  _applyTrailHaloVisibility(map, enabled ? 'satellite' : 'schematic');
+}
+```
+
+Комментарий заявляет «обратная совместимость для существующих
+unit-тестов», но в `tests/unit/base_layer.test.js` функция нигде не
+вызывается и из фабрики не экспортируется. Если она использовалась в
+промежуточной версии тестов — её можно безопасно удалить.
+
+### P3-4 — Избыточная защита `typeof restoreBaseLayerState === 'function'` в `rebuildMapOverlays()`
+
+`app.js:127–131`:
 ```js
 if (typeof restoreBaseLayerState === 'function') {
   restoreBaseLayerState();
 }
 ```
-Эта защита оправдана для `rebuildGpxOverlays` (ET-006), потому что
-GPX-фича живёт в отдельном файле `gpx.js`, который теоретически может
-не подгрузиться. ADR-004 §2 явно решает, что фича остаётся в `app.js`
-— значит функция всегда определена в том же файле, и `typeof === 'function'`
+
+ADR-004 §2 явно решает, что фича остаётся в `app.js` — функция всегда
+определена в том же файле. Защита оправдана для `rebuildGpxOverlays`
+(ET-006), где функция живёт в отдельно подгружаемом `gpx.js`. Здесь
 ничего не защищает.
 
-Не блокирует, но запутывает читателя кода. Уточнить намерение или
-просто вызвать функцию напрямую.
-
-### P3-2 — AC-08 пороги без метода замера
-
-**Где:** AC-08 — «первая спутниковая плитка отображается в течение
-≤ 500 мс», «смена визуально мгновенная (≤ 100 мс)». Тест-план не
-указывает, как именно мерить (Performance API, throttling сети,
-`map.once('idle')`). 500 мс ≈ длина сетевого RTT — пограничный
-порог, который без чёткого метода замера сорвётся то в одну, то в
-другую сторону.
-
-**Действие:** в TestPlan TP-Performance (если он есть в e2e suite,
-сейчас в `04-test-plan.yaml` есть только e2e-base-layer-workflow без
-явного НФТ-кейса) добавить процедуру замера и условия сети.
-
-### P3-3 — `rebuildMapOverlays()` уже вызывает `restoreTrailsState`/`restorePoiState` после `restoreTerrainState` — порядок halo-инициализации в «Спутник» нетривиален
-
-Маленькая ремарка к ADR-004 §«Вариант O»: правило «satellite-base
-первым» гарантирует только z-order **базы** относительно terrain/trails.
-Но halo-underlay-слои тоже должны лечь **под** соответствующие base-line-слои.
-Если halo-слои добавлены прямо в `style.json` (как предполагает ADR §5),
-их порядок относительно `trails-track` определяется порядком в массиве
-`layers` стиля, а не порядком восстановления. Стоит явно отметить в
-ADR §5 / Data §6, что в `style.json` halo-слой ставится **непосредственно
-перед** соответствующим базовым (выше `osm-base`, ниже `trails-track`),
-иначе halo окажется поверх базовой линии и испортит вид и на схеме, и
-на спутнике (включён только тогда halo не виден из-за `visibility:none`,
-но как только включится — проблема).
+Не блокирует, но запутывает читателя кода. То же относится к двум
+вхождениям в init-IIFE (там защита тоже стоит).
 
 ---
 
 ## Сводка
 
-| ID    | Severity | Категория                       | Действие        |
-|-------|----------|---------------------------------|-----------------|
-| P1-1  | P1       | Несуществующие слои grade1..5   | TRZ/ADR/Data/Risks — переписать под реальные id |
-| P1-2  | P1       | POI labels: тёмный текст + чёрный halo | TRZ/ADR — добавить смену text-color |
-| P1-3  | P1       | BRD vs TRZ — hillshade авто-выкл| Обновить BRD §5 |
-| P1-4  | P1       | background-color: 3 источника   | Свести к одному; исправить опечатку и baseline |
-| P1-5  | P1       | Контракт с layerState.basemap не задан | TRZ §5.2 + AC |
-| P1-6  | P1       | halo не синхронизирован с чекбоксами Грунтовки/Тропы | TRZ §5 |
-| P2-1  | P2       | TRZ §5.2 beforeId vs ADR O-A    | Поправить TRZ §5.2 |
-| P2-2  | P2       | Tech-Risks R-2 митигация противоречит BRD F-02 | Переформулировать R-2 |
-| P2-3  | P2       | Baseline dark `#1a1a1a` ≠ `#1a1a2e` в стиле | Data §5 |
-| P2-4  | P2       | BR.md — `TBD`                   | Заполнить |
-| P2-5  | P2       | Заголовок «ET-005» при id ET-007| Переименовать |
-| P2-6  | P2       | TRZ §3.2 двусмысленность позиции| Зафиксировать одну |
-| P3-1  | P3       | Избыточный `typeof === 'function'`| Уточнить или убрать |
-| P3-2  | P3       | AC-08 без метода замера         | Добавить процедуру в TestPlan |
-| P3-3  | P3       | Порядок halo в style.json не оговорён | Уточнить в ADR §5 |
+| ID    | Severity | Категория                                    |
+|-------|----------|----------------------------------------------|
+| P2-1  | P2       | BR.md: TBD и заголовок «ET-005»              |
+| P2-2  | P2       | Tech-Risks R-2 митигация частично vs BRD F-02|
+| P2-3  | P2       | TRZ §3.2 — двусмысленное указание про позицию|
+| P2-4  | P2       | Нет автотестов для CSS-hook и hook'ов halo (граница AC-02/04 P1-5/P1-6) |
+| P3-1  | P3       | TRZ §5.2 «без beforeId» vs код с `_firstOverlayLayerId` |
+| P3-2  | P3       | Мёртвая константа `SATELLITE_HALO_LAYER_IDS` |
+| P3-3  | P3       | Мёртвая функция `_toggleSatelliteHalo`       |
+| P3-4  | P3       | Избыточный `typeof === 'function'`           |
+
+---
 
 ## Что хорошо
 
-- ADR-004 — добротный: явное обоснование выбора по 5 осям (P/M/S/O/H)
-  с таблицей альтернатив, явная классификация «minor change»,
-  раздел «Технический долг» с конкретным сценарием миграции в
-  `basemap.js`.
-- Архитектурный реестр `docs/architecture/adr/README.md` и
-  `docs/architecture/README.md` обновлены синхронно с ET-007 (раздел
-  «Внешние тайл-провайдеры»).
-- Tech-Risks (R-1..R-10) — полное покрытие, со ссылками на конкретные
-  митигации (UI-тесты, идемпотентные паттерны, точки расширения).
-  R-2 — единственный «высокий», но честно зафиксирован.
-- Data §7 (PII) и Infra §3 (внешние сетевые вызовы) чётко описывают
-  расширение круга третьих сторон и сохраняют «приватный по умолчанию»
-  через ленивое создание source.
-- TestPlan по структуре богатый (unit + integration + e2e + error
-  handling). AC покрывает 10 сценариев, включая mobile (AC-09) и
-  регресс существующего функционала (AC-10).
+- **Покрытие P1 v1 — 6/6 в коде и в спецификации одновременно.** Это
+  редкий случай: чаще в одном из двух фронтов остаются концы.
+- **JS-suite 33 теста** через `new Function`-загрузку блока — изящный
+  способ исполнить реальный production-код в Node без бандлера и без
+  переписывания app.js в ES-модуль. Покрытие включает edge-cases
+  (private mode → localStorage недоступен, `window._map` отсутствует,
+  невалидное stored-значение, повторный toggle, dark/light темы,
+  z-order при пустом наборе overlay'ев).
+- **Маркеры блока `// >>> ET-007 base layer toggle block` /
+  `// <<<`** — позволяют как читать блок целиком (поиск 30+ функций
+  в 3132-строчном `app.js` — боль), так и подгружать его в тестах. То
+  же решение применил ET-002 для POI; единый паттерн.
+- **Декларативные halo-underlay-слои в `style.json` / `style-dark.json`,
+  расположенные перед соответствующими `trails-*`** — z-order
+  явно зафиксирован в стиле и закреплён регресс-тестом
+  `test_halo_layers_below_real_trails` (оба файла стиля). Любое
+  будущее «перенесём слой» сломает тест немедленно.
+- **`_savedBasemapState` + CSS-class hook** — корректное и минимально
+  инвазивное решение P1-5: модуль ET-007 не правит `layerState`
+  существующего модуля, не редактирует `toggleLayer()`, не лезет в
+  его обработчики; контракт реализован через одну приватную
+  переменную и одну CSS-зависимость. Это самый низкий blast radius,
+  который можно было выбрать.
+- **Idempotent `addSource/addLayer`** через `if (!map.getSource(…))` /
+  `if (!map.getLayer(…))` (R-6) — соответствует паттерну
+  `terrain` / `trails` / `poi`.
+- **Документация и changelog** во всех артефактах v2 точно указывают,
+  какие P-findings из v1 ими закрыты (`changelog: "v2 ... P1-1..P1-6"`).
+  Это резко упрощает повторное ревью.
 
 ## Что делать дальше
 
-1. Исправить P1 (см. Действие в каждом пункте) — обновить TRZ, ADR,
-   BRD, Data, Tech-Risks. После правок поднять `version` затронутых
-   документов до v2.
-2. Желательно — закрыть P2-1…P2-6 в том же раунде.
-3. P3 — на усмотрение Owner/Architect.
-4. После правок — повторное ревью артефактов (этот же документ, v2).
-5. **Только после этого** — переход к stage `development`.
-
-Кода в ветке нет; ревью кода будет отдельным шагом после
-реализации.
+1. **APPROVED → merge в main.** Никаких P0/P1 нет, регресс не
+   обнаружен, тесты зелёные.
+2. **Параллельно или одним PR** закрыть P2-1, P2-2, P2-3, P2-4. P2-4
+   — самое полезное (закрывает регрессионный риск для P1-5/P1-6
+   логики). Остальные — гигиена документации.
+3. **P3 — на усмотрение** разработчика/Owner. Самый полезный из них —
+   P3-1: либо синхронизировать TRZ с кодом (рекомендуется), либо
+   снять защитный `beforeId` (тогда поведение строго совпадает с
+   ADR §O-A).
+4. После merge — переход в `testing` stage. Браузерные кейсы
+   (E-01..E-10, TC-UI-01..14) требуют Playwright-инфраструктуры,
+   которой в репозитории пока нет (см. ET-002 ADR-0001, 07-infra-
+   requirements.md). До её появления покрытие AC обеспечивается
+   связкой статических Python-тестов + JS-unit + ручной приёмки
+   на test-стенде.