docs: review document for #614

Issue: #614
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-23 13:24:08 +00:00
parent a2d6371545
commit f234855e66
+177
View File
@@ -0,0 +1,177 @@
# CODE-REVIEW-614-r2
Issue: [#614](https://github.com/Matysh/houseplan-card/issues/614) — «Диалоги настроек: единая политика пустого/невалидного поля,
тосты `_saveMarker` → ошибки под полем, снимок baseline забывается до валидации».
Материал: `a2d63715451acf50d9fa088e62b83b2a3419716a` (ветка
`issue/614-dialog-validation-baseline`, рабочая копия уже на нём).
Заход: **r2**. Предыдущий вердикт — жёлтый, r1, зафиксирован в комментарии
issue от 2026-09-23T13:06:17Z на SHA `bc6b243f56362ba89f157b891955b71a004d8e81`
(документ `docs/reviews/CODE-REVIEW-614-r1.md`). Единственная находка r1 —
**M1 (Medium, в скоупе)**: ТЗ требует «unit + browser smoke» для AC1–AC3 и
юнит-часть AC4, но `generalProblems`, `spaceDialogProblems`, `markerProblems`
не имели ни одного юнит-теста и даже не компилировались для тестов
(`tsconfig.test.json` не включал `general-form-state.ts`, `space-form-state.ts`,
`marker-form-state.ts`).
## Дельта r1 → r2
```
git diff bc6b243f..a2d63715 --stat
docs/reviews/CODE-REVIEW-614-r1.md | 276 +++++++++++++++++++++++++++++++++++++
test/dialog-form-problems.test.mjs | 132 ++++++++++++++++++
tsconfig.test.json | 2 +-
```
Два содержательных файла, оба ровно про M1: новый юнит-тест и добавление трёх
модулей в тестовый проект. Продуктовый код (`src/**` за пределами тестового
конфига) не тронут вообще — дельта локальна, полный повторный разбор всего
диффа `origin/dev...HEAD` не требуется (правило §2.9): контракт поведения не
меняется, новая подсистема не затронута, ребейза нет.
Коммит дельты: `a2d63715` — `Issue: #614`, `User-Visible: no`. Верно: чисто
тестовая инфраструктура, пользовательского поведения не меняет, changelog не
требуется.
## Закрытие раунда r1
| Находка | Чем закрыта | Где это видно |
|---|---|---|
| M1 — нет юнит-тестов `generalProblems`/`spaceDialogProblems`/`markerProblems`, три файла не в `tsconfig.test.json` | Добавлен `test/dialog-form-problems.test.mjs` (6 тестов, импортирует все три функции из `test-build/editors/*.js`) + `tsconfig.test.json` теперь компилирует `general-form-state.ts`, `space-form-state.ts`, `marker-form-state.ts` | `test/dialog-form-problems.test.mjs:1-132`; `tsconfig.test.json` diff (строка со списком `"src/types.ts", ...` дополнена тремя путями) |
Проверил закрытие не на слово:
- Прочитал реализации всех трёх функций (`src/editors/general-form-state.ts:34-42`,
`src/editors/marker-form-state.ts:47-59`, `src/editors/space-form-state.ts:61-93`)
и построчно сверил с ассертами теста — тест бьёт по тем же веткам, что
называла находка M1: `glow > 0` (включая `0`, `-0`, `-1`, запятая), north
optional/строгие целые границы 0…359, `cellCm` через `GRID_CELL_CM_MIN/MAX`
и запятую, оба температурных поля раздельно + ошибка диапазона строго на
`max` (включая `min === max`), custom-север пространства только при
выбранном режиме, три поля устройства (`marker-name`, `marker-run-target`,
`marker-value-badge-source`) вместе и по отдельности плюс два «пустых»
случая (untouched badge, valid source).
- Пересобрал тестовый проект локально (`npx tsc -p tsconfig.test.json && node
scripts/fix-test-build.mjs`) и прогнал именно этот файл:
`node --test test/dialog-form-problems.test.mjs` — 6/6 pass.
- Тест умеет падать: заменил `glow > 0` на `glow >= 0` прямо в
`test-build/editors/general-form-state.js` (мутация в духе исходной
находки — «0 не проходит») и перезапустил — первый тест
(`generalProblems validates required glow text...`) красный, остальные
пять зелёные. Откатил мутацию, `test-build/` удалён (не коммитится, вне
репозитория ничего не осталось: `git status --short` чист).
- CI на этом самом SHA (`a2d63715`) уже гонял «Мутанты по диффу» (6 шардов) —
все зелёные: [run 35865769040](https://github.com/Matysh/houseplan-card/actions/runs/35865769040).
Мутационные guard'ы M3/M4/M7 (шесть мутантов, добавленных диффом r1) по-прежнему
ловятся — новый юнит-слой не ослабил и не заменил их, а закрыл ровно
недостающий уровень доказательства.
M1 закрыта полностью и по букве ТЗ (юнит-уровень для всех трёх `*Problems`),
и по существу (тест доказанно чувствителен к границам контракта).
## Унаследовано из r1
Без повторной проверки в r2 приняты выводы `docs/reviews/CODE-REVIEW-614-r1.md`
на SHA `bc6b243f56362ba89f157b891955b71a004d8e81` — дельта их не задевает
(продуктовый код не менялся):
- AC1 (общие настройки): raw/typed split, `TRANSIENT`-ключи исключены из
dirty, reset-to-defaults синхронен — смоки подтверждены.
- AC2 (пространство): `spaceDialogProblems` пересчитывает `cellCm` строго из
input, единый источник `GRID_CELL_CM_MIN/MAX`, температура и north
проверены раздельно, `_saveSpaceDialog`/онбординг перепроверяют перед
записью, в конфиг попадают только typed-поля.
- AC3 (устройство): три предусловия `_saveMarker` продублированы в
`markerProblems`, тосты удалены из всех четырёх локалей, `forgetMarkerBaseline`
перенесён после успешного сохранения, `requestClose` не спрашивает confirm
при отсутствии изменений.
- AC4 (warm revive): единый хелпер `at()`/`warmDialogBaseline` переносит
baseline атомарно с черновиком для всех четырёх форм; `warmBaselineKind`
ограничен нужными видами.
- AC5 (совместимость): raw-поля и baseline не сериализуются в конфиг/localStorage,
формат валидной записи не изменился.
- AC6 / i18n / трейлеры родительских коммитов (`1a6b12af` — `User-Visible: yes`,
оба changelog в том же коммите; `bc6b243f` — точечный тип-фикс, `User-Visible: no`) —
приняты как проверенные в r1.
- Бандл-бюджет, `npm test` 2837/2838 (1 skip не по теме), `bundle:sync` —
зелёные на SHA r1 и остаются зелёными на SHA r2 (тот же продуктовый код,
плюс независимо подтверждено свежим CI-прогоном на `a2d63715`, см. ниже).
## Как проверялось в r2
Дешёвые гейты на `a2d63715` уже подтверждены зелёным Validate
([run 35865769040](https://github.com/Matysh/houseplan-card/actions/runs/35865769040),
включая мутанты по диффу, все 6 шардов) — не перегонял `npx tsc --noEmit`,
`npm test`, `npm run build`. Дополнительно сам, точечно под дельту:
- `npx tsc -p tsconfig.test.json` — 0 ошибок (новые три файла компилируются
для тестов корректно).
- `node --test test/dialog-form-problems.test.mjs` — 6/6 pass.
- Мутация одной строки `general-form-state.ts` (глоу `>0` → `>=0`) →
соответствующий тест красный; откат → снова зелёный. Тест доказанно умеет
падать.
- Построчное чтение трёх `*Problems` против ассертов нового теста (см. выше).
Не гонял (дельта не даёт повода): `node scripts/check-docs.mjs` — diff не
трогает `src/**`; `npm run invariants` / geometry parity — diff не трогает
геометрию; browser smoke — дельта не меняет поведение UI, только добавляет
юнит-слой поверх уже проверенных смоками веток; `golden:verify` — рендер не
затронут; `pytest tests_backend` — `custom_components/**/*.py` не тронут.
Все эти гейты уже были обоснованно пропущены или пройдены в r1 на
продуктовом коде, который дельта r2 не меняет.
## Находки
Нет. M1 закрыта полностью, новых находок дельта не создала (два новых файла
— тестовая инфраструктура, без побочных эффектов на продукт).
## Что проверено и корректно
- `tsconfig.test.json`: добавление трёх путей ничего не удалило и не
переупорядочило существующий список — diff точечный, одна строка.
- `test/dialog-form-problems.test.mjs` соответствует контракту ТЗ
(«Контракт числового ввода», «Контракт ошибок устройства»): границы
0/359, `-0`, запятая-разделитель, пробелы-как-пусто, мусорный текст,
`min === max` для температуры, порядок трёх ошибок устройства — все
явно поименованные в ТЗ случаи покрыты.
- Импорты теста берут функции из `test-build/editors/*.js`, а не
переопределяют их локально — тест бьёт по реальной продуктовой логике,
не по дублирующей копии.
## Чего не проверял и почему
- Полный `origin/dev...HEAD` diff — не требуется §2.9: дельта локальна,
контракт поведения не меняется, продуктовый код не тронут; полный разбор
уже сделан в r1 на `bc6b243f`.
- Browser smoke, `golden:verify`, `invariants`, `pytest tests_backend`,
performance — дельта не даёт для них повода (см. выше), а сами они уже
подтверждены зелёными в r1 на неизменившемся с тех пор продуктовом коде.
- Полный `npm test` локально — не гонял повторно, положился на зелёный
CI-прогон на этом самом SHA (`a2d63715`, run 35865769040) плюс
прицельный прогон нового файла отдельно.
## Итог
Единственная находка r1 (M1) закрыта по существу: юнит-тесты добавлены,
компилируются, проходят, доказанно чувствительны к границам контракта, и
это подтверждено ещё и мутационным CI-прогоном на самом материале ревью.
Новых находок нет. High: 0, Medium: 0.
---
<!-- material-anchors: заполняется шагом публикации -->
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `issue/614-dialog-validation-baseline`, коммит `a2d63715451a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `91da3718e026e3f8403847f2686d2d244b20c5f2`
```
git log --all --format='%H %T' | grep 91da3718e026
```
- Тело issue: `3b8a95fd43a037cc6c1f3207c6f5b242ef566b28bde385f5e12b4a225d938221`
- Вердикт конвейера: `green` · High 0