From 49130d4ffdefa571f1e99f1436e779c1d5b9923d Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 20:44:46 +0000 Subject: [PATCH] docs: review document for #554 Issue: #554 User-Visible: no --- docs/reviews/SPEC-REVIEW-554-r1.md | 180 +++++++++++++++++++++++++++++ 1 file changed, 180 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-554-r1.md diff --git a/docs/reviews/SPEC-REVIEW-554-r1.md b/docs/reviews/SPEC-REVIEW-554-r1.md new file mode 100644 index 00000000..f61afcc2 --- /dev/null +++ b/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 (лимит лёгкого трека). + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `3aa9ffbd1a1a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c6b6b6f11f936b2a4d55991381f14406f1b377b2` + ``` + git log --all --format='%H %T' | grep c6b6b6f11f93 + ``` +- Тело issue: `3108990578ecf761611c8830494881c27f6d0cffdef2f0d283304bd9c32968e7` +- Вердикт конвейера: `green` · High 0