mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
committed by
Sergey Matyunin
parent
e54ff33ca1
commit
8f4ce99cb6
@@ -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» не применимы.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/580-outer-sun-wall-occlusion`, коммит `d37b2c9f8a43` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `cd142c70cdb0ab25b12dd4fa2a77c9d955f01940`
|
||||
```
|
||||
git log --all --format='%H %T' | grep cd142c70cdb0
|
||||
```
|
||||
- Тело issue: `25c7fa5fa2430ab0ce56a0f7684ab0fb192b932a209aba248b951ddd82189135`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user