houseplan-card/docs/reviews/SPEC-REVIEW-402-r2.md

SPEC-REVIEW-402-r2

• Issue: https://github.com/Matysh/houseplan-card/issues/402
• Артефакт ТЗ: docs/specs/402-confirm-outside-main-branch.md (полный трек, класс A)
• Заход: r2 · лимит циклов ревью ТЗ для полного трека — 4 (§4), блокирующих циклов израсходовано 1/4 (зелёные вердикты бюджет не тратят, #227)
• Ревьюер: Claude (роль «ревьюер ТЗ»), независимая сессия
• Раунд r1: вердикт красный, docs/reviews/SPEC-REVIEW-402-r1.md, база d94db87e (коммит, добавивший спецификацию)
• Раунд r2: правка ТЗ — коммит 11959e4c («docs: #402 spec revision 2 per SPEC-REVIEW-402-r1»), HEAD ревью — 11959e4c

Скоуп ревью (§2.10, по дельте)

r1 закончился красным (High: 1, Medium: 1). Дельта этого раунда — git diff d94db87e..11959e4c -- docs/specs/402-confirm-outside-main-branch.md: правка ровно двух мест ТЗ (текст «Touch и kiosk» + переформулировка обоснования не-скоупа для _tapConfirm/_vacCalConfirm), плюс AC8, AC9 и два пункта плана автотестов. Никакого продуктового кода в дереве нет (этап — ТЗ), контракт поведения не изменился, новая подсистема не задета, объём дельты явно меньше исходной задачи → разбор ограничен дельтой плюс проверкой, что она не сломала уже принятое (§2.10 п.4-5).

Как проверялось

1. Прочитан весь диапазон изменений r1→r2 (git diff d94db87e..HEAD), построчно, для файла ТЗ.
2. Найден вердикт r1 и SHA, на котором он получен (d94db87e, назван в шапке SPEC-REVIEW-402-r1.md — не пришлось восстанавливать).
3. По каждой находке r1 (H1, M1) сверено текстом дельты, чем именно она закрыта — раздел «Закрытие раунда r1» ниже.
4. Технические утверждения новой дельты сверены с текущим деревом на 11959e4c (код не менялся с d94db87e, но проверка не наследуется автоматически — новый текст мог сослаться на код неточно):
   • структура render() (src/houseplan-card.ts:11165-11878) — семь ранних return, отсутствие return между :11248 и :11872 (awk-проверка по диапазону) — подтверждает утверждение M1, что _tapConfirm (:11852), _vacCalConfirm (:11802) и _dangerConfirm (:11872-11878) лежат в одной и той же финальной ветке без разрыва;
   • docs/TOUCH-SUPPORT.md § «Safety floor that still applies to touch editors» (:69-79) — сверено число пунктов и их содержание против фразы ТЗ «запрещает best effort в трёх вещах» — см. находку L1;
   • docs/TOUCH-SUPPORT.md § «Testing and release gates» (:139-151) — сверено, что touch-editor safety floor входит в release-blocking гарантии (п.4), а не только View/kiosk (п.2) — это меняет оценку находки H1 (см. ниже);
   • hasTouch — паттерн существующих touch-смоков (grep -rn hasTouch demo/smoke_*.mjs) реально существует и используется ровно так, как описывает AC8 (demo/smoke_color_picker.mjs, demo/smoke_isometric_live_touch.mjs, demo/smoke_touch_tips.mjs);
   • смоки, на которые ссылается AC9 (тап и калибровка пылесоса), существуют: demo/smoke_tap_run.mjs, demo/smoke_tap_ctx.mjs, demo/smoke_vacuum.mjs, demo/smoke_vacuum_firstuse.mjs, demo/smoke_cold_view_vacuum.mjs.
5. Проверено, остаются ли §7.1-обязательные разделы на месте после правки — да, ни один не удалён, добавился один новый («Touch и kiosk»).
6. Гейты кода не гонялись — на этапе ТЗ продуктового диффа нет (см. «Чего не проверял»).

Закрытие раунда r1

Находка r1	Чем закрыта	Где это видно
H1 (High) — ТЗ не называет влияние на touch/kiosk	Добавлен раздел «Touch и kiosk»: цитирует TOUCH-SUPPORT.md § Safety floor, называет её блокирующей для touch, объясняет, почему перенос точки рендера не меняет touch-поведение (тот же компонент, разметка, scrim, футер не меняются), добавлен AC8 (смок в touch-эмуляции на буквальном сценарии issue)	docs/specs/402-confirm-outside-main-branch.md, раздел «Touch и kiosk» (строки ~112-127 текущей редакции), AC8
M1 (Medium, в скоупе) — ложное обоснование «свои ветки» для _tapConfirm/_vacCalConfirm	Формулировка заменена на две настоящие причины: (1) нет промиса — синхронный exec() / закрытие по hp-close; (2) точка входа недостижима из ранних веток. Добавлено явное требование к реализации не трогать расположение этих двух блоков при выносе _dangerConfirm, плюс AC9 фиксирует это как критерий приёмки	docs/specs/402-confirm-outside-main-branch.md, раздел «Скоуп / не-скоуп» (абзац «_tapConfirm и _vacCalConfirm — не в скоупе...»), AC9

M1 закрыта полностью и точно — построчная сверка (:11802, :11852, :11872-11878, отсутствие return между ними) подтверждает переформулировку слово в слово.

H1 закрыта по существу, с одной оговоркой (см. находку L1 ниже): текст верно поднимает safety floor и даёт достаточное объяснение для блокирующей части DoR (View/kiosk), но не называет явно 5 call site'ов _confirmDanger в houseplan-editor-runtime.ts, про которые r1 просил отдельную строку Touch editor: …. Разобрано отдельно ниже, почему это не держит вердикт красным/жёлтым.

Унаследовано из r1

Без повторной проверки принято всё, чего дельта не касалась — документ docs/reviews/SPEC-REVIEW-402-r1.md, SHA d94db87e:

• диагноз дефекта (структура render(), семь ранних веток, hp-confirm только в финальной) — построчно сверен в r1, дельта эти строки не меняла;
• корректность HpConfirmController (cancel/resolve/токен) — src/danger-confirm.ts не тронут дельтой;
• буквальный сценарий issue (_deleteServerPlan → houseplan-onboarding-runtime.ts:218-224,273) — не тронут;
• факт «noChange нигде не оборачивается в шаблон» — граница ТЗ для языкового гейта warm, код не менялся;
• AC1-AC4, AC6 — проверяемость и реалистичность доказательства (demo/smoke_danger_confirmation.mjs) — текст этих AC дельта не меняла;
• AC5 — рассуждение про недостижимость !hass/!_config после первого монтирования и про то, что noChange не трогает уже открытый диалог;
• AC7 — hp-confirm импортируется не лениво (houseplan-card.ts:14), бюджет не растёт;
• граница скоупа с #32 (содержимое диалога, ревалидация после await) и с #406 (alertdialog/aria-describedby) — абзац не менялся;
• соответствие docs/SCOPE.md (J4, J6) — не затронуто дельтой;
• присутствие всех обязательных разделов §7.1, кроме нового «Touch и kiosk» — остальные разделы не удалялись и не переписывались.

Находки

L1 (Low, наблюдение — не блокирует). Число в цитате TOUCH-SUPPORT.md неверно

Где: docs/specs/402-confirm-outside-main-branch.md, раздел «Touch и kiosk»: «docs/TOUCH-SUPPORT.md § Safety floor запрещает «best effort» в трёх вещах».

Проверено чтением: docs/TOUCH-SUPPORT.md:71-79 перечисляет шесть пунктов под «"Best effort" never permits:» — потерю данных, небезопасный вызов HA, обход подтверждения, залипание карточки вне View, поломку View/ kiosk соседним редактором, случайное сохранение геометрии от неверно понятого жеста. Ни разу не три.

Последствие: не меняет вывод ТЗ — пункт «обход подтверждения разрушающего действия» реален и процитирован верно, вывод («задача поднимает пол обратно») не зависит от точного числа. Это неточная цифра в пересказе документа, а не выданная за факт догадка о продуктовом поведении.

Решение ревьюера: снимаю как Low с записью, не блокирует. Автору стоит поправить «трёх» на «шесть» (или убрать число вовсе) в следующей правке, которую он уже будет делать по другому поводу — заводить цикл ради одного слова нецелесообразно (тот же принцип, что «r2 по #150» в §2.10).

L2 (Low, наблюдение — не блокирует). Явного упоминания touch-статуса пяти вызовов из houseplan-editor-runtime.ts по-прежнему нет

Где: раздел «Touch и kiosk» рассуждает в общем виде («тот же компонент, разметка не меняется») и доказывает это AC8, но AC8 сформулирован только для ветки онбординга; ни разу не назван houseplan-editor-runtime.ts и пять его вызовов _confirmDanger.

Почему это не блокирует, хотя r1 просил именно это. Я перепроверил довод по существу, а не только по форме:

• docs/TOUCH-SUPPORT.md:139-151 («Testing and release gates») перечисляет четыре release-blocking гарантии, и «touch-editor safety floor» — в их числе (п.4), не только View/kiosk (п.2). Так что для полноты формально стоило бы явно назвать и редакторские call site'ы, как просил r1.
• Но по коду («Контракт» ТЗ + AC1, AC2, AC4, AC5) hp-confirm выносится из цепочки render() как единый механизм, не привязанный к тому, кто вызвал _confirmDanger. AC4 («Открытое подтверждение переживает смену ветки») сформулирован без привязки к вызывающему — он покрывает и случай, когда подтверждение открыл редакторский call site, а карточка тем временем потеряла последнее пространство (model.length дошёл до нуля во время открытого диалога — ровно тот сценарий, который сам текст ТЗ называет «второй половиной дефекта»). Отдельного AC для редакторских call site'ов не нужно ровно потому, что контракт написан на уровне компонента, а не вызывающего.
• Поэтому вывод раздела «Touch и kiosk» («разметка, размер целей нажатия, scrim, футер не меняются ни на строку») уже покрывает и эти пять call site'ов — только не называет их явно. Это пробел в полноте текста, не в контракте: настоящей неопределённости, которая изменила бы AC или поведение, за этим не стоит.

Решение ревьюера: снимаю как Low, не как Medium — субстанция уже доказана на уровне механизма (AC1-AC5), формальная строка Touch editor: supported для пяти call site'ов была бы уместна для полноты, но её отсутствие не оставляет открытого продуктового вопроса и не меняет объём реализации. Не завожу повторный цикл ради одной строки текста (тот же принцип §2.10, что и для L1). Рекомендация автору — добавить эту строку попутно при следующей правке, не обязательно сейчас.

Что проверено и признано корректным (дельта r2)

• Раздел «Touch и kiosk» по существу верно определяет, что затронутая ветка (View-диалог онбординга) относится к «Fully supported» категории TOUCH-SUPPORT.md и что перенос точки рендера сам по себе не меняет touch-поведение компонента.
• AC8 технически обоснован: паттерн hasTouch реален и используется в существующих touch-смоках так же, как описано в АС.
• AC9 корректно фиксирует инвариант «блоки на месте» — построчная сверка подтверждает, что все три диалога (_tapConfirm, _vacCalConfirm, _dangerConfirm) в текущем дереве действительно в одной ветке без return между ними, то есть требование реализации не тащить их за компанию — обоснованное и проверяемое.
• M1 закрыта полностью и точно, без остаточных вопросов.
• Новый раздел не удаляет и не противоречит ни одному из ранее принятых разделов ТЗ (§7.1 набор полон).
• Пункты 6-7 плана автотестов (touch-повтор сценария 1, проверка «один hp-confirm в DOM») соответствуют AC8 и риску «двойной рендер», названному в разделе «Риски» ещё в r1.

Чего не проверял

• Гейты кода (npx tsc --noEmit, npm test, npm run build, check-docs, смоки, bundle:budget, инварианты модели) — не гонялись: на этапе ТЗ продуктового кода по-прежнему нет, дифф пуст. Это предмет код-ревью после реализации.
• scripts/mutation-gate.mjs / demo/smoke_danger_confirmation.mjs — не запускал; в r2 они не менялись, наследую проверку существования файлов из r1.
• Реальный браузерный рендер — не переисполнял; воспроизведение из аналитики issue взято на веру, как и в r1 (это будет предметом смок-доказательства AC1-AC9 в код-ревью).
• docs/specs/README.md — строка для #402 по-прежнему не добавлена; унаследованное из r1 решение не поднимать это отдельной находкой (известный долг §7.3 п.1) остаётся в силе.

Вывод

Обе находки r1 закрыты по существу (таблица выше). Дельта r2 не вносит новых Medium/High — только два наблюдения Low (неверное число в цитате документа, отсутствие явной строки про touch-статус пяти редакторских call site'ов), оба сняты ревьюером с записью и не блокируют выход в «Готово к разработке».

Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0