From f9accc11981c6c5eeed9328b445da4d7d7c04f4d Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 10:41:59 +0000 Subject: [PATCH] docs: review document for #218 Issue: #218 User-Visible: no --- docs/reviews/SPEC-REVIEW-218-r1.md | 216 +++++++++++++++++++++++++++++ 1 file changed, 216 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-218-r1.md diff --git a/docs/reviews/SPEC-REVIEW-218-r1.md b/docs/reviews/SPEC-REVIEW-218-r1.md new file mode 100644 index 00000000..d0625153 --- /dev/null +++ b/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): дельта ограничена одним +новым разделом, остальное наследуется из этого документа.