docs: review document for #174

Issue: #174
User-Visible: no

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

@@ -0,0 +1,177 @@
+# SPEC-REVIEW-174-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/174
+- **ТЗ:** [`docs/specs/174-linked-virtual-light-controller.md`](../specs/174-linked-virtual-light-controller.md)
+  (commit `4a8958291d28fbd86d0233e6ac9208b068b24f76`, ветка `issue/174-linked-virtual-light`)
+- **Ревьюер:** Claude (ревью ТЗ ≠ автор), этап `S4-spec-review`
+- **Цикл:** r1/4 (обычный трек — issue не `small`/`trivial`, что подтверждено
+  комментарием аналитики)
+
+## Скоуп ревью
+
+Ревью ТЗ #174: изменение authority-правила пассивного forced-источника
+«Всегда» + виртуальная лампа при наличии входящей связи от контроллера
+(умного выключателя/реле), плюс переадресация клика по такой лампе на реальные
+driver-сущности вместо operational `virtual_lights` toggle (#107).
+
+Не в скоупе ревью: код ещё не написан (issue в `S4-spec-review`), поэтому
+проверка кода/тестов — предмет будущего code-review.
+
+## Как проверялось
+
+Прочитано в заданном порядке:
+
+1. `docs/SCOPE.md` — job'ы J1/J3, инвариант замка (не затронут: driver-сущности
+   ограничены `light.*`/`switch.*`).
+2. `AGENTS.md`, `PROCESS.md` (полностью, включая §7.1 и §5).
+3. Issue #174 body + все 3 комментария (владелец подтвердил семантику
+   18.08.2026, аналитика оценила трек, автор сдал ТЗ).
+4. `docs/USER-GUIDE.ru.md` — раздел «"Тупая" лампа с умным выключателем»
+   (строки 559–589), сверены термины «Всегда», «Переключить состояние»,
+   «Управляет другими источниками света», Glow.
+5. Канонические документы подсистемы: `docs/LIGHT.md` (раздел «Source, state
+   and service identity»), `docs/specs/084-passive-forced-light-sources.md`,
+   `docs/specs/107-virtual-light-toggle.md`,
+   `docs/DEVICE-LIGHT-SETTINGS-MATRIX.ru.md`.
+6. Само ТЗ `docs/specs/174-linked-virtual-light-controller.md` целиком.
+7. Текущий код на ветке `issue/174-linked-virtual-light` (тот же коммит, где
+   лежит ТЗ; продуктовый код не менялся) — `src/devices.ts`
+   (`resolvedLightSources`, строки 484–608, включая безусловный override на
+   567–572), `src/device-toggle.ts` (`resolveToggleIntent`, 604–659;
+   `resolveControls`, 490–601; `sameToggleOperationTargets`, 686–698),
+   `src/virtual-light-state.ts` — чтобы убедиться, что цитаты кода и
+   утверждения о причине бага в ТЗ и в issue соответствуют реальному коду, а не
+   пересказу автора. Совпадение построчно точное (см. «Проверено и корректно»).
+8. Существование упомянутых тестовых/смоук-файлов:
+   `test/devices.test.mjs`, `test/device-toggle.test.mjs`,
+   `test/device-presentation.test.mjs`, `demo/smoke_virtual_light_toggle.mjs`,
+   `demo/smoke_controls.mjs` — все существуют, ТЗ не ссылается на
+   несуществующие файлы кроме нового `demo/smoke_linked_virtual_light.mjs`,
+   что явно помечено как новый файл.
+
+Гейты (typecheck/test/build) не прогонялись — на этапе ревью ТЗ кода нет,
+прогон гейтов здесь не предмет доказательства (это часть code-review,
+PROCESS.md §2.7).
+
+## Проверка формальных требований §7.1
+
+Все обязательные разделы присутствуют: сценарий и персона (§1), что человек
+увидит до/после одной фразой без терминов реализации (§2 — использует «Glow» и
+«HA state», но это установленный интерфейсный термин из
+`docs/USER-GUIDE.ru.md`/`docs/LIGHT.md`, а не изобретённый жаргон), проблема с
+подтверждённой причиной (§3, код процитирован точно), скоуп/не-скоуп (§6/§7),
+контракт состояния и действия (§8/§9), единые визуальные consumers (§10),
+модель данных и совместимость (§11), UX/i18n/touch (§12), AC1–AC14 с указанием
+доказательства (§13), план автотестов (§14), риски/perf/security (§15),
+откат (§16), release-артефакты (§17), явный блок принятых технических
+предположений (§18).
+
+Каждый AC пронумерован, имеет ровно один заявленный способ доказательства
+(`unit`/`smoke`/`ревью кода`/`build`) и формулировку, допускающую одну
+интерпретацию — за одним отмеченным ниже исключением (Low-1).
+
+Продуктовых вопросов владельцу не осталось: единственное решение владельца,
+требовавшееся для этой задачи (семантика «два умных устройства»), уже принято
+в комментарии от 18.08.2026 и корректно перенесено в §4 ТЗ. Автор не подменил
+догадкой ни одного продуктового пункта — там, где формат ответа мог требовать
+нового текста (driver-group hint), ТЗ явно откладывает решение как «отдельная
+продуктовая находка», а не придумывает текст (§12).
+
+## Находки
+
+### Low-1 — формулировка AC6 читается двусмысленно в отрыве от §9.1
+
+**Файл:** `docs/specs/174-linked-virtual-light-controller.md:309-310`
+
+AC6 гласит: «клик по controller сохраняет #84/#94 group semantics и
+переключает тот же driver projection, которым вычисляется linked source».
+При нескольких контроллерах на одну лампу эта фраза, прочитанная изолированно,
+может означать «клик по одному контроллеру должен переключить объединение
+driver-сущностей ВСЕХ контроллеров лампы» — что было бы неверно и опасно
+(один выключатель дистанционно переключил бы другой). По коду
+(`resolveControls`, `devices.ts:490-601`) и по явной строке §9.1 «в простой
+паре команда содержит реальный relay controller» видно, что имелось в виду
+другое: клик по контроллеру продолжает переключать только *свою* сущность,
+а утверждение AC6 — про переиспользование одного и того же вычислителя
+driver-проекции (не про объединение по всем входящим связям цели). Строка
+риска «Source click при нескольких controllers переключит только один relay» →
+«Детерминированный union/dedupe и group tests» (§15) относится к клику по
+**лампе** (AC4), а не по контроллеру, и путаницу создаёт именно совпадение
+формулировок.
+
+**Воспроизведение неоднозначности:** контроллер A и контроллер B оба ссылаются
+на одну и ту же passive-лампу L (валидная конфигурация по #84 §5.2, OR).
+Буквальное прочтение AC6 «тот же driver projection, которым вычисляется
+linked source» для L равно `{A.entity, B.entity}` — так что тест AC6 в
+такой формулировке можно было бы написать так, что клик по A потребует
+службы к B, чего продукт не хочет и чего не просит владелец.
+
+**Вердикт:** не блокирует — контекст §9.1 и таблица рисков снимают
+неоднозначность, а код (`resolveControls`) уже реализует корректную,
+per-controller семантику, которую AC6 обязан только сохранить. Снимаю без
+правки ТЗ: автор код-ревью обязан читать AC6 вместе с §9.1 буквально «свой
+relay, не объединение», и я фиксирую это здесь как явное толкование для
+будущего код-ревью, а не как повод для нового цикла.
+
+Больше High/Medium-находок нет.
+
+## Что проверено и корректно
+
+- **Причина бага подтверждена исполнением, а не пересказом.** Цитаты кода из
+  issue и ТЗ (`source.on = !control?.linked || ...`; `isManualVirtualLightMarker`
+  override в `resolvedLightSources`, и ранняя проверка в `resolveToggleIntent`)
+  совпадают построчно с текущим `dev`-кодом на ветке. Ни одно техническое
+  утверждение о причине не является догадкой.
+- **Продуктовое решение владельца корректно перенесено.** Формулировка §4 ТЗ
+  («два умных устройства», HA state — единственный источник истины,
+  lifecycle-граница create/remove link) слово в слово соответствует
+  комментарию владельца и последующей аналитике.
+- **Контракт §8/§9 согласован с уже принятыми канонами #84/#107**, а не
+  придумывает новую модель: OR нескольких контроллеров, zero-driver → dormant
+  (не constant-on), lossless `controls`, запрет `marker:*` в `callService` —
+  всё это уже нормативно закреплено в #84 и просто применяется к
+  ранее заблокированному случаю. Новизна ТЗ ограничена ровно точкой конфликта
+  (#84 OR vs #107 manual override) и симметричной переадресацией клика лампы —
+  это соответствует заявленному скоуфу и не расширяет его.
+- **Не входит в задачу (§7)** корректно исключает автосвязывание, новые
+  UI-поля, AND/NOT-логику, синхронизацию цвета/яркости и историю — ничего из
+  этого не запрашивал владелец.
+- **Compatibility (§11)** — не меняются `Marker`/`ServerConfig`/backend schema,
+  что подтверждается кодом: единственная правка — в вычислении `source.on` и в
+  выборе operation, оба уже существующих pure-функциях без сериализации.
+- **AC изложены с однозначным способом доказательства** и практически все —
+  с единственной трактовкой; мутационные требования присутствуют явно
+  (AC1, AC7) как и требует #85/PROCESS §2.7 «тест умеет падать».
+- **Release-артефакты (§17)** включают оба changelog, `USER-GUIDE.ru.md`,
+  `LIGHT.md`, `DEVICE-LIGHT-SETTINGS-MATRIX.ru.md`, supersession-заметку в
+  `107-virtual-light-toggle.md` — то есть именно те документы, чьи текущие
+  формулировки (`USER-GUIDE.ru.md:583-589`, `LIGHT.md:149-159`) сейчас описывают
+  старое (заменяемое) поведение и стали бы враньём без обновления.
+- **Откат (§16)** не требует миграции данных — корректно, так как формат
+  конфигурации не меняется.
+- **Технические предположения (§18)** отделены от продуктовых решений и явно
+  помечены как «можно менять свободно», ревьюеру не выдано ни одной догадки
+  под видом факта.
+
+## Чего не проверял
+
+- Реализацию — её нет: issue находится в `S4-spec-review`, продуктовый код не
+  менялся с предыдущего merged-состояния (`git diff origin/dev...HEAD` для
+  `src/**` пуст).
+- Гейты `typecheck`/`test`/`build`/browser smoke — не относятся к этапу
+  ревью ТЗ; будут предметом code-review (PROCESS.md §2.7/§8).
+- `custom_components/houseplan/**/*.py` — ТЗ и код подтверждают отсутствие
+  backend-изменений, поэтому backend-тесты не запускались.
+- Golden/perf-эталоны — ТЗ явно заявляет отсутствие нового visual baseline;
+  не перепроверял рендер вручную, так как речь о будущей реализации.
+
+## Вердикт
+
+Все обязательные разделы §7.1 присутствуют, каждый AC однозначен и имеет
+названный способ доказательства, продуктовых открытых вопросов не осталось
+(решение владельца зафиксировано и корректно перенесено), догадок под видом
+факта не найдено. Единственная находка — Low, снята без цикла возврата, с
+явной запиской для код-ревью.
+
+**Вердикт: зелёный · цикл r1/4 · High: 0 · Medium: 0 → нет · Документ:
+docs/reviews/SPEC-REVIEW-174-r1.md**