wiki/tasks/enduro-trails/CODE_REVIEW.md

# 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 для демонстрации, но не для публичного деплоя.