docs: review document for #377

Issue: #377
User-Visible: no

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

@@ -0,0 +1,248 @@
+# SPEC-REVIEW-377-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/377 — «Персист цвета
+  декора по умолчанию в серверный конфиг»
+- Этап: ТЗ на ревью (PROCESS.md §2.4), **полный трек** (выделено из #376в по
+  SPEC-REVIEW-376-r1 H1: новый персистентный ключ `settings.decor_default_style`
+  + кросс-модульный путь записи — критерий §5 «нет новых compatibility-полей»
+  не проходит)
+- ТЗ: `docs/specs/377-decor-default-persist.md`, ревизия 1, зафиксирована
+  комментарием автора на dev `e2e4cec1`
+- Материал ревью: тело issue #377 + оба комментария (аналитика §2.2, объявление
+  ревизии 1) сверены с `dev` на `e2e4cec1`
+- Заход: r1 · блокирующих циклов израсходовано **0 из 4** (полный трек)
+
+## Скоуп ревью
+
+Первый заход — предыдущего вердикта по #377 нет, разбор полный. Проверялось:
+соответствие `docs/SCOPE.md`; корректность классификации полного трека (уже
+установлена предшествующим SPEC-REVIEW-376-r1, проверена повторно на
+согласованность); наличие всех обязательных разделов §7.1; однозначность и
+доказуемость каждого AC; отсутствие догадок, выданных за факт — каждое
+техническое утверждение ТЗ о текущем коде сверено построчно с исходниками, а
+не принято на слово.
+
+Диапазон правки этого раунда — файл `docs/specs/377-decor-default-persist.md`
+(добавлен, продуктовый код не менялся): `git diff --stat 4eede5bc..e2e4cec1`
+показывает только два review-документа (#375, #376) и новый файл ТЗ.
+
+## Как проверялось
+
+1. Прочитаны целиком `docs/SCOPE.md`, `PROCESS.md` (§2.4, §2.9/§4 бюджет
+   циклов, §5 критерии трека, §7.1 обязательные разделы, §12 запреты), сверены
+   с сохранённым в памяти пониманием процесса.
+2. Прочитано тело issue #377 и оба комментария (аналитика, объявление ревизии).
+3. Прочитан весь текст `docs/specs/377-decor-default-persist.md`.
+4. Прочитан предшествующий `docs/reviews/SPEC-REVIEW-376-r1.md` целиком —
+   проверить, что классификация полного трека и содержание находок этого
+   документа корректно унаследованы новым ТЗ, а не переизобретены.
+5. Каждое фактическое утверждение ТЗ о текущем коде сверено с `dev`:
+   - `src/houseplan-card.ts:984` — `private _decorStyle: DecorStyle = {
+     ...DEFAULT_DECOR_STYLE }` — подтверждено, только in-memory поле.
+   - `src/houseplan-editor-runtime.ts:4481, 5306-5419` — оба места записи
+     (тулбар и контекстный трей) действительно пишут в один и тот же
+     `this.host._decorStyle` — подтверждено.
+   - `src/houseplan-editor-runtime.ts:1764` — `_writeConfig()` сериализованный
+     путь записи, использующий `enqueueSerializedWrite`/`_sendConfigCandidate`
+     — подтверждено, это общий канал, новый не создаётся.
+   - `src/houseplan-editor-runtime.ts:9403-9417` (`_saveSettingsDialog`) —
+     подтверждён паттерн «дефолт = отсутствие ключа» для `bg_color`/
+     `fill_colors`, на который ссылается контракт п.5.
+   - `custom_components/houseplan/validation.py:1883` — `vol.Optional(
+     "settings", default=dict): vol.Schema({...}, extra=vol.ALLOW_EXTRA)` —
+     номер строки и `ALLOW_EXTRA` подтверждены дословно; используется как
+     основание раздела «Откат» (старый бэкенд/фронт не ломается на новом
+     ключе) — корректно.
+   - `custom_components/houseplan/validation.py:1307-1330` (`_DECOR_COMMON`,
+     rect/ellipse) — диапазоны, предложенные для схемы
+     `decor_default_style` (opacity 0..1, width_cm 0.1..100, fill_opacity
+     0..1), дословно совпадают с уже принятыми диапазонами для decor-фигур —
+     не придуманы заново.
+   - `custom_components/houseplan/import_export.py:1145-1168` (`build_space_merge`)
+     и `:1660-1670` (полный импорт) — подтверждено: пространственный импорт
+     копирует `target_config = _json_copy(current_config)` и не трогает
+     глобальный `settings`, полный импорт переносит `settings` целиком за
+     вычетом `known_devices`/`new_device_ids` только при `!same_source` —
+     заявление «settings переносится целиком» для AC6 корректно в заявленном
+     объёме (полный экспорт/импорт, не пространственный).
+   - `src/editors/decor/geometry.ts:144` — `width_cm: Math.max(0.1, Math.min(
+     100, Number(style.widthCm) || 0.1))` — существующий явный код конвертации
+     из `DecorStyle.widthCm` (camelCase) в персистентное поле `width_cm`
+     (snake_case). Это прямо противоречит утверждению ТЗ (см. находку M1
+     ниже).
+   - `src/editors/decor/types.ts:78-85` — интерфейс `DecorStyle` объявлен как
+     `{ color, opacity, widthCm, fill, fillColor, fillOpacity }` —
+     **camelCase**, не snake_case, как утверждает ТЗ (см. M1).
+   - `docs/CHANGELOG.ru.md` (запись #360) — подтверждён факт существования
+     тулбар-пикера «основной цвет и прозрачность», на который опирается
+     раздел «Сценарий».
+   - `demo/smoke_*.mjs`, `debounce()` (`src/houseplan-editor-runtime.ts:722`) —
+     подтверждена техническая осуществимость дебаунс-записи (используемый
+     примитив уже существует в модуле), план автотестов не требует нового
+     инфраструктурного решения.
+6. Проверено отсутствие незакрытых меток неопределённости («уточнить», TBD,
+   голый «?») — не найдено.
+7. Проверена структура на соответствие обязательному списку §7.1: сценарий ·
+   что человек увидит до/после · проблема · скоуп/не-скоуп · контракт
+   поведения · UX · модель данных и миграция · i18n · AC1…AC7 с доказательством
+   · план автотестов · риски · откат · release-артефакты — все присутствуют.
+8. Гейты (`tsc`/`test`/`build`/`check-docs`/инварианты/смоки) не прогонялись:
+   diff этого раунда — только новый файл в `docs/specs/`, продуктовый код
+   (`src/**`, `custom_components/**`) не тронут, гейтам нечего было бы
+   доказать (тот же вывод, что и в SPEC-REVIEW-376-r1 для аналогичной стадии).
+
+## Находки
+
+### M1 (Medium, в скоупе) — контракт называет формат ключа `DecorStyle` неверно и не как предположение, а как проверенный факт
+
+**Файл:** `docs/specs/377-decor-default-persist.md`, раздел «Контракт
+поведения», пункт 7.
+
+Текст: «Формат ключа — snake_case, поля фронтового `DecorStyle`
+(`src/editors/decor/types.ts:78`): `color`, `opacity`, `width_cm`, `fill`,
+`fill_color`, `fill_opacity`.»
+
+Это не так: `src/editors/decor/types.ts:78-85` объявляет `DecorStyle` в
+**camelCase** — `color, opacity, widthCm, fill, fillColor, fillOpacity`.
+Snake_case-именование (`width_cm`, `fill_color`, `fill_opacity`) относится к
+персистентным полям фигур (`DecorBase`/`DecorRect` в том же файле, строки
+10-52), не к `DecorStyle`. Существующий код уже явно разводит эти две
+конвенции: `src/editors/decor/geometry.ts:144` содержит ручную конвертацию
+`width_cm: Math.max(0.1, Math.min(100, Number(style.widthCm) || 0.1))` именно
+потому, что `DecorStyle.widthCm` и персистентное `width_cm` — разные
+идентификаторы.
+
+Показательно, что ровно эта развилка (snake/camelCase формат нового ключа)
+в предшественнике этой задачи была корректно оформлена как явное
+предположение: SPEC-REVIEW-376-r1, раздел «Что проверено и корректно» —
+«Раздел «Принятые предположения» корректно выносит технические, не
+наблюдаемые пользователем решения (**формат ключа snake/camelCase**, путь
+записи только из runtime-редактора) в явный блок». В нынешнем ТЗ #377 такого
+раздела «Принятые предположения» нет вообще, а тот же вопрос вместо этого
+подан как проверенный факт со ссылкой на конкретную строку — и эта ссылка,
+при сверке, факт не подтверждает.
+
+**Почему это находка, а не мелочь.** AC2 требует «конфиг с ключом →
+`_decorStyle` = мердж ключа поверх `DEFAULT_DECOR_STYLE`», а AC1/AC6 фиксируют
+формат серверного ключа как snake_case. Между snake_case-ключом настроек и
+camelCase-полем `_decorStyle` нужен явный маппинг (`width_cm ↔ widthCm`,
+`fill_color ↔ fillColor`, `fill_opacity ↔ fillOpacity`); без него код,
+написанный «по букве» текущего пункта 7 (взять ключи настроек и наложить их
+на `_decorStyle` как если бы имена совпадали), молча не будет мержить три из
+шести полей — регресс ровно на тех полях, где имя меняется. Пока
+формулировка стоит как «факт со ссылкой», а не как предположение, у автора
+кода нет сигнала, что это место требует отдельного решения.
+
+**Чем чинится в этой же задаче:** один абзац, например: «Ключ настроек
+snake_case (`color`, `opacity`, `width_cm`, `fill`, `fill_color`,
+`fill_opacity`) конвертируется в camelCase-поля `DecorStyle`
+(`color`, `opacity`, `widthCm`, `fill`, `fillColor`, `fillOpacity`) по прямому
+соответствию имён — по аналогии с уже существующей конвертацией в
+`src/editors/decor/geometry.ts:144`. Принято предположительно, поменять
+свободно.» Ссылку на `types.ts:78` при этом стоит поправить на то, что там
+реально объявлено (camelCase), либо убрать как источник утверждения о
+snake_case.
+
+## Что проверено и корректно
+
+- Классификация полного трека обоснована и унаследована корректно: причина
+  («новый персистентный ключ + кросс-модульный путь записи», критерий §5 «нет
+  новых compatibility-полей» не пройден) совпадает с находкой H1
+  SPEC-REVIEW-376-r1, ТЗ прямо на неё ссылается, а не переизобретает
+  рассуждение заново.
+- Оба Medium предшественника (SPEC-REVIEW-376-r1 M1 «нет раздела откат», M2
+  «нет AC на текстовые пункты») к #377 не относятся — они были про пункты
+  (а/б/д/е) исходного #376, которые остались в #376 на `small`; #377 —
+  только пункт (в), и его собственный раздел «Откат» и AC1…AC7 присутствуют
+  полностью, каждый со способом доказательства (`unit`/`pytest`/`смок`/
+  `гейт`).
+- Задача в скоупе `docs/SCOPE.md`: полиш уже принятой фичи #360 (главный
+  тулбар), не открывает новый Core user job, не входит в «Out of scope»;
+  соответствует «Design consequence» — правка касается только
+  Background-редактора (admin-only поверхность), View не получает новых
+  путей записи.
+- Продуктовых вопросов владельцу не осталось: решение «персистить в
+  серверный конфиг» уже принято владельцем 29.08 и зафиксировано в теле
+  issue; ТЗ на него ссылается, а не переизобретает.
+- Сценарий и «что человек увидит до/после» отвечают на оба обязательных
+  вопроса §7.1 конкретно, без терминов реализации, и соответствуют
+  фактическому поведению тулбар-пикера #360 (подтверждено записью в
+  CHANGELOG.ru.md).
+- Контракт поведения (пп. 1-6, кроме уже описанного в M1 п.7) построчно
+  сверен с кодом и подтверждён: легаси-путь без ключа, дебаунс через
+  существующий примитив, единственный сериализованный канал записи
+  `_writeConfig`/`expected_rev` (#340), удаление ключа при возврате к
+  дефолту по образцу `bg_color`/`fill_colors`, запись только из
+  editor-runtime (холодный View класса #357 не задет — подтверждено: чтения
+  `_decorStyle` в `houseplan-card.ts` ограничены превью черновика/мебели,
+  которые в View не рендерятся).
+- Диапазоны валидации в наброске схемы (`opacity` 0..1, `width_cm` 0.1..100,
+  `fill_opacity` 0..1) не придуманы заново, а дословно совпадают с уже
+  принятыми диапазонами для decor-фигур той же кодовой базы — снижает риск,
+  что AC1 окажется неисполнимым или несогласованным с соседней схемой.
+- AC6 (импорт/экспорт byte-в-byte) корректно ограничен по объёму: заявление
+  подтверждено для полного экспорта/импорта; пространственный
+  (`build_space_merge`) импорт глобальный `settings` не трогает вовсе, что не
+  противоречит контракту (ключ глобальный, не per-space) и не заявлено в AC6.
+- i18n корректно отмечен как незадетый — в UI не добавляется новый текст,
+  проверка существующим паритет-гейтом достаточна.
+- Риски названы конкретно вместе со снимающим механизмом (гонка вкладок →
+  штатный conflict #340 + дебаунс; захламление settings → дебаунс + удаление
+  ключа при дефолте; обратная совместимость → опциональность в обе стороны),
+  а не общими словами.
+- Release-артефакты (CHANGELOG.md+.ru.md, USER-GUIDE.ru.md) названы по
+  правилу `docs/specs/README.md` — ключ меняет пользовательское поведение
+  (переживание перезагрузки), артефакты перечислены явно.
+
+## Чего не проверял
+
+- Гейты (`npx tsc --noEmit`, `npm test`, `npm run build`, `check-docs.mjs`,
+  инварианты модели, browser-смоки) — не прогонял: diff этого раунда не
+  затрагивает `src/**` или `custom_components/**`, только новый файл
+  `docs/specs/377-decor-default-persist.md`. Гейтам, завязанным на код,
+  нечего было бы доказать на стадии ТЗ (тот же вывод сделан в
+  SPEC-REVIEW-376-r1 для той же стадии).
+- Реализуемость дебаунса «1000 мс» и точную формулировку будущего
+  предложения в `USER-GUIDE.ru.md` — контракт называет смысл и число, не
+  финальный текст промо-абзаца; это в пределах допустимой недосказанности
+  для технической/текстовой детали, не находка.
+- Согласованность нового ключа `settings.decor_default_style` с
+  `scripts/config-field-registry.mjs` — реестр по `docs/CONFIG-COMPATIBILITY.md`
+  прямо заявлен как неполный («initially covers the known compatibility and
+  internal-field debt... not yet the complete canonical schema»); регистрация
+  нового (не легаси) поля не требуется этим документом и станет предметом
+  код-ревью, если понадобится.
+- Точный будущий API взаимодействия дебаунс-таймера с существующей очередью
+  `_writeChain`/`enqueueSerializedWrite` при одновременном геометрическом
+  изменении — техническая деталь реализации, агенты решают её сами (§7.1),
+  не наблюдаема пользователем и не входит в критерии готовности ТЗ.
+
+## Низкая находка (снята с записью)
+
+- Раздел «UX»/«Риски» не содержит явной фразы о влиянии на touch-контракт
+  (`docs/TOUCH-SUPPORT.md`), которую требует чек-лист DoR §2.5. По существу
+  влияние действительно нулевое: задача не добавляет новых элементов UI
+  («Ничего нового в UI не появляется», уже добавленные тулбар/трей-пикеры
+  #360 не меняются), поэтому вывод «нет влияния» тривиально следует из уже
+  написанного текста. Снимаю без правки, а не как повод для цикла — но
+  фиксирую, чтобы автор при следующей правке ТЗ добавил явную строку «touch:
+  нет» для формального прохождения DoR-чек-листа без повторной проверки
+  ревьюером.
+
+## Вердикт
+
+ТЗ структурно полное (все обязательные разделы §7.1 на месте), классификация
+полного трека унаследована корректно от SPEC-REVIEW-376-r1 и не оспаривается,
+почти все технические утверждения о текущем коде подтвердились построчно.
+Единственная находка — M1: контракт называет формат ключа `DecorStyle`
+snake_case со ссылкой на конкретную строку кода, которая при проверке
+показывает обратное (camelCase), и не помечает это место как предположение,
+хотя тот же вопрос в предшествующем ТЗ (#376) был корректно оформлен именно
+как явное предположение. Без исправления этого пункта реализация,
+написанная «по букве» контракта, рискует не замержить три из шести полей
+стиля при чтении настроек — то есть ровно то поведение, которое проверяет
+AC2. High-находок нет, Medium — один, в скоупе, чинится в этой же задаче
+одним абзацем без нового цикла по существу решения (только техническая
+правка формулировки ТЗ).
+
+`Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 1 → в задаче`