enduro-trails/docs/work-items/ET-006/12-review.md

type, work_item_id, title, version, status, created_at, updated_at, authors, branch

type	work_item_id	title	version	status	created_at	updated_at	authors	branch
code-review	ET-006	Code Review: Загрузка и визуализация GPX-треков	2	APPROVED	2026-05-22	2026-05-22	agent:reviewer	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/Анализа, приёмку не задерживают.