24 KiB
Ревью проекта elmAI (ответы на opus-questions.md)
Ревьювер: Opus 4.8 · 31 мая 2026 · v0.35.0-dev Разбор по коду:
obd/protocol.py,brain/client.py,brain/prompts.py,api/routes.py,api/db.py,api/parser.py,web/app.py. Android-модуль (elmer-android/) в workspace отсутствует — по нему выводы на основе описаний в вопросах.
Вопрос 1. Стейт-машина ELM327: баги и крайние случаи
🔴 Стартовый буфер не сбрасывается перед командой → десинхронизация
В _exec() сразу идёт _write(cmd) без очистки входного буфера. Если предыдущая команда отвалилась по таймауту, в ОС-буфере остаются «хвосты» (часть ответа, поздний >). Следующий _read() прочитает этот мусор как ответ на новую команду и классифицирует его неверно — классическая рассинхронизация ELM327.
def _write(self, cmd: str):
self._ser.reset_input_buffer() # сбросить хвосты предыдущего ответа
self._ser.write((cmd + "\r").encode())
self._ser.flush()
logger.debug(f"AndrOBD → {cmd}")
🔴 Состояние DISCONNECTED/ERROR затирается в send()
send() проверяет только State.ERROR перед _recover(). Но BUS ERROR в _handle() ставит DISCONNECTED, а в конце send() безусловно пишет self._state = State.READY. То есть после фатальной ошибки шины машина всё равно объявляется READY, и накопленный сбой маскируется.
def send(self, cmd: str) -> str:
if self._state in (State.ERROR, State.DISCONNECTED):
self._recover()
self._state = State.BUSY
result = self._exec(cmd, self._timing.ms)
# НЕ ставить READY безусловно — _exec мог уйти в ERROR
if self._state == State.BUSY:
self._state = State.READY
return result
🟡 > посреди мусора (вопрос 1.2)
_read() возвращает всё накопленное до первого >. Если ELM прислал SEARCHING... затем данные затем >, всё склеится в одну строку через \n, а Rsp.identify() смотрит только на начало (startswith) — реальные данные после SEARCHING будут потеряны/неверно классифицированы. AndrOBD обрабатывает каждую строку отдельно. Рекомендация: классифицировать построчно, а не всю склейку.
🟢 Бесконечный цикл в _exec() (вопрос 1.5)
Цикл жёстко ограничен range(10), по выходу — State.ERROR и return "". Бесконечного цикла нет. Но обратите внимание: при инициализации шаг t += 1000 за 10 итераций даёт суммарно до ~55с ожидания на одну команду — для INIT_TMO=10000 это может неприятно затянуть init().
🟡 Восстановление после BUS ERROR (вопрос 1.3)
Логика ATPC → ATSP0 корректна по сути, но ответы на них читаются _try_read() и молча выбрасываются. Если ATSP0 не подтвердился (ELM завис), машина об этом не узнает и пойдёт слать команды в неинициализированный протокол. Желательно проверять, что на ATSP0 пришёл OK/>, иначе — полный reset (ATZ).
🟡 Поллинг 1мс (вопрос 1.4)
1мс time.sleep в Python реально даёт ~1–15мс из-за гранулярности планировщика — на практике это не вредит (ELM медленнее), но и «честных» 1мс там нет. На быстрых ELM327 v1.5/v2.1 это не узкое место; узкое место — таймаут адаптива, а не поллинг. Менять не нужно.
Race conditions
В Python-версии всё однопоточное — гонок нет, пока один экземпляр AndrOBD не шарится между потоками. Если планируется параллельный доступ — добавьте threading.Lock вокруг send().
Вопрос 2. HTTP 499 при upload с мобильной сети
🔴 Нет идемпотентности → дубликаты при ретрае (вопрос 2.5)
Это главная проблема. Сценарий 499: сервер уже принял и обработал запрос (LLM-анализ 30–120с), но клиент отвалился по read timeout и шлёт ретрай. Результат — вторая полная LLM-сессия и вторая запись в sessions. upload_session() не имеет ключа идемпотентности.
Решение — клиент генерирует request_id (UUID), сервер кэширует результат:
data = request.get_json(silent=True)
req_id = data.get("request_id")
if req_id:
cached = db.get_session_by_request_id(req_id) # + колонка request_id UNIQUE
if cached:
return jsonify(cached["response_json"]), 200
🟡 Стратегия ретраев — нужен backoff и идемпотентность
3 ретрая с фиксированной задержкой 2с на мобильной сети мало помогают: если причина — долгий LLM-ответ (>read timeout 180с), то все 3 попытки упрутся в тот же таймаут и каждая запустит новый LLM-прогон. Рекомендация: exponential backoff (2/4/8с + jitter) и обязательно идемпотентность (см. выше), иначе ретраи только множат нагрузку.
🟡 Корень 499 — рассинхрон таймаутов клиент/сервер (вопрос 2.1)
Клиентский read 180с ≈ gunicorn timeout 180с. При длинном ответе LLM (Diagnoser.timeout=120, но сам upload может суммарно дольше) клиент рвёт соединение ровно в момент, когда сервер ещё пишет ответ. Прочие частые причины 499 на мобильной: смена сети Wi-Fi↔LTE (новый IP, старый сокет мёртв), NAT-таймаут оператора (часто 30–60с тишины), Doze/засыпание приложения. Рекомендация: клиентский read timeout должен быть строго больше серверного (например, 240с против gunicorn 180с), а сервер — отвечать быстрее (streaming, см. ниже).
🟡 Write timeout на медленной сети (вопрос 2.3)
Да, при толстом батче (raw_responses целиком) и слабом upload на LTE write timeout 60с реально достижим. Тело JSON со всеми сырыми ответами может быть десятки–сотни КБ.
🟡 Чанки/сжатие (вопрос 2.4)
Чанкинг избыточен для типичного объёма, а вот gzip тела даст быстрый выигрыш (JSON сжимается в 5–10 раз) и снимет риск write timeout:
// OkHttp: добавить gzip-обёртку RequestBody + заголовок
.header("Content-Encoding", "gzip")
Сервер: nginx сам разожмёт при наличии gunzip/decompression, либо Flask с request.get_data() + gzip.decompress. Это дешевле, чем переписывать на чанки.
Главная архитектурная рекомендация
Разделите «приём данных» и «LLM-анализ». Эндпоинт должен быстро (1–2с) принять батч, сохранить, вернуть session_id, а диагноз отдавать отдельным polling-эндпоинтом (GET /api/v1/session/<id>/result) или через streaming. Тогда 499 из-за долгого LLM исчезнет как класс.
Вопрос 3. Архитектура: три модуля + Android пакеты
🟢 Границы модулей в целом чистые
obd/ ничего не знает про brain/ и api/; brain/ — изолированный LLM-клиент; api/ оркестрирует. Направление зависимостей web → api → brain/obd корректное (вопрос 3.4 — да, правильное).
🟡 Импорты внутри функций (вопрос 3.2)
В routes.py все from brain.client import Diagnoser, from api.db import Database, from api.config import load сделаны внутри обработчиков. Это не «нормально», а компромисс — обычно так лечат циклические импорты или ускоряют старт. Минусы: load() читает конфиг с диска на каждый запрос, импорт-резолвинг повторяется. Рекомендация: поднять импорты на уровень модуля, а конфиг закэшировать:
# api/config.py
from functools import lru_cache
@lru_cache(maxsize=1)
def load(): ...
Если поднятие импортов ломает цикл — это сигнал, что цикл надо разорвать явно, а не прятать.
🟡 config.yaml (вопрос 3.3)
Конфиг сейчас грузится через api/config.py. Держать config.yaml в корне проекта (рядом с web/app.py) логичнее — он общий для api/, brain/, obd/, а не принадлежит только api/. Вынесите на верхний уровень, путь резолвьте от корня.
🟢 Заменяемость модулей (вопрос 3.5)
brain/ заменяется на локальный LLM тривиально — он зависит только от OpenAI-совместимого HTTP (/chat/completions). Достаточно сменить base_url/model в конфиге; код менять не нужно. obd/ тоже изолирован. Это хороший знак для дизайна.
Вопрос 4. SQL-схема: таблица sessions
🔴 Утечка соединений + конкурентный доступ (вопрос 4.5)
Database() создаётся в каждом запросе, открывает sqlite3.connect(...) и никогда не закрывается — connection leak. При 4 gunicorn-воркерах одновременные записи в один файл дают database is locked (SQLite по умолчанию: 1 писатель, нет ожидания). Минимум:
self.conn = sqlite3.connect(str(self.path), timeout=30, check_same_thread=False)
self.conn.execute("PRAGMA journal_mode=WAL") # параллельные читатели + 1 писатель
self.conn.execute("PRAGMA busy_timeout=30000")
И закрывать соединение (контекстный менеджер / try/finally / db.close()), либо держать один пул на воркер. WAL критичен для multi-worker.
🟡 30+ колонок в одной таблице (вопрос 4.1)
Для SQLite это нормально (лимит 2000 колонок), денормализация под аналитику оправдана. Но смешаны три логических домена: телефон, ELM, LLM. Это не баг, а запах. Пока таблица аналитическая (одна запись = одна сессия) — оставьте; если начнёте часто менять набор полей телефона/ELM — выносите в отдельные таблицы или JSON-колонку.
🟢 raw_responses как JSON TEXT (вопрос 4.2)
Ок для SQLite. При необходимости запросов внутрь — используйте json_extract() (есть в SQLite ≥3.38). Менять не нужно.
🟡 Индексы (вопрос 4.3)
created_at, vin, elm_mac, android_id — разумный набор. Но vin nullable и часто NULL — индекс будет «разреженным», это норм. Добавьте составной (android_id, created_at) если будете строить историю по устройству — иначе текущих достаточно.
🟡 Мёртвые таблицы (вопрос 4.4)
cars, diagnostic_tokens, llm_messages, ecu_parameters, dtc_codes создаются в _init_schema(), имеют методы-обёртки в db.py, но в текущем пути upload/chat не используются. Это «второй контур», который вводит в заблуждение (например, история диалога в /chat идёт из клиента, а не из llm_messages). Решение: либо подключите их (тогда /chat сможет хранить историю на сервере по VIN), либо удалите вместе с методами. Сейчас они — технический долг и риск рассинхрона схемы.
🟢 Инъекции
Все запросы параметризованы (?), SQL-инъекций нет.
Вопрос 5. LLM-интеграция: промпты и таймауты
🔴 Рассинхрон модели и таймаута в коде
brain/client.py:DEFAULT_MODEL = "gpt-oss-20b", а конфиг/доки —gpt-oss-120b. Дефолт-fallback тихо подменит модель, если конфиг недокинулmodel.Diagnoser.ask(... timeout=120), но в вопросе и nginx/gunicorn заявлено 180с. Таймаут захардкожен и не берётся из конфига.- Докстринги и комментарии говорят «DeepSeek», хотя API —
api.aillm.ru/ gpt-oss. Чисто косметика, но путает.
def __init__(self, api_key, model="gpt-oss-120b", base_url=DEFAULT_BASE, timeout=180):
...
self.timeout = timeout
def ask(self, messages):
resp = requests.post(..., timeout=self.timeout)
🟡 Нет streaming + heartbeat (вопрос 5.2)
requests.post без stream=True на 120–180с — это «чёрный ящик»: клиент не видит прогресса и рвёт по таймауту (см. Вопрос 2). Для длинной генерации лучше streaming (SSE) с проксированием токенов клиенту — тогда соединение «живое», NAT не закрывает, 499 пропадает. Минимум — heartbeat-байты каждые N секунд.
🟡 История диалога в /chat (вопрос 5.3)
История склеивается в один user-prompt строкой («Водитель: …/Автоэксперт: …»), а не передаётся как полноценный массив messages с ролями. Модель хуже держит контекст, и при длинной истории (даже срезанной до 10) промпт может раздуться. Лучше передавать историю настоящими role: user/assistant сообщениями (метод diagnose это уже умеет через history!) и считать токены, а не сообщения:
hist_msgs = [{"role": m["role"], "content": m["content"]} for m in history[-10:]]
answer = diagnoser.diagnose(SYSTEM_CHAT, question, history=hist_msgs)
Переполнения контекста сейчас никто не контролирует — добавьте бюджет по токенам.
🟡 Обработка ошибок LLM (вопрос 5.5)
Сейчас один общий except Exception → строка «LLM недоступен: {e}». Нет различия rate limit (429, нужен retry-after), timeout (нужен ретрай), 5xx (ретрай) vs 4xx (не ретраить). И текст исключения уходит прямо в ответ пользователю — может протечь URL/детали. Разделите коды:
try:
...
except requests.Timeout: # ретрай
except requests.HTTPError as e:
if e.response.status_code == 429: ... # backoff по Retry-After
🟢 Промпт для диагностики (вопрос 5.1)
SYSTEM_PROMPT сильный: 10 правил, явный формат с таблицами, проценты уверенности, «проверь перед заменой», секция «если не поможет». Это хорошо. Чего не хватает: (1) данных об авто (make/model/year/engine почти всегда отсутствуют — VIN есть, но не расшифровывается), (2) пробег/условия, (3) явного запрета галлюцинировать значения PID, которых нет в данных. Добавьте расшифровку VIN→марка/год (хотя бы WMI) перед отправкой — резко поднимет качество.
🟡 Выбор gpt-oss-120b (вопрос 5.4)
Для авто-диагностики ключевое — знание DTC и инженерная логика. 120b разумен как баланс цена/качество. Альтернативы под задачу: Qwen2.5-72B/Qwen3 (хорош в технике, но у вас отмечен CoT-leak баг на fp8-варианте), DeepSeek-V3 (сильная техничка), либо рассуждающая модель (o-серия/R1) для сложных взаимосвязей — но они дороже и медленнее, что усугубит проблему таймаутов из Вопроса 2. Вывод: 120b ок, менять стоит только если качество разбора DTC не устраивает.
Вопрос 6. Безопасность API
🔴 Любой эндпоинт без аутентификации → бесплатный прокси к платному LLM (вопросы 6.1, 6.4)
/api/v1/chat, /api/v1/session/upload, /api/v1/ping-llm дёргают платный LLM без какой-либо аутентификации и без rate limit. Любой, кто узнал домен, может в цикле слать /chat и жечь ваш токен api.aillm.ru, а /ping-llm вообще тратит LLM-вызов на каждый GET. Для MVP HTTPS защищает только канал, но не от абуза. Минимум:
- статический API-ключ приложения в заголовке (да, его можно вытащить из APK, но он отсекает массовый скан-абуз);
- rate limiting на nginx (
limit_req_zone) и/или Flask-Limiter по IP/android_id; /ping-llmне должен реально вызывать LLM на каждый пинг — кэшируйте результат на 1–5 мин.
limit_req_zone $binary_remote_addr zone=api:10m rate=10r/m;
location /api/v1/chat { limit_req zone=api burst=5 nodelay; ... }
🟡 XSS через diagnosis/raw (вопрос 6.3 — не инъекция, а отображение)
SQL-инъекции через raw_responses нет (запросы параметризованы). Но: ответ ELM327 и текст диагноза от LLM (markdown с таблицами) где-то рендерятся в вебе (web/templates/index.html, дашборд сессий). Если markdown/HTML вставляется без экранирования — это stored XSS: вредонос в raw ELM или в ответе LLM выполнится в браузере админа. Проверьте, что вывод экранируется (Jinja autoescape по умолчанию вкл — не отключайте |safe на этих полях; markdown рендерьте через санитайзер).
🟡 Утечка деталей в ответах
except ... return f"LLM недоступен: {e}" и error: str(e)[:100] отдают внутренние сообщения наружу. Логируйте полностью, клиенту — обобщённый текст.
🟡 API-ключ LLM (вопрос 6.5)
Текущая схема (ключ только на сервере, не в APK) — правильная, это лучшее в безопасности проекта. Дополнительный прокси не нужен; достаточно закрыть абуз (rate limit + ключ приложения), чтобы вашим серверным ключом не пользовались чужие.
Сводка по безопасности
| Мера | Приоритет | Статус |
|---|---|---|
| Rate limiting (nginx/Flask-Limiter) | 🔴 высокий | нет |
| Ключ приложения в заголовке | 🟡 средний | нет |
/ping-llm без реального LLM-вызова |
🟡 средний | вызывает LLM |
| Экранирование diagnosis/raw в вебе | 🟡 средний | проверить |
| Не отдавать текст исключений клиенту | 🟡 средний | отдаёт |
| Ключ LLM только на сервере | 🟢 | сделано |
Итоговый топ проблем (по убыванию важности)
- 🔴 Нет идемпотентности upload → дубликаты сессий и двойной расход LLM при 499/ретраях (Q2).
- 🔴 Открытые LLM-эндпоинты без auth/rate-limit → абуз платного токена (Q6).
- 🔴 SQLite: утечка соединений + нет WAL/busy_timeout при 4 воркерах →
database is locked(Q4). - 🔴 Долгий синхронный LLM в запросе — корень 499; разделить приём данных и анализ, добавить streaming (Q2, Q5).
- 🔴
reset_input_bufferперед командой в стейт-машине — иначе десинхрон ELM327 (Q1). - 🟡 Рассинхрон модели/таймаута в
client.py(20b vs 120b, 120с vs 180с) (Q5). - 🟡 История диалога
/chatстрокой вместо ролейmessages(Q5). - 🟡 Мёртвые таблицы в схеме — подключить или удалить (Q4).
Что уже хорошо 🟢
- Чистые границы модулей, заменяемый
brain/. - Сильный диагностический системный промпт.
- Параметризованный SQL (нет инъекций).
- Ключ LLM не в APK.
- Стейт-машина ограничена по ретраям (нет бесконечных циклов).