From 631bafea7679e7a768dbf444d2a0e6294714bebf Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 07:41:17 +0000 Subject: [PATCH] docs: review document for #366 Issue: #366 User-Visible: no --- docs/reviews/SPEC-REVIEW-366-r1.md | 229 +++++++++++++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-366-r1.md diff --git a/docs/reviews/SPEC-REVIEW-366-r1.md b/docs/reviews/SPEC-REVIEW-366-r1.md new file mode 100644 index 00000000..f6fa2f9e --- /dev/null +++ b/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 и (б) непомеченной догадки о видимом поведении, которую +процесс требует помечать явно, даже когда цена ошибки невелика.