review(ET-008): findings - 1xP0, 4xP1, 3xP2, 4xP3

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

@@ -2,188 +2,541 @@
 type: code-review
 work_item_id: ET-008
 title: "Review: GPS-треки с публичных платформ на карте"
-version: 1
+version: 2
 status: REQUEST_CHANGES
 created_at: 2026-06-01
+updated_at: 2026-06-01
 authors:
   - "agent:reviewer"
 reviewed_branch: feature/ET-008-gps
 base_branch: main
+reviewed_commits:
+  - 0060003 "feat(gps-tracks): ET-008 публичные GPS-треки с публичных платформ"
+  - 3734b98 "feat(ET-008): GPS tracks pipeline, API, frontend layer"
 verdict: REQUEST_CHANGES
 findings_summary:
   P0: 1
-  P1: 0
-  P2: 0
-  P3: 0
+  P1: 4
+  P2: 3
+  P3: 4
+changelog:
+  - "v1 (2026-06-01): REQUEST_CHANGES — на ветке отсутствовал код."
+  - "v2 (2026-06-01): код появился (~7700 LOC). Проведено code-review против ТЗ v2, AC v2 и ADR-005..011."
 ---
 
-# Code Review — ET-008
+# Code Review — ET-008 (v2)
 
 ## Verdict: **REQUEST_CHANGES**
 
-Причина: на ветке `feature/ET-008-gps` **нет ни одной строки кода**,
-реализующего ТЗ. PR содержит только artefacts стадий Analyst и Architect
-(BRD, ТЗ, AC, TestPlan, UI-кейсы, 7 ADR, infra/data/risks). Development
-stage не произвёл выхода — review.review невозможно провести как код-ревью,
-потому что нет кода. Отдельные оси (соответствие ТЗ / соответствие ADR /
-качество кода / качество тестов) — все «не выполнено», т.к. артефактов
-реализации не существует.
+На ветке появилась реализация (commits `0060003`, `3734b98`): backend
+пакет `src/api/gps_tracks/`, pipeline `scripts/gps_collect.py`, фронт
+`src/web/gps_tracks.js`, миграция, YAML-конфиги, docker-compose сервис
+`gps-collector`, тесты, fixtures. Архитектурно сборка следует ADR-005…008
+и REQ-F-01…F-03. Однако обнаружено **одно блокирующее (P0) расхождение**
+с ТЗ, ломающее основной сценарий просмотра на детальном zoom, и
+несколько P1-несоответствий контракту API.
 
 ## Что проверено
 
-Согласно регламенту reviewer'а:
-
-1. ✅ `docs/work-items/ET-008/02-trz.md` (v2, status: draft) — прочитан.
-2. ✅ `docs/work-items/ET-008/03-acceptance-criteria.md` (v2, draft) — прочитан.
-3. ✅ `docs/work-items/ET-008/06-adr/` — все 7 ADR (ADR-005…011) прочитаны.
-4. ✅ `CLAUDE.md` — прочитан.
-5. ✅ Git diff `main...feature/ET-008-gps` — проанализирован.
-
-## Состояние ветки
-
-`git diff main...feature/ET-008-gps --name-status` (19 файлов):
-
-| Статус | Файл |
-|--------|------|
-| M | `.task.md` |
-| M | `docs/architecture/README.md` |
-| M | `docs/architecture/adr/README.md` |
-| M | `docs/work-items/ET-008/00-business-request.md` |
-| M | `docs/work-items/ET-008/01-brd.md` |
-| M | `docs/work-items/ET-008/02-trz.md` |
-| M | `docs/work-items/ET-008/03-acceptance-criteria.md` |
-| M | `docs/work-items/ET-008/04-test-plan.yaml` |
-| M | `docs/work-items/ET-008/04b-ui-test-cases.md` |
-| A | `docs/work-items/ET-008/06-adr/ADR-005-storage-schema.md` |
-| A | `docs/work-items/ET-008/06-adr/ADR-006-dedup-algorithm.md` |
-| A | `docs/work-items/ET-008/06-adr/ADR-007-pipeline-architecture.md` |
-| A | `docs/work-items/ET-008/06-adr/ADR-008-tile-vs-geojson-strategy.md` |
-| A | `docs/work-items/ET-008/06-adr/ADR-009-osm-licensing.md` |
-| A | `docs/work-items/ET-008/06-adr/ADR-010-enduro-russia-licensing.md` |
-| A | `docs/work-items/ET-008/06-adr/ADR-011-ttrails-licensing.md` |
-| A | `docs/work-items/ET-008/07-infra-requirements.md` |
-| A | `docs/work-items/ET-008/08-data-requirements.md` |
-| A | `docs/work-items/ET-008/10-tech-risks.md` |
-
-Команды для воспроизведения:
-```
-git -C enduro-trails diff main...feature/ET-008-gps -- src/ tests/ migrations/ scripts/ config/
-# (empty output — нет ни одного изменения вне docs/)
-```
-
-Итого: **0 строк** в `src/`, `tests/`, `migrations/`, `scripts/`, `config/`,
-`docker-compose.yml`. Все изменения — в `docs/` и `.task.md`.
+1. ✅ `docs/work-items/ET-008/02-trz.md` v2 (draft) — REQ-F-01…F-20, REQ-NF-01…NF-07.
+2. ✅ `docs/work-items/ET-008/03-acceptance-criteria.md` v2 (draft) — AC-01…AC-17.
+3. ✅ `docs/work-items/ET-008/06-adr/` — ADR-005…011 (5 accepted, 2 proposed/blocking).
+4. ✅ `CLAUDE.md` — конвенции, фазы, правила для агентов.
+5. ✅ Git diff `main...feature/ET-008-gps` — 53 файла, +7705/-1218.
+6. ✅ Прочитан исходник backend (config.py, db.py, dedup.py, endpoint.py,
+   mvt.py, models.py, sources/{base,osm,enduro_russia,ttrails}.py).
+7. ✅ Прочитан исходник frontend (gps_tracks.js целиком, изменения в
+   app.js, app.css, index.html).
+8. ✅ Прочитана миграция, scripts/gps_collect.py, оба YAML, requirements.txt,
+   docker-compose.yml.
+9. ✅ Прочитаны 4 тест-файла (dedup, endpoint, mvt, sources/osm) и fixtures.
 
 ## Findings
 
-### F-01 [P0]: Отсутствует реализация ТЗ — все 20 REQ-F и все REQ-NF не выполнены
+### F-01 [P0]: GeoJSON-слой полностью скрывается из-за рассогласования имён properties
 
-**Severity:** P0 (blocker — «не реализовано требование ТЗ»).
+**Severity:** P0 (blocker — нарушено REQ-F-13/F-14, AC-04, AC-08; основной
+визуальный сценарий слоя на z ≥ 12 не работает).
 
-**Ссылка на правило:** регламент Reviewer'а, секция Severity: «не реализовано
-требование ТЗ → P0».
+**Где:**
+- `src/api/gps_tracks/endpoint.py`, `_row_to_geojson_feature()` (стр. 51–84)
+- `src/web/gps_tracks.js`, `applyGpsFilter()` (стр. 246–267) и
+  `_buildColorExpression()` (стр. 71–88)
 
-**Что ожидалось** (по ТЗ §5 «Файловая структура изменений», ст. 742–799):
+**Что обнаружено:**
 
-Backend (Python):
-- `src/api/gps_tracks/__init__.py`
-- `src/api/gps_tracks/models.py` — Pydantic + `ACTIVITY_TYPES` (REQ-F-07)
-- `src/api/gps_tracks/db.py` — SQLite/Spatialite обвязка (REQ-F-09, ADR-005)
-- `src/api/gps_tracks/dedup.py` — `compute_dedup_key()` (REQ-F-08, ADR-006)
-- `src/api/gps_tracks/mvt.py` — MVT-генерация (REQ-F-11, ADR-008)
-- `src/api/gps_tracks/endpoint.py` — FastAPI routes для REQ-F-10/11/12
-- `src/api/gps_tracks/config.py` — загрузка YAML
-- `src/api/gps_tracks/sources/{base,osm,enduro_russia,ttrails}.py` —
-  REQ-F-04/05/06 (ADR-009/010/011)
-- Регистрация роутов в `src/api/main.py` — не сделана
-- Обновление `src/api/requirements.txt` (`defusedxml`, `lxml`) — не сделано
+GeoJSON endpoint отдаёт в `properties` поля `activity_type` и `sources`
+(массив):
 
-Pipeline:
-- `scripts/gps_collect.py` — CLI-entry для REQ-F-03 (ADR-007) — отсутствует
+```python
+"properties": {
+    ...
+    "activity_type": row["activity_type"],
+    ...
+    "sources": sources,           # list
+    "external_urls": ext_urls,
+    ...
+}
+```
 
-Frontend (Web):
-- `src/web/gps_tracks.js` — слой/фильтры/popup (REQ-F-13…F-18) — отсутствует
-- Правки `src/web/app.js` — `restorePublicTracksState()` (REQ-F-19),
-  расширение `rebuildMapOverlays()`, halo по паттерну ET-007 (REQ-F-17, §7.2) — не сделаны
-- Правки `src/web/index.html` — чекбокс «Публичные треки» и
-  `#sheet-gps-filters` (REQ-F-13, REQ-F-14, ТЗ §3) — не сделаны
-- Правки `src/web/app.css` — `.terrain-link-btn`, `.gps-filter-grid`,
-  `.gps-stats-row` (ТЗ §3.1) — не сделаны
-- Правки `src/web/style.json`, `src/web/style-dark.json` — halo для
-  спутника (REQ-F-17) — не сделаны
+MVT-эндпойнт (правильно по ТЗ §4.3) отдаёт `activity` (скаляр) и `source`
+(скаляр первой sources):
 
-Конфигурация:
-- `config/gps_sources.yaml` (REQ-F-01) — отсутствует
-- `config/gps_regions.yaml` (REQ-F-02) — отсутствует
-- Директория `config/` вообще не существует в репозитории
+```python
+props = {
+    ...
+    "activity": row["activity_type"] or "other",
+    "source": first_source,
+    "sources": sources_str,        # comma-string
+    ...
+}
+```
 
-Миграции:
-- `migrations/gps_tracks_001_init.sql` (REQ-F-09, ADR-005) — отсутствует
+Клиентский фильтр в `applyGpsFilter()` использует **только** имена из
+MVT-схемы:
 
-Тесты (ТЗ §5, ст. 782–787):
-- `tests/api/test_gps_tracks_endpoint.py` — отсутствует
-- `tests/api/test_gps_tracks_mvt.py` — отсутствует
-- `tests/api/test_gps_tracks_dedup.py` — отсутствует
-- `tests/api/test_gps_tracks_sources_osm.py` — отсутствует
-- `tests/web/gps_tracks.test.js` — отсутствует
+```js
+const filter = ['all',
+  ['in', ['get', 'activity'], ['literal', activities]],
+  ['in', ['get', 'source'], ['literal', sources]]
+];
+map.setFilter(window.gpsTracksLayer.layerGeoId, filter);
+```
 
-Инфраструктура (`07-infra-requirements.md`):
-- Сервис `gps-collector` в `docker-compose.yml` с `profiles: ["batch"]`
-  (`docs/architecture/README.md` ст. 12, 41–46) — не добавлен
-- Cron-конфигурация на mva154 Mon/Thu 03:00 UTC — не сделана
+Для feature из GeoJSON-source-а `get('activity')` и `get('source')`
+возвращают `null` → `['in', null, ['literal', […]]]` = `false` → **все
+features фильтруются из показа**. То же касается `line-color`:
+`_buildColorExpression('source')` matches по `['get', 'source']` →
+GeoJSON-features попадают в fallback `'#808080'`.
 
-**Покрытие AC** (`03-acceptance-criteria.md`): из всех Gherkin-сценариев
-AC-01…AC-NN ни один не может быть исполнен — нет ни runtime, ни pipeline,
-ни схемы БД.
+**Воспроизведение:**
+1. Включить чекбокс «Публичные треки».
+2. Увеличить zoom до 12+. (`_syncGpsLayersVisibility` делает
+   `gps-tracks-layer-geo` видимым и скрывает MVT-слой.)
+3. На карте — ни одного публичного трека, хотя `/api/gps-tracks?bbox=…`
+   отдаёт >0 features (`returned > 0`, `total_in_bbox > 0`). Toast
+   «Показаны N треков из M…» возможен, но карта пустая.
 
-**Действие:** вернуть в стадию Development. Реализовать в порядке,
-рекомендованном ADR-007 §pipeline-architecture (config + db/migration →
-dedup → один source (OSM, ADR-009 accepted) → endpoints → MVT → UI),
-затем добавить тесты по `04-test-plan.yaml` и запустить ревью повторно.
+**Ссылка на правило:** Reviewer severity «не реализовано требование ТЗ» → P0.
+Затронуты:
+- REQ-F-14 (фильтры мгновенно действуют через setFilter).
+- REQ-F-17 (стили `gps-tracks-layer` с paint `match`).
+- AC-04 Scenario «Поля feature.properties» — `feature.properties` обязан
+  содержать `length_km`, см. также F-02 ниже.
+- AC-08 «Фильтрация по активности»: «на карте отображаются только enduro
+  и moto треки» — невозможно, т.к. на z ≥ 12 ВСЕ треки отфильтрованы.
 
-## Замечания на полях (НЕ блокирующие, к доработке вне этого ревью)
+**Что починить:**
+Унифицировать contract. Один из двух вариантов:
+- (рекомендуется) В `_row_to_geojson_feature` добавить дублирующие
+  поля `activity` (= `activity_type`) и `source` (= `sources[0]`) —
+  не ломая существующих потребителей. Параллельно проверить попап:
+  `_renderTrackPopupHtml` уже читает `props.activity_type || props.activity`
+  — менять не нужно.
+- Либо переписать `applyGpsFilter`/`_buildColorExpression` так, чтобы они
+  ветвились по `['has', 'activity']` vs `['get', 'activity_type']` + для
+  source использовать `['in', 'osm', ['get', 'sources']]` через
+  index-of / contains (MapLibre expressions поддерживают `in` с массивом
+  правой части).
 
-Эти пункты НЕ являются P-findings против реализации (её нет), просто
-служат напоминанием для следующего ревью, когда код появится:
-
-- ADR-010 (EnduroRussia) и ADR-011 (ttrails) согласно
-  `docs/architecture/README.md` ст. 53–60 и ТЗ ст. 59 имеют статус
-  `proposed/blocked`. Согласно ADR-007 §6 «licensing guard» — pipeline
-  ОБЯЗАН пропускать источник без `status: accepted`. Это поведение должно
-  иметь покрытие тестом (`test_pipeline_skips_unaccepted_source`).
-- В `src/api/main.py` (`route-line`, `trails-*` слои) при подключении
-  нового слоя `gps-tracks-layer` важно сохранить z-order, который ТЗ
-  фиксирует в §7.1: `gps-tracks-layer < gpx-layer-*`. В коде должен быть
-  явный `beforeId` в `map.addLayer(...)`.
-- В `applyBaseLayer()` (ET-007) необходимо добавить шаг по ТЗ §7.2.
-  Reviewer проверит, что halo `gps-tracks-halo-satellite` переключается
-  по тому же паттерну, что `trails-track-halo-satellite`.
-- Bbox-валидация на endpoint (REQ-NF-01): диапазон координат + площадь.
-  Должен быть unit-тест на отказ при невалидном bbox.
-- `defusedxml` для серверного GPX-парсинга (REQ-NF-01): обязательное
-  требование, проверится grep'ом на `lxml.etree.parse` без defusedxml.
-
-## Subjective / Style
-
-Не применимо — кода нет.
-
-## Тесты
-
-Не применимо — тестов нет. После реализации reviewer проверит покрытие
-относительно `04-test-plan.yaml` и `04b-ui-test-cases.md`.
-
-## Заметки по соответствию ADR
-
-Не применимо для кода. Сами ADR-005…011 не ревьюились в этом проходе
-(это работа архитектора/Owner); по PR-changes — приняты как контекст.
-
-## Рекомендация для CI
-
-- `04-test-plan.yaml` ссылается на тесты, которых нет → CI должен упасть
-  на сборе (pytest collection error) или на отсутствии модулей.
-- `make lint` пройдёт (нет нового Python-кода), `make test` упадёт.
-- Не закрывать ET-008 в Plane как Done.
+Тест, который должен поймать это (отсутствует):
+`tests/web/gps_tracks.test.js` — feature `{activity_type:'enduro'}` после
+`applyGpsFilter` остаётся видимой. Добавить в test-plan.
 
 ---
 
-**Итог:** REQUEST_CHANGES. Возврат на стадию Development.
+### F-02 [P1]: GeoJSON `feature.properties` отдаёт `length_m`, ТЗ требует `length_km`
+
+**Severity:** P1 (must-fix — нарушение контракта API, описанного в
+REQ-F-10 и AC-04; UI-попап показывает «—»).
+
+**Где:** `src/api/gps_tracks/endpoint.py`, `_row_to_geojson_feature()`
+(стр. 51–84).
+
+**ТЗ REQ-F-10** (ст. 280–305):
+```json
+"properties": {
+    "name": "...",
+    "activity_type": "...",
+    "user": "...",
+    "created_at": "...",
+    "length_km": 47.3,
+    "sources": [...],
+    "external_urls": [...]
+}
+```
+
+**AC-04 Scenario «Поля feature.properties»:**
+> Then каждая feature содержит: name, activity_type, user, created_at,
+> length_km, sources (array), external_urls (array)
+
+**Имеется в коде:**
+```python
+"length_m": row["length_m"],
+# и нет ни одного "length_km"
+```
+
+**Последствие:** В `_renderTrackPopupHtml` (`gps_tracks.js` стр. 325):
+```js
+const lengthKm = typeof props.length_km === 'number' ? props.length_km.toFixed(1) : '—';
+```
+для GeoJSON-feature (z ≥ 12) `props.length_km` всегда `undefined` → в
+попапе постоянно «📏 — км». MVT-features (z 8…11) показывают правильно,
+т.к. MVT-builder уже считает `length_km` (`mvt.py` стр. 148).
+
+**Что починить:** добавить в properties `length_km` (как минимум).
+Поле `length_m` оставить, если используется. Аналогично уточнить
+`points_count` и `created_at` для popup (см. также F-04).
+
+---
+
+### F-03 [P1]: REQ-F-04 не реализован полностью — все OSM-треки сохраняются как `activity_type='other'`
+
+**Severity:** P1 (must-fix — функциональный пробел; пользователь не
+сможет осмысленно использовать фильтр по активности, который явно
+закреплён в AC-08).
+
+**Где:** `src/api/gps_tracks/sources/osm.py`, `_parse_gpx_trackpoints()`
+(стр. 154–284).
+
+**ТЗ REQ-F-04** (ст. 119–144):
+> Для треков с gpx_id — дополнительный запрос
+> `GET /api/0.6/gpx/<id>` для метаданных (name, description, tags, user,
+> timestamp). Этот запрос делаем отложенно в batch: накопить 100 id →
+> запросить. Тип активности — из tags GPX-трека (`Tag: enduro/moto/mtb/hike/...`).
+
+**Реализовано:**
+- batch-запрос метаданных НЕ сделан;
+- `name`, `description`, `tags`, `user` всегда `None`/`[]`;
+- `activity_type` явно зашит как `"other"`:
+  ```python
+  track = TrackInsert(..., activity_type="other", ...)
+  ```
+- константа `OsmParser.MAPPING` (стр. 25–39) объявлена, но `map_activity`
+  не вызывается — мёртвый код.
+
+**Последствие:** в БД ВСЕ треки от OSM (единственного включённого
+источника) попадают как `activity_type='other'`. Фильтр по активности
+теряет смысл — пользователь видит только «Другое». AC-08 Scenario
+«Фильтрация по активности» не может пройти на реальных данных.
+
+**Что починить:** реализовать batch-fetch на `/api/0.6/gpx/<id>` по
+накоплению ID, как описано в ТЗ. Использовать `self.map_activity(...)`
+для tags. Если решено отложить — оформить ADR/коммент с явной отметкой
+«частичная реализация REQ-F-04, follow-up tracked в …» и понизить
+ожидания AC-08 в ТЗ (но это работа аналитика, не разработчика).
+
+---
+
+### F-04 [P1]: Health endpoint не соответствует REQ-F-12 / AC-06
+
+**Severity:** P1 (must-fix — нарушение контракта; AC-06 явно перечисляет
+обязательные поля, которых нет).
+
+**Где:** `src/api/gps_tracks/endpoint.py`, `gps_health()` (стр. 196–232).
+
+**ТЗ REQ-F-12 / AC-06** требуют поля:
+| Поле               | Тип    |
+| ------------------ | ------ |
+| `db_path`          | str    |
+| `db_size_mb`       | float  |
+| `tracks_total`     | int    |
+| `tracks_by_source` | dict   |
+| `tracks_by_activity` | dict |
+| `last_pipeline_run` | object с полями started/finished/regions/sources_ok/sources_error |
+| `tile_cache_size`  | int    |
+
+**Имеется:**
+```python
+return {
+    "status": "ok",
+    "db_path": db_path,
+    "total_tracks": total_tracks,                # должно быть tracks_total
+    "by_activity": by_activity,                  # должно быть tracks_by_activity
+    "recent_pipeline_runs": recent_runs,         # должно быть last_pipeline_run (объект)
+}
+```
+
+Отсутствуют: `db_size_mb`, `tracks_by_source`, `tile_cache_size`.
+Переименованы: `tracks_total → total_tracks`, `tracks_by_activity →
+by_activity`, `last_pipeline_run → recent_pipeline_runs` (массив, не
+объект).
+
+Также `recent_pipeline_runs` отдаёт «10 последних запусков», а ТЗ
+требует ОДИН последний (агрегированно). Это влияет на UI/админский
+view.
+
+**Что починить:** привести JSON-схему к контракту. Минимум — добавить
+`tracks_by_source` (вычислить по `sources_json` агрегацией в Python или
+JSON_EACH в SQL), `db_size_mb` (через `os.path.getsize`), `tile_cache_size`
+(через `len(_gps_tile_cache)` из `mvt.py`), `last_pipeline_run` объект
+(берём первую строку из `pipeline_runs ORDER BY started_at DESC`,
+агрегируем `sources_ok`/`sources_error` по последнему region).
+
+Tests `test_i40_health_endpoint` сейчас закреплены на текущей неправильной
+схеме — их тоже придётся обновить.
+
+---
+
+### F-05 [P1]: Z-order ET-006 не зафиксирован — `gps-tracks-layer` может оказаться выше `gpx-layer-*`
+
+**Severity:** P1 (must-fix — нарушение REQ-F-17 paint requirements §7.1
+и AC-10 Scenario «личный трек визуально выше публичных»).
+
+**Где:** `src/web/gps_tracks.js`, `_findGpsInsertPosition()` (стр. 191–196).
+
+**ТЗ §7.1:**
+> На карте оба видны параллельно; z-order:
+> `gps-tracks-layer` < `gpx-layer-*` (личные треки выше).
+
+**AC-10 Scenario «Совместимость с ET-006»:**
+> Then оба видны параллельно
+> And личный трек визуально выше публичных
+
+**Имеется:**
+```js
+function _findGpsInsertPosition(map) {
+  const style = map.getStyle && map.getStyle();
+  if (!style || !style.layers) return undefined;
+  const routeLayer = style.layers.find(l => l.id === 'route-line' || l.id.startsWith('route-'));
+  return routeLayer ? routeLayer.id : undefined;
+}
+```
+
+Функция ищет только `route-*`. Если `gpx-layer-*` уже добавлен в стиль
+(ET-006), но route-line ещё нет, gps-tracks-layer добавится **в конец**
+(`before = undefined` → addLayer без beforeId → поверх всего стиля),
+**в том числе поверх gpx-layer**. Это нарушает обязательное правило
+из §7.1 / AC-10.
+
+**Что починить:**
+```js
+function _findGpsInsertPosition(map) {
+  const style = map.getStyle && map.getStyle();
+  if (!style || !style.layers) return undefined;
+  // Приоритет beforeId: gpx-layer-* (ET-006), затем route-* (если нет gpx).
+  const gpxLayer = style.layers.find(l => l.id.startsWith('gpx-layer'));
+  if (gpxLayer) return gpxLayer.id;
+  const routeLayer = style.layers.find(l => l.id === 'route-line' || l.id.startsWith('route-'));
+  return routeLayer ? routeLayer.id : undefined;
+}
+```
+
+Соответствующий unit-тест добавить в `tests/web/gps_tracks.test.js`
+(в test-plan он есть как WEB-INTEG).
+
+---
+
+### F-06 [P2]: Валидация bbox по площади отсутствует (REQ-NF-01)
+
+**Severity:** P2 (should-fix).
+
+**Где:** `src/api/gps_tracks/endpoint.py`, `_parse_bbox()` (стр. 17–48).
+
+**ТЗ REQ-NF-01:**
+> Bbox-параметр валидируется (диапазон координат, площадь).
+
+**Имеется:** валидация диапазона `[-180,180]/[-90,90]`, проверка
+`west<east`/`south<north`. Площадь не проверяется.
+
+**Что починить:** добавить max площадь bbox (например, 10° × 10° = 100°²)
+для предотвращения «штормового» сканирования БД.
+
+---
+
+### F-07 [P2]: Дефолт `gps-tracks-sources` в localStorage включает отключённые источники
+
+**Severity:** P2.
+
+**Где:** `src/web/gps_tracks.js`, `window.gpsTracksLayer.filters.sources`
+(стр. 55).
+
+```js
+sources: ['osm', 'enduro_russia', 'ttrails'],
+```
+
+При том, что `config/gps_sources.yaml` явно держит `enduro_russia` и
+`ttrails` в `enabled: false` (соответствует ADR-010/011 status=proposed).
+ТЗ REQ-F-15 говорит дефолт = «все enabled». Не блокирует, но создаёт
+галки в UI для источников, по которым в БД ничего нет.
+
+**Что починить:** инициализировать sources списком из
+`/api/gps-tracks/health.tracks_by_source.keys()` при отсутствии
+localStorage-ключа. Это естественно подтянется после F-04.
+
+---
+
+### F-08 [P2]: «LRU-кэш» в `mvt.py` — на самом деле FIFO
+
+**Severity:** P2.
+
+**Где:** `src/api/gps_tracks/mvt.py`, `_gps_tile_cache` (стр. 12–24).
+
+```python
+def set_gps_cached_tile(z, x, y, data):
+    if len(_gps_tile_cache) >= _GPS_TILE_CACHE_MAX:
+        _gps_tile_cache.pop(next(iter(_gps_tile_cache)))   # FIFO, не LRU
+    _gps_tile_cache[(z, x, y)] = data
+```
+
+ТЗ REQ-NF-04: «LRU-кэш в памяти процесса FastAPI: 1024 записи». Текущая
+реализация — FIFO. Для тайлов это особо ухудшает: часто запрашиваемый
+тайл, попавший в кэш первым, будет вытеснен раньше, чем редкий тайл,
+попавший позже.
+
+**Что починить:** `from functools import lru_cache` нельзя из-за
+mutable invalidation; использовать `collections.OrderedDict` с
+`move_to_end` при чтении, либо `cachetools.LRUCache`. Совместимая идея —
+сделать `get_gps_cached_tile`:
+```python
+def get_gps_cached_tile(z, x, y):
+    key = (z, x, y)
+    if key in _gps_tile_cache:
+        _gps_tile_cache.move_to_end(key)   # OrderedDict
+        return _gps_tile_cache[key]
+    return None
+```
+
+---
+
+### F-09 [P3]: Мёртвое поле конфигурации `save_user_field`
+
+**Severity:** P3 (nice-to-have).
+
+**Где:** `config/gps_sources.yaml` — поле задано (`save_user_field:
+true|false`); ни один модуль его не читает. AC-16 Scenario «Pipeline не
+сохраняет запрещённые поля» рассчитывает, что это поле уважается.
+
+**Что починить:** в `OsmParser`/upsert обрабатывать `save_user_field=false`
+→ `user=None`. Сейчас OSM всё равно ставит `user=None` (см. F-03), но
+поле должно работать как контракт для будущих источников.
+
+---
+
+### F-10 [P3]: Лишний импорт `pytest_asyncio` в `tests/api/test_gps_tracks_endpoint.py`
+
+**Severity:** P3.
+
+`import pytest_asyncio` есть, но `@pytest_asyncio.fixture` нигде не
+используется (только `@pytest.mark.asyncio`). Не блокирует, но в чистом
+коде убирается.
+
+---
+
+### F-11 [P3]: `MockRow(dict)` в `tests/api/test_gps_tracks_mvt.py`
+
+**Severity:** P3.
+
+Тесты используют `class MockRow(dict)` как замену `sqlite3.Row`. Работает
+для текущего кода, но `sqlite3.Row` не поддерживает `__contains__` так
+же, как dict. Если в `mvt.py` появится `if "x" in row:`, тесты
+разойдутся с продом. Безопаснее использовать `sqlite3.Row` напрямую через
+`open_db + INSERT + fetchone()`.
+
+---
+
+### F-12 [P3]: Лишняя проверка `"source_priority" in existing.keys()` в `db.py`
+
+**Severity:** P3.
+
+`src/api/gps_tracks/db.py` стр. 116:
+```python
+existing_priority = existing["source_priority"] if "source_priority" in existing.keys() else 999
+```
+Колонка `source_priority` объявлена в миграции (`migrations/gps_tracks_001_init.sql`
+ст. 22) с `NOT NULL DEFAULT 999`. Проверка избыточна (read-protection
+осталась от какой-то ранней итерации). Лучше убрать — иначе создаётся
+впечатление, что колонка опциональна.
+
+---
+
+## Соответствие ADR
+
+| ADR | Status | Соблюдение в коде | Замечания |
+|-----|--------|-------------------|-----------|
+| ADR-005 storage-schema | accepted | ✅ соблюдено | таблица `tracks` (+`source_priority`), индексы и `pipeline_runs` совпадают |
+| ADR-006 dedup-algorithm | accepted | ✅ соблюдено | `compute_dedup_key` 1-в-1; покрыт unit-тестами U-10…U-14 |
+| ADR-007 pipeline-architecture | accepted | ✅ соблюдено | сервис `gps-collector` с `profiles:["batch"]`, license-guard в `_check_license_adr` |
+| ADR-008 tile-vs-geojson | accepted | ⚠️ частично | переключение по zoom есть; но contract фич GeoJSON vs MVT расходится (см. F-01) |
+| ADR-009 osm-licensing | accepted | ✅ соблюдено | attribution в source, рабочий парсер |
+| ADR-010 enduro-russia (proposed) | proposed | ✅ соблюдено | parser — заглушка, `enabled: false`, license-guard сработает |
+| ADR-011 ttrails (proposed) | proposed | ✅ соблюдено | то же |
+
+License-guard в `scripts/gps_collect.py` `_check_license_adr()` корректно
+читает YAML front-matter ADR. Покрытия unit-тестом нет — рекомендую
+добавить (`test_pipeline_skips_unaccepted_source`).
+
+## Тесты — оценка
+
+| Тест-файл | Покрытие test-plan | Качество |
+|-----------|-------------------|----------|
+| `test_gps_tracks_dedup.py` | U-10…U-14 | ✅ хорошо |
+| `test_gps_tracks_mvt.py` | U-50…U-52 | ✅ адекватно (см. F-11) |
+| `test_gps_tracks_endpoint.py` | I-20…I-23, I-30…I-31, I-40 | ⚠️ AC-06 не покрыт корректно — тест зафиксирован на неверной схеме (F-04) |
+| `test_gps_tracks_sources_osm.py` | U-42…U-44 | ✅ defusedxml проверен фикстурой xxe-payload.gpx |
+
+**Не покрыто тестами:**
+- `tests/web/gps_tracks.test.js` — заявлен в ТЗ §5 ст. 787 и
+  `04-test-plan.yaml`, ОТСУТСТВУЕТ. Был бы первой защитой от F-01.
+- pipeline (`scripts/gps_collect.py`) — нет ни одного теста на
+  `_check_license_adr`, `_collect_source_for_region`, dry-run, exit-code.
+  AC-02 Scenarios «Падение одного источника не валит остальные» и
+  «Dry-run» по факту не верифицированы.
+- bbox area-валидация — отсутствует и в коде (F-06), и в тестах.
+
+## Замечания, не доходящие до P-finding
+
+- `_simplify_coords` в `mvt.py` дублирует `simplify_coords` из `src/api/main.py`.
+  Не критично, но напрашивается общая утилита.
+- `src/api/main.py` ст. 17–20: `GPS_TRACKS_DB_PATH` вычисляется **до**
+  импорта shapely/fastapi, в строгом смысле это «нечистый» импорт-time
+  side-effect. Не блок.
+- В `endpoint.py` `init_db` вызывается на каждый запрос (`_get_conn`).
+  Это означает, что `executescript` выполняется на каждый запрос. SQL
+  использует `IF NOT EXISTS`, так что функционально ок, но это лишний
+  I/O. Рекомендую инициализировать БД один раз при `create_gps_router`.
+- `src/api/main.py` ст. 1255: `app.include_router(gps_router)` хорошо
+  встроено перед `StaticFiles` mount — порядок правильный.
+
+## Воспроизведение P0 для разработчика
+
+```bash
+# 1. Запустить с тестовой БД (см. test_i20_geojson_basic):
+pytest tests/api/test_gps_tracks_endpoint.py::test_i20_geojson_basic -q
+
+# 2. Вытащить feature.properties — наблюдать "activity_type", "sources" (list),
+#    отсутствие "length_km".
+
+# 3. Открыть DevTools в браузере на dev-стенде, проверить:
+window._map.queryRenderedFeatures({layers:['gps-tracks-layer-geo']})
+# → пустой массив на z >= 12, потому что setFilter с ['get','activity'] всё скрыл.
+
+# 4. Временный обход для отладки:
+window._map.setFilter('gps-tracks-layer-geo', null)
+# → треки появятся, но серые (line-color fallback) — что подтверждает F-01.
+```
+
+## Рекомендация для CI
+
+- `pytest tests/api/` сейчас зелёный, потому что:
+  - `test_i40_health_endpoint` фиксирует текущую неправильную схему
+    (F-04) → нужно поправить вместе с фиксом;
+  - frontend-тесты отсутствуют (F-01 не отлавливается).
+- `make lint` ожидаемо проходит.
+- Перед закрытием задачи CI должен прогонять и frontend-тесты
+  (`tests/web/gps_tracks.test.js`), которые на данный момент не написаны.
+- Не закрывать ET-008 в Plane как Done.
+
+## Итог
+
+**REQUEST_CHANGES.** Минимальный объём правок:
+1. F-01 (P0) — унифицировать имена properties между MVT и GeoJSON, +
+   web-тест на `applyGpsFilter`.
+2. F-02 (P1) — добавить `length_km` в GeoJSON.
+3. F-03 (P1) — реализовать batch-fetch `/api/0.6/gpx/<id>` и mapping
+   `activity_type` (или явная декларация частичной реализации REQ-F-04).
+4. F-04 (P1) — выровнять `gps_health()` под REQ-F-12 / AC-06.
+5. F-05 (P1) — корректный `beforeId` в `_findGpsInsertPosition`.
+
+После исправлений P0/P1 — повторить ревью; P2/P3 могут пройти отдельным
+PR follow-up.