Files
houseplan-card/docs/reviews/CODE-REVIEW-614-r2.md
T
2026-09-23 13:24:08 +00:00

13 KiB
Raw Blame History

CODE-REVIEW-614-r2

Issue: #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. Мутационные 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, включая мутанты по диффу, все 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