diff --git a/docs/reviews/SPEC-REVIEW-614-r1.md b/docs/reviews/SPEC-REVIEW-614-r1.md new file mode 100644 index 00000000..16888f0f --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-614-r1.md @@ -0,0 +1,214 @@ +# 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>`; `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