From d197381894274e3c169a6dfa048f2d0da6f7ea73 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 22:05:42 +0000 Subject: [PATCH] docs: review document for #211 Issue: #211 User-Visible: no --- docs/reviews/CODE-REVIEW-211-r2.md | 149 +++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-211-r2.md diff --git a/docs/reviews/CODE-REVIEW-211-r2.md b/docs/reviews/CODE-REVIEW-211-r2.md new file mode 100644 index 00000000..0a3868e0 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-211-r2.md @@ -0,0 +1,149 @@ +# CODE-REVIEW-211-r2 — коррекция compatibility-смока после полного Linux CI + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/211 +- **ТЗ:** `docs/specs/211-device-icons-visual-parity.md` (принято `SPEC-REVIEW-211-r2`, зелёный) +- **Предыдущий цикл:** `docs/reviews/CODE-REVIEW-211-r1.md` — зелёный, реализация + (`4e82976`, `270cf63`) уже слита в `dev` до того, как полный Linux smoke + вскрыл устаревшую проверку; поэтому временный `S8-merged` был гонкой двух + событий, а не итоговым статусом (см. комментарии владельца в issue). +- **Диапазон этого цикла:** `origin/dev..HEAD` = один коммит + `4d1b62b` — `test: align locked marker smoke with design package` · + `Issue: #211` · `User-Visible: no` +- **Цикл:** r2/4 + +## Скоуп проверки + +Весь диапазон `git diff origin/dev...HEAD` — это 11 строк в одном файле, +`demo/smoke_cover_no_plate.mjs` (7 добавлено, 4 удалено). Ничего в `src/**`, +`custom_components/**`, конфиге, i18n или changelog не тронуто — класс B +(гейт/тест), трейлер `User-Visible: no` соответствует. Изменение правит один +устаревший compatibility-факт (`lockedLockIsNeutral` → `lockedLockKeepsPackageFace`) +внутри смока, который сам по себе не входит в диапазон реализации #211, но +проверяет побочный контракт («у замка есть рамка, замок не становится +нейтральным»), задетый переходом на дизайн-пакет #179/#211. + +Продуктовый код в этом цикле не проверяется повторно — он уже был предметом +`CODE-REVIEW-211-r1` (зелёный) и в этом диапазоне не менялся. + +## Как проверялось + +### Гейты — прогнаны + +``` +npx tsc --noEmit → чисто +npm test → 939 pass, 0 fail, 0 skipped +npm run build → OK +cmp dist/houseplan-card.js custom_components/.../houseplan-card.js → идентичны +cmp dist/houseplan-card.js demo/srv/assets/houseplan-card.js → идентичны +node demo/smoke_cover_no_plate.mjs → все факты true, OK +node demo/smoke_device_icon_design.mjs → все факты true, OK (доп. проверка + основного AC3-смока лежащей рядом поверхности; продукт не менялся с r1, но + это самый дешёвый способ убедиться, что коррекция теста не разошлась с + уже принятым CSS) +``` + +### Проверка «тест умеет падать» (обязательная дисциплина, применена к +единственному изменённому предикату) + +Правило `.dev.lock-locked` в `src/styles.ts` вернул временно к состоянию до +основного фикса `4e82976` (`background: var(--device-core-bg); color: #000`, +без `theme-light`/`theme-dark` расщепления — то есть ровно то поведение, +которое проверял старый `lockedLockIsNeutral`), пересобрал `dist` и +синхронизировал `demo/srv/assets/houseplan-card.js` (демо-сервер отдаёт +именно эту копию, не `dist/` напрямую — без этого шага мутация не была бы +видна смоку). Результат: + +``` +"lockedLockKeepsPackageFace": false +FAILED (1): lockedLockKeepsPackageFace: expected true, got false +``` + +Все остальные факты смока остались зелёными — падение точечное, как и +должно быть для правки одного предиката. После проверки `src/styles.ts` +восстановлен из копии, `npm run build` прогнан повторно, `dist` синхронизирован +обратно в `demo/srv/assets` и в `custom_components/houseplan/frontend`; +`git status` — чистое дерево, диффа не осталось. + +Дополнительно проверено чтением: новые ожидаемые значения +(`rgb(0, 0, 0)`/`rgb(255, 255, 255)` для Light, `rgb(37, 37, 37)`/ +`rgb(255, 255, 255)` для Dark) — это буквальный computed-style результат +правил `.dev.lock-locked`, `.dev.theme-light.lock-locked`, +`.dev.theme-dark.lock-locked` (`src/styles.ts:2035-2048`), которые сами уже +были зафиксированы и приняты в `CODE-REVIEW-211-r1` как соответствующие §7.3 +ТЗ (`Lock Light: core black / glyph white`, `Lock Dark: core #252525 / glyph +white`). `--device-face-bg`/`--device-face-fg` транслируются в +`background`/`color` `.device-core` напрямую (`src/styles.ts:1988-1989`), так +что тест меряет ровно то, что видит пользователь, а не побочный токен. + +### Оценка ветвления по теме в самом тесте + +Смок не переключает `hass.themes.darkMode`, а демо-фикстура (`demo/srv/demo.html`) +не задаёт его вовсе, поэтому `device-face.ts:24` (`hass.themes.darkMode ? +'theme-dark' : 'theme-light'`) в этом прогоне детерминированно даёт +`theme-light` — веткой `rgb(37, 37, 37)` тест здесь фактически не исполняется. +Это не дефект: тёмная ветка того же контракта (`darkLockUsesWhiteGlyphAndDarkShell`) +уже отдельно и явно проверяется вычисляемым фактом в +`demo/smoke_device_icon_design.mjs` и была подтверждена в r1 независимым +before/after прогоном. Тернарное выражение здесь написано корректно и +симметрично на случай, если смок когда-нибудь начнёт переключать тему, но не +создаёт для этого дублирующего покрытия — фиксирую как наблюдение, не как +находку. + +### Коммит и трейлеры + +`4d1b62b`: `Issue: #211`, `User-Visible: no` — верно, диапазон не содержит +изменений продукта, изменений в changelog не требуется. + +## Что проверено и корректно + +- Правка ограничена одним файлом гейта (`demo/smoke_cover_no_plate.mjs`), + относится к предмету #211 (AC3, Lock state parity) и устраняет расхождение + между устаревшим pre-#179 предположением теста («заблокированный замок — + нейтральный, глиф всегда чёрный») и принятым дизайн-пакетом («core черный/ + `#252525`, глиф всегда белый»). +- Новый предикат `lockedLockKeepsPackageFace` измеряет именно тот CSS, + который производит видимый пиксель (`background`/`color` на `.device-core` + через `--device-face-bg`/`--device-face-fg`), не побочный токен. +- Предикат доказанно умеет падать: воспроизведена красная линия на + до-фиксовом правиле, падение точечное (1 из ~60 фактов файла), после + восстановления кода все факты снова зелёные. +- Остальные факты того же файла (cover plate/morph/valve/window/unlocked-lock) + не задеты диапазоном и остались зелёными — регрессии в соседних, не + относящихся к #211 проверках нет. +- `tsc`, unit-suite и build чистые; три копии бандла идентичны после + восстановления дерева. +- Коммит несёт корректный `Issue:`/`User-Visible:` трейлер для class B. + +## Находки + +Нет находок уровня High, Medium или Low. Единственное отмеченное выше +наблюдение (тернарная dark-ветка предиката не исполняется в этом конкретном +прогоне) не поднимаю как находку: поведение для Dark уже покрыто отдельным, +явно исполняемым фактом в другом смоке, а сам тернар написан правильно и не +вводит риск регрессии. + +## Чего не проверял и почему + +- **Полный набор из 127 browser smokes** — не запускал; диапазон правит один + предикат в одном уже существующем файле, не относящийся к нему smoke не + может быть задет текстовой правкой ассерта. +- **`npm run golden:verify`** — не запускал; диапазон не меняет ни один файл, + влияющий на рендер (только тестовый ассерт), видимый результат не может + измениться. +- **`python -m pytest tests_backend -q`** — не запускал; `custom_components/**/*.py` + не тронут. +- **Performance-профили** — не запускал; диапазон не касается DOM/CSS/JS + продукта. +- **Полный Linux CI-прогон** — не запускал локально; это гейт CI на выходе + из код-ревью, а не задача рецензента. (Именно такой прогон и обнаружил + устаревший факт, который правит этот коммит — второй такой же прогон на + этом SHA пройдёт штатно через пайплайн после зелёного вердикта.) + +## Вердикт + +Единственное изменение цикла — точечная и доказанная (падает на старом +поведении, зелёное на новом) коррекция стороннего compatibility-теста, +приводящая его в соответствие с уже принятым в r1 дизайн-контрактом #211/#179. +Продуктовый код не менялся, регрессий нет, трейлеры корректны. + +**Зелёный.**