From 25e4476cf704fdd7ae98c6376b734db670b670d4 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Fri, 22 May 2026 06:01:51 +0000 Subject: [PATCH] =?UTF-8?q?fix(gpx):=20=D1=83=D1=81=D1=82=D1=80=D0=B0?= =?UTF-8?q?=D0=BD=D0=B8=D1=82=D1=8C=20=D0=BF=D0=B0=D0=B4=D0=B5=D0=BD=D0=B8?= =?UTF-8?q?=D0=B5=20=D1=81=D1=82=D0=B0=D1=82=D0=B8=D1=81=D1=82=D0=B8=D0=BA?= =?UTF-8?q?=D0=B8=20=D0=BD=D0=B0=20=D0=B1=D0=BE=D0=BB=D1=8C=D1=88=D0=B8?= =?UTF-8?q?=D1=85=20=D1=82=D1=80=D0=B5=D0=BA=D0=B0=D1=85,=20=D1=83=D1=87?= =?UTF-8?q?=D0=B5=D1=81=D1=82=D1=8C=20=D0=B2=D1=81=D0=B5=20=D1=82=D1=80?= =?UTF-8?q?=D0=B5=D0=BA=D0=B8=20=D1=84=D0=B0=D0=B9=D0=BB=D0=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Правки по код-ревью ET-006 (docs/work-items/ET-006/12-review.md): - P1-1: trackStats считал min/max высот через Math.min/max.apply — на треках в сотни тысяч точек это бросало RangeError и валило загрузку файла (нарушение REQ-NF-01). Расчёт переписан на однопроходный аккумулятор (makeStatsAccumulator/accumulatePoint/finalizeStats) без apply. - P2-1: статистика и профиль высот учитывали только tracks[0]. Добавлены aggregateStats() и buildFileProfileSamples() — сводка и профиль теперь охватывают все треки файла (REQ-F-09, AC-02). - P2-2: расчёт статистики на async-пути парсинга вынесен в чанковый trackStatsChunked() — соответствие букве ADR-003 §2. - P3-1: ось и tooltip профиля высот форматируют расстояние через formatKm() — согласование с выбором км/мили из ET-005. - P3-2: childText() переименована в firstTagText() — имя соответствует фактическому поведению (поиск по всем потомкам). - P3-4: убран дублирующийся 'use strict'. Добавлены регрессионные unit-тесты: большой трек без падения, эквивалентность trackStatsChunked синхронному trackStats (в т.ч. на треке длиннее размера чанка), агрегация статистики и профиля по многотрековому файлу. Refs: ET-006 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/web/gpx.js | 237 ++++++++++++++++++++++++++++++----------- tests/unit/gpx.test.js | 96 ++++++++++++++++- 2 files changed, 271 insertions(+), 62 deletions(-) diff --git a/src/web/gpx.js b/src/web/gpx.js index 8f998e7..e37514c 100644 --- a/src/web/gpx.js +++ b/src/web/gpx.js @@ -1,5 +1,3 @@ -'use strict'; - /** * gpx.js — ET-006: загрузка и визуализация GPX-треков. * @@ -15,8 +13,9 @@ * `removeGpxTrack`), хелпер `showToast()` и хук `rebuildGpxOverlays()` * (вызывается из `rebuildMapOverlays()` app.js — REQ-F-13). * - * Парсинг — `DOMParser` в основном потоке, конвертация DOM → модель - * выполняется чанками с отдачей управления event loop (ADR-003). + * Парсинг — `DOMParser` в основном потоке; конвертация DOM → модель и + * расчёт статистики выполняются чанками с отдачей управления event loop + * (ADR-003). * * Для unit-тестов модуль дополнительно экспортируется через * `module.exports` (среда Node) — публикуются чистые функции и парсер. @@ -90,57 +89,141 @@ } /** - * Считает статистику трека по массиву точек [lon, lat, ele, time]. + * Создаёт аккумулятор статистики трека для однопроходного расчёта. + * + * Однопроходный обход (вместо `Math.min/max.apply` по массиву высот) + * обязателен для больших треков: `Function.prototype.apply` на сотнях + * тысяч аргументов бросает `RangeError: Maximum call stack size + * exceeded` и валит загрузку файла (ревью P1-1, REQ-NF-01). + * + * @returns {{distanceKm:number, elevGain:number, elevLoss:number, + * eleMin:number, eleMax:number, hasEle:boolean, ref:?number}} + */ + function makeStatsAccumulator() { + return { + distanceKm: 0, elevGain: 0, elevLoss: 0, + eleMin: Infinity, eleMax: -Infinity, hasEle: false, ref: null, + }; + } + + /** + * Учитывает точку `points[i]` в аккумуляторе: длина (Haversine от + * предыдущей точки), мин/макс высот и набор/сброс с фильтрацией шума + * < 2 м — мелкие колебания не сдвигают опорную высоту (TRZ §5.2, + * тест U-12). + * @param {object} acc - аккумулятор `makeStatsAccumulator()`. + * @param {Array} points - точки трека. + * @param {number} i - индекс обрабатываемой точки. + */ + function accumulatePoint(acc, points, i) { + if (i > 0) acc.distanceKm += haversineKm(points[i - 1], points[i]); + var e = points[i][2]; + if (e === null || e === undefined || isNaN(e)) return; + acc.hasEle = true; + if (e < acc.eleMin) acc.eleMin = e; + if (e > acc.eleMax) acc.eleMax = e; + if (acc.ref === null) { acc.ref = e; return; } + var d = e - acc.ref; + if (Math.abs(d) >= ELE_NOISE_M) { + if (d > 0) acc.elevGain += d; else acc.elevLoss += -d; + acc.ref = e; + } + } + + /** + * Сворачивает аккумулятор в итоговый объект статистики. При отсутствии + * данных высот поля высот — `null` (TRZ REQ-F-11; тесты U-05, U-14). + * @param {object} acc + * @returns {{distanceKm:number, elevGain:?number, elevLoss:?number, + * eleMin:?number, eleMax:?number}} + */ + function finalizeStats(acc) { + if (!acc.hasEle) { + return { + distanceKm: acc.distanceKm, elevGain: null, elevLoss: null, + eleMin: null, eleMax: null, + }; + } + return { + distanceKm: acc.distanceKm, + elevGain: Math.round(acc.elevGain), + elevLoss: Math.round(acc.elevLoss), + eleMin: Math.round(acc.eleMin), + eleMax: Math.round(acc.eleMax), + }; + } + + /** + * Считает статистику трека по массиву точек [lon, lat, ele, time] + * однопроходно (синхронно). * * Длина — сумма Haversine-сегментов. Набор/сброс высот — сумма дельт - * `ele` с фильтрацией шума: колебания < 2 м не сбрасывают опорную высоту - * (TRZ §5.2). При отсутствии данных высот поля высот возвращают `null` - * (TRZ REQ-F-11; тесты U-05, U-14). + * `ele` с фильтрацией шума < 2 м (TRZ §5.2). При отсутствии данных + * высот поля высот возвращают `null` (TRZ REQ-F-11; тесты U-05, U-14). * * @param {Array} points - точки трека. * @returns {{distanceKm:number, elevGain:?number, elevLoss:?number, * eleMin:?number, eleMax:?number}} */ function trackStats(points) { - var distanceKm = 0; - var i; - for (i = 1; i < points.length; i++) { - distanceKm += haversineKm(points[i - 1], points[i]); - } + var acc = makeStatsAccumulator(); + for (var i = 0; i < points.length; i++) accumulatePoint(acc, points, i); + return finalizeStats(acc); + } - var eles = []; - for (i = 0; i < points.length; i++) { - var e = points[i][2]; - if (e !== null && e !== undefined && !isNaN(e)) eles.push(e); - } - - if (eles.length === 0) { - return { - distanceKm: distanceKm, elevGain: null, elevLoss: null, - eleMin: null, eleMax: null, - }; - } - - var elevGain = 0; - var elevLoss = 0; - var ref = null; - for (i = 0; i < eles.length; i++) { - if (ref === null) { ref = eles[i]; continue; } - var d = eles[i] - ref; - // Шум < 2 м не сдвигает опорную высоту: мелкие колебания вокруг - // одного уровня не накапливаются в набор/сброс (тест U-12). - if (Math.abs(d) >= ELE_NOISE_M) { - if (d > 0) elevGain += d; else elevLoss += -d; - ref = eles[i]; + /** + * Считает статистику трека чанками, отдавая управление event loop между + * порциями. Реализует ADR-003 §2 («расчёт статистики — чанками»); + * применяется на пути асинхронного парсинга больших файлов (ревью P2-2). + * + * @param {Array} points - точки трека. + * @returns {Promise<{distanceKm:number, elevGain:?number, + * elevLoss:?number, eleMin:?number, eleMax:?number}>} + */ + function trackStatsChunked(points) { + return new Promise(function (resolve) { + var acc = makeStatsAccumulator(); + var i = 0; + function run() { + var end = Math.min(i + CHUNK_SIZE, points.length); + for (; i < end; i++) accumulatePoint(acc, points, i); + if (i < points.length) { setTimeout(run, 0); return; } + resolve(finalizeStats(acc)); } - } + run(); + }); + } + /** + * Агрегирует статистику всех треков файла в одну сводку. + * + * Активная сущность панели — файл (TRZ REQ-F-09, AC-02), но файл может + * содержать несколько `` (REQ-F-02). Длина и набор/сброс — суммы + * по трекам; мин/макс — экстремумы по трекам. Набор/сброс считаются в + * каждом треке отдельно и суммируются (а не по сквозному потоку точек): + * так скачок высоты на стыке треков не даёт ложный набор/сброс. Если + * ни у одного трека нет высот — поля высот `null` (ревью P2-1). + * + * @param {Array<{stats:object}>} tracks - треки файла. + * @returns {{distanceKm:number, elevGain:?number, elevLoss:?number, + * eleMin:?number, eleMax:?number}} + */ + function aggregateStats(tracks) { + var distanceKm = 0; + var elevGain = null, elevLoss = null, eleMin = null, eleMax = null; + tracks.forEach(function (t) { + var s = t.stats; + if (!s) return; + distanceKm += s.distanceKm || 0; + if (s.elevGain === null || s.elevGain === undefined) return; + elevGain = (elevGain === null ? 0 : elevGain) + s.elevGain; + elevLoss = (elevLoss === null ? 0 : elevLoss) + s.elevLoss; + eleMin = (eleMin === null) ? s.eleMin : Math.min(eleMin, s.eleMin); + eleMax = (eleMax === null) ? s.eleMax : Math.max(eleMax, s.eleMax); + }); return { - distanceKm: distanceKm, - elevGain: Math.round(elevGain), - elevLoss: Math.round(elevLoss), - eleMin: Math.round(Math.min.apply(null, eles)), - eleMax: Math.round(Math.max.apply(null, eles)), + distanceKm: distanceKm, elevGain: elevGain, elevLoss: elevLoss, + eleMin: eleMin, eleMax: eleMax, }; } @@ -233,12 +316,14 @@ } /** - * Возвращает текст первого дочернего элемента с указанным тегом. + * Возвращает текст первого элемента-потомка с указанным тегом. + * Поиск ведётся по всем потомкам (`getElementsByTagName`), не только + * по прямым детям — для структуры GPX это безопасно. * @param {Element} parent * @param {string} tag * @returns {string} */ - function childText(parent, tag) { + function firstTagText(parent, tag) { var els = parent.getElementsByTagName(tag); return els.length ? String(els[0].textContent || '').trim() : ''; } @@ -264,7 +349,7 @@ * @returns {{lon:number, lat:number, name:?string, ele:?number}} */ function waypointFromEl(el) { - var name = childText(el, 'name'); + var name = firstTagText(el, 'name'); var eleEls = el.getElementsByTagName('ele'); var ele = eleEls.length ? parseFloat(eleEls[0].textContent) : NaN; return { @@ -304,13 +389,13 @@ var tps = segs[j].getElementsByTagName('trkpt'); for (k = 0; k < tps.length; k++) ptEls.push(tps[k]); } - result.push({ name: childText(trk, 'name') || ('Трек ' + (i + 1)), ptEls: ptEls }); + result.push({ name: firstTagText(trk, 'name') || ('Трек ' + (i + 1)), ptEls: ptEls }); } var rtes = doc.getElementsByTagName('rte'); for (i = 0; i < rtes.length; i++) { var rte = rtes[i]; var rps = toArray(rte.getElementsByTagName('rtept')); - result.push({ name: childText(rte, 'name') || ('Маршрут ' + (i + 1)), ptEls: rps }); + result.push({ name: firstTagText(rte, 'name') || ('Маршрут ' + (i + 1)), ptEls: rps }); } return result; } @@ -375,8 +460,11 @@ } return mapChunked(raw[idx].ptEls, pointFromEl).then(function (pts) { var points = pts.filter(isValidPoint); - tracks.push({ name: raw[idx].name, points: points, stats: trackStats(points) }); - return nextTrack(idx + 1); + // Расчёт статистики — тоже чанками (ADR-003 §2; ревью P2-2). + return trackStatsChunked(points).then(function (stats) { + tracks.push({ name: raw[idx].name, points: points, stats: stats }); + return nextTrack(idx + 1); + }); }); } @@ -880,14 +968,15 @@ } /** - * Отрисовывает компактную сетку статистики активного трека - * (TRZ REQ-F-11; AC-08). + * Отрисовывает компактную сетку статистики активного файла — + * сводно по всем его трекам (TRZ REQ-F-11; AC-08; ревью P2-1). * @param {object} file */ function renderStats(file) { var el = document.getElementById('gpx-stats'); if (!el) return; - var st = file.tracks.length ? file.tracks[0].stats : null; + // Сводка по всем трекам файла, а не только tracks[0] (ревью P2-1). + var st = file.tracks.length ? aggregateStats(file.tracks) : null; var hasEle = st && st.elevGain !== null; var cells = [ @@ -908,8 +997,9 @@ // ─── Профиль высот (canvas) ────────────────────────────────────────────── /** - * Отрисовывает canvas-профиль высот активного трека и подключает - * интерактивность (tooltip + маркер-курсор на карте) — TRZ REQ-F-10. + * Отрисовывает canvas-профиль высот активного файла (по всем его + * трекам) и подключает интерактивность (tooltip + маркер-курсор на + * карте) — TRZ REQ-F-10; ревью P2-1. * @param {object} file */ function renderElevationProfile(file) { @@ -918,8 +1008,7 @@ var axisEl = document.getElementById('gpx-elevation-axis'); if (!canvas) return; - var track = file.tracks.length ? file.tracks[0] : null; - var samples = track ? buildProfileSamples(track) : []; + var samples = buildFileProfileSamples(file); if (samples.length < 2) { canvas.style.display = 'none'; @@ -984,9 +1073,11 @@ if (axisEl) { var spans = axisEl.getElementsByTagName('span'); if (spans.length >= 3) { - spans[0].textContent = '0 км'; - spans[1].textContent = (totalKm / 2).toFixed(1) + ' км'; - spans[2].textContent = totalKm.toFixed(1) + ' км'; + // Единицы — через formatKm, чтобы ось согласовалась со + // статистикой при выборе миль в ET-005 (ревью P3-1). + spans[0].textContent = formatKm(0); + spans[1].textContent = formatKm(totalKm / 2); + spans[2].textContent = formatKm(totalKm); } } @@ -1017,6 +1108,26 @@ return samples; } + /** + * Строит точки профиля высот для всего файла: треки склеиваются + * последовательно, расстояние `d` — нарастающим итогом от старта + * первого трека. Покрывает многотрековые файлы — профиль показывает + * все треки файла, а не только tracks[0] (ревью P2-1). + * @param {object} file - элемент модели window.gpxTracks. + * @returns {Array<{d:number, e:number, lon:number, lat:number}>} + */ + function buildFileProfileSamples(file) { + var samples = []; + var offset = 0; + file.tracks.forEach(function (track) { + buildProfileSamples(track).forEach(function (s) { + samples.push({ d: s.d + offset, e: s.e, lon: s.lon, lat: s.lat }); + }); + offset += (track.stats && track.stats.distanceKm) || 0; + }); + return samples; + } + /** * Навешивает обработчики наведения/тапа на canvas профиля: tooltip с * высотой и расстоянием + маркер-курсор на карте (TRZ REQ-F-10; AC-07). @@ -1040,7 +1151,8 @@ var tip = document.getElementById('gpx-elevation-tip'); if (tip) { - tip.textContent = Math.round(nearest.e) + ' м · ' + nearest.d.toFixed(1) + ' км'; + // Расстояние — через formatKm (учёт км/мили из ET-005, ревью P3-1). + tip.textContent = Math.round(nearest.e) + ' м · ' + formatKm(nearest.d); tip.style.display = 'block'; tip.style.left = Math.max(0, Math.min(profileState.cssW - 90, x - 45)) + 'px'; } @@ -1098,10 +1210,13 @@ MAX_FILE_BYTES: MAX_FILE_BYTES, haversineKm: haversineKm, trackStats: trackStats, + trackStatsChunked: trackStatsChunked, + aggregateStats: aggregateStats, colorForIndex: colorForIndex, tracksToGeoJSON: tracksToGeoJSON, waypointsToGeoJSON: waypointsToGeoJSON, fileBounds: fileBounds, + buildFileProfileSamples: buildFileProfileSamples, extractGpxModel: extractGpxModel, parseGpxText: parseGpxText, parseGpxAsync: parseGpxAsync, diff --git a/tests/unit/gpx.test.js b/tests/unit/gpx.test.js index 9a2ac9d..7ca7df3 100644 --- a/tests/unit/gpx.test.js +++ b/tests/unit/gpx.test.js @@ -7,7 +7,10 @@ * - unit-gpx-parser (U-01..U-08) * - unit-gpx-stats (U-10..U-14) * - unit-gpx-colors (U-20, U-21) - * плюс чистые функции построения GeoJSON и bbox. + * плюс чистые функции построения GeoJSON и bbox, плюс регрессии по + * замечаниям код-ревью ET-006: P1-1 (большие треки не валят расчёт + * статистики), P2-1 (агрегация статистики и профиля по всем трекам + * файла), P2-2 (чанковый расчёт статистики — trackStatsChunked). * * Тесты исполняют РЕАЛЬНЫЙ модуль src/web/gpx.js. Браузерный примитив * `DOMParser` (ADR-003) в Node отсутствует, поэтому подставляется @@ -398,6 +401,40 @@ test('trackStats: пустой трек — нулевая длина без п assert.equal(stats.elevGain, null); }); +test('P1-1: trackStats не падает на треке с сотнями тысяч точек высот', () => { + // Регрессия ревью P1-1: Math.min/max.apply на массиве такого размера + // бросал RangeError: Maximum call stack size exceeded → файл не + // загружался (нарушение REQ-NF-01). Однопроходный обход — без apply. + const points = []; + for (let i = 0; i < 500000; i++) { + points.push([37.6 + i * 1e-6, 55.7 + i * 1e-6, 100 + (i % 50)]); + } + let stats; + assert.doesNotThrow(() => { stats = Gpx.trackStats(points); }); + assert.equal(stats.eleMin, 100); + assert.equal(stats.eleMax, 149); + assert.ok(stats.distanceKm > 0, 'длина считается на большом треке'); +}); + +test('trackStatsChunked даёт тот же результат, что и синхронный trackStats', async () => { + const points = [ + [37.6, 55.70, 100], [37.6, 55.71, 150], [37.6, 55.72, 120], + [37.6, 55.73, 200], [37.6, 55.74, 180], + ]; + const chunked = await Gpx.trackStatsChunked(points); + assert.deepEqual(chunked, Gpx.trackStats(points)); +}); + +test('trackStatsChunked: расчёт верен на треке длиннее размера чанка', async () => { + // > CHUNK_SIZE (8000) точек — статистика проходит через несколько чанков. + const points = []; + for (let i = 0; i < 20000; i++) { + points.push([37.6 + i * 1e-5, 55.7, 100 + (i % 30)]); + } + const chunked = await Gpx.trackStatsChunked(points); + assert.deepEqual(chunked, Gpx.trackStats(points)); +}); + // ─── unit-gpx-colors : U-20, U-21 ────────────────────────────────────────── test('U-20: первый файл получает первый цвет палитры', () => { @@ -456,6 +493,63 @@ test('fileBounds: файл без точек → null', () => { assert.equal(Gpx.fileBounds({ tracks: [], waypoints: [] }), null); }); +// ─── Агрегация по файлу: aggregateStats / buildFileProfileSamples (P2-1) ──── + +test('P2-1: aggregateStats суммирует статистику всех треков файла', () => { + // Ревью P2-1: панель показывает один файл, но файл может содержать + // несколько — статистика должна охватывать их все, не только [0]. + const tracks = [ + { stats: { distanceKm: 10, elevGain: 100, elevLoss: 50, eleMin: 120, eleMax: 300 } }, + { stats: { distanceKm: 5, elevGain: 40, elevLoss: 20, eleMin: 90, eleMax: 250 } }, + ]; + const agg = Gpx.aggregateStats(tracks); + assert.equal(agg.distanceKm, 15); + assert.equal(agg.elevGain, 140); + assert.equal(agg.elevLoss, 70); + assert.equal(agg.eleMin, 90); + assert.equal(agg.eleMax, 300); +}); + +test('P2-1: aggregateStats — трек без высот не ломает агрегацию', () => { + const tracks = [ + { stats: { distanceKm: 10, elevGain: 100, elevLoss: 50, eleMin: 120, eleMax: 300 } }, + { stats: { distanceKm: 5, elevGain: null, elevLoss: null, eleMin: null, eleMax: null } }, + ]; + const agg = Gpx.aggregateStats(tracks); + assert.equal(agg.distanceKm, 15); + assert.equal(agg.elevGain, 100); + assert.equal(agg.elevLoss, 50); + assert.equal(agg.eleMin, 120); + assert.equal(agg.eleMax, 300); +}); + +test('P2-1: aggregateStats — все треки без высот → поля высот null', () => { + const tracks = [ + { stats: { distanceKm: 7, elevGain: null, elevLoss: null, eleMin: null, eleMax: null } }, + ]; + const agg = Gpx.aggregateStats(tracks); + assert.equal(agg.distanceKm, 7); + assert.equal(agg.elevGain, null); + assert.equal(agg.elevLoss, null); + assert.equal(agg.eleMin, null); + assert.equal(agg.eleMax, null); +}); + +test('P2-1: buildFileProfileSamples объединяет высоты всех треков файла', () => { + const t1 = { points: [[37.60, 55.70, 100], [37.61, 55.70, 200]] }; + const t2 = { points: [[37.70, 55.80, 300], [37.71, 55.80, 400]] }; + t1.stats = Gpx.trackStats(t1.points); + t2.stats = Gpx.trackStats(t2.points); + const samples = Gpx.buildFileProfileSamples({ tracks: [t1, t2] }); + // Все 4 точки с высотой попали в профиль — не только из tracks[0]. + assert.equal(samples.length, 4); + assert.deepEqual(samples.map((s) => s.e), [100, 200, 300, 400]); + // Расстояние — сквозное: второй трек смещён на длину первого. + assert.equal(samples[0].d, 0); + assert.ok(samples[2].d >= t1.stats.distanceKm - 1e-9); + assert.ok(samples[3].d > samples[2].d); +}); + // ─── Контракт модуля ─────────────────────────────────────────────────────── test('модуль публикует window.Gpx и onclick-обработчики', () => {