diff --git a/docs/reviews/SPEC-REVIEW-577-r1.md b/docs/reviews/SPEC-REVIEW-577-r1.md new file mode 100644 index 00000000..f8366a0f --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-577-r1.md @@ -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 → в задаче` + +--- + + + +## Материал раунда + +- Ветка: `issue/577-sun-ray-window-corners`, коммит `1df463d7cfe5` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `30be6a8ac43a2c27e5536bfb51f1599b6396628d` + ``` + git log --all --format='%H %T' | grep 30be6a8ac43a + ``` +- Тело issue: `3263b8c8be2272a314f407b5fa5713fa93150e3c1d59cf2de100dd640f46915a` +- Вердикт конвейера: `yellow` · High 0