docs: review document for #385

Issue: #385
User-Visible: no

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

@@ -0,0 +1,240 @@
+# CODE-REVIEW-385-r2
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/385
+- Заход: r2 · блокирующих циклов израсходовано до этого раунда: 0/4
+- Материал: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD` на SHA
+  `2e77fe09a554692bc23bf1f2497c2ae6c8c02059` (branch tip == материал ревью,
+  `origin/dev` = `fbbea475`, ветка полностью включает `dev` — `merge-base` совпадает
+  с `origin/dev`)
+- Спека: `docs/specs/385-audit-lows.md` (ревизия 2, SPEC-REVIEW-385-r2 зелёное)
+
+## Почему разбор полный, а не по дельте
+
+Пайплайн привёл ветку к `dev` до этого ревью: поверх легло 1 незнакомых коду
+r1 коммит(ов) `dev` (`fbbea475` — `ci: name the commit that actually broke
+golden`, класс B, только workflow, не пересекается с диффом задачи). Прежний
+r1 code-review (зелёный, `564bf419`) не смог слиться (конфликт), задача ушла в
+`S6-in-progress` на ребейз, и найденный SHA `564bf419` в текущей истории уже
+недостижим — ветка была перестроена. Согласно §7.2/§2.10 после ребейза это
+другой код, поэтому разбор — полный: весь диапазон `origin/dev...HEAD`
+перечитан заново, каждый AC1–AC6 перепроверен по коду и/или гейтом лично в
+этом раунде.
+
+## Скоуп диффа
+
+Класс A: `src/devices.ts`, `src/houseplan-editor-runtime.ts`,
+`custom_components/houseplan/import_export.py`.
+Класс B: `scripts/process-gate.mjs`, `scripts/mutation-gate.mjs` (+2 мутанта),
+`test/devices.test.mjs`, `test/process-gate.test.mjs`,
+`tests_backend/test_ha_import_export.py`, `demo/smoke_value_face_source.mjs`.
+Класс C: `docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`, `docs/images/screenshots.json`
+(fingerprint bump), `docs/reviews/CODE-REVIEW-385-r1.md` (артефакт r1,
+cherry-pick). Класс D: `custom_components/houseplan/frontend/**`, `dist/**` —
+байт-в-байт совпадают с локальной пересборкой, не рассматриваю отдельно.
+
+Один коммит `223f499e` несёт все четыре пункта (а)–(г) с трейлерами
+`Issue: #385` / `User-Visible: yes`; оба changelog в нём же. Коммит `2e77fe09`
+(документ r1, `User-Visible: no`) — публикационный, не продуктовый.
+
+## Как проверялось (гейты, лично на этом SHA)
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| typecheck | `npx tsc --noEmit` | чисто |
+| unit | `npm test` | 1592 pass / 0 fail / 1 skipped |
+| build + 3 копии бандла | `npm run build && cmp dist/houseplan-card.js custom_components/.../houseplan-card.js`; `npm run bundle:sync` | без diff, `git status --short` пуст после сборки |
+| check-docs | `node scripts/check-docs.mjs` | зелёный (7 файлов, 10 внешних ссылок); `sourceFingerprint` в `screenshots.json` уже обновлён под текущий `src/**` |
+| bundle:budget | `npm run bundle:budget` | initial View 277 995 / 300 000 Б gzip (запас 22 005 Б) — совпадает по порядку с заявленным автором |
+| no-new-any | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | 23 новых строки в 2 файлах, новых `any` нет |
+| smoke-select | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 33 слабые связи по общему имени `_markerDialog` (НЕОПРЕДЕЛЁННОСТЬ — ни одного прямого совпадения). Прогнал только именованный в AC `demo/smoke_value_face_source.mjs` — прямое совпадение по факту (это тот самый файл, который дифф меняет). Остальные 32 — общее имя `_markerDialog`, слабая связь; задача не задевает ничего вне диалога маркера и binding-выбора, прогон остальных не требовался |
+| smoke_value_face_source | `node demo/smoke_value_face_source.mjs` (после `bundle:sync`) | все 16 полей `true`, включая новые `sameBindingKeepsSource`, `sameVirtualKeepsSource` |
+| мутанты (новые, AC1/AC4) | `node scripts/mutation-gate.mjs --id=same-binding-click-resets-source`; `--id=release-proof-computed-for-every-commit` | оба **1/1 пойманы** |
+| golden:verify | — | не прогонял: дифф не меняет рендер/геометрию/стили (только обработчики кликов и данные), подтверждено — `imageSha256` во всех сценариях `screenshots.json` не изменились, поменялся только `sourceFingerprint` |
+| инварианты модели | — | не прогонял: дифф не трогает рёбра комнат, `layout`, `marker.space`, `open_spans`, толщину стен |
+| backend pytest | `python3 -m pytest tests_backend/test_ha_import_export.py -q` | **не завершился штатно**: в этом окружении нет `.venv-backend`/`homeassistant`/`voluptuous` — сборка теста падает на `import voluptuous as vol` ещё на этапе коллекции (не skip, а ImportError). AC5 закрыт **чтением кода**, не исполнением: см. раздел ниже |
+| py_compile (запасной для backend) | `python3 -m py_compile custom_components/houseplan/import_export.py tests_backend/test_ha_import_export.py` | чисто |
+
+## Разбор по AC (полный)
+
+**AC1** (клик по тому же кандидату в списке — no-op для value_source/badge,
+другой кандидат — сброс как раньше). `houseplan-editor-runtime.ts:12305-12325`:
+ранний `if (c.value === d.binding) { ...; return; }` перед сбросом. Мутант
+`same-binding-click-resets-source` (возвращает безусловный сброс) —
+**красный**, `smoke_value_face_source` — **зелёный** до и после мутации в
+верном направлении (ловится). Регрессия (клик по другому кандидату всё ещё
+сбрасывает) — поле `bindingResetToAuto`/`draftSourceExact`, оба `true`.
+**Доказано автотестом, тест умеет падать.**
+
+**AC2** (то же для «радио virtual» — повторный клик по уже активному
+virtual-binding не трогает value_source/badge). Реализовано симметрично AC1
+в `houseplan-editor-runtime.ts:12252-12260`: `if (d.binding === 'virtual') {
+...; return; }`. Смок добавил поле `sameVirtualKeepsSource`
+(`demo/smoke_value_face_source.mjs:176-183`) и оно зелёное.
+
+Но это доказательство **ложное** — проверено исполнением, а не только
+чтением. Я вручную заменил условие `if (d.binding === 'virtual')` на
+`if (false)` (эквивалент отсутствия фикса), пересобрал бандл
+(`bundle:sync`) и перезапустил `smoke_value_face_source.mjs`: все 16 полей,
+включая `sameVirtualKeepsSource`, остались **`true`** — мутация не поймана
+(проверено также отдельным вызовом `page.evaluate` в реальном Chromium этого
+проекта: клик по уже отмеченному `<input type="radio">` не порождает событие
+`change` вовсе, `count === 0` после двух `.click()` подряд). Причина
+структурная, не флак: в `_markerDialog` инвариант `bindingMode === 'virtual'
+⟺ binding === 'virtual'` держится во всех точках записи состояния
+(`:7657`, `:7730`, `:12262`, `:12278` — при уходе в `bindingMode: 'ha'`
+`binding` синхронно обнуляется, если был `'virtual'`). Значит `checked`
+радиокнопки всегда synced с `d.binding === 'virtual'`, а клик по уже
+отмеченной радиокнопке в браузере не эмитит `change` — ветка `if (d.binding
+=== 'virtual')` в обработчике **не может исполниться ни при каком реальном
+пользовательском клике**: это мёртвый код, а `sameVirtualKeepsSource`
+истинно независимо от того, есть guard или нет.
+
+Само поведение при этом корректно (сброса на «virtual» из «virtual» никогда
+не было и не может быть — по той же причине, по которой guard недостижим),
+поэтому пользовательского дефекта нет. Дефект — в доказательстве: спека
+(«Контракт (а)», раздел «Риски») прямо предписала чинить и радио-ветку с
+условием `d.binding === 'virtual'`, а хендофф автора заявил AC2 доказанным
+смоком — это не так, смок ничего не проверяет для этой ветки.
+
+**Medium (в скоупе задачи, не заводится отдельным issue — #202).** Ревьюер
+обязан либо получить тест, который умеет падать, либо честную запись
+«проверено чтением» (§2.7, правило 18) — здесь заявлено первое, а по факту
+ни то, ни другое: тест не падает, а чтением автор не подтвердил. Правка на
+выбор автора: (а) убрать недостижимый guard и строки `sameVirtualKeepsSource`
+из смока, заменив комментарий honest-запиской «эта ветка не нуждается в
+guard — see AC2 review», либо (б) оставить guard как документирующую
+защиту-на-будущее, но заменить claim AC2 в спеке/хендоффе на «проверено
+чтением: ветка недостижима через настоящий клик» вместо ложной ссылки на
+смок. Блокирующих последствий для пользователя нет — находка про честность
+доказательства AC, а не про баг.
+
+**AC3** (`rewriteMarkerControlReferences` не сажает `value_badge`/
+`value_source` как `undefined`). `src/devices.ts:889-898`: условные спреды.
+Юнит `test/devices.test.mjs` (новый тест) проверяет `'value_badge' in
+rewritten === false` и `'value_source' in rewritten === false` на голом
+маркере, и что существующий `value_source` по-прежнему переписывается.
+`npm test` зелёный. **Доказано автотестом**, падение проверено логически:
+без условного спреда старый код писал бы ключ безусловно (`{ ...marker,
+controls, value_badge: valueBadge, value_source: valueSource }`), что
+провалило бы `'value_badge' in rewritten === false` — тест умеет падать по
+построению (свойство `in` детектирует именно ключ с `undefined`).
+
+**AC4** (дорогая проверка гейта считается только для релизных коммитов,
+предикат — тот же, что в `makeCommit`). `scripts/process-gate.mjs:109-121`
+выносит `isReleaseCommit(subject, one)` целиком (оба дизъюнкта, включая
+`Release:`-трейлер) — используется и в `makeCommit` (:147), и в
+`parseRecords` (:169-172). Юнит со шпионом (`test/process-gate.test.mjs`,
+новый тест) гоняет 3 коммита (обычный / бета-приёмка с `Release:`-трейлером /
+стабильный релиз по subject) и проверяет: `isRelease` = `[false, true,
+true]`, вычислитель вызван **только** для двух релизных (`calls` содержит
+ровно их SHA), у нерелизного `releaseSourceViolations === null`. Мутант
+`release-proof-computed-for-every-commit` (возвращает безусловный вызов) —
+**красный**, `node --test test/process-gate.test.mjs` — зелёный. **Доказано
+автотестом, тест умеет падать.** Ровно закрывает M1 из SPEC-REVIEW-385-r1
+(узкий предикат считал бы бета-приёмку нерелизной и ронял бы pre-push/CI
+ложно).
+
+**AC5** (парная нейтрализация `value_badge`/`value_source` при экспорте,
+`dropped_marker_links == 2`). `custom_components/houseplan/import_export.py:504-527`
+— логика **не менялась** относительно `dev` (это тот же код, что уже был
+принят в #378/#379); диф добавляет только объясняющий комментарий (:504-511)
+и новый парный тест `tests_backend/test_ha_import_export.py:
+test_issue_385_space_export_drops_badge_and_value_face_links_together`.
+Прочитан построчно: маркер с внешним `value_badge.source.ref` **и** внешним
+`value_source.ref` — оба ветвления (:512-520 бейдж, :521-527 value_source)
+исполняются последовательно на одном и том же `marker`, каждое инкрементит
+`dropped_marker_links`; итог `== 2` соответствует тесту. `badge["enabled"] =
+False; badge["source"] = None` и `marker.pop("value_source", None)` дают
+именно те поля/отсутствие ключа, что проверяет тест.
+
+Не смог исполнить: в этом окружении нет `.venv-backend`/`homeassistant` —
+`python3 -m pytest tests_backend/test_ha_import_export.py` падает на
+`ImportError: No module named 'voluptuous'` уже на коллекции, HA-харнесс
+недоступен (только `py_compile`, который чист). **AC5 закрыт чтением кода,
+не исполнением** — заявляю это честно, а не как «verified». Риск невысокий:
+логика не менялась с версии, которую r1-код-ревью на `564bf419` уже гонял
+исполнением (`pytest tests_backend/` 446 passed) в среде с харнессом, а
+единственная новая часть — сам тест, чья корректность проверена построчным
+сравнением с кодом выше.
+
+**AC6** (полный гейт зелёный, бюджет ≈ 0). См. таблицу гейтов — все зелёные,
+бюджет 277 995 Б (запас 22 005 Б, дельта от 277 979 Б у автора — 16 Б, в
+пределах шума версии Rollup/окружения).
+
+## Закрытие раунда r1
+
+Документ r1: `docs/reviews/CODE-REVIEW-385-r1.md` (в дереве ветки, cherry-pick
+коммитом `2e77fe09`). Вердикт r1: зелёный, `564bf419`, High 0 / Medium 0,
+одна находка L1.
+
+| Находка r1 | Чем закрыта | Где видно |
+|---|---|---|
+| L1 (Low): вспомогательный `one(name)` в `process-gate.mjs` продублирован двумя реализациями (`makeCommit` через `matchAll`, `parseRecords` через `match`) — риск будущего дрейфа | Не закрыта, оставлена как есть — ревьюер r1 явно передал это на усмотрение автора вне скоупа AC4, не блокирует | `scripts/process-gate.mjs:129-130` (`all`/`one` через `matchAll`) и `:166` (локальный `one` через `match`) — оба выражения по-прежнему разные текстово, поведенчески идентичны, подтверждено новым юнитом AC4 (сверял лично) |
+
+r1 не содержал High/Medium — только это одно Low, поэтому таблица короткая:
+r1 не отправлял задачу на правки (зелёный вердикт цикла не расходует, #227),
+и переделывать было нечего — работу вернул только неудавшийся merge.
+
+## Унаследовано из r1 — не переоценивается, потому что переоценено с нуля
+
+Формально это полный, а не дельта-разбор (см. раздел выше), поэтому строгого
+«наследования без проверки» здесь нет: AC1, AC3, AC4, AC5, AC6 перечитаны и
+перепроверены гейтами заново в этом раунде на текущем SHA, а не приняты на
+слово из r1. Единственное, что действительно наследуется без повторного
+прогона, — сам факт, что r1 однажды провёл `pytest tests_backend/` целиком в
+среде с HA-харнессом и получил 446 passed / 1 skipped / 1 error (ошибка —
+известный флак `test_ha_upload.py::test_upload_ok`, не связан с диффом):
+это использовано выше как historical подтверждение для AC5 в дополнение к
+чтению кода, а не как замена сегодняшней проверки. Документ: r1, SHA
+`564bf419` (недостижим в текущей истории после ребейза, но зафиксирован в
+`docs/reviews/CODE-REVIEW-385-r1.md`).
+
+## Что проверено и корректно
+
+- Единственный коммит несёт оба трейлера, `User-Visible: yes` и правки в
+  оба changelog в нём же.
+- Изменение видимого поведения ограничено пунктом (а); (б)–(г) действительно
+  не меняют вывод для пользователя — подтверждено чтением и тестами.
+- Третьего места сброса value_source/badge при выборе binding нет: grep
+  `bindingMode:`/`binding: ` по всему файлу даёт ровно 4 точки записи
+  (`:7556` инбокс, `:7657`/`:7730` открытие диалога, `:12262`/`:12278`/`:12314`
+  переключатели) — все учтены, инвариант `bindingMode==='virtual' ⟺
+  binding==='virtual'` держится всюду.
+- Одно число — один источник: value_source/badge не создают дублирующего
+  отображения величины, эта задача не вводит новых видимых чисел.
+- Никакой новой геометрии, i18n, миграции конфига, touch-контракта не
+  затронуто — согласуется со спекой.
+
+## Чего не проверял и почему
+
+- **Полный `pytest tests_backend`** (HA-харнесс) — недоступен в этом
+  окружении (`.venv-backend` отсутствует, `homeassistant`/`voluptuous` не
+  установлены). Заменено чтением кода + `py_compile` + опорой на прежний
+  прогон r1 (см. «Унаследовано»). Это честно называемый пробел, не «verified».
+- **`golden:verify`** — дифф не меняет рендер (подтверждено неизменными
+  `imageSha256` в `screenshots.json`), пропуск обоснован §8.
+- **Инварианты модели** (`scripts/model-invariants.mjs`) — дифф не трогает
+  геометрию/`layout`/`marker.space`/toлщину, не запускал.
+- **31 из 33 «слабых» смоков** от `smoke-select.mjs` (общее имя
+  `_markerDialog`) — не прогонял: диф не касается ничего вне логики
+  binding-выбора и парных полей value_source/badge, которые целиком покрыты
+  прогнанным `smoke_value_face_source.mjs`.
+- **Полный `mutation-gate.mjs`** — прогонял только два новых мутанта
+  (`--id=`), не весь набор: он для этого диффа не требуется целиком (это
+  предрелизный/тяжёлый гейт), плюс полный прогон в этой песочнице падает на
+  несвязанном шаге `backend-test-guard.mjs` (`No module named pytest`) до
+  установки `pytest` — окружение, не регрессия диффа.
+- **`performance_smoke`** — в AC не назван, дифф не касается чувствительных
+  к перфу путей рендера (только обработчики кликов и статическая проверка
+  release-коммитов), не запускал.
+
+## Вердикт
+
+**Жёлтый.** Единственная находка — Medium (AC2, см. выше), в скоупе задачи,
+не блокирует пользователя, но нарушает дисциплину «тест умеет падать» для
+одного из шести AC и не позволяет мне подтвердить его честным «проверено
+чтением» вместо ложной ссылки на смок, которую несёт хендофф. High нет.
+Правка — на выбор автора (убрать недостижимый guard либо честно
+перезаявить способ доказательства), повторный цикл код-ревью обязателен
+(правило: правка по замечанию может задеть AC, который уже сочли
+выполненным, — перепроверяю прицельно AC1/AC2 в r3).