docs: review document for #377

Issue: #377
User-Visible: no

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

@@ -0,0 +1,223 @@
+# CODE-REVIEW-377-r2
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/377
+- Заход: r2 (номер — для имени документа; предыдущий код-ревью, r1, был
+  зелёным и цикл не тратил, см. §4/#227)
+- Ветка: `issue/377-decor-default-persist`, HEAD `82d710f1`
+- SHA материала ревью: `82d710f160d8572ef6687499c4b8acdc7995ed45`
+  (`git rev-parse HEAD`)
+- Диапазон: `git diff origin/dev...HEAD` — `origin/dev` = `720200e3`
+  (текущий тип), 43 файла, +1008/-488
+
+## Почему это ПОЛНЫЙ разбор, а не разбор по дельте
+
+Между r1 и r2 ветка была **ребейзнута на ушедший вперёд `dev`**: r1 вынесен
+на материале `2edaf1adac60f9939b836da87f5f1f549dab2548` — этого объекта в
+репозитории больше нет (`git cat-file -t` → `fatal: could not get object
+info`), он выпал при ребейзе. Автор сам зафиксировал это в issue сразу после
+публикации r1: «Пока шло ревью, `dev` продвинулся на 7 коммит(ов)... слияние
+приведёт ветку к dev, и это другой код (§7.2)» и следом провёл ребейз,
+затянувший в базу #373 (`feat: add tight house framing to space card`,
+`0d33691f`). Это ровно исключение из инструкции («ребейз на ушедший вперёд
+dev — после ребейза это другой код»), поэтому объём разбора в r2 — полный,
+а не только по находкам r1 (которых, забегая вперёд, и не было ни одной
+блокирующей).
+
+`git merge-base HEAD origin/dev` = `origin/dev` — ветка полностью лежит на
+текущей вершине `dev`, конфликтов нет, `git grep '<<<<<<<'` по дереву чист.
+
+## Скоуп
+
+Без изменений с r1: персист серверного ключа `settings.decor_default_style`
+(цвет/стиль декора «по умолчанию» из тулбара Background-редактора, #360) —
+переживает перезагрузку, общий для всех редакторов плана. Три поверхности:
+схема (`validation.py`), инициализация (`houseplan-card.ts`), запись
+(`houseplan-editor-runtime.ts`).
+
+## Как проверялось
+
+Два независимых источника уверенности, оба обязательны для полного разбора
+после ребейза:
+
+**1. Текстовое сличение продуктового кода #377 «было vs стало».** Для
+каждого из файлов, которые правит именно #377 (`validation.py`,
+`src/editors/decor/geometry.ts`, `src/houseplan-card.ts`,
+`src/houseplan-editor-runtime.ts`, `test/decor-style-persist.test.mjs`,
+`demo/smoke_decor_default_persist.mjs`, `tests_backend/test_validation.py`)
+взял текущий `git diff origin/dev...HEAD` построчно и сверил его с тем, что
+описывает и цитирует `docs/reviews/CODE-REVIEW-377-r1.md` (сам файл — часть
+этого диапазона, довезён ребейзом через cherry-pick, как и написал автор).
+Совпадает дословно: тот же маппинг 6 полей в `decorStyleFromSettings`/
+`decorStyleToSettings`, тот же `_seedDecorStyle` (единственный сдвиг —
+номера строк из-за более раннего кода, добавленного #373 в этом же файле),
+тот же `_updateDecorStyle`/`_persistDecorStyle` с дебаунсом 1000 мс и все
+шесть UI-точек, переведённые на него, та же схема `validation.py`, тот же
+набор из 10 мусорных вариантов + `"solid"` в pytest, тот же смок. Значит
+семантика правки не изменилась — только база под ней.
+
+**2. Проверка, что ребейз безопасен по существу**, а не только по
+отсутствию текстовых конфликтов: `#373` (единственный feat-коммит,
+затянутый в базу между r1 и r2) правит только `src/space-geometry.ts` — три
+новых экспорта (`resolveSpaceCardFit`, `itemOfGeometry`, `expandItem`,
+`structuralFrame`), все аддитивные, используются только `src/space-card.ts`.
+Ни один из файлов #377 (`houseplan-card.ts`, `houseplan-editor-runtime.ts`,
+`editors/decor/geometry.ts`, `validation.py`) не пересекается с диффом #373.
+Разобрано чтением: общих символов между двумя изменениями нет, путь записи
+декора (`_updateDecorStyle` → `_saveConfig` → `expected_rev`) и путь фрейминга
+space-card не имеют точек соприкосновения.
+
+**3. Гейты — прогнаны заново лично на актуальном SHA** (зелёного Validate
+на `82d710f1` нет — см. плашку задачи), поскольку старый прогон r1 стоял на
+объекте, которого больше не существует, и переносить его результат без
+повторного прогона на новой базе было бы недопустимо.
+
+### Гейты — прогнал сам
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Typecheck | `npx tsc --noEmit` | чисто |
+| Юниты | `npm test` | 1567 pass / 0 fail / 1 skipped (совпадает с заявкой автора после ребейза: «юниты 1567/0») |
+| Сборка | `npm run build` | ok, 9.6s |
+| Синхронизация бандлов | `npm run bundle:sync` | ok; `git status --porcelain` после — пусто (три копии дерева байт-в-байт совпадают с закоммиченными) |
+| Бюджет | `npm run bundle:budget` | initial View 277 310 Б / 300 000 (запас 22 690 Б) — байт-в-байт совпадает с заявкой автора после ребейза |
+| Доки | `node scripts/check-docs.mjs` | «Documentation checks passed (7 files, 10 external links)» — diff трогает `src/**` (в т.ч. через #373), гейт обязателен и зелёный. Отпечаток скриншотов (`sourceFingerprint`) обновлён коммитом `40ba222a` вместе с 10 канонических PNG — ожидаемо: любая правка `src/**` делает старый отпечаток устаревшим, а #373 такую правку внесла |
+| Backend (чистое подмножество) | `python -m pytest tests_backend -q --ignore=tests_backend/test_coordinate_canonicalization.py` (после `pip install pytest voluptuous`) | 235 passed, 1 skipped — совпадает с заявкой автора и с r1. `test_coordinate_canonicalization.py` падает на отсутствующем `homeassistant` при коллекции — не связано с #377, гейт CI |
+| Единственный источник числа | `node --test test/single-source-numbers.test.mjs` | 3/3 pass |
+| Мутанты (только новые, точечно `--id=`) | `mutation-gate.mjs --id=decor-default-style-seed-cut` и `--id=decor-default-style-debounce-cut` | оба «поймано 1 из 1» — тесты умеют падать |
+
+Полный `mutation-gate.mjs` без `--id=` не гонял — та же причина, что в r1:
+несоразмерно точечной правке, к тому же в этом окружении падает на
+несвязанном backend-guard'е (нет `homeassistant`).
+
+### Смоки — `scripts/smoke-select.mjs --base origin/dev --head HEAD`
+
+Инструмент на актуальном диапазоне дал те же 7 «прямых совпадений» и те же
+13 «слабых связей» (единое имя `_saveConfig`), что и в r1 — состав диффа
+#377 не изменился, поэтому выбор не изменился.
+
+Прогнал все 7 прямых совпадений — все OK: `smoke_decor_default_persist`
+(AC4/AC5), `smoke_bg_color`, `smoke_color_picker`, `smoke_decor`,
+`smoke_dialog_zombie`, `smoke_furniture`, `smoke_general_settings`.
+
+Из 13 слабых связей прогнал `smoke_config_writer` (та же, что в r1 и что
+заявлял автор) — OK. Остальные 12 не гонял: diff #377 по-прежнему не трогает
+геометрию/стены/толщину, а #373 (единственное новое в базе) отдельно
+проверено на отсутствие пересечения файлов (см. выше) — обоснование
+переносится из r1 без изменений. `npm run invariants` не требовался — diff
+#377 не трогает рёбра комнат, `layout`, `marker.space`, `open_spans` или
+записи толщины.
+
+`npm run golden:verify` не гонял: продуктовый код #377 не рендерит View
+(`_decorStyle` управляет только дефолтом для новых объектов декора/мебели в
+редакторе, не View) — это не изменилось с r1. Визуальные изменения в
+диапазоне (10 канонических PNG) целиком принадлежат #373 (fit-to-content
+пространства) и уже прошли отдельный цикл ревью этой задачи (`docs: review
+document for #373`, трижды в истории `dev`) — повторно ревьюить чужой
+змерженный и принятый результат в рамках #377 не требуется.
+
+## Закрытие раунда r1
+
+| Находка r1 | Чем закрыта | Где видно |
+|---|---|---|
+| Low: `_updateDecorStyle` заводит свой `setTimeout(1000)` и не флашит его в `disconnectedCallback` | Не чинилась — r1 снял находку без правки как приемлемый риск (цена ошибки мала, штатный `disconnectedCallback` не единственный путь доставки таймера) | `houseplan-editor-runtime.ts:9403-9411` (номера строк сдвинуты ребейзом, код идентичен); решение зафиксировано в `docs/reviews/CODE-REVIEW-377-r1.md`, раздел «Находки» |
+
+High/Medium-находок в r1 не было — таблица закрытия по ним пуста, возврата
+на правки не было.
+
+## Унаследовано из r1
+
+Полностью, без переоткрытия по существу, наследуется анализ AC1–AC7 и
+раздел «Что проверено и корректно» из `docs/reviews/CODE-REVIEW-377-r1.md`
+(SHA материала того ревью: `2edaf1adac60f9939b836da87f5f1f549dab2548`,
+документ смёржен в ветку cherry-pick'ом при ребейзе) — построчная сверка
+выше (раздел «Как проверялось», пункт 1) подтвердила, что продуктовый код
+#377 между r1 и r2 не изменился ни на символ, изменилась только база под
+ним. В частности без повторной сверки по существу приняты:
+
+- маппинг snake_case↔camelCase в `decorStyleFromSettings`/
+  `decorStyleToSettings` и соответствие ТЗ п.7;
+- логика однократного посева `_seedDecorStyle` из кэша и первого серверного
+  ответа, не переоткрывающая живую сессию;
+- то, что запись идёт исключительно штатным сериализованным путём
+  (`_saveConfig` → `_saveConfigDebounced` → `_writeConfig` →
+  `_sendConfigCandidate` с `expected_rev`);
+- то, что все шесть UI-точек изменения стиля переведены на
+  `_updateDecorStyle` (сверка «ни одна не забыта» из r1);
+- разбор AC6 через чтение `import_export.py` (путь `settings` для
+  non-same-source импорта не трогает `decor_default_style`);
+- разбор AC7 (недостижимость в холодном View — код рантайма грузится лениво
+  и нигде вне него `_decorStyle` не пишется).
+
+Не наследуется, а перепроверено заново в r2: все автоматические гейты (иной
+SHA — не тот же прогон), совместимость с #373 (новое в базе, r1 об этом
+знать не мог), актуальность отпечатка скриншотов документации.
+
+## Находки
+
+Новых находок в r2 нет. High/Medium — 0.
+
+**Low (унаследована из r1, по-прежнему снята без правки).** См. таблицу
+«Закрытие раунда r1» выше — код не менялся, довод не изменился.
+
+**Наблюдение вне находок.** Коммит `40ba222a` (`build: refresh bundle trees
+for #377; align the color-picker contract`) помимо ожидаемого пересбора
+бандлов и обновления `test/color-picker.test.mjs:67` (замена ожидаемого
+паттерна `this._decorStyle = {...}` на `this._updateDecorStyle({...})` —
+тест синхронизирован с уже существующим кодом `feat`-коммита, а не наоборот:
+сверено, что `houseplan-editor-runtime.ts` действительно вызывает
+`_updateDecorStyle` в этом месте с самого `feat`-коммита `10a1a266`). Это
+задним числом закрывает пробел покрытия, оставленный в `feat`-коммите
+(тест не был обновлён вместе с кодом), но не меняет поведение и не
+затрагивает AC — не поднимаю до Low, так как ошибки не произошло: `npm
+test` был бы красным до этого коммита, и это увидели бы до публикации
+ветки на ревью, что и произошло (гейт-перегон автора после ребейза).
+
+## Что проверено и корректно (специфично для r2)
+
+- Ребейз чист: `git merge-base HEAD origin/dev` = `origin/dev`, маркеров
+  конфликта нет.
+- Диапазон #377 (файлы, диффы, коммиты) не пересекается по файлам с #373 —
+  проверено построчным чтением диффа #373.
+- Все гейты, зелёные в r1, остаются зелёными на новой базе; числовые
+  показатели (юниты, backend, бюджет бандла) совпадают с тем, что заявил
+  автор в двух комментариях после ребейза («юниты 1567/0», «бюджет 277
+  310/300 000»).
+- Трейлеры (`Issue: #377`, `User-Visible`) на месте во всех 4 коммитах
+  диапазона; `feat`-коммит (`User-Visible: yes`) правит оба changelog и оба
+  USER-GUIDE в себе же, не отдельным коммитом.
+- `docs/reviews/CODE-REVIEW-377-r1.md` присутствует в дереве как обычный
+  файл истории ревью, довезённый ребейзом — само по себе не находка.
+
+## Чего не проверял
+
+- Заново — семантику продуктового кода #377 построчно «с нуля»: сверил
+  текстовым сличением с уже проверенным в r1 кодом (см. «Как проверялось»,
+  п.1) вместо повторного независимого прочтения каждой строки — обоснование
+  выше (§7.2 требует полноты разбора при таком ребейзе, но не требует
+  игнорировать то, что код внутри диапазона #377 доказуемо не менялся).
+- Визуальную корректность фрейминга space-card (#373) — чужая, уже принятая
+  задача, вне скоупа #377.
+- Полный `npm run golden:verify` и полный `demo/smoke_*.mjs` (все ~204) —
+  не оправдано объёмом правки #377; обоснование — выше.
+- Полный HA backend harness (`test_ha_*.py`,
+  `test_coordinate_canonicalization.py`) — недоступен в этом окружении (нет
+  `homeassistant`), гейт CI/бета, расхождение не связано с #377.
+- Живое ручное тестирование в браузере — замещено чтением кода и прогоном
+  автотестов/смоков.
+- Гонку двух вкладок (#340) — не тронута этой задачей, отдельно не
+  воспроизводил, как и в r1.
+
+## Вердикт
+
+Ребейз на ушедший вперёд `dev` потребовал полного разбора (§7.2): проведена
+как текстовая сверка продуктового кода #377 «было vs стало» (не изменился ни
+на символ), так и проверка отсутствия пересечения с тем, что реально
+привнёс новый `dev` (#373 — аддитивные, несвязанные файлы). Все 7 AC
+доказаны — унаследовано из r1 с подтверждением, что доказательство всё ещё
+соответствует коду. Все гейты, соразмерные объёму задачи, прогнаны заново на
+актуальном SHA и зелёные. High/Medium-находок нет, одна Low унаследована из
+r1 и остаётся снятой без правки.
+
+Вердикт: зелёный · заход r2 · блокирующих циклов 0/4 · High: 0 · Medium: 0
+
+Готово к очереди на пре-релиз.