docs: review document for #218

Issue: #218
User-Visible: no

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

@@ -0,0 +1,216 @@
+# SPEC-REVIEW-218-r1
+
+- **Issue:** #218 — одна комната с floating-point шумом в координатах гасит
+  свечение во всём пространстве
+- **Артефакт ТЗ:** `docs/specs/218-glow-floor-geometry.md`
+- **Ветка/SHA:** `issue/218-glow-floor-geometry` @ `c6ff34c`
+- **Трек:** обычный (не `small`) — файл ТЗ существует, что соответствует
+  отсутствию метки `small`/`trivial` на issue
+- **Цикл:** r1/4
+- **Вердикт:** жёлтый · High: 0 · Medium: 1 (в скоупе)
+
+## Скоуп ревью
+
+Первый цикл — разбор полный. Проверялись: тело issue #218 целиком (включая
+воспроизведение владельца и мутационный гейт, вписанный туда автором отчёта),
+оба комментария (аналитика владельца, хендофф автора ТЗ),
+`docs/specs/218-glow-floor-geometry.md` целиком, соответствие обязательным
+разделам §7.1 PROCESS.md, техническая достоверность «Подтверждённой причины»
+по фактическому коду, согласованность с каноном `docs/LIGHT.md` и с
+прецедентом #197 (`docs/WALL-THICKNESS.md`, `docs/ARCHITECTURE.md:414`).
+
+## Как проверялось
+
+- `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` — прочитаны целиком; §2.4/§7.1/§8
+  применены к ТЗ. Job — J1 («живая пространственная картина того, что сейчас
+  происходит»); анализ владельца в issue уже привязывает дефект к J1, ТЗ не
+  расширяет и не переопределяет привязку.
+- `docs/LIGHT.md` — сверена модель Glow («лампа освещает пол, который видит»,
+  §«From barriers to a lit region» про `intersectionPaths`, раздел про
+  fail-dark источника в кладке) — контракт §8.2/§11 ТЗ ей не противоречит и
+  явно её сохраняет.
+- `src/physical-geometry.ts:224-275` (`unionBodies`, `intersectionPaths`,
+  `geometryPolygonPaths`) — прочитаны построчно. Подтверждено дословно:
+  `unionBodies` глотает исключение polyclip и возвращает `null`
+  (`:228-230`); `intersectionPaths` строит `limit = unionBodies(bounds)` и при
+  `!limit` возвращает `[]` для всего вызова (`:263-265`), а падение самого
+  `intersection(base, limit)` тоже даёт `[]` с комментарием «fail dark»
+  (`:266-274`). Это ровно цепочка, которую ТЗ описывает в §3 — диагноз не
+  догадка, а прочитанный код.
+- `src/physical-geometry.ts:285-302` (`floorMinusBodies`) — подтверждён
+  прецедент последовательного `difference` как деградации при падении
+  объединения; ТЗ §8.2 предлагает симметричный покомнатный путь для
+  `intersectionPaths`, а не изобретает шаблон с нуля.
+- `src/houseplan-card.ts:14880-14998` (`_lightBarriers`) — подтверждено, что
+  `floor` уже является массивом полигонов **по комнате**
+  (`floor: polys.map((x) => x.poly)`, `:14987`), а не заранее слитой
+  геометрией. Это делает покомнатную деградацию из §8.2 технически
+  реализуемой без изменения формы входных данных, вопреки риску, что ТЗ
+  потребует данных, которых нет на этом уровне.
+- `src/houseplan-card.ts:15001-15117` (`_renderGlowLayer`) — подтверждено, что
+  `intersectionPaths([seen], floor)` вызывается на каждый источник и что уже
+  существует guard `pointInOpaquePlanBody` для источника в кладке (fail-dark,
+  который §7/§8.2 ТЗ явно обязуется не ослаблять).
+- `docs/WALL-THICKNESS.md:125,198,213`, `docs/ARCHITECTURE.md:414`,
+  `docs/TESTING.md:2304` — подтверждён прецедент #197 («один вырожденный стык
+  не должен гасить остальные стены всего пространства»), на который ссылается
+  и §4.4 ТЗ, и аналитика владельца в issue. Ссылка не декоративна — это
+  реальный принятый прецедент того же класса дефекта.
+- `demo/smoke_glow_fail_dark.mjs`, `demo/smoke_glow.mjs`,
+  `demo/smoke_glow_blending.mjs`, `demo/benchmark_glow.mjs` — существование
+  подтверждено; это ровно файлы, которые §16.2/AC5/AC9 ТЗ называют целевыми
+  гейтами, а не вымышленные пути.
+- `scripts/mutation-gate.mjs` — формат реестра (id/patches/guard/because)
+  сверен; таблица §16.3 ТЗ описывает мутанты на уровне намерения (что за
+  правку они бы отменили и какой guard должен упасть), а не выдаёт себя за
+  готовые патчи — этого от ТЗ и не требуется.
+- Пересчитан по коду масштаб `quantum = 1e-6` единицы плана: `GRID_PITCH =
+  NORM_W/GRID_N = 1000/240`, `_cellCm` по умолчанию 5 см
+  (`src/houseplan-card.ts:6013-6016`) → 1 единица плана ≈ 1.2 см, то есть
+  `1e-6` единицы ≈ 1.2·10⁻⁸ м, а не заявленные в ТЗ «10⁻⁹ м». См. находку Low
+  ниже — на исполнимость AC это не влияет.
+- Код продукта не менялся и не запускался на этапе `spec` — ревью
+  ограничивается статическим чтением `src/**` и `docs/**`; отдельно сказано в
+  разделе «Чего не проверял».
+
+## Находки
+
+### Medium (в скоупе, чинится в этом же ТЗ) — отсутствует обязательный раздел «Риски»
+
+PROCESS.md §7.1 фиксирует список обязательных разделов ТЗ: «…AC1…ACn с
+указанием доказательства · план автотестов · **риски** · откат ·
+release-артефакты». В `docs/specs/218-glow-floor-geometry.md` заголовка
+«Риски» нет вовсе (`grep -in "риск"` по файлу — ноль совпадений), и не только
+по названию: содержательного свода «что может пойти не так» нет ни в одном
+разделе. Формулировка «Quantum является техническим контрактом… изменение
+после ревью требует доказать…» (§8.1) — это governance-условие на будущее
+изменение константы, а не разбор рисков текущей задачи; §10 и §14 говорят про
+конкретный технический контракт, но не собирают риски документа в одном
+месте, как того требует чек-лист DoR (§2.5 PROCESS.md — «открытых продуктовых
+вопросов нет; риски перечислены» тоже требует явного перечня).
+
+Для сравнения — предыдущее ревью того же типа (`SPEC-REVIEW-217-r1.md`)
+прямо проверяло присутствие «риски (§15)» как отдельного пункта чек-листа;
+здесь пункта нет физически.
+
+**Почему это Medium, а не High:** раздел не создаёт открытой продуктовой
+неопределённости и не блокирует реализацию — содержательные предпосылки
+частично рассеяны по документу (governance quantum в §8.1, недоказанность
+корневой причины Glow-base в §10, стоимость прогона mutation-gate в §16.3), а
+инженерная часть ТЗ технически состоятельна (проверено кодом выше). Но раздел
+обязателен по §7.1 буквально, и его полное отсутствие — не опечатка, а пробел
+структуры документа, который автор должен закрыть перед выходом в
+«Готово к разработке».
+
+**Что чинить:** добавить раздел «Риски», собрав минимум:
+- изменение общего эпсилона `unionBodies()` затрагивает не только Glow, а
+  весь boolean-слой (масонри, floorMinusBodies, physicalBodiesPath) — риск
+  регрессии в соседних потребителях, если квантование когда-нибудь
+  перестанет быть no-op для валидных планов;
+- наблюдение Glow-base (§10) может не воспроизвестись той же причиной — риск
+  остаться недокументированным при отрицательном результате;
+- стоимость прогона `mutation-gate.mjs` для новых мутантов перед релизом
+  (уже упомянута в §16.3 как факт, но не как риск бюджета review-цикла);
+- легаси-планы с перекрывающимися комнатами (упомянуты в §8.2 как то, что
+  нормальный путь обязан беречь) — риск того, что покомнатный fallback даст
+  иное покрытие пола, чем текущий merged path, на таких планах.
+
+Это не требует переписывания ТЗ — достаточно одного раздела, синтезирующего
+уже имеющийся материал; технической неопределённости к решению владельцу это
+не добавляет.
+
+### Low (снята с записью, не блокирует)
+
+§8.1 ТЗ утверждает: «quantum `1e-6` единицы плана = 10⁻⁹ м». Пересчёт по
+`GRID_PITCH = NORM_W/GRID_N` (`src/space-geometry.ts:11`, `:197-199`) и
+`_cellCm` по умолчанию 5 см (`src/houseplan-card.ts:6013-6016`) даёт
+`1e-6` единицы ≈ `1.2e-8` м — на порядок больше заявленного. Расхождение
+чисто иллюстративное: качественный вывод абзаца («далеко за пределами любой
+видимой/физической точности плана») верен при любом разумном `cell_cm`, ни
+один AC и ни один тест не ссылается на конкретное число метров, и `cell_cm`
+конфигурируем per-space, так что «единица = 10⁻⁹ м» в принципе не может быть
+универсально точной константой. Снимаю без цикла: правки не требую, но если
+автор будет трогать ТЗ по Medium-находке выше, стоит заменить точное число на
+формулировку без привязки к конкретным метрам (например, «на много порядков
+меньше миллиметра при любом реалистичном масштабе плана»).
+
+## Проверка обязательных разделов §7.1
+
+Присутствуют и содержательны: сценарий (§1) · что человек увидит до/после
+(§2) · подтверждённая причина = проблема (§3) · нормативные источники и
+приоритет (§4) · цели (§5) · scope/не-scope (§6/§7) · контракт поведения
+(§8/§9/§10) · UX/accessibility/touch (§11) · данные/migration/privacy/i18n
+(§12) · архитектура (§13) · performance/security (§14) · AC1…AC10, каждый с
+доказательством (§15) · план автотестов (§16) · release-артефакты (§17) ·
+откат (§18) · принятые предположения (§19).
+
+**Отсутствует:** риски как отдельный раздел — единственный пункт списка §7.1,
+не найденный в документе ни по заголовку, ни по содержанию (см. Medium выше).
+
+Продуктовых открытых вопросов нет — и корректно нет: сценарий, ожидаемое
+поведение до/после и приоритет между «не гасить весь этаж» и «сохранить
+fail-dark для реально небезопасных случаев» уже зафиксированы решением
+владельца в комментарии-аналитике (не додуманы автором ТЗ). Единственная
+техническая неопределённость (наблюдение Glow-base, §10) явно помечена как
+«если не воспроизведётся — зафиксировать и не расширять #218», что
+соответствует правилу «размытое место не додумывается, а разбирается
+предположением с явной пометкой» (§19 п.5 ТЗ).
+
+Каждый AC1–AC10 однозначен, имеет названный способ доказательства
+(`unit`/`browser smoke`/`golden`/«source review»/`docs diff`) и, где применимо,
+явно требует падения до фикса («красный при mutant без normalization» — AC1;
+«тест красный на коде до #218» — AC2) — это соответствует правилу «AC
+доказывает автотест, который умеет падать», а не одно предъявление.
+
+## Проверка «догадка выдана за факт»
+
+Технические утверждения ТЗ о текущем поведении сверены с кодом построчно (см.
+раздел «Как проверялось») и подтверждены, а не переписаны со слов автора
+отчёта issue. Утверждения о том, чего ТЗ **не** гарантирует (наблюдение
+Glow-base, точная конверсия quantum в метры) либо явно помечены условием
+(§10), либо являются декоративной иллюстрацией без влияния на AC (см. Low).
+Других мест, где предположение подано как решённый факт без пометки, не
+найдено.
+
+## Соответствие SCOPE.md
+
+Задача устраняет полный, беззвучный отказ Core user job J1 («живая
+пространственная картина происходящего сейчас») для целого пространства на
+любом плане, где комната когда-либо резалась/двигалась — то есть практически
+на любом реальном плане. Не расширяет продуктовый скоуп: радиус, цвет,
+opacity, blend, анимация, visibility-алгоритм, проёмы и защита источника в
+кладке явно исключены в §7 ТЗ и остаются неизменными. Не вводит новых
+настроек, новых i18n-строк и не касается раздела «Out of scope» SCOPE.md.
+
+## Соответствие TOUCH-SUPPORT.md / CONFIG-COMPATIBILITY.md
+
+Изменение — чистая геометрия рендера без новых элементов UI и взаимодействий;
+§11 ТЗ прямо фиксирует идентичный render для mouse/touch/pen в View/kiosk и
+редакторах, что не создаёт нового touch-контракта и не требует отдельной
+проверки по `TOUCH-SUPPORT.md`. §12 ТЗ фиксирует отсутствие изменений
+конфига/схемы/миграции; при отсутствии новых compatibility-полей это не
+конфликтует с `CONFIG-COMPATIBILITY.md`.
+
+## Чего не проверял
+
+- Реальная browser-геометрия и golden-рендер не запускались — на этапе `spec`
+  кода ещё нет; проверка ограничена статическим чтением текущего `src/**` и
+  сверкой заявленной цепочки вызовов с ним.
+- Не проверялась реальная величина деградации/perf-профиль `benchmark_glow.mjs`
+  — на этой стадии нет реализации для профилирования; §14 ТЗ корректно
+  ограничивает решение одним линейным проходом и переносит измерение на
+  code-review.
+- Точная форма будущего `test/physical-geometry.test.mjs` и целевого smoke
+  (расширение `smoke_glow.mjs` либо новый файл) не проверялась предметно —
+  это технический выбор автора, явно оставленный реализации (§16.2 ТЗ), и
+  ревью ТЗ не обязано фиксировать его заранее.
+- Приватный реальный экспорт владельца не запрашивался и не читался — ТЗ
+  корректно ограничивается минимизированной анонимизированной fixture (§19
+  п.4), что и требуется privacy-контрактом §12.
+
+## Вердикт
+
+Жёлтый: High 0, Medium 1 — в скоупе, чинится автором в текущем ТЗ (добавить
+раздел «Риски»), без создания отдельного issue. После правки — повторный
+цикл разбирается по дельте (§2.9/§2.10 PROCESS.md): дельта ограничена одним
+новым разделом, остальное наследуется из этого документа.