From 21e5473045330f550032734fc805cad1fcd89013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CNaeel=E2=80=9D?= Date: Tue, 14 Jul 2026 22:19:57 +0400 Subject: [PATCH] =?UTF-8?q?docs:=20Sonnet=20review=20of=20migration=20plan?= =?UTF-8?q?=20=E2=80=94=202=20critical,=203=20important,=205=20improvement?= =?UTF-8?q?s=20+=20my=20analysis?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...sonnet-review-migration-plan-2026-07-14.md | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 History/sonnet-review-migration-plan-2026-07-14.md diff --git a/History/sonnet-review-migration-plan-2026-07-14.md b/History/sonnet-review-migration-plan-2026-07-14.md new file mode 100644 index 0000000..bd350fc --- /dev/null +++ b/History/sonnet-review-migration-plan-2026-07-14.md @@ -0,0 +1,132 @@ +# Sonnet Review: план миграции ВМ → Flask — анализ и ответ + +**Дата:** 2026-07-14 +**Источник:** Sonnet (Claude) — ревью `History/migration-vm-to-flask-plan-2026-07-14.md` + +--- + +## Резюме Соннета + +План **одобрен архитектурно**. Критичных блокеров — 2, важных дополнений — 3, улучшений — 5. + +--- + +## Критичные проблемы (Соннет) + +### 1. `site/` — конфликт с Python stdlib ⛔ + +> `from site.db import ...` — `site` это встроенный модуль Python. При запуске не из корня Python найдёт встроенный `site`, а не локальный. + +**Моё мнение:** ✅ **Согласен полностью.** Это реальная проблема. `site` — built-in модуль Python, импортируется при старте интерпретатора. Если `PYTHONPATH` или `sys.path` поставит корень раньше чем `site-packages` — получим наш пакет. Если наоборот — stdlib. Нестабильно. + +**Решение:** переименовать `site/` → `app/` или `contracts_app/`. Я за `app/` — короче, семантически понятно (Flask application factory), не конфликтует ни с чем. + +### 2. `/api/cleanup` — нулевая аутентификация ⛔ + +> Любой может POST /api/cleanup → потеря всех данных. + +**Моё мнение:** ✅ **Согласен.** Но с нюансом. Текущий `convert_server.py` тоже имеет этот эндпоинт без auth — и он на внешнем домене `contracts.kube5s.ru`. Так что это не регресс, а существующая дыра. + +**Решение:** +- Минимум: `X-Api-Key` в заголовке, сверка с `os.environ["API_KEY"]` +- Средний: nginx `allow 127.0.0.1; deny all;` на этот location (если cleanup вызывается только с самого Flask-контейнера) +- Правильный: убрать cleanup вообще, заменить на автоочистку по TTL (cron/APScheduler) + +Но это **отдельная задача**, не часть миграции. В рамках миграции — просто не ухудшить ситуацию. + +--- + +## Важные дополнения (Соннет) + +### 3. Файловый lock для classify — хрупкий + +> `/tmp/classify_{batch_id}.lock` — если Flask упадёт, lock остаётся навсегда. + +**Моё мнение:** ✅ **Согласен.** Lock-файл — антипаттерн в stateless-приложении. Сейчас в `convert_server.py` та же схема (subprocess + lock-файл), и она работает только потому что ВМ не рестартит. + +**Решение:** in-memory `dict[batch_id] → thread` + проверка `thread.is_alive()`. При старте Flask никаких lock-файлов нет. Лучше: store `classify_status='processing'` в БД и сбрасывать `processing → pending` при старте приложения. + +### 4. SSE — нет heartbeat + +> При медленном LLM nginx и браузеры режут соединение. + +**Моё мнение:** ✅ **Согласен.** Добавить `yield ": heartbeat\n\n"` раз в 15 секунд. SSE-комментарий (строка начинается с `:`) игнорируется EventSource, но сбрасывает таймауты nginx/браузера. + +### 5. Старые proxy-роуты — явно не сказано удалить + +> В плане написано «создать новый app.py», но не сказано явно «удалить старые /api/* proxy-роуты». + +**Моё мнение:** ✅ **Согласен, но это очевидно.** Новый `app.py` полностью заменяет старый — старых роутов `/chat`, `/api/prompts` (proxy на ВМ) не будет, потому что логика теперь локальная. Но в плане стоит написать явно: «удалить ВСЕ proxy-роуты, они больше не нужны». + +--- + +## Улучшения (Соннет) + +### 6. `prompts_bp.py` и `pages_bp.py` — не раскрыты + +**Моё мнение:** ✅ **Справедливо.** Но они тривиальны: `prompts_bp.py` = CRUD по таблице `prompts` (4 ручки), `pages_bp.py` = `render_template("index.html")` + `render_template("architect.html")`. Не расписывал детально потому что там нечего расписывать. Можно добавить один абзац для полноты. + +### 7. `drhider/` — не упоминается + +**Моё мнение:** ⚠️ **Справедливо, но осознанно.** `drhider/` — это отдельный сервис (обфускация персональных данных), не часть пайплайна сверки договоров. Он уже работает на Flask в `site/services/drhider.py` и `site/routes/drhider_bp.py`. В план миграции сверки он не входит — это другой сервис на том же хосте. Но стоит упомянуть это явно: «drhider не трогаем, он уже на Flask». + +### 8. `repository.py` — не объяснено как расширить + +**Моё мнение:** ⚠️ **Справедливо, но слишком рано.** Protocol-паттерн в `repository.py` уже есть, `PgRepository` частично реализован. Полное внедрение DI через репозиторий во все services — это Фаза 2, после того как базовая миграция заработает. Сейчас services используют `from site.db import documents` напрямую, и это ОК для первого шага. Переписывать всё на DI сразу = риск сломать работающий код. + +**План:** после миграции — отдельная задача «внедрить Repository DI во все services». + +### 9. Graceful shutdown — in-flight classify потоки + +**Моё мнение:** ✅ **Справедливо.** При `docker stop` daemon-потоки убиваются, документы застревают в `classify_status='processing'`. + +**Решение (уже в коде ВМ!):** `db/documents.reset_classify_status(batch_id)` сбрасывает `processing → pending`. Добавить вызов в `app.py` при старте: + +```python +# При старте: сбросить все застрявшие processing → pending +from site.db.connection import execute +execute("UPDATE documents SET classify_status='pending', error_message=NULL WHERE classify_status='processing'") +``` + +### 10. Тесты — только curl, нет pytest + +**Моё мнение:** ✅ **Справедливо.** В репо уже есть `tests/` с `conftest.py`. Нужен шаг «адаптировать тесты под `app.test_client()`». Но это **не блокер для миграции** — тесты можно дописать после. + +--- + +## Моя итоговая оценка + +| Категория | Пунктов | Критичность | +|-----------|---------|-------------| +| Критичные (надо исправить ДО миграции) | 2 | `site/` → `app/`, auth на cleanup | +| Важные (надо добавить в план) | 3 | lock→in-memory, heartbeat, явное удаление proxy | +| Улучшения (можно после миграции) | 5 | prompts/pages детали, drhider, repository DI, graceful, тесты | + +### Что делаем ДО начала миграции: + +1. **Переименовать `site/` → `app/`** — везде в плане, во всех импортах +2. **Добавить auth на `/api/cleanup`** — `X-Api-Key` или `require_local` + +### Что добавляем в план: + +3. **in-memory lock** для classify вместо файлового +4. **SSE heartbeat** раз в 15 сек +5. **Упомянуть явно:** старые proxy-роуты удалить, drhider не трогать +6. **Graceful shutdown:** сброс `processing → pending` при старте +7. **prompts_bp / pages_bp** — один абзац что там +8. **repository DI** — отложить на пост-миграцию + +### Что НЕ блокер: + +- `drhider` — уже на Flask, не часть миграции +- Repository DI — Фаза 2 +- pytest — после миграции +- prompts/pages — тривиальны + +--- + +## Вердикт + +План **рабочий**. Соннет подтвердил архитектурные решения (blueprints, разделение db/services/routes, генератор для SSE, DI). Критичные замечания — реальные и требуют правки плана. Остальное — улучшения надёжности, которые можно добавить в план сейчас или отложить. + +**Готовность к миграции:** 90%. После правки 2 критичных пунктов → можно начинать.