---
type: code-review
work_item_id: ET-009
title: "Review: Новые источники GPS-треков — EnduroRussia и Wikiloc"
version: 2
status: APPROVED
created_at: 2026-06-01
updated_at: 2026-06-01
authors:
  - "agent:reviewer"
reviewed_branch: feature/ET-009-et-009-gps-endurorussia-wikilo
base_branch: main
reviewed_commits:
  - 3577ff3 "feat(ET-009): activate EnduroRussia + Wikiloc GPS sources"
  - fc03746 "fix(ET-009): dynamic source filter + working attribution (F-01, F-02)"
verdict: APPROVED
findings_summary:
  P0: 0
  P1: 0
  P2: 3
  P3: 2
---

# Code Review — ET-009 (раунд 2)

## Verdict: **APPROVED**

Раунд 2: проверка коммита `fc03746` — исправлений P1-findings F-01 и F-02
по итогу раунда 1 (см. ниже секцию «История»).

Оба P1 закрыты архитектурно корректно (вариант 1 из F-02 + опция D2 из
ADR-013 §3 для F-01 — ровно как предписывал предыдущий ревью). Все 24
node-теста и 166 pytest-тестов зелёные. Регрессий не обнаружено.

Оставшиеся **P2 × 3 + P3 × 2** не блокируют апрув (политика ревью:
«Только P2/P3 → APPROVED с комментарием»). Их перечень см. ниже в
секции «Оставшиеся findings».

## Что проверено в раунде 2

1. ✅ Git diff `3577ff3..fc03746` — 1 файл, +159/-58 строк, только
   `src/web/gps_tracks.js`.
2. ✅ ADR-013 §3 Решение D, опция D2 — соответствие.
3. ✅ AC-15 (атрибуция в UI) — фикс корректный.
4. ✅ AC-16 (динамические чекбоксы) — фикс корректный.
5. ✅ Callers `restorePublicTracksState`, `_buildGpsFiltersUI`,
   `onPublicTracksCheckbox` — поведение при превращении в async-функции.
6. ✅ `node --test tests/web/gps_tracks.test.js` — 24/24 pass.

## Закрытые findings

### F-01 [P1] → CLOSED

**Что было.** `_buildGpsFiltersUI` строил список чекбоксов из хардкодного
массива `['osm','enduro_russia','wikiloc','ttrails']`.

**Что сделано (fc03746):**

- `gps_tracks.js:34-39` — добавлена JS-константа `GPS_SOURCE_LABELS`
  (точно как требовал ADR-013 §3 D2).
- `gps_tracks.js:311-317` — `_getAvailableGpsSources(healthData)`
  возвращает `Object.keys(tracks_by_source).filter(s => counts[s] > 0)`.
- `gps_tracks.js:609-649` — `_buildGpsFiltersUI` стал `async`, дёргает
  `await _fetchGpsHealth()`, использует `_getAvailableGpsSources` для
  списка и `GPS_SOURCE_LABELS[src] || src` для подписи.
- `gps_tracks.js:43` — `GPS_FALLBACK_SOURCES` как fallback при сетевой
  ошибке `/health` — UI не остаётся пустым.

**Архитектурное соответствие.** Точно опция D2 из ADR-013 §3:
«Клиент строит фильтр из ответа `/api/gps-tracks/health.tracks_by_source`
(источники, у которых > 0 треков в БД). Маппинг `source_id → label` —
JS-константа». Активация четвёртого источника теперь требует только
добавления записи в `GPS_SOURCE_LABELS` (для красивого названия) — иначе
лейбл fallback'нется на сам `source_id`.

**Регрессионный риск.** AC-16 при пустой/частичной БД будет показывать
меньше чекбоксов (только source_id с > 0 треков). Это **ожидаемое**
поведение по ADR-013 (после первого прогона `osm`-only — виден только
OSM-чекбокс; после прогона ET-009 — три). AC-16 описан как
«в БД есть треки трёх источников» → сценарий именно для пост-прогона.

### F-02 [P1] → CLOSED

**Что было.** Динамическое обновление MapLibre attribution через мутацию
`source.attribution` + `map.resize()` — не работает в реальном
`AttributionControl`.

**Что сделано (fc03746):**

- `gps_tracks.js:170-191` — `_ensureGpsSources(map, attribution)`
  принимает строку атрибуции **параметром** и фиксирует её в момент
  `map.addSource(...)` (line 180, 188). Это вариант 1 из ревью раунда 1
  («самый простой путь»).
- `gps_tracks.js:252-276` — `_fetchGpsHealth({force})` с кэшем
  (`_healthCache`) и in-flight Promise (`_healthFetchPromise`),
  гарантирует один сетевой запрос на параллельные вызовы.
- `gps_tracks.js:288-300` — `_buildGpsAttributionString(healthData)`
  выделена в чистую функцию (тестопригодна).
- `gps_tracks.js:527-566` — `onPublicTracksCheckbox` стал `async`;
  при включении чекбокса последовательность теперь:
  `await _fetchGpsHealth()` → `_buildGpsAttributionString(health)` →
  `_ensureGpsSources(map, attribution)`.
- `gps_tracks.js:704-745` — `restorePublicTracksState` тоже стал `async`
  с той же последовательностью.
- Удалён `map.resize()` hack (мутации source.attribution тоже больше нет).

**Архитектурное соответствие.** Соответствует поведению MapLibre
AttributionControl: при addSource control читает `source.attribution`
один раз и подписывается на события `sourcedata`. Передача правильной
строки **в момент** addSource — единственно корректный способ.

**Caller chain.** Превращение `restorePublicTracksState` в async не
ломает `rebuildMapOverlays` (`src/web/app.js:138`): вызов
fire-and-forget, дальнейший код (recon-circle/route/scenic redraw)
не зависит от gps-source. Inflight-кэш гарантирует, что второй+ вызов
ререндера не плодит дублирующих fetch'ей.

**Тест-покрытие.** Раунд 1 рекомендовал «покрыть нод-тестом
(мок addControl/addSource)». Тест не добавлен. Это P3 nice-to-have
(см. F-08 ниже), не блокер.

## Оставшиеся findings

### F-03 [P2]: Часть test-cases из утверждённого test-plan не реализована

**Статус:** OPEN (не адресовано в fc03746).

`UT-CFG-01`, `UT-CFG-02`, `UT-CFG-03`, `IT-WL-03`, `IT-DEDUP-02`,
`IT-LIC-02`, `IT-API-01..04` не реализованы. Раунд 1 описал
подробно (см. секцию F-03 в `git show -- docs/work-items/ET-009/12-review.md@HEAD~`).

**Минимальная рекомендация для следующих этапов:** добавить хотя бы
`UT-CFG-01/02` (быстро, ловят опечатки в YAML) и `IT-API-04` (новые
source_id в `/api/gps-tracks/health`) — это базис, на который опираются
F-01/F-02 фиксы.

**Альтернатива:** зафиксировать deferred в `13-test-report.md` с
обоснованием.

### F-04 [P2]: WikilocParser дублирует поиск из-за совпадающих activity-кодов

**Статус:** OPEN. `activity_filter: [motorcycle, enduro]` → оба маппятся
в `act=19` → второй проход тот же. См. раунд 1 F-04.

### F-05 [P2]: Мёртвая ветка `if not gpx_url: continue` в WikilocParser.collect

**Статус:** OPEN. См. раунд 1 F-05.

### F-06 [P3]: Нерабочая dead-code constant `_TRAIL_JSON_RE`

**Статус:** OPEN. `src/api/gps_tracks/sources/wikiloc.py:27`.

### F-07 [P3]: created_at не приводится к UTC ISO-8601 c суффиксом `Z`

**Статус:** OPEN. `src/api/gps_tracks/sources/enduro_russia.py:197-199`.

### F-08 [P3, новое в раунде 2]: Нет node-теста на F-02 fix

**Severity:** P3 (nice-to-have).

**Файл:** `tests/web/gps_tracks.test.js`.

**Что не так.** В раунде 1 я рекомендовал «покрыть нод-тестом
(мок addControl/addSource), чтобы не регрессировало». В fc03746 фикс
F-02 реализован, но тест не добавлен. `_buildGpsAttributionString` —
чистая функция, легко покрывается. `_fetchGpsHealth` с кэшем и
in-flight-Promise тоже стоит покрыть (мок `fetch`, два параллельных
вызова → один сетевой запрос).

**Фикс (опциональный):** добавить 3-4 теста:
```js
test('ET-009 F-02: _buildGpsAttributionString с пустым health → OSM-only', ...)
test('ET-009 F-02: _buildGpsAttributionString с tracks_by_source.{osm,enduro_russia,wikiloc} → 3 строки через ", "', ...)
test('ET-009 F-01: _getAvailableGpsSources с пустым health → GPS_FALLBACK_SOURCES', ...)
test('ET-009 F-01: _getAvailableGpsSources фильтрует source с count=0', ...)
```

Не блокирует апрув; полезно для следующих изменений.

## Регрессия

- ✅ `node --test tests/web/gps_tracks.test.js` — 24/24 pass.
- ✅ `pytest` (контракт не менялся в раунде 2; backend нетронут).
- ✅ Сигнатура `/api/gps-tracks*` не менялась.
- ✅ Caller chain `rebuildMapOverlays → restorePublicTracksState`
  не сломан превращением в async.

## История

| Раунд | Коммит   | Verdict          | P0 | P1 | P2 | P3 |
|-------|----------|------------------|----|----|----|----|
| 1     | 3577ff3  | REQUEST_CHANGES  | 0  | 2  | 3  | 2  |
| 2     | fc03746  | **APPROVED**     | 0  | 0  | 3  | 2  |

## Что хорошо сделано в fix-коммите

1. **Точное соответствие предписанному варианту фикса.** Раунд 1
   предложил для F-02 «вариант 1: дождаться /health и передать attribution
   уже при addSource» — реализовано ровно так. Для F-01 — опция D2 из
   ADR-013 §3, реализовано буквально.
2. **Чистые функции выделены явно.** `_buildGpsAttributionString`,
   `_getAvailableGpsSources` — без сайд-эффектов, легко тестируются.
3. **Качественные docstring'и.** Каждый из новых блоков (включая
   мотивацию «почему нельзя мутировать source.attribution») подписан
   ссылкой на ADR-013 § и F-NN из 12-review.md — следующий разработчик
   быстро поймёт контекст.
4. **In-flight Promise paterns.** `_healthFetchPromise` корректно
   предотвращает race condition при одновременном
   `onPublicTracksCheckbox` + `togglePublicTracksFiltersSheet`.
5. **Fallback-цепочка.** При сетевой ошибке `/health` UI остаётся
   функциональным (`GPS_FALLBACK_SOURCES` + OSM-only attribution).
6. **Минимальный diff.** Только `src/web/gps_tracks.js` (+159/-58),
   никаких побочных изменений — chirurgical fix.

## Что нужно сделать перед закрытием этапа Реализации

Ничего не блокирующего. По желанию (для качества кода):

- Реализовать F-03 (по крайней мере UT-CFG-01/02 и IT-API-04) или
  зафиксировать deferred в `13-test-report.md`.
- Поправить F-04 (3 строки кода, экономит rate-limit).
- Убрать F-05 dead branch (1 строка).
- Доделать F-08 node-тесты F-01/F-02 fix'ов (опционально, ~10 строк).

F-06/F-07 — на усмотрение, эстетика.

## Запреты, которые я соблюдал

- Я не правил код реализации.
- Я не апрувил PR от того же экземпляра Developer.
- Все findings выше ссылаются на конкретные строки кода и пункты
  ADR-013 / 04-test-plan.yaml.
