From f234855e66ee7baba528b7c856c5ab79e9491156 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:24:08 +0000 Subject: [PATCH] docs: review document for #614 Issue: #614 User-Visible: no --- docs/reviews/CODE-REVIEW-614-r2.md | 177 +++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-614-r2.md diff --git a/docs/reviews/CODE-REVIEW-614-r2.md b/docs/reviews/CODE-REVIEW-614-r2.md new file mode 100644 index 00000000..3072af27 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-614-r2.md @@ -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. + +--- + + + +--- + + + +## Материал раунда + +- Ветка: `issue/614-dialog-validation-baseline`, коммит `a2d63715451a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `91da3718e026e3f8403847f2686d2d244b20c5f2` + ``` + git log --all --format='%H %T' | grep 91da3718e026 + ``` +- Тело issue: `3b8a95fd43a037cc6c1f3207c6f5b242ef566b28bde385f5e12b4a225d938221` +- Вердикт конвейера: `green` · High 0