232 lines
17 KiB
Markdown
232 lines
17 KiB
Markdown
# Промпт для Соннета: code review только собственно сверки
|
||
|
||
Нужно провести строгий code review **только той части системы, которая выполняет собственно сверку документов после формирования группы**.
|
||
|
||
Не анализируй и не ревьюируй upload, WebDAV/VM upload, ZIP-распаковку, выбор папки, дедупликацию файлов, парсинг DOC/DOCX, garbage-фильтры, классификацию документов, определение типа документа, matching/grouping документов и UI загрузки. Эти части находятся вне области данного ревью.
|
||
|
||
## Контекст
|
||
|
||
Система получает уже сформированную группу договора и связанных документов. Затем она должна:
|
||
|
||
1. определить порядок документов группы;
|
||
2. получить текущую спецификацию;
|
||
3. передать текущую спецификацию и текст очередного документа в LLM;
|
||
4. получить операции `ADD`, `UPDATE`, `DELETE`, `UNRESOLVED` или режим `full_replace`;
|
||
5. корректно транслировать ссылки LLM на строки текущей спецификации;
|
||
6. применить операции к БД и сохранить историю событий;
|
||
7. отдать прогресс и результат через SSE;
|
||
8. показать пользователю фактический итог сверки.
|
||
|
||
Особенно проверь, что система не выдаёт успешный результат, если часть операций или документов фактически не обработана.
|
||
|
||
## Файлы для обязательного изучения
|
||
|
||
Изучи только эти файлы и их прямые зависимости, необходимые для понимания сверки:
|
||
|
||
1. `contracts-flask/site/services/process.py`
|
||
- основной pipeline сверки;
|
||
- порядок документов;
|
||
- получение текущей спецификации;
|
||
- подготовка текста документа;
|
||
- вызов LLM;
|
||
- трансляция `target_id` в `target_hash`;
|
||
- обработка `full_replace`;
|
||
- применение операций;
|
||
- арифметическая проверка;
|
||
- SSE-события логического pipeline.
|
||
|
||
2. `contracts-flask/site/routes/pipeline_bp.py`
|
||
- endpoint `GET /process-v2`;
|
||
- генерация SSE;
|
||
- heartbeat;
|
||
- закрытие/обрыв соединения;
|
||
- преобразование исключений в SSE-события;
|
||
- различие HTTP-ошибки до начала стрима и ошибки внутри стрима.
|
||
|
||
3. `contracts-flask/site/static/compare.js`
|
||
- `startCompareSSE()`;
|
||
- обработка `extract_start`, `llm_done`, `applied`, `extract_error`, `apply_error`, `done`, `error`;
|
||
- обработка `EventSource.onerror`;
|
||
- закрытие EventSource;
|
||
- соответствие frontend-событий backend-событиям.
|
||
|
||
4. `contracts-flask/site/static/app.js`
|
||
- функции запуска сверки группы;
|
||
- вызов `/api/apply-groups`;
|
||
- получение `contract_id`;
|
||
- запуск `/process-v2`;
|
||
- обработка success/error/connection error;
|
||
- изменение состояния группы после завершения.
|
||
|
||
5. `contracts-flask/site/services/llm.py`
|
||
- фактический вызов LLM для сверки;
|
||
- формат prompt и ответа;
|
||
- timeout/retry;
|
||
- обработка невалидного ответа;
|
||
- возможная потеря или искажение операций.
|
||
|
||
6. `contracts-flask/site/llm_prompt.py`
|
||
- prompt сверки;
|
||
- формат текущей спецификации;
|
||
- формат ожидаемых операций;
|
||
- ограничения для `ADD`, `UPDATE`, `DELETE`, `UNRESOLVED`, `full_replace`.
|
||
|
||
7. `contracts-flask/site/db/spec_events.py`
|
||
- `reset()`;
|
||
- `clear_current()`;
|
||
- `apply_ops()`;
|
||
- транзакции и атомарность;
|
||
- сохранение истории;
|
||
- статусы применённых и неразрешённых операций;
|
||
- поведение при частичной ошибке.
|
||
|
||
8. `contracts-flask/site/db/spec_current.py`
|
||
- получение текущих строк спецификации;
|
||
- идентификаторы и `name_hash`;
|
||
- получение `elements_json`;
|
||
- согласованность данных между текущим состоянием и историей.
|
||
|
||
9. `contracts-flask/site/db/supplements.py`
|
||
- только функции, используемые `process.py` для получения документов уже сформированной группы;
|
||
- порядок и фильтрация документов;
|
||
- отсутствие/дублирование документов.
|
||
|
||
10. `contracts-flask/site/services/metrics.py`
|
||
- `check_arithmetic()`;
|
||
- что именно проверяется;
|
||
- может ли ошибка арифметики повлиять на результат;
|
||
- почему проверка не должна маскировать ошибку применения.
|
||
|
||
## Тесты для обязательного изучения
|
||
|
||
1. `contracts-flask/tests/test_process_pipeline.py`
|
||
- какие сценарии реально покрыты;
|
||
- корректность `full_replace`;
|
||
- корректность трансляции `target_id`;
|
||
- отсутствие тестов на реальные SSE и ошибки LLM.
|
||
|
||
2. `contracts-flask/tests/test_grouping.py`
|
||
- только часть, необходимая для понимания структуры группы, передаваемой в `apply_groups`.
|
||
|
||
3. `contracts-flask/tests/test_metrics.py`
|
||
- только тесты арифметической проверки, относящиеся к операциям сверки.
|
||
|
||
4. Найди все остальные тесты, которые напрямую вызывают `run_pipeline`, `apply_ops`, `process-v2` или `startCompareSSE`, и включи их в анализ только если они действительно относятся к собственно сверке.
|
||
|
||
## Что проверять
|
||
|
||
### 1. Корректность алгоритма сверки
|
||
|
||
- Не теряется ли первая спецификация или базовое состояние.
|
||
- Правильно ли строится `current_spec` перед каждым следующим документом.
|
||
- Гарантирован ли правильный порядок обработки документов.
|
||
- Не зависит ли порядок от нестабильного `created_at` или формата даты.
|
||
- Корректно ли работает переход между несколькими документами группы.
|
||
- Не дублируются ли строки при повторном запуске.
|
||
- Корректно ли работает `full_replace` при уже существующих строках.
|
||
- Может ли `full_replace` удалить корректные данные при ошибочном/неполном ответе LLM.
|
||
|
||
### 2. Операции LLM
|
||
|
||
- Как `target_id` переводится в фактический идентификатор строки.
|
||
- Что происходит при `r0`, `r999`, `rX`, отсутствующем `target_id`, неверном `target_hash`.
|
||
- Не может ли LLM обновить не ту строку из-за изменения порядка строк.
|
||
- Что происходит с неизвестными действиями.
|
||
- Что происходит с отсутствующими полями `name`, `price`, `qty`, `sum`, `date_start`.
|
||
- Не приводит ли частично валидная операция к тихой потере данных.
|
||
- Сохраняется ли исходный ответ LLM и prompt для аудита.
|
||
- Как обрабатываются пустой, обрезанный, markdown-обёрнутый или частично повреждённый JSON-ответ.
|
||
|
||
### 3. БД, транзакции и атомарность
|
||
|
||
- Атомарно ли применяются операции одного документа.
|
||
- Что происходит, если пятая операция из десяти падает.
|
||
- Может ли current state измениться, а event history не сохраниться, или наоборот.
|
||
- Не приводит ли `reset()` к потере истории при повторном запуске.
|
||
- Различаются ли `applied`, `unresolved`, `failed` и действительно ли эти статусы отражают результат.
|
||
- Безопасен ли параллельный запуск двух сравнений одного договора.
|
||
- Безопасен ли одновременный запуск сравнений разных групп.
|
||
- Есть ли race condition между `apply-groups`, `/process-v2` и frontend state.
|
||
|
||
### 4. SSE и сетевые ошибки
|
||
|
||
- Совпадает ли событие завершения backend (`complete` или `done`) с событием, которое ожидает frontend.
|
||
- Может ли frontend навсегда оставить сравнение в состоянии `⏳`.
|
||
- Что происходит при disconnect после `applied`, но до события завершения.
|
||
- Что происходит при heartbeat без данных.
|
||
- Может ли `EventSource.onerror` ошибочно объявить ошибку после нормального закрытия.
|
||
- Показывает ли UI частичный результат как полный.
|
||
- Есть ли таймаут на клиенте и сервере.
|
||
- Возвращается ли пользователю причина ошибки LLM/API, а не только «Ошибка соединения».
|
||
- Не теряются ли последние SSE-события из-за proxy buffering.
|
||
|
||
### 5. Числовая и предметная корректность
|
||
|
||
- Проверяется ли арифметика `price * qty = sum` до применения и после применения.
|
||
- Как обрабатываются `None`, строки с запятой, целые/дробные числа, отрицательные значения и большие числа.
|
||
- Не изменяет ли арифметическая проверка данные или только логирует ошибку.
|
||
- Может ли LLM вернуть арифметически неверную операцию, которая всё равно попадёт в current state.
|
||
- Сохраняется ли точность Decimal/float.
|
||
- Как обрабатываются даты и отсутствие даты.
|
||
|
||
### 6. Повторяемость и идемпотентность
|
||
|
||
- Что произойдёт при повторном нажатии «Сравнить эту группу».
|
||
- Можно ли безопасно повторить сравнение после сетевого обрыва.
|
||
- Будут ли повторно добавлены те же строки.
|
||
- Как отделяется новый запуск от предыдущей истории.
|
||
- Есть ли идентификатор запуска и защита от повторного применения одного ответа.
|
||
|
||
## Обязательные вопросы от ревьюера
|
||
|
||
Ответь отдельно на следующие вопросы, даже если для ответа придётся изучить прямую зависимость:
|
||
|
||
1. Почему в реальном тесте один допник смог попасть в группу без базового договора, и может ли собственно pipeline сверки безопасно обработать такую неполную группу?
|
||
2. При каком именно условии `run_pipeline()` считает сверку успешной?
|
||
3. Может ли pipeline отправить `complete`, если один документ дал `extract_error` или часть операций не применилась?
|
||
4. Почему backend использует событие `complete`, а frontend-код может ожидать `done`? Это реальный баг или только устаревший комментарий/другая ветка?
|
||
5. Если LLM вернул `full_replace` с пустым `ops`, будет ли текущая спецификация очищена? Должно ли так происходить?
|
||
6. Если `apply_ops()` применил только часть операций, как это отражается в SSE и UI?
|
||
7. Есть ли транзакция, гарантирующая согласованность `spec_current` и `spec_events`?
|
||
8. Можно ли повторно запустить `/process-v2` для того же `contract_id` без дублирования или повреждения результата?
|
||
9. Что происходит при одновременном сравнении двух групп, относящихся к одному договору?
|
||
10. Как система отличает «LLM вернул корректный пустой результат» от «LLM вернул повреждённый/неполный ответ»?
|
||
11. Какие операции считаются `UNRESOLVED`, где они сохраняются и видит ли их пользователь?
|
||
12. Может ли `check_arithmetic()` обнаружить ошибку, но pipeline всё равно завершиться успешно?
|
||
13. Какой минимальный набор интеграционных тестов нужен для доказательства корректности реальной сверки?
|
||
14. Какие риски остаются именно в собственно сверке после исключения upload/classify/grouping из области анализа?
|
||
|
||
## Формат отчёта
|
||
|
||
Пиши отчёт в формате code review.
|
||
|
||
Сначала findings, отсортированные по серьёзности:
|
||
|
||
- `BLOCKER` — возможна потеря/порча данных или ложный успешный результат сверки;
|
||
- `HIGH` — неверное применение операций, нарушение атомарности, повторное применение или потеря результата;
|
||
- `MEDIUM` — ошибочное отображение статуса, частичный результат, нестабильность или отсутствие важной защиты;
|
||
- `LOW` — локальная проблема качества, диагностики или сопровождаемости.
|
||
|
||
Для каждого finding укажи:
|
||
|
||
- severity;
|
||
- файл и конкретный символ/участок кода;
|
||
- точную последовательность, которая приводит к проблеме;
|
||
- воспроизводимый сценарий;
|
||
- фактический ущерб;
|
||
- минимальное исправление;
|
||
- обязательный тест.
|
||
|
||
Не предлагай изменения вне области собственно сверки. Не исправляй код самостоятельно. Не делай общий обзор всего проекта. Если findings нет, напиши это явно и перечисли оставшиеся пробелы тестирования.
|
||
|
||
## Жёсткое ограничение по стоимости
|
||
|
||
Не пиши код, патчи, diff и готовые реализации. Не изменяй файлы и не выполняй команды. Твоя задача — только code review: факты, findings, доказательства, вопросы и минимальные рекомендации словами. Любые предлагаемые исправления описывай концептуально, без реализации.
|
||
|
||
В конце добавь:
|
||
|
||
1. ответы на 14 обязательных вопросов;
|
||
2. таблицу покрытия тестами;
|
||
3. минимальный план исправлений, если они нужны;
|
||
4. список файлов, которые действительно были изучены.
|