docs: review document for #554

Issue: #554
User-Visible: no

docs/reviews/SPEC-REVIEW-554-r1.md

@@ -0,0 +1,180 @@
+# SPEC-REVIEW-554-r1
+
+## Скоуп
+
+Issue #554 (лёгкий трек, `small`, `bug`, `P3`): при attachment upload
+low-disk guard (`check_quota` в `custom_components/houseplan/plans.py`)
+повторно вычитает из `disk_usage().free` размер staging-файла, который
+`/api/houseplan/upload` уже полностью записал на диск до вызова guard'а.
+Promotion — `os.replace()` в пределах одного `files_root`, второй копии не
+требуется, поэтому около порога `MIN_FREE_BYTES = 512 MiB` валидная загрузка
+получает ложный `low_disk_space`.
+
+ТЗ живёт в теле issue #554 под `## ТЗ` (owner-решение #517, дата ТЗ
+2026-09-12, после отсечки 2026-09-10 — файл в `docs/specs/` не заводится, и
+это верно). Трек `small` подтверждён аналитикой владельца в комментарии
+`S2` от 2026-09-12: сложность 3/10, одна поверхность, без миграции, нового
+UX-контракта, влияния на perf/touch — критерии §5 совпадают, отказ от
+лёгкого трека не обоснован и не требуется.
+
+Материал ревью — тело issue #554 на момент чтения (аналитика владельца уже
+оставлена как последний комментарий; ТЗ ниже неё не менялось после этого).
+Заход r1, первый цикл ревью ТЗ для этой задачи.
+
+## Как проверялось
+
+Ревью ТЗ на этапе `spec` — сверка утверждений ТЗ с фактическим кодом на
+`origin/dev` (`3aa9ffbd`), а не доверие рассказу автора:
+
+- прочитан `docs/SCOPE.md` — задача попадает в J4/J6 (сохранность и
+  пригодность пользовательских вложений), конфликта с mission/out-of-scope
+  нет;
+- прочитан `AGENTS.md`, `PROCESS.md` §2.4, §5, §7.1 — требования к разделам
+  ТЗ на лёгком треке (проблема · контракт · AC1…ACn с доказательством ·
+  откат) и порядок ревью;
+- прочитан `docs/USER-GUIDE.ru.md` (раздел лимитов, строка про 512 МБ) —
+  формулировка порога для пользователя не меняется этим ТЗ, значит правка
+  документации на уровне UI-текста не требуется;
+- прочитан `custom_components/houseplan/plans.py` (`check_quota`,
+  `dir_usage`, `MIN_FREE_BYTES` в `const.py`) и
+  `custom_components/houseplan/http_api.py` (`HouseplanUploadView.post`,
+  строки ~440–460) — подтверждено буквально: staged-файл пишется на диск
+  ДО вызова `check_quota(..., incoming=tmp_path.stat().st_size,
+  exclude=tmp_path)`, и guard вычитает `incoming` второй раз из уже
+  уменьшенного `disk_usage().free`;
+- проверено единственное другое место вызова `check_quota` —
+  `websocket_api.py:2349` (plan upload, `check_quota(plans_dir, len(raw),
+  ...)` **до** `path.write_bytes(raw)`) — это действительно
+  «ещё не записанный» путь, и утверждение ТЗ о нём («сохраняет прежний
+  reserve по умолчанию») корректно;
+- отдельно проверен decor-asset upload (`HouseplanDecorAssetView._store`,
+  http_api.py:225–338): он не использует `check_quota` вовсе и держит
+  файл в памяти (`blocks: list[bytes]`) до собственного инлайн-чека
+  `shutil.disk_usage(root).free - len(validated.data) < MIN_FREE_BYTES`
+  **до** записи на диск — тоже «ещё не записанный» путь, и то, что ТЗ не
+  включает этот файл/путь в затронутые модули, соответствует факту, а не
+  пропуск;
+- прочитаны существующие тесты `tests_backend/test_validation.py`
+  (`test_issue_498_check_quota_excludes_the_staged_upload_itself`,
+  `test_check_quota_refuses_when_the_disk_is_nearly_full`) и
+  `tests_backend/test_ha_upload.py` (конкурентный upload-тест с
+  `threading.Barrier`, строки ~122–140) — подтверждают, что #498
+  (double-count в store byte/count quota) уже закрыт отдельно от диск-guard'а,
+  и что инфраструктура для boundary/concurrency тестов, на которую опирается
+  план автотестов ТЗ, уже существует и расширяема без новой механики;
+- проверено, что `scripts/mutation-gate.mjs` уже содержит мутанты с
+  `guard: 'python3 -m pytest tests_backend/...'` для других backend-контрактов
+  — заявленный в AC5 механизм (именованный мутант на backend-защиту) не
+  является новой инфраструктурой, а используется тем же способом, что и в
+  проекте ранее;
+- проверено `docs/CONFIG-COMPATIBILITY.md` на предмет quota/disk-полей —
+  единственное упоминание quota касается несвязанного decor-asset
+  дедупликации по хешу; утверждение ТЗ «миграция и compatibility-поля не
+  нужны» ничем не опровергается.
+
+Гейты (typecheck/test/build) на этом этапе не прогонялись: этап `spec`
+проверяет постановку, а не код — кода по этой задаче ещё нет (только
+`.dev` на `3aa9ffbd`, без изменений в `custom_components/**`). Это
+осознанный пропуск, а не необходимость: ревью ТЗ гейтов не гоняет по
+`PROCESS.md` §2.4/§8 — они появляются в код-ревью.
+
+## Находки
+
+Ни одной. Ни High, ни Medium, ни Low.
+
+## Что проверено и корректно
+
+- **Соответствие `docs/SCOPE.md`.** Задача закрывает J4/J6 (сохранность и
+  пригодность прикреплённых пользовательских файлов); правки продукта нет —
+  это устранение ложного отказа существующей функции, а не новая
+  возможность. Конфликтов с out-of-scope и с правилом «никогда не удалять
+  файл пользователя по догадке» нет — cleanup/promotion не меняются (п.5
+  контракта, AC4).
+- **Соответствие §5 (лёгкий трек).** Все обязательные разделы присутствуют:
+  проблема (§1), контракт поведения (§2), критерии приёмки с доказательством
+  (§3), откат (§6). Дополнительно (не обязательно, но полезно и корректно)
+  — затронутые файлы (§4), совместимость (§5) и явный блок принятых
+  предположений (§7).
+- **Однозначность контракта.** Пять пунктов §2 ТЗ описывают ровно одно
+  изменение: новый параметр `additional_disk_bytes` у `check_quota`,
+  по умолчанию равный `incoming` (обратная совместимость для plan upload и
+  любых будущих вызовов «до записи»), attachment-путь передаёт `0` и
+  `exclude=tmp_path`. Разделение «логический размер для store-квоты» и
+  «сколько ещё физически предстоит записать» соответствует буквально тому,
+  что делает код сегодня одной функцией на два смысла сразу — источник
+  дефекта назван точно.
+- **Каждый AC проверяем и имеет способ доказательства.** AC1 и AC2 задают
+  точные граничные значения (`free == MIN_FREE_BYTES` /
+  `MIN_FREE_BYTES - 1` для staged; `MIN_FREE_BYTES + incoming` /
+  `... - 1` для not-yet-written) — оба воспроизводимы через существующий
+  паттерн `monkeypatch.setattr(shutil, "disk_usage", ...)`, уже
+  использованный в `test_check_quota_refuses_when_the_disk_is_nearly_full`.
+  AC3 и AC4 — регрессионные, привязаны к именованным существующим тестам
+  #498. AC5 — защитный AC с явно названным механизмом доказательства
+  (мутант в `scripts/mutation-gate.mjs`, тот же механизм что и у других
+  backend-защит в проекте) и корректно требует, чтобы ревьюер кода увидел
+  «тест умеет падать», а не поверил слову.
+- **Никаких выданных за факт догадок.** Все допущения (§7 ТЗ) явно помечены
+  как предположения и подтверждаются кодом независимо от авторского
+  заявления: staging и итоговый файл — один `files_root`/один том (иначе
+  `os.replace()` в `_promote()` не работал бы через директории уже сейчас);
+  `disk_usage().free` меряется после записи staging и отражает чужие
+  staged-файлы тоже — это свойство самого syscall, а не решение автора;
+  новый параметр — внутренний контракт, не раскрывается пользователю
+  (проверено: `docs/USER-GUIDE.ru.md` не описывает внутренние параметры
+  quota-функций, только конечный порог 512 МБ, который не меняется).
+- **Продуктовых вопросов владельцу нет и не требовалось.** Видимое
+  поведение однозначно: сейчас — ложный отказ загрузки вложения около
+  порога; после — успешная загрузка при фактически достаточном месте.
+  Погранично-продуктовых развилок (какая персона важнее, что считать
+  приемлемой деградацией) в задаче нет — это чистое исправление дефекта в
+  рамках уже описанного контракта (512 МБ резерва), а не новый UX.
+- **Один источник числа.** Единственная пользователь-видимая величина —
+  порог 512 МБ (`MIN_FREE_BYTES`) — не дублируется и не меняется; ТЗ явно
+  фиксирует, что глобальный резерв остаётся прежним (§5 «Совместимость и
+  ограничения»). Правило «одно число — один источник» не задето.
+
+## Чего не проверял
+
+- Собственно код исправления — его ещё нет (этап `spec`, статус
+  `S4-spec-review`, класс A не тронут). Все проверки кода выше — это
+  сверка утверждений ТЗ с **текущим** (нечинённым) поведением, а не аудит
+  будущей реализации.
+- Гейты `typecheck`/`test`/`build`/`pytest tests_backend` — не прогонялись,
+  не относится к этапу spec; будут частью код-ревью.
+- Полнота списка call sites `check_quota` подтверждена `grep` по всему
+  `custom_components/houseplan/**`, но не покрывает гипотетические внешние
+  вызовы (плагины/интеграции) — House Plan не предоставляет публичного
+  Python API для этого модуля, риск считаю нулевым.
+
+## Вердикт
+
+Зелёный. ТЗ полно для лёгкого трека, каждый AC однозначен и доказуем,
+технические утверждения проверены по коду и подтвердились буквально,
+продуктовых вопросов не осталось.
+
+---
+
+## Материал раунда
+
+- Issue: #554
+- Дерево ТЗ: тело issue #554, раздел `## ТЗ`, состояние на момент чтения
+  (после последнего комментария владельца `S2` от 2026-09-12T20:38:57Z,
+  без последующих правок тела).
+- Referenced code (для сверки утверждений ТЗ, не как объект правки):
+  `origin/dev` `3aa9ffbd1a1ac1970a91cedbc3d0d6f0a521692a`.
+- Заход: r1. Блокирующих циклов израсходовано: 0/2 (лимит лёгкого трека).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `3aa9ffbd1a1a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `c6b6b6f11f936b2a4d55991381f14406f1b377b2`
+  ```
+  git log --all --format='%H %T' | grep c6b6b6f11f93
+  ```
+- Тело issue: `3108990578ecf761611c8830494881c27f6d0cffdef2f0d283304bd9c32968e7`
+- Вердикт конвейера: `green` · High 0