From b40995f2d63a4a45fb05fb8b68da889d75b6daf0 Mon Sep 17 00:00:00 2001 From: Stream Date: Thu, 14 May 2026 23:30:01 +0300 Subject: [PATCH] auto-sync: 2026-05-14 23:30:01 --- tasks/enduro-trails/CODE_REVIEW.md | 219 +++++++++++++++++++++++++++++ 1 file changed, 219 insertions(+) create mode 100644 tasks/enduro-trails/CODE_REVIEW.md diff --git a/tasks/enduro-trails/CODE_REVIEW.md b/tasks/enduro-trails/CODE_REVIEW.md new file mode 100644 index 0000000..0c2fdc6 --- /dev/null +++ b/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` без санитизации: `
${name}
`. Если Nominatim вернёт `