houseplan-card/docs/reviews/SPEC-REVIEW-614-r1.md

SPEC-REVIEW-614-r1

Материал: тело issue #614 — первая редакция ТЗ, включающая раздел «Аналитика» (2026-09-23T07:57:15Z), пачку продуктовых вопросов Q1/Q2 (2026-09-23T07:57:19Z) и решение владельца «Q1 — Default; Q2 — Default» (2026-09-23T11:55:22Z). ТЗ уже отражает оба default-варианта дословно — открытых продуктовых вопросов к моменту ревью нет. Дерево кода на момент проверки — рабочая копия репозитория, HEAD отцеплен на 9beb6d45 (материал предыдущей задачи #625); код прочитан только для проверки утверждений ТЗ о текущем поведении, не как материал код-ревью этой задачи.

Заход r1, блокирующих циклов израсходовано 0 из 4 (первый заход, полный трек — обоснование трека в комментарии-аналитике корректно: сложность/риск 6/6 выше порога 3, задета не одна поверхность, а lifecycle четырёх диалогов и общий модуль dialog-baseline.ts).

Скоуп ревью

Issue #614: находки аудита M3 («пусто в DOM, старое в состоянии» для числовых полей четырёх форм), M4 (три toast-предусловия _saveMarker, забытый до сохранения baseline устройства) и M7 (warm revive переносит черновик без baseline). Проверялись: наличие обязательных разделов §7.1, однозначность и доказуемость AC1…AC6, соответствие описания текущему коду (general-settings- dialog.ts, general-form-state.ts, space-form.ts, space-form-state.ts, marker-dialog.ts, marker-form-state.ts, dialog-baseline.ts, houseplan-editor-runtime.ts, houseplan-card.ts), соответствие docs/SCOPE.md (J4/J6), терминологии docs/USER-GUIDE.ru.md и отсутствие конфликта с docs/TOUCH-SUPPORT.md/docs/CONFIG-COMPATIBILITY.md.

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

• Прочитаны docs/SCOPE.md целиком, AGENTS.md, разделы PROCESS.md §1–§9 (лимит циклов §4, лёгкий трек §5, цепочка артефактов §7.1, шаблоны §7.2, повторный раунд §2.10 — не применим, это r1).
• Прочитано тело issue #614 и все три комментария: аналитика, вопросы Q1/Q2, решение владельца. Убедился, что текст ТЗ в теле issue уже включает оба default-варианта дословно (см. разделы «Контракт числового ввода» и «Warm revive и baseline») — решение владельца не осталось только в комментарии.
• Прочитан docs/USER-GUIDE.ru.md:395-416 — подтверждена терминология «Проверить N полей», поведение Save/Cancel-confirm и стиль inline-ошибки под полем, которую ТЗ использует без изобретения новых слов.
• Прочитаны docs/TOUCH-SUPPORT.md и docs/CONFIG-COMPATIBILITY.md на предмет конфликта: warm-revive и session-only память уже описаны в TOUCH-SUPPORT.md как существующий паттерн (backgrounding/warm), схема конфига не расширяется — конфликтов не найдено.
• Построчно проверены все технические утверждения ТЗ о текущем коде, включая номера строк, названные в исходном отчёте аудита:
  • general-settings-dialog.ts:128-135 — подтверждено: onInput вызывает set({ glowRadius: v }) только при v != null && v > 0; при пустом/ невалидном вводе состояние не меняется, а unitInput рендерится с тем же String(d.glowRadius), поэтому Lit не перезаписывает уже очищенный пользователем DOM — ровно сценарий «пусто в DOM, старое в состоянии».
  • general-form-state.ts:33-36 (generalProblems) — подтверждено, что gs.error_glow_radius физически недостижим, т.к. d.glowRadius никогда не становится невалидным значением по вине бага выше.
  • general-settings-dialog.ts:200-203 (общий север) — прочитан отдельно от заявленной в issue строки; найден другой режим того же дефекта: непустой мусор здесь не «остаётся старым числом», а молча превращается в null («не задано»). Контракт ТЗ («непустой мусор... — ошибка») этот вариант закрывает так же, как и вариант из space-form.ts — расхождения с исходным описанием багов нет, оно просто более точное, чем текст находки M3.
  • space-form.ts:297-309 (tempMin/tempMax) — подтверждено: tempMin вообще не привязан к *Problem/invalid, tempMax получает индикацию только для отношения max > min, невалидный текст обоих полей молча отбрasывается strictNumber без ошибки и без сохранения текста.
  • space-form.ts:509-515 (north-deg, режим «своё направление») — подтверждено: Number.isFinite(n) ? Math.round(n) : d.northDeg — старое число при NaN.
  • space-form.ts:208-225 (масштаб) — подтверждено: cellCmInput показывается сырым, cellCm обновляется только при canonical > 0 и молча клампится в [CELL_CM_MIN, CELL_CM_MAX], spaceDialogProblems поле не проверяет. Контракт ТЗ («вне диапазона — ошибка, а не скрытое сохранение старого/ограниченного числа») корректно называет именно этот кламп как часть дефекта, а не только «пусто».
  • marker-form-state.ts:31-50 (markerProblems) — подтверждено: знает только marker.error_binding; virtual_name_required, run_target_required, value_badge_source_required в списке отсутствуют.
  • houseplan-editor-runtime.ts:7868-7885 (_saveMarker) — подтверждены все три toast-предусловия дословно (toast.virtual_name_required, toast.run_target_required, toast.value_badge_source_required) и то, что HA-binding-статус (ha_disabled/ha_binding_unverified) — отдельная четвёртая toast-проверка ниже по коду, которую ТЗ явно выносит в не-скоуп («Изменение проверок доступности HA binding... не входит»).
  • marker-dialog.ts:879 — подтверждено дословно: forgetMarkerBaseline(this.host); void this._saveMarker(); — снимок стирается синхронно до и независимо от результата _saveMarker.
  • dialog-baseline.ts:13-24 — подтверждено: baselines — WeakMap<host, Map<kind,key>>; dialogDirty возвращает true, если для хоста нет записи (baseline === undefined) — комментарий в файле сам описывает это как сознательный safe-default «нет снимка → Save доступен».
  • houseplan-card.ts:3655-3675 (warm revive) — подтверждено: в новый хост копируется только d.data (черновик) через switch (d.kind); так как baselines — WeakMap по объекту хоста, у нового хоста записи в ней нет вовсе → следующий рендер увидит dialogDirty() === true безусловно, независимо от того, менял ли пользователь что-либо — подтверждён механизм M7 buggy до основания, а не только его следствие.
  • space-dialog.ts:116-121 (strictNumber) — подтверждено: десятичная запятая (replace(',', '.')) уже поддержана, контракт ТЗ ничего нового здесь не требует.
  • space-form-state.ts:19-22 и marker-form-state.ts:14-15 — подтверждён существующий прецедент исключения «сырых»/«тач»-полей (cellCmInput, cellCmTouched) из ключа dirty-сравнения; контракт ТЗ «сырые поля исключаются из semantic dirty-key» — не новая механика, а явное расширение уже работающего паттерна на новые raw-поля.
• Сверены все переиспользуемые i18n-ключи (gs.error_glow_radius, gs.error_north, space.error_temp_range, space.error_north) во всех четырёх локалях src/i18n/settings/{en,ru,de,fr}.json — присутствуют и дают паритет; отсюда пять новых ключей (space.error_scale, space.error_temp_value, marker.error_virtual_name, marker.error_run_target, marker.error_value_badge_source) не дублируют уже существующий текст и понадобятся по-настоящему.
• Автотесты/гейты в этом раунде не гонялись: на стадии spec нет кода задачи для прогона (изменения не внесены), только чтение уже существующего дерева для проверки утверждений ТЗ — по аналогии с прежними ревью ТЗ (например, SPEC-REVIEW-613-r1).

Находки

Не найдено ни одной блокирующей (High) или Medium-находки в скоупе или вне скоупа.

Что проверено и корректно

• Все обязательные разделы §7.1 присутствуют и в правильном порядке: сценарий · что человек увидит до и после · проблема · скоуп/не-скоуп · контракт поведения (числовой ввод + ошибки устройства + warm revive) · UX и доступность · модель данных и совместимость · i18n · критерии приёмки AC1…AC6 с указанием способа доказательства · план автотестов · риски · откат · release-артефакты · отдельный блок «Принятые технические предположения» в конце.
• Каждое AC однозначно и называет способ доказательства (unit + browser smoke для AC1–AC4/AC6, unit + review code для AC5, что допустимо по §2.5 — «ревью кода» входит в перечисленный набор доказательств).
• Все технические утверждения ТЗ о текущем поведении кода подтверждены чтением исходников (список выше), включая один случай, где реальный код оказался даже точнее, чем формулировка находки M3 (общий север превращается в null, а не сохраняет старое число) — контракт ТЗ, сформулированный через конечное поведение («непустой мусор — ошибка»), а не через механизм бага, покрывает оба варианта без изменений.
• Продуктовые вопросы Q1/Q2 заданы корректно (что видит пользователь + вариант по умолчанию), владелец ответил, и итоговый текст ТЗ уже переписан под эти ответы — не осталось несведённого расхождения между вопросом, ответом и телом ТЗ.
• Блок «Принятые технические предположения» использован по назначению: туда попали только не наблюдаемые пользователем детали (имена transient- полей, порядок подсчёта дублирующихся ошибок диапазона, судьба internal guard в _saveMarker) — ни одна строка не выдаёт техническую догадку за продуктовое решение, и раздел «Риски» отдельно называет главный риск этого решения (дедупликация счётчика) с указанием, чем он проверяется (unit-тест).
• Не-скоуп сформулирован явно и корректно исключает смежные, но не затрагиваемые контракты: диапазоны/единицы измерения, пустые границы температуры комнаты (отдельный контракт наследования), необязательный радиус свечения устройства, доступность HA binding/радара/загрузок, перенос через полную перезагрузку страницы.
• i18n-план полон и проверяем: пять новых ключей не дублируют существующие, переиспользуемые ключи присутствуют во всех четырёх локалях.
• Модель данных/совместимость: заявлено «схема, revision, backend не меняются», подтверждено отсутствием в docs/CONFIG-COMPATIBILITY.md какого-либо контракта, затрагиваемого transient-полями диалоговых форм (тот документ описывает геометрические draft-структуры, не relevantные здесь).
• Touch: заявлено «не ухудшается, новых жестов нет, View/kiosk не затрагиваются» — корректно для admin-only редакторов по docs/ TOUCH-SUPPORT.md и docs/SCOPE.md (редакторы desktop-first).
• Скоуп ясно привязан к J4/J6 docs/SCOPE.md: честность формы при сохранении устройства/пространства/общих настроек — часть «Keep the plan true as the home evolves» (J6) и «zero-to-plan» онбординга без скрытых ловушек (J4). Обоснование трека (полный, не small) верно: несколько поверхностей, общий lifecycle-модуль, риск/сложность выше порога 3.

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

• Не запускал tsc/test/build/smoke — на стадии ТЗ нет кода изменений для прогона; вся проверка — чтение существующего дерева для сверки утверждений ТЗ (список выше).
• Не проверял связанные issue #600/#607 построчно — они использованы только как контекст «что уже сделано» из комментария-аналитики; сам их код вне скоупа этой задачи и не переоткрывается.
• Не оценивал итоговую реализацию выбора имён transient-полей и точного API переноса baseline — это явно оставлено «внутренней деталью» ТЗ и не входит в обязанности ревью ТЗ.
• Не проверял реальный рендер форм в браузере (demo/Playwright) — на этапе spec поведения ещё нет; это будет предметом code-review по конкретным smoke-файлам, названным в плане автотестов.

Вывод

Находок нет. ТЗ технически точно (все утверждения о текущем коде подтверждены построчным чтением, а не приняты на слово), продуктовые вопросы закрыты владельцем и отражены в тексте, все обязательные разделы на месте, каждый AC однозначен и снабжён способом доказательства. Issue готов к статусу «Готово к разработке».

---

---

Материал раунда

• Ветка: issue/614-dialog-validation-baseline, коммит 9beb6d45a01a — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 8076abadd7adf2b2438742c21adb1cd2dc267226

  git log --all --format='%H %T' | grep 8076abadd7ad
  

• Тело issue: 3b8a95fd43a037cc6c1f3207c6f5b242ef566b28bde385f5e12b4a225d938221
• Вердикт конвейера: green · High 0