docs: review document for #366

Issue: #366
User-Visible: no

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

@@ -0,0 +1,229 @@
+# SPEC-REVIEW-366-r1
+
+Issue: #366 — «Glow #20: движущиеся ворота на cover-сущности вызывают до ~100
+пересчётов геометрии света за одно открытие»
+Этап: ТЗ на ревью (S4-spec-review), лёгкий трек (`small`)
+Заход: r1 · блокирующих циклов ревью ТЗ израсходовано 0 из 2 (лимит §4 для лёгкого трека)
+Материал: тело issue #366 на момент ревью, дерево `dev` на SHA `f2e365726a6e28a9acf03b6a37b4d2099770229d`
+Вердикт: **жёлтый**
+
+## Скоуп
+
+Проблема реальна и локализована точно: `openingLightStateSignature`
+(`src/logic.ts:344-357`) строит ключ кэша барьеров света через
+`amount.toFixed(3)`, а `amount` для проёма типа door/gate с cover-сущностью,
+публикующей `current_position` в процентах, меняется на каждый процент хода
+(`openingAmount()`, `src/logic.ts` ~§320-330). Каждое новое значение —
+новая сигнатура → новый ключ LRU-пула (`_lightBarrierPool`,
+`src/houseplan-card.ts:10368-10373`) → полный пересчёт `physicalBodyParts`
+(`:10404-10412`) и вырезов (`:10383-10402`). Направление фикса (квантование
+amount к сетке 0.05 в единственной точке сборки) — то, что уже описано в
+проблеме автором issue как предпочтительный вариант; предложенный контракт
+ему соответствует.
+
+Задача действительно укладывается в `docs/SCOPE.md`: это не новая функция, а
+починка перформанс-дефекта уже принятой в J1/J7 (живой обзор дома) отрисовки
+Glow (#20), заметного на wall-планшете — ключевой поверхности для персон
+«household members» / «guests». Small-трек обоснован: одна поверхность (вход
+`_lightBarriers`), геометрические алгоритмы и миграции не тронуты, сложность
+≤3 по формулировке автора выглядит правдоподобно.
+
+## Как проверялось
+
+Ревью ТЗ, код ещё не написан — сборка/тесты/смоки не прогонялись, это
+предмет код-ревью. Материал: тело issue #366, `docs/SCOPE.md`, `AGENTS.md`,
+`PROCESS.md`, `docs/LIGHT.md`, текущее состояние `src/logic.ts` и
+`src/houseplan-card.ts` на `dev` (сверка описанных в ТЗ точек кода с
+реальным деревом), issue #20 (контекст, откуда взялась Glow-через-дверь и
+где лежит существующий смок), `test/logic.test.mjs` (существующие юниты на
+`openingLightStateSignature`), `demo/smoke_glow.mjs`,
+`demo/smoke_junction_limits.mjs`.
+
+Что именно сверено по коду:
+
+- `src/houseplan-card.ts:10352-10361` — единственное место, где строится
+  `passageStates` и вызывается `this._openingAmt(o)`; результат этого поля
+  (`amount`) действительно единолично питает и `openingLightStateSignature`
+  (`:10362-10366`), и длину выреза через `openingLightApertureLength`
+  (`:10383-10388`), и масштаб `lightPartitionCuts`/`scalePartitionOpeningCut`
+  (`:10399-10402`). Формулировка К1 «единственная точка» подтверждена —
+  квантование там действительно закрывает все три потребителя разом, а не
+  только два названных в тексте ТЗ явно (сигнатура + длина выреза; про
+  partition-cut ТЗ не говорит отдельно, но он неизбежно накрывается тем же
+  местом).
+- `src/houseplan-card.ts:12413-12420` (`_openingAmt`) — единственный
+  производитель значения; другие потребители того же метода (рендер
+  створки, `:8816`, `:12473`, `:12594`) действительно отдельные вызовы,
+  которые контракт К1 не трогает — «анимация остаётся плавной» подтверждается.
+- `src/houseplan-card.ts:9347-9357` (`_physicalBodiesR`, обычные стены) и
+  `:7512-7522` (`_partitionOpeningCuts`, структурные вырезы) используют
+  `_cfgEpoch`/конфиг напрямую, а не `_openingAmt` — эта часть геометрии вне
+  скоупа задачи по факту кода, не только по декларации ТЗ.
+- Формула квантования (`Math.round(clamp(x)/0.05)*0.05`) проверена в Node на
+  примерах из AC1: `0→0`, `1→1` точно; `0.30` и `0.31` дают один и тот же
+  float (`0.30000000000000004`); `0.33` даёт другой (`0.35000000000000003`);
+  `NaN/-1/2` зажимаются в `0`/`0`/`1`. Все утверждения AC1 воспроизводимы,
+  плавающая точка не портит проверяемые равенства.
+- Полный диапазон 0..1 с шагом 0.05 даёт 21 узел — AC2 «≤21 сигнатура» и К2
+  «≤21 пересчёт» математически согласованы.
+- `test/logic.test.mjs` содержит текущие юниты на `openingLightStateSignature`
+  с амплитудами `1`, `0.5`, `0`, `-2→0`, `2→1` — все совпадают с узлами сетки
+  0.05, так что при добавлении квантования выше по стеку (не внутри самой
+  функции) эти юниты не потребуют правки, как и утверждает AC3.
+- `demo/smoke_glow.mjs:598-631` — существующий смок реально двигает
+  `cover.glow_dynamic_door` через `current_position` и снимает скриншоты на
+  позициях `0`, `50` (и `100` раньше по тексту, :360) — обе используемые
+  доли (`0`, `0.5`, `1.0`) точно совпадают с узлами сетки 0.05, так что этот
+  смок останется зелёным без правки ассертов — подтверждает AC4 фактически,
+  но не по названию (см. находку M1 ниже).
+- `demo/smoke_junction_limits.mjs` — прочитан начало файла: это гвард лимитов
+  на запись стыков стен из #329 (проверка отказа сохранения, а не Glow и не
+  `_openingAmt`). Связи с изменённым кодом по факту не нашёл — см. M1.
+- `docs/LIGHT.md`, раздел «Caching» (строки 139-147) документирует сам факт
+  сигнатуры по amount, но не даёт числового контракта точности — квантование
+  формально его не нарушает, но раздел станет неполным без упоминания шага
+  (см. M2).
+
+## Находки
+
+### M1 (Medium, в скоупе) — AC4 не называет проверяемое доказательство однозначно
+
+`AC4` в ТЗ: «существующие смоки/golden #20 зелёные без правки ассертов (свип
+прогоняется по делу: `smoke_door_glow*` — имя уточню по факту — плюс
+`smoke_junction_limits` как полевой сосед)».
+
+Файла `demo/smoke_door_glow*.mjs` не существует (`ls demo/smoke_*.mjs` —
+проверено). Реальный тест, гоняющий именно cover-driven amount на дверь/ворота
+через Glow — `demo/smoke_glow.mjs` (строки 598-631, двигает
+`glow_dynamic_door` по `current_position`). Автор сам помечает имя как
+неопределённое («уточню по факту») — это ровно то, что §2.5 DoR запрещает:
+«у каждого [AC] указано, чем он доказывается» должно быть фактом на момент
+ухода в `S5-ready`, а не обещанием уточнить позже. Второй названный смок,
+`smoke_junction_limits`, судя по его коду (проверка отказа записи при
+нарушении лимитов стыков стен, #329) не имеет видимой связи с
+`_openingAmt`/Glow/`passageStates` — названо «полевым соседом» без указания,
+какое именно совпадающее поведение он защищает.
+
+**Почему в скоупе и чинится здесь:** правка ТЗ-текстовая, без нового решения
+— заменить неопределённое имя на `demo/smoke_glow.mjs` (он уже доказывает
+AC4 по факту, просто назван неверно) и либо обосновать релевантность
+`smoke_junction_limits` конкретной связью, либо убрать его и заменить
+результатом `node scripts/smoke-select.mjs --base origin/dev --head HEAD`,
+когда код появится — эта команда есть в гейтах и предназначена ровно для
+такого выбора.
+
+### M2 (Medium, в скоупе) — визуальная деградация подана как факт, а не как решение с owner-видимостью
+
+Контракт К2: «Визуальная цена: вырез света ступает по 5% длины проёма — на
+реальных планах неразличимо». Это утверждение о том, что *увидит человек* —
+ровно категория, которая по §7.1 адресуется владельцу («что человек видит»),
+и ровно тот шаблон, который процесс называет «худшим видом дефекта»: догадка
+о поведении, поданная как факт, без пометки «принято предположительно».
+
+По существу: до этой правки амплитуда cover входит в сигнатуру с точностью
+`toFixed(3)` (≈0.1%), т.е. вырез света визуально отслеживает движение почти
+непрерывно (ценой ~1000 пересчётов на полный ход — это и есть баг). После
+правки вырез будет физически перескакивать по 5%-ным узлам — не «то же самое
+чуть грубее», а намеренный отказ от непрерывности в обмен на
+производительность. Сам issue в разделе «Направление фикса» explicitly
+называет квант и debounce равноценными альтернативами и не выбирает между
+ними — то есть на момент постановки issue выбор ещё не был сделан
+владельцем, а ТЗ на S3 сделало его молча.
+
+Моя собственная оценка правдоподобия (не решение вместо владельца, а
+основание для важности этой находки): проём двери/ворот на плане обычно
+80–120 см, шаг 5% — это 4–6 см видимого скачка мягкого (blurred, см.
+`docs/LIGHT.md:130-132`, кромка размыта на 1.5-3.5px) выреза на масштабе
+комнаты — вероятно действительно малозаметно, особенно на фоне того, что
+сейчас деградация проявляется как подёргивание всей карточки. Но это
+«вероятно» — моя оценка, не зафиксированный факт, и распорядиться ею должен
+или явный блок «принято предположительно, ревьюер/владелец может оспорить»,
+или batched-вопрос владельцу с дефолтом «квант 0.05, деградация
+незначительна» — вариант, который стоит владельцу секунд по формату §7.1.
+
+**Почему Medium, не High:** обратимость полная (один revert, конфиг не
+затронут), риск — эстетический на фиче, которую сам владелец в #20 назвал
+`lowest priority` из-за её редкой применимости (contact-сенсор на *внутренней*
+двери — редкость). Блокировать цикл ради этого нет оснований, но оставить
+неотмеченной догадку как факт — тоже нельзя.
+
+**Как чинится:** добавить в ТЗ явный блок вида «принято предположительно:
+шаг 0.05 визуально неразличим на типовых проёмах 80–120 см; читатель волен
+оспорить» — это удовлетворяет и `M2`, и общее требование «не бывает сложной
+задачи без единого открытого вопроса» (сейчас в ТЗ такого блока нет вовсе).
+Отдельный поход к владельцу не обязателен — цена ошибки низкая и обратимая,
+достаточно того, чтобы предположение было промаркировано как предположение,
+а не как решённый факт.
+
+### L1 (Low, не блокирует) — строки в ТЗ отстали от `dev` на единицы
+
+`houseplan-card.ts:10361` (в ТЗ) — на текущем `dev` это `:10360`
+(`amount: this._openingAmt(o)`), аналогично `:10362`→фактически те же
+строки описания сигнатуры сдвинуты на 1. Смысл описания точен (я нашёл
+нужное место по цитате кода, не по номеру), правка не требуется для выхода
+в разработку, но при финальном коммите номера стоит свериться заново — за
+время ревью dev не двигался, но между S3 и S6 может.
+
+### L2 (Low, не блокирует) — `docs/LIGHT.md` не назван в release-артефактах
+
+Раздел «Caching» `docs/LIGHT.md:139-147` описывает сигнатуру по amount как
+часть контракта подсистемы, но не удельную точность. Квантование меняет этот
+контракт (сигнатура теперь дискретна по построению, а не «просто округлена
+для ключа») и вводит намеренную визуальную ступенчатость — то, что канонический
+документ подсистемы должен фиксировать по `AGENTS.md` («канонический документ
+затронутой подсистемы»). ТЗ называет из release-артефактов только
+changelog en+ru. Снимается легко: одна строка в LIGHT.md, тем же коммитом, не
+блокирует переход в `S5-ready`; фиксирую как Low с правом автора либо
+поправить, либо снять с запиской, если сочтёт достаточным changelog.
+
+## Что проверено и корректно
+
+- Технический выбор единственной точки квантования (`passageStates`)
+  реально исключает риск рассинхронизации «сигнатура vs геометрия» (принцип
+  «одно число — один источник», §8 PROCESS.md) — geometry (`rlen`,
+  partition-cut) и cache-key берут один и тот же квантованный `amount` из
+  одного и того же объекта, поэтому исключён класс дефектов вида #234/#233.
+- AC1 и AC2 математически корректны и воспроизводимы (проверено расчётом в
+  Node, см. выше) — квантование к сетке 0.05 действительно даёт ≤21 узел,
+  0 и 1 точные, тестовые примеры (`0.30`/`0.31` совпадают, `0.30`/`0.33`
+  различны) подтверждаются без артефактов плавающей точки.
+- AC3 (binary-двери байт-в-байт) обеспечен архитектурно: квантование
+  применяется выше `openingLightStateSignature`, а не внутри неё, и
+  существующие юниты, гоняющие функцию напрямую с сырыми amount, её не
+  затрагивают.
+- AC5 (мутационный тест «квант → identity» красит AC2) выполним: замена
+  `quantizeOpeningLightAmount` на identity вернёт ~1000 различных сигнатур на
+  свипе 0.001 вместо ≤21, ассерт «≤21» упадёт детерминированно — тест умеет
+  падать по построению AC, не только по обещанию.
+- Скоуп геометрии/миграций/touch действительно не затронут: структурные стены
+  (`_physicalBodiesR`) и partition-cut вне light-пути (`_partitionOpeningCuts`)
+  используют конфиг/epoch напрямую, не `_openingAmt` — small-трек обоснован
+  фактически, не только декларативно.
+- Откат описан верно и достаточен: один revert, кэш самоинвалидируется
+  сигнатурой, конфиг не участвует — миграции назад не требуется.
+- `User-Visible: yes` с указанием обоих changelog учтено верно, учитывая
+  находку M2 про формулировку.
+
+## Чего не проверял
+
+- Код не написан — `typecheck`/`test`/`build`, смоки, golden, инварианты
+  модели не прогонялись: на этапе ревью ТЗ это не предмет проверки, будет
+  предметом код-ревью (S7). Явно не мой гейт на этом этапе.
+- Не проверял реальным глазом (браузер/скриншот) визуальную
+  неразличимость 5%-ступени — только оценка масштаба (см. M2); это довод в
+  пользу пометить решение как предположение, а не довод «всё в порядке».
+- Не искал исчерпывающе все смоки/golden, задевающие `current_position` —
+  проверил только те, что явно упоминают `current_position`
+  (`smoke_glow`, `smoke_cover_no_plate`, `smoke_cover_tap`); последние два
+  проверяю по коду не относящимися к light-пути (тестируют плашку/тап
+  cover, не Glow), поэтому не разбирал их подробно.
+- `npm run inventory`/точные счётчики тестов не считал — не нужно на этом
+  этапе.
+
+## Итог
+
+0 High, 2 Medium в скоупе (M1, M2), 2 Low (L1, L2, на усмотрение автора).
+Технически контракт К1/К2 и AC1-AC3, AC5 — корректны и доказуемы; ТЗ
+возвращается автору не из-за ошибки в решении, а из-за (а) незакрытого
+доказательства AC4 и (б) непомеченной догадки о видимом поведении, которую
+процесс требует помечать явно, даже когда цена ошибки невелика.