From a5abfe8bd65b40a46ca03ac7fbc4eeb8d25e94e6 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 06:28:18 +0000 Subject: [PATCH] docs: review document for #295 Issue: #295 User-Visible: no --- docs/reviews/SPEC-REVIEW-295-r2.md | 113 +++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-295-r2.md diff --git a/docs/reviews/SPEC-REVIEW-295-r2.md b/docs/reviews/SPEC-REVIEW-295-r2.md new file mode 100644 index 00000000..4ac91efe --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-295-r2.md @@ -0,0 +1,113 @@ +# SPEC-REVIEW-295-r2 + +Issue: #295 +ТЗ: `docs/specs/295-preflight-diagnostics.md` +Ветка: `issue/295-preflight-diagnostics`, SHA материала ревью: `c3b4e3124e65a17693b44bb7fb02cbe291ea6633` +Заход: r2 · трек: обычный · лимит циклов 4 · блокирующих циклов израсходовано до этого ревью: 1/4 + +## Предыдущий раунд + +Вердикт r1: жёлтый, High: 1 (H1), Medium: 1 (M1), Low: 2 (L1, L2). +Документ: `docs/reviews/SPEC-REVIEW-295-r1.md`. +SHA материала r1 в самом документе назван — `c1b5ceb6b42b5ef506c5c625037334fca6ddf877` +(коммит `c1b5ceb6 docs: spec for #295 preflight failure diagnostics`); в тексте +вердикта, оставленном в issue, SHA не приведён — это несоответствие отмечаю +здесь, само по себе не находка нового раунда (SHA был доступен в документе, на +который вердикт ссылался). + +## Скоуп разбора r2 + +Правки r2 — точечные, диапазон `git diff c1b5ceb6..c3b4e312 -- docs/specs/295-preflight-diagnostics.md`: +изменения ограничены §1.1 (природа `detail`), частью §1.4 (пример JSON + разведение undefined/null), +§1.5 (канал версии интеграции), §2 (скоуп: добавлен `websocket_api.py`), новым пунктом +риска №4 в §4, и строкой статуса в шапке. Остальные разделы (§0, §1.2, §1.3, §3, §5, §6, §7, §8) +байт-в-байт совпадают с ревизией 1. Дельта локальна (правки по замечаниям одного раунда, без +ребейза, без смены подсистемы, без изменения контракта поведения за пределами того, что +просили H1/M1/L1/L2) — разбор по PROCESS.md §2.9 сужен до дельты и её последствий. + +## Как проверялось + +1. `git diff c1b5ceb6..c3b4e312 -- docs/specs/295-preflight-diagnostics.md` — построчно, что именно изменилось. +2. Проверка закрытия H1: перечитан барьер #199 в коде (`src/plan-geometry-preflight.ts:312-397` — + актуальные номера строк на HEAD, включая тело `checkOptimizeGeometry` целиком) и три существующих + privacy-теста (`test/plan-geometry-preflight.test.mjs:110-146`) — прогнал их логику вручную против + нового дизайна (`detail = error.name`), не изменяя код. +3. Проверка закрытия M1: `custom_components/houseplan/websocket_api.py` — текущий ответ + `ws_config_get` (`:1114-1161`) и блок импортов (`:21-68`, `VERSION` там не импортирован); + `custom_components/houseplan/const.py:49` (`VERSION = "1.68.0-beta.1"`); `import_export.py:526` + (то же поле в бэкапе) — подтвердил, что предложенное решение (добавить `integration_version` + в `houseplan/config/get`, импортировав `VERSION` из `.const`) технически осуществимо тем же + паттерном, что уже применён для бэкапов. `src/houseplan-card.ts:320` — `CARD_VERSION` существует + на фронте, есть с чем сравнивать. +4. Проверка L1/L2 — текстовая сверка новой формулировки §1.4 и нового пункта риска №4 с тем, что + просил r1. +5. Побочная проверка: не бросает ли что-то в `src` не-`Error` значение, что имело бы значение для + ветки «typeof error» в §1.1 — `grep 'throw ['"'"'"`]'` по `src/` → 0 совпадений, ветка не критична + для текущего кода, но не создаёт проблемы. +6. Проверил, что governing docs (`docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md`, `docs/CANVAS.md`, + `docs/ARCHITECTURE.md`) не менялись между SHA r1 и SHA r2 (`git log -1 -- <файл>` даёт более + ранние коммиты, чем `c1b5ceb6`) — основания вывода r1 по этим документам не протухли, + наследую без повторного чтения содержимого. + +Тяжёлые гейты (typecheck/test/build/смоки) не гонялись — этап ревью ТЗ, кода нет. + +## Закрытие раунда r1 + +| Находка | Чем закрыта | Где это видно | +|---|---|---| +| H1 (High) — контракт тихо отменял барьер #199 (`detail` = сырой `error.message`) | Выбран вариант (а) из r1 с усилением: `detail` = класс исключения (`error.name`, для не-`Error` — `typeof`), только для причин `*-exception`. Barrier-текст «exception text never escapes» не задет: класс — не содержимое. Три существующих privacy-теста (`test/plan-geometry-preflight.test.mjs:123-145`) продолжат проходить без изменений — все используют `new Error(...)`, значит `detail` станет `'Error'`, что не матчится ни с одним `assert.doesNotMatch` (`/secret/`, `/private floor detail/`, `/private preparation detail/`). Проверено чтением: логику подставил вручную, не исполняя. | `docs/specs/295-preflight-diagnostics.md:18` (§1.1, новая формулировка) | +| M1 (Medium) — предикат версии ссылался на несуществующий канал | Явное решение: бэкенд `websocket_api.py` добавляет `integration_version` в ответ `houseplan/config/get`, зеркально `import_export.py:526`; `websocket_api.py` внесён в затронутые файлы (+ тест). Технически чисто: `VERSION` уже есть в `const.py:49`, паттерн `"integration_version": VERSION` уже применяется в `import_export.py:526`, нужен только импорт в `websocket_api.py`. Фронт имеет `CARD_VERSION` (`src/houseplan-card.ts:320`) для сравнения. | `docs/specs/295-preflight-diagnostics.md:61` (§1.5), `:65` (§2, скоуп) | +| L1 (Low) — `detail: null` пример расходился с «поле не заполняется» | Разведено прямо текстом: в результате preflight — `undefined` (поле отсутствует), в JSON-блоке диагностики — явный `null` через `failure.detail ?? null`. | `docs/specs/295-preflight-diagnostics.md:51,57` | +| L2 (Low) — не было явного пункта о производительности | Добавлен риск №4 в §4: «не затрагивается» с обоснованием (ok-путь без новых операций, лог дедуплицирован). | `docs/specs/295-preflight-diagnostics.md:77` | + +Все четыре находки r1 закрыты фактическим текстом ТЗ, а не заявлением автора — сверено построчно +против диффа и против кода, который эти формулировки описывают. + +## Унаследовано из r1 + +Без повторной проверки в этом раунде — дельта их не касается, взято из +`docs/reviews/SPEC-REVIEW-295-r1.md` (материал на SHA `c1b5ceb6b42b5ef506c5c625037334fca6ddf877`): + +- Обязательные разделы §7.1 PROCESS.md присутствуют и не тронуты правкой (сценарий/До-После, + скоуп/не-скоуп, UX/данные/i18n/touch, AC с доказательствами, план тестов, откат, + release-артефакты) — раздел за разделом сверен в r1 «Что проверено и корректно». +- Фактическая база о текущем коде (7 значений `OptimizeGeometryFailureReason`, отсутствие + i18n-ключей `preflight_reason_*`, диалог печатает только имена, две точки вычисления preflight, + шесть потребителей `checkSpacePhysicalGeometry` читают только `.ok`) — подтверждена в r1 чтением + кода на той же ветке; правка r2 эти места не трогала. +- Соответствие docs/SCOPE.md (задача не создаёт нового Core user job, делает диагностируемым + существующий административный инструмент, ближе к J6) — оценка из r1, скоуп задачи в r2 не + расширился за пределы одного дополнительного backend-файла, уже находкой M1 предусмотренного. +- Откат (§8) — один revert, без модели/миграций — раздел байт-в-байт не менялся. +- Мутанты §5.7 используют существующий паттерн `mutation-gate.test.mjs`, клипборд-фолбэк повторяет + существующий UI-паттерн того же диалога — оценка из r1, разделы не менялись. +- Замечание r1 про нестандартный вид ключей i18n (дефис в хвосте `preflight_reason_wall-degraded-extra`) — + оставлено как не блокирующее в r1, раздел §1.2 в r2 не менялся, вывод не пересматриваю. + +## Новые находки r2 + +Нет. Правки r1→r2 закрывают все четыре находки предыдущего раунда корректно по существу, не +привнося нового расхождения с кодом или документами. Отдельно проверил, не сломала ли правка +что-то, ранее признанное верным (риск регрессии вроде #102): пример из §1.4 и текст §1.1 +взаимно согласованы, задетые тесты (`plan-geometry-preflight.test.mjs`) остаются зелёными по +приведённой выше логике, governing docs не требуют правки — и это явно, а не по умолчанию, +подтверждено самим текстом ревизии 2, а не только заявлено. + +## Чего не проверял + +- Реализацию (кода ещё нет) — этап ревью ТЗ. +- Разделы §0, §1.2, §1.3, §3, §5, §6, §7, §8 заново — не изменились в дельте r1→r2, унаследованы + из r1 без повторного построчного чтения кода, который они описывают (только сверил, что текст + файла в этих местах идентичен ревизии 1 через diff). +- `docs/USER-GUIDE.ru.md` текущую формулировку раздела «Оптимизировать» — как и в r1, финальный + текст является предметом код-ревью, не блокирует спеку. +- Не гонял тяжёлые гейты (typecheck/test/build/смоки/инварианты) — кода нет, гонять нечего. + +## Вывод + +Обе блокирующие/скоупные находки r1 (H1, M1) закрыты по существу и подтверждены по коду, а не +только текстом автора. Low-находки устранены. Новых находок в дельте r2 нет. Разбор был сужен до +дельты по PROCESS.md §2.9, так как дельта локальна: правки точечные, ответ на конкретные замечания +предыдущего раунда, без ребейза и без смены подсистемы. + +**Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0**