mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,196 @@
|
||||
# SPEC-REVIEW-577-r1
|
||||
|
||||
Issue: #577 «Солнечные лучи: выбирать внутренние или внешние углы окон»
|
||||
Этап: ТЗ на ревью (PROCESS.md §2.4), трек: полный (аналитика #577 явно называет
|
||||
провал критериев `small`: новый UX-контракт + новое compatibility-поле).
|
||||
Заход: r1 · блокирующих циклов израсходовано (до этого вердикта) 0 из 4.
|
||||
|
||||
## Материал ревью
|
||||
|
||||
Тело issue #577, раздел `## ТЗ`, на момент разбора — единственная редакция
|
||||
(комментарий-хендофф `Сделано: аналитика и полное ТЗ добавлены в тело issue...
|
||||
Следующий статус: S4-spec-review`, других правок после него не было). Метка на
|
||||
issue — `S4-spec-review`, `P3`, `feature`.
|
||||
|
||||
## Скоуп задачи
|
||||
|
||||
Одно глобальное compatibility-поле `settings.sun_ray_origin: 'inner' | 'outer'`,
|
||||
выбирающее грань окна, от которой строится источник уже существующих солнечных
|
||||
лучей (`docs/SUN.md`). Геометрия направления, длины, затухания, rim-линий,
|
||||
кеша, порога 3°, per-space `sun_rays`/`north_deg` не меняется — меняется только
|
||||
исходная грань пролёта. По `docs/SCOPE.md` это в рамках J1 («живой обзор дома
|
||||
на плане»): визуальный тюнинг уже одобренной (2026-08-03) фичи, не новая
|
||||
функция и не редактор.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью текстовое (этап spec, кода нет): тело issue #577 и его комментарии;
|
||||
`docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2.3/§2.4/§7.1/§2.5; канонический
|
||||
документ подсистемы `docs/SUN.md` полностью; смежный канон
|
||||
`docs/WALL-THICKNESS.md` §4–5 (Floor/fills/light/area, Sun); `docs/USER-GUIDE.ru.md`
|
||||
раздел 15 «Солнце: фон и оконные лучи» и раздел «Толстые стены»;
|
||||
`docs/CONFIG-COMPATIBILITY.md` (реестр compatibility-полей, разделы про
|
||||
`bg_mode` #146 и `plan_only` #167 как прецедент оформления новых
|
||||
compatibility-полей); `src/i18n/{ru,en,de,fr}.json` — проверка реальной строки
|
||||
`gs.sun_rays`/`space.sun_rays`, использованной в UX-разделе ТЗ, против живого
|
||||
кода (не против устаревшего текста гайда). Гейты (typecheck/test/build/golden)
|
||||
на этом этапе не гоняются — кода ещё нет, это чтение и сверка текста.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — раздел «Release-артефакты» не называет два документа, которые эта же задача обязана обновить
|
||||
|
||||
**Файл:** тело issue #577, раздел `### Release-артефакты`.
|
||||
|
||||
**Суть:** список документации к обновлению — `docs/CHANGELOG.md`/`.ru.md`,
|
||||
`docs/SUN.md`, `docs/USER-GUIDE.ru.md`, `docs/TESTING.md`, i18n, golden,
|
||||
docs screenshots. В нём отсутствуют два документа, которые прямо и однозначно
|
||||
затронуты контрактом этой задачи:
|
||||
|
||||
1. **`docs/WALL-THICKNESS.md` §5 «Sun»** — действующий канон прямо и
|
||||
безусловно формулирует ровно то поведение, которое `outer` меняет:
|
||||
«Wedges do not draw through wall bodies. Their full source span is
|
||||
translated from the centreline by half the wall depth along the
|
||||
receiving room's inward normal... Clip to the receiving room's inner
|
||||
contour.» После реализации `outer` эта фраза станет фактически неверной
|
||||
для одного из двух режимов (AC4 контракта ТЗ: «луч... проходит только
|
||||
через физический проём в теле стены» — то есть именно draws through the
|
||||
wall body). `docs/SUN.md` в разделе «Window light wedges» сам ссылается
|
||||
на `docs/WALL-THICKNESS.md` за этой геометрией («clipped by the
|
||||
receiving room's inner contour when wall thickness is set... see
|
||||
docs/WALL-THICKNESS.md»), то есть это не побочный, а прямо
|
||||
цитируемый источник контракта.
|
||||
2. **`docs/CONFIG-COMPATIBILITY.md`** — реестр каждого нового
|
||||
additive/compatibility-поля с fail-closed default; в файле уже есть
|
||||
прецедентные записи именно такого рода для `settings.bg_mode` («Four-phase
|
||||
background default and transfer (#146)») и `plan_only` («Additive
|
||||
plan-only space transfer (#167)»). Новое поле `settings.sun_ray_origin`
|
||||
— тот же класс изменения (глобальное enum-поле, `inner` по умолчанию,
|
||||
read-compatibility для отсутствующего/невалидного значения), но записи о
|
||||
нём в списке release-артефактов нет.
|
||||
|
||||
**Сценарий отказа:** исполнитель реализует задачу строго по перечисленным в
|
||||
ТЗ release-артефактам (SUN.md, USER-GUIDE.ru.md, TESTING.md, changelog) и
|
||||
проходит код-ревью. `docs/WALL-THICKNESS.md` остаётся с безусловным
|
||||
«Wedges do not draw through wall bodies» — читатель канона (следующий автор,
|
||||
следующий ревьюер по смежной задаче геометрии) получит неверную информацию о
|
||||
текущем поведении. Новое поле не появляется в реестре
|
||||
`docs/CONFIG-COMPATIBILITY.md` — следующий аудит совместимости
|
||||
(`npm run audit:config`) не будет знать о нём как о зарегистрированном
|
||||
compatibility-поле, и разрыв документации с кодом (ровно то, из-за чего в
|
||||
`docs/USER-GUIDE.ru.md` уже разошлась подпись переключателя «Солнце в окнах»
|
||||
против кода — см. «Что проверено» ниже) повторится ещё раз, уже в другом
|
||||
документе.
|
||||
|
||||
**Почему это Medium, а не High:** сама реализация и AC не блокируются —
|
||||
AC1–AC9 доказуемы независимо от того, назван ли `WALL-THICKNESS.md` в списке.
|
||||
Это дефект полноты обязательного раздела ТЗ (DoR, PROCESS.md §2.5:
|
||||
«release-артефакты названы... либо явное «нет»»), правится добавлением двух
|
||||
строк в существующий раздел, без изменения контракта, AC или скоупа —
|
||||
классический Medium-в-скоупе (§2.4, #202): чинится в этой же задаче,
|
||||
отдельный issue не заводится.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Обязательные разделы §7.1** — сценарий, «что человек увидит до/после»,
|
||||
проблема (вынесена в `## Проблема` перед `## ТЗ`, тело issue едино),
|
||||
скоуп/не-скоуп, контракт поведения, UX, модель данных и совместимость,
|
||||
i18n, критерии приёмки с доказательством, план автотестов, риски, откат,
|
||||
release-артефакты — присутствуют все.
|
||||
- **Default и fail-closed (контракт п.2)** согласован с действующим паттерном
|
||||
additive-поля в проекте (тот же приём, что `bg_mode`, `CONFIG-COMPATIBILITY.md`
|
||||
«Four-phase background default and transfer»): отсутствующее/невалидное
|
||||
значение → `inner`, без визуальной миграции существующих установок —
|
||||
корректно и проверяемо (AC2).
|
||||
- **Геометрия `inner` (контракт п.3)** дословно совпадает с текущим каноном
|
||||
`docs/WALL-THICKNESS.md` §5 и `docs/SUN.md` («translated from the centreline
|
||||
by half the wall depth along the receiving room's inward normal») —
|
||||
описание существующего поведения не искажено.
|
||||
- **Терминология UX-раздела.** ТЗ ссылается на переключатель «Солнце в окнах»
|
||||
непосредственно перед новым select. Строка `«Солнце в окнах»` — это ТОЧНОЕ
|
||||
совпадение с живым `gs.sun_rays`/`space.sun_rays` в `src/i18n/ru.json`, а не
|
||||
с текстом `docs/USER-GUIDE.ru.md` (там уже устаревшее «Солнечный свет через
|
||||
окна» — расхождение гайда с кодом, но не эта задача его создала и не в
|
||||
скоупе #577 его чинить; отдельный issue не заводится, находка ниже порога
|
||||
«завести отдельно» — тривиальная, чисто документная стилистика, не
|
||||
затрагивающая ничьё поведение). Термин «оконный тоннель» в AC4 тоже взят из
|
||||
живого текста гайда (`docs/USER-GUIDE.ru.md` раздел «Толстые стены»:
|
||||
«Солнечный луч начинается из внутренних углов оконного тоннеля») — не
|
||||
изобретён.
|
||||
- **Технический риск наружного источника явно назван самим автором** —
|
||||
«текущее clipping по внутреннему контуру комнаты способно молча обрезать
|
||||
участок от наружной до внутренней грани» в `### Риски`, и он же положен в
|
||||
AC4 (`golden` + `smoke`) как обязанность доказать проходимость через
|
||||
«физический проём» — примитив уже существует в кодовой базе
|
||||
(`opening-tunnel`, `data-kind: window`, `docs/WALL-THICKNESS.md` §4: «Glow
|
||||
leaving through a door uses the clear rectangular opening tunnel»,
|
||||
`docs/STYLING-HOOKS.md`), то есть AC4 не требует придумывать новую
|
||||
геометрию с нуля — она обоснованно достижима переиспользованием уже
|
||||
задокументированного механизма. Не находка: это техническая
|
||||
реализуемость, зона решения автора/код-ревью (PROCESS.md §7.1: «всё, чего
|
||||
пользователь не наблюдает, агенты решают сами»), а не продукт-вопрос
|
||||
владельцу.
|
||||
- **«Принятые предположения»** корректно ловят единственную реальную
|
||||
продуктовую неоднозначность: удлинение `len` от новой (внешней) исходной
|
||||
точки означает, что видимая длина луча ВНУТРИ комнаты у толстой стены в
|
||||
`outer` физически короче, чем в `inner`, при одинаковом номинальном `len`
|
||||
(AC5). Это явно зафиксировано как принятое решение, а не молча
|
||||
подразумевается — соответствует формату «принято предположительно, поменять
|
||||
свободно» (PROCESS.md §7.1).
|
||||
- **Не-скоуп** корректно исключает per-space override, изменение азимута/
|
||||
направления/длины формулы, новые режимы освещения и правки редакторов —
|
||||
граница не размыта.
|
||||
- **AC1–AC9** пронумерованы, у каждого указан способ доказательства
|
||||
(`unit`/`backend`/`smoke`/`golden`/«ревью кода»), формулировки проверяемы
|
||||
(конкретные геометрические и UI-утверждения, не «работает корректно»).
|
||||
- **i18n** — уровень детализации (русские тексты трёх строк + перечисление
|
||||
языков) соответствует принятой в архиве практике (сверено с
|
||||
`docs/specs/487-room-temperature-thresholds.md` §12: там тоже названы
|
||||
назначения ключей без итоговых английских строк) — не находка.
|
||||
- **Продуктовая рамка** — сценарий и «что человек увидит» отвечают на оба
|
||||
обязательных вопроса §7.1 одной фразой без терминов реализации; открытых
|
||||
продуктовых вопросов владельцу нет, и это оправдано: единственные реальные
|
||||
неоднозначности закрыты явным блоком предположений, который ревьюер вправе
|
||||
оспорить (см. предыдущий пункт) — не пустая формальность «решать нечего».
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверял `scripts/config-field-registry.mjs` построчно — только сверил
|
||||
наличие раздела-прецедента для аналогичных полей (`bg_mode`, `plan_only`) в
|
||||
`docs/CONFIG-COMPATIBILITY.md`.
|
||||
- Не проверял `custom_components/houseplan/validation.py` на предмет
|
||||
конкретного места добавления enum-валидации — это техническая деталь
|
||||
реализации, не предмет ревью ТЗ.
|
||||
- Не оценивал фактическую сложность реализации AC4 в `src/sun.ts`
|
||||
(доступность геометрии «физического проёма» именно в текущей сигнатуре
|
||||
`computeSunRays()`) — это код-ревью и локальные гейты следующего этапа,
|
||||
риск уже явно назван автором в ТЗ и покрыт `golden`/`smoke` в AC4.
|
||||
- Не запускал никаких гейтов (typecheck/test/build/golden) — на этапе
|
||||
ревью ТЗ кода нет, гонять нечего.
|
||||
- Английские/немецкие/французские тексты i18n не сформулированы (только
|
||||
предмет ключей) — это ожидаемый на этом этапе уровень детализации, не
|
||||
находка (см. выше).
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый. AC полностью выполнимы и проверяемы, продуктовая рамка и контракт
|
||||
поведения корректны и опираются на реальный канон, а не на догадку; единственная
|
||||
находка — Medium в скоупе задачи (неполный список release-артефактов, два
|
||||
канонических документа не названы для обновления). High-находок нет, отдельный
|
||||
issue не заводится.
|
||||
|
||||
`Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче`
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/577-sun-ray-window-corners`, коммит `1df463d7cfe5` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `30be6a8ac43a2c27e5536bfb51f1599b6396628d`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 30be6a8ac43a
|
||||
```
|
||||
- Тело issue: `3263b8c8be2272a314f407b5fa5713fa93150e3c1d59cf2de100dd640f46915a`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user