docs: review document for #341

Issue: #341
User-Visible: no

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

@@ -0,0 +1,146 @@
+# CODE-REVIEW — issue #341 · заход r1
+
+## Скоуп
+
+Issue #341 (трек `trivial`, метки `bug`/`P3`/`polish`): аудит C5 нашёл, что
+`src/labs.ts` защищает подписку на `hashchange`/`popstate` только
+модульным флагом `listening`. Home Assistant может перезагрузить ресурс
+карточки без перезагрузки страницы (обновление через HACS, dev-режим,
+повторная регистрация ресурса) — новая инстанция модуля не видит старый
+флаг и добавляет вторую пару обработчиков на тот же `window`, что даёт
+повторную обработку одного события.
+
+AC1–AC3 из тела issue:
+- AC1/AC2 — после двух инициализаций Labs в одном `window` одно событие
+  `hashchange`/`popstate` вызывает обработчик только актуальной инициализации.
+- AC3 — обычная одиночная инициализация и разрешение флагов не меняются.
+
+Диапазон: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD`.
+Коммиты:
+- `5f6bddea` fix: deduplicate Labs location listeners — `src/labs.ts`,
+  `test/labs.test.mjs`.
+- `2b6db4c8` docs: refresh screenshot fingerprint — `dist/**`,
+  `custom_components/houseplan/frontend/**`, `docs/images/**`,
+  `docs/images/screenshots.json` (следствие правки `src/**`, не отдельная
+  задача).
+
+Продуктовая рамка (SCOPE.md): фикс внутренний, к Core user jobs не
+привязан напрямую — это защита контракта Labs («One module-level browser
+listener; card instances only subscribe to its snapshot»), которым
+управляется presentation-only флаг `iso` (#89, J1-презентация). Правка не
+меняет UX, URL/storage-грамматику Labs, жизненный цикл карточки — всё
+согласно разделу «Не входит» issue. Изменение точечное: только
+`src/labs.ts` (class A) + `test/labs.test.mjs` (class B) + сгенерированные
+class D/C артефакты. Постороннего кода в диффе нет.
+
+## Как проверялось
+
+Дешёвые гейты на SHA `2b6db4c8` подтверждены Validate
+(https://github.com/Matysh/houseplan-card/actions/runs/33168458678, success) —
+`npx tsc --noEmit`, `npm test`, `npm run build` со сверкой бандла не
+перегонял.
+
+Сам прогнал точечно (диапазон правки узкий, стоимость минимальна):
+
+- `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs &&
+  node --test test/labs.test.mjs` → **green**, 6/6, включая новый тест
+  «a reloaded Labs module replaces both browser location listeners».
+- **Тест умеет падать**: временно убрал вызов
+  `window.__hpLabsListenerCleanup?.()` из собранного `test-build/labs.js`
+  (эмуляция кода до фикса) и перезапустил — новый тест краснеет
+  (`2 !== 1`, ожидание `firstPublishes === 1`, фактически `2`, т.к.
+  обработчик снятой инстанции продолжает публиковать на `hashchange`).
+  Откатил файл обратно, `git status` подтвердил чистое дерево.
+- `node scripts/check-docs.mjs` → **green** (7 файлов, 10 внешних ссылок) —
+  обязателен, т.к. диф трогает `src/**`.
+- Байт-в-байт сверка `dist/**` ↔ `custom_components/houseplan/frontend/**`
+  (`houseplan-card.js`, `houseplan-assets.json`, все файлы
+  `houseplan-assets/*`) → идентичны, `bundle:sync` не разошёлся.
+- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` →
+  **НЕОПРЕДЕЛЁННОСТЬ**: 1 файл `src/**`, ни один из 196 смоков не связан
+  доказуемо; символ без покрытия — `onLocationChange`. Решение: не
+  прогонять браузерные смоки — `onLocationChange` не рендерит и не меняет
+  DOM/маршрутизацию, это чистая функция диспетчеризации на
+  `publishBrowserLabs`, а контракт «ровно одна активная пара обработчиков»
+  полностью воспроизведён и провёрен модульным тестом с двумя реальными
+  ES-module инстанциями (через `import(...'?listener-instance=...')`),
+  что ближе к реальному сценарию (повторная загрузка ресурса), чем любой
+  браузерный смок мог бы дать.
+- Чтение кода (`src/labs.ts:147–212`): прослежен путь `ensureBrowserLabs` →
+  `window.__hpLabsListenerCleanup` для 1, 2 и 3 последовательных
+  инстанций — каждая новая инстанция снимает ровно предыдущую пару
+  обработчиков (через замыкание `cleanup`, ссылающееся на её же
+  `onLocationChange`) и кладёт на `window` свой `cleanup`; guard
+  `if (window.__hpLabsListenerCleanup === cleanup)` предотвращает
+  случайное затирание чужого cleanup при непоследовательном вызове.
+  Повторный вызов `ensureBrowserLabs` в РАМКАХ ОДНОЙ инстанции (обычный
+  случай — несколько карточек на одной странице) не трогает `if (!listening)`
+  вовсе — поведение AC3 не изменено кодом, а не только заявлением.
+
+Не прогонял: `npm run golden:verify` (диф не меняет рендер/геометрию/стили —
+`onLocationChange` не рисует ничего), `npm run invariants` (геометрия,
+`layout`, толщина стен, `open_spans` не затронуты), `python -m pytest
+tests_backend` (`custom_components/**/*.py` не менялся),
+performance-профили (не названы в AC, путь не в горячей точке рендера).
+`npm run bundle:budget` не перегонял — автор привёл число
+(256046 B при бюджете 282000 B) и разница от `iso`-строки правки
+незначительна; независимо сверил байт-идентичность копий бандла, что
+покрывает риск рассинхронизации сильнее, чем повторный бюджет-прогон.
+
+## Находки
+
+Нет находок High или Medium. Мелких Low-находок, требующих правки или
+явного отказа, не нашёл — код и тест соответствуют предложенному в issue
+решению («хранить состояние подписки на `window` под приватным глобальным
+ключом») буквально.
+
+## Что проверено и корректно
+
+- **AC1/AC2 доказаны автотестом**, тест умеет падать (см. выше) — это не
+  «verified» на честном слове, а воспроизведённый красный/зелёный цикл.
+- **AC3** — код, отвечающий за одиночную инициализацию, не тронут
+  (`if (!listening)` guard и его тело для случая true остаются пустой
+  веткой); резолвер флагов (`resolveLabs`, `liveLabsFlags` и т.д.)
+  вообще не в диффе — старые тесты резолвера все зелёные без изменений.
+- Три и более последовательных инстанций разобраны построчно — не только
+  «два», как в тесте, — инвариант «ровно одна активная пара» держится по
+  индукции для N инстанций.
+- `window.__hpLabsListenerCleanup` — новое глобальное свойство описано в
+  `declare global`, именование согласовано с существующим `__hpLabs`,
+  двойное подчёркивание держит приватность соглашения.
+- Labs-правила AGENTS.md соблюдены: изменение не вводит новый флаг, не
+  трогает грамматику URL/storage, не гейтит данные/HA-действия —
+  чисто presentation-инфраструктура регистрации слушателей.
+- Трейлеры обоих коммитов корректны: `Issue: #341`, `User-Visible: no` —
+  согласился с оценкой автора: фикс невидим при обычном использовании
+  (устраняет задвоение реакций на скрытый Labs-флаг `iso`, а не меняет
+  какое-либо наблюдаемое поведение), changelog обоснованно не трогался.
+- Байты бандла (`dist` ↔ integration copy) и docs-фингерпринт
+  (`check-docs.mjs`) сошлись — вторая правка `2b6db4c8` не оставила
+  `dev` с красным `docs`-джобом (прецедент #230/#234 не повторился).
+- Диф нигде не выходит за нужные классы файлов: A (`src/labs.ts`), B
+  (`test/labs.test.mjs`), C/D (сгенерированные скриншоты/бандл) — без
+  постороннего кода.
+- Единственное новое число, видимое в тесте (счётчики публикаций), не
+  дублирует какую-либо пользовательскую величину — сравнение «одно число,
+  один источник» здесь неприменимо: change не вводит новых значений на
+  экране.
+
+## Чего не проверял
+
+- Полный `npm test`/`npm run build`/`npx tsc --noEmit` по всему проекту —
+  положился на зелёный Validate на этом же SHA (см. «Как проверялось»).
+- Браузерные смоки — инструмент дал НЕОПРЕДЕЛЁННОСТЬ, обосновал отказ
+  выше; ручного захода в demo-стенд не делал.
+- `golden`, backend pytest, performance — вне зоны дифа, см. обоснование
+  выше.
+- Реальный сценарий HACS-перезагрузки ресурса в живом Home Assistant —
+  вне возможностей цикла ревью (нет ручного тестирования); заменено
+  точной симуляцией через две независимые ES-module инстанции, что и
+  является механизмом бага (новая инстанция модуля = новый `listening`).
+
+## Вердикт
+
+Зелёный. Все три AC выполнены и доказаны (автотест + чтение кода), фикс
+не выходит за скоуп issue, гейты, относящиеся к дифу, зелёные, трейлеры и
+byte-sync корректны. Блокирующих находок нет.