auto-sync: 2026-05-14 23:30:01

tasks/enduro-trails/CODE_REVIEW.md

@@ -0,0 +1,219 @@
+# Code Review: Enduro Trails
+
+**Дата:** 2026-05-14  
+**Проект:** Enduro Trails — веб-приложение для эндуро-маршрутов  
+**Стек:** Python (FastAPI) + MapLibre GL JS + SQLite  
+**Ревьюер:** Dev Agent
+
+---
+
+## Общая оценка
+
+Проект представляет собой рабочий прототип с хорошей функциональностью: векторные тайлы, роутинг через OSRM, разведка, красивые маршруты, линейка, метки. UI продуман для мобильных устройств. Однако есть серьёзные проблемы с архитектурой фронтенда (монолитный файл ~2800 строк), безопасностью, и потенциальные баги в работе с ресурсами.
+
+---
+
+## 1. Архитектура и структура кода
+
+### Critical
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 1 | Монолитный фронтенд | `static/app.js` | ~2800 строк в одном файле без модулей. Невозможно тестировать, сложно поддерживать. Нужно разбить на модули (ES modules или bundler). |
+
+### High
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 2 | Глобальное состояние | `static/app.js` | Всё состояние хранится в глобальных переменных (`routeMode`, `rulerPoints`, `scenicRoutes`, etc.). Нет state management — любая функция может мутировать что угодно. |
+| 3 | Смешение ответственностей в бэкенде | `app.py` | Один файл содержит: WKB-парсер, MVT-билдер, route stats, OSRM-клиент, scenic logic, tile cache. Нужно разбить на модули (`tiles.py`, `routing.py`, `scenic.py`, `db.py`). |
+| 4 | Нет разделения API и статики | `app.py` | FastAPI раздаёт и API, и статику. В продакшене статику должен раздавать nginx/CDN. |
+
+### Medium
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 5 | package.json содержит только ssh2 | `package.json` | Зависимость `ssh2` — это деплой-утилита, не имеет отношения к приложению. Нет build-системы для фронтенда. |
+| 6 | Мусорные файлы в репозитории | корень | `check_terrain.js`, `debug_terrain2.js`, `deploy_dark_style.js`, `fix_nginx_perms.js` и т.д. — одноразовые скрипты, засоряют проект. |
+| 7 | `__pycache__` в репозитории | корень | Скомпилированные `.pyc` файлы не должны быть в VCS. Нужен `.gitignore`. |
+
+---
+
+## 2. Качество кода
+
+### High
+
+| # | Проблема | Файл | Строки | Описание |
+|---|----------|------|--------|----------|
+| 8 | Дублирование кода поиска | `app.js` | множество | `_doOnboardSearch`, `doWaypointSearch`, `_doMiniOnboardSearch`, `doSearch` — 4 почти идентичные функции для Nominatim-поиска. Нужна одна универсальная. |
+| 9 | Дублирование haversine | `app.js` + `app.py` | — | `haversineKm`, `haversineM` (JS) и `haversine_m` (Python) — одна и та же формула в 3 вариантах. |
+| 10 | HTML в строках JS | `app.js` | множество | Огромные HTML-шаблоны через template literals. Нет санитизации, нет компонентного подхода. |
+
+### Medium
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 11 | Magic numbers | `app.py` | `0.0015`, `0.005`, `500.0`, `5000`, `10000` — snap radius, delta, sampling step без именованных констант. |
+| 12 | Inconsistent naming | `app.js` | Смесь camelCase (`routeMode`) и snake_case в комментариях. Функции то с underscore (`_doOnboardSearch`), то без. |
+| 13 | Неиспользуемые импорты | `app.py` | `from typing import List` — в Python 3.11 можно использовать `list[]` напрямую. Мелочь, но показывает отсутствие линтера. |
+| 14 | Отсутствие JSDoc/TypeScript | `app.js` | Ни одной аннотации типов. При 2800 строках это критично для поддержки. |
+
+### Low
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 15 | Комментарии на смеси языков | оба | Русские комментарии в коде — ок для команды, но docstrings API лучше на английском для совместимости с инструментами. |
+
+---
+
+## 3. Потенциальные баги
+
+### Critical
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 16 | SQLite connection не thread-safe | `app.py` | `get_db()` создаёт новое соединение при каждом вызове, но uvicorn с `workers=4` запускает multiprocess. SQLite в WAL mode поддерживает concurrent reads, но без WAL mode возможны `database is locked` ошибки под нагрузкой. |
+| 17 | Connection leak в `calc_route_stats` | `app.py` | В `post_route` и `_build_segmented_route` соединение открывается, но если исключение произойдёт до `conn.close()` — утечка. Нужен `try/finally` или context manager. |
+
+### High
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 18 | Tile cache без thread safety | `app.py` | `_tile_cache` — обычный dict, мутируется из async handlers. При `workers=4` каждый процесс имеет свой кэш (ок), но внутри одного процесса asyncio event loop однопоточный — тоже ок. Однако FIFO-вытеснение через `next(iter(...))` не гарантирует LRU-поведение. |
+| 19 | `initMiniRouteInteraction` клонирует DOM | `app.js` | `mini.cloneNode(true)` + `replaceChild` — каждый вызов пересоздаёт элемент. Это теряет все event listeners на дочерних элементах и может вызвать утечки памяти при частом переключении маршрутов. |
+| 20 | Race condition в buildRoute | `app.js` | `debounceBuildRoute` с 300ms debounce, но если пользователь быстро добавляет точки, предыдущий fetch не отменяется (нет AbortController). Результат старого запроса может перезаписать новый. |
+
+### Medium
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 21 | `removeRulerPoint` не обновляет индексы корректно | `app.js` | `btn.onclick` замыкает `idx` через `updateRulerLabels`, но после splice индексы сдвигаются. Повторное удаление может удалить не ту точку. |
+| 22 | Бесконечная рекурсия в `_osrm_segment_alternatives` | `app.py` | `depth < 2` ограничивает, но при `itertools.product(alts_a[:3], alts_b[:3])` на глубине 2 это до 9 комбинаций × 9 = 81 маршрут. Комбинаторный взрыв при 3+ waypoints. |
+| 23 | `selectMarkerType` использует `prompt()` | `app.js` | Блокирующий `prompt()` — плохой UX на мобильных. Некоторые браузеры ограничивают его. |
+
+---
+
+## 4. Производительность
+
+### High
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 24 | O(n) snap в `getRouteSegmentDistances` | `app.js` | Для каждого waypoint перебирает ВСЕ точки геометрии (`for (let j = 0; j < n; j++)`). При маршруте 500км с 10000 точек и 10 waypoints = 100000 итераций на каждый render. |
+| 25 | Синхронный SQLite в async handler | `app.py` | `get_db()` + `cur.execute()` блокируют event loop. FastAPI запускает sync-функции в threadpool только если endpoint не async. Здесь endpoints `async` — значит блокировка event loop при каждом запросе тайла. |
+| 26 | Нет HTTP caching для тайлов | `app.py` | Тайлы отдаются без `Cache-Control` header (кроме terrain). Браузер будет запрашивать их повторно при каждом pan/zoom. |
+| 27 | `reverseGeocode` на каждый render | `app.js` | `renderWaypointsList()` вызывает `reverseGeocode` для каждого waypoint при каждом обновлении списка. Хотя есть кэш, это лишние async операции и потенциальные race conditions с DOM. |
+
+### Medium
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 28 | Tile cache 512 entries — мало | `app.py` | При активном использовании карты 512 тайлов заполняются быстро. FIFO-вытеснение неоптимально — лучше LRU. |
+| 29 | `simplify_coords` создаёт Shapely объекты | `app.py` | Для каждого трека в тайле создаётся `LineString` + `simplify()`. На тайле с 15000 треков это дорого. Стоит кэшировать или упрощать на этапе импорта. |
+| 30 | Nominatim rate limiting | `app.js` | Nominatim имеет лимит 1 req/sec. При быстром вводе в поиск (debounce 400ms) можно получить 429. Нет обработки rate limit ответов. |
+
+### Low
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 31 | Загрузка SunCalc для авто-темы | `index.html` | Внешняя библиотека загружается всегда, даже если пользователь выбрал фиксированную тему. |
+
+---
+
+## 5. Безопасность
+
+### Critical
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 32 | XSS через Nominatim данные | `app.js` | Результаты поиска вставляются через `innerHTML` без санитизации: `<div class="wl-search-result-name">${name}</div>`. Если Nominatim вернёт `<script>` в display_name — XSS. |
+| 33 | XSS через marker name | `app.js` | `prompt()` результат используется в `innerHTML` popup через `escapeXml()` — это защищает, но `drawNamedMarker` использует `markerData.name` в popup HTML. `escapeXml` вызывается — ОК, но в `generateGPX` тоже — нужно проверить все пути. |
+
+### High
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 34 | CORS allow all origins | `app.py` | `allow_origins=["*"]` — любой сайт может делать запросы к API. Для прототипа допустимо, для продакшена — нет. |
+| 35 | Path traversal в terrain endpoint | `app.py` | `terrain_tile(layer, z, x, y)` — layer проверяется на whitelist, но z/x/y — нет валидации на отрицательные значения или `../`. `os.path.join` с отрицательным числом безопасен, но стоит добавить явную проверку. |
+| 36 | Нет rate limiting | `app.py` | API открыт без ограничений. Один клиент может забить сервер запросами к OSRM или тяжёлыми scenic-маршрутами. |
+| 37 | OSRM URL из env без валидации | `app.py` | `OSRM_URL` используется напрямую в httpx запросах. Если кто-то установит его в `http://internal-service/`, возможен SSRF. В контейнерной среде это менее критично. |
+
+### Medium
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 38 | localStorage без проверки | `app.js` | `loadMarkers()` парсит JSON из localStorage. Если данные повреждены — `try/catch` есть, но нет валидации структуры. Malformed data может вызвать runtime errors позже. |
+| 39 | Внешние CDN без SRI | `index.html` | `maplibre-gl@4.7.0` и `suncalc@1.9.0` загружаются с unpkg без `integrity` атрибута. Supply chain attack vector. |
+
+---
+
+## 6. UX/UI проблемы
+
+### High
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 40 | Нет offline-режима | — | Карта полностью зависит от сети. При потере связи (типично для эндуро) — белый экран. Нужен Service Worker + кэш тайлов. |
+| 41 | `prompt()` для имени метки | `app.js` | Нативный `prompt()` — плохой UX на мобильных, особенно iOS. Нужен кастомный inline-input. |
+| 42 | Нет undo/redo | `app.js` | Случайное удаление waypoint или метки необратимо. |
+
+### Medium
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 43 | Нет индикации ошибок сети | `app.js` | При сетевых ошибках пользователь видит только "❌ Error" без деталей и retry-кнопки. |
+| 44 | Toolbar переполнение на маленьких экранах | `app.css` | 6 кнопок в toolbar при ширине 320px — каждая ~53px. С учётом padding может быть тесно. |
+| 45 | Нет accessibility | `index.html` | Кнопки без `aria-label`, нет keyboard navigation, нет skip-links, контраст не проверен. |
+| 46 | Swipe-to-close конфликтует со scroll | `app.js` | Sheet swipe инициируется из top 50px, но `sheet-handle` маленький (36×4px). На практике пользователь может случайно закрыть sheet при скролле. |
+
+### Low
+
+| # | Проблема | Файл | Описание |
+|---|----------|------|----------|
+| 47 | Hardcoded `countrycodes=ru` | `app.js` | Поиск ограничен Россией. Если проект расширится — нужно параметризовать. |
+| 48 | Нет favicon | `index.html` | Браузер запрашивает `/favicon.ico` — 404. |
+
+---
+
+## 7. Рекомендации по улучшению
+
+### Приоритет 1 (Critical path)
+
+1. **Разбить `app.js` на модули** — минимум: `map.js`, `routing.js`, `ruler.js`, `markers.js`, `search.js`, `ui.js`, `state.js`. Использовать ES modules или Vite.
+2. **Исправить XSS** — использовать `textContent` вместо `innerHTML` для пользовательских данных, или DOMPurify.
+3. **Исправить блокировку event loop** — использовать `run_in_executor` для SQLite операций или сделать endpoints синхронными (FastAPI автоматически запустит в threadpool).
+4. **Добавить AbortController** — отменять предыдущие fetch-запросы при новых.
+
+### Приоритет 2 (High value)
+
+5. **Добавить Cache-Control для тайлов** — `Cache-Control: public, max-age=3600` для MVT тайлов.
+6. **Connection pooling** — использовать один connection per request с context manager.
+7. **Добавить SRI** для CDN-скриптов.
+8. **Service Worker** — кэширование тайлов и статики для offline.
+9. **Заменить `prompt()`** на inline-input в marker dialog.
+10. **Добавить `.gitignore`** — исключить `__pycache__`, `node_modules`, `*.pyc`, `.env`.
+
+### Приоритет 3 (Nice to have)
+
+11. **TypeScript** — постепенная миграция фронтенда.
+12. **Тесты** — pytest для API endpoints, unit tests для `calc_route_stats`, `wkb_to_coords`.
+13. **Linting** — ruff для Python, eslint для JS.
+14. **LRU cache** — заменить FIFO tile cache на `functools.lru_cache` или `cachetools.LRUCache`.
+15. **Error boundaries** — graceful degradation при недоступности OSRM.
+16. **Accessibility audit** — aria-labels, keyboard nav, focus management.
+
+---
+
+## Резюме
+
+| Категория | Critical | High | Medium | Low |
+|-----------|----------|------|--------|-----|
+| Архитектура | 1 | 3 | 3 | — |
+| Качество кода | — | 3 | 4 | 1 |
+| Баги | 2 | 3 | 3 | — |
+| Производительность | — | 4 | 3 | 1 |
+| Безопасность | 2 | 4 | 2 | — |
+| UX/UI | — | 3 | 4 | 2 |
+| **Итого** | **5** | **20** | **19** | **4** |
+
+**Вердикт:** Прототип функционально богатый и визуально качественный. Для перехода в продакшен необходимо: (1) модуляризация фронтенда, (2) исправление XSS, (3) async-safe работа с БД, (4) добавление тестов и линтеров. Текущее состояние — хороший MVP для демонстрации, но не для публичного деплоя.