mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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<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
|
||||
готов к статусу «Готово к разработке».
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: заполняется конвейером публикации -->
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/614-dialog-validation-baseline`, коммит `9beb6d45a01a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `8076abadd7adf2b2438742c21adb1cd2dc267226`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 8076abadd7ad
|
||||
```
|
||||
- Тело issue: `3b8a95fd43a037cc6c1f3207c6f5b242ef566b28bde385f5e12b4a225d938221`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user