From 8f4ce99cb6e51d297303aeb2d17b6f4939ccc521 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 15 Sep 2026 04:59:57 +0000 Subject: [PATCH] docs: review document for #580 Issue: #580 User-Visible: no --- docs/reviews/SPEC-REVIEW-580-r1.md | 223 +++++++++++++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-580-r1.md diff --git a/docs/reviews/SPEC-REVIEW-580-r1.md b/docs/reviews/SPEC-REVIEW-580-r1.md new file mode 100644 index 00000000..7966a8f2 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-580-r1.md @@ -0,0 +1,223 @@ +# SPEC-REVIEW-580-r1 + +**Issue:** #580 «Солнечные лучи от внешних углов проходят сквозь толстые стены» +**Этап:** ТЗ на ревью (`S4-spec-review`), трек — полный продуктовый +**Заход:** r1 · блокирующих циклов израсходовано 0/4 (лимит для полного трека — 4) +**Материал:** тело issue #580, раздел `## ТЗ`, на момент публикации комментария +«Передача ТЗ на ревью» (2026-09-15T04:52:39Z), ветка `issue/580-outer-sun-wall-occlusion` +от `d37b2c9f8a4369082fec6970117cc97dd1e303c7` без продуктовых изменений. + +## Скоуп + +Задача чинит регрессию в геометрии солнечных лучей режима `sun_ray_origin: outer` +(#577): на толстой наружной стене с окном при косом положении солнца +`computeSunRays()` строит комнатную часть луча пересечением ПОЛНОГО внешнего +пролёта окна с контуром комнаты, а не с интервалом, реально прошедшим сквозь +оконный туннель — откосы стены не отсекают лишнюю ширину, и часть заливки +визуально идёт сквозь тело стены. Правка ограничена `src/sun.ts` и его тестами; +UI, i18n, схема конфигурации и touch-контракт не затрагиваются согласно самому +ТЗ. + +Job по `docs/SCOPE.md`: J1/J5 (свет как часть спатиального обзора) не при +чём напрямую — window rays это отдельный decorative-визуал, но исправление +физически неверного визуала — прямая работа над качеством уже принятой фичи +#577, а не новая функциональность вне списка задач. Отклонений от SCOPE.md не +найдено. + +## Как проверялось + +1. Прочитан `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1–§10, до строки 871 + первой порции + смежные разделы §2.4/§2.5/§2.10/§7.1/§7.2). +2. Прочитано тело issue #580 (шапка бага + `## ТЗ`) и оба комментария + («Аналитика», «Передача ТЗ на ревью») через `gh issue view --json`. +3. Прочитан канонический документ подсистемы `docs/SUN.md` целиком, включая + раздел «Window light wedges — `settings.sun_ray_origin`» и список файлов. +4. Прочитан раздел «Лучи солнца» `docs/USER-GUIDE.ru.md` (строки ~1634–1665): + таблица уже содержит строку «Толстая внешняя стена, режим «внешние углы» — + Луч начинается с фасадной стороны и проходит к комнате только внутри + оконного тоннеля» — то есть **пользовательский текст уже описывает + исправленное поведение**, реализация ему пока не соответствует. +5. Прочитан код `computeSunRays()` в `src/sun.ts` (строки 402–459) построчно, + чтобы проверить, что диагноз ТЗ («второе пересечение снова допускает в + комнату всю исходную ширину луча») соответствует фактическому коду, а не + догадке: `polys = clipToRoom(quad, clipPoly)` действительно клипует ПОЛНЫЙ + `quad` (от внешнего пролёта) напрямую по контуру комнаты, а туннельная + часть добавляется ОТДЕЛЬНЫМ независимым клипом (`if (origin === 'outer' && + d > 0) { … polys.push(...) }`) — комнатная и туннельная части не связаны + пересечением друг с другом. Диагноз подтверждён чтением, не исполнением. +6. Прочитан существующий тест `#577` в `test/sun.test.mjs:468` — он проверяет + `outer`-режим при азимуте 270°/угле окна 90° (перпендикулярное падение), + поэтому не мог поймать именно эту регрессию на косом солнце. Технический + подход ТЗ (разделить туннельную и комнатную часть, для комнатной считать + траектории, прошедшие оба пролёта) реализуем поверх уже существующих + входов функции (`wallDepthByOpening`, координаты окна, контур комнаты) — + новых геометрических данных не требует. +7. Подтверждено существование `demo/smoke_sun.mjs` и `demo/golden/matrix.mjs`, + на которые ссылается план тестов AC4/AC5. + +Ручного тестирования UI не проводилось — этап ТЗ, кода ещё нет +(`issue/580-outer-sun-wall-occlusion` без продуктовых изменений). + +## Находки + +### Medium (в скоупе задачи — правится в том же ТЗ) + +**M1. ТЗ не отвечает на несколько обязательных пунктов §7.1 / чек-листа DoR +§2.5, оставляя их фактически открытыми, хотя ответ на каждый тривиален.** + +Отсутствуют как явные утверждения: + +- **Сценарий** (какая персона/поверхность/момент, `docs/SCOPE.md`) и **что + человек увидит до и после** одной фразой без терминов реализации — первые + два обязательных раздела ТЗ идут первыми не случайно (`PROCESS.md` §7.1). + Раздел `## ТЗ` начинается сразу с «Проблема» в терминах реализации + (`sun_ray_origin: outer`, «откосы», «внутренняя плоскость»). Материал для + этих двух разделов есть в шапке issue («Ожидаемое поведение»), но она тоже + сформулирована в терминах реализации и не перенесена в ТЗ как отдельный + продуктовый раздел. +- **i18n** — DoR требует «ключи en + ru перечислены»; в ТЗ нет даже строки + «нет новых ключей», хотя ответ очевиден. +- **Влияние на touch** (`docs/TOUCH-SUPPORT.md`, View и киоск — блокирующие + пункты DoR) — не упомянуто, хотя window rays рендерятся именно в View/ + киоске, для которых touch-совместимость обязательна к проверке по чек-листу. +- **Влияние на производительность** — не упомянуто, хотя правка меняет + горячий путь `computeSunRays()` (пересчёт при каждом изменении + азимута/элевации, см. `docs/SUN.md`: «Sun geometry is recomputed ONLY when + … change» — то есть путь всё равно активный и не редкий) добавлением + дополнительного пересечения полигонов. DoR требует «названо (или явно + «нет»)» — сейчас не названо никак. +- **Риски** как раздел самого ТЗ — риски перечислены только в отдельном + комментарии «Аналитика» (сложность 5/10, риск 6/10, «важно не изменить + режим внутренних углов, прямое падение, нулевую толщину и fade/rim»), но не + перенесены в тело `## ТЗ`. Аналитика и ТЗ — разные артефакты процесса + (§2.2 vs §2.3); DoR (§2.5) требует риски именно при переходе в «Готово к + разработке», то есть от ТЗ, а не от предшествующего комментария аналитика. + +Почему это Medium, а не Low: без явных ответов на эти пункты DoR-чек-лист +§2.5 формально не проходится ни по одному из пяти перечисленных пунктов — +не потому, что ответы сложны (все пять тривиальны — «нет» или короткая +фраза), а потому что их сейчас в ТЗ просто нет, и переход в `S5-ready` +по букве процесса не может считаться завершённым. Правка дешёвая: добавить +пять коротких строк/раздел, не переписывая контракт и AC. + +Как закрыть: добавить в тело `## ТЗ` явные строки: сценарий (персона, +поверхность View/kiosk, момент — низкое косое солнце), одну фразу «что +человек увидит» без терминов реализации, `i18n: нет новых ключей`, `touch: +не меняется (View/kiosk рендерят как раньше, взаимодействия нет)`, +`производительность: доп. пересечение полигона в уже пересчитываемом на +каждое изменение азимута/элевации пути, без нового порядка сложности — +либо явную оценку, если авторы считают иначе`, и короткий раздел «Риски» +(перенести формулировку из комментария аналитика). + +### Low (на усмотрение автора — можно поправить или снять с записью) + +**L1. Контракт п.4 использует немодальную формулировку («может оставаться +видимым»), которую ни один AC не проверяет.** + +> «Если направление настолько косое, что ни один луч не проходит через оба +> пролёта, комнатная часть отсутствует; свет внутри доступной части туннеля +> **может** оставаться видимым.» + +AC2 проверяет только пустоту комнатной части в этом случае +(«при предельно косом направлении без сквозной видимости комнатная часть +пуста»), но не утверждает и не проверяет, остаётся ли туннельная часть +освещённой. Формулировка «может» уместна как честное признание +неопределённости (поведение туннельной части не входит в эту правку и, +по всей видимости, не меняется относительно уже принятого #577 +поведения), но в разделе «Контракт», который по определению должен +описывать однозначное поведение, такая модальность читается как +недорешённый пункт. Не блокирует: последствия ограничены тем самым +пограничным случаем, который вне зависимости от трактовки не влияет на +AC1–AC3 (типичные и предельные-но-не-нулевые углы) и не расширяет скоуп. +Рекомендация: либо явно пометить как «поведение вне контракта данной задачи, +не меняется» (аналогично формулировке п.6 про взаимное затенение), либо +удалить хеджирующее слово и добавить проверку в AC4 (там уже есть +контрольная точка «за закрытым откосом»). + +## Что проверено и корректно + +- **Диагноз бага соответствует коду**, а не является догадкой: строки + 423/440–450 `src/sun.ts` подтверждают, что комнатная и туннельная части + луча в режиме `outer` считаются двумя независимыми клипами одного и того + же полного `quad`, без пересечения друг с другом — именно это описывает + «Причина» в комментарии «Аналитика» и «Проблема» в ТЗ. +- **Технический подход реализуем на существующих входных данных** + (`wallDepthByOpening`, координаты/угол/длина окна, контур комнаты) — не + требует новой геометрии стен (откосов, рам) сверх уже используемой глубины + `d`. Заявление «принято предположительно» про отсутствие моделирования + скошенных откосов и отдельной глубины рамы согласуется с тем, что этой + геометрии в проекте вообще нет ни для одного другого сценария (глубина + считается прямоугольным туннелем и в текущем, не только багованном, коде). +- **Регрессионный периметр назван и обоснован верно**: контракт п.3 (нет шва + между туннелем и комнатной частью, полная ширина при перпендикулярном + падении), п.5 (`inner`, нулевая толщина, направление/длина/fade/rim не + меняются) точно называют места, где чинить проще всего сломать — они прямо + соответствуют существующим тестам `#577` (`test/sun.test.mjs:468,495`) и + `DEV-EB173-01` (`test/sun.test.mjs:509`), которые эта задача не должна + регрессировать. +- **`docs/USER-GUIDE.ru.md` не нуждается в правке** — таблица «Поведение» уже + описывает целевое (исправленное), а не текущее ошибочное поведение + режима «внешние углы» (строка «...проходит к комнате только внутри + оконного тоннеля»). Хорошо, что автор не стал ничего менять в + пользовательской документации, поскольку менять там нечего; отдельно + отмечаю здесь как проверенный факт, а не находку. +- **AC1–AC7 однозначны и каждому указан способ доказательства** (`unit` ×3, + `browser smoke`, `golden`, гейты, `review`) — ни один AC не описывает + реализацию вместо наблюдаемого результата (AC1/AC2/AC4/AC5 говорят о + видимых/измеримых геометрических свойствах луча, AC3 — о регрессионной + идентичности, AC6/AC7 — гейты и объём диффа). +- **«Принятые предположения» оформлены по правилу §7.1** явным блоком, а не + вписаны в контракт как факт — ревьюер может их оспорить, оба технические + (не продуктовые), эскалация владельцу не требуется. +- **Не-скоуп назван явно и совпадает с ранее принятым лимитом** SUN.md + («mutual shading… NOT computed»): контракт п.6 не пытается расширить + задачу на взаимное затенение стен/крыльев. +- Track-решение («полный трек», не `small`) обосновано названным критерием + (сложность/риск выше 3, меняется геометрический алгоритм) — соответствует + требованию §2.2 называть нарушенный критерий, а не писать «обычный трек» + без обоснования. + +## Чего не проверял + +- Не проверял `npx tsc --noEmit` / `npm test` / `npm run build` — на этапе + ТЗ продуктового кода ещё нет (ветка `issue/580-outer-sun-wall-occlusion` + без изменений от `d37b2c9f`), гонять гейты не над чем. +- Не выполнял браузерные смоки и golden — они станут актуальны на код-ревью, + когда появится реализация AC1–AC5. +- Не проверял `python -m pytest tests_backend` — задача не касается + `custom_components/**/*.py`. +- Не пересчитывал производительность численно (нет кода) — отметил как + находку M1 отсутствие даже качественной оценки в тексте ТЗ. + +## Вердикт + +Жёлтый: High-находок нет, но найден Medium в скоупе задачи (M1) — несколько +обязательных пунктов чек-листа §7.1/DoR §2.5 не имеют явного ответа в теле +`## ТЗ`, хотя ответ на каждый тривиален и не меняет контракт/AC. Технический +разбор задачи (диагноз, подход, критерии приёмки) проверен по коду и +корректен; возврат нужен только чтобы явно закрыть пропущенные разделы, а не +чтобы переписывать решение. + +## Материал раунда + +- Issue: #580, репозиторий Matysh/houseplan-card. +- Материал ТЗ: тело issue, раздел `## ТЗ`, на момент комментария + `IC_kwDOTOcLQM8AAAABUkBntg` (2026-09-15T04:52:39Z, «Передача ТЗ на ревью»). +- Ветка `issue/580-outer-sun-wall-occlusion`, `HEAD = d37b2c9f8a4369082fec6970117cc97dd1e303c7` + (без продуктовых изменений на момент ревью). +- Заход r1, документ первый; предыдущих раундов нет — разделы «Закрытие + раунда r0» и «Унаследовано из r0» не применимы. + +--- + + + +## Материал раунда + +- Ветка: `issue/580-outer-sun-wall-occlusion`, коммит `d37b2c9f8a43` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `cd142c70cdb0ab25b12dd4fa2a77c9d955f01940` + ``` + git log --all --format='%H %T' | grep cd142c70cdb0 + ``` +- Тело issue: `25c7fa5fa2430ab0ce56a0f7684ab0fb192b932a209aba248b951ddd82189135` +- Вердикт конвейера: `yellow` · High 0