---
type: code-review
work_item_id: ET-006
title: "Code Review: Загрузка и визуализация GPX-треков"
version: 2
status: APPROVED
created_at: 2026-05-22
updated_at: 2026-05-22
authors:
  - "agent:reviewer"
branch: feature/ET-006-gpx-upload
---

# Code Review — ET-006: Загрузка и визуализация GPX-треков

## Вердикт

**APPROVED** — блокирующих findings нет.

Повторное ревью после коммита `25e4476` («fix(gpx): устранить падение
статистики на больших треках, учесть все треки файла»), который закрывает
все замечания v1. P1-1, P2-1, P2-2 исправлены и подтверждены чтением кода
и регрессионными тестами. P2-3 снят как ложное срабатывание (см. ниже).
Остаются только три P3 nice-to-have — не блокируют приёмку.

Реализация сильная: контракт интеграции ADR-002 соблюдён буквально,
стратегия парсинга ADR-003 выдержана полностью, REQ-F-01…F-13 присутствуют,
unit-тесты добротные и пополнены регрессиями по ревью.

## Что проверено (v2)

- ТЗ: `docs/work-items/ET-006/02-trz.md` (v2)
- AC: `docs/work-items/ET-006/03-acceptance-criteria.md` (v2)
- ADR: `06-adr/ADR-002` (структура модуля), `06-adr/ADR-003` (парсинг)
- Data Requirements: `08-data-requirements.md`
- Diff `origin/main...HEAD` (а не stale local `main` — см. P2-3):
  `src/web/gpx.js` (новый, 1242 строки), `src/web/index.html` (+46),
  `src/web/app.js` (+2: хук REQ-F-13), `src/web/app.css` (+148),
  `tests/unit/gpx.test.js`, `tests/unit/test_gpx_upload.py`
- Коммит-фикс `25e4476` целиком (`gpx.js` +233/-60, `gpx.test.js` +96)
- CLAUDE.md

---

## Статус findings прошлого ревью (v1)

### P1-1 — `trackStats` падал на больших треках — **ИСПРАВЛЕНО**

`Math.min/max.apply` по массиву высот убран. Расчёт переписан на
однопроходный аккумулятор (`makeStatsAccumulator` / `accumulatePoint` /
`finalizeStats`, `gpx.js:102-172`). `apply` в горячем пути отсутствует.
Проверена эквивалентность нового однопроходного расчёта старому: длина и
мин/макс — по всем точкам, набор/сброс — по той же подпоследовательности
точек с высотой, тот же фильтр шума `ELE_NOISE_M`. Добавлена регрессия
`gpx.test.js` — трек 500K точек, `assert.doesNotThrow`. REQ-NF-01 закрыт.

### P2-1 — статистика/профиль только по `tracks[0]` — **ИСПРАВЛЕНО**

Добавлены `aggregateStats()` (`gpx.js:211`) и `buildFileProfileSamples()`
(`gpx.js:1119`). `renderStats` (`gpx.js:979`) и `renderElevationProfile`
(`gpx.js:1011`) теперь сводят статистику и профиль по **всем** трекам
файла. Решение по семантике корректное: длина и набор/сброс суммируются
потрековно (скачок высоты на стыке треков не даёт ложный набор/сброс),
мин/макс — экстремумы. Покрыто тестами (агрегация, трек без высот, все
треки без высот, склейка профиля). Закрывает REQ-F-09/AC-02 для
многотрековых файлов.

### P2-2 — расчёт статистики не чанковался — **ИСПРАВЛЕНО**

Добавлен `trackStatsChunked()` (`gpx.js:183`), вызывается в
`extractGpxModelChunked` (`gpx.js:464`) — асинхронный путь парсинга
больших файлов. Эквивалентность синхронному `trackStats` подтверждена
тестом, в т.ч. на треке длиннее `CHUNK_SIZE`. ADR-003 §2 выдержан
буквально. Синхронный `trackStats` остаётся в `extractGpxModel` — этот
путь используется только unit-тестами, не загрузкой файла.

### P2-3 — «объём PR выходит за рамки ET-006» — **СНЯТО (ложное срабатывание)**

Замечание v1 — артефакт измерения. Прошлое ревью считало diff против
**локального `main`** (`832099c`), который был устаревшим. Против
`origin/main` (`6effac9`, уже содержит ET-001/002/005) diff PR —
**строго ET-006**: 19 файлов, `gpx.js` + 3 правки разметки + 2 строки
`app.js` + тесты + docs ET-006. Ни одного чужого work-item. Конвенция
CLAUDE.md «одна задача = одна ветка» не нарушена. Претензий нет.

### P3-1 — единицы профиля высот — **ИСПРАВЛЕНО**

Ось (`gpx.js:1078-1080`) и tooltip (`gpx.js:1155`) форматируют расстояние
через `formatKm()` — согласовано со статистикой и выбором км/мили (ET-005).

### P3-2 — имя `childText` — **ИСПРАВЛЕНО**

Переименована в `firstTagText` (`gpx.js:326`), JSDoc уточнён.

### P3-4 — дублирующийся `'use strict'` — **ИСПРАВЛЕНО**

Верхнеуровневый `'use strict'` убран; остался один — внутри IIFE
(`gpx.js:26`), что и нужно для строгого режима модуля.

---

## Остаточные findings (не блокируют)

### P3 — nice-to-have

- **P3-3.** Грязная история git: мерж устаревшей ветки (`8dc150a`)
  продублировал 4 коммита ET-006 (`62c2ee8`/`fda40f2`,
  `2104f12`/`a0546ab`, `73b29ae`/`9fc1ef4`, `dcf3d24`/`9b930c5`).
  Итоговое дерево корректно, diff чистый; запутаны лишь `git log` /
  `git bisect`. Исправление потребовало бы rewrite истории — на
  усмотрение Owner при мерже (можно squash).
- **P3-5.** Многосегментные треки: `collectRawTracks` склеивает все
  `<trkseg>` одного `<trk>` в один плоский `points` → один `LineString`.
  Соответствует модели TRZ §4 / data-requirements §3, формально по ТЗ.
  При разрыве сегментов рисуется прямая-перемычка, разрыв попадает в
  `distanceKm`. На будущее — рассмотреть `MultiLineString` (REQ-F-04
  это допускает).
- **P3-6.** Числовые примеры в `04-test-plan.yaml` некорректны: U-10
  «≈28.3 км» (Haversine даёт ≈25.5 км), U-11 «elevLoss = 70 м (30+20)»
  (30+20=50). Реализация и тесты следуют TRZ §5 правильно; расхождение
  задокументировано в шапке `gpx.test.js`. Примеры стоит поправить на
  этапе Анализа (правило CLAUDE.md №3) — артефакт не входит в код PR.

Новых P0/P1/P2 коммит-фикс не внёс — проверено построчно.

---

## Соответствие требованиям

### ТЗ — REQ-F / REQ-NF

| Требование | Статус | Комментарий |
|---|---|---|
| REQ-F-01 кнопка загрузки | OK | `btn-gpx-upload` между компасом и геолокацией; `accept=".gpx" multiple` |
| REQ-F-02 парсинг | OK | DOMParser, GPX 1.1, trk/trkseg/wpt/rte, fallback без namespace |
| REQ-F-03 валидация | OK | лимит 50 МБ, toast для empty и oversize |
| REQ-F-04 отрисовка трека | OK | line 4px, opacity 0.85, палитра 8 цветов, z-order ниже OSRM |
| REQ-F-05 waypoints | OK | circle + symbol, цвет файла, label по `<name>` |
| REQ-F-06 fit bounds | OK | padding 50, только по последнему файлу |
| REQ-F-07 множественная загрузка | OK | накопление, цвет по индексу |
| REQ-F-08 удаление | OK | снимаются слои/источники/маркеры; активный → детали скрываются |
| REQ-F-09 GPX Sheet | OK | `#sheet-gpx` на `.bottom-sheet`, авто-открытие, `tb-gpx` |
| REQ-F-10 профиль высот | OK | canvas 120px, интерактивность; сводно по всем трекам файла (P2-1 закрыт) |
| REQ-F-11 статистика | OK | 5 полей, «—» без высот; сводно по всем трекам файла (P2-1 закрыт) |
| REQ-F-12 интерактивность на карте | OK | click активирует трек, cursor pointer |
| REQ-F-13 сохранение при setStyle | OK | хук в `rebuildMapOverlays`, данные на `window`, z-order восстанавливается |
| REQ-NF-01 производительность | OK | P1-1 устранён; парсинг и расчёт статистики чанкуются (P2-2 закрыт) |
| REQ-NF-03 UX | OK | toast вместо alert, параллельно с роутингом |
| REQ-NF-04 хранение | OK | только в памяти, без localStorage |

### ADR

- **ADR-002** — соблюдён буквально. `gpx.js` — отдельный классический
  скрипт, подключён после `app.js`; `app.js` получил ровно один хук
  `if (typeof rebuildGpxOverlays === 'function') rebuildGpxOverlays();`
  (+ строка-комментарий), защищённый `typeof`; контракт интеграции
  (`_map`, `openSheet`/`closeSheet`, `showToast`) выдержан.
- **ADR-003** — соблюдён полностью. `DOMParser` в основном потоке, Web
  Worker не используется, конвертация DOM → модель **и расчёт
  статистики** идут чанками с отдачей управления event loop. Частичное
  отклонение из v1 (P2-2) устранено.

---

## Положительные моменты

- Контракт интеграции ADR-002 выдержан буквально — минимальная
  поверхность связности, `app.js` остаётся валидным без `gpx.js`.
- P1-1-фикс сделан правильно: однопроходный аккумулятор переиспользован
  и синхронным `trackStats`, и чанковым `trackStatsChunked` — единый
  путь расчёта, без дублирования логики.
- P2-1-фикс корректен по семантике: набор/сброс агрегируются потрековно,
  а не по сквозному потоку точек — стык треков не порождает ложный
  набор высоты.
- Z-order REQ-F-04: `ROUTE_BASE_LAYERS` совпадает с реальными id слоёв
  OSRM; в `rebuildMapOverlays` маршрут пересоздаётся раньше GPX —
  z-order переживает `setStyle` (AC-12).
- Безопасность: имя файла экранируется (`escapeHtml`), имена waypoints
  идут в MapLibre `text-field` (не в HTML), генерируемые id безопасны.
- Обработчики событий карты отслеживаются по файлу (`mapHandlers`) и
  корректно снимаются (`clearMapHandlers`) — нет утечки слушателей.
- Unit-тесты добротные: исполняют реальный модуль, покрывают U-01…U-21,
  чистые функции и регрессии по ревью (P1-1, P2-1, P2-2).

Интеграционные (I-01…I-12) и e2e (E-01…E-10) сценарии не
автоматизированы — ожидаемо, они в зоне этапа тестирования
(`test_gpx_upload.py` это фиксирует). К разработчику претензий нет.

---

## Итог

Блокирующих findings нет. PR готов к этапу тестирования. P3-3 / P3-5 /
P3-6 — на усмотрение Owner/Анализа, приёмку не задерживают.
