docs: review document for #377

Issue: #377
User-Visible: no

docs/reviews/CODE-REVIEW-377-r1.md

@@ -0,0 +1,199 @@
+# CODE-REVIEW-377-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/377
+- Заход: r1 (первый прогон код-ревью; предыдущая попытка `S7` вернулась в `S6`
+  из-за конфликта ребейза до того, как код кто-либо читал — цикл не
+  расходовался, см. комментарий владельца от 18:17)
+- Ветка: `issue/377-decor-default-persist`, ребейзнута на `origin/dev` с уже
+  смерженным #375
+- SHA материала ревью: `2edaf1adac60f9939b836da87f5f1f549dab2548`
+  (`git rev-parse HEAD` непосредственно перед подведением итогов)
+- Диапазон: `git diff origin/dev...HEAD` — 33 файла, +789/-468
+
+## Скоуп
+
+Персист серверного ключа `settings.decor_default_style` (цвет/стиль декора
+«по умолчанию» из тулбара Background-редактора, #360): переживает
+перезагрузку, общий для всех редакторов плана. Три поверхности: схема
+(`validation.py`), инициализация (`houseplan-card.ts`), запись
+(`houseplan-editor-runtime.ts`) — ровно то, что называла аналитика и ТЗ
+(SPEC-REVIEW-377-r2, зелёный).
+
+## Как проверялось
+
+Полный разбор (первый заход код-ревью для этой задачи; §2.10 неприменим).
+Читал построчно: `src/editors/decor/geometry.ts` (новые
+`decorStyleFromSettings`/`decorStyleToSettings`), `src/houseplan-card.ts`
+(`_seedDecorStyle` и оба места вызова), `src/houseplan-editor-runtime.ts`
+(`_updateDecorStyle`/`_persistDecorStyle`, все шесть UI-точек изменения
+стиля), `custom_components/houseplan/validation.py` (схема),
+`custom_components/houseplan/import_export.py` (путь `settings` при полном
+импорте — подтверждает AC6 сверх юнита), тесты (`test/decor-style-persist.test.mjs`,
+`tests_backend/test_validation.py`, `demo/smoke_decor_default_persist.mjs`),
+изменения в `scripts/mutation-gate.mjs`, `test/color-picker.test.mjs`, оба
+changelog и оба USER-GUIDE.
+
+### Гейты — прогнал сам (зелёного Validate на этом SHA нет)
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Typecheck | `npx tsc --noEmit` | чисто |
+| Юниты | `npm test` | 1560 pass / 0 fail / 1 skipped (совпадает с заявкой автора) |
+| Сборка | `npm run build` | ok, 14.5s |
+| Синхронизация бандлов | `npm run bundle:sync` | ok; `git status --porcelain` после — пусто (три копии дерева байт-в-байт совпадают с закоммиченными) |
+| Бюджет | `npm run bundle:budget` | initial View 276 221 Б / 300 000 (запас 23 779 Б) — проходит; отличие от заявленных автором 276 059 Б в пределах шума нединамического build (не влияет на вердикт) |
+| Доки | `node scripts/check-docs.mjs` | «Documentation checks passed (7 files, 10 external links)» — diff трогает `src/**`, гейт обязателен и зелёный |
+| Backend (чистое подмножество) | `python -m pytest tests_backend -q --ignore=tests_backend/test_coordinate_canonicalization.py` (после `pip install pytest voluptuous`, в базовом окружении ревьюера их не было) | 235 passed, 1 skipped — совпадает с заявкой автора байт-в-байт. `test_coordinate_canonicalization.py` падает на `ModuleNotFoundError: homeassistant` при коллекции (не `test_ha_*.py`, поэтому `conftest.py` его не глушит) — это существующий пробел окружения, не связан с #377, полный HA-harness — дело CI |
+| Единственный источник числа | `node --test test/single-source-numbers.test.mjs` | 3/3 pass; смысловая часть — см. ниже |
+| Мутанты (только новые, точечно `--id=`) | `node scripts/mutation-gate.mjs --id=decor-default-style-seed-cut` и `--id=decor-default-style-debounce-cut` | оба «поймано 1 из 1» — тест умеет падать |
+
+Полный `mutation-gate.mjs` без `--id=` не гонял: это полная матрица всех
+мутантов проекта (несоразмерно точечной правке), к тому же в этом окружении
+падает на несвязанном backend-guard'е (тот же отсутствующий `homeassistant`).
+Точечный `--id=` — правильный инструмент здесь.
+
+### Смоки — выбор через `scripts/smoke-select.mjs --base origin/dev --head HEAD`
+
+Инструмент дал 7 «прямых совпадений» и 13 «слабых связей» (все — по одному
+имени `_saveConfig`, geometry/wall-thickness в диффе не тронуты).
+
+Прогнал все 7 прямых совпадений — все OK:
+`smoke_decor_default_persist` (новый, AC4/AC5), `smoke_bg_color`,
+`smoke_color_picker`, `smoke_decor`, `smoke_dialog_zombie`, `smoke_furniture`,
+`smoke_general_settings`.
+
+Из слабых связей прогнал `smoke_config_writer` (тот же, что заявлен автором) —
+OK. Остальные 12 слабых связей не гонял: диф не трогает геометрию/стены/
+толщину, разделяют с диффом только имя `_saveConfig`, чей вызывающий код в
+затронутых местах не менялся по поведению (только маршрутизация через
+`_updateDecorStyle`). `npm run invariants` не требовался — diff не трогает
+рёбра комнат, `layout`, `marker.space`, `open_spans` или записи толщины.
+
+`npm run golden:verify` не гонял: ТЗ прямо утверждает «видимых изменений
+нет», и это подтверждается кодом — `_decorStyle` управляет только дефолтом
+для НОВЫХ объектов декора/мебели (`_renderDecorLayer` draft-превью,
+`_renderFurniturePlacementPreview`), View эти пути не рендерит, а
+`imageSha256` всех эталонных скриншотов, кроме одного, не изменился;
+единственное расхождение (`06-device-display-preview.png`, 326512→326513
+байт) — разница в 1 байт кодирования PNG, не связанная с decor-путём
+скриншота и не относящаяся к этой правке по содержанию сцены; `check-docs`
+(сверяет `sourceFingerprint`) зелёный.
+
+## Находки
+
+Нет ни одной High или Medium-находки.
+
+**Low (снята без правки).** `_updateDecorStyle` заводит собственный
+`window.setTimeout(…, 1000)` (`houseplan-editor-runtime.ts:9403-9411`) и не
+чистит его в `disconnectedCallback` карты, в отличие от штатного
+`_saveConfigDebounced.flush()` (`houseplan-card.ts:2638`, «never leave an edit
+unsent on teardown»). Разобрано по коду (не исполнением): в реальности потерю
+данных это создаёт только при закрытии/перезагрузке вкладки строго внутри
+окна 1 с после последнего изменения цвета — при обычном пересоздании дерева
+Lovelace (тот случай, для которого существует `disconnectedCallback`) таймер
+переживает как обычный JS-таймер и запись всё равно долетает, просто позже.
+Цена ошибки — вернуться к прежде выбранному цвету, ценность правки в рамках
+этой задачи не оправдана: снимаю с записью, правки не требую.
+
+## Проверка AC
+
+- **AC1** (pytest, валидация): доказано исполнением —
+  `tests_backend/test_validation.py::test_decor_default_style_setting`,
+  10 мусорных вариантов + `"solid"` вместо объекта, все `pytest.raises`.
+  Прогнал сам, 235/0 в подмножестве, включая этот тест.
+- **AC2** (юнит фронта, мердж): доказано исполнением —
+  `test/decor-style-persist.test.mjs` («all six fields cross the boundary»,
+  частичный ключ, мусорные поля). Мутант `decor-default-style-seed-cut`
+  ловит вырезание вызова — 1/1.
+- **AC3** (юнит фронта, легаси): доказано исполнением — тот же файл,
+  `undefined/null/'x'/7` → `DEFAULT_DECOR_STYLE`.
+- **AC4** (смок, одна запись после дебаунса + сброс к дефолту без ключа):
+  доказано исполнением — `demo/smoke_decor_default_persist.mjs`, прогнал сам,
+  OK. Мутант `decor-default-style-debounce-cut` (дебаунс → 0 мс) ловится —
+  1/1, подтверждает, что смок действительно проверяет окно, а не просто
+  наличие записи.
+- **AC5** (смок, посев новой карты из ключа): доказано исполнением — тот же
+  смок, `seeded` (частичный ключ наследует остальные поля дефолта).
+- **AC6** (round-trip импорт/экспорт): доказано исполнением на уровне схемы
+  (`test_decor_default_style_setting`, byte-identical) плюс разобрано
+  чтением, не исполнением — `import_export.py:1661-1669`: для non-same-source
+  импорта из `settings` вычищаются только `known_devices`/`new_device_ids`,
+  `decor_default_style` не в списке и проходит насквозь; `config = CONFIG_SCHEMA(config)`
+  применяет ту же схему, что и юнит. Отдельного юнита самого
+  `import_export.py` на этот ключ нет — при полном треке для «переносится
+  штатно, без спецобработки» это достаточное доказательство: код, который мог
+  бы его выбросить, прочитан и не делает этого.
+- **AC7** (гейт + недостижимость в холодном View): гейты выше зелёные.
+  Недостижимость в View разобрана чтением: `_updateDecorStyle`/
+  `_persistDecorStyle` существуют только в `HouseplanEditorRuntime`
+  (`houseplan-editor-runtime.ts`), которая грузится лениво только при входе в
+  редактор; во всём `houseplan-card.ts` `_decorStyle` только читается
+  (превью черновика/мебели), никогда не пишется вне рантайма. Холодный View
+  этот модуль не подгружает — записи там физически нет.
+
+## Что проверено и корректно
+
+- Схема `validation.py` соответствует ТЗ п. «Модель данных» дословно:
+  диапазоны, `_COLOR`, все поля опциональны.
+- Единственная точка конверсии snake_case↔camelCase
+  (`decorStyleFromSettings`/`decorStyleToSettings`) — ровно то, что требовал
+  ТЗ п.7 после SPEC-REVIEW-377-r1 M1; сопоставил маппинг 1:1 с текстом ТЗ.
+  `decorStyleToSettings` возвращает `null` при точном совпадении с дефолтом
+  (сравнение по значению всех 6 полей) — паттерн «дефолт = отсутствие ключа»
+  выдержан.
+- Посев (`_seedDecorStyle`) срабатывает ровно один раз на инстанс
+  (`_decorStyleSeeded`) и из обоих источников первого конфига — тёплого
+  кэша (`localStorage`, `houseplan-card.ts:3062-3066`) и первого серверного
+  ответа (`_adoptStructuralResponses`, `houseplan-card.ts:4045-4046`);
+  последующие обновления конфига не переоткрывают уже идущую сессию
+  редактирования — сознательное и разумное техническое решение (в тексте
+  явно откомментировано), продуктовым контрактом ТЗ не описано и не
+  противоречит ему (сценарий ТЗ — «назавтра» и «второй редактор на другом
+  устройстве», т.е. холодный старт, а не горячее обновление во время сессии).
+- Запись идёт исключительно штатным сериализованным путём (`_saveConfig` →
+  `_saveConfigDebounced` → `_writeConfig` → `_sendConfigCandidate` с
+  `expected_rev`) — новый канал не создан, конфликт/тост наследуются
+  бесплатно.
+- Все шесть UI-точек изменения стиля (`_decorSaveShape`, тулбар,
+  контекстный трей: цвет/непрозрачность, ширина, fill-чекбокс, fill-цвет,
+  ещё раз в `_renderDecorBar`) переведены на `_updateDecorStyle` — ни одна не
+  забыта (сверил построчно с прежними местами прямого присваивания
+  `host._decorStyle =` в диффе).
+- Трейлеры (`Issue: #377`, `User-Visible: yes/no`) на месте во всех трёх
+  коммитах; оба changelog и оба USER-GUIDE — в том же коммите, что
+  поведение (`ec76948b`), не отдельным «допишу потом».
+- Мутант-регистрация (`scripts/mutation-gate.mjs`) описывает ровно тот
+  дефект, который она ловит («seed-cut» — тихая деградация посева, приводит
+  к молчаливому нерабочему AC2/AC5; «debounce-cut» — спам записей),
+  соответствует формулировке в ТЗ «План автотестов».
+- «Один источник числа»: новый ключ не дублирует уже показываемое где-то
+  значение (это предпочтение по умолчанию для будущих объектов, а не
+  производная величина, показанная дважды) — `test/single-source-numbers.test.mjs`
+  зелёный, смысловых дублей не нашёл.
+
+## Чего не проверял
+
+- Полный `npm run golden:verify` и полный `demo/smoke_*.mjs` (все ~204) — не
+  оправдано объёмом правки; обоснование выбора — выше.
+- Полный HA backend harness (`test_ha_*.py`, `test_coordinate_canonicalization.py`)
+  — недоступен в этом окружении (нет `homeassistant`), это гейт CI/бета, не
+  код-ревью; расхождение не связано с #377.
+- Живое ручное тестирование в браузере (ручного цикла в процессе нет) —
+  замещено чтением кода и прогоном автотестов/смоков выше.
+- Гонка двух вкладок (штатный conflict #340) отдельно не воспроизводил —
+  путь записи переиспользует существующий механизм, для которого этот
+  сценарий уже покрыт в других смоках (`smoke_save_race.mjs`, не гонял —
+  слабая связь, поведение конфликта в этой задаче не менялось).
+- `import_export.py` полный интеграционный юнит именно на
+  `decor_default_style` (только чтение кода + существующий байт-в-байт юнит
+  схемы, см. AC6 выше).
+
+## Вердикт
+
+Все 7 AC доказаны (автотестом там, где это дешевле и надёжнее; чтением кода —
+там, где ТЗ само на это полагалось, с явной пометкой). High/Medium-находок
+нет, одна Low снята с записью. Гейты, соразмерные объёму задачи, зелёные.
+
+Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0
+
+Готово к очереди на пре-релиз.